Re: [PATCH v2 2/2] qemu: Default to virtio-net where available

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/02/2015 12:14 PM, Andrea Bolognani wrote:
This applies to all architectures except for ARM, which already
has its own logic to pick the best default.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1254044
---
Changes from v1:

   * make sure virtio-net is available using capabilities
     instead of blindly using it (thanks Martin)

   * change the default for all architectures (except arm)
     instead of just ppc64

Are we certain we want to do this even for x86 guests? I don't think it's a good idea - it makes the default into something for which no driver exists on the install media for *many* x86 guests, meaning it is highly likely that a "default" config would have non-functional networking. AFAIR this is why we didn't make virtio the default several years ago when we began recording a default into the XML (and why the last time changing the default was discussed, I believe it was pointing more towards something like e1000, i.e. something which 1) we are certain has a driver on every guest OS installation media that might be found, 2) is better maintained in qemu than the rtl8139, and 3) performs better than rtl8139 (although obviously not as good as virtio).

(I saw the original message from Martin suggesting this and meant to respond then, but managed to forget before I finished what I was doing).


  src/qemu/qemu_domain.c | 19 ++++++++++++++-----
  1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0a9ed6b..8f4efd3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1193,11 +1193,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
  }
static const char *
-qemuDomainDefaultNetModel(const virDomainDef *def)
+qemuDomainDefaultNetModel(const virDomainDef *def,
+                          virQEMUCapsPtr qemuCaps)
  {
-    if (ARCH_IS_S390(def->os.arch))
-        return "virtio";
-
      if (def->os.arch == VIR_ARCH_ARMV7L ||
          def->os.arch == VIR_ARCH_AARCH64) {
          if (STREQ(def->os.machine, "versatilepb"))
@@ -1211,6 +1209,10 @@ qemuDomainDefaultNetModel(const virDomainDef *def)
          return "lan9118";
      }
+ /* virtio should always be used where available */
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_NET))
+        return "virtio";
+
      return "rtl8139";
  }
@@ -1223,15 +1225,21 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
      int ret = -1;
      virQEMUDriverPtr driver = opaque;
      virQEMUDriverConfigPtr cfg = NULL;
+    virQEMUCapsPtr qemuCaps = NULL;
if (driver)
          cfg = virQEMUDriverGetConfig(driver);
+ /* This condition is actually a (temporary) hack for test suite which
+     * does not create capabilities cache */
+    if (driver && driver->qemuCapsCache)
+        qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
+
      if (dev->type == VIR_DOMAIN_DEVICE_NET &&
          dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
          !dev->data.net->model) {
          if (VIR_STRDUP(dev->data.net->model,
-                       qemuDomainDefaultNetModel(def)) < 0)
+                       qemuDomainDefaultNetModel(def, qemuCaps)) < 0)
              goto cleanup;
      }
@@ -1358,6 +1366,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, cleanup:
      virObjectUnref(cfg);
+    virObjectUnref(qemuCaps);
      return ret;
  }

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]