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

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

 



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@xxxxxxxxxx>

---
  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.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]