[PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Qu Wenruo]

In btrfs/003 of xfstest, it will check whether btrfs fi show can find
missing devices.

But before the patch, btrfs-progs will not check whether device missing
if given a mounted btrfs mountpoint/block device.
This patch fixes the bug and will pass btrfs/003.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Cc: Anand Jain <anand.jain@oracle.com>
---
 cmds-filesystem.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 384d1b9..4c9933d 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -363,6 +363,8 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
 		char *label, char *path)
 {
 	int i;
+	int fd;
+	int missing;
 	char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
 	struct btrfs_ioctl_dev_info_args *tmp_dev_info;
 	int ret;
@@ -385,6 +387,14 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
 
 	for (i = 0; i < fs_info->num_devices; i++) {
 		tmp_dev_info = (struct btrfs_ioctl_dev_info_args *)&dev_info[i];
+
+		/* Add check for missing devices even mounted */
+		fd = open((char *)tmp_dev_info->path, O_RDONLY);
+		if (fd < 0) {
+			missing = 1;
+			continue;
+		}
+		close(fd);
 		printf("\tdevid %4llu size %s used %s path %s\n",
 			tmp_dev_info->devid,
 			pretty_size(tmp_dev_info->total_bytes),
@@ -392,6 +402,8 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
 			tmp_dev_info->path);
 	}
 
+	if (missing)
+		printf("\t*** Some devices missing\n");
 	printf("\n");
 	return 0;
 }
-- 
1.8.5.4

[PATCH 2/2] btrfs-progs: Add -p/--print-missing options for btrfs fi show

[Qu Wenruo]

Since a mounted btrfs filesystem contains all the devices info even a
device is removed after mount(like btrfs/003 in xfstests),
we can use the info to print the known missing device if possible.

So -p/--print-missing options are added to print possible missing
devices.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-filesystem.c | 26 ++++++++++++++++++++------
 man/btrfs.8.in    |  4 +++-
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 4c9933d..77b142c 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -360,7 +360,7 @@ static u64 calc_used_bytes(struct btrfs_ioctl_space_args *si)
 static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
 		struct btrfs_ioctl_dev_info_args *dev_info,
 		struct btrfs_ioctl_space_args *space_info,
-		char *label, char *path)
+		char *label, char *path, int print_missing)
 {
 	int i;
 	int fd;
@@ -392,7 +392,14 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
 		fd = open((char *)tmp_dev_info->path, O_RDONLY);
 		if (fd < 0) {
 			missing = 1;
-			continue;
+			if (print_missing)
+				printf("\tdevid %4llu size %s used %s path %s (missing)\n",
+				       tmp_dev_info->devid,
+				       pretty_size(tmp_dev_info->total_bytes),
+				       pretty_size(tmp_dev_info->bytes_used),
+				       tmp_dev_info->path);
+			else
+				continue;
 		}
 		close(fd);
 		printf("\tdevid %4llu size %s used %s path %s\n",
@@ -440,7 +447,7 @@ static int check_arg_type(char *input)
 	return BTRFS_ARG_UNKNOWN;
 }
 
-static int btrfs_scan_kernel(void *search)
+static int btrfs_scan_kernel(void *search, int print_missing)
 {
 	int ret = 0, fd;
 	FILE *f;
@@ -477,7 +484,8 @@ static int btrfs_scan_kernel(void *search)
 		fd = open(mnt->mnt_dir, O_RDONLY);
 		if ((fd != -1) && !get_df(fd, &space_info_arg)) {
 			print_one_fs(&fs_info_arg, dev_info_arg,
-					space_info_arg, label, mnt->mnt_dir);
+				     space_info_arg, label, mnt->mnt_dir,
+				     print_missing);
 			kfree(space_info_arg);
 			memset(label, 0, sizeof(label));
 		}
@@ -500,6 +508,7 @@ static const char * const cmd_show_usage[] = {
 	"Show the structure of a filesystem",
 	"-d|--all-devices   show only disks under /dev containing btrfs filesystem",
 	"-m|--mounted       show only mounted btrfs",
+	"-p|--print-missing show known missing device if possible",
 	"If no argument is given, structure of all present filesystems is shown.",
 	NULL
 };
@@ -513,6 +522,7 @@ static int cmd_show(int argc, char **argv)
 	int ret;
 	int where = BTRFS_SCAN_LBLKID;
 	int type = 0;
+	int print_missing = 0;
 	char mp[BTRFS_PATH_NAME_MAX + 1];
 	char path[PATH_MAX];
 
@@ -521,9 +531,10 @@ static int cmd_show(int argc, char **argv)
 		static struct option long_options[] = {
 			{ "all-devices", no_argument, NULL, 'd'},
 			{ "mounted", no_argument, NULL, 'm'},
+			{ "print-missing", no_argument, NULL, 'p'},
 			{ NULL, no_argument, NULL, 0 },
 		};
-		int c = getopt_long(argc, argv, "dm", long_options,
+		int c = getopt_long(argc, argv, "dmp", long_options,
 					&long_index);
 		if (c < 0)
 			break;
@@ -534,6 +545,9 @@ static int cmd_show(int argc, char **argv)
 		case 'm':
 			where = BTRFS_SCAN_MOUNTED;
 			break;
+		case 'p':
+			print_missing = 1;
+			break;
 		default:
 			usage(cmd_show_usage);
 		}
@@ -571,7 +585,7 @@ static int cmd_show(int argc, char **argv)
 		goto devs_only;
 
 	/* show mounted btrfs */
-	ret = btrfs_scan_kernel(search);
+	ret = btrfs_scan_kernel(search, print_missing);
 	if (search && !ret)
 		return 0;
 
diff --git a/man/btrfs.8.in b/man/btrfs.8.in
index 8fea115..db2e355 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--mounted\fP|\fI--all-devices\fP|\fI<uuid>\fP]\fP
+\fBbtrfs\fP \fBfilesystem show\fP [\fI--mounted\fP|\fI--all-devices\fP|\fI--print-missing\fP|\fI<uuid>\fP]\fP
 .PP
 \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
 .PP
@@ -280,6 +280,8 @@ and unmounted.
 If \fB--mounted\fP is passed, it would probe btrfs kernel to list mounted btrfs filesystem(s);
 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.
+If \fB--print-missing\fP is passed, btrfs will try its best to print the missing device.
+This option is only useful if the filesystem is already mounted since mounted filesystem may contain the device list.
 .TP
 
 \fBfilesystem sync\fR\fI <path> \fR
-- 
1.8.5.4

Re: [PATCH 2/2] btrfs-progs: Add -p/--print-missing options for btrfs fi show

[Anand Jain]

  Whats needed is more comprehensive btrfs fi show
  which shows the flags (including missing) per disk.
  And also show the FS/Raid status. Which I am working on.

  sorry -p feature would be covered by default in the
  coming revamp of btrfs fi show.

Thanks, Anand


