| Message ID | 20240514153130.394307-4-clg@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vfio: Improve error reporting (part 2 | expand |
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
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 --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); } } }