[libvirt] [PATCH 2/2] Add tx_alg attribute to interface XML for virtio backend
Laine Stump
laine at laine.org
Mon Feb 7 20:56:58 UTC 2011
On 02/07/2011 12:06 PM, Eric Blake wrote:
> On 02/04/2011 02:00 PM, Laine Stump wrote:
>> qemu's virtio-net-pci driver allows setting the algorithm used for tx
>> packets to either "bh" or "timer". I don't know exactly how these
>> algorithms differ, but someone has requested the ability to choose
>> between them in libvirt's domain XML. See:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=629662
> Same as qemu aio - we don't have to know exactly what the difference is,
> to know that someone else who does know the difference wants to be able
> to choose :)
>
>> <interface ...>
>> ...
>> <model type='virtio'/>
>> <driver tx_alg='bh'/>
>> ...
>> </interface>
>>
>> I chose to put this setting as an attribute to<driver> rather than as
>> a sub-element to<tune> because it is specific to the virtio-net
>> driver, not something that is generally usable by all network drivers.
>> (note that this is the same placement as the "driver name=..."
>> attribute used to choose kernel vs. userland backend for the
>> virtio-net driver.)
> I take it that tx_alg applies to both options of driver name=... when
> using a virtio interface, right?
I assume so, but know little beyond the name of the option :-)
>> This is a bit troublesome to me, because I can see
>> lots of new virtio options that could potentially be requested (just
>> run "qemu-kvm -device virtio-net-pci,?" on a qemu that's version
>> 0.13.0 or newer, and compare that output to potential tunable items in
>> "-device e1000,?" or "-net tap,..."), so the attribute list could
>> potentially get quite long (which is something I was concerned about
>> when I first added the option to select kernel vs. userland backend,
>> but didn't realize just how many virtio-specific options there were).
> I'd like feedback from danpb or DV, since this might be a long-term XML
> commitment.
Me too! :-)
> Maybe it makes sense to introduce sub-elements of<driver>,
> according to the driver chosen?
>
> <interface ...>
> ...
> <driver>
> <tunable name='tx_alg'>bh</tunable>
> </driver>
> </interface>
>
> But I'm not sure if that is any better.
Well, there is already a <tune> element inside <interface> that works
similar to <memtune> and <cputune>:
<interface ...>
<tune>
<sndbuf>0</sndbuf>
<tunable2>100</tunable2>
</tune>
...
</interface>
One alternative would be to place tx_alg there, and ignore it when model
type != 'virtio'. That seems a bit cheesy, though. Another alternative
would be to put a "<tune> element inside <driver> that looks just like
memtune, cputune, and interface/tune.
Again, the issue here is concern over the attribute list of <driver>
getting excessively long. If that's not an issue, then leaving tx_alg as
a simple attribute will probably be fine.
>> If the option isn't listed there, the config item is ignored (should
>> it instead generate a warning log? error out and prevent the domain
>> from starting?)
> I would lean towards erroring out, the same way that we error out if:
>
> <driver name='vhost'/>
>
> is explicitly requested when the kernel module is not present.
Okay. I'll make it that way in the next revision.
[...]
>> + virBufferVSprintf(&buf, ",tx=%s",
>> + virDomainNetVirtioTxAlgTypeToString(net->driver.virtio.tx_alg));
>> + } else {
>> + /* What should we do if the option isn't available? log a
>> + * warning? prevent the domain from starting? Ignore it?
>> + * Right now we're ignoring it.
>> + */
> This would be the perfect place to error out with
> VIR_ERR_CONFIG_UNSUPPORTED.
>
More information about the libvir-list
mailing list