Re: [libvirt] [PATCH v2 2/2] qemu: Default to virtio-net where available
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;
}