Ping :)

Any ideas on this?

On 7/17/24 10:57 AM, Daniil Tatianin wrote:
On 5/29/24 6:27 PM, Philippe Mathieu-Daudé wrote:

On 29/5/24 16:34, Markus Armbruster wrote:
Daniil Tatianin <d-tatia...@yandex-team.ru> writes:

On 5/29/24 4:39 PM, Philippe Mathieu-Daudé wrote:

On 29/5/24 14:43, Daniil Tatianin wrote:
On 5/29/24 3:36 PM, Philippe Mathieu-Daudé wrote:

On 29/5/24 14:03, Markus Armbruster wrote:
Daniil Tatianin <d-tatia...@yandex-team.ru> writes:

This can be used to force-synchronize the time in guest after a long stop-cont pause, which can be useful for serverless-type workload.

Also add a comment to highlight the fact that this (and one other QMP
command) only works for the MC146818 RTC controller.

Acked-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Signed-off-by: Daniil Tatianin <d-tatia...@yandex-team.ru>
---

Changes since v0:
- Rename to rtc-inject-irq to match other similar API
- Add a comment to highlight that this only works for the I386 RTC

Changes since v1:
- Added a description below the QMP command to explain how it can be
    used and what it does.

Changes since v2:
- Add a 'broadcast' suffix.
- Change the comments to explain the flags we're setting.
- Change the command description to fix styling & explain that it's a broadcast command.

Changes since v3:
- Fix checkpatch complaints about usage of C99 comments

---
   hw/rtc/mc146818rtc.c         | 20 ++++++++++++++++++++
   include/hw/rtc/mc146818rtc.h |  1 +
   qapi/misc-target.json        | 19 +++++++++++++++++++
   3 files changed, 40 insertions(+)


diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 4e0a6492a9..7d388a3753 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -19,6 +19,25 @@
   { 'command': 'rtc-reset-reinjection',
     'if': 'TARGET_I386' }
   +##
+# @rtc-inject-irq-broadcast:
+#
+# Inject an RTC interrupt for all existing RTCs on the system.
+# The interrupt forces the guest to synchronize the time with RTC. +# This is useful after a long stop-cont pause, which is common for
+# serverless-type workload.

In previous version you said:

   > This isn't really related to migration though. Serverless is based
   > on constantly stopping and resuming the VM on e.g. every HTTP
   > request to an endpoint.

Which made some sense. Maybe mention HTTP? And point to that use case
(possibly with QMP commands) in the commit description?

Hmm, maybe it would be helpful for people who don't know what serverless means.

How about:
      This is useful after a long stop-const pause, which is common for serverless-type workloads,       e.g. stopping/resuming the VM on every HTTP request to an endpoint, which might involve       a long pause in between the requests, causing time drift in the guest.

Please help me understand your workflow. Your management layer call
@stop and @cont QMP commands, is that right?

Yes, that is correct.

@cont will emit a @RESUME event.

If we could listen to QAPI events from C code, we could have the
mc146818rtc device automatically sync on VM resume, and no need for
this async command.

Perhaps? I'm not sure how that would be implemented, but let's see what Markus has to say.

You can't listen on an event in QEMU itself.  You can only hook into the
place that generates the event.

Apparently "qemu/notify.h" could be use for QAPI events (currently
only used by migration). Big change, to be discussed later.

The RESUME event is sent from vm_prepare_start() in system/cpus.c.

Good spot, it is where we call synchronize_pre_resume() for vCPUs,
which is exactly what Daniil wants for RTC devices.

I'd rather we call here rtc_synchronize_pre_resume(), which would
mostly be qmp_rtc_inject_irq_broadcast() content, without using QMP
at all.

But for back-compat we need some CLI option "sync-rtc-on-resume"
default to false. Preferably a mc146818rtc property to KISS.

That would solve Daniil problem and make Markus/myself happier.

So I started looking into this, and I'm a bit unsure about what we want this API to look like. What I mean is there isn't a generic RTC abstraction in QEMU, likewise there isn't an "RTC" global variable you can easily use to hook up some sort of API or ops-like functions like cpu_accel does.

One simple solution I'm seeing is making an mc146818-specific API like mc146818rtc_synchronize_pre_resume(), and call that directly wrapped inside an ifdef CONFIG_MC146818RTC inside system/cpus.c. We can then check the sync-on-resume property inside of that helper and optionally just return from it if it's not set.

Any objections on this approach? Is there a better way to do this?

Thanks!
Paolo, any objection?

Regards,

Phil.

Reply via email to