diff mbox series

[v6,3/9] migration: Extend migration_file_set_error() with Error* argument

Message ID 20240514153130.394307-4-clg@redhat.com (mailing list archive)
State New, archived
Headers show
Series vfio: Improve error reporting (part 2 | expand

Commit Message

Cédric Le Goater May 14, 2024, 3:31 p.m. UTC 
Use it to update the current error of the migration stream if
available and if not, simply print out the error. Next changes will
update with an error to report.

Reviewed-by: Avihai Horon <avihaih@nvidia.com>
Acked-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---

 Changes in v6:

 - Commit log improvements (Avihai)
 
 include/migration/misc.h | 2 +-
 hw/vfio/common.c         | 2 +-
 hw/vfio/migration.c      | 4 ++--
 migration/migration.c    | 6 ++++--
 4 files changed, 8 insertions(+), 6 deletions(-)

Comments

Eric Auger May 15, 2024, 7:04 a.m. UTC | #1 
Hi Cédric,

On 5/14/24 17:31, Cédric Le Goater wrote:
> Use it to update the current error of the migration stream if
> available and if not, simply print out the error. Next changes will
> update with an error to report.
> 
> Reviewed-by: Avihai Horon <avihaih@nvidia.com>
> Acked-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
> 
>  Changes in v6:
> 
>  - Commit log improvements (Avihai)
>  
>  include/migration/misc.h | 2 +-
>  hw/vfio/common.c         | 2 +-
>  hw/vfio/migration.c      | 4 ++--
>  migration/migration.c    | 6 ++++--
>  4 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index bf7339cc1e6430226127fb6a878d06b458170858..bfadc5613bac614a316e5aed7da95d8c7845cf42 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -97,7 +97,7 @@ void migration_add_notifier_mode(NotifierWithReturn *notify,
>  
>  void migration_remove_notifier(NotifierWithReturn *notify);
>  bool migration_is_running(void);
> -void migration_file_set_error(int err);
> +void migration_file_set_error(int ret, Error *err);
>  
>  /* True if incoming migration entered POSTCOPY_INCOMING_DISCARD */
>  bool migration_in_incoming_postcopy(void);
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index b5102f54a6474a50c6366e8fbce23812d55e384e..ed5ee6349ced78b3bde68d2ee506f78ba1a9dd9c 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -150,7 +150,7 @@ bool vfio_viommu_preset(VFIODevice *vbasedev)
>  static void vfio_set_migration_error(int err)
nit: I would have renamed err into ret here to avoid any further confusion.
>  {
>      if (migration_is_setup_or_active()) {
> -        migration_file_set_error(err);
> +        migration_file_set_error(err, NULL);
>      }
>  }
>  
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 06ae40969b6c19037e190008e14f28be646278cd..bf2fd0759ba6e4fb103cc5c1a43edb180a3d0de4 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -726,7 +726,7 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>           * Migration should be aborted in this case, but vm_state_notify()
>           * currently does not support reporting failures.
>           */
> -        migration_file_set_error(ret);
> +        migration_file_set_error(ret, NULL);
>      }
>  
>      trace_vfio_vmstate_change_prepare(vbasedev->name, running,
> @@ -756,7 +756,7 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>           * Migration should be aborted in this case, but vm_state_notify()
>           * currently does not support reporting failures.
>           */
> -        migration_file_set_error(ret);
> +        migration_file_set_error(ret, NULL);
>      }
>  
>      trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state),
> diff --git a/migration/migration.c b/migration/migration.c
> index e88b24f1e6cbe82dad3f890c00e264d2ab6ad355..70d66a441bf04761decf91dbe57ce52c57fde58f 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2994,13 +2994,15 @@ static MigThrError postcopy_pause(MigrationState *s)
>      }
>  }
>  
> -void migration_file_set_error(int err)
> +void migration_file_set_error(int ret, Error *err)
>  {
>      MigrationState *s = current_migration;
>  
>      WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
>          if (s->to_dst_file) {
> -            qemu_file_set_error(s->to_dst_file, err);
> +            qemu_file_set_error_obj(s->to_dst_file, ret, err);
> +        } else if (err) {
> +            error_report_err(err);
>          }
>      }
>  }
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
Cédric Le Goater May 15, 2024, 5:15 p.m. UTC | #2 
On 5/15/24 09:04, Eric Auger wrote:
> Hi Cédric,
> 
> On 5/14/24 17:31, Cédric Le Goater wrote:
>> Use it to update the current error of the migration stream if
>> available and if not, simply print out the error. Next changes will
>> update with an error to report.
>>
>> Reviewed-by: Avihai Horon <avihaih@nvidia.com>
>> Acked-by: Fabiano Rosas <farosas@suse.de>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>
>>   Changes in v6:
>>
>>   - Commit log improvements (Avihai)
>>   
>>   include/migration/misc.h | 2 +-
>>   hw/vfio/common.c         | 2 +-
>>   hw/vfio/migration.c      | 4 ++--
>>   migration/migration.c    | 6 ++++--
>>   4 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/migration/misc.h b/include/migration/misc.h
>> index bf7339cc1e6430226127fb6a878d06b458170858..bfadc5613bac614a316e5aed7da95d8c7845cf42 100644
>> --- a/include/migration/misc.h
>> +++ b/include/migration/misc.h
>> @@ -97,7 +97,7 @@ void migration_add_notifier_mode(NotifierWithReturn *notify,
>>   
>>   void migration_remove_notifier(NotifierWithReturn *notify);
>>   bool migration_is_running(void);
>> -void migration_file_set_error(int err);
>> +void migration_file_set_error(int ret, Error *err);
>>   
>>   /* True if incoming migration entered POSTCOPY_INCOMING_DISCARD */
>>   bool migration_in_incoming_postcopy(void);
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index b5102f54a6474a50c6366e8fbce23812d55e384e..ed5ee6349ced78b3bde68d2ee506f78ba1a9dd9c 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -150,7 +150,7 @@ bool vfio_viommu_preset(VFIODevice *vbasedev)
>>   static void vfio_set_migration_error(int err)
> nit: I would have renamed err into ret here to avoid any further confusion.

That was done in v5 :

   https://lore.kernel.org/qemu-devel/20240506092053.388578-11-clg@redhat.com/

in the last patch, that I dropped in v6 because I believe it needs more
work. I will address these last changes, including the err->ret rename,
in a followup series if that's ok with you.


Thanks,

C.




>>   {
>>       if (migration_is_setup_or_active()) {
>> -        migration_file_set_error(err);
>> +        migration_file_set_error(err, NULL);
>>       }
>>   }
>>   
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 06ae40969b6c19037e190008e14f28be646278cd..bf2fd0759ba6e4fb103cc5c1a43edb180a3d0de4 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -726,7 +726,7 @@ static void vfio_vmstate_change_prepare(void *opaque, bool running,
>>            * Migration should be aborted in this case, but vm_state_notify()
>>            * currently does not support reporting failures.
>>            */
>> -        migration_file_set_error(ret);
>> +        migration_file_set_error(ret, NULL);
>>       }
>>   
>>       trace_vfio_vmstate_change_prepare(vbasedev->name, running,
>> @@ -756,7 +756,7 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
>>            * Migration should be aborted in this case, but vm_state_notify()
>>            * currently does not support reporting failures.
>>            */
>> -        migration_file_set_error(ret);
>> +        migration_file_set_error(ret, NULL);
>>       }
>>   
>>       trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state),
>> diff --git a/migration/migration.c b/migration/migration.c
>> index e88b24f1e6cbe82dad3f890c00e264d2ab6ad355..70d66a441bf04761decf91dbe57ce52c57fde58f 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -2994,13 +2994,15 @@ static MigThrError postcopy_pause(MigrationState *s)
>>       }
>>   }
>>   
>> -void migration_file_set_error(int err)
>> +void migration_file_set_error(int ret, Error *err)
>>   {
>>       MigrationState *s = current_migration;
>>   
>>       WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
>>           if (s->to_dst_file) {
>> -            qemu_file_set_error(s->to_dst_file, err);
>> +            qemu_file_set_error_obj(s->to_dst_file, ret, err);
>> +        } else if (err) {
>> +            error_report_err(err);
>>           }
>>       }
>>   }
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> 
> Eric
>
diff mbox series

