On Thu, Apr 11, 2013 at 12:02:05PM +0200, Michal Privoznik wrote:
> On 10.04.2013 12:08, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> >
> > Currently the virCgroupPtr struct contains 3 pieces of
> > information
> >
> > - path - path of the cgroup, relative to current process'
> > cgroup placement
> > - placement - current process' placement in each controller
> > - mounts - mount point of each controller
> >
> > When reading/writing cgroup settings, the path & placement
> > strings are combined to form the file path. This approach
> > only works if we assume all cgroups will be relative to
> > the current process' cgroup placement.
> >
> > To allow support for managing cgroups at any place in the
> > heirarchy a change is needed. The 'placement' data should
> > reflect the absolute path to the cgroup, and the 'path'
> > value should no longer be used to form the paths to the
> > cgroup attribute files.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> > src/util/vircgroup.c | 222 +++++++++++++++++++++++++++++++++++---------------
> > tests/vircgrouptest.c | 53 +++++++-----
> > 2 files changed, 188 insertions(+), 87 deletions(-)
> >
> > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> > index 2f52c92..c336806 100644
> > --- a/src/util/vircgroup.c
> > +++ b/src/util/vircgroup.c
> > @@ -101,6 +101,23 @@ bool virCgroupHasController(virCgroupPtr cgroup, int controller)
> > }
> >
> > #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
> > +static int virCgroupCopyMounts(virCgroupPtr group,
> > + virCgroupPtr parent)
> > +{
> > + int i;
> > + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) {
> > + if (!parent->controllers[i].mountPoint)
> > + continue;
> > +
> > + group->controllers[i].mountPoint =
> > + strdup(parent->controllers[i].mountPoint);
> > +
> > + if (!group->controllers[i].mountPoint)
> > + return -ENOMEM;
> > + }
> > + return 0;
> > +}
> > +
> > /*
> > * Process /proc/mounts figuring out what controllers are
> > * mounted and where
> > @@ -158,12 +175,61 @@ no_memory:
> > }
> >
> >
> > +static int virCgroupCopyPlacement(virCgroupPtr group,
> > + const char *path,
> > + virCgroupPtr parent)
> > +{
> > + int i;
> > + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) {
> > + if (!group->controllers[i].mountPoint)
> > + continue;
> > +
> > + if (path[0] == '/') {
> > + if (!(group->controllers[i].placement = strdup(path)))
> > + return -ENOMEM;
> > + } else {
> > + /*
> > + * parent=="/" + path="" => "/"
> > + * parent=="/libvirt.service" + path="" => "/libvirt.service"
> > + * parent=="/libvirt.service" + path="foo" => "/libvirt.service/foo"
> > + */
>
> s/path=/path==/
>
> > + if (virAsprintf(&group->controllers[i].placement,
> > + "%s%s%s",
> > + parent->controllers[i].placement,
> > + (STREQ(parent->controllers[i].placement, "/") ||
> > + STREQ(path, "") ? "" : "/"),
> > + path) < 0)
>
> No, please no. This is too big for my small brain. And it's easy to make
> a mistake here, as you just did. The closing parenthesis should be just
> before the ternary operator. In fact, both parentheses can be left out.
It isn't a mistake - I just used () for style reasons - get the second
line to indent, so that it was obviously a continuation. The () usage
I had has no functional effect since || is higher precedence than ?:
> > diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
> > index a68aa88..3f35f2e 100644
> > --- a/tests/vircgrouptest.c
> > +++ b/tests/vircgrouptest.c
>
> > @@ -246,4 +255,4 @@ mymain(void)
> > return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
> > }
> >
> > -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvircgroupmock.so")
> > +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/vircgroupmock.so")
> >
>
> This seems a bit unrelated. It's fixing pre-existing bug so it should go
> into separate patch.
Yep, that's a mistake
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list