Re: [Qemu-devel] [PULL 18/18] qapi: move RTC_CHANGE to the target schema

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 24 Sept 2021 at 13:21, Markus Armbruster <armbru@redhat.com> wrote:
>> ... this isn't really *target*-specific, it's *device*-specific: some
>> devices implement the event, some don't.
>>
>> Ideally, we'd just fix that.
>
> Would you want to tell the far end "this machine simply does
> not have an RTC device at all (because the hardware it's emulating
> doesn't have one) and so you won't get RTC_CHANGE events" ?

Well, RTC_CHANGE is "Emitted when the guest changes the RTC time."  If
the guest doesn't *have* an RTC...

> A good first step for getting more devices to implement the
> RTC_CHANGE support would be if there was any documentation on how
> to do it. The JSON schema says the offset should be "offset between
> base RTC clock (as specified by -rtc base), and new RTC clock value",
> but there aren't any hints (either there or elsewhere) as to how a
> device is supposed to determine that value, and there's no
> documentation of what the behaviour or intent is of the
> qemu_timedate_diff() function that the existing implementations
> use to calculate the offset.

RTC_CHANGE is from the bad old times, I'm afraid:

    commit 80cd34787fc0fc31b1a341c7b8d8e729c1b6ea58
    Author: Luiz Capitulino <lcapitulino@redhat.com>
    Date:   Thu Feb 25 12:11:44 2010 -0300

        QMP: Introduce RTC_CHANGE event

        Emitted whenever the RTC time changes.

        Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
        Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

No hint at why or how to use it.

I figure this is the matching libvirt commit:

    commit 32e6ac9c2601e19715d18f743cf805a3466d3385
    Author: Daniel P. Berrangé <berrange@redhat.com>
    Date:   Thu Mar 18 18:28:15 2010 +0000

        Add support for an explicit  RTC change event

        This introduces a new event type

           VIR_DOMAIN_EVENT_ID_RTC_CHANGE

        This event includes the new UTC offset measured in seconds.
        Thus there is a new callback definition for this event type

         typedef void (*virConnectDomainEventRTCChangeCallback)(virConnectPtr 
conn,
                                                                virDomainPtr 
dom,
                                                                long long 
utcoffset,
                                                                void *opaque);

        If the guest XML configuration for the <clock> is set to
        offset='variable', then the XML will automatically be
        updated with the new UTC offset value. This ensures that
        during migration/save/restore the new offset is preserved.

        * daemon/remote.c: Dispatch RTC change events to client
        * examples/domain-events/events-c/event-test.c: Watch for
          RTC change events
        * include/libvirt/libvirt.h.in: Define new RTC change event ID
          and callback signature
        * src/conf/domain_event.c, src/conf/domain_event.h,
          src/libvirt_private.syms: Extend API to handle RTC change events
        * src/qemu/qemu_driver.c: Connect to the QEMU monitor event
          for RTC changes and emit a libvirt RTC change event
        * src/remote/remote_driver.c: Receive and dispatch RTC change
          events to application
        * src/remote/remote_protocol.x: Wire protocol definition for
          RTC change events
        * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
          src/qemu/qemu_monitor_json.c: Watch for RTC_CHANGE event
          from QEMU monitor

Suggests it might be needed for migration.

How today's libvirt uses RTC_CHANGE would be good to know.  I don't have
the time to ferret it out myself.  Daniel, do you know?  If not, who
else might?

> Side note: probably the JSON schema should document the units
> for 'offset'. Code inspection suggests it wants seconds.

Doc bug, patch would be lovely.

> Side side note: the JSON event doesn't seem to contemplate
> the possibility that a machine might have more than one RTC...

Right.  It clearly needs an additional member @qom-path identifying the
RTC device.