> Am 23.02.2014 um 05:58 schrieb Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx>:
>
> ------------------------------
>> On Sun, Feb 23, 2014 2:26 AM GMT Sergei Antonov wrote:
>>
>> Add journal replay functionality. If filesystem is mounted for read-write and
>> a non-empty journal is found, the replay action will be done. The original
>> behaviour of switching to read-only mode in presense of journal by default is
>> retained since the driver does not support journaled metadata modification
>> anyway. After an attempt to replay has been done, the mount procedure repeats
>> loading volume header to get its updated state. Having performed journal replay
>> we do not run a risk of stumbling on inconsistency when working with the volume,
>> so this functionality adds stability, while not changing any existing behaviour.
>> Mount logic in super.c is changed as conservatively as possible.
>>
>> The replay code checks the integrity of the whole journal before actually
>> replaying sectors. If any error (e.g. checksum mismatch, invalid/misaligned
>> offsets) is detected, the volume will remain unchanged and the journal will not
>> be cleared.
>>
>> The code supports checksums for replay data in addition to checksums for header
>> and lists. Checksums for data is a relatively modern feature present on volumes
>> modified by modern OS X versions.
>>
>> Tested with:
>> drives with 512 and 4K sector sizes
>> default (8 MB) and custom (1 MB, 500 MB) journal sizes
>> big-endian (PowerPC) and little-endian (Intel) journal byte sexes
>>
>> Advantages over a previously presented
>> "[PATCH v3 00/15] hfsplus: introduce journal replay functionality"
>> by a different author:
>> support for big-endian journal
>> no redundant stuff like journal initialization
>> less intrusive changes
>> shorter, easier to review
>>
>> Signed-off-by: Sergei Antonov <saproj@xxxxxxxxx>
>> CC: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
>> CC: Christoph Hellwig <hch@xxxxxxxxxxxxx>
>> CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> CC: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
>> CC: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx>
>
> Acked-by: Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx>
>
> I appreciate you have done further work on the byte-swapping
> in the big-endian journals.
>
> Since you are not updating the journal header with its original
> endianness (I don't know whether this is a good thing or not -
> would like to know what the darwin kernel does), it seems
> that there is no need to keep journal_swab around in the super-block.
>
Yes. Accepted.
> Another comment below.
>
>> ---
>> fs/hfsplus/Makefile | 3 +-
>> fs/hfsplus/hfsplus_fs.h | 9 +
>> fs/hfsplus/hfsplus_raw.h | 65 +++-
>> fs/hfsplus/journal.c | 903 +++++++++++++++++++++++++++++++++++++++++++++++
>> fs/hfsplus/super.c | 30 +-
>> 5 files changed, 1004 insertions(+), 6 deletions(-)
>> create mode 100644 fs/hfsplus/journal.c
>>
>> diff --git a/fs/hfsplus/Makefile b/fs/hfsplus/Makefile
>> index 683fca2..927e89c 100644
>> --- a/fs/hfsplus/Makefile
>> +++ b/fs/hfsplus/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_HFSPLUS_FS) += hfsplus.o
>>
>> hfsplus-objs := super.o options.o inode.o ioctl.o extents.o catalog.o dir.o btree.o \
>> bnode.o brec.o bfind.o tables.o unicode.o wrapper.o bitmap.o part_tbl.o \
>> - attributes.o xattr.o xattr_user.o xattr_security.o xattr_trusted.o
>> + attributes.o xattr.o xattr_user.o xattr_security.o xattr_trusted.o \
>> + journal.o
>>
>> hfsplus-$(CONFIG_HFSPLUS_FS_POSIX_ACL) += posix_acl.o
>> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
>> index 08846425b..2edbfd1 100644
>> --- a/fs/hfsplus/hfsplus_fs.h
>> +++ b/fs/hfsplus/hfsplus_fs.h
> ...
>> @@ -166,6 +167,11 @@ struct hfsplus_sb_info {
>> u32 total_blocks;
>> u32 data_clump_blocks, rsrc_clump_blocks;
>>
>> + /* immutable data on the placement of the journal (if there is one) */
>> + u64 journal_offset; /* relative to volume start */
>> + u64 journal_size; /* relative to volume start */
>> + bool journal_swab; /* need swap bytes in journal */
>> +
>> /* mutable data from the volume header, protected by alloc_mutex */
>> u32 free_blocks;
>> struct mutex alloc_mutex;
> ...
>
>> --- a/fs/hfsplus/hfsplus_raw.h
>> +++ b/fs/hfsplus/hfsplus_raw.h
>> @@ -105,7 +105,7 @@ struct hfsplus_vh {
>> __be16 version;
>> __be32 attributes;
>> __be32 last_mount_vers;
>> - u32 reserved;
>> + __be32 jrnl_info_block;
>>
>> __be32 create_date;
>> __be32 modify_date;
>> @@ -145,6 +145,69 @@ struct hfsplus_vh {
>> #define HFSPLUS_VOL_JOURNALED (1 < 13)
>> #define HFSPLUS_VOL_SOFTLOCK (1 < 15)
>>
>> +/* Journal info */
>> +struct hfs_jrnl_info {
>> + __be32 flags;
>> + u32 unused[8];
>> + __be64 journal_offset; /* Misaligned! relative to volume start */
>> + __be64 journal_size; /* Misaligned! relative to volume start */
>> +} __packed;
>> +
> ...
>
> Storing journal_offset and journal_size in two places seems redundant
> - the journal info block is part of the volume header struct, which is accessible
> from the superblock struct.
It is not in volume header. Only its block number is in header.
>
> There are one or two spelling mistakes in one of the comments, as well
> as some grammar issues.
Thanks for spotting. Could you show them in a format like this?
- wrong text
+ corrected text
I'd like to know of them.--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
