migration: Cleanup migrate_fd_cleanup() on accessing to_dst_file

The cleanup function can in many cases needs cleanup on its own.

The major thing we want to do here is not referencing to_dst_file when
without the file mutex.  When at it, touch things elsewhere too to make it
look slightly better in general.

One thing to mention is, migration_thread has its own "running" boolean, so
it doesn't need to rely on to_dst_file being non-NULL.  Multifd has a
dependency so it needs to be skipped if to_dst_file is not yet set; add a
richer comment for such reason.

Resolves: Coverity CID 1527402
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240919163042.116767-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
This commit is contained in:
Peter Xu 2024-09-19 12:30:42 -04:00
parent ea8ae47bdd
commit 6dd4f44c4f

View File

@ -1405,6 +1405,9 @@ void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
static void migrate_fd_cleanup(MigrationState *s)
{
MigrationEventType type;
QEMUFile *tmp = NULL;
trace_migrate_fd_cleanup();
g_free(s->hostname);
s->hostname = NULL;
@ -1415,26 +1418,29 @@ static void migrate_fd_cleanup(MigrationState *s)
close_return_path_on_source(s);
if (s->to_dst_file) {
QEMUFile *tmp;
trace_migrate_fd_cleanup();
if (s->migration_thread_running) {
bql_unlock();
if (s->migration_thread_running) {
qemu_thread_join(&s->thread);
s->migration_thread_running = false;
}
qemu_thread_join(&s->thread);
s->migration_thread_running = false;
bql_lock();
}
multifd_send_shutdown();
qemu_mutex_lock(&s->qemu_file_lock);
WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
/*
* Close the file handle without the lock to make sure the critical
* section won't block for long.
*/
tmp = s->to_dst_file;
s->to_dst_file = NULL;
qemu_mutex_unlock(&s->qemu_file_lock);
}
if (tmp) {
/*
* Close the file handle without the lock to make sure the
* critical section won't block for long.
* We only need to shutdown multifd if tmp!=NULL, because if
* tmp==NULL, it means the main channel isn't established, while
* multifd is only setup after that (in migration_thread()).
*/
multifd_send_shutdown();
migration_ioc_unregister_yank_from_file(tmp);
qemu_fclose(tmp);
}