On 09/04/2012 04:35 PM, Kyle Mestery wrote:
> Add the ability to migrate per-port data on Open vSwitch
> ports during qemu live migration. A controller can use this
> to store data relating to each port, and have it migrated
> with the virtual machine and populated on the destination
> host.
Good job in figuring this out!
>
> Signed-off-by: Kyle Mestery <kmestery@xxxxxxxxx>
> Cc: Laine Stump <laine@xxxxxxxxx>
> ---
> 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 1b21ef6..8c1a8f1 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_OVS_PORT_DATA,
>
> 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", "ovsportdata");
>
> 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_OVS_PORT_DATA = (1 << QEMU_MIGRATION_COOKIE_FLAG_OVS_PORT_DATA),
> };
>
> typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
> @@ -95,6 +97,19 @@ struct _qemuMigrationCookieGraphics {
> char *tlsSubject;
> };
>
> +typedef struct _qemuMigrationCookieOvsPortData qemuMigrationCookieOvsPortData;
> +typedef qemuMigrationCookieOvsPortData *qemuMigrationCookieOvsPortDataPtr;
> +struct _qemuMigrationCookieOvsPortData {
> + /* How many virtual NICs are we saving data for? */
> + int nnets;
> +
> + /*
> + * Array of pointers to saved data. Each VIF will have it's own
> + * data to transfer.
> + */
> + char **portdata;
> +};
> +
> typedef struct _qemuMigrationCookie qemuMigrationCookie;
> typedef qemuMigrationCookie *qemuMigrationCookiePtr;
> struct _qemuMigrationCookie {
> @@ -120,6 +135,9 @@ struct _qemuMigrationCookie {
>
> /* If (flags & QEMU_MIGRATION_COOKIE_PERSISTENT) */
> virDomainDefPtr persistent;
> +
> + /* If (flags & QEMU_MIGRATION_COOKIE_OVS_PORT_DATA) */
> + qemuMigrationCookieOvsPortDataPtr ovsportdata;
> };
>
> static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
> @@ -132,6 +150,24 @@ static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
> }
>
>
> +static void qemuMigrationCookieOvsPortDataFree(qemuMigrationCookieOvsPortDataPtr
> + ovsportdata)
> +{
> + int i;
> +
> + if (!ovsportdata)
> + return;
> +
> + for (i = 0; i < ovsportdata->nnets; i++) {
> + VIR_FREE(ovsportdata->portdata[i]);
> + }
> +
> + VIR_FREE(ovsportdata->portdata);
> +
> + VIR_FREE(ovsportdata);
> +}
> +
> +
> static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
> {
> if (!mig)
> @@ -140,6 +176,10 @@ static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
> if (mig->flags & QEMU_MIGRATION_COOKIE_GRAPHICS)
> qemuMigrationCookieGraphicsFree(mig->graphics);
>
> + if (mig->flags & QEMU_MIGRATION_COOKIE_OVS_PORT_DATA) {
> + qemuMigrationCookieOvsPortDataFree(mig->ovsportdata);
> + }
> +
> VIR_FREE(mig->localHostname);
> VIR_FREE(mig->remoteHostname);
> VIR_FREE(mig->name);
> @@ -256,6 +296,60 @@ error:
> }
>
>
> +static qemuMigrationCookieOvsPortDataPtr
> +qemuMigrationCookieOvsPortDataAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> + virDomainDefPtr def)
> +{
> + qemuMigrationCookieOvsPortDataPtr mig;
> + int i;
> + virCommandPtr cmd = NULL;
> + virDomainNetDefPtr netptr;
> +
> + if (VIR_ALLOC(mig) < 0)
> + goto no_memory;
> +
> + mig->nnets = def->nnets;
> +
> + if (VIR_ALLOC_N(mig->portdata, 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 && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) {
> + if (VIR_ALLOC(mig->portdata[i]) < 0)
> + goto no_memory;
> +
> + cmd = virCommandNewArgList(OVSVSCTL, "get", "Interface",
> + netptr->ifname, "external_ids:PortData",
> + NULL);
> +
> + virCommandSetOutputBuffer(cmd, &mig->portdata[i]);
> +
> + /* Run the command */
> + if (virCommandRun(cmd, NULL) < 0) {
> + virReportSystemError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to run command to get OVS port data for "
> + "interface %s"), netptr->ifname);
> + goto error;
> + }
> +
> + /* Wipeout the newline */
> + mig->portdata[i][strlen(mig->portdata[i]) - 1] = '\0';
Both this run of ovs-vsctl and the one that re-populates the data at the
remote end should be done by functions in virnetdevopenvswitch.c
> + }
> + }
> +
> + return mig;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + qemuMigrationCookieOvsPortDataFree(mig);
> + return NULL;
> +}
> +
> +
> static qemuMigrationCookiePtr
> qemuMigrationCookieNew(virDomainObjPtr dom)
> {
> @@ -370,6 +464,27 @@ qemuMigrationCookieAddPersistent(qemuMigrationCookiePtr mig,
> }
>
>
> +static int
> +qemuMigrationCookieAddOvsPortData(qemuMigrationCookiePtr mig,
> + struct qemud_driver *driver,
> + virDomainObjPtr dom)
> +{
> + if (mig->flags & QEMU_MIGRATION_COOKIE_OVS_PORT_DATA) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Migration ovs port data data already present"));
> + return -1;
> + }
> +
> + if (dom->def->nnets >= 1) {
> + if (!(mig->ovsportdata = qemuMigrationCookieOvsPortDataAlloc(
> + driver, dom->def)))
> + return -1;
> + mig->flags |= QEMU_MIGRATION_COOKIE_OVS_PORT_DATA;
> + }
> +
> + return 0;
> +}
> +
>
> static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
> qemuMigrationCookieGraphicsPtr grap)
> @@ -389,6 +504,25 @@ static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
> }
>
>
> +static void qemuMigrationCookieOvsPortDataXMLFormat(virBufferPtr buf,
> + qemuMigrationCookieOvsPortDataPtr optr)
> +{
> + int i;
> +
> + virBufferAsprintf(buf, " <ovsportdata nnets='%d'>\n", optr->nnets);
A minor point - is there a reason for including nnets here, when it can
be implied from the number of <vif> elements and/or their "num" attributes?
But beyond that, I'm thinking we might want a more generic toplevel
element name (similar to the graphics one being called <graphics> even
though it could be used for vnc or spice), so that in the future it
could be passed to / retrieved from a separate network driver/daemon
without regard (within the migration code at least) to the specific type
of the interface (or at least allow for the migration code to get
multiple types without needing to clutter the namespace).
In the end, what we're really doing here is sending a last-instant
update to the state of the domain's network interface. Thinking of it in
that way, it would be really nice if it mimicked the <network> element
of the domain XML as much as possible, to make explanations easier and
make future additions more automatic, but I'm not sure how far we want
to go in that direction, since the <network> element of the domain is
unnecessarily complex for this specific job (e.g. it has both the
configuration (which could be type='network') *and* the actual network
connection that's used (which could have a different type if the config
type is 'network'). Another difference specific to this case is that, in
order to detect an interface that uses Open vSwitch, you need to look
both for type='bridge' and for <virtualport type='openvswitch'>.
So, to take this to the very end of that road of reasoning, how bad of
an idea would it be to make the "cookie" for each interface be the full
live XML of that interface, then just include the portdata inside
<virtualport> for every interface that has <virtualport
type='openvswitch'>?
The advantage of doing it this way is that this would automatically
handle any other need for last-minute interface state change info we
might encounter in the future (as long as 1) the desired information is
included in virDomainNetDef and handled by the standard
parser/formatter, and 2) the cookie eating code did something with that
data), and we could just call the parse/format functions in src/conf
rather than needing our own. (A side effect is that "virsh dumpxml
$myguest" would display the portdata for every Open vSwitch interface.
Not having seen the length of the data, I don't know if that's a plus or
a minus :-)
(Note that this would mean you would need to call
virDomainNetGetActualVirtPortProfile() on the receiving end)
(really, I can see a potential need for this for other types of devices
that need up-to-the-last-instant state info sent to the destination. But
of course the more far-reaching you try to be, the bigger the chance
you'll get it wrong :-P)
> + if (optr->nnets > 0)
> + virBufferAsprintf(buf, " <vifs>\n");
Stepping back from my larger change idea above, does having the extra
<vifs> level in the hierarchy make something easier (either now or in a
hypothetical future with more data sent in this cookie)? Or could we
just have the <vif> elements right below the toplevel?
Oh, and just to shake things up - we need to account for the possibility
where the source host is using Open vSwitch, and the destination isn't :-)
> + for (i = 0; i < optr->nnets; i++) {
> + virBufferAsprintf(buf, " <vif num='%d' portdata='%s'/>\n",
> + i, optr->portdata[i]);
> + }
> + if (optr->nnets > 0)
> + virBufferAsprintf(buf, " </vifs>\n");
> +
> + virBufferAddLit(buf, " </ovsportdata>\n");
> +}
> +
> +
> static int
> qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> virBufferPtr buf,
> @@ -439,6 +573,10 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
> virBufferAdjustIndent(buf, -2);
> }
>
> + if ((mig->flags & QEMU_MIGRATION_COOKIE_OVS_PORT_DATA) &&
> + mig->ovsportdata)
> + qemuMigrationCookieOvsPortDataXMLFormat(buf, mig->ovsportdata);
> +
> virBufferAddLit(buf, "</qemu-migration>\n");
> return 0;
> }
> @@ -516,6 +654,57 @@ error:
> }
>
>
> +static qemuMigrationCookieOvsPortDataPtr
> +qemuMigrationCookieOvsPortDataXMLParse(xmlXPathContextPtr ctxt)
> +{
> + qemuMigrationCookieOvsPortDataPtr optr;
> + int i;
> + int n;
> + xmlNodePtr *vifs = NULL;
> +
> + if (VIR_ALLOC(optr) < 0)
> + goto no_memory;
> +
> + if (virXPathInt("string(./ovsportdata/@nnets)", ctxt, &optr->nnets) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing nnets attribute in migration data"));
> + goto error;
> + }
> +
> + if (VIR_ALLOC_N(optr->portdata, optr->nnets) < 0)
> + goto no_memory;
> +
> + if ((n = virXPathNodeSet("./ovsportdata/vifs/vif", ctxt, &vifs)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing vif information"));
> + goto error;
> + }
> +
> + for (i = 0; i < n; i++) {
> + if (VIR_ALLOC(optr->portdata[i]) < 0)
> + goto no_memory;
> +
> + if (!(optr->portdata[i] = virXMLPropString(vifs[i], "portdata"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing portdata attribute in migration data"));
> + goto error;
> + }
> + }
> +
> + VIR_FREE(vifs);
> +
> + return optr;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + if (vifs)
> + VIR_FREE(vifs);
> + qemuMigrationCookieOvsPortDataFree(optr);
> + return NULL;
> +}
> +
> +
> static int
> qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> struct qemud_driver *driver,
> @@ -662,6 +851,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
> VIR_FREE(nodes);
> }
>
> + if ((flags & QEMU_MIGRATION_COOKIE_OVS_PORT_DATA) &&
> + virXPathBoolean("count(./ovsportdata) > 0", ctxt) &&
> + (!(mig->ovsportdata = qemuMigrationCookieOvsPortDataXMLParse(ctxt))))
> + goto error;
> +
> return 0;
>
> error:
> @@ -721,6 +915,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
> qemuMigrationCookieAddPersistent(mig, dom) < 0)
> return -1;
>
> + if (flags & QEMU_MIGRATION_COOKIE_OVS_PORT_DATA &&
> + qemuMigrationCookieAddOvsPortData(mig, driver, dom) < 0)
> + return -1;
> +
> if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
> return -1;
>
> @@ -1050,6 +1248,44 @@ qemuDomainMigrateGraphicsRelocate(struct qemud_driver *driver,
> }
>
>
> +static int
> +qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> + virDomainObjPtr vm,
> + qemuMigrationCookiePtr cookie)
> +{
> + virCommandPtr cmd = NULL;
> + virDomainNetDefPtr netptr;
> + int ret = 0;
> + int i;
> + virBufferPtr buf;
> +
> + if (VIR_ALLOC(buf) < 0) {
> + ret = -1;
> + goto out;
> + }
> +
> + for (i = 0; i < cookie->ovsportdata->nnets; i++) {
> + netptr = vm->def->nets[i];
> +
> + virBufferAsprintf(buf, "external_ids:PortData=%s",
> + cookie->ovsportdata->portdata[i]);
> +
> + cmd = virCommandNewArgList(OVSVSCTL, "set", "Interface",
> + netptr->ifname, virBufferCurrentContent(buf),
> + NULL);
> + /* Run the command */
> + if ((ret = virCommandRun(cmd, NULL)) < 0) {
> + virReportSystemError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to run command to set OVS port data for "
> + "interface %s"), vm->def->nets[i]->ifname);
> + }
> + }
> +
> +out:
> + 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
> @@ -1994,7 +2230,8 @@ cleanup:
>
> if (ret == 0 &&
> qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
> - QEMU_MIGRATION_COOKIE_PERSISTENT ) < 0)
> + QEMU_MIGRATION_COOKIE_PERSISTENT |
> + QEMU_MIGRATION_COOKIE_OVS_PORT_DATA) < 0)
> VIR_WARN("Unable to encode migration cookie");
>
> qemuMigrationCookieFree(mig);
> @@ -2929,6 +3166,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
>
> qemuDomainCleanupRemove(vm, qemuMigrationPrepareCleanup);
>
> + cookie_flags = QEMU_MIGRATION_COOKIE_OVS_PORT_DATA;
> if (flags & VIR_MIGRATE_PERSIST_DEST)
> cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
>
> @@ -2956,6 +3194,10 @@ qemuMigrationFinish(struct qemud_driver *driver,
> goto endjob;
> }
>
> + if (mig->ovsportdata)
> + if (qemuDomainMigrateOPDRelocate(driver, vm, mig) < 0)
> + VIR_WARN("unable to provide data for OVS port data relocation");
> +
> if (flags & VIR_MIGRATE_PERSIST_DEST) {
> virDomainDefPtr vmdef;
> if (vm->persistent)
In general ACK to the idea, but I think we should try to setup a more
general framework so that other types of network connections can plug in
their own data without needing to start over from scratch (and also to
isolate things like calling "ovs-vsctl" to virnetdevopenvswitch.c).
I would like to hear others' opinions on whether or not they think it's
reasonable to include the entire <network> live XML in the cookie, or if
they think that's going beyond the bounds of the intended purpose of the
migration cookie. I don't know if we should go all the way in that
direction, but possibly we should at least make it a *subset* of the
full domain <network> xml (as is done with the <graphics> cookie).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list