Currently, VFIO bytes_transferred is not reset properly:
1. bytes_transferred is not reset after a VM snapshot (so a migration
   following a snapshot will report incorrect value).
2. bytes_transferred is a single counter for all VFIO devices, however
   upon migration failure it is reset multiple times, by each VFIO
   device.

Fix it by introducing a new function vfio_reset_bytes_transferred() and
calling it during migration and snapshot start.

Remove existing bytes_transferred reset in VFIO migration state
notifier, which is not needed anymore.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/hw/vfio/vfio-common.h |  1 +
 migration/migration.h         |  1 +
 hw/vfio/migration.c           |  6 +++++-
 migration/migration.c         |  1 +
 migration/savevm.c            |  1 +
 migration/target.c            | 17 +++++++++++++++--
 6 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 3dc5f2104c..b4c28f318f 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -228,6 +228,7 @@ int vfio_block_multiple_devices_migration(Error **errp);
 void vfio_unblock_multiple_devices_migration(void);
 int vfio_block_giommu_migration(Error **errp);
 int64_t vfio_mig_bytes_transferred(void);
+void vfio_reset_bytes_transferred(void);
 
 #ifdef CONFIG_LINUX
 int vfio_get_region_info(VFIODevice *vbasedev, int index,
diff --git a/migration/migration.h b/migration/migration.h
index c859a0d35e..a80b22b703 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -514,6 +514,7 @@ bool migration_rate_limit(void);
 void migration_cancel(const Error *error);
 
 void populate_vfio_info(MigrationInfo *info);
+void reset_vfio_bytes_transferred(void);
 void postcopy_temp_page_reset(PostcopyTmpPage *tmp_page);
 
 #endif
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index a8bfbe4b89..79eb81dfd7 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -704,7 +704,6 @@ 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;
         /*
          * If setting the device in RUNNING state fails, the device should
          * be reset. To do so, use ERROR state as a recover state.
@@ -825,6 +824,11 @@ int64_t vfio_mig_bytes_transferred(void)
     return bytes_transferred;
 }
 
+void vfio_reset_bytes_transferred(void)
+{
+    bytes_transferred = 0;
+}
+
 int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
 {
     int ret = -ENOTSUP;
diff --git a/migration/migration.c b/migration/migration.c
index 7653787f74..096e8191d1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1628,6 +1628,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
      */
     memset(&mig_stats, 0, sizeof(mig_stats));
     memset(&compression_counters, 0, sizeof(compression_counters));
+    reset_vfio_bytes_transferred();
 
     return true;
 }
diff --git a/migration/savevm.c b/migration/savevm.c
index cdf4793924..95c2abf47c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1622,6 +1622,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
     migrate_init(ms);
     memset(&mig_stats, 0, sizeof(mig_stats));
     memset(&compression_counters, 0, sizeof(compression_counters));
+    reset_vfio_bytes_transferred();
     ms->to_dst_file = f;
 
     qemu_mutex_unlock_iothread();
diff --git a/migration/target.c b/migration/target.c
index 00ca007f97..f39c9a8d88 100644
--- a/migration/target.c
+++ b/migration/target.c
@@ -14,12 +14,25 @@
 #include "hw/vfio/vfio-common.h"
 #endif
 
+#ifdef CONFIG_VFIO
 void populate_vfio_info(MigrationInfo *info)
 {
-#ifdef CONFIG_VFIO
     if (vfio_mig_active()) {
         info->vfio = g_malloc0(sizeof(*info->vfio));
         info->vfio->transferred = vfio_mig_bytes_transferred();
     }
-#endif
 }
+
+void reset_vfio_bytes_transferred(void)
+{
+    vfio_reset_bytes_transferred();
+}
+#else
+void populate_vfio_info(MigrationInfo *info)
+{
+}
+
+void reset_vfio_bytes_transferred(void)
+{
+}
+#endif
-- 
2.26.3

