[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank insta
From: |
Peter Xu |
Subject: |
Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances |
Date: |
Wed, 13 Sep 2023 16:13:44 -0400 |
On Mon, Sep 11, 2023 at 02:13:19PM -0300, Fabiano Rosas wrote:
> The core yank code is strict about balanced registering and
> unregistering of yank functions.
>
> This creates a difficulty because the migration code registers one
> yank function per QIOChannel, but each QIOChannel can be referenced by
> more than one QEMUFile. The yank function should not be removed until
> all QEMUFiles have been closed.
>
> Keep a reference count of how many QEMUFiles are using a QIOChannel
> that has a yank function. Only unregister the yank function when all
> QEMUFiles have been closed.
>
> This improves the current code by removing the need for the programmer
> to know which QEMUFile is the last one to be cleaned up and fixes the
> theoretical issue of removing the yank function while another QEMUFile
> could still be using the ioc and require a yank.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
> migration/yank_functions.c | 81 ++++++++++++++++++++++++++++++++++----
> migration/yank_functions.h | 8 ++++
> 2 files changed, 81 insertions(+), 8 deletions(-)
I worry this over-complicate things.
If you prefer the cleaness that we operate always on qemufile level, can we
just register each yank function per-qemufile? I think qmp yank will
simply fail the 2nd call on the qemufile if the iochannel is shared with
the other one, but that's totally fine, IMHO.
What do you think?
In all cases, we should probably at least merge patch 1-8 if that can
resolve the CI issue. I think all of them are properly reviewed.
Thanks,
--
Peter Xu
- [PATCH v6 01/10] migration: Fix possible race when setting rp_state.error, (continued)
- [PATCH v6 01/10] migration: Fix possible race when setting rp_state.error, Fabiano Rosas, 2023/09/11
- [PATCH v6 05/10] migration: Consolidate return path closing code, Fabiano Rosas, 2023/09/11
- [PATCH v6 03/10] migration: Fix possible race when shutting down to_dst_file, Fabiano Rosas, 2023/09/11
- [PATCH v6 06/10] migration: Replace the return path retry logic, Fabiano Rosas, 2023/09/11
- [PATCH v6 07/10] migration: Move return path cleanup to main migration thread, Fabiano Rosas, 2023/09/11
- [PATCH v6 08/10] migration/yank: Use channel features, Fabiano Rosas, 2023/09/11
- [PATCH v6 04/10] migration: Remove redundant cleanup of postcopy_qemufile_src, Fabiano Rosas, 2023/09/11
- [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Fabiano Rosas, 2023/09/11
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances,
Peter Xu <=
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Fabiano Rosas, 2023/09/13
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Peter Xu, 2023/09/13
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Fabiano Rosas, 2023/09/14
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Peter Xu, 2023/09/14
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Lukas Straub, 2023/09/25
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Fabiano Rosas, 2023/09/25
- Re: [PATCH v6 09/10] migration/yank: Keep track of registered yank instances, Lukas Straub, 2023/09/25
[PATCH v6 10/10] migration: Add a wrapper to cleanup migration files, Fabiano Rosas, 2023/09/11