Review of projects/nand branch

[Grzegorz Bernacki]

Hi Marcel,

Please find updated status of fixing bugs inlined.

W dniu 2012-04-03 05:10, Grzegorz Bernacki pisze:
> W dniu 2012-04-02 23:37, Marcel Moolenaar pisze:
>> Grzegorz,
>>
>> I reviewed the changes on the projects/nand branch and in general
>> it's of high quality and any problems, improvements and/or cleanups
>> can be addressed after it gets merged into -current, with the
>> following caveat:
>> 1.  Changes to sys/kern, sys/geom and sys/sys should be reviewed and
>>      approved by people on fs at freebsd.org and/or geom at freebsd.org. I
>>      saw comments from pjd already for example.
Changes to geom has been reverted. We are working on remove rest of changes
from sys/kern and sys/sys
>>
>> 2.  Please address the following points before merging onto head:
>>
>> o   In include/Makefile: fs/fifofs is removed. Deliberate?
I applied incorrectly created patch. It was fixed with merge from HEAD.
>>
>> o   In sbin/Makefile: we should have a distinct MK_NANDFS option
>>      for use by the file system code.

- Is a separate MK_NANDFS knob really needed? Other filesystems don't seem to
follow this route
- The sys/fs/nandfs is only included per kernel config option, other userspace
components per MK_NAND
- Do you really think it is useful to have NAND framework built without NANDFS
and vice versa, the FS without userland tools for it?

>> o   In sbin/nandfs/nandfs.8: could elaborate for what one could
>>      use the snapshots.
Will be fixed
>> o   In sbin/nandfs/nandfs.h: define NANDFS_H.
Fixed
>> o   In sbin/nandfs/nandfs.c: usage() is wrong.
>> o   In sbin/nandfs/Makefile: $FreeBSD$ is missing.
Fixed
>> o   In sbin/mount_nandfs/mount_nandfs.8: copyright notice seems
>>      bogusly copied. Also, cleanerd is gone so it needs updating.
>> o   In sbin/mount_nandfs/mount_nandfs.c: cleanerd is gone, so
>>      this file could do with a some cleanups.
>> o   In sbin/mount_nandfs/Makefile: $FreeBSD$ is missing.
mount_nandfs have been removed.
>> o   In sbin/mount/mntopts.h: cleanerd is gone, so should not be
>>      needed anymore.
Fixed
>> o   In sbin/newfs_nandfs/newfs_nandfs.c: we have CRC32 code for
>>      re-use. No need to implement again.
Will be fixed later.
>>
>> o   In sbin/newfs_nandfs/Makefile: missing DPADD.
Fixed
>>
>> o   In share/mk/bsd.own.mk: Add NANDFS as well. May also want to
>>      add NANDSIM separately.
>> o   In share/man/man5/Makefile: should be NANDFS.
Both above will be fixed soon.
>>
>> o   In usr.sbin/nandtool/Makefile: missing $FreeBSD$
>> o   In usr.sbin/nandsim/Makefile: missing $FreeBSD$
Both above are fixed
>> o   usr.sbin/Makefile should have nandtool and nandsim when
>>      MK_NAND is defined.
>> o   In lib/Makefile: should be MK_NANDFS; not MK_NAND.
>> o   In lib/libstand/nandfs.c: should use common CRC32 impl.
>> o   In lib/libstand/Makefile: should be MK_NANDFS; not MK_NAND.
>> o   Please get buy-in for changes to sys/kern/vfs_vnops.c,
>>      sys/kern/vfs_bio.c and sys/kern/vfs_subr.c from people
>>      on fs at freebsd.org.
>> o   In sys/modules/Makefile: always build nandfs module. Make
>>      nandsim module dependent on MK_NAND or MK_NANDSIM if added.
All above will be fixed soon.
>>
>> o   Please get buy-in for changes to sys/geom/geom_dev.c,
>>      sys/geom/geom_disk.c, sys/geom/geom_disk.h, sys/geom/geom.h
>>      and sys/geom/geom_slice.c from people on geom at freebsd.org.
Geom changes has been removed.
>>
>> o   Please get buy-in for changes to sys/sys/disk.h and
>>      sys/sys/bio.h from people on either fs at freebsd.org or
>>      geom at freebsd.org.
Those changes has been removed.
>>
>> I also have a general usability question relating snapshots.
>> Currently snapshots are read-only. A useful feature in the
>> embedded space is to make a snapshot, attempt a software
>> update and revert to the snapshot if and when the update fails
>> or gets aborted. Is it possible to extend the snapshot feature
>> in the future to allow for this use case (i.e. ignore any and
>> all modifications that happened after a snapshot was made and
>> mount the snapshot R/W as representing the current/latest state
>> of the file system)?
We are working on this.

thanks,
Grzesiek