Re: [PATCH] btrfs-progs: Remove unused parameter

[Nikolay Borisov]

On 30.03.2018 05:56, Gu Jinxiang wrote:
> Parameter usagestr is not used, remove it.
> 
> Signed-off-by: Gu Jinxiang 

Reviewed-by: Nikolay Borisov 

> ---
>  cmds-filesystem.c | 2 +-
>  help.c| 2 +-
>  help.h| 3 +--
>  3 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 467aff11..5fa8cf2b 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -1093,7 +1093,7 @@ static int cmd_filesystem_resize(int argc, char **argv)
>   DIR *dirstream = NULL;
>   struct stat st;
>  
> - clean_args_no_options_relaxed(argc, argv, cmd_filesystem_resize_usage);
> + clean_args_no_options_relaxed(argc, argv);
>  
>   if (check_argc_exact(argc - optind, 2))
>   usage(cmd_filesystem_resize_usage);
> diff --git a/help.c b/help.c
> index 311a4320..f1dd3946 100644
> --- a/help.c
> +++ b/help.c
> @@ -115,7 +115,7 @@ void clean_args_no_options(int argc, char *argv[], const 
> char * const *usagestr)
>   * - "-- option1 option2 ..."
>   * - "option1 option2 ..."
>   */
> -void clean_args_no_options_relaxed(int argc, char *argv[], const char * 
> const *usagestr)
> +void clean_args_no_options_relaxed(int argc, char *argv[])
>  {
>   if (argc <= 1)
>   return;
> diff --git a/help.h b/help.h
> index efeded30..a1405942 100644
> --- a/help.h
> +++ b/help.h
> @@ -72,8 +72,7 @@ int check_argc_exact(int nargs, int expected);
>  int check_argc_min(int nargs, int expected);
>  int check_argc_max(int nargs, int expected);
>  void clean_args_no_options(int argc, char *argv[], const char * const 
> *usage);
> -void clean_args_no_options_relaxed(int argc, char *argv[],
> - const char * const *usagestr);
> +void clean_args_no_options_relaxed(int argc, char *argv[]);
>  
>  void fixup_argv0(char **argv, const char *token);
>  void set_argv0(char **argv);
> 
--
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

[PATCH v2 2/5] btrfs-progs: extent_io: Init eb->lru to avoid NULL pointer dereference

[Qu Wenruo]

eb->lru is not initialized in __alloc_extent_buffer(), so in the
following call chain, it could call NULL pointer dereference:

btrfs_clone_extent_buffer()
|- __alloc_extent_buffer()
   |- Now eb->lru is NULL (not initialized)

free_extent_buffer_final()
|- list_del_init(>lru)

Thankfully, current btrfs-progs won't trigger such bug as the only
btrfs_clone_extent_buffer() user is paths_from_inode(), which is not
used by anyone.
(But due to the usefulness of that function in future offline scrub, I'd
like to keep this dead code)

Anyway, initialize eb->lru in __alloc_extent_bufer() bring no harm.

Signed-off-by: Qu Wenruo 
---
 extent_io.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/extent_io.c b/extent_io.c
index 986ad5c0577c..3117782335ab 100644
--- a/extent_io.c
+++ b/extent_io.c
@@ -564,6 +564,7 @@ static struct extent_buffer *__alloc_extent_buffer(struct 
extent_io_tree *tree,
eb->cache_node.start = bytenr;
eb->cache_node.size = blocksize;
INIT_LIST_HEAD(>recow);
+   INIT_LIST_HEAD(>lru);
 
return eb;
 }
-- 
2.16.3

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

[PATCH v2 0/5] btrfs-progs: extent buffer related refactor and cleanup

[Qu Wenruo]

The patchset can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/eb_cleanup

Just like kernel cleanup and refactors, this patchset will embed
btrfs_fs_info structure into extent_buffer.

And fixes several possible NULL pointer dereference (although not
utilized in btrfs-progs yet).

Changelog:
v2:
  Embarrassingly, I forgot to install reiserfsprogs in my development
  machine, so the 3rd patch lacks one call site in
  convert/source-reiserfs.c.

Qu Wenruo (5):
  btrfs-progs: extent_io: Fix NULL pointer dereference in
free_extent_buffer_final()
  btrfs-progs: extent_io: Init eb->lru to avoid NULL pointer dereference
  btrfs-progs: extent_io: Refactor alloc_extent_buffer() to follow
kernel parameters
  btrfs-progs: Unify btrfs_leaf_free_psace() parameter with kernel
  btrfs-progs: print-tree: Remove btrfs_root parameter

 btrfs-corrupt-block.c |  2 +-
 check/main.c  |  2 +-
 check/mode-lowmem.c   |  2 +-
 cmds-inspect-dump-tree.c  | 31 ++
 convert/source-reiserfs.c |  3 +--
 ctree.c   | 65 +--
 ctree.h   |  3 ++-
 disk-io.c |  3 +--
 extent-tree.c |  8 +++---
 extent_io.c   | 17 -
 extent_io.h   |  3 ++-
 print-tree.c  | 20 ---
 print-tree.h  |  4 +--
 13 files changed, 85 insertions(+), 78 deletions(-)

-- 
2.16.3

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

[PATCH v2 5/5] btrfs-progs: print-tree: Remove btrfs_root parameter

[Qu Wenruo]

Just like kernel cleanup made by David, btrfs_print_leaf() and
btrfs_print_tree() doesn't need btrfs_root parameter at all.

With previous patches as preparation, now we can remove the btrfs_root
parameter.

Signed-off-by: Qu Wenruo 
---
 cmds-inspect-dump-tree.c | 31 ++-
 ctree.c  | 16 
 extent-tree.c|  8 
 print-tree.c | 20 +++-
 print-tree.h |  4 ++--
 5 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/cmds-inspect-dump-tree.c b/cmds-inspect-dump-tree.c
index 0802b31e9596..b0cd49b32664 100644
--- a/cmds-inspect-dump-tree.c
+++ b/cmds-inspect-dump-tree.c
@@ -33,8 +33,9 @@
 #include "utils.h"
 #include "help.h"
 
-static void print_extents(struct btrfs_root *root, struct extent_buffer *eb)
+static void print_extents(struct extent_buffer *eb)
 {
+   struct btrfs_fs_info *fs_info = eb->fs_info;
struct extent_buffer *next;
int i;
u32 nr;
@@ -43,13 +44,13 @@ static void print_extents(struct btrfs_root *root, struct 
extent_buffer *eb)
return;
 
if (btrfs_is_leaf(eb)) {
-   btrfs_print_leaf(root, eb);
+   btrfs_print_leaf(eb);
return;
}
 
nr = btrfs_header_nritems(eb);
for (i = 0; i < nr; i++) {
-   next = read_tree_block(root->fs_info,
+   next = read_tree_block(fs_info,
btrfs_node_blockptr(eb, i),
btrfs_node_ptr_generation(eb, i));
if (!extent_buffer_uptodate(next))
@@ -68,7 +69,7 @@ static void print_extents(struct btrfs_root *root, struct 
extent_buffer *eb)
btrfs_header_level(eb));
goto out;
}
-   print_extents(root, next);
+   print_extents(next);
free_extent_buffer(next);
}
 
@@ -331,7 +332,7 @@ int cmd_inspect_dump_tree(int argc, char **argv)
(unsigned long long)block_only);
goto close_root;
}
-   btrfs_print_tree(root, leaf, follow);
+   btrfs_print_tree(leaf, follow);
free_extent_buffer(leaf);
goto close_root;
}
@@ -358,20 +359,17 @@ int cmd_inspect_dump_tree(int argc, char **argv)
} else {
if (info->tree_root->node) {
printf("root tree\n");
-   btrfs_print_tree(info->tree_root,
-info->tree_root->node, 1);
+   btrfs_print_tree(info->tree_root->node, 1);
}
 
if (info->chunk_root->node) {
printf("chunk tree\n");
-   btrfs_print_tree(info->chunk_root,
-info->chunk_root->node, 1);
+   btrfs_print_tree(info->chunk_root->node, 1);
}
 
if (info->log_root_tree) {
printf("log root tree\n");
-   btrfs_print_tree(info->log_root_tree,
-   info->log_root_tree->node, 1);
+   btrfs_print_tree(info->log_root_tree->node, 1);
}
}
}
@@ -391,7 +389,7 @@ again:
goto close_root;
}
printf("root tree\n");
-   btrfs_print_tree(info->tree_root, info->tree_root->node, 1);
+   btrfs_print_tree(info->tree_root->node, 1);
goto close_root;
}
 
@@ -401,7 +399,7 @@ again:
goto close_root;
}
printf("chunk tree\n");
-   btrfs_print_tree(info->chunk_root, info->chunk_root->node, 1);
+   btrfs_print_tree(info->chunk_root->node, 1);
goto close_root;
}
 
@@ -411,8 +409,7 @@ again:
goto close_root;
}
printf("log root tree\n");
-   btrfs_print_tree(info->log_root_tree, info->log_root_tree->node,
-1);
+   btrfs_print_tree(info->log_root_tree->node, 1);
goto close_root;
}
 
