[Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi

This is a first attempt to add fsfreeze support to virtagent. The idea
is for the guest agent to walk the list of locally mounted file
systems in the guest, and issuing an ioctl to freeze them. The host
can then do a live snapshot of the guest, obtaining stable file
systems. After the snapshot, the host then calls the thaw function in
virtagent, which goes through the list of previously frozen file
systems and unfreezes them.

The list walking ignores remote file systems such as NFS and CIFS as
well as all pseudo file systems.

The guest agent code is in the first patch, and host agent code is in
the second patch. For now there is only human monitor support, but it
should be pretty straight forward to add QMP support as well.

Patches are against the virtagent-dev git tree.

Comments and suggestions welcome!

Cheers,
Jes


Jes Sorensen (2):
  Add virtagent file system freeze/thaw
  Add monitor commands for fsfreeze support

 hmp-commands.hx    |   48 +++++++++++
 virtagent-common.h |    9 ++
 virtagent-server.c |  196 +++++++++++++++++++++++++++++++++++++++++++
 virtagent.c        |  235 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtagent.h        |    9 ++
 5 files changed, 497 insertions(+), 0 deletions(-)

-- 
1.7.3.5

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

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

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 <Jes.Sorensen@redhat.com>
---
 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;
+	}
+	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;
+    }
+
+    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 (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;
+    }
+
+    while((entry = mount_list)) {
+        fd = qemu_open(entry->dirname, O_RDONLY);
+        if (fd == -1) {
+            ret = -1;
+            goto out;
+        }
+        ret = ioctl(fd, FITHAW);
+        if (ret < 0 && ret != EOPNOTSUPP) {
+            ret = -1;
+            goto out;
+	}
+        close(fd);
+
+        mount_list = entry->next;
+        qemu_free(entry->dirname);
+        qemu_free(entry->devtype);
+        qemu_free(entry);
+        i++;
+    }
+
+    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", 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 +427,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[] = {
-- 
1.7.3.5

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

[Stefan Hajnoczi]

On Tue, Feb 1, 2011 at 10:58 AM,  <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> 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 <Jes.Sorensen@redhat.com>
> ---
>  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

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

[Jes Sorensen]

On 02/01/11 15:12, Stefan Hajnoczi wrote:
> On Tue, Feb 1, 2011 at 10:58 AM,  <Jes.Sorensen@redhat.com> wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> 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?

I have to admit you lost me here, where do you get that 500ms time from?
Is that the XMLRPC polling time or? I just used the example code from
other agent calls.

> 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

I haven't looked at it, I designed the calls based on how they fit with
the Linux ioctls.

>> +       entry = qemu_malloc(sizeof(struct direntry));
>> +       if (!entry) {
>> +           goto fail;
>> +       }
> 
> qemu_malloc() never fails.

