[PATCH] btrfs-progs: skip fs with no seed when build seed/sprout mapping for fi show

[PATCH] btrfs-progs: skip fs with no seed when build seed/sprout mapping for fi show

[Gui Hecheng]

There is no need to try to build seed/sprout mapping for those btrfs
without seed devices, so just skip such fs.
We could get the total number of devices from the disk super block, if it
equals the number of items in list @fs_devices->devices, then there shouldn't
be any seed devices.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 cmds-filesystem.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 67fe52b..0d49cd1 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -795,6 +795,22 @@ out:
 	return ret;
 }
 
+static int no_seed_devices(struct btrfs_fs_devices *fs_devices)
+{
+	struct btrfs_device *device;
+	int dev_cnt_total, dev_cnt = 0;
+
+	device = list_first_entry(&fs_devices->devices, struct btrfs_device,
+				  dev_list);
+
+	dev_cnt_total = device->total_devs;
+
+	list_for_each_entry(device, &fs_devices->devices, dev_list)
+		dev_cnt++;
+
+	return dev_cnt_total == dev_cnt;
+}
+
 static int map_seed_devices(struct list_head *all_uuids)
 {
 	struct btrfs_fs_devices *cur_fs, *cur_seed;
@@ -812,6 +828,11 @@ static int map_seed_devices(struct list_head *all_uuids)
 						struct btrfs_device, dev_list);
 		if (!device)
 			continue;
+
+		/* skip fs without seeds */
+		if (no_seed_devices(cur_fs))
+			continue;
+
 		/*
 		 * open_ctree_* detects seed/sprout mapping
 		 */
