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

[Michal Privoznik]

On 01/19/2018 10:55 AM, Ján Tomko wrote:
> 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 at redhat.com>
>> ---
>> 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.

Agreed. Let me post such patch.

Michal