Re: [libvirt] [PATCH] Enable tuning of qemu network tap device "sndbuf" size

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

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.