Re: [systemd-devel] [PATCH 06/11] There is no ANSI support on common 3215 consoles

2014-08-01 Thread Dr. Werner Fink 
On Fri, Jun 20, 2014 at 07:03:47PM +0200, Lennart Poettering wrote:
 
 Also, we are not going to add code for any specific weird terminal
 settings. We will do three levels: TERM=linux for the full Linux
 console, TERM=vt102 otherwise, and TERM=dumb for the crap that can't do
 TERM=vt102. But we are not going to hardcode support for any other
 hardware into this. This is 2014. If IBM still can't build ANSI support in
 their terminals, then that's the own fault, and they will not get any
 support beyond TERM=dumb and no color. We will not work-around IBM's
 choices in systemd's source tree.
 
 Sorry.

Maybe I'm wrong but this looks like you have no idea how S390x works.
The terminal emulation is done by the linux kernel which uses the
capabilities of the two block oriented terminal interfaces of the
firmware of a s390 (https://en.wikipedia.org/wiki/IBM_3270). One of
the block oriented terminals are able to do coloring (3270) and the
other (3215) is not.  Writing Escape Sequences (including carriage return)
will cause very strange effects with the later block oriented terminal
(3215) and this is the default interface if you to not use

   conmode=3270

to switch to 3270 mode.  With 3270 the kernel then maps Escape Sequences
with the help of linux/drivers/s390/char/tty3270.c onto the real firmware
interface for the chosen block oriented terminal.

I do not think that IBM will ``fix'' their s390 firmware due systemd.
Beside this, what happens if the main system console is a printer
and not a virtual device.  Or what does happen if it is a serial
console which can not handle Escape Sequences or can not handle all
Escape Sequences or use other Escape Sequences as a common VT102?

Werner

-- 
  Having a smoking section in a restaurant is like having
  a peeing section in a swimming pool. -- Edward Burr


pgpBpq14w6OeI.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 06/11] There is no ANSI support on common 3215 consoles

2014-08-01 Thread Lennart Poettering 
On Fri, 01.08.14 16:07, Dr. Werner Fink (wer...@suse.de) wrote:

 On Fri, Jun 20, 2014 at 07:03:47PM +0200, Lennart Poettering wrote:
  
  Also, we are not going to add code for any specific weird terminal
  settings. We will do three levels: TERM=linux for the full Linux
  console, TERM=vt102 otherwise, and TERM=dumb for the crap that can't do
  TERM=vt102. But we are not going to hardcode support for any other
  hardware into this. This is 2014. If IBM still can't build ANSI support in
  their terminals, then that's the own fault, and they will not get any
  support beyond TERM=dumb and no color. We will not work-around IBM's
  choices in systemd's source tree.
  
  Sorry.
 
 Maybe I'm wrong but this looks like you have no idea how S390x works.
 The terminal emulation is done by the linux kernel which uses the
 capabilities of the two block oriented terminal interfaces of the
 firmware of a s390 (https://en.wikipedia.org/wiki/IBM_3270). One of
 the block oriented terminals are able to do coloring (3270) and the
 other (3215) is not.  Writing Escape Sequences (including carriage return)
 will cause very strange effects with the later block oriented terminal
 (3215) and this is the default interface if you to not use
 
conmode=3270
 
 to switch to 3270 mode.  With 3270 the kernel then maps Escape Sequences
 with the help of linux/drivers/s390/char/tty3270.c onto the real firmware
 interface for the chosen block oriented terminal.
 
 I do not think that IBM will ``fix'' their s390 firmware due systemd.
 Beside this, what happens if the main system console is a printer
 and not a virtual device.  Or what does happen if it is a serial
 console which can not handle Escape Sequences or can not handle all
 Escape Sequences or use other Escape Sequences as a common VT102?

That's all good and fine. But we are not going to add explicit support
for this exotic stuff in systemd.

For cases like this we should support TERM=dumb. People won't get color
during boot, but things will still work great otherwise.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 06/11] There is no ANSI support on common 3215 consoles

2014-06-20 Thread Lennart Poettering 
On Fri, 13.06.14 16:41, Werner Fink (wer...@suse.de) wrote:

 Therefore strip off the ANSI escape sequences for 3215 consoles
 but support 3270 consoles if found.

