<p dir="ltr"><br>
On Oct 30, 2015 4:08 PM, "Serge Hallyn" <<a href="mailto:serge.hallyn@ubuntu.com">serge.hallyn@ubuntu.com</a>> wrote:<br>
><br>
> Quoting Christian Brauner (<a href="mailto:christianvanbrauner@gmail.com">christianvanbrauner@gmail.com</a>):<br>
> > This functions updates absolute paths for overlay upper- and workdirs so users<br>
> > can simply clone and start new containers without worrying about absolute paths<br>
> > in lxc.mount.entry overlay entries.<br>
> ><br>
> > Signed-off-by: Christian Brauner <<a href="mailto:christianvanbrauner@gmail.com">christianvanbrauner@gmail.com</a>><br>
> > ---<br>
> >  src/lxc/confile.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++++++<br>
> >  src/lxc/confile.h |   3 ++<br>
> >  2 files changed, 106 insertions(+)<br>
> ><br>
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c<br>
> > index f7d6814..bc5b7ef 100644<br>
> > --- a/src/lxc/confile.c<br>
> > +++ b/src/lxc/confile.c<br>
> > @@ -2598,6 +2598,109 @@ void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_sub<br>
> >       }<br>
> >  }<br>
> ><br>
> > +bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char *oldpath,<br>
> > +                             const char *newpath, const char *oldname,<br>
> > +                             const char *newname, const char *ovldir)<br>
> > +{<br>
> > +     const char *key = "lxc.mount.entry";<br>
> > +     int ret;<br>
> > +     char *lstart = conf->unexpanded_config;<br>
> > +     char *lend;<br>
> > +     char *p;<br>
> > +     char *q;<br>
> > +     size_t newdirlen = strlen(ovldir) + strlen(newpath) + strlen(newname) + 2;<br>
> > +     size_t olddirlen = strlen(ovldir) + strlen(oldpath) + strlen(oldname) + 2;<br>
> > +     char *olddir = alloca(olddirlen + 1);<br>
> > +     char *newdir = alloca(newdirlen + 1);<br>
> > +<br>
> > +     ret = snprintf(olddir, olddirlen + 1, "%s=%s/%s", ovldir, oldpath, oldname);<br>
> > +     if (ret < 0 || ret >= olddirlen + 1) {<br>
> > +             ERROR("Bug in %s", __func__);<br>
> > +             return false;<br>
> > +     }<br>
> > +     ret = snprintf(newdir, newdirlen + 1, "%s=%s/%s", ovldir, newpath, newname);<br>
> > +     if (ret < 0 || ret >= newdirlen + 1) {<br>
> > +             ERROR("Bug in %s", __func__);<br>
> > +             return false;<br>
> > +     }<br>
> > +     if (!conf->unexpanded_config)<br>
> > +             return true;<br>
> > +     while (*lstart) {<br>
> > +             lend = strchr(lstart, '\n');<br>
> > +             if (!lend)<br>
> > +                     lend = lstart + strlen(lstart);<br>
> > +             else<br>
> > +                     lend++;<br>
> > +             if (strncmp(lstart, key, strlen(key)) != 0) {<br>
> > +                     lstart = lend;<br>
> > +                     continue;<br>
> > +             }<br>
> > +             p = strchr(lstart + strlen(key), '=');<br>
> > +             if (!p) {<br>
> > +                     lstart = lend;<br>
> > +                     continue;<br>
> > +             }<br>
> > +             p++;<br>
> > +             while (isblank(*p))<br>
> > +                     p++;<br>
> > +             if (!*p)<br>
> > +                     return true;<br>
><br>
> I think you should<br>
><br>
>                 if (p >= lend)<br>
>                         continue;<br>
><br>
> (and then you can skip the !*p check above as this should catch that)<br>
><br>
> What you have will only get confused on invalid configs, but this would<br>
> be stricter and make me feel better.</p>
<p dir="ltr">Agreed. Than we should change that in the other function (clone_update_unexp_hooks()) as well.</p>
<p dir="ltr">><br>
> > +             /* We check that when an lxc.mount.entry is found the substrings<br>
> > +              * " overlay " or " aufs " are present before we try to update<br>
> > +              * the line. This seems like a bit more safety. We can check for<br>
> > +              * " overlay " and " aufs " since both substrings need to have<br>
> > +              * at least one space before and after them. When the space<br>
> > +              * before or after is missing it is very likely that these<br>
> > +              * substrings are part of a path or something else. So we<br>
> > +              * shouldn't bother to do any further work...<br>
> > +              */<br>
> > +             if (!strstr(p, " overlay ") && !strstr(p, " aufs ")) {<br>
> > +                     lstart = lend;<br>
> > +                     continue;<br>
> > +             }<br>
><br>
> But this will find " overlay " in any subsequent lines too right?  Don't you<br>
> need to limit your search to the line?  Maybe by saving *lend and setting<br>
> *lend = '\0'?</p>
<p dir="ltr">But we do want to check in all lines, don't we? If we do not find the substring in the current line we skip it and check the next one.</p>
<p dir="ltr">><br>
> > +             if (!(q = strstr(p, olddir))) {<br>
> > +                     lstart = lend;<br>
> > +                     continue;<br>
> > +             }<br>
> > +<br>
> > +             /* replace the olddir with newdir */<br>
> > +             if (olddirlen >= newdirlen) {<br>
> > +                     size_t diff = olddirlen - newdirlen;<br>
> > +                     memcpy(q, newdir, newdirlen);<br>
> > +                     if (olddirlen != newdirlen) {<br>
> > +                             memmove(q + newdirlen, q + newdirlen + diff,<br>
> > +                                     strlen(q) - newdirlen - diff + 1);<br>
> > +                             lend -= diff;<br>
> > +                             conf->unexpanded_len -= diff;<br>
> > +                     }<br>
> > +                     lstart = lend;<br>
> > +             } else {<br>
> > +                     char *new;<br>
> > +                     size_t diff = newdirlen - olddirlen;<br>
> > +                     size_t oldlen = conf->unexpanded_len;<br>
> > +                     size_t newlen = oldlen + diff;<br>
> > +                     size_t poffset = q - conf->unexpanded_config;<br>
> > +                     new = realloc(conf->unexpanded_config, newlen + 1);<br>
> > +                     if (!new) {<br>
> > +                             ERROR("Out of memory");<br>
> > +                             return false;<br>
> > +                     }<br>
> > +                     conf->unexpanded_len = newlen;<br>
> > +                     conf->unexpanded_alloced = newlen + 1;<br>
> > +                     new[newlen-1] = '\0';<br>
> > +                     lend = new + (lend - conf->unexpanded_config);<br>
> > +                     /* move over the remainder to make room for the newdir */<br>
> > +                     memmove(new + poffset + newdirlen,<br>
> > +                             new + poffset + olddirlen,<br>
> > +                             oldlen - poffset - olddirlen + 1);<br>
> > +                     conf->unexpanded_config = new;<br>
> > +                     memcpy(new + poffset, newdir, newdirlen);<br>
> > +                     lstart = lend + diff;<br>
><br>
> Heh, I *think* the above bit is all right.</p>
<p dir="ltr">What do you mean? My calculations or the old one? :)</p>
<p dir="ltr">><br>
> > +             }<br>
> > +     }<br>
> > +     return true;<br>
> > +}<br>
> > +<br>
> >  bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,<br>
> >       const char *newpath, const char *oldname, const char *newname)<br>
> >  {<br>
> > diff --git a/src/lxc/confile.h b/src/lxc/confile.h<br>
> > index 8da3699..66af832 100644<br>
> > --- a/src/lxc/confile.h<br>
> > +++ b/src/lxc/confile.h<br>
> > @@ -61,5 +61,8 @@ extern bool do_append_unexp_config_line(struct lxc_conf *conf, const char *key,<br>
> >  extern void clear_unexp_config_line(struct lxc_conf *conf, const char *key, bool rm_subkeys);<br>
> >  extern bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,<br>
> >       const char *newpath, const char *oldname, const char *newmame);<br>
> > +bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char *oldpath,<br>
> > +                             const char *newpath, const char *oldname,<br>
> > +                             const char *newname, const char *ovld);<br>
> >  extern bool network_new_hwaddrs(struct lxc_conf *conf);<br>
> >  #endif<br>
> > --<br>
> > 2.6.2<br>
> ><br>
</p>