diff mbox series

[2/2] btrfs-progs: Ignore path devices during scan - static build support

Message ID 20210928123730.393551-2-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series [1/2] btrfs-progs: Ignore path device during device scan | expand

Commit Message

Nikolay Borisov Sept. 28, 2021, 12:37 p.m. UTC
Since libudev doesn't provide a static version of the library for static
build btrfs-progs will have to provide manual fallback. THis change does
exactly this by parsing the udev database files hosted /run/udev/data/.
Under that directory every block device should have a file with the
following name: bMAJ:MIN. So implement the bare minimum code necessary
to parse this file and search for the presence of DM_MULTIPATH_DEVICE_PATH
udev attribute. This could likely be racy since access to the udev
database is done outside of libudev but that's the best that can be
done when implementing this manually.

To reduce duplication of code also factor out the specifics of
is_path_device to __is_path_device which will contain the appropriate
implementation according to the build mode (i.e relying on libudev in
case of normal build or manual fall back code in case of static build)
or simply utilize the old logic (in case of a normal build and libudev
missing).

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 common/device-scan.c | 66 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 9 deletions(-)

Comments

David Sterba Sept. 29, 2021, 6:46 p.m. UTC | #1
On Tue, Sep 28, 2021 at 03:37:30PM +0300, Nikolay Borisov wrote:
> @@ -372,23 +373,56 @@ void free_seen_fsid(struct seen_fsid *seen_fsid_hash[])
>  	}
>  }
>  
> -#ifdef HAVE_LIBUDEV
> -static bool is_path_device(char *device_path)
> +#ifdef STATIC_BUILD
> +static bool __is_path_device(dev_t dev)
> +{
> +	FILE *file;
> +	char *line = NULL;
> +	size_t len = 0;
> +	ssize_t nread;
> +	bool ret = false;
> +	int ret2;
> +	struct stat dev_stat;
> +	char path[100];

	char path[PATH_MAX];

No arbitrary constants please. For paths always use a full buffer size,
though in this case it's not strictly necessary.

> +
> +	ret2 = snprintf(path, 100, "/run/udev/data/b%u:%u", major(dev_stat.st_rdev),
> +			minor(dev_stat.st_rdev));
> +
> +	if (ret2 >= 100 || ret2 < 0)

So >= 100 never happens and with PATH_MAX you can drop the part of the
condition as well.

> +		return false;
> +
> +	file = fopen(path, "r");
> +	if (file == NULL)
> +		return false;
> +
> +	while ((nread = getline(&line, &len, file)) != -1) {
> +		if (strstr(line, "DM_MULTIPATH_DEVICE_PATH=1")) {

So this is peeking into udev internal files but I like that you do
strstr, that sounds future proof enough for a fallback.

> +			ret = true;
> +			printf("found dm multipath line: %s\n", line);

Is this a debugging print? We have the pr_verbose helper that takes a
level of verbosity so for debugging you can do pr_verbose(3, "...").
This hasnt' been used much but messages used for developing a feature
and making sure it works as expected can be turned into high verbose
level messages for free and one day it becomes useful.

> +			break;
> +		}
> +	}
> +
> +	if (line)
> +		free(line);
> +
> +	fclose(file);
> +
> +	return ret;
> +}
> +#elif defined(HAVE_LIBUDEV)
> +static bool __is_path_device(dev_t device)

Please avoid functions with __, also s/path/multipath/ would be much
more clear.

