bug#30505: marionette/virtio-console issues lead to test failures

[Danny Milosavljevic]

Hi Ludo,

On Mon, 19 Feb 2018 21:35:05 +0100
l...@gnu.org (Ludovic Courtès) wrote:

> > It was a bad idea to do the "\x1b%G" in the first place.  
> 
> Because it’s redundant with IUTF8?

I meant because the Linux kernel does it already and it's better not to
have random multi-byte racy writes onto the tty while the mingetty
is starting up (and possibly buffering and pending half of another sequence).

As far as I understand it's not redundant to do both.

According to src/Linux/linux-4.12-rc2/drivers/tty/vt/vt.c, the 'G' controls
the conversion utf-8->unicode that happens before the virtual terminal
displays a corresponding character on the screen.

On the other hand, the termios iutf8 is meant for the program running in the 
session.

Say you have bash on vt1, then bash can check termios for the settings and find
out whether vt1 is UTF-8-capable (also has other settings like whether the
terminal already supports line editing etc - old-school terminals were quite
cool; a friend of mine salvaged a real one ^^).

Note that drivers/tty/vt/vt.c only copies ONE way, from the 'G' flag to the
termios (and that seldomly).
Makes sense since the programs shouldn't have a say in what the terminal can do 
:)

So I'd say guix services fiddling with termios is ... weird and the 'G' slightly
less weird.

> This ‘unicode-start’ procedure is essentially a port of the
> ‘unicode_start’ script from ‘kbd’.  I suppose the justification is to
> make sure we’re using UTF-8 input regardless of what the kernel defaults
> or command-line options are.

Yeah, but it's asking for trouble.

I just checked Linux 2.6.32.1, it defaults to utf8 (IUTF8 in termios, and 'G').

I'd suggest to remove both.

bug#30505: marionette/virtio-console issues lead to test failures

[Danny Milosavljevic]

Hi Ludo,

>I hadn’t noticed this is now part of ‘%base-services’.  It would be nice
if it were enabled on ARM only.  Thoughts?

Why?  It's not ARM-specific and there are people using headless x86 servers
posting on the mailing list :)

It's only enabled when you specify a serial port as console on the Linux
command line - that's not going to happen accidentially.

And once Linux uses the console for its messages it's nice to also have a
login process running in the end - otherwise it's kinda annoying having
only a read-only line when you sit right in front of the machine.

On Mon, 19 Feb 2018 16:54:44 +0100
l...@gnu.org (Ludovic Courtès) wrote:

> Commenting out (display "\x1b%G" (fdes->outport fd)) in (gnu services
> base) appear to solve the problem.  It seems that it used to affect just
> the terminal behind FD and now somehow broadcasts to all existing
> terminals?

It was a bad idea to do the "\x1b%G" in the first place.

There's a Linux kernel command-line parameter "vt.default_utf8" which
is set to true anyway.  In that case the iflag IUTF8 is set automatically
by Linux drivers/tty/vt/vt.c and the driver also does the same as "\x1b%G"
does in that case.

So what do these things in (gnu services base) accomplish?  Sounds like
they change nothing.

Maybe that was only done in later Linux kernels? I checked 3.4.103, it did that
already.

bug#30505: marionette/virtio-console issues lead to test failures

[Ludovic Courtès]

Heya,

Danny Milosavljevic  skribis:

> On Sun, 18 Feb 2018 01:01:31 +0100
> l...@gnu.org (Ludovic Courtès) wrote:
>
>> The “\x1b;%G” sequences correspond to the “select UTF-8” console code
>> (see console_codes(4)).  We’re receiving this as if we were a console,
>> but in fact all we want is to exchange raw bytes between the host and
>> the guest; we don’t want to be a full-fledged console.
>
> A lot of the tests pass console=... and thus in fact have a real console.
>
> It might be that our new automatic console getty interferes.
> I doubt it - I checked agetty sources and it doesn't touch iutf8.
> Might still be worth a try to remove agetty from %base-services.

This isn’t the culprit.

I hadn’t noticed this is now part of ‘%base-services’.  It would be nice
if it were enabled on ARM only.  Thoughts?

>> this happens on ‘core-updates’ and not ‘master’.
>
> That's a good question.
>
> Anyway, I think these codes are emitted by unicode_start - so
> as a first step, sabotage the kbd package so that it can't
> use unicode_start.  Does it work then?

Commenting out (display "\x1b%G" (fdes->outport fd)) in (gnu services
base) appear to solve the problem.  It seems that it used to affect just
the terminal behind FD and now somehow broadcasts to all existing
terminals?

Anyway, I’m unsure this ‘display’ call was needed at all.  It seems
redundant with the ‘tcsetattr’ call below.  So I think we’ll just remove
it.

Thoughts?

Ludo’.

bug#30505: marionette/virtio-console issues lead to test failures

[Danny Milosavljevic]

Hi Ludo,

On Sun, 18 Feb 2018 01:01:31 +0100
l...@gnu.org (Ludovic Courtès) wrote:

> The “\x1b;%G” sequences correspond to the “select UTF-8” console code
> (see console_codes(4)).  We’re receiving this as if we were a console,
> but in fact all we want is to exchange raw bytes between the host and
> the guest; we don’t want to be a full-fledged console.

A lot of the tests pass console=... and thus in fact have a real console.

It might be that our new automatic console getty interferes.
I doubt it - I checked agetty sources and it doesn't touch iutf8.
Might still be worth a try to remove agetty from %base-services.

check-system TESTS=basic works fine in master indeed ...

> this happens on ‘core-updates’ and not ‘master’.

That's a good question.

Anyway, I think these codes are emitted by unicode_start - so
as a first step, sabotage the kbd package so that it can't
use unicode_start.  Does it work then?

bug#30505: marionette/virtio-console issues lead to test failures

[Ludovic Courtès]

Hello,

On core-updates (or is it master now?), “make check-system TESTS=basic”
fails with:

--8<---cut here---start->8---
Test begin:
  test-name: "/run/current-system is a GC root"
  source-file: "/gnu/store/irm2375f5p7nzyqqllwjfaby3ywhi5wq-basic-builder"
  source-line: 2
  source-form: (test-eq "/run/current-system is a GC root" (quote success!) 
(marionette-eval (quote (begin (add-to-load-path 
"/gnu/store/pn333fnrdazadmzvkbyzby8cfr176yrh-guix-0.14.0-8.bc880f9/share/guile/site/2.2")
 (use-modules (srfi srfi-34) (guix store)) (let ((system (readlink 
"/run/current-system"))) (guard (c ((nix-protocol-error? c) (and (file-exists? 
system) (quote success! (with-store store (delete-paths store (list 
system)) #f) marionette))
Test end:
  result-kind: fail
  actual-value: #{\x1b;%G\x1b;%G\x1b;%G\x1b;%G\x1b;%G\x1b;%Gsuccess!}#
  expected-value: success!
--8<---cut here---end--->8---

We see similar issues with other system tests.

The “\x1b;%G” sequences correspond to the “select UTF-8” console code
(see console_codes(4)).  We’re receiving this as if we were a console,
but in fact all we want is to exchange raw bytes between the host and
the guest; we don’t want to be a full-fledged console.

I thought that using virtserialport instead of virtio-console might help
(see patch below), but the problem persists.  I’m not sure if it’s the
kernel that decides to send these codes or what.  Also unclear as to why
this happens on ‘core-updates’ and not ‘master’.

What am I missing?

Ludo’.


diff --git a/gnu/build/marionette.scm b/gnu/build/marionette.scm
index 7554a710a..173a67cef 100644
--- a/gnu/build/marionette.scm
+++ b/gnu/build/marionette.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2016, 2017 Ludovic Courtès 
+;;; Copyright © 2016, 2017, 2018 Ludovic Courtès 
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -97,8 +97,11 @@ QEMU monitor and to the guest's backdoor REPL."
   "-monitor" (string-append "unix:" socket-directory "/monitor")
   "-chardev" (string-append "socket,id=repl,path=" socket-directory
 "/repl")
+
+  ;; See
+  ;; .
   "-device" "virtio-serial"
-  "-device" "virtconsole,chardev=repl"))
+  "-device" "virtserialport,chardev=repl,name=org.gnu.guix.port.0"))
 
   (define (accept* port)
 (match (select (list port) '() (list port) timeout)
diff --git a/gnu/tests.scm b/gnu/tests.scm
index 3e4c3d4e3..31249f0be 100644
--- a/gnu/tests.scm
+++ b/gnu/tests.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2016, 2017 Ludovic Courtès 
+;;; Copyright © 2016, 2017, 2018 Ludovic Courtès 
 ;;; Copyright © 2017 Mathieu Othacehe 
 ;;; Copyright © 2017 Tobias Geerinckx-Rice 
 ;;;
@@ -69,7 +69,7 @@
   marionette-configuration make-marionette-configuration
   marionette-configuration?
   (device   marionette-configuration-device ;string
-(default "/dev/hvc0"))
+(default "/dev/virtio-ports/org.gnu.guix.port.0"))
   (imported-modules marionette-configuration-imported-modules
 (default '()))
   (requirements marionette-configuration-requirements ;list of symbols
@@ -87,17 +87,10 @@
 
 (modules '((ice-9 match)
(srfi srfi-9 gnu)
-   (guix build syscalls)
(rnrs bytevectors)))
 (start
- (with-imported-modules `((guix build syscalls)
-  ,@imported-modules)
+ (with-imported-modules imported-modules
#~(lambda ()
-   (define (clear-echo termios)
- (set-field termios (termios-local-flags)
-(logand (lognot (local-flags ECHO))
-(termios-local-flags termios
-
(define (self-quoting? x)
  (letrec-syntax ((one-of (syntax-rules ()
((_) #f)
@@ -112,20 +105,7 @@
   (dynamic-wind
 (const #t)
 (lambda ()
-  (let* ((repl(open-file #$device "r+0"))
- (termios (tcgetattr (fileno repl)))
- (console (open-file "/dev/console" "r+0")))
-;; Don't echo input back.
-(tcsetattr (fileno repl) (tcsetattr-action TCSANOW)
-   (clear-echo termios))
-
-;; Redirect output to the console.
-(close-fdes 1)
-