[libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
(series based on master commit 97cafa610ecf5) This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]: ------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. ------- Cole also mentioned that the machine features validation was a good place to start. I took it a step further and did it across the board on all qemu_command.c. This series didn't create any new validation, just moved them to domain define time. Any other outcome is unintended. Not all cases where covered in this work. For a first version I decided to do only the most trivial cases. These are the other cases I left out for another day: - all verifications contained in functions that are also called by qemu_hotplug.c. This means that the hotplug process will also use the validation code, and we can't just move it to qemu_domain.c. Besides, not all validation done in domain define time applies to hotplug, so it's not simply a matter of calling the same function from qemu_domain in qemu_hotplug. I have patches that moved the verification of all virtio options to qemu_domain.c, while also considering qemu_hotplug validation. In the end I decided to leave it away from this work for now because (1) it took 8 patches just for virtio case alone and (2) there are a lot of these cases in qemu_command.c and it would be too much to do it all at in this same series. - moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too. - SHMem: part of SHMem validation is being used by hotplug code. I could've moved the non-hotplug validation to define time, instead I decided it's better to to leave it to do it all at once in the future. - DBus-VMState: validation of this fella must be done by first querying if the hash vm->priv->dbusVMState has elements. qemuDomainDefValidate does not have 'vm->priv' in it's API, meaning that I would need to either change the API to include it (which means changing domainValidateCallback), or do the validation by another branch where I can get access to vm->priv. - vmcoreinfo: there are no challenges in moving vmcoreinfo validation to qemuDomainDefValidate. The problem were the unit tests. Moving this validation to domain define time breaks 925 tests on qemuxml2xmltest.c, including tests labelled as 'minimal' which should pass under any circurstances, as far as I understand. It is possible to just shoehorn QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but this would tamper the idea of having to deal with NONE or LATEST or a specific set of capabilities for certain tests. Another case I would rather discuss separately. [1] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html Daniel Henrique Barboza (26): qemu_command.c: move PSeries features validation to qemu_domain.c qemu_command.c: move mem.nosharepages validation to qemu_domain.c qemu_command.c: move validation of vmport to qemu_domain.c qemu_command.c: move nvdimm validation to qemu_domain.c qemu_command.c: move I/O APIC validation to qemu_domain.c numa_conf: add virDomainNumaNodesDistancesAreBeingSet() helper qemu_command.c move NUMA validation to qemu_domain.c qemu_command.c: move NVRAM validation to qemu_domain.c qemu_command: move qemuBuildSoundDevStr caps validation to qemu_domain qemu_command.c: move sound codec validation to qemu_domain.c qemu_command: move qemuBuildHubDevStr caps validation to qemu_domain qemu_command: move qemuBuildChrChardevStr caps validation to qemu_domain qemu: move qemuBuildHostdevCommandLine caps validation to qemu_domain qemu_command.c: move vmGenID validation to qemu_domain.c qemu: move qemuBuildSgaCommandLine validation to qemu_domain.c qemu: move virDomainClockDef validation to qemu_domain.c qemu: move qemuBuildPMCommandLine validation to qemu_domain.c qemu: move qemuBuildBootCommandLine validation to qemu_domain.c qemu_command.c: move pcihole64 validation to qemu_domain.c qemu: move qemuBuildGraphicsSDLCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsVNCCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsSPICECommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c qemu: move qemuBuildConsoleCommandLine validation to qemu_domain.c qemu: move qemuBuildTPMDevStr TPM validation to qemu_domain.c src/conf/numa_conf.c | 19 + src/conf/numa_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 575 +--------- src/qemu/qemu_command.h | 1 + src/qemu/qemu_domain.c | 983 ++++++++++++++++-- tests/qemuhotplugtest.c | 10 + tests/qemumemlocktest.c | 16 +- .../default-video-type-aarch64.xml | 2 +- .../default-video-type-ppc64.xml | 2 +- .../default-video-type-s390x.xml | 2 +- .../graphics-egl-headless.args | 2 +- .../graphics-spice-egl-headless.args | 2 +- .../graphics-vnc-egl-headless.args | 2 +- tests/qemuxml2argvtest.c | 80 +- ...ault-video-type-aarch64.aarch64-latest.xml | 4 +- .../default-video-type-ppc64.ppc64-latest.xml | 4 +- .../default-video-type-s390x.s390x-latest.xml | 4 +- tests/qemuxml2xmltest.c | 244 +++-- 19 files changed, 1246 insertions(+), 709 deletions(-) -- 2.23.0
Introduce a new function called qemuDomainDefValidatePSeriesFeature() that will center all the PSeries validation done in qemu_command.c. qemuDomainDefValidatePSeriesFeature() is then called during domain define time, in qemuDomainDefValidateFeatures(). qemuxml2argvtest.c is also changed to include all the caps that now are being validated in define time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 68 +--------------------- src/qemu/qemu_domain.c | 118 ++++++++++++++++++++++++++++++++++++--- tests/qemuxml2argvtest.c | 43 +++++++++++++- 3 files changed, 152 insertions(+), 77 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 49a0dad8d4..12a9d47f44 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7235,33 +7235,11 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON) { if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) { - const char *str; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("HTP resizing is not supported by this " - "QEMU binary")); - return -1; - } - - str = virDomainHPTResizingTypeToString(def->hpt_resizing); - if (!str) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid setting for HPT resizing")); - return -1; - } - - virBufferAsprintf(&buf, ",resize-hpt=%s", str); + virBufferAsprintf(&buf, ",resize-hpt=%s", + virDomainHPTResizingTypeToString(def->hpt_resizing)); } if (def->hpt_maxpagesize > 0) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Configuring the page size for HPT guests " - "is not supported by this QEMU binary")); - return -1; - } - virBufferAsprintf(&buf, ",cap-hpt-max-page-size=%lluk", def->hpt_maxpagesize); } @@ -7269,61 +7247,19 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, if (def->features[VIR_DOMAIN_FEATURE_HTM] != VIR_TRISTATE_SWITCH_ABSENT) { const char *str; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_HTM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("HTM configuration is not supported by this " - "QEMU binary")); - return -1; - } - str = virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_HTM]); - if (!str) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid setting for HTM state")); - return -1; - } - virBufferAsprintf(&buf, ",cap-htm=%s", str); } if (def->features[VIR_DOMAIN_FEATURE_NESTED_HV] != VIR_TRISTATE_SWITCH_ABSENT) { const char *str; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Nested HV configuration is not supported by " - "this QEMU binary")); - return -1; - } - str = virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_NESTED_HV]); - if (!str) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid setting for nested HV state")); - return -1; - } - virBufferAsprintf(&buf, ",cap-nested-hv=%s", str); } if (def->features[VIR_DOMAIN_FEATURE_CCF_ASSIST] != VIR_TRISTATE_SWITCH_ABSENT) { const char *str; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("ccf-assist configuration is not supported by this " - "QEMU binary")); - return -1; - } - str = virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_CCF_ASSIST]); - if (!str) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid setting for ccf-assist state")); - return -1; - } - virBufferAsprintf(&buf, ",cap-ccf-assist=%s", str); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6f53e17b6a..e25b439a39 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4812,6 +4812,114 @@ qemuDomainDefGetVcpuHotplugGranularity(const virDomainDef *def) #define QEMU_MAX_VCPUS_WITHOUT_EIM 255 +static int +qemuDomainDefValidatePSeriesFeature(const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + int feature) +{ + const char *str; + + if (def->features[feature] != VIR_TRISTATE_SWITCH_ABSENT && + !qemuDomainIsPSeries(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%s' feature is not supported for " + "architecture '%s' or machine type '%s'"), + virDomainFeatureTypeToString(feature), + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + + if (def->features[feature] == VIR_TRISTATE_SWITCH_ABSENT) + return 0; + + switch (feature) { + case VIR_DOMAIN_FEATURE_HPT: + if (def->features[feature] != VIR_TRISTATE_SWITCH_ON) + break; + + if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE) { + if (!virQEMUCapsGet(qemuCaps, + QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("HTP resizing is not supported by this " + "QEMU binary")); + return -1; + } + + str = virDomainHPTResizingTypeToString(def->hpt_resizing); + if (!str) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid setting for HPT resizing")); + return -1; + } + } + + if (def->hpt_maxpagesize > 0 && + !virQEMUCapsGet(qemuCaps, + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Configuring the page size for HPT guests " + "is not supported by this QEMU binary")); + return -1; + } + break; + + case VIR_DOMAIN_FEATURE_HTM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_HTM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("HTM configuration is not supported by this " + "QEMU binary")); + return -1; + } + + str = virTristateSwitchTypeToString(def->features[feature]); + if (!str) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid setting for HTM state")); + return -1; + } + + break; + + case VIR_DOMAIN_FEATURE_NESTED_HV: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Nested HV configuration is not supported by " + "this QEMU binary")); + return -1; + } + + str = virTristateSwitchTypeToString(def->features[feature]); + if (!str) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid setting for nested HV state")); + return -1; + } + + break; + + case VIR_DOMAIN_FEATURE_CCF_ASSIST: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ccf-assist configuration is not supported by " + "this QEMU binary")); + return -1; + } + + str = virTristateSwitchTypeToString(def->features[feature]); + if (!str) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid setting for ccf-assist state")); + return -1; + } + + break; + } + + return 0; +} + static int qemuDomainDefValidateFeatures(const virDomainDef *def, virQEMUCapsPtr qemuCaps) @@ -4839,16 +4947,8 @@ qemuDomainDefValidateFeatures(const virDomainDef *def, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: case VIR_DOMAIN_FEATURE_CCF_ASSIST: - if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT && - !qemuDomainIsPSeries(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("The '%s' feature is not supported for " - "architecture '%s' or machine type '%s'"), - featureName, - virArchToString(def->os.arch), - def->os.machine); + if (qemuDomainDefValidatePSeriesFeature(def, qemuCaps, i) < 0) return -1; - } break; case VIR_DOMAIN_FEATURE_GIC: diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ba4a92ec0a..cc1a82488e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1934,8 +1934,47 @@ mymain(void) QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST, QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); - DO_TEST_FAILURE("pseries-features", - QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); + + /* parse error: no QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT */ + DO_TEST_PARSE_ERROR("pseries-features", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HTM, + QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, + QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST); + + /* parse error: no QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE */ + DO_TEST_PARSE_ERROR("pseries-features", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HTM, + QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, + QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST, + QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); + + /* parse error: no QEMU_CAPS_MACHINE_PSERIES_CAP_HTM */ + DO_TEST_PARSE_ERROR("pseries-features", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, + QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, + QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST, + QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); + + /* parse error: no QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV */ + DO_TEST_PARSE_ERROR("pseries-features", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HTM, + QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST, + QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); + + /* parse error: no QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST */ + DO_TEST_PARSE_ERROR("pseries-features", + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, + QEMU_CAPS_MACHINE_PSERIES_CAP_HTM, + QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, + QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT); + DO_TEST_PARSE_ERROR("pseries-features-invalid-machine", NONE); DO_TEST("pseries-serial-native", -- 2.23.0
Move QEMU_CAPS_MEM_MERGE validation from qemuBuildMachineCommandLine() to qemuDomainDefValidateMemory(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 10 +--------- src/qemu/qemu_domain.c | 7 +++++++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 12a9d47f44..a56f324af2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7120,16 +7120,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, cfg->dumpGuestCore ? "on" : "off"); } - if (def->mem.nosharepages) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_MERGE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disable shared memory is not available " - "with this QEMU binary")); - return -1; - } - + if (def->mem.nosharepages) virBufferAddLit(&buf, ",mem-merge=off"); - } if (def->keywrap && !qemuAppendKeyWrapMachineParms(&buf, qemuCaps, def->keywrap)) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e25b439a39..d78c5904ff 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5049,6 +5049,13 @@ qemuDomainDefValidateMemory(const virDomainDef *def, return -1; } + if (mem->nosharepages && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_MERGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disable shared memory is not available " + "with this QEMU binary")); + return -1; + } + return 0; } -- 2.23.0
virQEMUCapsSupportsVmport() is now being called inside qemuDomainDefValidateFeatures() for VIR_DOMAIN_FEATURE_VMPORT feature. qemuxml2xmltest.c was changed to account for this caps being now validated at domain define time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 10 +--------- src/qemu/qemu_domain.c | 12 +++++++++++- tests/qemuxml2xmltest.c | 3 ++- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a56f324af2..e7365ba86a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7097,17 +7097,9 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, */ virBufferAddLit(&buf, ",usb=off"); - if (vmport) { - if (!virQEMUCapsSupportsVmport(qemuCaps, def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vmport is not available " - "with this QEMU binary")); - return -1; - } - + if (vmport != VIR_TRISTATE_SWITCH_ABSENT) virBufferAsprintf(&buf, ",vmport=%s", virTristateSwitchTypeToString(vmport)); - } if (smm) virBufferAsprintf(&buf, ",smm=%s", virTristateSwitchTypeToString(smm)); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d78c5904ff..d62e13f26c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4983,6 +4983,17 @@ qemuDomainDefValidateFeatures(const virDomainDef *def, } break; + case VIR_DOMAIN_FEATURE_VMPORT: + if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsSupportsVmport(qemuCaps, def)) { + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vmport 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: @@ -4993,7 +5004,6 @@ qemuDomainDefValidateFeatures(const virDomainDef *def, case VIR_DOMAIN_FEATURE_PVSPINLOCK: case VIR_DOMAIN_FEATURE_CAPABILITIES: case VIR_DOMAIN_FEATURE_PMU: - case VIR_DOMAIN_FEATURE_VMPORT: case VIR_DOMAIN_FEATURE_VMCOREINFO: case VIR_DOMAIN_FEATURE_MSRS: case VIR_DOMAIN_FEATURE_LAST: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e5bbd8dec4..4353c7a6b8 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -840,7 +840,8 @@ mymain(void) QEMU_CAPS_SPICE, QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_HDA_DUPLEX, - QEMU_CAPS_USB_REDIR); + QEMU_CAPS_USB_REDIR, + QEMU_CAPS_MACHINE_VMPORT_OPT); DO_TEST("pcie-root", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, -- 2.23.0
QEMU_CAPS_DEVICE_NVDIMM validation is now being done inside qemuDomainDefValidateMemory(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 5 ----- src/qemu/qemu_domain.c | 10 ++++++++++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e7365ba86a..25886bf49a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7180,11 +7180,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 d62e13f26c..8eb0905f22 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5021,6 +5021,7 @@ qemuDomainDefValidateMemory(const virDomainDef *def, { const long system_page_size = virGetSystemPageSizeKB(); const virDomainMemtune *mem = &def->mem; + size_t i; if (mem->nhugepages == 0) return 0; @@ -5066,6 +5067,15 @@ qemuDomainDefValidateMemory(const virDomainDef *def, return -1; } + for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->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; } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4353c7a6b8..0e2933e13e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -302,7 +302,7 @@ mymain(void) DO_TEST("hugepages-shared", NONE); DO_TEST("hugepages-memaccess", NONE); DO_TEST("hugepages-memaccess2", NONE); - DO_TEST("hugepages-nvdimm", NONE); + DO_TEST("hugepages-nvdimm", QEMU_CAPS_DEVICE_NVDIMM); DO_TEST("nosharepages", NONE); DO_TEST("restore-v2", NONE); DO_TEST("migrate", NONE); -- 2.23.0
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
QEMU_CAPS_DEVICE_NVDIMM validation is now being done inside qemuDomainDefValidateMemory().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 5 ----- src/qemu/qemu_domain.c | 10 ++++++++++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e7365ba86a..25886bf49a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7180,11 +7180,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 d62e13f26c..8eb0905f22 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5021,6 +5021,7 @@ qemuDomainDefValidateMemory(const virDomainDef *def, { const long system_page_size = virGetSystemPageSizeKB(); const virDomainMemtune *mem = &def->mem; + size_t i;
if (mem->nhugepages == 0) return 0; @@ -5066,6 +5067,15 @@ qemuDomainDefValidateMemory(const virDomainDef *def, return -1; }
+ for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->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; + } + } +
This should be added to a function like qemuDomainDeviceDefValidateMemory called via qemuDomainDeviceDefValidate - Cole
Validation of MACHINE_KERNEL_IRQCHIP and MACHINE_KERNEL_IRQCHIP_SPLIT QEMU caps are now being done in qemuDomainDefValidateFeatures(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 12 ------------ src/qemu/qemu_domain.c | 42 ++++++++++++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 25886bf49a..37339d6f0d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7186,20 +7186,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, } if (def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_NONE) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("I/O APIC tuning is not supported by this " - "QEMU binary")); - return -1; - } switch ((virDomainIOAPIC) def->features[VIR_DOMAIN_FEATURE_IOAPIC]) { case VIR_DOMAIN_IOAPIC_QEMU: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("split I/O APIC is not supported by this " - "QEMU binary")); - return -1; - } virBufferAddLit(&buf, ",kernel_irqchip=split"); break; case VIR_DOMAIN_IOAPIC_KVM: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8eb0905f22..b77b3cf845 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4931,15 +4931,39 @@ qemuDomainDefValidateFeatures(const virDomainDef *def, switch ((virDomainFeature) i) { case VIR_DOMAIN_FEATURE_IOAPIC: - if (def->features[i] != VIR_DOMAIN_IOAPIC_NONE && - !ARCH_IS_X86(def->os.arch)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("The '%s' feature is not supported for " - "architecture '%s' or machine type '%s'"), - featureName, - virArchToString(def->os.arch), - def->os.machine); - return -1; + if (def->features[i] != VIR_DOMAIN_IOAPIC_NONE) { + if (!ARCH_IS_X86(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%s' feature is not supported for " + "architecture '%s' or machine type '%s'"), + featureName, + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("I/O APIC tuning is not supported by " + "this QEMU binary")); + return -1; + } + + switch ((virDomainIOAPIC) def->features[i]) { + case VIR_DOMAIN_IOAPIC_QEMU: + if (!virQEMUCapsGet(qemuCaps, + QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("split I/O APIC is not supported by this " + "QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_IOAPIC_KVM: + case VIR_DOMAIN_IOAPIC_NONE: + case VIR_DOMAIN_IOAPIC_LAST: + break; + } } break; -- 2.23.0
Next patch will validate QEMU_CAPS_NUMA_DIST in a new qemu_domain.c function. The code to verify if a NUMA node distance is being set will still be needed in qemuBuildNumaArgStr() though. To avoid code repetition, let's put this logic in a helper to be used in qemuBuildNumaArgStr() and in the new function. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/numa_conf.c | 19 +++++++++++++++++++ src/conf/numa_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 15 +-------------- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 4bc22ec7d9..6f1257fd8e 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -1188,6 +1188,25 @@ virDomainNumaNodeDistanceIsUsingDefaults(virDomainNumaPtr numa, } +bool +virDomainNumaNodesDistancesAreBeingSet(virDomainNumaPtr numa) +{ + size_t ncells = virDomainNumaGetNodeCount(numa); + size_t i, j; + + for (i = 0; i < ncells; i++) { + for (j = 0; j < ncells; j++) { + if (virDomainNumaNodeDistanceIsUsingDefaults(numa, i, j)) + continue; + + return true; + } + } + + return false; +} + + size_t virDomainNumaGetNodeDistance(virDomainNumaPtr numa, size_t node, diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index e76a09c20c..b1b8e3274d 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -89,6 +89,8 @@ bool virDomainNumaNodeDistanceIsUsingDefaults(virDomainNumaPtr numa, size_t node, size_t sibling) ATTRIBUTE_NONNULL(1); +bool virDomainNumaNodesDistancesAreBeingSet(virDomainNumaPtr numa) + ATTRIBUTE_NONNULL(1); size_t virDomainNumaGetNodeDistance(virDomainNumaPtr numa, size_t node, size_t sibling) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5da5307fa3..df41eb3d16 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -812,6 +812,7 @@ virDomainNumaGetNodeMemoryAccessMode; virDomainNumaGetNodeMemorySize; virDomainNumaNew; virDomainNumaNodeDistanceIsUsingDefaults; +virDomainNumaNodesDistancesAreBeingSet; virDomainNumaSetNodeCount; virDomainNumaSetNodeCpumask; virDomainNumaSetNodeDistance; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 37339d6f0d..7849607b16 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7443,7 +7443,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, int ret = -1; size_t ncells = virDomainNumaGetNodeCount(def->numa); const long system_page_size = virGetSystemPageSizeKB(); - bool numa_distances = false; if (virDomainNumatuneHasPerNodeBinding(def->numa) && !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || @@ -7537,19 +7536,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, /* If NUMA node distance is specified for at least one pair * of nodes, we have to specify all the distances. Even * though they might be the default ones. */ - for (i = 0; i < ncells; i++) { - for (j = 0; j < ncells; j++) { - if (virDomainNumaNodeDistanceIsUsingDefaults(def->numa, i, j)) - continue; - - numa_distances = true; - break; - } - if (numa_distances) - break; - } - - if (numa_distances) { + if (virDomainNumaNodesDistancesAreBeingSet(def->numa)) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA_DIST)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("setting NUMA distances is not " -- 2.23.0
A new qemuDomainDefValidateNuma() function was created to host all the QEMU caps validation being done inside qemuBuildNumaArgStr(). This new function is called by qemuDomainValidateCpuCount() to allow NUMA validation in domain define time. Tests were changed to account for the QEMU capabilities that need to be present at domain define time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 42 ------------------------- src/qemu/qemu_domain.c | 66 ++++++++++++++++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 14 ++++----- tests/qemuxml2xmltest.c | 51 +++++++++++++++++-------------- 4 files changed, 102 insertions(+), 71 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7849607b16..f0f245d730 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7442,26 +7442,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, int rc; int ret = -1; size_t ncells = virDomainNumaGetNodeCount(def->numa); - const long system_page_size = virGetSystemPageSizeKB(); - - if (virDomainNumatuneHasPerNodeBinding(def->numa) && - !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE) || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Per-node memory binding is not supported " - "with this QEMU")); - goto cleanup; - } - - if (def->mem.nhugepages && - def->mem.hugepages[0].size != system_page_size && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("huge pages per NUMA node are not " - "supported with this QEMU")); - goto cleanup; - } if (!virDomainNumatuneNodesetIsAvailable(def->numa, priv->autoNodeset)) goto cleanup; @@ -7482,13 +7462,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, if (rc == 0) needBackend = true; - } else { - if (virDomainNumaGetNodeMemoryAccessMode(def->numa, i)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Shared memory mapping is not supported " - "with this QEMU")); - goto cleanup; - } } } @@ -7501,14 +7474,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, if (!(cpumask = virBitmapFormat(virDomainNumaGetNodeCpumask(def->numa, i)))) goto cleanup; - if (strchr(cpumask, ',') && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disjoint NUMA cpu ranges are not supported " - "with this QEMU")); - goto cleanup; - } - if (needBackend) { virCommandAddArg(cmd, "-object"); virCommandAddArgBuffer(cmd, &nodeBackends[i]); @@ -7537,13 +7502,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, * of nodes, we have to specify all the distances. Even * though they might be the default ones. */ if (virDomainNumaNodesDistancesAreBeingSet(def->numa)) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA_DIST)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting NUMA distances is not " - "supported with this qemu")); - goto cleanup; - } - for (i = 0; i < ncells; i++) { for (j = 0; j < ncells; j++) { size_t distance = virDomainNumaGetNodeDistance(def->numa, i, j); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b77b3cf845..503892a40f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5104,6 +5104,69 @@ qemuDomainDefValidateMemory(const virDomainDef *def, } +static int +qemuDomainDefValidateNuma(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + const long system_page_size = virGetSystemPageSizeKB(); + size_t ncells = virDomainNumaGetNodeCount(def->numa); + size_t i; + bool hasMemoryCap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE) || + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD); + + if (virDomainNumatuneHasPerNodeBinding(def->numa) && !hasMemoryCap) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Per-node memory binding is not supported " + "with this QEMU")); + return -1; + } + + if (def->mem.nhugepages && + def->mem.hugepages[0].size != system_page_size && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("huge pages per NUMA node are not " + "supported with this QEMU")); + return -1; + } + + for (i = 0; i < ncells; i++) { + g_autofree char * cpumask = NULL; + + if (!hasMemoryCap && + virDomainNumaGetNodeMemoryAccessMode(def->numa, i)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Shared memory mapping is not supported " + "with this QEMU")); + return -1; + } + + if (!(cpumask = virBitmapFormat(virDomainNumaGetNodeCpumask(def->numa, i)))) + return -1; + + if (strchr(cpumask, ',') && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disjoint NUMA cpu ranges are not supported " + "with this QEMU")); + return -1; + } + + } + + if (virDomainNumaNodesDistancesAreBeingSet(def->numa) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA_DIST)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting NUMA distances is not " + "supported with this qemu")); + return -1; + } + + return 0; +} + + static int qemuDomainValidateCpuCount(const virDomainDef *def, virQEMUCapsPtr qemuCaps) @@ -5270,6 +5333,9 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidateMemory(def, qemuCaps) < 0) goto cleanup; + if (qemuDomainDefValidateNuma(def, qemuCaps) < 0) + goto cleanup; + if (cfg->vncTLS && cfg->vncTLSx509secretUUID && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { for (i = 0; i < def->ngraphics; i++) { diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index cc1a82488e..3c081651cf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -944,11 +944,11 @@ mymain(void) QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD); - DO_TEST("hugepages-default", NONE); - DO_TEST("hugepages-default-2M", NONE); + DO_TEST("hugepages-default", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-default-2M", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("hugepages-default-system-size", NONE); DO_TEST_PARSE_ERROR("hugepages-default-1G-nodeset-2M", NONE); - DO_TEST("hugepages-nodeset", NONE); + DO_TEST("hugepages-nodeset", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST_PARSE_ERROR("hugepages-nodeset-nonexist", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_OBJECT_MEMORY_FILE, @@ -1711,10 +1711,10 @@ mymain(void) DO_TEST("cpu-numa2", NONE); DO_TEST("cpu-numa-no-memory-element", NONE); DO_TEST_PARSE_ERROR("cpu-numa3", NONE); - DO_TEST_FAILURE("cpu-numa-disjoint", NONE); + DO_TEST_PARSE_ERROR("cpu-numa-disjoint", NONE); DO_TEST("cpu-numa-disjoint", QEMU_CAPS_NUMA); DO_TEST_FAILURE("cpu-numa-memshared", QEMU_CAPS_OBJECT_MEMORY_RAM); - DO_TEST_FAILURE("cpu-numa-memshared", NONE); + DO_TEST_PARSE_ERROR("cpu-numa-memshared", NONE); DO_TEST("cpu-numa-memshared", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("cpu-host-model", NONE); DO_TEST("cpu-host-model-vendor", NONE); @@ -1777,12 +1777,12 @@ mymain(void) DO_TEST("numatune-memnode", QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM); - DO_TEST_FAILURE("numatune-memnode", NONE); + DO_TEST_PARSE_ERROR("numatune-memnode", NONE); DO_TEST("numatune-memnode-no-memory", QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM); - DO_TEST_FAILURE("numatune-memnode-no-memory", NONE); + DO_TEST_PARSE_ERROR("numatune-memnode-no-memory", NONE); DO_TEST("numatune-distances", QEMU_CAPS_NUMA, QEMU_CAPS_NUMA_DIST); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0e2933e13e..215d6b78cd 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -289,20 +289,21 @@ mymain(void) DO_TEST("pmu-feature-off", NONE); DO_TEST("pages-discard", NONE); - DO_TEST("pages-discard-hugepages", NONE); + DO_TEST("pages-discard-hugepages", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("pages-dimm-discard", NONE); - DO_TEST("hugepages-default", NONE); - DO_TEST("hugepages-default-2M", NONE); + DO_TEST("hugepages-default", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-default-2M", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("hugepages-default-system-size", NONE); - DO_TEST("hugepages-nodeset", NONE); - DO_TEST("hugepages-numa-default-2M", NONE); - DO_TEST("hugepages-numa-default-dimm", NONE); - DO_TEST("hugepages-numa-nodeset", NONE); - DO_TEST("hugepages-numa-nodeset-part", NONE); - DO_TEST("hugepages-shared", NONE); - DO_TEST("hugepages-memaccess", NONE); - DO_TEST("hugepages-memaccess2", NONE); - DO_TEST("hugepages-nvdimm", QEMU_CAPS_DEVICE_NVDIMM); + DO_TEST("hugepages-nodeset", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-numa-default-2M", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-numa-default-dimm", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-numa-nodeset", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-numa-nodeset-part", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-shared", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-memaccess", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-memaccess2", QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("hugepages-nvdimm", QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("nosharepages", NONE); DO_TEST("restore-v2", NONE); DO_TEST("migrate", NONE); @@ -490,7 +491,8 @@ mymain(void) DO_TEST("event_idx", NONE); DO_TEST("vhost_queues", NONE); DO_TEST("interface-driver", NONE); - DO_TEST("interface-server", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("interface-server", QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("virtio-lun", NONE); DO_TEST("usb-none", NONE); @@ -538,7 +540,8 @@ mymain(void) DO_TEST("seclabel-dynamic-none", NONE); DO_TEST("seclabel-device-multiple", NONE); DO_TEST_FULL("seclabel-dynamic-none-relabel", WHEN_INACTIVE, - ARG_QEMU_CAPS, QEMU_CAPS_DEVICE_CIRRUS_VGA, NONE); + ARG_QEMU_CAPS, QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_OBJECT_MEMORY_FILE, NONE); DO_TEST("numad-static-vcpu-no-numatune", NONE); DO_TEST("disk-scsi-lun-passthrough-sgio", @@ -572,7 +575,8 @@ mymain(void) DO_TEST("pseries-phb-numa-node", QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, - QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE); + QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("pseries-many-devices", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, @@ -1006,12 +1010,12 @@ mymain(void) DO_TEST("cpu-numa2", NONE); DO_TEST("cpu-numa-no-memory-element", NONE); DO_TEST("cpu-numa-disordered", NONE); - DO_TEST("cpu-numa-disjoint", NONE); - DO_TEST("cpu-numa-memshared", NONE); + DO_TEST("cpu-numa-disjoint", QEMU_CAPS_NUMA); + DO_TEST("cpu-numa-memshared", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("numatune-auto-prefer", NONE); - DO_TEST("numatune-memnode", NONE); - DO_TEST("numatune-memnode-no-memory", NONE); + DO_TEST("numatune-memnode", QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("numatune-memnode-no-memory", QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("bios-nvram", NONE); DO_TEST("bios-nvram-os-interleave", NONE); @@ -1200,10 +1204,12 @@ mymain(void) DO_TEST("memfd-memory-numa", QEMU_CAPS_OBJECT_MEMORY_MEMFD, - QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB); + QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("memfd-memory-default-hugepage", QEMU_CAPS_OBJECT_MEMORY_MEMFD, - QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB); + QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("acpi-table", NONE); @@ -1264,7 +1270,8 @@ mymain(void) DO_TEST("user-aliases", QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_QCOW2_LUKS); + QEMU_CAPS_QCOW2_LUKS, + QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("input-virtio-ccw", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_KEYBOARD, -- 2.23.0
A new function qemuDomainDeviceDefValidateNVRAM() was created to validate the NVRAM in domain define time. Unit test was adjusted to account for the extra QEMU_CAPS_DEVICE_NVRAM required during domain define. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 41 +++++++++++------------------------------ src/qemu/qemu_domain.c | 38 +++++++++++++++++++++++++++++++++++++- tests/qemuxml2xmltest.c | 3 ++- 3 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f0f245d730..58d7aa697e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4098,15 +4098,8 @@ qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && - dev->info.addr.spaprvio.has_reg) { - virBufferAsprintf(&buf, "spapr-nvram.reg=0x%llx", - dev->info.addr.spaprvio.reg); - } else { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("nvram address type must be spaprvio")); - return NULL; - } + virBufferAsprintf(&buf, "spapr-nvram.reg=0x%llx", + dev->info.addr.spaprvio.reg); return virBufferContentAndReset(&buf); } @@ -4114,31 +4107,19 @@ qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) static int qemuBuildNVRAMCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + const virDomainDef *def) { + g_autofree char *optstr = NULL; + if (!def->nvram) return 0; - if (qemuDomainIsPSeries(def)) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvram device is not supported by " - "this QEMU binary")); - return -1; - } - - g_autofree char *optstr = NULL; - virCommandAddArg(cmd, "-global"); - optstr = qemuBuildNVRAMDevStr(def->nvram); - if (!optstr) - return -1; - virCommandAddArg(cmd, optstr); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvram device is only supported for PPC64")); + virCommandAddArg(cmd, "-global"); + optstr = qemuBuildNVRAMDevStr(def->nvram); + if (!optstr) return -1; - } + + virCommandAddArg(cmd, optstr); return 0; } @@ -10261,7 +10242,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, chardevStdioLogd) < 0) return NULL; - if (qemuBuildNVRAMCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildNVRAMCommandLine(cmd, def) < 0) return NULL; if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 503892a40f..35ef2ae47d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5191,6 +5191,39 @@ qemuDomainValidateCpuCount(const virDomainDef *def, } +static int +qemuDomainDeviceDefValidateNVRAM(virDomainNVRAMDefPtr nvram, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + if (!nvram) + return 0; + + if (qemuDomainIsPSeries(def)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvram device is not supported by " + "this QEMU binary")); + return -1; + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvram device is only supported for PPC64")); + return -1; + } + + if (!(nvram->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO && + nvram->info.addr.spaprvio.has_reg)) { + + virReportError(VIR_ERR_XML_ERROR, "%s", + _("nvram address type must be spaprvio")); + return -1; + } + + return 0; +} + + static int qemuDomainDefValidate(const virDomainDef *def, void *opaque) @@ -7632,10 +7665,13 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, ret = qemuDomainDeviceDefValidateFS(dev->data.fs, def, qemuCaps); break; + case VIR_DOMAIN_DEVICE_NVRAM: + ret = qemuDomainDeviceDefValidateNVRAM(dev->data.nvram, def, qemuCaps); + break; + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_HUB: - case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_MEMORY: case VIR_DOMAIN_DEVICE_PANIC: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 215d6b78cd..3d4959a05c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -562,7 +562,8 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_RNG); DO_TEST("pseries-nvram", - QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_DEVICE_NVRAM); DO_TEST("pseries-panic-missing", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); DO_TEST("pseries-panic-no-address", -- 2.23.0
Move QEMU caps validation of QEMU_CAPS_OBJECT_USB_AUDIO and QEMU_CAPS_DEVICE_ICH9_INTEL_HDA to a new function in qemu_domain.c, qemuDomainDeviceDefValidateSound(). This function is called by qemuDomainDeviceDefValidate() to validate the sound device in domain define time. qemuxml2xmltest.c was adjusted to add the now required caps for domain definition. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 15 -------------- src/qemu/qemu_domain.c | 44 ++++++++++++++++++++++++++++++++++++++++- tests/qemuxml2xmltest.c | 4 +++- 3 files changed, 46 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 58d7aa697e..ef5f9c0582 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4280,30 +4280,15 @@ qemuBuildSoundDevStr(const virDomainDef *def, break; case VIR_DOMAIN_SOUND_MODEL_USB: model = "usb-audio"; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_USB_AUDIO)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("usb-audio controller is not supported " - "by this QEMU binary")); - return NULL; - } break; case VIR_DOMAIN_SOUND_MODEL_ICH9: model = "ich9-intel-hda"; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("The ich9-intel-hda audio controller " - "is not supported in this QEMU binary")); - return NULL; - } break; case VIR_DOMAIN_SOUND_MODEL_SB16: model = "sb16"; break; case VIR_DOMAIN_SOUND_MODEL_PCSPK: /* pc-speaker is handled separately */ case VIR_DOMAIN_SOUND_MODEL_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("sound card model '%s' is not supported by qemu"), - virDomainSoundModelTypeToString(sound->model)); return NULL; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 35ef2ae47d..0db96da3c2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5224,6 +5224,45 @@ qemuDomainDeviceDefValidateNVRAM(virDomainNVRAMDefPtr nvram, } +static int +qemuDomainDeviceDefValidateSound(virDomainSoundDefPtr sound, + virQEMUCapsPtr qemuCaps) +{ + switch ((virDomainSoundModel) sound->model) { + case VIR_DOMAIN_SOUND_MODEL_USB: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_USB_AUDIO)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-audio controller is not supported " + "by this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_SOUND_MODEL_ICH9: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("The ich9-intel-hda audio controller " + "is not supported in this QEMU binary")); + return -1; + } + break; + + case VIR_DOMAIN_SOUND_MODEL_ES1370: + case VIR_DOMAIN_SOUND_MODEL_AC97: + case VIR_DOMAIN_SOUND_MODEL_ICH6: + case VIR_DOMAIN_SOUND_MODEL_SB16: + case VIR_DOMAIN_SOUND_MODEL_PCSPK: + break; + case VIR_DOMAIN_SOUND_MODEL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("sound card model '%s' is not supported by qemu"), + virDomainSoundModelTypeToString(sound->model)); + return -1; + } + + return 0; +} + + static int qemuDomainDefValidate(const virDomainDef *def, void *opaque) @@ -7669,8 +7708,11 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, ret = qemuDomainDeviceDefValidateNVRAM(dev->data.nvram, def, qemuCaps); break; - case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_SOUND: + ret = qemuDomainDeviceDefValidateSound(dev->data.sound, qemuCaps); + break; + + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_HUB: case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_MEMORY: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 3d4959a05c..2200d4a652 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -409,7 +409,9 @@ mymain(void) DO_TEST("net-midonet", NONE); DO_TEST("net-openvswitch", NONE); DO_TEST("sound", NONE); - DO_TEST("sound-device", NONE); + DO_TEST("sound-device", + QEMU_CAPS_DEVICE_ICH9_INTEL_HDA, + QEMU_CAPS_OBJECT_USB_AUDIO); DO_TEST("watchdog", NONE); DO_TEST("net-bandwidth", QEMU_CAPS_DEVICE_VGA); DO_TEST("net-bandwidth2", QEMU_CAPS_DEVICE_VGA); -- 2.23.0
qemuBuildSoundCodecStr() validates if a given QEMU binary supports the sound codec. This validation can be moved to qemu_domain.c to be executed in domain define time. The codec validation was moved to the existing qemuDomainDeviceDefValidateSound() function. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 36 ++++-------------------------------- src/qemu/qemu_command.h | 1 + src/qemu/qemu_domain.c | 36 ++++++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 5 ++++- 4 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ef5f9c0582..3b4fae740c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -155,8 +155,6 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary, "" /* no secondary device for ramfb */, ); -VIR_ENUM_DECL(qemuSoundCodec); - VIR_ENUM_IMPL(qemuSoundCodec, VIR_DOMAIN_SOUND_CODEC_TYPE_LAST, "hda-duplex", @@ -4300,40 +4298,16 @@ qemuBuildSoundDevStr(const virDomainDef *def, } -static int -qemuSoundCodecTypeToCaps(int type) -{ - switch (type) { - case VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX: - return QEMU_CAPS_HDA_DUPLEX; - case VIR_DOMAIN_SOUND_CODEC_TYPE_MICRO: - return QEMU_CAPS_HDA_MICRO; - case VIR_DOMAIN_SOUND_CODEC_TYPE_OUTPUT: - return QEMU_CAPS_HDA_OUTPUT; - default: - return -1; - } -} - - static char * qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, - virDomainSoundCodecDefPtr codec, - virQEMUCapsPtr qemuCaps) + virDomainSoundCodecDefPtr codec) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; const char *stype; - int type, flags; + int type; type = codec->type; stype = qemuSoundCodecTypeToString(type); - flags = qemuSoundCodecTypeToCaps(type); - - if (flags == -1 || !virQEMUCapsGet(qemuCaps, flags)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("%s not supported in this QEMU binary"), stype); - return NULL; - } virBufferAsprintf(&buf, "%s,id=%s-codec%d,bus=%s.0,cad=%d", stype, sound->info.alias, codec->cad, sound->info.alias, codec->cad); @@ -4374,8 +4348,7 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, g_autofree char *codecstr = NULL; virCommandAddArg(cmd, "-device"); if (!(codecstr = - qemuBuildSoundCodecStr(sound, sound->codecs[j], - qemuCaps))) { + qemuBuildSoundCodecStr(sound, sound->codecs[j]))) { return -1; } @@ -4389,8 +4362,7 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, }; virCommandAddArg(cmd, "-device"); if (!(codecstr = - qemuBuildSoundCodecStr(sound, &codec, - qemuCaps))) { + qemuBuildSoundCodecStr(sound, &codec))) { return -1; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index f945b2d6d4..786991fd3d 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -41,6 +41,7 @@ #define QEMU_BLOCK_IOTUNE_MAX 1000000000000000LL VIR_ENUM_DECL(qemuVideo); +VIR_ENUM_DECL(qemuSoundCodec); virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, virLogManagerPtr logManager, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0db96da3c2..ee9de98df1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5224,10 +5224,28 @@ qemuDomainDeviceDefValidateNVRAM(virDomainNVRAMDefPtr nvram, } +static int +qemuSoundCodecTypeToCaps(int type) +{ + switch (type) { + case VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX: + return QEMU_CAPS_HDA_DUPLEX; + case VIR_DOMAIN_SOUND_CODEC_TYPE_MICRO: + return QEMU_CAPS_HDA_MICRO; + case VIR_DOMAIN_SOUND_CODEC_TYPE_OUTPUT: + return QEMU_CAPS_HDA_OUTPUT; + default: + return -1; + } +} + + static int qemuDomainDeviceDefValidateSound(virDomainSoundDefPtr sound, virQEMUCapsPtr qemuCaps) { + size_t i; + switch ((virDomainSoundModel) sound->model) { case VIR_DOMAIN_SOUND_MODEL_USB: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_USB_AUDIO)) { @@ -5259,6 +5277,24 @@ qemuDomainDeviceDefValidateSound(virDomainSoundDefPtr sound, return -1; } + if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 || + sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) { + for (i = 0; i < sound->ncodecs; i++) { + const char *stype; + int type, flags; + + type = sound->codecs[i]->type; + stype = qemuSoundCodecTypeToString(type); + flags = qemuSoundCodecTypeToCaps(type); + + if (flags == -1 || !virQEMUCapsGet(qemuCaps, flags)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%s not supported in this QEMU binary"), stype); + return -1; + } + } + } + return 0; } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2200d4a652..f3fec5ee19 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -411,7 +411,10 @@ mymain(void) DO_TEST("sound", NONE); DO_TEST("sound-device", QEMU_CAPS_DEVICE_ICH9_INTEL_HDA, - QEMU_CAPS_OBJECT_USB_AUDIO); + QEMU_CAPS_OBJECT_USB_AUDIO, + QEMU_CAPS_HDA_MICRO, + QEMU_CAPS_HDA_DUPLEX, + QEMU_CAPS_HDA_OUTPUT); DO_TEST("watchdog", NONE); DO_TEST("net-bandwidth", QEMU_CAPS_DEVICE_VGA); DO_TEST("net-bandwidth2", QEMU_CAPS_DEVICE_VGA); -- 2.23.0
Move QEMU caps validation of QEMU_CAPS_USB_HUB to a new function in qemu_domain.c, qemuDomainDeviceDefValidateHub(). This function is called by qemuDomainDeviceDefValidate() to validate the sound device in domain define time. qemuxml2xmltest.c was adjusted to add the now required caps for domain definition. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 13 ------------- src/qemu/qemu_domain.c | 26 +++++++++++++++++++++++++- tests/qemuxml2xmltest.c | 2 +- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3b4fae740c..0fa7f4693e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4736,19 +4736,6 @@ qemuBuildHubDevStr(const virDomainDef *def, { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if (dev->type != VIR_DOMAIN_HUB_TYPE_USB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("hub type %s not supported"), - virDomainHubTypeToString(dev->type)); - return NULL; - } - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_HUB)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("usb-hub not supported by QEMU binary")); - return NULL; - } - virBufferAddLit(&buf, "usb-hub"); virBufferAsprintf(&buf, ",id=%s", dev->info.alias); if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee9de98df1..fe24e736e8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5299,6 +5299,27 @@ qemuDomainDeviceDefValidateSound(virDomainSoundDefPtr sound, } +static int +qemuDomainDeviceDefValidateHub(virDomainHubDefPtr hub, + virQEMUCapsPtr qemuCaps) +{ + if (hub->type != VIR_DOMAIN_HUB_TYPE_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("hub type %s not supported"), + virDomainHubTypeToString(hub->type)); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_HUB)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-hub not supported by QEMU binary")); + return -1; + } + + return 0; +} + + static int qemuDomainDefValidate(const virDomainDef *def, void *opaque) @@ -7748,8 +7769,11 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, ret = qemuDomainDeviceDefValidateSound(dev->data.sound, qemuCaps); break; - case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_HUB: + ret = qemuDomainDeviceDefValidateHub(dev->data.hub, qemuCaps); + break; + + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_MEMORY: case VIR_DOMAIN_DEVICE_PANIC: diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f3fec5ee19..0145970774 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -520,7 +520,7 @@ mymain(void) DO_TEST("ppc64-usb-controller-legacy", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_PIIX3_USB_UHCI); - DO_TEST("usb-port-missing", NONE); + DO_TEST("usb-port-missing", QEMU_CAPS_USB_HUB); DO_TEST("usb-redir", NONE); DO_TEST("usb-redir-filter", NONE); DO_TEST("usb-redir-filter-version", NONE); -- 2.23.0
Move QEMU caps validation of QEMU_CAPS_CHARDEV_FILE_APPEND and QEMU_CAPS_CHARDEV_LOGFILE to qemuDomainChrSourceDefValidate(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 11 ----------- src/qemu/qemu_domain.c | 44 ++++++++++++++++++++++++++++++----------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0fa7f4693e..58409a85d9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5098,12 +5098,6 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, case VIR_DOMAIN_CHR_TYPE_FILE: virBufferAsprintf(&buf, "file,id=%s", charAlias); - if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("append not supported in this QEMU binary")); - return NULL; - } if (qemuBuildChrChardevFileStr(flags & QEMU_BUILD_CHARDEV_FILE_LOGD ? logManager : NULL, cmd, def, &buf, @@ -5239,11 +5233,6 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, } if (dev->logfile) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_LOGFILE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("logfile not supported in this QEMU binary")); - return NULL; - } if (qemuBuildChrChardevFileStr(logManager, cmd, def, &buf, "logfile", dev->logfile, "logappend", dev->logappend) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fe24e736e8..2b97b5f1bd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5525,7 +5525,8 @@ qemuDomainChrSourceReconnectDefValidate(const virDomainChrSourceReconnectDef *de static int -qemuDomainChrSourceDefValidate(const virDomainChrSourceDef *def) +qemuDomainChrSourceDefValidate(const virDomainChrSourceDef *def, + virQEMUCapsPtr qemuCaps) { switch ((virDomainChrType)def->type) { case VIR_DOMAIN_CHR_TYPE_TCP: @@ -5538,11 +5539,19 @@ qemuDomainChrSourceDefValidate(const virDomainChrSourceDef *def) return -1; break; + case VIR_DOMAIN_CHR_TYPE_FILE: + if (def->data.file.append != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FILE_APPEND)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("append not supported in this QEMU binary")); + return -1; + } + break; + case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_PIPE: case VIR_DOMAIN_CHR_TYPE_STDIO: case VIR_DOMAIN_CHR_TYPE_UDP: @@ -5553,6 +5562,14 @@ qemuDomainChrSourceDefValidate(const virDomainChrSourceDef *def) break; } + if (def->logfile) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_LOGFILE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("logfile not supported in this QEMU binary")); + return -1; + } + } + return 0; } @@ -5692,9 +5709,10 @@ qemuDomainChrTargetDefValidate(const virDomainChrDef *chr) static int qemuDomainChrDefValidate(const virDomainChrDef *dev, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { - if (qemuDomainChrSourceDefValidate(dev->source) < 0) + if (qemuDomainChrSourceDefValidate(dev->source, qemuCaps) < 0) return -1; if (qemuDomainChrTargetDefValidate(dev) < 0) @@ -5750,10 +5768,11 @@ qemuDomainChrDefValidate(const virDomainChrDef *dev, static int -qemuDomainSmartcardDefValidate(const virDomainSmartcardDef *def) +qemuDomainSmartcardDefValidate(const virDomainSmartcardDef *def, + virQEMUCapsPtr qemuCaps) { if (def->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH && - qemuDomainChrSourceDefValidate(def->data.passthru) < 0) + qemuDomainChrSourceDefValidate(def->data.passthru, qemuCaps) < 0) return -1; return 0; @@ -5765,7 +5784,7 @@ qemuDomainRNGDefValidate(const virDomainRNGDef *def, virQEMUCapsPtr qemuCaps G_GNUC_UNUSED) { if (def->backend == VIR_DOMAIN_RNG_BACKEND_EGD && - qemuDomainChrSourceDefValidate(def->source.chardev) < 0) + qemuDomainChrSourceDefValidate(def->source.chardev, qemuCaps) < 0) return -1; return 0; @@ -5773,9 +5792,10 @@ qemuDomainRNGDefValidate(const virDomainRNGDef *def, static int -qemuDomainRedirdevDefValidate(const virDomainRedirdevDef *def) +qemuDomainRedirdevDefValidate(const virDomainRedirdevDef *def, + virQEMUCapsPtr qemuCaps) { - if (qemuDomainChrSourceDefValidate(def->source) < 0) + if (qemuDomainChrSourceDefValidate(def->source, qemuCaps) < 0) return -1; return 0; @@ -7695,11 +7715,11 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_CHR: - ret = qemuDomainChrDefValidate(dev->data.chr, def); + ret = qemuDomainChrDefValidate(dev->data.chr, def, qemuCaps); break; case VIR_DOMAIN_DEVICE_SMARTCARD: - ret = qemuDomainSmartcardDefValidate(dev->data.smartcard); + ret = qemuDomainSmartcardDefValidate(dev->data.smartcard, qemuCaps); break; case VIR_DOMAIN_DEVICE_RNG: @@ -7707,7 +7727,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_REDIRDEV: - ret = qemuDomainRedirdevDefValidate(dev->data.redirdev); + ret = qemuDomainRedirdevDefValidate(dev->data.redirdev, qemuCaps); break; case VIR_DOMAIN_DEVICE_WATCHDOG: -- 2.23.0
Move QEMU caps validation of qemuBuildHostdevCommandLine() to qemuDomainDeviceDefValidateHostdev() and qemuDomainMdevDefValidate(), allowing them to be validated at 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 | 32 ------------------------------ src/qemu/qemu_domain.c | 42 +++++++++++++++++++++++++++++++++++++--- tests/qemumemlocktest.c | 16 +++++++++++---- tests/qemuxml2argvtest.c | 6 +++--- tests/qemuxml2xmltest.c | 12 +++++++----- 5 files changed, 61 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 58409a85d9..9431efa384 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5321,17 +5321,6 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, /* PCI */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - int backend = subsys->u.pci.backend; - - if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO PCI device assignment is not " - "supported by this version of qemu")); - return -1; - } - } - unsigned int bootIndex = hostdev->info->bootIndex; /* bootNet will be non-0 if boot order was set and no other @@ -5414,29 +5403,8 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, switch ((virMediatedDeviceModelType) mdevsrc->model) { case VIR_MDEV_MODEL_TYPE_VFIO_PCI: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO PCI device assignment is not " - "supported by this version of QEMU")); - return -1; - } - - break; case VIR_MDEV_MODEL_TYPE_VFIO_CCW: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO CCW device assignment is not " - "supported by this version of QEMU")); - return -1; - } - break; case VIR_MDEV_MODEL_TYPE_VFIO_AP: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_AP)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO AP device assignment is not " - "supported by this version of QEMU")); - return -1; - } break; case VIR_MDEV_MODEL_TYPE_LAST: default: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2b97b5f1bd..b39942c49f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6042,6 +6042,13 @@ qemuDomainMdevDefVFIOPCIValidate(const virDomainHostdevDef *hostdev, { const virDomainHostdevSubsysMediatedDev *dev; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VFIO PCI device assignment is not " + "supported by this version of QEMU")); + return -1; + } + /* VFIO-PCI does not support boot */ if (hostdev->info->bootIndex) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -6085,11 +6092,19 @@ qemuDomainMdevDefVFIOPCIValidate(const virDomainHostdevDef *hostdev, static int qemuDomainMdevDefVFIOAPValidate(const virDomainHostdevDef *hostdev, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { size_t i; bool vfioap_found = false; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_AP)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VFIO AP device assignment is not " + "supported by this version of QEMU")); + return -1; + } + /* VFIO-AP does not support boot */ if (hostdev->info->bootIndex) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -6131,8 +6146,14 @@ qemuDomainMdevDefValidate(const virDomainHostdevDef *hostdev, case VIR_MDEV_MODEL_TYPE_VFIO_PCI: return qemuDomainMdevDefVFIOPCIValidate(hostdev, def, qemuCaps); case VIR_MDEV_MODEL_TYPE_VFIO_AP: - return qemuDomainMdevDefVFIOAPValidate(hostdev, def); + return qemuDomainMdevDefVFIOAPValidate(hostdev, def, qemuCaps); case VIR_MDEV_MODEL_TYPE_VFIO_CCW: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VFIO CCW device assignment is not " + "supported by this version of QEMU")); + return -1; + } break; case VIR_MDEV_MODEL_TYPE_LAST: default: @@ -6150,6 +6171,8 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, const virDomainDef *def, virQEMUCapsPtr qemuCaps) { + int backend; + /* forbid capabilities mode hostdev in this kind of hypervisor */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -6162,9 +6185,22 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: break; + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + backend = hostdev->source.subsys.u.pci.backend; + + if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VFIO PCI device assignment is not " + "supported by this version of qemu")); + return -1; + } + } + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: if (hostdev->info->bootIndex) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c index 986dfb77bd..c70a426c16 100644 --- a/tests/qemumemlocktest.c +++ b/tests/qemumemlocktest.c @@ -106,6 +106,18 @@ mymain(void) DO_TEST("pc-kvm", 0); DO_TEST("pc-tcg", 0); + if (!(qemuCaps = virQEMUCapsNew())) { + ret = -1; + goto cleanup; + } + + virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI); + + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, qemuCaps) < 0) { + ret = -1; + goto cleanup; + }; + DO_TEST("pc-hardlimit", 2147483648); DO_TEST("pc-locked", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); DO_TEST("pc-hostdev", 2147483648); @@ -116,10 +128,6 @@ mymain(void) DO_TEST("pc-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); qemuTestSetHostArch(&driver, VIR_ARCH_PPC64); - if (!(qemuCaps = virQEMUCapsNew())) { - ret = -1; - goto cleanup; - } virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, qemuCaps) < 0) { diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3c081651cf..8a5524c3fb 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1638,9 +1638,9 @@ mymain(void) QEMU_CAPS_DEVICE_VFIO_CCW); DO_TEST_CAPS_ARCH_LATEST("hostdev-subsys-mdev-vfio-ccw-boot", "s390x"); - DO_TEST_FAILURE("hostdev-subsys-mdev-vfio-ccw", - QEMU_CAPS_CCW, - QEMU_CAPS_CCW_CSSID_UNRESTRICTED); + DO_TEST_PARSE_ERROR("hostdev-subsys-mdev-vfio-ccw", + QEMU_CAPS_CCW, + QEMU_CAPS_CCW_CSSID_UNRESTRICTED); DO_TEST_PARSE_ERROR("hostdev-subsys-mdev-vfio-ccw-duplicate-address", QEMU_CAPS_CCW, QEMU_CAPS_CCW_CSSID_UNRESTRICTED, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0145970774..bd505ff157 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -405,7 +405,7 @@ mymain(void) DO_TEST("net-virtio-rxtxqueuesize", NONE); DO_TEST("net-hostdev", NONE); DO_TEST("net-hostdev-bootorder", NONE); - DO_TEST("net-hostdev-vfio", NONE); + DO_TEST("net-hostdev-vfio", QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("net-midonet", NONE); DO_TEST("net-openvswitch", NONE); DO_TEST("sound", NONE); @@ -437,9 +437,10 @@ mymain(void) DO_TEST("hostdev-usb-address", NONE); DO_TEST("hostdev-pci-address", NONE); - DO_TEST("hostdev-pci-multifunction", NONE); - DO_TEST("hostdev-vfio", NONE); + DO_TEST("hostdev-pci-multifunction", QEMU_CAPS_DEVICE_VFIO_PCI); + DO_TEST("hostdev-vfio", QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("hostdev-vfio-zpci", + QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI, QEMU_CAPS_CCW); DO_TEST("hostdev-vfio-zpci-multidomain-many", @@ -453,10 +454,11 @@ mymain(void) QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_ZPCI); - DO_TEST("hostdev-mdev-precreated", NONE); + DO_TEST("hostdev-mdev-precreated", QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("hostdev-mdev-display", QEMU_CAPS_DEVICE_QXL, - QEMU_CAPS_VFIO_PCI_DISPLAY); + QEMU_CAPS_VFIO_PCI_DISPLAY, + QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("pci-rom", NONE); DO_TEST("pci-rom-disabled", NONE); DO_TEST("pci-rom-disabled-invalid", NONE); -- 2.23.0
QEMU_CAPS_DEVICE_VMGENID is now being validated by qemuDomainDefValidate(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 11 ++--------- src/qemu/qemu_domain.c | 7 +++++++ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9431efa384..1bbd69b891 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6005,8 +6005,7 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd, static int qemuBuildVMGenIDCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + const virDomainDef *def) { g_auto(virBuffer) opts = VIR_BUFFER_INITIALIZER; char guid[VIR_UUID_STRING_BUFLEN]; @@ -6014,12 +6013,6 @@ qemuBuildVMGenIDCommandLine(virCommandPtr cmd, if (!def->genidRequested) return 0; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMGENID)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU does not support the 'genid' capability")); - return -1; - } - virUUIDFormat(def->genid, guid); virBufferAsprintf(&opts, "vmgenid,guid=%s,id=vmgenid0", guid); @@ -10022,7 +10015,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildSmbiosCommandLine(cmd, driver, def) < 0) return NULL; - if (qemuBuildVMGenIDCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildVMGenIDCommandLine(cmd, def) < 0) return NULL; /* diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b39942c49f..e3ffa4978b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5391,6 +5391,13 @@ qemuDomainDefValidate(const virDomainDef *def, } } + if (def->genidRequested && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMGENID)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU does not support the 'genid' capability")); + goto cleanup; + } + /* QEMU 2.7 (detected via the availability of query-hotpluggable-cpus) * enforces stricter rules than previous versions when it comes to guest * CPU topology. Verify known constraints are respected */ -- 2.23.0
Move QEMU caps validation of qemuBuildSgaCommandLine() to qemuDomainDefValidate(), allowing validation at domain define time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 18 +++--------------- src/qemu/qemu_domain.c | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1bbd69b891..fe5333efdf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6025,23 +6025,11 @@ qemuBuildVMGenIDCommandLine(virCommandPtr cmd, static int qemuBuildSgaCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + const virDomainDef *def) { /* Serial graphics adapter */ - if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGA)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu does not support SGA")); - return -1; - } - if (!def->nserials) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("need at least one serial port to use SGA")); - return -1; - } + if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) virCommandAddArgList(cmd, "-device", "sga", NULL); - } return 0; } @@ -10039,7 +10027,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, virCommandAddArg(cmd, "-no-user-config"); virCommandAddArg(cmd, "-nodefaults"); - if (qemuBuildSgaCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildSgaCommandLine(cmd, def) < 0) return NULL; if (qemuBuildMonitorCommandLine(logManager, secManager, cmd, cfg, def, priv) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e3ffa4978b..94bb4e4fca 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5398,6 +5398,20 @@ qemuDomainDefValidate(const virDomainDef *def, goto cleanup; } + /* Serial graphics adapter */ + if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGA)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu does not support SGA")); + goto cleanup; + } + if (!def->nserials) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("need at least one serial port to use SGA")); + goto cleanup; + } + } + /* QEMU 2.7 (detected via the availability of query-hotpluggable-cpus) * enforces stricter rules than previous versions when it comes to guest * CPU topology. Verify known constraints are respected */ -- 2.23.0
@def->clock validation is done by qemuBuildClockCommandLine() and qemuBuildClockArgStr(). This patch centralize the validation done in both these functions to a new qemuDomainDefValidateClockTimers() function. This new function is then called by qemuDomainDefValidate(), promoting clock 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 | 35 ++------------ src/qemu/qemu_domain.c | 100 ++++++++++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 106 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fe5333efdf..13abee9a42 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6111,9 +6111,6 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) case -1: /* unspecified - use hypervisor default */ break; case VIR_DOMAIN_TIMER_TRACK_BOOT: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported rtc timer track '%s'"), - virDomainTimerTrackTypeToString(def->timers[i]->track)); return NULL; case VIR_DOMAIN_TIMER_TRACK_GUEST: virBufferAddLit(&buf, ",clock=vm"); @@ -6135,9 +6132,6 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) break; case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported rtc timer tickpolicy '%s'"), - virDomainTimerTickpolicyTypeToString(def->timers[i]->tickpolicy)); return NULL; } break; /* no need to check other timers - there is only one rtc */ @@ -6172,9 +6166,8 @@ qemuBuildClockCommandLine(virCommandPtr cmd, for (i = 0; i < def->clock.ntimers; i++) { switch ((virDomainTimerNameType)def->clock.timers[i]->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported timer type (name) '%s'"), - virDomainTimerNameTypeToString(def->clock.timers[i]->name)); + /* qemuDomainDefValidateClockTimers will handle this + * error condition */ return -1; case VIR_DOMAIN_TIMER_NAME_TSC: @@ -6185,7 +6178,7 @@ qemuBuildClockCommandLine(virCommandPtr cmd, break; case VIR_DOMAIN_TIMER_NAME_RTC: - /* Already handled in qemuBuildClockArgStr */ + /* Already handled in qemuDomainDefValidateClockTimers */ break; case VIR_DOMAIN_TIMER_NAME_PIT: @@ -6199,15 +6192,8 @@ qemuBuildClockCommandLine(virCommandPtr cmd, "kvm-pit.lost_tick_policy=delay", NULL); break; case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) { - /* do nothing - this is default for kvm-pit */ - } else { - /* can't catchup if we don't have kvm-pit */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported pit tickpolicy '%s'"), - virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); - return -1; - } + /* Do nothing - qemuDomainDefValidateClockTimers handled + * the possible error condition here. */ break; case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) @@ -6216,9 +6202,6 @@ qemuBuildClockCommandLine(virCommandPtr cmd, break; case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: /* no way to support this mode for pit in qemu */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported pit tickpolicy '%s'"), - virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); return -1; } break; @@ -6234,14 +6217,6 @@ qemuBuildClockCommandLine(virCommandPtr cmd, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) { if (def->clock.timers[i]->present == 0) virCommandAddArg(cmd, "-no-hpet"); - } else { - /* no hpet timer available. The only possible action - is to raise an error if present="yes" */ - if (def->clock.timers[i]->present == 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("hpet timer is not supported")); - return -1; - } } break; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 94bb4e4fca..4715976b1b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5320,6 +5320,103 @@ qemuDomainDeviceDefValidateHub(virDomainHubDefPtr hub, } +static int +qemuDomainDefValidateClockTimers(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->clock.ntimers; i++) { + virDomainTimerDefPtr timer = def->clock.timers[i]; + + switch ((virDomainTimerNameType)timer->name) { + case VIR_DOMAIN_TIMER_NAME_PLATFORM: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported timer type (name) '%s'"), + virDomainTimerNameTypeToString(timer->name)); + return -1; + + case VIR_DOMAIN_TIMER_NAME_TSC: + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + case VIR_DOMAIN_TIMER_NAME_LAST: + break; + + case VIR_DOMAIN_TIMER_NAME_RTC: + switch (timer->track) { + case -1: /* unspecified - use hypervisor default */ + case VIR_DOMAIN_TIMER_TRACK_GUEST: + case VIR_DOMAIN_TIMER_TRACK_WALL: + break; + case VIR_DOMAIN_TIMER_TRACK_BOOT: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported rtc timer track '%s'"), + virDomainTimerTrackTypeToString(timer->track)); + return -1; + } + + switch (timer->tickpolicy) { + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + /* This is the default - missed ticks delivered when + next scheduled, at normal rate */ + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + /* deliver ticks at a faster rate until caught up */ + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported rtc timer tickpolicy '%s'"), + virDomainTimerTickpolicyTypeToString( + timer->tickpolicy)); + return -1; + } + break; + + case VIR_DOMAIN_TIMER_NAME_PIT: + switch (timer->tickpolicy) { + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) { + /* can't catchup if we don't have kvm-pit */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported pit tickpolicy '%s'"), + virDomainTimerTickpolicyTypeToString( + timer->tickpolicy)); + return -1; + } + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + /* no way to support this mode for pit in qemu */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported pit tickpolicy '%s'"), + virDomainTimerTickpolicyTypeToString( + timer->tickpolicy)); + return -1; + } + break; + + case VIR_DOMAIN_TIMER_NAME_HPET: + /* no hpet timer available. The only possible action + is to raise an error if present="yes" */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET) && + timer->present == 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("hpet timer is not supported")); + return -1; + } + break; + } + } + + return 0; +} + + static int qemuDomainDefValidate(const virDomainDef *def, void *opaque) @@ -5412,6 +5509,9 @@ qemuDomainDefValidate(const virDomainDef *def, } } + if (qemuDomainDefValidateClockTimers(def, qemuCaps) < 0) + goto cleanup; + /* QEMU 2.7 (detected via the availability of query-hotpluggable-cpus) * enforces stricter rules than previous versions when it comes to guest * CPU topology. Verify known constraints are respected */ diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index bd505ff157..0fa8582dd5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -266,7 +266,7 @@ mymain(void) DO_TEST("cpu-host-kvmclock", NONE); DO_TEST("cpu-host-passthrough-features", NONE); DO_TEST("cpu-host-model-features", NONE); - DO_TEST("clock-catchup", NONE); + DO_TEST("clock-catchup", QEMU_CAPS_KVM_PIT_TICK_POLICY); DO_TEST("kvmclock", NONE); DO_TEST("clock-timer-hyperv-rtc", NONE); -- 2.23.0
Move the PM validation being done by qemuBuildPMCommandLine() to to a new qemuDomainDefValidatePM() function. This new function is called by qemuDomainDefValidate(), promoting PM 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 | 14 ++------------ src/qemu/qemu_domain.c | 35 +++++++++++++++++++++++++++++++++++ tests/qemuhotplugtest.c | 2 ++ tests/qemuxml2argvtest.c | 2 +- tests/qemuxml2xmltest.c | 20 ++++++++++++++------ 5 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 13abee9a42..5c29ec897b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6253,13 +6253,8 @@ qemuBuildPMCommandLine(virCommandPtr cmd, const char *pm_object = "PIIX4_PM"; if (qemuDomainIsQ35(def) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S3)) { + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S3)) pm_object = "ICH9-LPC"; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("setting ACPI S3 not supported")); - return -1; - } virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "%s.disable_s3=%d", @@ -6270,13 +6265,8 @@ qemuBuildPMCommandLine(virCommandPtr cmd, const char *pm_object = "PIIX4_PM"; if (qemuDomainIsQ35(def) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S4)) { + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S4)) pm_object = "ICH9-LPC"; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("setting ACPI S4 not supported")); - return -1; - } virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "%s.disable_s4=%d", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4715976b1b..a87f283f19 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5417,6 +5417,38 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, } +static int +qemuDomainDefValidatePM(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + bool q35Dom = qemuDomainIsQ35(def); + + if (def->pm.s3) { + bool q35ICH9_S3 = q35Dom && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S3); + + if (!q35ICH9_S3 && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S3 not supported")); + return -1; + } + } + + if (def->pm.s4) { + bool q35ICH9_S4 = q35Dom && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S4); + + if (!q35ICH9_S4 && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S4 not supported")); + return -1; + } + } + + return 0; +} + + static int qemuDomainDefValidate(const virDomainDef *def, void *opaque) @@ -5512,6 +5544,9 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidateClockTimers(def, qemuCaps) < 0) goto cleanup; + if (qemuDomainDefValidatePM(def, qemuCaps) < 0) + goto cleanup; + /* QEMU 2.7 (detected via the availability of query-hotpluggable-cpus) * enforces stricter rules than previous versions when it comes to guest * CPU topology. Verify known constraints are respected */ diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 8e7273b673..0645b936d0 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -82,6 +82,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_QXL); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_VGA); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8a5524c3fb..6f6585ba71 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1279,7 +1279,7 @@ mymain(void) DO_TEST("misc-disable-s3", QEMU_CAPS_PIIX_DISABLE_S3); DO_TEST("misc-disable-suspends", QEMU_CAPS_PIIX_DISABLE_S3, QEMU_CAPS_PIIX_DISABLE_S4); DO_TEST("misc-enable-s4", QEMU_CAPS_PIIX_DISABLE_S4); - DO_TEST_FAILURE("misc-enable-s4", NONE); + DO_TEST_PARSE_ERROR("misc-enable-s4", NONE); DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", NONE); DO_TEST_PARSE_ERROR("vhost_queues-invalid", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0fa8582dd5..29d3a31e9f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -386,9 +386,11 @@ mymain(void) DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); DO_TEST("misc-acpi", NONE); - DO_TEST("misc-disable-s3", NONE); - DO_TEST("misc-disable-suspends", NONE); - DO_TEST("misc-enable-s4", NONE); + DO_TEST("misc-disable-s3", QEMU_CAPS_PIIX_DISABLE_S3); + DO_TEST("misc-disable-suspends", + QEMU_CAPS_PIIX_DISABLE_S3, + QEMU_CAPS_PIIX_DISABLE_S4); + DO_TEST("misc-enable-s4", QEMU_CAPS_PIIX_DISABLE_S4); DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", NONE); DO_TEST("net-vhostuser", NONE); @@ -499,7 +501,9 @@ mymain(void) DO_TEST("vhost_queues", NONE); DO_TEST("interface-driver", NONE); DO_TEST("interface-server", QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_OBJECT_MEMORY_FILE); + QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_PIIX_DISABLE_S3, + QEMU_CAPS_PIIX_DISABLE_S4); DO_TEST("virtio-lun", NONE); DO_TEST("usb-none", NONE); @@ -530,7 +534,9 @@ mymain(void) DO_TEST("blkdeviotune-max", NONE); DO_TEST("blkdeviotune-group-num", NONE); DO_TEST("blkdeviotune-max-length", NONE); - DO_TEST("controller-usb-order", NONE); + DO_TEST("controller-usb-order", + QEMU_CAPS_PIIX_DISABLE_S3, + QEMU_CAPS_PIIX_DISABLE_S4); DO_TEST_FULL("seclabel-dynamic-baselabel", WHEN_INACTIVE, ARG_QEMU_CAPS, NONE); @@ -1279,7 +1285,9 @@ mymain(void) DO_TEST("user-aliases", QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_QCOW2_LUKS, - QEMU_CAPS_OBJECT_MEMORY_FILE); + QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_PIIX_DISABLE_S3, + QEMU_CAPS_PIIX_DISABLE_S4); DO_TEST("input-virtio-ccw", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_KEYBOARD, -- 2.23.0
Move the boot validation being done by qemuBuildBootCommandLine() to to a new qemuDomainDefValidateBoot() function. This new function is called by qemuDomainDefValidate(), allowing boot 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 | 17 +---------------- src/qemu/qemu_domain.c | 29 +++++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++-- tests/qemuxml2xmltest.c | 6 +++--- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5c29ec897b..7d4272a896 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6293,28 +6293,13 @@ qemuBuildBootCommandLine(virCommandPtr cmd, } if (def->os.bios.rt_set) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reboot timeout is not supported " - "by this QEMU binary")); - return -1; - } - virBufferAsprintf(&boot_buf, "reboot-timeout=%d,", def->os.bios.rt_delay); } - if (def->os.bm_timeout_set) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("splash timeout is not supported " - "by this QEMU binary")); - return -1; - } - + if (def->os.bm_timeout_set) virBufferAsprintf(&boot_buf, "splash-time=%u,", def->os.bm_timeout); - } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) virBufferAddLit(&boot_buf, "strict=on,"); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a87f283f19..0c04ba9dcc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5449,6 +5449,32 @@ qemuDomainDefValidatePM(const virDomainDef *def, } +static int +qemuDomainDefValidateBoot(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + if (def->os.bios.rt_set) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reboot timeout is not supported " + "by this QEMU binary")); + return -1; + } + } + + if (def->os.bm_timeout_set) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("splash timeout is not supported " + "by this QEMU binary")); + return -1; + } + } + + return 0; +} + + static int qemuDomainDefValidate(const virDomainDef *def, void *opaque) @@ -5547,6 +5573,9 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidatePM(def, qemuCaps) < 0) goto cleanup; + if (qemuDomainDefValidateBoot(def, qemuCaps) < 0) + goto cleanup; + /* QEMU 2.7 (detected via the availability of query-hotpluggable-cpus) * enforces stricter rules than previous versions when it comes to guest * CPU topology. Verify known constraints are respected */ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6f6585ba71..802ecc98f8 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -844,7 +844,7 @@ mymain(void) DO_TEST("boot-menu-enable", NONE); DO_TEST("boot-menu-enable-with-timeout", QEMU_CAPS_SPLASH_TIMEOUT); - DO_TEST_FAILURE("boot-menu-enable-with-timeout", NONE); + DO_TEST_PARSE_ERROR("boot-menu-enable-with-timeout", NONE); DO_TEST_PARSE_ERROR("boot-menu-enable-with-timeout-invalid", NONE); DO_TEST("boot-menu-disable", NONE); DO_TEST("boot-menu-disable-drive", NONE); @@ -860,7 +860,7 @@ mymain(void) DO_TEST("reboot-timeout-disabled", QEMU_CAPS_REBOOT_TIMEOUT); DO_TEST("reboot-timeout-enabled", QEMU_CAPS_REBOOT_TIMEOUT); - DO_TEST_FAILURE("reboot-timeout-enabled", NONE); + DO_TEST_PARSE_ERROR("reboot-timeout-enabled", NONE); DO_TEST("bios", QEMU_CAPS_DEVICE_ISA_SERIAL, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 29d3a31e9f..ee76d50a41 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -251,13 +251,13 @@ mymain(void) QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI); DO_TEST("boot-multi", NONE); - DO_TEST("boot-menu-enable-with-timeout", NONE); + DO_TEST("boot-menu-enable-with-timeout", QEMU_CAPS_SPLASH_TIMEOUT); DO_TEST("boot-menu-disable", NONE); DO_TEST("boot-menu-disable-with-timeout", NONE); DO_TEST("boot-order", NONE); - DO_TEST("reboot-timeout-enabled", NONE); - DO_TEST("reboot-timeout-disabled", NONE); + DO_TEST("reboot-timeout-enabled", QEMU_CAPS_REBOOT_TIMEOUT); + DO_TEST("reboot-timeout-disabled", QEMU_CAPS_REBOOT_TIMEOUT); DO_TEST("clock-utc", NONE); DO_TEST("clock-localtime", NONE); -- 2.23.0
Move the pcihole64 validation being done by qemuBuildGlobalControllerCommandLine() to the existing function qemuDomainDeviceDefValidateControllerPCI(), which provides domain define time validation. The existing pcihole64 validations in qemu_domain.c were replaced by the ones moved from qemu_command.c. The reason is that they are more specific, allowing VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT and VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT to have distinct validation, with exclusive QEMU caps and machine types. 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 | 37 +++++++++++++++++++++++++++++++------ tests/qemuxml2argvtest.c | 2 +- tests/qemuxml2xmltest.c | 6 +++--- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7d4272a896..31b8784070 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6392,8 +6392,7 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, static int qemuBuildGlobalControllerCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + const virDomainDef *def) { size_t i; @@ -6402,20 +6401,14 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd, if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && cont->opts.pciopts.pcihole64) { const char *hoststr = NULL; - bool cap = false; - bool machine = false; switch (cont->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: hoststr = "i440FX-pcihost"; - cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); - machine = qemuDomainIsI440FX(def); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: hoststr = "q35-pcihost"; - cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); - machine = qemuDomainIsQ35(def); break; default: @@ -6425,19 +6418,6 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd, return -1; } - if (!machine) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Setting the 64-bit PCI hole size is not " - "supported for machine '%s'"), def->os.machine); - return -1; - } - if (!cap) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("64-bit PCI hole size setting is not supported " - "with this QEMU binary")); - return -1; - } - virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%luK", hoststr, cont->opts.pciopts.pcihole64size); @@ -9995,7 +9975,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildIOMMUCommandLine(cmd, def, qemuCaps) < 0) return NULL; - if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildGlobalControllerCommandLine(cmd, def) < 0) return NULL; if (qemuBuildControllersCommandLine(cmd, def, qemuCaps) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c04ba9dcc..7f6daaf276 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7188,13 +7188,38 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *cont, /* pcihole64 */ switch ((virDomainControllerModelPCI) cont->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + if (pciopts->pcihole64 || pciopts->pcihole64size != 0) { + if (!qemuDomainIsI440FX(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Setting the 64-bit PCI hole size is not " + "supported for machine '%s'"), def->os.machine); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("64-bit PCI hole size setting is not supported " + "with this QEMU binary")); + return -1; + } + } + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: - /* The pcihole64 option only applies to x86 guests */ - if ((pciopts->pcihole64 || - pciopts->pcihole64size != 0) && - !ARCH_IS_X86(def->os.arch)) { - virReportControllerInvalidOption(cont, model, modelName, "pcihole64"); - return -1; + if (pciopts->pcihole64 || pciopts->pcihole64size != 0) { + if (!qemuDomainIsQ35(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Setting the 64-bit PCI hole size is not " + "supported for machine '%s'"), def->os.machine); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("64-bit PCI hole size setting is not supported " + "with this QEMU binary")); + return -1; + } } break; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 802ecc98f8..3a002bb393 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2526,7 +2526,7 @@ mymain(void) QEMU_CAPS_KVM, QEMU_CAPS_VIRTIO_SCSI); DO_TEST("pcihole64", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); - DO_TEST_FAILURE("pcihole64-none", NONE); + DO_TEST_PARSE_ERROR("pcihole64-none", NONE); DO_TEST("pcihole64-q35", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ee76d50a41..0eb523fd43 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -996,9 +996,9 @@ mymain(void) DO_TEST("s390-serial-console", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390); - DO_TEST("pcihole64", NONE); - DO_TEST("pcihole64-gib", NONE); - DO_TEST("pcihole64-none", NONE); + DO_TEST("pcihole64", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); + DO_TEST("pcihole64-gib", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); + DO_TEST("pcihole64-none", QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); DO_TEST("pcihole64-q35", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI, -- 2.23.0
There are validations for SDL, VNC, SPICE and EGL_HEADLESS around several BuildGraphics*CommandLine in qemu_command.c. This patch starts to move all of them to qemu_domain.c, inside the existent qemuDomainDeviceDefValidateGraphics() function. This function is called by qemuDomainDefValidate(), validating the graphics parameters in domain define time. In this patch we'll move the SDL validation code from qemuBuildGraphicsSDLCommandLine(). Tests were adapted to consider SDL validation in this earlier stage. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 14 +------------- src/qemu/qemu_domain.c | 29 +++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 31b8784070..70b80fad63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7380,19 +7380,10 @@ qemuBuildGraphicsSDLCommandLine(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED, virCommandAddArg(cmd, "-display"); virBufferAddLit(&opt, "sdl"); - if (graphics->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL_GL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("OpenGL for SDL is not supported with this QEMU " - "binary")); - return -1; - } - + if (graphics->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) virBufferAsprintf(&opt, ",gl=%s", virTristateSwitchTypeToString(graphics->data.sdl.gl)); - } - virCommandAddArgBuffer(cmd, &opt); return 0; @@ -7848,9 +7839,6 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(graphics->type)); return -1; case VIR_DOMAIN_GRAPHICS_TYPE_LAST: default: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7f6daaf276..b33e150690 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7614,6 +7614,35 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, } } + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + if (graphics->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL_GL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("OpenGL for SDL is not supported with this QEMU " + "binary")); + return -1; + } + } + break; + + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + break; + case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + break; + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported graphics type '%s'"), + virDomainGraphicsTypeToString(graphics->type)); + return -1; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + default: + return -1; + } + return 0; } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0eb523fd43..052c7034e1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1179,7 +1179,8 @@ mymain(void) QEMU_CAPS_VIRTIO_GPU_VIRGL); DO_TEST("video-virtio-gpu-sdl-gl", QEMU_CAPS_DEVICE_VIRTIO_GPU, - QEMU_CAPS_VIRTIO_GPU_VIRGL); + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_SDL_GL); DO_TEST("virtio-input", QEMU_CAPS_VIRTIO_KEYBOARD, -- 2.23.0
Move the VNC cap validation from qemuBuildGraphicsVNCCommandLine() to qemuDomainDeviceDefValidateGraphics(). This function is called by qemuDomainDefValidate(), validating the graphics parameters in domain define time. Tests were adapted to consider SDL validation in this earlier stage. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 6 ---- src/qemu/qemu_domain.c | 7 ++++- tests/qemuhotplugtest.c | 1 + tests/qemuxml2xmltest.c | 63 +++++++++++++++++++++++++++++------------ 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 70b80fad63..7661527d39 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7400,12 +7400,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virDomainGraphicsListenDefPtr glisten = NULL; bool escapeAddr; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc graphics are not supported with this QEMU")); - return -1; - } - if (!(glisten = virDomainGraphicsGetListen(graphics, 0))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing listen element")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b33e150690..c25adbc4f3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7627,9 +7627,14 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, break; case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc graphics are not supported with this QEMU")); + return -1; + } break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 0645b936d0..9646a30fb6 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -84,6 +84,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VNC); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 052c7034e1..f22bc0c9e6 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -341,20 +341,42 @@ mymain(void) DO_TEST("disk-mirror-old", NONE); DO_TEST("disk-mirror", NONE); DO_TEST("disk-active-commit", NONE); - DO_TEST("graphics-listen-network", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-vnc", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-vnc-websocket", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-vnc-sasl", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-vnc-tls", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-vnc-no-listen-attr", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-vnc-remove-generated-socket", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("graphics-listen-network", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); + DO_TEST("graphics-vnc", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-websocket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-sasl", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-tls", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-no-listen-attr", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-remove-generated-socket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); cfg->vncAutoUnixSocket = true; - DO_TEST("graphics-vnc-auto-socket-cfg", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("graphics-vnc-auto-socket-cfg", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); cfg->vncAutoUnixSocket = false; - DO_TEST("graphics-vnc-socket", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-vnc-auto-socket", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("graphics-vnc-socket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); + DO_TEST("graphics-vnc-auto-socket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); DO_TEST("graphics-vnc-egl-headless", - QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_EGL_HEADLESS); + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_EGL_HEADLESS, + QEMU_CAPS_VNC); DO_TEST_CAPS_ARCH_LATEST("default-video-type-aarch64", "aarch64"); DO_TEST_CAPS_ARCH_LATEST("default-video-type-ppc64", "ppc64"); @@ -418,8 +440,8 @@ mymain(void) QEMU_CAPS_HDA_DUPLEX, QEMU_CAPS_HDA_OUTPUT); DO_TEST("watchdog", NONE); - DO_TEST("net-bandwidth", QEMU_CAPS_DEVICE_VGA); - DO_TEST("net-bandwidth2", QEMU_CAPS_DEVICE_VGA); + DO_TEST("net-bandwidth", QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_VNC); + DO_TEST("net-bandwidth2", QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_VNC); DO_TEST("net-mtu", NONE); DO_TEST("net-coalesce", NONE); DO_TEST("net-many-models", NONE); @@ -460,7 +482,8 @@ mymain(void) DO_TEST("hostdev-mdev-display", QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_VFIO_PCI_DISPLAY, - QEMU_CAPS_DEVICE_VFIO_PCI); + QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_VNC); DO_TEST("pci-rom", NONE); DO_TEST("pci-rom-disabled", NONE); DO_TEST("pci-rom-disabled-invalid", NONE); @@ -503,7 +526,8 @@ mymain(void) DO_TEST("interface-server", QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_PIIX_DISABLE_S3, - QEMU_CAPS_PIIX_DISABLE_S4); + QEMU_CAPS_PIIX_DISABLE_S4, + QEMU_CAPS_VNC); DO_TEST("virtio-lun", NONE); DO_TEST("usb-none", NONE); @@ -671,7 +695,9 @@ mymain(void) QEMU_CAPS_SCSI_LSI); DO_TEST("console-virtio", NONE); DO_TEST("serial-target-port-auto", NONE); - DO_TEST("graphics-listen-network2", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("graphics-listen-network2", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_VNC); DO_TEST("graphics-spice-timeout", QEMU_CAPS_DEVICE_VGA); DO_TEST("numad-auto-vcpu-no-numatune", NONE); DO_TEST("numad-auto-memory-vcpu-no-cpuset-and-placement", NONE); @@ -1251,7 +1277,7 @@ mymain(void) QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW); - DO_TEST("video-none-device", NONE); + DO_TEST("video-none-device", QEMU_CAPS_VNC); DO_TEST_CAPS_LATEST("intel-iommu"); DO_TEST_CAPS_VER("intel-iommu", "2.6.0"); @@ -1288,7 +1314,8 @@ mymain(void) QEMU_CAPS_QCOW2_LUKS, QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_PIIX_DISABLE_S3, - QEMU_CAPS_PIIX_DISABLE_S4); + QEMU_CAPS_PIIX_DISABLE_S4, + QEMU_CAPS_VNC); DO_TEST("input-virtio-ccw", QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_KEYBOARD, -- 2.23.0
Move the SPICE caps validation from qemuBuildGraphicsSPICECommandLine() to a new function called qemuDomainDeviceDefValidateSPICEGraphics(). This function is called by qemuDomainDeviceDefValidateGraphics(), which in turn is called by qemuDomainDefValidate(), validating the graphics parameters in domain define time. This validation move exposed a flaw in the 'default-video-type' tests for PPC64, AARCH64 and s390 archs. The XML was considering 'spice' as the default video type, which isn't true for those architectures. This was flying under the radar until now because the SPICE validation was being made in 'virsh start' time, while the XML validation done in qemuxml2xmltest.c considers define time. All other tests were adapted to consider SPICE validation in this earlier stage. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 45 +--------- src/qemu/qemu_domain.c | 83 ++++++++++++++++++- tests/qemuhotplugtest.c | 7 ++ .../default-video-type-aarch64.xml | 2 +- .../default-video-type-ppc64.xml | 2 +- .../default-video-type-s390x.xml | 2 +- ...ault-video-type-aarch64.aarch64-latest.xml | 4 +- .../default-video-type-ppc64.ppc64-latest.xml | 4 +- .../default-video-type-s390x.s390x-latest.xml | 4 +- tests/qemuxml2xmltest.c | 62 +++++++++++--- 10 files changed, 150 insertions(+), 65 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7661527d39..aceb42a289 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7520,7 +7520,6 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, static int qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, virDomainGraphicsDefPtr graphics) { g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; @@ -7531,12 +7530,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, bool hasSecure = false; bool hasInsecure = false; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice graphics are not supported with this QEMU")); - return -1; - } - if (!(glisten = virDomainGraphicsGetListen(graphics, 0))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing listen element")); @@ -7545,13 +7538,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, switch (glisten->type) { case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_UNIX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unix socket for spice graphics are not supported " - "with this QEMU")); - return -1; - } - virBufferAddLit(&opt, "unix,addr="); virQEMUBuildBufferEscapeComma(&opt, glisten->socket); virBufferAddLit(&opt, ","); @@ -7566,12 +7552,6 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, } if (tlsPort > 0) { - if (!cfg->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice TLS port set in XML configuration, " - "but TLS is disabled in qemu.conf")); - return -1; - } virBufferAsprintf(&opt, "tls-port=%u,", tlsPort); hasSecure = true; } @@ -7710,35 +7690,16 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, if (graphics->data.spice.copypaste == VIR_TRISTATE_BOOL_NO) virBufferAddLit(&opt, "disable-copy-paste,"); - if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU can't disable file transfers through spice")); - return -1; - } else { - virBufferAddLit(&opt, "disable-agent-file-xfer,"); - } - } + if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO) + virBufferAddLit(&opt, "disable-agent-file-xfer,"); if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support spice OpenGL")); - return -1; - } - /* spice.gl is a TristateBool, but qemu expects on/off: use * TristateSwitch helper */ virBufferAsprintf(&opt, "gl=%s,", virTristateSwitchTypeToString(graphics->data.spice.gl)); if (graphics->data.spice.rendernode) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU doesn't support spice OpenGL rendernode")); - return -1; - } - virBufferAddLit(&opt, "rendernode="); virQEMUBuildBufferEscapeComma(&opt, graphics->data.spice.rendernode); virBufferAddLit(&opt, ","); @@ -7821,7 +7782,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: if (qemuBuildGraphicsSPICECommandLine(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 c25adbc4f3..9f29d2afbe 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7566,9 +7566,84 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm, } +static int +qemuDomainDeviceDefValidateSPICEGraphics(const virDomainGraphicsDef *graphics, + virQEMUDriverPtr driver, + virQEMUCapsPtr qemuCaps) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virDomainGraphicsListenDefPtr glisten = NULL; + int tlsPort = graphics->data.spice.tlsPort; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice graphics are not supported with this QEMU")); + return -1; + } + + glisten = virDomainGraphicsGetListen((virDomainGraphicsDefPtr)graphics, 0); + if (!glisten) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing listen element")); + return -1; + } + + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_UNIX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unix socket for spice graphics are not supported " + "with this QEMU")); + return -1; + } + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + if (tlsPort > 0 && !cfg->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration, " + "but TLS is disabled in qemu.conf")); + return -1; + } + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } + + if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU can't disable file transfers through spice")); + return -1; + } + + if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support spice OpenGL")); + return -1; + } + + if (graphics->data.spice.rendernode && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support spice OpenGL rendernode")); + return -1; + } + } + + return 0; +} + + static int qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, const virDomainDef *def, + virQEMUDriverPtr driver, virQEMUCapsPtr qemuCaps) { bool have_egl_headless = false; @@ -7635,6 +7710,12 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + if (qemuDomainDeviceDefValidateSPICEGraphics(graphics, driver, + qemuCaps) < 0) + return -1; + + break; + case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: @@ -8042,7 +8123,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_GRAPHICS: ret = qemuDomainDeviceDefValidateGraphics(dev->data.graphics, def, - qemuCaps); + driver, qemuCaps); break; case VIR_DOMAIN_DEVICE_INPUT: diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9646a30fb6..ea8fcd92d2 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -85,6 +85,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt, virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4); virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_VNC); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE); + virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_SPICE_FILE_XFER_DISABLE); if (qemuTestCapsCacheInsert(driver.qemuCapsCache, priv->qemuCaps) < 0) return -1; @@ -592,6 +594,7 @@ mymain(void) struct qemuHotplugTestData data = {0}; struct testQemuHotplugCpuParams cpudata; g_autofree char *fakerootdir = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE); @@ -610,6 +613,8 @@ mymain(void) if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; + cfg = virQEMUDriverGetConfig(&driver); + virEventRegisterDefaultImpl(); VIR_FREE(driver.config->spiceListen); @@ -676,6 +681,7 @@ mymain(void) " }" \ "}\r\n" + cfg->spiceTLS = true; DO_TEST_UPDATE("graphics-spice", "graphics-spice-nochange", false, false, NULL); DO_TEST_UPDATE("graphics-spice-timeout", "graphics-spice-timeout-nochange", false, false, "set_password", QMP_OK, "expire_password", QMP_OK); @@ -684,6 +690,7 @@ mymain(void) DO_TEST_UPDATE("graphics-spice", "graphics-spice-listen", true, false, NULL); DO_TEST_UPDATE("graphics-spice-listen-network", "graphics-spice-listen-network-password", false, false, "set_password", QMP_OK, "expire_password", QMP_OK); + cfg->spiceTLS = false; /* Strange huh? Currently, only graphics can be updated :-P */ DO_TEST_UPDATE("disk-cdrom", "disk-cdrom-nochange", true, false, NULL); diff --git a/tests/qemuxml2argvdata/default-video-type-aarch64.xml b/tests/qemuxml2argvdata/default-video-type-aarch64.xml index 03326d3c9b..f7d2d5d94a 100644 --- a/tests/qemuxml2argvdata/default-video-type-aarch64.xml +++ b/tests/qemuxml2argvdata/default-video-type-aarch64.xml @@ -11,6 +11,6 @@ <emulator>/usr/bin/qemu-system-aarch64</emulator> <controller type='usb' index='0' model='none'/> <memballoon model='none'/> - <graphics type='spice'/> + <graphics type='vnc'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/default-video-type-ppc64.xml b/tests/qemuxml2argvdata/default-video-type-ppc64.xml index 739e07fc19..ea5b966cfd 100644 --- a/tests/qemuxml2argvdata/default-video-type-ppc64.xml +++ b/tests/qemuxml2argvdata/default-video-type-ppc64.xml @@ -11,6 +11,6 @@ <emulator>/usr/bin/qemu-system-ppc64</emulator> <controller type='usb' index='0' model='none'/> <memballoon model='none'/> - <graphics type='spice'/> + <graphics type='vnc'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/default-video-type-s390x.xml b/tests/qemuxml2argvdata/default-video-type-s390x.xml index 9eda06a3a1..fe402d2c7f 100644 --- a/tests/qemuxml2argvdata/default-video-type-s390x.xml +++ b/tests/qemuxml2argvdata/default-video-type-s390x.xml @@ -11,6 +11,6 @@ <emulator>/usr/bin/qemu-system-s390x</emulator> <controller type='usb' index='0' model='none'/> <memballoon model='none'/> - <graphics type='spice'/> + <graphics type='vnc'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml b/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml index 4b660b8d70..1efea62f6f 100644 --- a/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml +++ b/tests/qemuxml2xmloutdata/default-video-type-aarch64.aarch64-latest.xml @@ -30,8 +30,8 @@ <target chassis='2' port='0x9'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> - <graphics type='spice'> - <listen type='none'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> </graphics> <video> <model type='virtio' heads='1' primary='yes'/> diff --git a/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml b/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml index 590a73b456..6c4bd5ef8b 100644 --- a/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml +++ b/tests/qemuxml2xmloutdata/default-video-type-ppc64.ppc64-latest.xml @@ -19,8 +19,8 @@ <controller type='pci' index='0' model='pci-root'/> <input type='keyboard' bus='usb'/> <input type='mouse' bus='usb'/> - <graphics type='spice'> - <listen type='none'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> </graphics> <video> <model type='vga' vram='16384' heads='1' primary='yes'/> diff --git a/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml b/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml index 21718db1ca..d4ccf82712 100644 --- a/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml +++ b/tests/qemuxml2xmloutdata/default-video-type-s390x.s390x-latest.xml @@ -17,8 +17,8 @@ <emulator>/usr/bin/qemu-system-s390x</emulator> <controller type='usb' index='0' model='none'/> <controller type='pci' index='0' model='pci-root'/> - <graphics type='spice'> - <listen type='none'/> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> </graphics> <video> <model type='virtio' heads='1' primary='yes'/> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f22bc0c9e6..d3ef08dcdf 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -382,22 +382,45 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("default-video-type-ppc64", "ppc64"); DO_TEST_CAPS_ARCH_LATEST("default-video-type-riscv64", "riscv64"); DO_TEST_CAPS_ARCH_LATEST("default-video-type-s390x", "s390x"); - DO_TEST("default-video-type-x86_64-caps-test-0", QEMU_CAPS_DEVICE_VGA); - DO_TEST("default-video-type-x86_64-caps-test-1", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("default-video-type-x86_64-caps-test-0", + QEMU_CAPS_DEVICE_VGA, + QEMU_CAPS_SPICE); + DO_TEST("default-video-type-x86_64-caps-test-1", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE); DO_TEST("graphics-sdl", QEMU_CAPS_DEVICE_VGA); DO_TEST("graphics-sdl-fullscreen", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-spice", QEMU_CAPS_DEVICE_QXL); - DO_TEST("graphics-spice-compression", QEMU_CAPS_DEVICE_QXL); - DO_TEST("graphics-spice-qxl-vga", QEMU_CAPS_DEVICE_QXL); - DO_TEST("graphics-spice-socket", QEMU_CAPS_DEVICE_CIRRUS_VGA); - DO_TEST("graphics-spice-auto-socket", QEMU_CAPS_DEVICE_CIRRUS_VGA); + + cfg->spiceTLS = true; + DO_TEST("graphics-spice", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_FILE_XFER_DISABLE); + DO_TEST("graphics-spice-compression", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE); + DO_TEST("graphics-spice-qxl-vga", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE); + DO_TEST("graphics-spice-socket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_UNIX); + DO_TEST("graphics-spice-auto-socket", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_UNIX); cfg->spiceAutoUnixSocket = true; - DO_TEST("graphics-spice-auto-socket-cfg", QEMU_CAPS_DEVICE_CIRRUS_VGA); + DO_TEST("graphics-spice-auto-socket-cfg", + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_SPICE); cfg->spiceAutoUnixSocket = false; + cfg->spiceTLS = false; DO_TEST("graphics-spice-egl-headless", QEMU_CAPS_DEVICE_QXL, - QEMU_CAPS_EGL_HEADLESS); + QEMU_CAPS_EGL_HEADLESS, + QEMU_CAPS_SPICE); DO_TEST("graphics-egl-headless-rendernode", QEMU_CAPS_DEVICE_CIRRUS_VGA, @@ -448,7 +471,13 @@ mymain(void) DO_TEST("serial-tcp-tlsx509-chardev", NONE); DO_TEST("serial-tcp-tlsx509-chardev-notls", NONE); - DO_TEST("serial-spiceport", QEMU_CAPS_DEVICE_QXL); + + cfg->spiceTLS = true; + DO_TEST("serial-spiceport", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_SPICE); + cfg->spiceTLS = false; + DO_TEST("serial-spiceport-nospice", NONE); DO_TEST("console-compat", NONE); DO_TEST("console-compat2", NONE); @@ -578,7 +607,8 @@ mymain(void) DO_TEST("seclabel-device-multiple", NONE); DO_TEST_FULL("seclabel-dynamic-none-relabel", WHEN_INACTIVE, ARG_QEMU_CAPS, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_OBJECT_MEMORY_FILE, NONE); + QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_SPICE, NONE); DO_TEST("numad-static-vcpu-no-numatune", NONE); DO_TEST("disk-scsi-lun-passthrough-sgio", @@ -698,7 +728,9 @@ mymain(void) DO_TEST("graphics-listen-network2", QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_VNC); - DO_TEST("graphics-spice-timeout", QEMU_CAPS_DEVICE_VGA); + DO_TEST("graphics-spice-timeout", + QEMU_CAPS_DEVICE_VGA, + QEMU_CAPS_SPICE); DO_TEST("numad-auto-vcpu-no-numatune", NONE); DO_TEST("numad-auto-memory-vcpu-no-cpuset-and-placement", NONE); DO_TEST("numad-auto-memory-vcpu-cpuset", NONE); @@ -1202,7 +1234,11 @@ mymain(void) QEMU_CAPS_VIRTIO_GPU_VIRGL); DO_TEST("video-virtio-gpu-spice-gl", QEMU_CAPS_DEVICE_VIRTIO_GPU, - QEMU_CAPS_VIRTIO_GPU_VIRGL); + QEMU_CAPS_VIRTIO_GPU_VIRGL, + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_FILE_XFER_DISABLE, + QEMU_CAPS_SPICE_GL, + QEMU_CAPS_SPICE_RENDERNODE); DO_TEST("video-virtio-gpu-sdl-gl", QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_VIRTIO_GPU_VIRGL, -- 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. Tests were adapted to consider validation in this earlier stage. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 10 +--------- src/qemu/qemu_domain.c | 7 +++++++ tests/qemuxml2argvdata/graphics-egl-headless.args | 2 +- .../qemuxml2argvdata/graphics-spice-egl-headless.args | 2 +- tests/qemuxml2argvdata/graphics-vnc-egl-headless.args | 2 +- tests/qemuxml2argvtest.c | 9 ++++++--- tests/qemuxml2xmltest.c | 6 ++++-- 7 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aceb42a289..efc70d6de9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7729,7 +7729,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; @@ -7737,13 +7736,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); @@ -7788,7 +7780,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 9f29d2afbe..415f0916a2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7717,6 +7717,13 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, break; case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + 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; + } + break; case VIR_DOMAIN_GRAPHICS_TYPE_RDP: case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: diff --git a/tests/qemuxml2argvdata/graphics-egl-headless.args b/tests/qemuxml2argvdata/graphics-egl-headless.args index 76d7583462..04b09803de 100644 --- a/tests/qemuxml2argvdata/graphics-egl-headless.args +++ b/tests/qemuxml2argvdata/graphics-egl-headless.args @@ -25,5 +25,5 @@ server,nowait \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ --display egl-headless \ +-display egl-headless,rendernode=/dev/dri/foo \ -vga cirrus diff --git a/tests/qemuxml2argvdata/graphics-spice-egl-headless.args b/tests/qemuxml2argvdata/graphics-spice-egl-headless.args index 54b7184376..159b6e15fb 100644 --- a/tests/qemuxml2argvdata/graphics-spice-egl-headless.args +++ b/tests/qemuxml2argvdata/graphics-spice-egl-headless.args @@ -27,7 +27,7 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -spice port=5903,addr=127.0.0.1,seamless-migration=on \ --display egl-headless \ +-display egl-headless,rendernode=/dev/dri/foo \ -vga qxl \ -global qxl-vga.ram_size=67108864 \ -global qxl-vga.vram_size=33554432 \ diff --git a/tests/qemuxml2argvdata/graphics-vnc-egl-headless.args b/tests/qemuxml2argvdata/graphics-vnc-egl-headless.args index 3c0901d1a5..5c01972ef8 100644 --- a/tests/qemuxml2argvdata/graphics-vnc-egl-headless.args +++ b/tests/qemuxml2argvdata/graphics-vnc-egl-headless.args @@ -27,5 +27,5 @@ server,nowait \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -vnc '[2001:1:2:3:4:5:1234:1234]:3' \ --display egl-headless \ +-display egl-headless,rendernode=/dev/dri/foo \ -vga cirrus diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3a002bb393..0af6083750 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1163,7 +1163,8 @@ mymain(void) DO_TEST("graphics-egl-headless", QEMU_CAPS_EGL_HEADLESS, - QEMU_CAPS_DEVICE_CIRRUS_VGA); + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_EGL_HEADLESS_RENDERNODE); DO_TEST_CAPS_LATEST("graphics-egl-headless"); DO_TEST_CAPS_LATEST("graphics-egl-headless-rendernode"); @@ -1207,7 +1208,8 @@ mymain(void) DO_TEST("graphics-vnc-egl-headless", QEMU_CAPS_VNC, QEMU_CAPS_EGL_HEADLESS, - QEMU_CAPS_DEVICE_CIRRUS_VGA); + QEMU_CAPS_DEVICE_CIRRUS_VGA, + QEMU_CAPS_EGL_HEADLESS_RENDERNODE); DO_TEST("graphics-sdl", QEMU_CAPS_DEVICE_VGA); @@ -1269,7 +1271,8 @@ mymain(void) DO_TEST("graphics-spice-egl-headless", QEMU_CAPS_SPICE, QEMU_CAPS_EGL_HEADLESS, - QEMU_CAPS_DEVICE_QXL); + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_EGL_HEADLESS_RENDERNODE); DO_TEST_CAPS_LATEST_PARSE_ERROR("graphics-spice-invalid-egl-headless"); DO_TEST_CAPS_LATEST("graphics-spice-gl-auto-rendernode"); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d3ef08dcdf..b449c21f18 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -376,7 +376,8 @@ mymain(void) DO_TEST("graphics-vnc-egl-headless", QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_EGL_HEADLESS, - QEMU_CAPS_VNC); + QEMU_CAPS_VNC, + QEMU_CAPS_EGL_HEADLESS_RENDERNODE); DO_TEST_CAPS_ARCH_LATEST("default-video-type-aarch64", "aarch64"); DO_TEST_CAPS_ARCH_LATEST("default-video-type-ppc64", "ppc64"); @@ -420,7 +421,8 @@ mymain(void) DO_TEST("graphics-spice-egl-headless", QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_EGL_HEADLESS, - QEMU_CAPS_SPICE); + QEMU_CAPS_SPICE, + QEMU_CAPS_EGL_HEADLESS_RENDERNODE); DO_TEST("graphics-egl-headless-rendernode", QEMU_CAPS_DEVICE_CIRRUS_VGA, -- 2.23.0
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
Move EGL Headless validation from qemuBuildGraphicsEGLHeadlessCommandLine() to qemuDomainDeviceDefValidateGraphics(). This function is called by qemuDomainDefValidate(), validating the graphics parameters in domain define time.
Tests were adapted to consider validation in this earlier stage.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 10 +--------- src/qemu/qemu_domain.c | 7 +++++++ tests/qemuxml2argvdata/graphics-egl-headless.args | 2 +- .../qemuxml2argvdata/graphics-spice-egl-headless.args | 2 +- tests/qemuxml2argvdata/graphics-vnc-egl-headless.args | 2 +- tests/qemuxml2argvtest.c | 9 ++++++--- tests/qemuxml2xmltest.c | 6 ++++-- 7 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aceb42a289..efc70d6de9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7729,7 +7729,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; @@ -7737,13 +7736,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); @@ -7788,7 +7780,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 9f29d2afbe..415f0916a2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7717,6 +7717,13 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef *graphics, break;
case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS: + 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; + } +
This looks like it is missing the associated check for graphics->data.egl_headless.rendernode, like the original check had. I wouldn't expect the test .args output later to change - Cole
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 | 37 +++++++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 16 +++++++++------- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index efc70d6de9..116ed45a12 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8271,24 +8271,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); @@ -8304,13 +8290,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, @@ -8326,9 +8305,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 415f0916a2..e4b70174ab 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5956,6 +5956,43 @@ 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: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected smartcard type %d"), + 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 b449c21f18..5c54060723 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1333,12 +1333,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); @@ -1353,7 +1354,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/9/19 6:15 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 | 37 +++++++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 16 +++++++++------- 3 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index efc70d6de9..116ed45a12 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8271,24 +8271,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); @@ -8304,13 +8290,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, @@ -8326,9 +8305,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, break;
default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), - smartcard->type); return -1;
This still returns -1. If somehow we hit this condition, the startup will fail but no error will be reported. I think just 'return 0;' here instead - Cole
On 12/16/19 5:46 PM, Cole Robinson wrote:
On 12/9/19 6:15 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 | 37 +++++++++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 16 +++++++++------- 3 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index efc70d6de9..116ed45a12 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8271,24 +8271,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); @@ -8304,13 +8290,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, @@ -8326,9 +8305,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, break;
default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), - smartcard->type); return -1;
This still returns -1. If somehow we hit this condition, the startup will fail but no error will be reported. I think just 'return 0;' here instead
Actually virReportEnumRangeError seems like the common pattern in this case - Cole
Console validation is currently being done by qemuBuildConsoleCommandLine(). This patch moves it to a new qemuDomainDefValidateConsole() function. This new function is then called by qemuDomainDefValidate(), validating the console in domain define time. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 15 -------------- src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 116ed45a12..d1befdab90 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8784,12 +8784,6 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager, switch (console->targetType) { case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCLPCONSOLE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("sclpconsole is not supported in this QEMU binary")); - return -1; - } - if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, console->source, @@ -8805,12 +8799,6 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager, break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCLPLMCONSOLE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("sclplmconsole is not supported in this QEMU binary")); - return -1; - } - if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, cmd, cfg, def, console->source, @@ -8844,9 +8832,6 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager, break; default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported console target type %s"), - NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); return -1; } } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e4b70174ab..3dba074c5c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5474,6 +5474,48 @@ qemuDomainDefValidateBoot(const virDomainDef *def, return 0; } +static int +qemuDomainDefValidateConsole(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + /* Explicit console devices */ + for (i = 0; i < def->nconsoles; i++) { + virDomainChrDefPtr console = def->consoles[i]; + + switch (console->targetType) { + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCLPCONSOLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclpconsole is not supported in this QEMU binary")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCLPLMCONSOLE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclplmconsole is not supported in this QEMU binary")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported console target type %s"), + NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); + return -1; + } + } + + return 0; +} + static int qemuDomainDefValidate(const virDomainDef *def, @@ -5650,6 +5692,9 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidateNuma(def, qemuCaps) < 0) goto cleanup; + if (qemuDomainDefValidateConsole(def, qemuCaps) < 0) + goto cleanup; + if (cfg->vncTLS && cfg->vncTLSx509secretUUID && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { for (i = 0; i < def->ngraphics; i++) { -- 2.23.0
qemuBuildTPMDevStr() does TPM model validation that can be moved to qemu_domain.c, allowing validation in domain define time. This patch moves it to the existing qemuDomainDeviceDefValidateTPM() function. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_command.c | 32 ++++---------------------------- src/qemu/qemu_domain.c | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d1befdab90..b77003d1b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9029,34 +9029,11 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, static char * -qemuBuildTPMDevStr(const virDomainDef *def, - virQEMUCapsPtr qemuCaps) +qemuBuildTPMDevStr(const virDomainDef *def) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; const virDomainTPMDef *tpm = def->tpm; const char *model = virDomainTPMModelTypeToString(tpm->model); - virQEMUCapsFlags flag; - - switch (tpm->model) { - case VIR_DOMAIN_TPM_MODEL_TIS: - flag = QEMU_CAPS_DEVICE_TPM_TIS; - break; - case VIR_DOMAIN_TPM_MODEL_CRB: - flag = QEMU_CAPS_DEVICE_TPM_CRB; - break; - case VIR_DOMAIN_TPM_MODEL_LAST: - default: - virReportEnumRangeError(virDomainTPMModel, tpm->model); - return NULL; - } - - if (!virQEMUCapsGet(qemuCaps, flag)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("The QEMU executable %s does not support TPM " - "model %s"), - def->emulator, model); - return NULL; - } virBufferAsprintf(&buf, "%s,tpmdev=tpm-%s,id=%s", model, tpm->info.alias, tpm->info.alias); @@ -9150,8 +9127,7 @@ qemuBuildTPMBackendStr(const virDomainDef *def, static int qemuBuildTPMCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) + const virDomainDef *def) { char *optstr; g_autofree char *chardev = NULL; @@ -9191,7 +9167,7 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, VIR_FREE(fdset); } - if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps))) + if (!(optstr = qemuBuildTPMDevStr(def))) return -1; virCommandAddArgList(cmd, "-device", optstr, NULL); @@ -9915,7 +9891,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, chardevStdioLogd) < 0) return NULL; - if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildTPMCommandLine(cmd, def) < 0) return NULL; if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3dba074c5c..f5369f5b36 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7600,6 +7600,8 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm, const virDomainDef *def, virQEMUCapsPtr qemuCaps) { + virQEMUCapsFlags flag; + /* TPM 1.2 and 2 are not compatible, so we choose a specific version here */ if (tpm->version == VIR_DOMAIN_TPM_VERSION_DEFAULT) tpm->version = VIR_DOMAIN_TPM_VERSION_1_2; @@ -7636,6 +7638,28 @@ qemuDomainDeviceDefValidateTPM(virDomainTPMDef *tpm, break; } + switch (tpm->model) { + case VIR_DOMAIN_TPM_MODEL_TIS: + flag = QEMU_CAPS_DEVICE_TPM_TIS; + break; + case VIR_DOMAIN_TPM_MODEL_CRB: + flag = QEMU_CAPS_DEVICE_TPM_CRB; + break; + case VIR_DOMAIN_TPM_MODEL_LAST: + default: + virReportEnumRangeError(virDomainTPMModel, tpm->model); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, flag)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The QEMU executable %s does not support TPM " + "model %s"), + def->emulator, + virDomainTPMModelTypeToString(tpm->model)); + return -1; + } + return 0; no_support: -- 2.23.0
offlist. Wow this looks awesome, thanks for tackling it! I'll give the cover letter a response tomorrow when I've had time to digest it On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
Cole also mentioned that the machine features validation was a good place to start. I took it a step further and did it across the board on all qemu_command.c.
This series didn't create any new validation, just moved them to domain define time. Any other outcome is unintended.
Not all cases where covered in this work. For a first version I decided to do only the most trivial cases. These are the other cases I left out for another day:
- all verifications contained in functions that are also called by qemu_hotplug.c. This means that the hotplug process will also use the validation code, and we can't just move it to qemu_domain.c. Besides, not all validation done in domain define time applies to hotplug, so it's not simply a matter of calling the same function from qemu_domain in qemu_hotplug. I have patches that moved the verification of all virtio options to qemu_domain.c, while also considering qemu_hotplug validation. In the end I decided to leave it away from this work for now because (1) it took 8 patches just for virtio case alone and (2) there are a lot of these cases in qemu_command.c and it would be too much to do it all at in this same series.
- moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too.
- SHMem: part of SHMem validation is being used by hotplug code. I could've moved the non-hotplug validation to define time, instead I decided it's better to to leave it to do it all at once in the future.
- DBus-VMState: validation of this fella must be done by first querying if the hash vm->priv->dbusVMState has elements. qemuDomainDefValidate does not have 'vm->priv' in it's API, meaning that I would need to either change the API to include it (which means changing domainValidateCallback), or do the validation by another branch where I can get access to vm->priv.
- vmcoreinfo: there are no challenges in moving vmcoreinfo validation to qemuDomainDefValidate. The problem were the unit tests. Moving this validation to domain define time breaks 925 tests on qemuxml2xmltest.c, including tests labelled as 'minimal' which should pass under any circurstances, as far as I understand. It is possible to just shoehorn QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but this would tamper the idea of having to deal with NONE or LATEST or a specific set of capabilities for certain tests. Another case I would rather discuss separately.
[1] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html
Daniel Henrique Barboza (26): qemu_command.c: move PSeries features validation to qemu_domain.c qemu_command.c: move mem.nosharepages validation to qemu_domain.c qemu_command.c: move validation of vmport to qemu_domain.c qemu_command.c: move nvdimm validation to qemu_domain.c qemu_command.c: move I/O APIC validation to qemu_domain.c numa_conf: add virDomainNumaNodesDistancesAreBeingSet() helper qemu_command.c move NUMA validation to qemu_domain.c qemu_command.c: move NVRAM validation to qemu_domain.c qemu_command: move qemuBuildSoundDevStr caps validation to qemu_domain qemu_command.c: move sound codec validation to qemu_domain.c qemu_command: move qemuBuildHubDevStr caps validation to qemu_domain qemu_command: move qemuBuildChrChardevStr caps validation to qemu_domain qemu: move qemuBuildHostdevCommandLine caps validation to qemu_domain qemu_command.c: move vmGenID validation to qemu_domain.c qemu: move qemuBuildSgaCommandLine validation to qemu_domain.c qemu: move virDomainClockDef validation to qemu_domain.c qemu: move qemuBuildPMCommandLine validation to qemu_domain.c qemu: move qemuBuildBootCommandLine validation to qemu_domain.c qemu_command.c: move pcihole64 validation to qemu_domain.c qemu: move qemuBuildGraphicsSDLCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsVNCCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsSPICECommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c qemu: move qemuBuildConsoleCommandLine validation to qemu_domain.c qemu: move qemuBuildTPMDevStr TPM validation to qemu_domain.c
src/conf/numa_conf.c | 19 + src/conf/numa_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 575 +--------- src/qemu/qemu_command.h | 1 + src/qemu/qemu_domain.c | 983 ++++++++++++++++-- tests/qemuhotplugtest.c | 10 + tests/qemumemlocktest.c | 16 +- .../default-video-type-aarch64.xml | 2 +- .../default-video-type-ppc64.xml | 2 +- .../default-video-type-s390x.xml | 2 +- .../graphics-egl-headless.args | 2 +- .../graphics-spice-egl-headless.args | 2 +- .../graphics-vnc-egl-headless.args | 2 +- tests/qemuxml2argvtest.c | 80 +- ...ault-video-type-aarch64.aarch64-latest.xml | 4 +- .../default-video-type-ppc64.ppc64-latest.xml | 4 +- .../default-video-type-s390x.s390x-latest.xml | 4 +- tests/qemuxml2xmltest.c | 244 +++-- 19 files changed, 1246 insertions(+), 709 deletions(-)
- Cole
On Mon, Dec 09, 2019 at 20:15:05 -0300, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
Nice work. Doing this was long overdue. My only suggestion is that after this we should move all validation into a separate file. qemu_domain was a code dumping place for a long time and since we now have a lot of common code moving it out would be benficial for cleaning up an making it more obvious.
On 12/10/19 5:00 AM, Peter Krempa wrote:
On Mon, Dec 09, 2019 at 20:15:05 -0300, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
Nice work. Doing this was long overdue. My only suggestion is that after this we should move all validation into a separate file. qemu_domain was a code dumping place for a long time and since we now have a lot of common code moving it out would be benficial for cleaning up an making it more obvious.
Got it. We can create this new file and move the validations from qemu_domain.c to the new file, before resuming moving more code out of qemu_command.c. That way we avoid moving the code twice. Thanks, DHB
On Tue, Dec 10, 2019 at 19:45:01 -0300, Daniel Henrique Barboza wrote:
On 12/10/19 5:00 AM, Peter Krempa wrote:
On Mon, Dec 09, 2019 at 20:15:05 -0300, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
Nice work. Doing this was long overdue. My only suggestion is that after this we should move all validation into a separate file. qemu_domain was a code dumping place for a long time and since we now have a lot of common code moving it out would be benficial for cleaning up an making it more obvious.
Got it. We can create this new file and move the validations from qemu_domain.c to the new file, before resuming moving more code out of qemu_command.c. That way we avoid moving the code twice.
Since you've got patches already it's okay to do it in two steps. It will be much appreciated if you move it to a new file, but it's not required for this series.
On Mon, Dec 09, 2019 at 08:15:05PM -0300, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
I've not looked at the patches, but surely moving this validate from start time, to be define time errors is going to cause functional regressions in our ABI behaviour. My libvirtd daemons installs have many XML files defined which will fail validation at various points in time, depending on what QEMU builds I happen to have deployed. I only need them to pass the validation when actually starting the VM. I understand the motivation of making the CLI code cleaner, but there is a flip side in that we're now separating logic that is conceptually quite closely related. The CLI code makes various assumptions and currently those assumptions are validated at the same place. If we move that validation elsewhere, we increase the liklihood that we're going to have bugs where the validation code is not matching the assumptions the CLI code is making.
- moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too.
IMHO this should only ever be validated at start time. The host CPU features seen at define time should not be assumed to be the same as those that will be present at start time. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Dec 10, 2019 at 09:58:38 +0000, Daniel Berrange wrote:
On Mon, Dec 09, 2019 at 08:15:05PM -0300, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
I've not looked at the patches, but surely moving this validate from start time, to be define time errors is going to cause functional regressions in our ABI behaviour.
My libvirtd daemons installs have many XML files defined which will fail validation at various points in time, depending on what QEMU builds I happen to have deployed. I only need them to pass the validation when actually starting the VM.
Validation is not done on the persistent or status XML files exactly for this reason. It's re-done when starting the VM so this should not be an issue.
On 12/10/19 5:56 AM, Peter Krempa wrote:
On Tue, Dec 10, 2019 at 09:58:38 +0000, Daniel Berrange wrote:
On Mon, Dec 09, 2019 at 08:15:05PM -0300, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
I've not looked at the patches, but surely moving this validate from start time, to be define time errors is going to cause functional regressions in our ABI behaviour.
My libvirtd daemons installs have many XML files defined which will fail validation at various points in time, depending on what QEMU builds I happen to have deployed. I only need them to pass the validation when actually starting the VM.
Validation is not done on the persistent or status XML files exactly for this reason.
It's re-done when starting the VM so this should not be an issue.
Yup, that's what the VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE flag is all about. It's used to skip validation at daemon startup, and various cases like DomainDefCopy Conceptually this moves closer to IMO the ideal model: 1) Serialize XML into C struct 2) Validate contents against hypervisor capabilities 3) Serialize C struct into command line string Which has benefits: simplifies code in #1 and #3 which is the interesting stuff, easier to test full code coverage of #1 and #3, gives us the option to move validation into domaincapabilities code so we can report to the user whether a feature will be rejected or not. Also I suspect if we moved to using virtblocks for cli stuff, untangling validation from the cli builder would be a step in that direction - Cole
Hi, I failed to make a reservation about what I've done in patch in the cover letter. In the commit msg I mentioned about the XML files considering SPICE support as default to ppc64, aarch64 and s390 archs and fixing all the xmls. It is worth disclaiming that what I can assert to be true is the lack of SPICE support for ppc64. I can't put my money on the lack of SPICE for the other 2 archs - what I did in the patch was to assume that the emulator capabilities for aarch64 and s390, that didn't report SPICE support, was accurate. I think it's an educated guess, but a guess nevertheless. Thanks, DHB On 12/9/19 8:15 PM, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
Cole also mentioned that the machine features validation was a good place to start. I took it a step further and did it across the board on all qemu_command.c.
This series didn't create any new validation, just moved them to domain define time. Any other outcome is unintended.
Not all cases where covered in this work. For a first version I decided to do only the most trivial cases. These are the other cases I left out for another day:
- all verifications contained in functions that are also called by qemu_hotplug.c. This means that the hotplug process will also use the validation code, and we can't just move it to qemu_domain.c. Besides, not all validation done in domain define time applies to hotplug, so it's not simply a matter of calling the same function from qemu_domain in qemu_hotplug. I have patches that moved the verification of all virtio options to qemu_domain.c, while also considering qemu_hotplug validation. In the end I decided to leave it away from this work for now because (1) it took 8 patches just for virtio case alone and (2) there are a lot of these cases in qemu_command.c and it would be too much to do it all at in this same series.
- moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too.
- SHMem: part of SHMem validation is being used by hotplug code. I could've moved the non-hotplug validation to define time, instead I decided it's better to to leave it to do it all at once in the future.
- DBus-VMState: validation of this fella must be done by first querying if the hash vm->priv->dbusVMState has elements. qemuDomainDefValidate does not have 'vm->priv' in it's API, meaning that I would need to either change the API to include it (which means changing domainValidateCallback), or do the validation by another branch where I can get access to vm->priv.
- vmcoreinfo: there are no challenges in moving vmcoreinfo validation to qemuDomainDefValidate. The problem were the unit tests. Moving this validation to domain define time breaks 925 tests on qemuxml2xmltest.c, including tests labelled as 'minimal' which should pass under any circurstances, as far as I understand. It is possible to just shoehorn QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but this would tamper the idea of having to deal with NONE or LATEST or a specific set of capabilities for certain tests. Another case I would rather discuss separately.
[1] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html
Daniel Henrique Barboza (26): qemu_command.c: move PSeries features validation to qemu_domain.c qemu_command.c: move mem.nosharepages validation to qemu_domain.c qemu_command.c: move validation of vmport to qemu_domain.c qemu_command.c: move nvdimm validation to qemu_domain.c qemu_command.c: move I/O APIC validation to qemu_domain.c numa_conf: add virDomainNumaNodesDistancesAreBeingSet() helper qemu_command.c move NUMA validation to qemu_domain.c qemu_command.c: move NVRAM validation to qemu_domain.c qemu_command: move qemuBuildSoundDevStr caps validation to qemu_domain qemu_command.c: move sound codec validation to qemu_domain.c qemu_command: move qemuBuildHubDevStr caps validation to qemu_domain qemu_command: move qemuBuildChrChardevStr caps validation to qemu_domain qemu: move qemuBuildHostdevCommandLine caps validation to qemu_domain qemu_command.c: move vmGenID validation to qemu_domain.c qemu: move qemuBuildSgaCommandLine validation to qemu_domain.c qemu: move virDomainClockDef validation to qemu_domain.c qemu: move qemuBuildPMCommandLine validation to qemu_domain.c qemu: move qemuBuildBootCommandLine validation to qemu_domain.c qemu_command.c: move pcihole64 validation to qemu_domain.c qemu: move qemuBuildGraphicsSDLCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsVNCCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsSPICECommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c qemu: move qemuBuildConsoleCommandLine validation to qemu_domain.c qemu: move qemuBuildTPMDevStr TPM validation to qemu_domain.c
src/conf/numa_conf.c | 19 + src/conf/numa_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 575 +--------- src/qemu/qemu_command.h | 1 + src/qemu/qemu_domain.c | 983 ++++++++++++++++-- tests/qemuhotplugtest.c | 10 + tests/qemumemlocktest.c | 16 +- .../default-video-type-aarch64.xml | 2 +- .../default-video-type-ppc64.xml | 2 +- .../default-video-type-s390x.xml | 2 +- .../graphics-egl-headless.args | 2 +- .../graphics-spice-egl-headless.args | 2 +- .../graphics-vnc-egl-headless.args | 2 +- tests/qemuxml2argvtest.c | 80 +- ...ault-video-type-aarch64.aarch64-latest.xml | 4 +- .../default-video-type-ppc64.ppc64-latest.xml | 4 +- .../default-video-type-s390x.s390x-latest.xml | 4 +- tests/qemuxml2xmltest.c | 244 +++-- 19 files changed, 1246 insertions(+), 709 deletions(-)
On Tue, Dec 10, 2019 at 07:22:37AM -0300, Daniel Henrique Barboza wrote:
Hi,
I failed to make a reservation about what I've done in patch in the cover letter. In the commit msg I mentioned about the XML files considering SPICE support as default to ppc64, aarch64 and s390 archs and fixing all the xmls.
It is worth disclaiming that what I can assert to be true is the lack of SPICE support for ppc64. I can't put my money on the lack of SPICE for the other 2 archs - what I did in the patch was to assume that the emulator capabilities for aarch64 and s390, that didn't report SPICE support, was accurate. I think it's an educated guess, but a guess nevertheless.
SPICE is present on arch64. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
Cole also mentioned that the machine features validation was a good place to start. I took it a step further and did it across the board on all qemu_command.c.
This series didn't create any new validation, just moved them to domain define time. Any other outcome is unintended.
Not all cases where covered in this work. For a first version I decided to do only the most trivial cases. These are the other cases I left out for another day:
Yes I think that's the right approach, handle the easy bits first and deal the complicated bits in their own series. Some cases will likely cause a lot of test suite churn, some cases may not even be worth it, we will see!
- all verifications contained in functions that are also called by qemu_hotplug.c. This means that the hotplug process will also use the validation code, and we can't just move it to qemu_domain.c. Besides, not all validation done in domain define time applies to hotplug, so it's not simply a matter of calling the same function from qemu_domain in qemu_hotplug. I have patches that moved the verification of all virtio options to qemu_domain.c, while also considering qemu_hotplug validation. In the end I decided to leave it away from this work for now because (1) it took 8 patches just for virtio case alone and (2) there are a lot of these cases in qemu_command.c and it would be too much to do it all at in this same series.
The hotplug code should already be calling into Validate code. When a device is hotplugged via qemu_driver, we get: qemu_driver.c -> qemuDomainAttachDeviceFlags -> qemuDomainAttachDeviceLiveAndConfig -> virDomainDeviceDefParse -> virDomainDeviceDefValidate -> deviceValidateCallback -> qemuDomainDeviceDefValidate So if device validation is moved into the correct qemuDomainDeviceDefValidateXXX function it will get called in the hotplug path so they won't be lost.
- moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too.
Hmm I glanced at the qemuBuildCpuModelArgStr checks but it doesn't strike me why those are an issue. Validate should always be triggered after PostParse, so the ordering of those two shouldn't impact things here. But I didn't attempt the change
- SHMem: part of SHMem validation is being used by hotplug code. I could've moved the non-hotplug validation to define time, instead I decided it's better to to leave it to do it all at once in the future.
shmem is just another device, so it should be fine as long as its final location is triggered by qemuDomainDeviceDefValidate
- DBus-VMState: validation of this fella must be done by first querying if the hash vm->priv->dbusVMState has elements. qemuDomainDefValidate does not have 'vm->priv' in it's API, meaning that I would need to either change the API to include it (which means changing domainValidateCallback), or do the validation by another branch where I can get access to vm->priv.
Yeah this one is a little convoluted. The code is using the vm->priv check to determine 'the user requested external slirp', which is really a factor of interface type='user' + a qemu.conf setting, which we _can_ check at validate time against qemuCaps. But it's minor so I suggest just ignoring it
- vmcoreinfo: there are no challenges in moving vmcoreinfo validation to qemuDomainDefValidate. The problem were the unit tests. Moving this validation to domain define time breaks 925 tests on qemuxml2xmltest.c, including tests labelled as 'minimal' which should pass under any circurstances, as far as I understand. It is possible to just shoehorn QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but this would tamper the idea of having to deal with NONE or LATEST or a specific set of capabilities for certain tests. Another case I would rather discuss separately.
I suspect a bug on your side. qemu_command.c has: static int qemuBuildVMCoreInfoCommandLine(virCommandPtr cmd, const virDomainDef *def, virQEMUCapsPtr qemuCaps) { 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; } If you just copy the middle section, you'll get tons of failures. But what you want to add is: if (def->features[VIR_DOMAIN_FEATURE_VMCOREINFO] == VIR_TRISTATE_SWITCH_ON && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMCOREINFO)) <error> Thanks, Cole
On 12/10/19 6:41 PM, Cole Robinson wrote:
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
[...]
The hotplug code should already be calling into Validate code. When a device is hotplugged via qemu_driver, we get:
qemu_driver.c -> qemuDomainAttachDeviceFlags -> qemuDomainAttachDeviceLiveAndConfig -> virDomainDeviceDefParse -> virDomainDeviceDefValidate -> deviceValidateCallback -> qemuDomainDeviceDefValidate
So if device validation is moved into the correct qemuDomainDeviceDefValidateXXX function it will get called in the hotplug path so they won't be lost.
Good to know. I assumed that the hotplug path was separated. Perhaps this info can be put somewhere in the docs folder as reference. This appears to be the kind of call hierarchy that doesn't change that often, so it wouldn't be a burden to keep the doc updated.
- moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too.
Hmm I glanced at the qemuBuildCpuModelArgStr checks but it doesn't strike me why those are an issue. Validate should always be triggered after PostParse, so the ordering of those two shouldn't impact things here. But I didn't attempt the change
Unfortunately I've erased the error I was seeing in this case. I recall something to do with VIR_CPU_MODE_HOST_MODEL being set in qemuDomainDefSetDefaultCPU(), but TBH it's easier to just move the code again and see what happens.
I suspect a bug on your side. qemu_command.c has:
I'll attempt that in the next iterations of this work. Thanks, Daniel
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
(series based on master commit 97cafa610ecf5)
This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]:
------- The benefits of moving to validate time is that XML is rejected by 'virsh define' rather than at 'virsh start' time. It also makes it easier to follow the cli building code, and makes it easier to verify qemu_command.c test suite code coverage for the important stuff like covering every CLI option. It's also a good intermediate step for sharing validation with domain capabilities building, like is done presently for rng models. -------
Cole also mentioned that the machine features validation was a good place to start. I took it a step further and did it across the board on all qemu_command.c.
This series didn't create any new validation, just moved them to domain define time. Any other outcome is unintended.
Not all cases where covered in this work. For a first version I decided to do only the most trivial cases. These are the other cases I left out for another day:
- all verifications contained in functions that are also called by qemu_hotplug.c. This means that the hotplug process will also use the validation code, and we can't just move it to qemu_domain.c. Besides, not all validation done in domain define time applies to hotplug, so it's not simply a matter of calling the same function from qemu_domain in qemu_hotplug. I have patches that moved the verification of all virtio options to qemu_domain.c, while also considering qemu_hotplug validation. In the end I decided to leave it away from this work for now because (1) it took 8 patches just for virtio case alone and (2) there are a lot of these cases in qemu_command.c and it would be too much to do it all at in this same series.
- moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too.
- SHMem: part of SHMem validation is being used by hotplug code. I could've moved the non-hotplug validation to define time, instead I decided it's better to to leave it to do it all at once in the future.
- DBus-VMState: validation of this fella must be done by first querying if the hash vm->priv->dbusVMState has elements. qemuDomainDefValidate does not have 'vm->priv' in it's API, meaning that I would need to either change the API to include it (which means changing domainValidateCallback), or do the validation by another branch where I can get access to vm->priv.
- vmcoreinfo: there are no challenges in moving vmcoreinfo validation to qemuDomainDefValidate. The problem were the unit tests. Moving this validation to domain define time breaks 925 tests on qemuxml2xmltest.c, including tests labelled as 'minimal' which should pass under any circurstances, as far as I understand. It is possible to just shoehorn QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but this would tamper the idea of having to deal with NONE or LATEST or a specific set of capabilities for certain tests. Another case I would rather discuss separately.
[1] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html
Daniel Henrique Barboza (26): qemu_command.c: move PSeries features validation to qemu_domain.c qemu_command.c: move mem.nosharepages validation to qemu_domain.c qemu_command.c: move validation of vmport to qemu_domain.c qemu_command.c: move nvdimm validation to qemu_domain.c qemu_command.c: move I/O APIC validation to qemu_domain.c numa_conf: add virDomainNumaNodesDistancesAreBeingSet() helper qemu_command.c move NUMA validation to qemu_domain.c qemu_command.c: move NVRAM validation to qemu_domain.c qemu_command: move qemuBuildSoundDevStr caps validation to qemu_domain qemu_command.c: move sound codec validation to qemu_domain.c qemu_command: move qemuBuildHubDevStr caps validation to qemu_domain qemu_command: move qemuBuildChrChardevStr caps validation to qemu_domain qemu: move qemuBuildHostdevCommandLine caps validation to qemu_domain qemu_command.c: move vmGenID validation to qemu_domain.c qemu: move qemuBuildSgaCommandLine validation to qemu_domain.c qemu: move virDomainClockDef validation to qemu_domain.c qemu: move qemuBuildPMCommandLine validation to qemu_domain.c qemu: move qemuBuildBootCommandLine validation to qemu_domain.c qemu_command.c: move pcihole64 validation to qemu_domain.c qemu: move qemuBuildGraphicsSDLCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsVNCCommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsSPICECommandLine validation to qemu_domain.c qemu: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c qemu: move qemuBuildConsoleCommandLine validation to qemu_domain.c qemu: move qemuBuildTPMDevStr TPM validation to qemu_domain.c
Reviewed-by: Cole Robinson <crobinso@redhat.com> I standardized the subject prefixes to 'qemu: command:' and pushed them all, except 4, 23, and 24, the ones I commented on Thanks, Cole
participants (4)
-
Cole Robinson -
Daniel Henrique Barboza -
Daniel P. Berrangé -
Peter Krempa