On 11/15/18 7:38 PM, John Ferlan wrote:

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).



I appreciate it. I'll make sure to execute 'make check' before sending
the patch next time.


Thanks,


Daniel




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