Bug 236475

Summary:	[ZFS] Invalid VOP_FSYNC options in vdev_file_io_start
Product:	Base System	Reporter:	Alan Somers <asomers>
Component:	kern	Assignee:	Andriy Gapon <avg>
Status:	Closed Not A Bug
Severity:	Affects Many People	CC:	lwhsu
Priority:	---
Version:	CURRENT
Hardware:	Any
OS:	Any

Description Alan Somers freebsd_committer

2019-03-11 20:27:21 UTC

From inspection I can see that vdev_file_io_start provides invalid options to VOP_FSYNC.  The only valid options are MNT_WAIT, MNT_NOWAIT, and MNT_LAZY, but that function uses FSYNC | FDSYNC.  This bug has been present ever since the first import of ZFS.

Comment 1 commit-hook freebsd_committer

2019-03-22 09:11:54 UTC

A commit references this bug:

Author: avg
Date: Fri Mar 22 09:11:46 UTC 2019
New revision: 345410
URL: https://svnweb.freebsd.org/changeset/base/345410

Log:
  ZFS vdev_file: use correct value for waitfor parameter of VOP_FSYNC

  PR:		236475
  Reported by:	asomers
  MFC after:	2 weeks

Changes:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c

Comment 2 Li-Wen Hsu freebsd_committer

2019-03-22 16:37:28 UTC

After r345410, initialing ZFS triggers assertion failure:

https://ci.freebsd.org/job/FreeBSD-head-amd64-test/10583/consoleFull

panic: solaris assert: flag == FSYNC, file: /usr/src/sys/cddl/compat/opensolaris/sys/vnode.h, line: 238

Should this `ASSERT(flag == FSYNC);` also need to be changed?

Comment 3 commit-hook freebsd_committer

2019-03-22 17:44:53 UTC

A commit references this bug:

Author: avg
Date: Fri Mar 22 17:44:47 UTC 2019
New revision: 345418
URL: https://svnweb.freebsd.org/changeset/base/345418

Log:
  Revert r345410, VOP_FSYNC change in ZFS vdev_file

  I overlooked the fact that that VOP_FSYNC() call is not a FreeBSD VFS
  call, but a macro that provides an illumos-compatible wrapper for the
  FreeBSD operation.

  PR:		236475
  Reported by:	lwhsu
  Pointyhat to:	avg

Changes:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_file.c

Comment 4 Alan Somers freebsd_committer

2019-03-22 17:48:15 UTC

Sorry for the false alarm, then.