[PATCH v3 0/2] vfio/migration: new trace events for 9.2

[PATCH v3 0/2] vfio/migration: new trace events for 9.2

[Maciej S. Szmigiero]

From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

This is essentially the v2 patch from here:
https://lore.kernel.org/qemu-devel/8fbb9cc40d9db570eff0a02e49104835014a5a4d.1730731549.git.maciej.szmigiero@oracle.com/
split into two separate patches, with a few small requested changes, like:
* Rename vfio_precopy_empty_hit into vfio_save_block_precopy_empty_hit,

* Add an 'event_' prefix to the related struct VFIOMigration new members,

* Change trace event '_started' suffix to '_start' for consistency with
other trace events (but keep the variable name as 'started' since it
tracks whether that event has already happened).

Maciej S. Szmigiero (2):
  vfio/migration: Add save_{iterate,complete_precopy}_start trace events
  vfio/migration: Add vfio_save_block_precopy_empty_hit trace event

 hw/vfio/migration.c           | 17 +++++++++++++++++
 hw/vfio/trace-events          |  3 +++
 include/hw/vfio/vfio-common.h |  3 +++
 3 files changed, 23 insertions(+)

[PATCH v3 1/2] vfio/migration: Add save_{iterate, complete_precopy}_start trace events

[Maciej S. Szmigiero]

From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

This way both the start and end points of migrating a particular VFIO
device are known.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
 hw/vfio/migration.c           | 9 +++++++++
 hw/vfio/trace-events          | 2 ++
 include/hw/vfio/vfio-common.h | 2 ++
 3 files changed, 13 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 992dc3b10257..87ddc210578f 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -472,6 +472,8 @@ static int vfio_save_setup(QEMUFile *f, void *opaque, Error **errp)
         return -ENOMEM;
     }
 
+    migration->event_save_iterate_started = false;
+
     if (vfio_precopy_supported(vbasedev)) {
         switch (migration->device_state) {
         case VFIO_DEVICE_STATE_RUNNING:
@@ -602,6 +604,11 @@ static int vfio_save_iterate(QEMUFile *f, void *opaque)
     VFIOMigration *migration = vbasedev->migration;
     ssize_t data_size;
 
+    if (!migration->event_save_iterate_started) {
+        trace_vfio_save_iterate_start(vbasedev->name);
+        migration->event_save_iterate_started = true;
+    }
+
     data_size = vfio_save_block(f, migration);
     if (data_size < 0) {
         return data_size;
@@ -630,6 +637,8 @@ static int vfio_save_complete_precopy(QEMUFile *f, void *opaque)
     int ret;
     Error *local_err = NULL;
 
+    trace_vfio_save_complete_precopy_start(vbasedev->name);
+
     /* We reach here with device state STOP or STOP_COPY only */
     ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP_COPY,
                                    VFIO_DEVICE_STATE_STOP, &local_err);
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 29789e8d276d..032a51a10592 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -159,8 +159,10 @@ vfio_migration_state_notifier(const char *name, int state) " (%s) state %d"
 vfio_save_block(const char *name, int data_size) " (%s) data_size %d"
 vfio_save_cleanup(const char *name) " (%s)"
 vfio_save_complete_precopy(const char *name, int ret) " (%s) ret %d"
+vfio_save_complete_precopy_start(const char *name) " (%s)"
 vfio_save_device_config_state(const char *name) " (%s)"
 vfio_save_iterate(const char *name, uint64_t precopy_init_size, uint64_t precopy_dirty_size) " (%s) precopy initial size %"PRIu64" precopy dirty size %"PRIu64
+vfio_save_iterate_start(const char *name) " (%s)"
 vfio_save_setup(const char *name, uint64_t data_buffer_size) " (%s) data buffer size %"PRIu64
 vfio_state_pending_estimate(const char *name, uint64_t precopy, uint64_t postcopy, uint64_t precopy_init_size, uint64_t precopy_dirty_size) " (%s) precopy %"PRIu64" postcopy %"PRIu64" precopy initial size %"PRIu64" precopy dirty size %"PRIu64
 vfio_state_pending_exact(const char *name, uint64_t precopy, uint64_t postcopy, uint64_t stopcopy_size, uint64_t precopy_init_size, uint64_t precopy_dirty_size) " (%s) precopy %"PRIu64" postcopy %"PRIu64" stopcopy size %"PRIu64" precopy initial size %"PRIu64" precopy dirty size %"PRIu64
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index fed499b199f0..282eadf4ebf5 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -73,6 +73,8 @@ typedef struct VFIOMigration {
     uint64_t precopy_init_size;
     uint64_t precopy_dirty_size;
     bool initial_data_sent;
+
+    bool event_save_iterate_started;
 } VFIOMigration;
 
 struct VFIOGroup;

Re: [PATCH v3 1/2] vfio/migration: Add save_{iterate,complete_precopy}_start trace events

[Cédric Le Goater]

On 11/4/24 22:29, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> 
> This way both the start and end points of migrating a particular VFIO
> device are known.
> 
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> ---
>   hw/vfio/migration.c           | 9 +++++++++
>   hw/vfio/trace-events          | 2 ++
>   include/hw/vfio/vfio-common.h | 2 ++
>   3 files changed, 13 insertions(+)
> 

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

Thanks,

C.

[PATCH v3 2/2] vfio/migration: Add vfio_save_block_precopy_empty_hit trace event

[Maciej S. Szmigiero]

From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

This way it is clearly known when there's no more data to send for that
device.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
 hw/vfio/migration.c           | 8 ++++++++
 hw/vfio/trace-events          | 1 +
 include/hw/vfio/vfio-common.h | 1 +
 3 files changed, 10 insertions(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 87ddc210578f..01aa11013e42 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -370,6 +370,10 @@ static ssize_t vfio_save_block(QEMUFile *f, VFIOMigration *migration)
          * please refer to the Linux kernel VFIO uAPI.
          */
         if (errno == ENOMSG) {
+            if (!migration->event_precopy_empty_hit) {
+                trace_vfio_save_block_precopy_empty_hit(migration->vbasedev->name);
+                migration->event_precopy_empty_hit = true;
+            }
             return 0;
         }
 
@@ -379,6 +383,9 @@ static ssize_t vfio_save_block(QEMUFile *f, VFIOMigration *migration)
         return 0;
     }
 
+    /* Non-empty read: re-arm the trace event */
+    migration->event_precopy_empty_hit = false;
+
     qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE);
     qemu_put_be64(f, data_size);
     qemu_put_buffer(f, migration->data_buffer, data_size);
@@ -473,6 +480,7 @@ static int vfio_save_setup(QEMUFile *f, void *opaque, Error **errp)
     }
 
     migration->event_save_iterate_started = false;
