Re: usb flashkey disk copy error

On Sun, 7 Sep 2003, raoul.megelas wrote:

> John-Mark Gurne wrote this message on Sun, Sep 07, 2003 at 08:45 +0200:
> raoul.megelas wrote:
>
> > I have a copy error between hdd and a flashkey 1gig usb (easydisk)
> > on Current dated August 28. Here is in short:
> >
> > mount -t msdos /dev/da2s1 /flashkey
> > cp myfile /flashkey/
> > diff myfile /flashkey/myfile
> > 	(ok).
>
> > could you try a fsync /flashkey/myfile before the umount?
> > ...
>
> You have found the trick, fsync after cp works fine.
> Thanks very much.
>
> But why the fsync is not automatically done by umount on umass?

msdosfs_unmount() seems to be missing a VOP_FSYNC() of the vnode for
the device file.  This is needed to flush dirty metadata, if any.
msdosfs_sync() is not missing this VOP_FSYNC(), and according to my
debugging code it occasionally does something (unlike for ffs_sync()
where there is almost always some dirty metadata.

Perhaps there is another bug for VOP_CLOSE() on the device file to not
do the sync, but ffs_unmount() does the VOP_FSYNC() explicitly (via
ffs_flushfiles()).  This may be just to get better error handling.  In
fact, there is another bug in ffs's not ignoring errors returned by
VOP_CLOSE(): they cause null pointer panics if VOP_CLOSE() actually
returns an error.  Quick fix for ffs_unmount():

%%%
Index: ffs_vfsops.c
===================================================================
RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.216
diff -u -r1.216 ffs_vfsops.c
--- ffs_vfsops.c	15 Aug 2003 20:03:19 -0000	1.216
+++ ffs_vfsops.c	17 Aug 2003 09:24:11 -0000
_at__at_ -993,6 +998,12 _at__at_
 	error = VOP_CLOSE(ump->um_devvp, FREAD|FWRITE, NOCRED, td);
 #endif

+	/*
+	 * XXX don't fail if VOP_CLOSE() failed since we have destroyed
+	 * our mount point and will soon destroy other resources.
+	 */
+	error = 0;
+
 	vrele(ump->um_devvp);

 	free(fs->fs_csp, M_UFSMNT);
%%%

To see this bug, arrange for a disk driver to return nonzero from its
close routine.  Calling vflush() and VOP_FSYNC() before committing to
finishing the unmount should result in there being no dirty blocks for
VOP_CLOSE() to flush, so the correct error handling for failure of
VOP_CLOSE() in the above may be to panic.

Bruce