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 -=|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list