[libvirt] [PATCHv3 05/14] Add virtio-related options to interfaces
Pavel Hrdina
phrdina at redhat.com
Thu Jun 8 10:30:15 UTC 2017
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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170608/d7bf558d/attachment-0001.sig>
More information about the libvir-list
mailing list