@@ -548,7 +545,7 @@ again:
printf(" tree ");
btrfs_print_key(_key);
printf("\n");
-   print_extents(tree_root_scan, buf);
+   print_extents(buf);
} else if (!skip) {
printf(" tree ");
btrfs_print_key(_key);
@@ 

[PATCH v2 4/5] btrfs-progs: Unify btrfs_leaf_free_psace() parameter with kernel

[Qu Wenruo]

Instead of struct btrfs_root, use struct btrfs_fs_info, since nodesize
is now a per-fs setting, and with the need to pass a @root, caller don't
need to wonder which root should be passed.

Signed-off-by: Qu Wenruo 
---
 btrfs-corrupt-block.c |  2 +-
 check/main.c  |  2 +-
 check/mode-lowmem.c   |  2 +-
 ctree.c   | 49 ++---
 ctree.h   |  3 ++-
 print-tree.c  |  2 +-
 6 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 59ee1b45922e..da0ec8c51e5a 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -726,7 +726,7 @@ out:
 static void shift_items(struct btrfs_root *root, struct extent_buffer *eb)
 {
int nritems = btrfs_header_nritems(eb);
-   int shift_space = btrfs_leaf_free_space(root, eb) / 2;
+   int shift_space = btrfs_leaf_free_space(root->fs_info, eb) / 2;
int slot = nritems / 2;
int i = 0;
unsigned int data_end = btrfs_item_offset_nr(eb, nritems - 1);
diff --git a/check/main.c b/check/main.c
index 891a6d797756..7fde46e459e4 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5959,7 +5959,7 @@ static int run_next_block(struct btrfs_root *root,
goto out;
 
if (btrfs_is_leaf(buf)) {
-   btree_space_waste += btrfs_leaf_free_space(root, buf);
+   btree_space_waste += btrfs_leaf_free_space(fs_info, buf);
for (i = 0; i < nritems; i++) {
struct btrfs_file_extent_item *fi;
 
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index dac3201b7d99..bfe45abab6cc 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -390,7 +390,7 @@ static void account_bytes(struct btrfs_root *root, struct 
btrfs_path *path,
total_extent_tree_bytes += eb->len;
 
if (level == 0) {
-   btree_space_waste += btrfs_leaf_free_space(root, eb);
+   btree_space_waste += btrfs_leaf_free_space(root->fs_info, eb);
} else {
free_nrs = (BTRFS_NODEPTRS_PER_BLOCK(root->fs_info) -
btrfs_header_nritems(eb));
diff --git a/ctree.c b/ctree.c
index e26a1c29bb75..20848f0f7024 100644
--- a/ctree.c
+++ b/ctree.c
@@ -480,11 +480,11 @@ btrfs_check_leaf(struct btrfs_root *root, struct 
btrfs_disk_key *parent_key,
   (unsigned long long)btrfs_header_bytenr(buf));
goto fail;
}
-   if (btrfs_leaf_free_space(root, buf) < 0) {
+   if (btrfs_leaf_free_space(root->fs_info, buf) < 0) {
ret = BTRFS_TREE_BLOCK_INVALID_FREE_SPACE;
fprintf(stderr, "leaf free space incorrect %llu %d\n",
(unsigned long long)btrfs_header_bytenr(buf),
-   btrfs_leaf_free_space(root, buf));
+   btrfs_leaf_free_space(root->fs_info, buf));
goto fail;
}
 
@@ -1193,7 +1193,7 @@ again:
} else {
p->slots[level] = slot;
if (ins_len > 0 &&
-   ins_len > btrfs_leaf_free_space(root, b)) {
+   ins_len > btrfs_leaf_free_space(root->fs_info, b)) {
int sret = split_leaf(trans, root, key,
  p, ins_len, ret == 0);
BUG_ON(sret > 0);
@@ -1634,16 +1634,17 @@ static int leaf_space_used(struct extent_buffer *l, int 
start, int nr)
  * the start of the leaf data.  IOW, how much room
  * the leaf has left for both items and data
  */
-int btrfs_leaf_free_space(struct btrfs_root *root, struct extent_buffer *leaf)
+int btrfs_leaf_free_space(struct btrfs_fs_info *fs_info,
+ struct extent_buffer *leaf)
 {
-   u32 nodesize = (root ? BTRFS_LEAF_DATA_SIZE(root->fs_info) : leaf->len);
int nritems = btrfs_header_nritems(leaf);
int ret;
-   ret = nodesize - leaf_space_used(leaf, 0, nritems);
+
+   ret = BTRFS_LEAF_DATA_SIZE(fs_info) - leaf_space_used(leaf, 0, nritems);
if (ret < 0) {
-   printk("leaf free space ret %d, leaf data size %u, used %d 
nritems %d\n",
-  ret, nodesize, leaf_space_used(leaf, 0, nritems),
-  nritems);
+   printk("leaf free space ret %d, leaf data size %lu, used %d 
nritems %d\n",
+  ret, BTRFS_LEAF_DATA_SIZE(fs_info),
+  leaf_space_used(leaf, 0, nritems), nritems);
}
return ret;
 }
@@ -1691,7 +1692,7 @@ static int push_leaf_right(struct btrfs_trans_handle 
*trans, struct btrfs_root
return PTR_ERR(right);
return -EIO;
}
-   free_space = btrfs_leaf_free_space(root, right);
+   free_space = btrfs_leaf_free_space(fs_info, right);
if (free_space < data_size) {
 

[PATCH v2 3/5] btrfs-progs: extent_io: Refactor alloc_extent_buffer() to follow kernel parameters

[Qu Wenruo]

Instead of using the internal struct extent_io_tree, use struct fs_info.

This does not only unify the interface between kernel and btrfs-progs,
but also makes later btrfs_print_tree() use less parameters.

Signed-off-by: Qu Wenruo 
---
 convert/source-reiserfs.c |  3 +--
 disk-io.c |  3 +--
 extent_io.c   | 14 +-
 extent_io.h   |  3 ++-
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c
index eeb68d962c5d..e0b3b685f764 100644
--- a/convert/source-reiserfs.c
+++ b/convert/source-reiserfs.c
@@ -350,8 +350,7 @@ static int convert_direct(struct btrfs_trans_handle *trans,
if (ret)
return ret;
 
-   eb = alloc_extent_buffer(>fs_info->extent_cache, key.objectid,
-sectorsize);
+   eb = alloc_extent_buffer(root->fs_info, key.objectid, sectorsize);
 
if (!eb)
return -ENOMEM;
diff --git a/disk-io.c b/disk-io.c
index 76958aef239e..610963357675 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -184,8 +184,7 @@ struct extent_buffer *btrfs_find_tree_block(struct 
btrfs_fs_info *fs_info,
 struct extent_buffer* btrfs_find_create_tree_block(
struct btrfs_fs_info *fs_info, u64 bytenr)
 {
-   return alloc_extent_buffer(_info->extent_cache, bytenr,
-   fs_info->nodesize);
+   return alloc_extent_buffer(fs_info, bytenr, fs_info->nodesize);
 }
 
 void readahead_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
diff --git a/extent_io.c b/extent_io.c
index 3117782335ab..198492699438 100644
--- a/extent_io.c
+++ b/extent_io.c
@@ -545,7 +545,7 @@ out:
return ret;
 }
 
-static struct extent_buffer *__alloc_extent_buffer(struct extent_io_tree *tree,
+static struct extent_buffer *__alloc_extent_buffer(struct btrfs_fs_info *info,
   u64 bytenr, u32 blocksize)
 {
struct extent_buffer *eb;
@@ -558,11 +558,12 @@ static struct extent_buffer *__alloc_extent_buffer(struct 
extent_io_tree *tree,
eb->len = blocksize;
eb->refs = 1;
eb->flags = 0;
-   eb->tree = tree;
eb->fd = -1;
eb->dev_bytenr = (u64)-1;
eb->cache_node.start = bytenr;
eb->cache_node.size = blocksize;
+   eb->fs_info = info;
+   eb->tree = >extent_cache;
INIT_LIST_HEAD(>recow);
INIT_LIST_HEAD(>lru);
 
@@ -573,9 +574,11 @@ struct extent_buffer *btrfs_clone_extent_buffer(struct 
extent_buffer *src)
 {
struct extent_buffer *new;
 
-   new = __alloc_extent_buffer(NULL, src->start, src->len);
+   new = __alloc_extent_buffer(src->fs_info, src->start, src->len);
if (!new)
return NULL;
+   /* cloned eb is not linked into fs_info->extent_cache */
+   new->tree = NULL;
 
copy_extent_buffer(new, src, 0, 0, src->len);
new->flags |= EXTENT_BUFFER_DUMMY;
@@ -665,10 +668,11 @@ static void trim_extent_buffer_cache(struct 
extent_io_tree *tree)
}
 }
 
-struct extent_buffer *alloc_extent_buffer(struct extent_io_tree *tree,
+struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
  u64 bytenr, u32 blocksize)
 {
struct extent_buffer *eb;
+   struct extent_io_tree *tree = _info->extent_cache;
struct cache_extent *cache;
 
cache = lookup_cache_extent(>cache, bytenr, blocksize);
@@ -685,7 +689,7 @@ struct extent_buffer *alloc_extent_buffer(struct 
extent_io_tree *tree,
  cache_node);
free_extent_buffer(eb);
}
-   eb = __alloc_extent_buffer(tree, bytenr, blocksize);
+   eb = __alloc_extent_buffer(fs_info, bytenr, blocksize);
if (!eb)
return NULL;
ret = insert_cache_extent(>cache, >cache_node);
diff --git a/extent_io.h b/extent_io.h
index 17a4a8298635..f8f73089ab40 100644
--- a/extent_io.h
+++ b/extent_io.h
@@ -98,6 +98,7 @@ struct extent_buffer {
int refs;
u32 flags;
int fd;
+   struct btrfs_fs_info *fs_info;
char data[] __attribute__((aligned(8)));
 };
 
@@ -145,7 +146,7 @@ struct extent_buffer *find_extent_buffer(struct 
extent_io_tree *tree,
 u64 bytenr, u32 blocksize);
 struct extent_buffer *find_first_extent_buffer(struct extent_io_tree *tree,
   u64 start);
-struct extent_buffer *alloc_extent_buffer(struct extent_io_tree *tree,
+struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
  u64 bytenr, u32 blocksize);
 struct extent_buffer *btrfs_clone_extent_buffer(struct extent_buffer *src);
 void free_extent_buffer(struct extent_buffer *eb);
-- 
2.16.3

--
To unsubscribe from this list: send the line 

[PATCH v2 1/5] btrfs-progs: extent_io: Fix NULL pointer dereference in free_extent_buffer_final()

[Qu Wenruo]

In free_extent_buffer_final() we access eb->tree->cache_size in
BUG_ON().
However eb->tree can be NULL if it's a cloned extent buffer.

Currently the cloned extent buffer is only used in backref.c,
paths_from_inode() function.
Thankfully that function is not used yet (but could be pretty useful to
convert inode number to path, so I'd like to keep such function).

Anyway, check eb->tree before accessing its member.

Signed-off-by: Qu Wenruo 
---
 extent_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extent_io.c b/extent_io.c
index eda1fb6f5897..986ad5c0577c 100644
--- a/extent_io.c
+++ b/extent_io.c
@@ -587,7 +587,7 @@ static void free_extent_buffer_final(struct extent_buffer 
*eb)
struct extent_io_tree *tree = eb->tree;
 
BUG_ON(eb->refs);
-   BUG_ON(tree->cache_size < eb->len);
+   BUG_ON(tree && tree->cache_size < eb->len);
list_del_init(>lru);
if (!(eb->flags & EXTENT_BUFFER_DUMMY)) {
remove_cache_extent(>cache, >cache_node);
-- 
2.16.3

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

[PATCH 1/5] btrfs-progs: extent_io: Fix NULL pointer dereference in free_extent_buffer_final()

[Qu Wenruo]

In free_extent_buffer_final() we access eb->tree->cache_size in
BUG_ON().
However eb->tree can be NULL if it's a cloned extent buffer.

Currently the cloned extent buffer is only used in backref.c,
paths_from_inode() function.
Thankfully that function is not used yet (but could be pretty useful to
convert inode number to path, so I'd like to keep such function).

Anyway, check eb->tree before accessing its member.

Signed-off-by: Qu Wenruo 
---
 extent_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extent_io.c b/extent_io.c
index eda1fb6f5897..986ad5c0577c 100644
--- a/extent_io.c
+++ b/extent_io.c
@@ -587,7 +587,7 @@ static void free_extent_buffer_final(struct extent_buffer 
*eb)
struct extent_io_tree *tree = eb->tree;
 
BUG_ON(eb->refs);
-   BUG_ON(tree->cache_size < eb->len);
+   BUG_ON(tree && tree->cache_size < eb->len);
list_del_init(>lru);
if (!(eb->flags & EXTENT_BUFFER_DUMMY)) {
remove_cache_extent(>cache, >cache_node);
-- 
2.16.3

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

[PATCH 3/5] btrfs-progs: extent_io: Refactor alloc_extent_buffer() to follow kernel parameters

[Qu Wenruo]

Instead of using the internal struct extent_io_tree, use struct fs_info.

This does not only unify the interface between kernel and btrfs-progs,
but also makes later btrfs_print_tree() use less parameters.

Signed-off-by: Qu Wenruo 
---
 disk-io.c   |  3 +--
 extent_io.c | 14 +-
 extent_io.h |  3 ++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index 76958aef239e..610963357675 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -184,8 +184,7 @@ struct extent_buffer *btrfs_find_tree_block(struct 
btrfs_fs_info *fs_info,
 struct extent_buffer* btrfs_find_create_tree_block(
struct btrfs_fs_info *fs_info, u64 bytenr)
 {
-   return alloc_extent_buffer(_info->extent_cache, bytenr,
-   fs_info->nodesize);
+   return alloc_extent_buffer(fs_info, bytenr, fs_info->nodesize);
 }
 
 void readahead_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
diff --git a/extent_io.c b/extent_io.c
index 3117782335ab..198492699438 100644
--- a/extent_io.c
+++ b/extent_io.c
@@ -545,7 +545,7 @@ out:
return ret;
 }
 
-static struct extent_buffer *__alloc_extent_buffer(struct extent_io_tree *tree,
+static struct extent_buffer *__alloc_extent_buffer(struct btrfs_fs_info *info,
   u64 bytenr, u32 blocksize)
 {
struct extent_buffer *eb;
@@ -558,11 +558,12 @@ static struct extent_buffer *__alloc_extent_buffer(struct 
extent_io_tree *tree,
eb->len = blocksize;
eb->refs = 1;
eb->flags = 0;
-   eb->tree = tree;
eb->fd = -1;
eb->dev_bytenr = (u64)-1;
eb->cache_node.start = bytenr;
eb->cache_node.size = blocksize;
+   eb->fs_info = info;
+   eb->tree = >extent_cache;
INIT_LIST_HEAD(>recow);
INIT_LIST_HEAD(>lru);
 
@@ -573,9 +574,11 @@ struct extent_buffer *btrfs_clone_extent_buffer(struct 
extent_buffer *src)
 {
struct extent_buffer *new;
 
-   new = __alloc_extent_buffer(NULL, src->start, src->len);
+   new = __alloc_extent_buffer(src->fs_info, src->start, src->len);
if (!new)
return NULL;
+   /* cloned eb is not linked into fs_info->extent_cache */
+   new->tree = NULL;
 
copy_extent_buffer(new, src, 0, 0, src->len);
new->flags |= EXTENT_BUFFER_DUMMY;
@@ -665,10 +668,11 @@ static void trim_extent_buffer_cache(struct 
extent_io_tree *tree)
}
 }
 
-struct extent_buffer *alloc_extent_buffer(struct extent_io_tree *tree,
+struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
  u64 bytenr, u32 blocksize)
 {
struct extent_buffer *eb;
+   struct extent_io_tree *tree = _info->extent_cache;
struct cache_extent *cache;
 
cache = lookup_cache_extent(>cache, bytenr, blocksize);
@@ -685,7 +689,7 @@ struct extent_buffer *alloc_extent_buffer(struct 
extent_io_tree *tree,
  cache_node);
free_extent_buffer(eb);
}
-   eb = __alloc_extent_buffer(tree, bytenr, blocksize);
+   eb = __alloc_extent_buffer(fs_info, bytenr, blocksize);
if (!eb)
return NULL;
ret = insert_cache_extent(>cache, >cache_node);
diff --git a/extent_io.h b/extent_io.h
index 17a4a8298635..f8f73089ab40 100644
--- a/extent_io.h
+++ b/extent_io.h
@@ -98,6 +98,7 @@ struct extent_buffer {
int refs;
u32 flags;
int fd;
+   struct btrfs_fs_info *fs_info;
char data[] __attribute__((aligned(8)));
 };
 
@@ -145,7 +146,7 @@ struct extent_buffer *find_extent_buffer(struct 
extent_io_tree *tree,
 u64 bytenr, u32 blocksize);
 struct extent_buffer *find_first_extent_buffer(struct extent_io_tree *tree,
   u64 start);
-struct extent_buffer *alloc_extent_buffer(struct extent_io_tree *tree,
+struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
  u64 bytenr, u32 blocksize);
 struct extent_buffer *btrfs_clone_extent_buffer(struct extent_buffer *src);
 void free_extent_buffer(struct extent_buffer *eb);
-- 
2.16.3

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

[PATCH 4/5] btrfs-progs: Unify btrfs_leaf_free_psace() parameter with kernel

[Qu Wenruo]

Instead of struct btrfs_root, use struct btrfs_fs_info, since nodesize
is now a per-fs setting, and with the need to pass a @root, caller don't
need to wonder which root should be passed.

Signed-off-by: Qu Wenruo 
---
 btrfs-corrupt-block.c |  2 +-
 check/main.c  |  2 +-
 check/mode-lowmem.c   |  2 +-
 ctree.c   | 49 ++---
 ctree.h   |  3 ++-
 print-tree.c  |  2 +-
 6 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index 59ee1b45922e..da0ec8c51e5a 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -726,7 +726,7 @@ out:
 static void shift_items(struct btrfs_root *root, struct extent_buffer *eb)
 {
int nritems = btrfs_header_nritems(eb);
-   int shift_space = btrfs_leaf_free_space(root, eb) / 2;
+   int shift_space = btrfs_leaf_free_space(root->fs_info, eb) / 2;
int slot = nritems / 2;
int i = 0;
unsigned int data_end = btrfs_item_offset_nr(eb, nritems - 1);
diff --git a/check/main.c b/check/main.c
index 891a6d797756..7fde46e459e4 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5959,7 +5959,7 @@ static int run_next_block(struct btrfs_root *root,
goto out;
 
if (btrfs_is_leaf(buf)) {
-   btree_space_waste += btrfs_leaf_free_space(root, buf);
+   btree_space_waste += btrfs_leaf_free_space(fs_info, buf);
for (i = 0; i < nritems; i++) {
struct btrfs_file_extent_item *fi;
 
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index dac3201b7d99..bfe45abab6cc 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -390,7 +390,7 @@ static void account_bytes(struct btrfs_root *root, struct 
btrfs_path *path,
total_extent_tree_bytes += eb->len;
 
if (level == 0) {
-   btree_space_waste += btrfs_leaf_free_space(root, eb);
+   btree_space_waste += btrfs_leaf_free_space(root->fs_info, eb);
} else {
free_nrs = (BTRFS_NODEPTRS_PER_BLOCK(root->fs_info) -
btrfs_header_nritems(eb));
diff --git a/ctree.c b/ctree.c
index e26a1c29bb75..20848f0f7024 100644
--- a/ctree.c
+++ b/ctree.c
@@ -480,11 +480,11 @@ btrfs_check_leaf(struct btrfs_root *root, struct 
btrfs_disk_key *parent_key,
   (unsigned long long)btrfs_header_bytenr(buf));
goto fail;
}
-   if (btrfs_leaf_free_space(root, buf) < 0) {
+   if (btrfs_leaf_free_space(root->fs_info, buf) < 0) {
ret = BTRFS_TREE_BLOCK_INVALID_FREE_SPACE;
fprintf(stderr, "leaf free space incorrect %llu %d\n",
(unsigned long long)btrfs_header_bytenr(buf),
-   btrfs_leaf_free_space(root, buf));
+   btrfs_leaf_free_space(root->fs_info, buf));
goto fail;
}
 
@@ -1193,7 +1193,7 @@ again:
} else {
p->slots[level] = slot;
if (ins_len > 0 &&
-   ins_len > btrfs_leaf_free_space(root, b)) {
+   ins_len > btrfs_leaf_free_space(root->fs_info, b)) {
int sret = split_leaf(trans, root, key,
  p, ins_len, ret == 0);
BUG_ON(sret > 0);
@@ -1634,16 +1634,17 @@ static int leaf_space_used(struct extent_buffer *l, int 
start, int nr)
  * the start of the leaf data.  IOW, how much room
  * the leaf has left for both items and data
  */
-int btrfs_leaf_free_space(struct btrfs_root *root, struct extent_buffer *leaf)
+int btrfs_leaf_free_space(struct btrfs_fs_info *fs_info,
+ struct extent_buffer *leaf)
 {
-   u32 nodesize = (root ? BTRFS_LEAF_DATA_SIZE(root->fs_info) : leaf->len);
int nritems = btrfs_header_nritems(leaf);
int ret;
-   ret = nodesize - leaf_space_used(leaf, 0, nritems);
+
+   ret = BTRFS_LEAF_DATA_SIZE(fs_info) - leaf_space_used(leaf, 0, nritems);
if (ret < 0) {
-   printk("leaf free space ret %d, leaf data size %u, used %d 
nritems %d\n",
-  ret, nodesize, leaf_space_used(leaf, 0, nritems),
-  nritems);
+   printk("leaf free space ret %d, leaf data size %lu, used %d 
nritems %d\n",
+  ret, BTRFS_LEAF_DATA_SIZE(fs_info),
+  leaf_space_used(leaf, 0, nritems), nritems);
}
return ret;
 }
@@ -1691,7 +1692,7 @@ static int push_leaf_right(struct btrfs_trans_handle 
*trans, struct btrfs_root
return PTR_ERR(right);
return -EIO;
}
-   free_space = btrfs_leaf_free_space(root, right);
+   free_space = btrfs_leaf_free_space(fs_info, right);
if (free_space < data_size) {
 

[PATCH 5/5] btrfs-progs: print-tree: Remove btrfs_root parameter

[Qu Wenruo]

Just like kernel cleanup made by David, btrfs_print_leaf() and
btrfs_print_tree() doesn't need btrfs_root parameter at all.

With previous patches as preparation, now we can remove the btrfs_root
parameter.

Signed-off-by: Qu Wenruo 
---
 cmds-inspect-dump-tree.c | 31 ++-
 ctree.c  | 16 
 extent-tree.c|  8 
 print-tree.c | 20 +++-
 print-tree.h |  4 ++--
 5 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/cmds-inspect-dump-tree.c b/cmds-inspect-dump-tree.c
index 0802b31e9596..b0cd49b32664 100644
--- a/cmds-inspect-dump-tree.c
+++ b/cmds-inspect-dump-tree.c
@@ -33,8 +33,9 @@
 #include "utils.h"
 #include "help.h"
 
-static void print_extents(struct btrfs_root *root, struct extent_buffer *eb)
+static void print_extents(struct extent_buffer *eb)
 {
+   struct btrfs_fs_info *fs_info = eb->fs_info;
struct extent_buffer *next;
int i;
u32 nr;
@@ -43,13 +44,13 @@ static void print_extents(struct btrfs_root *root, struct 
extent_buffer *eb)
return;
 
if (btrfs_is_leaf(eb)) {
-   btrfs_print_leaf(root, eb);
+   btrfs_print_leaf(eb);
return;
}
 
nr = btrfs_header_nritems(eb);
for (i = 0; i < nr; i++) {
-   next = read_tree_block(root->fs_info,
+   next = read_tree_block(fs_info,
btrfs_node_blockptr(eb, i),
btrfs_node_ptr_generation(eb, i));
if (!extent_buffer_uptodate(next))
@@ -68,7 +69,7 @@ static void print_extents(struct btrfs_root *root, struct 
extent_buffer *eb)
btrfs_header_level(eb));
goto out;
}
-   print_extents(root, next);
+   print_extents(next);
free_extent_buffer(next);
}
 
@@ -331,7 +332,7 @@ int cmd_inspect_dump_tree(int argc, char **argv)
(unsigned long long)block_only);
goto close_root;
}
-   btrfs_print_tree(root, leaf, follow);
+   btrfs_print_tree(leaf, follow);
free_extent_buffer(leaf);
goto close_root;
}
@@ -358,20 +359,17 @@ int cmd_inspect_dump_tree(int argc, char **argv)
} else {
if (info->tree_root->node) {
printf("root tree\n");
-   btrfs_print_tree(info->tree_root,
-info->tree_root->node, 1);
+   btrfs_print_tree(info->tree_root->node, 1);
}
 
if (info->chunk_root->node) {
printf("chunk tree\n");
-   btrfs_print_tree(info->chunk_root,
-info->chunk_root->node, 1);
+   btrfs_print_tree(info->chunk_root->node, 1);
}
 
if (info->log_root_tree) {
printf("log root tree\n");
-   btrfs_print_tree(info->log_root_tree,
-   info->log_root_tree->node, 1);
+   btrfs_print_tree(info->log_root_tree->node, 1);
}
}
}
@@ -391,7 +389,7 @@ again:
goto close_root;
}
printf("root tree\n");
-   btrfs_print_tree(info->tree_root, info->tree_root->node, 1);
+   btrfs_print_tree(info->tree_root->node, 1);
goto close_root;
}
 
@@ -401,7 +399,7 @@ again:
goto close_root;
}
printf("chunk tree\n");
-   btrfs_print_tree(info->chunk_root, info->chunk_root->node, 1);
+   btrfs_print_tree(info->chunk_root->node, 1);
goto close_root;
}
 
@@ -411,8 +409,7 @@ again:
goto close_root;
}
printf("log root tree\n");
-   btrfs_print_tree(info->log_root_tree, info->log_root_tree->node,
-1);
+   btrfs_print_tree(info->log_root_tree->node, 1);
goto close_root;
}
 
@@ -548,7 +545,7 @@ again:
printf(" tree ");
btrfs_print_key(_key);
printf("\n");
-   print_extents(tree_root_scan, buf);
+   print_extents(buf);
} else if (!skip) {
printf(" tree ");
btrfs_print_key(_key);
@@ 

[PATCH 2/5] btrfs-progs: extent_io: Init eb->lru to avoid NULL pointer dereference

[Qu Wenruo]

eb->lru is not initialized in __alloc_extent_buffer(), so in the
following call chain, it could call NULL pointer dereference:

btrfs_clone_extent_buffer()
|- __alloc_extent_buffer()
   |- Now eb->lru is NULL (not initialized)

free_extent_buffer_final()
|- list_del_init(>lru)

Thankfully, current btrfs-progs won't trigger such bug as the only
btrfs_clone_extent_buffer() user is paths_from_inode(), which is not
used by anyone.
(But due to the usefulness of that function in future offline scrub, I'd
like to keep this dead code)

Anyway, initialize eb->lru in __alloc_extent_bufer() bring no harm.

Signed-off-by: Qu Wenruo 
---
 extent_io.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/extent_io.c b/extent_io.c
index 986ad5c0577c..3117782335ab 100644
--- a/extent_io.c
+++ b/extent_io.c
@@ -564,6 +564,7 @@ static struct extent_buffer *__alloc_extent_buffer(struct 
extent_io_tree *tree,
eb->cache_node.start = bytenr;
eb->cache_node.size = blocksize;
INIT_LIST_HEAD(>recow);
+   INIT_LIST_HEAD(>lru);
 
return eb;
 }
-- 
2.16.3

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

Re: [RFC PATCH v3 0/7] btrfs-progs: Allow normal user to call "subvolume list/show"

[Misono Tomohiro]

On 2018/03/30 2:35, Goffredo Baroncelli wrote:
> Hi Misono,

Hello,

> 
> I tested you patch, and it seems to work. I verified that the output is 
> correct and the permission check is performed. However the output of "btrfs 
> sub list" in the "root" mode and the "user" mode are a bit different.
> 
> As reported before, I find your output more consistent, and understandable 
> than the original one, which is quite confusing about the sub-volume path 
> relationship. 
> 
> The problem which I am talking is probably due to the fact that I mount a 
> subvolume as root filesystem, and the root-subvolume (the one with ID=5) in a 
> subdirectory (/var/btrfs for what matters).

I didn't think this situation and thanks for pointing out.

Currently this RFC does not care about mounted subvolumes under the specified 
path.
However, it should be able to search those subvolumes too and list up all the 
subvolumes
including mounted ones (and below it), by changing progs code.

> 
> I think that the "stock" "btrfs sub list", trying to scan the metadata of 
> btrfs and merging these information with this filesystem layout (composed by 
> different subvolumes mounted in different places) doesn't do a good job.
> 
> The constraints that your API has (the fact that the subvolume has to be 
> accessed by the filesystem), create an output more friendly. Unfortunately 
> this output became again confusing when the command is started by root.
> 
> [Un]fortunately, the stock "btrfs sub list" has the capability to show all 
> the subvolumes, even the ones not mounted, so this can be entirely replaced 
> by your API.
> 
> My conclusion, is that your work is not consistent with the current 
> implementation; so I am suggesting to create a new subcommand ("btrfs sub 
> tree" ?) where you can use, without constraint, your api.

I agree that the current output of "sub list" is confusing first of all and
it is not easy to keep consistent behavior between user and root.

So, I think "sub list" (or new command) should be:
  - (default) list the subvolumes under the specified path, including 
subvolumes mounted below the specified path.
  Any user can do this (with appropriate permission checks).
  The path to be printed is the relative from the specified path.
  - (-a option) list all the subvolmumes in the filesystem. Only root can do 
this.
  The path to be printed is the absolute path from the toplevel 
subvolume.

This would need some changes in progs code, but I think we can use the same new 
ioctls I proposed[1] for the default behavior.

I'd like to update "sub list" command eventually rather than adding new 
command, even though this breaks the compatibility.
I want to know what other developer/user think.

[1] https://www.spinics.net/lists/linux-btrfs/msg76003.html

> 
> Another small differences that I found is in the subcommand "btrfs sub show". 
> This is not capable to show the snapshots of a subvolume; I think that, in 
> "user mode", it should be reported that there is a "missing capabilities" 
> problem than to show an empty list

Yes, this is because that only the snapshots *under the specified path* are 
searched.
This can be easily changed to search under the mount point, but this also 
results in
slight change in print format of the path for root. I tried to keep as much 
consistency in this version.

Thanks,
Tomohiro Misono

> 
> Below an example of what I am say:
> 
> ghigo@venice:/tmp$ btrfs sub cre a
> ghigo@venice:/tmp$ btrfs sub snap a c
> 
> ghigo@venice:/tmp$ # patched btrfs
> ghigo@venice:/tmp$ /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
> tmp/a
>   Name:   a
>   UUID:   0c19a7a4-a7f5-1849-9262-88ab3da504d0
>   Parent UUID:-
>   Received UUID:  -
>   Creation time:  2018-03-28 22:48:09 +0200
>   Subvolume ID:   598
>   Generation: 696908
>   Gen at creation:696907
>   Parent ID:  257
>   Top level ID:   257
>   Flags:  -
>   Snapshot(s):
> 
> 
> ghigo@venice:/tmp$ # stock btrfs
> ghigo@venice:/tmp$ sudo /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
>   ^^
> tmp/a
>   Name:   a
>   UUID:   0c19a7a4-a7f5-1849-9262-88ab3da504d0
>   Parent UUID:-
>   Received UUID:  -
>   Creation time:  2018-03-28 22:48:09 +0200
>   Subvolume ID:   598
>   Generation: 696908
>   Gen at creation:696907
>   Parent ID:  257
>   Top level ID:   257
>   Flags:  -
>   Snapshot(s):
>   debian/tmp/c
> ^
>
> 
> BR
> G.Baroncelli
> 
> 
> -
> Test performed:
> 
> ghigo@venice:/tmp$ sudo 

[PATCH] btrfs-progs: Remove unused parameter

[Gu Jinxiang]

Parameter usagestr is not used, remove it.

Signed-off-by: Gu Jinxiang 
---
 cmds-filesystem.c | 2 +-
 help.c| 2 +-
 help.h| 3 +--
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 467aff11..5fa8cf2b 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -1093,7 +1093,7 @@ static int cmd_filesystem_resize(int argc, char **argv)
DIR *dirstream = NULL;
struct stat st;
 
-   clean_args_no_options_relaxed(argc, argv, cmd_filesystem_resize_usage);
+   clean_args_no_options_relaxed(argc, argv);
 
if (check_argc_exact(argc - optind, 2))
usage(cmd_filesystem_resize_usage);
diff --git a/help.c b/help.c
index 311a4320..f1dd3946 100644
--- a/help.c
+++ b/help.c
@@ -115,7 +115,7 @@ void clean_args_no_options(int argc, char *argv[], const 
char * const *usagestr)
  * - "-- option1 option2 ..."
  * - "option1 option2 ..."
  */
-void clean_args_no_options_relaxed(int argc, char *argv[], const char * const 
*usagestr)
+void clean_args_no_options_relaxed(int argc, char *argv[])
 {
if (argc <= 1)
return;
diff --git a/help.h b/help.h
index efeded30..a1405942 100644
--- a/help.h
+++ b/help.h
@@ -72,8 +72,7 @@ int check_argc_exact(int nargs, int expected);
 int check_argc_min(int nargs, int expected);
 int check_argc_max(int nargs, int expected);
 void clean_args_no_options(int argc, char *argv[], const char * const *usage);
-void clean_args_no_options_relaxed(int argc, char *argv[],
-   const char * const *usagestr);
+void clean_args_no_options_relaxed(int argc, char *argv[]);
 
 void fixup_argv0(char **argv, const char *token);
 void set_argv0(char **argv);
-- 
2.14.3



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

[PATCH 1/7] btrfs: cleanup btrfs_check_super_csum() for better code flow

[Anand Jain]

We check the %csum_type twice. Drop one. Check for the csum_type and
then for the csum. Which also matches with the progs which have better
logic. This is preparatory patch to get proper error code from
btrfs_check_super_csum().

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
 fs/btrfs/disk-io.c | 38 +-
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1657d6aa4fa6..b9b435579527 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -399,32 +399,28 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
*fs_info,
struct btrfs_super_block *disk_sb =
(struct btrfs_super_block *)raw_disk_sb;
u16 csum_type = btrfs_super_csum_type(disk_sb);
-   int ret = 0;
-
-   if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
-   u32 crc = ~(u32)0;
-   char result[sizeof(crc)];
-
-   /*
-* The super_block structure does not span the whole
-* BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
-* is filled with zeros and is included in the checksum.
-*/
-   crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
-   crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-   btrfs_csum_final(crc, result);
-
-   if (memcmp(raw_disk_sb, result, sizeof(result)))
-   ret = 1;
-   }
+   u32 crc = ~(u32)0;
+   char result[sizeof(crc)];
 
if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
btrfs_err(fs_info, "unsupported checksum algorithm %u",
-   csum_type);
-   ret = 1;
+ csum_type);
+   return 1;
}
 
-   return ret;
+   /*
+* The super_block structure does not span the whole
+* BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
+* is filled with zeros and is included in the checksum.
+*/
+   crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
+ crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+   btrfs_csum_final(crc, result);
+
+   if (memcmp(raw_disk_sb, result, sizeof(result)))
+   return 1;
+
+   return 0;
 }
 
 /*
-- 
2.7.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

[PATCH v2 4/7] btrfs: check if the fsid in the primary sb and copy sb are same

[Anand Jain]

During the btrfs dev scan make sure that other copies of superblock
contain the same fsid as the primary SB. So that we bring to the
user notice if the superblock has been overwritten.

 mkfs.btrfs -fq /dev/sdc
 mkfs.btrfs -fq /dev/sdb
 dd if=/dev/sdb of=/dev/sdc count=4K skip=64K seek=64K obs=1 ibs=1
 mount /dev/sdc /btrfs

Caveat: Pls note that older btrfs-progs do not wipe the non-overwriting
stale superblock like copy2 if a smaller mkfs.btrfs -b   is created.
Thus this patch in the kernel will report error. The workaround is to wipe
the superblock manually, like
  dd if=/dev/zero of= seek=274877906944 ibs=1 obs=1 count4K
OR apply the btrfs-progs patch
  btrfs-progs: wipe copies of the stale superblock beyond -b size
which shall find and wipe the non overwriting superblock during mkfs.

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
v1->v2:
  Do an explicit read for primary superblock. Drop kzalloc().
  Fix split pr_err().

 fs/btrfs/volumes.c | 48 ++--
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e63723f23227..035affa447fa 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1198,39 +1198,67 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
 int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
  struct btrfs_fs_devices **fs_devices_ret)
 {
+   struct btrfs_super_block *disk_super_primary;
struct btrfs_super_block *disk_super;
struct btrfs_device *device;
struct block_device *bdev;
+   struct page *page_primary;
struct page *page;
int ret = 0;
u64 bytenr;
+   int i;
 
-   /*
-* we would like to check all the supers, but that would make
-* a btrfs mount succeed after a mkfs from a different FS.
-* So, we need to add a special mount option to scan for
-* later supers, using BTRFS_SUPER_MIRROR_MAX instead
-*/
-   bytenr = btrfs_sb_offset(0);
flags |= FMODE_EXCL;
 
bdev = blkdev_get_by_path(path, flags, holder);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
 
-   ret = btrfs_read_disk_super(bdev, bytenr, , _super);
+   /*
+* We would like to check all the supers and use one good copy,
+* but that would make a btrfs mount succeed after a mkfs from
+* a different FS.
+* So, we need to add a special mount option to scan for
+* later supers, using BTRFS_SUPER_MIRROR_MAX instead.
+* So, just validate if all copies of the superblocks are ok
+* and have the same fsid.
+*/
+   bytenr = btrfs_sb_offset(0);
+   ret = btrfs_read_disk_super(bdev, bytenr, _primary,
+   _super_primary);
if (ret < 0)
goto error_bdev_put;
 
+   for (i = 1; i < BTRFS_SUPER_MIRROR_MAX; i++) {
+   bytenr = btrfs_sb_offset(i);
+   ret = btrfs_read_disk_super(bdev, bytenr, , _super);
+   if (ret < 0) {
+   ret = 0;
+   continue;
+   }
+
+   if (memcmp(disk_super_primary->fsid, disk_super->fsid,
+  BTRFS_FSID_SIZE)) {
+   pr_err("BTRFS (device %pg): superblock fsid mismatch, 
primary %pU copy%d %pU",
+   bdev, disk_super_primary->fsid, i,
+   disk_super->fsid);
+   ret = -EINVAL;
+   btrfs_release_disk_super(page);
+   goto error_rel_primary;
+   }
+   btrfs_release_disk_super(page);
+   }
+
mutex_lock(_mutex);
-   device = device_list_add(path, disk_super);
+   device = device_list_add(path, disk_super_primary);
if (IS_ERR(device))
ret = PTR_ERR(device);
else
*fs_devices_ret = device->fs_devices;
mutex_unlock(_mutex);
 
-   btrfs_release_disk_super(page);
+error_rel_primary:
+   btrfs_release_disk_super(page_primary);
 
 error_bdev_put:
blkdev_put(bdev, flags);
-- 
2.7.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

[PATCH v4 0/7] Superblock read and verify cleanups

[Anand Jain]

v3->v4:
 Update changelog and signoff.
 Reintroduce explicit check for '-EUCLEAN'
  at Patch 2/8 and 5/8.

v2->v3:
 Squash
  4/8 btrfs: make btrfs_check_super_csum() non static
 to
  6/8 btrfs: verify superblock checksum during scan
 As in the individual patch mentioned
 
v1->v2:
 Various changes suggested by Nicokay. Thanks. For specific changes
 pls ref to the patch.

Patch 1-4/8 are preparatory patches adds cleanups and nonstatic requisites.

Patch 5/8 makes sure that all copies of the superblock have the same fsid
when we scan the device.

Patch 6/8 verifies superblock csum when we read it in the scan context.

Patch 7/8 fixes a bug that we weren't verifying the superblock csum for
the non-latest_bdev.

And 8/8 patch drops the redundant invalidate_bdev() call during mount.

There is a btrfs-progs patch which is a kind of related, as its found that
we weren't wiping the non-overwritten superblock, so it could cause
confusion during the superblock recovery process. So the patch btrfs-progs
1/1 adds code to wipe superblock if we aren't overwriting it.

Now since kernel patch 5/8 checks if all the superblock copies are
pointing to the same fsid on the disk, so the scan will fail if without
the above 1/1 btrfs-progs, as in the example below [1]. However the simple
workaround is to wipe the superblock manually [2] or apply the btrfs-progs
patch below.

[1]
 mkfs.btrfs -qf /dev/sdb <-- 1T disk
 mkfs.btrfs -b 256M  /dev/sdb
 ERROR: device scan failed on /dev/sdb

[2]
 dd if=/dev/zero of= seek=274877906944 ibs=1 obs=1 count4K

Unfortunately, the error messages should have been failed to register
[3] device into the kernel to be more appropriate to the error.

[3]
ret = ioctl(fd, BTRFS_IOC_SCAN_DEV, );
if (ret < 0) {
error("device scan failed on '%s': %m", fname);
ret = -errno;
}

Patches 1-7/8 were sent independently before. And  I found few more things
to fix alongs the line, and since they are related, so I am sending these
all together. Also, as there are minor changes, like in pr_err strings,
and splitting the unrelated changes into a separate patch, so though I am
thankful for the received reviewed-by, I couldn't include them here. Sorry.

Finally, here I am including the function relations [4] so that it will help
to review the code. And this flow is before these patches were applied.

[4]
In the long term, I suggest deprecating ioctl args which pass device path
(where possible), like in delete-device/replace. And
btrfs_read_dev_one_super() should replace btrfs_read_disk_super()

delete-device/replace:
btrfs_rm_device() || btrfs_dev_replace_by_ioctl()
|_btrfs_find_device_by_devspec()
  |_btrfs_find_device_missing_or_by_path()
|_btrfs_find_device_by_path()
  |_btrfs_get_bdev_and_sb()
|_btrfs_read_dev_super()
  |_btrfs_read_dev_one_super()
|___bread()

btrfs_mount_root()
 |
 |_btrfs_parse_early_options (-o device only)
 | |_btrfs_scan_one_device
 |   |_btrfs_read_disk_super()
 | |_read_cache_page_gfp()
 |
 |_btrfs_scan_one_device(mount-arg-dev only)
 | |_btrfs_read_disk_super()
 |   |_read_cache_page_gfp()
 |
 |
 |_btrfs_open_devices(fsid:all)
 |  |_btrfs_open_one_device()
 ||_btrfs_get_bdev_and_sb()  <--- invalidate_bdev(fsid:all)
 |  |_btrfs_read_dev_super()
 ||_btrfs_read_dev_one_super()
 |  |___bread()
 |
 |_btrfs_fill_super()
   |_btrfs_open_ctree()   <-- invalidate_bdev(latest_bdev) <-- redundant
 |_btrfs_read_dev_super(latest_bdev only)
 | |_btrfs_read_dev_one_super(latest_bdev only)
 |   |___bread(latest_bdev)
 |
 |_btrfs_check_super_csum(latest_bdev only) [*]
 |
 |_btrfs_read_chunk_tree
 | |_read_one_dev()
 |   |_open_seed_devices()
 | |_btrfs_open_devices(fs_devices->seed only)

scan/ready
|
|_btrfs_scan_one_device(ioctl-arg-dev only)
   |_btrfs_read_disk_super()
 |_read_cache_page_gfp()


Anand Jain (7):
  btrfs: cleanup btrfs_check_super_csum() for better code flow
  btrfs: return required error from btrfs_check_super_csum
  btrfs: cleanup btrfs_read_disk_super() to return std error
  btrfs: check if the fsid in the primary sb and copy sb are same
  btrfs: verify superblock checksum during scan
  btrfs: verify checksum for all devices in mount context
  btrfs: drop the redundant invalidate_bdev()

 fs/btrfs/disk-io.c | 72 ++
 fs/btrfs/disk-io.h |  1 +
 fs/btrfs/volumes.c | 72 ++
 3 files changed, 92 insertions(+), 53 deletions(-)

Anand Jain (1):
  btrfs-progs: wipe copies of the stale superblock beyond -b size

 utils.c | 35 +++
 1 file changed, 35 insertions(+)
-- 
2.7.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

[PATCH v4 2/7] btrfs: return required error from btrfs_check_super_csum

[Anand Jain]

Return the required -EINVAL and -EUCLEAN from the function
btrfs_check_super_csum(). And more the error log into the
parent function.

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
v2->v4:
 Check for -EUCLEAN in else if
v1->v2:
 Fix commit indent reported by checkpatch.pl
 Fix pr_err split string

 fs/btrfs/disk-io.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b9b435579527..c3a9407fc3a5 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -392,9 +392,10 @@ static int verify_parent_transid(struct extent_io_tree 
*io_tree,
 /*
  * Return 0 if the superblock checksum type matches the checksum value of that
  * algorithm. Pass the raw disk superblock data.
+ * Otherwise:  -EINVAL  if csum type is not found
+ * -EUCLEAN if csum does not match
  */
-static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
- char *raw_disk_sb)
+static int btrfs_check_super_csum(char *raw_disk_sb)
 {
struct btrfs_super_block *disk_sb =
(struct btrfs_super_block *)raw_disk_sb;
@@ -402,11 +403,8 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
*fs_info,
u32 crc = ~(u32)0;
char result[sizeof(crc)];
 
-   if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
-   btrfs_err(fs_info, "unsupported checksum algorithm %u",
- csum_type);
-   return 1;
-   }
+   if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes))
+   return -EINVAL;
 
/*
 * The super_block structure does not span the whole
@@ -418,7 +416,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
*fs_info,
btrfs_csum_final(crc, result);
 
if (memcmp(raw_disk_sb, result, sizeof(result)))
-   return 1;
+   return -EUCLEAN;
 
return 0;
 }
@@ -2570,9 +2568,14 @@ int open_ctree(struct super_block *sb,
 * We want to check superblock checksum, the type is stored inside.
 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
 */
-   if (btrfs_check_super_csum(fs_info, bh->b_data)) {
-   btrfs_err(fs_info, "superblock checksum mismatch");
-   err = -EINVAL;
+   err = btrfs_check_super_csum(bh->b_data);
+   if (err) {
+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
+   fs_devices->latest_bdev);
+   else if (err == -EUCLEAN)
+   pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
+   fs_devices->latest_bdev);
brelse(bh);
goto fail_alloc;
}
-- 
2.7.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

[PATCH v2 3/7] btrfs: cleanup btrfs_read_disk_super() to return std error

[Anand Jain]

The only caller btrfs_scan_one_device() sets -EINVAL for error from
btrfs_read_disk_super(), so this patch returns -EINVAL from the latter
function.

A preparatory patch to add csum check in btrfs_read_disk_super().

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
v1->v2:
 Check btrfs_read_disk_super() to be more explicit ret < 0

 fs/btrfs/volumes.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 87d183accdb2..e63723f23227 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1154,23 +1154,23 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
 
/* make sure our super fits in the device */
if (bytenr + PAGE_SIZE >= i_size_read(bdev->bd_inode))
-   return 1;
+   return -EINVAL;
 
/* make sure our super fits in the page */
if (sizeof(**disk_super) > PAGE_SIZE)
-   return 1;
+   return -EINVAL;
 
/* make sure our super doesn't straddle pages on disk */
index = bytenr >> PAGE_SHIFT;
if ((bytenr + sizeof(**disk_super) - 1) >> PAGE_SHIFT != index)
-   return 1;
+   return -EINVAL;
 
/* pull in the page with our super */
*page = read_cache_page_gfp(bdev->bd_inode->i_mapping,
   index, GFP_KERNEL);
 
if (IS_ERR_OR_NULL(*page))
-   return 1;
+   return -EINVAL;
 
p = kmap(*page);
 
@@ -1180,7 +1180,7 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
if (btrfs_super_bytenr(*disk_super) != bytenr ||
btrfs_super_magic(*disk_super) != BTRFS_MAGIC) {
btrfs_release_disk_super(*page);
-   return 1;
+   return -EINVAL;
}
 
if ((*disk_super)->label[0] &&
@@ -1218,10 +1218,9 @@ int btrfs_scan_one_device(const char *path, fmode_t 
flags, void *holder,
if (IS_ERR(bdev))
return PTR_ERR(bdev);
 
-   if (btrfs_read_disk_super(bdev, bytenr, , _super)) {
-   ret = -EINVAL;
+   ret = btrfs_read_disk_super(bdev, bytenr, , _super);
+   if (ret < 0)
goto error_bdev_put;
-   }
 
mutex_lock(_mutex);
device = device_list_add(path, disk_super);
-- 
2.7.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

Re: [PATCH v2 2/7] btrfs: return required error from btrfs_check_super_csum

[Anand Jain]

@@ -2570,9 +2568,14 @@ int open_ctree(struct super_block *sb,
 * We want to check superblock checksum, the type is stored inside.
 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
 */
-   if (btrfs_check_super_csum(fs_info, bh->b_data)) {
-   btrfs_err(fs_info, "superblock checksum mismatch");
-   err = -EINVAL;
+   err = btrfs_check_super_csum(bh->b_data);
+   if (err) {
+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
+   fs_devices->latest_bdev);
+   else
+   pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
+   fs_devices->latest_bdev);


nit: I think your initial version of specifically checking for EUCLEAN
was better but I don't have strong preferences.


 Fixed this in v4. Also updated the changelog.

 Thanks for the review.

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

[PATCH v4 5/7] btrfs: verify superblock checksum during scan

[Anand Jain]

During the scan context, we aren't verifying the superblock-
checksum when read.
This patch fixes it by adding the checksum verification function
btrfs_check_super_csum() in the function btrfs_read_disk_super().
And makes device scan to error fail if the primary superblock csum
is wrong, whereas if the copy-superblock csum is wrong it will just
just report mismatch and continue mount/scan as usual. When the
mount is successful We anyway overwrite all superblocks upon unmount.

The context in which this will be called is - device scan, device ready,
and mount -o device option.

Test script:
 Corrupt primary superblock and check if device scan and mount
 fails:
  mkfs.btrfs -fq /dev/sdc
  dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
  btrfs dev scan /dev/sdc
  mount /dev/sdc /btrfs

 Corrupt secondary superblock and check if device scan and mount
 is succcessful, check for the dmesg for errors.
  mkfs.btrfs -fq /dev/sdc
  dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=67108864
  btrfs dev scan /dev/sdc
  mount /dev/sdc /btrfs

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
v3->v4:
  Update changelog
v2->v3:
  squash 4/8 in here.
  4/8: btrfs: make btrfs_check_super_csum() non-static
v1->v2:
 changed title.
 use explicit (< 0) check for %errr.
 Un-split pr_err() string.
 Fix typo in the git commit log.
 Move the csum check after bytenr and btrfs magic verified.

 fs/btrfs/disk-io.c |  2 +-
 fs/btrfs/disk-io.h |  1 +
 fs/btrfs/volumes.c | 13 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c3a9407fc3a5..37b0cf489fdd 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -395,7 +395,7 @@ static int verify_parent_transid(struct extent_io_tree 
*io_tree,
  * Otherwise:  -EINVAL  if csum type is not found
  * -EUCLEAN if csum does not match
  */
-static int btrfs_check_super_csum(char *raw_disk_sb)
+int btrfs_check_super_csum(char *raw_disk_sb)
 {
struct btrfs_super_block *disk_sb =
(struct btrfs_super_block *)raw_disk_sb;
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index 70a88d61b547..c400cc68f913 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -69,6 +69,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int 
max_mirrors);
 struct buffer_head *btrfs_read_dev_super(struct block_device *bdev);
 int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
struct buffer_head **bh_ret);
+int btrfs_check_super_csum(char *raw_disk_sb);
 int btrfs_commit_super(struct btrfs_fs_info *fs_info);
 struct btrfs_root *btrfs_read_fs_root(struct btrfs_root *tree_root,
  struct btrfs_key *location);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 035affa447fa..760c6650e11b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1149,6 +1149,7 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
 struct page **page,
 struct btrfs_super_block **disk_super)
 {
+   int err;
void *p;
pgoff_t index;
 
@@ -1183,6 +1184,18 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
return -EINVAL;
}
 
+   err = btrfs_check_super_csum((char *) *disk_super);
+   if (err < 0) {
+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
type, bytenr=%llu",
+   bdev, bytenr);
+   else if (err == -EUCLEAN)
+   pr_err("BTRFS error (device %pg): superblock checksum 
failed, bytenr=%llu",
+   bdev, bytenr);
+   btrfs_release_disk_super(*page);
+   return err;
+   }
+
if ((*disk_super)->label[0] &&
(*disk_super)->label[BTRFS_LABEL_SIZE - 1])
(*disk_super)->label[BTRFS_LABEL_SIZE - 1] = '\0';
-- 
2.7.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

[PATCH] btrfs-progs: wipe copies of the stale superblock beyond -b size

[Anand Jain]

During the mkfs.btrfs -b  btrfs_prepare_device() zeros all
the superblock bytenr locations only if the bytenr is below the
blockcount. The problem with this is that if the BTRFS is recreated
with a smaller size then we will leave the stale superblock in the disk
which shall confuse the recovery. As shown in the test case below.

 mkfs.btrfs -qf /dev/mapper/vg-lv
 mkfs.btrfs -qf -b1G /dev/mapper/vg-lv
 btrfs in dump-super -a /dev/mapper/vg-lv | grep '.fsid|superblock:'

 superblock: bytenr=65536, device=/dev/mapper/vg-lv
 dev_item.fsid  ebc67d01-7fc5-43f0-90b4-d1925002551e [match]
 superblock: bytenr=67108864, device=/dev/mapper/vg-lv
 dev_item.fsid  ebc67d01-7fc5-43f0-90b4-d1925002551e [match]
 superblock: bytenr=274877906944, device=/dev/mapper/vg-lv
 dev_item.fsid  b97a9206-593b-4933-a424-c6a6ee23fe7c [match]

So if we find a valid superblock zero it even if it's beyond the
blockcount.

Signed-off-by: Anand Jain 
---
 utils.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/utils.c b/utils.c
index e9cb3a82fda6..6a9408b06e73 100644
--- a/utils.c
+++ b/utils.c
@@ -365,6 +365,41 @@ int btrfs_prepare_device(int fd, const char *file, u64 
*block_count_ret,
return 1;
}
 
+   /*
+* Check for the BTRFS SB copies up until btrfs_device_size() and zero
+* it. So that kernel (or user for the manual recovery) don't have to
+* confuse with the stale SB copy during recovery.
+*/
+   if (block_count != btrfs_device_size(fd, )) {
+   for (i = 1; i < BTRFS_SUPER_MIRROR_MAX; i++) {
+   struct btrfs_super_block *disk_super;
+   char buf[BTRFS_SUPER_INFO_SIZE];
+   disk_super = (struct btrfs_super_block *)buf;
+
+   /* Already zeroed above */
+   if (btrfs_sb_offset(i) < block_count)
+   continue;
+
+   /* Beyond actual disk size */
+   if (btrfs_sb_offset(i) >= btrfs_device_size(fd, ))
+   continue;
+
+   /* Does not contain any stale SB */
+   if (btrfs_read_dev_super(fd, disk_super,
+btrfs_sb_offset(i), 0))
+   continue;
+
+   ret = zero_dev_clamped(fd, btrfs_sb_offset(i),
+   BTRFS_SUPER_INFO_SIZE,
+   btrfs_device_size(fd, ));
+   if (ret < 0) {
+   error("failed to zero device '%s' bytenr %llu: 
%s",
+   file, btrfs_sb_offset(i), 
strerror(-ret));
+   return 1;
+   }
+   }
+   }
+
*block_count_ret = block_count;
return 0;
 }
-- 
2.7.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

[PATCH 7/7] btrfs: drop the redundant invalidate_bdev()

[Anand Jain]

During the mount context btrfs_open_devices() calls invalidate_bdev()
for all the devices. So drop the invalidate_bdev() in open_ctree()
which is only for the btrfs_fs_devices::latest_bdev.

The call trace is as shown below.

btrfs_mount_root()
 |
 |_btrfs_parse_early_options (-o device only)
 | |_btrfs_scan_one_device
 |   |_btrfs_read_disk_super()
 | |_read_cache_page_gfp()
 |
 |_btrfs_scan_one_device(mount-arg-dev only)
 | |_btrfs_read_disk_super()
 |   |_read_cache_page_gfp()
 |
 |
 |_btrfs_open_devices(fsid:all)
 |  |_btrfs_open_one_device()
 ||_btrfs_get_bdev_and_sb()  <--- invalidate_bdev(fsid:all)
 |  |_btrfs_read_dev_super()
 ||_btrfs_read_dev_one_super()
 |  |___bread()
 |
 |_btrfs_fill_super()
   |_btrfs_open_ctree()   <-- invalidate_bdev(latest_bdev) <-- redundant
 |_btrfs_read_dev_super(latest_bdev only)
   |_btrfs_read_dev_one_super(latest_bdev only)
 |___bread(latest_bdev)

Signed-off-by: Anand Jain 
---
 fs/btrfs/disk-io.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1eabcc4bf8d2..55b8a57435a9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2553,8 +2553,6 @@ int open_ctree(struct super_block *sb,
 
__setup_root(tree_root, fs_info, BTRFS_ROOT_TREE_OBJECTID);
 
-   invalidate_bdev(fs_devices->latest_bdev);
-
/*
 * Read super block and check the signature bytes only
 */
-- 
2.7.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

Re: Status of RAID5/6

[Zygo Blaxell]

On Wed, Mar 21, 2018 at 09:02:36PM +0100, Christoph Anton Mitterer wrote:
> Hey.
> 
> Some things would IMO be nice to get done/clarified (i.e. documented in
> the Wiki and manpages) from users'/admin's  POV:
> 
> Some basic questions:

I can answer some easy ones:

>   - compression+raid?

There is no interaction between compression and raid.  They happen on
different data trees at different levels of the stack.  So if the raid
works, compression does too.

>   - rebuild / replace of devices?

"replace" needs raid-level-specific support.  If the raid level doesn't
support replace, then users have to do device add followed by device
delete, which is considerably (orders of magnitude) slower.

>   - changing raid lvls?

btrfs uses a brute-force RAID conversion algorithm which always works, but
takes zero short cuts.  e.g. there is no speed optimization implemented
for cases like "convert 2-disk raid1 to 1-disk single" which can be
very fast in theory.  The worst-case running time is the only running
time available in btrfs.

Also, users have to understand how the different raid allocators work
to understand their behavior in specific situations.  Without this
understanding, the set of restrictions that pop up in practice can seem
capricious and arbitrary.  e.g. after adding 1 disk to a nearly-full
raid1, full balance is required to make the new space available, but
adding 2 disks makes all the free space available immediately.

Generally it always works if you repeatedly run full-balances in a loop
until you stop running out of space, but again, this is the worst case.

>   - anything to consider with raid when doing snapshots, send/receive
> or defrag?

Snapshot deletes cannot run at the same time as RAID convert/device
delete/device shrink/resize.  If one is started while the other is
running, it will be blocked until the other finishes.  Internally these
operations block each other on a mutex.

I don't know if snapshot deletes interact with device replace (the case
has never come up for me).  I wouldn't expect it to as device replace
is more similar to scrub than balance, and scrub has no such interaction.

Also note you can only run one balance, device shrink, or device delete
at a time.  If you start one of these three operations while another is
already running, the new request is rejected immediately.

As far as I know there are no other restrictions.

>   => and for each of these: for which raid levels?

Most of those features don't interact with anything specific to a raid
layer, so they work on all raid levels.

Device replace is the exception: all RAID levels in use on the filesystem
must support it, or the user must use device add and device delete instead.

[Aside:  I don't know if any RAID levels that do not support device
replace still exist, which makes my answer longer than it otherwise
would be]

>   Perhaps also confirmation for previous issues:
>   - I vaguely remember there were issues with either device delete or
> replace and that one of them was possibly super-slow?

Device replace is faster than device delete.  Replace does not modify
any metadata, while delete rewrites all the metadata referring to the
removed device.

Delete can be orders of magnitude slower than expected because of the
metadata modifications required.

>   - I also remember there were cases in which a fs could end up in
> permanent read-only state?

Any unrecovered metadata error 1 bit or larger will do that.  RAID level
is relevant only in terms of how well it can recover corrupted or
unreadable metadata blocks.

> - Clarifying questions on what is expected to work and how things are
>   expected to behave, e.g.:
>   - Can one plug a device (without deleting/removing it first) just
> under operation and will btrfs survive it?

On raid1 and raid10, yes.  On raid5/6 you will be at risk of write hole
problems if the filesystem is modified while the device is unplugged.

If the device is later reconnected, you should immediately scrub to
bring the metadata on the devices back in sync.  Data written to the
filesystem while the device was offline will be corrected if the csum
is different on the removed device.  If there is no csum data will be
silently corrupted.  If the csum is correct, but the data is not (this
occurs with 2^-32 probability on random data where the CRC happens to
be identical) then the data will be silently corrupted.

A full replace of the removed device would be better than a scrub,
as that will get a known good copy of the data.

If the device is offline for a long time, it should be wiped before being
reintroduced to the rest of the array to avoid data integrity issues.

It may be necessary to specify a different device name when mounting
a filesystem that has had a disk removed and later reinserted until
the scrub or replace action above is completed.

btrfs has no optimization like mdadm write-intent bitmaps; recovery
is always a full-device operation.  In theory btrfs 

Re: [PATCH] fstests: test btrfs fsync after hole punching with no-holes mode

[Jayashree Mohan]

Hi Filipe,

I tried running the xfstest above on kernel 4.15.0 and I haven't been
able to hit the bug. The xfstest passes clean for me. I compared the
btrfs-debug-tree in my case with the one attached by Eryu, and I see I
do not have any xattr as he does. However, for every run of the
xfstest, the extent tree info that I get from btrfs-debug-tree has
varying number of items in the first leaf node. (I have attached two
dump files for your reference.)

I also tried changing the offset at which fpunch is performed to match
the offset of the last extent in the first leaf of the extent tree -
however the md5 before and after crash was the same.

Could you give more details on this?

https://friendpaste.com/1wVAz7hG0U5SgYtZavbJhL
https://friendpaste.com/1wVAz7hG0U5SgYtZavxALg

Thanks,
Jayashree Mohan



On Thu, Mar 29, 2018 at 8:45 AM, Filipe Manana  wrote:
> On Wed, Mar 28, 2018 at 11:33 AM, Eryu Guan  wrote:
>> On Wed, Mar 28, 2018 at 09:48:17AM +0100, Filipe Manana wrote:
>>> On Wed, Mar 28, 2018 at 3:17 AM, Eryu Guan  wrote:
>>> > On Mon, Mar 26, 2018 at 11:59:21PM +0100, fdman...@kernel.org wrote:
>>> >> From: Filipe Manana 
>>> >>
>>> >> Test that when we have the no-holes mode enabled and a specific metadata
>>> >> layout, if we punch a hole and fsync the file, at replay time the whole
>>> >> hole was preserved.
>>> >>
>>> >> This issue is fixed by the following btrfs patch for the linux kernel:
>>> >>
>>> >>   "Btrfs: fix fsync after hole punching when using no-holes feature"
>>> >
>>> > I'd expect a test failure with 4.16-rc6 kernel, as the mentioned fix
>>> > above is not there. But test always passes for me. Did I miss anything?
>>> > btrfs-progs version is btrfs-progs-4.11.1-3.fc27.
>>>
>>> It should fail on any kernel, with any btrfs-progs version (which
>>> should be irrelevant).
>>> Somehow on your system we are not getting the specific metadata layout
>>> needed to trigger the issue.
>>>
>>> Can you apply the following patch on top of the test and provide the
>>> result 159.full file?
>>>
>>> https://friendpaste.com/6xAuLeN4xl1AGjO9Qc5I8L
>>>
>>> So that I can see what metadata layout you are getting.
>>> Thanks!
>>
>> Sure, please see attachment.
>
> Thanks!
> So you indeed get a different metadata layout, and that is because you
> have SELinux enabled which causes some xattr to be added to the test
> file (foobar):
>
> item 6 key (257 XATTR_ITEM 3817753667) itemoff 64932 itemsize 83
> location key (0 UNKNOWN.0 0) type XATTR
> transid 7 data_len 37 name_len 16
> name: security.selinux
> data unconfined_u:object_r:unlabeled_t:s0
>
> I can make the test work with and without selinux enabled (by punching
> holes on a few extents that are candidates to be at leaf boundaries).
> Is it worth it? (I assume most people run the tests without selinux)
>
> thanks
>
>>
>> Thanks,
>> Eryu
> --
> 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

Re: [RFC PATCH v3 0/7] btrfs-progs: Allow normal user to call "subvolume list/show"

[Goffredo Baroncelli]

Hi Misono,

I tested you patch, and it seems to work. I verified that the output is correct 
and the permission check is performed. However the output of "btrfs sub list" 
in the "root" mode and the "user" mode are a bit different.

As reported before, I find your output more consistent, and understandable than 
the original one, which is quite confusing about the sub-volume path 
relationship. 

The problem which I am talking is probably due to the fact that I mount a 
subvolume as root filesystem, and the root-subvolume (the one with ID=5) in a 
subdirectory (/var/btrfs for what matters).

I think that the "stock" "btrfs sub list", trying to scan the metadata of btrfs 
and merging these information with this filesystem layout (composed by 
different subvolumes mounted in different places) doesn't do a good job.

The constraints that your API has (the fact that the subvolume has to be 
accessed by the filesystem), create an output more friendly. Unfortunately this 
output became again confusing when the command is started by root.

[Un]fortunately, the stock "btrfs sub list" has the capability to show all the 
subvolumes, even the ones not mounted, so this can be entirely replaced by your 
API.

My conclusion, is that your work is not consistent with the current 
implementation; so I am suggesting to create a new subcommand ("btrfs sub tree" 
?) where you can use, without constraint, your api.

Another small differences that I found is in the subcommand "btrfs sub show". 
This is not capable to show the snapshots of a subvolume; I think that, in 
"user mode", it should be reported that there is a "missing capabilities" 
problem than to show an empty list

Below an example of what I am say:

ghigo@venice:/tmp$ btrfs sub cre a
ghigo@venice:/tmp$ btrfs sub snap a c

ghigo@venice:/tmp$ # patched btrfs
ghigo@venice:/tmp$ /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
tmp/a
Name:   a
UUID:   0c19a7a4-a7f5-1849-9262-88ab3da504d0
Parent UUID:-
Received UUID:  -
Creation time:  2018-03-28 22:48:09 +0200
Subvolume ID:   598
Generation: 696908
Gen at creation:696907
Parent ID:  257
Top level ID:   257
Flags:  -
Snapshot(s):


ghigo@venice:/tmp$ # stock btrfs
ghigo@venice:/tmp$ sudo /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
  ^^
tmp/a
Name:   a
UUID:   0c19a7a4-a7f5-1849-9262-88ab3da504d0
Parent UUID:-
Received UUID:  -
Creation time:  2018-03-28 22:48:09 +0200
Subvolume ID:   598
Generation: 696908
Gen at creation:696907
Parent ID:  257
Top level ID:   257
Flags:  -
Snapshot(s):
debian/tmp/c
^
   

BR
G.Baroncelli


-
Test performed:

ghigo@venice:/tmp$ sudo btrfs sub crea a
ghigo@venice:/tmp$ sudo btrfs sub crea a/a1
ghigo@venice:/tmp$ sudo btrfs sub crea a/a1/a2
ghigo@venice:/tmp$ sudo btrfs sub crea b
ghigo@venice:/tmp$ sudo btrfs sub crea b/b1
ghigo@venice:/tmp$ sudo chmod og-rwx b/.


# stock btrfs progs
ghigo@venice:/tmp$ sudo btrfs sub l .
ID 257 gen 696886 top level 5 path debian
ID 289 gen 587461 top level 257 path var/lib/machines
ID 299 gen 693561 top level 5 path boot
ID 582 gen 683965 top level 5 path i386
ID 592 gen 696884 top level 257 path tmp/a
ID 593 gen 696885 top level 592 path tmp/a/a1
ID 594 gen 696885 top level 593 path tmp/a/a1/a2
ID 595 gen 696887 top level 257 path tmp/b
ID 596 gen 696887 top level 595 path tmp/b/b1

# patched btrfs progs
ghigo@venice:/tmp$ /home/ghigo/btrfs/btrfs-progs/btrfs sub lis .
ID 592 gen 696884 top level 257 path a
ID 593 gen 696885 top level 592 path a/a1
ID 594 gen 696885 top level 593 path a/a1/a2
ID 595 gen 0 top level 257 path b


ghigo@venice:/tmp$ /home/ghigo/btrfs/btrfs-progs/btrfs sub show a
tmp/a
Name:   a
UUID:   17520c11-ec8b-5b49-a07e-37ba58113261
Parent UUID:-
Received UUID:  -
Creation time:  2018-03-28 22:19:48 +0200
Subvolume ID:   592
Generation: 696884
Gen at creation:696883
Parent ID:  257
Top level ID:   257
Flags:  -
Snapshot(s):


On 03/19/2018 08:30 AM, Misono, Tomohiro wrote:
> changelog:
> 
> v2 -> v3
>   - use get_euid() to check the caller's privilege (and remove 3rd patch)
>   - improve error handling
> v1 -> v2
>   - add independent error handling patch (1st patch)
>   - reimplement according to ioctl change
> 

Re: [PATCH] Btrfs: print error messages when failing to read trees

[Liu Bo]

On Wed, Mar 28, 2018 at 11:24 PM, Nikolay Borisov  wrote:
>
>
> On 29.03.2018 01:11, Liu Bo wrote:
>> When mount fails to read trees like fs tree, checksum tree, extent
>> tree, etc, there is not enough information about where went wrong.
>>
>> With this, messages like
>>
>> "BTRFS warning (device sdf): failed to read root (objectid=7): -5"
>>
>> would help us a bit.
>>
>> Signed-off-by: Liu Bo 
>> ---
>>  fs/btrfs/disk-io.c | 31 ++-
>>  1 file changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 21f34ad..954616c 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2334,23 +2334,29 @@ static int btrfs_read_roots(struct btrfs_fs_info 
>> *fs_info)
>>   location.offset = 0;
>>
>>   root = btrfs_read_tree_root(tree_root, );
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>>   set_bit(BTRFS_ROOT_TRACK_DIRTY, >state);
>>   fs_info->extent_root = root;
>>
>>   location.objectid = BTRFS_DEV_TREE_OBJECTID;
>>   root = btrfs_read_tree_root(tree_root, );
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>>   set_bit(BTRFS_ROOT_TRACK_DIRTY, >state);
>>   fs_info->dev_root = root;
>>   btrfs_init_devices_late(fs_info);
>>
>>   location.objectid = BTRFS_CSUM_TREE_OBJECTID;
>>   root = btrfs_read_tree_root(tree_root, );
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>>   set_bit(BTRFS_ROOT_TRACK_DIRTY, >state);
>>   fs_info->csum_root = root;
>>
>> @@ -2367,7 +2373,7 @@ static int btrfs_read_roots(struct btrfs_fs_info 
>> *fs_info)
>>   if (IS_ERR(root)) {
>>   ret = PTR_ERR(root);
>>   if (ret != -ENOENT)
>> - return ret;
>> + goto out;
>>   } else {
>>   set_bit(BTRFS_ROOT_TRACK_DIRTY, >state);
>>   fs_info->uuid_root = root;
>> @@ -2376,13 +2382,19 @@ static int btrfs_read_roots(struct btrfs_fs_info 
>> *fs_info)
>>   if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) {
>>   location.objectid = BTRFS_FREE_SPACE_TREE_OBJECTID;
>>   root = btrfs_read_tree_root(tree_root, );
>> - if (IS_ERR(root))
>> - return PTR_ERR(root);
>> + if (IS_ERR(root)) {
>> + ret = PTR_ERR(root);
>> + goto out;
>> + }
>>   set_bit(BTRFS_ROOT_TRACK_DIRTY, >state);
>>   fs_info->free_space_root = root;
>>   }
>>
>>   return 0;
>> +out:
>> + btrfs_warn(fs_info, "failed to read root (objectid=%llu): %d",
>> +location.objectid, ret);
>
> Since those are critical errors don't we want btrfs_err here?
>

Not much difference to me, I followed the convention, after failing to
read tree roots and falling back to recovery tree roots array,
btrfs_warn is used.

tree_root->node = read_tree_block();
if () {
   btrfs_warn(...);
   goto recovery_tree_roots;
}

thanks,
liubo


>> + return ret;
>>  }
>>
>>  int open_ctree(struct super_block *sb,
>> @@ -2953,6 +2965,7 @@ int open_ctree(struct super_block *sb,
>>   fs_info->fs_root = btrfs_read_fs_root_no_name(fs_info, );
>>   if (IS_ERR(fs_info->fs_root)) {
>>   err = PTR_ERR(fs_info->fs_root);
>> + btrfs_warn(fs_info, "failed to read fs tree: %d", err);
>
> ditto
>
>>   goto fail_qgroup;
>>   }
>>
>>
> --
> 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

Re: [PATCH] fstests: test btrfs fsync after hole punching with no-holes mode

[Filipe Manana]

On Wed, Mar 28, 2018 at 11:33 AM, Eryu Guan  wrote:
> On Wed, Mar 28, 2018 at 09:48:17AM +0100, Filipe Manana wrote:
>> On Wed, Mar 28, 2018 at 3:17 AM, Eryu Guan  wrote:
>> > On Mon, Mar 26, 2018 at 11:59:21PM +0100, fdman...@kernel.org wrote:
>> >> From: Filipe Manana 
>> >>
>> >> Test that when we have the no-holes mode enabled and a specific metadata
>> >> layout, if we punch a hole and fsync the file, at replay time the whole
>> >> hole was preserved.
>> >>
>> >> This issue is fixed by the following btrfs patch for the linux kernel:
>> >>
>> >>   "Btrfs: fix fsync after hole punching when using no-holes feature"
>> >
>> > I'd expect a test failure with 4.16-rc6 kernel, as the mentioned fix
>> > above is not there. But test always passes for me. Did I miss anything?
>> > btrfs-progs version is btrfs-progs-4.11.1-3.fc27.
>>
>> It should fail on any kernel, with any btrfs-progs version (which
>> should be irrelevant).
>> Somehow on your system we are not getting the specific metadata layout
>> needed to trigger the issue.
>>
>> Can you apply the following patch on top of the test and provide the
>> result 159.full file?
>>
>> https://friendpaste.com/6xAuLeN4xl1AGjO9Qc5I8L
>>
>> So that I can see what metadata layout you are getting.
>> Thanks!
>
> Sure, please see attachment.

Thanks!
So you indeed get a different metadata layout, and that is because you
have SELinux enabled which causes some xattr to be added to the test
file (foobar):

item 6 key (257 XATTR_ITEM 3817753667) itemoff 64932 itemsize 83
location key (0 UNKNOWN.0 0) type XATTR
transid 7 data_len 37 name_len 16
name: security.selinux
data unconfined_u:object_r:unlabeled_t:s0

I can make the test work with and without selinux enabled (by punching
holes on a few extents that are candidates to be at leaf boundaries).
Is it worth it? (I assume most people run the tests without selinux)

thanks

>
> Thanks,
> Eryu
--
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

Re: [PATCH v3 5/7] btrfs: verify superblock checksum during scan

[Nikolay Borisov]

On 29.03.2018 13:37, Anand Jain wrote:
> During the scan context, we aren't verifying the superblock-
> checksum when read.
> This patch fixes it by adding the checksum verification function
> btrfs_check_super_csum() in the function btrfs_read_disk_super().
> And makes device scan to error fail if the primary superblock csum
> is wrong, whereas if the copy-superblock csum is wrong it will just
> just report mismatch and continue mount/scan as usual. When the
> mount is successful We anyway overwrite all superblocks upon unmount.
> 
> The context in which this will be called is - device scan, device ready,
> and mount -o device option.
> 
> Test script:
> 
>  Corrupt primary superblock and check if device scan and mount
>  fails:
>   mkfs.btrfs -fq /dev/sdc
>   dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
>   btrfs dev scan /dev/sdc
>   mount /dev/sdc /btrfs
> 
>  Corrupt secondary superblock and check if device scan and mount
>  is succcessful, check for the dmesg for errors.
>   mkfs.btrfs -fq /dev/sdc
>   dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=67108864
>   btrfs dev scan /dev/sdc
>   mount /dev/sdc /btrfs
> 
> Signed-off-by: Anand Jain 
> v3->v2:
>  Also squash 4/8 in here.
>   4/8: btrfs: make btrfs_check_super_csum() non-static
> v1->v2:
>  changed title.
>  use explicit (< 0) check for %errr.
>  Un-split pr_err() string.
>  Fix typo in the git commit log.
>  Move the csum check after bytenr and btrfs magic verified.
> 
> btrfs: make btrfs_check_super_csum() non-static
> 
> In preparation to add the superblock csum verification for the
> scan context, make btrfs_check_super_csum() non-static.
> 
> Signed-off-by: Anand Jain 
> Reviewed-by: Nikolay Borisov 

This patch's changelog is garbaged.  Duplicated signed off by ,
changelog above scissors etc. Please review your patches carefully
before sending upstream.
> ---
>  fs/btrfs/disk-io.c |  2 +-
>  fs/btrfs/disk-io.h |  1 +
>  fs/btrfs/volumes.c | 13 +
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 4c573602480c..35dbbdc613cd 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -395,7 +395,7 @@ static int verify_parent_transid(struct extent_io_tree 
> *io_tree,
>   * Otherwise:-EINVAL  if csum type is not found
>   *   -EUCLEAN if csum does not match
>   */
> -static int btrfs_check_super_csum(char *raw_disk_sb)
> +int btrfs_check_super_csum(char *raw_disk_sb)
>  {
>   struct btrfs_super_block *disk_sb =
>   (struct btrfs_super_block *)raw_disk_sb;
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 70a88d61b547..c400cc68f913 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -69,6 +69,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int 
> max_mirrors);
>  struct buffer_head *btrfs_read_dev_super(struct block_device *bdev);
>  int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
>   struct buffer_head **bh_ret);
> +int btrfs_check_super_csum(char *raw_disk_sb);
>  int btrfs_commit_super(struct btrfs_fs_info *fs_info);
>  struct btrfs_root *btrfs_read_fs_root(struct btrfs_root *tree_root,
> struct btrfs_key *location);
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 035affa447fa..f1f074fbfee1 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1149,6 +1149,7 @@ static int btrfs_read_disk_super(struct block_device 
> *bdev, u64 bytenr,
>struct page **page,
>struct btrfs_super_block **disk_super)
>  {
> + int err;
>   void *p;
>   pgoff_t index;
>  
> @@ -1183,6 +1184,18 @@ static int btrfs_read_disk_super(struct block_device 
> *bdev, u64 bytenr,
>   return -EINVAL;
>   }
>  
> + err = btrfs_check_super_csum((char *) *disk_super);
> + if (err < 0) {
> + if (err == -EINVAL)
> + pr_err("BTRFS error (device %pg): unsupported checksum 
> type, bytenr=%llu",
> + bdev, bytenr);
> + else
> + pr_err("BTRFS error (device %pg): superblock checksum 
> failed, bytenr=%llu",
> + bdev, bytenr);
> + btrfs_release_disk_super(*page);
> + return err;
> + }
> +
>   if ((*disk_super)->label[0] &&
>   (*disk_super)->label[BTRFS_LABEL_SIZE - 1])
>   (*disk_super)->label[BTRFS_LABEL_SIZE - 1] = '\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

Re: [PATCH v2 4/7] btrfs: check if the fsid in the primary sb and copy sb are same

[Nikolay Borisov]

On 29.03.2018 13:37, Anand Jain wrote:
> During the btrfs dev scan make sure that other copies of superblock
> contain the same fsid as the primary SB. So that we bring to the
> user notice if the superblock has been overwritten.
> 
>  mkfs.btrfs -fq /dev/sdc
>  mkfs.btrfs -fq /dev/sdb
>  dd if=/dev/sdb of=/dev/sdc count=4K skip=64K seek=64K obs=1 ibs=1
>  mount /dev/sdc /btrfs
> 
> Caveat: Pls note that older btrfs-progs do not wipe the non-overwriting
> stale superblock like copy2 if a smaller mkfs.btrfs -b   is created.
> Thus this patch in the kernel will report error. The workaround is to wipe
> the superblock manually, like
>   dd if=/dev/zero of= seek=274877906944 ibs=1 obs=1 count4K
> OR apply the btrfs-progs patch
>   btrfs-progs: wipe copies of the stale superblock beyond -b size
> which shall find and wipe the non overwriting superblock during mkfs.
> 
> Signed-off-by: Anand Jain 
> v1->v2:
>   Do an explicit read for primary superblock. Drop kzalloc().
>   Fix split pr_err().

Same thing about the change log as in 2/7. Otherwise :

Reviewed-by: Nikolay Borisov 

> ---
>  fs/btrfs/volumes.c | 48 ++--
>  1 file changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index e63723f23227..035affa447fa 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1198,39 +1198,67 @@ static int btrfs_read_disk_super(struct block_device 
> *bdev, u64 bytenr,
>  int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
> struct btrfs_fs_devices **fs_devices_ret)
>  {
> + struct btrfs_super_block *disk_super_primary;
>   struct btrfs_super_block *disk_super;
>   struct btrfs_device *device;
>   struct block_device *bdev;
> + struct page *page_primary;
>   struct page *page;
>   int ret = 0;
>   u64 bytenr;
> + int i;
>  
> - /*
> -  * we would like to check all the supers, but that would make
> -  * a btrfs mount succeed after a mkfs from a different FS.
> -  * So, we need to add a special mount option to scan for
> -  * later supers, using BTRFS_SUPER_MIRROR_MAX instead
> -  */
> - bytenr = btrfs_sb_offset(0);
>   flags |= FMODE_EXCL;
>  
>   bdev = blkdev_get_by_path(path, flags, holder);
>   if (IS_ERR(bdev))
>   return PTR_ERR(bdev);
>  
> - ret = btrfs_read_disk_super(bdev, bytenr, , _super);
> + /*
> +  * We would like to check all the supers and use one good copy,
> +  * but that would make a btrfs mount succeed after a mkfs from
> +  * a different FS.
> +  * So, we need to add a special mount option to scan for
> +  * later supers, using BTRFS_SUPER_MIRROR_MAX instead.
> +  * So, just validate if all copies of the superblocks are ok
> +  * and have the same fsid.
> +  */
> + bytenr = btrfs_sb_offset(0);
> + ret = btrfs_read_disk_super(bdev, bytenr, _primary,
> + _super_primary);
>   if (ret < 0)
>   goto error_bdev_put;
>  
> + for (i = 1; i < BTRFS_SUPER_MIRROR_MAX; i++) {
> + bytenr = btrfs_sb_offset(i);
> + ret = btrfs_read_disk_super(bdev, bytenr, , _super);
> + if (ret < 0) {
> + ret = 0;
> + continue;
> + }
> +
> + if (memcmp(disk_super_primary->fsid, disk_super->fsid,
> +BTRFS_FSID_SIZE)) {
> + pr_err("BTRFS (device %pg): superblock fsid mismatch, 
> primary %pU copy%d %pU",
> + bdev, disk_super_primary->fsid, i,
> + disk_super->fsid);
> + ret = -EINVAL;
> + btrfs_release_disk_super(page);
> + goto error_rel_primary;
> + }
> + btrfs_release_disk_super(page);
> + }
> +
>   mutex_lock(_mutex);
> - device = device_list_add(path, disk_super);
> + device = device_list_add(path, disk_super_primary);
>   if (IS_ERR(device))
>   ret = PTR_ERR(device);
>   else
>   *fs_devices_ret = device->fs_devices;
>   mutex_unlock(_mutex);
>  
> - btrfs_release_disk_super(page);
> +error_rel_primary:
> + btrfs_release_disk_super(page_primary);
>  
>  error_bdev_put:
>   blkdev_put(bdev, flags);
> 
--
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

Re: [PATCH v2 2/7] btrfs: return required error from btrfs_check_super_csum

[Nikolay Borisov]

On 29.03.2018 13:36, Anand Jain wrote:
> Return the required -EINVAL and -EUCLEAN from the function
> btrfs_check_super_csum(). And more the error log into the
> parent function.
> 
> Signed-off-by: Anand Jain 
> v1->v2:
>  Fix commit indent reported by checkpatch.pl
>  Fix pr_err split string

The change log has to go below the scissors line. I guess it can be
fixed at merge time so I don't think a resubmit is warranted.

There is one nit below but I don't think it's critical so :

Reviewed-by: Nikolay Borisov 

> ---
>  fs/btrfs/disk-io.c | 25 ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b9b435579527..4c573602480c 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -392,9 +392,10 @@ static int verify_parent_transid(struct extent_io_tree 
> *io_tree,
>  /*
>   * Return 0 if the superblock checksum type matches the checksum value of 
> that
>   * algorithm. Pass the raw disk superblock data.
> + * Otherwise:-EINVAL  if csum type is not found
> + *   -EUCLEAN if csum does not match
>   */
> -static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
> -   char *raw_disk_sb)
> +static int btrfs_check_super_csum(char *raw_disk_sb)
>  {
>   struct btrfs_super_block *disk_sb =
>   (struct btrfs_super_block *)raw_disk_sb;
> @@ -402,11 +403,8 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
> *fs_info,
>   u32 crc = ~(u32)0;
>   char result[sizeof(crc)];
>  
> - if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
> - btrfs_err(fs_info, "unsupported checksum algorithm %u",
> -   csum_type);
> - return 1;
> - }
> + if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes))
> + return -EINVAL;
>  
>   /*
>* The super_block structure does not span the whole
> @@ -418,7 +416,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
> *fs_info,
>   btrfs_csum_final(crc, result);
>  
>   if (memcmp(raw_disk_sb, result, sizeof(result)))
> - return 1;
> + return -EUCLEAN;
>  
>   return 0;
>  }
> @@ -2570,9 +2568,14 @@ int open_ctree(struct super_block *sb,
>* We want to check superblock checksum, the type is stored inside.
>* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
>*/
> - if (btrfs_check_super_csum(fs_info, bh->b_data)) {
> - btrfs_err(fs_info, "superblock checksum mismatch");
> - err = -EINVAL;
> + err = btrfs_check_super_csum(bh->b_data);
> + if (err) {
> + if (err == -EINVAL)
> + pr_err("BTRFS error (device %pg): unsupported checksum 
> algorithm",
> + fs_devices->latest_bdev);
> + else
> + pr_err("BTRFS error (device %pg): superblock checksum 
> mismatch",
> + fs_devices->latest_bdev);

nit: I think your initial version of specifically checking for EUCLEAN
was better but I don't have strong preferences.
>   brelse(bh);
>   goto fail_alloc;
>   }
> 
--
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

[PATCH v3 5/7] btrfs: verify superblock checksum during scan

[Anand Jain]

During the scan context, we aren't verifying the superblock-
checksum when read.
This patch fixes it by adding the checksum verification function
btrfs_check_super_csum() in the function btrfs_read_disk_super().
And makes device scan to error fail if the primary superblock csum
is wrong, whereas if the copy-superblock csum is wrong it will just
just report mismatch and continue mount/scan as usual. When the
mount is successful We anyway overwrite all superblocks upon unmount.

The context in which this will be called is - device scan, device ready,
and mount -o device option.

Test script:

 Corrupt primary superblock and check if device scan and mount
 fails:
  mkfs.btrfs -fq /dev/sdc
  dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
  btrfs dev scan /dev/sdc
  mount /dev/sdc /btrfs

 Corrupt secondary superblock and check if device scan and mount
 is succcessful, check for the dmesg for errors.
  mkfs.btrfs -fq /dev/sdc
  dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=67108864
  btrfs dev scan /dev/sdc
  mount /dev/sdc /btrfs

Signed-off-by: Anand Jain 
v3->v2:
 Also squash 4/8 in here.
  4/8: btrfs: make btrfs_check_super_csum() non-static
v1->v2:
 changed title.
 use explicit (< 0) check for %errr.
 Un-split pr_err() string.
 Fix typo in the git commit log.
 Move the csum check after bytenr and btrfs magic verified.

btrfs: make btrfs_check_super_csum() non-static

In preparation to add the superblock csum verification for the
scan context, make btrfs_check_super_csum() non-static.

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
 fs/btrfs/disk-io.c |  2 +-
 fs/btrfs/disk-io.h |  1 +
 fs/btrfs/volumes.c | 13 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4c573602480c..35dbbdc613cd 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -395,7 +395,7 @@ static int verify_parent_transid(struct extent_io_tree 
*io_tree,
  * Otherwise:  -EINVAL  if csum type is not found
  * -EUCLEAN if csum does not match
  */
-static int btrfs_check_super_csum(char *raw_disk_sb)
+int btrfs_check_super_csum(char *raw_disk_sb)
 {
struct btrfs_super_block *disk_sb =
(struct btrfs_super_block *)raw_disk_sb;
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index 70a88d61b547..c400cc68f913 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -69,6 +69,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int 
max_mirrors);
 struct buffer_head *btrfs_read_dev_super(struct block_device *bdev);
 int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
struct buffer_head **bh_ret);
+int btrfs_check_super_csum(char *raw_disk_sb);
 int btrfs_commit_super(struct btrfs_fs_info *fs_info);
 struct btrfs_root *btrfs_read_fs_root(struct btrfs_root *tree_root,
  struct btrfs_key *location);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 035affa447fa..f1f074fbfee1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1149,6 +1149,7 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
 struct page **page,
 struct btrfs_super_block **disk_super)
 {
+   int err;
void *p;
pgoff_t index;
 
@@ -1183,6 +1184,18 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
return -EINVAL;
}
 
+   err = btrfs_check_super_csum((char *) *disk_super);
+   if (err < 0) {
+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
type, bytenr=%llu",
+   bdev, bytenr);
+   else
+   pr_err("BTRFS error (device %pg): superblock checksum 
failed, bytenr=%llu",
+   bdev, bytenr);
+   btrfs_release_disk_super(*page);
+   return err;
+   }
+
if ((*disk_super)->label[0] &&
(*disk_super)->label[BTRFS_LABEL_SIZE - 1])
(*disk_super)->label[BTRFS_LABEL_SIZE - 1] = '\0';
-- 
2.7.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

[PATCH v2 6/7] btrfs: verify checksum for all devices in mount context

[Anand Jain]

During mount context, we aren't verifying the superblock checksum
for all the devices, instead, we verify it only for the
struct btrfs_fs_device::latest_bdev. This patch fixes it by moving
the checksum verification code from the function open_ctree() into
the function btrfs_read_dev_one_super().

By doing this now we are verifying the superblock checksum in the
mount-context, device-replace and, device-delete context. The
device-replace and device-delete call-chain is as show below after
this patch.

delete-device/replace:
btrfs_rm_device() || btrfs_dev_replace_by_ioctl()
|_btrfs_find_device_by_devspec()
  |_btrfs_find_device_missing_or_by_path()
|_btrfs_find_device_by_path()
  |_btrfs_get_bdev_and_sb()
|_btrfs_read_dev_super()
  |_btrfs_read_dev_one_super()
|_btrfs_check_super_csum()

Test case:
 Before:
   mkfs.btrfs -fq -draid1 -mraid1 /dev/sdb /dev/sdc
   dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
   mount /dev/sdb /btrfs <-- success as kernel does not check csum
 for non-btrfs_fs_devices::latest_bdev.
 After:
   mkfs.btrfs -fq -draid1 -mraid1 /dev/sdb /dev/sdc
   dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
   mount /dev/sdb /btrfs
 mount: mount /dev/sdc on /btrfs failed: Structure needs cleaning
   mount -o degraded /dev/sdc /btrfs
 mount: mount /dev/sdc on /btrfs failed: Structure needs cleaning

 So the current recovery step is to fix the primary superblock, by
 using the btrfs-progs cli.

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
v1->v2:
 git commit log update. With the call-chain (which I believe will
  go away in the long term, we need the uuid from the userland not
  the disk-path). And add test case.
 Check err < 0 explicitly and drop check for "else if (err == -EUCLEAN)"
  v1:
err = btrfs_check_super_csum(bh->b_data);
if (err) {
if (err == -EINVAL)
pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
bdev);
else if (err == -EUCLEAN)
pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
bdev);
brelse(bh);
return err;
}
---
 fs/btrfs/disk-io.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 35dbbdc613cd..110465bec775 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2565,22 +2565,6 @@ int open_ctree(struct super_block *sb,
}
 
/*
-* We want to check superblock checksum, the type is stored inside.
-* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
-*/
-   err = btrfs_check_super_csum(bh->b_data);
-   if (err) {
-   if (err == -EINVAL)
-   pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
-   fs_devices->latest_bdev);
-   else
-   pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
-   fs_devices->latest_bdev);
-   brelse(bh);
-   goto fail_alloc;
-   }
-
-   /*
 * super_copy is zeroed at allocation time and we never touch the
 * following bytes up to INFO_SIZE, the checksum is calculated from
 * the whole block of INFO_SIZE
@@ -3126,6 +3110,7 @@ int btrfs_read_dev_one_super(struct block_device *bdev, 
int copy_num,
struct buffer_head *bh;
struct btrfs_super_block *super;
u64 bytenr;
+   int err;
 
bytenr = btrfs_sb_offset(copy_num);
if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
@@ -3146,6 +3131,22 @@ int btrfs_read_dev_one_super(struct block_device *bdev, 
int copy_num,
return -EINVAL;
}
 
+   /*
+* Check the superblock checksum, the type is stored inside.
+* Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
+*/
+   err = btrfs_check_super_csum(bh->b_data);
+   if (err < 0) {
+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
+   bdev);
+   else
+   pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
+   bdev);
+   brelse(bh);
+   return err;
+   }
+
*bh_ret = bh;
return 0;
 }
-- 
2.7.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

[PATCH v2 2/7] btrfs: return required error from btrfs_check_super_csum

[Anand Jain]

Return the required -EINVAL and -EUCLEAN from the function
btrfs_check_super_csum(). And more the error log into the
parent function.

Signed-off-by: Anand Jain 
v1->v2:
 Fix commit indent reported by checkpatch.pl
 Fix pr_err split string
---
 fs/btrfs/disk-io.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b9b435579527..4c573602480c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -392,9 +392,10 @@ static int verify_parent_transid(struct extent_io_tree 
*io_tree,
 /*
  * Return 0 if the superblock checksum type matches the checksum value of that
  * algorithm. Pass the raw disk superblock data.
+ * Otherwise:  -EINVAL  if csum type is not found
+ * -EUCLEAN if csum does not match
  */
-static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
- char *raw_disk_sb)
+static int btrfs_check_super_csum(char *raw_disk_sb)
 {
struct btrfs_super_block *disk_sb =
(struct btrfs_super_block *)raw_disk_sb;
@@ -402,11 +403,8 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
*fs_info,
u32 crc = ~(u32)0;
char result[sizeof(crc)];
 
-   if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
-   btrfs_err(fs_info, "unsupported checksum algorithm %u",
- csum_type);
-   return 1;
-   }
+   if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes))
+   return -EINVAL;
 
