On 7 Oct 2024, at 9:56 PM, Peter Xu <pet...@redhat.com> wrote:

!-------------------------------------------------------------------|
 CAUTION: External Email

|-------------------------------------------------------------------!

On Mon, Oct 07, 2024 at 03:44:51PM +0000, Shivam Kumar wrote:
If the client calls the QMP command to reset the migration
capabilities after the migration status is set to failed or cancelled

Is cancelled ok?

Asked because I think migrate_fd_cleanup() should still be in CANCELLING
stage there, so no one can disable multifd capability before that, it
should fail the QMP command.
I meant CANCELLED but I can see that currently, CANCELLED is only possible
after migrate_fd_cleanup is called. So, you are right. We won’t have a problem
in that path at least.

But FAILED indeed looks problematic.

IIUC it's not only to multifd alone - is it a race condition that
migrate_fd_cleanup() can be invoked without migration_is_running() keeps
being true?  Then I wonder what happens if a concurrent QMP "migrate"
happens together with migrate_fd_cleanup(), even with multifd always off.

Do we perhaps need to cleanup everything before the state changes to
FAILED?
Tried calling migrate_fd_cleanup before (and just after) setting the status to
failed. The migration thread gets stuck in close_return_path_on_source trying
to join rp_thread.
but before multifd cleanup starts, multifd cleanup can be skipped as
it will falsely assume that multifd was not used for migration. This
will eventually lead to source QEMU crashing due to the following
assertion failure:

yank_unregister_instance: Assertion `QLIST_EMPTY(&entry->yankfns)`
failed

Check multifd state to determine whether multifd was used or not for
the migration rather than checking the state of multifd migration
capability.

Signed-off-by: Shivam Kumar <shivam.kum...@nutanix.com>
---
migration/multifd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 9b200f4ad9..427c9a7956 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -487,7 +487,7 @@ void multifd_send_shutdown(void)
{
    int i;

-    if (!migrate_multifd()) {
+    if (!multifd_send_state) {
        return;
    }

--
2.22.3


--
Peter Xu

Reply via email to