Details
-
Bug
-
Resolution: Fixed
-
Minor
-
None
-
None
-
3
-
9223372036854775807
Description
There seems to be an issue with XATTR changelogs for some specific prefixes such as trusted or security, but not user.
# bash lustre/tests/llmount.sh # touch /mnt/lustre/blob # lctl --device lustre-MDT0000 changelog_register # setfattr -n trusted.test -v test /mnt/lustre/blob # lfs changelog lustre-MDT0000 1 -1(null) 08:13:35.501636480 2018.01.30 0x0 t=[0x200000401:0x1:0x0] j=setfattr.0 ef=0x1 u=0:0 # setfattr -n user.test -v test /mnt/lustre/blob # lfs changelog lustre-MDT0000 1 -1(null) 08:13:35.501636480 2018.01.30 0x0 t=[0x200000401:0x1:0x0] j=setfattr.0 ef=0x1 u=0:0 2 15XATTR 08:13:21.191376815 2018.01.30 0x0 t=[0x200000401:0x1:0x0] j=setfattr.0 ef=0x1 u=0:0
Attachments
Activity
John L. Hammond (jhammond@whamcloud.com) uploaded a new patch: https://review.whamcloud.com/33048
Subject: LU-10579 mdd: emit changelogs for securit* and trusted xattrs
Project: fs/lustre-release
Branch: b2_10
Current Patch Set: 1
Commit: 9958d6f33212d818a439801b88c480207e449908
Fixed in 2.11 by https://review.whamcloud.com/28251 LU-9727 lustre: add CL_GETXATTR for Changelogs.
This looks like fallout from LU-7264 so would be present in 2.10 and later.
I ran into this working on changelog processing for HPE DMF 7.
mdd_xattr_changelog_type() returns -1 to indicate that no record should be emitted. This is itself a rather dubious practice: I'd prefer that CL_XATTR records be generated for those cases as well. At present I got half my wish, because they're generated as -1 records.
The reason is that the return type is not int but enum changelog_rec_type, and this is an unsigned type. So checks like the one in mdd_declare_xattr_set() always fail, and the record is generated with type ((enum changelog_rec_type)-1).
static int mdd_declare_xattr_set(const struct lu_env *env, struct mdd_device *mdd, struct mdd_object *obj, const struct lu_buf *buf, const char *name, int fl, struct thandle *handle) { enum changelog_rec_type type; int rc; rc = mdo_declare_xattr_set(env, obj, buf, name, fl, handle); if (rc) return rc; type = mdd_xattr_changelog_type(env, mdd, name); >>> if (type < 0) >>> return 0; /* no changelog to store */ return mdd_declare_changelog_store(env, mdd, type, NULL, NULL, handle); }
My preferred solution is that these changelog records be generated for these cases, rather than dropped on the floor. This can be done by changing mdd_xattr_changelog_type() to look like this:
static enum changelog_rec_type mdd_xattr_changelog_type(const struct lu_env *env, struct mdd_device *mdd, const char *xattr_name) { /* Layout changes systematically recorded */ if (strcmp(XATTR_NAME_LOV, xattr_name) == 0 || strncmp(XATTR_LUSTRE_LOV, xattr_name, strlen(XATTR_LUSTRE_LOV)) == 0) return CL_LAYOUT; /* HSM information changes systematically recorded */ if (strcmp(XATTR_NAME_HSM, xattr_name) == 0) return CL_HSM; /* Everything else systematically recorded */ return CL_XATTR; }
If this is done then the checks of the return value can also be removed of course.
It seems that we're not handling these prefixes in mdd_xattr_changelog_type():
static enum changelog_rec_type mdd_xattr_changelog_type(const struct lu_env *env, struct mdd_device *mdd, const char *xattr_name) { /* Layout changes systematically recorded */ if (strcmp(XATTR_NAME_LOV, xattr_name) == 0 || strncmp(XATTR_LUSTRE_LOV, xattr_name, strlen(XATTR_LUSTRE_LOV)) == 0) return CL_LAYOUT; /* HSM information changes systematically recorded */ if (strcmp(XATTR_NAME_HSM, xattr_name) == 0) return CL_HSM; if (has_prefix(xattr_name, XATTR_USER_PREFIX) || has_prefix(xattr_name, XATTR_NAME_POSIX_ACL_ACCESS) || has_prefix(xattr_name, XATTR_NAME_POSIX_ACL_DEFAULT)) return CL_XATTR; return -1; }
Note that 33048 doesn't address the issue that returning -1 from mdd_xattr_changelog_type() does not, in fact, suppress the generation of a changelog entry.