| Message ID | 1275921752-29420-10-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
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) >
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) > > > >
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)
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(-)