[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 06/15] Add virtio-related options to interfaces



On Tue, Jun 06, 2017 at 01:36:20PM +0200, Ján Tomko wrote:
> <interface type='user'>
>   <mac address='52:54:56:5a:5c:5e'/>
>   <model type='virtio'/>
>   <driver iommu_platform='on' ats='on'/>
> </interface>
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1283251
> ---
>  docs/formatdomain.html.in                          | 19 +++++++
>  docs/schemas/domaincommon.rng                      | 12 +++++
>  src/conf/domain_conf.c                             | 63 ++++++++++++++++++++++
>  src/conf/domain_conf.h                             | 19 +++++++
>  .../qemuxml2argv-virtio-options.xml                |  2 +
>  .../qemuxml2xmlout-virtio-options.xml              |  2 +
>  6 files changed, 117 insertions(+)

[...]

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 89c8917..ef3383d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -5221,6 +5262,24 @@ virDomainDefValidate(virDomainDefPtr def,
>  }
>  
>  
> +static void
> +virDomainVirtioOptionsFormat(virBufferPtr buf,
> +                             virDomainVirtioOptionsPtr virtio)
> +{
> +    if (!virtio)
> +        return;
> +
> +    if (virtio->iommu_platform != VIR_TRISTATE_SWITCH_ABSENT) {
> +        virBufferAsprintf(buf, "iommu_platform='%s' ",
> +                          virTristateSwitchTypeToString(virtio->iommu_platform));
> +    }
> +    if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
> +        virBufferAsprintf(buf, "ats='%s' ",
> +                          virTristateSwitchTypeToString(virtio->ats));
> +    }
> +}

I don't like that we need to call virBufferTrim() every time this
function is used.  Let's switch the format string to " ats='%s'"
and drop all the virBufferTrim() calls from following patches.  It's
pointless to add the space in order to remove it right away.

Some of the calls of virDomainVirtioOptionsFormat() also requires to
call virBufferAddLit(..., "  "), it feels wrong.

The only place that could conflict whit this approach is in
virDomainVirtioNetDriverFormat() where you can easily call
virDomainVirtioOptionsFormat() after the virBufferTrim().

Pavel

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]