On Thu, Jul 09, 2015 at 18:10:51 +0200, Peter Krempa wrote: > On Wed, Jul 08, 2015 at 15:22:51 +0200, Jiri Denemark wrote: > > virDomainMigrateFinish* APIs were unfortunately designed to return the > > pointer to the domain on destination and NULL on error. This looks OK in > > normal cases but the same API is also called when we know migration > > failed and thus we expect Finish to return NULL even if it actually did > > all it was supposed to do without any error. The call is defined to > > return nonnull domain pointer over RPC, which means returning NULL will > > always result in an error being send. If this was not in fact an error, > > the API itself wouldn't set anything to the thread local virError, which > > makes the RPC layer come up with it's own "Library function returned > > error but did not set virError" error. > > > > This is quite confusing and also hard to detect by the caller. This > > patch adds a special error code which can be used to check that Finish > > successfully aborted migration. > > > > Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx> > > --- > > include/libvirt/virterror.h | 1 + > > src/qemu/qemu_migration.c | 6 ++++++ > > src/util/virerror.c | 3 +++ > > 3 files changed, 10 insertions(+) > > I'm kind of not sure whether I like this solution or not. I understand > the reasons for having it but I'm not liking it. ACK but I'd prefer > another opinion on this if possible. I don't like it either but fixing it in RPC is not an option since returning NULL domain would be an ABI incompatible change. And I know for sure I don't want to introduce a completely new Finish API just for this because it is an enormous pain :-) Jirka -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list