Re: [libvirt] [PATCH] Enable tuning of qemu network tap device "sndbuf" size
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)