On 02/07/2014 02:46 PM, Qu Wenruo wrote:
> Since a mounted btrfs filesystem contains all the devices info even a
> device is removed after mount(like btrfs/003 in xfstests),
> we can use the info to print the known missing device if possible.
>
> So -p/--print-missing options are added to print possible missing
> devices.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>   cmds-filesystem.c | 26 ++++++++++++++++++++------
>   man/btrfs.8.in    |  4 +++-
>   2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 4c9933d..77b142c 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -360,7 +360,7 @@ static u64 calc_used_bytes(struct btrfs_ioctl_space_args *si)
>   static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
>   		struct btrfs_ioctl_dev_info_args *dev_info,
>   		struct btrfs_ioctl_space_args *space_info,
> -		char *label, char *path)
> +		char *label, char *path, int print_missing)
>   {
>   	int i;
>   	int fd;
> @@ -392,7 +392,14 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
>   		fd = open((char *)tmp_dev_info->path, O_RDONLY);
>   		if (fd < 0) {
>   			missing = 1;
> -			continue;
> +			if (print_missing)
> +				printf("\tdevid %4llu size %s used %s path %s (missing)\n",
> +				       tmp_dev_info->devid,
> +				       pretty_size(tmp_dev_info->total_bytes),
> +				       pretty_size(tmp_dev_info->bytes_used),
> +				       tmp_dev_info->path);
> +			else
> +				continue;
>   		}
>   		close(fd);
>   		printf("\tdevid %4llu size %s used %s path %s\n",
> @@ -440,7 +447,7 @@ static int check_arg_type(char *input)
>   	return BTRFS_ARG_UNKNOWN;
>   }
>
> -static int btrfs_scan_kernel(void *search)
> +static int btrfs_scan_kernel(void *search, int print_missing)
>   {
>   	int ret = 0, fd;
>   	FILE *f;
> @@ -477,7 +484,8 @@ static int btrfs_scan_kernel(void *search)
>   		fd = open(mnt->mnt_dir, O_RDONLY);
>   		if ((fd != -1) && !get_df(fd, &space_info_arg)) {
>   			print_one_fs(&fs_info_arg, dev_info_arg,
> -					space_info_arg, label, mnt->mnt_dir);
> +				     space_info_arg, label, mnt->mnt_dir,
> +				     print_missing);
>   			kfree(space_info_arg);
>   			memset(label, 0, sizeof(label));
>   		}
> @@ -500,6 +508,7 @@ static const char * const cmd_show_usage[] = {
>   	"Show the structure of a filesystem",
>   	"-d|--all-devices   show only disks under /dev containing btrfs filesystem",
>   	"-m|--mounted       show only mounted btrfs",
> +	"-p|--print-missing show known missing device if possible",
>   	"If no argument is given, structure of all present filesystems is shown.",
>   	NULL
>   };
> @@ -513,6 +522,7 @@ static int cmd_show(int argc, char **argv)
>   	int ret;
>   	int where = BTRFS_SCAN_LBLKID;
>   	int type = 0;
> +	int print_missing = 0;
>   	char mp[BTRFS_PATH_NAME_MAX + 1];
>   	char path[PATH_MAX];
>
> @@ -521,9 +531,10 @@ static int cmd_show(int argc, char **argv)
>   		static struct option long_options[] = {
>   			{ "all-devices", no_argument, NULL, 'd'},
>   			{ "mounted", no_argument, NULL, 'm'},
> +			{ "print-missing", no_argument, NULL, 'p'},
>   			{ NULL, no_argument, NULL, 0 },
>   		};
> -		int c = getopt_long(argc, argv, "dm", long_options,
> +		int c = getopt_long(argc, argv, "dmp", long_options,
>   					&long_index);
>   		if (c < 0)
>   			break;
> @@ -534,6 +545,9 @@ static int cmd_show(int argc, char **argv)
>   		case 'm':
>   			where = BTRFS_SCAN_MOUNTED;
>   			break;
> +		case 'p':
> +			print_missing = 1;
> +			break;
>   		default:
>   			usage(cmd_show_usage);
>   		}
> @@ -571,7 +585,7 @@ static int cmd_show(int argc, char **argv)
>   		goto devs_only;
>
>   	/* show mounted btrfs */
> -	ret = btrfs_scan_kernel(search);
> +	ret = btrfs_scan_kernel(search, print_missing);
>   	if (search && !ret)
>   		return 0;
>
> diff --git a/man/btrfs.8.in b/man/btrfs.8.in
> index 8fea115..db2e355 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--mounted\fP|\fI--all-devices\fP|\fI<uuid>\fP]\fP
> +\fBbtrfs\fP \fBfilesystem show\fP [\fI--mounted\fP|\fI--all-devices\fP|\fI--print-missing\fP|\fI<uuid>\fP]\fP
>   .PP
>   \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
>   .PP
> @@ -280,6 +280,8 @@ and unmounted.
>   If \fB--mounted\fP is passed, it would probe btrfs kernel to list mounted btrfs filesystem(s);
>   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.
> +If \fB--print-missing\fP is passed, btrfs will try its best to print the missing device.
> +This option is only useful if the filesystem is already mounted since mounted filesystem may contain the device list.
>   .TP
>
>   \fBfilesystem sync\fR\fI <path> \fR
>

Re: [PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Anand Jain]

  IMO btrfs-progs shouldn't add its intelligence to know if disk
  is missing. If btrfs-kernel doesn't know when disk is missing
  that's a bug to fix in btrfs-kernel. yes that indeed true as
  of now in btrfs-kernel. btrfs kernel has no idea when disk
  goes missing, just -EIO doesn't tell btrfs that. I am trying
  to fix this first.

  But the problem is there isn't good way with in btrfs/FS
  to know when disk goes missing. did I miss anything ?


Thanks, Anand


On 02/07/2014 02:45 PM, Qu Wenruo wrote:
> In btrfs/003 of xfstest, it will check whether btrfs fi show can find
> missing devices.
>
> But before the patch, btrfs-progs will not check whether device missing
> if given a mounted btrfs mountpoint/block device.
> This patch fixes the bug and will pass btrfs/003.
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> Cc: Anand Jain <anand.jain@oracle.com>
> ---
>   cmds-filesystem.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 384d1b9..4c9933d 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -363,6 +363,8 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
>   		char *label, char *path)
>   {
>   	int i;
> +	int fd;
> +	int missing;
>   	char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>   	struct btrfs_ioctl_dev_info_args *tmp_dev_info;
>   	int ret;
> @@ -385,6 +387,14 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
>
>   	for (i = 0; i < fs_info->num_devices; i++) {
>   		tmp_dev_info = (struct btrfs_ioctl_dev_info_args *)&dev_info[i];
> +
> +		/* Add check for missing devices even mounted */
> +		fd = open((char *)tmp_dev_info->path, O_RDONLY);
> +		if (fd < 0) {
> +			missing = 1;
> +			continue;
> +		}
> +		close(fd);
>   		printf("\tdevid %4llu size %s used %s path %s\n",
>   			tmp_dev_info->devid,
>   			pretty_size(tmp_dev_info->total_bytes),
> @@ -392,6 +402,8 @@ static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
>   			tmp_dev_info->path);
>   	}
>
> +	if (missing)
> +		printf("\t*** Some devices missing\n");
>   	printf("\n");
>   	return 0;
>   }
>

Re: [PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Qu Wenruo]

On Fri, 07 Feb 2014 17:34:46 +0800, Anand Jain wrote:
>
>
>  IMO btrfs-progs shouldn't add its intelligence to know if disk
>  is missing. If btrfs-kernel doesn't know when disk is missing
>  that's a bug to fix in btrfs-kernel. yes that indeed true as
>  of now in btrfs-kernel. btrfs kernel has no idea when disk
>  goes missing, just -EIO doesn't tell btrfs that. I am trying
>  to fix this first.
>
>  But the problem is there isn't good way with in btrfs/FS
>  to know when disk goes missing. did I miss anything ?
Yes, kernel detection is the best way.
But since it has no better way to detect missing device, I think the 
btrfs-progs way fix is good enough for now.

Since btrfs fi show with "-d" options will scan the /dev to find fs and 
check missing disks,
I think adds some user-land check even using the ioctl way is still 
somewhat reasonable.

