virDomainCPUDefFree - free memory allocated virDomainCPUDefParseXML - parse job type virDomainCPUDefFormat - output job type Signed-off-by: Zhu Guihua <zhugh.fnst(a)cn.fujitsu.com&gt; --- 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> + ... + &lt;devices&gt; + &lt;cpu match='exact'&gt; + &lt;model fallback='allow'&gt;core2duo&lt;/model&gt; + &lt;vendor&gt;Intel&lt;/vendor&gt; + &lt;topology sockets='1' cores='2' threads='1'/&gt; + &lt;feature policy='disable' name='lahf_lm'/&gt; + &lt;/cpu&gt; + &lt;devices&gt; + ... +</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, 2015-02-04 at 09:25 +0000, Daniel P. Berrange wrote: Yes, it's duplicate. But I think it represents different meaning, it is under xml tag 'device'. Regards, Zhu

On Wed, 2015-02-04 at 09:46 +0000, Daniel P. Berrange wrote: Agree. It is easy to cause misunderstanding. I will find a better way to handle this. Thanks for your help. Regards, Zhu

The config level requires an insert or remove from domain definition structure. Signed-off-by: Zhu Guihua <zhugh.fnst(a)cn.fujitsu.com&gt; --- 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

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(a)cn.fujitsu.com&gt; --- 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

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(a)cn.fujitsu.com&gt; --- 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 hotunplug of a cpu device. Signed-off-by: Zhu Guihua <zhugh.fnst(a)cn.fujitsu.com&gt; --- 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

After hotplugging the CPUs, we need to re-detect threads corresponding to Vcpus. Signed-off-by: Zhu Guihua <zhugh.fnst(a)cn.fujitsu.com&gt; --- 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(a)redhat.com&gt; */ +#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