[libvirt] [RFC PATCH v2 00/12] qemu: add support to hot-plug/unplug cpu device

If you apply the folowing patchset in order [PATCH v3 0/7] cpu: add device_add foo-x86_64-cpu support https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg01552.html, [PATCH v2 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg03929.html and [PATCH v2 00/11] cpu: add i386 cpu hot remove support https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg01557.html, qemu can support hotplug and hot-unplug cpu device. So this patch series will make libvirt support hotplug and hot-unplug cpu device for qemu driver. This patch series add a new API to support cpu hot-plug/unplug, and leave the existing API by invoking qemu command 'cpu-add' as a legacy. This patch series realize cpu hot-plug/unplug by libvirt command 'attach-device' and 'detach-device', and invoke qemu command 'device_add' and 'device_del' to support this feature. v2: - update cpu device's definition, and cpu's apic_id is hidded to users. - add check for compatibility between host cpu and hot added cpu - add a capability for *-x86_64-cpu Zhu Guihua (12): domain_conf: add support for cpu device configuration in XML domain_conf: introduce cpu def helpers domain_conf: introduce cpu device hotplug helpers qemu_driver: implement cpu device hotplug on config level qemu_command: introduce a func for cpu device alias assignment domain_conf: allocate cpu's apic id dynamically qemu: add a capability for x86_64-cpu qemu: introduce qemuBuildCPUDeviceStr qemu: implement cpu device hotplug on live level qemu: implement cpu device hotunplug on live level qemu_monitor_json: sort JSON array of cpu info qemu_driver: detect threads corresponding to Vcpus docs/formatdomain.html.in | 28 ++++ docs/schemas/domaincommon.rng | 3 + src/conf/domain_conf.c | 189 +++++++++++++++++++++++++- src/conf/domain_conf.h | 33 +++++ src/libvirt_private.syms | 6 + src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 120 +++++++++++++++++ src/qemu/qemu_command.h | 10 ++ src/qemu/qemu_driver.c | 299 ++++++++++++++++++++++++------------------ src/qemu/qemu_driver.h | 8 ++ src/qemu/qemu_hotplug.c | 140 ++++++++++++++++++++ src/qemu/qemu_hotplug.h | 12 ++ src/qemu/qemu_monitor_json.c | 31 ++++- src/util/virbitmap.c | 2 +- src/util/virbitmap.h | 2 + 16 files changed, 753 insertions(+), 134 deletions(-) -- 1.9.3

This patch adds configuration support for the cpu device. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/conf/domain_conf.c | 34 +++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 16 ++++++++++++++++ src/qemu/qemu_driver.c | 6 ++++++ src/qemu/qemu_hotplug.c | 1 + 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10dbabd..0f4baaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -236,7 +236,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "rng", "shmem", "tpm", - "panic") + "panic", + "cpu") VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", @@ -1981,6 +1982,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) case VIR_DOMAIN_DEVICE_PANIC: virDomainPanicDefFree(def->data.panic); break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_NONE: break; @@ -2687,6 +2689,8 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device) return &device->data.tpm->info; case VIR_DOMAIN_DEVICE_PANIC: return &device->data.panic->info; + case VIR_DOMAIN_DEVICE_CPU: + return &device->data.cpu->info; /* The following devices do not contain virDomainDeviceInfo */ case VIR_DOMAIN_DEVICE_LEASE: @@ -2917,6 +2921,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->panic->info, opaque) < 0) return -1; } + device.type = VIR_DOMAIN_DEVICE_CPU; + for (i = 0; i < def->ncpus; i++) { + device.data.cpu = def->cpus[i]; + if (cb(def, &device, &def->cpus[i]->info, opaque) < 0) + return -1; + } /* Coverity is not very happy with this - all dead_error_condition */ #if !STATIC_ANALYSIS @@ -2950,6 +2960,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_PANIC: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_CPU: break; } #endif @@ -11175,6 +11186,7 @@ virDomainDeviceDefParse(const char *xmlStr, if (!(dev->data.panic = virDomainPanicDefParseXML(node))) goto error; break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: break; @@ -15552,6 +15564,19 @@ virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src, return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); } +static bool +virDomainCPUDefCheckABIStability(virDomainCPUDefPtr src, + virDomainCPUDefPtr dst) +{ + if (src->virCPU->model != dst->virCPU->model) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target CPU device model doesn't match source")); + return false; + } + + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); +} + /* This compares two configurations and looks for any differences * which will affect the guest ABI. This is primarily to allow * validation of custom XML config passed in during migration @@ -15985,6 +16010,11 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } + for (i = 0; i < src->ncpus; i++) { + if (!virDomainCPUDefCheckABIStability(src->cpus[i], dst->cpus[i])) + goto error; + } + /* Coverity is not very happy with this - all dead_error_condition */ #if !STATIC_ANALYSIS /* This switch statement is here to trigger compiler warning when adding @@ -16016,6 +16046,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src, case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_CPU: break; } #endif @@ -21448,6 +21479,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, case VIR_DOMAIN_DEVICE_PANIC: rc = virDomainPanicDefFormat(&buf, src->data.panic); break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 563c18b..b9d4e7c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -144,6 +144,9 @@ typedef virDomainShmemDef *virDomainShmemDefPtr; typedef struct _virDomainTPMDef virDomainTPMDef; typedef virDomainTPMDef *virDomainTPMDefPtr; +typedef struct _virDomainCPUDef virDomainCPUDef; +typedef virDomainCPUDef *virDomainCPUDefPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -168,6 +171,7 @@ typedef enum { VIR_DOMAIN_DEVICE_SHMEM, VIR_DOMAIN_DEVICE_TPM, VIR_DOMAIN_DEVICE_PANIC, + VIR_DOMAIN_DEVICE_CPU, VIR_DOMAIN_DEVICE_LAST } virDomainDeviceType; @@ -192,6 +196,7 @@ struct _virDomainDeviceDef { virDomainRedirdevDefPtr redirdev; virDomainSmartcardDefPtr smartcard; virDomainChrDefPtr chr; + virDomainCPUDefPtr cpu; virDomainMemballoonDefPtr memballoon; virDomainNVRAMDefPtr nvram; virDomainRNGDefPtr rng; @@ -1158,6 +1163,13 @@ struct _virDomainChrDef { virSecurityDeviceLabelDefPtr *seclabels; }; +struct _virDomainCPUDef { + virCPUDefPtr virCPU; + int apic_id; + + virDomainDeviceInfo info; +}; + typedef enum { VIR_DOMAIN_SMARTCARD_TYPE_HOST, VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES, @@ -2156,6 +2168,9 @@ struct _virDomainDef { size_t nshmems; virDomainShmemDefPtr *shmems; + size_t ncpus; + virDomainCPUDefPtr *cpus; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -2841,6 +2856,7 @@ VIR_ENUM_DECL(virDomainRNGModel) VIR_ENUM_DECL(virDomainRNGBackend) VIR_ENUM_DECL(virDomainTPMModel) VIR_ENUM_DECL(virDomainTPMBackend) +VIR_ENUM_DECL(virDomainCPU) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc0a48c..31c7655 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7004,6 +7004,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, dev->data.chr = NULL; break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: @@ -7079,6 +7080,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_CHR: ret = qemuDomainDetachChrDevice(driver, vm, dev->data.chr); break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: @@ -7198,6 +7200,7 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn, case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_REDIRDEV: case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: @@ -7322,6 +7325,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, dev->data.fs = NULL; break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: @@ -7438,6 +7442,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainFSDefFree(fs); break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: @@ -7553,6 +7558,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_REDIRDEV: case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1dbcc5d..cf8362c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2885,6 +2885,7 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, ret = qemuDomainRemoveChrDevice(driver, vm, dev->data.chr); break; + case VIR_DOMAIN_DEVICE_CPU: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: -- 1.9.3

