Re: [Qemu-devel] [RFC PATCH v2 00/49] Series short description

[Paolo Bonzini]

Il 30/07/2014 09:44, Pavel Dovgaluk ha scritto:
>> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo Bonzini
>>>> - patch 16 should also use subsections, and perhaps apply to all other
>>>> CPUs too?
>>>
>>>  We implemented replay only for i386 and ARM. If we'll change other 
>>> targets, it will not
>>> add record/replay capabilities to them, but can confuse the reviewers.
>>
>> Why are these fields only required by record/replay?
> 
> No, these fields can cause non-determinism for normal execution mode too.
> But these changes are not related to reverse execution. I can make patches 
> for them
> and submit them separately, if needed.

Yes, thanks.  If it causes nondeterminism for normal execution mode too,
it's better to apply them to all targets.  If possible use a subsection,
though.

>>>> - patch 27 makes sense but VIRTUAL is used to skip blinking when the VM
>>>> is stopped
>>>
>>>  Right, this is kind of hack. I haven't found better solution yet.
>>
>> I think it's okay to use REALTIME, just add runstate_is_running() to the
>> "if (now >= s->cursor_blink_time)".
>>
>> That said, "every read to virtual clock is written to the
>> replay log" worries me a bit from the point of view of thread-safety.
>> Do you need to log all reads because you don't use icount?  Reads can
>> only happen at given points if you use icount, and you could simply log
>> the icount value at each synchronization point.
> 
> We made two implementations of virtual clock. One uses host realtime clock
> and thus should be written into the log. Another one uses icount and 
> calculated
> in one of the replay modules (similar to regular icount implementation).
> This patch series contains only first implementation, because it looks more 
> convenient:
> user does not have to specify icount shift value, when starting the recording 
> or replaying.

I think from the upstreaming point of view it's better to stick with
what makes the code simpler.  Start by submitting only the icount-based
implementation, the other can come later.

Paolo