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

Re: [Libvir] PATCH: 1/16: public API



On Tue, Feb 12, 2008 at 04:30:07AM +0000, Daniel P. Berrange wrote:
> This defines the public API for the storage pool and volume support.
> The naming of the APIs follows the example set by the network and
> domain APIs whereever the logical functions match. The main change
> since previous iterations is the addition of an explicit API for
> virStoragePoolBuild and virStoragePoolDelete, to allow formatting
> of the underlying storage, and permanent deletion. Many of the APIs
> also now have an 'unsigned int flags' param. It is also possible to
> lookup a volume directly based on its path or key without having the
> associated pool object ahead of time.

  
>  include/libvirt/libvirt.h    |  202 +++++++++++++++++++++++++++++++++++++++++++
>  include/libvirt/libvirt.h.in |  202 +++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_sym.version      |   45 +++++++++
>  3 files changed, 449 insertions(+)
> 
> diff -r d674e201bd98 include/libvirt/libvirt.h
> --- a/include/libvirt/libvirt.h	Tue Feb 05 12:24:51 2008 -0500
> +++ b/include/libvirt/libvirt.h	Tue Feb 05 13:28:02 2008 -0500
> @@ -859,6 +859,208 @@ int			virNetworkSetAutostart	(virNetwork
>  int			virNetworkSetAutostart	(virNetworkPtr network,
>  						 int autostart);
>  
> +
> +/**
> + * virStoragePool:
> + *
> + * a virStoragePool is a private structure representing a storage pool
> + */
> +typedef struct _virStoragePool virStoragePool;
> +
> +/**
> + * virStoragePoolPtr:
> + *
> + * a virStoragePoolPtr is pointer to a virStoragePool private structure, this is the
> + * type used to reference a storage pool in the API.
> + */
> +typedef virStoragePool *virStoragePoolPtr;
> +
> +
> +typedef enum {
> +  VIR_STORAGE_POOL_INACTIVE = 0, /* Not running */
> +  VIR_STORAGE_POOL_BUILDING = 1, /* Initializing pool, not available */
> +  VIR_STORAGE_POOL_RUNNING = 2,  /* Running normally */
> +  VIR_STORAGE_POOL_DEGRADED = 3, /* Running degraded */
> +} virStoragePoolState;
> +
> +
> +typedef enum {
> +  VIR_STORAGE_POOL_BUILD_NEW  = 0,   /* Regular build from scratch */
> +  VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */
> +  VIR_STORAGE_POOL_BUILD_EXTEND = 2  /* Extend existing pool */

  Is shrinking not possible ? things like compressing to fewer volumes

> +} virStoragePoolBuildFlags;
> +
> +typedef enum {
> +  VIR_STORAGE_POOL_DELETE_NORMAL = 0, /* Delete metadata only    (fast) */
> +  VIR_STORAGE_POOL_DELETE_CLEAR = 1,  /* Clear all data to zeros (slow) */

  I would name it VIR_STORAGE_POOL_DELETE_ZEROED , it's more obvious what
the operstion does.

> +} virStoragePoolDeleteFlags;
> +
> +typedef struct _virStoragePoolInfo virStoragePoolInfo;
> +
> +struct _virStoragePoolInfo {
> +  int state;                     /* virStoragePoolState flags */
> +  unsigned long long capacity;   /* Logical size bytes */
> +  unsigned long long allocation; /* Current allocation bytes */
> +  unsigned long long available;  /* Remaining free space bytes */
> +};
> +
> +typedef virStoragePoolInfo *virStoragePoolInfoPtr;
> +
> +
> +/**
> + * virStorageVol:
> + *
> + * a virStorageVol is a private structure representing a storage volume
> + */
> +typedef struct _virStorageVol virStorageVol;
> +
> +/**
> + * virStorageVolPtr:
> + *
> + * a virStorageVolPtr is pointer to a virStorageVol private structure, this is the
> + * type used to reference a storage volume in the API.
> + */
> +typedef virStorageVol *virStorageVolPtr;
> +
> +
> +typedef enum {
> +  VIR_STORAGE_VOL_FILE = 0,     /* Regular file based volumes */
> +  VIR_STORAGE_VOL_BLOCK = 1,    /* Block based volumes */
> +  VIR_STORAGE_VOL_VIRTUAL = 2,  /* Volumes which aren't assignable to guests */
> +} virStorageVolType;

  I wonder if it's worth making the distinction between virtual storage with
local state and those without, the second having really good properties for
example when deciding to do  a migration. But current enum is fine already.

> +typedef enum {
> +  VIR_STORAGE_VOL_DELETE_NORMAL = 0, /* Delete metadata only    (fast) */
> +  VIR_STORAGE_VOL_DELETE_CLEAR = 1,  /* Clear all data to zeros (slow) */
> +} virStorageVolDeleteFlags;

  I still prefer ZEROED as CLEAr but not a big deal.

> +typedef struct _virStorageVolInfo virStorageVolInfo;
> +
> +struct _virStorageVolInfo {
> +  int type;                      /* virStorageVolType flags */
> +  unsigned long long capacity;   /* Logical size bytes */
> +  unsigned long long allocation; /* Current allocation bytes */
> +};
> +
> +typedef virStorageVolInfo *virStorageVolInfoPtr;
> +
> +/*
> + * Get connection from pool.
> + */
> +virConnectPtr		virStoragePoolGetConnect	(virStoragePoolPtr pool);
> +
> +/*
> + * List active storage pools
> + */
> +int			virConnectNumOfStoragePools	(virConnectPtr conn);
> +int			virConnectListStoragePools	(virConnectPtr conn,
> +							 char **const names,
> +							 int maxnames);
> +
> +/*
> + * List inactive storage pools
> + */
> +int			virConnectNumOfDefinedStoragePools(virConnectPtr conn);
> +int			virConnectListDefinedStoragePools(virConnectPtr conn,
> +							  char **const names,
> +							  int maxnames);
> +
> +/*
> + * Query a host for storage pools of a particular type
> + */
> +int                     virConnectDiscoverStoragePools(virConnectPtr conn,
> +						       const char *hostname,
> +						       const char *type,
> +						       unsigned int flags,
> +						       char ***xmlDesc);
> +
> +/*
> + * Lookup pool by name or uuid
> + */
> +virStoragePoolPtr	virStoragePoolLookupByName	(virConnectPtr conn,
> +							 const char *name);
> +virStoragePoolPtr 	virStoragePoolLookupByUUID	(virConnectPtr conn,
> +							 const unsigned char *uuid);
> +virStoragePoolPtr	virStoragePoolLookupByUUIDString(virConnectPtr conn,
> +							 const char *uuid);
> +virStoragePoolPtr	virStoragePoolLookupByVolume	(virStorageVolPtr vol);
> +
> +/*
> + * Creating/destroying pools
> + */
> +virStoragePoolPtr	virStoragePoolCreateXML		(virConnectPtr conn,
> +							 const char *xmlDesc);
> +virStoragePoolPtr	virStoragePoolDefineXML		(virConnectPtr conn,
> +							 const char *xmlDesc);
> +int			virStoragePoolBuild		(virStoragePoolPtr pool,
> +							 unsigned int flags);
> +int			virStoragePoolUndefine		(virStoragePoolPtr pool);
> +int			virStoragePoolCreate		(virStoragePoolPtr pool);


  Can we use a flag here ? Is the operation synchronous or not ?
Or is that an instant operation and only Build() is likely to take some time.
I would add an unused flag for safety here.

> +int			virStoragePoolDestroy		(virStoragePoolPtr pool);
> +int			virStoragePoolDelete		(virStoragePoolPtr pool,
> +							 unsigned int flags);
> +int			virStoragePoolFree		(virStoragePoolPtr pool);
> +int			virStoragePoolRefresh		(virStoragePoolPtr pool,
> +							 unsigned int flags);
> +
> +/*
> + * StoragePool information
> + */
> +const char*		virStoragePoolGetName		(virStoragePoolPtr pool);
> +int			virStoragePoolGetUUID		(virStoragePoolPtr pool,
> +							 unsigned char *uuid);
> +int			virStoragePoolGetUUIDString	(virStoragePoolPtr pool,
> +							 char *buf);
> +
> +int			virStoragePoolGetInfo		(virStoragePoolPtr vol,
> +							 virStoragePoolInfoPtr info);
> +
> +char *			virStoragePoolGetXMLDesc	(virStoragePoolPtr pool,
> +							 unsigned int flags);
> +
> +int			virStoragePoolGetAutostart	(virStoragePoolPtr pool,
> +							 int *autostart);
> +int			virStoragePoolSetAutostart	(virStoragePoolPtr pool,
> +							 int autostart);
> +
> +/*
> + * List/lookup storage volumes within a pool
> + */
> +int			virStoragePoolNumOfVolumes	(virStoragePoolPtr pool);
> +int			virStoragePoolListVolumes	(virStoragePoolPtr pool,
> +							 char **const names,
> +							 int maxnames);
> +
> +virConnectPtr		virStorageVolGetConnect		(virStorageVolPtr vol);
> +
> +/*
> + * Lookup volumes based on various attributes
> + */
> +virStorageVolPtr        virStorageVolLookupByName	(virStoragePoolPtr pool,
> +							 const char *name);
> +virStorageVolPtr	virStorageVolLookupByKey	(virConnectPtr conn,
> +							 const char *key);
> +virStorageVolPtr	virStorageVolLookupByPath	(virConnectPtr conn,
> +							 const char *path);
> +
> +
> +const char*		virStorageVolGetName		(virStorageVolPtr vol);
> +const char*		virStorageVolGetKey		(virStorageVolPtr vol);
> +
> +virStorageVolPtr	virStorageVolCreateXML		(virStoragePoolPtr pool,
> +							 const char *xmldesc,
> +							 unsigned int flags);
> +int			virStorageVolDelete		(virStorageVolPtr vol,
> +							 unsigned int flags);
> +int			virStorageVolFree		(virStorageVolPtr vol);
> +
> +int			virStorageVolGetInfo		(virStorageVolPtr vol,
> +							 virStorageVolInfoPtr info);
> +char *			virStorageVolGetXMLDesc		(virStorageVolPtr pool,
> +							 unsigned int flags);
> +
> +char *			virStorageVolGetPath		(virStorageVolPtr vol);
> +
>  #ifdef __cplusplus
>  }
>  #endif

  Looks fine to me in general.