+    migration->event_precopy_empty_hit = false;
 
     if (vfio_precopy_supported(vbasedev)) {
         switch (migration->device_state) {
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 032a51a10592..cab1cf1de0a2 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -157,6 +157,7 @@ vfio_migration_set_device_state(const char *name, const char *state) " (%s) stat
 vfio_migration_set_state(const char *name, const char *new_state, const char *recover_state) " (%s) new state %s, recover state %s"
 vfio_migration_state_notifier(const char *name, int state) " (%s) state %d"
 vfio_save_block(const char *name, int data_size) " (%s) data_size %d"
+vfio_save_block_precopy_empty_hit(const char *name) " (%s)"
 vfio_save_cleanup(const char *name) " (%s)"
 vfio_save_complete_precopy(const char *name, int ret) " (%s) ret %d"
 vfio_save_complete_precopy_start(const char *name) " (%s)"
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 282eadf4ebf5..e0ce6ec3a9b3 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -75,6 +75,7 @@ typedef struct VFIOMigration {
     bool initial_data_sent;
 
     bool event_save_iterate_started;
+    bool event_precopy_empty_hit;
 } VFIOMigration;
 
 struct VFIOGroup;

Re: [PATCH v3 2/2] vfio/migration: Add vfio_save_block_precopy_empty_hit trace event

[Cédric Le Goater]

On 11/4/24 22:29, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> 
> This way it is clearly known when there's no more data to send for that
> device.
> 
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> ---
>   hw/vfio/migration.c           | 8 ++++++++
>   hw/vfio/trace-events          | 1 +
>   include/hw/vfio/vfio-common.h | 1 +
>   3 files changed, 10 insertions(+)
> 

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

Thanks,

C.

Re: [PATCH v3 2/2] vfio/migration: Add vfio_save_block_precopy_empty_hit trace event

[Maciej S. Szmigiero]

On 5.11.2024 08:56, Cédric Le Goater wrote:
> On 11/4/24 22:29, Maciej S. Szmigiero wrote:
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>
>> This way it is clearly known when there's no more data to send for that
>> device.
>>
>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>> ---
>>   hw/vfio/migration.c           | 8 ++++++++
>>   hw/vfio/trace-events          | 1 +
>>   include/hw/vfio/vfio-common.h | 1 +
>>   3 files changed, 10 insertions(+)
>>
> 
> Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks, will you pick up these two patches or
is Peter supposed to do it?

> Thanks,
> C.

Thanks,
Maciej

Re: [PATCH v3 2/2] vfio/migration: Add vfio_save_block_precopy_empty_hit trace event

[Cédric Le Goater]

On 11/5/24 13:20, Maciej S. Szmigiero wrote:
> On 5.11.2024 08:56, Cédric Le Goater wrote:
>> On 11/4/24 22:29, Maciej S. Szmigiero wrote:
>>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>>
>>> This way it is clearly known when there's no more data to send for that
>>> device.
>>>
>>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>>> ---
>>>   hw/vfio/migration.c           | 8 ++++++++
>>>   hw/vfio/trace-events          | 1 +
>>>   include/hw/vfio/vfio-common.h | 1 +
>>>   3 files changed, 10 insertions(+)
>>>
>>
>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> 
> Thanks, will you pick up these two patches or
> is Peter supposed to do it?

I will send a small PR at the end of the day.


Thanks,

C.

Re: [PATCH v3 2/2] vfio/migration: Add vfio_save_block_precopy_empty_hit trace event

[Peter Xu]

On Tue, Nov 05, 2024 at 01:51:31PM +0100, Cédric Le Goater wrote:
> On 11/5/24 13:20, Maciej S. Szmigiero wrote:
> > On 5.11.2024 08:56, Cédric Le Goater wrote:
> > > On 11/4/24 22:29, Maciej S. Szmigiero wrote:
> > > > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> > > > 
> > > > This way it is clearly known when there's no more data to send for that
> > > > device.
> > > > 
> > > > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> > > > ---
> > > >   hw/vfio/migration.c           | 8 ++++++++
> > > >   hw/vfio/trace-events          | 1 +
> > > >   include/hw/vfio/vfio-common.h | 1 +
> > > >   3 files changed, 10 insertions(+)
> > > > 
> > > 
> > > Reviewed-by: Cédric Le Goater <clg@redhat.com>
> > 
> > Thanks, will you pick up these two patches or
> > is Peter supposed to do it?

When there's dependency, we can discuss how to merge. But I suppose all
standalone hw/vfio/* patches should by default go via Cédric.

> 
> I will send a small PR at the end of the day.

Thanks!

-- 
Peter Xu