Re: [Qemu-devel] [PATCH v7 2/3] VFIO: Helper function to retrieve container fd

[Alex Williamson]

On Wed, 2014-05-28 at 01:17 +1000, Gavin Shan wrote:
> On Tue, May 27, 2014 at 08:16:47AM -0600, Alex Williamson wrote:
> >On Tue, 2014-05-27 at 18:51 +1000, Gavin Shan wrote:
> >> The patch adds function vfio_get_container_fd_by_group_id() to retrieve
> >> the container's fd of the specified VFIO group. The fd will be used
> >> by subsequent patches.
> >> 
> >> Signed-off-by: Gavin Shan <address@hidden>
> >> ---
> >>  hw/misc/vfio.c         | 17 +++++++++++++++++
> >>  include/hw/misc/vfio.h |  1 +
> >>  2 files changed, 18 insertions(+)
> >> 
> >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> >> index 0796abf..424d467 100644
> >> --- a/hw/misc/vfio.c
> >> +++ b/hw/misc/vfio.c
> >> @@ -4310,3 +4310,20 @@ put_group_exit:
> >>  
> >>      return n;
> >>  }
> >> +
> >> +int vfio_get_container_fd_by_group_id(int32_t groupid)
> >> +{
> >> +    VFIOGroup *group;
> >> +
> >> +    QLIST_FOREACH(group, &group_list, next) {
> >> +        if (group->groupid == groupid) {
> >> +            if (group->container) {
> >> +                return group->container->fd;
> >> +            } else {
> >> +                return -ENOENT;
> >> +            }
> >> +        }
> >> +    }
> >> +
> >> +    return -ENOENT;
> >> +}
> >> diff --git a/include/hw/misc/vfio.h b/include/hw/misc/vfio.h
> >> index 53ec665..a5afebd 100644
> >> --- a/include/hw/misc/vfio.h
> >> +++ b/include/hw/misc/vfio.h
> >> @@ -30,4 +30,5 @@ static inline long vfio_kvm_notify(Notifier *n, unsigned 
> >> request, void *data)
> >>      return p.ret;
> >>  }
> >>  
> >> +extern int vfio_get_container_fd_by_group_id(int32_t groupid);
> >>  #endif
> >
> >NAK, get implies a reference, there is none here.
> >
> 
> Well, how about to have vfio_find_container_fd_by_group_id()
> or just vfio_container_fd_by_group_id() ?

Ugh, the patches are flying too fast, I can't even keep track that this
is a QEMU patch.  I'm not as concerned about the "get" in QEMU, but do
we really want to create a future hotplug issue where we've handed out a
file descriptor to another part of QEMU without a path to disable it
when it gets closed?  I really have no idea what else you're relying on
being exported here since we're building patches on top of out-of-tree
patches.