Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 24 Sep 2010 01:15:55 +0200
From:      Markus Gebert <markus.gebert@hostpoint.ch>
To:        Michael Naef <cal@linu.gs>
Cc:        freebsd-fs <freebsd-fs@freebsd.org>
Subject:   Re: Strange behaviour with sappend flag set on ZFS
Message-ID:  <66757A1E-E445-4AAD-8F57-382D85BFD579@hostpoint.ch>
In-Reply-To: <201009231938.09548.cal@linu.gs>
References:  <201009231938.09548.cal@linu.gs>

next in thread | previous in thread | raw e-mail | index | archive | help

On 23.09.2010, at 19:38, Michael Naef wrote:

> If I open (2) a file with O_APPEND the fd position pointer is set=20
> to the start of the file. When I then write to an fd on a ufs FS in=20
> FB6.4, it is automatically moved to the end and the bytes are=20
> appended. No problems appear with write to the sappend-flages file.
>=20
> However if I do the same on FB8.1/ZFS, the write fails as "not=20
> permitted".

To me, it seems that the zfs_write() VOP incorrectly uses FAPPEND =
instead of IO_APPEND when checking the ioflag argument to see if the =
current write is an appending one, here:

----
	/*
	 * If immutable or not appending then return EPERM
	 */
	pflags =3D zp->z_phys->zp_flags;
	if ((pflags & (ZFS_IMMUTABLE | ZFS_READONLY)) ||
	    ((pflags & ZFS_APPENDONLY) && !(ioflag & FAPPEND) &&
	    (uio->uio_loffset < zp->z_phys->zp_size))) {
		ZFS_EXIT(zfsvfs);
		return (EPERM);
	}
----

The function comment only mentions IO_APPEND and even later on in =
zfs_write() IO_APPEND is used to check wether the offset should be =
changed to EOF, so FAPPEND really seems to wrong in the above permission =
check.

CURRENT and STABLE-8 seem to be affected to. The following patch seems =
to fix it (at least Michi's test case works fine with it):

----
diff -ru =
../src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c =
./sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
--- ../src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c   =
2010-05-19 08:49:52.000000000 +0200
+++ ./sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c        =
2010-09-23 23:24:43.549846948 +0200
@@ -709,7 +709,7 @@
         */
        pflags =3D zp->z_phys->zp_flags;
        if ((pflags & (ZFS_IMMUTABLE | ZFS_READONLY)) ||
-           ((pflags & ZFS_APPENDONLY) && !(ioflag & FAPPEND) &&
+           ((pflags & ZFS_APPENDONLY) && !(ioflag & IO_APPEND) &&
            (uio->uio_loffset < zp->z_phys->zp_size))) {
                ZFS_EXIT(zfsvfs);
                return (EPERM);
----

Can someone commit this if the patch is ok? Or should I (or Michi) open =
a PR?


Markus





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?66757A1E-E445-4AAD-8F57-382D85BFD579>