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(a)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