Re: [PATCH] Move vgdb special case into remote_filesystem_is_local

[Pedro Alves <palves@redhat.com>]

On 05/15/2015 02:19 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 05/15/2015 10:02 AM, Gary Benson wrote:
>>> Pedro Alves wrote:
>>>> On 04/27/2015 03:51 PM, Gary Benson wrote:
>>>>> Valgrind GDB (vgdb) presents itself as a remote target but
>>>>> works on the local filesystem.  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.
>>>>
>>>> Can you give an example target where we'd want this to behave
>>>> differently?
>>>>
>>>> E.g,. what should happen with "target sim" ?
>>>
>>> I don't understand what you're asking.  "target sim" doesn't use
>>> remote_filesystem_is_local, (to_filesystem_is_local for sim is the
>>> default, tdefault_filesystem_is_local, which returns 1 always).
>>
>> 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?

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.

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

Thanks,
Pedro Alves