qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 06/17] migration: Add save_live_complete_precopy_{begin,en


From: Maciej S. Szmigiero
Subject: Re: [PATCH v2 06/17] migration: Add save_live_complete_precopy_{begin,end} handlers
Date: Mon, 9 Sep 2024 20:32:45 +0200
User-agent: Mozilla Thunderbird

On 9.09.2024 19:59, Peter Xu wrote:
On Thu, Sep 05, 2024 at 04:45:48PM +0300, Avihai Horon wrote:
On 27/08/2024 20:54, Maciej S. Szmigiero wrote:
External email: Use caution opening links or attachments


From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

These SaveVMHandlers help device provide its own asynchronous
transmission of the remaining data at the end of a precopy phase.

In this use case the save_live_complete_precopy_begin handler might
be used to mark the stream boundary before proceeding with asynchronous
transmission of the remaining data while the
save_live_complete_precopy_end handler might be used to mark the
stream boundary after performing the asynchronous transmission.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
   include/migration/register.h | 36 ++++++++++++++++++++++++++++++++++++
   migration/savevm.c           | 35 +++++++++++++++++++++++++++++++++++
   2 files changed, 71 insertions(+)

diff --git a/include/migration/register.h b/include/migration/register.h
index f60e797894e5..9de123252edf 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -103,6 +103,42 @@ typedef struct SaveVMHandlers {
        */
       int (*save_live_complete_precopy)(QEMUFile *f, void *opaque);

+    /**
+     * @save_live_complete_precopy_begin
+     *
+     * Called at the end of a precopy phase, before all
+     * @save_live_complete_precopy handlers and before launching
+     * all @save_live_complete_precopy_thread threads.
+     * The handler might, for example, mark the stream boundary before
+     * proceeding with asynchronous transmission of the remaining data via
+     * @save_live_complete_precopy_thread.
+     * When postcopy is enabled, devices that support postcopy will skip this 
step.
+     *
+     * @f: QEMUFile where the handler can synchronously send data before 
returning
+     * @idstr: this device section idstr
+     * @instance_id: this device section instance_id
+     * @opaque: data pointer passed to register_savevm_live()
+     *
+     * Returns zero to indicate success and negative for error
+     */
+    int (*save_live_complete_precopy_begin)(QEMUFile *f,
+                                            char *idstr, uint32_t instance_id,
+                                            void *opaque);
+    /**
+     * @save_live_complete_precopy_end
+     *
+     * Called at the end of a precopy phase, after @save_live_complete_precopy
+     * handlers and after all @save_live_complete_precopy_thread threads have
+     * finished. When postcopy is enabled, devices that support postcopy will
+     * skip this step.
+     *
+     * @f: QEMUFile where the handler can synchronously send data before 
returning
+     * @opaque: data pointer passed to register_savevm_live()
+     *
+     * Returns zero to indicate success and negative for error
+     */
+    int (*save_live_complete_precopy_end)(QEMUFile *f, void *opaque);
Is this handler necessary now that migration core is responsible for the
threads and joins them? I don't see VFIO implementing it later on.
Right, I spot the same thing.

This series added three hooks: begin, end, precopy_thread.

What I think is it only needs one, which is precopy_async.  My vague memory
was that was what we used to discuss too, so that when migration precopy
flushes the final round of iterable data, it does:

   (1) loop over all complete_precopy_async() and enqueue the tasks if
       existed into the migration worker pool.  Then,

   (2) loop over all complete_precopy() like before.

Optionally, we can enforce one vmstate handler only provides either
complete_precopy_async() or complete_precopy().  In this case VFIO can
update the two hooks during setup() by detecting multifd && !mapped_ram &&
nocomp.

The "_begin" hook is still necessary to mark the end of the device state
sent via the main migration stream (during the phase VM is still running)
since we can't start loading the multifd sent device state until all of
that earlier data finishes loading first.

We shouldn't send that boundary marker in .save_live_complete_precopy
either since it would meant unnecessary waiting for other devices
(not necessary VFIO ones) .save_live_complete_precopy bulk data.

And VFIO SaveVMHandlers are shared for all VFIO devices (and const) so
we can't really change them at runtime.

Thanks,
Maciej




reply via email to

[Prev in Thread] Current Thread [Next in Thread]