Commit <2020c6af8a8e4bb04acb629d089142be984484c8> fixed an issue with QEMU driver by reporting offline CPUs as well. However, doing so it introduced a regression into libxl and test drivers by completely ignoring the passed `hostcpus` variable. Move the virHostCPUGetAvailableCPUsBitmap() out of the helper into QEMU driver so it will not affect other drivers which gets the number of host CPUs differently. This was uncovered by running libvirt-dbus test suite which counts on the fact that test driver has hard-coded host definition and must not depend on the host at all. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 18 +++++------------- src/conf/domain_conf.h | 4 ++-- src/libxl/libxl_driver.c | 7 ++++++- src/qemu/qemu_driver.c | 6 +++++- src/test/test_driver.c | 8 ++++++-- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef67efa1da..8e7981bf25 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2090,33 +2090,25 @@ virDomainDefHasVcpuPin(const virDomainDef *def) * @maplen: length of one cpumap passed from caller (@cpumaps) * @ncpumaps: count of cpumaps of @maplen length in @cpumaps * @cpumaps: array of pinning information bitmaps to be filled - * @hostcpus: number of cpus in the host + * @hostcpus: default CPU pinning bitmap based on host CPUs * @autoCpuset: Cpu pinning bitmap used in case of automatic cpu pinning * * Fills the @cpumaps array as documented by the virDomainGetVcpuPinInfo API. * In case when automatic cpu pinning is supported, the bitmap should be passed - * as @autoCpuset. If @hostcpus is < 0 no error is reported (to pass through - * error message). + * as @autoCpuset. * - * Returns number of filled entries or -1 on error. + * Returns number of filled entries. */ int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, int maplen, int ncpumaps, unsigned char *cpumaps, - int hostcpus, + virBitmapPtr hostcpus, virBitmapPtr autoCpuset) { int maxvcpus = virDomainDefGetVcpusMax(def); size_t i; - g_autoptr(virBitmap) allcpumap = NULL; - - if (hostcpus < 0) - return -1; - - if (!(allcpumap = virHostCPUGetAvailableCPUsBitmap())) - return -1; for (i = 0; i < maxvcpus && i < ncpumaps; i++) { virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i); @@ -2130,7 +2122,7 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, else if (def->cpumask) bitmap = def->cpumask; else - bitmap = allcpumap; + bitmap = hostcpus; virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, i), maplen); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 011bf66cb4..68be32614c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3741,9 +3741,9 @@ int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, int maplen, int ncpumaps, unsigned char *cpumaps, - int hostcpus, + virBitmapPtr hostcpus, virBitmapPtr autoCpuset) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) G_GNUC_WARN_UNUSED_RESULT; bool virDomainDefHasMemballoon(const virDomainDef *def) ATTRIBUTE_NONNULL(1); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 4db3c0782e..dc602ea162 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2547,6 +2547,7 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps, libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm = NULL; virDomainDefPtr targetDef = NULL; + g_autoptr(virBitmap) hostcpus = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -2568,8 +2569,12 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps, /* Make sure coverity knows targetDef is valid at this point. */ sa_assert(targetDef); + if (!(hostcpus = virBitmapNew(libxl_get_max_cpus(cfg->ctx)))) + goto cleanup; + virBitmapSetAll(hostcpus); + ret = virDomainDefGetVcpuPinInfoHelper(targetDef, maplen, ncpumaps, cpumaps, - libxl_get_max_cpus(cfg->ctx), NULL); + hostcpus, NULL); cleanup: virDomainObjEndAPI(&vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0f98243fe4..9eaea8d613 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5288,6 +5288,7 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, virDomainDefPtr def; bool live; int ret = -1; + g_autoptr(virBitmap) hostcpus = NULL; virBitmapPtr autoCpuset = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -5302,11 +5303,14 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, if (!(def = virDomainObjGetOneDefState(vm, flags, &live))) goto cleanup; + if (!(hostcpus = virHostCPUGetAvailableCPUsBitmap())) + goto cleanup; + if (live) autoCpuset = QEMU_DOMAIN_PRIVATE(vm)->autoCpuset; ret = virDomainDefGetVcpuPinInfoHelper(def, maplen, ncpumaps, cpumaps, - virHostCPUGetCount(), autoCpuset); + hostcpus, autoCpuset); cleanup: virDomainObjEndAPI(&vm); return ret; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 01c4675c30..a75c992af1 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3072,6 +3072,7 @@ testDomainGetVcpuPinInfo(virDomainPtr dom, testDriverPtr driver = dom->conn->privateData; virDomainObjPtr privdom; virDomainDefPtr def; + g_autoptr(virBitmap) hostcpus = NULL; int ret = -1; if (!(privdom = testDomObjFromDomain(dom))) @@ -3080,9 +3081,12 @@ testDomainGetVcpuPinInfo(virDomainPtr dom, if (!(def = virDomainObjGetOneDef(privdom, flags))) goto cleanup; + if (!(hostcpus = virBitmapNew(VIR_NODEINFO_MAXCPUS(driver->nodeInfo)))) + goto cleanup; + virBitmapSetAll(hostcpus); + ret = virDomainDefGetVcpuPinInfoHelper(def, maplen, ncpumaps, cpumaps, - VIR_NODEINFO_MAXCPUS(driver->nodeInfo), - NULL); + hostcpus, NULL); cleanup: virDomainObjEndAPI(&privdom); -- 2.26.2

Commit <6328da04285d9f65cb323d399f731c20caf63f5a> introduced testDomainGetEmulatorPinInfo() into test driver but used virHostCPUGetCount() function to get the number of host CPUs. This would be correct for other drivers but in test driver we must not depend on the host, we have to use hard-coded host representation that we have in test driver. Follows the logic of testDomainGetVcpuPinInfo(). Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/test/test_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a75c992af1..d582c207f4 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2764,6 +2764,7 @@ testDomainGetEmulatorPinInfo(virDomainPtr dom, int maplen, unsigned int flags) { + testDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr def = NULL; virBitmapPtr cpumask = NULL; @@ -2780,8 +2781,7 @@ testDomainGetEmulatorPinInfo(virDomainPtr dom, if (!(def = virDomainObjGetOneDef(vm, flags))) goto cleanup; - if ((hostcpus = virHostCPUGetCount()) < 0) - goto cleanup; + hostcpus = VIR_NODEINFO_MAXCPUS(driver->nodeInfo); if (def->cputune.emulatorpin) { cpumask = def->cputune.emulatorpin; -- 2.26.2

Following the rationale from commit <2020c6af8a8e4bb04acb629d089142be984484c8> we should do the same thing for iothread info as well. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_driver.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9eaea8d613..07e9518a9b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5693,16 +5693,12 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, virDomainIOThreadInfoPtr *info_ret = NULL; virBitmapPtr bitmap = NULL; virBitmapPtr cpumask = NULL; - int hostcpus; size_t i; int ret = -1; if (targetDef->niothreadids == 0) return 0; - if ((hostcpus = virHostCPUGetCount()) < 0) - goto cleanup; - if (VIR_ALLOC_N(info_ret, targetDef->niothreadids) < 0) goto cleanup; @@ -5718,9 +5714,8 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, if (targetDef->cpumask) { cpumask = targetDef->cpumask; } else { - if (!(bitmap = virBitmapNew(hostcpus))) + if (!(bitmap = virHostCPUGetAvailableCPUsBitmap())) goto cleanup; - virBitmapSetAll(bitmap); cpumask = bitmap; } } -- 2.26.2