diff mbox series

[v2,2/3] vfio/migration: Emit VFIO migration QAPI event

Message ID 20240509090954.16447-3-avihaih@nvidia.com (mailing list archive)
State New
Headers show
Series qapi/vfio: Add VFIO migration QAPI event | expand

Commit Message

Avihai Horon May 9, 2024, 9:09 a.m. UTC
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 <avihaih@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(-)

Comments

Cédric Le Goater May 13, 2024, 2:01 p.m. UTC | #1
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 <avihaih@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.

> +    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.


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),
Avihai Horon May 13, 2024, 2:34 p.m. UTC | #2
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 <avihaih@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?

>
>> +    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),
>
Cédric Le Goater May 13, 2024, 2:43 p.m. UTC | #3
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 <avihaih@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),
>>
>
Avihai Horon May 13, 2024, 2:48 p.m. UTC | #4
On 13/05/2024 17:43, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> 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 <avihaih@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().
>
Ah, I see your point.

I will add the asserts then.

Thanks.

> 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),
>>>
>>
>
diff mbox series

Patch

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;
+    }
+
+    obj = vbasedev->ops->vfio_get_object(vbasedev);
+    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)
+{
+    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),