Re: [Qemu-devel] [PATCH v2 07/12] qga: fill default options in main()

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting marcandre.lureau@redhat.com (2015-08-25 17:10:19)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Fill all default options during main(). This is a preparation patch
> to allow to dump the configuration.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qga/main.c | 36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/qga/main.c b/qga/main.c
> index a8dda38..979ce5c 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -658,23 +658,6 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path)
>  {
>      GAChannelMethod channel_method;
> 
> -    if (method == NULL) {
> -        method = "virtio-serial";
> -    }
> -
> -    if (path == NULL) {
> -        if (strcmp(method, "virtio-serial") == 0 ) {
> -            /* try the default path for the virtio-serial port */
> -            path = QGA_VIRTIO_PATH_DEFAULT;
> -        } else if (strcmp(method, "isa-serial") == 0){
> -            /* try the default path for the serial port - COM1 */
> -            path = QGA_SERIAL_PATH_DEFAULT;
> -        } else {
> -            g_critical("must specify a path for this channel");
> -            return false;
> -        }
> -    }
> -
>      if (strcmp(method, "virtio-serial") == 0) {
>          s->virtio = true; /* virtio requires special handling in some cases */
>          channel_method = GA_CHANNEL_VIRTIO_SERIAL;
> @@ -1081,7 +1064,7 @@ static void config_free(GAConfig *config)
>      g_free(config);
>  }
> 
> - int main(int argc, char **argv)
> +int main(int argc, char **argv)
>  {
>      GAState *s;
>      GAConfig *config;
> @@ -1100,6 +1083,23 @@ static void config_free(GAConfig *config)
>          config->state_dir = g_strdup(dfl_pathnames.state_dir);
>      }
> 
> +    if (method == NULL) {
> +        method = g_strdup("virtio-serial");
> +    }
> +
> +    if (device_path == NULL) {
> +        if (strcmp(method, "virtio-serial") == 0) {
> +            /* try the default path for the virtio-serial port */
> +            device_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
> +        } else if (strcmp(method, "isa-serial") == 0) {
> +            /* try the default path for the serial port - COM1 */
> +            device_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
> +        } else {
> +            g_critical("must specify a path for this channel");
> +            goto out_bad;
> +        }
> +    }
> +

I think this needs to be config->method and config->device_path. It gets
fixed in the next patch, but looks like it'll break build bisect.

Since that requires a respin anyway: there's an extra space in
'int main()' that gets added in a prior patch then removed here, so
might as well fix that up while we're there.

>  #ifdef _WIN32
>      /* On win32 the state directory is application specific (be it the default
>       * or a user override). We got past the command line parsing; let's create
> -- 
> 2.4.3
>