Re: [PATCH 2/2] btrfs-progs: Ignore path devices during scan - static build support
From: David Sterba <hidden>
Date: 2021-09-29 18:46:59
On Tue, Sep 28, 2021 at 03:37:30PM +0300, Nikolay Borisov wrote:
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
{ 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