Re: [Qemu-devel] [RFC PATCH 1/2] VFIO: Drop vfio_container_do_ioctl()

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 08/20/2014 07:52 PM, Gavin Shan wrote:
> The patch drops vfio_container_do_ioctl() and merges its logic to
> parent function call vfio_container_ioctl() so that the subsequent
> patches can reused the found VFIO group in vfio_container_ioctl().
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  hw/misc/vfio.c | 33 +++++++++++++++------------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index 7d5f1bb..1a3e7eb 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -4419,8 +4419,8 @@ static void register_vfio_pci_dev_type(void)
>  
>  type_init(register_vfio_pci_dev_type)
>  
> -static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
> -                                   int req, void *param)
> +int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> +                         int req, void *param)
>  {
>      VFIOGroup *group;
>      VFIOContainer *container;
> @@ -4433,22 +4433,11 @@ static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
>      }
>  
>      container = group->container;
> -    if (group->container) {
> -        ret = ioctl(container->fd, req, param);
> -        if (ret < 0) {
> -            error_report("vfio: failed to ioctl container: ret=%d, %s",
> -                         ret, strerror(errno));
> -        }
> +    if (!container) {
> +        error_report("vfio: no container for group %d\n", groupid);
> +        goto out;
>      }
>  
> -    vfio_put_group(group);
> -
> -    return ret;
> -}
> -
> -int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> -                         int req, void *param)
> -{
>      /* We allow only certain ioctls to the container */
>      switch (req) {
>      case VFIO_CHECK_EXTENSION:
> @@ -4458,8 +4447,16 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
>      default:
>          /* Return an error on unknown requests */
>          error_report("vfio: unsupported ioctl %X", req);
> -        return -1;
> +        goto out;
>      }
>  
> -    return vfio_container_do_ioctl(as, groupid, req, param);
> +    ret = ioctl(container->fd, req, param);
> +    if (ret < 0) {
> +        error_report("vfio: failed to ioctl container: ret=%d, %s",
> +                     ret, strerror(errno));
> +    }
> +
> +out:
> +    vfio_put_group(group);
> +    return ret;


Nack. We specifically separated an ioctl-filtering function and actual code
which does ioctl. I understand that you want to save on vfio_get_group()
calls for the next EEH patch but this is not a hot path anyway.




>  }
> 


-- 
Alexey