Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Christian Brauner
On Mon, Nov 02, 2015 at 02:38:16PM +, 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 | 102 
> > ++
> >  src/lxc/confile.h |   3 ++
> >  2 files changed, 105 insertions(+)
> > 
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index f7d6814..47b0b8b 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf *conf, 
> > const char *key, bool rm_sub
> > }
> >  }
> >  
> > +bool clone_update_unexp_ovl_paths(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 >= lend)
> > +continue;
> > +/* Whenever an lxc.mount.entry entry is found in a line we 
> > check
> > +*  if the substring " overlay" or the substring " aufs" is
> > +*  present before doing any further work. We check for "
> > +*  overlay" and " aufs" since both substrings need to have 
> > at
> > +*  least one space before them in a valid overlay
> > +*  lxc.mount.entry (/A B overlay).  When the space before 
> > is
> > +*  missing it is very likely that these substrings are 
> > part of a
> > +*  path or something else. */
> > +   if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> > +   lstart = lend;
> > +   continue;
> > +   }
> > +   if (!(q = strstr(p, olddir))) {
> > +   lstart = lend;
> > +   continue;
> > +   }
> 
> You're still not doing anything to ensure that 'overlays'/'aufs' and the
> olddir are actually occurring on this same line.  So they could be showing
> up in a comment or just later line.  Temporarily turning lend into '\0' is
> the easiest way, but you could also do something like
> 
>   if (!(q = strstr(p, " overlay")) && !(q=strstr(p, " aufs")))
>   goto next;
>   if (q >= lend)
>   goto next;

Maybe we should avoid this check since it will fail when an aufs and overlay
lxc.mount.entry share (for whatever weird reason) a path. E.g.

lxc.mount.entry = /a b overlay upperdir=/some/path
lxc.mount.entry = /c d aufs br=/some/path

let's rather just use:

if (!strstr(p, " overlay") && !strstr(p, " aufs"))
goto next;
if (!(q = strstr(p, olddir)) || (q >= lend))
goto next;
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel


Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Christian Brauner
On Nov 2, 2015 6:43 PM, "Serge Hallyn"  wrote:
>
> Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > On Mon, Nov 02, 2015 at 03:52:25PM +0100, Christian Brauner wrote:
> > > On Mon, Nov 02, 2015 at 02:38:16PM +, 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 | 102
++
> > > > >  src/lxc/confile.h |   3 ++
> > > > >  2 files changed, 105 insertions(+)
> > > > >
> > > > > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > > > > index f7d6814..47b0b8b 100644
> > > > > --- a/src/lxc/confile.c
> > > > > +++ b/src/lxc/confile.c
> > > > > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct
lxc_conf *conf, const char *key, bool rm_sub
> > > > > }
> > > > >  }
> > > > >
> > > > > +bool clone_update_unexp_ovl_paths(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 >= lend)
> > > > > +continue;
> > > > > +/* Whenever an lxc.mount.entry entry is found in
a line we check
> > > > > +*  if the substring " overlay" or the substring
" aufs" is
> > > > > +*  present before doing any further work. We
check for "
> > > > > +*  overlay" and " aufs" since both substrings
need to have at
> > > > > +*  least one space before them in a valid overlay
> > > > > +*  lxc.mount.entry (/A B overlay).  When the
space before is
> > > > > +*  missing it is very likely that these
substrings are part of a
> > > > > +*  path or something else. */
> > > > > +   if (!strstr(p, " overlay") && !strstr(p, "
aufs")) {
> > > > > +   lstart = lend;
> > > > > +   continue;
> > > > > +   }
> > > > > +   if (!(q = strstr(p, olddir))) {
> > > > > +   lstart = lend;
> > > > > +   continue;
> > > > > +   }
> > > >
> > > > You're still not doing anything to ensure that 'overlays'/'aufs'
and the
> > > > olddir are actually occurring on this same line.  So they could be
showing
> > > > up in a comment or just later line.
> >
> > I don't think this is the case. At the beginning of the while() we
search for a
> > newline character and make lend point to the character immediately
after it.
> >
> >   lend = strchr(lstart, '\n');
> >   if (!lend)
> >   lend = lstart + strlen(lstart)

Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Serge Hallyn
Quoting Christian Brauner (christianvanbrau...@gmail.com):
> On Mon, Nov 02, 2015 at 02:38:16PM +, 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 | 102 
> > > ++
> > >  src/lxc/confile.h |   3 ++
> > >  2 files changed, 105 insertions(+)
> > > 
> > > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > > index f7d6814..47b0b8b 100644
> > > --- a/src/lxc/confile.c
> > > +++ b/src/lxc/confile.c
> > > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf 
> > > *conf, const char *key, bool rm_sub
> > >   }
> > >  }
> > >  
> > > +bool clone_update_unexp_ovl_paths(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 >= lend)
> > > +continue;
> > > +/* Whenever an lxc.mount.entry entry is found in a line 
> > > we check
> > > +*  if the substring " overlay" or the substring " aufs" 
> > > is
> > > +*  present before doing any further work. We check for "
> > > +*  overlay" and " aufs" since both substrings need to 
> > > have at
> > > +*  least one space before them in a valid overlay
> > > +*  lxc.mount.entry (/A B overlay).  When the space 
> > > before is
> > > +*  missing it is very likely that these substrings are 
> > > part of a
> > > +*  path or something else. */
> > > + if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> > > + lstart = lend;
> > > + continue;
> > > + }
> > > + if (!(q = strstr(p, olddir))) {
> > > + lstart = lend;
> > > + continue;
> > > + }
> > 
> > You're still not doing anything to ensure that 'overlays'/'aufs' and the
> > olddir are actually occurring on this same line.  So they could be showing
> > up in a comment or just later line.  Temporarily turning lend into '\0' is
> > the easiest way, but you could also do something like
> > 
> > if (!(q = strstr(p, " overlay")) && !(q=strstr(p, " aufs")))
> > goto next;
> > if (q >= lend)
> > goto next;
> > 
> > if (!(q = strstr(p, olddir)) || q >= lend)
> > goto next;
> > 
> > ...
> > 
> > break;
> > next:
> > lstart = lend;
> > continue;
> 
> Wait, what I want is to find all lines:
> 
> lxc.mount.entry = /A B overlay upperdir=/B,workdir=/C
> 
> This also includes lines like:
> 
> # lxc.mount.entry = /A B overlay upperdir=/B,workdir=/C

