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.szmigi...@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.szmigi...@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.
--
Peter Xu
