On Wed, May 16, 2018 at 10:46:45AM -0700, Christoph Hellwig wrote:
> On Wed, May 16, 2018 at 10:32:51AM -0700, Omar Sandoval wrote:
> > On Wed, May 16, 2018 at 10:25:31AM -0700, Christoph Hellwig wrote:
> > > On Wed, May 16, 2018 at 09:45:50AM -0700, Omar Sandoval wrote:
> > > > + if ((iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN) ||
> > > > + iomap->addr == IOMAP_NULL_ADDR) {
> > > > + pr_err("swapon: file has unallocated extents\n");
> > > > + return -EINVAL;
> > > > + }
> > >
> > > The two are really different cases - IOMAP_NULL_ADDR shouldn't really
> > > happen for any of the above. Although we might have to move the
> > > inline check before the type check above for the message to make sense.
> > >
> > > I have a patch in the local queue that makes inline a type instead of
> > > a flag, btw as it really isn't a flag.
That's what I thought -- we only see NULL_ADDR for holes and delalloc
extents, and we already check for both of those. But I didn't see
anything in iomap.h expressly saying that, so I decided to be defensive
and check it anyway.
IOWs it would be helpful to have a little more documentation about which
flags go together, particularly for things like IOMAP_F_BOUNDARY that
don't have any meaning in xfs.
> > So something like this, moving the inline check and removing the hole
> > check since that doesn't make sense for mapped or unwritten? Then the
> > inline flag check can be converted to a type check.
>
> Yes, this looks great!
Yeah, the v3 patch looks ok.
--D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
