| Message ID | 150896868721.17721.8644098584872122342@sif (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At 2017-10-26 05:58:07, "Michael Roth" <mdroth@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_xiao@126.com> wrote: >> >> > From: Chen Hanxiao <chenhanxiao@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 <chenhanxiao@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ý <tgolembi@redhat.com> > >I think it might be getNameByStringSID()/LookupAccountSidW() reporting a >stale GetLastError() value. > >Does this fix the issue? Not exactly. I tested your patch several times, it improved greatly. But failed only one time, got another error 1722. Build with my fallback patch and your suggestion, installation work perfectly. Regards, - Chen > >diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp >index ba7c94eb25..94c921e8de 100644 >--- a/qga/vss-win32/install.cpp >+++ b/qga/vss-win32/install.cpp >@@ -149,9 +149,10 @@ static HRESULT getNameByStringSID( > wchar_t domainName[BUFFER_SIZE]; > > chk(ConvertStringSidToSidW(sid, &psid)); >- LookupAccountSidW(NULL, psid, buffer, bufferLen, >- domainName, &domainNameLen, &groupType); >- hr = HRESULT_FROM_WIN32(GetLastError()); >+ if (!LookupAccountSidW(NULL, psid, buffer, bufferLen, >+ domainName, &domainNameLen, &groupType)) { >+ hr = HRESULT_FROM_WIN32(GetLastError()); >+ } > > LocalFree(psid); > >> >> >> -- >> Tomáš Golembiovský <tgolembi@redhat.com> >> >
Quoting Chen Hanxiao (2017-10-26 04:27:40) > > > At 2017-10-26 05:58:07, "Michael Roth" <mdroth@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_xiao@126.com> wrote: > >> > >> > From: Chen Hanxiao <chenhanxiao@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 <chenhanxiao@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ý <tgolembi@redhat.com> > > > >I think it might be getNameByStringSID()/LookupAccountSidW() reporting a > >stale GetLastError() value. > > > >Does this fix the issue? > > Not exactly. > I tested your patch several times, it improved greatly. > But failed only one time, > got another error 1722. Hmm, was that error also from the getNameByStringSID() call? > > Build with my fallback patch and your suggestion, > installation work perfectly. Thanks for testing. > > Regards, > - Chen > > > > >diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp > >index ba7c94eb25..94c921e8de 100644 > >--- a/qga/vss-win32/install.cpp > >+++ b/qga/vss-win32/install.cpp > >@@ -149,9 +149,10 @@ static HRESULT getNameByStringSID( > > wchar_t domainName[BUFFER_SIZE]; > > > > chk(ConvertStringSidToSidW(sid, &psid)); > >- LookupAccountSidW(NULL, psid, buffer, bufferLen, > >- domainName, &domainNameLen, &groupType); > >- hr = HRESULT_FROM_WIN32(GetLastError()); > >+ if (!LookupAccountSidW(NULL, psid, buffer, bufferLen, > >+ domainName, &domainNameLen, &groupType)) { > >+ hr = HRESULT_FROM_WIN32(GetLastError()); > >+ } > > > > LocalFree(psid); > > > >> > >> > >> -- > >> Tomáš Golembiovský <tgolembi@redhat.com> > >> > >
At 2017-10-26 17:59:34, "Michael Roth" <mdroth@linux.vnet.ibm.com> wrote: >Quoting Chen Hanxiao (2017-10-26 04:27:40) >> >> >> At 2017-10-26 05:58:07, "Michael Roth" <mdroth@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_xiao@126.com> wrote: >> >> >> >> > From: Chen Hanxiao <chenhanxiao@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 <chenhanxiao@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ý <tgolembi@redhat.com> >> > >> >I think it might be getNameByStringSID()/LookupAccountSidW() reporting a >> >stale GetLastError() value. >> > >> >Does this fix the issue? >> >> Not exactly. >> I tested your patch several times, it improved greatly. >> But failed only one time, >> got another error 1722. > >Hmm, was that error also from the getNameByStringSID() call? I don't know how to trace qemu-ga-win, but added some fprintf(stdout). + hr = getNameByStringSID(administratorsGroupSID, buffer, &bufferLen); + if (FAILED(hr)) { I saw my logs inside if (FAILED(hr)) { But it looks weird, as your patch already dealing with this senario. Regards, - Chen > >> >> Build with my fallback patch and your suggestion, >> installation work perfectly. > >Thanks for testing. > >>
diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp index ba7c94eb25..94c921e8de 100644 --- a/qga/vss-win32/install.cpp +++ b/qga/vss-win32/install.cpp @@ -149,9 +149,10 @@ static HRESULT getNameByStringSID( wchar_t domainName[BUFFER_SIZE]; chk(ConvertStringSidToSidW(sid, &psid)); - LookupAccountSidW(NULL, psid, buffer, bufferLen, - domainName, &domainNameLen, &groupType); - hr = HRESULT_FROM_WIN32(GetLastError()); + if (!LookupAccountSidW(NULL, psid, buffer, bufferLen, + domainName, &domainNameLen, &groupType)) { + hr = HRESULT_FROM_WIN32(GetLastError()); + } LocalFree(psid);