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(a)gmail.com&gt; --- 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

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(a)gmail.com&gt; --- 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

On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote: This should be added to a function like qemuDomainDeviceDefValidateMemory called via qemuDomainDeviceDefValidate - Cole

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(a)gmail.com&gt; --- 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 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(a)gmail.com&gt; --- 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

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(a)gmail.com&gt; --- 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_CHARDEV_FILE_APPEND and QEMU_CAPS_CHARDEV_LOGFILE to qemuDomainChrSourceDefValidate(). Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- 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

QEMU_CAPS_DEVICE_VMGENID is now being validated by qemuDomainDefValidate(). Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; --- 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

@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(a)gmail.com&gt; --- 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 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(a)gmail.com&gt; --- 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

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(a)gmail.com&gt; --- 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 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(a)gmail.com&gt; --- 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

On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote: 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

On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote: 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

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(a)gmail.com&gt; --- 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