[LTP] [PATCH] tst_find_backing_dev: match mount point if major/minor can't be found

[LTP] [PATCH] tst_find_backing_dev: match mount point if major/minor can't be found

[Jan Stancek]

Test fails on btrfs, because tst_find_backing_dev doesn't find
major/minor returned by stat()

Per https://lwn.net/Articles/866582
"btrfs allocates a separate device number (the usual major/minor pair)
for each subvolume ... and call to on a file within a subvolume will
return a device number that does not exist in files like mountinfo."

As fallback, if major/minor can't be found, use best match of mount path.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 lib/tst_device.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/tst_device.c b/lib/tst_device.c
index d296f9118cde..e560ec97460b 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -506,14 +506,22 @@ unsigned long tst_dev_bytes_written(const char *dev)
 	return dev_bytes_written;
 }
 
+static int count_match_len(const char *first, const char *second)
+{
+	int len = 0;
+
+	while (*first && *first++ == *second++)
+		len++;
+
+	return len;
+}
+
 void tst_find_backing_dev(const char *path, char *dev)
 {
 	struct stat buf;
 	FILE *file;
-	char line[PATH_MAX];
-	char *pre = NULL;
-	char *next = NULL;
-	unsigned int dev_major, dev_minor, line_mjr, line_mnr;
+	char line[PATH_MAX], mnt_point[PATH_MAX], mnt_source[PATH_MAX];
+	unsigned int dev_major, dev_minor, line_mjr, line_mnr, best_match_len = 0;
 
 	if (stat(path, &buf) < 0)
 		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
@@ -524,17 +532,20 @@ void tst_find_backing_dev(const char *path, char *dev)
 	*dev = '\0';
 
 	while (fgets(line, sizeof(line), file)) {
-		if (sscanf(line, "%*d %*d %d:%d", &line_mjr, &line_mnr) != 2)
+		if (sscanf(line, "%*d %*d %d:%d %*s %s %*s %*s %*s %*s %s",
+			&line_mjr, &line_mnr, mnt_point, mnt_source) != 4)
 			continue;
 
 		if (line_mjr == dev_major && line_mnr == dev_minor) {
-			pre = strstr(line, " - ");
-			pre = strtok_r(pre, " ", &next);
-			pre = strtok_r(NULL, " ", &next);
-			pre = strtok_r(NULL, " ", &next);
-			strcpy(dev, pre);
+			strcpy(dev, mnt_source);
 			break;
 		}
+
+		unsigned int len = count_match_len(path, mnt_point);
+		if (len > best_match_len) {
+			strcpy(dev, mnt_source);
+			best_match_len = len;
+		}
 	}
 
 	SAFE_FCLOSE(NULL, file);
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH] tst_find_backing_dev: match mount point if major/minor can't be found

[Cyril Hrubis]

Hi!
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  lib/tst_device.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index d296f9118cde..e560ec97460b 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -506,14 +506,22 @@ unsigned long tst_dev_bytes_written(const char *dev)
>  	return dev_bytes_written;
>  }
>  
> +static int count_match_len(const char *first, const char *second)
> +{
> +	int len = 0;
> +
> +	while (*first && *first++ == *second++)
> +		len++;
> +
> +	return len;
> +}
> +
>  void tst_find_backing_dev(const char *path, char *dev)
>  {
>  	struct stat buf;
>  	FILE *file;
> -	char line[PATH_MAX];
> -	char *pre = NULL;
> -	char *next = NULL;
> -	unsigned int dev_major, dev_minor, line_mjr, line_mnr;
> +	char line[PATH_MAX], mnt_point[PATH_MAX], mnt_source[PATH_MAX];
> +	unsigned int dev_major, dev_minor, line_mjr, line_mnr, best_match_len = 0;
>  
>  	if (stat(path, &buf) < 0)
>  		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
> @@ -524,17 +532,20 @@ void tst_find_backing_dev(const char *path, char *dev)
>  	*dev = '\0';
>  
>  	while (fgets(line, sizeof(line), file)) {
> -		if (sscanf(line, "%*d %*d %d:%d", &line_mjr, &line_mnr) != 2)
> +		if (sscanf(line, "%*d %*d %d:%d %*s %s %*s %*s %*s %*s %s",
> +			&line_mjr, &line_mnr, mnt_point, mnt_source) != 4)
>  			continue;

We did this before and it didn't work see: 5dfd9c29f094e3024ceab760715456436188ecab

We really have to parse the string based on the dash (" - ") to make it
work reliably.

Other than that the rest of the patch looks reasonable.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] tst_find_backing_dev: match mount point if major/minor can't be found

[Jan Stancek]

ioctl_loop05 fails on btrfs, because tst_find_backing_dev doesn't find
major/minor returned by stat()

Per https://lwn.net/Articles/866582
  "btrfs allocates a separate device number (the usual major/minor pair)
  for each subvolume ... and call to on a file within a subvolume will
  return a device number that does not exist in files like mountinfo."

As fallback, if there's no major/minor match, use best match of mount path.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 lib/tst_device.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/lib/tst_device.c b/lib/tst_device.c
index d296f9118cde..b1af0f0146d4 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -506,6 +506,16 @@ unsigned long tst_dev_bytes_written(const char *dev)
 	return dev_bytes_written;
 }
 
+static int count_match_len(const char *first, const char *second)
+{
+	int len = 0;
+
+	while (*first && *first++ == *second++)
+		len++;
+
+	return len;
+}
+
 void tst_find_backing_dev(const char *path, char *dev)
 {
 	struct stat buf;
@@ -514,6 +524,8 @@ void tst_find_backing_dev(const char *path, char *dev)
 	char *pre = NULL;
 	char *next = NULL;
 	unsigned int dev_major, dev_minor, line_mjr, line_mnr;
+	unsigned int len, best_match_len = 0;
+	char mnt_point[PATH_MAX];
 
 	if (stat(path, &buf) < 0)
 		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
@@ -524,17 +536,25 @@ void tst_find_backing_dev(const char *path, char *dev)
 	*dev = '\0';
 
 	while (fgets(line, sizeof(line), file)) {
-		if (sscanf(line, "%*d %*d %d:%d", &line_mjr, &line_mnr) != 2)
+		if (sscanf(line, "%*d %*d %d:%d %*s %s",
+			&line_mjr, &line_mnr, mnt_point) != 3)
 			continue;
 
+		pre = strstr(line, " - ");
+		pre = strtok_r(pre, " ", &next);
+		pre = strtok_r(NULL, " ", &next);
+		pre = strtok_r(NULL, " ", &next);
+
 		if (line_mjr == dev_major && line_mnr == dev_minor) {
-			pre = strstr(line, " - ");
-			pre = strtok_r(pre, " ", &next);
-			pre = strtok_r(NULL, " ", &next);
-			pre = strtok_r(NULL, " ", &next);
 			strcpy(dev, pre);
 			break;
 		}
+
+		len = count_match_len(path, mnt_point);
+		if (len > best_match_len) {
+			strcpy(dev, pre);
+			best_match_len = len;
+		}
 	}
 
 	SAFE_FCLOSE(NULL, file);
-- 
2.27.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] tst_find_backing_dev: match mount point if major/minor can't be found

[Cyril Hrubis]

Hi!
> +		len = count_match_len(path, mnt_point);
> +		if (len > best_match_len) {

I wonder if we should also check for minimal len to avoid trival prefix
matches such as "/". Maybe we should set the best_match_len to 1 to
begin with.

> +			strcpy(dev, pre);
> +			best_match_len = len;
> +		}
>  	}
>  
>  	SAFE_FCLOSE(NULL, file);
> -- 
> 2.27.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] tst_find_backing_dev: match mount point if major/minor can't be found

[Jan Stancek]

On Tue, May 17, 2022 at 5:30 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > +             len = count_match_len(path, mnt_point);
> > +             if (len > best_match_len) {
>
> I wonder if we should also check for minimal len to avoid trival prefix
> matches such as "/". Maybe we should set the best_match_len to 1 to
> begin with.

Sure, we can do that. Would you ack this for release as well?
This has been failing for quite a while on fedora (which uses btrfs).

>
> > +                     strcpy(dev, pre);
> > +                     best_match_len = len;
> > +             }
> >       }
> >
> >       SAFE_FCLOSE(NULL, file);
> > --
> > 2.27.0
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] tst_find_backing_dev: match mount point if major/minor can't be found

[Cyril Hrubis]

Hi!
> Sure, we can do that. Would you ack this for release as well?
> This has been failing for quite a while on fedora (which uses btrfs).

Sure, either way this version looks good.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] tst_find_backing_dev: match mount point if major/minor can't be found

[Jan Stancek]

On Thu, May 19, 2022 at 11:59 AM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Sure, we can do that. Would you ack this for release as well?
> > This has been failing for quite a while on fedora (which uses btrfs).
>
> Sure, either way this version looks good.
>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

pushed


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp