[libvirt] [PATCH 0/6 v4] Atomic API to list storage volumes

v3 - v4: * Just rebase on the top, and split each API from the big set. Osier Yang (6): list: Define new API virStoragePoolListAllVolumes list: Implemente RPC calls for virStoragePoolListAllVolumes list: Implement virStoragePoolListAllVolumes for storage driver list: Implement virStoragePoolListAllVolumes for test driver list: Use virStoragePoolListAllVolumes in virsh list: Expose virStoragePoolListAllVolumes to Python binding daemon/remote.c | 58 +++++++++ include/libvirt/libvirt.h.in | 3 + python/generator.py | 1 + python/libvirt-override-api.xml | 8 +- python/libvirt-override-virStoragePool.py | 11 ++ python/libvirt-override.c | 50 ++++++++ src/driver.h | 6 +- src/libvirt.c | 50 ++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 66 ++++++++++ src/remote/remote_protocol.x | 14 ++- src/remote_protocol-structs | 13 ++ src/storage/storage_driver.c | 67 ++++++++++ src/test/test_driver.c | 67 ++++++++++ tools/virsh-volume.c | 197 ++++++++++++++++++++++------- 15 files changed, 562 insertions(+), 50 deletions(-) create mode 100644 python/libvirt-override-virStoragePool.py -- 1.7.7.3

Simply returns the storage volume objects. No supported filter flags. include/libvirt/libvirt.h.in: Declare the API python/generator.py: Skip the function for generating. virStoragePool.py will be added in later patch. src/driver.h: virDrvStoragePoolListVolumesFlags src/libvirt.c: Implementation for the API. src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 3 ++ python/generator.py | 1 + src/driver.h | 6 ++++- src/libvirt.c | 50 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 60 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 0c52271..58cd6ff 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2654,6 +2654,9 @@ int virStoragePoolNumOfVolumes (virStoragePoolPtr pool) int virStoragePoolListVolumes (virStoragePoolPtr pool, char **const names, int maxnames); +int virStoragePoolListAllVolumes (virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags); virConnectPtr virStorageVolGetConnect (virStorageVolPtr vol); diff --git a/python/generator.py b/python/generator.py index 261efe0..276b4ff 100755 --- a/python/generator.py +++ b/python/generator.py @@ -461,6 +461,7 @@ skip_function = ( 'virDomainListAllSnapshots', # overridden in virDomain.py 'virDomainSnapshotListAllChildren', # overridden in virDomainSnapshot.py 'virConnectListAllStoragePools', # overridden in virConnect.py + 'virStoragePoolListAllVolumes', # overridden in virStoragePool.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 342f6b5..1bdfd29 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1311,7 +1311,10 @@ typedef int (*virDrvStoragePoolListVolumes) (virStoragePoolPtr pool, char **const names, int maxnames); - +typedef int + (*virDrvStoragePoolListAllVolumes) (virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags); typedef virStorageVolPtr (*virDrvStorageVolLookupByName) (virStoragePoolPtr pool, @@ -1419,6 +1422,7 @@ struct _virStorageDriver { virDrvStoragePoolSetAutostart poolSetAutostart; virDrvStoragePoolNumOfVolumes poolNumOfVolumes; virDrvStoragePoolListVolumes poolListVolumes; + virDrvStoragePoolListAllVolumes poolListAllVolumes; virDrvStorageVolLookupByName volLookupByName; virDrvStorageVolLookupByKey volLookupByKey; diff --git a/src/libvirt.c b/src/libvirt.c index 4e4f96b..700dbef 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -12596,6 +12596,54 @@ error: return -1; } +/** + * virStoragePoolListAllVolumes: + * @pool: Pointer to storage pool + * @vols: Pointer to a variable to store the array containing storage volume + * objects or NULL if the list is not required (just returns number + * of volumes). + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect the list of storage volumes, and allocate an array to store those + * objects. + * + * Returns the number of storage volumes found or -1 and sets @vols to + * NULL in case of error. On success, the array stored into @vols 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 virStorageVolFree() on each array element, then calling + * free() on @vols. + */ +int +virStoragePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) +{ + VIR_DEBUG("pool=%p, vols=%p, flags=%x", pool, vols, flags); + + virResetLastError(); + + if (!VIR_IS_STORAGE_POOL(pool)) { + virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (pool->conn->storageDriver && + pool->conn->storageDriver->poolListAllVolumes) { + int ret; + ret = pool->conn->storageDriver->poolListAllVolumes(pool, vols, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(pool->conn); + return -1; +} /** * virStoragePoolNumOfVolumes: @@ -12643,6 +12691,8 @@ error: * Fetch list of storage volume names, limiting to * at most maxnames. * + * To list the volume objects directly, see virStoragePoolListAllVolumes(). + * * Returns the number of names fetched, or -1 on error */ int diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f6068b4..53db37f 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -557,6 +557,7 @@ LIBVIRT_0.10.0 { LIBVIRT_0.10.2 { global: virConnectListAllStoragePools; + virStoragePoolListAllVolumes; } LIBVIRT_0.10.0; # .... define new API here using predicted next version number .... -- 1.7.7.3

On 09/04/12 17:32, Osier Yang wrote:
Simply returns the storage volume objects. No supported filter flags.
include/libvirt/libvirt.h.in: Declare the API python/generator.py: Skip the function for generating. virStoragePool.py will be added in later patch. src/driver.h: virDrvStoragePoolListVolumesFlags src/libvirt.c: Implementation for the API. src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 3 ++ python/generator.py | 1 + src/driver.h | 6 ++++- src/libvirt.c | 50 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 60 insertions(+), 1 deletions(-)
Maybe there are some properties that we could use to filter storage volumes, but those can also be added later. This patch looks OK so ACK if nobody else objects. Peter

On 09/07/2012 07:28 AM, Peter Krempa wrote:
On 09/04/12 17:32, Osier Yang wrote:
Simply returns the storage volume objects. No supported filter flags.
include/libvirt/libvirt.h.in: Declare the API python/generator.py: Skip the function for generating. virStoragePool.py will be added in later patch. src/driver.h: virDrvStoragePoolListVolumesFlags src/libvirt.c: Implementation for the API. src/libvirt_public.syms: Export the symbol to public --- include/libvirt/libvirt.h.in | 3 ++ python/generator.py | 1 + src/driver.h | 6 ++++- src/libvirt.c | 50 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 60 insertions(+), 1 deletions(-)
Maybe there are some properties that we could use to filter storage volumes, but those can also be added later.
Agreed that additional filters can be added later. Off the top of my head, I can think of several useful filters (and I'm pretty sure not all of these filter idears are even exposed by current libvirt API, so we would have to add other API before we could even filter on these bits): whether the storage volume is encrypted, whether it is compressed (possibly even by which compression method), whether the storage volume is sparse or fully allocated, whether the storage volume has a backing file or is standalone, whether the storage volume has internal snapshots, current lock manager status (unlocked, shared lock, read-write lock). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The RPC generator doesn't returning support list of object, this patch do the work manually. * daemon/remote.c: Implemente the server side handler remoteDispatchStoragePoolListAllVolumes * src/remote/remote_driver.c: Add remote driver handler remoteStoragePoolListAllVolumes * src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES and structs to represent the args and ret for it. * src/remote_protocol-structs: Likewise. --- daemon/remote.c | 58 ++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 14 ++++++++- src/remote_protocol-structs | 13 ++++++++ 4 files changed, 150 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 9056439..8208c71 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4153,6 +4153,64 @@ cleanup: return rv; } +static int +remoteDispatchStoragePoolListAllVolumes(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_storage_pool_list_all_volumes_args *args, + remote_storage_pool_list_all_volumes_ret *ret) +{ + virStorageVolPtr *vols = NULL; + virStoragePoolPtr pool = NULL; + int nvols = 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 (!(pool = get_nonnull_storage_pool(priv->conn, args->pool))) + goto cleanup; + + if ((nvols = virStoragePoolListAllVolumes(pool, + args->need_results ? &vols : NULL, + args->flags)) < 0) + goto cleanup; + + if (vols && nvols) { + if (VIR_ALLOC_N(ret->vols.vols_val, nvols) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret->vols.vols_len = nvols; + + for (i = 0; i < nvols; i++) + make_nonnull_storage_vol(ret->vols.vols_val + i, vols[i]); + } else { + ret->vols.vols_len = 0; + ret->vols.vols_val = NULL; + } + + ret->ret = nvols; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (vols) { + for (i = 0; i < nvols; i++) + virStorageVolFree(vols[i]); + VIR_FREE(vols); + } + 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 c1f9044..8d4420e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2920,6 +2920,71 @@ done: return rv; } +static int +remoteStoragePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) +{ + int rv = -1; + int i; + virStorageVolPtr *tmp_vols = NULL; + remote_storage_pool_list_all_volumes_args args; + remote_storage_pool_list_all_volumes_ret ret; + + struct private_data *priv = pool->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_storage_pool(&args.pool, pool); + args.need_results = !!vols; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(pool->conn, + priv, + 0, + REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES, + (xdrproc_t) xdr_remote_storage_pool_list_all_volumes_args, + (char *) &args, + (xdrproc_t) xdr_remote_storage_pool_list_all_volumes_ret, + (char *) &ret) == -1) + goto done; + + if (vols) { + if (VIR_ALLOC_N(tmp_vols, ret.vols.vols_len + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0; i < ret.vols.vols_len; i++) { + tmp_vols[i] = get_nonnull_storage_vol(pool->conn, ret.vols.vols_val[i]); + if (!tmp_vols[i]) { + virReportOOMError(); + goto cleanup; + } + } + *vols = tmp_vols; + tmp_vols = NULL; + } + + rv = ret.ret; + +cleanup: + if (tmp_vols) { + for (i = 0; i < ret.vols.vols_len; i++) + if (tmp_vols[i]) + virStorageVolFree(tmp_vols[i]); + VIR_FREE(tmp_vols); + } + + xdr_free((xdrproc_t) xdr_remote_storage_pool_list_all_volumes_ret, (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + + /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -5694,6 +5759,7 @@ static virStorageDriver storage_driver = { .poolSetAutostart = remoteStoragePoolSetAutostart, /* 0.4.1 */ .poolNumOfVolumes = remoteStoragePoolNumOfVolumes, /* 0.4.1 */ .poolListVolumes = remoteStoragePoolListVolumes, /* 0.4.1 */ + .poolListAllVolumes = remoteStoragePoolListAllVolumes, /* 0.10.0 */ .volLookupByName = remoteStorageVolLookupByName, /* 0.4.1 */ .volLookupByKey = remoteStorageVolLookupByKey, /* 0.4.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 9de3404..e540167 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2568,6 +2568,17 @@ struct remote_connect_list_all_storage_pools_ret { unsigned int ret; }; +struct remote_storage_pool_list_all_volumes_args { + remote_nonnull_storage_pool pool; + int need_results; + unsigned int flags; +}; + +struct remote_storage_pool_list_all_volumes_ret { + remote_nonnull_storage_vol vols<>; + unsigned int ret; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2899,7 +2910,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280, /* skipgen skipgen */ - REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281 /* skipgen skipgen priority:high */ + REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281, /* skipgen skipgen priority:high */ + REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES = 282 /* 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 560299b..f105a38 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2022,6 +2022,18 @@ struct remote_connect_list_all_storage_pools_ret { } pools; u_int ret; }; +struct remote_storage_pool_list_all_volumes_args { + remote_nonnull_storage_pool pool; + int need_results; + u_int flags; +}; +struct remote_storage_pool_list_all_volumes_ret { + struct { + u_int vols_len; + remote_nonnull_storage_vol * vols_val; + } vols; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2304,4 +2316,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280, REMOTE_PROC_CONNECT_LIST_ALL_STORAGE_POOLS = 281, + REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES = 282, }; -- 1.7.7.3

s/Implemente/implement in subject On 09/04/12 17:32, Osier Yang wrote:
The RPC generator doesn't returning support list of object, this patch do the work manually.
* daemon/remote.c: Implemente the server side handler remoteDispatchStoragePoolListAllVolumes
* src/remote/remote_driver.c: Add remote driver handler remoteStoragePoolListAllVolumes
* src/remote/remote_protocol.x: New RPC procedure REMOTE_PROC_STORAGE_POOL_LIST_ALL_VOLUMES and structs to represent the args and ret for it.
* src/remote_protocol-structs: Likewise. --- daemon/remote.c | 58 ++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 14 ++++++++- src/remote_protocol-structs | 13 ++++++++ 4 files changed, 150 insertions(+), 1 deletions(-)
Otherwise looks OK. ACK Peter

src/storage/storage_driver.c: Implement poolListAllVolumes. --- src/storage/storage_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 4f99eb9..4f83348 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1153,6 +1153,72 @@ storagePoolListVolumes(virStoragePoolPtr obj, return -1; } +static int +storagePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) { + virStorageDriverStatePtr driver = pool->conn->storagePrivateData; + virStoragePoolObjPtr obj; + int i; + virStorageVolPtr *tmp_vols = NULL; + virStorageVolPtr vol = NULL; + int nvols = 0; + int ret = -1; + + virCheckFlags(0, -1); + + storageDriverLock(driver); + obj = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid); + storageDriverUnlock(driver); + + if (!obj) { + virReportError(VIR_ERR_NO_STORAGE_POOL, "%s", + _("no storage pool with matching uuid")); + goto cleanup; + } + + if (!virStoragePoolObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("storage pool is not active")); + goto cleanup; + } + + /* Just returns the volumes count */ + if (!vols) { + ret = obj->volumes.count; + goto cleanup; + } + + if (VIR_ALLOC_N(tmp_vols, obj->volumes.count + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < obj->volumes.count; i++) { + if (!(vol = virGetStorageVol(pool->conn, obj->def->name, + obj->volumes.objs[i]->name, + obj->volumes.objs[i]->key))) + goto cleanup; + tmp_vols[nvols++] = vol; + } + + *vols = tmp_vols; + tmp_vols = NULL; + ret = nvols; + + cleanup: + if (tmp_vols) { + for (i = 0; i < nvols; i++) { + if (tmp_vols[i]) + virStorageVolFree(tmp_vols[i]); + } + } + + if (obj) + virStoragePoolObjUnlock(obj); + + return ret; +} static virStorageVolPtr storageVolumeLookupByName(virStoragePoolPtr obj, @@ -2329,6 +2395,7 @@ static virStorageDriver storageDriver = { .poolSetAutostart = storagePoolSetAutostart, /* 0.4.0 */ .poolNumOfVolumes = storagePoolNumVolumes, /* 0.4.0 */ .poolListVolumes = storagePoolListVolumes, /* 0.4.0 */ + .poolListAllVolumes = storagePoolListAllVolumes, /* 0.10.0 */ .volLookupByName = storageVolumeLookupByName, /* 0.4.0 */ .volLookupByKey = storageVolumeLookupByKey, /* 0.4.0 */ -- 1.7.7.3

On 09/04/12 17:32, Osier Yang wrote:
src/storage/storage_driver.c: Implement poolListAllVolumes. --- src/storage/storage_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 4f99eb9..4f83348 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1153,6 +1153,72 @@ storagePoolListVolumes(virStoragePoolPtr obj, return -1; }
+static int +storagePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) { + virStorageDriverStatePtr driver = pool->conn->storagePrivateData; + virStoragePoolObjPtr obj; + int i; + virStorageVolPtr *tmp_vols = NULL; + virStorageVolPtr vol = NULL; + int nvols = 0; + int ret = -1; + + virCheckFlags(0, -1); + + storageDriverLock(driver); + obj = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid); + storageDriverUnlock(driver); + + if (!obj) { + virReportError(VIR_ERR_NO_STORAGE_POOL, "%s", + _("no storage pool with matching uuid")); + goto cleanup; + } + + if (!virStoragePoolObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("storage pool is not active")); + goto cleanup; + } + + /* Just returns the volumes count */ + if (!vols) { + ret = obj->volumes.count; + goto cleanup; + } + + if (VIR_ALLOC_N(tmp_vols, obj->volumes.count + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < obj->volumes.count; i++) { + if (!(vol = virGetStorageVol(pool->conn, obj->def->name, + obj->volumes.objs[i]->name, + obj->volumes.objs[i]->key))) + goto cleanup; + tmp_vols[nvols++] = vol; + } + + *vols = tmp_vols; + tmp_vols = NULL; + ret = nvols; + + cleanup: + if (tmp_vols) { + for (i = 0; i < nvols; i++) { + if (tmp_vols[i]) + virStorageVolFree(tmp_vols[i]); + } + } + + if (obj) + virStoragePoolObjUnlock(obj); + + return ret; +}
static virStorageVolPtr storageVolumeLookupByName(virStoragePoolPtr obj, @@ -2329,6 +2395,7 @@ static virStorageDriver storageDriver = { .poolSetAutostart = storagePoolSetAutostart, /* 0.4.0 */ .poolNumOfVolumes = storagePoolNumVolumes, /* 0.4.0 */ .poolListVolumes = storagePoolListVolumes, /* 0.4.0 */ + .poolListAllVolumes = storagePoolListAllVolumes, /* 0.10.0 */
s/0.10.0/0.10.2/ ... Hm looks like that you've got that right in your github repo already.
.volLookupByName = storageVolumeLookupByName, /* 0.4.0 */ .volLookupByKey = storageVolumeLookupByKey, /* 0.4.0 */
Looks good. ACK Peter

src/test/test_driver.c: Implement poolListAllVolumes. --- src/test/test_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1504251..8d93129 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4555,6 +4555,72 @@ testStoragePoolListVolumes(virStoragePoolPtr pool, return -1; } +static int +testStoragePoolListAllVolumes(virStoragePoolPtr obj, + virStorageVolPtr **vols, + unsigned int flags) { + testConnPtr privconn = obj->conn->privateData; + virStoragePoolObjPtr pool; + int i; + virStorageVolPtr *tmp_vols = NULL; + virStorageVolPtr vol = NULL; + int nvols = 0; + int ret = -1; + + virCheckFlags(0, -1); + + testDriverLock(privconn); + pool = virStoragePoolObjFindByUUID(&privconn->pools, obj->uuid); + testDriverUnlock(privconn); + + if (!pool) { + virReportError(VIR_ERR_NO_STORAGE_POOL, "%s", + _("no storage pool with matching uuid")); + goto cleanup; + } + + if (!virStoragePoolObjIsActive(pool)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("storage pool is not active")); + goto cleanup; + } + + /* Just returns the volumes count */ + if (!vols) { + ret = pool->volumes.count; + goto cleanup; + } + + if (VIR_ALLOC_N(tmp_vols, pool->volumes.count + 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < pool->volumes.count; i++) { + if (!(vol = virGetStorageVol(obj->conn, pool->def->name, + pool->volumes.objs[i]->name, + pool->volumes.objs[i]->key))) + goto cleanup; + tmp_vols[nvols++] = vol; + } + + *vols = tmp_vols; + tmp_vols = NULL; + ret = nvols; + + cleanup: + if (tmp_vols) { + for (i = 0; i < nvols; i++) { + if (tmp_vols[i]) + virStorageVolFree(tmp_vols[i]); + } + } + + if (pool) + virStoragePoolObjUnlock(pool); + + return ret; +} static virStorageVolPtr testStorageVolumeLookupByName(virStoragePoolPtr pool, @@ -5697,6 +5763,7 @@ static virStorageDriver testStorageDriver = { .poolSetAutostart = testStoragePoolSetAutostart, /* 0.5.0 */ .poolNumOfVolumes = testStoragePoolNumVolumes, /* 0.5.0 */ .poolListVolumes = testStoragePoolListVolumes, /* 0.5.0 */ + .poolListAllVolumes = testStoragePoolListAllVolumes, /* 0.10.0 */ .volLookupByName = testStorageVolumeLookupByName, /* 0.5.0 */ .volLookupByKey = testStorageVolumeLookupByKey, /* 0.5.0 */ -- 1.7.7.3

On 09/04/12 17:32, Osier Yang wrote:
src/test/test_driver.c: Implement poolListAllVolumes. --- src/test/test_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-)
Same issue with the version tag as in 3/6 but also fixed in the repo. ACK Peter

tools/virsh-volume.c: * vshStorageVolSorter to sort storage vols by name * vshStorageVolumeListFree to free the volume objects list * vshStorageVolumeListCollect to collect the volume objects, trying to use new API first, fall back to older APIs if it's not supported. --- tools/virsh-volume.c | 197 ++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 150 insertions(+), 47 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 6ab271e..ec0b5b0 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -966,6 +966,133 @@ cmdVolDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; } +static int +vshStorageVolSorter(const void *a, const void *b) +{ + virStorageVolPtr *va = (virStorageVolPtr *) a; + virStorageVolPtr *vb = (virStorageVolPtr *) b; + + if (*va && !*vb) + return -1; + + if (!*va) + return *vb != NULL; + + return vshStrcasecmp(virStorageVolGetName(*va), + virStorageVolGetName(*vb)); +} + +struct vshStorageVolList { + virStorageVolPtr *vols; + size_t nvols; +}; +typedef struct vshStorageVolList *vshStorageVolListPtr; + +static void +vshStorageVolListFree(vshStorageVolListPtr list) +{ + int i; + + if (list && list->vols) { + for (i = 0; i < list->nvols; i++) { + if (list->vols[i]) + virStorageVolFree(list->vols[i]); + } + VIR_FREE(list->vols); + } + VIR_FREE(list); +} + +static vshStorageVolListPtr +vshStorageVolListCollect(vshControl *ctl, + virStoragePoolPtr pool, + unsigned int flags) +{ + vshStorageVolListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + char **names = NULL; + virStorageVolPtr vol = NULL; + bool success = false; + size_t deleted = 0; + int nvols = 0; + int ret = -1; + + /* try the list with flags support (0.10.0 and later) */ + if ((ret = virStoragePoolListAllVolumes(pool, + &list->vols, + flags)) >= 0) { + list->nvols = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + /* there was an error during the call */ + vshError(ctl, "%s", _("Failed to list volumes")); + goto cleanup; + +fallback: + /* fall back to old method (0.9.13 and older) */ + vshResetLibvirtError(); + + /* Determine the number of volumes in the pool */ + if ((nvols = virStoragePoolNumOfVolumes(pool)) < 0) { + vshError(ctl, "%s", _("Failed to list storage volumes")); + goto cleanup; + } + + if (nvols == 0) { + success = true; + return list; + } + + /* Retrieve the list of volume names in the pool */ + names = vshCalloc(ctl, nvols, sizeof(*names)); + if ((nvols = virStoragePoolListVolumes(pool, names, nvols)) < 0) { + vshError(ctl, "%s", _("Failed to list storage volumes")); + goto cleanup; + } + + list->vols = vshMalloc(ctl, sizeof(virStorageVolPtr) * (nvols)); + list->nvols = 0; + + /* get the vols */ + for (i = 0; i < nvols; i++) { + if (!(vol = virStorageVolLookupByName(pool, names[i]))) + continue; + list->vols[list->nvols++] = vol; + } + + /* truncate the list for not found vols */ + deleted = nvols - list->nvols; + +finished: + /* sort the list */ + if (list->vols && list->nvols) + qsort(list->vols, list->nvols, sizeof(*list->vols), vshStorageVolSorter); + + if (deleted) + VIR_SHRINK_N(list->vols, list->nvols, deleted); + + success = true; + +cleanup: + for (i = 0; i < nvols; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + + if (!success) { + vshStorageVolListFree(list); + list = NULL; + } + + return list; +} + /* * "vol-list" command */ @@ -986,14 +1113,13 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { virStorageVolInfo volumeInfo; virStoragePoolPtr pool; - char **activeNames = NULL; char *outputStr = NULL; const char *unit; double val; bool details = vshCommandOptBool(cmd, "details"); - int numVolumes = 0, i; + int i; int ret; - bool functionReturn; + bool functionReturn = false; int stringLength = 0; size_t allocStrLength = 0, capStrLength = 0; size_t nameStrLength = 0, pathStrLength = 0; @@ -1005,43 +1131,22 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) char *type; }; struct volInfoText *volInfoTexts = NULL; + vshStorageVolListPtr list = NULL; /* Look up the pool information given to us by the user */ if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL))) return false; - /* Determine the number of volumes in the pool */ - numVolumes = virStoragePoolNumOfVolumes(pool); - - if (numVolumes < 0) { - vshError(ctl, "%s", _("Failed to list storage volumes")); - virStoragePoolFree(pool); - return false; - } - - /* Retrieve the list of volume names in the pool */ - if (numVolumes > 0) { - activeNames = vshCalloc(ctl, numVolumes, sizeof(*activeNames)); - if ((numVolumes = virStoragePoolListVolumes(pool, activeNames, - numVolumes)) < 0) { - vshError(ctl, "%s", _("Failed to list active vols")); - VIR_FREE(activeNames); - virStoragePoolFree(pool); - return false; - } - - /* Sort the volume names */ - qsort(&activeNames[0], numVolumes, sizeof(*activeNames), vshNameSorter); + if (!(list = vshStorageVolListCollect(ctl, pool, 0))) + goto cleanup; - /* Set aside memory for volume information pointers */ - volInfoTexts = vshCalloc(ctl, numVolumes, sizeof(*volInfoTexts)); - } + if (list->nvols > 0) + volInfoTexts = vshCalloc(ctl, list->nvols, sizeof(*volInfoTexts)); /* Collect the rest of the volume information for display */ - for (i = 0; i < numVolumes; i++) { + for (i = 0; i < list->nvols; i++) { /* Retrieve volume info */ - virStorageVolPtr vol = virStorageVolLookupByName(pool, - activeNames[i]); + virStorageVolPtr vol = list->vols[i]; /* Retrieve the volume path */ if ((volInfoTexts[i].path = virStorageVolGetPath(vol)) == NULL) { @@ -1099,7 +1204,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) */ /* Keep the length of name string if longest so far */ - stringLength = strlen(activeNames[i]); + stringLength = strlen(virStorageVolGetName(list->vols[i])); if (stringLength > nameStrLength) nameStrLength = stringLength; @@ -1123,9 +1228,6 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) if (stringLength > allocStrLength) allocStrLength = stringLength; } - - /* Cleanup memory allocation */ - virStorageVolFree(vol); } /* If the --details option wasn't selected, we output the volume @@ -1138,8 +1240,8 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* The old output format */ vshPrintExtra(ctl, "%-20s %-40s\n", _("Name"), _("Path")); vshPrintExtra(ctl, "-----------------------------------------\n"); - for (i = 0; i < numVolumes; i++) { - vshPrint(ctl, "%-20s %-40s\n", activeNames[i], + for (i = 0; i < list->nvols; i++) { + vshPrint(ctl, "%-20s %-40s\n", virStorageVolGetName(list->vols[i]), volInfoTexts[i].path); } @@ -1210,9 +1312,9 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshPrintExtra(ctl, "\n"); /* Display the volume info rows */ - for (i = 0; i < numVolumes; i++) { + for (i = 0; i < list->nvols; i++) { vshPrint(ctl, outputStr, - activeNames[i], + virStorageVolGetName(list->vols[i]), volInfoTexts[i].path, volInfoTexts[i].type, volInfoTexts[i].capacity, @@ -1240,20 +1342,21 @@ asprintf_failure: cleanup: /* Safely free the memory allocated in this function */ - for (i = 0; i < numVolumes; i++) { - /* Cleanup the memory for one volume info structure per loop */ - VIR_FREE(volInfoTexts[i].path); - VIR_FREE(volInfoTexts[i].type); - VIR_FREE(volInfoTexts[i].capacity); - VIR_FREE(volInfoTexts[i].allocation); - VIR_FREE(activeNames[i]); + if (list && list->nvols) { + for (i = 0; i < list->nvols; i++) { + /* Cleanup the memory for one volume info structure per loop */ + VIR_FREE(volInfoTexts[i].path); + VIR_FREE(volInfoTexts[i].type); + VIR_FREE(volInfoTexts[i].capacity); + VIR_FREE(volInfoTexts[i].allocation); + } } /* Cleanup remaining memory */ VIR_FREE(outputStr); VIR_FREE(volInfoTexts); - VIR_FREE(activeNames); virStoragePoolFree(pool); + vshStorageVolListFree(list); /* Return the desired value */ return functionReturn; -- 1.7.7.3

On 09/04/12 17:32, Osier Yang wrote:
tools/virsh-volume.c: * vshStorageVolSorter to sort storage vols by name
* vshStorageVolumeListFree to free the volume objects list
* vshStorageVolumeListCollect to collect the volume objects, trying to use new API first, fall back to older APIs if it's not supported. --- tools/virsh-volume.c | 197 ++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 150 insertions(+), 47 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 6ab271e..ec0b5b0 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -966,6 +966,133 @@ cmdVolDumpXML(vshControl *ctl, const vshCmd *cmd) return ret; }
+static int +vshStorageVolSorter(const void *a, const void *b) +{ + virStorageVolPtr *va = (virStorageVolPtr *) a; + virStorageVolPtr *vb = (virStorageVolPtr *) b; + + if (*va && !*vb) + return -1; + + if (!*va) + return *vb != NULL; + + return vshStrcasecmp(virStorageVolGetName(*va), + virStorageVolGetName(*vb));
Missaligned.
+} + +struct vshStorageVolList { + virStorageVolPtr *vols; + size_t nvols; +}; +typedef struct vshStorageVolList *vshStorageVolListPtr; + +static void +vshStorageVolListFree(vshStorageVolListPtr list) +{ + int i; + + if (list && list->vols) { + for (i = 0; i < list->nvols; i++) { + if (list->vols[i]) + virStorageVolFree(list->vols[i]); + } + VIR_FREE(list->vols); + } + VIR_FREE(list); +} + +static vshStorageVolListPtr +vshStorageVolListCollect(vshControl *ctl, + virStoragePoolPtr pool, + unsigned int flags) +{ + vshStorageVolListPtr list = vshMalloc(ctl, sizeof(*list)); + int i; + char **names = NULL; + virStorageVolPtr vol = NULL; + bool success = false; + size_t deleted = 0; + int nvols = 0; + int ret = -1; + + /* try the list with flags support (0.10.0 and later) */ + if ((ret = virStoragePoolListAllVolumes(pool, + &list->vols, + flags)) >= 0) { + list->nvols = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + /* there was an error during the call */ + vshError(ctl, "%s", _("Failed to list volumes")); + goto cleanup; + +fallback: + /* fall back to old method (0.9.13 and older) */
s/0.9.13/0.10.2/
+ vshResetLibvirtError();
This error reset is not necessary as you don't have two fallback paths as in domain listing.
+ + /* Determine the number of volumes in the pool */ + if ((nvols = virStoragePoolNumOfVolumes(pool)) < 0) { + vshError(ctl, "%s", _("Failed to list storage volumes")); + goto cleanup; + } + + if (nvols == 0) { + success = true; + return list; + } + + /* Retrieve the list of volume names in the pool */ + names = vshCalloc(ctl, nvols, sizeof(*names)); + if ((nvols = virStoragePoolListVolumes(pool, names, nvols)) < 0) { + vshError(ctl, "%s", _("Failed to list storage volumes")); + goto cleanup; + } + + list->vols = vshMalloc(ctl, sizeof(virStorageVolPtr) * (nvols)); + list->nvols = 0; + + /* get the vols */ + for (i = 0; i < nvols; i++) { + if (!(vol = virStorageVolLookupByName(pool, names[i]))) + continue; + list->vols[list->nvols++] = vol; + } + + /* truncate the list for not found vols */ + deleted = nvols - list->nvols; + +finished: + /* sort the list */ + if (list->vols && list->nvols) + qsort(list->vols, list->nvols, sizeof(*list->vols), vshStorageVolSorter); + + if (deleted) + VIR_SHRINK_N(list->vols, list->nvols, deleted); + + success = true; + +cleanup: + for (i = 0; i < nvols; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + + if (!success) { + vshStorageVolListFree(list); + list = NULL; + } + + return list; +} + /* * "vol-list" command */ @@ -986,14 +1113,13 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { virStorageVolInfo volumeInfo; virStoragePoolPtr pool; - char **activeNames = NULL; char *outputStr = NULL; const char *unit; double val; bool details = vshCommandOptBool(cmd, "details"); - int numVolumes = 0, i; + int i; int ret; - bool functionReturn; + bool functionReturn = false; int stringLength = 0; size_t allocStrLength = 0, capStrLength = 0; size_t nameStrLength = 0, pathStrLength = 0; @@ -1005,43 +1131,22 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) char *type; }; struct volInfoText *volInfoTexts = NULL; + vshStorageVolListPtr list = NULL;
/* Look up the pool information given to us by the user */ if (!(pool = vshCommandOptPool(ctl, cmd, "pool", NULL))) return false;
- /* Determine the number of volumes in the pool */ - numVolumes = virStoragePoolNumOfVolumes(pool); - - if (numVolumes < 0) { - vshError(ctl, "%s", _("Failed to list storage volumes")); - virStoragePoolFree(pool); - return false; - } - - /* Retrieve the list of volume names in the pool */ - if (numVolumes > 0) { - activeNames = vshCalloc(ctl, numVolumes, sizeof(*activeNames)); - if ((numVolumes = virStoragePoolListVolumes(pool, activeNames, - numVolumes)) < 0) { - vshError(ctl, "%s", _("Failed to list active vols")); - VIR_FREE(activeNames); - virStoragePoolFree(pool); - return false; - } - - /* Sort the volume names */ - qsort(&activeNames[0], numVolumes, sizeof(*activeNames), vshNameSorter); + if (!(list = vshStorageVolListCollect(ctl, pool, 0))) + goto cleanup;
- /* Set aside memory for volume information pointers */ - volInfoTexts = vshCalloc(ctl, numVolumes, sizeof(*volInfoTexts)); - } + if (list->nvols > 0) + volInfoTexts = vshCalloc(ctl, list->nvols, sizeof(*volInfoTexts));
/* Collect the rest of the volume information for display */ - for (i = 0; i < numVolumes; i++) { + for (i = 0; i < list->nvols; i++) { /* Retrieve volume info */ - virStorageVolPtr vol = virStorageVolLookupByName(pool, - activeNames[i]); + virStorageVolPtr vol = list->vols[i];
/* Retrieve the volume path */ if ((volInfoTexts[i].path = virStorageVolGetPath(vol)) == NULL) { @@ -1099,7 +1204,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) */
/* Keep the length of name string if longest so far */ - stringLength = strlen(activeNames[i]); + stringLength = strlen(virStorageVolGetName(list->vols[i])); if (stringLength > nameStrLength) nameStrLength = stringLength;
@@ -1123,9 +1228,6 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) if (stringLength > allocStrLength) allocStrLength = stringLength; } - - /* Cleanup memory allocation */ - virStorageVolFree(vol); }
/* If the --details option wasn't selected, we output the volume @@ -1138,8 +1240,8 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* The old output format */ vshPrintExtra(ctl, "%-20s %-40s\n", _("Name"), _("Path")); vshPrintExtra(ctl, "-----------------------------------------\n"); - for (i = 0; i < numVolumes; i++) { - vshPrint(ctl, "%-20s %-40s\n", activeNames[i], + for (i = 0; i < list->nvols; i++) { + vshPrint(ctl, "%-20s %-40s\n", virStorageVolGetName(list->vols[i]), volInfoTexts[i].path); }
@@ -1210,9 +1312,9 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshPrintExtra(ctl, "\n");
/* Display the volume info rows */ - for (i = 0; i < numVolumes; i++) { + for (i = 0; i < list->nvols; i++) { vshPrint(ctl, outputStr, - activeNames[i], + virStorageVolGetName(list->vols[i]), volInfoTexts[i].path, volInfoTexts[i].type, volInfoTexts[i].capacity, @@ -1240,20 +1342,21 @@ asprintf_failure: cleanup:
/* Safely free the memory allocated in this function */ - for (i = 0; i < numVolumes; i++) { - /* Cleanup the memory for one volume info structure per loop */ - VIR_FREE(volInfoTexts[i].path); - VIR_FREE(volInfoTexts[i].type); - VIR_FREE(volInfoTexts[i].capacity); - VIR_FREE(volInfoTexts[i].allocation); - VIR_FREE(activeNames[i]); + if (list && list->nvols) { + for (i = 0; i < list->nvols; i++) { + /* Cleanup the memory for one volume info structure per loop */ + VIR_FREE(volInfoTexts[i].path); + VIR_FREE(volInfoTexts[i].type); + VIR_FREE(volInfoTexts[i].capacity); + VIR_FREE(volInfoTexts[i].allocation); + } }
/* Cleanup remaining memory */ VIR_FREE(outputStr); VIR_FREE(volInfoTexts); - VIR_FREE(activeNames); virStoragePoolFree(pool); + vshStorageVolListFree(list);
/* Return the desired value */ return functionReturn;
Looks like a sane replacement. ACK. Peter

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-virStoragePool.py: * New file, includes implementation of listAllVolumes. python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 8 ++++- python/libvirt-override-virStoragePool.py | 11 ++++++ python/libvirt-override.c | 50 +++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletions(-) create mode 100644 python/libvirt-override-virStoragePool.py diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index d16755c..8a228fb 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -315,7 +315,13 @@ <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'/> - <return type='str *' info='the list of Names of None in case of error'/> + <return type='str *' info='the list of Names or None in case of error'/> + </function> + <function name='virStoragePoolListAllVolumes' file='python'> + <info>return list of storage volume objects</info> + <arg name='pool' type='virStoragePoolPtr' info='pointer to the storage pool'/> + <arg name='flags' type='unsigned int' info='optional flags'/> + <return type='volume *' info='the list of volumes or None in case of error'/> </function> <function name='virStoragePoolGetInfo' file='python'> <info>Extract information about a storage pool. Note that if the connection used to get the domain is limited only a partial set of the information can be extracted.</info> diff --git a/python/libvirt-override-virStoragePool.py b/python/libvirt-override-virStoragePool.py new file mode 100644 index 0000000..ffe160c --- /dev/null +++ b/python/libvirt-override-virStoragePool.py @@ -0,0 +1,11 @@ + def listAllVolumes(self, flags): + """List all storage volumes and returns a list of storage volume objects""" + ret = libvirtmod.virStoragePoolListAllVolumes(self._o, flags) + if ret is None: + raise libvirtError("virStoragePoolListAllVolumes() failed", conn=self) + + retlist = list() + for volptr in ret: + retlist.append(virStorageVol(self, _obj=volptr)) + + return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1e3ad89..8b402b0 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3083,6 +3083,55 @@ libvirt_virStoragePoolListVolumes(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virStoragePoolListAllVolumes(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval = NULL; + PyObject *tmp = NULL; + virStoragePoolPtr pool; + virStorageVolPtr *vols = NULL; + int c_retval = 0; + int i; + unsigned int flags; + PyObject *pyobj_pool; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virStoragePoolListAllVolumes", + &pyobj_pool, &flags)) + return NULL; + + pool = (virStoragePoolPtr) PyvirStoragePool_Get(pyobj_pool); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virStoragePoolListAllVolumes(pool, &vols, 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_virStorageVolPtrWrap(vols[i])) || + PyList_SetItem(py_retval, i, tmp) < 0) { + Py_XDECREF(tmp); + Py_DECREF(py_retval); + py_retval = NULL; + goto cleanup; + } + /* python steals the pointer */ + vols[i] = NULL; + } + +cleanup: + for (i = 0; i < c_retval; i++) + if (vols[i]) + virStorageVolFree(vols[i]); + VIR_FREE(vols); + return py_retval; +} + + +static PyObject * libvirt_virStoragePoolGetAutostart(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; int c_retval, autostart; @@ -5927,6 +5976,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectListAllStoragePools", libvirt_virConnectListAllStoragePools, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetAutostart", libvirt_virStoragePoolGetAutostart, METH_VARARGS, NULL}, {(char *) "virStoragePoolListVolumes", libvirt_virStoragePoolListVolumes, METH_VARARGS, NULL}, + {(char *) "virStoragePoolListAllVolumes", libvirt_virStoragePoolListAllVolumes, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetInfo", libvirt_virStoragePoolGetInfo, METH_VARARGS, NULL}, {(char *) "virStorageVolGetInfo", libvirt_virStorageVolGetInfo, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetUUID", libvirt_virStoragePoolGetUUID, METH_VARARGS, NULL}, -- 1.7.7.3

On 09/04/12 17:32, Osier Yang wrote:
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-virStoragePool.py: * New file, includes implementation of listAllVolumes.
python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 8 ++++- python/libvirt-override-virStoragePool.py | 11 ++++++ python/libvirt-override.c | 50 +++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletions(-) create mode 100644 python/libvirt-override-virStoragePool.py
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index d16755c..8a228fb 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -315,7 +315,13 @@ <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'/> - <return type='str *' info='the list of Names of None in case of error'/> + <return type='str *' info='the list of Names or None in case of error'/> + </function> + <function name='virStoragePoolListAllVolumes' file='python'> + <info>return list of storage volume objects</info> + <arg name='pool' type='virStoragePoolPtr' info='pointer to the storage pool'/> + <arg name='flags' type='unsigned int' info='optional flags'/> + <return type='volume *' info='the list of volumes or None in case of error'/> </function> <function name='virStoragePoolGetInfo' file='python'> <info>Extract information about a storage pool. Note that if the connection used to get the domain is limited only a partial set of the information can be extracted.</info> diff --git a/python/libvirt-override-virStoragePool.py b/python/libvirt-override-virStoragePool.py new file mode 100644 index 0000000..ffe160c --- /dev/null +++ b/python/libvirt-override-virStoragePool.py
Hm, I'm no python binding expert. Does this file get used automaticaly?
@@ -0,0 +1,11 @@ + def listAllVolumes(self, flags): + """List all storage volumes and returns a list of storage volume objects""" + ret = libvirtmod.virStoragePoolListAllVolumes(self._o, flags) + if ret is None: + raise libvirtError("virStoragePoolListAllVolumes() failed", conn=self) + + retlist = list() + for volptr in ret: + retlist.append(virStorageVol(self, _obj=volptr)) + + return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1e3ad89..8b402b0 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3083,6 +3083,55 @@ libvirt_virStoragePoolListVolumes(PyObject *self ATTRIBUTE_UNUSED, }
static PyObject * +libvirt_virStoragePoolListAllVolumes(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval = NULL; + PyObject *tmp = NULL; + virStoragePoolPtr pool; + virStorageVolPtr *vols = NULL; + int c_retval = 0; + int i; + unsigned int flags; + PyObject *pyobj_pool; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virStoragePoolListAllVolumes", + &pyobj_pool, &flags)) + return NULL; + + pool = (virStoragePoolPtr) PyvirStoragePool_Get(pyobj_pool); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virStoragePoolListAllVolumes(pool, &vols, 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_virStorageVolPtrWrap(vols[i])) || + PyList_SetItem(py_retval, i, tmp) < 0) { + Py_XDECREF(tmp); + Py_DECREF(py_retval); + py_retval = NULL; + goto cleanup; + } + /* python steals the pointer */ + vols[i] = NULL; + } + +cleanup: + for (i = 0; i < c_retval; i++) + if (vols[i]) + virStorageVolFree(vols[i]); + VIR_FREE(vols); + return py_retval; +} + + +static PyObject * libvirt_virStoragePoolGetAutostart(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; int c_retval, autostart; @@ -5927,6 +5976,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectListAllStoragePools", libvirt_virConnectListAllStoragePools, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetAutostart", libvirt_virStoragePoolGetAutostart, METH_VARARGS, NULL}, {(char *) "virStoragePoolListVolumes", libvirt_virStoragePoolListVolumes, METH_VARARGS, NULL}, + {(char *) "virStoragePoolListAllVolumes", libvirt_virStoragePoolListAllVolumes, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetInfo", libvirt_virStoragePoolGetInfo, METH_VARARGS, NULL}, {(char *) "virStorageVolGetInfo", libvirt_virStorageVolGetInfo, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetUUID", libvirt_virStoragePoolGetUUID, METH_VARARGS, NULL},
ACK if the new python file is used automaticaly Peter

On 2012年09月07日 21:49, Peter Krempa wrote:
On 09/04/12 17:32, Osier Yang wrote:
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-virStoragePool.py: * New file, includes implementation of listAllVolumes.
python/libvirt-override.c: Implementation for the wrapper. --- python/libvirt-override-api.xml | 8 ++++- python/libvirt-override-virStoragePool.py | 11 ++++++ python/libvirt-override.c | 50 +++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletions(-) create mode 100644 python/libvirt-override-virStoragePool.py
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index d16755c..8a228fb 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -315,7 +315,13 @@ <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'/> - <return type='str *' info='the list of Names of None in case of error'/> + <return type='str *' info='the list of Names or None in case of error'/> + </function> + <function name='virStoragePoolListAllVolumes' file='python'> + <info>return list of storage volume objects</info> + <arg name='pool' type='virStoragePoolPtr' info='pointer to the storage pool'/> + <arg name='flags' type='unsigned int' info='optional flags'/> + <return type='volume *' info='the list of volumes or None in case of error'/> </function> <function name='virStoragePoolGetInfo' file='python'> <info>Extract information about a storage pool. Note that if the connection used to get the domain is limited only a partial set of the information can be extracted.</info> diff --git a/python/libvirt-override-virStoragePool.py b/python/libvirt-override-virStoragePool.py new file mode 100644 index 0000000..ffe160c --- /dev/null +++ b/python/libvirt-override-virStoragePool.py
Hm, I'm no python binding expert. Does this file get used automaticaly?
@@ -0,0 +1,11 @@ + def listAllVolumes(self, flags): + """List all storage volumes and returns a list of storage volume objects""" + ret = libvirtmod.virStoragePoolListAllVolumes(self._o, flags) + if ret is None: + raise libvirtError("virStoragePoolListAllVolumes() failed", conn=self) + + retlist = list() + for volptr in ret: + retlist.append(virStorageVol(self, _obj=volptr)) + + return retlist diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 1e3ad89..8b402b0 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3083,6 +3083,55 @@ libvirt_virStoragePoolListVolumes(PyObject *self ATTRIBUTE_UNUSED, }
static PyObject * +libvirt_virStoragePoolListAllVolumes(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval = NULL; + PyObject *tmp = NULL; + virStoragePoolPtr pool; + virStorageVolPtr *vols = NULL; + int c_retval = 0; + int i; + unsigned int flags; + PyObject *pyobj_pool; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virStoragePoolListAllVolumes", + &pyobj_pool, &flags)) + return NULL; + + pool = (virStoragePoolPtr) PyvirStoragePool_Get(pyobj_pool); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virStoragePoolListAllVolumes(pool, &vols, 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_virStorageVolPtrWrap(vols[i])) || + PyList_SetItem(py_retval, i, tmp) < 0) { + Py_XDECREF(tmp); + Py_DECREF(py_retval); + py_retval = NULL; + goto cleanup; + } + /* python steals the pointer */ + vols[i] = NULL; + } + +cleanup: + for (i = 0; i < c_retval; i++) + if (vols[i]) + virStorageVolFree(vols[i]); + VIR_FREE(vols); + return py_retval; +} + + +static PyObject * libvirt_virStoragePoolGetAutostart(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; int c_retval, autostart; @@ -5927,6 +5976,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectListAllStoragePools", libvirt_virConnectListAllStoragePools, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetAutostart", libvirt_virStoragePoolGetAutostart, METH_VARARGS, NULL}, {(char *) "virStoragePoolListVolumes", libvirt_virStoragePoolListVolumes, METH_VARARGS, NULL}, + {(char *) "virStoragePoolListAllVolumes", libvirt_virStoragePoolListAllVolumes, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetInfo", libvirt_virStoragePoolGetInfo, METH_VARARGS, NULL}, {(char *) "virStorageVolGetInfo", libvirt_virStorageVolGetInfo, METH_VARARGS, NULL}, {(char *) "virStoragePoolGetUUID", libvirt_virStoragePoolGetUUID, METH_VARARGS, NULL},
ACK if the new python file is used automaticaly
Yes, it's used automaticaly, pushed the set with nits fixed. Thanks! Osier
participants (3)
-
Eric Blake
-
Osier Yang
-
Peter Krempa