[libvirt] [PATCH] qemu: Fix a regression of attaching device

[Osier Yang]

于 2011年07月13日 23:03, Eric Blake 写道:
> On 07/13/2011 09:17 AM, Osier Yang wrote:
>> qemuDomainModifyDeviceFlags: Changing "ret" to "0", but don't
>> reset it to "-1" on failure, it's not good idea to change the value
>> of "ret" in the codes to do some condition checking. This patch
>> fix it.
> Can you identify which commit introduced the regression, and what
> behavior actually broke as a result?  The commit message will be more
> useful with that information.
>
>> ---
>>   src/qemu/qemu_driver.c |    6 +++---
>>   1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 0a6b48e..ae11c68 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4892,10 +4892,10 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>>                               _("unknown domain modify action %d"), action);
>>               break;
>>           }
>> -    } else
>> -        ret = 0;
>> +    }
>>
>> -    if (!ret&&  (flags&  VIR_DOMAIN_AFFECT_LIVE)) {
>> +    if (!(flags&  VIR_DOMAIN_AFFECT_CONFIG)&&
>> +        (flags&  VIR_DOMAIN_AFFECT_LIVE)) {
> I'm not sure I agree with this change.  The logic flow before this patch is:
>
> if config
>    make the change in a copy, or record that the change failed
> if live, and either no config or change to config copy is happy
>    make the change live, or record that change failed
> if config, and either no live or change to live is happy
>    commit the changed copy made earlier
>
> Your proposed change makes it so that we can now change live even if we
> are going to fail to change the config, which is not a good idea.

Yes, I didn't follow with the complete meaning of commit da1eba6b
pointed by Wen, a updated patch is following.

> Why not instead do:
>
> if config {
>    make change in a copy
>    if ret<  0
>      goto error
> }
> if live {
>    make change in live
>    if ret<  0
>      goto error
> }
> if config
>    commit the changed copy made earlier
>