Re: [Qemu-devel] [PATCH v3 08/12] qga: move agent run in a separate function

["Denis V. Lunev" <den-lists@parallels.com>]

On 08/26/2015 01:05 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Once the options are populated, move the running state to
> a run_agent() function.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>   qga/main.c | 164 +++++++++++++++++++++++++++++++++----------------------------
>   1 file changed, 89 insertions(+), 75 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 80f51fe..118847c 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1064,70 +1064,8 @@ static void config_free(GAConfig *config)
>       g_free(config);
>   }
>   
> -int main(int argc, char **argv)
> +static bool check_is_frozen(GAState *s)
>   {
> -    GAState *s;
> -    GAConfig *config;
> -
> -    module_call_init(MODULE_INIT_QAPI);
> -
> -    init_dfl_pathnames();
> -
> -    config = config_parse(argc, argv);
> -
> -    if (config->pid_filepath == NULL) {
> -        config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
> -    }
> -
> -    if (config->state_dir == NULL) {
> -        config->state_dir = g_strdup(dfl_pathnames.state_dir);
> -    }
> -
> -    if (config->method == NULL) {
> -        config->method = g_strdup("virtio-serial");
> -    }
> -
> -    if (config->channel_path == NULL) {
> -        if (strcmp(config->method, "virtio-serial") == 0) {
> -            /* try the default path for the virtio-serial port */
> -            config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
> -        } else if (strcmp(config->method, "isa-serial") == 0) {
> -            /* try the default path for the serial port - COM1 */
> -            config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
> -        } else {
> -            g_critical("must specify a path for this channel");
> -            goto out_bad;
> -        }
> -    }
> -
> -#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
> -     * the directory (with any intermediate directories). If we run into an
> -     * error later on, we won't try to clean up the directory, it is considered
> -     * persistent.
> -     */
> -    if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
> -        g_critical("unable to create (an ancestor of) the state directory"
> -                   " '%s': %s", config->state_dir, strerror(errno));
> -        return EXIT_FAILURE;
> -    }
> -#endif
> -
> -    s = g_malloc0(sizeof(GAState));
> -    s->log_level = config->log_level;
> -    s->log_file = stderr;
> -#ifdef CONFIG_FSFREEZE
> -    s->fsfreeze_hook = config->fsfreeze_hook;
> -#endif
> -    g_log_set_default_handler(ga_log, s);
> -    g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
> -    ga_enable_logging(s);
> -    s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
> -                                                 config->state_dir);
> -    s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
> -    s->frozen = false;
> -
>   #ifndef _WIN32
>       /* check if a previous instance of qemu-ga exited with filesystems' state
>        * marked as frozen. this could be a stale value (a non-qemu-ga process
> @@ -1153,7 +1091,31 @@ int main(int argc, char **argv)
>                     " guest-fsfreeze-thaw is issued, or filesystems are"
>                     " manually unfrozen and the file %s is removed",
>                     s->state_filepath_isfrozen);
> -        s->frozen = true;
> +        return true;
> +    }
> +#endif
> +    return false;
> +}
> +
> +static int run_agent(GAState *s, GAConfig *config)
> +{
> +    ga_state = s;
> +
> +    g_log_set_default_handler(ga_log, s);
> +    g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
> +    ga_enable_logging(s);
> +
> +#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
> +     * the directory (with any intermediate directories). If we run into an
> +     * error later on, we won't try to clean up the directory, it is considered
> +     * persistent.
> +     */
> +    if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
> +        g_critical("unable to create (an ancestor of) the state directory"
> +                   " '%s': %s", config->state_dir, strerror(errno));
> +        return EXIT_FAILURE;
>       }
>   #endif
>   
> @@ -1178,7 +1140,7 @@ int main(int argc, char **argv)
>               if (!log_file) {
>                   g_critical("unable to open specified log file: %s",
>                              strerror(errno));
> -                goto out_bad;
> +                return EXIT_FAILURE;
>               }
>               s->log_file = log_file;
>           }
> @@ -1189,7 +1151,7 @@ int main(int argc, char **argv)
>                                  s->pstate_filepath,
>                                  ga_is_frozen(s))) {
>           g_critical("failed to load persistent state");
> -        goto out_bad;
> +        return EXIT_FAILURE;
>       }
>   
>       config->blacklist = ga_command_blacklist_init(config->blacklist);
> @@ -1210,14 +1172,14 @@ int main(int argc, char **argv)
>   #ifndef _WIN32
>       if (!register_signal_handlers()) {
>           g_critical("failed to register signal handlers");
> -        goto out_bad;
> +        return EXIT_FAILURE;
>       }
>   #endif
>   
>       s->main_loop = g_main_loop_new(NULL, false);
>       if (!channel_init(ga_state, config->method, config->channel_path)) {
>           g_critical("failed to initialize guest agent channel");
> -        goto out_bad;
> +        return EXIT_FAILURE;
>       }
>   #ifndef _WIN32
>       g_main_loop_run(ga_state->main_loop);
> @@ -1231,20 +1193,72 @@ int main(int argc, char **argv)
>       }
>   #endif
>   
> -    ga_command_state_cleanup_all(ga_state->command_state);
> -    ga_channel_free(ga_state->channel);
> +    return EXIT_SUCCESS;
> +}
>   
> -    if (config->daemonize) {
> -        unlink(config->pid_filepath);
> +int main(int argc, char **argv)
> +{
> +    int ret = EXIT_SUCCESS;
> +    GAState *s = g_new0(GAState, 1);
> +    GAConfig *config;
> +
> +    module_call_init(MODULE_INIT_QAPI);
> +
> +    init_dfl_pathnames();
> +
> +    config = config_parse(argc, argv);
> +
> +    if (config->pid_filepath == NULL) {
> +        config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
> +    }
> +
> +    if (config->state_dir == NULL) {
> +        config->state_dir = g_strdup(dfl_pathnames.state_dir);
> +    }
> +
> +    if (config->method == NULL) {
> +        config->method = g_strdup("virtio-serial");
> +    }
> +
> +    if (config->channel_path == NULL) {
> +        if (strcmp(config->method, "virtio-serial") == 0) {
> +            /* try the default path for the virtio-serial port */
> +            config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
> +        } else if (strcmp(config->method, "isa-serial") == 0) {
> +            /* try the default path for the serial port - COM1 */
> +            config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
> +        } else {
> +            g_critical("must specify a path for this channel");
> +            ret = EXIT_FAILURE;
> +            goto end;
> +        }
> +    }
> +
> +    s->log_level = config->log_level;
> +    s->log_file = stderr;
> +#ifdef CONFIG_FSFREEZE
> +    s->fsfreeze_hook = config->fsfreeze_hook;
> +#endif
> +    s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
> +    s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
> +                                                 config->state_dir);
> +    s->frozen = check_is_frozen(s);
> +
> +    ret = run_agent(s, config);
> +
> +end:
> +    if (s->command_state) {
> +        ga_command_state_cleanup_all(s->command_state);
> +    }
> +    if (s->channel) {
> +        ga_channel_free(s->channel);
>       }
> -    return 0;
>   
> -out_bad:
>       if (config->daemonize) {
>           unlink(config->pid_filepath);
>       }
>   
>       config_free(config);
>   
> -    return EXIT_FAILURE;
> +    return ret;
>   }
Reviewed-by: Denis V. Lunev <den@openvz.org>