[libvirt] [PATCH 5/5] conf: Introduce new video type 'none'

Erik Skultety posted 5 patches 6 years, 10 months ago
There is a newer version of this series
[libvirt] [PATCH 5/5] conf: Introduce new video type 'none'
Posted by Erik Skultety 6 years, 10 months ago
Historically, we've always enabled an emulated video device every time we
see that graphics should be supported with a guest. With the appearance
of mediated devices which can support QEMU's vfio-display capability,
users might want to use such a device as the only video device.
Therefore introduce a new, effectively a 'disable', type for video
device.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 docs/formatdomain.html.in                          | 10 +++-
 docs/schemas/domaincommon.rng                      |  1 +
 src/conf/domain_conf.c                             | 55 ++++++++++++++++------
 src/conf/domain_conf.h                             |  1 +
 src/qemu/qemu_command.c                            | 13 +++--
 src/qemu/qemu_domain.c                             |  3 ++
 src/qemu/qemu_domain_address.c                     | 10 ++++
 tests/domaincapsschemadata/full.xml                |  1 +
 .../video-invalid-multiple-devices.xml             | 33 +++++++++++++
 tests/qemuxml2argvdata/video-none-device.args      | 27 +++++++++++
 tests/qemuxml2argvdata/video-none-device.xml       | 39 +++++++++++++++
 tests/qemuxml2argvtest.c                           |  4 +-
 tests/qemuxml2xmloutdata/video-none-device.xml     | 42 +++++++++++++++++
 tests/qemuxml2xmltest.c                            |  1 +
 14 files changed, 219 insertions(+), 21 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/video-invalid-multiple-devices.xml
 create mode 100644 tests/qemuxml2argvdata/video-none-device.args
 create mode 100644 tests/qemuxml2argvdata/video-none-device.xml
 create mode 100644 tests/qemuxml2xmloutdata/video-none-device.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f45eee6812..2e8196c21f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6639,9 +6639,15 @@ qemu-kvm -net nic,model=? /dev/null
           The <code>model</code> element has a mandatory <code>type</code>
           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>)
-          or "gop" (<span class="since">since 3.2.0</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>)
           depending on the hypervisor features available.
+          Note that type <code>"none"</code> is currently only available for
+          QEMU and the intended use case is to prevent libvirt from adding a
+          default emulated video card in case a host device like mdev should
+          handle the rendering, see also
+          <a href="#elementsGraphics">Graphical framebuffers</a>.
         </p>
         <p>
           You can provide the amount of video memory in kibibytes (blocks of
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1df479cda2..55da8c079b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3468,6 +3468,7 @@
                 <value>vbox</value>
                 <value>virtio</value>
                 <value>gop</value>
+                <value>none</value>
               </choice>
             </attribute>
             <group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 96ab6cf520..cd2a6c991c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -589,7 +589,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
               "qxl",
               "parallels",
               "virtio",
-              "gop")
+              "gop",
+              "none")
 
 VIR_ENUM_IMPL(virDomainVideoVGAConf, VIR_DOMAIN_VIDEO_VGACONF_LAST,
               "io",
@@ -5125,25 +5126,48 @@ static int
 virDomainDefPostParseVideo(virDomainDefPtr def,
                            void *opaque)
 {
+    size_t i;
+
     if (def->nvideos == 0)
         return 0;
 
-    virDomainDeviceDef device = {
-        .type = VIR_DOMAIN_DEVICE_VIDEO,
-        .data.video = def->videos[0],
-    };
-
-    /* Mark the first video as primary. If the user specified
-     * primary="yes", the parser already inserted the device at
-     * def->videos[0]
+    /* it doesn't make sense to pair video device type 'none' with any other
+     * types, there can be only a single video device in such case
      */
-    def->videos[0]->primary = true;
+    for (i = 0; i < def->nvideos; i++) {
+        if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE &&
+            def->nvideos > 1) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("video device type='none' cannot be paired "
+                             "with any other video device types"));
+            return -1;
+        }
+    }
 
