[libvirt] [PATCHv2 06/15] Add virtio-related options to interfaces
Pavel Hrdina
phrdina at redhat.com
Wed Jun 7 13:43:53 UTC 2017
On Tue, Jun 06, 2017 at 01:36:20PM +0200, Ján Tomko wrote:
> <interface type='user'>
> <mac address='52:54:56:5a:5c:5e'/>
> <model type='virtio'/>
> <driver iommu_platform='on' ats='on'/>
> </interface>
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1283251
> ---
> docs/formatdomain.html.in | 19 +++++++
> docs/schemas/domaincommon.rng | 12 +++++
> src/conf/domain_conf.c | 63 ++++++++++++++++++++++
> src/conf/domain_conf.h | 19 +++++++
> .../qemuxml2argv-virtio-options.xml | 2 +
> .../qemuxml2xmlout-virtio-options.xml | 2 +
> 6 files changed, 117 insertions(+)
I would spit this patch into two patches, one that introduces the virtio
related options and second one that introduces this element to the
interfaces.
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 2f1e030..dcc2e5e 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3451,6 +3451,19 @@
> </dd>
> </dl>
>
> + <h4><a name="elementsVirtio">Virtio-related options</a></h4>
> +
> + <p>
> + QEMU's virtio devices have some attributes related to the virtio transport under
> + the <code>driver</code> element:
> + The <code>iommu_platform</code> attribute enables the use of emulated IOMMU
I think that we tend to use camelCase for attributes and elements.
Isn't "iommuEnabled" or just "iommu", we don't need to follow QEMU
naming, it should be something generic.
> + by the device. The attribute <code>ats</code> controls the Address
> + Translation Service support for PCIe devices. This is needed to make use
> + of IOTLB support (see <a href="#elementsIommu">IOMMU device</a>).
> + Possible values are <code>on</code> or <code>off</code>.
> + <span class="since">Since 3.5.0</span>
> + </p>
> +
> <h4><a name="elementsControllers">Controllers</a></h4>
>
> <p>
> @@ -5142,6 +5155,12 @@ qemu-kvm -net nic,model=? /dev/null
> <b>In general you should leave this option alone, unless you
> are very certain you know what you are doing.</b>
> </dd>
> + <dt>virtio options</dt>
> + <dd>
> + For virtio interfaces,
> + <a href="#elementsVirtio">Virtio-specific options</a> can also be
> + set. (<span class="since">Since 3.5.0</span>)
> + </dd>
> </dl>
> <p>
> Offloading options for the host and guest can be configured using
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 6c3e885..d7f3b02 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2686,6 +2686,7 @@
> </optional>
> </group>
> </choice>
> + <ref name="virtioOptions"/>
> <interleave>
> <optional>
> <element name='host'>
> @@ -5006,6 +5007,17 @@
> </element>
> </define>
>
> + <define name="virtioOptions">
> + <optional>
> + <attribute name="iommu_platform">
> + <ref name="virOnOff"/>
> + </attribute>
> + <attribute name="ats">
> + <ref name="virOnOff"/>
> + </attribute>
> + </optional>
> + </define>
> +
> <define name="usbmaster">
> <element name="master">
> <attribute name="startport">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 89c8917..ef3383d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1105,6 +1105,46 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt)
> return &xmlopt->ns;
> }
>
> +static int
> +virDomainVirtioOptionsParseXML(xmlXPathContextPtr ctxt,
> + virDomainVirtioOptionsPtr *virtio)
> +{
> + char *str = NULL;
> + int ret = -1;
> + int val;
> + virDomainVirtioOptionsPtr res;
> +
> + if (VIR_ALLOC(*virtio) < 0)
> + return -1;
> +
> + res = *virtio;
> +
> + if ((str = virXPathString("string(./driver/@iommu_platform)", ctxt))) {
> + if ((val = virTristateSwitchTypeFromString(str)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("invalid iommu_platform value"));
> + goto cleanup;
> + }
> + res->iommu_platform = val;
> + }
> + VIR_FREE(str);
> +
> + if ((str = virXPathString("string(./driver/@ats)", ctxt))) {
> + if ((val = virTristateSwitchTypeFromString(str)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("invalid ats value"));
> + goto cleanup;
> + }
> + res->ats = val;
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(str);
> + return ret;
> +}
> +
>
> void
> virBlkioDeviceArrayClear(virBlkioDevicePtr devices,
> @@ -1952,6 +1992,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
> VIR_FREE(def->ifname);
> VIR_FREE(def->ifname_guest);
> VIR_FREE(def->ifname_guest_actual);
> + VIR_FREE(def->virtio);
>
> virNetDevIPInfoClear(&def->guestIP);
> virNetDevIPInfoClear(&def->hostIP);
> @@ -5221,6 +5262,24 @@ virDomainDefValidate(virDomainDefPtr def,
> }
>
>
> +static void
> +virDomainVirtioOptionsFormat(virBufferPtr buf,
> + virDomainVirtioOptionsPtr virtio)
> +{
> + if (!virtio)
> + return;
> +
> + if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) {
> + virBufferAsprintf(buf, "iommu_platform='%s' ",
> + virTristateSwitchTypeToString(virtio->iommu_platform));
> + }
> + if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
> + virBufferAsprintf(buf, "ats='%s' ",
> + virTristateSwitchTypeToString(virtio->ats));
> + }
> +}
> +
> +
> /* Generate a string representation of a device address
> * @info address Device address to stringify
> */
> @@ -10367,6 +10426,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
> goto error;
> }
>
> + if (virDomainVirtioOptionsParseXML(ctxt, &def->virtio) < 0)
> + goto error;
> +
> cleanup:
> ctxt->node = oldnode;
> VIR_FREE(macaddr);
> @@ -22104,6 +22166,7 @@ virDomainVirtioNetDriverFormat(char **outstr,
> virBufferAsprintf(&buf, "rx_queue_size='%u' ",
> def->driver.virtio.rx_queue_size);
>
> + virDomainVirtioOptionsFormat(&buf, def->virtio);
> virBufferTrim(&buf, " ", -1);
>
> if (virBufferCheckError(&buf) < 0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 7d1f05c..a17f217 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -155,6 +155,17 @@ typedef virDomainTPMDef *virDomainTPMDefPtr;
> typedef struct _virDomainIOMMUDef virDomainIOMMUDef;
> typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
>
> +typedef struct _virDomainVirtioOptions virDomainVirtioOptions;
> +typedef virDomainVirtioOptions *virDomainVirtioOptionsPtr;
> +
> +typedef enum {
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_DEFAULT,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_LEGACY,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_TRANSITIONAL,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_MODERN,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_LAST,
> +} virDomainDriverCompatibility;
This isn't used anywhere in the code and in the remaining patches,
isn't it just some leftover?
Pavel
-------------- 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/20170607/b449272d/attachment-0001.sig>
More information about the libvir-list
mailing list