From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47166) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWNmJ-0007Ol-7u for qemu-devel@nongnu.org; Mon, 31 Aug 2015 08:01:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWNmF-0000cp-5K for qemu-devel@nongnu.org; Mon, 31 Aug 2015 08:01:35 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:47721) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWNmE-0000Ye-5Q for qemu-devel@nongnu.org; Mon, 31 Aug 2015 08:01:31 -0400 References: <1438159544-6224-1-git-send-email-zhang.zhanghailiang@huawei.com> <1438159544-6224-32-git-send-email-zhang.zhanghailiang@huawei.com> <55E0E01E.5000708@redhat.com> From: zhanghailiang Message-ID: <55E441FB.1080408@huawei.com> Date: Mon, 31 Aug 2015 20:00:59 +0800 MIME-Version: 1.0 In-Reply-To: <55E0E01E.5000708@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v8 31/34] COLO: Add colo-set-checkpoint-period command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org, Markus Armbruster Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, dgilbert@redhat.com, arei.gonglei@huawei.com, amit.shah@redhat.com, Luiz Capitulino On 2015/8/29 6:26, Eric Blake wrote: > On 07/29/2015 02:45 AM, zhanghailiang wrote: >> With this command, we can control the period of checkpoint, if >> there is no comparison of net packets. >> >> Cc: Luiz Capitulino >> Cc: Eric Blake >> Cc: Markus Armbruster >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian >> --- >> hmp-commands.hx | 15 +++++++++++++++ >> hmp.c | 7 +++++++ >> hmp.h | 1 + >> migration/colo.c | 11 ++++++++++- >> qapi-schema.json | 13 +++++++++++++ >> qmp-commands.hx | 22 ++++++++++++++++++++++ >> stubs/migration-colo.c | 4 ++++ >> 7 files changed, 72 insertions(+), 1 deletion(-) > > Interface review: > >> +++ b/qapi-schema.json >> @@ -691,6 +691,19 @@ >> { 'command': 'colo-lost-heartbeat' } >> >> ## >> +# @colo-set-checkpoint-period >> +# >> +# Set colo checkpoint period >> +# >> +# @value: period of colo checkpoint in ms >> +# >> +# Returns: nothing on success > > Redundant line; you could omit it. > >> +# >> +# Since: 2.4 > > 2.5 > >> +## >> +{ 'command': 'colo-set-checkpoint-period', 'data': {'value': 'int'} } > > I hate write-only interfaces; where can I query the current period? > Yes, it is not graceful, actually, this should be convert to use migrate-set-parameters/query-migrate-parameters commands to set/get the value, just as Dave's "[RFC/COLO: 1/3] COLO: Hybrid mode" patch does. But these two commands should be reconstruct. Markus has promised to do this. Hi Markus, is it still in your schedule ? I will convert command to use migrate-set-parameters/query-migrate-parameters temporarily for next version. thanks. >> +++ b/stubs/migration-colo.c >> @@ -52,3 +52,7 @@ void qmp_colo_lost_heartbeat(Error **errp) >> " with --enable-colo option in order to support" >> " COLO feature"); >> } >> + >> +void qmp_colo_set_checkpoint_period(int64_t value, Error **errp) >> +{ >> +} > > Shouldn't the stub function set an error, rather than being a no-op? > Yes, we should set an error, will fix in next version, thanks.