[Libvir] Proposal: Check availability of driver calls

Daniel P. Berrange wrote: Yesterday I wrote: "It's important to note that the internals may be changed later, although that may complicate things if we want to allow people to run incompatible versions of the daemons. [Another email on that subject is coming up after this one]." When I actually looked into it, it turned out to be trickier and less useful than I thought, so the promised email never came. At the moment the function src/libvirt.c: virDomainMigrate is a kind of "coordination function", in that it coordinates the steps necessary (ie. domainMigratePrepare -> domainMigratePerform -> etc.) One of the things which this coordination function does first is to check that the drivers support migration at all. The code is: /* Check that migration is supported. * Note that in the remote case these functions always exist. * But the remote driver will throw NO_SUPPORT errors later. */ if (!conn->driver->domainMigratePerform || !dconn->driver->domainMigratePrepare) { virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } As noted in the comment, this test is only functional in the local case. The remote driver registers functions for every driver entrypoint. So I began to think about having an "is_available" function. The test above would be rewritten: if (!is_available (conn->driver->domainMigratePerform) || !is_available (dconn->driver->domainMigratePrepare)) { ... } and the idea is that is_available expands to a straightforward non-NULL test for local drivers, but something more complicated if the driver is the remote driver. Implementing is_available turns out to be less than simple however. I had some ideas along these lines: #define is_available(drv,fn) \ (!(drv)->_available \ ? (drv)->(fn) \ : (drv)->_available((drv,offsetof(typeof(drv),(fn))))) but this is wrong in several ways: (1) relies on GCC typeof extension, (2) not easy to write the _available function to do the right thing across different word sizes or structure packing (which might happen at the two ends of the remote connection). What seems to be better is some sort of private driver capabilities function. We might use it like this: if (!conn->driver->supports_feature (VIR_DRV_MIGRATION_V1) || !dconn->driver->supports_feature (VIR_DRV_MIGRATION_V1)) { ... } (Note that this is a completely private interface). In the remote case, the supports_feature call is passed through to the end driver. Now if we decide that the current internal implementation of virDomainMigrate is bogus, we can add the extra driver calls for a new migration implementation, then support both old and new by changing virDomainMigrate coordination function as so: if (conn->driver->supports_feature (VIR_DRV_MIGRATION_V2) && dconn->driver->supports_feature (VIR_DRV_MIGRATION_V2)) { // shiny new implementation of migration // ... } else if (conn->driver->supports_feature (VIR_DRV_MIGRATION_V1) && dconn->driver->supports_feature (VIR_DRV_MIGRATION_V1)) { // bogus old implementation of migration // ... } else { // no compatible implementation virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); return NULL; } The above allows us to do migrations from old libvirtd -> old libvirtd, or from new libvirtd -> new libvirtd. Plausibly we could add the other cases as well, depending upon how the new migration implementation worked. So that is my proposal. No code, but obviously simple to implement. 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