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