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@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.
Agreed. We should extract some common code in the future.
Thanks for rebasing and pushing this seires Eric.
Thanks,
Gui
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list