Hi John, thanks for the review! ----- Original Message ----- > From: "John Ferlan" <jferlan@xxxxxxxxxx> > To: "Francesco Romani" <fromani@xxxxxxxxxx>, libvir-list@xxxxxxxxxx > Sent: Monday, February 23, 2015 7:33:47 PM > Subject: Re: [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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list