[PATCH 0/3] Fix "filesystem" command when fs is on file image

[PATCH 0/3] Fix "filesystem" command when fs is on file image

[Nikolay Borisov]

This series fixes the following use case: 

truncate -s 3g btrfs.img
mkfs.btrfs btrfs.img
btrrfs fi show .img
ERROR: not a valid btrfs filesystem: /root/btrfs.img


As evident this currently produces an error due to libblkid not recognising the 
image file as a filesystem. This stems from the fact progs doesn't force the 
addition of the image file to libblkid's cache. This series rectifies this. 

Patch 1 extends btrfs_scan_devices to take an optional path argument which will 
be added to libblkid's cache. 

Patch 2 Makes 'btrfs filesystem' Utilizes this btrfs_scan_devices' new
interface if it detects we want to query a filesystem placed on an image file. 

Patch 3 Adds simple test case to ensure this works as expected and is not 
broken in the future. 

Nikolay Borisov (3):
  btrfs-progs: Make btrfs_scan_devices take a path
  btrfs-progs: Correctly identify fs on image files in "filesystem"
    subcommands
  btrfs-progs: tests: Test fs on image files is correctly recognised

 cmds-device.c                                   |  2 +-
 cmds-filesystem.c                               |  3 +--
 disk-io.c                                       |  2 +-
 tests/cli-tests/010-fi-show-on-new-file/test.sh | 16 ++++++++++++++++
 utils.c                                         | 18 +++++++++++++++---
 utils.h                                         |  2 +-
 6 files changed, 35 insertions(+), 8 deletions(-)
 create mode 100755 tests/cli-tests/010-fi-show-on-new-file/test.sh

-- 
2.7.4

[PATCH 1/3] btrfs-progs: Make btrfs_scan_devices take a path

[Nikolay Borisov]

Currently btrfs_scan_devices will only scan devices under /dev (and
some other places that libblkid understands). This may cause a btrfs
filesystem housed on a file to be missed when issuing certain commands.
This patch allow the caller to explicitly pass a path that should be
probed for a btrfs filesystem and added to libblkid's cache. This is a
prep patch and currently brings no functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 cmds-device.c     |  2 +-
 cmds-filesystem.c |  2 +-
 disk-io.c         |  2 +-
 utils.c           | 18 +++++++++++++++---
 utils.h           |  2 +-
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index d3102ba7c1df..c65de8957ed6 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -294,7 +294,7 @@ static int cmd_device_scan(int argc, char **argv)
 
 	if (all || argc - optind == 0) {
 		printf("Scanning for Btrfs filesystems\n");
-		ret = btrfs_scan_devices();
+		ret = btrfs_scan_devices(NULL);
 		error_on(ret, "error %d while scanning", ret);
 		ret = btrfs_register_all_devices();
 		error_on(ret, "there are %d errors while registering devices", ret);
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index b8beec13f0e5..4657deb20fde 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -771,7 +771,7 @@ static int cmd_filesystem_show(int argc, char **argv)
 		goto out;
 
 devs_only:
-	ret = btrfs_scan_devices();
+	ret = btrfs_scan_devices(NULL);
 
 	if (ret) {
 		error("blkid device scan returned %d", ret);
diff --git a/disk-io.c b/disk-io.c
index 797b9b79ea3c..0b9f3723d00a 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1056,7 +1056,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
 	}
 
 	if (!skip_devices && total_devs != 1) {
-		ret = btrfs_scan_devices();
+		ret = btrfs_scan_devices(NULL);
 		if (ret)
 			return ret;
 	}
diff --git a/utils.c b/utils.c
index 9e26c884cc6c..d4f590049cf3 100644
--- a/utils.c
+++ b/utils.c
@@ -928,7 +928,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
 
 	/* scan other devices */
 	if (is_btrfs && total_devs > 1) {
-		ret = btrfs_scan_devices();
+		ret = btrfs_scan_devices(NULL);
 		if (ret)
 			return ret;
 	}
@@ -1979,8 +1979,14 @@ int group_profile_max_safe_loss(u64 flags)
 		return -1;
 	}
 }
