Re: [PATCH 09/13] btrfs-progs: Revert "btrfs-progs: Record orphan data extent ref to corresponding root."

2018-11-07 Thread Su Yanjun




On 10/24/2018 8:29 AM, Qu Wenruo wrote:


On 2018/10/23 下午5:41, Su Yue wrote:

From: Su Yanjun 

The reason for revert is that according to the existing situation, the
probability of problem in the extent tree is higher than in the fs Tree.
So this feature should be removed.


The same problem as previous patch.

We need an example on how such repair could lead to further corruption.

Thanks,
Qu


Firstly In record_orphan_data_extents() function:

key.objectid = dback->owner;
key.type = BTRFS_EXTENT_DATA_KEY;
key.offset = dback->offset;//
ret = btrfs_search_slot(NULL, dest_root, , , 0, 0);

'dback->offset' is wrong use here.

Secondly when disk bytenr in file extent item is wrong, the file extent data is 
always inserted to fs tree which will lead to fs check failed.

Thanks,
Su


Signed-off-by: Su Yanjun 
---
  check/main.c  | 120 +-
  check/mode-original.h |  17 --
  ctree.h   |  10 
  disk-io.c |   1 -
  4 files changed, 1 insertion(+), 147 deletions(-)

diff --git a/check/main.c b/check/main.c
index 268de5dd5f26..bd1f322e0f12 100644
--- a/check/main.c
+++ b/check/main.c
@@ -511,22 +511,6 @@ cleanup:
return ERR_PTR(ret);
  }
  
-static void print_orphan_data_extents(struct list_head *orphan_extents,

- u64 objectid)
-{
-   struct orphan_data_extent *orphan;
-
-   if (list_empty(orphan_extents))
-   return;
-   printf("The following data extent is lost in tree %llu:\n",
-  objectid);
-   list_for_each_entry(orphan, orphan_extents, list) {
-   printf("\tinode: %llu, offset:%llu, disk_bytenr: %llu, disk_len: 
%llu\n",
-  orphan->objectid, orphan->offset, orphan->disk_bytenr,
-  orphan->disk_len);
-   }
-}
-
  static void print_inode_error(struct btrfs_root *root, struct inode_record 
*rec)
  {
u64 root_objectid = root->root_key.objectid;
@@ -578,9 +562,6 @@ static void print_inode_error(struct btrfs_root *root, 
struct inode_record *rec)
if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
fprintf(stderr, ", invalid inline ram bytes");
fprintf(stderr, "\n");
-   /* Print the orphan extents if needed */
-   if (errors & I_ERR_FILE_EXTENT_ORPHAN)
-   print_orphan_data_extents(>orphan_extents, root->objectid);
  
  	/* Print the holes if needed */

if (errors & I_ERR_FILE_EXTENT_DISCOUNT) {
@@ -683,18 +664,6 @@ static struct inode_record *get_inode_rec(struct 
cache_tree *inode_cache,
return rec;
  }
  
-static void free_orphan_data_extents(struct list_head *orphan_extents)

-{
-   struct orphan_data_extent *orphan;
-
-   while (!list_empty(orphan_extents)) {
-   orphan = list_entry(orphan_extents->next,
-   struct orphan_data_extent, list);
-   list_del(>list);
-   free(orphan);
-   }
-}
-
  static void free_inode_rec(struct inode_record *rec)
  {
struct inode_backref *backref;
@@ -707,7 +676,6 @@ static void free_inode_rec(struct inode_record *rec)
list_del(>list);
free(backref);
}
-   free_orphan_data_extents(>orphan_extents);
free_file_extent_holes(>holes);
free(rec);
  }
@@ -3286,7 +3254,6 @@ skip_walking:
  
  	free_corrupt_blocks_tree(_blocks);

root->fs_info->corrupt_blocks = NULL;
-   free_orphan_data_extents(>orphan_data_extents);
return ret;
  }
  
@@ -7137,88 +7104,6 @@ static int find_possible_backrefs(struct btrfs_fs_info *info,

return 0;
  }
  
-/*

- * Record orphan data ref into corresponding root.
- *
- * Return 0 if the extent item contains data ref and recorded.
- * Return 1 if the extent item contains no useful data ref
- *   On that case, it may contains only shared_dataref or metadata backref
- *   or the file extent exists(this should be handled by the extent bytenr
- *   recovery routine)
- * Return <0 if something goes wrong.
- */
-static int record_orphan_data_extents(struct btrfs_fs_info *fs_info,
- struct extent_record *rec)
-{
-   struct btrfs_key key;
-   struct btrfs_root *dest_root;
-   struct extent_backref *back, *tmp;
-   struct data_backref *dback;
-   struct orphan_data_extent *orphan;
-   struct btrfs_path path;
-   int recorded_data_ref = 0;
-   int ret = 0;
-
-   if (rec->metadata)
-   return 1;
-   btrfs_init_path();
-   rbtree_postorder_for_each_entry_safe(back, tmp,
->backref_tree, node) {
-   if (back->full_backref || !back->is_data ||
-   !back->found_extent_tree)
-   con

Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

2018-11-06 Thread Su Yanjun




On 11/7/2018 2:38 PM, Qu Wenruo wrote:


On 2018/11/7 下午2:21, Su Yanjun  wrote:


On 10/24/2018 8:45 AM, Qu Wenruo wrote:

On 2018/10/23 下午5:41, Su Yue wrote:

From: Su Yanjun 

In original mode, if some file extent item has unaligned extent backref,
fixup_extent_refs can't repair it. This patch will check extent
alignment
then delete file extent with unaligned extent backref.

This looks a little strange to me.

You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?

Then why not just delete the EXTENT_ITEM directly? No need to go back
checking if it has a corresponding EXTENT_DATA since unaligned one is
definitely corrupted.

For corrupted EXTENT_DATA, it should get deleted when we check fs tree.

This would save you a lot of codes.

Thanks,
Qu

The situation is that the file extent has wrong extent backref, actually
it doesn't exist.

Did you mean extent EXTENT_ITEM key's objectid is unaligned?

Would you please give an example on this case? Like:
( EXTENT_DATA 
disk bytenr  disk len 

And its backref like:
( EXTENT_ITEM )

And then mark where the number is incorrect.

Thanks,
Qu


As in /btrfs-progs/tests/fsck-tests/001-bad-file-extent-bytenr case:

item 7 key (257 EXTENT_DATA 0) itemoff 3453 itemsize 53

    generation 6 type 1 (regular)

    extent data disk byte 755944791 nr 1048576
    ^

    extent data offset 0 nr 1048576 ram 1048576

    extent compression 0 (none)

Thanks,

Su


Thanks,
Su













Re: [PATCH 10/13] btrfs-progs: check: fix bug in find_possible_backrefs

2018-11-06 Thread Su Yanjun




On 10/24/2018 8:34 AM, Qu Wenruo wrote:


On 2018/10/23 下午5:41, Su Yue wrote:

From: Su Yanjun 

It may cost more time to search all extent data of correspond files but
should not influence total speed too much cause that only corrupted
extent items are participated in.

Sorry, I didn't really get the point of the modification from the commit
message.

Would you please explain the purpose of this patch first?

Thanks,
Qu


In find_possible_backrefs() function:

key.objectid = dback->owner;
key.type = BTRFS_EXTENT_DATA_KEY;
key.offset = dback->offset;//
ret = btrfs_search_slot(NULL, root, , path, 0, 0);

'dback->offset' is not the offset in file range, we'll find wrong file extent.

Thanks,

Su


Signed-off-by: Su Yanjun 
---
  check/main.c | 110 ++-
  1 file changed, 92 insertions(+), 18 deletions(-)

diff --git a/check/main.c b/check/main.c
index bd1f322e0f12..90d9fd570287 100644
--- a/check/main.c
+++ b/check/main.c
@@ -7015,6 +7015,89 @@ out:
return ret ? ret : nr_del;
  }
  
+/*

+ * Based extent backref item, we find all file extent items in the fs tree. By
+ * the info we can rebuild the extent backref item
+ */
+static int __find_possible_backrefs(struct btrfs_root *root,
+   u64 owner, u64 offset, u64 bytenr, u64 *refs_ret,
+   u64 *bytes_ret)
+{
+   int ret = 0;
+   struct btrfs_path path;
+   struct btrfs_key key;
+   struct btrfs_key found_key;
+   struct btrfs_file_extent_item *fi;
+   struct extent_buffer *leaf;
+   u64 backref_offset, disk_bytenr;
+   int slot;
+
+   btrfs_init_path();
+
+   key.objectid = owner;
+   key.type = BTRFS_INODE_ITEM_KEY;
+   key.offset = 0;
+
+   ret = btrfs_search_slot(NULL, root, , , 0, 0);
+   if (ret > 0)
+   ret = -ENOENT;
+   if (ret) {
+   btrfs_release_path();
+   return ret;
+   }
+
+   btrfs_release_path();
+
+   key.objectid = owner;
+   key.type = BTRFS_EXTENT_DATA_KEY;
+   key.offset = 0;
+
+   ret = btrfs_search_slot(NULL, root, , , 0, 0);
+   if (ret < 0) {
+   btrfs_release_path();
+   return ret;
+   }
+
+   while (1) {
+   leaf = path.nodes[0];
+   slot = path.slots[0];
+
+   if (slot >= btrfs_header_nritems(leaf)) {
+   ret = btrfs_next_leaf(root, );
+   if (ret) {
+   if (ret > 0)
+   ret = 0;
+   break;
+   }
+
+   leaf = path.nodes[0];
+   slot = path.slots[0];
+   }
+
+   btrfs_item_key_to_cpu(leaf, _key, slot);
+   if ((found_key.objectid != owner) ||
+   (found_key.type != BTRFS_EXTENT_DATA_KEY))
+   break;
+
+   fi = btrfs_item_ptr(leaf, slot,
+   struct btrfs_file_extent_item);
+
+   backref_offset = found_key.offset -
+   btrfs_file_extent_offset(leaf, fi);
+   disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi);
+   *bytes_ret = btrfs_file_extent_disk_num_bytes(leaf,
+   fi);
+   if ((disk_bytenr == bytenr) &&
+   (backref_offset == offset)) {
+   (*refs_ret)++;
+   }
+   path.slots[0]++;
+   }
+
+   btrfs_release_path();
+   return ret;
+}
+
  static int find_possible_backrefs(struct btrfs_fs_info *info,
  struct btrfs_path *path,
  struct cache_tree *extent_cache,
@@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct btrfs_fs_info 
*info,
struct extent_backref *back, *tmp;
struct data_backref *dback;
struct cache_extent *cache;
-   struct btrfs_file_extent_item *fi;
struct btrfs_key key;
u64 bytenr, bytes;
+   u64 refs;
int ret;
  
  	rbtree_postorder_for_each_entry_safe(back, tmp,

@@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct btrfs_fs_info 
*info,
if (IS_ERR(root))
return PTR_ERR(root);
  
-		key.objectid = dback->owner;

-   key.type = BTRFS_EXTENT_DATA_KEY;
-   key.offset = dback->offset;
-   ret = btrfs_search_slot(NULL, root, , path, 0, 0);
-   if (ret) {
-   btrfs_release_path(path);
-   if (ret < 0)
-   return ret;
-   /* Didn't find it, we can carry on */
-   ret = 0;
+   refs = 0;
+   bytes = 0;
+   ret 

Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref

2018-11-06 Thread Su Yanjun




On 10/24/2018 8:45 AM, Qu Wenruo wrote:


On 2018/10/23 下午5:41, Su Yue wrote:

From: Su Yanjun 

In original mode, if some file extent item has unaligned extent backref,
fixup_extent_refs can't repair it. This patch will check extent alignment
then delete file extent with unaligned extent backref.

This looks a little strange to me.

You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?

Then why not just delete the EXTENT_ITEM directly? No need to go back
checking if it has a corresponding EXTENT_DATA since unaligned one is
definitely corrupted.

For corrupted EXTENT_DATA, it should get deleted when we check fs tree.

This would save you a lot of codes.

Thanks,
Qu
The situation is that the file extent has wrong extent backref, actually 
it doesn't exist.


Thanks,
Su







[PATCH] btrfs-progs: fix gcc8 default build warning caused by '-Wformat-truncation'

2018-10-25 Thread Su Yanjun
When using gcc8 compiles utils.c, it complains as below:

utils.c:852:45: warning: '%s' directive output may be truncated writing
up to 4095 bytes into a region of size 4084 [-Wformat-truncation=]
   snprintf(path, sizeof(path), "/dev/mapper/%s", name);
 ^~   
In file included from /usr/include/stdio.h:873,
 from utils.c:20:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk'
output between 13 and 4108 bytes into a destination of size 4096
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
  ^~~~
__bos (__s), __fmt, __va_arg_pack ());
~

This isn't a type of warning we care about, particularly when PATH_MAX
is much less than either.

Using the GCC option -Wno-format-truncation to disable this for default
build.

Signed-off-by: Su Yanjun 
---
 Makefile.extrawarn | 1 +
 configure.ac   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.extrawarn b/Makefile.extrawarn
index 1f4bda94a167..ed76fb5b5554 100644
--- a/Makefile.extrawarn
+++ b/Makefile.extrawarn
@@ -47,6 +47,7 @@ warning-1 += -Wold-style-definition
 warning-1 += $(call cc-option, -Wmissing-include-dirs)
 warning-1 += $(call cc-option, -Wunused-but-set-variable)
 warning-1 += $(call cc-disable-warning, missing-field-initializers)
+warning-1 += $(call cc-option, -Wformat-truncation)
 
 warning-2 := -Waggregate-return
 warning-2 += -Wcast-align
diff --git a/configure.ac b/configure.ac
index df02f20655d9..c626beca8b77 100644
--- a/configure.ac
+++ b/configure.ac
@@ -12,7 +12,7 @@ LIBBTRFS_MAJOR=0
 LIBBTRFS_MINOR=1
 LIBBTRFS_PATCHLEVEL=2
 
-CFLAGS=${CFLAGS:-"-g -O1 -Wall -D_FORTIFY_SOURCE=2"}
+CFLAGS=${CFLAGS:-"-g -O1 -Wall -D_FORTIFY_SOURCE=2 -Wno-format-truncation"}
 AC_SUBST([CFLAGS])
 
 AC_PREREQ([2.60])
-- 
2.19.1





[PATCH] btrfs-progs: fix compile warning when using gcc8 to compile btrfs-progs

2018-10-12 Thread Su Yanjun
When using gcc8 to compile btrfs-progs, it complains as below:

ctree.c: In function 'btrfs_search_slot_for_read':
ctree.c:1249:45: warning: passing argument 3 of 'btrfs_search_slot'
discards 'const' qualifier from pointer target type
[-Wdiscarded-qualifiers]
 ret = btrfs_search_slot(NULL, root, key, p, 0, 0);

Change btrfs_search_slot prototype with 'const' qualifier for argument 3.
Also fix similar problems as above change.

Signed-off-by: Su Yanjun 
---
 ctree.c | 19 ++-
 ctree.h | 10 +-
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/ctree.c b/ctree.c
index aa1568620205..c8dd73cf2ce2 100644
--- a/ctree.c
+++ b/ctree.c
@@ -27,7 +27,7 @@
 static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root
  *root, struct btrfs_path *path, int level);
 static int split_leaf(struct btrfs_trans_handle *trans, struct btrfs_root
- *root, struct btrfs_key *ins_key,
+ *root, const struct btrfs_key *ins_key,
  struct btrfs_path *path, int data_size, int extend);
 static int push_node_left(struct btrfs_trans_handle *trans,
  struct btrfs_root *root, struct extent_buffer *dst,
@@ -389,7 +389,7 @@ int btrfs_cow_block(struct btrfs_trans_handle *trans,
return ret;
 }
 
-int btrfs_comp_cpu_keys(struct btrfs_key *k1, struct btrfs_key *k2)
+int btrfs_comp_cpu_keys(const struct btrfs_key *k1, const struct btrfs_key *k2)
 {
if (k1->objectid > k2->objectid)
return 1;
@@ -409,7 +409,8 @@ int btrfs_comp_cpu_keys(struct btrfs_key *k1, struct 
btrfs_key *k2)
 /*
  * compare two keys in a memcmp fashion
  */
-static int btrfs_comp_keys(struct btrfs_disk_key *disk, struct btrfs_key *k2)
+static int btrfs_comp_keys(struct btrfs_disk_key *disk,
+   const struct btrfs_key *k2)
 {
struct btrfs_key k1;
 
@@ -602,7 +603,7 @@ static int noinline check_block(struct btrfs_root *root,
  * slot may point to max if the key is bigger than all of the keys
  */
 static int generic_bin_search(struct extent_buffer *eb, unsigned long p,
- int item_size, struct btrfs_key *key,
+ int item_size, const struct btrfs_key *key,
  int max, int *slot)
 {
int low = 0;
@@ -636,7 +637,7 @@ static int generic_bin_search(struct extent_buffer *eb, 
unsigned long p,
  * simple bin_search frontend that does the right thing for
  * leaves vs nodes
  */
-static int bin_search(struct extent_buffer *eb, struct btrfs_key *key,
+static int bin_search(struct extent_buffer *eb, const struct btrfs_key *key,
  int level, int *slot)
 {
if (level == 0)
@@ -1129,9 +1130,9 @@ out:
  * tree.  if ins_len < 0, nodes will be merged as we walk down the tree (if
  * possible)
  */
-int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root
- *root, struct btrfs_key *key, struct btrfs_path *p, int
- ins_len, int cow)
+int btrfs_search_slot(struct btrfs_trans_handle *trans,
+   struct btrfs_root *root, const struct btrfs_key *key,
+   struct btrfs_path *p, int ins_len, int cow)
 {
struct extent_buffer *b;
int slot;
@@ -2150,7 +2151,7 @@ static noinline int copy_for_split(struct 
btrfs_trans_handle *trans,
  */
 static noinline int split_leaf(struct btrfs_trans_handle *trans,
   struct btrfs_root *root,
-  struct btrfs_key *ins_key,
+  const struct btrfs_key *ins_key,
   struct btrfs_path *path, int data_size,
   int extend)
 {
diff --git a/ctree.h b/ctree.h
index 2a2437070ef9..cf0efae9c185 100644
--- a/ctree.h
+++ b/ctree.h
@@ -1973,7 +1973,7 @@ static inline void btrfs_disk_key_to_cpu(struct btrfs_key 
*cpu,
 }
 
 static inline void btrfs_cpu_key_to_disk(struct btrfs_disk_key *disk,
-struct btrfs_key *cpu)
+const struct btrfs_key *cpu)
 {
disk->offset = cpu_to_le64(cpu->offset);
disk->type = cpu->type;
@@ -2552,7 +2552,7 @@ u64 add_new_free_space(struct btrfs_block_group_cache 
*block_group,
 u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset);
 
 /* ctree.c */
-int btrfs_comp_cpu_keys(struct btrfs_key *k1, struct btrfs_key *k2);
+int btrfs_comp_cpu_keys(const struct btrfs_key *k1, const struct btrfs_key 
*k2);
 int btrfs_del_ptr(struct btrfs_root *root, struct btrfs_path *path,
int level, int slot);
 enum btrfs_tree_block_status
@@ -2595,9 +2595,9 @@ int btrfs_split_item(struct btrfs_trans_handle *trans,
 struct btrfs_path *path,
 struct btrfs_key *new_key,
 unsigned long split_offset);
-int 

[PATCH] btrfs-progs: fix compile warning when using gcc8 to compile btrfs-progs

2018-10-11 Thread Su Yanjun
When using gcc8 compiles utils.c, it complains as below:

utils.c:852:45: warning: '%s' directive output may be truncated writing
up to 4095 bytes into a region of size 4084 [-Wformat-truncation=]
   snprintf(path, sizeof(path), "/dev/mapper/%s", name);
 ^~   
In file included from /usr/include/stdio.h:873,
 from utils.c:20:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk'
output between 13 and 4108 bytes into a destination of size 4096
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
  ^~~~
__bos (__s), __fmt, __va_arg_pack ());
~

This isn't a type of warning we care about, particularly when PATH_MAX
is much less than either.

Using the GCC option -Wno-format-truncation to disable this.

Signed-off-by: Su Yanjun 
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index df02f20655d9..c626beca8b77 100644
--- a/configure.ac
+++ b/configure.ac
@@ -12,7 +12,7 @@ LIBBTRFS_MAJOR=0
 LIBBTRFS_MINOR=1
 LIBBTRFS_PATCHLEVEL=2
 
-CFLAGS=${CFLAGS:-"-g -O1 -Wall -D_FORTIFY_SOURCE=2"}
+CFLAGS=${CFLAGS:-"-g -O1 -Wall -D_FORTIFY_SOURCE=2 -Wno-format-truncation"}
 AC_SUBST([CFLAGS])
 
 AC_PREREQ([2.60])
-- 
2.19.1





[PATCH] btrfs-progs: tests: Add the testcase for subvolume name length limit test

2018-09-18 Thread Su Yanjun
Total of three conditions are tested. One for short name, one with
name length 255, the last one with more than 255.

This case should pass after commit
'btrfs-progs: change filename limit to 255 when creating subvolume'.

Signed-off-by: Su Yanjun 
---
 .../033-filename-length-limit/test.sh | 86 +++
 1 file changed, 86 insertions(+)
 create mode 100755 tests/misc-tests/033-filename-length-limit/test.sh

diff --git a/tests/misc-tests/033-filename-length-limit/test.sh 
b/tests/misc-tests/033-filename-length-limit/test.sh
new file mode 100755
index ..7764ad9b584c
--- /dev/null
+++ b/tests/misc-tests/033-filename-length-limit/test.sh
@@ -0,0 +1,86 @@
+#!/bin/bash
+#
+# test file name length limit settings
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+
+prepare_test_dev
+run_check "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+run_check_mount_test_dev
+run_check $SUDO_HELPER chmod a+rw "$TEST_MNT"
+
+cd "$TEST_MNT"
+
+longname=\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+0123456789\
+\
+01234
+
+#
+# subvolume name length limit test
+#
+
+# short name test
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create aaa
+# 255
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$longname"
+# 256, must fail
+run_mustfail "name 256 bytes long succeeded" \
+   $SUDO_HELPER "$TOP/btrfs" subvolume create "$longname"5
+# 255*2, must fail
+run_mustfail "name 2 * 255 bytes long succeeded" \
+   $SUDO_HELPER "$TOP/btrfs" subvolume create "$longname$longname"
+
+#
+# snapshot name length limit test
+#
+
+run_check $SUDO_HELPER mkdir snaps
+
+# short name test
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot aaa snaps/bbb
+# 255
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot aaa snaps/"$longname"
+# 256, must fail
+run_mustfail "name 256 bytes long succeeded" \
+   $SUDO_HELPER "$TOP/btrfs" subvolume snapshot aaa snaps/"$longname"5
+# 255*2, must fail
+run_mustfail "name 2 * 255 bytes long succeeded" \
+   $SUDO_HELPER "$TOP/btrfs" subvolume snapshot aaa 
snaps/"$longname$longname"
+
+cd ..
+
+run_check_umount_test_dev
-- 
2.18.0





[PATCH v2] btrfs-progs: change filename limit to 255 when creating subvolume

2018-09-17 Thread Su Yanjun
Modify the file name length limit to meet the Linux naming convention.
In addition, the file name length is always bigger than 0, no need to
compare with 0 again.

Changelog:
v2:
 Fix the same problem in creating snapshot routine.

Issue: #145
Signed-off-by: Su Yanjun 
---

v2: Also fix the same problem in creating snapshot routine.

 cmds-subvolume.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e7a884af1f5d..5a446c1ae2b4 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -155,7 +155,7 @@ static int cmd_subvol_create(int argc, char **argv)
}
 
len = strlen(newname);
-   if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
+   if (len > BTRFS_VOL_NAME_MAX) {
error("subvolume name too long: %s", newname);
goto out;
}
@@ -715,7 +715,7 @@ static int cmd_subvol_snapshot(int argc, char **argv)
}
 
len = strlen(newname);
-   if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
+   if (len > BTRFS_VOL_NAME_MAX) {
error("snapshot name too long '%s'", newname);
goto out;
}
-- 
2.18.0





Re: [PATCH] btrfs-progs: change filename limit to 255 when creating subvolume

2018-09-16 Thread Su Yanjun



On 9/14/2018 10:34 PM, David Sterba wrote:

On Wed, Sep 12, 2018 at 03:39:03PM +0800, Su Yanjun wrote:

Modify the file name length limit to meet the Linux naming convention.
In addition, the file name length is always bigger than 0, no need to
compare with 0 again.

Issue: #145
Signed-off-by: Su Yanjun 

Looks good, please send a test, thanks. You can copy portions of the
misc-tests/014-filesystem-label that does similar string length check
for the label.


The following is the tests for the patch.

[suyj@sarch tests]$ vim misc-tests-results.txt
=== START TEST 
/home/suyj/btrfs-progs/tests//misc-tests/033-filename-length-limit
$TEST_DEV not given, using /home/suyj/btrfs-progs/tests//test.img as 
fallback
== RUN CHECK /home/suyj/btrfs-progs/mkfs.btrfs -L BTRFS-TEST-LABEL 
-f /home/suyj/btrfs-progs/tests//test.img

btrfs-progs v4.17.1
See http://btrfs.wiki.kernel.org for more information.

Label:  BTRFS-TEST-LABEL
UUID:   925425a2-5557-4dea-93d3-3b4543707082
Node size:  16384
Sector size:    4096
Filesystem size:    2.00GiB
Block group profiles:
  Data: single    8.00MiB
  Metadata: DUP 102.38MiB
  System:   DUP   8.00MiB
SSD detected:   no
Incompat features:  extref, skinny-metadata
Number of devices:  1
Devices:
   ID    SIZE  PATH
    1 2.00GiB  /home/suyj/btrfs-progs/tests//test.img

== RUN CHECK root_helper mount -t btrfs -o loop 
/home/suyj/btrfs-progs/tests//test.img /home/suyj/btrfs-progs/tests//mnt

== RUN CHECK root_helper chmod a+rw /home/suyj/btrfs-progs/tests//mnt
== RUN CHECK root_helper /home/suyj/btrfs-progs/btrfs subvolume 
create aaa

Create subvolume './aaa'
== RUN CHECK root_helper /home/suyj/btrfs-progs/btrfs subvolume 
create 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234
Create subvolume 
'./012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234'
== RUN MUSTFAIL root_helper /home/suyj/btrfs-progs/btrfs subvolume 
create 
0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345
ERROR: cannot access 
0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345: 
File name too long
failed (expected): root_helper /home/suyj/btrfs-progs/btrfs subvolume 
create 
0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345
== RUN MUSTFAIL root_helper /home/suyj/btrfs-progs/btrfs subvolume 
create 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234
ERROR: cannot access 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234: 
File name too long
failed (expected): root_helper /home/suyj/btrfs-progs/btrfs subvolume 
create 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234

[PATCH] btrfs-progs: change filename limit to 255 when creating subvolume

2018-09-12 Thread Su Yanjun
Modify the file name length limit to meet the Linux naming convention. 
In addition, the file name length is always bigger than 0, no need to 
compare with 0 again.

Issue: #145
Signed-off-by: Su Yanjun 
---
 cmds-subvolume.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e7a884af..fe97fca3 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -715,7 +715,7 @@ static int cmd_subvol_snapshot(int argc, char **argv)
}
 
len = strlen(newname);
-   if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
+   if (len > BTRFS_VOL_NAME_MAX) {
error("snapshot name too long '%s'", newname);
goto out;
}
-- 
2.18.0