Good point, we have ugly malloc in qemu :( I wrote the code to handle
this outside QEMU first, to make sure it worked correctly and trying to
see how many times I could crash my laptop in the process. I'll fix it.

>> +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.

Good point, I'll fix this.

>> +
>> +    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.

Good point, will fix.

>> +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.

Yep, will fix

>> +
>> +    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.

Thanks for the review, all valid points!

Cheers,
Jes

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

[Stefan Hajnoczi]

On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 02/01/11 15:12, Stefan Hajnoczi wrote:
>> On Tue, Feb 1, 2011 at 10:58 AM,  <Jes.Sorensen@redhat.com> wrote:
>>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>
>>> 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?
>
> I have to admit you lost me here, where do you get that 500ms time from?
> Is that the XMLRPC polling time or? I just used the example code from
> other agent calls.

500 ms is made up.  I was thinking, "what would a reasonable polling
interval be?" and picked a sub-second number.

Can you explain how the timeout in fsfreeze can happen?  It's probably
because I don't know the virtagent details.

Stefan

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

[Jes Sorensen]

On 02/01/11 15:34, Stefan Hajnoczi wrote:
> On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>> I have to admit you lost me here, where do you get that 500ms time from?
>> Is that the XMLRPC polling time or? I just used the example code from
>> other agent calls.
> 
> 500 ms is made up.  I was thinking, "what would a reasonable polling
> interval be?" and picked a sub-second number.
> 
> Can you explain how the timeout in fsfreeze can happen?  It's probably
> because I don't know the virtagent details.

Ah ok.

>From what I understand, the XMLRPC code is setup to timeout if the guest
doesn't reply within a certain amount of time. In that case, the caller
needs to poll to wait for the guest to complete the freeze. This really
should only happen if you have a guest with a large number of very large
file systems. I don't know how likely it is to happen in real life.

Cheers,
Jes

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

[Stefan Hajnoczi]

On Tue, Feb 1, 2011 at 2:36 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 02/01/11 15:34, Stefan Hajnoczi wrote:
>> On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>>> I have to admit you lost me here, where do you get that 500ms time from?
>>> Is that the XMLRPC polling time or? I just used the example code from
>>> other agent calls.
>>
>> 500 ms is made up.  I was thinking, "what would a reasonable polling
>> interval be?" and picked a sub-second number.
>>
>> Can you explain how the timeout in fsfreeze can happen?  It's probably
>> because I don't know the virtagent details.
>
> Ah ok.
>
> From what I understand, the XMLRPC code is setup to timeout if the guest
> doesn't reply within a certain amount of time. In that case, the caller
> needs to poll to wait for the guest to complete the freeze. This really
> should only happen if you have a guest with a large number of very large
> file systems. I don't know how likely it is to happen in real life.

Perhaps Michael can confirm that the freeze function continues to
execute after timeout but the client is able to send fsstatus()
requests?

Stefan

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

[Michael Roth]

On 02/01/2011 08:41 AM, Stefan Hajnoczi wrote:
> On Tue, Feb 1, 2011 at 2:36 PM, Jes Sorensen<Jes.Sorensen@redhat.com>  wrote:
>> On 02/01/11 15:34, Stefan Hajnoczi wrote:
>>> On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen<Jes.Sorensen@redhat.com>  wrote:
>>>> I have to admit you lost me here, where do you get that 500ms time from?
>>>> Is that the XMLRPC polling time or? I just used the example code from
>>>> other agent calls.
>>>
>>> 500 ms is made up.  I was thinking, "what would a reasonable polling
>>> interval be?" and picked a sub-second number.
>>>
>>> Can you explain how the timeout in fsfreeze can happen?  It's probably
>>> because I don't know the virtagent details.
>>
>> Ah ok.
>>
>>  From what I understand, the XMLRPC code is setup to timeout if the guest
>> doesn't reply within a certain amount of time. In that case, the caller
>> needs to poll to wait for the guest to complete the freeze. This really
>> should only happen if you have a guest with a large number of very large
>> file systems. I don't know how likely it is to happen in real life.
>
> Perhaps Michael can confirm that the freeze function continues to
> execute after timeout but the client is able to send fsstatus()
> requests?

Ahh, yeah there's the confusion: we only execute one RPC at a time, so a 
polling function for a previous RPC won't work unless that RPC is being 
done concurrently, via fork()ing or something and communicating status 
via some method of IPC.

I touched on possible approaches to dealing with this in the response I 
just sent to this patch.

>
> Stefan

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

[Adam Litke]

On Tue, 2011-02-01 at 11:58 +0100, Jes.Sorensen@redhat.com wrote:
> +/*
> + * 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;
> +    }
> +
> +    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 (ret < 0 && ret != EOPNOTSUPP) {
> +            goto error;
> +        }

Here we silently ignore filesystems that do not support the FIFREEZE
ioctl.  Do we need to have a more complex return value so that we can
communicate which mount points could not be frozen?  Otherwise, an
unsuspecting host could retrieve a corrupted snapshot of that
filesystem, right?

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

-- 
Thanks,
Adam

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

[Jes Sorensen]

On 02/01/11 15:48, Adam Litke wrote:
> On Tue, 2011-02-01 at 11:58 +0100, Jes.Sorensen@redhat.com wrote:
>> +/*
>> + * 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;
>> +    }
>> +
>> +    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 (ret < 0 && ret != EOPNOTSUPP) {
>> +            goto error;
>> +        }
> 
> Here we silently ignore filesystems that do not support the FIFREEZE
> ioctl.  Do we need to have a more complex return value so that we can
> communicate which mount points could not be frozen?  Otherwise, an
> unsuspecting host could retrieve a corrupted snapshot of that
> filesystem, right?

That is correct, however most Linux file systems do support it, and for
the ones that don't, there really isn't anything we can do.

Cheers,
Jes

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

[Michael Roth]

On 02/01/2011 04:58 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> 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<Jes.Sorensen@redhat.com>
> ---
>   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,
> +};

Any reason for vs_* vs. va_*?

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

Can probably clean these up a bit, I believe fcntl.h/errno.h/stat.h are 
already available at least.

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

should be spaces instead of tabs

> +
> +    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;
> +    }
> +
> +    ret = build_mount_list();
> +    if (ret<  0) {
> +        goto out;
> +    }
> +
> +    fsfreeze_status = FREEZE_INPROGRESS;
> +
> +    entry = mount_list;

I think as we start adding more and more stateful RPCs, free-floating 
state variables can start getting a bit hairy to keep track of. 
Eventually I'd like to have state information that only applies to a 
subset of RPCs consolidated into a single object. I wouldn't focus on 
this too much because I'd like to have an interface to do this in the 
future (mainly so they can state objects can register themselves and 
provide a reset() function that can be called when, for instance, an 
agent disconnects/reconnects), but in the meantime I think it would be 
more readable to have a global va_fsfreeze_state object to track freeze 
status and mount points.

> +    while(entry) {
> +        fd = qemu_open(entry->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = errno;
> +            goto error;
> +        }
> +        ret = ioctl(fd, FIFREEZE);
> +        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;
> +    }
> +
> +    while((entry = mount_list)) {
> +        fd = qemu_open(entry->dirname, O_RDONLY);
> +        if (fd == -1) {
> +            ret = -1;
> +            goto out;
> +        }
> +        ret = ioctl(fd, FITHAW);
> +        if (ret<  0&&  ret != EOPNOTSUPP) {
> +            ret = -1;
> +            goto out;
> +	}

whitespace issues

> +        close(fd);
> +
> +        mount_list = entry->next;
> +        qemu_free(entry->dirname);
> +        qemu_free(entry->devtype);
> +        qemu_free(entry);
> +        i++;
> +    }
> +
> +    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", fsfreeze_status);
> +    SLOG("va_fsstatus()");
> +    return result;
> +}

Hmm, you mentioned before that these freezes may be long-running 
jobs...do the ioctl()'s not return until completion? There is global 
timeout in virtagent, currently under a minute, to prevent a virtagent 
monitor command from hanging the monitor session, so if it's unlikely 
you'll fit in this window we'll need to work on something to better 
support these this kinds of situations.

The 3 main approaches would be:

1) allow command-specific timeouts with values that are sane for the 
command in question, and potentially allow timeouts to be disabled
2) fork() long running jobs and provide a mechanism for them to provide 
asynchronous updates to us to we can query status
3) fork() long running jobs, have them provide status information 
elsewhere, and provide a polling function to check that status

3) would likely require something like writing status to a file and then 
provide a polling function to check it, which doesn't work here so 
that's probably out.

I'd initially planned on doing 2) at some point, but I'm beginning to 
think 1) is the better approach, since qemu "opts in" on how long it's 
willing to hang for a particular command, so there's not really any 
surprises. At least not to qemu...users might get worried after a while, 
so there is a bit of a trade-off. But it's also more user-friendly....no 
need for polling or dealing with asynchronous updates to figure out when 
an RPC has actually finished. Seem reasonable?

> +
>   typedef struct RPCFunction {
>       xmlrpc_value *(*func)(xmlrpc_env *env, xmlrpc_value *param, void *unused);
>       const char *func_name;
> @@ -237,6 +427,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[] = {

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

[Jes Sorensen]

On 02/01/11 17:50, Michael Roth wrote:
> On 02/01/2011 04:58 AM, Jes.Sorensen@redhat.com wrote:
>> +enum vs_fsfreeze_status {
>> +    FREEZE_ERROR = -1,
>> +    FREEZE_THAWED = 0,
>> +    FREEZE_INPROGRESS = 1,
>> +    FREEZE_FROZEN = 2,
>> +    FREEZE_THAWINPROGRESS = 3,
>> +};
> 
> Any reason for vs_* vs. va_*?

Hmmmm let me see if I can find a good excuse for that typo :)

>> 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>
> 
> Can probably clean these up a bit, I believe fcntl.h/errno.h/stat.h are
> already available at least.

Carry-over from writing the code outside of qemu. Would be much cleaner
than relying on the include everything and the kitchen sink in a global
header file, but thats how it is :(

>> +
>> +    fsfreeze_status = FREEZE_INPROGRESS;
>> +
>> +    entry = mount_list;
> 
> I think as we start adding more and more stateful RPCs, free-floating
> state variables can start getting a bit hairy to keep track of.
> Eventually I'd like to have state information that only applies to a
> subset of RPCs consolidated into a single object. I wouldn't focus on
> this too much because I'd like to have an interface to do this in the
> future (mainly so they can state objects can register themselves and
> provide a reset() function that can be called when, for instance, an
> agent disconnects/reconnects), but in the meantime I think it would be
> more readable to have a global va_fsfreeze_state object to track freeze
> status and mount points.

Urgh, what do you mean by object here? I have to admit the word object
always makes me cringe.... I changed the variables to have the va_ prefix.

>> +static xmlrpc_value *va_fsstatus(xmlrpc_env *env,
>> +                                 xmlrpc_value *params,
>> +                                 void *user_data)
>> +{
>> +    xmlrpc_value *result = xmlrpc_build_value(env, "i",
>> fsfreeze_status);
>> +    SLOG("va_fsstatus()");
>> +    return result;
>> +}
> 
> Hmm, you mentioned before that these freezes may be long-running
> jobs...do the ioctl()'s not return until completion? There is global
> timeout in virtagent, currently under a minute, to prevent a virtagent
> monitor command from hanging the monitor session, so if it's unlikely
> you'll fit in this window we'll need to work on something to better
> support these this kinds of situations.

I think 1 minute is fine, but we should probably look at something a
little more flexible for handling commands over the longer term. Maybe
have virtagent spawn threads for executing some commands?

> The 3 main approaches would be:
> 
> 1) allow command-specific timeouts with values that are sane for the
> command in question, and potentially allow timeouts to be disabled
> 2) fork() long running jobs and provide a mechanism for them to provide
> asynchronous updates to us to we can query status
> 3) fork() long running jobs, have them provide status information
> elsewhere, and provide a polling function to check that status
> 
> 3) would likely require something like writing status to a file and then
> provide a polling function to check it, which doesn't work here so
> that's probably out.
> 
> I'd initially planned on doing 2) at some point, but I'm beginning to
> think 1) is the better approach, since qemu "opts in" on how long it's
> willing to hang for a particular command, so there's not really any
> surprises. At least not to qemu...users might get worried after a while,
> so there is a bit of a trade-off. But it's also more user-friendly....no
> need for polling or dealing with asynchronous updates to figure out when
> an RPC has actually finished. Seem reasonable?

I am not sure which is really the best solution. Basically we will need
to classify commands into two categories, so if you issue a certain type
of command, like agent_fsfreeze() (basically when the agent is in
FREEZE_FROZEN state) only status commands are allowed to execute in
parallel. Anything that tries to issue a write to the file system will
hang until agent_fsthaw is called. However it would be useful to be able
to call in for non-blocking status commands etc.

I'll post a v2 in a minute that addresses the issues pointed out by
Stefan and you. I think the threading/timeout aspect is something we
need to look at for the longer term.

Cheers,
Jes

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

[Stefan Hajnoczi]

On Tue, Feb 1, 2011 at 10:58 AM,  <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> 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.

Does this add a requirement that guest agent code issues no disk I/O
in its main loop (e.g. logging)?  Otherwise we might deadlock
ourselves waiting for I/O which is never issued.

Stefan

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

[Jes Sorensen]

On 02/02/11 08:57, Stefan Hajnoczi wrote:
> On Tue, Feb 1, 2011 at 10:58 AM,  <Jes.Sorensen@redhat.com> wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> 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.
> 
> Does this add a requirement that guest agent code issues no disk I/O
> in its main loop (e.g. logging)?  Otherwise we might deadlock
> ourselves waiting for I/O which is never issued.

Yes very much so[1] - one reason why it would be nice to have virtagent
use threads to execute the actual commands. We should probably add a
flag to agent commands indicating whether they issue disk I/O or not, so
we can block attempts to execute commands that do so, while the guest is
frozen.

Cheers,
Jes

[1] speaking from experience ... a Linux desktop gets really upset if
you freeze the file systems from a command in an xterm.... ho hum

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

[Michael Roth]

On 02/02/2011 02:48 AM, Jes Sorensen wrote:
> On 02/02/11 08:57, Stefan Hajnoczi wrote:
>> On Tue, Feb 1, 2011 at 10:58 AM,<Jes.Sorensen@redhat.com>  wrote:
>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>
>>> 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.
>>
>> Does this add a requirement that guest agent code issues no disk I/O
>> in its main loop (e.g. logging)?  Otherwise we might deadlock
>> ourselves waiting for I/O which is never issued.
>
> Yes very much so[1] - one reason why it would be nice to have virtagent
> use threads to execute the actual commands. We should probably add a
> flag to agent commands indicating whether they issue disk I/O or not, so
> we can block attempts to execute commands that do so, while the guest is
> frozen.

**Warning, epic response**

For things like logging and i/o on a frozen system...I agree we'd need 
some flag for these kinds of situations. Maybe a disable_logging() 
flag....i really don't like this though... I'd imagine even syslogd() 
could block virtagent in this type of situation, so that would need to 
be disabled as well.

But doing so completely subverts our attempts and providing proper 
accounting of what the agent is doing to the user. A user can freeze the 
filesystem, knowing that logging would be disabled, then prod at 
whatever he wants. So the handling should be something specific to 
fsfreeze, with stricter requirements:

If a user calls fsfreeze(), we disable logging, but also disable the 
ability to do anything other than fsthaw() or fsstatus(). This actually 
solves the potential deadlocking problem for other RPCs as well...since 
they cant be executed in the first place.

So I think that addresses the agent deadlocking itself, post-freeze.

However, fsfreeze() itself might lock-up the agent as well...I'm not 
confident we can really put any kind of bound on how long it'll take to 
execute, and if we timeout on the client-side the agent can still block 
here.

Plus there are any number of other situations where an RPC can still 
hang things...in the future when we potentially allow things like script 
execution, they might do something like attempt to connect to a socket 
that's already in use and wait on the server for an arbitrary amount of 
time, or open a file on an nfs share that in currently unresponsive.

So a solution for these situations is still needed, and I'm starting to 
agree that threads are needed, but I don't think we should do RPCs 
concurrently (not sure if that's what is being suggested or not). At 
least, there's no pressing reason for it as things currently stand 
(there aren't currently any RPCs where fast response times are all that 
important, so it's okay to serialize them behind previous RPCs, and 
HMP/QMP are command at a time), and it's something that Im fairly 
confident can be added if the need arises in the future.

But for dealing with a situation where an RPC can hang the agent, I 
think one thread should do it. Basically:

We associate each RPC with a time limit. Some RPCs, very special ones 
that we'd trust with our kids, could potentially specify an unlimited 
timeout. The client side should use this same timeout on it's end. In 
the future we might allow the user to explicitly disable the timeout for 
a certain RPC. The logic would then be:

- read in a client RPC request
- start a thread to do RPC
- if there's a timeout, register an alarm(<timeout>), with a handler 
that will call something like pthread_kill(current_worker_thread). On 
the thread side, this signal will induce a pthread_exit()
- wait for the thread to return (pthread_join(current_worker_thread))
- return it's response back to the caller if it finished, return a 
timeout indication otherwise

>
> Cheers,
> Jes
>
> [1] speaking from experience ... a Linux desktop gets really upset if
> you freeze the file systems from a command in an xterm.... ho hum

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

[Stefan Hajnoczi]

On Thu, Feb 3, 2011 at 5:41 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> For things like logging and i/o on a frozen system...I agree we'd need some
> flag for these kinds of situations. Maybe a disable_logging() flag....i
> really don't like this though... I'd imagine even syslogd() could block
> virtagent in this type of situation, so that would need to be disabled as
> well.
>
> But doing so completely subverts our attempts and providing proper
> accounting of what the agent is doing to the user. A user can freeze the
> filesystem, knowing that logging would be disabled, then prod at whatever he
> wants. So the handling should be something specific to fsfreeze, with
> stricter requirements:
>
> If a user calls fsfreeze(), we disable logging, but also disable the ability
> to do anything other than fsthaw() or fsstatus(). This actually solves the
> potential deadlocking problem for other RPCs as well...since they cant be
> executed in the first place.
>
> So I think that addresses the agent deadlocking itself, post-freeze.
>
> However, fsfreeze() itself might lock-up the agent as well...I'm not
> confident we can really put any kind of bound on how long it'll take to
> execute, and if we timeout on the client-side the agent can still block
> here.
>
> Plus there are any number of other situations where an RPC can still hang
> things...in the future when we potentially allow things like script
> execution, they might do something like attempt to connect to a socket
> that's already in use and wait on the server for an arbitrary amount of
> time, or open a file on an nfs share that in currently unresponsive.
>
> So a solution for these situations is still needed, and I'm starting to
> agree that threads are needed, but I don't think we should do RPCs
> concurrently (not sure if that's what is being suggested or not). At least,
> there's no pressing reason for it as things currently stand (there aren't
> currently any RPCs where fast response times are all that important, so it's
> okay to serialize them behind previous RPCs, and HMP/QMP are command at a
> time), and it's something that Im fairly confident can be added if the need
> arises in the future.
>
> But for dealing with a situation where an RPC can hang the agent, I think
> one thread should do it. Basically:
>
> We associate each RPC with a time limit. Some RPCs, very special ones that
> we'd trust with our kids, could potentially specify an unlimited timeout.
> The client side should use this same timeout on it's end. In the future we
> might allow the user to explicitly disable the timeout for a certain RPC.
> The logic would then be:
>
> - read in a client RPC request
> - start a thread to do RPC
> - if there's a timeout, register an alarm(<timeout>), with a handler that
> will call something like pthread_kill(current_worker_thread). On the thread
> side, this signal will induce a pthread_exit()
> - wait for the thread to return (pthread_join(current_worker_thread))
> - return it's response back to the caller if it finished, return a timeout
> indication otherwise

I'm not sure about a timeout inside virtagent.  A client needs to
protect itself with its own timeout and shouldn't rely on the server
to prevent it from locking up - especially since the server is a guest
which we have no control over.  So the timeout does not help the
guest.

Aborting an RPC handler could leave the system in an inconsistent
state unless we are careful.  For example, aborting freeze requires
thawing those file systems that have been successfully frozen so far.
For other handlers it might leave temporary files around, or if they
are not carefully written may partially update files in-place and
leave them corrupted.

So instead of a blanket timeout, I think handlers that perform
operations that may block for unknown periods of time could
specifically use timeouts.  That gives the handler control to perform
cleanup.

Stefan

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

[Michael Roth]

On 02/04/2011 12:13 AM, Stefan Hajnoczi wrote:
> On Thu, Feb 3, 2011 at 5:41 PM, Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>> For things like logging and i/o on a frozen system...I agree we'd need some
>> flag for these kinds of situations. Maybe a disable_logging() flag....i
>> really don't like this though... I'd imagine even syslogd() could block
>> virtagent in this type of situation, so that would need to be disabled as
>> well.
>>
>> But doing so completely subverts our attempts and providing proper
>> accounting of what the agent is doing to the user. A user can freeze the
>> filesystem, knowing that logging would be disabled, then prod at whatever he
>> wants. So the handling should be something specific to fsfreeze, with
>> stricter requirements:
>>
>> If a user calls fsfreeze(), we disable logging, but also disable the ability
>> to do anything other than fsthaw() or fsstatus(). This actually solves the
>> potential deadlocking problem for other RPCs as well...since they cant be
>> executed in the first place.
>>
>> So I think that addresses the agent deadlocking itself, post-freeze.
>>
>> However, fsfreeze() itself might lock-up the agent as well...I'm not
>> confident we can really put any kind of bound on how long it'll take to
>> execute, and if we timeout on the client-side the agent can still block
>> here.
>>
>> Plus there are any number of other situations where an RPC can still hang
>> things...in the future when we potentially allow things like script
>> execution, they might do something like attempt to connect to a socket
>> that's already in use and wait on the server for an arbitrary amount of
>> time, or open a file on an nfs share that in currently unresponsive.
>>
>> So a solution for these situations is still needed, and I'm starting to
>> agree that threads are needed, but I don't think we should do RPCs
>> concurrently (not sure if that's what is being suggested or not). At least,
>> there's no pressing reason for it as things currently stand (there aren't
>> currently any RPCs where fast response times are all that important, so it's
>> okay to serialize them behind previous RPCs, and HMP/QMP are command at a
>> time), and it's something that Im fairly confident can be added if the need
>> arises in the future.
>>
>> But for dealing with a situation where an RPC can hang the agent, I think
>> one thread should do it. Basically:
>>
>> We associate each RPC with a time limit. Some RPCs, very special ones that
>> we'd trust with our kids, could potentially specify an unlimited timeout.
>> The client side should use this same timeout on it's end. In the future we
>> might allow the user to explicitly disable the timeout for a certain RPC.
>> The logic would then be:
>>
>> - read in a client RPC request
>> - start a thread to do RPC
>> - if there's a timeout, register an alarm(<timeout>), with a handler that
>> will call something like pthread_kill(current_worker_thread). On the thread
>> side, this signal will induce a pthread_exit()
>> - wait for the thread to return (pthread_join(current_worker_thread))
>> - return it's response back to the caller if it finished, return a timeout
>> indication otherwise
>
> I'm not sure about a timeout inside virtagent.  A client needs to
> protect itself with its own timeout and shouldn't rely on the server
> to prevent it from locking up - especially since the server is a guest
> which we have no control over.  So the timeout does not help the
> guest.

We actually have timeouts for the client already (though they'll need to 
be reworked a bit to handle the proposed solutions), what I'm proposing 
is an additional timeout on the guest/server side for the actual RPCs, 
since a blocking RPC can still hang the guest agent.

>
> Aborting an RPC handler could leave the system in an inconsistent
> state unless we are careful.  For example, aborting freeze requires
> thawing those file systems that have been successfully frozen so far.
> For other handlers it might leave temporary files around, or if they
> are not carefully written may partially update files in-place and
> leave them corrupted.
>
> So instead of a blanket timeout, I think handlers that perform
> operations that may block for unknown periods of time could
> specifically use timeouts.  That gives the handler control to perform
> cleanup.

Good point. Although, I'm not sure I want to push timeout handling to 
the actual RPCs though....something as simple as open()/read() can block 
indefinitely in certain situations, and it'll be difficult to account 
for every situation, and the resulting code will be tedious as well. I'd 
really like the make the actual RPC as simple as possible, since it's 
something that may be extended heavily over time.

So what if we simply allow an RPC to register a timeout handler at the 
beginning of the RPC call? So when the thread doing the RPC exits we:

- check to see if thread exited as a result of timeout
- check to see if a timeout handler was registered, if so, call it, 
reset the handler, then return a timeout indication
- if it didn't time out, return the response

The only burden this puts on the RPC author is that information they 
need to recover state would need to be accessible outside the thread, 
which is easily done by encapsulating state in static/global structs. So 
the timeout handler for fsfreeze, as it is currently written, would be 
something like:

va_fsfreeze_timeout_handler():
     foreach mnt in fsfreeze.mount_list:
         unfreeze(mnt)
     fsfreeze.mount_list = NULL

We'll need to be careful about lists/objects being in weird states due 
to the forced exit, but I think it's doable.

>
> Stefan

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

[Stefan Hajnoczi]

On Fri, Feb 4, 2011 at 4:27 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
>> Aborting an RPC handler could leave the system in an inconsistent
>> state unless we are careful.  For example, aborting freeze requires
>> thawing those file systems that have been successfully frozen so far.
>> For other handlers it might leave temporary files around, or if they
>> are not carefully written may partially update files in-place and
>> leave them corrupted.
>>
>> So instead of a blanket timeout, I think handlers that perform
>> operations that may block for unknown periods of time could
>> specifically use timeouts.  That gives the handler control to perform
>> cleanup.
>
> Good point. Although, I'm not sure I want to push timeout handling to the
> actual RPCs though....something as simple as open()/read() can block
> indefinitely in certain situations, and it'll be difficult to account for
> every situation, and the resulting code will be tedious as well. I'd really
> like the make the actual RPC as simple as possible, since it's something
> that may be extended heavily over time.
>
> So what if we simply allow an RPC to register a timeout handler at the
> beginning of the RPC call? So when the thread doing the RPC exits we:
>
> - check to see if thread exited as a result of timeout
> - check to see if a timeout handler was registered, if so, call it, reset
> the handler, then return a timeout indication
> - if it didn't time out, return the response
>
> The only burden this puts on the RPC author is that information they need to
> recover state would need to be accessible outside the thread, which is
> easily done by encapsulating state in static/global structs. So the timeout
> handler for fsfreeze, as it is currently written, would be something like:
>
> va_fsfreeze_timeout_handler():
>    foreach mnt in fsfreeze.mount_list:
>        unfreeze(mnt)
>    fsfreeze.mount_list = NULL
>
> We'll need to be careful about lists/objects being in weird states due to
> the forced exit, but I think it's doable.

Yeah, still requires discipline but it could work.

Stefan

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

[Jes Sorensen]

On 02/03/11 18:41, Michael Roth wrote:
> On 02/02/2011 02:48 AM, Jes Sorensen wrote:
>> Yes very much so[1] - one reason why it would be nice to have virtagent
>> use threads to execute the actual commands. We should probably add a
>> flag to agent commands indicating whether they issue disk I/O or not, so
>> we can block attempts to execute commands that do so, while the guest is
>> frozen.
> 
> **Warning, epic response**
> 
> For things like logging and i/o on a frozen system...I agree we'd need
> some flag for these kinds of situations. Maybe a disable_logging()
> flag....i really don't like this though... I'd imagine even syslogd()
> could block virtagent in this type of situation, so that would need to
> be disabled as well.

One way to resolve this would be to have the logging handled in it's own
thread, which uses non blocking calls to do the actual logging.
Obviously we'd have to use a non file system based communication method
between the main thread and the logging thread :)

> But doing so completely subverts our attempts and providing proper
> accounting of what the agent is doing to the user. A user can freeze the
> filesystem, knowing that logging would be disabled, then prod at
> whatever he wants. So the handling should be something specific to
> fsfreeze, with stricter requirements:
> 
> If a user calls fsfreeze(), we disable logging, but also disable the
> ability to do anything other than fsthaw() or fsstatus(). This actually
> solves the potential deadlocking problem for other RPCs as well...since
> they cant be executed in the first place.

I disagree that we should block all calls, except for fsfreeze/fsstatus/
fsthaw in this case. There are other calls that could be valid in this
situations, so I think it needs to be evaluated on a case by case basis.

> So a solution for these situations is still needed, and I'm starting to
> agree that threads are needed, but I don't think we should do RPCs
> concurrently (not sure if that's what is being suggested or not). At
> least, there's no pressing reason for it as things currently stand
> (there aren't currently any RPCs where fast response times are all that
> important, so it's okay to serialize them behind previous RPCs, and
> HMP/QMP are command at a time), and it's something that Im fairly
> confident can be added if the need arises in the future.

Eventually I think we will need to be able to support concurrent RPC
calls. There can be situations where an operation takes a long time
while it is valid to be able to ping the guest agent to verify that it
is still alive etc.

Cheers,
Jes

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

[Michael Roth]

On 02/04/2011 05:03 AM, Jes Sorensen wrote:
> On 02/03/11 18:41, Michael Roth wrote:
>> On 02/02/2011 02:48 AM, Jes Sorensen wrote:
>>> Yes very much so[1] - one reason why it would be nice to have virtagent
>>> use threads to execute the actual commands. We should probably add a
>>> flag to agent commands indicating whether they issue disk I/O or not, so
>>> we can block attempts to execute commands that do so, while the guest is
>>> frozen.
>>
>> **Warning, epic response**
>>
>> For things like logging and i/o on a frozen system...I agree we'd need
>> some flag for these kinds of situations. Maybe a disable_logging()
>> flag....i really don't like this though... I'd imagine even syslogd()
>> could block virtagent in this type of situation, so that would need to
>> be disabled as well.
>
> One way to resolve this would be to have the logging handled in it's own
> thread, which uses non blocking calls to do the actual logging.
> Obviously we'd have to use a non file system based communication method
> between the main thread and the logging thread :)
>

I suspect syslogd() already buffers to some extent. But the problem 
there, as well the problem with having our own buffered logging 
implementation, is that we can't rely on being able to log an arbitrary 
number of messages. As some point that interface would need to either 
block, or stop dropping log messages.

If it blocks, we're deadlocked again. If it drops messages, it's trivial 
for someone to flood it with messages after the fsfreeze() to get back 
into a state where they can execute RPC without any accounting.

So a seperate logging thread doesn't buy us much, and what we come up 
with is rely gonna come down to syslogd:

1) if syslogd blocks, we must disable logging after fsfreeze(). 
Buffering, on syslogd's side or our own, only buys us time. If we 
disable logging, we must only allow absolutely required/benign RPCs. 
fsstatus/fsthaw are required, things like va.ping are benign, and 
potentially useful, in this particular situation. 
copyfile/shutdown/exec/etc should be considered "high-risk"...we want to 
make sure these are logged.

2) if syslogd doesnt block, it either drops messages at some point with 
no indication to us, or it drops them and provides some indication. If 
there's no indication, we must treat this the same way we treat 1), 
since we must assume that logging is effectively disabled. So only 
required or benign RPCs.

if we get some indication when a call to syslog() fails to log/buffer, 
we can allow all RPCs, but treat failures to log as a cue to immediately 
cleanup and exit the RPC. fsfreeze() under this circumstance will need 
to make sure it only logs after it does the unfreeze, else we may never 
be able to unfreeze at that point forward.

So the solution is dependent on syslogd's behavior. Ill have to do some 
testing to confirm...but I think the above covers the possibilities.

>> But doing so completely subverts our attempts and providing proper
>> accounting of what the agent is doing to the user. A user can freeze the
>> filesystem, knowing that logging would be disabled, then prod at
>> whatever he wants. So the handling should be something specific to
>> fsfreeze, with stricter requirements:
>>
>> If a user calls fsfreeze(), we disable logging, but also disable the
>> ability to do anything other than fsthaw() or fsstatus(). This actually
>> solves the potential deadlocking problem for other RPCs as well...since
>> they cant be executed in the first place.
>
> I disagree that we should block all calls, except for fsfreeze/fsstatus/
> fsthaw in this case. There are other calls that could be valid in this
> situations, so I think it needs to be evaluated on a case by case basis.
>
>> So a solution for these situations is still needed, and I'm starting to
>> agree that threads are needed, but I don't think we should do RPCs
>> concurrently (not sure if that's what is being suggested or not). At
>> least, there's no pressing reason for it as things currently stand
>> (there aren't currently any RPCs where fast response times are all that
>> important, so it's okay to serialize them behind previous RPCs, and
>> HMP/QMP are command at a time), and it's something that Im fairly
>> confident can be added if the need arises in the future.
>
> Eventually I think we will need to be able to support concurrent RPC
> calls. There can be situations where an operation takes a long time
> while it is valid to be able to ping the guest agent to verify that it
> is still alive etc.

Currently we're limited to 1 RPC at a time by the monitor/hmp/qmp 
(except for some niche cases where virtagent has multiple requests in 
flight). Ideally this will remain the interface we expose to users, but 
there could be situations where this isn't the case...for instance, 
other bits of qemu making direct calls into virtagent. I think we can 
add support for concurrent RPCs incrementally though...I've thought over 
the implementation details a bit and it seems to be fairly 
straightforward. I think we need to get the basic use cases down first 
though.

>
> Cheers,
> Jes
>

[Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch adds the following monitor commands:

agent_fsfreeze:
 - Freezes all local file systems in the guest. Command will print
   the number of file systems that were frozen.
agent_fsthaw:
 - Thaws all local file systems in the guest. Command will print
   the number of file systems that were thawed.
agent_fsstatus:
 - Prints the current status of file systems in the guest:
   Thawed, frozen, thaw in progress, freeze in progress, error.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 hmp-commands.hx    |   48 +++++++++++
 virtagent-common.h |    1 +
 virtagent.c        |  235 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtagent.h        |    9 ++
 4 files changed, 293 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9c7ac0b..f4150da 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1310,6 +1310,54 @@ STEXI
 Fetch and re-negotiate guest agent capabilties
 ETEXI
 
+    {
+        .name       = "agent_fsfreeze",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Freeze all local file systems mounted in the guest",
+        .user_print = do_agent_fsfreeze_print,
+        .mhandler.cmd_async = do_agent_fsfreeze,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsfreeze
+@findex agent_fsfreeze
+Freeze all local mounted file systems in guest
+ETEXI
+
+    {
+        .name       = "agent_fsthaw",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Thaw all local file systems mounted in the guest",
+        .user_print = do_agent_fsthaw_print,
+        .mhandler.cmd_async = do_agent_fsthaw,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsthaw
+@findex agent_fsthaw
+Thaw all local mounted file systems in guest
+ETEXI
+
+    {
+        .name       = "agent_fsstatus",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Display status of file system freeze progress in guest",
+        .user_print = do_agent_fsstatus_print,
+        .mhandler.cmd_async = do_agent_fsstatus,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsstatus
+@findex agent_fsstatus
+Get status of file system freeze in guest
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/virtagent-common.h b/virtagent-common.h
index 220a4b6..383de84 100644
--- a/virtagent-common.h
+++ b/virtagent-common.h
@@ -24,6 +24,7 @@
 #include "monitor.h"
 #include "virtagent-server.h"
 #include "virtagent.h"
+#include "qint.h"
 
 #define DEBUG_VA
 
diff --git a/virtagent.c b/virtagent.c
index b5e7944..4277802 100644
--- a/virtagent.c
+++ b/virtagent.c
@@ -640,3 +640,238 @@ int va_send_hello(void)
     xmlrpc_DECREF(params);
     return ret;
 }
+
+void do_agent_fsfreeze_print(Monitor *mon, const QObject *data)
+{
+    TRACE("called");
+
+    monitor_printf(mon, "File systems frozen: %" PRId64 "\n",
+                   qint_get_int((qobject_to_qint(data))));
+}
+
+static void do_agent_fsfreeze_cb(const char *resp_data,
+                                 size_t resp_data_len,
+                                 MonitorCompletion *mon_cb,
+                                 void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsfreeze", params, do_agent_fsfreeze_cb,
+                    cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
+
+void do_agent_fsthaw_print(Monitor *mon, const QObject *data)
+{
+    TRACE("called");
+
+    monitor_printf(mon, "File systems thawed: %" PRId64 "\n",
+                   qint_get_int((qobject_to_qint(data))));
+}
+
+static void do_agent_fsthaw_cb(const char *resp_data,
+                               size_t resp_data_len,
+                               MonitorCompletion *mon_cb,
+                               void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsthaw(Monitor *mon, const QDict *mon_params,
+                    MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsthaw", params, do_agent_fsthaw_cb, cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
+
+void do_agent_fsstatus_print(Monitor *mon, const QObject *data)
+{
+    int64_t fsstatus;
+    TRACE("called");
+
+    fsstatus = qint_get_int((qobject_to_qint(data)));
+
+    monitor_printf(mon, "File systems freeze status: ");
+    switch(fsstatus) {
+    case FREEZE_THAWED:
+        monitor_printf(mon, "Thawed");
+        break;
+    case FREEZE_INPROGRESS:
+        monitor_printf(mon, "Freeze in progress");
+        break;
+    case FREEZE_THAWINPROGRESS:
+        monitor_printf(mon, "Thaw in progress");
+        break;
+    case FREEZE_FROZEN:
+        monitor_printf(mon, "Frozen");
+        break;
+    case FREEZE_ERROR:
+        monitor_printf(mon, "Error");
+        break;
+    default:
+        monitor_printf(mon, "unknown");
+    }
+
+    monitor_printf(mon, "\n");
+}
+
+static void do_agent_fsstatus_cb(const char *resp_data,
+                                 size_t resp_data_len,
+                                 MonitorCompletion *mon_cb,
+                                 void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsstatus(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsstatus", params, do_agent_fsstatus_cb,
+                    cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
diff --git a/virtagent.h b/virtagent.h
index dba90d0..0d7575d 100644
--- a/virtagent.h
+++ b/virtagent.h
@@ -46,5 +46,14 @@ int do_agent_capabilities(Monitor *mon, const QDict *mon_params,
                   MonitorCompletion cb, void *opaque);
 int va_client_init_capabilities(void);
 int va_send_hello(void);
+void do_agent_fsfreeze_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque);
+void do_agent_fsthaw_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsthaw(Monitor *mon, const QDict *mon_params,
+                    MonitorCompletion cb, void *opaque);
+void do_agent_fsstatus_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsstatus(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque);
 
 #endif /* VIRTAGENT_H */
-- 
1.7.3.5

Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Vasiliy G Tolstov]

