Re: [PATCH v3 1/1] ceph: add getvxattr op

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/17/22 3:36 PM, Milind Changire wrote:
On Mon, Jan 17, 2022 at 12:49 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote:

On 1/17/22 11:59 AM, Milind Changire wrote:
Problem:
Directory vxattrs like ceph.dir.pin* and ceph.dir.layout* may not be
propagated to the client as frequently to keep them updated. This
creates vxattr availability problems.

Solution:
Adds new getvxattr op to fetch ceph.dir.pin*, ceph.dir.layout* and
ceph.file.layout* vxattrs.
If the entire layout for a dir or a file is being set, then it is
expected that the layout be set in standard JSON format. Individual
field value retrieval is not wrapped in JSON. The JSON format also
applies while setting the vxattr if the entire layout is being set in
one go.
As a temporary measure, setting a vxattr can also be done in the old
format. The old format will be deprecated in the future.

URL: https://tracker.ceph.com/issues/51062
Signed-off-by: Milind Changire <mchangir@xxxxxxxxxx>
---
   fs/ceph/inode.c              | 51 ++++++++++++++++++++++++++++++++++++
   fs/ceph/mds_client.c         | 27 ++++++++++++++++++-
   fs/ceph/mds_client.h         | 12 ++++++++-
   fs/ceph/strings.c            |  1 +
   fs/ceph/super.h              |  1 +
   fs/ceph/xattr.c              | 34 ++++++++++++++++++++++++
   include/linux/ceph/ceph_fs.h |  1 +
   7 files changed, 125 insertions(+), 2 deletions(-)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index e3322fcb2e8d..b63746a7a0e0 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2291,6 +2291,57 @@ int __ceph_do_getattr(struct inode *inode, struct page *locked_page,
       return err;
   }

+int ceph_do_getvxattr(struct inode *inode, const char *name, void *value,
+                   size_t size)
+{
+     struct ceph_fs_client *fsc = ceph_sb_to_client(inode->i_sb);
+     struct ceph_mds_client *mdsc = fsc->mdsc;
+     struct ceph_mds_request *req;
+     int mode = USE_AUTH_MDS;
+     int err;
+     char *xattr_value;
+     size_t xattr_value_len;
+
+     req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_GETVXATTR, mode);
+     if (IS_ERR(req)) {
+             err = -ENOMEM;
+             goto out;
+     }
+
+     req->r_path2 = kstrdup(name, GFP_NOFS);
+     if (!req->r_path2) {
+             err = -ENOMEM;
+             goto put;
+     }
+
+     ihold(inode);
+     req->r_inode = inode;
+     err = ceph_mdsc_do_request(mdsc, NULL, req);
+     if (err < 0)
+             goto put;
+
+     xattr_value = req->r_reply_info.xattr_info.xattr_value;
+     xattr_value_len = req->r_reply_info.xattr_info.xattr_value_len;
+
+     dout("do_getvxattr xattr_value_len:%u, size:%u\n", xattr_value_len, size);
Need some help here.
The kernel CI reported the following warnings:
1. for i386 build that size_t is unsigned int
2. for riscv build that size_t is unsigned long int

So the above (dout) statement gets a warning either way if I change the expected
arguments to be either %u or %lu.

You can use the %zu, more detail please see https://www.kernel.org/doc/Documentation/printk-formats.txt.



+
+     err = xattr_value_len;
+     if (size == 0)
+             goto put;
+
+     if (xattr_value_len > size) {
+             err = -ERANGE;
+             goto put;
+     }
+
[...]
+                     return false;
+     }
+     return true;
+}
+
   ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
                     size_t size)
   {
@@ -927,6 +951,16 @@ ssize_t __ceph_getxattr(struct inode *inode, const char *name, void *value,
       int req_mask;
       ssize_t err;

+     if (!strncmp(name, XATTR_CEPH_PREFIX, XATTR_CEPH_PREFIX_LEN) &&
+         ceph_cluster_has_feature(inode, CEPHFS_FEATURE_GETVXATTR)) {
+             err = ceph_do_getvxattr(inode, name, value, size);
+             /* if cluster doesn't support xattr, we try to service it
+              * locally
+              */
+             if (err >= 0)
+                     return err;
+     }
+
If the 'Fa' or 'Fr' caps is issued to kclient, could we get this vxattr
locally instead of getting it from MDS every time ?
The new mechanism is meant to supersede the old one.
1. The new layout output format is JSON
2. The new mechanism also recursively resolves the layout to the
closest ancestor
3. The new mechanism deals with ceph vxattrs exclusively at the server end
    (currently getvxattr handles only a limited subset of all the vxattrs)
4. There's no way to fetch ceph.dir.pin* vxattrs locally
(see https://github.com/ceph/ceph/pull/42001 for userspace work)

Okay.




       /* let's see if a virtual xattr was requested */
       vxattr = ceph_match_vxattr(inode, name);
       if (vxattr) {
diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index 7ad6c3d0db7d..66db21ac5f0c 100644
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h
@@ -328,6 +328,7 @@ enum {
       CEPH_MDS_OP_LOOKUPPARENT = 0x00103,
       CEPH_MDS_OP_LOOKUPINO  = 0x00104,
       CEPH_MDS_OP_LOOKUPNAME = 0x00105,
+     CEPH_MDS_OP_GETVXATTR  = 0x00106,

       CEPH_MDS_OP_SETXATTR   = 0x01105,
       CEPH_MDS_OP_RMXATTR    = 0x01106,




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux