On 12/19/2011 10:16 AM, Daniel P. Berrange wrote:
On Thu, Dec 15, 2011 at 06:50:14PM +0800, Hu Tao wrote:
> This series adds a new command numatune to get/set numatune parameters.
>
> Besides libnuma, cpuset cgroup parameters are also set according to
> numatune parameters. But for now, only cpuset.mems is supported.
I've not done a detailed review, but in general I like the design
and impl of this series, as compared to the previous v1/v2 posting.
I still think we need a v5, containing my various improvements (which
I'll post as v4), and addressing these additional concerns:
Is tools/virsh.c allowed to directly use conf/domain_conf.h, or must we
move the string conversion of numa mode into src/util?
Where do we guarantee that the user cannot specify more cpus than the
host has available? In other words, 'virsh numatune dom --nodeset 0-15'
should probably fail on a host that doesn't have 16 processors.
What about my concern that altering the nodeset of a running domain
needs to affect dumpxml for that domain?
Also, my testing revealed that the patch series is buggy, but I haven't
yet debugged why (probably something wrong with the manual rpc code)
# tools/virsh numatune dom
error: Unable to get numa parameters
error: Unable to encode message payload
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org