| Message ID | 20180810095757.24316-1-cgxu519@gmx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: don't allow cross-quota link | expand |
This needs to be consistent across kernel- and user-space, and I believe we settled on continuing to allow cross-quota links (despite the accounting issues) because otherwise the introduction of new quotas makes it incredibly messy. Unless I missed a change, Zheng? -Greg On Fri, Aug 10, 2018 at 2:57 AM, Chengguang Xu <cgxu519@gmx.com> wrote: > Currently, we allow making hard link bewteen different > quota realms and it may cause inaccurate quota accouting. > This patch adds quota realm check when doing hard link. > > Signed-off-by: Chengguang Xu <cgxu519@gmx.com> > --- > fs/ceph/dir.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > index 82928cea0209..87fa996dbad4 100644 > --- a/fs/ceph/dir.c > +++ b/fs/ceph/dir.c > @@ -993,6 +993,10 @@ static int ceph_link(struct dentry *old_dentry, struct inode *dir, > if (ceph_snap(dir) != CEPH_NOSNAP) > return -EROFS; > > + /* don't allow cross-quota link */ > + if (!ceph_quota_is_same_realm(d_inode(old_dentry), dir)) > + return -EXDEV; > + > dout("link in dir %p old_dentry %p dentry %p\n", dir, > old_dentry, dentry); > req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_LINK, USE_AUTH_MDS); > -- > 2.17.1 >
On Sat, Aug 11, 2018 at 12:17 AM Gregory Farnum <gfarnum@redhat.com> wrote: > > This needs to be consistent across kernel- and user-space, and I > believe we settled on continuing to allow cross-quota links (despite > the accounting issues) because otherwise the introduction of new > quotas makes it incredibly messy. Unless I missed a change, Zheng? > -Greg > I agree with Greg Regards Yan, Zheng > On Fri, Aug 10, 2018 at 2:57 AM, Chengguang Xu <cgxu519@gmx.com> wrote: > > Currently, we allow making hard link bewteen different > > quota realms and it may cause inaccurate quota accouting. > > This patch adds quota realm check when doing hard link. > > > > Signed-off-by: Chengguang Xu <cgxu519@gmx.com> > > --- > > fs/ceph/dir.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > > index 82928cea0209..87fa996dbad4 100644 > > --- a/fs/ceph/dir.c > > +++ b/fs/ceph/dir.c > > @@ -993,6 +993,10 @@ static int ceph_link(struct dentry *old_dentry, struct inode *dir, > > if (ceph_snap(dir) != CEPH_NOSNAP) > > return -EROFS; > > > > + /* don't allow cross-quota link */ > > + if (!ceph_quota_is_same_realm(d_inode(old_dentry), dir)) > > + return -EXDEV; > > + > > dout("link in dir %p old_dentry %p dentry %p\n", dir, > > old_dentry, dentry); > > req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_LINK, USE_AUTH_MDS); > > -- > > 2.17.1 > >
On Fri, Aug 10, 2018 at 9:15 AM, Gregory Farnum <gfarnum@redhat.com> wrote: > This needs to be consistent across kernel- and user-space, and I > believe we settled on continuing to allow cross-quota links (despite > the accounting issues) The current behavior of the user-space client is to disallow links across quota boundaries (at least for rename). See also: http://tracker.ceph.com/issues/24305 >because otherwise the introduction of new > quotas makes it incredibly messy. Unless I missed a change, Zheng? How does it make new quotas messy?
On Tue, 14 Aug 2018, Yan, Zheng wrote: > On Sat, Aug 11, 2018 at 12:17 AM Gregory Farnum <gfarnum@redhat.com> wrote: > > > > This needs to be consistent across kernel- and user-space, and I > > believe we settled on continuing to allow cross-quota links (despite > > the accounting issues) because otherwise the introduction of new > > quotas makes it incredibly messy. Unless I missed a change, Zheng? > > -Greg > > > > I agree with Greg Yeah, agree on consistency. Should this be a hard-coded behavior, or should it be a configurable paramter of the file system whether cross-quota links are allowed? s > > Regards > Yan, Zheng > > > On Fri, Aug 10, 2018 at 2:57 AM, Chengguang Xu <cgxu519@gmx.com> wrote: > > > Currently, we allow making hard link bewteen different > > > quota realms and it may cause inaccurate quota accouting. > > > This patch adds quota realm check when doing hard link. > > > > > > Signed-off-by: Chengguang Xu <cgxu519@gmx.com> > > > --- > > > fs/ceph/dir.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > > > index 82928cea0209..87fa996dbad4 100644 > > > --- a/fs/ceph/dir.c > > > +++ b/fs/ceph/dir.c > > > @@ -993,6 +993,10 @@ static int ceph_link(struct dentry *old_dentry, struct inode *dir, > > > if (ceph_snap(dir) != CEPH_NOSNAP) > > > return -EROFS; > > > > > > + /* don't allow cross-quota link */ > > > + if (!ceph_quota_is_same_realm(d_inode(old_dentry), dir)) > > > + return -EXDEV; > > > + > > > dout("link in dir %p old_dentry %p dentry %p\n", dir, > > > old_dentry, dentry); > > > req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_LINK, USE_AUTH_MDS); > > > -- > > > 2.17.1 > > > > >
On Mon, Aug 13, 2018 at 6:27 PM, Patrick Donnelly <pdonnell@redhat.com> wrote: > On Fri, Aug 10, 2018 at 9:15 AM, Gregory Farnum <gfarnum@redhat.com> wrote: >> This needs to be consistent across kernel- and user-space, and I >> believe we settled on continuing to allow cross-quota links (despite >> the accounting issues) > > The current behavior of the user-space client is to disallow links > across quota boundaries (at least for rename). See also: > http://tracker.ceph.com/issues/24305 > >>because otherwise the introduction of new >> quotas makes it incredibly messy. Unless I missed a change, Zheng? > > How does it make new quotas messy? If we add a new "quota realm" that interrupts existing hard links, what would we do? Just grandfather them in, and deal with the mess, but treat them like a special case we mostly ignore? Fully examine the tree for hard links and reject the new quota realm if it splits one? Fully support hard links across boundaries, but still block new cross-quota-realm hard links so users can't get themselves *more* confused? I'd forgotten that CephFS disallows renames across quota boundaries now. That does seem like another thing we should be generally consistent about. Does anybody remember why we did that? Was it just in preparation for the "subvolume" behavior that we eventually decided against? The EXDEV was introduced in the initial quota work from the Kylin et al group (hash bbfeaaea53f1cee3d798debc790c37ff5a5276bc). I think my preference would be to just allow the hard links *and* rename if the user has permission over both trees. But if that's not acceptable, we should enforce that condition on the MDS, not (at least not only) in the clients. -Greg
Gregory Farnum <gfarnum@redhat.com> writes: > On Mon, Aug 13, 2018 at 6:27 PM, Patrick Donnelly <pdonnell@redhat.com> wrote: >> On Fri, Aug 10, 2018 at 9:15 AM, Gregory Farnum <gfarnum@redhat.com> wrote: >>> This needs to be consistent across kernel- and user-space, and I >>> believe we settled on continuing to allow cross-quota links (despite >>> the accounting issues) >> >> The current behavior of the user-space client is to disallow links >> across quota boundaries (at least for rename). See also: >> http://tracker.ceph.com/issues/24305 >> >>>because otherwise the introduction of new >>> quotas makes it incredibly messy. Unless I missed a change, Zheng? >> >> How does it make new quotas messy? > > If we add a new "quota realm" that interrupts existing hard links, > what would we do? Just grandfather them in, and deal with the mess, > but treat them like a special case we mostly ignore? Fully examine the > tree for hard links and reject the new quota realm if it splits one? > Fully support hard links across boundaries, but still block new > cross-quota-realm hard links so users can't get themselves *more* > confused? > > I'd forgotten that CephFS disallows renames across quota boundaries > now. That does seem like another thing we should be generally > consistent about. Does anybody remember why we did that? The kernel client has the same behavior, and my assumption at that time was that the fuse client was simply being consistent with what other filesystems do. For example, xfs and ext4 don't allow renames when using project quotas (if using project inheritance). Cheers,
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index 82928cea0209..87fa996dbad4 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -993,6 +993,10 @@ static int ceph_link(struct dentry *old_dentry, struct inode *dir, if (ceph_snap(dir) != CEPH_NOSNAP) return -EROFS; + /* don't allow cross-quota link */ + if (!ceph_quota_is_same_realm(d_inode(old_dentry), dir)) + return -EXDEV; + dout("link in dir %p old_dentry %p dentry %p\n", dir, old_dentry, dentry); req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_LINK, USE_AUTH_MDS);
Currently, we allow making hard link bewteen different quota realms and it may cause inaccurate quota accouting. This patch adds quota realm check when doing hard link. Signed-off-by: Chengguang Xu <cgxu519@gmx.com> --- fs/ceph/dir.c | 4 ++++ 1 file changed, 4 insertions(+)