Re: [libvirt] [PATCH 2/9] conf: Extract code filling data for virDomainGetVcpuPinInfo
On 02/24/2016 09:22 AM, Peter Krempa wrote:
The implementation of the inner guts of the function is similar for
all
drivers, so we can add a helper and not have to reimplement it three
times.
---
src/conf/domain_conf.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 8 ++++++
src/libvirt_private.syms | 1 +
src/libxl/libxl_driver.c | 38 +++-------------------------
src/qemu/qemu_driver.c | 43 +++-----------------------------
src/test/test_driver.c | 38 ++++------------------------
6 files changed, 84 insertions(+), 108 deletions(-)
ACK with one nit...
One difference I noted, the 'libxl' code would:
memset(cpumaps, 0x00, maplen * ncpumaps);
just before filling. Should that be replicated in the common code? If
not then for the libxl code should the lines be kept?
John
[...]
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 404016e..a99c99c 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2423,8 +2423,7 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainObjPtr vm = NULL;
virDomainDefPtr targetDef = NULL;
- int hostcpus, vcpu, ret = -1;
- virBitmapPtr allcpumap = NULL;
+ int ret = -1;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -2445,41 +2444,10 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
/* Make sure coverity knows targetDef is valid at this point. */
sa_assert(targetDef);
- /* Clamp to actual number of vcpus */
- if (ncpumaps > virDomainDefGetVcpus(targetDef))
- ncpumaps = virDomainDefGetVcpus(targetDef);
-
- if ((hostcpus = libxl_get_max_cpus(cfg->ctx)) < 0)
- goto cleanup;
-
- if (!(allcpumap = virBitmapNew(hostcpus)))
- goto cleanup;
-
- virBitmapSetAll(allcpumap);
-
- memset(cpumaps, 0x00, maplen * ncpumaps);
^^^^^
-
- for (vcpu = 0; vcpu < ncpumaps; vcpu++) {
- virDomainVcpuInfoPtr vcpuinfo = virDomainDefGetVcpu(targetDef, vcpu);
- virBitmapPtr bitmap = NULL;
-
- if (!vcpuinfo->online)
- continue;
-
- if (vcpuinfo->cpumask)
- bitmap = vcpuinfo->cpumask;
- else if (targetDef->cpumask)
- bitmap = targetDef->cpumask;
- else
- bitmap = allcpumap;
-
- virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, vcpu), maplen);
- }
-
- ret = ncpumaps;
+ ret = virDomainDefGetVcpuPinInfoHelper(targetDef, maplen, ncpumaps, cpumaps,
+ libxl_get_max_cpus(cfg->ctx), NULL);
cleanup:
- virBitmapFree(allcpumap);
if (vm)
virObjectUnlock(vm);
virObjectUnref(cfg);
[...]