[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/3] Add optional migrate version to hmp migrate
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH 3/3] Add optional migrate version to hmp migrate command |
Date: |
Wed, 28 May 2014 13:37:20 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
"Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> 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 <address@hidden>
> 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.