On Thu, Jan 13, 2011 at 01:45:29AM -0500, Laine Stump wrote:
This is in response to a request in:
https://bugzilla.redhat.com/show_bug.cgi?id=665293
In short, under heavy load, it's possible for qemu's networking to lock up due to the tap device's default 1MB sndbuf being inadequate. adding "sndbuf=0" to the qemu commandline -netdevice option will alleviate this problem (sndbuf=0 actually sets it to 0xffffffff).
Because we must be able to explicitly specify "0" as a value, the standard practice of "0 means not specified" won't work here. Instead, virDomainNetDef also has a sndbuf_specified, which defaults to 0, but is set to 1 if some value was given.
The sndbuf value is put inside a <tune> element of each <interface> in the domain. The intent is that further tunable settings will also be placed inside this elemnent.
<interface type='network'> ... <tune> <sndbuf>0</sndbuf> ... </tune> </interface> ---
Note that in qemuBuildHostNetStr() I have moved
if (vhostfd && *vhostfd) { virBufferVSprintf(&buf, ",vhost=on,vhostfd=%s", vhostfd);
into a newly created "if (is_tap) { ... }" block. This always should have been inside such a conditional, but none existed until now. (I can make this a separate patch if anyone wants, but it seemed so simple and obvious that I took the slothenly way out :-)
Also, as with the vhost patch, I didn't get the html docs updated for this addition either. I will add both in a single followup patch next week.
docs/schemas/domain.rng | 10 ++++++++++ src/conf/domain_conf.c | 31 +++++++++++++++++++++++++++++-- src/conf/domain_conf.h | 4 ++++ src/qemu/qemu_command.c | 19 +++++++++++++++++-- 4 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04ed502..5d1b8cf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2280,6 +2280,7 @@ err_exit: static virDomainNetDefPtr virDomainNetDefParseXML(virCapsPtr caps, xmlNodePtr node, + xmlXPathContextPtr ctxt, int flags ATTRIBUTE_UNUSED) { virDomainNetDefPtr def; xmlNodePtr cur; @@ -2298,15 +2299,19 @@ virDomainNetDefParseXML(virCapsPtr caps, char *internal = NULL; char *devaddr = NULL; char *mode = NULL; + unsigned long sndbuf; virNWFilterHashTablePtr filterparams = NULL; virVirtualPortProfileParams virtPort; bool virtPortParsed = false; + xmlNodePtr oldnode = ctxt->node;
if (VIR_ALLOC(def) < 0) { virReportOOMError(); return NULL; }
+ ctxt->node = node; + type = virXMLPropString(node, "type"); if (type != NULL) { if ((int)(def->type = virDomainNetTypeFromString(type)) < 0) { @@ -2593,7 +2598,13 @@ virDomainNetDefParseXML(virCapsPtr caps, } }
+ if (virXPathULong("string(./tune/sndbuf)", ctxt, &sndbuf) >= 0) { + def->tune.sndbuf = sndbuf; + def->tune.sndbuf_specified = 1; + }
This is parsing a 'long'
@@ -6315,6 +6336,12 @@ virDomainNetDefFormat(virBufferPtr buf, VIR_FREE(attrs); }
+ if (def->tune.sndbuf_specified) { + virBufferAddLit(buf, " <tune>\n"); + virBufferVSprintf(buf, " <sndbuf>%d</sndbuf>\n", def->tune.sndbuf); + virBufferAddLit(buf, " </tune>\n"); + }
But this is printing an int
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 451ccad..2d35d68 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -346,6 +346,10 @@ struct _virDomainNetDef { virVirtualPortProfileParams virtPortProfile; } direct; } data; + struct { + int sndbuf_specified : 1; + int sndbuf; + } tune;
And this is storing an int. They should really all be ints, or all be longs. Also bitfields should be 'unsigned' rather than int, otherwise you're actually storing 0 and -1 as possible values, rather than 0 & 1. Or alternatively could just use a 'bool' here. Since there's only one bitfield, padding means we're not saving any space. Regards, Daniel