[6/7] Implement multiple-filesystem support for remote targets

Message ID 1429186791-6867-7-git-send-email-gbenson@redhat.com
State Superseded
Headers

Commit Message

Gary Benson April 16, 2015, 12:19 p.m. UTC
  This commit allows GDB to access executables and shared libraries
on remote targets where the inferior and the remote stub do not
share a common filesystem.  A new packet "vFile:setfs" is added
to the remote protocol and the three remote hostio functions with
filename arguments are modified to send "vFile:setfs" packets as
necessary.

gdb/ChangeLog:

	* remote.c (struct remote_state) <fs_pid>: New field.
	(new_remote_state): Initialize the above.
	(PACKET_vFile_setfs): New enum value.
	(remote_fs_pid): New static variable.
	(remote_hostio_set_fs_deferred): New function.
	(remote_hostio_set_filesystem): Likewise.
	(remote_hostio_open): Call the above.
	(remote_hostio_unlink): Likewise.
	(remote_hostio_readlink): Likewise.
	(init_remote_ops): Initialize to_fileio_set_fs.
	(_initialize_remote): Register new "set/show remote
	hostio-setfs-packet" command.
	* NEWS: Mention that GDB and gdbserver support attaching to
	processes in different mount namespaces on GNU/Linux systems.
	Announce new vFile:setfs packet.

gdb/doc/ChangeLog:

	* gdb.texinfo (Remote Configuration): Document the
	"set/show remote hostio-setfs-packet" command.
	(Host I/O Packets): Document the vFile:setfs packet.
---
 gdb/ChangeLog       |   18 +++++++++++++++
 gdb/NEWS            |    8 +++++++
 gdb/doc/ChangeLog   |    6 +++++
 gdb/doc/gdb.texinfo |   11 +++++++++
 gdb/remote.c        |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 101 insertions(+), 0 deletions(-)
  

Comments

Eli Zaretskii April 16, 2015, 3:12 p.m. UTC | #1
> From: Gary Benson <gbenson@redhat.com>
> Date: Thu, 16 Apr 2015 13:19:50 +0100
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Remote Configuration): Document the
> 	"set/show remote hostio-setfs-packet" command.
> 	(Host I/O Packets): Document the vFile:setfs packet.
> ---
>  gdb/ChangeLog       |   18 +++++++++++++++
>  gdb/NEWS            |    8 +++++++
>  gdb/doc/ChangeLog   |    6 +++++
>  gdb/doc/gdb.texinfo |   11 +++++++++

I think the patch for the docs leaves too much unsaid to be useful.
E.g., after reading it, I was left as clueless as before regarding
what this feature is about.  What is a "mount namespace", for
starters?  What does it mean to "set a filesystem"?

Please consider explaining a bit of background about this in the
manual.

Thanks.
  
Pedro Alves April 17, 2015, 3:06 p.m. UTC | #2
On 04/16/2015 01:19 PM, Gary Benson wrote:
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -367,6 +367,12 @@ struct remote_state
>  
>    /* The branch trace configuration.  */
>    struct btrace_config btrace_config;
> +
> +  /* The argument to the last "vFile:setfs:" packet we sent, used
> +     to avoid sending repeated unnecessary "vFile:setfs:" packets.
> +     Initialized to -1 to indicate that no "vFile:setfs:" packet
> +     has yet been sent.  */
> +  int fs_pid;
>  };
>  

> +/* Process ID of inferior whose filesystem remote_hostio functions
> +   that take FILENAME arguments will use.  Zero means to use the
> +   remote stub's filesystem.  */
> +
> +static int remote_fs_pid = 0;

What's the rationale for not putting this one in "struct remote_state" ?

Thanks,
Pedro Alves
  
Pedro Alves April 17, 2015, 3:31 p.m. UTC | #3
On 04/16/2015 01:19 PM, Gary Benson wrote:

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 62cbdcb..a9f05e4 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -42,6 +42,10 @@
>    (no "set sysroot" or "file" commands are required).  See "New remote
>    packets" below.
>  
> +* GDB and gdbserver now support attaching to processes running in
> +  different mount namespaces from the debugger on GNU/Linux systems.
> +  See "New remote packets" below.

