| Message ID | 20130606150645.10486.23215.stgit@hds.com |
|---|---|
| State | New |
| Headers | show |
I'm trying to wrap my head around the build changes first. On 06/06/13 17:06, Tomoki Sekiyama wrote: > Implements a basic stub of software VSS provider. Currently, this > module only provides a relay function of events between > qemu-guest-agent and Windows VSS when VSS finished filesystem freeze > and when qemu snapshot is done. > > In the future, this module could be extended to support the other VSS > functions, such as query for snapshot volumes and recovery. > > To build type library (.tlb) for qga-provider.dll from COM IDL (.idl), > VisualC++, MIDL and stdole2.tlb in Windows SDK are required. > > This patch also adds pre-compiled .tlb file in the repository in order > to enable cross-compile qemu-ga.exe for Windows with VSS support. > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> > --- > configure | 5 > Makefile | 2 > Makefile.objs | 2 > qga/Makefile.objs | 6 > qga/vss-win32-provider/Makefile.objs | 24 ++ > create mode 100644 qga/vss-win32-provider/Makefile.objs > diff --git a/configure b/configure > index cafe830..8e45fad 100755 > --- a/configure > +++ b/configure > @@ -3490,9 +3490,12 @@ if test "$softmmu" = yes ; then > virtfs=no > fi > fi > - if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then > + if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then > if [ "$guest_agent" = "yes" ]; then > tools="qemu-ga\$(EXESUF) $tools" > + if [ "$mingw32" = "yes" ]; then > + tools="qga/vss-win32-provider/qga-provider.dll qga/vss-win32-provider/qga-provider.tlb $tools" > + fi > fi > fi > fi This adds three targets to "tools" on mingw32 -- "qemu-ga" hasn't been there until now. Is this actually a small, unrelated bugfix? I'm asking because I build upstream qemu-ga.exe on my RHEL-6 laptop as follows -- I don't have pixman installed, and configure forces me to specify --disable-tools as well: $ ./configure --without-pixman --disable-system --disable-tools \ --cross-prefix=i686-pc-mingw32- $ make qemu-ga.exe Plain "make" after the above ./configure doesn't produce anything useful. Will I have to specify the DLL and TLB targets too on the command line? (Apologies, I can't try it out now, msiextract doesn't work for me.) > diff --git a/Makefile b/Makefile > index 4851ba0..636358d 100644 > --- a/Makefile > +++ b/Makefile > @@ -235,7 +235,7 @@ clean: > rm -f qemu-options.def > find . -name '*.[oda]' -type f -exec rm -f {} + > find . -name '*.l[oa]' -type f -exec rm -f {} + > - rm -f $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ > + rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ > rm -Rf .libs > rm -f qemu-img-cmds.h > @# May not be present in GENERATED_HEADERS OK I guess. > diff --git a/Makefile.objs b/Makefile.objs > index 286ce06..b6c1505 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -102,6 +102,7 @@ common-obj-y += disas/ > # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed > # by libqemuutil.a. These should be moved to a separate .json schema. > qga-obj-y = qga/ qapi-types.o qapi-visit.o > +qga-prv-obj-y = qga/ > > vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) > > @@ -113,6 +114,7 @@ nested-vars += \ > stub-obj-y \ > util-obj-y \ > qga-obj-y \ > + qga-prv-obj-y \ > block-obj-y \ > common-obj-y > dummy := $(call unnest-vars) What does this do? Does "qga-prv-obj-y" stand for "all objects under 'qga'"? Or does it mean "look into qga/Makefile.objs for further objects"? > diff --git a/qga/Makefile.objs b/qga/Makefile.objs > index b8d7cd0..8d93866 100644 > --- a/qga/Makefile.objs > +++ b/qga/Makefile.objs > @@ -3,3 +3,9 @@ qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o > qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o > qga-obj-y += qapi-generated/qga-qapi-types.o qapi-generated/qga-qapi-visit.o > qga-obj-y += qapi-generated/qga-qmp-marshal.o > + > +ifeq ($(CONFIG_QGA_VSS),y) > +QEMU_CFLAGS += -DHAS_VSS_SDK Isn't this superfluous? First, I can find no other reference to HAS_VSS_SDK in the series, second, CONFIG_QGA_VSS would be available directly as a macro to source files. > +qga-obj-y += vss-win32-provider/ > +qga-prv-obj-y += vss-win32-provider/ > +endif So we're probably just saying "look into vss-win32-provider/Makefile.objs for further object files", for both variables. > diff --git a/qga/vss-win32-provider/Makefile.objs b/qga/vss-win32-provider/Makefile.objs > new file mode 100644 > index 0000000..1fe1f8f > --- /dev/null > +++ b/qga/vss-win32-provider/Makefile.objs > @@ -0,0 +1,24 @@ > +# rules to build qga-provider.dll > + > +qga-obj-y += qga-provider.dll > +qga-prv-obj-y += provider.o install.o > + > +obj-qga-prv-obj-y = $(addprefix $(obj)/, $(qga-prv-obj-y)) > +$(obj-qga-prv-obj-y): QEMU_CXXFLAGS = $(filter-out -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wold-style-declaration -Wold-style-definition -Wredundant-decls -fstack-protector-all, $(QEMU_CFLAGS)) -Wno-unknown-pragmas -Wno-delete-non-virtual-dtor > + > +$(obj)/qga-provider.dll: LDFLAGS = -shared -Wl,--add-stdcall-alias,--enable-stdcall-fixup -lole32 -loleaut32 -lshlwapi -luuid -static > +$(obj)/qga-provider.dll: $(obj-qga-prv-obj-y) $(SRC_PATH)/$(obj)/qga-provider.def $(obj)/qga-provider.tlb > + $(call quiet-command,$(CXX) -o $@ $(qga-prv-obj-y) $(SRC_PATH)/qga/vss-win32-provider/qga-provider.def $(CXXFLAGS) $(LDFLAGS)," LINK $(TARGET_DIR)$@") I'm having a hard time following this. Looks like "qga-prv-obj-y" consists of nothing more than "provider.o" and "install.o", and that "qga-provider.dll" depends on them. In theory, would it be possible *not* to introduce "qga-prv-obj-y" in the higher-level Makefile.obj files, only here? Why does "qga-provider.dll" depend on ""qga-provider.tlb"? It is not used in the link command. Also, why is it necessary to collect the files "provider.o", "install.o" and "qga-provider.def" into a DLL, and link qemu-ga.exe against that DLL (via qga-obj-y)? Is this a VSS requirement? Can't we simply link these objects into qemu-ga.exe statically, like the rest of "qga-obj-y"? Will some VSS service load the provider DLL independently of qemu-ga.exe? If so, (a) how will they communicate, (b) shouldn't "qga-obj-y" be independent of "qga-provider.dll" (and that would explain the high-level introduction of "qga-prv-obj-y")? > + > + > +# rules to build qga-provider.tlb > +# Currently, only native build is supported because building .tlb > +# (TypeLibrary) from .idl requires WindowsSDK and MIDL (and cl.exe in VC++). > +MIDL=$(WIN_SDK)/Bin/midl > + > +$(obj)/qga-provider.tlb: $(SRC_PATH)/$(obj)/qga-provider.idl > +ifeq ($(wildcard $(SRC_PATH)/$(obj)/qga-provider.tlb),) > + $(call quiet-command,$(MIDL) -tlb $@ -I $(WIN_SDK)/Include $<," MIDL $(TARGET_DIR)$@") > +else > + $(call quiet-command,cp $(dir $<)qga-provider.tlb $@, " COPY $(TARGET_DIR)$@") > +endif Could you please draw a dependency graph between these files? Like idl --> tlb def --> dll <-- (provider.o, install.o) Does one of "tlb" and "dll" depend on the other, or are they needed "in parallel"? ... I'll try to continue here. I've peaked forward a little bit, and CQGAVssProvider::CommitSnapshots() seems to be the central method. It kicks the "frozen" event, then blocks until the "thaw" event is kicked back. I wonder how those will translate to QMP communication with libvirt. Thanks! Laszlo > qga/vss-win32-provider.h | 26 ++ > qga/vss-win32-provider/install.cpp | 493 +++++++++++++++++++++++++++++++ > qga/vss-win32-provider/provider.cpp | 479 ++++++++++++++++++++++++++++++ > qga/vss-win32-provider/qga-provider.def | 10 + > qga/vss-win32-provider/qga-provider.idl | 20 + > qga/vss-win32-provider/qga-provider.tlb | Bin > qga/vss-win32.h | 86 +++++ > 12 files changed, 1151 insertions(+), 2 deletions(-) > create mode 100644 qga/vss-win32-provider.h > create mode 100644 qga/vss-win32-provider/install.cpp > create mode 100644 qga/vss-win32-provider/provider.cpp > create mode 100644 qga/vss-win32-provider/qga-provider.def > create mode 100644 qga/vss-win32-provider/qga-provider.idl > create mode 100644 qga/vss-win32-provider/qga-provider.tlb > create mode 100644 qga/vss-win32.h > > diff --git a/qga/vss-win32-provider.h b/qga/vss-win32-provider.h > new file mode 100644 > index 0000000..a437e71 > --- /dev/null > +++ b/qga/vss-win32-provider.h > @@ -0,0 +1,26 @@ > +/* > + * QEMU Guest Agent win32 VSS provider declarations > + * > + * Copyright Hitachi Data Systems Corp. 2013 > + * > + * Authors: > + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#ifndef VSS_WIN32_PROVIDER_H > +#define VSS_WIN32_PROVIDER_H > + > +#include <windows.h> > + > +STDAPI VSSCheckOSVersion(void); > + > +STDAPI COMRegister(void); > +STDAPI COMUnregister(void); > + > +STDAPI DllRegisterServer(void); > +STDAPI DllUnregisterServer(void); > + > +#endif > diff --git a/qga/vss-win32-provider/install.cpp b/qga/vss-win32-provider/install.cpp > new file mode 100644 > index 0000000..d6f1d55 > --- /dev/null > +++ b/qga/vss-win32-provider/install.cpp > @@ -0,0 +1,493 @@ > +/* > + * QEMU Guest Agent win32 VSS Provider installer > + * > + * Copyright Hitachi Data Systems Corp. 2013 > + * > + * Authors: > + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#include <stdio.h> > +#include <string.h> > + > +#include "../vss-win32.h" > +#include "inc/win2003/vscoordint.h" > +#include "../vss-win32-provider.h" > + > +#include <comadmin.h> > +#include <wbemidl.h> > +#include <comutil.h> > + > +extern HINSTANCE g_hinstDll; > + > +const GUID CLSID_COMAdminCatalog = { 0xF618C514, 0xDFB8, 0x11d1, > + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; > +const GUID IID_ICOMAdminCatalog = { 0xDD662187, 0xDFC2, 0x11d1, > + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; > +const GUID CLSID_WbemLocator = { 0x4590f811, 0x1d3a, 0x11d0, > + {0x89, 0x1f, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; > +const GUID IID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf, > + {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; > + > +static void errmsg(DWORD err, const char *text) > +{ > + char *msg = NULL, *nul = strchr(text, '('); > + int len = nul ? nul - text : -1; > + > + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | > + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, > + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), > + (char *)&msg, 0, NULL); > + printf("%.*s. (Error: %lx) %s\n", len, text, err, msg); > + LocalFree(msg); > +} > + > +static void errmsg_dialog(DWORD err, const char *text, const char *opt = "") > +{ > + char *msg, buf[512]; > + > + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | > + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, > + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), > + (char *)&msg, 0, NULL); > + snprintf(buf, sizeof(buf), "%s%s. (Error: %lx) %s\n", text, opt, err, msg); > + MessageBox(NULL, buf, "Error from " QGA_PROVIDER_NAME, MB_OK|MB_ICONERROR); > + LocalFree(msg); > +} > + > +#define _chk(hr, status, msg, err_label) \ > + do { \ > + hr = (status); \ > + if (FAILED(hr)) { \ > + errmsg(hr, msg); \ > + goto err_label; \ > + } \ > + } while (0) > + > +#define chk(status) _chk(hr, status, "Failed to " #status, out) > + > +template<class T> > +HRESULT put_Value(ICatalogObject *pObj, LPCWSTR name, T val) > +{ > + return pObj->put_Value(_bstr_t(name), _variant_t(val)); > +} > + > +/* Check whether this OS version supports VSS providers */ > +STDAPI VSSCheckOSVersion(void) > +{ > + OSVERSIONINFO OSver; > + > + OSver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); > + GetVersionEx(&OSver); > + if ((OSver.dwMajorVersion == 5 && OSver.dwMinorVersion >= 2) || > + OSver.dwMajorVersion > 5) { > + return S_OK; > + } > + return S_FALSE; > +} > + > +/* Lookup Administrators group name from winmgmt */ > +static HRESULT GetAdminName(_bstr_t &name) > +{ > + HRESULT hr; > + IWbemLocator *pLoc = NULL; > + IWbemServices *pSvc = NULL; > + IEnumWbemClassObject *pEnum = NULL; > + IWbemClassObject *pWobj = NULL; > + ULONG returned; > + _variant_t var; > + > + chk(CoCreateInstance(CLSID_WbemLocator, NULL, CLSCTX_INPROC_SERVER, > + IID_IWbemLocator, (LPVOID *)&pLoc)); > + chk(pLoc->ConnectServer(_bstr_t(L"ROOT\\CIMV2"), NULL, NULL, NULL, > + 0, 0, 0, &pSvc)); > + chk(CoSetProxyBlanket(pSvc, RPC_C_AUTHN_WINNT, RPC_C_AUTHZ_NONE, > + NULL, RPC_C_AUTHN_LEVEL_CALL, > + RPC_C_IMP_LEVEL_IMPERSONATE, NULL, EOAC_NONE)); > + chk(pSvc->ExecQuery(_bstr_t(L"WQL"), > + _bstr_t(L"select * from Win32_Account where " > + "SID='S-1-5-32-544' and localAccount=TRUE"), > + WBEM_FLAG_RETURN_IMMEDIATELY | WBEM_FLAG_FORWARD_ONLY, > + NULL, &pEnum)); > + if (!pEnum) { > + errmsg(E_FAIL, "Failed to query for Administrators"); > + goto out; > + } > + chk(pEnum->Next(WBEM_INFINITE, 1, &pWobj, &returned)); > + if (returned == 0) { > + errmsg(E_FAIL, "Failed to query for Administrators"); > + goto out; > + } > + > + chk(pWobj->Get(_bstr_t(L"Name"), 0, &var, 0, 0)); > + name = var; > +out: > + if (pLoc) { > + pLoc->Release(); > + } > + if (pSvc) { > + pSvc->Release(); > + } > + if (pEnum) { > + pEnum->Release(); > + } > + if (pWobj) { > + pWobj->Release(); > + } > + return hr; > +} > + > +/* Register this module to COM+ Applications Catalog */ > +STDAPI COMRegister(void) > +{ > + HRESULT hr = E_FAIL; > + IUnknown *pUnknown = NULL; > + ICOMAdminCatalog *pCatalog = NULL; > + ICatalogCollection *pApps = NULL, *pRoles = NULL, *pUsersInRole = NULL; > + ICatalogObject *pObj = NULL; > + long n; > + _bstr_t name; > + _variant_t key; > + CHAR dllPath[MAX_PATH], tlbPath[MAX_PATH]; > + > + if (!g_hinstDll) { > + errmsg(E_FAIL, "Failed to initialize DLL"); > + goto out; > + } > + > + if (VSSCheckOSVersion() == S_FALSE) { > + printf("VSS provider is not supported in this OS version.\n"); > + return S_FALSE; /* VSS feature is disabled */ > + } > + > + COMUnregister(); > + > + chk(CoInitialize(NULL)); > + chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER, > + IID_IUnknown, (void **)&pUnknown)); > + chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog, (void **)&pCatalog)); > + > + /* Install COM+ Component */ > + > + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), > + (IDispatch **)&pApps)); > + chk(pApps->Populate()); > + chk(pApps->Add((IDispatch **)&pObj)); > + chk(put_Value(pObj, L"Name", QGA_PROVIDER_LNAME)); > + chk(put_Value(pObj, L"Description", QGA_PROVIDER_LNAME)); > + chk(put_Value(pObj, L"ApplicationAccessChecksEnabled", true)); > + chk(put_Value(pObj, L"Authentication", short(6))); > + chk(put_Value(pObj, L"AuthenticationCapability", short(2))); > + chk(put_Value(pObj, L"ImpersonationLevel", short(2))); > + chk(pApps->SaveChanges(&n)); > + chk(pObj->get_Key(&key)); > + > + pObj->Release(); > + pObj = NULL; > + > + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { > + errmsg(GetLastError(), "GetModuleFileName failed"); > + goto out; > + } > + n = strlen(dllPath); > + if (n < 3) { > + errmsg(E_FAIL, "Failed to lookup dll"); > + } > + strcpy(tlbPath, dllPath); > + strcpy(tlbPath+n-3, "TLB"); > + printf("Registering " QGA_PROVIDER_NAME ":\n"); > + printf(" %s\n", dllPath); > + printf(" %s\n", tlbPath); > + if (!PathFileExists(tlbPath)) { > + errmsg(ERROR_FILE_NOT_FOUND, "Failed to lookup tlb"); > + goto out; > + } > + > + chk(pCatalog->InstallComponent(_bstr_t(QGA_PROVIDER_LNAME), > + _bstr_t(dllPath), _bstr_t(tlbPath), > + _bstr_t(""))); > + > + /* Setup roles of the applicaion */ > + > + chk(pApps->GetCollection(_bstr_t(L"Roles"), key, > + (IDispatch **)&pRoles)); > + chk(pRoles->Populate()); > + chk(pRoles->Add((IDispatch **)&pObj)); > + chk(put_Value(pObj, L"Name", L"Administrators")); > + chk(put_Value(pObj, L"Description", L"Administrators group")); > + chk(pRoles->SaveChanges(&n)); > + chk(pObj->get_Key(&key)); > + > + pObj->Release(); > + pObj = NULL; > + > + /* Setup users in the role */ > + > + chk(pRoles->GetCollection(_bstr_t(L"UsersInRole"), key, > + (IDispatch **)&pUsersInRole)); > + chk(pUsersInRole->Populate()); > + > + chk(pUsersInRole->Add((IDispatch **)&pObj)); > + chk(GetAdminName(name)); > + chk(put_Value(pObj, L"User", _bstr_t(".\\") + name)); > + > + pObj->Release(); > + pObj = NULL; > + > + chk(pUsersInRole->Add((IDispatch **)&pObj)); > + chk(put_Value(pObj, L"User", L"SYSTEM")); > + chk(pUsersInRole->SaveChanges(&n)); > + > +out: > + if (pUnknown) { > + pUnknown->Release(); > + } > + if (pCatalog) { > + pCatalog->Release(); > + } > + if (pApps) { > + pApps->Release(); > + } > + if (pRoles) { > + pRoles->Release(); > + } > + if (pUsersInRole) { > + pUsersInRole->Release(); > + } > + if (pObj) { > + pObj->Release(); > + } > + > + if (FAILED(hr)) { > + COMUnregister(); > + } > + CoUninitialize(); > + > + return hr; > +} > + > +/* Unregister this module from COM+ Applications Catalog */ > +STDAPI COMUnregister(void) > +{ > + HRESULT hr; > + IUnknown *pUnknown = NULL; > + ICOMAdminCatalog *pCatalog = NULL; > + ICatalogCollection *pColl = NULL; > + ICatalogObject *pObj = NULL; > + _variant_t var; > + long i, n; > + > + if (VSSCheckOSVersion() == S_FALSE) { > + printf("VSS provider is not supported in this OS version.\n"); > + return S_FALSE; /* VSS feature is disabled */ > + } > + > + chk(DllUnregisterServer()); > + > + chk(CoInitialize(NULL)); > + chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER, > + IID_IUnknown, (void **)&pUnknown)); > + chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog, (void **)&pCatalog)); > + > + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), > + (IDispatch **)&pColl)); > + chk(pColl->Populate()); > + > + chk(pColl->get_Count(&n)); > + for (i = n - 1; i >= 0; i--) { > + chk(pColl->get_Item(i, (IDispatch **)&pObj)); > + chk(pObj->get_Value(_bstr_t(L"Name"), &var)); > + if (var == _variant_t(QGA_PROVIDER_LNAME)) { > + printf("Removing COM+ Application: %S\n", (wchar_t *)_bstr_t(var)); > + chk(pColl->Remove(i)); > + } > + } > + chk(pColl->SaveChanges(&n)); > + > +out: > + if (pUnknown) { > + pUnknown->Release(); > + } > + if (pCatalog) { > + pCatalog->Release(); > + } > + if (pColl) { > + pColl->Release(); > + } > + if (pObj) { > + pObj->Release(); > + } > + CoUninitialize(); > + > + return hr; > +} > + > + > +static BOOL CreateRegistryKey(LPCTSTR key, LPCTSTR value, LPCTSTR data) > +{ > + HKEY hKey; > + LONG ret; > + DWORD size; > + > + ret = RegCreateKeyEx(HKEY_CLASSES_ROOT, key, 0, NULL, > + REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, &hKey, NULL); > + if (ret != ERROR_SUCCESS) { > + goto out; > + } > + > + if (data != NULL) { > + size = (lstrlen(data) + 1) * sizeof(TCHAR); > + } else { > + size = 0; > + } > + > + ret = RegSetValueEx(hKey, value, 0, REG_SZ, (LPBYTE)data, size); > + RegCloseKey(hKey); > + > +out: > + if (ret != ERROR_SUCCESS) { > + /* We cannot printf here, and need MessageBox to report an error. */ > + errmsg_dialog(ret, "Cannot add registry ", key); > + return FALSE; > + } > + return TRUE; > +} > + > +/* Register this dll as a VSS provider */ > +STDAPI DllRegisterServer(void) > +{ > + IVssAdmin *pVssAdmin = NULL; > + HRESULT hr = E_FAIL; > + char dllPath[MAX_PATH]; > + char key[256]; > + > + CoInitialize(NULL); > + > + if (!g_hinstDll) { > + errmsg_dialog(hr, "Module instance is not available"); > + goto out; > + } > + > + /* Add this module to registery */ > + > + sprintf(key, "CLSID\\%s", g_szClsid); > + if (!CreateRegistryKey(key, NULL, g_szClsid)) { > + goto out; > + } > + > + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { > + errmsg_dialog(GetLastError(), "GetModuleFileName failed"); > + goto out; > + } > + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); > + if (!CreateRegistryKey(key, NULL, dllPath)) { > + goto out; > + } > + > + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); > + if (!CreateRegistryKey(key, "ThreadingModel", "Apartment")) { > + goto out; > + } > + > + sprintf(key, "CLSID\\%s\\ProgID", g_szClsid); > + if (!CreateRegistryKey(key, NULL, g_szProgid)) { > + goto out; > + } > + > + if (!CreateRegistryKey(g_szProgid, NULL, QGA_PROVIDER_NAME)) { > + goto out; > + } > + > + sprintf(key, "%s\\CLSID", g_szProgid); > + if (!CreateRegistryKey(key, NULL, g_szClsid)) { > + goto out; > + } > + > + hr = CoCreateInstance(CLSID_VSSCoordinator, > + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); > + if (FAILED(hr)) { > + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); > + goto out; > + } > + > + hr = pVssAdmin->RegisterProvider( > + g_gProviderId, CLSID_QGAVSSProvider, > + const_cast<WCHAR*>(QGA_PROVIDER_LNAME), VSS_PROV_SOFTWARE, > + const_cast<WCHAR*>(QGA_PROVIDER_VERSION), g_gProviderVersion); > + if (FAILED(hr)) { > + errmsg_dialog(hr, "RegisterProvider failed"); > + goto out; > + } > + > +out: > + if (pVssAdmin) { > + pVssAdmin->Release(); > + } > + CoUninitialize(); > + > + if (FAILED(hr)) { > + DllUnregisterServer(); > + } > + > + return hr; > +} > + > +/* Unregister this VSS hardware provider from the system */ > +STDAPI DllUnregisterServer(void) > +{ > + TCHAR key[256]; > + IVssAdmin *pVssAdmin = NULL; > + > + CoInitialize(NULL); > + > + HRESULT hr = CoCreateInstance(CLSID_VSSCoordinator, > + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); > + if (SUCCEEDED(hr)) { > + hr = pVssAdmin->UnregisterProvider(g_gProviderId); > + pVssAdmin->Release(); > + } else { > + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); > + } > + > + sprintf(key, "CLSID\\%s", g_szClsid); > + SHDeleteKey(HKEY_CLASSES_ROOT, key); > + SHDeleteKey(HKEY_CLASSES_ROOT, g_szProgid); > + > + CoUninitialize(); > + > + return S_OK; /* Uninstall should never fail */ > +} > + > + > +/* Support functions for _bstr_t in MinGW: Originally written by: Diaa Sami */ > + > +void __stdcall _com_issue_error(HRESULT hr) > +{ > + printf("_com_issue_error() called with parameter HRESULT = %lu", hr); > +} > + > +namespace _com_util > +{ > + char * __stdcall ConvertBSTRToString(BSTR bstr) > + { > + const unsigned int stringLength = lstrlenW(bstr); > + char *const ascii = new char [stringLength + 1]; > + > + wcstombs(ascii, bstr, stringLength + 1); > + > + return ascii; > + } > + > + BSTR __stdcall ConvertStringToBSTR(const char *const ascii) > + { > + const unsigned int stringLength = lstrlenA(ascii); > + BSTR bstr = SysAllocStringLen(NULL, stringLength); > + > + mbstowcs(bstr, ascii, stringLength + 1); > + > + return bstr; > + } > +} > diff --git a/qga/vss-win32-provider/provider.cpp b/qga/vss-win32-provider/provider.cpp > new file mode 100644 > index 0000000..90a3d25 > --- /dev/null > +++ b/qga/vss-win32-provider/provider.cpp > @@ -0,0 +1,479 @@ > +/* > + * QEMU Guest Agent win32 VSS Provider implementations > + * > + * Copyright Hitachi Data Systems Corp. 2013 > + * > + * Authors: > + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#include <stdio.h> > +#include "../vss-win32.h" > +#include "inc/win2003/vscoordint.h" > +#include "inc/win2003/vsprov.h" > + > +static long g_nComObjsInUse; > +HINSTANCE g_hinstDll; > + > +/* VSS common GUID's */ > + > +const CLSID CLSID_VSSCoordinator = { 0xE579AB5F, 0x1CC4, 0x44b4, > + {0xBE, 0xD9, 0xDE, 0x09, 0x91, 0xFF, 0x06, 0x23} }; > +const IID IID_IVssAdmin = { 0x77ED5996, 0x2F63, 0x11d3, > + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; > + > +const IID IID_IVssHardwareSnapshotProvider = { 0x9593A157, 0x44E9, 0x4344, > + {0xBB, 0xEB, 0x44, 0xFB, 0xF9, 0xB0, 0x6B, 0x10} }; > +const IID IID_IVssSoftwareSnapshotProvider = { 0x609e123e, 0x2c5a, 0x44d3, > + {0x8f, 0x01, 0x0b, 0x1d, 0x9a, 0x47, 0xd1, 0xff} }; > +const IID IID_IVssProviderCreateSnapshotSet = { 0x5F894E5B, 0x1E39, 0x4778, > + {0x8E, 0x23, 0x9A, 0xBA, 0xD9, 0xF0, 0xE0, 0x8C} }; > +const IID IID_IVssProviderNotifications = { 0xE561901F, 0x03A5, 0x4afe, > + {0x86, 0xD0, 0x72, 0xBA, 0xEE, 0xCE, 0x70, 0x04} }; > + > +const IID IID_IVssEnumObject = { 0xAE1C7110, 0x2F60, 0x11d3, > + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; > + > + > +void LockModule(BOOL block) > +{ > + if (block) { > + InterlockedIncrement(&g_nComObjsInUse); > + } else { > + InterlockedDecrement(&g_nComObjsInUse); > + } > +} > + > +/* Empty enumerator for VssObject */ > + > +class CQGAVSSEnumObject : public IVssEnumObject > +{ > +public: > + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); > + STDMETHODIMP_(ULONG) AddRef(); > + STDMETHODIMP_(ULONG) Release(); > + > + /* IVssEnumObject Methods */ > + STDMETHODIMP Next( > + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched); > + STDMETHODIMP Skip(ULONG celt); > + STDMETHODIMP Reset(void); > + STDMETHODIMP Clone(IVssEnumObject **ppenum); > + > + /* CQGAVSSEnumObject Methods */ > + CQGAVSSEnumObject(); > + ~CQGAVSSEnumObject(); > + > +private: > + long m_nRefCount; > +}; > + > +CQGAVSSEnumObject::CQGAVSSEnumObject() > +{ > + m_nRefCount = 0; > + LockModule(TRUE); > +} > + > +CQGAVSSEnumObject::~CQGAVSSEnumObject() > +{ > + LockModule(FALSE); > +} > + > +STDMETHODIMP CQGAVSSEnumObject::QueryInterface(REFIID riid, void **ppObj) > +{ > + if (riid == IID_IUnknown || riid == IID_IVssEnumObject) { > + *ppObj = static_cast<void*>(static_cast<IVssEnumObject*>(this)); > + AddRef(); > + return S_OK; > + } > + ppObj = NULL; > + return E_NOINTERFACE; > +} > + > +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::AddRef() > +{ > + return InterlockedIncrement(&m_nRefCount); > +} > + > +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::Release() > +{ > + long nRefCount = InterlockedDecrement(&m_nRefCount); > + if (m_nRefCount == 0) { > + delete this; > + } > + return nRefCount; > +} > + > +STDMETHODIMP CQGAVSSEnumObject::Next( > + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched) > +{ > + *pceltFetched = 0; > + return S_FALSE; > +} > + > +STDMETHODIMP CQGAVSSEnumObject::Skip(ULONG celt) > +{ > + return S_FALSE; > +} > + > +STDMETHODIMP CQGAVSSEnumObject::Reset(void) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVSSEnumObject::Clone(IVssEnumObject **ppenum) > +{ > + return E_NOTIMPL; > +} > + > + > +/* QGAVssProvider */ > + > +class CQGAVssProvider : > + public IVssSoftwareSnapshotProvider, > + public IVssProviderCreateSnapshotSet, > + public IVssProviderNotifications > +{ > +public: > + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); > + STDMETHODIMP_(ULONG) AddRef(); > + STDMETHODIMP_(ULONG) Release(); > + > + /* IVssSoftwareSnapshotProvider Methods */ > + STDMETHODIMP SetContext(LONG lContext); > + STDMETHODIMP GetSnapshotProperties( > + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp); > + STDMETHODIMP Query( > + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, > + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum); > + STDMETHODIMP DeleteSnapshots( > + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, > + BOOL bForceDelete, LONG *plDeletedSnapshots, > + VSS_ID *pNondeletedSnapshotID); > + STDMETHODIMP BeginPrepareSnapshot( > + VSS_ID SnapshotSetId, VSS_ID SnapshotId, > + VSS_PWSZ pwszVolumeName, LONG lNewContext); > + STDMETHODIMP IsVolumeSupported( > + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider); > + STDMETHODIMP IsVolumeSnapshotted( > + VSS_PWSZ pwszVolumeName, BOOL *pbSnapshotsPresent, > + LONG *plSnapshotCompatibility); > + STDMETHODIMP SetSnapshotProperty( > + VSS_ID SnapshotId, VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, > + VARIANT vProperty); > + STDMETHODIMP RevertToSnapshot(VSS_ID SnapshotId); > + STDMETHODIMP QueryRevertStatus(VSS_PWSZ pwszVolume, IVssAsync **ppAsync); > + > + /* IVssProviderCreateSnapshotSet Methods */ > + STDMETHODIMP EndPrepareSnapshots(VSS_ID SnapshotSetId); > + STDMETHODIMP PreCommitSnapshots(VSS_ID SnapshotSetId); > + STDMETHODIMP CommitSnapshots(VSS_ID SnapshotSetId); > + STDMETHODIMP PostCommitSnapshots( > + VSS_ID SnapshotSetId, LONG lSnapshotsCount); > + STDMETHODIMP PreFinalCommitSnapshots(VSS_ID SnapshotSetId); > + STDMETHODIMP PostFinalCommitSnapshots(VSS_ID SnapshotSetId); > + STDMETHODIMP AbortSnapshots(VSS_ID SnapshotSetId); > + > + /* IVssProviderNotifications Methods */ > + STDMETHODIMP OnLoad(IUnknown *pCallback); > + STDMETHODIMP OnUnload(BOOL bForceUnload); > + > + /* CQGAVssProvider Methods */ > + CQGAVssProvider(); > + ~CQGAVssProvider(); > + > +private: > + long m_nRefCount; > +}; > + > +CQGAVssProvider::CQGAVssProvider() > +{ > + m_nRefCount = 0; > + LockModule(TRUE); > +} > + > +CQGAVssProvider::~CQGAVssProvider() > +{ > + LockModule(FALSE); > +} > + > +STDMETHODIMP CQGAVssProvider::QueryInterface(REFIID riid, void **ppObj) > +{ > + if (riid == IID_IUnknown) { > + *ppObj = static_cast<void*>(this); > + AddRef(); > + return S_OK; > + } else if (riid == IID_IVssSoftwareSnapshotProvider) { > + *ppObj = static_cast<void*>( > + static_cast<IVssSoftwareSnapshotProvider*>(this)); > + AddRef(); > + return S_OK; > + } else if (riid == IID_IVssProviderCreateSnapshotSet) { > + *ppObj = static_cast<void*>( > + static_cast<IVssProviderCreateSnapshotSet*>(this)); > + AddRef(); > + return S_OK; > + } else if (riid == IID_IVssProviderNotifications) { > + *ppObj = static_cast<void*>( > + static_cast<IVssProviderNotifications*>(this)); > + AddRef(); > + return S_OK; > + } > + *ppObj = NULL; > + return E_NOINTERFACE; > +} > + > +STDMETHODIMP_(ULONG) CQGAVssProvider::AddRef() > +{ > + return InterlockedIncrement(&m_nRefCount); > +} > + > +STDMETHODIMP_(ULONG) CQGAVssProvider::Release() > +{ > + long nRefCount = InterlockedDecrement(&m_nRefCount); > + if (m_nRefCount == 0) { > + delete this; > + } > + return nRefCount; > +} > + > + > +/* > + * IVssSoftwareSnapshotProvider methods > + */ > + > +STDMETHODIMP CQGAVssProvider::SetContext(LONG lContext) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::GetSnapshotProperties( > + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp) > +{ > + return VSS_E_OBJECT_NOT_FOUND; > +} > + > +STDMETHODIMP CQGAVssProvider::Query( > + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, > + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum) > +{ > + *ppEnum = new CQGAVSSEnumObject; > + (*ppEnum)->AddRef(); > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::DeleteSnapshots( > + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, > + BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID *pNondeletedSnapshotID) > +{ > + return E_NOTIMPL; > +} > + > +STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot( > + VSS_ID SnapshotSetId, VSS_ID SnapshotId, > + VSS_PWSZ pwszVolumeName, LONG lNewContext) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::IsVolumeSupported( > + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider) > +{ > + *pbSupportedByThisProvider = TRUE; > + > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::IsVolumeSnapshotted(VSS_PWSZ pwszVolumeName, > + BOOL *pbSnapshotsPresent, LONG *plSnapshotCompatibility) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::SetSnapshotProperty(VSS_ID SnapshotId, > + VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, VARIANT vProperty) > +{ > + return E_NOTIMPL; > +} > + > +STDMETHODIMP CQGAVssProvider::RevertToSnapshot(VSS_ID SnapshotId) > +{ > + return E_NOTIMPL; > +} > + > +STDMETHODIMP CQGAVssProvider::QueryRevertStatus( > + VSS_PWSZ pwszVolume, IVssAsync **ppAsync) > +{ > + return S_OK; > +} > + > + > +/* > + * IVssProviderCreateSnapshotSet methods > + */ > + > +STDMETHODIMP CQGAVssProvider::EndPrepareSnapshots(VSS_ID SnapshotSetId) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::PreCommitSnapshots(VSS_ID SnapshotSetId) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::CommitSnapshots(VSS_ID SnapshotSetId) > +{ > + HRESULT hr = S_OK; > + HANDLE hEvent, hEvent2; > + > + hEvent = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_FROZEN); > + if (hEvent == INVALID_HANDLE_VALUE) { > + hr = E_FAIL; > + goto out; > + } > + > + hEvent2 = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_THAW); > + if (hEvent == INVALID_HANDLE_VALUE) { > + CloseHandle(hEvent); > + hr = E_FAIL; > + goto out; > + } > + > + SetEvent(hEvent); > + if (WaitForSingleObject(hEvent2, 60*1000) != WAIT_OBJECT_0) { > + hr = E_ABORT; > + } > + > + CloseHandle(hEvent2); > + CloseHandle(hEvent); > +out: > + return hr; > +} > + > +STDMETHODIMP CQGAVssProvider::PostCommitSnapshots( > + VSS_ID SnapshotSetId, LONG lSnapshotsCount) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::PreFinalCommitSnapshots(VSS_ID SnapshotSetId) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId) > +{ > + return S_OK; > +} > + > +/* > + * IVssProviderNotifications methods > + */ > + > +STDMETHODIMP CQGAVssProvider::OnLoad(IUnknown *pCallback) > +{ > + return S_OK; > +} > + > +STDMETHODIMP CQGAVssProvider::OnUnload(BOOL bForceUnload) > +{ > + return S_OK; > +} > + > + > +/* > + * CQGAVssProviderFactory class > + */ > + > +class CQGAVssProviderFactory : public IClassFactory > +{ > +public: > + STDMETHODIMP QueryInterface(REFIID riid, void **ppv); > + STDMETHODIMP_(ULONG) AddRef(); > + STDMETHODIMP_(ULONG) Release(); > + STDMETHODIMP CreateInstance( > + IUnknown *pUnknownOuter, REFIID iid, void **ppv); > + STDMETHODIMP LockServer(BOOL bLock) { return E_NOTIMPL; } > +private: > + long m_nRefCount; > +}; > + > +STDMETHODIMP CQGAVssProviderFactory::QueryInterface(REFIID riid, void **ppv) > +{ > + if (riid == IID_IUnknown || riid == IID_IClassFactory) { > + *ppv = static_cast<void*>(this); > + AddRef(); > + return S_OK; > + } > + *ppv = NULL; > + return E_NOINTERFACE; > +} > + > +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::AddRef() > +{ > + LockModule(TRUE); > + return 2; > +} > + > +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::Release() > +{ > + LockModule(FALSE); > + return 1; > +} > + > +STDMETHODIMP CQGAVssProviderFactory::CreateInstance( > + IUnknown *pUnknownOuter, REFIID iid, void **ppv) > +{ > + if (pUnknownOuter) { > + return CLASS_E_NOAGGREGATION; > + } > + CQGAVssProvider *pObj = new CQGAVssProvider; > + if (!pObj) { > + return E_OUTOFMEMORY; > + } > + return pObj->QueryInterface(iid, ppv); > +} > + > + > +/* > + * DLL functions > + */ > + > +STDAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv) > +{ > + static CQGAVssProviderFactory factory; > + > + *ppv = NULL; > + if (IsEqualCLSID(rclsid, CLSID_QGAVSSProvider)) { > + return factory.QueryInterface(riid, ppv); > + } > + return CLASS_E_CLASSNOTAVAILABLE; > +} > + > +STDAPI DllCanUnloadNow() > +{ > + return g_nComObjsInUse == 0 ? S_OK : S_FALSE; > +} > + > +EXTERN_C > +BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved) > +{ > + switch (dwReason) { > + > + case DLL_PROCESS_ATTACH: > + g_hinstDll = hinstDll; > + DisableThreadLibraryCalls(hinstDll); > + break; > + } > + > + return TRUE; > +} > diff --git a/qga/vss-win32-provider/qga-provider.def b/qga/vss-win32-provider/qga-provider.def > new file mode 100644 > index 0000000..9f3afc8 > --- /dev/null > +++ b/qga/vss-win32-provider/qga-provider.def > @@ -0,0 +1,10 @@ > +LIBRARY "QGA-PROVIDER.DLL" > + > +EXPORTS > + VSSCheckOSVersion PRIVATE > + COMRegister PRIVATE > + COMUnregister PRIVATE > + DllCanUnloadNow PRIVATE > + DllGetClassObject PRIVATE > + DllRegisterServer PRIVATE > + DllUnregisterServer PRIVATE > diff --git a/qga/vss-win32-provider/qga-provider.idl b/qga/vss-win32-provider/qga-provider.idl > new file mode 100644 > index 0000000..17abca0 > --- /dev/null > +++ b/qga/vss-win32-provider/qga-provider.idl > @@ -0,0 +1,20 @@ > +import "oaidl.idl"; > +import "ocidl.idl"; > + > +[ > + uuid(103B8142-6CE5-48A7-BDE1-794D3192FCF1), > + version(1.0), > + helpstring("QGAVSSProvider Type Library") > +] > +library QGAVSSHWProviderLib > +{ > + importlib("stdole2.tlb"); > + [ > + uuid(6E6A3492-8D4D-440C-9619-5E5D0CC31CA8), > + helpstring("QGAVSSProvider Class") > + ] > + coclass QGAVSSHWProvider > + { > + [default] interface IUnknown; > + }; > +}; > diff --git a/qga/vss-win32-provider/qga-provider.tlb b/qga/vss-win32-provider/qga-provider.tlb > new file mode 100644 > index 0000000000000000000000000000000000000000..226452a1861371ffe0cad1019cf90fdfdcd5ef49 > GIT binary patch > literal 1528 > zcmeYbb_-!*U}OLRP8Kl5;0UB3A_y8H!@$4<WGF*9|A9aP$W{R21|SCUVfs9Pj1;IC > zKahR`)X0Ox{{ZC6An~sN`2tA%H9-9hNPHcj{0byK4>OPh6a(1_GM|T)fx!kz-UG<D > zK;nbc0l9GX===tt`Vb`fD?q*q5+7YXI$u?Zf#C;G4-9~uhYKVCC4f!`hZ{(Z0*HVD > zkhwswGqAt}ki;hd*$F@lQbP%-z+r|5P#hGW*vvKniaRx03p~wP?y>h_rLW<nKOg@= > z6{hYgzgH7@QE<^MU>KC!yoBjb#vz`9Lwu4+R-SJ!kIOX4xLBUUGN9-NyTyP76j}@n > z2f!qQ8-xepfXD+7rW+{SKz4&@7#qX~^1#sn3O|tFK>%ciE<<riN`6kNkzPqoQh0bc > zNbM*XJ|Un0jALSb15^rEE6h+Y8tCpA798vm9#E8DmYI@T<dc~c4pSpwQKEn@FU<fE > zfvHyrsVqoU0O~4AEUE;iEfI8i=bXgi;_z?|20Ng!&PAz-C8;S2NtFt|o-RHLWvNBQ > znfZAN=6VJOdIqMZrV5EA3T{Q23NES13Py$shQ?OLW>&_Q3PuKoMqI)S5zj9Ngog_= > gXfrXehlhjmFbIJB4$8MKU>*YlD1Z9^F{m5{03Vre%>V!Z > > literal 0 > HcmV?d00001 > > diff --git a/qga/vss-win32.h b/qga/vss-win32.h > new file mode 100644 > index 0000000..21655cf > --- /dev/null > +++ b/qga/vss-win32.h > @@ -0,0 +1,86 @@ > +/* > + * QEMU Guest Agent win32 VSS common declarations > + * > + * Copyright Hitachi Data Systems Corp. 2013 > + * > + * Authors: > + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#ifndef VSS_WIN32_H > +#define VSS_WIN32_H > + > +#define __MIDL_user_allocate_free_DEFINED__ > +#include "config-host.h" > +#include <windows.h> > +#include <shlwapi.h> > + > +/* Reduce warnings to include vss.h */ > + > +/* Ignore annotations for MS IDE */ > +#define __in IN > +#define __out OUT > +#define __RPC_unique_pointer > +#define __RPC_string > +#define __RPC__deref_inout_opt > +#define __RPC__out > +#ifndef __RPC__out_ecount_part > +#define __RPC__out_ecount_part(x, y) > +#endif > +#define _declspec(x) > +#undef uuid > +#define uuid(x) > + > +/* Undef some duplicated error codes redefined in vss.h */ > +#undef VSS_E_BAD_STATE > +#undef VSS_E_PROVIDER_NOT_REGISTERED > +#undef VSS_E_PROVIDER_VETO > +#undef VSS_E_OBJECT_NOT_FOUND > +#undef VSS_E_VOLUME_NOT_SUPPORTED > +#undef VSS_E_VOLUME_NOT_SUPPORTED_BY_PROVIDER > +#undef VSS_E_OBJECT_ALREADY_EXISTS > +#undef VSS_E_UNEXPECTED_PROVIDER_ERROR > +#undef VSS_E_INVALID_XML_DOCUMENT > +#undef VSS_E_MAXIMUM_NUMBER_OF_VOLUMES_REACHED > +#undef VSS_E_MAXIMUM_NUMBER_OF_SNAPSHOTS_REACHED > + > +/* > + * VSS headers must be installed from Microsoft VSS SDK 7.2 available at: > + * http://www.microsoft.com/en-us/download/details.aspx?id=23490 > + */ > +#include "inc/win2003/vss.h" > + > +/* Macros to convert char definitions to wchar */ > +#define _L(a) L##a > +#define L(a) _L(a) > + > +/* Constants for QGA VSS Provider */ > + > +#define QGA_PROVIDER_NAME "QEMU Guest Agent VSS Provider" > +#define QGA_PROVIDER_LNAME L(QGA_PROVIDER_NAME) > +#define QGA_PROVIDER_VERSION L(QEMU_VERSION) > + > +#define EVENT_NAME_FROZEN "Global\\QGAVSSEvent-frozen" > +#define EVENT_NAME_THAW "Global\\QGAVSSEvent-thaw" > + > +const GUID g_gProviderId = { 0x3629d4ed, 0xee09, 0x4e0e, > + {0x9a, 0x5c, 0x6d, 0x8b, 0xa2, 0x87, 0x2a, 0xef} }; > +const GUID g_gProviderVersion = { 0x11ef8b15, 0xcac6, 0x40d6, > + {0x8d, 0x5c, 0x8f, 0xfc, 0x16, 0x3f, 0x24, 0xca} }; > + > +const CLSID CLSID_QGAVSSProvider = { 0x6e6a3492, 0x8d4d, 0x440c, > + {0x96, 0x19, 0x5e, 0x5d, 0x0c, 0xc3, 0x1c, 0xa8} }; > + > +const TCHAR g_szClsid[] = TEXT("{6E6A3492-8D4D-440C-9619-5E5D0CC31CA8}"); > +const TCHAR g_szProgid[] = TEXT("QGAVSSProvider"); > + > +/* Enums undefined in VSS SDK 7.2 but defined in newer Windows SDK */ > +enum __VSS_VOLUME_SNAPSHOT_ATTRIBUTES { > + VSS_VOLSNAP_ATTR_NO_AUTORECOVERY = 0x00000002, > + VSS_VOLSNAP_ATTR_TXF_RECOVERY = 0x02000000 > +}; > + > +#endif > >
Il 25/06/2013 18:03, Laszlo Ersek ha scritto: >> > @@ -113,6 +114,7 @@ nested-vars += \ >> > stub-obj-y \ >> > util-obj-y \ >> > qga-obj-y \ >> > + qga-prv-obj-y \ >> > block-obj-y \ >> > common-obj-y >> > dummy := $(call unnest-vars) > What does this do? Does "qga-prv-obj-y" stand for "all objects under > 'qga'"? > > Or does it mean "look into qga/Makefile.objs for further objects"? The latter. I'd rather have it spelled fully, though (qga-win32-provider-obj-y). > > +qga-obj-y += vss-win32-provider/ > > +qga-prv-obj-y += vss-win32-provider/ > > +endif > > So we're probably just saying "look into > vss-win32-provider/Makefile.objs for further object files", for both > variables. Yes. > Looks like "qga-prv-obj-y" consists of nothing more than "provider.o" > and "install.o", and that "qga-provider.dll" depends on them. In theory, > would it be possible *not* to introduce "qga-prv-obj-y" in the > higher-level Makefile.obj files, only here? No, you need to specify it at all levels. > Why does "qga-provider.dll" depend on ""qga-provider.tlb"? It is not > used in the link command. > > Also, why is it necessary to collect the files "provider.o", "install.o" > and "qga-provider.def" into a DLL, and link qemu-ga.exe against that DLL > (via qga-obj-y)? Is this a VSS requirement? Can't we simply link these > objects into qemu-ga.exe statically, like the rest of "qga-obj-y"? I think dynamic linking is needed because VSS is loading the provider as a COM component here: + hr = pVssbc->AddToSnapshotSet(buf, g_gProviderId, &pid); But I don't know why you need to link the DLL to qemu-ga.exe. Paolo
Thanks for your review. On 6/25/13 12:03 , "Laszlo Ersek" <lersek@redhat.com> wrote: >I'm trying to wrap my head around the build changes first. >> diff --git a/configure b/configure >> index cafe830..8e45fad 100755 >> --- a/configure >> +++ b/configure >> @@ -3490,9 +3490,12 @@ if test "$softmmu" = yes ; then >> virtfs=no >> fi >> fi >> - if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; >>then >> + if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o >>"$mingw32" = "yes" ] ; then >> if [ "$guest_agent" = "yes" ]; then >> tools="qemu-ga\$(EXESUF) $tools" >> + if [ "$mingw32" = "yes" ]; then >> + tools="qga/vss-win32-provider/qga-provider.dll >>qga/vss-win32-provider/qga-provider.tlb $tools" >> + fi >> fi >> fi >> fi > >This adds three targets to "tools" on mingw32 -- "qemu-ga" hasn't been >there until now. Is this actually a small, unrelated bugfix? That is true, it is not necessary to build qemu-ga.exe. The intention was to do everything by just `make`. >I'm asking because I build upstream qemu-ga.exe on my RHEL-6 laptop as >follows -- I don't have pixman installed, and configure forces me to >specify --disable-tools as well: > >$ ./configure --without-pixman --disable-system --disable-tools \ > --cross-prefix=i686-pc-mingw32- >$ make qemu-ga.exe > >Plain "make" after the above ./configure doesn't produce anything >useful. Will I have to specify the DLL and TLB targets too on the >command line? (Apologies, I can't try it out now, msiextract doesn't >work for me.) You don't have to specify DLL and TLB targets because qemu-ga.exe has dependencies to them. >> diff --git a/Makefile.objs b/Makefile.objs >> index 286ce06..b6c1505 100644 >> --- a/Makefile.objs >> +++ b/Makefile.objs >> @@ -102,6 +102,7 @@ common-obj-y += disas/ >> # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed >> # by libqemuutil.a. These should be moved to a separate .json schema. >> qga-obj-y = qga/ qapi-types.o qapi-visit.o >> +qga-prv-obj-y = qga/ >> >> vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) >> >> @@ -113,6 +114,7 @@ nested-vars += \ >> stub-obj-y \ >> util-obj-y \ >> qga-obj-y \ >> + qga-prv-obj-y \ >> block-obj-y \ >> common-obj-y >> dummy := $(call unnest-vars) > >What does this do? Does "qga-prv-obj-y" stand for "all objects under >'qga'"? No, these are objects for qga/vss-win32-provider/, used to build qga-provider.dll. It looks better to be fully spelled as Paolo said in another mail. >Or does it mean "look into qga/Makefile.objs for further objects"? Yes... >>diff --git a/qga/Makefile.objs b/qga/Makefile.objs >> index b8d7cd0..8d93866 100644 >> --- a/qga/Makefile.objs >> +++ b/qga/Makefile.objs >> @@ -3,3 +3,9 @@ qga-obj-$(CONFIG_POSIX) += commands-posix.o >>channel-posix.o >> qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o >>service-win32.o >> qga-obj-y += qapi-generated/qga-qapi-types.o >>qapi-generated/qga-qapi-visit.o >> qga-obj-y += qapi-generated/qga-qmp-marshal.o >> + >> +ifeq ($(CONFIG_QGA_VSS),y) >> +QEMU_CFLAGS += -DHAS_VSS_SDK > >Isn't this superfluous? First, I can find no other reference to >HAS_VSS_SDK in the series, second, CONFIG_QGA_VSS would be available >directly as a macro to source files. It is referenced in e.g. qga/commands-win32.c in patch 8/10. But as you say, I will omit this, by the second reason. >> +qga-obj-y += vss-win32-provider/ >> +qga-prv-obj-y += vss-win32-provider/ >> +endif > >So we're probably just saying "look into >vss-win32-provider/Makefile.objs for further object files", for both >variables. Yes... >> diff --git a/qga/vss-win32-provider/Makefile.objs >>b/qga/vss-win32-provider/Makefile.objs >> new file mode 100644 >> index 0000000..1fe1f8f >> --- /dev/null >> +++ b/qga/vss-win32-provider/Makefile.objs >> @@ -0,0 +1,24 @@ >> +# rules to build qga-provider.dll >> + >> +qga-obj-y += qga-provider.dll >> +qga-prv-obj-y += provider.o install.o >> + >> +obj-qga-prv-obj-y = $(addprefix $(obj)/, $(qga-prv-obj-y)) >> +$(obj-qga-prv-obj-y): QEMU_CXXFLAGS = $(filter-out -Wstrict-prototypes >>-Wmissing-prototypes -Wnested-externs -Wold-style-declaration >>-Wold-style-definition -Wredundant-decls -fstack-protector-all, >>$(QEMU_CFLAGS)) -Wno-unknown-pragmas -Wno-delete-non-virtual-dtor >> + >> +$(obj)/qga-provider.dll: LDFLAGS = -shared >>-Wl,--add-stdcall-alias,--enable-stdcall-fixup -lole32 -loleaut32 >>-lshlwapi -luuid -static >> +$(obj)/qga-provider.dll: $(obj-qga-prv-obj-y) >>$(SRC_PATH)/$(obj)/qga-provider.def $(obj)/qga-provider.tlb >> + $(call quiet-command,$(CXX) -o $@ $(qga-prv-obj-y) >>$(SRC_PATH)/qga/vss-win32-provider/qga-provider.def $(CXXFLAGS) >>$(LDFLAGS)," LINK $(TARGET_DIR)$@") > >I'm having a hard time following this. > >Looks like "qga-prv-obj-y" consists of nothing more than "provider.o" >and "install.o", and that "qga-provider.dll" depends on them. In theory, >would it be possible *not* to introduce "qga-prv-obj-y" in the >higher-level Makefile.obj files, only here? Unfortunately, we need this at all levels to handle nested directories correctly, otherwise it fails on out-tree build. >Why does "qga-provider.dll" depend on ""qga-provider.tlb"? It is not >used in the link command. This was because .tlb is required to install .dll correctly. But it is not dependent on build time, so shouldn't be specified here. >Also, why is it necessary to collect the files "provider.o", "install.o" >and "qga-provider.def" into a DLL, and link qemu-ga.exe against that DLL >(via qga-obj-y)? Is this a VSS requirement? Can't we simply link these >objects into qemu-ga.exe statically, like the rest of "qga-obj-y"? > >Will some VSS service load the provider DLL independently of >qemu-ga.exe? Yes, VSS provider DLL is registered to VSS on installation of qemu-ga service (using .TLB), and loaded into VSS service component when snapshot is started. >If so, (a) how will they communicate Using COM interface. >(b) shouldn't "qga-obj-y" be independent of "qga-provider.dll" The DLL is linked to qemu-ga.exe because qemu-ga.exe kicks the registration of the DLL (COMRegister(), which depends on the DLL's Internal variable) at the installation of the qemu-ga service. >Could you please draw a dependency graph between these files? Like > > idl --> tlb > def --> dll <-- (provider.o, install.o) > > Does one of "tlb" and "dll" depend on the other, or are they needed "in > parallel"? This graph is describing build dependencies correctly. At runtime, DLL needs TLB, otherwise it cannot be loaded into VSS. >... I'll try to continue here. I've peaked forward a little bit, and >CQGAVssProvider::CommitSnapshots() seems to be the central method. It >kicks the "frozen" event, then blocks until the "thaw" event is kicked >back. I wonder how those will translate to QMP communication with >libvirt. CommitSnapshots() is called by VSS, when the requestor of qemu-ga (introduced in next patch) initiates snapshot receiving "guest-fsfreeze-freeze" command. "frozen" event is used to tell qemu-ga.exe that the fs is frozen, then qemu-ga.exe will tell libvirt to "guest-fsfreeze-freeze" is done. "thaw" event is kicked by qemnu-ga.exe when it receives "guest-fsfreeze-thaw" command. The command is finished when VSS notify qeum-ga.exe requestor that VSS snapshot process is completed. >Thanks! >Laszlo Again, thank you for taking your time for this! Thanks, Tomoki Sekiyama
Il 25/06/2013 20:23, Tomoki Sekiyama ha scritto: >> >(b) shouldn't "qga-obj-y" be independent of "qga-provider.dll" > The DLL is linked to qemu-ga.exe because qemu-ga.exe kicks the > registration of the DLL (COMRegister(), which depends on the DLL's > Internal variable) at the installation of the qemu-ga service. > Does the DLL also support DllRegisterServer? I don't know Windows very much, but if so I wonder if LoadLibrary/GetProcAddress is preferrable to linking with the DLL. Paolo
>Il 25/06/2013 20:23, Tomoki Sekiyama ha scritto: >>> >(b) shouldn't "qga-obj-y" be independent of "qga-provider.dll" >> The DLL is linked to qemu-ga.exe because qemu-ga.exe kicks the >> registration of the DLL (COMRegister(), which depends on the DLL's >> Internal variable) at the installation of the qemu-ga service. > > Does the DLL also support DllRegisterServer? I don't know Windows very > much, but if so I wonder if LoadLibrary/GetProcAddress is preferrable to > linking with the DLL. > > Paolo I think that is feasible. That will cut off qemu-ga.exe from qga-provider.{dll,tlb}, so then, we should build them separately, like `make qemu-ga.exe qga/vss-win32-provider/qga-provider.{dll,tlb}`. Thanks, Tomoki Sekiyama
On 06/25/13 20:23, Tomoki Sekiyama wrote: > On 6/25/13 12:03 , "Laszlo Ersek" <lersek@redhat.com> wrote: >>> +ifeq ($(CONFIG_QGA_VSS),y) >>> +QEMU_CFLAGS += -DHAS_VSS_SDK >> >> Isn't this superfluous? First, I can find no other reference to >> HAS_VSS_SDK in the series, second, CONFIG_QGA_VSS would be available >> directly as a macro to source files. > > It is referenced in e.g. qga/commands-win32.c in patch 8/10. Ah sorry. "ack" (that I sometimes use for recursive grepping, outside of git clones) didn't return any hits in the directory where I had saved your patch emails (for git-am), that's why I said "no references in the series". Perhaps "ack" ignored the .eml files due to their CRLF line endings... It did find the references with option "--all". (Of course I should have noticed something was fishy, since "ack" didn't find the very occurrence I was questioning! :)) > But as you say, I will omit this, by the second reason. OK, thanks. >> Will some VSS service load the provider DLL independently of >> qemu-ga.exe? > > Yes, VSS provider DLL is registered to VSS on installation of qemu-ga > service (using .TLB), and loaded into VSS service component when > snapshot is started. > >> If so, (a) how will they communicate > > Using COM interface. [...] >> ... I'll try to continue here. I've peaked forward a little bit, and >> CQGAVssProvider::CommitSnapshots() seems to be the central method. It >> kicks the "frozen" event, then blocks until the "thaw" event is kicked >> back. I wonder how those will translate to QMP communication with >> libvirt. > > CommitSnapshots() is called by VSS, when the requestor of qemu-ga > (introduced in next patch) initiates snapshot receiving > "guest-fsfreeze-freeze" command. > "frozen" event is used to tell qemu-ga.exe that the fs is frozen, > then qemu-ga.exe will tell libvirt to "guest-fsfreeze-freeze" is done. > > "thaw" event is kicked by qemnu-ga.exe when it receives > "guest-fsfreeze-thaw" command. The command is finished when VSS notify > qeum-ga.exe requestor that VSS snapshot process is completed. Very interesting. I'm curious about the requestor in the next patch. Based on the VSS description in MSDN, I thought the requestor would make some kind of blocking call (like a normal function call) into VSS that could not return until the full process was done (ie. filesystems were re-thawed). This likely isn't the case, as CommitSnapshots() appears to signal (wake) qemu-ga.exe asynchronously -- the requestor (qemu-ga) and the provider (the DLL in the VSS component) seem to run in parallel. The more I think I understand of your design the more I like it. I'll attempt to continue the review tomorrow. Thanks! Laszlo
Il 06/06/2013 17:06, Tomoki Sekiyama ha scritto: > +STDAPI VSSCheckOSVersion(void); > + > +STDAPI COMRegister(void); > +STDAPI COMUnregister(void); > + > +STDAPI DllRegisterServer(void); > +STDAPI DllUnregisterServer(void); Can you explain the difference between COMRegister/COMUnregister and DllRegisterServer/DllUnregisterServer (and why the COM+ part need not be done by regsvr32)? Also, why does COMUnregister call DllUnregisterServer but COMRegister does not call DllRegisterServer? Also, is it needed to call VSSCheckOSVersion from the requestor? I would think that checking VSSAPI.DLL is stronger than checking the version, and indeed you do that check too. Sorry for the probably stupid questions, COM stuff is way beyond my Windows knowledge. Paolo
From: Paolo Bonzini [paolo.bonzini@gmail.com] on behalf of Paolo Bonzini [pbonzini@redhat.com] > Il 06/06/2013 17:06, Tomoki Sekiyama ha scritto: >> +STDAPI VSSCheckOSVersion(void); >> + >> +STDAPI COMRegister(void); >> +STDAPI COMUnregister(void); >> + >> +STDAPI DllRegisterServer(void); >> +STDAPI DllUnregisterServer(void); > > Can you explain the difference between COMRegister/COMUnregister and > DllRegisterServer/DllUnregisterServer (and why the COM+ part need not be > done by regsvr32)? Also, why does COMUnregister call > DllUnregisterServer but COMRegister does not call DllRegisterServer? COMRegister and COMUnregister are called by`qemu-ga -s install`, to register/unregister the DLL into/from COM+ application catalogue. DllRegisterServer is automatically called inside pCatalog->InstallComponent() (like regsvr32 does), and register this DLL as a VSS provider. DllUnregisterServer will do the oposite. ICOMAdminCatalog (pCatalog) does not provide a method to uninstall component, so COMUnregister calls DllUnregisterServer by itself. > Also, is it needed to call VSSCheckOSVersion from the requestor? I > would think that checking VSSAPI.DLL is stronger than checking the > version, and indeed you do that check too. In Windows XP, VSSAPI.DLL exists but it has different functionality and interfaces from newer Windows. http://msdn.microsoft.com/en-us/library/windows/desktop/aa384627(v=vs.85).aspx It is checking the OS version because this patchset only supports Windows 2003 or later. Thanks, Tomoki Sekiyama
Il 26/06/2013 00:31, Tomoki Sekiyama ha scritto: > From: Paolo Bonzini [paolo.bonzini@gmail.com] on behalf of Paolo Bonzini [pbonzini@redhat.com] >> Il 06/06/2013 17:06, Tomoki Sekiyama ha scritto: >>> +STDAPI VSSCheckOSVersion(void); >>> + >>> +STDAPI COMRegister(void); >>> +STDAPI COMUnregister(void); >>> + >>> +STDAPI DllRegisterServer(void); >>> +STDAPI DllUnregisterServer(void); >> >> Can you explain the difference between COMRegister/COMUnregister and >> DllRegisterServer/DllUnregisterServer (and why the COM+ part need not be >> done by regsvr32)? Also, why does COMUnregister call >> DllUnregisterServer but COMRegister does not call DllRegisterServer? > > COMRegister and COMUnregister are called by`qemu-ga -s install`, > to register/unregister the DLL into/from COM+ application catalogue. > > DllRegisterServer is automatically called inside > pCatalog->InstallComponent() (like regsvr32 does), and register > this DLL as a VSS provider. DllUnregisterServer will do the oposite. > > ICOMAdminCatalog (pCatalog) does not provide a method to uninstall > component, so COMUnregister calls DllUnregisterServer by itself. Understood, thanks. Just one question remains: why is the COM+ part not needed when you invoke regsvr32? Thanks, Paolo >> Also, is it needed to call VSSCheckOSVersion from the requestor? I >> would think that checking VSSAPI.DLL is stronger than checking the >> version, and indeed you do that check too. > > In Windows XP, VSSAPI.DLL exists but it has different functionality > and interfaces from newer Windows. > http://msdn.microsoft.com/en-us/library/windows/desktop/aa384627(v=vs.85).aspx > > It is checking the OS version because this patchset only supports > Windows 2003 or later. > > Thanks, > Tomoki Sekiyama >
On 6/26/13 1:59 , "Paolo Bonzini" <pbonzini@redhat.com> wrote: >Il 26/06/2013 00:31, Tomoki Sekiyama ha scritto: >> From: Paolo Bonzini [paolo.bonzini@gmail.com] on behalf of Paolo >>Bonzini [pbonzini@redhat.com] >>> Il 06/06/2013 17:06, Tomoki Sekiyama ha scritto: >>>> +STDAPI VSSCheckOSVersion(void); >>>> + >>>> +STDAPI COMRegister(void); >>>> +STDAPI COMUnregister(void); >>>> + >>>> +STDAPI DllRegisterServer(void); >>>> +STDAPI DllUnregisterServer(void); >>> >>> Can you explain the difference between COMRegister/COMUnregister and >>> DllRegisterServer/DllUnregisterServer (and why the COM+ part need not >>>be >>> done by regsvr32)? Also, why does COMUnregister call >>> DllUnregisterServer but COMRegister does not call DllRegisterServer? >> >> COMRegister and COMUnregister are called by`qemu-ga -s install`, >> to register/unregister the DLL into/from COM+ application catalogue. >> >> DllRegisterServer is automatically called inside >> pCatalog->InstallComponent() (like regsvr32 does), and register >> this DLL as a VSS provider. DllUnregisterServer will do the oposite. >> >> ICOMAdminCatalog (pCatalog) does not provide a method to uninstall >> component, so COMUnregister calls DllUnregisterServer by itself. > >Understood, thanks. Just one question remains: why is the COM+ part not >needed when you invoke regsvr32? A VSS Provider is implemented as a COM+ application. To register COM+ applications, regsvr32 is not enough. It only register COM component, which is a part of COM+ application. We need to use a specialized installer (COMRegister() in this series) to the whole COM+ application to the catalogue. Thanks, Tomoki Sekiyama
On 06/25/13 18:03, Laszlo Ersek wrote: > On 06/06/13 17:06, Tomoki Sekiyama wrote: Comparing the .def file and the provider header file: >> diff --git a/qga/vss-win32-provider/qga-provider.def b/qga/vss-win32-provider/qga-provider.def >> new file mode 100644 >> index 0000000..9f3afc8 >> --- /dev/null >> +++ b/qga/vss-win32-provider/qga-provider.def >> @@ -0,0 +1,10 @@ >> +LIBRARY "QGA-PROVIDER.DLL" >> + >> +EXPORTS >> + VSSCheckOSVersion PRIVATE >> + COMRegister PRIVATE >> + COMUnregister PRIVATE >> + DllCanUnloadNow PRIVATE >> + DllGetClassObject PRIVATE >> + DllRegisterServer PRIVATE >> + DllUnregisterServer PRIVATE >> diff --git a/qga/vss-win32-provider.h b/qga/vss-win32-provider.h >> new file mode 100644 >> index 0000000..a437e71 >> --- /dev/null >> +++ b/qga/vss-win32-provider.h >> @@ -0,0 +1,26 @@ >> +/* >> + * QEMU Guest Agent win32 VSS provider declarations >> + * >> + * Copyright Hitachi Data Systems Corp. 2013 >> + * >> + * Authors: >> + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#ifndef VSS_WIN32_PROVIDER_H >> +#define VSS_WIN32_PROVIDER_H >> + >> +#include <windows.h> >> + >> +STDAPI VSSCheckOSVersion(void); >> + >> +STDAPI COMRegister(void); >> +STDAPI COMUnregister(void); >> + >> +STDAPI DllRegisterServer(void); >> +STDAPI DllUnregisterServer(void); >> + >> +#endif (a) what is the reason for not listing DllCanUnloadNow() and DllGetClassObject() in the header file? (b) Does STDAPI imply a return type? Or are you just going with the implicit "int"? (Based on my encounters with EFIAPI, I think it's the latter.) >> diff --git a/qga/vss-win32.h b/qga/vss-win32.h >> new file mode 100644 >> index 0000000..21655cf >> --- /dev/null >> +++ b/qga/vss-win32.h >> @@ -0,0 +1,86 @@ >> +/* >> + * QEMU Guest Agent win32 VSS common declarations >> + * >> + * Copyright Hitachi Data Systems Corp. 2013 >> + * >> + * Authors: >> + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#ifndef VSS_WIN32_H >> +#define VSS_WIN32_H >> + >> +#define __MIDL_user_allocate_free_DEFINED__ >> +#include "config-host.h" >> +#include <windows.h> >> +#include <shlwapi.h> >> + >> +/* Reduce warnings to include vss.h */ >> + >> +/* Ignore annotations for MS IDE */ >> +#define __in IN >> +#define __out OUT >> +#define __RPC_unique_pointer >> +#define __RPC_string >> +#define __RPC__deref_inout_opt >> +#define __RPC__out >> +#ifndef __RPC__out_ecount_part >> +#define __RPC__out_ecount_part(x, y) >> +#endif >> +#define _declspec(x) >> +#undef uuid >> +#define uuid(x) >> + >> +/* Undef some duplicated error codes redefined in vss.h */ >> +#undef VSS_E_BAD_STATE >> +#undef VSS_E_PROVIDER_NOT_REGISTERED >> +#undef VSS_E_PROVIDER_VETO >> +#undef VSS_E_OBJECT_NOT_FOUND >> +#undef VSS_E_VOLUME_NOT_SUPPORTED >> +#undef VSS_E_VOLUME_NOT_SUPPORTED_BY_PROVIDER >> +#undef VSS_E_OBJECT_ALREADY_EXISTS >> +#undef VSS_E_UNEXPECTED_PROVIDER_ERROR >> +#undef VSS_E_INVALID_XML_DOCUMENT >> +#undef VSS_E_MAXIMUM_NUMBER_OF_VOLUMES_REACHED >> +#undef VSS_E_MAXIMUM_NUMBER_OF_SNAPSHOTS_REACHED >> + >> +/* >> + * VSS headers must be installed from Microsoft VSS SDK 7.2 available at: >> + * http://www.microsoft.com/en-us/download/details.aspx?id=23490 >> + */ >> +#include "inc/win2003/vss.h" >> + >> +/* Macros to convert char definitions to wchar */ >> +#define _L(a) L##a >> +#define L(a) _L(a) (Sigh. The more windows code I'm looking at the more it convinces me that EFI was 100% meant for windows originally. Sad.) >> + >> +/* Constants for QGA VSS Provider */ >> + >> +#define QGA_PROVIDER_NAME "QEMU Guest Agent VSS Provider" >> +#define QGA_PROVIDER_LNAME L(QGA_PROVIDER_NAME) >> +#define QGA_PROVIDER_VERSION L(QEMU_VERSION) >> + >> +#define EVENT_NAME_FROZEN "Global\\QGAVSSEvent-frozen" >> +#define EVENT_NAME_THAW "Global\\QGAVSSEvent-thaw" No idea what I'm talking about, but since we're qualifying the second component with the "QGAVSSEvent" prefix, shouldn't we just replace the first component ("Global") with it? Or would it effect "event routing"? (Tangential question, anyway.) >> + >> +const GUID g_gProviderId = { 0x3629d4ed, 0xee09, 0x4e0e, >> + {0x9a, 0x5c, 0x6d, 0x8b, 0xa2, 0x87, 0x2a, 0xef} }; >> +const GUID g_gProviderVersion = { 0x11ef8b15, 0xcac6, 0x40d6, >> + {0x8d, 0x5c, 0x8f, 0xfc, 0x16, 0x3f, 0x24, 0xca} }; >> + >> +const CLSID CLSID_QGAVSSProvider = { 0x6e6a3492, 0x8d4d, 0x440c, >> + {0x96, 0x19, 0x5e, 0x5d, 0x0c, 0xc3, 0x1c, 0xa8} }; >> + >> +const TCHAR g_szClsid[] = TEXT("{6E6A3492-8D4D-440C-9619-5E5D0CC31CA8}"); OK, these match each other and "qga-provider.idl". >> +const TCHAR g_szProgid[] = TEXT("QGAVSSProvider"); >> + >> +/* Enums undefined in VSS SDK 7.2 but defined in newer Windows SDK */ >> +enum __VSS_VOLUME_SNAPSHOT_ATTRIBUTES { >> + VSS_VOLSNAP_ATTR_NO_AUTORECOVERY = 0x00000002, >> + VSS_VOLSNAP_ATTR_TXF_RECOVERY = 0x02000000 >> +}; >> + >> +#endif >> >> OK. >> diff --git a/qga/vss-win32-provider/install.cpp b/qga/vss-win32-provider/install.cpp >> new file mode 100644 >> index 0000000..d6f1d55 >> --- /dev/null >> +++ b/qga/vss-win32-provider/install.cpp >> @@ -0,0 +1,493 @@ >> +/* >> + * QEMU Guest Agent win32 VSS Provider installer >> + * >> + * Copyright Hitachi Data Systems Corp. 2013 >> + * >> + * Authors: >> + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#include <stdio.h> >> +#include <string.h> >> + >> +#include "../vss-win32.h" >> +#include "inc/win2003/vscoordint.h" >> +#include "../vss-win32-provider.h" >> + >> +#include <comadmin.h> >> +#include <wbemidl.h> >> +#include <comutil.h> >> + >> +extern HINSTANCE g_hinstDll; >> + >> +const GUID CLSID_COMAdminCatalog = { 0xF618C514, 0xDFB8, 0x11d1, >> + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; >> +const GUID IID_ICOMAdminCatalog = { 0xDD662187, 0xDFC2, 0x11d1, >> + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; >> +const GUID CLSID_WbemLocator = { 0x4590f811, 0x1d3a, 0x11d0, >> + {0x89, 0x1f, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; >> +const GUID IID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf, >> + {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; Would it be possible to make these static? Not too important of course. >> + >> +static void errmsg(DWORD err, const char *text) >> +{ >> + char *msg = NULL, *nul = strchr(text, '('); >> + int len = nul ? nul - text : -1; >> + >> + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | >> + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, >> + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), >> + (char *)&msg, 0, NULL); (The FormatMessage() specification in MSDN is simply incredible. "(char *)&msg", seriously? Your code is correct, but the interface is unbelievable.) >> + printf("%.*s. (Error: %lx) %s\n", len, text, err, msg); (a) Since we're printing an error message here, I suspect it should go to stderr. (b) In case "text" doesn't contain an opening paren, "len" is set to -1. "A negative precision is taken as if the precision were omitted", IOW the entire string will be printed. Clever! (c) Does DWORD expand to "uint32_t"? In that case "%lx" is too big on 64-bit (LP64). We should probably use "%x" or "%"PRIx32. (Or do we build qemu-ga.exe only for 32-bit? I seem to remember something like that.) >> + LocalFree(msg); >> +} >> + >> +static void errmsg_dialog(DWORD err, const char *text, const char *opt = "") >> +{ >> + char *msg, buf[512]; >> + >> + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | >> + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, >> + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), >> + (char *)&msg, 0, NULL); >> + snprintf(buf, sizeof(buf), "%s%s. (Error: %lx) %s\n", text, opt, err, msg); >> + MessageBox(NULL, buf, "Error from " QGA_PROVIDER_NAME, MB_OK|MB_ICONERROR); Do you need a trailing newline for a message box? >> + LocalFree(msg); >> +} >> + >> +#define _chk(hr, status, msg, err_label) \ >> + do { \ >> + hr = (status); \ >> + if (FAILED(hr)) { \ >> + errmsg(hr, msg); \ >> + goto err_label; \ >> + } \ >> + } while (0) >> + >> +#define chk(status) _chk(hr, status, "Failed to " #status, out) >> + >> +template<class T> >> +HRESULT put_Value(ICatalogObject *pObj, LPCWSTR name, T val) >> +{ >> + return pObj->put_Value(_bstr_t(name), _variant_t(val)); >> +} >> + >> +/* Check whether this OS version supports VSS providers */ >> +STDAPI VSSCheckOSVersion(void) >> +{ >> + OSVERSIONINFO OSver; >> + >> + OSver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); >> + GetVersionEx(&OSver); >> + if ((OSver.dwMajorVersion == 5 && OSver.dwMinorVersion >= 2) || >> + OSver.dwMajorVersion > 5) { >> + return S_OK; >> + } >> + return S_FALSE; >> +} >> + Seems OK. >> +/* Lookup Administrators group name from winmgmt */ >> +static HRESULT GetAdminName(_bstr_t &name) >> +{ I can see that you want to call GetAdminName() in chk(), hence you must return a HRESULT and store the value through the parameter list. However, can we keep the C++ features to a minimum? I'd prefer if the non-const reference were replaced with a pointer-to-_bstr_t. (Tangential, change only if you respin anyway and don't mind it.) >> + HRESULT hr; >> + IWbemLocator *pLoc = NULL; >> + IWbemServices *pSvc = NULL; >> + IEnumWbemClassObject *pEnum = NULL; >> + IWbemClassObject *pWobj = NULL; >> + ULONG returned; >> + _variant_t var; >> + >> + chk(CoCreateInstance(CLSID_WbemLocator, NULL, CLSCTX_INPROC_SERVER, >> + IID_IWbemLocator, (LPVOID *)&pLoc)); >> + chk(pLoc->ConnectServer(_bstr_t(L"ROOT\\CIMV2"), NULL, NULL, NULL, >> + 0, 0, 0, &pSvc)); >> + chk(CoSetProxyBlanket(pSvc, RPC_C_AUTHN_WINNT, RPC_C_AUTHZ_NONE, >> + NULL, RPC_C_AUTHN_LEVEL_CALL, >> + RPC_C_IMP_LEVEL_IMPERSONATE, NULL, EOAC_NONE)); >> + chk(pSvc->ExecQuery(_bstr_t(L"WQL"), >> + _bstr_t(L"select * from Win32_Account where " >> + "SID='S-1-5-32-544' and localAccount=TRUE"), >> + WBEM_FLAG_RETURN_IMMEDIATELY | WBEM_FLAG_FORWARD_ONLY, >> + NULL, &pEnum)); (signal to noise ratio converging to zero... :) I don't doubt this is the official way though.) Furthermore, aren't you missing a CoInitialize(NULL) call before CoCreateInstance()? Other CoCreateInstance() calls are preceded by it. ... Well this is called only from COMRegister(), after such an init call, so it should be OK. >> + if (!pEnum) { >> + errmsg(E_FAIL, "Failed to query for Administrators"); >> + goto out; I think you forgot to set "hr" to something ugly -- the code below the "out" label will simply return it, and "hr" is currently something nice. (The last chk() succeeded.) >> + } This branch is probably for the case when ExecQuery() succeeds, but doesn't return any rows, right? >> + chk(pEnum->Next(WBEM_INFINITE, 1, &pWobj, &returned)); >> + if (returned == 0) { >> + errmsg(E_FAIL, "Failed to query for Administrators"); >> + goto out; >> + } >> + >> + chk(pWobj->Get(_bstr_t(L"Name"), 0, &var, 0, 0)); >> + name = var; This assigns a _variant_t to a _bstr_t (... for which I proposed *name = var above, but it's irrelevant now). Can this conversion (or the assignment operator) fail? If so, does either throw an exception? >> +out: >> + if (pLoc) { >> + pLoc->Release(); >> + } >> + if (pSvc) { >> + pSvc->Release(); >> + } >> + if (pEnum) { >> + pEnum->Release(); >> + } >> + if (pWobj) { >> + pWobj->Release(); >> + } >> + return hr; >> +} I'm sure this is idiomatic code, but do these pointers point to static / long-term instances? I'm missing the equivalent of "delete pLoc" and so on. Each Release() member function gets the implicit "this" parameter here, but do they free it too? (Honestly, I can't decide which would be worse, if they did or they didn't.) ... Ah, it's probably reference counting. OK. >> + >> +/* Register this module to COM+ Applications Catalog */ >> +STDAPI COMRegister(void) >> +{ >> + HRESULT hr = E_FAIL; >> + IUnknown *pUnknown = NULL; >> + ICOMAdminCatalog *pCatalog = NULL; >> + ICatalogCollection *pApps = NULL, *pRoles = NULL, *pUsersInRole = NULL; >> + ICatalogObject *pObj = NULL; >> + long n; >> + _bstr_t name; >> + _variant_t key; >> + CHAR dllPath[MAX_PATH], tlbPath[MAX_PATH]; >> + >> + if (!g_hinstDll) { >> + errmsg(E_FAIL, "Failed to initialize DLL"); >> + goto out; OK, "hr" is E_FAIL here. >> + } >> + >> + if (VSSCheckOSVersion() == S_FALSE) { >> + printf("VSS provider is not supported in this OS version.\n"); I suggest stderr. >> + return S_FALSE; /* VSS feature is disabled */ Somewhat inconsistent with the above, you issue "return" although you used "goto out" before. No resources are leaked so it's no problem in practice. (You could have written "return" above.) >> + } >> + >> + COMUnregister(); What requires / justifies this? (I can see another call below, on the error path.) I think whatever COMUnregister() would release depends first on the success of COMRegister(), doesn't it? You've told Paolo, On 06/26/13 00:31, Tomoki Sekiyama wrote: > COMRegister and COMUnregister are called by`qemu-ga -s install`, > to register/unregister the DLL into/from COM+ application catalogue. and indeed I can see a COMRegister() call in 09/10. Nonetheless that doesn't seem to justify this call to COMUnregister() -- if the DLL has been previously registered in the application catalog, a new registration attempt should fail. >> + >> + chk(CoInitialize(NULL)); Can this fail in practice? If so, we could have error handling trouble, because "out:" will call CoUninitialize() without any successful initialization. >> + chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER, >> + IID_IUnknown, (void **)&pUnknown)); >> + chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog, (void **)&pCatalog)); >> + >> + /* Install COM+ Component */ >> + >> + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), >> + (IDispatch **)&pApps)); >> + chk(pApps->Populate()); >> + chk(pApps->Add((IDispatch **)&pObj)); >> + chk(put_Value(pObj, L"Name", QGA_PROVIDER_LNAME)); >> + chk(put_Value(pObj, L"Description", QGA_PROVIDER_LNAME)); >> + chk(put_Value(pObj, L"ApplicationAccessChecksEnabled", true)); >> + chk(put_Value(pObj, L"Authentication", short(6))); >> + chk(put_Value(pObj, L"AuthenticationCapability", short(2))); >> + chk(put_Value(pObj, L"ImpersonationLevel", short(2))); >> + chk(pApps->SaveChanges(&n)); This looks like a globally visible change that is not rolled back if something fails later in this function. Is the COMUnregister() call on the error handling path supposed to undo these? Complicated :) Then, should we check "n", or does the retval check suffice? >> + chk(pObj->get_Key(&key)); >> + >> + pObj->Release(); >> + pObj = NULL; Ah OK. You're reusing pObj below. >> + >> + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { >> + errmsg(GetLastError(), "GetModuleFileName failed"); >> + goto out; "hr" has not been set to a failure code. >> + } >> + n = strlen(dllPath); >> + if (n < 3) { >> + errmsg(E_FAIL, "Failed to lookup dll"); Presumably you wanted to set "hr" and jump to "out" as well. >> + } >> + strcpy(tlbPath, dllPath); >> + strcpy(tlbPath+n-3, "TLB"); >> + printf("Registering " QGA_PROVIDER_NAME ":\n"); >> + printf(" %s\n", dllPath); >> + printf(" %s\n", tlbPath); These are arguably diagnostic messages (destined for stderr), but I won't press it :) >> + if (!PathFileExists(tlbPath)) { >> + errmsg(ERROR_FILE_NOT_FOUND, "Failed to lookup tlb"); >> + goto out; "hr" not set properly. >> + } >> + >> + chk(pCatalog->InstallComponent(_bstr_t(QGA_PROVIDER_LNAME), >> + _bstr_t(dllPath), _bstr_t(tlbPath), >> + _bstr_t(""))); >> + >> + /* Setup roles of the applicaion */ >> + >> + chk(pApps->GetCollection(_bstr_t(L"Roles"), key, >> + (IDispatch **)&pRoles)); >> + chk(pRoles->Populate()); >> + chk(pRoles->Add((IDispatch **)&pObj)); >> + chk(put_Value(pObj, L"Name", L"Administrators")); >> + chk(put_Value(pObj, L"Description", L"Administrators group")); >> + chk(pRoles->SaveChanges(&n)); >> + chk(pObj->get_Key(&key)); >> + >> + pObj->Release(); >> + pObj = NULL; >> + >> + /* Setup users in the role */ >> + >> + chk(pRoles->GetCollection(_bstr_t(L"UsersInRole"), key, >> + (IDispatch **)&pUsersInRole)); >> + chk(pUsersInRole->Populate()); >> + >> + chk(pUsersInRole->Add((IDispatch **)&pObj)); >> + chk(GetAdminName(name)); >> + chk(put_Value(pObj, L"User", _bstr_t(".\\") + name)); >> + >> + pObj->Release(); >> + pObj = NULL; >> + >> + chk(pUsersInRole->Add((IDispatch **)&pObj)); >> + chk(put_Value(pObj, L"User", L"SYSTEM")); >> + chk(pUsersInRole->SaveChanges(&n)); >> + >> +out: >> + if (pUnknown) { >> + pUnknown->Release(); >> + } >> + if (pCatalog) { >> + pCatalog->Release(); >> + } >> + if (pApps) { >> + pApps->Release(); >> + } >> + if (pRoles) { >> + pRoles->Release(); >> + } >> + if (pUsersInRole) { >> + pUsersInRole->Release(); >> + } >> + if (pObj) { >> + pObj->Release(); >> + } >> + >> + if (FAILED(hr)) { >> + COMUnregister(); I've actually grown to accept this COMUnregister() call here... Undoing all the crap from before staircase-wise would be a nightmare. >> + } >> + CoUninitialize(); Another question: can you nest CoInitialize() calls (and more importantly, does CoUninitialize() respect that)? Consider the following call chain: COMRegister() CoInitialize() COMUnregister() -- on the "out:" path DllUnregisterServer() CoInitialize() -- init after init CoUninitialize() CoInitialize() CoUninitialize() CoUninitialize() -- right here, uninit after uninit Will these work (especially the final two)? >> + >> + return hr; >> +} >> + >> +/* Unregister this module from COM+ Applications Catalog */ >> +STDAPI COMUnregister(void) >> +{ >> + HRESULT hr; >> + IUnknown *pUnknown = NULL; >> + ICOMAdminCatalog *pCatalog = NULL; >> + ICatalogCollection *pColl = NULL; >> + ICatalogObject *pObj = NULL; >> + _variant_t var; >> + long i, n; >> + >> + if (VSSCheckOSVersion() == S_FALSE) { >> + printf("VSS provider is not supported in this OS version.\n"); I suggest stderr again. >> + return S_FALSE; /* VSS feature is disabled */ >> + } >> + >> + chk(DllUnregisterServer()); Apparently DllUnregisterServer() can't fail. (And it shouldn't, we depend on it, since COMUnregister() is also used as destructor after partial construction, so we must not bail here.) I propose to change the return type of DllUnregisterServer() to void, just to drive the point home. >> + >> + chk(CoInitialize(NULL)); (same question as before -- can this fail? See unconditional uninit at the bottom.) I'll continue from here. Thanks Laszlo >> + chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER, >> + IID_IUnknown, (void **)&pUnknown)); >> + chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog, (void **)&pCatalog)); >> + >> + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), >> + (IDispatch **)&pColl)); >> + chk(pColl->Populate()); >> + >> + chk(pColl->get_Count(&n)); >> + for (i = n - 1; i >= 0; i--) { >> + chk(pColl->get_Item(i, (IDispatch **)&pObj)); >> + chk(pObj->get_Value(_bstr_t(L"Name"), &var)); >> + if (var == _variant_t(QGA_PROVIDER_LNAME)) { >> + printf("Removing COM+ Application: %S\n", (wchar_t *)_bstr_t(var)); >> + chk(pColl->Remove(i)); >> + } >> + } >> + chk(pColl->SaveChanges(&n)); >> + >> +out: >> + if (pUnknown) { >> + pUnknown->Release(); >> + } >> + if (pCatalog) { >> + pCatalog->Release(); >> + } >> + if (pColl) { >> + pColl->Release(); >> + } >> + if (pObj) { >> + pObj->Release(); >> + } >> + CoUninitialize(); >> + >> + return hr; >> +} >> + >> + >> +static BOOL CreateRegistryKey(LPCTSTR key, LPCTSTR value, LPCTSTR data) >> +{ >> + HKEY hKey; >> + LONG ret; >> + DWORD size; >> + >> + ret = RegCreateKeyEx(HKEY_CLASSES_ROOT, key, 0, NULL, >> + REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, &hKey, NULL); >> + if (ret != ERROR_SUCCESS) { >> + goto out; >> + } >> + >> + if (data != NULL) { >> + size = (lstrlen(data) + 1) * sizeof(TCHAR); >> + } else { >> + size = 0; >> + } >> + >> + ret = RegSetValueEx(hKey, value, 0, REG_SZ, (LPBYTE)data, size); >> + RegCloseKey(hKey); >> + >> +out: >> + if (ret != ERROR_SUCCESS) { >> + /* We cannot printf here, and need MessageBox to report an error. */ >> + errmsg_dialog(ret, "Cannot add registry ", key); >> + return FALSE; >> + } >> + return TRUE; >> +} >> + >> +/* Register this dll as a VSS provider */ >> +STDAPI DllRegisterServer(void) >> +{ >> + IVssAdmin *pVssAdmin = NULL; >> + HRESULT hr = E_FAIL; >> + char dllPath[MAX_PATH]; >> + char key[256]; >> + >> + CoInitialize(NULL); >> + >> + if (!g_hinstDll) { >> + errmsg_dialog(hr, "Module instance is not available"); >> + goto out; >> + } >> + >> + /* Add this module to registery */ >> + >> + sprintf(key, "CLSID\\%s", g_szClsid); >> + if (!CreateRegistryKey(key, NULL, g_szClsid)) { >> + goto out; >> + } >> + >> + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { >> + errmsg_dialog(GetLastError(), "GetModuleFileName failed"); >> + goto out; >> + } >> + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); >> + if (!CreateRegistryKey(key, NULL, dllPath)) { >> + goto out; >> + } >> + >> + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); >> + if (!CreateRegistryKey(key, "ThreadingModel", "Apartment")) { >> + goto out; >> + } >> + >> + sprintf(key, "CLSID\\%s\\ProgID", g_szClsid); >> + if (!CreateRegistryKey(key, NULL, g_szProgid)) { >> + goto out; >> + } >> + >> + if (!CreateRegistryKey(g_szProgid, NULL, QGA_PROVIDER_NAME)) { >> + goto out; >> + } >> + >> + sprintf(key, "%s\\CLSID", g_szProgid); >> + if (!CreateRegistryKey(key, NULL, g_szClsid)) { >> + goto out; >> + } >> + >> + hr = CoCreateInstance(CLSID_VSSCoordinator, >> + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); >> + if (FAILED(hr)) { >> + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); >> + goto out; >> + } >> + >> + hr = pVssAdmin->RegisterProvider( >> + g_gProviderId, CLSID_QGAVSSProvider, >> + const_cast<WCHAR*>(QGA_PROVIDER_LNAME), VSS_PROV_SOFTWARE, >> + const_cast<WCHAR*>(QGA_PROVIDER_VERSION), g_gProviderVersion); >> + if (FAILED(hr)) { >> + errmsg_dialog(hr, "RegisterProvider failed"); >> + goto out; >> + } >> + >> +out: >> + if (pVssAdmin) { >> + pVssAdmin->Release(); >> + } >> + CoUninitialize(); >> + >> + if (FAILED(hr)) { >> + DllUnregisterServer(); >> + } >> + >> + return hr; >> +} >> + >> +/* Unregister this VSS hardware provider from the system */ >> +STDAPI DllUnregisterServer(void) >> +{ >> + TCHAR key[256]; >> + IVssAdmin *pVssAdmin = NULL; >> + >> + CoInitialize(NULL); >> + >> + HRESULT hr = CoCreateInstance(CLSID_VSSCoordinator, >> + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); >> + if (SUCCEEDED(hr)) { >> + hr = pVssAdmin->UnregisterProvider(g_gProviderId); >> + pVssAdmin->Release(); >> + } else { >> + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); >> + } >> + >> + sprintf(key, "CLSID\\%s", g_szClsid); >> + SHDeleteKey(HKEY_CLASSES_ROOT, key); >> + SHDeleteKey(HKEY_CLASSES_ROOT, g_szProgid); >> + >> + CoUninitialize(); >> + >> + return S_OK; /* Uninstall should never fail */ >> +} >> + >> + >> +/* Support functions for _bstr_t in MinGW: Originally written by: Diaa Sami */ >> + >> +void __stdcall _com_issue_error(HRESULT hr) >> +{ >> + printf("_com_issue_error() called with parameter HRESULT = %lu", hr); >> +} >> + >> +namespace _com_util >> +{ >> + char * __stdcall ConvertBSTRToString(BSTR bstr) >> + { >> + const unsigned int stringLength = lstrlenW(bstr); >> + char *const ascii = new char [stringLength + 1]; >> + >> + wcstombs(ascii, bstr, stringLength + 1); >> + >> + return ascii; >> + } >> + >> + BSTR __stdcall ConvertStringToBSTR(const char *const ascii) >> + { >> + const unsigned int stringLength = lstrlenA(ascii); >> + BSTR bstr = SysAllocStringLen(NULL, stringLength); >> + >> + mbstowcs(bstr, ascii, stringLength + 1); >> + >> + return bstr; >> + } >> +} > >> diff --git a/qga/vss-win32-provider/provider.cpp b/qga/vss-win32-provider/provider.cpp >> new file mode 100644 >> index 0000000..90a3d25 >> --- /dev/null >> +++ b/qga/vss-win32-provider/provider.cpp >> @@ -0,0 +1,479 @@ >> +/* >> + * QEMU Guest Agent win32 VSS Provider implementations >> + * >> + * Copyright Hitachi Data Systems Corp. 2013 >> + * >> + * Authors: >> + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#include <stdio.h> >> +#include "../vss-win32.h" >> +#include "inc/win2003/vscoordint.h" >> +#include "inc/win2003/vsprov.h" >> + >> +static long g_nComObjsInUse; >> +HINSTANCE g_hinstDll; >> + >> +/* VSS common GUID's */ >> + >> +const CLSID CLSID_VSSCoordinator = { 0xE579AB5F, 0x1CC4, 0x44b4, >> + {0xBE, 0xD9, 0xDE, 0x09, 0x91, 0xFF, 0x06, 0x23} }; >> +const IID IID_IVssAdmin = { 0x77ED5996, 0x2F63, 0x11d3, >> + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; >> + >> +const IID IID_IVssHardwareSnapshotProvider = { 0x9593A157, 0x44E9, 0x4344, >> + {0xBB, 0xEB, 0x44, 0xFB, 0xF9, 0xB0, 0x6B, 0x10} }; >> +const IID IID_IVssSoftwareSnapshotProvider = { 0x609e123e, 0x2c5a, 0x44d3, >> + {0x8f, 0x01, 0x0b, 0x1d, 0x9a, 0x47, 0xd1, 0xff} }; >> +const IID IID_IVssProviderCreateSnapshotSet = { 0x5F894E5B, 0x1E39, 0x4778, >> + {0x8E, 0x23, 0x9A, 0xBA, 0xD9, 0xF0, 0xE0, 0x8C} }; >> +const IID IID_IVssProviderNotifications = { 0xE561901F, 0x03A5, 0x4afe, >> + {0x86, 0xD0, 0x72, 0xBA, 0xEE, 0xCE, 0x70, 0x04} }; >> + >> +const IID IID_IVssEnumObject = { 0xAE1C7110, 0x2F60, 0x11d3, >> + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; >> + >> + >> +void LockModule(BOOL block) >> +{ >> + if (block) { >> + InterlockedIncrement(&g_nComObjsInUse); >> + } else { >> + InterlockedDecrement(&g_nComObjsInUse); >> + } >> +} >> + >> +/* Empty enumerator for VssObject */ >> + >> +class CQGAVSSEnumObject : public IVssEnumObject >> +{ >> +public: >> + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); >> + STDMETHODIMP_(ULONG) AddRef(); >> + STDMETHODIMP_(ULONG) Release(); >> + >> + /* IVssEnumObject Methods */ >> + STDMETHODIMP Next( >> + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched); >> + STDMETHODIMP Skip(ULONG celt); >> + STDMETHODIMP Reset(void); >> + STDMETHODIMP Clone(IVssEnumObject **ppenum); >> + >> + /* CQGAVSSEnumObject Methods */ >> + CQGAVSSEnumObject(); >> + ~CQGAVSSEnumObject(); >> + >> +private: >> + long m_nRefCount; >> +}; >> + >> +CQGAVSSEnumObject::CQGAVSSEnumObject() >> +{ >> + m_nRefCount = 0; >> + LockModule(TRUE); >> +} >> + >> +CQGAVSSEnumObject::~CQGAVSSEnumObject() >> +{ >> + LockModule(FALSE); >> +} >> + >> +STDMETHODIMP CQGAVSSEnumObject::QueryInterface(REFIID riid, void **ppObj) >> +{ >> + if (riid == IID_IUnknown || riid == IID_IVssEnumObject) { >> + *ppObj = static_cast<void*>(static_cast<IVssEnumObject*>(this)); >> + AddRef(); >> + return S_OK; >> + } >> + ppObj = NULL; >> + return E_NOINTERFACE; >> +} >> + >> +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::AddRef() >> +{ >> + return InterlockedIncrement(&m_nRefCount); >> +} >> + >> +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::Release() >> +{ >> + long nRefCount = InterlockedDecrement(&m_nRefCount); >> + if (m_nRefCount == 0) { >> + delete this; >> + } >> + return nRefCount; >> +} >> + >> +STDMETHODIMP CQGAVSSEnumObject::Next( >> + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched) >> +{ >> + *pceltFetched = 0; >> + return S_FALSE; >> +} >> + >> +STDMETHODIMP CQGAVSSEnumObject::Skip(ULONG celt) >> +{ >> + return S_FALSE; >> +} >> + >> +STDMETHODIMP CQGAVSSEnumObject::Reset(void) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVSSEnumObject::Clone(IVssEnumObject **ppenum) >> +{ >> + return E_NOTIMPL; >> +} >> + >> + >> +/* QGAVssProvider */ >> + >> +class CQGAVssProvider : >> + public IVssSoftwareSnapshotProvider, >> + public IVssProviderCreateSnapshotSet, >> + public IVssProviderNotifications >> +{ >> +public: >> + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); >> + STDMETHODIMP_(ULONG) AddRef(); >> + STDMETHODIMP_(ULONG) Release(); >> + >> + /* IVssSoftwareSnapshotProvider Methods */ >> + STDMETHODIMP SetContext(LONG lContext); >> + STDMETHODIMP GetSnapshotProperties( >> + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp); >> + STDMETHODIMP Query( >> + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, >> + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum); >> + STDMETHODIMP DeleteSnapshots( >> + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, >> + BOOL bForceDelete, LONG *plDeletedSnapshots, >> + VSS_ID *pNondeletedSnapshotID); >> + STDMETHODIMP BeginPrepareSnapshot( >> + VSS_ID SnapshotSetId, VSS_ID SnapshotId, >> + VSS_PWSZ pwszVolumeName, LONG lNewContext); >> + STDMETHODIMP IsVolumeSupported( >> + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider); >> + STDMETHODIMP IsVolumeSnapshotted( >> + VSS_PWSZ pwszVolumeName, BOOL *pbSnapshotsPresent, >> + LONG *plSnapshotCompatibility); >> + STDMETHODIMP SetSnapshotProperty( >> + VSS_ID SnapshotId, VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, >> + VARIANT vProperty); >> + STDMETHODIMP RevertToSnapshot(VSS_ID SnapshotId); >> + STDMETHODIMP QueryRevertStatus(VSS_PWSZ pwszVolume, IVssAsync **ppAsync); >> + >> + /* IVssProviderCreateSnapshotSet Methods */ >> + STDMETHODIMP EndPrepareSnapshots(VSS_ID SnapshotSetId); >> + STDMETHODIMP PreCommitSnapshots(VSS_ID SnapshotSetId); >> + STDMETHODIMP CommitSnapshots(VSS_ID SnapshotSetId); >> + STDMETHODIMP PostCommitSnapshots( >> + VSS_ID SnapshotSetId, LONG lSnapshotsCount); >> + STDMETHODIMP PreFinalCommitSnapshots(VSS_ID SnapshotSetId); >> + STDMETHODIMP PostFinalCommitSnapshots(VSS_ID SnapshotSetId); >> + STDMETHODIMP AbortSnapshots(VSS_ID SnapshotSetId); >> + >> + /* IVssProviderNotifications Methods */ >> + STDMETHODIMP OnLoad(IUnknown *pCallback); >> + STDMETHODIMP OnUnload(BOOL bForceUnload); >> + >> + /* CQGAVssProvider Methods */ >> + CQGAVssProvider(); >> + ~CQGAVssProvider(); >> + >> +private: >> + long m_nRefCount; >> +}; >> + >> +CQGAVssProvider::CQGAVssProvider() >> +{ >> + m_nRefCount = 0; >> + LockModule(TRUE); >> +} >> + >> +CQGAVssProvider::~CQGAVssProvider() >> +{ >> + LockModule(FALSE); >> +} >> + >> +STDMETHODIMP CQGAVssProvider::QueryInterface(REFIID riid, void **ppObj) >> +{ >> + if (riid == IID_IUnknown) { >> + *ppObj = static_cast<void*>(this); >> + AddRef(); >> + return S_OK; >> + } else if (riid == IID_IVssSoftwareSnapshotProvider) { >> + *ppObj = static_cast<void*>( >> + static_cast<IVssSoftwareSnapshotProvider*>(this)); >> + AddRef(); >> + return S_OK; >> + } else if (riid == IID_IVssProviderCreateSnapshotSet) { >> + *ppObj = static_cast<void*>( >> + static_cast<IVssProviderCreateSnapshotSet*>(this)); >> + AddRef(); >> + return S_OK; >> + } else if (riid == IID_IVssProviderNotifications) { >> + *ppObj = static_cast<void*>( >> + static_cast<IVssProviderNotifications*>(this)); >> + AddRef(); >> + return S_OK; >> + } >> + *ppObj = NULL; >> + return E_NOINTERFACE; >> +} >> + >> +STDMETHODIMP_(ULONG) CQGAVssProvider::AddRef() >> +{ >> + return InterlockedIncrement(&m_nRefCount); >> +} >> + >> +STDMETHODIMP_(ULONG) CQGAVssProvider::Release() >> +{ >> + long nRefCount = InterlockedDecrement(&m_nRefCount); >> + if (m_nRefCount == 0) { >> + delete this; >> + } >> + return nRefCount; >> +} >> + >> + >> +/* >> + * IVssSoftwareSnapshotProvider methods >> + */ >> + >> +STDMETHODIMP CQGAVssProvider::SetContext(LONG lContext) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::GetSnapshotProperties( >> + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp) >> +{ >> + return VSS_E_OBJECT_NOT_FOUND; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::Query( >> + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, >> + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum) >> +{ >> + *ppEnum = new CQGAVSSEnumObject; >> + (*ppEnum)->AddRef(); >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::DeleteSnapshots( >> + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, >> + BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID *pNondeletedSnapshotID) >> +{ >> + return E_NOTIMPL; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot( >> + VSS_ID SnapshotSetId, VSS_ID SnapshotId, >> + VSS_PWSZ pwszVolumeName, LONG lNewContext) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::IsVolumeSupported( >> + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider) >> +{ >> + *pbSupportedByThisProvider = TRUE; >> + >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::IsVolumeSnapshotted(VSS_PWSZ pwszVolumeName, >> + BOOL *pbSnapshotsPresent, LONG *plSnapshotCompatibility) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::SetSnapshotProperty(VSS_ID SnapshotId, >> + VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, VARIANT vProperty) >> +{ >> + return E_NOTIMPL; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::RevertToSnapshot(VSS_ID SnapshotId) >> +{ >> + return E_NOTIMPL; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::QueryRevertStatus( >> + VSS_PWSZ pwszVolume, IVssAsync **ppAsync) >> +{ >> + return S_OK; >> +} >> + >> + >> +/* >> + * IVssProviderCreateSnapshotSet methods >> + */ >> + >> +STDMETHODIMP CQGAVssProvider::EndPrepareSnapshots(VSS_ID SnapshotSetId) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::PreCommitSnapshots(VSS_ID SnapshotSetId) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::CommitSnapshots(VSS_ID SnapshotSetId) >> +{ >> + HRESULT hr = S_OK; >> + HANDLE hEvent, hEvent2; >> + >> + hEvent = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_FROZEN); >> + if (hEvent == INVALID_HANDLE_VALUE) { >> + hr = E_FAIL; >> + goto out; >> + } >> + >> + hEvent2 = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_THAW); >> + if (hEvent == INVALID_HANDLE_VALUE) { >> + CloseHandle(hEvent); >> + hr = E_FAIL; >> + goto out; >> + } >> + >> + SetEvent(hEvent); >> + if (WaitForSingleObject(hEvent2, 60*1000) != WAIT_OBJECT_0) { >> + hr = E_ABORT; >> + } >> + >> + CloseHandle(hEvent2); >> + CloseHandle(hEvent); >> +out: >> + return hr; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::PostCommitSnapshots( >> + VSS_ID SnapshotSetId, LONG lSnapshotsCount) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::PreFinalCommitSnapshots(VSS_ID SnapshotSetId) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId) >> +{ >> + return S_OK; >> +} >> + >> +/* >> + * IVssProviderNotifications methods >> + */ >> + >> +STDMETHODIMP CQGAVssProvider::OnLoad(IUnknown *pCallback) >> +{ >> + return S_OK; >> +} >> + >> +STDMETHODIMP CQGAVssProvider::OnUnload(BOOL bForceUnload) >> +{ >> + return S_OK; >> +} >> + >> + >> +/* >> + * CQGAVssProviderFactory class >> + */ >> + >> +class CQGAVssProviderFactory : public IClassFactory >> +{ >> +public: >> + STDMETHODIMP QueryInterface(REFIID riid, void **ppv); >> + STDMETHODIMP_(ULONG) AddRef(); >> + STDMETHODIMP_(ULONG) Release(); >> + STDMETHODIMP CreateInstance( >> + IUnknown *pUnknownOuter, REFIID iid, void **ppv); >> + STDMETHODIMP LockServer(BOOL bLock) { return E_NOTIMPL; } >> +private: >> + long m_nRefCount; >> +}; >> + >> +STDMETHODIMP CQGAVssProviderFactory::QueryInterface(REFIID riid, void **ppv) >> +{ >> + if (riid == IID_IUnknown || riid == IID_IClassFactory) { >> + *ppv = static_cast<void*>(this); >> + AddRef(); >> + return S_OK; >> + } >> + *ppv = NULL; >> + return E_NOINTERFACE; >> +} >> + >> +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::AddRef() >> +{ >> + LockModule(TRUE); >> + return 2; >> +} >> + >> +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::Release() >> +{ >> + LockModule(FALSE); >> + return 1; >> +} >> + >> +STDMETHODIMP CQGAVssProviderFactory::CreateInstance( >> + IUnknown *pUnknownOuter, REFIID iid, void **ppv) >> +{ >> + if (pUnknownOuter) { >> + return CLASS_E_NOAGGREGATION; >> + } >> + CQGAVssProvider *pObj = new CQGAVssProvider; >> + if (!pObj) { >> + return E_OUTOFMEMORY; >> + } >> + return pObj->QueryInterface(iid, ppv); >> +} >> + >> + >> +/* >> + * DLL functions >> + */ >> + >> +STDAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv) >> +{ >> + static CQGAVssProviderFactory factory; >> + >> + *ppv = NULL; >> + if (IsEqualCLSID(rclsid, CLSID_QGAVSSProvider)) { >> + return factory.QueryInterface(riid, ppv); >> + } >> + return CLASS_E_CLASSNOTAVAILABLE; >> +} >> + >> +STDAPI DllCanUnloadNow() >> +{ >> + return g_nComObjsInUse == 0 ? S_OK : S_FALSE; >> +} >> + >> +EXTERN_C >> +BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved) >> +{ >> + switch (dwReason) { >> + >> + case DLL_PROCESS_ATTACH: >> + g_hinstDll = hinstDll; >> + DisableThreadLibraryCalls(hinstDll); >> + break; >> + } >> + >> + return TRUE; >> +} >
Il 26/06/2013 16:32, Laszlo Ersek ha scritto: >>> >> +#define EVENT_NAME_FROZEN "Global\\QGAVSSEvent-frozen" >>> >> +#define EVENT_NAME_THAW "Global\\QGAVSSEvent-thaw" > No idea what I'm talking about, but since we're qualifying the second > component with the "QGAVSSEvent" prefix, shouldn't we just replace the > first component ("Global") with it? Or would it effect "event routing"? Yes: http://msdn.microsoft.com/en-us/library/windows/desktop/ms684305%28v=vs.85%29.aspx says Terminal Services: The name can have a "Global\" or "Local\" prefix to explicitly open an object in the global or session namespace. The remainder of the name can contain any character except the backslash character (\). For more information, see Kernel Object Namespaces. Paolo
On 6/26/13 10:32 , "Laszlo Ersek" <lersek@redhat.com> wrote: >On 06/25/13 18:03, Laszlo Ersek wrote: >> On 06/06/13 17:06, Tomoki Sekiyama wrote: > >Comparing the .def file and the provider header file: >... >(a) what is the reason for not listing DllCanUnloadNow() and >DllGetClassObject() in the header file? Because these two are reserved functions for COM, and not Referenced from qemu-ga.exe. >(b) Does STDAPI imply a return type? Or are you just going with the >implicit "int"? (Based on my encounters with EFIAPI, I think it's the >latter.) It is extracted as 'extern "C" HRESULT __stdcall'. (__stdcall specifies calling convention.) >>>+#define EVENT_NAME_FROZEN "Global\\QGAVSSEvent-frozen" >>> +#define EVENT_NAME_THAW "Global\\QGAVSSEvent-thaw" > >No idea what I'm talking about, but since we're qualifying the second >component with the "QGAVSSEvent" prefix, shouldn't we just replace the >first component ("Global") with it? Or would it effect "event routing"? >(Tangential question, anyway.) Latter. "Global\\" is a reserved namespace in Windows used for communication between services and the other program, so we can't omit this. >>> +const GUID CLSID_COMAdminCatalog = { 0xF618C514, 0xDFB8, 0x11d1, >>> + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; >>> +const GUID IID_ICOMAdminCatalog = { 0xDD662187, 0xDFC2, 0x11d1, >>> + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; >>> +const GUID CLSID_WbemLocator = { 0x4590f811, 0x1d3a, 0x11d0, >>> + {0x89, 0x1f, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; >>> +const GUID IID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf, >>> + {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; > >Would it be possible to make these static? Not too important of course. Some of these are declared as "extern" in headers in MinGW or VSS SDK, so unfortunately we can't make them static. >>>+ >>> +static void errmsg(DWORD err, const char *text) >>> +{ >>> + char *msg = NULL, *nul = strchr(text, '('); >>> + int len = nul ? nul - text : -1; >>> + >>> + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | >>> + FORMAT_MESSAGE_FROM_SYSTEM | >>>FORMAT_MESSAGE_IGNORE_INSERTS, >>> + NULL, err, MAKELANGID(LANG_NEUTRAL, >>>SUBLANG_DEFAULT), >>> + (char *)&msg, 0, NULL); > >(The FormatMessage() specification in MSDN is simply incredible. >"(char *)&msg", seriously? Your code is correct, but the interface is >unbelievable.) Yeah, really... >>> + printf("%.*s. (Error: %lx) %s\n", len, text, err, msg); > >(a) Since we're printing an error message here, I suspect it should go >to stderr. OK. >(b) In case "text" doesn't contain an opening paren, "len" is set to -1. >"A negative precision is taken as if the precision were omitted", IOW >the entire string will be printed. Clever! Maybe need some comments... >(c) Does DWORD expand to "uint32_t"? In that case "%lx" is too big on >64-bit (LP64). We should probably use "%x" or "%"PRIx32. (Or do we build >qemu-ga.exe only for 32-bit? I seem to remember something like that.) DWORD is typedef as unsigned long int. MinGW64 uses LLP64 model, so it is 32bit both in 32-bit and 64-bit MinGW. So if I use %x here, gcc outputs: qga/vss-win32-provider/install.cpp:44:57: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'DWORD {aka long unsigned int}' [-Wformat] >>>+ snprintf(buf, sizeof(buf), "%s%s. (Error: %lx) %s\n", text, opt, >>>err, msg); >>> + MessageBox(NULL, buf, "Error from " QGA_PROVIDER_NAME, >>>MB_OK|MB_ICONERROR); > >Do you need a trailing newline for a message box? No. I will remove "\n". Thanks. >>> +/* Lookup Administrators group name from winmgmt */ >>> +static HRESULT GetAdminName(_bstr_t &name) >>> +{ > >I can see that you want to call GetAdminName() in chk(), hence you must >return a HRESULT and store the value through the parameter list. >However, can we keep the C++ features to a minimum? I'd prefer if the >non-const reference were replaced with a pointer-to-_bstr_t. >(Tangential, change only if you respin anyway and don't mind it.) OK, I will make this into 'static HRESULT GetAdminName(_bstr_t *name)'. >>> + if (!pEnum) { >>> + errmsg(E_FAIL, "Failed to query for Administrators"); >>> + goto out; > >I think you forgot to set "hr" to something ugly -- the code below the >"out" label will simply return it, and "hr" is currently something nice. >(The last chk() succeeded.) I will add "hr = E_FAIL;" here. Thanks. >>> + } > >This branch is probably for the case when ExecQuery() succeeds, but >doesn't return any rows, right? We must not come here (while the reference is accurate). It is just in case. >>> + chk(pEnum->Next(WBEM_INFINITE, 1, &pWobj, &returned)); >>> + if (returned == 0) { >>> + errmsg(E_FAIL, "Failed to query for Administrators"); >>> + goto out; >>> + } >>> + >>> + chk(pWobj->Get(_bstr_t(L"Name"), 0, &var, 0, 0)); >>> + name = var; > >This assigns a _variant_t to a _bstr_t (... for which I proposed *name = >var above, but it's irrelevant now). > >Can this conversion (or the assignment operator) fail? If so, does >either throw an exception? Hm, yes. I will add try ... catch here. >>> +out: >>> + if (pLoc) { >>> + pLoc->Release(); >>> + } >>> + if (pSvc) { >>> + pSvc->Release(); >>> + } >>> + if (pEnum) { >>> + pEnum->Release(); >>> + } >>> + if (pWobj) { >>> + pWobj->Release(); >>> + } >>> + return hr; >>> +} > >I'm sure this is idiomatic code, but do these pointers point to static / >long-term instances? I'm missing the equivalent of "delete pLoc" and so >on. Each Release() member function gets the implicit "this" parameter >here, but do they free it too? (Honestly, I can't decide which would be >worse, if they did or they didn't.) > >... Ah, it's probably reference counting. OK. Right, it's decrementing reference counters. >>>+ if (!g_hinstDll) { >>>+ errmsg(E_FAIL, "Failed to initialize DLL"); >>>+ goto out; > >OK, "hr" is E_FAIL here. Ah, this must be rewritten to "return", as described below. >>>+ } >>>+ >>>+ if (VSSCheckOSVersion() == S_FALSE) { >>> + printf("VSS provider is not supported in this OS version.\n"); > >I suggest stderr. OK. >>> + return S_FALSE; /* VSS feature is disabled */ > >Somewhat inconsistent with the above, you issue "return" although you >used "goto out" before. No resources are leaked so it's no problem in >practice. (You could have written "return" above.) This must be return, because we must not call CoUninitialize() in the "out:" path. I will rewrite "goto out" above into "return". >>> + } >>> + >>> + COMUnregister(); > >What requires / justifies this? (I can see another call below, on the >error path.) I think whatever COMUnregister() would release depends >first on the success of COMRegister(), doesn't it? It was intended to clean up something in the catalog with the same name, just in case. >You've told Paolo, > >On 06/26/13 00:31, Tomoki Sekiyama wrote: >> COMRegister and COMUnregister are called by`qemu-ga -s install`, >> to register/unregister the DLL into/from COM+ application catalogue. > >and indeed I can see a COMRegister() call in 09/10. Nonetheless that >doesn't seem to justify this call to COMUnregister() -- if the DLL has >been previously registered in the application catalog, a new >registration attempt should fail. Okey, then I will remove this COMUnregister(). >>> + >>> + chk(CoInitialize(NULL)); > >Can this fail in practice? If so, we could have error handling trouble, >because "out:" will call CoUninitialize() without any successful >initialization. Ah, it won't fail. I will remove chk() here. >>>+ /* Install COM+ Component */ >>> + >>> + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), >>> + (IDispatch **)&pApps)); >>> + chk(pApps->Populate()); >>> + chk(pApps->Add((IDispatch **)&pObj)); >>> + chk(put_Value(pObj, L"Name", QGA_PROVIDER_LNAME)); >>> + chk(put_Value(pObj, L"Description", QGA_PROVIDER_LNAME)); >>> + chk(put_Value(pObj, L"ApplicationAccessChecksEnabled", true)); >>> + chk(put_Value(pObj, L"Authentication", short(6))); >>> + chk(put_Value(pObj, L"AuthenticationCapability", short(2))); >>> + chk(put_Value(pObj, L"ImpersonationLevel", short(2))); >>> + chk(pApps->SaveChanges(&n)); > >This looks like a globally visible change that is not rolled back if >something fails later in this function. Is the COMUnregister() call on >the error handling path supposed to undo these? Complicated :) This code is creating a tree, whose root is an "Application" with the name of QGA_PROVIDER_LNAME. COMUnregister will erase whole tree by erasing the root, and every modification below will be rolled back. >Then, should we check "n", or does the retval check suffice? If something fails, it returns error (E_***), so I think the retval check is good enough. >>> + >>> + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { >>> + errmsg(GetLastError(), "GetModuleFileName failed"); >>> + goto out; > >"hr" has not been set to a failure code. Oops, I will add a code to set hr here, >>> + } >>> + n = strlen(dllPath); >>> + if (n < 3) { >>> + errmsg(E_FAIL, "Failed to lookup dll"); > >Presumably you wanted to set "hr" and jump to "out" as well. And here too, with "goto out;". >>> + } >>> + strcpy(tlbPath, dllPath); >>> + strcpy(tlbPath+n-3, "TLB"); >>> + printf("Registering " QGA_PROVIDER_NAME ":\n"); >>> + printf(" %s\n", dllPath); >>> + printf(" %s\n", tlbPath); > >These are arguably diagnostic messages (destined for stderr), but I >won't press it :) OK. >>> + if (!PathFileExists(tlbPath)) { >>> + errmsg(ERROR_FILE_NOT_FOUND, "Failed to lookup tlb"); >>> + goto out; > >"hr" not set properly. And here too. ... >>> + CoUninitialize(); > >Another question: can you nest CoInitialize() calls (and more >importantly, does CoUninitialize() respect that)? Consider the following >call chain: > >COMRegister() > CoInitialize() > COMUnregister() -- on the "out:" path > DllUnregisterServer() > CoInitialize() -- init after init > CoUninitialize() > CoInitialize() > CoUninitialize() > CoUninitialize() -- right here, uninit after uninit > >Will these work (especially the final two)? Yes, it can be nested, as far as CoInitialize and CoUninitialize are balanced. >>>+/* Unregister this module from COM+ Applications Catalog */ >>> +STDAPI COMUnregister(void) >>> +{ >>> + HRESULT hr; >>> + IUnknown *pUnknown = NULL; >>> + ICOMAdminCatalog *pCatalog = NULL; >>> + ICatalogCollection *pColl = NULL; >>> + ICatalogObject *pObj = NULL; >>> + _variant_t var; >>> + long i, n; >>> + >>> + if (VSSCheckOSVersion() == S_FALSE) { >>> + printf("VSS provider is not supported in this OS version.\n"); > >I suggest stderr again. OK. >>> + return S_FALSE; /* VSS feature is disabled */ >>> + } >>> + >>> + chk(DllUnregisterServer()); > >Apparently DllUnregisterServer() can't fail. (And it shouldn't, we >depend on it, since COMUnregister() is also used as destructor after >partial construction, so we must not bail here.) I propose to change the >return type of DllUnregisterServer() to void, just to drive the point >home. OK. I can't make it void (because it is a reserved interface for COM), but will remove chk here. >>> + >>> + chk(CoInitialize(NULL)); > >(same question as before -- can this fail? See unconditional uninit at >the bottom.) Will remove chk from here too. >I'll continue from here. Thanks a lot for reviewing this long complicated windows code! Tomoki Sekiyama
On 06/26/13 16:32, Laszlo Ersek wrote: > On 06/25/13 18:03, Laszlo Ersek wrote: >> On 06/06/13 17:06, Tomoki Sekiyama wrote: >>> +/* Unregister this module from COM+ Applications Catalog */ >>> +STDAPI COMUnregister(void) >>> +{ >>> + HRESULT hr; >>> + IUnknown *pUnknown = NULL; >>> + ICOMAdminCatalog *pCatalog = NULL; >>> + ICatalogCollection *pColl = NULL; >>> + ICatalogObject *pObj = NULL; >>> + _variant_t var; >>> + long i, n; >>> + >>> + if (VSSCheckOSVersion() == S_FALSE) { >>> + printf("VSS provider is not supported in this OS version.\n"); >>> + return S_FALSE; /* VSS feature is disabled */ >>> + } >>> + >>> + chk(DllUnregisterServer()); >>> + >>> + chk(CoInitialize(NULL)); picking up here >>> + chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER, >>> + IID_IUnknown, (void **)&pUnknown)); >>> + chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog, (void **)&pCatalog)); >>> + >>> + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), >>> + (IDispatch **)&pColl)); >>> + chk(pColl->Populate()); >>> + >>> + chk(pColl->get_Count(&n)); >>> + for (i = n - 1; i >= 0; i--) { >>> + chk(pColl->get_Item(i, (IDispatch **)&pObj)); >>> + chk(pObj->get_Value(_bstr_t(L"Name"), &var)); >>> + if (var == _variant_t(QGA_PROVIDER_LNAME)) { >>> + printf("Removing COM+ Application: %S\n", (wchar_t *)_bstr_t(var)); (stderr) >>> + chk(pColl->Remove(i)); >>> + } I think you leak a pObj reference here, at the end of the iteration. The next round will set pObj to something else; I think we should call pObj->Release() here, and set pObj to NULL (for the case when this is the last iteration). I'm not sure if you're allowed to call pObj->Release() after the pColl()->Remove(i) call. So maybe call pObj->Release() in an else branch. (In this case however the out: logic should be modified as well.) >>> + } >>> + chk(pColl->SaveChanges(&n)); Right, there's not much to do if deregistration fails... >>> + >>> +out: >>> + if (pUnknown) { >>> + pUnknown->Release(); >>> + } >>> + if (pCatalog) { >>> + pCatalog->Release(); >>> + } >>> + if (pColl) { >>> + pColl->Release(); >>> + } >>> + if (pObj) { >>> + pObj->Release(); >>> + } >>> + CoUninitialize(); >>> + >>> + return hr; >>> +} >>> + >>> + >>> +static BOOL CreateRegistryKey(LPCTSTR key, LPCTSTR value, LPCTSTR data) >>> +{ >>> + HKEY hKey; >>> + LONG ret; >>> + DWORD size; >>> + >>> + ret = RegCreateKeyEx(HKEY_CLASSES_ROOT, key, 0, NULL, >>> + REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, &hKey, NULL); >>> + if (ret != ERROR_SUCCESS) { >>> + goto out; >>> + } >>> + >>> + if (data != NULL) { >>> + size = (lstrlen(data) + 1) * sizeof(TCHAR); I think we should drop the multiplication by sizeof(TCHAR), it's 1. According to MSDN, TCHAR could be something wider than "char" (ie. WCHAR) "for Unicode platforms". However in all your CreateRegistryKey() calls, the argument passed as "data" depends on sizeof(TCHAR)==1, directly or indirectly. I think it's best to be honest about it. For example, >>> + } else { >>> + size = 0; >>> + } >>> + >>> + ret = RegSetValueEx(hKey, value, 0, REG_SZ, (LPBYTE)data, size); >>> + RegCloseKey(hKey); >>> + >>> +out: >>> + if (ret != ERROR_SUCCESS) { >>> + /* We cannot printf here, and need MessageBox to report an error. */ >>> + errmsg_dialog(ret, "Cannot add registry ", key); right here we equate (const char *) with LPCTSTR (by virtue of the third arg being "key"). You might also replace lstrlen() with strlen() for consistency. (Tangential anyhow.) >>> + return FALSE; >>> + } >>> + return TRUE; >>> +} >>> + >>> +/* Register this dll as a VSS provider */ >>> +STDAPI DllRegisterServer(void) >>> +{ >>> + IVssAdmin *pVssAdmin = NULL; >>> + HRESULT hr = E_FAIL; >>> + char dllPath[MAX_PATH]; >>> + char key[256]; >>> + >>> + CoInitialize(NULL); >>> + >>> + if (!g_hinstDll) { >>> + errmsg_dialog(hr, "Module instance is not available"); >>> + goto out; >>> + } >>> + >>> + /* Add this module to registery */ >>> + >>> + sprintf(key, "CLSID\\%s", g_szClsid); >>> + if (!CreateRegistryKey(key, NULL, g_szClsid)) { >>> + goto out; >>> + } >>> + >>> + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { >>> + errmsg_dialog(GetLastError(), "GetModuleFileName failed"); >>> + goto out; >>> + } >>> + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); >>> + if (!CreateRegistryKey(key, NULL, dllPath)) { >>> + goto out; >>> + } >>> + >>> + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); (you could reuse "key" from the previous sprintf()) >>> + if (!CreateRegistryKey(key, "ThreadingModel", "Apartment")) { >>> + goto out; >>> + } >>> + >>> + sprintf(key, "CLSID\\%s\\ProgID", g_szClsid); >>> + if (!CreateRegistryKey(key, NULL, g_szProgid)) { >>> + goto out; >>> + } >>> + >>> + if (!CreateRegistryKey(g_szProgid, NULL, QGA_PROVIDER_NAME)) { >>> + goto out; >>> + } >>> + >>> + sprintf(key, "%s\\CLSID", g_szProgid); >>> + if (!CreateRegistryKey(key, NULL, g_szClsid)) { >>> + goto out; >>> + } >>> + >>> + hr = CoCreateInstance(CLSID_VSSCoordinator, >>> + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); (indentation) >>> + if (FAILED(hr)) { >>> + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); >>> + goto out; >>> + } >>> + >>> + hr = pVssAdmin->RegisterProvider( >>> + g_gProviderId, CLSID_QGAVSSProvider, >>> + const_cast<WCHAR*>(QGA_PROVIDER_LNAME), VSS_PROV_SOFTWARE, >>> + const_cast<WCHAR*>(QGA_PROVIDER_VERSION), g_gProviderVersion); (indentation) >>> + if (FAILED(hr)) { >>> + errmsg_dialog(hr, "RegisterProvider failed"); >>> + goto out; (goto unnecessary) >>> + } >>> + >>> +out: >>> + if (pVssAdmin) { >>> + pVssAdmin->Release(); >>> + } >>> + CoUninitialize(); >>> + >>> + if (FAILED(hr)) { >>> + DllUnregisterServer(); >>> + } >>> + >>> + return hr; >>> +} >>> + >>> +/* Unregister this VSS hardware provider from the system */ >>> +STDAPI DllUnregisterServer(void) >>> +{ >>> + TCHAR key[256]; >>> + IVssAdmin *pVssAdmin = NULL; >>> + >>> + CoInitialize(NULL); >>> + >>> + HRESULT hr = CoCreateInstance(CLSID_VSSCoordinator, >>> + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); (indentation, maybe) >>> + if (SUCCEEDED(hr)) { >>> + hr = pVssAdmin->UnregisterProvider(g_gProviderId); >>> + pVssAdmin->Release(); >>> + } else { >>> + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); >>> + } >>> + >>> + sprintf(key, "CLSID\\%s", g_szClsid); >>> + SHDeleteKey(HKEY_CLASSES_ROOT, key); >>> + SHDeleteKey(HKEY_CLASSES_ROOT, g_szProgid); >>> + >>> + CoUninitialize(); >>> + >>> + return S_OK; /* Uninstall should never fail */ >>> +} Seems OK. >>> + >>> + >>> +/* Support functions for _bstr_t in MinGW: Originally written by: Diaa Sami */ >>> + Where does this code originate from? What is its license? >>> +void __stdcall _com_issue_error(HRESULT hr) >>> +{ >>> + printf("_com_issue_error() called with parameter HRESULT = %lu", hr); >>> +} This wouldn't be hard to reimplement anyway, just print the message to stderr. Plus it's missing \n. I googled the function name, and some people put up a message box here. Not sure under what circumstances this function is called. >>> + >>> +namespace _com_util >>> +{ >>> + char * __stdcall ConvertBSTRToString(BSTR bstr) >>> + { >>> + const unsigned int stringLength = lstrlenW(bstr); >>> + char *const ascii = new char [stringLength + 1]; >>> + >>> + wcstombs(ascii, bstr, stringLength + 1); >>> + >>> + return ascii; >>> + } The BSTR, _bstr_t, LPCTSTR etc mess is incredible. Is BSTR just (wchar_t*)? These COM interfaces seem broken by the way; how does one report a conversion error? wcstombs() is locale-dependent and can fail. I'll just pretend that whatever "bstr" contains in UTF-16 will always be representable in pure ASCII. >>> + >>> + BSTR __stdcall ConvertStringToBSTR(const char *const ascii) >>> + { >>> + const unsigned int stringLength = lstrlenA(ascii); >>> + BSTR bstr = SysAllocStringLen(NULL, stringLength); >>> + >>> + mbstowcs(bstr, ascii, stringLength + 1); >>> + >>> + return bstr; >>> + } >>> +} >>> diff --git a/qga/vss-win32-provider/provider.cpp b/qga/vss-win32-provider/provider.cpp >>> new file mode 100644 >>> index 0000000..90a3d25 >>> --- /dev/null >>> +++ b/qga/vss-win32-provider/provider.cpp >>> @@ -0,0 +1,479 @@ >>> +/* >>> + * QEMU Guest Agent win32 VSS Provider implementations >>> + * >>> + * Copyright Hitachi Data Systems Corp. 2013 >>> + * >>> + * Authors: >>> + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >>> + * See the COPYING file in the top-level directory. >>> + */ >>> + >>> +#include <stdio.h> >>> +#include "../vss-win32.h" >>> +#include "inc/win2003/vscoordint.h" >>> +#include "inc/win2003/vsprov.h" >>> + >>> +static long g_nComObjsInUse; >>> +HINSTANCE g_hinstDll; >>> + >>> +/* VSS common GUID's */ >>> + >>> +const CLSID CLSID_VSSCoordinator = { 0xE579AB5F, 0x1CC4, 0x44b4, >>> + {0xBE, 0xD9, 0xDE, 0x09, 0x91, 0xFF, 0x06, 0x23} }; >>> +const IID IID_IVssAdmin = { 0x77ED5996, 0x2F63, 0x11d3, >>> + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; >>> + >>> +const IID IID_IVssHardwareSnapshotProvider = { 0x9593A157, 0x44E9, 0x4344, >>> + {0xBB, 0xEB, 0x44, 0xFB, 0xF9, 0xB0, 0x6B, 0x10} }; >>> +const IID IID_IVssSoftwareSnapshotProvider = { 0x609e123e, 0x2c5a, 0x44d3, >>> + {0x8f, 0x01, 0x0b, 0x1d, 0x9a, 0x47, 0xd1, 0xff} }; >>> +const IID IID_IVssProviderCreateSnapshotSet = { 0x5F894E5B, 0x1E39, 0x4778, >>> + {0x8E, 0x23, 0x9A, 0xBA, 0xD9, 0xF0, 0xE0, 0x8C} }; >>> +const IID IID_IVssProviderNotifications = { 0xE561901F, 0x03A5, 0x4afe, >>> + {0x86, 0xD0, 0x72, 0xBA, 0xEE, 0xCE, 0x70, 0x04} }; >>> + >>> +const IID IID_IVssEnumObject = { 0xAE1C7110, 0x2F60, 0x11d3, >>> + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; >>> + >>> + >>> +void LockModule(BOOL block) Did you mean "lock" instead of "block"? >>> +{ >>> + if (block) { >>> + InterlockedIncrement(&g_nComObjsInUse); >>> + } else { >>> + InterlockedDecrement(&g_nComObjsInUse); >>> + } >>> +} >>> + >>> +/* Empty enumerator for VssObject */ >>> + >>> +class CQGAVSSEnumObject : public IVssEnumObject >>> +{ >>> +public: >>> + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); >>> + STDMETHODIMP_(ULONG) AddRef(); >>> + STDMETHODIMP_(ULONG) Release(); >>> + >>> + /* IVssEnumObject Methods */ >>> + STDMETHODIMP Next( >>> + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched); >>> + STDMETHODIMP Skip(ULONG celt); >>> + STDMETHODIMP Reset(void); >>> + STDMETHODIMP Clone(IVssEnumObject **ppenum); >>> + >>> + /* CQGAVSSEnumObject Methods */ >>> + CQGAVSSEnumObject(); >>> + ~CQGAVSSEnumObject(); >>> + >>> +private: >>> + long m_nRefCount; >>> +}; >>> + >>> +CQGAVSSEnumObject::CQGAVSSEnumObject() >>> +{ >>> + m_nRefCount = 0; I think idiomatic C++ might want to put it on the member initializer list, but this way is correct as well, and maybe more understandable (and I asked for minimizing C++ features :)) so OK. >>> + LockModule(TRUE); >>> +} >>> + >>> +CQGAVSSEnumObject::~CQGAVSSEnumObject() >>> +{ >>> + LockModule(FALSE); >>> +} >>> + >>> +STDMETHODIMP CQGAVSSEnumObject::QueryInterface(REFIID riid, void **ppObj) >>> +{ >>> + if (riid == IID_IUnknown || riid == IID_IVssEnumObject) { >>> + *ppObj = static_cast<void*>(static_cast<IVssEnumObject*>(this)); Storing the address of the base object, right? Actually (based on what the other class does below), do you think it's right to return the base object's address for IID_IUnknown too, rather than static_cast<void*>(this)? ... We have a single base object here so in practice these addresses should be the same. >>> + AddRef(); >>> + return S_OK; >>> + } >>> + ppObj = NULL; Indirection operator missing ("*ppObj = NULL")? >>> + return E_NOINTERFACE; >>> +} >>> + >>> +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::AddRef() >>> +{ >>> + return InterlockedIncrement(&m_nRefCount); >>> +} >>> + >>> +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::Release() >>> +{ >>> + long nRefCount = InterlockedDecrement(&m_nRefCount); >>> + if (m_nRefCount == 0) { >>> + delete this; I guess we're dead sure the object was allocated with "new". The destructor invoked here may also remove the last reference to the module. No further access to this->XXX is allowed. http://www.parashift.com/c++-faq-lite/delete-this.html OK. >>> + } >>> + return nRefCount; >>> +} >>> + >>> +STDMETHODIMP CQGAVSSEnumObject::Next( >>> + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched) >>> +{ >>> + *pceltFetched = 0; >>> + return S_FALSE; >>> +} >>> + >>> +STDMETHODIMP CQGAVSSEnumObject::Skip(ULONG celt) >>> +{ >>> + return S_FALSE; >>> +} >>> + >>> +STDMETHODIMP CQGAVSSEnumObject::Reset(void) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVSSEnumObject::Clone(IVssEnumObject **ppenum) >>> +{ >>> + return E_NOTIMPL; >>> +} (This class seems to track references to the module and do nothing else.) >>> + >>> + >>> +/* QGAVssProvider */ >>> + >>> +class CQGAVssProvider : >>> + public IVssSoftwareSnapshotProvider, >>> + public IVssProviderCreateSnapshotSet, >>> + public IVssProviderNotifications >>> +{ >>> +public: >>> + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); >>> + STDMETHODIMP_(ULONG) AddRef(); >>> + STDMETHODIMP_(ULONG) Release(); >>> + >>> + /* IVssSoftwareSnapshotProvider Methods */ >>> + STDMETHODIMP SetContext(LONG lContext); >>> + STDMETHODIMP GetSnapshotProperties( >>> + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp); >>> + STDMETHODIMP Query( >>> + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, >>> + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum); >>> + STDMETHODIMP DeleteSnapshots( >>> + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, >>> + BOOL bForceDelete, LONG *plDeletedSnapshots, >>> + VSS_ID *pNondeletedSnapshotID); >>> + STDMETHODIMP BeginPrepareSnapshot( >>> + VSS_ID SnapshotSetId, VSS_ID SnapshotId, >>> + VSS_PWSZ pwszVolumeName, LONG lNewContext); >>> + STDMETHODIMP IsVolumeSupported( >>> + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider); >>> + STDMETHODIMP IsVolumeSnapshotted( >>> + VSS_PWSZ pwszVolumeName, BOOL *pbSnapshotsPresent, >>> + LONG *plSnapshotCompatibility); >>> + STDMETHODIMP SetSnapshotProperty( >>> + VSS_ID SnapshotId, VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, >>> + VARIANT vProperty); >>> + STDMETHODIMP RevertToSnapshot(VSS_ID SnapshotId); >>> + STDMETHODIMP QueryRevertStatus(VSS_PWSZ pwszVolume, IVssAsync **ppAsync); >>> + >>> + /* IVssProviderCreateSnapshotSet Methods */ >>> + STDMETHODIMP EndPrepareSnapshots(VSS_ID SnapshotSetId); >>> + STDMETHODIMP PreCommitSnapshots(VSS_ID SnapshotSetId); >>> + STDMETHODIMP CommitSnapshots(VSS_ID SnapshotSetId); >>> + STDMETHODIMP PostCommitSnapshots( >>> + VSS_ID SnapshotSetId, LONG lSnapshotsCount); >>> + STDMETHODIMP PreFinalCommitSnapshots(VSS_ID SnapshotSetId); >>> + STDMETHODIMP PostFinalCommitSnapshots(VSS_ID SnapshotSetId); >>> + STDMETHODIMP AbortSnapshots(VSS_ID SnapshotSetId); >>> + >>> + /* IVssProviderNotifications Methods */ >>> + STDMETHODIMP OnLoad(IUnknown *pCallback); >>> + STDMETHODIMP OnUnload(BOOL bForceUnload); >>> + >>> + /* CQGAVssProvider Methods */ >>> + CQGAVssProvider(); >>> + ~CQGAVssProvider(); >>> + >>> +private: >>> + long m_nRefCount; >>> +}; >>> + >>> +CQGAVssProvider::CQGAVssProvider() >>> +{ >>> + m_nRefCount = 0; >>> + LockModule(TRUE); >>> +} >>> + >>> +CQGAVssProvider::~CQGAVssProvider() >>> +{ >>> + LockModule(FALSE); >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::QueryInterface(REFIID riid, void **ppObj) >>> +{ >>> + if (riid == IID_IUnknown) { >>> + *ppObj = static_cast<void*>(this); >>> + AddRef(); >>> + return S_OK; >>> + } else if (riid == IID_IVssSoftwareSnapshotProvider) { (No "else" needed if "return" is the last statement in the "if"'s block.) >>> + *ppObj = static_cast<void*>( >>> + static_cast<IVssSoftwareSnapshotProvider*>(this)); >>> + AddRef(); >>> + return S_OK; >>> + } else if (riid == IID_IVssProviderCreateSnapshotSet) { >>> + *ppObj = static_cast<void*>( >>> + static_cast<IVssProviderCreateSnapshotSet*>(this)); >>> + AddRef(); >>> + return S_OK; >>> + } else if (riid == IID_IVssProviderNotifications) { >>> + *ppObj = static_cast<void*>( >>> + static_cast<IVssProviderNotifications*>(this)); >>> + AddRef(); >>> + return S_OK; >>> + } >>> + *ppObj = NULL; >>> + return E_NOINTERFACE; >>> +} Seems OK. Could be reworked into a switch too to save some space, maybe, but don't bother. >>> + >>> +STDMETHODIMP_(ULONG) CQGAVssProvider::AddRef() >>> +{ >>> + return InterlockedIncrement(&m_nRefCount); >>> +} >>> + >>> +STDMETHODIMP_(ULONG) CQGAVssProvider::Release() >>> +{ >>> + long nRefCount = InterlockedDecrement(&m_nRefCount); >>> + if (m_nRefCount == 0) { >>> + delete this; >>> + } >>> + return nRefCount; >>> +} >>> + >>> + >>> +/* >>> + * IVssSoftwareSnapshotProvider methods >>> + */ >>> + >>> +STDMETHODIMP CQGAVssProvider::SetContext(LONG lContext) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::GetSnapshotProperties( >>> + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp) >>> +{ >>> + return VSS_E_OBJECT_NOT_FOUND; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::Query( >>> + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, >>> + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum) >>> +{ >>> + *ppEnum = new CQGAVSSEnumObject; >>> + (*ppEnum)->AddRef(); >>> + return S_OK; >>> +} OK, so the pattern is, refcount is always incremented in QueryXXXX(), no matter whether we return the address of one of our own base objects (in which case we increment our own refcount), or we create a new object (strictly with "new") and increment the refcount to 1 on that. The caller of QueryXXXX() is responsible for calling Release() later. >>> + >>> +STDMETHODIMP CQGAVssProvider::DeleteSnapshots( >>> + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, >>> + BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID *pNondeletedSnapshotID) >>> +{ >>> + return E_NOTIMPL; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot( >>> + VSS_ID SnapshotSetId, VSS_ID SnapshotId, >>> + VSS_PWSZ pwszVolumeName, LONG lNewContext) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::IsVolumeSupported( >>> + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider) >>> +{ >>> + *pbSupportedByThisProvider = TRUE; >>> + >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::IsVolumeSnapshotted(VSS_PWSZ pwszVolumeName, >>> + BOOL *pbSnapshotsPresent, LONG *plSnapshotCompatibility) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::SetSnapshotProperty(VSS_ID SnapshotId, >>> + VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, VARIANT vProperty) >>> +{ >>> + return E_NOTIMPL; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::RevertToSnapshot(VSS_ID SnapshotId) >>> +{ >>> + return E_NOTIMPL; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::QueryRevertStatus( >>> + VSS_PWSZ pwszVolume, IVssAsync **ppAsync) >>> +{ >>> + return S_OK; >>> +} Shouldn't you set *ppAsync to something here? (No idea, just asking -- S_OK could imply something on output.) Same for IsVolumeSnapshotted() above. >>> + >>> + >>> +/* >>> + * IVssProviderCreateSnapshotSet methods >>> + */ >>> + >>> +STDMETHODIMP CQGAVssProvider::EndPrepareSnapshots(VSS_ID SnapshotSetId) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::PreCommitSnapshots(VSS_ID SnapshotSetId) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::CommitSnapshots(VSS_ID SnapshotSetId) >>> +{ >>> + HRESULT hr = S_OK; >>> + HANDLE hEvent, hEvent2; >>> + >>> + hEvent = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_FROZEN); >>> + if (hEvent == INVALID_HANDLE_VALUE) { >>> + hr = E_FAIL; >>> + goto out; >>> + } >>> + >>> + hEvent2 = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_THAW); >>> + if (hEvent == INVALID_HANDLE_VALUE) { >>> + CloseHandle(hEvent); >>> + hr = E_FAIL; >>> + goto out; >>> + } >>> + >>> + SetEvent(hEvent); I think we should add a comment here -- this is where qemu-ga.exe / libvirt will create the snapshot. >>> + if (WaitForSingleObject(hEvent2, 60*1000) != WAIT_OBJECT_0) { >>> + hr = E_ABORT; >>> + } >>> + >>> + CloseHandle(hEvent2); >>> + CloseHandle(hEvent); >>> +out: >>> + return hr; >>> +} Seems correct in general. However I think you could shave off a few lines from this function by reorganizing the "out:" path, for example by moving CloseHandle(hEvent) there, and replacing the first goto with a return -- currently the goto's actually waste space; even plain returns would be more compressed. Second, maybe the magic constant 60*1000 (known from the VSS docs) should be a macro. >>> + >>> +STDMETHODIMP CQGAVssProvider::PostCommitSnapshots( >>> + VSS_ID SnapshotSetId, LONG lSnapshotsCount) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::PreFinalCommitSnapshots(VSS_ID SnapshotSetId) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +/* >>> + * IVssProviderNotifications methods >>> + */ >>> + >>> +STDMETHODIMP CQGAVssProvider::OnLoad(IUnknown *pCallback) >>> +{ >>> + return S_OK; >>> +} >>> + >>> +STDMETHODIMP CQGAVssProvider::OnUnload(BOOL bForceUnload) >>> +{ >>> + return S_OK; >>> +} (Side question: are these methods all abstract in the base class? Can we save a few LOCs by simply inheriting some functions?) >>> + >>> + >>> +/* >>> + * CQGAVssProviderFactory class >>> + */ >>> + >>> +class CQGAVssProviderFactory : public IClassFactory >>> +{ >>> +public: >>> + STDMETHODIMP QueryInterface(REFIID riid, void **ppv); >>> + STDMETHODIMP_(ULONG) AddRef(); >>> + STDMETHODIMP_(ULONG) Release(); >>> + STDMETHODIMP CreateInstance( >>> + IUnknown *pUnknownOuter, REFIID iid, void **ppv); >>> + STDMETHODIMP LockServer(BOOL bLock) { return E_NOTIMPL; } >>> +private: >>> + long m_nRefCount; >>> +}; >>> + >>> +STDMETHODIMP CQGAVssProviderFactory::QueryInterface(REFIID riid, void **ppv) >>> +{ >>> + if (riid == IID_IUnknown || riid == IID_IClassFactory) { >>> + *ppv = static_cast<void*>(this); Again, in practice the pointer value should be correct for both interfaces (single inheritance, only one base object), but I'd feel safer if we had two static casts. Does the C++ standard (or some C++ ABI) guarantee that the first base object is always at offset 0 in the derived object? (In C, the standard requires that there be no padding at the beginning of a structure.) >>> + AddRef(); >>> + return S_OK; >>> + } >>> + *ppv = NULL; >>> + return E_NOINTERFACE; >>> +} >>> + >>> +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::AddRef() >>> +{ >>> + LockModule(TRUE); >>> + return 2; >>> +} >>> + >>> +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::Release() >>> +{ >>> + LockModule(FALSE); >>> + return 1; >>> +} Would it be preferable to change the prototype of LockModule() to return the post-increment / post-decrement value of "g_nComObjsInUse" (ie. whatever InterlockedIncrement() or InterlockedDecrement() returns in LockModule()), and just forward that retval in these two functions? The values "2" and "1" seem quite arbitrary. Also, the private data member "m_nRefCount" is unused. This class has reference counting but no constructor or destructor. >>> + >>> +STDMETHODIMP CQGAVssProviderFactory::CreateInstance( >>> + IUnknown *pUnknownOuter, REFIID iid, void **ppv) >>> +{ >>> + if (pUnknownOuter) { >>> + return CLASS_E_NOAGGREGATION; >>> + } >>> + CQGAVssProvider *pObj = new CQGAVssProvider; >>> + if (!pObj) { >>> + return E_OUTOFMEMORY; >>> + } (We generally assume that memory allocation never fails.) >>> + return pObj->QueryInterface(iid, ppv); This may leak. If CQGAVssProvider::QueryInterface() doesn't recognize the requested interface, it will set *ppv to NULL, and the caller will have no chance to call Release() on it later. That last bit is actually correct (we haven't bumped the refcount to 1), but accordingly we should delete pObj here in that case. >>> +} >>> + >>> + >>> +/* >>> + * DLL functions >>> + */ >>> + >>> +STDAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv) >>> +{ (Right, this is the factory factory function. Awesome :/) >>> + static CQGAVssProviderFactory factory; >>> + >>> + *ppv = NULL; >>> + if (IsEqualCLSID(rclsid, CLSID_QGAVSSProvider)) { >>> + return factory.QueryInterface(riid, ppv); >>> + } >>> + return CLASS_E_CLASSNOTAVAILABLE; >>> +} Not sure what to suggest here. I just don't like the factory object being static *and* having reference counting. ... Basically you translate references to the factory object to references to the module. I guess I could see the logic in that if you deleted the "m_nRefCount" member. However the externally visible AddRef() and Release() return values are broken in any case. Somehow the existence of AddRef() and Release() seems fundamentally broken for a static object -- you simply can't go to refcount==0, which would be the only situation when the DLL could be removed. What about this: - factories would be objects allocated with "new", - real reference counting for them (with constructor and destructor too), - the ctor/dtor would massage LockModule(). In this aspect CQGAVssProviderFactory would work exactly like the CQGAVssProvider class, and DllGetClassObject() -- the factory factory method -- would work like CQGAVssProviderFactory::CreateInstance() -- the factory method. Of course I have no clue how a factory object must be released officially, I assume though with the usual ->Release() call, which can be optionally followed by DLL removal. These windows interfaces are utterly over-engineered. >>> + >>> +STDAPI DllCanUnloadNow() >>> +{ >>> + return g_nComObjsInUse == 0 ? S_OK : S_FALSE; >>> +} Don't you need some kind of atomic or locked read here? We could read a stale value here. Granted, most stale values would err on the safe side (ie. read >0 instead of ==0), but in theory the other mistake is possible, no? >>> + >>> +EXTERN_C >>> +BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved) >>> +{ >>> + switch (dwReason) { >>> + >>> + case DLL_PROCESS_ATTACH: >>> + g_hinstDll = hinstDll; >>> + DisableThreadLibraryCalls(hinstDll); >>> + break; >>> + } >>> + >>> + return TRUE; >>> +} >> Seems fine I guess, though an "if" would be more idiomatic. No more comments for this patch from me. As usual I don't insist on fixing anything, I only raise points that you might want to address. Laszlo
On 6/27/13 11:01 , "Laszlo Ersek" <lersek@redhat.com> wrote: >On 06/26/13 16:32, Laszlo Ersek wrote: >> On 06/25/13 18:03, Laszlo Ersek wrote: >>> On 06/06/13 17:06, Tomoki Sekiyama wrote: >>>>+ chk(pColl->get_Count(&n)); >>>> + for (i = n - 1; i >= 0; i--) { >>>> + chk(pColl->get_Item(i, (IDispatch **)&pObj)); >>>> + chk(pObj->get_Value(_bstr_t(L"Name"), &var)); >>>> + if (var == _variant_t(QGA_PROVIDER_LNAME)) { >>>> + printf("Removing COM+ Application: %S\n", (wchar_t >>>>*)_bstr_t(var)); > >(stderr) Sure. >>>> + chk(pColl->Remove(i)); >>>> + } > >I think you leak a pObj reference here, at the end of the iteration. The >next round will set pObj to something else; I think we should call >pObj->Release() here, and set pObj to NULL (for the case when this is >the last iteration). I will add "pObj->Release(); pObj = NULL;" here. >I'm not sure if you're allowed to call pObj->Release() after the >pColl()->Remove(i) call. So maybe call pObj->Release() in an else >branch. (In this case however the out: logic should be modified as >well.) It's safe to call Release() after Remove(), by reference counter. ... >>>> +static BOOL CreateRegistryKey(LPCTSTR key, LPCTSTR value, LPCTSTR >>>>data) >>>> +{ >>>> + HKEY hKey; >>>> + LONG ret; >>>> + DWORD size; >>>> + >>>> + ret = RegCreateKeyEx(HKEY_CLASSES_ROOT, key, 0, NULL, >>>> + REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, &hKey, NULL); >>>> + if (ret != ERROR_SUCCESS) { >>>> + goto out; >>>> + } >>>> + >>>> + if (data != NULL) { >>>> + size = (lstrlen(data) + 1) * sizeof(TCHAR); > >I think we should drop the multiplication by sizeof(TCHAR), it's 1. >According to MSDN, TCHAR could be something wider than "char" (ie. >WCHAR) "for Unicode platforms". > >However in all your CreateRegistryKey() calls, the argument passed as >"data" depends on sizeof(TCHAR)==1, directly or indirectly. I think it's >best to be honest about it. For example, > >>>> + } else { >>>> + size = 0; >>>> + } >>>> + >>>> + ret = RegSetValueEx(hKey, value, 0, REG_SZ, (LPBYTE)data, size); >>>> + RegCloseKey(hKey); >>>> + >>>> +out: >>>> + if (ret != ERROR_SUCCESS) { >>>> + /* We cannot printf here, and need MessageBox to report an >>>>error. */ >>>> + errmsg_dialog(ret, "Cannot add registry ", key); > >right here we equate (const char *) with LPCTSTR (by virtue of the third >arg being "key"). > >You might also replace lstrlen() with strlen() for consistency. > >(Tangential anyhow.) Right, I assume sizeof(TCHAR) == 1. I will take the simpler way. >>>>+/* Register this dll as a VSS provider */ >>>> +STDAPI DllRegisterServer(void) >>>> +{ ... >>>> + sprintf(key, "CLSID\\%s", g_szClsid); >>>> + if (!CreateRegistryKey(key, NULL, g_szClsid)) { >>>> + goto out; >>>> + } >>>> + >>>> + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { >>>> + errmsg_dialog(GetLastError(), "GetModuleFileName failed"); >>>> + goto out; >>>> + } >>>> + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); >>>> + if (!CreateRegistryKey(key, NULL, dllPath)) { >>>> + goto out; >>>> + } >>>> + >>>> + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); > >(you could reuse "key" from the previous sprintf()) OK. ... > (indentation) I will fix them. ... >>>>+/* Support functions for _bstr_t in MinGW: Originally written by: >>>>Diaa Sami */ >>>> + > >Where does this code originate from? What is its license? This code is from MinGW User ML, and says "use this code however you like, but don't remove the comment at the beginning". But I will reimplement these by myself, >>>> +void __stdcall _com_issue_error(HRESULT hr) >>>> +{ >>>> + printf("_com_issue_error() called with parameter HRESULT = %lu", >>>>hr); >>>> +} > >This wouldn't be hard to reimplement anyway, just print the message to >stderr. Plus it's missing \n. Maybe I can use errmsg() here. >I googled the function name, and some people put up a message box here. >Not sure under what circumstances this function is called. It is used inside MinGW headers. I regard this as a user-customized error notifier function. >>>> + >>>> +namespace _com_util >>>> +{ >>>> + char * __stdcall ConvertBSTRToString(BSTR bstr) >>>> + { >>>> + const unsigned int stringLength = lstrlenW(bstr); >>>> + char *const ascii = new char [stringLength + 1]; >>>> + >>>> + wcstombs(ascii, bstr, stringLength + 1); >>>> + >>>> + return ascii; >>>> + } > >The BSTR, _bstr_t, LPCTSTR etc mess is incredible. Is BSTR just >(wchar_t*)? BSTR is usable like wchar_t*, but needs special allocator which prepend size information at the memory area pointed. >These COM interfaces seem broken by the way; how does one report a >conversion error? wcstombs() is locale-dependent and can fail. I'll just >pretend that whatever "bstr" contains in UTF-16 will always be >representable in pure ASCII. Hmm, althogh I believe only ASCII strings (which appear in this source) is passed to this function, I will add some conversion error outputs. Also, qemu-ga.exe actually use only ConvertStringToBSTR, so I will omit ConvertBSTRToString. >>>>+void LockModule(BOOL block) > >Did you mean "lock" instead of "block"? Ah, it was bLock ... But will use lock here. >>>>+STDMETHODIMP CQGAVSSEnumObject::QueryInterface(REFIID riid, void >>>>**ppObj) >>>> +{ >>>> + if (riid == IID_IUnknown || riid == IID_IVssEnumObject) { >>>> + *ppObj = >>>>static_cast<void*>(static_cast<IVssEnumObject*>(this)); > >Storing the address of the base object, right? Yes. >Actually (based on what the other class does below), do you think it's >right to return the base object's address for IID_IUnknown too, rather >than static_cast<void*>(this)? ... We have a single base object here so >in practice these addresses should be the same. Both is OK I think, if IUnknown pointer value is valid to call QueryInterface, AddRef and Release virtual methods, and is always the same value for the same object to distinguish two objects. >>>> + AddRef(); >>>> + return S_OK; >>>> + } >>>> + ppObj = NULL; > >Indirection operator missing ("*ppObj = NULL")? Oops, I will fix this. >>>>+STDMETHODIMP_(ULONG) CQGAVSSEnumObject::Release() >>>> +{ >>>> + long nRefCount = InterlockedDecrement(&m_nRefCount); >>>> + if (m_nRefCount == 0) { >>>> + delete this; > >I guess we're dead sure the object was allocated with "new". Yes, it is only created by CQGAVssProvider::Query(); >>>>+STDMETHODIMP CQGAVssProvider::QueryInterface(REFIID riid, void >>>>**ppObj) >>>> +{ >>>> + if (riid == IID_IUnknown) { >>>> + *ppObj = static_cast<void*>(this); >>>> + AddRef(); >>>> + return S_OK; >>>> + } else if (riid == IID_IVssSoftwareSnapshotProvider) { > >(No "else" needed if "return" is the last statement in the "if"'s >block.) OK. ... >>>> +STDMETHODIMP CQGAVssProvider::QueryRevertStatus( >>>> + VSS_PWSZ pwszVolume, IVssAsync **ppAsync) >>>> +{ >>>> + return S_OK; >>>> +} > >Shouldn't you set *ppAsync to something here? (No idea, just asking -- >S_OK could imply something on output.) This should be E_NOTIMPL, because we don't support revert via VSS. >Same for IsVolumeSnapshotted() above. Right, I will add return values. >>>> +STDMETHODIMP CQGAVssProvider::CommitSnapshots(VSS_ID SnapshotSetId) >>>> +{ ... >>>>+ SetEvent(hEvent); > >I think we should add a comment here -- this is where qemu-ga.exe / >libvirt will create the snapshot. OK. >>>> + if (WaitForSingleObject(hEvent2, 60*1000) != WAIT_OBJECT_0) { >>>> + hr = E_ABORT; >>>> + } >>>> + >>>> + CloseHandle(hEvent2); >>>> + CloseHandle(hEvent); >>>> +out: >>>> + return hr; >>>> +} > >Seems correct in general. > >However I think you could shave off a few lines from this function by >reorganizing the "out:" path, for example by moving CloseHandle(hEvent) >there, and replacing the first goto with a return -- currently the >goto's actually waste space; even plain returns would be more >compressed. I will replace goto with return. >Second, maybe the magic constant 60*1000 (known from the VSS docs) >should be a macro. OK. >>>> +/* >>>> + * IVssProviderNotifications methods >>>> + */ >>>> + >>>> +STDMETHODIMP CQGAVssProvider::OnLoad(IUnknown *pCallback) >>>> +{ >>>> + return S_OK; >>>> +} >>>> + >>>> +STDMETHODIMP CQGAVssProvider::OnUnload(BOOL bForceUnload) >>>> +{ >>>> + return S_OK; >>>> +} > >(Side question: are these methods all abstract in the base class? Can we >save a few LOCs by simply inheriting some functions?) These are all abstract. :/ >>>>+STDMETHODIMP CQGAVssProviderFactory::QueryInterface(REFIID riid, void >>>>**ppv) >>>> +{ >>>> + if (riid == IID_IUnknown || riid == IID_IClassFactory) { >>>> + *ppv = static_cast<void*>(this); > >Again, in practice the pointer value should be correct for both >interfaces (single inheritance, only one base object), but I'd feel >safer if we had two static casts. Does the C++ standard (or some C++ >ABI) guarantee that the first base object is always at offset 0 in the >derived object? (In C, the standard requires that there be no padding at >the beginning of a structure.) OK, I will add double static_cast. (Actually these class methods are virtual, this should OK as vtable can be resolved....) >>>> + AddRef(); >>>> + return S_OK; >>>> + } >>>> + *ppv = NULL; >>>> + return E_NOINTERFACE; >>>> +} >>>> + >>>> +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::AddRef() >>>> +{ >>>> + LockModule(TRUE); >>>> + return 2; >>>> +} >>>> + >>>> +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::Release() >>>> +{ >>>> + LockModule(FALSE); >>>> + return 1; >>>> +} > >Would it be preferable to change the prototype of LockModule() to return >the post-increment / post-decrement value of "g_nComObjsInUse" (ie. >whatever InterlockedIncrement() or InterlockedDecrement() returns in >LockModule()), and just forward that retval in these two functions? The >values "2" and "1" seem quite arbitrary. > >Also, the private data member "m_nRefCount" is unused. This class has >reference counting but no constructor or destructor. Um, I will rework the reference counting for the factory class, instead of omitting it. >>>> + >>>> +STDMETHODIMP CQGAVssProviderFactory::CreateInstance( >>>> + IUnknown *pUnknownOuter, REFIID iid, void **ppv) >>>> +{ >>>> + if (pUnknownOuter) { >>>> + return CLASS_E_NOAGGREGATION; >>>> + } >>>> + CQGAVssProvider *pObj = new CQGAVssProvider; >>>> + if (!pObj) { >>>> + return E_OUTOFMEMORY; >>>> + } > >(We generally assume that memory allocation never fails.) Ah, OK... >>>> + return pObj->QueryInterface(iid, ppv); > >This may leak. > >If CQGAVssProvider::QueryInterface() doesn't recognize the requested >interface, it will set *ppv to NULL, and the caller will have no chance >to call Release() on it later. That last bit is actually correct (we >haven't bumped the refcount to 1), but accordingly we should delete pObj >here in that case. True. I will fix this. >>>> +STDAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv) >>>> +{ > >(Right, this is the factory factory function. Awesome :/) > >>>> + static CQGAVssProviderFactory factory; >>>> + >>>> + *ppv = NULL; >>>> + if (IsEqualCLSID(rclsid, CLSID_QGAVSSProvider)) { >>>> + return factory.QueryInterface(riid, ppv); >>>> + } >>>> + return CLASS_E_CLASSNOTAVAILABLE; >>>> +} > >Not sure what to suggest here. I just don't like the factory object >being static *and* having reference counting. > >... Basically you translate references to the factory object to >references to the module. I guess I could see the logic in that if you >deleted the "m_nRefCount" member. However the externally visible >AddRef() and Release() return values are broken in any case. Somehow the >existence of AddRef() and Release() seems fundamentally broken for a >static object -- you simply can't go to refcount==0, which would be the >only situation when the DLL could be removed. > >What about this: >- factories would be objects allocated with "new", >- real reference counting for them (with constructor and destructor > too), >- the ctor/dtor would massage LockModule(). > >In this aspect CQGAVssProviderFactory would work exactly like the >CQGAVssProvider class, and DllGetClassObject() -- the factory factory >method -- would work like CQGAVssProviderFactory::CreateInstance() -- >the factory method. This sounds better than the current implementation. I will rework like above. >Of course I have no clue how a factory object must be released >officially, I assume though with the usual ->Release() call, which can >be optionally followed by DLL removal. These windows interfaces are >utterly over-engineered. Yeah, usually these stuff will be automatically generated by IDE using C++ templates, but MinGW doesn't support such mechanism. >>>> +STDAPI DllCanUnloadNow() >>>> +{ >>>> + return g_nComObjsInUse == 0 ? S_OK : S_FALSE; >>>> +} > >Don't you need some kind of atomic or locked read here? We could read a >stale value here. Granted, most stale values would err on the safe side >(ie. read >0 instead of ==0), but in theory the other mistake is >possible, no? MSDN says "Simple reads and writes to properly-aligned 32bit variables are atomic", and I couldn't find a function to atomic read provided. (Maybe InterlockedCompareExchange(&g_nComObjsInUse, 0, 0)...?) >>>> +EXTERN_C >>>> +BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID >>>>lpReserved) >>>> +{ >>>> + switch (dwReason) { >>>> + >>>> + case DLL_PROCESS_ATTACH: >>>> + g_hinstDll = hinstDll; >>>> + DisableThreadLibraryCalls(hinstDll); >>>> + break; >>>> + } >>>> + >>>> + return TRUE; >>>> +} >>> > >Seems fine I guess, though an "if" would be more idiomatic. OK, make this in to if. I really appreciate your review. Thnaks, Tomoki Sekiyama
Il 28/06/2013 00:25, Tomoki Sekiyama ha scritto: >>>>> >>>> +STDMETHODIMP CQGAVssProviderFactory::CreateInstance( >>>>> >>>> + IUnknown *pUnknownOuter, REFIID iid, void **ppv) >>>>> >>>> +{ >>>>> >>>> + if (pUnknownOuter) { >>>>> >>>> + return CLASS_E_NOAGGREGATION; >>>>> >>>> + } >>>>> >>>> + CQGAVssProvider *pObj = new CQGAVssProvider; >>>>> >>>> + if (!pObj) { >>>>> >>>> + return E_OUTOFMEMORY; >>>>> >>>> + } >> > >> >(We generally assume that memory allocation never fails.) > Ah, OK... Actually, we do because we use g_malloc/g_free. The functions exit on memory allocation failure. I'm not sure the same is true of the new operator... doesn't it throw an exception on allocation failure (that's what I vaguely remember)? Also, this is not running in the context of qemu-ga, so I think it is better to be more conservative and trap memory allocation failure. Paolo
Il 28/06/2013 12:40, Laszlo Ersek ha scritto: >> > Also, this is not running in the context of qemu-ga, so I think it is >> > better to be more conservative and trap memory allocation failure. > In that case other "new" calls must assume the nothrow form too, plus > other allocation functions should be checked as well (eg. > SysAllocStringLen(), although its only use might be in the function that > Tomoki plans to remove anyway). Agreed (nothrow or try...catch, depending on what's better for the C++ runtime; by the way, Tomoki-san, I suspect this will require libstdc++ when compiled with GCC---can you confirm?). Thanks, Paolo
On 06/28/13 09:05, Paolo Bonzini wrote: > Il 28/06/2013 00:25, Tomoki Sekiyama ha scritto: >>>>>>>>>> +STDMETHODIMP CQGAVssProviderFactory::CreateInstance( >>>>>>>>>> + IUnknown *pUnknownOuter, REFIID iid, void **ppv) >>>>>>>>>> +{ >>>>>>>>>> + if (pUnknownOuter) { >>>>>>>>>> + return CLASS_E_NOAGGREGATION; >>>>>>>>>> + } >>>>>>>>>> + CQGAVssProvider *pObj = new CQGAVssProvider; >>>>>>>>>> + if (!pObj) { >>>>>>>>>> + return E_OUTOFMEMORY; >>>>>>>>>> + } >>>> >>>> (We generally assume that memory allocation never fails.) >> Ah, OK... > > Actually, we do because we use g_malloc/g_free. The functions exit on > memory allocation failure. I'm not sure the same is true of the new > operator... doesn't it throw an exception on allocation failure (that's > what I vaguely remember)? It throws std::bad_alloc on failure. There's another new operator (the nothrow form) thar returns 0 on failure. 18.4.1.1 Single-object forms [lib.new.delete.single]; p9: [Example: T* p1 = new T; // throws bad_alloc if it fails T* p2 = new(nothrow) T; // returns 0 if it fails —end example] ( "nothrow" in the above is std::nothrow, an object with static storage duration, of type "nothrow_t" -- it's a dummy argument so that operator new() can have to prototypes. It is passed by const reference. 18.4 Dynamic memory management [lib.support.dynamic]; p1: namespace std { class bad_alloc; struct nothrow_t {}; extern const nothrow_t nothrow; /* ... */ } void* operator new(std::size_t size) throw(std::bad_alloc); void* operator new(std::size_t size, const std::nothrow_t&) throw(); ) As far as I can remember, older C++ implementations had problems with bad_alloc. I believe though that any gcc release frome after the stone age should handle this correctly; see also -fcheck-new. Of course I have no idea what happens when a C++ exception tries to propagate past a "DLL boundary". > Also, this is not running in the context of qemu-ga, so I think it is > better to be more conservative and trap memory allocation failure. In that case other "new" calls must assume the nothrow form too, plus other allocation functions should be checked as well (eg. SysAllocStringLen(), although its only use might be in the function that Tomoki plans to remove anyway). Laszlo
>>>>> +STDAPI DllCanUnloadNow() >>>>> +{ >>>>> + return g_nComObjsInUse == 0 ? S_OK : S_FALSE; >>>>> +} >> >> Don't you need some kind of atomic or locked read here? We could read a >> stale value here. Granted, most stale values would err on the safe side >> (ie. read >0 instead of ==0), but in theory the other mistake is >> possible, no? > > MSDN says "Simple reads and writes to properly-aligned 32bit variables > are atomic", and I couldn't find a function to atomic read provided. > (Maybe InterlockedCompareExchange(&g_nComObjsInUse, 0, 0)...?) Alright then. Thanks! Laszlo
On 6/28/13 6:40 , "Paolo Bonzini" <pbonzini@redhat.com> wrote: >Il 28/06/2013 12:40, Laszlo Ersek ha scritto: >>> > Also, this is not running in the context of qemu-ga, so I think it is >>> > better to be more conservative and trap memory allocation failure. >> In that case other "new" calls must assume the nothrow form too, plus >> other allocation functions should be checked as well (eg. >> SysAllocStringLen(), although its only use might be in the function that >> Tomoki plans to remove anyway). > >Agreed (nothrow or try...catch, depending on what's better for the C++ >runtime; I tried something like 'main(){ for(;;) new int[999]; }' and it caused: Terminate called after throwing an instance of 'std::bad_alloc' what(): std::bad_alloc abnormal program termination I should catch and handle this. > by the way, Tomoki-san, I suspect this will require libstdc++ >when compiled with GCC---can you confirm?). Yes, we need "libstdc++-6.dll" from MinGW to run qemu-ga.exe. >Thanks, > >Paolo Thanks, Tomoki Sekiyama
On 26/06/2013 01:31, Tomoki Sekiyama wrote: >> Also, is it needed to call VSSCheckOSVersion from the requestor? I >> would think that checking VSSAPI.DLL is stronger than checking the >> version, and indeed you do that check too. > > In Windows XP, VSSAPI.DLL exists but it has different functionality > and interfaces from newer Windows. > http://msdn.microsoft.com/en-us/library/windows/desktop/aa384627(v=vs.85).aspx > > It is checking the OS version because this patchset only supports > Windows 2003 or later. So why don't you query for the interface you supports rather than checking the OS version? Thanks, Gal. > Thanks, > Tomoki Sekiyama >
On 6/30/13 9:16 , "Gal Hammer" <ghammer@redhat.com> wrote: >On 26/06/2013 01:31, Tomoki Sekiyama wrote: > >>> Also, is it needed to call VSSCheckOSVersion from the requestor? I >>> would think that checking VSSAPI.DLL is stronger than checking the >>> version, and indeed you do that check too. >> >> In Windows XP, VSSAPI.DLL exists but it has different functionality >> and interfaces from newer Windows. >> >>http://msdn.microsoft.com/en-us/library/windows/desktop/aa384627(v=vs.85) >>.aspx >> >> It is checking the OS version because this patchset only supports >> Windows 2003 or later. > >So why don't you query for the interface you supports rather than >checking the OS version? Hmm, I don't know what I should query for. Microsoft's document above only provides a table of compatibility between VSS SDK version and OS version. Of course I can install the component and test whether it works or not, but I want to know that before installing something. >Thanks, > > Gal. Thanks, Tomoki Sekiyama
On 01/07/2013 21:57, Tomoki Sekiyama wrote: >> So why don't you query for the interface you supports rather than >> checking the OS version? > > Hmm, I don't know what I should query for. Microsoft's document above only > provides a table of compatibility between VSS SDK version and OS version. > > Of course I can install the component and test whether it works or not, > but I want to know that before installing something. I'll check it myself and try to suggest a patch. Sorry for missing the face that it is done during installation and not on a "freeze" request. Gal.
diff --git a/Makefile b/Makefile index 4851ba0..636358d 100644 --- a/Makefile +++ b/Makefile @@ -235,7 +235,7 @@ clean: rm -f qemu-options.def find . -name '*.[oda]' -type f -exec rm -f {} + find . -name '*.l[oa]' -type f -exec rm -f {} + - rm -f $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ + rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ rm -Rf .libs rm -f qemu-img-cmds.h @# May not be present in GENERATED_HEADERS diff --git a/Makefile.objs b/Makefile.objs index 286ce06..b6c1505 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -102,6 +102,7 @@ common-obj-y += disas/ # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed # by libqemuutil.a. These should be moved to a separate .json schema. qga-obj-y = qga/ qapi-types.o qapi-visit.o +qga-prv-obj-y = qga/ vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) @@ -113,6 +114,7 @@ nested-vars += \ stub-obj-y \ util-obj-y \ qga-obj-y \ + qga-prv-obj-y \ block-obj-y \ common-obj-y dummy := $(call unnest-vars) diff --git a/configure b/configure index cafe830..8e45fad 100755 --- a/configure +++ b/configure @@ -3490,9 +3490,12 @@ if test "$softmmu" = yes ; then virtfs=no fi fi - if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then + if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then if [ "$guest_agent" = "yes" ]; then tools="qemu-ga\$(EXESUF) $tools" + if [ "$mingw32" = "yes" ]; then + tools="qga/vss-win32-provider/qga-provider.dll qga/vss-win32-provider/qga-provider.tlb $tools" + fi fi fi fi diff --git a/qga/Makefile.objs b/qga/Makefile.objs index b8d7cd0..8d93866 100644 --- a/qga/Makefile.objs +++ b/qga/Makefile.objs @@ -3,3 +3,9 @@ qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o qga-obj-y += qapi-generated/qga-qapi-types.o qapi-generated/qga-qapi-visit.o qga-obj-y += qapi-generated/qga-qmp-marshal.o + +ifeq ($(CONFIG_QGA_VSS),y) +QEMU_CFLAGS += -DHAS_VSS_SDK +qga-obj-y += vss-win32-provider/ +qga-prv-obj-y += vss-win32-provider/ +endif diff --git a/qga/vss-win32-provider.h b/qga/vss-win32-provider.h new file mode 100644 index 0000000..a437e71 --- /dev/null +++ b/qga/vss-win32-provider.h @@ -0,0 +1,26 @@ +/* + * QEMU Guest Agent win32 VSS provider declarations + * + * Copyright Hitachi Data Systems Corp. 2013 + * + * Authors: + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef VSS_WIN32_PROVIDER_H +#define VSS_WIN32_PROVIDER_H + +#include <windows.h> + +STDAPI VSSCheckOSVersion(void); + +STDAPI COMRegister(void); +STDAPI COMUnregister(void); + +STDAPI DllRegisterServer(void); +STDAPI DllUnregisterServer(void); + +#endif diff --git a/qga/vss-win32-provider/Makefile.objs b/qga/vss-win32-provider/Makefile.objs new file mode 100644 index 0000000..1fe1f8f --- /dev/null +++ b/qga/vss-win32-provider/Makefile.objs @@ -0,0 +1,24 @@ +# rules to build qga-provider.dll + +qga-obj-y += qga-provider.dll +qga-prv-obj-y += provider.o install.o + +obj-qga-prv-obj-y = $(addprefix $(obj)/, $(qga-prv-obj-y)) +$(obj-qga-prv-obj-y): QEMU_CXXFLAGS = $(filter-out -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wold-style-declaration -Wold-style-definition -Wredundant-decls -fstack-protector-all, $(QEMU_CFLAGS)) -Wno-unknown-pragmas -Wno-delete-non-virtual-dtor + +$(obj)/qga-provider.dll: LDFLAGS = -shared -Wl,--add-stdcall-alias,--enable-stdcall-fixup -lole32 -loleaut32 -lshlwapi -luuid -static +$(obj)/qga-provider.dll: $(obj-qga-prv-obj-y) $(SRC_PATH)/$(obj)/qga-provider.def $(obj)/qga-provider.tlb + $(call quiet-command,$(CXX) -o $@ $(qga-prv-obj-y) $(SRC_PATH)/qga/vss-win32-provider/qga-provider.def $(CXXFLAGS) $(LDFLAGS)," LINK $(TARGET_DIR)$@") + + +# rules to build qga-provider.tlb +# Currently, only native build is supported because building .tlb +# (TypeLibrary) from .idl requires WindowsSDK and MIDL (and cl.exe in VC++). +MIDL=$(WIN_SDK)/Bin/midl + +$(obj)/qga-provider.tlb: $(SRC_PATH)/$(obj)/qga-provider.idl +ifeq ($(wildcard $(SRC_PATH)/$(obj)/qga-provider.tlb),) + $(call quiet-command,$(MIDL) -tlb $@ -I $(WIN_SDK)/Include $<," MIDL $(TARGET_DIR)$@") +else + $(call quiet-command,cp $(dir $<)qga-provider.tlb $@, " COPY $(TARGET_DIR)$@") +endif diff --git a/qga/vss-win32-provider/install.cpp b/qga/vss-win32-provider/install.cpp new file mode 100644 index 0000000..d6f1d55 --- /dev/null +++ b/qga/vss-win32-provider/install.cpp @@ -0,0 +1,493 @@ +/* + * QEMU Guest Agent win32 VSS Provider installer + * + * Copyright Hitachi Data Systems Corp. 2013 + * + * Authors: + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include <stdio.h> +#include <string.h> + +#include "../vss-win32.h" +#include "inc/win2003/vscoordint.h" +#include "../vss-win32-provider.h" + +#include <comadmin.h> +#include <wbemidl.h> +#include <comutil.h> + +extern HINSTANCE g_hinstDll; + +const GUID CLSID_COMAdminCatalog = { 0xF618C514, 0xDFB8, 0x11d1, + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; +const GUID IID_ICOMAdminCatalog = { 0xDD662187, 0xDFC2, 0x11d1, + {0xA2, 0xCF, 0x00, 0x80, 0x5F, 0xC7, 0x92, 0x35} }; +const GUID CLSID_WbemLocator = { 0x4590f811, 0x1d3a, 0x11d0, + {0x89, 0x1f, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; +const GUID IID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf, + {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} }; + +static void errmsg(DWORD err, const char *text) +{ + char *msg = NULL, *nul = strchr(text, '('); + int len = nul ? nul - text : -1; + + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (char *)&msg, 0, NULL); + printf("%.*s. (Error: %lx) %s\n", len, text, err, msg); + LocalFree(msg); +} + +static void errmsg_dialog(DWORD err, const char *text, const char *opt = "") +{ + char *msg, buf[512]; + + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (char *)&msg, 0, NULL); + snprintf(buf, sizeof(buf), "%s%s. (Error: %lx) %s\n", text, opt, err, msg); + MessageBox(NULL, buf, "Error from " QGA_PROVIDER_NAME, MB_OK|MB_ICONERROR); + LocalFree(msg); +} + +#define _chk(hr, status, msg, err_label) \ + do { \ + hr = (status); \ + if (FAILED(hr)) { \ + errmsg(hr, msg); \ + goto err_label; \ + } \ + } while (0) + +#define chk(status) _chk(hr, status, "Failed to " #status, out) + +template<class T> +HRESULT put_Value(ICatalogObject *pObj, LPCWSTR name, T val) +{ + return pObj->put_Value(_bstr_t(name), _variant_t(val)); +} + +/* Check whether this OS version supports VSS providers */ +STDAPI VSSCheckOSVersion(void) +{ + OSVERSIONINFO OSver; + + OSver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); + GetVersionEx(&OSver); + if ((OSver.dwMajorVersion == 5 && OSver.dwMinorVersion >= 2) || + OSver.dwMajorVersion > 5) { + return S_OK; + } + return S_FALSE; +} + +/* Lookup Administrators group name from winmgmt */ +static HRESULT GetAdminName(_bstr_t &name) +{ + HRESULT hr; + IWbemLocator *pLoc = NULL; + IWbemServices *pSvc = NULL; + IEnumWbemClassObject *pEnum = NULL; + IWbemClassObject *pWobj = NULL; + ULONG returned; + _variant_t var; + + chk(CoCreateInstance(CLSID_WbemLocator, NULL, CLSCTX_INPROC_SERVER, + IID_IWbemLocator, (LPVOID *)&pLoc)); + chk(pLoc->ConnectServer(_bstr_t(L"ROOT\\CIMV2"), NULL, NULL, NULL, + 0, 0, 0, &pSvc)); + chk(CoSetProxyBlanket(pSvc, RPC_C_AUTHN_WINNT, RPC_C_AUTHZ_NONE, + NULL, RPC_C_AUTHN_LEVEL_CALL, + RPC_C_IMP_LEVEL_IMPERSONATE, NULL, EOAC_NONE)); + chk(pSvc->ExecQuery(_bstr_t(L"WQL"), + _bstr_t(L"select * from Win32_Account where " + "SID='S-1-5-32-544' and localAccount=TRUE"), + WBEM_FLAG_RETURN_IMMEDIATELY | WBEM_FLAG_FORWARD_ONLY, + NULL, &pEnum)); + if (!pEnum) { + errmsg(E_FAIL, "Failed to query for Administrators"); + goto out; + } + chk(pEnum->Next(WBEM_INFINITE, 1, &pWobj, &returned)); + if (returned == 0) { + errmsg(E_FAIL, "Failed to query for Administrators"); + goto out; + } + + chk(pWobj->Get(_bstr_t(L"Name"), 0, &var, 0, 0)); + name = var; +out: + if (pLoc) { + pLoc->Release(); + } + if (pSvc) { + pSvc->Release(); + } + if (pEnum) { + pEnum->Release(); + } + if (pWobj) { + pWobj->Release(); + } + return hr; +} + +/* Register this module to COM+ Applications Catalog */ +STDAPI COMRegister(void) +{ + HRESULT hr = E_FAIL; + IUnknown *pUnknown = NULL; + ICOMAdminCatalog *pCatalog = NULL; + ICatalogCollection *pApps = NULL, *pRoles = NULL, *pUsersInRole = NULL; + ICatalogObject *pObj = NULL; + long n; + _bstr_t name; + _variant_t key; + CHAR dllPath[MAX_PATH], tlbPath[MAX_PATH]; + + if (!g_hinstDll) { + errmsg(E_FAIL, "Failed to initialize DLL"); + goto out; + } + + if (VSSCheckOSVersion() == S_FALSE) { + printf("VSS provider is not supported in this OS version.\n"); + return S_FALSE; /* VSS feature is disabled */ + } + + COMUnregister(); + + chk(CoInitialize(NULL)); + chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER, + IID_IUnknown, (void **)&pUnknown)); + chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog, (void **)&pCatalog)); + + /* Install COM+ Component */ + + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), + (IDispatch **)&pApps)); + chk(pApps->Populate()); + chk(pApps->Add((IDispatch **)&pObj)); + chk(put_Value(pObj, L"Name", QGA_PROVIDER_LNAME)); + chk(put_Value(pObj, L"Description", QGA_PROVIDER_LNAME)); + chk(put_Value(pObj, L"ApplicationAccessChecksEnabled", true)); + chk(put_Value(pObj, L"Authentication", short(6))); + chk(put_Value(pObj, L"AuthenticationCapability", short(2))); + chk(put_Value(pObj, L"ImpersonationLevel", short(2))); + chk(pApps->SaveChanges(&n)); + chk(pObj->get_Key(&key)); + + pObj->Release(); + pObj = NULL; + + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { + errmsg(GetLastError(), "GetModuleFileName failed"); + goto out; + } + n = strlen(dllPath); + if (n < 3) { + errmsg(E_FAIL, "Failed to lookup dll"); + } + strcpy(tlbPath, dllPath); + strcpy(tlbPath+n-3, "TLB"); + printf("Registering " QGA_PROVIDER_NAME ":\n"); + printf(" %s\n", dllPath); + printf(" %s\n", tlbPath); + if (!PathFileExists(tlbPath)) { + errmsg(ERROR_FILE_NOT_FOUND, "Failed to lookup tlb"); + goto out; + } + + chk(pCatalog->InstallComponent(_bstr_t(QGA_PROVIDER_LNAME), + _bstr_t(dllPath), _bstr_t(tlbPath), + _bstr_t(""))); + + /* Setup roles of the applicaion */ + + chk(pApps->GetCollection(_bstr_t(L"Roles"), key, + (IDispatch **)&pRoles)); + chk(pRoles->Populate()); + chk(pRoles->Add((IDispatch **)&pObj)); + chk(put_Value(pObj, L"Name", L"Administrators")); + chk(put_Value(pObj, L"Description", L"Administrators group")); + chk(pRoles->SaveChanges(&n)); + chk(pObj->get_Key(&key)); + + pObj->Release(); + pObj = NULL; + + /* Setup users in the role */ + + chk(pRoles->GetCollection(_bstr_t(L"UsersInRole"), key, + (IDispatch **)&pUsersInRole)); + chk(pUsersInRole->Populate()); + + chk(pUsersInRole->Add((IDispatch **)&pObj)); + chk(GetAdminName(name)); + chk(put_Value(pObj, L"User", _bstr_t(".\\") + name)); + + pObj->Release(); + pObj = NULL; + + chk(pUsersInRole->Add((IDispatch **)&pObj)); + chk(put_Value(pObj, L"User", L"SYSTEM")); + chk(pUsersInRole->SaveChanges(&n)); + +out: + if (pUnknown) { + pUnknown->Release(); + } + if (pCatalog) { + pCatalog->Release(); + } + if (pApps) { + pApps->Release(); + } + if (pRoles) { + pRoles->Release(); + } + if (pUsersInRole) { + pUsersInRole->Release(); + } + if (pObj) { + pObj->Release(); + } + + if (FAILED(hr)) { + COMUnregister(); + } + CoUninitialize(); + + return hr; +} + +/* Unregister this module from COM+ Applications Catalog */ +STDAPI COMUnregister(void) +{ + HRESULT hr; + IUnknown *pUnknown = NULL; + ICOMAdminCatalog *pCatalog = NULL; + ICatalogCollection *pColl = NULL; + ICatalogObject *pObj = NULL; + _variant_t var; + long i, n; + + if (VSSCheckOSVersion() == S_FALSE) { + printf("VSS provider is not supported in this OS version.\n"); + return S_FALSE; /* VSS feature is disabled */ + } + + chk(DllUnregisterServer()); + + chk(CoInitialize(NULL)); + chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER, + IID_IUnknown, (void **)&pUnknown)); + chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog, (void **)&pCatalog)); + + chk(pCatalog->GetCollection(_bstr_t(L"Applications"), + (IDispatch **)&pColl)); + chk(pColl->Populate()); + + chk(pColl->get_Count(&n)); + for (i = n - 1; i >= 0; i--) { + chk(pColl->get_Item(i, (IDispatch **)&pObj)); + chk(pObj->get_Value(_bstr_t(L"Name"), &var)); + if (var == _variant_t(QGA_PROVIDER_LNAME)) { + printf("Removing COM+ Application: %S\n", (wchar_t *)_bstr_t(var)); + chk(pColl->Remove(i)); + } + } + chk(pColl->SaveChanges(&n)); + +out: + if (pUnknown) { + pUnknown->Release(); + } + if (pCatalog) { + pCatalog->Release(); + } + if (pColl) { + pColl->Release(); + } + if (pObj) { + pObj->Release(); + } + CoUninitialize(); + + return hr; +} + + +static BOOL CreateRegistryKey(LPCTSTR key, LPCTSTR value, LPCTSTR data) +{ + HKEY hKey; + LONG ret; + DWORD size; + + ret = RegCreateKeyEx(HKEY_CLASSES_ROOT, key, 0, NULL, + REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, &hKey, NULL); + if (ret != ERROR_SUCCESS) { + goto out; + } + + if (data != NULL) { + size = (lstrlen(data) + 1) * sizeof(TCHAR); + } else { + size = 0; + } + + ret = RegSetValueEx(hKey, value, 0, REG_SZ, (LPBYTE)data, size); + RegCloseKey(hKey); + +out: + if (ret != ERROR_SUCCESS) { + /* We cannot printf here, and need MessageBox to report an error. */ + errmsg_dialog(ret, "Cannot add registry ", key); + return FALSE; + } + return TRUE; +} + +/* Register this dll as a VSS provider */ +STDAPI DllRegisterServer(void) +{ + IVssAdmin *pVssAdmin = NULL; + HRESULT hr = E_FAIL; + char dllPath[MAX_PATH]; + char key[256]; + + CoInitialize(NULL); + + if (!g_hinstDll) { + errmsg_dialog(hr, "Module instance is not available"); + goto out; + } + + /* Add this module to registery */ + + sprintf(key, "CLSID\\%s", g_szClsid); + if (!CreateRegistryKey(key, NULL, g_szClsid)) { + goto out; + } + + if (!GetModuleFileName(g_hinstDll, dllPath, sizeof(dllPath))) { + errmsg_dialog(GetLastError(), "GetModuleFileName failed"); + goto out; + } + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); + if (!CreateRegistryKey(key, NULL, dllPath)) { + goto out; + } + + sprintf(key, "CLSID\\%s\\InprocServer32", g_szClsid); + if (!CreateRegistryKey(key, "ThreadingModel", "Apartment")) { + goto out; + } + + sprintf(key, "CLSID\\%s\\ProgID", g_szClsid); + if (!CreateRegistryKey(key, NULL, g_szProgid)) { + goto out; + } + + if (!CreateRegistryKey(g_szProgid, NULL, QGA_PROVIDER_NAME)) { + goto out; + } + + sprintf(key, "%s\\CLSID", g_szProgid); + if (!CreateRegistryKey(key, NULL, g_szClsid)) { + goto out; + } + + hr = CoCreateInstance(CLSID_VSSCoordinator, + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); + if (FAILED(hr)) { + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); + goto out; + } + + hr = pVssAdmin->RegisterProvider( + g_gProviderId, CLSID_QGAVSSProvider, + const_cast<WCHAR*>(QGA_PROVIDER_LNAME), VSS_PROV_SOFTWARE, + const_cast<WCHAR*>(QGA_PROVIDER_VERSION), g_gProviderVersion); + if (FAILED(hr)) { + errmsg_dialog(hr, "RegisterProvider failed"); + goto out; + } + +out: + if (pVssAdmin) { + pVssAdmin->Release(); + } + CoUninitialize(); + + if (FAILED(hr)) { + DllUnregisterServer(); + } + + return hr; +} + +/* Unregister this VSS hardware provider from the system */ +STDAPI DllUnregisterServer(void) +{ + TCHAR key[256]; + IVssAdmin *pVssAdmin = NULL; + + CoInitialize(NULL); + + HRESULT hr = CoCreateInstance(CLSID_VSSCoordinator, + NULL, CLSCTX_ALL, IID_IVssAdmin, (void **)&pVssAdmin); + if (SUCCEEDED(hr)) { + hr = pVssAdmin->UnregisterProvider(g_gProviderId); + pVssAdmin->Release(); + } else { + errmsg_dialog(hr, "CoCreateInstance(VSSCoordinator) failed"); + } + + sprintf(key, "CLSID\\%s", g_szClsid); + SHDeleteKey(HKEY_CLASSES_ROOT, key); + SHDeleteKey(HKEY_CLASSES_ROOT, g_szProgid); + + CoUninitialize(); + + return S_OK; /* Uninstall should never fail */ +} + + +/* Support functions for _bstr_t in MinGW: Originally written by: Diaa Sami */ + +void __stdcall _com_issue_error(HRESULT hr) +{ + printf("_com_issue_error() called with parameter HRESULT = %lu", hr); +} + +namespace _com_util +{ + char * __stdcall ConvertBSTRToString(BSTR bstr) + { + const unsigned int stringLength = lstrlenW(bstr); + char *const ascii = new char [stringLength + 1]; + + wcstombs(ascii, bstr, stringLength + 1); + + return ascii; + } + + BSTR __stdcall ConvertStringToBSTR(const char *const ascii) + { + const unsigned int stringLength = lstrlenA(ascii); + BSTR bstr = SysAllocStringLen(NULL, stringLength); + + mbstowcs(bstr, ascii, stringLength + 1); + + return bstr; + } +} diff --git a/qga/vss-win32-provider/provider.cpp b/qga/vss-win32-provider/provider.cpp new file mode 100644 index 0000000..90a3d25 --- /dev/null +++ b/qga/vss-win32-provider/provider.cpp @@ -0,0 +1,479 @@ +/* + * QEMU Guest Agent win32 VSS Provider implementations + * + * Copyright Hitachi Data Systems Corp. 2013 + * + * Authors: + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include <stdio.h> +#include "../vss-win32.h" +#include "inc/win2003/vscoordint.h" +#include "inc/win2003/vsprov.h" + +static long g_nComObjsInUse; +HINSTANCE g_hinstDll; + +/* VSS common GUID's */ + +const CLSID CLSID_VSSCoordinator = { 0xE579AB5F, 0x1CC4, 0x44b4, + {0xBE, 0xD9, 0xDE, 0x09, 0x91, 0xFF, 0x06, 0x23} }; +const IID IID_IVssAdmin = { 0x77ED5996, 0x2F63, 0x11d3, + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; + +const IID IID_IVssHardwareSnapshotProvider = { 0x9593A157, 0x44E9, 0x4344, + {0xBB, 0xEB, 0x44, 0xFB, 0xF9, 0xB0, 0x6B, 0x10} }; +const IID IID_IVssSoftwareSnapshotProvider = { 0x609e123e, 0x2c5a, 0x44d3, + {0x8f, 0x01, 0x0b, 0x1d, 0x9a, 0x47, 0xd1, 0xff} }; +const IID IID_IVssProviderCreateSnapshotSet = { 0x5F894E5B, 0x1E39, 0x4778, + {0x8E, 0x23, 0x9A, 0xBA, 0xD9, 0xF0, 0xE0, 0x8C} }; +const IID IID_IVssProviderNotifications = { 0xE561901F, 0x03A5, 0x4afe, + {0x86, 0xD0, 0x72, 0xBA, 0xEE, 0xCE, 0x70, 0x04} }; + +const IID IID_IVssEnumObject = { 0xAE1C7110, 0x2F60, 0x11d3, + {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} }; + + +void LockModule(BOOL block) +{ + if (block) { + InterlockedIncrement(&g_nComObjsInUse); + } else { + InterlockedDecrement(&g_nComObjsInUse); + } +} + +/* Empty enumerator for VssObject */ + +class CQGAVSSEnumObject : public IVssEnumObject +{ +public: + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); + STDMETHODIMP_(ULONG) AddRef(); + STDMETHODIMP_(ULONG) Release(); + + /* IVssEnumObject Methods */ + STDMETHODIMP Next( + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched); + STDMETHODIMP Skip(ULONG celt); + STDMETHODIMP Reset(void); + STDMETHODIMP Clone(IVssEnumObject **ppenum); + + /* CQGAVSSEnumObject Methods */ + CQGAVSSEnumObject(); + ~CQGAVSSEnumObject(); + +private: + long m_nRefCount; +}; + +CQGAVSSEnumObject::CQGAVSSEnumObject() +{ + m_nRefCount = 0; + LockModule(TRUE); +} + +CQGAVSSEnumObject::~CQGAVSSEnumObject() +{ + LockModule(FALSE); +} + +STDMETHODIMP CQGAVSSEnumObject::QueryInterface(REFIID riid, void **ppObj) +{ + if (riid == IID_IUnknown || riid == IID_IVssEnumObject) { + *ppObj = static_cast<void*>(static_cast<IVssEnumObject*>(this)); + AddRef(); + return S_OK; + } + ppObj = NULL; + return E_NOINTERFACE; +} + +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::AddRef() +{ + return InterlockedIncrement(&m_nRefCount); +} + +STDMETHODIMP_(ULONG) CQGAVSSEnumObject::Release() +{ + long nRefCount = InterlockedDecrement(&m_nRefCount); + if (m_nRefCount == 0) { + delete this; + } + return nRefCount; +} + +STDMETHODIMP CQGAVSSEnumObject::Next( + ULONG celt, VSS_OBJECT_PROP *rgelt, ULONG *pceltFetched) +{ + *pceltFetched = 0; + return S_FALSE; +} + +STDMETHODIMP CQGAVSSEnumObject::Skip(ULONG celt) +{ + return S_FALSE; +} + +STDMETHODIMP CQGAVSSEnumObject::Reset(void) +{ + return S_OK; +} + +STDMETHODIMP CQGAVSSEnumObject::Clone(IVssEnumObject **ppenum) +{ + return E_NOTIMPL; +} + + +/* QGAVssProvider */ + +class CQGAVssProvider : + public IVssSoftwareSnapshotProvider, + public IVssProviderCreateSnapshotSet, + public IVssProviderNotifications +{ +public: + STDMETHODIMP QueryInterface(REFIID riid, void **ppObj); + STDMETHODIMP_(ULONG) AddRef(); + STDMETHODIMP_(ULONG) Release(); + + /* IVssSoftwareSnapshotProvider Methods */ + STDMETHODIMP SetContext(LONG lContext); + STDMETHODIMP GetSnapshotProperties( + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp); + STDMETHODIMP Query( + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum); + STDMETHODIMP DeleteSnapshots( + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, + BOOL bForceDelete, LONG *plDeletedSnapshots, + VSS_ID *pNondeletedSnapshotID); + STDMETHODIMP BeginPrepareSnapshot( + VSS_ID SnapshotSetId, VSS_ID SnapshotId, + VSS_PWSZ pwszVolumeName, LONG lNewContext); + STDMETHODIMP IsVolumeSupported( + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider); + STDMETHODIMP IsVolumeSnapshotted( + VSS_PWSZ pwszVolumeName, BOOL *pbSnapshotsPresent, + LONG *plSnapshotCompatibility); + STDMETHODIMP SetSnapshotProperty( + VSS_ID SnapshotId, VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, + VARIANT vProperty); + STDMETHODIMP RevertToSnapshot(VSS_ID SnapshotId); + STDMETHODIMP QueryRevertStatus(VSS_PWSZ pwszVolume, IVssAsync **ppAsync); + + /* IVssProviderCreateSnapshotSet Methods */ + STDMETHODIMP EndPrepareSnapshots(VSS_ID SnapshotSetId); + STDMETHODIMP PreCommitSnapshots(VSS_ID SnapshotSetId); + STDMETHODIMP CommitSnapshots(VSS_ID SnapshotSetId); + STDMETHODIMP PostCommitSnapshots( + VSS_ID SnapshotSetId, LONG lSnapshotsCount); + STDMETHODIMP PreFinalCommitSnapshots(VSS_ID SnapshotSetId); + STDMETHODIMP PostFinalCommitSnapshots(VSS_ID SnapshotSetId); + STDMETHODIMP AbortSnapshots(VSS_ID SnapshotSetId); + + /* IVssProviderNotifications Methods */ + STDMETHODIMP OnLoad(IUnknown *pCallback); + STDMETHODIMP OnUnload(BOOL bForceUnload); + + /* CQGAVssProvider Methods */ + CQGAVssProvider(); + ~CQGAVssProvider(); + +private: + long m_nRefCount; +}; + +CQGAVssProvider::CQGAVssProvider() +{ + m_nRefCount = 0; + LockModule(TRUE); +} + +CQGAVssProvider::~CQGAVssProvider() +{ + LockModule(FALSE); +} + +STDMETHODIMP CQGAVssProvider::QueryInterface(REFIID riid, void **ppObj) +{ + if (riid == IID_IUnknown) { + *ppObj = static_cast<void*>(this); + AddRef(); + return S_OK; + } else if (riid == IID_IVssSoftwareSnapshotProvider) { + *ppObj = static_cast<void*>( + static_cast<IVssSoftwareSnapshotProvider*>(this)); + AddRef(); + return S_OK; + } else if (riid == IID_IVssProviderCreateSnapshotSet) { + *ppObj = static_cast<void*>( + static_cast<IVssProviderCreateSnapshotSet*>(this)); + AddRef(); + return S_OK; + } else if (riid == IID_IVssProviderNotifications) { + *ppObj = static_cast<void*>( + static_cast<IVssProviderNotifications*>(this)); + AddRef(); + return S_OK; + } + *ppObj = NULL; + return E_NOINTERFACE; +} + +STDMETHODIMP_(ULONG) CQGAVssProvider::AddRef() +{ + return InterlockedIncrement(&m_nRefCount); +} + +STDMETHODIMP_(ULONG) CQGAVssProvider::Release() +{ + long nRefCount = InterlockedDecrement(&m_nRefCount); + if (m_nRefCount == 0) { + delete this; + } + return nRefCount; +} + + +/* + * IVssSoftwareSnapshotProvider methods + */ + +STDMETHODIMP CQGAVssProvider::SetContext(LONG lContext) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::GetSnapshotProperties( + VSS_ID SnapshotId, VSS_SNAPSHOT_PROP *pProp) +{ + return VSS_E_OBJECT_NOT_FOUND; +} + +STDMETHODIMP CQGAVssProvider::Query( + VSS_ID QueriedObjectId, VSS_OBJECT_TYPE eQueriedObjectType, + VSS_OBJECT_TYPE eReturnedObjectsType, IVssEnumObject **ppEnum) +{ + *ppEnum = new CQGAVSSEnumObject; + (*ppEnum)->AddRef(); + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::DeleteSnapshots( + VSS_ID SourceObjectId, VSS_OBJECT_TYPE eSourceObjectType, + BOOL bForceDelete, LONG *plDeletedSnapshots, VSS_ID *pNondeletedSnapshotID) +{ + return E_NOTIMPL; +} + +STDMETHODIMP CQGAVssProvider::BeginPrepareSnapshot( + VSS_ID SnapshotSetId, VSS_ID SnapshotId, + VSS_PWSZ pwszVolumeName, LONG lNewContext) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::IsVolumeSupported( + VSS_PWSZ pwszVolumeName, BOOL *pbSupportedByThisProvider) +{ + *pbSupportedByThisProvider = TRUE; + + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::IsVolumeSnapshotted(VSS_PWSZ pwszVolumeName, + BOOL *pbSnapshotsPresent, LONG *plSnapshotCompatibility) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::SetSnapshotProperty(VSS_ID SnapshotId, + VSS_SNAPSHOT_PROPERTY_ID eSnapshotPropertyId, VARIANT vProperty) +{ + return E_NOTIMPL; +} + +STDMETHODIMP CQGAVssProvider::RevertToSnapshot(VSS_ID SnapshotId) +{ + return E_NOTIMPL; +} + +STDMETHODIMP CQGAVssProvider::QueryRevertStatus( + VSS_PWSZ pwszVolume, IVssAsync **ppAsync) +{ + return S_OK; +} + + +/* + * IVssProviderCreateSnapshotSet methods + */ + +STDMETHODIMP CQGAVssProvider::EndPrepareSnapshots(VSS_ID SnapshotSetId) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::PreCommitSnapshots(VSS_ID SnapshotSetId) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::CommitSnapshots(VSS_ID SnapshotSetId) +{ + HRESULT hr = S_OK; + HANDLE hEvent, hEvent2; + + hEvent = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_FROZEN); + if (hEvent == INVALID_HANDLE_VALUE) { + hr = E_FAIL; + goto out; + } + + hEvent2 = OpenEvent(EVENT_ALL_ACCESS, FALSE, EVENT_NAME_THAW); + if (hEvent == INVALID_HANDLE_VALUE) { + CloseHandle(hEvent); + hr = E_FAIL; + goto out; + } + + SetEvent(hEvent); + if (WaitForSingleObject(hEvent2, 60*1000) != WAIT_OBJECT_0) { + hr = E_ABORT; + } + + CloseHandle(hEvent2); + CloseHandle(hEvent); +out: + return hr; +} + +STDMETHODIMP CQGAVssProvider::PostCommitSnapshots( + VSS_ID SnapshotSetId, LONG lSnapshotsCount) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::PreFinalCommitSnapshots(VSS_ID SnapshotSetId) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::PostFinalCommitSnapshots(VSS_ID SnapshotSetId) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::AbortSnapshots(VSS_ID SnapshotSetId) +{ + return S_OK; +} + +/* + * IVssProviderNotifications methods + */ + +STDMETHODIMP CQGAVssProvider::OnLoad(IUnknown *pCallback) +{ + return S_OK; +} + +STDMETHODIMP CQGAVssProvider::OnUnload(BOOL bForceUnload) +{ + return S_OK; +} + + +/* + * CQGAVssProviderFactory class + */ + +class CQGAVssProviderFactory : public IClassFactory +{ +public: + STDMETHODIMP QueryInterface(REFIID riid, void **ppv); + STDMETHODIMP_(ULONG) AddRef(); + STDMETHODIMP_(ULONG) Release(); + STDMETHODIMP CreateInstance( + IUnknown *pUnknownOuter, REFIID iid, void **ppv); + STDMETHODIMP LockServer(BOOL bLock) { return E_NOTIMPL; } +private: + long m_nRefCount; +}; + +STDMETHODIMP CQGAVssProviderFactory::QueryInterface(REFIID riid, void **ppv) +{ + if (riid == IID_IUnknown || riid == IID_IClassFactory) { + *ppv = static_cast<void*>(this); + AddRef(); + return S_OK; + } + *ppv = NULL; + return E_NOINTERFACE; +} + +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::AddRef() +{ + LockModule(TRUE); + return 2; +} + +STDMETHODIMP_(ULONG) CQGAVssProviderFactory::Release() +{ + LockModule(FALSE); + return 1; +} + +STDMETHODIMP CQGAVssProviderFactory::CreateInstance( + IUnknown *pUnknownOuter, REFIID iid, void **ppv) +{ + if (pUnknownOuter) { + return CLASS_E_NOAGGREGATION; + } + CQGAVssProvider *pObj = new CQGAVssProvider; + if (!pObj) { + return E_OUTOFMEMORY; + } + return pObj->QueryInterface(iid, ppv); +} + + +/* + * DLL functions + */ + +STDAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv) +{ + static CQGAVssProviderFactory factory; + + *ppv = NULL; + if (IsEqualCLSID(rclsid, CLSID_QGAVSSProvider)) { + return factory.QueryInterface(riid, ppv); + } + return CLASS_E_CLASSNOTAVAILABLE; +} + +STDAPI DllCanUnloadNow() +{ + return g_nComObjsInUse == 0 ? S_OK : S_FALSE; +} + +EXTERN_C +BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved) +{ + switch (dwReason) { + + case DLL_PROCESS_ATTACH: + g_hinstDll = hinstDll; + DisableThreadLibraryCalls(hinstDll); + break; + } + + return TRUE; +} diff --git a/qga/vss-win32-provider/qga-provider.def b/qga/vss-win32-provider/qga-provider.def new file mode 100644 index 0000000..9f3afc8 --- /dev/null +++ b/qga/vss-win32-provider/qga-provider.def @@ -0,0 +1,10 @@ +LIBRARY "QGA-PROVIDER.DLL" + +EXPORTS + VSSCheckOSVersion PRIVATE + COMRegister PRIVATE + COMUnregister PRIVATE + DllCanUnloadNow PRIVATE + DllGetClassObject PRIVATE + DllRegisterServer PRIVATE + DllUnregisterServer PRIVATE diff --git a/qga/vss-win32-provider/qga-provider.idl b/qga/vss-win32-provider/qga-provider.idl new file mode 100644 index 0000000..17abca0 --- /dev/null +++ b/qga/vss-win32-provider/qga-provider.idl @@ -0,0 +1,20 @@ +import "oaidl.idl"; +import "ocidl.idl"; + +[ + uuid(103B8142-6CE5-48A7-BDE1-794D3192FCF1), + version(1.0), + helpstring("QGAVSSProvider Type Library") +] +library QGAVSSHWProviderLib +{ + importlib("stdole2.tlb"); + [ + uuid(6E6A3492-8D4D-440C-9619-5E5D0CC31CA8), + helpstring("QGAVSSProvider Class") + ] + coclass QGAVSSHWProvider + { + [default] interface IUnknown; + }; +}; diff --git a/qga/vss-win32-provider/qga-provider.tlb b/qga/vss-win32-provider/qga-provider.tlb new file mode 100644 index 0000000000000000000000000000000000000000..226452a1861371ffe0cad1019cf90fdfdcd5ef49 GIT binary patch literal 1528 zcmeYbb_-!*U}OLRP8Kl5;0UB3A_y8H!@$4<WGF*9|A9aP$W{R21|SCUVfs9Pj1;IC zKahR`)X0Ox{{ZC6An~sN`2tA%H9-9hNPHcj{0byK4>OPh6a(1_GM|T)fx!kz-UG<D zK;nbc0l9GX===tt`Vb`fD?q*q5+7YXI$u?Zf#C;G4-9~uhYKVCC4f!`hZ{(Z0*HVD zkhwswGqAt}ki;hd*$F@lQbP%-z+r|5P#hGW*vvKniaRx03p~wP?y>h_rLW<nKOg@= z6{hYgzgH7@QE<^MU>KC!yoBjb#vz`9Lwu4+R-SJ!kIOX4xLBUUGN9-NyTyP76j}@n z2f!qQ8-xepfXD+7rW+{SKz4&@7#qX~^1#sn3O|tFK>%ciE<<riN`6kNkzPqoQh0bc zNbM*XJ|Un0jALSb15^rEE6h+Y8tCpA798vm9#E8DmYI@T<dc~c4pSpwQKEn@FU<fE zfvHyrsVqoU0O~4AEUE;iEfI8i=bXgi;_z?|20Ng!&PAz-C8;S2NtFt|o-RHLWvNBQ znfZAN=6VJOdIqMZrV5EA3T{Q23NES13Py$shQ?OLW>&_Q3PuKoMqI)S5zj9Ngog_= gXfrXehlhjmFbIJB4$8MKU>*YlD1Z9^F{m5{03Vre%>V!Z literal 0 HcmV?d00001 diff --git a/qga/vss-win32.h b/qga/vss-win32.h new file mode 100644 index 0000000..21655cf --- /dev/null +++ b/qga/vss-win32.h @@ -0,0 +1,86 @@ +/* + * QEMU Guest Agent win32 VSS common declarations + * + * Copyright Hitachi Data Systems Corp. 2013 + * + * Authors: + * Tomoki Sekiyama <tomoki.sekiyama@hds.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef VSS_WIN32_H +#define VSS_WIN32_H + +#define __MIDL_user_allocate_free_DEFINED__ +#include "config-host.h" +#include <windows.h> +#include <shlwapi.h> + +/* Reduce warnings to include vss.h */ + +/* Ignore annotations for MS IDE */ +#define __in IN +#define __out OUT +#define __RPC_unique_pointer +#define __RPC_string +#define __RPC__deref_inout_opt +#define __RPC__out +#ifndef __RPC__out_ecount_part +#define __RPC__out_ecount_part(x, y) +#endif +#define _declspec(x) +#undef uuid +#define uuid(x) + +/* Undef some duplicated error codes redefined in vss.h */ +#undef VSS_E_BAD_STATE +#undef VSS_E_PROVIDER_NOT_REGISTERED +#undef VSS_E_PROVIDER_VETO +#undef VSS_E_OBJECT_NOT_FOUND +#undef VSS_E_VOLUME_NOT_SUPPORTED +#undef VSS_E_VOLUME_NOT_SUPPORTED_BY_PROVIDER +#undef VSS_E_OBJECT_ALREADY_EXISTS +#undef VSS_E_UNEXPECTED_PROVIDER_ERROR +#undef VSS_E_INVALID_XML_DOCUMENT +#undef VSS_E_MAXIMUM_NUMBER_OF_VOLUMES_REACHED +#undef VSS_E_MAXIMUM_NUMBER_OF_SNAPSHOTS_REACHED + +/* + * VSS headers must be installed from Microsoft VSS SDK 7.2 available at: + * http://www.microsoft.com/en-us/download/details.aspx?id=23490 + */ +#include "inc/win2003/vss.h" + +/* Macros to convert char definitions to wchar */ +#define _L(a) L##a +#define L(a) _L(a) + +/* Constants for QGA VSS Provider */ + +#define QGA_PROVIDER_NAME "QEMU Guest Agent VSS Provider" +#define QGA_PROVIDER_LNAME L(QGA_PROVIDER_NAME) +#define QGA_PROVIDER_VERSION L(QEMU_VERSION) + +#define EVENT_NAME_FROZEN "Global\\QGAVSSEvent-frozen" +#define EVENT_NAME_THAW "Global\\QGAVSSEvent-thaw" + +const GUID g_gProviderId = { 0x3629d4ed, 0xee09, 0x4e0e, + {0x9a, 0x5c, 0x6d, 0x8b, 0xa2, 0x87, 0x2a, 0xef} }; +const GUID g_gProviderVersion = { 0x11ef8b15, 0xcac6, 0x40d6, + {0x8d, 0x5c, 0x8f, 0xfc, 0x16, 0x3f, 0x24, 0xca} }; + +const CLSID CLSID_QGAVSSProvider = { 0x6e6a3492, 0x8d4d, 0x440c, + {0x96, 0x19, 0x5e, 0x5d, 0x0c, 0xc3, 0x1c, 0xa8} }; + +const TCHAR g_szClsid[] = TEXT("{6E6A3492-8D4D-440C-9619-5E5D0CC31CA8}"); +const TCHAR g_szProgid[] = TEXT("QGAVSSProvider"); + +/* Enums undefined in VSS SDK 7.2 but defined in newer Windows SDK */ +enum __VSS_VOLUME_SNAPSHOT_ATTRIBUTES { + VSS_VOLSNAP_ATTR_NO_AUTORECOVERY = 0x00000002, + VSS_VOLSNAP_ATTR_TXF_RECOVERY = 0x02000000 +}; + +#endif
Implements a basic stub of software VSS provider. Currently, this module only provides a relay function of events between qemu-guest-agent and Windows VSS when VSS finished filesystem freeze and when qemu snapshot is done. In the future, this module could be extended to support the other VSS functions, such as query for snapshot volumes and recovery. To build type library (.tlb) for qga-provider.dll from COM IDL (.idl), VisualC++, MIDL and stdole2.tlb in Windows SDK are required. This patch also adds pre-compiled .tlb file in the repository in order to enable cross-compile qemu-ga.exe for Windows with VSS support. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> --- Makefile | 2 Makefile.objs | 2 configure | 5 qga/Makefile.objs | 6 qga/vss-win32-provider.h | 26 ++ qga/vss-win32-provider/Makefile.objs | 24 ++ qga/vss-win32-provider/install.cpp | 493 +++++++++++++++++++++++++++++++ qga/vss-win32-provider/provider.cpp | 479 ++++++++++++++++++++++++++++++ qga/vss-win32-provider/qga-provider.def | 10 + qga/vss-win32-provider/qga-provider.idl | 20 + qga/vss-win32-provider/qga-provider.tlb | Bin qga/vss-win32.h | 86 +++++ 12 files changed, 1151 insertions(+), 2 deletions(-) create mode 100644 qga/vss-win32-provider.h create mode 100644 qga/vss-win32-provider/Makefile.objs create mode 100644 qga/vss-win32-provider/install.cpp create mode 100644 qga/vss-win32-provider/provider.cpp create mode 100644 qga/vss-win32-provider/qga-provider.def create mode 100644 qga/vss-win32-provider/qga-provider.idl create mode 100644 qga/vss-win32-provider/qga-provider.tlb create mode 100644 qga/vss-win32.h