Re: [Qemu-devel] [RISU PATCH v4 07/10] risu: add simple trace and replay support

[Peter Maydell]

On 6 June 2017 at 15:19, Alex Bennée <address@hidden> wrote:
>
> Peter Maydell <address@hidden> writes:
>
>> On 2 June 2017 at 17:08, Alex Bennée <address@hidden> wrote:
>>> This adds a very dumb and easily breakable trace and replay support. In
>>> --master mode the various risu ops trigger a write of register/memory
>>> state into a binary file which can be played back to an apprentice.
>>> Currently there is no validation of the image source so feeding the
>>> wrong image will fail straight away.
>>>
>>> The trace files will get very big for any appreciable sized test file
>>> and this will be addressed in later patches.
>>>
>>> Signed-off-by: Alex Bennée <address@hidden>
>>>
>>> ---
>>> v4
>>>   - fix formatting mess
>>>   - abort() instead of reporting de-sync
>>>   - don't fake return for compiler
>>> v3
>>>   - fix options parsing
>>>   - re-factored so no need for copy & paste
>>> v2
>>>   - moved read/write functions into main risu.c
>>>   - cleaned up formatting
>>>   - report more in apprentice --trace mode
>>> ---
>>>  risu.c | 97 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>>  1 file changed, 84 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/risu.c b/risu.c
>>> index 22571cd..137cbbf 100644
>>> --- a/risu.c
>>> +++ b/risu.c
>>> @@ -31,6 +31,7 @@
>>>  void *memblock = 0;
>>>
>>>  int apprentice_socket, master_socket;
>>> +int trace_file = 0;
>>>
>>>  sigjmp_buf jmpbuf;
>>>
>>> @@ -40,10 +41,12 @@ int test_fp_exc = 0;
>>>  long executed_tests = 0;
>>>  void report_test_status(void *pc)
>>>  {
>>> -   executed_tests += 1;
>>> -   if (executed_tests % 100 == 0) {
>>> -      fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
>>> -              executed_tests, pc);
>>> +   if (ismaster || trace_file) {
>>> +      executed_tests += 1;
>>> +      if (executed_tests % 100 == 0) {
>>> +         fprintf(stderr,"Executed %ld test instructions (pc=%p)\r",
>>> +                 executed_tests, pc);
>>> +      }
>>>     }
>>>  }
>>>
>>> @@ -54,6 +57,12 @@ int read_sock(void *ptr, size_t bytes)
>>>     return recv_data_pkt(master_socket, ptr, bytes);
>>>  }
>>>
>>> +int write_trace(void *ptr, size_t bytes)
>>> +{
>>> +   size_t res = write(trace_file, ptr, bytes);
>>> +   return (res == bytes) ? 0 : 1;
>>> +}
>>> +
>>>  void respond_sock(int r)
>>>  {
>>>     send_response_byte(master_socket, r);
>>> @@ -66,9 +75,36 @@ int write_sock(void *ptr, size_t bytes)
>>>     return send_data_pkt(apprentice_socket, ptr, bytes);
>>>  }
>>>
>>> +int read_trace(void *ptr, size_t bytes)
>>> +{
>>> +   size_t res = read(trace_file, ptr, bytes);
>>> +   return (res == bytes) ? 0 : 1;
>>> +}
>>> +
>>> +void respond_trace(int r)
>>> +{
>>> +   switch (r) {
>>> +      case 0: /* test ok */
>>> +      case 1: /* end of test */
>>> +         break;
>>> +      default:
>>> +         /* should not get here */
>>> +         abort();
>>> +         break;
>>> +   }
>>> +}
>>> +
>>>  void master_sigill(int sig, siginfo_t *si, void *uc)
>>>  {
>>> -   switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
>>> +   int r;
>>> +
>>> +   if (trace_file) {
>>> +      r = send_register_info(write_trace, uc);
>>> +   } else {
>>> +      r = recv_and_compare_register_info(read_sock, respond_sock, uc);
>>> +   }
>>
>> It's a bit weird that in the trace file case we send the data
>> from the master, but in the normal case we send it from
>> the apprentice end.
>
> It seemed the natural way round (master generating the "gold" stream
> that the apprentice checks). I could switch it or make the apprentice
> responsible for checking it's own work?

I guess it's not worth switching unless it causes the code to
neatly simplify because you don't need the if when both branches
of it are calling the same function...

thanks
-- PMM