[libvirt] [PATCH v2] qemu: Add support for SGA

[Daniel P. Berrange]

On Tue, Jul 05, 2011 at 12:29:43PM +0200, Michal Privoznik wrote:
> This patch creates new attribute 'sga' for <serial> element. Serial Graphics
> Adapter allows users to see BIOS messages from the very first moment
> domain boots up. Therefore, users can choose boot medium, set PXE, etc.
> 
> However, to be able to use this, one need SGABIOS, which is accessible
> here: http://code.google.com/p/sgabios/
> ---
> diff to v1:
> -move from <video> to <serial> as Dan suggested:
> https://www.redhat.com/archives/libvir-list/2011-July/msg00134.html
> 
>  docs/formatdomain.html.in                          |   10 ++++--
>  docs/schemas/domain.rng                            |   20 +++++++++---
>  src/conf/domain_conf.c                             |   34 ++++++++++++++++++-
>  src/conf/domain_conf.h                             |   10 ++++++
>  src/qemu/qemu_capabilities.c                       |    3 ++
>  src/qemu/qemu_capabilities.h                       |    1 +
>  src/qemu/qemu_command.c                            |    5 +++
>  .../qemuxml2argv-serial-pty-chardev.args           |    3 +-
>  .../qemuxml2argv-serial-pty-chardev.xml            |    2 +-
>  tests/qemuxml2argvtest.c                           |    3 +-
>  10 files changed, 78 insertions(+), 13 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index fe8d74c..1e9eee5 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2040,7 +2040,7 @@ qemu-kvm -net nic,model=? /dev/null
>        <source path='/dev/pts/2'/>
>        <target port='0'/>
>      </parallel>
> -    <serial type='pty'>
> +    <serial type='pty' sga='on'>
>        <source path='/dev/pts/3'/>
>        <target port='0'/>
>      </serial>
> @@ -2105,7 +2105,7 @@ qemu-kvm -net nic,model=? /dev/null
>  <pre>
>    ...
>    <devices>
> -    <serial type='pty'>
> +    <serial type='pty' sga='on'>
>        <source path='/dev/pts/3'/>
>        <target port='0'/>
>      </serial>
> @@ -2115,7 +2115,11 @@ qemu-kvm -net nic,model=? /dev/null
>      <p>
>        <code>target</code> can have a <code>port</code> attribute, which
>        specifies the port number. Ports are numbered starting from 0. There are
> -      usually 0, 1 or 2 serial ports.
> +      usually 0, 1 or 2 serial ports. The <code>sga</code> attribute enables
> +      or disables Serial Graphics Adapter. Accepted values are <code>on</code>
> +      and <code>off</code>. To be able to use this feature, you need to install
> +      <a href="http://code.google.com/p/sgabios/">SGABios</a>. SGA
> +      <span class="since">Since 0.9.4</span>

This is describing a private implementation detail of QEMU. The libvirt
XML should be documented in a more general fashion.

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6e4480e..29936a6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3884,6 +3884,11 @@ qemuBuildCommandLine(virConnectPtr conn,
>                  virCommandAddArg(cmd, "-device");
>                  virCommandAddArgFormat(cmd, "isa-serial,chardev=char%s,id=%s",
>                                         serial->info.alias, serial->info.alias);
> +
> +                if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
> +                    serial->source.sga == VIR_DOMAIN_CHR_SGA_ON &&
> +                    qemuCapsGet(qemuCaps, QEMU_CAPS_SGA))
> +                    virCommandAddArgList(cmd, "-device", "sga", NULL);
>              } else {
>                  virCommandAddArg(cmd, "-serial");
>                  if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL)))

Looking at this makes me see a flaw in my suggestion to add an
attribute to <serial>. The SGA option ROM isn't associated with
a particular serial port, nor can you add it multiple times for
each serial port. It just uses the "primary" serial port. So I think
we might be better off putting it in the same place as the other
BIOS options.

It is regretable that we have  <bootmenu enable='no'/>, when it
really should have been  <bios bootmenu=yes|no>, to which we could
have added other attributes :-(

What do people think about adding a new element for this

    <bios useserial='yes|no'/>

???

Also, agree with Eric that whatever we use in XML, we should raise an
error if it is requested in the XML, but not supported in QEMU

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|