[libvirt] [PATCH v2 0/3] Introduce new video model type 'none'

v1 here https://www.redhat.com/archives/libvir-list/2018-June/msg01793.html Since v1: - there were only small fixes needed as per the review - decided not to split the patch as requested by the reviewer because the first patch would contain 90% of the changes, both in qemu driver and domain_conf to make the test suite happy (PCI address auto-assignment issues) and a single hunk in qemu_command.c in the second patch to actually enable the feature - this just wasn't worth doing, better keep it together in this case - added some tiny reword patch 1 - added a news update Erik Skultety (3): docs: Rephrase the mediated devices hostdev section a bit conf: Introduce new video type 'none' docs: news: Provide an update about the video type 'none' docs/formatdomain.html.in | 23 ++++++--- docs/news.xml | 14 ++++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 55 ++++++++++++++++------ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 14 ++++-- 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 + 15 files changed, 243 insertions(+), 25 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 -- 2.14.4

Currently it reads: Refer MDEV to create a mediated device on the host ...even though it resembles English, it's not a proper English. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/formatdomain.html.in | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a3afe137bf..84ab5a9d12 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4509,10 +4509,12 @@ determines how the host's vfio driver will expose the device to the guest. Currently, <code>model='vfio-pci'</code> and <code>model='vfio-ccw'</code> (<span class="since">Since 4.4.0</span>) - is supported. Refer <a href="drvnodedev.html#MDEV">MDEV</a> to create - a mediated device on the host. There are also some implications on the - usage of guest's address type depending on the <code>model</code> - attribute, see the <code>address</code> element below. + is supported. <a href="drvnodedev.html#MDEV">MDEV</a> section + provides more information about mediated devices as well as how to + create mediated devices on the host. There are also some implications + on the usage of guest's address type depending on the + <code>model</code> attribute, see the <code>address</code> element + below. </dd> </dl> <p> -- 2.14.4

