Re: fusefs-kmod broken?

On Mon, Aug 23, 2010 at 04:32:58PM +0200, Ian FREISLICH wrote:
> Kostik Belousov wrote:
> > 
> > --7hK5U8dVDlZxii7z
> > Content-Type: text/plain; charset=us-ascii
> > Content-Disposition: inline
> > Content-Transfer-Encoding: quoted-printable
> > 
> > On Mon, Aug 23, 2010 at 03:47:23PM +0200, Ed Schouten wrote:
> > > * Kostik Belousov <kostikbel_at_gmail.com> wrote:
> > > > On Mon, Aug 23, 2010 at 03:35:55PM +0200, Ed Schouten wrote:
> > > > > * Kostik Belousov <kostikbel_at_gmail.com> wrote:
> > > > > > Which most likely means that fusesfs filled its own struct fileops
> > > > > > without properly initializing fo_truncate member.
> > > > >=20
> > > > > It's a bit misleading that cdevs automatically patch the table, while
> > > > > the fileops don't. Maybe it would be a good idea to patch finit() to
> > > > I do not understand your first sentence. Would you please elaborate ?
> > >=20
> > > Say, you create a cdev, if you don't implement all ops, it will check
> > > for null pointers and return error codes accordingly. This doesn't
> > > happen for fileops, which is probably one of the reasons why people
> > > sometimes forget to implement them.
> > >=20
> > > Wouldn't it be better to prevent this form of footshooting by adding
> > > assertions? This will add some overhead for any file descriptor created,
> > > but a kernel with INVARIANTS isn't meant to be fast.
> > Thanks, I see it now.
> > 
> > The cdev interface definitely falls into the public kernel interface.
> > Having to fill all cdevsw methods for a random driver is too much
> > burden put on the several dozens maintainers.
> > 
> > On the other hand, file level is not much widely used by third-party
> > components, and even in-tree code implements only ten different file
> > types.
> > 
> > I would not object loudly if someone put such checks as proposed
> > under INVARIANTS, but also I do not see a real point in having them.
> > Might be slightly better to put the checks, again under INVARIANTS,
> > in the fo_XXX() wrappers.
> 
> So, in this case is the fusefs module broken?  I'm guessing it is.
> I don't like the way fuse_fileops is initialised in fuse4bsd.  I
> would prefer for the struct to be zeroed and then the fo_xxx
> implimented bits set as appropriate.  That way when the struct is
> changed, you don't get stung again.
> 
> Patch attached to that makes fusefs-kmod not blowup kernels post this change.
> 
> Ian
> 
> -- 
> Ian Freislich
> 
> Index: files/patch-fuse_module__fuse_vnops.c
> ===================================================================
> RCS file: /home/ncvs/ports/sysutils/fusefs-kmod/files/patch-fuse_module__fuse_vnops.c,v
> retrieving revision 1.4
> diff -u -d -r1.4 patch-fuse_module__fuse_vnops.c
> --- files/patch-fuse_module__fuse_vnops.c	30 Oct 2008 15:36:35 -0000	1.4
> +++ files/patch-fuse_module__fuse_vnops.c	23 Aug 2010 14:27:17 -0000
> +_at__at_ -214,6 +214,7 _at__at_
> +          * following fields are filled from vnops, but "vnops.foo" is not
> +          * legitimate in a definition, so we set them at module load time
> + 	 */
> ++	.fo_truncate = NULL,
> + 	.fo_ioctl    = NULL,
> + 	.fo_poll     = NULL,
> + 	.fo_kqfilter = NULL,
Did you tested this ? I suppose that it would not change anything.
Fuse, most likely, lacks real implementation of .fo_truncate method.

The implementation was required for long time, otherwise file
truncation would not work.