| Message ID | 20241007154451.107007-1-shivam.kumar1@nutanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use multifd state to determine if multifd cleanup is needed | expand |
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. 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? > 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.kumar1@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 >
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; }
If the client calls the QMP command to reset the migration capabilities after the migration status is set to failed or cancelled 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.kumar1@nutanix.com> --- migration/multifd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)