Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Max Reitz <mreitz@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Cleber Rosa <crosa@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind
Date: Fri, 30 Apr 2021 23:03:41 +0200	[thread overview]
Message-ID: <68f9e63f-ec81-5d6f-a787-dd825009f3e5@redhat.com> (raw)
In-Reply-To: <a370f621-ed39-9bbf-1546-da057026ca3b@redhat.com>

On 30/04/2021 15:02, Max Reitz wrote:
> On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote:
>> As with gdbserver, valgrind delays the test execution, so
>> the default QMP socket timeout timeout too soon.
> 
> I’m curious: The default timeouts should be long enough for slow 
> systems, too, though (e.g. heavily-loaded CI systems).  I would expect 
> that valgrind is used on developer systems where there is more leeway, 
> so the timeouts should still work.

As said in patch 5, I will check again which timeout is essential to 
avoid and which not.

Emanuele
> 
> But in practice, thinking about that doesn’t matter.  If valgrind leads 
> to a timeout being hit, that wouldn’t be nice.  OTOH, if you run 
> valgrind to debug a test/qemu, you don’t particularly care about the 
> timeouts anyway.
> 
> So in principle, this patch sounds good to me, it’s just that it’s based 
> on patch 5, which I don’t fully agree with.
> 
> Max
> 
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>>   python/qemu/machine.py        | 2 +-
>>   tests/qemu-iotests/iotests.py | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index d6142271c2..dce96e1858 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -410,7 +410,7 @@ def _launch(self) -> None:
>>                                          shell=False,
>>                                          close_fds=False)
>> -        if 'gdbserver' in self._wrapper:
>> +        if 'gdbserver' in self._wrapper or 'valgrind' in self._wrapper:
>>               self._qmp_timer = None
>>           self._post_launch()
>> diff --git a/tests/qemu-iotests/iotests.py 
>> b/tests/qemu-iotests/iotests.py
>> index a2e8604674..94597433fa 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -489,7 +489,7 @@ def log(msg: Msg,
>>   class Timeout:
>>       def __init__(self, seconds, errmsg="Timeout"):
>> -        if qemu_gdb:
>> +        if qemu_gdb or qemu_valgrind:
>>               self.seconds = 3000
>>           else:
>>               self.seconds = seconds
>> @@ -700,7 +700,7 @@ def qmp_to_opts(self, obj):
>>           return ','.join(output_list)
>>       def get_qmp_events(self, wait: bool = False) -> List[QMPMessage]:
>> -        if qemu_gdb:
>> +        if qemu_gdb or qemu_valgrind:
>>               wait = 0.0
>>           return super().get_qmp_events(wait=wait)
>>
> 