On Tue, 2011-02-01 at 11:58 +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Hi
> 
> This is a first attempt to add fsfreeze support to virtagent. The idea
> is for the guest agent to walk the list of locally mounted file
> systems in the guest, and issuing an ioctl to freeze them. The host
> can then do a live snapshot of the guest, obtaining stable file
> systems. After the snapshot, the host then calls the thaw function in
> virtagent, which goes through the list of previously frozen file
> systems and unfreezes them.
> 
> The list walking ignores remote file systems such as NFS and CIFS as
> well as all pseudo file systems.
> 
> The guest agent code is in the first patch, and host agent code is in
> the second patch. For now there is only human monitor support, but it
> should be pretty straight forward to add QMP support as well.
> 
> Patches are against the virtagent-dev git tree.
> 
> Comments and suggestions welcome!
> 
> Cheers,
> Jes

Hello. Very nice feature. Sorry for offropic, but can this feature can
be used to modify partiotion table on already mounted device (for
example root on ext3? )
Thank You.

-- 
Vasiliy G Tolstov <v.tolstov@selfip.ru>
Selfip.Ru

Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Jes Sorensen]

On 02/01/11 12:25, Vasiliy G Tolstov wrote:
> Hello. Very nice feature. Sorry for offropic, but can this feature can
> be used to modify partiotion table on already mounted device (for
> example root on ext3? )
> Thank You.
> 

No it cannot, that would be a totally different issue that wouldn't be
affected by this.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Richard W.M. Jones]

On Tue, Feb 01, 2011 at 02:25:12PM +0300, Vasiliy G Tolstov wrote:
> On Tue, 2011-02-01 at 11:58 +0100, Jes.Sorensen@redhat.com wrote:
> > From: Jes Sorensen <Jes.Sorensen@redhat.com>
> > 
> > Hi
> > 
> > This is a first attempt to add fsfreeze support to virtagent. The idea
> > is for the guest agent to walk the list of locally mounted file
> > systems in the guest, and issuing an ioctl to freeze them. The host
> > can then do a live snapshot of the guest, obtaining stable file
> > systems. After the snapshot, the host then calls the thaw function in
> > virtagent, which goes through the list of previously frozen file
> > systems and unfreezes them.
> > 
> > The list walking ignores remote file systems such as NFS and CIFS as
> > well as all pseudo file systems.
> > 
> > The guest agent code is in the first patch, and host agent code is in
> > the second patch. For now there is only human monitor support, but it
> > should be pretty straight forward to add QMP support as well.
> > 
> > Patches are against the virtagent-dev git tree.
> > 
> > Comments and suggestions welcome!
> > 
> > Cheers,
> > Jes
> 
> Hello. Very nice feature. Sorry for offropic, but can this feature can
> be used to modify partiotion table on already mounted device (for
> example root on ext3? )

