All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] btrfs-progs: coalesce of patches
@ 2013-06-10 14:56 Anand Jain
  2013-06-10 14:56 ` [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
                   ` (13 more replies)
  0 siblings, 14 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

A patch set revised/rebased on integration-20130524
these were posted before but havn't made into the repo yet.

I.
Introduce btrfs filesystem show --kernel

There are bugs surrounding btrfs filesystem show,
when ran immediately after the oprations like
btrfs fi label <mnt> <label> and btrfs dev delete <mnt>
the btrfs fi show will still show the contents which is
staled by the preceding label and device operation as show
below

btrfs fi show fails to report label immediately after label

----
btrfs fi label /btrfs test10

btrfs fi show
Label: 'test'  uuid: 22b6f318-07d4-4af4-b8e5-b2682cb83a7a
        Total devices 3 FS bytes used 28.00KB
        devid    3 size 15.00GB used 0.00 path /dev/dm-6
        devid    4 size 44.99GB used 1.03GB path /dev/dm-4

btrfs fi label /btrfs
test10

btrfs fi show --kernel
Label: 'test10'  uuid: 22b6f318-07d4-4af4-b8e5-b2682cb83a7a
        Total devices 2 FS bytes used 28.00KB
        devid    3 size 15.00GB used 0.00 path /dev/dm-6
        devid    4 size 44.99GB used 1.03GB path /dev/dm-4
-----

After dropping the cache its fine, but to expect the user
to drop the caches to get a consistent view is not a good idea..

-----
btrfs fi show
Label: 'test3'  uuid: 0a621111-ad84-4d80-842a-dd9c1c60bf51
    Total devices 2 FS bytes used 200.41MB
    devid    2 size 48.23GB used 2.03GB path /dev/dm-5
    devid    1 size 44.99GB used 2.04GB path /dev/dm-4

btrfs fi label /btrfs
test5

echo 3 > /proc/sys/vm/drop_caches
btrfs fi show
Label: 'test5'  uuid: 0a621111-ad84-4d80-842a-dd9c1c60bf51
    Total devices 2 FS bytes used 200.41MB
    devid    2 size 48.23GB used 2.03GB path /dev/dm-5
    devid    1 size 44.99GB used 2.04GB path /dev/dm-4
-----

bugs with btrfs fi show and btrfs device operation..
----
btrfs fi show
Label: 'test'  uuid: 22b6f318-07d4-4af4-b8e5-b2682cb83a7a
        Total devices 3 FS bytes used 28.00KB
        devid    3 size 15.00GB used 0.00 path /dev/dm-6
        devid    4 size 44.99GB used 0.00 path /dev/dm-4
        devid    2 size 48.23GB used 1.03GB path /dev/dm-5

btrfs dev del /dev/dm-5 /btrfs

(dm-5 is replaced with sde, the previous patch use mapper
will fix this, But reports missing since SB is not yet
updated on the disk) 
btrfs fi show
Label: 'test'  uuid: 22b6f318-07d4-4af4-b8e5-b2682cb83a7a
        Total devices 3 FS bytes used 28.00KB
        devid    3 size 15.00GB used 0.00 path /dev/dm-6
        devid    4 size 44.99GB used 1.03GB path /dev/dm-4
        devid    2 size 48.23GB used 1.03GB path /dev/sde


btrfs fi show --mapper
Label: 'test'  uuid: 22b6f318-07d4-4af4-b8e5-b2682cb83a7a
        Total devices 3 FS bytes used 28.00KB
        devid    4 size 44.99GB used 1.03GB path /dev/mapper/mpathe
        devid    3 size 15.00GB used 0.00 path /dev/mapper/mpathbp1
        *** Some devices missing

The newly introduced --kernel option is in line with the
device operation...

btrfs fi show --kernel
Label: 'test'  uuid: 22b6f318-07d4-4af4-b8e5-b2682cb83a7a
        Total devices 2 FS bytes used 28.00KB
        devid    3 size 15.00GB used 0.00 path /dev/dm-6
        devid    4 size 44.99GB used 1.03GB path /dev/dm-4
--------

In long run, I hope to make the --kernel a default option
for the mounted fs and scan devpath to look for non-mounted
btrfs.

II.
  btrfs-progs: device delete to get errors from the kernel
  v3: rebased with other patches here.

III. (patch 1/8 to 5/8)
The idea was to introduce /dev/mapper to find for btrfs disk, 
However I found first we need to congregate the disk scan
procedure at a function so it would help to consistently tune
it across the btrfs-progs. As of now both fi show and
dev scan use the disks scan they do it on their own.

So here it would congregate btrfs-disk scans at the function
scan_devs_for_btrfs, adds /dev/mapper to be used to scan
for btrfs, and updates its calling functions and few bug fixes.

v1->v2:
 Rebased on top of David' integration branch origin/integration-20130524

 patch 1 to 5 (below) are made independent of the idea to have the
 /dev/mapper as one of the path to recognize the btrfs disks.
 Which means they can be installed with out having anything
 new. It just adds the framework/improves to integrated a
 /dev/mapper path, which is only done in the patch 6.
 So patch 1 to 5 are safe.
 Also in v1 I suggested that we have -d option instead of
 long option --all-devices which I have dropped that idea
 here in v2.

 Patch 6: adds a new option --mapper to the filesystem show
 and device scan which inturn will use /dev/mapper to scan
 for the btrfs.

 Example output of using the --mapper option is as below..

btrfs filesystem show --mapper
Label: none  uuid: 0a621111-ad84-4d80-842a-dd9c1c60bf51
	Total devices 2 FS bytes used 1.17MB
	devid    1 size 44.99GB used 2.04GB path /dev/mapper/mpathe
	devid    2 size 48.23GB used 2.03GB path /dev/mapper/mpathd

Label: none  uuid: bad9105f-bdc6-4626-9ba7-80bd97aebe19
	Total devices 1 FS bytes used 28.00KB
	devid    1 size 15.00GB used 2.04GB path /dev/mapper/mpathbp1

Btrfs v0.20-rc1-350-g7731651

btrfs device scan --mapper
-----
[1118885.473298] device fsid bad9105f-bdc6-4626-9ba7-80bd97aebe19 devid 1 transid 4 /dev/mapper/mpathbp1
[1118885.474077] device fsid 0a621111-ad84-4d80-842a-dd9c1c60bf51 devid 2 transid 103 /dev/mapper/mpathd
[1118885.474133] device fsid 0a621111-ad84-4d80-842a-dd9c1c60bf51 devid 1 transid 103 /dev/mapper/mpathe
-----

Anand Jain (9):
  btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
  btrfs-progs: label option in btrfs filesystem show is not coded
  btrfs-progs: update device scan usage
  btrfs-progs: congregate dev scan
  btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is
    provided
  btrfs-progs: scan /dev/mapper in filesystem show and device scan
  btrfs-progs: device delete to get errors from the kernel
  btrfs-progs: get_label_mounted to return label instead of print
  btrfs-progs: introduce btrfs filesystem show --kernel

 btrfs-find-root.c |   2 +-
 cmds-device.c     |  29 ++++--
 cmds-filesystem.c |  21 ++--
 ctree.h           |   1 -
 disk-io.c         |   2 +-
 ioctl.h           | 107 ++++++++++++++++++-
 man/btrfs.8.in    |  16 +--
 utils.c           | 300 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 utils.h           |  13 ++-
 volumes.c         |  20 +++-
 volumes.h         |   3 +
 11 files changed, 467 insertions(+), 47 deletions(-)

-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 20:00   ` Eric Sandeen
  2013-06-10 14:56 ` [PATCH 2/9 v2] btrfs-progs: label option in btrfs filesystem show is not coded Anand Jain
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

btrfs_scan_for_fsid uses only one argument run_ioctl out of 3
so remove the rest two of them

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfs-find-root.c | 2 +-
 disk-io.c         | 2 +-
 utils.c           | 5 ++---
 utils.h           | 3 +--
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 810d835..e736cb5 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -110,7 +110,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
 	}
 
 	if (total_devs != 1) {
-		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
+		ret = btrfs_scan_for_fsid(1);
 		if (ret)
 			goto out;
 	}
diff --git a/disk-io.c b/disk-io.c
index 9ffe6e4..acd5480 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -838,7 +838,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 	}
 
 	if (total_devs != 1) {
-		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
+		ret = btrfs_scan_for_fsid(1);
 		if (ret)
 			goto out;
 	}
diff --git a/utils.c b/utils.c
index 7b4cd74..25f3cb4 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) {
-		if((ret = btrfs_scan_for_fsid(fs_devices_mnt, total_devs, 1)))
+		if((ret = btrfs_scan_for_fsid(1)))
 			return ret;
 	}
 
@@ -1110,8 +1110,7 @@ fail:
 	return ret;
 }
 
-int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
-			int run_ioctls)
+int btrfs_scan_for_fsid(int run_ioctls)
 {
 	int ret;
 
diff --git a/utils.h b/utils.h
index 3c17e14..dba37e8 100644
--- a/utils.h
+++ b/utils.h
@@ -35,8 +35,7 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
 		      struct btrfs_root *root, int fd, char *path,
 		      u64 block_count, u32 io_width, u32 io_align,
 		      u32 sectorsize);
-int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
-			int run_ioctls);
+int btrfs_scan_for_fsid(int run_ioctls);
 void btrfs_register_one_device(char *fname);
 int btrfs_scan_one_dir(char *dirname, int run_ioctl);
 int check_mounted(const char *devicename);
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 2/9 v2] btrfs-progs: label option in btrfs filesystem show is not coded
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
  2013-06-10 14:56 ` [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 14:56 ` [PATCH 3/9 v2] btrfs-progs: update device scan usage Anand Jain
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

appears to be a cut and paste error

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c | 2 +-
 man/btrfs.8.in    | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index f41a72a..c6a7100 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -232,7 +232,7 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
 }
 
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices] [<uuid>|<label>]",
+	"btrfs filesystem show [--all-devices|<uuid>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 9b1f294..ae984f8 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -31,7 +31,7 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBfilesystem label\fP\fI <dev> [newlabel]\fP
 .PP
-\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|<uuid>|<label>]\fP
+\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|<uuid>]\fP
 .PP
 \fBbtrfs\fP \fBfilesystem balance\fP\fI <path> \fP
 .PP
@@ -282,9 +282,9 @@ NOTE: Currently there are the following limitations:
 - the filesystem should not have more than one device.
 .TP
 
