[libvirt] [PATCH 00/11] vCPU information storage improvements

Cleanups and improvements that will make adding the new vCPU hotplug that was recently added to qemu easier. Peter Krempa (11): conf: Annotate that private data for objects are not copied conf: Extract code formatting vCPU info conf: Rename virDomainVcpuInfoPtr to virDomainVcpuDefPtr conf: Don't report errors from virDomainDefGetVcpu tests: qemuxml2xml: Format status XML header dynamically conf: convert def->vcpus to a array of pointers conf: Add private data for virDomainVcpuDef qemu: domain: Add vcpu private data structure qemu: domain: Extract formating and parsing of vCPU thread ids qemu: Add cpu ID to the vCPU pid list in the status XML qemu: Store vCPU thread ids in vcpu private data objects src/conf/domain_conf.c | 127 +++++++++++++++++--------- src/conf/domain_conf.h | 32 ++++--- src/hyperv/hyperv_driver.c | 3 +- src/libxl/libxl_domain.c | 2 +- src/libxl/libxl_driver.c | 6 +- src/lxc/lxc_native.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 16 ++-- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_domain.c | 206 +++++++++++++++++++++++++++++++----------- src/qemu/qemu_domain.h | 18 +++- src/qemu/qemu_driver.c | 22 ++--- src/qemu/qemu_parse_command.c | 9 +- src/qemu/qemu_process.c | 6 +- src/test/test_driver.c | 8 +- src/vbox/vbox_common.c | 4 +- src/vmx/vmx.c | 2 +- src/vz/vz_sdk.c | 2 +- src/xen/xm_internal.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- src/xenconfig/xen_common.c | 13 ++- src/xenconfig/xen_common.h | 3 +- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 3 +- src/xenconfig/xen_xm.c | 3 +- tests/qemuxml2xmltest.c | 73 ++++++++++++--- 27 files changed, 396 insertions(+), 176 deletions(-) -- 2.9.0

Our copy functions format and parse XML thus are not able to copy data. Annotate the private data pointers that this is happening. --- src/conf/domain_conf.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a1acebe..eb3a5f3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2404,6 +2404,8 @@ typedef virDomainXMLPrivateDataCallbacks *virDomainXMLPrivateDataCallbacksPtr; struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataAllocFunc alloc; virDomainXMLPrivateDataFreeFunc free; + /* note that private data for devices are not copied when using + * virDomainDefCopy and similar functions */ virDomainXMLPrivateDataNewFunc diskNew; virDomainXMLPrivateDataNewFunc hostdevNew; virDomainXMLPrivateDataFormatFunc format; -- 2.9.0

