Re: [libvirt] [PATCH 02/50] list: Expose pool type via virStoragePoolGetInfo

On 2012年07月20日 20:44, Daniel P. Berrange wrote: >On Fri, Jul 20, 2012 at 02:28:26PM +0200, Jiri Denemark wrote: >>On Fri, Jul 20, 2012 at 00:40:33 +0800, Osier Yang wrote: >>>Mainly for later patches' use, to filter the pools by pool type. >>> >>>include/libvirt/libvirt.h.in: Add enum virStoragePoolType; Add >>>pool type to virStoragePoolInfo. >>> >>>src/conf/storage_conf.h: Remove enum virStoragePoolType. >>> >>>src/storage/storage_driver.c: Expose pool type via storagePoolGetInfo. >>>--- >>> include/libvirt/libvirt.h.in | 17 +++++++++++++++++ >>> src/conf/storage_conf.h | 19 ------------------- >>> src/storage/storage_driver.c | 2 ++ >>> 3 files changed, 19 insertions(+), 19 deletions(-) >>> >>>diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in >>>index e34438c..2820b2a 100644 >>>--- a/include/libvirt/libvirt.h.in >>>+++ b/include/libvirt/libvirt.h.in >>>@@ -2335,6 +2335,22 @@ typedef struct _virStoragePool virStoragePool; >>> */ >>> typedef virStoragePool *virStoragePoolPtr; >>> >>>+typedef enum { >>>+ VIR_STORAGE_POOL_DIR, /* Local directory */ >>>+ VIR_STORAGE_POOL_FS, /* Local filesystem */ >>>+ VIR_STORAGE_POOL_NETFS, /* Networked filesystem - eg NFS, GFS, etc */ >>>+ VIR_STORAGE_POOL_LOGICAL, /* Logical volume groups / volumes */ >>>+ VIR_STORAGE_POOL_DISK, /* Disk partitions */ >>>+ VIR_STORAGE_POOL_ISCSI, /* iSCSI targets */ >>>+ VIR_STORAGE_POOL_SCSI, /* SCSI HBA */ >>>+ VIR_STORAGE_POOL_MPATH, /* Multipath devices */ >>>+ VIR_STORAGE_POOL_RBD, /* RADOS Block Device */ >>>+ VIR_STORAGE_POOL_SHEEPDOG, /* Sheepdog device */ >>>+ >>>+#ifdef VIR_ENUM_SENTINELS >>>+ VIR_STORAGE_POOL_LAST, >>>+#endif >>>+} virStoragePoolType; >>> >>> typedef enum { >>> VIR_STORAGE_POOL_INACTIVE = 0, /* Not running */ >>>@@ -2365,6 +2381,7 @@ typedef enum { >>> typedef struct _virStoragePoolInfo virStoragePoolInfo; >>> >>> struct _virStoragePoolInfo { >>>+ int type; /* virStoragePoolType */ >>> int state; /* virStoragePoolState flags */ >>> unsigned long long capacity; /* Logical size bytes */ >>> unsigned long long allocation; /* Current allocation bytes */ >> >>Oops, you can't change public structs. Any combination of an app and libvirt >>library that would not have the same definition of this struct would fail. Oh, my bad. How about a new API like: int virStoragePoolGetInfoFlags (virStoragePoolPtr pool, virTypedParameterPtr params, int *nparams, unsigned int flags); With the param fields like: # define VIR_STORAGE_POOL_GET_INFO_STATE "state" # define VIR_STORAGE_POOL_GET_INFO_TYPE "type" # define VIR_STORAGE_POOL_GET_INFO_CAPACITY "capacity" # define VIR_STORAGE_POOL_GET_INFO_ALLOCATION "allocation" Assuming one wants to get more info about a pool in future, we would need a new API like this, with no suffering from not able to change to public struct. > >Fortunately no other part of this patch series appears to rely on this >extra field. Just remove this addition& the place in storage_driver.c >which sets it. The rest of this patch series can still be reviewed >as is The 'type' is used to filter the returned pool objects, so patches 1/50 to 14/50 should be skipped, though there is several patches in the range not related with storage pool specificly.

I will add a new API virStoragePoolGetInfoFlags if no disagreement, and rebase the storage pool patches as a v2.