[libvirt PATCH] Move default Input bus logic to PostParse handling
A new enum type "Default" has been added for Input bus. The logic that handled default input bus types in virDomainInputParseXML() has been moved to a new function virDomainInputDefPostParse() in domain_postparse.c Link to Issue: https://gitlab.com/libvirt/libvirt/-/issues/8 Signed-off-by: K Shiva <shiva_kr@riseup.net> --- src/conf/domain_conf.c | 27 +++------------------------ src/conf/domain_conf.h | 1 + src/conf/domain_postparse.c | 30 ++++++++++++++++++++++++++++++ src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_hotplug.c | 2 ++ 6 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b03a3ff011..22af3f1d8a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -905,6 +905,7 @@ VIR_ENUM_IMPL(virDomainInput, VIR_ENUM_IMPL(virDomainInputBus, VIR_DOMAIN_INPUT_BUS_LAST, + "default", "ps2", "usb", "xen", @@ -10693,7 +10694,6 @@ virDomainPanicDefParseXML(virDomainXMLOption *xmlopt, /* Parse the XML definition for an input device */ static virDomainInputDef * virDomainInputDefParseXML(virDomainXMLOption *xmlopt, - const virDomainDef *dom, xmlNodePtr node, xmlXPathContextPtr ctxt, unsigned int flags) @@ -10741,27 +10741,7 @@ virDomainInputDefParseXML(virDomainXMLOption *xmlopt, } } 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; - } + def->bus = VIR_DOMAIN_INPUT_BUS_DEFAULT; } if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0) @@ -13766,7 +13746,7 @@ virDomainDeviceDefParse(const char *xmlStr, return NULL; break; case VIR_DOMAIN_DEVICE_INPUT: - if (!(dev->data.input = virDomainInputDefParseXML(xmlopt, def, node, + if (!(dev->data.input = virDomainInputDefParseXML(xmlopt, node, ctxt, flags))) return NULL; break; @@ -18872,7 +18852,6 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, for (i = 0; i < n; i++) { virDomainInputDef *input = virDomainInputDefParseXML(xmlopt, - def, nodes[i], ctxt, flags); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 511067a050..2a8fc6f90d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1510,6 +1510,7 @@ typedef enum { } virDomainInputType; typedef enum { + VIR_DOMAIN_INPUT_BUS_DEFAULT, VIR_DOMAIN_INPUT_BUS_PS2, VIR_DOMAIN_INPUT_BUS_USB, VIR_DOMAIN_INPUT_BUS_XEN, diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c index b756e2cde8..7ef478e3e1 100644 --- a/src/conf/domain_postparse.c +++ b/src/conf/domain_postparse.c @@ -649,6 +649,33 @@ virDomainFSDefPostParse(virDomainFSDef *fs) return 0; } +static void +virDomainInputDefPostParse(virDomainInputDef *input, + const virDomainDef *def) +{ + if (input->bus == VIR_DOMAIN_INPUT_BUS_DEFAULT) { + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if ((input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE || + input->type == VIR_DOMAIN_INPUT_TYPE_KBD) && + (ARCH_IS_X86(def->os.arch) || def->os.arch == VIR_ARCH_NONE)) { + } else if (ARCH_IS_S390(def->os.arch) || + input->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { + input->bus = VIR_DOMAIN_INPUT_BUS_VIRTIO; + } else if (input->type == VIR_DOMAIN_INPUT_TYPE_EVDEV) { + input->bus = VIR_DOMAIN_INPUT_BUS_NONE; + } else { + input->bus = VIR_DOMAIN_INPUT_BUS_USB; + } + } else if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || + def->os.type == VIR_DOMAIN_OSTYPE_XENPVH) { + input->bus = VIR_DOMAIN_INPUT_BUS_XEN; + } else { + if ((def->virtType == VIR_DOMAIN_VIRT_VZ || + def->virtType == VIR_DOMAIN_VIRT_PARALLELS)) + input->bus = VIR_DOMAIN_INPUT_BUS_PARALLELS; + } + } +} static int virDomainDeviceDefPostParseCommon(virDomainDeviceDef *dev, @@ -701,6 +728,9 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_NET: case VIR_DOMAIN_DEVICE_INPUT: + virDomainInputDefPostParse(dev->data.input, def); + ret = 0; + break; case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_WATCHDOG: case VIR_DOMAIN_DEVICE_GRAPHICS: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4ca93bf3dc..135e35f43a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4345,6 +4345,7 @@ qemuBuildInputCommandLine(virCommand *cmd, if (!(props = qemuBuildInputVirtioDevProps(def, input, qemuCaps))) return -1; + case VIR_DOMAIN_INPUT_BUS_DEFAULT: case VIR_DOMAIN_INPUT_BUS_PS2: case VIR_DOMAIN_INPUT_BUS_XEN: case VIR_DOMAIN_INPUT_BUS_PARALLELS: diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 7d3d072d5a..49c5e199fa 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -984,6 +984,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, } return 0; + case VIR_DOMAIN_INPUT_BUS_DEFAULT: case VIR_DOMAIN_INPUT_BUS_PS2: case VIR_DOMAIN_INPUT_BUS_USB: case VIR_DOMAIN_INPUT_BUS_XEN: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5072798cb7..f7a41e376e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3053,6 +3053,7 @@ qemuDomainAttachInputDevice(virDomainObj *vm, goto cleanup; break; + case VIR_DOMAIN_INPUT_BUS_DEFAULT: case VIR_DOMAIN_INPUT_BUS_PS2: case VIR_DOMAIN_INPUT_BUS_XEN: case VIR_DOMAIN_INPUT_BUS_PARALLELS: @@ -5799,6 +5800,7 @@ qemuDomainDetachPrepInput(virDomainObj *vm, *detach = input = vm->def->inputs[idx]; switch ((virDomainInputBus) input->bus) { + case VIR_DOMAIN_INPUT_BUS_DEFAULT: case VIR_DOMAIN_INPUT_BUS_PS2: case VIR_DOMAIN_INPUT_BUS_XEN: case VIR_DOMAIN_INPUT_BUS_PARALLELS: -- 2.40.0
On Sat, Apr 22, 2023 at 11:25:30PM +0530, K Shiva wrote:
A new enum type "Default" has been added for Input bus. The logic that handled default input bus types in virDomainInputParseXML() has been moved to a new function virDomainInputDefPostParse() in domain_postparse.c Link to Issue: https://gitlab.com/libvirt/libvirt/-/issues/8
Signed-off-by: K Shiva <shiva_kr@riseup.net> --- src/conf/domain_conf.c | 27 +++------------------------ src/conf/domain_conf.h | 1 + src/conf/domain_postparse.c | 30 ++++++++++++++++++++++++++++++ src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_hotplug.c | 2 ++ 6 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b03a3ff011..22af3f1d8a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -905,6 +905,7 @@ VIR_ENUM_IMPL(virDomainInput,
VIR_ENUM_IMPL(virDomainInputBus, VIR_DOMAIN_INPUT_BUS_LAST, + "default", "ps2", "usb", "xen", @@ -10693,7 +10694,6 @@ virDomainPanicDefParseXML(virDomainXMLOption *xmlopt, /* Parse the XML definition for an input device */ static virDomainInputDef * virDomainInputDefParseXML(virDomainXMLOption *xmlopt, - const virDomainDef *dom, xmlNodePtr node, xmlXPathContextPtr ctxt, unsigned int flags) @@ -10741,27 +10741,7 @@ virDomainInputDefParseXML(virDomainXMLOption *xmlopt, }
} 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; - } + def->bus = VIR_DOMAIN_INPUT_BUS_DEFAULT;
Since @def here is zero-initialized and _BUS_DEFAULT is 0 (the first variant of that enum) you can skip this branch completely. And if you look at the model parsing above, we should ideally forbid the user to set bus="default" too. Reviewed-by: Martin Kletzander <mkletzan@redhat.com> And I would squash this in before pushing if you're OK with that: diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 0d8b128f9538..222dd989f54f 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -10733,15 +10733,12 @@ virDomainInputDefParseXML(virDomainXMLOption *xmlopt, goto error; } - if (bus) { - if ((def->bus = virDomainInputBusTypeFromString(bus)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input bus type '%1$s'"), bus); - goto error; - } - - } else { - def->bus = VIR_DOMAIN_INPUT_BUS_DEFAULT; + if (bus && + ((def->bus = virDomainInputBusTypeFromString(bus)) < 0 || + def->bus == VIR_DOMAIN_INPUT_BUS_DEFAULT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown input bus type '%1$s'"), bus); + goto error; } if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0) -- Martin
On 4/24/23 12:15, Martin Kletzander wrote:
And I would squash this in before pushing if you're OK with that:
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 0d8b128f9538..222dd989f54f 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -10733,15 +10733,12 @@ virDomainInputDefParseXML(virDomainXMLOption *xmlopt, goto error; }
- if (bus) { - if ((def->bus = virDomainInputBusTypeFromString(bus)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown input bus type '%1$s'"), bus); - goto error; - } - - } else { - def->bus = VIR_DOMAIN_INPUT_BUS_DEFAULT; + if (bus && + ((def->bus = virDomainInputBusTypeFromString(bus)) < 0 || + def->bus == VIR_DOMAIN_INPUT_BUS_DEFAULT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown input bus type '%1$s'"), bus); + goto error; }
if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0) --
Martin
Greetings Mentor Martin, I have understood your corrections. Please proceed with the squash and the commit. Thank You K Shiva
participants (3)
-
K Shiva -
Martin Kletzander -
Shiva