Stefan,
--On 6 August 2013 14:02:18 +0200 Stefan Hajnoczi <stefa...@redhat.com>
wrote:
There is still too much happening in this patch. Making
qemu_new_clock()/qemu_free_clock() public and moving the clock source
constants can be done in a single patch.
OK I'll split it up.
The next patch can change the semantics of qemu_clock_deadline() to
return INT32_MAX when the clock source is disabled. I'm not sure why
you do this and whether you checked that existing users continue to work
correctly?
Rationale: I suspect it doesn't really matter (see below) but the
rationale was that if the clock is disabled, timers will expire in
an infinite time. I was trying to make treatment of expire times
consistent throughout qemu-timer, and this was the one place a
disabled clock was being treated as if the timers were going to
expire.
Audit: The only users (at least by the time my patch set is finished) are:
* cpus.c within qtest_warp(), which appears not to consider the case of
vm_clock being disabled. I do not think this function has been written
considering the possibility that there are no active vm_clock timers
or where the deadline is > INT32_MAX ns away.
* qtest_process_command(), evaluating clock_step, which calls the above.
My preference would be to move these to qemu_clock_deadline_ns (without
the INT32_MAX check) and delete the old qemu_clock_deadline routine
entirely, but I don't really understand the full set of circumstances
in which the qtest routines are meant to work.
Of course cpus.c now uses qemu_clock_deadline_ns_all for a couple of
the previous two uses, and in patch 14 of my series I've commented
that I think the previous use was buggy but have maintained bug-for-bug
compatibility.
I'd particularly like comments on patch 14, which I've mostly blind
coded based on what Paolo asked for. I'm afraid I found the icount
stuff a bit opaque. I'll hold off from v7 until someone's had a look
at these.
Please include
an explanation of why qemu_timeout_ns_to_ms() will be needed in the
future (there are no callers in this patch).
You mean in the commit text as well as the following?
+/* Transition function to convert a nanosecond timeout to ms
+ * This is used where a system does not support ppoll
+ */
--
Alex Bligh
