From patchwork Wed Apr 18 22:04:09 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Roth X-Patchwork-Id: 153613 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id CE8A6B6EF1 for ; Thu, 19 Apr 2012 08:04:43 +1000 (EST) Received: from localhost ([::1]:46216 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SKczR-0008Mk-8o for incoming@patchwork.ozlabs.org; Wed, 18 Apr 2012 18:04:41 -0400 Received: from eggs.gnu.org ([208.118.235.92]:47051) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SKczD-0008Lr-9n for qemu-devel@nongnu.org; Wed, 18 Apr 2012 18:04:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SKcz8-00051l-Kx for qemu-devel@nongnu.org; Wed, 18 Apr 2012 18:04:26 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:61336) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SKcz8-00051U-Bc for qemu-devel@nongnu.org; Wed, 18 Apr 2012 18:04:22 -0400 Received: by obbwd18 with SMTP id wd18so5041375obb.4 for ; Wed, 18 Apr 2012 15:04:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer; bh=KnFaPU9FhxPewKS2StBMqFp8fgmjDj7af83py6Sb/ZQ=; b=wxRbmovJc2c1zkyE1RD5u5ZYJeb2IItXrHzpLkOEjx0Al89yTGkGqOt5zF909wzNes MqoKw8pQ6SEuBiHNzSccKUfjV4fpb68b0ttRq3s/5R42T4OHwUCgUfYawkpkBT/DfkPF mJKJdFfojxp8UV2lTa+LGfm3t6TAdjm0B9cYpolx6KTo2fI1kXjP9uQ4pBTkxJKyE5kx cS0GgbWPQV3lpWeFz05hK6P6VyO+4sL3Ul5HdlYmKDoOfhSTQ+PqPw/77yL+keuaQkRK IlGYyu5qd9f0za2x0GbN2ISnU4x4Db/ooj0RXsFEvMh215tUa1wdsuBrKcqGmQNI0wdV 7bkw== Received: by 10.182.152.72 with SMTP id uw8mr5264522obb.73.1334786659879; Wed, 18 Apr 2012 15:04:19 -0700 (PDT) Received: from localhost.localdomain ([32.97.110.59]) by mx.google.com with ESMTPS id d9sm286497obz.6.2012.04.18.15.04.17 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 18 Apr 2012 15:04:19 -0700 (PDT) From: Michael Roth To: qemu-devel@nongnu.org Date: Wed, 18 Apr 2012 17:04:09 -0500 Message-Id: <1334786651-30519-1-git-send-email-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 1.7.4.1 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.214.173 Cc: mprivozn@redhat.com, jcody@redhat.com Subject: [Qemu-devel] [PATCH 1/3] qemu-ga: improve recovery options for fsfreeze X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 | 27 ++++++--- qga/commands-posix.c | 143 ++++++++++++++++++++++++++++++------------------ 2 files changed, 106 insertions(+), 64 deletions(-) diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index cf18876..dc25be0 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,12 @@ # # 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 fsfreeze. To be safe, rather than relying on status reported here, users +# should always issue freeze/unfreeze in pairs to allow a guest to return to +# a fully-functional state: an extra unfreeze will simply result in a no-op. +# # Since: 0.15.0 ## { 'command': 'guest-fsfreeze-status', @@ -320,9 +322,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 +335,14 @@ ## # @guest-fsfreeze-thaw: # -# Unfreeze frozen guest fileystems +# Unfreeze all frozen guest filesystems +# +# Returns: Number of file systems thawed by this call # -# Returns: Number of file systems thawed -# If error, -1 (unknown error) or -errno +# Note: if return value does not match the previous call to +# guest-fsfreeze-freeze, all filesystems should still be +# thawed, it just means some freezable filesystems were +# unfrozen before this call. # # Since: 0.15.0 ## diff --git a/qga/commands-posix.c b/qga/commands-posix.c index faf970d..bd2e6e1 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,50 @@ 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. note that this will also + * occur when filesystems are encountered that were already + * frozen, and since we can't reliably distinguish these particular + * instances, you must call guest-fsfreeze-thaw first, or wait + * till the filesystem in thawed, before this command will succeed. */ 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 +479,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)