[libvirt] [PATCH 3/4] qemu: Don't report false error from MigrateFinish

Jiri Denemark jdenemar at redhat.com
Fri Jul 10 09:34:12 UTC 2015


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 at redhat.com>
> > ---
> >  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




More information about the libvir-list mailing list