On Thu, Aug 01, 2024 at 02:41:01PM -0300, Fabiano Rosas wrote:
> When a channel fails to create, the code currently just returns. This
> is wrong for two reasons:
>
> 1) Channel n+1 will not get to initialize it's semaphores, leading to
> an assert when terminate_threads tries to post to it:
>
> qemu-system-x86_64: ../util/qemu-thread-posix.c:92:
> qemu_mutex_lock_impl: Assertion `mutex->initialized' failed.
>
> 2) (theoretical) If channel n-1 already started creation it will
> defeat the purpose of the channels_created logic which is in place
> to avoid migrate_fd_cleanup() to run while channels are still being
> created.
>
> This cannot really happen today because the current failure cases
> for multifd_new_send_channel_create() are all synchronous,
> resulting from qio_channel_file_new_path() getting a bad
> filename. This would hit all channels equally.
>
> But I don't want to set a trap for future people, so have all
> channels try to create (even if failing), and only fail after the
> channels_created semaphore has been posted.
>
> While here, remove the error_report_err call. There's one already at
> migrate_fd_cleanup later on.
>
> Cc: qemu-sta...@nongnu.org
> Reported-by: Jim Fehlig <jfeh...@suse.com>
> Fixes: bd8b0a8f82 ("migration/multifd: Move multifd_send_setup error handling
> in to the function")
Should it be this one instead?
b7b03eb614 ("migration/multifd: Add outgoing QIOChannelFile support")
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
Reviewed-by: Peter Xu <pet...@redhat.com>
PS: what's your plan on your other multifd SendData series? I got a bit
overloaded on downstream stuff and I still have plenty review debts
recently (CPR one of them.. needs follow ups), so just to say I may delay a
bit on reading that one. I assume it's next-release stuff anyway, but let
me know otherwise.
Thanks,
--
Peter Xu
