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

[Gui Jianfeng]

Eric Blake wrote:
> On 02/21/2011 10:34 PM, Gui Jianfeng wrote:
>> Remote protocol implementation of virDomainSetBlkioParameters and virDomainGetBlkioParameters.
>>
>> Signed-off-by: Gui Jianfeng <guijianfeng at cn.fujitsu.com>
>> ---
>>  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.

Agreed. We should extract some common code in the future.
Thanks for rebasing and pushing this seires Eric.

Thanks,
Gui

>