On 22.10.2012 23:30, Laine Stump wrote:
> From: Kyle Mestery <kmestery@xxxxxxxxx>
>
> 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.
>
> Signed-off-by: Kyle Mestery <kmestery@xxxxxxxxx>
> ---
> src/qemu/qemu_migration.c | 238 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 236 insertions(+), 2 deletions(-)
ACK but see my comments below.
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 487182e..67276f0 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1,3 +1,4 @@
> +
> /*
> * qemu_migration.c: QEMU migration handling
> *
> @@ -70,6 +71,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 +79,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 +98,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;
> +};
> +
> typedef struct _qemuMigrationCookie qemuMigrationCookie;
> typedef qemuMigrationCookie *qemuMigrationCookiePtr;
> struct _qemuMigrationCookie {
> @@ -120,6 +144,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 +159,23 @@ static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
> }
>
>
> +static void
> +qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network)
> +{
> + int i;
> +
> + if (!network)
> + return;
> +
> + if (network->net) {
> + for (i = 0; i < network->nnets; i++)
> + VIR_FREE(network->net[i].portdata);
> + }
You could have spared one level of nesting if you'd just only ... [1]
> + VIR_FREE(network->net);
> + VIR_FREE(network);
> +}
> +
> +
> static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
> {
> if (!mig)
> @@ -140,6 +184,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);
> + }
These curly braces aren't required. But it is not against coding style
neither.
> +
> VIR_FREE(mig->localHostname);
> VIR_FREE(mig->remoteHostname);
> VIR_FREE(mig->name);
> @@ -256,6 +304,49 @@ error:
> }
>
>
> +static qemuMigrationCookieNetworkPtr
> +qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> + virDomainDefPtr def)
> +{
> + qemuMigrationCookieNetworkPtr mig;
> + int i;
> +
> + if (VIR_ALLOC(mig) < 0)
> + goto no_memory;
> +
> + mig->nnets = def->nnets;
[1]: ... set this after the VIR_ALLOC_N below.
> +
> + if (VIR_ALLOC_N(mig->net, def->nnets) <0)
> + goto no_memory;
> +
> + for (i = 0; i < def->nnets; i++) {
> + virDomainNetDefPtr netptr;
> + virNetDevVPortProfilePtr vport;
> +
> + netptr = def->nets[i];
> + vport = virDomainNetGetActualVirtPortProfile(netptr);
> +
> + if (vport) {
> + mig->net[i].vporttype = vport->virtPortType;
> +
> + switch (vport->virtPortType) {
> + case VIR_NETDEV_VPORT_PROFILE_NONE:
> + case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> + case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> + default:
> + break;
> + }
> + }
> + }
> + return mig;
> +
> +no_memory:
> + virReportOOMError();
> + qemuMigrationCookieNetworkFree(mig);
> + return NULL;
> +}
> +
> static qemuMigrationCookiePtr
> qemuMigrationCookieNew(virDomainObjPtr dom)
> {
> @@ -370,6 +461,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) {
> + mig->network = qemuMigrationCookieNetworkAlloc(driver, dom->def);
> + if (!mig->network)
> + return -1;
> + mig->flags |= QEMU_MIGRATION_COOKIE_NETWORK;
> + }
> +
> + return 0;
> +}
> +
>
> static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
> qemuMigrationCookieGraphicsPtr grap)
> @@ -389,6 +501,32 @@ 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 */
I believe '[i]' wants to be moved to 'net' to match the condition below.
> + if (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) {
> + virBufferAddLit(buf, ">\n");
> + virBufferEscapeString(buf, " <portdata>%s</portdata>\n",
> + optr->net[i].portdata);
> + virBufferAddLit(buf, " </interface>\n");
> + } else {
> + virBufferAddLit(buf, "/>\n");
> + }
> + }
> + }
> + virBufferAddLit(buf, " </network>\n");
> +}
> +
> +
Maybe if all 'net[i].vporttype' are VIR_NETDEV_VPORT_PROFILE_NONE so
there is no <interface/> added, we don't need to add empty <network/>
neither. But that just cosmetics and I can live with this version as-is.
> static int
> qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> virBufferPtr buf,
> @@ -439,6 +577,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 +657,58 @@ error:
> }
>
>
> +static qemuMigrationCookieNetworkPtr
> +qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt)
> +{
> + qemuMigrationCookieNetworkPtr optr;
> + int i;
> + int n;
> + xmlNodePtr *interfaces = NULL;
> + char *vporttype;
> + xmlNodePtr save_ctxt = ctxt->node;
> +
> + if (VIR_ALLOC(optr) < 0)
> + goto no_memory;
> +
> + if ((n = virXPathNodeSet("./network/interface", ctxt, &interfaces)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing interface information"));
> + goto error;
> + }
> +
> + optr->nnets = n;
> + if (VIR_ALLOC_N(optr->net, optr->nnets) <0)
> + goto no_memory;
> +
> + for (i = 0; i < n; i++) {
> + /* portdata is optional, and may not exist */
> + ctxt->node = interfaces[i];
> + optr->net[i].portdata = virXPathString("string(./portdata[1])", ctxt);
> +
> + 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);
> +
> +cleanup:
> + ctxt->node = save_ctxt;
> + return optr;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + VIR_FREE(interfaces);
> + qemuMigrationCookieNetworkFree(optr);
> + optr = NULL;
> + goto cleanup;
> +}
> +
> +
> static int
> qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> struct qemud_driver *driver,
> @@ -663,6 +856,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:
> @@ -722,6 +920,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;
>
> @@ -1084,6 +1286,32 @@ 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:
> + case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> + case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> + case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> + default:
> + break;
> + }
> + }
> +
> + 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
> @@ -2029,7 +2257,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);
> @@ -2963,6 +3192,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;
>
> @@ -2990,6 +3220,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)
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list