On Tue, Aug 17, 2021 at 01:59:22PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Tue, Aug 17, 2021 at 12:56 PM Michal Privoznik <mpriv...@redhat.com>
> wrote:
>
> > When opening a path that starts with "/dev/fdset/" the control
> > jumps into qemu_parse_fdset() and then into
> > monitor_fdset_dup_fd_add(). In here, corresponding fdset is found
> > and then all FDs from the set are iterated over trying to find an
> > FD that matches expected access mode. For instance, if caller
> > wants O_WRONLY then the FD set has to contain an O_WRONLY FD.
> >
> > If no such FD is found then errno is set to EACCES which results
> > in very misleading error messages, for instance:
> >
> > Could not dup FD for /dev/fdset/3 flags 441: Permission denied
> >
> > There is no permission issue, the problem is that there was no FD
> > within given fdset that was in expected access mode. Therefore,
> > let's set errno to EBADFD, which gives us somewhat better
> > error messages:
> >
> > Could not dup FD for /dev/fdset/3 flags 441: File descriptor in bad state
> >
> > Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
> >
>
> I am not sure this is any better. If you try to open a read-only file, the
> system also reports EACCES (Permission denied). This is what the current
> code models, I believe.
If we want better error reporting quality for this method we ought
to just stop using errno and wire up a Error **errp parameter.
>
>
> > ---
> > monitor/misc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/monitor/misc.c b/monitor/misc.c
> > index ffe7966870..a0eda0d574 100644
> > --- a/monitor/misc.c
> > +++ b/monitor/misc.c
> > @@ -1347,7 +1347,7 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int
> > flags)
> > }
> >
> > if (fd == -1) {
> > - errno = EACCES;
> > + errno = EBADFD;
> > return -1;
> > }
> >
> > --
> > 2.31.1
> >
> >
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
