* [PATCH] ceph: add a new vxattr to return auth mds for an inode @ 2021-07-27 11:35 Jeff Layton 2021-07-27 14:46 ` Luis Henriques 2021-07-27 18:42 ` [PATCH v2] " Jeff Layton 0 siblings, 2 replies; 7+ messages in thread From: Jeff Layton @ 2021-07-27 11:35 UTC (permalink / raw) To: ceph-devel; +Cc: idryomov 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"); + 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] ceph: add a new vxattr to return auth mds for an inode 2021-07-27 11:35 [PATCH] ceph: add a new vxattr to return auth mds for an inode Jeff Layton @ 2021-07-27 14:46 ` Luis Henriques 2021-07-27 15:17 ` Jeff Layton 2021-07-27 18:42 ` [PATCH v2] " Jeff Layton 1 sibling, 1 reply; 7+ messages in thread From: Luis Henriques @ 2021-07-27 14:46 UTC (permalink / raw) To: Jeff Layton; +Cc: ceph-devel, idryomov 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ceph: add a new vxattr to return auth mds for an inode 2021-07-27 14:46 ` Luis Henriques @ 2021-07-27 15:17 ` Jeff Layton 2021-07-27 15:45 ` Luis Henriques 0 siblings, 1 reply; 7+ messages in thread From: Jeff Layton @ 2021-07-27 15:17 UTC (permalink / raw) To: Luis Henriques; +Cc: ceph-devel, idryomov, Sage Weil 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. > > + 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ceph: add a new vxattr to return auth mds for an inode 2021-07-27 15:17 ` Jeff Layton @ 2021-07-27 15:45 ` Luis Henriques 2021-07-27 15:53 ` Jeff Layton 0 siblings, 1 reply; 7+ messages in thread From: Luis Henriques @ 2021-07-27 15:45 UTC (permalink / raw) To: Jeff Layton; +Cc: ceph-devel, idryomov, Sage Weil 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> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ceph: add a new vxattr to return auth mds for an inode 2021-07-27 15:45 ` Luis Henriques @ 2021-07-27 15:53 ` Jeff Layton 0 siblings, 0 replies; 7+ messages in thread From: Jeff Layton @ 2021-07-27 15:53 UTC (permalink / raw) To: Luis Henriques; +Cc: ceph-devel, idryomov, Sage Weil On Tue, 2021-07-27 at 16:45 +0100, Luis Henriques wrote: > 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 > Oh yeah, ok. We could do that I guess. I don't care much either way, but that might be cleaner. Let me think on it a bit. I also think we'll need to hold the ci->i_ceph_lock around the i_auth_cap access. I'll need to send a v2 patch for that anyway. > > > > + 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> > > -- Jeff Layton <jlayton@kernel.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] ceph: add a new vxattr to return auth mds for an inode 2021-07-27 11:35 [PATCH] ceph: add a new vxattr to return auth mds for an inode Jeff Layton 2021-07-27 14:46 ` Luis Henriques @ 2021-07-27 18:42 ` Jeff Layton 2021-07-28 10:28 ` Luis Henriques 1 sibling, 1 reply; 7+ messages in thread From: Jeff Layton @ 2021-07-27 18:42 UTC (permalink / raw) To: ceph-devel; +Cc: idryomov, pdonnell, lhenriques 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. 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ceph: add a new vxattr to return auth mds for an inode 2021-07-27 18:42 ` [PATCH v2] " Jeff Layton @ 2021-07-28 10:28 ` Luis Henriques 0 siblings, 0 replies; 7+ messages in thread From: Luis Henriques @ 2021-07-28 10:28 UTC (permalink / raw) To: Jeff Layton; +Cc: ceph-devel, idryomov, pdonnell 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-07-28 10:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-27 11:35 [PATCH] ceph: add a new vxattr to return auth mds for an inode Jeff Layton 2021-07-27 14:46 ` Luis Henriques 2021-07-27 15:17 ` Jeff Layton 2021-07-27 15:45 ` Luis Henriques 2021-07-27 15:53 ` Jeff Layton 2021-07-27 18:42 ` [PATCH v2] " Jeff Layton 2021-07-28 10:28 ` Luis Henriques
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).