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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list