[Qemu-devel] [PATCH] qemu-ga: execute fsfreeze-freeze in reverse order of mounts

[Qemu-devel] [PATCH] qemu-ga: execute fsfreeze-freeze in reverse order of mounts

[Tomoki Sekiyama]

Currently, fsfreeze-freeze may cause deadlock if a guest has loopback mounts
of image files in its disk; e.g.:

    # mount | grep ^/
    /dev/vda1 / type ext4 (rw,noatime,seclabel,data=ordered)
    /tmp/disk.img on /mnt type ext4 (rw,relatime,seclabel)

To avoid the deadlock, this freeze filesystems in reverse order of mounts.

Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
---
 qga/commands-posix.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index e199738..f453132 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -566,7 +566,7 @@ typedef struct FsMount {
     QTAILQ_ENTRY(FsMount) next;
 } FsMount;
 
-typedef QTAILQ_HEAD(, FsMount) FsMountList;
+typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
 
 static void free_fs_mount_list(FsMountList *mounts)
 {
@@ -728,7 +728,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err)
     /* cannot risk guest agent blocking itself on a write in this state */
     ga_set_frozen(ga_state);
 
-    QTAILQ_FOREACH(mount, &mounts, next) {
+    QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, next) {
         fd = qemu_open(mount->dirname, O_RDONLY);
         if (fd == -1) {
             error_setg_errno(err, errno, "failed to open %s", mount->dirname);

Re: [Qemu-devel] [PATCH] qemu-ga: execute fsfreeze-freeze in reverse order of mounts

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

On 10/01/2013 03:09 PM, Tomoki Sekiyama wrote:
> Currently, fsfreeze-freeze may cause deadlock if a guest has loopback mounts
> of image files in its disk; e.g.:
> 
>     # mount | grep ^/
>     /dev/vda1 / type ext4 (rw,noatime,seclabel,data=ordered)
>     /tmp/disk.img on /mnt type ext4 (rw,relatime,seclabel)
> 
> To avoid the deadlock, this freeze filesystems in reverse order of mounts.

s/freeze/freezes/

> 
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
> ---
>  qga/commands-posix.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Thaw already worked in the forward direction; with your patch, this
means thaw undoes the actions by freeze in the opposite order (last
frozen is first thawed), which is also a good thing.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] qemu-ga: execute fsfreeze-freeze in reverse order of mounts

[Michael Roth]

Quoting Tomoki Sekiyama (2013-10-01 16:09:53)
> Currently, fsfreeze-freeze may cause deadlock if a guest has loopback mounts
> of image files in its disk; e.g.:
> 
>     # mount | grep ^/
>     /dev/vda1 / type ext4 (rw,noatime,seclabel,data=ordered)
>     /tmp/disk.img on /mnt type ext4 (rw,relatime,seclabel)
> 
> To avoid the deadlock, this freeze filesystems in reverse order of mounts.
> 
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>

Thanks, applied to qga tree:

https://github.com/mdroth/qemu/commits/qga

> ---
>  qga/commands-posix.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index e199738..f453132 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -566,7 +566,7 @@ typedef struct FsMount {
>      QTAILQ_ENTRY(FsMount) next;
>  } FsMount;
> 
> -typedef QTAILQ_HEAD(, FsMount) FsMountList;
> +typedef QTAILQ_HEAD(FsMountList, FsMount) FsMountList;
> 
>  static void free_fs_mount_list(FsMountList *mounts)
>  {
> @@ -728,7 +728,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err)
>      /* cannot risk guest agent blocking itself on a write in this state */
>      ga_set_frozen(ga_state);
> 
> -    QTAILQ_FOREACH(mount, &mounts, next) {
> +    QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, next) {
>          fd = qemu_open(mount->dirname, O_RDONLY);
>          if (fd == -1) {
>              error_setg_errno(err, errno, "failed to open %s", mount->dirname);