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

Re: [libvirt] [PATCHv2 08/13] list: add virDomainListAllSnapshots API



On 06/18/2012 06:22 AM, Peter Krempa wrote:
> On 06/15/12 06:18, Eric Blake wrote:
>> There was an inherent race between virDomainSnapshotNum() and
>> virDomainSnapshotListNames(), where an additional snapshot could
>> be created in the meantime, or where a snapshot could be deleted
>> before converting the name back to a virDomainSnapshotPtr.  It
>> was also an awkward name: the function operates on domains, not
>> domain snapshots.  virDomainSnapshotListChildrenNames() suffered
>> from the same inherent race, although its naming was nicer.
>>

>> + *
>> + * Returns the number of domain snapshots found or -1 in case of error.
> 
> As the new API guarantees to NULL the list on failure I'd state it here
> as I did in the domain list series:
> 
> Returns the number of domain snapshots found or -1 and sets snaps to
> NULL in case of error.
> 

> 
> ACK with the docs fixed.

Sure, here's what I squashed in before pushing 6-8:

diff --git i/src/libvirt.c w/src/libvirt.c
index 79ed324..0aa50cb 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -17123,9 +17123,8 @@ error:
  * @flags: bitwise-OR of supported virDomainSnapshotListFlags
  *
  * Collect the list of domain snapshots for the given domain, and store
- * their names in @names.  Caller is responsible for freeing each member
- * of the array.  The value to use for @nameslen can be determined by
- * virDomainSnapshotNum() with the same @flags.
+ * their names in @names.  The value to use for @nameslen can be determined
+ * by virDomainSnapshotNum() with the same @flags.
  *
  * By default, this command covers all snapshots; it is also possible to
  * limit things to just snapshots with no parents, when @flags includes
@@ -17149,14 +17148,17 @@ error:
  * whether they have metadata that would prevent the removal of the last
  * reference to a domain.
  *
- * Returns the number of domain snapshots found or -1 in case of error.
  * Note that this command is inherently racy: another connection can
  * define a new snapshot between a call to virDomainSnapshotNum() and
  * this call.  You are only guaranteed that all currently defined
  * snapshots were listed if the return is less than @nameslen.  Likewise,
  * you should be prepared for virDomainSnapshotLookupByName() to fail when
  * converting a name from this call into a snapshot object, if another
- * connection deletes the snapshot in the meantime.
+ * connection deletes the snapshot in the meantime.  For more control over
+ * the results, see virDomainListAllSnapshots().
+ *
+ * Returns the number of domain snapshots found or -1 in case of error.
+ * The caller is responsible for freeing each member of the array.
  */
 int
 virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen,
@@ -17203,9 +17205,8 @@ error:
  * @flags: bitwise-OR of supported virDomainSnapshotListFlags
  *
  * Collect the list of domain snapshots for the given domain, and allocate
- * an array to store those objects.  Caller is responsible for calling
- * virDomainSnapshotFree() on each member of the array, then free() on the
- * array itself.
+ * an array to store those objects.  This API solves the race inherent in
+ * virDomainSnapshotListNames().
  *
  * By default, this command covers all snapshots; it is also possible to
  * limit things to just snapshots with no parents, when @flags includes
@@ -17229,9 +17230,12 @@ error:
  * whether they have metadata that would prevent the removal of the last
  * reference to a domain.
  *
- * Returns the number of domain snapshots found or -1 in case of error.
- * On success, the array stored into @snaps is guaranteed to have an
- * extra allocated element set to NULL, to make iteration easier.
+ * Returns the number of domain snapshots found or -1 and sets @snaps to
+ * NULL in case of error.  On success, the array stored into @snaps is
+ * guaranteed to have an extra allocated element set to NULL but not
included
+ * in the return count, to make iteration easier.  The caller is
responsible
+ * for calling virDomainSnapshotFree() on each array element, then calling
+ * free() on @snaps.
  */
 int
 virDomainListAllSnapshots(virDomainPtr domain, virDomainSnapshotPtr
**snaps,
@@ -17336,9 +17340,9 @@ error:
  * @flags: bitwise-OR of supported virDomainSnapshotListFlags
  *
  * Collect the list of domain snapshots that are children of the given
- * snapshot, and store their names in @names.  Caller is responsible for
- * freeing each member of the array.  The value to use for @nameslen can
- * be determined by virDomainSnapshotNumChildren() with the same @flags.
+ * snapshot, and store their names in @names.  The value to use for
+ * @nameslen can be determined by virDomainSnapshotNumChildren() with
+ * the same @flags.
  *
  * By default, this command covers only direct children; it is also
possible
  * to expand things to cover all descendants, when @flags includes
@@ -17369,7 +17373,11 @@ error:
  * snapshots were listed if the return is less than @nameslen.  Likewise,
  * you should be prepared for virDomainSnapshotLookupByName() to fail when
  * converting a name from this call into a snapshot object, if another
- * connection deletes the snapshot in the meantime.
+ * connection deletes the snapshot in the meantime.  For more control over
+ * the results, see virDomainSnapshotListAllChildren().
+ *
+ * Returns the number of domain snapshots found or -1 in case of error.
+ * The caller is responsible for freeing each member of the array.
  */
 int
 virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
@@ -17420,9 +17428,8 @@ error:
  * @flags: bitwise-OR of supported virDomainSnapshotListFlags
  *
  * Collect the list of domain snapshots that are children of the given
- * snapshot, and allocate an array to store those objects.  Caller is
- * responsible for calling virDomainSnapshotFree() on each member of the
- * array, then free() on the array itself.
+ * snapshot, and allocate an array to store those objects.  This API solves
+ * the race inherent in virDomainSnapshotListChildrenNames().
  *
  * By default, this command covers only direct children; it is also
possible
  * to expand things to cover all descendants, when @flags includes
@@ -17446,9 +17453,12 @@ error:
  * whether they have metadata that would prevent the removal of the last
  * reference to a domain.
  *
- * Returns the number of domain snapshots found or -1 in case of error.
- * On success, the array stored into @snaps is guaranteed to have an
- * extra allocated element set to NULL, to make iteration easier.
+ * Returns the number of domain snapshots found or -1 and sets @snaps to
+ * NULL in case of error.  On success, the array stored into @snaps is
+ * guaranteed to have an extra allocated element set to NULL but not
included
+ * in the return count, to make iteration easier.  The caller is
responsible
+ * for calling virDomainSnapshotFree() on each array element, then calling
+ * free() on @snaps.
  */
 int
 virDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot,

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



Attachment: signature.asc
Description: OpenPGP digital signature


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