On Thu, 2 Nov 2017 18:58:49 +0800 (CST)
"Chen Hanxiao" <chen_han_x...@126.com> wrote:
> At 2017-10-27 09:05:25, "Michael Roth" <mdr...@linux.vnet.ibm.com> wrote:
> >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:
> >> > >
> >> > > 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.
>
> Sorry for the late reply.
>
> I tested the upstream qga-win on that windows VM for several times.
> It works fine by my limit tests.
>
I'm glad to hear that.
Tomas
> Regards,
> - Chen
>
>
>
> >
> >>
> >>
> >> --
> >> Tomáš Golembiovský <tgole...@redhat.com>
> >>
> >
--
Tomáš Golembiovský <tgole...@redhat.com>
