From 9e8aded432884477bcd4fa1c7e849a196412bcc4 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Mon, 16 Apr 2012 19:52:17 -0500 Subject: 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 --- qga/commands-posix.c | 139 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 85 insertions(+), 54 deletions(-) (limited to 'qga') 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_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_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) { + 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) -- cgit v1.2.3 From f22d85e9e67262db34504f4079745f9843da6a92 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Tue, 17 Apr 2012 19:01:45 -0500 Subject: 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 --- qga/commands-posix.c | 35 +++++++++-------------------------- qga/guest-agent-core.h | 3 +++ 2 files changed, 12 insertions(+), 26 deletions(-) (limited to 'qga') 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); -- cgit v1.2.3