On Fri, Jan 19, 2018 at 10:02:45AM +0100, Michal Privoznik wrote:
After aeda1b8c56dc5 we tried to stop reporting I/O errors on
expected monitor HUP. We've achieved that by not setting
mon->lastError once we've received SHUTDOWN event. However, this
makes us to deadlock in case there's thread that enters the
monitor after the event is received. The problem is, we're no
longer setting mon->lastError and therefore qemuMonitorSend()
does not return early and continues execution until virCondWait()
(which will never wake up).
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/qemu/qemu_monitor.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 85c7d68a1..9f3e3eb14 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1063,7 +1063,7 @@ qemuMonitorSend(qemuMonitorPtr mon,
{
int ret = -1;
- /* Check whether qemu quit unexpectedly */
+ /* Check whether qemu quit unexpectedly, */
if (mon->lastError.code != VIR_ERR_OK) {
VIR_DEBUG("Attempt to send command while error is set %s",
NULLSTR(mon->lastError.message));
@@ -1071,6 +1071,12 @@ qemuMonitorSend(qemuMonitorPtr mon,
return -1;
}
+ /* or expectedly. */
+ if (mon->willhangup) {
+ VIR_DEBUG("Attempt to send command while domain is shutting down");
+ return -1;
+ }
+
Not reporting an error here is not desirable in all the cases. Quietly failing to send a monitor command might be fine for the GetAllDomainStats use case, but exiting an API called on a specific domain without setting an error is not okay. IMO the right fix of the deadlock is to revert the patch. Jan
mon->msg = msg;
qemuMonitorUpdateWatch(mon);
--
2.13.6
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
