[libvirt] [PATCH 03/10] qemu: Migration job on source daemon
Eric Blake
eblake at redhat.com
Fri Jul 22 17:20:16 UTC 2011
On 07/18/2011 06:27 PM, Jiri Denemark wrote:
> Make MIGRATION_OUT use the new helper methods.
>
> This also introduces new protection to migration v3 process: the
> migration job is held from Begin to Confirm to avoid changes to a domain
> during migration (esp. between Begin and Perform phases). This change is
> automatically applied to p2p and tunneled migrations. For normal
> migration, this requires support from a client. In other words, if an
> old (pre 0.9.4) client starts normal migration of a domain, the domain
> will not be protected against changes between Begin and Perform steps.
> ---
> include/libvirt/libvirt.h.in | 3 +
> src/libvirt.c | 27 ++++-
> src/libvirt_internal.h | 6 +
> src/qemu/qemu_driver.c | 61 +++++++++-
> src/qemu/qemu_migration.c | 272 ++++++++++++++++++++++++++++++------------
> src/qemu/qemu_migration.h | 3 +-
> 6 files changed, 285 insertions(+), 87 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 607b5bc..18cc521 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -674,6 +674,9 @@ typedef enum {
> VIR_MIGRATE_NON_SHARED_DISK = (1<< 6), /* migration with non-shared storage with full disk copy */
> VIR_MIGRATE_NON_SHARED_INC = (1<< 7), /* migration with non-shared storage with incremental copy */
> /* (same base image shared between source and destination) */
> + VIR_MIGRATE_CHANGE_PROTECTION = (1<< 8), /* protect for changing domain configuration through the
> + * whole migration process; this will be used automatically
> + * if both parties support it */
Nice that the flag is automatic, but:
> @@ -3779,10 +3781,14 @@ virDomainMigrateVersion3(virDomainPtr domain,
> return NULL;
> }
>
> + if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
> + VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION))
> + protection = VIR_MIGRATE_CHANGE_PROTECTION;
> +
on the converse direction, if the flag is explicitly requested of an
0.9.4 client but you are using an 0.9.3 source, then we should fail the
migration rather than let the migration proceed unsafely (for
precedence, we reject migrations when PEER2PEER flag is set but the
driver lacks the feature).
Also, we should document this flag in the comments of the public migrate
functions.
When the client is managing migration, then you only need support for
change protection on the source side; if the destination side lacks it,
we don't care. You only checked for the capability on the source side,
so for wider compatibility, we should validate the flag, then mask it
out before issuing any callbacks to the remote driver; thus an 0.9.4
source and 0.9.3 destination would still benefit from the protection (as
it is only the source that needs it) when driven by an 0.9.4 client.
[If an 0.9.3 client drives the migration, even with an 0.9.4 source, you
don't get the protection, and if the migration isn't using protocol v3,
you don't need the protection.] So for once, flag manipulation in
libvirt.c is the right thing to do here.
Meanwhile, when the source is managing migration (via the peer2peer or
direct callbacks), then the flag has to be preserved from client to
source, since libvirt.c does not automatically add the flag. In this
case, older sources will reject the flag as unknown.
In version 3 migration, we should pass the flag to all three source
calls in the protocol (you left off confirm), for consistency.
I'd like someone to double-check my diff before I push anything, but I
think that what I have in the diff below captures the above analysis.
> @@ -2340,6 +2406,61 @@ cleanup:
> return ret;
> }
>
> +int
> +qemuMigrationPerform(struct qemud_driver *driver,
Nice way to refactor things (it took me a few reads, not to mention the
confusion in the fact that migrationPerform is used to drive both the
entire p2p process as well as just a phase within the v2 and v3 process).
> +
> + if ((flags& (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) {
> + if (cookieinlen) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("received unexpected cookie with P2P migration"));
> + return -1;
> + }
> +
> + return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri, uri,
> + cookiein, cookieinlen, cookieout,
> + cookieoutlen, flags, dname, resource,
> + v3proto);
qemuMigrationPerformJob either does just phase 2 unprotected, or the
entire peer-2-peer sequence. But peer2peer is protected by default
(since the entire job on the source side was triggered by a single API
call). Since we know the source is protected (whether or not the flag
is set), and since we don't want to fail when talking to an older
destination that doesn't understand the flag, then peer2peer migration
should always clear the protection flag (basically, doPeer2PeerMigrate3
is a reimplementation of virDomainMigrateVersion3, and since the former
learned how to autoset the bit when needed, so should the latter - it's
just that the latter doesn't need to set the bit).
I would offer ACK to your patch plus my delta, but it's risky enough
that I'd like a second review. Here's my proposed delta:
diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index 4b98815..b1bda31 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -678,7 +678,7 @@ typedef enum {
/* (same base image
shared between source and destination) */
VIR_MIGRATE_CHANGE_PROTECTION = (1 << 8), /* protect for changing
domain configuration through the
* whole migration
process; this will be used automatically
- * if both parties
support it */
+ * when supported */
} virDomainMigrateFlags;
diff --git i/src/libvirt.c w/src/libvirt.c
index 9dc526c..dbef4a5 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -4276,7 +4276,7 @@ confirm:
cookieoutlen = 0;
ret = domain->conn->driver->domainMigrateConfirm3
(domain, cookiein, cookieinlen,
- flags, cancelled);
+ flags | protection, cancelled);
/* If Confirm3 returns -1, there's nothing more we can
* do, but fortunately worst case is that there is a
* domain left in 'paused' state on source.
@@ -4295,7 +4295,7 @@ confirm:
/*
- * In normal migration, the libvirt client co-ordinates communcation
+ * In normal migration, the libvirt client co-ordinates communication
* between the 2 libvirtd instances on source & dest hosts.
*
* In this peer-2-peer migration alternative, the libvirt client
@@ -4380,7 +4380,7 @@ virDomainMigratePeer2Peer (virDomainPtr domain,
/*
- * In normal migration, the libvirt client co-ordinates communcation
+ * In normal migration, the libvirt client co-ordinates communication
* between the 2 libvirtd instances on source & dest hosts.
*
* Some hypervisors support an alternative, direct migration where
@@ -4467,6 +4467,9 @@ virDomainMigrateDirect (virDomainPtr domain,
* VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful,
undefine the
* domain on the source host.
* VIR_MIGRATE_PAUSED Leave the domain suspended on the remote side.
+ * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration
+ * changes during the migration process
(set
+ * automatically when supported).
*
* VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
* Applications using the VIR_MIGRATE_PEER2PEER flag will probably
@@ -4574,6 +4577,19 @@ virDomainMigrate (virDomainPtr domain,
goto error;
}
} else {
+ /* Change protection requires support only on source side, and
+ * is only needed in v3 migration, which automatically re-adds
+ * the flag for just the source side. We mask it out for
+ * non-peer2peer to allow migration from newer source to an
+ * older destination that rejects the flag. */
+ if (flags & VIR_MIGRATE_CHANGE_PROTECTION &&
+ !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+
VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION)) {
+ virLibConnError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("cannot enforce change protection"));
+ goto error;
+ }
+ flags &= ~VIR_MIGRATE_CHANGE_PROTECTION;
if (flags & VIR_MIGRATE_TUNNELLED) {
virLibConnError(VIR_ERR_OPERATION_INVALID,
_("cannot perform tunnelled migration
without using peer2peer flag"));
@@ -4642,6 +4658,9 @@ error:
* VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful,
undefine the
* domain on the source host.
* VIR_MIGRATE_PAUSED Leave the domain suspended on the remote side.
+ * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration
+ * changes during the migration process
(set
+ * automatically when supported).
*
* VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
* Applications using the VIR_MIGRATE_PEER2PEER flag will probably
@@ -4756,6 +4775,19 @@ virDomainMigrate2(virDomainPtr domain,
goto error;
}
} else {
+ /* Change protection requires support only on source side, and
+ * is only needed in v3 migration, which automatically re-adds
+ * the flag for just the source side. We mask it out for
+ * non-peer2peer to allow migration from newer source to an
+ * older destination that rejects the flag. */
+ if (flags & VIR_MIGRATE_CHANGE_PROTECTION &&
+ !VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
+
VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION)) {
+ virLibConnError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+ _("cannot enforce change protection"));
+ goto error;
+ }
+ flags &= ~VIR_MIGRATE_CHANGE_PROTECTION;
if (flags & VIR_MIGRATE_TUNNELLED) {
virLibConnError(VIR_ERR_OPERATION_INVALID,
_("cannot perform tunnelled migration
without using peer2peer flag"));
@@ -4831,6 +4863,9 @@ error:
* on the destination host.
* VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful,
undefine the
* domain on the source host.
+ * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration
+ * changes during the migration process
(set
+ * automatically when supported).
*
* The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag.
* If the VIR_MIGRATE_PEER2PEER flag is NOT set, the duri parameter
@@ -4952,6 +4987,9 @@ error:
* on the destination host.
* VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful,
undefine the
* domain on the source host.
+ * VIR_MIGRATE_CHANGE_PROTECTION Protect against domain configuration
+ * changes during the migration process
(set
+ * automatically when supported).
*
* The operation of this API hinges on the VIR_MIGRATE_PEER2PEER flag.
*
diff --git i/src/qemu/qemu_migration.c w/src/qemu/qemu_migration.c
index dca2b16..1121749 100644
--- i/src/qemu/qemu_migration.c
+++ w/src/qemu/qemu_migration.c
@@ -1994,6 +1994,11 @@ static int doPeer2PeerMigrate3(struct
qemud_driver *driver,
NULLSTR(dconnuri), NULLSTR(uri), flags,
NULLSTR(dname), resource);
+ /* Unlike the virDomainMigrateVersion3 counterpart, we don't need
+ * to worry about auto-setting the VIR_MIGRATE_CHANGE_PROTECTION
+ * bit here, because we are already running inside the context of
+ * a single job. */
+
dom_xml = qemuMigrationBegin(driver, vm, xmlin,
&cookieout, &cookieoutlen);
if (!dom_xml)
@@ -2187,7 +2192,7 @@ static int doPeer2PeerMigrate(struct qemud_driver
*driver,
p2p = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
VIR_DRV_FEATURE_MIGRATION_P2P);
/* v3proto reflects whether the caller used Perform3, but with
- * p2p migrate, regardless of whether Perform3 or Perform3
+ * p2p migrate, regardless of whether Perform2 or Perform3
* were used, we decide protocol based on what target supports
*/
*v3proto = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
@@ -2207,6 +2212,13 @@ static int doPeer2PeerMigrate(struct qemud_driver
*driver,
goto cleanup;
}
+ /* Change protection is only required on the source side (us), and
+ * only for v3 migration when begin and perform are separate jobs.
+ * But peer-2-peer is already a single job, and we still want to
+ * talk to older destinations that would reject the flag.
+ * Therefore it is safe to clear the bit here. */
+ flags &= ~VIR_MIGRATE_CHANGE_PROTECTION;
+
if (*v3proto)
ret = doPeer2PeerMigrate3(driver, sconn, dconn, vm, xmlin,
dconnuri, uri, flags, dname, resource);
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list