diff mbox

[2/3] qga: vss-win32: Fix interference with snapshot creation by other VSS requesters

Message ID 20140113172529.11163.32538.stgit@helix
State New
Headers show

Commit Message

Tomoki Sekiyama Jan. 13, 2014, 5:25 p.m. UTC
When a VSS requester such as vshadow.exe or diskshadow.exe requests to
create disk snapshots, Windows may choose qemu-ga VSS provider if it is
only provider registered on the system. However, because it provides only a
function to freeze the filesystem, the snapshotting fails.

This patch adds a check into CQGAVssProvider::IsVolumeSupported() to reject
the request from other VSS requesters, so that the other provider is chosen.

The check of requester is done by confirming event channels between
qemu-ga's requester and provider established. To ensure that the events are
initialized when CQGAVssProvider::IsVolumeSupported() is called, it moves
the initialization earlier.

Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
---
 qga/vss-win32/provider.cpp  |   11 ++++++++-
 qga/vss-win32/requester.cpp |   52 ++++++++++++++++++++++---------------------
 2 files changed, 36 insertions(+), 27 deletions(-)

Comments

Gal Hammer Jan. 14, 2014, 8:33 a.m. UTC | #1
On 13/01/2014 19:25, Tomoki Sekiyama wrote:
> When a VSS requester such as vshadow.exe or diskshadow.exe requests to
> create disk snapshots, Windows may choose qemu-ga VSS provider if it is
> only provider registered on the system. However, because it provides only a
> function to freeze the filesystem, the snapshotting fails.
>
> This patch adds a check into CQGAVssProvider::IsVolumeSupported() to reject
> the request from other VSS requesters, so that the other provider is chosen.
>
> The check of requester is done by confirming event channels between
> qemu-ga's requester and provider established. To ensure that the events are
> initialized when CQGAVssProvider::IsVolumeSupported() is called, it moves
> the initialization earlier.
>
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>

Reviewed-by: Gal Hammer <ghammer@redhat.com>

     Gal.
Yan Vugenfirer Jan. 19, 2014, 10:21 a.m. UTC | #2
On Jan 13, 2014, at 7:25 PM, Tomoki Sekiyama <tomoki.sekiyama@hds.com> wrote:

> When a VSS requester such as vshadow.exe or diskshadow.exe requests to
> create disk snapshots, Windows may choose qemu-ga VSS provider if it is
> only provider registered on the system. However, because it provides only a
> function to freeze the filesystem, the snapshotting fails.
> 
> This patch adds a check into CQGAVssProvider::IsVolumeSupported() to reject
> the request from other VSS requesters, so that the other provider is chosen.
> 
> The check of requester is done by confirming event channels between
> qemu-ga's requester and provider established. To ensure that the events are
> initialized when CQGAVssProvider::IsVolumeSupported() is called, it moves
> the initialization earlier.
> 
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
> ---
> qga/vss-win32/provider.cpp  |   11 ++++++++-
> qga/vss-win32/requester.cpp |   52 ++++++++++++++++++++++---------------------
> 2 files changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
> index c3030d8..b233646 100644
> --- a/qga/vss-win32/provider.cpp
> +++ b/qga/vss-win32/provider.cpp
> @@ -291,8 +291,17 @@ STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot(
> STDMETHODIMP CQGAVssProvider::IsVolumeSupported(
>     VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider)
> {
> -    *pbSupportedByThisProvider = TRUE;
> +    HANDLE hEventFrozen;
> +
> +    /* Check if a requester is qemu-ga by whether an event is created */
> +    hEventFrozen = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_FROZEN);
> +    if (!hEventFrozen) {
> +        *pbSupportedByThisProvider = FALSE;
> +        return S_OK;
> +    }
> +    CloseHandle(hEventFrozen);
> 
> +    *pbSupportedByThisProvider = TRUE;
>     return S_OK;
> }
> 
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 0a55447..922e74d 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -252,6 +252,32 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
> 
>     CoInitialize(NULL);
> 
> +    /* Allow unrestricted access to events */
> +    InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION);
> +    SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE);
> +    sa.nLength = sizeof(sa);
> +    sa.lpSecurityDescriptor = &sd;
> +    sa.bInheritHandle = FALSE;
> +
> +    vss_ctx.hEventFrozen = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_FROZEN);
> +    if (!vss_ctx.hEventFrozen) {
> +        err_set(errset, GetLastError(), "failed to create event %s",
> +                EVENT_NAME_FROZEN);
> +        goto out;
> +    }
> +    vss_ctx.hEventThaw = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_THAW);
> +    if (!vss_ctx.hEventThaw) {
> +        err_set(errset, GetLastError(), "failed to create event %s",
> +                EVENT_NAME_THAW);
> +        goto out;
> +    }
> +    vss_ctx.hEventTimeout = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_TIMEOUT);
> +    if (!vss_ctx.hEventTimeout) {
> +        err_set(errset, GetLastError(), "failed to create event %s",
> +                EVENT_NAME_TIMEOUT);
> +        goto out;
> +    }
> +
>     assert(pCreateVssBackupComponents != NULL);
>     hr = pCreateVssBackupComponents(&vss_ctx.pVssbc);
>     if (FAILED(hr)) {
> @@ -362,32 +388,6 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
>         goto out;
>     }
> 
> -    /* Allow unrestricted access to events */
> -    InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION);
> -    SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE);
> -    sa.nLength = sizeof(sa);
> -    sa.lpSecurityDescriptor = &sd;
> -    sa.bInheritHandle = FALSE;
> -
> -    vss_ctx.hEventFrozen = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_FROZEN);
> -    if (!vss_ctx.hEventFrozen) {
> -        err_set(errset, GetLastError(), "failed to create event %s",
> -                EVENT_NAME_FROZEN);
> -        goto out;
> -    }
> -    vss_ctx.hEventThaw = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_THAW);
> -    if (!vss_ctx.hEventThaw) {
> -        err_set(errset, GetLastError(), "failed to create event %s",
> -                EVENT_NAME_THAW);
> -        goto out;
> -    }
> -    vss_ctx.hEventTimeout = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_TIMEOUT);
> -    if (!vss_ctx.hEventTimeout) {
> -        err_set(errset, GetLastError(), "failed to create event %s",
> -                EVENT_NAME_TIMEOUT);
> -        goto out;
> -    }
> -
>     /*
>      * Start VSS quiescing operations.
>      * CQGAVssProvider::CommitSnapshots will kick vss_ctx.hEventFrozen
> 
> 

Reviewed-by: Yan Vugenfirer <yvugenfi@redhat.com>
diff mbox

Patch

diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
index c3030d8..b233646 100644
--- a/qga/vss-win32/provider.cpp
+++ b/qga/vss-win32/provider.cpp
@@ -291,8 +291,17 @@  STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot(
 STDMETHODIMP CQGAVssProvider::IsVolumeSupported(
     VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider)
 {
-    *pbSupportedByThisProvider = TRUE;
+    HANDLE hEventFrozen;
+
+    /* Check if a requester is qemu-ga by whether an event is created */
+    hEventFrozen = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_FROZEN);
+    if (!hEventFrozen) {
+        *pbSupportedByThisProvider = FALSE;
+        return S_OK;
+    }
+    CloseHandle(hEventFrozen);
 
