On Fri, 17 Jun 2011 23:38:16 -0300
Luiz Capitulino <lcapitul...@redhat.com> wrote:
> On Fri, 17 Jun 2011 15:19:56 -0500
> Michael Roth <mdr...@linux.vnet.ibm.com> wrote:
>
> > On 06/16/2011 01:52 PM, Luiz Capitulino wrote:
> > > On Tue, 14 Jun 2011 15:06:23 -0500
> > > Michael Roth<mdr...@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.
> > >>
> > >> Signed-off-by: Michael Roth<mdr...@linux.vnet.ibm.com>
> > >> ---
> > >> qerror.c | 4 +
> > >> qerror.h | 3 +
> > >> qga/guest-agent-commands.c | 522
> > >> ++++++++++++++++++++++++++++++++++++++++++++
> > >> qga/guest-agent-core.h | 1 +
> > >> 4 files changed, 530 insertions(+), 0 deletions(-)
> > >> create mode 100644 qga/guest-agent-commands.c
> > >>
> > >> diff --git a/qerror.c b/qerror.c
> > >> index d7fcd93..24f0c48 100644
> > >> --- a/qerror.c
> > >> +++ b/qerror.c
> > >> @@ -213,6 +213,10 @@ static const QErrorStringTable qerror_table[] = {
> > >> .error_fmt = QERR_VNC_SERVER_FAILED,
> > >> .desc = "Could not start VNC server on %(target)",
> > >> },
> > >> + {
> > >> + .error_fmt = QERR_QGA_LOGGING_FAILED,
> > >> + .desc = "failed to write log statement due to logging
> > >> being disabled",
> > >> + },
> > >> {}
> > >> };
> > >>
> > >> diff --git a/qerror.h b/qerror.h
> > >> index 7a89a50..bf3d5a9 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..6f9886a
> > >> --- /dev/null
> > >> +++ b/qga/guest-agent-commands.c
> > >> @@ -0,0 +1,522 @@
> > >> +/*
> > >> + * QEMU Guest Agent commands
> > >> + *
> > >> + * Copyright IBM Corp. 2011
> > >> + *
> > >> + * Authors:
> > >> + * Michael Roth<mdr...@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"
> > >> +
> > >> +static GAState *ga_state;
> > >> +
> > >> +static bool logging_enabled(void)
> > >> +{
> > >> + return ga_logging_enabled(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 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 = g_malloc0(sizeof(GuestAgentInfo));
> > >> +
> > >> + info->version = g_strdup(QGA_VERSION);
> > >> +
> > >> + return info;
> > >> +}
> > >> +
> > >> +void qmp_guest_shutdown(const char *shutdown_mode, Error **err)
> > >
> > > I'd call the argument just 'mode'.
> > >
> > >> +{
> > >> + int ret;
> > >> + const char *shutdown_flag;
> > >> +
> > >> + if (!logging_enabled()) {
> > >> + error_set(err, QERR_QGA_LOGGING_FAILED);
> > >> + return;
> > >> + }
> > >> +
> > >> + slog("guest-shutdown called, shutdown_mode: %s", shutdown_mode);
> > >> + if (strcmp(shutdown_mode, "halt") == 0) {
> > >> + shutdown_flag = "-H";
> > >> + } else if (strcmp(shutdown_mode, "powerdown") == 0) {
> > >> + shutdown_flag = "-P";
> > >> + } else if (strcmp(shutdown_mode, "reboot") == 0) {
> > >> + shutdown_flag = "-r";
> > >> + } else {
> > >> + error_set(err, QERR_INVALID_PARAMETER_VALUE, "shutdown_mode",
> > >> + "halt|powerdown|reboot");
> > >> + return;
> > >> + }
> > >> +
> > >> + ret = fork();
> > >> + if (ret == 0) {
> > >> + /* child, start the shutdown */
> > >> + setsid();
> > >> + fclose(stdin);
> > >> + fclose(stdout);
> > >> + fclose(stderr);
> > >
> > > Would be nice to have a log file, whose descriptor is passed to the
> > > child.
> > >
> > >> +
> > >> + sleep(5);
> > >
> > > Why sleep()?
> > >
> >
> > Want to give the agent time to send a response. It's still racy, but
> > less so that immediately issuing the shutdown.
>
> It's only less race for the particular test-case you're testing :)
>
> > Ideal we'd push the
> > sleep() into shutdown's time param, but that only has minute resolution.
>
> I don't think so, because this is an implementation detail. The right thing
> would be to make the child wait for an "go ahead" signal from the parent.
>
> This could be done by calling pause() in the child and making the parent
> send a SIGUSR1 when the response has been sent.
Using pause() this way is obviously racy too, we have to block the signal
in the parent and call sigsuspend() in the child.
>
> One way to do it w/o tying sever actions (like to send a response) to
> agent commands, would be to introduce a "response sent" callback, so that
> commands could register and have their specific actions executed at the
> right time.
>
> >
> > >> + ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
> > >> + "hypervisor initiated shutdown", (char*)NULL);
> > >> + exit(!!ret);
> > >> + } else if (ret< 0) {
> > >> + error_set(err, QERR_UNDEFINED_ERROR);
> > >> + }
> > >
> > > Doesn't have the parent process wait for the child, so that exec() errors
> > > can be reported?
> >
> > The exec() won't return until the shutdown is executed, so the RPC's
> > behavior would be racy. At some point I documented that the shutdown is
> > an async "request" that may or may not complete but that was lost in the
> > reworking. I'll clarify in the schema.
>
> Seems fine.
>
> >
> > >
> > >> +}
> > >> +
> > >> +typedef struct GuestFileHandle {
> > >> + uint64_t id;
> > >> + FILE *fh;
> > >> +} GuestFileHandle;
> > >> +
> > >> +static struct {
> > >> + GSList *filehandles;
> > >
> > > Wouldn't this be simpler if we use qemu list implementation instead?
> > >
> >
> > YES! This stuff is terribly verbose. I was trying to stick with glib
> > outside of QMP/QAPI stuff though...and I think more glib stuff will find
> > it's way into here over time that'll make appearances of
> > GSList/gmalloc/etc inevitable.
> >
> > But if intermixing is not a big deal I'm more than happy to "allow" qemu
> > malloc/list stuff where it makes sense, and refactor these accordingly.
>
> It makes sense to me.
>
> > >> + uint64_t last_id;
> > >> +} guest_file_state;
> > >> +
> > >> +static int64_t guest_file_handle_add(FILE *fh)
> > >> +{
> > >> + GuestFileHandle *gfh;
> > >> +
> > >> + gfh = g_malloc(sizeof(GuestFileHandle));
> > >> + gfh->id = guest_file_state.last_id++;
> > >
> > > I don't know if the uint64_t limit can be reached in practice, but I'd
> > > expect a bitmap, so that you can return ids in guest_file_handle_remove().
> > >
> > > Another simpler option would be to use the real fd instead. I mean, the
> > > one
> > > returned by the guest kernel.
> > >
> >
> > I think I'll go with this. I was hesitant to do this at first since I
> > didn't want users specifying FDs opened outside of guest-file-open, but
> > so long as we only rely on our internal list of open FDs I guess that's
> > not applicable.
>
> Yes.
>
> > >> + gfh->fh = fh;
> > >> + guest_file_state.filehandles =
> > >> g_slist_append(guest_file_state.filehandles,
> > >> + gfh);
> > >> + return gfh->id;
> > >> +}
> > >> +
> > >> +static gint guest_file_handle_match(gconstpointer elem, gconstpointer
> > >> id_p)
> > >> +{
> > >> + const uint64_t *id = id_p;
> > >> + const GuestFileHandle *gfh = elem;
> > >> +
> > >> + g_assert(gfh);
> > >> + return (gfh->id != *id);
> > >> +}
> > >> +
> > >> +static FILE *guest_file_handle_find(int64_t id)
> > >> +{
> > >> + GSList *elem = g_slist_find_custom(guest_file_state.filehandles,&id,
> > >> + guest_file_handle_match);
> > >> + GuestFileHandle *gfh;
> > >> +
> > >> + if (elem) {
> > >> + g_assert(elem->data);
> > >> + gfh = elem->data;
> > >> + return gfh->fh;
> > >> + }
> > >> +
> > >> + return NULL;
> > >> +}
> > >> +
> > >> +static void guest_file_handle_remove(int64_t id)
> > >> +{
> > >> + GSList *elem = g_slist_find_custom(guest_file_state.filehandles,&id,
> > >> + guest_file_handle_match);
> > >> + gpointer data = elem->data;
> > >> +
> > >> + if (!data) {
> > >> + return;
> > >> + }
> > >> + guest_file_state.filehandles =
> > >> g_slist_remove(guest_file_state.filehandles,
> > >> + data);
> > >> + g_free(data);
> > >> +}
> > >> +
> > >> +int64_t qmp_guest_file_open(const char *filepath, const char *mode,
> > >> Error **err)
> > >> +{
> > >> + FILE *fh;
> > >> + int fd, ret;
> > >> + int64_t id = -1;
> > >> +
> > >> + if (!logging_enabled()) {
> > >> + error_set(err, QERR_QGA_LOGGING_FAILED);
> > >> + goto out;
> > >
> > > No need to have a goto here, just do return -1. This true for other
> > > functions
> > > too.
> > >
> > >> + }
> > >> + 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);
> > >> + goto out;
> > >> + }
> > >> +
> > >> + /* 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);
> > >> + goto out;
> > >> + }
> > >> +
> > >> + id = guest_file_handle_add(fh);
> > >> + slog("guest-file-open, filehandle: %ld", id);
> > >> +out:
> > >> + return id;
> > >> +}
> > >> +
> > >> +struct GuestFileRead *qmp_guest_file_read(int64_t filehandle, int64_t
> > >> count,
> > >> + Error **err)
> > >> +{
> > >> + GuestFileRead *read_data;
> > >> + guchar *buf;
> > >> + FILE *fh = guest_file_handle_find(filehandle);
> > >> + size_t read_count;
> > >> +
> > >> + if (!fh) {
> > >> + error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> > >> + return NULL;
> > >> + }
> > >> +
> > >> + read_data = g_malloc0(sizeof(GuestFileRead));
> > >> + buf = g_malloc(count);
> > >
> > > What happens if the client passes a bogus value? Like -1 or a very big
> > > number?
> > >
> > > I think count has to be checked against the file size. You could call
> > > stat()
> > > and store the value. Also, you're not checking g_malloc()'s return, so a
> > > bad
> > > allocation can crash the agent.
> > >
> >
> > All good points
> >
> > >> +
> > >> + read_count = fread(buf, 1, count, fh);
> > >> + buf[read_count] = 0;
> > >
> > > We need to allocate an additional byte to do that.
> > >
> > >> + read_data->count = read_count;
> > >> + read_data->eof = feof(fh);
> > >> + if (read_count) {
> > >> + read_data->buf = g_base64_encode(buf, read_count);
> > >> + }
> > >> + g_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;
> > >> + FILE *fh = guest_file_handle_find(filehandle);
> > >> +
> > >> + if (!fh) {
> > >> + error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> > >> + return NULL;
> > >> + }
> > >> +
> > >> + write_data = g_malloc0(sizeof(GuestFileWrite));
> > >> + data = g_base64_decode(data_b64,&data_len);
> > >> + write_count = fwrite(data, 1, MIN(count, data_len), fh);
> > >
> > > IMO, we should do what the user is asking us to do, if it's impossible we
> > > should return an error. IOW, we should use count. It's okay if the buffer
> > > is bigger then count, but if count is bigger then we should return an
> > > error.
> > >
> > > What does write() do in this case? segfaults?
> > >
> > >> + write_data->count = write_count;
> > >> + write_data->eof = feof(fh);
> > >> + g_free(data);
> > >> + clearerr(fh);
> > >> +
> > >> + return write_data;
> > >> +}
> > >> +
> > >> +struct GuestFileSeek *qmp_guest_file_seek(int64_t filehandle, int64_t
> > >> offset,
> > >> + int64_t whence, Error **err)
> > >> +{
> > >> + GuestFileSeek *seek_data;
> > >> + FILE *fh = guest_file_handle_find(filehandle);
> > >> + int ret;
> > >> +
> > >> + if (!fh) {
> > >> + error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> > >> + return NULL;
> > >> + }
> > >> +
> > >> + seek_data = g_malloc0(sizeof(GuestFileRead));
> > >> + ret = fseek(fh, offset, whence);
> > >> + if (ret == -1) {
> > >> + error_set(err, QERR_UNDEFINED_ERROR);
> > >> + g_free(seek_data);
> > >> + return NULL;
> > >> + }
> > >> + seek_data->position = ftell(fh);
> > >> + seek_data->eof = feof(fh);
> > >> + clearerr(fh);
> > >> +
> > >> + return seek_data;
> > >> +}
> > >> +
> > >> +void qmp_guest_file_close(int64_t filehandle, Error **err)
> > >> +{
> > >> + FILE *fh = guest_file_handle_find(filehandle);
> > >> +
> > >> + if (!logging_enabled()) {
> > >> + error_set(err, QERR_QGA_LOGGING_FAILED);
> > >> + return;
> > >> + }
> > >> + slog("guest-file-close called, filehandle: %ld", filehandle);
> > >> + if (!fh) {
> > >> + error_set(err, QERR_FD_NOT_FOUND, "filehandle");
> > >> + return;
> > >> + }
> > >> +
> > >> + fclose(fh);
> > >> + guest_file_handle_remove(filehandle);
> > >> +}
> > >> +
> > >> +/*
> > >> + * Walk the mount table and build a list of local file systems
> > >> + */
> > >> +
> > >> +struct direntry {
> > >> + char *dirname;
> > >> + char *devtype;
> > >> + struct direntry *next;
> > >
> > > Wouldn't it be better to use qemu's list implementation?
> > >
> >
> > yes, ill fix all the list stuff up to use qemu's
> >
> > >> +};
> > >> +
> > >> +struct {
> > >> + struct direntry *mount_list;
> > >> + GuestFsfreezeStatus status;
> > >> +} guest_fsfreeze_state;
> > >> +
> > >> +static int guest_fsfreeze_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) {
> > >> + g_warning("fsfreeze: unable to read mtab");
> > >> + 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 = g_malloc(sizeof(struct direntry));
> > >> + entry->dirname = qemu_strdup(mnt->mnt_dir);
> > >> + entry->devtype = qemu_strdup(mnt->mnt_type);
> > >> + entry->next = guest_fsfreeze_state.mount_list;
> > >> +
> > >> + guest_fsfreeze_state.mount_list = entry;
> > >> + }
> > >> +
> > >> + endmntent(fp);
> > >> +
> > >> + return 0;
> > >> +
> > >> +fail:
> > >> + while(guest_fsfreeze_state.mount_list) {
> > >> + next = guest_fsfreeze_state.mount_list->next;
> > >> + g_free(guest_fsfreeze_state.mount_list->dirname);
> > >> + g_free(guest_fsfreeze_state.mount_list->devtype);
> > >> + g_free(guest_fsfreeze_state.mount_list);
> > >> + guest_fsfreeze_state.mount_list = next;
> > >> + }
> > >> +
> > >> + 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 direntry *entry;
> > >> + int fd;
> > >> +
> > >> + if (!logging_enabled()) {
> > >> + error_set(err, QERR_QGA_LOGGING_FAILED);
> > >> + goto out;
> > >> + }
> > >> +
> > >> + slog("guest-fsfreeze called");
> > >> +
> > >> + if (guest_fsfreeze_state.status != GUEST_FSFREEZE_STATUS_THAWED) {
> > >> + ret = 0;
> > >> + goto out;
> > >> + }
> > >> +
> > >> + ret = guest_fsfreeze_build_mount_list();
> > >> + if (ret< 0) {
> > >> + goto out;
> > >> + }
> > >> +
> > >> + guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_INPROGRESS;
> > >> +
> > >> + /* cannot risk guest agent blocking itself on a write in this state
> > >> */
> > >> + disable_logging();
> > >> +
> > >> + entry = guest_fsfreeze_state.mount_list;
> > >> + while(entry) {
> > >
> > > A for() loop would be clearer, imho.
> > >
> > >> + fd = qemu_open(entry->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;
> > >
> > > Does the FIFREEZE ioctl() call returns the errno code? If it doesn't, then
> > > we have to set it as the qemu_open() does above.
> > >
> >
> > Nope, -1 + errno, good catch.
> >
> > >> + }
> > >> + close(fd);
> > >> +
> > >> + entry = entry->next;
> > >> + 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;
> > >> + }
> > >
> > > I'm not sure this correct. What happens if an error happens in the
> > > first iteration of the while loop? A better way would to check 'ret'
> > > instead.
> > >
> >
> > I believe this is meant to track states where some filesystems have been
> > frozen and others not. We want to return the count, but not the error
> > via fsfreeze_status. If we bail in the first iteration it means FIFREEZE
> > was never completed successfully. We should reset state the THAWED then
> > though.
> >
> > >> + goto out;
> > >> +}
> > >> +
> > >> +/*
> > >> + * Walk list of frozen file systems in the guest, and thaw them.
> > >> + */
> > >> +int64_t qmp_guest_fsfreeze_thaw(Error **err)
> > >> +{
> > >> + int ret;
> > >> + struct direntry *entry;
> > >> + int fd, i = 0;
> > >> +
> > >> + if (guest_fsfreeze_state.status != GUEST_FSFREEZE_STATUS_FROZEN&&
> > >> + guest_fsfreeze_state.status !=
> > >> GUEST_FSFREEZE_STATUS_INPROGRESS) {
> > >> + ret = 0;
> > >> + goto out_enable_logging;
> > >> + }
> > >> +
> > >> + while((entry = guest_fsfreeze_state.mount_list)) {
> > >> + fd = qemu_open(entry->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;
> > >> + }
> > >> + close(fd);
> > >> +
> > >> + guest_fsfreeze_state.mount_list = entry->next;
> > >> + g_free(entry->dirname);
> > >> + g_free(entry->devtype);
> > >> + g_free(entry);
> > >> + 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;
> > >> +}
> > >> +
> > >> +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);
> > >> +}
> > >> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> > >> index 66d1729..a85a5e4 100644
> > >> --- a/qga/guest-agent-core.h
> > >> +++ b/qga/guest-agent-core.h
> > >> @@ -18,6 +18,7 @@
> > >> 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));
> > >
> >
>
