5:09 a.m.
Add helpers to remember existing errors when they might be overwritten
and use it to fix "unknown error" reported when attaching disk with lock
manager enabled.
Peter Krempa (3):
util: error: Add helpers for saving and restoring of last error
qemu: hotplug: Use new helpers for storing libvirt errors
qemu: Restore errors when rolling back disk image state
src/libvirt_private.syms | 2 +
src/qemu/qemu_hotplug.c | 97 ++++++++++++++++--------------------------------
src/util/virerror.c | 45 ++++++++++++++++++++++
src/util/virerror.h | 3 ++
4 files changed, 82 insertions(+), 65 deletions(-)
--
2.14.1
5:09 a.m.
New subject: [libvirt] [PATCH 1/3] util: error: Add helpers for saving and restoring of last error
Some cleanup paths overwrite a usefull error message with a less useful
one and we then try to preserve the original message. The handlers added
in this patch will simplify the operations since they are designed right
for the purpose.
---
src/libvirt_private.syms | 2 ++
src/util/virerror.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
src/util/virerror.h | 3 +++
3 files changed, 50 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f30a04b14..62e05186d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1604,6 +1604,8 @@ ebtablesRemoveForwardAllowIn;
virDispatchError;
virErrorCopyNew;
virErrorInitialize;
+virErrorPreserveLast;
+virErrorRestore;
virErrorSetErrnoFromLastError;
virLastErrorIsSystemErrno;
virRaiseErrorFull;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index a5a2d6ed1..1f15c5dbb 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -371,6 +371,51 @@ virSaveLastError(void)
return to;
}
+
+/**
+ * virErrorPreserveLast:
+ * @saveerr: pointer to virErrorPtr for storing last error object
+ *
+ * Preserves the currently set last error (for the thread) into @saveerr so that
+ * it can be restored via virErrorRestore(). @saveerr must be passed to
+ * virErrorRestore()
+ */
+void
+virErrorPreserveLast(virErrorPtr *saveerr)
+{
+ int saved_errno = errno;
+ virErrorPtr lasterr = virGetLastError();
+
+ *saveerr = NULL;
+
+ if (lasterr)
+ *saveerr = virErrorCopyNew(lasterr);
+
+ errno = saved_errno;
+}
+
+
+/**
+ * virErrorRestore:
+ * @savederr: error object holding saved error
+ *
+ * Restores the error passed via @savederr and clears associated memory.
+ */
+void
+virErrorRestore(virErrorPtr *savederr)
+{
+ int saved_errno = errno;
+
+ if (!*savederr)
+ return;
+
+ virSetError(*savederr);
+ virFreeError(*savederr);
+ *savederr = NULL;
+ errno = saved_errno;
+}
+
+
/**
* virResetError:
* @err: pointer to the virError to clean up
diff --git a/src/util/virerror.h b/src/util/virerror.h
index 234864812..54530d081 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -196,4 +196,7 @@ void virErrorSetErrnoFromLastError(void);
bool virLastErrorIsSystemErrno(int errnum);
+void virErrorPreserveLast(virErrorPtr *saveerr);
+void virErrorRestore(virErrorPtr *savederr);
+
#endif
--
2.14.1
5:09 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: hotplug: Use new helpers for storing libvirt errors
The helpers allow to simplify restoring original errors in most cases.
---
src/qemu/qemu_hotplug.c | 93 +++++++++++++++----------------------------------
1 file changed, 28 insertions(+), 65 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b365078ec..35d73f74e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -433,7 +433,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (driveAdded && qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
VIR_WARN("Unable to remove drive %s (%s) after failed "
"qemuMonitorAddDevice", drivealias, drivestr);
@@ -444,11 +444,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
if (qemuDomainObjExitMonitor(driver, vm) < 0)
releaseaddr = false;
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
-
+ virErrorRestore(&orig_err);
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
@@ -722,7 +718,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (driveAdded && qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
VIR_WARN("Unable to remove drive %s (%s) after failed "
"qemuMonitorAddDevice", drivealias, drivestr);
@@ -732,10 +728,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (encobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
ignore_value(qemuDomainObjExitMonitor(driver, vm));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
@@ -819,16 +812,13 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (driveAdded && qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
VIR_WARN("Unable to remove drive %s (%s) after failed "
"qemuMonitorAddDevice", drivealias, drivestr);
}
ignore_value(qemuDomainObjExitMonitor(driver, vm));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
@@ -1356,7 +1346,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (!virDomainObjIsActive(vm))
goto cleanup;
- originalError = virSaveLastError();
+ virErrorPreserveLast(&originalError);
if (vlan < 0) {
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
char *netdev_name;
@@ -1387,8 +1377,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
VIR_FREE(hostnet_name);
}
}
- virSetError(originalError);
- virFreeError(originalError);
+ virErrorRestore(&originalError);
goto cleanup;
}
@@ -1562,7 +1551,7 @@ qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
if (!tlsAlias && !secAlias)
return;
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
@@ -1576,10 +1565,7 @@ qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
ignore_value(qemuDomainObjExitMonitor(driver, vm));
cleanup:
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
}
@@ -1621,12 +1607,9 @@ qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
return qemuDomainObjExitMonitor(driver, vm);
error:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
ignore_value(qemuDomainObjExitMonitor(driver, vm));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
qemuDomainDelTLSObjects(driver, vm, asyncJob, secAlias, tlsAlias);
return -1;
@@ -1788,15 +1771,12 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
/* detach associated chardev on error */
if (chardevAdded)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
ignore_value(qemuDomainObjExitMonitor(driver, vm));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
secAlias, tlsAlias);
goto audit;
@@ -2051,15 +2031,12 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
/* detach associated chardev on error */
if (chardevAttached)
qemuMonitorDetachCharDev(priv->mon, charAlias);
ignore_value(qemuDomainObjExitMonitor(driver, vm));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
secAlias, tlsAlias);
@@ -2202,17 +2179,14 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (objAdded)
ignore_value(qemuMonitorDelObject(priv->mon, objAlias));
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && chardevAdded)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
if (qemuDomainObjExitMonitor(driver, vm) < 0)
releaseaddr = false;
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
secAlias, tlsAlias);
@@ -2349,15 +2323,12 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (objAdded)
ignore_value(qemuMonitorDelObject(priv->mon, objalias));
if (qemuDomainObjExitMonitor(driver, vm) < 0)
mem = NULL;
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
if (!mem)
goto audit;
@@ -2368,10 +2339,9 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
mem = NULL;
/* reset the mlock limit */
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
ignore_value(qemuDomainAdjustMaxMemLock(vm));
- virSetError(orig_err);
- virFreeError(orig_err);
+ virErrorRestore(&orig_err);
goto audit;
}
@@ -2561,17 +2531,14 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (driveAdded && qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
VIR_WARN("Unable to remove drive %s (%s) after failed "
"qemuMonitorAddDevice",
drvstr, devstr);
}
ignore_value(qemuDomainObjExitMonitor(driver, vm));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
virDomainAuditHostdev(vm, hostdev, "attach", false);
@@ -2846,7 +2813,7 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
return ret;
exit_monitor:
- orig_err = virSaveLastError();
+ virErrorPreserveLast(&orig_err);
if (release_backing) {
if (shmem->server.enabled)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
@@ -2857,10 +2824,7 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
release_address = false;
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ virErrorRestore(&orig_err);
goto audit;
}
@@ -2948,10 +2912,9 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm,
_("failed to add new filter rules to '%s' "
"- attempting to restore old rules"),
olddev->ifname);
- errobj = virSaveLastError();
+ virErrorPreserveLast(&errobj);
ignore_value(virDomainConfNWFilterInstantiate(vm->def->uuid, olddev));
- virSetError(errobj);
- virFreeError(errobj);
+ virErrorRestore(&errobj);
return -1;
}
return 0;
--
2.14.1
5:09 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: Restore errors when rolling back disk image state
Some operations done to rollback disk image labelling and locking might
overwrite (or clear) the actual error. Remember the original error when
tearing down disk access so that it's not obscured.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1461301
---
src/qemu/qemu_hotplug.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 35d73f74e..7dd6e5fd9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -94,6 +94,7 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = -1;
virStorageSourcePtr origsrc = NULL;
+ virErrorPtr orig_err = NULL;
if (overridesrc) {
origsrc = disk->src;
@@ -102,6 +103,7 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
/* just tear down the disk access */
if (teardown) {
+ virErrorPreserveLast(&orig_err);
ret = 0;
goto rollback_cgroup;
}
@@ -145,6 +147,8 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
if (origsrc)
disk->src = origsrc;
+ virErrorRestore(&orig_err);
+
virObjectUnref(cfg);
return ret;
--
2.14.1
7:52 a.m.
On Tue, Sep 12, 2017 at 12:09:43PM +0200, Peter Krempa wrote:
Add helpers to remember existing errors when they might be
overwritten
and use it to fix "unknown error" reported when attaching disk with lock
manager enabled.
Peter Krempa (3):
util: error: Add helpers for saving and restoring of last error
qemu: hotplug: Use new helpers for storing libvirt errors
qemu: Restore errors when rolling back disk image state
src/libvirt_private.syms | 2 +
src/qemu/qemu_hotplug.c | 97 ++++++++++++++++--------------------------------
src/util/virerror.c | 45 ++++++++++++++++++++++
src/util/virerror.h | 3 ++
4 files changed, 82 insertions(+), 65 deletions(-)
ACK serues
Jan
2656
days inactive
2658
days old
4 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa