On Wed, Aug 01, 2007 at 02:45:57PM +0100, Richard W.M. Jones wrote:
Daniel Veillard wrote:
+typedef enum { + /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/ + * domainMigratePerform/domainMigrateFinish. + */ + VIR_DRV_FEATURE_MIGRATION_V1 = 1, + + /* Driver is not local. */ + VIR_DRV_FEATURE_REMOTE = 2, +} virDrvFeature;
Probably best done with defines than enums, as you want to be able to compose them the fact it comes from a type doesn't help, and I have learnt the hard way that enums sucks in C in general.
What in particular? I'm not particularly concerned either way since enums are basically just as unsafe as #defines in C, but it is nice for the library user to be able to connect the argument prototype ("virDrvFeature feature") to the list of permitted types.
+typedef int + (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature feature);
I would rather use , int features) where you OR the features, allows to know in one call if you get what you want or not.
In the case where you've got multiple different migration possibilities (VIR_DRV_FEATURE_MIGRATE_V1 & VIR_DRV_FEATURE_MIGRATE_V2) then this saves you one remote call in the legacy case (VIR_DRV_FEATURE_MIGRATE_V2 not supported so we have to do a second remote call to check VIR_DRV_FEATURE_MIGRATE_V1).
You seems to think it's useful only for migrate. I want that to be usable for a lot more. You talk to a remote server, you want to know if it supports any of the entry point in the driver table, how do you do this ?
On the other hand, it complicates the interface. You need to return an array rather than a single int. (OK, so you can return a bit array, but now the feature list had better always be <= 32 entries long). And in the case where someone queries VIR_DRV_FEATURE_MIGRATE_V1 & VIR_DRV_FEATURE_REMOTE you need to have two different drivers answering a single request.
I think this complicates things unnecessarily ...
I think this would be a waste to design it with a single narrowly focused usage in mind, when it can be far more generally useful. 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/