[libvirt] [PATCH 0/3] qemu_cgroup cleanups
*** BLARGGH HERE *** Ján Tomko (3): rename qemuSetupHostdevCGroup to qemuSetupHostdevCgroup Simplify qemuSetupChrSourceCgroup and its callers qemuSetupChrSourceCgroup: rename dev to source src/qemu/qemu_cgroup.c | 37 ++++++++++++++++--------------------- src/qemu/qemu_cgroup.h | 2 +- src/qemu/qemu_hotplug.c | 6 +++--- 3 files changed, 20 insertions(+), 25 deletions(-) -- 2.4.6
Change CGroup to Cgroup to match other functions in the file. --- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_cgroup.h | 2 +- src/qemu/qemu_hotplug.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index a8e0b8c..7b7411d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -239,7 +239,7 @@ qemuSetupHostSCSIDeviceCgroup(virSCSIDevicePtr dev ATTRIBUTE_UNUSED, } int -qemuSetupHostdevCGroup(virDomainObjPtr vm, +qemuSetupHostdevCgroup(virDomainObjPtr vm, virDomainHostdevDefPtr dev) { int ret = -1; @@ -592,7 +592,7 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, goto cleanup; for (i = 0; i < vm->def->nhostdevs; i++) { - if (qemuSetupHostdevCGroup(vm, vm->def->hostdevs[i]) < 0) + if (qemuSetupHostdevCgroup(vm, vm->def->hostdevs[i]) < 0) goto cleanup; } diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 711a6de..2bcf071 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -36,7 +36,7 @@ int qemuSetupDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk); int qemuTeardownDiskCgroup(virDomainObjPtr vm, virDomainDiskDefPtr disk); -int qemuSetupHostdevCGroup(virDomainObjPtr vm, +int qemuSetupHostdevCgroup(virDomainObjPtr vm, virDomainHostdevDefPtr dev) ATTRIBUTE_RETURN_CHECK; int qemuTeardownHostdevCgroup(virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 89e5c0d..f2aa81d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1281,7 +1281,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, break; } - if (qemuSetupHostdevCGroup(vm, hostdev) < 0) + if (qemuSetupHostdevCgroup(vm, hostdev) < 0) goto error; teardowncgroup = true; @@ -1890,7 +1890,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, added = true; - if (qemuSetupHostdevCGroup(vm, hostdev) < 0) + if (qemuSetupHostdevCgroup(vm, hostdev) < 0) goto cleanup; teardowncgroup = true; @@ -1985,7 +1985,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, return -1; } - if (qemuSetupHostdevCGroup(vm, hostdev) < 0) + if (qemuSetupHostdevCgroup(vm, hostdev) < 0) goto cleanup; teardowncgroup = true; -- 2.4.6
On 11/20/2015 05:04 AM, Ján Tomko wrote:
Change CGroup to Cgroup to match other functions in the file. --- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_cgroup.h | 2 +- src/qemu/qemu_hotplug.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-)
ACK (although qemu_hotplug.c didn't git am -3 cleanly to my top of tree) John
The domain definition is not needed in any of these functions. Only pass it to qemuSetupChardevCgroup, which is used as a callback for virDomainChrDefForeach. Use the right type for passing virDomainObjPtr instead of void* where possible. --- src/qemu/qemu_cgroup.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 7b7411d..f3cf41c 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -149,11 +149,9 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm, static int -qemuSetupChrSourceCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainChrSourceDefPtr dev, - void *opaque) +qemuSetupChrSourceCgroup(virDomainObjPtr vm, + virDomainChrSourceDefPtr dev) { - virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; int ret; @@ -171,25 +169,25 @@ qemuSetupChrSourceCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, } static int -qemuSetupChardevCgroup(virDomainDefPtr def, +qemuSetupChardevCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrDefPtr dev, void *opaque) { - return qemuSetupChrSourceCgroup(def, &dev->source, opaque); + virDomainObjPtr vm = opaque; + + return qemuSetupChrSourceCgroup(vm, &dev->source); } static int -qemuSetupTPMCgroup(virDomainDefPtr def, - virDomainTPMDefPtr dev, - void *opaque) +qemuSetupTPMCgroup(virDomainObjPtr vm, + virDomainTPMDefPtr dev) { int ret = 0; switch (dev->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - ret = qemuSetupChrSourceCgroup(def, &dev->data.passthrough.source, - opaque); + ret = qemuSetupChrSourceCgroup(vm, &dev->data.passthrough.source); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; @@ -585,10 +583,7 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, vm) < 0) goto cleanup; - if (vm->def->tpm && - (qemuSetupTPMCgroup(vm->def, - vm->def->tpm, - vm) < 0)) + if (vm->def->tpm && qemuSetupTPMCgroup(vm, vm->def->tpm) < 0) goto cleanup; for (i = 0; i < vm->def->nhostdevs; i++) { -- 2.4.6
On 11/20/2015 05:04 AM, Ján Tomko wrote:
The domain definition is not needed in any of these functions. Only pass it to qemuSetupChardevCgroup, which is used as a callback for virDomainChrDefForeach.
Use the right type for passing virDomainObjPtr instead of void* where possible. --- src/qemu/qemu_cgroup.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 7b7411d..f3cf41c 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -149,11 +149,9 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm,
static int -qemuSetupChrSourceCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, - virDomainChrSourceDefPtr dev, - void *opaque) +qemuSetupChrSourceCgroup(virDomainObjPtr vm, + virDomainChrSourceDefPtr dev) { - virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; int ret;
@@ -171,25 +169,25 @@ qemuSetupChrSourceCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, }
static int -qemuSetupChardevCgroup(virDomainDefPtr def, +qemuSetupChardevCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrDefPtr dev, void *opaque) { - return qemuSetupChrSourceCgroup(def, &dev->source, opaque); + virDomainObjPtr vm = opaque; + + return qemuSetupChrSourceCgroup(vm, &dev->source); }
static int -qemuSetupTPMCgroup(virDomainDefPtr def, - virDomainTPMDefPtr dev, - void *opaque) +qemuSetupTPMCgroup(virDomainObjPtr vm, + virDomainTPMDefPtr dev) { int ret = 0;
switch (dev->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - ret = qemuSetupChrSourceCgroup(def, &dev->data.passthrough.source, - opaque); + ret = qemuSetupChrSourceCgroup(vm, &dev->data.passthrough.source); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; @@ -585,10 +583,7 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, vm) < 0) goto cleanup;
- if (vm->def->tpm && - (qemuSetupTPMCgroup(vm->def, - vm->def->tpm, - vm) < 0)) + if (vm->def->tpm && qemuSetupTPMCgroup(vm, vm->def->tpm) < 0)
Why not just pass "vm"... qemuSetupTPMCgroup could easily deref "->def->tpm)" ACK - your call if you want to keep/remove the 2nd parameter. John
goto cleanup;
for (i = 0; i < vm->def->nhostdevs; i++) {
On Fri, Nov 20, 2015 at 09:42:43AM -0500, John Ferlan wrote:
On 11/20/2015 05:04 AM, Ján Tomko wrote:
The domain definition is not needed in any of these functions. Only pass it to qemuSetupChardevCgroup, which is used as a callback for virDomainChrDefForeach.
Use the right type for passing virDomainObjPtr instead of void* where possible. --- src/qemu/qemu_cgroup.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)
...
@@ -585,10 +583,7 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, vm) < 0) goto cleanup;
- if (vm->def->tpm && - (qemuSetupTPMCgroup(vm->def, - vm->def->tpm, - vm) < 0)) + if (vm->def->tpm && qemuSetupTPMCgroup(vm, vm->def->tpm) < 0)
Why not just pass "vm"... qemuSetupTPMCgroup could easily deref "->def->tpm)"
ACK - your call if you want to keep/remove the 2nd parameter.
Thanks. I removed the second parameter as well and pushed the series. Jan
John
goto cleanup;
for (i = 0; i < vm->def->nhostdevs; i++) {
We do not have a pointer to the device here, just its source. --- src/qemu/qemu_cgroup.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f3cf41c..3ce7fc6 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -150,20 +150,20 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm, static int qemuSetupChrSourceCgroup(virDomainObjPtr vm, - virDomainChrSourceDefPtr dev) + virDomainChrSourceDefPtr source) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret; - if (dev->type != VIR_DOMAIN_CHR_TYPE_DEV) + if (source->type != VIR_DOMAIN_CHR_TYPE_DEV) return 0; - VIR_DEBUG("Process path '%s' for device", dev->data.file.path); + VIR_DEBUG("Process path '%s' for device", source->data.file.path); - ret = virCgroupAllowDevicePath(priv->cgroup, dev->data.file.path, + ret = virCgroupAllowDevicePath(priv->cgroup, source->data.file.path, VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupPath(vm, priv->cgroup, "allow", - dev->data.file.path, "rw", ret == 0); + source->data.file.path, "rw", ret == 0); return ret; } -- 2.4.6
On 11/20/2015 05:04 AM, Ján Tomko wrote:
We do not have a pointer to the device here, just its source. --- src/qemu/qemu_cgroup.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
ACK John
participants (2)
-
John Ferlan -
Ján Tomko