[libvirt] [PATCH v2 2/3] conf, docs: Add support for coalesce setting(s)

Martin Kletzander mkletzan at redhat.com
Fri Apr 21 11:30:01 UTC 2017


On Fri, Apr 21, 2017 at 11:19:36AM +0200, Michal Privoznik wrote:
>On 04/20/2017 02:21 PM, Martin Kletzander wrote:
>> We are currently parsing only rx_max_coalesced_frames because that's
>> the only value that makes sense for us.  The tun device just added
>> support for this one and the others are only supported by hardware
>> devices which we don't need to worry about as the only way we'd pass
>> those to the domain is using <hostdev/> or <interface type='hostdev'/>.
>> And in those cases the guest can modify the settings itself.
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  docs/formatdomain.html.in                          |  24 ++++
>>  docs/schemas/domaincommon.rng                      | 131 +++++++++++++++++++++
>>  src/conf/domain_conf.c                             |  80 +++++++++++++
>>  src/conf/domain_conf.h                             |   2 +
>>  src/qemu/qemu_domain.c                             |  31 +++++
>>  .../qemuxml2argvdata/qemuxml2argv-net-coalesce.xml |  68 +++++++++++
>>  .../qemuxml2xmlout-net-coalesce.xml                |  71 +++++++++++
>>  tests/qemuxml2xmltest.c                            |   1 +
>>  8 files changed, 408 insertions(+)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-coalesce.xml
>>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index b1e38f00e423..ea64b7fd1193 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -5405,6 +5405,30 @@ qemu-kvm -net nic,model=? /dev/null
>>        <span class="since">Since 3.1.0</span>
>>      </p>
>>
>> +    <h5><a name="coalesce">Coalesce settings</a></h5>
>> +<pre>
>> +...
>> +<devices>
>> +  <interface type='network'>
>> +    <source network='default'/>
>> +    <target dev='vnet0'/>
>> +    <b><coalesce>
>> +      <rx_max_coalesced_frames>5</rx_max_coalesced_frames>
>> +    </coalesce></b>
>> +  </interface>
>> +</devices>
>> +...</pre>
>> +
>> +    <p>
>> +      This element provides means of setting coalesce settings for some
>> +      interface devices (currently only type <code>network</code>
>> +      and <code>bridge</code>.  Currently there is just one sub-element
>> +      named <code>rx_max_coalesced_frames</code> which accepts a non-negative
>> +      integer that specifies the maximum number of packets that will be received
>> +      before an interrupt.
>> +      <span class="since">Since 3.3.0</span>
>> +    </p>
>
>This does not correspond to the schema or tests introduced later in the
>patch.
>

Good catch, thanks for noticing.

>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 705deb39a1bf..cbeebdc56880 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -6772,6 +6772,77 @@ virDomainNetIPInfoParseXML(const char *source,
>>      return ret;
>>  }
>>
>> +
>> +static virNetDevCoalescePtr
>> +virDomainNetDefCoalesceParseXML(xmlNodePtr node,
>> +                                xmlXPathContextPtr ctxt)
>> +{
>> +    virNetDevCoalescePtr ret = NULL;
>> +    xmlNodePtr save = NULL;
>> +    char *str = NULL;
>> +    unsigned long long tmp = 0;
>> +
>> +    save = ctxt->node;
>> +    ctxt->node = node;
>> +
>> +    str = virXPathString("string(./rx/frames/@max)", ctxt);
>> +    if (!str)
>> +        goto cleanup;
>> +
>> +    if (!ret && VIR_ALLOC(ret) < 0)
>> +        goto cleanup;
>> +
>> +    if (virStrToLong_ullp(str, NULL, 10, &tmp) < 0) {
>> +        virReportError(VIR_ERR_XML_DETAIL,
>> +                       _("cannot parse value '%s' for coalesce parameter"),
>> +                       str);
>> +        VIR_FREE(str);
>> +        goto error;
>> +    }
>> +    VIR_FREE(str);
>> +
>> +    if (tmp > UINT32_MAX) {
>> +        virReportError(VIR_ERR_OVERFLOW,
>> +                       _("value '%llu' is too big for coalesce "
>> +                         "parameter, maximum is '%lu'"),
>> +                       tmp, (unsigned long) UINT32_MAX);
>> +        goto error;
>> +    }
>> +    ret->rx_max_coalesced_frames = tmp;
>
>I wonder if we can turn this into a macro (despite some of us hating
>them) because potentially we'll parse all the 32 coalesce attributes (or
>how many there are). But for now, this is okay as is.
>

I did that.  Multiple times.  GET_COAL(rx, frames, max) for example and
I tried various ways how to do that so it's nicely extensible.  But
after a while I gave up because it would be used once and whatever.
Maybe nobody will ever want to add any other parameter...

>> +
>> + cleanup:
>> +    ctxt->node = save;
>> +    return ret;
>> +
>> + error:
>> +    VIR_FREE(ret);
>> +    goto cleanup;
>> +}
>
>Michal
-------------- 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/20170421/5f9d558f/attachment-0001.sig>


More information about the libvir-list mailing list