Re: [PATCH v2 1/2] qapi/migration.json: add reason to MIGRATION event

[Peter Xu <peterx@redhat.com>]

On Wed, Feb 21, 2024 at 06:47:47PM +0500, Roman Khapov wrote:
> If I understood you right, you suggest to improve migrate_generate_event to
> accept MigrationState* instead of int* state (which is pointing to field
> MigrationState.state in all usages), and get error reason from
> MigrationState.error, if the new state is MIGRATION_STATE_FAILED, is it?

migrate_generate_event() is a migration internal function, it can directly
reference s->error with error_mutex held.

> 
> That sounds reasonable, thanks!
> 
> But I'm not sure if I got the idea of changing migrate_set_error correctly,
> can you explain in more details, please?

I fat-fingered.. sorry.  I wanted to say migrate_set_state() below, and I
think migrate_set_state() can be kept untouched.

But please consider the other reviewer's comment first: if query-migrate
(or HMP "info migrate") works for you, then this interface is not needed.

> 
> On 2/20/24 10:39, Peter Xu wrote:
> > On Thu, Feb 15, 2024 at 05:27:58PM +0500, Roman Khapov wrote:
> > >       migrate_set_state(&mis->state, MIGRATION_STATUS_COLO,
> > > -                      MIGRATION_STATUS_COMPLETED);
> > > +                      MIGRATION_STATUS_COMPLETED, NULL);
> > Instead of enforcing migrate_set_error() to always pass an error, maybe we
> > can allow migrate_generate_event() to fetch s->error in FAILED state, if
> > that hint ever existed?
> > 
> -- 
> Best regards,
> Roman Khapov
> 

-- 
Peter Xu