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

[Peter Xu]

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.

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?

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,

-- 
Peter Xu