Re: [Libvir] PATCH: 1/16: public API
On Thu, Feb 14, 2008 at 06:06:08AM -0500, Daniel Veillard wrote:
On Tue, Feb 12, 2008 at 04:30:07AM +0000, Daniel P. Berrange wrote:
> +
> +
> +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
I guess it could conceivably be supported. I should rename this to
be VIR_STORAGE_POOL_BUILD_RESIZE since the distinction between extending
and shrinking is not really relevant from the context of this API.
> +} 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.
Good idea.
> +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.
Sorry, there's a mistake here - the VIR_STORAGE_VOL_VIRTUAL should not be
there anymore. Since I removed the concept of 'singleton pools' it is no
longer neccessary - previously it would be used for a volume which pointed
to a LVM volume group - but this was just stupid, so i killed it.
> +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.
Yep, I like it too
> +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.
Definitely. I should have a flags paramater on all 'Create' methods since they
can
take non-trivial time - eg to login to the remote server, and we might wish to
control the behaviour via flags.
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.
That works with me - it is non-triival work keeping them in sync, even with
excellant quilt/mq tools.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|