Date: Tue, 03 Apr 2012 05:10:03 +0200 From: Grzegorz Bernacki <gjb@semihalf.com> To: Marcel Moolenaar <marcel@xcllnt.net> Cc: geom@FreeBSD.org, fs@FreeBSD.org Subject: Re: Review of projects/nand branch Message-ID: <4F7A6A0B.5000308@semihalf.com> In-Reply-To: <EE2F4D7D-F6D9-48D4-923A-99DAA62698B2@xcllnt.net> References: <EE2F4D7D-F6D9-48D4-923A-99DAA62698B2@xcllnt.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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@freebsd.org and/or geom@freebsd.org. I > saw comments from pjd already for example. > 2. Please address the following points before merging onto head: > > o In include/Makefile: fs/fifofs is removed. Deliberate? > o In sbin/Makefile: we should have a distinct MK_NANDFS option > for use by the file system code. > o In sbin/nandfs/nandfs.8: could elaborate for what one could > use the snapshots. > o In sbin/nandfs/nandfs.h: define NANDFS_H. > o In sbin/nandfs/nandfs.c: usage() is wrong. > o In sbin/nandfs/Makefile: $FreeBSD$ is missing. > 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. > o In sbin/mount/mntopts.h: cleanerd is gone, so should not be > needed anymore. > o In sbin/newfs_nandfs/newfs_nandfs.c: we have CRC32 code for > re-use. No need to implement again. > o In sbin/newfs_nandfs/Makefile: missing DPADD. > 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. > o In usr.sbin/nandtool/Makefile: missing $FreeBSD$ > o In usr.sbin/nandsim/Makefile: missing $FreeBSD$ > 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@freebsd.org. > o In sys/modules/Makefile: always build nandfs module. Make > nandsim module dependent on MK_NAND or MK_NANDSIM if added. > 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@freebsd.org. > o Please get buy-in for changes to sys/sys/disk.h and > sys/sys/bio.h from people on either fs@freebsd.org or > geom@freebsd.org. > > 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)? > > I'd like to thank Semihalf and the Foundation for bringing > this code into FreeBSD. Well done! > Thanks for the comments. We are going to get rid of all the changes in files from sys/geom, sys/sys, and sys/kern directories. Soon those changes will be merged into nand project branch. For the rest of your changes, let us go through them and then we will response to them. thanks, grzesiek
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4F7A6A0B.5000308>