[lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

Christian Brauner christianvanbrauner at gmail.com
Fri Oct 30 15:21:42 UTC 2015


On Oct 30, 2015 4:08 PM, "Serge Hallyn" <serge.hallyn at ubuntu.com> wrote:
>
> Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > This functions updates absolute paths for overlay upper- and workdirs
so users
> > can simply clone and start new containers without worrying about
absolute paths
> > in lxc.mount.entry overlay entries.
> >
> > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> > ---
> >  src/lxc/confile.c | 103
++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/lxc/confile.h |   3 ++
> >  2 files changed, 106 insertions(+)
> >
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index f7d6814..bc5b7ef 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -2598,6 +2598,109 @@ void clear_unexp_config_line(struct lxc_conf
*conf, const char *key, bool rm_sub
> >       }
> >  }
> >
> > +bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char
*oldpath,
> > +                             const char *newpath, const char *oldname,
> > +                             const char *newname, const char *ovldir)
> > +{
> > +     const char *key = "lxc.mount.entry";
> > +     int ret;
> > +     char *lstart = conf->unexpanded_config;
> > +     char *lend;
> > +     char *p;
> > +     char *q;
> > +     size_t newdirlen = strlen(ovldir) + strlen(newpath) +
strlen(newname) + 2;
> > +     size_t olddirlen = strlen(ovldir) + strlen(oldpath) +
strlen(oldname) + 2;
> > +     char *olddir = alloca(olddirlen + 1);
> > +     char *newdir = alloca(newdirlen + 1);
> > +
> > +     ret = snprintf(olddir, olddirlen + 1, "%s=%s/%s", ovldir,
oldpath, oldname);
> > +     if (ret < 0 || ret >= olddirlen + 1) {
> > +             ERROR("Bug in %s", __func__);
> > +             return false;
> > +     }
> > +     ret = snprintf(newdir, newdirlen + 1, "%s=%s/%s", ovldir,
newpath, newname);
> > +     if (ret < 0 || ret >= newdirlen + 1) {
> > +             ERROR("Bug in %s", __func__);
> > +             return false;
> > +     }
> > +     if (!conf->unexpanded_config)
> > +             return true;
> > +     while (*lstart) {
> > +             lend = strchr(lstart, '\n');
> > +             if (!lend)
> > +                     lend = lstart + strlen(lstart);
> > +             else
> > +                     lend++;
> > +             if (strncmp(lstart, key, strlen(key)) != 0) {
> > +                     lstart = lend;
> > +                     continue;
> > +             }
> > +             p = strchr(lstart + strlen(key), '=');
> > +             if (!p) {
> > +                     lstart = lend;
> > +                     continue;
> > +             }
> > +             p++;
> > +             while (isblank(*p))
> > +                     p++;
> > +             if (!*p)
> > +                     return true;
>
> I think you should
>
>                 if (p >= lend)
>                         continue;
>
> (and then you can skip the !*p check above as this should catch that)
>
> What you have will only get confused on invalid configs, but this would
> be stricter and make me feel better.

Agreed. Than we should change that in the other function
(clone_update_unexp_hooks()) as well.

>
> > +             /* We check that when an lxc.mount.entry is found the
substrings
> > +              * " overlay " or " aufs " are present before we try to
update
> > +              * the line. This seems like a bit more safety. We can
check for
> > +              * " overlay " and " aufs " since both substrings need to
have
> > +              * at least one space before and after them. When the
space
> > +              * before or after is missing it is very likely that these
> > +              * substrings are part of a path or something else. So we
> > +              * shouldn't bother to do any further work...
> > +              */
> > +             if (!strstr(p, " overlay ") && !strstr(p, " aufs ")) {
> > +                     lstart = lend;
> > +                     continue;
> > +             }
>
> But this will find " overlay " in any subsequent lines too right?  Don't
you
> need to limit your search to the line?  Maybe by saving *lend and setting
> *lend = '\0'?

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.

>
> > +             if (!(q = strstr(p, olddir))) {
> > +                     lstart = lend;
> > +                     continue;
> > +             }
> > +
> > +             /* replace the olddir with newdir */
> > +             if (olddirlen >= newdirlen) {
> > +                     size_t diff = olddirlen - newdirlen;
> > +                     memcpy(q, newdir, newdirlen);
> > +                     if (olddirlen != newdirlen) {
> > +                             memmove(q + newdirlen, q + newdirlen +
diff,
> > +                                     strlen(q) - newdirlen - diff + 1);
> > +                             lend -= diff;
> > +                             conf->unexpanded_len -= diff;
> > +                     }
> > +                     lstart = lend;
> > +             } else {
> > +                     char *new;
> > +                     size_t diff = newdirlen - olddirlen;
> > +                     size_t oldlen = conf->unexpanded_len;
> > +                     size_t newlen = oldlen + diff;
> > +                     size_t poffset = q - conf->unexpanded_config;
> > +                     new = realloc(conf->unexpanded_config, newlen +
1);
> > +                     if (!new) {
> > +                             ERROR("Out of memory");
> > +                             return false;
> > +                     }
> > +                     conf->unexpanded_len = newlen;
> > +                     conf->unexpanded_alloced = newlen + 1;
> > +                     new[newlen-1] = '\0';
> > +                     lend = new + (lend - conf->unexpanded_config);
> > +                     /* move over the remainder to make room for the
newdir */
> > +                     memmove(new + poffset + newdirlen,
> > +                             new + poffset + olddirlen,
> > +                             oldlen - poffset - olddirlen + 1);
> > +                     conf->unexpanded_config = new;
> > +                     memcpy(new + poffset, newdir, newdirlen);
> > +                     lstart = lend + diff;
>
> Heh, I *think* the above bit is all right.

What do you mean? My calculations or the old one? :)

>
> > +             }
> > +     }
> > +     return true;
> > +}
> > +
> >  bool clone_update_unexp_hooks(struct lxc_conf *conf, const char
*oldpath,
> >       const char *newpath, const char *oldname, const char *newname)
> >  {
> > diff --git a/src/lxc/confile.h b/src/lxc/confile.h
> > index 8da3699..66af832 100644
> > --- a/src/lxc/confile.h
> > +++ b/src/lxc/confile.h
> > @@ -61,5 +61,8 @@ extern bool do_append_unexp_config_line(struct
lxc_conf *conf, const char *key,
> >  extern void clear_unexp_config_line(struct lxc_conf *conf, const char
*key, bool rm_subkeys);
> >  extern bool clone_update_unexp_hooks(struct lxc_conf *conf, const char
*oldpath,
> >       const char *newpath, const char *oldname, const char *newmame);
> > +bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char
*oldpath,
> > +                             const char *newpath, const char *oldname,
> > +                             const char *newname, const char *ovld);
> >  extern bool network_new_hwaddrs(struct lxc_conf *conf);
> >  #endif
> > --
> > 2.6.2
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20151030/25c18324/attachment.html>


More information about the lxc-devel mailing list