Re: [libvirt] [PATCH] qemu: bulk stats: implement (cpu) tune group.
Hi John, thanks for the review!
----- Original Message -----
From: "John Ferlan" <jferlan(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)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