[v2] qemu: Default to video type=virtio for machvirt

[libvirt] [PATCH v2 5/5] qemu: Default to video type=virtio for machvirt

Posted by Cole Robinson 7 years, 8 months ago

arm/aarch64 -M virt on KVM doesn't and will never work with standard
VGA card emulation. The recommended method is to use type=virtio, so
let's make it the default for video devices without an explicit type
set by the user

https://bugzilla.redhat.com/show_bug.cgi?id=1404112
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 src/qemu/qemu_domain.c                             |  2 +
 .../qemuxml2argv-aarch64-video-default.args        | 26 ++++++++++++
 .../qemuxml2argv-aarch64-video-default.xml         | 17 ++++++++
 tests/qemuxml2argvtest.c                           |  6 +++
 .../qemuxml2xmlout-aarch64-video-default.xml       | 46 ++++++++++++++++++++++
 tests/qemuxml2xmltest.c                            | 10 +++++
 6 files changed, 107 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-default.xml

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7d8c2c1ce..6f9fafb6e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3670,6 +3670,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
         if (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) {
             if ARCH_IS_PPC64(def->os.arch)
                 dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA;
+            else if (qemuDomainIsVirt(def))
+                dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
             else
                 dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
         }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args
new file mode 100644
index 000000000..359c3875d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args
@@ -0,0 +1,26 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-aarch64 \
+-name aarch64-vgpu \
+-S \
+-M virt \
+-cpu cortex-a57 \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid f3197c89-6457-44fe-b26d-897090ba6541 \
+-nodefconfig \
+-nodefaults \
+-chardev socket,id=charmonitor,\
+path=/tmp/lib/domain--1-aarch64-vgpu/monitor.sock,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
+-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \
+-vnc 127.0.0.1:0 \
+-device virtio-gpu-pci,id=video0,bus=pci.2,addr=0x1
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml
new file mode 100644
index 000000000..bc4ea48f0
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml
@@ -0,0 +1,17 @@
+<domain type='qemu'>
+  <name>aarch64-vgpu</name>
+  <uuid>f3197c89-6457-44fe-b26d-897090ba6541</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='aarch64' machine='virt'>hvm</type>
+  </os>
+  <cpu mode='custom' match='exact'>
+    <model fallback='allow'>cortex-a57</model>
+  </cpu>
+  <devices>
+    <emulator>/usr/bin/qemu-system-aarch64</emulator>
+    <graphics type='vnc'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5cdbc78eb..a536e9397 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2371,6 +2371,12 @@ mymain(void)
             QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420,
             QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
             QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_BOOTINDEX);
+    DO_TEST("aarch64-video-default",
+            QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_GPEX,
+            QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420,
+            QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+            QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+            QEMU_CAPS_VNC);
     DO_TEST("aarch64-aavmf-virtio-mmio",
             QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
             QEMU_CAPS_DEVICE_VIRTIO_MMIO,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-default.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-default.xml
new file mode 100644
index 000000000..47b46d0d0
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-default.xml
@@ -0,0 +1,46 @@
+<domain type='qemu'>
+  <name>aarch64-vgpu</name>
+  <uuid>f3197c89-6457-44fe-b26d-897090ba6541</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='aarch64' machine='virt'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <gic version='2'/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='allow'>cortex-a57</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-aarch64</emulator>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='pci' index='1' model='dmi-to-pci-bridge'>
+      <model name='i82801b11-bridge'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <controller type='pci' index='2' model='pci-bridge'>
+      <model name='pci-bridge'/>
+      <target chassisNr='2'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='pci' index='3' model='pcie-root-port'>
+      <model name='ioh3420'/>
+      <target chassis='3' port='0x10'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </controller>
+    <graphics type='vnc' port='-1' autoport='yes'>
+      <listen type='address'/>
+    </graphics>
+    <video>
+      <model type='virtio' heads='1' primary='yes'/>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/>
+    </video>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 311b71356..97ff36cb1 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1128,6 +1128,16 @@ mymain(void)
             QEMU_CAPS_DEVICE_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_PCIE_ROOT_PORT);
+    DO_TEST("aarch64-video-default",
+            QEMU_CAPS_NODEFCONFIG,
+            QEMU_CAPS_OBJECT_GPEX,
+            QEMU_CAPS_DEVICE_PCI_BRIDGE,
+            QEMU_CAPS_DEVICE_IOH3420,
+            QEMU_CAPS_PCI_MULTIFUNCTION,
+            QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+            QEMU_CAPS_DEVICE_VIRTIO_GPU,
+            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+            QEMU_CAPS_VNC);
 
     DO_TEST_FULL("aarch64-gic-none", WHEN_BOTH, GIC_NONE, NONE);
     DO_TEST_FULL("aarch64-gic-none-v2", WHEN_BOTH, GIC_V2, NONE);
-- 
2.13.5

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

Re: [libvirt] [PATCH v2 5/5] qemu: Default to video type=virtio for machvirt

Posted by Pavel Hrdina 7 years, 8 months ago

On Sun, Aug 27, 2017 at 11:04:42AM -0400, Cole Robinson wrote:
> arm/aarch64 -M virt on KVM doesn't and will never work with standard
> VGA card emulation. The recommended method is to use type=virtio, so
> let's make it the default for video devices without an explicit type
> set by the user

Missing a period at the end of sentence.

> https://bugzilla.redhat.com/show_bug.cgi?id=1404112

I would put an empty line after the BZ link.

> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
>  src/qemu/qemu_domain.c                             |  2 +
>  .../qemuxml2argv-aarch64-video-default.args        | 26 ++++++++++++
>  .../qemuxml2argv-aarch64-video-default.xml         | 17 ++++++++
>  tests/qemuxml2argvtest.c                           |  6 +++
>  .../qemuxml2xmlout-aarch64-video-default.xml       | 46 ++++++++++++++++++++++
>  tests/qemuxml2xmltest.c                            | 10 +++++
>  6 files changed, 107 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-default.xml
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 7d8c2c1ce..6f9fafb6e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3670,6 +3670,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>          if (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) {
>              if ARCH_IS_PPC64(def->os.arch)
>                  dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA;
> +            else if (qemuDomainIsVirt(def))
> +                dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
>              else
>                  dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
>          }
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args
> new file mode 100644
> index 000000000..359c3875d
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.args
> @@ -0,0 +1,26 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-aarch64 \
> +-name aarch64-vgpu \
> +-S \
> +-M virt \
> +-cpu cortex-a57 \
> +-m 1024 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid f3197c89-6457-44fe-b26d-897090ba6541 \
> +-nodefconfig \
> +-nodefaults \
> +-chardev socket,id=charmonitor,\
> +path=/tmp/lib/domain--1-aarch64-vgpu/monitor.sock,server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=readline \
> +-no-acpi \
> +-boot c \
> +-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1 \
> +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x0 \
> +-device ioh3420,port=0x10,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \
> +-vnc 127.0.0.1:0 \
> +-device virtio-gpu-pci,id=video0,bus=pci.2,addr=0x1
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml
> new file mode 100644
> index 000000000..bc4ea48f0
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-default.xml
> @@ -0,0 +1,17 @@
> +<domain type='qemu'>
> +  <name>aarch64-vgpu</name>
> +  <uuid>f3197c89-6457-44fe-b26d-897090ba6541</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='aarch64' machine='virt'>hvm</type>
> +  </os>
> +  <cpu mode='custom' match='exact'>
> +    <model fallback='allow'>cortex-a57</model>
> +  </cpu>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-aarch64</emulator>
> +    <graphics type='vnc'/>

It is possible to skip the graphics and use <video/> instead.  This
would make the test only about video device but I'm fine with having
the graphics in the test as well.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 1/5] qemu: whitelist valid video types
[libvirt] [PATCH v2 2/5] conf: domain: add VIDEO_TYPE_DEFAULT
[libvirt] [PATCH v2 3/5] conf: domain: move video type validation to DeviceDefValidate
[libvirt] [PATCH v2 4/5] qemu: Set default video type in qemu PostParse
[libvirt] [PATCH v2 5/5] qemu: Default to video type=virtio for machvirt