[libvirt] [PATCH] qemu: QMP capabilities support starts with 1.2

Per the code comment in qemuCapsInitQMPBasic() and Eric, we should only use QMP for capabilities probing starting with 1.2 and newer. --- src/qemu/qemu_capabilities.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 271273c..64c0bea 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2314,7 +2314,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package)); - if (!(major >= 1 || (major == 1 && minor >= 1))) { + if (!(major >= 1 || (major == 1 && minor >= 2))) { VIR_DEBUG("Not new enough for QMP capabilities detection"); ret = 0; goto cleanup; -- 1.7.8.6

On Thu, Nov 1, 2012 at 5:15 PM, Doug Goldstein <cardoe@cardoe.com> wrote:
Per the code comment in qemuCapsInitQMPBasic() and Eric, we should only use QMP for capabilities probing starting with 1.2 and newer. --- src/qemu/qemu_capabilities.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 271273c..64c0bea 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2314,7 +2314,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package));
- if (!(major >= 1 || (major == 1 && minor >= 1))) { + if (!(major >= 1 || (major == 1 && minor >= 2))) { VIR_DEBUG("Not new enough for QMP capabilities detection"); ret = 0; goto cleanup; -- 1.7.8.6
Ignore this. I had a complete and utter brain melt down and committed what I was tinkering with. -- Doug Goldstein

On 11/01/2012 04:22 PM, Doug Goldstein wrote:
On Thu, Nov 1, 2012 at 5:15 PM, Doug Goldstein <cardoe@cardoe.com> wrote:
Per the code comment in qemuCapsInitQMPBasic() and Eric, we should only use QMP for capabilities probing starting with 1.2 and newer. --- src/qemu/qemu_capabilities.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 271273c..64c0bea 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2314,7 +2314,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package));
- if (!(major >= 1 || (major == 1 && minor >= 1))) { + if (!(major >= 1 || (major == 1 && minor >= 2))) { VIR_DEBUG("Not new enough for QMP capabilities detection"); ret = 0; goto cleanup; -- 1.7.8.6
Ignore this. I had a complete and utter brain melt down and committed what I was tinkering with.
Actually, your patch looks like it makes sense, and should get in before the 1.0.0 release. ACK. I hate the negative logic though; I'd almost rather see: if (major < 1 || (major == 1 && minor < 2)) not new enough -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 11/01/2012 04:30 PM, Eric Blake wrote:
On 11/01/2012 04:22 PM, Doug Goldstein wrote:
On Thu, Nov 1, 2012 at 5:15 PM, Doug Goldstein <cardoe@cardoe.com> wrote:
Per the code comment in qemuCapsInitQMPBasic() and Eric, we should only use QMP for capabilities probing starting with 1.2 and newer. --- src/qemu/qemu_capabilities.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 271273c..64c0bea 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2314,7 +2314,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, VIR_DEBUG("Got version %d.%d.%d (%s)", major, minor, micro, NULLSTR(package));
- if (!(major >= 1 || (major == 1 && minor >= 1))) { + if (!(major >= 1 || (major == 1 && minor >= 2))) {
Your change is a no-op. If major >= 1, then the second arm of the || is irrelevant. If major < 1, then the second arm of the || is always false because major != 1, without ever looking at minor.
Ignore this. I had a complete and utter brain melt down and committed what I was tinkering with.
Actually, your patch looks like it makes sense, and should get in before the 1.0.0 release.
ACK.
I hate the negative logic though; I'd almost rather see:
if (major < 1 || (major == 1 && minor < 2))
This, however, IS what what we want, so I modified your commit, added my sign-off, and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Doug Goldstein
-
Eric Blake