-    /* videos[0] might have been added in AddImplicitDevices, after we've
-     * done the per-device post-parse */
-    if (virDomainDefPostParseDeviceIterator(def, &device,
-                                            NULL, opaque) < 0)
-        return -1;
+    if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) {
+        /* we don't want to format any values we automatically fill in for
+         * videos into the XML, so clear them
+         */
+        virDomainVideoDefClear(def->videos[0]);
+        def->videos[0]->type = VIR_DOMAIN_VIDEO_TYPE_NONE;
+    } else {
+        virDomainDeviceDef device = {
+            .type = VIR_DOMAIN_DEVICE_VIDEO,
+            .data.video = def->videos[0],
+        };
+
+        /* Mark the first video as primary. If the user specified
+         * primary="yes", the parser already inserted the device at
+         * def->videos[0]
+         */
+        def->videos[0]->primary = true;
+
+        /* videos[0] might have been added in AddImplicitDevices, after we've
+         * done the per-device post-parse */
+        if (virDomainDefPostParseDeviceIterator(def, &device,
+                                                NULL, opaque) < 0)
+            return -1;
+    }
 
     return 0;
 }
@@ -15093,6 +15117,7 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
     case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
     case VIR_DOMAIN_VIDEO_TYPE_VIRTIO:
     case VIR_DOMAIN_VIDEO_TYPE_GOP:
+    case VIR_DOMAIN_VIDEO_TYPE_NONE:
     case VIR_DOMAIN_VIDEO_TYPE_LAST:
     default:
         return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6d73a0b5d3..c1970e0f62 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1424,6 +1424,7 @@ typedef enum {
     VIR_DOMAIN_VIDEO_TYPE_PARALLELS, /* pseudo device for VNC in containers */
     VIR_DOMAIN_VIDEO_TYPE_VIRTIO,
     VIR_DOMAIN_VIDEO_TYPE_GOP,
+    VIR_DOMAIN_VIDEO_TYPE_NONE,
 
     VIR_DOMAIN_VIDEO_TYPE_LAST
 } virDomainVideoType;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a2a27b5b9b..bc798f9d2d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -105,7 +105,8 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
               "qxl",
               "", /* don't support parallels */
               "", /* no need for virtio */
-              "" /* don't support gop */);
+              "" /* don't support gop */,
+              "none" /* no display */);
 
 VIR_ENUM_DECL(qemuDeviceVideo)
 
@@ -119,7 +120,8 @@ VIR_ENUM_IMPL(qemuDeviceVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
               "qxl-vga",
               "", /* don't support parallels */
               "virtio-vga",
-              "" /* don't support gop */);
+              "" /* don't support gop */,
+              "none" /* no display */);
 
 VIR_ENUM_DECL(qemuDeviceVideoSecondary)
 
@@ -133,7 +135,8 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary, VIR_DOMAIN_VIDEO_TYPE_LAST,
               "qxl",
               "", /* don't support parallels */
               "virtio-gpu",
-              "" /* don't support gop */);
+              "" /* don't support gop */,
+              "" /* 'none' doesn't make sense here */);
 
 VIR_ENUM_DECL(qemuSoundCodec)
 
@@ -4421,6 +4424,10 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
         char *str = NULL;
         virDomainVideoDefPtr video = def->videos[i];
 
+        /* no cmdline needed for type 'none' */
+        if (video->type == VIR_DOMAIN_VIDEO_TYPE_NONE)
+            return 0;
+
         if (video->primary) {
             if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY)) {
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 42b7635ef4..51aba8d527 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4444,6 +4444,9 @@ static int
 qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video)
 {
     switch ((virDomainVideoType) video->type) {
+    case VIR_DOMAIN_VIDEO_TYPE_NONE:
+        /* nothing to be validated for 'none' */
+        return 0;
     case VIR_DOMAIN_VIDEO_TYPE_XEN:
     case VIR_DOMAIN_VIDEO_TYPE_VBOX:
     case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index ab2ac022f1..bc9786dd60 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -821,6 +821,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
 
         case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
         case VIR_DOMAIN_VIDEO_TYPE_GOP:
+        case VIR_DOMAIN_VIDEO_TYPE_NONE:
         case VIR_DOMAIN_VIDEO_TYPE_LAST:
             return 0;
         }
@@ -1540,6 +1541,13 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
          * at slot 2.
          */
         virDomainVideoDefPtr primaryVideo = def->videos[0];
+
+        /* for video type 'none' skip this whole procedure */
+        if (primaryVideo->type == VIR_DOMAIN_VIDEO_TYPE_NONE) {
+            ret = 0;
+            goto cleanup;
+        }
+
         if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) {
             memset(&tmp_addr, 0, sizeof(tmp_addr));
             tmp_addr.slot = 2;
@@ -2105,6 +2113,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
 
     /* Video devices */
     for (i = 0; i < def->nvideos; i++) {
+        if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE)
+            break;
 
         if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info))
             continue;
diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml
index d3faf38da0..474df90283 100644
--- a/tests/domaincapsschemadata/full.xml
+++ b/tests/domaincapsschemadata/full.xml
@@ -73,6 +73,7 @@
         <value>parallels</value>
         <value>virtio</value>
         <value>gop</value>
+        <value>none</value>
       </enum>
     </video>
     <hostdev supported='yes'>
diff --git a/tests/qemuxml2argvdata/video-invalid-multiple-devices.xml b/tests/qemuxml2argvdata/video-invalid-multiple-devices.xml
new file mode 100644
index 0000000000..3f105efaae
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-invalid-multiple-devices.xml
@@ -0,0 +1,33 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static' cpuset='1-4,8-20,525'>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='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <video>
+      <model type='qxl'/>
+    </video>
+    <video>
+      <model type='none'/>
+    </video>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/video-none-device.args b/tests/qemuxml2argvdata/video-none-device.args
new file mode 100644
index 0000000000..1b03c0cb97
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-none-device.args
@@ -0,0 +1,27 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-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 \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-vnc 127.0.0.1:0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/video-none-device.xml b/tests/qemuxml2argvdata/video-none-device.xml
new file mode 100644
index 0000000000..4b591562b7
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-none-device.xml
@@ -0,0 +1,39 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='vnc'/>
+    <video>
+      <model type='none'/>
+    </video>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8293be949d..4ed165d76b 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2010,7 +2010,9 @@ mymain(void)
             QEMU_CAPS_DEVICE_VIRTIO_VGA,
             QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
             QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS);
-    DO_TEST_PARSE_ERROR("video-invalid", NONE);
+    DO_TEST("video-none-device",
+            QEMU_CAPS_VNC);
+    DO_TEST_PARSE_ERROR("video-invalid-multiple-devices", NONE);
 
     DO_TEST("virtio-rng-default",
             QEMU_CAPS_DEVICE_VIRTIO_RNG,
diff --git a/tests/qemuxml2xmloutdata/video-none-device.xml b/tests/qemuxml2xmloutdata/video-none-device.xml
new file mode 100644
index 0000000000..6e76b394fe
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/video-none-device.xml
@@ -0,0 +1,42 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <graphics type='vnc' port='-1' autoport='yes'>
+      <listen type='address'/>
+    </graphics>
+    <video>
+      <model type='none'/>
+    </video>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 0095c27cf6..e482705f0e 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1143,6 +1143,7 @@ mymain(void)
             QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS,
             QEMU_CAPS_VNC,
             QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW);
+    DO_TEST("video-none-device", NONE);
 
     DO_TEST("intel-iommu",
             QEMU_CAPS_DEVICE_INTEL_IOMMU);
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] conf: Introduce new video type 'none'
Posted by John Ferlan 6 years, 10 months ago

