Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Doug Evans]

Gary Benson writes:
  > This commit allows GDB to access executables and shared libraries
  > on native Linux targets where GDB and the inferior have different
  > mount namespaces.
  >
  > gdb/ChangeLog:
  >
  > 	* linux-nat.c (nat/linux-namespaces.h): New include.
  > 	(fileio.h): Likewise.
  > 	(linux_nat_filesystem_is_local): New function.
  > 	(linux_nat_fileio_pid_of): Likewise.
  > 	(linux_nat_fileio_open): Likewise.
  > 	(linux_nat_fileio_readlink): Likewise.
  > 	(linux_nat_fileio_unlink): Likewise.
  > 	(linux_nat_add_target): Initialize to_filesystem_is_local,
  > 	to_fileio_open, to_fileio_readlink and to_fileio_unlink.
  > 	(_initialize_linux_nat): New "set/show debug lin-ns" commands.
  > 	* NEWS: Mention new "set/show debug lin-ns" commands.

Hi.

Nit: "set debug" options need a doc entry.
[I didn't see it in another patch.
I also didn't see this option used in this patch.]

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Gary Benson]

Doug Evans wrote:
> Gary Benson writes:
> > This commit allows GDB to access executables and shared libraries
> > on native Linux targets where GDB and the inferior have different
> > mount namespaces.
> >
> > gdb/ChangeLog:
> >
> > 	* linux-nat.c (nat/linux-namespaces.h): New include.
> > 	(fileio.h): Likewise.
> > 	(linux_nat_filesystem_is_local): New function.
> > 	(linux_nat_fileio_pid_of): Likewise.
> > 	(linux_nat_fileio_open): Likewise.
> > 	(linux_nat_fileio_readlink): Likewise.
> > 	(linux_nat_fileio_unlink): Likewise.
> > 	(linux_nat_add_target): Initialize to_filesystem_is_local,
> > 	to_fileio_open, to_fileio_readlink and to_fileio_unlink.
> > 	(_initialize_linux_nat): New "set/show debug lin-ns" commands.
> > 	* NEWS: Mention new "set/show debug lin-ns" commands.
> 
> Nit: "set debug" options need a doc entry.
> [I didn't see it in another patch.
> I also didn't see this option used in this patch.]

It's used in nat/linux-namespaces.c.
I'll add some documentation for it.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Gary Benson]

Doug Evans wrote:
> Gary Benson writes:
> > This commit allows GDB to access executables and shared libraries
> > on native Linux targets where GDB and the inferior have different
> > mount namespaces.
> >
> > gdb/ChangeLog:
> >
> > 	* linux-nat.c (nat/linux-namespaces.h): New include.
> > 	(fileio.h): Likewise.
> > 	(linux_nat_filesystem_is_local): New function.
> > 	(linux_nat_fileio_pid_of): Likewise.
> > 	(linux_nat_fileio_open): Likewise.
> > 	(linux_nat_fileio_readlink): Likewise.
> > 	(linux_nat_fileio_unlink): Likewise.
> > 	(linux_nat_add_target): Initialize to_filesystem_is_local,
> > 	to_fileio_open, to_fileio_readlink and to_fileio_unlink.
> > 	(_initialize_linux_nat): New "set/show debug lin-ns" commands.
> > 	* NEWS: Mention new "set/show debug lin-ns" commands.
> 
> Nit: "set debug" options need a doc entry.
> [I didn't see it in another patch.

Eli, is this ok?  (Not as a standalone patch, I'll merge it...)

Thanks,
Gary

-- 
gdb/doc/ChangeLog:

	* gdb.texinfo (Debugging Output): Document the "set/show debug
	linux-namespaces" command.
---
 gdb/doc/ChangeLog   |    5 +++++
 gdb/doc/gdb.texinfo |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9ea846a..e1f2e58 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23484,6 +23484,11 @@ Displays the current state of @value{GDBN} JIT debugging.
 Turns on or off debugging messages from the Linux LWP debug support.
 @item show debug lin-lwp
 Show the current state of Linux LWP debugging messages.
+@item set debug linux-namespaces
+@cindex @sc{gnu}/Linux namespaces debug messages
+Turns on or off debugging messages from the Linux namespaces debug support.
+@item show debug linux-namespaces
+Show the current state of Linux namespaces debugging messages.
 @item set debug mach-o
 @cindex Mach-O symbols processing
 Control display of debugging messages related to Mach-O symbols
-- 
1.7.1

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Eli Zaretskii]

> From: Gary Benson <gbenson@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Pedro Alves <palves@redhat.com>,
>         Doug Evans <dje@google.com>
> Date: Tue,  9 Jun 2015 14:20:36 +0100
> 
> Eli, is this ok?

Yes, thanks.

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Gary Benson]

Pedro Alves wrote:
> On 04/30/2015 01:05 PM, Gary Benson wrote:
> > This commit allows GDB to access executables and shared libraries
> > on native Linux targets where GDB and the inferior have different
> > mount namespaces.
> 
> > +/* Implementation of to_fileio_open.  */
> > +
> > +static int
> > +linux_nat_fileio_open (struct target_ops *self,
> > +		       struct inferior *inf, const char *filename,
> > +		       int flags, int mode, int *target_errno)
> > +{
> > +  int nat_flags;
> > +  int fd;
> > +
> > +  if (fileio_to_host_openflags (flags, &nat_flags) == -1)
> > +    {
> > +      *target_errno = FILEIO_EINVAL;
> > +      return -1;
> > +    }
> > +
> > +  /* We do not need to convert MODE, since the fileio protocol
> > +     uses the standard values.  */
> 
> Hmm, I'm not seeing how the fileio constants have to necessarily
> match the host's.  So I don't understand this comment.  What
> are these standard values?

Hmmm, I just copied this from inf_child_fileio_open.  Maybe I should
fix that first, with some convertor function that the compiler can
essentially optimize away *if* the values really do match.

> Otherwise this looks good to me.  I'll leave the "set debug" flag's
> name up to you.

Cool, thanks.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Pedro Alves]

On 04/30/2015 01:05 PM, Gary Benson wrote:
> This commit allows GDB to access executables and shared libraries
> on native Linux targets where GDB and the inferior have different
> mount namespaces.


> +/* Implementation of to_fileio_open.  */
> +
> +static int
> +linux_nat_fileio_open (struct target_ops *self,
> +		       struct inferior *inf, const char *filename,
> +		       int flags, int mode, int *target_errno)
> +{
> +  int nat_flags;
> +  int fd;
> +
> +  if (fileio_to_host_openflags (flags, &nat_flags) == -1)
> +    {
> +      *target_errno = FILEIO_EINVAL;
> +      return -1;
> +    }
> +
> +  /* We do not need to convert MODE, since the fileio protocol
> +     uses the standard values.  */

Hmm, I'm not seeing how the fileio constants have to necessarily
match the host's.  So I don't understand this comment.  What
are these standard values?

> +  fd = linux_mntns_open_cloexec (linux_nat_fileio_pid_of (inf),
> +				 filename, nat_flags, mode);
> +  if (fd == -1)
> +    *target_errno = host_to_fileio_error (errno);
> +
> +  return fd;
> +}
> +

Otherwise this looks good to me.  I'll leave the "set debug" flag's
name up to you.

Thanks,
Pedro Alves

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Gary Benson]

