qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 24/24] vfio/migration: Multifd device state transfer suppo


From: Maciej S. Szmigiero
Subject: Re: [PATCH v3 24/24] vfio/migration: Multifd device state transfer support - send side
Date: Thu, 12 Dec 2024 23:52:27 +0100
User-agent: Mozilla Thunderbird

On 12.12.2024 12:10, Cédric Le Goater wrote:
On 12/11/24 00:06, Maciej S. Szmigiero wrote:
On 9.12.2024 10:28, Avihai Horon wrote:

On 17/11/2024 21:20, Maciej S. Szmigiero wrote:
External email: Use caution opening links or attachments


From: "Maciej S. Szmigiero" <maciej.szmigiero@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.szmigiero@oracle.com>
---
  hw/vfio/migration.c  | 155 +++++++++++++++++++++++++++++++++++++++++++
  hw/vfio/trace-events |   2 +
  2 files changed, 157 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index b54879fe6209..8709672ada48 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
(...)
+
      trace_vfio_save_complete_precopy_start(vbasedev->name);

      /* We reach here with device state STOP or STOP_COPY only */
@@ -974,12 +1011,129 @@ static int vfio_save_complete_precopy(QEMUFile *f, void 
*opaque)
      return ret;
  }

+static int
+vfio_save_complete_precopy_async_thread_config_state(VFIODevice *vbasedev,
+                                                     char *idstr,
+                                                     uint32_t instance_id,
+                                                     uint32_t idx)
+{
+    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));
+
+    ret = vfio_save_device_config_state(f, vbasedev, NULL);
+    if (ret) {
+        return ret;
+    }
+
+    ret = qemu_fflush(f);
+    if (ret) {
+        return ret;
+    }
+
+    packet_len = sizeof(*packet) + bioc->usage;
+    packet = g_malloc0(packet_len);
+    packet->idx = idx;
+    packet->flags = VFIO_DEVICE_STATE_CONFIG_STATE;
+    memcpy(&packet->data, bioc->data, bioc->usage);
+
+    if (!multifd_queue_device_state(idstr, instance_id,
+                                    (char *)packet, packet_len)) {
+        return -1;
+    }
+
+    qatomic_add(&bytes_transferred, packet_len);
+
+    return 0;
+}
+
+static int vfio_save_complete_precopy_thread(char *idstr,
+                                             uint32_t instance_id,
+                                             bool *abort_flag,
+                                             void *opaque)
+{
+    VFIODevice *vbasedev = opaque;
+    VFIOMigration *migration = vbasedev->migration;
+    int ret;
+    g_autofree VFIODeviceStatePacket *packet = NULL;
+    uint32_t idx;
+
+    if (!migration->multifd_transfer) {
+        /* Nothing to do, vfio_save_complete_precopy() does the transfer. */
+        return 0;
+    }
+
+    trace_vfio_save_complete_precopy_thread_start(vbasedev->name,
+                                                  idstr, instance_id);
+
+    /* We reach here with device state STOP or STOP_COPY only */
+    ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
+                                   VFIO_DEVICE_STATE_STOP, NULL);
+    if (ret) {
+        goto ret_finish;
+    }
+
+    packet = g_malloc0(sizeof(*packet) + migration->data_buffer_size);
+
+    for (idx = 0; ; idx++) {
+        ssize_t data_size;
+        size_t packet_size;
+
+        if (qatomic_read(abort_flag)) {
+            ret = -ECANCELED;
+            goto ret_finish;
+        }
+
+        data_size = read(migration->data_fd, &packet->data,
+                         migration->data_buffer_size);
+        if (data_size < 0) {
+            ret = -errno;
+            goto ret_finish;
+        } else if (data_size == 0) {
+            break;
+        }
+
+        packet->idx = idx;
+        packet_size = sizeof(*packet) + data_size;
+
+        if (!multifd_queue_device_state(idstr, instance_id,
+                                        (char *)packet, packet_size)) {
+            ret = -1;
+            goto ret_finish;
+        }
+
+        qatomic_add(&bytes_transferred, packet_size);
+    }
+
+    ret = vfio_save_complete_precopy_async_thread_config_state(vbasedev, idstr,
+                                                               instance_id,
+                                                               idx);

I am not sure it's safe to save the config space asyncly in the thread, as it 
might be dependent on other device's non-iterable state being loaded first.
See commit d329f5032e17 ("vfio: Move the saving of the config space to the right 
place in VFIO migration") which moved config space saving to the non-iterable state 
saving.

That's an important information - thanks for pointing this out.

Since we don't want to lose this config state saving parallelism
(and the future config state saving parallelism) on unaffected platform
we'll probably need to disable this functionality for ARM64.

By the way, this kind of an implicit dependency in VMState between devices
is really hard to manage, there should be a way to specify it in code somehow..

vmstate has a MigrationPriority field to order loading between
devices. Maybe we could extend but I think it is better to handle
ordering at the device level when there are no external dependencies.
It should be well documented though in the code.


To be clear, by "handling ordering at the device level" you mean
just disabling this functionality for ARM64 as proposed above?


Thanks,

C.



Thanks.


Thanks,
Maciej





reply via email to

[Prev in Thread] Current Thread [Next in Thread]