Re: [libvirt] [PATCH 1/1 V2] Migrate per-port data for Open vSwitch ports during Qemu Live Migration

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'>?

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

> + 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).