Yes, but if i have 

lxc.mount.entry = /a B none bind,create=dir
# maybe i should use overlayfs?
# then i could mount it at upperdir=/B

You'll be treating that as one line.

> that are comments. If a comment is a valid lxc.mount.entry entry it seems
> updating it does not really ma

Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Serge Hallyn
Quoting Christian Brauner (christianvanbrau...@gmail.com):
> On Mon, Nov 02, 2015 at 03:52:25PM +0100, Christian Brauner wrote:
> > On Mon, Nov 02, 2015 at 02:38:16PM +, 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 | 102 
> > > > ++
> > > >  src/lxc/confile.h |   3 ++
> > > >  2 files changed, 105 insertions(+)
> > > > 
> > > > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > > > index f7d6814..47b0b8b 100644
> > > > --- a/src/lxc/confile.c
> > > > +++ b/src/lxc/confile.c
> > > > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf 
> > > > *conf, const char *key, bool rm_sub
> > > > }
> > > >  }
> > > >  
> > > > +bool clone_update_unexp_ovl_paths(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 >= lend)
> > > > +continue;
> > > > +/* Whenever an lxc.mount.entry entry is found in a 
> > > > line we check
> > > > +*  if the substring " overlay" or the substring " 
> > > > aufs" is
> > > > +*  present before doing any further work. We check for 
> > > > "
> > > > +*  overlay" and " aufs" since both substrings need to 
> > > > have at
> > > > +*  least one space before them in a valid overlay
> > > > +*  lxc.mount.entry (/A B overlay).  When the space 
> > > > before is
> > > > +*  missing it is very likely that these substrings are 
> > > > part of a
> > > > +*  path or something else. */
> > > > +   if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> > > > +   lstart = lend;
> > > > +   continue;
> > > > +   }
> > > > +   if (!(q = strstr(p, olddir))) {
> > > > +   lstart = lend;
> > > > +   continue;
> > > > +   }
> > > 
> > > You're still not doing anything to ensure that 'overlays'/'aufs' and the
> > > olddir are actually occurring on this same line.  So they could be showing
> > > up in a comment or just later line.
> 
> I don't think this is the case. At the beginning of the while() we search for 
> a
> newline character and make lend point to the character immediately after it.
> 
>   lend = strchr(lstart, '\n');
>   if (!lend)
>   lend = lstart + strlen(lstart);
>   else
>   lend++;
> 
> Then we check
> 
>

Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Christian Brauner
On Mon, Nov 02, 2015 at 03:52:25PM +0100, Christian Brauner wrote:
> On Mon, Nov 02, 2015 at 02:38:16PM +, 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 | 102 
> > > ++
> > >  src/lxc/confile.h |   3 ++
> > >  2 files changed, 105 insertions(+)
> > > 
> > > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > > index f7d6814..47b0b8b 100644
> > > --- a/src/lxc/confile.c
> > > +++ b/src/lxc/confile.c
> > > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf 
> > > *conf, const char *key, bool rm_sub
> > >   }
> > >  }
> > >  
> > > +bool clone_update_unexp_ovl_paths(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 >= lend)
> > > +continue;
> > > +/* Whenever an lxc.mount.entry entry is found in a line 
> > > we check
> > > +*  if the substring " overlay" or the substring " aufs" 
> > > is
> > > +*  present before doing any further work. We check for "
> > > +*  overlay" and " aufs" since both substrings need to 
> > > have at
> > > +*  least one space before them in a valid overlay
> > > +*  lxc.mount.entry (/A B overlay).  When the space 
> > > before is
> > > +*  missing it is very likely that these substrings are 
> > > part of a
> > > +*  path or something else. */
> > > + if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> > > + lstart = lend;
> > > + continue;
> > > + }
> > > + if (!(q = strstr(p, olddir))) {
> > > + lstart = lend;
> > > + continue;
> > > + }
> > 
> > You're still not doing anything to ensure that 'overlays'/'aufs' and the
> > olddir are actually occurring on this same line.  So they could be showing
> > up in a comment or just later line.