--- src/conf/domain_conf.c | 45 +++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cf5eb1d..74075f0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -22779,6 +22779,35 @@ virDomainCputuneDefFormat(virBufferPtr buf, } +static int +virDomainCpuDefFormat(virBufferPtr buf, + const virDomainDef *def) +{ + char *cpumask = NULL; + int ret = -1; + + virBufferAddLit(buf, "<vcpu"); + virBufferAsprintf(buf, " placement='%s'", + virDomainCpuPlacementModeTypeToString(def->placement_mode)); + + if (def->cpumask && !virBitmapIsAllSet(def->cpumask)) { + if ((cpumask = virBitmapFormat(def->cpumask)) == NULL) + goto cleanup; + virBufferAsprintf(buf, " cpuset='%s'", cpumask); + } + if (virDomainDefHasVcpusOffline(def)) + virBufferAsprintf(buf, " current='%u'", virDomainDefGetVcpus(def)); + virBufferAsprintf(buf, ">%u</vcpu>\n", virDomainDefGetVcpusMax(def)); + + ret = 0; + + cleanup: + VIR_FREE(cpumask); + + return ret; +} + + /* This internal version appends to an existing buffer * (possibly with auto-indent), rather than flattening * to string. @@ -22954,20 +22983,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</memoryBacking>\n"); } - virBufferAddLit(buf, "<vcpu"); - virBufferAsprintf(buf, " placement='%s'", - virDomainCpuPlacementModeTypeToString(def->placement_mode)); - - if (def->cpumask && !virBitmapIsAllSet(def->cpumask)) { - char *cpumask = NULL; - if ((cpumask = virBitmapFormat(def->cpumask)) == NULL) - goto error; - virBufferAsprintf(buf, " cpuset='%s'", cpumask); - VIR_FREE(cpumask); - } - if (virDomainDefHasVcpusOffline(def)) - virBufferAsprintf(buf, " current='%u'", virDomainDefGetVcpus(def)); - virBufferAsprintf(buf, ">%u</vcpu>\n", virDomainDefGetVcpusMax(def)); + if (virDomainCpuDefFormat(buf, def) < 0) + goto error; if (def->niothreadids > 0) { virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n", -- 2.9.0

--- src/conf/domain_conf.c | 22 +++++++++++----------- src/conf/domain_conf.h | 10 +++++----- src/libxl/libxl_domain.c | 2 +- src/libxl/libxl_driver.c | 2 +- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_driver.c | 14 +++++++------- src/qemu/qemu_process.c | 4 ++-- src/test/test_driver.c | 4 ++-- src/vz/vz_sdk.c | 2 +- 9 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 74075f0..b660a8b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1307,7 +1307,7 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) static void -virDomainVcpuInfoClear(virDomainVcpuInfoPtr info) +virDomainVcpuDefClear(virDomainVcpuDefPtr info) { if (!info) return; @@ -1331,7 +1331,7 @@ virDomainDefSetVcpusMax(virDomainDefPtr def, return -1; } else { for (i = maxvcpus; i < def->maxvcpus; i++) - virDomainVcpuInfoClear(&def->vcpus[i]); + virDomainVcpuDefClear(&def->vcpus[i]); VIR_SHRINK_N(def->vcpus, def->maxvcpus, def->maxvcpus - maxvcpus); } @@ -1423,7 +1423,7 @@ virDomainDefGetOnlineVcpumap(const virDomainDef *def) } -virDomainVcpuInfoPtr +virDomainVcpuDefPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) { @@ -1442,7 +1442,7 @@ static virDomainThreadSchedParamPtr virDomainDefGetVcpuSched(virDomainDefPtr def, unsigned int vcpu) { - virDomainVcpuInfoPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo; if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) return NULL; @@ -1508,7 +1508,7 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, virBitmapSetAll(allcpumap); for (i = 0; i < maxvcpus && i < ncpumaps; i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i); virBitmapPtr bitmap = NULL; if (vcpu->cpumask) @@ -2522,7 +2522,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainResourceDefFree(def->resource); for (i = 0; i < def->maxvcpus; i++) - virDomainVcpuInfoClear(&def->vcpus[i]); + virDomainVcpuDefClear(&def->vcpus[i]); VIR_FREE(def->vcpus); /* hostdevs must be freed before nets (or any future "intelligent @@ -4178,7 +4178,7 @@ static void virDomainDefRemoveOfflineVcpuPin(virDomainDefPtr def) { size_t i; - virDomainVcpuInfoPtr vcpu; + virDomainVcpuDefPtr vcpu; for (i = 0; i < virDomainDefGetVcpusMax(def); i++) { vcpu = virDomainDefGetVcpu(def, i); @@ -14885,7 +14885,7 @@ static int virDomainVcpuPinDefParseXML(virDomainDefPtr def, xmlNodePtr node) { - virDomainVcpuInfoPtr vcpu; + virDomainVcpuDefPtr vcpu; unsigned int vcpuid; char *tmp = NULL; int ret = -1; @@ -18518,8 +18518,8 @@ virDomainDefVcpuCheckAbiStability(virDomainDefPtr src, } for (i = 0; i < src->maxvcpus; i++) { - virDomainVcpuInfoPtr svcpu = &src->vcpus[i]; - virDomainVcpuInfoPtr dvcpu = &dst->vcpus[i]; + virDomainVcpuDefPtr svcpu = &src->vcpus[i]; + virDomainVcpuDefPtr dvcpu = &dst->vcpus[i]; if (svcpu->online != dvcpu->online) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -22717,7 +22717,7 @@ virDomainCputuneDefFormat(virBufferPtr buf, for (i = 0; i < def->maxvcpus; i++) { char *cpumask; - virDomainVcpuInfoPtr vcpu = def->vcpus + i; + virDomainVcpuDefPtr vcpu = def->vcpus + i; if (!vcpu->cpumask) continue; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index eb3a5f3..2620b22 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2029,10 +2029,10 @@ struct _virDomainCputune { }; -typedef struct _virDomainVcpuInfo virDomainVcpuInfo; -typedef virDomainVcpuInfo *virDomainVcpuInfoPtr; +typedef struct _virDomainVcpuDef virDomainVcpuDef; +typedef virDomainVcpuDef *virDomainVcpuDefPtr; -struct _virDomainVcpuInfo { +struct _virDomainVcpuDef { bool online; virBitmapPtr cpumask; @@ -2117,7 +2117,7 @@ struct _virDomainDef { virDomainBlkiotune blkio; virDomainMemtune mem; - virDomainVcpuInfoPtr vcpus; + virDomainVcpuDefPtr vcpus; size_t maxvcpus; int placement_mode; virBitmapPtr cpumask; @@ -2251,7 +2251,7 @@ unsigned int virDomainDefGetVcpusMax(const virDomainDef *def); int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus); unsigned int virDomainDefGetVcpus(const virDomainDef *def); virBitmapPtr virDomainDefGetOnlineVcpumap(const virDomainDef *def); -virDomainVcpuInfoPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) +virDomainVcpuDefPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) ATTRIBUTE_RETURN_CHECK; unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 9feba08..0e26b91 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -841,7 +841,7 @@ int libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); - virDomainVcpuInfoPtr vcpu; + virDomainVcpuDefPtr vcpu; libxl_bitmap map; virBitmapPtr cpumask = NULL; size_t i; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 2661168..0e762f3 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2314,7 +2314,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainDefPtr targetDef = NULL; virBitmapPtr pcpumap = NULL; - virDomainVcpuInfoPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo; virDomainObjPtr vm; int ret = -1; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1e04a68..2dca874 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -874,7 +874,7 @@ qemuRestoreCgroupState(virDomainObjPtr vm) goto error; for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); if (!vcpu->online) continue; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f9a3b15..724e8ac 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1475,7 +1475,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo); for (i = 0; i < virDomainDefGetVcpusMax(vm->def) && ncpuinfo < maxinfo; i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); pid_t vcpupid = qemuDomainGetVcpuPid(vm, i); if (!vcpu->online) @@ -4606,7 +4606,7 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, unsigned int vcpu) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainVcpuInfoPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo; int ret = -1; int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); @@ -4658,7 +4658,7 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, unsigned int vcpu) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainVcpuInfoPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo; int ret = -1; int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); @@ -4905,7 +4905,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, virBitmapPtr cpumap) { virBitmapPtr tmpmap = NULL; - virDomainVcpuInfoPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_vcpu = NULL; char *str = NULL; @@ -4992,7 +4992,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, virDomainDefPtr persistentDef; int ret = -1; virBitmapPtr pcpumap = NULL; - virDomainVcpuInfoPtr vcpuinfo = NULL; + virDomainVcpuDefPtr vcpuinfo = NULL; virQEMUDriverConfigPtr cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -9346,7 +9346,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, virCgroupFree(&cgroup_temp); for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); if (!vcpu->online) continue; @@ -9757,7 +9757,7 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, return 0; for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); if (!vcpu->online) continue; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 129c070..96eaeb2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4577,7 +4577,7 @@ qemuProcessSetupVcpu(virDomainObjPtr vm, unsigned int vcpuid) { pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid); - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); qemuDomainObjPrivatePtr priv = vm->privateData; char *mem_mask = NULL; virDomainNumatuneMemMode mem_mode; @@ -4658,7 +4658,7 @@ qemuProcessSetupVcpu(virDomainObjPtr vm, static int qemuProcessSetupVcpus(virDomainObjPtr vm) { - virDomainVcpuInfoPtr vcpu; + virDomainVcpuDefPtr vcpu; unsigned int maxvcpus = virDomainDefGetVcpusMax(vm->def); size_t i; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4ed631b..6853626 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2457,7 +2457,7 @@ static int testDomainGetVcpus(virDomainPtr domain, memset(cpumaps, 0, maxinfo * maplen); for (i = 0; i < maxinfo; i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i); virBitmapPtr bitmap = NULL; if (!vcpu->online) @@ -2493,7 +2493,7 @@ static int testDomainPinVcpu(virDomainPtr domain, unsigned char *cpumap, int maplen) { - virDomainVcpuInfoPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo; virDomainObjPtr privdom; virDomainDefPtr def; int ret = -1; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index ae218e9..9d0bc0d 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2284,7 +2284,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def) } for (i = 0; i < virDomainDefGetVcpusMax(def); i++) { - virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i); if (vcpu->cpumask && !virBitmapEqual(def->cpumask, vcpu->cpumask)) { -- 2.9.0

Most callers make sure that it's never called with an out of range vCPU. Every other caller reports a different error explicitly. Drop the error reporting and clean up some dead code paths. --- src/conf/domain_conf.c | 6 +----- src/qemu/qemu_driver.c | 10 ++-------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b660a8b..a3aa83e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1427,12 +1427,8 @@ virDomainVcpuDefPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) { - if (vcpu >= def->maxvcpus) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("vCPU '%u' is not present in domain definition"), - vcpu); + if (vcpu >= def->maxvcpus) return NULL; - } return &def->vcpus[vcpu]; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 724e8ac..b993995 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4606,14 +4606,11 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, unsigned int vcpu) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainVcpuDefPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu); int ret = -1; int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); - if (!(vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu))) - return -1; - if (vcpuinfo->online) { virReportError(VIR_ERR_INVALID_ARG, _("vCPU '%u' is already online"), vcpu); @@ -4658,14 +4655,11 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, unsigned int vcpu) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainVcpuDefPtr vcpuinfo; + virDomainVcpuDefPtr vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu); int ret = -1; int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); - if (!(vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu))) - return -1; - if (!vcpuinfo->online) { virReportError(VIR_ERR_INVALID_ARG, _("vCPU '%u' is already offline"), vcpu); -- 2.9.0

Status XML tests were done by prepending a constant string to an existing XML. With the planned changes the header will depend on data present in the definition rather than just on the data that was parsed. The first dynamic element in the header will be the vcpu thread list. Reuse and rename qemuXML2XMLPreFormatCallback for gathering the relevant data when checking the active XML parsing and formating and pass the bitmap to a newly crated header generator. --- tests/qemuxml2xmltest.c | 72 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 11 deletions(-) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ae328c2..a672fbb 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -33,13 +33,21 @@ struct testInfo { char *outActiveName; char *outInactiveName; + virBitmapPtr activeVcpus; + virQEMUCapsPtr qemuCaps; }; static int -qemuXML2XMLPreFormatCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, - const void *opaque ATTRIBUTE_UNUSED) +qemuXML2XMLActivePreFormatCallback(virDomainDefPtr def, + const void *opaque) { + struct testInfo *info = (struct testInfo *) opaque; + + /* store vCPU bitmap so that the status XML can be created faithfully */ + if (!info->activeVcpus) + info->activeVcpus = virDomainDefGetOnlineVcpumap(def); + return 0; } @@ -50,7 +58,8 @@ testXML2XMLActive(const void *opaque) return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName, info->outActiveName, true, - qemuXML2XMLPreFormatCallback, opaque, 0, + qemuXML2XMLActivePreFormatCallback, + opaque, 0, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); } @@ -62,18 +71,17 @@ testXML2XMLInactive(const void *opaque) return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName, info->outInactiveName, false, - qemuXML2XMLPreFormatCallback, opaque, 0, + NULL, opaque, 0, TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS); } -static const char testStatusXMLPrefix[] = +static const char testStatusXMLPrefixHeader[] = "<domstatus state='running' reason='booted' pid='3803518'>\n" " <taint flag='high-privileges'/>\n" -" <monitor path='/var/lib/libvirt/qemu/test.monitor' json='1' type='unix'/>\n" -" <vcpus>\n" -" <vcpu pid='3803519'/>\n" -" </vcpus>\n" +" <monitor path='/var/lib/libvirt/qemu/test.monitor' json='1' type='unix'/>\n"; + +static const char testStatusXMLPrefixFooter[] = " <qemuCaps>\n" " <flag name='vnet-hdr'/>\n" " <flag name='qxl.vgamem_mb'/>\n" @@ -95,6 +103,40 @@ static const char testStatusXMLSuffix[] = "</domstatus>\n"; +static void +testGetStatuXMLPrefixVcpus(virBufferPtr buf, + const struct testInfo *data) +{ + ssize_t vcpuid = -1; + + virBufferAddLit(buf, "<vcpus>\n"); + virBufferAdjustIndent(buf, 2); + + while ((vcpuid = virBitmapNextSetBit(data->activeVcpus, vcpuid)) >= 0) + virBufferAsprintf(buf, "<vcpu pid='%zd'/>\n", vcpuid + 3803519); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</vcpus>\n"); +} + + +static char * +testGetStatusXMLPrefix(const struct testInfo *data) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAdd(&buf, testStatusXMLPrefixHeader, -1); + virBufferAdjustIndent(&buf, 2); + + testGetStatuXMLPrefixVcpus(&buf, data); + + virBufferAdjustIndent(&buf, -2); + virBufferAdd(&buf, testStatusXMLPrefixFooter, -1); + + return virBufferContentAndReset(&buf); +} + + static int testCompareStatusXMLToXMLFiles(const void *opaque) { @@ -105,6 +147,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque) char *expect = NULL; char *actual = NULL; char *source = NULL; + char *header = NULL; char *inFile = NULL, *outActiveFile = NULL; int ret = -1; int keepBlanksDefault = xmlKeepBlanksDefault(0); @@ -114,8 +157,11 @@ testCompareStatusXMLToXMLFiles(const void *opaque) if (virTestLoadFile(data->outActiveName, &outActiveFile) < 0) goto cleanup; + if (!(header = testGetStatusXMLPrefix(data))) + goto cleanup; + /* construct faked source status XML */ - virBufferAdd(&buf, testStatusXMLPrefix, -1); + virBufferAdd(&buf, header, -1); virBufferAdjustIndent(&buf, 2); virBufferAddStr(&buf, inFile); virBufferAdjustIndent(&buf, -2); @@ -127,7 +173,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque) } /* construct the expect string */ - virBufferAdd(&buf, testStatusXMLPrefix, -1); + virBufferAdd(&buf, header, -1); virBufferAdjustIndent(&buf, 2); virBufferAddStr(&buf, outActiveFile); virBufferAdjustIndent(&buf, -2); @@ -175,6 +221,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque) VIR_FREE(actual); VIR_FREE(source); VIR_FREE(inFile); + VIR_FREE(header); VIR_FREE(outActiveFile); return ret; } @@ -187,6 +234,9 @@ testInfoFree(struct testInfo *info) VIR_FREE(info->outActiveName); VIR_FREE(info->outInactiveName); + virBitmapFree(info->activeVcpus); + info->activeVcpus = NULL; + virObjectUnref(info->qemuCaps); } -- 2.9.0

On 07.07.2016 15:41, Peter Krempa wrote:
Status XML tests were done by prepending a constant string to an existing XML. With the planned changes the header will depend on data present in the definition rather than just on the data that was parsed.
The first dynamic element in the header will be the vcpu thread list. Reuse and rename qemuXML2XMLPreFormatCallback for gathering the relevant data when checking the active XML parsing and formating and pass the bitmap to a newly crated header generator. --- tests/qemuxml2xmltest.c | 72 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 61 insertions(+), 11 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ae328c2..a672fbb 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -33,13 +33,21 @@ struct testInfo { char *outActiveName; char *outInactiveName;
+ virBitmapPtr activeVcpus; + virQEMUCapsPtr qemuCaps; };
static int -qemuXML2XMLPreFormatCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, - const void *opaque ATTRIBUTE_UNUSED) +qemuXML2XMLActivePreFormatCallback(virDomainDefPtr def, + const void *opaque) { + struct testInfo *info = (struct testInfo *) opaque; + + /* store vCPU bitmap so that the status XML can be created faithfully */ + if (!info->activeVcpus) + info->activeVcpus = virDomainDefGetOnlineVcpumap(def);
This won't fly, @info is never cleared, in particular, its @activeVcpus is never set so it will contain some garbage that's on the stack when we get here: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff1d76824 in virBitmapNextSetBit (bitmap=0x432bc0 <_start>, pos=0) at util/virbitmap.c:930 930 bits = bitmap->map[nl] & ~((1UL << nb) - 1); (gdb) bt #0 0x00007ffff1d76824 in virBitmapNextSetBit (bitmap=0x432bc0 <_start>, pos=0) at util/virbitmap.c:930 #1 0x0000000000432e1e in testGetStatuXMLPrefixVcpus (buf=0x7fffffffd560, data=0x7fffffffd680) at qemuxml2xmltest.c:115 #2 0x0000000000432ecb in testGetStatusXMLPrefix (data=0x7fffffffd680) at qemuxml2xmltest.c:131 #3 0x0000000000433003 in testCompareStatusXMLToXMLFiles (opaque=0x7fffffffd680) at qemuxml2xmltest.c:160 #4 0x0000000000447a52 in virTestRun (title=0x55d07e "QEMU XML-2-XML-status minimal", body=0x432f14 <testCompareStatusXMLToXMLFiles>, data=0x7fffffffd680) at testutils.c:179 #5 0x000000000043375f in mymain () at qemuxml2xmltest.c:360 #6 0x0000000000449447 in virTestMain (argc=1, argv=0x7fffffffd8e8, func=0x43361c <mymain>) at testutils.c:969 #7 0x0000000000446973 in main (argc=1, argv=0x7fffffffd8e8) at qemuxml2xmltest.c:891 ACK with this squashed in: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a672fbb..eb392f4 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -311,6 +311,8 @@ mymain(void) struct testInfo info; virQEMUDriverConfigPtr cfg = NULL; + memset(&info, 0, sizeof(info)); + if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; Michal

--- src/conf/domain_conf.c | 44 +++++++++++++++++++++++++++++++------------- src/conf/domain_conf.h | 2 +- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3aa83e..c2d7259 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1306,14 +1306,26 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) } +static virDomainVcpuDefPtr +virDomainVcpuDefNew(void) +{ + virDomainVcpuDefPtr ret; + + ignore_value(VIR_ALLOC(ret)); + + return ret; +} + + static void -virDomainVcpuDefClear(virDomainVcpuDefPtr info) +virDomainVcpuDefFree(virDomainVcpuDefPtr info) { if (!info) return; virBitmapFree(info->cpumask); info->cpumask = NULL; + VIR_FREE(info); } @@ -1321,6 +1333,7 @@ int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int maxvcpus) { + size_t oldmax = def->maxvcpus; size_t i; if (def->maxvcpus == maxvcpus) @@ -1329,9 +1342,14 @@ virDomainDefSetVcpusMax(virDomainDefPtr def, if (def->maxvcpus < maxvcpus) { if (VIR_EXPAND_N(def->vcpus, def->maxvcpus, maxvcpus - def->maxvcpus) < 0) return -1; + + for (i = oldmax; i < def->maxvcpus; i++) { + if (!(def->vcpus[i] = virDomainVcpuDefNew())) + return -1; + } } else { for (i = maxvcpus; i < def->maxvcpus; i++) - virDomainVcpuDefClear(&def->vcpus[i]); + virDomainVcpuDefFree(def->vcpus[i]); VIR_SHRINK_N(def->vcpus, def->maxvcpus, def->maxvcpus - maxvcpus); } @@ -1346,7 +1364,7 @@ virDomainDefHasVcpusOffline(const virDomainDef *def) size_t i; for (i = 0; i < def->maxvcpus; i++) { - if (!def->vcpus[i].online) + if (!def->vcpus[i]->online) return true; } @@ -1375,10 +1393,10 @@ virDomainDefSetVcpus(virDomainDefPtr def, } for (i = 0; i < vcpus; i++) - def->vcpus[i].online = true; + def->vcpus[i]->online = true; for (i = vcpus; i < def->maxvcpus; i++) - def->vcpus[i].online = false; + def->vcpus[i]->online = false; return 0; } @@ -1391,7 +1409,7 @@ virDomainDefGetVcpus(const virDomainDef *def) unsigned int ret = 0; for (i = 0; i < def->maxvcpus; i++) { - if (def->vcpus[i].online) + if (def->vcpus[i]->online) ret++; } @@ -1415,7 +1433,7 @@ virDomainDefGetOnlineVcpumap(const virDomainDef *def) return NULL; for (i = 0; i < def->maxvcpus; i++) { - if (def->vcpus[i].online) + if (def->vcpus[i]->online) ignore_value(virBitmapSetBit(ret, i)); } @@ -1430,7 +1448,7 @@ virDomainDefGetVcpu(virDomainDefPtr def, if (vcpu >= def->maxvcpus) return NULL; - return &def->vcpus[vcpu]; + return def->vcpus[vcpu]; } @@ -1459,7 +1477,7 @@ virDomainDefHasVcpuPin(const virDomainDef *def) size_t i; for (i = 0; i < def->maxvcpus; i++) { - if (def->vcpus[i].cpumask) + if (def->vcpus[i]->cpumask) return true; } @@ -2518,7 +2536,7 @@ void virDomainDefFree(virDomainDefPtr def) virDomainResourceDefFree(def->resource); for (i = 0; i < def->maxvcpus; i++) - virDomainVcpuDefClear(&def->vcpus[i]); + virDomainVcpuDefFree(def->vcpus[i]); VIR_FREE(def->vcpus); /* hostdevs must be freed before nets (or any future "intelligent @@ -18514,8 +18532,8 @@ virDomainDefVcpuCheckAbiStability(virDomainDefPtr src, } for (i = 0; i < src->maxvcpus; i++) { - virDomainVcpuDefPtr svcpu = &src->vcpus[i]; - virDomainVcpuDefPtr dvcpu = &dst->vcpus[i]; + virDomainVcpuDefPtr svcpu = src->vcpus[i]; + virDomainVcpuDefPtr dvcpu = dst->vcpus[i]; if (svcpu->online != dvcpu->online) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -22713,7 +22731,7 @@ virDomainCputuneDefFormat(virBufferPtr buf, for (i = 0; i < def->maxvcpus; i++) { char *cpumask; - virDomainVcpuDefPtr vcpu = def->vcpus + i; + virDomainVcpuDefPtr vcpu = def->vcpus[i]; if (!vcpu->cpumask) continue; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2620b22..3fd6c07 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2117,7 +2117,7 @@ struct _virDomainDef { virDomainBlkiotune blkio; virDomainMemtune mem; - virDomainVcpuDefPtr vcpus; + virDomainVcpuDefPtr *vcpus; size_t maxvcpus; int placement_mode; virBitmapPtr cpumask; -- 2.9.0

Allow to store driver specific data on a per-vcpu basis. Move of the virDomainDef*Vcpus* functions was necessary as virDomainXMLOptionPtr was declared below this block and I didn't want to split the function headers. --- src/conf/domain_conf.c | 28 +++++++++++++++++++++------- src/conf/domain_conf.h | 22 ++++++++++++++-------- src/hyperv/hyperv_driver.c | 3 ++- src/libxl/libxl_driver.c | 4 ++-- src/lxc/lxc_native.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 16 ++++++++++------ src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_parse_command.c | 9 +++++---- src/test/test_driver.c | 4 +++- src/vbox/vbox_common.c | 4 ++-- src/vmx/vmx.c | 2 +- src/xen/xm_internal.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- src/xenconfig/xen_common.c | 13 ++++++++----- src/xenconfig/xen_common.h | 3 ++- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 3 ++- src/xenconfig/xen_xm.c | 3 ++- 20 files changed, 81 insertions(+), 47 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c2d7259..e660f8e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1307,12 +1307,23 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) static virDomainVcpuDefPtr -virDomainVcpuDefNew(void) +virDomainVcpuDefNew(virDomainXMLOptionPtr xmlopt) { + virObjectPtr priv = NULL; virDomainVcpuDefPtr ret; - ignore_value(VIR_ALLOC(ret)); + if (xmlopt && xmlopt->privateData.vcpuNew && + !(priv = xmlopt->privateData.vcpuNew())) + goto cleanup; + + if (VIR_ALLOC(ret) < 0) + goto cleanup; + + ret->privateData = priv; + priv = NULL; + cleanup: + virObjectUnref(priv); return ret; } @@ -1325,13 +1336,15 @@ virDomainVcpuDefFree(virDomainVcpuDefPtr info) virBitmapFree(info->cpumask); info->cpumask = NULL; + virObjectUnref(info->privateData); VIR_FREE(info); } int virDomainDefSetVcpusMax(virDomainDefPtr def, - unsigned int maxvcpus) + unsigned int maxvcpus, + virDomainXMLOptionPtr xmlopt) { size_t oldmax = def->maxvcpus; size_t i; @@ -1344,7 +1357,7 @@ virDomainDefSetVcpusMax(virDomainDefPtr def, return -1; for (i = oldmax; i < def->maxvcpus; i++) { - if (!(def->vcpus[i] = virDomainVcpuDefNew())) + if (!(def->vcpus[i] = virDomainVcpuDefNew(xmlopt))) return -1; } } else { @@ -15465,7 +15478,8 @@ virDomainIOThreadSchedParse(xmlNodePtr node, static int virDomainVcpuParse(virDomainDefPtr def, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + virDomainXMLOptionPtr xmlopt) { int n; char *tmp = NULL; @@ -15483,7 +15497,7 @@ virDomainVcpuParse(virDomainDefPtr def, maxvcpus = 1; } - if (virDomainDefSetVcpusMax(def, maxvcpus) < 0) + if (virDomainDefSetVcpusMax(def, maxvcpus, xmlopt) < 0) goto cleanup; if ((n = virXPathUInt("string(./vcpu[1]/@current)", ctxt, &vcpus)) < 0) { @@ -15952,7 +15966,7 @@ virDomainDefParseXML(xmlDocPtr xml, &def->mem.swap_hard_limit) < 0) goto error; - if (virDomainVcpuParse(def, ctxt) < 0) + if (virDomainVcpuParse(def, ctxt, xmlopt) < 0) goto error; /* Optional - iothreads */ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3fd6c07..c34fc50 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2037,6 +2037,8 @@ struct _virDomainVcpuDef { virBitmapPtr cpumask; virDomainThreadSchedParam sched; + + virObjectPtr privateData; }; typedef struct _virDomainBlkiotune virDomainBlkiotune; @@ -2245,14 +2247,6 @@ struct _virDomainDef { xmlNodePtr metadata; }; -int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int vcpus); -bool virDomainDefHasVcpusOffline(const virDomainDef *def); -unsigned int virDomainDefGetVcpusMax(const virDomainDef *def); -int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus); -unsigned int virDomainDefGetVcpus(const virDomainDef *def); -virBitmapPtr virDomainDefGetOnlineVcpumap(const virDomainDef *def); -virDomainVcpuDefPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) - ATTRIBUTE_RETURN_CHECK; unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size); @@ -2408,6 +2402,7 @@ struct _virDomainXMLPrivateDataCallbacks { * virDomainDefCopy and similar functions */ virDomainXMLPrivateDataNewFunc diskNew; virDomainXMLPrivateDataNewFunc hostdevNew; + virDomainXMLPrivateDataNewFunc vcpuNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; }; @@ -2439,6 +2434,17 @@ virDomainObjIsActive(virDomainObjPtr dom) return dom->def->id != -1; } +int virDomainDefSetVcpusMax(virDomainDefPtr def, + unsigned int vcpus, + virDomainXMLOptionPtr xmlopt); +bool virDomainDefHasVcpusOffline(const virDomainDef *def); +unsigned int virDomainDefGetVcpusMax(const virDomainDef *def); +int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus); +unsigned int virDomainDefGetVcpus(const virDomainDef *def); +virBitmapPtr virDomainDefGetOnlineVcpumap(const virDomainDef *def); +virDomainVcpuDefPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) + ATTRIBUTE_RETURN_CHECK; + virDomainObjPtr virDomainObjNew(virDomainXMLOptionPtr caps) ATTRIBUTE_NONNULL(1); diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 9c7faf0..b642a02 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -876,7 +876,8 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) def->mem.cur_balloon = memorySettingData->data->VirtualQuantity * 1024; /* megabyte to kilobyte */ if (virDomainDefSetVcpusMax(def, - processorSettingData->data->VirtualQuantity) < 0) + processorSettingData->data->VirtualQuantity, + NULL) < 0) goto cleanup; if (virDomainDefSetVcpus(def, diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 0e762f3..f153f69 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -557,7 +557,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver) def = NULL; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); - if (virDomainDefSetVcpusMax(vm->def, d_info.vcpu_max_id + 1)) + if (virDomainDefSetVcpusMax(vm->def, d_info.vcpu_max_id + 1, driver->xmlopt)) goto cleanup; if (virDomainDefSetVcpus(vm->def, d_info.vcpu_online) < 0) @@ -2182,7 +2182,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, switch (flags) { case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_CONFIG: - if (virDomainDefSetVcpusMax(def, nvcpus) < 0) + if (virDomainDefSetVcpusMax(def, nvcpus, driver->xmlopt) < 0) goto cleanup; break; diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index acbc20b..a34204e 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -1020,7 +1020,7 @@ lxcParseConfigString(const char *config, /* Value not handled by the LXC driver, setting to * minimum required to make XML parsing pass */ - if (virDomainDefSetVcpusMax(vmdef, 1) < 0) + if (virDomainDefSetVcpusMax(vmdef, 1, xmlopt) < 0) goto error; if (virDomainDefSetVcpus(vmdef, 1) < 0) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 50f4902..99ce95c 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -572,7 +572,7 @@ int openvzLoadDomains(struct openvz_driver *driver) if (ret == 0 || vcpus == 0) vcpus = openvzGetNodeCPUs(); - if (virDomainDefSetVcpusMax(def, vcpus) < 0) + if (virDomainDefSetVcpusMax(def, vcpus, driver->xmlopt) < 0) goto cleanup; if (virDomainDefSetVcpus(def, vcpus) < 0) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 844193a..f9a89cf 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -70,7 +70,8 @@ static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid); static int openvzConnectGetMaxVcpus(virConnectPtr conn, const char *type); static int openvzDomainGetMaxVcpus(virDomainPtr dom); static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, - unsigned int nvcpus); + unsigned int nvcpus, + virDomainXMLOptionPtr xmlopt); static int openvzDomainSetMemoryInternal(virDomainObjPtr vm, unsigned long long memory); static int openvzGetVEStatus(virDomainObjPtr vm, int *status, int *reason); @@ -1032,7 +1033,8 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla goto cleanup; } if (virDomainDefGetVcpusMax(vm->def)) { - if (openvzDomainSetVcpusInternal(vm, virDomainDefGetVcpusMax(vm->def)) < 0) { + if (openvzDomainSetVcpusInternal(vm, virDomainDefGetVcpusMax(vm->def), + driver->xmlopt) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set number of vCPUs")); goto cleanup; @@ -1130,7 +1132,8 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); if (virDomainDefGetVcpusMax(vm->def) > 0) { - if (openvzDomainSetVcpusInternal(vm, virDomainDefGetVcpusMax(vm->def)) < 0) { + if (openvzDomainSetVcpusInternal(vm, virDomainDefGetVcpusMax(vm->def), + driver->xmlopt) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set number of vCPUs")); goto cleanup; @@ -1347,7 +1350,8 @@ static int openvzDomainGetMaxVcpus(virDomainPtr dom) } static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, - unsigned int nvcpus) + unsigned int nvcpus, + virDomainXMLOptionPtr xmlopt) { char str_vcpus[32]; const char *prog[] = { VZCTL, "--quiet", "set", PROGRAM_SENTINEL, @@ -1364,7 +1368,7 @@ static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, if (virRun(prog, NULL) < 0) return -1; - if (virDomainDefSetVcpusMax(vm->def, nvcpus) < 0) + if (virDomainDefSetVcpusMax(vm->def, nvcpus, xmlopt) < 0) return -1; if (virDomainDefSetVcpus(vm->def, nvcpus) < 0) @@ -1402,7 +1406,7 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto cleanup; } - if (openvzDomainSetVcpusInternal(vm, nvcpus) < 0) { + if (openvzDomainSetVcpusInternal(vm, nvcpus, driver->xmlopt) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set number of vCPUs")); goto cleanup; diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index dce20bc..3dd8927 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3296,7 +3296,7 @@ phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) goto err; } - if (virDomainDefSetVcpusMax(&def, vcpus) < 0) + if (virDomainDefSetVcpusMax(&def, vcpus, phyp_driver->xmlopt) < 0) goto err; if (virDomainDefSetVcpus(&def, vcpus) < 0) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b993995..28ef5aa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4849,7 +4849,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if (virDomainDefSetVcpusMax(persistentDef, nvcpus) < 0) + if (virDomainDefSetVcpusMax(persistentDef, nvcpus, driver->xmlopt) < 0) goto endjob; } else { if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 09bd09a..3f7e445 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1659,7 +1659,8 @@ qemuParseCommandLineMem(virDomainDefPtr dom, static int qemuParseCommandLineSmp(virDomainDefPtr dom, - const char *val) + const char *val, + virDomainXMLOptionPtr xmlopt) { unsigned int sockets = 0; unsigned int cores = 0; @@ -1701,7 +1702,7 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, if (maxcpus == 0) maxcpus = vcpus; - if (virDomainDefSetVcpusMax(dom, maxcpus) < 0) + if (virDomainDefSetVcpusMax(dom, maxcpus, xmlopt) < 0) goto error; if (virDomainDefSetVcpus(dom, vcpus) < 0) @@ -1819,7 +1820,7 @@ qemuParseCommandLine(virCapsPtr caps, def->id = -1; def->mem.cur_balloon = 64 * 1024; virDomainDefSetMemoryTotal(def, def->mem.cur_balloon); - if (virDomainDefSetVcpusMax(def, 1) < 0) + if (virDomainDefSetVcpusMax(def, 1, xmlopt) < 0) goto error; if (virDomainDefSetVcpus(def, 1) < 0) goto error; @@ -1899,7 +1900,7 @@ qemuParseCommandLine(virCapsPtr caps, goto error; } else if (STREQ(arg, "-smp")) { WANT_VALUE(); - if (qemuParseCommandLineSmp(def, val) < 0) + if (qemuParseCommandLineSmp(def, val, xmlopt) < 0) goto error; } else if (STREQ(arg, "-uuid")) { WANT_VALUE(); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6853626..61f0207 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2336,6 +2336,7 @@ static int testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, unsigned int flags) { + testDriverPtr driver = domain->conn->privateData; virDomainObjPtr privdom = NULL; virDomainDefPtr def; virDomainDefPtr persistentDef; @@ -2383,7 +2384,8 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, if (persistentDef) { if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - if (virDomainDefSetVcpusMax(persistentDef, nrCpus) < 0) + if (virDomainDefSetVcpusMax(persistentDef, nrCpus, + driver->xmlopt) < 0) goto cleanup; } else { if (virDomainDefSetVcpus(persistentDef, nrCpus) < 0) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 8e49268..a14ab67 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3885,7 +3885,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) virDomainDefSetMemoryTotal(def, memorySize * 1024); gVBoxAPI.UIMachine.GetCPUCount(machine, &CPUCount); - if (virDomainDefSetVcpusMax(def, CPUCount) < 0) + if (virDomainDefSetVcpusMax(def, CPUCount, data->xmlopt) < 0) goto cleanup; if (virDomainDefSetVcpus(def, CPUCount) < 0) @@ -6044,7 +6044,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, def->dom->os.type = VIR_DOMAIN_OSTYPE_HVM; def->dom->os.arch = virArchFromHost(); gVBoxAPI.UIMachine.GetCPUCount(machine, &CPUCount); - if (virDomainDefSetVcpusMax(def->dom, CPUCount) < 0) + if (virDomainDefSetVcpusMax(def->dom, CPUCount, data->xmlopt) < 0) goto cleanup; if (virDomainDefSetVcpus(def->dom, CPUCount) < 0) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index d443dd0..0f557a8 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1457,7 +1457,7 @@ virVMXParseConfig(virVMXContext *ctx, goto cleanup; } - if (virDomainDefSetVcpusMax(def, numvcpus) < 0) + if (virDomainDefSetVcpusMax(def, numvcpus, xmlopt) < 0) goto cleanup; if (virDomainDefSetVcpus(def, numvcpus) < 0) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 3c34652..8335078 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -700,7 +700,7 @@ xenXMDomainSetVcpusFlags(virConnectPtr conn, } if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - if (virDomainDefSetVcpusMax(entry->def, vcpus) < 0) + if (virDomainDefSetVcpusMax(entry->def, vcpus, priv->xmlopt) < 0) goto cleanup; } else { if (virDomainDefSetVcpus(entry->def, vcpus) < 0) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 676ed5b..9a861c1 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1505,7 +1505,7 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) vcpus = xenapiDomainGetMaxVcpus(dom); - if (virDomainDefSetVcpusMax(defPtr, vcpus) < 0) + if (virDomainDefSetVcpusMax(defPtr, vcpus, priv->xmlopt) < 0) goto error; if (virDomainDefSetVcpus(defPtr, vcpus) < 0) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index f62a5b1..8365a2c 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -483,7 +483,9 @@ xenParsePCI(virConfPtr conf, virDomainDefPtr def) static int -xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) +xenParseCPUFeatures(virConfPtr conf, + virDomainDefPtr def, + virDomainXMLOptionPtr xmlopt) { unsigned long count = 0; const char *str = NULL; @@ -492,7 +494,7 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) if (xenConfigGetULong(conf, "vcpus", &count, 1) < 0) return -1; - if (virDomainDefSetVcpusMax(def, count) < 0) + if (virDomainDefSetVcpusMax(def, count, xmlopt) < 0) return -1; if (virDomainDefSetVcpus(def, count) < 0) @@ -502,7 +504,7 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) if (xenConfigGetULong(conf, "maxvcpus", &count, 0) < 0) return -1; - if (virDomainDefSetVcpusMax(def, count) < 0) + if (virDomainDefSetVcpusMax(def, count, xmlopt) < 0) return -1; } @@ -1051,7 +1053,8 @@ int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps, - const char *nativeFormat) + const char *nativeFormat, + virDomainXMLOptionPtr xmlopt) { if (xenParseGeneralMeta(conf, def, caps) < 0) return -1; @@ -1062,7 +1065,7 @@ xenParseConfigCommon(virConfPtr conf, if (xenParseEventsActions(conf, def) < 0) return -1; - if (xenParseCPUFeatures(conf, def) < 0) + if (xenParseCPUFeatures(conf, def, xmlopt) < 0) return -1; if (xenParseTimeOffset(conf, def) < 0) diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index 1c74bee..9055692 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -59,7 +59,8 @@ int xenConfigCopyStringOpt(virConfPtr conf, int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps, - const char *nativeFormat); + const char *nativeFormat, + virDomainXMLOptionPtr xmlopt); int xenFormatConfigCommon(virConfPtr conf, virDomainDefPtr def, diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index ea6c177..40dc53c 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1233,7 +1233,7 @@ xenParseSxpr(const struct sexpr *root, } } - if (virDomainDefSetVcpusMax(def, sexpr_int(root, "domain/vcpus")) < 0) + if (virDomainDefSetVcpusMax(def, sexpr_int(root, "domain/vcpus"), xmlopt) < 0) goto error; vcpus = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index d524a82..dcd4849 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -594,7 +594,8 @@ xenParseXL(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1; - if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XL) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XL, + xmlopt) < 0) goto cleanup; if (xenParseXLOS(conf, def, caps) < 0) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index 1023ed2..124c94a 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -447,7 +447,8 @@ xenParseXM(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1; - if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XM) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XM, + xmlopt) < 0) goto cleanup; if (xenParseXMOS(conf, def) < 0) -- 2.9.0

On 07.07.2016 15:41, Peter Krempa wrote:
Allow to store driver specific data on a per-vcpu basis.
Move of the virDomainDef*Vcpus* functions was necessary as virDomainXMLOptionPtr was declared below this block and I didn't want to split the function headers. --- src/conf/domain_conf.c | 28 +++++++++++++++++++++------- src/conf/domain_conf.h | 22 ++++++++++++++-------- src/hyperv/hyperv_driver.c | 3 ++- src/libxl/libxl_driver.c | 4 ++-- src/lxc/lxc_native.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 16 ++++++++++------ src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_parse_command.c | 9 +++++---- src/test/test_driver.c | 4 +++- src/vbox/vbox_common.c | 4 ++-- src/vmx/vmx.c | 2 +- src/xen/xm_internal.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- src/xenconfig/xen_common.c | 13 ++++++++----- src/xenconfig/xen_common.h | 3 ++- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 3 ++- src/xenconfig/xen_xm.c | 3 ++- 20 files changed, 81 insertions(+), 47 deletions(-)
You forgot to update Virtuozzo (vz) driver.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c2d7259..e660f8e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1307,12 +1307,23 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def)
static virDomainVcpuDefPtr -virDomainVcpuDefNew(void) +virDomainVcpuDefNew(virDomainXMLOptionPtr xmlopt) { + virObjectPtr priv = NULL; virDomainVcpuDefPtr ret;
- ignore_value(VIR_ALLOC(ret)); + if (xmlopt && xmlopt->privateData.vcpuNew && + !(priv = xmlopt->privateData.vcpuNew())) + goto cleanup; + + if (VIR_ALLOC(ret) < 0) + goto cleanup; + + ret->privateData = priv; + priv = NULL;
+ cleanup: + virObjectUnref(priv); return ret;
Funny, my compiler fails to see that @ret might be used uninitialized here.. All that's needed is just vcpuNew() function to fail. Initialize the @ret properly please.
}
ACK if you squash this in: diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 9d0bc0d..7871230 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1309,7 +1309,8 @@ prlsdkConvertDomainState(VIRTUAL_MACHINE_STATE domainState, static int prlsdkConvertCpuInfo(PRL_HANDLE sdkdom, - virDomainDefPtr def) + virDomainDefPtr def, + virDomainXMLOptionPtr xmlopt) { char *buf; int hostcpus; @@ -1327,7 +1328,7 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom, if (cpuCount > hostcpus) cpuCount = hostcpus; - if (virDomainDefSetVcpusMax(def, cpuCount) < 0) + if (virDomainDefSetVcpusMax(def, cpuCount, xmlopt) < 0) goto cleanup; if (virDomainDefSetVcpus(def, cpuCount) < 0) @@ -1706,7 +1707,7 @@ prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom) convert to Kbytes */ def->mem.cur_balloon = ram << 10; - if (prlsdkConvertCpuInfo(sdkdom, def) < 0) + if (prlsdkConvertCpuInfo(sdkdom, def, driver->xmlopt) < 0) goto error; if (prlsdkConvertCpuMode(sdkdom, def) < 0) Michal

Members will be added in follow-up patches. --- src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 13 +++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 42b5511..4c3df04 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -811,6 +811,46 @@ qemuDomainHostdevPrivateDispose(void *obj) } +static virClassPtr qemuDomainVcpuPrivateClass; +static void qemuDomainVcpuPrivateDispose(void *obj); + +static int +qemuDomainVcpuPrivateOnceInit(void) +{ + qemuDomainVcpuPrivateClass = virClassNew(virClassForObject(), + "qemuDomainVcpuPrivate", + sizeof(qemuDomainVcpuPrivate), + qemuDomainVcpuPrivateDispose); + if (!qemuDomainVcpuPrivateClass) + return -1; + else + return 0; +} + +VIR_ONCE_GLOBAL_INIT(qemuDomainVcpuPrivate) + +static virObjectPtr +qemuDomainVcpuPrivateNew(void) +{ + qemuDomainVcpuPrivatePtr priv; + + if (qemuDomainVcpuPrivateInitialize() < 0) + return NULL; + + if (!(priv = virObjectNew(qemuDomainVcpuPrivateClass))) + return NULL; + + return (virObjectPtr) priv; +} + + +static void +qemuDomainVcpuPrivateDispose(void *obj ATTRIBUTE_UNUSED) +{ + return; +} + + /* qemuDomainSecretPlainSetup: * @conn: Pointer to connection * @secinfo: Pointer to secret info @@ -1636,6 +1676,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .alloc = qemuDomainObjPrivateAlloc, .free = qemuDomainObjPrivateFree, .diskNew = qemuDomainDiskPrivateNew, + .vcpuNew = qemuDomainVcpuPrivateNew, .hostdevNew = qemuDomainHostdevPrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c49f31c..16d0996 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -312,6 +312,19 @@ struct _qemuDomainDiskPrivate { # define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev) \ ((qemuDomainHostdevPrivatePtr) (hostdev)->privateData) + +typedef struct _qemuDomainVcpuPrivate qemuDomainVcpuPrivate; +typedef qemuDomainVcpuPrivate *qemuDomainVcpuPrivatePtr; +struct _qemuDomainVcpuPrivate { + virObject parent; + + int dummy; +}; + +# define QEMU_DOMAIN_VCPU_PRIVATE(vcpu) \ + ((qemuDomainVcpuPrivatePtr) (vcpu)->privateData) + + struct qemuDomainDiskInfo { bool removable; bool locked; -- 2.9.0

Further patches will be adding index and modifying the source variables so this will make it more clear. --- src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4c3df04..d5f3042 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1322,6 +1322,27 @@ qemuDomainObjPrivateFree(void *data) } +static void +qemuDomainObjPrivateXMLFormatVcpus(virBufferPtr buf, + int *vcpupids, + int nvcpupids) +{ + size_t i; + + if (!nvcpupids) + return; + + virBufferAddLit(buf, "<vcpus>\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < nvcpupids; i++) + virBufferAsprintf(buf, "<vcpu pid='%d'/>\n", vcpupids[i]); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</vcpus>\n"); +} + + static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainObjPtr vm) @@ -1349,16 +1370,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainChrTypeToString(priv->monConfig->type)); } - - if (priv->nvcpupids) { - size_t i; - virBufferAddLit(buf, "<vcpus>\n"); - virBufferAdjustIndent(buf, 2); - for (i = 0; i < priv->nvcpupids; i++) - virBufferAsprintf(buf, "<vcpu pid='%d'/>\n", priv->vcpupids[i]); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</vcpus>\n"); - } + qemuDomainObjPrivateXMLFormatVcpus(buf, priv->vcpupids, priv->nvcpupids); if (priv->qemuCaps) { size_t i; @@ -1447,6 +1459,29 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, return 0; } + +static int +qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node, + unsigned int idx, + qemuDomainObjPrivatePtr priv) +{ + char *pidstr; + int ret = -1; + + if (!(pidstr = virXMLPropString(node, "pid"))) + goto cleanup; + + if (virStrToLong_i(pidstr, NULL, 10, &(priv->vcpupids[idx])) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(pidstr); + return ret; +} + + static int qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, virDomainObjPtr vm, @@ -1497,27 +1532,18 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, goto error; } - n = virXPathNodeSet("./vcpus/vcpu", ctxt, &nodes); - if (n < 0) + if ((n = virXPathNodeSet("./vcpus/vcpu", ctxt, &nodes)) < 0) goto error; - if (n) { - priv->nvcpupids = n; - if (VIR_REALLOC_N(priv->vcpupids, priv->nvcpupids) < 0) - goto error; - for (i = 0; i < n; i++) { - char *pidstr = virXMLPropString(nodes[i], "pid"); - if (!pidstr) - goto error; + priv->nvcpupids = n; + if (VIR_REALLOC_N(priv->vcpupids, priv->nvcpupids) < 0) + goto error; - if (virStrToLong_i(pidstr, NULL, 10, &(priv->vcpupids[i])) < 0) { - VIR_FREE(pidstr); - goto error; - } - VIR_FREE(pidstr); - } - VIR_FREE(nodes); + for (i = 0; i < n; i++) { + if (qemuDomainObjPrivateXMLParseVcpu(nodes[i], i, priv) < 0) + goto error; } + VIR_FREE(nodes); if ((n = virXPathNodeSet("./qemuCaps/flag", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.9.0

Note the vcpu ID so that once we allow non-contiguous vCPU topologies it will be possible to pair thread id's with the vcpus. --- src/qemu/qemu_domain.c | 13 ++++++++++++- tests/qemuxml2xmltest.c | 3 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d5f3042..07ef68f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1336,7 +1336,7 @@ qemuDomainObjPrivateXMLFormatVcpus(virBufferPtr buf, virBufferAdjustIndent(buf, 2); for (i = 0; i < nvcpupids; i++) - virBufferAsprintf(buf, "<vcpu pid='%d'/>\n", vcpupids[i]); + virBufferAsprintf(buf, "<vcpu id='%zu' pid='%d'/>\n", i, vcpupids[i]); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</vcpus>\n"); @@ -1465,9 +1465,19 @@ qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node, unsigned int idx, qemuDomainObjPrivatePtr priv) { + char *idstr; char *pidstr; int ret = -1; + if ((idstr = virXMLPropString(node, "id"))) { + if (virStrToLong_uip(idstr, NULL, 10, &idx) < 0 || + idx >= priv->nvcpupids) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid vcpu index '%s'"), idstr); + goto cleanup; + } + } + if (!(pidstr = virXMLPropString(node, "pid"))) goto cleanup; @@ -1477,6 +1487,7 @@ qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node, ret = 0; cleanup: + VIR_FREE(idstr); VIR_FREE(pidstr); return ret; } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a672fbb..618bfac 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -113,7 +113,8 @@ testGetStatuXMLPrefixVcpus(virBufferPtr buf, virBufferAdjustIndent(buf, 2); while ((vcpuid = virBitmapNextSetBit(data->activeVcpus, vcpuid)) >= 0) - virBufferAsprintf(buf, "<vcpu pid='%zd'/>\n", vcpuid + 3803519); + virBufferAsprintf(buf, "<vcpu id='%zd' pid='%zd'/>\n", + vcpuid, vcpuid + 3803519); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</vcpus>\n"); -- 2.9.0

Rather than storing them in an external array store them directly. --- src/qemu/qemu_domain.c | 116 ++++++++++++++++++++++++++++-------------------- src/qemu/qemu_domain.h | 7 +-- src/qemu/qemu_process.c | 2 - 3 files changed, 71 insertions(+), 54 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 07ef68f..e731b57 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1295,7 +1295,6 @@ qemuDomainObjPrivateFree(void *data) virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs); virDomainChrSourceDefFree(priv->monConfig); qemuDomainObjFreeJob(priv); - VIR_FREE(priv->vcpupids); VIR_FREE(priv->lockState); VIR_FREE(priv->origname); @@ -1324,19 +1323,25 @@ qemuDomainObjPrivateFree(void *data) static void qemuDomainObjPrivateXMLFormatVcpus(virBufferPtr buf, - int *vcpupids, - int nvcpupids) + virDomainDefPtr def) { size_t i; - - if (!nvcpupids) - return; + size_t maxvcpus = virDomainDefGetVcpusMax(def); + virDomainVcpuDefPtr vcpu; + pid_t tid; virBufferAddLit(buf, "<vcpus>\n"); virBufferAdjustIndent(buf, 2); - for (i = 0; i < nvcpupids; i++) - virBufferAsprintf(buf, "<vcpu id='%zu' pid='%d'/>\n", i, vcpupids[i]); + for (i = 0; i < maxvcpus; i++) { + vcpu = virDomainDefGetVcpu(def, i); + tid = QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid; + + if (!vcpu->online || tid == 0) + continue; + + virBufferAsprintf(buf, "<vcpu id='%zu' pid='%d'/>\n", i, tid); + } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</vcpus>\n"); @@ -1370,7 +1375,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainChrTypeToString(priv->monConfig->type)); } - qemuDomainObjPrivateXMLFormatVcpus(buf, priv->vcpupids, priv->nvcpupids); + qemuDomainObjPrivateXMLFormatVcpus(buf, vm->def); if (priv->qemuCaps) { size_t i; @@ -1463,27 +1468,31 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, static int qemuDomainObjPrivateXMLParseVcpu(xmlNodePtr node, unsigned int idx, - qemuDomainObjPrivatePtr priv) + virDomainDefPtr def) { + virDomainVcpuDefPtr vcpu; char *idstr; char *pidstr; + unsigned int tmp; int ret = -1; - if ((idstr = virXMLPropString(node, "id"))) { - if (virStrToLong_uip(idstr, NULL, 10, &idx) < 0 || - idx >= priv->nvcpupids) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid vcpu index '%s'"), idstr); - goto cleanup; - } + idstr = virXMLPropString(node, "id"); + + if ((idstr && virStrToLong_uip(idstr, NULL, 10, &idx)) < 0 || + !(vcpu = virDomainDefGetVcpu(def, idx))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid vcpu index '%s'"), idstr); + goto cleanup; } if (!(pidstr = virXMLPropString(node, "pid"))) goto cleanup; - if (virStrToLong_i(pidstr, NULL, 10, &(priv->vcpupids[idx])) < 0) + if (virStrToLong_uip(pidstr, NULL, 10, &tmp) < 0) goto cleanup; + QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid = tmp; + ret = 0; cleanup: @@ -1546,12 +1555,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, if ((n = virXPathNodeSet("./vcpus/vcpu", ctxt, &nodes)) < 0) goto error; - priv->nvcpupids = n; - if (VIR_REALLOC_N(priv->vcpupids, priv->nvcpupids) < 0) - goto error; - for (i = 0; i < n; i++) { - if (qemuDomainObjPrivateXMLParseVcpu(nodes[i], i, priv) < 0) + if (qemuDomainObjPrivateXMLParseVcpu(nodes[i], i, vm->def) < 0) goto error; } VIR_FREE(nodes); @@ -5505,9 +5510,18 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm) bool qemuDomainHasVcpuPids(virDomainObjPtr vm) { - qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); + virDomainVcpuDefPtr vcpu; + + for (i = 0; i < maxvcpus; i++) { + vcpu = virDomainDefGetVcpu(vm->def, i); - return priv->nvcpupids > 0; + if (QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid > 0) + return true; + } + + return false; } @@ -5520,14 +5534,10 @@ qemuDomainHasVcpuPids(virDomainObjPtr vm) */ pid_t qemuDomainGetVcpuPid(virDomainObjPtr vm, - unsigned int vcpu) + unsigned int vcpuid) { - qemuDomainObjPrivatePtr priv = vm->privateData; - - if (vcpu >= priv->nvcpupids) - return 0; - - return priv->vcpupids[vcpu]; + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); + return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid; } @@ -5547,9 +5557,12 @@ qemuDomainDetectVcpuPids(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob) { + virDomainVcpuDefPtr vcpu; + size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); pid_t *cpupids = NULL; - int ncpupids = 0; - qemuDomainObjPrivatePtr priv = vm->privateData; + int ncpupids; + size_t i; + int ret = -1; /* * Current QEMU *can* report info about host threads mapped @@ -5580,22 +5593,32 @@ qemuDomainDetectVcpuPids(virQEMUDriverPtr driver, * to try to do this hard work. */ if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU) - goto done; + return 0; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); + ncpupids = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &cpupids); if (qemuDomainObjExitMonitor(driver, vm) < 0) { - VIR_FREE(cpupids); - return -2; + ret = -2; + goto cleanup; } + /* failure to get the VCPU <-> PID mapping or to execute the query * command will not be treated fatal as some versions of qemu don't * support this command */ if (ncpupids <= 0) { virResetLastError(); - ncpupids = 0; - goto done; + ret = 0; + goto cleanup; + } + + for (i = 0; i < maxvcpus; i++) { + vcpu = virDomainDefGetVcpu(vm->def, i); + + if (i < ncpupids) + QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid = cpupids[i]; + else + QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid = 0; } if (ncpupids != virDomainDefGetVcpus(vm->def)) { @@ -5603,15 +5626,14 @@ qemuDomainDetectVcpuPids(virQEMUDriverPtr driver, _("got wrong number of vCPU pids from QEMU monitor. " "got %d, wanted %d"), ncpupids, virDomainDefGetVcpus(vm->def)); - VIR_FREE(cpupids); - return -1; + goto cleanup; } - done: - VIR_FREE(priv->vcpupids); - priv->nvcpupids = ncpupids; - priv->vcpupids = cpupids; - return ncpupids; + ret = ncpupids; + + cleanup: + VIR_FREE(cpupids); + return ret; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 16d0996..114db98 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -184,9 +184,6 @@ struct _qemuDomainObjPrivate { bool beingDestroyed; char *pidfile; - int nvcpupids; - int *vcpupids; - virDomainPCIAddressSetPtr pciaddrs; virDomainCCWAddressSetPtr ccwaddrs; virDomainVirtioSerialAddrSetPtr vioserialaddrs; @@ -318,7 +315,7 @@ typedef qemuDomainVcpuPrivate *qemuDomainVcpuPrivatePtr; struct _qemuDomainVcpuPrivate { virObject parent; - int dummy; + pid_t tid; /* vcpu thread id */ }; # define QEMU_DOMAIN_VCPU_PRIVATE(vcpu) \ @@ -649,7 +646,7 @@ int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, const virDomainMemoryDef *mem); bool qemuDomainHasVcpuPids(virDomainObjPtr vm); -pid_t qemuDomainGetVcpuPid(virDomainObjPtr vm, unsigned int vcpu); +pid_t qemuDomainGetVcpuPid(virDomainObjPtr vm, unsigned int vcpuid); int qemuDomainDetectVcpuPids(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 96eaeb2..c6129be 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5899,8 +5899,6 @@ void qemuProcessStop(virQEMUDriverPtr driver, vm->taint = 0; vm->pid = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); - VIR_FREE(priv->vcpupids); - priv->nvcpupids = 0; for (i = 0; i < vm->def->niothreadids; i++) vm->def->iothreadids[i]->thread_id = 0; virObjectUnref(priv->qemuCaps); -- 2.9.0

On 07.07.2016 15:40, Peter Krempa wrote:
Cleanups and improvements that will make adding the new vCPU hotplug that was recently added to qemu easier.
Peter Krempa (11): conf: Annotate that private data for objects are not copied conf: Extract code formatting vCPU info conf: Rename virDomainVcpuInfoPtr to virDomainVcpuDefPtr conf: Don't report errors from virDomainDefGetVcpu tests: qemuxml2xml: Format status XML header dynamically conf: convert def->vcpus to a array of pointers conf: Add private data for virDomainVcpuDef qemu: domain: Add vcpu private data structure qemu: domain: Extract formating and parsing of vCPU thread ids qemu: Add cpu ID to the vCPU pid list in the status XML qemu: Store vCPU thread ids in vcpu private data objects
src/conf/domain_conf.c | 127 +++++++++++++++++--------- src/conf/domain_conf.h | 32 ++++--- src/hyperv/hyperv_driver.c | 3 +- src/libxl/libxl_domain.c | 2 +- src/libxl/libxl_driver.c | 6 +- src/lxc/lxc_native.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 16 ++-- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_domain.c | 206 +++++++++++++++++++++++++++++++----------- src/qemu/qemu_domain.h | 18 +++- src/qemu/qemu_driver.c | 22 ++--- src/qemu/qemu_parse_command.c | 9 +- src/qemu/qemu_process.c | 6 +- src/test/test_driver.c | 8 +- src/vbox/vbox_common.c | 4 +- src/vmx/vmx.c | 2 +- src/vz/vz_sdk.c | 2 +- src/xen/xm_internal.c | 2 +- src/xenapi/xenapi_driver.c | 2 +- src/xenconfig/xen_common.c | 13 ++- src/xenconfig/xen_common.h | 3 +- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 3 +- src/xenconfig/xen_xm.c | 3 +- tests/qemuxml2xmltest.c | 73 ++++++++++++--- 27 files changed, 396 insertions(+), 176 deletions(-)
ACK series, but see my comments to individual patches before pushing. Michal
participants (2)
-
Michal Privoznik
-
Peter Krempa