From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 872 invoked by alias); 9 Jun 2015 14:11:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 861 invoked by uid 89); 9 Jun 2015 14:11:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 09 Jun 2015 14:11:21 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 8192319F24E; Tue, 9 Jun 2015 14:11:20 +0000 (UTC) Received: from blade.nx (ovpn-116-112.ams2.redhat.com [10.36.116.112]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t59EBJhP007381; Tue, 9 Jun 2015 10:11:19 -0400 Received: by blade.nx (Postfix, from userid 1000) id 781BC2626FB; Tue, 9 Jun 2015 15:11:18 +0100 (BST) Date: Tue, 09 Jun 2015 14:11:00 -0000 From: Gary Benson To: Pedro Alves Cc: gdb-patches@sourceware.org, Eli Zaretskii , Doug Evans , Iago =?iso-8859-1?Q?L=F3pez?= Galeiras Subject: Re: [PATCH 8/9 v2] Implement vFile:setfs in gdbserver Message-ID: <20150609141118.GA29533@blade.nx> References: <1429186791-6867-1-git-send-email-gbenson@redhat.com> <1430395542-16017-9-git-send-email-gbenson@redhat.com> <555DF2EE.2030707@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <555DF2EE.2030707@redhat.com> X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00130.txt.bz2 Pedro Alves wrote: > On 04/30/2015 01:05 PM, Gary Benson wrote: > > This commit implements the "vFile:setfs" packet in gdbserver. > > > > gdb/gdbserver/ChangeLog: > > > > * target.h (struct target_ops) : New field. > > : Likewise. > > : Likewise. > > * linux-low.c (nat/linux-namespaces.h): New include. > > (linux_target_ops): Initialize the_target->multifs_open, > > the_target->multifs_unlink and the_target->multifs_readlink. > > * hostio.c (hostio_fs_pid): New static variable. > > (handle_setfs): New function. > > (handle_open): Use the_target->multifs_open as appropriate. > > (handle_unlink): Use the_target->multifs_unlink as appropriate. > > (handle_readlink): Use the_target->multifs_readlink as > > appropriate. > > (handle_vFile): Handle vFile:setfs packets. > > --- > > gdb/gdbserver/ChangeLog | 16 +++++++++++ > > gdb/gdbserver/hostio.c | 62 ++++++++++++++++++++++++++++++++++++++++++-- > > gdb/gdbserver/linux-low.c | 4 +++ > > gdb/gdbserver/target.h | 21 +++++++++++++++ > > 4 files changed, 100 insertions(+), 3 deletions(-) > > > > diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c > > index 9e858d9..10cff5a 100644 > > --- a/gdb/gdbserver/hostio.c > > +++ b/gdb/gdbserver/hostio.c > > @@ -243,6 +243,47 @@ hostio_reply_with_data (char *own_buf, char *buffer, int len, > > return input_index; > > } > > > > +/* Process ID of inferior whose filesystem hostio functions > > + that take FILENAME arguments will use. Zero means to use > > + our own filesystem. */ > > + > > +static int hostio_fs_pid = 0; > > > This should be cleared if GDB reconnects. Ok, I added a hostio_handle_new_gdb_connection function to do this that's called just after target_handle_new_gdb_connection in server.c. > > +/* Handle a "vFile:setfs:" packet. */ > > + > > +static void > > +handle_setfs (char *own_buf) > > +{ > > + char *p; > > + int pid; > > + > > + /* If the target doesn't have any of the in-filesystem-of methods > > + then there's no point in GDB sending "vFile:setfs:" packets. We > > + reply with an empty packet (i.e. we pretend we don't understand > > + "vFile:setfs:") and that should stop GDB sending any more. */ > > + if (the_target->multifs_open == NULL > > + && the_target->multifs_unlink == NULL > > + && the_target->multifs_readlink == NULL) > > + { > > + own_buf[0] = '\0'; > > + return; > > + } > > For the setns/ENOSYS case, how about somehow probing whether setns > actually works here (with a new method, or probing one of the > existing ones with getpid()) and return empty packet, instead of > ending up with open failing with ENOSYS later on? Probing and then telling GDB we don't understand "vFile:setfs:" isn't that good of an idea. If we do that with an inferior in another mount namespace then the end result is that gdbserver will access the wrong filesystem. You might get "file not found", or you might get an open filehandle on another file (i.e. the file with the same name but in gdbserver's filesystem). If the inferior's filesystem is different from gdbserver and gdbserver cannot access that filesystem then the only correct response is to fail. I've updated linux_mntns_access_fs to translate ENOSYS from setns into ENOTSUP, so neither native nor gdbserver will ever get ENOSYS from this code. From [1] ENOTSUP seems to be the perfect code for linux_mntns_{open_cloexec,unlink,readlink} to be returning: A function returns this error when certain parameter values are valid, but the functionality they request is not available. This can mean that the function does not implement a particular command or option value or flag bit at all. ... If the entire function is not available at all in the implementation, it returns ENOSYS instead. Thanks for hassling me about this, it took me a while to understand what you were saying. Cheers, Gary -- [1] http://lkml.iu.edu/hypermail/linux/kernel/0208.0/0121.html