[libvirt] [PATCH v3 RESEND 02/12] Introduce NBD migration cookie
Jiri Denemark
jdenemar at redhat.com
Wed Feb 20 13:40:48 UTC 2013
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
More information about the libvir-list
mailing list