virDomainCPUDefFree - free memory allocated virDomainCPUDefParseXML - parse job type virDomainCPUDefFormat - output job type Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- docs/formatdomain.html.in | 28 +++++++++++++++ docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 82 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 5 files changed, 115 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8d5f89..10cbd29 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5830,6 +5830,34 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> + <h4><a name="elementsCpu">CPU device</a></h4> + <p> + CPU device allows to be hot added to the guest. + <span class="since">Since 1.2.12, QEMU and KVM only</span> + </p> + +<pre> + ... + <devices> + <cpu match='exact'> + <model fallback='allow'>core2duo</model> + <vendor>Intel</vendor> + <topology sockets='1' cores='2' threads='1'/> + <feature policy='disable' name='lahf_lm'/> + </cpu> + <devices> + ... +</pre> + <dl> + <dt><code>model</code></dt> + <dd> + <p> + The attribute can be omitted and will default to a model + started up by the guest. + </p> + </dd> + </dl> + <h3><a name="seclabel">Security label</a></h3> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9d6c1ee..1ceb811 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4003,6 +4003,9 @@ <optional> <ref name="panic"/> </optional> + <optional> + <ref name='cpu'> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0f4baaf..dfe0d65 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1913,6 +1913,18 @@ void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def) VIR_FREE(def); } +void virDomainCPUDefFree(virDomainCPUDefPtr def) +{ + if (!def) + return; + + virCPUDefFree(def->virCPU); + + virDomainDeviceInfoClear(&def->info); + + VIR_FREE(def); +} + void virDomainDeviceDefFree(virDomainDeviceDefPtr def) { if (!def) @@ -1983,6 +1995,8 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def) virDomainPanicDefFree(def->data.panic); break; case VIR_DOMAIN_DEVICE_CPU: + virDomainCPUDefFree(def->data.cpu); + break; case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_NONE: break; @@ -11052,6 +11066,44 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt, return ret; } +static virDomainCPUDefPtr +virDomainCPUDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + const virDomainDef *def, + unsigned int flags) +{ + virDomainCPUDefPtr dev; + + if (VIR_ALLOC(dev) < 0) + return NULL; + + dev->virCPU = virCPUDefParseXML(node, ctxt, VIR_CPU_TYPE_AUTO); + if (!dev->virCPU) + goto cleanup; + + if (!dev->virCPU->model) { + if (def->cpu->model) { + dev->virCPU->model = def->cpu->model; + } else if (def->os.arch == VIR_ARCH_I686) { + if (virAsprintf(&dev->virCPU->model, "qemu32") < 0) + goto cleanup; + } else { + if (virAsprintf(&dev->virCPU->model, "qemu64") < 0) + goto cleanup; + } + } + + if (virDomainDeviceInfoParseXML(node, NULL, &dev->info, flags) < 0) + goto cleanup; + + return dev; + + cleanup: + virDomainCPUDefFree(dev); + dev = NULL; + return dev; +} + virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, const virDomainDef *def, @@ -11187,6 +11239,9 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_CPU: + if (!(dev->data.cpu = virDomainCPUDefParseXML(node, ctxt, def, flags))) + goto error; + break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: break; @@ -18145,6 +18200,27 @@ virDomainChrDefFormat(virBufferPtr buf, } static int +virDomainCPUDefFormat(virBufferPtr buf, + virDomainCPUDefPtr def, + unsigned int flags) +{ + if (virCPUDefFormatBufFull(buf, def->virCPU, false) < 0) + return -1; + + virBufferTrim(buf, "</cpu>\n", -1); + virBufferTrim(buf, "", 4); + virBufferAdjustIndent(buf, 2); + + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + return -1; + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</cpu>\n"); + + return 0; +} + +static int virDomainSmartcardDefFormat(virBufferPtr buf, virDomainSmartcardDefPtr def, unsigned int flags) @@ -20065,6 +20141,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto error; } + for (n = 0; n < def->ncpus; n++) + if (virDomainCPUDefFormat(buf, def->cpus[n], flags) < 0) + goto error; + if (def->nvram) virDomainNVRAMDefFormat(buf, def->nvram, flags); @@ -21480,6 +21560,8 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src, rc = virDomainPanicDefFormat(&buf, src->data.panic); break; case VIR_DOMAIN_DEVICE_CPU: + rc = virDomainCPUDefFormat(&buf, src->data.cpu, flags); + break; case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9d4e7c..4096ecc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2371,6 +2371,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def); +void virDomainCPUDefFree(virDomainCPUDefPtr def); void virDomainShmemDefFree(virDomainShmemDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75a6d83..897a598 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -181,6 +181,7 @@ virDomainControllerModelUSBTypeFromString; virDomainControllerModelUSBTypeToString; virDomainControllerRemove; virDomainControllerTypeToString; +virDomainCPUDefFree; virDomainCpuPlacementModeTypeFromString; virDomainCpuPlacementModeTypeToString; virDomainDefAddImplicitControllers; -- 1.9.3

On Wed, Feb 04, 2015 at 05:18:20PM +0800, Zhu Guihua wrote:
virDomainCPUDefFree - free memory allocated virDomainCPUDefParseXML - parse job type virDomainCPUDefFormat - output job type
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- docs/formatdomain.html.in | 28 +++++++++++++++ docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 82 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 5 files changed, 115 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8d5f89..10cbd29 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5830,6 +5830,34 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+ <h4><a name="elementsCpu">CPU device</a></h4> + <p> + CPU device allows to be hot added to the guest. + <span class="since">Since 1.2.12, QEMU and KVM only</span> + </p> + +<pre> + ... + <devices> + <cpu match='exact'> + <model fallback='allow'>core2duo</model> + <vendor>Intel</vendor> + <topology sockets='1' cores='2' threads='1'/> + <feature policy='disable' name='lahf_lm'/> + </cpu> + <devices>
NACK, it is just madness to duplicate the CPU info we already have in a different place in the XML. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, 2015-02-04 at 09:25 +0000, Daniel P. Berrange wrote:
On Wed, Feb 04, 2015 at 05:18:20PM +0800, Zhu Guihua wrote:
virDomainCPUDefFree - free memory allocated virDomainCPUDefParseXML - parse job type virDomainCPUDefFormat - output job type
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- docs/formatdomain.html.in | 28 +++++++++++++++ docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 82 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 5 files changed, 115 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8d5f89..10cbd29 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5830,6 +5830,34 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+ <h4><a name="elementsCpu">CPU device</a></h4> + <p> + CPU device allows to be hot added to the guest. + <span class="since">Since 1.2.12, QEMU and KVM only</span> + </p> + +<pre> + ... + <devices> + <cpu match='exact'> + <model fallback='allow'>core2duo</model> + <vendor>Intel</vendor> + <topology sockets='1' cores='2' threads='1'/> + <feature policy='disable' name='lahf_lm'/> + </cpu> + <devices>
NACK, it is just madness to duplicate the CPU info we already have in a different place in the XML.
Yes, it's duplicate. But I think it represents different meaning, it is under xml tag 'device'. Regards, Zhu
Regards, Daniel

On Wed, Feb 04, 2015 at 05:42:42PM +0800, Zhu Guihua wrote:
On Wed, 2015-02-04 at 09:25 +0000, Daniel P. Berrange wrote:
On Wed, Feb 04, 2015 at 05:18:20PM +0800, Zhu Guihua wrote:
virDomainCPUDefFree - free memory allocated virDomainCPUDefParseXML - parse job type virDomainCPUDefFormat - output job type
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- docs/formatdomain.html.in | 28 +++++++++++++++ docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 82 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 5 files changed, 115 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8d5f89..10cbd29 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5830,6 +5830,34 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+ <h4><a name="elementsCpu">CPU device</a></h4> + <p> + CPU device allows to be hot added to the guest. + <span class="since">Since 1.2.12, QEMU and KVM only</span> + </p> + +<pre> + ... + <devices> + <cpu match='exact'> + <model fallback='allow'>core2duo</model> + <vendor>Intel</vendor> + <topology sockets='1' cores='2' threads='1'/> + <feature policy='disable' name='lahf_lm'/> + </cpu> + <devices>
NACK, it is just madness to duplicate the CPU info we already have in a different place in the XML.
Yes, it's duplicate. But I think it represents different meaning, it is under xml tag 'device'.
Regardless my NACK stands. This kind of duplication is just madness that causes problems for applications & developers alike. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wed, 2015-02-04 at 09:46 +0000, Daniel P. Berrange wrote:
On Wed, Feb 04, 2015 at 05:42:42PM +0800, Zhu Guihua wrote:
On Wed, 2015-02-04 at 09:25 +0000, Daniel P. Berrange wrote:
On Wed, Feb 04, 2015 at 05:18:20PM +0800, Zhu Guihua wrote:
virDomainCPUDefFree - free memory allocated virDomainCPUDefParseXML - parse job type virDomainCPUDefFormat - output job type
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- docs/formatdomain.html.in | 28 +++++++++++++++ docs/schemas/domaincommon.rng | 3 ++ src/conf/domain_conf.c | 82 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 5 files changed, 115 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8d5f89..10cbd29 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5830,6 +5830,34 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl>
+ <h4><a name="elementsCpu">CPU device</a></h4> + <p> + CPU device allows to be hot added to the guest. + <span class="since">Since 1.2.12, QEMU and KVM only</span> + </p> + +<pre> + ... + <devices> + <cpu match='exact'> + <model fallback='allow'>core2duo</model> + <vendor>Intel</vendor> + <topology sockets='1' cores='2' threads='1'/> + <feature policy='disable' name='lahf_lm'/> + </cpu> + <devices>
NACK, it is just madness to duplicate the CPU info we already have in a different place in the XML.
Yes, it's duplicate. But I think it represents different meaning, it is under xml tag 'device'.
Regardless my NACK stands. This kind of duplication is just madness that causes problems for applications & developers alike.
Agree. It is easy to cause misunderstanding. I will find a better way to handle this. Thanks for your help. Regards, Zhu
Regards, Daniel

virDomainCPUFind - to find a cpu within VM def virDomainCPUInsert - wrapper for inserting a new cpu device into VM def virDomainCPURemove - wrapper for removing cpu from VM def Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/conf/domain_conf.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 13 +++++++++++ src/libvirt_private.syms | 3 +++ 3 files changed, 74 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dfe0d65..f83eada 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12128,6 +12128,64 @@ virDomainChrRemove(virDomainDefPtr vmdef, return ret; } +int +virDomainCPUInsert(virDomainDefPtr vmdef, + virDomainCPUDefPtr cpu) +{ + return VIR_APPEND_ELEMENT(vmdef->cpus, vmdef->ncpus, cpu); +} + +bool +virDomainCPUEquals(virDomainCPUDefPtr src, + virDomainCPUDefPtr tgt) +{ + bool ret = false; + + if (!src || !tgt) + return src == tgt; + + ret = virCPUDefIsEqual(src->virCPU, tgt->virCPU); + + return ret; +} + +virDomainCPUDefPtr +virDomainCPUFind(virDomainDefPtr def, + virDomainCPUDefPtr target) +{ + virDomainCPUDefPtr cpu; + size_t i; + + for (i = 0; i < def->ncpus; i++) { + cpu = def->cpus[i]; + if (virDomainCPUEquals(cpu, target)) + return cpu; + } + + return NULL; +} + +virDomainCPUDefPtr +virDomainCPURemove(virDomainDefPtr vmdef, + virDomainCPUDefPtr cpu) +{ + virDomainCPUDefPtr ret; + size_t i; + + for (i = 0; i < vmdef->ncpus; i++) { + ret = vmdef->cpus[i]; + + if (virDomainCPUEquals(ret, cpu)) + break; + } + + if (i == vmdef->ncpus) + return NULL; + + VIR_DELETE_ELEMENT(vmdef->cpus, i, vmdef->ncpus); + return ret; +} + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4096ecc..144f79a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2674,6 +2674,19 @@ virDomainChrDefPtr virDomainChrRemove(virDomainDefPtr vmdef, virDomainChrDefPtr chr); +int +virDomainCPUInsert(virDomainDefPtr vmdef, + virDomainCPUDefPtr cpu); +bool +virDomainCPUEquals(virDomainCPUDefPtr src, + virDomainCPUDefPtr tgt); +virDomainCPUDefPtr +virDomainCPUFind(virDomainDefPtr def, + virDomainCPUDefPtr cpu); +virDomainCPUDefPtr +virDomainCPURemove(virDomainDefPtr vmdef, + virDomainCPUDefPtr cpu); + int virDomainSaveXML(const char *configDir, virDomainDefPtr def, const char *xml); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 897a598..df4f508 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -182,8 +182,11 @@ virDomainControllerModelUSBTypeToString; virDomainControllerRemove; virDomainControllerTypeToString; virDomainCPUDefFree; +virDomainCPUFind; +virDomainCPUInsert; virDomainCpuPlacementModeTypeFromString; virDomainCpuPlacementModeTypeToString; +virDomainCPURemove; virDomainDefAddImplicitControllers; virDomainDefCheckABIStability; virDomainDefClearCCWAddresses; -- 1.9.3

The config level requires an insert or remove from domain definition structure. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 31c7655..4ec3b3c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7326,6 +7326,11 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, break; case VIR_DOMAIN_DEVICE_CPU: + if (virDomainCPUInsert(vmdef, dev->data.cpu) < 0) + return -1; + dev->data.cpu = NULL; + break; + case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: @@ -7362,6 +7367,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainControllerDefPtr cont, det_cont; virDomainChrDefPtr chr; virDomainFSDefPtr fs; + virDomainCPUDefPtr cpu; int idx; switch ((virDomainDeviceType) dev->type) { @@ -7443,6 +7449,14 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, break; case VIR_DOMAIN_DEVICE_CPU: + if (!(cpu = virDomainCPURemove(vmdef, dev->data.cpu))) + return -1; + + virDomainCPUDefFree(cpu); + virDomainCPUDefFree(dev->data.cpu); + dev->data.cpu = NULL; + break; + case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: -- 1.9.3

This function used to set a alias name for cpu device. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/qemu/qemu_command.c | 32 ++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 3 +++ 2 files changed, 35 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 100deed..6201e29 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -948,6 +948,34 @@ qemuAssignDeviceChrAlias(virDomainDefPtr def, } int +qemuAssignDeviceCPUAlias(virDomainDefPtr def, + virDomainCPUDefPtr cpu, + int idx) +{ + if (idx == -1) { + size_t i; + idx = 0; + + for (i = 0; i < def->ncpus; i++) { + int thisidx; + if ((thisidx = qemuDomainDeviceAliasIndex(&def->cpus[i]->info, "cpu")) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to determine device index from CPU device")); + return -1; + } + + if (thisidx >= idx) + idx = thisidx + 1; + } + } + + if (virAsprintf(&cpu->info.alias, "cpu%d", idx) < 0) + return -1; + + return 0; +} + +int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { size_t i; @@ -1017,6 +1045,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0) return -1; } + for (i = 0; i< def->ncpus; i++) { + if (qemuAssignDeviceCPUAlias(def, def->cpus[i], i) < 0) + return -1; + } for (i = 0; i < def->nhubs; i++) { if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) < 0) return -1; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 8eaf1e4..c63fd30 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -265,6 +265,9 @@ int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr r int qemuAssignDeviceChrAlias(virDomainDefPtr def, virDomainChrDefPtr chr, ssize_t idx); +int qemuAssignDeviceCPUAlias(virDomainDefPtr def, + virDomainCPUDefPtr cpu, + int idx); int qemuParseKeywords(const char *str, -- 1.9.3

Add a bitmap apic_idmap to store status of APIC IDs. If you want to use an APIC ID, you can find a minimum value which has not been used in the bitmap. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 5 ++++- 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f83eada..45c8e87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13020,6 +13020,12 @@ virDomainDefParseXML(xmlDocPtr xml, } } + if (!(def->apic_id_map = virBitmapNew(def->maxvcpus))) + goto error; + + for (i = 0; i < def->vcpus; i++) + ignore_value(virBitmapSetBit(def->apic_id_map, i)); + tmp = virXPathString("string(./vcpu[1]/@placement)", ctxt); if (tmp) { if ((def->placement_mode = @@ -16432,6 +16438,15 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) } } +uint32_t +virDomainCPUGetFreeApicID(virDomainDefPtr def) +{ + int idx; + idx = virBitmapNextClearBit(def->apic_id_map, 0); + + return idx; +} + int virDomainEmulatorPinAdd(virDomainDefPtr def, unsigned char *cpumap, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 144f79a..ff8e0b9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2074,6 +2074,7 @@ struct _virDomainDef { unsigned short maxvcpus; int placement_mode; virBitmapPtr cpumask; + virBitmapPtr apic_id_map; unsigned int iothreads; @@ -2551,6 +2552,8 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, void virDomainVcpuPinDel(virDomainDefPtr def, int vcpu); +uint32_t virDomainCPUGetFreeApicID(virDomainDefPtr def); + int virDomainEmulatorPinAdd(virDomainDefPtr def, unsigned char *cpumap, int maplen); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index df4f508..b23c45c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -430,6 +430,7 @@ virDomainVcpuPinDefFree; virDomainVcpuPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; +virDomainCPUGetFreeApicID; virDomainVideoDefaultRAM; virDomainVideoDefaultType; virDomainVideoDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4ec3b3c..c6318bc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4371,6 +4371,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, int ncpupids; virCgroupPtr cgroup_vcpu = NULL; char *mem_mask = NULL; + uint32_t apic_id; qemuDomainObjEnterMonitor(driver, vm); @@ -4380,13 +4381,15 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (nvcpus > vcpus) { for (i = vcpus; i < nvcpus; i++) { /* Online new CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, true); + apic_id = virDomainCPUGetFreeApicID(vm->def); + rc = qemuMonitorSetCPU(priv->mon, apic_id, true); if (rc == 0) goto unsupported; if (rc < 0) goto exit_monitor; vcpus++; + ignore_value(virBitmapSetBit(vm->def->apic_id_map, apic_id)); } } else { for (i = vcpus - 1; i >= nvcpus; i--) { -- 1.9.3

This patch adds a new capability bit QEMU_CAPS_DEVICE_X86_64_CPU. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 13f3cd3..1f6facd 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -277,6 +277,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vmware-svga.vgamem_mb", "qxl.vgamem_mb", "qxl-vga.vgamem_mb", + "x86_64_cpu", ); @@ -1524,6 +1525,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "usb-audio", QEMU_CAPS_OBJECT_USB_AUDIO }, { "iothread", QEMU_CAPS_OBJECT_IOTHREAD}, { "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM }, + { "x86_64_cpu", QEMU_CAPS_DEVICE_X86_64_CPU }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { @@ -3157,6 +3159,7 @@ virQEMUCapsInitArchQMPBasic(virQEMUCapsPtr qemuCaps, virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_ACPI); virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_HPET); virQEMUCapsSet(qemuCaps, QEMU_CAPS_NO_KVM_PIT); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_X86_64_CPU); } ret = 0; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 12e1688..6872e09 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -223,6 +223,7 @@ typedef enum { QEMU_CAPS_VMWARE_SVGA_VGAMEM = 181, /* -device vmware-svga.vgamem_mb */ QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */ QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ + QEMU_CAPS_DEVICE_X86_64_CPU = 184, /* -device *x86_64-cpu */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; -- 1.9.3

qemuBuildCPUDeviceStr being introduced is responsible for creating command line argument for '-device' for given cpu device, and checking host cpu compat. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/qemu/qemu_command.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 7 ++++ 2 files changed, 95 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6201e29..513b726 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6003,6 +6003,88 @@ static char *qemuBuildTPMDevStr(const virDomainDef *def, } +int +qemuBuildCPUDeviceStr(char **deviceStr, + virQEMUDriverPtr driver, + const virDomainDef *def, + virDomainCPUDefPtr dev, + virQEMUCapsPtr qemuCaps) +{ + char *model = NULL; + virCPUDefPtr host = NULL; + char *compare_msg = NULL; + virCPUCompareResult cmp; + virCapsPtr caps = NULL; + bool compareAgainstHost = ((def->virtType == VIR_DOMAIN_VIRT_KVM || + def->cpu->mode != VIR_CPU_MODE_CUSTOM) && + def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH); + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X86_64_CPU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%s not supported in this QEMU binary"), dev->virCPU->model); + goto error; + } + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto error; + + host = caps->host.cpu; + + /* For KVM, CPU features are not emulated, so should consider host compat */ + if (compareAgainstHost) { + cmp = cpuGuestData(host, dev->virCPU, NULL, &compare_msg); + switch (cmp) { + case VIR_CPU_COMPARE_INCOMPATIBLE: + if (compare_msg) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("guest and host CPU are not compatible: %s"), + compare_msg); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("guest CPU is not compatible with host CPU")); + } + case VIR_CPU_COMPARE_ERROR: + goto error; + + default: + break; + } + } + + if (virAsprintf(&model, "%s-x86_64-cpu", dev->virCPU->model) < 0) + goto error; + + if (virAsprintf(deviceStr, "%s,id=%s,apic-id=%d", + model, dev->info.alias, dev->apic_id) < 0) + goto error; + + return 0; + + error: + VIR_FREE(model); + VIR_FREE(compare_msg); + virObjectUnref(caps); + return -1; +} + +static int +qemuBuildCPUDeviceCommandLine(virCommandPtr cmd, + virQEMUDriverPtr driver, + const virDomainDef *def, + virDomainCPUDefPtr dev, + virQEMUCapsPtr qemuCaps) +{ + char *devstr = NULL; + + if (qemuBuildCPUDeviceStr(&devstr, driver, def, dev, qemuCaps) < 0) + return -1; + + virCommandAddArgList(cmd, "-device", devstr, NULL); + VIR_FREE(devstr); + return 0; +} + + static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -9922,6 +10004,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + /* add cpu devices */ + for (i = 0; i < def->ncpus; i++) { + if (qemuBuildCPUDeviceCommandLine(cmd, driver, def, def->cpus[i], qemuCaps) < 0) + goto error; + } + if (def->nvram) { if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index c63fd30..18e0dfb 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -94,6 +94,13 @@ qemuBuildChrDeviceStr(char **deviceStr, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps); +int +qemuBuildCPUDeviceStr(char **deviceStr, + virQEMUDriverPtr driver, + const virDomainDef *def, + virDomainCPUDefPtr cpu, + virQEMUCapsPtr qemuCaps); + /* With vlan == -1, use netdev syntax, else old hostnet */ char *qemuBuildHostNetStr(virDomainNetDefPtr net, virQEMUDriverPtr driver, -- 1.9.3

This patch implements live hotplug of a cpu device. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 6 ++++++ src/qemu/qemu_hotplug.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 8 +++++++ 3 files changed, 71 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c6318bc..00a36e7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7008,6 +7008,12 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; case VIR_DOMAIN_DEVICE_CPU: + ret = qemuDomainAttachCPUDevice(driver, vm, + dev->data.cpu); + if (!ret) + dev->data.cpu = NULL; + break; + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cf8362c..f51555d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1543,6 +1543,63 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, return ret; } +int +qemuDomainCPUInsert(virDomainDefPtr vmdef, + virDomainCPUDefPtr cpu) +{ + if (virDomainCPUInsert(vmdef, cpu) < 0) + return -1; + + return 0; +} + +int qemuDomainAttachCPUDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainCPUDefPtr cpu) +{ + int ret = -1; + char *devstr = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr vmdef = vm->def; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("qemu does not support -device")); + goto cleanup; + } + + if (qemuAssignDeviceCPUAlias(vmdef, cpu, -1) < 0) + goto cleanup; + + cpu->apic_id = virDomainCPUGetFreeApicID(vmdef); + + if (qemuBuildCPUDeviceStr(&devstr, driver, vmdef, cpu, priv->qemuCaps) < 0) + goto cleanup; + + if (qemuDomainCPUInsert(vmdef, cpu) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) { + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + goto cleanup; + } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } + + ignore_value(virBitmapSetBit(vm->def->apic_id_map, cpu->apic_id)); + ret = 0; + + cleanup: + VIR_FREE(devstr); + return ret; +} + static int qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index a30788d..c583fb7 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -106,6 +106,14 @@ virDomainChrDefPtr qemuDomainChrRemove(virDomainDefPtr vmdef, virDomainChrDefPtr chr); +int +qemuDomainAttachCPUDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainCPUDefPtr cpu); +int +qemuDomainCPUInsert(virDomainDefPtr vmdef, + virDomainCPUDefPtr cpu); + int qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev); -- 1.9.3

This patch implements live hotunplug of a cpu device. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_hotplug.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 4 +++ 3 files changed, 81 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 00a36e7..bf33d7a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7090,6 +7090,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachChrDevice(driver, vm, dev->data.chr); break; case VIR_DOMAIN_DEVICE_CPU: + ret = qemuDomainDetachCPUDevice(driver, vm, dev->data.cpu); + break; case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f51555d..975ed1d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2918,6 +2918,27 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, } +static int +qemuDomainRemoveCPUDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainCPUDefPtr cpu) +{ + virObjectEventPtr event; + + VIR_DEBUG("Removing cpu device %s from domain %p %s", + cpu->info.alias, vm, vm->def->name); + + event = virDomainEventDeviceRemovedNewFromObj(vm, cpu->info.alias); + if (event) + qemuDomainEventQueue(driver, event); + + virDomainCPURemove(vm->def, cpu); + virDomainCPUDefFree(cpu); + + return 0; +} + + int qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3843,3 +3864,57 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, VIR_FREE(devstr); return ret; } + +int qemuDomainDetachCPUDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainCPUDefPtr cpu) +{ + int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr vmdef = vm->def; + virDomainCPUDefPtr tmpCPU; + int rc = 0; + + if (!(tmpCPU = virDomainCPUFind(vmdef, cpu))) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device not present in domain configration")); + return ret; + } + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("qemu does not support -device")); + } + + if (!tmpCPU->info.alias) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("detaching cpu device without id is not supported")); + return ret; + } + + qemuDomainMarkDeviceForRemoval(vm, &tmpCPU->info); + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorDelDevice(priv->mon, tmpCPU->info.alias) < 0) { + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + goto cleanup; + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } + + ignore_value(virBitmapClearBit(vm->def->apic_id_map, tmpCPU->apic_id)); + + rc = qemuDomainWaitForDeviceRemoval(vm); + if (rc == 0 || rc == 1) + ret = qemuDomainRemoveCPUDevice(driver, vm, tmpCPU); + else + goto cleanup; + + cleanup: + qemuDomainResetDeviceRemoval(vm); + return ret; +} diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index c583fb7..c168ac5 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -113,6 +113,10 @@ qemuDomainAttachCPUDevice(virQEMUDriverPtr driver, int qemuDomainCPUInsert(virDomainDefPtr vmdef, virDomainCPUDefPtr cpu); +int +qemuDomainDetachCPUDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainCPUDefPtr cpu); int qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, -- 1.9.3

JSON array of cpu info is sorted in order to find thread id of cpu smoothly. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/qemu/qemu_monitor_json.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index da5c14d..96a964c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1162,6 +1162,21 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) return ret; } +static int +qemuCPUInfoCompare(const void *a, + const void *b) +{ + virJSONValuePtr *entrya = (virJSONValuePtr *)a; + virJSONValuePtr *entryb = (virJSONValuePtr *)b; + int ia; + int ib; + + virJSONValueObjectGetNumberInt(*entrya, "CPU", &ia); + virJSONValueObjectGetNumberInt(*entryb, "CPU", &ib); + + return ia - ib; +} + /* * [ { "CPU": 0, "current": true, "halted": false, "pc": 3227107138 }, @@ -1176,6 +1191,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply, size_t i; int *threads = NULL; int ncpus; + virJSONValuePtr *entryarray = NULL; if (!(data = virJSONValueObjectGet(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1198,16 +1214,26 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply, if (VIR_ALLOC_N(threads, ncpus) < 0) goto cleanup; + if (VIR_ALLOC_N(entryarray, ncpus) < 0) + goto cleanup; + for (i = 0; i < ncpus; i++) { virJSONValuePtr entry = virJSONValueArrayGet(data, i); - int thread; if (!entry) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpu information was missing an array element")); goto cleanup; } - if (virJSONValueObjectGetNumberInt(entry, "thread_id", &thread) < 0) { + entryarray[i] = entry; + } + + qsort(entryarray, ncpus, sizeof(entryarray[0]), qemuCPUInfoCompare); + + for (i = 0; i < ncpus; i++) { + int thread; + if (virJSONValueObjectGetNumberInt(entryarray[i], "thread_id", &thread) < 0) { + /* Some older qemu versions don't report the thread_id, * so treat this as non-fatal, simply returning no data */ ret = 0; @@ -1223,6 +1249,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply, cleanup: VIR_FREE(threads); + VIR_FREE(entryarray); return ret; } -- 1.9.3