Eli Zaretskii wrote:
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: Eli Zaretskii <eliz@gnu.org>, Pedro Alves <palves@redhat.com>,
> >         Doug Evans <dje@google.com>,
> >         Iago López Galeiras <iago@endocode.com>
> > Date: Thu, 30 Apr 2015 13:05:39 +0100
> > 
> > +set debug lin-ns
> > +show debug lin-ns
> > +  Control display of debugging info regarding Linux namespaces.
> 
> Would it be better to have a more mnemonic name, like
> "linux-namespace", for example?  IMO, "lin-ns" goes too far in
> making its name shorter.

I can change that.  (I think I modelled it after the lin-lwp option).

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Eli Zaretskii]

> From: Gary Benson <gbenson@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Pedro Alves <palves@redhat.com>,
>         Doug Evans <dje@google.com>,
>         Iago López Galeiras <iago@endocode.com>
> Date: Thu, 30 Apr 2015 13:05:39 +0100
> 
> +set debug lin-ns
> +show debug lin-ns
> +  Control display of debugging info regarding Linux namespaces.

Would it be better to have a more mnemonic name, like
"linux-namespace", for example?  IMO, "lin-ns" goes too far in making
its name shorter.

Otherwise, the documentation parts are OK.

Thanks.

[PATCH 6/9 v2] Implement mount namespace support for native Linux targets

[Gary Benson]

This commit allows GDB to access executables and shared libraries
on native Linux targets where GDB and the inferior have different
mount namespaces.

gdb/ChangeLog:

	* linux-nat.c (nat/linux-namespaces.h): New include.
	(fileio.h): Likewise.
	(linux_nat_filesystem_is_local): New function.
	(linux_nat_fileio_pid_of): Likewise.
	(linux_nat_fileio_open): Likewise.
	(linux_nat_fileio_readlink): Likewise.
	(linux_nat_fileio_unlink): Likewise.
	(linux_nat_add_target): Initialize to_filesystem_is_local,
	to_fileio_open, to_fileio_readlink and to_fileio_unlink.
	(_initialize_linux_nat): New "set/show debug lin-ns" commands.
	* NEWS: Mention new "set/show debug lin-ns" commands.
---
 gdb/ChangeLog   |   14 +++++++
 gdb/NEWS        |    4 ++
 gdb/linux-nat.c |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+), 0 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index d463b52..1c0e5ea 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -89,6 +89,10 @@ set|show record btrace bts buffer-size
   The obtained size may differ from the requested size.  Use "info
   record" to see the obtained buffer size.
 
