[PATCH 1/3 v3] btrfs-progs: there is devid 0 when replace is running

[PATCH 1/3 v3] btrfs-progs: there is devid 0 when replace is running

[Anand Jain]

From: Anand Jain <anand.jain@oracle.com>

as of now, when we replace a disk, it is added to the
dev list with devid 0. And we fail to obtain details
of devid 0 because we don't query devid 0 at all.

reproducer:

 btrfs rep start /dev/sdb /dev/sdf /btrfs
 btrfs fi show
 Label: none  uuid: f8fb9819-16c8-47b7-b62f-0ff90f8c56cd
        Total devices 3 FS bytes used 1.94GiB
        devid    1 size 1.10GiB used 1.10GiB path /dev/sdb
        devid    2 size 1.10GiB used 1.08GiB path /dev/sdc
        devid    0 size 0.00 used 0.00 path

  this patch will make it proper by querying devid 0.

 btrfs repl start /dev/sdb /dev/sdf /btrfs
 btrfs fi show /btrfs
 Label: none  uuid: f8fb9819-16c8-47b7-b62f-0ff90f8c56cd
        Total devices 3 FS bytes used 1.94GiB
        devid    0 size 1.10GiB used 1.10GiB path /dev/sdf
        devid    1 size 1.10GiB used 1.10GiB path /dev/sdb
        devid    2 size 1.10GiB used 1.08GiB path /dev/sdc

 Its fine to query devid 0 when there is no replace
 activity as well, because we just skip the error ENODEV

 btrfs fi show /btrfs
 Label: none  uuid: f8fb9819-16c8-47b7-b62f-0ff90f8c56cd
        Total devices 2 FS bytes used 1.94GiB
        devid    1 size 1.10GiB used 1.10GiB path /dev/sdf
        devid    2 size 1.10GiB used 1.08GiB path /dev/sdc

Signed-off-by: Anand Jain <Anand.Jain@oracle.com>
---
 v3: it needs to handle per devid probe
 v2: commit message fix

 utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils.c b/utils.c
index de513b6..e0c750d 100644
--- a/utils.c
+++ b/utils.c
@@ -1637,7 +1637,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 	int fd = -1;
 	int ret = 0;
 	int ndevs = 0;
-	int i = 1;
+	int i = 0;
 	struct btrfs_fs_devices *fs_devices_mnt = NULL;
 	struct btrfs_ioctl_dev_info_args *di_args;
 	char mp[BTRFS_PATH_NAME_MAX + 1];
-- 
1.8.4.2

[PATCH 2/3] btrfs-progs: latest_devid is not always the probed devid

[Anand Jain]

btrfs-progs picks the latest_dev based on first probed
greatest trans-id. However below test case proofs that
approach is wrong.

$ mkfs.btrfs -d raid1 -m raid1 /dev/sde /dev/sdf
$ modprobe -r btrfs && modprobe btrfs
$ mount -o degraded /dev/sde /btrfs
$ touch /btrfs/testfile && btrfs fi sync /btrfs

The above steps will make /dev/sdf not part of the btrfs.

and as below when you use /dev/sdf the btrfs dev stat
and dev scrub picks up wrong disk

$ btrfs dev stat /dev/sdf
[/dev/sde].write_io_errs   0
[/dev/sde].read_io_errs    0
[/dev/sde].flush_io_errs   0
[/dev/sde].corruption_errs 0
[/dev/sde].generation_errs 0

$ btrfs scrub start -B /dev/sdf
scrub done for 2e99c881-6abd-4f8a-8290-e2f8d0acc575
 scrub started at Mon Feb 24 14:45:06 2014 and finished after 0 seconds
 total bytes scrubbed: 256.00KiB with 0 errors

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

diff --git a/utils.c b/utils.c
index e0c750d..231031e 100644
--- a/utils.c
+++ b/utils.c
@@ -1646,6 +1646,11 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 	memset(fi_args, 0, sizeof(*fi_args));
 
 	if (is_block_device(path)) {
+
+		struct btrfs_super_block *disk_super;
+		char *buf;
+		u64 devid;
+
 		/* Ensure it's mounted, then set path to the mountpoint */
 		fd = open(path, O_RDONLY);
 		if (fd < 0) {
@@ -1665,8 +1670,23 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 		path = mp;
 		/* Only fill in this one device */
 		fi_args->num_devices = 1;
-		fi_args->max_id = fs_devices_mnt->latest_devid;
-		i = fs_devices_mnt->latest_devid;
+
+		buf = malloc(4096);
+		if (!buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		disk_super = (struct btrfs_super_block *)buf;
+		ret = btrfs_read_dev_super(fd, disk_super, BTRFS_SUPER_INFO_OFFSET);
+		if (ret < 0) {
+			ret = -EIO;
+			goto out;
+		}
+		devid = btrfs_stack_device_id(&disk_super->dev_item);
+
+		fi_args->max_id = devid;
+		i = devid;
+
 		memcpy(fi_args->fsid, fs_devices_mnt->fsid, BTRFS_FSID_SIZE);
 		close(fd);
 	}
-- 
1.8.4.2

[PATCH 3/3] btrfs-progs: Fix bug when scanned for devid which was missing and deleted

[Anand Jain]

get_fs_info() provides the info of the specific
device/devid, however when we delete the missing disk
the super-block on the disk isn't cleared, and since
btrfs-progs makes its decision by reading the disk super
block, so it doesn't know about the kernel previous action,
And now when we tried to probe kernel for the devid it fails.

reproducer:

$ mkfs.btrfs -d raid1 -m raid1 /dev/sde /dev/sdf
$ modprobe -r btrfs && modprobe btrfs
$ mount -o degraded /dev/sde /btrfs
$ btrfs dev add /dev/sdd /btrfs
$ btrfs dev del missing /btrfs
$ btrfs scrub start -B /dev/sdf
btrfs: utils.c:1741: get_fs_info: Assertion `!(ndevs == 0)' failed.
Aborted (core dumped)

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

diff --git a/utils.c b/utils.c
index 231031e..bce92ab 100644
--- a/utils.c
+++ b/utils.c
@@ -1726,8 +1726,15 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 		ndevs++;
 	}
 
-	BUG_ON(ndevs == 0);
-	ret = 0;
+	/*
+	* only when the only dev we wanted to find is not there then
+	* let any error be returned
+	*/
+	if (fi_args->num_devices != 1) {
+		BUG_ON(ndevs == 0);
+		ret = 0;
+	}
+
 out:
 	close_file_or_dir(fd, dirstream);
 	return ret;
-- 
1.8.4.2

Re: [PATCH 2/3] btrfs-progs: latest_devid is not always the probed devid

[David Sterba]

On Mon, Feb 24, 2014 at 07:43:38PM +0800, Anand Jain wrote:
> @@ -1646,6 +1646,11 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>  	memset(fi_args, 0, sizeof(*fi_args));
>  
>  	if (is_block_device(path)) {
> +
> +		struct btrfs_super_block *disk_super;
> +		char *buf;
> +		u64 devid;
> +
>  		/* Ensure it's mounted, then set path to the mountpoint */
>  		fd = open(path, O_RDONLY);
>  		if (fd < 0) {
> @@ -1665,8 +1670,23 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>  		path = mp;
>  		/* Only fill in this one device */
>  		fi_args->num_devices = 1;
> -		fi_args->max_id = fs_devices_mnt->latest_devid;
> -		i = fs_devices_mnt->latest_devid;
> +
> +		buf = malloc(4096);
> +		if (!buf) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +		disk_super = (struct btrfs_super_block *)buf;
> +		ret = btrfs_read_dev_super(fd, disk_super, BTRFS_SUPER_INFO_OFFSET);
> +		if (ret < 0) {
> +			ret = -EIO;
> +			goto out;
> +		}
> +		devid = btrfs_stack_device_id(&disk_super->dev_item);
> +
> +		fi_args->max_id = devid;
> +		i = devid;
> +
>  		memcpy(fi_args->fsid, fs_devices_mnt->fsid, BTRFS_FSID_SIZE);
>  		close(fd);

This is leaking memory, but as this is userspace code we can use the
staack for the superblock. I've committed this with the following fixup:

--- a/utils.c
+++ b/utils.c
@@ -1680,7 +1680,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
        if (is_block_device(path)) {

                struct btrfs_super_block *disk_super;
-               char *buf;
+               char buf[BTRFS_SUPER_INFO_SIZE];
                u64 devid;

                /* Ensure it's mounted, then set path to the mountpoint */
@@ -1703,11 +1703,6 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
                /* Only fill in this one device */
                fi_args->num_devices = 1;

-               buf = malloc(4096);
-               if (!buf) {
-                       ret = -ENOMEM;
-                       goto out;
-               }
                disk_super = (struct btrfs_super_block *)buf;
                ret = btrfs_read_dev_super(fd, disk_super, BTRFS_SUPER_INFO_OFFSET);
                if (ret < 0) {

Re: [PATCH 2/3] btrfs-progs: latest_devid is not always the probed devid

[Anand Jain]

On 26/02/2014 01:27, David Sterba wrote:
> On Mon, Feb 24, 2014 at 07:43:38PM +0800, Anand Jain wrote:
>> @@ -1646,6 +1646,11 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>>   	memset(fi_args, 0, sizeof(*fi_args));
>>
>>   	if (is_block_device(path)) {
>> +
>> +		struct btrfs_super_block *disk_super;
>> +		char *buf;
>> +		u64 devid;
>> +
>>   		/* Ensure it's mounted, then set path to the mountpoint */
>>   		fd = open(path, O_RDONLY);
>>   		if (fd < 0) {
>> @@ -1665,8 +1670,23 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>>   		path = mp;
>>   		/* Only fill in this one device */
>>   		fi_args->num_devices = 1;
>> -		fi_args->max_id = fs_devices_mnt->latest_devid;
>> -		i = fs_devices_mnt->latest_devid;
>> +
>> +		buf = malloc(4096);
>> +		if (!buf) {
>> +			ret = -ENOMEM;
>> +			goto out;
>> +		}
>> +		disk_super = (struct btrfs_super_block *)buf;
>> +		ret = btrfs_read_dev_super(fd, disk_super, BTRFS_SUPER_INFO_OFFSET);
>> +		if (ret < 0) {
>> +			ret = -EIO;
>> +			goto out;
>> +		}
>> +		devid = btrfs_stack_device_id(&disk_super->dev_item);
>> +
>> +		fi_args->max_id = devid;
>> +		i = devid;
>> +
>>   		memcpy(fi_args->fsid, fs_devices_mnt->fsid, BTRFS_FSID_SIZE);
>>   		close(fd);
>
> This is leaking memory, but as this is userspace code we can use the
> staack for the superblock. I've committed this with the following fixup:

  oh no my bad. thanks for the catch.



> --- a/utils.c
> +++ b/utils.c
> @@ -1680,7 +1680,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>          if (is_block_device(path)) {
>
>                  struct btrfs_super_block *disk_super;
> -               char *buf;
> +               char buf[BTRFS_SUPER_INFO_SIZE];
>                  u64 devid;
>
>                  /* Ensure it's mounted, then set path to the mountpoint */
> @@ -1703,11 +1703,6 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>                  /* Only fill in this one device */
>                  fi_args->num_devices = 1;
>
> -               buf = malloc(4096);
> -               if (!buf) {
> -                       ret = -ENOMEM;
> -                       goto out;
> -               }
>                  disk_super = (struct btrfs_super_block *)buf;
>                  ret = btrfs_read_dev_super(fd, disk_super, BTRFS_SUPER_INFO_OFFSET);
>                  if (ret < 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
>