Re: [Qemu-devel] [PATCH 1/4] monitor: Less restrictive fd matching for fd sets

[Corey Bryant]

On 10/05/2012 02:35 PM, Eric Blake wrote:
> On 10/05/2012 12:07 PM, Corey Bryant wrote:
> > Currently, in order to use a file descriptor that resides in an
> > fd set, the access mode flag of the qemu_open() call has to be
> > an exact match of the access mode flag of an fd in the requested
> > fd set.
> >
> > This patch lightens up that restriction.  For example, if QEMU
> > attempts to qemu_open() "/dev/fdset/2" with O_RDONLY or O_WRONLY,
> > and an fd in fd set 2 has O_RDWR, the call will now be successful.
>
> I was right for wondering if we'd need this, back when fdsets were first
> implemented. :)

Yeah yeah yeah :)

> However...
>
> > Signed-off-by: Corey Bryant <address@hidden>
> > ---
> >   monitor.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index a0e3ffb..34a968c 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -2304,7 +2304,8 @@ int monitor_fdset_get_fd(int64_t fdset_id, int flags)
> >                   return -1;
> >               }
> >
> > -            if ((flags & O_ACCMODE) == (mon_fd_flags & O_ACCMODE)) {
> > +            if ((mon_fd_flags & O_ACCMODE) == O_RDWR ||
> > +                (mon_fd_flags & O_ACCMODE) == (flags & O_ACCMODE)) {
> >                   return mon_fdset_fd->fd;
> >               }
>
> Is this always the right fd to return?  Suppose I create a set that
> contains two fds, one O_RDONLY and another O_RDWR; and that the current
> qemu_open() is requesting O_RDONLY.  It feels like we would want to get
> the O_RDONLY fd returned, even if it comes later in the set, since it is
> nominally safer.
>
> That is, I wonder if this should really be implemented as an iteration
> over the set that selects the best possible match, where more than one
> fd can match, but at the end of the iteration, we have the closest match
> possible, rather than the first (possibly inexact) match.

Actually I think we can go back to requiring an exact match if we use 
your new proposal of adding a new command that allows multiple fd's to 
be added to an fd set over the command line.

--
Regards,
Corey Bryant