On Wed, 25 Oct 2017 16:58:07 -0500
Michael Roth <mdr...@linux.vnet.ibm.com> wrote:
> Quoting Tomáš Golembiovský (2017-09-29 16:31:02)
> > On Fri, 29 Sep 2017 17:11:22 +0800
> > Chen Hanxiao <chen_han_x...@126.com> wrote:
> >
> > > From: Chen Hanxiao <chenhanx...@gmail.com>
> > >
> > > On some of windows (win08 sp2),
> > > it doesn't work by calling LookupAccountSidW with
> > > well-known SIDs,
> > > We got an error:
> > > error 997 overlapped I/O operation is in progress
> > >
> > > But hardcoded names work.
> > >
> > > This patch introduces a workaroud for this issue:
> > > if LookupAccountSidW failed, try hardcoded one.
> > >
> > > Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
> > > ---
> > > qga/vss-win32/install.cpp | 10 ++++++++--
> > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> >
> > I wonder if it's caused by qga itself or a race/conflict with some other
> > app. But still...
> >
> >
> > Reviewed-by: Tomáš Golembiovský <tgole...@redhat.com>
>
> I think it might be getNameByStringSID()/LookupAccountSidW() reporting a
> stale GetLastError() value.
>
I suppose you're right! Taking a closer look there's one more issue with
getNameByStringSID(). The call ConvertStringSidToSidW() does not return
HRESULT so using chk() makes no sense.
I propose slightly modified version of your fix:
diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index ba7c94eb25..65f68f94a3 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -148,10 +148,15 @@ static HRESULT getNameByStringSID(
DWORD domainNameLen = BUFFER_SIZE;
wchar_t domainName[BUFFER_SIZE];
- chk(ConvertStringSidToSidW(sid, &psid));
- LookupAccountSidW(NULL, psid, buffer, bufferLen,
- domainName, &domainNameLen, &groupType);
- hr = HRESULT_FROM_WIN32(GetLastError());
+ if (!ConvertStringSidToSidW(sid, &psid) {
+ hr = HRESULT_FROM_WIN32(GetLastError());
+ goto out;
+ }
+ if (!LookupAccountSidW(NULL, psid, buffer, bufferLen,
+ domainName, &domainNameLen, &groupType)) {
+ hr = HRESULT_FROM_WIN32(GetLastError());
+ /* Fall through and free psid */
+ }
LocalFree(psid);
--
Tomáš Golembiovský <tgole...@redhat.com>
