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 <danielhb413@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
