On Thu, May 23, 2013 at 12:06:56PM +0800, Gao feng wrote: > use virLXCControllerChown to make codes clearer. > > Signed-off-by: Gao feng <gaofeng@xxxxxxxxxxxxxx> > --- > src/lxc/lxc_controller.c | 81 ++++++++++++++++++------------------------------ > 1 file changed, 31 insertions(+), 50 deletions(-) > > diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c > index 7d27135..1cceecf 100644 > --- a/src/lxc/lxc_controller.c > +++ b/src/lxc/lxc_controller.c > @@ -1121,6 +1121,29 @@ cleanup2: > return rc; > } > > +static int > +virLXCControllerChown(virLXCControllerPtr ctrl, > + char *path) > +{ > + uid_t uid = -1; > + gid_t gid = -1; > + > + if (!ctrl->def->idmap.uidmap) > + return 0; > + > + uid = ctrl->def->idmap.uidmap[0].target; > + gid = ctrl->def->idmap.gidmap[0].target; > + > + if (chown(path, uid, gid) < 0) { > + virReportSystemError(errno, > + _("Failed to change owner of %s to %u:%u"), > + path, uid, gid); > + return -1; > + } > + > + return 0; > +} I suggest you move this patch to be before current #7. That way patches 7-11 can use this function right away, avoiding all the code below > + > > static int > virLXCControllerSetupUsernsMap(virDomainIdMapEntryPtr map, > @@ -1260,8 +1283,6 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) > size_t i; > int ret = -1; > char *path = NULL; > - uid_t uid = (uid_t)-1; > - gid_t gid = (gid_t)-1; > const struct { > int maj; > int min; > @@ -1278,11 +1299,6 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) > if (virLXCControllerSetupDev(ctrl) < 0) > goto out; > > - if (ctrl->def->idmap.uidmap) { > - uid = ctrl->def->idmap.uidmap[0].target; > - gid = ctrl->def->idmap.gidmap[0].target; > - } > - > /* Populate /dev/ with a few important bits */ > for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { > if (virAsprintf(&path, "/%s/%s.dev/%s", > @@ -1301,12 +1317,9 @@ static int virLXCControllerPopulateDevices(virLXCControllerPtr ctrl) > goto out; > } > > - if (chown(path, uid, gid) < 0) { > - virReportSystemError(errno, > - _("Failed to change owner of %s to %u:%u"), > - devs[i].path, uid, gid); > + if (virLXCControllerChown(ctrl, path) < 0) > goto out; > - } > + > VIR_FREE(path); > } > > @@ -1506,15 +1519,8 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) > char *opts = NULL; > char *devpts = NULL; > char *path = NULL; > - uid_t uid = (uid_t)-1; > - gid_t gid = (gid_t)-1; > int ret = -1; > > - if (ctrl->def->idmap.uidmap) { > - uid = ctrl->def->idmap.uidmap[0].target; > - gid = ctrl->def->idmap.gidmap[0].target; > - } > - > VIR_DEBUG("Setting up private /dev/pts"); > > mount_options = virSecurityManagerGetMountOptions(ctrl->securityManager, > @@ -1558,20 +1564,11 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) > goto cleanup; > } > > - if (chown(ctrl->devptmx, uid, gid) < 0) { > - virReportSystemError(errno, > - _("Failed to change the owner of" > - "%s to %u:%u"), > - path, uid, gid); > + if (virLXCControllerChown(ctrl, ctrl->devptmx) < 0) > goto cleanup; > - } > - if (chown(devpts, uid, gid) < 0) { > - virReportSystemError(errno, > - _("Failed to change the owner of" > - "%s to %u:%u"), > - devpts, uid, gid); > + > + if (virLXCControllerChown(ctrl, devpts) < 0) > goto cleanup; > - } > > if (access(ctrl->devptmx, W_OK) < 0) { > if (virAsprintf(&path, "/%s/%s.dev/ptmx", > @@ -1586,13 +1583,8 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) > virReportSystemError(errno, _("Failed to make device %s"), path); > goto cleanup; > } > - if (chown(path, uid, gid) < 0) { > - virReportSystemError(errno, > - _("Failed to change the owner of" > - "%s to %u:%u"), > - path, uid, gid); > + if (virLXCControllerChown(ctrl, path) < 0) > goto cleanup; > - } > } > > > @@ -1619,15 +1611,8 @@ virLXCControllerSetupConsoles(virLXCControllerPtr ctrl, > { > size_t i; > int ret = -1; > - uid_t uid = (uid_t)-1; > - gid_t gid = (gid_t)-1; > char *ttyHostPath = NULL; > > - if (ctrl->def->idmap.uidmap) { > - uid = ctrl->def->idmap.uidmap[0].target; > - gid = ctrl->def->idmap.gidmap[0].target; > - } > - > for (i = 0; i < ctrl->nconsoles; i++) { > VIR_DEBUG("Opening tty on private %s", ctrl->devptmx); > if (lxcCreateTty(ctrl, > @@ -1639,13 +1624,9 @@ virLXCControllerSetupConsoles(virLXCControllerPtr ctrl, > } > > /* Change the owner of tty device to the root user of container */ > - if (chown(ttyHostPath, uid, gid) < 0) { > - virReportSystemError(errno, > - _("Failed to change owner of tty" > - " %s to %u:%u"), > - ttyHostPath, uid, gid); > + if (virLXCControllerChown(ctrl, ttyHostPath) < 0) > goto out; > - } > + > VIR_FREE(ttyHostPath); > } 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