On 28.02.2025 10:13, Cédric Le Goater wrote:
On 2/26/25 22:05, Maciej S. Szmigiero wrote:
On 26.02.2025 17:43, Cédric Le Goater wrote:
On 2/19/25 21:34, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>
Implement the multifd device state transfer via additional per-device
thread inside save_live_complete_precopy_thread handler.
Switch between doing the data transfer in the new handler and doing it
in the old save_state handler depending on the
x-migration-multifd-transfer device property value.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>
---
hw/vfio/migration-multifd.c | 139 ++++++++++++++++++++++++++++++++++
hw/vfio/migration-multifd.h | 5 ++
hw/vfio/migration.c | 26 +++++--
hw/vfio/trace-events | 2 +
include/hw/vfio/vfio-common.h | 8 ++
5 files changed, 174 insertions(+), 6 deletions(-)
diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index 7200f6f1c2a2..0cfa9d31732a 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
(..)
+{
+ VFIODevice *vbasedev = d->handler_opaque;
+ VFIOMigration *migration = vbasedev->migration;
+ bool ret;
+ g_autofree VFIODeviceStatePacket *packet = NULL;
+ uint32_t idx;
+
+ if (!vfio_multifd_transfer_enabled(vbasedev)) {
+ /* Nothing to do, vfio_save_complete_precopy() does the transfer. */
+ return true;
+ }
+
+ trace_vfio_save_complete_precopy_thread_start(vbasedev->name,
+ d->idstr, d->instance_id);
+
+ /* We reach here with device state STOP or STOP_COPY only */
+ if (vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
+ VFIO_DEVICE_STATE_STOP, errp)) {
+ ret = false;
These "ret = false" can be avoided if the variable is set at the
top of the function.
I inverted the "ret" logic here as in vfio_load_bufs_thread()
to make it false by default and set to true just before early
exit label.
ok. Let's see what it looks like in v6.
+ goto ret_finish;
goto thread_exit ?
As I asked in one of the previous patches,
do this comment mean that your want to rename ret_finish 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_finish to thread_exit then.
(..)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index b962309f7c27..69dcf2dac2fa 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
(..)
@@ -238,8 +238,7 @@ static int vfio_load_buffer(QEMUFile *f, VFIODevice
*vbasedev,
return ret;
}
-static int vfio_save_device_config_state(QEMUFile *f, void *opaque,
- Error **errp)
+int vfio_save_device_config_state(QEMUFile *f, void *opaque, Error **errp)
{
VFIODevice *vbasedev = opaque;
int ret;
@@ -453,6 +452,10 @@ static int vfio_save_setup(QEMUFile *f, void *opaque,
Error **errp)
uint64_t stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
int ret;
+ if (!vfio_multifd_transfer_setup(vbasedev, errp)) {
+ return -EINVAL;
+ }
+
please move to another patch with the similar change of patch 25.
This patch is about the send/save side while patch 25
is called "*receive* init/cleanup".
So adding save setup to patch called "receive init" wouldn't be
consistent with that patch subject.
In that case, could please add an extra patch checking for the consistency
of the settings ?
I split out wiring vfio_multifd_setup() and vfio_multifd_cleanup() into
general VFIO load/save setup and cleanup methods from this patch and
patch "Multifd device state transfer support - receive init/cleanup"
into a brand new patch/commit.
By the way, due to changes discussed over the last two days
vfio_multifd_setup() (aka vfio_multifd_transfer_setup()) not only
does consistency checking but also allocates VFIOMultifd:
https://lore.kernel.org/qemu-devel/6546c3a4-bd81-42ea-88a2-b2f88ec2f...@maciej.szmigiero.name/
Thanks,
C.
Thanks,
Maciej