On 06/28/2018 08:15 AM, Erik Skultety wrote:
> Historically, we've always enabled an emulated video device every time we
> see that graphics should be supported with a guest. With the appearance
> of mediated devices which can support QEMU's vfio-display capability,
> users might want to use such a device as the only video device.
> Therefore introduce a new, effectively a 'disable', type for video
> device.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  docs/formatdomain.html.in                          | 10 +++-
>  docs/schemas/domaincommon.rng                      |  1 +
>  src/conf/domain_conf.c                             | 55 ++++++++++++++++------
>  src/conf/domain_conf.h                             |  1 +
>  src/qemu/qemu_command.c                            | 13 +++--
>  src/qemu/qemu_domain.c                             |  3 ++
>  src/qemu/qemu_domain_address.c                     | 10 ++++
>  tests/domaincapsschemadata/full.xml                |  1 +
>  .../video-invalid-multiple-devices.xml             | 33 +++++++++++++
>  tests/qemuxml2argvdata/video-none-device.args      | 27 +++++++++++
>  tests/qemuxml2argvdata/video-none-device.xml       | 39 +++++++++++++++
>  tests/qemuxml2argvtest.c                           |  4 +-
>  tests/qemuxml2xmloutdata/video-none-device.xml     | 42 +++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  1 +
>  14 files changed, 219 insertions(+), 21 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/video-invalid-multiple-devices.xml
>  create mode 100644 tests/qemuxml2argvdata/video-none-device.args
>  create mode 100644 tests/qemuxml2argvdata/video-none-device.xml
>  create mode 100644 tests/qemuxml2xmloutdata/video-none-device.xml
> 

I assume this has to do with the egl-headless discussion from the other
series, so rather than "none", why not go with headless?  The problem I
see w/ NONE is it's usage/meaning typically as, well, NONE or not
defined; whereas, for graphics it would then mean, well it's defined,
but it's something else.

If this isn't a headless device, then I'll need to revisit my comments
below...

Also please split domain/docs/xml2ml from qemu/xml2argv

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index f45eee6812..2e8196c21f 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6639,9 +6639,15 @@ qemu-kvm -net nic,model=? /dev/null
>            The <code>model</code> element has a mandatory <code>type</code>
>            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>)
> -          or "gop" (<span class="since">since 3.2.0</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>)
>            depending on the hypervisor features available.
> +          Note that type <code>"none"</code> is currently only available for
> +          QEMU and the intended use case is to prevent libvirt from adding a
> +          default emulated video card in case a host device like mdev should
> +          handle the rendering, see also
> +          <a href="#elementsGraphics">Graphical framebuffers</a>.

"host device like mdev" - I assume you meant "type"... In any case, not
sure it's possible from the domain/xml2xml viewpoint to say QEMU only -
having it in domain. Perhaps consider something like:

"The <code>headless</code> type is designed to be used in coordination
with a host device graphics co-processor such as is provided via
Mediated Device vGPUs. When using a <code>headless</code> type, it must
be the only graphics device defined for the domain.

You could add a link from mdev to
https://libvirt.org/drvnodedev.html#MDEV just like the host device mdev
description did or a link to the hostdev section. It's too bad there
wasn't an anchor for that directly...



>          </p>
>          <p>
>            You can provide the amount of video memory in kibibytes (blocks of
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 1df479cda2..55da8c079b 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3468,6 +3468,7 @@
>                  <value>vbox</value>
>                  <value>virtio</value>
>                  <value>gop</value>
> +                <value>none</value>

headless

>                </choice>
>              </attribute>
>              <group>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 96ab6cf520..cd2a6c991c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -589,7 +589,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
>                "qxl",
>                "parallels",
>                "virtio",
> -              "gop")
> +              "gop",
> +              "none")

"headless"

