All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error
@ 2017-10-26 23:30 Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 1/6] qga: group agent init/cleanup init separate routines Michael Roth
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Michael Roth @ 2017-10-26 23:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: sameeh, yan, daniel

The following patches are also available at:
  https://github.com/mdroth/qemu/commits/qga-retry-path

This series adds a --retry-path option to qemu-ga to allow the agent
to periodically re-attempt opening a channel if the channel is not
yet present (e.g. qemu-ga is installed before virtio-serial drivers)
or if the channel encounters an error that might be recoverable (e.g.
the channel is hot unplugged/plugged).

This functionality is already possible via init systems like systemd (and
is already set up that way on Fedora), but we implement this option
to handle other OSs like Windows (where this was originally reported as
a problem).

PATCH 1-3 implement various refactorings that more clearly encapsulate
the agent's lifecycle into:

 a) config parsing (via main())
 b) initializing the agent (via initialize_agent())
 c) starting the agent (via run_agent())
 d) stopping the agent (via stop_agent())
 e) cleaning up the agent (via cleanup_agent())
 f) cleaning up the config (via main())

PATCH 4 implements the actual --retry-path option, which simply wraps
run_agent() in a continuous loop until the agent is explicitly told
to exit.

 qga/channel-win32.c       |   3 +-
 qga/installer/qemu-ga.wxs |   2 +-
 qga/main.c                | 205 ++++++++++++++++++++++++++++++++++++++++++++++++------------------------
 3 files changed, 141 insertions(+), 69 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/6] qga: group agent init/cleanup init separate routines
  2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
@ 2017-10-26 23:30 ` Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 2/6] qga: hang GAConfig/socket_activation off of GAState global Michael Roth
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Michael Roth @ 2017-10-26 23:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: sameeh, yan, daniel

This patch better separates the init/cleanup routines out into
separate functions to make make the start-up procedure a bit easier
to follow. This will be useful when we eventually break out the
actual start/stop of the agent's main loop into separates routines
that can be called multiple times after the init phase.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 82 ++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 50 insertions(+), 32 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 62a62755bd..73aa9ac507 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1250,9 +1250,21 @@ static bool check_is_frozen(GAState *s)
     return false;
 }
 
-static int run_agent(GAState *s, GAConfig *config, int socket_activation)
+static GAState *initialize_agent(GAConfig *config)
 {
-    ga_state = s;
+    GAState *s = g_new0(GAState, 1);
+
+    g_assert_null(ga_state);
+
+    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);
 
     g_log_set_default_handler(ga_log, s);
     g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
@@ -1268,7 +1280,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
     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;
+        return NULL;
     }
 #endif
 
@@ -1293,7 +1305,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
             if (!log_file) {
                 g_critical("unable to open specified log file: %s",
                            strerror(errno));
-                return EXIT_FAILURE;
+                return NULL;
             }
             s->log_file = log_file;
         }
@@ -1304,7 +1316,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
                                s->pstate_filepath,
                                ga_is_frozen(s))) {
         g_critical("failed to load persistent state");
-        return EXIT_FAILURE;
+        return NULL;
     }
 
     config->blacklist = ga_command_blacklist_init(config->blacklist);
@@ -1325,12 +1337,37 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
 #ifndef _WIN32
     if (!register_signal_handlers()) {
         g_critical("failed to register signal handlers");
-        return EXIT_FAILURE;
+        return NULL;
     }
 #endif
 
     s->main_loop = g_main_loop_new(NULL, false);
 
+    ga_state = s;
+    return s;
+}
+
+static void cleanup_agent(GAState *s)
+{
+    if (s->command_state) {
+        ga_command_state_cleanup_all(s->command_state);
+        ga_command_state_free(s->command_state);
+        json_message_parser_destroy(&s->parser);
+    }
+    if (s->channel) {
+        ga_channel_free(s->channel);
+    }
+    g_free(s->pstate_filepath);
+    g_free(s->state_filepath_isfrozen);
+    if (s->main_loop) {
+        g_main_loop_unref(s->main_loop);
+    }
+    g_free(s);
+    ga_state = NULL;
+}
+
+static int run_agent(GAState *s, GAConfig *config, int socket_activation)
+{
     if (!channel_init(ga_state, config->method, config->channel_path,
                       socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
         g_critical("failed to initialize guest agent channel");
@@ -1354,7 +1391,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
 int main(int argc, char **argv)
 {
     int ret = EXIT_SUCCESS;
-    GAState *s = g_new0(GAState, 1);
+    GAState *s;
     GAConfig *config = g_new0(GAConfig, 1);
     int socket_activation;
 
@@ -1422,44 +1459,25 @@ int main(int argc, char **argv)
         }
     }
 
-    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);
-
     if (config->dumpconf) {
         config_dump(config);
         goto end;
     }
 
+    s = initialize_agent(config);
+    if (!s) {
+        g_critical("error initializing guest agent");
+        goto end;
+    }
     ret = run_agent(s, config, socket_activation);
+    cleanup_agent(s);
 
 end:
-    if (s->command_state) {
-        ga_command_state_cleanup_all(s->command_state);
-        ga_command_state_free(s->command_state);
-        json_message_parser_destroy(&s->parser);
-    }
-    if (s->channel) {
-        ga_channel_free(s->channel);
-    }
-    g_free(s->pstate_filepath);
-    g_free(s->state_filepath_isfrozen);
-
     if (config->daemonize) {
         unlink(config->pid_filepath);
     }
 
     config_free(config);
-    if (s->main_loop) {
-        g_main_loop_unref(s->main_loop);
-    }
-    g_free(s);
 
     return ret;
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/6] qga: hang GAConfig/socket_activation off of GAState global
  2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 1/6] qga: group agent init/cleanup init separate routines Michael Roth
@ 2017-10-26 23:30 ` Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 3/6] qga: move w32 service handling out of run_agent() Michael Roth
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Michael Roth @ 2017-10-26 23:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: sameeh, yan, daniel

