Libguestfs - May 2017 - [PATCH 3/5] s390x: appliance: Use /dev/ttysclp0 for serial console.

Thanks: Cole Robinson, Dan Horak, Thomas Huth.
---
 lib/launch-libvirt.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index f66c8e0ef..4adb2cfb3 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -1359,6 +1359,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
         return -1;
     }
 
+#ifndef __s390x__
     /* Console. */
     start_element ("serial") {
       attribute ("type", "unix");
@@ -1370,6 +1371,22 @@ construct_libvirt_xml_devices (guestfs_h *g,
         attribute ("port", "0");
       } end_element ();
     } end_element ();
+#else
+    /* https://bugzilla.redhat.com/show_bug.cgi?id=1376547#c14
+     * and https://libvirt.org/formatdomain.html#elementCharConsole
+     */
+    start_element ("console") {
+      attribute ("type", "unix");
+      start_element ("source") {
+        attribute ("mode", "connect");
+        attribute ("path", params->data->console_path);
+      } end_element ();
+      start_element ("target") {
+	attribute ("type", "sclp");
+        attribute ("port", "0");
+      } end_element ();
+    } end_element ();
+#endif
 
     /* Virtio-serial for guestfsd communication. */
     start_element ("channel") {
-- 
2.13.0

The same change to the direct backend as made to the libvirt backend
in the previous commit.
---
 lib/launch-direct.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index 4d5d6b996..53ffd1d6f 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -595,7 +595,18 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   arg ("-device", VIRTIO_SERIAL);
 
   /* Create the serial console. */
+#ifndef __s390x__
   arg ("-serial", "stdio");
+#else
+  start_list ("-chardev") {
+    append_list ("stdio");
+    append_list ("id=charconsole0");
+  } end_list ();
+  start_list ("-device") {
+    append_list ("sclpconsole");
+    append_list ("chardev=charconsole0");
+  } end_list ();
+#endif
 
   if (g->verbose &&
       guestfs_int_qemu_supports_device (g, data->qemu_data,
-- 
2.13.0

---
 lib/appliance-kcmdline.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/appliance-kcmdline.c b/lib/appliance-kcmdline.c
index 4dde7a865..a394e6e67 100644
--- a/lib/appliance-kcmdline.c
+++ b/lib/appliance-kcmdline.c
@@ -40,11 +40,19 @@
   guestfs_int_string_is_valid ((term), 1, 16, \
                                VALID_FLAG_ALPHA|VALID_FLAG_DIGIT,
"-_")
 
-#if defined(__powerpc64__)
+#ifdef __powerpc64__
 #define SERIAL_CONSOLE "console=hvc0 console=ttyS0"
-#elif defined(__arm__) || defined(__aarch64__)
+#endif
+
+#if defined(__arm__) || defined(__aarch64__)
 #define SERIAL_CONSOLE "console=ttyAMA0"
-#else
+#endif
+
+#ifdef __s390x__
+#define SERIAL_CONSOLE "console=ttysclp0"
+#endif
+
+#ifndef SERIAL_CONSOLE
 #define SERIAL_CONSOLE "console=ttyS0"
 #endif
 
-- 
2.13.0

Although we can't use it at the moment because it is incorrect
on at least s390x (RHBZ#1351968).
---
 appliance/init | 1 +
 1 file changed, 1 insertion(+)

diff --git a/appliance/init b/appliance/init
index 2df5f92ce..50a1dc3bd 100755
--- a/appliance/init
+++ b/appliance/init
@@ -185,6 +185,7 @@ else
     sleep 2
 
     # Get name of the serial port, from console= passed by libguestfs.
+    # XXX Consider using /proc/consoles
     guestfs_serial=$(grep -Eo 'console=[^[:space:]]+' /proc/cmdline |
                      sed s/console=//)
 
-- 
2.13.0

PCI devices don't exist/work.  You would see errors such as:

qemu-system-s390x: -device virtio-rng-pci,rng=rng0: MSI-X support is mandatory
in the S390 architecture
---
 lib/guestfs-internal.h | 38 +++++++++++++++++++++++++++++---------
 lib/launch-direct.c    |  4 ++--
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index a04ccff09..ebab006b9 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -142,20 +142,40 @@
 /* Maximum size of Windows explorer.exe.  2.6MB on Windows 7. */
 #define MAX_WINDOWS_EXPLORER_SIZE (4 * 1000 * 1000)
 
-/* Differences in device names on ARM (virtio-mmio) vs normal
- * hardware with PCI.
+/* Differences in device names on ARMv7 (virtio-mmio), s/390x (CCW) vs
+ * normal hardware with PCI.
  */
-#if !defined(__arm__)
-#define VIRTIO_BLK "virtio-blk-pci"
-#define VIRTIO_SCSI "virtio-scsi-pci"
-#define VIRTIO_SERIAL "virtio-serial-pci"
-#define VIRTIO_NET "virtio-net-pci"
-#else /* ARMv7 */
+#ifdef __arm__
 #define VIRTIO_BLK "virtio-blk-device"
 #define VIRTIO_SCSI "virtio-scsi-device"
 #define VIRTIO_SERIAL "virtio-serial-device"
 #define VIRTIO_NET "virtio-net-device"
-#endif /* ARMv7 */
+#define VIRTIO_RNG "virtio-rng-device"
+#endif
+
+#ifdef __s390x__
+#define VIRTIO_BLK "virtio-blk-ccw"
+#define VIRTIO_SCSI "virtio-scsi-ccw"
+#define VIRTIO_SERIAL "virtio-serial-ccw"
+#define VIRTIO_NET "virtio-net-ccw"
+#define VIRTIO_RNG "virtio-rng-ccw"
+#endif
+
+#ifndef VIRTIO_BLK
+#define VIRTIO_BLK "virtio-blk-pci"
+#endif
+#ifndef VIRTIO_SCSI
+#define VIRTIO_SCSI "virtio-scsi-pci"
+#endif
+#ifndef VIRTIO_SERIAL
+#define VIRTIO_SERIAL "virtio-serial-pci"
+#endif
+#ifndef VIRTIO_NET
+#define VIRTIO_NET "virtio-net-pci"
+#endif
+#ifndef VIRTIO_RNG
+#define VIRTIO_RNG "virtio-rng-pci"
+#endif
 
 /* Machine types. */
 #ifdef __arm__
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index 53ffd1d6f..2327f92bb 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -551,14 +551,14 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
    * when needing entropy.
    */
   if (guestfs_int_qemu_supports_device (g, data->qemu_data,
-                                        "virtio-rng-pci")) {
+                                        VIRTIO_RNG)) {
     start_list ("-object") {
       append_list ("rng-random");
       append_list ("filename=/dev/urandom");
       append_list ("id=rng0");
     } end_list ();
     start_list ("-device") {
-      append_list ("virtio-rng-pci");
+      append_list (VIRTIO_RNG);
       append_list ("rng=rng0");
     } end_list ();
   }
-- 
2.13.0

On Wednesday, 17 May 2017 13:09:03 CEST Richard W.M. Jones
wrote:
> ---
>  lib/appliance-kcmdline.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/appliance-kcmdline.c b/lib/appliance-kcmdline.c
> index 4dde7a865..a394e6e67 100644
> --- a/lib/appliance-kcmdline.c
> +++ b/lib/appliance-kcmdline.c
> @@ -40,11 +40,19 @@
>    guestfs_int_string_is_valid ((term), 1, 16, \
>                                 VALID_FLAG_ALPHA|VALID_FLAG_DIGIT,
"-_")
>  
> -#if defined(__powerpc64__)
> +#ifdef __powerpc64__
>  #define SERIAL_CONSOLE "console=hvc0 console=ttyS0"
> -#elif defined(__arm__) || defined(__aarch64__)
> +#endif
> +
> +#if defined(__arm__) || defined(__aarch64__)
>  #define SERIAL_CONSOLE "console=ttyAMA0"
> -#else
> +#endif
> +
> +#ifdef __s390x__
> +#define SERIAL_CONSOLE "console=ttysclp0"
> +#endif
TBH I'd find better a #ifdef ... #elif ... #endif chain.

-- 
Pino Toscano

On Wednesday, 17 May 2017 13:09:05 CEST Richard W.M. Jones
wrote:
> PCI devices don't exist/work.  You would see errors such as:
> 
> qemu-system-s390x: -device virtio-rng-pci,rng=rng0: MSI-X support is
mandatory in the S390 architecture
> ---
>  lib/guestfs-internal.h | 38 +++++++++++++++++++++++++++++---------
>  lib/launch-direct.c    |  4 ++--
>  2 files changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
> index a04ccff09..ebab006b9 100644
> --- a/lib/guestfs-internal.h
> +++ b/lib/guestfs-internal.h
> @@ -142,20 +142,40 @@
>  /* Maximum size of Windows explorer.exe.  2.6MB on Windows 7. */
>  #define MAX_WINDOWS_EXPLORER_SIZE (4 * 1000 * 1000)
>  
> -/* Differences in device names on ARM (virtio-mmio) vs normal
> - * hardware with PCI.
> +/* Differences in device names on ARMv7 (virtio-mmio), s/390x (CCW) vs
> + * normal hardware with PCI.
>   */
> -#if !defined(__arm__)
> -#define VIRTIO_BLK "virtio-blk-pci"
> -#define VIRTIO_SCSI "virtio-scsi-pci"
> -#define VIRTIO_SERIAL "virtio-serial-pci"
> -#define VIRTIO_NET "virtio-net-pci"
> -#else /* ARMv7 */
> +#ifdef __arm__
>  #define VIRTIO_BLK "virtio-blk-device"
>  #define VIRTIO_SCSI "virtio-scsi-device"
>  #define VIRTIO_SERIAL "virtio-serial-device"
>  #define VIRTIO_NET "virtio-net-device"
> -#endif /* ARMv7 */
> +#define VIRTIO_RNG "virtio-rng-device"
> +#endif
> +
> +#ifdef __s390x__
> +#define VIRTIO_BLK "virtio-blk-ccw"
> +#define VIRTIO_SCSI "virtio-scsi-ccw"
> +#define VIRTIO_SERIAL "virtio-serial-ccw"
> +#define VIRTIO_NET "virtio-net-ccw"
> +#define VIRTIO_RNG "virtio-rng-ccw"
> +#endif
As in patch #3, I'd find better a #ifdef ... #elif ... #endif chain.

-- 
Pino Toscano

On Wednesday, 17 May 2017 13:09:01 CEST Richard W.M. Jones
wrote:
> Thanks: Cole Robinson, Dan Horak, Thomas Huth.
> ---
This series, plus the extra two patches for the tests, mostly LGTM.
Just a couple of notes left (actually the same for two patches).

Thanks,
-- 
Pino Toscano