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 <mpriv...@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
