[libvirt] [PATCH 0/3] vcpu fixes

Ján Tomko (3): Properly free vcpupin info for unplugged CPUs Make virDomainVcpuPinDel return void Fix error for out of range vcpu in qemuDomainPinVcpuFlags src/conf/domain_conf.c | 31 ++----------------------------- src/conf/domain_conf.h | 5 +---- src/libvirt_private.syms | 1 - src/libxl/libxl_driver.c | 7 +------ src/qemu/qemu_driver.c | 22 ++++------------------ 5 files changed, 8 insertions(+), 58 deletions(-) -- 1.8.3.2

Remove the pointer from def->cputune.vcpupin after unplugging the CPU and also free the bitmap contained in the structure by calling virDomainVcpuPinDel instead of VIR_FREE. Introduced by commit 0df1a79. This makes virDomainLookupVcpuPin redundant. https://bugzilla.redhat.com/show_bug.cgi?id=1088165 --- src/conf/domain_conf.c | 20 -------------------- src/conf/domain_conf.h | 3 --- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 6 +----- 4 files changed, 1 insertion(+), 29 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 05fa3f9..cb0df3d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10966,26 +10966,6 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node, goto cleanup; } -/* - * Return the vcpupin related with the vcpu id on SUCCESS, or - * NULL on failure. - */ -virDomainVcpuPinDefPtr -virDomainLookupVcpuPin(virDomainDefPtr def, - int vcpuid) -{ - size_t i; - - if (!def->cputune.vcpupin) - return NULL; - - for (i = 0; i < def->cputune.nvcpupin; i++) { - if (def->cputune.vcpupin[i]->vcpuid == vcpuid) - return def->cputune.vcpupin[i]; - } - - return NULL; -} int virDomainDefMaybeAddController(virDomainDefPtr def, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2ff0bc4..3426c48 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2671,9 +2671,6 @@ int virDomainObjListExport(virDomainObjListPtr doms, virDomainObjListFilter filter, unsigned int flags); -virDomainVcpuPinDefPtr virDomainLookupVcpuPin(virDomainDefPtr def, - int vcpuid); - int virDomainDefMaybeAddController(virDomainDefPtr def, int type, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0e81f2f..972b184 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -292,7 +292,6 @@ virDomainLifecycleTypeToString; virDomainLiveConfigHelperMethod; virDomainLockFailureTypeFromString; virDomainLockFailureTypeToString; -virDomainLookupVcpuPin; virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; virDomainMemDumpTypeFromString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5970585..3fbaa62 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4120,8 +4120,6 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } } else { for (i = oldvcpus - 1; i >= nvcpus; i--) { - virDomainVcpuPinDefPtr vcpupin = NULL; - if (priv->cgroup) { if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu) < 0) goto cleanup; @@ -4132,9 +4130,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } /* Free vcpupin setting */ - if ((vcpupin = virDomainLookupVcpuPin(vm->def, i))) { - VIR_FREE(vcpupin); - } + ignore_value(virDomainVcpuPinDel(vm->def, i)); } } -- 1.8.3.2

On 04/22/14 14:52, Ján Tomko wrote:
Remove the pointer from def->cputune.vcpupin after unplugging the CPU and also free the bitmap contained in the structure by calling virDomainVcpuPinDel instead of VIR_FREE.
Introduced by commit 0df1a79.
This makes virDomainLookupVcpuPin redundant.
https://bugzilla.redhat.com/show_bug.cgi?id=1088165 --- src/conf/domain_conf.c | 20 -------------------- src/conf/domain_conf.h | 3 --- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 6 +----- 4 files changed, 1 insertion(+), 29 deletions(-)
ACK, Peter

