Re: [Qemu-devel] [PATCH] Change 'query-version' to output broken down version string
On Fri, Jun 04, 2010 at 03:34:48PM -0300, Luiz Capitulino wrote:
On Wed, 2 Jun 2010 13:40:06 +0100
Daniel P. Berrange berra...@redhat.com 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.
Right.
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?
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
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.
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.
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.
@@ -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: }
*/
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
--
|: 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 :|
Re: [Qemu-devel] [PATCH] Change 'query-version' to output broken down version string
On Wed, 2 Jun 2010 13:40:06 +0100
Daniel P. Berrange berra...@redhat.com 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.
Right.
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.
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.
---
monitor.c | 33 ++---
1 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/monitor.c b/monitor.c
index 3ee365c..a33f7a8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -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: }
*/
This comment doesn't exist anymore, now we should update qemu-monitor.hx
(search for 'query-version' there).
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)
