[Qemu-devel] [Resend][PATCH] qga-win: don't hang if vss hold writes timeout

Chen Hanxiao posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171017064142.4519-1-chen_han_xiao@126.com
Test checkpatch passed
Test docker passed
Test s390x passed
qga/vss-win32/requester.cpp | 12 ++++++++++++
1 file changed, 12 insertions(+)
[Qemu-devel] [Resend][PATCH] qga-win: don't hang if vss hold writes timeout
Posted by Chen Hanxiao 6 years, 6 months ago
From: Chen Hanxiao <chenhanxiao@gmail.com>

When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
is executed, VSS may timeout when trying to hold writes.

Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
is logged in the Event Viewer.

At that time, if we call AbortBackup, qga may hang forever.

This patch will solve this issue.

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>

Reviewed-by: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
---
 qga/vss-win32/requester.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 301762d8b1..3d9c9716c0 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -419,6 +419,16 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
             break;
         }
     }
+
+    if (wait_status == WAIT_TIMEOUT) {
+        err_set(errset, E_FAIL,
+                "timeout when try to receive Frozen event from VSS provider");
+        /* If we are here, VSS had timeout.
+         * Don't call AbortBackup, just return directly.
+         */
+        goto out1;
+    }
+
     if (wait_status != WAIT_OBJECT_0) {
         err_set(errset, E_FAIL,
                 "couldn't receive Frozen event from VSS provider");
@@ -432,6 +442,8 @@ out:
     if (vss_ctx.pVssbc) {
         vss_ctx.pVssbc->AbortBackup();
     }
+
+out1:
     requester_cleanup();
     CoUninitialize();
 }
-- 
2.13.5


Re: [Qemu-devel] [Resend][PATCH] qga-win: don't hang if vss hold writes timeout
Posted by Michael Roth 6 years, 5 months ago
Quoting Chen Hanxiao (2017-10-17 01:41:42)
> From: Chen Hanxiao <chenhanxiao@gmail.com>
> 
> When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
> is executed, VSS may timeout when trying to hold writes.
> 
> Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
> is logged in the Event Viewer.
> 
> At that time, if we call AbortBackup, qga may hang forever.
> 
> This patch will solve this issue.
> 
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Cc: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>
> 
> Reviewed-by: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>
> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>

Thanks, applied to QGA tree:

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

Unless I missed another thread though, I don't think Tomoki gave it
an actual Reviewed-by, just a "looks good", so I've removed it from
the patch. (A "looks good" is certainly helpful, particularly in this
case, but actual Signed-off-by/Tested-by/Reviewed-by tags should
only be added if they were explicitly provided by the person in
question since they imply some level of accountability if something
goes wrong. Less formal tags like Suggested-by/Reported-by/etc are
generally okay though).

> ---
>  qga/vss-win32/requester.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 301762d8b1..3d9c9716c0 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -419,6 +419,16 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
>              break;
>          }
>      }
> +
> +    if (wait_status == WAIT_TIMEOUT) {
> +        err_set(errset, E_FAIL,
> +                "timeout when try to receive Frozen event from VSS provider");
> +        /* If we are here, VSS had timeout.
> +         * Don't call AbortBackup, just return directly.
> +         */
> +        goto out1;
> +    }
> +
>      if (wait_status != WAIT_OBJECT_0) {
>          err_set(errset, E_FAIL,
>                  "couldn't receive Frozen event from VSS provider");
> @@ -432,6 +442,8 @@ out:
>      if (vss_ctx.pVssbc) {
>          vss_ctx.pVssbc->AbortBackup();
>      }
> +
> +out1:
>      requester_cleanup();
>      CoUninitialize();
>  }
> -- 
> 2.13.5
>