| Message ID | 20210727113509.7714-1-jlayton@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | ceph: add a new vxattr to return auth mds for an inode | expand |
On Tue, Jul 27, 2021 at 07:35:09AM -0400, Jeff Layton wrote: > Add a new vxattr that shows what MDS is authoritative for an inode (if > we happen to have auth caps). If we don't have an auth cap for the inode > then just return -1. > > URL: https://tracker.ceph.com/issues/1276 > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/xattr.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > index 1242db8d3444..70664a19b8dc 100644 > --- a/fs/ceph/xattr.c > +++ b/fs/ceph/xattr.c > @@ -340,6 +340,15 @@ static ssize_t ceph_vxattrcb_caps(struct ceph_inode_info *ci, char *val, > ceph_cap_string(issued), issued); > } > > +static ssize_t ceph_vxattrcb_auth_mds(struct ceph_inode_info *ci, > + char *val, size_t size) > +{ > + /* return -1 if we don't have auth caps (and thus can't tell) */ > + if (!ci->i_auth_cap) > + return ceph_fmt_xattr(val, size, "-1"); I don't really have an opinion on this as I don't have a usecase for this xattr (other than debug, of course). But I just checked a similar function ceph_vxattrcb_layout_pool_namespace() and, if there's no value for ns for an inode, it just returns 0. Anyway, just my 5c, as I'm OK with returning a '-1' string too. Cheers, -- Luís > + return ceph_fmt_xattr(val, size, "%d", ci->i_auth_cap->session->s_mds); > +} > + > #define CEPH_XATTR_NAME(_type, _name) XATTR_CEPH_PREFIX #_type "." #_name > #define CEPH_XATTR_NAME2(_type, _name, _name2) \ > XATTR_CEPH_PREFIX #_type "." #_name "." #_name2 > @@ -473,6 +482,13 @@ static struct ceph_vxattr ceph_common_vxattrs[] = { > .exists_cb = NULL, > .flags = VXATTR_FLAG_READONLY, > }, > + { > + .name = "ceph.auth_mds", > + .name_size = sizeof("ceph.auth_mds"), > + .getxattr_cb = ceph_vxattrcb_auth_mds, > + .exists_cb = NULL, > + .flags = VXATTR_FLAG_READONLY, > + }, > { .name = NULL, 0 } /* Required table terminator */ > }; > > -- > 2.31.1 >
On Tue, Jul 27, 2021 at 11:17:39AM -0400, Jeff Layton wrote: > On Tue, 2021-07-27 at 15:46 +0100, Luis Henriques wrote: > > On Tue, Jul 27, 2021 at 07:35:09AM -0400, Jeff Layton wrote: > > > Add a new vxattr that shows what MDS is authoritative for an inode (if > > > we happen to have auth caps). If we don't have an auth cap for the inode > > > then just return -1. > > > > > > URL: https://tracker.ceph.com/issues/1276 > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > > --- > > > fs/ceph/xattr.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > > > index 1242db8d3444..70664a19b8dc 100644 > > > --- a/fs/ceph/xattr.c > > > +++ b/fs/ceph/xattr.c > > > @@ -340,6 +340,15 @@ static ssize_t ceph_vxattrcb_caps(struct ceph_inode_info *ci, char *val, > > > ceph_cap_string(issued), issued); > > > } > > > > > > +static ssize_t ceph_vxattrcb_auth_mds(struct ceph_inode_info *ci, > > > + char *val, size_t size) > > > +{ > > > + /* return -1 if we don't have auth caps (and thus can't tell) */ > > > + if (!ci->i_auth_cap) > > > + return ceph_fmt_xattr(val, size, "-1"); > > > > I don't really have an opinion on this as I don't have a usecase for this > > xattr (other than debug, of course). But I just checked a similar function > > ceph_vxattrcb_layout_pool_namespace() and, if there's no value for ns for an > > inode, it just returns 0. > > > > Anyway, just my 5c, as I'm OK with returning a '-1' string too. > > > > > TBH, I don't have much of a use-case for this either, but it was > requested by Sage long ago. That said, I figure it might be useful in > some cases, particularly when troubleshooting pinning issues. > > Pool numbering is a bit different, as I think pool 0 is not valid, > whereas we index mds's starting with 0. I'm fine with a different > convention here, but I considered it as a safe way to say "I don't know" > in this situation. Right, but what I meant by returning 0 was to really do: if (!ci->i_auth_cap) return 0; and not to return the string "0". Anyway, as I said, I don't have an opinion on this and returning the "-1" string for the MDS index sounds good too. Cheers, -- Luís > > > + return ceph_fmt_xattr(val, size, "%d", ci->i_auth_cap->session->s_mds); > > > +} > > > + > > > #define CEPH_XATTR_NAME(_type, _name) XATTR_CEPH_PREFIX #_type "." #_name > > > #define CEPH_XATTR_NAME2(_type, _name, _name2) \ > > > XATTR_CEPH_PREFIX #_type "." #_name "." #_name2 > > > @@ -473,6 +482,13 @@ static struct ceph_vxattr ceph_common_vxattrs[] = { > > > .exists_cb = NULL, > > > .flags = VXATTR_FLAG_READONLY, > > > }, > > > + { > > > + .name = "ceph.auth_mds", > > > + .name_size = sizeof("ceph.auth_mds"), > > > + .getxattr_cb = ceph_vxattrcb_auth_mds, > > > + .exists_cb = NULL, > > > + .flags = VXATTR_FLAG_READONLY, > > > + }, > > > { .name = NULL, 0 } /* Required table terminator */ > > > }; > > > > > > -- > > > 2.31.1 > > > > > -- > Jeff Layton <jlayton@kernel.org> >
On Tue, Jul 27, 2021 at 02:42:53PM -0400, Jeff Layton wrote: > Add a new vxattr that shows what MDS is authoritative for an inode (if > we happen to have auth caps). If we don't have an auth cap for the inode > then just return -1. > > URL: https://tracker.ceph.com/issues/1276 > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/xattr.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > v2: ensure we hold the i_ceph_lock when working with the i_auth_cap. Yeah, this lock is definitely needed here. LGTM. Cheers, -- Luís > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > index 1242db8d3444..159a1ffa4f4b 100644 > --- a/fs/ceph/xattr.c > +++ b/fs/ceph/xattr.c > @@ -340,6 +340,18 @@ static ssize_t ceph_vxattrcb_caps(struct ceph_inode_info *ci, char *val, > ceph_cap_string(issued), issued); > } > > +static ssize_t ceph_vxattrcb_auth_mds(struct ceph_inode_info *ci, > + char *val, size_t size) > +{ > + int ret; > + > + spin_lock(&ci->i_ceph_lock); > + ret = ceph_fmt_xattr(val, size, "%d", > + ci->i_auth_cap ? ci->i_auth_cap->session->s_mds : -1); > + spin_unlock(&ci->i_ceph_lock); > + return ret; > +} > + > #define CEPH_XATTR_NAME(_type, _name) XATTR_CEPH_PREFIX #_type "." #_name > #define CEPH_XATTR_NAME2(_type, _name, _name2) \ > XATTR_CEPH_PREFIX #_type "." #_name "." #_name2 > @@ -473,6 +485,13 @@ static struct ceph_vxattr ceph_common_vxattrs[] = { > .exists_cb = NULL, > .flags = VXATTR_FLAG_READONLY, > }, > + { > + .name = "ceph.auth_mds", > + .name_size = sizeof("ceph.auth_mds"), > + .getxattr_cb = ceph_vxattrcb_auth_mds, > + .exists_cb = NULL, > + .flags = VXATTR_FLAG_READONLY, > + }, > { .name = NULL, 0 } /* Required table terminator */ > }; > > -- > 2.31.1 >
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c index 1242db8d3444..70664a19b8dc 100644 --- a/fs/ceph/xattr.c +++ b/fs/ceph/xattr.c @@ -340,6 +340,15 @@ static ssize_t ceph_vxattrcb_caps(struct ceph_inode_info *ci, char *val, ceph_cap_string(issued), issued); } +static ssize_t ceph_vxattrcb_auth_mds(struct ceph_inode_info *ci, + char *val, size_t size) +{ + /* return -1 if we don't have auth caps (and thus can't tell) */ + if (!ci->i_auth_cap) + return ceph_fmt_xattr(val, size, "-1"); + return ceph_fmt_xattr(val, size, "%d", ci->i_auth_cap->session->s_mds); +} + #define CEPH_XATTR_NAME(_type, _name) XATTR_CEPH_PREFIX #_type "." #_name #define CEPH_XATTR_NAME2(_type, _name, _name2) \ XATTR_CEPH_PREFIX #_type "." #_name "." #_name2 @@ -473,6 +482,13 @@ static struct ceph_vxattr ceph_common_vxattrs[] = { .exists_cb = NULL, .flags = VXATTR_FLAG_READONLY, }, + { + .name = "ceph.auth_mds", + .name_size = sizeof("ceph.auth_mds"), + .getxattr_cb = ceph_vxattrcb_auth_mds, + .exists_cb = NULL, + .flags = VXATTR_FLAG_READONLY, + }, { .name = NULL, 0 } /* Required table terminator */ };
Add a new vxattr that shows what MDS is authoritative for an inode (if we happen to have auth caps). If we don't have an auth cap for the inode then just return -1. URL: https://tracker.ceph.com/issues/1276 Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/xattr.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)