On 18.02.20 23:07, Peter Xu wrote: > On Wed, Feb 12, 2020 at 02:42:41PM +0100, David Hildenbrand wrote: >> Everybody discards the error. Let's error_report() instead so this error >> doesn't get lost. >> >> Cc: Richard Henderson <[email protected]> >> Cc: Paolo Bonzini <[email protected]> >> Cc: Eduardo Habkost <[email protected]> >> Cc: Marcel Apfelbaum <[email protected]> >> Cc: Alex Williamson <[email protected]> >> Cc: Stefan Hajnoczi <[email protected]> >> Signed-off-by: David Hildenbrand <[email protected]> > > IMHO error_setg() should be preferred comparing to error_report() > because it has a context to be delivered to the caller, so the error > has a better chance to be used in a better way (e.g., QMP only > supports error_setg()).
Please note that I decided to go for error_report() because that's also what's being done in the matching opposite way: qemu_vfio_do_mapping(). > > A better solution is that we deliver the error upper. For example, > qemu_vfio_dma_map() is one caller of qemu_vfio_undo_mapping, if you > see the callers of qemu_vfio_dma_map() you'll notice most of them has > Error** defined (e.g., nvme_init_queue). Then we can link all of them > up. Propagating errors is helpful if the caller can actually do something with the error. If it's a function that's supposed to never fail (which is the case here obviously), then there is not much benefit in doing so. > > Another lazy solution (and especially if vfio-helpers are still mostly > used only by advanced users) is we can simply pass in &error_abort for > the three callers then they won't be missed... I prefer this variant, as it matches qemu_vfio_do_mapping() - except that we don't report -errno - which is fine, because the function is expected to not fail in any sane use case. Thanks! -- Thanks, David / dhildenb
