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

2015-10-30 Thread Christian Brauner 
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 
---
 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;
+   /* 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;
+   }
+   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 +

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

2015-10-30 Thread Christian Brauner 
On Oct 30, 2015 4:08 PM, "Serge Hallyn"  wrote:
>
> Quoting Christian Brauner (christianvanbrau...@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 
> > ---
> >  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 -=

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

2015-10-30 Thread Serge Hallyn 
Quoting Christian Brauner (christianvanbrau...@gmail.com):
> On Oct 30, 2015 4:08 PM, "Serge Hallyn"  wrote:
> >
> > Quoting Christian Brauner (christianvanbrau...@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 
> > > ---
> > >  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.

Right, so the you set *lend back to the saved *lend before going to the
top of the loop - there's someplace else i lxc that does this.
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel