Re: [Libvir] Re: [PATCH] Re: Proposal: Check availability of driver calls (repost)

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.