* [PATCH 0/3] btrfs-progs: remove full /dev scanning
@ 2014-08-20 22:21 Eric Sandeen
2014-08-20 22:22 ` [PATCH 1/3] btrfs-progs: scan /proc/partitions not all of /dev with "-d" Eric Sandeen
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Eric Sandeen @ 2014-08-20 22:21 UTC (permalink / raw)
To: linux-btrfs
btrfs fileystem show and btrfs device scan today both have
the "-d" option to scan everything under /dev. But we also
have a mechanism to scan everything in /proc/partitions, which
should always be sufficient.
If anyone knows why we'd find something deep under /dev but
not in /proc/partitions, speak now or forever hold your peace...
Tested this by running through a matrix of -d, -m, or "" args
for show/scan, for a 2-device fs, with and without a symlinked
device, with and without a symlinked mountpoint. All output was
identical.
Thanks,
-Eric
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] btrfs-progs: scan /proc/partitions not all of /dev with "-d"
2014-08-20 22:21 [PATCH 0/3] btrfs-progs: remove full /dev scanning Eric Sandeen
@ 2014-08-20 22:22 ` Eric Sandeen
2014-08-26 8:27 ` Anand Jain
2014-08-20 22:23 ` [PATCH 2/3] btrfs-progs: don't fall back to recursive /dev scan Eric Sandeen
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2014-08-20 22:22 UTC (permalink / raw)
To: linux-btrfs
We can scan for btrfs devices in a few ways. By default
libblkid is used for "device scan" and "filesystem show";
with the -m option only mounted filesystems are scanned,
and with -d we physically read every system device.
But there's no reason for the complexity of a descent through
/dev; /proc/partitions has every device known to the kernel, so
just use that when -d is specified.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/cmds-device.c b/cmds-device.c
index b6772b9..519681a 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -224,7 +224,7 @@ static int cmd_scan_dev(int argc, char **argv)
break;
switch (c) {
case 'd':
- where = BTRFS_SCAN_DEV;
+ where = BTRFS_SCAN_PROC;
all = 1;
break;
default:
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index bf87bbe..0ad7e8f 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -614,7 +614,7 @@ static int cmd_show(int argc, char **argv)
break;
switch (c) {
case 'd':
- where = BTRFS_SCAN_DEV;
+ where = BTRFS_SCAN_PROC;
break;
case 'm':
where = BTRFS_SCAN_MOUNTED;
@@ -638,7 +638,7 @@ static int cmd_show(int argc, char **argv)
* right away
*/
if (type == BTRFS_ARG_BLKDEV) {
- if (where == BTRFS_SCAN_DEV) {
+ if (where == BTRFS_SCAN_PROC) {
/* we need to do this because
* legacy BTRFS_SCAN_DEV
* provides /dev/dm-x paths
@@ -681,7 +681,7 @@ static int cmd_show(int argc, char **argv)
}
}
- if (where == BTRFS_SCAN_DEV)
+ if (where == BTRFS_SCAN_PROC)
goto devs_only;
/* show mounted btrfs */
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] btrfs-progs: don't fall back to recursive /dev scan
2014-08-20 22:21 [PATCH 0/3] btrfs-progs: remove full /dev scanning Eric Sandeen
2014-08-20 22:22 ` [PATCH 1/3] btrfs-progs: scan /proc/partitions not all of /dev with "-d" Eric Sandeen
@ 2014-08-20 22:23 ` Eric Sandeen
2014-08-26 8:27 ` Anand Jain
2014-08-20 22:24 ` [PATCH 3/3] btrfs-progs: remove BTRFS_SCAN_DEV and btrfs_scan_one_dir Eric Sandeen
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2014-08-20 22:23 UTC (permalink / raw)
To: linux-btrfs
If we didn't find what we are looking for in /proc/partitions,
we're not going to find it by scanning every node under /dev, either.
But that's just what btrfs_scan_for_fsid() does.
Remove that fallback; at that point btrfs_scan_for_fsid() just calls
scan_for_btrfs(), so remove the wrapper & call it directly.
Side note: so, these paths always use /proc/partitions, not libblkid.
Userspace-intiated scans default to libblkid. I presume this is
part of the design, and intentional? Anyway, not changing it now!
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/disk-io.c b/disk-io.c
index d10d647..077eb7e 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1018,7 +1018,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
}
if (total_devs != 1) {
- ret = btrfs_scan_for_fsid(run_ioctl);
+ ret = scan_for_btrfs(BTRFS_SCAN_PROC, run_ioctl);
if (ret)
return ret;
}
diff --git a/utils.c b/utils.c
index d61cbec..12ed7a2 100644
--- a/utils.c
+++ b/utils.c
@@ -1151,7 +1151,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(!BTRFS_UPDATE_KERNEL)))
+ if ((ret = scan_for_btrfs(BTRFS_SCAN_PROC, !BTRFS_UPDATE_KERNEL)))
return ret;
}
@@ -1339,16 +1339,6 @@ fail:
return ret;
}
-int btrfs_scan_for_fsid(int run_ioctls)
-{
- int ret;
-
- ret = scan_for_btrfs(BTRFS_SCAN_PROC, run_ioctls);
- if (ret)
- ret = scan_for_btrfs(BTRFS_SCAN_DEV, run_ioctls);
- return ret;
-}
-
int btrfs_device_already_in_root(struct btrfs_root *root, int fd,
int super_offset)
{
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] btrfs-progs: remove BTRFS_SCAN_DEV and btrfs_scan_one_dir
2014-08-20 22:21 [PATCH 0/3] btrfs-progs: remove full /dev scanning Eric Sandeen
2014-08-20 22:22 ` [PATCH 1/3] btrfs-progs: scan /proc/partitions not all of /dev with "-d" Eric Sandeen
2014-08-20 22:23 ` [PATCH 2/3] btrfs-progs: don't fall back to recursive /dev scan Eric Sandeen
@ 2014-08-20 22:24 ` Eric Sandeen
2014-08-26 8:29 ` Anand Jain
2014-08-21 8:44 ` [PATCH 0/3] btrfs-progs: remove full /dev scanning Anand Jain
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2014-08-20 22:24 UTC (permalink / raw)
To: linux-btrfs
After the previous 2 patches, nothing uses
whole-dev-tree scanning, so remove the code which
implemented that functionality.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/utils.c b/utils.c
index 12ed7a2..765a214 100644
--- a/utils.c
+++ b/utils.c
@@ -1228,117 +1228,6 @@ void btrfs_register_one_device(char *fname)
close(fd);
}
-int btrfs_scan_one_dir(char *dirname, int run_ioctl)
-{
- DIR *dirp = NULL;
- struct dirent *dirent;
- struct pending_dir *pending;
- struct stat st;
- int ret;
- int fd;
- int dirname_len;
- char *fullpath;
- struct list_head pending_list;
- struct btrfs_fs_devices *tmp_devices;
- u64 num_devices;
-
- INIT_LIST_HEAD(&pending_list);
-
- pending = malloc(sizeof(*pending));
- if (!pending)
- return -ENOMEM;
- strcpy(pending->name, dirname);
-
-again:
- dirname_len = strlen(pending->name);
- fullpath = malloc(PATH_MAX);
- dirname = pending->name;
-
- if (!fullpath) {
- ret = -ENOMEM;
- goto fail;
- }
- dirp = opendir(dirname);
- if (!dirp) {
- fprintf(stderr, "Unable to open %s for scanning\n", dirname);
- ret = -errno;
- goto fail;
- }
- while(1) {
- dirent = readdir(dirp);
- if (!dirent)
- break;
- if (dirent->d_name[0] == '.')
- continue;
- if (dirname_len + strlen(dirent->d_name) + 2 > PATH_MAX) {
- ret = -EFAULT;
- goto fail;
- }
- snprintf(fullpath, PATH_MAX, "%s/%s", dirname, dirent->d_name);
- ret = lstat(fullpath, &st);
- if (ret < 0) {
- fprintf(stderr, "failed to stat %s\n", fullpath);
- continue;
- }
- if (S_ISLNK(st.st_mode))
- continue;
- if (S_ISDIR(st.st_mode)) {
- struct pending_dir *next = malloc(sizeof(*next));
- if (!next) {
- ret = -ENOMEM;
- goto fail;
- }
- strcpy(next->name, fullpath);
- list_add_tail(&next->list, &pending_list);
- }
- if (!S_ISBLK(st.st_mode)) {
- continue;
- }
- fd = open(fullpath, O_RDONLY);
- if (fd < 0) {
- /* ignore the following errors:
- ENXIO (device don't exists)
- ENOMEDIUM (No medium found ->
- like a cd tray empty)
- */
- if(errno != ENXIO && errno != ENOMEDIUM)
- fprintf(stderr, "failed to read %s: %s\n",
- fullpath, strerror(errno));
- continue;
- }
- ret = btrfs_scan_one_device(fd, fullpath, &tmp_devices,
- &num_devices,
- BTRFS_SUPER_INFO_OFFSET, 0);
- if (ret == 0 && run_ioctl > 0) {
- btrfs_register_one_device(fullpath);
- }
- close(fd);
- }
- if (!list_empty(&pending_list)) {
- free(pending);
- pending = list_entry(pending_list.next, struct pending_dir,
- list);
- free(fullpath);
- list_del(&pending->list);
- closedir(dirp);
- dirp = NULL;
- goto again;
- }
- ret = 0;
-fail:
- free(pending);
- free(fullpath);
- while (!list_empty(&pending_list)) {
- pending = list_entry(pending_list.next, struct pending_dir,
- list);
- list_del(&pending->list);
- free(pending);
- }
- if (dirp)
- closedir(dirp);
- return ret;
-}
-
int btrfs_device_already_in_root(struct btrfs_root *root, int fd,
int super_offset)
{
@@ -2291,9 +2180,6 @@ int scan_for_btrfs(int where, int update_kernel)
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;
case BTRFS_SCAN_LBLKID:
ret = btrfs_scan_lblkid(update_kernel);
break;
diff --git a/utils.h b/utils.h
index 0c9b65f..3a05131 100644
--- a/utils.h
+++ b/utils.h
@@ -27,9 +27,8 @@
#define BTRFS_MKFS_SMALL_VOLUME_SIZE (1024 * 1024 * 1024)
#define BTRFS_SCAN_PROC (1ULL << 0)
-#define BTRFS_SCAN_DEV (1ULL << 1)
-#define BTRFS_SCAN_MOUNTED (1ULL << 2)
-#define BTRFS_SCAN_LBLKID (1ULL << 3)
+#define BTRFS_SCAN_MOUNTED (1ULL << 1)
+#define BTRFS_SCAN_LBLKID (1ULL << 2)
#define BTRFS_UPDATE_KERNEL 1
@@ -70,7 +69,6 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
u32 sectorsize);
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);
char *canonicalize_dm_name(const char *ptname);
char *canonicalize_path(const char *path);
int check_mounted(const char *devicename);
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-20 22:21 [PATCH 0/3] btrfs-progs: remove full /dev scanning Eric Sandeen
` (2 preceding siblings ...)
2014-08-20 22:24 ` [PATCH 3/3] btrfs-progs: remove BTRFS_SCAN_DEV and btrfs_scan_one_dir Eric Sandeen
@ 2014-08-21 8:44 ` Anand Jain
2014-08-21 14:29 ` Eric Sandeen
2014-08-21 18:08 ` Goffredo Baroncelli
2014-08-26 9:52 ` Anand Jain
5 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2014-08-21 8:44 UTC (permalink / raw)
To: Eric Sandeen, linux-btrfs
A long time back there was an attempt to remove it but
this avoided it. Pls ref to the link in this discussion.
https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg27272.html
Thanks, Anand
On 08/21/2014 06:21 AM, Eric Sandeen wrote:
> btrfs fileystem show and btrfs device scan today both have
> the "-d" option to scan everything under /dev. But we also
> have a mechanism to scan everything in /proc/partitions, which
> should always be sufficient.
>
> If anyone knows why we'd find something deep under /dev but
> not in /proc/partitions, speak now or forever hold your peace...
>
> Tested this by running through a matrix of -d, -m, or "" args
> for show/scan, for a 2-device fs, with and without a symlinked
> device, with and without a symlinked mountpoint. All output was
> identical.
>
> Thanks,
> -Eric
> --
> 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] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-21 8:44 ` [PATCH 0/3] btrfs-progs: remove full /dev scanning Anand Jain
@ 2014-08-21 14:29 ` Eric Sandeen
2014-08-26 8:24 ` Anand Jain
2014-08-26 11:08 ` David Sterba
0 siblings, 2 replies; 17+ messages in thread
From: Eric Sandeen @ 2014-08-21 14:29 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 8/21/14, 3:44 AM, Anand Jain wrote:
>
>
> A long time back there was an attempt to remove it but
> this avoided it. Pls ref to the link in this discussion.
>
> https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg27272.html
Hm, I guess I don't understand this. How is udev related to whether
or not /proc/partitions is sufficient vs. recursive /dev?
To be clear, my patchset keeps the -d / --all-devices option.
It simply discovers all devices via /proc/partitions, not via
a full /dev tree walk.
Thanks,
-Eric
> Thanks, Anand
>
> On 08/21/2014 06:21 AM, Eric Sandeen wrote:
>> btrfs fileystem show and btrfs device scan today both have
>> the "-d" option to scan everything under /dev. But we also
>> have a mechanism to scan everything in /proc/partitions, which
>> should always be sufficient.
>>
>> If anyone knows why we'd find something deep under /dev but
>> not in /proc/partitions, speak now or forever hold your peace...
>>
>> Tested this by running through a matrix of -d, -m, or "" args
>> for show/scan, for a 2-device fs, with and without a symlinked
>> device, with and without a symlinked mountpoint. All output was
>> identical.
>>
>> Thanks,
>> -Eric
>> --
>> 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] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-20 22:21 [PATCH 0/3] btrfs-progs: remove full /dev scanning Eric Sandeen
` (3 preceding siblings ...)
2014-08-21 8:44 ` [PATCH 0/3] btrfs-progs: remove full /dev scanning Anand Jain
@ 2014-08-21 18:08 ` Goffredo Baroncelli
2014-08-26 10:55 ` David Sterba
2014-08-26 9:52 ` Anand Jain
5 siblings, 1 reply; 17+ messages in thread
From: Goffredo Baroncelli @ 2014-08-21 18:08 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
On 08/21/2014 12:21 AM, Eric Sandeen wrote:
> btrfs fileystem show and btrfs device scan today both have
> the "-d" option to scan everything under /dev. But we also
> have a mechanism to scan everything in /proc/partitions, which
> should always be sufficient.
>
> If anyone knows why we'd find something deep under /dev but
> not in /proc/partitions, speak now or forever hold your peace...
In the past I worked on this. From my commit messages [1]
[...]
the devices "scanned" are extracted from /proc/partitions. This
should avoid to scan devices not suitable for a btrfs filesystem like cdrom
and floppy or to scan not existant devices.
[...]
Of course I (we) don't care cdrom and/or floppy, but this raises
the question: are there other block devices which aren't showed in
proc/partitions ?
I am thinking to some less common hardware like
NON USB sd disk (I saw this kind of hardware, but now I don't have
it in my hands....).
GB
[1] 0dbd99fb3e117cd5f87eda492b6b4fab1b5bea23
>
> Tested this by running through a matrix of -d, -m, or "" args
> for show/scan, for a 2-device fs, with and without a symlinked
> device, with and without a symlinked mountpoint. All output was
> identical.
>
> Thanks,
> -Eric
> --
> 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
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-21 14:29 ` Eric Sandeen
@ 2014-08-26 8:24 ` Anand Jain
2014-08-26 11:08 ` David Sterba
1 sibling, 0 replies; 17+ messages in thread
From: Anand Jain @ 2014-08-26 8:24 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
On 21/08/2014 22:29, Eric Sandeen wrote:
> On 8/21/14, 3:44 AM, Anand Jain wrote:
>>
>>
>> A long time back there was an attempt to remove it but
>> this avoided it. Pls ref to the link in this discussion.
>>
>> https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg27272.html
>
> Hm, I guess I don't understand this. How is udev related to whether
> or not /proc/partitions is sufficient vs. recursive /dev?
>
> To be clear, my patchset keeps the -d / --all-devices option.
>
> It simply discovers all devices via /proc/partitions, not via
> a full /dev tree walk.
Makes sense.
Thanks, Anand
> Thanks,
> -Eric
>
>
>> Thanks, Anand
>>
>> On 08/21/2014 06:21 AM, Eric Sandeen wrote:
>>> btrfs fileystem show and btrfs device scan today both have
>>> the "-d" option to scan everything under /dev. But we also
>>> have a mechanism to scan everything in /proc/partitions, which
>>> should always be sufficient.
>>>
>>> If anyone knows why we'd find something deep under /dev but
>>> not in /proc/partitions, speak now or forever hold your peace...
>>>
>>> Tested this by running through a matrix of -d, -m, or "" args
>>> for show/scan, for a 2-device fs, with and without a symlinked
>>> device, with and without a symlinked mountpoint. All output was
>>> identical.
>>>
>>> Thanks,
>>> -Eric
>>> --
>>> 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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: scan /proc/partitions not all of /dev with "-d"
2014-08-20 22:22 ` [PATCH 1/3] btrfs-progs: scan /proc/partitions not all of /dev with "-d" Eric Sandeen
@ 2014-08-26 8:27 ` Anand Jain
0 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2014-08-26 8:27 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
Reviewed-by: Anand Jain <anand.jain@oracle.com>
On 21/08/2014 06:22, Eric Sandeen wrote:
> We can scan for btrfs devices in a few ways. By default
> libblkid is used for "device scan" and "filesystem show";
> with the -m option only mounted filesystems are scanned,
> and with -d we physically read every system device.
>
> But there's no reason for the complexity of a descent through
> /dev; /proc/partitions has every device known to the kernel, so
> just use that when -d is specified.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> diff --git a/cmds-device.c b/cmds-device.c
> index b6772b9..519681a 100644
> --- a/cmds-device.c
> +++ b/cmds-device.c
> @@ -224,7 +224,7 @@ static int cmd_scan_dev(int argc, char **argv)
> break;
> switch (c) {
> case 'd':
> - where = BTRFS_SCAN_DEV;
> + where = BTRFS_SCAN_PROC;
> all = 1;
> break;
> default:
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index bf87bbe..0ad7e8f 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -614,7 +614,7 @@ static int cmd_show(int argc, char **argv)
> break;
> switch (c) {
> case 'd':
> - where = BTRFS_SCAN_DEV;
> + where = BTRFS_SCAN_PROC;
> break;
> case 'm':
> where = BTRFS_SCAN_MOUNTED;
> @@ -638,7 +638,7 @@ static int cmd_show(int argc, char **argv)
> * right away
> */
> if (type == BTRFS_ARG_BLKDEV) {
> - if (where == BTRFS_SCAN_DEV) {
> + if (where == BTRFS_SCAN_PROC) {
> /* we need to do this because
> * legacy BTRFS_SCAN_DEV
> * provides /dev/dm-x paths
> @@ -681,7 +681,7 @@ static int cmd_show(int argc, char **argv)
> }
> }
>
> - if (where == BTRFS_SCAN_DEV)
> + if (where == BTRFS_SCAN_PROC)
> goto devs_only;
>
> /* show mounted btrfs */
>
> --
> 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] 17+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: don't fall back to recursive /dev scan
2014-08-20 22:23 ` [PATCH 2/3] btrfs-progs: don't fall back to recursive /dev scan Eric Sandeen
@ 2014-08-26 8:27 ` Anand Jain
0 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2014-08-26 8:27 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
Reviewed-by: Anand Jain <anand.jain@oracle.com>
On 21/08/2014 06:23, Eric Sandeen wrote:
> If we didn't find what we are looking for in /proc/partitions,
> we're not going to find it by scanning every node under /dev, either.
>
> But that's just what btrfs_scan_for_fsid() does.
>
> Remove that fallback; at that point btrfs_scan_for_fsid() just calls
> scan_for_btrfs(), so remove the wrapper & call it directly.
>
> Side note: so, these paths always use /proc/partitions, not libblkid.
> Userspace-intiated scans default to libblkid. I presume this is
> part of the design, and intentional? Anyway, not changing it now!
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/disk-io.c b/disk-io.c
> index d10d647..077eb7e 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1018,7 +1018,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
> }
>
> if (total_devs != 1) {
> - ret = btrfs_scan_for_fsid(run_ioctl);
> + ret = scan_for_btrfs(BTRFS_SCAN_PROC, run_ioctl);
> if (ret)
> return ret;
> }
> diff --git a/utils.c b/utils.c
> index d61cbec..12ed7a2 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1151,7 +1151,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(!BTRFS_UPDATE_KERNEL)))
> + if ((ret = scan_for_btrfs(BTRFS_SCAN_PROC, !BTRFS_UPDATE_KERNEL)))
> return ret;
> }
>
> @@ -1339,16 +1339,6 @@ fail:
> return ret;
> }
>
> -int btrfs_scan_for_fsid(int run_ioctls)
> -{
> - int ret;
> -
> - ret = scan_for_btrfs(BTRFS_SCAN_PROC, run_ioctls);
> - if (ret)
> - ret = scan_for_btrfs(BTRFS_SCAN_DEV, run_ioctls);
> - return ret;
> -}
> -
> int btrfs_device_already_in_root(struct btrfs_root *root, int fd,
> int super_offset)
> {
>
> --
> 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] 17+ messages in thread
* Re: [PATCH 3/3] btrfs-progs: remove BTRFS_SCAN_DEV and btrfs_scan_one_dir
2014-08-20 22:24 ` [PATCH 3/3] btrfs-progs: remove BTRFS_SCAN_DEV and btrfs_scan_one_dir Eric Sandeen
@ 2014-08-26 8:29 ` Anand Jain
0 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2014-08-26 8:29 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
:-( to see the entire btrfs_scan_one_dir() going away.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
On 21/08/2014 06:24, Eric Sandeen wrote:
> After the previous 2 patches, nothing uses
> whole-dev-tree scanning, so remove the code which
> implemented that functionality.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/utils.c b/utils.c
> index 12ed7a2..765a214 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1228,117 +1228,6 @@ void btrfs_register_one_device(char *fname)
> close(fd);
> }
>
> -int btrfs_scan_one_dir(char *dirname, int run_ioctl)
> -{
> - DIR *dirp = NULL;
> - struct dirent *dirent;
> - struct pending_dir *pending;
> - struct stat st;
> - int ret;
> - int fd;
> - int dirname_len;
> - char *fullpath;
> - struct list_head pending_list;
> - struct btrfs_fs_devices *tmp_devices;
> - u64 num_devices;
> -
> - INIT_LIST_HEAD(&pending_list);
> -
> - pending = malloc(sizeof(*pending));
> - if (!pending)
> - return -ENOMEM;
> - strcpy(pending->name, dirname);
> -
> -again:
> - dirname_len = strlen(pending->name);
> - fullpath = malloc(PATH_MAX);
> - dirname = pending->name;
> -
> - if (!fullpath) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> - dirp = opendir(dirname);
> - if (!dirp) {
> - fprintf(stderr, "Unable to open %s for scanning\n", dirname);
> - ret = -errno;
> - goto fail;
> - }
> - while(1) {
> - dirent = readdir(dirp);
> - if (!dirent)
> - break;
> - if (dirent->d_name[0] == '.')
> - continue;
> - if (dirname_len + strlen(dirent->d_name) + 2 > PATH_MAX) {
> - ret = -EFAULT;
> - goto fail;
> - }
> - snprintf(fullpath, PATH_MAX, "%s/%s", dirname, dirent->d_name);
> - ret = lstat(fullpath, &st);
> - if (ret < 0) {
> - fprintf(stderr, "failed to stat %s\n", fullpath);
> - continue;
> - }
> - if (S_ISLNK(st.st_mode))
> - continue;
> - if (S_ISDIR(st.st_mode)) {
> - struct pending_dir *next = malloc(sizeof(*next));
> - if (!next) {
> - ret = -ENOMEM;
> - goto fail;
> - }
> - strcpy(next->name, fullpath);
> - list_add_tail(&next->list, &pending_list);
> - }
> - if (!S_ISBLK(st.st_mode)) {
> - continue;
> - }
> - fd = open(fullpath, O_RDONLY);
> - if (fd < 0) {
> - /* ignore the following errors:
> - ENXIO (device don't exists)
> - ENOMEDIUM (No medium found ->
> - like a cd tray empty)
> - */
> - if(errno != ENXIO && errno != ENOMEDIUM)
> - fprintf(stderr, "failed to read %s: %s\n",
> - fullpath, strerror(errno));
> - continue;
> - }
> - ret = btrfs_scan_one_device(fd, fullpath, &tmp_devices,
> - &num_devices,
> - BTRFS_SUPER_INFO_OFFSET, 0);
> - if (ret == 0 && run_ioctl > 0) {
> - btrfs_register_one_device(fullpath);
> - }
> - close(fd);
> - }
> - if (!list_empty(&pending_list)) {
> - free(pending);
> - pending = list_entry(pending_list.next, struct pending_dir,
> - list);
> - free(fullpath);
> - list_del(&pending->list);
> - closedir(dirp);
> - dirp = NULL;
> - goto again;
> - }
> - ret = 0;
> -fail:
> - free(pending);
> - free(fullpath);
> - while (!list_empty(&pending_list)) {
> - pending = list_entry(pending_list.next, struct pending_dir,
> - list);
> - list_del(&pending->list);
> - free(pending);
> - }
> - if (dirp)
> - closedir(dirp);
> - return ret;
> -}
> -
> int btrfs_device_already_in_root(struct btrfs_root *root, int fd,
> int super_offset)
> {
> @@ -2291,9 +2180,6 @@ int scan_for_btrfs(int where, int update_kernel)
> 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;
> case BTRFS_SCAN_LBLKID:
> ret = btrfs_scan_lblkid(update_kernel);
> break;
> diff --git a/utils.h b/utils.h
> index 0c9b65f..3a05131 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -27,9 +27,8 @@
> #define BTRFS_MKFS_SMALL_VOLUME_SIZE (1024 * 1024 * 1024)
>
> #define BTRFS_SCAN_PROC (1ULL << 0)
> -#define BTRFS_SCAN_DEV (1ULL << 1)
> -#define BTRFS_SCAN_MOUNTED (1ULL << 2)
> -#define BTRFS_SCAN_LBLKID (1ULL << 3)
> +#define BTRFS_SCAN_MOUNTED (1ULL << 1)
> +#define BTRFS_SCAN_LBLKID (1ULL << 2)
>
> #define BTRFS_UPDATE_KERNEL 1
>
> @@ -70,7 +69,6 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
> u32 sectorsize);
> 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);
> char *canonicalize_dm_name(const char *ptname);
> char *canonicalize_path(const char *path);
> int check_mounted(const char *devicename);
>
> --
> 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] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-20 22:21 [PATCH 0/3] btrfs-progs: remove full /dev scanning Eric Sandeen
` (4 preceding siblings ...)
2014-08-21 18:08 ` Goffredo Baroncelli
@ 2014-08-26 9:52 ` Anand Jain
5 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2014-08-26 9:52 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-btrfs
Apparently we don't need -d option at all. The libblkid does
cover both udev and /proc/partitions.
----
EVALUATE=<methods>
Defines LABEL and UUID evaluation method(s). Currently, the libblkid
library supports "udev" and "scan" methods. More than one methods may be
specified in a comma separated list. Default is "udev,scan". The "udev"
method uses udev /dev/disk/by-* symlinks and the "scan" method scans all
block devices from the /proc/partitions file.
----
We could mark -d option as deprecated (in btrfs dev scan and
btrfs fi show) or if agreed I would prefer totally removal.
That will clean up the code quite nicely.
Thanks, Anand
On 21/08/2014 06:21, Eric Sandeen wrote:
> btrfs fileystem show and btrfs device scan today both have
> the "-d" option to scan everything under /dev. But we also
> have a mechanism to scan everything in /proc/partitions, which
> should always be sufficient.
>
> If anyone knows why we'd find something deep under /dev but
> not in /proc/partitions, speak now or forever hold your peace...
>
> Tested this by running through a matrix of -d, -m, or "" args
> for show/scan, for a 2-device fs, with and without a symlinked
> device, with and without a symlinked mountpoint. All output was
> identical.
>
> Thanks,
> -Eric
> --
> 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] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-21 18:08 ` Goffredo Baroncelli
@ 2014-08-26 10:55 ` David Sterba
2014-08-26 14:51 ` Eric Sandeen
0 siblings, 1 reply; 17+ messages in thread
From: David Sterba @ 2014-08-26 10:55 UTC (permalink / raw)
To: Goffredo Baroncelli; +Cc: Eric Sandeen, linux-btrfs
On Thu, Aug 21, 2014 at 08:08:57PM +0200, Goffredo Baroncelli wrote:
> Of course I (we) don't care cdrom and/or floppy, but this raises
> the question: are there other block devices which aren't showed in
> proc/partitions ?
cdrom appears as /dev/sr0, floppy is /dev/fd0 if the respective modules
are loaded, no breakage here.
> I am thinking to some less common hardware like
> NON USB sd disk (I saw this kind of hardware, but now I don't have
> it in my hands....).
A block device whose driver calls register_blkdev should appear in
proc/partitions, if not then I think it's a bug or a very non-standard
interface.
I don't know about any other cases where full /dev scan would reveal
something that /proc/partitions not, so I guess it's safe to remove it.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-21 14:29 ` Eric Sandeen
2014-08-26 8:24 ` Anand Jain
@ 2014-08-26 11:08 ` David Sterba
1 sibling, 0 replies; 17+ messages in thread
From: David Sterba @ 2014-08-26 11:08 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Anand Jain, linux-btrfs
On Thu, Aug 21, 2014 at 09:29:56AM -0500, Eric Sandeen wrote:
> On 8/21/14, 3:44 AM, Anand Jain wrote:
> >
> >
> > A long time back there was an attempt to remove it but
> > this avoided it. Pls ref to the link in this discussion.
> >
> > https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg27272.html
>
> Hm, I guess I don't understand this. How is udev related to whether
> or not /proc/partitions is sufficient vs. recursive /dev?
The argument about udev is probably invalid, /proc/filesystem is
populated by kernel afaics.
> To be clear, my patchset keeps the -d / --all-devices option.
>
> It simply discovers all devices via /proc/partitions, not via
> a full /dev tree walk.
I'm fine with that.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-26 10:55 ` David Sterba
@ 2014-08-26 14:51 ` Eric Sandeen
2014-08-26 14:53 ` Chris Mason
0 siblings, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2014-08-26 14:51 UTC (permalink / raw)
To: dsterba, Goffredo Baroncelli, linux-btrfs
On 8/26/14, 5:55 AM, David Sterba wrote:
> On Thu, Aug 21, 2014 at 08:08:57PM +0200, Goffredo Baroncelli wrote:
>> Of course I (we) don't care cdrom and/or floppy, but this raises
>> the question: are there other block devices which aren't showed in
>> proc/partitions ?
>
> cdrom appears as /dev/sr0, floppy is /dev/fd0 if the respective modules
> are loaded, no breakage here.
>
>> I am thinking to some less common hardware like
>> NON USB sd disk (I saw this kind of hardware, but now I don't have
>> it in my hands....).
>
> A block device whose driver calls register_blkdev should appear in
> proc/partitions, if not then I think it's a bug or a very non-standard
> interface.
>
> I don't know about any other cases where full /dev scan would reveal
> something that /proc/partitions not, so I guess it's safe to remove it.
Yesterday, Chris did mention <something something mumble> about /dev/mapper/*
[12:57] <cmason> I got inconsistent results from /proc/partitions
[12:57] <cmason> this was 3 years ago now
[12:58] <cmason> it didn't find everything for dm devices, yeah
[12:58] <cmason> especially dm multipath got really confused
and I'll be honest, I didn't test with devicemapper devices, much
less dm multipath, so I suppose that's warranted prior to removal.
I'll see what I can do.
If there's a problem, I bet there's a solution that doesn't involve
scanning everything under /dev ...
-Eric
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-26 14:51 ` Eric Sandeen
@ 2014-08-26 14:53 ` Chris Mason
2014-08-26 22:56 ` Anand Jain
0 siblings, 1 reply; 17+ messages in thread
From: Chris Mason @ 2014-08-26 14:53 UTC (permalink / raw)
To: Eric Sandeen, dsterba, Goffredo Baroncelli, linux-btrfs
On 08/26/2014 10:51 AM, Eric Sandeen wrote:
> On 8/26/14, 5:55 AM, David Sterba wrote:
>> On Thu, Aug 21, 2014 at 08:08:57PM +0200, Goffredo Baroncelli wrote:
>>> Of course I (we) don't care cdrom and/or floppy, but this raises
>>> the question: are there other block devices which aren't showed in
>>> proc/partitions ?
>>
>> cdrom appears as /dev/sr0, floppy is /dev/fd0 if the respective modules
>> are loaded, no breakage here.
>>
>>> I am thinking to some less common hardware like
>>> NON USB sd disk (I saw this kind of hardware, but now I don't have
>>> it in my hands....).
>>
>> A block device whose driver calls register_blkdev should appear in
>> proc/partitions, if not then I think it's a bug or a very non-standard
>> interface.
>>
>> I don't know about any other cases where full /dev scan would reveal
>> something that /proc/partitions not, so I guess it's safe to remove it.
>
> Yesterday, Chris did mention <something something mumble> about /dev/mapper/*
>
> [12:57] <cmason> I got inconsistent results from /proc/partitions
> [12:57] <cmason> this was 3 years ago now
> [12:58] <cmason> it didn't find everything for dm devices, yeah
> [12:58] <cmason> especially dm multipath got really confused
>
> and I'll be honest, I didn't test with devicemapper devices, much
> less dm multipath, so I suppose that's warranted prior to removal.
>
> I'll see what I can do.
>
> If there's a problem, I bet there's a solution that doesn't involve
> scanning everything under /dev ...
That's definitely the problem I hit, but it was years ago. I'm willing
to bet we're safe to keep /proc/partitions now.
-chris
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: remove full /dev scanning
2014-08-26 14:53 ` Chris Mason
@ 2014-08-26 22:56 ` Anand Jain
0 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2014-08-26 22:56 UTC (permalink / raw)
To: Chris Mason; +Cc: Eric Sandeen, dsterba, Goffredo Baroncelli, linux-btrfs
On 26/08/2014 22:53, Chris Mason wrote:
> On 08/26/2014 10:51 AM, Eric Sandeen wrote:
>> On 8/26/14, 5:55 AM, David Sterba wrote:
>>> On Thu, Aug 21, 2014 at 08:08:57PM +0200, Goffredo Baroncelli wrote:
>>>> Of course I (we) don't care cdrom and/or floppy, but this raises
>>>> the question: are there other block devices which aren't showed in
>>>> proc/partitions ?
>>>
>>> cdrom appears as /dev/sr0, floppy is /dev/fd0 if the respective modules
>>> are loaded, no breakage here.
>>>
>>>> I am thinking to some less common hardware like
>>>> NON USB sd disk (I saw this kind of hardware, but now I don't have
>>>> it in my hands....).
>>>
>>> A block device whose driver calls register_blkdev should appear in
>>> proc/partitions, if not then I think it's a bug or a very non-standard
>>> interface.
>>>
>>> I don't know about any other cases where full /dev scan would reveal
>>> something that /proc/partitions not, so I guess it's safe to remove it.
>>
>> Yesterday, Chris did mention <something something mumble> about /dev/mapper/*
>>
>> [12:57] <cmason> I got inconsistent results from /proc/partitions
>> [12:57] <cmason> this was 3 years ago now
>> [12:58] <cmason> it didn't find everything for dm devices, yeah
>> [12:58] <cmason> especially dm multipath got really confused
>>
>> and I'll be honest, I didn't test with devicemapper devices, much
>> less dm multipath, so I suppose that's warranted prior to removal.
>>
>> I'll see what I can do.
>>
>> If there's a problem, I bet there's a solution that doesn't involve
>> scanning everything under /dev ...
>
> That's definitely the problem I hit, but it was years ago. I'm willing
> to bet we're safe to keep /proc/partitions now.
I was tempted to remove BTRFS_SCAN_PROC completely when I introduced
BTRFS_SCAN_LBLKID, but then there was less clarity on this as it is
now.
The point I am trying to make is that we could remove BTRFS_SCAN_PROC
all together and keep only the BTRFS_SCAN_LBLKID.
(Which means we shall remove the -d option under btrfs filesystem
show and btrfs device scan).
We could remove scanning /proc/partition (the BTRFS_SCAN_PROC)
because the libblkid (under scan method BTRFS_SCAN_LBLKID) would scan
both udev and /proc/partition by default. (as in the libblkid
man page). Further libblkid is end user configurable. end user can
override the default configuration and can scan either of udev
/dev/disk/by-* symlinks or /proc/partitions only.
Further if still some device is not found then it would be a libblkid
bug/enhancement, so we need to fix it under libblkid.
The scan method BTRFS_SCAN_PROC is redundant.
Thanks, Anand
> -chris
>
> --
> 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] 17+ messages in thread
end of thread, other threads:[~2014-08-26 22:56 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-20 22:21 [PATCH 0/3] btrfs-progs: remove full /dev scanning Eric Sandeen
2014-08-20 22:22 ` [PATCH 1/3] btrfs-progs: scan /proc/partitions not all of /dev with "-d" Eric Sandeen
2014-08-26 8:27 ` Anand Jain
2014-08-20 22:23 ` [PATCH 2/3] btrfs-progs: don't fall back to recursive /dev scan Eric Sandeen
2014-08-26 8:27 ` Anand Jain
2014-08-20 22:24 ` [PATCH 3/3] btrfs-progs: remove BTRFS_SCAN_DEV and btrfs_scan_one_dir Eric Sandeen
2014-08-26 8:29 ` Anand Jain
2014-08-21 8:44 ` [PATCH 0/3] btrfs-progs: remove full /dev scanning Anand Jain
2014-08-21 14:29 ` Eric Sandeen
2014-08-26 8:24 ` Anand Jain
2014-08-26 11:08 ` David Sterba
2014-08-21 18:08 ` Goffredo Baroncelli
2014-08-26 10:55 ` David Sterba
2014-08-26 14:51 ` Eric Sandeen
2014-08-26 14:53 ` Chris Mason
2014-08-26 22:56 ` Anand Jain
2014-08-26 9:52 ` 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.