Re: Heads Up: commit 2ec2ba7e232d just hit main

On Sat, Apr 5, 2025 at 8:50 AM Rick Macklem <rick.macklem@gmail.com> wrote:
>
> On Fri, Apr 4, 2025 at 6:27 PM Shawn Webb <shawn.webb@hardenedbsd.org> wrote:
> >
> > On Sat, Apr 05, 2025 at 01:04:25AM +0000, Shawn Webb wrote:
> > > On Fri, Apr 04, 2025 at 05:40:21PM -0700, Rick Macklem wrote:
> > > > On Fri, Apr 4, 2025 at 10:50 AM Shawn Webb <shawn.webb@hardenedbsd.org> wrote:
> > > > >
> > > > > On Thu, Apr 03, 2025 at 06:12:59PM -0700, Rick Macklem wrote:
> > > > > > On Thu, Apr 3, 2025 at 4:52 PM Shawn Webb <shawn.webb@hardenedbsd.org> wrote:
> > > > > > >
> > > > > > > On Wed, Apr 02, 2025 at 01:51:26PM -0700, Rick Macklem wrote:
> > > > > > > > The commit 2ec2ba7e232d just hit main.  I do not think it will
> > > > > > > > cause problems, but it is fairly large.
> > > > > > > >
> > > > > > > > Man page updates will be done as separate commits.
> > > > > > > >
> > > > > > > > Hopefully this will not cause grief, rick
> > > > > > >
> > > > > > > Hey Rick,
> > > > > > >
> > > > > > > The patch review test plan mentions a patch to ZFS itself to support
> > > > > > > named attributes. Is that patch available somewhere?
> > > > > > The ZFS patch is currently in phabricator as D49654.
> > > > > > Feel free to review it.
> > > > > >
> > > > > > It can also be found at:
> > > > > > https://people.freebsd.org/~rmacklem/zfs-xattr.patch
> > > > > > (this is a smaller diff which can be applied to an up-to-date main src
> > > > > > tree easily)
> > > > >
> > > > > Hey Rick,
> > > > >
> > > > > I applied that zfs patch, but trying pathconf(2) on a file on a ZFS
> > > > > dataset with xattr=on (which seems to be the default) returns 0. Am I
> > > > > doing something wrong?
> > > > >
> > > > > ==== BEGIN LOG ====
> > > > > hbsd-current-01[shawn]:/home/shawn/tmp $ ./xattrtest xattrtest
> > > > > xattrtest: Named attributes not enabled: No error: 0
> > > > > hbsd-current-01[shawn]:/home/shawn/tmp (1) $ zfs list /usr/home/shawn
> > > > > NAME             USED  AVAIL  REFER  MOUNTPOINT
> > > > > rpool/usr/home  10.4G  71.4G  9.85G  /usr/home
> > > > > hbsd-current-01[shawn]:/home/shawn/tmp $ zfs get xattr rpool/usr/home
> > > > > NAME            PROPERTY  VALUE  SOURCE
> > > > > rpool/usr/home  xattr     on     default
> > > > > ==== END LOG ====
> > > > >
> > > > > That xattrtest application is yours from:
> > > > > https://people.freebsd.org/~rmacklem/xattrtest.c
> > > > No idea. It works for me. You used up-to-date kernel sources?
> > > > (Check that VIRF_NAMEDATTR is defined in sys/sys/vnode.h.)
> > > > Oh, and one more thing to check. zfs_xattr_compat needs to be non-zero.
> > > > (It's found in sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c.
> > > > It's initialized to 1 and I don't see anything that sets it to 0?)
> > >
> > > It is indeed set to 1.
> > >
> > > >
> > > > The only thing I can think if is, if you changed xattr to on, you need to
> > > > reboot (or at least remount) to get it to take effect.
> > > > (Maybe try setting it to "dir" and then reboot/remount. Maybe there is
> > > > a difference between "on" and "dir"?)
> > >
> > > Yeah, tried rebooting and still no go. Note that xattr defaults to
> > > "on" in FreeBSD by default. My src tree is synced up to FreeBSD commit
> > > 7e70d94acd68b3ac6b45f49d4ab7a0f7867c3ea7. I brought in the ZFS patch
> > > you linked to.
> > >
> > > >
> > > > Or, did you build zfs.ko some other way than as part of a kernel build?
> > > > (It needs the patched .h files in the kernel sources, not something in
> > > > /usr/include/sys
> > > > that has not yet been updated.)
> > > > All the ZFS changes are #ifdef'd, since OpenZFS requires the sources
> > > > build for older kernels. (Basically #ifdef'd on that VIRF_NAMEDATTR mentioned
> > > > above.)
> > >
> > > Perhaps I need to do a clean build. I'll try that and report back.
> > >
> > > >
> > > > It does remind me that I need to try a build of zfs.ko by doing a "make" in
> > > > the module directory.
> > > >
> > > > You can try the attached trivial patch and see if it spits out "pathconf ret=1"
> > > > on the console.
> > >
> > > I'll try that after a clean rebuild of the kernel.
> >
> > Clean rebuild didn't resolve it. However, I made some progress.
> >
> > I created a dataset specifically for this since I can't really unmount
> > my /usr/home dataset, so my example down below is a bit different than
> > the previous example I gave.
> >
> > The xattr property is set to "on" for the dataset. However, it's not
> > mounted with the xattr property. In order to get it to work, I had to
> > run the following commands:
> >
> > ==== BEGIN LOG ====
> > $ sudo zfs umount rpool/scratch/xattr
> > $ sudo mount -t zfs -o xattr rpool/scratch/xattr /scratch/xattr
> > $ zfs get xattr rpool/scratch/xattr
> > NAME                 PROPERTY  VALUE  SOURCE
> > rpool/scratch/xattr  xattr     on     local
> > $ mount | grep xattr
> > rpool/scratch/xattr on /scratch/xattr (zfs, local, noatime, nfsv4acls, named attributes)
> > $ mount | grep named
> > rpool/scratch/xattr on /scratch/xattr (zfs, local, noatime, nfsv4acls, named attributes)
> > ==== END LOG ====
> >
> > So it looks like FreeBSD does not honor the xattr zfs property,
> > otherwise you would see a whole bunch of datasets mounted with the
> > "named attributes" flag in that last `mount | grep` command.
> I've looked at this a little more...
> There is a function xattr_changed_cb() that updates the xattr property
> information.
> It gets called when "zfs set xattr=XX <file-system>" is done or when
> the mount option "xattr" is set.
>
> The question seems to be "Why was the stuff not correctly set for your file
> systems, although they show xattr=on?"
> Maybe I'll try and ask somewhere there are ZFS wizards or look in some
> OpenZFS commit logs.
>
> Have your ZFS file systems that do not have "named attribute" set been
> around for quite a while? (I'm thinking that they might have been configured
> differently when they were created and never changed.)
>
> It does look like the commands (done as root) should do the same thing
> as your umount/mount did.
> # zfs set xattr=off <file-system>
> # zfs set xattr=dir <file-system>
Oh, and followed by a reboot or umount/mount without the xattr mount option.
>
> I'll poke around and see if I can find out what the story is, rick
>
> >
> > --
> > Shawn Webb
> > Cofounder / Security Engineer
> > HardenedBSD
> >
> > Signal Username:  shawn_webb.74
> > Tor-ified Signal: +1 303-901-1600 / shawn_webb_opsec.50
> > https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc