| Message ID | 20230301011918.64629-1-xiubli@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] ceph: do not print the whole xattr value if it's too long | expand |
On Wed, Mar 1, 2023 at 2:19 AM <xiubli@redhat.com> wrote: > > From: Xiubo Li <xiubli@redhat.com> > > If the xattr's value size is long enough the kernel will warn and > then will fail the xfstests test case. > > Just print part of the value string if it's too long. > > Cc: stable@vger.kernel.org > URL: https://tracker.ceph.com/issues/58404 Hi Xiubo, Does this really need to go to stable kernels? None of the douts are printed by default. > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > > V2: > - switch to use min() from Jeff's comment > - s/XATTR_MAX_VAL/MAX_XATTR_VAL/g > > > fs/ceph/xattr.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c > index b10d459c2326..887a65279fcf 100644 > --- a/fs/ceph/xattr.c > +++ b/fs/ceph/xattr.c > @@ -561,6 +561,7 @@ static struct ceph_vxattr *ceph_match_vxattr(struct inode *inode, > return NULL; > } > > +#define MAX_XATTR_VAL 256 Perhaps MAX_XATTR_VAL_PRINT_LEN? Also, I'd add a blank like after the define -- it's used by more than one function. Thanks, Ilya
On 01/03/2023 19:54, Ilya Dryomov wrote: > On Wed, Mar 1, 2023 at 2:19 AM <xiubli@redhat.com> wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> If the xattr's value size is long enough the kernel will warn and >> then will fail the xfstests test case. >> >> Just print part of the value string if it's too long. >> >> Cc: stable@vger.kernel.org >> URL: https://tracker.ceph.com/issues/58404 > Hi Xiubo, > > Does this really need to go to stable kernels? None of the douts are > printed by default. Ilya, That's okay, not a must. > >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> >> V2: >> - switch to use min() from Jeff's comment >> - s/XATTR_MAX_VAL/MAX_XATTR_VAL/g >> >> >> fs/ceph/xattr.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c >> index b10d459c2326..887a65279fcf 100644 >> --- a/fs/ceph/xattr.c >> +++ b/fs/ceph/xattr.c >> @@ -561,6 +561,7 @@ static struct ceph_vxattr *ceph_match_vxattr(struct inode *inode, >> return NULL; >> } >> >> +#define MAX_XATTR_VAL 256 > Perhaps MAX_XATTR_VAL_PRINT_LEN? Also, I'd add a blank like after the > define -- it's used by more than one function. Looks better. I will revise this. Thanks - Xiubo > Thanks, > > Ilya >
diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c index b10d459c2326..887a65279fcf 100644 --- a/fs/ceph/xattr.c +++ b/fs/ceph/xattr.c @@ -561,6 +561,7 @@ static struct ceph_vxattr *ceph_match_vxattr(struct inode *inode, return NULL; } +#define MAX_XATTR_VAL 256 static int __set_xattr(struct ceph_inode_info *ci, const char *name, int name_len, const char *val, int val_len, @@ -654,8 +655,10 @@ static int __set_xattr(struct ceph_inode_info *ci, dout("__set_xattr_val p=%p\n", p); } - dout("__set_xattr_val added %llx.%llx xattr %p %.*s=%.*s\n", - ceph_vinop(&ci->netfs.inode), xattr, name_len, name, val_len, val); + dout("__set_xattr_val added %llx.%llx xattr %p %.*s=%.*s%s\n", + ceph_vinop(&ci->netfs.inode), xattr, name_len, name, + min(val_len, MAX_XATTR_VAL), val, + val_len > MAX_XATTR_VAL ? "..." : ""); return 0; } @@ -681,8 +684,9 @@ static struct ceph_inode_xattr *__get_xattr(struct ceph_inode_info *ci, else if (c > 0) p = &(*p)->rb_right; else { - dout("__get_xattr %s: found %.*s\n", name, - xattr->val_len, xattr->val); + dout("__get_xattr %s: found %.*s%s\n", name, + min(xattr->val_len, MAX_XATTR_VAL), xattr->val, + xattr->val_len > MAX_XATTR_VAL ? "..." : ""); return xattr; } }