> --- a/src/libvirt_sym.version	Tue Feb 05 12:24:51 2008 -0500
> +++ b/src/libvirt_sym.version	Tue Feb 05 13:28:02 2008 -0500
> @@ -116,6 +116,49 @@
>  	virNetworkGetAutostart;
>  	virNetworkSetAutostart;
>  
> +        virStoragePoolGetConnect;
> +	virConnectNumOfStoragePools;
> +	virConnectNumOfDefinedStoragePools;
> +	virConnectListStoragePools;
> +	virConnectListDefinedStoragePools;
> +	virConnectDiscoverStoragePools;
> +	virStoragePoolLookupByName;
> +	virStoragePoolLookupByUUID;
> +	virStoragePoolLookupByUUIDString;
> +        virStoragePoolLookupByVolume;
> +	virStoragePoolCreateXML;
> +	virStoragePoolDefineXML;
> +	virStoragePoolUndefine;
> +	virStoragePoolCreate;
> +	virStoragePoolBuild;
> +	virStoragePoolDestroy;
> +	virStoragePoolDelete;
> +	virStoragePoolRefresh;
> +	virStoragePoolFree;
> +	virStoragePoolGetName;
> +	virStoragePoolGetUUID;
> +	virStoragePoolGetUUIDString;
> +	virStoragePoolGetInfo;
> +	virStoragePoolGetXMLDesc;
> +	virStoragePoolSetAutostart;
> +	virStoragePoolGetAutostart;
> +        virStoragePoolNumOfVolumes;
> +        virStoragePoolListVolumes;
> +
> +	virConnectNumOfStorageVolumes;
> +	virConnectListStorageVolumes;
> +	virStorageVolLookupByName;
> +	virStorageVolLookupByKey;
> +	virStorageVolLookupByPath;
> +	virStorageVolCreateXML;
> +	virStorageVolDelete;
> +	virStorageVolFree;
> +	virStorageVolGetName;
> +	virStorageVolGetKey;
> +	virStorageVolGetInfo;
> +	virStorageVolGetXMLDesc;
> +	virStorageVolGetPath;
> +
>          /* Symbols with __ are private only
>             for use by the libvirtd daemon.
>             They are not part of stable ABI
> @@ -133,6 +176,8 @@
>  
>  	__virGetDomain;
>  	__virGetNetwork;
> +	__virGetStoragePool;
> +	__virGetStorageVol;

  Okay, +1, 
I would love to see this or part of it commited before the next 
round, even if all entry points are present, as a temporary way
to decrease patches and separate the agreed upon from what would need 
to be refined. I guess it's not a problem until we make a new release.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


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