From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58884 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P9EBI-0006Vu-84 for qemu-devel@nongnu.org; Fri, 22 Oct 2010 05:45:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P9E0c-0008LN-1C for qemu-devel@nongnu.org; Fri, 22 Oct 2010 05:33:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37043) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P9E0b-0008Kw-Or for qemu-devel@nongnu.org; Fri, 22 Oct 2010 05:33:57 -0400 Date: Fri, 22 Oct 2010 11:27:23 +0200 From: "Michael S. Tsirkin" Message-ID: <20101022092723.GA20827@redhat.com> References: <20101020095616.GD10783@redhat.com> <20101021051524.GB31309@valinux.co.jp> <20101021080701.GB27985@redhat.com> <20101021235315.GG31309@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20101021235315.GG31309@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH v6 08/12] pcie/aer: helper functions for pcie aer capability List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: skandasa@cisco.com, adnan@khaleel.us, etmartin@cisco.com, qemu-devel@nongnu.org, wexu2@cisco.com On Fri, Oct 22, 2010 at 08:53:15AM +0900, Isaku Yamahata wrote: > On Thu, Oct 21, 2010 at 10:07:01AM +0200, Michael S. Tsirkin wrote: > > On Thu, Oct 21, 2010 at 02:15:24PM +0900, Isaku Yamahata wrote: > > > Thank you for detailed review. > > > > > > On Wed, Oct 20, 2010 at 11:56:16AM +0200, Michael S. Tsirkin wrote: > > > > > +static uint32_t aer_log_del(PCIEAERLog *aer_log) > > > > > +{ > > > > > + uint32_t i = aer_log->consumer; > > > > > + aer_log->consumer = aer_log_next(aer_log->consumer, aer_log->log_max); > > > > > + return i; > > > > > +} > > > > > > > > > > > > Please just use 'int' or 'unsigned' instead of uint32_t if you simply > > > > want to say 'a number'. Using specific length makes it impossible to > > > > say where you *really* want a value. > > > > > > PCIEAERLog is saved/loaded. So explicit sized number is chosen. > > > > I didn't notice. But consumer/producer are not guest visible, are they? > > If yes I think it's a mistake to save/load them, tying us to > > a specific implementation: just calculate and save the # of > > valid entries in the log. > > ACIEAERLog and PCIEAERErr themselves are the implementation specific > internal states which is not visible to guest. > So there is no point of arguing that consumer/producer are > specific to implementation. Well the errors can be read out by the guest, so they are potentially guest visible. Thus I think the only thing we want to migrate is the list of errors logged. > > Also I put the comment here but it is a general one: pls go over the > > code, and just take a look at what types you use all over and think > > whether size really matters. In most places it does not, it just must be > > big enough, so use int or unsigned there. It will be much harder for > > others to do so later. > > Will do. > -- > yamahata