thr3ads.net - Libguestfs - [Libguestfs] [PATCH] lib: direct: Remove use of sga [Sep 2021]

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2021-Sep-08 15:42 UTC

[Libguestfs] [PATCH] lib: direct: Remove use of sga

sga (or "sgabios" or "Serial Graphics Adapter") is an option
ROM for
seabios which directs output to the serial adapter.  This is very
useful for debugging BIOS problems during boot.

RHEL wants to deprecate this feature (in fact, they just deprecated it
without telling us).  However there is an equivalent feature now in
seabios which can be enabled using either -nographic or
-machine graphics=off

This commit removes sga and enables -machine graphics=off in the
direct backend.

(We cannot do the same for the libvirt backend because libvirt has no
feature to implement this yet).
---
 lib/launch-direct.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index 972e77e13..e5b9a5611 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -544,6 +544,13 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
       append_list ("gic-version=host");
 #endif
     append_list_format ("accel=%s", accel_val);
+#if defined(__i386__) || defined(__x86_64__)
+    /* Tell seabios to send debug messages to the serial port.
+     * This used to be done by sgabios.
+     */
+    if (g->verbose)
+      append_list ("graphics=off");
+#endif
   } end_list ();
 
   cpu_model = guestfs_int_get_cpu_model (has_kvm && !force_tcg);
@@ -665,18 +672,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   } end_list ();
 #endif
 
-  if (g->verbose &&
-      guestfs_int_qemu_supports_device (g, data->qemu_data,
-                                        "Serial Graphics Adapter")) {
-    /* Use sgabios instead of vgabios.  This means we'll see BIOS
-     * messages on the serial port, and also works around this bug
-     * in qemu 1.1.0:
-     * https://bugs.launchpad.net/qemu/+bug/1021649
-     * QEmu has included sgabios upstream since just before 1.0.
-     */
-    arg ("-device", "sga");
-  }
-
   /* Set up virtio-serial for the communications channel. */
   start_list ("-chardev") {
     append_list ("socket");
-- 
2.32.0

Laszlo Ersek

2021-Sep-09 11:11 UTC

head link

[Libguestfs] [PATCH] lib: direct: Remove use of sga

On 09/08/21 17:42, Richard W.M. Jones wrote:> sga (or "sgabios" or "Serial Graphics Adapter") is an
option ROM for
> seabios which directs output to the serial adapter.  This is very
> useful for debugging BIOS problems during boot.
> 
> RHEL wants to deprecate this feature (in fact, they just deprecated it
> without telling us).  However there is an equivalent feature now in
> seabios which can be enabled using either -nographic or
> -machine graphics=off
(

That's from SeaBIOS commit 0ebc29f9c4db ("paravirt: serial console
configuration.", 2017-09-22), as far as I can tell. The containing
series is:

     1  44270bc1d285 std: add cp437 to unicode map
     2  90fa51152714 kbd: make enqueue_key public, add ascii_to_keycode
     3  1bda724cc567 romfile: add support for constant files.
     4  0ebc29f9c4db paravirt: serial console configuration.
     5  d6728f301d7e add serial console support

Part of "rel-1.11.0".

)
> 
> This commit removes sga and enables -machine graphics=off in the
> direct backend.
> 
> (We cannot do the same for the libvirt backend because libvirt has no
> feature to implement this yet).
> ---
>  lib/launch-direct.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/launch-direct.c b/lib/launch-direct.c
> index 972e77e13..e5b9a5611 100644
> --- a/lib/launch-direct.c
> +++ b/lib/launch-direct.c
> @@ -544,6 +544,13 @@ launch_direct (guestfs_h *g, void *datav, const char
*arg)
>        append_list ("gic-version=host");
>  #endif
>      append_list_format ("accel=%s", accel_val);
> +#if defined(__i386__) || defined(__x86_64__)
> +    /* Tell seabios to send debug messages to the serial port.
> +     * This used to be done by sgabios.
> +     */
> +    if (g->verbose)
> +      append_list ("graphics=off");
> +#endif
>    } end_list ();
>  
>    cpu_model = guestfs_int_get_cpu_model (has_kvm && !force_tcg);
> @@ -665,18 +672,6 @@ launch_direct (guestfs_h *g, void *datav, const char
*arg)
>    } end_list ();
>  #endif
>  
> -  if (g->verbose &&
> -      guestfs_int_qemu_supports_device (g, data->qemu_data,
> -                                        "Serial Graphics
Adapter")) {
> -    /* Use sgabios instead of vgabios.  This means we'll see BIOS
> -     * messages on the serial port, and also works around this bug
> -     * in qemu 1.1.0:
> -     * https://bugs.launchpad.net/qemu/+bug/1021649
> -     * QEmu has included sgabios upstream since just before 1.0.
> -     */
> -    arg ("-device", "sga");
> -  }
> -
>    /* Set up virtio-serial for the communications channel. */
>    start_list ("-chardev") {
>      append_list ("socket");
> 
So, I'm asking this question mainly for my own education:

How does libguestfs deal with different QEMU versions / QEMU feature
deprecation? Is there some kind of feature detection?

For example, why would it be less good to implement the change as follows:

  if (g->verbose) {
    if (guestfs_int_qemu_supports_device(...) {
      arg ("-device", "sga");
    } else {
#if defined(__i386__) || defined(__x86_64__)
      arg ("-machine", "graphics=off");
#endif
    }

This would continue working with SeaBIOS preceding "rel-1.11.0", if at
the same time, the QEMU board had SGA support.

But: do we care? (That's what I'd like to learn about libguestfs,
primarily.)

The patch looks good to me, otherwise.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks
Laszlo

Daniel P. Berrangé

2021-Sep-09 11:22 UTC

head link

[Libguestfs] [PATCH] lib: direct: Remove use of sga

On Wed, Sep 08, 2021 at 04:42:05PM +0100, Richard W.M. Jones
wrote:> sga (or "sgabios" or "Serial Graphics Adapter") is an
option ROM for
> seabios which directs output to the serial adapter.  This is very
> useful for debugging BIOS problems during boot.
> 
> RHEL wants to deprecate this feature (in fact, they just deprecated it
> without telling us).  However there is an equivalent feature now in
> seabios which can be enabled using either -nographic or
> -machine graphics=off
FWIW, this is available since SeaBIOS 1.11, bundled with
QEMU 2.11.0 which is the minimum libvirt supports these
days. I presume libguestfs min version exceeds that ?
> This commit removes sga and enables -machine graphics=off in the
> direct backend.
> 
> (We cannot do the same for the libvirt backend because libvirt has no
> feature to implement this yet).
You likely won't need any changes to libvirt side. Once I fully confirm
that it has no impact on live migration, we'll transparentely update
libvirt to use the new syntax.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Libguestfs - Sep 2021 - [PATCH] lib: direct: Remove use of sga

[Libguestfs] [PATCH] lib: direct: Remove use of sga

[Libguestfs] [PATCH] lib: direct: Remove use of sga

[Libguestfs] [PATCH] lib: direct: Remove use of sga