| Message ID | 20241206230838.1111496-6-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | migration/block: disk activation rewrite | expand |
Peter Xu <peterx@redhat.com> writes: > Src QEMU sets block_inactive=true very early before the invalidation takes > place. It means if something wrong happened during setting the flag but > before reaching qemu_savevm_state_complete_precopy_non_iterable() where it > did the invalidation work, it'll make block_inactive flag inconsistent. > > For example, think about when qemu_savevm_state_complete_precopy_iterable() > can fail: it will have block_inactive set to true even if all block drives > are active. > > Fix that by only update the flag after the invalidation is done. > > No Fixes for any commit, because it's not an issue if bdrv_activate_all() > is re-entrant upon all-active disks - false positive block_inactive can > bring nothing more than "trying to active the blocks but they're already > active". However let's still do it right to avoid the inconsistent flag > v.s. reality. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de>
diff --git a/migration/migration.c b/migration/migration.c index e6db9cfc50..ba5deec5bc 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2749,14 +2749,11 @@ static int migration_completion_precopy(MigrationState *s, goto out_unlock; } - /* - * Inactivate disks except in COLO, and track that we have done so in order - * to remember to reactivate them if migration fails or is cancelled. - */ - s->block_inactive = !migrate_colo(); migration_rate_set(RATE_LIMIT_DISABLED); + + /* Inactivate disks except in COLO */ ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false, - s->block_inactive); + !migrate_colo()); out_unlock: bql_unlock(); return ret; diff --git a/migration/savevm.c b/migration/savevm.c index 80726726fe..706b77ffab 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1558,6 +1558,8 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, qemu_file_set_error(f, ret); return ret; } + /* Remember that we did this */ + s->block_inactive = true; } if (!in_postcopy) { /* Postcopy stream will still be going */
Src QEMU sets block_inactive=true very early before the invalidation takes place. It means if something wrong happened during setting the flag but before reaching qemu_savevm_state_complete_precopy_non_iterable() where it did the invalidation work, it'll make block_inactive flag inconsistent. For example, think about when qemu_savevm_state_complete_precopy_iterable() can fail: it will have block_inactive set to true even if all block drives are active. Fix that by only update the flag after the invalidation is done. No Fixes for any commit, because it's not an issue if bdrv_activate_all() is re-entrant upon all-active disks - false positive block_inactive can bring nothing more than "trying to active the blocks but they're already active". However let's still do it right to avoid the inconsistent flag v.s. reality. Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration.c | 9 +++------ migration/savevm.c | 2 ++ 2 files changed, 5 insertions(+), 6 deletions(-)