console: T2529: migrate from ttyUSB device to new device in /dev/serial/by-bus

During testing it was discovered that there is a well known problem (we had for
ethernet interfaces) also in the serial port world. They will be enumerated and
mapped to /dev/ttyUSBxxx differently from boot to boot. This is especially
painful on my development APU4 board which also has a Sierra Wireless MC7710
LTE module installed.

The serial port will toggle between ttyUSB2 and ttyUSB5 depending on the
amount of serial port extenders attached (FT4232H).

The shipped udev rule (/usr/lib/udev/rules.d/60-serial.rules) partly solves
this by enumerating the devices into /dev/serial/by-id folder with their name
and serial number - it's a very good idea but I've found that not all of the
FT4232H dongles have a serial number programmed - this leads to the situation
that when you plug in two cables with both having serial number 0 - only one
device symlink will appear - the previous one is always overwritten by the
latter one.

Derive /usr/lib/udev/rules.d/60-serial.rules and create a /dev/serial/by-bus
directory and group devices by attached USB root port.

4 files changed, 93 insertions, 37 deletions

diff --git a/data/templates/getty/serial-getty.service.tmpl b/data/templates/getty/serial-getty.service.tmpl
new file mode 100644
index 000000000..0183eae7d
--- /dev/null
+++ b/data/templates/getty/serial-getty.service.tmpl
@@ -0,0 +1,37 @@
+[Unit]
+Description=Serial Getty on %I
+Documentation=man:agetty(8) man:systemd-getty-generator(8)
+Documentation=http://0pointer.de/blog/projects/serial-console.html
+BindsTo=dev-%i.device
+After=dev-%i.device systemd-user-sessions.service plymouth-quit-wait.service getty-pre.target
+After=vyos-router.service
+
+# If additional gettys are spawned during boot then we should make
+# sure that this is synchronized before getty.target, even though
+# getty.target didn't actually pull it in.
+Before=getty.target
+IgnoreOnIsolate=yes
+
+# IgnoreOnIsolate causes issues with sulogin, if someone isolates
+# rescue.target or starts rescue.service from multi-user.target or
+# graphical.target.
+Conflicts=rescue.service
+Before=rescue.service
+
+[Service]
+# The '-o' option value tells agetty to replace 'login' arguments with an
+# option to preserve environment (-p), followed by '--' for safety, and then
+# the entered username.
+ExecStart=-/sbin/agetty -o '-p -- \\u' --keep-baud {{ speed }} %I $TERM
+Type=idle
+Restart=always
+UtmpIdentifier=%I
+TTYPath=/dev/%I
+TTYReset=yes
+TTYVHangup=yes
+KillMode=process
+IgnoreSIGPIPE=no
+SendSIGHUP=yes
+
+[Install]
+WantedBy=getty.target
diff --git a/interface-definitions/system-console.xml.in b/interface-definitions/system-console.xml.in
index ccaaa51b2..71e63d0cb 100644
--- a/interface-definitions/system-console.xml.in
+++ b/interface-definitions/system-console.xml.in
@@ -11,20 +11,24 @@
           <tagNode name="device">
             <properties>
               <help>Serial console device name</help>
+              <completionHelp>
+                <script>ls -1 /dev | grep -e ttyS -e hvc</script>
+                <script>if [ -d /dev/serial/by-bus ]; then ls -1 /dev/serial/by-bus; fi</script>
+              </completionHelp>
               <valueHelp>
                 <format>ttySN</format>
-                <description>Serial device name</description>
+                <description>TTY device name, regular serial port</description>
               </valueHelp>
               <valueHelp>
-                <format>ttyUSBX</format>
-                <description>USB Serial device name</description>
+                <format>usbNbXpY</format>
+                <description>TTY device name, USB based</description>
               </valueHelp>
               <valueHelp>
-                <format>hvc0</format>
+                <format>hvcN</format>
                 <description>Xen console</description>
               </valueHelp>
               <constraint>
-                  <regex>^(ttyS|ttyUSB|hvc)[0-9]+$</regex>
+                  <regex>^(ttyS[0-9]+|hvc[0-9]+|usb[0-9]+b.*)$</regex>
               </constraint>
             </properties>
             <children>