Reading this, I'm left wondering what this really means, because I
think we could already attach before.  What the user needed to do before
was point the "set sysroot" somewhere, while now, if the sysroot
is left to "target:", that's no longer necessary.  Is that right?

Thanks,
Pedro Alves
  
Gary Benson April 17, 2015, 3:59 p.m. UTC | #4
Pedro Alves wrote:
> On 04/16/2015 01:19 PM, Gary Benson wrote:
> > --- a/gdb/remote.c
> > +++ b/gdb/remote.c
> > @@ -367,6 +367,12 @@ struct remote_state
> >  
> >    /* The branch trace configuration.  */
> >    struct btrace_config btrace_config;
> > +
> > +  /* The argument to the last "vFile:setfs:" packet we sent, used
> > +     to avoid sending repeated unnecessary "vFile:setfs:" packets.
> > +     Initialized to -1 to indicate that no "vFile:setfs:" packet
> > +     has yet been sent.  */
> > +  int fs_pid;
> >  };
> >  
> 
> > +/* Process ID of inferior whose filesystem remote_hostio functions
> > +   that take FILENAME arguments will use.  Zero means to use the
> > +   remote stub's filesystem.  */
> > +
> > +static int remote_fs_pid = 0;
> 
> What's the rationale for not putting this one in "struct
> remote_state" ?

I don't have one :)  Would you prefer it there?

Cheers,
Gary
  
Gary Benson April 17, 2015, 4:01 p.m. UTC | #5
Pedro Alves wrote:
> On 04/16/2015 01:19 PM, Gary Benson wrote:
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index 62cbdcb..a9f05e4 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -42,6 +42,10 @@
> >    (no "set sysroot" or "file" commands are required).  See "New remote
> >    packets" below.
> >  
> > +* GDB and gdbserver now support attaching to processes running in
> > +  different mount namespaces from the debugger on GNU/Linux systems.
> > +  See "New remote packets" below.
> 
> Reading this, I'm left wondering what this really means, because I
> think we could already attach before.  What the user needed to do
> before was point the "set sysroot" somewhere, while now, if the
> sysroot is left to "target:", that's no longer necessary.  Is that
> right?

Yeah, it's more about accessing files than attaching per se.
I think I need to spend some time rewriting the docs for this
series...

Cheers,
Gary
  
Pedro Alves April 17, 2015, 4:07 p.m. UTC | #6
On 04/17/2015 04:59 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 04/16/2015 01:19 PM, Gary Benson wrote:
>>> --- a/gdb/remote.c
>>> +++ b/gdb/remote.c
>>> @@ -367,6 +367,12 @@ struct remote_state
>>>  
>>>    /* The branch trace configuration.  */
>>>    struct btrace_config btrace_config;
>>> +
>>> +  /* The argument to the last "vFile:setfs:" packet we sent, used
>>> +     to avoid sending repeated unnecessary "vFile:setfs:" packets.
>>> +     Initialized to -1 to indicate that no "vFile:setfs:" packet
>>> +     has yet been sent.  */
>>> +  int fs_pid;
>>>  };
>>>  
>>
>>> +/* Process ID of inferior whose filesystem remote_hostio functions
>>> +   that take FILENAME arguments will use.  Zero means to use the
>>> +   remote stub's filesystem.  */
>>> +
>>> +static int remote_fs_pid = 0;
>>
>> What's the rationale for not putting this one in "struct
>> remote_state" ?
> 
> I don't have one :)  Would you prefer it there?

It's not really about personal preference: if on a
multi-remote-connections world the flag would be remote connection
specific, it should be in remote_state.  It seemed that way
to me, but if it isn't, then it should be left a global.

Thanks,
Pedro Alves
  
Gary Benson April 17, 2015, 4:20 p.m. UTC | #7
Pedro Alves wrote:
> On 04/17/2015 04:59 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> >> On 04/16/2015 01:19 PM, Gary Benson wrote:
> > > > --- a/gdb/remote.c
> > > > +++ b/gdb/remote.c
> > > > @@ -367,6 +367,12 @@ struct remote_state
> > > >  
> > > >    /* The branch trace configuration.  */
> > > >    struct btrace_config btrace_config;
> > > > +
> > > > +  /* The argument to the last "vFile:setfs:" packet we sent, used
> > > > +     to avoid sending repeated unnecessary "vFile:setfs:" packets.
> > > > +     Initialized to -1 to indicate that no "vFile:setfs:" packet
> > > > +     has yet been sent.  */
> > > > +  int fs_pid;
> > > >  };
> > > >  
> > >
> > > > +/* Process ID of inferior whose filesystem remote_hostio functions
> > > > +   that take FILENAME arguments will use.  Zero means to use the
> > > > +   remote stub's filesystem.  */
> > > > +
> > > > +static int remote_fs_pid = 0;
> > >
> > > What's the rationale for not putting this one in "struct
> > > remote_state" ?
> > 
> > I don't have one :)  Would you prefer it there?
> 
> It's not really about personal preference: if on a
> multi-remote-connections world the flag would be remote connection
> specific, it should be in remote_state.  It seemed that way to me,
> but if it isn't, then it should be left a global.

Ah, it is probably connection-specific, I'll move it.

Cheers,
Gary
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 62cbdcb..a9f05e4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -42,6 +42,10 @@ 
   (no "set sysroot" or "file" commands are required).  See "New remote
   packets" below.
 
+* GDB and gdbserver now support attaching to processes running in
+  different mount namespaces from the debugger on GNU/Linux systems.
+  See "New remote packets" below.
+
 * Python Scripting
 
   ** gdb.Objfile objects have a new attribute "username",
@@ -116,6 +120,10 @@  qXfer:exec-file:read
   Return the full absolute name of the file that was executed to
   create a process running on the remote system.
 
+vFile:setfs:
+  Set the filesystem accessed by vFile: packets with filename
+  arguments.
+
 * The info record command now shows the recording format and the
   branch tracing configuration for the current thread when using
   the btrace record target.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e7872dd..fc0f4b1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -19889,6 +19889,10 @@  are:
 @tab @code{vFile:fstat}
 @tab Host I/O
 
+@item @code{hostio-setfs-packet}
+@tab @code{vFile:setfs}
+@tab Host I/O
+
 @item @code{noack-packet}
 @tab @code{QStartNoAckMode}
 @tab Packet acknowledgment
@@ -37533,6 +37537,13 @@  An empty response indicates that this operation is not recognized.
 These are the supported Host I/O operations:
 
 @table @samp
+@item vFile:setfs: @var{pid}
+Set the filesystem accessed by @code{vFile} packets with
+@var{filename} arguments to the filesystem as seen by process
+@var{pid}.  If @var{pid} is zero, set the filesystem to that
+seen by the remote stub.  Return 0 on success, or -1 if an
+error occurs.
+
 @item vFile:open: @var{filename}, @var{flags}, @var{mode}
 Open a file at @var{filename} and return a file descriptor for it, or
 return -1 if an error occurs.  The @var{filename} is a string,
diff --git a/gdb/remote.c b/gdb/remote.c
index 3b2325f..3202871 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -367,6 +367,12 @@  struct remote_state
 
   /* The branch trace configuration.  */
   struct btrace_config btrace_config;
+
+  /* The argument to the last "vFile:setfs:" packet we sent, used
+     to avoid sending repeated unnecessary "vFile:setfs:" packets.
+     Initialized to -1 to indicate that no "vFile:setfs:" packet
+     has yet been sent.  */
+  int fs_pid;
 };
 
 /* Private data that we'll store in (struct thread_info)->private.  */
@@ -409,6 +415,7 @@  new_remote_state (void)
   result->buf = xmalloc (result->buf_size);
   result->remote_traceframe_number = -1;
   result->last_sent_signal = GDB_SIGNAL_0;
+  result->fs_pid = -1;
 
   return result;
 }
@@ -1243,6 +1250,7 @@  enum {
   PACKET_Z2,
   PACKET_Z3,
   PACKET_Z4,
+  PACKET_vFile_setfs,
   PACKET_vFile_open,
   PACKET_vFile_pread,
   PACKET_vFile_pwrite,
@@ -9879,6 +9887,46 @@  remote_hostio_send_command (int command_bytes, int which_packet,
   return ret;
 }
 
+/* Process ID of inferior whose filesystem remote_hostio functions
+   that take FILENAME arguments will use.  Zero means to use the
+   remote stub's filesystem.  */
+
+static int remote_fs_pid = 0;
+
+/* Implementation of to_fileio_set_fs.  */
+
+static void
+remote_hostio_set_fs_deferred (struct target_ops *self, int pid)
+{
+  remote_fs_pid = pid;
+}
+
+/* Set the filesystem remote_hostio functions that take FILENAME
+   arguments will use.  */
+
+static void
+remote_hostio_set_filesystem (void)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  if (rs->fs_pid == -1 || remote_fs_pid != rs->fs_pid)
+    {
+      char *p = rs->buf;
+      int left = get_remote_packet_size () - 1;
+      int remote_errno;
+      char arg[9];
+
+      remote_buffer_add_string (&p, &left, "vFile:setfs:");
+
+      xsnprintf (arg, sizeof (arg), "%x", remote_fs_pid);
+      remote_buffer_add_string (&p, &left, arg);
+
+      if (remote_hostio_send_command (p - rs->buf, PACKET_vFile_setfs,
+				      &remote_errno, NULL, NULL) == 0)
+	rs->fs_pid = remote_fs_pid;
+    }
+}
+
 /* Return nonzero if the filesystem accessed by the target_fileio_*
    methods is the local filesystem, zero otherwise.  */
 
@@ -9901,6 +9949,8 @@  remote_hostio_open (struct target_ops *self,
   char *p = rs->buf;
   int left = get_remote_packet_size () - 1;
 
+  remote_hostio_set_filesystem ();
+
   remote_buffer_add_string (&p, &left, "vFile:open:");
 
   remote_buffer_add_bytes (&p, &left, (const gdb_byte *) filename,
@@ -10015,6 +10065,8 @@  remote_hostio_unlink (struct target_ops *self,
   char *p = rs->buf;
   int left = get_remote_packet_size () - 1;
 
+  remote_hostio_set_filesystem ();
+
   remote_buffer_add_string (&p, &left, "vFile:unlink:");
 
   remote_buffer_add_bytes (&p, &left, (const gdb_byte *) filename,
@@ -10040,6 +10092,8 @@  remote_hostio_readlink (struct target_ops *self,
   int read_len;
   char *ret;
 
+  remote_hostio_set_filesystem ();
+
   remote_buffer_add_string (&p, &left, "vFile:readlink:");
 
   remote_buffer_add_bytes (&p, &left, (const gdb_byte *) filename,
@@ -11756,6 +11810,7 @@  Specify the serial device it is connected to\n\
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
   remote_ops.to_supports_disable_randomization
     = remote_supports_disable_randomization;
+  remote_ops.to_fileio_set_fs = remote_hostio_set_fs_deferred;
   remote_ops.to_filesystem_is_local = remote_filesystem_is_local;
   remote_ops.to_fileio_open = remote_hostio_open;
   remote_ops.to_fileio_pwrite = remote_hostio_pwrite;
@@ -12322,6 +12377,9 @@  Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qTStatus],
 			 "qTStatus", "trace-status", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_vFile_setfs],
+			 "vFile:setfs", "hostio-setfs", 0);
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_vFile_open],
 			 "vFile:open", "hostio-open", 0);