On Wed, Mar 20, 2019 at 02:05:15PM +0000, Paul Durrant wrote: > > -----Original Message----- > > From: Paul Durrant > > Sent: 20 March 2019 13:59 > > To: 'Konrad Rzeszutek Wilk' <konrad.w...@oracle.com> > > Cc: xen-devel@lists.xenproject.org > > Subject: RE: [PATCH] public/io/blkif.h: make the comments on "sectors" > > self-consistent > > > > > -----Original Message----- > > > From: Konrad Rzeszutek Wilk [mailto:konrad.w...@oracle.com] > > > Sent: 20 March 2019 13:53 > > > To: Paul Durrant <paul.durr...@citrix.com> > > > Cc: xen-devel@lists.xenproject.org > > > Subject: Re: [PATCH] public/io/blkif.h: make the comments on "sectors" > > > self-consistent > > > > > > On Wed, Mar 20, 2019 at 12:52:28PM +0000, Paul Durrant wrote: > > > > Currently the comment at line #267 claims that the value should be > > > > expressed in number logical sectors, whereas the comment at line #613 > > > > states that the value should be expressed strictly in units of 512 > > > > bytes. > > > > > > > > Signed-off-by: Paul Durrant <paul.durr...@citrix.com> > > > > --- > > > > Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> > > > > > > > > Looking at xen-blkfront in Linux, I'm also not convinced that it would > > > > function correctly is sector-size != 512 anyway so I wonder whether this > > > > patch should go further and define that sector-size is strictly 512. > > > > > > I thought it actually did work with a CD-ROM backed ISO using blkfront? > > > > > > > I've not tried that. What worries me is the call to xlvbd_alloc_gendisk() > > which takes sectors as an > > argument and passes it through to set_capacity() without scaling with > > sector-size in any way, which is > > would presumably need to do is sector-size != 512 (if we believe that > > sectors should be in terms of > > 512 byte units). > > Looking at xen-blkback, this actually just echoes the result of > get_capacity() into 'sectors', which would suggest the comment at line #613 > is the one that is bogus... but how many other frontends have been coded > against that? So, it would seem to me that the only way to get out of this > compatibility mess is to make sector-size strictly 512.
Bye bye 4096 sectore-size :-) Ugh, and we do <<9 all over the code so it is fairly baked. Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> > > Paul > > > > > Paul > > > > > > --- > > > > xen/include/public/io/blkif.h | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/xen/include/public/io/blkif.h > > > > b/xen/include/public/io/blkif.h > > > > index 15a71e3fea..d7c904d9dc 100644 > > > > --- a/xen/include/public/io/blkif.h > > > > +++ b/xen/include/public/io/blkif.h > > > > @@ -264,8 +264,7 @@ > > > > * sectors > > > > * Values: <uint64_t> > > > > * > > > > - * The size of the backend device, expressed in units of its > > > > logical > > > > - * sector size ("sector-size"). > > > > + * The size of the backend device, expressed in units of 512 > > > > bytes. > > > > * > > > > > > > > ***************************************************************************** > > > > * Frontend XenBus Nodes > > > > -- > > > > 2.20.1.2.gb21ebb671 > > > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel