On 6.03.2025 15:26, Cédric Le Goater wrote:
On 3/6/25 15:23, Avihai Horon wrote:

On 06/03/2025 16:13, Maciej S. Szmigiero wrote:
External email: Use caution opening links or attachments


On 6.03.2025 14:37, Avihai Horon wrote:

On 06/03/2025 12:32, Cédric Le Goater wrote:
External email: Use caution opening links or attachments


On 3/6/25 11:15, Maciej S. Szmigiero wrote:
On 6.03.2025 07:47, Avihai Horon wrote:

On 05/03/2025 0:03, Maciej S. Szmigiero wrote:
External email: Use caution opening links or attachments


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 if VFIO multifd transfer is enabled
or not.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigi...@oracle.com>
---
  hw/vfio/migration-multifd.c   | 142 ++++++++++++++++++++++++++++++++++
  hw/vfio/migration-multifd.h   |   6 ++
  hw/vfio/migration.c           |  22 ++++--
  hw/vfio/trace-events          |   2 +
  include/hw/vfio/vfio-common.h |   6 ++
  5 files changed, 172 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index 1d81233c755f..bfb9a72fa450 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -496,6 +496,148 @@ bool vfio_multifd_setup(VFIODevice *vbasedev, bool 
alloc_multifd, Error **errp)
      return true;
  }

+void vfio_multifd_emit_dummy_eos(VFIODevice *vbasedev, QEMUFile *f)
+{
+    assert(vfio_multifd_transfer_enabled(vbasedev));
+
+    /*
+     * Emit dummy NOP data on the main migration channel since the actual
+     * device state transfer is done via multifd channels.
+     */
+    qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+}
+
+static bool
+vfio_save_complete_precopy_thread_config_state(VFIODevice *vbasedev,
+                                               char *idstr,
+                                               uint32_t instance_id,
+                                               uint32_t idx,
+                                               Error **errp)
+{
+    g_autoptr(QIOChannelBuffer) bioc = NULL;
+    g_autoptr(QEMUFile) f = NULL;
+    int ret;
+    g_autofree VFIODeviceStatePacket *packet = NULL;
+    size_t packet_len;
+
+    bioc = qio_channel_buffer_new(0);
+    qio_channel_set_name(QIO_CHANNEL(bioc), "vfio-device-config-save");
+
+    f = qemu_file_new_output(QIO_CHANNEL(bioc));
+
+    if (vfio_save_device_config_state(f, vbasedev, errp)) {
+        return false;
+    }
+
+    ret = qemu_fflush(f);
+    if (ret) {
+        error_setg(errp, "%s: save config state flush failed: %d",
+                   vbasedev->name, ret);
+        return false;
+    }
+
+    packet_len = sizeof(*packet) + bioc->usage;
+    packet = g_malloc0(packet_len);
+    packet->version = VFIO_DEVICE_STATE_PACKET_VER_CURRENT;
+    packet->idx = idx;
+    packet->flags = VFIO_DEVICE_STATE_CONFIG_STATE;

The packet is sent on the wire.
Shouldn't we use cpu_to_be32() for version, idx and flags? Also below in 
vfio_multifd_save_complete_precopy_thread().
And then use be32_to_cpu() in patch #26 when receiving the packet?

Is it even possible to migrate to a host with different endianess here?

Also AFAIK big endian hosts barely exist today, is any of them even 
VFIO-capable?

s390x is VFIO capable. VFIO PCI migration is not supported on these.

It is indeed a niche use case and not even applicable today, but if we want to 
add support for it after the release, we will have to add a compatibility 
option for older QEMUs.
If we add support for it now, then we can avoid the compatibility option.

It's a really small change and it can come even after the series is merged, as 
a fix.
So IMHO it wouldn't hurt, for completeness.

For sure, any such bit stream change will need re-testing the whole VFIO 
migration.

But I will be testing the queued buffers size limit anyway so it would make
sense to test both at the same time.

Wouldn't it make more sense, however, to squash this endianess change already
to the relevant patches rather than to have such bit stream modifying patch on 
the top?

It would help prevent backporting mistakes - when someone forgets about this 
last patch
and ends with a different bit stream.

I agree.
Whatever you and Cedric decide.


PR was sent. So it will be a "Fixes" patch.

I have posted both of these patches now.

Thanks,

C.

Thanks,
Maciej


Reply via email to