There are some experimental patches to libguestfs to do live
filesystem and partition manipulations now:

  https://www.redhat.com/archives/libguestfs/2011-January/msg00096.html

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Vasiliy G Tolstov]

On Tue, 2011-02-01 at 16:04 +0000, Richard W.M. Jones wrote:

> There are some experimental patches to libguestfs to do live
> filesystem and partition manipulations now:
> 
>   https://www.redhat.com/archives/libguestfs/2011-January/msg00096.html
> 
> Rich.
> 

Sorry, but i can't found any info about operating on already mounted
disk. As i known, libguestfs allows operating inside virtual machine via
userspace daemon. But in my case this works inside kernel module. But if
fs is  using it partition table not able to altered because bdev is
busy. Fdisk allows to change partition table, but says, that kernel must
be rebooted to see any changes. I think, that if i can after fdisk
change inside kernel some info about partitions i can do online resizing
fs (ext3). But i see, that partitions is only be operated by ioctl....

If some body says me what functions inside kernel may be exported (says
export_symbol_gpl) to change part info, i'm very glad =)

Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Richard W.M. Jones]

On Tue, Feb 01, 2011 at 11:04:47PM +0300, Vasiliy G Tolstov wrote:
> On Tue, 2011-02-01 at 16:04 +0000, Richard W.M. Jones wrote:
> 
> > There are some experimental patches to libguestfs to do live
> > filesystem and partition manipulations now:
> > 
> >   https://www.redhat.com/archives/libguestfs/2011-January/msg00096.html
> > 
> > Rich.
> > 
> 
> Sorry, but i can't found any info about operating on already mounted
> disk. As i known, libguestfs allows operating inside virtual machine via
> userspace daemon.