diff --git a/src/conf_mode/system_console.py b/src/conf_mode/system_console.py
index d1d9b7c70..a3e450a36 100755
--- a/src/conf_mode/system_console.py
+++ b/src/conf_mode/system_console.py
@@ -18,40 +18,46 @@ import os
 
 from vyos.config import Config
 from vyos.util import call
+from vyos.template import render
 from vyos import ConfigError, airbag
 airbag.enable()
 
-serial_getty_file = '/lib/systemd/system/serial-getty@.service'
-
 def get_config():
     conf = Config()
     base = ['system', 'console']
 
-    if not conf.exists(base):
-        return None
-
     # retrieve configuration at once
     console = conf.get_config_dict(base)
 
-    # set default values
-    if 'device' in console.keys():
-        for device in console['device'].keys():
-            # no speed setting has been configured - use default value
-            if not 'speed' in console['device'][device].keys():
-                tmp = { 'speed': '' }
-                if device.startswith('hvc'):
-                    tmp['speed'] = 38400
-                else:
-                    tmp['speed'] = 115200
+    # bail out early if no serial console is configured
+    if 'device' not in console.keys():
+        return console
 
-                console['device'][device].update(tmp)
+    # convert CLI values to system values
+    for device in console['device'].keys():
+        # no speed setting has been configured - use default value
+        if not 'speed' in console['device'][device].keys():
+            tmp = { 'speed': '' }
+            if device.startswith('hvc'):
+                tmp['speed'] = 38400
+            else:
+                tmp['speed'] = 115200
+
+            console['device'][device].update(tmp)
+
+        if device.startswith('usb'):
+            # It is much easiert to work with the native ttyUSBn name when using
+            # getty, but that name may change across reboots - depending on the
+            # amount of connected devices. We will resolve the fixed device name
+            # to its dynamic device file - and create a new dict entry for it.
+            #
+            # updating the dict must come as last step in the loop!
+            tmp = os.path.basename(os.readlink('/dev/serial/by-bus/usb0b1p1.0'))
+            console['device'][tmp] = console['device'].pop(device)
 
     return console
 
 def verify(console):
-    if not os.path.isfile(serial_getty_file):
-        raise ConfigError(f'Could not open: {serial_getty_file}')
-
     return None
 
 def generate(console):
@@ -63,20 +69,9 @@ def generate(console):
                 call(f'systemctl stop {basename}')
                 os.unlink(os.path.join(root, basename))
 
-    # bail out early if serial device is not configured
-    if not console or 'device' not in console.keys():
-        return None
-
     for device in console['device'].keys():
-        serial_getty_device_file = f'{base_dir}/serial-getty@{device}.service'
-        serial_getty_wants_file = f'{base_dir}/getty.target.wants/serial-getty@{device}.service'
-
-        with open(serial_getty_file, 'r') as f:
-            tmp = f.read()
-        tmp = tmp.replace('115200,38400,9600', str(console['device'][device]['speed']))
-
-        with open(serial_getty_device_file, 'w') as f:
-            f.write(tmp)
+        config_file = base_dir + f'/serial-getty@{device}.service'
+        render(config_file, 'getty/serial-getty.service.tmpl', console['device'][device])
 
     # Reload systemd manager configuration
     call('systemctl daemon-reload')
diff --git a/src/migration-scripts/system/16-to-17 b/src/migration-scripts/system/16-to-17
index ca3b10f49..981149d1b 100755
--- a/src/migration-scripts/system/16-to-17
+++ b/src/migration-scripts/system/16-to-17
@@ -47,6 +47,26 @@ else:
         if config.exists(dev_path + ['modem']):
             config.delete(dev_path + ['modem'])
 
+        # Only continue on USB based serial consoles
+        if not 'ttyUSB' in device:
+            continue
+
+        # A serial console has been configured but it does no longer
+        # exist on the system - cleanup
+        if not os.path.exists(f'/dev/{device}'):
+            config.delete(dev_path)
+            continue
+
+        # migrate from ttyUSB device to new device in /dev/serial/by-bus
+        for root, dirs, files in os.walk('/dev/serial/by-bus'):
+            for usb_device in files:
+                device_file = os.path.realpath(os.path.join(root, usb_device))
+                # migrate to new USB device names (T2529)
+                if os.path.basename(device_file) == device:
+                    config.copy(dev_path, base + ['device', usb_device])
+                    # Delete old USB node from config
+                    config.delete(dev_path)
+
     try:
         with open(file_name, 'w') as f:
             f.write(config.to_string())