| Message ID | 1603704987-20977-18-git-send-email-kwankhede@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add migration support for VFIO devices | expand |
On 26/10/2020 10.36, Kirti Wankhede wrote: > Added amount of bytes transferred to the VM at destination by all VFIO > devices > > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > hw/vfio/common.c | 19 +++++++++++++++++++ > hw/vfio/migration.c | 9 +++++++++ > include/hw/vfio/vfio-common.h | 3 +++ > migration/migration.c | 17 +++++++++++++++++ > monitor/hmp-cmds.c | 6 ++++++ > qapi/migration.json | 17 +++++++++++++++++ > 6 files changed, 71 insertions(+) [...] > diff --git a/migration/migration.c b/migration/migration.c > index 0575ecb37953..995ccd96a774 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -57,6 +57,10 @@ > #include "qemu/queue.h" > #include "multifd.h" > > +#ifdef CONFIG_VFIO > +#include "hw/vfio/vfio-common.h" > +#endif > + > #define MAX_THROTTLE (128 << 20) /* Migration transfer speed throttling */ > > /* Amount of time to allocate to each "chunk" of bandwidth-throttled > @@ -1002,6 +1006,17 @@ static void populate_disk_info(MigrationInfo *info) > } > } > > +static void populate_vfio_info(MigrationInfo *info) > +{ > +#ifdef CONFIG_VFIO > + if (vfio_mig_active()) { > + info->has_vfio = true; > + info->vfio = g_malloc0(sizeof(*info->vfio)); > + info->vfio->transferred = vfio_mig_bytes_transferred(); > + } > +#endif > +} Hi! I'm afraid, but this #ifdef CONFIG_VFIO here likely does not work as expected: migration/migration.c is common code, i.e. it is compiled only once for all targets. But CONFIG_VFIO is a target-specific config switch, so this is only defined properly for target specific .c files. So depending on which target has been compiled first, the code might be included or not for all the other targets, no matter whether they have VFIO or not. To fix this issue, I think it's likely best to move the function into a new file instead and include that via specific_ss.add() in the meson.build file. Thomas
On 10/26/20 4:36 AM, Kirti Wankhede wrote: > Added amount of bytes transferred to the VM at destination by all VFIO > devices > > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > +++ b/qapi/migration.json > @@ -147,6 +147,18 @@ > 'active', 'postcopy-active', 'postcopy-paused', > 'postcopy-recover', 'completed', 'failed', 'colo', > 'pre-switchover', 'device', 'wait-unplug' ] } > +## > +# @VfioStats: > +# > +# Detailed VFIO devices migration statistics > +# > +# @transferred: amount of bytes transferred to the target VM by VFIO devices > +# > +# Since: 5.2 5.2 is last year; this should be 6.0 (if this makes it in time for soft freeze) or even 6.1 > +# > +## > +{ 'struct': 'VfioStats', > + 'data': {'transferred': 'int' } } > > ## > # @MigrationInfo: > @@ -208,11 +220,16 @@ > # > # @socket-address: Only used for tcp, to know what the real port is (Since 4.0) > # > +# @vfio: @VfioStats containing detailed VFIO devices migration statistics, > +# only returned if VFIO device is present, migration is supported by all > +# VFIO devices and status is 'active' or 'completed' (since 5.2) here, too > +# > # Since: 0.14.0 > ## > { 'struct': 'MigrationInfo', > 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', > '*disk': 'MigrationStats', > + '*vfio': 'VfioStats', > '*xbzrle-cache': 'XBZRLECacheStats', > '*total-time': 'int', > '*expected-downtime': 'int', >
On 3/16/21 8:22 AM, Eric Blake wrote: > On 10/26/20 4:36 AM, Kirti Wankhede wrote: Hmm, just now realizing this is an old thread... >> Added amount of bytes transferred to the VM at destination by all VFIO >> devices >> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> --- > >> +++ b/qapi/migration.json >> @@ -147,6 +147,18 @@ >> 'active', 'postcopy-active', 'postcopy-paused', >> 'postcopy-recover', 'completed', 'failed', 'colo', >> 'pre-switchover', 'device', 'wait-unplug' ] } >> +## >> +# @VfioStats: >> +# >> +# Detailed VFIO devices migration statistics >> +# >> +# @transferred: amount of bytes transferred to the target VM by VFIO devices >> +# >> +# Since: 5.2 > > 5.2 is last year; this should be 6.0 (if this makes it in time for soft > freeze) or even 6.1 ...but it was revived because of Thomas' notice of a problem with CONFIG_VFIO with what is already in-tree, rather than something still awaiting review. My (untimely) review is thus unnecessary noise; sorry.
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 49c68a5253ae..56f6fee66a55 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -292,6 +292,25 @@ const MemoryRegionOps vfio_region_ops = { * Device state interfaces */ +bool vfio_mig_active(void) +{ + VFIOGroup *group; + VFIODevice *vbasedev; + + if (QLIST_EMPTY(&vfio_group_list)) { + return false; + } + + QLIST_FOREACH(group, &vfio_group_list, next) { + QLIST_FOREACH(vbasedev, &group->device_list, next) { + if (vbasedev->migration_blocker) { + return false; + } + } + } + return true; +} + static bool vfio_devices_all_stopped_and_saving(VFIOContainer *container) { VFIOGroup *group; diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index ffedbcca179d..2d657289c68e 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -45,6 +45,8 @@ #define VFIO_MIG_FLAG_DEV_SETUP_STATE (0xffffffffef100003ULL) #define VFIO_MIG_FLAG_DEV_DATA_STATE (0xffffffffef100004ULL) +static int64_t bytes_transferred; + static inline int vfio_mig_access(VFIODevice *vbasedev, void *val, int count, off_t off, bool iswrite) { @@ -255,6 +257,7 @@ static int vfio_save_buffer(QEMUFile *f, VFIODevice *vbasedev, uint64_t *size) *size = data_size; } + bytes_transferred += data_size; return ret; } @@ -785,6 +788,7 @@ static void vfio_migration_state_notifier(Notifier *notifier, void *data) case MIGRATION_STATUS_CANCELLING: case MIGRATION_STATUS_CANCELLED: case MIGRATION_STATUS_FAILED: + bytes_transferred = 0; ret = vfio_migration_set_state(vbasedev, ~(VFIO_DEVICE_STATE_SAVING | VFIO_DEVICE_STATE_RESUMING), VFIO_DEVICE_STATE_RUNNING); @@ -866,6 +870,11 @@ err: /* ---------------------------------------------------------------------- */ +int64_t vfio_mig_bytes_transferred(void) +{ + return bytes_transferred; +} + int vfio_migration_probe(VFIODevice *vbasedev, Error **errp) { VFIOContainer *container = vbasedev->group->container; diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index b1c1b18fd228..24e299d97425 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -203,6 +203,9 @@ extern const MemoryRegionOps vfio_region_ops; typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList; extern VFIOGroupList vfio_group_list; +bool vfio_mig_active(void); +int64_t vfio_mig_bytes_transferred(void); + #ifdef CONFIG_LINUX int vfio_get_region_info(VFIODevice *vbasedev, int index, struct vfio_region_info **info); diff --git a/migration/migration.c b/migration/migration.c index 0575ecb37953..995ccd96a774 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -57,6 +57,10 @@ #include "qemu/queue.h" #include "multifd.h" +#ifdef CONFIG_VFIO +#include "hw/vfio/vfio-common.h" +#endif + #define MAX_THROTTLE (128 << 20) /* Migration transfer speed throttling */ /* Amount of time to allocate to each "chunk" of bandwidth-throttled @@ -1002,6 +1006,17 @@ static void populate_disk_info(MigrationInfo *info) } } +static void populate_vfio_info(MigrationInfo *info) +{ +#ifdef CONFIG_VFIO + if (vfio_mig_active()) { + info->has_vfio = true; + info->vfio = g_malloc0(sizeof(*info->vfio)); + info->vfio->transferred = vfio_mig_bytes_transferred(); + } +#endif +} + static void fill_source_migration_info(MigrationInfo *info) { MigrationState *s = migrate_get_current(); @@ -1026,6 +1041,7 @@ static void fill_source_migration_info(MigrationInfo *info) populate_time_info(info, s); populate_ram_info(info, s); populate_disk_info(info); + populate_vfio_info(info); break; case MIGRATION_STATUS_COLO: info->has_status = true; @@ -1034,6 +1050,7 @@ static void fill_source_migration_info(MigrationInfo *info) case MIGRATION_STATUS_COMPLETED: populate_time_info(info, s); populate_ram_info(info, s); + populate_vfio_info(info); break; case MIGRATION_STATUS_FAILED: info->has_status = true; diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 9789f4277f50..56e9bad33d94 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -357,6 +357,12 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) } monitor_printf(mon, "]\n"); } + + if (info->has_vfio) { + monitor_printf(mon, "vfio device transferred: %" PRIu64 " kbytes\n", + info->vfio->transferred >> 10); + } + qapi_free_MigrationInfo(info); } diff --git a/qapi/migration.json b/qapi/migration.json index a5da513c9e05..3c7582052725 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -147,6 +147,18 @@ 'active', 'postcopy-active', 'postcopy-paused', 'postcopy-recover', 'completed', 'failed', 'colo', 'pre-switchover', 'device', 'wait-unplug' ] } +## +# @VfioStats: +# +# Detailed VFIO devices migration statistics +# +# @transferred: amount of bytes transferred to the target VM by VFIO devices +# +# Since: 5.2 +# +## +{ 'struct': 'VfioStats', + 'data': {'transferred': 'int' } } ## # @MigrationInfo: @@ -208,11 +220,16 @@ # # @socket-address: Only used for tcp, to know what the real port is (Since 4.0) # +# @vfio: @VfioStats containing detailed VFIO devices migration statistics, +# only returned if VFIO device is present, migration is supported by all +# VFIO devices and status is 'active' or 'completed' (since 5.2) +# # Since: 0.14.0 ## { 'struct': 'MigrationInfo', 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', '*disk': 'MigrationStats', + '*vfio': 'VfioStats', '*xbzrle-cache': 'XBZRLECacheStats', '*total-time': 'int', '*expected-downtime': 'int',