On Thu, May 20, 2010 at 03:39:05PM +0200, Jim Meyering wrote:
Chris Wright wrote:
> * Eric Blake (eblake(a)redhat.com) wrote:
>> On 05/19/2010 04:37 PM, Chris Wright wrote:
>> > Looks like some cut'n paste error to me.
>>
>> While we're at it, there have been some complaints, at least on IRC,
>> that some recent qemu builds changed -help output to start with "QEMU
>> emulator version" instead of "QEMU PC emulator version", which
fails to
>> match QEMU_VERSION_STR. Is that something we should be worrying about,
>> while we're touching this function?
>
> That's what had me looking at it ;)
>
> [chrisw@x200 qemu-kvm]$ ./x86_64-softmmu/qemu-system-x86_64 -help | head -1
> QEMU emulator version 0.12.50 (qemu-kvm-devel), Copyright (c) 2003-2008 Fabrice
Bellard
>
> [chrisw@x200 qemu-kvm]$ qemu-kvm -help | head -1
> QEMU PC emulator version 0.11.0 (qemu-kvm-0.11.0), Copyright (c) 2003-2008 Fabrice
Bellard
>
> This keys off of only "emulator version", so should not have the
> same parsing issue.
>
> Signed-off-by: Chris Wright <chrisw(a)redhat.com>
> ---
> src/qemu/qemu_conf.c | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 5fa8c0a..359c311 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1247,6 +1247,8 @@ static unsigned long long qemudComputeCmdFlags(const char
*help,
> /* We parse the output of 'qemu -help' to get the QEMU
> * version number. The first bit is easy, just parse
> * 'QEMU PC emulator version x.y.z'.
> + * or
> + * 'QEMU emulator version x.y.z'.
> *
> * With qemu-kvm, however, that is followed by a string
> * in parenthesis as follows:
> @@ -1259,7 +1261,7 @@ static unsigned long long qemudComputeCmdFlags(const char
*help,
> * and later, we just need the QEMU version number and
> * whether it is KVM QEMU or mainline QEMU.
> */
> -#define QEMU_VERSION_STR "QEMU PC emulator version"
> +#define QEMU_VERSION_STR "emulator version"
> #define QEMU_KVM_VER_PREFIX "(qemu-kvm-"
> #define KVM_VER_PREFIX "(kvm-"
>
> @@ -1277,9 +1279,10 @@ int qemudParseHelpStr(const char *qemu,
>
> *flags = *version = *is_kvm = *kvm_version = 0;
>
> - if (!STRPREFIX(p, QEMU_VERSION_STR))
> + p = strstr(p, QEMU_VERSION_STR);
> + if (!p)
> goto fail;
> p += strlen(QEMU_VERSION_STR);
Hi Chris,
That patch looks fine, and is nicely minimal.
However, it does relax the test significantly.
Rather than requiring that QEMU_VERSION_STR be a prefix,
it would allow it to appear anywhere within the -help output.
While I really doubt it'd ever make a difference,
it's easy to retain the stricter test, so I wrote this.
Either way is fine with me.
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5c14eb8..f43c6e2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1259,7 +1259,8 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
* and later, we just need the QEMU version number and
* whether it is KVM QEMU or mainline QEMU.
*/
-#define QEMU_VERSION_STR "QEMU PC emulator version"
+#define QEMU_VERSION_STR_1 "QEMU PC emulator version"
+#define QEMU_VERSION_STR_2 "QEMU emulator version"
#define QEMU_KVM_VER_PREFIX "(qemu-kvm-"
#define KVM_VER_PREFIX "(kvm-"
@@ -1277,11 +1278,13 @@ int qemudParseHelpStr(const char *qemu,
*flags = *version = *is_kvm = *kvm_version = 0;
- if (!STRPREFIX(p, QEMU_VERSION_STR))
+ if (STRPREFIX(p, QEMU_VERSION_STR_1))
+ p += strlen(QEMU_VERSION_STR_1);
+ else if (STRPREFIX(p, QEMU_VERSION_STR_2))
+ p += strlen(QEMU_VERSION_STR_2);
+ else
goto fail;
- p += strlen(QEMU_VERSION_STR);
-
SKIP_BLANKS(p);
major = virParseNumber(&p);
ACK, prefer the stricter check. One day QEMU might even provide this in a
reliably parsable format like JSON...
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://deltacloud.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|