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

[Osier Yang]

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.

The left are fine to review.

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

Thanks for pointing it out.

Regards,
Osier