From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40571) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z515r-0005Re-Ig for qemu-devel@nongnu.org; Tue, 16 Jun 2015 20:20:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z515m-0005Bb-UJ for qemu-devel@nongnu.org; Tue, 16 Jun 2015 20:20:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56229) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z515m-0005BS-Ny for qemu-devel@nongnu.org; Tue, 16 Jun 2015 20:20:34 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 05B6C35C12D for ; Wed, 17 Jun 2015 00:20:33 +0000 (UTC) From: Juan Quintela In-Reply-To: <555CB022.50901@redhat.com> (Eric Blake's message of "Wed, 20 May 2015 10:02:42 -0600") References: <1432136124-24572-1-git-send-email-quintela@redhat.com> <1432136124-24572-3-git-send-email-quintela@redhat.com> <555CB022.50901@redhat.com> Date: Wed, 17 Jun 2015 02:20:31 +0200 Message-ID: <871thbi3sg.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 2/3] migration: create migration event Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: amit.shah@redhat.com, jdenemar@redhat.com, qemu-devel@nongnu.org Eric Blake wrote: > On 05/20/2015 09:35 AM, Juan Quintela wrote: >> We have one argument that tells us what event has happened. >> >> Signed-off-by: Juan Quintela >> --- >> docs/qmp/qmp-events.txt | 16 ++++++++++++++++ >> migration/migration.c | 12 ++++++++++++ >> qapi/event.json | 14 ++++++++++++++ >> 3 files changed, 42 insertions(+) >> >> diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt >> index 4c13d48..3797709 100644 >> --- a/docs/qmp/qmp-events.txt >> +++ b/docs/qmp/qmp-events.txt >> @@ -473,6 +473,22 @@ Example: >> { "timestamp": {"seconds": 1290688046, "microseconds": 417172}, >> "event": "SPICE_MIGRATE_COMPLETED" } >> >> +MIGRATION >> +--------- >> + >> +Emitted when a migration event happens >> + >> +Data: None. >> + >> + - "status": migration status >> + "": error has been ignored > > Uggh. Looking for an empty string is awkward. We are using MigrationStatus from qapi-schema.json, add the comment stating that. > >> + "report": error has been reported to the device >> + "stop": the VM is going to stop because of the error >> + >> +Example: >> + >> +{"timestamp": {"seconds": 1432121972, "microseconds": 744001}, >> + "event": "MIGRATION", "data": {"status": "completed"}} > > The example lists "completed", but the documentation does not mention > it. Might be good to expand the docs to mention all states, and/or point > to the enum definition. See above. > > >> +++ b/qapi/event.json >> @@ -243,6 +243,20 @@ >> { 'event': 'SPICE_MIGRATE_COMPLETED' } >> >> ## >> +# @MIGRATION >> +# >> +# Emitted when a migration event happens >> +# >> +# @status: @MigrationStatus describing the current migration status. >> +# If this field is not returned, no migration process >> +# has been initiated > > Rather than returning an empty string,... > >> +# >> +# Since: 2.4 >> +## >> +{ 'event': 'MIGRATION', >> + 'data': {'status': 'MigrationStatus'}} > > ...this field should be marked optional, as in '*status'. Then in your > callers, you'll have to pass true or false for has_status, so that you > can omit status when there is none. But really, when will this event > ever be omitted if migration has not been initiated? Maybe it is just > bogus documentation that you can return an empty string, as I didn't see > any addition of a call to qapi_event_send_migration() that would pass an > empty string on the wire. So it sounds to me like the interface is > okay, but the documentation is wrong. It is wrong documentation, sorry for the inconvenience. Later, Juan.