+set debug lin-ns
+show debug lin-ns
+  Control display of debugging info regarding Linux namespaces.
+
 * The command 'thread apply all' can now support new option '-ascending'
   to call its specified command for all threads in ascending order.
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b04aa68..85149a2 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -66,6 +66,8 @@
 #include "target-descriptions.h"
 #include "filestuff.h"
 #include "objfiles.h"
+#include "nat/linux-namespaces.h"
+#include "fileio.h"
 
 #ifndef SPUFS_MAGIC
 #define SPUFS_MAGIC 0x23c9b64e
@@ -4842,6 +4844,104 @@ linux_nat_core_of_thread (struct target_ops *ops, ptid_t ptid)
   return -1;
 }
 
+/* Implementation of to_filesystem_is_local.  */
+
+static int
+linux_nat_filesystem_is_local (struct target_ops *ops)
+{
+  struct inferior *inf = current_inferior ();
+
+  if (inf->fake_pid_p || inf->pid == 0)
+    return 1;
+
+  return linux_ns_same (inf->pid, LINUX_NS_MNT);
+}
+
+/* Convert the INF argument passed to a to_fileio_* method
+   to a process ID suitable for passing to its corresponding
+   linux_mntns_* function.  If INF is non-NULL then the
+   caller is requesting the filesystem seen by INF.  If INF
+   is NULL then the caller is requesting the filesystem seen
+   by the GDB.  We fall back to GDB's filesystem in the case
+   that INF is non-NULL but its PID is unknown.  */
+
+static pid_t
+linux_nat_fileio_pid_of (struct inferior *inf)
+{
+  if (inf == NULL || inf->fake_pid_p || inf->pid == 0)
+    return getpid ();
+  else
+    return inf->pid;
+}
+
+/* Implementation of to_fileio_open.  */
+
+static int
+linux_nat_fileio_open (struct target_ops *self,
+		       struct inferior *inf, const char *filename,
+		       int flags, int mode, int *target_errno)
+{
+  int nat_flags;
+  int fd;
+
+  if (fileio_to_host_openflags (flags, &nat_flags) == -1)
+    {
+      *target_errno = FILEIO_EINVAL;
+      return -1;
+    }
+
+  /* We do not need to convert MODE, since the fileio protocol
+     uses the standard values.  */
+  fd = linux_mntns_open_cloexec (linux_nat_fileio_pid_of (inf),
+				 filename, nat_flags, mode);
+  if (fd == -1)
+    *target_errno = host_to_fileio_error (errno);
+
+  return fd;
+}
+
+/* Implementation of to_fileio_readlink.  */
+
+static char *
+linux_nat_fileio_readlink (struct target_ops *self,
+			   struct inferior *inf, const char *filename,
+			   int *target_errno)
+{
+  char buf[PATH_MAX];
+  int len;
+  char *ret;
+
+  len = linux_mntns_readlink (linux_nat_fileio_pid_of (inf),
+			      filename, buf, sizeof (buf));
+  if (len < 0)
+    {
+      *target_errno = host_to_fileio_error (errno);
+      return NULL;
+    }
+
+  ret = xmalloc (len + 1);
+  memcpy (ret, buf, len);
+  ret[len] = '\0';
+  return ret;
+}
+
+/* Implementation of to_fileio_unlink.  */
+
+static int
+linux_nat_fileio_unlink (struct target_ops *self,
+			 struct inferior *inf, const char *filename,
+			 int *target_errno)
+{
+  int ret;
+
+  ret = linux_mntns_unlink (linux_nat_fileio_pid_of (inf),
+			    filename);
+  if (ret == -1)
+    *target_errno = host_to_fileio_error (errno);
+
+  return ret;
+}
+
 void
 linux_nat_add_target (struct target_ops *t)
 {
@@ -4895,6 +4995,11 @@ linux_nat_add_target (struct target_ops *t)
 
   t->to_core_of_thread = linux_nat_core_of_thread;
 
+  t->to_filesystem_is_local = linux_nat_filesystem_is_local;
+  t->to_fileio_open = linux_nat_fileio_open;
+  t->to_fileio_readlink = linux_nat_fileio_readlink;
+  t->to_fileio_unlink = linux_nat_fileio_unlink;
+
   /* We don't change the stratum; this target will sit at
      process_stratum and thread_db will set at thread_stratum.  This
      is a little strange, since this is a multi-threaded-capable
@@ -5012,6 +5117,15 @@ Enables printf debugging output."),
 			     show_debug_linux_nat,
 			     &setdebuglist, &showdebuglist);
 
+  add_setshow_boolean_cmd ("lin-ns", class_maintenance,
+			   &debug_linux_namespaces, _("\
+Set debugging of GNU/Linux namespaces module."), _("\
+Show debugging of GNU/Linux namespaces module."), _("\
+Enables printf debugging output."),
+			   NULL,
+			   NULL,
+			   &setdebuglist, &showdebuglist);
+
   /* Save this mask as the default.  */
   sigprocmask (SIG_SETMASK, NULL, &normal_mask);
 
-- 
1.7.1