Re: [libvirt] [PATCH v2 2/3] conf: Introduce new video type 'none'
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 > > --- > > 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 model element has a mandatory type > > attribute which takes the value "vga", "cirrus", "vmvga", "xen", > > "vbox", "qxl" (since 0.8.6), > > - "virtio" (since 1.3.0) > > - or "gop" (since 3.2.0) > > + "virtio" (since 1.3.0), > > + "gop" (since 3.2.0), or > > + "none" (since 4.6.0) > > depending on the hypervisor features available. > > + The purpose of the type none 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 video device along with type > > + none. > > + Refer to Host device assignment to > > see > > + how to add a mediated device into a guest. > > > > > > 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 @@ > > vbox > > virtio > > gop > > +none > > > > > > > > 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) { > > +
Re: [libvirt] [PATCH v2 2/3] conf: Introduce new video type 'none'
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 --- 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 model element has a mandatory type attribute which takes the value "vga", "cirrus", "vmvga", "xen", "vbox", "qxl" (since 0.8.6), - "virtio" (since 1.3.0) - or "gop" (since 3.2.0) + "virtio" (since 1.3.0), + "gop" (since 3.2.0), or + "none" (since 4.6.0) depending on the hypervisor features available. + The purpose of the type none 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 video device along with type + none. + Refer to Host device assignment to see + how to add a mediated device into a guest. 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 @@ vbox virtio gop +none 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, , -
[libvirt] [PATCH v2 2/3] conf: Introduce new video type 'none'
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 --- 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 model element has a mandatory type attribute which takes the value "vga", "cirrus", "vmvga", "xen", "vbox", "qxl" (since 0.8.6), - "virtio" (since 1.3.0) - or "gop" (since 3.2.0) + "virtio" (since 1.3.0), + "gop" (since 3.2.0), or + "none" (since 4.6.0) depending on the hypervisor features available. + The purpose of the type none 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 video device along with type + none. + Refer to Host device assignment to see + how to add a mediated device into a guest. 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 @@ vbox virtio gop +none 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, , -NULL, opaque) < 0) -return -1; +if (def->videos[0]->type ==