Proposal: Check availability of driver calls (repost)

"Richard W.M. Jones" <rjones@xxxxxxxxxx> · Tue, 31 Jul 2007 12:54:03 +0100

--
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 ---

Subject: Proposal: Check availability of driver calls
From: "Richard W.M. Jones" <rjones@xxxxxxxxxx>
Date: Tue, 24 Jul 2007 12:53:37 +0100
Cc: libvir-list@xxxxxxxxxx
In-reply-to: <20070724001447.GC15217@xxxxxxxxxx>
References: <469E687D.7090300@xxxxxxxxxx> <20070720145823.GC7642@xxxxxxxxxx>	<46A47C35.6010104@xxxxxxxxxx> <20070724001447.GC15217@xxxxxxxxxx>
User-agent: Thunderbird 2.0.0.5 (X11/20070719)

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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

--- End Message ---
Attachment:
smime.p7s

Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list