On Thu, May 10, 2018 at 08:42:50AM +0200, Christoph Hellwig wrote:
> On Wed, May 09, 2018 at 09:46:28AM -0700, Darrick J. Wong wrote:
> > On Wed, May 09, 2018 at 09:48:07AM +0200, Christoph Hellwig wrote:
> > > This adds a simple iomap-based implementation of the legacy ->bmap
> > > interface. Note that we can't easily add checks for rt or reflink
> > > files, so these will have to remain in the callers. This interface
> > > just needs to die..
> >
> > You /can/ check these...
> >
> > if (iomap->bdev != inode->i_sb->s_bdev)
> > return 0;
> > if (iomap->flags & IOMAP_F_SHARED)
> > return 0;
>
> The latter only checks for a shared extent, not a file with possibly
> shared extents. I'd rather keep the check for a file with possible
> shared extents.
<nod>
> > > +static loff_t
> > > +iomap_bmap_actor(struct inode *inode, loff_t pos, loff_t length,
> > > + void *data, struct iomap *iomap)
> > > +{
> > > + sector_t *bno = data;
> > > +
> > > + if (iomap->type == IOMAP_MAPPED)
> > > + *bno = (iomap->addr + pos - iomap->offset) >> inode->i_blkbits;
> >
> > Does this need to be careful w.r.t. overflow on systems where sector_t
> > is a 32-bit unsigned long?
> >
> > Also, ioctl_fibmap() typecasts the returned sector_t to an int, which
> > also seems broken. I agree the interface needs to die, but ioctls take
> > a long time to deprecate.
>
> Not much we can do about the interface.
Yes, the interface is fubar, but if file /foo maps to block 8589934720
then do we return the truncated result 128?
--D
