Re: [Qemu-devel] [PATCH 09/19] Change 'query-version' to output broken down version string

[Luiz Capitulino]

On Mon, 07 Jun 2010 10:11:28 -0500
Anthony Liguori <address@hidden> wrote:

> On 06/07/2010 09:42 AM, Daniel P. Berrange wrote:
> > A previous discussion brought up the fact that clients should
> > not have to parse version string from QMP, it should be given
> > to them pre-split.
> >
> > Change query-version output format from:
> >
> >    { "qemu": "0.11.50", "package": "" }
> >
> > to:
> >
> >    { qemu: { "major": 0, "minor": 11, "micro": 5 }, "package": "" }
> >    
> 
> We need to drop package entirely.

 Why? It's something that shouldn't be in the greeting message, but it's
a component of the version, so the only way to query it from QMP is by
using query-version.

> Instead vendors should use the vendor specific namespace to extend the 
> version.

 IMHO, vendor in QMP's terminology is someone adding extensions to
the protocol. Most distros won't do that, but they may carry not
upstream merged patches.

> I'd suggest changing the version output to:
> 
> { 'major': 0, 'minor': 11, 'micro': 5 }
> 
> For something like kvm, it can introduce a '__org.linux-kvm.release', 2 
> to that main dictionary.

 I think we need both, 'vendor' and 'package' keys and maybe 'vendor'
should be part of the greeting message so that clients can identify they
are talking to a non standard version.

> 
> Regards,
> 
> Anthony Liguori
> 
> > The major, minor&  micro keys are all integer values. package is
> > an arbitrary string whose format is defined by the OS package
> > maintainer.
> >
> > There is no need to preserve the existing string format of the 'qemu'
> > key, since QMP is not yet declared stable.
> >
> > Signed-off-by: Daniel P. Berrange<address@hidden>
> > ---
> >   monitor.c |   47 +++++++++++++++++++++++++++++++++++++++++++----
> >   1 files changed, 43 insertions(+), 4 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 15b53b9..f0406e8 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -670,17 +670,56 @@ help:
> >   static void do_info_version_print(Monitor *mon, const QObject *data)
> >   {
> >       QDict *qdict;
> > +    QDict *qemu;
> >
> >       qdict = qobject_to_qdict(data);
> > +    qemu = qdict_get_qdict(qdict, "qemu");
> >
> > -    monitor_printf(mon, "%s%s\n", qdict_get_str(qdict, "qemu"),
> > -                                  qdict_get_str(qdict, "package"));
> > +    monitor_printf(mon, "%" PRId64 ".%" PRId64 ".%" PRId64 "%s\n",
> > +              qdict_get_int(qemu, "major"),
> > +              qdict_get_int(qemu, "minor"),
> > +              qdict_get_int(qemu, "micro"),
> > +              qdict_get_str(qdict, "package"));
> >   }
> >
> > +
> > +/**
> > + * do_info_version(): Show QEMU version
> > + *
> > + * Return a QDict with the following information:
> > + *
> > + * - "qemu": QEMU upstream version
> > + * - "package": QEMU packager's version
> > + *
> > + * The 'qemu' key value is a QDict containing three
> > + * integer values
> > + *
> > + * - "major": QEMU's major version
> > + * - "minor": QEMU's minor version
> > + * - "micro": QEMU's micro version
> > + *
> > + * The 'package' key value is a string in an format
> > + * defined by the OS distributor to reflect their
> > + * packaging of QEMU.
> > + *
> > + * Example:
> > + *
> > + * { qemu: { "major": 0, "minor": 11, "micro": 5 }, "package": "" }
> > + */
> >   static void do_info_version(Monitor *mon, QObject **ret_data)
> >   {
> > -    *ret_data = qobject_from_jsonf("{ 'qemu': %s, 'package': %s }",
> > -                                   QEMU_VERSION, QEMU_PKGVERSION);
> > +    const char *version = QEMU_VERSION;
> > +    int major = 0, minor = 0, micro = 0;
> > +    char *tmp;
> > +
> > +    major = strtol(version,&tmp, 10);
> > +    tmp++;
> > +    minor = strtol(tmp,&tmp, 10);
> > +    tmp++;
> > +    micro = strtol(tmp,&tmp, 10);
> > +
> > +    *ret_data = qobject_from_jsonf("{ 'qemu': { 'major': %d, 'minor': %d, 
> > 'micro': %d }, 'package': %s }",
> > +                                   major, minor, micro, QEMU_PKGVERSION);
> >   }
> >
> >   static void do_info_name_print(Monitor *mon, const QObject *data)
> >    
> 
>