On Tue, Jan 02, 2024 at 02:28:04PM +0000, Joey Gouly wrote: > Hi Kent, > > On Sat, Dec 30, 2023 at 02:58:02PM -0500, Kent Overstreet wrote: > > Add checks to all the VFS paths for "are we in a RO snapshot?". > > > > Note - we don't check this when setting inode options via our xattr > > interface, since those generally only affect data placement, not > > contents of data. > > > > Signed-off-by: Kent Overstreet <[email protected]> > > --- > > fs/bcachefs/acl.c | 3 ++- > > fs/bcachefs/fs-ioctl.c | 12 +++++------- > > fs/bcachefs/fs.c | 38 +++++++++++++++++++++++++++++++++----- > > fs/bcachefs/subvolume.c | 18 ++++++++++++++++++ > > fs/bcachefs/subvolume.h | 3 +++ > > fs/bcachefs/xattr.c | 3 ++- > > 6 files changed, 63 insertions(+), 14 deletions(-) > > > > diff --git a/fs/bcachefs/acl.c b/fs/bcachefs/acl.c > > index f3809897f00a..a9f3b0817b41 100644 > > --- a/fs/bcachefs/acl.c > > +++ b/fs/bcachefs/acl.c > > @@ -366,7 +366,8 @@ int bch2_set_acl(struct mnt_idmap *idmap, > > bch2_trans_begin(trans); > > acl = _acl; > > > > - ret = bch2_inode_peek(trans, &inode_iter, &inode_u, inode_inum(inode), > > + ret = bch2_subvol_is_ro_trans(trans, inode->ei_subvol); > > + bch2_inode_peek(trans, &inode_iter, &inode_u, inode_inum(inode), > > BTREE_ITER_INTENT); > > Looks like this is throwing away the return value of `bch2_inode_peek`, > because > the previous line ends in ';' but it should end in '?:' to match other changes > in this commit? > > > if (ret) > > goto btree_err; > > diff --git a/fs/bcachefs/fs-ioctl.c b/fs/bcachefs/fs-ioctl.c > > index 8098a3a299d1..e0a19a73c8e1 100644 > > --- a/fs/bcachefs/fs-ioctl.c > > +++ b/fs/bcachefs/fs-ioctl.c > > @@ -100,7 +100,8 @@ static int bch2_ioc_setflags(struct bch_fs *c, > > } > > > > mutex_lock(&inode->ei_update_lock); > > - ret = bch2_write_inode(c, inode, bch2_inode_flags_set, &s, > > + ret = bch2_subvol_is_ro(c, inode->ei_subvol) ?: > > + bch2_write_inode(c, inode, bch2_inode_flags_set, &s, > > ATTR_CTIME); > > mutex_unlock(&inode->ei_update_lock); > > > > @@ -183,13 +184,10 @@ static int bch2_ioc_fssetxattr(struct bch_fs *c, > > } > > > > mutex_lock(&inode->ei_update_lock); > > - ret = bch2_set_projid(c, inode, fa.fsx_projid); > > - if (ret) > > - goto err_unlock; > > - > > - ret = bch2_write_inode(c, inode, fssetxattr_inode_update_fn, &s, > > + ret = bch2_subvol_is_ro(c, inode->ei_subvol) ?: > > + bch2_set_projid(c, inode, fa.fsx_projid) ?: > > + bch2_write_inode(c, inode, fssetxattr_inode_update_fn, &s, > > ATTR_CTIME); > > -err_unlock: > > mutex_unlock(&inode->ei_update_lock); > > err: > > inode_unlock(&inode->v); > > diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c > > index 34cb22a9c05d..0e1d709100bb 100644 > > --- a/fs/bcachefs/fs.c > > +++ b/fs/bcachefs/fs.c > > @@ -258,7 +258,8 @@ __bch2_create(struct mnt_idmap *idmap, > > retry: > > bch2_trans_begin(trans); > > > > - ret = bch2_create_trans(trans, > > + ret = bch2_subvol_is_ro_trans(trans, dir->ei_subvol); > > + bch2_create_trans(trans, > > inode_inum(dir), &dir_u, &inode_u, > > !(flags & BCH_CREATE_TMPFILE) > > ? &dentry->d_name : NULL, > > Same again.
yup, they were fortunately spotted before the pull request went out
