[libvirt] [PATCH v2 01/12] Introduce /domain/devices/interface/driver/@queues attribute

[Laine Stump]

On 05/14/2013 06:10 AM, Michal Privoznik wrote:
> On 14.05.2013 11:33, Michal Privoznik wrote:
>> On 13.05.2013 22:38, Laine Stump wrote:
>>> On 05/13/2013 01:22 PM, Michal Privoznik wrote:
>>>> This attribute is going to represent number of queues for
>>>> multique vhost network interface. This commit implements XML
>>>> extension part of the feature and add one test as well. For now,
>>>> we can only do xml2xml test as qemu command line generation code
>>>> is not adapted yet.
>>>> ---
>>>>  docs/formatdomain.html.in                          | 11 ++++-
>>>>  docs/schemas/domaincommon.rng                      |  5 +++
>>>>  src/conf/domain_conf.c                             | 15 +++++++
>>>>  src/conf/domain_conf.h                             |  1 +
>>>>  src/qemu/qemu_domain.c                             | 27 +++++++-----
>>>>  tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml  |  2 +-
>>>>  .../qemuxml2argv-net-virtio-device.xml             |  2 +-
>>>>  .../qemuxml2argvdata/qemuxml2argv-vhost_queues.xml | 51 ++++++++++++++++++++++
>>>>  tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml |  2 +-
>>>>  tests/qemuxml2xmltest.c                            |  1 +
>>>>  10 files changed, 103 insertions(+), 14 deletions(-)
>>>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml
>>>>
>>>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>>>> index 8004428..d671491 100644
>>>> --- a/docs/schemas/domaincommon.rng
>>>> +++ b/docs/schemas/domaincommon.rng
>>>> @@ -1990,6 +1990,11 @@
>>>>                  </attribute>
>>>>                </optional>
>>>>                <optional>
>>>> +                <attribute name='queues'>
>>>> +                  <ref name="positiveInteger"/>
>>>
>>> Should a lower limit be put on it in the RNG? (does qemu have a
>>> documented limit?)
>> I don't think so. QEMU doesn't have anything documented, but they are
>> using uint16_t to store the max_queues internally. However, when setting
>> it in virtio_net_device_init they use:
>>
>> n->max_queues = MAX(n->nic_conf.queues, 1);
>>
>> where n->nic_conf.queues is int32_t. Maybe we should ask on the qemu list.
>>
> D'oh! So I've just asked a kernel guy privately and he says, kernel has
> the limit of 8 queues. But anyway, do we need to enforce this limit or
> should we let kernel to report an error instead? What if: we introduce
> the limit, but kernel lifts the limit someday? Users are effectively
> forced to update the libvirt, and we are forced to keep an eye on kernel
> if the limit has changed or not (any volunteer?). Therefore I vote for
> keeping this part as is. I don't think putting another check point is
> desired.

If someone enters "queues='2345987234'" will the kernel report the
failure when we attempt to open the 9th fd? Or would be be allowed to
open fds until some more catastrophic failure happened (i.e. we reach
the open fds limit for libvirtd)? If the former, then it's probably okay
to not impose any artificial limit, and let the error be exposed
organically. If not, then we may need to impose some limit that is so
high it would never be reached, but still low enough that reaching it
wouldn't put us into a DoS situation.