Re: [Qemu-devel] [PATCH v15 00/15] PTimer fixes/features and ARM MPTimer conversion

[Dmitry Osipenko <digetx@gmail.com>]

On 06.09.2016 01:12, Peter Maydell wrote:
> On 5 September 2016 at 22:52, Dmitry Osipenko <digetx@gmail.com> wrote:
>> On 05.09.2016 21:14, Peter Maydell wrote:
>>> I tried "check this compiles and passes the tests for each commit",
>>> and what I found was that you get a lot of new warnings running
>>> 'make check' about "Timer with period zero, disabling", which were
>>> not there before. Could you look into this and fix it, please?
> 
>> Sure, I'll silence those ptimer error messages by making them conditional of
>> qtest_enabled(). The messages are actually harmless, however not very useful in
>> the context of testing. Thanks a lot for returning to that series, I appreciate it.
> 
> I had a look at the rest of the patches, but to be honest I found
> it very difficult to figure out whether any of the changes were
> making the right changes or the wrong changes. So it's not clear
> to me that "silence the warning if running under qtest" is right:
> why is the warning being produced at all?
>  

The ptimer tests cover all ptimer behaviour cases, including the cases where
ptimer stops because of the error condition. This helps to ensure that further
applied ptimer patches (like new policies) are not affecting old ptimer (policy)
behaviour, including those error cases. So the warning message is being emitted
each time some of the ptimer tests checks the error condition behaviour.

If you have any thoughts on how to make review of this series easier for you,
please tell. I'm open to suggestions.

BTW, I'm going to turn "Fix counter - 1 returned by ptimer_get_count for the
active timer" patch into a ptimer policy. The patch is correct for all of the
current ptimer users, however that "counter - 1" feature could be useful for
some of the future added timers, like nios2 timer [0], and could be already used
by some of the QEMU forks. So it might be better to retain old behaviour for the
default policy.

[0] https://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg06423.html

-- 
Dmitry