Re: [libvirt] RFE: virConnectListAllDomains()

On 05/19/2012 01:52 AM, Eric Blake wrote: >Use of virConnectListDomains() and virConnectListDefinedDomains() is: > >1. inherently racy. A domain can change between active and inactive >between two back-to-back calls, and thus be entirely skipped or >enumerated twice when concatenating lists. > >2. painful to use. ListDomains gives ids, ListDefinedDomains gives >names, and the user must then call virDomainLookupByID() and >virDomainLookupByName() to convert into UUIDs. > >3. requires pre-allocation. The user must call virConnectNumOfDomains() >then over-allocate before calling virConnectListDomains(), in order to >guarantee that the list size didn't change between the two calls. > >This is a proposal for a new API that addresses all three points - by >returning virDomainPtr rather than id or strings, the UUID of each >domain can be grabbed in one shot. By consolidating things into a >single API call, there is no race in trying to piece together the >complete list. By having libvirt allocate the resulting array, rather >than making the caller pre-allocate, the user doesn't have to worry >about a race between getting a count and using that count. It also >provides the convenience of returning smaller lists based on various >filtering groups. > >Thoughts before I expand this API and add the actual implementation? I definitely agree with all of your points. I had very similar ideas how to implement such a functionality that actualy resulted in a very similar functionality that I've got almost ready and implemented (I was struggling with python bindings for this as I didn't notice that some of the code for looking up domains was automaticaly generated). Therefore I'd be glad if you would comment on my implementation and save work yourself :) > >diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in >index a817db8..ea63d9f 100644 >--- i/include/libvirt/libvirt.h.in >+++ w/include/libvirt/libvirt.h.in >@@ -1192,6 +1192,38 @@ int virConnectListDomains >(virConnectPtr conn, > */ > int virConnectNumOfDomains (virConnectPtr conn); > >+/** >+ * virConnectListAllDomainsFlags: >+ * >+ * Flags used to tune which domains are listed by >virConnectListAllDomains(). >+ * Note that these flags come in groups; if all bits from a group are 0, >+ * then that group is not used to filter results. >+ */ >+typedef enum { >+ VIR_CONNECT_LIST_DOMAINS_ACTIVE = 1<< 0, >+ VIR_CONNECT_LIST_DOMAINS_INACTIVE = 1<< 1, >+ >+ VIR_CONNECT_LIST_DOMAINS_PERSISTENT = 1<< 2, >+ VIR_CONNECT_LIST_DOMAINS_TRANSIENT = 1<< 3, >+ >+ VIR_CONNECT_LIST_DOMAINS_RUNNING = 1<< 4, >+ VIR_CONNECT_LIST_DOMAINS_PAUSED = 1<< 5, >+ VIR_CONNECT_LIST_DOMAINS_SHUTOFF = 1<< 6, >+ VIR_CONNECT_LIST_DOMAINS_OTHER = 1<< 7, >+ >+ VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE = 1<< 8, >+ VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE = 1<< 9, >+ >+ VIR_CONNECT_LIST_DOMAINS_AUTOSTART = 1<< 10, >+ VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART = 1<< 11, >+ >+ VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT = 1<< 12, >+ VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT = 1<< 13, >+} virConnectListAllDomainsFlags; This collection of filters is impressive, I'll add them as a followup or in v2 as v1 is almost ready. >+ >+int virConnectListAllDomains(virConnectPtr conn, >+ virDomainPtr *doms, >+ unsigned int flags); My take of implementing this was basicaly identical (even the function name). The only thing i took differently was adding a parameter "ndomains" to enable limiting of size of the returned array. I don't think that this will be widely used, but it might be useful to get some of the guests if the complete list exceeds RPC limit. Do you think I should keep it?