On 01.07.24 10:57, David Woodhouse wrote:

On Fri, 2024-06-28 at 22:27 +0100, David Woodhouse wrote:

quoted

On 28 June 2024 17:38:15 BST, Peter Hilber [off-list ref] wrote:

quoted

On 28.06.24 14:15, David Woodhouse wrote:

quoted

On Fri, 2024-06-28 at 13:33 +0200, Peter Hilber wrote:

quoted

On 27.06.24 16:52, David Woodhouse wrote:

quoted

I already added a flags field, so this might look something like:

        /*
         * Smearing flags. The UTC clock exposed through this structure
         * is only ever true UTC, but a guest operating system may
         * choose to offer a monotonic smeared clock to its users. This
         * merely offers a hint about what kind of smearing to perform,
         * for consistency with systems in the nearby environment.
         */
#define VMCLOCK_FLAGS_SMEAR_UTC_SLS (1<<5) /* draft-kuhn-leapsecond-00.txt */

(UTC-SLS is probably a bad example but are there formal definitions for
anything else?)

I think it could also be more generic, like flags for linear smearing,
cosine smearing(?), and smear_start_sec and smear_end_sec fields (relative
to the leap second start). That could also represent UTC-SLS, and
noon-to-noon, and it would be well-defined.

This should reduce the likelihood that the guest doesn't know the smearing
variant.

I'm wary of making it too generic. That would seem to encourage a
*proliferation* of false "UTC-like" clocks.

It's bad enough that we do smearing at all, let alone that we don't
have a single definition of how to do it.

I made the smearing hint a full uint8_t instead of using bits in flags,
in the end. That gives us a full 255 ways of lying to users about what
the time is, so we're unlikely to run out. And it's easy enough to add
a new VMCLOCK_SMEARING_XXX type to the 'registry' for any new methods
that get invented.

My concern is that the registry update may come after a driver has already
been implemented, so that it may be hard to ensure that the smearing which
has been chosen is actually implemented.

Well yes, but why in the name of all that is holy would anyone want
to invent *new* ways to lie to users about the time? If we capture
the existing ones as we write this, surely it's a good thing that
there's a barrier to entry for adding more?

Ultimately though, this isn't the hill for me to die on. I'm pushing on
that topic because I want to avoid the proliferation of *ambiguity*. If
we have a precision clock, we should *know* what the time is.

So how about this proposal. I line up the fields in the proposed shared
memory structure to match your virtio-rtc proposal, using 'subtype' as
you proposed. But, instead of the 'subtype' being valid only for
VIRTIO_RTC_CLOCK_UTC, we define a new top-level type for *smeared* UTC.

So, you have:

+\begin{lstlisting}
+#define VIRTIO_RTC_CLOCK_UTC 0
+#define VIRTIO_RTC_CLOCK_TAI 1
+#define VIRTIO_RTC_CLOCK_MONO 2
+\end{lstlisting}

I propose that you add

#define VIRTIO_RTC_CLOCK_SMEARED_UTC 3

If my proposed memory structure is subsumed into the virtio-rtc
proposal we'd literally use the same names, but for the time being I'll
update mine to:

Do you intend vmclock and virtio-rtc to be ABI compatible? FYI, I see a
potential problem in that Virtio does avoid the use of signed integers so
far. I did not check carefully if there might be other problems, yet.

	/*
	 * What time is exposed in the time_sec/time_frac_sec fields?
	 */
	uint8_t time_type;
#define VMCLOCK_TIME_UTC		0	/* Since 1970-01-01 00:00:00z */
#define VMCLOCK_TIME_TAI		1	/* Since 1970-01-01 00:00:00z */
#define VMCLOCK_TIME_MONOTONIC		2	/* Since undefined epoch */
#define VMCLOCK_TIME_INVALID		3	/* virtio-rtc uses this for smeared UTC */


I can then use your smearing subtype values as the 'hint' field in the
shared memory structure. You currently have:

+\begin{lstlisting}
+#define VIRTIO_RTC_SUBTYPE_STRICT 0
+#define VIRTIO_RTC_SUBTYPE_SMEAR 1
+#define VIRTIO_RTC_SUBTYPE_SMEAR_NOON_LINEAR 2
+#define VIRTIO_RTC_SUBTYPE_LEAP_UNSPECIFIED 3
+\end{lstlisting}

I agree with the above part of your proposal.

I can certainly ensure that 'noon linear' has the same value. I don't
think you need both 'SMEAR' and 'LEAP_UNSPECIFIED' though:


+\item VIRTIO_RTC_SUBTYPE_SMEAR deviates from the UTC standard by
+	smearing time in the vicinity of the leap second, in a not
+	precisely defined manner. This avoids clock steps due to UTC
+	leap seconds.

...

+\item VIRTIO_RTC_SUBTYPE_LEAP_UNSPECIFIED may deviate from the UTC
+	standard w.r.t.\ leap second introduction in an unspecified
way
+	(leap seconds may, or may not, be smeared).

To the client, both of those just mean "for a day or so around a leap
second event, you can't trust this device to know what the time is".
There isn't any point in separating "does lie to you" from "might lie
to you", surely? The guest can't do anything useful with that
distinction. Let's drop SMEAR and keep only LEAP_UNSPECIFIED?

As for VIRTIO_RTC_SUBTYPE_SMEAR, I think this could be dropped indeed
(resp., UTC_SLS may be added).

But VIRTIO_RTC_CLOCK_SMEARED_UTC is an assurance that there will be no
steps (in particular, steps backwards, which some clients might not like)
due to leap seconds, while LEAP_UNSPECIFIED provides no such guarantee.

So I think this might be better handled by adding, alongside

#define VIRTIO_RTC_CLOCK_SMEARED_UTC 3

#define VIRTIO_RTC_CLOCK_LEAP_UNSPECIFIED_UTC 4

(or any better name, like VIRTIO_RTC_CLOCK_MAYBE_SMEARED_UTC).

And if you *really* want to parameterise it, I think that's a bad idea
and it encourages the proliferation of different time "standards", but
I'd probably just suck it up and do whatever you do because that's not
strictly within the remit of my live-migration part.

I think the above proposal to have subtypes for
VIRTIO_RTC_CLOCK_SMEARED_UTC should work.