On 05/14/2016 02:14 AM, Liu Bo wrote:
On Mon, May 09, 2016 at 09:31:37AM +0800, Qu Wenruo wrote:
David Sterba wrote on 2016/05/06 16:35 +0200:
On Thu, May 05, 2016 at 09:08:54AM +0800, Qu Wenruo wrote:
An early check can compare against some reasonable value, but the
total_bytes value must be equal to the sum of all device sizes
(disk_total_bytes). I'm not sure if we have enough information to verify
that at this point though.
That's what I had in mind, the problem is that only the first device
information is recorded in superblock.
At this moment We have device_num but we don't know the size of other devices.
Thanks,
-liubo
What about error out if we found sb->total_bytes <
sb->dev_item->total_bytes?
As we are just doing early check, no need to be comprehensive, but spot
obvious problem.
Ok.
I'm gonna check for total_bytes and num_devices after loading chunk
tree.
For exact device_num and sb->total_bytes, we may do post check when
device tree are loaded?
Splitting early_check() and post_check() seems valid for me.
(Also I prefer post_check() just warning, not forced exit)
Why just warning? Superblock total_bytes and device sizes must be
correct, otherwise all sorts of operations can fail randomly.
Because if we exit, we can't even do fsck.
Maybe we need a new flag to control whether exit or warn at post_check().
Thanks,
Qu
IMHO for kernel part, we have to exit in order to avoid any panic due to those
invalid value.
I'm OK with this.
For fsck code, we can go forth and back to fix them. In fact I don't
think fsck could work out anything, as superblock checksum has _matched_
but the values inside superblock are invalid, in this case we cannot trust
other parts in this FS image, then how can we expect fsck to fix it by reading
other parts?
For rw fsck, that may cause huge problem and I agree with you on error out.
But for ro fsck, it's a little overkilled for me.
Currently, if we found error in extent tree, we still continue checking
fstree, to shows what is really wrong.
And for case like btrfs-image restored images, its dev extents doesn't
even match with its chunk (may be it's already fixed?), but that's not a
big problem for ro btrfsck, and we can go on without problem.
So the same case is here for ro btrfsck.
As long as that's ro btrfsck, we could just continue as we don't really
need the total_bytes in superblock.
Thanks,
Qu
Thanks,
-liubo
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
