Re: [PATCH v2 20/23] qemu: command: Add support for sparse vcpu topologies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]