* [PATCH] ext4: setattr: Forbid setting data journalling when delalloc enabled
@ 2012-01-09 15:45 Lukas Czerner
2012-01-09 15:55 ` Ted Ts'o
0 siblings, 1 reply; 3+ messages in thread
From: Lukas Czerner @ 2012-01-09 15:45 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, Lukas Czerner
Ext4 does not support data journalling with delayed allocation enabled.
We even does not allow to mount the file system with delayed allocation
and data journalling enabled, but it can be set via FS_IOC_SETFLAGS
which is wrong.
When data journalling and delayed allocation is enabled it can lead to
troubles. Here is one example how to reproduce an oops:
mkfs.ext4 /dev/sdd
mount /dev/sdd /mnt/test1
dd if=/dev/zero of=/mnt/test1/file bs=1M count=4
chattr +j /mnt/test1/file
dd if=/dev/zero of=/mnt/test1/file bs=1M count=4 conv=notrunc
chattr -j /mnt/test1/file
Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
fs/ext4/ioctl.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index a567968..f1b8226 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -126,7 +126,21 @@ flags_err:
if (err)
goto flags_out;
+ /*
+ * We absolutely can not journal data when delayed allocation
+ * is enabled. This is not possible to do via mount option
+ * and it should not be possible to do via setattr as well.
+ * Delayed allocation with data=journal is simply not
+ * supported.
+ */
if ((jflag ^ oldflags) & (EXT4_JOURNAL_DATA_FL))
+ if (test_opt(sb, DELALLOC)) {
+ ext4_msg(sb, KERN_ERR,
+ "Data journalling not supported "
+ "with delayed allocation.");
+ err = -EOPNOTSUPP;
+ goto flags_out;
+ }
err = ext4_change_inode_journal_flag(inode, jflag);
if (err)
goto flags_out;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ext4: setattr: Forbid setting data journalling when delalloc enabled
2012-01-09 15:45 [PATCH] ext4: setattr: Forbid setting data journalling when delalloc enabled Lukas Czerner
@ 2012-01-09 15:55 ` Ted Ts'o
2012-01-09 16:22 ` Lukas Czerner
0 siblings, 1 reply; 3+ messages in thread
From: Ted Ts'o @ 2012-01-09 15:55 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-ext4
On Mon, Jan 09, 2012 at 04:45:03PM +0100, Lukas Czerner wrote:
> Ext4 does not support data journalling with delayed allocation enabled.
> We even does not allow to mount the file system with delayed allocation
> and data journalling enabled, but it can be set via FS_IOC_SETFLAGS
> which is wrong.
>
> When data journalling and delayed allocation is enabled it can lead to
> troubles. Here is one example how to reproduce an oops:
>
> mkfs.ext4 /dev/sdd
> mount /dev/sdd /mnt/test1
> dd if=/dev/zero of=/mnt/test1/file bs=1M count=4
> chattr +j /mnt/test1/file
> dd if=/dev/zero of=/mnt/test1/file bs=1M count=4 conv=notrunc
> chattr -j /mnt/test1/file
>
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
Don't we also need to add a check in ext4_should_journal_data() in
fs/ext4/ext4_jbd2.h? Otherwise we could get in trouble if the data
journalling flag had been set previously when the file system was
mounted nodelalloc.
In fact it could be argued that we don't need to return EOPNOSUPP, or
printk a message, but simply make sure that the data journal flag is
ignored in if delayed allocation is enabled. If that's true, we can
allow the user to set or clear the journal flag as much as they like;
it would just be ignored.
Thanks,
- Ted
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ext4: setattr: Forbid setting data journalling when delalloc enabled
2012-01-09 15:55 ` Ted Ts'o
@ 2012-01-09 16:22 ` Lukas Czerner
0 siblings, 0 replies; 3+ messages in thread
From: Lukas Czerner @ 2012-01-09 16:22 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Lukas Czerner, linux-ext4
On Mon, 9 Jan 2012, Ted Ts'o wrote:
> On Mon, Jan 09, 2012 at 04:45:03PM +0100, Lukas Czerner wrote:
> > Ext4 does not support data journalling with delayed allocation enabled.
> > We even does not allow to mount the file system with delayed allocation
> > and data journalling enabled, but it can be set via FS_IOC_SETFLAGS
> > which is wrong.
> >
> > When data journalling and delayed allocation is enabled it can lead to
> > troubles. Here is one example how to reproduce an oops:
> >
> > mkfs.ext4 /dev/sdd
> > mount /dev/sdd /mnt/test1
> > dd if=/dev/zero of=/mnt/test1/file bs=1M count=4
> > chattr +j /mnt/test1/file
> > dd if=/dev/zero of=/mnt/test1/file bs=1M count=4 conv=notrunc
> > chattr -j /mnt/test1/file
> >
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
>
> Don't we also need to add a check in ext4_should_journal_data() in
> fs/ext4/ext4_jbd2.h? Otherwise we could get in trouble if the data
> journalling flag had been set previously when the file system was
> mounted nodelalloc.
Ah, of course you're right. We should add a check into
ext4_should_journal_data().
>
> In fact it could be argued that we don't need to return EOPNOSUPP, or
> printk a message, but simply make sure that the data journal flag is
> ignored in if delayed allocation is enabled. If that's true, we can
> allow the user to set or clear the journal flag as much as they like;
> it would just be ignored.
I am fine with ignoring the flag silently without any information for
the user that the flag will be ignored when delalloc is enabled. After
all it should be clear from documentation that we do not allow data
journalling and delayed allocation together.
Thanks!
-Lukas
>
> Thanks,
>
> - Ted
>
--
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-01-09 16:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-09 15:45 [PATCH] ext4: setattr: Forbid setting data journalling when delalloc enabled Lukas Czerner
2012-01-09 15:55 ` Ted Ts'o
2012-01-09 16:22 ` Lukas Czerner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox