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

[Laine Stump]

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;
>   }
>