-\fBfilesystem show\fR [--all-devices|<uuid>|<label>]\fR
-Show the btrfs filesystem with some additional info. If no \fIUUID\fP or 
-\fIlabel\fP is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
+\fBfilesystem show\fR [--all-devices|<uuid>]\fR
+Show the btrfs filesystem with some additional info. If no \fIUUID\fP
+is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
 If \fB--all-devices\fP is passed, all the devices under /dev are scanned;
 otherwise the devices list is extracted from the /proc/partitions file.
 .TP
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 3/9 v2] btrfs-progs: update device scan usage
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
  2013-06-10 14:56 ` [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
  2013-06-10 14:56 ` [PATCH 2/9 v2] btrfs-progs: label option in btrfs filesystem show is not coded Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 14:56 ` [PATCH 4/9 v3] btrfs-progs: congregate dev scan Anand Jain
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

the btrfs device scan usage does not publish --all-devices
option so add it

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-device.c b/cmds-device.c
index 41e79d3..d25159b 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -178,7 +178,7 @@ static int cmd_rm_dev(int argc, char **argv)
 }
 
 static const char * const cmd_scan_dev_usage[] = {
-	"btrfs device scan [<device>...]",
+	"btrfs device scan [<--all-devices>|<device> [<device>...]]",
 	"Scan devices for a btrfs filesystem",
 	NULL
 };
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 4/9 v3] btrfs-progs: congregate dev scan
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (2 preceding siblings ...)
  2013-06-10 14:56 ` [PATCH 3/9 v2] btrfs-progs: update device scan usage Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 14:56 ` [PATCH 5/9 v2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

the dev scan to find btrfs is performed at two locations
all most the same way one at filesystem show and another
at device scan. They both follow the same steps. This
patch does not alter anything except that it brings these
two same logic into the function scan_for_btrfs so that
we can play tweaking it.

the patch which recommends to use /dev/mapper
will also need it

v3:
bring in btrfs_scan_for_fsid to use scan_for_btrfs

thanks

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c     | 11 +++--------
 cmds-filesystem.c |  9 +++------
 utils.c           | 22 ++++++++++++++++++++--
 utils.h           |  5 ++++-
 4 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index d25159b..b6ecb3b 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -186,26 +186,21 @@ static const char * const cmd_scan_dev_usage[] = {
 static int cmd_scan_dev(int argc, char **argv)
 {
 	int	i, fd, e;
-	int	checklist = 1;
+	int	where = BTRFS_SCAN_PROC;
 	int	devstart = 1;
 
 	if( argc > 1 && !strcmp(argv[1],"--all-devices")){
 		if (check_argc_max(argc, 2))
 			usage(cmd_scan_dev_usage);
 
-		checklist = 0;
+		where = BTRFS_SCAN_DEV;
 		devstart += 1;
 	}
 
 	if(argc<=devstart){
-
 		int ret;
-
 		printf("Scanning for Btrfs filesystems\n");
-		if(checklist)
-			ret = btrfs_scan_block_devices(1);
-		else
-			ret = btrfs_scan_one_dir("/dev", 1);
+		ret = scan_for_btrfs(where, 1);
 		if (ret){
 			fprintf(stderr, "ERROR: error %d while scanning\n", ret);
 			return 18;
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index c6a7100..0d76d58 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -245,21 +245,18 @@ static int cmd_show(int argc, char **argv)
 	struct list_head *cur_uuid;
 	char *search = 0;
 	int ret;
-	int checklist = 1;
+	int where = BTRFS_SCAN_PROC;
 	int searchstart = 1;
 
 	if( argc > 1 && !strcmp(argv[1],"--all-devices")){
-		checklist = 0;
+		where = BTRFS_SCAN_DEV;
 		searchstart += 1;
 	}
 
 	if (check_argc_max(argc, searchstart + 1))
 		usage(cmd_show_usage);
 
-	if(checklist)
-		ret = btrfs_scan_block_devices(0);
-	else
-		ret = btrfs_scan_one_dir("/dev", 0);
+	ret = scan_for_btrfs(where, 0);
 
 	if (ret){
 		fprintf(stderr, "ERROR: error %d while scanning\n", ret);
diff --git a/utils.c b/utils.c
index 25f3cb4..6b2344d 100644
--- a/utils.c
+++ b/utils.c
@@ -1114,9 +1114,9 @@ int btrfs_scan_for_fsid(int run_ioctls)
 {
 	int ret;
 
-	ret = btrfs_scan_block_devices(run_ioctls);
+	ret = scan_for_btrfs(BTRFS_SCAN_PROC, run_ioctls);
 	if (ret)
-		ret = btrfs_scan_one_dir("/dev", run_ioctls);
+		ret = scan_for_btrfs(BTRFS_SCAN_DEV, run_ioctls);
 	return ret;
 }
 
@@ -1806,3 +1806,21 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 	close(fd);
 	return 0;
 }
+
+/*
+ * scans devs for the btrfs
+*/
+int scan_for_btrfs(int where, int update_kernel)
+{
+	int ret = 0;
+
+	switch (where) {
+	case BTRFS_SCAN_PROC:
+		ret = btrfs_scan_block_devices(update_kernel);
+		break;
+	case BTRFS_SCAN_DEV:
+		ret = btrfs_scan_one_dir("/dev", update_kernel);
+		break;
+	}
+	return ret;
+}
diff --git a/utils.h b/utils.h
index dba37e8..78f3a65 100644
--- a/utils.h
+++ b/utils.h
@@ -24,6 +24,9 @@
 
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
+#define BTRFS_SCAN_PROC	1
+#define BTRFS_SCAN_DEV	2
+
 int make_btrfs(int fd, const char *device, const char *label,
 	       u64 blocks[6], u64 num_bytes, u32 nodesize,
 	       u32 leafsize, u32 sectorsize, u32 stripesize);
@@ -64,5 +67,5 @@ u64 btrfs_device_size(int fd, struct stat *st);
 /* Helper to always get proper size of the destination string */
 #define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest))
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
-
+int scan_for_btrfs(int where, int update_kernel);
 #endif
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 5/9 v2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (3 preceding siblings ...)
  2013-06-10 14:56 ` [PATCH 4/9 v3] btrfs-progs: congregate dev scan Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 14:56 ` [PATCH 6/9 v2] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

we would need btrfs_scan_one_dir to san devs under /dev/mapper,
but /dev/mapper has links to the actual devs and current implementation
of btrfs_scan_one_dir skips links so it does not pick any
links under /dev/mapper. skipping links is fine when scanning whole of
/dev. But when we just want to scan /dev/mapper we want to avoid skipping
links otherwise we would be left with nothing.

This patch just adds the check if we are scanning devs
ONLY under /dev/mapper if when so it will not skip links

Thanks

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/utils.c b/utils.c
index 6b2344d..a329b7a 100644
--- a/utils.c
+++ b/utils.c
@@ -1018,6 +1018,7 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
 	struct list_head pending_list;
 	struct btrfs_fs_devices *tmp_devices;
 	u64 num_devices;
+	int skip_link = 1;
 
 	INIT_LIST_HEAD(&pending_list);
 
@@ -1026,6 +1027,9 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
 		return -ENOMEM;
 	strcpy(pending->name, dirname);
 
+	if (!strncmp(dirname, "/dev/mapper", strlen("/dev/mapper")))
+		skip_link = 0;
+
 again:
 	dirname_len = strlen(pending->name);
 	fullpath = malloc(PATH_MAX);
@@ -1057,7 +1061,7 @@ again:
 			fprintf(stderr, "failed to stat %s\n", fullpath);
 			continue;
 		}
-		if (S_ISLNK(st.st_mode))
+		if (skip_link && S_ISLNK(st.st_mode))
 			continue;
 		if (S_ISDIR(st.st_mode)) {
 			struct pending_dir *next = malloc(sizeof(*next));
@@ -1068,7 +1072,7 @@ again:
 			strcpy(next->name, fullpath);
 			list_add_tail(&next->list, &pending_list);
 		}
-		if (!S_ISBLK(st.st_mode)) {
+		if (skip_link && !S_ISBLK(st.st_mode)) {
 			continue;
 		}
 		fd = open(fullpath, O_RDONLY);
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 6/9 v2] btrfs-progs: scan /dev/mapper in filesystem show and device scan
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (4 preceding siblings ...)
  2013-06-10 14:56 ` [PATCH 5/9 v2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 14:56 ` [PATCH 7/9 v3] btrfs-progs: device delete to get errors from the kernel Anand Jain
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

Currently, btrsf fi show and btrfs dev scan uses
/proc/partitions (by default) (which gives priority
to dm-<x> over sd<y> paths) and with --all-devices it
will scan /dev only (when it skips links under /dev/mapper).

However using /dev/mapper paths are in common practice
with mount, fstab, and lvm, so its better to be consistent
with them.

This patch adds --mapper option to device scan and
filesystem show cli, when used will look for btrfs
dev under /dev/mapper and will use the links provided
under /dev/mapper.

eg:
btrfs fi show --mapper
Label: none  uuid: 0a621111-ad84-4d80-842a-dd9c1c60bf51
        Total devices 2 FS bytes used 1.17MB
        devid    1 size 44.99GB used 2.04GB path /dev/mapper/mpathe
        devid    2 size 48.23GB used 2.03GB path /dev/mapper/mpathd

Label: none  uuid: bad9105f-bdc6-4626-9ba7-80bd97aebe19
        Total devices 1 FS bytes used 28.00KB
        devid    1 size 15.00GB used 2.04GB path /dev/mapper/mpathbp1

In the long run I want the usage of /dev/mapper path
along with /proc/partitions be the default option to
scan for the btrfs devs. /proc/partitions must be scanned
as well because to include the mapper blacklisted devs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c     |  8 +++++++-
 cmds-filesystem.c |  7 +++++--
 man/btrfs.8.in    | 12 +++++++-----
 utils.c           |  3 +++
 utils.h           |  5 +++--
 5 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index b6ecb3b..ef6bc60 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -178,7 +178,7 @@ static int cmd_rm_dev(int argc, char **argv)
 }
 
 static const char * const cmd_scan_dev_usage[] = {
-	"btrfs device scan [<--all-devices>|<device> [<device>...]]",
+	"btrfs device scan [<--all-devices>|<--mapper>|<device> [<device>...]]",
 	"Scan devices for a btrfs filesystem",
 	NULL
 };
@@ -195,6 +195,12 @@ static int cmd_scan_dev(int argc, char **argv)
 
 		where = BTRFS_SCAN_DEV;
 		devstart += 1;
+	} else if( argc > 1 && !strcmp(argv[1],"--mapper")){
+		if (check_argc_max(argc, 2))
+			usage(cmd_scan_dev_usage);
+
+		where = BTRFS_SCAN_MAPPER;
+		devstart += 1;
 	}
 
 	if(argc<=devstart){
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 0d76d58..9b7bcf1 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -232,7 +232,7 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
 }
 
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices|<uuid>]",
+	"btrfs filesystem show [--all-devices|--mapper|<uuid>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
@@ -248,9 +248,12 @@ static int cmd_show(int argc, char **argv)
 	int where = BTRFS_SCAN_PROC;
 	int searchstart = 1;
 
-	if( argc > 1 && !strcmp(argv[1],"--all-devices")){
+	if (argc > 1 && !strcmp(argv[1],"--all-devices")) {
 		where = BTRFS_SCAN_DEV;
 		searchstart += 1;
+	} else if (argc > 1 && !strcmp(argv[1],"--mapper")) {
+		where = BTRFS_SCAN_MAPPER;
+		searchstart += 1;
 	}
 
 	if (check_argc_max(argc, searchstart + 1))
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index ae984f8..8988b16 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -31,11 +31,11 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBfilesystem label\fP\fI <dev> [newlabel]\fP
 .PP
-\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|<uuid>]\fP
+\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|--mapper|<uuid>]\fP
 .PP
 \fBbtrfs\fP \fBfilesystem balance\fP\fI <path> \fP
 .PP
-\fBbtrfs\fP \fBdevice scan\fP\fI [--all-devices|<device> [<device>...]]\fP
+\fBbtrfs\fP \fBdevice scan\fP\fI [--all-devices|--mapper|<device> [<device>...]]\fP
 .PP
 \fBbtrfs\fP \fBdevice stats\fP [-z] {\fI<path>\fP|\fI<device>\fP}
 .PP
@@ -282,10 +282,11 @@ NOTE: Currently there are the following limitations:
 - the filesystem should not have more than one device.
 .TP
 
-\fBfilesystem show\fR [--all-devices|<uuid>]\fR
+\fBfilesystem show\fR [--all-devices|--mapper|<uuid>]\fR
 Show the btrfs filesystem with some additional info. If no \fIUUID\fP
 is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
 If \fB--all-devices\fP is passed, all the devices under /dev are scanned;
+If \fB--mapper\fP is passed, all the devices under /dev/mapper are scanned;
 otherwise the devices list is extracted from the /proc/partitions file.
 .TP
 
@@ -314,11 +315,12 @@ Add device(s) to the filesystem identified by \fI<path>\fR.
 Remove device(s) from a filesystem identified by \fI<path>\fR.
 .TP
 
-\fBdevice scan\fR \fI[--all-devices|<device> [<device>...]\fR
+\fBdevice scan\fR \fI[--all-devices|--mapper|<device> [<device>...]\fR
 If one or more devices are passed, these are scanned for a btrfs filesystem. 
 If no devices are passed, \fBbtrfs\fR scans all the block devices listed
 in the /proc/partitions file.
-Finally, if \fB--all-devices\fP is passed, all the devices under /dev are 
+If \fB--all-devices\fP is passed, all the devices under /dev are 
+Finally, if \fB--mapper\fP is passed, all the devices under /dev/mapper are 
 scanned.
 .TP
 
diff --git a/utils.c b/utils.c
index a329b7a..f9545b8 100644
--- a/utils.c
+++ b/utils.c
@@ -1825,6 +1825,9 @@ int scan_for_btrfs(int where, int update_kernel)
 	case BTRFS_SCAN_DEV:
 		ret = btrfs_scan_one_dir("/dev", update_kernel);
 		break;
+	case BTRFS_SCAN_MAPPER:
+		ret = btrfs_scan_one_dir("/dev/mapper", update_kernel);
+		break;
 	}
 	return ret;
 }
diff --git a/utils.h b/utils.h
index 78f3a65..733d13b 100644
--- a/utils.h
+++ b/utils.h
@@ -24,8 +24,9 @@
 
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
-#define BTRFS_SCAN_PROC	1
-#define BTRFS_SCAN_DEV	2
+#define BTRFS_SCAN_PROC      1
+#define BTRFS_SCAN_DEV       2
+#define BTRFS_SCAN_MAPPER    3
 
 int make_btrfs(int fd, const char *device, const char *label,
 	       u64 blocks[6], u64 num_bytes, u32 nodesize,
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 7/9 v3] btrfs-progs: device delete to get errors from the kernel
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (5 preceding siblings ...)
  2013-06-10 14:56 ` [PATCH 6/9 v2] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 14:56 ` [PATCH 8/9] btrfs-progs: get_label_mounted to return label instead of print Anand Jain
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

when user runs command btrfs dev del the raid requisite error if any
goes to the /var/log/messages, its not good idea to clutter messages
with these user (knowledge) errors, further user don't have to review
the system messages to know problem with the cli it should be dropped
to the user as part of the cli return.

to bring this feature created a set of the ERROR defined
BTRFS_ERROR_DEV* error codes and created their error string.

I expect this enum to be added with other error which we might
want to communicate to the user land

v3:
moved the code with in the file no logical change

v1->v2:
introduce error codes for the device mgmt usage

v1:
add another parameter to the ioctl arg structure to carry the error string

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c | 10 ++++++++--
 ioctl.h       | 43 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index ef6bc60..9525fcf 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -163,8 +163,14 @@ static int cmd_rm_dev(int argc, char **argv)
 		strncpy_null(arg.name, argv[i]);
 		res = ioctl(fdmnt, BTRFS_IOC_RM_DEV, &arg);
 		e = errno;
-		if(res<0){
-			fprintf(stderr, "ERROR: error removing the device '%s' - %s\n",
+		if (res > 0) {
+			fprintf(stderr,
+				"ERROR: error removing the device '%s' - %s\n",
+				argv[i], btrfs_err_str(res));
+			ret++;
+		} else if (res < 0) {
+			fprintf(stderr,
+				"ERROR: error removing the device '%s' - %s\n",
 				argv[i], strerror(e));
 			ret++;
 		}
diff --git a/ioctl.h b/ioctl.h
index f786410..a40a4a1 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -455,6 +455,48 @@ struct btrfs_ioctl_qgroup_create_args {
 	__u64 create;
 	__u64 qgroupid;
 };
+
+/* Error codes as returned by the kernel */
+enum btrfs_err_code {
+	notused,
+	BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_TGT_REPLACE,
+	BTRFS_ERROR_DEV_MISSING_NOT_FOUND,
+	BTRFS_ERROR_DEV_ONLY_WRITABLE,
+	BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS
+};
+
+/* An error code to error string mapping for the kernel
+*  error codes
+*/
+static inline char *btrfs_err_str(enum btrfs_err_code err_code)
+{
+	switch (err_code) {
+		case BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET:
+			return "unable to go below two devices on raid1";
+		case BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET:
+			return "unable to go below four devices on raid10";
+		case BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET:
+			return "unable to go below two devices on raid5";
+		case BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET:
+			return "unable to go below three devices on raid6";
+		case BTRFS_ERROR_DEV_TGT_REPLACE:
+			return "unable to remove the dev_replace target dev";
+		case BTRFS_ERROR_DEV_MISSING_NOT_FOUND:
+			return "no missing devices found to remove";
+		case BTRFS_ERROR_DEV_ONLY_WRITABLE:
+			return "unable to remove the only writeable device";
+		case BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS:
+			return "add/delete/balance/replace/resize operation "\
+				"in progress";
+		default:
+			return NULL;
+	}
+}
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -550,7 +592,6 @@ struct btrfs_ioctl_clone_range_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
-
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 8/9] btrfs-progs: get_label_mounted to return label instead of print
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (6 preceding siblings ...)
  2013-06-10 14:56 ` [PATCH 7/9 v3] btrfs-progs: device delete to get errors from the kernel Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-21  7:41   ` [PATCH 08/13 v2] " Anand Jain
  2013-06-10 14:56 ` [PATCH 9/9 v2] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

This would help to reuse the function

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/utils.c b/utils.c
index f9545b8..b205005 100644
--- a/utils.c
+++ b/utils.c
@@ -1313,7 +1313,7 @@ static int get_label_unmounted(const char *dev)
  * mounted path rather than device.  Return the corresponding error
  * the user specified the device path.
  */
-static int get_label_mounted(const char *mount_path)
+static int get_label_mounted(const char *mount_path, char *labelp)
 {
 	char label[BTRFS_LABEL_SIZE];
 	int fd;
@@ -1331,16 +1331,24 @@ static int get_label_mounted(const char *mount_path)
 		return -1;
 	}
 
-	fprintf(stdout, "%s\n", label);
+	strncpy(labelp, label, sizeof(label));
 	close(fd);
 	return 0;
 }
 
 int get_label(const char *btrfs_dev)
 {
-	return is_existing_blk_or_reg_file(btrfs_dev) ?
-		get_label_unmounted(btrfs_dev) :
-		get_label_mounted(btrfs_dev);
+	int ret;
+	char label[BTRFS_LABEL_SIZE];
+
+	if (is_existing_blk_or_reg_file(btrfs_dev))
+		ret = get_label_unmounted(btrfs_dev);
+	else {
+		ret = get_label_mounted(btrfs_dev, label);
+		if (!ret)
+			fprintf(stdout, "%s\n", label);
+	}
+	return ret;
 }
 
 int set_label(const char *btrfs_dev, const char *label)
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 9/9 v2] btrfs-progs: introduce btrfs filesystem show --kernel
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (7 preceding siblings ...)
  2013-06-10 14:56 ` [PATCH 8/9] btrfs-progs: get_label_mounted to return label instead of print Anand Jain
@ 2013-06-10 14:56 ` Anand Jain
  2013-06-10 14:59 ` [PATCH 0/2] btrfs: coalesce of patches Anand Jain
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:56 UTC (permalink / raw)
  To: linux-btrfs

As of now btrfs filesystem show reads directly from
disks. So sometimes output can be stale, mainly when
user want to verify their latest operation like,
labeling or device delete or add... etc.

This patch adds --kernel option to the 'filesystem show'
subcli, which will read from the kernel instead of
the disks directly.

This is implemented by adding new ioctl BTRFS_IOC_GET_FSIDS
and BTRFS_IOC_GET_DEVS which is to read filesystem info and
device info from the kernel using the btrfs-control
interface. Which will have advantage of using the the
interface even when the fs is not mounted if in case needed.

This interface finds that there are stale fsid when the
btrfs is unmounted, this will be a new issue to resolve.

This btrfs-progs depends on the kernel patch
btrfs: add framework to read fs info and dev info from the kernel

paramter self_sz:
  For future enhancements. When payload structure changes
  use this to communicate version differences.

ioctl header and payload:
  These ioctl uses header and payload model where
  new payload (a structure) can be easily plugged-in
  to get new information from the kernel.

v1->v2:
	.code optimized to remove redundancy

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c |   7 +-
 ctree.h           |   1 -
 ioctl.h           |  64 ++++++++++++++
 utils.c           | 244 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h           |   4 +
 volumes.c         |  20 ++++-
 volumes.h         |   3 +
 7 files changed, 339 insertions(+), 4 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 9b7bcf1..c5d2264 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -232,7 +232,7 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
 }
 
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices|--mapper|<uuid>]",
+	"btrfs filesystem show [--all-devices|--mapper|--kernel|<uuid>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
@@ -254,6 +254,9 @@ static int cmd_show(int argc, char **argv)
 	} else if (argc > 1 && !strcmp(argv[1],"--mapper")) {
 		where = BTRFS_SCAN_MAPPER;
 		searchstart += 1;
+	} else if (argc > 1 && !strcmp(argv[1],"--kernel")) {
+		where = BTRFS_SCAN_KERNEL;
+		searchstart += 1;
 	}
 
 	if (check_argc_max(argc, searchstart + 1))
@@ -262,7 +265,7 @@ static int cmd_show(int argc, char **argv)
 	ret = scan_for_btrfs(where, 0);
 
 	if (ret){
-		fprintf(stderr, "ERROR: error %d while scanning\n", ret);
+		fprintf(stderr, "ERROR: %d while scanning\n", ret);
 		return 18;
 	}
 	
diff --git a/ctree.h b/ctree.h
index 0af7477..5e8c610 100644
--- a/ctree.h
+++ b/ctree.h
@@ -350,7 +350,6 @@ struct btrfs_header {
  * room to translate 14 chunks with 3 stripes each.
  */
 #define BTRFS_SYSTEM_CHUNK_ARRAY_SIZE 2048
-#define BTRFS_LABEL_SIZE 256
 
 /*
  * just in case we somehow lose the roots and are not able to mount,
diff --git a/ioctl.h b/ioctl.h
index a40a4a1..438c82a 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -28,6 +28,7 @@ extern "C" {
 
 #define BTRFS_IOCTL_MAGIC 0x94
 #define BTRFS_VOL_NAME_MAX 255
+#define BTRFS_LABEL_SIZE 256
 
 /* this should be 4k */
 #define BTRFS_PATH_NAME_MAX 4087
@@ -497,6 +498,65 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 	}
 }
 
+
+/* ioctl header */
+struct btrfs_ioctl_header {
+	__u64 sz_self;	/* in/out sz of header*/
+	__u64 sz;	/* in/out sz of payload list*/
+	__u64 count;	/* in/out number of payload units*/
+} __attribute__ ((__packed__));
+
+/* ioctl payloads */
+#define BTRFS_FSIDS_LEN	16
+#define BTRFS_FS_MOUNTED	(1LLU << 0)
+
+struct btrfs_ioctl_fsinfo {
+	__u64 sz_self;
+	__u8 fsid[BTRFS_FSID_SIZE]; /* out */
+	__u64 num_devices;
+	__u64 total_devices;
+	__u64 missing_devices;
+	__u64 total_rw_bytes;
+	__u64 bytes_used;
+	__u64 flags;
+	__u64 default_mount_subvol_id;
+	char label[BTRFS_LABEL_SIZE];
+} __attribute__ ((__packed__));
+
+struct btrfs_ioctl_fs_list {
+	__u64 all; /* in */
+	struct btrfs_ioctl_fsinfo fsinfo[BTRFS_FSIDS_LEN]; /* out */
+} __attribute__ ((__packed__));
+
+/* flags below */
+#define BTRFS_DEVS_LEN	16
+#define BTRFS_DEV_WRITEABLE	(1LLU << 0)
+#define BTRFS_DEV_IN_FS_MD	(1LLU << 1)
+#define BTRFS_DEV_MISSING	(1LLU << 2)
+#define BTRFS_DEV_CAN_DISCARD	(1LLU << 3)
+#define BTRFS_DEV_SUBSTITUTED	(1LLU << 4)
+
+struct btrfs_ioctl_devinfo {
+	__u64 sz_self;
+	__u64 flags;
+	__u64 devid;
+	__u64 total_bytes;
+	__u64 disk_total_bytes;
+	__u64 bytes_used;
+	__u64 type;
+	__u64 generation;
+	__u32 io_align;
+	__u32 io_width;
+	__u32 sector_size;
+	__u8 uuid[BTRFS_UUID_SIZE];
+	__u8 name[BTRFS_PATH_NAME_MAX + 1];
+} __attribute__ ((__packed__));
+
+struct btrfs_ioctl_dev_list {
+	__u8 fsid[BTRFS_FSID_SIZE]; /* in */
+	struct btrfs_ioctl_devinfo dev[BTRFS_DEVS_LEN]; /* out */
+} __attribute__ ((__packed__));
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -592,6 +652,10 @@ struct btrfs_ioctl_clone_range_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
+#define BTRFS_IOC_GET_FSIDS _IOWR(BTRFS_IOCTL_MAGIC, 54, \
+				struct btrfs_ioctl_header)
+#define BTRFS_IOC_GET_DEVS _IOWR(BTRFS_IOCTL_MAGIC, 55, \
+				struct btrfs_ioctl_header)
 #ifdef __cplusplus
 }
 #endif
diff --git a/utils.c b/utils.c
index b205005..fe5a6e0 100644
--- a/utils.c
+++ b/utils.c
@@ -1819,6 +1819,247 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 	return 0;
 }
 
+static struct btrfs_ioctl_header * alloc_ioctl_payload(void **plp,
+		size_t *sz_pl, __u64 mul, __u64 rq_len)
+{
+	struct btrfs_ioctl_header *header;
+	size_t sz;
+	__u64 cnt = 1;
+	size_t sz_payload = *sz_pl;
+
+	if (rq_len)
+		cnt = rq_len/mul + ((rq_len % mul) ? 1 : 0);
+
+	*sz_pl = sz_payload * mul * cnt;
+	sz = sizeof(*header) + *sz_pl;
+	header = malloc(sz);
+	if (!header)
+		return NULL;
+
+	memset(header, 0, sz);
+	header->sz_self = sizeof(*header);
+	header->sz = *sz_pl;
+	header->count = mul * cnt;
+	*plp = ((__u8 *)header) + sizeof(*header);
+
+	return header;
+}
+
+/* scans for fsid(s) in the kernel using the btrfs-control
+ * interface.
+ * returns:
+ * > 0 : Success, and number indicates number of fsid(s) found,
+ *       out_fslist points to the fsid(s)
+ *       the caller must free out_fslist when it is not null
+ * 0   : when no fsid is found, but successful
+ * < 0 : upon error
+ */
+int scan_fsid_kernel(struct btrfs_ioctl_fs_list **out_fslist, int all)
+{
+	int res, fd, e;
+	struct btrfs_ioctl_header *argp;
+	struct btrfs_ioctl_fs_list *fslist;
+	__u64 alloc_cnt;
+	size_t pl_sz;
+	__u64 req_cnt = 1;
+
+	*out_fslist = NULL;
+	fd = open("/dev/btrfs-control", O_RDWR);
+	e = errno;
+	if (fd < 0) {
+		perror("failed to open /dev/btrfs-control");
+		return -e;
+	}
+
+again:
+	pl_sz = sizeof(*fslist);
+	argp = alloc_ioctl_payload((void **)&fslist, &pl_sz,
+					BTRFS_FSIDS_LEN, req_cnt);
+	if (!argp) {
+		close(fd);
+		return -ENOMEM;
+	}
+	fslist->all = all;
+	alloc_cnt = argp->count;
+	res = ioctl(fd, BTRFS_IOC_GET_FSIDS, argp);
+	e = errno;
+	if (res) {
+		printf("ERROR: get_fsid ioctl failed %d - %s\n",
+			res, strerror(e));
+		req_cnt = 0;
+		goto out;
+	}
+	/* ioctl returns fsid count in count parameter*/
+	req_cnt = argp->count;
+	if (req_cnt > alloc_cnt) {
+		/* if kernel has more than initially allocated
+		 * count that means alloc more then call ioctl
+		 * again
+		 */
+		free(argp);
+		goto again;
+	}
+	if (!req_cnt)
+		goto out;
+	*out_fslist = malloc(pl_sz);
+	if (!*out_fslist) {
+		res = -ENOMEM;
+		goto out;
+	}
+	memcpy(*out_fslist, fslist, pl_sz);
+out:
+	free(argp);
+	close(fd);
+	return req_cnt;
+}
+
+/* scans for devs for a given fsid in the kernel using the
+ * btrfs-control interface.
+ * returns:
+ * > 0 : for success, and number indicates number of devs found,
+ *       out_devlist points to the devs
+ *       the caller must free out_devlist when it is not null
+ * 0   : when no dev is found, but successful
+ * < 0 : upon error
+ */
+int get_devs_kernel(struct btrfs_ioctl_dev_list **out_devlist, __u8 *fsid)
+{
+	int res, fd, e;
+	struct btrfs_ioctl_header *argp;
+	struct btrfs_ioctl_dev_list *devlist;
+	__u64 alloc_cnt;
+	size_t pl_sz = 0;
+	__u64 req_cnt = 1;
+
+	*out_devlist = NULL;
+	fd = open("/dev/btrfs-control", O_RDWR);
+	e = errno;
+	if (fd < 0) {
+		perror("failed to open /dev/btrfs-control");
+		return -e;
+	}
+
+again:
+	pl_sz = sizeof(*devlist);
+	argp = alloc_ioctl_payload((void **)&devlist, &pl_sz,
+					BTRFS_DEVS_LEN, req_cnt);
+	if (!argp) {
+		close(fd);
+		return -ENOMEM;
+	}
+	memcpy(&devlist->fsid, fsid, BTRFS_FSID_SIZE);
+	alloc_cnt = argp->count;
+	res = ioctl(fd, BTRFS_IOC_GET_DEVS, argp);
+	e = errno;
+	if (res) {
+		printf("ERROR: scan_fsid ioctl failed %d - %s\n",
+			res, strerror(e));
+		goto out;
+	}
+	/* ioctl returns fsid count in count parameter*/
+	req_cnt = argp->count;
+	if (req_cnt > alloc_cnt) {
+		/* if kernel has more than initially alloc-ed count
+		 * then alloc more chunk then call ioctl again
+		*/
+		free(argp);
+		goto again;
+	}
+	if (!req_cnt)
+		goto out;
+	*out_devlist = malloc(pl_sz);
+	if (!*out_devlist) {
+		res = -ENOMEM;
+		goto out;
+	}
+	memcpy(*out_devlist, devlist, pl_sz);
+out:
+	free(argp);
+	close(fd);
+	return req_cnt;
+}
+
+
+/* scan kernel for the btrfs dev registered */
+int btrfs_scan_kernel()
+{
+	int e;
+	int dev_cnt;
+	int fsid_cnt;
+	int i, j;
+	int all = 1;
+
+	struct btrfs_ioctl_fsinfo *fsinfo;
+	struct btrfs_ioctl_fs_list *fslist = NULL;
+	struct btrfs_ioctl_devinfo *dev;
+	struct btrfs_ioctl_dev_list *devlist = NULL;
+
+	struct btrfs_fs_devices *fs_devices;
+	struct btrfs_super_block *disk_super;
+
+	disk_super = (struct btrfs_super_block *)malloc(sizeof(*disk_super));
+	memset(disk_super, '0', sizeof(disk_super));
+
+	fsid_cnt = scan_fsid_kernel(&fslist, all);
+	e = errno;
+	if (fsid_cnt < 0) {
+		printf("ERROR: scan for fs failed %d, - %s\n",
+			fsid_cnt, strerror(e));
+		return 1;
+	}
+
+	if (!fsid_cnt) {
+		BUG_ON(fslist);
+		return 0;
+	}
+	for (i = 0; i < fsid_cnt; i++) {
+		fsinfo = &fslist->fsinfo[i];
+		/* we are not keen in the fsid which are
+		 * not mounted/stale skip them
+		 */
+		if (!(fsinfo->flags & BTRFS_FS_MOUNTED))
+			continue;
+		dev_cnt = get_devs_kernel(&devlist, fsinfo->fsid);
+		e = errno;
+		if (dev_cnt < 0) {
+			free(fslist);
+			printf("Error: get_devs failed %d, - %s\n",
+				dev_cnt, strerror(e));
+			return 1;
+		}
+		if (!dev_cnt)
+			continue;
+
+		device_list_fini(fsinfo->fsid);
+		for (j = 0; j < dev_cnt; j++) {
+			dev = &devlist->dev[j];
+			if (fsinfo->flags & BTRFS_FS_MOUNTED) {
+				memcpy(disk_super->fsid, fsinfo->fsid, BTRFS_FSID_SIZE);
+				memcpy(disk_super->dev_item.uuid, dev->uuid,
+						BTRFS_UUID_SIZE);
+				memcpy(disk_super->label, fsinfo->label, BTRFS_LABEL_SIZE);
+				disk_super->num_devices = dev_cnt;
+				disk_super->generation = dev->generation;
+
+				disk_super->bytes_used = fsinfo->bytes_used;
+				disk_super->dev_item.total_bytes = dev->disk_total_bytes;
+				disk_super->dev_item.bytes_used = dev->bytes_used;
+
+				device_list_add((char *)dev->name, disk_super,
+						dev->devid, &fs_devices);
+				memset(disk_super, '0', sizeof(disk_super));
+			}
+		}
+		if (devlist)
+			free(devlist);
+	}
+
+	if (fslist)
+		free(fslist);
+
+	return 0;
+}
+
 /*
  * scans devs for the btrfs
 */
@@ -1836,6 +2077,9 @@ int scan_for_btrfs(int where, int update_kernel)
 	case BTRFS_SCAN_MAPPER:
 		ret = btrfs_scan_one_dir("/dev/mapper", update_kernel);
 		break;
+	case BTRFS_SCAN_KERNEL:
+		ret = btrfs_scan_kernel();
+		break;
 	}
 	return ret;
 }
diff --git a/utils.h b/utils.h
index 733d13b..116d1e3 100644
--- a/utils.h
+++ b/utils.h
@@ -27,6 +27,7 @@
 #define BTRFS_SCAN_PROC      1
 #define BTRFS_SCAN_DEV       2
 #define BTRFS_SCAN_MAPPER    3
+#define BTRFS_SCAN_KERNEL    4
 
 int make_btrfs(int fd, const char *device, const char *label,
 	       u64 blocks[6], u64 num_bytes, u32 nodesize,
@@ -69,4 +70,7 @@ u64 btrfs_device_size(int fd, struct stat *st);
 #define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest))
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
 int scan_for_btrfs(int where, int update_kernel);
+int btrfs_scan_kernel(void);
+int scan_fsid_kernel(struct btrfs_ioctl_fs_list **fslist, int all);
+int get_devs_kernel(struct btrfs_ioctl_dev_list **devlist, __u8 *fsid);
 #endif
diff --git a/volumes.c b/volumes.c
index aa1c3dd..81904c6 100644
--- a/volumes.c
+++ b/volumes.c
@@ -87,7 +87,7 @@ static struct btrfs_fs_devices *find_fsid(u8 *fsid)
 	return NULL;
 }
 
-static int device_list_add(const char *path,
+int device_list_add(const char *path,
 			   struct btrfs_super_block *disk_super,
 			   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
 {
@@ -154,6 +154,24 @@ static int device_list_add(const char *path,
 	return 0;
 }
 
+void device_list_fini(u8 *fsid)
+{
+	struct list_head *fsids;
+	struct list_head *cur_fsid;
+	struct btrfs_fs_devices *fs_devices;
+
+	fsids = btrfs_scanned_uuids();
+	list_for_each(cur_fsid, fsids) {
+		fs_devices = list_entry(cur_fsid, struct btrfs_fs_devices,
+				list);
+		if (!memcmp(fs_devices->fsid, fsid, BTRFS_FSID_SIZE)) {
+			list_del(&fs_devices->devices);
+			list_del(&fs_devices->list);
+			break;
+		}
+	}
+}
+
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
 {
 	struct btrfs_fs_devices *seed_devices;
diff --git a/volumes.h b/volumes.h
index 911f788..6286d83 100644
--- a/volumes.h
+++ b/volumes.h
@@ -190,4 +190,7 @@ int btrfs_add_system_chunk(struct btrfs_trans_handle *trans,
 int btrfs_chunk_readonly(struct btrfs_root *root, u64 chunk_offset);
 struct btrfs_device *btrfs_find_device_by_devid(struct btrfs_root *root,
                                                 u64 devid, int instance);
+int device_list_add(const char *path, struct btrfs_super_block *disk_super,
+			   u64 devid, struct btrfs_fs_devices **fs_devices_ret);
+void device_list_fini(u8 *fsid);
 #endif
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 0/2] btrfs: coalesce of patches
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (8 preceding siblings ...)
  2013-06-10 14:56 ` [PATCH 9/9 v2] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
@ 2013-06-10 14:59 ` Anand Jain
  2013-06-10 14:59   ` [PATCH 1/2] btrfs: device delete to get errors from the kernel Anand Jain
  2013-06-10 14:59   ` [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info " Anand Jain
  2013-07-08  7:39 ` [PATCH 13/13] btrfs-progs: fix memory leaks of device_list_add() Anand Jain
                   ` (3 subsequent siblings)
  13 siblings, 2 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:59 UTC (permalink / raw)
  To: linux-btrfs


Anand Jain (2):
  btrfs: device delete to get errors from the kernel
  btrfs: add framework to read fs info and dev info from the kernel

 fs/btrfs/ioctl.c           |  22 ++++----
 fs/btrfs/super.c           | 100 +++++++++++++++++++++++++++++++++---
 fs/btrfs/volumes.c         | 125 +++++++++++++++++++++++++++++++++++++--------
 fs/btrfs/volumes.h         |   2 +
 include/uapi/linux/btrfs.h | 103 ++++++++++++++++++++++++++++++++++++-
 5 files changed, 313 insertions(+), 39 deletions(-)

-- 
1.8.1.227.g44fe835


^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 1/2] btrfs: device delete to get errors from the kernel
  2013-06-10 14:59 ` [PATCH 0/2] btrfs: coalesce of patches Anand Jain
@ 2013-06-10 14:59   ` Anand Jain
  2013-06-10 14:59   ` [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info " Anand Jain
  1 sibling, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:59 UTC (permalink / raw)
  To: linux-btrfs

when user runs command btrfs dev del the raid requisite error if any
goes to the /var/log/messages, its not good idea to clutter messages
with these user (knowledge) errors, further user don't have to review
the system messages to know problem with the cli it should be dropped
to the user as part of the cli return.

to bring this feature created a set of the ERROR defined
BTRFS_ERROR_DEV* error codes and created their error string.

I expect this enum to be added with other error which we might
want to communicate to the user land

v3:
moved the code with in the file no logical change

v1->v2:
introduce error codes for the device mgmt usage

v1:
adds a parameter in the ioctl arg struct to carry the error string

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ioctl.c           | 22 +++++++++++-----------
 fs/btrfs/volumes.c         | 26 +++++++-------------------
 include/uapi/linux/btrfs.h | 41 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 0f81d67..0e7bcc5 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2354,14 +2354,6 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)
 	if (ret)
 		return ret;
 
-	if (atomic_xchg(&root->fs_info->mutually_exclusive_operation_running,
-			1)) {
-		pr_info("btrfs: dev add/delete/balance/replace/resize operation in progress\n");
-		mnt_drop_write_file(file);
-		return -EINVAL;
-	}
-
-	mutex_lock(&root->fs_info->volume_mutex);
 	vol_args = memdup_user(arg, sizeof(*vol_args));
 	if (IS_ERR(vol_args)) {
 		ret = PTR_ERR(vol_args);
@@ -2369,12 +2361,20 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)
 	}
 
 	vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
-	ret = btrfs_rm_device(root, vol_args->name);
 
-	kfree(vol_args);
-out:
+	if (atomic_xchg(&root->fs_info->mutually_exclusive_operation_running,
+			1)) {
+		ret = BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
+		goto out;
+	}
+
+	mutex_lock(&root->fs_info->volume_mutex);
+	ret = btrfs_rm_device(root, vol_args->name);
 	mutex_unlock(&root->fs_info->volume_mutex);
 	atomic_set(&root->fs_info->mutually_exclusive_operation_running, 0);
+
+out:
+	kfree(vol_args);
 	mnt_drop_write_file(file);
 	return ret;
 }
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 2494008..024fc11 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1462,31 +1462,23 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 	btrfs_dev_replace_unlock(&root->fs_info->dev_replace);
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices <= 4) {
-		printk(KERN_ERR "btrfs: unable to go below four devices "
-		       "on raid10\n");
-		ret = -EINVAL;
+		ret = BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
 		goto out;
 	}
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices <= 2) {
-		printk(KERN_ERR "btrfs: unable to go below two "
-		       "devices on raid1\n");
-		ret = -EINVAL;
+		ret = BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET;
 		goto out;
 	}
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID5) &&
 	    root->fs_info->fs_devices->rw_devices <= 2) {
-		printk(KERN_ERR "btrfs: unable to go below two "
-		       "devices on raid5\n");
-		ret = -EINVAL;
+		ret = BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET;
 		goto out;
 	}
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID6) &&
 	    root->fs_info->fs_devices->rw_devices <= 3) {
-		printk(KERN_ERR "btrfs: unable to go below three "
-		       "devices on raid6\n");
-		ret = -EINVAL;
+		ret = BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET;
 		goto out;
 	}
 
@@ -1512,8 +1504,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 		bh = NULL;
 		disk_super = NULL;
 		if (!device) {
-			printk(KERN_ERR "btrfs: no missing devices found to "
-			       "remove\n");
+			ret = BTRFS_ERROR_DEV_MISSING_NOT_FOUND;
 			goto out;
 		}
 	} else {
@@ -1535,15 +1526,12 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 	}
 
 	if (device->is_tgtdev_for_dev_replace) {
-		pr_err("btrfs: unable to remove the dev_replace target dev\n");
-		ret = -EINVAL;
+		ret = BTRFS_ERROR_DEV_TGT_REPLACE;
 		goto error_brelse;
 	}
 
 	if (device->writeable && root->fs_info->fs_devices->rw_devices == 1) {
-		printk(KERN_ERR "btrfs: unable to remove the only writeable "
-		       "device\n");
-		ret = -EINVAL;
+		ret = BTRFS_ERROR_DEV_ONLY_WRITABLE;
 		goto error_brelse;
 	}
 
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ef0df5..de3ec34 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -447,6 +447,46 @@ struct btrfs_ioctl_send_args {
 	__u64 reserved[4];		/* in */
 };
 
