From owner-freebsd-geom@FreeBSD.ORG Tue Apr 3 03:10:17 2012 Return-Path: Delivered-To: geom@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F0380106564A; Tue, 3 Apr 2012 03:10:16 +0000 (UTC) (envelope-from gjb@semihalf.com) Received: from smtp.semihalf.com (smtp.semihalf.com [213.17.239.109]) by mx1.freebsd.org (Postfix) with ESMTP id 686278FC08; Tue, 3 Apr 2012 03:10:16 +0000 (UTC) Received: from localhost (unknown [213.17.239.109]) by smtp.semihalf.com (Postfix) with ESMTP id 0413AC4B60; Tue, 3 Apr 2012 05:10:02 +0200 (CEST) X-Virus-Scanned: by amavisd-new at semihalf.com Received: from smtp.semihalf.com ([213.17.239.109]) by localhost (smtp.semihalf.com [213.17.239.109]) (amavisd-new, port 10024) with ESMTP id 3VSuFgcJfFBz; Tue, 3 Apr 2012 05:10:01 +0200 (CEST) Received: from [172.17.136.194] (adsl-66-120-169-242.dsl.sntc01.pacbell.net [66.120.169.242]) by smtp.semihalf.com (Postfix) with ESMTPSA id 8D9C6C4B48; Tue, 3 Apr 2012 05:09:59 +0200 (CEST) Message-ID: <4F7A6A0B.5000308@semihalf.com> Date: Tue, 03 Apr 2012 05:10:03 +0200 From: Grzegorz Bernacki User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 MIME-Version: 1.0 To: Marcel Moolenaar References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: geom@FreeBSD.org, fs@FreeBSD.org Subject: Re: 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: Tue, 03 Apr 2012 03:10:17 -0000 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