Re: [PATCH v3 04/15] vfio/common: Propagate KVM_SET_DEVICE_ATTR error if any

["Cédric Le Goater" <clg@redhat.com>]

On 10/3/23 12:14, Eric Auger wrote:
> In the VFIO_SPAPR_TCE_v2_IOMMU container case, when
> KVM_SET_DEVICE_ATTR fails, we currently don't propagate the
> error as we do on the vfio_spapr_create_window() failure
> case. Let's align the code. Take the opportunity to
> reword the error message and make it more explicit.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> I think thise should end up in the
>      if (!container->initialized) {
>          if (!container->error) {
> path and call the error_propagate_prepend()

We could have this case also

     if (memory_region_is_ram_device(section->mr)) {
         error_report("failed to vfio_dma_map. pci p2p may not work");
         return;
     }

which was added by commit 567b5b309abe ("vfio/pci: Relax DMA map errors
for MMIO regions"). There were a few changes in the failure path, like
commit ac6dc3894fbb ("vfio: Generalize vfio_listener_region_add failure
path") and it is unclear to me which one will be used. Anyhow, this needs
some cleanup and this is what this patchset is proposing. Let's move on :

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/vfio/common.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 4e122fc4e4..c54a72ec80 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -878,11 +878,11 @@ static void vfio_listener_region_add(MemoryListener *listener,
>                   QLIST_FOREACH(group, &container->group_list, container_next) {
>                       param.groupfd = group->fd;
>                       if (ioctl(vfio_kvm_device_fd, KVM_SET_DEVICE_ATTR, &attr)) {
> -                        error_report("vfio: failed to setup fd %d "
> -                                     "for a group with fd %d: %s",
> -                                     param.tablefd, param.groupfd,
> -                                     strerror(errno));
> -                        return;
> +                        error_setg_errno(&err, errno,
> +                                         "vfio: failed GROUP_SET_SPAPR_TCE for "
> +                                         "KVM VFIO device %d and group fd %d",
> +                                         param.tablefd, param.groupfd);
> +                        goto fail;
>                       }
>                       trace_vfio_spapr_group_attach(param.groupfd, param.tablefd);
>                   }