Hi Cédric,
On 3/6/24 14:34, Cédric Le Goater wrote:
> vfio_set_migration_error() sets the 'return' error on the migration
> stream if a migration is in progress. To improve error reporting, add
> a new Error* argument to also set the Error object on the migration
> stream, if a migration is progress.
>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>
> Changes in v4:
>
> - Dropped log_global_stop() and log_global_sync() changes
>
> hw/vfio/common.c | 39 ++++++++++++++++++++-------------------
> 1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 65a11dc088524647541db97b7b8d6f07e5044728..e26574617e5ef75c27a84dc9bb13c8f040353b6c 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -148,16 +148,18 @@ bool vfio_viommu_preset(VFIODevice *vbasedev)
> return vbasedev->bcontainer->space->as != &address_space_memory;
> }
>
> -static void vfio_set_migration_error(int err)
> +static void vfio_set_migration_error(int ret, Error *err)
> {
> MigrationState *ms = migrate_get_current();
>
> if (migration_is_setup_or_active(ms->state)) {
> WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) {
> if (ms->to_dst_file) {
> - qemu_file_set_error(ms->to_dst_file, err);
> + qemu_file_set_error_obj(ms->to_dst_file, ret, err);
> }
> }
> + } else {
does that case exist?
Eric
> + error_report_err(err);
> }
> }
>
> @@ -304,9 +306,10 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> iova, iova + iotlb->addr_mask);
>
> if (iotlb->target_as != &address_space_memory) {
> - error_report("Wrong target AS \"%s\", only system memory is allowed",
> - iotlb->target_as->name ? iotlb->target_as->name : "none");
> - vfio_set_migration_error(-EINVAL);
> + error_setg(&local_err,
> + "Wrong target AS \"%s\", only system memory is allowed",
> + iotlb->target_as->name ? iotlb->target_as->name : "none");
> + vfio_set_migration_error(-EINVAL, local_err);
> return;
> }
>
> @@ -339,11 +342,12 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> ret = vfio_container_dma_unmap(bcontainer, iova,
> iotlb->addr_mask + 1, iotlb);
> if (ret) {
> - error_report("vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
> - "0x%"HWADDR_PRIx") = %d (%s)",
> - bcontainer, iova,
> - iotlb->addr_mask + 1, ret, strerror(-ret));
> - vfio_set_migration_error(ret);
> + error_setg(&local_err,
> + "vfio_container_dma_unmap(%p, 0x%"HWADDR_PRIx", "
> + "0x%"HWADDR_PRIx") = %d (%s)",
> + bcontainer, iova,
> + iotlb->addr_mask + 1, ret, strerror(-ret));
> + vfio_set_migration_error(ret, local_err);
> }
> }
> out:
> @@ -1125,8 +1129,7 @@ static void vfio_listener_log_global_stop(MemoryListener *listener)
> if (ret) {
> error_prepend(&local_err,
> "vfio: Could not stop dirty page tracking - ");
> - error_report_err(local_err);
> - vfio_set_migration_error(ret);
> + vfio_set_migration_error(ret, local_err);
> }
> }
>
> @@ -1243,14 +1246,14 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> trace_vfio_iommu_map_dirty_notify(iova, iova + iotlb->addr_mask);
>
> if (iotlb->target_as != &address_space_memory) {
> - error_report("Wrong target AS \"%s\", only system memory is allowed",
> - iotlb->target_as->name ? iotlb->target_as->name : "none");
> + error_setg(&local_err,
> + "Wrong target AS \"%s\", only system memory is allowed",
> + iotlb->target_as->name ? iotlb->target_as->name : "none");
> goto out;
> }
>
> rcu_read_lock();
> if (!vfio_get_xlat_addr(iotlb, NULL, &translated_addr, NULL, &local_err)) {
> - error_report_err(local_err);
> goto out_lock;
> }
>
> @@ -1261,7 +1264,6 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> "vfio_iommu_map_dirty_notify(%p, 0x%"HWADDR_PRIx", "
> "0x%"HWADDR_PRIx") failed :", bcontainer, iova,
> iotlb->addr_mask + 1);
> - error_report_err(local_err);
> }
>
> out_lock:
> @@ -1269,7 +1271,7 @@ out_lock:
>
> out:
> if (ret) {
> - vfio_set_migration_error(ret);
> + vfio_set_migration_error(ret, local_err);
> }
> }
>
> @@ -1389,8 +1391,7 @@ static void vfio_listener_log_sync(MemoryListener *listener,
> if (vfio_devices_all_dirty_tracking(bcontainer)) {
> ret = vfio_sync_dirty_bitmap(bcontainer, section, &local_err);
> if (ret) {
> - error_report_err(local_err);
> - vfio_set_migration_error(ret);
> + vfio_set_migration_error(ret, local_err);
> }
> }
> }