On Mon, Dec 19, 2011 at 05:39:10PM -0700, Eric Blake wrote:
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?
Maybe a later patch to do this because there are a lot of unrelated
type-string conversion codes?
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.
I think it's fine to set nodeset in domain's xml to any value valid the
user wants. But in the --live case, setting a nodeset exceeds the real
num of cpus should fail.
What about my concern that altering the nodeset of a running domain
needs to affect dumpxml for that domain?
done.
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
The bug is in the rpc generating code which unexpectedly removes a
line of code that shouldn't be. More details see patch 9.
Eric Blake (1):
Fix a bug related to VIR_TYPED_PARAM_STRING
Hu Tao (8):
virsh: add support to VIR_TYPED_PARAM_STRING in vshGetTypedParamValue
Add functions to set/get cgroup cpuset parameters
use cpuset to manage numa
add new API virDomain{G, S}etNumaParameters
Add virDomain{G, S}etNumaParameters support to the remote driver
Implement virDomain{G, S}etNumaParameters for the qemu driver
add new command numatune to virsh
rpc: removes the removal of unused variable i
daemon/remote.c | 85 ++++++++++++--
include/libvirt/libvirt.h.in | 39 ++++++
python/generator.py | 2 +
src/conf/domain_conf.h | 8 --
src/driver.h | 15 +++
src/libvirt.c | 129 ++++++++++++++++++++
src/libvirt_private.syms | 2 +
src/libvirt_public.syms | 6 +
src/qemu/qemu_cgroup.c | 19 +++
src/qemu/qemu_driver.c | 273 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 50 ++++++++
src/remote/remote_protocol.x | 24 ++++-
src/remote_protocol-structs | 22 ++++
src/rpc/genprotocol.pl | 2 +-
src/util/cgroup.c | 32 +++++
src/util/cgroup.h | 3 +
tools/virsh.c | 163 +++++++++++++++++++++++++
tools/virsh.pod | 19 +++
18 files changed, 872 insertions(+), 21 deletions(-)
--
1.7.4.4