[libvirt] [PATCHv2 00/15] Allow virtio devices to use vIOMMU
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 Ján Tomko (15): conf: eliminate monster condition in virDomainControllerDefFormat virDomainControllerDefFormat: do not mix attributes and subelements conf: add device_iotlb attribute to iommu qemu: format device-iotlb on intel-iommu command line qemuxml2xmltest: add virtio-options test Add virtio-related options to interfaces add virtio-related options to memballoon Add virtio-related options to disks Add virtio-related options to controllers Add virtio-related options to filesystems Add virtio-related options to rng devices Add virtio-related options to video Add virtio-related options to input devices qemuxml2argvtest: add virtio-options test case qemu: format virtio-related options on the command line docs/formatdomain.html.in | 87 ++++++++ docs/schemas/domaincommon.rng | 40 ++++ src/conf/domain_conf.c | 224 ++++++++++++++++++--- src/conf/domain_conf.h | 27 +++ src/qemu/qemu_capabilities.c | 15 +- src/qemu/qemu_capabilities.h | 5 + src/qemu/qemu_command.c | 69 +++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 3 + .../qemuxml2argv-intel-iommu-device-iotlb.args | 19 ++ .../qemuxml2argv-intel-iommu-device-iotlb.xml | 31 +++ .../qemuxml2argv-virtio-options.args | 57 ++++++ .../qemuxml2argv-virtio-options.xml | 104 ++++++++++ tests/qemuxml2argvtest.c | 18 ++ .../qemuxml2xmlout-intel-iommu-device-iotlb.xml | 1 + .../qemuxml2xmlout-virtio-options.xml | 104 ++++++++++ tests/qemuxml2xmltest.c | 2 + 16 files changed, 772 insertions(+), 34 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml -- 2.10.2
Move most of the subelement formatting out of the giant if. --- src/conf/domain_conf.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 958a5b7..02b2e49 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21473,11 +21473,7 @@ virDomainControllerDefFormat(virBufferPtr buf, break; } - if (pciModel || pciTarget || - def->queues || def->cmd_per_lun || def->max_sectors || def->ioeventfd || - def->iothread || - virDomainDeviceInfoNeedsFormat(&def->info, flags) || pcihole64) { - + if (pciModel || pciTarget) { if (pciModel) { modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); if (!modelName) { @@ -21514,17 +21510,17 @@ virDomainControllerDefFormat(virBufferPtr buf, virBufferAddLit(&childBuf, "</target>\n"); } } + } - virDomainControllerDriverFormat(&childBuf, def); + virDomainControllerDriverFormat(&childBuf, def); - if (virDomainDeviceInfoNeedsFormat(&def->info, flags) && - virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) - return -1; + if (virDomainDeviceInfoNeedsFormat(&def->info, flags) && + virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) + return -1; - if (pcihole64) { - virBufferAsprintf(&childBuf, "<pcihole64 unit='KiB'>%lu</" - "pcihole64>\n", def->opts.pciopts.pcihole64size); - } + if (pcihole64) { + virBufferAsprintf(&childBuf, "<pcihole64 unit='KiB'>%lu</" + "pcihole64>\n", def->opts.pciopts.pcihole64size); } if (virBufferUse(&childBuf)) { -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:15PM +0200, Ján Tomko wrote:
Move most of the subelement formatting out of the giant if. --- src/conf/domain_conf.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Move out the PCI controller's subelements formatting out of the switch handling attributes. This removes the need for a few bool variables. --- src/conf/domain_conf.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 02b2e49..e50628f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21409,7 +21409,6 @@ virDomainControllerDefFormat(virBufferPtr buf, const char *type = virDomainControllerTypeToString(def->type); const char *model = NULL; const char *modelName = NULL; - bool pcihole64 = false, pciModel = false, pciTarget = false; virBuffer childBuf = VIR_BUFFER_INITIALIZER; virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); @@ -21456,25 +21455,12 @@ virDomainControllerDefFormat(virBufferPtr buf, } break; - case VIR_DOMAIN_CONTROLLER_TYPE_PCI: - if (def->opts.pciopts.pcihole64) - pcihole64 = true; - if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) - pciModel = true; - if (def->opts.pciopts.chassisNr != -1 || - def->opts.pciopts.chassis != -1 || - def->opts.pciopts.port != -1 || - def->opts.pciopts.busNr != -1 || - def->opts.pciopts.numaNode != -1) - pciTarget = true; - break; - default: break; } - if (pciModel || pciTarget) { - if (pciModel) { + if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) { modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); if (!modelName) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -21485,7 +21471,11 @@ virDomainControllerDefFormat(virBufferPtr buf, virBufferAsprintf(&childBuf, "<model name='%s'/>\n", modelName); } - if (pciTarget) { + if (def->opts.pciopts.chassisNr != -1 || + def->opts.pciopts.chassis != -1 || + def->opts.pciopts.port != -1 || + def->opts.pciopts.busNr != -1 || + def->opts.pciopts.numaNode != -1) { virBufferAddLit(&childBuf, "<target"); if (def->opts.pciopts.chassisNr != -1) virBufferAsprintf(&childBuf, " chassisNr='%d'", @@ -21518,7 +21508,8 @@ virDomainControllerDefFormat(virBufferPtr buf, virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) return -1; - if (pcihole64) { + if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + def->opts.pciopts.pcihole64) { virBufferAsprintf(&childBuf, "<pcihole64 unit='KiB'>%lu</" "pcihole64>\n", def->opts.pciopts.pcihole64size); } -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:16PM +0200, Ján Tomko wrote:
Move out the PCI controller's subelements formatting out of the switch handling attributes. This removes the need for a few bool variables. --- src/conf/domain_conf.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add a new device_iotlb attribute to the iommu device to control the device IOTLB support for intel-iommu. https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 15 ++++++++++- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-device-iotlb.xml | 31 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-device-iotlb.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 07208ee..2f1e030 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7449,6 +7449,15 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 3.4.0</span> (QEMU/KVM only) </p> </dd> + <dt><code>device_iotlb</code></dt> + <dd> + <p> + The <code>device_iotlb</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on the device IOTLB descriptor. + <span class="since">Since 3.5.0</span> (QEMU/KVM only) + </p> + </dd> </dl> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4d9f8d1..6c3e885 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3964,6 +3964,11 @@ <ref name="virOnOff"/> </attribute> </optional> + <optional> + <attribute name="device_iotlb"> + <ref name="virOnOff"/> + </attribute> + </optional> </element> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e50628f..89c8917 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14208,6 +14208,14 @@ virDomainIOMMUDefParseXML(xmlNodePtr node, } iommu->caching_mode = val; } + VIR_FREE(tmp); + if ((tmp = virXPathString("string(./driver/@device_iotlb)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown device_iotlb value: %s"), tmp); + goto cleanup; + } + iommu->device_iotlb = val; + } VIR_FREE(tmp); if ((tmp = virXPathString("string(./driver/@eim)", ctxt))) { @@ -24258,7 +24266,8 @@ virDomainIOMMUDefFormat(virBufferPtr buf, virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT || - iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT) { + iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT || + iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT) { virBufferAddLit(&childBuf, "<driver"); if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { virBufferAsprintf(&childBuf, " intremap='%s'", @@ -24272,6 +24281,10 @@ virDomainIOMMUDefFormat(virBufferPtr buf, virBufferAsprintf(&childBuf, " eim='%s'", virTristateSwitchTypeToString(iommu->eim)); } + if (iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, " device_iotlb='%s'", + virTristateSwitchTypeToString(iommu->device_iotlb)); + } virBufferAddLit(&childBuf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 446b117..7d1f05c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2212,6 +2212,7 @@ struct _virDomainIOMMUDef { virTristateSwitch intremap; virTristateSwitch caching_mode; virTristateSwitch eim; + virTristateSwitch device_iotlb; }; /* * Guest VM main configuration diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml new file mode 100644 index 0000000..0cdf2aa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml @@ -0,0 +1,31 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <ioapic driver='qemu'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <iommu model='intel'> + <driver intremap='on' device_iotlb='on'/> + </iommu> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml new file mode 120000 index 0000000..3120d9f --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index fff13e2..5e8cead 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1128,6 +1128,7 @@ mymain(void) DO_TEST("intel-iommu-ioapic", NONE); DO_TEST("intel-iommu-caching-mode", NONE); DO_TEST("intel-iommu-eim", NONE); + DO_TEST("intel-iommu-device-iotlb", NONE); DO_TEST("cpu-check-none", NONE); DO_TEST("cpu-check-partial", NONE); -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:17PM +0200, Ján Tomko wrote:
Add a new device_iotlb attribute to the iommu device to control the device IOTLB support for intel-iommu.
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 9 +++++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 15 ++++++++++- src/conf/domain_conf.h | 1 + .../qemuxml2argv-intel-iommu-device-iotlb.xml | 31 ++++++++++++++++++++++ .../qemuxml2xmlout-intel-iommu-device-iotlb.xml | 1 + tests/qemuxml2xmltest.c | 1 + 7 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-intel-iommu-device-iotlb.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 07208ee..2f1e030 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7449,6 +7449,15 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 3.4.0</span> (QEMU/KVM only) </p> </dd> + <dt><code>device_iotlb</code></dt> + <dd> + <p> + The <code>device_iotlb</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on the device IOTLB descriptor. + <span class="since">Since 3.5.0</span> (QEMU/KVM only)
How about we use just "iotlb"? The IOTLB is used to store address translation requests for devices. The device_iotlb is kind of misleading and we don't have to follow QEMU naming. I would probably change the "... device IOTLB descriptor." to something that indicates what the IOTLB is for. For example "... IOTLB used to cache address translation requests from devices." The code itself is good. Pavel
Format the device-iotlb attribute. https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_command.c | 11 +++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-device-iotlb.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++++ 6 files changed, 44 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 03c5585..cc78eb9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -372,6 +372,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "intel-iommu.intremap", "intel-iommu.caching-mode", "intel-iommu.eim", + + "intel-iommu.device-iotlb", /* 260 */ ); @@ -1730,6 +1732,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIntelIOMMU[] = { { "intremap", QEMU_CAPS_INTEL_IOMMU_INTREMAP }, { "caching-mode", QEMU_CAPS_INTEL_IOMMU_CACHING_MODE }, { "eim", QEMU_CAPS_INTEL_IOMMU_EIM }, + { "device-iotlb", QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB }, }; /* see documentation for virQEMUCapsQMPSchemaGetByPath for the query format */ diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index eba9814..275bbcf 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -411,6 +411,9 @@ typedef enum { QEMU_CAPS_INTEL_IOMMU_CACHING_MODE, /* intel-iommu.caching-mode */ QEMU_CAPS_INTEL_IOMMU_EIM, /* intel-iommu.eim */ + /* 260 */ + QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB, /* intel-iommu.device-iotlb */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 015af10..1f54ba0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6697,6 +6697,13 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, "with this QEMU binary")); return -1; } + if (iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu: device IOTLB is not supported " + "with this QEMU binary")); + return -1; + } break; case VIR_DOMAIN_IOMMU_MODEL_LAST: break; @@ -6734,6 +6741,10 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virBufferAsprintf(&opts, ",eim=%s", virTristateSwitchTypeToString(iommu->eim)); } + if (iommu->device_iotlb != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&opts, ",device-iotlb=%s", + virTristateSwitchTypeToString(iommu->device_iotlb)); + } case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 95b04dd..d51ee46 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -215,6 +215,7 @@ <flag name='intel-iommu.intremap'/> <flag name='intel-iommu.caching-mode'/> <flag name='intel-iommu.eim'/> + <flag name='intel-iommu.device-iotlb'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args new file mode 100644 index 0000000..6d8f8e2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args @@ -0,0 +1,19 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-machine q35,accel=kvm,kernel_irqchip=split \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device intel-iommu,intremap=on,device-iotlb=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b7d7cc2..455caa4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2536,6 +2536,13 @@ mymain(void) QEMU_CAPS_INTEL_IOMMU_INTREMAP, QEMU_CAPS_INTEL_IOMMU_EIM, QEMU_CAPS_DEVICE_INTEL_IOMMU); + DO_TEST("intel-iommu-device-iotlb", + QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_MACHINE_KERNEL_IRQCHIP, + QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT, + QEMU_CAPS_INTEL_IOMMU_INTREMAP, + QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB, + QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:18PM +0200, Ján Tomko wrote:
Format the device-iotlb attribute.
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 3 +++ src/qemu/qemu_command.c | 11 +++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-intel-iommu-device-iotlb.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++++ 6 files changed, 44 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-device-iotlb.args
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add a test case with all the virtio devices we know to demonstrate the addition of new options. https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- .../qemuxml2argv-virtio-options.xml | 92 ++++++++++++++++++++++ .../qemuxml2xmlout-virtio-options.xml | 92 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 185 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml new file mode 100644 index 0000000..9ead938 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -0,0 +1,92 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' 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-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img1'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img2'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/export/fs1'/> + <target dir='fs1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </filesystem> + <filesystem type='mount' accessmode='mapped'> + <driver type='path' wrpolicy='immediate'/> + <source dir='/export/fs2'/> + <target dir='fs2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </filesystem> + <interface type='user'> + <mac address='52:54:56:58:5a:5c'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </interface> + <interface type='user'> + <mac address='52:54:56:5a:5c:5e'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </interface> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + </input> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'> + <acceleration accel3d='yes'/> + </model> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </memballoon> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml new file mode 100644 index 0000000..9ead938 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -0,0 +1,92 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' 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-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img1'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img2'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/export/fs1'/> + <target dir='fs1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </filesystem> + <filesystem type='mount' accessmode='mapped'> + <driver type='path' wrpolicy='immediate'/> + <source dir='/export/fs2'/> + <target dir='fs2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </filesystem> + <interface type='user'> + <mac address='52:54:56:58:5a:5c'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </interface> + <interface type='user'> + <mac address='52:54:56:5a:5c:5e'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </interface> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + </input> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'> + <acceleration accel3d='yes'/> + </model> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </memballoon> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5e8cead..08698b1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1106,6 +1106,7 @@ mymain(void) DO_TEST("memorybacking-set", NONE); DO_TEST("memorybacking-unset", NONE); + DO_TEST("virtio-options", QEMU_CAPS_VIRTIO_SCSI); virObjectUnref(cfg); -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:19PM +0200, Ján Tomko wrote:
Add a test case with all the virtio devices we know to demonstrate the addition of new options.
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- .../qemuxml2argv-virtio-options.xml | 92 ++++++++++++++++++++++ .../qemuxml2xmlout-virtio-options.xml | 92 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 185 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
Some of the devices has two entries and some has only one entry. Based on the following patches the two entries are to demonstrate in tests that setting it "on" works and setting it "off" works but this is not tested for all devices. I would either split it into two patches where one would test "on" and second one "off" or just test the "on" part and have only one patch. Pavel
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu_platform='on' ats='on'/> </interface> https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 19 +++++++ docs/schemas/domaincommon.rng | 12 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++ src/conf/domain_conf.h | 19 +++++++ .../qemuxml2argv-virtio-options.xml | 2 + .../qemuxml2xmlout-virtio-options.xml | 2 + 6 files changed, 117 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 2f1e030..dcc2e5e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3451,6 +3451,19 @@ </dd> </dl> + <h4><a name="elementsVirtio">Virtio-related options</a></h4> + + <p> + QEMU's virtio devices have some attributes related to the virtio transport under + the <code>driver</code> element: + The <code>iommu_platform</code> attribute enables the use of emulated IOMMU + by the device. The attribute <code>ats</code> controls the Address + Translation Service support for PCIe devices. This is needed to make use + of IOTLB support (see <a href="#elementsIommu">IOMMU device</a>). + Possible values are <code>on</code> or <code>off</code>. + <span class="since">Since 3.5.0</span> + </p> + <h4><a name="elementsControllers">Controllers</a></h4> <p> @@ -5142,6 +5155,12 @@ qemu-kvm -net nic,model=? /dev/null <b>In general you should leave this option alone, unless you are very certain you know what you are doing.</b> </dd> + <dt>virtio options</dt> + <dd> + For virtio interfaces, + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </dd> </dl> <p> Offloading options for the host and guest can be configured using diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6c3e885..d7f3b02 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2686,6 +2686,7 @@ </optional> </group> </choice> + <ref name="virtioOptions"/> <interleave> <optional> <element name='host'> @@ -5006,6 +5007,17 @@ </element> </define> + <define name="virtioOptions"> + <optional> + <attribute name="iommu_platform"> + <ref name="virOnOff"/> + </attribute> + <attribute name="ats"> + <ref name="virOnOff"/> + </attribute> + </optional> + </define> + <define name="usbmaster"> <element name="master"> <attribute name="startport"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89c8917..ef3383d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1105,6 +1105,46 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) return &xmlopt->ns; } +static int +virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt, + virDomainVirtioOptionsPtr *virtio) +{ + char *str = NULL; + int ret = -1; + int val; + virDomainVirtioOptionsPtr res; + + if (VIR_ALLOC(*virtio) < 0) + return -1; + + res = *virtio; + + if ((str = virXPathString("string(./driver/@iommu_platform)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid iommu_platform value")); + goto cleanup; + } + res->iommu_platform = val; + } + VIR_FREE(str); + + if ((str = virXPathString("string(./driver/@ats)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid ats value")); + goto cleanup; + } + res->ats = val; + } + + ret = 0; + + cleanup: + VIR_FREE(str); + return ret; +} + void virBlkioDeviceArrayClear(virBlkioDevicePtr devices, @@ -1952,6 +1992,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) VIR_FREE(def->ifname); VIR_FREE(def->ifname_guest); VIR_FREE(def->ifname_guest_actual); + VIR_FREE(def->virtio); virNetDevIPInfoClear(&def->guestIP); virNetDevIPInfoClear(&def->hostIP); @@ -5221,6 +5262,24 @@ virDomainDefValidate(virDomainDefPtr def, } +static void +virDomainVirtioOptionsFormat(virBufferPtr buf, + virDomainVirtioOptionsPtr virtio) +{ + if (!virtio) + return; + + if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, "iommu_platform='%s' ", + virTristateSwitchTypeToString(virtio->iommu_platform)); + } + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, "ats='%s' ", + virTristateSwitchTypeToString(virtio->ats)); + } +} + + /* Generate a string representation of a device address * @info address Device address to stringify */ @@ -10367,6 +10426,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: ctxt->node = oldnode; VIR_FREE(macaddr); @@ -22104,6 +22166,7 @@ virDomainVirtioNetDriverFormat(char **outstr, virBufferAsprintf(&buf, "rx_queue_size='%u' ", def->driver.virtio.rx_queue_size); + virDomainVirtioOptionsFormat(&buf, def->virtio); virBufferTrim(&buf, " ", -1); if (virBufferCheckError(&buf) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7d1f05c..a17f217 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -155,6 +155,17 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr; +typedef struct _virDomainVirtioOptions virDomainVirtioOptions; +typedef virDomainVirtioOptions *virDomainVirtioOptionsPtr; + +typedef enum { + VIR_DOMAIN_DRIVER_COMPATIBILITY_DEFAULT, + VIR_DOMAIN_DRIVER_COMPATIBILITY_LEGACY, + VIR_DOMAIN_DRIVER_COMPATIBILITY_TRANSITIONAL, + VIR_DOMAIN_DRIVER_COMPATIBILITY_MODERN, + VIR_DOMAIN_DRIVER_COMPATIBILITY_LAST, +} virDomainDriverCompatibility; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -1039,6 +1050,7 @@ struct _virDomainNetDef { int linkstate; unsigned int mtu; virNetDevCoalescePtr coalesce; + virDomainVirtioOptionsPtr virtio; }; typedef enum { @@ -2214,6 +2226,12 @@ struct _virDomainIOMMUDef { virTristateSwitch eim; virTristateSwitch device_iotlb; }; + +struct _virDomainVirtioOptions { + virTristateSwitch iommu_platform; + virTristateSwitch ats; +}; + /* * Guest VM main configuration * @@ -3186,6 +3204,7 @@ VIR_ENUM_DECL(virDomainMemorySource) VIR_ENUM_DECL(virDomainMemoryAllocation) VIR_ENUM_DECL(virDomainIOMMUModel) VIR_ENUM_DECL(virDomainShmemModel) +VIR_ENUM_DECL(virDomainDriverCompatibility) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index 9ead938..a2bbbee 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -53,11 +53,13 @@ <interface type='user'> <mac address='52:54:56:58:5a:5c'/> <model type='virtio'/> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </interface> <interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> + <driver iommu_platform='off' ats='off'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <input type='mouse' bus='virtio'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index 9ead938..a2bbbee 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -53,11 +53,13 @@ <interface type='user'> <mac address='52:54:56:58:5a:5c'/> <model type='virtio'/> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </interface> <interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> + <driver iommu_platform='off' ats='off'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <input type='mouse' bus='virtio'> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:20PM +0200, Ján Tomko wrote:
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu_platform='on' ats='on'/> </interface>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 19 +++++++ docs/schemas/domaincommon.rng | 12 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++ src/conf/domain_conf.h | 19 +++++++ .../qemuxml2argv-virtio-options.xml | 2 + .../qemuxml2xmlout-virtio-options.xml | 2 + 6 files changed, 117 insertions(+)
I would spit this patch into two patches, one that introduces the virtio related options and second one that introduces this element to the interfaces.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 2f1e030..dcc2e5e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3451,6 +3451,19 @@ </dd> </dl>
+ <h4><a name="elementsVirtio">Virtio-related options</a></h4> + + <p> + QEMU's virtio devices have some attributes related to the virtio transport under + the <code>driver</code> element: + The <code>iommu_platform</code> attribute enables the use of emulated IOMMU
I think that we tend to use camelCase for attributes and elements. Isn't "iommuEnabled" or just "iommu", we don't need to follow QEMU naming, it should be something generic.
+ by the device. The attribute <code>ats</code> controls the Address + Translation Service support for PCIe devices. This is needed to make use + of IOTLB support (see <a href="#elementsIommu">IOMMU device</a>). + Possible values are <code>on</code> or <code>off</code>. + <span class="since">Since 3.5.0</span> + </p> + <h4><a name="elementsControllers">Controllers</a></h4>
<p> @@ -5142,6 +5155,12 @@ qemu-kvm -net nic,model=? /dev/null <b>In general you should leave this option alone, unless you are very certain you know what you are doing.</b> </dd> + <dt>virtio options</dt> + <dd> + For virtio interfaces, + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </dd> </dl> <p> Offloading options for the host and guest can be configured using diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6c3e885..d7f3b02 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2686,6 +2686,7 @@ </optional> </group> </choice> + <ref name="virtioOptions"/> <interleave> <optional> <element name='host'> @@ -5006,6 +5007,17 @@ </element> </define>
+ <define name="virtioOptions"> + <optional> + <attribute name="iommu_platform"> + <ref name="virOnOff"/> + </attribute> + <attribute name="ats"> + <ref name="virOnOff"/> + </attribute> + </optional> + </define> + <define name="usbmaster"> <element name="master"> <attribute name="startport"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89c8917..ef3383d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1105,6 +1105,46 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) return &xmlopt->ns; }
+static int +virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt, + virDomainVirtioOptionsPtr *virtio) +{ + char *str = NULL; + int ret = -1; + int val; + virDomainVirtioOptionsPtr res; + + if (VIR_ALLOC(*virtio) < 0) + return -1; + + res = *virtio; + + if ((str = virXPathString("string(./driver/@iommu_platform)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid iommu_platform value")); + goto cleanup; + } + res->iommu_platform = val; + } + VIR_FREE(str); + + if ((str = virXPathString("string(./driver/@ats)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid ats value")); + goto cleanup; + } + res->ats = val; + } + + ret = 0; + + cleanup: + VIR_FREE(str); + return ret; +} +
void virBlkioDeviceArrayClear(virBlkioDevicePtr devices, @@ -1952,6 +1992,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) VIR_FREE(def->ifname); VIR_FREE(def->ifname_guest); VIR_FREE(def->ifname_guest_actual); + VIR_FREE(def->virtio);
virNetDevIPInfoClear(&def->guestIP); virNetDevIPInfoClear(&def->hostIP); @@ -5221,6 +5262,24 @@ virDomainDefValidate(virDomainDefPtr def, }
+static void +virDomainVirtioOptionsFormat(virBufferPtr buf, + virDomainVirtioOptionsPtr virtio) +{ + if (!virtio) + return; + + if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, "iommu_platform='%s' ", + virTristateSwitchTypeToString(virtio->iommu_platform)); + } + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, "ats='%s' ", + virTristateSwitchTypeToString(virtio->ats)); + } +} + + /* Generate a string representation of a device address * @info address Device address to stringify */ @@ -10367,6 +10426,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
+ if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: ctxt->node = oldnode; VIR_FREE(macaddr); @@ -22104,6 +22166,7 @@ virDomainVirtioNetDriverFormat(char **outstr, virBufferAsprintf(&buf, "rx_queue_size='%u' ", def->driver.virtio.rx_queue_size);
+ virDomainVirtioOptionsFormat(&buf, def->virtio); virBufferTrim(&buf, " ", -1);
if (virBufferCheckError(&buf) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7d1f05c..a17f217 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -155,6 +155,17 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
+typedef struct _virDomainVirtioOptions virDomainVirtioOptions; +typedef virDomainVirtioOptions *virDomainVirtioOptionsPtr; + +typedef enum { + VIR_DOMAIN_DRIVER_COMPATIBILITY_DEFAULT, + VIR_DOMAIN_DRIVER_COMPATIBILITY_LEGACY, + VIR_DOMAIN_DRIVER_COMPATIBILITY_TRANSITIONAL, + VIR_DOMAIN_DRIVER_COMPATIBILITY_MODERN, + VIR_DOMAIN_DRIVER_COMPATIBILITY_LAST, +} virDomainDriverCompatibility;
This isn't used anywhere in the code and in the remaining patches, isn't it just some leftover? Pavel
On Wed, Jun 07, 2017 at 03:43:53PM +0200, Pavel Hrdina wrote:
On Tue, Jun 06, 2017 at 01:36:20PM +0200, Ján Tomko wrote:
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu_platform='on' ats='on'/> </interface>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 19 +++++++ docs/schemas/domaincommon.rng | 12 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++ src/conf/domain_conf.h | 19 +++++++ .../qemuxml2argv-virtio-options.xml | 2 + .../qemuxml2xmlout-virtio-options.xml | 2 + 6 files changed, 117 insertions(+)
I would spit this patch into two patches, one that introduces the virtio related options and second one that introduces this element to the interfaces.
That can be done for the schema and documentation changes, but code with an unused static function would not compile.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 2f1e030..dcc2e5e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3451,6 +3451,19 @@ </dd> </dl>
+ <h4><a name="elementsVirtio">Virtio-related options</a></h4> + + <p> + QEMU's virtio devices have some attributes related to the virtio transport under + the <code>driver</code> element: + The <code>iommu_platform</code> attribute enables the use of emulated IOMMU
I think that we tend to use camelCase for attributes and elements. Isn't "iommuEnabled" or just "iommu", we don't need to follow QEMU naming, it should be something generic.
Right, iommu is better than arguing about the aesthetics of camel case. Jan
On Wed, Jun 07, 2017 at 03:56:44PM +0200, Ján Tomko wrote:
On Wed, Jun 07, 2017 at 03:43:53PM +0200, Pavel Hrdina wrote:
On Tue, Jun 06, 2017 at 01:36:20PM +0200, Ján Tomko wrote:
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu_platform='on' ats='on'/> </interface>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 19 +++++++ docs/schemas/domaincommon.rng | 12 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++ src/conf/domain_conf.h | 19 +++++++ .../qemuxml2argv-virtio-options.xml | 2 + .../qemuxml2xmlout-virtio-options.xml | 2 + 6 files changed, 117 insertions(+)
I would spit this patch into two patches, one that introduces the virtio related options and second one that introduces this element to the interfaces.
That can be done for the schema and documentation changes, but code with an unused static function would not compile.
Oh, right, I didn't realize that, so ignore that comment :). Pavel
On Tue, Jun 06, 2017 at 01:36:20PM +0200, Ján Tomko wrote:
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu_platform='on' ats='on'/> </interface>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 19 +++++++ docs/schemas/domaincommon.rng | 12 +++++ src/conf/domain_conf.c | 63 ++++++++++++++++++++++ src/conf/domain_conf.h | 19 +++++++ .../qemuxml2argv-virtio-options.xml | 2 + .../qemuxml2xmlout-virtio-options.xml | 2 + 6 files changed, 117 insertions(+)
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89c8917..ef3383d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[...]
@@ -5221,6 +5262,24 @@ virDomainDefValidate(virDomainDefPtr def, }
+static void +virDomainVirtioOptionsFormat(virBufferPtr buf, + virDomainVirtioOptionsPtr virtio) +{ + if (!virtio) + return; + + if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, "iommu_platform='%s' ", + virTristateSwitchTypeToString(virtio->iommu_platform)); + } + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, "ats='%s' ", + virTristateSwitchTypeToString(virtio->ats)); + } +}
I don't like that we need to call virBufferTrim() every time this function is used. Let's switch the format string to " ats='%s'" and drop all the virBufferTrim() calls from following patches. It's pointless to add the space in order to remove it right away. Some of the calls of virDomainVirtioOptionsFormat() also requires to call virBufferAddLit(..., " "), it feels wrong. The only place that could conflict whit this approach is in virDomainVirtioNetDriverFormat() where you can easily call virDomainVirtioOptionsFormat() after the virBufferTrim(). Pavel
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 21 +++++++++++++++++++++ src/conf/domain_conf.h | 1 + .../qemuxml2argv-virtio-options.xml | 1 + .../qemuxml2xmlout-virtio-options.xml | 1 + 6 files changed, 36 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dcc2e5e..c286f50 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6916,6 +6916,7 @@ qemu-kvm -net nic,model=? /dev/null <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> <stats period='10'/> + <driver iommu_platform='on' ats='on'/> </memballoon> </devices> </domain></pre> @@ -6960,6 +6961,12 @@ qemu-kvm -net nic,model=? /dev/null <span class='since'>Since 1.1.1, requires QEMU 1.5</span> </p> </dd> + <dt><code>driver</code></dt> + <dd> + For model <code>virtio</code> memballoon, + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </dd> </dl> <h4><a name="elementsRng">Random number generator device</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d7f3b02..5b424c7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3784,6 +3784,11 @@ </attribute> </element> </optional> + <optional> + <element name="driver"> + <ref name="virtioOptions"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef3383d..7744563 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2291,6 +2291,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) return; virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -12954,6 +12955,9 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: VIR_FREE(model); VIR_FREE(deflate); @@ -22908,6 +22912,23 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return -1; } + if (def->virtio) { + virBuffer driverBuf = VIR_BUFFER_INITIALIZER; + + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + virBufferTrim(&driverBuf, " ", -1); + + if (virBufferCheckError(&driverBuf) < 0) { + virBufferFreeAndReset(&childrenBuf); + return -1; + } + if (virBufferUse(&driverBuf)) { + virBufferAddLit(&childrenBuf, "<driver "); + virBufferAddBuffer(&childrenBuf, &driverBuf); + virBufferAddLit(&childrenBuf, "/>\n"); + } + } + if (!virBufferUse(&childrenBuf)) { virBufferAddLit(buf, "/>\n"); } else { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a17f217..a3bb2cf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1616,6 +1616,7 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; int period; /* seconds between collections */ int autodeflate; /* enum virTristateSwitch */ + virDomainVirtioOptionsPtr virtio; }; struct _virDomainNVRAMDef { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index a2bbbee..96ec700 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -85,6 +85,7 @@ </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + <driver iommu_platform='on' ats='on'/> </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index a2bbbee..96ec700 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -85,6 +85,7 @@ </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + <driver iommu_platform='on' ats='on'/> </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:21PM +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 21 +++++++++++++++++++++ src/conf/domain_conf.h | 1 + .../qemuxml2argv-virtio-options.xml | 1 + .../qemuxml2xmlout-virtio-options.xml | 1 + 6 files changed, 36 insertions(+)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 4 ++-- 6 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c286f50..6852f49 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3011,6 +3011,11 @@ <code>bus</code> and "pci" or "ccw" <code>address</code> types. <span class='since'>Since 1.2.8 (QEMU 2.1)</span> </li> + <li> + For virtio disks, + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </li> </ul> </dd> <dt><code>backenddomain</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5b424c7..a77a04f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1770,6 +1770,7 @@ <optional> <ref name="detect_zeroes"/> </optional> + <ref name="virtioOptions"/> <empty/> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7744563..92bd1da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1731,6 +1731,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->product); VIR_FREE(def->domain_name); VIR_FREE(def->blkdeviotune.group_name); + VIR_FREE(def->virtio); virDomainDeviceInfoClear(&def->info); virObjectUnref(def->privateData); @@ -8404,6 +8405,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + /* Disk volume types will have authentication information handled in * virStorageTranslateDiskSourcePool */ @@ -21285,6 +21289,11 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(&driverBuf, " iothread='%u'", def->iothread); if (def->detect_zeroes) virBufferAsprintf(&driverBuf, " detect_zeroes='%s'", detect_zeroes); + + virBufferAddLit(&driverBuf, " "); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + virBufferTrim(&driverBuf, " ", -1); + if (virBufferUse(&driverBuf)) { virBufferAddLit(buf, "<driver"); virBufferAddBuffer(buf, &driverBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a3bb2cf..b43ff3e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -675,6 +675,7 @@ struct _virDomainDiskDef { unsigned int iothread; /* unused = 0, > 0 specific thread # */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ char *domain_name; /* backend domain name */ + virDomainVirtioOptionsPtr virtio; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index 96ec700..25a524a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -15,13 +15,13 @@ <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> + <driver name='qemu' type='raw' iommu_platform='on' ats='on'/> <source file='/var/lib/libvirt/images/img1'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </disk> <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> + <driver name='qemu' type='raw' iommu_platform='off' ats='off'/> <source file='/var/lib/libvirt/images/img2'/> <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index 96ec700..25a524a 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -15,13 +15,13 @@ <devices> <emulator>/usr/bin/qemu-system-x86_64</emulator> <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> + <driver name='qemu' type='raw' iommu_platform='on' ats='on'/> <source file='/var/lib/libvirt/images/img1'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </disk> <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> + <driver name='qemu' type='raw' iommu_platform='off' ats='off'/> <source file='/var/lib/libvirt/images/img2'/> <target dev='vdb' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:22PM +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 9 +++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 4 ++-- 6 files changed, 20 insertions(+), 4 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 2 ++ tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 2 ++ 6 files changed, 20 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6852f49..86e0bf8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3600,6 +3600,12 @@ <code>iothread</code> value. The <code>iothread</code> value must be within the range 1 to the domain iothreads value. </dd> + <dt>virtio options</dt> + <dd> + For virtio controllers, + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </dd> </dl> <p> USB companion controllers have an optional diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a77a04f..01c35ae 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2073,6 +2073,7 @@ <optional> <ref name="driverIOThread"/> </optional> + <ref name="virtioOptions"/> </element> </optional> </interleave> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 92bd1da..a794595 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1872,6 +1872,7 @@ void virDomainControllerDefFree(virDomainControllerDefPtr def) return; virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -9017,6 +9018,9 @@ virDomainControllerDefParseXML(xmlNodePtr node, cur = cur->next; } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + /* node is parsed differently from target attributes because * someone thought it should be a subelement instead... */ @@ -21476,6 +21480,10 @@ virDomainControllerDriverFormat(virBufferPtr buf, if (def->iothread) virBufferAsprintf(&driverBuf, " iothread='%u'", def->iothread); + virBufferAddLit(&driverBuf, " "); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + virBufferTrim(&driverBuf, " ", -1); + if (virBufferUse(&driverBuf)) { virBufferAddLit(buf, "<driver"); virBufferAddBuffer(buf, &driverBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b43ff3e..0f57d76 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -820,6 +820,7 @@ struct _virDomainControllerDef { virDomainUSBControllerOpts usbopts; } opts; virDomainDeviceInfo info; + virDomainVirtioOptionsPtr virtio; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index 25a524a..dd84ecd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -33,10 +33,12 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='scsi' index='0' model='virtio-scsi'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </controller> <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> <filesystem type='mount' accessmode='passthrough'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index 25a524a..dd84ecd 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -33,10 +33,12 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='scsi' index='0' model='virtio-scsi'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> </controller> <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> <filesystem type='mount' accessmode='passthrough'> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:23PM +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 2 ++ tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 2 ++ 6 files changed, 20 insertions(+)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 3 ++- tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 3 ++- 6 files changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 86e0bf8..ef30079 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3296,6 +3296,11 @@ or "handle", but no formats. Virtuozzo driver supports a type of "ploop" with a format of "ploop". </li> + <li> + For virtio-backed devices, + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </li> </ul> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 01c35ae..3608f84 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2256,6 +2256,7 @@ <value>immediate</value> </attribute> </optional> + <ref name='virtioOptions'/> <empty/> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a794595..e1b785a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1904,6 +1904,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def) virStorageSourceFree(def->src); VIR_FREE(def->dst); virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -9431,6 +9432,9 @@ virDomainFSDefParseXML(xmlNodePtr node, goto error; } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + def->src->path = source; source = NULL; def->dst = target; @@ -21674,6 +21678,10 @@ virDomainFSDefFormat(virBufferPtr buf, } + virBufferAddLit(&driverBuf, " "); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + virBufferTrim(&driverBuf, " ", -1); + if (virBufferUse(&driverBuf)) { virBufferAddLit(buf, "<driver"); virBufferAddBuffer(buf, &driverBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0f57d76..b9ca8a0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -883,6 +883,7 @@ struct _virDomainFSDef { unsigned long long space_hard_limit; /* in bytes */ unsigned long long space_soft_limit; /* in bytes */ bool symlinksResolved; + virDomainVirtioOptionsPtr virtio; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index dd84ecd..4fc17d2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -42,12 +42,13 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> <filesystem type='mount' accessmode='passthrough'> + <driver iommu_platform='on' ats='on'/> <source dir='/export/fs1'/> <target dir='fs1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </filesystem> <filesystem type='mount' accessmode='mapped'> - <driver type='path' wrpolicy='immediate'/> + <driver type='path' wrpolicy='immediate' iommu_platform='on' ats='on'/> <source dir='/export/fs2'/> <target dir='fs2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index dd84ecd..4fc17d2 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -42,12 +42,13 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> <filesystem type='mount' accessmode='passthrough'> + <driver iommu_platform='on' ats='on'/> <source dir='/export/fs1'/> <target dir='fs1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </filesystem> <filesystem type='mount' accessmode='mapped'> - <driver type='path' wrpolicy='immediate'/> + <driver type='path' wrpolicy='immediate' iommu_platform='on' ats='on'/> <source dir='/export/fs2'/> <target dir='fs2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:24PM +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 3 ++- tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 3 ++- 6 files changed, 19 insertions(+), 2 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 11 +++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 16 ++++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 1 + .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 1 + 6 files changed, 35 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ef30079..a546cbb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7064,6 +7064,17 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> </dd> + <dt><code>driver</code></dt> + <dd> + The subelement <code>driver</code> can be used to tune the device: + <dl> + <dt>virtio options</dt> + <dd> + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </dd> + </dl> + </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3608f84..3909a56 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4966,6 +4966,11 @@ <interleave> <ref name="rng-backend"/> <optional> + <element name="driver"> + <ref name="virtioOptions"/> + </element> + </optional> + <optional> <ref name="rng-rate"/> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e1b785a..f92fecc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12902,6 +12902,9 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: VIR_FREE(model); VIR_FREE(backend); @@ -23093,6 +23096,7 @@ virDomainRNGDefFormat(virBufferPtr buf, { const char *model = virDomainRNGModelTypeToString(def->model); const char *backend = virDomainRNGBackendTypeToString(def->backend); + virBuffer driverBuf = VIR_BUFFER_INITIALIZER; virBufferAsprintf(buf, "<rng model='%s'>\n", model); virBufferAdjustIndent(buf, 2); @@ -23121,6 +23125,17 @@ virDomainRNGDefFormat(virBufferPtr buf, break; } + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferCheckError(&driverBuf) < 0) + return -1; + + if (virBufferUse(&driverBuf)) { + virBufferTrim(&driverBuf, " ", -1); + virBufferAddLit(buf, "<driver "); + virBufferAddBuffer(buf, &driverBuf); + virBufferAddLit(buf, "/>\n"); + } + if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) { if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; @@ -23149,6 +23164,7 @@ virDomainRNGDefFree(virDomainRNGDefPtr def) } virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->virtio); VIR_FREE(def); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9ca8a0..f1887ac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2027,6 +2027,7 @@ struct _virDomainRNGDef { } source; virDomainDeviceInfo info; + virDomainVirtioOptionsPtr virtio; }; typedef enum { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index 4fc17d2..e852894 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -92,6 +92,7 @@ </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> </rng> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index 4fc17d2..e852894 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -92,6 +92,7 @@ </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> </rng> </devices> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:25PM +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 11 +++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 16 ++++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 1 + .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 1 + 6 files changed, 35 insertions(+)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 12 +++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 24 ++++++++++++++++++++-- src/conf/domain_conf.h | 1 + .../qemuxml2argv-virtio-options.xml | 1 + .../qemuxml2xmlout-virtio-options.xml | 1 + 6 files changed, 42 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a546cbb..5c0f3bf 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6136,6 +6136,18 @@ qemu-kvm -net nic,model=? /dev/null The optional <code>address</code> sub-element can be used to tie the video device to a particular PCI slot. </dd> + + <dt><code>driver</code></dt> + <dd> + The subelement <code>driver</code> can be used to tune the device: + <dl> + <dt>virtio options</dt> + <dd> + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </dd> + </dl> + </dd> </dl> <h4><a name="elementsConsole">Consoles, serial, parallel & channel devices</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3909a56..3df4fa4 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3219,6 +3219,11 @@ <define name="video"> <element name="video"> <optional> + <element name="driver"> + <ref name="virtioOptions"/> + </element> + </optional> + <optional> <element name="model"> <choice> <attribute name="type"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f92fecc..9381551 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2338,6 +2338,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def) virDomainDeviceInfoClear(&def->info); VIR_FREE(def->accel); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -13478,11 +13479,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) static virDomainVideoDefPtr virDomainVideoDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, const virDomainDef *dom, unsigned int flags) { virDomainVideoDefPtr def; xmlNodePtr cur; + xmlNodePtr saved = ctxt->node; char *type = NULL; char *heads = NULL; char *vram = NULL; @@ -13491,6 +13494,8 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *vgamem = NULL; char *primary = NULL; + ctxt->node = node; + if (VIR_ALLOC(def) < 0) return NULL; @@ -13592,7 +13597,12 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: + ctxt->node = saved; + VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); @@ -14391,7 +14401,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_VIDEO: - if (!(dev->data.video = virDomainVideoDefParseXML(node, def, flags))) + if (!(dev->data.video = virDomainVideoDefParseXML(node, ctxt, def, flags))) goto error; break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -18334,7 +18344,7 @@ virDomainDefParseXML(xmlDocPtr xml, virDomainVideoDefPtr video; ssize_t insertAt = -1; - if (!(video = virDomainVideoDefParseXML(nodes[i], def, flags))) + if (!(video = virDomainVideoDefParseXML(nodes[i], ctxt, def, flags))) goto error; if (video->primary) { @@ -23290,6 +23300,7 @@ virDomainVideoDefFormat(virBufferPtr buf, unsigned int flags) { const char *model = virDomainVideoTypeToString(def->type); + virBuffer driverBuf = VIR_BUFFER_INITIALIZER; if (!model) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -23299,6 +23310,15 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, "<video>\n"); virBufferAdjustIndent(buf, 2); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferCheckError(&driverBuf) < 0) + return -1; + if (virBufferUse(&driverBuf)) { + virBufferTrim(&driverBuf, " ", -1); + virBufferAddLit(buf, "<driver "); + virBufferAddBuffer(buf, &driverBuf); + virBufferAddLit(buf, "/>\n"); + } virBufferAsprintf(buf, "<model type='%s'", model); if (def->ram) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1887ac..3ee187e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1383,6 +1383,7 @@ struct _virDomainVideoDef { bool primary; virDomainVideoAccelDefPtr accel; virDomainDeviceInfo info; + virDomainVirtioOptionsPtr virtio; }; /* graphics console modes */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index e852894..be702cb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -81,6 +81,7 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <video> + <driver iommu_platform='on' ats='on'/> <model type='virtio' heads='1' primary='yes'> <acceleration accel3d='yes'/> </model> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index e852894..be702cb 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -81,6 +81,7 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <video> + <driver iommu_platform='on' ats='on'/> <model type='virtio' heads='1' primary='yes'> <acceleration accel3d='yes'/> </model> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:26PM +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 12 +++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 24 ++++++++++++++++++++-- src/conf/domain_conf.h | 1 + .../qemuxml2argv-virtio-options.xml | 1 + .../qemuxml2xmlout-virtio-options.xml | 1 + 6 files changed, 42 insertions(+), 2 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 13 +++++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 4 ++++ .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 4 ++++ 6 files changed, 42 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5c0f3bf..c9c3592 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5715,6 +5715,19 @@ qemu-kvm -net nic,model=? /dev/null event device passed through to guests. (KVM only) </p> + <p> + The subelement <code>driver</code> can be used to tune the virtio + options of the device: + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </p> + <p> + The <code>compatibility</code> attribute of the <code>driver</code> element + can be used to specify the compatibility of virtio devices. Allowed values + are <code>legacy</code>, <code>transitional</code> and <code>modern</code>. + <span class="since">Since 2.2.0</span>. + </p> + <h4><a name="elementsHub">Hub devices</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3df4fa4..78ff0a2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3990,6 +3990,11 @@ <define name="input"> <element name="input"> + <optional> + <element name="driver"> + <ref name="virtioOptions"/> + </element> + </optional> <choice> <group> <attribute name="type"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9381551..6b1c8a2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1386,6 +1386,7 @@ void virDomainInputDefFree(virDomainInputDefPtr def) virDomainDeviceInfoClear(&def->info); VIR_FREE(def->source.evdev); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -11567,6 +11568,9 @@ virDomainInputDefParseXML(const virDomainDef *dom, goto error; } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: VIR_FREE(evdev); VIR_FREE(type); @@ -23359,6 +23363,7 @@ virDomainInputDefFormat(virBufferPtr buf, const char *type = virDomainInputTypeToString(def->type); const char *bus = virDomainInputBusTypeToString(def->bus); virBuffer childbuf = VIR_BUFFER_INITIALIZER; + virBuffer driverBuf = VIR_BUFFER_INITIALIZER; /* don't format keyboard into migratable XML for backward compatibility */ if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && @@ -23382,6 +23387,16 @@ virDomainInputDefFormat(virBufferPtr buf, type, bus); virBufferAdjustIndent(&childbuf, virBufferGetIndent(buf, false) + 2); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferCheckError(&driverBuf) < 0) + return -1; + + if (virBufferUse(&driverBuf)) { + virBufferTrim(&driverBuf, " ", -1); + virBufferAddLit(&childbuf, "<driver "); + virBufferAddBuffer(&childbuf, &driverBuf); + virBufferAddLit(&childbuf, "/>\n"); + } virBufferEscapeString(&childbuf, "<source evdev='%s'/>\n", def->source.evdev); if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3ee187e..767b6a0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1289,6 +1289,7 @@ struct _virDomainInputDef { char *evdev; } source; virDomainDeviceInfo info; + virDomainVirtioOptionsPtr virtio; }; typedef enum { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index be702cb..f614e68 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -66,15 +66,19 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <input type='mouse' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </input> <input type='keyboard' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> </input> <input type='tablet' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> </input> <input type='passthrough' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <source evdev='/dev/input/event1234'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> </input> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml index be702cb..f614e68 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -66,15 +66,19 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <input type='mouse' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </input> <input type='keyboard' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> </input> <input type='tablet' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> </input> <input type='passthrough' bus='virtio'> + <driver iommu_platform='on' ats='on'/> <source evdev='/dev/input/event1234'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> </input> -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:27PM +0200, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 13 +++++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 4 ++++ .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml | 4 ++++ 6 files changed, 42 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5c0f3bf..c9c3592 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5715,6 +5715,19 @@ qemu-kvm -net nic,model=? /dev/null event device passed through to guests. (KVM only) </p>
+ <p> + The subelement <code>driver</code> can be used to tune the virtio + options of the device: + <a href="#elementsVirtio">Virtio-specific options</a> can also be + set. (<span class="since">Since 3.5.0</span>) + </p> + <p> + The <code>compatibility</code> attribute of the <code>driver</code> element + can be used to specify the compatibility of virtio devices. Allowed values + are <code>legacy</code>, <code>transitional</code> and <code>modern</code>. + <span class="since">Since 2.2.0</span>. + </p>
So there is a documentation for the driver compatibility but there is no code to handle these attributes, so I guess that it's really some leftover. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Add a test case to demonstrate the addition of new command line options https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- .../qemuxml2argv-virtio-options.args | 47 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 9 +++++ 2 files changed, 56 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args new file mode 100644 index 0000000..b8b5910 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args @@ -0,0 +1,47 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x8 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x9 \ +-usb \ +-drive file=/var/lib/libvirt/images/img1,format=raw,if=none,\ +id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0xa,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-drive file=/var/lib/libvirt/images/img2,format=raw,if=none,\ +id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0xb,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/fs1 \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=fs1,bus=pci.0,addr=0x3 \ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ +path=/export/fs2 \ +-device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,mount_tag=fs2,bus=pci.0,addr=0x4 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:56:58:5a:5c,bus=pci.0,addr=0x6 \ +-net user,vlan=0,name=hostnet0 \ +-device virtio-net-pci,vlan=1,id=net1,mac=52:54:56:5a:5c:5e,bus=pci.0,addr=0x7 \ +-net user,vlan=1,name=hostnet1 \ +-device virtio-mouse-pci,id=input0,bus=pci.0,addr=0xe \ +-device virtio-keyboard-pci,id=input1,bus=pci.0,addr=0x10 \ +-device virtio-tablet-pci,id=input2,bus=pci.0,addr=0x11 \ +-device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\ +addr=0x12 \ +-device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc \ +-object rng-random,id=objrng0,filename=/dev/random \ +-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0xd diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 455caa4..609ea1e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2545,6 +2545,15 @@ mymain(void) QEMU_CAPS_DEVICE_INTEL_IOMMU); DO_TEST("cpu-hotplug-startup", QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS); + 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_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT, + 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); DO_TEST("fd-memory-numa-topology", QEMU_CAPS_MEM_PATH, QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_KVM); -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:28PM +0200, Ján Tomko wrote:
Add a test case to demonstrate the addition of new command line options
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- .../qemuxml2argv-virtio-options.args | 47 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 9 +++++ 2 files changed, 56 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args
This needs to be updated based on what you choose to do with patch 5. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Format iommu_platform= and ats= for virtio devices. https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- src/qemu/qemu_capabilities.c | 12 ++++- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 58 ++++++++++++++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 2 + .../qemuxml2argv-virtio-options.args | 44 +++++++++------- tests/qemuxml2argvtest.c | 4 +- 6 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc78eb9..8b31174 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -374,6 +374,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "intel-iommu.eim", "intel-iommu.device-iotlb", /* 260 */ + "virtio.iommu_platform", + "virtio.ats", ); @@ -1853,7 +1855,7 @@ struct virQEMUCapsPropTypeObjects { const char **objects; }; -static const char *virQEMUCapsVirtioPCIDisableLegacyObjects[] = { +static const char *virQEMUCapsVirtioPCIObjects[] = { "virtio-balloon-pci", "virtio-blk-pci", "virtio-scsi-pci", @@ -1872,7 +1874,13 @@ static const char *virQEMUCapsVirtioPCIDisableLegacyObjects[] = { static struct virQEMUCapsPropTypeObjects virQEMUCapsPropObjects[] = { { "disable-legacy", QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, - virQEMUCapsVirtioPCIDisableLegacyObjects } + virQEMUCapsVirtioPCIObjects }, + { "iommu_platform", + QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM, + virQEMUCapsVirtioPCIObjects }, + { "ats", + QEMU_CAPS_VIRTIO_PCI_ATS, + virQEMUCapsVirtioPCIObjects }, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 275bbcf..e57cae9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -413,6 +413,8 @@ typedef enum { /* 260 */ QEMU_CAPS_INTEL_IOMMU_DEVICE_IOTLB, /* intel-iommu.device-iotlb */ + QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM, /* virtio-*-pci.iommu_platform */ + QEMU_CAPS_VIRTIO_PCI_ATS, /* virtio-*-pci.ats */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1f54ba0..59da13f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -389,6 +389,38 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, } static int +qemuBuildVirtioOptionsStr(virBufferPtr buf, + virDomainVirtioOptionsPtr virtio, + virQEMUCapsPtr qemuCaps) +{ + if (!virtio) + return 0; + + if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the iommu_platform setting is not supported " + "with this QEMU binary")); + return -1; + } + virBufferAsprintf(buf, ",iommu_platform=%s", + virTristateSwitchTypeToString(virtio->iommu_platform)); + } + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_ATS)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the ats setting is not supported with this " + "QEMU binary")); + return -1; + } + virBufferAsprintf(buf, ",ats=%s", + virTristateSwitchTypeToString(virtio->ats)); + } + + return 0; +} + +static int qemuBuildRomStr(virBufferPtr buf, virDomainDeviceInfoPtr info) { @@ -2167,6 +2199,10 @@ qemuBuildDriveDevStr(const virDomainDef *def, (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) ? "on" : "off"); } + + if (qemuBuildVirtioOptionsStr(&opt, disk->virtio, qemuCaps) < 0) + goto error; + if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) goto error; break; @@ -2490,6 +2526,8 @@ qemuBuildFSDevStr(const virDomainDef *def, QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); + qemuBuildVirtioOptionsStr(&opt, fs->virtio, qemuCaps); + if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) goto error; @@ -2734,6 +2772,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->iothread); } } + if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0) + goto error; break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: virBufferAddLit(&buf, "lsi"); @@ -2779,6 +2819,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, ",vectors=%d", def->opts.vioserial.vectors); } + if (qemuBuildVirtioOptionsStr(&buf, def->virtio, qemuCaps) < 0) + goto error; break; case VIR_DOMAIN_CONTROLLER_TYPE_CCID: @@ -3773,12 +3815,16 @@ qemuBuildNicDevStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",id=%s", net->info.alias); virBufferAsprintf(&buf, ",mac=%s", virMacAddrFormat(&net->mac, macaddr)); + if (qemuBuildDeviceAddressStr(&buf, def, &net->info, qemuCaps) < 0) goto error; if (qemuBuildRomStr(&buf, &net->info) < 0) goto error; if (bootindex && virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) virBufferAsprintf(&buf, ",bootindex=%u", bootindex); + if (usingVirtio && + qemuBuildVirtioOptionsStr(&buf, net->virtio, qemuCaps) < 0) + goto error; if (virBufferCheckError(&buf) < 0) goto error; @@ -4053,6 +4099,9 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, virTristateSwitchTypeToString(def->memballoon->autodeflate)); } + if (qemuBuildVirtioOptionsStr(&buf, def->memballoon->virtio, qemuCaps) < 0) + goto error; + virCommandAddArg(cmd, "-device"); virCommandAddArgBuffer(cmd, &buf); return 0; @@ -4183,6 +4232,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) goto error; + if (qemuBuildVirtioOptionsStr(&buf, dev->virtio, qemuCaps) < 0) + goto error; + if (virBufferCheckError(&buf) < 0) goto error; @@ -4495,6 +4547,9 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0) goto error; + if (qemuBuildVirtioOptionsStr(&buf, video->virtio, qemuCaps) < 0) + goto error; + if (virBufferCheckError(&buf) < 0) goto error; @@ -5863,6 +5918,9 @@ qemuBuildRNGDevStr(const virDomainDef *def, virBufferAddLit(&buf, ",period=1000"); } + if (qemuBuildVirtioOptionsStr(&buf, dev->virtio, qemuCaps) < 0) + goto error; + if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) goto error; if (virBufferCheckError(&buf) < 0) diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index d51ee46..58dd9f6 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -216,6 +216,8 @@ <flag name='intel-iommu.caching-mode'/> <flag name='intel-iommu.eim'/> <flag name='intel-iommu.device-iotlb'/> + <flag name='virtio.iommu_platform'/> + <flag name='virtio.ats'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args index b8b5910..677366d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args @@ -16,32 +16,42 @@ QEMU_AUDIO_DRV=none \ -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ -no-acpi \ -boot c \ --device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x8 \ --device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x9 \ +-device virtio-scsi-pci,iommu_platform=on,ats=on,id=scsi0,bus=pci.0,addr=0x8 \ +-device virtio-serial-pci,id=virtio-serial0,iommu_platform=on,ats=on,bus=pci.0,\ +addr=0x9 \ -usb \ -drive file=/var/lib/libvirt/images/img1,format=raw,if=none,\ id=drive-virtio-disk0 \ --device virtio-blk-pci,bus=pci.0,addr=0xa,drive=drive-virtio-disk0,\ -id=virtio-disk0 \ +-device virtio-blk-pci,iommu_platform=on,ats=on,bus=pci.0,addr=0xa,\ +drive=drive-virtio-disk0,id=virtio-disk0 \ -drive file=/var/lib/libvirt/images/img2,format=raw,if=none,\ id=drive-virtio-disk1 \ --device virtio-blk-pci,bus=pci.0,addr=0xb,drive=drive-virtio-disk1,\ -id=virtio-disk1 \ +-device virtio-blk-pci,iommu_platform=off,ats=off,bus=pci.0,addr=0xb,\ +drive=drive-virtio-disk1,id=virtio-disk1 \ -fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/fs1 \ --device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=fs1,bus=pci.0,addr=0x3 \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=fs1,iommu_platform=on,\ +ats=on,bus=pci.0,addr=0x3 \ -fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ path=/export/fs2 \ --device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,mount_tag=fs2,bus=pci.0,addr=0x4 \ --device virtio-net-pci,vlan=0,id=net0,mac=52:54:56:58:5a:5c,bus=pci.0,addr=0x6 \ +-device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,mount_tag=fs2,iommu_platform=on,\ +ats=on,bus=pci.0,addr=0x4 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:56:58:5a:5c,bus=pci.0,addr=0x6,\ +iommu_platform=on,ats=on \ -net user,vlan=0,name=hostnet0 \ --device virtio-net-pci,vlan=1,id=net1,mac=52:54:56:5a:5c:5e,bus=pci.0,addr=0x7 \ +-device virtio-net-pci,vlan=1,id=net1,mac=52:54:56:5a:5c:5e,bus=pci.0,addr=0x7,\ +iommu_platform=off,ats=off \ -net user,vlan=1,name=hostnet1 \ --device virtio-mouse-pci,id=input0,bus=pci.0,addr=0xe \ --device virtio-keyboard-pci,id=input1,bus=pci.0,addr=0x10 \ --device virtio-tablet-pci,id=input2,bus=pci.0,addr=0x11 \ +-device virtio-mouse-pci,id=input0,bus=pci.0,addr=0xe,iommu_platform=on,ats=on \ +-device virtio-keyboard-pci,id=input1,bus=pci.0,addr=0x10,iommu_platform=on,\ +ats=on \ +-device virtio-tablet-pci,id=input2,bus=pci.0,addr=0x11,iommu_platform=on,\ +ats=on \ -device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\ -addr=0x12 \ --device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc \ +addr=0x12,iommu_platform=on,ats=on \ +-device virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2,iommu_platform=on,\ +ats=on \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,iommu_platform=on,\ +ats=on \ -object rng-random,id=objrng0,filename=/dev/random \ --device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0xd +-device virtio-rng-pci,rng=objrng0,id=rng0,iommu_platform=on,ats=on,bus=pci.0,\ +addr=0xd diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 609ea1e..b410b20 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2553,7 +2553,9 @@ mymain(void) QEMU_CAPS_VIRTIO_GPU_VIRGL, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM, + QEMU_CAPS_VIRTIO_PCI_ATS); DO_TEST("fd-memory-numa-topology", QEMU_CAPS_MEM_PATH, QEMU_CAPS_OBJECT_MEMORY_FILE, QEMU_CAPS_KVM); -- 2.10.2
On Tue, Jun 06, 2017 at 01:36:29PM +0200, Ján Tomko wrote:
Format iommu_platform= and ats= for virtio devices.
https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- src/qemu/qemu_capabilities.c | 12 ++++- src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 58 ++++++++++++++++++++++ tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 2 + .../qemuxml2argv-virtio-options.args | 44 +++++++++------- tests/qemuxml2argvtest.c | 4 +- 6 files changed, 102 insertions(+), 20 deletions(-)
I've just realized that we format the iommu options only for some controller types or disk devices with bus="virtio". Shouldn't we check this in post parse callback and don't allow to specify the iommu options where it doesn't make sense? Otherwise the patch itself is good, so Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Ján Tomko -
Pavel Hrdina