|
From: | Cédric Le Goater |
Subject: | Re: [PATCH 1/2] migration: Add a file_error argument to close_return_path_on_source() |
Date: | Fri, 2 Feb 2024 15:45:54 +0100 |
User-agent: | Mozilla Thunderbird |
On 2/2/24 15:30, Fabiano Rosas wrote:
Cédric Le Goater <clg@redhat.com> writes:close_return_path_on_source() retrieves the migration error from the the QEMUFile '->to_dst_file' to know if a shutdown is required to exit the return-path thread. However, in migrate_fd_cleanup(), '->to_dst_file' is cleaned up before calling close_return_path_on_source() and the shutdown is never performed, leaving the source and destination waiting for an event to occur.Isn't this just missing qemu_file_shutdown() at migrate_fd_cleanup? if (s->to_dst_file) { ... migration_ioc_unregister_yank_from_file(tmp); + qemu_file_shutdown(tmp); qemu_fclose(tmp); }
That would make the return-path thread exit indeed. It should not be necessary when there are no errors though and this is done outside of the close_return_path_on_source() helper. There could be side effects. I took into account Peter's comment and replaced the changes of PATCH 1 with : @@ -2372,8 +2372,7 @@ static bool close_return_path_on_source( * cause it to unblock if it's stuck waiting for the destination. */ WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) { - if (ms->to_dst_file && ms->rp_state.from_dst_file && - qemu_file_get_error(ms->to_dst_file)) { + if (migrate_has_error(ms) && ms->rp_state.from_dst_file) { qemu_file_shutdown(ms->rp_state.from_dst_file); } } Nevertheless, we need to qemu_file_shutdown() correctly the socket for this to work and the problem seems more complex than just moving code as I did in PATCH 2. Thanks, C.
[Prev in Thread] | Current Thread | [Next in Thread] |