Re: [Qemu-devel] [PATCH 1/1] Add -version-simple argument, printing only version number.

[Daniel P. Berrange]

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"<address@hidden>  writes:
> >>
> >>     
> >>>On Thu, May 13, 2010 at 10:32:52AM +0200, address@hidden wrote:
> >>>       
> >>>>From: Jes Sorensen<address@hidden>
> >>>>
> >>>>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<address@hidden>
> >>>>---
> >>>>  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
> >>>>address@hidden -version-simple
> >>>>address@hidden -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
-- 
|: 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 :|