From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47587) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XaSU6-0001fN-Uu for qemu-devel@nongnu.org; Sat, 04 Oct 2014 12:47:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XaSTx-0007ZR-UP for qemu-devel@nongnu.org; Sat, 04 Oct 2014 12:47:06 -0400 Received: from mail-wi0-x233.google.com ([2a00:1450:400c:c05::233]:43454) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XaSTx-0007ZL-NY for qemu-devel@nongnu.org; Sat, 04 Oct 2014 12:46:57 -0400 Received: by mail-wi0-f179.google.com with SMTP id d1so1306005wiv.6 for ; Sat, 04 Oct 2014 09:46:56 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5430247B.6080706@redhat.com> Date: Sat, 04 Oct 2014 18:46:51 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-20-git-send-email-dgilbert@redhat.com> In-Reply-To: <1412358473-31398-20-git-send-email-dgilbert@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: "Dr. David Alan Gilbert (git)" , qemu-devel@nongnu.org Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, lilei@linux.vnet.ibm.com, quintela@redhat.com, cristian.klein@cs.umu.se, amit.shah@redhat.com, yanghy@cn.fujitsu.com Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto: > > +/* These are ORable flags */ ... make them an "enum". > +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? 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... Is it by chance that they are only used in savevm.c? Should they be moved to a header file? > > + 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. Paolo