Re: [PATCH v2 04/23] qemu: Forbid config when topology based cpu count doesn't match the config

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

 




On 08/19/2016 10:38 AM, Peter Krempa wrote:
> As of qemu commit:
> commit a32ef3bfc12c8d0588f43f74dcc5280885bbdb30
> Author: Thomas Huth <thuth@xxxxxxxxxx>
> Date:   Wed Jul 22 15:59:50 2015 +0200
> 
>     vl: Add another sanity check to smp_parse() function
> 
> v2.4.0-952-ga32ef3b
> 
> configuration where the maximum CPU count doesn't match the topology is
> rejected. Prior to that only configurations where the topology would
> contain more cpus than the maximum count would be rejected.
> 
> Use QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS as a relevant recent enough
> witness to avoid breaking old configs.
> ---
> 
> Notes:
>     v2:
>     -fixed typo in commit message
>     - already ACKed
> 
>  src/qemu/qemu_domain.c | 38 ++++++++++++++++++++++++++++++++------
>  1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 4b8c878..c56dc75 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2314,12 +2314,21 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>  static int
>  qemuDomainDefValidate(const virDomainDef *def,
>                        virCapsPtr caps ATTRIBUTE_UNUSED,
> -                      void *opaque ATTRIBUTE_UNUSED)
> +                      void *opaque)
>  {
> +    virQEMUDriverPtr driver = opaque;
> +    virQEMUCapsPtr qemuCaps = NULL;
> +    size_t topologycpus;
> +    int ret = -1;
> +
> +    if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
> +                                            def->emulator)))
> +        goto cleanup;
> +
>      if (def->mem.min_guarantee) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("Parameter 'min_guarantee' not supported by QEMU."));
> -        return -1;
> +        goto cleanup;
>      }
> 
>      if (def->os.loader &&
> @@ -2330,7 +2339,7 @@ qemuDomainDefValidate(const virDomainDef *def,
>          if (!qemuDomainMachineIsQ35(def)) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("Secure boot is supported with q35 machine types only"));
> -            return -1;
> +            goto cleanup;
>          }
> 
>          /* Now, technically it is possible to have secure boot on
> @@ -2339,17 +2348,34 @@ qemuDomainDefValidate(const virDomainDef *def,
>          if (def->os.arch != VIR_ARCH_X86_64) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("Secure boot is supported for x86_64 architecture only"));
> -            return -1;
> +            goto cleanup;
>          }
> 
>          if (def->features[VIR_DOMAIN_FEATURE_SMM] != VIR_TRISTATE_SWITCH_ON) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("Secure boot requires SMM feature enabled"));
> -            return -1;
> +            goto cleanup;
>          }
>      }
> 
> -    return 0;
> +    /* qemu as of 2.5.0 rejects SMP topologies that don't match the cpu count */
> +    if (def->cpu && def->cpu->sockets) {
> +        topologycpus = def->cpu->sockets * def->cpu->cores * def->cpu->threads;
> +        if (topologycpus != virDomainDefGetVcpusMax(def)) {
> +            /* presence of query-hotpluggable-cpus should be a good enough witness */
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("CPU topology doesn't match maximum vcpu count"));
> +                goto cleanup;
> +            }

Would it be a good idea to add a VIR_WARN or VIR_INFO (something) that
will give a heads up that the configuration may prevent some future
start? Especially since you went through all the math before detecting
the capability...


John
> +        }
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    virObjectUnref(qemuCaps);
> +    return ret;
>  }
> 
> 

--
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]