Change 'query-version' to output broken down version string
PATCH 1/1 Change 'query-version' to output broken down version string
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:
{ "major": 0, "minor": 11, "micro": 5, "package": "" }
major, minor & micro 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 'qemu' field, since QMP is not yet declared stable.
--- monitor.c | 33 ++++++++++++++++++++++++++------- 1 files changed, 26 insertions(+), 7 deletions(-)
monitor.c +26 -7
@@ -673,8 +673,11 @@ static void do_info_version_print(Monitor *mon, const QObject *data) qdict =qobject_to_qdict (data); -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 (qdict,"major" ), +qdict_get_int (qdict,"minor" ), +qdict_get_int (qdict,"micro" ), +qdict_get_str (qdict,"package" )); }/**
@@ -682,17 +685,33 @@ static void do_info_version_print(Monitor *mon, const QObject *data)* * Return a QDict with the following information: * -* - "qemu": QEMU's version -* - "package": package's version +* - "major": QEMU's major version +* - "minor": QEMU's minor version +* - "micro": QEMU's micro version +* - "package": QEMU packager's version +* +* The first three values are guarenteed to be +* integers. The final 'package' value is a string +* in an arbitrary packager specific format * * Example: * -* { "qemu": "0.11.50", "package": "" } +* { "major": 0, "minor": 11, "micro": 5, "package": "" } */ static void do_info_version (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 ("{ 'major': %d, 'minor': %d, 'micro': %d, 'package': %s }" , + major, minor, micro,QEMU_PKGVERSION ); }static void do_info_name_print (Monitor *mon,const QObject *data)
Signature · 1 line
1.6.6.1
Quoted 3 lines · On Wed, 2 Jun 2010 13:40:06 +0100 "Daniel P. Berrange" <[email protected]>…
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.
Right.
Quoted 11 lines
Change query-version output format from:
{ "qemu": "0.11.50", "package": "" }
to:
{ "major": 0, "minor": 11, "micro": 5, "package": "" }
major, minor & micro are all integer values. package is an arbitrary string whose format is defined by the OS package maintainer.
Looks good to me, a few comments:
1. Does QEMU have a naming convention for its versioning scheme or are we creating one right now?
2. The "package" member concerns me a bit, as package maintainers can put anything there, for example qemu-kvm has "(qemu-kvm-devel)" and we could have worst cases like: "-0.341.121-foobar-release".
Perhaps we could let package alone and have a "downstream" member which could be either just a plain string or a dict with keys for name and version.
3. Actually, qemu-kvm has " (qemu-kvm-devel)", the leading whitespace is a bug, maybe you could address it in this patch in case we decide to stay with "package"?
Two more comments below.
Quoted 2 lines
There is no need to preserve the existing 'qemu' field, since QMP is not yet declared stable.
I've added the 'qemu' member to allow easy expansion in case we
decided to have a 'qmp' key.
Quoted 32 lines
--- monitor.c | 33 ++++++++++++++++++++++++++------- 1 files changed, 26 insertions(+), 7 deletions(-)monitor.c +9
-4@@ -673,8 +673,11 @@ static void do_info_version_print(Monitor *mon, const QObject *data) qdict =qobject_to_qdict (data); -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 (qdict,"major" ), +qdict_get_int (qdict,"minor" ), +qdict_get_int (qdict,"micro" ), +qdict_get_str (qdict,"package" )); }/** @@ -682,17 +685,33 @@ static void do_info_version_print(Monitor *mon, const QObject *data)* * Return a QDict with the following information: * -* - "qemu": QEMU's version -* - "package": package's version +* - "major": QEMU's major version +* - "minor": QEMU's minor version +* - "micro": QEMU's micro version +* - "package": QEMU packager's version
monitor.c static void do_info_version_print(Monitor *mon, const QObject *data) · line 700
Quoted 10 lines
monitor.c +5
-1+* +* The first three values are guarenteed to be +* integers. The final 'package' value is a string +* in an arbitrary packager specific format * * Example: * -* { "qemu": "0.11.50", "package": "" } +* { "major": 0, "minor": 11, "micro": 5, "package": "" } */
This comment doesn't exist anymore, now we should update qemu-monitor.hx
(search for 'query-version' there).
Quoted 19 lines
monitor.c +12
-2static void do_info_version (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 ("{ 'major': %d, 'minor': %d, 'micro': %d, 'package': %s }" , + major, minor, micro,QEMU_PKGVERSION ); }static void do_info_name_print (Monitor *mon,const QObject *data)
Quoted 17 lines · Luiz Capitulino
Quoted 3 lines · On Wed, 2 Jun 2010 13:40:06 +0100 "Daniel P. Berrange" <[email protected]>…
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.
Right.
Quoted 8 lines
Change query-version output format from:
{ "qemu": "0.11.50", "package": "" }
to:
{ "major": 0, "minor": 11, "micro": 5, "package": "" }
Quoted 8 lines
Quoted 3 lines
major, minor & micro are all integer values. package is an arbitrary string whose format is defined by the OS package maintainer.
Looks good to me, a few comments:
1. Does QEMU have a naming convention for its versioning scheme or are we creating one right now?
There doesn't appear to be one, but with 3 entry tuples, calling them major, minor, micro is fairly standard terminology. Other suggestions welcome if people don't like that
Quoted 3 lines
2. The "package" member concerns me a bit, as package maintainers can put anything there, for example qemu-kvm has "(qemu-kvm-devel)" and we could have worst cases like: "-0.341.121-foobar-release".
This is entirely the point of the 'package' member really. It is there for OS distro package maintainers to put an arbitrary vendor specific version data in. In RPM world this would be the 'Release' field.
Quoted 6 lines
Perhaps we could let package alone and have a "downstream" member which could be either just a plain string or a dict with keys for name and version.
3. Actually, qemu-kvm has " (qemu-kvm-devel)", the leading whitespace is a bug, maybe you could address it in this patch in case we decide to stay with "package"?
KVM version I think would not be in the 'package' field, but in some other key.
Quoted 5 lines
Quoted 2 lines
There is no need to preserve the existing 'qemu' field, since QMP is not yet declared stable.
I've added the 'qemu' member to allow easy expansion in case we
decided to have a 'qmp' key.
Perhaps it should be nested then,
{
"qemu": { "major": 0, "minor": 11, "micro": 5, } "package": ""
}
To allow for multiple version numbers to be included in their fully split up format.
Quoted 15 lines
Quoted 15 lines
monitor.c +8
-2@@ -682,17 +685,33 @@ static void do_info_version_print(Monitor *mon, const QObject *data)* * Return a QDict with the following information: * -* - "qemu": QEMU's version -* - "package": package's version +* - "major": QEMU's major version +* - "minor": QEMU's minor version +* - "micro": QEMU's micro version +* - "package": QEMU packager's version +* +* The first three values are guarenteed to be +* integers. The final 'package' value is a string +* in an arbitrary packager specific format *
Quoted 8 lines
monitor.c static void do_info_version_print(Monitor *mon, const QObject *data) · line 700
Quoted 5 lines
monitor.c +1
-1* Example: * -* { "qemu": "0.11.50", "package": "" } +* { "major": 0, "minor": 11, "micro": 5, "package": "" } */ This comment doesn't exist anymore, now we should update qemu-monitor.hx
(search for 'query-version' there).
Ok, I'll fix this when next rebasing
Daniel
Signature · 4 lines
|: 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 :|