[libvirt] [PATCH] qemu: add bochs-display device

[Erik Skultety]

On Mon, Jul 01, 2019 at 03:30:26PM +0200, Ján Tomko wrote:
> On Fri, Jun 28, 2019 at 08:53:44AM +0200, Peter Krempa wrote:
> > 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.
>
> We do not need it.
>
> > While
> > we try to model stuff universally, in this case keeping the qemu name
> > verbatim probably makes sense.
> >
>
> There are people who might like to argue otherwise [0], but in QEMU the
> device shares a namespace with all the other devices,
> in libvirt you already know it's a display from the <video> tag.

Agreed.

Erik