Re: [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item

[Qu Wenruo]

On 2017年09月28日 14:09, Nikolay Borisov wrote:

On 28.09.2017 06:36, Qu Wenruo wrote:

Output the invalid member name and its bad value, along with its
expected value range or alignment.

Signed-off-by: Qu Wenruo <quwenruo.bt...@gmx.com>
---
  fs/btrfs/tree-checker.c | 92 +++++++++++++++++++++++++++++++++++++------------
  1 file changed, 70 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 52e9ab8c2a79..1324fcae93c0 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -63,6 +63,47 @@ static void generic_err(const struct btrfs_root *root,
        va_end(args);
  }

+/*

+ * Customized reporter for extent data item, since its key objectid and
+ * offset has its own meaning.
+ */
+__printf(4, 5)
+static void file_extent_err(const struct btrfs_root *root,
+                           const struct extent_buffer *eb,
+                           int slot, const char *fmt, ...)
+{
+       struct btrfs_key key;
+       struct va_format vaf;
+       va_list args;
+
+       btrfs_item_key_to_cpu(eb, &key, slot);
+       va_start(args, fmt);
+
+       vaf.fmt = fmt;
+       vaf.va = &args;
+
+       btrfs_crit(root->fs_info,
+               "corrupt %s root=%llu tree_block=%llu slot=%d ino=%llu 
file_offset=%llu: %pV",

nit: Again, consider whether we should have : after the first %s so that
the string is consistent among different verifiers.

Consistence is important indeed.

I'll update the patchset to address it.

Thanks for pointing it out,
Qu

+               btrfs_header_level(eb) == 0 ? "leaf" : "node",
+               root->objectid, btrfs_header_bytenr(eb), slot,
+               key.objectid, key.offset, &vaf);
+       va_end(args);
+}
+
+/*
+ * Return 0 if the btrfs_file_extent_##name is aligned to @align
+ * Else return 1
+ */
+#define CHECK_FI_ALIGN(root, leaf, slot, fi, name, align)              \
+({                                                                     \
+       if (!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align))     \
+               file_extent_err(root, leaf, slot,                       \
+                       "invalid file extent %s, have %llu, should be aligned to 
%u",\
+                       #name, btrfs_file_extent_##name(leaf, fi),      \
+                       align);                                         \
+       (!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align));       \
+})
+
  static int check_extent_data_item(struct btrfs_root *root,
                                  struct extent_buffer *leaf,
                                  struct btrfs_key *key, int slot)
@@ -72,15 +113,19 @@ static int check_extent_data_item(struct btrfs_root *root,
        u32 item_size = btrfs_item_size_nr(leaf, slot);

if (!IS_ALIGNED(key->offset, sectorsize)) {

-               CORRUPT("unaligned key offset for file extent",
-                       leaf, root, slot);
+               file_extent_err(root, leaf, slot,
+                       "unaligned key offset, have %llu should be aligned to 
%u",
+                       key->offset, sectorsize);
                return -EUCLEAN;
        }

fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item); if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {

-               CORRUPT("invalid file extent type", leaf, root, slot);
+               file_extent_err(root, leaf, slot,
+                       "invalid file extent type, have %u expect range [0, 
%u]",
+                       btrfs_file_extent_type(leaf, fi),
+                       BTRFS_FILE_EXTENT_TYPES);
                return -EUCLEAN;
        }

@@ -89,18 +134,24 @@ static int check_extent_data_item(struct btrfs_root *root,

         * and must be caught in open_ctree().
         */
        if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
-               CORRUPT("invalid file extent compression", leaf, root, slot);
+               file_extent_err(root, leaf, slot,
+                       "invalid file extent compression, have %u expect range [0, 
%u]",
+                       btrfs_file_extent_compression(leaf, fi),
+                       BTRFS_COMPRESS_TYPES);
                return -EUCLEAN;
        }
        if (btrfs_file_extent_encryption(leaf, fi)) {
-               CORRUPT("invalid file extent encryption", leaf, root, slot);
+               file_extent_err(root, leaf, slot,
+                       "invalid file extent encryption, have %u expect 0",
+                       btrfs_file_extent_encryption(leaf, fi));
                return -EUCLEAN;
        }
        if (btrfs_file_extent_type(leaf, fi) == BTRFS_FILE_EXTENT_INLINE) {
                /* Inline extent must have 0 as key offset */
                if (key->offset) {
-                       CORRUPT("inline extent has non-zero key offset",
-                               leaf, root, slot);
+                       file_extent_err(root, leaf, slot,
+                               "invalid offset for inline extent, have %llu expect 
0",
+                               key->offset);
                        return -EUCLEAN;
                }

@@ -112,8 +163,10 @@ static int check_extent_data_item(struct btrfs_root *root,

                /* Uncompressed inline extent size must match item size */
                if (item_size != BTRFS_FILE_EXTENT_INLINE_DATA_START +
                    btrfs_file_extent_ram_bytes(leaf, fi)) {
-                       CORRUPT("plaintext inline extent has invalid size",
-                               leaf, root, slot);
+                       file_extent_err(root, leaf, slot,
+                               "invalid uncompressed inline extent size, have %u 
expect %llu",
+                               item_size, BTRFS_FILE_EXTENT_INLINE_DATA_START +
+                               btrfs_file_extent_ram_bytes(leaf, fi));
                        return -EUCLEAN;
                }
                return 0;
@@ -121,22 +174,17 @@ static int check_extent_data_item(struct btrfs_root *root,

/* Regular or preallocated extent has fixed item size */

        if (item_size != sizeof(*fi)) {
-               CORRUPT(
-               "regluar or preallocated extent data item size is invalid",
-                       leaf, root, slot);
+               file_extent_err(root, leaf, slot,
+                       "invalid extent data item size for reg/prealloc, have %u 
expect %lu",
+                       item_size, sizeof(*fi));
                return -EUCLEAN;
        }
-       if (!IS_ALIGNED(btrfs_file_extent_ram_bytes(leaf, fi), sectorsize) ||
-           !IS_ALIGNED(btrfs_file_extent_disk_bytenr(leaf, fi), sectorsize) ||
-           !IS_ALIGNED(btrfs_file_extent_disk_num_bytes(leaf, fi), sectorsize) 
||
-           !IS_ALIGNED(btrfs_file_extent_offset(leaf, fi), sectorsize) ||
-           !IS_ALIGNED(btrfs_file_extent_num_bytes(leaf, fi), sectorsize)) {
-               CORRUPT(
-               "regular or preallocated extent data item has unaligned value",
-                       leaf, root, slot);
+       if (CHECK_FI_ALIGN(root, leaf, slot, fi, ram_bytes, sectorsize) ||
+           CHECK_FI_ALIGN(root, leaf, slot, fi, disk_bytenr, sectorsize) ||
+           CHECK_FI_ALIGN(root, leaf, slot, fi, disk_num_bytes, sectorsize) ||
+           CHECK_FI_ALIGN(root, leaf, slot, fi, offset, sectorsize) ||
+           CHECK_FI_ALIGN(root, leaf, slot, fi, num_bytes, sectorsize))
                return -EUCLEAN;
-       }
-
        return 0;
  }

--
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