From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49357) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBYeh-0005T1-7W for qemu-devel@nongnu.org; Tue, 22 Dec 2015 20:55:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBYec-0001lU-UV for qemu-devel@nongnu.org; Tue, 22 Dec 2015 20:55:55 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:59945) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBYec-0001jy-Bp for qemu-devel@nongnu.org; Tue, 22 Dec 2015 20:55:50 -0500 References: <1450167779-9960-1-git-send-email-zhang.zhanghailiang@huawei.com> <1450167779-9960-26-git-send-email-zhang.zhanghailiang@huawei.com> <56742E4A.2070609@redhat.com> From: Hailiang Zhang Message-ID: <5679FF08.5020703@huawei.com> Date: Wed, 23 Dec 2015 09:55:20 +0800 MIME-Version: 1.0 In-Reply-To: <56742E4A.2070609@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v12 25/38] qmp event: Add event notification for COLO error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, Markus Armbruster , yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, dgilbert@redhat.com, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com, Michael Roth , hongyang.yang@easystack.cn On 2015/12/19 0:03, Eric Blake wrote: > On 12/15/2015 01:22 AM, zhanghailiang wrote: >> If some errors happen during VM's COLO FT stage, it's important to notify the users >> of this event. Together with 'colo_lost_heartbeat', users can intervene in COLO's >> failover work immediately. >> If users don't want to get involved in COLO's failover verdict, >> it is still necessary to notify users that we exited COLO mode. >> >> Cc: Markus Armbruster >> Cc: Michael Roth >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian >> --- >> v11: >> - Fix several typos found by Eric >> >> Signed-off-by: zhanghailiang >> --- > >> +++ b/docs/qmp-events.txt >> @@ -184,6 +184,23 @@ Example: >> Note: The "ready to complete" status is always reset by a BLOCK_JOB_ERROR >> event. >> >> +COLO_EXIT >> +--------- >> + >> +Emitted when VM finishes COLO mode due to some errors happening or >> +at the request of users. >> + >> +Data: >> + >> + - "mode": COLO mode, primary or secondary side (json-string) >> + - "reason": the exit reason, internal error or external request. (json-string) >> + - "error": error message (json-string, operation) > > s/operation/optional/ > May want to word it as: > > - "error": error message for human consumption (json-string, optional) > > to point out that machines shouldn't parse it. > Good idea, i will fix it like that. >> +++ b/migration/colo.c >> @@ -18,6 +18,7 @@ >> #include "qemu/error-report.h" >> #include "qemu/sockets.h" >> #include "migration/failover.h" >> +#include "qapi-event.h" >> >> /* colo buffer */ >> #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) >> @@ -349,6 +350,11 @@ static void colo_process_checkpoint(MigrationState *s) >> out: >> if (ret < 0) { >> error_report("%s: %s", __func__, strerror(-ret)); > > Unrelated: I mentioned in another thread that we may want to start > thinking about adding error_report_errno(); this would be another client. > Hmm, yes, we may need such a helper function. >> +++ b/qapi-schema.json >> @@ -778,6 +778,22 @@ >> 'data': [ 'unknown', 'primary', 'secondary'] } >> >> ## >> +# @COLOExitReason >> +# >> +# The reason for a COLO exit >> +# >> +# @unknown: unknown reason >> +# > > If we never return 'unknown', then it is not worth having it in the enum > (we can always add it later if we find a reason to have it; but adding > it now feels premature if the code base isn't using it). > You are right, it should never happen, i will remove it in next version, thanks. > Otherwise looks okay to me. >