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

[Osier Yang]

On 2012年07月20日 21:01, Daniel P. Berrange wrote:
> 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.

Patch 12/50 will explain it: if the server side is old enough without
the new API listAllStoragePools, and virsh is new enough to have the
new introduced option "--type". I.e. new virsh talks to old libvirt,
It will need to get the pool type to filter the results in virsh layer.

Regards,
Osier