[libvirt] [PATCH 0/9] Support new peer-2-peer migration mode & public API

This series is an update of http://www.redhat.com/archives/libvir-list/2009-September/msg00540.html There isn't as much functional change here as you might presume from the number of patches. Since Chris' tunnelled migration code is added, I thought it better to do alot of small refactoring steps rather than munge it all in one patch. One particular notable change since last time is that the new virDomainMigrateToURI method does *not* mandate use of the new "VIR_MIGRATE_PEER2PEER" flag anymore. I will now explain why... Traditionally migration has been a 3 step process 1. client -> dest (prepare) 2. client -> source (perform) 3. client -> dest (finish) This is why virDomainMigrate requires the extra 'dconn' parameter from the client app With the tunnelled migration / peer-to-peer migration, the extra 'dconn' parameter is no longer required from the client, because the source libvirt driver directly opens a connection to the destination libvirtd daemon on the remote host. ie libvirtd is talking to another libvirtd peer-2-peer. In considering the implementation of the PEER_TO_PEER flag for the Xen and VMWare drivers though I realized that even this is not technically neccessary, and in fact detrimental to use of libvirt for migrating with these drivers. Both Xen and VMWare have their own management daemon which is perfectly capable of handling the entire process without any need for libvirtd to be involved on the destination host. libvirt need only initiate the migration op on the source host. If you look at the impl of Prepare/Finish ops in the Xen driver this should be obvious, since they are pretty much no-ops Thus we in fact have 3 possible migration processes * Traditional libvirt client managed client -> dest (prepare) client -> source (perform) client -> dest (finish) * New peer-to-peer migration (on which tunnelled mig is built) client -> source (perform) source -> dest (open) source -> dest (prepareTunnel) source -> dest (streamSend) source -> dest (finish) source -> dest (close) * New direct migration (hypervisor managed) client -> source (perform) source -> dest (whatever the HV's native migration does) In terms of libvirt public APIs this works out as follows * Traditional libvirt client managed A hypervisor specific URI style, passed to virDomainMigrate() with no flags set * New peer-to-peer migration (on which tunnelled mig is built) The standard libvirt URI style, passed to virDomainMigrate() or virDomainMigrateToURI() with VIR_DOMAIN_MIGRATE_PEER2PEER flag set. The virDomainMigrateToURI() method is better since it avoids the unneccessary burden on the client of connecting to the remote libvirtd. * New direct migration (hypervisor managed) A hypervisor specific URI style, passed to virDomainMigrateToURI() with no flags set The QEMU driver can't support the latter, since it has no native management daemon - it requires libvirtd's help. Xen and VMWare, and probably VirtualBox can support this just fine. This is good for flexibility of usage in terms of authentication too, since each of these 3 modes has different characteristics * Traditional libvirt client managed client -> source libvirtd auth client -> dest libvirtd auth possible hypervisor -> hypervisor auth * New peer-to-peer migration, without tunnelling client -> source libvirtd auth source -> dest libvirtd auth possible hypervisor -> hypervisor auth * New peer-to-peer migration, without tunnelling client -> source libvirtd auth source -> dest libvirtd auth * New direct migration (hypervisor managed) client -> source libvirtd auth possible hypervisor -> hypervisor auth NB, 'client -> source libvirtd auth' is essentially no-op if initiating migration from the source host directly in all these cases. With with the combination of the two virDomainMigrate and virDomainMigrateToURI methods, and the VIR_MIGRATE_PEER2PEER flag, I believe we're well placed to cover all the main deployment/auth scenarios for migration in all hypervisors without imposing extra auth burden ourselves as we did in the past Further things that need to be done though: - Xen could easily support PEER2PEER and TUNNELLED flags - Add a VIR_MIGRATE_SECURE flag, to indicate that the app only wants the migration to be performed if the HV can guarentee the data channel is secure. - Document the scenarios I just outlined and give some mre guidance to app developers/administrators about the tradeoff between each scenario Regards, Daniel

Move the VIR_DRV_FEATURE* constants into libvirt_internal.h since these flags are indicating whether APIs in the libvirt_internal.h file are supported by a driver * src/driver.h: Remove VIR_DRV_FEATURE* constants * src/libvirt_internal.h: Add VIR_DRV_FEATURE* constants, using an enum instead of #define * src/internal.h: pull in libvirt_internal.h --- src/driver.h | 19 ------------------- src/internal.h | 2 ++ src/libvirt_internal.h | 27 +++++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/driver.h b/src/driver.h index c926614..2773a95 100644 --- a/src/driver.h +++ b/src/driver.h @@ -44,25 +44,6 @@ typedef enum { VIR_DRV_OPEN_ERROR = -2, } virDrvOpenStatus; -/* Feature detection. This is a libvirt-private interface for determining - * what features are supported by the driver. - * - * The remote driver passes features through to the real driver at the - * remote end unmodified, except if you query a VIR_DRV_FEATURE_REMOTE* - * feature. - */ - /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/ - * domainMigratePerform/domainMigrateFinish. - */ -#define VIR_DRV_FEATURE_MIGRATION_V1 1 - - /* Driver is not local. */ -#define VIR_DRV_FEATURE_REMOTE 2 - - /* Driver supports V2-style virDomainMigrate, ie. domainMigratePrepare2/ - * domainMigratePerform/domainMigrateFinish2. - */ -#define VIR_DRV_FEATURE_MIGRATION_V2 3 /* Internal feature-detection macro. Don't call drv->supports_feature * directly, because it may be NULL, use this macro instead. diff --git a/src/internal.h b/src/internal.h index 8fa579c..bd1cfe6 100644 --- a/src/internal.h +++ b/src/internal.h @@ -24,6 +24,8 @@ #include "libvirt/libvirt.h" #include "libvirt/virterror.h" +#include "libvirt_internal.h" + /* On architectures which lack these limits, define them (ie. Cygwin). * Note that the libvirt code should be robust enough to handle the * case where actual value is longer than these limits (eg. by setting diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 8f1ac3d..d3edcfa 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -17,6 +17,9 @@ * 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 ABI sensitive. Things here impact the wire + * protocol ABI in the remote driver. Same rules as for things + * include/libvirt/libvirt.h apply. ie this file is *append* only */ #ifndef __LIBVIRT_H_ @@ -31,6 +34,30 @@ int virStateReload(void); int virStateActive(void); #endif +/* Feature detection. This is a libvirt-private interface for determining + * what features are supported by the driver. + * + * The remote driver passes features through to the real driver at the + * remote end unmodified, except if you query a VIR_DRV_FEATURE_REMOTE* + * feature. + * + */ +enum { + /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/ + * domainMigratePerform/domainMigrateFinish. + */ + VIR_DRV_FEATURE_MIGRATION_V1 = 1, + + /* Driver is not local. */ + VIR_DRV_FEATURE_REMOTE = 2, + + /* Driver supports V2-style virDomainMigrate, ie. domainMigratePrepare2/ + * domainMigratePerform/domainMigrateFinish2. + */ + VIR_DRV_FEATURE_MIGRATION_V2 = 3, +}; + + int virDrvSupportsFeature (virConnectPtr conn, int feature); int virDomainMigratePrepare (virConnectPtr dconn, -- 1.6.2.5

