On Thu, May 20, 2010 at 03:39:05PM +0200, Jim Meyering wrote: > Chris Wright wrote: > > * Eric Blake (eblake@xxxxxxxxxx) 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@xxxxxxxxxx> > > --- > > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list