Hi John, thanks for the review! ----- Original Message -----
From: "John Ferlan" <jferlan@redhat.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com Sent: Monday, February 23, 2015 7:33:47 PM Subject: Re: [libvirt] [PATCH] qemu: bulk stats: implement (cpu) tune group.
On 02/11/2015 09:22 AM, Francesco Romani wrote:
Management applications, like oVirt, may need to setup cpu quota limits to enforce QoS for VMs.
For this purpose, management applications also need to check how VMs are behaving with respect to CPU quota. This data is avaialble using the virDomainGetSchedulerParameters API.
This patch adds a new group to bulk stats API to obtain the same information.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1191428 --- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 16 ++++++++ src/qemu/qemu_driver.c | 84 ++++++++++++++++++++++++++++++++++++++++ tools/virsh-domain-monitor.c | 7 ++++ 4 files changed, 108 insertions(+)
In general looks good... There's a few spelling and spacing nits below which I could fix up before pushing for you...
Oops. Will fix, spell-check again and resubmit.
You are missing 'virsh.pod' - something easily added as well.
Will add.
The one question I have is around the switch name (looking for any other thoughts...)
I don't really have strong opinions here, so whatever fits best for you guys should be fine for me.
Should the option be "cpu-tune" instead of "tune-cpu", especially since the name of the function has "*CpuTune"? Or even 'sched-info' to match the 'virsh schedinfo $dom' command? I suppose some day there'd be 'numa-tune' data desired as well, but that's a different issue...
I'm aware (= because we oVirt team plan/want/use them :)) of NUMA and I/O tune information which could be requested in the future. Bests, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani