[libvirt] [PATCH 0/3] Transport Open vSwitch per-port data during live migration
This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together. The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed. Kyle Mestery (3): 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. Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData(). src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevopenvswitch.c | 70 +++++++++++ src/util/virnetdevopenvswitch.h | 6 + 4 files changed, 339 insertions(+), 2 deletions(-) -- 1.7.11.4
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@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; +}; + 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) +{ + 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]); + } + } + 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; + + 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; + default: + mig->net[i]->portdata = NULL; + 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); + 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")); + 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; + if (VIR_ALLOC(optr->net[i]->portdata) < 0) + goto no_memory; + + /* portdata is optional, and may not exist */ + optr->net[i]->portdata = virXMLPropString(interfaces[i], "portdata"); + + 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; + } + } + + 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) -- 1.7.11.4
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@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!
On Oct 22, 2012, at 4:21 PM, Laine Stump <laine@laine.org> wrote:
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.
This is great, thanks Laine! I just now see your updated patches, I will pull those down, apply, rebuild and let you know how my testing goes with a ACK/NACK reply. Thanks! Kyle
Signed-off-by: Kyle Mestery <kmestery@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!
Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Signed-off-by: Kyle Mestery <kmestery@cisco.com> --- src/libvirt_private.syms | 2 ++ src/util/virnetdevopenvswitch.c | 70 +++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 6 ++++ 3 files changed, 78 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eebc52a..449744e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1477,7 +1477,9 @@ virNetDevMacVLanVPortProfileRegisterCallback; # virnetdevopenvswitch.h virNetDevOpenvswitchAddPort; +virNetDevOpenvswitchGetMigrateData; virNetDevOpenvswitchRemovePort; +virNetDevOpenvswitchSetMigrateData; # virnetdevtap.h diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index a6993b6..841f693 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -179,3 +179,73 @@ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const ch virCommandFree(cmd); return ret; } + +/** + * virNetDevOpenvswitchGetMigrateData: + * @migrate: a pointer to store the data into, allocated by this function + * @ifname: name of the interface for which data is being migrated + * + * Allocates data to be migrated specific to Open vSwitch + * + * Returns 0 in case of success or -1 in case of failure + */ +int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) +{ + virCommandPtr cmd = NULL; + int ret = 0; + + cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", "get", "Interface", + ifname, "external_ids:PortData", NULL); + + virCommandSetOutputBuffer(cmd, migrate); + + /* 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"), ifname); + ret = -1; + goto error; + } + + /* Wipeout the newline */ + (*migrate)[strlen(*migrate) - 1] = '\0'; + +error: + return ret; +} + +/** + * virNetDevOpenvswitchSetMigrateData: + * @migrate: the data which was transferred during migration + * @ifname: the name of the interface the data is associated with + * + * Repopulates OVS per-port data on destination host + * + * Returns 0 in case of success or -1 in case of failure + */ +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) +{ + virCommandPtr cmd = NULL; + int ret = 0; + virBufferPtr buf; + + if (VIR_ALLOC(buf) < 0) { + ret = -1; + goto error; + } + + virBufferAsprintf(buf, "external_ids:PortData=%s", migrate); + + cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", "set", "Interface", 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"), ifname); + } + +error: + return ret; +} diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 7e5b618..147cd6f 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -42,4 +42,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, int virNetDevOpenvswitchRemovePort(const char *brname, const char *ifname) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + #endif /* __VIR_NETDEV_OPENVSWITCH_H__ */ -- 1.7.11.4
On 10/01/2012 11:18 AM, Kyle Mestery wrote:
Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration.
Signed-off-by: Kyle Mestery <kmestery@cisco.com> --- src/libvirt_private.syms | 2 ++ src/util/virnetdevopenvswitch.c | 70 +++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 6 ++++ 3 files changed, 78 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eebc52a..449744e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1477,7 +1477,9 @@ virNetDevMacVLanVPortProfileRegisterCallback;
# virnetdevopenvswitch.h virNetDevOpenvswitchAddPort; +virNetDevOpenvswitchGetMigrateData; virNetDevOpenvswitchRemovePort; +virNetDevOpenvswitchSetMigrateData;
# virnetdevtap.h diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index a6993b6..841f693 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -179,3 +179,73 @@ int virNetDevOpenvswitchRemovePort(const char *brname ATTRIBUTE_UNUSED, const ch virCommandFree(cmd); return ret; } + +/** + * virNetDevOpenvswitchGetMigrateData: + * @migrate: a pointer to store the data into, allocated by this function + * @ifname: name of the interface for which data is being migrated + * + * Allocates data to be migrated specific to Open vSwitch + * + * Returns 0 in case of success or -1 in case of failure + */ +int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) +{ + virCommandPtr cmd = NULL; + int ret = 0; + + cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", "get", "Interface", + ifname, "external_ids:PortData", NULL); + + virCommandSetOutputBuffer(cmd, migrate); + + /* 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"), ifname); + ret = -1; + goto error; + } + + /* Wipeout the newline */ + (*migrate)[strlen(*migrate) - 1] = '\0'; + +error: + return ret; +} + +/** + * virNetDevOpenvswitchSetMigrateData: + * @migrate: the data which was transferred during migration + * @ifname: the name of the interface the data is associated with + * + * Repopulates OVS per-port data on destination host + * + * Returns 0 in case of success or -1 in case of failure + */ +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) +{ + virCommandPtr cmd = NULL; + int ret = 0; + virBufferPtr buf; + + if (VIR_ALLOC(buf) < 0) { + ret = -1; + goto error; + } + + virBufferAsprintf(buf, "external_ids:PortData=%s", migrate); + + cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", "set", "Interface", 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"), ifname); + } + +error: + return ret; +} diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 7e5b618..147cd6f 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -42,4 +42,10 @@ int virNetDevOpenvswitchAddPort(const char *brname, int virNetDevOpenvswitchRemovePort(const char *brname, const char *ifname) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + +int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + #endif /* __VIR_NETDEV_OPENVSWITCH_H__ */
ACK, but I'm going to resubmit it rebased on the new 1/3.
Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData(). Signed-off-by: Kyle Mestery <kmestery@cisco.com> --- src/qemu/qemu_migration.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a17ccbd..b4ff3c5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -311,7 +311,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, { qemuMigrationCookieNetworkPtr mig; int i; - virDomainNetDefPtr netptr ATTRIBUTE_UNUSED; + virDomainNetDefPtr netptr; if (VIR_ALLOC(mig) < 0) goto no_memory; @@ -339,6 +339,13 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, case VIR_NETDEV_VPORT_PROFILE_8021QBH: break; case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + if (virNetDevOpenvswitchGetMigrateData(&mig->net[i]->portdata, + netptr->ifname) != 0) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _("Unable to run command to get OVS port data for " + "interface %s"), netptr->ifname); + goto error; + } break; default: mig->net[i]->portdata = NULL; @@ -347,6 +354,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, } } +error: return mig; no_memory: @@ -1278,8 +1286,8 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED, virDomainObjPtr vm, qemuMigrationCookiePtr cookie) { - virDomainNetDefPtr netptr ATTRIBUTE_UNUSED; - int ret = 0; + virDomainNetDefPtr netptr; + int ret = -1; int i; for (i = 0; i < cookie->network->nnets; i++) { @@ -1293,12 +1301,21 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED, case VIR_NETDEV_VPORT_PROFILE_8021QBH: break; case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + if (virNetDevOpenvswitchSetMigrateData(cookie->network->net[i]->portdata, + netptr->ifname) != 0) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _("Unable to run command to set OVS port data for " + "interface %s"), netptr->ifname); + goto cleanup; + } break; default: break; } } + ret = 0; +cleanup: return ret; } -- 1.7.11.4
On 10/01/2012 11:18 AM, Kyle Mestery wrote:
Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData().
Signed-off-by: Kyle Mestery <kmestery@cisco.com> --- src/qemu/qemu_migration.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a17ccbd..b4ff3c5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -311,7 +311,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, { qemuMigrationCookieNetworkPtr mig; int i; - virDomainNetDefPtr netptr ATTRIBUTE_UNUSED; + virDomainNetDefPtr netptr;
if (VIR_ALLOC(mig) < 0) goto no_memory; @@ -339,6 +339,13 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, case VIR_NETDEV_VPORT_PROFILE_8021QBH: break; case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + if (virNetDevOpenvswitchGetMigrateData(&mig->net[i]->portdata, + netptr->ifname) != 0) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _("Unable to run command to get OVS port data for " + "interface %s"), netptr->ifname); + goto error; + } break; default: mig->net[i]->portdata = NULL; @@ -347,6 +354,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED, } }
+error: return mig;
no_memory: @@ -1278,8 +1286,8 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED, virDomainObjPtr vm, qemuMigrationCookiePtr cookie) { - virDomainNetDefPtr netptr ATTRIBUTE_UNUSED; - int ret = 0; + virDomainNetDefPtr netptr; + int ret = -1; int i;
for (i = 0; i < cookie->network->nnets; i++) { @@ -1293,12 +1301,21 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED, case VIR_NETDEV_VPORT_PROFILE_8021QBH: break; case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: + if (virNetDevOpenvswitchSetMigrateData(cookie->network->net[i]->portdata, + netptr->ifname) != 0) { + virReportSystemError(VIR_ERR_INTERNAL_ERROR, + _("Unable to run command to set OVS port data for " + "interface %s"), netptr->ifname); + goto cleanup; + } break; default: break; } }
+ ret = 0; +cleanup: return ret; }
Again, ACK, but I'm resubmitting it rebased + "net[i]->portdata" changed to "net[i].portdata".
On Oct 1, 2012, at 10:18 AM, Kyle Mestery <kmestery@cisco.com> wrote:
This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together.
The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed.
Kyle Mestery (3): 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. Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData().
src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevopenvswitch.c | 70 +++++++++++ src/util/virnetdevopenvswitch.h | 6 + 4 files changed, 339 insertions(+), 2 deletions(-)
Just curious if anyone has had a chance to review this series yet? I believe I've addressed all the comments I've received so far. I may need to rebase and send it out again since it's been almost 2 weeks since I sent it out. Thanks, Kyle
On 10/11/2012 05:06 PM, Kyle Mestery (kmestery) wrote:
On Oct 1, 2012, at 10:18 AM, Kyle Mestery <kmestery@cisco.com> wrote:
This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together.
The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed.
Kyle Mestery (3): 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. Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData().
src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevopenvswitch.c | 70 +++++++++++ src/util/virnetdevopenvswitch.h | 6 + 4 files changed, 339 insertions(+), 2 deletions(-)
Just curious if anyone has had a chance to review this series yet? I believe I've addressed all the comments I've received so far. I may need to rebase and send it out again since it's been almost 2 weeks since I sent it out.
Sorry I haven't responded. Lately it's been one deadline after another (actually I'm working on the latest as I type). One thought I've had about this - since you're just taking the data directly from ovs-vsctl and printf-ing it into a buffer, there is a potential that the data could contain xml meta characters (either by design / on purpose, or perhaps if an attacker takes over ovs-vsctl or the database in some way). This could leave the system that's the target of the migration open to attacks based on injecting "something else" into the migration cookie. Is virBufferEscapeString() enough to both guarantee nothing like that happens, as well as getting the exact same string at the destination? I *think* so, but am not sure. Also, I'm still not so sure about having the data as an attribute when it could potentially been very long. DV, what's your opinion about this? Is it okay to have very long strings as attributes, or is it considered in better taste to put something that is, say, 1000 characters long as a separate element? Anyway, at this point I don't think you need to rebase/resend. By the beginning of next week hopefully someone more wise than me about XML will have answered these two questions, and I can just squash in the minor changes and push it.
On Oct 11, 2012, at 4:25 PM, Laine Stump <laine@laine.org> wrote:
On 10/11/2012 05:06 PM, Kyle Mestery (kmestery) wrote:
On Oct 1, 2012, at 10:18 AM, Kyle Mestery <kmestery@cisco.com> wrote:
This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together.
The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed.
Kyle Mestery (3): 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. Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData().
src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevopenvswitch.c | 70 +++++++++++ src/util/virnetdevopenvswitch.h | 6 + 4 files changed, 339 insertions(+), 2 deletions(-)
Just curious if anyone has had a chance to review this series yet? I believe I've addressed all the comments I've received so far. I may need to rebase and send it out again since it's been almost 2 weeks since I sent it out.
Sorry I haven't responded. Lately it's been one deadline after another (actually I'm working on the latest as I type).
Thanks Laine!
One thought I've had about this - since you're just taking the data directly from ovs-vsctl and printf-ing it into a buffer, there is a potential that the data could contain xml meta characters (either by design / on purpose, or perhaps if an attacker takes over ovs-vsctl or the database in some way). This could leave the system that's the target of the migration open to attacks based on injecting "something else" into the migration cookie.
Is virBufferEscapeString() enough to both guarantee nothing like that happens, as well as getting the exact same string at the destination? I *think* so, but am not sure.
Also, I'm still not so sure about having the data as an attribute when it could potentially been very long. DV, what's your opinion about this? Is it okay to have very long strings as attributes, or is it considered in better taste to put something that is, say, 1000 characters long as a separate element?
Anyway, at this point I don't think you need to rebase/resend. By the beginning of next week hopefully someone more wise than me about XML will have answered these two questions, and I can just squash in the minor changes and push it.
Thanks Laine! Let me know if you need anything else on my end. Thanks, Kyle
On Oct 11, 2012, at 4:36 PM, Kyle Mestery (kmestery) <kmestery@cisco.com> wrote:
On Oct 11, 2012, at 4:25 PM, Laine Stump <laine@laine.org> wrote:
On 10/11/2012 05:06 PM, Kyle Mestery (kmestery) wrote:
On Oct 1, 2012, at 10:18 AM, Kyle Mestery <kmestery@cisco.com> wrote:
This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together.
The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed.
Kyle Mestery (3): 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. Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData().
src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevopenvswitch.c | 70 +++++++++++ src/util/virnetdevopenvswitch.h | 6 + 4 files changed, 339 insertions(+), 2 deletions(-)
Just curious if anyone has had a chance to review this series yet? I believe I've addressed all the comments I've received so far. I may need to rebase and send it out again since it's been almost 2 weeks since I sent it out.
Sorry I haven't responded. Lately it's been one deadline after another (actually I'm working on the latest as I type).
Thanks Laine!
One thought I've had about this - since you're just taking the data directly from ovs-vsctl and printf-ing it into a buffer, there is a potential that the data could contain xml meta characters (either by design / on purpose, or perhaps if an attacker takes over ovs-vsctl or the database in some way). This could leave the system that's the target of the migration open to attacks based on injecting "something else" into the migration cookie.
Is virBufferEscapeString() enough to both guarantee nothing like that happens, as well as getting the exact same string at the destination? I *think* so, but am not sure.
Also, I'm still not so sure about having the data as an attribute when it could potentially been very long. DV, what's your opinion about this? Is it okay to have very long strings as attributes, or is it considered in better taste to put something that is, say, 1000 characters long as a separate element?
Anyway, at this point I don't think you need to rebase/resend. By the beginning of next week hopefully someone more wise than me about XML will have answered these two questions, and I can just squash in the minor changes and push it.
Thanks Laine! Let me know if you need anything else on my end.
Thanks, Kyle
Just pinging again on this patch set Laine. I don't think anyone responded on your XML questions yet, so I'm unsure what the current status of this patch set is. I assume it's in the same state? Thanks! Kyle
On 10/22/2012 09:41 AM, Kyle Mestery (kmestery) wrote:
On Oct 11, 2012, at 4:36 PM, Kyle Mestery (kmestery) <kmestery@cisco.com> wrote:
On Oct 11, 2012, at 4:25 PM, Laine Stump <laine@laine.org> wrote:
On 10/11/2012 05:06 PM, Kyle Mestery (kmestery) wrote:
On Oct 1, 2012, at 10:18 AM, Kyle Mestery <kmestery@cisco.com> wrote:
This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together.
The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed.
Kyle Mestery (3): 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. Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData().
src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevopenvswitch.c | 70 +++++++++++ src/util/virnetdevopenvswitch.h | 6 + 4 files changed, 339 insertions(+), 2 deletions(-) Just curious if anyone has had a chance to review this series yet? I believe I've addressed all the comments I've received so far. I may need to rebase and send it out again since it's been almost 2 weeks since I sent it out. Sorry I haven't responded. Lately it's been one deadline after another (actually I'm working on the latest as I type).
Thanks Laine!
One thought I've had about this - since you're just taking the data directly from ovs-vsctl and printf-ing it into a buffer, there is a potential that the data could contain xml meta characters (either by design / on purpose, or perhaps if an attacker takes over ovs-vsctl or the database in some way). This could leave the system that's the target of the migration open to attacks based on injecting "something else" into the migration cookie.
Is virBufferEscapeString() enough to both guarantee nothing like that happens, as well as getting the exact same string at the destination? I *think* so, but am not sure.
Also, I'm still not so sure about having the data as an attribute when it could potentially been very long. DV, what's your opinion about this? Is it okay to have very long strings as attributes, or is it considered in better taste to put something that is, say, 1000 characters long as a separate element?
Anyway, at this point I don't think you need to rebase/resend. By the beginning of next week hopefully someone more wise than me about XML will have answered these two questions, and I can just squash in the minor changes and push it. Thanks Laine! Let me know if you need anything else on my end.
Thanks, Kyle
Just pinging again on this patch set Laine. I don't think anyone responded on your XML questions yet, so I'm unsure what the current status of this patch set is. I assume it's in the same state?
Yes. That was to be my first task this morning, but I arrived at the keyboard to a string of questions on IRC. I'll hopefully post something for you to test before noon (EST). If it works properly, and you approve of the changes, then I'll push.
On Oct 22, 2012, at 9:16 AM, Laine Stump <laine@laine.org> wrote:
On 10/22/2012 09:41 AM, Kyle Mestery (kmestery) wrote:
On Oct 11, 2012, at 4:36 PM, Kyle Mestery (kmestery) <kmestery@cisco.com> wrote:
On Oct 11, 2012, at 4:25 PM, Laine Stump <laine@laine.org> wrote:
On 10/11/2012 05:06 PM, Kyle Mestery (kmestery) wrote:
On Oct 1, 2012, at 10:18 AM, Kyle Mestery <kmestery@cisco.com> wrote:
This series of commits has the end goal of allowing per-port data stored in the Open vSwitch DB to be transported during live migration. This is done by first providing a generic infrastructure for transporting network data, adding some utility functions specific to Open vSwitch, and hooking the two together.
The framework provided is generic in that other networking data could be transferred as well by simply adding in additional hooks as needed.
Kyle Mestery (3): 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. Add utility functions for Open vSwitch to both save per-port data before a live migration, and restore the per-port data after a live migration. Transport Open vSwitch per-port data during live migration by using the utility functions virNetDevOpenvswitchGetMigrateData() and virNetDevOpenvswitchSetMigrateData().
src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- src/util/virnetdevopenvswitch.c | 70 +++++++++++ src/util/virnetdevopenvswitch.h | 6 + 4 files changed, 339 insertions(+), 2 deletions(-) Just curious if anyone has had a chance to review this series yet? I believe I've addressed all the comments I've received so far. I may need to rebase and send it out again since it's been almost 2 weeks since I sent it out. Sorry I haven't responded. Lately it's been one deadline after another (actually I'm working on the latest as I type).
Thanks Laine!
One thought I've had about this - since you're just taking the data directly from ovs-vsctl and printf-ing it into a buffer, there is a potential that the data could contain xml meta characters (either by design / on purpose, or perhaps if an attacker takes over ovs-vsctl or the database in some way). This could leave the system that's the target of the migration open to attacks based on injecting "something else" into the migration cookie.
Is virBufferEscapeString() enough to both guarantee nothing like that happens, as well as getting the exact same string at the destination? I *think* so, but am not sure.
Also, I'm still not so sure about having the data as an attribute when it could potentially been very long. DV, what's your opinion about this? Is it okay to have very long strings as attributes, or is it considered in better taste to put something that is, say, 1000 characters long as a separate element?
Anyway, at this point I don't think you need to rebase/resend. By the beginning of next week hopefully someone more wise than me about XML will have answered these two questions, and I can just squash in the minor changes and push it. Thanks Laine! Let me know if you need anything else on my end.
Thanks, Kyle
Just pinging again on this patch set Laine. I don't think anyone responded on your XML questions yet, so I'm unsure what the current status of this patch set is. I assume it's in the same state?
Yes. That was to be my first task this morning, but I arrived at the keyboard to a string of questions on IRC. I'll hopefully post something for you to test before noon (EST). If it works properly, and you approve of the changes, then I'll push.
Thanks again for the update Laine! Will test things out once you shoot the patches out. Thanks, Kyle
participants (3)
-
Kyle Mestery -
Kyle Mestery (kmestery)
-
Laine Stump