After hotplugging the CPUs, we need to re-detect threads corresponding to Vcpus. Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 274 ++++++++++++++++++++++++----------------------- src/qemu/qemu_driver.h | 8 ++ src/qemu/qemu_hotplug.c | 7 ++ src/util/virbitmap.c | 2 +- src/util/virbitmap.h | 2 + 6 files changed, 160 insertions(+), 134 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b23c45c..4399b58 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1047,6 +1047,7 @@ virBitmapFree; virBitmapGetBit; virBitmapIsAllClear; virBitmapIsAllSet; +virBitmapIsSet; virBitmapLastSetBit; virBitmapNew; virBitmapNewCopy; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bf33d7a..745bedd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4357,89 +4357,49 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); } -static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +/* After hotplugging the CPUs we need to re-detect threads corresponding + * * to the virtual CPUs. Some older versions don't provide the thread ID + * * or don't have the "info cpus" command (and they don't support multiple + * * CPUs anyways), so errors in the re-detection will not be treated + * * fatal */ +int +qemuDomainDetectVcpu(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int apic_id, + bool plug) { qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; - int rc = 1; - int ret = -1; int oldvcpus = vm->def->vcpus; int vcpus = oldvcpus; pid_t *cpupids = NULL; int ncpupids; + int ret = -1; virCgroupPtr cgroup_vcpu = NULL; char *mem_mask = NULL; - uint32_t apic_id; - - qemuDomainObjEnterMonitor(driver, vm); - - /* We need different branches here, because we want to offline - * in reverse order to onlining, so any partial fail leaves us in a - * reasonably sensible state */ - if (nvcpus > vcpus) { - for (i = vcpus; i < nvcpus; i++) { - /* Online new CPU */ - apic_id = virDomainCPUGetFreeApicID(vm->def); - rc = qemuMonitorSetCPU(priv->mon, apic_id, true); - if (rc == 0) - goto unsupported; - if (rc < 0) - goto exit_monitor; - - vcpus++; - ignore_value(virBitmapSetBit(vm->def->apic_id_map, apic_id)); - } - } else { - for (i = vcpus - 1; i >= nvcpus; i--) { - /* Offline old CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, false); - if (rc == 0) - goto unsupported; - if (rc < 0) - goto exit_monitor; - - vcpus--; - } - } - - /* hotplug succeeded */ + int idx = 0; + size_t i; + int *thread = NULL; ret = 0; + if (plug) + vcpus++; + else + vcpus--; + + qemuDomainObjEnterMonitor(driver, vm); - /* After hotplugging the CPUs we need to re-detect threads corresponding - * to the virtual CPUs. Some older versions don't provide the thread ID - * or don't have the "info cpus" command (and they don't support multiple - * CPUs anyways), so errors in the re-detection will not be treated - * fatal */ if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { virResetLastError(); - goto exit_monitor; - } - if (qemuDomainObjExitMonitor(driver, vm) < 0) { - ret = -1; goto cleanup; } - /* check if hotplug has failed */ - if (vcpus < oldvcpus && ncpupids == oldvcpus) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("qemu didn't unplug the vCPUs properly")); - vcpus = oldvcpus; - ret = -1; - goto cleanup; + for (i = 0; i < apic_id; i++) { + if (virBitmapIsSet(vm->def->apic_id_map, i)) + idx++; } - if (ncpupids != vcpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("got wrong number of vCPU pids from QEMU monitor. " - "got %d, wanted %d"), - ncpupids, vcpus); - vcpus = oldvcpus; - ret = -1; + if (VIR_ALLOC_N(thread, vcpus) < 0) goto cleanup; - } if (virDomainNumatuneGetMode(vm->def->numatune, -1) == VIR_DOMAIN_NUMATUNE_MEM_STRICT && @@ -4448,105 +4408,153 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, &mem_mask, -1) < 0) goto cleanup; - if (nvcpus > oldvcpus) { - for (i = oldvcpus; i < nvcpus; i++) { - if (priv->cgroup) { - int rv = -1; - /* Create cgroup for the onlined vcpu */ - if (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0) - goto cleanup; - - if (mem_mask && - virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) - goto cleanup; + if (vcpus > oldvcpus) { + if (priv->cgroup) { + int rv = -1; + if (virCgroupNewVcpu(priv->cgroup, apic_id, true, &cgroup_vcpu) < 0) + goto cleanup; - /* Add vcpu thread to the cgroup */ - rv = virCgroupAddTask(cgroup_vcpu, cpupids[i]); - if (rv < 0) { - virReportSystemError(-rv, - _("unable to add vcpu %zu task %d to cgroup"), - i, cpupids[i]); - virCgroupRemove(cgroup_vcpu); - goto cleanup; - } + if (mem_mask && + virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) + goto cleanup; + rv = virCgroupAddTask(cgroup_vcpu, cpupids[idx]); + if (rv < 0) { + virReportSystemError(-rv, + _("unable to add vcpu %d task %d to cgroup"), + apic_id, cpupids[idx]); + virCgroupRemove(cgroup_vcpu); + goto cleanup; } + } - /* Inherit def->cpuset */ - if (vm->def->cpumask) { - /* vm->def->cputune.vcpupin can't be NULL if - * vm->def->cpumask is not NULL. - */ - virDomainVcpuPinDefPtr vcpupin = NULL; + if (vm->def->cpumask) { + virDomainVcpuPinDefPtr vcpupin = NULL; - if (VIR_ALLOC(vcpupin) < 0) - goto cleanup; + if (VIR_ALLOC(vcpupin) < 0) + goto cleanup; - vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN); - virBitmapCopy(vcpupin->cpumask, vm->def->cpumask); - vcpupin->vcpuid = i; - if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin, vcpupin) < 0) { - virBitmapFree(vcpupin->cpumask); - VIR_FREE(vcpupin); + vcpupin->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN); + virBitmapCopy(vcpupin->cpumask, vm->def->cpumask); + vcpupin->vcpuid = apic_id; + if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, + vm->def->cputune.nvcpupin, vcpupin) < 0) { + virBitmapFree(vcpupin->cpumask); + VIR_FREE(vcpupin); + ret = -1; + goto cleanup; + } + + if (cgroup_vcpu) { + if (qemuSetupCgroupVcpuPin(cgroup_vcpu, + vm->def->cputune.vcpupin, + vm->def->cputune.nvcpupin, apic_id) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("failed to set cpuset.cpus in cgroup" + " for vcpu %d"), apic_id); ret = -1; goto cleanup; } - - if (cgroup_vcpu) { - if (qemuSetupCgroupVcpuPin(cgroup_vcpu, - vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin, i) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("failed to set cpuset.cpus in cgroup" - " for vcpu %zu"), i); - ret = -1; - goto cleanup; - } - } else { - if (virProcessSetAffinity(cpupids[i], - vcpupin->cpumask) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("failed to set cpu affinity for vcpu %zu"), - i); - ret = -1; - goto cleanup; - } + } else { + if (virProcessSetAffinity(cpupids[idx], + vcpupin->cpumask) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to set cpu affinity for vcpu %d"), + apic_id); + ret = -1; + goto cleanup; } } virCgroupFree(&cgroup_vcpu); } } else { - for (i = oldvcpus - 1; i >= nvcpus; i--) { - if (priv->cgroup) { - if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu) < 0) - goto cleanup; - - /* Remove cgroup for the offlined vcpu */ - virCgroupRemove(cgroup_vcpu); - virCgroupFree(&cgroup_vcpu); - } + if (priv->cgroup) { + if (virCgroupNewVcpu(priv->cgroup, apic_id, false, &cgroup_vcpu) < 0) + goto cleanup; - /* Free vcpupin setting */ - virDomainVcpuPinDel(vm->def, i); + virCgroupRemove(cgroup_vcpu); + virCgroupFree(&cgroup_vcpu); } + + virDomainVcpuPinDel(vm->def, apic_id); } priv->nvcpupids = ncpupids; VIR_FREE(priv->vcpupids); priv->vcpupids = cpupids; cpupids = NULL; + thread = NULL; cleanup: + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + vm->def->vcpus = vcpus; VIR_FREE(cpupids); + VIR_FREE(thread); VIR_FREE(mem_mask); - if (virDomainObjIsActive(vm)) - vm->def->vcpus = vcpus; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); + virDomainAuditVcpu(vm, oldvcpus, vcpus, "update", true); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); return ret; +} + +static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int nvcpus) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + int rc = 1; + int ret = -1; + int oldvcpus = vm->def->vcpus; + int vcpus = oldvcpus; + uint32_t apic_id; + + /* We need different branches here, because we want to offline + * in reverse order to onlining, so any partial fail leaves us in a + * reasonably sensible state */ + if (nvcpus > vcpus) { + for (i = vcpus; i < nvcpus; i++) { + /* Online new CPU */ + qemuDomainObjEnterMonitor(driver, vm); + apic_id = virDomainCPUGetFreeApicID(vm->def); + rc = qemuMonitorSetCPU(priv->mon, apic_id, true); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto exit_monitor; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } + + vcpus++; + ignore_value(virBitmapSetBit(vm->def->apic_id_map, apic_id)); + + if (qemuDomainDetectVcpu(driver, vm, apic_id, true) < 0) + goto cleanup; + } + } else { + for (i = vcpus - 1; i >= nvcpus; i--) { + /* Offline old CPU */ + rc = qemuMonitorSetCPU(priv->mon, i, false); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto exit_monitor; + + vcpus--; + } + } + + /* hotplug succeeded */ + + ret = 0; + + cleanup: + return ret; unsupported: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_driver.h b/src/qemu/qemu_driver.h index df7533a..55ce618 100644 --- a/src/qemu/qemu_driver.h +++ b/src/qemu/qemu_driver.h @@ -20,10 +20,18 @@ * * Author: Daniel P. Berrange <berrange@redhat.com> */ +#include "qemu_conf.h" +#include "domain_conf.h" #ifndef __QEMU_DRIVER_H__ # define __QEMU_DRIVER_H__ int qemuRegister(void); +int +qemuDomainDetectVcpu(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int apic_id, + bool plug); + #endif /* __QEMU_DRIVER_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 975ed1d..90ba17a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -31,6 +31,7 @@ #include "qemu_command.h" #include "qemu_hostdev.h" #include "qemu_interface.h" +#include "qemu_driver.h" #include "domain_audit.h" #include "netdev_bandwidth_conf.h" #include "domain_nwfilter.h" @@ -1595,6 +1596,9 @@ int qemuDomainAttachCPUDevice(virQEMUDriverPtr driver, ignore_value(virBitmapSetBit(vm->def->apic_id_map, cpu->apic_id)); ret = 0; + if (qemuDomainDetectVcpu(driver, vm, cpu->apic_id, true) < 0) + ret = -1; + cleanup: VIR_FREE(devstr); return ret; @@ -3914,6 +3918,9 @@ int qemuDomainDetachCPUDevice(virQEMUDriverPtr driver, else goto cleanup; + if (qemuDomainDetectVcpu(driver, vm, tmpCPU->apic_id, false) < 0) + ret = -1; + cleanup: qemuDomainResetDeviceRemoval(vm); return ret; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 05c50e4..168b8db 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -153,7 +153,7 @@ int virBitmapClearBit(virBitmapPtr bitmap, size_t b) } /* Helper function. caller must ensure b < bitmap->max_bit */ -static bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) +bool virBitmapIsSet(virBitmapPtr bitmap, size_t b) { return !!(bitmap->map[VIR_BITMAP_UNIT_OFFSET(b)] & VIR_BITMAP_BIT(b)); } diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 565264c..57fb195 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -60,6 +60,8 @@ int virBitmapSetBit(virBitmapPtr bitmap, size_t b) int virBitmapClearBit(virBitmapPtr bitmap, size_t b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +bool virBitmapIsSet(virBitmapPtr bitmap, size_t b); + /* * Get setting of bit position @b in @bitmap and store in @result */ -- 1.9.3

Hi all, Any comments about this series? I'm not sure whether this series' method to support cpu hotplug in libvirt is reasonable, so could anyone give more suggestions about this function? Thanks. Regards, Zhu On Wed, 2015-02-04 at 17:18 +0800, Zhu Guihua wrote:
If you apply the folowing patchset in order [PATCH v3 0/7] cpu: add device_add foo-x86_64-cpu support https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg01552.html, [PATCH v2 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg03929.html and [PATCH v2 00/11] cpu: add i386 cpu hot remove support https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg01557.html, qemu can support hotplug and hot-unplug cpu device.
So this patch series will make libvirt support hotplug and hot-unplug cpu device for qemu driver.
This patch series add a new API to support cpu hot-plug/unplug, and leave the existing API by invoking qemu command 'cpu-add' as a legacy.
This patch series realize cpu hot-plug/unplug by libvirt command 'attach-device' and 'detach-device', and invoke qemu command 'device_add' and 'device_del' to support this feature.
v2: - update cpu device's definition, and cpu's apic_id is hidded to users. - add check for compatibility between host cpu and hot added cpu - add a capability for *-x86_64-cpu
Zhu Guihua (12): domain_conf: add support for cpu device configuration in XML domain_conf: introduce cpu def helpers domain_conf: introduce cpu device hotplug helpers qemu_driver: implement cpu device hotplug on config level qemu_command: introduce a func for cpu device alias assignment domain_conf: allocate cpu's apic id dynamically qemu: add a capability for x86_64-cpu qemu: introduce qemuBuildCPUDeviceStr qemu: implement cpu device hotplug on live level qemu: implement cpu device hotunplug on live level qemu_monitor_json: sort JSON array of cpu info qemu_driver: detect threads corresponding to Vcpus
docs/formatdomain.html.in | 28 ++++ docs/schemas/domaincommon.rng | 3 + src/conf/domain_conf.c | 189 +++++++++++++++++++++++++- src/conf/domain_conf.h | 33 +++++ src/libvirt_private.syms | 6 + src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 120 +++++++++++++++++ src/qemu/qemu_command.h | 10 ++ src/qemu/qemu_driver.c | 299 ++++++++++++++++++++++++------------------ src/qemu/qemu_driver.h | 8 ++ src/qemu/qemu_hotplug.c | 140 ++++++++++++++++++++ src/qemu/qemu_hotplug.h | 12 ++ src/qemu/qemu_monitor_json.c | 31 ++++- src/util/virbitmap.c | 2 +- src/util/virbitmap.h | 2 + 16 files changed, 753 insertions(+), 134 deletions(-)

On Thu, Feb 12, 2015 at 19:53:13 +0800, Zhu Guihua wrote:
Hi all,
Any comments about this series?
I'm not sure whether this series' method to support cpu hotplug in libvirt is reasonable, so could anyone give more suggestions about this function? Thanks.
Well, as Dan pointed out in his review for this series and previous version, we are not convinced that we need a way to specify the CPU model and other parameters as having dissimilar CPUs doesn't make sense. A solution is either to build the cpu plug on top of the existing API or provide enough information to convince us that having the cpu model in the XML actually makes sense.
Regards, Zhu
Peter

HI, Do you have plan to update this patchset based on the comments recently or have the latest one to post? I noticed that the patchset for memory hotplug had got merged, is there any plan to merge this patchset? On Fri, Feb 13, 2015 at 12:13 AM, Peter Krempa <pkrempa@redhat.com> wrote:
On Thu, Feb 12, 2015 at 19:53:13 +0800, Zhu Guihua wrote:
Hi all,
Any comments about this series?
I'm not sure whether this series' method to support cpu hotplug in libvirt is reasonable, so could anyone give more suggestions about this function? Thanks.
Well, as Dan pointed out in his review for this series and previous version, we are not convinced that we need a way to specify the CPU model and other parameters as having dissimilar CPUs doesn't make sense.
A solution is either to build the cpu plug on top of the existing API or provide enough information to convince us that having the cpu model in the XML actually makes sense.
Regards, Zhu
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Regards, Zhi Yong Wu

On 03/24/2015 04:30 PM, Zhi Yong Wu wrote:
HI,
Do you have plan to update this patchset based on the comments recently or have the latest one to post?
The develop plan for cpu hotplug in qemu has been changed, it is to add socket-based device_add. So I think we could not continue this cpu hotplug work in libvirt until the interface about this feature in qemu could be determined. Thanks, Zhu
I noticed that the patchset for memory hotplug had got merged, is there any plan to merge this patchset?
On Fri, Feb 13, 2015 at 12:13 AM, Peter Krempa <pkrempa@redhat.com> wrote:
On Thu, Feb 12, 2015 at 19:53:13 +0800, Zhu Guihua wrote:
Hi all,
Any comments about this series?
I'm not sure whether this series' method to support cpu hotplug in libvirt is reasonable, so could anyone give more suggestions about this function? Thanks. Well, as Dan pointed out in his review for this series and previous version, we are not convinced that we need a way to specify the CPU model and other parameters as having dissimilar CPUs doesn't make sense.
A solution is either to build the cpu plug on top of the existing API or provide enough information to convince us that having the cpu model in the XML actually makes sense.
Regards, Zhu Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (4)
-
Daniel P. Berrange
-
Peter Krempa
-
Zhi Yong Wu
-
Zhu Guihua