Re: [PATCH] Move vgdb special case into remote_filesystem_is_local

[Gary Benson <gbenson@redhat.com>]

Pedro Alves wrote:
> On 05/15/2015 02:19 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > When you say:
> > >
> > >  gdb_bfd_open contained a special case to make vgdb work with
> > >  "target:" sysroots, but the implementation meant that GDB would
> > >  fall back to the local filesystem if *any* to_fileio_open
> > >  method failed with ENOSYS for *any* reason.
> > >
> > > I'd prefer to get an example target for one of those "if *any*
> > > to_fileio_open ... *any* reason".  I'd like to understand the
> > > real motivation for the change.  Because otherwise I get to
> > > wonder why would we handle any other target that goes through
> > > this path differently.
> > 
> > Ah, ok, I get what you're asking.
> > 
> > In what's upstream right now, the only path (I think) that you can
> > get to the point in gdb_bfd_open with the workaround is if you're
> > using a remote target that doesn't support file retrieval.  But,
> > in the namespace-awareness series I posted, target_fileio_open can
> > fail with ENOSYS if setns is not available.  That's the reason I
> > made the change.
> 
> I'm still confused on that rationale, as it leaves one important
> detail out: when target_fileio_open fails with ENOSYS because setns
> is not available, I assume that gdb falls back to the local
> filesystem.  But isn't that what should happen?
> 
> After your patch, we'll issue remote_hostio_open from within
> remote_filesystem_is_local, and if the remote side doesn't support
> setns, we'll get ENOSYS to "open", and thus fallback to local
> anyway?

I'm trying to catch the specific case that a) you're using a remote
target, b) that doesn't support file retrieval, and c) the user has
not set any sysroot.  In that case the user is presumably using a
"remote" client that operates on the local filesystem... so GDB should
access the local filesystem.

For any other target_fileio_open failures GDB should not continue.
For example, the user attaches to a process in a container, and that
process's executable is "/bin/bash".  If GDB can't open /bin/bash
_in_that_container_ (because setns isn't implemented) then GDB should
not try to access /bin/bash in it's own container.  They might be
different files.

> In any case, (I have yet to go read your v2 of that series), it
> sounds wrong suspicious to return ENOSYS for that case.  ENOSYS
> would indicate that "open" is not implemented, but that doesn't
> sound like the case you have.  "open" is indeed implemented, but it
> happens that it can't satisfy the requested path.  Thus, something
> like EINVAL, EOPNOTSUPP or ENODEV may be more appropriate.

EOPNOTSUPP is for sockets I think.  ENODEV seems a better match than
EINVAL, but I don't really have a good feel for how these are used
in other cases.

I could change the subsequent series to detech ENOSYS from setns
and return with errno == ENODEV or EINVAL if you like?

> As I said, I agree with moving the checks to
> target_filesystem_is_local time, but for a different rationale.

Ok.

Cheers,
Gary

-- 
http://gbenson.net/