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

[Michal Privoznik]

On 14.05.2013 13:59, Laine Stump wrote:
> 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.

The former one. Kernel shouts:

2013-05-14 10:04:44.370+0000: 12723: error : virNetDevTapCreate:202 :
Unable to create tap device vnet0: Argument list too long

So I think we're safe here.

Michal