[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <5E568C7A-6A5B-4699-9266-8A0CD2537E5F@whamcloud.com>
Date: Fri, 29 Jul 2011 12:25:45 -0600
From: Andreas Dilger <adilger@...mcloud.com>
To: Bernd Schubert <bernd.schubert@...m.fraunhofer.de>
Cc: tytso@....edu, linux-ext4@...r.kernel.org,
Johann Lombardi <johann@...mcloud.com>
Subject: Re: [PATCH 1/2] ext2fs: add multi-mount protection (INCOMPAT_MMP)
On 2011-07-29, at 11:44 AM, Bernd Schubert wrote:
> thanks a lot for sending those patches! From a previous review I still remember ext2fs_mmp_read(), see below.
> I will look through the other parts later on.
>
>> +errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf)
>> +{
>> + /* ext2fs_open reserves fd0,1,2 to avoid stdio collision */
>> + if (fs->mmp_fd<= 0) {
>> + fs->mmp_fd = open(fs->device_name, O_RDWR | O_DIRECT);
>> + if (fs->mmp_fd< 0) {
>> + retval = EXT2_ET_MMP_OPEN_DIRECT;
>> + goto out;
>> + }
>> + }
>> +
>> + if (ext2fs_llseek(fs->mmp_fd, mmp_blk * fs->blocksize, SEEK_SET) !=
>> + mmp_blk * fs->blocksize) {
>> + retval = EXT2_ET_LLSEEK_FAILED;
>> + goto out;
>> + }
>> +
>> + if (read(fs->mmp_fd, fs->mmp_cmp, fs->blocksize) != fs->blocksize) {
>> + retval = EXT2_ET_SHORT_READ;
>> + goto out;
>> + }
>
> While ext2fs_llseek() and read() works fine, I still wonder why the code does not use io_channel_read_blk64() here, similarly as io_channel_write_blk64() write in ext2fs_mmp_write().
One of the reasons for doing a direct open+seek+read here is because previous
versions of e2fsprogs did not allow O_DIRECT reads via io_channel_read_blk64(),
which is critical for the correct MMP functioning. It cannot allow read from
a cached block, either in libext2fs or in the VM, though a cached block on the
disk is OK as long as it is visible to all hosts that are accessing it.
Cheers, Andreas
--
Andreas Dilger
Principal Engineer
Whamcloud, Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists