11:06 a.m.
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?
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. 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.
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.
@@ -6808,12 +6828,16 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, " <model
type='%s'/>\n",
def->model);
if (STREQ(def->model, "virtio") &&
- def->driver.virtio.name) {
+ (def->driver.virtio.name || def->driver.virtio.tx_alg)) {
virBufferAddLit(buf, " <driver");
if (def->driver.virtio.name) {
virBufferVSprintf(buf, " name='%s'",
virDomainNetBackendTypeToString(def->driver.virtio.name));
}
+ if (def->driver.virtio.tx_alg) {
+ virBufferVSprintf(buf, " tx_alg='%s'",
+
virDomainNetVirtioTxAlgTypeToString(def->driver.virtio.tx_alg));
+ }
virBufferAddLit(buf, "/>\n");
Obviously, this would change if we settle on a different XML representation.
+++ b/src/qemu/qemu_capabilities.c
@@ -1063,6 +1063,7 @@ qemuCapsExtractDeviceStr(const char *qemu,
"-device", "?",
"-device", "pci-assign,?",
"-device", "virtio-blk-pci,?",
+ "-device", "virtio-net-pci,?",
NULL);
virCommandAddEnvPassCommon(cmd);
/* qemu -help goes to stdout, but qemu -device ? goes to stderr. */
@@ -1104,6 +1105,8 @@ qemuCapsParseDeviceStr(const char *str, unsigned long long *flags)
if (strstr(str, "pci-assign.bootindex"))
*flags |= QEMUD_CMD_FLAG_PCI_BOOTINDEX;
}
+ if (strstr(str, "virtio-net-pci.tx="))
+ *flags |= QEMUD_CMD_FLAG_VIRTIO_TX_ALG;
That is just so slick for detecting the new bit! I'm glad I added that
improvement in -device string parsing.
+++ b/src/qemu/qemu_capabilities.h
@@ -92,6 +92,7 @@ enum qemuCapsFlags {
QEMUD_CMD_FLAG_CCID_PASSTHRU = (1LL << 55), /* -device ccid-card-passthru */
QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL << 56), /* newer -chardev spicevmc */
QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL << 57), /* older -device spicevmc*/
+ QEMUD_CMD_FLAG_VIRTIO_TX_ALG = (1LL << 58), /* -device
virtio-net-pci,tx=string */
5 more bits left. Get your patches in now, before we run out of space.
Last one in has to rewrite this to be a bitset :)
virBufferAdd(&buf, nic, strlen(nic));
+ if (usingVirtio && net->driver.virtio.tx_alg) {
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_VIRTIO_TX_ALG) {
+ 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.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org