Quoting Tomáš Golembiovský (2017-10-26 08:54:37)
> 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);
Thanks! It's not yet clear if this fixes the issue completely (though it
seems likely), but it's clearly a bug either way so I've gone ahead and
applied it to the qga tree:
https://github.com/mdroth/qemu/commit/8cedc80555e5a395a4fda7130b0115015e775c48
Chen, if there's still an issue with the updated patch please let me know.
>
>
> --
> Tomáš Golembiovský <tgole...@redhat.com>
>
