| Message ID | 998e7209-d700-5006-a2a0-e4c37ef0c024@sandeen.net (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Looks ok to me. You can add my review. Reviewed by: Allison Henderson <allison.henderson@oracle.com> Thanks! On 05/22/2018 02:58 PM, Eric Sandeen wrote: > After we lost ustat(2) in commit 4e7a824, we ended up with a slightly > bonkers method to determine if our target block device was mounted: > it goes through every entry returned by getmntent and stats the dir > to see if its underlying device matches ours. > > Unfortunately that dir might be a hung nfs server and sadness ensues. > > So just do a really simple sanity check before we try to stat the > mountpoint: does its device start with a / ? If not, skip it. > > Fixes: 4e7a824 ("libxfs/linux.c: Replace use of ustat by stat") > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > V2: just look for fully-qualified device paths vs expecting an actual > block device. > > diff --git a/libxfs/linux.c b/libxfs/linux.c > index 0bace3e..5c8602e 100644 > --- a/libxfs/linux.c > +++ b/libxfs/linux.c > @@ -77,7 +77,17 @@ platform_check_mount(char *name, char *block, struct > stat *s, int flags) > progname, name); > return 1; > } > + /* > + * This whole business is to work out if our block device is mounted > + * after we lost ustat(2), see: > + * 4e7a824 libxfs/linux.c: Replace use of ustat by stat > + * We don't really want to stat every single mounted directory, > + * as that may include tmpfs, cgroups, procfs or - worst - hung nfs > + * servers. So first, a simple check: does the "dev" start with "/" ? > + */ > while ((mnt = getmntent(f)) != NULL) { > + if (mnt->mnt_fsname[0] != '/') > + continue; > if (stat(mnt->mnt_dir, &mst) < 0) > continue; > if (mst.st_dev != s->st_rdev) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at > https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwICaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=LHZQ8fHvy6wDKXGTWcm97burZH5sQKHRDMaY1UthQxc&m=R6pqMHln20gGSsVL6glkacPVahf-7ntcy9uginrHVqU&s=svjEUGJ89ba2TeolXCPU9AjtORnBzV-56WGzef1Q6X0&e= > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/libxfs/linux.c b/libxfs/linux.c index 0bace3e..5c8602e 100644 --- a/libxfs/linux.c +++ b/libxfs/linux.c @@ -77,7 +77,17 @@ platform_check_mount(char *name, char *block, struct stat *s, int flags) progname, name); return 1; } + /* + * This whole business is to work out if our block device is mounted + * after we lost ustat(2), see: + * 4e7a824 libxfs/linux.c: Replace use of ustat by stat + * We don't really want to stat every single mounted directory, + * as that may include tmpfs, cgroups, procfs or - worst - hung nfs + * servers. So first, a simple check: does the "dev" start with "/" ? + */ while ((mnt = getmntent(f)) != NULL) { + if (mnt->mnt_fsname[0] != '/') + continue; if (stat(mnt->mnt_dir, &mst) < 0) continue; if (mst.st_dev != s->st_rdev)
After we lost ustat(2) in commit 4e7a824, we ended up with a slightly bonkers method to determine if our target block device was mounted: it goes through every entry returned by getmntent and stats the dir to see if its underlying device matches ours. Unfortunately that dir might be a hung nfs server and sadness ensues. So just do a really simple sanity check before we try to stat the mountpoint: does its device start with a / ? If not, skip it. Fixes: 4e7a824 ("libxfs/linux.c: Replace use of ustat by stat") Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- V2: just look for fully-qualified device paths vs expecting an actual block device. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html