From 9e8aded432884477bcd4fa1c7e849a196412bcc4 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Mon, 16 Apr 2012 19:52:17 -0500 Subject: [PATCH 1/3] qemu-ga: improve recovery options for fsfreeze guest-fsfreeze-thaw relies on state information obtained from guest-fsfreeze-freeze to determine what filesystems to unfreeze. This is unreliable due to the fact that that state does not account for FIFREEZE being issued by other processes, or previous instances of qemu-ga. This means in certain situations we cannot thaw filesystems even with a responsive qemu-ga instance at our disposal. This patch allows guest-fsfreeze-thaw to be issued unconditionally. It also adds some additional logic to allow us to thaw filesystems regardless of how many times the filesystem's "frozen" refcount has been incremented by any guest processes. Also, guest-fsfreeze-freeze now operates atomically: on success all freezable filesystems are frozen, and on error all filesystems are thawed. The ambiguous "GUEST_FSFREEZE_STATUS_ERROR" state is no longer entered. Signed-off-by: Michael Roth --- qapi-schema-guest.json | 26 +++++--- qga/commands-posix.c | 139 +++++++++++++++++++++++++---------------- 2 files changed, 101 insertions(+), 64 deletions(-) diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index cf18876c57..0eedb98765 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -296,14 +296,10 @@ # # @frozen: all non-network guest filesystems frozen # -# @error: failure to thaw 1 or more -# previously frozen filesystems, or failure to open a previously -# cached filesytem (filesystem unmounted/directory changes, etc). -# # Since: 0.15.0 ## { 'enum': 'GuestFsfreezeStatus', - 'data': [ 'thawed', 'frozen', 'error' ] } + 'data': [ 'thawed', 'frozen' ] } ## # @guest-fsfreeze-status: @@ -312,6 +308,10 @@ # # Returns: GuestFsfreezeStatus ("thawed", "frozen", etc., as defined below) # +# Note: This may fail to properly report the current state as a result of +# qemu-ga having been restarted, or other guest processes having issued +# an fs freeze/thaw. +# # Since: 0.15.0 ## { 'command': 'guest-fsfreeze-status', @@ -320,9 +320,10 @@ ## # @guest-fsfreeze-freeze: # -# Sync and freeze all non-network guest filesystems +# Sync and freeze all freezable, local guest filesystems # -# Returns: Number of file systems frozen on success +# Returns: Number of file systems currently frozen. On error, all filesystems +# will be thawed. # # Since: 0.15.0 ## @@ -332,10 +333,15 @@ ## # @guest-fsfreeze-thaw: # -# Unfreeze frozen guest fileystems +# Unfreeze all frozen guest filesystems # -# Returns: Number of file systems thawed -# If error, -1 (unknown error) or -errno +# Returns: Number of file systems thawed by this call +# +# Note: if return value does not match the previous call to +# guest-fsfreeze-freeze, this likely means some freezable +# filesystems were unfrozen before this call, and that the +# filesystem state may have changed before issuing this +# command. # # Since: 0.15.0 ## diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 087c3af7ff..869d6ee831 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -332,28 +332,38 @@ typedef struct GuestFsfreezeMount { QTAILQ_ENTRY(GuestFsfreezeMount) next; } GuestFsfreezeMount; +typedef QTAILQ_HEAD(, GuestFsfreezeMount) GuestFsfreezeMountList; + struct { GuestFsfreezeStatus status; - QTAILQ_HEAD(, GuestFsfreezeMount) mount_list; } guest_fsfreeze_state; +static void guest_fsfreeze_free_mount_list(GuestFsfreezeMountList *mounts) +{ + GuestFsfreezeMount *mount, *temp; + + if (!mounts) { + return; + } + + QTAILQ_FOREACH_SAFE(mount, mounts, next, temp) { + QTAILQ_REMOVE(mounts, mount, next); + g_free(mount->dirname); + g_free(mount->devtype); + g_free(mount); + } +} + /* * Walk the mount table and build a list of local file systems */ -static int guest_fsfreeze_build_mount_list(void) +static int guest_fsfreeze_build_mount_list(GuestFsfreezeMountList *mounts) { struct mntent *ment; - GuestFsfreezeMount *mount, *temp; + GuestFsfreezeMount *mount; char const *mtab = MOUNTED; FILE *fp; - QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) { - QTAILQ_REMOVE(&guest_fsfreeze_state.mount_list, mount, next); - g_free(mount->dirname); - g_free(mount->devtype); - g_free(mount); - } - fp = setmntent(mtab, "r"); if (!fp) { g_warning("fsfreeze: unable to read mtab"); @@ -377,7 +387,7 @@ static int guest_fsfreeze_build_mount_list(void) mount->dirname = g_strdup(ment->mnt_dir); mount->devtype = g_strdup(ment->mnt_type); - QTAILQ_INSERT_TAIL(&guest_fsfreeze_state.mount_list, mount, next); + QTAILQ_INSERT_TAIL(mounts, mount, next); } endmntent(fp); @@ -400,17 +410,15 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) int64_t qmp_guest_fsfreeze_freeze(Error **err) { int ret = 0, i = 0; - struct GuestFsfreezeMount *mount, *temp; + GuestFsfreezeMountList mounts; + struct GuestFsfreezeMount *mount; int fd; char err_msg[512]; slog("guest-fsfreeze called"); - if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) { - return 0; - } - - ret = guest_fsfreeze_build_mount_list(); + QTAILQ_INIT(&mounts); + ret = guest_fsfreeze_build_mount_list(&mounts); if (ret < 0) { return ret; } @@ -418,43 +426,46 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) /* cannot risk guest agent blocking itself on a write in this state */ disable_logging(); - QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) { + QTAILQ_FOREACH(mount, &mounts, next) { fd = qemu_open(mount->dirname, O_RDONLY); if (fd == -1) { - sprintf(err_msg, "failed to open %s, %s", mount->dirname, strerror(errno)); + sprintf(err_msg, "failed to open %s, %s", mount->dirname, + strerror(errno)); error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); goto error; } /* we try to cull filesytems we know won't work in advance, but other * filesytems may not implement fsfreeze for less obvious reasons. - * these will report EOPNOTSUPP, so we simply ignore them. when - * thawing, these filesystems will return an EINVAL instead, due to - * not being in a frozen state. Other filesystem-specific - * errors may result in EINVAL, however, so the user should check the - * number * of filesystems returned here against those returned by the - * thaw operation to determine whether everything completed - * successfully + * these will report EOPNOTSUPP. we simply ignore these when tallying + * the number of frozen filesystems. + * + * any other error means a failure to freeze a filesystem we + * expect to be freezable, so return an error in those cases + * and return system to thawed state. */ ret = ioctl(fd, FIFREEZE); - if (ret < 0 && errno != EOPNOTSUPP) { - sprintf(err_msg, "failed to freeze %s, %s", mount->dirname, strerror(errno)); - error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); - close(fd); - goto error; + if (ret == -1) { + if (errno != EOPNOTSUPP) { + sprintf(err_msg, "failed to freeze %s, %s", + mount->dirname, strerror(errno)); + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg); + close(fd); + goto error; + } + } else { + i++; } close(fd); - - i++; } guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_FROZEN; + guest_fsfreeze_free_mount_list(&mounts); return i; error: - if (i > 0) { - qmp_guest_fsfreeze_thaw(NULL); - } + guest_fsfreeze_free_mount_list(&mounts); + qmp_guest_fsfreeze_thaw(NULL); return 0; } @@ -464,39 +475,59 @@ error: int64_t qmp_guest_fsfreeze_thaw(Error **err) { int ret; - GuestFsfreezeMount *mount, *temp; - int fd, i = 0; - bool has_error = false; + GuestFsfreezeMountList mounts; + GuestFsfreezeMount *mount; + int fd, i = 0, logged; - QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) { + QTAILQ_INIT(&mounts); + ret = guest_fsfreeze_build_mount_list(&mounts); + if (ret) { + error_set(err, QERR_QGA_COMMAND_FAILED, + "failed to enumerate filesystems"); + return 0; + } + + QTAILQ_FOREACH(mount, &mounts, next) { + logged = false; fd = qemu_open(mount->dirname, O_RDONLY); if (fd == -1) { - has_error = true; - continue; - } - ret = ioctl(fd, FITHAW); - if (ret < 0 && errno != EOPNOTSUPP && errno != EINVAL) { - has_error = true; - close(fd); continue; } + /* we have no way of knowing whether a filesystem was actually unfrozen + * as a result of a successful call to FITHAW, only that if an error + * was returned the filesystem was *not* unfrozen by that particular + * call. + * + * since multiple preceeding FIFREEZEs require multiple calls to FITHAW + * to unfreeze, continuing issuing FITHAW until an error is returned, + * in which case either the filesystem is in an unfreezable state, or, + * more likely, it was thawed previously (and remains so afterward). + * + * also, since the most recent successful call is the one that did + * the actual unfreeze, we can use this to provide an accurate count + * of the number of filesystems unfrozen by guest-fsfreeze-thaw, which + * may * be useful for determining whether a filesystem was unfrozen + * during the freeze/thaw phase by a process other than qemu-ga. + */ + do { + ret = ioctl(fd, FITHAW); + if (ret == 0 && !logged) { + i++; + logged = true; + } + } while (ret == 0); close(fd); - i++; } - if (has_error) { - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_ERROR; - } else { - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED; - } + guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED; enable_logging(); + guest_fsfreeze_free_mount_list(&mounts); return i; } static void guest_fsfreeze_init(void) { guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED; - QTAILQ_INIT(&guest_fsfreeze_state.mount_list); } static void guest_fsfreeze_cleanup(void) From f22d85e9e67262db34504f4079745f9843da6a92 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Tue, 17 Apr 2012 19:01:45 -0500 Subject: [PATCH 2/3] qemu-ga: add a whitelist for fsfreeze-safe commands Currently we rely on fsfreeze/thaw commands disabling/enabling logging then having other commands check whether logging is disabled to avoid executing if they aren't safe for running while a filesystem is frozen. Instead, have an explicit whitelist of fsfreeze-safe commands, and consolidate logging and command enablement/disablement into a pair of helper functions: ga_set_frozen()/ga_unset_frozen() Signed-off-by: Michael Roth --- qapi/qmp-core.h | 1 + qapi/qmp-registry.c | 14 ++++- qemu-ga.c | 120 +++++++++++++++++++++++++++++++++++++++-- qga/commands-posix.c | 35 ++++-------- qga/guest-agent-core.h | 3 ++ 5 files changed, 140 insertions(+), 33 deletions(-) diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h index 3bb3acb589..431ddbb337 100644 --- a/qapi/qmp-core.h +++ b/qapi/qmp-core.h @@ -38,6 +38,7 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn); QmpCommand *qmp_find_command(const char *name); QObject *qmp_dispatch(QObject *request); void qmp_disable_command(const char *name); +void qmp_enable_command(const char *name); bool qmp_command_is_enabled(const char *name); char **qmp_get_command_list(void); diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c index 25c89ad098..43d5cdeb64 100644 --- a/qapi/qmp-registry.c +++ b/qapi/qmp-registry.c @@ -40,18 +40,28 @@ QmpCommand *qmp_find_command(const char *name) return NULL; } -void qmp_disable_command(const char *name) +static void qmp_toggle_command(const char *name, bool enabled) { QmpCommand *cmd; QTAILQ_FOREACH(cmd, &qmp_commands, node) { if (strcmp(cmd->name, name) == 0) { - cmd->enabled = false; + cmd->enabled = enabled; return; } } } +void qmp_disable_command(const char *name) +{ + qmp_toggle_command(name, false); +} + +void qmp_enable_command(const char *name) +{ + qmp_toggle_command(name, true); +} + bool qmp_command_is_enabled(const char *name) { QmpCommand *cmd; diff --git a/qemu-ga.c b/qemu-ga.c index 74a1b02c68..ac29b733ef 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -56,10 +56,22 @@ struct GAState { GAService service; #endif bool delimit_response; + bool frozen; + GList *blacklist; }; struct GAState *ga_state; +/* commands that are safe to issue while filesystems are frozen */ +static const char *ga_freeze_whitelist[] = { + "guest-ping", + "guest-info", + "guest-sync", + "guest-fsfreeze-status", + "guest-fsfreeze-thaw", + NULL +}; + #ifdef _WIN32 DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data, LPVOID ctx); @@ -68,6 +80,15 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[]); static void quit_handler(int sig) { + /* if we're frozen, don't exit unless we're absolutely forced to, + * because it's basically impossible for graceful exit to complete + * unless all log/pid files are on unfreezable filesystems. there's + * also a very likely chance killing the agent before unfreezing + * the filesystems is a mistake (or will be viewed as one later). + */ + if (ga_is_frozen(ga_state)) { + return; + } g_debug("received signal num %d, quitting", sig); if (g_main_loop_is_running(ga_state->main_loop)) { @@ -206,6 +227,87 @@ void ga_set_response_delimited(GAState *s) s->delimit_response = true; } +static gint ga_strcmp(gconstpointer str1, gconstpointer str2) +{ + return strcmp(str1, str2); +} + +/* disable commands that aren't safe for fsfreeze */ +static void ga_disable_non_whitelisted(void) +{ + char **list_head, **list; + bool whitelisted; + int i; + + list_head = list = qmp_get_command_list(); + while (*list != NULL) { + whitelisted = false; + i = 0; + while (ga_freeze_whitelist[i] != NULL) { + if (strcmp(*list, ga_freeze_whitelist[i]) == 0) { + whitelisted = true; + } + i++; + } + if (!whitelisted) { + g_debug("disabling command: %s", *list); + qmp_disable_command(*list); + } + g_free(*list); + list++; + } + g_free(list_head); +} + +/* [re-]enable all commands, except those explictly blacklisted by user */ +static void ga_enable_non_blacklisted(GList *blacklist) +{ + char **list_head, **list; + + list_head = list = qmp_get_command_list(); + while (*list != NULL) { + if (g_list_find_custom(blacklist, *list, ga_strcmp) == NULL && + !qmp_command_is_enabled(*list)) { + g_debug("enabling command: %s", *list); + qmp_enable_command(*list); + } + g_free(*list); + list++; + } + g_free(list_head); +} + +bool ga_is_frozen(GAState *s) +{ + return s->frozen; +} + +void ga_set_frozen(GAState *s) +{ + if (ga_is_frozen(s)) { + return; + } + /* disable all non-whitelisted (for frozen state) commands */ + ga_disable_non_whitelisted(); + g_warning("disabling logging due to filesystem freeze"); + ga_disable_logging(s); + s->frozen = true; +} + +void ga_unset_frozen(GAState *s) +{ + if (!ga_is_frozen(s)) { + return; + } + + ga_enable_logging(s); + g_warning("logging re-enabled"); + + /* enable all disabled, non-blacklisted commands */ + ga_enable_non_blacklisted(s->blacklist); + s->frozen = false; +} + #ifndef _WIN32 static void become_daemon(const char *pidfile) { @@ -513,12 +615,13 @@ int main(int argc, char **argv) { "blacklist", 1, NULL, 'b' }, #ifdef _WIN32 { "service", 1, NULL, 's' }, -#endif +#endif { NULL, 0, NULL, 0 } }; int opt_ind = 0, ch, daemonize = 0, i, j, len; GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; FILE *log_file = stderr; + GList *blacklist = NULL; GAState *s; module_call_init(MODULE_INIT_QAPI); @@ -568,14 +671,12 @@ int main(int argc, char **argv) for (j = 0, i = 0, len = strlen(optarg); i < len; i++) { if (optarg[i] == ',') { optarg[i] = 0; - qmp_disable_command(&optarg[j]); - g_debug("disabling command: %s", &optarg[j]); + blacklist = g_list_append(blacklist, &optarg[j]); j = i + 1; } } if (j < i) { - qmp_disable_command(&optarg[j]); - g_debug("disabling command: %s", &optarg[j]); + blacklist = g_list_append(blacklist, &optarg[j]); } break; } @@ -615,6 +716,15 @@ int main(int argc, char **argv) g_log_set_default_handler(ga_log, s); g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR); s->logging_enabled = true; + s->frozen = false; + if (blacklist) { + s->blacklist = blacklist; + do { + g_debug("disabling command: %s", (char *)blacklist->data); + qmp_disable_command(blacklist->data); + blacklist = g_list_next(blacklist); + } while (blacklist); + } s->command_state = ga_command_state_new(); ga_command_state_init(s, s->command_state); ga_command_state_init_all(s->command_state); diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 869d6ee831..d58730ad80 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -316,16 +316,6 @@ static void guest_file_init(void) #if defined(CONFIG_FSFREEZE) -static void disable_logging(void) -{ - ga_disable_logging(ga_state); -} - -static void enable_logging(void) -{ - ga_enable_logging(ga_state); -} - typedef struct GuestFsfreezeMount { char *dirname; char *devtype; @@ -334,10 +324,6 @@ typedef struct GuestFsfreezeMount { typedef QTAILQ_HEAD(, GuestFsfreezeMount) GuestFsfreezeMountList; -struct { - GuestFsfreezeStatus status; -} guest_fsfreeze_state; - static void guest_fsfreeze_free_mount_list(GuestFsfreezeMountList *mounts) { GuestFsfreezeMount *mount, *temp; @@ -400,7 +386,11 @@ static int guest_fsfreeze_build_mount_list(GuestFsfreezeMountList *mounts) */ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) { - return guest_fsfreeze_state.status; + if (ga_is_frozen(ga_state)) { + return GUEST_FSFREEZE_STATUS_FROZEN; + } + + return GUEST_FSFREEZE_STATUS_THAWED; } /* @@ -424,7 +414,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) } /* cannot risk guest agent blocking itself on a write in this state */ - disable_logging(); + ga_set_frozen(ga_state); QTAILQ_FOREACH(mount, &mounts, next) { fd = qemu_open(mount->dirname, O_RDONLY); @@ -459,7 +449,6 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) close(fd); } - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_FROZEN; guest_fsfreeze_free_mount_list(&mounts); return i; @@ -519,23 +508,17 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) close(fd); } - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED; - enable_logging(); + ga_unset_frozen(ga_state); guest_fsfreeze_free_mount_list(&mounts); return i; } -static void guest_fsfreeze_init(void) -{ - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED; -} - static void guest_fsfreeze_cleanup(void) { int64_t ret; Error *err = NULL; - if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) { + if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) { ret = qmp_guest_fsfreeze_thaw(&err); if (ret < 0 || err) { slog("failed to clean up frozen filesystems"); @@ -964,7 +947,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) void ga_command_state_init(GAState *s, GACommandState *cs) { #if defined(CONFIG_FSFREEZE) - ga_command_state_add(cs, guest_fsfreeze_init, guest_fsfreeze_cleanup); + ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup); #endif ga_command_state_add(cs, guest_file_init, NULL); } diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h index 304525d3c2..bbb8b9b125 100644 --- a/qga/guest-agent-core.h +++ b/qga/guest-agent-core.h @@ -32,3 +32,6 @@ void ga_disable_logging(GAState *s); void ga_enable_logging(GAState *s); void slog(const gchar *fmt, ...); void ga_set_response_delimited(GAState *s); +bool ga_is_frozen(GAState *s); +void ga_set_frozen(GAState *s); +void ga_unset_frozen(GAState *s); From f789aa7baff33e74c549a249aba3ae7a364d7642 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Wed, 18 Apr 2012 16:28:01 -0500 Subject: [PATCH 3/3] qemu-ga: persist tracking of fsfreeze state via filesystem Currently, qemu-ga may die/get killed/go away for whatever reason after guest-fsfreeze-freeze has been issued, and before guest-fsfreeze-thaw has been issued. This means the only way to unfreeze the guest is via VNC/network/console access, but obtaining that access after-the-fact can often be very difficult when filesystems are frozen. Logins will almost always hang, for instance. In many cases the only recourse would be to reboot the guest without any quiescing of volatile state, which makes this a corner-case worth giving some attention to. A likely failsafe for this situation would be to use a watchdog to restart qemu-ga if it goes away. There are some precautions qemu-ga needs to take in order to avoid immediately hanging itself on I/O, however, namely, we must disable logging and defer to processing/creation of user-specific logfiles, along with creation of the pid file if we're running as a daemon. We also need to disable non-fsfreeze-safe commands, as we normally would when processing the guest-fsfreeze-freeze command. To track when we need to do this in a way that persists between multiple invocations of qemu-ga, we create a file on the guest filesystem before issuing the fsfreeze, and delete it when doing the thaw. On qemu-ga startup, we check for the existance of this file to determine the need to take the above precautions. We're forced to do it this way since a more traditional approach such as reading/writing state to a dedicated state file will cause access/modification time updates, respectively, both of which will hang if the file resides on a frozen filesystem. Both can occur even if relatime is enabled. Checking for file existence will not update the access time, however, so it's a safe way to check for fsfreeze state. An actual watchdog-based restart of qemu-ga can itself cause an access time update that would thus hang the invocation of qemu-ga, but the logic to workaround that can be handled via the watchdog, so we don't address that here (for relatime we'd periodically touch the qemu-ga binary if the file $qga_statedir/qga.state.isfrozen is not present, this avoids qemu-ga updates or the 1 day relatime threshold causing an access-time update if we try to respawn qemu-ga shortly after it goes away) Signed-off-by: Michael Roth --- qapi-schema-guest.json | 3 +- qemu-ga.c | 220 ++++++++++++++++++++++++++++++++--------- 2 files changed, 175 insertions(+), 48 deletions(-) diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index 0eedb98765..d7a073ee70 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -309,8 +309,7 @@ # Returns: GuestFsfreezeStatus ("thawed", "frozen", etc., as defined below) # # Note: This may fail to properly report the current state as a result of -# qemu-ga having been restarted, or other guest processes having issued -# an fs freeze/thaw. +# some other guest processes having issued an fs freeze/thaw. # # Since: 0.15.0 ## diff --git a/qemu-ga.c b/qemu-ga.c index ac29b733ef..216be39072 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -18,6 +18,7 @@ #ifndef _WIN32 #include #include +#include #endif #include "json-streamer.h" #include "json-parser.h" @@ -41,6 +42,7 @@ #define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0" #endif #define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid" +#define QGA_STATEDIR_DEFAULT "/tmp" #define QGA_SENTINEL_BYTE 0xFF struct GAState { @@ -58,6 +60,11 @@ struct GAState { bool delimit_response; bool frozen; GList *blacklist; + const char *state_filepath_isfrozen; + struct { + const char *log_filepath; + const char *pid_filepath; + } deferred_options; }; struct GAState *ga_state; @@ -147,6 +154,8 @@ static void usage(const char *cmd) " %s)\n" " -l, --logfile set logfile path, logs to stderr by default\n" " -f, --pidfile specify pidfile (default is %s)\n" +" -t, --statedir specify dir to store state information (absolute paths\n" +" only, default is %s)\n" " -v, --verbose log extra debugging information\n" " -V, --version print version information and exit\n" " -d, --daemonize become a daemon\n" @@ -158,7 +167,8 @@ static void usage(const char *cmd) " -h, --help display this help and exit\n" "\n" "Report bugs to \n" - , cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT); + , cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT, + QGA_STATEDIR_DEFAULT); } static const char *ga_log_level_str(GLogLevelFlags level) @@ -227,6 +237,41 @@ void ga_set_response_delimited(GAState *s) s->delimit_response = true; } +#ifndef _WIN32 +static bool ga_open_pidfile(const char *pidfile) +{ + int pidfd; + char pidstr[32]; + + pidfd = open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR); + if (pidfd == -1 || lockf(pidfd, F_TLOCK, 0)) { + g_critical("Cannot lock pid file, %s", strerror(errno)); + return false; + } + + if (ftruncate(pidfd, 0) || lseek(pidfd, 0, SEEK_SET)) { + g_critical("Failed to truncate pid file"); + goto fail; + } + sprintf(pidstr, "%d", getpid()); + if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr)) { + g_critical("Failed to write pid file"); + goto fail; + } + + return true; + +fail: + unlink(pidfile); + return false; +} +#else /* _WIN32 */ +static bool ga_open_pidfile(const char *pidfile) +{ + return true; +} +#endif + static gint ga_strcmp(gconstpointer str1, gconstpointer str2) { return strcmp(str1, str2); @@ -277,6 +322,28 @@ static void ga_enable_non_blacklisted(GList *blacklist) g_free(list_head); } +static bool ga_create_file(const char *path) +{ + int fd = open(path, O_CREAT | O_WRONLY, S_IWUSR | S_IRUSR); + if (fd == -1) { + g_warning("unable to open/create file %s: %s", path, strerror(errno)); + return false; + } + close(fd); + return true; +} + +static bool ga_delete_file(const char *path) +{ + int ret = unlink(path); + if (ret == -1) { + g_warning("unable to delete file: %s: %s", path, strerror(errno)); + return false; + } + + return true; +} + bool ga_is_frozen(GAState *s) { return s->frozen; @@ -292,6 +359,10 @@ void ga_set_frozen(GAState *s) g_warning("disabling logging due to filesystem freeze"); ga_disable_logging(s); s->frozen = true; + if (!ga_create_file(s->state_filepath_isfrozen)) { + g_warning("unable to create %s, fsfreeze may not function properly", + s->state_filepath_isfrozen); + } } void ga_unset_frozen(GAState *s) @@ -300,20 +371,38 @@ void ga_unset_frozen(GAState *s) return; } + /* if we delayed creation/opening of pid/log files due to being + * in a frozen state at start up, do it now + */ + if (s->deferred_options.log_filepath) { + s->log_file = fopen(s->deferred_options.log_filepath, "a"); + if (!s->log_file) { + s->log_file = stderr; + } + s->deferred_options.log_filepath = NULL; + } ga_enable_logging(s); - g_warning("logging re-enabled"); + g_warning("logging re-enabled due to filesystem unfreeze"); + if (s->deferred_options.pid_filepath) { + if (!ga_open_pidfile(s->deferred_options.pid_filepath)) { + g_warning("failed to create/open pid file"); + } + s->deferred_options.pid_filepath = NULL; + } /* enable all disabled, non-blacklisted commands */ ga_enable_non_blacklisted(s->blacklist); s->frozen = false; + if (!ga_delete_file(s->state_filepath_isfrozen)) { + g_warning("unable to delete %s, fsfreeze may not function properly", + s->state_filepath_isfrozen); + } } -#ifndef _WIN32 static void become_daemon(const char *pidfile) { +#ifndef _WIN32 pid_t pid, sid; - int pidfd; - char *pidstr = NULL; pid = fork(); if (pid < 0) { @@ -323,20 +412,11 @@ static void become_daemon(const char *pidfile) exit(EXIT_SUCCESS); } - pidfd = open(pidfile, O_CREAT|O_WRONLY|O_EXCL, S_IRUSR|S_IWUSR); - if (pidfd == -1) { - g_critical("Cannot create pid file, %s", strerror(errno)); - exit(EXIT_FAILURE); - } - - if (asprintf(&pidstr, "%d", getpid()) == -1) { - g_critical("Cannot allocate memory"); - goto fail; - } - if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr)) { - free(pidstr); - g_critical("Failed to write pid file"); - goto fail; + if (pidfile) { + if (!ga_open_pidfile(pidfile)) { + g_critical("failed to create pidfile"); + exit(EXIT_FAILURE); + } } umask(0); @@ -351,15 +431,14 @@ static void become_daemon(const char *pidfile) close(STDIN_FILENO); close(STDOUT_FILENO); close(STDERR_FILENO); - free(pidstr); return; fail: unlink(pidfile); g_critical("failed to daemonize"); exit(EXIT_FAILURE); -} #endif +} static int send_response(GAState *s, QObject *payload) { @@ -597,9 +676,11 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[]) int main(int argc, char **argv) { - const char *sopt = "hVvdm:p:l:f:b:s:"; - const char *method = NULL, *path = NULL, *pidfile = QGA_PIDFILE_DEFAULT; - const char *log_file_name = NULL; + const char *sopt = "hVvdm:p:l:f:b:s:t:"; + const char *method = NULL, *path = NULL; + const char *log_filepath = NULL; + const char *pid_filepath = QGA_PIDFILE_DEFAULT; + const char *state_dir = QGA_STATEDIR_DEFAULT; #ifdef _WIN32 const char *service = NULL; #endif @@ -616,11 +697,11 @@ int main(int argc, char **argv) #ifdef _WIN32 { "service", 1, NULL, 's' }, #endif + { "statedir", 1, NULL, 't' }, { NULL, 0, NULL, 0 } }; int opt_ind = 0, ch, daemonize = 0, i, j, len; GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; - FILE *log_file = stderr; GList *blacklist = NULL; GAState *s; @@ -635,17 +716,14 @@ int main(int argc, char **argv) path = optarg; break; case 'l': - log_file_name = optarg; - log_file = fopen(log_file_name, "a"); - if (!log_file) { - g_critical("unable to open specified log file: %s", - strerror(errno)); - return EXIT_FAILURE; - } + log_filepath = optarg; break; case 'f': - pidfile = optarg; + pid_filepath = optarg; break; + case 't': + state_dir = optarg; + break; case 'v': /* enable all log levels */ log_level = G_LOG_LEVEL_MASK; @@ -684,7 +762,7 @@ int main(int argc, char **argv) case 's': service = optarg; if (strcmp(service, "install") == 0) { - return ga_install_service(path, log_file_name); + return ga_install_service(path, log_filepath); } else if (strcmp(service, "uninstall") == 0) { return ga_uninstall_service(); } else { @@ -703,20 +781,70 @@ int main(int argc, char **argv) } } + s = g_malloc0(sizeof(GAState)); + s->log_level = log_level; + s->log_file = stderr; + 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", + state_dir); + s->frozen = false; #ifndef _WIN32 - if (daemonize) { - g_debug("starting daemon"); - become_daemon(pidfile); + /* 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 + * or reboot may have since unfrozen them), but better to require an + * uneeded unfreeze than to risk hanging on start-up + */ + struct stat st; + if (stat(s->state_filepath_isfrozen, &st) == -1) { + /* it's okay if the file doesn't exist, but if we can't access for + * some other reason, such as permissions, there's a configuration + * that needs to be addressed. so just bail now before we get into + * more trouble later + */ + if (errno != ENOENT) { + g_critical("unable to access state file at path %s: %s", + s->state_filepath_isfrozen, strerror(errno)); + return EXIT_FAILURE; + } + } else { + g_warning("previous instance appears to have exited with frozen" + " filesystems. deferring logging/pidfile creation and" + " disabling non-fsfreeze-safe commands until" + " guest-fsfreeze-thaw is issued, or filesystems are" + " manually unfrozen and the file %s is removed", + s->state_filepath_isfrozen); + s->frozen = true; } #endif - s = g_malloc0(sizeof(GAState)); - s->log_file = log_file; - s->log_level = log_level; - g_log_set_default_handler(ga_log, s); - g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR); - s->logging_enabled = true; - s->frozen = false; + if (ga_is_frozen(s)) { + if (daemonize) { + /* delay opening/locking of pidfile till filesystem are unfrozen */ + s->deferred_options.pid_filepath = pid_filepath; + become_daemon(NULL); + } + if (log_filepath) { + /* delay opening the log file till filesystems are unfrozen */ + s->deferred_options.log_filepath = log_filepath; + } + ga_disable_logging(s); + ga_disable_non_whitelisted(); + } else { + if (daemonize) { + become_daemon(pid_filepath); + } + if (log_filepath) { + s->log_file = fopen(log_filepath, "a"); + if (!s->log_file) { + g_critical("unable to open specified log file: %s", + strerror(errno)); + goto out_bad; + } + } + } + if (blacklist) { s->blacklist = blacklist; do { @@ -758,13 +886,13 @@ int main(int argc, char **argv) ga_channel_free(ga_state->channel); if (daemonize) { - unlink(pidfile); + unlink(pid_filepath); } return 0; out_bad: if (daemonize) { - unlink(pidfile); + unlink(pid_filepath); } return EXIT_FAILURE; }