[Qemu-devel] [PATCH 3/3] Add optional migrate version to hmp migrate command
From: Dr. David Alan Gilbert dgilb...@redhat.com
Uses the new 'optional parameter with string' parameter type
e.g.
migrate -v 2.0.0 (foo) exec: whatever
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
hmp-commands.hx | 7 ++--
hmp.c | 106 +++-
2 files changed, 109 insertions(+), 4 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 4cbceda..a3732f2 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -887,13 +887,14 @@ ETEXI
{
.name = migrate,
-.args_type = detach:-d,blk:-b,inc:-i,uri:s,
-.params = [-d] [-b] [-i] uri,
+.args_type = detach:-d,blk:-b,inc:-i,dest-version:+v,uri:s,
+.params = [-d] [-b] [-i] [-v version] uri,
.help = migrate to URI (using -d to not wait for completion)
\n\t\t\t -b for migration without shared storage with
full copy of disk\n\t\t\t -i for migration without
shared storage with incremental copy of disk
- (base image shared between src and destination),
+ (base image shared between src and destination)
+ \n\t\t\t -v to specify destination QEMU version,
.mhandler.cmd = hmp_migrate,
},
diff --git a/hmp.c b/hmp.c
index 411e4bc..e06d8c9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1272,18 +1272,122 @@ static void hmp_migrate_status_cb(void *opaque)
qapi_free_MigrationInfo(info);
}
+/* Parse a string into a VersionInfo structure
+ * if the input string is NULL, return NULL.
+ * The input is the same as that output by 'info version'
+ * example input:
+ *3.1.4 (arbitrary-package-name-3.1.4-stuff)
+ *2.0.0
+ */
+static VersionInfo *parse_version_info(Monitor *mon, const char *s)
+{
+char *tmps;
+VersionInfo *vi = NULL;
+const char *err_text;
+
+if (!s) {
+return NULL;
+}
+
+if (!*s) {
+err_text = Empty version string;
+goto badstring;
+}
+
+/*
+ * There must be an easier way; but scanf's %n is apparently not
+ * portable to check if we hit the end of the string after the numeric
+ * part.
+ */
+vi = g_malloc(sizeof(VersionInfo));
+errno = 0;
+if (!isdigit(*s)) {
+err_text = Parsing major version;
+goto badstring;
+}
+vi-qemu.major = (int)strtol(s, tmps, 10);
+if (errno || (*tmps != '.')) {
+err_text = Parsing major version;
+goto badstring;
+}
+if (!isdigit(*(++tmps))) {
+err_text = Parsing minor version;
+goto badstring;
+}
+vi-qemu.minor = (int)strtol(tmps, tmps, 10);
+if (errno || (*tmps != '.')) {
+err_text = Parsing minor version;
+goto badstring;
+}
+if (!isdigit(*(++tmps))) {
+err_text = Parsing micro version;
+goto badstring;
+}
+vi-qemu.micro = (int)strtol(tmps, tmps, 10);
+if (errno) {
+err_text = Parsing micro version;
+goto badstring;
+}
+
+/*
+ * At this point we're either at the end (fine) or we should have a
+ * version in brackets
+ */
+if (*tmps) {
+/* Expect a (package version) */
+char *open_bracket = strchr(tmps, '(');
+if (!open_bracket) {
+err_text = Finding open bracket;
+goto badstring;
+}
+char *close_bracket = strchr(open_bracket+1, ')');
+if (!close_bracket) {
+err_text = Finding close bracket;
+goto badstring;
+}
+
+tmps = g_strdup(open_bracket+1);
+*strchr(tmps, ')') = '\0';
+vi-package = tmps;
+} else {
+/* Just makes everything consistent later */
+vi-package = g_malloc(1);
+vi-package[0] = '\0';
+}
+
+return vi;
+
+badstring:
+monitor_printf(mon, Failed to parse version (%s)\n, err_text);
+if (vi) {
+g_free(vi);
+}
+return NULL;
+}
+
void hmp_migrate(Monitor *mon, const QDict *qdict)
{
int detach = qdict_get_try_bool(qdict, detach, 0);
int blk = qdict_get_try_bool(qdict, blk, 0);
int inc = qdict_get_try_bool(qdict, inc, 0);
const char *uri = qdict_get_str(qdict, uri);
+const char *dest_ver_s = qdict_get_try_str(qdict, dest-version);
+VersionInfo *vi = parse_version_info(mon, dest_ver_s);
Error *err = NULL;
+if (dest_ver_s !vi) {
+/* There's a version string but we couldn't parse it */
+return;
+}
+
qmp_migrate(uri, !!blk, blk, !!inc, inc,
false, false, /* detach */
-false, NULL, /* dest_version */
+!!vi, vi, /* dest_version */
err);
+if (vi) {
+qapi_free_VersionInfo(vi);
+}
+
if (err) {
monitor_printf(mon, migrate: %s\n, error_get_pretty(err));
error_free(err);
--
1.9.3
Re: [Qemu-devel] [PATCH 3/3] Add optional migrate version to hmp migrate command
Dr. David Alan Gilbert (git) dgilb...@redhat.com wrote:
From: Dr. David Alan Gilbert dgilb...@redhat.com
Uses the new 'optional parameter with string' parameter type
e.g.
migrate -v 2.0.0 (foo) exec: whatever
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
diff --git a/hmp.c b/hmp.c
index 411e4bc..e06d8c9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1272,18 +1272,122 @@ static void hmp_migrate_status_cb(void *opaque)
qapi_free_MigrationInfo(info);
}
+/* Parse a string into a VersionInfo structure
+ * if the input string is NULL, return NULL.
+ * The input is the same as that output by 'info version'
+ * example input:
+ *3.1.4 (arbitrary-package-name-3.1.4-stuff)
+ *2.0.0
+ */
This function is similar to qmp_query_version(), could we refactor them
to share the same code?
+static VersionInfo *parse_version_info(Monitor *mon, const char *s)
+{
+char *tmps;
+VersionInfo *vi = NULL;
+const char *err_text;
+
+if (!s) {
+return NULL;
+}
+
+if (!*s) {
+err_text = Empty version string;
+goto badstring;
+}
+
+/*
+ * There must be an easier way; but scanf's %n is apparently not
+ * portable to check if we hit the end of the string after the numeric
+ * part.
+ */
+vi = g_malloc(sizeof(VersionInfo));
+errno = 0;
+if (!isdigit(*s)) {
+err_text = Parsing major version;
+goto badstring;
+}
+vi-qemu.major = (int)strtol(s, tmps, 10);
qemu_query_version() don't need this cast.
And it has much less error checking.
+if (errno || (*tmps != '.')) {
+err_text = Parsing major version;
+goto badstring;
+}
+if (!isdigit(*(++tmps))) {
+err_text = Parsing minor version;
+goto badstring;
+}
+vi-qemu.minor = (int)strtol(tmps, tmps, 10);
+if (errno || (*tmps != '.')) {
+err_text = Parsing minor version;
+goto badstring;
+}
+if (!isdigit(*(++tmps))) {
+err_text = Parsing micro version;
+goto badstring;
+}
+vi-qemu.micro = (int)strtol(tmps, tmps, 10);
+if (errno) {
+err_text = Parsing micro version;
+goto badstring;
+}
+
+/*
+ * At this point we're either at the end (fine) or we should have a
+ * version in brackets
+ */
+if (*tmps) {
+/* Expect a (package version) */
+char *open_bracket = strchr(tmps, '(');
+if (!open_bracket) {
+err_text = Finding open bracket;
+goto badstring;
+}
+char *close_bracket = strchr(open_bracket+1, ')');
+if (!close_bracket) {
+err_text = Finding close bracket;
+goto badstring;
+}
+
+tmps = g_strdup(open_bracket+1);
+*strchr(tmps, ')') = '\0';
+vi-package = tmps;
+} else {
+/* Just makes everything consistent later */
+vi-package = g_malloc(1);
+vi-package[0] = '\0';
+}
+
+return vi;
+
+badstring:
+monitor_printf(mon, Failed to parse version (%s)\n, err_text);
+if (vi) {
+g_free(vi);
+}
+return NULL;
+}
Only thing that I can think about is that we could set the destination
version as a migration capability. Just a thought, don't know if it
would be better/worse. What do you think?
Later, Juan.
Re: [Qemu-devel] [PATCH 3/3] Add optional migrate version to hmp migrate command
* Juan Quintela (quint...@redhat.com) wrote:
Dr. David Alan Gilbert (git) dgilb...@redhat.com wrote:
From: Dr. David Alan Gilbert dgilb...@redhat.com
Uses the new 'optional parameter with string' parameter type
e.g.
migrate -v 2.0.0 (foo) exec: whatever
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
diff --git a/hmp.c b/hmp.c
index 411e4bc..e06d8c9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1272,18 +1272,122 @@ static void hmp_migrate_status_cb(void *opaque)
qapi_free_MigrationInfo(info);
}
+/* Parse a string into a VersionInfo structure
+ * if the input string is NULL, return NULL.
+ * The input is the same as that output by 'info version'
+ * example input:
+ *3.1.4 (arbitrary-package-name-3.1.4-stuff)
+ *2.0.0
+ */
This function is similar to qmp_query_version(), could we refactor them
to share the same code?
See below.
+static VersionInfo *parse_version_info(Monitor *mon, const char *s)
+{
+char *tmps;
+VersionInfo *vi = NULL;
+const char *err_text;
+
+if (!s) {
+return NULL;
+}
+
+if (!*s) {
+err_text = Empty version string;
+goto badstring;
+}
+
+/*
+ * There must be an easier way; but scanf's %n is apparently not
+ * portable to check if we hit the end of the string after the numeric
+ * part.
+ */
+vi = g_malloc(sizeof(VersionInfo));
+errno = 0;
+if (!isdigit(*s)) {
+err_text = Parsing major version;
+goto badstring;
+}
+vi-qemu.major = (int)strtol(s, tmps, 10);
qemu_query_version() don't need this cast.
And it has much less error checking.
Yes I think we might be able to share some of it with qemu_query_version
if you think it's worth it. This code is written to be quite picky since the
text is coming from the user.
And yes, you're right those casts can go.
+if (errno || (*tmps != '.')) {
+err_text = Parsing major version;
+goto badstring;
+}
+if (!isdigit(*(++tmps))) {
+err_text = Parsing minor version;
+goto badstring;
+}
+vi-qemu.minor = (int)strtol(tmps, tmps, 10);
+if (errno || (*tmps != '.')) {
+err_text = Parsing minor version;
+goto badstring;
+}
+if (!isdigit(*(++tmps))) {
+err_text = Parsing micro version;
+goto badstring;
+}
+vi-qemu.micro = (int)strtol(tmps, tmps, 10);
+if (errno) {
+err_text = Parsing micro version;
+goto badstring;
+}
+
+/*
+ * At this point we're either at the end (fine) or we should have a
+ * version in brackets
+ */
+if (*tmps) {
+/* Expect a (package version) */
+char *open_bracket = strchr(tmps, '(');
+if (!open_bracket) {
+err_text = Finding open bracket;
+goto badstring;
+}
+char *close_bracket = strchr(open_bracket+1, ')');
+if (!close_bracket) {
+err_text = Finding close bracket;
+goto badstring;
+}
+
+tmps = g_strdup(open_bracket+1);
+*strchr(tmps, ')') = '\0';
+vi-package = tmps;
+} else {
+/* Just makes everything consistent later */
+vi-package = g_malloc(1);
+vi-package[0] = '\0';
+}
+
+return vi;
+
+badstring:
+monitor_printf(mon, Failed to parse version (%s)\n, err_text);
+if (vi) {
+g_free(vi);
+}
+return NULL;
+}
Only thing that I can think about is that we could set the destination
version as a migration capability. Just a thought, don't know if it
would be better/worse. What do you think?
When I originally thought about this I was going to make it a capability,
and Daniel Berrange wondered why I hadn't made it a parameter to migrate :-)
I realised that if I made it a capability, the caller would have to be more
careful when doing a 2nd migration (e.g. after a failed migration) otherwise
there was a risk of the old version still being used. As a parameter to
'migrate' it's obviously safe against that type of bug.
Also, all the existing capabilities are simple numbers, so this seemed easier
than having to rework that.
Dave
Later, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
