| Message ID | 20200305142945.216465-3-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Ram blocks with resizeable anonymous allocations under POSIX | expand |
On Thursday, March 5, 2020 11:29:32 AM -03 David Hildenbrand wrote: > Everybody discards the error. Let's error_report() instead so this error > doesn't get lost. > > This is now the same error handling as in qemu_vfio_do_mapping(). However, > we don't report any errors via the return value to the caller. This seems > to be one of these "will never happen, but let's better print an error > because the caller can't handle it either way" cases. > > Cc: Richard Henderson <rth@twiddle.net> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > Cc: Alex Williamson <alex.williamson@redhat.com> > Cc: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- Acked-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > util/vfio-helpers.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c > index f31aa77ffe..7085ca702c 100644 > --- a/util/vfio-helpers.c > +++ b/util/vfio-helpers.c > @@ -541,8 +541,7 @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void > *host, size_t size, /** > * Undo the DMA mapping from @s with VFIO, and remove from mapping list. > */ > -static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping, > - Error **errp) > +static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping) > { > int index; > struct vfio_iommu_type1_dma_unmap unmap = { > @@ -557,7 +556,7 @@ static void qemu_vfio_undo_mapping(QEMUVFIOState *s, > IOVAMapping *mapping, assert(QEMU_IS_ALIGNED(mapping->size, > qemu_real_host_page_size)); assert(index >= 0 && index < s->nr_mappings); > if (ioctl(s->container, VFIO_IOMMU_UNMAP_DMA, &unmap)) { > - error_setg_errno(errp, errno, "VFIO_UNMAP_DMA failed"); > + error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno)); > } > memmove(mapping, &s->mappings[index + 1], > sizeof(s->mappings[0]) * (s->nr_mappings - index - 1)); > @@ -622,7 +621,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, > size_t size, assert(qemu_vfio_verify_mappings(s)); > ret = qemu_vfio_do_mapping(s, host, size, iova0); > if (ret) { > - qemu_vfio_undo_mapping(s, mapping, NULL); > + qemu_vfio_undo_mapping(s, mapping); > goto out; > } > s->low_water_mark += size; > @@ -682,7 +681,7 @@ void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host) > if (!m) { > goto out; > } > - qemu_vfio_undo_mapping(s, m, NULL); > + qemu_vfio_undo_mapping(s, m); > out: > qemu_mutex_unlock(&s->lock); > } > @@ -699,7 +698,7 @@ void qemu_vfio_close(QEMUVFIOState *s) > return; > } > while (s->nr_mappings) { > - qemu_vfio_undo_mapping(s, &s->mappings[s->nr_mappings - 1], NULL); > + qemu_vfio_undo_mapping(s, &s->mappings[s->nr_mappings - 1]); > } > ram_block_notifier_remove(&s->ram_notifier); > qemu_vfio_reset(s); -- Murilo
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c index f31aa77ffe..7085ca702c 100644 --- a/util/vfio-helpers.c +++ b/util/vfio-helpers.c @@ -541,8 +541,7 @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size, /** * Undo the DMA mapping from @s with VFIO, and remove from mapping list. */ -static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping, - Error **errp) +static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping) { int index; struct vfio_iommu_type1_dma_unmap unmap = { @@ -557,7 +556,7 @@ static void qemu_vfio_undo_mapping(QEMUVFIOState *s, IOVAMapping *mapping, assert(QEMU_IS_ALIGNED(mapping->size, qemu_real_host_page_size)); assert(index >= 0 && index < s->nr_mappings); if (ioctl(s->container, VFIO_IOMMU_UNMAP_DMA, &unmap)) { - error_setg_errno(errp, errno, "VFIO_UNMAP_DMA failed"); + error_report("VFIO_UNMAP_DMA failed: %s", strerror(errno)); } memmove(mapping, &s->mappings[index + 1], sizeof(s->mappings[0]) * (s->nr_mappings - index - 1)); @@ -622,7 +621,7 @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size, assert(qemu_vfio_verify_mappings(s)); ret = qemu_vfio_do_mapping(s, host, size, iova0); if (ret) { - qemu_vfio_undo_mapping(s, mapping, NULL); + qemu_vfio_undo_mapping(s, mapping); goto out; } s->low_water_mark += size; @@ -682,7 +681,7 @@ void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host) if (!m) { goto out; } - qemu_vfio_undo_mapping(s, m, NULL); + qemu_vfio_undo_mapping(s, m); out: qemu_mutex_unlock(&s->lock); } @@ -699,7 +698,7 @@ void qemu_vfio_close(QEMUVFIOState *s) return; } while (s->nr_mappings) { - qemu_vfio_undo_mapping(s, &s->mappings[s->nr_mappings - 1], NULL); + qemu_vfio_undo_mapping(s, &s->mappings[s->nr_mappings - 1]); } ram_block_notifier_remove(&s->ram_notifier); qemu_vfio_reset(s);
Everybody discards the error. Let's error_report() instead so this error doesn't get lost. This is now the same error handling as in qemu_vfio_do_mapping(). However, we don't report any errors via the return value to the caller. This seems to be one of these "will never happen, but let's better print an error because the caller can't handle it either way" cases. Cc: Richard Henderson <rth@twiddle.net> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> --- util/vfio-helpers.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)