On 08/19/2016 10:38 AM, Peter Krempa wrote:
> Add support for using the new approach to hotplug vcpus using device_add
> during startup of qemu to allow sparse vcpu topologies.
>
> There are a few limitations imposed by qemu on the supported
> configuration:
> - vcpu0 needs to be always present and not hotpluggable
> - non-hotpluggable cpus need to be ordered at the beginning
> - order of the vcpus needs to be unique for every single hotpluggable
> entity
>
> Qemu also doesn't really allow to query the information necessary to
> start a VM with the vcpus directly on the commandline. Fortunately they
> can be hotplugged during startup.
>
> The new hotplug code uses the following approach:
> - non-hotpluggable vcpus are counted and put to the -smp option
> - qemu is started
> - qemu is queried for the necessary information
> - the configuration is checked
> - the hotpluggable vcpus are hotplugged
> - vcpus are started
>
Should these "rules" be listed in the html somewhere? Certainly could
be challenging for the first few people to try and set something up... I
it seems the "order" (from the XML) only matters if online, true?
> This patch adds a lot of checking code and enables the support to
> specify the individual vcpu element with qemu.
> ---
> src/qemu/qemu_command.c | 20 ++-
> src/qemu/qemu_domain.c | 76 ++++++++-
> src/qemu/qemu_process.c | 173 +++++++++++++++++++++
> .../qemuxml2argv-cpu-hotplug-startup.args | 20 +++
> .../qemuxml2argv-cpu-hotplug-startup.xml | 29 ++++
> tests/qemuxml2argvtest.c | 2 +
> 6 files changed, 315 insertions(+), 5 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.xml
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 28e5a7e..c1dc390 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7082,17 +7082,29 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>
> static int
> qemuBuildSmpCommandLine(virCommandPtr cmd,
> - const virDomainDef *def)
> + virDomainDefPtr def)
> {
> char *smp;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> + unsigned int maxvcpus = virDomainDefGetVcpusMax(def);
> + unsigned int nvcpus = 0;
> + virDomainVcpuDefPtr vcpu;
> + size_t i;
> +
> + /* count un-hotpluggable enabled vcpus. Hotpluggable ones will be added
> + * in a different way */
> + for (i = 0; i < maxvcpus; i++) {
> + vcpu = virDomainDefGetVcpu(def, i);
> + if (vcpu->online && vcpu->hotpluggable == VIR_TRISTATE_BOOL_NO)
> + nvcpus++;
> + }
>
> virCommandAddArg(cmd, "-smp");
>
> - virBufferAsprintf(&buf, "%u", virDomainDefGetVcpus(def));
> + virBufferAsprintf(&buf, "%u", nvcpus);
>
> - if (virDomainDefHasVcpusOffline(def))
> - virBufferAsprintf(&buf, ",maxcpus=%u", virDomainDefGetVcpusMax(def));
> + if (nvcpus != maxvcpus)
> + virBufferAsprintf(&buf, ",maxcpus=%u", maxvcpus);
> /* sockets, cores, and threads are either all zero
> * or all non-zero, thus checking one of them is enough */
> if (def->cpu && def->cpu->sockets) {
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index aa93498..1602824 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2253,6 +2253,76 @@ qemuDomainRecheckInternalPaths(virDomainDefPtr def,
>
>
> static int
> +qemuDomainDefVcpusPostParse(virDomainDefPtr def)
> +{
> + unsigned int maxvcpus = virDomainDefGetVcpusMax(def);
> + virDomainVcpuDefPtr vcpu;
> + virDomainVcpuDefPtr prevvcpu;
> + size_t i;
> + bool has_order = false;
> +
> + /* vcpu 0 needs to be present, first, and non-hotpluggable */
> + vcpu = virDomainDefGetVcpu(def, 0);
> + if (!vcpu->online) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("vcpu 0 can't be offline"));
> + return -1;
> + }
> + if (vcpu->hotpluggable == VIR_TRISTATE_BOOL_YES) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("vcpu0 can't be hotpluggable"));
> + return -1;
> + }
> + if (vcpu->order != 0 && vcpu->order != 1) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("vcpu0 must be enabled first"));
> + return -1;
> + }
> +
> + if (vcpu->order != 0)
> + has_order = true;
> +
> + prevvcpu = vcpu;
> +
> + /* all online vcpus or no online vcpu need to have order set */
s/no online/non online/ ?
> + for (i = 1; i < maxvcpus; i++) {
> + vcpu = virDomainDefGetVcpu(def, i);
> +
> + if (vcpu->online &&
> + (vcpu->order != 0) != has_order) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("all vcpus must have either set or unset order"));
> + return -1;
> + }
> +
> + /* few conditions for non-hotpluggable (thus onine) vcpus */
s/onine/online
> + if (vcpu->hotpluggable == VIR_TRISTATE_BOOL_NO) {
> + /* they can be ordered only at the beinning */
s/beinning/beginning
> + if (prevvcpu->hotpluggable == VIR_TRISTATE_BOOL_YES) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("online non-hotpluggable vcpus need to be at "
> + "the beginning"));
"need to be ordered prior to hotplugable vcpus"
? Yours works, just trying to work "order" in there since that's what's
important.
> + return -1;
> + }
> +
> + /* they need to be in order (qemu doesn't support any order yet).
> + * Also note that multiple vcpus may share order on some platforms */
> + if (prevvcpu->order > vcpu->order) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("online non-hotpluggable vcpus must be ordered "
> + "in ascending order"));
> + return -1;
> + }
> + }
> +
> + prevvcpu = vcpu;
> + }
> +
> + return 0;
> +}
> +
> +
> +static int
> qemuDomainDefPostParse(virDomainDefPtr def,
> virCapsPtr caps,
> unsigned int parseFlags,
> @@ -2307,6 +2377,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
> if (virSecurityManagerVerify(driver->securityManager, def) < 0)
> goto cleanup;
>
> + if (qemuDomainDefVcpusPostParse(def) < 0)
> + goto cleanup;
> +
> ret = 0;
> cleanup:
> virObjectUnref(qemuCaps);
> @@ -2709,7 +2782,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = {
> .deviceValidateCallback = qemuDomainDeviceDefValidate,
>
> .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG |
> - VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN
> + VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN |
> + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS,
> };
>
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f3915a5..c31d2ad 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4760,6 +4760,167 @@ qemuProcessSetupIOThreads(virDomainObjPtr vm)
> }
>
>
> +static int
> +qemuProcessValidateHotpluggableVcpus(virDomainDefPtr def)
> +{
> + virDomainVcpuDefPtr vcpu;
> + virDomainVcpuDefPtr subvcpu;
> + qemuDomainVcpuPrivatePtr vcpupriv;
> + unsigned int maxvcpus = virDomainDefGetVcpusMax(def);
> + size_t i = 0;
> + size_t j;
> + virBitmapPtr ordermap = NULL;
> + int ret = -1;
> +
> + if (!(ordermap = virBitmapNew(maxvcpus)))
> + goto cleanup;
> +
> + /* validate:
> + * - all hotpluggable entities to be hotplugged have the correct data
> + * - vcpus belonging to a hotpluggable entity share configuration
> + * - order of the hotpluggable entities is unique
> + */
> + for (i = 0; i < maxvcpus; i++) {
> + vcpu = virDomainDefGetVcpu(def, i);
> + vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu);
> +
> + /* skip over hotpluggable entities */
> + if (vcpupriv->vcpus == 0)
> + continue;
> +
> + if (vcpu->order != 0) {
> + if (virBitmapIsBitSet(ordermap, vcpu->order - 1)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("duplicate vcpu order '%u'"), vcpu->order - 1);
> + goto cleanup;
> + }
> +
> + ignore_value(virBitmapSetBit(ordermap, vcpu->order - 1));
> + }
> +
> +
> + for (j = i + 1; j < (i + vcpupriv->vcpus); j++) {
> + subvcpu = virDomainDefGetVcpu(def, j);
> + if (subvcpu->hotpluggable != vcpu->hotpluggable ||
> + subvcpu->online != vcpu->online ||
> + subvcpu->order != vcpu->order) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("vcpus '%zu' and '%zu' are in the same hotplug "
> + "group but differ in configuration"), i, j);
> + goto cleanup;
> + }
> + }
> +
> + if (vcpu->online && vcpu->hotpluggable == VIR_TRISTATE_BOOL_YES) {
> + if ((vcpupriv->socket_id == -1 && vcpupriv->core_id == -1 &&
> + vcpupriv->thread_id == -1) ||
> + !vcpupriv->type) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("vcpu '%zu' is missing hotplug data"), i);
> + goto cleanup;
> + }
> + }
> + }
> +
> + ret = 0;
> + cleanup:
> + virBitmapFree(ordermap);
> + return ret;
> +}
> +
> +
> +static int
> +qemuDomainHasHotpluggableStartupVcpus(virDomainDefPtr def)
> +{
> + size_t maxvcpus = virDomainDefGetVcpusMax(def);
> + virDomainVcpuDefPtr vcpu;
> + size_t i;
> +
> + for (i = 0; i < maxvcpus; i++) {
> + vcpu = virDomainDefGetVcpu(def, i);
> +
> + if (vcpu->online && vcpu->hotpluggable == VIR_TRISTATE_BOOL_YES)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +
> +static int
> +qemuProcessVcpusSortOrder(const void *a,
> + const void *b)
> +{
> + const virDomainVcpuDef *vcpua = a;
> + const virDomainVcpuDef *vcpub = b;
> +
> + return vcpua->order - vcpub->order;
> +}
> +
> +
> +static int
> +qemuProcessSetupHotpluggableVcpus(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + qemuDomainAsyncJob asyncJob)
> +{
> + unsigned int maxvcpus = virDomainDefGetVcpusMax(vm->def);
> + virDomainVcpuDefPtr vcpu;
> + qemuDomainVcpuPrivatePtr vcpupriv;
> + virJSONValuePtr vcpuprops = NULL;
> + size_t i;
> + int ret = -1;
> + int rc;
> +
> + virDomainVcpuDefPtr *bootHotplug = NULL;
> + size_t nbootHotplug = 0;
> +
> + for (i = 0; i < maxvcpus; i++) {
> + vcpu = virDomainDefGetVcpu(vm->def, i);
> + vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu);
> +
> + if (vcpu->hotpluggable == VIR_TRISTATE_BOOL_YES && vcpu->online &&
> + vcpupriv->vcpus != 0) {
> + if (virAsprintf(&vcpupriv->alias, "vcpu%zu", i) < 0)
> + goto cleanup;
Is it possible the ->alias is already filled in?? Just checking we
cannot overwrite here... I don't believe so since this is called at
Launch only...
> +
> + if (VIR_APPEND_ELEMENT(bootHotplug, nbootHotplug, vcpu) < 0)
> + goto cleanup;
> + }
> + }
Yet another "roll your eyes" Coverity complaint...
if nbootHotplug == 0, then bootHotplug == NULL, which is bad for qsort.
[it was the only new one too]
John
> +
> + qsort(bootHotplug, nbootHotplug, sizeof(*bootHotplug),
> + qemuProcessVcpusSortOrder);
> +
> + for (i = 0; i < nbootHotplug; i++) {
> + vcpu = bootHotplug[i];
> +
> + if (!(vcpuprops = qemuBuildHotpluggableCPUProps(vcpu)))
> + goto cleanup;
> +
> + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> + goto cleanup;
> +
> + rc = qemuMonitorAddDeviceArgs(qemuDomainGetMonitor(vm), vcpuprops);
> + vcpuprops = NULL;
> +
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
> +
> + if (rc < 0)
> + goto cleanup;
> +
> + virJSONValueFree(vcpuprops);
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(bootHotplug);
> + virJSONValueFree(vcpuprops);
> + return ret;
> +}
> +
> +
> /**
> * qemuProcessPrepareDomain
> *
> @@ -5236,6 +5397,18 @@ qemuProcessLaunch(virConnectPtr conn,
> if (qemuSetupCpusetMems(vm) < 0)
> goto cleanup;
>
> + VIR_DEBUG("setting up hotpluggable cpus");
> + if (qemuDomainHasHotpluggableStartupVcpus(vm->def)) {
> + if (qemuDomainRefreshVcpuInfo(driver, vm, asyncJob, false) < 0)
> + goto cleanup;
> +
> + if (qemuProcessValidateHotpluggableVcpus(vm->def) < 0)
> + goto cleanup;
> +
> + if (qemuProcessSetupHotpluggableVcpus(driver, vm, asyncJob) < 0)
> + goto cleanup;
> + }
> +
> VIR_DEBUG("Refreshing VCPU info");
> if (qemuDomainRefreshVcpuInfo(driver, vm, asyncJob, false) < 0)
> goto cleanup;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.args
> new file mode 100644
> index 0000000..035f250
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.args
> @@ -0,0 +1,20 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest1 \
> +-S \
> +-M pc \
> +-m 214 \
> +-smp 1,maxcpus=6,sockets=3,cores=2,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot n \
> +-usb \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.xml
> new file mode 100644
> index 0000000..58718aa
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-hotplug-startup.xml
> @@ -0,0 +1,29 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219100</memory>
> + <currentMemory unit='KiB'>219100</currentMemory>
> + <vcpu placement='static' current='3'>6</vcpu>
> + <vcpus>
> + <vcpu id='0' enabled='yes' hotpluggable='no' order='1'/>
> + <vcpu id='1' enabled='no' hotpluggable='yes'/>
> + <vcpu id='2' enabled='no' hotpluggable='yes'/>
> + <vcpu id='3' enabled='no' hotpluggable='yes'/>
> + <vcpu id='4' enabled='yes' hotpluggable='yes' order='2'/>
> + <vcpu id='5' enabled='yes' hotpluggable='yes' order='3'/>
> + </vcpus>
> + <os>
> + <type arch='x86_64' machine='pc'>hvm</type>
> + <boot dev='network'/>
> + </os>
> + <cpu>
> + <topology sockets="3" cores="2" threads="1"/>
> + </cpu>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index e8b8cb4..39abe72 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2106,6 +2106,8 @@ mymain(void)
> DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_PCI_BRIDGE,
> QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_INTEL_IOMMU);
>
> + DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS);
> +
> qemuTestDriverFree(&driver);
>
> return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list