[lxc-devel] [PATCH] tests/startone: wrong set_cgroup_item check

Purcareata Bogdan-B43198 B43198 at freescale.com
Wed Jul 3 08:56:03 UTC 2013


> -----Original Message-----
> From: Serge Hallyn [mailto:serge.hallyn at ubuntu.com]
> Sent: Tuesday, July 02, 2013 5:27 PM
> To: Purcareata Bogdan-B43198
> Cc: lxc-devel at lists.sourceforge.net
> Subject: Re: [lxc-devel] [PATCH] tests/startone: wrong set_cgroup_item check
> 
> Quoting Bogdan Purcareata (bogdan.purcareata at freescale.com):
> > Minor typo.
> >
> > Signed-off-by: Bogdan Purcareata <bogdan.purcareata at freescale.com>
> > ---
> >  src/tests/startone.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/tests/startone.c b/src/tests/startone.c
> > index d781e75..2c1f39b 100644
> > --- a/src/tests/startone.c
> > +++ b/src/tests/startone.c
> > @@ -201,7 +201,7 @@ int main(int argc, char *argv[])
> >
> >  	sprintf(buf, "FROZEN");
> >  	b = c->set_cgroup_item(c, "freezer.state", buf);
> > -	if (!b) {
> > +	if (b) {
> 
> Maybe I'm not thinking right, but why do you think setting freezer.state
> should fail?

After looking at the lxcapi_set_cgroup_item implementation I'm a bit confused:

src/lxc/lxccontainer.c:
1383 static bool lxcapi_set_cgroup_item(struct lxc_container *c, const char *subsys, const char *value)
1384 {
1385   int ret;
1386
[ ... ] initial checks
1395
1396   ret = lxc_cgroup_set(c->name, subsys, value, c->config_path) == 0;
1397
1398   container_disk_unlock(c);
1399   return ret == 0;
1400 }

lxc_cgroup_set is basically a call to do_cgroup_set, which returns 0 (false) on success (both functions are in src/lxc/cgroup.c).

So calls return this:

do_cgroup_set -> 0 (false) on success
lxc_cgroup_set -> false on success
ret (in lxcapi_set_cgroup_item) = (false == false) = true on success
lxcapi_set_cgroup_item -> (true == false) = false on success

That's why I thought the check should have been made for (b) instead of (!b) - since 1 (true) is the error case here.
Now I'm thinking maybe there should be a bit of refactoring in the code?

> 
> >  		fprintf(stderr, "%d: not able to set freezer.state.\n", __LINE__);
> >  		goto out;
> >  	}
> > --
> > 1.7.10.4
> >
> >
> >
> > ------------------------------------------------------------------------------
> > This SF.net email is sponsored by Windows:
> >
> > Build for Windows Store.
> >
> > http://p.sf.net/sfu/windows-dev2dev
> > _______________________________________________
> > Lxc-devel mailing list
> > Lxc-devel at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel






More information about the lxc-devel mailing list