[libvirt] [PATCHv2 3/7] interface: implement public APIs for libvirt transactional network changes

[Laine Stump]

On 05/24/2011 04:44 AM, Daniel P. Berrange wrote:
> On Tue, May 24, 2011 at 02:13:12PM +0800, Daniel Veillard wrote:
>> On Thu, May 19, 2011 at 04:51:25PM -0400, Laine Stump wrote:
>>> From: Michal Privoznik<mprivozn at redhat.com>
>>>
>>> ---
>>>   src/libvirt.c |  141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>   1 files changed, 139 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/libvirt.c b/src/libvirt.c
>>> index ff16c48..786ce15 100644
>>> --- a/src/libvirt.c
>>> +++ b/src/libvirt.c
>>> @@ -8143,7 +8143,9 @@ error:
>>>    * @xml: the XML description for the interface, preferably in UTF-8
>>>    * @flags: and OR'ed set of extraction flags, not used yet
>>>    *
>>> - * Define an interface (or modify existing interface configuration)
>>> + * Define an interface (or modify existing interface configuration).
>>> + * This can, however be affected by virInterfaceChangeBegin
>>> + * and/or friends.
>>    Small nit, I would suggest making the 4 extra comments a bit more
>>    explicit that it's about transaction support, something along the
>>    lines of:
>>
>>      * This can however be affected by the transaction support for
>>      * interface configuration change, see virInterfaceChangeBegin(),
>>      * virInterfaceChangeCommit() and related functions.

I made the comments more verbose (possibly *too* verbose) in the 
upcoming V3 of this patch. If the explanation is too much, you can let 
me know and I'll dial it back before pushing.

>> [...]
>>> @@ -8374,6 +8382,135 @@ virInterfaceFree(virInterfacePtr iface)
>>>       return 0;
>>>   }
>>>
>>> +/**
>>> + * virInterfaceChangeBegin:
>>> + * @conn: pointer to hypervisor connection
>>> + * @flags: flags, not used yet
>>> + *
>>> + * This functions creates a restore point to which one can return
>>> + * later by calling virInterfaceChangeRollback. This function should
>>> + * be called before any transaction with interface configuration.
>>> + * Once knowing, new configuration works, it can be commited via
>>> + * virInterfaceChangeCommit, which frees restore point.
>>     which frees the restore point.
>>
>> I would like the description of what happens ona  sequence of
>>     virInterfaceChangeBegin()
>>     virInterfaceChangeBegin()
>>
>> calls for the same connection, is that an error ? Will netcf implement
>> this, this behaviour should probably be documented.
> It should return the VIR_ERR_INVALID_OPERATION code with
> a message that a transaction is already active.

Yes, netcf checks for this and returns an error (likewise if commit or 
rollback is called when there is no open transaction). I will make sure 
this is translated to VIR_ERR_INVALID_OPERATION so that it is logged 
properly.

(it's good that you pointed this out now, because netcf had been just 
returning a generic error for this, so I modified the netcf patches to 
return a new unique code, and have updated patch 6/7 (the netcf driver 
additions) to recognize this new code when it's available).

>>> +/**
>>> + * virInterfaceChangeCommit:
>>> + * @conn: pointer to hypervisor connection
>>> + * @flags: flags, not used yet
>>> + *
>>> + * This commits the changes made to interfaces and frees restore point
>>    "frees the restore point"
>>
>>> + * created by virInterfaceChangeBegin.
>>    Same thing behaviour of virInterfaceChangeCommit() in case there was
>>    no Begin() associated should be documented.
> Likewise VIR_ERR_INVALID_OPERATION

Yup.