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

Re: [libvirt] [PATCH] Support a new peer-to-peer migration mode & public API



On Fri, Sep 25, 2009 at 02:27:23PM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 22, 2009 at 02:43:20PM +0200, Daniel Veillard wrote:
> > [ Sending again due to mail output problems ]
> > 
> > On Thu, Sep 17, 2009 at 06:25:01PM +0100, Daniel P. Berrange wrote:
> > > Introduces several new public API options for migration
> > > 
> > >  - VIR_MIGRATE_PEER2PEER: With this flag the client only
> > >    invokes the virDomainMigratePerform method, expecting
> > >    the source host driver to do whatever is required to
> > >    complete the entire migration process.
> > >  - VIR_MIGRATE_TUNNELLED: With this flag the actual data
> > >    for migration will be tunnelled over the libvirtd RPC
> > >    channel. This requires that VIR_MIGRATE_PEER2PEER is
> > >    also set.
> > 
> >   Hum, I would rather add that when entering libvirt.c entry point
> > set explicitely VIR_MIGRATE_PEER2PEER if VIR_MIGRATE_TUNNELLED is asked
> > for, that's an internal impl. detail.
> 
> I didn't do that, because I wanted to leave open the option
> to supporting  TUNNELLED mode, without PEER2PEER mode. I
> don't really know what that would look like, but I think it
> is better future proofing to not presume one implies the other.

  Okay, let's go for this.

> > >  - virDomainMigrateToURI: This is variant of the existing
> > >    virDomainMigrate method which can be used when the
> > >    VIR_MIGRATE_PEER2PEER flag is set. The benefit of this
> > >    method is that no virConnectPtr object is required for
> > >    the destination host, only a destination URI.
> > 
> >   Sounds good if we have proper error handling. I'm also wondering
> > how authentification and access control can be handled in that mode,
> > the normal method of access somehow garantee that the user credentials
> > can be checked in some ways, but if it's direct daemon to daemon
> > talking how can this be performed ?
> 
> The client application authenticates with the source libvirtd
> in the normal manner. The source libvirtd will authenticate
> with the destination libvirtd directly.
> 
> The client application will not be authenticating with the
> destination libvirtd itself.
> 
> While this may sound like a security issue, I don't think it
> really is. It is more of an authorization problem - eg the
> admin needs to declare somewhere which libvirtd instances 
> are allowed to migrate guests between then. This authorization
> problem is really outside the scope of the client application
> or our public app API 

  Okay, as soon as you want to allow automatic migrations that
will be considered normal.

> > >  
> > > +int virDomainMigrateToURI (virDomainPtr domain, const char *duri,
> > > +                           unsigned long flags, const char *dname,
> > > +                           unsigned long bandwidth);
> > > +
> > 
> >   s/duri/dest/ or target, the fact it's a connection URI should be made
> >   obvious in the function comment.
> 
> Yep, the function comment later only clearly describes what URIs are
> acceptable. 
> 
> > [...]
> > > +    } else {
> > > +        if (flags & VIR_MIGRATE_TUNNELLED) {
> > > +            virLibConnError(domain->conn, VIR_ERR_OPERATION_INVALID,
> > > +                            _("cannot perform tunnelled migration without using peer2peer flag"));
> > > +            goto error;
> > > +        }
> > 
> >   I don't really agree, we should just or the flag and hide this from
> >   the API user.
> 
> I really did want to leave open the option of relaxing this restriction
> in the future, and thus didn't want to automatically set the PEER2PEER
> flag - once its set we can never unset it in the future.
> 
> > > +
> > > +
> > >  /*
> > >   * Not for public use.  This function is part of the internal
> > >   * implementation of migration in the remote case.
> > > diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
> > > index 5913798..5f1a7fe 100644
> > > --- a/src/libvirt_internal.h
> > > +++ b/src/libvirt_internal.h
> > > @@ -17,6 +17,7 @@
> > >   * License along with this library; if not, write to the Free Software
> > >   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> > >   *
> > > + * NB This file is (secret) ABI sensitive. Append only
> > 
> >    Huh ???
> 
> The methods here are encoded in the wire protocol ABI,
> but not included in the public libvirt.h header. Hence
> they are ABI sensitive, but secret & thus append only
> like the rest of our API
> 
> > > diff --git a/src/xen_unified.c b/src/xen_unified.c
> > > index dfa9ca5..954b187 100644
> > > --- a/src/xen_unified.c
> > > +++ b/src/xen_unified.c
> > > @@ -455,8 +455,11 @@ static int
> > >  xenUnifiedSupportsFeature (virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
> > >  {
> > >      switch (feature) {
> > > -    case VIR_DRV_FEATURE_MIGRATION_V1: return 1;
> > > -    default: return 0;
> > > +    case VIR_DRV_FEATURE_MIGRATION_V1:
> > > +    case VIR_DRV_FEATURE_MIGRATION_P2P:
> > > +        return 1;
> > > +    default:
> > > +        return 0;
> > >      }
> > >  }
> > >  
> > > diff --git a/src/xend_internal.c b/src/xend_internal.c
> > > index 7f55116..da5c039 100644
> > > --- a/src/xend_internal.c
> > > +++ b/src/xend_internal.c
> > > @@ -4400,6 +4400,12 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
> > >          strcpy (live, "1");
> > >          flags &= ~VIR_MIGRATE_LIVE;
> > >      }
> > > +    /* Trivially support this in Xen, since XenD on dest is always
> > > +     * ready to accept incoming migration */
> > > +    if ((flags & VIR_MIGRATE_PEER2PEER)) {
> > > +        flags &= ~VIR_MIGRATE_PEER2PEER;
> > > +    }
> > > +    /* XXX we could easily do tunnelled migration too if we want to */
> > >      if (flags != 0) {
> > >          virXendError (conn, VIR_ERR_NO_SUPPORT,
> > >                        "%s", _("xenDaemonDomainMigrate: unsupported flag"));
> > 
> >   Sounds and looks good to me, with the small caveat of the required
> > VIR_MIGRATE_PEER2PEER flag and making sure it would work later with
> > QEmu/KVM before commiting this.
> > 
> >   I must admit peer-2-peer left me a bit puzzled at the beginning
> > it wasn't obvious what kind of transfer it actually meant. Only de
> > the documentation clarified it but I have no better suggestion :-)
> > "direct" maybe ...
> 
> I called it peer-2-peer, because  the 2 libvirtd instances are talking
> to each other p2p, rather than having all communcation go via
> the client application.

  it's just that p2p raises the idea of a mesh of machines in my mind
and I'm probably not the only one :-) No problem !

 ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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