7:40 a.m.
On Mon, Feb 18, 2013 at 15:38:35 +0100, Michal Privoznik wrote:
This migration cookie is meant for two purposes. The first is to be
sent
in begin phase from source to destination to let it know we support new
implementation of VIR_MIGRATE_NON_SHARED_{DISK,INC} so destination can
start NBD server. Then, the second purpose is, destination can let us
know, on which port the NBD server is running.
...
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 36e55d2..82c3f97 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
...
@@ -121,6 +127,14 @@ struct _qemuMigrationCookieNetwork {
qemuMigrationCookieNetDataPtr net;
};
+typedef struct _qemuMigrationCookieNBD qemuMigrationCookieNBD;
+typedef qemuMigrationCookieNBD *qemuMigrationCookieNBDPtr;
+struct _qemuMigrationCookieNBD {
+ int port; /* on which port does NBD server listen for incoming data.
+ Zero value has special meaning - it is there just to let
+ destination know we (the source) do support NBD. */
I think you can drop this note about 0 being special. The support for
NBD can detected by an empty <nbd> element in the cookie (i.e.,
QEMU_MIGRATION_COOKIE_NBD is set in mig->flags and mig->ndb is not
NULL). And it seems the parsing code does what I suggest and does not
need port='0' attribute in <nbd/> once 9/12 is applied.
+};
+
typedef struct _qemuMigrationCookie qemuMigrationCookie;
typedef qemuMigrationCookie *qemuMigrationCookiePtr;
struct _qemuMigrationCookie {
...
@@ -837,6 +883,12 @@
qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
goto error;
}
+ /* nbd is optional */
+ if (val == QEMU_MIGRATION_COOKIE_FLAG_NBD) {
+ VIR_FREE(str);
+ continue;
+ }
+
This does not make sense at all, just remove this hunk. Features marked
as mandatory are always mandatory and NBD is no exception.
if ((flags & (1 << val)) == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unsupported migration cookie feature %s"),
@@ -889,6 +941,27 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
(!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt))))
goto error;
+ if (flags & QEMU_MIGRATION_COOKIE_NBD &&
+ virXPathBoolean("boolean(./nbd)", ctxt)) {
+ char *port;
+
+ port = virXPathString("string(./nbd/@port)", ctxt);
+ if (port) {
+ if (VIR_ALLOC(mig->nbd) < 0) {
+ virReportOOMError();
+ goto error;
+ }
+ if (virStrToLong_i(port, NULL, 10, &mig->nbd->port) < 0) {
+ VIR_FREE(port);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed nbd port '%s'"),
+ port);
+ goto error;
+ }
+ VIR_FREE(port);
+ }
We even have virXPathInt that does the conversion for you, btw. But
anyway, sender would send <nbd/> if port == 0 and the receiver would
just ignore it since there is no port attribute present. Something's
strange here. And you actually fix that in 9/12.
+ }
+
virObjectUnref(caps);
return 0;
...
@@ -1666,7 +1755,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr
driver,
origname = NULL;
if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
- QEMU_MIGRATION_COOKIE_LOCKSTATE)))
+ QEMU_MIGRATION_COOKIE_LOCKSTATE |
+ QEMU_MIGRATION_COOKIE_NBD)))
goto cleanup;
if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
@@ -1726,8 +1816,19 @@ done:
else
cookieFlags = QEMU_MIGRATION_COOKIE_GRAPHICS;
- if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
- cookieFlags) < 0) {
+ if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)
&&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) {
I think we should only do this if the source actually asked for NBD,
that is mig->nbd is not NULL.
+ /* TODO support NBD for TUNNELLED migration */
+ if (flags & VIR_MIGRATE_TUNNELLED)
+ VIR_DEBUG("NBD in tunnelled migration is currently not
supported");
+ else {
+ cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
+ priv->nbdPort = 0;
+ }
+ }
+
+ if (qemuMigrationBakeCookie(mig, driver, vm, cookieout,
+ cookieoutlen, cookieFlags) < 0) {
/* We could tear down the whole guest here, but
* cookie data is (so far) non-critical, so that
* seems a little harsh. We'll just warn for now.
...
@@ -2241,8 +2353,9 @@ qemuMigrationRun(virQEMUDriverPtr driver,
return -1;
}
- if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
- QEMU_MIGRATION_COOKIE_GRAPHICS)))
+ mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
+ cookieFlags | QEMU_MIGRATION_COOKIE_GRAPHICS);
+ if (!mig)
goto cleanup;
I think we should emit a log message when we support NBD but it was not
present in migration cookie. And perhaps clear the flag from
cookieFlags. Now I see you did that in 7/12.
if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig) < 0)
...
Jirka