Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw

[Stefan Hajnoczi]

On Tue, Feb 1, 2011 at 10:58 AM,  <address@hidden> wrote:
> From: Jes Sorensen <address@hidden>
>
> Implement freeze/thaw support in the guest, allowing the host to
> request the guest freezes all it's file systems before a live snapshot
> is performed.
>  - fsfreeze(): Walk the list of mounted local real file systems,
>               and freeze them.
>  - fsthaw():   Walk the list of previously frozen file systems and
>               thaw them.
>  - fsstatus(): Return the current status of freeze/thaw. The host must
>               poll this function, in case fsfreeze() returned with a
>               timeout, to wait for the operation to finish.

It is desirable to minimize the freeze time, which may interrupt or
degrade the service that applications inside the VM can provide.
Polling means we have to choose a fixed value (500 ms?) at which to
check for freeze completion.  In this example we could have up to 500
ms extra time spent in freeze because it completed right after we
polled.  Any thoughts on this?

In terms of the fsfreeze(), fsthaw(), fsstatus() API, are you looking
at Windows Volume Shadow Copy Services and does this API fit that
model (I haven't looked at it in detail yet)?
http://msdn.microsoft.com/en-us/library/bb968832(v=vs.85).aspx

> Signed-off-by: Jes Sorensen <address@hidden>
> ---
>  virtagent-common.h |    8 ++
>  virtagent-server.c |  196 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 204 insertions(+), 0 deletions(-)
>
> diff --git a/virtagent-common.h b/virtagent-common.h
> index 5d8f5c1..220a4b6 100644
> --- a/virtagent-common.h
> +++ b/virtagent-common.h
> @@ -61,6 +61,14 @@ typedef struct VAContext {
>     const char *channel_path;
>  } VAContext;
>
> +enum vs_fsfreeze_status {
> +    FREEZE_ERROR = -1,
> +    FREEZE_THAWED = 0,
> +    FREEZE_INPROGRESS = 1,
> +    FREEZE_FROZEN = 2,
> +    FREEZE_THAWINPROGRESS = 3,
> +};
> +
>  enum va_job_status {
>     VA_JOB_STATUS_PENDING = 0,
>     VA_JOB_STATUS_OK,
> diff --git a/virtagent-server.c b/virtagent-server.c
> index 7bb35b2..cf2a3f0 100644
> --- a/virtagent-server.c
> +++ b/virtagent-server.c
> @@ -14,6 +14,13 @@
>  #include <syslog.h>
>  #include "qemu_socket.h"
>  #include "virtagent-common.h"
> +#include <mntent.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/errno.h>
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +#include <linux/fs.h>
>
>  static VAServerData *va_server_data;
>  static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */
> @@ -217,6 +224,189 @@ static xmlrpc_value *va_hello(xmlrpc_env *env,
>     return result;
>  }
>
> +
> +/*
> + * Walk the mount table and build a list of local file systems
> + */
> +
> +struct direntry {
> +    char *dirname;
> +    char *devtype;
> +    struct direntry *next;
> +};
> +
> +static struct direntry *mount_list;
> +static int fsfreeze_status;
> +
> +static int build_mount_list(void)
> +{
> +    struct mntent *mnt;
> +    struct direntry *entry;
> +    struct direntry *next;
> +    char const *mtab = MOUNTED;
> +    FILE *fp;
> +
> +    fp = setmntent(mtab, "r");
> +    if (!fp) {
> +       fprintf(stderr, "unable to read mtab\n");
> +       goto fail;
> +    }
> +
> +    while ((mnt = getmntent(fp))) {
> +       /*
> +        * An entry which device name doesn't start with a '/' is
> +        * either a dummy file system or a network file system.
> +        * Add special handling for smbfs and cifs as is done by
> +        * coreutils as well.
> +        */
> +       if ((mnt->mnt_fsname[0] != '/') ||
> +           (strcmp(mnt->mnt_type, "smbfs") == 0) ||
> +           (strcmp(mnt->mnt_type, "cifs") == 0)) {
> +           continue;
> +       }
> +
> +       entry = qemu_malloc(sizeof(struct direntry));
> +       if (!entry) {
> +           goto fail;
> +       }

qemu_malloc() never fails.

> +       entry->dirname = qemu_strdup(mnt->mnt_dir);
> +       entry->devtype = qemu_strdup(mnt->mnt_type);
> +       entry->next = mount_list;
> +
> +       mount_list = entry;
> +    }
> +
> +    endmntent(fp);
> +
> +    return 0;
> +
> +fail:
> +    while(mount_list) {
> +       next = mount_list->next;
> +       qemu_free(mount_list->dirname);
> +       qemu_free(mount_list->devtype);
> +       qemu_free(mount_list);
> +       mount_list = next;
> +    }
> +
> +    return -1;
> +}
> +
> +/*
> + * va_fsfreeze(): Walk list of mounted file systems in the guest, and
> + *   freeze the ones which are real local file systems.
> + * rpc return values: Number of file systems frozen, -1 on error.
> + */
> +static xmlrpc_value *va_fsfreeze(xmlrpc_env *env,
> +                                 xmlrpc_value *params,
> +                                 void *user_data)
> +{
> +    xmlrpc_int32 ret = 0, i = 0;
> +    xmlrpc_value *result;
> +    struct direntry *entry;
> +    int fd;
> +    SLOG("va_fsfreeze()");
> +
> +    if (fsfreeze_status == FREEZE_FROZEN) {
> +        ret = 0;
> +        goto out;
> +    }

The only valid status here is FREEZE_THAWED?  Perhaps we should test
for that specifically.

> +
> +    ret = build_mount_list();
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    fsfreeze_status = FREEZE_INPROGRESS;
> +
> +    entry = mount_list;
> +    while(entry) {
> +        fd = qemu_open(entry->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = errno;
> +            goto error;
> +        }
> +        ret = ioctl(fd, FIFREEZE);

If you close(fd) here then it won't leak or need extra code in the error path.

> +        if (ret < 0 && ret != EOPNOTSUPP) {
> +            goto error;
> +        }
> +
> +        close(fd);
> +        entry = entry->next;
> +        i++;
> +    }
> +
> +    fsfreeze_status = FREEZE_FROZEN;
> +    ret = i;
> +out:
> +    result = xmlrpc_build_value(env, "i", ret);
> +    return result;
> +error:
> +    if (i > 0) {
> +        fsfreeze_status = FREEZE_ERROR;
> +    }
> +    goto out;
> +}
> +
> +/*
> + * va_fsthaw(): Walk list of frozen file systems in the guest, and
> + *   thaw them.
> + * rpc return values: Number of file systems thawed on success, -1 on error.
> + */
> +static xmlrpc_value *va_fsthaw(xmlrpc_env *env,
> +                               xmlrpc_value *params,
> +                               void *user_data)
> +{
> +    xmlrpc_int32 ret;
> +    xmlrpc_value *result;
> +    struct direntry *entry;
> +    int fd, i = 0;
> +    SLOG("va_fsthaw()");
> +
> +    if (fsfreeze_status == FREEZE_THAWED) {
> +        ret = 0;
> +        goto out;
> +    }

A stricter check would be status FREEZE_FROZEN.

> +
> +    while((entry = mount_list)) {
> +        fd = qemu_open(entry->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = -1;
> +            goto out;
> +        }
> +        ret = ioctl(fd, FITHAW);

Same thing about close(fd) here.

Stefan