>  {
>  	struct udev *udev = NULL;
>  	struct udev_device *dev = NULL;
> -	struct stat dev_stat;
>  	const char *val;
>  	bool ret = false;
>  
> -	if (stat(device_path, &dev_stat) < 0)
> -		return false;
> -
>  	udev = udev_new();
>  	if (!udev)
>  		goto out;
>  
> -	dev = udev_device_new_from_devnum(udev, 'b', dev_stat.st_rdev);
> +	dev = udev_device_new_from_devnum(udev, 'b', device);
>  	if (!dev)
>  		goto out;
>  
> @@ -401,8 +435,24 @@ static bool is_path_device(char *device_path)
>  
>  	return ret;
>  }
> +#else
> +static bool __is_path_device(dev_t device)
> +{
> +	return false;
> +}
>  #endif
>  
> +static bool is_path_device(char *device_path)
> +{
> +	struct stat dev_stat;
> +
> +	if (stat(device_path, &dev_stat) < 0)
> +		return false;
> +
> +	return __is_path_device(dev_stat.st_rdev);
> +
> +}
> +
>  int btrfs_scan_devices(int verbose)
>  {
>  	int fd = -1;
> @@ -433,10 +483,8 @@ int btrfs_scan_devices(int verbose)
>  		/* if we are here its definitely a btrfs disk*/
>  		strncpy_null(path, blkid_dev_devname(dev));
>  
> -#ifdef HAVE_LIBUDEV
>  		if (is_path_device(path))
>  			continue;
> -#endif
>  
>  		fd = open(path, O_RDONLY);
>  		if (fd < 0) {
> -- 
> 2.17.1
diff mbox series

Patch

diff --git a/common/device-scan.c b/common/device-scan.c
index 2ed0e34d3664..9779dd1aedf3 100644
--- a/common/device-scan.c
+++ b/common/device-scan.c
@@ -29,6 +29,7 @@ 
 #include <dirent.h>
 #include <blkid/blkid.h>
 #include <uuid/uuid.h>
+#include <sys/sysmacros.h>
 #ifdef HAVE_LIBUDEV
 #include <sys/stat.h>
 #include <libudev.h>
@@ -372,23 +373,56 @@  void free_seen_fsid(struct seen_fsid *seen_fsid_hash[])
 	}
 }
 
-#ifdef HAVE_LIBUDEV
-static bool is_path_device(char *device_path)
+#ifdef STATIC_BUILD
+static bool __is_path_device(dev_t dev)
+{
+	FILE *file;
+	char *line = NULL;
+	size_t len = 0;
+	ssize_t nread;
+	bool ret = false;
+	int ret2;
+	struct stat dev_stat;
+	char path[100];
+
+	ret2 = snprintf(path, 100, "/run/udev/data/b%u:%u", major(dev_stat.st_rdev),
+			minor(dev_stat.st_rdev));
+
+	if (ret2 >= 100 || ret2 < 0)
+		return false;
+
+	file = fopen(path, "r");
+	if (file == NULL)
+		return false;
+
+	while ((nread = getline(&line, &len, file)) != -1) {
+		if (strstr(line, "DM_MULTIPATH_DEVICE_PATH=1")) {
+			ret = true;
+			printf("found dm multipath line: %s\n", line);
+			break;
+		}
+	}
+
+	if (line)
+		free(line);
+
+	fclose(file);
+
+	return ret;
+}
+#elif defined(HAVE_LIBUDEV)
+static bool __is_path_device(dev_t device)
 {
 	struct udev *udev = NULL;
 	struct udev_device *dev = NULL;
-	struct stat dev_stat;
 	const char *val;
 	bool ret = false;
 
-	if (stat(device_path, &dev_stat) < 0)
-		return false;
-
 	udev = udev_new();
 	if (!udev)
 		goto out;
 
-	dev = udev_device_new_from_devnum(udev, 'b', dev_stat.st_rdev);
+	dev = udev_device_new_from_devnum(udev, 'b', device);
 	if (!dev)
 		goto out;
 
@@ -401,8 +435,24 @@  static bool is_path_device(char *device_path)
 
 	return ret;
 }
+#else
+static bool __is_path_device(dev_t device)
+{
+	return false;
+}
 #endif
 
+static bool is_path_device(char *device_path)
+{
+	struct stat dev_stat;
+
+	if (stat(device_path, &dev_stat) < 0)
+		return false;
+
+	return __is_path_device(dev_stat.st_rdev);
+
+}
+
 int btrfs_scan_devices(int verbose)
 {
 	int fd = -1;
@@ -433,10 +483,8 @@  int btrfs_scan_devices(int verbose)
 		/* if we are here its definitely a btrfs disk*/
 		strncpy_null(path, blkid_dev_devname(dev));
 
-#ifdef HAVE_LIBUDEV
 		if (is_path_device(path))
 			continue;
-#endif
 
 		fd = open(path, O_RDONLY);
 		if (fd < 0) {