Re: [PATCH v3 06/24] migration: Add qemu_loadvm_load_state_buffer() and its handler

[Maciej S. Szmigiero]

On 4.12.2024 22:32, Peter Xu wrote:
> On Sun, Nov 17, 2024 at 08:20:01PM +0100, Maciej S. Szmigiero wrote:
> > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> >
> > qemu_loadvm_load_state_buffer() and its load_state_buffer
> > SaveVMHandler allow providing device state buffer to explicitly
> > specified device via its idstr and instance id.
> >
> > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> One nitpick:
>
> > ---
> >   include/migration/register.h | 17 +++++++++++++++++
> >   migration/savevm.c           | 23 +++++++++++++++++++++++
> >   migration/savevm.h           |  3 +++
> >   3 files changed, 43 insertions(+)
> >
> > diff --git a/include/migration/register.h b/include/migration/register.h
> > index ff0faf5f68c8..39991f3cc5d0 100644
> > --- a/include/migration/register.h
> > +++ b/include/migration/register.h
> > @@ -229,6 +229,23 @@ typedef struct SaveVMHandlers {
> >        */
> >       int (*load_state)(QEMUFile *f, void *opaque, int version_id);
> >   
> > +    /* This runs outside the BQL. */
> > +
> > +    /**
> > +     * @load_state_buffer
> > +     *
> > +     * Load device state buffer provided to qemu_loadvm_load_state_buffer().
> > +     *
> > +     * @opaque: data pointer passed to register_savevm_live()
> > +     * @buf: the data buffer to load
> > +     * @len: the data length in buffer
> > +     * @errp: pointer to Error*, to store an error if it happens.
> > +     *
> > +     * Returns zero to indicate success and negative for error
> > +     */
> > +    int (*load_state_buffer)(void *opaque, char *buf, size_t len,
> > +                             Error **errp);
> > +
> >       /**
> >        * @load_setup
> >        *
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index a254c38edcca..1f58a2fa54ae 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -3085,6 +3085,29 @@ int qemu_loadvm_approve_switchover(void)
> >       return migrate_send_rp_switchover_ack(mis);
> >   }
> >   
> > +int qemu_loadvm_load_state_buffer(const char *idstr, uint32_t instance_id,
> > +                                  char *buf, size_t len, Error **errp)
>
> Suggest to always return bool as success/fail, especially when using
> Error**.

Will change the return type to bool then.

Thanks,
Maciej