From: Avihai Horon <avihaih@nvidia.com>
To: "Cédric Le Goater" <clg@redhat.com>, qemu-devel@nongnu.org
Cc: Alex Williamson <alex.williamson@redhat.com>,
Markus Armbruster <armbru@redhat.com>,
Michael Roth <michael.roth@amd.com>,
Eric Blake <eblake@redhat.com>, Peter Xu <peterx@redhat.com>,
Fabiano Rosas <farosas@suse.de>,
Joao Martins <joao.m.martins@oracle.com>,
Maor Gottlieb <maorg@nvidia.com>
Subject: Re: [PATCH v2 2/3] vfio/migration: Emit VFIO migration QAPI event
Date: Mon, 13 May 2024 17:34:53 +0300 [thread overview]
Message-ID: <d4252893-905f-46ec-8113-a39ef5623d34@nvidia.com> (raw)
In-Reply-To: <8fe65dfe-dd9c-47c0-8769-3479484b3e5e@redhat.com>
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),
>
next prev parent reply other threads:[~2024-05-13 14:35 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 9:09 [PATCH v2 0/3] qapi/vfio: Add VFIO migration QAPI event Avihai Horon
2024-05-09 9:09 ` [PATCH v2 1/3] " Avihai Horon
2024-05-13 14:15 ` Cédric Le Goater
2024-05-09 9:09 ` [PATCH v2 2/3] vfio/migration: Emit " Avihai Horon
2024-05-13 14:01 ` Cédric Le Goater
2024-05-13 14:34 ` Avihai Horon [this message]
2024-05-13 14:43 ` Cédric Le Goater
2024-05-13 14:48 ` Avihai Horon
2024-05-09 9:09 ` [PATCH v2 3/3] vfio/migration: Don't emit STOP_COPY VFIO migration QAPI event twice Avihai Horon
2024-05-13 14:13 ` Cédric Le Goater
2024-05-13 14:38 ` Avihai Horon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d4252893-905f-46ec-8113-a39ef5623d34@nvidia.com \
--to=avihaih@nvidia.com \
--cc=alex.williamson@redhat.com \
--cc=armbru@redhat.com \
--cc=clg@redhat.com \
--cc=eblake@redhat.com \
--cc=farosas@suse.de \
--cc=joao.m.martins@oracle.com \
--cc=maorg@nvidia.com \
--cc=michael.roth@amd.com \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).