From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56684) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbQbp-0003US-84 for qemu-devel@nongnu.org; Tue, 07 Oct 2014 04:59:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XbQbj-0000KG-3T for qemu-devel@nongnu.org; Tue, 07 Oct 2014 04:59:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15576) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbQbi-0000Il-SH for qemu-devel@nongnu.org; Tue, 07 Oct 2014 04:58:59 -0400 Date: Tue, 7 Oct 2014 09:58:42 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20141007085841.GC2404@work-vm> References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-20-git-send-email-dgilbert@redhat.com> <5430247B.6080706@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5430247B.6080706@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 19/47] Rework loadvm path for subloops 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: > > > > +/* These are ORable flags */ > > ... make them an "enum". OK, will do - I'd generally tended to avoid using enum for things that were ORable where the combinations weren't themselves members of the enum; but I can do that. > > +const int LOADVM_EXITCODE_QUITLOOP = 1; > > +const int LOADVM_EXITCODE_QUITPARENT = 2; > > LOADVM_QUIT_ALL, LOADVM_QUIT respectively? > > +const int LOADVM_EXITCODE_KEEPHANDLERS = 4; > > + > > Is it more common to drop or keep handlers? I'ts more common to drop them. > In either case, please add a comment to the three constants that details > how to use them. In particular, please document why you should drop > (resp. keep) handlers... Does this make it clearer: /* ORable flags that control the (potentially nested) loadvm_state loops */ enum LoadVMExitCodes { /* Quit the loop level that received this command */ LOADVM_QUIT_LOOP = 1, /* Quit this loop and our parent */ LOADVM_QUIT_PARENT = 2, /* * Keep the LoadStateEntry handler list after the loop exits, * because they're being used in another thread. */ LOADVM_KEEP_HANDLERS = 4, }; > Is it by chance that they are only used in savevm.c? Should they be > moved to a header file? They're local. > > + if (exitcode & LOADVM_EXITCODE_QUITPARENT) { > > + DPRINTF("loadvm_handlers_state_main: End of loop with QUITPARENT"); > > + exitcode &= ~LOADVM_EXITCODE_QUITPARENT; > > + exitcode &= LOADVM_EXITCODE_QUITLOOP; > > Either you want |=, or the first &= is useless. Ooh nicely spotted; yes that should be |= - now I need to figure out why this didn't break things. The idea is we have: 1 outer loadvm_state loop 2 receives packaged command 3 inner_loadvm_state loop 4 receives handle_listen 5 < QUITPARENT 6 < QUITLOOP 7 < QUITLOOP 8 exits so QUITPARENT causes it's parent to exit, and to do that the inner loop transforms QUITPARENT into QUITLOOP as it's exit. Dave > > Paolo -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK