[libvirt] [PATCH 23/24] maint: clean up error reporting in migration

[John Ferlan]

On 12/28/2013 11:11 AM, Eric Blake wrote:
> While auditing the error reporting, I noticed that migration
> had some issues.  Some of the static helper functions tried
> to call virDispatchError(), even though their caller will also
> report the error.  Also, if a migration is cancelled early
> because a uri was not set, we did not guarantee that the finish
> stage would not overwrite the first error message.
> 
> * src/qemu/qemu_migration.c (doPeer2PeerMigrate2)
> (doPeer2PeerMigrate3): Preserve first error when cancelling.
> * src/libvirt.c (virDomainMigrateVersion3Full): Likewise.
> (virDomainMigrateVersion1, virDomainMigrateVersion2)
> (virDomainMigrateDirect): Avoid redundant error dispatch.
> (virDomainMigrateFinish2, virDomainMigrateFinish3)
> (virDomainMigrateFinish3Params): Don't report error on cleanup
> path.
> (virDomainMigratePeer2PeerFull, virDomainMigrateDirect)
> (virDomainMigrate2): Use correct errors.
> (virDomainMigrate*): Prefer virReportError over virLibConnError.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/libvirt.c             | 181 ++++++++++++++++++++++++----------------------
>  src/qemu/qemu_migration.c |  10 ++-
>  2 files changed, 104 insertions(+), 87 deletions(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index a74bfc7..637bfc1 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c

<...snip...>


> 
> @@ -6387,7 +6396,7 @@ virDomainMigrateFinish2(virConnectPtr dconn,
>                                                    cookie, cookielen,
>                                                    uri, flags,
>                                                    retcode);
> -        if (!ret)
> +        if (!ret && !retcode)

This one (and the two below) feel like a bug fix unrelated to error
reporting/processing...

>              goto error;
>          return ret;
>      }
> @@ -6679,7 +6688,7 @@ virDomainMigrateFinish3(virConnectPtr dconn,
>                                                    cookieout, cookieoutlen,
>                                                    dconnuri, uri, flags,
>                                                    cancelled);
> -        if (!ret)
> +        if (!ret && !cancelled)

here..

>              goto error;
>          return ret;
>      }
> @@ -6954,7 +6963,7 @@ virDomainMigrateFinish3Params(virConnectPtr dconn,
>          ret = dconn->driver->domainMigrateFinish3Params(
>                  dconn, params, nparams, cookiein, cookieinlen,
>                  cookieout, cookieoutlen, flags, cancelled);
> -        if (!ret)
> +        if (!ret && !cancelled)

here...

>              goto error;
>          return ret;
>      }

ACK in general - your call on whether to split out the extra checks
pointed out above or leave things as they are.  If in fact those are
"bugs" that "should" go into a RHEL release, then perhaps they need to
be separated...

John