Re: [PATCH] replay: synchronize on every virtual timer callback

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

+Cleber

On 5/20/20 8:54 AM, Pavel Dovgalyuk wrote:
> On 19.05.2020 18:42, Philippe Mathieu-Daudé wrote:
>> On 5/19/20 12:38 PM, Pavel Dovgalyuk wrote:
>>>
>>> On 19.05.2020 13:32, Alex Bennée wrote:
>>>> Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
>>>>
>>>>> On 19.05.2020 11:11, Alex Bennée wrote:
>>>>>> Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
>>>>>>
>>>>>>> On 18.05.2020 18:56, Alex Bennée wrote:
>>>>>>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>>>>>>
>>>>>>>>> + Alex
>>>>>>>>>
>>>>>>>>> On 5/6/20 10:17 AM, Pavel Dovgalyuk wrote:
>>>>>>>>>> Sometimes virtual timer callbacks depend on order
>>>>>>>>>> of virtual timer processing and warping of virtual clock.
>>>>>>>>>> Therefore every callback should be logged to make replay 
>>>>>>>>>> deterministic.
>>>>>>>>>> This patch creates a checkpoint before every virtual timer 
>>>>>>>>>> callback.
>>>>>>>>>> With these checkpoints virtual timers processing and clock 
>>>>>>>>>> warping
>>>>>>>>>> events order is completely deterministic.
>>>>>>>>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
>>>>>>>>>> ---
>>>>>>>>>>      util/qemu-timer.c |    5 +++++
>>>>>>>>>>      1 file changed, 5 insertions(+)
>>>>>>>>>> diff --git a/util/qemu-timer.c b/util/qemu-timer.c
>>>>>>>>>> index d548d3c1ad..47833f338f 100644
>>>>>>>>>> --- a/util/qemu-timer.c
>>>>>>>>>> +++ b/util/qemu-timer.c
>>>>>>>>>> @@ -588,6 +588,11 @@ bool timerlist_run_timers(QEMUTimerList 
>>>>>>>>>> *timer_list)
>>>>>>>>>> qemu_mutex_lock(&timer_list->active_timers_lock);
>>>>>>>>>>                progress = true;
>>>>>>>>>> +        /*
>>>>>>>>>> +         * Callback may insert new checkpoints, therefore add 
>>>>>>>>>> new checkpoint
>>>>>>>>>> +         * for the virtual timers.
>>>>>>>>>> +         */
>>>>>>>>>> +        need_replay_checkpoint = timer_list->clock->type == 
>>>>>>>>>> QEMU_CLOCK_VIRTUAL;
>>>>>>>>>>          }
>>>>>>>>>> qemu_mutex_unlock(&timer_list->active_timers_lock);
>>>>>>>> So the problem I have with this as with all the record/replay 
>>>>>>>> stuff I
>>>>>>>> need want to review is it's very hard to see things in action. I 
>>>>>>>> added a
>>>>>>>> *very* basic record/replay test to the aarch64 softmmu tests but 
>>>>>>>> they
>>>>>>>> won't exercise any of this code because no timers get fired. I'm
>>>>>>>> assuming the sort of tests that is really needed is something 
>>>>>>>> that not
>>>>>>>> only causes QEMU_CLOCK_VIRTUAL timers to fire and trigger logged HW
>>>>>>>> events and ensure that things don't get confused in the process.
>>>>>>> I encounter most of the bugs in different OS boot scenarios.
>>>>>>>
>>>>>>> We also have internal tests that include some computational, 
>>>>>>> disk, and
>>>>>>> network interaction tasks.
>>>>>>>
>>>>>>> Is it possible to add a test like booting a "real" OS and replaying
>>>>>>> it?
>>>>>> Yes - for these bigger more complex setups we should use the 
>>>>>> acceptance
>>>>>> tests that run under Avocado. See "make check-acceptance".
>>>>> I've installed avocado and avocado-framework, but got the following 
>>>>> error:
>>>>>
>>>>> venv/bin/python: No module named avocado
>>>> Hmm make check-acceptance should automatically setup local copies of
>>>> avocado using virtualenv. You shouldn't need to install the system
>>>> version.
>>>>
>>>
>>> What should I try then?
>>
>> My workflow running selected tests is:
>>
>> $ git clone qemu
>> $ mkdir qemu/build
>> $ cd qemu/build
>> qemu/build$ ../configure
>> qemu/build$ make arm-softmmu/all
>> qemu/build$ make check-venv
>> qemu/build$ tests/venv/bin/python -m avocado \
>>             --show=app,console -t machine:virt \
>>             run tests/acceptance/
>>
>> 'make check-acceptance' runs all the tests for the available QEMU 
>> targets built. It should call check-venv automatically.
> 
> Thanks. Download has started with these command lines.

Good news!

> 
> But usually I run configure directly from the source directory. Could it 
> be the cause of the failure?

To be honest last time I ran ./configure from source directory was more 
than 2 years ago. The acceptance CI testing use out-of-tree build.

I'm surprised it didn't worked as expected for you, because when Cleber 
implemented it, he was using in-tree builds. Maybe it bit-rotten since.

I'm not interested in trying/debugging/maintaining it, but if you think 
it is worthwhile, I'll first simply add a job testing in-tree 
acceptance, shot it to Travis and see.

> 
> 
> Pavel Dovgalyuk
> 
> 
> 
>