Please don't reply to lustre-devel. Instead, comment in Bugzilla by using the
following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10930
What |Removed |Added
----------------------------------------------------------------------------
Attachment #9195|review?([EMAIL PROTECTED]|review+
Flag|m) |
(From update of attachment 9195)
>+int ll_dir_getstripe(struct inode *inode, struct lov_mds_md **lmmp,
>+ int *lmm_size, struct ptlrpc_request **request)
>+{
>+ rc = mdc_getattr(sbi->ll_mdc_exp, &fid,
>+ OBD_MD_FLEASIZE|OBD_MD_FLDIREA,
>+ lmmsize, &req);
Please align arguments with '(' on previous line.
>+ body = lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF,
>+ sizeof(*body));
Align arguments with '('
>+ if (!(body->valid & (OBD_MD_FLEASIZE | OBD_MD_FLDIREA)) ||
>+ lmmsize == 0) {
Also align with 'if (' on previous line.
>+ lmm = lustre_msg_buf(req->rq_repmsg, REPLY_REC_OFF + 1,
>+ lmmsize);
Same.
>@@ -488,44 +559,29 @@ static int ll_dir_ioctl(struct inode *in
>+ rc = ll_lov_getstripe_ea_info(inode, filename, &lmm,
>&lmmsize, &request);
Wrap at 80 columns.
>+ if(rc < 0){
>+ if(rc == -ENODATA && (cmd == IOC_MDC_GETFILEINFO ||
>+ cmd == LL_IOC_MDC_GETINFO))
Align with '(cmd' on previous line.
>+int ll_lov_getstripe_ea_info(struct inode *inode, const char *filename,
>+ struct lov_mds_md **lmmp, int *lmm_size,
>+ struct ptlrpc_request **request)
>+{
>+ rc = mdc_getattr_name(sbi->ll_mdc_exp, &fid,
>+ filename, strlen(filename) + 1,
>+ OBD_MD_FLEASIZE | OBD_MD_FLDIREA,
>+ lmmsize, &req);
Please align arguments with '(' after mdc_getattr_name.
>@@ -149,6 +149,27 @@ int ll_setxattr(struct dentry *dentry, c
>+ if (strncmp(name, XATTR_TRUSTED_PREFIX, 8) == 0 &&
>+ strcmp(name + 8, "lov") == 0){
Align with 'if ('
>+ rc = ll_lov_setstripe_ea_info(inode, &f, flags, lump,
>sizeof(struct lov_user_md));
Wrap at 80 columns.
>+ }
>+ else if(S_ISDIR(inode->i_mode)){
>+ rc = ll_dir_setstripe(inode, lump);
Put '} else if (...' on same line.
>@@ -284,12 +305,53 @@ ssize_t ll_getxattr(struct dentry *dentr
>+ if (strncmp(name, XATTR_TRUSTED_PREFIX, 8) == 0 &&
>+ strcmp(name + 8, "lov") == 0){
Space between '== 0) {'.
>+ if ( S_ISREG(inode->i_mode)) {
>+ const char *filename = dentry->d_name.name;
>+ struct dentry *parent = dentry->d_parent;
>+ rc = ll_lov_getstripe_ea_info(parent->d_inode,
>filename, &lmm, &lmmsize, &request);
Please don't use temporary variables here, Lustre is very short of stack space.
Also needs to wrap at 80 columns.
At this point does the client already have lli->lli_smd? That would mean we
don't need to do an RPC to get the stripe data at all, because it was done
during the inode lookup or stat and the client already has it. Can you please
do a simple test with a file in the root directory and check how many MDS RPCs
it does for this inode?
>+ }
>+ else if (S_ISDIR(inode->i_mode)) {
>+ rc = ll_dir_getstripe(inode, &lmm, &lmmsize,
>&request);
>+ }
>+ else { /* symlink ? */
>+ return ENODATA;
Put '} else ...' on same line. Should this be -ENODATA?
> ssize_t ll_listxattr(struct dentry *dentry, char *buffer, size_t size)
> {
>+ int rc = 0, rc1=0;
Space around ' = '. Please use "rc2" or "err", as that is more common in other
parts of the code.
>@@ -297,6 +359,44 @@ ssize_t ll_listxattr(struct dentry *dent
>+ rc = ll_getxattr_common(inode, NULL, buffer, size, OBD_MD_FLXATTRLS);
>+
>+ if(!capable(CAP_SYS_ADMIN)){
>+ if(lsm == NULL)
Space between 'if (' everywhere.
>+ size_t prefix_len = sizeof(XATTR_TRUSTED_PREFIX)-1;
>+ size_t name_len = sizeof("lov") -1;
>+ size_t total_len = prefix_len + name_len + 1;
Please use "const size_t" and maybe compiler will optimize this away.
Does this need to be done for even root users? When I run "getfattr -d
/mnt/lustre/foo" it doesn't show trusted.lov even as root. I have to
specifically ask for it with "-n trusted.lov".
Most of the changes are not defects, just needed to match the CodingGuidelines
Please don't reply to lustre-devel. Instead, comment in Bugzilla by using the
following link:
on the wiki: https://mail.clusterfs.com/wikis/lustre/CodingGuidelines
Can you please write some simple regression tests for sanity.sh (not using
star) to verify that the NEW functionality introduced here works correctly.
That means getxattr -d (if installed) should return the trusted.lov EA for
regular files as root and non-root users, and mknod+setfattr should set the
striping correctly.
_______________________________________________
Lustre-devel mailing list
[email protected]
https://mail.clusterfs.com/mailman/listinfo/lustre-devel
