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

[Daniel P. Berrange]

On Fri, Jul 20, 2012 at 09:00:21PM +0800, Osier Yang wrote:
> 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.

Filtering is done inside the storage driver, so I don't see why
this needs to be exposed in the public API.

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

I don't see any need for this.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|