[libvirt] [PATCH] Fix netdev detection on RHEL6x versions of qemu
This allows the attach-device derived functions to work on the vanilla RHEL6 versions of qemu. Looking for the '-spice' parameter differentiates the RHEL from non-RHEL versions. Signed-off-by: Neil Wilson <neil@brightbox.co.uk> --- src/qemu/qemu_capabilities.c | 7 ++++--- tests/qemuhelptest.c | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 71a54a5..587de9e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -962,10 +962,11 @@ qemuCapsComputeCmdFlags(const char *help, qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE); if (strstr(help, "-netdev")) { - /* Disable -netdev on 0.12 since although it exists, - * the corresponding netdev_add/remove monitor commands + /* Disable -netdev on non-RHEL6 versions of 0.12 since although + * it exists,the corresponding netdev_add/remove monitor commands * do not, and we need them to be able todo hotplug */ - if (version >= 13000) + if ((version >= 13000) || + ((version >= 12001) && (strstr(help, "-spice")))) qemuCapsSet(flags, QEMU_CAPS_NETDEV); } diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 2522396..ceed35f 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -331,6 +331,7 @@ mymain(void) QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_BALLOON, QEMU_CAPS_DEVICE, + QEMU_CAPS_NETDEV, QEMU_CAPS_SMP_TOPOLOGY, QEMU_CAPS_RTC, QEMU_CAPS_VNET_HOST, @@ -454,6 +455,7 @@ mymain(void) QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_BALLOON, QEMU_CAPS_DEVICE, + QEMU_CAPS_NETDEV, QEMU_CAPS_SMP_TOPOLOGY, QEMU_CAPS_RTC, QEMU_CAPS_VNET_HOST, -- 1.7.4.1
On Tue, May 24, 2011 at 07:16:28AM +0100, Neil Wilson wrote:
This allows the attach-device derived functions to work on the vanilla RHEL6 versions of qemu. Looking for the '-spice' parameter differentiates the RHEL from non-RHEL versions.
Signed-off-by: Neil Wilson <neil@brightbox.co.uk> --- src/qemu/qemu_capabilities.c | 7 ++++--- tests/qemuhelptest.c | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 71a54a5..587de9e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -962,10 +962,11 @@ qemuCapsComputeCmdFlags(const char *help, qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE);
if (strstr(help, "-netdev")) { - /* Disable -netdev on 0.12 since although it exists, - * the corresponding netdev_add/remove monitor commands + /* Disable -netdev on non-RHEL6 versions of 0.12 since although + * it exists,the corresponding netdev_add/remove monitor commands * do not, and we need them to be able todo hotplug */ - if (version >= 13000) + if ((version >= 13000) || + ((version >= 12001) && (strstr(help, "-spice")))) qemuCapsSet(flags, QEMU_CAPS_NETDEV); }
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 2522396..ceed35f 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -331,6 +331,7 @@ mymain(void) QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_BALLOON, QEMU_CAPS_DEVICE, + QEMU_CAPS_NETDEV, QEMU_CAPS_SMP_TOPOLOGY, QEMU_CAPS_RTC, QEMU_CAPS_VNET_HOST, @@ -454,6 +455,7 @@ mymain(void) QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_BALLOON, QEMU_CAPS_DEVICE, + QEMU_CAPS_NETDEV, QEMU_CAPS_SMP_TOPOLOGY, QEMU_CAPS_RTC, QEMU_CAPS_VNET_HOST,
IMHO RHEL specific hacks don't belong in upstream libvirt code. 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 Tue, 2011-05-24 at 10:10 +0100, Daniel P. Berrange wrote:
IMHO RHEL specific hacks don't belong in upstream libvirt code.
Currently there are checks against the qemu-kvm RHEL6 help files in the test system of libvirt, and it is incorrect - as RHEL6 qemu uses netdev, not add_host. So why are you testing against the 'RHEL hacked' version of qemu if that doesn't belong in upstream code? Fundamentally if you install an up to date libvirt onto a RHEL6 machine, the attach-interface code fails as there is no 'add_host' functionality in the qemu version on that server. RHEL6 qemu uses netdev. Attach-interface doesn't work when libvirt is compiled on RHEL6. You have my proposal for fixing it. What is the alternative? Rgs NeilW
On Tue, May 24, 2011 at 10:31:19AM +0100, Neil Wilson wrote:
On Tue, 2011-05-24 at 10:10 +0100, Daniel P. Berrange wrote:
IMHO RHEL specific hacks don't belong in upstream libvirt code.
Currently there are checks against the qemu-kvm RHEL6 help files in the test system of libvirt, and it is incorrect - as RHEL6 qemu uses netdev, not add_host.
So why are you testing against the 'RHEL hacked' version of qemu if that doesn't belong in upstream code?
Fundamentally if you install an up to date libvirt onto a RHEL6 machine, the attach-interface code fails as there is no 'add_host' functionality in the qemu version on that server. RHEL6 qemu uses netdev.
Attach-interface doesn't work when libvirt is compiled on RHEL6. You have my proposal for fixing it. What is the alternative?
Use the libvirt that ships with RHEL6, or apply the RHEL6 specific patches manually when building an alternative libvirt for RHEL6. It isn't sustainable for upstream projects to deal with hacks for every crazy non-upstream change that distros make. If distros choose to change things in ways that diverge from upstream behaviour, then it is their job to maintain the fixes for other code which breaks as a result. 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 Tue, 2011-05-24 at 10:37 +0100, Daniel P. Berrange wrote:
Use the libvirt that ships with RHEL6, or apply the RHEL6 specific patches manually when building an alternative libvirt for RHEL6. It isn't sustainable for upstream projects to deal with hacks for every crazy non-upstream change that distros make. If distros choose to change things in ways that diverge from upstream behaviour, then it is their job to maintain the fixes for other code which breaks as a result.
That still doesn't explain why the libvirt test system is checking the 'non-standard' versions of RHEL6 qemu then. Why is that code there if you are not supporting that 'hack'? Rgs NeilW
On 05/24/2011 03:42 AM, Neil Wilson wrote:
On Tue, 2011-05-24 at 10:37 +0100, Daniel P. Berrange wrote:
Use the libvirt that ships with RHEL6, or apply the RHEL6 specific patches manually when building an alternative libvirt for RHEL6. It isn't sustainable for upstream projects to deal with hacks for every crazy non-upstream change that distros make. If distros choose to change things in ways that diverge from upstream behaviour, then it is their job to maintain the fixes for other code which breaks as a result.
That still doesn't explain why the libvirt test system is checking the 'non-standard' versions of RHEL6 qemu then.
If something is directly observable in -help output, then we can key off of it. But keying off of -spice in order to detect whether -netdev works seems fishy - you should key off of direct features, rather than special-casing knowledge that one orthogonal feature "implies" another. Ultimately, the nicest fix might be to use the 'help' monitor command to probe exactly which other monitor commands are available, to learn whether the monitor supports netdev_add. But how do you probe the monitor commands until you already started the qemu command line with -netdev already in the command line? Another possible fix would be to have some way for 'qemu -help' to output the list of supported monitor commands, then get that backported into RHEL qemu. But right now, Dan's advice of using the RHEL-specific libvirt patches when building libvirt on RHEL, if you want to take full advantage of RHEL qemu, makes the most sense to me. Meanwhile, as to your question about the libvirt testsuit including RHEL qemu -help output: that merely proves that libvirt is properly parsing the available -help output, and not that libvirt is inferring any special properties of RHEL qemu that were not directly advertised in -help. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Wed, 2011-06-01 at 13:38 -0600, Eric Blake wrote:
Meanwhile, as to your question about the libvirt testsuit including RHEL qemu -help output: that merely proves that libvirt is properly parsing the available -help output, and not that libvirt is inferring any special properties of RHEL qemu that were not directly advertised in -help.
I appreciate your input but I feel that it suffers from the same problem you complain of. Libvirt is not properly parsing the help message as the decisions it makes means that the attach-device derived functions do not work with the qemu supplied on a major platform. The actual fault is with 12.x standard qemu advertising netdev when it doesn't have it. Libvirt is using 'special properties not directly advertised in -help' to infer that 12.x netdev is broken. So really you're special casing the standard qemu, and actually you'd probably be better removing that special case altogether and letting the standard qemu fail on attach (which frankly less people will be using), and feeding the problem upstream to the qemu developer to fix the 12.x branch help message. I'll leave it with you guys at that. The patch is on the list for anybody suffering the same issues I am. NeilW
On Thu, Jun 02, 2011 at 10:04:44AM +0100, Neil Wilson wrote:
On Wed, 2011-06-01 at 13:38 -0600, Eric Blake wrote:
Meanwhile, as to your question about the libvirt testsuit including RHEL qemu -help output: that merely proves that libvirt is properly parsing the available -help output, and not that libvirt is inferring any special properties of RHEL qemu that were not directly advertised in -help.
I appreciate your input but I feel that it suffers from the same problem you complain of.
Libvirt is not properly parsing the help message as the decisions it makes means that the attach-device derived functions do not work with the qemu supplied on a major platform.
The actual fault is with 12.x standard qemu advertising netdev when it doesn't have it. Libvirt is using 'special properties not directly advertised in -help' to infer that 12.x netdev is broken.
12.x does have netdev, but we don't want to use it because it does not support hotplug, only with initial startup. Hence it is better to continue using the old network syntax to avoid loosing functionality 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 :|
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Neil Wilson