Re: Proposal: Check availability of driver calls (repost)

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

 



On Tue, Jul 31, 2007 at 12:54:03PM +0100, Richard W.M. Jones wrote:
> 


  In general I like the idea of a feature checking API, internally
it used to be one just needed to check the pointer in the driver table,
with remote that's not possible anymore. I'm not 100% sure we want to
expose this as a library API though, at least not in that way. And
yes I would really appreciate to have versionning in place before 
actually putting this in a release, this helps tremendously in the
long term.

  So general +1 except we may need to revisit the technical way to do
things.

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

> Date: Tue, 24 Jul 2007 12:53:37 +0100
> From: "Richard W.M. Jones" <rjones@xxxxxxxxxx>
> Subject:  Proposal: Check availability of driver calls
> To: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> Cc: libvir-list@xxxxxxxxxx
> 
> 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



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





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


-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard@xxxxxxxxxx  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]