Yes you're right that 'guestfsd' running in the guest cannot change
the partition table seen by the kernel when partitions are mounted as
filesystems.  This would require some changes to the kernel.

I was just pointing out you can use libguestfs "live" now and do a
huge range of filesystem operations, including changing the partition
table on disk.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Stefan Hajnoczi]

On Tue, Feb 1, 2011 at 10:58 AM,  <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> This is a first attempt to add fsfreeze support to virtagent. The idea
> is for the guest agent to walk the list of locally mounted file
> systems in the guest, and issuing an ioctl to freeze them. The host
> can then do a live snapshot of the guest, obtaining stable file
> systems. After the snapshot, the host then calls the thaw function in
> virtagent, which goes through the list of previously frozen file
> systems and unfreezes them.

Any plans for a call-out to pre/post freeze and thaw scripts so that
applications can flush cached data to disk and brace themselves for
freeze?

Stefan

Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support

[Jes Sorensen]

On 02/01/11 15:16, Stefan Hajnoczi wrote:
> On Tue, Feb 1, 2011 at 10:58 AM,  <Jes.Sorensen@redhat.com> wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> This is a first attempt to add fsfreeze support to virtagent. The idea
>> is for the guest agent to walk the list of locally mounted file
>> systems in the guest, and issuing an ioctl to freeze them. The host
>> can then do a live snapshot of the guest, obtaining stable file
>> systems. After the snapshot, the host then calls the thaw function in
>> virtagent, which goes through the list of previously frozen file
>> systems and unfreezes them.
> 
> Any plans for a call-out to pre/post freeze and thaw scripts so that
> applications can flush cached data to disk and brace themselves for
> freeze?

