From owner-freebsd-geom@FreeBSD.ORG Sat Apr 14 04:13:32 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 949C5106564A; Sat, 14 Apr 2012 04:13:32 +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 048C98FC12; Sat, 14 Apr 2012 04:13:31 +0000 (UTC) Received: from localhost (unknown [213.17.239.109]) by smtp.semihalf.com (Postfix) with ESMTP id C3749D8D82; Sat, 14 Apr 2012 06:13:22 +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 Ut97wpc9CmkH; Sat, 14 Apr 2012 06:13:21 +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 82913D8D7C; Sat, 14 Apr 2012 06:13:20 +0200 (CEST) Message-ID: <4F88F966.5030300@semihalf.com> Date: Sat, 14 Apr 2012 06:13:26 +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: <4F7A6A0B.5000308@semihalf.com> In-Reply-To: <4F7A6A0B.5000308@semihalf.com> 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: Sat, 14 Apr 2012 04:13:32 -0000 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@freebsd.org and/or geom@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@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@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@freebsd.org or >> geom@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