Re: [libvirt] [PATCH 20/47] vircgroup: extract virCgroupV1SetOwner

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina(a)redhat.com&gt; wrote: > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/util/vircgroup.c | 54 +----------------------------- > src/util/vircgroupbackend.h | 7 ++++ > src/util/vircgroupv1.c | 65 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 73 insertions(+), 53 deletions(-) > > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c > index 29faf5fb2c..479a2bf664 100644 > --- a/src/util/vircgroup.c > +++ b/src/util/vircgroup.c > @@ -3284,59 +3284,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup, > gid_t gid, > int controllers) > { > - int ret = -1; > - size_t i; > - DIR *dh = NULL; > - int direrr; > - > - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { > - VIR_AUTOFREE(char *) base = NULL; > - struct dirent *de; > - > - if (!((1 << i) & controllers)) > - continue; > - > - if (!cgroup->controllers[i].mountPoint) > - continue; > - > - if (virAsprintf(&base, "%s%s", cgroup->controllers[i].mountPoint, > - cgroup->controllers[i].placement) < 0) > - goto cleanup; > - > - if (virDirOpen(&dh, base) < 0) > - goto cleanup; > - > - while ((direrr = virDirRead(dh, &de, base)) > 0) { > - VIR_AUTOFREE(char *) entry = NULL; > - > - if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) > - goto cleanup; > - > - if (chown(entry, uid, gid) < 0) { > - virReportSystemError(errno, > - _("cannot chown '%s' to (%u, %u)"), > - entry, uid, gid); > - goto cleanup; > - } > - } > - if (direrr < 0) > - goto cleanup; > - > - if (chown(base, uid, gid) < 0) { > - virReportSystemError(errno, > - _("cannot chown '%s' to (%u, %u)"), > - base, uid, gid); > - goto cleanup; > - } > - > - VIR_DIR_CLOSE(dh); > - } > - > - ret = 0; > - > - cleanup: > - VIR_DIR_CLOSE(dh); > - return ret; > + return cgroup->backend->setOwner(cgroup, uid, gid, controllers); > } > > > diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h > index 70deb47461..4538654068 100644 > --- a/src/util/vircgroupbackend.h > +++ b/src/util/vircgroupbackend.h > @@ -123,6 +123,12 @@ typedef int > const char *oldroot, > const char *mountopts); > > +typedef int > +(*virCgroupSetOwnerCB)(virCgroupPtr cgroup, > + uid_t uid, > + gid_t gid, > + int controllers); > + > struct _virCgroupBackend { > virCgroupBackendType type; > > @@ -144,6 +150,7 @@ struct _virCgroupBackend { > virCgroupAddTaskCB addTask; > virCgroupHasEmptyTasksCB hasEmptyTasks; > virCgroupBindMountCB bindMount; > + virCgroupSetOwnerCB setOwner; > }; > typedef struct _virCgroupBackend virCgroupBackend; > typedef virCgroupBackend *virCgroupBackendPtr; > diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c > index 0514ba392d..d74fc8ff96 100644 > --- a/src/util/vircgroupv1.c > +++ b/src/util/vircgroupv1.c > @@ -866,6 +866,70 @@ virCgroupV1BindMount(virCgroupPtr group, > } > > > +static int > +virCgroupV1SetOwner(virCgroupPtr cgroup, > + uid_t uid, > + gid_t gid, > + int controllers) > +{ > + int ret = -1; > + size_t i; > + VIR_AUTOFREE(char *) base = NULL; > + VIR_AUTOFREE(char *) entry = NULL; > I would avoid these changes here in order to keep the diff as minimal as possible.