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@xxxxxxxxxx +1-919-301-3266 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