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

["Cédric Le Goater" <clg@redhat.com>]

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,
>