Re: [PATCH] hfsplus: add journal replay

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



------------------------------
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.

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.

There are one or two spelling mistakes in one of the comments, as well
as some grammar issues. 
--
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




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux