[libvirt] [PATCH v2 0/6] Enable ramfb parameter for mediated devices
This patch series is a continuation of a previous series that was already mostly merged. The one unmerged patch from the previous series is patch 6 in this series and has been rebased to git master. The other 5 patches are an attempt to address some things that Cole suggested in the previous review. I'm not sure whether I split the patches up too finely or not, but I'm happy to combine a couple of these patches if requested. Jonathon Jongsma (6): qemu: set domain capabilities for ramfb device qemu: use VIR_AUTOUNREF in qemuDomainDeviceDefValidate() qemu: fix domain device validation qemu: consolidate video validation qemu: unify domain caps validation for video devices qemu: add 'ramfb' attribute for mediated devices docs/formatdomain.html.in | 8 + docs/schemas/domaincommon.rng | 5 + src/conf/domain_capabilities.c | 17 ++- src/conf/domain_conf.c | 11 ++ src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_command.c | 17 ++- src/qemu/qemu_domain.c | 142 +++++++++--------- src/qemu/qemu_process.c | 62 -------- .../domaincapsschemadata/qemu_3.0.0.s390x.xml | 1 + .../qemu_3.1.0.x86_64.xml | 1 + .../qemu_4.0.0.x86_64.xml | 1 + .../qemu_4.1.0.x86_64.xml | 1 + .../qemu_4.2.0.x86_64.xml | 1 + ...tdev-mdev-display-ramfb.x86_64-latest.args | 37 +++++ .../hostdev-mdev-display-ramfb.xml | 33 ++++ tests/qemuxml2argvtest.c | 1 + 17 files changed, 203 insertions(+), 138 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.xml -- 2.21.0
commit 9bfcf0f62d9cf16db526a948242a7409ae883209 added the QEMU_CAPS_DEVICE_RAMFB capability but did not set the domain capability. This patch sets the domain capabilities for the ramfb device and updates the tests. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ tests/domaincapsschemadata/qemu_3.0.0.s390x.xml | 1 + tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml | 1 + tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml | 1 + tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml | 1 + tests/domaincapsschemadata/qemu_4.2.0.x86_64.xml | 1 + 6 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 091e48c7e1..11559d3d92 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5400,6 +5400,8 @@ virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_VIRTIO); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY)) VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_BOCHS); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB)) + VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_RAMFB); return 0; } diff --git a/tests/domaincapsschemadata/qemu_3.0.0.s390x.xml b/tests/domaincapsschemadata/qemu_3.0.0.s390x.xml index 0e81e2ea33..d32f7b5875 100644 --- a/tests/domaincapsschemadata/qemu_3.0.0.s390x.xml +++ b/tests/domaincapsschemadata/qemu_3.0.0.s390x.xml @@ -158,6 +158,7 @@ <video supported='yes'> <enum name='modelType'> <value>virtio</value> + <value>ramfb</value> </enum> </video> <hostdev supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml index 059403cebc..ecb1f06e90 100644 --- a/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_3.1.0.x86_64.xml @@ -128,6 +128,7 @@ <value>qxl</value> <value>virtio</value> <value>bochs</value> + <value>ramfb</value> </enum> </video> <hostdev supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml index eb24b9a604..2a475f20be 100644 --- a/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_4.0.0.x86_64.xml @@ -128,6 +128,7 @@ <value>qxl</value> <value>virtio</value> <value>bochs</value> + <value>ramfb</value> </enum> </video> <hostdev supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml b/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml index f5685d2068..07b869859c 100644 --- a/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_4.1.0.x86_64.xml @@ -132,6 +132,7 @@ <value>qxl</value> <value>virtio</value> <value>bochs</value> + <value>ramfb</value> </enum> </video> <hostdev supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_4.2.0.x86_64.xml b/tests/domaincapsschemadata/qemu_4.2.0.x86_64.xml index 5bd376bb2e..2f86eac7f0 100644 --- a/tests/domaincapsschemadata/qemu_4.2.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_4.2.0.x86_64.xml @@ -132,6 +132,7 @@ <value>qxl</value> <value>virtio</value> <value>bochs</value> + <value>ramfb</value> </enum> </video> <hostdev supported='yes'> -- 2.21.0
This allows us to simplify the function and avoid jumping to 'cleanup'. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_domain.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dc7568fe18..516ae7e444 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7197,8 +7197,8 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, { int ret = 0; virQEMUDriverPtr driver = opaque; - virQEMUCapsPtr qemuCaps = NULL; - virDomainCapsPtr domCaps = NULL; + VIR_AUTOUNREF(virQEMUCapsPtr) qemuCaps = NULL; + VIR_AUTOUNREF(virDomainCapsPtr) domCaps = NULL; if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) @@ -7208,13 +7208,13 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, def->os.machine, def->os.arch, def->virtType))) - goto cleanup; + return -1; if ((ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps)) < 0) - goto cleanup; + return ret; if (virDomainCapsDeviceDefValidate(domCaps, dev, def) < 0) - goto cleanup; + return ret; switch ((virDomainDeviceType)dev->type) { case VIR_DOMAIN_DEVICE_NET: @@ -7300,9 +7300,6 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; } - cleanup: - virObjectUnref(qemuCaps); - virObjectUnref(domCaps); return ret; } -- 2.21.0
On 10/11/19 5:27 PM, Jonathon Jongsma wrote:
This allows us to simplify the function and avoid jumping to 'cleanup'.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_domain.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dc7568fe18..516ae7e444 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7197,8 +7197,8 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, { int ret = 0; virQEMUDriverPtr driver = opaque; - virQEMUCapsPtr qemuCaps = NULL; - virDomainCapsPtr domCaps = NULL; + VIR_AUTOUNREF(virQEMUCapsPtr) qemuCaps = NULL; + VIR_AUTOUNREF(virDomainCapsPtr) domCaps = NULL;
if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) @@ -7208,13 +7208,13 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, def->os.machine, def->os.arch, def->virtType))) - goto cleanup; + return -1;
This patch actually uncovers a bug in the domcaps caching, qemumemlocktest is failing. I sent a fix now and CCd you. Previously even when this function failed, we would return 0; So this patch has a side effect besides the cleanup. I think you should move patch #3 before this and have it contain all the bug fixing here. Then do the VIR_AUTOFREE cleanup on top, which should be a no-op. Without cleaning up the function, the bug fixing will look a bit weird: if (!(qemuCaps = virQEMUBlah())) { ret = -1; goto cleanup; } That's due to this function being a little funky because it's using 'ret' as both the function return code, and to capture the return value of functions we are calling. Elsewhere in libvirt we will use a separate 'int rc' to capture return values. Then the code is more like: int ret = -1; int rc; if (foo() < 0) goto cleanup; if ((rc = bar()) < 0) { ret = rc; goto cleanup; } switch()... ret = rc; cleanup: return ret; But after the VIR_AUTOFREE cleanup that distinction probably doesn't matter, so if the bug fix is a bit ugly than IMO that's fine because the VIR_AUTOFREE conversion will clean it up anyways But, did you run the test suite on this series? There are the errors I hit here, but there's also tons of qemuxml2xml breakage with caused by the video validation move. Please make sure you run 'make check' and 'make syntax-check' on every commit in a series before sending. I'll reply to the other patches with more info. Thanks, Cole
When the virDomainCapsDeviceDefValidate() function returned an error status (-1), we were aborting the function early, but returning the default return value (0). This patch properly returns an error in that case. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 516ae7e444..bc455e7da3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7213,7 +7213,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, if ((ret = qemuDomainDeviceDefValidateAddress(dev, qemuCaps)) < 0) return ret; - if (virDomainCapsDeviceDefValidate(domCaps, dev, def) < 0) + if ((ret = virDomainCapsDeviceDefValidate(domCaps, dev, def)) < 0) return ret; switch ((virDomainDeviceType)dev->type) { -- 2.21.0
Move video validation logic from qemuProcessStartValidateVideo() to qemuDomainDeviceDefValidateVideo() (which is in fact called from the aforementioned function). Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_domain.c | 172 +++++++++++++++++++++++++--------------- src/qemu/qemu_process.c | 62 --------------- 2 files changed, 107 insertions(+), 127 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bc455e7da3..def90a0f7d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5706,83 +5706,125 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, static int -qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) +qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video, + virQEMUCapsPtr qemuCaps) { - switch ((virDomainVideoType) video->type) { - case VIR_DOMAIN_VIDEO_TYPE_NONE: - return 0; - 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"), - virDomainVideoTypeToString(video->type)); - return -1; - case VIR_DOMAIN_VIDEO_TYPE_VGA: - case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: - case VIR_DOMAIN_VIDEO_TYPE_VMVGA: - case VIR_DOMAIN_VIDEO_TYPE_QXL: - case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: - case VIR_DOMAIN_VIDEO_TYPE_BOCHS: - case VIR_DOMAIN_VIDEO_TYPE_RAMFB: - case VIR_DOMAIN_VIDEO_TYPE_LAST: - break; - } - - if (!video->primary && - video->type != VIR_DOMAIN_VIDEO_TYPE_QXL && - video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type '%s' is only valid as primary " - "video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - - if (video->accel && video->accel->accel2d == VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("qemu does not support the accel2d setting")); - return -1; - } - - if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { - if (video->vram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'vram' must be less than '%u'"), - UINT_MAX / 1024); + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU does not support 'vhost-user' video device")); + return -1; + } + } else { + switch ((virDomainVideoType) video->type) { + case VIR_DOMAIN_VIDEO_TYPE_NONE: + return 0; + 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"), + virDomainVideoTypeToString(video->type)); + return -1; + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + case VIR_DOMAIN_VIDEO_TYPE_QXL: + case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: + case VIR_DOMAIN_VIDEO_TYPE_BOCHS: + case VIR_DOMAIN_VIDEO_TYPE_RAMFB: + case VIR_DOMAIN_VIDEO_TYPE_LAST: + break; + } + if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_RAMFB && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("this QEMU does not support '%s' video device"), + virDomainVideoTypeToString(video->type)); return -1; } - if (video->ram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'ram' must be less than '%u'"), - UINT_MAX / 1024); + + if (!video->primary && + video->type != VIR_DOMAIN_VIDEO_TYPE_QXL && + video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type '%s' is only valid as primary " + "video device"), + virDomainVideoTypeToString(video->type)); return -1; } - if (video->vgamem) { - if (video->vgamem < 1024) { + + if (video->accel) { + if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && + (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%s 3d acceleration is not supported"), + virDomainVideoTypeToString(video->type)); + return -1; + } + if (video->accel->accel2d == VIR_TRISTATE_SWITCH_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be at least 1 MiB " - "(1024 KiB)")); + _("qemu does not support the accel2d setting")); return -1; } + } - if (video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be power of two")); + if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + return -1; + } + if (video->ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'ram' must be less than '%u'"), + UINT_MAX / 1024); return -1; } + if (video->vgamem) { + if (video->vgamem < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be at least 1 MiB " + "(1024 KiB)")); + return -1; + } + + if (video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be power of two")); + return -1; + } + } } - } - if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA || - video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA) { - if (video->vram && video->vram < 1024) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("value for 'vram' must be at least " - "1 MiB (1024 KiB)")); - return -1; + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA || + video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA) { + if (video->vram && video->vram < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("value for 'vram' must be at least " + "1 MiB (1024 KiB)")); + return -1; + } } } @@ -7247,7 +7289,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_VIDEO: - ret = qemuDomainDeviceDefValidateVideo(dev->data.video); + ret = qemuDomainDeviceDefValidateVideo(dev->data.video, qemuCaps); break; case VIR_DOMAIN_DEVICE_DISK: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c14c09da11..df7b7c2d3e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5240,65 +5240,6 @@ qemuProcessStartValidateGraphics(virDomainObjPtr vm) return 0; } - -static int -qemuProcessStartValidateVideo(virDomainObjPtr vm, - virQEMUCapsPtr qemuCaps) -{ - size_t i; - virDomainVideoDefPtr video; - - for (i = 0; i < vm->def->nvideos; i++) { - video = vm->def->videos[i]; - - if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { - if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU does not support 'vhost-user' video device")); - return -1; - } - } else { - if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_RAMFB && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("this QEMU does not support '%s' video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - - if (video->accel) { - if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && - (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("%s 3d acceleration is not supported"), - virDomainVideoTypeToString(video->type)); - return -1; - } - } - } - } - - return 0; -} - - static int qemuProcessStartValidateIOThreads(virDomainObjPtr vm, virQEMUCapsPtr qemuCaps) @@ -5483,9 +5424,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, if (qemuProcessStartValidateGraphics(vm) < 0) return -1; - if (qemuProcessStartValidateVideo(vm, qemuCaps) < 0) - return -1; - if (qemuProcessStartValidateIOThreads(vm, qemuCaps) < 0) return -1; -- 2.21.0
On 10/11/19 5:27 PM, Jonathon Jongsma wrote:
Move video validation logic from qemuProcessStartValidateVideo() to qemuDomainDeviceDefValidateVideo() (which is in fact called from the aforementioned function).
As mentioned in the other response, this patch adds a lot of test suite breakage. By moving more validation from process start time to XML parse time, it exposes that many test suite test cases do not have the full list of capabilities specified that it would need to actually boot the XML under test. Most of the example failures are in qemuxml2xml test. Run VIR_TEST_DEBUG=1 ./tests/qemuxml2xmltest to see each failure and the associated error message. Most of these cases will just be adding the associated CIRRUS or VGA qemuCaps to the list in qemuxml2xmltest.c. You can also look at the same test case in qemuxml2argvtest.c and copy the caps list from there. Also this patch should be broken up even more IMO. Some ideas: * Add a qemuDomainDeviceDefValidateVideoModel from the existing code qemu_domain.c code, which should be a no-op * Extend it to handle the vhostuser validation * Move the video->accel validation to qemuDomainDeviceDefValidateVideo. it may need to grow an extra check for backend != vhostuser because we will be de-nesting it * Move the remaining model validation to qemuDomainDeviceDefValidateVideoModel Some of that will also help split up the test suite breakage, rather than requiring you to adjust them all at once in one patch. Thanks, Cole
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_domain.c | 172 +++++++++++++++++++++++++--------------- src/qemu/qemu_process.c | 62 --------------- 2 files changed, 107 insertions(+), 127 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bc455e7da3..def90a0f7d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5706,83 +5706,125 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev,
static int -qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) +qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video, + virQEMUCapsPtr qemuCaps) { - switch ((virDomainVideoType) video->type) { - case VIR_DOMAIN_VIDEO_TYPE_NONE: - return 0; - 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"), - virDomainVideoTypeToString(video->type)); - return -1; - case VIR_DOMAIN_VIDEO_TYPE_VGA: - case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: - case VIR_DOMAIN_VIDEO_TYPE_VMVGA: - case VIR_DOMAIN_VIDEO_TYPE_QXL: - case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: - case VIR_DOMAIN_VIDEO_TYPE_BOCHS: - case VIR_DOMAIN_VIDEO_TYPE_RAMFB: - case VIR_DOMAIN_VIDEO_TYPE_LAST: - break; - } - - if (!video->primary && - video->type != VIR_DOMAIN_VIDEO_TYPE_QXL && - video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type '%s' is only valid as primary " - "video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - - if (video->accel && video->accel->accel2d == VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("qemu does not support the accel2d setting")); - return -1; - } - - if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { - if (video->vram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'vram' must be less than '%u'"), - UINT_MAX / 1024); + if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU does not support 'vhost-user' video device")); + return -1; + } + } else { + switch ((virDomainVideoType) video->type) { + case VIR_DOMAIN_VIDEO_TYPE_NONE: + return 0; + 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"), + virDomainVideoTypeToString(video->type)); + return -1; + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + case VIR_DOMAIN_VIDEO_TYPE_QXL: + case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: + case VIR_DOMAIN_VIDEO_TYPE_BOCHS: + case VIR_DOMAIN_VIDEO_TYPE_RAMFB: + case VIR_DOMAIN_VIDEO_TYPE_LAST: + break; + } + if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_RAMFB && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("this QEMU does not support '%s' video device"), + virDomainVideoTypeToString(video->type)); return -1; } - if (video->ram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'ram' must be less than '%u'"), - UINT_MAX / 1024); + + if (!video->primary && + video->type != VIR_DOMAIN_VIDEO_TYPE_QXL && + video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type '%s' is only valid as primary " + "video device"), + virDomainVideoTypeToString(video->type)); return -1; } - if (video->vgamem) { - if (video->vgamem < 1024) { + + if (video->accel) { + if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && + (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("%s 3d acceleration is not supported"), + virDomainVideoTypeToString(video->type)); + return -1; + } + if (video->accel->accel2d == VIR_TRISTATE_SWITCH_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be at least 1 MiB " - "(1024 KiB)")); + _("qemu does not support the accel2d setting")); return -1; } + }
- if (video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be power of two")); + if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + return -1; + } + if (video->ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'ram' must be less than '%u'"), + UINT_MAX / 1024); return -1; } + if (video->vgamem) { + if (video->vgamem < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be at least 1 MiB " + "(1024 KiB)")); + return -1; + } + + if (video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be power of two")); + return -1; + } + } } - }
- if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA || - video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA) { - if (video->vram && video->vram < 1024) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("value for 'vram' must be at least " - "1 MiB (1024 KiB)")); - return -1; + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA || + video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA) { + if (video->vram && video->vram < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("value for 'vram' must be at least " + "1 MiB (1024 KiB)")); + return -1; + } } }
@@ -7247,7 +7289,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break;
case VIR_DOMAIN_DEVICE_VIDEO: - ret = qemuDomainDeviceDefValidateVideo(dev->data.video); + ret = qemuDomainDeviceDefValidateVideo(dev->data.video, qemuCaps); break;
case VIR_DOMAIN_DEVICE_DISK: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c14c09da11..df7b7c2d3e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5240,65 +5240,6 @@ qemuProcessStartValidateGraphics(virDomainObjPtr vm) return 0; }
- -static int -qemuProcessStartValidateVideo(virDomainObjPtr vm, - virQEMUCapsPtr qemuCaps) -{ - size_t i; - virDomainVideoDefPtr video; - - for (i = 0; i < vm->def->nvideos; i++) { - video = vm->def->videos[i]; - - if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { - if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU does not support 'vhost-user' video device")); - return -1; - } - } else { - if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_RAMFB && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("this QEMU does not support '%s' video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - - if (video->accel) { - if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON && - (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("%s 3d acceleration is not supported"), - virDomainVideoTypeToString(video->type)); - return -1; - } - } - } - } - - return 0; -} - - static int qemuProcessStartValidateIOThreads(virDomainObjPtr vm, virQEMUCapsPtr qemuCaps) @@ -5483,9 +5424,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, if (qemuProcessStartValidateGraphics(vm) < 0) return -1;
- if (qemuProcessStartValidateVideo(vm, qemuCaps) < 0) - return -1; - if (qemuProcessStartValidateIOThreads(vm, qemuCaps) < 0) return -1;
- Cole
As suggested by Cole, this patch uses the domain capabilities to validate the supported video model types. By using the domain capabilities, the validation will automatically adjust when new enum members are added. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/conf/domain_capabilities.c | 17 ++++++++++++- src/qemu/qemu_domain.c | 45 ---------------------------------- 2 files changed, 16 insertions(+), 46 deletions(-) diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index fe93388cce..baf33b4722 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -686,6 +686,19 @@ virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps, return 0; } +static int +virDomainCapsDeviceVideoDefValidate(virDomainCapsPtr const caps, + const virDomainVideoDefPtr dev) +{ + if (ENUM_VALUE_MISSING(caps->video.modelType, dev->type)) { + ENUM_VALUE_ERROR("video model", + virDomainVideoTypeToString(dev->type)); + return -1; + } + + return 0; +} + int virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps, @@ -698,6 +711,9 @@ virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps, case VIR_DOMAIN_DEVICE_RNG: ret = virDomainCapsDeviceRNGDefValidate(caps, dev->data.rng); break; + case VIR_DOMAIN_DEVICE_VIDEO: + ret = virDomainCapsDeviceVideoDefValidate(caps, dev->data.video); + break; case VIR_DOMAIN_DEVICE_DISK: case VIR_DOMAIN_DEVICE_REDIRDEV: @@ -706,7 +722,6 @@ virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps, case VIR_DOMAIN_DEVICE_CHR: case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_HOSTDEV: - case VIR_DOMAIN_DEVICE_VIDEO: case VIR_DOMAIN_DEVICE_MEMORY: case VIR_DOMAIN_DEVICE_VSOCK: case VIR_DOMAIN_DEVICE_INPUT: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index def90a0f7d..bb34638427 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5717,51 +5717,6 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video, return -1; } } else { - switch ((virDomainVideoType) video->type) { - case VIR_DOMAIN_VIDEO_TYPE_NONE: - return 0; - 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"), - virDomainVideoTypeToString(video->type)); - return -1; - case VIR_DOMAIN_VIDEO_TYPE_VGA: - case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: - case VIR_DOMAIN_VIDEO_TYPE_VMVGA: - case VIR_DOMAIN_VIDEO_TYPE_QXL: - case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: - case VIR_DOMAIN_VIDEO_TYPE_BOCHS: - case VIR_DOMAIN_VIDEO_TYPE_RAMFB: - case VIR_DOMAIN_VIDEO_TYPE_LAST: - break; - } - if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY)) || - (video->type == VIR_DOMAIN_VIDEO_TYPE_RAMFB && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_RAMFB))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("this QEMU does not support '%s' video device"), - virDomainVideoTypeToString(video->type)); - return -1; - } - if (!video->primary && video->type != VIR_DOMAIN_VIDEO_TYPE_QXL && video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { -- 2.21.0
The 'ramfb' attribute provides a framebuffer to the guest that can be used as a boot display for the vgpu For example, the following configuration can be used to provide a vgpu with a boot display: <hostdev mode='subsystem' type='mdev' model='vfio-pci' display='on' ramfb='on'> <source> <address uuid='$UUID'/> </source> </hostdev> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- docs/formatdomain.html.in | 8 ++++ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 11 ++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 17 ++++++++- ...tdev-mdev-display-ramfb.x86_64-latest.args | 37 +++++++++++++++++++ .../hostdev-mdev-display-ramfb.xml | 33 +++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 8 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7ec57135f6..81e8cafe87 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4846,6 +4846,14 @@ <a href="#elementsGraphics">graphical framebuffer</a> in order to use this attribute, currently only supported with VNC, Spice and egl-headless graphics devices. + + <span class="since">Since version 5.9.0</span>, there is an optional + <code>ramfb</code> attribute for devices with + <code>model='vfio-pci'</code>. Supported values are either + <code>on</code> or <code>off</code> (default is 'off'). When + enabled, this attribute provides a memory framebuffer device to the + guest. This framebuffer will be used as a boot display when a vgpu + device is the primary display. <p> Note: There are also some implications on the usage of guest's address type depending on the <code>model</code> attribute, diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ead5a25068..57e0dcf6ed 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4769,6 +4769,11 @@ <value>vfio-ap</value> </choice> </attribute> + <optional> + <attribute name="ramfb"> + <ref name="virOnOff"/> + </attribute> + </optional> <optional> <attribute name="display"> <ref name="virOnOff"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c1705a07b6..d6cf2132d7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8161,6 +8161,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, VIR_AUTOFREE(char *) backendStr = NULL; VIR_AUTOFREE(char *) model = NULL; VIR_AUTOFREE(char *) display = NULL; + VIR_AUTOFREE(char *) ramfb = NULL; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -8176,6 +8177,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, rawio = virXMLPropString(node, "rawio"); model = virXMLPropString(node, "model"); display = virXMLPropString(node, "display"); + ramfb = virXMLPropString(node, "ramfb"); /* @type is passed in from the caller rather than read from the * xml document, because it is specified in different places for @@ -8284,6 +8286,15 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, display); return -1; } + + if (ramfb && + (mdevsrc->ramfb = virTristateSwitchTypeFromString(ramfb)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown value '%s' for <hostdev> attribute " + "'ramfb'"), + ramfb); + return -1; + } } switch (def->source.subsys.type) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b7ae57aa9d..c73034b5a7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -262,6 +262,7 @@ struct _virDomainHostdevSubsysMediatedDev { int model; /* enum virMediatedDeviceModelType */ int display; /* virTristateSwitch */ char uuidstr[VIR_UUID_STRING_BUFLEN]; /* mediated device's uuid string */ + int ramfb; /* virTristateSwitch */ }; typedef enum { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 50cc3bdf7c..049a5b54d9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5417,6 +5417,17 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, return virBufferContentAndReset(&buf); } +static const char * +qemuBuildHostdevMdevModelTypeString(virDomainHostdevSubsysMediatedDevPtr mdev) +{ + /* when the 'ramfb' attribute is set, we must use the nohotplug variant + * rather than 'vfio-pci' */ + if (mdev->model == VIR_MDEV_MODEL_TYPE_VFIO_PCI && + mdev->ramfb == VIR_TRISTATE_SWITCH_ON) + return "vfio-pci-nohotplug"; + + return virMediatedDeviceModelTypeToString(mdev->model); +} char * qemuBuildHostdevMediatedDevStr(const virDomainDef *def, @@ -5431,7 +5442,7 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef *def, if (!(mdevPath = virMediatedDeviceGetSysfsPath(mdevsrc->uuidstr))) return NULL; - dev_str = virMediatedDeviceModelTypeToString(mdevsrc->model); + dev_str = qemuBuildHostdevMdevModelTypeString(mdevsrc); if (!dev_str) return NULL; @@ -5449,6 +5460,10 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef *def, if (dev->info->bootIndex) virBufferAsprintf(&buf, ",bootindex=%u", dev->info->bootIndex); + if (mdevsrc->ramfb == VIR_TRISTATE_SWITCH_ON) + virBufferAsprintf(&buf, ",ramfb=%s", + virTristateSwitchTypeToString(mdevsrc->ramfb)); + if (virBufferCheckError(&buf) < 0) return NULL; diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.x86_64-latest.args new file mode 100644 index 0000000000..30d2f83318 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.x86_64-latest.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest2 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest2/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest2/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest2/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest2,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-vnc 127.0.0.1:0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\ +vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 \ +-device vfio-pci-nohotplug,id=hostdev0,\ +sysfsdev=/sys/bus/mdev/devices/53764d0e-85a0-42b4-af5c-2046b460b1dc,display=on,\ +bus=pci.0,addr=0x3,ramfb=on \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.xml new file mode 100644 index 0000000000..2e7851b2b0 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-ramfb.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest2</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> + <controller type='usb' index='0'> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'> + </controller> + <graphics type='vnc'/> + <hostdev mode='subsystem' type='mdev' model='vfio-pci' display='on' ramfb='on'> + <source> + <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/> + </source> + </hostdev> + <video> + <model type='qxl' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 834a289532..cd51754260 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1638,6 +1638,7 @@ mymain(void) DO_TEST_PARSE_ERROR("hostdev-mdev-display-missing-graphics", QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_VFIO_PCI_DISPLAY); + DO_TEST_CAPS_LATEST("hostdev-mdev-display-ramfb"); DO_TEST_PARSE_ERROR("hostdev-vfio-zpci-wrong-arch", QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("hostdev-vfio-zpci", -- 2.21.0
participants (2)
-
Cole Robinson -
Jonathon Jongsma