[libvirt] [PATCH v3] qemu: Support numad

[Osier Yang]

On 03/16/2012 06:12 AM, Eric Blake wrote:
> 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).

I will do these work in seperate patches. Thanks.

Osier