Thanks
Qu
>
>
> Thanks, Anand
>
>
> On 02/07/2014 02:45 PM, Qu Wenruo wrote:
>> In btrfs/003 of xfstest, it will check whether btrfs fi show can find
>> missing devices.
>>
>> But before the patch, btrfs-progs will not check whether device missing
>> if given a mounted btrfs mountpoint/block device.
>> This patch fixes the bug and will pass btrfs/003.
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> Cc: Anand Jain <anand.jain@oracle.com>
>> ---
>>   cmds-filesystem.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>> index 384d1b9..4c9933d 100644
>> --- a/cmds-filesystem.c
>> +++ b/cmds-filesystem.c
>> @@ -363,6 +363,8 @@ static int print_one_fs(struct 
>> btrfs_ioctl_fs_info_args *fs_info,
>>           char *label, char *path)
>>   {
>>       int i;
>> +    int fd;
>> +    int missing;
>>       char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>>       struct btrfs_ioctl_dev_info_args *tmp_dev_info;
>>       int ret;
>> @@ -385,6 +387,14 @@ static int print_one_fs(struct 
>> btrfs_ioctl_fs_info_args *fs_info,
>>
>>       for (i = 0; i < fs_info->num_devices; i++) {
>>           tmp_dev_info = (struct btrfs_ioctl_dev_info_args 
>> *)&dev_info[i];
>> +
>> +        /* Add check for missing devices even mounted */
>> +        fd = open((char *)tmp_dev_info->path, O_RDONLY);
>> +        if (fd < 0) {
>> +            missing = 1;
>> +            continue;
>> +        }
>> +        close(fd);
>>           printf("\tdevid %4llu size %s used %s path %s\n",
>>               tmp_dev_info->devid,
>>               pretty_size(tmp_dev_info->total_bytes),
>> @@ -392,6 +402,8 @@ static int print_one_fs(struct 
>> btrfs_ioctl_fs_info_args *fs_info,
>>               tmp_dev_info->path);
>>       }
>>
>> +    if (missing)
>> +        printf("\t*** Some devices missing\n");
>>       printf("\n");
>>       return 0;
>>   }
>>
>

Re: [PATCH 2/2] btrfs-progs: Add -p/--print-missing options for btrfs fi show

[Qu Wenruo]

On fri, 07 Feb 2014 17:26:11 +0800, Anand Jain wrote:
>
>  Whats needed is more comprehensive btrfs fi show
>  which shows the flags (including missing) per disk.
Yes indeed.
>  And also show the FS/Raid status. Which I am working on.
>
>  sorry -p feature would be covered by default in the
>  coming revamp of btrfs fi show.
That's all right.
Just a kind remind, if the output format changes, don't forget to modify 
the related xfstests testcase.

Thanks,
Qu
>
> Thanks, Anand
>
>
> On 02/07/2014 02:46 PM, Qu Wenruo wrote:
>> Since a mounted btrfs filesystem contains all the devices info even a
>> device is removed after mount(like btrfs/003 in xfstests),
>> we can use the info to print the known missing device if possible.
>>
>> So -p/--print-missing options are added to print possible missing
>> devices.
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>   cmds-filesystem.c | 26 ++++++++++++++++++++------
>>   man/btrfs.8.in    |  4 +++-
>>   2 files changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>> index 4c9933d..77b142c 100644
>> --- a/cmds-filesystem.c
>> +++ b/cmds-filesystem.c
>> @@ -360,7 +360,7 @@ static u64 calc_used_bytes(struct 
>> btrfs_ioctl_space_args *si)
>>   static int print_one_fs(struct btrfs_ioctl_fs_info_args *fs_info,
>>           struct btrfs_ioctl_dev_info_args *dev_info,
>>           struct btrfs_ioctl_space_args *space_info,
>> -        char *label, char *path)
>> +        char *label, char *path, int print_missing)
>>   {
>>       int i;
>>       int fd;
>> @@ -392,7 +392,14 @@ static int print_one_fs(struct 
>> btrfs_ioctl_fs_info_args *fs_info,
>>           fd = open((char *)tmp_dev_info->path, O_RDONLY);
>>           if (fd < 0) {
>>               missing = 1;
>> -            continue;
>> +            if (print_missing)
>> +                printf("\tdevid %4llu size %s used %s path %s 
>> (missing)\n",
>> +                       tmp_dev_info->devid,
>> + pretty_size(tmp_dev_info->total_bytes),
>> + pretty_size(tmp_dev_info->bytes_used),
>> +                       tmp_dev_info->path);
>> +            else
>> +                continue;
>>           }
>>           close(fd);
>>           printf("\tdevid %4llu size %s used %s path %s\n",
>> @@ -440,7 +447,7 @@ static int check_arg_type(char *input)
>>       return BTRFS_ARG_UNKNOWN;
>>   }
>>
>> -static int btrfs_scan_kernel(void *search)
>> +static int btrfs_scan_kernel(void *search, int print_missing)
>>   {
>>       int ret = 0, fd;
>>       FILE *f;
>> @@ -477,7 +484,8 @@ static int btrfs_scan_kernel(void *search)
>>           fd = open(mnt->mnt_dir, O_RDONLY);
>>           if ((fd != -1) && !get_df(fd, &space_info_arg)) {
>>               print_one_fs(&fs_info_arg, dev_info_arg,
>> -                    space_info_arg, label, mnt->mnt_dir);
>> +                     space_info_arg, label, mnt->mnt_dir,
>> +                     print_missing);
>>               kfree(space_info_arg);
>>               memset(label, 0, sizeof(label));
>>           }
>> @@ -500,6 +508,7 @@ static const char * const cmd_show_usage[] = {
>>       "Show the structure of a filesystem",
>>       "-d|--all-devices   show only disks under /dev containing btrfs 
>> filesystem",
>>       "-m|--mounted       show only mounted btrfs",
>> +    "-p|--print-missing show known missing device if possible",
>>       "If no argument is given, structure of all present filesystems 
>> is shown.",
>>       NULL
>>   };
>> @@ -513,6 +522,7 @@ static int cmd_show(int argc, char **argv)
>>       int ret;
>>       int where = BTRFS_SCAN_LBLKID;
>>       int type = 0;
>> +    int print_missing = 0;
>>       char mp[BTRFS_PATH_NAME_MAX + 1];
>>       char path[PATH_MAX];
>>
>> @@ -521,9 +531,10 @@ static int cmd_show(int argc, char **argv)
>>           static struct option long_options[] = {
>>               { "all-devices", no_argument, NULL, 'd'},
>>               { "mounted", no_argument, NULL, 'm'},
>> +            { "print-missing", no_argument, NULL, 'p'},
>>               { NULL, no_argument, NULL, 0 },
>>           };
>> -        int c = getopt_long(argc, argv, "dm", long_options,
>> +        int c = getopt_long(argc, argv, "dmp", long_options,
>>                       &long_index);
>>           if (c < 0)
>>               break;
>> @@ -534,6 +545,9 @@ static int cmd_show(int argc, char **argv)
>>           case 'm':
>>               where = BTRFS_SCAN_MOUNTED;
>>               break;
>> +        case 'p':
>> +            print_missing = 1;
>> +            break;
>>           default:
>>               usage(cmd_show_usage);
>>           }
>> @@ -571,7 +585,7 @@ static int cmd_show(int argc, char **argv)
>>           goto devs_only;
>>
>>       /* show mounted btrfs */
>> -    ret = btrfs_scan_kernel(search);
>> +    ret = btrfs_scan_kernel(search, print_missing);
>>       if (search && !ret)
>>           return 0;
>>
>> diff --git a/man/btrfs.8.in b/man/btrfs.8.in
>> index 8fea115..db2e355 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--mounted\fP|\fI--all-devices\fP|\fI<uuid>\fP]\fP
>> +\fBbtrfs\fP \fBfilesystem show\fP 
>> [\fI--mounted\fP|\fI--all-devices\fP|\fI--print-missing\fP|\fI<uuid>\fP]\fP
>>   .PP
>>   \fBbtrfs\fP \fBfilesystem sync\fP\fI <path> \fP
>>   .PP
>> @@ -280,6 +280,8 @@ and unmounted.
>>   If \fB--mounted\fP is passed, it would probe btrfs kernel to list 
>> mounted btrfs filesystem(s);
>>   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.
>> +If \fB--print-missing\fP is passed, btrfs will try its best to print 
>> the missing device.
>> +This option is only useful if the filesystem is already mounted 
>> since mounted filesystem may contain the device list.
>>   .TP
>>
>>   \fBfilesystem sync\fR\fI <path> \fR
>>
> -- 
> 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
>

Re: [PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Anand Jain]

  Below shows the bug cascading to this patch.

  And now to fix this I think we shouldn't fix/workaround in the
  btrfs-progs again!, fix it in the btrfs-kernel (or leave it open
  until suitable fix is found, I tried and failed. but don't fix it
  in a wrong way). If you want to help to fix this problem: Find out
  if we could get kobject notification with in kernel when disks gets
  disappeared.

  I have been advocating btrfs-progs should _not_ add its intelligence
  and it should be as transparent as possible in showing the kernel's
  status. This should be seriously considered.

(-----------
  For patches to take this approach the core problem here is different
  and hope we could correct it..
  First, we have a superficial and wrong measuring tape (xfstest) and
  we are trying to fix the product using it And in between is btrfs-progs
  which is trying to add more superficial-ness.
  2nd, btrfs Wiki has a theory and thus sets the direction that
  btrfs-progs would copy code from btrfs-kernel, I seriously doubt
  if that's a good idea.
  If you want to make btrfs-progs as intelligent as btrfs-kernel
  (which I don't understand why you should ?  since the purpose of
  btrfs-progs and btrfs-kernel are different) then first you need
  develop a mini synchronization mechanism between btrfs-progs and btrfs
  kernel which is as good as two active nodes FS which says from my
  experience with Solaris/SAM-QFS. Developing a synchronization
  mechanism is not in the plan here. Further from the End user
  Application (DB) performance perspective calling sync at the need of
  something like btrfs-progs is a very very bad idea. Applications would
  experience jitters in their steady state performance. Once Solaris had
  this issue and we fixed it.
-----------)

  Have fun. ;-)

----------------------------------------------------------------
$ btrfs dev scan
Scanning for Btrfs filesystems
$ mount /dev/sdc /btrfs
$ btrfs fi show
Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
	Total devices 2 FS bytes used 663.81MiB
	devid    1 size 1.10GiB used 1.10GiB path /dev/sdf
	devid    2 size 1.10GiB used 1.08GiB path /dev/sdc

$ devmgt show
host0 sda
host1 sdf
host2 sdc
host3 sdd
host4 sde
$ devmgt detach /dev/sdf
-----/dev/kmsg----
	sd 1:0:0:0: [sdf] Stopping disk
	 SUBSYSTEM=scsi
	 DEVICE=+scsi:1:0:0:0
	ata2.00: disabled
------------------
detach /dev/sdf successful

(as a known bug btrfs kernel does not know device is missing, missing 
flag isn't set, as shown below)

$ btrfs-devlist
fsid name uuid (seed_fsid sprout_fsid)
	(fs_latest_devid fs_num_devices fs_open_devices fs_rw_devices 
fs_missing_devices fs_total_devices) fs_total_rw_bytes 
fs_num_can_discard fs_latest_trans
	devid gen total_bytes disk_total_bytes bytes_used type io_align 
io_width sector_size fmode
	fs_flags
	dev_flags

dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdf 
13715cc5-3aeb-4523-b02c-a072fd427a00 (null null)
	(2 2 2 2 0 2) 2363490304 0 7
	1 5 1181745152 1181745152 1181745152 0 4096 4096 4096 0x83
	fs_Mounted|not_fs_Seeding|fs_Rotating
	Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev

dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdc 
12ad34f7-8d58-44fa-95cf-b2bbc0cec69d (null null)
	(2 2 2 2 0 2) 2363490304 0 7
	2 7 1181745152 1181745152 1160773632 0 4096 4096 4096 0x83
	fs_Mounted|not_fs_Seeding|fs_Rotating
	Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev


(below btrfs-progs patch added intelligence to tell the world that 
device is missing)

Ref:
~~~~~~~
commit 2ae6a037efd52ae0fa30052d456ad07f074f5d54
Author: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date:   Fri Feb 7 15:07:19 2014 +0800

     btrfs-progs: Add missing devices check for mounted btrfs.
~~~~~~~

$ btrfs fi show
Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
	Total devices 2 FS bytes used 663.81MiB
	devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
	*** Some devices missing

$ btrfs dev add /dev/sde /btrfs
$ btrfs fi show
Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
	Total devices 3 FS bytes used 663.81MiB
	devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
	devid    3 size 1.04GiB used 0.00 path /dev/sde
	*** Some devices missing


Now the bug is delete missing fails. Since kernel don't
understand whats missing.

$ btrfs dev del missing /btrfs
ERROR: error removing the device 'missing' - no missing devices found to 
remove
$
---------------------------------------------------------------------------





On 02/10/2014 08:36 AM, Qu Wenruo wrote:
> On Fri, 07 Feb 2014 17:34:46 +0800, Anand Jain wrote:
>>
>>
>>  IMO btrfs-progs shouldn't add its intelligence to know if disk
>>  is missing. If btrfs-kernel doesn't know when disk is missing
>>  that's a bug to fix in btrfs-kernel. yes that indeed true as
>>  of now in btrfs-kernel. btrfs kernel has no idea when disk
>>  goes missing, just -EIO doesn't tell btrfs that. I am trying
>>  to fix this first.
>>
>>  But the problem is there isn't good way with in btrfs/FS
>>  to know when disk goes missing. did I miss anything ?
> Yes, kernel detection is the best way.
> But since it has no better way to detect missing device, I think the
> btrfs-progs way fix is good enough for now.
>
> Since btrfs fi show with "-d" options will scan the /dev to find fs and
> check missing disks,
> I think adds some user-land check even using the ioctl way is still
> somewhat reasonable.
>
> Thanks
> Qu
>>
>>
>> Thanks, Anand
>>
>>
>> On 02/07/2014 02:45 PM, Qu Wenruo wrote:
>>> In btrfs/003 of xfstest, it will check whether btrfs fi show can find
>>> missing devices.
>>>
>>> But before the patch, btrfs-progs will not check whether device missing
>>> if given a mounted btrfs mountpoint/block device.
>>> This patch fixes the bug and will pass btrfs/003.
>>>
>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> Cc: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   cmds-filesystem.c | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>>> index 384d1b9..4c9933d 100644
>>> --- a/cmds-filesystem.c
>>> +++ b/cmds-filesystem.c
>>> @@ -363,6 +363,8 @@ static int print_one_fs(struct
>>> btrfs_ioctl_fs_info_args *fs_info,
>>>           char *label, char *path)
>>>   {
>>>       int i;
>>> +    int fd;
>>> +    int missing;
>>>       char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>>>       struct btrfs_ioctl_dev_info_args *tmp_dev_info;
>>>       int ret;
>>> @@ -385,6 +387,14 @@ static int print_one_fs(struct
>>> btrfs_ioctl_fs_info_args *fs_info,
>>>
>>>       for (i = 0; i < fs_info->num_devices; i++) {
>>>           tmp_dev_info = (struct btrfs_ioctl_dev_info_args
>>> *)&dev_info[i];
>>> +
>>> +        /* Add check for missing devices even mounted */
>>> +        fd = open((char *)tmp_dev_info->path, O_RDONLY);
>>> +        if (fd < 0) {
>>> +            missing = 1;
>>> +            continue;
>>> +        }
>>> +        close(fd);
>>>           printf("\tdevid %4llu size %s used %s path %s\n",
>>>               tmp_dev_info->devid,
>>>               pretty_size(tmp_dev_info->total_bytes),
>>> @@ -392,6 +402,8 @@ static int print_one_fs(struct
>>> btrfs_ioctl_fs_info_args *fs_info,
>>>               tmp_dev_info->path);
>>>       }
>>>
>>> +    if (missing)
>>> +        printf("\t*** Some devices missing\n");
>>>       printf("\n");
>>>       return 0;
>>>   }
>>>
>>
>
> --
> 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

Re: [PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Qu Wenruo]

Yes, the deleted device scan is still one of the deep problems yet.

But my patch is not intented to deal anything related to the problem.

For me, I am *only* going to deal with the *exit code* problem,
'btrfs fi show <device>' executes correctly(OK, only part of it exactlly),
but exit code is still 1, which is the bug I'm trying to fix it.

IMO, the users/admins may never be interested in the inside mechanism 
nor algorithm,
but the output and exit value things.

This bug is much like the previous 'btrfs fi show' bugs that breaks some 
xfstests test case
(always showing a error due to the scan_kernel_v2 function which calls a 
unimplemented ioctl interface),
if some patch breaks the old exit code or output, then it should be 
fixed to maintain the old
output/exit code (except some big decision is made to change it).

So for the output/exit code consistence, it should be fixed even the 
patch may not  means a cure but
only a workaround for you.

Thanks,
Qu.

于 2014年04月09日 11:04, Anand Jain 写道:
>
>
>  Below shows the bug cascading to this patch.
>
>  And now to fix this I think we shouldn't fix/workaround in the
>  btrfs-progs again!, fix it in the btrfs-kernel (or leave it open
>  until suitable fix is found, I tried and failed. but don't fix it
>  in a wrong way). If you want to help to fix this problem: Find out
>  if we could get kobject notification with in kernel when disks gets
>  disappeared.
>
>  I have been advocating btrfs-progs should _not_ add its intelligence
>  and it should be as transparent as possible in showing the kernel's
>  status. This should be seriously considered.
>
> (-----------
>  For patches to take this approach the core problem here is different
>  and hope we could correct it..
>  First, we have a superficial and wrong measuring tape (xfstest) and
>  we are trying to fix the product using it And in between is btrfs-progs
>  which is trying to add more superficial-ness.
>  2nd, btrfs Wiki has a theory and thus sets the direction that
>  btrfs-progs would copy code from btrfs-kernel, I seriously doubt
>  if that's a good idea.
>  If you want to make btrfs-progs as intelligent as btrfs-kernel
>  (which I don't understand why you should ?  since the purpose of
>  btrfs-progs and btrfs-kernel are different) then first you need
>  develop a mini synchronization mechanism between btrfs-progs and btrfs
>  kernel which is as good as two active nodes FS which says from my
>  experience with Solaris/SAM-QFS. Developing a synchronization
>  mechanism is not in the plan here. Further from the End user
>  Application (DB) performance perspective calling sync at the need of
>  something like btrfs-progs is a very very bad idea. Applications would
>  experience jitters in their steady state performance. Once Solaris had
>  this issue and we fixed it.
> -----------)
>
>  Have fun. ;-)
>
> ----------------------------------------------------------------
> $ btrfs dev scan
> Scanning for Btrfs filesystems
> $ mount /dev/sdc /btrfs
> $ btrfs fi show
> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>     Total devices 2 FS bytes used 663.81MiB
>     devid    1 size 1.10GiB used 1.10GiB path /dev/sdf
>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>
> $ devmgt show
> host0 sda
> host1 sdf
> host2 sdc
> host3 sdd
> host4 sde
> $ devmgt detach /dev/sdf
> -----/dev/kmsg----
>     sd 1:0:0:0: [sdf] Stopping disk
>      SUBSYSTEM=scsi
>      DEVICE=+scsi:1:0:0:0
>     ata2.00: disabled
> ------------------
> detach /dev/sdf successful
>
> (as a known bug btrfs kernel does not know device is missing, missing 
> flag isn't set, as shown below)
>
> $ btrfs-devlist
> fsid name uuid (seed_fsid sprout_fsid)
>     (fs_latest_devid fs_num_devices fs_open_devices fs_rw_devices 
> fs_missing_devices fs_total_devices) fs_total_rw_bytes 
> fs_num_can_discard fs_latest_trans
>     devid gen total_bytes disk_total_bytes bytes_used type io_align 
> io_width sector_size fmode
>     fs_flags
>     dev_flags
>
> dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdf 
> 13715cc5-3aeb-4523-b02c-a072fd427a00 (null null)
>     (2 2 2 2 0 2) 2363490304 0 7
>     1 5 1181745152 1181745152 1181745152 0 4096 4096 4096 0x83
>     fs_Mounted|not_fs_Seeding|fs_Rotating
>     Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev 
>
>
> dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdc 
> 12ad34f7-8d58-44fa-95cf-b2bbc0cec69d (null null)
>     (2 2 2 2 0 2) 2363490304 0 7
>     2 7 1181745152 1181745152 1160773632 0 4096 4096 4096 0x83
>     fs_Mounted|not_fs_Seeding|fs_Rotating
>     Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev 
>
>
>
> (below btrfs-progs patch added intelligence to tell the world that 
> device is missing)
>
> Ref:
> ~~~~~~~
> commit 2ae6a037efd52ae0fa30052d456ad07f074f5d54
> Author: Qu Wenruo <quwenruo@cn.fujitsu.com>
> Date:   Fri Feb 7 15:07:19 2014 +0800
>
>     btrfs-progs: Add missing devices check for mounted btrfs.
> ~~~~~~~
>
> $ btrfs fi show
> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>     Total devices 2 FS bytes used 663.81MiB
>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>     *** Some devices missing
>
> $ btrfs dev add /dev/sde /btrfs
> $ btrfs fi show
> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>     Total devices 3 FS bytes used 663.81MiB
>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>     devid    3 size 1.04GiB used 0.00 path /dev/sde
>     *** Some devices missing
>
>
> Now the bug is delete missing fails. Since kernel don't
> understand whats missing.
>
> $ btrfs dev del missing /btrfs
> ERROR: error removing the device 'missing' - no missing devices found 
> to remove
> $
> --------------------------------------------------------------------------- 
>
>
>
>
>
>
> On 02/10/2014 08:36 AM, Qu Wenruo wrote:
>> On Fri, 07 Feb 2014 17:34:46 +0800, Anand Jain wrote:
>>>
>>>
>>>  IMO btrfs-progs shouldn't add its intelligence to know if disk
>>>  is missing. If btrfs-kernel doesn't know when disk is missing
>>>  that's a bug to fix in btrfs-kernel. yes that indeed true as
>>>  of now in btrfs-kernel. btrfs kernel has no idea when disk
>>>  goes missing, just -EIO doesn't tell btrfs that. I am trying
>>>  to fix this first.
>>>
>>>  But the problem is there isn't good way with in btrfs/FS
>>>  to know when disk goes missing. did I miss anything ?
>> Yes, kernel detection is the best way.
>> But since it has no better way to detect missing device, I think the
>> btrfs-progs way fix is good enough for now.
>>
>> Since btrfs fi show with "-d" options will scan the /dev to find fs and
>> check missing disks,
>> I think adds some user-land check even using the ioctl way is still
>> somewhat reasonable.
>>
>> Thanks
>> Qu
>>>
>>>
>>> Thanks, Anand
>>>
>>>
>>> On 02/07/2014 02:45 PM, Qu Wenruo wrote:
>>>> In btrfs/003 of xfstest, it will check whether btrfs fi show can find
>>>> missing devices.
>>>>
>>>> But before the patch, btrfs-progs will not check whether device 
>>>> missing
>>>> if given a mounted btrfs mountpoint/block device.
>>>> This patch fixes the bug and will pass btrfs/003.
>>>>
>>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>>> Cc: Anand Jain <anand.jain@oracle.com>
>>>> ---
>>>>   cmds-filesystem.c | 12 ++++++++++++
>>>>   1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>>>> index 384d1b9..4c9933d 100644
>>>> --- a/cmds-filesystem.c
>>>> +++ b/cmds-filesystem.c
>>>> @@ -363,6 +363,8 @@ static int print_one_fs(struct
>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>           char *label, char *path)
>>>>   {
>>>>       int i;
>>>> +    int fd;
>>>> +    int missing;
>>>>       char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>>>>       struct btrfs_ioctl_dev_info_args *tmp_dev_info;
>>>>       int ret;
>>>> @@ -385,6 +387,14 @@ static int print_one_fs(struct
>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>
>>>>       for (i = 0; i < fs_info->num_devices; i++) {
>>>>           tmp_dev_info = (struct btrfs_ioctl_dev_info_args
>>>> *)&dev_info[i];
>>>> +
>>>> +        /* Add check for missing devices even mounted */
>>>> +        fd = open((char *)tmp_dev_info->path, O_RDONLY);
>>>> +        if (fd < 0) {
>>>> +            missing = 1;
>>>> +            continue;
>>>> +        }
>>>> +        close(fd);
>>>>           printf("\tdevid %4llu size %s used %s path %s\n",
>>>>               tmp_dev_info->devid,
>>>>               pretty_size(tmp_dev_info->total_bytes),
>>>> @@ -392,6 +402,8 @@ static int print_one_fs(struct
>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>               tmp_dev_info->path);
>>>>       }
>>>>
>>>> +    if (missing)
>>>> +        printf("\t*** Some devices missing\n");
>>>>       printf("\n");
>>>>       return 0;
>>>>   }
>>>>
>>>
>>
>> -- 
>> 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
> -- 
> 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

Re: [PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Anand Jain]

A bit of background of btrfs fi show.

As such original btrfs fi show had too many problems since btrfs-progs
wasn't much consulting btrfs-kernel to determine various
mounted device status. This was a serious problem sometime back.
Various patches brings btrfs-progs to communicate with kernel
and to tell the world the exact status as seen by the kernel.
And avoid any btrfs status fabrication in the user land for the
mounted disks.

The old code was move into the option "-d" and newer which
would print status as per the kernel comes under "-m"
which stands for mounted.

The cli btrfs fi show (with no option) would use both -m
and user-land (lblkid) for unmounted btrfs to show various
disks status.

But here this patch is again bring the old problem and idea into life,
that is it fabricates "missing" at the userland for the mounted btrfs.
As shown in the below testcase it has bad cascading consequences.

This patch must be taken out.

If your worry is about xfstests/btrfs/003 fails, yes it does, we
don't have the right solution as of now to fix it.


On 09/04/2014 11:26, Qu Wenruo wrote:
> Yes, the deleted device scan is still one of the deep problems yet.
>
> But my patch is not intented to deal anything related to the problem.


> For me, I am *only* going to deal with the *exit code* problem,
> 'btrfs fi show <device>' executes correctly(OK, only part of it exactlly),
> but exit code is still 1, which is the bug I'm trying to fix it.

Looks out of context, am I missing something ?

> IMO, the users/admins may never be interested in the inside mechanism
> nor algorithm,
> but the output and exit value things.

user trust btrfs-progs to tell them whats going on in the btrfs kernel.

there is no point in fabricating things (like missing) at the user land
when btrfs kernel isn't aware of it.

> This bug is much like the previous 'btrfs fi show' bugs that breaks some
> xfstests test case

I guess you are talking about the btrfs/003 test case, think the 
correct-way that a xfstest should have determined if the disk is
missing is to dump the btrfs_fs_devices -> btrfs_device and to check
if flag missing is set. xfstest trust btrfs-progs but btrfs-progs don't
do it.

If there is real test program which would check btrfs disk status from
the btrfs kernel memory then again this bug (missing flag is not set)
would exist.


> (always showing a error due to the scan_kernel_v2 function which calls a
> unimplemented ioctl interface),

The problem is its matching kernel patch is not integrated.


> if some patch breaks the old exit code or output, then it should be
> fixed to maintain the old
> output/exit code (except some big decision is made to change it).

Yes but fix it in the right way - notify btrfs kernel about the
missing disks, which would/should set missing flag in the btrfs_device
list.

> So for the output/exit code consistence, it should be fixed even the
> patch may not  means a cure but
> only a workaround for you.

There is no short cut. As shown your patch has the cascading problem.
When btrfs-kernel fixes this issue, And if David and Josef agrees a
patch on top of btrfs-devlist patch will help to fix this problem
once for all.

Thanks, Anand

> Thanks,
> Qu.
>
> 于 2014年04月09日 11:04, Anand Jain 写道:
>>
>>
>>  Below shows the bug cascading to this patch.
>>
>>  And now to fix this I think we shouldn't fix/workaround in the
>>  btrfs-progs again!, fix it in the btrfs-kernel (or leave it open
>>  until suitable fix is found, I tried and failed. but don't fix it
>>  in a wrong way). If you want to help to fix this problem: Find out
>>  if we could get kobject notification with in kernel when disks gets
>>  disappeared.
>>
>>  I have been advocating btrfs-progs should _not_ add its intelligence
>>  and it should be as transparent as possible in showing the kernel's
>>  status. This should be seriously considered.
>>
>> (-----------
>>  For patches to take this approach the core problem here is different
>>  and hope we could correct it..
>>  First, we have a superficial and wrong measuring tape (xfstest) and
>>  we are trying to fix the product using it And in between is btrfs-progs
>>  which is trying to add more superficial-ness.
>>  2nd, btrfs Wiki has a theory and thus sets the direction that
>>  btrfs-progs would copy code from btrfs-kernel, I seriously doubt
>>  if that's a good idea.
>>  If you want to make btrfs-progs as intelligent as btrfs-kernel
>>  (which I don't understand why you should ?  since the purpose of
>>  btrfs-progs and btrfs-kernel are different) then first you need
>>  develop a mini synchronization mechanism between btrfs-progs and btrfs
>>  kernel which is as good as two active nodes FS which says from my
>>  experience with Solaris/SAM-QFS. Developing a synchronization
>>  mechanism is not in the plan here. Further from the End user
>>  Application (DB) performance perspective calling sync at the need of
>>  something like btrfs-progs is a very very bad idea. Applications would
>>  experience jitters in their steady state performance. Once Solaris had
>>  this issue and we fixed it.
>> -----------)
>>
>>  Have fun. ;-)
>>
>> ----------------------------------------------------------------
>> $ btrfs dev scan
>> Scanning for Btrfs filesystems
>> $ mount /dev/sdc /btrfs
>> $ btrfs fi show
>> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>>     Total devices 2 FS bytes used 663.81MiB
>>     devid    1 size 1.10GiB used 1.10GiB path /dev/sdf
>>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>>
>> $ devmgt show
>> host0 sda
>> host1 sdf
>> host2 sdc
>> host3 sdd
>> host4 sde
>> $ devmgt detach /dev/sdf
>> -----/dev/kmsg----
>>     sd 1:0:0:0: [sdf] Stopping disk
>>      SUBSYSTEM=scsi
>>      DEVICE=+scsi:1:0:0:0
>>     ata2.00: disabled
>> ------------------
>> detach /dev/sdf successful
>>
>> (as a known bug btrfs kernel does not know device is missing, missing
>> flag isn't set, as shown below)
>>
>> $ btrfs-devlist
>> fsid name uuid (seed_fsid sprout_fsid)
>>     (fs_latest_devid fs_num_devices fs_open_devices fs_rw_devices
>> fs_missing_devices fs_total_devices) fs_total_rw_bytes
>> fs_num_can_discard fs_latest_trans
>>     devid gen total_bytes disk_total_bytes bytes_used type io_align
>> io_width sector_size fmode
>>     fs_flags
>>     dev_flags
>>
>> dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdf
>> 13715cc5-3aeb-4523-b02c-a072fd427a00 (null null)
>>     (2 2 2 2 0 2) 2363490304 0 7
>>     1 5 1181745152 1181745152 1181745152 0 4096 4096 4096 0x83
>>     fs_Mounted|not_fs_Seeding|fs_Rotating
>>
>> Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev
>>
>>
>> dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdc
>> 12ad34f7-8d58-44fa-95cf-b2bbc0cec69d (null null)
>>     (2 2 2 2 0 2) 2363490304 0 7
>>     2 7 1181745152 1181745152 1160773632 0 4096 4096 4096 0x83
>>     fs_Mounted|not_fs_Seeding|fs_Rotating
>>
>> Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev
>>
>>
>>
>> (below btrfs-progs patch added intelligence to tell the world that
>> device is missing)
>>
>> Ref:
>> ~~~~~~~
>> commit 2ae6a037efd52ae0fa30052d456ad07f074f5d54
>> Author: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> Date:   Fri Feb 7 15:07:19 2014 +0800
>>
>>     btrfs-progs: Add missing devices check for mounted btrfs.
>> ~~~~~~~
>>
>> $ btrfs fi show
>> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>>     Total devices 2 FS bytes used 663.81MiB
>>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>>     *** Some devices missing
>>
>> $ btrfs dev add /dev/sde /btrfs
>> $ btrfs fi show
>> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>>     Total devices 3 FS bytes used 663.81MiB
>>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>>     devid    3 size 1.04GiB used 0.00 path /dev/sde
>>     *** Some devices missing
>>
>>
>> Now the bug is delete missing fails. Since kernel don't
>> understand whats missing.
>>
>> $ btrfs dev del missing /btrfs
>> ERROR: error removing the device 'missing' - no missing devices found
>> to remove
>> $
>> ---------------------------------------------------------------------------
>>
>>
>>
>>
>>
>>
>> On 02/10/2014 08:36 AM, Qu Wenruo wrote:
>>> On Fri, 07 Feb 2014 17:34:46 +0800, Anand Jain wrote:
>>>>
>>>>
>>>>  IMO btrfs-progs shouldn't add its intelligence to know if disk
>>>>  is missing. If btrfs-kernel doesn't know when disk is missing
>>>>  that's a bug to fix in btrfs-kernel. yes that indeed true as
>>>>  of now in btrfs-kernel. btrfs kernel has no idea when disk
>>>>  goes missing, just -EIO doesn't tell btrfs that. I am trying
>>>>  to fix this first.
>>>>
>>>>  But the problem is there isn't good way with in btrfs/FS
>>>>  to know when disk goes missing. did I miss anything ?
>>> Yes, kernel detection is the best way.
>>> But since it has no better way to detect missing device, I think the
>>> btrfs-progs way fix is good enough for now.
>>>
>>> Since btrfs fi show with "-d" options will scan the /dev to find fs and
>>> check missing disks,
>>> I think adds some user-land check even using the ioctl way is still
>>> somewhat reasonable.
>>>
>>> Thanks
>>> Qu
>>>>
>>>>
>>>> Thanks, Anand
>>>>
>>>>
>>>> On 02/07/2014 02:45 PM, Qu Wenruo wrote:
>>>>> In btrfs/003 of xfstest, it will check whether btrfs fi show can find
>>>>> missing devices.
>>>>>
>>>>> But before the patch, btrfs-progs will not check whether device
>>>>> missing
>>>>> if given a mounted btrfs mountpoint/block device.
>>>>> This patch fixes the bug and will pass btrfs/003.
>>>>>
>>>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>>>> Cc: Anand Jain <anand.jain@oracle.com>
>>>>> ---
>>>>>   cmds-filesystem.c | 12 ++++++++++++
>>>>>   1 file changed, 12 insertions(+)
>>>>>
>>>>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>>>>> index 384d1b9..4c9933d 100644
>>>>> --- a/cmds-filesystem.c
>>>>> +++ b/cmds-filesystem.c
>>>>> @@ -363,6 +363,8 @@ static int print_one_fs(struct
>>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>>           char *label, char *path)
>>>>>   {
>>>>>       int i;
>>>>> +    int fd;
>>>>> +    int missing;
>>>>>       char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>>>>>       struct btrfs_ioctl_dev_info_args *tmp_dev_info;
>>>>>       int ret;
>>>>> @@ -385,6 +387,14 @@ static int print_one_fs(struct
>>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>>
>>>>>       for (i = 0; i < fs_info->num_devices; i++) {
>>>>>           tmp_dev_info = (struct btrfs_ioctl_dev_info_args
>>>>> *)&dev_info[i];
>>>>> +
>>>>> +        /* Add check for missing devices even mounted */
>>>>> +        fd = open((char *)tmp_dev_info->path, O_RDONLY);
>>>>> +        if (fd < 0) {
>>>>> +            missing = 1;
>>>>> +            continue;
>>>>> +        }
>>>>> +        close(fd);
>>>>>           printf("\tdevid %4llu size %s used %s path %s\n",
>>>>>               tmp_dev_info->devid,
>>>>>               pretty_size(tmp_dev_info->total_bytes),
>>>>> @@ -392,6 +402,8 @@ static int print_one_fs(struct
>>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>>               tmp_dev_info->path);
>>>>>       }
>>>>>
>>>>> +    if (missing)
>>>>> +        printf("\t*** Some devices missing\n");
>>>>>       printf("\n");
>>>>>       return 0;
>>>>>   }
>>>>>
>>>>
>>>
>>> --
>>> 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
>> --
>> 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
>
> --
> 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

Re: [PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Qu Wenruo]

于 2014年04月09日 12:33, Anand Jain 写道:
>
>
> A bit of background of btrfs fi show.
>
> As such original btrfs fi show had too many problems since btrfs-progs
> wasn't much consulting btrfs-kernel to determine various
> mounted device status. This was a serious problem sometime back.
> Various patches brings btrfs-progs to communicate with kernel
> and to tell the world the exact status as seen by the kernel.
> And avoid any btrfs status fabrication in the user land for the
> mounted disks.
>
> The old code was move into the option "-d" and newer which
> would print status as per the kernel comes under "-m"
> which stands for mounted.
>
> The cli btrfs fi show (with no option) would use both -m
> and user-land (lblkid) for unmounted btrfs to show various
> disks status.
>
> But here this patch is again bring the old problem and idea into life,
> that is it fabricates "missing" at the userland for the mounted btrfs.
> As shown in the below testcase it has bad cascading consequences.
>
> This patch must be taken out.
>
> If your worry is about xfstests/btrfs/003 fails, yes it does, we
> don't have the right solution as of now to fix it.

Yes, I'm still worry about the xfstests testcase.
Personally, I think you should try to remove the disk remove test in 
btrfs/003 first.
(Though I think it could be much harder to archieve it)

If xfstests devs can be persuaded and NOTE added to btrfs-progs document,
I'll be completely OK for the revert of this patch.

>
>
> On 09/04/2014 11:26, Qu Wenruo wrote:
>> Yes, the deleted device scan is still one of the deep problems yet.
>>
>> But my patch is not intented to deal anything related to the problem.
>
>
>> For me, I am *only* going to deal with the *exit code* problem,
>> 'btrfs fi show <device>' executes correctly(OK, only part of it 
>> exactlly),
>> but exit code is still 1, which is the bug I'm trying to fix it.
>
> Looks out of context, am I missing something ?

Sorry for getting mixed with the patch dealing with the exit code.
(Previous patch I sent)

>
>> IMO, the users/admins may never be interested in the inside mechanism
>> nor algorithm,
>> but the output and exit value things.
>
> user trust btrfs-progs to tell them whats going on in the btrfs kernel.

IMO user trust btrfs-progs to give them right info on btrfs, not care 
nor aware of the relationship
between kernel.

>
> there is no point in fabricating things (like missing) at the user land
> when btrfs kernel isn't aware of it.

 From the respect of normal users/admins, they does not cares how it is 
done but the result.
So if user finds missing devices still output in 'btrfs fi show', they 
will be confused.

Though I think any sane users/admins will call 'btrfs dev del' first,
so the problem still comes to the btrfs/003 test case.

>
>> This bug is much like the previous 'btrfs fi show' bugs that breaks some
>> xfstests test case
>
> I guess you are talking about the btrfs/003 test case, think the 
> correct-way that a xfstest should have determined if the disk is
> missing is to dump the btrfs_fs_devices -> btrfs_device and to check
> if flag missing is set. xfstest trust btrfs-progs but btrfs-progs don't
> do it.
>
> If there is real test program which would check btrfs disk status from
> the btrfs kernel memory then again this bug (missing flag is not set)
> would exist.

This bug is not the dev delete but the error message due to the 
unimplemented
btrfs ioctl things below.

>
>
>> (always showing a error due to the scan_kernel_v2 function which calls a
>> unimplemented ioctl interface),
>
> The problem is its matching kernel patch is not integrated.
>
>
>> if some patch breaks the old exit code or output, then it should be
>> fixed to maintain the old
>> output/exit code (except some big decision is made to change it).
>
> Yes but fix it in the right way - notify btrfs kernel about the
> missing disks, which would/should set missing flag in the btrfs_device
> list.

As I mentioned, this patch is just a workaround for testcase btrfs/003.
No mean to fix the whole things.

As mentioned at the beginning, I prefer to remove the device remove test
in testcase, then the patch can be reverted without any complain from me.

Thanks,
Qu.
>
>> So for the output/exit code consistence, it should be fixed even the
>> patch may not  means a cure but
>> only a workaround for you.
>
> There is no short cut. As shown your patch has the cascading problem.
> When btrfs-kernel fixes this issue, And if David and Josef agrees a
> patch on top of btrfs-devlist patch will help to fix this problem
> once for all.
>
> Thanks, Anand
>
>> Thanks,
>> Qu.
>>
>> 于 2014年04月09日 11:04, Anand Jain 写道:
>>>
>>>
>>>  Below shows the bug cascading to this patch.
>>>
>>>  And now to fix this I think we shouldn't fix/workaround in the
>>>  btrfs-progs again!, fix it in the btrfs-kernel (or leave it open
>>>  until suitable fix is found, I tried and failed. but don't fix it
>>>  in a wrong way). If you want to help to fix this problem: Find out
>>>  if we could get kobject notification with in kernel when disks gets
>>>  disappeared.
>>>
>>>  I have been advocating btrfs-progs should _not_ add its intelligence
>>>  and it should be as transparent as possible in showing the kernel's
>>>  status. This should be seriously considered.
>>>
>>> (-----------
>>>  For patches to take this approach the core problem here is different
>>>  and hope we could correct it..
>>>  First, we have a superficial and wrong measuring tape (xfstest) and
>>>  we are trying to fix the product using it And in between is 
>>> btrfs-progs
>>>  which is trying to add more superficial-ness.
>>>  2nd, btrfs Wiki has a theory and thus sets the direction that
>>>  btrfs-progs would copy code from btrfs-kernel, I seriously doubt
>>>  if that's a good idea.
>>>  If you want to make btrfs-progs as intelligent as btrfs-kernel
>>>  (which I don't understand why you should ?  since the purpose of
>>>  btrfs-progs and btrfs-kernel are different) then first you need
>>>  develop a mini synchronization mechanism between btrfs-progs and btrfs
>>>  kernel which is as good as two active nodes FS which says from my
>>>  experience with Solaris/SAM-QFS. Developing a synchronization
>>>  mechanism is not in the plan here. Further from the End user
>>>  Application (DB) performance perspective calling sync at the need of
>>>  something like btrfs-progs is a very very bad idea. Applications would
>>>  experience jitters in their steady state performance. Once Solaris had
>>>  this issue and we fixed it.
>>> -----------)
>>>
>>>  Have fun. ;-)
>>>
>>> ----------------------------------------------------------------
>>> $ btrfs dev scan
>>> Scanning for Btrfs filesystems
>>> $ mount /dev/sdc /btrfs
>>> $ btrfs fi show
>>> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>>>     Total devices 2 FS bytes used 663.81MiB
>>>     devid    1 size 1.10GiB used 1.10GiB path /dev/sdf
>>>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>>>
>>> $ devmgt show
>>> host0 sda
>>> host1 sdf
>>> host2 sdc
>>> host3 sdd
>>> host4 sde
>>> $ devmgt detach /dev/sdf
>>> -----/dev/kmsg----
>>>     sd 1:0:0:0: [sdf] Stopping disk
>>>      SUBSYSTEM=scsi
>>>      DEVICE=+scsi:1:0:0:0
>>>     ata2.00: disabled
>>> ------------------
>>> detach /dev/sdf successful
>>>
>>> (as a known bug btrfs kernel does not know device is missing, missing
>>> flag isn't set, as shown below)
>>>
>>> $ btrfs-devlist
>>> fsid name uuid (seed_fsid sprout_fsid)
>>>     (fs_latest_devid fs_num_devices fs_open_devices fs_rw_devices
>>> fs_missing_devices fs_total_devices) fs_total_rw_bytes
>>> fs_num_can_discard fs_latest_trans
>>>     devid gen total_bytes disk_total_bytes bytes_used type io_align
>>> io_width sector_size fmode
>>>     fs_flags
>>>     dev_flags
>>>
>>> dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdf
>>> 13715cc5-3aeb-4523-b02c-a072fd427a00 (null null)
>>>     (2 2 2 2 0 2) 2363490304 0 7
>>>     1 5 1181745152 1181745152 1181745152 0 4096 4096 4096 0x83
>>>     fs_Mounted|not_fs_Seeding|fs_Rotating
>>>
>>> Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev 
>>>
>>>
>>>
>>> dfbf136d-e8d2-489b-8ee1-be0d5999769d /dev/sdc
>>> 12ad34f7-8d58-44fa-95cf-b2bbc0cec69d (null null)
>>>     (2 2 2 2 0 2) 2363490304 0 7
>>>     2 7 1181745152 1181745152 1160773632 0 4096 4096 4096 0x83
>>>     fs_Mounted|not_fs_Seeding|fs_Rotating
>>>
>>> Writable|MD|not_Missing|not_Discard|not_Replace_tgt|not_Run_pending|not_Nobarriers|Stat_valid|Stat_dirty|Bdev 
>>>
>>>
>>>
>>>
>>> (below btrfs-progs patch added intelligence to tell the world that
>>> device is missing)
>>>
>>> Ref:
>>> ~~~~~~~
>>> commit 2ae6a037efd52ae0fa30052d456ad07f074f5d54
>>> Author: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> Date:   Fri Feb 7 15:07:19 2014 +0800
>>>
>>>     btrfs-progs: Add missing devices check for mounted btrfs.
>>> ~~~~~~~
>>>
>>> $ btrfs fi show
>>> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>>>     Total devices 2 FS bytes used 663.81MiB
>>>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>>>     *** Some devices missing
>>>
>>> $ btrfs dev add /dev/sde /btrfs
>>> $ btrfs fi show
>>> Label: none  uuid: dfbf136d-e8d2-489b-8ee1-be0d5999769d
>>>     Total devices 3 FS bytes used 663.81MiB
>>>     devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
>>>     devid    3 size 1.04GiB used 0.00 path /dev/sde
>>>     *** Some devices missing
>>>
>>>
>>> Now the bug is delete missing fails. Since kernel don't
>>> understand whats missing.
>>>
>>> $ btrfs dev del missing /btrfs
>>> ERROR: error removing the device 'missing' - no missing devices found
>>> to remove
>>> $
>>> --------------------------------------------------------------------------- 
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On 02/10/2014 08:36 AM, Qu Wenruo wrote:
>>>> On Fri, 07 Feb 2014 17:34:46 +0800, Anand Jain wrote:
>>>>>
>>>>>
>>>>>  IMO btrfs-progs shouldn't add its intelligence to know if disk
>>>>>  is missing. If btrfs-kernel doesn't know when disk is missing
>>>>>  that's a bug to fix in btrfs-kernel. yes that indeed true as
>>>>>  of now in btrfs-kernel. btrfs kernel has no idea when disk
>>>>>  goes missing, just -EIO doesn't tell btrfs that. I am trying
>>>>>  to fix this first.
>>>>>
>>>>>  But the problem is there isn't good way with in btrfs/FS
>>>>>  to know when disk goes missing. did I miss anything ?
>>>> Yes, kernel detection is the best way.
>>>> But since it has no better way to detect missing device, I think the
>>>> btrfs-progs way fix is good enough for now.
>>>>
>>>> Since btrfs fi show with "-d" options will scan the /dev to find fs 
>>>> and
>>>> check missing disks,
>>>> I think adds some user-land check even using the ioctl way is still
>>>> somewhat reasonable.
>>>>
>>>> Thanks
>>>> Qu
>>>>>
>>>>>
>>>>> Thanks, Anand
>>>>>
>>>>>
>>>>> On 02/07/2014 02:45 PM, Qu Wenruo wrote:
>>>>>> In btrfs/003 of xfstest, it will check whether btrfs fi show can 
>>>>>> find
>>>>>> missing devices.
>>>>>>
>>>>>> But before the patch, btrfs-progs will not check whether device
>>>>>> missing
>>>>>> if given a mounted btrfs mountpoint/block device.
>>>>>> This patch fixes the bug and will pass btrfs/003.
>>>>>>
>>>>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>>>>> Cc: Anand Jain <anand.jain@oracle.com>
>>>>>> ---
>>>>>>   cmds-filesystem.c | 12 ++++++++++++
>>>>>>   1 file changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
>>>>>> index 384d1b9..4c9933d 100644
>>>>>> --- a/cmds-filesystem.c
>>>>>> +++ b/cmds-filesystem.c
>>>>>> @@ -363,6 +363,8 @@ static int print_one_fs(struct
>>>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>>>           char *label, char *path)
>>>>>>   {
>>>>>>       int i;
>>>>>> +    int fd;
>>>>>> +    int missing;
>>>>>>       char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
>>>>>>       struct btrfs_ioctl_dev_info_args *tmp_dev_info;
>>>>>>       int ret;
>>>>>> @@ -385,6 +387,14 @@ static int print_one_fs(struct
>>>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>>>
>>>>>>       for (i = 0; i < fs_info->num_devices; i++) {
>>>>>>           tmp_dev_info = (struct btrfs_ioctl_dev_info_args
>>>>>> *)&dev_info[i];
>>>>>> +
>>>>>> +        /* Add check for missing devices even mounted */
>>>>>> +        fd = open((char *)tmp_dev_info->path, O_RDONLY);
>>>>>> +        if (fd < 0) {
>>>>>> +            missing = 1;
>>>>>> +            continue;
>>>>>> +        }
>>>>>> +        close(fd);
>>>>>>           printf("\tdevid %4llu size %s used %s path %s\n",
>>>>>>               tmp_dev_info->devid,
>>>>>>               pretty_size(tmp_dev_info->total_bytes),
>>>>>> @@ -392,6 +402,8 @@ static int print_one_fs(struct
>>>>>> btrfs_ioctl_fs_info_args *fs_info,
>>>>>>               tmp_dev_info->path);
>>>>>>       }
>>>>>>
>>>>>> +    if (missing)
>>>>>> +        printf("\t*** Some devices missing\n");
>>>>>>       printf("\n");
>>>>>>       return 0;
>>>>>>   }
>>>>>>
>>>>>
>>>>
>>>> -- 
>>>> 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
>>> -- 
>>> 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
>>
>> -- 
>> 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

Re: [PATCH 1/2] btrfs-progs: Add missing devices check for mounted btrfs.

[Anand Jain]

> As mentioned at the beginning, I prefer to remove the device remove test
> in testcase, then the patch can be reverted without any complain from me.

  Thats a wrong approach as well.
  The test case 003 is a very real case at the data centers.
  Especially the iscsi/san luns go offline/online all the time.