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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list