From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL8qO-0005IK-1m for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:10:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UL8qK-0005lM-Nz for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:09:59 -0400 Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:49467) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL8qK-0005l8-H1 for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:09:56 -0400 Received: by mail-wi0-f182.google.com with SMTP id hi18so3025036wib.15 for ; Thu, 28 Mar 2013 02:09:55 -0700 (PDT) Date: Thu, 28 Mar 2013 10:09:52 +0100 From: Stefan Hajnoczi Message-ID: <20130328090952.GA24580@stefanha-thinkpad.redhat.com> References: <1362820866-23426-1-git-send-email-dmitry@daynix.com> <1362820866-23426-6-git-send-email-dmitry@daynix.com> <5153143B.3090507@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH V14 5/5] VMXNET3 device implementation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dmitry Fleytman Cc: Alexander Graf , qemu-devel , Yan Vugenfirer , Anthony Liguori , Gerhard Wiesinger , Paolo Bonzini On Wed, Mar 27, 2013 at 06:36:59PM +0200, Dmitry Fleytman wrote: > On Wed, Mar 27, 2013 at 6:06 PM, Alexander Graf wrote: > > > > > On 27.03.2013, at 16:46, Paolo Bonzini wrote: > > > > > Il 27/03/2013 15:49, Alexander Graf ha scritto: > > >>>> +#if defined(HOST_WORDS_BIGENDIAN) > > >>>> +#define const_cpu_to_le64(x) bswap_64(x) > > >>>> +#define __BIG_ENDIAN_BITFIELD > > >> Ah, sorry, I replied to the wrong version. > > >> > > >> ARE YOU KIDDING ME? BIG ENDIAN BITFIELD? BITFIELDS ARE _IMPLEMENTATION > > SPECIFIC_! > > >> > > >> Can we please revert this whole patch set and send the authors back to > > school? > > > > > > Can we please maintain a decent tone? > > > > > > First, this file comes from Linux. __BIG_ENDIAN_BITFIELD is a Linux > > > #define. No doubt it is wrong to define it based on > > > HOST_WORDS_BIGENDIAN, it is better to use a configure check. But it's > > > not the reason why PPC compilation fails. > > > > No, but it indicates that the code isn't written with portability in mind. > > > > It's simply wrong to define it in the first place. You shouldn't do any > > assumptions how bitfields are laid out in memory / registers. Linux gets > > away with it mostly because it's heavily tied to gcc, but we shouldn't take > > the same assumptions in QEMU code. > > > > First of all I'd like to emphasize that vmxnet3 and pvscsi patches are our > first experience in writing QEMU devices so most probably we miss some > common practices of the project. > The codebase is huge and it takes time to learn things like this. > At the same time we are open for constructive criticism and ready to > improve the code we submit. > > Regarding this specific case, we used an approach from current QEMU code, > namely bitfield definitions technique from slirp/ip.h and slirp/tcp.h files. > We do realize this is not the best way, if there are examples of better > techniques in QEMU codebase please point it out. It may not be obvious since it's part of the source tree but slirp is not a good example of QEMU code. It actually comes from http://slirp.sourceforge.net/. Stefan