On 28.02.2025 10:11, Cédric Le Goater wrote:
On 2/26/25 22:05, Maciej S. Szmigiero wrote:
On 26.02.2025 14:49, Cédric Le Goater wrote:
On 2/19/25 21:34, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>
Since it's important to finish loading device state transferred via the
main migration channel (via save_live_iterate SaveVMHandler) before
starting loading the data asynchronously transferred via multifd the thread
doing the actual loading of the multifd transferred data is only started
from switchover_start SaveVMHandler.
switchover_start handler is called when MIG_CMD_SWITCHOVER_START
sub-command of QEMU_VM_COMMAND is received via the main migration channel.
This sub-command is only sent after all save_live_iterate data have already
been posted so it is safe to commence loading of the multifd-transferred
device state upon receiving it - loading of save_live_iterate data happens
synchronously in the main migration thread (much like the processing of
MIG_CMD_SWITCHOVER_START) so by the time MIG_CMD_SWITCHOVER_START is
processed all the proceeding data must have already been loaded.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>
---
hw/vfio/migration-multifd.c | 225 ++++++++++++++++++++++++++++++++++++
hw/vfio/migration-multifd.h | 2 +
hw/vfio/migration.c | 12 ++
hw/vfio/trace-events | 5 +
4 files changed, 244 insertions(+)
diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index 5d5ee1393674..b3a88c062769 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
(..)
+ while (true) {
+ VFIOStateBuffer *lb;
+
+ /*
+ * Always check cancellation first after the buffer_ready wait below in
+ * case that cond was signalled by
vfio_load_cleanup_load_bufs_thread().
+ */
+ if (vfio_load_bufs_thread_want_exit(multifd, should_quit)) {
+ error_setg(errp, "operation cancelled");
+ ret = false;
+ goto ret_signal;
goto thread_exit ?
I'm not sure that I fully understand this comment.
Do you mean to rename ret_signal label to thread_exit?
Yes. I find label 'thread_exit' more meaning full. This is minor since
there is only one 'exit' label.
Renamed ret_signal to thread_exit then.
(..)
+ config_ret = vfio_load_bufs_thread_load_config(vbasedev);
+ if (config_ret) {
+ error_setg(errp, "load config state failed: %d", config_ret);
+ ret = false;
+ }
please move to next patch. This is adding nothing to this patch
since it's returning -EINVAL.
That's the whole point - if someone were to accidentally enable this
(for example by forgetting to apply the next patch when backporting
the series) it would fail safely with EINVAL instead of having a
half-broken implementation.
OK. Let's keep it that way.
Thanks,
C.
Thanks,
Maciej
