Question about the FFS snapshot code

[Xin LI]

Hi folks,

I have by chance got a panic right after another panic's reboot, which
seems to be related to the patch attached.  The problem seems to be
not easy to reproduce and I don't have much knowledge about the snapshot
code, so I would rather request for some comment on the proposaled
change.

The rationale:

Revision 1.27 of ffs_snapshot.c has introduced an additional cleanup
during taking a snapshot, as the commit log said:

      ``When taking a snapshot, we must check for active files that have
	been unlinked (e.g., with a zero link count). We have to expunge
	all trace of these files from the snapshot so that they are neither
	reclaimed prematurely by fsck nor saved unnecessarily by dump.''

And on HEAD, we found some related code, starting at line 447:

		/*
		 * If there is a fragment, clear it here.
		 */
		blkno = 0;
		loc = howmany(xp->i_size, fs->fs_bsize) - 1;
		if (loc < NDADDR) {
			len = fragroundup(fs, blkoff(fs, xp->i_size));
-------->>>>> len could be 0?
			if (len < fs->fs_bsize) {
				ffs_blkfree(copy_fs, vp, DIP(xp, i_db[loc]),
				    len, xp->i_number);
				blkno = DIP(xp, i_db[loc]);
				DIP_SET(xp, i_db[loc], 0);
			}
		}

I have a suspect that len could got a zero after the line marked. Subsequent
``if'' statement would pass beacuse 0 is always less then fs->fs_bsize.
This might get some unwanted result, in my opinion.

Any comments is appreciated.

Cheers,
-- 
Xin LI <delphij frontfree net>	http://www.delphij.net/
See complete headers for GPG key and other information.

-------------- next part --------------
Index: ffs_snapshot.c
===================================================================
RCS file: /home/fcvs/src/sys/ufs/ffs/ffs_snapshot.c,v
retrieving revision 1.84
diff -u -r1.84 ffs_snapshot.c
--- ffs_snapshot.c	28 Jul 2004 06:41:27 -0000	1.84
+++ ffs_snapshot.c	10 Aug 2004 17:30:04 -0000
@@ -451,7 +451,7 @@
 		loc = howmany(xp->i_size, fs->fs_bsize) - 1;
 		if (loc < NDADDR) {
 			len = fragroundup(fs, blkoff(fs, xp->i_size));
-			if (len < fs->fs_bsize) {
+			if ((len != 0) && (len < fs->fs_bsize)) {
 				ffs_blkfree(copy_fs, vp, DIP(xp, i_db[loc]),
 				    len, xp->i_number);
 				blkno = DIP(xp, i_db[loc]);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20040812/6e91cf2a/attachment.bin