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 8792f5e..1631421 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12895,6 +12895,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 = @@ -16288,6 +16294,15 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) } } +uint32_t +virDomainCPUGetFreeApicID(virDomainDefPtr def) +{ + int i; + i = virBitmapNextClearBit(def->apic_id_map, 0); + + return i; +} + int virDomainEmulatorPinAdd(virDomainDefPtr def, unsigned char *cpumap, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 09ab194..8869d26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2062,6 +2062,7 @@ struct _virDomainDef { unsigned short maxvcpus; int placement_mode; virBitmapPtr cpumask; + virBitmapPtr apic_id_map; unsigned int iothreads; @@ -2530,6 +2531,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 a2eec83..d08e400 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -426,6 +426,7 @@ virDomainVcpuPinDefFree; virDomainVcpuPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; +virDomainCPUGetFreeApicID; virDomainVideoDefaultRAM; virDomainVideoDefaultType; virDomainVideoDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5994558..6bc7d8d 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

On Wed, 2015-01-21 at 09:56 +0100, Peter Krempa wrote: Got it, thanks. Regards, Zhu

On Wed, Jan 21, 2015 at 16:00:54 +0800, Zhu Guihua wrote: The ABI stability check code needs to be implemented. The copy function needs to be impelemnted too (I didn't check next patch thoug yet) @@ -1158,6 +1163,13 @@ struct _virDomainChrDef { I've looked through the next patch and you don't parse any device info for the CPU type. Is it necessary to have it present at all in that case? Function declarations should be along with function definitions. Same here Breaks "make syntax-check" again: Expected symbol virDomainCPUDefFree is not in ELF library The symbols entry can be added only when the function is defined. Peter

On Wed, Jan 21, 2015 at 16:00:55 +0800, Zhu Guihua wrote: This patch lacks addition to the RNG schemas that would describe the elements that are added and the correct values. Also lacks change to the docs/formatdomain.html.in ... The driver should rather be an enum value that is parsed from the string rather than stroing an inline string. This limits the namespace of devices libvirt supports but simplifies all matching of the driver later on. Atoi is not allowed, use virStrToLong* instead. Also spaces after function name is forbidden. Um? Why not virBufferAsprintf(buf, " apic_id='%d', apic_id)? You won't need the intermediate string. Also it leaks the apic_id string. Peter

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(a)cn.fujitsu.com&gt; --- src/conf/domain_conf.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 13 +++++++++++ src/libvirt_private.syms | 3 +++ 3 files changed, 75 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1f05056..45f954f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12142,6 +12142,65 @@ 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; + + if (src->apic_id == tgt->apic_id) + ret = true; + + 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 618eef3..4b71052 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 9ceff71..032e9a9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -167,6 +167,9 @@ virDomainChrTcpProtocolTypeToString; virDomainChrTypeFromString; virDomainChrTypeToString; virDomainCPUDefFree; +virDomainCPUInsert; +virDomainCPUFind; +virDomainCPURemove; virDomainClockBasisTypeToString; virDomainClockOffsetTypeFromString; virDomainClockOffsetTypeToString; -- 1.9.3

This function used to set a alias name for cpu device. Signed-off-by: Zhu Guihua <zhugh.fnst(a)cn.fujitsu.com&gt; --- 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 c041ee7..69d0a2a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -946,6 +946,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; @@ -1015,6 +1043,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 dcc7127..2898876 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -264,6 +264,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

On Wed, Jan 21, 2015 at 16:00:59 +0800, Zhu Guihua wrote: Misleading subject. This patch adds the capability, but does not impelment support for the device. ... Spurious whitespace change. Peter

On Wed, Jan 21, 2015 at 16:01:00 +0800, Zhu Guihua wrote: You blindly assume that the user passed the correct model here and not any other invalid string. As I've already said, having this converted to an enum makes things easier. Having a buffer for a single virBufferAsprintf case is a bit overkill. As I've asked before. The question here is whether this is equivalent with just increasing the vCPU count in the "-cpu" argument. If not it will require a bit more work for setting cgroups, numa pinning and other stuff. If they are equivalent we should use that instead and have the existing code do the magic. Peter

This patch implements live hotplug of a cpu device. Signed-off-by: Zhu Guihua <zhugh.fnst(a)cn.fujitsu.com&gt; --- src/qemu/qemu_driver.c | 6 ++++++ src/qemu/qemu_hotplug.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 7 +++++++ 3 files changed, 68 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a88f6b4..ddc7eeb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6995,6 +6995,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 f6d7667..bff0d14 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1541,6 +1541,61 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, return ret; } +int +qemuDomainCPUInsert(virDomainDefPtr vmdef, + virDomainCPUDefPtr cpu) +{ + if (virDomainCPUFind(vmdef, cpu)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cpu already exists")); + return -1; + } + + 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; + + if (qemuBuildCPUDeviceStr(&devstr, cpu, priv->qemuCaps) < 0) + goto cleanup; + + if (qemuDomainCPUInsert(vmdef, cpu) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) { + qemuDomainObjExitMonitor(driver, vm); + goto cleanup; + } + qemuDomainObjExitMonitor(driver, vm); + + 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 19ab9a0..6cdead3 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -106,6 +106,13 @@ qemuDomainChrInsert(virDomainDefPtr vmdef, 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, -- 1.9.3

On Wed, 2015-01-21 at 10:27 +0100, Peter Krempa wrote: I will confirm it, thanks. Regards, Zhu

On Wed, Jan 21, 2015 at 16:01:02 +0800, Zhu Guihua wrote: If cpu is not identical (in meaning of being the same pointer), just a definition denoting the same device this will leak the definition that was previously stored in vm->def. Peter

On Wed, Jan 21, 2015 at 16:01:03 +0800, Zhu Guihua wrote: This function is not used anywhere in this patch. Is this (and the other related hunks) here by accident? Peter

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/qemu/qemu_driver.c | 271 ++++++++++++++++++++++++------------------------ src/qemu/qemu_driver.h | 8 ++ src/qemu/qemu_hotplug.c | 7 ++ 3 files changed, 152 insertions(+), 134 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 004bc35..09ac088 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) { + 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,148 @@ 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: + qemuDomainObjExitMonitor(driver, vm); + 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; + + qemuDomainObjExitMonitor(driver, vm); + 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 41013d9..8a20304 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" @@ -1591,6 +1592,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; @@ -3903,6 +3907,9 @@ int qemuDomainDetachCPUDevice(virQEMUDriverPtr driver, else goto cleanup; + if (qemuDomainDetectVcpu(driver, vm, tmpCPU->apic_id, false) < 0) + ret = -1; + cleanup: qemuDomainResetDeviceRemoval(vm); return ret; -- 1.9.3