+    *pbSupportedByThisProvider = TRUE;
     return S_OK;
 }
 
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 0a55447..922e74d 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -252,6 +252,32 @@  void requester_freeze(int *num_vols, ErrorSet *errset)
 
     CoInitialize(NULL);
 
+    /* Allow unrestricted access to events */
+    InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION);
+    SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE);
+    sa.nLength = sizeof(sa);
+    sa.lpSecurityDescriptor = &sd;
+    sa.bInheritHandle = FALSE;
+
+    vss_ctx.hEventFrozen = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_FROZEN);
+    if (!vss_ctx.hEventFrozen) {
+        err_set(errset, GetLastError(), "failed to create event %s",
+                EVENT_NAME_FROZEN);
+        goto out;
+    }
+    vss_ctx.hEventThaw = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_THAW);
+    if (!vss_ctx.hEventThaw) {
+        err_set(errset, GetLastError(), "failed to create event %s",
+                EVENT_NAME_THAW);
+        goto out;
+    }
+    vss_ctx.hEventTimeout = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_TIMEOUT);
+    if (!vss_ctx.hEventTimeout) {
+        err_set(errset, GetLastError(), "failed to create event %s",
+                EVENT_NAME_TIMEOUT);
+        goto out;
+    }
+
     assert(pCreateVssBackupComponents != NULL);
     hr = pCreateVssBackupComponents(&vss_ctx.pVssbc);
     if (FAILED(hr)) {
@@ -362,32 +388,6 @@  void requester_freeze(int *num_vols, ErrorSet *errset)
         goto out;
     }
 
-    /* Allow unrestricted access to events */
-    InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION);
-    SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE);
-    sa.nLength = sizeof(sa);
-    sa.lpSecurityDescriptor = &sd;
-    sa.bInheritHandle = FALSE;
-
-    vss_ctx.hEventFrozen = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_FROZEN);
-    if (!vss_ctx.hEventFrozen) {
-        err_set(errset, GetLastError(), "failed to create event %s",
-                EVENT_NAME_FROZEN);
-        goto out;
-    }
-    vss_ctx.hEventThaw = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_THAW);
-    if (!vss_ctx.hEventThaw) {
-        err_set(errset, GetLastError(), "failed to create event %s",
-                EVENT_NAME_THAW);
-        goto out;
-    }
-    vss_ctx.hEventTimeout = CreateEvent(&sa, TRUE, FALSE, EVENT_NAME_TIMEOUT);
-    if (!vss_ctx.hEventTimeout) {
-        err_set(errset, GetLastError(), "failed to create event %s",
-                EVENT_NAME_TIMEOUT);
-        goto out;
-    }
-
     /*
      * Start VSS quiescing operations.
      * CQGAVssProvider::CommitSnapshots will kick vss_ctx.hEventFrozen