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 :|