[libvirt] [PATCH 2/3] parallels: move up updating autostart parameter in prlsdkLoadDomain

[Maxim Nestratov]

21.05.2015 9:55, Dmitry Guryanov пишет:
>
>
> On 05/18/2015 05:44 PM, Maxim Nestratov wrote:
>> It is better to get all necessary parameters and check them on newly
>> created configuration before actually creating a domain with them or
>> applying them to an existing domain.
>
> What problem could it cause?
First of all if we ask something from parallels dispatcher it takes time 
and doing such things with locked domain isn't a good practice. I am not 
saying that PrlVmCfg_GetAutoStart will go to dispatcher but other SDK 
functions will. What I wanted here is to group SDK calls together.
Secondly, there can be a race when we call prlsdkLoadDomain function 
from domain creation event handler after the domain is deleted by 
concurrent extern call. So it's better to fail before we actually create 
a domain.
>>
>> Signed-off-by: Maxim Nestratov <mnestratov at parallels.com>
>> ---
>>   src/parallels/parallels_sdk.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/parallels/parallels_sdk.c 
>> b/src/parallels/parallels_sdk.c
>> index faae1ae..5c15e94 100644
>> --- a/src/parallels/parallels_sdk.c
>> +++ b/src/parallels/parallels_sdk.c
>> @@ -1312,6 +1312,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
>>               *s = '\0';
>>       }
>>   +    pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart);
>> +    prlsdkCheckRetGoto(pret, error);
>> +
>>       if (virDomainDefAddImplicitControllers(def) < 0)
>>           goto error;
>>   @@ -1349,15 +1352,6 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
>>       dom->privateDataFreeFunc = prlsdkDomObjFreePrivate;
>>       dom->persistent = 1;
>>   -    if (prlsdkGetDomainState(sdkdom, &domainState) < 0)
>> -        goto error;
>> -
>> -    if (prlsdkConvertDomainState(domainState, envId, dom) < 0)
>> -        goto error;
>> -
>> -    pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart);
>> -    prlsdkCheckRetGoto(pret, error);
>> -
>>       switch (autostart) {
>>       case PAO_VM_START_ON_LOAD:
>>           dom->autostart = 1;
>> @@ -1371,6 +1365,12 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
>>           goto error;
>>       }
>>   +    if (prlsdkGetDomainState(sdkdom, &domainState) < 0)
>> +        goto error;
>> +
>
> Why don't you move this ^^ call?

Makes sence even more than PrlVmCfg_GetAutoStart. Just missed it.
I'll add it in the second version of the patchset.
>
>> +    if (prlsdkConvertDomainState(domainState, envId, dom) < 0)
>> +        goto error;
>> +
>>       if (!pdom->sdkdom) {
>>           pret = PrlHandle_AddRef(sdkdom);
>>           prlsdkCheckRetGoto(pret, error);
>