On 26/06/2023 11:23, Avihai Horon wrote:
> Currently, VFIO bytes_transferred is not reset properly:
> 1. bytes_transferred is not reset after a VM snapshot (so a migration
>     following a snapshot will report incorrect value).
> 2. bytes_transferred is a single counter for all VFIO devices, however
>     upon migration failure it is reset multiple times, by each VFIO
>     device.
>
> Fix it by introducing a new function vfio_reset_bytes_transferred() and
> calling it during migration and snapshot start.
>
> Remove existing bytes_transferred reset in VFIO migration state
> notifier, which is not needed anymore.

Forgot fixes tag:

Fixes: 3710586caa5d ("qapi: Add VFIO devices migration stats in 
Migration stats")

> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> ---
>   include/hw/vfio/vfio-common.h |  1 +
>   migration/migration.h         |  1 +
>   hw/vfio/migration.c           |  6 +++++-
>   migration/migration.c         |  1 +
>   migration/savevm.c            |  1 +
>   migration/target.c            | 17 +++++++++++++++--
>   6 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 3dc5f2104c..b4c28f318f 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -228,6 +228,7 @@ int vfio_block_multiple_devices_migration(Error **errp);
>   void vfio_unblock_multiple_devices_migration(void);
>   int vfio_block_giommu_migration(Error **errp);
>   int64_t vfio_mig_bytes_transferred(void);
> +void vfio_reset_bytes_transferred(void);
>   
>   #ifdef CONFIG_LINUX
>   int vfio_get_region_info(VFIODevice *vbasedev, int index,
> diff --git a/migration/migration.h b/migration/migration.h
> index c859a0d35e..a80b22b703 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -514,6 +514,7 @@ bool migration_rate_limit(void);
>   void migration_cancel(const Error *error);
>   
>   void populate_vfio_info(MigrationInfo *info);
> +void reset_vfio_bytes_transferred(void);
>   void postcopy_temp_page_reset(PostcopyTmpPage *tmp_page);
>   
>   #endif
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index a8bfbe4b89..79eb81dfd7 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -704,7 +704,6 @@ 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;
>           /*
>            * If setting the device in RUNNING state fails, the device should
>            * be reset. To do so, use ERROR state as a recover state.
> @@ -825,6 +824,11 @@ int64_t vfio_mig_bytes_transferred(void)
>       return bytes_transferred;
>   }
>   
> +void vfio_reset_bytes_transferred(void)
> +{
> +    bytes_transferred = 0;
> +}
> +
>   int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>   {
>       int ret = -ENOTSUP;
> diff --git a/migration/migration.c b/migration/migration.c
> index 7653787f74..096e8191d1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1628,6 +1628,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
>        */
>       memset(&mig_stats, 0, sizeof(mig_stats));
>       memset(&compression_counters, 0, sizeof(compression_counters));
> +    reset_vfio_bytes_transferred();
>   
>       return true;
>   }
> diff --git a/migration/savevm.c b/migration/savevm.c
> index cdf4793924..95c2abf47c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1622,6 +1622,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>       migrate_init(ms);
>       memset(&mig_stats, 0, sizeof(mig_stats));
>       memset(&compression_counters, 0, sizeof(compression_counters));
> +    reset_vfio_bytes_transferred();
>       ms->to_dst_file = f;
>   
>       qemu_mutex_unlock_iothread();
> diff --git a/migration/target.c b/migration/target.c
> index 00ca007f97..f39c9a8d88 100644
> --- a/migration/target.c
> +++ b/migration/target.c
> @@ -14,12 +14,25 @@
>   #include "hw/vfio/vfio-common.h"
>   #endif
>   
> +#ifdef CONFIG_VFIO
>   void populate_vfio_info(MigrationInfo *info)
>   {
> -#ifdef CONFIG_VFIO
>       if (vfio_mig_active()) {
>           info->vfio = g_malloc0(sizeof(*info->vfio));
>           info->vfio->transferred = vfio_mig_bytes_transferred();
>       }
> -#endif
>   }
> +
> +void reset_vfio_bytes_transferred(void)
> +{
> +    vfio_reset_bytes_transferred();
> +}
> +#else
> +void populate_vfio_info(MigrationInfo *info)
> +{
> +}
> +
> +void reset_vfio_bytes_transferred(void)
> +{
> +}
> +#endif

Hello Avihai,

On 6/26/23 10:23, Avihai Horon wrote:
> Currently, VFIO bytes_transferred is not reset properly:
> 1. bytes_transferred is not reset after a VM snapshot (so a migration
>     following a snapshot will report incorrect value).
> 2. bytes_transferred is a single counter for all VFIO devices, however
>     upon migration failure it is reset multiple times, by each VFIO
>     device.
> 
> Fix it by introducing a new function vfio_reset_bytes_transferred() and
> calling it during migration and snapshot start.
> 
> Remove existing bytes_transferred reset in VFIO migration state
> notifier, which is not needed anymore.

a Fixes: tag would be useful.

> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> ---
>   include/hw/vfio/vfio-common.h |  1 +
>   migration/migration.h         |  1 +
>   hw/vfio/migration.c           |  6 +++++-
>   migration/migration.c         |  1 +
>   migration/savevm.c            |  1 +
>   migration/target.c            | 17 +++++++++++++++--
>   6 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 3dc5f2104c..b4c28f318f 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -228,6 +228,7 @@ int vfio_block_multiple_devices_migration(Error **errp);
>   void vfio_unblock_multiple_devices_migration(void);
>   int vfio_block_giommu_migration(Error **errp);
>   int64_t vfio_mig_bytes_transferred(void);
> +void vfio_reset_bytes_transferred(void);
>   
>   #ifdef CONFIG_LINUX
>   int vfio_get_region_info(VFIODevice *vbasedev, int index,
> diff --git a/migration/migration.h b/migration/migration.h
> index c859a0d35e..a80b22b703 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -514,6 +514,7 @@ bool migration_rate_limit(void);
>   void migration_cancel(const Error *error);
>   
>   void populate_vfio_info(MigrationInfo *info);
> +void reset_vfio_bytes_transferred(void);
>   void postcopy_temp_page_reset(PostcopyTmpPage *tmp_page);
>   
>   #endif
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index a8bfbe4b89..79eb81dfd7 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -704,7 +704,6 @@ 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;
>           /*
>            * If setting the device in RUNNING state fails, the device should
>            * be reset. To do so, use ERROR state as a recover state.
> @@ -825,6 +824,11 @@ int64_t vfio_mig_bytes_transferred(void)
>       return bytes_transferred;
>   }
>   
> +void vfio_reset_bytes_transferred(void)
> +{
> +    bytes_transferred = 0;
> +}
> +
>   int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>   {
>       int ret = -ENOTSUP;
> diff --git a/migration/migration.c b/migration/migration.c
> index 7653787f74..096e8191d1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1628,6 +1628,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
>        */
>       memset(&mig_stats, 0, sizeof(mig_stats));
>       memset(&compression_counters, 0, sizeof(compression_counters));
> +    reset_vfio_bytes_transferred();
>   
>       return true;
>   }
> diff --git a/migration/savevm.c b/migration/savevm.c
> index cdf4793924..95c2abf47c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1622,6 +1622,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>       migrate_init(ms);
>       memset(&mig_stats, 0, sizeof(mig_stats));
>       memset(&compression_counters, 0, sizeof(compression_counters));
> +    reset_vfio_bytes_transferred();
>       ms->to_dst_file = f;
>   
>       qemu_mutex_unlock_iothread();
> diff --git a/migration/target.c b/migration/target.c
> index 00ca007f97..f39c9a8d88 100644
> --- a/migration/target.c
> +++ b/migration/target.c
> @@ -14,12 +14,25 @@
>   #include "hw/vfio/vfio-common.h"
>   #endif
>   
> +#ifdef CONFIG_VFIO
>   void populate_vfio_info(MigrationInfo *info)
>   {
> -#ifdef CONFIG_VFIO
>       if (vfio_mig_active()) {
>           info->vfio = g_malloc0(sizeof(*info->vfio));
>           info->vfio->transferred = vfio_mig_bytes_transferred();
>       }
> -#endif
>   }
> +
> +void reset_vfio_bytes_transferred(void)
> +{
> +    vfio_reset_bytes_transferred();
> +}
> +#else
> +void populate_vfio_info(MigrationInfo *info)
> +{
> +}
> +
> +void reset_vfio_bytes_transferred(void)
> +{
> +}
> +#endif

I would simply use static inline in migration.h if !CONFIG_VFIO.

Thanks,

C.

On 26/06/2023 12:52, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> Hello Avihai,
>
> On 6/26/23 10:23, Avihai Horon wrote:
>> Currently, VFIO bytes_transferred is not reset properly:
>> 1. bytes_transferred is not reset after a VM snapshot (so a migration
>>     following a snapshot will report incorrect value).
>> 2. bytes_transferred is a single counter for all VFIO devices, however
>>     upon migration failure it is reset multiple times, by each VFIO
>>     device.
>>
>> Fix it by introducing a new function vfio_reset_bytes_transferred() and
>> calling it during migration and snapshot start.
>>
>> Remove existing bytes_transferred reset in VFIO migration state
>> notifier, which is not needed anymore.
>
> a Fixes: tag would be useful.

Sure, I will add.

>
>
>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>> ---
>>   include/hw/vfio/vfio-common.h |  1 +
>>   migration/migration.h         |  1 +
>>   hw/vfio/migration.c           |  6 +++++-
>>   migration/migration.c         |  1 +
>>   migration/savevm.c            |  1 +
>>   migration/target.c            | 17 +++++++++++++++--
>>   6 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h 
>> b/include/hw/vfio/vfio-common.h
>> index 3dc5f2104c..b4c28f318f 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -228,6 +228,7 @@ int vfio_block_multiple_devices_migration(Error 
>> **errp);
>>   void vfio_unblock_multiple_devices_migration(void);
>>   int vfio_block_giommu_migration(Error **errp);
>>   int64_t vfio_mig_bytes_transferred(void);
>> +void vfio_reset_bytes_transferred(void);
>>
>>   #ifdef CONFIG_LINUX
>>   int vfio_get_region_info(VFIODevice *vbasedev, int index,
>> diff --git a/migration/migration.h b/migration/migration.h
>> index c859a0d35e..a80b22b703 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -514,6 +514,7 @@ bool migration_rate_limit(void);
>>   void migration_cancel(const Error *error);
>>
>>   void populate_vfio_info(MigrationInfo *info);
>> +void reset_vfio_bytes_transferred(void);
>>   void postcopy_temp_page_reset(PostcopyTmpPage *tmp_page);
>>
>>   #endif
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index a8bfbe4b89..79eb81dfd7 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -704,7 +704,6 @@ 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;
>>           /*
>>            * If setting the device in RUNNING state fails, the device 
>> should
>>            * be reset. To do so, use ERROR state as a recover state.
>> @@ -825,6 +824,11 @@ int64_t vfio_mig_bytes_transferred(void)
>>       return bytes_transferred;
>>   }
>>
>> +void vfio_reset_bytes_transferred(void)
>> +{
>> +    bytes_transferred = 0;
>> +}
>> +
>>   int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>>   {
>>       int ret = -ENOTSUP;
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 7653787f74..096e8191d1 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1628,6 +1628,7 @@ static bool migrate_prepare(MigrationState *s, 
>> bool blk, bool blk_inc,
>>        */
>>       memset(&mig_stats, 0, sizeof(mig_stats));
>>       memset(&compression_counters, 0, sizeof(compression_counters));
>> +    reset_vfio_bytes_transferred();
>>
>>       return true;
>>   }
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index cdf4793924..95c2abf47c 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1622,6 +1622,7 @@ static int qemu_savevm_state(QEMUFile *f, Error 
>> **errp)
>>       migrate_init(ms);
>>       memset(&mig_stats, 0, sizeof(mig_stats));
>>       memset(&compression_counters, 0, sizeof(compression_counters));
>> +    reset_vfio_bytes_transferred();
>>       ms->to_dst_file = f;
>>
>>       qemu_mutex_unlock_iothread();
>> diff --git a/migration/target.c b/migration/target.c
>> index 00ca007f97..f39c9a8d88 100644
>> --- a/migration/target.c
>> +++ b/migration/target.c
>> @@ -14,12 +14,25 @@
>>   #include "hw/vfio/vfio-common.h"
>>   #endif
>>
>> +#ifdef CONFIG_VFIO
>>   void populate_vfio_info(MigrationInfo *info)
>>   {
>> -#ifdef CONFIG_VFIO
>>       if (vfio_mig_active()) {
>>           info->vfio = g_malloc0(sizeof(*info->vfio));
>>           info->vfio->transferred = vfio_mig_bytes_transferred();
>>       }
>> -#endif
>>   }
>> +
>> +void reset_vfio_bytes_transferred(void)
>> +{
>> +    vfio_reset_bytes_transferred();
>> +}
>> +#else
>> +void populate_vfio_info(MigrationInfo *info)
>> +{
>> +}
>> +
>> +void reset_vfio_bytes_transferred(void)
>> +{
>> +}
>> +#endif
>
> I would simply use static inline in migration.h if !CONFIG_VFIO.

I'm not sure it's possible to use CONFIG_VFIO in migration.h. Got this 
during compilation:
   migration/migration.h:517:8: error: attempt to use poisoned "CONFIG_VFIO"

Plus, see 43bd0bf30fce ("migration: Move populate_vfio_info() into a 
separate file") commit message:

     The CONFIG_VFIO switch only works in target specific code. Since
     migration/migration.c is common code, the #ifdef does not have
     the intended behavior here. Move the related code to a separate
     file now which gets compiled via specific_ss instead.

Or did I misunderstand you?

Thanks.

On 6/26/23 13:46, Avihai Horon wrote:
> 
> On 26/06/2023 12:52, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Hello Avihai,
>>
>> On 6/26/23 10:23, Avihai Horon wrote:
>>> Currently, VFIO bytes_transferred is not reset properly:
>>> 1. bytes_transferred is not reset after a VM snapshot (so a migration
>>>     following a snapshot will report incorrect value).
>>> 2. bytes_transferred is a single counter for all VFIO devices, however
>>>     upon migration failure it is reset multiple times, by each VFIO
>>>     device.
>>>
>>> Fix it by introducing a new function vfio_reset_bytes_transferred() and
>>> calling it during migration and snapshot start.
>>>
>>> Remove existing bytes_transferred reset in VFIO migration state
>>> notifier, which is not needed anymore.
>>
>> a Fixes: tag would be useful.
> 
> Sure, I will add.

You could reply with the appropriate tag and the tooling will automatically
collect it. No need to resend for that.
  
> 
>>
>>
>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>> ---
>>>   include/hw/vfio/vfio-common.h |  1 +
>>>   migration/migration.h         |  1 +
>>>   hw/vfio/migration.c           |  6 +++++-
>>>   migration/migration.c         |  1 +
>>>   migration/savevm.c            |  1 +
>>>   migration/target.c            | 17 +++++++++++++++--
>>>   6 files changed, 24 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index 3dc5f2104c..b4c28f318f 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -228,6 +228,7 @@ int vfio_block_multiple_devices_migration(Error **errp);
>>>   void vfio_unblock_multiple_devices_migration(void);
>>>   int vfio_block_giommu_migration(Error **errp);
>>>   int64_t vfio_mig_bytes_transferred(void);
>>> +void vfio_reset_bytes_transferred(void);
>>>
>>>   #ifdef CONFIG_LINUX
>>>   int vfio_get_region_info(VFIODevice *vbasedev, int index,
>>> diff --git a/migration/migration.h b/migration/migration.h
>>> index c859a0d35e..a80b22b703 100644
>>> --- a/migration/migration.h
>>> +++ b/migration/migration.h
>>> @@ -514,6 +514,7 @@ bool migration_rate_limit(void);
>>>   void migration_cancel(const Error *error);
>>>
>>>   void populate_vfio_info(MigrationInfo *info);
>>> +void reset_vfio_bytes_transferred(void);
>>>   void postcopy_temp_page_reset(PostcopyTmpPage *tmp_page);
>>>
>>>   #endif
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index a8bfbe4b89..79eb81dfd7 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -704,7 +704,6 @@ 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;
>>>           /*
>>>            * If setting the device in RUNNING state fails, the device should
>>>            * be reset. To do so, use ERROR state as a recover state.
>>> @@ -825,6 +824,11 @@ int64_t vfio_mig_bytes_transferred(void)
>>>       return bytes_transferred;
>>>   }
>>>
>>> +void vfio_reset_bytes_transferred(void)
>>> +{
>>> +    bytes_transferred = 0;
>>> +}
>>> +
>>>   int vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>>>   {
>>>       int ret = -ENOTSUP;
>>> diff --git a/migration/migration.c b/migration/migration.c
>>> index 7653787f74..096e8191d1 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -1628,6 +1628,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
>>>        */
>>>       memset(&mig_stats, 0, sizeof(mig_stats));
>>>       memset(&compression_counters, 0, sizeof(compression_counters));
>>> +    reset_vfio_bytes_transferred();
>>>
>>>       return true;
>>>   }
>>> diff --git a/migration/savevm.c b/migration/savevm.c
>>> index cdf4793924..95c2abf47c 100644
>>> --- a/migration/savevm.c
>>> +++ b/migration/savevm.c
>>> @@ -1622,6 +1622,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>>>       migrate_init(ms);
>>>       memset(&mig_stats, 0, sizeof(mig_stats));
>>>       memset(&compression_counters, 0, sizeof(compression_counters));
>>> +    reset_vfio_bytes_transferred();
>>>       ms->to_dst_file = f;
>>>
>>>       qemu_mutex_unlock_iothread();
>>> diff --git a/migration/target.c b/migration/target.c
>>> index 00ca007f97..f39c9a8d88 100644
>>> --- a/migration/target.c
>>> +++ b/migration/target.c
>>> @@ -14,12 +14,25 @@
>>>   #include "hw/vfio/vfio-common.h"
>>>   #endif
>>>
>>> +#ifdef CONFIG_VFIO
>>>   void populate_vfio_info(MigrationInfo *info)
>>>   {
>>> -#ifdef CONFIG_VFIO
>>>       if (vfio_mig_active()) {
>>>           info->vfio = g_malloc0(sizeof(*info->vfio));
>>>           info->vfio->transferred = vfio_mig_bytes_transferred();
>>>       }
>>> -#endif
>>>   }
>>> +
>>> +void reset_vfio_bytes_transferred(void)
>>> +{
>>> +    vfio_reset_bytes_transferred();
>>> +}
>>> +#else
>>> +void populate_vfio_info(MigrationInfo *info)
>>> +{
>>> +}
>>> +
>>> +void reset_vfio_bytes_transferred(void)
>>> +{
>>> +}
>>> +#endif
>>
>> I would simply use static inline in migration.h if !CONFIG_VFIO.
> 
> I'm not sure it's possible to use CONFIG_VFIO in migration.h. Got this during compilation:
>    migration/migration.h:517:8: error: attempt to use poisoned "CONFIG_VFIO"
> 
> Plus, see 43bd0bf30fce ("migration: Move populate_vfio_info() into a separate file") commit message:
> 
>      The CONFIG_VFIO switch only works in target specific code. Since
>      migration/migration.c is common code, the #ifdef does not have
>      the intended behavior here. Move the related code to a separate
>      file now which gets compiled via specific_ss instead.
> 
> Or did I misunderstand you?

I didn't realize that was not possible. Leave it that way then.

Thanks,

C.