[libvirt] [PATCH 1/3] qemu_migration: Add hooks to transport network data during migration
Laine Stump
laine at laine.org
Mon Oct 22 21:21:57 UTC 2012
On 10/01/2012 11:18 AM, Kyle Mestery wrote:
> Add the ability for the Qemu V3 migration protocol to
> include transporting network configuration. A generic
> framework is proposed with this patch to allow for the
> transfer of opaque data.
I changed this patch to 1) put the portdata in a separate element rather
than having an extremely long attribute, 2) escape the portdata to avoid
"surprises" if the data happened to have a character that has a
syntactical meaning for xml, and 3) clean up miscellaneous other
problems I've noted below.
Because I'm unable to test these changes myself (and they are more than
trivial), I'm reposting the series. Please test it and report back
whether or not it works (ie ACK or NACK). If ACK, then I'll push the
updated patches.
>
> Signed-off-by: Kyle Mestery <kmestery at cisco.com>
> ---
> src/qemu/qemu_migration.c | 246 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 244 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index db69a0a..a17ccbd 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -70,6 +70,7 @@ enum qemuMigrationCookieFlags {
> QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
> QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
> QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
> + QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
>
> QEMU_MIGRATION_COOKIE_FLAG_LAST
> };
> @@ -77,12 +78,13 @@ enum qemuMigrationCookieFlags {
> VIR_ENUM_DECL(qemuMigrationCookieFlag);
> VIR_ENUM_IMPL(qemuMigrationCookieFlag,
> QEMU_MIGRATION_COOKIE_FLAG_LAST,
> - "graphics", "lockstate", "persistent");
> + "graphics", "lockstate", "persistent", "network");
>
> enum qemuMigrationCookieFeatures {
> QEMU_MIGRATION_COOKIE_GRAPHICS = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
> QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 << QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
> QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
> + QEMU_MIGRATION_COOKIE_NETWORK = (1 << QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
> };
>
> typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
> @@ -95,6 +97,27 @@ struct _qemuMigrationCookieGraphics {
> char *tlsSubject;
> };
>
> +typedef struct _qemuMigrationCookieNetdata qemuMigrationCookieNetdata;
> +typedef qemuMigrationCookieNetdata *qemuMigrationCookieNetdataPtr;
> +struct _qemuMigrationCookieNetdata {
> + int vporttype; /* enum virNetDevVPortProfile */
> +
> + /*
> + * Array of pointers to saved data. Each VIF will have it's own
> + * data to transfer.
> + */
> + char *portdata;
> +};
> +
> +typedef struct _qemuMigrationCookieNetwork qemuMigrationCookieNetwork;
> +typedef qemuMigrationCookieNetwork *qemuMigrationCookieNetworkPtr;
> +struct _qemuMigrationCookieNetwork {
> + /* How many virtual NICs are we saving data for? */
> + int nnets;
> +
> + qemuMigrationCookieNetdataPtr *net;
I changed this to "qemuMigratiponCookieNetdataPtr net;" to reduce the
levels of indirection. The effect is that this is now points to an array
of qemuMigrationCookieNetdata, rather than pointing to an array of
pointers to qemu.....Netdata. All the rest of the code (in this patch
and in 3/3) was adjusted accordingly.
> +};
> +
> typedef struct _qemuMigrationCookie qemuMigrationCookie;
> typedef qemuMigrationCookie *qemuMigrationCookiePtr;
> struct _qemuMigrationCookie {
> @@ -120,6 +143,9 @@ struct _qemuMigrationCookie {
>
> /* If (flags & QEMU_MIGRATION_COOKIE_PERSISTENT) */
> virDomainDefPtr persistent;
> +
> + /* If (flags & QEMU_MIGRATION_COOKIE_NETWORK) */
> + qemuMigrationCookieNetworkPtr network;
> };
>
> static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
> @@ -132,6 +158,25 @@ static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
> }
>
>
> +static void qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr
> + network)
Here and in a few other places, I changed the formatting to this:
static void
functionName(xxxx xxx,
xxxx xxx)
(note the return type and function name on separate lines, and the first
characters of the arg types lining up rather than being offset).
> +{
> + int i;
> +
> + if (!network)
> + return;
> +
> + for (i = 0; i < network->nnets; i++) {
> + if (network->net[i]) {
> + VIR_FREE(network->net[i]->portdata);
> + VIR_FREE(network->net[i]);
> + }
> + }
This was adjusted for the one layer less of indirection.
> + VIR_FREE(network->net);
> + VIR_FREE(network);
> +}
> +
> +
> static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
> {
> if (!mig)
> @@ -140,6 +185,10 @@ static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
> if (mig->flags & QEMU_MIGRATION_COOKIE_GRAPHICS)
> qemuMigrationCookieGraphicsFree(mig->graphics);
>
> + if (mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) {
> + qemuMigrationCookieNetworkFree(mig->network);
> + }
> +
> VIR_FREE(mig->localHostname);
> VIR_FREE(mig->remoteHostname);
> VIR_FREE(mig->name);
> @@ -256,6 +305,57 @@ error:
> }
>
>
> +static qemuMigrationCookieNetworkPtr
> +qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> + virDomainDefPtr def)
> +{
> + qemuMigrationCookieNetworkPtr mig;
> + int i;
> + virDomainNetDefPtr netptr ATTRIBUTE_UNUSED;
I moved this into the for loop and eliminated the ATTRIBUTE_UNUSED (and
actually *do* use it, as an arg to virDomainNetGetActualVirtPortProfile)
> +
> + if (VIR_ALLOC(mig) < 0)
> + goto no_memory;
> +
> + mig->nnets = def->nnets;
> +
> + if (VIR_ALLOC_N(mig->net, def->nnets) <0)
> + goto no_memory;
> +
> + for (i = 0; i < def->nnets; i++) {
> + virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(def->nets[i]);
> + netptr = def->nets[i];
> +
> + if (vport) {
> + if (VIR_ALLOC(mig->net[i]) < 0)
> + goto no_memory;
> +
> + mig->net[i]->vporttype = vport->virtPortType;
> +
> + switch (vport->virtPortType) {
> + case VIR_NETDEV_VPORT_PROFILE_NONE:
> + break;
> + case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> + break;
> + case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> + break;
> + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> + break;
All those breaks were combined.
> + default:
> + mig->net[i]->portdata = NULL;
This is unnecessary, as all newly allocated memory is initialized to 0.
> + break;
> + }
> + }
> + }
> +
> + return mig;
> +
> +no_memory:
> + virReportOOMError();
> + qemuMigrationCookieNetworkFree(mig);
> + return NULL;
> +}
> +
> +
> static qemuMigrationCookiePtr
> qemuMigrationCookieNew(virDomainObjPtr dom)
> {
> @@ -370,6 +470,27 @@ qemuMigrationCookieAddPersistent(qemuMigrationCookiePtr mig,
> }
>
>
> +static int
> +qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig,
> + struct qemud_driver *driver,
> + virDomainObjPtr dom)
> +{
> + if (mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Network migration data already present"));
> + return -1;
> + }
> +
> + if (dom->def->nnets > 0) {
> + if (!(mig->network = qemuMigrationCookieNetworkAlloc(
> + driver, dom->def)))
> + return -1;
> + mig->flags |= QEMU_MIGRATION_COOKIE_NETWORK;
> + }
> +
> + return 0;
> +}
> +
>
> static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
> qemuMigrationCookieGraphicsPtr grap)
> @@ -389,6 +510,27 @@ static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
> }
>
>
> +static void qemuMigrationCookieNetworkXMLFormat(virBufferPtr buf,
> + qemuMigrationCookieNetworkPtr optr)
> +{
> + int i;
> +
> + virBufferAsprintf(buf, " <network>\n");
> + for (i = 0; i < optr->nnets; i++) {
> + /* If optr->net->vporttype[i] is not set, there is nothing to transfer */
> + if (optr->net[i] && optr->net[i]->vporttype != VIR_NETDEV_VPORT_PROFILE_NONE) {
> + virBufferAsprintf(buf, " <interface index='%d' vporttype='%s'",
> + i, virNetDevVPortTypeToString(optr->net[i]->vporttype));
> + if (optr->net[i]->portdata)
> + virBufferAsprintf(buf, " portdata='%s'", optr->net[i]->portdata);
Changed this to fix the indent of the <interface> line (should be 4
spaces) and put portdata into its own subelement, as well as escaping
the portdata as its formatted into the buffer.
> + virBufferAddLit(buf, "/>\n");
> + }
> + }
> +
> + virBufferAddLit(buf, " </network>\n");
> +}
> +
> +
> static int
> qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> virBufferPtr buf,
> @@ -439,6 +581,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> virBufferAdjustIndent(buf, -2);
> }
>
> + if ((mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) && mig->network)
> + qemuMigrationCookieNetworkXMLFormat(buf, mig->network);
> +
> virBufferAddLit(buf, "</qemu-migration>\n");
> return 0;
> }
> @@ -516,6 +661,58 @@ error:
> }
>
>
> +static qemuMigrationCookieNetworkPtr
> +qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt)
> +{
> + qemuMigrationCookieNetworkPtr optr;
> + int i;
> + int n;
> + xmlNodePtr *interfaces = NULL;
> + char *vporttype;
> +
> + if (VIR_ALLOC(optr) < 0)
> + goto no_memory;
> +
> + if ((n = virXPathNodeSet("./network/interface", ctxt, &interfaces)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing interace information"));
s/interrace/interface/ :-)
> + goto error;
> + }
> +
> + optr->nnets = n;
> + if (VIR_ALLOC_N(optr->net, optr->nnets) <0)
> + goto no_memory;
> +
> + for (i = 0; i < n; i++) {
> + if (VIR_ALLOC(optr->net[i]) < 0)
> + goto no_memory;
This alloc is no longer necessary due to changing the data structure.
> + if (VIR_ALLOC(optr->net[i]->portdata) < 0)
> + goto no_memory;
This alloc never was proper - it's just allocating a single character
that is later leaked.
> +
> + /* portdata is optional, and may not exist */
> + optr->net[i]->portdata = virXMLPropString(interfaces[i], "portdata");
Get this with virXPathString("string(./portdata[1])", ctxt) instead
(after setting ctxt->node = interfaces[i])
> +
> + if (!(vporttype = virXMLPropString(interfaces[i], "vporttype"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing vporttype attribute in migration data"));
> + goto error;
> + }
> + optr->net[i]->vporttype = virNetDevVPortTypeFromString(vporttype);
> + }
> +
> + VIR_FREE(interfaces);
> +
> + return optr;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + VIR_FREE(interfaces);
> + qemuMigrationCookieNetworkFree(optr);
> + return NULL;
> +}
> +
> +
> static int
> qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> struct qemud_driver *driver,
> @@ -662,6 +859,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> VIR_FREE(nodes);
> }
>
> + if ((flags & QEMU_MIGRATION_COOKIE_NETWORK) &&
> + virXPathBoolean("count(./network) > 0", ctxt) &&
> + (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt))))
> + goto error;
> +
> return 0;
>
> error:
> @@ -721,6 +923,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
> qemuMigrationCookieAddPersistent(mig, dom) < 0)
> return -1;
>
> + if (flags & QEMU_MIGRATION_COOKIE_NETWORK &&
> + qemuMigrationCookieAddNetwork(mig, driver, dom) < 0)
> + return -1;
> +
> if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
> return -1;
>
> @@ -1067,6 +1273,36 @@ qemuDomainMigrateGraphicsRelocate(struct qemud_driver *driver,
> }
>
>
> +static int
> +qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> + virDomainObjPtr vm,
> + qemuMigrationCookiePtr cookie)
> +{
> + virDomainNetDefPtr netptr ATTRIBUTE_UNUSED;
> + int ret = 0;
> + int i;
> +
> + for (i = 0; i < cookie->network->nnets; i++) {
> + netptr = vm->def->nets[i];
> +
> + switch (cookie->network->net[i]->vporttype) {
> + case VIR_NETDEV_VPORT_PROFILE_NONE:
> + break;
> + case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> + break;
> + case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> + break;
> + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> + break;
> + default:
> + break;
I combined all the breaks.
> + }
> + }
> +
> + return ret;
> +}
> +
> +
> /* This is called for outgoing non-p2p migrations when a connection to the
> * client which initiated the migration was closed but we were waiting for it
> * to follow up with the next phase, that is, in between
> @@ -2011,7 +2247,8 @@ cleanup:
>
> if (ret == 0 &&
> qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
> - QEMU_MIGRATION_COOKIE_PERSISTENT ) < 0)
> + QEMU_MIGRATION_COOKIE_PERSISTENT |
> + QEMU_MIGRATION_COOKIE_NETWORK) < 0)
> VIR_WARN("Unable to encode migration cookie");
>
> qemuMigrationCookieFree(mig);
> @@ -2946,6 +3183,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
>
> qemuDomainCleanupRemove(vm, qemuMigrationPrepareCleanup);
>
> + cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK;
> if (flags & VIR_MIGRATE_PERSIST_DEST)
> cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
>
> @@ -2973,6 +3211,10 @@ qemuMigrationFinish(struct qemud_driver *driver,
> goto endjob;
> }
>
> + if (mig->network)
> + if (qemuDomainMigrateOPDRelocate(driver, vm, mig) < 0)
> + VIR_WARN("unable to provide network data for relocation");
> +
> if (flags & VIR_MIGRATE_PERSIST_DEST) {
> virDomainDefPtr vmdef;
> if (vm->persistent)
Otherwise good. counter-patch coming up!
More information about the libvir-list
mailing list