Re: [libvirt] [PATCH] qemu: Don't send any monitor commands afer SHUTDOWN event

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(a)redhat.com&gt; > --- > 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.