>  
>  VIR_ENUM_IMPL(virDomainVideoVGAConf, VIR_DOMAIN_VIDEO_VGACONF_LAST,
>                "io",
> @@ -5125,25 +5126,48 @@ static int
>  virDomainDefPostParseVideo(virDomainDefPtr def,
>                             void *opaque)
>  {
> +    size_t i;
> +
>      if (def->nvideos == 0)
>          return 0;
>  
> -    virDomainDeviceDef device = {
> -        .type = VIR_DOMAIN_DEVICE_VIDEO,
> -        .data.video = def->videos[0],
> -    };
> -
> -    /* Mark the first video as primary. If the user specified
> -     * primary="yes", the parser already inserted the device at
> -     * def->videos[0]
> +    /* it doesn't make sense to pair video device type 'none' with any other
> +     * types, there can be only a single video device in such case
>       */
> -    def->videos[0]->primary = true;
> +    for (i = 0; i < def->nvideos; i++) {
> +        if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE &&

_HEADLESS

> +            def->nvideos > 1) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("video device type='none' cannot be paired "
> +                             "with any other video device types"));

Consider, "A '%s' video type must be the only video device defined for
the domain."  virDomainVideoTypeToString(VIR_DOMAIN_VIDEO_TYPE_HEADLESS);


> +            return -1;
> +        }
> +    }
>  
> -    /* videos[0] might have been added in AddImplicitDevices, after we've
> -     * done the per-device post-parse */
> -    if (virDomainDefPostParseDeviceIterator(def, &device,
> -                                            NULL, opaque) < 0)
> -        return -1;
> +    if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) {
> +        /* we don't want to format any values we automatically fill in for
> +         * videos into the XML, so clear them
> +         */
> +        virDomainVideoDefClear(def->videos[0]);
> +        def->videos[0]->type = VIR_DOMAIN_VIDEO_TYPE_NONE;
> +    } else {
> +        virDomainDeviceDef device = {
> +            .type = VIR_DOMAIN_DEVICE_VIDEO,
> +            .data.video = def->videos[0],
> +        };
> +
> +        /* Mark the first video as primary. If the user specified
> +         * primary="yes", the parser already inserted the device at
> +         * def->videos[0]
> +         */
> +        def->videos[0]->primary = true;
> +
> +        /* videos[0] might have been added in AddImplicitDevices, after we've
> +         * done the per-device post-parse */
> +        if (virDomainDefPostParseDeviceIterator(def, &device,
> +                                                NULL, opaque) < 0)
> +            return -1;
> +    }
>  
>      return 0;
>  }
> @@ -15093,6 +15117,7 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
>      case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
>      case VIR_DOMAIN_VIDEO_TYPE_VIRTIO:
>      case VIR_DOMAIN_VIDEO_TYPE_GOP:
> +    case VIR_DOMAIN_VIDEO_TYPE_NONE:
>      case VIR_DOMAIN_VIDEO_TYPE_LAST:
>      default:
>          return 0;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 6d73a0b5d3..c1970e0f62 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1424,6 +1424,7 @@ typedef enum {
>      VIR_DOMAIN_VIDEO_TYPE_PARALLELS, /* pseudo device for VNC in containers */
>      VIR_DOMAIN_VIDEO_TYPE_VIRTIO,
>      VIR_DOMAIN_VIDEO_TYPE_GOP,
> +    VIR_DOMAIN_VIDEO_TYPE_NONE,
>  
>      VIR_DOMAIN_VIDEO_TYPE_LAST
>  } virDomainVideoType;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a2a27b5b9b..bc798f9d2d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -105,7 +105,8 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
>                "qxl",
>                "", /* don't support parallels */
>                "", /* no need for virtio */
> -              "" /* don't support gop */);
> +              "" /* don't support gop */,
> +              "none" /* no display */);
>  
>  VIR_ENUM_DECL(qemuDeviceVideo)
>  
> @@ -119,7 +120,8 @@ VIR_ENUM_IMPL(qemuDeviceVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
>                "qxl-vga",
>                "", /* don't support parallels */
>                "virtio-vga",
> -              "" /* don't support gop */);
> +              "" /* don't support gop */,
> +              "none" /* no display */);
>  

I assume these become egl-headless, although I'm not 100% sure based on
Gerd's comment from the other series today.

>  VIR_ENUM_DECL(qemuDeviceVideoSecondary)
>  
> @@ -133,7 +135,8 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary, VIR_DOMAIN_VIDEO_TYPE_LAST,
>                "qxl",
>                "", /* don't support parallels */
>                "virtio-gpu",
> -              "" /* don't support gop */);
> +              "" /* don't support gop */,
> +              "" /* 'none' doesn't make sense here */);
>  
>  VIR_ENUM_DECL(qemuSoundCodec)
>  
> @@ -4421,6 +4424,10 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
>          char *str = NULL;
>          virDomainVideoDefPtr video = def->videos[i];
>  
> +        /* no cmdline needed for type 'none' */
> +        if (video->type == VIR_DOMAIN_VIDEO_TYPE_NONE)
> +            return 0;
> +