For w32 services we rely on the global GAState to access resources
associated with the agent within service_main(). Currently this is
sufficient for starting the agent since we open the channel once prior
to calling service_main(), and simply start the GMainLoop to start the
agent from within service_main().

Eventually we want to be able to also [re-]open the communication
channel from within service_main(), which requires access to
config/socket_activation variables, so we hang them off GAState in
preparation for that.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 54 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 73aa9ac507..6826cd5f45 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -66,6 +66,25 @@ typedef struct GAPersistentState {
     int64_t fd_counter;
 } GAPersistentState;
 
+typedef struct GAConfig {
+    char *channel_path;
+    char *method;
+    char *log_filepath;
+    char *pid_filepath;
+#ifdef CONFIG_FSFREEZE
+    char *fsfreeze_hook;
+#endif
+    char *state_dir;
+#ifdef _WIN32
+    const char *service;
+#endif
+    gchar *bliststr; /* blacklist may point to this string */
+    GList *blacklist;
+    int daemonize;
+    GLogLevelFlags log_level;
+    int dumpconf;
+} GAConfig;
+
 struct GAState {
     JSONMessageParser parser;
     GMainLoop *main_loop;
@@ -91,6 +110,8 @@ struct GAState {
 #endif
     gchar *pstate_filepath;
     GAPersistentState pstate;
+    GAConfig *config;
+    int socket_activation;
 };
 
 struct GAState *ga_state;
@@ -944,25 +965,6 @@ static GList *split_list(const gchar *str, const gchar *delim)
     return list;
 }
 
-typedef struct GAConfig {
-    char *channel_path;
-    char *method;
-    char *log_filepath;
-    char *pid_filepath;
-#ifdef CONFIG_FSFREEZE
-    char *fsfreeze_hook;
-#endif
-    char *state_dir;
-#ifdef _WIN32
-    const char *service;
-#endif
-    gchar *bliststr; /* blacklist may point to this string */
-    GList *blacklist;
-    int daemonize;
-    GLogLevelFlags log_level;
-    int dumpconf;
-} GAConfig;
-
 static void config_load(GAConfig *config)
 {
     GError *gerr = NULL;
@@ -1250,7 +1252,7 @@ static bool check_is_frozen(GAState *s)
     return false;
 }
 
