[libvirt] [PATCH 1/1 V2] Migrate per-port data for Open vSwitch ports during Qemu Live Migration
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. Signed-off-by: Kyle Mestery <kmestery@cisco.com> Cc: Laine Stump <laine@laine.org> --- 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'; + } + } + + 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); + if (optr->nnets > 0) + virBufferAsprintf(buf, " <vifs>\n"); + 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) -- 1.7.11.4
On Tue, Sep 04, 2012 at 04:35:24PM -0400, 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.
Signed-off-by: Kyle Mestery <kmestery@cisco.com> Cc: Laine Stump <laine@laine.org> [....] +static void qemuMigrationCookieOvsPortDataXMLFormat(virBufferPtr buf, + qemuMigrationCookieOvsPortDataPtr optr) +{ + int i; + + virBufferAsprintf(buf, " <ovsportdata nnets='%d'>\n", optr->nnets); + if (optr->nnets > 0) + virBufferAsprintf(buf, " <vifs>\n"); + 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"); +}
I'm not specialist of the networking layer, but it looks to me that this should work (but can someone explain the error scenario if the receiving end don't understand the extra XML passed in the migration data ?). My main concern is tying that generic mechanism to OVS, seems to me this should be split in 2 patches: - one patch adding the generic framework to adding XML networking related data to the XML passed along the domain in the migration - one patch using that generic mechanism to carry the OpenVSwitch per port data The first part defining the data extension etc ought to be generic (but probably network related) and the second part specific to OVS Laine, am I off track ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Sep 6, 2012, at 12:35 AM, Daniel Veillard wrote:
On Tue, Sep 04, 2012 at 04:35:24PM -0400, 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.
Signed-off-by: Kyle Mestery <kmestery@cisco.com> Cc: Laine Stump <laine@laine.org> [....] +static void qemuMigrationCookieOvsPortDataXMLFormat(virBufferPtr buf, + qemuMigrationCookieOvsPortDataPtr optr) +{ + int i; + + virBufferAsprintf(buf, " <ovsportdata nnets='%d'>\n", optr->nnets); + if (optr->nnets > 0) + virBufferAsprintf(buf, " <vifs>\n"); + 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"); +}
I'm not specialist of the networking layer, but it looks to me that this should work (but can someone explain the error scenario if the receiving end don't understand the extra XML passed in the migration data ?). My main concern is tying that generic mechanism to OVS, seems to me this should be split in 2 patches: - one patch adding the generic framework to adding XML networking related data to the XML passed along the domain in the migration - one patch using that generic mechanism to carry the OpenVSwitch per port data The first part defining the data extension etc ought to be generic (but probably network related) and the second part specific to OVS
Laine, am I off track ?
Daniel
Thanks for the review Daniel. Laine and I spoke offline, and he had similar comments to you. I will rework the patch as you describe above, and incorporate any additional comments Laine has as well. Thanks! Kyle
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@cisco.com> Cc: Laine Stump <laine@laine.org> --- 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).
On Thu, Sep 06, 2012 at 11:58:52AM -0400, Laine Stump wrote:
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.
+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'>?
I'm not sure I entirely like the idea of duplicating the full live network interface XML that we've already sent across, mostly because it is duplicating a whole lot of information.
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 :-)
Admittedly from a position of ignorance about what the data is, I would tend to view this data as internal technical implementation data and not really relevant to app developers/users. AFAIK, it isn't data that an app developer would want to interpret or manipulate - just opaque state to be passed across at migration. Considering your point about generality though, I think we could argue that the top level element name could be something different. Eg instead of <ovsportdata> perhaps, <interfacestate type='bridge|direct|etc'> where type matches the types on <interface> in the domain XML, and then subelements which vary per type as needed.
+ 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"); +} +
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).
If the graphics cookie is a subset of the domain <graphics> schema that was a pure accident on my part - I didn't put any effort into making it so :-) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Sep 6, 2012, at 10:58 AM, Laine Stump wrote:
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@cisco.com> Cc: Laine Stump <laine@laine.org> --- 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
Hi Laine: I'm in the process of reworking this patch along the lines you and Daniel have provided input towards. I defined some helper functions in virnetdevopenvswitch.c, but when calling them from qemu_migration.c, the build is failing during linking. I suspect I need to add whatever gets built out of src/util to the qemu stuff, but before going down that path, wanted to run this by the list. Here is the error I see: make[3]: Leaving directory `/production/git/local/libvirt/libvirt/python' Making all in tests make[3]: Entering directory `/production/git/local/libvirt/libvirt/python/tests' make[3]: Nothing to be done for `all'. make[3]: Leaving directory `/production/git/local/libvirt/libvirt/python/tests' make[2]: Leaving directory `/production/git/local/libvirt/libvirt/python' Making all in tests make[2]: Entering directory `/production/git/local/libvirt/libvirt/tests' CCLD qemuxml2argvtest ../src/.libs/libvirt_driver_qemu_impl.a(libvirt_driver_qemu_impl_la-qemu_migration.o): In function `qemuMigrationCookieNetworkAlloc': /production/git/local/libvirt/libvirt/src/qemu/qemu_migration.c:343: undefined reference to `virNetDevOpenvswitchGetMigrateData' ../src/.libs/libvirt_driver_qemu_impl.a(libvirt_driver_qemu_impl_la-qemu_migration.o): In function `qemuDomainMigrateOPDRelocate': /production/git/local/libvirt/libvirt/src/qemu/qemu_migration.c:1302: undefined reference to `virNetDevOpenvswitchSetMigrateData' collect2: error: ld returned 1 exit status make[2]: *** [qemuxml2argvtest] Error 1 make[2]: Leaving directory `/production/git/local/libvirt/libvirt/tests' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/production/git/local/libvirt/libvirt' make: *** [all] Error 2 [kmestery@fedora-build libvirt]$ Any pointers appreciated! Once I get this addressed, I think the new patches are ready for submission. Thanks! Kyle
+ } + } + + 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).
On 09/12/2012 11:08 PM, Kyle Mestery (kmestery) wrote:
Hi Laine:
I'm in the process of reworking this patch along the lines you and Daniel have provided input towards. I defined some helper functions in virnetdevopenvswitch.c, but when calling them from qemu_migration.c, the build is failing during linking. I suspect I need to add whatever gets built out of src/util to the qemu stuff, but before going down that path, wanted to run this by the list. Here is the error I see:
make[3]: Leaving directory `/production/git/local/libvirt/libvirt/python' Making all in tests make[3]: Entering directory `/production/git/local/libvirt/libvirt/python/tests' make[3]: Nothing to be done for `all'. make[3]: Leaving directory `/production/git/local/libvirt/libvirt/python/tests' make[2]: Leaving directory `/production/git/local/libvirt/libvirt/python' Making all in tests make[2]: Entering directory `/production/git/local/libvirt/libvirt/tests' CCLD qemuxml2argvtest ../src/.libs/libvirt_driver_qemu_impl.a(libvirt_driver_qemu_impl_la-qemu_migration.o): In function `qemuMigrationCookieNetworkAlloc': /production/git/local/libvirt/libvirt/src/qemu/qemu_migration.c:343: undefined reference to `virNetDevOpenvswitchGetMigrateData' ../src/.libs/libvirt_driver_qemu_impl.a(libvirt_driver_qemu_impl_la-qemu_migration.o): In function `qemuDomainMigrateOPDRelocate': /production/git/local/libvirt/libvirt/src/qemu/qemu_migration.c:1302: undefined reference to `virNetDevOpenvswitchSetMigrateData'
Usually this means that you haven't added the new utility functions to src/libvirt_private.syms. Find the section for the source file the new function was added to, then add in the name of your new function with a semicolon at the end of the line. Be sure to keep the names in alphabetic order (within each file) lest you incur the wrath of Eric! :-)
On Sep 13, 2012, at 3:09 PM, Laine Stump wrote:
On 09/12/2012 11:08 PM, Kyle Mestery (kmestery) wrote:
Hi Laine:
I'm in the process of reworking this patch along the lines you and Daniel have provided input towards. I defined some helper functions in virnetdevopenvswitch.c, but when calling them from qemu_migration.c, the build is failing during linking. I suspect I need to add whatever gets built out of src/util to the qemu stuff, but before going down that path, wanted to run this by the list. Here is the error I see:
make[3]: Leaving directory `/production/git/local/libvirt/libvirt/python' Making all in tests make[3]: Entering directory `/production/git/local/libvirt/libvirt/python/tests' make[3]: Nothing to be done for `all'. make[3]: Leaving directory `/production/git/local/libvirt/libvirt/python/tests' make[2]: Leaving directory `/production/git/local/libvirt/libvirt/python' Making all in tests make[2]: Entering directory `/production/git/local/libvirt/libvirt/tests' CCLD qemuxml2argvtest ../src/.libs/libvirt_driver_qemu_impl.a(libvirt_driver_qemu_impl_la-qemu_migration.o): In function `qemuMigrationCookieNetworkAlloc': /production/git/local/libvirt/libvirt/src/qemu/qemu_migration.c:343: undefined reference to `virNetDevOpenvswitchGetMigrateData' ../src/.libs/libvirt_driver_qemu_impl.a(libvirt_driver_qemu_impl_la-qemu_migration.o): In function `qemuDomainMigrateOPDRelocate': /production/git/local/libvirt/libvirt/src/qemu/qemu_migration.c:1302: undefined reference to `virNetDevOpenvswitchSetMigrateData'
Usually this means that you haven't added the new utility functions to src/libvirt_private.syms. Find the section for the source file the new function was added to, then add in the name of your new function with a semicolon at the end of the line. Be sure to keep the names in alphabetic order (within each file) lest you incur the wrath of Eric! :-)
Awesome, that was the issue, thanks for the help Laine! Kyle
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Kyle Mestery -
Kyle Mestery (kmestery)
-
Laine Stump