Received-SPF: Pass (protection.outlook.com: domain of mit.edu designates
 18.9.28.11 as permitted sender) receiver=protection.outlook.com;
 client-ip=18.9.28.11; helo=outgoing.mit.edu;
Date:   Wed, 6 Mar 2019 11:48:46 -0500
From:   "Theodore Y. Ts'o" <tytso@mit.edu>
To:     Artem Blagodarenko <artem.blagodarenko@gmail.com>
CC:     <linux-ext4@vger.kernel.org>, <adilger.kernel@dilger.ca>
Subject: Re: [PATCH v7 1/4] ext2fs: opening filesystem code refactoring
Message-ID: <20190306164846.GA3485@mit.edu>
References: <20190129175134.26652-1-c17828@cray.com>
 <20190129175134.26652-2-c17828@cray.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20190129175134.26652-2-c17828@cray.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2019 16:48:48.3956
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 0acae9ab-6219-4a47-0998-08d6a2539b10
X-MS-Exchange-CrossTenant-Id: 64afd9ba-0ecf-4acf-bc36-935f6235ba8b
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=64afd9ba-0ecf-4acf-bc36-935f6235ba8b;Ip=[18.9.28.11];Helo=[outgoing.mit.edu]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR01MB4858
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

On Tue, Jan 29, 2019 at 08:51:31PM +0300, Artem Blagodarenko wrote:
> There are similar opening filesystem code in different utilities.
> 
> The patch moves improved handling from try_open_fs()
> into ext2fs_open(). This function make one of the action
> based on parameters:
> 1) open filesystem with given superblock, superblock size
> 2) open filesystem with given superblock, but try to
> find right block size

This changes the behavior of ext2fs_open() when the superblock and
blocksize parameters is zero.  Previously we read the primary
superblock at the known location, and a blocksize of zero means "don't
care" and so we just do the right thing.  With your patch, it's going
to loop for all possible block sizes, failing with
EXT2_ET_UNEXPECTED_BLOCK_SIZE, until we stumble on the correct file
system.  That's unfortunate, especially if the device is opened with
IO_FLAG_DIRECT_IO.

So the brute force searching should be done using if an explicit flag
is given.  Using a new flag to request this new behavior is safer, but
it's probably OK if we only do the blocksize search if the superblock
parameter is zero --- you'll note that's what the code you were
replacing in dumpe2fs and e2fsck was doing.

More generally, for this whole patch series, in the "happy case" where
the file system is fine, e2fsck and e2image should *not* do any extra
work.

						- Ted