Re: [PATCH 5/6 v3] remote-protocol: Remote protocol implementation of virDomainSetBlkioParameters and virDomainGetBlkioParameters.

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

 



On 02/21/2011 10:34 PM, Gui Jianfeng wrote:
> Remote protocol implementation of virDomainSetBlkioParameters and virDomainGetBlkioParameters.
> 
> Signed-off-by: Gui Jianfeng <guijianfeng@xxxxxxxxxxxxxx>
> ---
>  daemon/remote.c                     |  210 +++++++++++++++++++++++++++++++++++
>  daemon/remote_dispatch_args.h       |    2 +
>  daemon/remote_dispatch_prototypes.h |   16 +++
>  daemon/remote_dispatch_ret.h        |    1 +
>  daemon/remote_dispatch_table.h      |   10 ++
>  src/remote/remote_driver.c          |  173 ++++++++++++++++++++++++++++-
>  src/remote/remote_protocol.c        |   89 +++++++++++++++
>  src/remote/remote_protocol.h        |   55 +++++++++
>  src/remote/remote_protocol.x        |   44 +++++++-
>  src/remote_protocol-structs         |   35 ++++++
>  10 files changed, 632 insertions(+), 3 deletions(-)

ACK, although I had quite the rough time applying this after the
conflicts with virDomainSetMemoryFlags.  I ended up shuffling things to
match shuffles earlier in the series.

> +++ b/src/remote/remote_protocol.x
> @@ -128,6 +128,9 @@ const REMOTE_NWFILTER_NAME_LIST_MAX = 1024;
>  /* Upper limit on list of scheduler parameters. */
>  const REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX = 16;
>  
> +/* Upper limit on list of blkio parameters. */
> +const REMOTE_DOMAIN_blkio_PARAMETERS_MAX = 16;
> +

Wrong case.  How'd you test this?

> @@ -2103,7 +2143,9 @@ enum remote_procedure {
>  
>      REMOTE_PROC_DOMAIN_OPEN_CONSOLE = 201,
>      REMOTE_PROC_DOMAIN_IS_UPDATED = 202,
> -    REMOTE_PROC_GET_SYSINFO = 203
> +    REMOTE_PROC_GET_SYSINFO = 203,
> +    REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS = 204,
> +    REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 205

SET_MEMORY_FLAGS beat you to 204; you get 205 and 206.

Hmm, I guess that completes my review, so I'm pushing it.  Thanks again
for the work!

However, I do have a parting comment that this was a _lot_ of code
duplication; if we ever add another cgroup tunable, it would be worth
the time to first factor out the common elements into something that
memtune, blkio, and that new cgroup tunable could share.  For example,
we don't need three copies of the union for i/ui/l/ul/d/b - that should
be something easy to share between tunables.

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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]