[libvirt] [PATCH 2/3] conf: introduce 'poll_us' attribute for domain interface

[Laine Stump]

On 06/15/2017 10:17 AM, sferdjao at redhat.com wrote:
> From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui at redhat.com>
> 
> In this commit we introduce 'poll_us' attribute which will be passed
> then to the QEMU command line to enable support of busy polling. This
> commit also take into account that attribute to generate the XML.
> 
> Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui at redhat.com>
> ---
>  src/conf/domain_conf.c | 16 ++++++++++++++++
>  src/conf/domain_conf.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0507ec1..803f517 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -9748,6 +9748,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *event_idx = NULL;
>      char *queues = NULL;
>      char *rx_queue_size = NULL;
> +    char *pollus = NULL;

since the option is "poll-us", but you can't have "-" in an identifier
name, this should either be "poll_us", or "pollUS" (there is some amount
of preference for camelCase in C and XML identifier names in libvirt,
but it is by no means universal, so probably best just to go along with
what's done for nearby identifiers in a similar class, e.g. event_idx).

(Of course some purists would say that we shouldn't include the type of
units in the name, but since this number is so specific to a particular
driver, I think it's reasonable to treat it as an opaque setting, named
"poll-us", with no units-type associated with it).

>      char *str = NULL;
>      char *filter = NULL;
>      char *internal = NULL;
> @@ -9921,6 +9922,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  event_idx = virXMLPropString(cur, "event_idx");
>                  queues = virXMLPropString(cur, "queues");
>                  rx_queue_size = virXMLPropString(cur, "rx_queue_size");
> +                pollus = virXMLPropString(cur, "poll_us");
>              } else if (xmlStrEqual(cur->name, BAD_CAST "filterref")) {
>                  if (filter) {
>                      virReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -10318,6 +10320,17 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>              }
>              def->driver.virtio.rx_queue_size = q;
>          }
> +        if (pollus) {
> +            unsigned int us;
> +            if (virStrToLong_uip(pollus, NULL, 10, &us) < 0) {
> +                virReportError(VIR_ERR_XML_DETAIL,
> +                               _("'poll_us' attribute must be positive number: %s"),

"...must be positive" or "...must be a positive integer"

> +                               pollus);
> +                goto error;
> +            }
> +            if (us > 1)
> +                def->driver.virtio.pollus = us;
> +        }
>          if ((str = virXPathString("string(./driver/host/@csum)", ctxt))) {
>              if ((val = virTristateSwitchTypeFromString(str)) <= 0) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -10515,6 +10528,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      VIR_FREE(event_idx);
>      VIR_FREE(queues);
>      VIR_FREE(rx_queue_size);
> +    VIR_FREE(pollus);
>      VIR_FREE(str);
>      VIR_FREE(filter);
>      VIR_FREE(type);
> @@ -22308,6 +22322,8 @@ virDomainVirtioNetDriverFormat(char **outstr,
>      if (def->driver.virtio.rx_queue_size)
>          virBufferAsprintf(&buf, " rx_queue_size='%u'",
>                            def->driver.virtio.rx_queue_size);
> +    if (def->driver.virtio.pollus)
> +        virBufferAsprintf(&buf, " poll_us='%u'", def->driver.virtio.pollus);
>  
>      virDomainVirtioOptionsFormat(&buf, def->virtio);
>  
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index e67b6fd..d97d776 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -967,6 +967,7 @@ struct _virDomainNetDef {
>              virTristateSwitch event_idx;
>              unsigned int queues; /* Multiqueue virtio-net */
>              unsigned int rx_queue_size;
> +            unsigned int pollus; /* busy polling for tap */
>              struct {
>                  virTristateSwitch csum;
>                  virTristateSwitch gso;
> 

Generally I like to have the schema change (RNG file) and
formatdomain.html.in (documentation) change, along with xml2xml tests.
in the same commit as the change to the parser/formatter, but you've
included them in the commit that connects the parser/formatter change
into the qemu driver. Of course it all comes out the same in the end,
but separating it in the way I suggest makes each patch self documenting
and self-testing (xml2argv tests will still be added in the final patch).