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

[Daniel P. Berrange]

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