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

[Michael Roth]

On 02/02/2011 02:42 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.
>
> Signed-off-by: Jes Sorensen<address@hidden>
> ---
>   virtagent-common.h |    8 ++
>   virtagent-server.c |  190 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 198 insertions(+), 0 deletions(-)
>
> diff --git a/virtagent-common.h b/virtagent-common.h
> index 5d8f5c1..7c6d9ef 100644
> --- a/virtagent-common.h
> +++ b/virtagent-common.h
> @@ -61,6 +61,14 @@ typedef struct VAContext {
>       const char *channel_path;
>   } VAContext;
>
> +enum va_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..ffbe163 100644
> --- a/virtagent-server.c
> +++ b/virtagent-server.c
> @@ -14,6 +14,10 @@
>   #include<syslog.h>
>   #include "qemu_socket.h"
>   #include "virtagent-common.h"
> +#include<mntent.h>
> +#include<sys/types.h>
> +#include<sys/ioctl.h>
> +#include<linux/fs.h>
>
>   static VAServerData *va_server_data;
>   static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */
> @@ -217,6 +221,186 @@ 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 *va_mount_list;
> +static int va_fsfreeze_status;

And what I meant in the last RFC about using "objects" was to 
encapsulate global state information for a particular group of commands 
in single data type/variable. We're gonna end up with a similar set of 
variables for stateful RPCs like copyfile and potentially a few for 
things like spice. So to avoid having things get too cluttered up I'd 
prefer something like, in this particular case:

typedef struct VAFSFreezeState {
    struct direntry *mount_list;
    int status;
} VAFSFeezeState;

static VAFSFreezeState va_fsfreeze_state;

> +
> +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;
> +    }

You have tabs instead of spaces here

> +
> +    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));
> +       entry->dirname = qemu_strdup(mnt->mnt_dir);
> +       entry->devtype = qemu_strdup(mnt->mnt_type);
> +       entry->next = va_mount_list;
> +
> +       va_mount_list = entry;

Here too

> +    }
> +
> +    endmntent(fp);
> +
> +    return 0;
> +
> +fail:
> +    while(va_mount_list) {
> +       next = va_mount_list->next;

Here too. Might be some other spots but you get the point :)

> +        qemu_free(va_mount_list->dirname);
> +        qemu_free(va_mount_list->devtype);
> +        qemu_free(va_mount_list);
> +        va_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 (va_fsfreeze_status != FREEZE_THAWED) {
> +        ret = 0;
> +        goto out;
> +    }
> +
> +    ret = build_mount_list();
> +    if (ret<  0) {
> +        goto out;
> +    }
> +
> +    va_fsfreeze_status = FREEZE_INPROGRESS;
> +
> +    entry = va_mount_list;
> +    while(entry) {
> +        fd = qemu_open(entry->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = errno;
> +            goto error;
> +        }
> +        ret = ioctl(fd, FIFREEZE);
> +        close(fd);
> +        if (ret<  0&&  ret != EOPNOTSUPP) {
> +            goto error;
> +        }
> +
> +        entry = entry->next;
> +        i++;
> +    }
> +
> +    va_fsfreeze_status = FREEZE_FROZEN;
> +    ret = i;
> +out:
> +    result = xmlrpc_build_value(env, "i", ret);
> +    return result;
> +error:
> +    if (i>  0) {
> +        va_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 (va_fsfreeze_status != FREEZE_FROZEN) {
> +        ret = 0;
> +        goto out;
> +    }
> +
> +    while((entry = va_mount_list)) {
> +        fd = qemu_open(entry->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = -1;
> +            goto out;
> +        }
> +        ret = ioctl(fd, FITHAW);
> +        close(fd);
> +        if (ret<  0&&  ret != EOPNOTSUPP) {
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        va_mount_list = entry->next;
> +        qemu_free(entry->dirname);
> +        qemu_free(entry->devtype);
> +        qemu_free(entry);
> +        i++;
> +    }
> +
> +    va_fsfreeze_status = FREEZE_THAWED;
> +    ret = i;
> +out:
> +    result = xmlrpc_build_value(env, "i", ret);
> +    return result;
> +}
> +
> +/* va_fsstatus(): Return status of freeze/thaw
> + * rpc return values: fsfreeze_status
> + */
> +static xmlrpc_value *va_fsstatus(xmlrpc_env *env,
> +                                 xmlrpc_value *params,
> +                                 void *user_data)
> +{
> +    xmlrpc_value *result = xmlrpc_build_value(env, "i", va_fsfreeze_status);
> +    SLOG("va_fsstatus()");
> +    return result;
> +}
> +
>   typedef struct RPCFunction {
>       xmlrpc_value *(*func)(xmlrpc_env *env, xmlrpc_value *param, void *unused);
>       const char *func_name;
> @@ -237,6 +421,12 @@ static RPCFunction guest_functions[] = {
>         .func_name = "va.ping" },
>       { .func = va_capabilities,
>         .func_name = "va.capabilities" },
> +    { .func = va_fsfreeze,
> +      .func_name = "va.fsfreeze" },
> +    { .func = va_fsthaw,
> +      .func_name = "va.fsthaw" },
> +    { .func = va_fsstatus,
> +      .func_name = "va.fsstatus" },
>       { NULL, NULL }
>   };
>   static RPCFunction host_functions[] = {