Re: [libvirt PATCH] Move default Input bus logic to PostParse handling

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>

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

Attachment: signature.asc
Description: PGP signature