[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




--
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
--- Begin Message ---
Daniel P. Berrange wrote:
On Mon, Jul 23, 2007 at 11:00:21AM +0100, Richard W.M. Jones wrote:
step virDrvDomainMigrateFinish() might be needed, it could for example resume on the target side and also verify the domain is actually okay.
That could improve error handling and feels a lot more like a transactional
system where you really want an atomic work/fail operation and nothing else.
Yes. 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].

We need to treat the binary on-the-wire ABI for the client<->daemon the
same way as our public ELF library ABI. Never break it, only add to it.
So if we think the internals client<->server needs may change we should
avoid including this code in a release until we're more satisfied with
its longevity.

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

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

--
Libvir-list mailing list
Libvir-list redhat com
https://www.redhat.com/mailman/listinfo/libvir-list

--- End Message ---

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]