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

[Laszlo Ersek]

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