+/* Error codes as returned by the kernel */
+enum btrfs_err_code {
+	notused,
+	BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_TGT_REPLACE,
+	BTRFS_ERROR_DEV_MISSING_NOT_FOUND,
+	BTRFS_ERROR_DEV_ONLY_WRITABLE,
+	BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS
+};
+/* An error code to error string mapping for the kernel
+*  error codes
+*/
+static inline char *btrfs_err_str(enum btrfs_err_code err_code)
+{
+	switch (err_code) {
+		case BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET:
+			return "unable to go below two devices on raid1";
+		case BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET:
+			return "unable to go below four devices on raid10";
+		case BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET:
+			return "unable to go below two devices on raid5";
+		case BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET:
+			return "unable to go below three devices on raid6";
+		case BTRFS_ERROR_DEV_TGT_REPLACE:
+			return "unable to remove the dev_replace target dev";
+		case BTRFS_ERROR_DEV_MISSING_NOT_FOUND:
+			return "no missing devices found to remove";
+		case BTRFS_ERROR_DEV_ONLY_WRITABLE:
+			return "unable to remove the only writeable device";
+		case BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS:
+			return "add/delete/balance/replace/resize operation "\
+				"in progress";
+		default:
+			return NULL;
+	}
+}
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -538,5 +578,4 @@ struct btrfs_ioctl_send_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
-
 #endif /* _UAPI_LINUX_BTRFS_H */
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-10 14:59 ` [PATCH 0/2] btrfs: coalesce of patches Anand Jain
  2013-06-10 14:59   ` [PATCH 1/2] btrfs: device delete to get errors from the kernel Anand Jain
@ 2013-06-10 14:59   ` Anand Jain
  2013-06-10 19:40     ` Josef Bacik
                       ` (3 more replies)
  1 sibling, 4 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-10 14:59 UTC (permalink / raw)
  To: linux-btrfs

This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
which reads the btrfs_fs_devices and btrfs_device structure
from the kernel respectively.

The information in these structure are useful to report the
device/fs information in line with the kernel operations and
thus immediately addresses the problem that 'btrfs fi show'
command reports the stale information after device device add
remove operation is performed. That is because btrfs fi show
reads the disks directly.

Further the frame-work provided here would help to enhance
the btrfs-progs/library to read the other fs information and
its device information. Also the frame work provided here is
easily extensible to retrieve any other structure as future
needs.

v1->v2:
  .code optimized
  .get the device generation number as well, so that
   btrfs-progs could print using print_one_uuid

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           | 100 +++++++++++++++++++++++++++++++++++++++++----
 fs/btrfs/volumes.c         |  99 +++++++++++++++++++++++++++++++++++++++++++-
 fs/btrfs/volumes.h         |   2 +
 include/uapi/linux/btrfs.h |  62 ++++++++++++++++++++++++++++
 4 files changed, 255 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index f0857e0..8b39a08 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1554,38 +1554,124 @@ static struct file_system_type btrfs_fs_type = {
 };
 MODULE_ALIAS_FS("btrfs");
 
+static size_t get_ioctl_sz(size_t pl_list_sz, u64 mul, u64 alloc_cnt)
+{
+	if (alloc_cnt)
+		return (sizeof(struct btrfs_ioctl_header)
+					+ pl_list_sz * alloc_cnt/mul);
+
+	return (sizeof(struct btrfs_ioctl_header) + pl_list_sz);
+}
+
+static int get_ioctl_header_and_payload(unsigned long arg,
+		size_t unit_sz, int default_len,
+		struct btrfs_ioctl_header **argp, size_t *sz)
+{
+	u64 cnt = 0;
+	struct btrfs_ioctl_header *ioctl_head;
+	size_t sz_head = sizeof(struct btrfs_ioctl_header);
+
+	*sz = get_ioctl_sz(unit_sz, default_len, cnt);
+	ioctl_head = *argp = (struct btrfs_ioctl_header *)
+				memdup_user((void __user *)arg, *sz);
+
+	if (IS_ERR(ioctl_head))
+		return PTR_ERR(ioctl_head);
+
+	cnt = ioctl_head->count;
+	if (cnt > default_len) {
+		kfree(ioctl_head);
+		/* Check for any mal-formed ioctl */
+		if (cnt % default_len)
+			return -EFAULT;
+		/* user has allocated more, so re-read
+		 * by using the recomupted size
+		*/
+		*sz = get_ioctl_sz(unit_sz, default_len, cnt);
+		ioctl_head = *argp = (struct btrfs_ioctl_header *)
+				memdup_user((void __user *)arg, *sz);
+		if (IS_ERR(ioctl_head))
+			return PTR_ERR(ioctl_head);
+	}
+
+	ioctl_head->sz_self = sz_head;
+	ioctl_head->sz = unit_sz;
+	ioctl_head->count = default_len;
+
+	if (ioctl_head->sz_self != sz_head || ioctl_head->sz != unit_sz
+		|| ioctl_head->count != default_len)
+		return 1;
+
+	return 0;
+}
+
 /*
  * used by btrfsctl to scan devices when no FS is mounted
  */
 static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 				unsigned long arg)
 {
-	struct btrfs_ioctl_vol_args *vol;
+	struct btrfs_ioctl_vol_args *vol = NULL;
 	struct btrfs_fs_devices *fs_devices;
-	int ret = -ENOTTY;
+	struct btrfs_ioctl_header *argp = NULL;
+ 	int ret = -ENOTTY;
+	size_t sz = 0;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
-	vol = memdup_user((void __user *)arg, sizeof(*vol));
-	if (IS_ERR(vol))
-		return PTR_ERR(vol);
-
 	switch (cmd) {
 	case BTRFS_IOC_SCAN_DEV:
+		vol = memdup_user((void __user *)arg, sizeof(*vol));
+		if (IS_ERR(vol))
+			return PTR_ERR(vol);
 		ret = btrfs_scan_one_device(vol->name, FMODE_READ,
 					    &btrfs_fs_type, &fs_devices);
+		kfree(vol);
 		break;
 	case BTRFS_IOC_DEVICES_READY:
+		vol = memdup_user((void __user *)arg, sizeof(*vol));
+		if (IS_ERR(vol))
+			return PTR_ERR(vol);
 		ret = btrfs_scan_one_device(vol->name, FMODE_READ,
 					    &btrfs_fs_type, &fs_devices);
+		kfree(vol);
 		if (ret)
 			break;
 		ret = !(fs_devices->num_devices == fs_devices->total_devices);
 		break;
+	case BTRFS_IOC_GET_FSIDS:
+		ret = get_ioctl_header_and_payload(arg,
+				sizeof(struct btrfs_ioctl_fs_list),
+				BTRFS_FSIDS_LEN, &argp, &sz);
+		if (ret == 1) {
+			ret = copy_to_user((void __user *)arg, argp, sz);
+			kfree(argp);
+			return -EFAULT;
+		} else if (ret)
+			return -EFAULT;
+		ret = btrfs_get_fsids(argp);
+		if (ret == 0 && copy_to_user((void __user *)arg, argp, sz))
+			ret = -EFAULT;
+		kfree(argp);
+		break;
+	case BTRFS_IOC_GET_DEVS:
+		ret = get_ioctl_header_and_payload(arg,
+				sizeof(struct btrfs_ioctl_dev_list),
+				BTRFS_DEVS_LEN, &argp, &sz);
+		if (ret == 1) {
+			ret = copy_to_user((void __user *)arg, argp, sz);
+			kfree(argp);
+			return -EFAULT;
+		} else if (ret)
+			return -EFAULT;
+		ret = btrfs_get_devs(argp);
+		if (ret == 0 && copy_to_user((void __user *)arg, argp, sz))
+			ret = -EFAULT;
+		kfree(argp);
+		break;
 	}
 
-	kfree(vol);
 	return ret;
 }
 
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 024fc11..de8399b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1819,7 +1819,7 @@ static int btrfs_prepare_sprout(struct btrfs_root *root)
 }
 
 /*
- * strore the expected generation for seed devices in device items.
+ * store the expected generation for seed devices in device items.
  */
 static int btrfs_finish_sprout(struct btrfs_trans_handle *trans,
 			       struct btrfs_root *root)
@@ -5988,3 +5988,100 @@ int btrfs_scratch_superblock(struct btrfs_device *device)
 
 	return 0;
 }
+
+int btrfs_get_fsids(struct btrfs_ioctl_header *argp)
+{
+	u64 cnt = 0;
+	struct btrfs_device *device;
+	struct btrfs_fs_devices *fs_devices;
+	struct btrfs_ioctl_fs_list *fslist;
+	struct btrfs_ioctl_fsinfo *fsinfo;
+
+	fslist = (struct btrfs_ioctl_fs_list *)((u8 *)argp
+						+ sizeof(*argp));
+
+	list_for_each_entry(fs_devices, &fs_uuids, list) {
+		if (!fslist->all && !fs_devices->opened)
+			continue;
+		if (cnt < argp->count) {
+			fsinfo = &fslist->fsinfo[cnt];
+			memcpy(fsinfo->fsid, fs_devices->fsid,
+					BTRFS_FSID_SIZE);
+			fsinfo->num_devices = fs_devices->num_devices;
+			fsinfo->missing_devices = fs_devices->missing_devices;
+			fsinfo->total_devices = fs_devices->total_devices;
+
+			if (fs_devices->opened) {
+				fsinfo->flags = BTRFS_FS_MOUNTED;
+				/* just get a devs sb */
+				device = list_entry(fs_devices->devices.next,
+						struct btrfs_device, dev_list);
+
+				fsinfo->bytes_used = device->dev_root\
+						->fs_info->super_copy->bytes_used;
+				memcpy(fsinfo->label, device->dev_root\
+						->fs_info->super_copy->label,
+						BTRFS_LABEL_SIZE);
+			}
+		}
+		cnt++;
+	}
+	argp->count = cnt;
+	return 0;
+}
+
+int btrfs_get_devs(struct btrfs_ioctl_header *argp)
+{
+	u64 cnt = 0;
+	u64 alloc_cnt = argp->count;
+	struct btrfs_device *device;
+	struct btrfs_fs_devices *fs_devices;
+	struct btrfs_ioctl_dev_list *devlist;
+	struct btrfs_ioctl_devinfo *dev;
+
+	devlist = (struct btrfs_ioctl_dev_list *)
+					((u8 *)argp + sizeof(*argp));
+
+	/* Todo: optimize. there must be better way of doing this*/
+	list_for_each_entry(fs_devices, &fs_uuids, list) {
+		if (memcmp(devlist->fsid, fs_devices->fsid, BTRFS_FSID_SIZE))
+			continue;
+
+		list_for_each_entry(device, &fs_devices->devices, dev_list) {
+			if (cnt < alloc_cnt) {
+				dev = &devlist->dev[cnt];
+				if (device->writeable)
+					dev->flags = BTRFS_DEV_WRITEABLE;
+				if (device->in_fs_metadata)
+					dev->flags = dev->flags |
+						BTRFS_DEV_IN_FS_MD;
+				if (device->missing)
+					dev->flags = dev->flags |
+						BTRFS_DEV_MISSING;
+				if (device->can_discard)
+					dev->flags = dev->flags |
+							BTRFS_DEV_CAN_DISCARD;
+				if (device->is_tgtdev_for_dev_replace)
+					dev->flags = dev->flags |
+						BTRFS_DEV_SUBSTITUTED;
+
+				dev->devid = device->devid;
+				dev->disk_total_bytes = device->disk_total_bytes;
+				dev->bytes_used = device->bytes_used;
+
+				dev->type = device->type;
+				dev->io_align = device->io_align;
+				dev->io_width = device->io_width;
+				dev->sector_size = device->sector_size;
+				memcpy(dev->uuid, device->uuid, BTRFS_UUID_SIZE);
+				memcpy(dev->name, rcu_str_deref(device->name),
+						BTRFS_PATH_NAME_MAX);
+				dev->generation = device->generation;
+			}
+			cnt++;
+		}
+		break;
+	}
+	argp->count = cnt;
+	return 0;
+}
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index f6247e2..48ebd3e 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -371,4 +371,6 @@ static inline void btrfs_dev_stat_reset(struct btrfs_device *dev,
 {
 	btrfs_dev_stat_set(dev, index, 0);
 }
+int btrfs_get_fsids(struct btrfs_ioctl_header *fsid);
+int btrfs_get_devs(struct btrfs_ioctl_header *argp);
 #endif
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index de3ec34..9d552ef 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -487,6 +487,64 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 	}
 }
 
+/* ioctl header */
+struct btrfs_ioctl_header {
+	__u64 sz_self;	/* in/out */
+	__u64 sz;	/* in/out */
+	__u64 count;	/* in/out */
+}__attribute__ ((__packed__));
+
+/* ioctl payloads */
+#define BTRFS_FSIDS_LEN	16
+#define BTRFS_FS_MOUNTED	(1LLU << 0)
+#define BTRFS_LABEL_SIZE	256
+
+struct btrfs_ioctl_fsinfo {
+	__u64 sz_self;
+	__u8 fsid[BTRFS_FSID_SIZE]; /* out */
+	__u64 num_devices;
+	__u64 total_devices;
+	__u64 missing_devices;
+	__u64 total_rw_bytes;
+	__u64 bytes_used;
+	__u64 flags;
+	__u64 default_mount_subvol_id;
+	char label[BTRFS_LABEL_SIZE];
+}__attribute__ ((__packed__));
+
+struct btrfs_ioctl_fs_list {
+	__u64 all; /* in */
+	struct btrfs_ioctl_fsinfo fsinfo[BTRFS_FSIDS_LEN]; /* out */
+}__attribute__ ((__packed__));
+
+#define BTRFS_DEVS_LEN	16
+#define BTRFS_DEV_WRITEABLE	(1LLU << 0)
+#define BTRFS_DEV_IN_FS_MD	(1LLU << 1)
+#define BTRFS_DEV_MISSING	(1LLU << 2)
+#define BTRFS_DEV_CAN_DISCARD	(1LLU << 3)
+#define BTRFS_DEV_SUBSTITUTED	(1LLU << 4)
+
+struct btrfs_ioctl_devinfo {
+	__u64 sz_self;
+	__u64 flags;
+	__u64 devid;
+	__u64 total_bytes;
+	__u64 disk_total_bytes;
+	__u64 bytes_used;
+	__u64 type;
+	__u64 generation;
+	__u32 io_align;
+	__u32 io_width;
+	__u32 sector_size;
+	__u8 uuid[BTRFS_UUID_SIZE];
+	__u8 name[BTRFS_PATH_NAME_MAX + 1];
+}__attribute__ ((__packed__));
+
+struct btrfs_ioctl_dev_list {
+	__u8 fsid[BTRFS_FSID_SIZE]; /* in */
+	struct btrfs_ioctl_devinfo dev[BTRFS_DEVS_LEN]; /* out */
+}__attribute__ ((__packed__));
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -578,4 +636,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
+#define BTRFS_IOC_GET_FSIDS _IOWR(BTRFS_IOCTL_MAGIC, 54, \
+				struct btrfs_ioctl_header)
+#define BTRFS_IOC_GET_DEVS _IOWR(BTRFS_IOCTL_MAGIC, 55, \
+				struct btrfs_ioctl_header)
 #endif /* _UAPI_LINUX_BTRFS_H */
-- 
1.8.1.227.g44fe835

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-10 14:59   ` [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info " Anand Jain
@ 2013-06-10 19:40     ` Josef Bacik
  2013-06-11 13:10       ` anand jain
  2013-06-10 20:30     ` Zach Brown
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 56+ messages in thread
From: Josef Bacik @ 2013-06-10 19:40 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Jun 10, 2013 at 08:59:15AM -0600, Anand Jain wrote:
> This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
> which reads the btrfs_fs_devices and btrfs_device structure
> from the kernel respectively.
> 
> The information in these structure are useful to report the
> device/fs information in line with the kernel operations and
> thus immediately addresses the problem that 'btrfs fi show'
> command reports the stale information after device device add
> remove operation is performed. That is because btrfs fi show
> reads the disks directly.
> 
> Further the frame-work provided here would help to enhance
> the btrfs-progs/library to read the other fs information and
> its device information. Also the frame work provided here is
> easily extensible to retrieve any other structure as future
> needs.
>

Please submit an xfstest along with this to test the new functionality so we
have something to test it with.  Make sure it runs properly if your patches are
not in place since they obviously won't be for most people.  Once you have an
xfstest I can properly test these patches.  Thanks,

Josef 

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
  2013-06-10 14:56 ` [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
@ 2013-06-10 20:00   ` Eric Sandeen
  2013-06-11 13:15     ` anand jain
  0 siblings, 1 reply; 56+ messages in thread
From: Eric Sandeen @ 2013-06-10 20:00 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On 6/10/13 9:56 AM, Anand Jain wrote:
> btrfs_scan_for_fsid uses only one argument run_ioctl out of 3
> so remove the rest two of them

and run_ioctl is only ever '1' (and it's completely unobvious
at the call point what '1' means).

Why not just go with 0 args?

Then only btrfs_scan_one_dir and btrfs_scan_block_devices
will have a non-obvious 0/1 arg.... :(

-Eric


> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  btrfs-find-root.c | 2 +-
>  disk-io.c         | 2 +-
>  utils.c           | 5 ++---
>  utils.h           | 3 +--
>  4 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/btrfs-find-root.c b/btrfs-find-root.c
> index 810d835..e736cb5 100644
> --- a/btrfs-find-root.c
> +++ b/btrfs-find-root.c
> @@ -110,7 +110,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
>  	}
>  
>  	if (total_devs != 1) {
> -		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
> +		ret = btrfs_scan_for_fsid(1);
>  		if (ret)
>  			goto out;
>  	}
> diff --git a/disk-io.c b/disk-io.c
> index 9ffe6e4..acd5480 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -838,7 +838,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
>  	}
>  
>  	if (total_devs != 1) {
> -		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
> +		ret = btrfs_scan_for_fsid(1);
>  		if (ret)
>  			goto out;
>  	}
> diff --git a/utils.c b/utils.c
> index 7b4cd74..25f3cb4 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) {
> -		if((ret = btrfs_scan_for_fsid(fs_devices_mnt, total_devs, 1)))
> +		if((ret = btrfs_scan_for_fsid(1)))
>  			return ret;
>  	}
>  
> @@ -1110,8 +1110,7 @@ fail:
>  	return ret;
>  }
>  
> -int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
> -			int run_ioctls)
> +int btrfs_scan_for_fsid(int run_ioctls)
>  {
>  	int ret;
>  
> diff --git a/utils.h b/utils.h
> index 3c17e14..dba37e8 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -35,8 +35,7 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
>  		      struct btrfs_root *root, int fd, char *path,
>  		      u64 block_count, u32 io_width, u32 io_align,
>  		      u32 sectorsize);
> -int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
> -			int run_ioctls);
> +int btrfs_scan_for_fsid(int run_ioctls);
>  void btrfs_register_one_device(char *fname);
>  int btrfs_scan_one_dir(char *dirname, int run_ioctl);
>  int check_mounted(const char *devicename);
> 


^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-10 14:59   ` [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info " Anand Jain
  2013-06-10 19:40     ` Josef Bacik
@ 2013-06-10 20:30     ` Zach Brown
  2013-06-11 14:05       ` anand jain
  2013-06-11 14:24     ` Josef Bacik
  2013-06-21  7:32     ` [PATCH 2/2 v3] btrfs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl Anand Jain
  3 siblings, 1 reply; 56+ messages in thread
From: Zach Brown @ 2013-06-10 20:30 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Jun 10, 2013 at 10:59:15PM +0800, Anand Jain wrote:
> This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
> which reads the btrfs_fs_devices and btrfs_device structure
> from the kernel respectively.

Why not just use sysfs to export the device lists?

> The information in these structure are useful to report the
> device/fs information in line with the kernel operations and
> thus immediately addresses the problem that 'btrfs fi show'
> command reports the stale information after device device add
> remove operation is performed. That is because btrfs fi show
> reads the disks directly.

Hmm.  Would it be enough to get the block device address_space in sync
with the btrfs stuctures?  Would that get rid of the need for this
interface?

But sure, for the sake of argument and code review, let's say that we
wanted some ioctls to read the btrfs kernel device lists.

> Further the frame-work provided here would help to enhance

Please don't add a layer of generic encoding above these new ioctls.
It's not necessary and it makes more work for the various parts of the
world that want to do deep ioctl inspection (think, for example, of
strace).

> +static size_t get_ioctl_sz(size_t pl_list_sz, u64 mul, u64 alloc_cnt)
> +
> +static int get_ioctl_header_and_payload(unsigned long arg,
> +		size_t unit_sz, int default_len,
> +		struct btrfs_ioctl_header **argp, size_t *sz)

This adds a lot of fiddly math and allocation that can go wrong for no
benefit.  We can restructure the interface so all of this code goes
away.

1) Have a simple single fixed input structure for each ioctl.  Maybe
with some extra padding and a flags argument if you think stuff is going
to be added over time.  No generic header.  No casting.  The ioctl
number defines the input structure.  If you need a different structure
later, use a different ioctl number.

2) Don't have a fixed array of output structs embedded in the input
struct.  Have a pointer to the array of output structs in the input
struct.  Probably with a number of elements found there.

3) Don't copy the giant user output buffer into the kernel, write to it,
then copy it back to user space.  Instead have the kernel copy_to_user()
each output structure to the userspace pointer as it goes.  Have the
ioctl() return a positive count of the number of elements copied.