On Thu, Jul 12, 2018 at 05:08:46PM +0200, Erik Skultety wrote:
Currently it reads: Refer MDEV to create a mediated device on the host
...even though it resembles English, it's not a proper English.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/formatdomain.html.in | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
Looks like a more properer English now. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

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 | 13 ++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 55 ++++++++++++++++------ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 14 ++++-- 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, 223 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 84ab5a9d12..03d94f0533 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6622,9 +6622,18 @@ 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. + 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). + This legacy behaviour can be inconvenient in cases where GPU mediated + devices are meant to be the only rendering device within a guest and + so specifying another <code>video</code> device along with type + <code>none</code>. + Refer to <a id="elementsHostDev">Host device assignment</a> to see + how to add a mediated device into a guest. </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 bd687ce9d3..ed989c000f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3451,6 +3451,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 7396616eda..d4927f0226 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -590,7 +590,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", @@ -5091,25 +5092,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", + _("a '%s' video type must be the only video device " + "defined for the domain")); + 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; } @@ -15023,6 +15047,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 0f10e242fd..fe2fcdc081 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1423,6 +1423,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 44ae8dcef7..545b979a6f 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) @@ -4447,6 +4450,11 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, { size_t i; + /* no cmdline needed for type 'none' */ + if (def->nvideos > 0 && + def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) + return 0; + for (i = 0; i < def->nvideos; i++) { char *str = NULL; virDomainVideoDefPtr video = def->videos[i]; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed76495309..af8c5f8b0b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4471,6 +4471,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 6ea80616af..e045b2627e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -799,6 +799,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; } @@ -1518,6 +1519,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; @@ -2083,6 +2091,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 a929e4314e..af6d61bd16 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1973,7 +1973,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 c0b228515c..fa24a0f4eb 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1140,6 +1140,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

On Thu, Jul 12, 2018 at 05:08:47PM +0200, 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 | 13 ++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 55 ++++++++++++++++------ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 14 ++++-- 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, 223 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 84ab5a9d12..03d94f0533 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6622,9 +6622,18 @@ 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. + 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). + This legacy behaviour can be inconvenient in cases where GPU mediated + devices are meant to be the only rendering device within a guest and + so specifying another <code>video</code> device along with type + <code>none</code>. + Refer to <a id="elementsHostDev">Host device assignment</a> to see + how to add a mediated device into a guest. </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 bd687ce9d3..ed989c000f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3451,6 +3451,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 7396616eda..d4927f0226 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -590,7 +590,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", @@ -5091,25 +5092,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", + _("a '%s' video type must be the only video device " + "defined for the domain")); + return -1; + } + }
This looks like something virDomainDefValidate should do.
- /* 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;
Is there anything else than 'heads = 1' we fill in by default? If we clear other data here, it's because the user specified it, isn't it? Ideally, the parser would only fill in the data that was explicitly specified in the XML input and defaults would be filled in here, but this will do until we fix the parser.
+ } 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; } @@ -15023,6 +15047,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 0f10e242fd..fe2fcdc081 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1423,6 +1423,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 44ae8dcef7..545b979a6f 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 */);
This device is not formatted on the command line, so "" should be enough.
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 */);
Same here.
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)
@@ -4447,6 +4450,11 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, { size_t i;
+ /* no cmdline needed for type 'none' */ + if (def->nvideos > 0 && + def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) + return 0; +
I'd just do: if (video->type == VIR_DOMAIN_VIDEO_TYPE_NONE) continue; in the loop below - we already verified that this video can only appear once and as the only device in the XML.
for (i = 0; i < def->nvideos; i++) { char *str = NULL; virDomainVideoDefPtr video = def->videos[i]; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed76495309..af8c5f8b0b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4471,6 +4471,9 @@ static int qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) { switch ((virDomainVideoType) video->type) { + case VIR_DOMAIN_VIDEO_TYPE_NONE: + /* nothing to be validated for 'none' */
Strictly speaking this is not true, specifying any attributes for type none is nonsense. But we cleared the device definition in postParse already.
+ 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 6ea80616af..e045b2627e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -799,6 +799,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; } @@ -1518,6 +1519,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; + }
I'd rather adjust the above condition to: if (def->nvideos > 0 && def->videos[0]->type != VIR_DOMAIN_VIDEO_TYPE_NONE) to prevent our auto-assignment code to use 0:0:2.0 unless requested, but I cannot estimate the likelihood of someone using <video type='none'/> wanting to add one in the future.
+ if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) { memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 2; @@ -2083,6 +2091,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
/* Video devices */ for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) + break;
continue; for consistency.
if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info)) continue;
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Fri, Jul 13, 2018 at 01:56:32PM +0200, Ján Tomko wrote:
On Thu, Jul 12, 2018 at 05:08:47PM +0200, 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 | 13 ++++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 55 ++++++++++++++++------ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 14 ++++-- 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, 223 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 84ab5a9d12..03d94f0533 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6622,9 +6622,18 @@ 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. + 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). + This legacy behaviour can be inconvenient in cases where GPU mediated + devices are meant to be the only rendering device within a guest and + so specifying another <code>video</code> device along with type + <code>none</code>. + Refer to <a id="elementsHostDev">Host device assignment</a> to see + how to add a mediated device into a guest. </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 bd687ce9d3..ed989c000f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3451,6 +3451,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 7396616eda..d4927f0226 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -590,7 +590,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", @@ -5091,25 +5092,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", + _("a '%s' video type must be the only video device " + "defined for the domain")); + return -1; + } + }
This looks like something virDomainDefValidate should do.
- /* 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;
Is there anything else than 'heads = 1' we fill in by default? If we clear other data here, it's because the user specified it, isn't it?
Ideally, the parser would only fill in the data that was explicitly specified in the XML input and defaults would be filled in here, but this will do until we fix the parser.
+ } 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; } @@ -15023,6 +15047,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 0f10e242fd..fe2fcdc081 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1423,6 +1423,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 44ae8dcef7..545b979a6f 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 */);
This device is not formatted on the command line, so "" should be enough.
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 */);
Same here.
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)
@@ -4447,6 +4450,11 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, { size_t i;
+ /* no cmdline needed for type 'none' */ + if (def->nvideos > 0 && + def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) + return 0; +
I'd just do:
if (video->type == VIR_DOMAIN_VIDEO_TYPE_NONE) continue;
in the loop below - we already verified that this video can only appear once and as the only device in the XML.
for (i = 0; i < def->nvideos; i++) { char *str = NULL; virDomainVideoDefPtr video = def->videos[i]; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed76495309..af8c5f8b0b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4471,6 +4471,9 @@ static int qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) { switch ((virDomainVideoType) video->type) { + case VIR_DOMAIN_VIDEO_TYPE_NONE: + /* nothing to be validated for 'none' */
Strictly speaking this is not true, specifying any attributes for type none is nonsense. But we cleared the device definition in postParse already.
+ 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 6ea80616af..e045b2627e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -799,6 +799,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; } @@ -1518,6 +1519,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; + }
I'd rather adjust the above condition to:
if (def->nvideos > 0 && def->videos[0]->type != VIR_DOMAIN_VIDEO_TYPE_NONE)
to prevent our auto-assignment code to use 0:0:2.0 unless requested, but I cannot estimate the likelihood of someone using <video type='none'/> wanting to add one in the future.
+ if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) { memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 2; @@ -2083,6 +2091,8 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
/* Video devices */ for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE) + break;
continue;
for consistency.
if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info)) continue;
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Thanks, I incorporated your suggestions and pushed the series. Erik

Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/news.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 773c95b0da..93832acc4c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -46,6 +46,20 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + qemu: Introduce a new video model of type 'none' + </summary> + <description> + Historically, libvirt would add a default 'cirrus' video device to + a guest if the XML specified 'graphics' but lacked 'video'. This can + be incovenient with GPU mediated devices which can serve as the only + rendering devices within the guest, rather than still relying on an + emulated GPU which would also be the primary device. Having a 'none' + model is our only backwards compatible option how to turn this legacy + behaviour off when needed. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.14.4