On Mon, Oct 05, 2009 at 12:44:48PM +0100, Daniel P. Berrange wrote:
Move the VIR_DRV_FEATURE* constants into libvirt_internal.h since these flags are indicating whether APIs in the libvirt_internal.h file are supported by a driver
* src/driver.h: Remove VIR_DRV_FEATURE* constants * src/libvirt_internal.h: Add VIR_DRV_FEATURE* constants, using an enum instead of #define * src/internal.h: pull in libvirt_internal.h --- src/driver.h | 19 ------------------- src/internal.h | 2 ++ src/libvirt_internal.h | 27 +++++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/src/driver.h b/src/driver.h index c926614..2773a95 100644 --- a/src/driver.h +++ b/src/driver.h @@ -44,25 +44,6 @@ typedef enum { VIR_DRV_OPEN_ERROR = -2, } virDrvOpenStatus;
-/* Feature detection. This is a libvirt-private interface for determining - * what features are supported by the driver. - * - * The remote driver passes features through to the real driver at the - * remote end unmodified, except if you query a VIR_DRV_FEATURE_REMOTE* - * feature. - */ - /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/ - * domainMigratePerform/domainMigrateFinish. - */ -#define VIR_DRV_FEATURE_MIGRATION_V1 1 - - /* Driver is not local. */ -#define VIR_DRV_FEATURE_REMOTE 2 - - /* Driver supports V2-style virDomainMigrate, ie. domainMigratePrepare2/ - * domainMigratePerform/domainMigrateFinish2. - */ -#define VIR_DRV_FEATURE_MIGRATION_V2 3
/* Internal feature-detection macro. Don't call drv->supports_feature * directly, because it may be NULL, use this macro instead. diff --git a/src/internal.h b/src/internal.h index 8fa579c..bd1cfe6 100644 --- a/src/internal.h +++ b/src/internal.h @@ -24,6 +24,8 @@ #include "libvirt/libvirt.h" #include "libvirt/virterror.h"
+#include "libvirt_internal.h" + /* On architectures which lack these limits, define them (ie. Cygwin). * Note that the libvirt code should be robust enough to handle the * case where actual value is longer than these limits (eg. by setting diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 8f1ac3d..d3edcfa 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -17,6 +17,9 @@ * 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 ABI sensitive. Things here impact the wire + * protocol ABI in the remote driver. Same rules as for things + * include/libvirt/libvirt.h apply. ie this file is *append* only */
#ifndef __LIBVIRT_H_ @@ -31,6 +34,30 @@ int virStateReload(void); int virStateActive(void); #endif
+/* Feature detection. This is a libvirt-private interface for determining + * what features are supported by the driver. + * + * The remote driver passes features through to the real driver at the + * remote end unmodified, except if you query a VIR_DRV_FEATURE_REMOTE* + * feature. + * + */ +enum { + /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/ + * domainMigratePerform/domainMigrateFinish. + */ + VIR_DRV_FEATURE_MIGRATION_V1 = 1, + + /* Driver is not local. */ + VIR_DRV_FEATURE_REMOTE = 2, + + /* Driver supports V2-style virDomainMigrate, ie. domainMigratePrepare2/ + * domainMigratePerform/domainMigrateFinish2. + */ + VIR_DRV_FEATURE_MIGRATION_V2 = 3, +}; + + int virDrvSupportsFeature (virConnectPtr conn, int feature);
int virDomainMigratePrepare (virConnectPtr dconn,
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/

Since virMigratePrepareTunnel() is used for migration over the native libvirt connection, there is never any need to pass the target URI to this method. * daemon/remote.c, src/driver.h, src/libvirt.c, src/libvirt_internal.h, src/qemu/qemu_driver.c, src/remote/remote_driver.c, src/remote/remote_protocol.c, src/remote/remote_protocol.h, src/remote/remote_protocol.x: Remove 'uri_in' parameter from virMigratePrepareTunnel() method --- daemon/remote.c | 4 +--- src/driver.h | 1 - src/libvirt.c | 7 +++---- src/libvirt_internal.h | 9 ++++----- src/qemu/qemu_driver.c | 8 +------- src/remote/remote_driver.c | 2 -- src/remote/remote_protocol.c | 2 -- src/remote/remote_protocol.h | 1 - src/remote/remote_protocol.x | 1 - 9 files changed, 9 insertions(+), 26 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 6a7790e..4296fc3 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1503,12 +1503,10 @@ remoteDispatchDomainMigratePrepareTunnel(struct qemud_server *server ATTRIBUTE_U void *ret ATTRIBUTE_UNUSED) { int r; - char *uri_in; char *dname; struct qemud_client_stream *stream; CHECK_CONN (client); - uri_in = args->uri_in == NULL ? NULL : *args->uri_in; dname = args->dname == NULL ? NULL : *args->dname; stream = remoteCreateClientStream(conn, hdr); @@ -1517,7 +1515,7 @@ remoteDispatchDomainMigratePrepareTunnel(struct qemud_server *server ATTRIBUTE_U return -1; } - r = virDomainMigratePrepareTunnel(conn, stream->st, uri_in, + r = virDomainMigratePrepareTunnel(conn, stream->st, args->flags, dname, args->resource, args->dom_xml); if (r == -1) { diff --git a/src/driver.h b/src/driver.h index 2773a95..0c8f923 100644 --- a/src/driver.h +++ b/src/driver.h @@ -332,7 +332,6 @@ typedef int (*virDrvDomainMigratePrepareTunnel) (virConnectPtr conn, virStreamPtr st, - const char *uri_in, unsigned long flags, const char *dname, unsigned long resource, diff --git a/src/libvirt.c b/src/libvirt.c index 1ed4804..2839273 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3487,15 +3487,14 @@ error: int virDomainMigratePrepareTunnel(virConnectPtr conn, virStreamPtr st, - const char *uri_in, unsigned long flags, const char *dname, unsigned long bandwidth, const char *dom_xml) { - VIR_DEBUG("conn=%p, stream=%p, uri_in=%s, flags=%lu, dname=%s, " - "bandwidth=%lu, dom_xml=%s", conn, st, uri_in, flags, + VIR_DEBUG("conn=%p, stream=%p, flags=%lu, dname=%s, " + "bandwidth=%lu, dom_xml=%s", conn, st, flags, NULLSTR(dname), bandwidth, dom_xml); virResetLastError(); @@ -3516,7 +3515,7 @@ virDomainMigratePrepareTunnel(virConnectPtr conn, } if (conn->driver->domainMigratePrepareTunnel) { - int rv = conn->driver->domainMigratePrepareTunnel(conn, st, uri_in, + int rv = conn->driver->domainMigratePrepareTunnel(conn, st, flags, dname, bandwidth, dom_xml); if (rv < 0) diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index d3edcfa..d7cfd95 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -67,14 +67,14 @@ int virDomainMigratePrepare (virConnectPtr dconn, char **uri_out, unsigned long flags, const char *dname, - unsigned long bandwidth); + unsigned long resource); int virDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, - unsigned long bandwidth); + unsigned long resource); virDomainPtr virDomainMigrateFinish (virConnectPtr dconn, const char *dname, const char *cookie, @@ -88,7 +88,7 @@ int virDomainMigratePrepare2 (virConnectPtr dconn, char **uri_out, unsigned long flags, const char *dname, - unsigned long bandwidth, + unsigned long resource, const char *dom_xml); virDomainPtr virDomainMigrateFinish2 (virConnectPtr dconn, const char *dname, @@ -97,9 +97,8 @@ virDomainPtr virDomainMigrateFinish2 (virConnectPtr dconn, const char *uri, unsigned long flags, int retcode); -int virDomainMigratePrepareTunnel(virConnectPtr conn, +int virDomainMigratePrepareTunnel(virConnectPtr dconn, virStreamPtr st, - const char *uri_in, unsigned long flags, const char *dname, unsigned long resource, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 95e672b..1c5cb19 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6075,7 +6075,6 @@ static virStreamDriver qemuStreamMigDrv = { static int qemudDomainMigratePrepareTunnel(virConnectPtr dconn, virStreamPtr st, - const char *uri_in, unsigned long flags, const char *dname, unsigned long resource ATTRIBUTE_UNUSED, @@ -6098,11 +6097,6 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, "%s", _("no domain XML passed")); goto cleanup; } - if (!uri_in) { - qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("no URI passed")); - goto cleanup; - } if (!(flags & VIR_MIGRATE_TUNNELLED)) { qemudReportError(dconn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("PrepareTunnel called but no TUNNELLED flag set")); @@ -6459,7 +6453,7 @@ static int doTunnelMigrate(virDomainPtr dom, goto close_stream; } - internalret = dconn->driver->domainMigratePrepareTunnel(dconn, st, uri, + internalret = dconn->driver->domainMigratePrepareTunnel(dconn, st, flags, dname, resource, dom_xml); VIR_FREE(dom_xml); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 25aaf32..284593a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7053,7 +7053,6 @@ static virStreamDriver remoteStreamDrv = { static int remoteDomainMigratePrepareTunnel(virConnectPtr conn, virStreamPtr st, - const char *uri_in, unsigned long flags, const char *dname, unsigned long resource, @@ -7072,7 +7071,6 @@ remoteDomainMigratePrepareTunnel(virConnectPtr conn, st->driver = &remoteStreamDrv; st->privateData = privst; - args.uri_in = uri_in == NULL ? NULL : (char **) &uri_in; args.flags = flags; args.dname = dname == NULL ? NULL : (char **) &dname; args.resource = resource; diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 8c61712..1449ed4 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -2701,8 +2701,6 @@ bool_t xdr_remote_domain_migrate_prepare_tunnel_args (XDR *xdrs, remote_domain_migrate_prepare_tunnel_args *objp) { - if (!xdr_remote_string (xdrs, &objp->uri_in)) - return FALSE; if (!xdr_uint64_t (xdrs, &objp->flags)) return FALSE; if (!xdr_remote_string (xdrs, &objp->dname)) diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index 245f411..d87e8bc 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -1530,7 +1530,6 @@ struct remote_secret_lookup_by_usage_ret { typedef struct remote_secret_lookup_by_usage_ret remote_secret_lookup_by_usage_ret; struct remote_domain_migrate_prepare_tunnel_args { - remote_string uri_in; uint64_t flags; remote_string dname; uint64_t resource; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 537a838..2b3c03b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1356,7 +1356,6 @@ struct remote_secret_lookup_by_usage_ret { }; struct remote_domain_migrate_prepare_tunnel_args { - remote_string uri_in; unsigned hyper flags; remote_string dname; unsigned hyper resource; -- 1.6.2.5

On Mon, Oct 05, 2009 at 12:44:49PM +0100, Daniel P. Berrange wrote:
Since virMigratePrepareTunnel() is used for migration over the native libvirt connection, there is never any need to pass the target URI to this method.
Okay this was added after last release so never part of a release so we can still change the RPC protocol for this, 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/

The code for tunnelled migration wierdly required the app to pass a NULL 'dconn' parameter, only to have to use virConnectOpen itself shortly thereafter to get a 'dconn' object. Remove this bogus check & require the app to always pas 'dconn' as before * src/libvirt.c: Require 'dconn' for virDomainMigrate calls again and remove call to virConnectOpen --- src/libvirt.c | 79 +++++++++++++++----------------------------------------- 1 files changed, 21 insertions(+), 58 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 2839273..952b699 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3058,35 +3058,12 @@ virDomainMigrateVersion2 (virDomainPtr domain, */ static virDomainPtr virDomainMigrateTunnelled(virDomainPtr domain, + virConnectPtr dconn, unsigned long flags, const char *dname, const char *uri, unsigned long bandwidth) { - virConnectPtr dconn; - virDomainPtr ddomain = NULL; - - if (uri == NULL) { - /* if you are doing a secure migration, you *must* also pass a uri */ - virLibConnError(domain->conn, VIR_ERR_INVALID_ARG, - _("requested TUNNELLED migration, but no URI passed")); - return NULL; - } - - if (domain->conn->flags & VIR_CONNECT_RO) { - virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); - return NULL; - } - - /* FIXME: do we even need this check? In theory, V1 of the protocol - * should be able to do tunnelled migration as well - */ - if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, - VIR_DRV_FEATURE_MIGRATION_V2)) { - virLibConnError(domain->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); - return NULL; - } - /* Perform the migration. The driver isn't supposed to return * until the migration is complete. */ @@ -3094,18 +3071,7 @@ virDomainMigrateTunnelled(virDomainPtr domain, (domain, NULL, 0, uri, flags, dname, bandwidth) == -1) return NULL; - dconn = virConnectOpen(uri); - if (dconn == NULL) - /* FIXME: this is pretty crappy; as far as we know, the migration has - * now succeeded, but we can't connect back to the other side - */ - return NULL; - - ddomain = virDomainLookupByName(dconn, dname ? dname : domain->name); - - virConnectClose(dconn); - - return ddomain; + return virDomainLookupByName(dconn, dname ? dname : domain->name); } /** @@ -3182,32 +3148,29 @@ virDomainMigrate (virDomainPtr domain, goto error; } + /* Now checkout the destination */ + if (!VIR_IS_CONNECT(dconn)) { + virLibConnError(domain->conn, VIR_ERR_INVALID_CONN, __FUNCTION__); + goto error; + } + if (dconn->flags & VIR_CONNECT_RO) { + /* NB, deliberately report error against source object, not dest */ + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + if (flags & VIR_MIGRATE_TUNNELLED) { - /* tunnelled migration is more or less a completely different migration - * protocol. dconn has to be NULL, uri has to be set, and the flow - * of logic is completely different. Hence, here we split off from - * the main migration flow and use a separate function. - */ - if (dconn != NULL) { - virLibConnError(domain->conn, VIR_ERR_INVALID_ARG, - _("requested TUNNELLED migration, but non-NULL dconn")); - goto error; + char *dstURI = NULL; + if (uri == NULL) { + dstURI = virConnectGetURI(dconn); + if (!uri) + return NULL; } - ddomain = virDomainMigrateTunnelled(domain, flags, dname, uri, bandwidth); - } - else { - /* Now checkout the destination */ - if (!VIR_IS_CONNECT(dconn)) { - virLibConnError(domain->conn, VIR_ERR_INVALID_CONN, __FUNCTION__); - goto error; - } - if (dconn->flags & VIR_CONNECT_RO) { - /* NB, deliberately report error against source object, not dest */ - virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); - goto error; - } + ddomain = virDomainMigrateTunnelled(domain, dconn, flags, dname, uri ? uri : dstURI, bandwidth); + VIR_FREE(dstURI); + } else { /* Check that migration is supported by both drivers. */ if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_V1) && -- 1.6.2.5

The code for tunnelled migration was added in a dedicated method, but the native migration code is still inline in the top level qemudDomainMigratePerform() API. Move the native code out into a dedicated method too to make things more maintainable. * src/qemu/qemu_driver.c: Pull code for performing a native QEMU migration out into separate method --- src/qemu/qemu_driver.c | 123 ++++++++++++++++++++++++++++------------------- 1 files changed, 73 insertions(+), 50 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1c5cb19..dfbeb59 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6400,6 +6400,72 @@ cleanup: } + +/* Perform migration using QEMU's native TCP migrate support, + * not encrypted obviously + */ +static int doNativeMigrate(virDomainPtr dom, + virDomainObjPtr vm, + const char *uri, + unsigned long flags ATTRIBUTE_UNUSED, + const char *dname ATTRIBUTE_UNUSED, + unsigned long resource) +{ + int ret = -1; + xmlURIPtr uribits; + int status; + unsigned long long transferred, remaining, total; + + /* Issue the migrate command. */ + if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) { + /* HACK: source host generates bogus URIs, so fix them up */ + char *tmpuri; + if (virAsprintf(&tmpuri, "tcp://%s", uri + strlen("tcp:")) < 0) { + virReportOOMError(dom->conn); + goto cleanup; + } + uribits = xmlParseURI(tmpuri); + VIR_FREE(tmpuri); + } else { + uribits = xmlParseURI(uri); + } + if (!uribits) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot parse URI %s"), uri); + goto cleanup; + } + + if (resource > 0 && + qemuMonitorSetMigrationSpeed(vm, resource) < 0) + goto cleanup; + + if (qemuMonitorMigrateToHost(vm, 0, uribits->server, uribits->port) < 0) + goto cleanup; + + /* it is also possible that the migrate didn't fail initially, but + * rather failed later on. Check the output of "info migrate" + */ + if (qemuMonitorGetMigrationStatus(vm, &status, + &transferred, + &remaining, + &total) < 0) { + goto cleanup; + } + + if (status != QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { + qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("migrate did not successfully complete")); + goto cleanup; + } + + ret = 0; + +cleanup: + xmlFreeURI(uribits); + return ret; +} + + static int doTunnelMigrate(virDomainPtr dom, virDomainObjPtr vm, const char *uri, @@ -6545,6 +6611,8 @@ static int doTunnelMigrate(virDomainPtr dom, goto qemu_cancel_migration; } + + /* XXX should honour the 'resource' parameter here */ for (;;) { bytes = saferead(client_sock, buffer, sizeof(buffer)); if (bytes < 0) { @@ -6609,7 +6677,7 @@ qemudDomainMigratePerform (virDomainPtr dom, int cookielen ATTRIBUTE_UNUSED, const char *uri, unsigned long flags, - const char *dname ATTRIBUTE_UNUSED, + const char *dname, unsigned long resource) { struct qemud_driver *driver = dom->conn->privateData; @@ -6617,9 +6685,6 @@ qemudDomainMigratePerform (virDomainPtr dom, virDomainEventPtr event = NULL; int ret = -1; int paused = 0; - int status; - xmlURIPtr uribits = NULL; - unsigned long long transferred, remaining, total; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -6651,53 +6716,13 @@ qemudDomainMigratePerform (virDomainPtr dom, event = NULL; } - if (resource > 0 && - qemuMonitorSetMigrationSpeed(vm, resource) < 0) - goto cleanup; - - if (!(flags & VIR_MIGRATE_TUNNELLED)) { - /* Issue the migrate command. */ - if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) { - /* HACK: source host generates bogus URIs, so fix them up */ - char *tmpuri; - if (virAsprintf(&tmpuri, "tcp://%s", uri + strlen("tcp:")) < 0) { - virReportOOMError(dom->conn); - goto cleanup; - } - uribits = xmlParseURI(tmpuri); - VIR_FREE(tmpuri); - } else { - uribits = xmlParseURI(uri); - } - if (!uribits) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot parse URI %s"), uri); - goto cleanup; - } - - if (qemuMonitorMigrateToHost(vm, 0, uribits->server, uribits->port) < 0) - goto cleanup; - - /* it is also possible that the migrate didn't fail initially, but - * rather failed later on. Check the output of "info migrate" - */ - if (qemuMonitorGetMigrationStatus(vm, &status, - &transferred, - &remaining, - &total) < 0) { - goto cleanup; - } - - if (status != QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { - qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("migrate did not successfully complete")); - goto cleanup; - } - } - else { + if ((flags & VIR_MIGRATE_TUNNELLED)) { if (doTunnelMigrate(dom, vm, uri, flags, dname, resource) < 0) /* doTunnelMigrate already set the error, so just get out */ goto cleanup; + } else { + if (doNativeMigrate(dom, vm, uri, flags, dname, resource) < 0) + goto cleanup; } /* Clean up the source domain. */ @@ -6730,8 +6755,6 @@ cleanup: VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } - if (uribits) - xmlFreeURI(uribits); if (vm) virDomainObjUnlock(vm); if (event) -- 1.6.2.5

virStreamAbort is needed when the caller wishes to terminate the stream early, not when virStreamSend fails. * qemu/qemu_driver.c: Fix calling of virStreamAbort during tunnelled migration --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dfbeb59..561b0c8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6616,6 +6616,7 @@ static int doTunnelMigrate(virDomainPtr dom, for (;;) { bytes = saferead(client_sock, buffer, sizeof(buffer)); if (bytes < 0) { + virStreamAbort(st); virReportSystemError(dconn, errno, "%s", _("tunnelled migration failed to read from qemu")); goto close_client_sock; @@ -6627,7 +6628,6 @@ static int doTunnelMigrate(virDomainPtr dom, if (virStreamSend(st, buffer, bytes) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("Failed to write migration data to remote libvirtd")); - virStreamAbort(st); goto close_client_sock; } } -- 1.6.2.5

Simplify the doTunnelMigrate() method by pulling out the code which opens/closes the virConnectPtr object into a parent method * qemu/qemu_driver.c: Add doPeer2PeerMigrate which then calls doTunnelMigrate with dconn & dom_xml --- docs/libvirt-api.xml | 8 ++++- docs/libvirt-refs.xml | 18 +++++++++++ src/qemu/qemu_driver.c | 78 +++++++++++++++++++++++++++++++---------------- 3 files changed, 75 insertions(+), 29 deletions(-) diff --git a/docs/libvirt-api.xml b/docs/libvirt-api.xml index 88dc246..a32c397 100644 --- a/docs/libvirt-api.xml +++ b/docs/libvirt-api.xml @@ -47,6 +47,7 @@ <exports symbol='VIR_MIGRATE_LIVE' type='enum'/> <exports symbol='VIR_DOMAIN_EVENT_STOPPED_DESTROYED' type='enum'/> <exports symbol='VIR_DOMAIN_EVENT_DEFINED_ADDED' type='enum'/> + <exports symbol='VIR_VCPU_BLOCKED' type='enum'/> <exports symbol='VIR_SECRET_USAGE_TYPE_NONE' type='enum'/> <exports symbol='VIR_DOMAIN_EVENT_STARTED_MIGRATED' type='enum'/> <exports symbol='VIR_STREAM_EVENT_HANGUP' type='enum'/> @@ -82,7 +83,7 @@ <exports symbol='VIR_STREAM_EVENT_WRITABLE' type='enum'/> <exports symbol='VIR_DOMAIN_SCHED_FIELD_DOUBLE' type='enum'/> <exports symbol='VIR_DOMAIN_SCHED_FIELD_LLONG' type='enum'/> - <exports symbol='VIR_VCPU_BLOCKED' type='enum'/> + <exports symbol='VIR_MIGRATE_TUNNELLED' type='enum'/> <exports symbol='VIR_DOMAIN_SCHED_FIELD_BOOLEAN' type='enum'/> <exports symbol='VIR_DOMAIN_XML_INACTIVE' type='enum'/> <exports symbol='VIR_STORAGE_VOL_BLOCK' type='enum'/> @@ -718,7 +719,8 @@ <enum name='VIR_FROM_XML' file='virterror' value='5' type='virErrorDomain' info='Error in the XML code'/> <enum name='VIR_MEMORY_PHYSICAL' file='libvirt' value='2' type='virDomainMemoryFlags' info=' addresses are physical addresses'/> <enum name='VIR_MEMORY_VIRTUAL' file='libvirt' value='1' type='virDomainMemoryFlags' info='addresses are virtual addresses'/> - <enum name='VIR_MIGRATE_LIVE' file='libvirt' value='1' type='virDomainMigrateFlags' info=' live migration'/> + <enum name='VIR_MIGRATE_LIVE' file='libvirt' value='1' type='virDomainMigrateFlags' info='live migration'/> + <enum name='VIR_MIGRATE_TUNNELLED' file='libvirt' value='2' type='virDomainMigrateFlags' info=' tunnelled migration'/> <enum name='VIR_SECRET_USAGE_TYPE_NONE' file='libvirt' value='0' type='virSecretUsageType'/> <enum name='VIR_SECRET_USAGE_TYPE_VOLUME' file='libvirt' value='1' type='virSecretUsageType' info=' Expect more owner types later...'/> <enum name='VIR_STORAGE_POOL_BUILDING' file='libvirt' value='1' type='virStoragePoolState' info='Initializing pool, not available'/> @@ -1596,6 +1598,8 @@ host given by dconn (a connection to the destination host). Flags may be one of more of the following: VIR_MIGRATE_LIVE Attempt a live migration. + VIR_MIGRATE_TUNNELLED Attempt to do a migration tunnelled through the + libvirt RPC mechanism If a hypervisor supports renaming domains during migration, then you may set the dname parameter to the new name (otherwise diff --git a/docs/libvirt-refs.xml b/docs/libvirt-refs.xml index 08034e4..0352e77 100644 --- a/docs/libvirt-refs.xml +++ b/docs/libvirt-refs.xml @@ -154,6 +154,7 @@ <reference name='VIR_MEMORY_PHYSICAL' href='html/libvirt-libvirt.html#VIR_MEMORY_PHYSICAL'/> <reference name='VIR_MEMORY_VIRTUAL' href='html/libvirt-libvirt.html#VIR_MEMORY_VIRTUAL'/> <reference name='VIR_MIGRATE_LIVE' href='html/libvirt-libvirt.html#VIR_MIGRATE_LIVE'/> + <reference name='VIR_MIGRATE_TUNNELLED' href='html/libvirt-libvirt.html#VIR_MIGRATE_TUNNELLED'/> <reference name='VIR_NODEINFO_MAXCPUS' href='html/libvirt-libvirt.html#VIR_NODEINFO_MAXCPUS'/> <reference name='VIR_SECRET_USAGE_TYPE_NONE' href='html/libvirt-libvirt.html#VIR_SECRET_USAGE_TYPE_NONE'/> <reference name='VIR_SECRET_USAGE_TYPE_VOLUME' href='html/libvirt-libvirt.html#VIR_SECRET_USAGE_TYPE_VOLUME'/> @@ -659,6 +660,7 @@ <ref name='VIR_MEMORY_PHYSICAL'/> <ref name='VIR_MEMORY_VIRTUAL'/> <ref name='VIR_MIGRATE_LIVE'/> + <ref name='VIR_MIGRATE_TUNNELLED'/> <ref name='VIR_NODEINFO_MAXCPUS'/> <ref name='VIR_SECRET_USAGE_TYPE_NONE'/> <ref name='VIR_SECRET_USAGE_TYPE_VOLUME'/> @@ -1596,6 +1598,7 @@ <ref name='VIR_MEMORY_PHYSICAL'/> <ref name='VIR_MEMORY_VIRTUAL'/> <ref name='VIR_MIGRATE_LIVE'/> + <ref name='VIR_MIGRATE_TUNNELLED'/> <ref name='VIR_NODEINFO_MAXCPUS'/> <ref name='VIR_SECRET_USAGE_TYPE_NONE'/> <ref name='VIR_SECRET_USAGE_TYPE_VOLUME'/> @@ -2654,6 +2657,9 @@ <ref name='virNetworkGetUUIDString'/> <ref name='virSecretGetUUIDString'/> </word> + <word name='RPC'> + <ref name='virDomainMigrate'/> + </word> <word name='Re-attach'> <ref name='virNodeDeviceReAttach'/> </word> @@ -2912,6 +2918,9 @@ <word name='VIR_MIGRATE_LIVE'> <ref name='virDomainMigrate'/> </word> + <word name='VIR_MIGRATE_TUNNELLED'> + <ref name='virDomainMigrate'/> + </word> <word name='VIR_SECRET_USAGE_TYPE_VOLUME'> <ref name='virSecretGetUsageID'/> </word> @@ -5604,6 +5613,9 @@ <word name='means'> <ref name='virDomainPinVcpu'/> </word> + <word name='mechanism'> + <ref name='virDomainMigrate'/> + </word> <word name='mem'> <ref name='_virNodeInfo'/> </word> @@ -7381,6 +7393,9 @@ <ref name='virStoragePoolRef'/> <ref name='virStorageVolRef'/> </word> + <word name='through'> + <ref name='virDomainMigrate'/> + </word> <word name='time'> <ref name='_virDomainInfo'/> <ref name='_virVcpuInfo'/> @@ -7443,6 +7458,9 @@ <word name='trying'> <ref name='virConnectOpen'/> </word> + <word name='tunnelled'> + <ref name='virDomainMigrate'/> + </word> <word name='turn'> <ref name='virConnectOpen'/> </word> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 561b0c8..fe5a376 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6467,7 +6467,9 @@ cleanup: static int doTunnelMigrate(virDomainPtr dom, + virConnectPtr dconn, virDomainObjPtr vm, + const char *dom_xml, const char *uri, unsigned long flags, const char *dname, @@ -6477,13 +6479,11 @@ static int doTunnelMigrate(virDomainPtr dom, int client_sock, qemu_sock; struct sockaddr_un sa_qemu, sa_client; socklen_t addrlen; - virConnectPtr dconn; virDomainPtr ddomain; int retval = -1; ssize_t bytes; char buffer[65536]; virStreamPtr st; - char *dom_xml = NULL; char *unixfile = NULL; int internalret; unsigned int qemuCmdFlags; @@ -6494,35 +6494,15 @@ static int doTunnelMigrate(virDomainPtr dom, * destination side is completely setup before we touch the source */ - dconn = virConnectOpen(uri); - if (dconn == NULL) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - _("Failed to connect to remote libvirt URI %s"), uri); - return -1; - } - if (!VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, - VIR_DRV_FEATURE_MIGRATION_V2)) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", - _("Destination libvirt does not support required migration protocol 2")); - goto close_dconn; - } - st = virStreamNew(dconn, 0); if (st == NULL) /* virStreamNew only fails on OOM, and it reports the error itself */ - goto close_dconn; - - dom_xml = virDomainDefFormat(dom->conn, vm->def, VIR_DOMAIN_XML_SECURE); - if (!dom_xml) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("failed to get domain xml")); - goto close_stream; - } + goto cleanup; internalret = dconn->driver->domainMigratePrepareTunnel(dconn, st, flags, dname, resource, dom_xml); - VIR_FREE(dom_xml); + if (internalret < 0) /* domainMigratePrepareTunnel sets the error for us */ goto close_stream; @@ -6663,13 +6643,57 @@ close_stream: /* don't call virStreamFree(), because that resets any pending errors */ virUnrefStream(st); -close_dconn: +cleanup: + return retval; +} + + +static int doPeer2PeerMigrate(virDomainPtr dom, + virDomainObjPtr vm, + const char *uri, + unsigned long flags, + const char *dname, + unsigned long resource) +{ + int ret = -1; + virConnectPtr dconn = NULL; + char *dom_xml; + + /* the order of operations is important here; we make sure the + * destination side is completely setup before we touch the source + */ + + dconn = virConnectOpen(uri); + if (dconn == NULL) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("Failed to connect to remote libvirt URI %s"), uri); + return -1; + } + if (!VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, + VIR_DRV_FEATURE_MIGRATION_V2)) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", + _("Destination libvirt does not support required migration protocol 2")); + goto cleanup; + } + + dom_xml = virDomainDefFormat(dom->conn, vm->def, VIR_DOMAIN_XML_SECURE); + if (!dom_xml) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("failed to get domain xml")); + goto cleanup; + } + + ret = doTunnelMigrate(dom, dconn, vm, dom_xml, uri, flags, dname, resource); + +cleanup: + VIR_FREE(dom_xml); /* don't call virConnectClose(), because that resets any pending errors */ virUnrefConnect(dconn); - return retval; + return ret; } + /* Perform is the second step, and it runs on the source host. */ static int qemudDomainMigratePerform (virDomainPtr dom, @@ -6717,7 +6741,7 @@ qemudDomainMigratePerform (virDomainPtr dom, } if ((flags & VIR_MIGRATE_TUNNELLED)) { - if (doTunnelMigrate(dom, vm, uri, flags, dname, resource) < 0) + if (doPeer2PeerMigrate(dom, vm, uri, flags, dname, resource) < 0) /* doTunnelMigrate already set the error, so just get out */ goto cleanup; } else { -- 1.6.2.5

Simplify the doTunnelMigrate code by pulling out the code for sending all tunnelled data into separate helper * qemu/qemu_driver.c: introduce doTunnelSendAll() method --- src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++++++--------------------- 1 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fe5a376..eea85b3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6466,6 +6466,40 @@ cleanup: } +static int doTunnelSendAll(virDomainPtr dom, + virStreamPtr st, + int sock) +{ + char buffer[65536]; + int nbytes = sizeof(buffer); + + /* XXX should honour the 'resource' parameter here */ + for (;;) { + nbytes = saferead(sock, buffer, nbytes); + if (nbytes < 0) { + virStreamAbort(st); + virReportSystemError(dom->conn, errno, "%s", + _("tunnelled migration failed to read from qemu")); + return -1; + } + else if (nbytes == 0) + /* EOF; get out of here */ + break; + + if (virStreamSend(st, buffer, nbytes) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("Failed to write migration data to remote libvirtd")); + return -1; + } + } + + if (virStreamFinish(st) < 0) + /* virStreamFinish set the error for us */ + return -1; + + return 0; +} + static int doTunnelMigrate(virDomainPtr dom, virConnectPtr dconn, virDomainObjPtr vm, @@ -6482,7 +6516,6 @@ static int doTunnelMigrate(virDomainPtr dom, virDomainPtr ddomain; int retval = -1; ssize_t bytes; - char buffer[65536]; virStreamPtr st; char *unixfile = NULL; int internalret; @@ -6591,34 +6624,8 @@ static int doTunnelMigrate(virDomainPtr dom, goto qemu_cancel_migration; } + retval = doTunnelSendAll(dom, st, client_sock); - /* XXX should honour the 'resource' parameter here */ - for (;;) { - bytes = saferead(client_sock, buffer, sizeof(buffer)); - if (bytes < 0) { - virStreamAbort(st); - virReportSystemError(dconn, errno, "%s", - _("tunnelled migration failed to read from qemu")); - goto close_client_sock; - } - else if (bytes == 0) - /* EOF; get out of here */ - break; - - if (virStreamSend(st, buffer, bytes) < 0) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - _("Failed to write migration data to remote libvirtd")); - goto close_client_sock; - } - } - - if (virStreamFinish(st) < 0) - /* virStreamFinish set the error for us */ - goto close_client_sock; - - retval = 0; - -close_client_sock: close(client_sock); qemu_cancel_migration: -- 1.6.2.5

On 10/05/2009 01:44 PM, Daniel P. Berrange wrote:
+ char buffer[65536]; + int nbytes = sizeof(buffer);
I just noticed this, it's safer to malloc such a big array (it is actually from Chris patch, but you may take the opportunity to fix it). Paolo

On Mon, Oct 05, 2009 at 03:29:47PM +0200, Paolo Bonzini wrote:
On 10/05/2009 01:44 PM, Daniel P. Berrange wrote:
+ char buffer[65536]; + int nbytes = sizeof(buffer);
I just noticed this, it's safer to malloc such a big array (it is actually from Chris patch, but you may take the opportunity to fix it).
Yes, that's a good point. Someday we should also consider getting rid of every occurrance of char buf[PATH_MAX] since POSIX doesn't require PATH_MAX to be defined, and explicitly allows it to have a value so large that it could never be allocated on the stack or heap. Though its unlikely that we'd come across such an OS, its good to avoid potentially large stack arrays. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re-arrange the doTunnelMigrate method putting all non-QEMU local state setup steps first. This maximises chances of success before then starting destination QEMU for receiving incoming migration. Altogether this can reduce the number of goto cleanup labels to something more managable. * qemu/qemu_driver.c: Re-order steps in doTunnelMigrate --- src/qemu/qemu_driver.c | 119 +++++++++++++++++++++++++++++------------------- 1 files changed, 72 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eea85b3..a15f8ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6510,47 +6510,43 @@ static int doTunnelMigrate(virDomainPtr dom, unsigned long resource) { struct qemud_driver *driver = dom->conn->privateData; - int client_sock, qemu_sock; + int client_sock = -1; + int qemu_sock = -1; struct sockaddr_un sa_qemu, sa_client; socklen_t addrlen; - virDomainPtr ddomain; + virDomainPtr ddomain = NULL; int retval = -1; - ssize_t bytes; - virStreamPtr st; + virStreamPtr st = NULL; char *unixfile = NULL; int internalret; unsigned int qemuCmdFlags; int status; unsigned long long transferred, remaining, total; - /* the order of operations is important here; we make sure the - * destination side is completely setup before we touch the source + /* + * The order of operations is important here to avoid touching + * the source VM until we are very sure we can successfully + * start the migration operation. + * + * 1. setup local support infrastructure (eg sockets) + * 2. setup destination fully + * 3. start migration on source */ - st = virStreamNew(dconn, 0); - if (st == NULL) - /* virStreamNew only fails on OOM, and it reports the error itself */ - goto cleanup; - internalret = dconn->driver->domainMigratePrepareTunnel(dconn, st, - flags, dname, - resource, dom_xml); - - if (internalret < 0) - /* domainMigratePrepareTunnel sets the error for us */ - goto close_stream; + /* Stage 1. setup local support infrastructure */ if (virAsprintf(&unixfile, "%s/qemu.tunnelmigrate.src.%s", driver->stateDir, vm->def->name) < 0) { virReportOOMError(dom->conn); - goto finish_migrate; + goto cleanup; } qemu_sock = socket(AF_UNIX, SOCK_STREAM, 0); if (qemu_sock < 0) { virReportSystemError(dom->conn, errno, "%s", _("cannot open tunnelled migration socket")); - goto free_unix_path; + goto cleanup; } memset(&sa_qemu, 0, sizeof(sa_qemu)); sa_qemu.sun_family = AF_UNIX; @@ -6559,20 +6555,20 @@ static int doTunnelMigrate(virDomainPtr dom, qemudReportError(dom->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Unix socket '%s' too big for destination"), unixfile); - goto close_qemu_sock; + goto cleanup; } unlink(unixfile); if (bind(qemu_sock, (struct sockaddr *)&sa_qemu, sizeof(sa_qemu)) < 0) { virReportSystemError(dom->conn, errno, _("Cannot bind to unix socket '%s' for tunnelled migration"), unixfile); - goto close_qemu_sock; + goto cleanup; } if (listen(qemu_sock, 1) < 0) { virReportSystemError(dom->conn, errno, _("Cannot listen on unix socket '%s' for tunnelled migration"), unixfile); - goto close_qemu_sock; + goto cleanup; } /* check that this qemu version supports the unix migration */ @@ -6580,25 +6576,53 @@ static int doTunnelMigrate(virDomainPtr dom, qemudReportError(dom->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Cannot extract Qemu version from '%s'"), vm->def->emulator); - goto close_qemu_sock; + goto cleanup; + } + + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX) && + !(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) { + qemudReportError(dom->conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("Source qemu is too old to support tunnelled migration")); + goto cleanup; } + + + /* Stage 2. setup destination fully + * + * Once stage 2 has completed successfully, we *must* call finish + * to cleanup the target whether we succeed or fail + */ + st = virStreamNew(dconn, 0); + if (st == NULL) + /* virStreamNew only fails on OOM, and it reports the error itself */ + goto cleanup; + + internalret = dconn->driver->domainMigratePrepareTunnel(dconn, st, + flags, dname, + resource, dom_xml); + + if (internalret < 0) + /* domainMigratePrepareTunnel sets the error for us */ + goto cleanup; + + /* 3. start migration on source */ if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX) internalret = qemuMonitorMigrateToUnix(vm, 1, unixfile); else if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) { const char *args[] = { "nc", "-U", unixfile, NULL }; internalret = qemuMonitorMigrateToCommand(vm, 1, args, "/dev/null"); - } - else { - qemudReportError(dom->conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("Source qemu is too old to support tunnelled migration")); - goto close_qemu_sock; + } else { + internalret = -1; } if (internalret < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("tunnelled migration monitor command failed")); - goto close_qemu_sock; + goto finish; } + /* From this point onwards we *must* call cancel to abort the + * migration on source if anything goes wrong */ + /* it is also possible that the migrate didn't fail initially, but * rather failed later on. Check the output of "info migrate" */ @@ -6606,13 +6630,13 @@ static int doTunnelMigrate(virDomainPtr dom, &transferred, &remaining, &total) < 0) { - goto qemu_cancel_migration; + goto cancel; } if (status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s",_("migrate failed")); - goto qemu_cancel_migration; + goto cancel; } addrlen = sizeof(sa_client); @@ -6621,36 +6645,37 @@ static int doTunnelMigrate(virDomainPtr dom, continue; virReportSystemError(dom->conn, errno, "%s", _("tunnelled migration failed to accept from qemu")); - goto qemu_cancel_migration; + goto cancel; } retval = doTunnelSendAll(dom, st, client_sock); - close(client_sock); - -qemu_cancel_migration: +cancel: if (retval != 0) qemuMonitorMigrateCancel(vm); -close_qemu_sock: - close(qemu_sock); - -free_unix_path: - unlink(unixfile); - VIR_FREE(unixfile); - -finish_migrate: +finish: dname = dname ? dname : dom->name; ddomain = dconn->driver->domainMigrateFinish2 (dconn, dname, NULL, 0, uri, flags, retval); + +cleanup: + if (client_sock != -1) + close(client_sock); + if (qemu_sock != -1) + close(qemu_sock); + if (ddomain) virUnrefDomain(ddomain); -close_stream: - /* don't call virStreamFree(), because that resets any pending errors */ - virUnrefStream(st); + if (unixfile) { + unlink(unixfile); + VIR_FREE(unixfile); + } -cleanup: + if (st) + /* don't call virStreamFree(), because that resets any pending errors */ + virUnrefStream(st); return retval; } -- 1.6.2.5

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. - virDomainMigrateToURI: This is variant of the existing virDomainMigrate method which does not require any virConnectPtr for the destination host. Given suitable driver support, this allows for all the same modes as virDomainMigrate() The URI for VIR_MIGRATE_PEER2PEER must be a valid libvirt URI. For non-p2p migration a hypervisor specific migration URI is used. virDomainMigrateToURI without a PEER2PEER flag is only support for Xen currently, and it involves XenD talking directly to XenD, no libvirtd involved at all. * include/libvirt/libvirt.h.in: Add VIR_MIGRATE_PEER2PEER flag for migration * src/libvirt_internal.h: Add feature flags for peer to peer migration (VIR_FEATURE_MIGRATE_P2P) and direct migration (VIR_MIGRATE_PEER2PEER mode) * src/libvirt.c: Implement support for VIR_MIGRATE_PEER2PEER and virDomainMigrateToURI APIs. * src/xen/xen_driver.c: Advertise support for DIRECT migration * src/xen/xend_internal.c: Add TODO item for p2p migration * src/libvirt_public.syms: Export virDomainMigrateToURI method * src/qemu/qemu_driver.c: Add support for PEER2PEER and migration, and adapt TUNNELLED migration. * tools/virsh.c: Add --p2p and --direct args and use the new virDomainMigrateToURI method where possible. --- include/libvirt/libvirt.h.in | 9 +- src/libvirt.c | 274 +++++++++++++++++++++++++++++++++++------- src/libvirt_internal.h | 11 ++ src/libvirt_public.syms | 1 + src/qemu/qemu_driver.c | 69 ++++++++++- src/xen/xen_driver.c | 7 +- src/xen/xend_internal.c | 2 + tools/virsh.c | 57 +++++----- 8 files changed, 346 insertions(+), 84 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 60be41b..f51a565 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -336,8 +336,9 @@ typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr; /* Domain migration flags. */ typedef enum { - VIR_MIGRATE_LIVE = 1, /* live migration */ - VIR_MIGRATE_TUNNELLED = 2, /* tunnelled migration */ + VIR_MIGRATE_LIVE = (1 << 0), /* live migration */ + VIR_MIGRATE_PEER2PEER = (1 << 1), /* direct source -> dest host control channel */ + VIR_MIGRATE_TUNNELLED = (1 << 2), /* tunnel migration data over libvirtd connection */ } virDomainMigrateFlags; /* Domain migration. */ @@ -345,6 +346,10 @@ virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, unsigned long flags, const char *dname, const char *uri, unsigned long bandwidth); +int virDomainMigrateToURI (virDomainPtr domain, const char *duri, + unsigned long flags, const char *dname, + unsigned long bandwidth); + /** * VIR_NODEINFO_MAXCPUS: * @nodeinfo: virNodeInfo instance diff --git a/src/libvirt.c b/src/libvirt.c index 952b699..b14942d 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -35,7 +35,6 @@ #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" -#include "libvirt_internal.h" #include "driver.h" #include "uuid.h" @@ -3044,36 +3043,75 @@ virDomainMigrateVersion2 (virDomainPtr domain, return ddomain; } + + /* + * This is sort of a migration v3 + * + * In this version, the client does not talk to the destination + * libvirtd. The source libvirtd will still try to talk to the + * destination libvirtd though, and will do the prepare/perform/finish + * steps. + */ +static int +virDomainMigratePeer2Peer (virDomainPtr domain, + unsigned long flags, + const char *dname, + const char *uri, + unsigned long bandwidth) +{ + if (!domain->conn->driver->domainMigratePerform) { + virLibConnError (domain->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; + } + + /* Perform the migration. The driver isn't supposed to return + * until the migration is complete. + */ + return domain->conn->driver->domainMigratePerform(domain, + NULL, /* cookie */ + 0, /* cookielen */ + uri, + flags, + dname, + bandwidth); +} + + /* - * Tunnelled migration has the following flow: + * This is a variation on v1 & 2 migration + * + * This is for hypervisors which can directly handshake + * without any libvirtd involvement on destination either + * from client, or source libvirt. * - * virDomainMigrate(src, uri) - * - virDomainMigratePerform(src, uri) - * - dst = virConnectOpen(uri) - * - virDomainMigratePrepareTunnel(dst) - * - while (1) - * - virStreamSend(dst, data) - * - virDomainMigrateFinish(dst) - * - virConnectClose(dst) + * eg, XenD can talk direct to XenD, so libvirtd on dest + * does not need to be involved at all, or even running */ -static virDomainPtr -virDomainMigrateTunnelled(virDomainPtr domain, - virConnectPtr dconn, - unsigned long flags, - const char *dname, - const char *uri, - unsigned long bandwidth) -{ +static int +virDomainMigrateDirect (virDomainPtr domain, + unsigned long flags, + const char *dname, + const char *uri, + unsigned long bandwidth) +{ + if (!domain->conn->driver->domainMigratePerform) { + virLibConnError (domain->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + return -1; + } + /* Perform the migration. The driver isn't supposed to return * until the migration is complete. */ - if (domain->conn->driver->domainMigratePerform - (domain, NULL, 0, uri, flags, dname, bandwidth) == -1) - return NULL; - - return virDomainLookupByName(dconn, dname ? dname : domain->name); + return domain->conn->driver->domainMigratePerform(domain, + NULL, /* cookie */ + 0, /* cookielen */ + uri, + flags, + dname, + bandwidth); } + /** * virDomainMigrate: * @domain: a domain object @@ -3087,24 +3125,34 @@ virDomainMigrateTunnelled(virDomainPtr domain, * host given by dconn (a connection to the destination host). * * Flags may be one of more of the following: - * VIR_MIGRATE_LIVE Attempt a live migration. - * VIR_MIGRATE_TUNNELLED Attempt to do a migration tunnelled through the - * libvirt RPC mechanism + * VIR_MIGRATE_LIVE Do not pause the VM during migration + * VIR_MIGRATE_PEER2PEER Direct connection between source & destination hosts + * VIR_MIGRATE_TUNNELLED Tunnel migration data over the libvirt RPC channel + * + * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. + * Applications using the VIR_MIGRATE_PEER2PEER flag will probably + * prefer to invoke virDomainMigrateToURI, avoiding the need to + * open connection to the destination host themselves. * * If a hypervisor supports renaming domains during migration, * then you may set the dname parameter to the new name (otherwise * it keeps the same name). If this is not supported by the * hypervisor, dname must be NULL or else you will get an error. * - * Since typically the two hypervisors connect directly to each - * other in order to perform the migration, you may need to specify - * a path from the source to the destination. This is the purpose - * of the uri parameter. If uri is NULL, then libvirt will try to - * find the best method. Uri may specify the hostname or IP address - * of the destination host as seen from the source. Or uri may be - * a URI giving transport, hostname, user, port, etc. in the usual - * form. Refer to driver documentation for the particular URIs - * supported. + * If the VIR_MIGRATE_PEER2PEER flag is set, the uri parameter + * must be a valid libvirt connection URI, by which the source + * libvirt driver can connect to the destination libvirt. If + * omitted, the dconn connection object will be queried for its + * current URI. + * + * If the VIR_MIGRATE_PEER2PEER flag is NOT set, the URI parameter + * takes a hypervisor specific format. The hypervisor capabilities + * XML includes details of the support URI schemes. If omitted + * the dconn will be asked for a default URI. + * + * In either case it is typically only neccessary to specify a + * URI if the destination host has multiple interfaces and a + * specific interface is required to transmit migration data. * * The maximum bandwidth (in Mbps) that will be used to do migration * can be specified with the bandwidth parameter. If set to 0, @@ -3159,18 +3207,35 @@ virDomainMigrate (virDomainPtr domain, goto error; } - if (flags & VIR_MIGRATE_TUNNELLED) { - char *dstURI = NULL; - if (uri == NULL) { - dstURI = virConnectGetURI(dconn); - if (!uri) - return NULL; - } + if (flags & VIR_MIGRATE_PEER2PEER) { + if (VIR_DRV_SUPPORTS_FEATURE (domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_MIGRATION_P2P)) { + char *dstURI = NULL; + if (uri == NULL) { + dstURI = virConnectGetURI(dconn); + if (!uri) + return NULL; + } - ddomain = virDomainMigrateTunnelled(domain, dconn, flags, dname, uri ? uri : dstURI, bandwidth); + if (virDomainMigratePeer2Peer(domain, flags, dname, uri ? uri : dstURI, bandwidth) < 0) { + VIR_FREE(dstURI); + goto error; + } + VIR_FREE(dstURI); - VIR_FREE(dstURI); + ddomain = virDomainLookupByName (dconn, dname ? dname : domain->name); + } else { + /* This driver does not support peer to peer migration */ + virLibConnError (domain->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + goto error; + } } else { + if (flags & VIR_MIGRATE_TUNNELLED) { + virLibConnError(domain->conn, VIR_ERR_OPERATION_INVALID, + _("cannot perform tunnelled migration without using peer2peer flag")); + goto error; + } + /* Check that migration is supported by both drivers. */ if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, VIR_DRV_FEATURE_MIGRATION_V1) && @@ -3183,13 +3248,14 @@ virDomainMigrate (virDomainPtr domain, VIR_DRV_FEATURE_MIGRATION_V2)) ddomain = virDomainMigrateVersion2(domain, dconn, flags, dname, uri, bandwidth); else { + /* This driver does not support any migration method */ virLibConnError(domain->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); goto error; } } - if (ddomain == NULL) - goto error; + if (ddomain == NULL) + goto error; return ddomain; @@ -3199,6 +3265,122 @@ error: return NULL; } + +/** + * virDomainMigrateToURI: + * @domain: a domain object + * @duri: mandatory URI for the destination host + * @flags: flags + * @dname: (optional) rename domain to this at destination + * @bandwidth: (optional) specify migration bandwidth limit in Mbps + * + * Migrate the domain object from its current host to the destination + * host given by dconn (a connection to the destination host). + * + * Flags may be one of more of the following: + * VIR_MIGRATE_LIVE Do not pause the VM during migration + * VIR_MIGRATE_PEER2PEER Direct connection between source & destination hosts + * VIR_MIGRATE_TUNNELLED Tunnel migration data over the libvirt RPC channel + * + * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. + * Applications using the VIR_MIGRATE_PEER2PEER flag will probably + * prefer to invoke virDomainMigrateToURI, avoiding the need to + * open connection to the destination host themselves. + * + * If a hypervisor supports renaming domains during migration, + * then you may set the dname parameter to the new name (otherwise + * it keeps the same name). If this is not supported by the + * hypervisor, dname must be NULL or else you will get an error. + * + * If the VIR_MIGRATE_PEER2PEER flag is set, the duri parameter + * must be a valid libvirt connection URI, by which the source + * libvirt driver can connect to the destination libvirt. If + * omitted, the dconn connection object will be queried for its + * current URI. + * + * If the VIR_MIGRATE_PEER2PEER flag is NOT set, the duri parameter + * takes a hypervisor specific format. The hypervisor capabilities + * XML includes details of the support URI schemes. If omitted + * the dconn will be asked for a default URI. Not all hypervisors + * will support this mode of migration, so if the VIR_MIGRATE_PEER2PEER + * flag is not set, then it may be neccessary to use the alternative + * virDomainMigrate API providing an explicit virConnectPtr for the + * destination host + * + * The maximum bandwidth (in Mbps) that will be used to do migration + * can be specified with the bandwidth parameter. If set to 0, + * libvirt will choose a suitable default. Some hypervisors do + * not support this feature and will return an error if bandwidth + * is not 0. + * + * To see which features are supported by the current hypervisor, + * see virConnectGetCapabilities, /capabilities/host/migration_features. + * + * There are many limitations on migration imposed by the underlying + * technology - for example it may not be possible to migrate between + * different processors even with the same architecture, or between + * different types of hypervisor. + * + * Returns 0 if the migration succeeded, -1 upon error. + */ +int +virDomainMigrateToURI (virDomainPtr domain, + const char *duri, + unsigned long flags, + const char *dname, + unsigned long bandwidth) +{ + DEBUG("domain=%p, duri=%p, flags=%lu, dname=%s, bandwidth=%lu", + domain, NULLSTR(duri), flags, NULLSTR(dname), bandwidth); + + virResetLastError(); + + /* First checkout the source */ + if (!VIR_IS_CONNECTED_DOMAIN (domain)) { + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + return -1; + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (duri == NULL) { + virLibConnError (domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (flags & VIR_MIGRATE_PEER2PEER) { + if (VIR_DRV_SUPPORTS_FEATURE (domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_MIGRATION_P2P)) { + if (virDomainMigratePeer2Peer (domain, flags, dname, duri, bandwidth) < 0) + goto error; + } else { + /* No peer to peer migration supported */ + virLibConnError (domain->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + goto error; + } + } else { + if (VIR_DRV_SUPPORTS_FEATURE (domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_MIGRATION_DIRECT)) { + if (virDomainMigrateDirect (domain, flags, dname, duri, bandwidth) < 0) + goto error; + } else { + /* Cannot do a migration with only the perform step */ + virLibConnError (domain->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + goto error; + } + } + + return 0; + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(domain->conn); + return -1; +} + + /* * 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 d7cfd95..2ee09e9 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -55,6 +55,17 @@ enum { * domainMigratePerform/domainMigrateFinish2. */ VIR_DRV_FEATURE_MIGRATION_V2 = 3, + + /* Driver supports peer-2-peer virDomainMigrate ie source host + * does all the prepare/perform/finish steps directly + */ + VIR_DRV_FEATURE_MIGRATION_P2P = 4, + + /* Driver supports migration with only the source host involved, + * no libvirtd connetions on the destination at all, only the + * perform step is used. + */ + VIR_DRV_FEATURE_MIGRATION_DIRECT = 5, }; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 7226e88..8921c1a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -326,6 +326,7 @@ LIBVIRT_0.7.2 { virStreamFinish; virStreamAbort; virStreamFree; + virDomainMigrateToURI; } LIBVIRT_0.7.1; # .... define new API here using predicted next version number .... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a15f8ed..e876894 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2382,8 +2382,11 @@ static int qemudSupportsFeature (virConnectPtr conn ATTRIBUTE_UNUSED, int feature) { switch (feature) { - case VIR_DRV_FEATURE_MIGRATION_V2: return 1; - default: return 0; + case VIR_DRV_FEATURE_MIGRATION_V2: + case VIR_DRV_FEATURE_MIGRATION_P2P: + return 1; + default: + return 0; } } @@ -6412,7 +6415,7 @@ static int doNativeMigrate(virDomainPtr dom, unsigned long resource) { int ret = -1; - xmlURIPtr uribits; + xmlURIPtr uribits = NULL; int status; unsigned long long transferred, remaining, total; @@ -6680,6 +6683,57 @@ cleanup: } +/* This is essentially a simplified re-impl of + * virDomainMigrateVersion2 from libvirt.c, but running in source + * libvirtd context, instead of client app context */ +static int doNonTunnelMigrate(virDomainPtr dom, + virConnectPtr dconn, + virDomainObjPtr vm, + const char *dom_xml, + const char *uri ATTRIBUTE_UNUSED, + unsigned long flags, + const char *dname, + unsigned long resource) +{ + virDomainPtr ddomain = NULL; + int retval = -1; + char *uri_out = NULL; + + /* NB we don't pass 'uri' into this, since that's the libvirtd + * URI in this context - so we let dest pick it */ + if (dconn->driver->domainMigratePrepare2(dconn, + NULL, /* cookie */ + 0, /* cookielen */ + NULL, /* uri */ + &uri_out, + flags, dname, + resource, dom_xml) < 0) + /* domainMigratePrepare2 sets the error for us */ + goto cleanup; + + if (uri_out == NULL) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("domainMigratePrepare2 did not set uri")); + } + + if (doNativeMigrate(dom, vm, uri_out, flags, dname, resource) < 0) + goto finish; + + retval = 0; + +finish: + dname = dname ? dname : dom->name; + ddomain = dconn->driver->domainMigrateFinish2 + (dconn, dname, NULL, 0, uri_out, flags, retval); + + if (ddomain) + virUnrefDomain(ddomain); + +cleanup: + return retval; +} + + static int doPeer2PeerMigrate(virDomainPtr dom, virDomainObjPtr vm, const char *uri, @@ -6702,9 +6756,9 @@ static int doPeer2PeerMigrate(virDomainPtr dom, return -1; } if (!VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, - VIR_DRV_FEATURE_MIGRATION_V2)) { + VIR_DRV_FEATURE_MIGRATION_P2P)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", - _("Destination libvirt does not support required migration protocol 2")); + _("Destination libvirt does not support peer-to-peer migration protocol")); goto cleanup; } @@ -6715,7 +6769,10 @@ static int doPeer2PeerMigrate(virDomainPtr dom, goto cleanup; } - ret = doTunnelMigrate(dom, dconn, vm, dom_xml, uri, flags, dname, resource); + if (flags & VIR_MIGRATE_TUNNELLED) + ret = doTunnelMigrate(dom, dconn, vm, dom_xml, uri, flags, dname, resource); + else + ret = doNonTunnelMigrate(dom, dconn, vm, dom_xml, uri, flags, dname, resource); cleanup: VIR_FREE(dom_xml); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 76b896a..5273a11 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.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_DIRECT: + return 1; + default: + return 0; } } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 49bdba9..27d215e 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -4409,6 +4409,8 @@ xenDaemonDomainMigratePerform (virDomainPtr domain, strcpy (live, "1"); flags &= ~VIR_MIGRATE_LIVE; } + /* XXX we could easily do tunnelled & peer2peer migration too + if we want to. support these... */ if (flags != 0) { virXendError (conn, VIR_ERR_NO_SUPPORT, "%s", _("xenDaemonDomainMigrate: unsupported flag")); diff --git a/tools/virsh.c b/tools/virsh.c index 2222269..46c5454 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2462,6 +2462,8 @@ static const vshCmdInfo info_migrate[] = { static const vshCmdOptDef opts_migrate[] = { {"live", VSH_OT_BOOL, 0, gettext_noop("live migration")}, + {"p2p", VSH_OT_BOOL, 0, gettext_noop("peer-2-peer migration")}, + {"direct", VSH_OT_BOOL, 0, gettext_noop("direct migration")}, {"tunnelled", VSH_OT_BOOL, 0, gettext_noop("tunnelled migration")}, {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, {"desturi", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("connection URI of the destination host")}, @@ -2478,8 +2480,6 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) const char *migrateuri; const char *dname; int flags = 0, found, ret = FALSE; - virConnectPtr dconn = NULL; - virDomainPtr ddom = NULL; if (!vshConnectionUsability (ctl, ctl->conn, TRUE)) return FALSE; @@ -2499,40 +2499,41 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool (cmd, "live")) flags |= VIR_MIGRATE_LIVE; - + if (vshCommandOptBool (cmd, "p2p")) + flags |= VIR_MIGRATE_PEER2PEER; if (vshCommandOptBool (cmd, "tunnelled")) flags |= VIR_MIGRATE_TUNNELLED; - if (!(flags & VIR_MIGRATE_TUNNELLED)) { - /* For regular live migration, temporarily connect to the destination - * host. For tunnelled migration, that will be done by the remote - * libvirtd. - */ - dconn = virConnectOpenAuth(desturi, virConnectAuthPtrDefault, 0); - if (!dconn) goto done; - } - else { - /* when doing tunnelled migration, use migrateuri if it's available, - * but if not, fall back to desturi. This allows both of these - * to work: - * - * virsh migrate guest qemu+tls://dest/system - * virsh migrate guest qemu+tls://dest/system qemu+tls://dest-alt/system - */ - if (migrateuri == NULL) - migrateuri = desturi; - } + if ((flags & VIR_MIGRATE_PEER2PEER) || + vshCommandOptBool (cmd, "direct")) { + /* For peer2peer migration or direct migration we only expect one URI + * a libvirt URI, or a hypervisor specific URI. */ - /* Migrate. */ - ddom = virDomainMigrate(dom, dconn, flags, dname, migrateuri, 0); - if (!ddom) goto done; + if (migrateuri != NULL) { + vshError(ctl, FALSE, "%s", _("migrate: Unexpected migrateuri for peer2peer/direct migration")); + goto done; + } - ret = TRUE; + if (virDomainMigrateToURI (dom, desturi, flags, dname, 0) == 0) + ret = TRUE; + } else { + /* For traditional live migration, connect to the destination host directly. */ + virConnectPtr dconn = NULL; + virDomainPtr ddom = NULL; + + dconn = virConnectOpenAuth (desturi, virConnectAuthPtrDefault, 0); + if (!dconn) goto done; + + ddom = virDomainMigrate (dom, dconn, flags, dname, migrateuri, 0); + if (ddom) { + virDomainFree(ddom); + ret = TRUE; + } + virConnectClose (dconn); + } done: if (dom) virDomainFree (dom); - if (ddom) virDomainFree (ddom); - if (dconn) virConnectClose (dconn); return ret; } -- 1.6.2.5

On Mon, Oct 05, 2009 at 12:44:47PM +0100, Daniel P. Berrange wrote:
This series is an update of
http://www.redhat.com/archives/libvir-list/2009-September/msg00540.html
There isn't as much functional change here as you might presume from the number of patches. Since Chris' tunnelled migration code is added, I thought it better to do alot of small refactoring steps rather than munge it all in one patch.
One particular notable change since last time is that the new virDomainMigrateToURI method does *not* mandate use of the new "VIR_MIGRATE_PEER2PEER" flag anymore. I will now explain why...
ah ! I won't complain, but I'm curious :)
Traditionally migration has been a 3 step process
1. client -> dest (prepare) 2. client -> source (perform) 3. client -> dest (finish)
This is why virDomainMigrate requires the extra 'dconn' parameter from the client app
With the tunnelled migration / peer-to-peer migration, the extra 'dconn' parameter is no longer required from the client, because the source libvirt driver directly opens a connection to the destination libvirtd daemon on the remote host. ie libvirtd is talking to another libvirtd peer-2-peer.
In considering the implementation of the PEER_TO_PEER flag for the Xen and VMWare drivers though I realized that even this is not technically neccessary, and in fact detrimental to use of libvirt for migrating with these drivers. Both Xen and VMWare have their own management daemon which is perfectly capable of handling the entire process without any need for libvirtd to be involved on the destination host. libvirt need only initiate the migration op on the source host. If you look at the impl of Prepare/Finish ops in the Xen driver this should be obvious, since they are pretty much no-ops
Okay
Thus we in fact have 3 possible migration processes
* Traditional libvirt client managed
client -> dest (prepare) client -> source (perform) client -> dest (finish)
* New peer-to-peer migration (on which tunnelled mig is built)
client -> source (perform) source -> dest (open) source -> dest (prepareTunnel) source -> dest (streamSend) source -> dest (finish) source -> dest (close)
* New direct migration (hypervisor managed)
client -> source (perform) source -> dest (whatever the HV's native migration does)
okay so we split out the last one from the P2P version
In terms of libvirt public APIs this works out as follows
* Traditional libvirt client managed
A hypervisor specific URI style, passed to virDomainMigrate() with no flags set
* New peer-to-peer migration (on which tunnelled mig is built)
The standard libvirt URI style, passed to virDomainMigrate() or virDomainMigrateToURI() with VIR_DOMAIN_MIGRATE_PEER2PEER flag set.
The virDomainMigrateToURI() method is better since it avoids the unneccessary burden on the client of connecting to the remote libvirtd.
* New direct migration (hypervisor managed)
A hypervisor specific URI style, passed to virDomainMigrateToURI() with no flags set
Okay understood, makes sense.
The QEMU driver can't support the latter, since it has no native management daemon - it requires libvirtd's help. Xen and VMWare, and probably VirtualBox can support this just fine.
This is good for flexibility of usage in terms of authentication too, since each of these 3 modes has different characteristics
* Traditional libvirt client managed
client -> source libvirtd auth client -> dest libvirtd auth possible hypervisor -> hypervisor auth
* New peer-to-peer migration, without tunnelling
client -> source libvirtd auth source -> dest libvirtd auth possible hypervisor -> hypervisor auth
* New peer-to-peer migration, without tunnelling
client -> source libvirtd auth source -> dest libvirtd auth
* New direct migration (hypervisor managed)
client -> source libvirtd auth possible hypervisor -> hypervisor auth
NB, 'client -> source libvirtd auth' is essentially no-op if initiating migration from the source host directly in all these cases.
With with the combination of the two virDomainMigrate and virDomainMigrateToURI methods, and the VIR_MIGRATE_PEER2PEER flag, I believe we're well placed to cover all the main deployment/auth scenarios for migration in all hypervisors without imposing extra auth burden ourselves as we did in the past
We need some documentation for all those options, people already get lost when there is only one command for an action, but if you can have 3 ways and multiple auth schemes it gets scary :-)
Further things that need to be done though:
- Xen could easily support PEER2PEER and TUNNELLED flags
- Add a VIR_MIGRATE_SECURE flag, to indicate that the app only wants the migration to be performed if the HV can guarentee the data channel is secure.
good point !
- Document the scenarios I just outlined and give some mre guidance to app developers/administrators about the tradeoff between each scenario
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/

On Mon, Oct 05, 2009 at 12:44:47PM +0100, Daniel P. Berrange wrote:
This series is an update of
http://www.redhat.com/archives/libvir-list/2009-September/msg00540.html
There isn't as much functional change here as you might presume from the number of patches. Since Chris' tunnelled migration code is added, I thought it better to do alot of small refactoring steps rather than munge it all in one patch.
Okay, I reread patches 3 to 9 and I'm fine with those. The malloc() argument is valid, but it should not block commiting, though it would be nice to fix before pushing those patches ! thanks ! 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/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Paolo Bonzini