Patch

diff --git a/include/migration/misc.h b/include/migration/misc.h
index bf7339cc1e6430226127fb6a878d06b458170858..bfadc5613bac614a316e5aed7da95d8c7845cf42 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -97,7 +97,7 @@  void migration_add_notifier_mode(NotifierWithReturn *notify,
 
 void migration_remove_notifier(NotifierWithReturn *notify);
 bool migration_is_running(void);
-void migration_file_set_error(int err);
+void migration_file_set_error(int ret, Error *err);
 
 /* True if incoming migration entered POSTCOPY_INCOMING_DISCARD */
 bool migration_in_incoming_postcopy(void);
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index b5102f54a6474a50c6366e8fbce23812d55e384e..ed5ee6349ced78b3bde68d2ee506f78ba1a9dd9c 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -150,7 +150,7 @@  bool vfio_viommu_preset(VFIODevice *vbasedev)
 static void vfio_set_migration_error(int err)
 {
     if (migration_is_setup_or_active()) {
-        migration_file_set_error(err);
+        migration_file_set_error(err, NULL);
     }
 }
 
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 06ae40969b6c19037e190008e14f28be646278cd..bf2fd0759ba6e4fb103cc5c1a43edb180a3d0de4 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -726,7 +726,7 @@  static void vfio_vmstate_change_prepare(void *opaque, bool running,
          * Migration should be aborted in this case, but vm_state_notify()
          * currently does not support reporting failures.
          */
-        migration_file_set_error(ret);
+        migration_file_set_error(ret, NULL);
     }
 
     trace_vfio_vmstate_change_prepare(vbasedev->name, running,
@@ -756,7 +756,7 @@  static void vfio_vmstate_change(void *opaque, bool running, RunState state)
          * Migration should be aborted in this case, but vm_state_notify()
          * currently does not support reporting failures.
          */
-        migration_file_set_error(ret);
+        migration_file_set_error(ret, NULL);
     }
 
     trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state),
diff --git a/migration/migration.c b/migration/migration.c
index e88b24f1e6cbe82dad3f890c00e264d2ab6ad355..70d66a441bf04761decf91dbe57ce52c57fde58f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2994,13 +2994,15 @@  static MigThrError postcopy_pause(MigrationState *s)
     }
 }
 
-void migration_file_set_error(int err)
+void migration_file_set_error(int ret, Error *err)
 {
     MigrationState *s = current_migration;
 
     WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
         if (s->to_dst_file) {
-            qemu_file_set_error(s->to_dst_file, err);
+            qemu_file_set_error_obj(s->to_dst_file, ret, err);
+        } else if (err) {
+            error_report_err(err);
         }
     }
 }