@@ -976,8 +997,8 @@ devs_only:
 	}
 
 	/*
-	 * scan_for_btrfs() don't build seed/sprout mapping,
-	 * do mapping build for each scanned fs here
+	 * The seed/sprout mapping are not detected yet,
+	 * do mapping build for all umounted fs
 	 */
 	ret = map_seed_devices(&all_uuids);
 	if (ret) {
-- 
1.8.1.4

[PATCH 1/2] btrfs-progs: remove BUG_ON on num of devices for btrfs fi show

[Gui Hecheng]

The following BUG_ON:
	BUG_ON(ndevs >= fi_args->num_devices)
is not needed, because it always fails with seed devices present.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
 utils.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/utils.c b/utils.c
index f10c178..9bcc1a0 100644
--- a/utils.c
+++ b/utils.c
@@ -1889,7 +1889,6 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 	i++;
 
 	for (; i <= fi_args->max_id; ++i) {
-		BUG_ON(ndevs >= fi_args->num_devices);
 		ret = get_device_info(fd, i, &di_args[ndevs]);
 		if (ret == -ENODEV)
 			continue;
-- 
1.8.1.4

[PATCH 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices

[Gui Hecheng]

The @fi_args->num_devices in @get_fs_info() does not include seed devices.
We could just correct it by searching the chunk tree and count how
many dev_items there are in total which includes seed devices.

Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
---
*Note*
This is just a temporary workaround to fix this problem in order to
make users happy, because a new ioctl or sysfs interface to handle this
problem needs more discussions and efforts. After the work implemented
and accepted, we could drop this.
---
 utils.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/utils.c b/utils.c
index 9bcc1a0..a6dcb8a 100644
--- a/utils.c
+++ b/utils.c
@@ -1781,6 +1781,73 @@ int get_device_info(int fd, u64 devid,
 	return ret ? -errno : 0;
 }
 
+u64 find_max_id(struct btrfs_ioctl_search_args *search_args, int nr_items)
+{
+	struct btrfs_dev_item *dev_item;
+	char *buf = search_args->buf;
+
+	buf += (nr_items - 1) * (sizeof(struct btrfs_ioctl_search_header)
+					+ sizeof(struct btrfs_dev_item));
+	buf += sizeof(struct btrfs_ioctl_search_header);
+
+	dev_item = (struct btrfs_dev_item *)buf;
+
+	return btrfs_stack_device_id(dev_item);
+}
+
+int correct_fs_info(int fd, struct btrfs_ioctl_fs_info_args *fi_args)
+{
+	int ret;
+	int max_items;
+	u64 start_devid = 1;
+	struct btrfs_ioctl_search_args search_args;
+	struct btrfs_ioctl_search_key *search_key = &search_args.key;
+
+	fi_args->num_devices = 0;
+
+	max_items = BTRFS_SEARCH_ARGS_BUFSIZE
+		/ (sizeof(struct btrfs_ioctl_search_header)
+				+ sizeof(struct btrfs_dev_item));
+
+	search_key->tree_id = BTRFS_CHUNK_TREE_OBJECTID;
+	search_key->min_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+	search_key->max_objectid = BTRFS_DEV_ITEMS_OBJECTID;
+	search_key->min_type = BTRFS_DEV_ITEM_KEY;
+	search_key->max_type = BTRFS_DEV_ITEM_KEY;
+	search_key->min_transid = 0;
+	search_key->max_transid = (u64)-1;
+	search_key->nr_items = max_items;
+	search_key->max_offset = (u64)-1;
+
+again:
+	search_key->min_offset = start_devid;
+
+	ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &search_args);
+	if (ret < 0)
+		return -errno;
+
+	fi_args->num_devices += (u64)search_key->nr_items;
+
+	if (search_key->nr_items == max_items) {
+		start_devid = find_max_id(&search_args,
+				search_key->nr_items) + 1;
+		goto again;
+	}
+
+	/* get the lastest max_id to stay consistent with the num_devices */
+	if (search_key->nr_items == 0)
+		/*
+		 * last tree_search returns an empty buf, use the devid of
+		 * the last dev_item of the previous tree_search
+		 */
+		fi_args->max_id = start_devid - 1;
+	else
+		fi_args->max_id = find_max_id(&search_args,
+						search_key->nr_items);
+
+	return 0;
+}
+
 /*
  * For a given path, fill in the ioctl fs_ and info_ args.
  * If the path is a btrfs mountpoint, fill info for all devices.
@@ -1860,6 +1927,13 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 			ret = -errno;
 			goto out;
 		}
+
+		/*
+		 * The fs_args->num_devices does not include seed devices
+		 */
+		ret = correct_fs_info(fd, fi_args);
+		if (ret)
+			goto out;
 	}
 
 	if (!fi_args->num_devices)
-- 
1.8.1.4

Re: [PATCH 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices

[David Sterba]

On Fri, Nov 07, 2014 at 10:07:43AM +0800, Gui Hecheng wrote:
> The @fi_args->num_devices in @get_fs_info() does not include seed devices.
> We could just correct it by searching the chunk tree and count how
> many dev_items there are in total which includes seed devices.
> 
> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> ---
> *Note*
> This is just a temporary workaround to fix this problem in order to
> make users happy, because a new ioctl or sysfs interface to handle this
> problem needs more discussions and efforts. After the work implemented
> and accepted, we could drop this.

Nice, thanks. I agree that this kind of workaround is best possible for
the moment, and I'm glad to see that it's not that much code to get the
seeding devices right. This would also work with older kernels without
the updated sysfs/ioctl interfaces, so this is likely to stay for a long
time.

> +u64 find_max_id(struct btrfs_ioctl_search_args *search_args, int nr_items)

That's a very generic name for a function that does a very specialized
thing, but I don't have a suggestion right now.

> +int correct_fs_info(int fd, struct btrfs_ioctl_fs_info_args *fi_args)

Same here, make fs_info correct but in what way? A comment would be good
as well.

Re: [PATCH 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices

[anand jain]

really nice fix. Thanks Gui.
Anand


On 08/11/2014 02:16, David Sterba wrote:
> On Fri, Nov 07, 2014 at 10:07:43AM +0800, Gui Hecheng wrote:
>> The @fi_args->num_devices in @get_fs_info() does not include seed devices.
>> We could just correct it by searching the chunk tree and count how
>> many dev_items there are in total which includes seed devices.
>>
>> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
>> ---
>> *Note*
>> This is just a temporary workaround to fix this problem in order to
>> make users happy, because a new ioctl or sysfs interface to handle this
>> problem needs more discussions and efforts. After the work implemented
>> and accepted, we could drop this.
>
> Nice, thanks. I agree that this kind of workaround is best possible for
> the moment, and I'm glad to see that it's not that much code to get the
> seeding devices right. This would also work with older kernels without
> the updated sysfs/ioctl interfaces, so this is likely to stay for a long
> time.
>
>> +u64 find_max_id(struct btrfs_ioctl_search_args *search_args, int nr_items)
>
> That's a very generic name for a function that does a very specialized
> thing, but I don't have a suggestion right now.
>
>> +int correct_fs_info(int fd, struct btrfs_ioctl_fs_info_args *fi_args)
>
> Same here, make fs_info correct but in what way? A comment would be good
> as well.
> --
> 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 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices

[Mike Fleetwood]

On 7 November 2014 18:16, David Sterba <dsterba@suse.cz> wrote:
> On Fri, Nov 07, 2014 at 10:07:43AM +0800, Gui Hecheng wrote:
>> The @fi_args->num_devices in @get_fs_info() does not include seed devices.
>> We could just correct it by searching the chunk tree and count how
>> many dev_items there are in total which includes seed devices.
>>
>> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
>> ---
>> *Note*
>> This is just a temporary workaround to fix this problem in order to
>> make users happy, because a new ioctl or sysfs interface to handle this
>> problem needs more discussions and efforts. After the work implemented
>> and accepted, we could drop this.
>
> Nice, thanks. I agree that this kind of workaround is best possible for
> the moment, and I'm glad to see that it's not that much code to get the
> seeding devices right. This would also work with older kernels without
> the updated sysfs/ioctl interfaces, so this is likely to stay for a long
> time.
>
>> +u64 find_max_id(struct btrfs_ioctl_search_args *search_args, int nr_items)
>
> That's a very generic name for a function that does a very specialized
> thing, but I don't have a suggestion right now.

Is find_max_device_id a suitable name?

>> +int correct_fs_info(int fd, struct btrfs_ioctl_fs_info_args *fi_args)
>
> Same here, make fs_info correct but in what way? A comment would be good
> as well.

Sorry, no suggestion for this function name.

Thanks,
Mike

Re: [PATCH 2/2] btrfs-progs: fix wrong num_devices for btrfs fi show with seed devices

[Gui Hecheng]

On Sat, 2014-11-08 at 19:47 +0000, Mike Fleetwood wrote:
> On 7 November 2014 18:16, David Sterba <dsterba@suse.cz> wrote:
> > On Fri, Nov 07, 2014 at 10:07:43AM +0800, Gui Hecheng wrote:
> >> The @fi_args->num_devices in @get_fs_info() does not include seed devices.
> >> We could just correct it by searching the chunk tree and count how
> >> many dev_items there are in total which includes seed devices.
> >>
> >> Signed-off-by: Gui Hecheng <guihc.fnst@cn.fujitsu.com>
> >> ---
> >> *Note*
> >> This is just a temporary workaround to fix this problem in order to
> >> make users happy, because a new ioctl or sysfs interface to handle this
> >> problem needs more discussions and efforts. After the work implemented
> >> and accepted, we could drop this.
> >
> > Nice, thanks. I agree that this kind of workaround is best possible for
> > the moment, and I'm glad to see that it's not that much code to get the
> > seeding devices right. This would also work with older kernels without
> > the updated sysfs/ioctl interfaces, so this is likely to stay for a long
> > time.
> >
> >> +u64 find_max_id(struct btrfs_ioctl_search_args *search_args, int nr_items)
> >
> > That's a very generic name for a function that does a very specialized
> > thing, but I don't have a suggestion right now.
> 
> Is find_max_device_id a suitable name?

I think this one is really more clear.

> >> +int correct_fs_info(int fd, struct btrfs_ioctl_fs_info_args *fi_args)
> >
> > Same here, make fs_info correct but in what way? A comment would be good
> > as well.
> 
> Sorry, no suggestion for this function name.

may be @search_chunk_tree_for_fs_info?

Thanks for the suggestions.

-Gui

> Thanks,
> Mike

Re: [PATCH] btrfs-progs: skip fs with no seed when build seed/sprout mapping for fi show

[David Sterba]

On Fri, Nov 07, 2014 at 10:07:41AM +0800, Gui Hecheng wrote:
> +static int no_seed_devices(struct btrfs_fs_devices *fs_devices)

Please reverse the meaning of the function, ie. something like
'has_seed_devices'. It's more natural to deal with negation operators
when they're explicit rather than hidden.

Thanks.