This could go outside the loop and a direct def->videos[0].type == type
logic, although it's technically fine as is, just makes it "appear" as
if we quit when we find this rather than knowing that it only can occur
at [0]. Of course that's true in qemuDomainAssignDevicePCISlots too I
suppose, so whatever. Just looks strange.

John

>          if (video->primary) {
>              if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY)) {
>  
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 42b7635ef4..51aba8d527 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4444,6 +4444,9 @@ static int
>  qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video)
>  {
>      switch ((virDomainVideoType) video->type) {
> +    case VIR_DOMAIN_VIDEO_TYPE_NONE:
> +        /* nothing to be validated for 'none' */
> +        return 0;
>      case VIR_DOMAIN_VIDEO_TYPE_XEN:
>      case VIR_DOMAIN_VIDEO_TYPE_VBOX:
>      case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index ab2ac022f1..bc9786dd60 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -821,6 +821,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>  
>          case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
>          case VIR_DOMAIN_VIDEO_TYPE_GOP:
> +        case VIR_DOMAIN_VIDEO_TYPE_NONE:
>          case VIR_DOMAIN_VIDEO_TYPE_LAST:
>              return 0;
>          }
> @@ -1540,6 +1541,13 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>           * at slot 2.
>           */
>          virDomainVideoDefPtr primaryVideo = def->videos[0];
> +
> +        /* for video type 'none' skip this whole procedure */
> +        if (primaryVideo->type == VIR_DOMAIN_VIDEO_TYPE_NONE) {
> +            ret = 0;
> +            goto cleanup;
> +        }
> +
>          if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) {
>              memset(&tmp_addr, 0, sizeof(tmp_addr));
>              tmp_addr.slot = 2;
> @@ -2105,6 +2113,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
>  
>      /* Video devices */
>      for (i = 0; i < def->nvideos; i++) {
> +        if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE)
> +            break;
>  
>          if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info))
>              continue;
> diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml
> index d3faf38da0..474df90283 100644
> --- a/tests/domaincapsschemadata/full.xml
> +++ b/tests/domaincapsschemadata/full.xml
> @@ -73,6 +73,7 @@
>          <value>parallels</value>
>          <value>virtio</value>
>          <value>gop</value>
> +        <value>none</value>
>        </enum>
>      </video>
>      <hostdev supported='yes'>
> diff --git a/tests/qemuxml2argvdata/video-invalid-multiple-devices.xml b/tests/qemuxml2argvdata/video-invalid-multiple-devices.xml
> new file mode 100644
> index 0000000000..3f105efaae
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/video-invalid-multiple-devices.xml
> @@ -0,0 +1,33 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static' cpuset='1-4,8-20,525'>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='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <video>
> +      <model type='qxl'/>
> +    </video>
> +    <video>
> +      <model type='none'/>
> +    </video>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/video-none-device.args b/tests/qemuxml2argvdata/video-none-device.args
> new file mode 100644
> index 0000000000..1b03c0cb97
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/video-none-device.args
> @@ -0,0 +1,27 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i686 \
> +-name QEMUGuest1 \
> +-S \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off \
> +-m 214 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-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 \
> +-boot c \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-vnc 127.0.0.1:0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/video-none-device.xml b/tests/qemuxml2argvdata/video-none-device.xml
> new file mode 100644
> index 0000000000..4b591562b7
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/video-none-device.xml
> @@ -0,0 +1,39 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>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='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <graphics type='vnc'/>
> +    <video>
> +      <model type='none'/>
> +    </video>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> +    </memballoon>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 8293be949d..4ed165d76b 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2010,7 +2010,9 @@ mymain(void)
>              QEMU_CAPS_DEVICE_VIRTIO_VGA,
>              QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
>              QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS);
> -    DO_TEST_PARSE_ERROR("video-invalid", NONE);
> +    DO_TEST("video-none-device",
> +            QEMU_CAPS_VNC);
> +    DO_TEST_PARSE_ERROR("video-invalid-multiple-devices", NONE);
>  
>      DO_TEST("virtio-rng-default",
>              QEMU_CAPS_DEVICE_VIRTIO_RNG,
> diff --git a/tests/qemuxml2xmloutdata/video-none-device.xml b/tests/qemuxml2xmloutdata/video-none-device.xml
> new file mode 100644
> index 0000000000..6e76b394fe
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/video-none-device.xml
> @@ -0,0 +1,42 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>
> +  <vcpu placement='static'>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='block' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> +    </controller>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <graphics type='vnc' port='-1' autoport='yes'>
> +      <listen type='address'/>
> +    </graphics>
> +    <video>
> +      <model type='none'/>
> +    </video>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> +    </memballoon>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 0095c27cf6..e482705f0e 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1143,6 +1143,7 @@ mymain(void)
>              QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS,
>              QEMU_CAPS_VNC,
>              QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW);
> +    DO_TEST("video-none-device", NONE);
>  
>      DO_TEST("intel-iommu",
>              QEMU_CAPS_DEVICE_INTEL_IOMMU);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] conf: Introduce new video type 'none'
Posted by Erik Skultety 6 years, 10 months ago
On Mon, Jul 02, 2018 at 03:48:10PM -0400, John Ferlan wrote:
>
>
> On 06/28/2018 08:15 AM, Erik Skultety wrote:
> > Historically, we've always enabled an emulated video device every time we
> > see that graphics should be supported with a guest. With the appearance
> > of mediated devices which can support QEMU's vfio-display capability,
> > users might want to use such a device as the only video device.
> > Therefore introduce a new, effectively a 'disable', type for video
> > device.
> >
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >  docs/formatdomain.html.in                          | 10 +++-
> >  docs/schemas/domaincommon.rng                      |  1 +
> >  src/conf/domain_conf.c                             | 55 ++++++++++++++++------
> >  src/conf/domain_conf.h                             |  1 +
> >  src/qemu/qemu_command.c                            | 13 +++--
> >  src/qemu/qemu_domain.c                             |  3 ++
> >  src/qemu/qemu_domain_address.c                     | 10 ++++
> >  tests/domaincapsschemadata/full.xml                |  1 +
> >  .../video-invalid-multiple-devices.xml             | 33 +++++++++++++
> >  tests/qemuxml2argvdata/video-none-device.args      | 27 +++++++++++
> >  tests/qemuxml2argvdata/video-none-device.xml       | 39 +++++++++++++++
> >  tests/qemuxml2argvtest.c                           |  4 +-
> >  tests/qemuxml2xmloutdata/video-none-device.xml     | 42 +++++++++++++++++
> >  tests/qemuxml2xmltest.c                            |  1 +
> >  14 files changed, 219 insertions(+), 21 deletions(-)
> >  create mode 100644 tests/qemuxml2argvdata/video-invalid-multiple-devices.xml
> >  create mode 100644 tests/qemuxml2argvdata/video-none-device.args
> >  create mode 100644 tests/qemuxml2argvdata/video-none-device.xml
> >  create mode 100644 tests/qemuxml2xmloutdata/video-none-device.xml
> >
>
> I assume this has to do with the egl-headless discussion from the other
> series, so rather than "none", why not go with headless?  The problem I
> see w/ NONE is it's usage/meaning typically as, well, NONE or not
> defined; whereas, for graphics it would then mean, well it's defined,
> but it's something else.

Well, to me, 'headless' would mean that a I could see a video device on the PCI
bus in the guest handled by some kind of 'headless'-relared driver. The point
here is that there must not be any additional device within the guest. The
problem is (I know you're familiar with this, it's just to recap this for other
readers) that we add a default video device when there's a graphics element but
no video in the XML. Having a type 'none' translating into a "no device" was
the best I could come up with that would match the reality.

>
> If this isn't a headless device, then I'll need to revisit my comments
> below...
>
> Also please split domain/docs/xml2ml from qemu/xml2argv

I'll incorporate the rest of the suggestions you had and resend (I'll merge the
rest).

Erik

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