Re: [PATCH 3/8] hw/scsi-disk: move MIG_EVENT_FAILED into a new function

[Eric Blake <eblake@redhat.com>]

On Tue, Oct 13, 2026 at 02:05:27PM -0400, Stefan Hajnoczi wrote:
> On Tue, Oct 13, 2026 at 10:00:15AM -0500, Eric Blake wrote:
> > On Fri, Oct 09, 2026 at 11:30:53AM -0400, Stefan Hajnoczi wrote:
> > > Move the MIG_EVENT_FAILED logic from scsi_block_migration_notifier()
> > > into its own new scsi_block_migration_failed() function. The next commit
> > > adds logic for handling another event, so doing that in dedicated
> > > functions makes the code easier to read.
> > > 
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > ---
> > >  hw/scsi/scsi-disk.c | 26 ++++++++++++++++----------
> > >  1 file changed, 16 insertions(+), 10 deletions(-)
> > 
> > > 
> > > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> > > index deb33ac1dda..3d88e57eded 100644
> > > --- a/hw/scsi/scsi-disk.c
> > > +++ b/hw/scsi/scsi-disk.c
> > > @@ -2761,20 +2761,26 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
> > >   * fails because the destination may have already preempted and we need to get
> > >   * the reservation back.
> > >   */
> > 
> > I think this comment...
> > 
> > > +static void scsi_block_migration_failed(SCSIDiskState *s)
> > > +{
> > > +    SCSIDevice *d = &s->qdev;
> > > +    Error *local_err = NULL;
> > > +
> > > +    if (!scsi_generic_pr_state_preempt(d, &local_err)) {
> > > +        /* MIG_EVENT_FAILED cannot fail, so just warn */
> > > +        error_prepend(&local_err, "scsi-block migration rollback: ");
> > > +        warn_report_err(local_err);
> > > +    }
> > > +}
> > > +
> > >  static int scsi_block_migration_notifier(NotifierWithReturn *notifier,
> > 
> > ...belongs better with the function it was originally introducing.
> 
> I think it's clearer to explain scsi_block_migration_failed()'s purpose
> in its doc comment rather than in scsi_block_migration_notifier(), which
> this patch series turns into just a dispatch function that doesn't do
> anything itself.
> 
> scsi_block_migration_notifier() will dispatch MIG_EVENT_DONE in a later
> commit, so it's more general and no longer does the MIG_EVENT_FAILED
> work itself.
> 
> What do you think?

Okay, that explanation makes sense. I'll take a look at your v2 now.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org