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
