On 02/21/2011 10:34 PM, Gui Jianfeng wrote:
> Remote protocol implementation of virDomainSetBlkioParameters and 
> virDomainGetBlkioParameters.
> 
> Signed-off-by: Gui Jianfeng <[email protected]>
> ---
>  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   [email protected]    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to