[libvirt] [PATCH] qemu: Return meaningful error when qemu dies early

https://bugzilla.redhat.com/show_bug.cgi?id=844378 When qemu dies early after connecting to its monitor but before we actually try to read something from the monitor, we would just fail domain start with useless message: "An error occurred, but the cause is unknown" This is because the real error gets reported in a monitor EOF handler executing within libvirt's event loop. The fix is to take any error set in qemuMonitor structure and propagate it into the thread-local error when qemuMonitorClose is called and no thread-local error is set. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b2af0ae..bca2612 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -924,6 +924,12 @@ void qemuMonitorClose(qemuMonitorPtr mon) virCondSignal(&mon->notify); } + /* Propagate existing monitor error in case the current thread has no + * error set. + */ + if (mon->lastError.code != VIR_ERR_OK && !virGetLastError()) + virSetError(&mon->lastError); + virObjectUnlock(mon); virObjectUnref(mon); } -- 1.9.0

On Mon, Mar 17, 2014 at 13:21:56 +0100, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=844378
When qemu dies early after connecting to its monitor but before we actually try to read something from the monitor, we would just fail domain start with useless message:
"An error occurred, but the cause is unknown"
This is because the real error gets reported in a monitor EOF handler executing within libvirt's event loop.
The fix is to take any error set in qemuMonitor structure and propagate it into the thread-local error when qemuMonitorClose is called and no thread-local error is set.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Oops, I forgot to send notes: To reproduce the issue, just apply the following patch and kill qemu process while libvirtd is sleeping: diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c index 400625a..3b77ecc 100644 --- i/src/qemu/qemu_process.c +++ w/src/qemu/qemu_process.c @@ -1415,6 +1415,9 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int logfd) if (mon) ignore_value(qemuMonitorSetDomainLog(mon, logfd)); + VIR_DEBUG("Sleeping"); + sleep(5); + virObjectLock(vm); virObjectUnref(vm); priv->monStart = 0;

On 17.03.2014 13:21, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=844378
When qemu dies early after connecting to its monitor but before we actually try to read something from the monitor, we would just fail domain start with useless message:
"An error occurred, but the cause is unknown"
This is because the real error gets reported in a monitor EOF handler executing within libvirt's event loop.
The fix is to take any error set in qemuMonitor structure and propagate it into the thread-local error when qemuMonitorClose is called and no thread-local error is set.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b2af0ae..bca2612 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -924,6 +924,12 @@ void qemuMonitorClose(qemuMonitorPtr mon) virCondSignal(&mon->notify); }
+ /* Propagate existing monitor error in case the current thread has no + * error set. + */ + if (mon->lastError.code != VIR_ERR_OK && !virGetLastError()) + virSetError(&mon->lastError); + virObjectUnlock(mon); virObjectUnref(mon); }
ACK Michal

On Thu, Mar 20, 2014 at 15:51:44 +0100, Michal Privoznik wrote:
On 17.03.2014 13:21, Jiri Denemark wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=844378
When qemu dies early after connecting to its monitor but before we actually try to read something from the monitor, we would just fail domain start with useless message:
"An error occurred, but the cause is unknown"
This is because the real error gets reported in a monitor EOF handler executing within libvirt's event loop.
The fix is to take any error set in qemuMonitor structure and propagate it into the thread-local error when qemuMonitorClose is called and no thread-local error is set.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_monitor.c | 6 ++++++ 1 file changed, 6 insertions(+)
ACK
Pushed, thanks. Jirka
participants (2)
-
Jiri Denemark
-
Michal Privoznik