-static GAState *initialize_agent(GAConfig *config)
+static GAState *initialize_agent(GAConfig *config, int socket_activation)
 {
     GAState *s = g_new0(GAState, 1);
 
@@ -1343,6 +1345,8 @@ static GAState *initialize_agent(GAConfig *config)
 
     s->main_loop = g_main_loop_new(NULL, false);
 
+    s->config = config;
+    s->socket_activation = socket_activation;
     ga_state = s;
     return s;
 }
@@ -1366,10 +1370,10 @@ static void cleanup_agent(GAState *s)
     ga_state = NULL;
 }
 
-static int run_agent(GAState *s, GAConfig *config, int socket_activation)
+static int run_agent(GAState *s)
 {
-    if (!channel_init(ga_state, config->method, config->channel_path,
-                      socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
+    if (!channel_init(s, s->config->method, s->config->channel_path,
+                      s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
         g_critical("failed to initialize guest agent channel");
         return EXIT_FAILURE;
     }
@@ -1464,12 +1468,12 @@ int main(int argc, char **argv)
         goto end;
     }
 
-    s = initialize_agent(config);
+    s = initialize_agent(config, socket_activation);
     if (!s) {
         g_critical("error initializing guest agent");
         goto end;
     }
-    ret = run_agent(s, config, socket_activation);
+    ret = run_agent(s);
     cleanup_agent(s);
 
 end:
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 3/6] qga: move w32 service handling out of run_agent()
  2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 1/6] qga: group agent init/cleanup init separate routines Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 2/6] qga: hang GAConfig/socket_activation off of GAState global Michael Roth
@ 2017-10-26 23:30 ` Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 4/6] qga: add --retry-path option for re-initializing channel on failure Michael Roth
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Michael Roth @ 2017-10-26 23:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: sameeh, yan, daniel

Eventually we want a w32 service to be able to restart the qga main
loop from within service_main(). To allow for this we move service
handling out of run_agent() such that service_main() calls
run_agent() instead of the reverse.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 6826cd5f45..2422ccf2f8 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -133,6 +133,7 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
                                   LPVOID ctx);
 VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
 #endif
+static int run_agent(GAState *s);
 
 static void
 init_dfl_pathnames(void)
@@ -768,7 +769,7 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[])
     service->status.dwWaitHint = 0;
     SetServiceStatus(service->status_handle, &service->status);
 
-    g_main_loop_run(ga_state->main_loop);
+    run_agent(ga_state);
 
     service->status.dwCurrentState = SERVICE_STOPPED;
     SetServiceStatus(service->status_handle, &service->status);
@@ -1377,17 +1378,8 @@ static int run_agent(GAState *s)
         g_critical("failed to initialize guest agent channel");
         return EXIT_FAILURE;
     }
-#ifndef _WIN32
+
     g_main_loop_run(ga_state->main_loop);
-#else
-    if (config->daemonize) {
-        SERVICE_TABLE_ENTRY service_table[] = {
-            { (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
-        StartServiceCtrlDispatcher(service_table);
-    } else {
-        g_main_loop_run(ga_state->main_loop);
-    }
-#endif
 
     return EXIT_SUCCESS;
 }
@@ -1473,7 +1465,18 @@ int main(int argc, char **argv)
         g_critical("error initializing guest agent");
         goto end;
     }
+
+#ifdef _WIN32
+    if (config->daemonize) {
+        SERVICE_TABLE_ENTRY service_table[] = {
+            { (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
+        StartServiceCtrlDispatcher(service_table);
+    } else {
+        ret = run_agent(s);
+    }
+#endif
     ret = run_agent(s);
+
     cleanup_agent(s);
 
 end:
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 4/6] qga: add --retry-path option for re-initializing channel on failure
  2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
                   ` (2 preceding siblings ...)
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 3/6] qga: move w32 service handling out of run_agent() Michael Roth
@ 2017-10-26 23:30 ` Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 5/6] qga-win: install service with --retry-path set by default Michael Roth
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Michael Roth @ 2017-10-26 23:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: sameeh, yan, daniel

This adds an option to instruct the agent to periodically attempt
re-opening the communication channel after a channel error has
occurred. The main use-case for this is providing an OS-independent
way of allowing the agent to survive situations like hotplug/unplug of
the communication channel, or initial guest set up where the agent may
be installed/started prior to the installation of the channel device's
driver.

There are nicer ways of implementing this functionality via things
like systemd services, but this option is useful for platforms like
*BSD/w32.

Currently a channel error will result in the GSource for that channel
being removed from the GMainLoop, but the main loop continuing to run.
That behavior results in a dead loop when --retry-path isn't set, and
prevents us from knowing when to attempt re-opening the channel when
it is set, so we also force the loop to exit as part of this patch.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 54 insertions(+), 8 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 2422ccf2f8..8154e70c3c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -55,6 +55,7 @@
 #endif
 #define QGA_SENTINEL_BYTE 0xFF
 #define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
+#define QGA_RETRY_INTERVAL 5
 
 static struct {
     const char *state_dir;
@@ -83,6 +84,7 @@ typedef struct GAConfig {
     int daemonize;
     GLogLevelFlags log_level;
     int dumpconf;
+    bool retry_path;
 } GAConfig;
 
 struct GAState {
@@ -112,6 +114,7 @@ struct GAState {
     GAPersistentState pstate;
     GAConfig *config;
     int socket_activation;
+    bool force_exit;
 };
 
 struct GAState *ga_state;
@@ -134,6 +137,7 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
 VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
 #endif
 static int run_agent(GAState *s);
+static void stop_agent(GAState *s, bool requested);
 
 static void
 init_dfl_pathnames(void)
@@ -182,9 +186,7 @@ static void quit_handler(int sig)
     }
     g_debug("received signal num %d, quitting", sig);
 
-    if (g_main_loop_is_running(ga_state->main_loop)) {
-        g_main_loop_quit(ga_state->main_loop);
-    }
+    stop_agent(ga_state, true);
 }
 
 #ifndef _WIN32
@@ -269,6 +271,10 @@ QEMU_COPYRIGHT "\n"
 "                    to list available RPCs)\n"
 "  -D, --dump-conf   dump a qemu-ga config file based on current config\n"
 "                    options / command-line parameters to stdout\n"
+"  -r, --retry-path  attempt re-opening path if it's unavailable or closed\n"
+"                    due to an error which may be recoverable in the future\n"
+"                    (virtio-serial driver re-install, serial device hot\n"
+"                    plug/unplug, etc.)\n"
 "  -h, --help        display this help and exit\n"
 "\n"
 QEMU_HELP_BOTTOM "\n"
@@ -670,6 +676,7 @@ static gboolean channel_event_cb(GIOCondition condition, gpointer data)
     switch (status) {
     case G_IO_STATUS_ERROR:
         g_warning("error reading channel");
+        stop_agent(s, false);
         return false;
     case G_IO_STATUS_NORMAL:
         buf[count] = 0;
@@ -1013,6 +1020,10 @@ static void config_load(GAConfig *config)
         /* enable all log levels */
         config->log_level = G_LOG_LEVEL_MASK;
     }
+    if (g_key_file_has_key(keyfile, "general", "retry-path", NULL)) {
+        config->retry_path =
+            g_key_file_get_boolean(keyfile, "general", "retry-path", &gerr);
+    }
     if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
         config->bliststr =
             g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
@@ -1074,6 +1085,8 @@ static void config_dump(GAConfig *config)
     g_key_file_set_string(keyfile, "general", "statedir", config->state_dir);
     g_key_file_set_boolean(keyfile, "general", "verbose",
                            config->log_level == G_LOG_LEVEL_MASK);
+    g_key_file_set_boolean(keyfile, "general", "retry-path",
+                           config->retry_path);
     tmp = list_join(config->blacklist, ',');
     g_key_file_set_string(keyfile, "general", "blacklist", tmp);
     g_free(tmp);
@@ -1092,7 +1105,7 @@ static void config_dump(GAConfig *config)
 
 static void config_parse(GAConfig *config, int argc, char **argv)
 {
-    const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
+    const char *sopt = "hVvdm:p:l:f:F::b:s:t:Dr";
     int opt_ind = 0, ch;
     const struct option lopt[] = {
         { "help", 0, NULL, 'h' },
@@ -1112,6 +1125,7 @@ static void config_parse(GAConfig *config, int argc, char **argv)
         { "service", 1, NULL, 's' },
 #endif
         { "statedir", 1, NULL, 't' },
+        { "retry-path", 0, NULL, 'r' },
         { NULL, 0, NULL, 0 }
     };
 
@@ -1156,6 +1170,9 @@ static void config_parse(GAConfig *config, int argc, char **argv)
         case 'D':
             config->dumpconf = 1;
             break;
+        case 'r':
+            config->retry_path = true;
+            break;
         case 'b': {
             if (is_help_option(optarg)) {
                 qmp_for_each_command(&ga_commands, ga_print_cmd, NULL);
@@ -1359,9 +1376,6 @@ static void cleanup_agent(GAState *s)
         ga_command_state_free(s->command_state);
         json_message_parser_destroy(&s->parser);
     }
-    if (s->channel) {
-        ga_channel_free(s->channel);
-    }
     g_free(s->pstate_filepath);
     g_free(s->state_filepath_isfrozen);
     if (s->main_loop) {
@@ -1371,7 +1385,7 @@ static void cleanup_agent(GAState *s)
     ga_state = NULL;
 }
 
-static int run_agent(GAState *s)
+static int run_agent_once(GAState *s)
 {
     if (!channel_init(s, s->config->method, s->config->channel_path,
                       s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
@@ -1381,9 +1395,41 @@ static int run_agent(GAState *s)
 
     g_main_loop_run(ga_state->main_loop);
 
+    if (s->channel) {
+        ga_channel_free(s->channel);
+    }
+
     return EXIT_SUCCESS;
 }
 
+static int run_agent(GAState *s)
+{
+    int ret = EXIT_SUCCESS;
+
+    s->force_exit = false;
+
+    do {
+        ret = run_agent_once(s);
+        if (s->config->retry_path && !s->force_exit) {
+            g_warning("agent stopped unexpectedly, restarting...");
+            sleep(QGA_RETRY_INTERVAL);
+        }
+    } while (s->config->retry_path && !s->force_exit);
+
+    return ret;
+}
+
+static void stop_agent(GAState *s, bool requested)
+{
+    if (!s->force_exit) {
+        s->force_exit = requested;
+    }
+
+    if (g_main_loop_is_running(s->main_loop)) {
+        g_main_loop_quit(s->main_loop);
+    }
+}
+
 int main(int argc, char **argv)
 {
     int ret = EXIT_SUCCESS;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 5/6] qga-win: install service with --retry-path set by default
  2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
                   ` (3 preceding siblings ...)
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 4/6] qga: add --retry-path option for re-initializing channel on failure Michael Roth
@ 2017-10-26 23:30 ` Michael Roth
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 6/6] qga-win: report specific error when failing to open channel Michael Roth
  2017-10-26 23:38 ` [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error no-reply
  6 siblings, 0 replies; 8+ messages in thread
From: Michael Roth @ 2017-10-26 23:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: sameeh, yan, daniel

It's nicer from a management perspective that the agent can survive
hotplug/unplug of the channel device, or be started prior to the
installation of the channel device's driver without and still be able
to resume normal function afterward. On linux there are alternatives
like systemd to support this, but on w32 --retry-path is the only
option so it makes sense to set it by default when installed as a
w32 service.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/installer/qemu-ga.wxs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 5af11627f8..fb2273507f 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -78,7 +78,7 @@
               Account="LocalSystem"
               ErrorControl="ignore"
               Interactive="no"
-              Arguments="-d"
+              Arguments="-d --retry-path"
               >
             </ServiceInstall>
             <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" />
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 6/6] qga-win: report specific error when failing to open channel
  2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
                   ` (4 preceding siblings ...)
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 5/6] qga-win: install service with --retry-path set by default Michael Roth
@ 2017-10-26 23:30 ` Michael Roth
  2017-10-26 23:38 ` [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error no-reply
  6 siblings, 0 replies; 8+ messages in thread
From: Michael Roth @ 2017-10-26 23:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: sameeh, yan, daniel

Useful in general, but especially now that errors might occur more
frequently with --retry-path set.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/channel-win32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 7e6dc4d26f..335542f748 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -302,7 +302,8 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
                            OPEN_EXISTING,
                            FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, NULL);
     if (c->handle == INVALID_HANDLE_VALUE) {
-        g_critical("error opening path %s", newpath);
+        g_critical("error opening path %s: %s", newpath,
+                   g_win32_error_message(GetLastError()));
         return false;
     }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error
  2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
                   ` (5 preceding siblings ...)
  2017-10-26 23:30 ` [Qemu-devel] [PATCH 6/6] qga-win: report specific error when failing to open channel Michael Roth
@ 2017-10-26 23:38 ` no-reply
  6 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2017-10-26 23:38 UTC (permalink / raw)
  To: mdroth; +Cc: famz, qemu-devel, yan, daniel, sameeh

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error
Type: series
Message-id: 20171026233054.21133-1-mdroth@linux.vnet.ibm.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20171026233054.21133-1-mdroth@linux.vnet.ibm.com -> patchew/20171026233054.21133-1-mdroth@linux.vnet.ibm.com
Switched to a new branch 'test'
2c0ad7d947 qga-win: report specific error when failing to open channel
0201eef61d qga-win: install service with --retry-path set by default
432d6851ff qga: add --retry-path option for re-initializing channel on failure
d242b19057 qga: move w32 service handling out of run_agent()
522ea4562e qga: hang GAConfig/socket_activation off of GAState global
615c58083f qga: group agent init/cleanup init separate routines

