[libvirt] [PATCH] qemu: Fix parsing of x86 CPU models

QEMU changed the output of -cpu ? and CPU models can now be followed by its description. Our parser just used both model name and its description as a model name, which made any model with a description unusable with libvirt. --- src/qemu/qemu_capabilities.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a5eb995..d16a7bc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -379,7 +379,7 @@ typedef int qemuCapsPtr caps); /* Format: - * <arch> <model> + * <arch> <model> <description> * qemu-0.13 encloses some model names in []: * <arch> [<model>] */ @@ -389,6 +389,7 @@ qemuCapsParseX86Models(const char *output, { const char *p = output; const char *next; + const char *end; int ret = -1; do { @@ -416,8 +417,12 @@ qemuCapsParseX86Models(const char *output, goto cleanup; } - if (next) - len = next - p - 1; + end = next ? next - 1 : NULL; + if ((t = strchr(p, ' ')) && (!next || t < next)) + end = t; + + if (end) + len = end - p; else len = strlen(p); -- 1.7.12

On Mon, Oct 08, 2012 at 04:30:03PM +0200, Jiri Denemark wrote:
QEMU changed the output of -cpu ? and CPU models can now be followed by its description. Our parser just used both model name and its description as a model name, which made any model with a description unusable with libvirt.
What version of QEMU changed this ? 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 Mon, Oct 08, 2012 at 15:35:38 +0100, Daniel P. Berrange wrote:
On Mon, Oct 08, 2012 at 04:30:03PM +0200, Jiri Denemark wrote:
QEMU changed the output of -cpu ? and CPU models can now be followed by its description. Our parser just used both model name and its description as a model name, which made any model with a description unusable with libvirt.
What version of QEMU changed this ?
Hmm, I think I see why you are asking... I just checked and the changset that introduced this is v1.2.0-266-g6cdf885, with which we will use QMP probing. Thus, we don't need this code for working with upstream qemu. (Not that this would be the first case of committing a patch that makes libvirt work better with downstream qemu with backported patches.) Jirka

On Mon, Oct 08, 2012 at 04:48:00PM +0200, Jiri Denemark wrote:
On Mon, Oct 08, 2012 at 15:35:38 +0100, Daniel P. Berrange wrote:
On Mon, Oct 08, 2012 at 04:30:03PM +0200, Jiri Denemark wrote:
QEMU changed the output of -cpu ? and CPU models can now be followed by its description. Our parser just used both model name and its description as a model name, which made any model with a description unusable with libvirt.
What version of QEMU changed this ?
Hmm, I think I see why you are asking... I just checked and the changset that introduced this is v1.2.0-266-g6cdf885, with which we will use QMP probing. Thus, we don't need this code for working with upstream qemu. (Not that this would be the first case of committing a patch that makes libvirt work better with downstream qemu with backported patches.)
My feeling is that we came to agreement with the QEMU developers that we would exclusively useful QMP probing with any QEMU from 1.2.0 or later and would not try to follow changes they make to the non-QMP interfaces we previously relied on. So if there are any changes to -help, -cpu, -M or -device output from 1.2.0 onwards, we really should not try to follow them in libvirt master. I could see this patch being acceptable for our newest stable release stream branch though. Traditionally our requriement was that patches for stable should go in master first, but I think this scenario is an example where we should relax that rule and put the change in the stable branch only. 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 Mon, Oct 08, 2012 at 15:55:09 +0100, Daniel P. Berrange wrote:
My feeling is that we came to agreement with the QEMU developers that we would exclusively useful QMP probing with any QEMU from 1.2.0 or later and would not try to follow changes they make to the non-QMP interfaces we previously relied on. So if there are any changes to -help, -cpu, -M or -device output from 1.2.0 onwards, we really should not try to follow them in libvirt master.
I could see this patch being acceptable for our newest stable release stream branch though. Traditionally our requriement was that patches for stable should go in master first, but I think this scenario is an example where we should relax that rule and put the change in the stable branch only.
Although it will only help if someone updates QEMU while staying on a stable libvirt branch. I'm not saying it's a bad idea, though :-) Can I take this as an ACK for pushing into the stable branch? Jirka

On Mon, Oct 08, 2012 at 05:08:00PM +0200, Jiri Denemark wrote:
On Mon, Oct 08, 2012 at 15:55:09 +0100, Daniel P. Berrange wrote:
My feeling is that we came to agreement with the QEMU developers that we would exclusively useful QMP probing with any QEMU from 1.2.0 or later and would not try to follow changes they make to the non-QMP interfaces we previously relied on. So if there are any changes to -help, -cpu, -M or -device output from 1.2.0 onwards, we really should not try to follow them in libvirt master.
I could see this patch being acceptable for our newest stable release stream branch though. Traditionally our requriement was that patches for stable should go in master first, but I think this scenario is an example where we should relax that rule and put the change in the stable branch only.
Although it will only help if someone updates QEMU while staying on a stable libvirt branch. I'm not saying it's a bad idea, though :-) Can I take this as an ACK for pushing into the stable branch?
Hmm, now that I've said this, I have a strong feeling that this is a lost cause because other changes to QEMU master are going to break compatibility with the libvirt stable branch soon enough (if not already). Perhaps we should just update drvqemu.html to clearly indicate that if you have QEMU >= 1.3.0 you need to have libvirt >= 1.0.0. 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 (2)
-
Daniel P. Berrange
-
Jiri Denemark