>  static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>  				unsigned long arg)
>  {
> -	struct btrfs_ioctl_vol_args *vol;
> +	struct btrfs_ioctl_vol_args *vol = NULL;
>  	struct btrfs_fs_devices *fs_devices;
> -	int ret = -ENOTTY;
> +	struct btrfs_ioctl_header *argp = NULL;
> + 	int ret = -ENOTTY;
> +	size_t sz = 0;
>  
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
> -	vol = memdup_user((void __user *)arg, sizeof(*vol));
> -	if (IS_ERR(vol))
> -		return PTR_ERR(vol);
> -
>  	switch (cmd) {
>  	case BTRFS_IOC_SCAN_DEV:
> +		vol = memdup_user((void __user *)arg, sizeof(*vol));
> +		if (IS_ERR(vol))
> +			return PTR_ERR(vol);

Hmm, having to duplicate that previously shared setup into each ioctl
block is a sign that the layering is wrong.

Don't smush the new ioctls into case bodies of this existing simple
fuction that worked with the _vol_args ioctls.  Rename this
btrfs_ioctl_vol_args, or something, and call it from a tiny new
btrfs_control_ioctl() for its two ioctls.  That new high level btrfs
ioctl dispatch function can then call the two new _get_devs and
_get_fsids functions.

> +	case BTRFS_IOC_GET_FSIDS:
> +		ret = get_ioctl_header_and_payload(arg,
> +				sizeof(struct btrfs_ioctl_fs_list),
> +				BTRFS_FSIDS_LEN, &argp, &sz);
> +		if (ret == 1) {
> +			ret = copy_to_user((void __user *)arg, argp, sz);
> +			kfree(argp);
> +			return -EFAULT;
> +		} else if (ret)
> +			return -EFAULT;
> +		ret = btrfs_get_fsids(argp);
> +		if (ret == 0 && copy_to_user((void __user *)arg, argp, sz))
> +			ret = -EFAULT;
> +		kfree(argp);
> +		break;

All of this can go away if you have trivial input and array-of-output
args that the ioctl helper works with.

	case BTRFS_IOC_GET_FSIDS:
		ret = btrfs_ioctl_get_fsids(arg);
		break;

> +int btrfs_get_fsids(struct btrfs_ioctl_header *argp)
> +{
> +	u64 cnt = 0;
> +	struct btrfs_device *device;
> +	struct btrfs_fs_devices *fs_devices;
> +	struct btrfs_ioctl_fs_list *fslist;
> +	struct btrfs_ioctl_fsinfo *fsinfo;
> +
> +	fslist = (struct btrfs_ioctl_fs_list *)((u8 *)argp
> +						+ sizeof(*argp));

Instead of working with an allocated copy of the input, copy the user
input struct onto the stack here.

	struct btrfs_ioctl_get_fsids_input in;

	if (copy_from_user(&in, argp, sizeof(in)))
		return -EFAULT;

(Or you could get_user() each field.. but that's probably not worth it
for a small input struct.)

> +
> +	list_for_each_entry(fs_devices, &fs_uuids, list) {

Surely this needs to be protected by some lock?  The uuid_mutex, maybe?

> +			fsinfo = &fslist->fsinfo[cnt];
> +			memcpy(fsinfo->fsid, fs_devices->fsid,
> +					BTRFS_FSID_SIZE);

And then copy each device's output struct back to userspace one at a
time instead of building them up in the giant allocated kernel buffer.

This might get a little hairy if you don't want to block under the
uuid_mutex, but that's solvable too (dummy cursor items on the list,
probably).

> +				fsinfo->bytes_used = device->dev_root\
> +						->fs_info->super_copy->bytes_used;

A line continuation in the middle of a pointer deref?  Cache the
super_copy pointer on the stack, maybe.  It's probably better to use a
function that copies a single device's output struct to get all those
indentation levels back.

> +	/* Todo: optimize. there must be better way of doing this*/
> +	list_for_each_entry(fs_devices, &fs_uuids, list) {

(same locking question)

> +				if (device->in_fs_metadata)
> +					dev->flags = dev->flags |
> +						BTRFS_DEV_IN_FS_MD;

	'a = a | b' -> 'a |= b'

> +				memcpy(dev->name, rcu_str_deref(device->name),
> +						BTRFS_PATH_NAME_MAX);

Hmm.  Don't we need to be in rcu_read_lock() to use rcu_str_deref()?

> +struct btrfs_ioctl_fsinfo {
> +	__u64 sz_self;
> +	__u8 fsid[BTRFS_FSID_SIZE]; /* out */
> +	__u64 num_devices;
> +	__u64 total_devices;
> +	__u64 missing_devices;
> +	__u64 total_rw_bytes;
> +	__u64 bytes_used;
> +	__u64 flags;
> +	__u64 default_mount_subvol_id;
> +	char label[BTRFS_LABEL_SIZE];
> +}__attribute__ ((__packed__));

It'd be __packed, but I'd naturally align the structs to u64s and not
use it.

> +struct btrfs_ioctl_fs_list {
> +	__u64 all; /* in */
> +	struct btrfs_ioctl_fsinfo fsinfo[BTRFS_FSIDS_LEN]; /* out */
> +}__attribute__ ((__packed__));

I'd turn this into something like:

struct btrfs_ioctl_get_fsinfo_input {
	__u64 flags;  /* bit for all */
	__u64 nr_fsinfo;
	__u64 fsinfo_ptr; /* u64 ptr to avoid compat */
};

> +struct btrfs_ioctl_devinfo {
> +	__u64 sz_self;
> +	__u64 flags;
> +	__u64 devid;
> +	__u64 total_bytes;
> +	__u64 disk_total_bytes;
> +	__u64 bytes_used;
> +	__u64 type;
> +	__u64 generation;
> +	__u32 io_align;
> +	__u32 io_width;
> +	__u32 sector_size;
 __u32 _padding;
> +	__u8 uuid[BTRFS_UUID_SIZE];
> +	__u8 name[BTRFS_PATH_NAME_MAX + 1];

Pad that nutty 4088 byte string to a multiple of u64s and you can
remove the packed attribute.

- z

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-10 19:40     ` Josef Bacik
@ 2013-06-11 13:10       ` anand jain
  2013-06-11 13:15         ` Josef Bacik
  0 siblings, 1 reply; 56+ messages in thread
From: anand jain @ 2013-06-11 13:10 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs



On 06/11/2013 03:40 AM, Josef Bacik wrote:
> On Mon, Jun 10, 2013 at 08:59:15AM -0600, Anand Jain wrote:
>> This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
>> which reads the btrfs_fs_devices and btrfs_device structure
>> from the kernel respectively.
>>
>> The information in these structure are useful to report the
>> device/fs information in line with the kernel operations and
>> thus immediately addresses the problem that 'btrfs fi show'
>> command reports the stale information after device device add
>> remove operation is performed. That is because btrfs fi show
>> reads the disks directly.
>>
>> Further the frame-work provided here would help to enhance
>> the btrfs-progs/library to read the other fs information and
>> its device information. Also the frame work provided here is
>> easily extensible to retrieve any other structure as future
>> needs.
>>
>
> Please submit an xfstest along with this to test the new functionality so we
> have something to test it with.  Make sure it runs properly if your patches are
> not in place since they obviously won't be for most people.  Once you have an
> xfstest I can properly test these patches.  Thanks,

  This kernel patch supports a new cli option --kernel
  under the existing command 'btrfs filesystem show'.
  xfstest first of all would need testcase to test the
  btrfs filesystem show which isn't there yet. Doing it
  here deviate too much from the point here. But let
  me try to quick get that.

Thanks, Anand

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-11 13:10       ` anand jain
@ 2013-06-11 13:15         ` Josef Bacik
  0 siblings, 0 replies; 56+ messages in thread
From: Josef Bacik @ 2013-06-11 13:15 UTC (permalink / raw)
  To: anand jain; +Cc: Josef Bacik, linux-btrfs

On Tue, Jun 11, 2013 at 07:10:12AM -0600, anand jain wrote:
> 
> 
> On 06/11/2013 03:40 AM, Josef Bacik wrote:
> > On Mon, Jun 10, 2013 at 08:59:15AM -0600, Anand Jain wrote:
> >> This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
> >> which reads the btrfs_fs_devices and btrfs_device structure
> >> from the kernel respectively.
> >>
> >> The information in these structure are useful to report the
> >> device/fs information in line with the kernel operations and
> >> thus immediately addresses the problem that 'btrfs fi show'
> >> command reports the stale information after device device add
> >> remove operation is performed. That is because btrfs fi show
> >> reads the disks directly.
> >>
> >> Further the frame-work provided here would help to enhance
> >> the btrfs-progs/library to read the other fs information and
> >> its device information. Also the frame work provided here is
> >> easily extensible to retrieve any other structure as future
> >> needs.
> >>
> >
> > Please submit an xfstest along with this to test the new functionality so we
> > have something to test it with.  Make sure it runs properly if your patches are
> > not in place since they obviously won't be for most people.  Once you have an
> > xfstest I can properly test these patches.  Thanks,
> 
>   This kernel patch supports a new cli option --kernel
>   under the existing command 'btrfs filesystem show'.
>   xfstest first of all would need testcase to test the
>   btrfs filesystem show which isn't there yet. Doing it
>   here deviate too much from the point here. But let
>   me try to quick get that.
> 

No you just need a testcase to make sure --kernel is working properly, so make
it use SCRATCH_DEV_POOL and do things like make a file system, remove a device,
make sure its no longer there, add it back and make sure it pops up again, that
sort of thing.  We are trying to make btrfs more stable and I have no interest
in taking in new features/ioctls without a way to test them properly, so it is
not beside the point.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
  2013-06-10 20:00   ` Eric Sandeen
@ 2013-06-11 13:15     ` anand jain
  0 siblings, 0 replies; 56+ messages in thread
From: anand jain @ 2013-06-11 13:15 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-btrfs




On 06/11/2013 04:00 AM, Eric Sandeen wrote:
> On 6/10/13 9:56 AM, Anand Jain wrote:
>> btrfs_scan_for_fsid uses only one argument run_ioctl out of 3
>> so remove the rest two of them
>
> and run_ioctl is only ever '1' (and it's completely unobvious
> at the call point what '1' means).

There was this below patch for it. It was defined
as BTRFS_SCAN_REGISTER here.

   subject: v6: access to backup superblock

I need to rebase this patch though.

> Why not just go with 0 args?

> Then only btrfs_scan_one_dir and btrfs_scan_block_devices
> will have a non-obvious 0/1 arg.... :(
>
> -Eric

btrfs_scan_for_fsid as such needs a revamp so that
it will use kernel-probes if device is mounted, so
as of now I would like to keep it as it is.

Thanks for the comments.

Anand


^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-10 20:30     ` Zach Brown
@ 2013-06-11 14:05       ` anand jain
  2013-06-11 17:50         ` Zach Brown
  0 siblings, 1 reply; 56+ messages in thread
From: anand jain @ 2013-06-11 14:05 UTC (permalink / raw)
  To: Zach Brown; +Cc: linux-btrfs



On 06/11/2013 04:30 AM, Zach Brown wrote:
> On Mon, Jun 10, 2013 at 10:59:15PM +0800, Anand Jain wrote:
>> This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
>> which reads the btrfs_fs_devices and btrfs_device structure
>> from the kernel respectively.
>
> Why not just use sysfs to export the device lists?

  Hmm. I see sysfs being used but isn't ioctl more easy
  to get stuff out from the kernel (except for dumping
  address_space) ? Providing a complete sysfs interface
  for btrfs can be a RFC as well. For now ioctl will help
  to fix the bugs.

>> The information in these structure are useful to report the
>> device/fs information in line with the kernel operations and
>> thus immediately addresses the problem that 'btrfs fi show'
>> command reports the stale information after device device add
>> remove operation is performed. That is because btrfs fi show
>> reads the disks directly.
>
> Hmm.  Would it be enough to get the block device address_space in sync
> with the btrfs stuctures?  Would that get rid of the need for this
> interface?

  Absolutely ! I have that to experiment in linux for
  a long time. More to come on that as time permits from
  the bug fixes which is kind of urgent need as of now.

> But sure, for the sake of argument and code review, let's say that we
> wanted some ioctls to read the btrfs kernel device lists.

  thanks.

>> Further the frame-work provided here would help to enhance
>
> Please don't add a layer of generic encoding above these new ioctls.
> It's not necessary and it makes more work for the various parts of the
> world that want to do deep ioctl inspection (think, for example, of
> strace).

   Agreed. Debugging has to be easier.

>> +static size_t get_ioctl_sz(size_t pl_list_sz, u64 mul, u64 alloc_cnt)
>> +
>> +static int get_ioctl_header_and_payload(unsigned long arg,
>> +		size_t unit_sz, int default_len,
>> +		struct btrfs_ioctl_header **argp, size_t *sz)
>
> This adds a lot of fiddly math and allocation that can go wrong for no
> benefit.  We can restructure the interface so all of this code goes
> away.
>
> 1) Have a simple single fixed input structure for each ioctl.  Maybe
> with some extra padding and a flags argument if you think stuff is going
> to be added over time.  No generic header.  No casting.  The ioctl
> number defines the input structure.  If you need a different structure
> later, use a different ioctl number.

  -No generic header and No casting-  Why not ? anything that
  might not work with strace ?

  (Header-payload model are typically favorites of IO protocol drivers.
  they are easily extensible, checking for compatibility is easier, and
  code re-useability is great).

  Thanks for the review comments below. I will look into that.

Anand
-------
> 2) Don't have a fixed array of output structs embedded in the input
> struct.  Have a pointer to the array of output structs in the input
> struct.  Probably with a number of elements found there.



> 3) Don't copy the giant user output buffer into the kernel, write to it,
> then copy it back to user space.  Instead have the kernel copy_to_user()
> each output structure to the userspace pointer as it goes.  Have the
> ioctl() return a positive count of the number of elements copied.



>>   static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>>   				unsigned long arg)
>>   {
>> -	struct btrfs_ioctl_vol_args *vol;
>> +	struct btrfs_ioctl_vol_args *vol = NULL;
>>   	struct btrfs_fs_devices *fs_devices;
>> -	int ret = -ENOTTY;
>> +	struct btrfs_ioctl_header *argp = NULL;
>> + 	int ret = -ENOTTY;
>> +	size_t sz = 0;
>>
>>   	if (!capable(CAP_SYS_ADMIN))
>>   		return -EPERM;
>>
>> -	vol = memdup_user((void __user *)arg, sizeof(*vol));
>> -	if (IS_ERR(vol))
>> -		return PTR_ERR(vol);
>> -
>>   	switch (cmd) {
>>   	case BTRFS_IOC_SCAN_DEV:
>> +		vol = memdup_user((void __user *)arg, sizeof(*vol));
>> +		if (IS_ERR(vol))
>> +			return PTR_ERR(vol);
>
> Hmm, having to duplicate that previously shared setup into each ioctl
> block is a sign that the layering is wrong.
>
> Don't smush the new ioctls into case bodies of this existing simple
> fuction that worked with the _vol_args ioctls.  Rename this
> btrfs_ioctl_vol_args, or something, and call it from a tiny new
> btrfs_control_ioctl() for its two ioctls.  That new high level btrfs
> ioctl dispatch function can then call the two new _get_devs and
> _get_fsids functions.



>> +	case BTRFS_IOC_GET_FSIDS:
>> +		ret = get_ioctl_header_and_payload(arg,
>> +				sizeof(struct btrfs_ioctl_fs_list),
>> +				BTRFS_FSIDS_LEN, &argp, &sz);
>> +		if (ret == 1) {
>> +			ret = copy_to_user((void __user *)arg, argp, sz);
>> +			kfree(argp);
>> +			return -EFAULT;
>> +		} else if (ret)
>> +			return -EFAULT;
>> +		ret = btrfs_get_fsids(argp);
>> +		if (ret == 0 && copy_to_user((void __user *)arg, argp, sz))
>> +			ret = -EFAULT;
>> +		kfree(argp);
>> +		break;
>
> All of this can go away if you have trivial input and array-of-output
> args that the ioctl helper works with.
>
> 	case BTRFS_IOC_GET_FSIDS:
> 		ret = btrfs_ioctl_get_fsids(arg);
> 		break;


>> +int btrfs_get_fsids(struct btrfs_ioctl_header *argp)
>> +{
>> +	u64 cnt = 0;
>> +	struct btrfs_device *device;
>> +	struct btrfs_fs_devices *fs_devices;
>> +	struct btrfs_ioctl_fs_list *fslist;
>> +	struct btrfs_ioctl_fsinfo *fsinfo;
>> +
>> +	fslist = (struct btrfs_ioctl_fs_list *)((u8 *)argp
>> +						+ sizeof(*argp));
>
> Instead of working with an allocated copy of the input, copy the user
> input struct onto the stack here.
>
> 	struct btrfs_ioctl_get_fsids_input in;
>
> 	if (copy_from_user(&in, argp, sizeof(in)))
> 		return -EFAULT;
>
> (Or you could get_user() each field.. but that's probably not worth it
> for a small input struct.)



>> +
>> +	list_for_each_entry(fs_devices, &fs_uuids, list) {
>
> Surely this needs to be protected by some lock?  The uuid_mutex, maybe?



>> +			fsinfo = &fslist->fsinfo[cnt];
>> +			memcpy(fsinfo->fsid, fs_devices->fsid,
>> +					BTRFS_FSID_SIZE);
>
> And then copy each device's output struct back to userspace one at a
> time instead of building them up in the giant allocated kernel buffer.
>
> This might get a little hairy if you don't want to block under the
> uuid_mutex, but that's solvable too (dummy cursor items on the list,
> probably).



>> +				fsinfo->bytes_used = device->dev_root\
>> +						->fs_info->super_copy->bytes_used;
>
> A line continuation in the middle of a pointer deref?  Cache the
> super_copy pointer on the stack, maybe.  It's probably better to use a
> function that copies a single device's output struct to get all those
> indentation levels back.
>
>> +	/* Todo: optimize. there must be better way of doing this*/
>> +	list_for_each_entry(fs_devices, &fs_uuids, list) {
>
> (same locking question)



>> +				if (device->in_fs_metadata)
>> +					dev->flags = dev->flags |
>> +						BTRFS_DEV_IN_FS_MD;
>
> 	'a = a | b' -> 'a |= b'



>> +				memcpy(dev->name, rcu_str_deref(device->name),
>> +						BTRFS_PATH_NAME_MAX);
>
> Hmm.  Don't we need to be in rcu_read_lock() to use rcu_str_deref()?



>> +struct btrfs_ioctl_fsinfo {
>> +	__u64 sz_self;
>> +	__u8 fsid[BTRFS_FSID_SIZE]; /* out */
>> +	__u64 num_devices;
>> +	__u64 total_devices;
>> +	__u64 missing_devices;
>> +	__u64 total_rw_bytes;
>> +	__u64 bytes_used;
>> +	__u64 flags;
>> +	__u64 default_mount_subvol_id;
>> +	char label[BTRFS_LABEL_SIZE];
>> +}__attribute__ ((__packed__));
>
> It'd be __packed, but I'd naturally align the structs to u64s and not
> use it.



>> +struct btrfs_ioctl_fs_list {
>> +	__u64 all; /* in */
>> +	struct btrfs_ioctl_fsinfo fsinfo[BTRFS_FSIDS_LEN]; /* out */
>> +}__attribute__ ((__packed__));
>
> I'd turn this into something like:
>
> struct btrfs_ioctl_get_fsinfo_input {
> 	__u64 flags;  /* bit for all */
> 	__u64 nr_fsinfo;
> 	__u64 fsinfo_ptr; /* u64 ptr to avoid compat */
> };



>> +struct btrfs_ioctl_devinfo {
>> +	__u64 sz_self;
>> +	__u64 flags;
>> +	__u64 devid;
>> +	__u64 total_bytes;
>> +	__u64 disk_total_bytes;
>> +	__u64 bytes_used;
>> +	__u64 type;
>> +	__u64 generation;
>> +	__u32 io_align;
>> +	__u32 io_width;
>> +	__u32 sector_size;
>   __u32 _padding;
>> +	__u8 uuid[BTRFS_UUID_SIZE];
>> +	__u8 name[BTRFS_PATH_NAME_MAX + 1];
>
> Pad that nutty 4088 byte string to a multiple of u64s and you can
> remove the packed attribute.




> - z
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-10 14:59   ` [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info " Anand Jain
  2013-06-10 19:40     ` Josef Bacik
  2013-06-10 20:30     ` Zach Brown
@ 2013-06-11 14:24     ` Josef Bacik
  2013-06-21  7:02       ` Anand Jain
  2013-06-21  7:32     ` [PATCH 2/2 v3] btrfs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl Anand Jain
  3 siblings, 1 reply; 56+ messages in thread
From: Josef Bacik @ 2013-06-11 14:24 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Mon, Jun 10, 2013 at 08:59:15AM -0600, Anand Jain wrote:
> This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
> which reads the btrfs_fs_devices and btrfs_device structure
> from the kernel respectively.
> 
> The information in these structure are useful to report the
> device/fs information in line with the kernel operations and
> thus immediately addresses the problem that 'btrfs fi show'
> command reports the stale information after device device add
> remove operation is performed. That is because btrfs fi show
> reads the disks directly.
> 
> Further the frame-work provided here would help to enhance
> the btrfs-progs/library to read the other fs information and
> its device information. Also the frame work provided here is
> easily extensible to retrieve any other structure as future
> needs.
> 
> v1->v2:
>   .code optimized
>   .get the device generation number as well, so that
>    btrfs-progs could print using print_one_uuid
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

In fact NACK altogether on this patch, you can get the same info out with the
TREE_SEARCH ioctl, just do that in btrfs-progs and don't add yet another ioctl.
Thanks,

Josef

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-11 14:05       ` anand jain
@ 2013-06-11 17:50         ` Zach Brown
  0 siblings, 0 replies; 56+ messages in thread
From: Zach Brown @ 2013-06-11 17:50 UTC (permalink / raw)
  To: anand jain; +Cc: linux-btrfs

> >1) Have a simple single fixed input structure for each ioctl.  Maybe
> >with some extra padding and a flags argument if you think stuff is going
> >to be added over time.  No generic header.  No casting.  The ioctl
> >number defines the input structure.  If you need a different structure
> >later, use a different ioctl number.
> 
>  -No generic header and No casting-  Why not ?

Because it brings no benefits.  All the supposed benefits can be achived
through careful use of the existing interfaces, ioctl or otherwise.  And
it has costs: code complexity, run-time cpu/memory, etc.  Its
benefit/cost ratio is not flattering :).

- z

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info from the kernel
  2013-06-11 14:24     ` Josef Bacik
@ 2013-06-21  7:02       ` Anand Jain
  0 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-21  7:02 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs


  TREE_SEARCH ioctl doesn't help in this case
  the dev item don't provide all the info
  that btrfs-progs might need in the long run
  (and even the current needs).

Thanks, Anand

On 06/11/2013 10:24 PM, Josef Bacik wrote:
> On Mon, Jun 10, 2013 at 08:59:15AM -0600, Anand Jain wrote:
>> This adds two ioctl BTRFS_IOC_GET_FSIDS and BTRFS_IOC_GET_DEVS
>> which reads the btrfs_fs_devices and btrfs_device structure
>> from the kernel respectively.
>>
>> The information in these structure are useful to report the
>> device/fs information in line with the kernel operations and
>> thus immediately addresses the problem that 'btrfs fi show'
>> command reports the stale information after device device add
>> remove operation is performed. That is because btrfs fi show
>> reads the disks directly.
>>
>> Further the frame-work provided here would help to enhance
>> the btrfs-progs/library to read the other fs information and
>> its device information. Also the frame work provided here is
>> easily extensible to retrieve any other structure as future
>> needs.
>>
>> v1->v2:
>>    .code optimized
>>    .get the device generation number as well, so that
>>     btrfs-progs could print using print_one_uuid
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>
> In fact NACK altogether on this patch, you can get the same info out with the
> TREE_SEARCH ioctl, just do that in btrfs-progs and don't add yet another ioctl.
> Thanks,
>
> Josef

^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 2/2 v3] btrfs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl
  2013-06-10 14:59   ` [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info " Anand Jain
                       ` (2 preceding siblings ...)
  2013-06-11 14:24     ` Josef Bacik
@ 2013-06-21  7:32     ` Anand Jain
  2013-06-24 17:03       ` Josef Bacik
  3 siblings, 1 reply; 56+ messages in thread
From: Anand Jain @ 2013-06-21  7:32 UTC (permalink / raw)
  To: linux-btrfs

btrfs-progs has to read fs info from the kernel to
read the latest info instead of reading it from the disks,
which generally is a stale info after certain critical
operation.

getting used_bytes parameter will help to fix
	btrfs filesystem show --kernel
to show the current info of the fs

v2->v3:
	everything is changed dropped the plan to introduce
	the new ioctl, as of now filesystem show could 
	manage if we add the used_bytes parameter to the
	BTRFS_IOC_FS_INFO and
	Update the title from
	 add framework to read fs info and dev info from the kernel

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ioctl.c           | 16 ++++++++++++++--
 include/uapi/linux/btrfs.h |  3 ++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 0e7bcc5..082c9fc 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2384,7 +2384,10 @@ static long btrfs_ioctl_fs_info(struct btrfs_root *root, void __user *arg)
 	struct btrfs_ioctl_fs_info_args *fi_args;
 	struct btrfs_device *device;
 	struct btrfs_device *next;
-	struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices;
+	struct btrfs_fs_info *info = root->fs_info;
+	struct btrfs_fs_devices *fs_devices = info->fs_devices;
+	struct btrfs_space_info *si;
+	u64 used_bytes = 0;
 	int ret = 0;
 
 	if (!capable(CAP_SYS_ADMIN))
@@ -2394,8 +2397,17 @@ static long btrfs_ioctl_fs_info(struct btrfs_root *root, void __user *arg)
 	if (!fi_args)
 		return -ENOMEM;
 
+	rcu_read_lock();
+	list_for_each_entry_rcu(si, &info->space_info, list) {
+		used_bytes += si->bytes_used + si->bytes_reserved
+				+ si->bytes_pinned + si->bytes_readonly
+				+ si->bytes_may_use;
+	}
+	rcu_read_unlock();
+	fi_args->used_bytes = used_bytes;
+
 	fi_args->num_devices = fs_devices->num_devices;
-	memcpy(&fi_args->fsid, root->fs_info->fsid, sizeof(fi_args->fsid));
+	memcpy(&fi_args->fsid, info->fsid, sizeof(fi_args->fsid));
 
 	mutex_lock(&fs_devices->device_list_mutex);
 	list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) {
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index de3ec34..44bac0c 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -181,7 +181,8 @@ struct btrfs_ioctl_fs_info_args {
 	__u64 max_id;				/* out */
 	__u64 num_devices;			/* out */
 	__u8 fsid[BTRFS_FSID_SIZE];		/* out */
-	__u64 reserved[124];			/* pad to 1k */
+	__u64 used_bytes;                       /* out */
+	__u64 reserved[123];			/* pad to 1k */
 };
 
 /* balance control ioctl modes */
-- 
1.8.1.227.g44fe835


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 08/13 v2] btrfs-progs: get_label_mounted to return label instead of print
  2013-06-10 14:56 ` [PATCH 8/9] btrfs-progs: get_label_mounted to return label instead of print Anand Jain
@ 2013-06-21  7:41   ` Anand Jain
  0 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-21  7:41 UTC (permalink / raw)
  To: linux-btrfs

This would help to reuse the function

v1->v2: removed static and updated .h file, in a preparation
        work for the following patch

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 18 +++++++++++++-----
 utils.h |  1 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/utils.c b/utils.c
index f9545b8..fc3c41e 100644
--- a/utils.c
+++ b/utils.c
@@ -1313,7 +1313,7 @@ static int get_label_unmounted(const char *dev)
  * mounted path rather than device.  Return the corresponding error
  * the user specified the device path.
  */
-static int get_label_mounted(const char *mount_path)
+int get_label_mounted(const char *mount_path, char *labelp)
 {
 	char label[BTRFS_LABEL_SIZE];
 	int fd;
@@ -1331,16 +1331,24 @@ static int get_label_mounted(const char *mount_path)
 		return -1;
 	}
 
-	fprintf(stdout, "%s\n", label);
+	strncpy(labelp, label, sizeof(label));
 	close(fd);
 	return 0;
 }
 
 int get_label(const char *btrfs_dev)
 {
-	return is_existing_blk_or_reg_file(btrfs_dev) ?
-		get_label_unmounted(btrfs_dev) :
-		get_label_mounted(btrfs_dev);
+	int ret;
+	char label[BTRFS_LABEL_SIZE];
+
+	if (is_existing_blk_or_reg_file(btrfs_dev))
+		ret = get_label_unmounted(btrfs_dev);
+	else {
+		ret = get_label_mounted(btrfs_dev, label);
+		if (!ret)
+			fprintf(stdout, "%s\n", label);
+	}
+	return ret;
 }
 
 int set_label(const char *btrfs_dev, const char *label)
diff --git a/utils.h b/utils.h
index 733d13b..1fa1c5a 100644
--- a/utils.h
+++ b/utils.h
@@ -69,4 +69,5 @@ u64 btrfs_device_size(int fd, struct stat *st);
 #define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest))
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
 int scan_for_btrfs(int where, int update_kernel);
+int get_label_mounted(const char *mount_path, char *labelp);
 #endif
-- 
1.8.1.227.g44fe835


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v3] btrfs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl
  2013-06-21  7:32     ` [PATCH 2/2 v3] btrfs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl Anand Jain
@ 2013-06-24 17:03       ` Josef Bacik
  2013-06-25  3:00         ` Anand Jain
  0 siblings, 1 reply; 56+ messages in thread
From: Josef Bacik @ 2013-06-24 17:03 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Fri, Jun 21, 2013 at 03:32:28PM +0800, Anand Jain wrote:
> btrfs-progs has to read fs info from the kernel to
> read the latest info instead of reading it from the disks,
> which generally is a stale info after certain critical
> operation.
> 
> getting used_bytes parameter will help to fix
> 	btrfs filesystem show --kernel
> to show the current info of the fs
> 
> v2->v3:
> 	everything is changed dropped the plan to introduce
> 	the new ioctl, as of now filesystem show could 
> 	manage if we add the used_bytes parameter to the
> 	BTRFS_IOC_FS_INFO and
> 	Update the title from
> 	 add framework to read fs info and dev info from the kernel
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/ioctl.c           | 16 ++++++++++++++--
>  include/uapi/linux/btrfs.h |  3 ++-
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 0e7bcc5..082c9fc 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2384,7 +2384,10 @@ static long btrfs_ioctl_fs_info(struct btrfs_root *root, void __user *arg)
>  	struct btrfs_ioctl_fs_info_args *fi_args;
>  	struct btrfs_device *device;
>  	struct btrfs_device *next;
> -	struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices;
> +	struct btrfs_fs_info *info = root->fs_info;
> +	struct btrfs_fs_devices *fs_devices = info->fs_devices;
> +	struct btrfs_space_info *si;
> +	u64 used_bytes = 0;
>  	int ret = 0;
>  
>  	if (!capable(CAP_SYS_ADMIN))
> @@ -2394,8 +2397,17 @@ static long btrfs_ioctl_fs_info(struct btrfs_root *root, void __user *arg)
>  	if (!fi_args)
>  		return -ENOMEM;
>  
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(si, &info->space_info, list) {
> +		used_bytes += si->bytes_used + si->bytes_reserved
> +				+ si->bytes_pinned + si->bytes_readonly
> +				+ si->bytes_may_use;
> +	}
> +	rcu_read_unlock();
> +	fi_args->used_bytes = used_bytes;
> +

So what exactly are we using this number for?  ->bytes_may_used is going to
change all the damned time because of reservations and it could even make
used_bytes to appear to be larger than total_bytes because of overcommitting.
So why exactly do you want this value?  Also how are you going to deal with
older kernels where this isn't populated?  Thanks,

Josef

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2 v3] btrfs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl
  2013-06-24 17:03       ` Josef Bacik
@ 2013-06-25  3:00         ` Anand Jain
  0 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-06-25  3:00 UTC (permalink / raw)
  To: Josef Bacik; +Cc: linux-btrfs



On 06/25/2013 01:03 AM, Josef Bacik wrote:
> On Fri, Jun 21, 2013 at 03:32:28PM +0800, Anand Jain wrote:
>> btrfs-progs has to read fs info from the kernel to
>> read the latest info instead of reading it from the disks,
>> which generally is a stale info after certain critical
>> operation.
>>
>> getting used_bytes parameter will help to fix
>> 	btrfs filesystem show --kernel
>> to show the current info of the fs
>>
>> v2->v3:
>> 	everything is changed dropped the plan to introduce
>> 	the new ioctl, as of now filesystem show could
>> 	manage if we add the used_bytes parameter to the
>> 	BTRFS_IOC_FS_INFO and
>> 	Update the title from
>> 	 add framework to read fs info and dev info from the kernel
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/ioctl.c           | 16 ++++++++++++++--
>>   include/uapi/linux/btrfs.h |  3 ++-
>>   2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index 0e7bcc5..082c9fc 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -2384,7 +2384,10 @@ static long btrfs_ioctl_fs_info(struct btrfs_root *root, void __user *arg)
>>   	struct btrfs_ioctl_fs_info_args *fi_args;
>>   	struct btrfs_device *device;
>>   	struct btrfs_device *next;
>> -	struct btrfs_fs_devices *fs_devices = root->fs_info->fs_devices;
>> +	struct btrfs_fs_info *info = root->fs_info;
>> +	struct btrfs_fs_devices *fs_devices = info->fs_devices;
>> +	struct btrfs_space_info *si;
>> +	u64 used_bytes = 0;
>>   	int ret = 0;
>>
>>   	if (!capable(CAP_SYS_ADMIN))
>> @@ -2394,8 +2397,17 @@ static long btrfs_ioctl_fs_info(struct btrfs_root *root, void __user *arg)
>>   	if (!fi_args)
>>   		return -ENOMEM;
>>
>> +	rcu_read_lock();
>> +	list_for_each_entry_rcu(si, &info->space_info, list) {
>> +		used_bytes += si->bytes_used + si->bytes_reserved
>> +				+ si->bytes_pinned + si->bytes_readonly
>> +				+ si->bytes_may_use;
>> +	}
>> +	rcu_read_unlock();
>> +	fi_args->used_bytes = used_bytes;
>> +
>
> So what exactly are we using this number for?  ->bytes_may_used is going to
> change all the damned time because of reservations and it could even make
> used_bytes to appear to be larger than total_bytes because of overcommitting.
 > So why exactly do you want this value?

  used for and to be consistent with what btrfs fi show does as of now,
  it shows the number of bytes used in the FS.

  just figured out that I could also use BTRFS_IOC_SPACE_INFO
  and do that math outside kernel. So pls. drop this kernel patch.
  I will be sending out a new progs patch which doesn't need kernel
  ioctl changes.

> Also how are you going to deal with
> older kernels where this isn't populated?  Thanks,

  Since the ioctl arg size didn't change here (as I was using the
  reserved space) it will just show 0.0 when old kernel is used.
  Not a best way (as it lets user to wonder) but a good trade-off
  to make the fix as simple as possible.

Thanks, Anand

^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 13/13] btrfs-progs: fix memory leaks of device_list_add()
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (9 preceding siblings ...)
  2013-06-10 14:59 ` [PATCH 0/2] btrfs: coalesce of patches Anand Jain
@ 2013-07-08  7:39 ` Anand Jain
  2013-07-15  4:58   ` Anand Jain
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 56+ messages in thread
From: Anand Jain @ 2013-07-08  7:39 UTC (permalink / raw)
  To: linux-btrfs

memory allocated by device_list_add has to be freed, the
function introduced here device_list_remove() would just
go that.
however the challenging part is about where we would
call this function.

there are two ways its handled
 the threads calling open_ctree_broken(), open_ctree() and
 open_ctree_fd() (which leads call to device_list_add)
 would anyway call close_ctree() so we put device_list_remove()
 there which will take care of freeing memory.

 now for threads calling device_list_add() outside of
 open_ctree(), has to call device_list_remove() separately
 which can be called as a last function in the thread
 this patch just does that.

device_list_remove accepts() NULL (deletes entire list)
or fsid (which would delete only the fsid matched in the
device_fs list). As of now though all calling functions
use NULL, I see potential that we would use fsid when we
have to create a single device list using both the
device-tree and from the btrfs-kernel.

further, mkfs.c thread should call device_list_remove()
as well, however mkfs.c uses a lot of in-flight exits()
which makes it very difficult to bring in this fix into
mkfs.c.  I shall be doing it in a separate patch.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c     |    4 +++
 cmds-filesystem.c |   11 ++++++++-
 cmds-replace.c    |    2 +
 cmds-scrub.c      |    3 ++
 disk-io.c         |    1 +
 mkfs.c            |    1 +
 utils.h           |    1 +
 volumes.c         |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 volumes.h         |    3 ++
 9 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 9525fcf..e4a1f1b 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -29,6 +29,7 @@
 #include "utils.h"
 
 #include "commands.h"
+#include "volumes.h"
 
 /* FIXME - imported cruft, fix sparse errors and warnings */
 #ifdef __CHECKER__
@@ -128,6 +129,7 @@ static int cmd_add_dev(int argc, char **argv)
 	}
 
 	close(fdmnt);
+	device_list_remove(NULL);
 	if (ret)
 		return ret+20;
 	else
@@ -213,6 +215,7 @@ static int cmd_scan_dev(int argc, char **argv)
 		int ret;
 		printf("Scanning for Btrfs filesystems\n");
 		ret = scan_for_btrfs(where, 1);
+		device_list_remove(NULL);
 		if (ret){
 			fprintf(stderr, "ERROR: error %d while scanning\n", ret);
 			return 18;
@@ -397,6 +400,7 @@ static int cmd_dev_stats(int argc, char **argv)
 out:
 	free(di_args);
 	close(fdmnt);
+	device_list_remove(NULL);
 
 	return err;
 }
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index a028e1d..1c26476 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -455,6 +455,7 @@ static int cmd_show(int argc, char **argv)
 		print_one_uuid(fs_devices);
 	}
 	printf("%s\n", BTRFS_BUILD_VERSION);
+	device_list_remove(NULL);
 	return 0;
 }
 
@@ -683,13 +684,19 @@ static const char * const cmd_label_usage[] = {
 
 static int cmd_label(int argc, char **argv)
 {
+	int ret;
 	if (check_argc_min(argc, 2) || check_argc_max(argc, 3))
 		usage(cmd_label_usage);
 
 	if (argc > 2)
-		return set_label(argv[1], argv[2]);
+		ret = set_label(argv[1], argv[2]);
 	else
-		return get_label(argv[1]);
+		ret = get_label(argv[1]);
+
+	if (is_existing_blk_or_reg_file(argv[1]))
+		device_list_remove(NULL);
+
+	return ret;
 }
 
 const struct cmd_group filesystem_cmd_group = {
diff --git a/cmds-replace.c b/cmds-replace.c
index c68986a..99a5abc 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -340,6 +340,7 @@ static int cmd_start_replace(int argc, char **argv)
 		}
 	}
 	close(fdmnt);
+	device_list_remove(NULL);
 	return 0;
 
 leave_with_error:
@@ -349,6 +350,7 @@ leave_with_error:
 		close(fdsrcdev);
 	if (fddstdev != -1)
 		close(fddstdev);
+	device_list_remove(NULL);
 	return -1;
 }
 
diff --git a/cmds-scrub.c b/cmds-scrub.c
index 95dfee3..08aab54 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -1496,6 +1496,7 @@ out:
 			unlink(sock_path);
 	}
 	close(fdmnt);
+	device_list_remove(NULL);
 
 	if (err)
 		return 1;
@@ -1564,6 +1565,7 @@ static int cmd_scrub_cancel(int argc, char **argv)
 out:
 	if (fdmnt != -1)
 		close(fdmnt);
+	device_list_remove(NULL);
 	return ret;
 }
 
@@ -1722,6 +1724,7 @@ out:
 	free(di_args);
 	if (fdres > -1)
 		close(fdres);
+	device_list_remove(NULL);
 
 	return err;
 }
diff --git a/disk-io.c b/disk-io.c
index 3937e3f..9b72576 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1338,6 +1338,7 @@ int close_ctree(struct btrfs_root *root)
 	}
 
 	close_all_devices(fs_info);
+	device_list_remove(NULL);
 	free_mapping_cache(fs_info);
 	extent_io_tree_cleanup(&fs_info->extent_cache);
 	extent_io_tree_cleanup(&fs_info->free_space_cache);
diff --git a/mkfs.c b/mkfs.c
index 95fceb3..5d131dd 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1461,6 +1461,7 @@ int main(int ac, char **av)
 		if (is_block_device(file))
 			if (test_dev_for_mkfs(file, force_overwrite, estr)) {
 				fprintf(stderr, "Error: %s", estr);
+				device_list_remove(NULL);
 				exit(1);
 			}
 	}
diff --git a/utils.h b/utils.h
index 1fa1c5a..f5b03a7 100644
--- a/utils.h
+++ b/utils.h
@@ -70,4 +70,5 @@ u64 btrfs_device_size(int fd, struct stat *st);
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
 int scan_for_btrfs(int where, int update_kernel);
 int get_label_mounted(const char *mount_path, char *labelp);
+int is_existing_blk_or_reg_file(const char* filename);
 #endif
diff --git a/volumes.c b/volumes.c
index 00c384a..fa27055 100644
--- a/volumes.c
+++ b/volumes.c
@@ -87,7 +87,7 @@ static struct btrfs_fs_devices *find_fsid(u8 *fsid)
 	return NULL;
 }
 
-static int device_list_add(const char *path,
+int device_list_add(const char *path,
 			   struct btrfs_super_block *disk_super,
 			   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
 {
@@ -155,6 +155,60 @@ static int device_list_add(const char *path,
 	return 0;
 }
 
+/* This will remove given fsid and it devices from the list,
+ * or when arg is NULL it will delete all.
+ */
+int device_list_remove(u8 *fsid)
+{
+	struct list_head *fsids;
+	struct list_head *cur_fsid;
+	struct btrfs_fs_devices *fs_devices;
+	struct list_head *cur_dev;
+	struct btrfs_device *device;
+	int del = 1;
+
+	fsids = btrfs_scanned_uuids();
+	list_for_each(cur_fsid, fsids) {
+		fs_devices = list_entry(cur_fsid, struct btrfs_fs_devices,
+				list);
+		if (fsid && memcmp(fs_devices->fsid, fsid, BTRFS_FSID_SIZE))
+			continue;
+
+		/* first check if all devs are closed before remove */
+		list_for_each(cur_dev, &fs_devices->devices) {
+			device = list_entry(cur_dev,
+					struct btrfs_device, dev_list);
+			if (device->fd > 0) {
+				fprintf(stderr, "attempted to remove device "\
+					"list without closing\n");
+				if (fsid)
+					return 1;
+				else {
+					del = 0;
+					break;
+				}
+			}
+		}
+
+		if (del) {
+			list_for_each(cur_dev, &fs_devices->devices) {
+				device = list_entry(cur_dev,
+					struct btrfs_device, dev_list);
+				list_del(&device->dev_list);
+				if (device->name)
+					kfree(device->name);
+				if (device->label)
+					kfree(device->label);
+				kfree(device);
+			}
+			list_del(&fs_devices->list);
+			kfree(fs_devices);
+		}
+		del = 1;
+	}
+	return 0;
+}
+
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
 {
 	struct btrfs_fs_devices *seed_devices;
diff --git a/volumes.h b/volumes.h
index 911f788..55a0b71 100644
--- a/volumes.h
+++ b/volumes.h
@@ -190,4 +190,7 @@ int btrfs_add_system_chunk(struct btrfs_trans_handle *trans,
 int btrfs_chunk_readonly(struct btrfs_root *root, u64 chunk_offset);
 struct btrfs_device *btrfs_find_device_by_devid(struct btrfs_root *root,
                                                 u64 devid, int instance);
+int device_list_add(const char *path, struct btrfs_super_block *disk_super,
+			   u64 devid, struct btrfs_fs_devices **fs_devices_ret);
+int device_list_remove(u8 *fsid);
 #endif
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* Re: [PATCH 13/13] btrfs-progs: fix memory leaks of device_list_add()
  2013-07-08  7:39 ` [PATCH 13/13] btrfs-progs: fix memory leaks of device_list_add() Anand Jain
