[libvirt] [PATCH 0/5] Introduce new video model type 'none'
Historically, we've always been adding a default video device onto the cmdline whenever a graphical framebuffer was requested but a video device was missing. With the appearance of mdev vgpus, having an emulated video device is suboptimal, especially with spice where the streaming client will by default open multiple windows, one for each video device (or 'head' in this case). Therefore, we should have a mechanism to disable the 'default video device addition' and this series does that by introducing a new video model 'none'. This can be applied and tried independently, however, it truly only makes sense on top of [1]. [1] https://www.redhat.com/archives/libvir-list/2018-June/msg01740.html Erik Skultety (5): qemu: address: Handle all the video devices within a single loop conf: Introduce virDomainVideoDefClear helper conf: Introduce virDomainDefPostParseVideo helper qemu: validate: Enforce compile time switch type checking for videos conf: Introduce new video type 'none' docs/formatdomain.html.in | 10 ++- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 86 +++++++++++++++++----- src/conf/domain_conf.h | 4 +- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 13 +++- src/qemu/qemu_domain.c | 6 +- src/qemu/qemu_domain_address.c | 20 +++-- 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, 255 insertions(+), 33 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
We've been handling the primary video device separately from all the other ones when in fact the code to do that was the same. Therefore, let's handle all the devices within the existing 'for' loop. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_domain_address.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e9f460d77a..ab2ac022f1 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2103,15 +2103,9 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, goto error; } - /* Assign a PCI slot to the primary video card if there is not an - * assigned address. */ - if (def->nvideos > 0 && - virDeviceInfoPCIAddressWanted(&def->videos[0]->info)) { - if (qemuDomainPCIAddressReserveNextAddr(addrs, &def->videos[0]->info) < 0) - goto error; - } + /* Video devices */ + for (i = 0; i < def->nvideos; i++) { - for (i = 1; i < def->nvideos; i++) { if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info)) continue; -- 2.14.4
On 06/28/2018 08:14 AM, Erik Skultety wrote:
We've been handling the primary video device separately from all the other ones when in fact the code to do that was the same. Therefore, let's handle all the devices within the existing 'for' loop.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_domain_address.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
May be worthwhile to note that since commit id 133fb140 moved validation of the video device type, thus now it's possible to combine the PCI address checking into one for loop. Reviewed-by: John Ferlan <jferlan@redhat.com> John
Future patches rely on the ability to reset the contents of the virDomainVideoDef structure rather than re-allocating it. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/conf/domain_conf.c | 14 +++++++++++++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 70686ebd88..87dbb9a433 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2549,7 +2549,8 @@ virDomainVideoDefNew(void) } -void virDomainVideoDefFree(virDomainVideoDefPtr def) +void +virDomainVideoDefClear(virDomainVideoDefPtr def) { if (!def) return; @@ -2559,6 +2560,17 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def) VIR_FREE(def->accel); VIR_FREE(def->virtio); VIR_FREE(def->driver); + + memset(def, 0, sizeof(*def)); +} + + +void virDomainVideoDefFree(virDomainVideoDefPtr def) +{ + if (!def) + return; + + virDomainVideoDefClear(def); VIR_FREE(def); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f31c78d5e6..834a7c1d7f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2891,6 +2891,7 @@ void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); virDomainVideoDefPtr virDomainVideoDefNew(void); void virDomainVideoDefFree(virDomainVideoDefPtr def); +void virDomainVideoDefClear(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefNew(void); void virDomainHostdevDefClear(virDomainHostdevDefPtr def); void virDomainHostdevDefFree(virDomainHostdevDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5499a368c0..5bd08d3f67 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -567,6 +567,7 @@ virDomainTPMModelTypeToString; virDomainUSBDeviceDefForeach; virDomainVideoDefaultRAM; virDomainVideoDefaultType; +virDomainVideoDefClear; virDomainVideoDefFree; virDomainVideoDefNew; virDomainVideoTypeFromString; -- 2.14.4
On 06/28/2018 08:14 AM, Erik Skultety wrote:
Future patches rely on the ability to reset the contents of the virDomainVideoDef structure rather than re-allocating it.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/conf/domain_conf.c | 14 +++++++++++++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 15 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Move the video post parse bits into a separate helper as the logic is going to be extended in the future. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/conf/domain_conf.c | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 87dbb9a433..96ab6cf520 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5121,6 +5121,34 @@ virDomainDefBootOrderPostParse(virDomainDefPtr def) } +static int +virDomainDefPostParseVideo(virDomainDefPtr def, + void *opaque) +{ + 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] + */ + 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; +} + + static int virDomainDefPostParseCommon(virDomainDefPtr def, struct virDomainDefPostParseDeviceIteratorData *data) @@ -5157,21 +5185,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def, if (virDomainDefAddImplicitDevices(def) < 0) return -1; - if (def->nvideos != 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] */ - 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, data) < 0) - return -1; - } + if (virDomainDefPostParseVideo(def, data) < 0) + return -1; if (def->nserials != 0) { virDomainDeviceDef device = { -- 2.14.4
On 06/28/2018 08:15 AM, Erik Skultety wrote:
Move the video post parse bits into a separate helper as the logic is going to be extended in the future.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/conf/domain_conf.c | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
There wasn't an explicit type case to the video type enum in qemuDomainDeviceDefValidateVideo, _TYPE_GOP was also missing from the switch. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/conf/domain_conf.h | 2 +- src/qemu/qemu_domain.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 834a7c1d7f..6d73a0b5d3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1454,7 +1454,7 @@ struct _virDomainVideoDriverDef { }; struct _virDomainVideoDef { - int type; + int type; /* enum virDomainVideoType */ unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /* kibibytes (multiples of 1024) */ unsigned int vram64; /* kibibytes (multiples of 1024) */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 32128bf04d..42b7635ef4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4443,10 +4443,11 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, static int qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) { - switch (video->type) { + switch ((virDomainVideoType) video->type) { case VIR_DOMAIN_VIDEO_TYPE_XEN: case VIR_DOMAIN_VIDEO_TYPE_VBOX: case VIR_DOMAIN_VIDEO_TYPE_PARALLELS: + case VIR_DOMAIN_VIDEO_TYPE_GOP: case VIR_DOMAIN_VIDEO_TYPE_DEFAULT: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("video type '%s' is not supported with QEMU"), -- 2.14.4
On 06/28/2018 08:15 AM, Erik Skultety wrote:
There wasn't an explicit type case to the video type enum in qemuDomainDeviceDefValidateVideo, _TYPE_GOP was also missing from the switch.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/conf/domain_conf.h | 2 +- src/qemu/qemu_domain.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
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
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);
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
participants (2)
-
Erik Skultety -
John Ferlan