Re: [libvirt] [PATCH v3] bios: Add support for SGA
On 08.07.2011 22:46, Eric Blake wrote:
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 @@
> <boot dev='cdrom'/>
> <bootmenu enable='yes'/>
> <smbios mode='sysinfo'/>
> +<bios useserial='yes'/>
> </os>
> ...</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.
Thanks pushed.
Michal