Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a5b38b3d24..d486ce5278 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5427,7 +5427,7 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, int ret = -1; int hostcpus; virBitmapPtr cpumask = NULL; - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL; virBitmapPtr autoCpuset = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -5468,7 +5468,6 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - virBitmapFree(bitmap); return ret; } -- 2.26.2

Use g_auto* in the string and in the bitmap. Remove the cleanup label since it's now unneeded. Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- tools/virsh-domain.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 085b88b097..23d41a4ddf 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6886,16 +6886,15 @@ virshVcpuinfoInactive(vshControl *ctl, int maxcpu, bool pretty) { - unsigned char *cpumaps = NULL; + g_autofree unsigned char *cpumaps = NULL; size_t cpumaplen; int ncpus; - virBitmapPtr vcpus = NULL; + g_autoptr(virBitmap) vcpus = NULL; ssize_t nextvcpu = -1; - bool ret = false; bool first = true; if (!(vcpus = virshDomainGetVcpuBitmap(ctl, dom, true))) - goto cleanup; + return false; cpumaplen = VIR_CPU_MAPLEN(maxcpu); cpumaps = vshMalloc(ctl, virBitmapSize(vcpus) * cpumaplen); @@ -6903,7 +6902,7 @@ virshVcpuinfoInactive(vshControl *ctl, if ((ncpus = virDomainGetVcpuPinInfo(dom, virBitmapSize(vcpus), cpumaps, cpumaplen, VIR_DOMAIN_AFFECT_CONFIG)) < 0) - goto cleanup; + return false; while ((nextvcpu = virBitmapNextSetBit(vcpus, nextvcpu)) >= 0) { if (!first) @@ -6918,15 +6917,10 @@ virshVcpuinfoInactive(vshControl *ctl, if (virshVcpuinfoPrintAffinity(ctl, VIR_GET_CPUMAP(cpumaps, cpumaplen, nextvcpu), maxcpu, pretty) < 0) - goto cleanup; + return false; } - ret = true; - - cleanup: - virBitmapFree(vcpus); - VIR_FREE(cpumaps); - return ret; + return true; } -- 2.26.2

This function reads the string in sysfspath/cpu/present and parses it manually to retrieve the number of present CPUs. virHostCPUGetPresentBitmap() reads and parses the same file, using a more robust parser via virBitmapParseUnlimited(), but returns a bitmap. Let's drop all the manual parsing done here and simply return the size of the resulting bitmap from virHostCPUGetPresentBitmap(). Given that no more parsing is being done manually in the function, rename it to virHostCPUCountLinux(). Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- src/util/virhostcpu.c | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 3023bca831..615250d05d 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -856,33 +856,17 @@ virHostCPUGetStatsLinux(FILE *procstat, } -/* Determine the number of CPUs (maximum CPU id + 1) from a file containing - * a list of CPU ids, like the Linux sysfs cpu/present file */ +/* Determine the number of CPUs (maximum CPU id + 1) present in + * the host. */ static int -virHostCPUParseCountLinux(void) +virHostCPUCountLinux(void) { - char *str = NULL; - char *tmp; - int ret = -1; + g_autoptr(virBitmap) present = virHostCPUGetPresentBitmap(); - if (virFileReadValueString(&str, "%s/cpu/present", SYSFS_SYSTEM_PATH) < 0) + if (!present) return -1; - tmp = str; - do { - if (virStrToLong_i(tmp, &tmp, 10, &ret) < 0 || - !strchr(",-", *tmp)) { - virReportError(VIR_ERR_NO_SUPPORT, - _("failed to parse %s"), str); - ret = -1; - goto cleanup; - } - } while (*tmp++ && *tmp); - ret++; - - cleanup: - VIR_FREE(str); - return ret; + return virBitmapSize(present); } #endif @@ -1031,7 +1015,7 @@ int virHostCPUGetCount(void) { #if defined(__linux__) - return virHostCPUParseCountLinux(); + return virHostCPUCountLinux(); #elif defined(__FreeBSD__) || defined(__APPLE__) return virHostCPUGetCountAppleFreeBSD(); #else -- 2.26.2

The output of vcpupin and emulatorpin for a domain with vcpu placement='static' is based on a default bitmap that contains all possible CPUs in the host, regardless of the CPUs being offline or not. E.g. for a Linux host with this CPU setup (from lscpu): On-line CPU(s) list: 0,8,16,24,32,40,(...),184 Off-line CPU(s) list: 1-7,9-15,17-23,25-31,(...),185-191 And a domain with this configuration: <vcpu placement='static'>1</vcpu> 'virsh vcpupin' will return the following: $ sudo ./run tools/virsh vcpupin vcpupin_test VCPU CPU Affinity ---------------------- 0 0-191 This is benign by its own, but can make the user believe that all CPUs from the 0-191 range are eligible for pinning. Which can lead to situations like this: $ sudo ./run tools/virsh vcpupin vcpupin_test 0 1 error: Invalid value '1' for 'cpuset.cpus': Invalid argument This is exarcebated by the fact that 'virsh vcpuinfo' considers only available host CPUs in the 'CPU Affinity' field: $ sudo ./run tools/virsh vcpuinfo vcpupin_test (...) CPU Affinity: y-------y-------y-------(...) This patch changes the default bitmap of vcpupin and emulatorpin, in the case of domains with static vcpu placement, to all available CPUs instead of all possible CPUs. Aside from making it consistent with the behavior of 'vcpuinfo', users will now have one less incentive to try to pin a vcpu in an offline CPU. https://bugzilla.redhat.com/show_bug.cgi?id=1434276 Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- src/conf/domain_conf.c | 4 +--- src/qemu/qemu_driver.c | 7 +------ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 31ba78b950..6f95023aaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2089,11 +2089,9 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, if (hostcpus < 0) return -1; - if (!(allcpumap = virBitmapNew(hostcpus))) + if (!(allcpumap = virHostCPUGetAvailableCPUsBitmap())) return -1; - virBitmapSetAll(allcpumap); - for (i = 0; i < maxvcpus && i < ncpumaps; i++) { virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i); virBitmapPtr bitmap = NULL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d486ce5278..22f0313394 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5425,7 +5425,6 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, virDomainDefPtr def; bool live; int ret = -1; - int hostcpus; virBitmapPtr cpumask = NULL; g_autoptr(virBitmap) bitmap = NULL; virBitmapPtr autoCpuset = NULL; @@ -5442,9 +5441,6 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, if (!(def = virDomainObjGetOneDefState(vm, flags, &live))) goto cleanup; - if ((hostcpus = virHostCPUGetCount()) < 0) - goto cleanup; - if (live) autoCpuset = QEMU_DOMAIN_PRIVATE(vm)->autoCpuset; @@ -5456,9 +5452,8 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, autoCpuset) { cpumask = autoCpuset; } else { - if (!(bitmap = virBitmapNew(hostcpus))) + if (!(bitmap = virHostCPUGetAvailableCPUsBitmap())) goto cleanup; - virBitmapSetAll(bitmap); cpumask = bitmap; } -- 2.26.2