/*
 * The super_block structure does not span the whole
@@ -418,7 +416,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
*fs_info,
btrfs_csum_final(crc, result);
 
if (memcmp(raw_disk_sb, result, sizeof(result)))
-   return 1;
+   return -EUCLEAN;
 
return 0;
 }
@@ -2570,9 +2568,14 @@ int open_ctree(struct super_block *sb,
 * We want to check superblock checksum, the type is stored inside.
 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
 */
-   if (btrfs_check_super_csum(fs_info, bh->b_data)) {
-   btrfs_err(fs_info, "superblock checksum mismatch");
-   err = -EINVAL;
+   err = btrfs_check_super_csum(bh->b_data);
+   if (err) {
+   if (err == -EINVAL)
+   pr_err("BTRFS error (device %pg): unsupported checksum 
algorithm",
+   fs_devices->latest_bdev);
+   else
+   pr_err("BTRFS error (device %pg): superblock checksum 
mismatch",
+   fs_devices->latest_bdev);
brelse(bh);
goto fail_alloc;
}
-- 
2.7.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

[PATCH v2 4/7] btrfs: check if the fsid in the primary sb and copy sb are same

[Anand Jain]

During the btrfs dev scan make sure that other copies of superblock
contain the same fsid as the primary SB. So that we bring to the
user notice if the superblock has been overwritten.

 mkfs.btrfs -fq /dev/sdc
 mkfs.btrfs -fq /dev/sdb
 dd if=/dev/sdb of=/dev/sdc count=4K skip=64K seek=64K obs=1 ibs=1
 mount /dev/sdc /btrfs

Caveat: Pls note that older btrfs-progs do not wipe the non-overwriting
stale superblock like copy2 if a smaller mkfs.btrfs -b   is created.
Thus this patch in the kernel will report error. The workaround is to wipe
the superblock manually, like
  dd if=/dev/zero of= seek=274877906944 ibs=1 obs=1 count4K
OR apply the btrfs-progs patch
  btrfs-progs: wipe copies of the stale superblock beyond -b size
which shall find and wipe the non overwriting superblock during mkfs.

Signed-off-by: Anand Jain 
v1->v2:
  Do an explicit read for primary superblock. Drop kzalloc().
  Fix split pr_err().
---
 fs/btrfs/volumes.c | 48 ++--
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e63723f23227..035affa447fa 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1198,39 +1198,67 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
 int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
  struct btrfs_fs_devices **fs_devices_ret)
 {
+   struct btrfs_super_block *disk_super_primary;
struct btrfs_super_block *disk_super;
struct btrfs_device *device;
struct block_device *bdev;
+   struct page *page_primary;
struct page *page;
int ret = 0;
u64 bytenr;
+   int i;
 
-   /*
-* we would like to check all the supers, but that would make
-* a btrfs mount succeed after a mkfs from a different FS.
-* So, we need to add a special mount option to scan for
-* later supers, using BTRFS_SUPER_MIRROR_MAX instead
-*/
-   bytenr = btrfs_sb_offset(0);
flags |= FMODE_EXCL;
 
bdev = blkdev_get_by_path(path, flags, holder);
if (IS_ERR(bdev))
return PTR_ERR(bdev);
 
-   ret = btrfs_read_disk_super(bdev, bytenr, , _super);
+   /*
+* We would like to check all the supers and use one good copy,
+* but that would make a btrfs mount succeed after a mkfs from
+* a different FS.
+* So, we need to add a special mount option to scan for
+* later supers, using BTRFS_SUPER_MIRROR_MAX instead.
+* So, just validate if all copies of the superblocks are ok
+* and have the same fsid.
+*/
+   bytenr = btrfs_sb_offset(0);
+   ret = btrfs_read_disk_super(bdev, bytenr, _primary,
+   _super_primary);
if (ret < 0)
goto error_bdev_put;
 
+   for (i = 1; i < BTRFS_SUPER_MIRROR_MAX; i++) {
+   bytenr = btrfs_sb_offset(i);
+   ret = btrfs_read_disk_super(bdev, bytenr, , _super);
+   if (ret < 0) {
+   ret = 0;
+   continue;
+   }
+
+   if (memcmp(disk_super_primary->fsid, disk_super->fsid,
+  BTRFS_FSID_SIZE)) {
+   pr_err("BTRFS (device %pg): superblock fsid mismatch, 
primary %pU copy%d %pU",
+   bdev, disk_super_primary->fsid, i,
+   disk_super->fsid);
+   ret = -EINVAL;
+   btrfs_release_disk_super(page);
+   goto error_rel_primary;
+   }
+   btrfs_release_disk_super(page);
+   }
+
mutex_lock(_mutex);
-   device = device_list_add(path, disk_super);
+   device = device_list_add(path, disk_super_primary);
if (IS_ERR(device))
ret = PTR_ERR(device);
else
*fs_devices_ret = device->fs_devices;
mutex_unlock(_mutex);
 
-   btrfs_release_disk_super(page);
+error_rel_primary:
+   btrfs_release_disk_super(page_primary);
 
 error_bdev_put:
blkdev_put(bdev, flags);
-- 
2.7.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

[PATCH] btrfs-progs: wipe copies of the stale superblock beyond -b size

[Anand Jain]

During the mkfs.btrfs -b  btrfs_prepare_device() zeros all
the superblock bytenr locations only if the bytenr is below the
blockcount. The problem with this is that if the BTRFS is recreated
with a smaller size then we will leave the stale superblock in the disk
which shall confuse the recovery. As shown in the test case below.

 mkfs.btrfs -qf /dev/mapper/vg-lv
 mkfs.btrfs -qf -b1G /dev/mapper/vg-lv
 btrfs in dump-super -a /dev/mapper/vg-lv | grep '.fsid|superblock:'

 superblock: bytenr=65536, device=/dev/mapper/vg-lv
 dev_item.fsid  ebc67d01-7fc5-43f0-90b4-d1925002551e [match]
 superblock: bytenr=67108864, device=/dev/mapper/vg-lv
 dev_item.fsid  ebc67d01-7fc5-43f0-90b4-d1925002551e [match]
 superblock: bytenr=274877906944, device=/dev/mapper/vg-lv
 dev_item.fsid  b97a9206-593b-4933-a424-c6a6ee23fe7c [match]

So if we find a valid superblock zero it even if it's beyond the
blockcount.

Signed-off-by: Anand Jain 
---
 utils.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/utils.c b/utils.c
index e9cb3a82fda6..6a9408b06e73 100644
--- a/utils.c
+++ b/utils.c
@@ -365,6 +365,41 @@ int btrfs_prepare_device(int fd, const char *file, u64 
*block_count_ret,
return 1;
}
 
+   /*
+* Check for the BTRFS SB copies up until btrfs_device_size() and zero
+* it. So that kernel (or user for the manual recovery) don't have to
+* confuse with the stale SB copy during recovery.
+*/
+   if (block_count != btrfs_device_size(fd, )) {
+   for (i = 1; i < BTRFS_SUPER_MIRROR_MAX; i++) {
+   struct btrfs_super_block *disk_super;
+   char buf[BTRFS_SUPER_INFO_SIZE];
+   disk_super = (struct btrfs_super_block *)buf;
+
+   /* Already zeroed above */
+   if (btrfs_sb_offset(i) < block_count)
+   continue;
+
+   /* Beyond actual disk size */
+   if (btrfs_sb_offset(i) >= btrfs_device_size(fd, ))
+   continue;
+
+   /* Does not contain any stale SB */
+   if (btrfs_read_dev_super(fd, disk_super,
+btrfs_sb_offset(i), 0))
+   continue;
+
+   ret = zero_dev_clamped(fd, btrfs_sb_offset(i),
+   BTRFS_SUPER_INFO_SIZE,
+   btrfs_device_size(fd, ));
+   if (ret < 0) {
+   error("failed to zero device '%s' bytenr %llu: 
%s",
+   file, btrfs_sb_offset(i), 
strerror(-ret));
+   return 1;
+   }
+   }
+   }
+
*block_count_ret = block_count;
return 0;
 }
-- 
2.7.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

[PATCH 7/7] btrfs: drop the redundant invalidate_bdev()

[Anand Jain]

During the mount context btrfs_open_devices() calls invalidate_bdev()
for all the devices. So drop the invalidate_bdev() in open_ctree()
which is only for the btrfs_fs_devices::latest_bdev.

The call trace is as shown below.

btrfs_mount_root()
 |
 |_btrfs_parse_early_options (-o device only)
 | |_btrfs_scan_one_device
 |   |_btrfs_read_disk_super()
 | |_read_cache_page_gfp()
 |
 |_btrfs_scan_one_device(mount-arg-dev only)
 | |_btrfs_read_disk_super()
 |   |_read_cache_page_gfp()
 |
 |
 |_btrfs_open_devices(fsid:all)
 |  |_btrfs_open_one_device()
 ||_btrfs_get_bdev_and_sb()  <--- invalidate_bdev(fsid:all)
 |  |_btrfs_read_dev_super()
 ||_btrfs_read_dev_one_super()
 |  |___bread()
 |
 |_btrfs_fill_super()
   |_btrfs_open_ctree()   <-- invalidate_bdev(latest_bdev) <-- redundant
 |_btrfs_read_dev_super(latest_bdev only)
   |_btrfs_read_dev_one_super(latest_bdev only)
 |___bread(latest_bdev)