-
-int btrfs_scan_devices(void)
+/**
+ * btrfs_scan_devices - scan all devices known to libblkid for the presence of
+ *			btrfs filesystems
+ *
+ * @device - if given, points to either a block device or regular file that
+ * should explicitly be added to libblkid's cache. Can be NULL
+ */
+int btrfs_scan_devices(char *device)
 {
 	int fd = -1;
 	int ret;
@@ -1998,7 +2004,13 @@ int btrfs_scan_devices(void)
 		error("blkid cache get failed");
 		return 1;
 	}
+
 	blkid_probe_all(cache);
+
+	/* Add passed device to libblkid's cache if needed */
+	if (device)
+		blkid_get_dev(cache, device, BLKID_DEV_NORMAL);
+
 	iter = blkid_dev_iterate_begin(cache);
 	blkid_dev_set_search(iter, "TYPE", "btrfs");
 	while (blkid_dev_next(iter, &dev) == 0) {
diff --git a/utils.h b/utils.h
index 7c5eb798557d..4df63a5b0260 100644
--- a/utils.h
+++ b/utils.h
@@ -140,7 +140,7 @@ int csum_tree_block(struct btrfs_fs_info *root, struct extent_buffer *buf,
 		    int verify);
 int ask_user(const char *question);
 int lookup_path_rootid(int fd, u64 *rootid);
-int btrfs_scan_devices(void);
+int btrfs_scan_devices(char *device);
 int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
 int find_mount_root(const char *path, char **mount_root);
 int get_device_info(int fd, u64 devid,
-- 
2.7.4

[PATCH 2/3] btrfs-progs: Correctly identify fs on image files in "filesystem" subcommands

[Nikolay Borisov]

Currently if a new filesystem is created and interrogated with we get
an error:

    truncate -s 3g btrfs.img
    mkfs.btrfs btrfs.img
    btrfs fi show btrfs.img
    ERROR: not a valid btrfs filesystem: /root/btrfs.img

The reason is that the image is not automatically recognised by libblkid
meaning btrfs_scan_devices will not "see" the device, resulting in the
aofrementioned error.

Fix it by detecting when this condition occurs and pass the path to the
file to btrfs_scan_devices, which will correctly add it to libblkid's
cache.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 cmds-filesystem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 4657deb20fde..0327563154d5 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -771,8 +771,7 @@ static int cmd_filesystem_show(int argc, char **argv)
 		goto out;
 
 devs_only:
-	ret = btrfs_scan_devices(NULL);
-
+	ret = btrfs_scan_devices(type == BTRFS_ARG_REG ? search : NULL);
 	if (ret) {
 		error("blkid device scan returned %d", ret);
 		return 1;
-- 
2.7.4

[PATCH 3/3] btrfs-progs: tests: Test fs on image files is correctly recognised

[Nikolay Borisov]

This ensures that 'btrfs filesystem show' can correctly identify a
filesystem on a newly created local file.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 tests/cli-tests/010-fi-show-on-new-file/test.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100755 tests/cli-tests/010-fi-show-on-new-file/test.sh

diff --git a/tests/cli-tests/010-fi-show-on-new-file/test.sh b/tests/cli-tests/010-fi-show-on-new-file/test.sh
new file mode 100755
index 000000000000..668f0f9b2cfe
--- /dev/null
+++ b/tests/cli-tests/010-fi-show-on-new-file/test.sh
@@ -0,0 +1,16 @@
+#!/bin/bash
+# test for 'filesystem show' on fresh local file
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+IMAGE=$(mktemp -u btrfs-XXXXXX.img)
+
+truncate -s3g "$IMAGE"
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$IMAGE"
+run_check $SUDO_HELPER "$TOP/btrfs" filesystem show "$IMAGE"
+
+rm -f "$IMAGE"
+
-- 
2.7.4

Re: [PATCH 0/3] Fix "filesystem" command when fs is on file image

[Qu Wenruo]

On 2019/5/16 下午4:42, Nikolay Borisov wrote:
> This series fixes the following use case:
>
> truncate -s 3g btrfs.img
> mkfs.btrfs btrfs.img
> btrrfs fi show .img
> ERROR: not a valid btrfs filesystem: /root/btrfs.img
>
>
> As evident this currently produces an error due to libblkid not recognising the
> image file as a filesystem. This stems from the fact progs doesn't force the
> addition of the image file to libblkid's cache. This series rectifies this.

I'm wondering if we could just fall back to open_ctree() and print
needed info using btrfs_fs_info.

open_ctree() will scan the passed device no matter if it's a block
device or a file or whatever, and get the correct result of fsid anyway.
No need to bother blkid cache or whatever.

Thanks,
Qu

>
> Patch 1 extends btrfs_scan_devices to take an optional path argument which will
> be added to libblkid's cache.
>
> Patch 2 Makes 'btrfs filesystem' Utilizes this btrfs_scan_devices' new
> interface if it detects we want to query a filesystem placed on an image file.
>
> Patch 3 Adds simple test case to ensure this works as expected and is not
> broken in the future.
>
> Nikolay Borisov (3):
>   btrfs-progs: Make btrfs_scan_devices take a path
>   btrfs-progs: Correctly identify fs on image files in "filesystem"
>     subcommands
>   btrfs-progs: tests: Test fs on image files is correctly recognised
>
>  cmds-device.c                                   |  2 +-
>  cmds-filesystem.c                               |  3 +--
>  disk-io.c                                       |  2 +-
>  tests/cli-tests/010-fi-show-on-new-file/test.sh | 16 ++++++++++++++++
>  utils.c                                         | 18 +++++++++++++++---
>  utils.h                                         |  2 +-
>  6 files changed, 35 insertions(+), 8 deletions(-)
>  create mode 100755 tests/cli-tests/010-fi-show-on-new-file/test.sh
>

Re: [PATCH 0/3] Fix "filesystem" command when fs is on file image

[Nikolay Borisov]

On 16.05.19 г. 12:06 ч., Qu Wenruo wrote:
> 
> 
> On 2019/5/16 下午4:42, Nikolay Borisov wrote:
>> This series fixes the following use case:
>>
>> truncate -s 3g btrfs.img
>> mkfs.btrfs btrfs.img
>> btrrfs fi show .img
>> ERROR: not a valid btrfs filesystem: /root/btrfs.img
>>
>>
>> As evident this currently produces an error due to libblkid not recognising the
>> image file as a filesystem. This stems from the fact progs doesn't force the
>> addition of the image file to libblkid's cache. This series rectifies this.
> 
> I'm wondering if we could just fall back to open_ctree() and print
> needed info using btrfs_fs_info.
> 
> open_ctree() will scan the passed device no matter if it's a block
> device or a file or whatever, and get the correct result of fsid anyway.
> No need to bother blkid cache or whatever.

I agree, tested it locally and it worked, so will send revised version
with a more contained change, just in cmds-filesystem.c

> 
> Thanks,
> Qu
> 
>>
>> Patch 1 extends btrfs_scan_devices to take an optional path argument which will
>> be added to libblkid's cache.
>>
>> Patch 2 Makes 'btrfs filesystem' Utilizes this btrfs_scan_devices' new
>> interface if it detects we want to query a filesystem placed on an image file.
>>
>> Patch 3 Adds simple test case to ensure this works as expected and is not
>> broken in the future.
>>
>> Nikolay Borisov (3):
>>   btrfs-progs: Make btrfs_scan_devices take a path
>>   btrfs-progs: Correctly identify fs on image files in "filesystem"
>>     subcommands
>>   btrfs-progs: tests: Test fs on image files is correctly recognised
>>
>>  cmds-device.c                                   |  2 +-
>>  cmds-filesystem.c                               |  3 +--
>>  disk-io.c                                       |  2 +-
>>  tests/cli-tests/010-fi-show-on-new-file/test.sh | 16 ++++++++++++++++
>>  utils.c                                         | 18 +++++++++++++++---
>>  utils.h                                         |  2 +-
>>  6 files changed, 35 insertions(+), 8 deletions(-)
>>  create mode 100755 tests/cli-tests/010-fi-show-on-new-file/test.sh
>>
>