[v1] qemu: Improve handling of architecture-specific defaults

[PATCH 11/33] qemu: Clean up qemuDomainDefaultNetModel()

Posted by Andrea Bolognani 8 months, 3 weeks ago

Group things together where it makes sense, avoid unnecessary
uses of 'else if', plus other tweaks.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d56ac929ea..9289c1fa18 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5396,12 +5396,14 @@ static int
 qemuDomainDefaultNetModel(const virDomainDef *def,
                           virQEMUCaps *qemuCaps)
 {
-    if (ARCH_IS_S390(def->os.arch))
+    /* When there are no backwards compatibility concerns getting in
+     * the way, virtio is a good default */
+    if (ARCH_IS_S390(def->os.arch) ||
+        qemuDomainIsRISCVVirt(def)) {
         return VIR_DOMAIN_NET_MODEL_VIRTIO;
+    }
 
-    if (def->os.arch == VIR_ARCH_ARMV6L ||
-        def->os.arch == VIR_ARCH_ARMV7L ||
-        def->os.arch == VIR_ARCH_AARCH64) {
+    if (ARCH_IS_ARM(def->os.arch)) {
         if (STREQ(def->os.machine, "versatilepb"))
             return VIR_DOMAIN_NET_MODEL_SMC91C111;
 
@@ -5413,10 +5415,6 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
         return VIR_DOMAIN_NET_MODEL_LAN9118;
     }
 
-    /* virtio is a sensible default for RISC-V virt guests */
-    if (qemuDomainIsRISCVVirt(def))
-        return VIR_DOMAIN_NET_MODEL_VIRTIO;
-
     /* In all other cases the model depends on the capabilities. If they were
      * not provided don't report any default. */
     if (!qemuCaps)
@@ -5427,9 +5425,9 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
      * system than the previous one */
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RTL8139))
         return VIR_DOMAIN_NET_MODEL_RTL8139;
-    else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000))
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000))
         return VIR_DOMAIN_NET_MODEL_E1000;
-    else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET))
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET))
         return VIR_DOMAIN_NET_MODEL_VIRTIO;
 
     /* We've had no luck detecting support for any network device,
-- 
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Re: [PATCH 11/33] qemu: Clean up qemuDomainDefaultNetModel()

Posted by Peter Krempa 8 months, 3 weeks ago

On Wed, Jan 24, 2024 at 20:37:31 +0100, Andrea Bolognani wrote:
> Group things together where it makes sense, avoid unnecessary
> uses of 'else if', plus other tweaks.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)


> @@ -5427,9 +5425,9 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
>       * system than the previous one */
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RTL8139))
>          return VIR_DOMAIN_NET_MODEL_RTL8139;
> -    else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000))
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000))
>          return VIR_DOMAIN_NET_MODEL_E1000;
> -    else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET))
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET))
>          return VIR_DOMAIN_NET_MODEL_VIRTIO;

Separate these by empty lines once you've broke up the 'else if'
pattern.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Re: Re: [PATCH 11/33] qemu: Clean up qemuDomainDefaultNetModel()

Posted by Andrea Bolognani 8 months, 3 weeks ago

On Thu, Jan 25, 2024 at 11:12:33AM +0100, Peter Krempa wrote:
> On Wed, Jan 24, 2024 at 20:37:31 +0100, Andrea Bolognani wrote:
> > @@ -5427,9 +5425,9 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
> >       * system than the previous one */
> >      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RTL8139))
> >          return VIR_DOMAIN_NET_MODEL_RTL8139;
> > -    else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000))
> > +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_E1000))
> >          return VIR_DOMAIN_NET_MODEL_E1000;
> > -    else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET))
> > +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET))
> >          return VIR_DOMAIN_NET_MODEL_VIRTIO;
>
> Separate these by empty lines once you've broke up the 'else if'
> pattern.

When a bunch of capabilities are being checked one after the other in
first-one-wins fashion, I kinda find it more readable not to have
empty lines in between them, but I'm happy either way :)

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org