From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47178) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhMhA-0001BT-Is for qemu-devel@nongnu.org; Thu, 23 Oct 2014 14:01:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhMh4-00058N-Ar for qemu-devel@nongnu.org; Thu, 23 Oct 2014 14:01:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38402) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhMh3-00058G-UB for qemu-devel@nongnu.org; Thu, 23 Oct 2014 14:01:02 -0400 Date: Thu, 23 Oct 2014 19:00:42 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20141023180042.GH7659@work-vm> References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-17-git-send-email-dgilbert@redhat.com> <543038FD.8030204@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <543038FD.8030204@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 16/47] Return path: Source handling of return path List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, lilei@linux.vnet.ibm.com, quintela@redhat.com, cristian.klein@cs.umu.se, qemu-devel@nongnu.org, amit.shah@redhat.com, yanghy@cn.fujitsu.com * Paolo Bonzini (pbonzini@redhat.com) wrote: > Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto: > > +/* Source side RP state */ > > +struct MigrationRetPathState { > > + uint32_t latest_ack; > > + QemuThread rp_thread; > > + bool error; > > Should the QemuFile be in here? Yes, done. > > +}; > > + > > Also please do not abbrev words, and add a typedef that matches the > struct if it is useful. If it is not, just embed the struct without > giving the type a name (struct { } rp_state). Done. > > > +static bool migration_already_active(MigrationState *ms) > > +{ > > + switch (ms->state) { > > + case MIG_STATE_ACTIVE: > > + case MIG_STATE_SETUP: > > + return true; > > + > > + default: > > + return false; > > + > > + } > > +} > > Should CANCELLING also be considered active? It is on the source->dest > path. Hmm, possibly - but my intention here was just to round up all of the places that already checked for ACTIVE+SETUP so that I could add POSTCOPY_ACTIVE; only one of those places also checked for CANCELLING, so I left it out. > > +static void await_outgoing_return_path_close(MigrationState *ms) > > +{ > > + /* > > + * If this is a normal exit then the destination will send a SHUT and the > > + * rp_thread will exit, however if there's an error we need to cause > > + * it to exit, which we can do by a shutdown. > > + * (canceling must also shutdown to stop us getting stuck here if > > + * the destination died at just the wrong place) > > + */ > > + if (qemu_file_get_error(ms->file) && ms->return_path) { > > + qemu_file_shutdown(ms->return_path); > > + } > > As mentioned early, I think it's simpler to let these function handle > themselves the case where there is no return path, and call them > unconditionally. I still need to think about that. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK