[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]