On Tue, 2014-08-26 at 20:27 +1000, Alexey Kardashevskiy wrote:
> 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 <gws...@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.
I think the split was a result of a comment from me suggesting that a
wrapper function might make the code flow better. As implemented, I
don't think that result was every actually realized, but the code was
functional, so I didn't push it any further. I don't have any issue
with recombining it. Thanks,
Alex
