[Qemu-devel] [PATCH 17/26] qga: move agent run in a separate function

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

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>
Reviewed-by: Denis V. Lunev <den@openvz.org>
*fixed up an s/ga_state/s/ artifact causing segfault
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 166 +++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 90 insertions(+), 76 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 38ee196..0fef3be 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1059,70 +1059,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
@@ -1148,7 +1086,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
 
@@ -1173,7 +1135,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;
         }
@@ -1184,7 +1146,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);
@@ -1205,14 +1167,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);
@@ -1226,21 +1188,73 @@ int main(int argc, char **argv)
     }
 #endif
 
-    g_list_free_full(ga_state->blacklist, g_free);
-    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);
     }
-    return 0;
 
-out_bad:
+    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);
+    }
+    g_list_free_full(s->blacklist, g_free);
+
     if (config->daemonize) {
         unlink(config->pid_filepath);
     }
 
     config_free(config);
 
-    return EXIT_FAILURE;
+    return ret;
 }
-- 
1.9.1