=== OUTPUT BEGIN ===
Checking PATCH 1/6: qga: group agent init/cleanup init separate routines...
ERROR: Use g_assert or g_assert_not_reached
#29: FILE: qga/main.c:1257:
+    g_assert_null(ga_state);

total: 1 errors, 0 warnings, 143 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/6: qga: hang GAConfig/socket_activation off of GAState global...
Checking PATCH 3/6: qga: move w32 service handling out of run_agent()...
Checking PATCH 4/6: qga: add --retry-path option for re-initializing channel on failure...
Checking PATCH 5/6: qga-win: install service with --retry-path set by default...
Checking PATCH 6/6: qga-win: report specific error when failing to open channel...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-10-26 23:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-26 23:30 [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error Michael Roth
2017-10-26 23:30 ` [Qemu-devel] [PATCH 1/6] qga: group agent init/cleanup init separate routines Michael Roth
2017-10-26 23:30 ` [Qemu-devel] [PATCH 2/6] qga: hang GAConfig/socket_activation off of GAState global Michael Roth
2017-10-26 23:30 ` [Qemu-devel] [PATCH 3/6] qga: move w32 service handling out of run_agent() Michael Roth
2017-10-26 23:30 ` [Qemu-devel] [PATCH 4/6] qga: add --retry-path option for re-initializing channel on failure Michael Roth
2017-10-26 23:30 ` [Qemu-devel] [PATCH 5/6] qga-win: install service with --retry-path set by default Michael Roth
2017-10-26 23:30 ` [Qemu-devel] [PATCH 6/6] qga-win: report specific error when failing to open channel Michael Roth
2017-10-26 23:38 ` [Qemu-devel] [PATCH 0/6] qga: add support for re-opening channel on error no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.