From mboxrd@z Thu Jan 1 00:00:00 1970 From: Martin Lucina Subject: Re: [PATCH] xenconsole: Allow non-interactive use Date: Thu, 23 Jul 2015 17:09:15 +0200 Message-ID: <20150723150915.GA23015@nodbug.lucina.net> References: <1437584888-9074-1-git-send-email-martin@lucina.net> <1437641330.19412.37.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZII7X-0003qq-AJ for xen-devel@lists.xenproject.org; Thu, 23 Jul 2015 15:09:15 +0000 Content-Disposition: inline In-Reply-To: <1437641330.19412.37.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, Wei Liu , Ian Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Thursday, 23.07.2015 at=A009:48, Ian Campbell wrote: > On Wed, 2015-07-22 at 19:08 +0200, Martin Lucina wrote: > > If xenconsole is run with stdin closed or redirected to /dev/null, > > console_loop() will return immediately due to failure to read from > > STDIN_FILENO. This patch tests if stdin and stdout are both connected = > > to > > a TTY and, if not, xenconsole will not attempt to read from stdin or > > modify stdout terminal attributes. > > = > > Existing behaviour when xenconsole is run from a terminal does not > > change. > > = > > This allows for non-interactive use, eg. running "xl create -c" under > > systemd or piping the output of "xl console" to another command. > > = > > Signed-off-by: Martin Lucina > > Cc: Ian Jackson > > Cc: Stefano Stabellini > > Cc: Ian Campbell > > Cc: Wei Liu > = > Acked-by: Ian Campbell > = > WRT the 4.6 freeze I'm torn between calling this a feature or a bugfix. > = > A pair of nits, which probably aren't worth acting on: > = > > @@ -176,8 +177,13 @@ static int console_loop(int fd, struct xs_handle > > *xs, char *pty_path) > > fd_set fds; > > = > > FD_ZERO(&fds); > > - FD_SET(STDIN_FILENO, &fds); > > - max_fd =3D STDIN_FILENO; > > + if (interactive) { > > + FD_SET(STDIN_FILENO, &fds); > > + max_fd =3D STDIN_FILENO; > > + } > > + else { > > + max_fd =3D -1; > > + } > = > Looking at the rest of the file and tools/console subtree it seems the > prevailing coding style is: > = > } else > max_fd =3D -1; > = > (i.e. } brace on the same line as the else and no {} for single > statements after an else). > = > But maybe it would be better to set max_fd =3D -1 on declaration and do > the max dance here as with the following cases? Declaring max_fd =3D -1 is indeed clearer, I can do a v2 with that change if you like. One other bug that my change makes potentially easier to trigger is that you can run "xl console DOMID" multiple times with the same DOMID and the result is broken; each instance gets part of the data written to the console. Any ideas on how to address this in a simple fashion? Martin