From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH] blkif: add indirect descriptors interface to public headers Date: Tue, 12 Nov 2013 14:49:56 +0000 Message-ID: <52824014.8050201@citrix.com> References: <1384252612-22573-1-git-send-email-roger.pau@citrix.com> <9AAE0902D5BC7E449B7C8E4E778ABCD017021D@AMSPEX01CL01.citrite.net> <52823734.6020008@citrix.com> <52824CA60200007800102714@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VgFI1-0002pR-8o for xen-devel@lists.xenproject.org; Tue, 12 Nov 2013 14:50:03 +0000 In-Reply-To: <52824CA60200007800102714@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: "xen-devel@lists.xenproject.org" , Paul Durrant , "Keir (Xen.org)" , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org On 12/11/13 14:43, Jan Beulich wrote: >>>> On 12.11.13 at 15:12, David Vrabel wrote: >> On 12/11/13 13:46, Paul Durrant wrote: >>>> + uint64_t id; /* private guest value, echoed in resp */ >>>> + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ >>>> + blkif_vdev_t handle; /* same as for read/write requests */ >>>> + grant_ref_t >>>> indirect_grefs[BLKIF_MAX_INDIRECT_PAGES_PER_REQUEST]; >>>> +#ifdef __i386__ >>>> + uint64_t pad; /* Make it 64 byte aligned on i386 */ >>>> +#endif >> >> This last field looks a bit odd though. Why is it needed? > > On 64-bit there's a 32 bit hole before "id" and another one at the > end of the structure. In order for the structure size to be as > specified in the comment, both these holes need to be accounted > for. Ok. >>>> +}; >>>> +typedef struct blkif_request_indirect blkif_request_indirect_t; >>>> + >>>> +struct blkif_request_segment_aligned { >>>> + grant_ref_t gref; /* reference to I/O buffer frame */ >>>> + /* @first_sect: first sector in frame to transfer (inclusive). */ >>>> + /* @last_sect: last sector in frame to transfer (inclusive). */ >>>> + uint8_t first_sect, last_sect; >> >> Missing uint8_t padding here? > > Note that there are _two_ uint8_t-s. Doh. That's poor style though. A line per field, please. David