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

Re: [libvirt] [PATCH 2/6 v3] libvirt: Implements virDomainSetBlkioParameters and virDomainGetBlkioParameters and initialization



On 02/21/2011 10:31 PM, Gui Jianfeng wrote:
> Implements virDomainSetBlkioParameters and virDomainGetBlkioParameters and initialization
> 
> Signed-off-by: Gui Jianfeng <guijianfeng cn fujitsu com>
> ---
>  src/driver.h                |   14 ++++++
>  src/esx/esx_driver.c        |    2 +
>  src/libvirt.c               |  108 +++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms     |    6 ++

Merge conflicts; how fun.

> +++ b/src/driver.h
> @@ -135,6 +135,18 @@ typedef int
>          (*virDrvDomainSetMemory)	(virDomainPtr domain,
>                                           unsigned long memory);
>  typedef int
> +        (*virDrvDomainSetBlkioParameters)

...

> +                                         unsigned int flags);
> +typedef int
>          (*virDrvDomainSetMemoryParameters)
>                                          (virDomainPtr domain,
>                                           virMemoryParameterPtr params,

Maybe it's just me, but I'd rather see SetMemory, SetMemoryFlags,
SetMemoryParameters all in a group, then SetBlkio after that.  I
rearranged your patch to order things differently.

> @@ -615,6 +627,8 @@ struct _virDriver {
>      virDrvDomainSetMemoryParameters domainSetMemoryParameters;
>      virDrvDomainGetMemoryParameters domainGetMemoryParameters;
>      virDrvDomainOpenConsole domainOpenConsole;
> +    virDrvDomainSetBlkioParameters domainSetBlkioParameters;
> +    virDrvDomainGetBlkioParameters domainGetBlkioParameters;

Likewise, here it's nicer to stick the related callbacks next to one
another (SetMemoryFlags, SetMemoryParameters, SetBlkioParameters).

That means I basically rewrote this patch.  Oh well; at least it's a
trivial reordering.

Most of this code benefits from copy-and-paste from MemoryParameters. I
wonder why :)

> +++ b/src/libvirt_public.syms
> @@ -424,4 +424,10 @@ LIBVIRT_0.8.8 {
>          virConnectGetSysinfo;
>  } LIBVIRT_0.8.6;
>  
> +LIBVIRT_0.8.9 {
> +    global:
> +        virDomainSetBlkioParameters;
> +        virDomainGetBlkioParameters;
> +} LIBVIRT_0.8.8;

I fixed this merge conflict, too.

ACK as adjusted; again, I'll push once I finish applying your series
(now that I've adjusted things, I'm sure to have merge conflicts with
several remaining patches to trivially work out).

-- 
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]