@ 2013-07-15  4:58   ` Anand Jain
  0 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  4:58 UTC (permalink / raw)
  To: linux-btrfs




  A complete fix for the memory leak caused by device_list_add()
  is more complicated than initial idea and so sorry that I
  have to pull back this patch as well.


  The main reason-
     There are uncoordinated scan for btrfs which would result
     in doing the same thing multiple times.

     For a eg: consider the following..
-----------
int cmd_check(int argc, char **argv)
{
::
         if((ret = check_mounted(argv[optind])) < 0) {
::
         info = open_ctree_fs_info(argv[optind], bytenr, 0, rw, 1);
-----------

     Both of these functions (check_mounted() and open_ctree_fs_info())
     would perform a complete scan of /dev separately. This is
     one of the situation and there are quite a number of threads in
     btrfs-progs which would scan the /dev multiple times.
     this affects performance and will be noticeable
     in data centers with large number of disks/luns.


  I have to dig to develop a comprehensive fix, basically we need
  to revamp and develop a centralized device identification
  and management for the btrfs-progs which will also fix the
  apparent memory leak issues.

  Any feedback comments are welcome.

Thanks, Anand



 > further, mkfs.c thread should call device_list_remove()
 > as well, however mkfs.c uses a lot of in-flight exits()
 > which makes it very difficult to bring in this fix into
 > mkfs.c.  I shall be doing it in a separate patch.


^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches
@ 2013-07-15  5:30 ` Anand Jain
  2013-07-15  5:30   ` [PATCH 01/11] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
                     ` (11 more replies)
  0 siblings, 12 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

 This is the rebased patch set, rebased on top of the
 integration-20130710

 Dropped 
	btrfs-progs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl
 Introduced
	btrfs-progs: move out print in cmd_df to another function
	btrfs-progs: get string for the group profile and type

 (These changes were needed as part of show --kernel support,
 with out introducing new ioctl)

Anand Jain (11):
  btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
  btrfs-progs: label option in btrfs filesystem show is not coded
  btrfs-progs: update device scan usage
  btrfs-progs: congregate dev scan
  btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is
    provided
  btrfs-progs: scan /dev/mapper in filesystem show and device scan
  btrfs-progs: device delete to get errors from the kernel
  btrfs-progs: get_label_mounted to return label instead of print
  btrfs-progs: move out print in cmd_df to another function
  btrfs-progs: get string for the group profile and type
  btrfs-progs: introduce btrfs filesystem show --kernel

 btrfs-find-root.c |    2 +-
 cmds-device.c     |   29 +++--
 cmds-filesystem.c |  349 ++++++++++++++++++++++++++++++++++++++++-------------
 ctree.h           |   11 ++
 disk-io.c         |    2 +-
 ioctl.h           |   43 +++++++-
 man/btrfs.8.in    |   15 ++-
 utils.c           |   56 +++++++--
 utils.h           |   10 +-
 9 files changed, 395 insertions(+), 122 deletions(-)

-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 01/11] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 02/11] btrfs-progs: label option in btrfs filesystem show is not coded Anand Jain
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

btrfs_scan_for_fsid uses only one argument run_ioctl out of 3
so remove the rest two of them

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfs-find-root.c |    2 +-
 disk-io.c         |    2 +-
 utils.c           |    5 ++---
 utils.h           |    3 +--
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 967486a..253c200 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -110,7 +110,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
 	}
 
 	if (total_devs != 1) {
-		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
+		ret = btrfs_scan_for_fsid(1);
 		if (ret)
 			goto out;
 	}
diff --git a/disk-io.c b/disk-io.c
index 7c842be..5b84b02 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -716,7 +716,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 	}
 
 	if (total_devs != 1) {
-		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
+		ret = btrfs_scan_for_fsid(1);
 		if (ret)
 			goto out;
 	}
diff --git a/utils.c b/utils.c
index ca4eb1b..4f74d78 100644
--- a/utils.c
+++ b/utils.c
@@ -950,7 +950,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,
 
 	/* scan other devices */
 	if (is_btrfs && total_devs > 1) {
-		if((ret = btrfs_scan_for_fsid(fs_devices_mnt, total_devs, 1)))
+		if((ret = btrfs_scan_for_fsid(1)))
 			return ret;
 	}
 
@@ -1132,8 +1132,7 @@ fail:
 	return ret;
 }
 
-int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
-			int run_ioctls)
+int btrfs_scan_for_fsid(int run_ioctls)
 {
 	int ret;
 
diff --git a/utils.h b/utils.h
index 36fb591..5e5ea53 100644
--- a/utils.h
+++ b/utils.h
@@ -35,8 +35,7 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
 		      struct btrfs_root *root, int fd, char *path,
 		      u64 block_count, u32 io_width, u32 io_align,
 		      u32 sectorsize);
-int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
-			int run_ioctls);
+int btrfs_scan_for_fsid(int run_ioctls);
 void btrfs_register_one_device(char *fname);
 int btrfs_scan_one_dir(char *dirname, int run_ioctl);
 int check_mounted(const char *devicename);
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 02/11] btrfs-progs: label option in btrfs filesystem show is not coded
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
  2013-07-15  5:30   ` [PATCH 01/11] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 03/11] btrfs-progs: update device scan usage Anand Jain
                     ` (9 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c |    2 +-
 man/btrfs.8.in    |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 222e458..a1a8830 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -223,7 +223,7 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
 }
 
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices] [<uuid>|<label>]",
+	"btrfs filesystem show [--all-devices|<uuid>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 7df2ead..bc94579 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -31,7 +31,7 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBfilesystem label\fP\fI <dev> [newlabel]\fP
 .PP
-\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|<uuid>|<label>]\fP
+\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|<uuid>]\fP
 .PP
 \fBbtrfs\fP \fBfilesystem balance\fP\fI <path> \fP
 .PP
@@ -273,9 +273,9 @@ following constraints exist for a label:
 .IP
 - the maximum allowable length shall be less than 256 chars
 
-\fBfilesystem show\fR [--all-devices|<uuid>|<label>]\fR
-Show the btrfs filesystem with some additional info. If no \fIUUID\fP or 
-\fIlabel\fP is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
+\fBfilesystem show\fR [--all-devices|<uuid>]\fR
+Show the btrfs filesystem with some additional info. If no \fIUUID\fP
+is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
 If \fB--all-devices\fP is passed, all the devices under /dev are scanned;
 otherwise the devices list is extracted from the /proc/partitions file.
 .TP
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 03/11] btrfs-progs: update device scan usage
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
  2013-07-15  5:30   ` [PATCH 01/11] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
  2013-07-15  5:30   ` [PATCH 02/11] btrfs-progs: label option in btrfs filesystem show is not coded Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 04/11] btrfs-progs: congregate dev scan Anand Jain
                     ` (8 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

the btrfs device scan usage didnt publish --all-devices
option so add it

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 9e7328b..c94fcd5 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -178,7 +178,7 @@ static int cmd_rm_dev(int argc, char **argv)
 }
 
 static const char * const cmd_scan_dev_usage[] = {
-	"btrfs device scan [<device>...]",
+	"btrfs device scan [<--all-devices>|<device> [<device>...]]",
 	"Scan devices for a btrfs filesystem",
 	NULL
 };
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 04/11] btrfs-progs: congregate dev scan
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (2 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 03/11] btrfs-progs: update device scan usage Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 05/11] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
                     ` (7 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

the dev scan to find btrfs is performed at two locations
all most the same way one at filesystem show and another
at device scan. They both follow the same steps. This
patch does not alter anything except that it brings these
two same logic into the function scan_for_btrfs so that
we can play tweaking it.

the patch which recommends to use /dev/mapper
will also need it

v3:
bring in btrfs_scan_for_fsid to use scan_for_btrfs

thanks

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c     |   11 +++--------
 cmds-filesystem.c |    9 +++------
 utils.c           |   22 ++++++++++++++++++++--
 utils.h           |    5 ++++-
 4 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index c94fcd5..59edcb9 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -186,26 +186,21 @@ static const char * const cmd_scan_dev_usage[] = {
 static int cmd_scan_dev(int argc, char **argv)
 {
 	int	i, fd, e;
-	int	checklist = 1;
+	int	where = BTRFS_SCAN_PROC;
 	int	devstart = 1;
 
 	if( argc > 1 && !strcmp(argv[1],"--all-devices")){
 		if (check_argc_max(argc, 2))
 			usage(cmd_scan_dev_usage);
 
-		checklist = 0;
+		where = BTRFS_SCAN_DEV;
 		devstart += 1;
 	}
 
 	if(argc<=devstart){
-
 		int ret;
-
 		printf("Scanning for Btrfs filesystems\n");
-		if(checklist)
-			ret = btrfs_scan_block_devices(1);
-		else
-			ret = btrfs_scan_one_dir("/dev", 1);
+		ret = scan_for_btrfs(where, 1);
 		if (ret){
 			fprintf(stderr, "ERROR: error %d while scanning\n", ret);
 			return 18;
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index a1a8830..6ef6e6b 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -236,21 +236,18 @@ static int cmd_show(int argc, char **argv)
 	struct list_head *cur_uuid;
 	char *search = 0;
 	int ret;
-	int checklist = 1;
+	int where = BTRFS_SCAN_PROC;
 	int searchstart = 1;
 
 	if( argc > 1 && !strcmp(argv[1],"--all-devices")){
-		checklist = 0;
+		where = BTRFS_SCAN_DEV;
 		searchstart += 1;
 	}
 
 	if (check_argc_max(argc, searchstart + 1))
 		usage(cmd_show_usage);
 
-	if(checklist)
-		ret = btrfs_scan_block_devices(0);
-	else
-		ret = btrfs_scan_one_dir("/dev", 0);
+	ret = scan_for_btrfs(where, 0);
 
 	if (ret){
 		fprintf(stderr, "ERROR: error %d while scanning\n", ret);
diff --git a/utils.c b/utils.c
index 4f74d78..345bfdc 100644
--- a/utils.c
+++ b/utils.c
@@ -1136,9 +1136,9 @@ int btrfs_scan_for_fsid(int run_ioctls)
 {
 	int ret;
 
-	ret = btrfs_scan_block_devices(run_ioctls);
+	ret = scan_for_btrfs(BTRFS_SCAN_PROC, run_ioctls);
 	if (ret)
-		ret = btrfs_scan_one_dir("/dev", run_ioctls);
+		ret = scan_for_btrfs(BTRFS_SCAN_DEV, run_ioctls);
 	return ret;
 }
 
@@ -1830,3 +1830,21 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
 	close(fd);
 	return 0;
 }
+
+/*
+ * scans devs for the btrfs
+*/
+int scan_for_btrfs(int where, int update_kernel)
+{
+	int ret = 0;
+
+	switch (where) {
+	case BTRFS_SCAN_PROC:
+		ret = btrfs_scan_block_devices(update_kernel);
+		break;
+	case BTRFS_SCAN_DEV:
+		ret = btrfs_scan_one_dir("/dev", update_kernel);
+		break;
+	}
+	return ret;
+}
diff --git a/utils.h b/utils.h
index 5e5ea53..bdfa76b 100644
--- a/utils.h
+++ b/utils.h
@@ -24,6 +24,9 @@
 
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
+#define BTRFS_SCAN_PROC	1
+#define BTRFS_SCAN_DEV		2
+
 int make_btrfs(int fd, const char *device, const char *label,
 	       u64 blocks[6], u64 num_bytes, u32 nodesize,
 	       u32 leafsize, u32 sectorsize, u32 stripesize);
@@ -72,5 +75,5 @@ u64 btrfs_device_size(int fd, struct stat *st);
 /* Helper to always get proper size of the destination string */
 #define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest))
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
-
+int scan_for_btrfs(int where, int update_kernel);
 #endif
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 05/11] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (3 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 04/11] btrfs-progs: congregate dev scan Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-08-05 16:53     ` David Sterba
  2013-07-15  5:30   ` [PATCH 06/11] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
                     ` (6 subsequent siblings)
  11 siblings, 1 reply; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

we would need btrfs_scan_one_dir to san devs under /dev/mapper,
but /dev/mapper has links to the actual devs and current implementation
of btrfs_scan_one_dir skips links so it does not pick any
links under /dev/mapper. skipping links is fine when scanning whole of
/dev. But when we just want to scan /dev/mapper we want to avoid skipping
links otherwise we would be left with nothing.

This patch just adds the check if we are scanning devs
ONLY under /dev/mapper if when so it will not skip links

Thanks

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/utils.c b/utils.c
index 345bfdc..59010c9 100644
--- a/utils.c
+++ b/utils.c
@@ -1040,6 +1040,7 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
 	struct list_head pending_list;
 	struct btrfs_fs_devices *tmp_devices;
 	u64 num_devices;
+	int skip_link = 1;
 
 	INIT_LIST_HEAD(&pending_list);
 
@@ -1048,6 +1049,9 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
 		return -ENOMEM;
 	strcpy(pending->name, dirname);
 
+	if (!strncmp(dirname, "/dev/mapper", strlen("/dev/mapper")))
+		skip_link = 0;
+
 again:
 	dirname_len = strlen(pending->name);
 	fullpath = malloc(PATH_MAX);
@@ -1079,7 +1083,7 @@ again:
 			fprintf(stderr, "failed to stat %s\n", fullpath);
 			continue;
 		}
-		if (S_ISLNK(st.st_mode))
+		if (skip_link && S_ISLNK(st.st_mode))
 			continue;
 		if (S_ISDIR(st.st_mode)) {
 			struct pending_dir *next = malloc(sizeof(*next));
@@ -1090,7 +1094,7 @@ again:
 			strcpy(next->name, fullpath);
 			list_add_tail(&next->list, &pending_list);
 		}
-		if (!S_ISBLK(st.st_mode)) {
+		if (skip_link && !S_ISBLK(st.st_mode)) {
 			continue;
 		}
 		fd = open(fullpath, O_RDONLY);
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 06/11] btrfs-progs: scan /dev/mapper in filesystem show and device scan
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (4 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 05/11] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-08-05 17:04     ` David Sterba
  2013-07-15  5:30   ` [PATCH 07/11] btrfs-progs: device delete to get errors from the kernel Anand Jain
                     ` (5 subsequent siblings)
  11 siblings, 1 reply; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

Currently, btrsf fi show and btrfs dev scan uses
/proc/partitions (by default) (which gives priority
to dm-<x> over sd<y> paths) and with --all-devices it
will scan /dev only (where it skips links under /dev/mapper).

However using /dev/mapper paths are in common practice
with mount, fstab, and lvm, so its better to be consistent
with them.

This patch adds --mapper option to btrfs device scan and
btrfs filesystem show cli, when used will look for btrfs
devs under /dev/mapper and will use the links provided
under the /dev/mapper.

eg:
btrfs fi show --mapper
Label: none  uuid: 0a621111-ad84-4d80-842a-dd9c1c60bf51
        Total devices 2 FS bytes used 1.17MB
        devid    1 size 44.99GB used 2.04GB path /dev/mapper/mpathe
        devid    2 size 48.23GB used 2.03GB path /dev/mapper/mpathd

Label: none  uuid: bad9105f-bdc6-4626-9ba7-80bd97aebe19
        Total devices 1 FS bytes used 28.00KB
        devid    1 size 15.00GB used 2.04GB path /dev/mapper/mpathbp1

In the long run I want the usage of /dev/mapper path
(along with /proc/partitions) be the default option to
scan for the btrfs devs. (/proc/partitions must be scanned
as well because to include the mapper blacklisted devs.)

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c     |    8 +++++++-
 cmds-filesystem.c |    7 +++++--
 man/btrfs.8.in    |   11 ++++++-----
 utils.c           |    3 +++
 utils.h           |    5 +++--
 5 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 59edcb9..abf1360 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -178,7 +178,7 @@ static int cmd_rm_dev(int argc, char **argv)
 }
 
 static const char * const cmd_scan_dev_usage[] = {
-	"btrfs device scan [<--all-devices>|<device> [<device>...]]",
+	"btrfs device scan [<--all-devices>|<--mapper>|<device> [<device>...]]",
 	"Scan devices for a btrfs filesystem",
 	NULL
 };
@@ -195,6 +195,12 @@ static int cmd_scan_dev(int argc, char **argv)
 
 		where = BTRFS_SCAN_DEV;
 		devstart += 1;
+	} else if( argc > 1 && !strcmp(argv[1],"--mapper")){
+		if (check_argc_max(argc, 2))
+			usage(cmd_scan_dev_usage);
+
+		where = BTRFS_SCAN_MAPPER;
+		devstart += 1;
 	}
 
 	if(argc<=devstart){
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 6ef6e6b..0f5a30a 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -223,7 +223,7 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
 }
 
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices|<uuid>]",
+	"btrfs filesystem show [--all-devices|--mapper|<uuid>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
@@ -239,9 +239,12 @@ static int cmd_show(int argc, char **argv)
 	int where = BTRFS_SCAN_PROC;
 	int searchstart = 1;
 
-	if( argc > 1 && !strcmp(argv[1],"--all-devices")){
+	if (argc > 1 && !strcmp(argv[1],"--all-devices")) {
 		where = BTRFS_SCAN_DEV;
 		searchstart += 1;
+	} else if (argc > 1 && !strcmp(argv[1],"--mapper")) {
+		where = BTRFS_SCAN_MAPPER;
+		searchstart += 1;
 	}
 
 	if (check_argc_max(argc, searchstart + 1))
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index bc94579..eebf87d 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -31,11 +31,11 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBfilesystem label\fP\fI <dev> [newlabel]\fP
 .PP
-\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|<uuid>]\fP
+\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|--mapper|<uuid>]\fP
 .PP
 \fBbtrfs\fP \fBfilesystem balance\fP\fI <path> \fP
 .PP