Signed-off-by: Anand Jain 
---
 fs/btrfs/disk-io.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 110465bec775..628c2b1d1349 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2553,8 +2553,6 @@ int open_ctree(struct super_block *sb,
 
__setup_root(tree_root, fs_info, BTRFS_ROOT_TREE_OBJECTID);
 
-   invalidate_bdev(fs_devices->latest_bdev);
-
/*
 * Read super block and check the signature bytes only
 */
-- 
2.7.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

[PATCH 1/7] btrfs: cleanup btrfs_check_super_csum() for better code flow

[Anand Jain]

We check the %csum_type twice. Drop one. Check for the csum_type and
then for the csum. Which also matches with the progs which have better
logic. This is preparatory patch to get proper error code from
btrfs_check_super_csum().

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
---
 fs/btrfs/disk-io.c | 38 +-
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1657d6aa4fa6..b9b435579527 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -399,32 +399,28 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
*fs_info,
struct btrfs_super_block *disk_sb =
(struct btrfs_super_block *)raw_disk_sb;
u16 csum_type = btrfs_super_csum_type(disk_sb);
-   int ret = 0;
-
-   if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
-   u32 crc = ~(u32)0;
-   char result[sizeof(crc)];
-
-   /*
-* The super_block structure does not span the whole
-* BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
-* is filled with zeros and is included in the checksum.
-*/
-   crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
-   crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-   btrfs_csum_final(crc, result);
-
-   if (memcmp(raw_disk_sb, result, sizeof(result)))
-   ret = 1;
-   }
+   u32 crc = ~(u32)0;
+   char result[sizeof(crc)];
 
