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

On Thu, May 19, 2011 at 04:51:25PM -0400, Laine Stump wrote: > From: Michal Privoznik <mprivozn(a)redhat.com&gt; > > --- > 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. [...] > @@ -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.

> +/** > + * 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.