[libvirt] [PATCH libvirt-java 8/9] Implement and use virDomainMigrateToURI2 instead of virDomainMigrateToURI

[Jiri Denemark]

On Tue, Jan 08, 2013 at 09:18:30 +0100, Claudio Bley wrote:
> At Mon, 07 Jan 2013 17:10:16 -0700,
> Eric Blake wrote:
> > 
> > On 01/05/2013 04:48 AM, Wido den Hollander wrote:
> > > The migrateToURI method now uses virDomainMigrateToURI2 so we can support
> > > some more features.
> > > 
> > 
> > > +    public int migrateToURI(String dconnuri, String miguri, String dxml, long flags, String dname, long bandwidth) throws LibvirtException {
> > > +        int returnValue = libvirt.virDomainMigrateToURI2(VDP, dconnuri, miguri, dxml, new NativeLong(flags), dname, new NativeLong(bandwidth));
> > > +        processError();
> > > +        return returnValue;
> > > +    }
> > > +
> > 
> > >       */
> > >      public int migrateToURI(String uri, long flags, String dname, long bandwidth) throws LibvirtException {
> > > -        int returnValue = libvirt.virDomainMigrateToURI(VDP, uri, new NativeLong(flags), dname, new NativeLong(bandwidth));
> > > -        processError();
> > > -        return returnValue;
> > > +        return migrateToURI(uri, null, null, flags, dname, bandwidth);
> > >      }
> > 
> > When you make changes like this, you need to worry about back-compat
> > issues.  Remember, older versions of libvirt did not have
> > virDomainMigreateToURI2, so you may be effectively turning the old
> > migrateToURI(4-args) into a call to a new API, which will break when
> > targetting older libvirt, whereas if you kept it as a call to the older
> > libvirt.virDomainMigrateToURI, the older libvirt can still do the
> > migration.
> 
> I already checked this. (Wido, it would have been nice if you said
> when this libvirt function came into existence, so review would have
> been easier.)
> 
> Currently, libvirt 0.9.12 is required by libvirt-java evidenced by
> this line in build.properties:
> 
> libvirt.required=0.9.12
> 
> virDomainMigreateToURI2 was introduced in libvirt 0.9.2.

Of course, but the requirement applies to client libvirt library that
libvirt-java links to. In case of stateful drivers (such as QEMU, LXC,
...), applications use this library to talk to a libvirt daemon which
may even run on a remote host and you have no control over its version.
Thus, while the libvirt-java requires libvirt 0.9.12, it doesn't mean it
can't talk to libvirt daemons that are older (as long as the app does
not use anything that was introduced later, of course). That's how it
works in general.

However, the migration APIs are a bit special since they actually
contain some additional logic that decides what API calls need to be
made depending on arguments and flags. In other words, the code in
libvirt library will make sure to call the older variant that does not
support dxml when libvirtd it talks to is not new enough and the did not
explicitly set dxml to something non-null. That is, I think these
particular changes to replace migration APIs with their *2 variants do
not break backward compatibility.

> > There may be more patches like this in your series, but in
> > general, you should favor forwarding calls to the older API, not the
> > newer API, so that the clients will work against as many libvirt
> > versions as possible.
> 
> OK, this makes sense. But where do we draw the line?

We do not draw lines in libvirt :-) Something that used to work should
not just stop working when you upgrade your libvirt library or a
binding.

Jirka