Michael and I were discussing this earlier, we need to add it somehow.
It could be done as call-outs from the freeze call, or from separate
agent calls.

Cheers,
Jes

[Qemu-devel] [PATCH v2 0/2] virtagent - fsfreeze support

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi

This is a first attempt to add fsfreeze support to virtagent. The idea
is for the guest agent to walk the list of locally mounted file
systems in the guest, and issuing an ioctl to freeze them. The host
can then do a live snapshot of the guest, obtaining stable file
systems. After the snapshot, the host then calls the thaw function in
virtagent, which goes through the list of previously frozen file
systems and unfreezes them.

The list walking ignores remote file systems such as NFS and CIFS as
well as all pseudo file systems.

The guest agent code is in the first patch, and host agent code is in
the second patch. For now there is only human monitor support, but it
should be pretty straight forward to add QMP support as well.

Comments and suggestions welcome!

v2 of the patch addresses the issues pointed out by Stefan and Michael.

Cheers,
Jes


Jes Sorensen (2):
  Add virtagent file system freeze/thaw
  Add monitor commands for fsfreeze support

 hmp-commands.hx    |   48 +++++++++++
 virtagent-common.h |    9 ++
 virtagent-server.c |  190 ++++++++++++++++++++++++++++++++++++++++++
 virtagent.c        |  235 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtagent.h        |    9 ++
 5 files changed, 491 insertions(+), 0 deletions(-)

-- 
1.7.3.5

[Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch adds the following monitor commands:

agent_fsfreeze:
 - Freezes all local file systems in the guest. Command will print
   the number of file systems that were frozen.
agent_fsthaw:
 - Thaws all local file systems in the guest. Command will print
   the number of file systems that were thawed.
agent_fsstatus:
 - Prints the current status of file systems in the guest:
   Thawed, frozen, thaw in progress, freeze in progress, error.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 hmp-commands.hx    |   48 +++++++++++
 virtagent-common.h |    1 +
 virtagent.c        |  235 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtagent.h        |    9 ++
 4 files changed, 293 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9c7ac0b..f4150da 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1310,6 +1310,54 @@ STEXI
 Fetch and re-negotiate guest agent capabilties
 ETEXI
 
+    {
+        .name       = "agent_fsfreeze",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Freeze all local file systems mounted in the guest",
+        .user_print = do_agent_fsfreeze_print,
+        .mhandler.cmd_async = do_agent_fsfreeze,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsfreeze
+@findex agent_fsfreeze
+Freeze all local mounted file systems in guest
+ETEXI
+
+    {
+        .name       = "agent_fsthaw",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Thaw all local file systems mounted in the guest",
+        .user_print = do_agent_fsthaw_print,
+        .mhandler.cmd_async = do_agent_fsthaw,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsthaw
+@findex agent_fsthaw
+Thaw all local mounted file systems in guest
+ETEXI
+
+    {
+        .name       = "agent_fsstatus",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Display status of file system freeze progress in guest",
+        .user_print = do_agent_fsstatus_print,
+        .mhandler.cmd_async = do_agent_fsstatus,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsstatus
+@findex agent_fsstatus
+Get status of file system freeze in guest
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/virtagent-common.h b/virtagent-common.h
index 7c6d9ef..ff7bf23 100644
--- a/virtagent-common.h
+++ b/virtagent-common.h
@@ -24,6 +24,7 @@
 #include "monitor.h"
 #include "virtagent-server.h"
 #include "virtagent.h"
+#include "qint.h"
 
 #define DEBUG_VA
 
diff --git a/virtagent.c b/virtagent.c
index b5e7944..4277802 100644
--- a/virtagent.c
+++ b/virtagent.c
@@ -640,3 +640,238 @@ int va_send_hello(void)
     xmlrpc_DECREF(params);
     return ret;
 }
+
+void do_agent_fsfreeze_print(Monitor *mon, const QObject *data)
+{
+    TRACE("called");
+
+    monitor_printf(mon, "File systems frozen: %" PRId64 "\n",
+                   qint_get_int((qobject_to_qint(data))));
+}
+
+static void do_agent_fsfreeze_cb(const char *resp_data,
+                                 size_t resp_data_len,
+                                 MonitorCompletion *mon_cb,
+                                 void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsfreeze", params, do_agent_fsfreeze_cb,
+                    cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
+
+void do_agent_fsthaw_print(Monitor *mon, const QObject *data)
+{
+    TRACE("called");
+
+    monitor_printf(mon, "File systems thawed: %" PRId64 "\n",
+                   qint_get_int((qobject_to_qint(data))));
+}
+
+static void do_agent_fsthaw_cb(const char *resp_data,
+                               size_t resp_data_len,
+                               MonitorCompletion *mon_cb,
+                               void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsthaw(Monitor *mon, const QDict *mon_params,
+                    MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsthaw", params, do_agent_fsthaw_cb, cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
+
+void do_agent_fsstatus_print(Monitor *mon, const QObject *data)
+{
+    int64_t fsstatus;
+    TRACE("called");
+
+    fsstatus = qint_get_int((qobject_to_qint(data)));
+
+    monitor_printf(mon, "File systems freeze status: ");
+    switch(fsstatus) {
+    case FREEZE_THAWED:
+        monitor_printf(mon, "Thawed");
+        break;
+    case FREEZE_INPROGRESS:
+        monitor_printf(mon, "Freeze in progress");
+        break;
+    case FREEZE_THAWINPROGRESS:
+        monitor_printf(mon, "Thaw in progress");
+        break;
+    case FREEZE_FROZEN:
+        monitor_printf(mon, "Frozen");
+        break;
+    case FREEZE_ERROR:
+        monitor_printf(mon, "Error");
+        break;
+    default:
+        monitor_printf(mon, "unknown");
+    }
+
+    monitor_printf(mon, "\n");
+}
+
+static void do_agent_fsstatus_cb(const char *resp_data,
+                                 size_t resp_data_len,
+                                 MonitorCompletion *mon_cb,
+                                 void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsstatus(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsstatus", params, do_agent_fsstatus_cb,
+                    cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
diff --git a/virtagent.h b/virtagent.h
index dba90d0..0d7575d 100644
--- a/virtagent.h
+++ b/virtagent.h
@@ -46,5 +46,14 @@ int do_agent_capabilities(Monitor *mon, const QDict *mon_params,
                   MonitorCompletion cb, void *opaque);
 int va_client_init_capabilities(void);
 int va_send_hello(void);
+void do_agent_fsfreeze_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque);
+void do_agent_fsthaw_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsthaw(Monitor *mon, const QDict *mon_params,
+                    MonitorCompletion cb, void *opaque);
+void do_agent_fsstatus_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsstatus(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque);
 
 #endif /* VIRTAGENT_H */
-- 
1.7.3.5

[Qemu-devel] [PATCH v3 0/2] virtagent - fsfreeze support

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi

This is a first attempt to add fsfreeze support to virtagent. The idea
is for the guest agent to walk the list of locally mounted file
systems in the guest, and issuing an ioctl to freeze them. The host
can then do a live snapshot of the guest, obtaining stable file
systems. After the snapshot, the host then calls the thaw function in
virtagent, which goes through the list of previously frozen file
systems and unfreezes them.

The list walking ignores remote file systems such as NFS and CIFS as
well as all pseudo file systems.

The guest agent code is in the first patch, and host agent code is in
the second patch. For now there is only human monitor support, but it
should be pretty straight forward to add QMP support as well.

Comments and suggestions welcome!

v3 of the patch encapsulates the freeze states in a struct and fixes
some tab issues that I had missed. Both pointed out by Michael Roth.

v2 of the patch addresses the issues pointed out by Stefan and Michael.

Note I will be gone all of next week, so if I don't reply it's because
I am busy skiing :)

Cheers,
Jes

Jes Sorensen (2):
  Add virtagent file system freeze/thaw
  Add monitor commands for fsfreeze support

 hmp-commands.hx    |   48 +++++++++++
 virtagent-common.h |    9 ++
 virtagent-server.c |  195 +++++++++++++++++++++++++++++++++++++++++++
 virtagent.c        |  235 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtagent.h        |    9 ++
 5 files changed, 496 insertions(+), 0 deletions(-)

-- 
1.7.3.5

[Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch adds the following monitor commands:

agent_fsfreeze:
 - Freezes all local file systems in the guest. Command will print
   the number of file systems that were frozen.
agent_fsthaw:
 - Thaws all local file systems in the guest. Command will print
   the number of file systems that were thawed.
agent_fsstatus:
 - Prints the current status of file systems in the guest:
   Thawed, frozen, thaw in progress, freeze in progress, error.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 hmp-commands.hx    |   48 +++++++++++
 virtagent-common.h |    1 +
 virtagent.c        |  235 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtagent.h        |    9 ++
 4 files changed, 293 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9c7ac0b..f4150da 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1310,6 +1310,54 @@ STEXI
 Fetch and re-negotiate guest agent capabilties
 ETEXI
 
+    {
+        .name       = "agent_fsfreeze",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Freeze all local file systems mounted in the guest",
+        .user_print = do_agent_fsfreeze_print,
+        .mhandler.cmd_async = do_agent_fsfreeze,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsfreeze
+@findex agent_fsfreeze
+Freeze all local mounted file systems in guest
+ETEXI
+
+    {
+        .name       = "agent_fsthaw",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Thaw all local file systems mounted in the guest",
+        .user_print = do_agent_fsthaw_print,
+        .mhandler.cmd_async = do_agent_fsthaw,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsthaw
+@findex agent_fsthaw
+Thaw all local mounted file systems in guest
+ETEXI
+
+    {
+        .name       = "agent_fsstatus",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Display status of file system freeze progress in guest",
+        .user_print = do_agent_fsstatus_print,
+        .mhandler.cmd_async = do_agent_fsstatus,
+        .flags      = MONITOR_CMD_ASYNC,
+    },
+
+STEXI
+@item agent_fsstatus
+@findex agent_fsstatus
+Get status of file system freeze in guest
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/virtagent-common.h b/virtagent-common.h
index 7c6d9ef..ff7bf23 100644
--- a/virtagent-common.h
+++ b/virtagent-common.h
@@ -24,6 +24,7 @@
 #include "monitor.h"
 #include "virtagent-server.h"
 #include "virtagent.h"
+#include "qint.h"
 
 #define DEBUG_VA
 
diff --git a/virtagent.c b/virtagent.c
index b5e7944..4277802 100644
--- a/virtagent.c
+++ b/virtagent.c
@@ -640,3 +640,238 @@ int va_send_hello(void)
     xmlrpc_DECREF(params);
     return ret;
 }
+
+void do_agent_fsfreeze_print(Monitor *mon, const QObject *data)
+{
+    TRACE("called");
+
+    monitor_printf(mon, "File systems frozen: %" PRId64 "\n",
+                   qint_get_int((qobject_to_qint(data))));
+}
+
+static void do_agent_fsfreeze_cb(const char *resp_data,
+                                 size_t resp_data_len,
+                                 MonitorCompletion *mon_cb,
+                                 void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsfreeze", params, do_agent_fsfreeze_cb,
+                    cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
+
+void do_agent_fsthaw_print(Monitor *mon, const QObject *data)
+{
+    TRACE("called");
+
+    monitor_printf(mon, "File systems thawed: %" PRId64 "\n",
+                   qint_get_int((qobject_to_qint(data))));
+}
+
+static void do_agent_fsthaw_cb(const char *resp_data,
+                               size_t resp_data_len,
+                               MonitorCompletion *mon_cb,
+                               void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsthaw(Monitor *mon, const QDict *mon_params,
+                    MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsthaw", params, do_agent_fsthaw_cb, cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
+
+void do_agent_fsstatus_print(Monitor *mon, const QObject *data)
+{
+    int64_t fsstatus;
+    TRACE("called");
+
+    fsstatus = qint_get_int((qobject_to_qint(data)));
+
+    monitor_printf(mon, "File systems freeze status: ");
+    switch(fsstatus) {
+    case FREEZE_THAWED:
+        monitor_printf(mon, "Thawed");
+        break;
+    case FREEZE_INPROGRESS:
+        monitor_printf(mon, "Freeze in progress");
+        break;
+    case FREEZE_THAWINPROGRESS:
+        monitor_printf(mon, "Thaw in progress");
+        break;
+    case FREEZE_FROZEN:
+        monitor_printf(mon, "Frozen");
+        break;
+    case FREEZE_ERROR:
+        monitor_printf(mon, "Error");
+        break;
+    default:
+        monitor_printf(mon, "unknown");
+    }
+
+    monitor_printf(mon, "\n");
+}
+
+static void do_agent_fsstatus_cb(const char *resp_data,
+                                 size_t resp_data_len,
+                                 MonitorCompletion *mon_cb,
+                                 void *mon_data)
+{
+    xmlrpc_value *resp = NULL;
+    xmlrpc_env env;
+    xmlrpc_int32 retval = 0;
+    QInt *qint;
+
+    TRACE("called");
+
+    if (resp_data == NULL) {
+        LOG("error handling RPC request");
+        return;
+    }
+
+    xmlrpc_env_init(&env);
+    resp = xmlrpc_parse_response(&env, resp_data, resp_data_len);
+    if (va_rpc_has_error(&env)) {
+        LOG("error parsing RPC response");
+        return;
+    }
+
+    xmlrpc_parse_value(&env, resp, "i", &retval);
+    if (va_rpc_has_error(&env)) {
+        retval = -1;
+        goto out;
+    }
+
+out:
+    qint = qint_from_int(retval);
+    xmlrpc_DECREF(resp);
+    if (mon_cb) {
+        mon_cb(mon_data, QOBJECT(qint));
+    }
+    qobject_decref(QOBJECT(qint));
+}
+
+int do_agent_fsstatus(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque)
+{
+    xmlrpc_env env;
+    xmlrpc_value *params;
+    int ret;
+
+    TRACE("called");
+
+    xmlrpc_env_init(&env);
+    params = xmlrpc_build_value(&env, "()");
+    if (va_rpc_has_error(&env)) {
+        return -1;
+    }
+
+    ret = va_do_rpc(&env, "va.fsstatus", params, do_agent_fsstatus_cb,
+                    cb, opaque);
+    if (ret) {
+        qerror_report(QERR_VA_FAILED, ret, strerror(ret));
+    }
+    xmlrpc_DECREF(params);
+    return ret;
+}
diff --git a/virtagent.h b/virtagent.h
index dba90d0..0d7575d 100644
--- a/virtagent.h
+++ b/virtagent.h
@@ -46,5 +46,14 @@ int do_agent_capabilities(Monitor *mon, const QDict *mon_params,
                   MonitorCompletion cb, void *opaque);
 int va_client_init_capabilities(void);
 int va_send_hello(void);
+void do_agent_fsfreeze_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque);
+void do_agent_fsthaw_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsthaw(Monitor *mon, const QDict *mon_params,
+                    MonitorCompletion cb, void *opaque);
+void do_agent_fsstatus_print(Monitor *mon, const QObject *qobject);
+int do_agent_fsstatus(Monitor *mon, const QDict *mon_params,
+                      MonitorCompletion cb, void *opaque);
 
 #endif /* VIRTAGENT_H */
-- 
1.7.3.5