[libvirt] [PATCHv3 00/14] Allow virtio devices to use vIOMMU

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 new in v3: * eliminated multi-word attribute names * eliminated extra virBufferTrims * added ABI stability checks * post-parse checks for non-virtio disks and nets * removed duplicate devices from the test file Ján Tomko (14): conf: add iotlb attribute to iommu qemu: format device-iotlb on intel-iommu command line qemuxml2xmltest: add virtio-options test conf: use a leading space in virDomainVirtioNetDriverFormat 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 | 82 ++++++ docs/schemas/domaincommon.rng | 40 +++ src/conf/domain_conf.c | 283 ++++++++++++++++++++- src/conf/domain_conf.h | 18 ++ 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 | 50 ++++ .../qemuxml2argv-virtio-options.xml | 92 +++++++ tests/qemuxml2argvtest.c | 18 ++ .../qemuxml2xmlout-intel-iommu-device-iotlb.xml | 1 + .../qemuxml2xmlout-virtio-options.xml | 1 + tests/qemuxml2xmltest.c | 2 + 16 files changed, 715 insertions(+), 14 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 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml -- 2.10.2

Add a new 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 | 10 +++++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 23 +++++++++++++++- 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, 71 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..fa0edd1 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7449,6 +7449,16 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 3.4.0</span> (QEMU/KVM only) </p> </dd> + <dt><code>iotlb</code></dt> + <dd> + <p> + The <code>iotlb</code> attribute with possible values + <code>on</code> and <code>off</code> can be used to + turn on the IOTLB used to cache address translation + requests from devices. + <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..5c542c7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3964,6 +3964,11 @@ <ref name="virOnOff"/> </attribute> </optional> + <optional> + <attribute name="iotlb"> + <ref name="virOnOff"/> + </attribute> + </optional> </element> </optional> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d4ee92e..625449a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14222,6 +14222,14 @@ virDomainIOMMUDefParseXML(xmlNodePtr node, } iommu->caching_mode = val; } + VIR_FREE(tmp); + if ((tmp = virXPathString("string(./driver/@iotlb)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_XML_ERROR, _("unknown iotlb value: %s"), tmp); + goto cleanup; + } + iommu->iotlb = val; + } VIR_FREE(tmp); if ((tmp = virXPathString("string(./driver/@eim)", ctxt))) { @@ -19950,6 +19958,14 @@ virDomainIOMMUDefCheckABIStability(virDomainIOMMUDefPtr src, virTristateSwitchTypeToString(src->eim)); return false; } + if (src->iotlb != dst->iotlb) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain IOMMU device iotlb value '%s' " + "does not match source '%s'"), + virTristateSwitchTypeToString(dst->iotlb), + virTristateSwitchTypeToString(src->iotlb)); + return false; + } return true; } @@ -24264,7 +24280,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->iotlb != VIR_TRISTATE_SWITCH_ABSENT) { virBufferAddLit(&childBuf, "<driver"); if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { virBufferAsprintf(&childBuf, " intremap='%s'", @@ -24278,6 +24295,10 @@ virDomainIOMMUDefFormat(virBufferPtr buf, virBufferAsprintf(&childBuf, " eim='%s'", virTristateSwitchTypeToString(iommu->eim)); } + if (iommu->iotlb != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, " iotlb='%s'", + virTristateSwitchTypeToString(iommu->iotlb)); + } virBufferAddLit(&childBuf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1231aea..597fb22 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2213,6 +2213,7 @@ struct _virDomainIOMMUDef { virTristateSwitch intremap; virTristateSwitch caching_mode; virTristateSwitch eim; + virTristateSwitch 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..3eb08ab --- /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' 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 Wed, Jun 07, 2017 at 09:24:51PM +0200, Ján Tomko wrote:
Add a new 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 | 10 +++++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 23 +++++++++++++++- 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, 71 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
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Format the device-iotlb attribute. https://bugzilla.redhat.com/show_bug.cgi?id=1283251 Reviewed-by: Pavel Hrdina <phrdina@redhat.com> --- 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 7f22492..7da0988 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..cae1e17 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->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->iotlb != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&opts, ",device-iotlb=%s", + virTristateSwitchTypeToString(iommu->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

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 | 81 ++++++++++++++++++++++ .../qemuxml2xmlout-virtio-options.xml | 1 + tests/qemuxml2xmltest.c | 1 + 3 files changed, 83 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml create mode 120000 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..c88cf64 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -0,0 +1,81 @@ +<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> + <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> + <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 120000 index 0000000..d757b8a --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-virtio-options.xml \ No newline at end of file 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 Wed, Jun 07, 2017 at 09:24:53PM +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 | 81 ++++++++++++++++++++++ .../qemuxml2xmlout-virtio-options.xml | 1 + tests/qemuxml2xmltest.c | 1 + 3 files changed, 83 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-options.xml
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Instead of formatting a space after every option. --- src/conf/domain_conf.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 625449a..710048c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -22097,29 +22097,27 @@ virDomainVirtioNetDriverFormat(char **outstr, { virBuffer buf = VIR_BUFFER_INITIALIZER; if (def->driver.virtio.name) { - virBufferAsprintf(&buf, "name='%s' ", + virBufferAsprintf(&buf, " name='%s'", virDomainNetBackendTypeToString(def->driver.virtio.name)); } if (def->driver.virtio.txmode) { - virBufferAsprintf(&buf, "txmode='%s' ", + virBufferAsprintf(&buf, " txmode='%s'", virDomainNetVirtioTxModeTypeToString(def->driver.virtio.txmode)); } if (def->driver.virtio.ioeventfd) { - virBufferAsprintf(&buf, "ioeventfd='%s' ", + virBufferAsprintf(&buf, " ioeventfd='%s'", virTristateSwitchTypeToString(def->driver.virtio.ioeventfd)); } if (def->driver.virtio.event_idx) { - virBufferAsprintf(&buf, "event_idx='%s' ", + virBufferAsprintf(&buf, " event_idx='%s'", virTristateSwitchTypeToString(def->driver.virtio.event_idx)); } if (def->driver.virtio.queues) - virBufferAsprintf(&buf, "queues='%u' ", def->driver.virtio.queues); + virBufferAsprintf(&buf, " queues='%u'", def->driver.virtio.queues); if (def->driver.virtio.rx_queue_size) - virBufferAsprintf(&buf, "rx_queue_size='%u' ", + virBufferAsprintf(&buf, " rx_queue_size='%u'", def->driver.virtio.rx_queue_size); - virBufferTrim(&buf, " ", -1); - if (virBufferCheckError(&buf) < 0) return -1; @@ -22367,10 +22365,10 @@ virDomainNetDefFormat(virBufferPtr buf, if (!gueststr && !hoststr) { if (str) - virBufferAsprintf(buf, "<driver %s/>\n", str); + virBufferAsprintf(buf, "<driver%s/>\n", str); } else { if (str) - virBufferAsprintf(buf, "<driver %s>\n", str); + virBufferAsprintf(buf, "<driver%s>\n", str); else virBufferAddLit(buf, "<driver>\n"); virBufferAdjustIndent(buf, 2); -- 2.10.2

On Wed, Jun 07, 2017 at 09:24:54PM +0200, Ján Tomko wrote:
Instead of formatting a space after every option. --- src/conf/domain_conf.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu='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 | 121 +++++++++++++++++++++ src/conf/domain_conf.h | 10 ++ .../qemuxml2argv-virtio-options.xml | 1 + 5 files changed, 163 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fa0edd1..e9ff73b 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</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 5c542c7..15fd72f 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"> + <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 710048c..22b8653 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1112,6 +1112,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)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid iommu value")); + goto cleanup; + } + res->iommu = 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; +} + virSaveCookieCallbacksPtr virDomainXMLOptionGetSaveCookie(virDomainXMLOptionPtr xmlopt) @@ -1966,6 +2006,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); @@ -4340,6 +4381,28 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev, static int +virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio) +{ + if (!virtio) + return 0; + + if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu driver option is only supported " + "for virtio devices")); + return -1; + } + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ats driver option is only supported " + "for virtio devices")); + return -1; + } + return 0; +} + + +static int virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, const virDomainDef *def, virCapsPtr caps ATTRIBUTE_UNUSED, @@ -4437,6 +4500,13 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, } } + if (dev->type == VIR_DOMAIN_DEVICE_NET) { + virDomainNetDefPtr net = dev->data.net; + if (STRNEQ_NULLABLE(net->model, "virtio") && + virDomainCheckVirtioOptions(net->virtio) < 0) + return -1; + } + return 0; } @@ -5235,6 +5305,24 @@ virDomainDefValidate(virDomainDefPtr def, } +static void +virDomainVirtioOptionsFormat(virBufferPtr buf, + virDomainVirtioOptionsPtr virtio) +{ + if (!virtio) + return; + + if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, " iommu='%s'", + virTristateSwitchTypeToString(virtio->iommu)); + } + 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 */ @@ -10381,6 +10469,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: ctxt->node = oldnode; VIR_FREE(macaddr); @@ -19005,6 +19096,30 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, static bool +virDomainVirtioOptionsCheckABIStability(virDomainVirtioOptionsPtr src, + virDomainVirtioOptionsPtr dst) +{ + if (src->iommu != dst->iommu) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target device iommu option '%s' does not " + "match source '%s'"), + virTristateSwitchTypeToString(dst->iommu), + virTristateSwitchTypeToString(src->iommu)); + return false; + } + if (src->ats != dst->ats) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target device ats option '%s' does not " + "match source '%s'"), + virTristateSwitchTypeToString(dst->ats), + virTristateSwitchTypeToString(src->ats)); + return false; + } + return true; +} + + +static bool virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, virDomainDiskDefPtr dst) { @@ -19163,6 +19278,10 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -22118,6 +22237,8 @@ virDomainVirtioNetDriverFormat(char **outstr, virBufferAsprintf(&buf, " rx_queue_size='%u'", def->driver.virtio.rx_queue_size); + virDomainVirtioOptionsFormat(&buf, def->virtio); + if (virBufferCheckError(&buf) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 597fb22..b3e607b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -156,6 +156,9 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr; +typedef struct _virDomainVirtioOptions virDomainVirtioOptions; +typedef virDomainVirtioOptions *virDomainVirtioOptionsPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -1040,6 +1043,7 @@ struct _virDomainNetDef { int linkstate; unsigned int mtu; virNetDevCoalescePtr coalesce; + virDomainVirtioOptionsPtr virtio; }; typedef enum { @@ -2215,6 +2219,12 @@ struct _virDomainIOMMUDef { virTristateSwitch eim; virTristateSwitch iotlb; }; + +struct _virDomainVirtioOptions { + virTristateSwitch iommu; + virTristateSwitch ats; +}; + /* * Guest VM main configuration * diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index c88cf64..3357bc6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -47,6 +47,7 @@ <interface type='user'> <mac address='52:54:56:58:5a:5c'/> <model type='virtio'/> + <driver iommu='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </interface> <input type='mouse' bus='virtio'> -- 2.10.2

On Wed, Jun 07, 2017 at 09:24:55PM +0200, Ján Tomko wrote:
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu='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 | 121 +++++++++++++++++++++ src/conf/domain_conf.h | 10 ++ .../qemuxml2argv-virtio-options.xml | 1 + 5 files changed, 163 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fa0edd1..e9ff73b 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</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 5c542c7..15fd72f 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"> + <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 710048c..22b8653 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1112,6 +1112,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)", ctxt))) { + if ((val = virTristateSwitchTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid iommu value")); + goto cleanup; + } + res->iommu = 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; +} +
virSaveCookieCallbacksPtr virDomainXMLOptionGetSaveCookie(virDomainXMLOptionPtr xmlopt) @@ -1966,6 +2006,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); @@ -4340,6 +4381,28 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev,
static int +virDomainCheckVirtioOptions(virDomainVirtioOptionsPtr virtio) +{ + if (!virtio) + return 0; + + if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iommu driver option is only supported " + "for virtio devices")); + return -1; + } + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ats driver option is only supported " + "for virtio devices")); + return -1; + } + return 0; +} + + +static int virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, const virDomainDef *def, virCapsPtr caps ATTRIBUTE_UNUSED, @@ -4437,6 +4500,13 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, } }
+ if (dev->type == VIR_DOMAIN_DEVICE_NET) { + virDomainNetDefPtr net = dev->data.net; + if (STRNEQ_NULLABLE(net->model, "virtio") && + virDomainCheckVirtioOptions(net->virtio) < 0) + return -1; + } + return 0; }
@@ -5235,6 +5305,24 @@ virDomainDefValidate(virDomainDefPtr def, }
+static void +virDomainVirtioOptionsFormat(virBufferPtr buf, + virDomainVirtioOptionsPtr virtio) +{ + if (!virtio) + return; + + if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(buf, " iommu='%s'", + virTristateSwitchTypeToString(virtio->iommu)); + } + 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 */ @@ -10381,6 +10469,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
+ if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: ctxt->node = oldnode; VIR_FREE(macaddr); @@ -19005,6 +19096,30 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
static bool +virDomainVirtioOptionsCheckABIStability(virDomainVirtioOptionsPtr src, + virDomainVirtioOptionsPtr dst) +{ + if (src->iommu != dst->iommu) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target device iommu option '%s' does not " + "match source '%s'"), + virTristateSwitchTypeToString(dst->iommu), + virTristateSwitchTypeToString(src->iommu)); + return false; + } + if (src->ats != dst->ats) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target device ats option '%s' does not " + "match source '%s'"), + virTristateSwitchTypeToString(dst->ats), + virTristateSwitchTypeToString(src->ats)); + return false; + } + return true; +} + + +static bool virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, virDomainDiskDefPtr dst) { @@ -19163,6 +19278,10 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; }
+ if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false;
Shouldn't we also check !!src->virtion == !!dst->virtio? The parser for virtio options always allocates @virtio so technically it shouldn't happen but throughout the code we check whether @virtio is allocated. I've missed the ABI stability check in the previous review, good catch. Pavel
+ if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false;
@@ -22118,6 +22237,8 @@ virDomainVirtioNetDriverFormat(char **outstr, virBufferAsprintf(&buf, " rx_queue_size='%u'", def->driver.virtio.rx_queue_size);
+ virDomainVirtioOptionsFormat(&buf, def->virtio); + if (virBufferCheckError(&buf) < 0) return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 597fb22..b3e607b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -156,6 +156,9 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
+typedef struct _virDomainVirtioOptions virDomainVirtioOptions; +typedef virDomainVirtioOptions *virDomainVirtioOptionsPtr; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -1040,6 +1043,7 @@ struct _virDomainNetDef { int linkstate; unsigned int mtu; virNetDevCoalescePtr coalesce; + virDomainVirtioOptionsPtr virtio; };
typedef enum { @@ -2215,6 +2219,12 @@ struct _virDomainIOMMUDef { virTristateSwitch eim; virTristateSwitch iotlb; }; + +struct _virDomainVirtioOptions { + virTristateSwitch iommu; + virTristateSwitch ats; +}; + /* * Guest VM main configuration * diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml index c88cf64..3357bc6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -47,6 +47,7 @@ <interface type='user'> <mac address='52:54:56:58:5a:5c'/> <model type='virtio'/> + <driver iommu='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </interface> <input type='mouse' bus='virtio'> -- 2.10.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jun 08, 2017 at 12:30:15PM +0200, Pavel Hrdina wrote:
On Wed, Jun 07, 2017 at 09:24:55PM +0200, Ján Tomko wrote:
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu='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 | 121 +++++++++++++++++++++ src/conf/domain_conf.h | 10 ++ .../qemuxml2argv-virtio-options.xml | 1 + 5 files changed, 163 insertions(+)
@@ -19163,6 +19278,10 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; }
+ if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false;
Shouldn't we also check !!src->virtion == !!dst->virtio? The parser for virtio options always allocates @virtio so technically it shouldn't happen but throughout the code we check whether @virtio is allocated.
virtio can be NULL in case the domain definition was not created by parsing XML. Without the non-NULL checks in the formatter, some of the tests were crashing. If someone somehow manages to bypass the parser for the QEMU driver, which is the only one where these options make any sense, I'm okay with not having its ABI stability checked, as long as we don't crash. Jan

On Thu, Jun 08, 2017 at 12:37:29PM +0200, Ján Tomko wrote:
On Thu, Jun 08, 2017 at 12:30:15PM +0200, Pavel Hrdina wrote:
On Wed, Jun 07, 2017 at 09:24:55PM +0200, Ján Tomko wrote:
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <driver iommu='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 | 121 +++++++++++++++++++++ src/conf/domain_conf.h | 10 ++ .../qemuxml2argv-virtio-options.xml | 1 + 5 files changed, 163 insertions(+)
@@ -19163,6 +19278,10 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, return false; }
+ if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false;
Shouldn't we also check !!src->virtion == !!dst->virtio? The parser for virtio options always allocates @virtio so technically it shouldn't happen but throughout the code we check whether @virtio is allocated.
virtio can be NULL in case the domain definition was not created by parsing XML. Without the non-NULL checks in the formatter, some of the tests were crashing.
If someone somehow manages to bypass the parser for the QEMU driver, which is the only one where these options make any sense, I'm okay with not having its ABI stability checked, as long as we don't crash.
Jan
Fair enough, in that case for patches 5-12 Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 24 ++++++++++++++++++++++ src/conf/domain_conf.h | 1 + .../qemuxml2argv-virtio-options.xml | 1 + 5 files changed, 38 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e9ff73b..77ccb2d 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='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 15fd72f..7cea0c5 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 22b8653..58ba1da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2305,6 +2305,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) return; virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -12997,6 +12998,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); @@ -19608,6 +19612,10 @@ virDomainMemballoonDefCheckABIStability(virDomainMemballoonDefPtr src, return false; } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -22978,6 +22986,22 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return -1; } + if (def->virtio) { + virBuffer driverBuf = VIR_BUFFER_INITIALIZER; + + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + + 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 b3e607b..b6594d3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1609,6 +1609,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 3357bc6..b16a984 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -73,6 +73,7 @@ </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + <driver iommu='on' ats='on'/> </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> -- 2.10.2

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 2 +- 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 77ccb2d..c867fb8 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 7cea0c5..d106550 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 58ba1da..a03ed09 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1745,6 +1745,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); @@ -4467,6 +4468,10 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, } } + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + virDomainCheckVirtioOptions(disk->virtio) < 0) + return -1; + if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) return -1; @@ -8447,6 +8452,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + /* Disk volume types will have authentication information handled in * virStorageTranslateDiskSourcePool */ @@ -19172,6 +19180,10 @@ virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, return false; } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -21360,6 +21372,9 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(&driverBuf, " iothread='%u'", def->iothread); if (def->detect_zeroes) virBufferAsprintf(&driverBuf, " detect_zeroes='%s'", detect_zeroes); + + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferUse(&driverBuf)) { virBufferAddLit(buf, "<driver"); virBufferAddBuffer(buf, &driverBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b6594d3..df64eb0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -668,6 +668,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 b16a984..6dd82de 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -15,7 +15,7 @@ <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='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'/> -- 2.10.2

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 2 ++ 5 files changed, 20 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c867fb8..77242f0 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 d106550..1d46392 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 a03ed09..a3c06c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1886,6 +1886,7 @@ void virDomainControllerDefFree(virDomainControllerDefPtr def) return; virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -9064,6 +9065,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... */ @@ -19240,6 +19244,10 @@ virDomainControllerDefCheckABIStability(virDomainControllerDefPtr src, } } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -21557,6 +21565,8 @@ virDomainControllerDriverFormat(virBufferPtr buf, if (def->iothread) virBufferAsprintf(&driverBuf, " iothread='%u'", def->iothread); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferUse(&driverBuf)) { virBufferAddLit(buf, "<driver"); virBufferAddBuffer(buf, &driverBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index df64eb0..9795767 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -813,6 +813,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 6dd82de..867e56c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -27,10 +27,12 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='scsi' index='0' model='virtio-scsi'> + <driver iommu='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='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> <filesystem type='mount' accessmode='passthrough'> -- 2.10.2

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 10 ++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 3 ++- 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 77242f0..efd7a5f 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 1d46392..8ff553d 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 a3c06c9..9c71c8a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1918,6 +1918,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def) virStorageSourceFree(def->src); VIR_FREE(def->dst); virDomainDeviceInfoClear(&def->info); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -9478,6 +9479,9 @@ virDomainFSDefParseXML(xmlNodePtr node, goto error; } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + def->src->path = source; source = NULL; def->dst = target; @@ -19272,6 +19276,10 @@ virDomainFsDefCheckABIStability(virDomainFSDefPtr src, return false; } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -21757,6 +21765,8 @@ virDomainFSDefFormat(virBufferPtr buf, } + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferUse(&driverBuf)) { virBufferAddLit(buf, "<driver"); virBufferAddBuffer(buf, &driverBuf); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9795767..acb6748 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -876,6 +876,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 867e56c..33da214 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -36,12 +36,13 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> <filesystem type='mount' accessmode='passthrough'> + <driver iommu='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='on' ats='on'/> <source dir='/export/fs2'/> <target dir='fs2'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> -- 2.10.2

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 11 +++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 1 + .../qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 1 + 5 files changed, 37 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index efd7a5f..fd2195a 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 8ff553d..c7a0867 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 9c71c8a..c6f5d93 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12949,6 +12949,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); @@ -19663,6 +19666,10 @@ virDomainRNGDefCheckABIStability(virDomainRNGDefPtr src, return false; } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -23176,6 +23183,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); @@ -23204,6 +23212,16 @@ virDomainRNGDefFormat(virBufferPtr buf, break; } + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferCheckError(&driverBuf) < 0) + return -1; + + if (virBufferUse(&driverBuf)) { + virBufferAddLit(buf, "<driver"); + virBufferAddBuffer(buf, &driverBuf); + virBufferAddLit(buf, "/>\n"); + } + if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) { if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; @@ -23232,6 +23250,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 acb6748..4025874 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2020,6 +2020,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 33da214..b7b9501 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -80,6 +80,7 @@ </memballoon> <rng model='virtio'> <backend model='random'>/dev/random</backend> + <driver iommu='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> </rng> </devices> -- 2.10.2

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 12 ++++++++++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 27 ++++++++++++++++++++-- src/conf/domain_conf.h | 1 + .../qemuxml2argv-virtio-options.xml | 1 + 5 files changed, 44 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fd2195a..d7af60f 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 c7a0867..0955fc8 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 c6f5d93..d3bbbac 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2352,6 +2352,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def) virDomainDeviceInfoClear(&def->info); VIR_FREE(def->accel); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -13525,11 +13526,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; @@ -13538,6 +13541,8 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *vgamem = NULL; char *primary = NULL; + ctxt->node = node; + if (VIR_ALLOC(def) < 0) return NULL; @@ -13639,7 +13644,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); @@ -14438,7 +14448,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: @@ -18373,7 +18383,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) { @@ -19440,6 +19450,10 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, } } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -23376,6 +23390,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, @@ -23385,6 +23400,14 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, "<video>\n"); virBufferAdjustIndent(buf, 2); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferCheckError(&driverBuf) < 0) + return -1; + if (virBufferUse(&driverBuf)) { + 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 4025874..a692c32 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1376,6 +1376,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 b7b9501..85d1145 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -69,6 +69,7 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <video> + <driver iommu='on' ats='on'/> <model type='virtio' heads='1' primary='yes'> <acceleration accel3d='yes'/> </model> -- 2.10.2

https://bugzilla.redhat.com/show_bug.cgi?id=1283251 --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml | 4 ++++ 5 files changed, 35 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d7af60f..52119f0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5715,6 +5715,13 @@ 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> + <h4><a name="elementsHub">Hub devices</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0955fc8..4950ddc 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 d3bbbac..5c32f93 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1400,6 +1400,7 @@ void virDomainInputDefFree(virDomainInputDefPtr def) virDomainDeviceInfoClear(&def->info); VIR_FREE(def->source.evdev); + VIR_FREE(def->virtio); VIR_FREE(def); } @@ -11614,6 +11615,9 @@ virDomainInputDefParseXML(const virDomainDef *dom, goto error; } + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0) + goto error; + cleanup: VIR_FREE(evdev); VIR_FREE(type); @@ -19354,6 +19358,10 @@ virDomainInputDefCheckABIStability(virDomainInputDefPtr src, return false; } + if (src->virtio && dst->virtio && + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) + return false; + if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) return false; @@ -23448,6 +23456,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 && @@ -23471,6 +23480,15 @@ virDomainInputDefFormat(virBufferPtr buf, type, bus); virBufferAdjustIndent(&childbuf, virBufferGetIndent(buf, false) + 2); + virDomainVirtioOptionsFormat(&driverBuf, def->virtio); + if (virBufferCheckError(&driverBuf) < 0) + return -1; + + if (virBufferUse(&driverBuf)) { + 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 a692c32..e67b6fd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1282,6 +1282,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 85d1145..773038a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.xml @@ -54,15 +54,19 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </interface> <input type='mouse' bus='virtio'> + <driver iommu='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </input> <input type='keyboard' bus='virtio'> + <driver iommu='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> </input> <input type='tablet' bus='virtio'> + <driver iommu='on' ats='on'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> </input> <input type='passthrough' bus='virtio'> + <driver iommu='on' ats='on'/> <source evdev='/dev/input/event1234'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> </input> -- 2.10.2

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 | 41 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 9 +++++ 2 files changed, 50 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..fa12347 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args @@ -0,0 +1,41 @@ +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 \ +-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-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 Wed, Jun 07, 2017 at 09:25:03PM +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 | 41 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 9 +++++ 2 files changed, 50 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args
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 | 37 ++++++++------ tests/qemuxml2argvtest.c | 4 +- 6 files changed, 98 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7da0988..c0c39bd 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 cae1e17..75027c4 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 != VIR_TRISTATE_SWITCH_ABSENT) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the iommu setting is not supported " + "with this QEMU binary")); + return -1; + } + virBufferAsprintf(buf, ",iommu_platform=%s", + virTristateSwitchTypeToString(virtio->iommu)); + } + 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 fa12347..b53f1ba 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-options.args @@ -16,26 +16,35 @@ 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 \ -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-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 Wed, Jun 07, 2017 at 09:25:04PM +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 | 37 ++++++++------ tests/qemuxml2argvtest.c | 4 +- 6 files changed, 98 insertions(+), 17 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Ján Tomko
-
Pavel Hrdina