[libvirt] [PATCH v2 0/2] qemu: Default to virtio-net where available
This new version of the patch addresses Martin's comments by using capabilities to detect whether virtio-net can be used, and if that is the case, does so on all architectures. Plus, the commit messages are slightly less terse this time around :) Cheers. Andrea Bolognani (2): qemu: Introduce QEMU_CAPS_DEVICE_VIRTIO_NET capability qemu: Default to virtio-net where available src/qemu/qemu_capabilities.c | 7 ++++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 19 ++++++++++++++----- tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 18 ++++++++++++------ 11 files changed, 40 insertions(+), 12 deletions(-) -- 2.4.3
As the name suggests, this capability can be used to check whether a QEMU binary supports the virtio-net-* devices. --- src/qemu/qemu_capabilities.c | 7 ++++++- src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemuhelptest.c | 18 ++++++++++++------ 10 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 43d11af..7bb6819 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -291,7 +291,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "gpex-pcihost", "ioh3420", "x3130-upstream", - "xio3130-downstream", + + "xio3130-downstream", /* 195 */ + "virtio-net", ); @@ -1576,6 +1578,9 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "ioh3420", QEMU_CAPS_DEVICE_IOH3420 }, { "x3130-upstream", QEMU_CAPS_DEVICE_X3130_UPSTREAM }, { "xio3130-downstream", QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM }, + { "virtio-net-pci", QEMU_CAPS_DEVICE_VIRTIO_NET }, + { "virtio-net-ccw", QEMU_CAPS_DEVICE_VIRTIO_NET }, + { "virtio-net-s390", QEMU_CAPS_DEVICE_VIRTIO_NET }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2d32d6e..94ba0ba 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -235,6 +235,7 @@ typedef enum { QEMU_CAPS_DEVICE_IOH3420 = 193, /* -device ioh3420 */ QEMU_CAPS_DEVICE_X3130_UPSTREAM = 194, /* -device x3130-upstream */ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM = 195, /* -device xio3130-downstream */ + QEMU_CAPS_DEVICE_VIRTIO_NET = 196, /* -device virtio-net-* */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index ba16635..a28cf47 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -123,4 +123,5 @@ <flag name='ioh3420'/> <flag name='x3130-upstream'/> <flag name='xio3130-downstream'/> + <flag name='virtio-net'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index 51cd6d9..cfe68a6 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -138,4 +138,5 @@ <flag name='ioh3420'/> <flag name='x3130-upstream'/> <flag name='xio3130-downstream'/> + <flag name='virtio-net'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index 03d0a3e..5b51710 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -139,4 +139,5 @@ <flag name='ioh3420'/> <flag name='x3130-upstream'/> <flag name='xio3130-downstream'/> + <flag name='virtio-net'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index e2f22e4..c0c72cf 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -148,4 +148,5 @@ <flag name='ioh3420'/> <flag name='x3130-upstream'/> <flag name='xio3130-downstream'/> + <flag name='virtio-net'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 874a050..1b1f7d7 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -154,4 +154,5 @@ <flag name='ioh3420'/> <flag name='x3130-upstream'/> <flag name='xio3130-downstream'/> + <flag name='virtio-net'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index dd3bcda..e459f83 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -154,4 +154,5 @@ <flag name='ioh3420'/> <flag name='x3130-upstream'/> <flag name='xio3130-downstream'/> + <flag name='virtio-net'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 3ee2d6f..ba19550 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -170,4 +170,5 @@ <flag name='ioh3420'/> <flag name='x3130-upstream'/> <flag name='xio3130-downstream'/> + <flag name='virtio-net'/> </qemuCaps> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 8f317d4..c17b31b 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -409,7 +409,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DEVICE_USB_KBD, - QEMU_CAPS_DEVICE_PCI_BRIDGE); + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_VIRTIO_NET); DO_TEST("qemu-kvm-0.12.3", 12003, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -524,7 +525,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_KBD, QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_HOST_PCI_MULTIDOMAIN, - QEMU_CAPS_DEVICE_IVSHMEM); + QEMU_CAPS_DEVICE_IVSHMEM, + QEMU_CAPS_DEVICE_VIRTIO_NET); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -591,7 +593,8 @@ mymain(void) QEMU_CAPS_DEVICE_USB_SERIAL, QEMU_CAPS_DEVICE_USB_NET, QEMU_CAPS_DEVICE_USB_KBD, - QEMU_CAPS_DEVICE_PCI_BRIDGE); + QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_VIRTIO_NET); DO_TEST("qemu-kvm-0.12.1.2-rhel62-beta", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -666,7 +669,8 @@ mymain(void) QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_USB_KBD, - QEMU_CAPS_DEVICE_USB_STORAGE); + QEMU_CAPS_DEVICE_USB_STORAGE, + QEMU_CAPS_DEVICE_VIRTIO_NET); DO_TEST("qemu-1.0", 1000000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -756,7 +760,8 @@ mymain(void) QEMU_CAPS_DEVICE_IVSHMEM, QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_X3130_UPSTREAM, - QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM); + QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM, + QEMU_CAPS_DEVICE_VIRTIO_NET); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -859,7 +864,8 @@ mymain(void) QEMU_CAPS_DEVICE_IVSHMEM, QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_X3130_UPSTREAM, - QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM); + QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM, + QEMU_CAPS_DEVICE_VIRTIO_NET); DO_TEST_FULL("qemu-1.2.0", 1002000, 0, 0, VIR_ERR_CONFIG_UNSUPPORTED, QEMU_CAPS_LAST); DO_TEST_FULL("qemu-kvm-1.2.0", 1002000, 1, 0, VIR_ERR_CONFIG_UNSUPPORTED, -- 2.4.3
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 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; } -- 2.4.3
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; }
On Wed, Sep 02, 2015 at 04:08:52PM -0400, Laine Stump wrote:
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).
Agreed, making virtio-net the default is a bad idea because the guest OS support for it is pretty limited to essentially just modern Linux guests out of the box. So nack to this patch. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Sep 03, 2015 at 09:36:47AM +0100, Daniel P. Berrange wrote:
On Wed, Sep 02, 2015 at 04:08:52PM -0400, Laine Stump wrote:
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).
Agreed, making virtio-net the default is a bad idea because the guest OS support for it is pretty limited to essentially just modern Linux guests out of the box. So nack to this patch.
Oh and I meant to say, the task of getting "optimal" defaults for the various guest OS really falls to libosinfo, which has info on what each OS is capable of using. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, 2015-09-02 at 16:08 -0400, Laine Stump wrote:
* 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).
Good point. We could switch the default to virtio just for ppc64 then, leaving the capability check in place of course, and change the global default from rtl8139 to e1000 in a separate series. How does this sound? Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Thu, Sep 03, 2015 at 10:48:58AM +0200, Andrea Bolognani wrote:
On Wed, 2015-09-02 at 16:08 -0400, Laine Stump wrote:
* 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).
Good point. We could switch the default to virtio just for ppc64 then, leaving the capability check in place of course, and change the global default from rtl8139 to e1000 in a separate series.
I think we should just do nothing. If people/apps care about optimal defaults, nothing libvirt changes will make them happy. Apps need to be just using libosinfo to get the sensible defaults on a per OS basis. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Sep 03, 2015 at 09:53:26AM +0100, Daniel P. Berrange wrote:
On Thu, Sep 03, 2015 at 10:48:58AM +0200, Andrea Bolognani wrote:
On Wed, 2015-09-02 at 16:08 -0400, Laine Stump wrote:
* 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).
Good point. We could switch the default to virtio just for ppc64 then, leaving the capability check in place of course, and change the global default from rtl8139 to e1000 in a separate series.
I think we should just do nothing. If people/apps care about optimal defaults, nothing libvirt changes will make them happy. Apps need to be just using libosinfo to get the sensible defaults on a per OS basis.
OK, so maybe at least using virtio instead of rtl8139 only in case rtl8139 is not available.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, 2015-09-03 at 09:53 +0100, Daniel P. Berrange wrote:
I think we should just do nothing. If people/apps care about optimal defaults, nothing libvirt changes will make them happy. Apps need to be just using libosinfo to get the sensible defaults on a per OS basis.
This is not about optimal defaults though, it's about being able to install the OS at all without having to explicitly specify that you want to use virtio: $ sudo virt-install \ [...] \ --os-variant=rhel7.0 \ --network network=default ERROR internal error: early end of file from monitor: possible problem: 2015-09-03T09:00:42.154356Z qemu-kvm: -device rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:d4:d8:8f,\ bus=pci.0,addr=0x1: 'rtl8139' is not a valid device model name This happens because we assume rtl8139 will be available, but that's not the case eg. on RHEL ppc64. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Thu, Sep 03, 2015 at 11:12:23AM +0200, Andrea Bolognani wrote:
On Thu, 2015-09-03 at 09:53 +0100, Daniel P. Berrange wrote:
I think we should just do nothing. If people/apps care about optimal defaults, nothing libvirt changes will make them happy. Apps need to be just using libosinfo to get the sensible defaults on a per OS basis.
This is not about optimal defaults though, it's about being able to install the OS at all without having to explicitly specify that you want to use virtio:
$ sudo virt-install \ [...] \ --os-variant=rhel7.0 \ --network network=default
ERROR internal error: early end of file from monitor: possible problem: 2015-09-03T09:00:42.154356Z qemu-kvm: -device rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:d4:d8:8f,\ bus=pci.0,addr=0x1: 'rtl8139' is not a valid device model name
This happens because we assume rtl8139 will be available, but that's not the case eg. on RHEL ppc64.
Right, but changing upstream to default to virtio, just because RHEL downstream disabled other devices is not right. Either upstream will need to dynamically change if it finds the device not available, or downstream RHEL libvirt should have a downstream only patch to workaround the RHEL QEMU PPC64 changes. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, 2015-09-03 at 10:17 +0100, Daniel P. Berrange wrote:
This happens because we assume rtl8139 will be available, but that's not the case eg. on RHEL ppc64.
Right, but changing upstream to default to virtio, just because RHEL downstream disabled other devices is not right.
Just assuming rtl8139 will be available, as we're doing right now, is not very nice either :)
Either upstream will need to dynamically change if it finds the device not available, or downstream RHEL libvirt should have a downstream only patch to workaround the RHEL QEMU PPC64 changes.
Yeah, Martin suggested doing something similar to the first option as well. Let's just probe for a bunch of network devices and use the first one that's available, okay? 1. rtl8139 2. e1000 3. virtio-net Any other we should try? Any drawbacks to this approach? Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (4)
-
Andrea Bolognani -
Daniel P. Berrange -
Laine Stump -
Martin Kletzander