4:27 p.m.
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
4:27 p.m.
New subject: [libvirt] [PATCH v2 1/6] qemu: set domain capabilities for ramfb device
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(a)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
4:27 p.m.
New subject: [libvirt] [PATCH v2 2/6] qemu: use VIR_AUTOUNREF in qemuDomainDeviceDefValidate()
This allows us to simplify the function and avoid jumping to 'cleanup'.
Signed-off-by: Jonathon Jongsma <jjongsma(a)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
11:16 a.m.
New subject: [libvirt] [PATCH v2 2/6] qemu: use VIR_AUTOUNREF in qemuDomainDeviceDefValidate()
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(a)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
4:27 p.m.
New subject: [libvirt] [PATCH v2 3/6] qemu: fix domain device validation
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(a)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
4:27 p.m.
New subject: [libvirt] [PATCH v2 4/6] qemu: consolidate video validation
Move video validation logic from qemuProcessStartValidateVideo() to
qemuDomainDeviceDefValidateVideo() (which is in fact called from the
aforementioned function).
Signed-off-by: Jonathon Jongsma <jjongsma(a)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
12:03 p.m.
New subject: [libvirt] [PATCH v2 4/6] qemu: consolidate video validation
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(a)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
4:27 p.m.
New subject: [libvirt] [PATCH v2 5/6] qemu: unify domain caps validation for video devices
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(a)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
4:27 p.m.
New subject: [libvirt] [PATCH v2 6/6] qemu: add 'ramfb' attribute for mediated devices
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(a)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
1862
days inactive
1866
days old
8 comments
2 participants
participants (2)
-
Cole Robinson -
Jonathon Jongsma