From owner-freebsd-geom@FreeBSD.ORG Mon Apr 2 21:37:38 2012 Return-Path: Delivered-To: geom@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 914F4106566C; Mon, 2 Apr 2012 21:37:38 +0000 (UTC) (envelope-from marcel@xcllnt.net) Received: from mail.xcllnt.net (mail.xcllnt.net [70.36.220.4]) by mx1.freebsd.org (Postfix) with ESMTP id 2D8F38FC15; Mon, 2 Apr 2012 21:37:31 +0000 (UTC) Received: from sa-nc-ipg-172-23-0-161.static.jnpr.net (natint3.juniper.net [66.129.224.36]) (authenticated bits=0) by mail.xcllnt.net (8.14.5/8.14.5) with ESMTP id q32LbO76067348 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Mon, 2 Apr 2012 14:37:30 -0700 (PDT) (envelope-from marcel@xcllnt.net) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Apple Message framework v1257) From: Marcel Moolenaar Date: Mon, 2 Apr 2012 14:37:18 -0700 Content-Transfer-Encoding: 7bit Message-Id: To: Grzegorz Bernacki X-Mailer: Apple Mail (2.1257) Cc: geom@FreeBSD.org, fs@FreeBSD.org Subject: Review of projects/nand branch X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Apr 2012 21:37:38 -0000 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, -- Marcel Moolenaar marcel@xcllnt.net