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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list