[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 07/05/2011 04:29 AM, 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

I'll let Dan comment on whether this better fits what he envisioned, but
I have some code comments:

> +++ b/docs/schemas/domain.rng
> @@ -1618,11 +1618,21 @@
>      -->
>    <define name="qemucdev">
>      <ref name="qemucdevSrcType"/>
> -    <optional>
> -      <attribute name="tty">
> -        <ref name="absFilePath"/>
> -      </attribute>
> -    </optional>
> +    <interleave>
> +      <optional>
> +        <attribute name="tty">
> +          <ref name="absFilePath"/>
> +        </attribute>
> +      </optional>
> +      <optional>
> +        <attribute name="sga">
> +          <choice>
> +            <value>on</value>
> +            <value>off</value>
> +          </choice>
> +        </attribute>

Aren't attributes already interleaved in RNG notation?  That is, I
thought that <interleave> was only necessary for sub-elements.

The easiest way to test would be (temporarily) listing 'sga=' prior to
'tty=' and seeing if you still pass the rng validation tests (at that
point, it is the rng test that is important; if you fail the tests for
round-trip from xml -> parsed -> xml, that's okay, since our parser
outputs in fixed order - the real goal of this exercise is to prove that
we are liberal in what we accept).

> +++ 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);

This should issue an error if we lack QEMU_CAPS_SGA but requested
VIR_DOMAIN_CHR_SGA_ON, rather than silently ignoring the request.
Meanwhile, it is okay to have an explicit VIR_DOMAIN_CHR_SGA_OFF even if
we lack the qemu feature.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]