Re: [libvirt] [PATCH v3] qemu: Support numad

On 03/08/2012 06:36 AM, Osier Yang wrote: > numad is an user-level daemon that monitors NUMA topology and > processes resource consumption to facilitate good NUMA resource > alignment of applications/virtual machines to improve performance > and minimize cost of remote memory latencies. It provides a > pre-placement advisory interface, so significant processes can > be pre-bound to nodes with sufficient available resources. Better late than never, but here's a few things you might want to tweak: > +#if defined(NUMAD) > +static char * > +qemuGetNumadAdvice(virDomainDefPtr def) > +{ > + virCommandPtr cmd = NULL; > + char *args = NULL; > + char *output = NULL; > + > + if (virAsprintf(&args, "%d:%lu", def->vcpus, def->mem.cur_balloon)< 0) { > + virReportOOMError(); > + goto out; > + } This prints into a temporary, > + cmd = virCommandNewArgList(NUMAD, "-w", args, NULL); only to then copy it to cmd and have to free the temporary. It's fewer lines of code, and less malloc pressure, to avoid the temporary by printing directly into cmd: cmd = virCommandNewArgList(NUMAD, "-w", NULL); virCommandAddArgFormat(&cmd, "%d:%llu", def->vcpus, def->mem.cur_balloon);

> + if (vm->def->cpumask) { > + /* XXX why don't we keep 'cpumask' in the libvirt cpumap > + * format to start with ?!?! */ > + for (i = 0 ; i< maxcpu&& i< vm->def->cpumasklen ; i++) > + if (vm->def->cpumask[i]) > + VIR_USE_CPU(cpumap, i); This comment just verbalizes what has already been bothering me. It would be really nice if someone could convert all of our internal uses of cpumask (with its inefficient one-byte-per-cpu representation) into virBitmask(), as well as making virBitmask provide convenience functions for converting to and from our stringized format as well as from the public api cpumap (8 cpus per byte).