Re: [PATCH v4 00/25] migration: Improve error reporting

[Cédric Le Goater]

On 3/12/24 12:50, Peter Xu wrote:
> On Tue, Mar 12, 2024 at 10:58:51AM +0100, Cédric Le Goater wrote:
> > On 3/12/24 08:16, Cédric Le Goater wrote:
> > > On 3/11/24 21:24, Peter Xu wrote:
> > > > On Fri, Mar 08, 2024 at 04:15:08PM +0800, Peter Xu wrote:
> > > > > On Wed, Mar 06, 2024 at 02:34:15PM +0100, Cédric Le Goater wrote:
> > > > > > * [1-4] already queued in migration-next.
> > > > > >     migration: Report error when shutdown fails
> > > > > >     migration: Remove SaveStateHandler and LoadStateHandler typedefs
> > > > > >     migration: Add documentation for SaveVMHandlers
> > > > > >     migration: Do not call PRECOPY_NOTIFY_SETUP notifiers in case of error
> > > > > > * [5-9] are prequisite changes in other components related to the
> > > > > >     migration save_setup() handler. They make sure a failure is not
> > > > > >     returned without setting an error.
> > > > > >     s390/stattrib: Add Error** argument to set_migrationmode() handler
> > > > > >     vfio: Always report an error in vfio_save_setup()
> > > > > >     migration: Always report an error in block_save_setup()
> > > > > >     migration: Always report an error in ram_save_setup()
> > > > > >     migration: Add Error** argument to vmstate_save()
> > > > > >
> > > > > > * [10-15] are the core changes in migration and memory components to
> > > > > >     propagate an error reported in a save_setup() handler.
> > > > > >
> > > > > >     migration: Add Error** argument to qemu_savevm_state_setup()
> > > > > >     migration: Add Error** argument to .save_setup() handler
> > > > > >     migration: Add Error** argument to .load_setup() handler
> > > > >
> > > > > Further queued 5-12 in migration-staging (until here), thanks.
> > > >
> > > > Just to keep a record: due to the virtio failover test failure and the
> > > > other block migration uncertainty in patch 7 (in which case we may want to
> > > > have a fix on sectors==0 case), I unqueued this chunk for 9.0.
> > >
> > > ok. I will ask the block folks for help to understand if sectors==0
> > > is also an error in the save_setup context. May be  we can still
> > > merge these in 9.0 cycle.
> >
> > I discussed with Kevin and sectors==0 is not an error case, the loop
> > should simply continue. That said, commit 66db46ca83b8 ("migration:
> > Deprecate block migration") would let us remove all that code in
> > the next cycle which is even simpler.
>
> Thanks for taking a look.  I can try to have a look at removing block
> migration in 9.1.

Just sent a 9.0 fix for the block part.

> Regarding to the failover failure - I still think what you posted as a
> "hack" could be an official patch.  Do you plan to send it?  
> Or do you have anything else in mind?

I was hoping to fix the test case instead. I can try to improve the hack
I sent this afternoon.

Thanks,

C.


> For 9.0, we're missing softfreeze. IIUC we can only merge things like
> regression fixes, documentation updates, some test changess, etc.. into rc
> windows. With QEMU's heavy reliance on CI now I don't even think most test
> case changes would be applicable for RCs unless it's never run in a CI.  So
> unless there's a strong need, it'll be easier if we wait for 9.1 (but yet
> again, we can still queue them earlier, so they will appear in the 1st 9.1
> pull).
>
> Thanks,