Before, it only returned -1 on failure to shrink the array. Since the switch to VIR_DELETE_ELEMENT in commit 2133441, it returns either 0 or 0. --- src/conf/domain_conf.c | 11 ++--------- src/conf/domain_conf.h | 2 +- src/libxl/libxl_driver.c | 7 +------ src/qemu/qemu_driver.c | 16 +++------------- 4 files changed, 7 insertions(+), 29 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cb0df3d..57eb215 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14510,27 +14510,20 @@ virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, return -1; } -int +void virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) { int n; virDomainVcpuPinDefPtr *vcpupin_list = def->cputune.vcpupin; - /* No vcpupin exists yet */ - if (!def->cputune.nvcpupin) { - return 0; - } - for (n = 0; n < def->cputune.nvcpupin; n++) { if (vcpupin_list[n]->vcpuid == vcpu) { virBitmapFree(vcpupin_list[n]->cpumask); VIR_FREE(vcpupin_list[n]); VIR_DELETE_ELEMENT(vcpupin_list, n, def->cputune.nvcpupin); - break; + return; } } - - return 0; } int diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3426c48..c1cc854 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2300,7 +2300,7 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, int maplen, int vcpu); -int virDomainVcpuPinDel(virDomainDefPtr def, int vcpu); +void virDomainVcpuPinDel(virDomainDefPtr def, int vcpu); int virDomainEmulatorPinAdd(virDomainDefPtr def, unsigned char *cpumap, diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b3f8df6..a6ae8a1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1945,12 +1945,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, /* full bitmap means reset the settings (if any). */ if (virBitmapIsAllSet(pcpumap)) { - if (virDomainVcpuPinDel(targetDef, vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to delete vcpupin xml for vcpu '%d'"), - vcpu); - goto endjob; - } + virDomainVcpuPinDel(targetDef, vcpu); goto done; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3fbaa62..6996b80 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4130,7 +4130,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } /* Free vcpupin setting */ - ignore_value(virDomainVcpuPinDel(vm->def, i)); + virDomainVcpuPinDel(vm->def, i); } } @@ -4423,12 +4423,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } if (doReset) { - if (virDomainVcpuPinDel(vm->def, vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to delete vcpupin xml of " - "a running domain")); - goto cleanup; - } + virDomainVcpuPinDel(vm->def, vcpu); } else { if (vm->def->cputune.vcpupin) virDomainVcpuPinDefArrayFree(vm->def->cputune.vcpupin, vm->def->cputune.nvcpupin); @@ -4448,12 +4443,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (doReset) { - if (virDomainVcpuPinDel(persistentDef, vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to delete vcpupin xml of " - "a persistent domain")); - goto cleanup; - } + virDomainVcpuPinDel(persistentDef, vcpu); } else { if (!persistentDef->cputune.vcpupin) { if (VIR_ALLOC(persistentDef->cputune.vcpupin) < 0) -- 1.8.3.2

On 04/22/14 14:52, Ján Tomko wrote:
Before, it only returned -1 on failure to shrink the array. Since the switch to VIR_DELETE_ELEMENT in commit 2133441, it returns either 0 or 0. --- src/conf/domain_conf.c | 11 ++--------- src/conf/domain_conf.h | 2 +- src/libxl/libxl_driver.c | 7 +------ src/qemu/qemu_driver.c | 16 +++------------- 4 files changed, 7 insertions(+), 29 deletions(-)
ACK, Peter

Changes: error: invalid argument: vcpu number out of range 2 > 2 to slightly less confusing: error: invalid argument: vcpu number out of range 2 > 1 --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6996b80..c3ac6d4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4355,7 +4355,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, if (vcpu > (priv->nvcpupids-1)) { virReportError(VIR_ERR_INVALID_ARG, _("vcpu number out of range %d > %d"), - vcpu, priv->nvcpupids); + vcpu, priv->nvcpupids - 1); goto cleanup; } -- 1.8.3.2

On 04/22/14 14:52, Ján Tomko wrote:
Changes: error: invalid argument: vcpu number out of range 2 > 2 to slightly less confusing: error: invalid argument: vcpu number out of range 2 > 1 --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK, Peter
participants (2)
-
Ján Tomko
-
Peter Krempa