Re: [PATCH] xfs_repair: fix max block offset test

[Eric Sandeen <sandeen@sandeen.net>]

On 12/18/14 9:18 AM, Brian Foster wrote:

hought this looked familiar:
> 
> http://oss.sgi.com/archives/xfs/2014-09/msg00524.html
> 
> Either one is fine with me. If we use the fix below, I think we should
> update the error message since it technically refers to the extent
> offset and we slightly tweak the meaning of the failure.
> 
> Brian

Ok, so here's what I wonder about with your patch.

+               if (irec.br_startoff > fs_max_file_offset)  {

What if the extent starting logical offset is *at* fs_max_file_offset
(which is sadly in blocks, not bytes, for extra confusion points),
but the extent is 1GB in length?

Isn't this a problem because we've now got files in the block which
are beyond fs_max_file_offset, and probably also beyond sb_maxbytes?

I'm not sure I follow why you feel the message needs to change with
my version:

+		/* Ensure this extent does not extend beyond the max offset */
+		if (irec.br_startoff + irec.br_blockcount - 1 >
+							fs_max_file_offset) {
                        do_warn(
_("inode %" PRIu64 " - extent offset too large - start %" PRIu64 ", "
  "count %" PRIu64 ", offset %" PRIu64 "\n"),
                                ino, irec.br_startblock, irec.br_blockcount,
                                irec.br_startoff);

If any (block) offset in the file is beyond the max, then we say the offset is
too large, and give it's physical block, its length, and its starting offset
(granted, the message is a bit *weird* - br_startblock really doesn't matter
here, but I don't think I've fundamentally changed the meaning of the test).

However, I've come to learn that you are almost always right about subtleties
like this, so perhaps you can explain again to me, using smaller words.  :)

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs