Various problems were detected by xfstests.
This series aims to fix them.
v2:
- swap patch 3 and 4;
- fixed commit message for patch 1;
- fixed compile error in patch 2.
Konstantin Komarov (4):
fs/ntfs3: Keep preallocated only if option prealloc enabled
fs/ntfs3: Restore ntfs_xattr_get_acl and ntfs_xattr_set_acl functions
fs/ntfs3: Update i_ctime when xattr is added
fs/ntfs3: Optimize locking in ntfs_save_wsl_perm
fs/ntfs3/file.c | 2 +-
fs/ntfs3/xattr.c | 123 ++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 113 insertions(+), 12 deletions(-)
--
2.33.0
Ctime wasn't updated after setfacl command.
This commit fixes xfstest generic/307
Fixes: be71b5cba2e6 ("fs/ntfs3: Add attrib operations")
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/xattr.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 3ccdb8c2ac0b..157b70aecb4f 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -992,6 +992,9 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
err = ntfs_set_ea(inode, name, name_len, value, size, flags);
out:
+ inode->i_ctime = current_time(inode);
+ mark_inode_dirty(inode);
+
return err;
}
--
2.33.0
Apparently we need to maintain these functions with
ntfs_get_acl_ex and ntfs_set_acl_ex.
This commit fixes xfstest generic/099
Fixes: 95dd8b2c1ed0 ("fs/ntfs3: Remove unnecessary functions")
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/xattr.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 95 insertions(+), 1 deletion(-)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 2143099cffdf..3ccdb8c2ac0b 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -112,7 +112,7 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
return -ENOMEM;
if (!size) {
- ;
+ /* EA info persists, but xattr is empty. Looks like EA problem. */
} else if (attr_ea->non_res) {
struct runs_tree run;
@@ -616,6 +616,67 @@ int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
return ntfs_set_acl_ex(mnt_userns, inode, acl, type);
}
+static int ntfs_xattr_get_acl(struct user_namespace *mnt_userns,
+ struct inode *inode, int type, void *buffer,
+ size_t size)
+{
+ struct posix_acl *acl;
+ int err;
+
+ if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
+ ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
+ return -EOPNOTSUPP;
+ }
+
+ acl = ntfs_get_acl(inode, type);
+ if (IS_ERR(acl))
+ return PTR_ERR(acl);
+
+ if (!acl)
+ return -ENODATA;
+
+ err = posix_acl_to_xattr(mnt_userns, acl, buffer, size);
+ posix_acl_release(acl);
+
+ return err;
+}
+
+static int ntfs_xattr_set_acl(struct user_namespace *mnt_userns,
+ struct inode *inode, int type, const void *value,
+ size_t size)
+{
+ struct posix_acl *acl;
+ int err;
+
+ if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
+ ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
+ return -EOPNOTSUPP;
+ }
+
+ if (!inode_owner_or_capable(mnt_userns, inode))
+ return -EPERM;
+
+ if (!value) {
+ acl = NULL;
+ } else {
+ acl = posix_acl_from_xattr(mnt_userns, value, size);
+ if (IS_ERR(acl))
+ return PTR_ERR(acl);
+
+ if (acl) {
+ err = posix_acl_valid(mnt_userns, acl);
+ if (err)
+ goto release_and_out;
+ }
+ }
+
+ err = ntfs_set_acl(mnt_userns, inode, acl, type);
+
+release_and_out:
+ posix_acl_release(acl);
+ return err;
+}
+
/*
* ntfs_init_acl - Initialize the ACLs of a new inode.
*
@@ -782,6 +843,23 @@ static int ntfs_getxattr(const struct xattr_handler *handler, struct dentry *de,
goto out;
}
+#ifdef CONFIG_NTFS3_FS_POSIX_ACL
+ if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
+ !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
+ sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
+ (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
+ !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
+ sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
+ /* TODO: init_user_ns? */
+ err = ntfs_xattr_get_acl(
+ &init_user_ns, inode,
+ name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
+ ? ACL_TYPE_ACCESS
+ : ACL_TYPE_DEFAULT,
+ buffer, size);
+ goto out;
+ }
+#endif
/* Deal with NTFS extended attribute. */
err = ntfs_get_ea(inode, name, name_len, buffer, size, NULL);
@@ -894,6 +972,22 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
goto out;
}
+#ifdef CONFIG_NTFS3_FS_POSIX_ACL
+ if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
+ !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
+ sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
+ (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
+ !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
+ sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
+ err = ntfs_xattr_set_acl(
+ mnt_userns, inode,
+ name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
+ ? ACL_TYPE_ACCESS
+ : ACL_TYPE_DEFAULT,
+ value, size);
+ goto out;
+ }
+#endif
/* Deal with NTFS extended attribute. */
err = ntfs_set_ea(inode, name, name_len, value, size, flags);
--
2.33.0
If size of file was reduced, we still kept allocated blocks.
This commit makes ntfs3 work as other fs like btrfs.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=214719
Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")
Reported-by: Ganapathi Kamath <[email protected]>
Tested-by: Ganapathi Kamath <[email protected]>
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 43b1451bff53..3ac0482c6880 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -494,7 +494,7 @@ static int ntfs_truncate(struct inode *inode, loff_t new_size)
down_write(&ni->file.run_lock);
err = attr_set_size(ni, ATTR_DATA, NULL, 0, &ni->file.run, new_size,
- &new_valid, true, NULL);
+ &new_valid, ni->mi.sbi->options->prealloc, NULL);
up_write(&ni->file.run_lock);
if (new_valid < ni->i_valid)
--
2.33.0
Right now in ntfs_save_wsl_perm we lock/unlock 4 times.
This commit fixes this situation.
We add "locked" argument to ntfs_set_ea.
Suggested-by: Kari Argillander <[email protected]>
Signed-off-by: Konstantin Komarov <[email protected]>
---
fs/ntfs3/xattr.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
index 157b70aecb4f..6d8b1cd7681d 100644
--- a/fs/ntfs3/xattr.c
+++ b/fs/ntfs3/xattr.c
@@ -259,7 +259,7 @@ static int ntfs_get_ea(struct inode *inode, const char *name, size_t name_len,
static noinline int ntfs_set_ea(struct inode *inode, const char *name,
size_t name_len, const void *value,
- size_t val_size, int flags)
+ size_t val_size, int flags, bool locked)
{
struct ntfs_inode *ni = ntfs_i(inode);
struct ntfs_sb_info *sbi = ni->mi.sbi;
@@ -278,7 +278,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
u64 new_sz;
void *p;
- ni_lock(ni);
+ if (!locked)
+ ni_lock(ni);
run_init(&ea_run);
@@ -467,7 +468,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
mark_inode_dirty(&ni->vfs_inode);
out:
- ni_unlock(ni);
+ if (!locked)
+ ni_unlock(ni);
run_close(&ea_run);
kfree(ea_all);
@@ -595,7 +597,7 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
flags = 0;
}
- err = ntfs_set_ea(inode, name, name_len, value, size, flags);
+ err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
if (err == -ENODATA && !size)
err = 0; /* Removing non existed xattr. */
if (!err)
@@ -989,7 +991,7 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
}
#endif
/* Deal with NTFS extended attribute. */
- err = ntfs_set_ea(inode, name, name_len, value, size, flags);
+ err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
out:
inode->i_ctime = current_time(inode);
@@ -1007,35 +1009,37 @@ int ntfs_save_wsl_perm(struct inode *inode)
{
int err;
__le32 value;
+ struct ntfs_inode *ni = ntfs_i(inode);
- /* TODO: refactor this, so we don't lock 4 times in ntfs_set_ea */
+ ni_lock(ni);
value = cpu_to_le32(i_uid_read(inode));
err = ntfs_set_ea(inode, "$LXUID", sizeof("$LXUID") - 1, &value,
- sizeof(value), 0);
+ sizeof(value), 0, true); /* true == already locked. */
if (err)
goto out;
value = cpu_to_le32(i_gid_read(inode));
err = ntfs_set_ea(inode, "$LXGID", sizeof("$LXGID") - 1, &value,
- sizeof(value), 0);
+ sizeof(value), 0, true);
if (err)
goto out;
value = cpu_to_le32(inode->i_mode);
err = ntfs_set_ea(inode, "$LXMOD", sizeof("$LXMOD") - 1, &value,
- sizeof(value), 0);
+ sizeof(value), 0, true);
if (err)
goto out;
if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
value = cpu_to_le32(inode->i_rdev);
err = ntfs_set_ea(inode, "$LXDEV", sizeof("$LXDEV") - 1, &value,
- sizeof(value), 0);
+ sizeof(value), 0, true);
if (err)
goto out;
}
out:
+ ni_unlock(ni);
/* In case of error should we delete all WSL xattr? */
return err;
}
--
2.33.0
On Tue, Oct 26, 2021 at 07:42:15PM +0300, Konstantin Komarov wrote:
> Right now in ntfs_save_wsl_perm we lock/unlock 4 times.
> This commit fixes this situation.
> We add "locked" argument to ntfs_set_ea.
>
> Suggested-by: Kari Argillander <[email protected]>
> Signed-off-by: Konstantin Komarov <[email protected]>
Add tag if you fix Joes nit.
Reviewed-by: Kari Argillander <[email protected]>
> ---
> fs/ntfs3/xattr.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index 157b70aecb4f..6d8b1cd7681d 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -259,7 +259,7 @@ static int ntfs_get_ea(struct inode *inode, const char *name, size_t name_len,
>
> static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> size_t name_len, const void *value,
> - size_t val_size, int flags)
> + size_t val_size, int flags, bool locked)
> {
> struct ntfs_inode *ni = ntfs_i(inode);
> struct ntfs_sb_info *sbi = ni->mi.sbi;
> @@ -278,7 +278,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> u64 new_sz;
> void *p;
>
> - ni_lock(ni);
> + if (!locked)
> + ni_lock(ni);
>
> run_init(&ea_run);
>
> @@ -467,7 +468,8 @@ static noinline int ntfs_set_ea(struct inode *inode, const char *name,
> mark_inode_dirty(&ni->vfs_inode);
>
> out:
> - ni_unlock(ni);
> + if (!locked)
> + ni_unlock(ni);
>
> run_close(&ea_run);
> kfree(ea_all);
> @@ -595,7 +597,7 @@ static noinline int ntfs_set_acl_ex(struct user_namespace *mnt_userns,
> flags = 0;
> }
>
> - err = ntfs_set_ea(inode, name, name_len, value, size, flags);
> + err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
> if (err == -ENODATA && !size)
> err = 0; /* Removing non existed xattr. */
> if (!err)
> @@ -989,7 +991,7 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
> }
> #endif
> /* Deal with NTFS extended attribute. */
> - err = ntfs_set_ea(inode, name, name_len, value, size, flags);
> + err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
Did you miss Joes comment or is there another reason why there is still
is (true|0) arguments?
https://lore.kernel.org/ntfs3/[email protected]/
argillander
>
> out:
> inode->i_ctime = current_time(inode);
> @@ -1007,35 +1009,37 @@ int ntfs_save_wsl_perm(struct inode *inode)
> {
> int err;
> __le32 value;
> + struct ntfs_inode *ni = ntfs_i(inode);
>
> - /* TODO: refactor this, so we don't lock 4 times in ntfs_set_ea */
> + ni_lock(ni);
> value = cpu_to_le32(i_uid_read(inode));
> err = ntfs_set_ea(inode, "$LXUID", sizeof("$LXUID") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true); /* true == already locked. */
> if (err)
> goto out;
>
> value = cpu_to_le32(i_gid_read(inode));
> err = ntfs_set_ea(inode, "$LXGID", sizeof("$LXGID") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true);
> if (err)
> goto out;
>
> value = cpu_to_le32(inode->i_mode);
> err = ntfs_set_ea(inode, "$LXMOD", sizeof("$LXMOD") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true);
> if (err)
> goto out;
>
> if (S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) {
> value = cpu_to_le32(inode->i_rdev);
> err = ntfs_set_ea(inode, "$LXDEV", sizeof("$LXDEV") - 1, &value,
> - sizeof(value), 0);
> + sizeof(value), 0, true);
> if (err)
> goto out;
> }
>
> out:
> + ni_unlock(ni);
> /* In case of error should we delete all WSL xattr? */
> return err;
> }
> --
> 2.33.0
>
>
>
On Tue, Oct 26, 2021 at 07:40:57PM +0300, Konstantin Komarov wrote:
> If size of file was reduced, we still kept allocated blocks.
> This commit makes ntfs3 work as other fs like btrfs.
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=214719
> Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation")
>
> Reported-by: Ganapathi Kamath <[email protected]>
> Tested-by: Ganapathi Kamath <[email protected]>
> Signed-off-by: Konstantin Komarov <[email protected]>
Reviewed-by: Kari Argillander <[email protected]>
> ---
> fs/ntfs3/file.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
> index 43b1451bff53..3ac0482c6880 100644
> --- a/fs/ntfs3/file.c
> +++ b/fs/ntfs3/file.c
> @@ -494,7 +494,7 @@ static int ntfs_truncate(struct inode *inode, loff_t new_size)
>
> down_write(&ni->file.run_lock);
> err = attr_set_size(ni, ATTR_DATA, NULL, 0, &ni->file.run, new_size,
> - &new_valid, true, NULL);
> + &new_valid, ni->mi.sbi->options->prealloc, NULL);
> up_write(&ni->file.run_lock);
>
> if (new_valid < ni->i_valid)
> --
> 2.33.0
>
>
On Tue, Oct 26, 2021 at 07:41:27PM +0300, Konstantin Komarov wrote:
> Apparently we need to maintain these functions with
> ntfs_get_acl_ex and ntfs_set_acl_ex.
> This commit fixes xfstest generic/099
I like how you phrase this in one other patch
Fixes generic/099
but no need to change
> Fixes: 95dd8b2c1ed0 ("fs/ntfs3: Remove unnecessary functions")
>
> Signed-off-by: Konstantin Komarov <[email protected]>
Reviewed-by: Kari Argillander <[email protected]>
> ---
> fs/ntfs3/xattr.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index 2143099cffdf..3ccdb8c2ac0b 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -112,7 +112,7 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea,
> return -ENOMEM;
>
> if (!size) {
> - ;
> + /* EA info persists, but xattr is empty. Looks like EA problem. */
> } else if (attr_ea->non_res) {
> struct runs_tree run;
>
> @@ -616,6 +616,67 @@ int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
> return ntfs_set_acl_ex(mnt_userns, inode, acl, type);
> }
>
> +static int ntfs_xattr_get_acl(struct user_namespace *mnt_userns,
> + struct inode *inode, int type, void *buffer,
> + size_t size)
> +{
> + struct posix_acl *acl;
> + int err;
> +
> + if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
> + ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
> + return -EOPNOTSUPP;
> + }
> +
> + acl = ntfs_get_acl(inode, type);
> + if (IS_ERR(acl))
> + return PTR_ERR(acl);
> +
> + if (!acl)
> + return -ENODATA;
> +
> + err = posix_acl_to_xattr(mnt_userns, acl, buffer, size);
> + posix_acl_release(acl);
> +
> + return err;
> +}
> +
> +static int ntfs_xattr_set_acl(struct user_namespace *mnt_userns,
> + struct inode *inode, int type, const void *value,
> + size_t size)
> +{
> + struct posix_acl *acl;
> + int err;
> +
> + if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
> + ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
> + return -EOPNOTSUPP;
> + }
> +
> + if (!inode_owner_or_capable(mnt_userns, inode))
> + return -EPERM;
> +
> + if (!value) {
> + acl = NULL;
> + } else {
> + acl = posix_acl_from_xattr(mnt_userns, value, size);
> + if (IS_ERR(acl))
> + return PTR_ERR(acl);
> +
> + if (acl) {
> + err = posix_acl_valid(mnt_userns, acl);
> + if (err)
> + goto release_and_out;
> + }
> + }
> +
> + err = ntfs_set_acl(mnt_userns, inode, acl, type);
> +
> +release_and_out:
> + posix_acl_release(acl);
> + return err;
> +}
> +
> /*
> * ntfs_init_acl - Initialize the ACLs of a new inode.
> *
> @@ -782,6 +843,23 @@ static int ntfs_getxattr(const struct xattr_handler *handler, struct dentry *de,
> goto out;
> }
>
> +#ifdef CONFIG_NTFS3_FS_POSIX_ACL
> + if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
> + !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
> + sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
> + (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
> + !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
> + sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
> + /* TODO: init_user_ns? */
> + err = ntfs_xattr_get_acl(
> + &init_user_ns, inode,
> + name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
> + ? ACL_TYPE_ACCESS
> + : ACL_TYPE_DEFAULT,
> + buffer, size);
> + goto out;
> + }
> +#endif
> /* Deal with NTFS extended attribute. */
> err = ntfs_get_ea(inode, name, name_len, buffer, size, NULL);
>
> @@ -894,6 +972,22 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
> goto out;
> }
>
> +#ifdef CONFIG_NTFS3_FS_POSIX_ACL
> + if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
> + !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
> + sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
> + (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
> + !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
> + sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
> + err = ntfs_xattr_set_acl(
> + mnt_userns, inode,
> + name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
> + ? ACL_TYPE_ACCESS
> + : ACL_TYPE_DEFAULT,
> + value, size);
> + goto out;
> + }
> +#endif
> /* Deal with NTFS extended attribute. */
> err = ntfs_set_ea(inode, name, name_len, value, size, flags);
>
> --
> 2.33.0
>
On Tue, Oct 26, 2021 at 07:41:50PM +0300, Konstantin Komarov wrote:
> Ctime wasn't updated after setfacl command.
> This commit fixes xfstest generic/307
When I run xfstest I get
generic/307 [20:37:41][ 21.436315] run fstests generic/307 at 2021-10-26 20:37:41
[ 23.362544] vdc:
[failed, exit status 1] [20:37:45]- output mismatch (see /results/ntfs3/results-default/generic/307.out.bad)
--- tests/generic/307.out 2021-08-03 00:08:10.000000000 +0000
+++ /results/ntfs3/results-default/generic/307.out.bad 2021-10-26 20:37:45.172171949 +0000
@@ -1,2 +1,4 @@
QA output created by 307
Silence is golden
+setfacl: symbol lookup error: setfacl: undefined symbol: walk_tree
+error: ctime not updated after setfacl
...
(Run 'diff -u /root/xfstests/tests/generic/307.out /results/ntfs3/results-default/generic/307.out.bad' to see the entire diff)
any ideas you get different result?
> Fixes: be71b5cba2e6 ("fs/ntfs3: Add attrib operations")
>
> Signed-off-by: Konstantin Komarov <[email protected]>
> ---
> fs/ntfs3/xattr.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
> index 3ccdb8c2ac0b..157b70aecb4f 100644
> --- a/fs/ntfs3/xattr.c
> +++ b/fs/ntfs3/xattr.c
> @@ -992,6 +992,9 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
> err = ntfs_set_ea(inode, name, name_len, value, size, flags);
>
> out:
> + inode->i_ctime = current_time(inode);
> + mark_inode_dirty(inode);
> +
> return err;
> }
>
> --
> 2.33.0
>
>
Hi Konstantin,
I love your patch! Yet something to improve:
[auto build test ERROR on linux/master]
[also build test ERROR on linus/master v5.15-rc7 next-20211026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Konstantin-Komarov/fs-ntfs3-Various-fixes-for-xattr-and-files/20211027-004353
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8ec3401acda35f5fe1fd4a0c1a5a422e7178acad
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Konstantin-Komarov/fs-ntfs3-Various-fixes-for-xattr-and-files/20211027-004353
git checkout 8ec3401acda35f5fe1fd4a0c1a5a422e7178acad
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
fs/ntfs3/xattr.c: In function 'ntfs_xattr_get_acl':
>> fs/ntfs3/xattr.c:634:15: error: too few arguments to function 'ntfs_get_acl'
634 | acl = ntfs_get_acl(inode, type);
| ^~~~~~~~~~~~
fs/ntfs3/xattr.c:533:19: note: declared here
533 | struct posix_acl *ntfs_get_acl(struct inode *inode, int type, bool rcu)
| ^~~~~~~~~~~~
vim +/ntfs_get_acl +634 fs/ntfs3/xattr.c
621
622 static int ntfs_xattr_get_acl(struct user_namespace *mnt_userns,
623 struct inode *inode, int type, void *buffer,
624 size_t size)
625 {
626 struct posix_acl *acl;
627 int err;
628
629 if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
630 ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
631 return -EOPNOTSUPP;
632 }
633
> 634 acl = ntfs_get_acl(inode, type);
635 if (IS_ERR(acl))
636 return PTR_ERR(acl);
637
638 if (!acl)
639 return -ENODATA;
640
641 err = posix_acl_to_xattr(mnt_userns, acl, buffer, size);
642 posix_acl_release(acl);
643
644 return err;
645 }
646
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On 26.10.2021 23:41, Kari Argillander wrote:
> On Tue, Oct 26, 2021 at 07:41:50PM +0300, Konstantin Komarov wrote:
>> Ctime wasn't updated after setfacl command.
>> This commit fixes xfstest generic/307
>
> When I run xfstest I get
>
> generic/307 [20:37:41][ 21.436315] run fstests generic/307 at 2021-10-26 20:37:41
> [ 23.362544] vdc:
> [failed, exit status 1] [20:37:45]- output mismatch (see /results/ntfs3/results-default/generic/307.out.bad)
> --- tests/generic/307.out 2021-08-03 00:08:10.000000000 +0000
> +++ /results/ntfs3/results-default/generic/307.out.bad 2021-10-26 20:37:45.172171949 +0000
> @@ -1,2 +1,4 @@
> QA output created by 307
> Silence is golden
> +setfacl: symbol lookup error: setfacl: undefined symbol: walk_tree
> +error: ctime not updated after setfacl
> ...
> (Run 'diff -u /root/xfstests/tests/generic/307.out /results/ntfs3/results-default/generic/307.out.bad' to see the entire diff)
>
> any ideas you get different result?
>
What are mount options for this test?
generic/307 passes with "acl" and "sparse,acl".
Can you try to locate where is "undefined symbol: walk_tree" coming from?
>> Fixes: be71b5cba2e6 ("fs/ntfs3: Add attrib operations")
>>
>> Signed-off-by: Konstantin Komarov <[email protected]>
>> ---
>> fs/ntfs3/xattr.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
>> index 3ccdb8c2ac0b..157b70aecb4f 100644
>> --- a/fs/ntfs3/xattr.c
>> +++ b/fs/ntfs3/xattr.c
>> @@ -992,6 +992,9 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
>> err = ntfs_set_ea(inode, name, name_len, value, size, flags);
>>
>> out:
>> + inode->i_ctime = current_time(inode);
>> + mark_inode_dirty(inode);
>> +
>> return err;
>> }
>>
>> --
>> 2.33.0
>>
>>