[libvirt] [PATCH] qemu driver: fix version parsing fix

[Daniel P. Berrange]

On Thu, May 20, 2010 at 03:39:05PM +0200, Jim Meyering wrote:
> Chris Wright wrote:
> > * Eric Blake (eblake at 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 at 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 at 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 at 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 :|