Skip to contentSkip to conversation

HFSPlus: clear dirty flags on remount read-only

PATCH In review Timo Schlüßler 2 participants 3 messages Nov 29, 2015 – Dec 3, 2015

Subsystems (2)
VFS Hfsplus filesystem
lore
Share & tools
lore

Timo Schlüßler

author
#0 lore raw

PATCH 1/2 HFSPlus: clear dirty flags on remount read-only

Remounting a HFSPlus filesystem read-only doesn't clear the "DIRTY"-flags (not HFSPLUS_VOL_UNMNT and HFSPLUS_VOL_INCNSTNT) correctly. Subsequent mounts then report a dirty filesystem and enforce a check before willing to mount it read-write again.

Signed-off-by Timo Schlüßler <[email protected]>

---

fs/hfsplus/super.c 2015-11-22 20:14:38.000000000 +0100 renamed from fs/hfsplus/super.c.orig 2015-11-22 20:11:25.000000000 +0100 +8 -1
@@ -325,11 +325,12 @@ static int hfsplus_statfs(struct dentry
 
 static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
 {
+       struct hfsplus_vh *vhdr = HFSPLUS_SB(sb)->s_vhdr;
+
        sync_filesystem(sb);
        if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
                return 0;
        if (!(*flags & MS_RDONLY)) {
-               struct hfsplus_vh *vhdr = HFSPLUS_SB(sb)->s_vhdr;
                int force = 0;
 
                if (!hfsplus_parse_options_remount(data, &force))

@@ -352,6 +353,12 @@ static int hfsplus_remount(struct super_
                        sb->s_flags |= MS_RDONLY;
                        *flags |= MS_RDONLY;
                }
+       } else {
+               vhdr->modify_date = hfsp_now2mt();
+               vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_UNMNT);
+               vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_INCNSTNT);
+
+               hfsplus_sync_fs(sb, 1);
        }
        return 0;
 }
maintainer
#1 lore raw

Hi Timo,

Quoted 24 lines · Timo Schlüßler

Remounting a HFSPlus filesystem read-only doesn't clear the "DIRTY"-flags (not HFSPLUS_VOL_UNMNT and HFSPLUS_VOL_INCNSTNT) correctly. Subsequent mounts then report a dirty filesystem and enforce a check before willing to mount it read-write again.

Signed-off-by Timo Schlüßler <[email protected]>

---

fs/hfsplus/super.c 2015-11-22 20:14:38.000000000 +0100 renamed from fs/hfsplus/super.c.orig 2015-11-22 20:11:25.000000000 +0100 +2 -1
@@ -325,11 +325,12 @@ static int hfsplus_statfs(struct dentry
 
 static int hfsplus_remount(struct super_block *sb, int *flags, char *data)
 {
+       struct hfsplus_vh *vhdr = HFSPLUS_SB(sb)->s_vhdr;
+
        sync_filesystem(sb);
        if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
                return 0;
        if (!(*flags & MS_RDONLY)) {
-               struct hfsplus_vh *vhdr = HFSPLUS_SB(sb)->s_vhdr;
                int force = 0;
 
                if (!hfsplus_parse_options_remount(data, &force))

fs/hfsplus/super.c 2015-11-22 20:14:38.000000000 +0100 static int hfsplus_remount(struct super_ · line 359

Quoted 7 lines
fs/hfsplus/super.c 2015-11-22 20:14:38.000000000 +0100 renamed from fs/hfsplus/super.c.orig 2015-11-22 20:11:25.000000000 +0100 +4 -0
@@ -352,6 +353,12 @@ static int hfsplus_remount(struct super_
                        sb->s_flags |= MS_RDONLY;
                        *flags |= MS_RDONLY;
                }
+       } else {
+               vhdr->modify_date = hfsp_now2mt();
+               vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_UNMNT);
+               vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_INCNSTNT);

If you set HFSPLUS_VOL_UNMNT and clear HFSPLUS_VOL_INCNSTNT bits on operation of remount in READ-ONLY state then you should do opposite operation on remount in READ-WRITE state. But, as far as I can judge, you've missed this change. Such modification should be in the same patch.

fs/hfsplus/super.c 2015-11-22 20:14:38.000000000 +0100 static int hfsplus_remount(struct super_ · line 361

Quoted 2 lines
fs/hfsplus/super.c 2015-11-22 20:14:38.000000000 +0100 renamed from fs/hfsplus/super.c.orig 2015-11-22 20:11:25.000000000 +0100 +2 -0
+
+               hfsplus_sync_fs(sb, 1);

Are you sure that hfsplus_sync_fs() should be called here? As far as I can see, sync_filesystem() does it for you. What the reason for second call? Or do I miss something?

Thanks,
Vyacheslav Dubeyko.

author
#2 lore raw

Hello Vyacheslav,

thank you for your quick reply.

Quoted 8 lines · Vyacheslav Dubeyko

Hi Timo,

If you set HFSPLUS_VOL_UNMNT and clear HFSPLUS_VOL_INCNSTNT bits on operation of remount in READ-ONLY state then you should do opposite operation on remount in READ-WRITE state. But, as far as I can judge, you've missed this change. Such modification should be in the same patch.

Of course, I will add this change to the patch.

Quoted 3 lines

Are you sure that hfsplus_sync_fs() should be called here? As far as I can see, sync_filesystem() does it for you. What the reason for second call? Or do I miss something?

I wanted to avoid reordering the original code and also avoid to duplicate the call to sync_filesystem() (as I did in the end int the HFS patch). Now when adding the second change to clear HFSPLUS_VOL_UNMNT and set HFSPLUS_VOL_INCNSTNT on remounting READ-WRITE, the reordering makes perfect sense.

Quoted 3 lines

Thanks,
Vyacheslav Dubeyko.

Best regards,
Timo Schlüßler