Re: [PATCH for-9.1 v5 09/14] memory: Add Error** argument to .log_global_start() handler

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

On 3/20/24 15:42, Peter Xu wrote:
> On Wed, Mar 20, 2024 at 07:49:05AM +0100, Cédric Le Goater wrote:
>> Modify all .log_global_start() handlers to take an Error** parameter
>> and return a bool. Adapt memory_global_dirty_log_start() to interrupt
>> on the first error the loop on handlers. In such case, a rollback is
>> performed to stop dirty logging on all listeners where it was
>> previously enabled.
>>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Anthony Perard <anthony.perard@citrix.com>
>> Cc: Paul Durrant <paul@xen.org>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> Still one comment below:
> 
>> @@ -3014,8 +3044,11 @@ static void listener_add_address_space(MemoryListener *listener,
>>           listener->begin(listener);
>>       }
>>       if (global_dirty_tracking) {
>> +        /*
>> +         * Migration has already started. Assert on any error.
> 
> If you won't mind, I can change this to:
> 
>    /*
>     * Currently only VFIO can fail log_global_start(), and it's not allowed
>     * to hotplug a VFIO device during migration, so this should never fail
>     * when invoked.  If it can start to fail in the future, we need to be
>     * able to fail the whole listener_add_address_space() and its callers.
>     */

Sure, or I will in a v6. Markus had a comment on 8/14.

Thanks,

C.