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

Jonathon Jongsma posted 1 patch 4 years, 9 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190627190318.22813-1-jjongsma@redhat.com
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
[libvirt] [PATCH] qemu: add bochs-display device
Posted by Jonathon Jongsma 4 years, 9 months ago
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@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

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).
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 31db599ab9..2ccb393432 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3584,6 +3584,7 @@
                 <value>virtio</value>
                 <value>gop</value>
                 <value>none</value>
+                <value>bochs-display</value>
               </choice>
             </attribute>
             <group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3323c9a5b1..f6da230c18 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -738,6 +738,7 @@ VIR_ENUM_IMPL(virDomainVideo,
               "virtio",
               "gop",
               "none",
+              "bochs-display",
 );
 
 VIR_ENUM_IMPL(virDomainVideoVGAConf,
@@ -15158,6 +15159,7 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
     case VIR_DOMAIN_VIDEO_TYPE_VGA:
     case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
     case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
+    case VIR_DOMAIN_VIDEO_TYPE_BOCHS_DISPLAY:
         if (def->virtType == VIR_DOMAIN_VIRT_VBOX)
             return 8 * 1024;
         else if (def->virtType == VIR_DOMAIN_VIRT_VMWARE)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c1b5fc1337..e8e468426b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1385,6 +1385,7 @@ typedef enum {
     VIR_DOMAIN_VIDEO_TYPE_VIRTIO,
     VIR_DOMAIN_VIDEO_TYPE_GOP,
     VIR_DOMAIN_VIDEO_TYPE_NONE,
+    VIR_DOMAIN_VIDEO_TYPE_BOCHS_DISPLAY,
 
     VIR_DOMAIN_VIDEO_TYPE_LAST
 } virDomainVideoType;
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",
     );
 
 
@@ -1121,6 +1124,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
     { "virtio-serial-pci-transitional", QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL },
     { "virtio-serial-pci-non-transitional", QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL },
     { "max-x86_64-cpu", QEMU_CAPS_X86_MAX_CPU },
+    { "bochs-display", QEMU_CAPS_DEVICE_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 */
+
     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
@@ -111,6 +111,7 @@ VIR_ENUM_IMPL(qemuVideo,
               "", /* no need for virtio */
               "" /* don't support gop */,
               "" /* 'none' doesn't make sense here */,
+              "bochs-display",
 );
 
 VIR_ENUM_DECL(qemuDeviceVideo);
@@ -128,6 +129,7 @@ VIR_ENUM_IMPL(qemuDeviceVideo,
               "virtio-vga",
               "" /* don't support gop */,
               "" /* 'none' doesn't make sense here */,
+              "bochs-display",
 );
 
 VIR_ENUM_DECL(qemuDeviceVideoSecondary);
@@ -145,6 +147,7 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary,
               "virtio-gpu",
               "" /* don't support gop */,
               "" /* 'none' doesn't make sense here */,
+              "bochs-display",
 );
 
 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);
+        }
     }
 
     if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d71d9b3273..4e7ed45ff9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4969,6 +4969,7 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video)
     case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
     case VIR_DOMAIN_VIDEO_TYPE_QXL:
     case VIR_DOMAIN_VIDEO_TYPE_VIRTIO:
+    case VIR_DOMAIN_VIDEO_TYPE_BOCHS_DISPLAY:
     case VIR_DOMAIN_VIDEO_TYPE_LAST:
         break;
     }
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;
 
         case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
index 40718981a8..61be1df782 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
@@ -151,6 +151,7 @@
   <flag name='memory-backend-memfd.hugetlb'/>
   <flag name='iothread.poll-max-ns'/>
   <flag name='memory-backend-file.align'/>
+  <flag name='bochs-display'/>
   <version>2012050</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>42900757</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
index c6394db602..7a322030bd 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
@@ -198,6 +198,7 @@
   <flag name='memory-backend-file.align'/>
   <flag name='nvdimm.unarmed'/>
   <flag name='x86-max-cpu'/>
+  <flag name='bochs-display'/>
   <version>3000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100757</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
