[libvirt] [PATCH v2 0/4] move qemucaps validations from qemu_command to qemu_domain
Most of the patches were pushed in the first version. These are the patches that didn't make the cut and needed new versions. After these patches I'll refrain from doing more validation moves to qemu_domain.c. I'll send a RFC discussing how we can move all the validations to a new file first, then we can resume this work in the definitive location instead. Changes from previous version 1 [1]: - patch 1 (former patch 04): moved the validation to a new qemuDomainDeviceDefValidateMemory() function which is called by qemuDomainDeviceDefValidate(). - patch 2 (former patch 23): added the missing "if (graphics->data.egl_headless.rendernode)" check. - patch 3 (former patch 24): virReportEnumRangeError is now being used in the "default" label error case. - patch 4 (new): added the VMCOREINFO validation case that I mentioned about in the cover letter of [1] as a difficult case, but Cole mentioned that it was simpler than that and that I was probably missing something. Cole was right. [1] https://www.redhat.com/archives/libvir-list/2019-December/msg00570.html Daniel Henrique Barboza (4): qemu: command: move NVDIMM validation to qemu_domain.c qemu: command: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c qemu: command: move validation of vmcoreinfo to qemu_domain.c src/qemu/qemu_command.c | 51 ++---------------------------- src/qemu/qemu_domain.c | 70 +++++++++++++++++++++++++++++++++++++++-- tests/qemuxml2xmltest.c | 30 +++++++++--------- 3 files changed, 87 insertions(+), 64 deletions(-) -- 2.23.0
Move the NVDIMM validation from qemuBuildMachineCommandLine() to a new function in qemu_domain.c, qemuDomainDeviceDefValidateMemory(), which is called by qemuDomainDeviceDefValidate(). This allows NVDIMM validation to occur in domain define time. It also increments memory hotplug validation, which can be seen by the failures in the hotplug tests in qemuxml2xmltest.c that needed to be adjusted after the move. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 5 ----- src/qemu/qemu_domain.c | 18 +++++++++++++++++- tests/qemuxml2xmltest.c | 12 ++++++------ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 836057a4ff..3267b0d4b5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6977,11 +6977,6 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, for (i = 0; i < def->nmems; i++) { if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvdimm isn't supported by this QEMU binary")); - return -1; - } virBufferAddLit(&buf, ",nvdimm=on"); break; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 20a9629760..83964db595 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5637,6 +5637,19 @@ qemuDomainDeviceDefValidateSound(virDomainSoundDefPtr sound, return 0; } +static int +qemuDomainDeviceDefValidateMemory(virDomainMemoryDefPtr mem, + virQEMUCapsPtr qemuCaps) +{ + if (mem->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvdimm isn't supported by this QEMU binary")); + return -1; + } + + return 0; +} static int qemuDomainDefValidate(const virDomainDef *def, @@ -8365,9 +8378,12 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, ret = qemuDomainDeviceDefValidateSound(dev->data.sound, qemuCaps); break; + case VIR_DOMAIN_DEVICE_MEMORY: + ret = qemuDomainDeviceDefValidateMemory(dev->data.memory, qemuCaps); + break; + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_SHMEM: - case VIR_DOMAIN_DEVICE_MEMORY: case VIR_DOMAIN_DEVICE_PANIC: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 55bbd924fb..60ae68c58c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1221,12 +1221,12 @@ mymain(void) DO_TEST("memory-hotplug", NONE); DO_TEST("memory-hotplug-nonuma", NONE); DO_TEST("memory-hotplug-dimm", NONE); - DO_TEST("memory-hotplug-nvdimm", NONE); - DO_TEST("memory-hotplug-nvdimm-access", NONE); - DO_TEST("memory-hotplug-nvdimm-label", NONE); - DO_TEST("memory-hotplug-nvdimm-align", NONE); - DO_TEST("memory-hotplug-nvdimm-pmem", NONE); - DO_TEST("memory-hotplug-nvdimm-readonly", NONE); + DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("memory-hotplug-nvdimm-align", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("memory-hotplug-nvdimm-pmem", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("memory-hotplug-nvdimm-readonly", QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("net-udp", NONE); DO_TEST("video-virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU); -- 2.23.0
Move EGL Headless validation from qemuBuildGraphicsEGLHeadlessCommandLine() to qemuDomainDeviceDefValidateGraphics(). This function is called by qemuDomainDefValidate(), validating the graphics parameters in domain define time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 10 +--------- src/qemu/qemu_domain.c | 8 ++++++++ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3267b0d4b5..b3f069d5d4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7733,7 +7733,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, static int qemuBuildGraphicsEGLHeadlessCommandLine(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED, virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, virDomainGraphicsDefPtr graphics) { g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; @@ -7741,13 +7740,6 @@ qemuBuildGraphicsEGLHeadlessCommandLine(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED virBufferAddLit(&opt, "egl-headless"); if (graphics->data.egl_headless.rendernode) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS_RENDERNODE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support OpenGL rendernode " - "with egl-headless graphics type")); - return -1; - } - virBufferAddLit(&opt, ",rendernode="); virQEMUBuildBufferEscapeComma(&opt, graphics->data.egl_headless.rendernode); @@ -7792,7 +7784,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: if (qemuBuildGraphicsEGLHeadlessCommandLine(cfg, cmd, - qemuCaps, graphics) < 0) + graphics) < 0) return -1; break; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 83964db595..fe353e5bc1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7941,6 +7941,14 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + if (graphics->data.egl_headless.rendernode && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS_RENDERNODE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support OpenGL rendernode " + "with egl-headless graphics type")); + return -1; + } + break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: -- 2.23.0
Move smartcard validation being done by qemuBuildSmartcardCommandLine() to the existing qemuDomainSmartcardDefValidate() function. This function is called by qemuDomainDeviceDefValidate(), allowing smartcard validation in domain define time. Tests were adapted to consider the new caps being needed in this earlier stage. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 24 ------------------------ src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 16 +++++++++------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b3f069d5d4..67f7caf9c6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8275,24 +8275,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, switch (smartcard->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - return -1; - } - virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); break; case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - return -1; - } - virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { virBufferAsprintf(&opt, ",cert%zu=", i + 1); @@ -8308,13 +8294,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, break; case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard " - "passthrough mode support")); - return -1; - } - if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, smartcard->data.passthru, @@ -8330,9 +8309,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, break; default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), - smartcard->type); return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fe353e5bc1..f6683d11e0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6156,6 +6156,39 @@ static int qemuDomainSmartcardDefValidate(const virDomainSmartcardDef *def, virQEMUCapsPtr qemuCaps) { + switch (def->type) { + case VIR_DOMAIN_SMARTCARD_TYPE_HOST: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + return -1; + } + break; + + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + return -1; + } + break; + + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard " + "passthrough mode support")); + return -1; + } + break; + + default: + virReportEnumRangeError(virDomainSmartcardType, def->type); + return -1; + } + if (def->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH && qemuDomainChrSourceDefValidate(def->data.passthru, qemuCaps) < 0) return -1; diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 60ae68c58c..64321bcb80 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1332,12 +1332,13 @@ mymain(void) DO_TEST("cpu-check-default-partial2", NONE); DO_TEST("vmcoreinfo", NONE); - DO_TEST("smartcard-host", NONE); - DO_TEST("smartcard-host-certificates", NONE); - DO_TEST("smartcard-host-certificates-database", NONE); - DO_TEST("smartcard-passthrough-tcp", NONE); - DO_TEST("smartcard-passthrough-spicevmc", NONE); - DO_TEST("smartcard-controller", NONE); + DO_TEST("smartcard-host", QEMU_CAPS_CCID_EMULATED); + DO_TEST("smartcard-host-certificates", QEMU_CAPS_CCID_EMULATED); + DO_TEST("smartcard-host-certificates-database", + QEMU_CAPS_CCID_EMULATED); + DO_TEST("smartcard-passthrough-tcp", QEMU_CAPS_CCID_PASSTHRU); + DO_TEST("smartcard-passthrough-spicevmc", QEMU_CAPS_CCID_PASSTHRU); + DO_TEST("smartcard-controller", QEMU_CAPS_CCID_EMULATED); DO_TEST("pseries-cpu-compat-power9", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); @@ -1352,7 +1353,8 @@ mymain(void) QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4, - QEMU_CAPS_VNC); + QEMU_CAPS_VNC, + QEMU_CAPS_CCID_EMULATED); DO_TEST("input-virtio-ccw", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_KEYBOARD, -- 2.23.0
On 12/17/19 7:36 PM, Daniel Henrique Barboza wrote:
Move smartcard validation being done by qemuBuildSmartcardCommandLine() to the existing qemuDomainSmartcardDefValidate() function. This function is called by qemuDomainDeviceDefValidate(), allowing smartcard validation in domain define time.
Tests were adapted to consider the new caps being needed in this earlier stage.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 24 ------------------------ src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 16 +++++++++------- 3 files changed, 42 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b3f069d5d4..67f7caf9c6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8275,24 +8275,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
switch (smartcard->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - return -1; - } - virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); break;
case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - return -1; - } - virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { virBufferAsprintf(&opt, ",cert%zu=", i + 1); @@ -8308,13 +8294,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard " - "passthrough mode support")); - return -1; - } - if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, smartcard->data.passthru, @@ -8330,9 +8309,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, break;
default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), - smartcard->type); return -1; }
We want a virReportEnumRangeError here too. I fixed that and pushed this series Thanks, Cole
Move the validation of vmcoreinfo from qemuBuildVMCoreInfoCommandLine() to qemuDomainDefValidateFeatures(), allowing for validation at domain define time. qemuxml2xmltest.c was changed to account for this caps being now validated at this earlier stage. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 12 ++---------- src/qemu/qemu_domain.c | 11 ++++++++++- tests/qemuxml2xmltest.c | 2 +- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 67f7caf9c6..44cc647359 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9217,21 +9217,13 @@ qemuBuildSEVCommandLine(virDomainObjPtr vm, virCommandPtr cmd, static int qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + const virDomainDef *def) { virTristateSwitch vmci = def->features[VIR_DOMAIN_FEATURE_VMCOREINFO]; if (vmci != VIR_TRISTATE_SWITCH_ON) return 0; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMCOREINFO)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vmcoreinfo is not available " - "with this QEMU binary")); - return -1; - } - virCommandAddArgList(cmd, "-device", "vmcoreinfo", NULL); return 0; } @@ -9933,7 +9925,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildNVRAMCommandLine(cmd, def) < 0) return NULL; - if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildVMCoreInfoCommandLine(cmd, def) < 0) return NULL; if (qemuBuildSEVCommandLine(vm, cmd, def->sev) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f6683d11e0..2dbe6f6454 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5149,6 +5149,16 @@ qemuDomainDefValidateFeatures(const virDomainDef *def, } break; + case VIR_DOMAIN_FEATURE_VMCOREINFO: + if (def->features[i] == VIR_TRISTATE_SWITCH_ON && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMCOREINFO)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vmcoreinfo is not available " + "with this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_APIC: case VIR_DOMAIN_FEATURE_PAE: @@ -5159,7 +5169,6 @@ qemuDomainDefValidateFeatures(const virDomainDef *def, case VIR_DOMAIN_FEATURE_PVSPINLOCK: case VIR_DOMAIN_FEATURE_CAPABILITIES: case VIR_DOMAIN_FEATURE_PMU: - case VIR_DOMAIN_FEATURE_VMCOREINFO: case VIR_DOMAIN_FEATURE_MSRS: case VIR_DOMAIN_FEATURE_LAST: break; diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 64321bcb80..5ab00e5552 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1330,7 +1330,7 @@ mymain(void) DO_TEST("cpu-check-default-none2", NONE); DO_TEST("cpu-check-default-partial", NONE); DO_TEST("cpu-check-default-partial2", NONE); - DO_TEST("vmcoreinfo", NONE); + DO_TEST("vmcoreinfo", QEMU_CAPS_DEVICE_VMCOREINFO); DO_TEST("smartcard-host", QEMU_CAPS_CCID_EMULATED); DO_TEST("smartcard-host-certificates", QEMU_CAPS_CCID_EMULATED); -- 2.23.0
participants (2)
-
Cole Robinson -
Daniel Henrique Barboza