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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