if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
btrfs_err(fs_info, "unsupported checksum algorithm %u",
-   csum_type);
-   ret = 1;
+ csum_type);
+   return 1;
}
 
-   return ret;
+   /*
+* The super_block structure does not span the whole
+* BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
+* is filled with zeros and is included in the checksum.
+*/
+   crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
+ crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+   btrfs_csum_final(crc, result);
+
+   if (memcmp(raw_disk_sb, result, sizeof(result)))
+   return 1;
+
+   return 0;
 }
 
 /*
-- 
2.7.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

[PATCH v3 0/9] Superblock read and verify cleanups

[Anand Jain]

v2->v3:
 Squash
  4/8 btrfs: make btrfs_check_super_csum() non static
 to
  6/8 btrfs: verify superblock checksum during scan
 As in the individual patch mentioned
 
v1->v2:
 Various changes suggested by Nicokay. Thanks. For specific changes
 pls ref to the patch.

Patch 1-4/8 are preparatory patches adds cleanups and nonstatic requisites.

Patch 5/8 makes sure that all copies of the superblock have the same fsid
when we scan the device.

Patch 6/8 verifies superblock csum when we read it in the scan context.

Patch 7/8 fixes a bug that we weren't verifying the superblock csum for
the non-latest_bdev.

And 8/8 patch drops the redundant invalidate_bdev() call during mount.

There is a btrfs-progs patch which is a kind of related, as its found that
we weren't wiping the non-overwritten superblock, so it could cause
confusion during the superblock recovery process. So the patch btrfs-progs
1/1 adds code to wipe superblock if we aren't overwriting it.

Now since kernel patch 5/8 checks if all the superblock copies are
pointing to the same fsid on the disk, so the scan will fail if without
the above 1/1 btrfs-progs, as in the example below [1]. However the simple
workaround is to wipe the superblock manually [2] or apply the btrfs-progs
patch below.

[1]
 mkfs.btrfs -qf /dev/sdb <-- 1T disk
 mkfs.btrfs -b 256M  /dev/sdb
 ERROR: device scan failed on /dev/sdb

[2]
 dd if=/dev/zero of= seek=274877906944 ibs=1 obs=1 count4K

Unfortunately, the error messages should have been failed to register
[3] device into the kernel to be more appropriate to the error.

[3]
ret = ioctl(fd, BTRFS_IOC_SCAN_DEV, );
if (ret < 0) {
error("device scan failed on '%s': %m", fname);
ret = -errno;
}


Patches 1-7/8 were sent independently before. And  I found few more things
to fix alongs the line, and since they are related, so I am sending these
all together. Also, as there are minor changes, like in pr_err strings,
and splitting the unrelated changes into a separate patch, so though I am
thankful for the received reviewed-by, I couldn't include them here. Sorry.

Finally, here I am including the function relations [4] so that it will help
to review the code. And this flow is before these patches were applied.

[4]
In the long term, I suggest deprecating ioctl args which pass device path
(where possible), like in delete-device/replace. And
btrfs_read_dev_one_super() should replace btrfs_read_disk_super()

delete-device/replace:
btrfs_rm_device() || btrfs_dev_replace_by_ioctl()
|_btrfs_find_device_by_devspec()
  |_btrfs_find_device_missing_or_by_path()
|_btrfs_find_device_by_path()
  |_btrfs_get_bdev_and_sb()
|_btrfs_read_dev_super()
  |_btrfs_read_dev_one_super()
|___bread()

btrfs_mount_root()
 |
 |_btrfs_parse_early_options (-o device only)
 | |_btrfs_scan_one_device
 |   |_btrfs_read_disk_super()
 | |_read_cache_page_gfp()
 |
 |_btrfs_scan_one_device(mount-arg-dev only)
 | |_btrfs_read_disk_super()
 |   |_read_cache_page_gfp()
 |
 |
 |_btrfs_open_devices(fsid:all)
 |  |_btrfs_open_one_device()
 ||_btrfs_get_bdev_and_sb()  <--- invalidate_bdev(fsid:all)
 |  |_btrfs_read_dev_super()
 ||_btrfs_read_dev_one_super()
 |  |___bread()
 |
 |_btrfs_fill_super()
   |_btrfs_open_ctree()   <-- invalidate_bdev(latest_bdev) <-- redundant
 |_btrfs_read_dev_super(latest_bdev only)
 | |_btrfs_read_dev_one_super(latest_bdev only)
 |   |___bread(latest_bdev)
 |
 |_btrfs_check_super_csum(latest_bdev only) [*]
 |
 |_btrfs_read_chunk_tree
 | |_read_one_dev()
 |   |_open_seed_devices()
 | |_btrfs_open_devices(fs_devices->seed only)

scan/ready
|
|_btrfs_scan_one_device(ioctl-arg-dev only)
   |_btrfs_read_disk_super()
 |_read_cache_page_gfp()


Anand Jain (8):
  btrfs: cleanup btrfs_check_super_csum() for better code flow
  btrfs: return required error from btrfs_check_super_csum
  btrfs: cleanup btrfs_read_disk_super() to return std error
  btrfs: check if the fsid in the primary sb and copy sb are same
  btrfs: verify checksum when superblock is read for scan
  btrfs: verify checksum for all devices in mount context
  btrfs: drop the redundant invalidate_bdev()

 fs/btrfs/disk-io.c | 72 +++---
 fs/btrfs/disk-io.h |  1 +
 fs/btrfs/volumes.c | 84 ++
 3 files changed, 102 insertions(+), 55 deletions(-)

Anand Jain (1):
  btrfs-progs: wipe copies of the stale superblock beyond -b size

 utils.c | 35 +++
 1 file changed, 35 insertions(+)

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

[PATCH v2 3/7] btrfs: cleanup btrfs_read_disk_super() to return std error

[Anand Jain]

The only caller btrfs_scan_one_device() sets -EINVAL for error from
btrfs_read_disk_super(), so this patch returns -EINVAL from the latter
function.

A preparatory patch to add csum check in btrfs_read_disk_super().

Signed-off-by: Anand Jain 
Reviewed-by: Nikolay Borisov 
v1->v2:
 Check btrfs_read_disk_super() to be more explicit ret < 0
---
 fs/btrfs/volumes.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 87d183accdb2..e63723f23227 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1154,23 +1154,23 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
 
/* make sure our super fits in the device */
if (bytenr + PAGE_SIZE >= i_size_read(bdev->bd_inode))
-   return 1;
+   return -EINVAL;
 
