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

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

[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 <timo@schluessler.org>
---

--- linux/fs/hfsplus/super.c.orig       2015-11-22 20:11:25.000000000 +0100
+++ linux/fs/hfsplus/super.c    2015-11-22 20:14:38.000000000 +0100
@@ -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;
 }

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

[Vyacheslav Dubeyko]

Hi Timo,

On Sun, 2015-11-29 at 15:59 +0100, Timo Schlüßler wrote:
> 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 <timo@schluessler.org>
> ---
> 
> --- linux/fs/hfsplus/super.c.orig       2015-11-22 20:11:25.000000000 +0100
> +++ linux/fs/hfsplus/super.c    2015-11-22 20:14:38.000000000 +0100
> @@ -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);

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.

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

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

[Timo Schlüßler]

Hello Vyacheslav,

thank you for your quick reply.

On 29.11.2015 23:50, Vyacheslav Dubeyko wrote:
> 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.

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

> Thanks,
> Vyacheslav Dubeyko.
>

Best regards,
Timo Schlüßler