Re: [PATCH v3 20/24] vfio/migration: Add load_device_config_state_start trace event

[Cédric Le Goater]

On 11/17/24 20:20, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>
> And rename existing load_device_config_state trace event to
> load_device_config_state_end for consistency since it is triggered at the
> end of loading of the VFIO device config state.
>
> This way both the start and end points of particular device config
> loading operation (a long, BQL-serialized operation) are known.
>
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>


I think we should add more trace events regarding the new threads
this series is adding. At all level :

  hw/vfio/trace-events
  migration/trace-events
  util/trace-events


Some time ago, Peter proposed a series adding an "info migrationthreads"
hmp command [*]. I found it useful for dev/debug. I wonder about its
status.

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

[*] migration: query-migrationthreads enhancements and cleanups
    https://lore.kernel.org/all/20240930195837.825728-1-peterx@redhat.com/


> ---
>   hw/vfio/migration.c  | 4 +++-
>   hw/vfio/trace-events | 3 ++-
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 9e2657073012..4b2b06b45195 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -285,6 +285,8 @@ static int vfio_load_device_config_state(QEMUFile *f, void 
> *opaque)
>       VFIODevice *vbasedev = opaque;
>       uint64_t data;
>   
> +    trace_vfio_load_device_config_state_start(vbasedev->name);
> +
>       if (vbasedev->ops && vbasedev->ops->vfio_load_config) {
>           int ret;
>   
> @@ -303,7 +305,7 @@ static int vfio_load_device_config_state(QEMUFile *f, void *opaque)
>           return -EINVAL;
>       }
>   
> -    trace_vfio_load_device_config_state(vbasedev->name);
> +    trace_vfio_load_device_config_state_end(vbasedev->name);
>       return qemu_file_get_error(f);
>   }
>   
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index cab1cf1de0a2..1bebe9877d88 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -149,7 +149,8 @@ vfio_display_edid_write_error(void) ""
>   
>   # migration.c
>   vfio_load_cleanup(const char *name) " (%s)"
> -vfio_load_device_config_state(const char *name) " (%s)"
> +vfio_load_device_config_state_start(const char *name) " (%s)"
> +vfio_load_device_config_state_end(const char *name) " (%s)"
>   vfio_load_state(const char *name, uint64_t data) " (%s) data 0x%"PRIx64
>   vfio_load_state_device_data(const char *name, uint64_t data_size, int ret) " (%s) size 
> %"PRIu64" ret %d"
>   vfio_migration_realize(const char *name) " (%s)"