qga/vss-win32/install.cpp | 1 + qga/vss-win32/provider.cpp | 200 +++++++++++++++++++++++++++++++++++- qga/vss-win32/requester.cpp | 26 +++++ 3 files changed, 226 insertions(+), 1 deletion(-)
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Since commit 374f4676788c, the provider's GetSnapshotProperties()
returns S_OK for any snapshot ID without filling in the properties. VSS
consults every registered provider when a requester looks up a snapshot,
so lookups of snapshots owned by other providers now succeed with an
all-zero VSS_SNAPSHOT_PROP. Backup software relying on it fails, e.g.
Veeam: "Original volume name is not specified for the shadow (shadow
volume id: '{00000000-0000-0000-0000-000000000000}')".
Simply reverting is not an option: 374f4676788c switched to S_OK because
returning VSS_E_OBJECT_NOT_FOUND unconditionally broke qemu-ga's own
snapshot sets. The provider must answer for its own snapshots only. It
does not create real shadow copies, but it does take part in the
snapshot set during qemu-ga fsfreeze cycles. Remember the snapshot IDs
received in BeginPrepareSnapshot(), return their required properties,
and return VSS_E_OBJECT_NOT_FOUND for anything else. Fill every required
VSS_SNAPSHOT_PROP member except the exposed name and path, which are
optional. The snapshot device object is an empty string because the
host-side snapshot has no guest-visible device.
Entries are dropped when VSS deletes the snapshots or the set, or when
the set is aborted (AbortSnapshots()).
COM+ may hand out several CQGAVssProvider instances (and serve several
client processes), so this registry is module-global. It is per-process
by design and safe because the COM+ application is hosted by a single NT
service (see CreateServiceForApplication() in install.cpp): COM+ forbids
pooling and recycling for service-hosted applications, so exactly one
surrogate process ever hosts the provider.
On the requester side, the snapshot set is created in the persistent
VSS_CTX_APP_ROLLBACK context, so VSS keeps it after the freeze/thaw
cycle and may keep querying providers about it. The provider registry
does not outlive the cycle (requester_thaw() stops the hosting service,
and the provider process with it), so later queries would get "not
found" for a set VSS still believes exists. Make requester_thaw() delete
the snapshot set explicitly at the end of the cycle, whatever its
outcome (including BackupComplete() failure and aborted snapshot sets).
A failure to delete is only logged, since the guest filesystems have
already been thawed.
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4584
Buglink: https://redhat.atlassian.net/browse/RHEL-250662
Fixes: 374f4676788c ("qga: Fix VSS provider GetSnapshotProperties return value")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
v2:
- fill the required VSS_SNAPSHOT_PROP fields
---
qga/vss-win32/install.cpp | 1 +
qga/vss-win32/provider.cpp | 200 +++++++++++++++++++++++++++++++++++-
qga/vss-win32/requester.cpp | 26 +++++
3 files changed, 226 insertions(+), 1 deletion(-)
diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index 09d94edd4f53..3b58dc840520 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -331,6 +331,7 @@ STDAPI COMRegister(void)
chk(put_Value(pObj, L"Authentication", short(6)));
chk(put_Value(pObj, L"AuthenticationCapability", short(2)));
chk(put_Value(pObj, L"ImpersonationLevel", short(2)));
+ /* NT-service-hosted apps have ConcurrentApps=1 enforced */
chk(pApps->SaveChanges(&n));
/* The app should be deleted if something fails after SaveChanges */
diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
index 1c8eb236e57f..bbc30dd57d85 100644
--- a/qga/vss-win32/provider.cpp
+++ b/qga/vss-win32/provider.cpp
@@ -11,6 +11,11 @@
*/
#include "qemu/osdep.h"
+
+#include <vector>
+#include <algorithm>
+#include <string>
+
#include "vss-common.h"
#include "vss-debug.h"
#ifdef HAVE_VSS_SDK
@@ -138,6 +143,170 @@ STDMETHODIMP CQGAVSSEnumObject::Clone(IVssEnumObject **ppenum)
return E_NOTIMPL;
}
+static VSS_PWSZ qga_vss_strdup(const WCHAR *value)
+{
+ size_t len = (wcslen(value) + 1) * sizeof(WCHAR);
+ VSS_PWSZ str = (VSS_PWSZ)CoTaskMemAlloc(len);
+
+ if (str) {
+ memcpy(str, value, len);
+ }
+ return str;
+}
+
+static VSS_PWSZ qga_vss_strdup(const std::wstring &value)
+{
+ return qga_vss_strdup(value.c_str());
+}
+
+static VSS_TIMESTAMP qga_vss_timestamp(void)
+{
+ FILETIME ft;
+ ULARGE_INTEGER value;
+
+ GetSystemTimeAsFileTime(&ft);
+ value.LowPart = ft.dwLowDateTime;
+ value.HighPart = ft.dwHighDateTime;
+ return value.QuadPart;
+}
+
+/* Registry of the snapshot IDs for this provider */
+class QGAVssSnapshotRegistry
+{
+public:
+ bool Track(VSS_ID set_id, VSS_ID snapshot_id,
+ VSS_PWSZ volume_name, LONG context);
+ HRESULT GetProperties(VSS_ID snapshot_id, VSS_SNAPSHOT_PROP *prop) const;
+ LONG RemoveSet(VSS_ID set_id); /* returns how many were removed */
+ bool Remove(VSS_ID snapshot_id);
+
+private:
+ struct Entry {
+ VSS_ID set_id;
+ VSS_ID snapshot_id;
+ std::wstring volume_name;
+ LONG context;
+ VSS_TIMESTAMP timestamp;
+ };
+
+ std::vector<Entry> m_entries;
+ mutable SRWLOCK m_lock = SRWLOCK_INIT;
+};
+
+static QGAVssSnapshotRegistry qga_vss_registry;
+
+bool QGAVssSnapshotRegistry::Track(VSS_ID set_id, VSS_ID snapshot_id,
+ VSS_PWSZ volume_name, LONG context)
+{
+ bool ok;
+
+ AcquireSRWLockExclusive(&m_lock);
+ try {
+ m_entries.push_back(Entry { set_id, snapshot_id, volume_name,
+ context, qga_vss_timestamp() });
+ ok = true;
+ } catch (...) {
+ ok = false;
+ }
+ ReleaseSRWLockExclusive(&m_lock);
+ return ok;
+}
+
+HRESULT QGAVssSnapshotRegistry::GetProperties(VSS_ID snapshot_id,
+ VSS_SNAPSHOT_PROP *prop) const
+{
+ HRESULT hr = S_OK;
+ VSS_PWSZ device = NULL, original = NULL;
+ VSS_PWSZ originating = NULL, service = NULL;
+
+ AcquireSRWLockShared(&m_lock);
+ auto it = std::find_if(m_entries.begin(), m_entries.end(),
+ [&](const Entry &e) {
+ return IsEqualGUID(e.snapshot_id, snapshot_id);
+ });
+ if (it == m_entries.end()) {
+ hr = VSS_E_OBJECT_NOT_FOUND;
+ goto out;
+ }
+
+ {
+ const Entry &e = *it;
+ WCHAR machine[256] = L"";
+ DWORD machine_size = ARRAYSIZE(machine);
+
+ if (!GetComputerNameExW(ComputerNameDnsFullyQualified, machine,
+ &machine_size)) {
+ *machine = L'\0';
+ }
+
+ device = qga_vss_strdup(L"");
+ original = qga_vss_strdup(e.volume_name);
+ originating = qga_vss_strdup(machine);
+ service = qga_vss_strdup(machine);
+ if (!device || !original || !originating || !service) {
+ hr = E_OUTOFMEMORY;
+ goto out;
+ }
+
+ memset(prop, 0, sizeof(*prop));
+ prop->m_SnapshotId = e.snapshot_id;
+ prop->m_SnapshotSetId = e.set_id;
+ prop->m_lSnapshotsCount = std::count_if(m_entries.begin(),
+ m_entries.end(), [&](const Entry &o) {
+ return IsEqualGUID(o.set_id, e.set_id);
+ });
+ prop->m_pwszSnapshotDeviceObject = device;
+ prop->m_pwszOriginalVolumeName = original;
+ prop->m_pwszOriginatingMachine = originating;
+ prop->m_pwszServiceMachine = service;
+ prop->m_ProviderId = g_gProviderId;
+ prop->m_lSnapshotAttributes = e.context;
+ prop->m_tsCreationTimestamp = e.timestamp;
+ prop->m_eStatus = VSS_SS_CREATED;
+
+ device = original = originating = service = NULL;
+ }
+
+out:
+ CoTaskMemFree(device);
+ CoTaskMemFree(original);
+ CoTaskMemFree(originating);
+ CoTaskMemFree(service);
+ ReleaseSRWLockShared(&m_lock);
+ return hr;
+}
+
+LONG QGAVssSnapshotRegistry::RemoveSet(VSS_ID set_id)
+{
+ LONG removed;
+
+ AcquireSRWLockExclusive(&m_lock);
+ auto it = std::remove_if(m_entries.begin(), m_entries.end(),
+ [&](const Entry &e) {
+ return IsEqualGUID(e.set_id, set_id);
+ });
+ removed = m_entries.end() - it;
+ m_entries.erase(it, m_entries.end());
+ ReleaseSRWLockExclusive(&m_lock);
+ return removed;
+}
+
+bool QGAVssSnapshotRegistry::Remove(VSS_ID snapshot_id)
+{
+ bool found;
+
+ AcquireSRWLockExclusive(&m_lock);
+ auto it = std::find_if(m_entries.begin(), m_entries.end(),
+ [&](const Entry &e) {
+ return IsEqualGUID(e.snapshot_id, snapshot_id);
+ });
+ found = it != m_entries.end();
+ if (found) {
+ m_entries.erase(it);
+ }
+ ReleaseSRWLockExclusive(&m_lock);
+ return found;
+}
/* QGAVssProvider */
@@ -266,7 +435,7 @@ STDMETHODIMP CQGAVssProvider::SetContext(LONG lContext)
STDMETHODIMP CQGAVssProvider::GetSnapshotProperties(
VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp)
{
- return S_OK;
+ return qga_vss_registry.GetProperties(SnapshotId, pProp);
}
STDMETHODIMP CQGAVssProvider::Query(
@@ -286,6 +455,27 @@ STDMETHODIMP CQGAVssProvider::DeleteSnapshots(
VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType,
BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID *pNondeletedSnapshotID)
{
+ LONG deleted = 0;
+
+ switch (eSourceObjectType) {
+ case VSS_OBJECT_SNAPSHOT:
+ if (qga_vss_registry.Remove(SourceObjectId)) {
+ deleted = 1;
+ }
+ break;
+ case VSS_OBJECT_SNAPSHOT_SET:
+ deleted = qga_vss_registry.RemoveSet(SourceObjectId);
+ break;
+ default:
+ break;
+ }
+
+ if (deleted > 0) {
+ *plDeletedSnapshots = deleted;
+ *pNondeletedSnapshotID = GUID_NULL;
+ return S_OK;
+ }
+
*plDeletedSnapshots = 0;
*pNondeletedSnapshotID = SourceObjectId;
return S_OK;
@@ -295,6 +485,13 @@ STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot(
VSS_ID SnapshotSetId, VSS_ID SnapshotId,
VSS_PWSZ pwszVolumeName, LONG lNewContext)
{
+ if (!pwszVolumeName) {
+ return E_INVALIDARG;
+ }
+ if (!qga_vss_registry.Track(SnapshotSetId, SnapshotId,
+ pwszVolumeName, lNewContext)) {
+ return E_OUTOFMEMORY;
+ }
return S_OK;
}
@@ -411,6 +608,7 @@ STDMETHODIMP CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId)
STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId)
{
+ qga_vss_registry.RemoveSet(SnapshotSetId);
return S_OK;
}
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 1b0155c691ef..565eefcfb283 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -52,6 +52,7 @@ static t_VssFreeSnapshotProperties pVssFreeSnapshotProperties;
static struct QGAVSSContext {
IVssBackupComponents *pVssbc; /* VSS requester interface */
IVssAsync *pAsyncSnapshot; /* async info of VSS snapshot operation */
+ VSS_ID idSnapshotSet; /* snapshot set created for this cycle */
HANDLE hEventFrozen; /* notify fs/writer freeze from provider */
HANDLE hEventThaw; /* request provider to thaw */
HANDLE hEventTimeout; /* notify timeout in provider */
@@ -116,6 +117,7 @@ static void requester_cleanup(void)
vss_ctx.pVssbc->Release();
vss_ctx.pVssbc = NULL;
}
+ vss_ctx.idSnapshotSet = GUID_NULL;
vss_ctx.cFrozenVols = 0;
qga_debug_end;
}
@@ -446,6 +448,7 @@ void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
err_set(errset, hr, "failed to start snapshot set");
goto out;
}
+ vss_ctx.idSnapshotSet = guidSnapshotSet;
if (mountpoints) {
PWCHAR volume_name_wchar;
@@ -680,6 +683,29 @@ void requester_thaw(int *num_vols, void *mountpints, ErrorSet *errset)
err_set(errset, hr, "failed to do snapshot set");
}
+ /*
+ * The snapshot set is created in VSS_CTX_APP_ROLLBACK context, so
+ * it is persistent: if it was committed, VSS keeps it (and keeps
+ * querying providers about it) until it is explicitly deleted. The
+ * freeze/thaw cycle is over whatever the outcome above, so delete
+ * it now. Failure is not fatal here, only log it.
+ */
+ {
+ LONG lDeletedSnapshots = 0;
+ VSS_ID idNonDeletedSnapshot;
+ HRESULT hrDelete;
+
+ hrDelete = vss_ctx.pVssbc->DeleteSnapshots(vss_ctx.idSnapshotSet,
+ VSS_OBJECT_SNAPSHOT_SET,
+ TRUE,
+ &lDeletedSnapshots,
+ &idNonDeletedSnapshot);
+ if (FAILED(hrDelete)) {
+ qga_debug("failed to delete snapshot set after thaw,"
+ " hr=0x%08lx", hrDelete);
+ }
+ }
+
if (err_is_set(errset)) {
vss_ctx.pVssbc->AbortBackup();
}
--
2.56.0.116.g6de20f6092dc
On Tue, Oct 27, 2026 at 6:06 PM <marcandre.lureau@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since commit 374f4676788c, the provider's GetSnapshotProperties()
> returns S_OK for any snapshot ID without filling in the properties. VSS
> consults every registered provider when a requester looks up a snapshot,
> so lookups of snapshots owned by other providers now succeed with an
> all-zero VSS_SNAPSHOT_PROP. Backup software relying on it fails, e.g.
> Veeam: "Original volume name is not specified for the shadow (shadow
> volume id: '{00000000-0000-0000-0000-000000000000}')".
>
> Simply reverting is not an option: 374f4676788c switched to S_OK because
> returning VSS_E_OBJECT_NOT_FOUND unconditionally broke qemu-ga's own
> snapshot sets. The provider must answer for its own snapshots only. It
> does not create real shadow copies, but it does take part in the
> snapshot set during qemu-ga fsfreeze cycles. Remember the snapshot IDs
> received in BeginPrepareSnapshot(), return their required properties,
> and return VSS_E_OBJECT_NOT_FOUND for anything else. Fill every required
> VSS_SNAPSHOT_PROP member except the exposed name and path, which are
> optional. The snapshot device object is an empty string because the
> host-side snapshot has no guest-visible device.
>
> Entries are dropped when VSS deletes the snapshots or the set, or when
> the set is aborted (AbortSnapshots()).
>
> COM+ may hand out several CQGAVssProvider instances (and serve several
> client processes), so this registry is module-global. It is per-process
> by design and safe because the COM+ application is hosted by a single NT
> service (see CreateServiceForApplication() in install.cpp): COM+ forbids
> pooling and recycling for service-hosted applications, so exactly one
> surrogate process ever hosts the provider.
>
> On the requester side, the snapshot set is created in the persistent
> VSS_CTX_APP_ROLLBACK context, so VSS keeps it after the freeze/thaw
> cycle and may keep querying providers about it. The provider registry
> does not outlive the cycle (requester_thaw() stops the hosting service,
> and the provider process with it), so later queries would get "not
> found" for a set VSS still believes exists. Make requester_thaw() delete
> the snapshot set explicitly at the end of the cycle, whatever its
> outcome (including BackupComplete() failure and aborted snapshot sets).
> A failure to delete is only logged, since the guest filesystems have
> already been thawed.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4584
> Buglink: https://redhat.atlassian.net/browse/RHEL-250662
> Fixes: 374f4676788c ("qga: Fix VSS provider GetSnapshotProperties return
> value")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> v2:
> - fill the required VSS_SNAPSHOT_PROP fields
> ---
> qga/vss-win32/install.cpp | 1 +
> qga/vss-win32/provider.cpp | 200 +++++++++++++++++++++++++++++++++++-
> qga/vss-win32/requester.cpp | 26 +++++
> 3 files changed, 226 insertions(+), 1 deletion(-)
>
> diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
> index 09d94edd4f53..3b58dc840520 100644
> --- a/qga/vss-win32/install.cpp
> +++ b/qga/vss-win32/install.cpp
> @@ -331,6 +331,7 @@ STDAPI COMRegister(void)
> chk(put_Value(pObj, L"Authentication", short(6)));
> chk(put_Value(pObj, L"AuthenticationCapability", short(2)));
> chk(put_Value(pObj, L"ImpersonationLevel", short(2)));
> + /* NT-service-hosted apps have ConcurrentApps=1 enforced */
> chk(pApps->SaveChanges(&n));
>
> /* The app should be deleted if something fails after SaveChanges */
> diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
> index 1c8eb236e57f..bbc30dd57d85 100644
> --- a/qga/vss-win32/provider.cpp
> +++ b/qga/vss-win32/provider.cpp
> @@ -11,6 +11,11 @@
> */
>
> #include "qemu/osdep.h"
> +
> +#include <vector>
> +#include <algorithm>
> +#include <string>
> +
> #include "vss-common.h"
> #include "vss-debug.h"
> #ifdef HAVE_VSS_SDK
> @@ -138,6 +143,170 @@ STDMETHODIMP CQGAVSSEnumObject::Clone(IVssEnumObject
> **ppenum)
> return E_NOTIMPL;
> }
>
> +static VSS_PWSZ qga_vss_strdup(const WCHAR *value)
> +{
> + size_t len = (wcslen(value) + 1) * sizeof(WCHAR);
> + VSS_PWSZ str = (VSS_PWSZ)CoTaskMemAlloc(len);
> +
> + if (str) {
> + memcpy(str, value, len);
> + }
> + return str;
> +}
> +
> +static VSS_PWSZ qga_vss_strdup(const std::wstring &value)
> +{
> + return qga_vss_strdup(value.c_str());
> +}
> +
> +static VSS_TIMESTAMP qga_vss_timestamp(void)
> +{
> + FILETIME ft;
> + ULARGE_INTEGER value;
> +
> + GetSystemTimeAsFileTime(&ft);
> + value.LowPart = ft.dwLowDateTime;
> + value.HighPart = ft.dwHighDateTime;
> + return value.QuadPart;
> +}
> +
> +/* Registry of the snapshot IDs for this provider */
> +class QGAVssSnapshotRegistry
> +{
> +public:
> + bool Track(VSS_ID set_id, VSS_ID snapshot_id,
> + VSS_PWSZ volume_name, LONG context);
> + HRESULT GetProperties(VSS_ID snapshot_id, VSS_SNAPSHOT_PROP *prop)
> const;
> + LONG RemoveSet(VSS_ID set_id); /* returns how many were removed */
> + bool Remove(VSS_ID snapshot_id);
> +
> +private:
> + struct Entry {
> + VSS_ID set_id;
> + VSS_ID snapshot_id;
> + std::wstring volume_name;
> + LONG context;
> + VSS_TIMESTAMP timestamp;
> + };
> +
> + std::vector<Entry> m_entries;
> + mutable SRWLOCK m_lock = SRWLOCK_INIT;
> +};
> +
> +static QGAVssSnapshotRegistry qga_vss_registry;
> +
> +bool QGAVssSnapshotRegistry::Track(VSS_ID set_id, VSS_ID snapshot_id,
> + VSS_PWSZ volume_name, LONG context)
> +{
> + bool ok;
> +
> + AcquireSRWLockExclusive(&m_lock);
> + try {
> + m_entries.push_back(Entry { set_id, snapshot_id, volume_name,
> + context, qga_vss_timestamp() });
> + ok = true;
> + } catch (...) {
> + ok = false;
> + }
> + ReleaseSRWLockExclusive(&m_lock);
> + return ok;
> +}
> +
> +HRESULT QGAVssSnapshotRegistry::GetProperties(VSS_ID snapshot_id,
> + VSS_SNAPSHOT_PROP *prop)
> const
> +{
> + HRESULT hr = S_OK;
> + VSS_PWSZ device = NULL, original = NULL;
> + VSS_PWSZ originating = NULL, service = NULL;
> +
> + AcquireSRWLockShared(&m_lock);
> + auto it = std::find_if(m_entries.begin(), m_entries.end(),
> + [&](const Entry &e) {
> + return IsEqualGUID(e.snapshot_id, snapshot_id);
> + });
> + if (it == m_entries.end()) {
> + hr = VSS_E_OBJECT_NOT_FOUND;
> + goto out;
> + }
> +
> + {
> + const Entry &e = *it;
> + WCHAR machine[256] = L"";
> + DWORD machine_size = ARRAYSIZE(machine);
> +
> + if (!GetComputerNameExW(ComputerNameDnsFullyQualified, machine,
> + &machine_size)) {
> + *machine = L'\0';
> + }
> +
> + device = qga_vss_strdup(L"");
> + original = qga_vss_strdup(e.volume_name);
> + originating = qga_vss_strdup(machine);
> + service = qga_vss_strdup(machine);
> + if (!device || !original || !originating || !service) {
> + hr = E_OUTOFMEMORY;
> + goto out;
> + }
> +
> + memset(prop, 0, sizeof(*prop));
> + prop->m_SnapshotId = e.snapshot_id;
> + prop->m_SnapshotSetId = e.set_id;
> + prop->m_lSnapshotsCount = std::count_if(m_entries.begin(),
> + m_entries.end(), [&](const Entry &o) {
> + return IsEqualGUID(o.set_id, e.set_id);
> + });
> + prop->m_pwszSnapshotDeviceObject = device;
> + prop->m_pwszOriginalVolumeName = original;
> + prop->m_pwszOriginatingMachine = originating;
> + prop->m_pwszServiceMachine = service;
> + prop->m_ProviderId = g_gProviderId;
> + prop->m_lSnapshotAttributes = e.context;
> + prop->m_tsCreationTimestamp = e.timestamp;
> + prop->m_eStatus = VSS_SS_CREATED;
> +
> + device = original = originating = service = NULL;
> + }
> +
> +out:
> + CoTaskMemFree(device);
> + CoTaskMemFree(original);
> + CoTaskMemFree(originating);
> + CoTaskMemFree(service);
> + ReleaseSRWLockShared(&m_lock);
> + return hr;
> +}
> +
> +LONG QGAVssSnapshotRegistry::RemoveSet(VSS_ID set_id)
> +{
> + LONG removed;
> +
> + AcquireSRWLockExclusive(&m_lock);
> + auto it = std::remove_if(m_entries.begin(), m_entries.end(),
> + [&](const Entry &e) {
> + return IsEqualGUID(e.set_id, set_id);
> + });
> + removed = m_entries.end() - it;
> + m_entries.erase(it, m_entries.end());
> + ReleaseSRWLockExclusive(&m_lock);
> + return removed;
> +}
> +
> +bool QGAVssSnapshotRegistry::Remove(VSS_ID snapshot_id)
> +{
> + bool found;
> +
> + AcquireSRWLockExclusive(&m_lock);
> + auto it = std::find_if(m_entries.begin(), m_entries.end(),
> + [&](const Entry &e) {
> + return IsEqualGUID(e.snapshot_id, snapshot_id);
> + });
> + found = it != m_entries.end();
> + if (found) {
> + m_entries.erase(it);
> + }
> + ReleaseSRWLockExclusive(&m_lock);
> + return found;
> +}
>
> /* QGAVssProvider */
>
> @@ -266,7 +435,7 @@ STDMETHODIMP CQGAVssProvider::SetContext(LONG lContext)
> STDMETHODIMP CQGAVssProvider::GetSnapshotProperties(
> VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp)
> {
> - return S_OK;
> + return qga_vss_registry.GetProperties(SnapshotId, pProp);
> }
>
> STDMETHODIMP CQGAVssProvider::Query(
> @@ -286,6 +455,27 @@ STDMETHODIMP CQGAVssProvider::DeleteSnapshots(
> VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType,
> BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID
> *pNondeletedSnapshotID)
> {
> + LONG deleted = 0;
> +
> + switch (eSourceObjectType) {
> + case VSS_OBJECT_SNAPSHOT:
> + if (qga_vss_registry.Remove(SourceObjectId)) {
> + deleted = 1;
> + }
> + break;
> + case VSS_OBJECT_SNAPSHOT_SET:
> + deleted = qga_vss_registry.RemoveSet(SourceObjectId);
> + break;
> + default:
> + break;
> + }
> +
> + if (deleted > 0) {
> + *plDeletedSnapshots = deleted;
> + *pNondeletedSnapshotID = GUID_NULL;
> + return S_OK;
> + }
> +
> *plDeletedSnapshots = 0;
> *pNondeletedSnapshotID = SourceObjectId;
> return S_OK;
>
return VSS_E_OBJECT_NOT_FOUND in this case looks more correct
> @@ -295,6 +485,13 @@ STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot(
> VSS_ID SnapshotSetId, VSS_ID SnapshotId,
> VSS_PWSZ pwszVolumeName, LONG lNewContext)
> {
> + if (!pwszVolumeName) {
> + return E_INVALIDARG;
> + }
> + if (!qga_vss_registry.Track(SnapshotSetId, SnapshotId,
> + pwszVolumeName, lNewContext)) {
> + return E_OUTOFMEMORY;
> + }
> return S_OK;
> }
>
> @@ -411,6 +608,7 @@ STDMETHODIMP
> CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId)
>
> STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId)
> {
> + qga_vss_registry.RemoveSet(SnapshotSetId);
> return S_OK;
> }
>
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 1b0155c691ef..565eefcfb283 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -52,6 +52,7 @@ static t_VssFreeSnapshotProperties
> pVssFreeSnapshotProperties;
> static struct QGAVSSContext {
> IVssBackupComponents *pVssbc; /* VSS requester interface */
> IVssAsync *pAsyncSnapshot; /* async info of VSS snapshot
> operation */
> + VSS_ID idSnapshotSet; /* snapshot set created for this cycle
> */
> HANDLE hEventFrozen; /* notify fs/writer freeze from
> provider */
> HANDLE hEventThaw; /* request provider to thaw */
> HANDLE hEventTimeout; /* notify timeout in provider */
> @@ -116,6 +117,7 @@ static void requester_cleanup(void)
> vss_ctx.pVssbc->Release();
> vss_ctx.pVssbc = NULL;
> }
> + vss_ctx.idSnapshotSet = GUID_NULL;
> vss_ctx.cFrozenVols = 0;
> qga_debug_end;
> }
> @@ -446,6 +448,7 @@ void requester_freeze(int *num_vols, void
> *mountpoints, ErrorSet *errset)
> err_set(errset, hr, "failed to start snapshot set");
> goto out;
> }
> + vss_ctx.idSnapshotSet = guidSnapshotSet;
>
> if (mountpoints) {
> PWCHAR volume_name_wchar;
> @@ -680,6 +683,29 @@ void requester_thaw(int *num_vols, void *mountpints,
> ErrorSet *errset)
> err_set(errset, hr, "failed to do snapshot set");
> }
>
> + /*
> + * The snapshot set is created in VSS_CTX_APP_ROLLBACK context, so
> + * it is persistent: if it was committed, VSS keeps it (and keeps
> + * querying providers about it) until it is explicitly deleted. The
> + * freeze/thaw cycle is over whatever the outcome above, so delete
> + * it now. Failure is not fatal here, only log it.
> + */
> + {
> + LONG lDeletedSnapshots = 0;
> + VSS_ID idNonDeletedSnapshot;
> + HRESULT hrDelete;
> +
> + hrDelete = vss_ctx.pVssbc->DeleteSnapshots(vss_ctx.idSnapshotSet,
> +
> VSS_OBJECT_SNAPSHOT_SET,
> + TRUE,
> + &lDeletedSnapshots,
> + &idNonDeletedSnapshot);
> + if (FAILED(hrDelete)) {
> + qga_debug("failed to delete snapshot set after thaw,"
> + " hr=0x%08lx", hrDelete);
> + }
> + }
> +
> if (err_is_set(errset)) {
> vss_ctx.pVssbc->AbortBackup();
> }
> --
> 2.56.0.116.g6de20f6092dc
>
>
Hi
On Wed, Oct 28, 2026 at 12:08 PM Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
>
>
>
>
> On Tue, Oct 27, 2026 at 6:06 PM <marcandre.lureau@redhat.com> wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Since commit 374f4676788c, the provider's GetSnapshotProperties()
>> returns S_OK for any snapshot ID without filling in the properties. VSS
>> consults every registered provider when a requester looks up a snapshot,
>> so lookups of snapshots owned by other providers now succeed with an
>> all-zero VSS_SNAPSHOT_PROP. Backup software relying on it fails, e.g.
>> Veeam: "Original volume name is not specified for the shadow (shadow
>> volume id: '{00000000-0000-0000-0000-000000000000}')".
>>
>> Simply reverting is not an option: 374f4676788c switched to S_OK because
>> returning VSS_E_OBJECT_NOT_FOUND unconditionally broke qemu-ga's own
>> snapshot sets. The provider must answer for its own snapshots only. It
>> does not create real shadow copies, but it does take part in the
>> snapshot set during qemu-ga fsfreeze cycles. Remember the snapshot IDs
>> received in BeginPrepareSnapshot(), return their required properties,
>> and return VSS_E_OBJECT_NOT_FOUND for anything else. Fill every required
>> VSS_SNAPSHOT_PROP member except the exposed name and path, which are
>> optional. The snapshot device object is an empty string because the
>> host-side snapshot has no guest-visible device.
>>
>> Entries are dropped when VSS deletes the snapshots or the set, or when
>> the set is aborted (AbortSnapshots()).
>>
>> COM+ may hand out several CQGAVssProvider instances (and serve several
>> client processes), so this registry is module-global. It is per-process
>> by design and safe because the COM+ application is hosted by a single NT
>> service (see CreateServiceForApplication() in install.cpp): COM+ forbids
>> pooling and recycling for service-hosted applications, so exactly one
>> surrogate process ever hosts the provider.
>>
>> On the requester side, the snapshot set is created in the persistent
>> VSS_CTX_APP_ROLLBACK context, so VSS keeps it after the freeze/thaw
>> cycle and may keep querying providers about it. The provider registry
>> does not outlive the cycle (requester_thaw() stops the hosting service,
>> and the provider process with it), so later queries would get "not
>> found" for a set VSS still believes exists. Make requester_thaw() delete
>> the snapshot set explicitly at the end of the cycle, whatever its
>> outcome (including BackupComplete() failure and aborted snapshot sets).
>> A failure to delete is only logged, since the guest filesystems have
>> already been thawed.
>>
>> Cc: qemu-stable@nongnu.org
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4584
>> Buglink: https://redhat.atlassian.net/browse/RHEL-250662
>> Fixes: 374f4676788c ("qga: Fix VSS provider GetSnapshotProperties return value")
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>> v2:
>> - fill the required VSS_SNAPSHOT_PROP fields
>> ---
>> qga/vss-win32/install.cpp | 1 +
>> qga/vss-win32/provider.cpp | 200 +++++++++++++++++++++++++++++++++++-
>> qga/vss-win32/requester.cpp | 26 +++++
>> 3 files changed, 226 insertions(+), 1 deletion(-)
>>
>> diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
>> index 09d94edd4f53..3b58dc840520 100644
>> --- a/qga/vss-win32/install.cpp
>> +++ b/qga/vss-win32/install.cpp
>> @@ -331,6 +331,7 @@ STDAPI COMRegister(void)
>> chk(put_Value(pObj, L"Authentication", short(6)));
>> chk(put_Value(pObj, L"AuthenticationCapability", short(2)));
>> chk(put_Value(pObj, L"ImpersonationLevel", short(2)));
>> + /* NT-service-hosted apps have ConcurrentApps=1 enforced */
>> chk(pApps->SaveChanges(&n));
>>
>> /* The app should be deleted if something fails after SaveChanges */
>> diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
>> index 1c8eb236e57f..bbc30dd57d85 100644
>> --- a/qga/vss-win32/provider.cpp
>> +++ b/qga/vss-win32/provider.cpp
>> @@ -11,6 +11,11 @@
>> */
>>
>> #include "qemu/osdep.h"
>> +
>> +#include <vector>
>> +#include <algorithm>
>> +#include <string>
>> +
>> #include "vss-common.h"
>> #include "vss-debug.h"
>> #ifdef HAVE_VSS_SDK
>> @@ -138,6 +143,170 @@ STDMETHODIMP CQGAVSSEnumObject::Clone(IVssEnumObject **ppenum)
>> return E_NOTIMPL;
>> }
>>
>> +static VSS_PWSZ qga_vss_strdup(const WCHAR *value)
>> +{
>> + size_t len = (wcslen(value) + 1) * sizeof(WCHAR);
>> + VSS_PWSZ str = (VSS_PWSZ)CoTaskMemAlloc(len);
>> +
>> + if (str) {
>> + memcpy(str, value, len);
>> + }
>> + return str;
>> +}
>> +
>> +static VSS_PWSZ qga_vss_strdup(const std::wstring &value)
>> +{
>> + return qga_vss_strdup(value.c_str());
>> +}
>> +
>> +static VSS_TIMESTAMP qga_vss_timestamp(void)
>> +{
>> + FILETIME ft;
>> + ULARGE_INTEGER value;
>> +
>> + GetSystemTimeAsFileTime(&ft);
>> + value.LowPart = ft.dwLowDateTime;
>> + value.HighPart = ft.dwHighDateTime;
>> + return value.QuadPart;
>> +}
>> +
>> +/* Registry of the snapshot IDs for this provider */
>> +class QGAVssSnapshotRegistry
>> +{
>> +public:
>> + bool Track(VSS_ID set_id, VSS_ID snapshot_id,
>> + VSS_PWSZ volume_name, LONG context);
>> + HRESULT GetProperties(VSS_ID snapshot_id, VSS_SNAPSHOT_PROP *prop) const;
>> + LONG RemoveSet(VSS_ID set_id); /* returns how many were removed */
>> + bool Remove(VSS_ID snapshot_id);
>> +
>> +private:
>> + struct Entry {
>> + VSS_ID set_id;
>> + VSS_ID snapshot_id;
>> + std::wstring volume_name;
>> + LONG context;
>> + VSS_TIMESTAMP timestamp;
>> + };
>> +
>> + std::vector<Entry> m_entries;
>> + mutable SRWLOCK m_lock = SRWLOCK_INIT;
>> +};
>> +
>> +static QGAVssSnapshotRegistry qga_vss_registry;
>> +
>> +bool QGAVssSnapshotRegistry::Track(VSS_ID set_id, VSS_ID snapshot_id,
>> + VSS_PWSZ volume_name, LONG context)
>> +{
>> + bool ok;
>> +
>> + AcquireSRWLockExclusive(&m_lock);
>> + try {
>> + m_entries.push_back(Entry { set_id, snapshot_id, volume_name,
>> + context, qga_vss_timestamp() });
>> + ok = true;
>> + } catch (...) {
>> + ok = false;
>> + }
>> + ReleaseSRWLockExclusive(&m_lock);
>> + return ok;
>> +}
>> +
>> +HRESULT QGAVssSnapshotRegistry::GetProperties(VSS_ID snapshot_id,
>> + VSS_SNAPSHOT_PROP *prop) const
>> +{
>> + HRESULT hr = S_OK;
>> + VSS_PWSZ device = NULL, original = NULL;
>> + VSS_PWSZ originating = NULL, service = NULL;
>> +
>> + AcquireSRWLockShared(&m_lock);
>> + auto it = std::find_if(m_entries.begin(), m_entries.end(),
>> + [&](const Entry &e) {
>> + return IsEqualGUID(e.snapshot_id, snapshot_id);
>> + });
>> + if (it == m_entries.end()) {
>> + hr = VSS_E_OBJECT_NOT_FOUND;
>> + goto out;
>> + }
>> +
>> + {
>> + const Entry &e = *it;
>> + WCHAR machine[256] = L"";
>> + DWORD machine_size = ARRAYSIZE(machine);
>> +
>> + if (!GetComputerNameExW(ComputerNameDnsFullyQualified, machine,
>> + &machine_size)) {
>> + *machine = L'\0';
>> + }
>> +
>> + device = qga_vss_strdup(L"");
>> + original = qga_vss_strdup(e.volume_name);
>> + originating = qga_vss_strdup(machine);
>> + service = qga_vss_strdup(machine);
>> + if (!device || !original || !originating || !service) {
>> + hr = E_OUTOFMEMORY;
>> + goto out;
>> + }
>> +
>> + memset(prop, 0, sizeof(*prop));
>> + prop->m_SnapshotId = e.snapshot_id;
>> + prop->m_SnapshotSetId = e.set_id;
>> + prop->m_lSnapshotsCount = std::count_if(m_entries.begin(),
>> + m_entries.end(), [&](const Entry &o) {
>> + return IsEqualGUID(o.set_id, e.set_id);
>> + });
>> + prop->m_pwszSnapshotDeviceObject = device;
>> + prop->m_pwszOriginalVolumeName = original;
>> + prop->m_pwszOriginatingMachine = originating;
>> + prop->m_pwszServiceMachine = service;
>> + prop->m_ProviderId = g_gProviderId;
>> + prop->m_lSnapshotAttributes = e.context;
>> + prop->m_tsCreationTimestamp = e.timestamp;
>> + prop->m_eStatus = VSS_SS_CREATED;
>> +
>> + device = original = originating = service = NULL;
>> + }
>> +
>> +out:
>> + CoTaskMemFree(device);
>> + CoTaskMemFree(original);
>> + CoTaskMemFree(originating);
>> + CoTaskMemFree(service);
>> + ReleaseSRWLockShared(&m_lock);
>> + return hr;
>> +}
>> +
>> +LONG QGAVssSnapshotRegistry::RemoveSet(VSS_ID set_id)
>> +{
>> + LONG removed;
>> +
>> + AcquireSRWLockExclusive(&m_lock);
>> + auto it = std::remove_if(m_entries.begin(), m_entries.end(),
>> + [&](const Entry &e) {
>> + return IsEqualGUID(e.set_id, set_id);
>> + });
>> + removed = m_entries.end() - it;
>> + m_entries.erase(it, m_entries.end());
>> + ReleaseSRWLockExclusive(&m_lock);
>> + return removed;
>> +}
>> +
>> +bool QGAVssSnapshotRegistry::Remove(VSS_ID snapshot_id)
>> +{
>> + bool found;
>> +
>> + AcquireSRWLockExclusive(&m_lock);
>> + auto it = std::find_if(m_entries.begin(), m_entries.end(),
>> + [&](const Entry &e) {
>> + return IsEqualGUID(e.snapshot_id, snapshot_id);
>> + });
>> + found = it != m_entries.end();
>> + if (found) {
>> + m_entries.erase(it);
>> + }
>> + ReleaseSRWLockExclusive(&m_lock);
>> + return found;
>> +}
>>
>> /* QGAVssProvider */
>>
>> @@ -266,7 +435,7 @@ STDMETHODIMP CQGAVssProvider::SetContext(LONG lContext)
>> STDMETHODIMP CQGAVssProvider::GetSnapshotProperties(
>> VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp)
>> {
>> - return S_OK;
>> + return qga_vss_registry.GetProperties(SnapshotId, pProp);
>> }
>>
>> STDMETHODIMP CQGAVssProvider::Query(
>> @@ -286,6 +455,27 @@ STDMETHODIMP CQGAVssProvider::DeleteSnapshots(
>> VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType,
>> BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID *pNondeletedSnapshotID)
>> {
>> + LONG deleted = 0;
>> +
>> + switch (eSourceObjectType) {
>> + case VSS_OBJECT_SNAPSHOT:
>> + if (qga_vss_registry.Remove(SourceObjectId)) {
>> + deleted = 1;
>> + }
>> + break;
>> + case VSS_OBJECT_SNAPSHOT_SET:
>> + deleted = qga_vss_registry.RemoveSet(SourceObjectId);
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + if (deleted > 0) {
>> + *plDeletedSnapshots = deleted;
>> + *pNondeletedSnapshotID = GUID_NULL;
>> + return S_OK;
>> + }
>> +
>> *plDeletedSnapshots = 0;
>> *pNondeletedSnapshotID = SourceObjectId;
>> return S_OK;
>
>
> return VSS_E_OBJECT_NOT_FOUND in this case looks more correct
Possibly, but I didn't want to change the existing behaviour here
without good reasons (it was fixed in 2014 by commit
d9e1f574cb6eac0a3a2f97b67d2e7a3ad9c1dc95).
>
>>
>> @@ -295,6 +485,13 @@ STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot(
>> VSS_ID SnapshotSetId, VSS_ID SnapshotId,
>> VSS_PWSZ pwszVolumeName, LONG lNewContext)
>> {
>> + if (!pwszVolumeName) {
>> + return E_INVALIDARG;
>> + }
>> + if (!qga_vss_registry.Track(SnapshotSetId, SnapshotId,
>> + pwszVolumeName, lNewContext)) {
>> + return E_OUTOFMEMORY;
>> + }
>> return S_OK;
>> }
>>
>> @@ -411,6 +608,7 @@ STDMETHODIMP CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId)
>>
>> STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId)
>> {
>> + qga_vss_registry.RemoveSet(SnapshotSetId);
>> return S_OK;
>> }
>>
>> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
>> index 1b0155c691ef..565eefcfb283 100644
>> --- a/qga/vss-win32/requester.cpp
>> +++ b/qga/vss-win32/requester.cpp
>> @@ -52,6 +52,7 @@ static t_VssFreeSnapshotProperties pVssFreeSnapshotProperties;
>> static struct QGAVSSContext {
>> IVssBackupComponents *pVssbc; /* VSS requester interface */
>> IVssAsync *pAsyncSnapshot; /* async info of VSS snapshot operation */
>> + VSS_ID idSnapshotSet; /* snapshot set created for this cycle */
>> HANDLE hEventFrozen; /* notify fs/writer freeze from provider */
>> HANDLE hEventThaw; /* request provider to thaw */
>> HANDLE hEventTimeout; /* notify timeout in provider */
>> @@ -116,6 +117,7 @@ static void requester_cleanup(void)
>> vss_ctx.pVssbc->Release();
>> vss_ctx.pVssbc = NULL;
>> }
>> + vss_ctx.idSnapshotSet = GUID_NULL;
>> vss_ctx.cFrozenVols = 0;
>> qga_debug_end;
>> }
>> @@ -446,6 +448,7 @@ void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
>> err_set(errset, hr, "failed to start snapshot set");
>> goto out;
>> }
>> + vss_ctx.idSnapshotSet = guidSnapshotSet;
>>
>> if (mountpoints) {
>> PWCHAR volume_name_wchar;
>> @@ -680,6 +683,29 @@ void requester_thaw(int *num_vols, void *mountpints, ErrorSet *errset)
>> err_set(errset, hr, "failed to do snapshot set");
>> }
>>
>> + /*
>> + * The snapshot set is created in VSS_CTX_APP_ROLLBACK context, so
>> + * it is persistent: if it was committed, VSS keeps it (and keeps
>> + * querying providers about it) until it is explicitly deleted. The
>> + * freeze/thaw cycle is over whatever the outcome above, so delete
>> + * it now. Failure is not fatal here, only log it.
>> + */
>> + {
>> + LONG lDeletedSnapshots = 0;
>> + VSS_ID idNonDeletedSnapshot;
>> + HRESULT hrDelete;
>> +
>> + hrDelete = vss_ctx.pVssbc->DeleteSnapshots(vss_ctx.idSnapshotSet,
>> + VSS_OBJECT_SNAPSHOT_SET,
>> + TRUE,
>> + &lDeletedSnapshots,
>> + &idNonDeletedSnapshot);
>> + if (FAILED(hrDelete)) {
>> + qga_debug("failed to delete snapshot set after thaw,"
>> + " hr=0x%08lx", hrDelete);
>> + }
>> + }
>> +
>> if (err_is_set(errset)) {
>> vss_ctx.pVssbc->AbortBackup();
>> }
>> --
>> 2.56.0.116.g6de20f6092dc
>>
Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Tested with reproducer from
https://gitlab.com/qemu-project/qemu/-/work_items/4584
Behavious is expected
On Wed, Oct 28, 2026 at 12:21 PM Marc-André Lureau <
marcandre.lureau@redhat.com> wrote:
> Hi
>
> On Wed, Oct 28, 2026 at 12:08 PM Kostiantyn Kostiuk <kkostiuk@redhat.com>
> wrote:
> >
> >
> >
> >
> >
> > On Tue, Oct 27, 2026 at 6:06 PM <marcandre.lureau@redhat.com> wrote:
> >>
> >> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>
> >> Since commit 374f4676788c, the provider's GetSnapshotProperties()
> >> returns S_OK for any snapshot ID without filling in the properties. VSS
> >> consults every registered provider when a requester looks up a snapshot,
> >> so lookups of snapshots owned by other providers now succeed with an
> >> all-zero VSS_SNAPSHOT_PROP. Backup software relying on it fails, e.g.
> >> Veeam: "Original volume name is not specified for the shadow (shadow
> >> volume id: '{00000000-0000-0000-0000-000000000000}')".
> >>
> >> Simply reverting is not an option: 374f4676788c switched to S_OK because
> >> returning VSS_E_OBJECT_NOT_FOUND unconditionally broke qemu-ga's own
> >> snapshot sets. The provider must answer for its own snapshots only. It
> >> does not create real shadow copies, but it does take part in the
> >> snapshot set during qemu-ga fsfreeze cycles. Remember the snapshot IDs
> >> received in BeginPrepareSnapshot(), return their required properties,
> >> and return VSS_E_OBJECT_NOT_FOUND for anything else. Fill every required
> >> VSS_SNAPSHOT_PROP member except the exposed name and path, which are
> >> optional. The snapshot device object is an empty string because the
> >> host-side snapshot has no guest-visible device.
> >>
> >> Entries are dropped when VSS deletes the snapshots or the set, or when
> >> the set is aborted (AbortSnapshots()).
> >>
> >> COM+ may hand out several CQGAVssProvider instances (and serve several
> >> client processes), so this registry is module-global. It is per-process
> >> by design and safe because the COM+ application is hosted by a single NT
> >> service (see CreateServiceForApplication() in install.cpp): COM+ forbids
> >> pooling and recycling for service-hosted applications, so exactly one
> >> surrogate process ever hosts the provider.
> >>
> >> On the requester side, the snapshot set is created in the persistent
> >> VSS_CTX_APP_ROLLBACK context, so VSS keeps it after the freeze/thaw
> >> cycle and may keep querying providers about it. The provider registry
> >> does not outlive the cycle (requester_thaw() stops the hosting service,
> >> and the provider process with it), so later queries would get "not
> >> found" for a set VSS still believes exists. Make requester_thaw() delete
> >> the snapshot set explicitly at the end of the cycle, whatever its
> >> outcome (including BackupComplete() failure and aborted snapshot sets).
> >> A failure to delete is only logged, since the guest filesystems have
> >> already been thawed.
> >>
> >> Cc: qemu-stable@nongnu.org
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4584
> >> Buglink: https://redhat.atlassian.net/browse/RHEL-250662
> >> Fixes: 374f4676788c ("qga: Fix VSS provider GetSnapshotProperties
> return value")
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> ---
> >> v2:
> >> - fill the required VSS_SNAPSHOT_PROP fields
> >> ---
> >> qga/vss-win32/install.cpp | 1 +
> >> qga/vss-win32/provider.cpp | 200 +++++++++++++++++++++++++++++++++++-
> >> qga/vss-win32/requester.cpp | 26 +++++
> >> 3 files changed, 226 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
> >> index 09d94edd4f53..3b58dc840520 100644
> >> --- a/qga/vss-win32/install.cpp
> >> +++ b/qga/vss-win32/install.cpp
> >> @@ -331,6 +331,7 @@ STDAPI COMRegister(void)
> >> chk(put_Value(pObj, L"Authentication", short(6)));
> >> chk(put_Value(pObj, L"AuthenticationCapability", short(2)));
> >> chk(put_Value(pObj, L"ImpersonationLevel", short(2)));
> >> + /* NT-service-hosted apps have ConcurrentApps=1 enforced */
> >> chk(pApps->SaveChanges(&n));
> >>
> >> /* The app should be deleted if something fails after SaveChanges
> */
> >> diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
> >> index 1c8eb236e57f..bbc30dd57d85 100644
> >> --- a/qga/vss-win32/provider.cpp
> >> +++ b/qga/vss-win32/provider.cpp
> >> @@ -11,6 +11,11 @@
> >> */
> >>
> >> #include "qemu/osdep.h"
> >> +
> >> +#include <vector>
> >> +#include <algorithm>
> >> +#include <string>
> >> +
> >> #include "vss-common.h"
> >> #include "vss-debug.h"
> >> #ifdef HAVE_VSS_SDK
> >> @@ -138,6 +143,170 @@ STDMETHODIMP
> CQGAVSSEnumObject::Clone(IVssEnumObject **ppenum)
> >> return E_NOTIMPL;
> >> }
> >>
> >> +static VSS_PWSZ qga_vss_strdup(const WCHAR *value)
> >> +{
> >> + size_t len = (wcslen(value) + 1) * sizeof(WCHAR);
> >> + VSS_PWSZ str = (VSS_PWSZ)CoTaskMemAlloc(len);
> >> +
> >> + if (str) {
> >> + memcpy(str, value, len);
> >> + }
> >> + return str;
> >> +}
> >> +
> >> +static VSS_PWSZ qga_vss_strdup(const std::wstring &value)
> >> +{
> >> + return qga_vss_strdup(value.c_str());
> >> +}
> >> +
> >> +static VSS_TIMESTAMP qga_vss_timestamp(void)
> >> +{
> >> + FILETIME ft;
> >> + ULARGE_INTEGER value;
> >> +
> >> + GetSystemTimeAsFileTime(&ft);
> >> + value.LowPart = ft.dwLowDateTime;
> >> + value.HighPart = ft.dwHighDateTime;
> >> + return value.QuadPart;
> >> +}
> >> +
> >> +/* Registry of the snapshot IDs for this provider */
> >> +class QGAVssSnapshotRegistry
> >> +{
> >> +public:
> >> + bool Track(VSS_ID set_id, VSS_ID snapshot_id,
> >> + VSS_PWSZ volume_name, LONG context);
> >> + HRESULT GetProperties(VSS_ID snapshot_id, VSS_SNAPSHOT_PROP *prop)
> const;
> >> + LONG RemoveSet(VSS_ID set_id); /* returns how many were removed */
> >> + bool Remove(VSS_ID snapshot_id);
> >> +
> >> +private:
> >> + struct Entry {
> >> + VSS_ID set_id;
> >> + VSS_ID snapshot_id;
> >> + std::wstring volume_name;
> >> + LONG context;
> >> + VSS_TIMESTAMP timestamp;
> >> + };
> >> +
> >> + std::vector<Entry> m_entries;
> >> + mutable SRWLOCK m_lock = SRWLOCK_INIT;
> >> +};
> >> +
> >> +static QGAVssSnapshotRegistry qga_vss_registry;
> >> +
> >> +bool QGAVssSnapshotRegistry::Track(VSS_ID set_id, VSS_ID snapshot_id,
> >> + VSS_PWSZ volume_name, LONG context)
> >> +{
> >> + bool ok;
> >> +
> >> + AcquireSRWLockExclusive(&m_lock);
> >> + try {
> >> + m_entries.push_back(Entry { set_id, snapshot_id, volume_name,
> >> + context, qga_vss_timestamp() });
> >> + ok = true;
> >> + } catch (...) {
> >> + ok = false;
> >> + }
> >> + ReleaseSRWLockExclusive(&m_lock);
> >> + return ok;
> >> +}
> >> +
> >> +HRESULT QGAVssSnapshotRegistry::GetProperties(VSS_ID snapshot_id,
> >> + VSS_SNAPSHOT_PROP *prop)
> const
> >> +{
> >> + HRESULT hr = S_OK;
> >> + VSS_PWSZ device = NULL, original = NULL;
> >> + VSS_PWSZ originating = NULL, service = NULL;
> >> +
> >> + AcquireSRWLockShared(&m_lock);
> >> + auto it = std::find_if(m_entries.begin(), m_entries.end(),
> >> + [&](const Entry &e) {
> >> + return IsEqualGUID(e.snapshot_id, snapshot_id);
> >> + });
> >> + if (it == m_entries.end()) {
> >> + hr = VSS_E_OBJECT_NOT_FOUND;
> >> + goto out;
> >> + }
> >> +
> >> + {
> >> + const Entry &e = *it;
> >> + WCHAR machine[256] = L"";
> >> + DWORD machine_size = ARRAYSIZE(machine);
> >> +
> >> + if (!GetComputerNameExW(ComputerNameDnsFullyQualified, machine,
> >> + &machine_size)) {
> >> + *machine = L'\0';
> >> + }
> >> +
> >> + device = qga_vss_strdup(L"");
> >> + original = qga_vss_strdup(e.volume_name);
> >> + originating = qga_vss_strdup(machine);
> >> + service = qga_vss_strdup(machine);
> >> + if (!device || !original || !originating || !service) {
> >> + hr = E_OUTOFMEMORY;
> >> + goto out;
> >> + }
> >> +
> >> + memset(prop, 0, sizeof(*prop));
> >> + prop->m_SnapshotId = e.snapshot_id;
> >> + prop->m_SnapshotSetId = e.set_id;
> >> + prop->m_lSnapshotsCount = std::count_if(m_entries.begin(),
> >> + m_entries.end(), [&](const Entry &o) {
> >> + return IsEqualGUID(o.set_id, e.set_id);
> >> + });
> >> + prop->m_pwszSnapshotDeviceObject = device;
> >> + prop->m_pwszOriginalVolumeName = original;
> >> + prop->m_pwszOriginatingMachine = originating;
> >> + prop->m_pwszServiceMachine = service;
> >> + prop->m_ProviderId = g_gProviderId;
> >> + prop->m_lSnapshotAttributes = e.context;
> >> + prop->m_tsCreationTimestamp = e.timestamp;
> >> + prop->m_eStatus = VSS_SS_CREATED;
> >> +
> >> + device = original = originating = service = NULL;
> >> + }
> >> +
> >> +out:
> >> + CoTaskMemFree(device);
> >> + CoTaskMemFree(original);
> >> + CoTaskMemFree(originating);
> >> + CoTaskMemFree(service);
> >> + ReleaseSRWLockShared(&m_lock);
> >> + return hr;
> >> +}
> >> +
> >> +LONG QGAVssSnapshotRegistry::RemoveSet(VSS_ID set_id)
> >> +{
> >> + LONG removed;
> >> +
> >> + AcquireSRWLockExclusive(&m_lock);
> >> + auto it = std::remove_if(m_entries.begin(), m_entries.end(),
> >> + [&](const Entry &e) {
> >> + return IsEqualGUID(e.set_id, set_id);
> >> + });
> >> + removed = m_entries.end() - it;
> >> + m_entries.erase(it, m_entries.end());
> >> + ReleaseSRWLockExclusive(&m_lock);
> >> + return removed;
> >> +}
> >> +
> >> +bool QGAVssSnapshotRegistry::Remove(VSS_ID snapshot_id)
> >> +{
> >> + bool found;
> >> +
> >> + AcquireSRWLockExclusive(&m_lock);
> >> + auto it = std::find_if(m_entries.begin(), m_entries.end(),
> >> + [&](const Entry &e) {
> >> + return IsEqualGUID(e.snapshot_id, snapshot_id);
> >> + });
> >> + found = it != m_entries.end();
> >> + if (found) {
> >> + m_entries.erase(it);
> >> + }
> >> + ReleaseSRWLockExclusive(&m_lock);
> >> + return found;
> >> +}
> >>
> >> /* QGAVssProvider */
> >>
> >> @@ -266,7 +435,7 @@ STDMETHODIMP CQGAVssProvider::SetContext(LONG
> lContext)
> >> STDMETHODIMP CQGAVssProvider::GetSnapshotProperties(
> >> VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp)
> >> {
> >> - return S_OK;
> >> + return qga_vss_registry.GetProperties(SnapshotId, pProp);
> >> }
> >>
> >> STDMETHODIMP CQGAVssProvider::Query(
> >> @@ -286,6 +455,27 @@ STDMETHODIMP CQGAVssProvider::DeleteSnapshots(
> >> VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType,
> >> BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID
> *pNondeletedSnapshotID)
> >> {
> >> + LONG deleted = 0;
> >> +
> >> + switch (eSourceObjectType) {
> >> + case VSS_OBJECT_SNAPSHOT:
> >> + if (qga_vss_registry.Remove(SourceObjectId)) {
> >> + deleted = 1;
> >> + }
> >> + break;
> >> + case VSS_OBJECT_SNAPSHOT_SET:
> >> + deleted = qga_vss_registry.RemoveSet(SourceObjectId);
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> + if (deleted > 0) {
> >> + *plDeletedSnapshots = deleted;
> >> + *pNondeletedSnapshotID = GUID_NULL;
> >> + return S_OK;
> >> + }
> >> +
> >> *plDeletedSnapshots = 0;
> >> *pNondeletedSnapshotID = SourceObjectId;
> >> return S_OK;
> >
> >
> > return VSS_E_OBJECT_NOT_FOUND in this case looks more correct
>
> Possibly, but I didn't want to change the existing behaviour here
> without good reasons (it was fixed in 2014 by commit
> d9e1f574cb6eac0a3a2f97b67d2e7a3ad9c1dc95).
>
> >
> >>
> >> @@ -295,6 +485,13 @@ STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot(
> >> VSS_ID SnapshotSetId, VSS_ID SnapshotId,
> >> VSS_PWSZ pwszVolumeName, LONG lNewContext)
> >> {
> >> + if (!pwszVolumeName) {
> >> + return E_INVALIDARG;
> >> + }
> >> + if (!qga_vss_registry.Track(SnapshotSetId, SnapshotId,
> >> + pwszVolumeName, lNewContext)) {
> >> + return E_OUTOFMEMORY;
> >> + }
> >> return S_OK;
> >> }
> >>
> >> @@ -411,6 +608,7 @@ STDMETHODIMP
> CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId)
> >>
> >> STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId)
> >> {
> >> + qga_vss_registry.RemoveSet(SnapshotSetId);
> >> return S_OK;
> >> }
> >>
> >> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> >> index 1b0155c691ef..565eefcfb283 100644
> >> --- a/qga/vss-win32/requester.cpp
> >> +++ b/qga/vss-win32/requester.cpp
> >> @@ -52,6 +52,7 @@ static t_VssFreeSnapshotProperties
> pVssFreeSnapshotProperties;
> >> static struct QGAVSSContext {
> >> IVssBackupComponents *pVssbc; /* VSS requester interface */
> >> IVssAsync *pAsyncSnapshot; /* async info of VSS snapshot
> operation */
> >> + VSS_ID idSnapshotSet; /* snapshot set created for this
> cycle */
> >> HANDLE hEventFrozen; /* notify fs/writer freeze from
> provider */
> >> HANDLE hEventThaw; /* request provider to thaw */
> >> HANDLE hEventTimeout; /* notify timeout in provider */
> >> @@ -116,6 +117,7 @@ static void requester_cleanup(void)
> >> vss_ctx.pVssbc->Release();
> >> vss_ctx.pVssbc = NULL;
> >> }
> >> + vss_ctx.idSnapshotSet = GUID_NULL;
> >> vss_ctx.cFrozenVols = 0;
> >> qga_debug_end;
> >> }
> >> @@ -446,6 +448,7 @@ void requester_freeze(int *num_vols, void
> *mountpoints, ErrorSet *errset)
> >> err_set(errset, hr, "failed to start snapshot set");
> >> goto out;
> >> }
> >> + vss_ctx.idSnapshotSet = guidSnapshotSet;
> >>
> >> if (mountpoints) {
> >> PWCHAR volume_name_wchar;
> >> @@ -680,6 +683,29 @@ void requester_thaw(int *num_vols, void
> *mountpints, ErrorSet *errset)
> >> err_set(errset, hr, "failed to do snapshot set");
> >> }
> >>
> >> + /*
> >> + * The snapshot set is created in VSS_CTX_APP_ROLLBACK context, so
> >> + * it is persistent: if it was committed, VSS keeps it (and keeps
> >> + * querying providers about it) until it is explicitly deleted. The
> >> + * freeze/thaw cycle is over whatever the outcome above, so delete
> >> + * it now. Failure is not fatal here, only log it.
> >> + */
> >> + {
> >> + LONG lDeletedSnapshots = 0;
> >> + VSS_ID idNonDeletedSnapshot;
> >> + HRESULT hrDelete;
> >> +
> >> + hrDelete =
> vss_ctx.pVssbc->DeleteSnapshots(vss_ctx.idSnapshotSet,
> >> +
> VSS_OBJECT_SNAPSHOT_SET,
> >> + TRUE,
> >> + &lDeletedSnapshots,
> >> +
> &idNonDeletedSnapshot);
> >> + if (FAILED(hrDelete)) {
> >> + qga_debug("failed to delete snapshot set after thaw,"
> >> + " hr=0x%08lx", hrDelete);
> >> + }
> >> + }
> >> +
> >> if (err_is_set(errset)) {
> >> vss_ctx.pVssbc->AbortBackup();
> >> }
> >> --
> >> 2.56.0.116.g6de20f6092dc
> >>
>
>
© 2016 - 2026 Red Hat, Inc.