On a Saturday in 2022, Jamm02 wrote:
All the checks against dom->os.type and dom->virtType in virDomainInputDefParseXML shifted to virDomainInputDefValidate function
The code you're adding is not validation, it is the logic for filling the default bus. *Validate functions should not alter the device configuration. A virDomainDefPostParseInput function would be more appropriate for this code.
Signed-off-by: Jamm02 <codeguy.moteen@xxxxxxxxx>
---
src/conf/domain_conf.c | 49 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e0dfc9e45f..b237fd9ab1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11837,6 +11837,55 @@ virDomainPanicDefParseXML(virDomainXMLOption *xmlopt,
return NULL;
}
+static virDomainInputDef *
+virDomainInputDefValidate(const virDomainDef *dom,
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
Only the *Parse functions should be dealing with XML directly. The later phases (PostParse and Validate) should operate on virDomainInputDef. Also, this function you're introducing is not used anywhere and returns an incompletely parsed virDomainInputDef, which is not helpful. Instead, it should return error/success (-1 / 0)
+{
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+ virDomainInputDef *def;
+ g_autofree char *bus = NULL;
+
+ def = g_new0(virDomainInputDef, 1);
+
+ ctxt->node = node;
+ bus = virXMLPropString(node, "bus");
+
+ if (bus) {
+ if ((def->bus = virDomainInputBusTypeFromString(bus)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown input bus type '%s'"), bus);
+ goto error;
+ }
This should stay in the parser. To be able to tell later whether there was a bus specified in the XML, a new enum value VIR_DOMAIN_INPUT_BUS_DEFAULT can be added.
+
+ } else {
+ if (dom->os.type == VIR_DOMAIN_OSTYPE_HVM) {
+ if ((def->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
+ def->type == VIR_DOMAIN_INPUT_TYPE_KBD) &&
+ (ARCH_IS_X86(dom->os.arch) || dom->os.arch == VIR_ARCH_NONE)) {
+ def->bus = VIR_DOMAIN_INPUT_BUS_PS2;
+ } else if (ARCH_IS_S390(dom->os.arch) ||
+ def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
+ def->bus = VIR_DOMAIN_INPUT_BUS_VIRTIO;
+ } else if (def->type == VIR_DOMAIN_INPUT_TYPE_EVDEV) {
+ def->bus = VIR_DOMAIN_INPUT_BUS_NONE;
+ } else {
+ def->bus = VIR_DOMAIN_INPUT_BUS_USB;
+ }
+ } else if (dom->os.type == VIR_DOMAIN_OSTYPE_XEN ||
+ dom->os.type == VIR_DOMAIN_OSTYPE_XENPVH) {
+ def->bus = VIR_DOMAIN_INPUT_BUS_XEN;
+ } else {
+ if ((dom->virtType == VIR_DOMAIN_VIRT_VZ ||
+ dom->virtType == VIR_DOMAIN_VIRT_PARALLELS))
+ def->bus = VIR_DOMAIN_INPUT_BUS_PARALLELS;
+ }
+ }
On success, the control flow would continue to the error label below and always return NULL. Jano
+ + error: + virDomainInputDefFree(def); + return NULL; +} /* Parse the XML definition for an input device */ static virDomainInputDef * virDomainInputDefParseXML(virDomainXMLOption *xmlopt, -- 2.35.1
Attachment:
signature.asc
Description: PGP signature
