* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> > 15.06.2018 15:06, Dr. David Alan Gilbert wrote:
> > > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
> > > > Invalidate cache before source start in case of failed migration.
> > > >
> > > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> > > Why doesn't the code at the bottom of migration_completion,
> > > fail_invalidate: and the code in migrate_fd_cancel handle this?
> > >
> > > What case did you see it in that those didn't handle?
> > > (Also I guess it probably should set s->block_inactive = false)
> >
> > on source I see:
> >
> > 81392@1529065750.766289:migrate_set_state new state 7
> > 81392@1529065750.766330:migration_thread_file_err
> > 81392@1529065750.766332:migration_thread_after_loop
> >
> > so, we are leaving loop on
> > if (qemu_file_get_error(s->to_dst_file)) {
> > migrate_set_state(&s->state, current_active_state,
> > MIGRATION_STATUS_FAILED);
> > trace_migration_thread_file_err();
> > break;
> > }
> >
> > and skip migration_completion()
>
> Yeh, OK; I'd seen soemthing else a few days ago, where a cancellation
> test that had previously ended with a 'cancelled' state has now ended up
> in 'failed' (which is the state 7 you have above).
> I suspect there's something else going on as well; I think what is
> supposed to happen in the case of 'cancel' is that it spins in 'cancelling'
> for
> a while in migrate_fd_cancel and then at the bottom of migrate_fd_cancel
> it does the recovery, but because it's going to failed instead, then
> it's jumping over that recovery.
Going back and actually looking at the patch again;
can I ask for 1 small change;
Can you set s->block_inactive = false in the case where you
don't get the local_err (Like we do at the bottom of migrate_fd_cancel)
Does that make sense?
Thanks,
Dave
> Dave
>
> >
> > >
> > > Dave
> > >
> > > > ---
> > > >
> > > > migration/migration.c | 9 ++++++++-
> > > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index 1e99ec9b7e..8f39e0dc02 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -2806,7 +2806,14 @@ static void
> > > > migration_iteration_finish(MigrationState *s)
> > > > case MIGRATION_STATUS_FAILED:
> > > > case MIGRATION_STATUS_CANCELLED:
> > > > if (s->vm_was_running) {
> > > > - vm_start();
> > > > + Error *local_err = NULL;
> > > > + bdrv_invalidate_cache_all(&local_err);
> > > > + if (local_err) {
> > > > + error_reportf_err(local_err, "Can't invalidate disks
> > > > before "
> > > > + "source vm start");
> > > > + } else {
> > > > + vm_start();
> > > > + }
> > > > } else {
> > > > if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
> > > > runstate_set(RUN_STATE_POSTMIGRATE);
> > > > --
> > > > 2.11.1
> > > >
> > > --
> > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> >
> >
> > --
> > Best regards,
> > Vladimir
> >
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
