* [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only @ 2025-04-25 8:52 ` Hans Holmberg 2025-04-25 13:06 ` hch ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Hans Holmberg @ 2025-04-25 8:52 UTC (permalink / raw) To: Carlos Maiolino Cc: Dave Chinner, Darrick J . Wong, hch, linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org, Hans Holmberg Allow read-only mounts on rtdevs and logdevs that are marked as read-only and make sure those mounts can't be remounted read-write. Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com> --- I will post a couple of xfstests to add coverage for these cases. fs/xfs/xfs_super.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index b2dd0c0bf509..d7ac1654bc80 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -380,10 +380,14 @@ xfs_blkdev_get( struct file **bdev_filep) { int error = 0; + blk_mode_t mode; - *bdev_filep = bdev_file_open_by_path(name, - BLK_OPEN_READ | BLK_OPEN_WRITE | BLK_OPEN_RESTRICT_WRITES, - mp->m_super, &fs_holder_ops); + mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES; + if (!xfs_is_readonly(mp)) + mode |= BLK_OPEN_WRITE; + + *bdev_filep = bdev_file_open_by_path(name, mode, + mp->m_super, &fs_holder_ops); if (IS_ERR(*bdev_filep)) { error = PTR_ERR(*bdev_filep); *bdev_filep = NULL; @@ -1969,6 +1973,20 @@ xfs_remount_rw( struct xfs_sb *sbp = &mp->m_sb; int error; + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp && + bdev_read_only(mp->m_logdev_targp->bt_bdev)) { + xfs_warn(mp, + "ro->rw transition prohibited by read-only logdev"); + return -EACCES; + } + + if (mp->m_rtdev_targp && + bdev_read_only(mp->m_rtdev_targp->bt_bdev)) { + xfs_warn(mp, + "ro->rw transition prohibited by read-only rtdev"); + return -EACCES; + } + if (xfs_has_norecovery(mp)) { xfs_warn(mp, "ro->rw transition prohibited on norecovery mount"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-25 8:52 ` [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only Hans Holmberg @ 2025-04-25 13:06 ` hch 2025-04-25 14:54 ` Darrick J. Wong 2025-04-28 9:52 ` Carlos Maiolino 2 siblings, 0 replies; 9+ messages in thread From: hch @ 2025-04-25 13:06 UTC (permalink / raw) To: Hans Holmberg Cc: Carlos Maiolino, Dave Chinner, Darrick J . Wong, hch, linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org On Fri, Apr 25, 2025 at 08:52:53AM +0000, Hans Holmberg wrote: > Allow read-only mounts on rtdevs and logdevs that are marked as > read-only and make sure those mounts can't be remounted read-write. > > Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com> Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-25 8:52 ` [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only Hans Holmberg 2025-04-25 13:06 ` hch @ 2025-04-25 14:54 ` Darrick J. Wong 2025-04-28 12:27 ` Hans Holmberg 2025-04-28 9:52 ` Carlos Maiolino 2 siblings, 1 reply; 9+ messages in thread From: Darrick J. Wong @ 2025-04-25 14:54 UTC (permalink / raw) To: Hans Holmberg Cc: Carlos Maiolino, Dave Chinner, hch, linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org On Fri, Apr 25, 2025 at 08:52:53AM +0000, Hans Holmberg wrote: > Allow read-only mounts on rtdevs and logdevs that are marked as > read-only and make sure those mounts can't be remounted read-write. If the log device is readonly, does that mean the filesystem gets mounted norecovery too? Your test might want to check that a dirty log is not recovered even if the filesystem mounts. --D > Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com> > --- > > I will post a couple of xfstests to add coverage for these cases. > > fs/xfs/xfs_super.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index b2dd0c0bf509..d7ac1654bc80 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -380,10 +380,14 @@ xfs_blkdev_get( > struct file **bdev_filep) > { > int error = 0; > + blk_mode_t mode; > > - *bdev_filep = bdev_file_open_by_path(name, > - BLK_OPEN_READ | BLK_OPEN_WRITE | BLK_OPEN_RESTRICT_WRITES, > - mp->m_super, &fs_holder_ops); > + mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES; > + if (!xfs_is_readonly(mp)) > + mode |= BLK_OPEN_WRITE; > + > + *bdev_filep = bdev_file_open_by_path(name, mode, > + mp->m_super, &fs_holder_ops); > if (IS_ERR(*bdev_filep)) { > error = PTR_ERR(*bdev_filep); > *bdev_filep = NULL; > @@ -1969,6 +1973,20 @@ xfs_remount_rw( > struct xfs_sb *sbp = &mp->m_sb; > int error; > > + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp && > + bdev_read_only(mp->m_logdev_targp->bt_bdev)) { > + xfs_warn(mp, > + "ro->rw transition prohibited by read-only logdev"); > + return -EACCES; > + } > + > + if (mp->m_rtdev_targp && > + bdev_read_only(mp->m_rtdev_targp->bt_bdev)) { > + xfs_warn(mp, > + "ro->rw transition prohibited by read-only rtdev"); > + return -EACCES; > + } > + > if (xfs_has_norecovery(mp)) { > xfs_warn(mp, > "ro->rw transition prohibited on norecovery mount"); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-25 14:54 ` Darrick J. Wong @ 2025-04-28 12:27 ` Hans Holmberg 0 siblings, 0 replies; 9+ messages in thread From: Hans Holmberg @ 2025-04-28 12:27 UTC (permalink / raw) To: Darrick J. Wong Cc: Carlos Maiolino, Dave Chinner, hch, linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org On 25/04/2025 16:54, Darrick J. Wong wrote: > On Fri, Apr 25, 2025 at 08:52:53AM +0000, Hans Holmberg wrote: >> Allow read-only mounts on rtdevs and logdevs that are marked as >> read-only and make sure those mounts can't be remounted read-write. > > If the log device is readonly, does that mean the filesystem gets > mounted norecovery too? Your test might want to check that a dirty log > is not recovered even if the filesystem mounts. > Read only-mounts do not mean that norecovery is set, but xfs will fail to mount if recovery is needed and the logdev/rtdev/.. is read-only. To mount in this state, the user needs to pass in norecovery. > --D > >> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com> >> --- >> >> I will post a couple of xfstests to add coverage for these cases. >> >> fs/xfs/xfs_super.c | 24 +++++++++++++++++++++--- >> 1 file changed, 21 insertions(+), 3 deletions(-) >> >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c >> index b2dd0c0bf509..d7ac1654bc80 100644 >> --- a/fs/xfs/xfs_super.c >> +++ b/fs/xfs/xfs_super.c >> @@ -380,10 +380,14 @@ xfs_blkdev_get( >> struct file **bdev_filep) >> { >> int error = 0; >> + blk_mode_t mode; >> >> - *bdev_filep = bdev_file_open_by_path(name, >> - BLK_OPEN_READ | BLK_OPEN_WRITE | BLK_OPEN_RESTRICT_WRITES, >> - mp->m_super, &fs_holder_ops); >> + mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES; >> + if (!xfs_is_readonly(mp)) >> + mode |= BLK_OPEN_WRITE; >> + >> + *bdev_filep = bdev_file_open_by_path(name, mode, >> + mp->m_super, &fs_holder_ops); >> if (IS_ERR(*bdev_filep)) { >> error = PTR_ERR(*bdev_filep); >> *bdev_filep = NULL; >> @@ -1969,6 +1973,20 @@ xfs_remount_rw( >> struct xfs_sb *sbp = &mp->m_sb; >> int error; >> >> + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp && >> + bdev_read_only(mp->m_logdev_targp->bt_bdev)) { >> + xfs_warn(mp, >> + "ro->rw transition prohibited by read-only logdev"); >> + return -EACCES; >> + } >> + >> + if (mp->m_rtdev_targp && >> + bdev_read_only(mp->m_rtdev_targp->bt_bdev)) { >> + xfs_warn(mp, >> + "ro->rw transition prohibited by read-only rtdev"); >> + return -EACCES; >> + } >> + >> if (xfs_has_norecovery(mp)) { >> xfs_warn(mp, >> "ro->rw transition prohibited on norecovery mount"); >> -- >> 2.34.1 >> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-25 8:52 ` [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only Hans Holmberg 2025-04-25 13:06 ` hch 2025-04-25 14:54 ` Darrick J. Wong @ 2025-04-28 9:52 ` Carlos Maiolino 2025-04-28 11:51 ` Hans Holmberg 2 siblings, 1 reply; 9+ messages in thread From: Carlos Maiolino @ 2025-04-28 9:52 UTC (permalink / raw) To: Hans Holmberg Cc: Dave Chinner, Darrick J . Wong, hch, linux-xfs@vger.kernel.org On Fri, Apr 25, 2025 at 08:52:53AM +0000, Hans Holmberg wrote: > Allow read-only mounts on rtdevs and logdevs that are marked as > read-only and make sure those mounts can't be remounted read-write. > > Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com> > --- > > I will post a couple of xfstests to add coverage for these cases. > > fs/xfs/xfs_super.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index b2dd0c0bf509..d7ac1654bc80 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -380,10 +380,14 @@ xfs_blkdev_get( > struct file **bdev_filep) > { > int error = 0; > + blk_mode_t mode; > > - *bdev_filep = bdev_file_open_by_path(name, > - BLK_OPEN_READ | BLK_OPEN_WRITE | BLK_OPEN_RESTRICT_WRITES, > - mp->m_super, &fs_holder_ops); > + mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES; Does BLK_OPEN_RESTRICT_WRITES makes sense without BLK_OPEN_WRITE? Perhaps this should be OR'ed together with OPEN_WRITE below? -Carlos > + if (!xfs_is_readonly(mp)) > + mode |= BLK_OPEN_WRITE; > + > + *bdev_filep = bdev_file_open_by_path(name, mode, > + mp->m_super, &fs_holder_ops); > if (IS_ERR(*bdev_filep)) { > error = PTR_ERR(*bdev_filep); > *bdev_filep = NULL; > @@ -1969,6 +1973,20 @@ xfs_remount_rw( > struct xfs_sb *sbp = &mp->m_sb; > int error; > > + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp && > + bdev_read_only(mp->m_logdev_targp->bt_bdev)) { > + xfs_warn(mp, > + "ro->rw transition prohibited by read-only logdev"); > + return -EACCES; > + } > + > + if (mp->m_rtdev_targp && > + bdev_read_only(mp->m_rtdev_targp->bt_bdev)) { > + xfs_warn(mp, > + "ro->rw transition prohibited by read-only rtdev"); > + return -EACCES; > + } > + > if (xfs_has_norecovery(mp)) { > xfs_warn(mp, > "ro->rw transition prohibited on norecovery mount"); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-28 9:52 ` Carlos Maiolino @ 2025-04-28 11:51 ` Hans Holmberg 2025-04-28 12:54 ` Carlos Maiolino 0 siblings, 1 reply; 9+ messages in thread From: Hans Holmberg @ 2025-04-28 11:51 UTC (permalink / raw) To: Carlos Maiolino Cc: Dave Chinner, Darrick J . Wong, hch, linux-xfs@vger.kernel.org On 28/04/2025 11:52, Carlos Maiolino wrote: > On Fri, Apr 25, 2025 at 08:52:53AM +0000, Hans Holmberg wrote: >> Allow read-only mounts on rtdevs and logdevs that are marked as >> read-only and make sure those mounts can't be remounted read-write. >> >> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com> >> --- >> >> I will post a couple of xfstests to add coverage for these cases. >> >> fs/xfs/xfs_super.c | 24 +++++++++++++++++++++--- >> 1 file changed, 21 insertions(+), 3 deletions(-) >> >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c >> index b2dd0c0bf509..d7ac1654bc80 100644 >> --- a/fs/xfs/xfs_super.c >> +++ b/fs/xfs/xfs_super.c >> @@ -380,10 +380,14 @@ xfs_blkdev_get( >> struct file **bdev_filep) >> { >> int error = 0; >> + blk_mode_t mode; >> >> - *bdev_filep = bdev_file_open_by_path(name, >> - BLK_OPEN_READ | BLK_OPEN_WRITE | BLK_OPEN_RESTRICT_WRITES, >> - mp->m_super, &fs_holder_ops); >> + mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES; > > Does BLK_OPEN_RESTRICT_WRITES makes sense without BLK_OPEN_WRITE? > Perhaps this should be OR'ed together with OPEN_WRITE below? BLK_OPEN_RESTRICT_WRITES disallows other writers to mounted devs, and I presume we want this for read-only mounts as well? > > > -Carlos > >> + if (!xfs_is_readonly(mp)) >> + mode |= BLK_OPEN_WRITE; > >> + >> + *bdev_filep = bdev_file_open_by_path(name, mode, >> + mp->m_super, &fs_holder_ops); >> if (IS_ERR(*bdev_filep)) { >> error = PTR_ERR(*bdev_filep); >> *bdev_filep = NULL; >> @@ -1969,6 +1973,20 @@ xfs_remount_rw( >> struct xfs_sb *sbp = &mp->m_sb; >> int error; >> >> + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp && >> + bdev_read_only(mp->m_logdev_targp->bt_bdev)) { >> + xfs_warn(mp, >> + "ro->rw transition prohibited by read-only logdev"); >> + return -EACCES; >> + } >> + >> + if (mp->m_rtdev_targp && >> + bdev_read_only(mp->m_rtdev_targp->bt_bdev)) { >> + xfs_warn(mp, >> + "ro->rw transition prohibited by read-only rtdev"); >> + return -EACCES; >> + } >> + >> if (xfs_has_norecovery(mp)) { >> xfs_warn(mp, >> "ro->rw transition prohibited on norecovery mount"); >> -- >> 2.34.1 >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-28 11:51 ` Hans Holmberg @ 2025-04-28 12:54 ` Carlos Maiolino 2025-04-28 13:04 ` hch 0 siblings, 1 reply; 9+ messages in thread From: Carlos Maiolino @ 2025-04-28 12:54 UTC (permalink / raw) To: Hans Holmberg Cc: Dave Chinner, Darrick J . Wong, hch, linux-xfs@vger.kernel.org On Mon, Apr 28, 2025 at 11:51:24AM +0000, Hans Holmberg wrote: > On 28/04/2025 11:52, Carlos Maiolino wrote: > > On Fri, Apr 25, 2025 at 08:52:53AM +0000, Hans Holmberg wrote: > >> Allow read-only mounts on rtdevs and logdevs that are marked as > >> read-only and make sure those mounts can't be remounted read-write. > >> > >> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com> > >> --- > >> > >> I will post a couple of xfstests to add coverage for these cases. > >> > >> fs/xfs/xfs_super.c | 24 +++++++++++++++++++++--- > >> 1 file changed, 21 insertions(+), 3 deletions(-) > >> > >> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > >> index b2dd0c0bf509..d7ac1654bc80 100644 > >> --- a/fs/xfs/xfs_super.c > >> +++ b/fs/xfs/xfs_super.c > >> @@ -380,10 +380,14 @@ xfs_blkdev_get( > >> struct file **bdev_filep) > >> { > >> int error = 0; > >> + blk_mode_t mode; > >> > >> - *bdev_filep = bdev_file_open_by_path(name, > >> - BLK_OPEN_READ | BLK_OPEN_WRITE | BLK_OPEN_RESTRICT_WRITES, > >> - mp->m_super, &fs_holder_ops); > >> + mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES; > > > > Does BLK_OPEN_RESTRICT_WRITES makes sense without BLK_OPEN_WRITE? > > Perhaps this should be OR'ed together with OPEN_WRITE below? > > BLK_OPEN_RESTRICT_WRITES disallows other writers to mounted devs, and I > presume we want this for read-only mounts as well? Thanks, it wasn't really clear to me what the purpose of RESTRICT_WRITES was, thanks for the clarification, this looks good to me: Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> > > > > > > > -Carlos > > > >> + if (!xfs_is_readonly(mp)) > >> + mode |= BLK_OPEN_WRITE; > > > >> + > >> + *bdev_filep = bdev_file_open_by_path(name, mode, > >> + mp->m_super, &fs_holder_ops); > >> if (IS_ERR(*bdev_filep)) { > >> error = PTR_ERR(*bdev_filep); > >> *bdev_filep = NULL; > >> @@ -1969,6 +1973,20 @@ xfs_remount_rw( > >> struct xfs_sb *sbp = &mp->m_sb; > >> int error; > >> > >> + if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp && > >> + bdev_read_only(mp->m_logdev_targp->bt_bdev)) { > >> + xfs_warn(mp, > >> + "ro->rw transition prohibited by read-only logdev"); > >> + return -EACCES; > >> + } > >> + > >> + if (mp->m_rtdev_targp && > >> + bdev_read_only(mp->m_rtdev_targp->bt_bdev)) { > >> + xfs_warn(mp, > >> + "ro->rw transition prohibited by read-only rtdev"); > >> + return -EACCES; > >> + } > >> + > >> if (xfs_has_norecovery(mp)) { > >> xfs_warn(mp, > >> "ro->rw transition prohibited on norecovery mount"); > >> -- > >> 2.34.1 > >> > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-28 12:54 ` Carlos Maiolino @ 2025-04-28 13:04 ` hch 2025-04-29 11:58 ` Hans Holmberg 0 siblings, 1 reply; 9+ messages in thread From: hch @ 2025-04-28 13:04 UTC (permalink / raw) To: Carlos Maiolino Cc: Hans Holmberg, Dave Chinner, Darrick J . Wong, hch, linux-xfs@vger.kernel.org On Mon, Apr 28, 2025 at 02:54:59PM +0200, Carlos Maiolino wrote: > > BLK_OPEN_RESTRICT_WRITES disallows other writers to mounted devs, and I > > presume we want this for read-only mounts as well? > > Thanks, it wasn't really clear to me what the purpose of RESTRICT_WRITES was, > thanks for the clarification, this looks good to me: It also mastches what most other file systems are doing by using the sb_open_mode helper. Thinking about tht we should probably use it as well as the superblock is available in mp->m_super here. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only 2025-04-28 13:04 ` hch @ 2025-04-29 11:58 ` Hans Holmberg 0 siblings, 0 replies; 9+ messages in thread From: Hans Holmberg @ 2025-04-29 11:58 UTC (permalink / raw) To: hch, Carlos Maiolino Cc: Dave Chinner, Darrick J . Wong, linux-xfs@vger.kernel.org On 28/04/2025 15:05, hch wrote: > On Mon, Apr 28, 2025 at 02:54:59PM +0200, Carlos Maiolino wrote: >>> BLK_OPEN_RESTRICT_WRITES disallows other writers to mounted devs, and I >>> presume we want this for read-only mounts as well? >> >> Thanks, it wasn't really clear to me what the purpose of RESTRICT_WRITES was, >> thanks for the clarification, this looks good to me: > > It also mastches what most other file systems are doing by using the > sb_open_mode helper. Thinking about tht we should probably use it as > well as the superblock is available in mp->m_super here. > > Yeah, sb_open_mode does what we want so i'll use that in a V2, ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-04-29 11:58 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <M6FcYEJbADh29bAOdxfu6Qm-ktiyMPYZw39bsvsx-RJNJsTgTMpoahi2HA9UAqfEH9ueyBk3Kry5vydrxmxWrA==@protonmail.internalid>
2025-04-25 8:52 ` [PATCH] xfs: allow ro mounts if rtdev or logdev are read-only Hans Holmberg
2025-04-25 13:06 ` hch
2025-04-25 14:54 ` Darrick J. Wong
2025-04-28 12:27 ` Hans Holmberg
2025-04-28 9:52 ` Carlos Maiolino
2025-04-28 11:51 ` Hans Holmberg
2025-04-28 12:54 ` Carlos Maiolino
2025-04-28 13:04 ` hch
2025-04-29 11:58 ` Hans Holmberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox