diff mbox

[1/1] Add -version-simple argument, printing only version number.

Message ID 1273739572-30840-2-git-send-email-Jes.Sorensen@redhat.com
State New
Headers show

Commit Message

Jes Sorensen May 13, 2010, 8:32 a.m. UTC
From: Jes Sorensen <Jes.Sorensen@redhat.com>

Add -version-simple argument for QEMU, printing just the version
number, without any supporting text.

This makes it simpler for other apps, such as libvirt, to parse the
version string from QEMU independant of how the naming string may
change.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qemu-options.hx |    8 ++++++++
 vl.c            |    9 +++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

Comments

Daniel P. Berrangé May 13, 2010, 1:33 p.m. UTC | #1
On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Add -version-simple argument for QEMU, printing just the version
> number, without any supporting text.
> 
> This makes it simpler for other apps, such as libvirt, to parse the
> version string from QEMU independant of how the naming string may
> change.
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  qemu-options.hx |    8 ++++++++
>  vl.c            |    9 +++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 12f6b51..e4f3979 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -27,6 +27,14 @@ STEXI
>  Display version information and exit
>  ETEXI
>  
> +DEF("version-simple", 0, QEMU_OPTION_version_simple,
> +    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
> +STEXI
> +@item -version-simple
> +@findex -version-simple
> +Display basic version number information and exit
> +ETEXI
> +
>  DEF("M", HAS_ARG, QEMU_OPTION_M,
>      "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
>  STEXI
> diff --git a/vl.c b/vl.c
> index 85bcc84..5adca87 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2015,6 +2015,11 @@ static void version(void)
>      printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
>  }
>  
> +static void version_simple(void)
> +{
> +    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
> +}
> +
>  static void help(int exitcode)
>  {
>      const char *options_help =
> @@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>                  version();
>                  exit(0);
>                  break;
> +            case QEMU_OPTION_version_simple:
> +                version_simple();
> +                exit(0);
> +                break;
>              case QEMU_OPTION_m: {
>                  uint64_t value;
>                  char *ptr;

This omits the KVM version string which is something we also want to see.
It would also be nice to avoid having to parse the -help output to determine
ARGV supported too. I wonder if it would be a good idea to just produce a
well structured equivalent to -help that provides the same data, but in 
JSON format for sane parsing. That would let peple easily determine the
supported ARGV as well as version number(s)

Regards,
Daniel
Jes Sorensen May 13, 2010, 1:41 p.m. UTC | #2
On 05/13/10 15:33, Daniel P. Berrange wrote:
> This omits the KVM version string which is something we also want to see.
> It would also be nice to avoid having to parse the -help output to determine
> ARGV supported too. I wonder if it would be a good idea to just produce a
> well structured equivalent to -help that provides the same data, but in 
> JSON format for sane parsing. That would let peple easily determine the
> supported ARGV as well as version number(s)

I just made it print the version numbers that were already part of the
regular string, but I see no problem adding a KVM version number to the
output as well.

As for the JSON stuff, then I'll volunteer you to write the patch for it
:) To be honest, I have no clue what JSON is!

Cheers,
Jes
Blue Swirl May 13, 2010, 7:30 p.m. UTC | #3
On 5/13/10, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
>  > From: Jes Sorensen <Jes.Sorensen@redhat.com>
>  >
>  > Add -version-simple argument for QEMU, printing just the version
>  > number, without any supporting text.
>  >
>  > This makes it simpler for other apps, such as libvirt, to parse the
>  > version string from QEMU independant of how the naming string may
>  > change.
>  >
>  > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>  > ---
>  >  qemu-options.hx |    8 ++++++++
>  >  vl.c            |    9 +++++++++
>  >  2 files changed, 17 insertions(+), 0 deletions(-)
>  >
>  > diff --git a/qemu-options.hx b/qemu-options.hx
>  > index 12f6b51..e4f3979 100644
>  > --- a/qemu-options.hx
>  > +++ b/qemu-options.hx
>  > @@ -27,6 +27,14 @@ STEXI
>  >  Display version information and exit
>  >  ETEXI
>  >
>  > +DEF("version-simple", 0, QEMU_OPTION_version_simple,
>  > +    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
>  > +STEXI
>  > +@item -version-simple
>  > +@findex -version-simple
>  > +Display basic version number information and exit
>  > +ETEXI
>  > +
>  >  DEF("M", HAS_ARG, QEMU_OPTION_M,
>  >      "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
>  >  STEXI
>  > diff --git a/vl.c b/vl.c
>  > index 85bcc84..5adca87 100644
>  > --- a/vl.c
>  > +++ b/vl.c
>  > @@ -2015,6 +2015,11 @@ static void version(void)
>  >      printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
>  >  }
>  >
>  > +static void version_simple(void)
>  > +{
>  > +    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
>  > +}
>  > +
>  >  static void help(int exitcode)
>  >  {
>  >      const char *options_help =
>  > @@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>  >                  version();
>  >                  exit(0);
>  >                  break;
>  > +            case QEMU_OPTION_version_simple:
>  > +                version_simple();
>  > +                exit(0);
>  > +                break;
>  >              case QEMU_OPTION_m: {
>  >                  uint64_t value;
>  >                  char *ptr;
>
>
> This omits the KVM version string which is something we also want to see.
>  It would also be nice to avoid having to parse the -help output to determine
>  ARGV supported too. I wonder if it would be a good idea to just produce a
>  well structured equivalent to -help that provides the same data, but in
>  JSON format for sane parsing. That would let peple easily determine the
>  supported ARGV as well as version number(s)

Perhaps QMP could be used to dump the same information, something
equivalent to monitor command 'info argv'.

Otherwise, I'd suggest to use a name with 'machine' or 'json' in it,
like -QEMU-machine-protocol-info-argv, or -get-json-info-argv.

Still, -version-simple may be useful for shell scripting etc.
Markus Armbruster May 14, 2010, 9:42 a.m. UTC | #4
"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> 
>> Add -version-simple argument for QEMU, printing just the version
>> number, without any supporting text.
>> 
>> This makes it simpler for other apps, such as libvirt, to parse the
>> version string from QEMU independant of how the naming string may
>> change.
>> 
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> ---
>>  qemu-options.hx |    8 ++++++++
>>  vl.c            |    9 +++++++++
>>  2 files changed, 17 insertions(+), 0 deletions(-)
>> 
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 12f6b51..e4f3979 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -27,6 +27,14 @@ STEXI
>>  Display version information and exit
>>  ETEXI
>>  
>> +DEF("version-simple", 0, QEMU_OPTION_version_simple,
>> +    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
>> +STEXI
>> +@item -version-simple
>> +@findex -version-simple
>> +Display basic version number information and exit
>> +ETEXI
>> +
>>  DEF("M", HAS_ARG, QEMU_OPTION_M,
>>      "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
>>  STEXI
>> diff --git a/vl.c b/vl.c
>> index 85bcc84..5adca87 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2015,6 +2015,11 @@ static void version(void)
>>      printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
>>  }
>>  
>> +static void version_simple(void)
>> +{
>> +    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
>> +}
>> +
>>  static void help(int exitcode)
>>  {
>>      const char *options_help =
>> @@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>>                  version();
>>                  exit(0);
>>                  break;
>> +            case QEMU_OPTION_version_simple:
>> +                version_simple();
>> +                exit(0);
>> +                break;
>>              case QEMU_OPTION_m: {
>>                  uint64_t value;
>>                  char *ptr;
>
> This omits the KVM version string which is something we also want to see.
> It would also be nice to avoid having to parse the -help output to determine
> ARGV supported too. I wonder if it would be a good idea to just produce a
> well structured equivalent to -help that provides the same data, but in 
> JSON format for sane parsing. That would let peple easily determine the
> supported ARGV as well as version number(s)

I'm all for machine-readable self-documentation.  And the place for that
is QMP.  Humble beginnings are already there:

{ "execute": "query-version", "arguments": { } }
--> {"return": {"qemu": "0.12.50", "package": ""}}

{ "execute": "query-commands", "arguments": { } }
--> {"return": [{"name": "quit"}, {"name": "eject"}, [...]

Any practical problems with use of QMP instead of parsing command line
option output?
Daniel P. Berrangé May 14, 2010, 10:06 a.m. UTC | #5
On Fri, May 14, 2010 at 11:42:57AM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> 
> >> Add -version-simple argument for QEMU, printing just the version
> >> number, without any supporting text.
> >> 
> >> This makes it simpler for other apps, such as libvirt, to parse the
> >> version string from QEMU independant of how the naming string may
> >> change.
> >> 
> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> >> ---
> >>  qemu-options.hx |    8 ++++++++
> >>  vl.c            |    9 +++++++++
> >>  2 files changed, 17 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index 12f6b51..e4f3979 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -27,6 +27,14 @@ STEXI
> >>  Display version information and exit
> >>  ETEXI
> >>  
> >> +DEF("version-simple", 0, QEMU_OPTION_version_simple,
> >> +    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
> >> +STEXI
> >> +@item -version-simple
> >> +@findex -version-simple
> >> +Display basic version number information and exit
> >> +ETEXI
> >> +
> >>  DEF("M", HAS_ARG, QEMU_OPTION_M,
> >>      "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
> >>  STEXI
> >> diff --git a/vl.c b/vl.c
> >> index 85bcc84..5adca87 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -2015,6 +2015,11 @@ static void version(void)
> >>      printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
> >>  }
> >>  
> >> +static void version_simple(void)
> >> +{
> >> +    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
> >> +}
> >> +
> >>  static void help(int exitcode)
> >>  {
> >>      const char *options_help =
> >> @@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
> >>                  version();
> >>                  exit(0);
> >>                  break;
> >> +            case QEMU_OPTION_version_simple:
> >> +                version_simple();
> >> +                exit(0);
> >> +                break;
> >>              case QEMU_OPTION_m: {
> >>                  uint64_t value;
> >>                  char *ptr;
> >
> > This omits the KVM version string which is something we also want to see.
> > It would also be nice to avoid having to parse the -help output to determine
> > ARGV supported too. I wonder if it would be a good idea to just produce a
> > well structured equivalent to -help that provides the same data, but in 
> > JSON format for sane parsing. That would let peple easily determine the
> > supported ARGV as well as version number(s)
> 
> I'm all for machine-readable self-documentation.  And the place for that
> is QMP.  Humble beginnings are already there:
> 
> { "execute": "query-version", "arguments": { } }
> --> {"return": {"qemu": "0.12.50", "package": ""}}
> 
> { "execute": "query-commands", "arguments": { } }
> --> {"return": [{"name": "quit"}, {"name": "eject"}, [...]
> 
> Any practical problems with use of QMP instead of parsing command line
> option output?

It is unneccessarily complex for such a simple task, requiring you to 
configure & connect to the monitor & do the capabilities negotiaton
and then issue the command. 

To just query the version requires this ridiculous interaction:

  $ qemu -chardev stdio,id=monitor -monitor chardev=monitor,mode=control
  {"execute":"qmp_capabilities"}
  {"QMP": {"version": {"qemu": "0.12.1", "package": " (qemu-kvm-0.12.1.2)"}, "capabilities": []}}
  {"execute":"query-version"}
  {"return": {"qemu": "0.12.50", "package": ""}}
  

I'm suggesting we just allow some simple syntactic sugar on the command 
line for the handful of QMP commands that are just returning static info
about the binary, that are not affected by VM state. 

eg, make this work:

  $ qemu -query-version
  {"qemu": "0.12.50", "package": ""}

Daniel
Markus Armbruster May 14, 2010, 11:24 a.m. UTC | #6
"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Fri, May 14, 2010 at 11:42:57AM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
[...]
>> > It would also be nice to avoid having to parse the -help output to determine
>> > ARGV supported too. I wonder if it would be a good idea to just produce a
>> > well structured equivalent to -help that provides the same data, but in 
>> > JSON format for sane parsing. That would let peple easily determine the
>> > supported ARGV as well as version number(s)
>> 
>> I'm all for machine-readable self-documentation.  And the place for that
>> is QMP.  Humble beginnings are already there:
>> 
>> { "execute": "query-version", "arguments": { } }
>> --> {"return": {"qemu": "0.12.50", "package": ""}}
>> 
>> { "execute": "query-commands", "arguments": { } }
>> --> {"return": [{"name": "quit"}, {"name": "eject"}, [...]
>> 
>> Any practical problems with use of QMP instead of parsing command line
>> option output?
>
> It is unneccessarily complex for such a simple task, requiring you to 
> configure & connect to the monitor & do the capabilities negotiaton
> and then issue the command. 
>
> To just query the version requires this ridiculous interaction:
>
>   $ qemu -chardev stdio,id=monitor -monitor chardev=monitor,mode=control
>   {"execute":"qmp_capabilities"}
>   {"QMP": {"version": {"qemu": "0.12.1", "package": " (qemu-kvm-0.12.1.2)"}, "capabilities": []}}
>   {"execute":"query-version"}
>   {"return": {"qemu": "0.12.50", "package": ""}}

Actually,

$ qemu -nodefaults -nographic -S -chardev stdio,id=qmp -mon mode=control,chardev=qmp </dev/null
{"QMP": {"version": {"qemu": "0.12.50", "package": ""}, "capabilities": []}}

suffices, with the minor wart that you have to SIGINT out.

> I'm suggesting we just allow some simple syntactic sugar on the command 
> line for the handful of QMP commands that are just returning static info
> about the binary, that are not affected by VM state. 
>
> eg, make this work:
>
>   $ qemu -query-version
>   {"qemu": "0.12.50", "package": ""}

I wouldn't mind.
Anthony Liguori May 14, 2010, 1:21 p.m. UTC | #7
On 05/13/2010 03:32 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> Add -version-simple argument for QEMU, printing just the version
> number, without any supporting text.
>    

I'm not a huge fan of the name.

But what information are we trying to convey?  Just major/minor number 
or would qemu-kvm also throw some info in there?

Do version numbers even matter because 0.13 from qemu.git is going to be 
a hell of a lot different from 0.13 in RHEL6.x.

What are the consumers of this information actually doing with it?

Regards,

Anthony Liguori

> This makes it simpler for other apps, such as libvirt, to parse the
> version string from QEMU independant of how the naming string may
> change.
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> ---
>   qemu-options.hx |    8 ++++++++
>   vl.c            |    9 +++++++++
>   2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 12f6b51..e4f3979 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -27,6 +27,14 @@ STEXI
>   Display version information and exit
>   ETEXI
>
> +DEF("version-simple", 0, QEMU_OPTION_version_simple,
> +    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
> +STEXI
> +@item -version-simple
> +@findex -version-simple
> +Display basic version number information and exit
> +ETEXI
> +
>   DEF("M", HAS_ARG, QEMU_OPTION_M,
>       "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
>   STEXI
> diff --git a/vl.c b/vl.c
> index 85bcc84..5adca87 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2015,6 +2015,11 @@ static void version(void)
>       printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
>   }
>
> +static void version_simple(void)
> +{
> +    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
> +}
> +
>   static void help(int exitcode)
>   {
>       const char *options_help =
> @@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>                   version();
>                   exit(0);
>                   break;
> +            case QEMU_OPTION_version_simple:
> +                version_simple();
> +                exit(0);
> +                break;
>               case QEMU_OPTION_m: {
>                   uint64_t value;
>                   char *ptr;
>
Anthony Liguori May 14, 2010, 1:27 p.m. UTC | #8
On 05/13/2010 08:33 AM, Daniel P. Berrange wrote:
> On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
>    
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> Add -version-simple argument for QEMU, printing just the version
>> number, without any supporting text.
>>
>> This makes it simpler for other apps, such as libvirt, to parse the
>> version string from QEMU independant of how the naming string may
>> change.
>>
>> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
>> ---
>>   qemu-options.hx |    8 ++++++++
>>   vl.c            |    9 +++++++++
>>   2 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 12f6b51..e4f3979 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -27,6 +27,14 @@ STEXI
>>   Display version information and exit
>>   ETEXI
>>
>> +DEF("version-simple", 0, QEMU_OPTION_version_simple,
>> +    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
>> +STEXI
>> +@item -version-simple
>> +@findex -version-simple
>> +Display basic version number information and exit
>> +ETEXI
>> +
>>   DEF("M", HAS_ARG, QEMU_OPTION_M,
>>       "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
>>   STEXI
>> diff --git a/vl.c b/vl.c
>> index 85bcc84..5adca87 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2015,6 +2015,11 @@ static void version(void)
>>       printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
>>   }
>>
>> +static void version_simple(void)
>> +{
>> +    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
>> +}
>> +
>>   static void help(int exitcode)
>>   {
>>       const char *options_help =
>> @@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>>                   version();
>>                   exit(0);
>>                   break;
>> +            case QEMU_OPTION_version_simple:
>> +                version_simple();
>> +                exit(0);
>> +                break;
>>               case QEMU_OPTION_m: {
>>                   uint64_t value;
>>                   char *ptr;
>>      
> This omits the KVM version string which is something we also want to see.
> It would also be nice to avoid having to parse the -help output to determine
> ARGV supported too. I wonder if it would be a good idea to just produce a
> well structured equivalent to -help that provides the same data, but in
> JSON format for sane parsing. That would let peple easily determine the
> supported ARGV as well as version number(s)
>    

Can we do this all via the monitor?  IOW, can libvirt invoke qemu 
blindly and strictly interact with the monitor?

Regards,

Anthony Liguori

> Regards,
> Daniel
>
Daniel P. Berrangé May 14, 2010, 1:32 p.m. UTC | #9
On Fri, May 14, 2010 at 08:27:54AM -0500, Anthony Liguori wrote:
> On 05/13/2010 08:33 AM, Daniel P. Berrange wrote:
> >On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
> >   
> >>From: Jes Sorensen<Jes.Sorensen@redhat.com>
> >>
> >>Add -version-simple argument for QEMU, printing just the version
> >>number, without any supporting text.
> >>
> >>This makes it simpler for other apps, such as libvirt, to parse the
> >>version string from QEMU independant of how the naming string may
> >>change.
> >>
> >>Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> >>---
> >>  qemu-options.hx |    8 ++++++++
> >>  vl.c            |    9 +++++++++
> >>  2 files changed, 17 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/qemu-options.hx b/qemu-options.hx
> >>index 12f6b51..e4f3979 100644
> >>--- a/qemu-options.hx
> >>+++ b/qemu-options.hx
> >>@@ -27,6 +27,14 @@ STEXI
> >>  Display version information and exit
> >>  ETEXI
> >>
> >>+DEF("version-simple", 0, QEMU_OPTION_version_simple,
> >>+    "-version-simple display version information and exit\n", 
> >>QEMU_ARCH_ALL)
> >>+STEXI
> >>+@item -version-simple
> >>+@findex -version-simple
> >>+Display basic version number information and exit
> >>+ETEXI
> >>+
> >>  DEF("M", HAS_ARG, QEMU_OPTION_M,
> >>      "-M machine      select emulated machine (-M ? for list)\n", 
> >>      QEMU_ARCH_ALL)
> >>  STEXI
> >>diff --git a/vl.c b/vl.c
> >>index 85bcc84..5adca87 100644
> >>--- a/vl.c
> >>+++ b/vl.c
> >>@@ -2015,6 +2015,11 @@ static void version(void)
> >>      printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", 
> >>      Copyright (c) 2003-2008 Fabrice Bellard\n");
> >>  }
> >>
> >>+static void version_simple(void)
> >>+{
> >>+    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
> >>+}
> >>+
> >>  static void help(int exitcode)
> >>  {
> >>      const char *options_help =
> >>@@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
> >>                  version();
> >>                  exit(0);
> >>                  break;
> >>+            case QEMU_OPTION_version_simple:
> >>+                version_simple();
> >>+                exit(0);
> >>+                break;
> >>              case QEMU_OPTION_m: {
> >>                  uint64_t value;
> >>                  char *ptr;
> >>     
> >This omits the KVM version string which is something we also want to see.
> >It would also be nice to avoid having to parse the -help output to 
> >determine
> >ARGV supported too. I wonder if it would be a good idea to just produce a
> >well structured equivalent to -help that provides the same data, but in
> >JSON format for sane parsing. That would let peple easily determine the
> >supported ARGV as well as version number(s)
> >   
> 
> Can we do this all via the monitor?  IOW, can libvirt invoke qemu 
> blindly and strictly interact with the monitor?

See my other message in this thread...

Daniel
Daniel P. Berrangé May 14, 2010, 1:34 p.m. UTC | #10
On Fri, May 14, 2010 at 01:24:44PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > On Fri, May 14, 2010 at 11:42:57AM +0200, Markus Armbruster wrote:
> >> "Daniel P. Berrange" <berrange@redhat.com> writes:
> [...]
> >> > It would also be nice to avoid having to parse the -help output to determine
> >> > ARGV supported too. I wonder if it would be a good idea to just produce a
> >> > well structured equivalent to -help that provides the same data, but in 
> >> > JSON format for sane parsing. That would let peple easily determine the
> >> > supported ARGV as well as version number(s)
> >> 
> >> I'm all for machine-readable self-documentation.  And the place for that
> >> is QMP.  Humble beginnings are already there:
> >> 
> >> { "execute": "query-version", "arguments": { } }
> >> --> {"return": {"qemu": "0.12.50", "package": ""}}
> >> 
> >> { "execute": "query-commands", "arguments": { } }
> >> --> {"return": [{"name": "quit"}, {"name": "eject"}, [...]
> >> 
> >> Any practical problems with use of QMP instead of parsing command line
> >> option output?
> >
> > It is unneccessarily complex for such a simple task, requiring you to 
> > configure & connect to the monitor & do the capabilities negotiaton
> > and then issue the command. 
> >
> > To just query the version requires this ridiculous interaction:
> >
> >   $ qemu -chardev stdio,id=monitor -monitor chardev=monitor,mode=control
> >   {"execute":"qmp_capabilities"}
> >   {"QMP": {"version": {"qemu": "0.12.1", "package": " (qemu-kvm-0.12.1.2)"}, "capabilities": []}}
> >   {"execute":"query-version"}
> >   {"return": {"qemu": "0.12.50", "package": ""}}
> 
> Actually,
> 
> $ qemu -nodefaults -nographic -S -chardev stdio,id=qmp -mon mode=control,chardev=qmp </dev/null
> {"QMP": {"version": {"qemu": "0.12.50", "package": ""}, "capabilities": []}}
> 
> suffices, with the minor wart that you have to SIGINT out.

Only in this particular example. The same pain I illustrate still exists
for other static query actions such as query-device, or a hypothetical 
query-argv for getting supported command line args. I really think we need
to map these into the more concise & easily accessible style:

> > eg, make this work:
> >
> >   $ qemu -query-version
> >   {"qemu": "0.12.50", "package": ""}
> 
> I wouldn't mind.

Regards,
Daniel
Anthony Liguori May 14, 2010, 1:48 p.m. UTC | #11
On 05/14/2010 05:06 AM, Daniel P. Berrange wrote:
> On Fri, May 14, 2010 at 11:42:57AM +0200, Markus Armbruster wrote:
>    
>> "Daniel P. Berrange"<berrange@redhat.com>  writes:
>>
>>      
>>> On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
>>>        
>>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>>
>>>> Add -version-simple argument for QEMU, printing just the version
>>>> number, without any supporting text.
>>>>
>>>> This makes it simpler for other apps, such as libvirt, to parse the
>>>> version string from QEMU independant of how the naming string may
>>>> change.
>>>>
>>>> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>> ---
>>>>   qemu-options.hx |    8 ++++++++
>>>>   vl.c            |    9 +++++++++
>>>>   2 files changed, 17 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>>> index 12f6b51..e4f3979 100644
>>>> --- a/qemu-options.hx
>>>> +++ b/qemu-options.hx
>>>> @@ -27,6 +27,14 @@ STEXI
>>>>   Display version information and exit
>>>>   ETEXI
>>>>
>>>> +DEF("version-simple", 0, QEMU_OPTION_version_simple,
>>>> +    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
>>>> +STEXI
>>>> +@item -version-simple
>>>> +@findex -version-simple
>>>> +Display basic version number information and exit
>>>> +ETEXI
>>>> +
>>>>   DEF("M", HAS_ARG, QEMU_OPTION_M,
>>>>       "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
>>>>   STEXI
>>>> diff --git a/vl.c b/vl.c
>>>> index 85bcc84..5adca87 100644
>>>> --- a/vl.c
>>>> +++ b/vl.c
>>>> @@ -2015,6 +2015,11 @@ static void version(void)
>>>>       printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
>>>>   }
>>>>
>>>> +static void version_simple(void)
>>>> +{
>>>> +    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
>>>> +}
>>>> +
>>>>   static void help(int exitcode)
>>>>   {
>>>>       const char *options_help =
>>>> @@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
>>>>                   version();
>>>>                   exit(0);
>>>>                   break;
>>>> +            case QEMU_OPTION_version_simple:
>>>> +                version_simple();
>>>> +                exit(0);
>>>> +                break;
>>>>               case QEMU_OPTION_m: {
>>>>                   uint64_t value;
>>>>                   char *ptr;
>>>>          
>>> This omits the KVM version string which is something we also want to see.
>>> It would also be nice to avoid having to parse the -help output to determine
>>> ARGV supported too. I wonder if it would be a good idea to just produce a
>>> well structured equivalent to -help that provides the same data, but in
>>> JSON format for sane parsing. That would let peple easily determine the
>>> supported ARGV as well as version number(s)
>>>        
>> I'm all for machine-readable self-documentation.  And the place for that
>> is QMP.  Humble beginnings are already there:
>>
>> { "execute": "query-version", "arguments": { } }
>> -->  {"return": {"qemu": "0.12.50", "package": ""}}
>>
>> { "execute": "query-commands", "arguments": { } }
>> -->  {"return": [{"name": "quit"}, {"name": "eject"}, [...]
>>
>> Any practical problems with use of QMP instead of parsing command line
>> option output?
>>      
> It is unneccessarily complex for such a simple task, requiring you to
> configure&  connect to the monitor&  do the capabilities negotiaton
> and then issue the command.
>
> To just query the version requires this ridiculous interaction:
>
>    $ qemu -chardev stdio,id=monitor -monitor chardev=monitor,mode=control
>    {"execute":"qmp_capabilities"}
>    {"QMP": {"version": {"qemu": "0.12.1", "package": " (qemu-kvm-0.12.1.2)"}, "capabilities": []}}
>    {"execute":"query-version"}
>    {"return": {"qemu": "0.12.50", "package": ""}}
>
>
> I'm suggesting we just allow some simple syntactic sugar on the command
> line for the handful of QMP commands that are just returning static info
> about the binary, that are not affected by VM state.
>
> eg, make this work:
>
>    $ qemu -query-version
>    {"qemu": "0.12.50", "package": ""}
>    

No need for package.  Vendors can use vendor extensions to add whatever 
info they want.

But we should avoid an encoded string, it would be better as:

{"major": 0, "minor": 12, "release": 50}

And then it could be:

{"major": 0, "minor": 12, "release": 50, "__org.linux-kvm.release": 1, 
"__com.redhat.RHEL6.release": 13}

We could also just pretty print it:

major: 0
minor: 12
release: 50
__org.linux-kvm.release: 1
__com.redhat.RHEL6.release: 13

Regards,

Anthony Liguori

> Daniel
>
Daniel P. Berrangé May 14, 2010, 1:57 p.m. UTC | #12
On Fri, May 14, 2010 at 08:48:58AM -0500, Anthony Liguori wrote:
> On 05/14/2010 05:06 AM, Daniel P. Berrange wrote:
> >On Fri, May 14, 2010 at 11:42:57AM +0200, Markus Armbruster wrote:
> >   
> >>"Daniel P. Berrange"<berrange@redhat.com>  writes:
> >>
> >>     
> >>>On Thu, May 13, 2010 at 10:32:52AM +0200, Jes.Sorensen@redhat.com wrote:
> >>>       
> >>>>From: Jes Sorensen<Jes.Sorensen@redhat.com>
> >>>>
> >>>>Add -version-simple argument for QEMU, printing just the version
> >>>>number, without any supporting text.
> >>>>
> >>>>This makes it simpler for other apps, such as libvirt, to parse the
> >>>>version string from QEMU independant of how the naming string may
> >>>>change.
> >>>>
> >>>>Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> >>>>---
> >>>>  qemu-options.hx |    8 ++++++++
> >>>>  vl.c            |    9 +++++++++
> >>>>  2 files changed, 17 insertions(+), 0 deletions(-)
> >>>>
> >>>>diff --git a/qemu-options.hx b/qemu-options.hx
> >>>>index 12f6b51..e4f3979 100644
> >>>>--- a/qemu-options.hx
> >>>>+++ b/qemu-options.hx
> >>>>@@ -27,6 +27,14 @@ STEXI
> >>>>  Display version information and exit
> >>>>  ETEXI
> >>>>
> >>>>+DEF("version-simple", 0, QEMU_OPTION_version_simple,
> >>>>+    "-version-simple display version information and exit\n", 
> >>>>QEMU_ARCH_ALL)
> >>>>+STEXI
> >>>>+@item -version-simple
> >>>>+@findex -version-simple
> >>>>+Display basic version number information and exit
> >>>>+ETEXI
> >>>>+
> >>>>  DEF("M", HAS_ARG, QEMU_OPTION_M,
> >>>>      "-M machine      select emulated machine (-M ? for list)\n", 
> >>>>      QEMU_ARCH_ALL)
> >>>>  STEXI
> >>>>diff --git a/vl.c b/vl.c
> >>>>index 85bcc84..5adca87 100644
> >>>>--- a/vl.c
> >>>>+++ b/vl.c
> >>>>@@ -2015,6 +2015,11 @@ static void version(void)
> >>>>      printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", 
> >>>>      Copyright (c) 2003-2008 Fabrice Bellard\n");
> >>>>  }
> >>>>
> >>>>+static void version_simple(void)
> >>>>+{
> >>>>+    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
> >>>>+}
> >>>>+
> >>>>  static void help(int exitcode)
> >>>>  {
> >>>>      const char *options_help =
> >>>>@@ -2960,6 +2965,10 @@ int main(int argc, char **argv, char **envp)
> >>>>                  version();
> >>>>                  exit(0);
> >>>>                  break;
> >>>>+            case QEMU_OPTION_version_simple:
> >>>>+                version_simple();
> >>>>+                exit(0);
> >>>>+                break;
> >>>>              case QEMU_OPTION_m: {
> >>>>                  uint64_t value;
> >>>>                  char *ptr;
> >>>>         
> >>>This omits the KVM version string which is something we also want to see.
> >>>It would also be nice to avoid having to parse the -help output to 
> >>>determine
> >>>ARGV supported too. I wonder if it would be a good idea to just produce a
> >>>well structured equivalent to -help that provides the same data, but in
> >>>JSON format for sane parsing. That would let peple easily determine the
> >>>supported ARGV as well as version number(s)
> >>>       
> >>I'm all for machine-readable self-documentation.  And the place for that
> >>is QMP.  Humble beginnings are already there:
> >>
> >>{ "execute": "query-version", "arguments": { } }
> >>-->  {"return": {"qemu": "0.12.50", "package": ""}}
> >>
> >>{ "execute": "query-commands", "arguments": { } }
> >>-->  {"return": [{"name": "quit"}, {"name": "eject"}, [...]
> >>
> >>Any practical problems with use of QMP instead of parsing command line
> >>option output?
> >>     
> >It is unneccessarily complex for such a simple task, requiring you to
> >configure&  connect to the monitor&  do the capabilities negotiaton
> >and then issue the command.
> >
> >To just query the version requires this ridiculous interaction:
> >
> >   $ qemu -chardev stdio,id=monitor -monitor chardev=monitor,mode=control
> >   {"execute":"qmp_capabilities"}
> >   {"QMP": {"version": {"qemu": "0.12.1", "package": " 
> >   (qemu-kvm-0.12.1.2)"}, "capabilities": []}}
> >   {"execute":"query-version"}
> >   {"return": {"qemu": "0.12.50", "package": ""}}
> >
> >
> >I'm suggesting we just allow some simple syntactic sugar on the command
> >line for the handful of QMP commands that are just returning static info
> >about the binary, that are not affected by VM state.
> >
> >eg, make this work:
> >
> >   $ qemu -query-version
> >   {"qemu": "0.12.50", "package": ""}
> >   
> 
> No need for package.  Vendors can use vendor extensions to add whatever 
> info they want.

Yeah I don't know what 'package' is doing - its jsut what 'query-version'
currently prints in QMP

> But we should avoid an encoded string, it would be better as:
> 
> {"major": 0, "minor": 12, "release": 50}
> 
> And then it could be:
> 
> {"major": 0, "minor": 12, "release": 50, "__org.linux-kvm.release": 1, 
> "__com.redhat.RHEL6.release": 13}
> 
> We could also just pretty print it:
> 
> major: 0
> minor: 12
> release: 50
> __org.linux-kvm.release: 1
> __com.redhat.RHEL6.release: 13

Pretty printing re-introduces the problem of reliable parsing. I think we
should allow for a JSON format output since apps already need to have a
good parser for that - no point writing another parser. Perhaps allow
for the arg to take a format value,  '-query-version [pretty|json]'

Regards,
Daniel
Chris Lalancette May 14, 2010, 1:58 p.m. UTC | #13
On 05/14/2010 09:21 AM, Anthony Liguori wrote:
> On 05/13/2010 03:32 AM, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> Add -version-simple argument for QEMU, printing just the version
>> number, without any supporting text.
>>    
> 
> I'm not a huge fan of the name.
> 
> But what information are we trying to convey?  Just major/minor number
> or would qemu-kvm also throw some info in there?
> 
> Do version numbers even matter because 0.13 from qemu.git is going to be
> a hell of a lot different from 0.13 in RHEL6.x.
> 
> What are the consumers of this information actually doing with it?

At the moment libvirt uses it mostly to determine what features are
present in a particular version of Qemu that is on the machine.  So
for instrance, for 0.13 we turn on the JSON flag so we know to use
the QMP monitor instead of the text monitor.  There are many other
examples as well.

Now, I agree that this isn't the best way to do it; much better would
be a way for libvirt to query the capabilities of Qemu directly and
not rely on versions (not the least because of the problem you point
out above).  But up until now we haven't had a better way to do it.
Daniel P. Berrangé May 14, 2010, 2:06 p.m. UTC | #14
On Fri, May 14, 2010 at 08:21:55AM -0500, Anthony Liguori wrote:
> On 05/13/2010 03:32 AM, Jes.Sorensen@redhat.com wrote:
> >From: Jes Sorensen<Jes.Sorensen@redhat.com>
> >
> >Add -version-simple argument for QEMU, printing just the version
> >number, without any supporting text.
> >   
> 
> I'm not a huge fan of the name.
> 
> But what information are we trying to convey?  Just major/minor number 
> or would qemu-kvm also throw some info in there?
> 
> Do version numbers even matter because 0.13 from qemu.git is going to be 
> a hell of a lot different from 0.13 in RHEL6.x.

Version numbers are the last resort. We'll try all other avenues for
detecting a feature, before turning to a version number, precisely
because of the variance you mention here. When we do toggle something
based on a version though, we'll be conservative basing decision off
upstream QEMU 0.13 functionality, ignoring what extra bits a distro 
might have backported (or even disabled).

Daniel
Markus Armbruster May 14, 2010, 2:25 p.m. UTC | #15
Anthony Liguori <aliguori@linux.vnet.ibm.com> writes:

> Can we do this all via the monitor?  IOW, can libvirt invoke qemu
> blindly and strictly interact with the monitor?

I think that's exactly where should be heading longer term.  It's not a
practical immediate solution, e.g. because device_add can't cold-plug,
yet.
Jes Sorensen May 17, 2010, 6:54 a.m. UTC | #16
On 05/14/10 15:21, Anthony Liguori wrote:
> On 05/13/2010 03:32 AM, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> Add -version-simple argument for QEMU, printing just the version
>> number, without any supporting text.
> 
> I'm not a huge fan of the name.

It was the lesser evil I could come up with since -version was already
taken, but I am open to alternatives.

> But what information are we trying to convey?  Just major/minor number
> or would qemu-kvm also throw some info in there?
> 
> Do version numbers even matter because 0.13 from qemu.git is going to be
> a hell of a lot different from 0.13 in RHEL6.x.
> 
> What are the consumers of this information actually doing with it?

I think this has already been discussed, I was off on Friday so I'll
leave it to the other thread.

Cheers,
Jes
diff mbox

Patch

diff --git a/qemu-options.hx b/qemu-options.hx
index 12f6b51..e4f3979 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -27,6 +27,14 @@  STEXI
 Display version information and exit
 ETEXI
 
+DEF("version-simple", 0, QEMU_OPTION_version_simple,
+    "-version-simple display version information and exit\n", QEMU_ARCH_ALL)
+STEXI
+@item -version-simple
+@findex -version-simple
+Display basic version number information and exit
+ETEXI
+
 DEF("M", HAS_ARG, QEMU_OPTION_M,
     "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/vl.c b/vl.c
index 85bcc84..5adca87 100644
--- a/vl.c
+++ b/vl.c
@@ -2015,6 +2015,11 @@  static void version(void)
     printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
 }
 
+static void version_simple(void)
+{
+    printf(QEMU_VERSION QEMU_PKGVERSION "\n");
+}
+
 static void help(int exitcode)
 {
     const char *options_help =
@@ -2960,6 +2965,10 @@  int main(int argc, char **argv, char **envp)
                 version();
                 exit(0);
                 break;
+            case QEMU_OPTION_version_simple:
+                version_simple();
+                exit(0);
+                break;
             case QEMU_OPTION_m: {
                 uint64_t value;
                 char *ptr;