[PATCH 15/18] qemu: Improve qemuDomainDefaultNetModel()

Andrea Bolognani posted 18 patches 12 months ago
[PATCH 15/18] qemu: Improve qemuDomainDefaultNetModel()
Posted by Andrea Bolognani 12 months ago
Outside of x86, where we need to try pretty hard to pick hardware
that obsolete operating systems might be able to use, and Arm,
where some of the less virtualization-friendly machine types have
their own specific default, the sensible choice is virtio-net.

This is even more true for any architecture that's going to be
introduced in the future, as recent experience has shown that
they will be developed with virtualization in mind and use
virtio as extensively as possible.

Note that the default for ppc64 is changed as part of this.
That's technically a breaking change, but it should be safe to
apply since

  * it doesn't affect existing guests;
  * virt-manager already prefers virtio-net anyway.

rtl8139 was a pretty bad default for the architecture in the
first place, which was only applied accidentally due to its
existing status as default for x86.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_domain.c                               | 12 ++++--------
 .../ppc64-default-net.ppc64-latest.args              |  2 +-
 .../ppc64-default-net.ppc64-latest.xml               |  2 +-
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index de36641137..fac83e8bb7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5392,12 +5392,7 @@ static int
 qemuDomainDefaultNetModel(const virDomainDef *def,
                           virQEMUCaps *qemuCaps)
 {
-    if (ARCH_IS_S390(def->os.arch))
-        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;
 
@@ -5409,8 +5404,9 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
         return VIR_DOMAIN_NET_MODEL_LAN9118;
     }
 
-    /* virtio is a sensible default for RISC-V virt guests */
-    if (qemuDomainIsRISCVVirt(def))
+    /* For all remaining non-x86 architectures, virtio-net is a good
+     * default */
+    if (!ARCH_IS_X86(def->os.arch))
         return VIR_DOMAIN_NET_MODEL_VIRTIO;
 
     /* In all other cases the model depends on the capabilities. If they were
diff --git a/tests/qemuxml2argvdata/ppc64-default-net.ppc64-latest.args b/tests/qemuxml2argvdata/ppc64-default-net.ppc64-latest.args
index 4180c247d4..d1716817fb 100644
--- a/tests/qemuxml2argvdata/ppc64-default-net.ppc64-latest.args
+++ b/tests/qemuxml2argvdata/ppc64-default-net.ppc64-latest.args
@@ -27,7 +27,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
 -no-shutdown \
 -boot strict=on \
 -netdev user,id=hostnet0 \
--device '{"driver":"rtl8139","netdev":"hostnet0","id":"net0","mac":"52:54:00:09:a4:37","bus":"pci.0","addr":"0x1"}' \
+-device '{"driver":"virtio-net-pci","netdev":"hostnet0","id":"net0","mac":"52:54:00:09:a4:37","bus":"pci.0","addr":"0x1"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2xmloutdata/ppc64-default-net.ppc64-latest.xml b/tests/qemuxml2xmloutdata/ppc64-default-net.ppc64-latest.xml
index fa83b6e290..8b59dcaafd 100644
--- a/tests/qemuxml2xmloutdata/ppc64-default-net.ppc64-latest.xml
+++ b/tests/qemuxml2xmloutdata/ppc64-default-net.ppc64-latest.xml
@@ -24,7 +24,7 @@
     </controller>
     <interface type='user'>
       <mac address='52:54:00:09:a4:37'/>
-      <model type='rtl8139'/>
+      <model type='virtio'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
     </interface>
     <audio id='1' type='none'/>
-- 
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH 15/18] qemu: Improve qemuDomainDefaultNetModel()
Posted by Peter Krempa 12 months ago
On Wed, Jan 17, 2024 at 10:54:48 +0100, Andrea Bolognani wrote:
> Outside of x86, where we need to try pretty hard to pick hardware
> that obsolete operating systems might be able to use, and Arm,
> where some of the less virtualization-friendly machine types have
> their own specific default, the sensible choice is virtio-net.
> 
> This is even more true for any architecture that's going to be
> introduced in the future, as recent experience has shown that
> they will be developed with virtualization in mind and use
> virtio as extensively as possible.
> 
> Note that the default for ppc64 is changed as part of this.
> That's technically a breaking change, but it should be safe to
> apply since
> 
>   * it doesn't affect existing guests;
>   * virt-manager already prefers virtio-net anyway.
> 
> rtl8139 was a pretty bad default for the architecture in the
> first place, which was only applied accidentally due to its
> existing status as default for x86.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---

IMO it would be better to split this patch into the refactor portion,
which will explicitly keep ppc64 with the old default and then open the
debate about changing the default as another patch.

For the change itself, I understand why it would be beneficial but AFAIK
we never did this before. At the very least the PPC maintainers should
voice their opinion.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: Re: [PATCH 15/18] qemu: Improve qemuDomainDefaultNetModel()
Posted by Andrea Bolognani 12 months ago
On Wed, Jan 17, 2024 at 01:59:57PM +0100, Peter Krempa wrote:
> On Wed, Jan 17, 2024 at 10:54:48 +0100, Andrea Bolognani wrote:
> > Outside of x86, where we need to try pretty hard to pick hardware
> > that obsolete operating systems might be able to use, and Arm,
> > where some of the less virtualization-friendly machine types have
> > their own specific default, the sensible choice is virtio-net.
> >
> > This is even more true for any architecture that's going to be
> > introduced in the future, as recent experience has shown that
> > they will be developed with virtualization in mind and use
> > virtio as extensively as possible.
> >
> > Note that the default for ppc64 is changed as part of this.
> > That's technically a breaking change, but it should be safe to
> > apply since
> >
> >   * it doesn't affect existing guests;
> >   * virt-manager already prefers virtio-net anyway.
> >
> > rtl8139 was a pretty bad default for the architecture in the
> > first place, which was only applied accidentally due to its
> > existing status as default for x86.
>
> IMO it would be better to split this patch into the refactor portion,
> which will explicitly keep ppc64 with the old default and then open the
> debate about changing the default as another patch.

Yeah, that makes a lot of sense.

> For the change itself, I understand why it would be beneficial but AFAIK
> we never did this before.

Well, we've made changes that have effectively modified
architecture-specific defaults in the past, but checking back it was
mostly to fix scenarios where things wouldn't work at all, and we
also keyed those off capabilities.

So you're right, this is unusual and something that we might
ultimately decide not to do. I'd be okay with that. Though it's a
shame to have bad defaults in libvirt, what matters is that
higher-level tools are doing the right thing, and they already are.

> At the very least the PPC maintainers should
> voice their opinion.

We have a PPC maintainer in libvirt? :)

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