* [PATCH] libext2fs/ismounted.c: check open(O_EXCL) before mntent file
@ 2020-02-25 14:34 Lukas Czerner
2020-03-02 14:30 ` Carlos Maiolino
2020-03-03 13:53 ` [PATCH v2] " Lukas Czerner
0 siblings, 2 replies; 4+ messages in thread
From: Lukas Czerner @ 2020-02-25 14:34 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, Zdenek Kabelac, Karel Zak
Currently the ext2fs_check_mount_point() will use the open(O_EXCL) check
on linux after all the other checks are done. However it is not
necessary to check mntent if open(O_EXCL) succeeds because it means that
the device is not mounted.
Moreover the commit ea4d53b7 introduced a regression where a following
set of commands fails:
vgcreate mygroup /dev/sda
lvcreate -L 1G -n lvol0 mygroup
mkfs.ext4 /dev/mygroup/lvol0
mount /dev/mygroup/lvol0 /mnt
lvrename /dev/mygroup/lvol0 /dev/mygroup/lvol1
lvcreate -L 1G -n lvol0 mygroup
mkfs.ext4 /dev/mygroup/lvol0 <<<--- This fails
It fails because it thinks that /dev/mygroup/lvol0 is mounted because
the device name in /proc/mounts is not updated following the lvrename.
Move the open(O_EXCL) check before the mntent check and return
immediatelly if the device is not busy.
Fixes: ea4d53b7 ("libext2fs/ismounted.c: check device id in advance to skip false device names")
Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
Reported-by: Karel Zak <kzak@redhat.com>
---
lib/ext2fs/ismounted.c | 41 ++++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c
index c0215692..f4ef012f 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -352,6 +352,7 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
char *mtpt, int mtlen)
{
errcode_t retval = 0;
+ int busy = 0;
if (getenv("EXT2FS_PRETEND_RO_MOUNT")) {
*mount_flags = EXT2_MF_MOUNTED | EXT2_MF_READONLY;
@@ -366,6 +367,29 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
return 0;
}
+#ifdef __linux__ /* This only works on Linux 2.6+ systems */
+ {
+ struct stat st_buf;
+
+ if (stat(device, &st_buf) == 0 &&
+ ext2fsP_is_disk_device(st_buf.st_mode)) {
+ int fd = open(device, O_RDONLY | O_EXCL);
+
+ if (fd >= 0) {
+ /*
+ * The device is not busy so it's
+ * definitelly not mounted. No need to
+ * to perform any more checks.
+ */
+ close(fd);
+ *mount_flags = 0;
+ return 0;
+ } else if (errno == EBUSY)
+ busy = 1;
+ }
+ }
+#endif
+
if (is_swap_device(device)) {
*mount_flags = EXT2_MF_MOUNTED | EXT2_MF_SWAP;
strncpy(mtpt, "<swap>", mtlen);
@@ -386,21 +410,8 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
if (retval)
return retval;
-#ifdef __linux__ /* This only works on Linux 2.6+ systems */
- {
- struct stat st_buf;
-
- if (stat(device, &st_buf) == 0 &&
- ext2fsP_is_disk_device(st_buf.st_mode)) {
- int fd = open(device, O_RDONLY | O_EXCL);
-
- if (fd >= 0)
- close(fd);
- else if (errno == EBUSY)
- *mount_flags |= EXT2_MF_BUSY;
- }
- }
-#endif
+ if (busy)
+ *mount_flags |= EXT2_MF_BUSY;
return 0;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] libext2fs/ismounted.c: check open(O_EXCL) before mntent file
2020-02-25 14:34 [PATCH] libext2fs/ismounted.c: check open(O_EXCL) before mntent file Lukas Czerner
@ 2020-03-02 14:30 ` Carlos Maiolino
2020-03-03 13:53 ` [PATCH v2] " Lukas Czerner
1 sibling, 0 replies; 4+ messages in thread
From: Carlos Maiolino @ 2020-03-02 14:30 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-ext4, tytso, Zdenek Kabelac, Karel Zak
On Tue, Feb 25, 2020 at 03:34:45PM +0100, Lukas Czerner wrote:
> Currently the ext2fs_check_mount_point() will use the open(O_EXCL) check
> on linux after all the other checks are done. However it is not
> necessary to check mntent if open(O_EXCL) succeeds because it means that
> the device is not mounted.
>
> Moreover the commit ea4d53b7 introduced a regression where a following
> set of commands fails:
>
> vgcreate mygroup /dev/sda
> lvcreate -L 1G -n lvol0 mygroup
> mkfs.ext4 /dev/mygroup/lvol0
> mount /dev/mygroup/lvol0 /mnt
> lvrename /dev/mygroup/lvol0 /dev/mygroup/lvol1
> lvcreate -L 1G -n lvol0 mygroup
> mkfs.ext4 /dev/mygroup/lvol0 <<<--- This fails
>
> It fails because it thinks that /dev/mygroup/lvol0 is mounted because
> the device name in /proc/mounts is not updated following the lvrename.
>
> Move the open(O_EXCL) check before the mntent check and return
> immediatelly if the device is not busy.
>
> Fixes: ea4d53b7 ("libext2fs/ismounted.c: check device id in advance to skip false device names")
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
> Reported-by: Karel Zak <kzak@redhat.com>
> ---
...
> + if (fd >= 0) {
> + /*
> + * The device is not busy so it's
> + * definitelly not mounted. No need to
> + * to perform any more checks.
> + */
> + close(fd);
> + *mount_flags = 0;
> + return 0;
> + } else if (errno == EBUSY)
> + busy = 1;
^ I think you are supposed to use {} for the 'else if'
branch too, once the first branch uses it
Other than the small coding style above, which depends on maintainer, the code
looks ok, and you can add:
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Cheers
> + }
> + }
> +#endif
> +
> if (is_swap_device(device)) {
> *mount_flags = EXT2_MF_MOUNTED | EXT2_MF_SWAP;
> strncpy(mtpt, "<swap>", mtlen);
> @@ -386,21 +410,8 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
> if (retval)
> return retval;
>
> -#ifdef __linux__ /* This only works on Linux 2.6+ systems */
> - {
> - struct stat st_buf;
> -
> - if (stat(device, &st_buf) == 0 &&
> - ext2fsP_is_disk_device(st_buf.st_mode)) {
> - int fd = open(device, O_RDONLY | O_EXCL);
> -
> - if (fd >= 0)
> - close(fd);
> - else if (errno == EBUSY)
> - *mount_flags |= EXT2_MF_BUSY;
> - }
> - }
> -#endif
> + if (busy)
> + *mount_flags |= EXT2_MF_BUSY;
>
> return 0;
> }
> --
> 2.21.1
>
--
Carlos
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] libext2fs/ismounted.c: check open(O_EXCL) before mntent file
2020-02-25 14:34 [PATCH] libext2fs/ismounted.c: check open(O_EXCL) before mntent file Lukas Czerner
2020-03-02 14:30 ` Carlos Maiolino
@ 2020-03-03 13:53 ` Lukas Czerner
2020-03-07 18:11 ` Theodore Y. Ts'o
1 sibling, 1 reply; 4+ messages in thread
From: Lukas Czerner @ 2020-03-03 13:53 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, Zdenek Kabelac, Karel Zak, Carlos Maiolino
Currently the ext2fs_check_mount_point() will use the open(O_EXCL) check
on linux after all the other checks are done. However it is not
necessary to check mntent if open(O_EXCL) succeeds because it means that
the device is not mounted.
Moreover the commit ea4d53b7 introduced a regression where a following
set of commands fails:
vgcreate mygroup /dev/sda
lvcreate -L 1G -n lvol0 mygroup
mkfs.ext4 /dev/mygroup/lvol0
mount /dev/mygroup/lvol0 /mnt
lvrename /dev/mygroup/lvol0 /dev/mygroup/lvol1
lvcreate -L 1G -n lvol0 mygroup
mkfs.ext4 /dev/mygroup/lvol0 <<<--- This fails
It fails because it thinks that /dev/mygroup/lvol0 is mounted because
the device name in /proc/mounts is not updated following the lvrename.
Move the open(O_EXCL) check before the mntent check and return
immediatelly if the device is not busy.
Fixes: ea4d53b7 ("libext2fs/ismounted.c: check device id in advance to skip false device names")
Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
Reported-by: Karel Zak <kzak@redhat.com>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
lib/ext2fs/ismounted.c | 42 +++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c
index c0215692..46d330d9 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -352,6 +352,7 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
char *mtpt, int mtlen)
{
errcode_t retval = 0;
+ int busy = 0;
if (getenv("EXT2FS_PRETEND_RO_MOUNT")) {
*mount_flags = EXT2_MF_MOUNTED | EXT2_MF_READONLY;
@@ -366,6 +367,30 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
return 0;
}
+#ifdef __linux__ /* This only works on Linux 2.6+ systems */
+ {
+ struct stat st_buf;
+
+ if (stat(device, &st_buf) == 0 &&
+ ext2fsP_is_disk_device(st_buf.st_mode)) {
+ int fd = open(device, O_RDONLY | O_EXCL);
+
+ if (fd >= 0) {
+ /*
+ * The device is not busy so it's
+ * definitelly not mounted. No need to
+ * to perform any more checks.
+ */
+ close(fd);
+ *mount_flags = 0;
+ return 0;
+ } else if (errno == EBUSY) {
+ busy = 1;
+ }
+ }
+ }
+#endif
+
if (is_swap_device(device)) {
*mount_flags = EXT2_MF_MOUNTED | EXT2_MF_SWAP;
strncpy(mtpt, "<swap>", mtlen);
@@ -386,21 +411,8 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
if (retval)
return retval;
-#ifdef __linux__ /* This only works on Linux 2.6+ systems */
- {
- struct stat st_buf;
-
- if (stat(device, &st_buf) == 0 &&
- ext2fsP_is_disk_device(st_buf.st_mode)) {
- int fd = open(device, O_RDONLY | O_EXCL);
-
- if (fd >= 0)
- close(fd);
- else if (errno == EBUSY)
- *mount_flags |= EXT2_MF_BUSY;
- }
- }
-#endif
+ if (busy)
+ *mount_flags |= EXT2_MF_BUSY;
return 0;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] libext2fs/ismounted.c: check open(O_EXCL) before mntent file
2020-03-03 13:53 ` [PATCH v2] " Lukas Czerner
@ 2020-03-07 18:11 ` Theodore Y. Ts'o
0 siblings, 0 replies; 4+ messages in thread
From: Theodore Y. Ts'o @ 2020-03-07 18:11 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-ext4, Zdenek Kabelac, Karel Zak, Carlos Maiolino
On Tue, Mar 03, 2020 at 02:53:48PM +0100, Lukas Czerner wrote:
> Currently the ext2fs_check_mount_point() will use the open(O_EXCL) check
> on linux after all the other checks are done. However it is not
> necessary to check mntent if open(O_EXCL) succeeds because it means that
> the device is not mounted.
>
> Moreover the commit ea4d53b7 introduced a regression where a following
> set of commands fails:
>
> vgcreate mygroup /dev/sda
> lvcreate -L 1G -n lvol0 mygroup
> mkfs.ext4 /dev/mygroup/lvol0
> mount /dev/mygroup/lvol0 /mnt
> lvrename /dev/mygroup/lvol0 /dev/mygroup/lvol1
> lvcreate -L 1G -n lvol0 mygroup
> mkfs.ext4 /dev/mygroup/lvol0 <<<--- This fails
>
> It fails because it thinks that /dev/mygroup/lvol0 is mounted because
> the device name in /proc/mounts is not updated following the lvrename.
>
> Move the open(O_EXCL) check before the mntent check and return
> immediatelly if the device is not busy.
>
> Fixes: ea4d53b7 ("libext2fs/ismounted.c: check device id in advance to skip false device names")
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> Reported-by: Zdenek Kabelac <zkabelac@redhat.com>
> Reported-by: Karel Zak <kzak@redhat.com>
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Applied, thanks.
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-07 18:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 14:34 [PATCH] libext2fs/ismounted.c: check open(O_EXCL) before mntent file Lukas Czerner
2020-03-02 14:30 ` Carlos Maiolino
2020-03-03 13:53 ` [PATCH v2] " Lukas Czerner
2020-03-07 18:11 ` Theodore Y. Ts'o
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.