On 11/14/18 2:52 PM, Daniel Henrique Barboza wrote:
> Adding maxCpu validation in qemuDomainDefValidate allows the user to
> spot over the board maxCpus counts at editing time, instead of
> facing a runtime error when starting the domain. This check is also
> arch independent.
> 
> This leaves us with 2 calls to qemuProcessValidateCpuCount: one in
> qemuProcessStartValidateXML and the new one at qemuDomainDefValidate.
> 
> The call in qemuProcessStartValidateXML is redundant. Following
> up in that code, there is a call to virDomainDefValidate, which
> in turn will call config.domainValidateCallback. In this case, the
> callback function is qemuDomainDefValidate. This means that, on startup
> time, qemuProcessValidateCpuCount will be called twice.
> 
> To avoid that, let's also remove the qemuProcessValidateCpuCount call
> from qemuProcessStartValidateXML.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
> ---
>  src/qemu/qemu_domain.c  |  4 ++++
>  src/qemu/qemu_process.c | 14 +-------------
>  2 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 37926850b2..3b86d28216 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4084,6 +4084,10 @@ qemuDomainDefValidate(const virDomainDef *def,
>          }
>      }
>  
> +    if (qemuProcessValidateCpuCount(def, qemuCaps) < 0) {
> +        goto cleanup;
> +    }
> +

make check would have told you to remove the { ... } for the one line
goto cleanup;

I'll take care of it for you (as well as the merge conflict it creates
in the subsequent patch).

John

>      if (ARCH_IS_X86(def->os.arch) &&
>          virDomainDefGetVcpusMax(def) > QEMU_MAX_VCPUS_WITHOUT_EIM) {
>          if (!qemuDomainIsQ35(def)) {
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4d134bd7be..2adbf375ee 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5140,21 +5140,9 @@ qemuProcessStartValidateDisks(virDomainObjPtr vm,
>  static int
>  qemuProcessStartValidateXML(virQEMUDriverPtr driver,
>                              virDomainObjPtr vm,
> -                            virQEMUCapsPtr qemuCaps,
>                              virCapsPtr caps,
>                              unsigned int flags)
>  {
> -    /* The bits we validate here are XML configs that we previously
> -     * accepted. We reject them at VM startup time rather than parse
> -     * time so that pre-existing VMs aren't rejected and dropped from
> -     * the VM list when libvirt is updated.
> -     *
> -     * If back compat isn't a concern, XML validation should probably
> -     * be done at parse time.
> -     */
> -    if (qemuProcessValidateCpuCount(vm->def, qemuCaps) < 0)
> -        return -1;
> -
>      /* checks below should not be executed when starting a qemu process for a
>       * VM that was running before (migration, snapshots, save). It's more
>       * important to start such VM than keep the configuration clean */
> @@ -5204,7 +5192,7 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
>  
>      }
>  
> -    if (qemuProcessStartValidateXML(driver, vm, qemuCaps, caps, flags) < 0)
> +    if (qemuProcessStartValidateXML(driver, vm, caps, flags) < 0)
>          return -1;
>  
>      if (qemuProcessStartValidateGraphics(vm) < 0)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to