-\fBbtrfs\fP \fBdevice scan\fP\fI [--all-devices|<device> [<device>...]]\fP
+\fBbtrfs\fP \fBdevice scan\fP\fI [--all-devices|--mapper|<device> [<device>...]]\fP
 .PP
 \fBbtrfs\fP \fBdevice stats\fP [-z] {\fI<path>\fP|\fI<device>\fP}
 .PP
@@ -273,10 +273,11 @@ following constraints exist for a label:
 .IP
 - the maximum allowable length shall be less than 256 chars
 
-\fBfilesystem show\fR [--all-devices|<uuid>]\fR
+\fBfilesystem show\fR [--all-devices|--mapper|<uuid>]\fR
 Show the btrfs filesystem with some additional info. If no \fIUUID\fP
 is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
 If \fB--all-devices\fP is passed, all the devices under /dev are scanned;
+If \fB--mapper\fP is passed, all the devices under /dev/mapper are scanned;
 otherwise the devices list is extracted from the /proc/partitions file.
 .TP
 
@@ -305,10 +306,10 @@ Add device(s) to the filesystem identified by \fI<path>\fR.
 Remove device(s) from a filesystem identified by \fI<path>\fR.
 .TP
 
-\fBdevice scan\fR \fI[--all-devices|<device> [<device>...]\fR
+\fBdevice scan\fR \fI[--all-devices|--mapper|<device> [<device>...]\fR
 If one or more devices are passed, these are scanned for a btrfs filesystem. 
 If no devices are passed, \fBbtrfs\fR scans all the block devices listed
-in the /proc/partitions file.
+If \fB--mapper\fP is passed, all the devices under /dev/mapper are scanned.
 Finally, if \fB--all-devices\fP is passed, all the devices under /dev are 
 scanned.
 .TP
diff --git a/utils.c b/utils.c
index 59010c9..a7f2ca3 100644
--- a/utils.c
+++ b/utils.c
@@ -1849,6 +1849,9 @@ int scan_for_btrfs(int where, int update_kernel)
 	case BTRFS_SCAN_DEV:
 		ret = btrfs_scan_one_dir("/dev", update_kernel);
 		break;
+	case BTRFS_SCAN_MAPPER:
+		ret = btrfs_scan_one_dir("/dev/mapper", update_kernel);
+		break;
 	}
 	return ret;
 }
diff --git a/utils.h b/utils.h
index bdfa76b..a10c51a 100644
--- a/utils.h
+++ b/utils.h
@@ -24,8 +24,9 @@
 
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 
-#define BTRFS_SCAN_PROC	1
-#define BTRFS_SCAN_DEV		2
+#define BTRFS_SCAN_PROC      1
+#define BTRFS_SCAN_DEV       2
+#define BTRFS_SCAN_MAPPER    3
 
 int make_btrfs(int fd, const char *device, const char *label,
 	       u64 blocks[6], u64 num_bytes, u32 nodesize,
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 07/11] btrfs-progs: device delete to get errors from the kernel
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (5 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 06/11] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 08/11] btrfs-progs: get_label_mounted to return label instead of print Anand Jain
                     ` (4 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

when user runs command btrfs dev del the raid requisite error if any
goes to the /var/log/messages, its not good idea to clutter messages
with these user (knowledge) errors, further user don't have to review
the system messages to know problem with the cli it should be dropped
to the user as part of the cli return.

to bring this feature created a set of the ERROR defined
BTRFS_ERROR_DEV* error codes and created their error string.

I expect this enum to be added with other error which we might
want to communicate to the user land

v3:
moved the code with in the file no logical change

v1->v2:
introduce error codes for the device mgmt usage

v1:
add another parameter to the ioctl arg structure to carry the error string

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c |   10 ++++++++--
 ioctl.h       |   43 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index abf1360..196e5e3 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -163,8 +163,14 @@ static int cmd_rm_dev(int argc, char **argv)
 		strncpy_null(arg.name, argv[i]);
 		res = ioctl(fdmnt, BTRFS_IOC_RM_DEV, &arg);
 		e = errno;
-		if(res<0){
-			fprintf(stderr, "ERROR: error removing the device '%s' - %s\n",
+		if (res > 0) {
+			fprintf(stderr,
+				"ERROR: error removing the device '%s' - %s\n",
+				argv[i], btrfs_err_str(res));
+			ret++;
+		} else if (res < 0) {
+			fprintf(stderr,
+				"ERROR: error removing the device '%s' - %s\n",
 				argv[i], strerror(e));
 			ret++;
 		}
diff --git a/ioctl.h b/ioctl.h
index f786410..a40a4a1 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -455,6 +455,48 @@ struct btrfs_ioctl_qgroup_create_args {
 	__u64 create;
 	__u64 qgroupid;
 };
+
+/* Error codes as returned by the kernel */
+enum btrfs_err_code {
+	notused,
+	BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET,
+	BTRFS_ERROR_DEV_TGT_REPLACE,
+	BTRFS_ERROR_DEV_MISSING_NOT_FOUND,
+	BTRFS_ERROR_DEV_ONLY_WRITABLE,
+	BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS
+};
+
+/* An error code to error string mapping for the kernel
+*  error codes
+*/
+static inline char *btrfs_err_str(enum btrfs_err_code err_code)
+{
+	switch (err_code) {
+		case BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET:
+			return "unable to go below two devices on raid1";
+		case BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET:
+			return "unable to go below four devices on raid10";
+		case BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET:
+			return "unable to go below two devices on raid5";
+		case BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET:
+			return "unable to go below three devices on raid6";
+		case BTRFS_ERROR_DEV_TGT_REPLACE:
+			return "unable to remove the dev_replace target dev";
+		case BTRFS_ERROR_DEV_MISSING_NOT_FOUND:
+			return "no missing devices found to remove";
+		case BTRFS_ERROR_DEV_ONLY_WRITABLE:
+			return "unable to remove the only writeable device";
+		case BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS:
+			return "add/delete/balance/replace/resize operation "\
+				"in progress";
+		default:
+			return NULL;
+	}
+}
+
 #define BTRFS_IOC_SNAP_CREATE _IOW(BTRFS_IOCTL_MAGIC, 1, \
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_DEFRAG _IOW(BTRFS_IOCTL_MAGIC, 2, \
@@ -550,7 +592,6 @@ struct btrfs_ioctl_clone_range_args {
 				      struct btrfs_ioctl_get_dev_stats)
 #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \
 				    struct btrfs_ioctl_dev_replace_args)
-
 #ifdef __cplusplus
 }
 #endif
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 08/11] btrfs-progs: get_label_mounted to return label instead of print
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (6 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 07/11] btrfs-progs: device delete to get errors from the kernel Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 09/11] btrfs-progs: move out print in cmd_df to another function Anand Jain
                     ` (3 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

This would help to reuse the function

v1->v2: removed static and updated .h file, in a preparation
        work for the following patch

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c |   18 +++++++++++++-----
 utils.h |    1 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/utils.c b/utils.c
index a7f2ca3..e8a42bf 100644
--- a/utils.c
+++ b/utils.c
@@ -1335,7 +1335,7 @@ static int get_label_unmounted(const char *dev)
  * mounted path rather than device.  Return the corresponding error
  * the user specified the device path.
  */
-static int get_label_mounted(const char *mount_path)
+int get_label_mounted(const char *mount_path, char *labelp)
 {
 	char label[BTRFS_LABEL_SIZE];
 	int fd;
@@ -1353,16 +1353,24 @@ static int get_label_mounted(const char *mount_path)
 		return -1;
 	}
 
-	fprintf(stdout, "%s\n", label);
+	strncpy(labelp, label, sizeof(label));
 	close(fd);
 	return 0;
 }
 
 int get_label(const char *btrfs_dev)
 {
-	return is_existing_blk_or_reg_file(btrfs_dev) ?
-		get_label_unmounted(btrfs_dev) :
-		get_label_mounted(btrfs_dev);
+	int ret;
+	char label[BTRFS_LABEL_SIZE];
+
+	if (is_existing_blk_or_reg_file(btrfs_dev))
+		ret = get_label_unmounted(btrfs_dev);
+	else {
+		ret = get_label_mounted(btrfs_dev, label);
+		if (!ret)
+			fprintf(stdout, "%s\n", label);
+	}
+	return ret;
 }
 
 int set_label(const char *btrfs_dev, const char *label)
diff --git a/utils.h b/utils.h
index a10c51a..708b580 100644
--- a/utils.h
+++ b/utils.h
@@ -77,4 +77,5 @@ u64 btrfs_device_size(int fd, struct stat *st);
 #define strncpy_null(dest, src) __strncpy__null(dest, src, sizeof(dest))
 int test_dev_for_mkfs(char *file, int force_overwrite, char *estr);
 int scan_for_btrfs(int where, int update_kernel);
+int get_label_mounted(const char *mount_path, char *labelp);
 #endif
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 09/11] btrfs-progs: move out print in cmd_df to another function
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (7 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 08/11] btrfs-progs: get_label_mounted to return label instead of print Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 10/11] btrfs-progs: get string for the group profile and type Anand Jain
                     ` (2 subsequent siblings)
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

This is a prepatory work for the following btrfs fi show command
fixes. So that we have a function get_df to get the fs sizes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c |   96 +++++++++++++++++++++++++++++++----------------------
 1 files changed, 56 insertions(+), 40 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 0f5a30a..b1e105b 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -38,31 +38,12 @@ static const char * const filesystem_cmd_group_usage[] = {
 	NULL
 };
 
-static const char * const cmd_df_usage[] = {
-	"btrfs filesystem df <path>",
-	"Show space usage information for a mount point",
-	NULL
-};
-
-static int cmd_df(int argc, char **argv)
+static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
 {
-	struct btrfs_ioctl_space_args *sargs, *sargs_orig;
-	u64 count = 0, i;
-	int ret;
-	int fd;
-	int e;
-	char *path;
-
-	if (check_argc_exact(argc, 2))
-		usage(cmd_df_usage);
-
-	path = argv[1];
-
-	fd = open_file_or_dir(path);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
-	}
+	u64 count = 0;
+	int ret, e;
+	struct btrfs_ioctl_space_args *sargs_orig;
+	struct btrfs_ioctl_space_args *sargs;
 
 	sargs_orig = sargs = malloc(sizeof(struct btrfs_ioctl_space_args));
 	if (!sargs)
@@ -74,14 +55,10 @@ static int cmd_df(int argc, char **argv)
 	ret = ioctl(fd, BTRFS_IOC_SPACE_INFO, sargs);
 	e = errno;
 	if (ret) {
-		fprintf(stderr, "ERROR: couldn't get space info on '%s' - %s\n",
-			path, strerror(e));
-		close(fd);
 		free(sargs);
-		return ret;
+		return -e;
 	}
 	if (!sargs->total_spaces) {
-		close(fd);
 		free(sargs);
 		return 0;
 	}
@@ -91,7 +68,6 @@ static int cmd_df(int argc, char **argv)
 	sargs = realloc(sargs, sizeof(struct btrfs_ioctl_space_args) +
 			(count * sizeof(struct btrfs_ioctl_space_info)));
 	if (!sargs) {
-		close(fd);
 		free(sargs_orig);
 		return -ENOMEM;
 	}
@@ -102,15 +78,20 @@ static int cmd_df(int argc, char **argv)
 	ret = ioctl(fd, BTRFS_IOC_SPACE_INFO, sargs);
 	e = errno;
 	if (ret) {
-		fprintf(stderr, "ERROR: couldn't get space info on '%s' - %s\n",
-			path, strerror(e));
-		close(fd);
 		free(sargs);
-		return ret;
+		return -e;
 	}
+	*sargs_ret = sargs;
+	return ret;
+}
 
+static void print_df(struct btrfs_ioctl_space_args *sargs)
+{
+	u64 i;
 	for (i = 0; i < sargs->total_spaces; i++) {
 		char description[80];
+		char *total_bytes;
+		char *used_bytes;
 		int written = 0;
 		u64 flags = sargs->spaces[i].flags;
 
@@ -153,14 +134,49 @@ static int cmd_df(int argc, char **argv)
 			written += 7;
 		}
 
-		printf("%s: total=%s, used=%s\n", description,
-			pretty_size(sargs->spaces[i].total_bytes),
-			pretty_size(sargs->spaces[i].used_bytes));
+		total_bytes = pretty_size(sargs->spaces[i].total_bytes);
+		used_bytes = pretty_size(sargs->spaces[i].used_bytes);
+		printf("%s: total=%s, used=%s\n", description, total_bytes,
+		       used_bytes);
 	}
-	close(fd);
-	free(sargs);
+}
 
-	return 0;
+
+static const char * const cmd_df_usage[] = {
+	"btrfs filesystem df <path>",
+	"Show space usage information for a mount point",
+	NULL
+};
+
+static int cmd_df(int argc, char **argv)
+{
+	struct btrfs_ioctl_space_args *sargs = NULL;
+	int ret;
+	int fd;
+	char *path;
+
+	if (check_argc_exact(argc, 2))
+		usage(cmd_df_usage);
+
+	path = argv[1];
+
+	fd = open_file_or_dir(path);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
+		return 12;
+	}
+	ret = get_df(fd, &sargs);
+
+	if (!ret) {
+		if (sargs) {
+			print_df(sargs);
+			free(sargs);
+		}
+	} else
+		fprintf(stderr, "ERROR: get_df failed %s\n", strerror(ret));
+
+	close(fd);
+	return ret;
 }
 
 static int uuid_search(struct btrfs_fs_devices *fs_devices, char *search)
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 10/11] btrfs-progs: get string for the group profile and type
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (8 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 09/11] btrfs-progs: move out print in cmd_df to another function Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-07-15  5:30   ` [PATCH 11/11] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
  2013-08-05 17:36   ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches David Sterba
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

Code can be well reused and this is the prepatory work
for the patch following this.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c |   83 ++++++++++++++++++++++++++++++----------------------
 ctree.h           |   11 +++++++
 2 files changed, 59 insertions(+), 35 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index b1e105b..5225db0 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -38,6 +38,44 @@ static const char * const filesystem_cmd_group_usage[] = {
 	NULL
 };
 
+static char * group_type_str(u64 flag)
+{
+	switch (flag & BTRFS_BLOCK_GROUP_TYPE_MASK) {
+	case BTRFS_BLOCK_GROUP_DATA:
+		return "data";
+	case BTRFS_BLOCK_GROUP_SYSTEM:
+		return "system";
+	case BTRFS_BLOCK_GROUP_METADATA:
+		return "metadata";
+	case BTRFS_BLOCK_GROUP_DATA|BTRFS_BLOCK_GROUP_METADATA:
+		return "mixed";
+	default:
+		return "unknown";
+	}
+}
+
+static char * group_profile_str(u64 flag)
+{
+	switch (flag & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
+	case 0:
+		return "single";
+	case BTRFS_BLOCK_GROUP_RAID0:
+		return "RAID0";
+	case BTRFS_BLOCK_GROUP_RAID1:
+		return "RAID1";
+	case BTRFS_BLOCK_GROUP_RAID5:
+		return "RAID5";
+	case BTRFS_BLOCK_GROUP_RAID6:
+		return "RAID6";
+	case BTRFS_BLOCK_GROUP_DUP:
+		return "DUP";
+	case BTRFS_BLOCK_GROUP_RAID10:
+		return "RAID10";
+	default:
+		return "unknown";
+	}
+}
+
 static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
 {
 	u64 count = 0;
@@ -94,45 +132,20 @@ static void print_df(struct btrfs_ioctl_space_args *sargs)
 		char *used_bytes;
 		int written = 0;
 		u64 flags = sargs->spaces[i].flags;
+		char g_str[64];
+		int g_sz;
 
 		memset(description, 0, 80);
 
-		if (flags & BTRFS_BLOCK_GROUP_DATA) {
-			if (flags & BTRFS_BLOCK_GROUP_METADATA) {
-				snprintf(description, 14, "%s",
-					 "Data+Metadata");
-				written += 13;
-			} else {
-				snprintf(description, 5, "%s", "Data");
-				written += 4;
-			}
-		} else if (flags & BTRFS_BLOCK_GROUP_SYSTEM) {
-			snprintf(description, 7, "%s", "System");
-			written += 6;
-		} else if (flags & BTRFS_BLOCK_GROUP_METADATA) {
-			snprintf(description, 9, "%s", "Metadata");
-			written += 8;
-		}
+		strcpy(g_str, group_type_str(flags));
+		g_sz = strlen(g_str);
+		snprintf(description, g_sz + 1, "%s", g_str);
+		written += g_sz;
 
-		if (flags & BTRFS_BLOCK_GROUP_RAID0) {
-			snprintf(description+written, 8, "%s", ", RAID0");
-			written += 7;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID1) {
-			snprintf(description+written, 8, "%s", ", RAID1");
-			written += 7;
-		} else if (flags & BTRFS_BLOCK_GROUP_DUP) {
-			snprintf(description+written, 6, "%s", ", DUP");
-			written += 5;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID10) {
-			snprintf(description+written, 9, "%s", ", RAID10");
-			written += 8;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID5) {
-			snprintf(description+written, 9, "%s", ", RAID5");
-			written += 7;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID6) {
-			snprintf(description+written, 9, "%s", ", RAID6");
-			written += 7;
-		}
+		strcpy(g_str, group_profile_str(flags));
+		g_sz = strlen(g_str);
+		snprintf(description+written, g_sz + 3, ", %s", g_str);
+		written += g_sz + 2;
 
 		total_bytes = pretty_size(sargs->spaces[i].total_bytes);
 		used_bytes = pretty_size(sargs->spaces[i].used_bytes);
diff --git a/ctree.h b/ctree.h
index e0decb9..1ad62bc 100644
--- a/ctree.h
+++ b/ctree.h
@@ -826,6 +826,17 @@ struct btrfs_csum_item {
 #define BTRFS_BLOCK_GROUP_RAID6    (1ULL << 8)
 #define BTRFS_BLOCK_GROUP_RESERVED	BTRFS_AVAIL_ALLOC_BIT_SINGLE
 
+#define BTRFS_BLOCK_GROUP_TYPE_MASK	(BTRFS_BLOCK_GROUP_DATA |    \
+					 BTRFS_BLOCK_GROUP_SYSTEM |  \
+					 BTRFS_BLOCK_GROUP_METADATA)
+
+#define BTRFS_BLOCK_GROUP_PROFILE_MASK	(BTRFS_BLOCK_GROUP_RAID0 |   \
+					 BTRFS_BLOCK_GROUP_RAID1 |   \
+					 BTRFS_BLOCK_GROUP_RAID5 |   \
+					 BTRFS_BLOCK_GROUP_RAID6 |   \
+					 BTRFS_BLOCK_GROUP_DUP |     \
+					 BTRFS_BLOCK_GROUP_RAID10)
+
 /* used in struct btrfs_balance_args fields */
 #define BTRFS_AVAIL_ALLOC_BIT_SINGLE	(1ULL << 48)
 
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 11/11] btrfs-progs: introduce btrfs filesystem show --kernel
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (9 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 10/11] btrfs-progs: get string for the group profile and type Anand Jain
@ 2013-07-15  5:30   ` Anand Jain
  2013-08-05 17:36   ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches David Sterba
  11 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-07-15  5:30 UTC (permalink / raw)
  To: linux-btrfs, dsterba

As of now btrfs filesystem show reads directly from
disks. So sometimes output can be stale, mainly when
user want to verify their last operation like,
labeling or device delete or add... etc.

This patch adds --kernel option to the 'filesystem show'
subcli, which will read from the kernel instead of
the disks directly.

also this path adds the group profile info to the
output

eg:
-----------------
btrfs fi show --kernel
Label: none  uuid: 39f55f14-e5ca-4a01-899d-915fd35bde05 mounted: /btrfs
	Group profile: metadata: RAID1  data: RAID1
	Total devices 2 FS bytes used 7.40GB
	devid    1 size 48.23GB used 11.04GB path /dev/dm-5
	devid    2 size 44.99GB used 11.03GB path /dev/mapper/mpathe

Label: none  uuid: a0beeb78-0019-4bdf-8002-0900a123ee07 mounted: /btrfs1
	Group profile: mixed: single
	Total devices 1 FS bytes used 7.40GB
	devid    1 size 15.00GB used 9.01GB path /dev/mapper/mpathbp1

btrfs fi show --kernel /btrfs2
Label: none  uuid: 9d6a347e-e8a0-44fe-9d2a-d28ee45ef33f mounted: /btrfs2
	Group profile: metadata: DUP  data: single
	Total devices 1 FS bytes used 2.22MB
	devid    1 size 15.00GB used 1.32GB path /dev/mapper/mpathcp1

btrfs fi show --kernel 9d6a347e-e8a0-44fe-9d2a-d28ee45ef33f
Label: none  uuid: 9d6a347e-e8a0-44fe-9d2a-d28ee45ef33f mounted: /btrfs2
	Group profile: metadata: DUP  data: single
	Total devices 1 FS bytes used 2.22MB
	devid    1 size 15.00GB used 1.32GB path /dev/mapper/mpathcp1
------------

v3->v4:
	dropped the dependence of used_bytes from the ioctl
	kernel, Instead used the get_df to calculate the
	used space.
	dropped the function device_list_add_from_kernel
	to update the original device_list_add instead
	I have my own print and device filters, this way I
	can add the group profile information in the show
	output.
v2->v3:
	Do the stuffs without adding new ioctl
	new dependencies: this patch also depends on
	path 9/13 to 12/13 also sent here.
v1->v2:
	code optimized to remove redundancy

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c |  162 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 155 insertions(+), 7 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 5225db0..148192a 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -22,6 +22,9 @@
 #include <errno.h>
 #include <uuid/uuid.h>
 #include <ctype.h>
+#include <mntent.h>
+#include <fcntl.h>
+#include <linux/limits.h>
 
 #include "kerncompat.h"
 #include "ctree.h"
@@ -251,8 +254,125 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
 	printf("\n");
 }
 
+/* adds up all the used spaces as reported by the space info ioctl
+ */
+static u64 cal_used_bytes(struct btrfs_ioctl_space_args *si)
+{
+	u64 ret = 0;
+	int i;
+	for (i = 0; i < si->total_spaces; i++)
+		ret += si->spaces[i].used_bytes;
+	return ret;
+}
+
+static int print_one_fs(struct btrfs_ioctl_fs_info_args *fi,
+		struct btrfs_ioctl_dev_info_args *di_n,
+		struct btrfs_ioctl_space_args *si_n, char *label, char *path)
+{
+	int i;
+	char uuidbuf[37];
+	struct btrfs_ioctl_dev_info_args *di = di_n;
+	u64 flags;
+
+	uuid_unparse(fi->fsid, uuidbuf);
+	printf("Label: %s  uuid: %s mounted: %s\n",
+		strlen(label)?label:"none", uuidbuf, path);
+	printf("\tGroup profile:");
+	for (i = si_n->total_spaces - 1; i >= 0; i--) {
+		flags = si_n->spaces[i].flags;
+		if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
+			continue;
+		printf(" %s: %s", group_type_str(flags),
+					group_profile_str(flags));
+		printf(" ");
+	}
+	printf("\n");
+
+	printf("\tTotal devices %llu FS bytes used %s\n",
+				fi->num_devices,
+			pretty_size(cal_used_bytes(si_n)));
+
+	for (i = 0; i < fi->num_devices; i++) {
+		di = (struct btrfs_ioctl_dev_info_args *)&di_n[i];
+		printf("\tdevid    %llu size %s used %s path %s\n",
+			di->devid,
+			pretty_size(di->total_bytes),
+			pretty_size(di->bytes_used),
+			di->path);
+	}
+
+	printf("\n");
+	return 0;
+}
+
+/* This function checks if the given input parameter is
+ * an uuid or a path
+ * return -1: some error in the given input
+ * return 0: unknow input
+ * return 1: given input is uuid
+ * return 2: given input is path
+ */
+static int check_arg_type(char *input, u8 *processed)
+{
+	int ret = 0;
+	if (!uuid_parse(input, processed))
+		ret = 1;
+	else if (realpath(input, (char *)processed))
+		ret = 2;
+	return ret;
+}
+
+
+static int btrfs_scan_kernel(void *input, int type)
+{
+	int ret = 0, fd;
+	FILE *f;
+	struct mntent *mnt;
+	struct btrfs_ioctl_fs_info_args fi;
+	struct btrfs_ioctl_dev_info_args *di = NULL;
+	struct btrfs_ioctl_space_args *si;
+	char label[BTRFS_LABEL_SIZE];
+
+	if ((f = setmntent ("/proc/mounts", "r")) == NULL)
+		return -errno;
+
+	while ((mnt = getmntent (f)) != NULL) {
+		if (strcmp(mnt->mnt_type, "btrfs"))
+			continue;
+		ret = get_fs_info(mnt->mnt_dir, &fi, &di);
+		if (ret)
+			return ret;
+
+		switch (type) {
+		case 0:
+			break;
+		case 1:
+			if (uuid_compare(fi.fsid, (u8 *)input))
+				continue;
+			break;
+		case 2:
+			if (strcmp(input, mnt->mnt_dir))
+				continue;
+			break;
+		default:
+			break;
+		}
+
+		fd = open(mnt->mnt_dir, O_RDONLY);
+		if (fd > 0 && !get_df(fd, &si)) {
+			get_label_mounted(mnt->mnt_dir, label);
+			print_one_fs(&fi, di, si, label, mnt->mnt_dir);
+			free(si);
+		}
+		if (fd > 0)
+			close(fd);
+		free(di);
+	}
+	return ret;
+}
+
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices|--mapper|<uuid>]",
+	"btrfs filesystem show [--all-devices|--mapper|--kernel|<uuid>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
@@ -264,9 +384,10 @@ static int cmd_show(int argc, char **argv)
 	struct btrfs_fs_devices *fs_devices;
 	struct list_head *cur_uuid;
 	char *search = 0;
-	int ret;
+	int ret = 0;
 	int where = BTRFS_SCAN_PROC;
 	int searchstart = 1;
+	u8 processed[PATH_MAX];
 
 	if (argc > 1 && !strcmp(argv[1],"--all-devices")) {
 		where = BTRFS_SCAN_DEV;
@@ -274,15 +395,42 @@ static int cmd_show(int argc, char **argv)
 	} else if (argc > 1 && !strcmp(argv[1],"--mapper")) {
 		where = BTRFS_SCAN_MAPPER;
 		searchstart += 1;
+	} else if (argc > 1 && !strcmp(argv[1],"--kernel")) {
+		where = 0;
+		searchstart += 1;
 	}
 
-	if (check_argc_max(argc, searchstart + 1))
-		usage(cmd_show_usage);
+	if (!where) {
+		if (! (searchstart < argc))
+			ret = btrfs_scan_kernel(NULL, 0);
 
-	ret = scan_for_btrfs(where, 0);
+		while (searchstart < argc) {
+			ret = check_arg_type(argv[searchstart], processed);
+			if (ret < 0) {
+				fprintf(stderr, "ERROR at the input %s\n",
+							argv[searchstart]);
+				return 1;
+			}
+			if (!ret) {
+				fprintf(stderr, "ERROR unknown %s\n",
+					argv[searchstart]);
+				return 1;
+			}
 
-	if (ret){
-		fprintf(stderr, "ERROR: error %d while scanning\n", ret);
+			ret = btrfs_scan_kernel(processed, ret);
+			if (ret)
+				break;
+			searchstart++;
+		}
+		if (ret)
+			fprintf(stderr, "ERROR: failed to obtain one or more FS %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = scan_for_btrfs(where, 0);
+	if (ret) {
+		fprintf(stderr, "ERROR: %d while scanning\n", ret);
 		return 18;
 	}
 	
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 56+ messages in thread

* Re: [PATCH 05/11] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided
  2013-07-15  5:30   ` [PATCH 05/11] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
@ 2013-08-05 16:53     ` David Sterba
  0 siblings, 0 replies; 56+ messages in thread
From: David Sterba @ 2013-08-05 16:53 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs, dsterba

On Mon, Jul 15, 2013 at 01:30:51PM +0800, Anand Jain wrote:
> @@ -1048,6 +1049,9 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
>  		return -ENOMEM;
>  	strcpy(pending->name, dirname);
>  
> +	if (!strncmp(dirname, "/dev/mapper", strlen("/dev/mapper")))
> +		skip_link = 0;

This would break if it's called with "/dev/mapper/", I suggest to use
realpath on the dirname argument first. And you can use strcmp.

david

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 06/11] btrfs-progs: scan /dev/mapper in filesystem show and device scan
  2013-07-15  5:30   ` [PATCH 06/11] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
@ 2013-08-05 17:04     ` David Sterba
  2013-08-13  4:07       ` anand jain
  0 siblings, 1 reply; 56+ messages in thread
From: David Sterba @ 2013-08-05 17:04 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs, dsterba

On Mon, Jul 15, 2013 at 01:30:52PM +0800, Anand Jain wrote:
> This patch adds --mapper option to btrfs device scan and
> btrfs filesystem show cli, when used will look for btrfs
> devs under /dev/mapper and will use the links provided
> under the /dev/mapper.

> In the long run I want the usage of /dev/mapper path
> (along with /proc/partitions) be the default option to
> scan for the btrfs devs. (/proc/partitions must be scanned
> as well because to include the mapper blacklisted devs.)

Well, we want to avoid using own scanning and always consult the blkid
cache, so I'd rather stop adding user-visible changes to this command.

david

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches
  2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
                     ` (10 preceding siblings ...)
  2013-07-15  5:30   ` [PATCH 11/11] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
@ 2013-08-05 17:36   ` David Sterba
  2013-08-06 15:08     ` anand jain
  11 siblings, 1 reply; 56+ messages in thread
From: David Sterba @ 2013-08-05 17:36 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs, dsterba

On Mon, Jul 15, 2013 at 01:30:46PM +0800, Anand Jain wrote:
> Anand Jain (11):
>   btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
>   btrfs-progs: label option in btrfs filesystem show is not coded
>   btrfs-progs: update device scan usage
>   btrfs-progs: congregate dev scan
>   btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is
>     provided
>   btrfs-progs: scan /dev/mapper in filesystem show and device scan
>   btrfs-progs: device delete to get errors from the kernel
>   btrfs-progs: get_label_mounted to return label instead of print
>   btrfs-progs: move out print in cmd_df to another function
>   btrfs-progs: get string for the group profile and type
>   btrfs-progs: introduce btrfs filesystem show --kernel

patches 1-4,7,8 added to integration, skipped 5,6 as they enhance dev
scan that'll need a rewrite anyway.

9-11 are sort of independent of the previous changes so I'd like to look
at them in context of all the other df/show patches.

david

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches
  2013-08-05 17:36   ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches David Sterba
@ 2013-08-06 15:08     ` anand jain
  0 siblings, 0 replies; 56+ messages in thread
From: anand jain @ 2013-08-06 15:08 UTC (permalink / raw)
  To: dsterba, linux-btrfs


On 06/08/2013 01:36, David Sterba wrote:
> On Mon, Jul 15, 2013 at 01:30:46PM +0800, Anand Jain wrote:
>> Anand Jain (11):
>>    btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments
>>    btrfs-progs: label option in btrfs filesystem show is not coded
>>    btrfs-progs: update device scan usage
>>    btrfs-progs: congregate dev scan
>>    btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is
>>      provided
>>    btrfs-progs: scan /dev/mapper in filesystem show and device scan
>>    btrfs-progs: device delete to get errors from the kernel
>>    btrfs-progs: get_label_mounted to return label instead of print
>>    btrfs-progs: move out print in cmd_df to another function
>>    btrfs-progs: get string for the group profile and type
>>    btrfs-progs: introduce btrfs filesystem show --kernel
>
> patches 1-4,7,8 added to integration, skipped 5,6 as they enhance dev
> scan that'll need a rewrite anyway.
>
> 9-11 are sort of independent of the previous changes so I'd like to look
> at them in context of all the other df/show patches.

  makes sense. I am looking at them.

Thanks,  Anand

^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 0/2 v2] introduce btrfs filesystem show --kernel
@ 2013-08-08  8:07 ` Anand Jain
  2013-08-08  8:07   ` [PATCH 1/2] btrfs-progs: move out print in cmd_df to another function Anand Jain
  2013-08-08  8:07   ` [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
  0 siblings, 2 replies; 56+ messages in thread
From: Anand Jain @ 2013-08-08  8:07 UTC (permalink / raw)
  To: linux-btrfs

This patch set introduces --kernel option for filesystem show 
for the reason as mentioned in the patch 1/2 below
1/1 is the preparatory patch

Anand Jain (2):
  btrfs-progs: move out print in cmd_df to another function
  btrfs-progs: introduce btrfs filesystem show --kernel

 cmds-filesystem.c | 355 ++++++++++++++++++++++++++++++++++++++++--------------
 ctree.h           |  11 ++
 man/btrfs.8.in    |   5 +-
 3 files changed, 281 insertions(+), 90 deletions(-)

-- 
1.8.1.191.g414c78c

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 1/2] btrfs-progs: move out print in cmd_df to another function
  2013-08-08  8:07 ` [PATCH 0/2 v2] introduce btrfs filesystem show --kernel Anand Jain
@ 2013-08-08  8:07   ` Anand Jain
  2013-08-08  8:07   ` [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
  1 sibling, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-08-08  8:07 UTC (permalink / raw)
  To: linux-btrfs

This is a prepatory work for the following btrfs fi show command
fixes. So that we have a function get_df to get the fs sizes

v2:
combined the other patches as below and rebase
 btrfs-progs: get string for the group profile and type

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c | 190 +++++++++++++++++++++++++++++++-----------------------
 ctree.h           |  11 ++++
 2 files changed, 122 insertions(+), 79 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index a4e30ea..be8afde 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -44,28 +44,51 @@ static const char * const cmd_df_usage[] = {
 	NULL
 };
 
-static int cmd_df(int argc, char **argv)
+static char * group_type_str(u64 flag)
 {
-	struct btrfs_ioctl_space_args *sargs, *sargs_orig;
-	u64 count = 0, i;
-	int ret;
-	int fd;
-	int e;
-	char *path;
-	DIR  *dirstream = NULL;
-
-	if (check_argc_exact(argc, 2))
-		usage(cmd_df_usage);
-
-	path = argv[1];
+	switch (flag & BTRFS_BLOCK_GROUP_TYPE_MASK) {
+	case BTRFS_BLOCK_GROUP_DATA:
+		return "data";
+	case BTRFS_BLOCK_GROUP_SYSTEM:
+		return "system";
+	case BTRFS_BLOCK_GROUP_METADATA:
+		return "metadata";
+	case BTRFS_BLOCK_GROUP_DATA|BTRFS_BLOCK_GROUP_METADATA:
+		return "mixed";
+	default:
+		return "unknown";
+	}
+}
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+static char * group_profile_str(u64 flag)
+{
+	switch (flag & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
+	case 0:
+		return "single";
+	case BTRFS_BLOCK_GROUP_RAID0:
+		return "RAID0";
+	case BTRFS_BLOCK_GROUP_RAID1:
+		return "RAID1";
+	case BTRFS_BLOCK_GROUP_RAID5:
+		return "RAID5";
+	case BTRFS_BLOCK_GROUP_RAID6:
+		return "RAID6";
+	case BTRFS_BLOCK_GROUP_DUP:
+		return "DUP";
+	case BTRFS_BLOCK_GROUP_RAID10:
+		return "RAID10";
+	default:
+		return "unknown";
 	}
+}
+
+static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
+{
+	u64 count = 0;
+	int ret, e;
+	struct btrfs_ioctl_space_args *sargs;
 
-	sargs_orig = sargs = malloc(sizeof(struct btrfs_ioctl_space_args));
+	sargs = malloc(sizeof(struct btrfs_ioctl_space_args));
 	if (!sargs)
 		return -ENOMEM;
 
@@ -75,89 +98,98 @@ static int cmd_df(int argc, char **argv)
 	ret = ioctl(fd, BTRFS_IOC_SPACE_INFO, sargs);
 	e = errno;
 	if (ret) {
-		fprintf(stderr, "ERROR: couldn't get space info on '%s' - %s\n",
-			path, strerror(e));
-		goto out;
+		fprintf(stderr, "ERROR: couldn't get space info - %s\n",
+			strerror(e));
+		free(sargs);
+		return ret;
 	}
 	if (!sargs->total_spaces) {
-		ret = 0;
-		goto out;
+		free(sargs);
+		return 0;
 	}
-
 	count = sargs->total_spaces;
+	free(sargs);
 
-	sargs = realloc(sargs, sizeof(struct btrfs_ioctl_space_args) +
+	sargs = malloc(sizeof(struct btrfs_ioctl_space_args) +
 			(count * sizeof(struct btrfs_ioctl_space_info)));
-	if (!sargs) {
-		sargs = sargs_orig;
+	if (!sargs)
 		ret = -ENOMEM;
-		goto out;
-	}
 
 	sargs->space_slots = count;
 	sargs->total_spaces = 0;
-
 	ret = ioctl(fd, BTRFS_IOC_SPACE_INFO, sargs);
 	e = errno;
 	if (ret) {
-		fprintf(stderr, "ERROR: couldn't get space info on '%s' - %s\n",
-			path, strerror(e));
-		goto out;
+		fprintf(stderr, "ERROR: get space info count %llu - %s\n",
+				count, strerror(e));
+		free(sargs);
+		return ret;
 	}
+	*sargs_ret = sargs;
+	return 0;
+}
 
-	for (i = 0; i < sargs->total_spaces; i++) {
-		char description[80];
-		int written = 0;
-		u64 flags = sargs->spaces[i].flags;
+static void print_df(struct btrfs_ioctl_space_args *sargs)
+{
+	char description[80];
+	char *total_bytes;
+	char *used_bytes;
+	u64 flags;
+	u64 i;
+	int written;
+	char g_str[64];
+	int g_sz;
 
+	for (i = 0; i < sargs->total_spaces; i++) {
+		flags = sargs->spaces[i].flags;
+		written = 0;
 		memset(description, 0, 80);
 
-		if (flags & BTRFS_BLOCK_GROUP_DATA) {
-			if (flags & BTRFS_BLOCK_GROUP_METADATA) {
-				snprintf(description, 14, "%s",
-					 "Data+Metadata");
-				written += 13;
-			} else {
-				snprintf(description, 5, "%s", "Data");
-				written += 4;
-			}
-		} else if (flags & BTRFS_BLOCK_GROUP_SYSTEM) {
-			snprintf(description, 7, "%s", "System");
-			written += 6;
-		} else if (flags & BTRFS_BLOCK_GROUP_METADATA) {
-			snprintf(description, 9, "%s", "Metadata");
-			written += 8;
-		}
+		strcpy(g_str, group_type_str(flags));
+		g_sz = strlen(g_str);
+		snprintf(description, g_sz + 1, "%s", g_str);
+		written += g_sz;
 
-		if (flags & BTRFS_BLOCK_GROUP_RAID0) {
-			snprintf(description+written, 8, "%s", ", RAID0");
-			written += 7;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID1) {
-			snprintf(description+written, 8, "%s", ", RAID1");
-			written += 7;
-		} else if (flags & BTRFS_BLOCK_GROUP_DUP) {
-			snprintf(description+written, 6, "%s", ", DUP");
-			written += 5;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID10) {
-			snprintf(description+written, 9, "%s", ", RAID10");
-			written += 8;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID5) {
-			snprintf(description+written, 9, "%s", ", RAID5");
-			written += 7;
-		} else if (flags & BTRFS_BLOCK_GROUP_RAID6) {
-			snprintf(description+written, 9, "%s", ", RAID6");
-			written += 7;
-		}
+		strcpy(g_str, group_profile_str(flags));
+		g_sz = strlen(g_str);
+		snprintf(description+written, g_sz + 3, ", %s", g_str);
+		written += g_sz + 2;
 
-		printf("%s: total=%s, used=%s\n", description,
-			pretty_size(sargs->spaces[i].total_bytes),
-			pretty_size(sargs->spaces[i].used_bytes));
+		total_bytes = pretty_size(sargs->spaces[i].total_bytes);
+		used_bytes = pretty_size(sargs->spaces[i].used_bytes);
+		printf("%s: total=%s, used=%s\n", description, total_bytes,
+		       used_bytes);
 	}
-out:
-	close_file_or_dir(fd, dirstream);
-	free(sargs);
+}
 
-	return 0;
+static int cmd_df(int argc, char **argv)
+{
+	struct btrfs_ioctl_space_args *sargs = NULL;
+	int ret;
+	int fd;
+	char *path;
+	DIR  *dirstream = NULL;
+
+	if (check_argc_exact(argc, 2))
+		usage(cmd_df_usage);
+
+	path = argv[1];
+
+	fd = open_file_or_dir(path, &dirstream);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
+		return 12;
+	}
+	ret = get_df(fd, &sargs);
+
+	if (!ret && sargs) {
+		print_df(sargs);
+		free(sargs);
+	} else
+		fprintf(stderr, "ERROR: get_df failed %s\n", strerror(ret));
+
+	close_file_or_dir(fd, dirstream);
+	return ret;
 }
 
 static int uuid_search(struct btrfs_fs_devices *fs_devices, char *search)
diff --git a/ctree.h b/ctree.h
index 257396d..21bfa30 100644
--- a/ctree.h
+++ b/ctree.h
@@ -826,6 +826,17 @@ struct btrfs_csum_item {
 #define BTRFS_BLOCK_GROUP_RAID6    (1ULL << 8)
 #define BTRFS_BLOCK_GROUP_RESERVED	BTRFS_AVAIL_ALLOC_BIT_SINGLE
 
+#define BTRFS_BLOCK_GROUP_TYPE_MASK	(BTRFS_BLOCK_GROUP_DATA |    \
+					 BTRFS_BLOCK_GROUP_SYSTEM |  \
+					 BTRFS_BLOCK_GROUP_METADATA)
+
+#define BTRFS_BLOCK_GROUP_PROFILE_MASK	(BTRFS_BLOCK_GROUP_RAID0 |   \
+					 BTRFS_BLOCK_GROUP_RAID1 |   \
+					 BTRFS_BLOCK_GROUP_RAID5 |   \
+					 BTRFS_BLOCK_GROUP_RAID6 |   \
+					 BTRFS_BLOCK_GROUP_DUP |     \
+					 BTRFS_BLOCK_GROUP_RAID10)
+
 /* used in struct btrfs_balance_args fields */
 #define BTRFS_AVAIL_ALLOC_BIT_SINGLE	(1ULL << 48)
 
-- 
1.8.1.191.g414c78c

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel
  2013-08-08  8:07 ` [PATCH 0/2 v2] introduce btrfs filesystem show --kernel Anand Jain
  2013-08-08  8:07   ` [PATCH 1/2] btrfs-progs: move out print in cmd_df to another function Anand Jain
@ 2013-08-08  8:07   ` Anand Jain
  2013-08-08 18:08     ` Zach Brown
  1 sibling, 1 reply; 56+ messages in thread
From: Anand Jain @ 2013-08-08  8:07 UTC (permalink / raw)
  To: linux-btrfs

As of now btrfs filesystem show reads directly from
disks. So sometimes output can be stale, mainly when
user want to verify their last operation like,
labeling or device delete or add... etc.

This patch adds --kernel option to the 'filesystem show'
subcli, which will read from the kernel instead of
the disks directly.

also this path adds the group profile info to the
output

eg:
-----------------
btrfs fi show --kernel
Label: none  uuid: 39f55f14-e5ca-4a01-899d-915fd35bde05 mounted: /btrfs
        Group profile: metadata: RAID1  data: RAID1
        Total devices 2 FS bytes used 7.40GB
        devid    1 size 48.23GB used 11.04GB path /dev/dm-5
        devid    2 size 44.99GB used 11.03GB path /dev/mapper/mpathe

Label: none  uuid: a0beeb78-0019-4bdf-8002-0900a123ee07 mounted: /btrfs1
        Group profile: mixed: single
        Total devices 1 FS bytes used 7.40GB
        devid    1 size 15.00GB used 9.01GB path /dev/mapper/mpathbp1

btrfs fi show --kernel /btrfs2
Label: none  uuid: 9d6a347e-e8a0-44fe-9d2a-d28ee45ef33f mounted: /btrfs2
        Group profile: metadata: DUP  data: single
        Total devices 1 FS bytes used 2.22MB
        devid    1 size 15.00GB used 1.32GB path /dev/mapper/mpathcp1

btrfs fi show --kernel 9d6a347e-e8a0-44fe-9d2a-d28ee45ef33f
Label: none  uuid: 9d6a347e-e8a0-44fe-9d2a-d28ee45ef33f mounted: /btrfs2
        Group profile: metadata: DUP  data: single
        Total devices 1 FS bytes used 2.22MB
        devid    1 size 15.00GB used 1.32GB path /dev/mapper/mpathcp1
------------

v3->v4:
        dropped the dependence of used_bytes from the ioctl
        kernel, Instead used the get_df to calculate the
        used space.
        dropped the function device_list_add_from_kernel
        to update the original device_list_add instead
        I have my own print and device filters, this way I
        can add the group profile information in the show
        output.
v2->v3:
        Do the stuffs without adding new ioctl
        new dependencies: this patch also depends on
        path 9/13 to 12/13 also sent here.
v1->v2:
        code optimized to remove redundancy

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 man/btrfs.8.in    |   5 +-
 2 files changed, 159 insertions(+), 11 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index be8afde..74ad30b 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -22,6 +22,9 @@
 #include <errno.h>
 #include <uuid/uuid.h>
 #include <ctype.h>
+#include <mntent.h>
+#include <fcntl.h>
+#include <linux/limits.h>
 
 #include "kerncompat.h"
 #include "ctree.h"
@@ -251,8 +254,124 @@ static void print_one_uuid(struct btrfs_fs_devices *fs_devices)
 	printf("\n");
 }
 
+/* adds up all the used spaces as reported by the space info ioctl
+ */
+static u64 cal_used_bytes(struct btrfs_ioctl_space_args *si)
+{
+	u64 ret = 0;
+	int i;
+	for (i = 0; i < si->total_spaces; i++)
+		ret += si->spaces[i].used_bytes;
+	return ret;
+}
+
+static int print_one_fs(struct btrfs_ioctl_fs_info_args *fi,
+		struct btrfs_ioctl_dev_info_args *di_n,
+		struct btrfs_ioctl_space_args *si_n, char *label, char *path)
+{
+	int i;
+	char uuidbuf[37];
+	struct btrfs_ioctl_dev_info_args *di = di_n;
+	u64 flags;
+
+	uuid_unparse(fi->fsid, uuidbuf);
+	printf("Label: %s  uuid: %s mounted: %s\n",
+		strlen(label)?label:"none", uuidbuf, path);
+	printf("\tGroup profile:");
+	for (i = si_n->total_spaces - 1; i >= 0; i--) {
+		flags = si_n->spaces[i].flags;
+		if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
+			continue;
+		printf(" %s: %s", group_type_str(flags),
+					group_profile_str(flags));
+		printf(" ");
+	}
+	printf("\n");
+
+	printf("\tTotal devices %llu FS bytes used %s\n",
+				fi->num_devices,
+			pretty_size(cal_used_bytes(si_n)));
+
+	for (i = 0; i < fi->num_devices; i++) {
+		di = (struct btrfs_ioctl_dev_info_args *)&di_n[i];
+		printf("\tdevid    %llu size %s used %s path %s\n",
+			di->devid,
+			pretty_size(di->total_bytes),
+			pretty_size(di->bytes_used),
+			di->path);
+	}
+
+	printf("\n");
+	return 0;
+}
+
+/* This function checks if the given input parameter is
+ * an uuid or a path
+ * return -1: some error in the given input
+ * return 0: unknow input
+ * return 1: given input is uuid
+ * return 2: given input is path
+ */
+static int check_arg_type(char *input, u8 *processed)
+{
+	int ret = 0;
+	if (!uuid_parse(input, processed))
+		ret = 1;
+	else if (realpath(input, (char *)processed))
+		ret = 2;
+	return ret;
+}
+
+static int btrfs_scan_kernel(void *input, int type)
+{
+	int ret = 0, fd;
+	FILE *f;
+	struct mntent *mnt;
+	struct btrfs_ioctl_fs_info_args fi;
+	struct btrfs_ioctl_dev_info_args *di = NULL;
+	struct btrfs_ioctl_space_args *si;
+	char label[BTRFS_LABEL_SIZE];
+
+	if ((f = setmntent ("/proc/mounts", "r")) == NULL)
+		return -errno;
+
+	while ((mnt = getmntent (f)) != NULL) {
+		if (strcmp(mnt->mnt_type, "btrfs"))
+			continue;
+		ret = get_fs_info(mnt->mnt_dir, &fi, &di);
+		if (ret)
+			return ret;
+
+		switch (type) {
+		case 0:
+			break;
+		case 1:
+			if (uuid_compare(fi.fsid, (u8 *)input))
+				continue;
+			break;
+		case 2:
+			if (strcmp(input, mnt->mnt_dir))
+				continue;
+			break;
+		default:
+			break;
+		}
+
+		fd = open(mnt->mnt_dir, O_RDONLY);
+		if (fd > 0 && !get_df(fd, &si)) {
+			get_label_mounted(mnt->mnt_dir, label);
+			print_one_fs(&fi, di, si, label, mnt->mnt_dir);
+			free(si);
+		}
+		if (fd > 0)
+			close(fd);
+		free(di);
+	}
+	return ret;
+}
+
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices|<uuid>]",
+	"btrfs filesystem show [--all-devices|--mapper|--kernel|<uuid>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
@@ -264,23 +383,51 @@ static int cmd_show(int argc, char **argv)
 	struct btrfs_fs_devices *fs_devices;
 	struct list_head *cur_uuid;
 	char *search = 0;
-	int ret;
+	int ret = 0;
 	int where = BTRFS_SCAN_PROC;
 	int searchstart = 1;
+	u8 processed[PATH_MAX];
 
-	if( argc > 1 && !strcmp(argv[1],"--all-devices")){
+	if( argc > 1 && !strcmp(argv[1], "--all-devices")){
 		where = BTRFS_SCAN_DEV;
 		searchstart += 1;
+	} else if (argc > 1 && !strcmp(argv[1], "--kernel")) {
+		where = 0;
+		searchstart += 1;
 	}
 
-	if (check_argc_max(argc, searchstart + 1))
-		usage(cmd_show_usage);
+	if (!where) {
+		if (! (searchstart < argc))
+			ret = btrfs_scan_kernel(NULL, 0);
 
-	ret = scan_for_btrfs(where, 0);
+		while (searchstart < argc) {
+			ret = check_arg_type(argv[searchstart], processed);
+			if (ret < 0) {
+				fprintf(stderr, "ERROR at input %s\n",
+							argv[searchstart]);
+				return 1;
+			}
+			if (!ret) {
+				fprintf(stderr, "ERROR unknown %s\n",
+					argv[searchstart]);
+				return 1;
+			}
+
+			ret = btrfs_scan_kernel(processed, ret);
+			if (ret)
+				break;
+			searchstart++;
+		}
+		if (ret)
+			fprintf(stderr, "ERROR: scan kernel failed, %d\n",
+				ret);
+		return ret;
+	}
 
-	if (ret){
-		fprintf(stderr, "ERROR: error %d while scanning\n", ret);
-		return 18;
+	ret = scan_for_btrfs(where, 0);
+	if (ret) {
+		fprintf(stderr, "ERROR: %d while scanning\n", ret);
+		return 1;
 	}
 	
 	if(searchstart < argc)
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index e9ec1a7..6383469 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -25,7 +25,7 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBfilesystem df\fP\fI <path>\fP
 .PP
-\fBbtrfs\fP \fBfilesystem show\fP [--all-devices|\fI<uuid>\fP|\fI<label>\fP]\fP
+\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|--kernel] [\fI<uuid>|<path>]\fP\fP
 .PP
 \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
 .PP
@@ -254,11 +254,12 @@ Show information of a given subvolume in the \fI<path>\fR.
 Show space usage information for a mount point.
 .TP
 
-\fBfilesystem show\fR [--all-devices|\fI<uuid>\fR|\fI<label>\fR]\fR
+\fBfilesystem show\fR [--all-devices|--kernel] [\fI<uuid>|<path>]\fP\fP
 Show the btrfs filesystem with some additional info. If no \fIUUID\fP or
 \fIlabel\fP is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
 If \fB--all-devices\fP is passed, all the devices under /dev are scanned;
 otherwise the devices list is extracted from the /proc/partitions file.
+The --kernel will scan the btrfs kernel for the mounted btrfs.
 .TP
 
 \fBfilesystem sync\fR\fI <path> \fR
-- 
1.8.1.191.g414c78c

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 0/2] scan /dev/mapper in filesystem show and device scan
  2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
                   ` (12 preceding siblings ...)
  2013-08-08  8:07 ` [PATCH 0/2 v2] introduce btrfs filesystem show --kernel Anand Jain
@ 2013-08-08  8:09 ` Anand Jain
  2013-08-08  8:09   ` [PATCH 1/2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
                     ` (2 more replies)
  13 siblings, 3 replies; 56+ messages in thread
