This introduces a new API to list the storage pool objects, 4 groups of flags are provided to filter the returned pools: * Active or not * Autostarting or not * Persistent or not * And the pool type. include/libvirt/libvirt.h.in: New enum virConnectListAllStoragePoolFlags; Declare the API. python/generator.py: Skip the generating src/driver.h: (virDrvConnectListAllStoragePools) src/libvirt.c: Implementation for the API. src/libvirt_public.syms: Export the symbol. --- include/libvirt/libvirt.h.in | 33 ++++++++++++ python/generator.py | 5 +- src/driver.h | 5 ++ src/libvirt.c | 112 +++++++++++++++++++++++++++++++++++++++--- src/libvirt_public.syms | 5 ++ 5 files changed, 150 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index cfe5047..0c52271 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2554,6 +2554,39 @@ int virConnectListDefinedStoragePools(virConnectPtr conn, int maxnames); /* + * virConnectListAllStoragePoolsFlags: + * + * Flags used to tune pools returned by virConnectListAllStoragePools(). + * 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_STORAGE_POOLS_INACTIVE = 1 << 0, + VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE = 1 << 1, + + VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT = 1 << 2, + VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT = 1 << 3, + + VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART = 1 << 4, + VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART = 1 << 5, + + /* List pools by type */ + VIR_CONNECT_LIST_STORAGE_POOLS_DIR = 1 << 6, + VIR_CONNECT_LIST_STORAGE_POOLS_FS = 1 << 7, + VIR_CONNECT_LIST_STORAGE_POOLS_NETFS = 1 << 8, + VIR_CONNECT_LIST_STORAGE_POOLS_LOGICAL = 1 << 9, + VIR_CONNECT_LIST_STORAGE_POOLS_DISK = 1 << 10, + VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI = 1 << 11, + VIR_CONNECT_LIST_STORAGE_POOLS_SCSI = 1 << 12, + VIR_CONNECT_LIST_STORAGE_POOLS_MPATH = 1 << 13, + VIR_CONNECT_LIST_STORAGE_POOLS_RBD = 1 << 14, + VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG = 1 << 15, +} virConnectListAllStoragePoolsFlags; + +int virConnectListAllStoragePools(virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags); +/* * Query a host for storage pools of a particular type */ char * virConnectFindStoragePoolSources(virConnectPtr conn, diff --git a/python/generator.py b/python/generator.py index 7beb361..261efe0 100755 --- a/python/generator.py +++ b/python/generator.py @@ -337,7 +337,7 @@ foreign_encoding_args = ( # ####################################################################### -# Class methods which are written by hand in libvir.c but the Python-level +# Class methods which are written by hand in libvirt.c but the Python-level # code is still automatically generated (so they are not in skip_function()). skip_impl = ( 'virConnectGetVersion', @@ -457,9 +457,10 @@ skip_function = ( 'virConnectDomainEventDeregisterAny', # overridden in virConnect.py 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError - 'virConnectListAllDomains', #overridden in virConnect.py + 'virConnectListAllDomains', # overridden in virConnect.py 'virDomainListAllSnapshots', # overridden in virDomain.py 'virDomainSnapshotListAllChildren', # overridden in virDomainSnapshot.py + 'virConnectListAllStoragePools', # overridden in virConnect.py 'virStreamRecvAll', # Pure python libvirt-override-virStream.py 'virStreamSendAll', # Pure python libvirt-override-virStream.py diff --git a/src/driver.h b/src/driver.h index e88ab28..342f6b5 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1252,6 +1252,10 @@ typedef int (*virDrvConnectListDefinedStoragePools) (virConnectPtr conn, char **const names, int maxnames); +typedef int + (*virDrvConnectListAllStoragePools) (virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags); typedef char * (*virDrvConnectFindStoragePoolSources) (virConnectPtr conn, const char *type, @@ -1396,6 +1400,7 @@ struct _virStorageDriver { virDrvConnectListStoragePools listPools; virDrvConnectNumOfDefinedStoragePools numOfDefinedPools; virDrvConnectListDefinedStoragePools listDefinedPools; + virDrvConnectListAllStoragePools listAllPools; virDrvConnectFindStoragePoolSources findPoolSources; virDrvStoragePoolLookupByName poolLookupByName; virDrvStoragePoolLookupByUUID poolLookupByUUID; diff --git a/src/libvirt.c b/src/libvirt.c index b034ed6..4e4f96b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -11416,6 +11416,90 @@ virStoragePoolGetConnect (virStoragePoolPtr pool) } /** + * virConnectListAllStoragePools: + * @conn: Pointer to the hypervisor connection. + * @pools: Pointer to a variable to store the array containing storage pool + * objects or NULL if the list is not required (just returns number + * of pools). + * @flags: bitwise-OR of virConnectListAllStoragePoolsFlags. + * + * Collect the list of storage pools, and allocate an array to store those + * objects. This API solves the race inherent between + * virConnectListStoragePools and virConnectListDefinedStoragePools. + * + * Normally, all storage pools are returned; however, @flags can be used to + * filter the results for a smaller list of targeted pools. The valid + * flags are divided into groups, where each group contains bits that + * describe mutually exclusive attributes of a pool, and where all bits + * within a group describe all possible pools. + * + * The first group of @flags is VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE (online) + * and VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE (offline) to filter the pools + * by state. + * + * The second group of @flags is VIR_CONNECT_LIST_STORAGE_POOLS_PERSITENT + * (defined) and VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT (running but not + * defined), to filter the pools by whether they have persistent config or not. + * + * The third group of @flags is VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART + * and VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART, to filter the pools by + * whether they are marked as autostart or not. + * + * The last group of @flags is provided to filter the pools by the types, + * the flags include: + * VIR_CONNECT_LIST_STORAGE_POOLS_DIR + * VIR_CONNECT_LIST_STORAGE_POOLS_FS + * VIR_CONNECT_LIST_STORAGE_POOLS_NETFS + * VIR_CONNECT_LIST_STORAGE_POOLS_LOGICAL + * VIR_CONNECT_LIST_STORAGE_POOLS_DISK + * VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI + * VIR_CONNECT_LIST_STORAGE_POOLS_SCSI + * VIR_CONNECT_LIST_STORAGE_POOLS_MPATH + * VIR_CONNECT_LIST_STORAGE_POOLS_RBD + * VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG + * + * Returns the number of storage pools found or -1 and sets @pools to + * NULL in case of error. On success, the array stored into @pools 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 virStoragePoolFree() on each array element, then calling + * free() on @pools. + */ +int +virConnectListAllStoragePools(virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, pools=%p, flags=%x", conn, pools, flags); + + virResetLastError(); + + if (pools) + *pools = NULL; + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (conn->storageDriver && + conn->storageDriver->listAllPools) { + int ret; + ret = conn->storageDriver->listAllPools(conn, pools, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** * virConnectNumOfStoragePools: * @conn: pointer to hypervisor connection * @@ -11457,11 +11541,17 @@ error: * @names: array of char * to fill with pool names (allocated by caller) * @maxnames: size of the names array * - * Provides the list of names of active storage pools - * upto maxnames. If there are more than maxnames, the - * remaining names will be silently ignored. + * Provides the list of names of active storage pools up to maxnames. + * If there are more than maxnames, the remaining names will be silently + * ignored. * - * Returns 0 on success, -1 on error + * For more control over the results, see virConnectListAllStoragePools(). + * + * Returns the number of pools found or -1 in case of error. Note that + * this command is inherently racy; a pool can be started between a call to + * virConnectNumOfStoragePools() and this call; you are only guaranteed + * that all currently active pools were listed if the return is less than + * @maxnames. */ int virConnectListStoragePools(virConnectPtr conn, @@ -11540,11 +11630,17 @@ error: * @names: array of char * to fill with pool names (allocated by caller) * @maxnames: size of the names array * - * Provides the list of names of inactive storage pools - * upto maxnames. If there are more than maxnames, the - * remaining names will be silently ignored. + * Provides the list of names of inactive storage pools up to maxnames. + * If there are more than maxnames, the remaining names will be silently + * ignored. * - * Returns 0 on success, -1 on error + * For more control over the results, see virConnectListAllStoragePools(). + * + * Returns the number of names provided in the array or -1 in case of error. + * Note that this command is inherently racy; a pool can be defined between + * a call to virConnectNumOfDefinedStoragePools() and this call; you are only + * guaranteed that all currently defined pools were listed if the return + * is less than @maxnames. The client must call free() on each returned name. */ int virConnectListDefinedStoragePools(virConnectPtr conn, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 92ae95a..f6068b4 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -554,4 +554,9 @@ LIBVIRT_0.10.0 { virDomainGetEmulatorPinInfo; } LIBVIRT_0.9.13; +LIBVIRT_0.10.2 { + global: + virConnectListAllStoragePools; +} LIBVIRT_0.10.0; + # .... define new API here using predicted next version number .... -- 1.7.7.3

src/conf/storage_conf.c: Add virStoragePoolMatch to filter the pools; Add virStoragePoolList to iterate over the pool objects with filter. src/conf/storage_conf.h: Declare virStoragePoolMatch, virStoragePoolList, and the macros for filters. src/libvirt_private.syms: Export helper virStoragePoolList. --- src/conf/storage_conf.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/storage_conf.h | 35 ++++++++++++++ src/libvirt_private.syms | 1 + 3 files changed, 152 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 3132aae..b14564f 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1883,3 +1883,119 @@ void virStoragePoolObjUnlock(virStoragePoolObjPtr obj) { virMutexUnlock(&obj->lock); } + +#define MATCH(FLAG) (flags & (FLAG)) +static bool +virStoragePoolMatch(virStoragePoolObjPtr poolobj, + unsigned int flags) +{ + /* filter by active state */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) && + !((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE) && + virStoragePoolObjIsActive(poolobj)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE) && + !virStoragePoolObjIsActive(poolobj)))) + return false; + + /* filter by persistence */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_PERSISTENT) && + !((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT) && + poolobj->configFile) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT) && + !poolobj->configFile))) + return false; + + /* filter by autostart option */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART) && + !((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART) && + poolobj->autostart) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART) && + !poolobj->autostart))) + return false; + + /* filter by pool type */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE)) { + if (!((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_DIR) && + (poolobj->def->type == VIR_STORAGE_POOL_DIR)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FS) && + (poolobj->def->type == VIR_STORAGE_POOL_FS)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_NETFS) && + (poolobj->def->type == VIR_STORAGE_POOL_NETFS)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_LOGICAL) && + (poolobj->def->type == VIR_STORAGE_POOL_LOGICAL)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_DISK) && + (poolobj->def->type == VIR_STORAGE_POOL_DISK)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI) && + (poolobj->def->type == VIR_STORAGE_POOL_ISCSI)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_SCSI) && + (poolobj->def->type == VIR_STORAGE_POOL_SCSI)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_MPATH) && + (poolobj->def->type == VIR_STORAGE_POOL_MPATH)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_RBD) && + (poolobj->def->type == VIR_STORAGE_POOL_RBD)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG) && + (poolobj->def->type == VIR_STORAGE_POOL_SHEEPDOG)))) + return false; + } + + return true; +} +#undef MATCH + +int +virStoragePoolList(virConnectPtr conn, + virStoragePoolObjList poolobjs, + virStoragePoolPtr **pools, + unsigned int flags) +{ + virStoragePoolPtr *tmp_pools = NULL; + virStoragePoolPtr pool = NULL; + int npools = 0; + int ret = -1; + int i; + + if (pools) { + if (VIR_ALLOC_N(tmp_pools, poolobjs.count + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + } + + for (i = 0; i < poolobjs.count; i++) { + virStoragePoolObjPtr poolobj = poolobjs.objs[i]; + virStoragePoolObjLock(poolobj); + if (virStoragePoolMatch(poolobj, flags)) { + if (pools) { + if (!(pool = virGetStoragePool(conn, + poolobj->def->name, + poolobj->def->uuid))) { + virStoragePoolObjUnlock(poolobj); + goto cleanup; + } + tmp_pools[npools] = pool; + } + npools++; + } + virStoragePoolObjUnlock(poolobj); + } + + if (tmp_pools) { + /* trim the array to the final size */ + ignore_value(VIR_REALLOC_N(tmp_pools, npools + 1)); + *pools = tmp_pools; + tmp_pools = NULL; + } + + ret = npools; + +cleanup: + if (tmp_pools) { + for (i = 0; i < npools; i++) { + if (tmp_pools[i]) + virStoragePoolFree(tmp_pools[i]); + } + } + + VIR_FREE(tmp_pools); + return ret; +} diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 4fb99df..bfa0819 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -518,4 +518,39 @@ enum virStoragePartedFsType { }; VIR_ENUM_DECL(virStoragePartedFsType) +# define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE \ + (VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | \ + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE) + +# define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_PERSISTENT \ + (VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT | \ + VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT) + +# define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART \ + (VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART | \ + VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART) + +# define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE \ + (VIR_CONNECT_LIST_STORAGE_POOLS_DIR | \ + VIR_CONNECT_LIST_STORAGE_POOLS_FS | \ + VIR_CONNECT_LIST_STORAGE_POOLS_NETFS | \ + VIR_CONNECT_LIST_STORAGE_POOLS_LOGICAL | \ + VIR_CONNECT_LIST_STORAGE_POOLS_DISK | \ + VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI | \ + VIR_CONNECT_LIST_STORAGE_POOLS_SCSI | \ + VIR_CONNECT_LIST_STORAGE_POOLS_MPATH | \ + VIR_CONNECT_LIST_STORAGE_POOLS_RBD | \ + VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG) + +# define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL \ + (VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE | \ + VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_PERSISTENT | \ + VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART | \ + VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) + +int virStoragePoolList(virConnectPtr conn, + virStoragePoolObjList poolobjs, + virStoragePoolPtr **pools, + unsigned int flags); + #endif /* __VIR_STORAGE_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 65067d6..abfee47 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1058,6 +1058,7 @@ virStoragePoolDefParseString; virStoragePoolFormatDiskTypeToString; virStoragePoolFormatFileSystemNetTypeToString; virStoragePoolFormatFileSystemTypeToString; +virStoragePoolList; virStoragePoolLoadAllConfigs; virStoragePoolObjAssignDef; virStoragePoolObjClearVols; -- 1.7.7.3

The RPC generator doesn't support returning list of object, this patch do the work manually. * daemon/remote.c: Implement the server side handler remoteDispatchConnectListAllStoragePools * src/remote/remote_driver.c: Add remote driver handler remoteConnectListAllStoragePools. * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 54 +++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 12 ++++++++ 4 files changed, 142 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 24928f4..9056439 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4099,6 +4099,60 @@ cleanup: return rv; } +static int +remoteDispatchConnectListAllStoragePools(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_connect_list_all_storage_pools_args *args, + remote_connect_list_all_storage_pools_ret *ret) +{ + virStoragePoolPtr *pools = NULL; + int npools = 0; + int i; + int rv = -1; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if ((npools = virConnectListAllStoragePools(priv->conn, + args->need_results ? &pools : NULL, + args->flags)) < 0) + goto cleanup; + + if (pools && npools) { + if (VIR_ALLOC_N(ret->pools.pools_val, npools) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret->pools.pools_len = npools; + + for (i = 0; i < npools; i++) + make_nonnull_storage_pool(ret->pools.pools_val + i, pools[i]); + } else { + ret->pools.pools_len = 0; + ret->pools.pools_val = NULL; + } + + ret->ret = npools; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (pools) { + for (i = 0; i < npools; i++) + virStoragePoolFree(pools[i]); + VIR_FREE(pools); + } + return rv; +} + /*----- Helpers. -----*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index cf1f079..c1f9044 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2857,6 +2857,69 @@ done: return rv; } +static int +remoteConnectListAllStoragePools (virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags) +{ + int rv = -1; + int i; + virStoragePoolPtr *tmp_pools = NULL; + remote_connect_list_all_storage_pools_args args; + remote_connect_list_all_storage_pools_ret ret; + + struct private_data *priv = conn->privateData; + + remoteDriverLock(priv); + + args.need_results = !!pools; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, + priv, + 0, + REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS, + (xdrproc_t) xdr_remote_connect_list_all_storage_pools_args, + (char *) &args, + (xdrproc_t) xdr_remote_connect_list_all_storage_pools_ret, + (char *) &ret) == -1) + goto done; + + if (pools) { + if (VIR_ALLOC_N(tmp_pools, ret.pools.pools_len + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < ret.pools.pools_len; i++) { + tmp_pools[i] = get_nonnull_storage_pool(conn, ret.pools.pools_val[i]); + if (!tmp_pools[i]) { + virReportOOMError(); + goto cleanup; + } + } + *pools = tmp_pools; + tmp_pools = NULL; + } + + rv = ret.ret; + +cleanup: + if (tmp_pools) { + for (i = 0; i < ret.pools.pools_len; i++) + if (tmp_pools[i]) + virStoragePoolFree(tmp_pools[i]); + VIR_FREE(tmp_pools); + } + + xdr_free((xdrproc_t) xdr_remote_connect_list_all_storage_pools_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -5612,6 +5675,7 @@ static virStorageDriver storage_driver = { .listPools = remoteListStoragePools, /* 0.4.1 */ .numOfDefinedPools = remoteNumOfDefinedStoragePools, /* 0.4.1 */ .listDefinedPools = remoteListDefinedStoragePools, /* 0.4.1 */ + .listAllPools = remoteConnectListAllStoragePools, /* 0.10.0 */ .findPoolSources = remoteFindStoragePoolSources, /* 0.4.5 */ .poolLookupByName = remoteStoragePoolLookupByName, /* 0.4.1 */ .poolLookupByUUID = remoteStoragePoolLookupByUUID, /* 0.4.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 085d5d9..9de3404 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2558,6 +2558,15 @@ struct remote_connect_list_all_domains_ret { unsigned int ret; }; +struct remote_connect_list_all_storage_pools_args { + int need_results; + unsigned int flags; +}; + +struct remote_connect_list_all_storage_pools_ret { + remote_nonnull_storage_pool pools<>; + unsigned int ret; +}; /*----- Protocol. -----*/ @@ -2888,7 +2897,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_HOSTNAME = 277, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280, /* skipgen skipgen */ + + REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281 /* skipgen skipgen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ed40ccb..560299b 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2011,6 +2011,17 @@ struct remote_connect_list_all_domains_ret { } domains; u_int ret; }; +struct remote_connect_list_all_storage_pools_args { + int need_results; + u_int flags; +}; +struct remote_connect_list_all_storage_pools_ret { + struct { + u_int pools_len; + remote_nonnull_domain * pools_val; + } pools; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2292,4 +2303,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278, REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280, + REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281, }; -- 1.7.7.3

On 09/04/2012 05:48 PM, Eric Blake wrote: Correction; one other change is also necessary: --- remote_protocol-structs 2012-09-04 18:06:02.780444947 -0600 +++ remote_protocol-struct-t3 2012-09-04 18:06:40.975481943 -0600 @@ -2018,7 +2018,7 @@ struct remote_connect_list_all_storage_pools_ret { struct { u_int pools_len; - remote_nonnull_domain * pools_val; + remote_nonnull_storage_pool * pools_val; } pools; u_int ret; }; -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/04/2012 09:16 AM, Osier Yang wrote: 0.10.2 ACK with that fix. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/04/2012 09:16 AM, Osier Yang wrote: 0.10.2. ACK with that fix. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/04/2012 09:16 AM, Osier Yang wrote: Not quite right. If you're going to make the macro reusable, I think it would be better to put it in the VSH_ namespace, and update all existing callers. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

tools/virsh-pool.c: * vshStoragePoolSorter to sort the pool list by pool name. * struct vshStoragePoolList to present the pool list, pool info is collected by list->poolinfo if 'details' is specified by user. * vshStoragePoolListFree to free the pool list * vshStoragePoolListCollect to collect the pool list, new API virStorageListAllPools is tried first, if it's not supported, fall back to older APIs. * New options --persistent, --transient, --autostart, --no-autostart and --type for pool-list. --persistent or --transient is to filter the returned pool list by whether the pool is persistent or not. --autostart or --no-autostart is to filter the returned pool list by whether the pool is autostarting or not. --type is to filter the pools by pool types. E.g. % virsh pool-list --all --persistent --type dir,disk tools/virsh.pod: * Add documentations for the new options. --- tools/virsh-pool.c | 631 +++++++++++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 24 ++- 2 files changed, 652 insertions(+), 3 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index fd239d2..0b328cc 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -36,6 +36,7 @@ #include "memory.h" #include "util.h" #include "xml.h" +#include "conf/storage_conf.h" virStoragePoolPtr vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, @@ -551,6 +552,232 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; } +static int +vshStoragePoolSorter(const void *a, const void *b) +{ + virStoragePoolPtr *pa = (virStoragePoolPtr *) a; + virStoragePoolPtr *pb = (virStoragePoolPtr *) b; + + if (*pa && !*pb) + return -1; + + if (!*pa) + return *pb != NULL; + + return vshStrcasecmp(virStoragePoolGetName(*pa), + virStoragePoolGetName(*pb)); +} + +struct vshStoragePoolList { + virStoragePoolPtr *pools; + size_t npools; +}; +typedef struct vshStoragePoolList *vshStoragePoolListPtr; + +static void +vshStoragePoolListFree(vshStoragePoolListPtr list) +{ + int i; + + if (list && list->pools) { + for (i = 0; i < list->npools; i++) { + if (list->pools[i]) + virStoragePoolFree(list->pools[i]); + } + VIR_FREE(list->pools); + } + VIR_FREE(list); +} + +static vshStoragePoolListPtr +vshStoragePoolListCollect(vshControl *ctl, + unsigned int flags) +{ + vshStoragePoolListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + int ret; + char **names = NULL; + virStoragePoolPtr pool; + bool success = false; + size_t deleted = 0; + int persistent; + int autostart; + int nActivePools = 0; + int nInactivePools = 0; + int nAllPools = 0; + + /* try the list with flags support (0.10.0 and later) */ + if ((ret = virConnectListAllStoragePools(ctl->conn, + &list->pools, + flags)) >= 0) { + list->npools = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + if (last_error && last_error->code == VIR_ERR_INVALID_ARG) { + /* try the new API again but mask non-guaranteed flags */ + unsigned int newflags = flags & (VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE); + vshResetLibvirtError(); + if ((ret = virConnectListAllStoragePools(ctl->conn, &list->pools, + newflags)) >= 0) { + list->npools = ret; + goto filter; + } + } + + /* there was an error during the first or second call */ + vshError(ctl, "%s", _("Failed to list pools")); + goto cleanup; + + +fallback: + /* fall back to old method (0.9.13 and older) */ + vshResetLibvirtError(); + + /* There is no way to get the pool type */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE)) { + vshError(ctl, "%s", _("Filtering using --type is not supported " + "by this libvirt")); + goto cleanup; + } + + /* Get the number of active pools */ + if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || + MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE)) { + if ((nActivePools = virConnectNumOfStoragePools(ctl->conn)) < 0) { + vshError(ctl, "%s", _("Failed to get the number of active pools ")); + goto cleanup; + } + } + + /* Get the number of inactive pools */ + if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || + MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE)) { + if ((nInactivePools = virConnectNumOfDefinedStoragePools(ctl->conn)) < 0) { + vshError(ctl, "%s", _("Failed to get the number of inactive pools")); + goto cleanup; + } + } + + nAllPools = nActivePools + nInactivePools; + + if (nAllPools == 0) + return list; + + names = vshMalloc(ctl, sizeof(char *) * nAllPools); + + /* Retrieve a list of active storage pool names */ + if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || + MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE)) { + if (virConnectListStoragePools(ctl->conn, + names, nActivePools) < 0) { + vshError(ctl, "%s", _("Failed to list active pools")); + goto cleanup; + } + } + + /* Add the inactive storage pools to the end of the name list */ + if (!MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) || + MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE)) { + if (virConnectListDefinedStoragePools(ctl->conn, + &names[nActivePools], + nInactivePools) < 0) { + vshError(ctl, "%s", _("Failed to list inactive pools")); + goto cleanup; + } + } + + list->pools = vshMalloc(ctl, sizeof(virStoragePoolPtr) * (nAllPools)); + list->npools = 0; + + /* get active pools */ + for (i = 0; i < nActivePools; i++) { + if (!(pool = virStoragePoolLookupByName(ctl->conn, names[i]))) + continue; + list->pools[list->npools++] = pool; + } + + /* get inactive pools */ + for (i = 0; i < nInactivePools; i++) { + if (!(pool = virStoragePoolLookupByName(ctl->conn, names[i]))) + continue; + list->pools[list->npools++] = pool; + } + + /* truncate pools that weren't found */ + deleted = nAllPools - list->npools; + +filter: + /* filter list the list if the list was acquired by fallback means */ + for (i = 0; i < list->npools; i++) { + pool = list->pools[i]; + + /* persistence filter */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_PERSISTENT)) { + if ((persistent = virStoragePoolIsPersistent(pool)) < 0) { + vshError(ctl, "%s", _("Failed to get pool persistence info")); + goto cleanup; + } + + if (!((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT) && persistent) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT) && !persistent))) + goto remove_entry; + } + + /* autostart filter */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART)) { + if (virStoragePoolGetAutostart(pool, &autostart) < 0) { + vshError(ctl, "%s", _("Failed to get pool autostart state")); + goto cleanup; + } + + if (!((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART) && autostart) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART) && !autostart))) + goto remove_entry; + } + + /* the pool matched all filters, it may stay */ + continue; + +remove_entry: + /* the pool has to be removed as it failed one of the filters */ + virStoragePoolFree(list->pools[i]); + list->pools[i] = NULL; + deleted++; + } + +finished: + /* sort the list */ + if (list->pools && list->npools) + qsort(list->pools, list->npools, + sizeof(*list->pools), vshStoragePoolSorter); + + /* truncate the list if filter simulation deleted entries */ + if (deleted) + VIR_SHRINK_N(list->pools, list->npools, deleted); + + success = true; + +cleanup: + for (i = 0; i < nAllPools; i++) + VIR_FREE(names[i]); + + if (!success) { + vshStoragePoolListFree(list); + list = NULL; + } + + VIR_FREE(names); + return list; +} + /* * "pool-list" command */ @@ -563,6 +790,11 @@ static const vshCmdInfo info_pool_list[] = { static const vshCmdOptDef opts_pool_list[] = { {"inactive", VSH_OT_BOOL, 0, N_("list inactive pools")}, {"all", VSH_OT_BOOL, 0, N_("list inactive & active pools")}, + {"transient", VSH_OT_BOOL, 0, N_("list transient pools")}, + {"persistent", VSH_OT_BOOL, 0, N_("list persistent pools")}, + {"autostart", VSH_OT_BOOL, 0, N_("list pools with autostart enabled")}, + {"no-autostart", VSH_OT_BOOL, 0, N_("list pools with autostart disabled")}, + {"type", VSH_OT_STRING, 0, N_("only list pool of specified type(s) (if supported)")}, {"details", VSH_OT_BOOL, 0, N_("display extended details for pools")}, {NULL, 0, 0, NULL} }; @@ -571,7 +803,403 @@ static bool cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { virStoragePoolInfo info; - char **poolNames = NULL; + int i, ret; + bool functionReturn = false; + size_t stringLength = 0, nameStrLength = 0; + size_t autostartStrLength = 0, persistStrLength = 0; + size_t stateStrLength = 0, capStrLength = 0; + size_t allocStrLength = 0, availStrLength = 0; + struct poolInfoText { + char *state; + char *autostart; + char *persistent; + char *capacity; + char *allocation; + char *available; + }; + struct poolInfoText *poolInfoTexts = NULL; + unsigned int flags = VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE; + vshStoragePoolListPtr list = NULL; + const char *type = NULL; + bool details = vshCommandOptBool(cmd, "details"); + bool inactive, all; + + inactive = vshCommandOptBool(cmd, "inactive"); + all = vshCommandOptBool(cmd, "all"); + + if (inactive) + flags = VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE; + + if (all) + flags = VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | + VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE; + + if (vshCommandOptBool(cmd, "autostart")) + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART; + + if (vshCommandOptBool(cmd, "no-autostart")) + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_NO_AUTOSTART; + + if (vshCommandOptBool(cmd, "persistent")) + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT; + + if (vshCommandOptBool(cmd, "transient")) + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_TRANSIENT; + + if (vshCommandOptString(cmd, "type", &type) < 0) { + vshError(ctl, "%s", _("Invalid argument for 'type'")); + return false; + } + + if (type) { + int poolType = -1; + char **poolTypes = NULL; + int npoolTypes = 0; + + npoolTypes = vshStringToArray((char *)type, &poolTypes); + + for (i = 0; i < npoolTypes; i++) { + if ((poolType = virStoragePoolTypeFromString(poolTypes[i])) < 0) { + vshError(ctl, "%s", _("Invalid pool type")); + VIR_FREE(poolTypes); + return false; + } + + switch(poolType) { + case VIR_STORAGE_POOL_DIR: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_DIR; + break; + case VIR_STORAGE_POOL_FS: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_FS; + break; + case VIR_STORAGE_POOL_NETFS: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_NETFS; + break; + case VIR_STORAGE_POOL_LOGICAL: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_LOGICAL; + break; + case VIR_STORAGE_POOL_DISK: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_DISK; + break; + case VIR_STORAGE_POOL_ISCSI: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI; + break; + case VIR_STORAGE_POOL_SCSI: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_SCSI; + break; + case VIR_STORAGE_POOL_MPATH: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_MPATH; + break; + case VIR_STORAGE_POOL_RBD: + flags |= VIR_CONNECT_LIST_STORAGE_POOLS_RBD; + break; + default: + break; + } + } + VIR_FREE(poolTypes); + } + + if (!(list = vshStoragePoolListCollect(ctl, flags))) + goto cleanup; + + poolInfoTexts = vshCalloc(ctl, list->npools, sizeof(*poolInfoTexts)); + + /* Collect the storage pool information for display */ + for (i = 0; i < list->npools; i++) { + int autostart = 0, persistent = 0; + + /* Retrieve the autostart status of the pool */ + if (virStoragePoolGetAutostart(list->pools[i], &autostart) < 0) + poolInfoTexts[i].autostart = vshStrdup(ctl, _("no autostart")); + else + poolInfoTexts[i].autostart = vshStrdup(ctl, autostart ? + _("yes") : _("no")); + + /* Retrieve the persistence status of the pool */ + if (details) { + persistent = virStoragePoolIsPersistent(list->pools[i]); + vshDebug(ctl, VSH_ERR_DEBUG, "Persistent flag value: %d\n", + persistent); + if (persistent < 0) + poolInfoTexts[i].persistent = vshStrdup(ctl, _("unknown")); + else + poolInfoTexts[i].persistent = vshStrdup(ctl, persistent ? + _("yes") : _("no")); + + /* Keep the length of persistent string if longest so far */ + stringLength = strlen(poolInfoTexts[i].persistent); + if (stringLength > persistStrLength) + persistStrLength = stringLength; + } + + /* Collect further extended information about the pool */ + if (virStoragePoolGetInfo(list->pools[i], &info) != 0) { + /* Something went wrong retrieving pool info, cope with it */ + vshError(ctl, "%s", _("Could not retrieve pool information")); + poolInfoTexts[i].state = vshStrdup(ctl, _("unknown")); + if (details) { + poolInfoTexts[i].capacity = vshStrdup(ctl, _("unknown")); + poolInfoTexts[i].allocation = vshStrdup(ctl, _("unknown")); + poolInfoTexts[i].available = vshStrdup(ctl, _("unknown")); + } + } else { + /* Decide which state string to display */ + if (details) { + /* --details option was specified, we're using detailed state + * strings */ + switch (info.state) { + case VIR_STORAGE_POOL_INACTIVE: + poolInfoTexts[i].state = vshStrdup(ctl, _("inactive")); + break; + case VIR_STORAGE_POOL_BUILDING: + poolInfoTexts[i].state = vshStrdup(ctl, _("building")); + break; + case VIR_STORAGE_POOL_RUNNING: + poolInfoTexts[i].state = vshStrdup(ctl, _("running")); + break; + case VIR_STORAGE_POOL_DEGRADED: + poolInfoTexts[i].state = vshStrdup(ctl, _("degraded")); + break; + case VIR_STORAGE_POOL_INACCESSIBLE: + poolInfoTexts[i].state = vshStrdup(ctl, _("inaccessible")); + break; + } + + /* Create the pool size related strings */ + if (info.state == VIR_STORAGE_POOL_RUNNING || + info.state == VIR_STORAGE_POOL_DEGRADED) { + double val; + const char *unit; + + /* Create the capacity output string */ + val = vshPrettyCapacity(info.capacity, &unit); + ret = virAsprintf(&poolInfoTexts[i].capacity, + "%.2lf %s", val, unit); + if (ret < 0) + /* An error occurred creating the string, return */ + goto asprintf_failure; + + /* Create the allocation output string */ + val = vshPrettyCapacity(info.allocation, &unit); + ret = virAsprintf(&poolInfoTexts[i].allocation, + "%.2lf %s", val, unit); + if (ret < 0) + /* An error occurred creating the string, return */ + goto asprintf_failure; + + /* Create the available space output string */ + val = vshPrettyCapacity(info.available, &unit); + ret = virAsprintf(&poolInfoTexts[i].available, + "%.2lf %s", val, unit); + if (ret < 0) + /* An error occurred creating the string, return */ + goto asprintf_failure; + } else { + /* Capacity related information isn't available */ + poolInfoTexts[i].capacity = vshStrdup(ctl, _("-")); + poolInfoTexts[i].allocation = vshStrdup(ctl, _("-")); + poolInfoTexts[i].available = vshStrdup(ctl, _("-")); + } + + /* Keep the length of capacity string if longest so far */ + stringLength = strlen(poolInfoTexts[i].capacity); + if (stringLength > capStrLength) + capStrLength = stringLength; + + /* Keep the length of allocation string if longest so far */ + stringLength = strlen(poolInfoTexts[i].allocation); + if (stringLength > allocStrLength) + allocStrLength = stringLength; + + /* Keep the length of available string if longest so far */ + stringLength = strlen(poolInfoTexts[i].available); + if (stringLength > availStrLength) + availStrLength = stringLength; + } else { + /* --details option was not specified, only active/inactive + * state strings are used */ + if (virStoragePoolIsActive(list->pools[i])) + poolInfoTexts[i].state = vshStrdup(ctl, _("active")); + else + poolInfoTexts[i].state = vshStrdup(ctl, _("inactive")); + } + } + + /* Keep the length of name string if longest so far */ + stringLength = strlen(virStoragePoolGetName(list->pools[i])); + if (stringLength > nameStrLength) + nameStrLength = stringLength; + + /* Keep the length of state string if longest so far */ + stringLength = strlen(poolInfoTexts[i].state); + if (stringLength > stateStrLength) + stateStrLength = stringLength; + + /* Keep the length of autostart string if longest so far */ + stringLength = strlen(poolInfoTexts[i].autostart); + if (stringLength > autostartStrLength) + autostartStrLength = stringLength; + } + + /* If the --details option wasn't selected, we output the pool + * info using the fixed string format from previous versions to + * maintain backward compatibility. + */ + + /* Output basic info then return if --details option not selected */ + if (!details) { + /* Output old style header */ + vshPrintExtra(ctl, "%-20s %-10s %-10s\n", _("Name"), _("State"), + _("Autostart")); + vshPrintExtra(ctl, "-----------------------------------------\n"); + + /* Output old style pool info */ + for (i = 0; i < list->npools; i++) { + const char *name = virStoragePoolGetName(list->pools[i]); + vshPrint(ctl, "%-20s %-10s %-10s\n", + name, + poolInfoTexts[i].state, + poolInfoTexts[i].autostart); + } + + /* Cleanup and return */ + functionReturn = true; + goto cleanup; + } + + /* We only get here if the --details option was selected. */ + + /* Use the length of name header string if it's longest */ + stringLength = strlen(_("Name")); + if (stringLength > nameStrLength) + nameStrLength = stringLength; + + /* Use the length of state header string if it's longest */ + stringLength = strlen(_("State")); + if (stringLength > stateStrLength) + stateStrLength = stringLength; + + /* Use the length of autostart header string if it's longest */ + stringLength = strlen(_("Autostart")); + if (stringLength > autostartStrLength) + autostartStrLength = stringLength; + + /* Use the length of persistent header string if it's longest */ + stringLength = strlen(_("Persistent")); + if (stringLength > persistStrLength) + persistStrLength = stringLength; + + /* Use the length of capacity header string if it's longest */ + stringLength = strlen(_("Capacity")); + if (stringLength > capStrLength) + capStrLength = stringLength; + + /* Use the length of allocation header string if it's longest */ + stringLength = strlen(_("Allocation")); + if (stringLength > allocStrLength) + allocStrLength = stringLength; + + /* Use the length of available header string if it's longest */ + stringLength = strlen(_("Available")); + if (stringLength > availStrLength) + availStrLength = stringLength; + + /* Display the string lengths for debugging. */ + vshDebug(ctl, VSH_ERR_DEBUG, "Longest name string = %lu chars\n", + (unsigned long) nameStrLength); + vshDebug(ctl, VSH_ERR_DEBUG, "Longest state string = %lu chars\n", + (unsigned long) stateStrLength); + vshDebug(ctl, VSH_ERR_DEBUG, "Longest autostart string = %lu chars\n", + (unsigned long) autostartStrLength); + vshDebug(ctl, VSH_ERR_DEBUG, "Longest persistent string = %lu chars\n", + (unsigned long) persistStrLength); + vshDebug(ctl, VSH_ERR_DEBUG, "Longest capacity string = %lu chars\n", + (unsigned long) capStrLength); + vshDebug(ctl, VSH_ERR_DEBUG, "Longest allocation string = %lu chars\n", + (unsigned long) allocStrLength); + vshDebug(ctl, VSH_ERR_DEBUG, "Longest available string = %lu chars\n", + (unsigned long) availStrLength); + + /* Create the output template. Each column is sized according to + * the longest string. + */ + char *outputStr; + ret = virAsprintf(&outputStr, + "%%-%lus %%-%lus %%-%lus %%-%lus %%%lus %%%lus %%%lus\n", + (unsigned long) nameStrLength, + (unsigned long) stateStrLength, + (unsigned long) autostartStrLength, + (unsigned long) persistStrLength, + (unsigned long) capStrLength, + (unsigned long) allocStrLength, + (unsigned long) availStrLength); + if (ret < 0) { + /* An error occurred creating the string, return */ + goto asprintf_failure; + } + + /* Display the header */ + vshPrint(ctl, outputStr, _("Name"), _("State"), _("Autostart"), + _("Persistent"), _("Capacity"), _("Allocation"), _("Available")); + for (i = nameStrLength + stateStrLength + autostartStrLength + + persistStrLength + capStrLength + + allocStrLength + availStrLength + + 12; i > 0; i--) + vshPrintExtra(ctl, "-"); + vshPrintExtra(ctl, "\n"); + + /* Display the pool info rows */ + for (i = 0; i < list->npools; i++) { + vshPrint(ctl, outputStr, + virStoragePoolGetName(list->pools[i]), + poolInfoTexts[i].state, + poolInfoTexts[i].autostart, + poolInfoTexts[i].persistent, + poolInfoTexts[i].capacity, + poolInfoTexts[i].allocation, + poolInfoTexts[i].available); + } + + /* Cleanup and return */ + functionReturn = true; + goto cleanup; + +asprintf_failure: + /* Display an appropriate error message then cleanup and return */ + switch (errno) { + case ENOMEM: + /* Couldn't allocate memory */ + vshError(ctl, "%s", _("Out of memory")); + break; + default: + /* Some other error */ + vshError(ctl, _("virAsprintf failed (errno %d)"), errno); + } + functionReturn = false; + +cleanup: + if (list && list->npools) { + for (i = 0; i < list->npools; i++) { + VIR_FREE(poolInfoTexts[i].state); + VIR_FREE(poolInfoTexts[i].autostart); + VIR_FREE(poolInfoTexts[i].persistent); + VIR_FREE(poolInfoTexts[i].capacity); + VIR_FREE(poolInfoTexts[i].allocation); + VIR_FREE(poolInfoTexts[i].available); + } + } + VIR_FREE(poolInfoTexts); + + vshStoragePoolListFree(list); + return functionReturn; +} + +#if 0 +static bool +cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + virStoragePoolInfo info; int i, ret; bool functionReturn; int numActivePools = 0, numInactivePools = 0, numAllPools = 0; @@ -956,6 +1584,7 @@ cleanup: /* Return the desired value */ return functionReturn; } +#endif /* * "find-storage-pool-sources-as" command diff --git a/tools/virsh.pod b/tools/virsh.pod index 98ed4b0..9aeb47e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2185,13 +2185,33 @@ variables, and defaults to C<vi>. Returns basic information about the I<pool> object. -=item B<pool-list> [I<--inactive> | I<--all>] [I<--details>] +=item B<pool-list> [I<--inactive>] [I<--all>] + [I<--persistent>] [I<--transient>] + [I<--autostart>] [I<--no-autostart>] + [[I<--details>] [<type>] List pool objects known to libvirt. By default, only active pools are listed; I<--inactive> lists just the inactive pools, and I<--all> -lists all pools. The I<--details> option instructs virsh to additionally +lists all pools. + +Except the default, I<--inactive>, and I<--all>, you may want to specify more +filtering flags. I<--persistent> is to list the persistent pools, I<--transient> +is to list the transient pools. I<--autostart> is to list the autostarting pools, +I<--no-autostart> is to list the pools with autostarting disabled. + +You may also want to list pools with specified types using I<type>, the +pool types must be separated by comma, e.g. --type dir,disk. The valid pool +types include 'dir', 'fs', 'netfs', 'logical', 'disk', 'iscsi', 'scsi', +'mpath', 'rbd', and 'sheepdog'. + +The I<--details> option instructs virsh to additionally display pool persistence and capacity related information where available. +NOTE: When talking to older servers, this command is forced to use a series of +API calls with an inherent race, where a pool might not be listed or might appear +more than once if it changed state between calls while the list was being +collected. Newer servers do not have this problem. + =item B<pool-name> I<uuid> Convert the I<uuid> to a pool name. -- 1.7.7.3

On 09/05/2012 12:36 AM, Osier Yang wrote: If you rename things to VSH_MATCH in 8/10, then this patch will be impacted. I'm not sure if virsh is supposed to be able to use conf/*.h files; you're not the first offender, but the more we do this, the more we are admitting that our public API is insufficient. I'm wondering if we should move the filter group constants into libvirt.h, and make them part of the public API... Regardless of what we decide about the above, though, I think it would be independent cleanup patches and doesn't affect this patch. 0.10.2 Why two spaces after ==? 0.10.1 Missing a plural, and the parenthetical '(if supported)' doesn't add any value. Maybe: N_("filter pools by type (multiple types separated by commas)") Hmm. What happens if we add new pool types in the future? If libvirt 0.10.3 supports the new pool type 'foo', but I am connecting with virsh 0.10.2 as the client, then I am unable to specify the pool type, even though it is valid to the server. But I don't have any good suggestions on how to make the full list of supported pool types available, and which bits they map to, unless we do something like add a new API to return a list of supported pool types and then have this virsh function consult that list instead of hard-coding the list at the time of the libvirt release when virsh was compiled. Probably not worth the effort. If we hit the default case, that means that conf/storage_conf.h has added new pool types but we forgot to update this case statement to cope with them. I think you want to raise an error here instead of doing silent fallthrough, so that we can diagnose such an issue. Reads awkwardly. How about: In addition, there are several sets of s/is to list/lists/g I think the things that I pointed out are either too big (and would deserve a separate patch if we decide to do anything at all), or are easily fixable, so I'm okay giving: ACK with findings fixed. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/05/2012 12:52 PM, Eric Blake wrote: Yes. (or whatever it takes to not use non-public .h files in virsh). virsh should only use the public libvirt API; if it needs something that's private to libvirt, either that piece of code should be rewritten, or the public API should be enhanced. (But, as you say, Osier isn't the first offender, so it's okay to let this temporarily slip by).

On 2012年09月06日 00:52, Eric Blake wrote: Updated when pushing. Updated when pushing. Likewise. Added ... vshError(ctl, "%s", _("Unknown pool type")); ... when pushing. Updated when pushing. Likewise.

The implementation is done manually as the generator does not support wrapping lists of C pointers into Python objects. python/libvirt-override-api.xml: Document python/libvirt-override-virConnect.py: Add listAllStoragePools python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 6 ++++ python/libvirt-override-virConnect.py | 12 ++++++++ python/libvirt-override.c | 47 +++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 0 deletions(-) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 67ef36e..d16755c 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -306,6 +306,12 @@ <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> <return type='str *' info='the list of Names of None in case of error'/> </function> + <function name='virConnectListAllStoragePools' file='python'> + <info>returns list of all storage pools</info> + <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> + <arg name='flags' type='unsigned int' info='optional flags'/> + <return type='pool *' info='the list of pools or None in case of error'/> + </function> <function name='virStoragePoolListVolumes' file='python'> <info>list the storage volumes, stores the pointers to the names in @names</info> <arg name='pool' type='virStoragePoolPtr' info='pointer to the storage pool'/> diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 50177ab..87a737f 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -206,3 +206,15 @@ retlist.append(virDomain(self, _obj=domptr)) return retlist + + def listAllStoragePools(self, flags): + """Returns a list of storage pool objects""" + ret = libvirtmod.virConnectListAllStoragePools(self._o, flags) + if ret is None: + raise libvirtError("virConnectListAllStoragePools() failed", conn=self) + + retlist = list() + for poolptr in ret: + retlist.append(virStoragePool(self, _obj=poolptr)) + + return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 65e8c69..1e3ad89 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2983,6 +2983,52 @@ libvirt_virConnectListDefinedStoragePools(PyObject *self ATTRIBUTE_UNUSED, return py_retval; } +static PyObject * +libvirt_virConnectListAllStoragePools(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *pyobj_conn; + PyObject *py_retval = NULL; + PyObject *tmp = NULL; + virConnectPtr conn; + virStoragePoolPtr *pools = NULL; + int c_retval = 0; + int i; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virConnectListAllStoragePools", + &pyobj_conn, &flags)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virConnectListAllStoragePools(conn, &pools, flags); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + + if (!(py_retval = PyList_New(c_retval))) + goto cleanup; + + for (i = 0; i < c_retval; i++) { + if (!(tmp = libvirt_virStoragePoolPtrWrap(pools[i])) || + PyList_SetItem(py_retval, i, tmp) < 0) { + Py_XDECREF(tmp); + Py_DECREF(py_retval); + py_retval = NULL; + goto cleanup; + } + /* python steals the pointer */ + pools[i] = NULL; + } + +cleanup: + for (i = 0; i < c_retval; i++) + if (pools[i]) + virStoragePoolFree(pools[i]); + VIR_FREE(pools); + return py_retval; +} static PyObject * libvirt_virStoragePoolListVolumes(PyObject *self ATTRIBUTE_UNUSED, @@ -5878,6 +5924,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetVcpuPinInfo", libvirt_virDomainGetVcpuPinInfo, METH_VARARGS, NULL}, {(char *) "virConnectListStoragePools", libvirt_virConnectListStoragePools, METH_VARARGS, NULL}, {(char *) "virConnectListDefinedStoragePools", libvirt_virConnectListDefinedStoragePools, METH_VARARGS, NULL}, + {(char *) "virConnectListAllStoragePools", libvirt_virConnectListAllStoragePools, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetAutostart", libvirt_virStoragePoolGetAutostart, METH_VARARGS, NULL}, {(char *) "virStoragePoolListVolumes", libvirt_virStoragePoolListVolumes, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetInfo", libvirt_virStoragePoolGetInfo, METH_VARARGS, NULL}, -- 1.7.7.3