Re: [PATCH v4 28/33] vfio/migration: Multifd device state transfer support - load thread
On 12.02.2025 17:19, Cédric Le Goater wrote:
On 2/12/25 16:48, Cédric Le Goater wrote:
On 1/30/25 11:08, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero"
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
---
hw/vfio/migration.c | 229 +++
hw/vfio/trace-events | 5 +
2 files changed, 234 insertions(+)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 0c0caec1bd64..ab5b097f59c9 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -301,8 +301,16 @@ typedef struct VFIOStateBuffer {
} VFIOStateBuffer;
typedef struct VFIOMultifd {
+ QemuThread load_bufs_thread;
+ bool load_bufs_thread_running;
+ bool load_bufs_thread_want_exit;
+
+ bool load_bufs_iter_done;
+ QemuCond load_bufs_iter_done_cond;
+
VFIOStateBuffers load_bufs;
QemuCond load_bufs_buffer_ready_cond;
+ QemuCond load_bufs_thread_finished_cond;
QemuMutex load_bufs_mutex; /* Lock order: this lock -> BQL */
uint32_t load_buf_idx;
uint32_t load_buf_idx_last;
@@ -449,6 +457,171 @@ static bool vfio_load_state_buffer(void *opaque, char
*data, size_t data_size,
return true;
}
+static VFIOStateBuffer *vfio_load_state_buffer_get(VFIOMultifd *multifd)
+{
+ VFIOStateBuffer *lb;
+ guint bufs_len;
+
+ bufs_len = vfio_state_buffers_size_get(&multifd->load_bufs);
+ if (multifd->load_buf_idx >= bufs_len) {
+ assert(multifd->load_buf_idx == bufs_len);
+ return NULL;
+ }
+
+ lb = vfio_state_buffers_at(&multifd->load_bufs,
+ multifd->load_buf_idx);
+ if (!lb->is_present) {
+ return NULL;
+ }
+
+ return lb;
+}
+
+static int vfio_load_bufs_thread_load_config(VFIODevice *vbasedev)
+{
+ return -EINVAL;
+}
+
+static bool vfio_load_state_buffer_write(VFIODevice *vbasedev,
+ VFIOStateBuffer *lb,
+ Error **errp)
+{
+ VFIOMigration *migration = vbasedev->migration;
+ VFIOMultifd *multifd = migration->multifd;
+ g_autofree char *buf = NULL;
+ char *buf_cur;
+ size_t buf_len;
+
+ if (!lb->len) {
+ return true;
+ }
+
+ trace_vfio_load_state_device_buffer_load_start(vbasedev->name,
+ multifd->load_buf_idx);
+
+ /* lb might become re-allocated when we drop the lock */
+ buf = g_steal_pointer(&lb->data);
+ buf_cur = buf;
+ buf_len = lb->len;
+ while (buf_len > 0) {
+ ssize_t wr_ret;
+ int errno_save;
+
+ /*
+ * Loading data to the device takes a while,
+ * drop the lock during this process.
+ */
+ qemu_mutex_unlock(&multifd->load_bufs_mutex);
+ wr_ret = write(migration->data_fd, buf_cur, buf_len);
+ errno_save = errno;
+ qemu_mutex_lock(&multifd->load_bufs_mutex);
+
+ if (wr_ret < 0) {
+ error_setg(errp,
+ "writing state buffer %" PRIu32 " failed: %d",
+ multifd->load_buf_idx, errno_save);
+ return false;
+ }
+
+ assert(wr_ret <= buf_len);
+ buf_len -= wr_ret;
+ buf_cur += wr_ret;
+ }
+
+ trace_vfio_load_state_device_buffer_load_end(vbasedev->name,
+ multifd->load_buf_idx);
+
+ return true;
+}
+
+static bool vfio_load_bufs_thread_want_abort(VFIOMultifd *multifd,
+ bool *should_quit)
+{
+ return multifd->load_bufs_thread_want_exit || qatomic_read(should_quit);
+}
_abort or _exit or _quit ? I would opt for vfio_load_bufs_thread_want_exit()
to match multifd->load_bufs_thread_want_exit.
+static bool vfio_load_bufs_thread(void *opaque, bool *should_quit, Error
**errp)
+{
+ VFIODevice *vbasedev = opaque;
+ VFIOMigration *migration = vbasedev->migration;
+ VFIOMultifd *multifd = migration->multifd;
+ bool ret = true;
+ int config_ret;
+
+ assert(multifd);
+ QEMU_LOCK_GUARD(&multifd->load_bufs_mutex);
+
+
Re: [PATCH v4 28/33] vfio/migration: Multifd device state transfer support - load thread
On 12.02.2025 16:48, Cédric Le Goater wrote:
On 1/30/25 11:08, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero"
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
---
hw/vfio/migration.c | 229 +++
hw/vfio/trace-events | 5 +
2 files changed, 234 insertions(+)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 0c0caec1bd64..ab5b097f59c9 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -301,8 +301,16 @@ typedef struct VFIOStateBuffer {
} VFIOStateBuffer;
typedef struct VFIOMultifd {
+ QemuThread load_bufs_thread;
+ bool load_bufs_thread_running;
+ bool load_bufs_thread_want_exit;
+
+ bool load_bufs_iter_done;
+ QemuCond load_bufs_iter_done_cond;
+
VFIOStateBuffers load_bufs;
QemuCond load_bufs_buffer_ready_cond;
+ QemuCond load_bufs_thread_finished_cond;
QemuMutex load_bufs_mutex; /* Lock order: this lock -> BQL */
uint32_t load_buf_idx;
uint32_t load_buf_idx_last;
@@ -449,6 +457,171 @@ static bool vfio_load_state_buffer(void *opaque, char
*data, size_t data_size,
return true;
}
+static VFIOStateBuffer *vfio_load_state_buffer_get(VFIOMultifd *multifd)
+{
+ VFIOStateBuffer *lb;
+ guint bufs_len;
+
+ bufs_len = vfio_state_buffers_size_get(&multifd->load_bufs);
+ if (multifd->load_buf_idx >= bufs_len) {
+ assert(multifd->load_buf_idx == bufs_len);
+ return NULL;
+ }
+
+ lb = vfio_state_buffers_at(&multifd->load_bufs,
+ multifd->load_buf_idx);
+ if (!lb->is_present) {
+ return NULL;
+ }
+
+ return lb;
+}
+
+static int vfio_load_bufs_thread_load_config(VFIODevice *vbasedev)
+{
+ return -EINVAL;
+}
+
+static bool vfio_load_state_buffer_write(VFIODevice *vbasedev,
+ VFIOStateBuffer *lb,
+ Error **errp)
+{
+ VFIOMigration *migration = vbasedev->migration;
+ VFIOMultifd *multifd = migration->multifd;
+ g_autofree char *buf = NULL;
+ char *buf_cur;
+ size_t buf_len;
+
+ if (!lb->len) {
+ return true;
+ }
+
+ trace_vfio_load_state_device_buffer_load_start(vbasedev->name,
+ multifd->load_buf_idx);
+
+ /* lb might become re-allocated when we drop the lock */
+ buf = g_steal_pointer(&lb->data);
+ buf_cur = buf;
+ buf_len = lb->len;
+ while (buf_len > 0) {
+ ssize_t wr_ret;
+ int errno_save;
+
+ /*
+ * Loading data to the device takes a while,
+ * drop the lock during this process.
+ */
+ qemu_mutex_unlock(&multifd->load_bufs_mutex);
+ wr_ret = write(migration->data_fd, buf_cur, buf_len);
+ errno_save = errno;
+ qemu_mutex_lock(&multifd->load_bufs_mutex);
+
+ if (wr_ret < 0) {
+ error_setg(errp,
+ "writing state buffer %" PRIu32 " failed: %d",
+ multifd->load_buf_idx, errno_save);
+ return false;
+ }
+
+ assert(wr_ret <= buf_len);
+ buf_len -= wr_ret;
+ buf_cur += wr_ret;
+ }
+
+ trace_vfio_load_state_device_buffer_load_end(vbasedev->name,
+ multifd->load_buf_idx);
+
+ return true;
+}
+
+static bool vfio_load_bufs_thread_want_abort(VFIOMultifd *multifd,
+ bool *should_quit)
+{
+ return multifd->load_bufs_thread_want_exit || qatomic_read(should_quit);
+}
_abort or _exit or _quit ? I would opt for vfio_load_bufs_thread_want_exit()
to match multifd->load_bufs_thread_want_exit.
Will rename to vfio_load_bufs_thread_want_exit().
+static bool vfio_load_bufs_thread(void *opaque, bool *should_quit, Error
**errp)
+{
+ VFIODevice *vbasedev = opaque;
+ VFIOMigration *migration = vbasedev->migration;
+ VFIOMultifd *multifd = migration->multifd;
+ bool ret = true;
+ int config_ret;
+
+ assert(multifd);
+ QEMU_LOCK_GUARD(&multifd->load_bufs_mutex);
+
+
Re: [PATCH v4 28/33] vfio/migration: Multifd device state transfer support - load thread
On 2/12/25 16:48, Cédric Le Goater wrote:
On 1/30/25 11:08, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero"
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
---
hw/vfio/migration.c | 229 +++
hw/vfio/trace-events | 5 +
2 files changed, 234 insertions(+)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 0c0caec1bd64..ab5b097f59c9 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -301,8 +301,16 @@ typedef struct VFIOStateBuffer {
} VFIOStateBuffer;
typedef struct VFIOMultifd {
+ QemuThread load_bufs_thread;
+ bool load_bufs_thread_running;
+ bool load_bufs_thread_want_exit;
+
+ bool load_bufs_iter_done;
+ QemuCond load_bufs_iter_done_cond;
+
VFIOStateBuffers load_bufs;
QemuCond load_bufs_buffer_ready_cond;
+ QemuCond load_bufs_thread_finished_cond;
QemuMutex load_bufs_mutex; /* Lock order: this lock -> BQL */
uint32_t load_buf_idx;
uint32_t load_buf_idx_last;
@@ -449,6 +457,171 @@ static bool vfio_load_state_buffer(void *opaque, char
*data, size_t data_size,
return true;
}
+static VFIOStateBuffer *vfio_load_state_buffer_get(VFIOMultifd *multifd)
+{
+ VFIOStateBuffer *lb;
+ guint bufs_len;
+
+ bufs_len = vfio_state_buffers_size_get(&multifd->load_bufs);
+ if (multifd->load_buf_idx >= bufs_len) {
+ assert(multifd->load_buf_idx == bufs_len);
+ return NULL;
+ }
+
+ lb = vfio_state_buffers_at(&multifd->load_bufs,
+ multifd->load_buf_idx);
+ if (!lb->is_present) {
+ return NULL;
+ }
+
+ return lb;
+}
+
+static int vfio_load_bufs_thread_load_config(VFIODevice *vbasedev)
+{
+ return -EINVAL;
+}
+
+static bool vfio_load_state_buffer_write(VFIODevice *vbasedev,
+ VFIOStateBuffer *lb,
+ Error **errp)
+{
+ VFIOMigration *migration = vbasedev->migration;
+ VFIOMultifd *multifd = migration->multifd;
+ g_autofree char *buf = NULL;
+ char *buf_cur;
+ size_t buf_len;
+
+ if (!lb->len) {
+ return true;
+ }
+
+ trace_vfio_load_state_device_buffer_load_start(vbasedev->name,
+ multifd->load_buf_idx);
+
+ /* lb might become re-allocated when we drop the lock */
+ buf = g_steal_pointer(&lb->data);
+ buf_cur = buf;
+ buf_len = lb->len;
+ while (buf_len > 0) {
+ ssize_t wr_ret;
+ int errno_save;
+
+ /*
+ * Loading data to the device takes a while,
+ * drop the lock during this process.
+ */
+ qemu_mutex_unlock(&multifd->load_bufs_mutex);
+ wr_ret = write(migration->data_fd, buf_cur, buf_len);
+ errno_save = errno;
+ qemu_mutex_lock(&multifd->load_bufs_mutex);
+
+ if (wr_ret < 0) {
+ error_setg(errp,
+ "writing state buffer %" PRIu32 " failed: %d",
+ multifd->load_buf_idx, errno_save);
+ return false;
+ }
+
+ assert(wr_ret <= buf_len);
+ buf_len -= wr_ret;
+ buf_cur += wr_ret;
+ }
+
+ trace_vfio_load_state_device_buffer_load_end(vbasedev->name,
+ multifd->load_buf_idx);
+
+ return true;
+}
+
+static bool vfio_load_bufs_thread_want_abort(VFIOMultifd *multifd,
+ bool *should_quit)
+{
+ return multifd->load_bufs_thread_want_exit || qatomic_read(should_quit);
+}
_abort or _exit or _quit ? I would opt for vfio_load_bufs_thread_want_exit()
to match multifd->load_bufs_thread_want_exit.
+static bool vfio_load_bufs_thread(void *opaque, bool *should_quit, Error
**errp)
+{
+ VFIODevice *vbasedev = opaque;
+ VFIOMigration *migration = vbasedev->migration;
+ VFIOMultifd *multifd = migration->multifd;
+ bool ret = true;
+ int config_ret;
+
+ assert(multifd);
+ QEMU_LOCK_GUARD(&multifd->load_bufs_mutex);
+
+ assert(multifd->load_bufs_thread_running);
+
+ while
Re: [PATCH v4 28/33] vfio/migration: Multifd device state transfer support - load thread
On 1/30/25 11:08, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero"
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
---
hw/vfio/migration.c | 229 +++
hw/vfio/trace-events | 5 +
2 files changed, 234 insertions(+)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 0c0caec1bd64..ab5b097f59c9 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -301,8 +301,16 @@ typedef struct VFIOStateBuffer {
} VFIOStateBuffer;
typedef struct VFIOMultifd {
+QemuThread load_bufs_thread;
+bool load_bufs_thread_running;
+bool load_bufs_thread_want_exit;
+
+bool load_bufs_iter_done;
+QemuCond load_bufs_iter_done_cond;
+
VFIOStateBuffers load_bufs;
QemuCond load_bufs_buffer_ready_cond;
+QemuCond load_bufs_thread_finished_cond;
QemuMutex load_bufs_mutex; /* Lock order: this lock -> BQL */
uint32_t load_buf_idx;
uint32_t load_buf_idx_last;
@@ -449,6 +457,171 @@ static bool vfio_load_state_buffer(void *opaque, char
*data, size_t data_size,
return true;
}
+static VFIOStateBuffer *vfio_load_state_buffer_get(VFIOMultifd *multifd)
+{
+VFIOStateBuffer *lb;
+guint bufs_len;
+
+bufs_len = vfio_state_buffers_size_get(&multifd->load_bufs);
+if (multifd->load_buf_idx >= bufs_len) {
+assert(multifd->load_buf_idx == bufs_len);
+return NULL;
+}
+
+lb = vfio_state_buffers_at(&multifd->load_bufs,
+ multifd->load_buf_idx);
+if (!lb->is_present) {
+return NULL;
+}
+
+return lb;
+}
+
+static int vfio_load_bufs_thread_load_config(VFIODevice *vbasedev)
+{
+return -EINVAL;
+}
+
+static bool vfio_load_state_buffer_write(VFIODevice *vbasedev,
+ VFIOStateBuffer *lb,
+ Error **errp)
+{
+VFIOMigration *migration = vbasedev->migration;
+VFIOMultifd *multifd = migration->multifd;
+g_autofree char *buf = NULL;
+char *buf_cur;
+size_t buf_len;
+
+if (!lb->len) {
+return true;
+}
+
+trace_vfio_load_state_device_buffer_load_start(vbasedev->name,
+ multifd->load_buf_idx);
+
+/* lb might become re-allocated when we drop the lock */
+buf = g_steal_pointer(&lb->data);
+buf_cur = buf;
+buf_len = lb->len;
+while (buf_len > 0) {
+ssize_t wr_ret;
+int errno_save;
+
+/*
+ * Loading data to the device takes a while,
+ * drop the lock during this process.
+ */
+qemu_mutex_unlock(&multifd->load_bufs_mutex);
+wr_ret = write(migration->data_fd, buf_cur, buf_len);
+errno_save = errno;
+qemu_mutex_lock(&multifd->load_bufs_mutex);
+
+if (wr_ret < 0) {
+error_setg(errp,
+ "writing state buffer %" PRIu32 " failed: %d",
+ multifd->load_buf_idx, errno_save);
+return false;
+}
+
+assert(wr_ret <= buf_len);
+buf_len -= wr_ret;
+buf_cur += wr_ret;
+}
+
+trace_vfio_load_state_device_buffer_load_end(vbasedev->name,
+ multifd->load_buf_idx);
+
+return true;
+}
+
+static bool vfio_load_bufs_thread_want_abort(VFIOMultifd *multifd,
+ bool *should_quit)
+{
+return multifd->load_bufs_thread_want_exit || qatomic_read(should_quit);
+}
_abort or _exit or _quit ? I would opt for vfio_load_bufs_thread_want_exit()
to match multifd->load_bufs_thread_want_exit.
+static bool vfio_load_bufs_thread(void *opaque, bool *should_quit, Error
**errp)
+{
+VFIODevice *vbasedev = opaque;
+VFIOMigration *migration = vbasedev->migration;
+VFIOMultifd *multifd = migration->multifd;
+bool ret = true;
+int config_ret;
+
+assert(multifd);
+QEMU_LOCK_GUARD(&multifd->load_bufs_mutex);
+
+assert(multifd->load_bufs_thread_running);
+
+while (!vfio_load_bufs_thread_want_abort(multifd,
