Re: [LTP] [PATCH v3 3/3] tst_find_backing_dev(): add support for overlayfs

[Petr Vorel <pvorel@suse.cz>]

Hi Jeff,


> Add checks for overlayfs in tst_find_backing_dev.  As with btrfs, only
> a single device is checked (the upper one) and returned from
> tst_find_backing_dev().

> The implementation uses both /proc/self/mountinfo and /proc/self/mounts.
> The former is used to map a device to a mountpoint, and the latter is
> used to get the file system options for the mountpoint.  All of the
> information is present in mountinfo, but the file format is more complex,
> and there are no glibc helpers for parsing it.

> The '#define _GNU_SOURCE' was added for the use of the strchrnul(3)
> function.

> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

> ---
> v3: Address review comments from Petr:
>     - Don't split long strings across lines
>     - use TST_OVERLAYFS_MAGIC instead of the kernel's definition
>     Note that I did not use SAFE_SSCANF, as tst_device.c uses the old
>     style safe macros, and that function is not covered.
> v2: Don't use libmount.  Instead, map from device number to mount-point
>     using /proc/self/mountinfo, and then use the mntent.h helpers to get
>     the mount options for the mountpoint from /proc/self/mounts.

LGTM, thanks for a very nice work!

Reviewed-by: Petr Vorel <pvorel@suse.cz>

I'd prefer others have look into it before merging.

Again, I'll apply minor formatting changes before merge (using SAFE_STAT(),
moving else branch after break to it's own and checkpatch.pl fixes).

Kind regards,
Petr

+++ lib/tst_device.c
@@ -575,7 +573,7 @@ static void btrfs_get_uevent_path(char *tmp_path, char *uevent_path)
 
 static char *overlay_mount_from_dev(dev_t dev)
 {
-	unsigned dev_major, dev_minor, mnt_major, mnt_minor;
+	unsigned int dev_major, dev_minor, mnt_major, mnt_minor;
 	FILE *fp;
 	char line[PATH_MAX];
 	char *mountpoint;
@@ -627,6 +625,7 @@ static char *overlay_get_upperdir(char *mountpoint)
 				 mountpoint, mnt->mnt_type);
 
 		optstr = hasmntopt(mnt, "upperdir");
+
 		if (optstr) {
 			optstart = strchr(optstr, '=');
 			optstart++;
@@ -634,11 +633,11 @@ static char *overlay_get_upperdir(char *mountpoint)
 			upperdir = calloc(optend - optstart + 1, 1);
 			memcpy(upperdir, optstart, optend - optstart);
 			break;
-		} else {
-			tst_brkm(TBROK, NULL,
-				 "mount point %s does not contain an upperdir",
-				 mountpoint);
 		}
+
+		tst_brkm(TBROK, NULL,
+			 "mount point %s does not contain an upperdir",
+			 mountpoint);
 	}
 	endmntent(mntf);
 
@@ -679,26 +678,21 @@ static char *overlay_get_upperdir(char *mountpoint)
  */
 static void overlay_get_uevent_path(char *tmp_path, char *uevent_path)
 {
-	int ret;
 	struct stat st;
 	char *mountpoint, *upperdir;
 
 	tst_resm(TINFO, "Use OVERLAYFS specific strategy");
 
-	ret = stat(tmp_path, &st);
-	if (ret)
-		tst_brkm(TBROK | TERRNO, NULL, "stat failed");
+	SAFE_STAT(NULL, tmp_path, &st);
 
 	mountpoint = overlay_mount_from_dev(st.st_dev);
 	upperdir = overlay_get_upperdir(mountpoint);
 	free(mountpoint);
 
-	ret = stat(upperdir, &st);
+	SAFE_STAT(NULL, upperdir, &st);
 	free(upperdir);
-	if (ret)
-		tst_brkm(TBROK | TERRNO, NULL, "stat failed");
 
-	tst_resm(TINFO, "Warning: used first of multiple backing devices.");
+	tst_resm(TINFO, "WARNING: used first of multiple backing devices");
 	sprintf(uevent_path, "/sys/dev/block/%d:%d/uevent",
 		major(st.st_dev), minor(st.st_dev));
 }

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