/* make sure our super fits in the page */
if (sizeof(**disk_super) > PAGE_SIZE)
-   return 1;
+   return -EINVAL;
 
/* make sure our super doesn't straddle pages on disk */
index = bytenr >> PAGE_SHIFT;
if ((bytenr + sizeof(**disk_super) - 1) >> PAGE_SHIFT != index)
-   return 1;
+   return -EINVAL;
 
/* pull in the page with our super */
*page = read_cache_page_gfp(bdev->bd_inode->i_mapping,
   index, GFP_KERNEL);
 
if (IS_ERR_OR_NULL(*page))
-   return 1;
+   return -EINVAL;
 
p = kmap(*page);
 
@@ -1180,7 +1180,7 @@ static int btrfs_read_disk_super(struct block_device 
*bdev, u64 bytenr,
if (btrfs_super_bytenr(*disk_super) != bytenr ||
btrfs_super_magic(*disk_super) != BTRFS_MAGIC) {
btrfs_release_disk_super(*page);
-   return 1;
+   return -EINVAL;
}
 
if ((*disk_super)->label[0] &&
@@ -1218,10 +1218,9 @@ int btrfs_scan_one_device(const char *path, fmode_t 
flags, void *holder,
if (IS_ERR(bdev))
return PTR_ERR(bdev);
 
-   if (btrfs_read_disk_super(bdev, bytenr, , _super)) {
-   ret = -EINVAL;
+   ret = btrfs_read_disk_super(bdev, bytenr, , _super);
+   if (ret < 0)
goto error_bdev_put;
-   }
 
mutex_lock(_mutex);
device = device_list_add(path, disk_super);
-- 
2.7.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

[PATCH typo-fixed] fstests: btrfs: 159 superblock corruption test case

[Anand Jain]

Verify if the superblock corruption is handled correctly.

Signed-off-by: Anand Jain 
---
 tests/btrfs/159 | 142 
 tests/btrfs/159.out |  35 +
 tests/btrfs/group   |   1 +
 3 files changed, 178 insertions(+)
 create mode 100755 tests/btrfs/159
 create mode 100644 tests/btrfs/159.out

diff --git a/tests/btrfs/159 b/tests/btrfs/159
new file mode 100755
index ..f42ba4b777e7
--- /dev/null
+++ b/tests/btrfs/159
@@ -0,0 +1,142 @@
+#! /bin/bash
+# FS QA Test 159
+#
+# Test if the superblock corruption is handled correctly.
+#   - Check and validate superblock csum in mount and scan context
+#   - Check and validate superblock for all disks in the fs
+#   - Make sure if the copies are really a copy of the primary superblock
+#
+#-
+# Copyright (c) 2018 Oracle.  All Rights Reserved.
+# Author: Anand Jain 
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _scratch_dev_pool_put
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/module
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+_require_loadable_fs_module "btrfs"
+_require_command "$WIPEFS_PROG" wipefs
+
+_scratch_dev_pool_get 2
+
+MNT_OPT=$(echo $MOUNT_OPTIONS | cut -d" " -f2-)
+DEV_GOOD=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+DEV_BAD=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+
+wipe()
+{
+   $WIPEFS_PROG -a $DEV_GOOD > /dev/null 2>&1
+   $WIPEFS_PROG -a $DEV_BAD > /dev/null 2>&1
+}
+
+# If copy1 fsid does not match with primary fail the scan thus the mount as 
well
+check_copy1_fsid()
+{
+   local bytenr=67108864
+   echo -e "\\ncheck_copy1_fsid\\n{" | tee -a $seqres.full
+
+   wipe
+   $MKFS_BTRFS_PROG -fq $DEV_GOOD
+   $MKFS_BTRFS_PROG -fq $DEV_BAD
+   _reload_fs_module "btrfs"
+
+   dd status=none of=$DEV_BAD if=$DEV_GOOD ibs=1 obs=1 skip=$bytenr 
seek=$bytenr count=4K|\
+   tee -a $seqres.full
+
+   _mount -o $MNT_OPT $DEV_BAD $SCRATCH_MNT 2>&1 | _filter_scratch_pool
+
+   echo -e "good\\n}" | tee -a $seqres.full
+}
+
+# As in Linux kernel 4.16 if the primary is corrupted mount will fail.
+# Which might change in the long run.
+check_primary()
+{
+   local bytenr=65536
+   echo -e "\\ncheck_primary\\n{" | tee -a $seqres.full
+
+   wipe
+   _scratch_pool_mkfs "-mraid1 -draid1"
+   _reload_fs_module "btrfs"
+
+   #To corrupt a disk block, read in hex, write in dec
+   od -j$bytenr -N1 -An -x $DEV_BAD |\
+   dd status=none of=$DEV_BAD ibs=1 obs=1 seek=$bytenr count=1|\
+   tee -a $seqres.full
+   _mount -o $MNT_OPT,device=$DEV_GOOD $DEV_BAD $SCRATCH_MNT 2>&1 | 
_filter_scratch_pool
+
+   echo -e "good\\n}" | tee -a $seqres.full
+}
+
+# If copy1 or copy2 is corrupted we still should be able to mount
+check_copy1()
+{
+   local bytenr=67108864
+   echo -e "\\ncheck_copy1\\n{" | tee -a $seqres.full
+
+   wipe
+   _scratch_pool_mkfs "-mraid1 -draid1"
+   _reload_fs_module "btrfs"
+
+   #corrupt the disk block bytenr, read in hex, write in dec
+   od -j$bytenr -N1 -An -x $DEV_BAD |\
+   dd status=none of=$DEV_BAD ibs=1 obs=1 seek=$bytenr count=1|\
+   tee -a $seqres.full
+   _mount -o $MNT_OPT,device=$DEV_GOOD $DEV_BAD $SCRATCH_MNT 2>&1 | 
_filter_scratch_pool
+
+   echo -e "good\\n}" | tee -a $seqres.full
+
+   _scratch_unmount
+}
+
+check_copy1_fsid
+
+check_primary
+check_copy1
+
+echo -e "\\nReverse the good and bad device"
+# Generally devid 1 is used to read tree at the mount time, now reverse the
+# devid on which the corrupt superblock will reside.
+dev_tmp=$DEV_GOOD
+DEV_GOOD=$DEV_BAD
+DEV_BAD=$dev_tmp
+check_primary
+check_copy1
+
+status=0
+exit
diff --git a/tests/btrfs/159.out b/tests/btrfs/159.out
new file mode 100644
index ..f8f776f28870
--- /dev/null
+++ 

Re: [PATCH v2 5/8] btrfs: check if the fsid in the primary sb and copy sb are same

[Anand Jain]

-   btrfs_release_disk_super(page);
->  error_bdev_put:


You need a check whether page_primary is set here and release it,
otherwise you are leaking it. This needs to handle both the primary sb
read failure (where page_primary might not be set) as well as any FSID
mismatch from loop. (where it will be set)> 


 Right. I should. Will do.

Thanks. Anand


blkdev_put(bdev, flags);
  


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

Re: [PATCH v2 6/8] btrfs: verify superblock checksum during scan

[Anand Jain]

On 03/28/2018 03:21 PM, Nikolay Borisov wrote:



On 28.03.2018 02:39, Anand Jain wrote:

During the scan context, we aren't verifying the superblock-
checksum when read.
This patch fixes it by adding the checksum verification function
btrfs_check_super_csum() in the function btrfs_read_disk_super().
And makes device scan to error fail if the primary superblock csum
is wrong, whereas if the copy-superblock csum is wrong it will just
just report mismatch and continue mount/scan as usual. When the


Where in this patch do you deal with the secondary sb.




btrfs_read_disk_super verifies only the primary sb which corresponds to
the first part of the sentence. But I'm confused about the second?


 btrfs_read_disk_super() actaully reads and csum verifies the copy_num 
as provided.


bytenr = btrfs_sb_offset(copy_num);


I also think that 4/8 should be merged into this one.


 Right. I can do that.

Thanks, Anand
--
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

[PATCH] fstests: btrfs: 159 superblock courruption test case

[Anand Jain]

Verify if the superblock corruption is handled correctly.

Signed-off-by: Anand Jain 
---
 tests/btrfs/159 | 142 
 tests/btrfs/159.out |  35 +
 tests/btrfs/group   |   1 +
 3 files changed, 178 insertions(+)
 create mode 100755 tests/btrfs/159
 create mode 100644 tests/btrfs/159.out

diff --git a/tests/btrfs/159 b/tests/btrfs/159
new file mode 100755
index ..f42ba4b777e7
--- /dev/null
+++ b/tests/btrfs/159
@@ -0,0 +1,142 @@
+#! /bin/bash
+# FS QA Test 159
+#
+# Test if the superblock corruption is handled correctly.
+#   - Check and validate superblock csum in mount and scan context
+#   - Check and validate superblock for all disks in the fs
+#   - Make sure if the copies are really a copy of the primary superblock
+#
+#-
+# Copyright (c) 2018 Oracle.  All Rights Reserved.
+# Author: Anand Jain 
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _scratch_dev_pool_put
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/module
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch_dev_pool 2
+_require_loadable_fs_module "btrfs"
+_require_command "$WIPEFS_PROG" wipefs
+
+_scratch_dev_pool_get 2
+
+MNT_OPT=$(echo $MOUNT_OPTIONS | cut -d" " -f2-)
+DEV_GOOD=$(echo $SCRATCH_DEV_POOL | awk '{print $1}')
+DEV_BAD=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
+
+wipe()
+{
+   $WIPEFS_PROG -a $DEV_GOOD > /dev/null 2>&1
+   $WIPEFS_PROG -a $DEV_BAD > /dev/null 2>&1
+}
+
+# If copy1 fsid does not match with primary fail the scan thus the mount as 
well
+check_copy1_fsid()
+{
+   local bytenr=67108864
+   echo -e "\\ncheck_copy1_fsid\\n{" | tee -a $seqres.full
+
+   wipe
+   $MKFS_BTRFS_PROG -fq $DEV_GOOD
+   $MKFS_BTRFS_PROG -fq $DEV_BAD
+   _reload_fs_module "btrfs"
+
+   dd status=none of=$DEV_BAD if=$DEV_GOOD ibs=1 obs=1 skip=$bytenr 
seek=$bytenr count=4K|\
+   tee -a $seqres.full
+
+   _mount -o $MNT_OPT $DEV_BAD $SCRATCH_MNT 2>&1 | _filter_scratch_pool
+
+   echo -e "good\\n}" | tee -a $seqres.full
+}
+
+# As in Linux kernel 4.16 if the primary is corrupted mount will fail.
+# Which might change in the long run.
+check_primary()
+{
+   local bytenr=65536
+   echo -e "\\ncheck_primary\\n{" | tee -a $seqres.full
+
+   wipe
+   _scratch_pool_mkfs "-mraid1 -draid1"
+   _reload_fs_module "btrfs"
+
+   #To corrupt a disk block, read in hex, write in dec
+   od -j$bytenr -N1 -An -x $DEV_BAD |\
+   dd status=none of=$DEV_BAD ibs=1 obs=1 seek=$bytenr count=1|\
+   tee -a $seqres.full
+   _mount -o $MNT_OPT,device=$DEV_GOOD $DEV_BAD $SCRATCH_MNT 2>&1 | 
_filter_scratch_pool
+
+   echo -e "good\\n}" | tee -a $seqres.full
+}
+
+# If copy1 or copy2 is corrupted we still should be able to mount
+check_copy1()
+{
+   local bytenr=67108864
+   echo -e "\\ncheck_copy1\\n{" | tee -a $seqres.full
+
+   wipe
+   _scratch_pool_mkfs "-mraid1 -draid1"
+   _reload_fs_module "btrfs"
+
+   #corrupt the disk block bytenr, read in hex, write in dec
+   od -j$bytenr -N1 -An -x $DEV_BAD |\
+   dd status=none of=$DEV_BAD ibs=1 obs=1 seek=$bytenr count=1|\
+   tee -a $seqres.full
+   _mount -o $MNT_OPT,device=$DEV_GOOD $DEV_BAD $SCRATCH_MNT 2>&1 | 
_filter_scratch_pool
+
+   echo -e "good\\n}" | tee -a $seqres.full
+
+   _scratch_unmount
+}
+
+check_copy1_fsid
+
+check_primary
+check_copy1
+
+echo -e "\\nReverse the good and bad device"
+# Generally devid 1 is used to read tree at the mount time, now reverse the
+# devid on which the corrupt superblock will reside.
+dev_tmp=$DEV_GOOD
+DEV_GOOD=$DEV_BAD
+DEV_BAD=$dev_tmp
+check_primary
+check_copy1
+
+status=0
+exit
diff --git a/tests/btrfs/159.out b/tests/btrfs/159.out
new file mode 100644
index ..f8f776f28870
--- /dev/null
+++ 

[PATCH] btrfs-progs: Do not add extra slash if given path end with it

[Gu Jinxiang]

When use a given path end with a slash like below,
the output of path will have double slash.

Do not add extra slash if there is already one in the given
path.

$ btrfs filesystem du ./test/
output:
Total   Exclusive  Set shared  Filename
0.00B   0.00B   -  /home/gujx/device/tmp/test//foo

Signed-off-by: Gu Jinxiang 
---
 cmds-fi-du.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-fi-du.c b/cmds-fi-du.c
index 8a44665c..74f57320 100644
--- a/cmds-fi-du.c
+++ b/cmds-fi-du.c
@@ -449,7 +449,7 @@ static int du_add_file(const char *filename, int dirfd,
}
 
pathtmp = pathp;
-   if (pathp == path)
+   if (pathp == path || *(pathp-1) == '/')
ret = sprintf(pathp, "%s", filename);
else
ret = sprintf(pathp, "/%s", filename);
-- 
2.14.3



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

Re: [PATCH v2 1/3] btrfs-progs: fi usage: change warning message more appropriately

[Nikolay Borisov]

On 29.03.2018 11:22, Misono Tomohiro wrote:
> "fi usage" shows the warning "RAID5/6 numbers will be incorrect" when
> running without root privilege even if raid5/6 is not used.  What
> happens is it cannot get the per device profile usage info, so change
> the message more appropriately.
> 
> Reviewed-by: Qu Wenruo 
> Signed-off-by: Tomohiro Misono 
> ---
>  cmds-fi-usage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
> index de7ad668..f2307fe0 100644
> --- a/cmds-fi-usage.c
> +++ b/cmds-fi-usage.c
> @@ -629,7 +629,7 @@ int load_chunk_and_device_info(int fd, struct chunk_info 
> **chunkinfo,
>   ret = load_chunk_info(fd, chunkinfo, chunkcount);
>   if (ret == -EPERM) {
>   warning(
> -"cannot read detailed chunk info, RAID5/6 numbers will be incorrect, run as 
> root");
> +"cannot read detailed chunk info. per device usage will not be shown, run as 
> root");
^^^
nit: New sentences must start with a capital letter :)
>   } else if (ret) {
>   return ret;
>   }
> 
--
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

[PATCH v2 2/3] btrfs-progs: fi usage: change to output more info without root privilege

[Misono Tomohiro]

Although per device usage cannot be shown without root privilege,
per profile usage can be shown.

To achieve this, we just basically need to remove the check of nullness
of chunkinfo in print_filesystem_usage_by_chunk(), because other
functions except print_unused() properly handles chunkinfo by
chunkcount, which is 0 if chunkinfo is null.

As a result, "fi usage" always includes the information of "fi df".

Reviewed-by: Qu Wenruo 
Signed-off-by: Tomohiro Misono 
---
 cmds-fi-usage.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index f2307fe0..2d45b3bb 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -923,9 +923,11 @@ static void _cmd_filesystem_usage_linear(unsigned 
unit_mode,
printf("\n");
}
 
-   printf("Unallocated:\n");
-   print_unused(info_ptr, info_count, device_info_ptr, device_info_count,
-   unit_mode | UNITS_NEGATIVE);
+   if (info_count) {
+   printf("Unallocated:\n");
+   print_unused(info_ptr, info_count, device_info_ptr,
+   device_info_count, unit_mode | UNITS_NEGATIVE);
+   }
 }
 
 static int print_filesystem_usage_by_chunk(int fd,
@@ -936,9 +938,6 @@ static int print_filesystem_usage_by_chunk(int fd,
struct btrfs_ioctl_space_args *sargs;
int ret = 0;
 
-   if (!chunkinfo)
-   return 0;
-
sargs = load_space_info(fd, path);
if (!sargs) {
ret = 1;
-- 
2.14.3

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

[PATCH 1/5] libbtrfsutil: don't return free space cache inodes from deleted_subvolumes()

[Omar Sandoval]

From: Omar Sandoval 

Deleted free space cache inodes also get an orphan item in the root
tree, but we shouldn't report those as deleted subvolumes. Deleted
subvolumes will still have the root item, so we can just do an extra
tree search.

Reported-by: Tomohiro Misono 
Signed-off-by: Omar Sandoval 
---
 libbtrfsutil/subvolume.c | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index d9728281..d6c0ced8 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -1339,21 +1339,31 @@ PUBLIC enum btrfs_util_error 
btrfs_util_deleted_subvolumes_fd(int fd,
}
 
header = (struct btrfs_ioctl_search_header *)(search.buf + 
buf_off);
-   if (*n >= capacity) {
-   size_t new_capacity = capacity ? capacity * 2 : 1;
-   uint64_t *new_ids;
 
-   new_ids = reallocarray(*ids, new_capacity,
-  sizeof(**ids));
-   if (!new_ids)
-   return BTRFS_UTIL_ERROR_NO_MEMORY;
-
-   *ids = new_ids;
-   capacity = new_capacity;
+   /*
+* The orphan item might be for a free space cache inode, so
+* check if there's a matching root item.
+*/
+   err = btrfs_util_subvolume_info_fd(fd, header->offset, NULL);
+   if (!err) {
+   if (*n >= capacity) {
+   size_t new_capacity;
+   uint64_t *new_ids;
+
+   new_capacity = capacity ? capacity * 2 : 1;
+   new_ids = reallocarray(*ids, new_capacity,
+  sizeof(**ids));
+   if (!new_ids)
+   return BTRFS_UTIL_ERROR_NO_MEMORY;
+
+   *ids = new_ids;
+   capacity = new_capacity;
+   }
+   (*ids)[(*n)++] = header->offset;
+   } else if (err != BTRFS_UTIL_ERROR_SUBVOLUME_NOT_FOUND) {
+   goto out;
}
 
-   (*ids)[(*n)++] = header->offset;
-
items_pos++;
buf_off += sizeof(*header) + header->len;
search.key.min_offset = header->offset + 1;
-- 
2.16.3

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

[PATCH 2/5] libbtrfsutil: fix memory leak in deleted_subvolumes()

[Omar Sandoval]

From: Omar Sandoval 

If we fail to reallocate the ID array, we still need to free it.

Signed-off-by: Omar Sandoval 
---
 libbtrfsutil/subvolume.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index d6c0ced8..867b3e10 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -1353,8 +1353,10 @@ PUBLIC enum btrfs_util_error 
btrfs_util_deleted_subvolumes_fd(int fd,
new_capacity = capacity ? capacity * 2 : 1;
new_ids = reallocarray(*ids, new_capacity,
   sizeof(**ids));
-   if (!new_ids)
-   return BTRFS_UTIL_ERROR_NO_MEMORY;
+   if (!new_ids) {
+   err = BTRFS_UTIL_ERROR_NO_MEMORY;
+   goto out;
+   }
 
*ids = new_ids;
capacity = new_capacity;
-- 
2.16.3

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

[PATCH 4/5] libbtrfsutil: always build libbtrfsutil.so.$MAJOR

[Omar Sandoval]

From: Omar Sandoval 

Otherwise, make test-libbtrfsutil from a fresh checkout fails.

Signed-off-by: Omar Sandoval 
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 699f864d..fbd6677a 100644
--- a/Makefile
+++ b/Makefile
@@ -417,7 +417,7 @@ libbtrfsutil.so.$(libbtrfsutil_major) libbtrfsutil.so: 
libbtrfsutil.so.$(libbtrf
$(Q)$(LN_S) -f $< $@
 
 ifeq ($(PYTHON_BINDINGS),1)
-libbtrfsutil_python: libbtrfsutil.so libbtrfsutil/btrfsutil.h
+libbtrfsutil_python: libbtrfsutil.so.$(libbtrfsutil_major) libbtrfsutil.so 
libbtrfsutil/btrfsutil.h
@echo "[PY] libbtrfsutil"
$(Q)cd libbtrfsutil/python; \
CFLAGS= LDFLAGS= $(PYTHON) setup.py $(SETUP_PY_Q) build_ext -i 
build
-- 
2.16.3

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

[PATCH 0/5] libbtrfsutil: misc fixes

[Omar Sandoval]

From: Omar Sandoval 

Hi, Dave,

This is a handful of minor fixes for libbtrfsutil that would be good to
get in before the release. Patch 1 fixes an issue reported by Tomohiro a
few weeks back. Patch 2 fixes a memory leak I noticed while writing
patch 1. Patch 3 makes us use the local mkfs.btrfs so that the tests can
run on the CI system. Patch 4 fixes an issue I noticed while testing
that fix. Patch 5 fixes a test failure caused by "btrfs-progs: mkfs: add
uuid and otime to ROOT_ITEM of, FS_TREE".

Based on your devel branch. Please apply!

Omar Sandoval (5):
  libbtrfsutil: don't return free space cache inodes from
deleted_subvolumes()
  libbtrfsutil: fix memory leak in deleted_subvolumes()
  libbtrfsutil: use local mkfs.btrfs for tests if it exists
  libbtrfsutil: always build libbtrfsutil.so.$MAJOR
  libbtrfsutil: fix test assumptions about top-level subvolume

 Makefile|  4 ++--
 libbtrfsutil/python/tests/__init__.py   |  6 -
 libbtrfsutil/python/tests/test_subvolume.py |  8 ---
 libbtrfsutil/subvolume.c| 34 +++--
 4 files changed, 35 insertions(+), 17 deletions(-)

-- 
2.16.3

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

[PATCH 5/5] libbtrfsutil: fix test assumptions about top-level subvolume

[Omar Sandoval]

From: Omar Sandoval 

Since "btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of, FS_TREE",
the top-level subvolume has a non-zero UUID, ctime, and otime. Fix the
subvolume_info() test to not check for zero.

Signed-off-by: Omar Sandoval 
---
 libbtrfsutil/python/tests/test_subvolume.py | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libbtrfsutil/python/tests/test_subvolume.py 
b/libbtrfsutil/python/tests/test_subvolume.py
index a46d4a34..93396cba 100644
--- a/libbtrfsutil/python/tests/test_subvolume.py
+++ b/libbtrfsutil/python/tests/test_subvolume.py
@@ -95,7 +95,8 @@ class TestSubvolume(BtrfsTestCase):
 self.assertEqual(info.parent_id, 0)
 self.assertEqual(info.dir_id, 0)
 self.assertEqual(info.flags, 0)
-self.assertEqual(info.uuid, bytes(16))
+self.assertIsInstance(info.uuid, bytes)
+self.assertEqual(len(info.uuid), 16)
 self.assertEqual(info.parent_uuid, bytes(16))
 self.assertEqual(info.received_uuid, bytes(16))
 self.assertNotEqual(info.generation, 0)
@@ -103,8 +104,8 @@ class TestSubvolume(BtrfsTestCase):
 self.assertEqual(info.otransid, 0)
 self.assertEqual(info.stransid, 0)
 self.assertEqual(info.rtransid, 0)
-self.assertEqual(info.ctime, 0)
-self.assertEqual(info.otime, 0)
+self.assertIsInstance(info.ctime, float)
+self.assertIsInstance(info.otime, float)
 self.assertEqual(info.stime, 0)
 self.assertEqual(info.rtime, 0)
 
@@ -117,6 +118,7 @@ class TestSubvolume(BtrfsTestCase):
 self.assertEqual(info.dir_id, 256)
 self.assertEqual(info.flags, 0)
 self.assertIsInstance(info.uuid, bytes)
+self.assertEqual(len(info.uuid), 16)
 self.assertEqual(info.parent_uuid, bytes(16))
 self.assertEqual(info.received_uuid, bytes(16))
 self.assertNotEqual(info.generation, 0)
-- 
2.16.3

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