Hmm, this looks messy.

Please add a global systemd PID 1 setting next to the arg_show_status
variable that controls whether color shall be enable for status
output. Then, make this configurable via the kernel cmdline, and in the
configuration file like show_status itself. That would be one patch.

And then, optionally prepare a second patch that adds support for
looking for TERM= on the kernel cmdline. If we see it there we will
initialize arg_show_status from it, and turn off color if it is set to
dumb. Also, return this value as default terminal setting for $TERM
for /dev/console, but no other devices.

That way, s390 installations may simply set TERM=dumb on the kernel
cmdline and the right thing happens, their /dev/console will get color
disabled, and any raw tty clients will get TERM=dumb set for what they
need.

 +#if defined (__s390__) || defined (__s390x__)
 +if (cached_on_tty) {
 +const char *e = getenv(TERM);
 +if (!e)
 +return cached_on_tty;
 +if (streq(e, dumb) || strneq(e, ibm3, 4)) {
 +char *mode = NULL;
 +int r = parse_env_file(/proc/cmdline, 
 WHITESPACE, conmode, mode, NULL);
 +if (r  0 || !mode || !streq(mode, 3270))
 +cached_on_tty = 0;
 +}
 +}
 +#endif

 +
 +#if defined (__s390__) || defined (__s390x__)
 +if (streq(tty, ttyS0)) {
 +char *mode = NULL;
 +int r = parse_env_file(/proc/cmdline, WHITESPACE, 
 conmode, mode, NULL);
 +if (r  0 || !mode || !streq(mode, 3270))
 +return TERM=dumb;
 +if (streq(mode, 3270))
 +return TERM=ibm327x;
 +}
 +if (streq(tty, ttyS1))
 +return TERM=vt220;
 +#endif
 +return TERM=vt102;

Yuck. No! We won't do such messy checks. No arch-specific stuff. No
hardcoding of ttyS0 or ttyS1 or whatever other tty. Support for checking
TERM= in the kernel cmdline should be something generic. 

Sorry, but we will not add any specific per-arch hacks to systemd like
this. We can fix this, but then find nice generic ways to do this, but
not this messy pile of s390 hardcoded assumption. That's not how we do
things. 

Also, we are not going to add code for any specific weird terminal
settings. We will do three levels: TERM=linux for the full Linux
console, TERM=vt102 otherwise, and TERM=dumb for the crap that can't do
TERM=vt102. But we are not going to hardcode support for any other
hardware into this. This is 2014. If IBM still can't build ANSI support in
their terminals, then that's the own fault, and they will not get any
support beyond TERM=dumb and no color. We will not work-around IBM's
choices in systemd's source tree.

Sorry.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 06/11] There is no ANSI support on common 3215 consoles

2014-06-19 Thread Zbigniew Jędrzejewski-Szmek 
On Fri, Jun 13, 2014 at 04:41:05PM +0200, Werner Fink wrote:
 Therefore strip off the ANSI escape sequences for 3215 consoles
 but support 3270 consoles if found.
 
 ---
  src/core/manager.c |   24 
  src/shared/util.c  |   80 
 +---
  src/shared/util.h  |1 +
  3 files changed, 96 insertions(+), 9 deletions(-)
 
 diff --git src/core/manager.c src/core/manager.c
 index 0cb2044..f78ac18 100644
 --- src/core/manager.c
 +++ src/core/manager.c
 @@ -112,7 +112,7 @@ static int manager_watch_jobs_in_progress(Manager *m) {
  
  #define CYLON_BUFFER_EXTRA (2*(sizeof(ANSI_RED_ON)-1) + 
 sizeof(ANSI_HIGHLIGHT_RED_ON)-1 + 2*(sizeof(ANSI_HIGHLIGHT_OFF)-1))

The goal of this patch seems correct. Nevertheless, it is quite
intrusive. Could you rework the patch to:

1. centralize the ansi console detection, like we do with on_tty(),
   so it is implicitly checked on first access and then cached
   inside of the function;
2. replace ANSI_RED_ON/OFF/... with ansi_red_on(), which calls the ansi console
   status function and returns either ANSI_RED_ON/OFF/... or ;
