7:22 a.m.
Or, how to turn a single patch into a seven patches series
using the astounding power of review comments!
Andrea Bolognani (7):
tests: Add missing virtio-input capabilities
qemu: Check type range for virtio-input devices
qemu: Remove duplicated option formatting for virtio devices
qemu: Always format iothread for virtio-blk
qemu: Check for virtio-input capabilites at validate time
qemu: Refactor virtio-input capabilities checks
qemu: Unify generation of command line for virtio devices
src/qemu/qemu_command.c | 227 +++++++++++++++++-----------------------
src/qemu/qemu_domain.c | 58 +++++++++-
tests/qemuxml2xmltest.c | 24 ++++-
3 files changed, 173 insertions(+), 136 deletions(-)
--
2.17.1
7:22 a.m.
New subject: [libvirt] [PATCH v2 1/7] tests: Add missing virtio-input capabilities
A few qemuxml2xml tests for virtio-input devices are
missing the capabilities used for the corresponding
qemuxml2argv tests: this wasn't a problem until now
because capabilities were only checked at command line
generation time, but we're going to change that later.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
tests/qemuxml2xmltest.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 47da7a7201..caf79625d4 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1098,12 +1098,30 @@ mymain(void)
DO_TEST("video-virtio-gpu-virgl", NONE);
DO_TEST("video-virtio-gpu-spice-gl", NONE);
DO_TEST("video-virtio-gpu-sdl-gl", NONE);
- DO_TEST("virtio-input", NONE);
- DO_TEST("virtio-input-passthrough", NONE);
+
+ DO_TEST("virtio-input",
+ QEMU_CAPS_VIRTIO_KEYBOARD,
+ QEMU_CAPS_VIRTIO_MOUSE,
+ QEMU_CAPS_VIRTIO_TABLET);
+ DO_TEST("virtio-input-passthrough",
+ QEMU_CAPS_VIRTIO_INPUT_HOST);
DO_TEST("memorybacking-set", NONE);
DO_TEST("memorybacking-unset", NONE);
- DO_TEST("virtio-options", QEMU_CAPS_VIRTIO_SCSI);
+
+ DO_TEST("virtio-options",
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_VIRTIO_KEYBOARD,
+ QEMU_CAPS_VIRTIO_MOUSE,
+ QEMU_CAPS_VIRTIO_TABLET,
+ QEMU_CAPS_VIRTIO_INPUT_HOST,
+ QEMU_CAPS_DEVICE_VIRTIO_GPU,
+ QEMU_CAPS_VIRTIO_GPU_VIRGL,
+ QEMU_CAPS_DEVICE_VIRTIO_RNG,
+ QEMU_CAPS_OBJECT_RNG_RANDOM,
+ QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+ QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM,
+ QEMU_CAPS_VIRTIO_PCI_ATS);
virObjectUnref(cfg);
--
2.17.1
7:34 a.m.
New subject: [libvirt] [PATCH v2 1/7] tests: Add missing virtio-input capabilities
On Thu, Sep 06, 2018 at 02:22:13PM +0200, Andrea Bolognani wrote:
A few qemuxml2xml tests for virtio-input devices are
missing the capabilities used for the corresponding
qemuxml2argv tests: this wasn't a problem until now
because capabilities were only checked at command line
generation time, but we're going to change that later.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
tests/qemuxml2xmltest.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:22 a.m.
New subject: [libvirt] [PATCH v2 2/7] qemu: Check type range for virtio-input devices
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 877269f52e..72d8bb4cf6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4096,7 +4096,10 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
virQEMUBuildBufferEscapeComma(&buf, dev->source.evdev);
break;
case VIR_DOMAIN_INPUT_TYPE_LAST:
- break;
+ default:
+ virReportEnumRangeError(virDomainInputType,
+ dev->type);
+ goto error;
}
if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
--
2.17.1
7:37 a.m.
New subject: [libvirt] [PATCH v2 2/7] qemu: Check type range for virtio-input devices
On Thu, Sep 06, 2018 at 02:22:14PM +0200, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 877269f52e..72d8bb4cf6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4096,7 +4096,10 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
virQEMUBuildBufferEscapeComma(&buf, dev->source.evdev);
break;
case VIR_DOMAIN_INPUT_TYPE_LAST:
- break;
+ default:
+ virReportEnumRangeError(virDomainInputType,
+ dev->type);
This would comfortably fit on one line.
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:22 a.m.
New subject: [libvirt] [PATCH v2 3/7] qemu: Remove duplicated option formatting for virtio devices
There are several functions where we pointlessly duplicate
parts of the format string and pass the same arguments:
refactor them so that the common parts are formatted separately
from the variable parts.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 42 ++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 72d8bb4cf6..b283f7ca83 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2004,8 +2004,6 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
case VIR_DOMAIN_DISK_BUS_VIRTIO:
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
virBufferAddLit(&opt, "virtio-blk-ccw");
- if (disk->iothread)
- virBufferAsprintf(&opt, ",iothread=iothread%u",
disk->iothread);
} else if (disk->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
virBufferAddLit(&opt, "virtio-blk-s390");
@@ -2014,9 +2012,14 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
virBufferAddLit(&opt, "virtio-blk-device");
} else {
virBufferAddLit(&opt, "virtio-blk-pci");
- if (disk->iothread)
- virBufferAsprintf(&opt, ",iothread=iothread%u",
disk->iothread);
}
+
+ if (disk->iothread &&
+ (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
+ disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+ virBufferAsprintf(&opt, ",iothread=iothread%u",
disk->iothread);
+ }
+
qemuBuildIoEventFdStr(&opt, disk->ioeventfd, qemuCaps);
if (disk->event_idx &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_EVENT_IDX)) {
@@ -4064,7 +4067,7 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
_("virtio-mouse is not supported by this QEMU
binary"));
goto error;
}
- virBufferAsprintf(&buf, "virtio-mouse%s,id=%s", suffix,
dev->info.alias);
+ virBufferAsprintf(&buf, "virtio-mouse%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_TABLET:
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) ||
@@ -4074,7 +4077,7 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
_("virtio-tablet is not supported by this QEMU
binary"));
goto error;
}
- virBufferAsprintf(&buf, "virtio-tablet%s,id=%s", suffix,
dev->info.alias);
+ virBufferAsprintf(&buf, "virtio-tablet%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_KBD:
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD) ||
@@ -4084,7 +4087,7 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
_("virtio-keyboard is not supported by this QEMU
binary"));
goto error;
}
- virBufferAsprintf(&buf, "virtio-keyboard%s,id=%s", suffix,
dev->info.alias);
+ virBufferAsprintf(&buf, "virtio-keyboard%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_INPUT_HOST)) {
@@ -4092,8 +4095,7 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
_("virtio-input-host is not supported by this QEMU
binary"));
goto error;
}
- virBufferAsprintf(&buf, "virtio-input-host%s,id=%s,evdev=", suffix,
dev->info.alias);
- virQEMUBuildBufferEscapeComma(&buf, dev->source.evdev);
+ virBufferAsprintf(&buf, "virtio-input-host%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_LAST:
default:
@@ -4102,6 +4104,13 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
goto error;
}
+ virBufferAsprintf(&buf, ",id=%s", dev->info.alias);
+
+ if (dev->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) {
+ virBufferAddLit(&buf, ",evdev=");
+ virQEMUBuildBufferEscapeComma(&buf, dev->source.evdev);
+ }
+
if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
goto error;
@@ -5834,17 +5843,16 @@ qemuBuildRNGDevStr(const virDomainDef *def,
goto error;
if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
- virBufferAsprintf(&buf, "virtio-rng-ccw,rng=obj%s,id=%s",
- dev->info.alias, dev->info.alias);
+ virBufferAddLit(&buf, "virtio-rng-ccw");
else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
- virBufferAsprintf(&buf, "virtio-rng-s390,rng=obj%s,id=%s",
- dev->info.alias, dev->info.alias);
+ virBufferAddLit(&buf, "virtio-rng-s390");
else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
- virBufferAsprintf(&buf, "virtio-rng-device,rng=obj%s,id=%s",
- dev->info.alias, dev->info.alias);
+ virBufferAddLit(&buf, "virtio-rng-device");
else
- virBufferAsprintf(&buf, "virtio-rng-pci,rng=obj%s,id=%s",
- dev->info.alias, dev->info.alias);
+ virBufferAddLit(&buf, "virtio-rng-pci");
+
+ virBufferAsprintf(&buf, ",rng=obj%s,id=%s",
+ dev->info.alias, dev->info.alias);
if (dev->rate > 0) {
virBufferAsprintf(&buf, ",max-bytes=%u", dev->rate);
--
2.17.1
7:39 a.m.
New subject: [libvirt] [PATCH v2 3/7] qemu: Remove duplicated option formatting for virtio devices
On Thu, Sep 06, 2018 at 02:22:15PM +0200, Andrea Bolognani wrote:
There are several functions where we pointlessly duplicate
parts of the format string and pass the same arguments:
refactor them so that the common parts are formatted separately
from the variable parts.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 42 ++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:22 a.m.
New subject: [libvirt] [PATCH v2 4/7] qemu: Always format iothread for virtio-blk
So far we've only formatted it for virtio-blk-pci and
virtio-blk-ccw, but other virtio-blk devices also support
the corresponding option; moreover, we've always formatted
it for all virtio-scsi devices.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b283f7ca83..e5743fad9d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2014,11 +2014,8 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
virBufferAddLit(&opt, "virtio-blk-pci");
}
- if (disk->iothread &&
- (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW ||
- disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+ if (disk->iothread)
virBufferAsprintf(&opt, ",iothread=iothread%u",
disk->iothread);
- }
qemuBuildIoEventFdStr(&opt, disk->ioeventfd, qemuCaps);
if (disk->event_idx &&
--
2.17.1
7:39 a.m.
New subject: [libvirt] [PATCH v2 4/7] qemu: Always format iothread for virtio-blk
On Thu, Sep 06, 2018 at 02:22:16PM +0200, Andrea Bolognani wrote:
So far we've only formatted it for virtio-blk-pci and
virtio-blk-ccw, but other virtio-blk devices also support
the corresponding option; moreover, we've always formatted
it for all virtio-scsi devices.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:22 a.m.
New subject: [libvirt] [PATCH v2 5/7] qemu: Check for virtio-input capabilites at validate time
The appropriate time to ensure the required capabilities are
present is validate rather than command line generation: add
a new qemuDomainDeviceDefValidateInput() function and move
all existing checks there.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 26 ------------------
src/qemu/qemu_domain.c | 59 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 58 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e5743fad9d..db7c3ad698 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4057,41 +4057,15 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
switch ((virDomainInputType)dev->type) {
case VIR_DOMAIN_INPUT_TYPE_MOUSE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE) ||
- (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-mouse is not supported by this QEMU
binary"));
- goto error;
- }
virBufferAsprintf(&buf, "virtio-mouse%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_TABLET:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) ||
- (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-tablet is not supported by this QEMU
binary"));
- goto error;
- }
virBufferAsprintf(&buf, "virtio-tablet%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_KBD:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD) ||
- (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-keyboard is not supported by this QEMU
binary"));
- goto error;
- }
virBufferAsprintf(&buf, "virtio-keyboard%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_INPUT_HOST)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-input-host is not supported by this QEMU
binary"));
- goto error;
- }
virBufferAsprintf(&buf, "virtio-input-host%s", suffix);
break;
case VIR_DOMAIN_INPUT_TYPE_LAST:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f161cf6c84..05e90c3615 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5727,6 +5727,60 @@ qemuDomainDeviceDefValidateGraphics(const virDomainGraphicsDef
*graphics,
}
+static int
+qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
+ const virDomainDef *def ATTRIBUTE_UNUSED,
+ virQEMUCapsPtr qemuCaps)
+{
+ if (input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO)
+ return 0;
+
+ switch ((virDomainInputType)input->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE) ||
+ (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio-mouse is not supported by this QEMU
binary"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) ||
+ (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio-tablet is not supported by this QEMU
binary"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_KBD:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD) ||
+ (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio-keyboard is not supported by this QEMU
binary"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_INPUT_HOST)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio-input-host is not supported by this QEMU
binary"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainInputType,
+ input->type);
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int
qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def,
@@ -5796,9 +5850,12 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
qemuCaps);
break;
+ case VIR_DOMAIN_DEVICE_INPUT:
+ ret = qemuDomainDeviceDefValidateInput(dev->data.input, def, qemuCaps);
+ break;
+
case VIR_DOMAIN_DEVICE_LEASE:
case VIR_DOMAIN_DEVICE_FS:
- case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
--
2.17.1
7:40 a.m.
New subject: [libvirt] [PATCH v2 5/7] qemu: Check for virtio-input capabilites at validate time
s/ites/ities/
On Thu, Sep 06, 2018 at 02:22:17PM +0200, Andrea Bolognani wrote:
The appropriate time to ensure the required capabilities are
present is validate rather than command line generation: add
a new qemuDomainDeviceDefValidateInput() function and move
all existing checks there.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 26 ------------------
src/qemu/qemu_domain.c | 59 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 58 insertions(+), 27 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:22 a.m.
New subject: [libvirt] [PATCH v2 6/7] qemu: Refactor virtio-input capabilities checks
The checks and error messages are mostly the same across
all virtio-input devices, so we can avoid having multiple
copies of the same code.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 51 +++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 05e90c3615..cd4e78993f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5732,43 +5732,33 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
const virDomainDef *def ATTRIBUTE_UNUSED,
virQEMUCapsPtr qemuCaps)
{
+ const char *baseName;
+ int cap;
+ int ccwCap;
+
if (input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO)
return 0;
switch ((virDomainInputType)input->type) {
case VIR_DOMAIN_INPUT_TYPE_MOUSE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE) ||
- (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-mouse is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-mouse";
+ cap = QEMU_CAPS_VIRTIO_MOUSE;
+ ccwCap = QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW;
break;
case VIR_DOMAIN_INPUT_TYPE_TABLET:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) ||
- (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-tablet is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-tablet";
+ cap = QEMU_CAPS_VIRTIO_TABLET;
+ ccwCap = QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW;
break;
case VIR_DOMAIN_INPUT_TYPE_KBD:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD) ||
- (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-keyboard is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-keyboard";
+ cap = QEMU_CAPS_VIRTIO_KEYBOARD;
+ ccwCap = QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW;
break;
case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_INPUT_HOST)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-input-host is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-input-host";
+ cap = QEMU_CAPS_VIRTIO_INPUT_HOST;
+ ccwCap = QEMU_CAPS_VIRTIO_INPUT_HOST;
break;
case VIR_DOMAIN_INPUT_TYPE_LAST:
default:
@@ -5777,6 +5767,15 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, cap) ||
+ (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
+ !virQEMUCapsGet(qemuCaps, ccwCap))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s is not supported by this QEMU binary"),
+ baseName);
+ return -1;
+ }
+
return 0;
}
--
2.17.1
8:05 a.m.
New subject: [libvirt] [PATCH v2 6/7] qemu: Refactor virtio-input capabilities checks
On Thu, Sep 06, 2018 at 02:22:18PM +0200, Andrea Bolognani wrote:
The checks and error messages are mostly the same across
all virtio-input devices, so we can avoid having multiple
copies of the same code.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 51 +++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 05e90c3615..cd4e78993f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5732,43 +5732,33 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
const virDomainDef *def ATTRIBUTE_UNUSED,
virQEMUCapsPtr qemuCaps)
{
+ const char *baseName;
+ int cap;
+ int ccwCap;
+
if (input->bus != VIR_DOMAIN_INPUT_BUS_VIRTIO)
return 0;
switch ((virDomainInputType)input->type) {
case VIR_DOMAIN_INPUT_TYPE_MOUSE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE) ||
- (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-mouse is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-mouse";
+ cap = QEMU_CAPS_VIRTIO_MOUSE;
+ ccwCap = QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW;
break;
case VIR_DOMAIN_INPUT_TYPE_TABLET:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) ||
- (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-tablet is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-tablet";
+ cap = QEMU_CAPS_VIRTIO_TABLET;
+ ccwCap = QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW;
break;
case VIR_DOMAIN_INPUT_TYPE_KBD:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD) ||
- (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW))) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-keyboard is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-keyboard";
+ cap = QEMU_CAPS_VIRTIO_KEYBOARD;
+ ccwCap = QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW;
break;
case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_INPUT_HOST)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("virtio-input-host is not supported by this QEMU
binary"));
- return -1;
- }
+ baseName = "virtio-input-host";
+ cap = QEMU_CAPS_VIRTIO_INPUT_HOST;
+ ccwCap = QEMU_CAPS_VIRTIO_INPUT_HOST;
This is incorrect.
I see there is a 'virtio-input-host-device' for the 's390-ccw-virtio'
machine, but it is the only device prefixed with 'virtio-' that does not
have a -ccw counterpart.
Maybe (ab)use 'QEMU_CAPS_LAST' here as a capability that is never set?
break;
case VIR_DOMAIN_INPUT_TYPE_LAST:
default:
@@ -5777,6 +5767,15 @@ qemuDomainDeviceDefValidateInput(const virDomainInputDef *input,
return -1;
}
+ if (!virQEMUCapsGet(qemuCaps, cap) ||
+ (input->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
+ !virQEMUCapsGet(qemuCaps, ccwCap))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s is not supported by this QEMU binary"),
Consider '%s'
+ baseName);
+ return -1;
+ }
Also, the data itself might look nicer in an array:
struct qemuDomainVirtioInputCapsType {
const char *baseName;
int cap;
int ccwCap;
};
static const struct qemuDomainVirtioInputCapsType qemuDomainVirtioInputCaps[] = {
[VIR_DOMAIN_INPUT_TYPE_MOUSE] = {
"virtio-mouse",
QEMU_CAPS_VIRTIO_MOUSE,
QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW
},
[VIR_DOMAIN_INPUT_TYPE_TABLET] = {
"virtio-tablet",
QEMU_CAPS_VIRTIO_TABLET,
QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW,
},
[VIR_DOMAIN_INPUT_TYPE_KBD] = {
"virtio-keyboard",
QEMU_CAPS_VIRTIO_KEYBOARD,
QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW
},
[VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH] = {
"virtio-input-host",
QEMU_CAPS_VIRTIO_INPUT_HOST,
QEMU_CAPS_LAST
},
};
verify(ARRAY_CARDINALITY(qemuDomainVirtioInputCaps) == VIR_DOMAIN_INPUT_TYPE_LAST);
With missing virtio-input-host-ccw addressed:
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:22 a.m.
New subject: [libvirt] [PATCH v2 7/7] qemu: Unify generation of command line for virtio devices
A virtio device such as
<controller type='scsi' model='virtio-scsi'/>
will be translated to one of four different QEMU devices
based on the address type. This behavior is the same for
all virtio devices, but unfortunately we have separate
ad-hoc code dealing with each and every one of them: not
only this is pointless duplication, but it turns out
that most of that code is not robust against new address
types being introduced and some of it is outright buggy.
Introduce a new function, qemuBuildVirtioDevStr(), which
deals with the issue in a generic fashion, and rewrite
all existing code to use it.
This fixes a bunch of issues such as virtio-serial-pci
being used with virtio-mmio addresses and virtio-gpu
not being usable at all with virtio-mmio addresses.
It also introduces a couple of minor regressions,
namely no longer erroring out when attempting to
use virtio-balloon and virtio-input devices with
virtio-s390 addresses; that said, virtio-s390 has
been superseded by virtio-ccw such a long time ago
that recent QEMU releases have dropped support for
the former entirely, so re-implementing such
device-specific validation is not worth it.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 181 ++++++++++++++++++----------------------
1 file changed, 81 insertions(+), 100 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index db7c3ad698..041e4bd7fb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -397,6 +397,59 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
return ret;
}
+
+static int
+qemuBuildVirtioDevStr(virBufferPtr buf,
+ const char *baseName,
+ virDomainDeviceAddressType type)
+{
+ const char *implName = NULL;
+ int ret = -1;
+
+ switch (type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ implName = "pci";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
+ implName = "device";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
+ implName = "ccw";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+ implName = "s390";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unexpected address type for '%s'"),
baseName);
+ goto done;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainDeviceAddressType, type);
+ goto done;
+ }
+
+ virBufferAsprintf(buf, "%s-%s", baseName, implName);
+
+ ret = 0;
+
+ done:
+ return ret;
+}
+
+
static int
qemuBuildVirtioOptionsStr(virBufferPtr buf,
virDomainVirtioOptionsPtr virtio,
@@ -2002,17 +2055,8 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
break;
case VIR_DOMAIN_DISK_BUS_VIRTIO:
- if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- virBufferAddLit(&opt, "virtio-blk-ccw");
- } else if (disk->info.type ==
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
- virBufferAddLit(&opt, "virtio-blk-s390");
- } else if (disk->info.type ==
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) {
- virBufferAddLit(&opt, "virtio-blk-device");
- } else {
- virBufferAddLit(&opt, "virtio-blk-pci");
- }
+ if (qemuBuildVirtioDevStr(&opt, "virtio-blk", disk->info.type)
< 0)
+ goto error;
if (disk->iothread)
virBufferAsprintf(&opt, ",iothread=iothread%u",
disk->iothread);
@@ -2546,10 +2590,8 @@ qemuBuildFSDevStr(const virDomainDef *def,
goto error;
}
- if (fs->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
- virBufferAddLit(&opt, "virtio-9p-ccw");
- else
- virBufferAddLit(&opt, "virtio-9p-pci");
+ if (qemuBuildVirtioDevStr(&opt, "virtio-9p", fs->info.type) < 0)
+ goto error;
virBufferAsprintf(&opt, ",id=%s", fs->info.alias);
virBufferAsprintf(&opt, ",fsdev=%s%s",
@@ -2744,7 +2786,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
int *nusbcontroller)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- int address_type = def->info.type;
*devstr = NULL;
@@ -2752,19 +2793,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
switch ((virDomainControllerModelSCSI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
- switch (address_type) {
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
- virBufferAddLit(&buf, "virtio-scsi-ccw");
- break;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
- virBufferAddLit(&buf, "virtio-scsi-s390");
- break;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
- virBufferAddLit(&buf, "virtio-scsi-device");
- break;
- default:
- virBufferAddLit(&buf, "virtio-scsi-pci");
- }
+ if (qemuBuildVirtioDevStr(&buf, "virtio-scsi",
def->info.type) < 0)
+ goto error;
if (def->iothread) {
virBufferAsprintf(&buf, ",iothread=iothread%u",
@@ -2804,22 +2834,9 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
- switch (address_type) {
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
- virBufferAddLit(&buf, "virtio-serial-pci");
- break;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
- virBufferAddLit(&buf, "virtio-serial-ccw");
- break;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
- virBufferAddLit(&buf, "virtio-serial-s390");
- break;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
- virBufferAddLit(&buf, "virtio-serial-device");
- break;
- default:
- virBufferAddLit(&buf, "virtio-serial");
- }
+ if (qemuBuildVirtioDevStr(&buf, "virtio-serial", def->info.type)
< 0)
+ goto error;
+
virBufferAsprintf(&buf, ",id=%s", def->info.alias);
if (def->opts.vioserial.ports != -1) {
virBufferAsprintf(&buf, ",max_ports=%d",
@@ -3517,24 +3534,18 @@ qemuBuildNicDevStr(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- const char *nic = net->model;
bool usingVirtio = false;
char macaddr[VIR_MAC_STRING_BUFLEN];
if (STREQ(net->model, "virtio")) {
- if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
- nic = "virtio-net-ccw";
- else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
- nic = "virtio-net-s390";
- else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
- nic = "virtio-net-device";
- else
- nic = "virtio-net-pci";
+ if (qemuBuildVirtioDevStr(&buf, "virtio-net", net->info.type)
< 0)
+ goto error;
usingVirtio = true;
+ } else {
+ virBufferAddStr(&buf, net->model);
}
- virBufferAdd(&buf, nic, -1);
if (usingVirtio && net->driver.virtio.txmode) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TX_ALG)) {
virBufferAddLit(&buf, ",tx=");
@@ -3929,21 +3940,9 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd,
return -1;
}
- switch (def->memballoon->info.type) {
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
- virBufferAddLit(&buf, "virtio-balloon-pci");
- break;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
- virBufferAddLit(&buf, "virtio-balloon-ccw");
- break;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
- virBufferAddLit(&buf, "virtio-balloon-device");
- break;
- default:
- virReportError(VIR_ERR_XML_ERROR,
- _("memballoon unsupported with address type
'%s'"),
-
virDomainDeviceAddressTypeToString(def->memballoon->info.type));
- goto error;
+ if (qemuBuildVirtioDevStr(&buf, "virtio-balloon",
+ def->memballoon->info.type) < 0) {
+ goto error;
}
virBufferAsprintf(&buf, ",id=%s", def->memballoon->info.alias);
@@ -4040,33 +4039,23 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- const char *suffix;
-
- if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- suffix = "-pci";
- } else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- suffix = "-ccw";
- } else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) {
- suffix = "-device";
- } else {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported address type %s for virtio input
device"),
- virDomainDeviceAddressTypeToString(dev->info.type));
- goto error;
- }
switch ((virDomainInputType)dev->type) {
case VIR_DOMAIN_INPUT_TYPE_MOUSE:
- virBufferAsprintf(&buf, "virtio-mouse%s", suffix);
+ if (qemuBuildVirtioDevStr(&buf, "virtio-mouse", dev->info.type)
< 0)
+ goto error;
break;
case VIR_DOMAIN_INPUT_TYPE_TABLET:
- virBufferAsprintf(&buf, "virtio-tablet%s", suffix);
+ if (qemuBuildVirtioDevStr(&buf, "virtio-tablet", dev->info.type)
< 0)
+ goto error;
break;
case VIR_DOMAIN_INPUT_TYPE_KBD:
- virBufferAsprintf(&buf, "virtio-keyboard%s", suffix);
+ if (qemuBuildVirtioDevStr(&buf, "virtio-keyboard",
dev->info.type) < 0)
+ goto error;
break;
case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
- virBufferAsprintf(&buf, "virtio-input-host%s", suffix);
+ if (qemuBuildVirtioDevStr(&buf, "virtio-input-host",
dev->info.type) < 0)
+ goto error;
break;
case VIR_DOMAIN_INPUT_TYPE_LAST:
default:
@@ -4379,10 +4368,8 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
}
if (STREQ(model, "virtio-gpu")) {
- if (video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
- virBufferAsprintf(&buf, "%s-ccw", model);
- else
- virBufferAsprintf(&buf, "%s-pci", model);
+ if (qemuBuildVirtioDevStr(&buf, "virtio-gpu", video->info.type)
< 0)
+ goto error;
} else {
virBufferAsprintf(&buf, "%s", model);
}
@@ -5813,14 +5800,8 @@ qemuBuildRNGDevStr(const virDomainDef *def,
dev->source.file))
goto error;
- if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
- virBufferAddLit(&buf, "virtio-rng-ccw");
- else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
- virBufferAddLit(&buf, "virtio-rng-s390");
- else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
- virBufferAddLit(&buf, "virtio-rng-device");
- else
- virBufferAddLit(&buf, "virtio-rng-pci");
+ if (qemuBuildVirtioDevStr(&buf, "virtio-rng", dev->info.type) <
0)
+ goto error;
virBufferAsprintf(&buf, ",rng=obj%s,id=%s",
dev->info.alias, dev->info.alias);
--
2.17.1
8:15 a.m.
New subject: [libvirt] [PATCH v2 7/7] qemu: Unify generation of command line for virtio devices
On Thu, Sep 06, 2018 at 02:22:19PM +0200, Andrea Bolognani wrote:
A virtio device such as
<controller type='scsi' model='virtio-scsi'/>
will be translated to one of four different QEMU devices
based on the address type. This behavior is the same for
all virtio devices, but unfortunately we have separate
ad-hoc code dealing with each and every one of them: not
only this is pointless duplication, but it turns out
that most of that code is not robust against new address
types being introduced and some of it is outright buggy.
Introduce a new function, qemuBuildVirtioDevStr(), which
deals with the issue in a generic fashion, and rewrite
all existing code to use it.
This fixes a bunch of issues such as virtio-serial-pci
being used with virtio-mmio addresses and virtio-gpu
not being usable at all with virtio-mmio addresses.
It also introduces a couple of minor regressions,
namely no longer erroring out when attempting to
use virtio-balloon and virtio-input devices with
virtio-s390 addresses; that said, virtio-s390 has
been superseded by virtio-ccw such a long time ago
that recent QEMU releases have dropped support for
the former entirely,
I approve of such disregard for antique virtio.
so re-implementing such
device-specific validation is not worth it.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 181 ++++++++++++++++++----------------------
1 file changed, 81 insertions(+), 100 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index db7c3ad698..041e4bd7fb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -397,6 +397,59 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
return ret;
}
+
+static int
+qemuBuildVirtioDevStr(virBufferPtr buf,
+ const char *baseName,
+ virDomainDeviceAddressType type)
+{
+ const char *implName = NULL;
+ int ret = -1;
+
+ switch (type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ implName = "pci";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
+ implName = "device";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
+ implName = "ccw";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+ implName = "s390";
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unexpected address type for '%s'"),
baseName);
+ goto done;
Just return -1;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainDeviceAddressType, type);
+ goto done;
Here too.
It's unlikely we'll end up having to do something worth cleaning up
before we even validate the supplied parameters.
+ }
+
+ virBufferAsprintf(buf, "%s-%s", baseName, implName);
+
+ ret = 0;
+
+ done:
+ return ret;
Then this can be simply:
return 0;
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2269
days inactive
2269
days old
14 comments
2 participants
participants (2)
-
Andrea Bolognani -
Ján Tomko