From: Anand Jain @ 2013-08-08  8:09 UTC (permalink / raw)
  To: linux-btrfs

This patch brings the /dev/mapper to be used as the path for
the btrfs kernel through dev scan
1/2 is the preparatory patch

Anand Jain (2):
  btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is
    provided
  btrfs-progs: scan /dev/mapper in filesystem show and device scan

 cmds-device.c     |  8 +++++++-
 cmds-filesystem.c |  7 +++++--
 man/btrfs.8.in    | 22 ++++++++++++----------
 utils.c           | 22 +++++++++++++++++++---
 utils.h           |  1 +
 5 files changed, 44 insertions(+), 16 deletions(-)

-- 
1.8.1.191.g414c78c


^ permalink raw reply	[flat|nested] 56+ messages in thread

* [PATCH 1/2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided
  2013-08-08  8:09 ` [PATCH 0/2] scan /dev/mapper in filesystem show and device scan Anand Jain
@ 2013-08-08  8:09   ` Anand Jain
  2013-08-08  8:09   ` [PATCH 2/2] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
  2013-08-08  8:10   ` [PATCH 0/2] " anand jain
  2 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-08-08  8:09 UTC (permalink / raw)
  To: linux-btrfs

This is preparatory work to introduce /dev/mapper path usage

we need btrfs_scan_one_dir to san devs under /dev/mapper,
but /dev/mapper has links to the actual devs and current implementation
of btrfs_scan_one_dir skips links so it does not pick any
dev under /dev/mapper. skip the links are fine when scanning whole of
/dev But not when we just want to scan /dev/mapper

This patch just adds to check if we are scanning devs or
/dev/mapper only, if when latter it will not skip links

Thanks

v2: changes as per David review

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/utils.c b/utils.c
index 8a57967..038e599 100644
--- a/utils.c
+++ b/utils.c
@@ -1039,13 +1039,26 @@ int btrfs_scan_one_dir(char *dirname, int run_ioctl)
 	struct list_head pending_list;
 	struct btrfs_fs_devices *tmp_devices;
 	u64 num_devices;
+	int skip_link = 1;
+	char rdir[PATH_MAX];
+	char rdirp = NULL;
 
 	INIT_LIST_HEAD(&pending_list);
 
 	pending = malloc(sizeof(*pending));
 	if (!pending)
 		return -ENOMEM;
-	strcpy(pending->name, dirname);
+
+	rdirp = realpath(dirname, rdir);
+	if (!rdirp) {
+		free(pending);
+		return -errno;
+	}
+
+	strcpy(pending->name, rdir);
+
+	if (!strcmp(rdir, "/dev/mapper"))
+		skip_link = 0;
 
 again:
 	dirname_len = strlen(pending->name);
@@ -1078,7 +1091,7 @@ again:
 			fprintf(stderr, "failed to stat %s\n", fullpath);
 			continue;
 		}
-		if (S_ISLNK(st.st_mode))
+		if (skip_link && S_ISLNK(st.st_mode))
 			continue;
 		if (S_ISDIR(st.st_mode)) {
 			struct pending_dir *next = malloc(sizeof(*next));
@@ -1089,7 +1102,7 @@ again:
 			strcpy(next->name, fullpath);
 			list_add_tail(&next->list, &pending_list);
 		}
-		if (!S_ISBLK(st.st_mode)) {
+		if (skip_link && !S_ISBLK(st.st_mode)) {
 			continue;
 		}
 		fd = open(fullpath, O_RDONLY);
-- 
1.8.1.191.g414c78c

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* [PATCH 2/2] btrfs-progs: scan /dev/mapper in filesystem show and device scan
  2013-08-08  8:09 ` [PATCH 0/2] scan /dev/mapper in filesystem show and device scan Anand Jain
  2013-08-08  8:09   ` [PATCH 1/2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
@ 2013-08-08  8:09   ` Anand Jain
  2013-08-08  8:10   ` [PATCH 0/2] " anand jain
  2 siblings, 0 replies; 56+ messages in thread
From: Anand Jain @ 2013-08-08  8:09 UTC (permalink / raw)
  To: linux-btrfs

Currently, btrsf fi show and btrfs dev scan uses
/proc/partitions (by default) (which gives priority
to dm-<x> over sd<y> paths) and with --all-devices it
will scan /dev only (where it skips links under /dev/mapper).

However using /dev/mapper paths are in common practice
with mount, fstab, and lvm, so its better to be consistent
with them.

This patch adds --mapper option to btrfs device scan and
btrfs filesystem show cli, when used will look for btrfs
devs under /dev/mapper and will use the links provided
under the /dev/mapper.

eg:
btrfs fi show --mapper
Label: none  uuid: 0a621111-ad84-4d80-842a-dd9c1c60bf51
        Total devices 2 FS bytes used 1.17MB
        devid    1 size 44.99GB used 2.04GB path /dev/mapper/mpathe
        devid    2 size 48.23GB used 2.03GB path /dev/mapper/mpathd

Label: none  uuid: bad9105f-bdc6-4626-9ba7-80bd97aebe19
        Total devices 1 FS bytes used 28.00KB
        devid    1 size 15.00GB used 2.04GB path /dev/mapper/mpathbp1

In the long run mapper path when present (along with /proc/partitions)
can be the default option to scan for the btrfs devs.
(/proc/partitions must be scanned as well because to
include the mapper blacklisted (from mapper) devs.)

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c     |  8 +++++++-
 cmds-filesystem.c |  7 +++++--
 man/btrfs.8.in    | 22 ++++++++++++----------
 utils.c           |  3 +++
 utils.h           |  1 +
 5 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index be2aaff..6d1b378 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -186,7 +186,7 @@ static int cmd_rm_dev(int argc, char **argv)
 }
 
 static const char * const cmd_scan_dev_usage[] = {
-	"btrfs device scan [<--all-devices>|<device> [<device>...]]",
+	"btrfs device scan [<--all-devices>|<--mapper>|<device> [<device>...]]",
 	"Scan devices for a btrfs filesystem",
 	NULL
 };
@@ -203,6 +203,12 @@ static int cmd_scan_dev(int argc, char **argv)
 
 		where = BTRFS_SCAN_DEV;
 		devstart += 1;
+	} else if( argc > 1 && !strcmp(argv[1],"--mapper")){
+		if (check_argc_max(argc, 2))
+			usage(cmd_scan_dev_usage);
+
+		where = BTRFS_SCAN_MAPPER;
+		devstart += 1;
 	}
 
 	if(argc<=devstart){
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 74ad30b..88cace3 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -371,7 +371,7 @@ static int btrfs_scan_kernel(void *input, int type)
 }
 
 static const char * const cmd_show_usage[] = {
-	"btrfs filesystem show [--all-devices|--mapper|--kernel|<uuid>]",
+	"btrfs filesystem show [--all-devices|--mapper|--kernel] [<uuid>|<path>]",
 	"Show the structure of a filesystem",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
@@ -388,9 +388,12 @@ static int cmd_show(int argc, char **argv)
 	int searchstart = 1;
 	u8 processed[PATH_MAX];
 
-	if( argc > 1 && !strcmp(argv[1], "--all-devices")){
+	if (argc > 1 && !strcmp(argv[1], "--all-devices")){
 		where = BTRFS_SCAN_DEV;
 		searchstart += 1;
+	} else if (argc > 1 && !strcmp(argv[1], "--mapper")) {
+		where = BTRFS_SCAN_MAPPER;
+		searchstart += 1;
 	} else if (argc > 1 && !strcmp(argv[1], "--kernel")) {
 		where = 0;
 		searchstart += 1;
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 6383469..821f138 100644
--- a/man/btrfs.8.in
+++ b/man/btrfs.8.in
@@ -25,7 +25,7 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBfilesystem df\fP\fI <path>\fP
 .PP
-\fBbtrfs\fP \fBfilesystem show\fP\fI [--all-devices|--kernel] [\fI<uuid>|<path>]\fP\fP
+\fBbtrfs\fP \fBfilesystem show [\fP\fI--all-devices\fP|\fI--mapper\fP|\fI--kernel\fP] [\fI<uuid>\fP|\fI<path>\fP]
 .PP
 \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
 .PP
@@ -51,7 +51,7 @@ btrfs \- control a btrfs filesystem
 .PP
 \fBbtrfs\fP \fBdevice delete\fP \fI<device>\fP [\fI<device>...\fP] \fI<path>\fP
 .PP
-\fBbtrfs\fP \fBdevice scan\fP [--all-devices|\fI<device> \fP[\fI<device>...\fP]
+\fBbtrfs\fP \fBdevice scan\fP [\fI--all-devices\fP|\fI--mapper\fP|\fI<device>\fP [\fI<device>...\fP]
 .PP
 \fBbtrfs\fP \fBdevice ready\fP\fI <device>\fP
 .PP
@@ -254,12 +254,13 @@ Show information of a given subvolume in the \fI<path>\fR.
 Show space usage information for a mount point.
 .TP
 
-\fBfilesystem show\fR [--all-devices|--kernel] [\fI<uuid>|<path>]\fP\fP
-Show the btrfs filesystem with some additional info. If no \fIUUID\fP or
-\fIlabel\fP is passed, \fBbtrfs\fR show info of all the btrfs filesystem.
-If \fB--all-devices\fP is passed, all the devices under /dev are scanned;
+\fBfilesystem show\fR [\fI--all-devices\fP|\fI--mapper\fP|\fI--kernel\fP] [\fI<uuid>\fP|\fI<path>\fP]\fP
+Show the btrfs filesystem with some additional info. If no \fIuuid\fP
+is passed, it will show info of all the btrfs filesystem.
+If \fI--all-devices\fP is passed, all the devices under /dev are scanned;
+If \fI--mapper\fP is passed, the devices under /dev/mapper are scanned;
 otherwise the devices list is extracted from the /proc/partitions file.
-The --kernel will scan the btrfs kernel for the mounted btrfs.
+The \fI--kernel\fP will scan the btrfs kernel for the mounted btrfs.
 .TP
 
 \fBfilesystem sync\fR\fI <path> \fR
@@ -390,11 +391,12 @@ Add device(s) to the filesystem identified by \fI<path>\fR.
 Remove device(s) from a filesystem identified by \fI<path>\fR.
 .TP
 
-\fBdevice scan\fR [--all-devices|\fI<device> \fP[\fI<device>...\fP]\fR
+\fBdevice scan\fR [\fI--all-devices\fP|\fI--mapper\fP|\fI<device>\fP...]\fR
 If one or more devices are passed, these are scanned for a btrfs filesystem. 
-If no devices are passed, \fBbtrfs\fR scans all the block devices listed
+If no devices are passed, then it will scans all the block devices listed
 in the /proc/partitions file.
-Finally, if \fB--all-devices\fP is passed, all the devices under /dev are 
+If \fI--mapper\fP is passed, all the devices under /dev/mapper are scanned
+Finally, if \fI--all-devices\fP is passed, all the devices under /dev are 
 scanned.
 .TP
 
diff --git a/utils.c b/utils.c
index 038e599..382d102 100644
--- a/utils.c
+++ b/utils.c
@@ -1923,6 +1923,9 @@ int scan_for_btrfs(int where, int update_kernel)
 	case BTRFS_SCAN_DEV:
 		ret = btrfs_scan_one_dir("/dev", update_kernel);
 		break;
+	case BTRFS_SCAN_MAPPER:
+		ret = btrfs_scan_one_dir("/dev/mapper", update_kernel);
+		break;
 	}
 	return ret;
 }
diff --git a/utils.h b/utils.h
index 6419c3e..13e2fd5 100644
--- a/utils.h
+++ b/utils.h
@@ -27,6 +27,7 @@
 
 #define BTRFS_SCAN_PROC	1
 #define BTRFS_SCAN_DEV		2
+#define BTRFS_SCAN_MAPPER	3
 
 #define BTRFS_ERR_STR_LEN	100
 
-- 
1.8.1.191.g414c78c

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo

^ permalink raw reply related	[flat|nested] 56+ messages in thread

* Re: [PATCH 0/2] scan /dev/mapper in filesystem show and device scan
  2013-08-08  8:09 ` [PATCH 0/2] scan /dev/mapper in filesystem show and device scan Anand Jain
  2013-08-08  8:09   ` [PATCH 1/2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
  2013-08-08  8:09   ` [PATCH 2/2] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
@ 2013-08-08  8:10   ` anand jain
  2 siblings, 0 replies; 56+ messages in thread
From: anand jain @ 2013-08-08  8:10 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs



  Oh., I missed the libblkid part of David recommendation.
  I will be rewriting this patch set. sorry about that.

Thanks, Anand

On 08/08/2013 16:09, Anand Jain wrote:
> This patch brings the /dev/mapper to be used as the path for
> the btrfs kernel through dev scan
> 1/2 is the preparatory patch
>
> Anand Jain (2):
>    btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is
>      provided
>    btrfs-progs: scan /dev/mapper in filesystem show and device scan
>
>   cmds-device.c     |  8 +++++++-
>   cmds-filesystem.c |  7 +++++--
>   man/btrfs.8.in    | 22 ++++++++++++----------
>   utils.c           | 22 +++++++++++++++++++---
>   utils.h           |  1 +
>   5 files changed, 44 insertions(+), 16 deletions(-)
>

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel
  2013-08-08  8:07   ` [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
@ 2013-08-08 18:08     ` Zach Brown
  2013-08-09 10:57       ` anand jain
  0 siblings, 1 reply; 56+ messages in thread
From: Zach Brown @ 2013-08-08 18:08 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Thu, Aug 08, 2013 at 04:07:07PM +0800, Anand Jain wrote:
> As of now btrfs filesystem show reads directly from
> disks. So sometimes output can be stale, mainly when
> user want to verify their last operation like,
> labeling or device delete or add... etc.
> 
> This patch adds --kernel option to the 'filesystem show'
> subcli, which will read from the kernel instead of
> the disks directly.

Why should this be an option?

When mounted, the kernel cache is authoritative.  It was always a bug to
read stale data from disk.

The kernel should be read first, and if that isn't available it can fall
back to offering unreliable data from disk with a giant wraning.

Right?

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel
  2013-08-08 18:08     ` Zach Brown
@ 2013-08-09 10:57       ` anand jain
  2013-08-09 18:03         ` Zach Brown
  0 siblings, 1 reply; 56+ messages in thread
From: anand jain @ 2013-08-09 10:57 UTC (permalink / raw)
  To: Zach Brown; +Cc: linux-btrfs




On 09/08/2013 02:08, Zach Brown wrote:
> On Thu, Aug 08, 2013 at 04:07:07PM +0800, Anand Jain wrote:
>> As of now btrfs filesystem show reads directly from
>> disks. So sometimes output can be stale, mainly when
>> user want to verify their last operation like,
>> labeling or device delete or add... etc.
>>
>> This patch adds --kernel option to the 'filesystem show'
>> subcli, which will read from the kernel instead of
>> the disks directly.
>
> Why should this be an option?
>
> When mounted, the kernel cache is authoritative.  It was always a bug to
> read stale data from disk.
>
> The kernel should be read first, and if that isn't available it can fall
> back to offering unreliable data from disk with a giant wraning.
>
> Right?

  Yes !
  For the no option - what I like is more an intelligent way
  of listing all btrfs - mounted and unmounted by appropriately
  reading from the kernel and disks respectively.

  However to do this, we need a better disk list with in btrfs-progs
  which isn't there yet. I am getting those related components ready
  to be there someday.

  So at the moment an interim solution for the bug, which will
  also help the long term design.

Thanks, Anand
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel
  2013-08-09 10:57       ` anand jain
@ 2013-08-09 18:03         ` Zach Brown
  0 siblings, 0 replies; 56+ messages in thread
From: Zach Brown @ 2013-08-09 18:03 UTC (permalink / raw)
  To: anand jain; +Cc: linux-btrfs

> >Right?
> 
>  Yes !
>  For the no option - what I like is more an intelligent way
>  of listing all btrfs - mounted and unmounted by appropriately
>  reading from the kernel and disks respectively.

Great!

It'll be good to get this fixed.  I stumbled across this behaviour of
btrfs fi show lying to me just yesterday.

- z

^ permalink raw reply	[flat|nested] 56+ messages in thread

* Re: [PATCH 06/11] btrfs-progs: scan /dev/mapper in filesystem show and device scan
  2013-08-05 17:04     ` David Sterba
@ 2013-08-13  4:07       ` anand jain
  0 siblings, 0 replies; 56+ messages in thread
From: anand jain @ 2013-08-13  4:07 UTC (permalink / raw)
  To: dsterba, linux-btrfs



On 06/08/2013 01:04, David Sterba wrote:
> On Mon, Jul 15, 2013 at 01:30:52PM +0800, Anand Jain wrote:
>> This patch adds --mapper option to btrfs device scan and
>> btrfs filesystem show cli, when used will look for btrfs
>> devs under /dev/mapper and will use the links provided
>> under the /dev/mapper.
>
>> In the long run I want the usage of /dev/mapper path
>> (along with /proc/partitions) be the default option to
>> scan for the btrfs devs. (/proc/partitions must be scanned
>> as well because to include the mapper blacklisted devs.)
>
> Well, we want to avoid using own scanning and always consult the blkid
> cache, so I'd rather stop adding user-visible changes to this command.




  here is a test prog "t" (attached inline below) using blkid.
  t picks up both non multipath path and mapper path as below.
----
# ./t
Device: /dev/sdb        b03095dc-8eb5-40c9-8790-da6977110799
Device: /dev/sdc        abc347f2-dfcc-46bf-a4ac-e21fba7ff1a3
Device: /dev/mapper/mpatha      b03095dc-8eb5-40c9-8790-da6977110799
Device: /dev/mapper/mpathb      abc347f2-dfcc-46bf-a4ac-e21fba7ff1a3
Found 4
------

  However when

  /etc/multipath.conf
  defaults {
         user_friendly_names no
  }

  then the mapper paths are not friendly enough to use
-------
# ./t
Device: /dev/sdb        b03095dc-8eb5-40c9-8790-da6977110799
Device: /dev/sdc        abc347f2-dfcc-46bf-a4ac-e21fba7ff1a3
Device: /dev/mapper/1ATA     VBOX HARDDISK VB96b6139c-83f38bf1 
b03095dc-8eb5-40c9-8790-da6977110799
Device: /dev/mapper/1ATA     VBOX HARDDISK VBc6ab3781-f63da170 
abc347f2-dfcc-46bf-a4ac-e21fba7ff1a3
Found 4
------

  in this case the /dev/dm-<n> would have been better to use.
  and our own scan does the better job here.. as it
  reads /proc/partition and gives priority to dm-<n> paths
------
btrfs fi show
Label: none  uuid: abc347f2-dfcc-46bf-a4ac-e21fba7ff1a3
         Total devices 1 FS bytes used 28.00KiB
         devid    1 size 2.00GiB used 240.75MiB path /dev/dm-1

Label: none  uuid: b03095dc-8eb5-40c9-8790-da6977110799
         Total devices 1 FS bytes used 28.00KiB
         devid    1 size 1.98GiB used 238.25MiB path /dev/dm-0
-------

  so as of now having our scan is better than blkid.

  Now for the users who want to use the user friendly name
  provided by mapper. the newly introduced option --mapper
  which just looks under /dev/mapper will help.

Thanks, Anand


----------------t.c--------------
#include <stdio.h>
#include <blkid/blkid.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <string.h>

main()
{
         char *type;
         char str[100];
         int total = 0;
         blkid_dev_iterate iter = NULL;
         blkid_dev dev = NULL;
         blkid_cache cache = NULL;

         memset(str, '0', 100);
         if (blkid_get_cache(&cache, 0) < 0)
                 return -1;

         blkid_probe_all(cache);
         iter = blkid_dev_iterate_begin(cache);
         //blkid_dev_set_search(iter, NULL, NULL);
         blkid_dev_set_search(iter, "TYPE", "btrfs");

         while (blkid_dev_next(iter, &dev) == 0) {
                 dev = blkid_verify(cache, dev);
                 if (!dev)
                         continue;
                 else {
                         total++;
                         printf("Device: %s\t%s\n",
                 blkid_dev_devname(dev),
                                 blkid_get_tag_value(cache,"UUID",
                                 blkid_dev_devname(dev)));
                 }
         }
         blkid_dev_iterate_end(iter);
         printf("Found %d\n", total);
         return total;
}


^ permalink raw reply	[flat|nested] 56+ messages in thread

end of thread, other threads:[~2013-08-13  4:07 UTC | newest]

Thread overview: 56+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-10 14:56 [PATCH 0/9] btrfs-progs: coalesce of patches Anand Jain
2013-06-10 14:56 ` [PATCH 1/9] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
2013-06-10 20:00   ` Eric Sandeen
2013-06-11 13:15     ` anand jain
2013-06-10 14:56 ` [PATCH 2/9 v2] btrfs-progs: label option in btrfs filesystem show is not coded Anand Jain
2013-06-10 14:56 ` [PATCH 3/9 v2] btrfs-progs: update device scan usage Anand Jain
2013-06-10 14:56 ` [PATCH 4/9 v3] btrfs-progs: congregate dev scan Anand Jain
2013-06-10 14:56 ` [PATCH 5/9 v2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
2013-06-10 14:56 ` [PATCH 6/9 v2] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
2013-06-10 14:56 ` [PATCH 7/9 v3] btrfs-progs: device delete to get errors from the kernel Anand Jain
2013-06-10 14:56 ` [PATCH 8/9] btrfs-progs: get_label_mounted to return label instead of print Anand Jain
2013-06-21  7:41   ` [PATCH 08/13 v2] " Anand Jain
2013-06-10 14:56 ` [PATCH 9/9 v2] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
2013-06-10 14:59 ` [PATCH 0/2] btrfs: coalesce of patches Anand Jain
2013-06-10 14:59   ` [PATCH 1/2] btrfs: device delete to get errors from the kernel Anand Jain
2013-06-10 14:59   ` [PATCH 2/2 v2] btrfs: add framework to read fs info and dev info " Anand Jain
2013-06-10 19:40     ` Josef Bacik
2013-06-11 13:10       ` anand jain
2013-06-11 13:15         ` Josef Bacik
2013-06-10 20:30     ` Zach Brown
2013-06-11 14:05       ` anand jain
2013-06-11 17:50         ` Zach Brown
2013-06-11 14:24     ` Josef Bacik
2013-06-21  7:02       ` Anand Jain
2013-06-21  7:32     ` [PATCH 2/2 v3] btrfs: obtain used_bytes in BTRFS_IOC_FS_INFO ioctl Anand Jain
2013-06-24 17:03       ` Josef Bacik
2013-06-25  3:00         ` Anand Jain
2013-07-08  7:39 ` [PATCH 13/13] btrfs-progs: fix memory leaks of device_list_add() Anand Jain
2013-07-15  4:58   ` Anand Jain
2013-07-15  5:30 ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches Anand Jain
2013-07-15  5:30   ` [PATCH 01/11] btrfs-progs: btrfs_scan_for_fsid doesn't need all the arguments Anand Jain
2013-07-15  5:30   ` [PATCH 02/11] btrfs-progs: label option in btrfs filesystem show is not coded Anand Jain
2013-07-15  5:30   ` [PATCH 03/11] btrfs-progs: update device scan usage Anand Jain
2013-07-15  5:30   ` [PATCH 04/11] btrfs-progs: congregate dev scan Anand Jain
2013-07-15  5:30   ` [PATCH 05/11] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
2013-08-05 16:53     ` David Sterba
2013-07-15  5:30   ` [PATCH 06/11] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
2013-08-05 17:04     ` David Sterba
2013-08-13  4:07       ` anand jain
2013-07-15  5:30   ` [PATCH 07/11] btrfs-progs: device delete to get errors from the kernel Anand Jain
2013-07-15  5:30   ` [PATCH 08/11] btrfs-progs: get_label_mounted to return label instead of print Anand Jain
2013-07-15  5:30   ` [PATCH 09/11] btrfs-progs: move out print in cmd_df to another function Anand Jain
2013-07-15  5:30   ` [PATCH 10/11] btrfs-progs: get string for the group profile and type Anand Jain
2013-07-15  5:30   ` [PATCH 11/11] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
2013-08-05 17:36   ` [PATCH 00/11 v2 (resend)] btrfs-progs: coalesce of patches David Sterba
2013-08-06 15:08     ` anand jain
2013-08-08  8:07 ` [PATCH 0/2 v2] introduce btrfs filesystem show --kernel Anand Jain
2013-08-08  8:07   ` [PATCH 1/2] btrfs-progs: move out print in cmd_df to another function Anand Jain
2013-08-08  8:07   ` [PATCH 2/2] btrfs-progs: introduce btrfs filesystem show --kernel Anand Jain
2013-08-08 18:08     ` Zach Brown
2013-08-09 10:57       ` anand jain
2013-08-09 18:03         ` Zach Brown
2013-08-08  8:09 ` [PATCH 0/2] scan /dev/mapper in filesystem show and device scan Anand Jain
2013-08-08  8:09   ` [PATCH 1/2] btrfs-progs: btrfs_scan_one_dir not to skip links when /dev/mapper is provided Anand Jain
2013-08-08  8:09   ` [PATCH 2/2] btrfs-progs: scan /dev/mapper in filesystem show and device scan Anand Jain
2013-08-08  8:10   ` [PATCH 0/2] " anand jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.