On Thu, Jul 12, 2018 at 05:08:48PM +0200, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/news.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 773c95b0da..93832acc4c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -46,6 +46,20 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + qemu: Introduce a new video model of type 'none' + </summary> + <description> + Historically, libvirt would add a default 'cirrus' video device to + a guest if the XML specified 'graphics' but lacked 'video'.
Historical behavior is hardly news :P
This can + be incovenient with GPU mediated devices which can serve as the only + rendering devices within the guest, rather than still relying on an + emulated GPU which would also be the primary device. Having a 'none' + model is our only backwards compatible option how to turn this legacy + behaviour off when needed.
How about: Introduce a new video model type that disables the automatic addition of a video device to domains with 'graphics' specified in their XML. This can be useful with GPU mediated devices which can serve as the only rendering devices within the guest. Jano
+ </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.14.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Jul 13, 2018 at 02:02:10PM +0200, Ján Tomko wrote:
On Thu, Jul 12, 2018 at 05:08:48PM +0200, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/news.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 773c95b0da..93832acc4c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -46,6 +46,20 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + qemu: Introduce a new video model of type 'none' + </summary> + <description> + Historically, libvirt would add a default 'cirrus' video device to + a guest if the XML specified 'graphics' but lacked 'video'.
Historical behavior is hardly news :P
This can + be incovenient with GPU mediated devices which can serve as the only + rendering devices within the guest, rather than still relying on an + emulated GPU which would also be the primary device. Having a 'none' + model is our only backwards compatible option how to turn this legacy + behaviour off when needed.
How about:
Introduce a new video model type that disables the automatic addition of a video device to domains with 'graphics' specified in their XML.
This can be useful with GPU mediated devices which can serve as the only rendering devices within the guest.
Much better, thanks. I didn't get an explicit ACK or a RB, so may I presume one? Erik

On Wed, Jul 18, 2018 at 02:34:05PM +0200, Erik Skultety wrote:
On Fri, Jul 13, 2018 at 02:02:10PM +0200, Ján Tomko wrote:
On Thu, Jul 12, 2018 at 05:08:48PM +0200, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/news.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 773c95b0da..93832acc4c 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -46,6 +46,20 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + qemu: Introduce a new video model of type 'none' + </summary> + <description> + Historically, libvirt would add a default 'cirrus' video device to + a guest if the XML specified 'graphics' but lacked 'video'.
Historical behavior is hardly news :P
This can + be incovenient with GPU mediated devices which can serve as the only + rendering devices within the guest, rather than still relying on an + emulated GPU which would also be the primary device. Having a 'none' + model is our only backwards compatible option how to turn this legacy + behaviour off when needed.
How about:
Introduce a new video model type that disables the automatic addition of a video device to domains with 'graphics' specified in their XML.
This can be useful with GPU mediated devices which can serve as the only rendering devices within the guest.
Much better, thanks. I didn't get an explicit ACK or a RB, so may I presume one?
Sure. If you don't want to: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Erik Skultety
-
Ján Tomko