[libvirt] [PATCH] qemu: add bochs-display device
Peter Krempa
pkrempa at redhat.com
Fri Jun 28 06:53:44 UTC 2019
On Thu, Jun 27, 2019 at 14:03:18 -0500, Jonathon Jongsma wrote:
> qemu provides the bochs-display video device since 3.0. This patch adds
> support for this device in libvirt. See Gerd's post for more details:
> https://www.kraxel.org/blog/2018/10/qemu-vga-emulation-and-bochs-display/
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1643404
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
> Note that the documentation may need to be changed depending on which version the patch makes it
> into. I suppose it'll miss 5.5.0 since we're in freeze right now.
>
> Note: depending on which distribution you're using, you may need to copy the vgabios into place in
> order to test. For example:
> $ sudo ln -s /path/to/qemu/pc-bios/vgabios-bochs-display.bin /usr/share/qemu/
>
> docs/formatdomain.html.in | 5 +--
> docs/schemas/domaincommon.rng | 1 +
> src/conf/domain_conf.c | 2 ++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_capabilities.c | 4 +++
> src/qemu/qemu_capabilities.h | 3 ++
> src/qemu/qemu_command.c | 18 +++++++----
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_domain_address.c | 1 +
> .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 +
> .../caps_3.0.0.x86_64.xml | 1 +
> .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 +
> .../caps_3.1.0.x86_64.xml | 1 +
> .../caps_4.0.0.aarch64.xml | 1 +
> .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
> .../caps_4.0.0.riscv32.xml | 1 +
> .../caps_4.0.0.riscv64.xml | 1 +
> .../caps_4.0.0.x86_64.xml | 1 +
> .../caps_4.1.0.x86_64.xml | 1 +
> .../video-bochs-display-device.args | 32 +++++++++++++++++++
> .../video-bochs-display-device.xml | 29 +++++++++++++++++
> tests/qemuxml2argvtest.c | 3 ++
> 22 files changed, 102 insertions(+), 8 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/video-bochs-display-device.args
> create mode 100644 tests/qemuxml2argvdata/video-bochs-display-device.xml
We usually split out config/schema, and capability changes into separate
patches.
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index a7a6ec32a5..9298ee7b16 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6990,8 +6990,9 @@ qemu-kvm -net nic,model=? /dev/null
> attribute which takes the value "vga", "cirrus", "vmvga", "xen",
> "vbox", "qxl" (<span class="since">since 0.8.6</span>),
> "virtio" (<span class="since">since 1.3.0</span>),
> - "gop" (<span class="since">since 3.2.0</span>), or
> - "none" (<span class="since">since 4.6.0</span>)
> + "gop" (<span class="since">since 3.2.0</span>),
> + "none" (<span class="since">since 4.6.0</span>, or "bochs-display"
> + (<span class="since">since 5.5.0</span>)
> depending on the hypervisor features available.
> The purpose of the type <code>none</code> is to instruct libvirt not
> to add a default video device in the guest (see the paragraph above).
I'm wondering whether we need to use the '-display' suffix here. While
we try to model stuff universally, in this case keeping the qemu name
verbatim probably makes sense.
[...]
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 02e84edc15..ec68d05112 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -533,6 +533,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
> "x86-max-cpu",
> "cpu-unavailable-features",
> "canonical-cpu-features",
> +
> + /* 330 */
> + "bochs-display",
> );
[...]
> static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 915ba6cb2e..3cb56e63f4 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -515,6 +515,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
> QEMU_CAPS_CPU_UNAVAILABLE_FEATURES, /* "unavailable-features" CPU property */
> QEMU_CAPS_CANONICAL_CPU_FEATURES, /* avoid CPU feature aliases */
>
> + /* 335 */
> + QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
The section numbers don't match with the above one.
> +
> QEMU_CAPS_LAST /* this must always be the last item */
> } virQEMUCapsFlags;
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 688dc324c6..5455b42f4a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
[...]
> @@ -145,6 +147,7 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary,
> "virtio-gpu",
> "" /* don't support gop */,
> "" /* 'none' doesn't make sense here */,
> + "bochs-display",
You are using it with the 'qemuDeviceVideoSecondary' array, but
qemuDomainDeviceDefValidateVideo allows only QXL and VIRTIO as secondary
graphics, so one of them needs to change.
> );
>
> VIR_ENUM_DECL(qemuSoundCodec);
> @@ -4748,13 +4751,16 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
> if (video->heads)
> virBufferAsprintf(&buf, ",max_outputs=%u", video->heads);
> }
> - } else if (video->vram &&
> - ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
> - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
> - (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
> - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) {
> + } else if (video->vram) {
> + if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
> + (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
> + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM))) {
>
> - virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024);
> + virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024);
> + } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS_DISPLAY) {
> + virBufferAsprintf(&buf, ",vgamem=%uk", video->vram);
> + }
This logic would probably benefit of some separate refactoring first
before adding actual functionality.
> }
>
> if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
[...]
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 4b99e8ca93..677a3f0499 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -929,6 +929,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
> case VIR_DOMAIN_VIDEO_TYPE_VBOX:
> case VIR_DOMAIN_VIDEO_TYPE_QXL:
> case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
> + case VIR_DOMAIN_VIDEO_TYPE_BOCHS_DISPLAY:
> return pciFlags;
The blog article you linked above says:
"No I/O ports needed. You can plug it into an PCIe slot."
Since the idea is to use it with modern machine types (which usually use
PCIe) as a replacement for VGA I think that fact needs to be taken into
account.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190628/e70d232e/attachment-0001.sig>
More information about the libvir-list
mailing list