I don't think this is the case. At the beginning of the while() we search for a
newline character and make lend point to the character immediately after it.

lend = strchr(lstart, '\n');
if (!lend)
lend = lstart + strlen(lstart);
else
lend++;

Then we check

if (!strstr(p, " overlay") && !strstr(p, " aufs"))

if this check fails we set lstart to lend which points at the character after
'\n'. The same for the next line etc. etc.

If on a line the check:

if (!strstr(p, " overlay") && !strstr(p, " aufs"))

is true we then check for the presence of olddir in p. 

if (!(q = strstr(p, olddir))) {
lstart = lend;
continue;
}

And we do that for every single line. But the algorithm guarantees, as far as I
understand it, that " overlay" or " aufs" and the olddir app

Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Christian Brauner
On Mon, Nov 02, 2015 at 02:38:16PM +, 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 | 102 
> > ++
> >  src/lxc/confile.h |   3 ++
> >  2 files changed, 105 insertions(+)
> > 
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index f7d6814..47b0b8b 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf *conf, 
> > const char *key, bool rm_sub
> > }
> >  }
> >  
> > +bool clone_update_unexp_ovl_paths(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 >= lend)
> > +continue;
> > +/* Whenever an lxc.mount.entry entry is found in a line we 
> > check
> > +*  if the substring " overlay" or the substring " aufs" is
> > +*  present before doing any further work. We check for "
> > +*  overlay" and " aufs" since both substrings need to have 
> > at
> > +*  least one space before them in a valid overlay
> > +*  lxc.mount.entry (/A B overlay).  When the space before 
> > is
> > +*  missing it is very likely that these substrings are 
> > part of a
> > +*  path or something else. */
> > +   if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> > +   lstart = lend;
> > +   continue;
> > +   }
> > +   if (!(q = strstr(p, olddir))) {
> > +   lstart = lend;
> > +   continue;
> > +   }
> 
> You're still not doing anything to ensure that 'overlays'/'aufs' and the
> olddir are actually occurring on this same line.  So they could be showing
> up in a comment or just later line.  Temporarily turning lend into '\0' is
> the easiest way, but you could also do something like
> 
>   if (!(q = strstr(p, " overlay")) && !(q=strstr(p, " aufs")))
>   goto next;
>   if (q >= lend)
>   goto next;
> 
>   if (!(q = strstr(p, olddir)) || q >= lend)
>   goto next;
> 
>   ...
> 
>   break;
> next:
>   lstart = lend;
>   continue;

Wait, what I want is to find all lines:

lxc.mount.entry = /A B overlay upperdir=/B,workdir=/C

This also includes lines like:

# lxc.mount.entry = /A B overlay upperdir=/B,workdir=/C

that are comments. If a comment is a valid lxc.mount.entry entry it seems
updating it does not really matter. (That's also what clone_update_unexp_hooks()
does for lxc.hook.* entries.)

I don't want to stop on the *first* match... I want to update all
lxc.mount.entry entries. The way you do it seems like it stops as soon as it has
found the *first* match. Is this what you want/expect from this function?
___
lxc-devel mailing 

Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Christian Brauner
On Mon, Nov 02, 2015 at 02:38:16PM +, 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 | 102 
> > ++
> >  src/lxc/confile.h |   3 ++
> >  2 files changed, 105 insertions(+)
> > 
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index f7d6814..47b0b8b 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf *conf, 
> > const char *key, bool rm_sub
> > }
> >  }
> >  
> > +bool clone_update_unexp_ovl_paths(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 >= lend)
> > +continue;
> > +/* Whenever an lxc.mount.entry entry is found in a line we 
> > check
> > +*  if the substring " overlay" or the substring " aufs" is
> > +*  present before doing any further work. We check for "
> > +*  overlay" and " aufs" since both substrings need to have 
> > at
> > +*  least one space before them in a valid overlay
> > +*  lxc.mount.entry (/A B overlay).  When the space before 
> > is
> > +*  missing it is very likely that these substrings are 
> > part of a
> > +*  path or something else. */
> > +   if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> > +   lstart = lend;
> > +   continue;
> > +   }
> > +   if (!(q = strstr(p, olddir))) {
> > +   lstart = lend;
> > +   continue;
> > +   }
> 
> You're still not doing anything to ensure that 'overlays'/'aufs' and the
> olddir are actually occurring on this same line.  So they could be showing
> up in a comment or just later line.  Temporarily turning lend into '\0' is
> the easiest way, but you could also do something like
> 
>   if (!(q = strstr(p, " overlay")) && !(q=strstr(p, " aufs")))
>   goto next;
>   if (q >= lend)
>   goto next;
> 
>   if (!(q = strstr(p, olddir)) || q >= lend)
>   goto next;
> 
>   ...
> 
>   break;
> next:
>   lstart = lend;
>   continue;

Ah, I didn't get what you wanted me to do the first time.
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel


Re: [lxc-devel] [PATCH 1/2 v3] Add clone_update_unexp_ovl_paths() function

2015-11-02 Thread Serge Hallyn
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 | 102 
> ++
>  src/lxc/confile.h |   3 ++
>  2 files changed, 105 insertions(+)
> 
> diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> index f7d6814..47b0b8b 100644
> --- a/src/lxc/confile.c
> +++ b/src/lxc/confile.c
> @@ -2598,6 +2598,108 @@ void clear_unexp_config_line(struct lxc_conf *conf, 
> const char *key, bool rm_sub
>   }
>  }
>  
> +bool clone_update_unexp_ovl_paths(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 >= lend)
> +continue;
> +/* Whenever an lxc.mount.entry entry is found in a line we 
> check
> +*  if the substring " overlay" or the substring " aufs" is
> +*  present before doing any further work. We check for "
> +*  overlay" and " aufs" since both substrings need to have at
> +*  least one space before them in a valid overlay
> +*  lxc.mount.entry (/A B overlay).  When the space before is
> +*  missing it is very likely that these substrings are part 
> of a
> +*  path or something else. */
> + if (!strstr(p, " overlay") && !strstr(p, " aufs")) {
> + lstart = lend;
> + continue;
> + }
> + if (!(q = strstr(p, olddir))) {
> + lstart = lend;
> + continue;
> + }

You're still not doing anything to ensure that 'overlays'/'aufs' and the
olddir are actually occurring on this same line.  So they could be showing
up in a comment or just later line.  Temporarily turning lend into '\0' is
the easiest way, but you could also do something like

if (!(q = strstr(p, " overlay")) && !(q=strstr(p, " aufs")))
goto next;
if (q >= lend)
goto next;

if (!(q = strstr(p, olddir)) || q >= lend)
goto next;

...

break;
next:
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