-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add support for FreeBSD's Solaris style extended attribute interface #17540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I wonder if it is allowed to rename named attributes into regular
directories or back? Should we check tdvp also, or it is done somewhere
else?
You are correct. Renaming can only happen within the named attribute dir.
This is checked in FreeBSD above the VOP layer (local syscalls and NFSv4
server).
In _PC_NAMEDATTR_ENABLED case you take MNT_ILOCK() for this check, but
not here. I have no idea what is this lock, but it looks different.
Good catch. mnt_flag is covered by the MNT_ILOCK() according
to sys/mount.h so I should acquire/release the lock.
I'll add that and update the pull request this coming weekend.
Hopefully I can just update the file, "git commit --amend" and
then "git push --force"? (I know diddly about git and mostly
curse at it;-)
Thanks for taking a look, rick
ps: I don't think the two test failures have anything to do with
my code. The only change that isn't in FreeBSD specific files
is LOOKUP_NAMED_ATTR that was added to xvattr.h and the
semantics is unchanged unless __FreeBSD_version >= 1500040.
|
Yes. That would be right.
It seems an unrelated noise. I've restarted them once more. |
Good catch. mnt_flag is covered by the MNT_ILOCK() according
to sys/mount.h so I should acquire/release the lock.
I'll admit I am always "on the fence" whether it is worth
locking/releasing a mutex when you are just reading a variable.
(Long ago alc@ explained to me that acquiring any mutex was enough
to guarantee the thread will see the post-mtx_unlock() value of
any memory address. As such, so long as there is a mtx_lock()
somewhere after the mtx_unlock() that follows setting MNT_NAMEDATTR,
it should be ok.)
But, I think it is just easier to acquire/release the mutex,
especially since this won't be happening frequently.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice to add some tests so we can exercise this new interface in the CI.
FreeBSD commit 2ec2ba7e232d added the Solaris style syscall interface for extended attributes. This patch wires this interface into the FreeBSD ZFS port, since this style of extended attributes is supported by OpenZFS internally when the "xattr" property is set to "dir". Some specific changes: LOOKUP_NAMED_ATTR is defined to indicate the need to set V_NAMEDATTR for calls to zfs_zaccess(). V_NAMEDATTR indicates that the access checking does need to be done for FreeBSD. The access checking code for extended attributes was copy/pasted from the Linux port into zfs_zaccess() in the FreeBSD port. Most of the changes are in zfs_freebsd_lookup() and zfs_freebsd_create(). The semantics of these functions should remain unchanged unless named attributes are being manipulated. All the code changes are enabled for __FreeBSD_version 1500040 and newer. Signed-off-by: Rick Macklem <[email protected]>
FreeBSD commit 2ec2ba7e232d added the Solaris style syscall interface for extended attributes.
This patch wires this interface into the FreeBSD ZFS port, since this style of extended attributes
is supported by OpenZFS internally when the "xattr" property is set to "dir".
Motivation and Context
The Solaris system call interface to extended attributes, which provides a directory
with the extended attributes in it as files, is preferred by some to the Linux/FreeBSD
system call interface.
Since the Solaris style system call interface is now in FreeBSD, this patch wires in
the support (which already exists in OpenZFS internally) for the FreeBSD port.
Description
The FreeBSD VOP interface now has OPENNAMED as a new flag for looking
up extended attributes and the extended attribute directory. (The named
attribute terminology is used to try and avoid confusion with the Linux/FreeBSD
system call interface for extended attributes.)
FreeBSD marks the vnodes with VIRF_NAMEDDIR and VIRF_NAMEDATTR for
the extended attribute directory and the extended attribute files, respectively.
Most of the changes are done to zfs_freebsd_lookup(), where it recognizes an
extended attribute lookup via the OPENNAMED flag and the extended attribute
directory via the VIRF_NAMEDDIR vnode flag.
zfs_zaccess() is modified by adding the access code that was in the Linux
port and enabling it for these extended attributes, using new LOOKUP_NAMED_ATTR
and V_NAMEDATTR flags.
zfs_freebsd_create() is modified to handle creation of new extended attributes.
A check for the attribute already existing is a "sa" block is missing and will be
added in a future commit.
Types of changes
Checklist:
Signed-off-by
.