Patchwork [09/19] Change 'query-version' to output broken down version string

login
register
mail settings
Submitter Daniel P. Berrange
Date June 7, 2010, 2:42 p.m.
Message ID <1275921752-29420-10-git-send-email-berrange@redhat.com>
Download mbox | patch
Permalink /patch/54871/
State New
Headers show

Comments

Daniel P. Berrange - June 7, 2010, 2:42 p.m.
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": "" }

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 <berrange@redhat.com>
---
 monitor.c |   47 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 43 insertions(+), 4 deletions(-)
Anthony Liguori - June 7, 2010, 3:11 p.m.
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.

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

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.

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<berrange@redhat.com>
> ---
>   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)
>
Luiz Capitulino - June 9, 2010, 8:04 p.m.
On Mon, 07 Jun 2010 10:11:28 -0500
Anthony Liguori <anthony@codemonkey.ws> 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<berrange@redhat.com>
> > ---
> >   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)
> >    
> 
>

Patch

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)