Re: [Qemu-devel] [PATCH v6 4/4] guest agent: add guest agent RPCs/commands

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue,  5 Jul 2011 08:21:40 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> This adds the initial set of QMP/QAPI commands provided by the guest
> agent:
> 
> guest-sync
> guest-ping
> guest-info
> guest-shutdown
> guest-file-open
> guest-file-read
> guest-file-write
> guest-file-seek
> guest-file-close
> guest-fsfreeze-freeze
> guest-fsfreeze-thaw
> guest-fsfreeze-status
> 
> The input/output specification for these commands are documented in the
> schema.
> 
> Example usage:
> 
>   host:
>     qemu -device virtio-serial \
>          -chardev socket,path=/tmp/vs0.sock,server,nowait,id=qga0 \
>          -device virtserialport,chardev=qga0,name=qga0
>          ...
> 
>     echo "{'execute':'guest-info'}" | socat stdio \
>          unix-connect:/tmp/qga0.sock
> 
>   guest:
>     qemu-ga -c virtio-serial -p /dev/virtio-ports/qga0 \
>             -p /var/run/qemu-guest-agent.pid -d
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  Makefile                   |   15 ++-
>  qemu-ga.c                  |    4 +
>  qerror.h                   |    3 +
>  qga/guest-agent-commands.c |  501 ++++++++++++++++++++++++++++++++++++++++++++
>  qga/guest-agent-core.h     |    2 +
>  5 files changed, 523 insertions(+), 2 deletions(-)
>  create mode 100644 qga/guest-agent-commands.c
> 
> diff --git a/Makefile b/Makefile
> index b2e8593..7e4f722 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -175,15 +175,26 @@ $(qapi-dir)/test-qmp-commands.h: $(qapi-dir)/test-qmp-marshal.c
>  $(qapi-dir)/test-qmp-marshal.c: $(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
>  	    $(call quiet-command,python $(SRC_PATH)/scripts/qapi-commands.py -o "$(qapi-dir)" -p "test-" < $<, "  GEN   $@")
>  
> +$(qapi-dir)/qga-qapi-types.c: $(qapi-dir)/qga-qapi-types.h
> +$(qapi-dir)/qga-qapi-types.h: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py
> +	$(call quiet-command,python $(SRC_PATH)/scripts/qapi-types.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
> +$(qapi-dir)/qga-qapi-visit.c: $(qapi-dir)/qga-qapi-visit.h
> +$(qapi-dir)/qga-qapi-visit.h: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py
> +	$(call quiet-command,python $(SRC_PATH)/scripts/qapi-visit.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
> +$(qapi-dir)/qga-qmp-marshal.c: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
> +	$(call quiet-command,python $(SRC_PATH)/scripts/qapi-commands.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
> +
>  test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h)
>  test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
>  
>  test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h test-qmp-marshal.c test-qmp-commands.h)
>  test-qmp-commands: test-qmp-commands.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o $(qapi-dir)/test-qmp-marshal.o module.o
>  
> -QGALIB=qga/guest-agent-command-state.o
> +QGALIB=qga/guest-agent-command-state.o qga/guest-agent-commands.o
> +
> +qemu-ga.o: $(qapi-dir)/qga-qapi-types.c $(qapi-dir)/qga-qapi-types.h $(qapi-dir)/qga-qapi-visit.c $(qapi-dir)/qga-qmp-marshal.c
>  
> -qemu-ga$(EXESUF): qemu-ga.o $(QGALIB) qemu-tool.o qemu-error.o error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) $(qapi-obj-y) qemu-timer-common.o qemu-sockets.o module.o qapi/qmp-dispatch.o qapi/qmp-registry.o
> +qemu-ga$(EXESUF): qemu-ga.o $(QGALIB) qemu-tool.o qemu-error.o error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) $(qapi-obj-y) qemu-timer-common.o qemu-sockets.o module.o qapi/qmp-dispatch.o qapi/qmp-registry.o $(qapi-dir)/qga-qapi-visit.o $(qapi-dir)/qga-qmp-marshal.o
>  
>  QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
>  
> diff --git a/qemu-ga.c b/qemu-ga.c
> index 649c16a..04ead22 100644
> --- a/qemu-ga.c
> +++ b/qemu-ga.c
> @@ -637,6 +637,9 @@ 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->command_state = ga_command_state_new();
> +    ga_command_state_init(s, s->command_state);
> +    ga_command_state_init_all(s->command_state);
>      ga_state = s;
>  
>      module_call_init(MODULE_INIT_QAPI);
> @@ -645,6 +648,7 @@ int main(int argc, char **argv)
>  
>      g_main_loop_run(ga_state->main_loop);
>  
> +    ga_command_state_cleanup_all(ga_state->command_state);
>      unlink(pidfile);
>  
>      return 0;
> diff --git a/qerror.h b/qerror.h
> index 9a9fa5b..0f618ac 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -184,4 +184,7 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_FEATURE_DISABLED \
>      "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
>  
> +#define QERR_QGA_LOGGING_FAILED \
> +    "{ 'class': 'QgaLoggingFailed', 'data': {} }"
> +
>  #endif /* QERROR_H */
> diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
> new file mode 100644
> index 0000000..42390fb
> --- /dev/null
> +++ b/qga/guest-agent-commands.c
> @@ -0,0 +1,501 @@
> +/*
> + * QEMU Guest Agent commands
> + *
> + * Copyright IBM Corp. 2011
> + *
> + * Authors:
> + *  Michael Roth      <mdroth@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include <glib.h>
> +#include <mntent.h>
> +#include <sys/types.h>
> +#include <sys/ioctl.h>
> +#include <linux/fs.h>
> +#include "qga/guest-agent-core.h"
> +#include "qga-qmp-commands.h"
> +#include "qerror.h"
> +#include "qemu-queue.h"
> +
> +static GAState *ga_state;
> +
> +static void disable_logging(void)
> +{
> +    ga_disable_logging(ga_state);
> +}
> +
> +static void enable_logging(void)
> +{
> +    ga_enable_logging(ga_state);
> +}
> +
> +/* Note: in some situations, like with the fsfreeze, logging may be
> + * temporarilly disabled. if it is necessary that a command be able
> + * to log for accounting purposes, check ga_logging_enabled() beforehand,
> + * and use the QERR_QGA_LOGGING_DISABLED to generate an error
> + */
> +static void slog(const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    va_start(ap, fmt);
> +    g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
> +    va_end(ap);
> +}
> +
> +int64_t qmp_guest_sync(int64_t id, Error **errp)
> +{
> +    return id;
> +}
> +
> +void qmp_guest_ping(Error **err)
> +{
> +    slog("guest-ping called");
> +}
> +
> +struct GuestAgentInfo *qmp_guest_info(Error **err)
> +{
> +    GuestAgentInfo *info = qemu_mallocz(sizeof(GuestAgentInfo));
> +
> +    info->version = g_strdup(QGA_VERSION);
> +
> +    return info;
> +}
> +
> +void qmp_guest_shutdown(const char *mode, Error **err)
> +{
> +    int ret;
> +    const char *shutdown_flag;
> +
> +    slog("guest-shutdown called, mode: %s", mode);
> +    if (strcmp(mode, "halt") == 0) {
> +        shutdown_flag = "-H";
> +    } else if (strcmp(mode, "powerdown") == 0) {
> +        shutdown_flag = "-P";
> +    } else if (strcmp(mode, "reboot") == 0) {
> +        shutdown_flag = "-r";
> +    } else {
> +        error_set(err, QERR_INVALID_PARAMETER_VALUE, "mode",
> +                  "halt|powerdown|reboot");
> +        return;
> +    }
> +
> +    ret = fork();
> +    if (ret == 0) {
> +        /* child, start the shutdown */
> +        setsid();
> +        fclose(stdin);
> +        fclose(stdout);
> +        fclose(stderr);
> +
> +        sleep(5);

If we're required to return a response before the shutdown happens, this
is a bug and I'm afraid that the right way to this is a bit complex.

Otherwise we can just leave it out.

> +        ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
> +                    "hypervisor initiated shutdown", (char*)NULL);
> +        if (ret) {
> +            slog("guest-shutdown failed: %s", strerror(errno));
> +        }
> +        exit(!!ret);
> +    } else if (ret < 0) {
> +        error_set(err, QERR_UNDEFINED_ERROR);
> +    }
> +}
> +
> +typedef struct GuestFileHandle {
> +    uint64_t id;
> +    FILE *fh;
> +    QTAILQ_ENTRY(GuestFileHandle) next;
> +} GuestFileHandle;
> +
> +static struct {
> +    QTAILQ_HEAD(, GuestFileHandle) filehandles;
> +} guest_file_state;
> +
> +static void guest_file_handle_add(FILE *fh)
> +{
> +    GuestFileHandle *gfh;
> +
> +    gfh = qemu_mallocz(sizeof(GuestFileHandle));
> +    gfh->id = fileno(fh);
> +    gfh->fh = fh;
> +    QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next);
> +}
> +
> +static GuestFileHandle *guest_file_handle_find(int64_t id)
> +{
> +    GuestFileHandle *gfh;
> +
> +    QTAILQ_FOREACH(gfh, &guest_file_state.filehandles, next)
> +    {
> +        if (gfh->id == id) {
> +            return gfh;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +int64_t qmp_guest_file_open(const char *filepath, bool has_mode, const char *mode, Error **err)
> +{
> +    FILE *fh;
> +    int fd;
> +    int64_t ret = -1;
> +
> +    if (!has_mode) {
> +        mode = "r";
> +    }
> +    slog("guest-file-open called, filepath: %s, mode: %s", filepath, mode);
> +    fh = fopen(filepath, mode);
> +    if (!fh) {
> +        error_set(err, QERR_OPEN_FILE_FAILED, filepath);
> +        return -1;
> +    }
> +
> +    /* set fd non-blocking to avoid common use cases (like reading from a
> +     * named pipe) from hanging the agent
> +     */
> +    fd = fileno(fh);
> +    ret = fcntl(fd, F_GETFL);
> +    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
> +    if (ret == -1) {
> +        error_set(err, QERR_OPEN_FILE_FAILED, filepath);
> +        fclose(fh);
> +        return -1;
> +    }
> +
> +    guest_file_handle_add(fh);
> +    slog("guest-file-open, filehandle: %d", fd);
> +    return fd;
> +}
> +
> +struct GuestFileRead *qmp_guest_file_read(int64_t filehandle, int64_t count,
> +                                          Error **err)
> +{
> +    GuestFileHandle *gfh = guest_file_handle_find(filehandle);
> +    GuestFileRead *read_data;
> +    guchar *buf;
> +    FILE *fh;
> +    size_t read_count;
> +
> +    if (!gfh) {
> +        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> +        return NULL;
> +    }
> +
> +    if (count < 0 || count > QGA_READ_LIMIT) {
> +        error_set(err, QERR_INVALID_PARAMETER, "count");
> +        return NULL;
> +    }

Are we imposing that limit because of the malloc() call below? If that's
the case I think it's wrong, because we don't know the VM (neither the guest)
better than the client.

The best thing we can do here is to limit it to the file size. Additionally
to this we could have a command-line option to allow the sysadmin set his/her
own limit.

> +
> +    fh = gfh->fh;
> +    read_data = qemu_mallocz(sizeof(GuestFileRead) + 1);
> +    buf = qemu_mallocz(count+1);
> +    if (!buf) {
> +        error_set(err, QERR_UNDEFINED_ERROR);
> +        return NULL;
> +    }

qemu_malloc() functions never fail...

> +
> +    read_count = fread(buf, 1, count, fh);

Isn't 'nmemb' and 'size' swapped?

> +    buf[read_count] = 0;
> +    read_data->count = read_count;
> +    read_data->eof = feof(fh);
> +    if (read_count) {
> +        read_data->buf = g_base64_encode(buf, read_count);
> +    }
> +    qemu_free(buf);
> +    /* clear error and eof. error is generally due to EAGAIN from non-blocking
> +     * mode, and no real way to differenitate from a real error since we only
> +     * get boolean error flag from ferror()
> +     */
> +    clearerr(fh);
> +
> +    return read_data;
> +}
> +
> +GuestFileWrite *qmp_guest_file_write(int64_t filehandle, const char *data_b64,
> +                                     int64_t count, Error **err)
> +{
> +    GuestFileWrite *write_data;
> +    guchar *data;
> +    gsize data_len;
> +    int write_count;
> +    GuestFileHandle *gfh = guest_file_handle_find(filehandle);
> +    FILE *fh;
> +
> +    if (!gfh) {
> +        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> +        return NULL;
> +    }
> +
> +    fh = gfh->fh;
> +    data = g_base64_decode(data_b64, &data_len);
> +    if (count > data_len) {
> +        qemu_free(data);
> +        error_set(err, QERR_INVALID_PARAMETER, "count");
> +        return NULL;
> +    }
> +    write_data = qemu_mallocz(sizeof(GuestFileWrite));
> +    write_count = fwrite(data, 1, count, fh);
> +    write_data->count = write_count;
> +    write_data->eof = feof(fh);
> +    qemu_free(data);
> +    clearerr(fh);

Shouldn't we check for errors instead of doing this?

Btw, I think it's a good idea to offer guest-file-flush() too (or do a flush()
here, but that's probably bad).

> +
> +    return write_data;
> +}
> +
> +struct GuestFileSeek *qmp_guest_file_seek(int64_t filehandle, int64_t offset,
> +                                          int64_t whence, Error **err)
> +{
> +    GuestFileSeek *seek_data;
> +    GuestFileHandle *gfh = guest_file_handle_find(filehandle);
> +    FILE *fh;
> +    int ret;
> +
> +    if (!gfh) {
> +        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> +        return NULL;
> +    }
> +
> +    fh = gfh->fh;
> +    seek_data = qemu_mallocz(sizeof(GuestFileRead));
> +    ret = fseek(fh, offset, whence);
> +    if (ret == -1) {
> +        error_set(err, QERR_UNDEFINED_ERROR);
> +        qemu_free(seek_data);
> +        return NULL;
> +    }
> +    seek_data->position = ftell(fh);
> +    seek_data->eof = feof(fh);
> +    clearerr(fh);

Again, I don't see why we should do this.

> +
> +    return seek_data;
> +}
> +
> +void qmp_guest_file_close(int64_t filehandle, Error **err)
> +{
> +    GuestFileHandle *gfh = guest_file_handle_find(filehandle);
> +
> +    slog("guest-file-close called, filehandle: %ld", filehandle);
> +    if (!gfh) {
> +        error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> +        return;
> +    }
> +
> +    fclose(gfh->fh);
> +    QTAILQ_REMOVE(&guest_file_state.filehandles, gfh, next);
> +    qemu_free(gfh);
> +}
> +
> +static void guest_file_init(void)
> +{
> +    QTAILQ_INIT(&guest_file_state.filehandles);
> +}
> +
> +typedef struct GuestFsfreezeMount {
> +    char *dirname;
> +    char *devtype;
> +    QTAILQ_ENTRY(GuestFsfreezeMount) next;
> +} GuestFsfreezeMount;
> +
> +struct {
> +    GuestFsfreezeStatus status;
> +    QTAILQ_HEAD(, GuestFsfreezeMount) mount_list;
> +} guest_fsfreeze_state;
> +
> +/*
> + * Walk the mount table and build a list of local file systems
> + */
> +static int guest_fsfreeze_build_mount_list(void)
> +{
> +    struct mntent *ment;
> +    GuestFsfreezeMount *mount, *temp;
> +    char const *mtab = MOUNTED;
> +    FILE *fp;
> +
> +    fp = setmntent(mtab, "r");
> +    if (!fp) {
> +        g_warning("fsfreeze: unable to read mtab");
> +        goto fail;
> +    }
> +
> +    while ((ment = 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 ((ment->mnt_fsname[0] != '/') ||
> +            (strcmp(ment->mnt_type, "smbfs") == 0) ||
> +            (strcmp(ment->mnt_type, "cifs") == 0)) {
> +            continue;
> +        }
> +
> +        mount = qemu_mallocz(sizeof(GuestFsfreezeMount));
> +        mount->dirname = qemu_strdup(ment->mnt_dir);
> +        mount->devtype = qemu_strdup(ment->mnt_type);
> +
> +        QTAILQ_INSERT_TAIL(&guest_fsfreeze_state.mount_list, mount, next);
> +    }
> +
> +    endmntent(fp);
> +
> +    return 0;
> +
> +fail:
> +    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
> +        QTAILQ_REMOVE(&guest_fsfreeze_state.mount_list, mount, next);
> +        qemu_free(mount->dirname);
> +        qemu_free(mount->devtype);
> +        qemu_free(mount);
> +    }

This doesn't seem to be used.

> +
> +    return -1;
> +}
> +
> +/*
> + * Return status of freeze/thaw
> + */
> +GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
> +{
> +    return guest_fsfreeze_state.status;
> +}
> +
> +/*
> + * Walk list of mounted file systems in the guest, and freeze the ones which
> + * are real local file systems.
> + */
> +int64_t qmp_guest_fsfreeze_freeze(Error **err)
> +{
> +    int ret = 0, i = 0;
> +    struct GuestFsfreezeMount *mount, *temp;
> +    int fd;
> +
> +    slog("guest-fsfreeze called");
> +
> +    if (guest_fsfreeze_state.status != GUEST_FSFREEZE_STATUS_THAWED) {

return 0;

> +        ret = 0;
> +        goto out;
> +    }
> +
> +    ret = guest_fsfreeze_build_mount_list();
> +    if (ret < 0) {

return ret;

> +        goto out;
> +    }
> +
> +    guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_INPROGRESS;
> +
> +    /* 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) {
> +        fd = qemu_open(mount->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = errno;
> +            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 reason 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
> +         */
> +        ret = ioctl(fd, FIFREEZE);
> +        if (ret < 0 && errno != EOPNOTSUPP) {
> +            close(fd);
> +            goto error;
> +        }
> +        close(fd);
> +
> +        i++;
> +    }
> +
> +    guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_FROZEN;
> +    ret = i;
> +out:
> +    return ret;
> +error:
> +    if (i > 0) {
> +        guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_ERROR;
> +    }

Shouldn't you undo everything that has been done so far? Which is
freeing the build list and thawing the file-systems that were frozen
before the error?

> +    goto out;
> +}
> +
> +/*
> + * Walk list of frozen file systems in the guest, and thaw them.
> + */
> +int64_t qmp_guest_fsfreeze_thaw(Error **err)
> +{
> +    int ret;
> +    GuestFsfreezeMount *mount, *temp;
> +    int fd, i = 0;
> +
> +    if (guest_fsfreeze_state.status != GUEST_FSFREEZE_STATUS_FROZEN &&
> +        guest_fsfreeze_state.status != GUEST_FSFREEZE_STATUS_INPROGRESS) {

I don't follow why we're checking against INPROGRESS here.

> +        ret = 0;
> +        goto out_enable_logging;
> +    }
> +
> +    QTAILQ_FOREACH_SAFE(mount, &guest_fsfreeze_state.mount_list, next, temp) {
> +        fd = qemu_open(mount->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = -errno;
> +            goto out;
> +        }
> +        ret = ioctl(fd, FITHAW);
> +        if (ret < 0 && errno != EOPNOTSUPP && errno != EINVAL) {
> +            ret = -errno;
> +            close(fd);
> +            goto out;

Shouldn't you continue and try to thaw the other file-systems in the list?

> +        }
> +        close(fd);
> +
> +        QTAILQ_REMOVE(&guest_fsfreeze_state.mount_list, mount, next);
> +        qemu_free(mount->dirname);
> +        qemu_free(mount->devtype);
> +        qemu_free(mount);
> +        i++;
> +    }
> +
> +    guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED;
> +    ret = i;
> +out_enable_logging:
> +    enable_logging();
> +out:
> +    return ret;
> +}
> +
> +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)
> +{
> +    int64_t ret;
> +    Error *err = NULL;
> +
> +    if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) {
> +        ret = qmp_guest_fsfreeze_thaw(&err);
> +        if (ret < 0 || err) {
> +            slog("failed to clean up frozen filesystems");
> +        }
> +    }
> +}
> +
> +/* register init/cleanup routines for stateful command groups */
> +void ga_command_state_init(GAState *s, GACommandState *cs)
> +{
> +    ga_state = s;
> +    ga_command_state_add(cs, guest_fsfreeze_init, guest_fsfreeze_cleanup);
> +    ga_command_state_add(cs, guest_file_init, NULL);
> +}
> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> index 66d1729..3501ff4 100644
> --- a/qga/guest-agent-core.h
> +++ b/qga/guest-agent-core.h
> @@ -14,10 +14,12 @@
>  #include "qemu-common.h"
>  
>  #define QGA_VERSION "1.0"
> +#define QGA_READ_LIMIT 4 << 20 /* 4MB block size max for chunked reads */
>  
>  typedef struct GAState GAState;
>  typedef struct GACommandState GACommandState;
>  
> +void ga_command_state_init(GAState *s, GACommandState *cs);
>  void ga_command_state_add(GACommandState *cs,
>                            void (*init)(void),
>                            void (*cleanup)(void));