From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56996) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYIFA-0004am-TM for qemu-devel@nongnu.org; Wed, 18 Mar 2015 13:59:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYIF6-000218-04 for qemu-devel@nongnu.org; Wed, 18 Mar 2015 13:59:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51064) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYIF5-00020V-PC for qemu-devel@nongnu.org; Wed, 18 Mar 2015 13:58:55 -0400 Date: Wed, 18 Mar 2015 17:58:40 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20150318175840.GK2355@work-vm> References: <1424883128-9841-1-git-send-email-dgilbert@redhat.com> <1424883128-9841-27-git-send-email-dgilbert@redhat.com> <20150313051920.GE11973@voom.redhat.com> <20150313134753.GL2486@work-vm> <20150316063002.GH5741@voom.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150316063002.GH5741@voom.redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 26/45] Postcopy page-map-incoming (PMI) structure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, quintela@redhat.com, qemu-devel@nongnu.org, amit.shah@redhat.com, pbonzini@redhat.com, yanghy@cn.fujitsu.com * David Gibson (david@gibson.dropbear.id.au) wrote: > On Fri, Mar 13, 2015 at 01:47:53PM +0000, Dr. David Alan Gilbert wrote: > > * David Gibson (david@gibson.dropbear.id.au) wrote: > > > On Wed, Feb 25, 2015 at 04:51:49PM +0000, Dr. David Alan Gilbert (git) wrote: > > > > From: "Dr. David Alan Gilbert" > [snip] > > > > + mis->postcopy_pmi.state0[BIT_WORD(bitmap_index)] |= shifted_mask; > > > > + } else { > > > > + mis->postcopy_pmi.state0[BIT_WORD(bitmap_index)] &= ~shifted_mask; > > > > + } > > > > + if (state & 2) { > > > > + mis->postcopy_pmi.state1[BIT_WORD(bitmap_index)] |= shifted_mask; > > > > + } else { > > > > + mis->postcopy_pmi.state1[BIT_WORD(bitmap_index)] &= ~shifted_mask; > > > > + } > > > > +} > > > > + > > > > +/* > > > > + * Retrieve the state of the given page > > > > + * Note: This version for use by callers already holding the lock > > > > + */ > > > > +static PostcopyPMIState postcopy_pmi_get_state_nolock( > > > > + MigrationIncomingState *mis, > > > > + size_t bitmap_index) > > > > +{ > > > > + bool b0, b1; > > > > + > > > > + b0 = test_hpbits(mis, bitmap_index, mis->postcopy_pmi.state0); > > > > + b1 = test_hpbits(mis, bitmap_index, mis->postcopy_pmi.state1); > > > > + > > > > + return (b0 ? 1 : 0) + (b1 ? 2 : 0); > > > > > > Ugh.. this is a hidden dependency on the PostcopyPMIState enum > > > elements never changing value. Safer to code it as: > > > if (!b0 && !b1) { > > > return POSTCOPY_PMI_MISSING; > > > } else if (...) > > > ... > > > > > > and let gcc sort it out. > > > > Again, I was trying to make this just the interface; so it doesn't > > know or care about the enum mapping; we can change the enum mapping to > > the bits without changing this function (or the callers) at all. > > So.. I'm not entirely clear what you mean by that. I think what > you're saying is that this function basically returns an arbitrary bit > pattern derived from the state maps, and the enum provides the mapping > from those bit patterns to meaningful states? > > That's.. subtle :/. I'm saying that I'd like everywhere to work in terms of the enum; but since I can't store the array of enums I need to convert somewhere; if I can keep the conversion to only being a couple of functions that know about the bit layout and everything else uses those functions, then it feels safe/clean. Dave > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK