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

[Pavel Dovgalyuk <dovgaluk@ispras.ru>]

On 20.05.2020 10:18, Philippe Mathieu-Daudé wrote:
> +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.

Maybe that copy has some garbage.

I started with new cloned repository and in-tree build, and everything 
was ok.