On 5/13/24 16:34, Avihai Horon wrote:

On 13/05/2024 17:01, Cédric Le Goater wrote:
External email: Use caution opening links or attachments


On 5/9/24 11:09, Avihai Horon wrote:
Emit VFIO migration QAPI event when a VFIO device changes its migration
state. This can be used by management applications to get updates on the
current state of the VFIO device for their own purposes.

A new per VFIO device capability, "migration-events", is added so events
can be enabled only for the required devices. It is disabled by default.

Signed-off-by: Avihai Horon <avih...@nvidia.com>
---
  include/hw/vfio/vfio-common.h |  1 +
  hw/vfio/migration.c           | 56 +++++++++++++++++++++++++++++++++--
  hw/vfio/pci.c                 |  2 ++
  3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b9da6c08ef..3ec5f2425e 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -115,6 +115,7 @@ typedef struct VFIODevice {
      bool no_mmap;
      bool ram_block_discard_allowed;
      OnOffAuto enable_migration;
+    bool migration_events;
      VFIODeviceOps *ops;
      unsigned int num_irqs;
      unsigned int num_regions;
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 06ae40969b..5a359c4c78 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -24,6 +24,7 @@
  #include "migration/register.h"
  #include "migration/blocker.h"
  #include "qapi/error.h"
+#include "qapi/qapi-events-vfio.h"
  #include "exec/ramlist.h"
  #include "exec/ram_addr.h"
  #include "pci.h"
@@ -80,6 +81,55 @@ static const char *mig_state_to_str(enum 
vfio_device_mig_state state)
      }
  }

+static VfioMigrationState
+mig_state_to_qapi_state(enum vfio_device_mig_state state)
+{
+    switch (state) {
+    case VFIO_DEVICE_STATE_STOP:
+        return QAPI_VFIO_MIGRATION_STATE_STOP;
+    case VFIO_DEVICE_STATE_RUNNING:
+        return QAPI_VFIO_MIGRATION_STATE_RUNNING;
+    case VFIO_DEVICE_STATE_STOP_COPY:
+        return QAPI_VFIO_MIGRATION_STATE_STOP_COPY;
+    case VFIO_DEVICE_STATE_RESUMING:
+        return QAPI_VFIO_MIGRATION_STATE_RESUMING;
+    case VFIO_DEVICE_STATE_RUNNING_P2P:
+        return QAPI_VFIO_MIGRATION_STATE_RUNNING_P2P;
+    case VFIO_DEVICE_STATE_PRE_COPY:
+        return QAPI_VFIO_MIGRATION_STATE_PRE_COPY;
+    case VFIO_DEVICE_STATE_PRE_COPY_P2P:
+        return QAPI_VFIO_MIGRATION_STATE_PRE_COPY_P2P;
+    default:
+        g_assert_not_reached();
+    }
+}
+
+static void vfio_migration_send_event(VFIODevice *vbasedev)
+{
+    VFIOMigration *migration = vbasedev->migration;
+    DeviceState *dev = vbasedev->dev;
+    g_autofree char *qom_path = NULL;
+    Object *obj;
+
+    if (!vbasedev->migration_events) {
+        return;
+    }

I would add an assert on vbasedev->ops->vfio_get_object

+    obj = vbasedev->ops->vfio_get_object(vbasedev);

and another assert on obj.

vfio_migration_init() already checks these:

     if (!vbasedev->ops->vfio_get_object) {
         return -EINVAL;
     }

     obj = vbasedev->ops->vfio_get_object(vbasedev);
     if (!obj) {
         return -EINVAL;
     }

Do you think these checks in migration init are enough?

I am sure they are today. These extra asserts are to avoid issues if
the code is moved around or if anyone finds inspiration by reading
vfio_migration_send_event().

Thanks,

C.





+    qom_path = object_get_canonical_path(obj);
+
+    qapi_event_send_vfio_migration(
+        dev->id, qom_path, mig_state_to_qapi_state(migration->device_state));
+}
+
+static void set_state(VFIODevice *vbasedev, enum vfio_device_mig_state state)

to avoid the conflict with vfio_migration_set_state(), let's call it :
vfio_migration_set_device_state() ? We want a 'vfio_migration_' prefix.

Sure, I will rename to that.

Thanks.



Thanks,

C.




+{
+    VFIOMigration *migration = vbasedev->migration;
+
+    migration->device_state = state;
+    vfio_migration_send_event(vbasedev);
+}
+
  static int vfio_migration_set_state(VFIODevice *vbasedev,
                                      enum vfio_device_mig_state new_state,
                                      enum vfio_device_mig_state recover_state)
@@ -125,12 +175,12 @@ static int vfio_migration_set_state(VFIODevice *vbasedev,
              goto reset_device;
          }

-        migration->device_state = recover_state;
+        set_state(vbasedev, recover_state);

          return ret;
      }

-    migration->device_state = new_state;
+    set_state(vbasedev, new_state);
      if (mig_state->data_fd != -1) {
          if (migration->data_fd != -1) {
              /*
@@ -156,7 +206,7 @@ reset_device:
                   strerror(errno));
      }

-    migration->device_state = VFIO_DEVICE_STATE_RUNNING;
+    set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING);

      return ret;
  }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 64780d1b79..8840602c50 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3362,6 +3362,8 @@ static Property vfio_pci_dev_properties[] = {
                      VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
      DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
                              vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
+    DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
+                     vbasedev.migration_events, false),
      DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
      DEFINE_PROP_BOOL("x-balloon-allowed", VFIOPCIDevice,
                       vbasedev.ram_block_discard_allowed, false),




Reply via email to