On 01/12/2011 11:45 PM, 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.
s/elemnent/element/
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.
Added to my list of things to remind you about, if you forget :)
+ unsigned long sndbuf;
+ if (virXPathULong("string(./tune/sndbuf)", ctxt, &sndbuf) >= 0) { + def->tune.sndbuf = sndbuf;
Hmm, we don't have virXPathInt or virXPathUInt. Type mismatch. If sndbuf > INT_MAX, we've got issues. Should def->tune.sndbuf be unsigned? If so...
@@ -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);
this changes to %lu
+ struct { + int sndbuf_specified : 1; + int sndbuf;
and this line changes
@@ -1662,8 +1665,11 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, type_sep, net->info.alias); }
- if (vhostfd && *vhostfd) { - virBufferVSprintf(&buf, ",vhost=on,vhostfd=%s", vhostfd); + if (is_tap) { + if (vhostfd && *vhostfd) + virBufferVSprintf(&buf, ",vhost=on,vhostfd=%s", vhostfd); + if (net->tune.sndbuf_specified) + virBufferVSprintf(&buf, ",sndbuf=%d", net->tune.sndbuf);
%lu
def->backend = VIR_DOMAIN_NET_BACKEND_TYPE_QEMU; } def->backend_specified = 1; + } else if (STREQ(keywords[i], "sndbuf") && values[i]) { + if (virStrToLong_i(values[i], NULL, 10, &def->tune.sndbuf) < 0) {
Oh, we don't have virStrToLong_ul. How odd. Maybe we should do a prerequisite patch that rounds out the type system so that both xml.h and util.h have a matching set of functions (for every type we can parse out of xml, we also have a way to parse it out of a string), so that you can pick the best type to use. At any rate, 'unsigned long' may be overkill, but I do think this should use 'unsigned int' rather than int in the in-memory representation. I'll go ahead and propose a quickie patch to fix the missing utility functions, then we can do a v2 of this on top of that with a sane type throughout. But other than the type mismatch and one spelling nit, I like the approach of this patch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org