index ee6921ff92..400dc45be4 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
@@ -156,6 +156,7 @@
   <flag name='memory-backend-file.align'/>
   <flag name='memory-backend-file.pmem'/>
   <flag name='overcommit'/>
+  <flag name='bochs-display'/>
   <version>3000091</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>42900758</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
index a8cb061bf3..434c644ad4 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
@@ -201,6 +201,7 @@
   <flag name='nvdimm.unarmed'/>
   <flag name='overcommit'/>
   <flag name='x86-max-cpu'/>
+  <flag name='bochs-display'/>
   <version>3000092</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100758</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
index 250b7edd52..8fe369f518 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
@@ -163,6 +163,7 @@
   <flag name='machine.virt.iommu'/>
   <flag name='bitmap-merge'/>
   <flag name='nbd-bitmap'/>
+  <flag name='bochs-display'/>
   <version>4000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>61700758</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index 24b55002a6..2df230c4f7 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -168,6 +168,7 @@
   <flag name='query-current-machine'/>
   <flag name='bitmap-merge'/>
   <flag name='nbd-bitmap'/>
+  <flag name='bochs-display'/>
   <version>4000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>42900758</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
index 230e1e7c99..f4acda457a 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
@@ -166,6 +166,7 @@
   <flag name='query-current-machine'/>
   <flag name='bitmap-merge'/>
   <flag name='nbd-bitmap'/>
+  <flag name='bochs-display'/>
   <version>4000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
index 4b2f4cf628..e71d83ee06 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
@@ -166,6 +166,7 @@
   <flag name='query-current-machine'/>
   <flag name='bitmap-merge'/>
   <flag name='nbd-bitmap'/>
+  <flag name='bochs-display'/>
   <version>4000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>0</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
index 716b756979..1d44a5a1ba 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
@@ -205,6 +205,7 @@
   <flag name='bitmap-merge'/>
   <flag name='nbd-bitmap'/>
   <flag name='x86-max-cpu'/>
+  <flag name='bochs-display'/>
   <version>4000000</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100758</microcodeVersion>
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index 9cbf65b405..f336aeb48c 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -207,6 +207,7 @@
   <flag name='x86-max-cpu'/>
   <flag name='cpu-unavailable-features'/>
   <flag name='canonical-cpu-features'/>
+  <flag name='bochs-display'/>
   <version>4000050</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100759</microcodeVersion>
diff --git a/tests/qemuxml2argvdata/video-bochs-display-device.args b/tests/qemuxml2argvdata/video-bochs-display-device.args
new file mode 100644
index 0000000000..f88e9ccb04
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-bochs-display-device.args
@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 1024 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\
+id=drive-ide0-0-0,cache=none \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
+-device bochs-display,id=video0,vgamem=16384k,bus=pci.0,addr=0x2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/video-bochs-display-device.xml b/tests/qemuxml2argvdata/video-bochs-display-device.xml
new file mode 100644
index 0000000000..f64fed4c7e
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-bochs-display-device.xml
@@ -0,0 +1,29 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2' cache='none'/>
+      <source file='/var/lib/libvirt/images/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='ide' index='0'/>
+    <video>
+      <model type='bochs-display' vram='16384' heads='1'/>
+    </video>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 91ca35d469..07b3689776 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2006,6 +2006,9 @@ mymain(void)
             QEMU_CAPS_DEVICE_VIRTIO_VGA,
             QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
             QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS);
+    DO_TEST("video-bochs-display-device",
+            QEMU_CAPS_DEVICE_BOCHS_DISPLAY,
+            QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
     DO_TEST("video-none-device",
             QEMU_CAPS_VNC);
     DO_TEST_PARSE_ERROR("video-invalid-multiple-devices", NONE);
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add bochs-display device
Posted by Peter Krempa 4 years, 9 months ago
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@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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add bochs-display device
Posted by Ján Tomko 4 years, 9 months ago
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@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.

>[...]
>

[0] then again I still haven't found the time to incorporate the review
feedback for the debugcon series.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add bochs-display device
Posted by Erik Skultety 4 years, 9 months ago
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@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

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