Re: [PATCH v3] bios: Add support for SGA

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

 



On 07/08/2011 07:48 AM, Michal Privoznik wrote:
> This patch creates new <bios> element which, at this time has the only

s/the only/only the/

> attribute useserial='yes|no'. This attribute allow users to use
> Serial Graphics Adapter and see BIOS messages from the very first moment
> domain boots up. Therefore, users can choose boot medium, set PXE, etc.
> ---
> diff to v2:
> -move from <serial> to <bios>
> -include Eric's and Dan's suggestions
> 
> 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                     |    9 ++++++
>  docs/schemas/domain.rng                       |   14 +++++++++
>  src/conf/domain_conf.c                        |   27 ++++++++++++++++-
>  src/conf/domain_conf.h                        |   13 ++++++++
>  src/qemu/qemu_capabilities.c                  |    3 ++
>  src/qemu/qemu_capabilities.h                  |    1 +
>  src/qemu/qemu_command.c                       |   20 +++++++++++++
>  tests/qemuxml2argvdata/qemuxml2argv-bios.args |    6 ++++
>  tests/qemuxml2argvdata/qemuxml2argv-bios.xml  |   39 +++++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                      |    1 +
>  10 files changed, 131 insertions(+), 2 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 10d87a9..9cc0bca 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -86,6 +86,7 @@
>      &lt;boot dev='cdrom'/&gt;
>      &lt;bootmenu enable='yes'/&gt;
>      &lt;smbios mode='sysinfo'/&gt;
> +    &lt;bios useserial='yes'/&gt;
>    &lt;/os&gt;
>    ...</pre>
>  
> @@ -137,6 +138,14 @@
>        specified, the hypervisor default is used. <span class="since">
>        Since 0.8.7</span>
>        </dd>
> +      <dt><code>bios</code></dt>
> +      <dd>This element has attribute <code>useserial</code> with possible
> +        values <code>yes</code> or <code>no</code>. It enables or disables
> +        Serial Graphics Adapter which allows users to see BIOS messages
> +        on a serial port. Therefore, one need to have

s/need to have/needs to have a/

> +        <a href="#elementCharSerial">serial port</a> defined.
> +        <span class="since">Since 0.9.4</span>
> +      </dd>
>      </dl>
>  

> +++ b/src/conf/domain_conf.h
> @@ -923,6 +923,18 @@ enum virDomainLifecycleCrashAction {
>      VIR_DOMAIN_LIFECYCLE_CRASH_LAST
>  };
>  
> +enum virDomainBIOSUseserial {
> +    VIR_DOMAIN_BIOS_USESERIAL_DEFAULT = 0,
> +    VIR_DOMAIN_BIOS_USESERIAL_YES,
> +    VIR_DOMAIN_BIOS_USESERIAL_NO
> +};
> +
> +typedef struct _virDomainBIOSDef virDomainBIOSDef;
> +typedef virDomainBIOSDef *virDomainBIOSDefPtr;
> +struct _virDomainBIOSDef {
> +    int useserial;
> +};
> +
>  /* Operating system configuration data & machine / arch */
>  typedef struct _virDomainOSDef virDomainOSDef;
>  typedef virDomainOSDef *virDomainOSDefPtr;
> @@ -942,6 +954,7 @@ struct _virDomainOSDef {
>      char *bootloader;
>      char *bootloaderArgs;
>      int smbios_mode;
> +    virDomainBIOSDef bios;

I'm wondering if we could just have done 'int bios_serial' here, instead
of creating the intermediate type _virDomainBIOSDef.  I guess if we ever
add more attributes or subelements to <bios>, then the struct will be
nice, but until then it seems a bit heavyweight.  But what you have is
not wrong, so no change necessary.

ACK.

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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]