3. instead of parsing the message to extract status in the status_printf
   function, pass the status as a (numerical) argument, and use this
   to decide the output. The mapping should be stored in a table;
4. use parse_proc_cmdline to parse /proc/cmdline.

Thanks,
Zbyszek

  
 -static void draw_cylon(char buffer[], size_t buflen, unsigned width, 
 unsigned pos) {
 +static void draw_cylon(char buffer[], size_t buflen, unsigned width, 
 unsigned pos, bool ansi_console) {
  char *p = buffer;
  
  assert(buflen = CYLON_BUFFER_EXTRA + width + 1);
 @@ -121,12 +121,14 @@ static void draw_cylon(char buffer[], size_t buflen, 
 unsigned width, unsigned po
  if (pos  1) {
  if (pos  2)
  p = mempset(p, ' ', pos-2);
 -p = stpcpy(p, ANSI_RED_ON);
 +if (ansi_console)
 +p = stpcpy(p, ANSI_RED_ON);
  *p++ = '*';
  }
  
  if (pos  0  pos = width) {
 -p = stpcpy(p, ANSI_HIGHLIGHT_RED_ON);
 +if (ansi_console)
 +p = stpcpy(p, ANSI_HIGHLIGHT_RED_ON);
  *p++ = '*';
  }
  
 @@ -137,7 +139,8 @@ static void draw_cylon(char buffer[], size_t buflen, 
 unsigned width, unsigned po
  *p++ = '*';
  if (pos  width-1)
  p = mempset(p, ' ', width-1-pos);
 -strcpy(p, ANSI_HIGHLIGHT_OFF);
 +if (ansi_console)
 +strcpy(p, ANSI_HIGHLIGHT_OFF);
  }
  }
  
 @@ -154,6 +157,7 @@ void manager_flip_auto_status(Manager *m, bool enable) {
  }
  
  static void manager_print_jobs_in_progress(Manager *m) {
 +static int is_ansi_console = -1;
  _cleanup_free_ char *job_of_n = NULL;
  Iterator i;
  Job *j;
 @@ -178,10 +182,20 @@ static void manager_print_jobs_in_progress(Manager *m) {
  assert(counter == print_nr + 1);
  assert(j);
  
 +if (_unlikely_(is_ansi_console  0)) {
 +int fd = open_terminal(/dev/console, 
 O_RDONLY|O_NOCTTY|O_CLOEXEC);
 +if (fd  0)
 +is_ansi_console = 0;
 +else {
 +is_ansi_console = (int)ansi_console(fd);
 +close(fd);
 +}
 +}
 +
  cylon_pos = m-jobs_in_progress_iteration % 14;
  if (cylon_pos = 8)
  cylon_pos = 14 - cylon_pos;
 -draw_cylon(cylon, sizeof(cylon), 6, cylon_pos);
 +draw_cylon(cylon, sizeof(cylon), 6, cylon_pos, 
 (bool)is_ansi_console);
  
  m-jobs_in_progress_iteration++;
  
 diff --git src/shared/util.c src/shared/util.c
 index a7aec5c..03b01de 100644
 --- src/shared/util.c
 +++ src/shared/util.c
 @@ -2937,6 +2937,7 @@ int status_vprintf(const char *status, bool ellipse, 
 bool ephemeral, const char
  struct iovec iovec[6] = {};
  int n = 0;
  static bool prev_ephemeral;
 +static int is_ansi_console = -1;
  
  assert(format);
  
 @@ -2950,6 +2951,41 @@ int status_vprintf(const char *status, bool ellipse, 
 bool ephemeral, const char
  if (fd  0)
  return fd;
  
 +if (_unlikely_(is_ansi_console  0))
 +is_ansi_console = (int)ansi_console(fd);
 +
 +if (status  !is_ansi_console) {
 +const char *esc, *ptr;
 +esc = strchr(status, 0x1B);
 +if (esc  (ptr = strpbrk(esc, SOFDTI*))) {
 +switch(*ptr) {
 +case 'S':
 +status =  SKIP ;
 +break;
 +case 'O':
 +status =   OK  ;
 +break;
 +