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

[Osier Yang]

On 2012年07月20日 21:10, Osier Yang wrote:
> 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.

set-NACK for the idea, server can't have the poolGetInfoFlags if it
doesn't support listAllStoragePools. But any other idea if want to
have the "--cap" support?

Regards,
Osier