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).
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 ... Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
-- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list