3:43 p.m.
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.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
v2 split bug reporting changes out from error message changes
src/libvirt.c | 23 ++++++++++++++++-------
src/qemu/qemu_migration.c | 10 +++++++++-
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 6357c49..fce0ec4 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -4561,7 +4561,6 @@ virDomainMigrateVersion2(virDomainPtr domain,
*/
if (!domain->conn->driver->domainGetXMLDesc) {
virLibConnError(VIR_ERR_INTERNAL_ERROR, __FUNCTION__);
- virDispatchError(domain->conn);
return NULL;
}
@@ -4593,8 +4592,9 @@ virDomainMigrateVersion2(virDomainPtr domain,
if (uri == NULL && uri_out == NULL) {
virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s",
_("domainMigratePrepare2 did not set uri"));
- virDispatchError(domain->conn);
cancelled = 1;
+ /* Make sure Finish doesn't overwrite the error */
+ orig_err = virSaveLastError();
goto finish;
}
if (uri_out)
@@ -4625,6 +4625,8 @@ finish:
VIR_DEBUG("Finish2 %p ret=%d", dconn, ret);
ddomain = dconn->driver->domainMigrateFinish2
(dconn, dname, cookie, cookielen, uri, destflags, cancelled);
+ if (cancelled && ddomain)
+ VIR_ERROR(_("finish step ignored that migration was cancelled"));
done:
if (orig_err) {
@@ -4787,13 +4789,19 @@ virDomainMigrateVersion3Full(virDomainPtr domain,
if (useParams &&
virTypedParamsReplaceString(¶ms, &nparams,
VIR_MIGRATE_PARAM_URI,
- uri_out) < 0)
+ uri_out) < 0) {
+ cancelled = 1;
+ orig_err = virSaveLastError();
goto finish;
+ }
} else if (!uri &&
virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_URI, &uri) <= 0) {
virLibConnError(VIR_ERR_INTERNAL_ERROR, "%s",
_("domainMigratePrepare3 did not set uri"));
+ cancelled = 1;
+ orig_err = virSaveLastError();
+ goto finish;
}
if (flags & VIR_MIGRATE_OFFLINE) {
@@ -4872,6 +4880,8 @@ finish:
(dconn, dname, cookiein, cookieinlen, &cookieout, &cookieoutlen,
NULL, uri, destflags, cancelled);
}
+ if (cancelled && ddomain)
+ VIR_ERROR(_("finish step ignored that migration was cancelled"));
/* If ddomain is NULL, then we were unable to start
* the guest on the target, and must restart on the
@@ -5097,7 +5107,6 @@ virDomainMigrateDirect(virDomainPtr domain,
if (!domain->conn->driver->domainMigratePerform) {
virReportUnsupportedError();
- virDispatchError(domain->conn);
return -1;
}
@@ -6403,7 +6412,7 @@ virDomainMigrateFinish2(virConnectPtr dconn,
cookie, cookielen,
uri, flags,
retcode);
- if (!ret)
+ if (!ret && !retcode)
goto error;
return ret;
}
@@ -6695,7 +6704,7 @@ virDomainMigrateFinish3(virConnectPtr dconn,
cookieout, cookieoutlen,
dconnuri, uri, flags,
cancelled);
- if (!ret)
+ if (!ret && !cancelled)
goto error;
return ret;
}
@@ -6970,7 +6979,7 @@ virDomainMigrateFinish3Params(virConnectPtr dconn,
ret = dconn->driver->domainMigrateFinish3Params(
dconn, params, nparams, cookiein, cookieinlen,
cookieout, cookieoutlen, flags, cancelled);
- if (!ret)
+ if (!ret && !cancelled)
goto error;
return ret;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a8a493a..407fb70 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3569,6 +3569,7 @@ static int doPeer2PeerMigrate2(virQEMUDriverPtr driver,
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("domainMigratePrepare2 did not set uri"));
cancelled = true;
+ orig_err = virSaveLastError();
goto finish;
}
@@ -3608,6 +3609,8 @@ finish:
(dconn, dname, cookie, cookielen,
uri_out ? uri_out : dconnuri, destflags, cancelled);
qemuDomainObjExitRemote(vm);
+ if (cancelled && ddomain)
+ VIR_ERROR(_("finish step ignored that migration was cancelled"));
cleanup:
if (ddomain) {
@@ -3769,11 +3772,14 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver,
uri = uri_out;
if (useParams &&
virTypedParamsReplaceString(¶ms, &nparams,
- VIR_MIGRATE_PARAM_URI, uri_out) < 0)
+ VIR_MIGRATE_PARAM_URI, uri_out) < 0) {
+ orig_err = virSaveLastError();
goto finish;
+ }
} else if (!uri && !(flags & VIR_MIGRATE_TUNNELLED)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("domainMigratePrepare3 did not set uri"));
+ orig_err = virSaveLastError();
goto finish;
}
@@ -3850,6 +3856,8 @@ finish:
dconnuri, uri, destflags, cancelled);
qemuDomainObjExitRemote(vm);
}
+ if (cancelled && ddomain)
+ VIR_ERROR(_("finish step ignored that migration was cancelled"));
/* If ddomain is NULL, then we were unable to start
* the guest on the target, and must restart on the
--
1.8.4.2