qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] qga-win: fall back to hardcoded user and group


From: Chen Hanxiao
Subject: Re: [Qemu-devel] [PATCH] qga-win: fall back to hardcoded user and group names if LookupAccountSidW failed
Date: Thu, 2 Nov 2017 18:58:49 +0800 (CST)

At 2017-10-27 09:05:25, "Michael Roth" <address@hidden> wrote:
>Quoting Tomáš Golembiovský (2017-10-26 08:54:37)
>> On Wed, 25 Oct 2017 16:58:07 -0500
>> Michael Roth <address@hidden> wrote:
>> 
>> > Quoting Tomáš Golembiovský (2017-09-29 16:31:02)
>> > > On Fri, 29 Sep 2017 17:11:22 +0800
>> > > Chen Hanxiao <address@hidden> wrote:
>> > > 
>> > > Reviewed-by: Tomáš Golembiovský <address@hidden>  
>> > 
>> > 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.

Regards,
- Chen



>
>>  
>> 
>> -- 
>> Tomáš Golembiovský <address@hidden>
>> 
>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]