From owner-svn-src-head@FreeBSD.ORG Mon Jan 19 11:45:04 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A253D1065670; Mon, 19 Jan 2009 11:45:04 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail05.syd.optusnet.com.au (mail05.syd.optusnet.com.au [211.29.132.186]) by mx1.freebsd.org (Postfix) with ESMTP id 3F99F8FC1A; Mon, 19 Jan 2009 11:45:04 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-107-120-227.carlnfd1.nsw.optusnet.com.au (c122-107-120-227.carlnfd1.nsw.optusnet.com.au [122.107.120.227]) by mail05.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id n0JBj1MY012601 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 19 Jan 2009 22:45:02 +1100 Date: Mon, 19 Jan 2009 22:45:01 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Stanislav Sedov In-Reply-To: <200901181404.n0IE4uXw075698@svn.freebsd.org> Message-ID: <20090119220943.P37158@delplex.bde.org> References: <200901181404.n0IE4uXw075698@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r187395 - head/sys/gnu/fs/ext2fs X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Jan 2009 11:45:05 -0000 On Sun, 18 Jan 2009, Stanislav Sedov wrote: > Author: stas > Date: Sun Jan 18 14:04:56 2009 > New Revision: 187395 > URL: http://svn.freebsd.org/changeset/base/187395 > > Log: > - Obtain inode sizes and location of the first inode based on the contents > of superblock rather than using hardcoded values. This fixes ext2fs on > filesystems with inode sized other than 128. > > Submitted by: Alex Lyashkov (based on) > MFC after: 2 weeks This was not suitable for committing verbatim. > Modified: head/sys/gnu/fs/ext2fs/ext2_fs.h > ============================================================================== > --- head/sys/gnu/fs/ext2fs/ext2_fs.h Sun Jan 18 13:04:38 2009 (r187394) > +++ head/sys/gnu/fs/ext2fs/ext2_fs.h Sun Jan 18 14:04:56 2009 (r187395) > @@ -150,8 +150,8 @@ > #else /* !notyet */ ^^^^^^^^^^^^^^^^^^^ > #define EXT2_INODES_PER_BLOCK(s) ((s)->s_inodes_per_block) > /* Should be sizeof(struct ext2_inode): */ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > -#define EXT2_INODE_SIZE 128 > -#define EXT2_FIRST_INO 11 > +#define EXT2_INODE_SIZE(s) ((s)->s_inode_size) > +#define EXT2_FIRST_INO(s) ((s)->s_first_inode) > #endif /* notyet */ Please read the code before changing parts of it. The comment is now just wrong. One value that EXT2_INODE_SIZE certainly should _not_ be is the fixed size of our data structure, since we are now trying to support a variable size and even have a parameter to do that. The ifdef is now more bogus than before. The "!notyet" part now essentially duplicates the old unchanged "notyet" part, except for having bugs not present in the old version. This ifdef is a FreeBSD hack which should have been removed. Linux ext2fs just has the "notyet" part. Here is complete old ifdef: % #ifdef notyet % #ifdef __KERNEL__ % #define EXT2_ADDR_PER_BLOCK_BITS(s) ((s)->u.ext2_sb.s_addr_per_block_bits) % #define EXT2_INODE_SIZE(s) ((s)->u.ext2_sb.s_inode_size) % #define EXT2_FIRST_INO(s) ((s)->u.ext2_sb.s_first_ino) This is literally the same as in Linux. It cannot be used directly, now for only minor reasons: - FreeBSD doesn't have "u.ext2.sb". After deleting "u.ext2_sb." from the above, it seems to be right. - this was already done in the !notyet part for the 1st macro - your change does this for the other 2 macros, but this is not enough -- see below. % #else % #define EXT2_INODE_SIZE(s) (((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \ % EXT2_GOOD_OLD_INODE_SIZE : \ % (s)->s_inode_size) % #define EXT2_FIRST_INO(s) (((s)->s_rev_level == EXT2_GOOD_OLD_REV) ? \ % EXT2_GOOD_OLD_FIRST_INO : \ % (s)->s_first_ino) s_inode_size and s_first_ino cannot be used directly because they aren't supported by EXT2_GOOD_OLD_REV file systems. The conversion is done in the above macros for the !KERNEL case. It is done during superblock initialization in Linux ext2fs (at least the in-core copy of the superblock is written to). It is not done in FreeBSD ext2fs. Thus support for EXT2_GOOD_OLD_REV file systems has been broken. The change seems to be correct except for the above, and 1 other major style bug. Bruce