[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/6] Add missing checks for QEMU domain state in memory parameter code



On 03/18/2011 10:36 AM, Daniel P. Berrange wrote:
> qemuDomainSetMemoryParameters and qemuDomainGetMemoryParameters
> forgot to do a check on virDomainIsActive(), resulting in bogus
> error messages from later parts of their impl
> 
> * src/qemu/qemu_driver.c: Add missing checks on virDomainIsActive()
> ---
>  src/qemu/qemu_driver.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)

In looking at the code, at least qemuSetSchedulerParameters and
qemuDomain{Set,Get}BlkioParameters have the same bug, given the level of
copy and paste between those two APIs.

Maybe qemu_cgroup.h should provide a helper function that does both the
virDomainObjIsActive and virCgroupForDomain check in a single call, to
avoid duplication all over qemu_driver.c?

>  
> +    if (!virDomainObjIsActive (vm)) {

Why the space before (vm)?

This patch seems unrelated to the upload/download block API addition at
hand, but is independently useful.  NACK on this version since it is
incomplete, so looking forward to v2 as a separate thread.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]