[lxc-devel] [PATCH] cgmanager get/set: clean up child

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> On Thu, 14 Aug 2014 21:56:04 +0000
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Make sure we reap our child at cgm_{s,g}et.
> > 
> > Not doing this resulted in a defunct child of Tycho's
> > lxc-restore.
> > 
> > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> 
> Acked-by: Dwight Engen <dwight.engen at oracle.com>
> (with one question below)
> 
> > ---
> >  src/lxc/cgmanager.c | 29 ++++++++++++++++++++---------
> >  1 file changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> > index c4f48e9..c46f80d 100644
> > --- a/src/lxc/cgmanager.c
> > +++ b/src/lxc/cgmanager.c
> > @@ -794,32 +794,37 @@ static int cgm_get(const char *filename, char
> > *value, size_t len, const char *na close(p[1]);
> >  		return -1;
> >  	}
> > -	if (!pid)
> > +	if (!pid) // do_cgm_get exits
> >  		do_cgm_get(name, lxcpath, filename, p[1], len &&
> > value); close(p[1]);
> >  	ret = read(p[0], &newlen, sizeof(newlen));
> >  	if (ret != sizeof(newlen)) {
> >  		close(p[0]);
> > -		return -1;
> > +		ret = -1;
> > +		goto out;
> >  	}
> >  	if (!len || !value) {
> >  		close(p[0]);
> > -		return newlen;
> > +		ret = newlen;
> > +		goto out;
> >  	}
> >  	memset(value, 0, len);
> > +	ret = -1;
> >  	if (newlen < 0) { // child is reporting an error
> >  		close(p[0]);
> > -		return -1;
> > +		goto out;
> >  	}
> >  	if (newlen == 0) { // empty read
> >  		close(p[0]);
> > -		return 0;
> > +		goto out;
> 
> So this is a bug fix here because it looks like it used to return 0 and
> now it will return -1?

D'oh, no I'm not sure about that.  For instance, cpuset.mems could be
empty, which could cause an empty read.  (Right?)

> >  	}
> >  	readlen = newlen > len ? len : newlen;
> >  	ret = read(p[0], value, readlen);
> >  	close(p[0]);
> > -	if (ret != readlen)
> > -		return -1;
> > +	if (ret != readlen) {
> > +		ret = -1;
> > +		goto out;
> > +	}
> >  	if (newlen >= len) {
> >  		value[len-1] = '\0';
> >  		newlen = len-1;
> > @@ -828,7 +833,11 @@ static int cgm_get(const char *filename, char
> > *value, size_t len, const char *na value[newlen++] = '\n';
> >  		value[newlen] = '\0';
> >  	}
> > -	return newlen;
> > +	ret = newlen;
> > +out:
> > +	if (wait_for_pid(pid))
> > +		WARN("do_cgm_get exited with error");
> > +	return ret;
> >  }
> >  
> >  static void do_cgm_set(const char *name, const char *lxcpath, const
> > char *filename, const char *value, int outp) @@ -920,11 +929,13 @@
> > static int cgm_set(const char *filename, const char *value, const
> > char *name, co close(p[0]); return -1;
> >  	}
> > -	if (!pid)
> > +	if (!pid) // do_cgm_set exits
> >  		do_cgm_set(name, lxcpath, filename, value, p[1]);
> >  	close(p[1]);
> >  	ret = read(p[0], &v, sizeof(v));
> >  	close(p[0]);
> > +	if (wait_for_pid(pid))
> > +		WARN("do_cgm_set exited with error");
> >  	if (ret != sizeof(v) || !v)
> >  		return -1;
> >  	return 0;
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel