Re: [PATCH RFC 01/30] ext4: EXT4 snapshots (Experimental)

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

 



On Mon, 9 May 2011, amir73il@xxxxxxxxxxxxxxxxxxxxx wrote:

> From: Amir Goldstein <amir73il@xxxxxxxxxxxx>
> 
> Built-in snapshots support for ext4.
> Requires that the filesystem has the has_snapshot and exclude_bitmap
> features and that block size is equal to system page size.
> Snapshots are not supported with 64bit and meta_bg features and the
> filesystem must be mounted with ordered data mode.
> 
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxxxxx>
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
> ---
>  fs/ext4/Kconfig          |   11 +++
>  fs/ext4/Makefile         |    2 +
>  fs/ext4/balloc.c         |    1 +
>  fs/ext4/ext4.h           |   15 ++++
>  fs/ext4/ext4_jbd2.c      |    3 +
>  fs/ext4/ext4_jbd2.h      |   25 ++++++
>  fs/ext4/extents.c        |    3 +
>  fs/ext4/file.c           |    1 +
>  fs/ext4/ialloc.c         |    1 +
>  fs/ext4/inode.c          |    3 +
>  fs/ext4/ioctl.c          |    3 +
>  fs/ext4/mballoc.c        |    1 +
>  fs/ext4/namei.c          |    1 +
>  fs/ext4/resize.c         |    1 +
>  fs/ext4/snapshot.h       |  193 ++++++++++++++++++++++++++++++++++++++++++++++
>  fs/ext4/super.c          |   43 ++++++++++
>  16 files changed, 307 insertions(+), 0 deletions(-)
>  create mode 100644 fs/ext4/snapshot.h
>  create mode 100644 fs/ext4/snapshot_debug.h
> 
> diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
> index 9ed1bb1..8970525 100644
> --- a/fs/ext4/Kconfig
> +++ b/fs/ext4/Kconfig
> @@ -83,3 +83,14 @@ config EXT4_DEBUG
>  
>  	  If you select Y here, then you will be able to turn on debugging
>  	  with a command such as "echo 1 > /sys/kernel/debug/ext4/mballoc-debug"
> +
> +config EXT4_FS_SNAPSHOT
> +	bool "EXT4 snapshots (Experimental)"
> +	depends on EXT4_FS && EXPERIMENTAL
> +	default n
> +	help
> +	  Built-in snapshots support for ext4.
> +	  Requires that the filesystem has the has_snapshot and exclude_bitmap
> +	  features and that block size is equal to system page size.
> +	  Snapshots are not supported with 64bit and meta_bg features and the
> +	  filesystem must be mounted with ordered data mode.

What exactly do you mean by not supported with 64bit feature ? Maybe I
am being dense, but I do not get it.

> diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
> index 058b54d..16a779d 100644
> --- a/fs/ext4/Makefile
> +++ b/fs/ext4/Makefile
> @@ -19,3 +19,5 @@ ext4-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o page-io.o \
>  ext4-$(CONFIG_EXT4_FS_XATTR)		+= xattr.o xattr_user.o xattr_trusted.o
>  ext4-$(CONFIG_EXT4_FS_POSIX_ACL)	+= acl.o
>  ext4-$(CONFIG_EXT4_FS_SECURITY)		+= xattr_security.o
> +ext4-$(CONFIG_EXT4_FS_SNAPSHOT)		+= snapshot.o snapshot_ctl.o
> +ext4-$(CONFIG_EXT4_FS_SNAPSHOT)		+= snapshot_inode.o snapshot_buffer.o
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index 1288f80..350f502 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -20,6 +20,7 @@
>  #include "ext4.h"
>  #include "ext4_jbd2.h"
>  #include "mballoc.h"
> +#include "snapshot.h"
>  
>  /*
>   * balloc.c contains the blocks allocation and deallocation routines
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index f495b22..ca25e67 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -886,6 +886,20 @@ struct ext4_inode_info {
>  #define EXT2_FLAGS_SIGNED_HASH		0x0001  /* Signed dirhash in use */
>  #define EXT2_FLAGS_UNSIGNED_HASH	0x0002  /* Unsigned dirhash in use */
>  #define EXT2_FLAGS_TEST_FILESYS		0x0004	/* to test development code */
> +#define EXT4_FLAGS_IS_SNAPSHOT		0x0010 /* Is a snapshot image */
> +#define EXT4_FLAGS_FIX_SNAPSHOT		0x0020 /* Corrupted snapshot */
> +#define EXT4_FLAGS_FIX_EXCLUDE		0x0040 /* Bad exclude bitmap */

Would not it be better to call it EXT4_FLAGS_BAD_SNAPSHOT and
EXT4_FLAGS_BAD_EXCLUDE_BMAP ?

> +
> +#define EXT4_SET_FLAGS(sb, mask)				 \
> +	do {							 \
> +		EXT4_SB(sb)->s_es->s_flags |= cpu_to_le32(mask); \
> +	} while (0)
> +#define EXT4_CLEAR_FLAGS(sb, mask)				 \
> +	do {							 \
> +		EXT4_SB(sb)->s_es->s_flags &= ~cpu_to_le32(mask);\
> +	} while (0)
> +#define EXT4_TEST_FLAGS(sb, mask)				 \
> +	(EXT4_SB(sb)->s_es->s_flags & cpu_to_le32(mask))
>  
>  /*
>   * Mount flags
> @@ -1351,6 +1365,7 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
>  #define EXT4_FEATURE_RO_COMPAT_GDT_CSUM		0x0010
>  #define EXT4_FEATURE_RO_COMPAT_DIR_NLINK	0x0020
>  #define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE	0x0040
> +#define EXT4_FEATURE_RO_COMPAT_HAS_SNAPSHOT	0x0080 /* Ext4 has snapshots */
>  
>  #define EXT4_FEATURE_INCOMPAT_COMPRESSION	0x0001
>  #define EXT4_FEATURE_INCOMPAT_FILETYPE		0x0002
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index 6e272ef..560020d 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -1,8 +1,11 @@
>  /*
>   * Interface between ext4 and JBD
> + *
> + * Snapshot metadata COW hooks, Amir Goldstein <amir73il@xxxxxxxxxxxx>, 2011
>   */
>  
>  #include "ext4_jbd2.h"
> +#include "snapshot.h"
>  
>  #include <trace/events/ext4.h>
>  
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index e25e99b..8ffffb1 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -10,6 +10,8 @@
>   * option, any later version, incorporated herein by reference.
>   *
>   * Ext4-specific journaling extensions.
> + *
> + * Snapshot extra COW credits, Amir Goldstein <amir73il@xxxxxxxxxxxx>, 2011
>   */
>  
>  #ifndef _EXT4_JBD2_H
> @@ -18,6 +20,7 @@
>  #include <linux/fs.h>
>  #include <linux/jbd2.h>
>  #include "ext4.h"
> +#include "snapshot.h"
>  
>  #define EXT4_JOURNAL(inode)	(EXT4_SB((inode)->i_sb)->s_journal)
>  
> @@ -272,6 +275,11 @@ static inline int ext4_should_journal_data(struct inode *inode)
>  		return 0;
>  	if (!S_ISREG(inode->i_mode))
>  		return 1;
> +#ifdef CONFIG_EXT4_FS_SNAPSHOT
> +	if (EXT4_SNAPSHOTS(inode->i_sb))
> +		/* snapshots enforce ordered data */
> +		return 0;
> +#endif
>  	if (test_opt(inode->i_sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA)
>  		return 1;
>  	if (ext4_test_inode_flag(inode, EXT4_INODE_JOURNAL_DATA))
> @@ -285,6 +293,11 @@ static inline int ext4_should_order_data(struct inode *inode)
>  		return 0;
>  	if (!S_ISREG(inode->i_mode))
>  		return 0;
> +#ifdef CONFIG_EXT4_FS_SNAPSHOT
> +	if (EXT4_SNAPSHOTS(inode->i_sb))
> +		/* snapshots enforce ordered data */
> +		return 1;
> +#endif
>  	if (ext4_test_inode_flag(inode, EXT4_INODE_JOURNAL_DATA))
>  		return 0;
>  	if (test_opt(inode->i_sb, DATA_FLAGS) == EXT4_MOUNT_ORDERED_DATA)
> @@ -298,6 +311,11 @@ static inline int ext4_should_writeback_data(struct inode *inode)
>  		return 0;
>  	if (EXT4_JOURNAL(inode) == NULL)
>  		return 1;
> +#ifdef CONFIG_EXT4_FS_SNAPSHOT
> +	if (EXT4_SNAPSHOTS(inode->i_sb))
> +		/* snapshots enforce ordered data */
> +		return 0;
> +#endif
>  	if (ext4_test_inode_flag(inode, EXT4_INODE_JOURNAL_DATA))
>  		return 0;
>  	if (test_opt(inode->i_sb, DATA_FLAGS) == EXT4_MOUNT_WRITEBACK_DATA)
> @@ -320,6 +338,11 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
>  		return 0;
>  	if (!S_ISREG(inode->i_mode))
>  		return 0;
> +#ifdef CONFIG_EXT4_FS_SNAPSHOT
> +	if (EXT4_SNAPSHOTS(inode->i_sb))
> +		/* XXX: should snapshots support dioread_nolock? */
> +		return 0;
> +#endif
>  	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
>  		return 0;
>  	if (ext4_should_journal_data(inode))
> @@ -327,4 +350,6 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
>  	return 1;
>  }

Since EXT4_SNAPSHOTS(sb) returns 0 when not configured in, I do not
think those ifdefs are needed.

>  
> +#ifdef CONFIG_EXT4_FS_SNAPSHOT
> +#endif
>  #endif	/* _EXT4_JBD2_H */
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 7296cd1..0c3ea93 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -18,6 +18,8 @@
>   * You should have received a copy of the GNU General Public Licens
>   * along with this program; if not, write to the Free Software
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-
> + *
> + * Snapshot move-on-write (MOW), Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>, 2011
>   */
>  
>  /*
> @@ -43,6 +45,7 @@
>  #include <linux/fiemap.h>
>  #include "ext4_jbd2.h"
>  #include "ext4_extents.h"
> +#include "snapshot.h"
>  
>  static int ext4_ext_truncate_extend_restart(handle_t *handle,
>  					    struct inode *inode,
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 7b80d54..60b3b19 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -28,6 +28,7 @@
>  #include "ext4_jbd2.h"
>  #include "xattr.h"
>  #include "acl.h"
> +#include "snapshot.h"
>  
>  /*
>   * Called when an inode is released. Note that this is different
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 2fd3b0e..831d49a 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -28,6 +28,7 @@
>  #include "ext4_jbd2.h"
>  #include "xattr.h"
>  #include "acl.h"
> +#include "snapshot.h"
>  
>  #include <trace/events/ext4.h>
>  
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 4ccb6eb..a597ff1 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -20,6 +20,8 @@
>   *	(jj@xxxxxxxxxxxxxxxxxxxxxx)
>   *
>   *  Assorted race fixes, rewrite of ext4_get_block() by Al Viro, 2000
> + *
> + *  Snapshot inode extensions, Amir Goldstein <amir73il@xxxxxxxxxxxx>, 2011
>   */
>  
>  #include <linux/module.h>
> @@ -49,6 +51,7 @@
>  #include "ext4_extents.h"
>  
>  #include <trace/events/ext4.h>
> +#include "snapshot.h"
>  
>  #define MPAGE_DA_EXTENT_TAIL 0x01
>  
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index eb3bc2f..a426332 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -5,6 +5,8 @@
>   * Remy Card (card@xxxxxxxxxxx)
>   * Laboratoire MASI - Institut Blaise Pascal
>   * Universite Pierre et Marie Curie (Paris VI)
> + *
> + * Snapshot control API, Amir Goldstein <amir73il@xxxxxxxxxxxx>, 2011
>   */
>  
>  #include <linux/fs.h>
> @@ -17,6 +19,7 @@
>  #include <asm/uaccess.h>
>  #include "ext4_jbd2.h"
>  #include "ext4.h"
> +#include "snapshot.h"
>  
>  long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 2be85af..4952b7b 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -25,6 +25,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/slab.h>
>  #include <trace/events/ext4.h>
> +#include "snapshot.h"
>  
>  /*
>   * MUSTDO:
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index ad87584..b70fa13 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -39,6 +39,7 @@
>  
>  #include "xattr.h"
>  #include "acl.h"
> +#include "snapshot.h"
>  
>  /*
>   * define how far ahead to read directories while searching them.
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index a11c00a..ee9b999 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -15,6 +15,7 @@
>  #include <linux/slab.h>
>  
>  #include "ext4_jbd2.h"
> +#include "snapshot.h"

It would be better for reviewers if you'll add those includes when you
start using them.

>  
>  #define outside(b, first, last)	((b) < (first) || (b) >= (last))
>  #define inside(b, first, last)	((b) >= (first) && (b) < (last))
> diff --git a/fs/ext4/snapshot.h b/fs/ext4/snapshot.h
> new file mode 100644
> index 0000000..a927090
> --- /dev/null
> +++ b/fs/ext4/snapshot.h
> @@ -0,0 +1,193 @@
> +/*
> + * linux/fs/ext4/snapshot.h
> + *
> + * Written by Amir Goldstein <amir73il@xxxxxxxxxxxx>, 2008
> + *
> + * Copyright (C) 2008-2011 CTERA Networks
> + *
> + * This file is part of the Linux kernel and is made available under
> + * the terms of the GNU General Public License, version 2, or at your
> + * option, any later version, incorporated herein by reference.
> + *
> + * Ext4 snapshot extensions.

This is great place to write more about snapshot design and
implementation. If it is added later in a different file, then ignore it
:).

> + */
> +
> +#ifndef _LINUX_EXT4_SNAPSHOT_H
> +#define _LINUX_EXT4_SNAPSHOT_H
> +
> +#include <linux/version.h>
> +#include <linux/delay.h>
> +#include "ext4.h"
> +
> +
> +/*
> + * use signed 64bit for snapshot image addresses
> + * negative addresses are used to reference snapshot meta blocks
> + */
> +#define ext4_snapblk_t long long

typedef signed long long int ext4_snapblk_t maybe ?

> +
> +/*
> + * We assert that file system block size == page size (on mount time)
> + * and that the first file system block is block 0 (on snapshot create).
> + * Snapshot inode direct blocks are reserved for snapshot meta blocks.
> + * Snapshot inode single indirect blocks are not used.
> + * Snapshot image starts at the first double indirect block, so all blocks in
> + * Snapshot image block group blocks are mapped by a single DIND block:
> + * 4k: 32k blocks_per_group = 32 IND (4k) blocks = 32 groups per DIND
> + * 8k: 64k blocks_per_group = 32 IND (8k) blocks = 64 groups per DIND
> + * 16k: 128k blocks_per_group = 32 IND (16k) blocks = 128 groups per DIND
> + */
> +#define SNAPSHOT_BLOCK_SIZE		PAGE_SIZE
> +#define SNAPSHOT_BLOCK_SIZE_BITS	PAGE_SHIFT
> +#define	SNAPSHOT_ADDR_PER_BLOCK		(SNAPSHOT_BLOCK_SIZE / sizeof(__u32))

> +#define SNAPSHOT_ADDR_PER_BLOCK_BITS	(SNAPSHOT_BLOCK_SIZE_BITS - 2)

#define	SNAPSHOT_ADDR_PER_BLOCK		(1 << SNAPSHOT_BLOCK_SIZE_BITS )

> +#define SNAPSHOT_DIR_BLOCKS		EXT4_NDIR_BLOCKS
> +#define SNAPSHOT_IND_BLOCKS		SNAPSHOT_ADDR_PER_BLOCK
> +
> +#define SNAPSHOT_BLOCKS_PER_GROUP_BITS	(SNAPSHOT_BLOCK_SIZE_BITS + 3)
> +#define SNAPSHOT_BLOCKS_PER_GROUP				\
> +	(1<<SNAPSHOT_BLOCKS_PER_GROUP_BITS) /* 8*PAGE_SIZE */

> +#define SNAPSHOT_BLOCK_GROUP(block)				\
> +	((block)>>SNAPSHOT_BLOCKS_PER_GROUP_BITS)
> +#define SNAPSHOT_BLOCK_GROUP_OFFSET(block)			\
> +	((block)&(SNAPSHOT_BLOCKS_PER_GROUP-1))

formating is wrong.

> +#define SNAPSHOT_BLOCK_TUPLE(block)				\
> +	(ext4_fsblk_t)SNAPSHOT_BLOCK_GROUP_OFFSET(block),	\
> +	(ext4_fsblk_t)SNAPSHOT_BLOCK_GROUP(block)

This is confusing, but is you're using it really often, so be it.

> +#define SNAPSHOT_IND_PER_BLOCK_GROUP_BITS			\
> +	(SNAPSHOT_BLOCKS_PER_GROUP_BITS-SNAPSHOT_ADDR_PER_BLOCK_BITS)
> +#define SNAPSHOT_IND_PER_BLOCK_GROUP				\
> +	(1<<SNAPSHOT_IND_PER_BLOCK_GROUP_BITS) /* 32 */
> +#define SNAPSHOT_DIND_BLOCK_GROUPS_BITS				\
> +	(SNAPSHOT_ADDR_PER_BLOCK_BITS-SNAPSHOT_IND_PER_BLOCK_GROUP_BITS)
> +#define SNAPSHOT_DIND_BLOCK_GROUPS				\
> +	(1<<SNAPSHOT_DIND_BLOCK_GROUPS_BITS)

formating

> +
> +#define SNAPSHOT_BLOCK_OFFSET					\
> +	(SNAPSHOT_DIR_BLOCKS+SNAPSHOT_IND_BLOCKS)
> +#define SNAPSHOT_BLOCK(iblock)					\
> +	((ext4_snapblk_t)(iblock) - SNAPSHOT_BLOCK_OFFSET)
> +#define SNAPSHOT_IBLOCK(block)					\
> +	(ext4_fsblk_t)((block) + SNAPSHOT_BLOCK_OFFSET)

I do not see SNAPSHOT_BLOCK() defined anywhere.

> +
> +
> +
> +#ifdef CONFIG_EXT4_FS_SNAPSHOT
> +#define EXT4_SNAPSHOT_VERSION "ext4 snapshot v1.0.13-6 (2-May-2010)"
> +
> +#define SNAPSHOT_BYTES_OFFSET					\
> +	(SNAPSHOT_BLOCK_OFFSET << SNAPSHOT_BLOCK_SIZE_BITS)
> +#define SNAPSHOT_ISIZE(size)					\
> +	((size) + SNAPSHOT_BYTES_OFFSET)
> +/* Snapshot block device size is recorded in i_disksize */
> +#define SNAPSHOT_SET_SIZE(inode, size)				\
> +	(EXT4_I(inode)->i_disksize = SNAPSHOT_ISIZE(size))

I do not have a clue what that means. And to be honest I am getting a
bit lost in macros, could you add some comments explaining what it is
used for ?

> +#define SNAPSHOT_SIZE(inode)					\
> +	(EXT4_I(inode)->i_disksize - SNAPSHOT_BYTES_OFFSET)
> +#define SNAPSHOT_SET_BLOCKS(inode, blocks)			\
> +	SNAPSHOT_SET_SIZE((inode),				\
> +			(loff_t)(blocks) << SNAPSHOT_BLOCK_SIZE_BITS)
> +#define SNAPSHOT_BLOCKS(inode)					\
> +	(ext4_fsblk_t)(SNAPSHOT_SIZE(inode) >> SNAPSHOT_BLOCK_SIZE_BITS)
> +/* Snapshot shrink/merge/clean progress is exported via i_size */
> +#define SNAPSHOT_PROGRESS(inode)				\
> +	(ext4_fsblk_t)((inode)->i_size >> SNAPSHOT_BLOCK_SIZE_BITS)
> +#define SNAPSHOT_SET_ENABLED(inode)				\
> +	i_size_write((inode), SNAPSHOT_SIZE(inode))
> +#define SNAPSHOT_SET_PROGRESS(inode, blocks)			\
> +	snapshot_size_extend((inode), (blocks))
> +/* Disabled/deleted snapshot i_size is 1 block, to allow read of super block */
> +#define SNAPSHOT_SET_DISABLED(inode)				\
> +	snapshot_size_truncate((inode), 1)
> +/* Removed snapshot i_size and i_disksize are 0, since all blocks were freed */
> +#define SNAPSHOT_SET_REMOVED(inode)				\
> +	do {							\
> +		EXT4_I(inode)->i_disksize = 0;			\
> +		snapshot_size_truncate((inode), 0);		\
> +	} while (0)
> +
> +static inline void snapshot_size_extend(struct inode *inode,
> +			ext4_fsblk_t blocks)
> +{
> +	i_size_write((inode), (loff_t)(blocks) << SNAPSHOT_BLOCK_SIZE_BITS);
> +}
> +
> +static inline void snapshot_size_truncate(struct inode *inode,
> +			ext4_fsblk_t blocks)
> +{
> +	loff_t i_size = (loff_t)blocks << SNAPSHOT_BLOCK_SIZE_BITS;
> +
> +	i_size_write(inode, i_size);
> +	truncate_inode_pages(&inode->i_data, i_size);
> +}
> +
> +/* Is ext4 configured for snapshots support? */
> +static inline int EXT4_SNAPSHOTS(struct super_block *sb)
> +{
> +	return EXT4_HAS_RO_COMPAT_FEATURE(sb,
> +			EXT4_FEATURE_RO_COMPAT_HAS_SNAPSHOT);
> +}
> +
> +#define ext4_snapshot_cow(handle, inode, block, bh, cow) 0
> +
> +#define ext4_snapshot_move(handle, inode, block, pcount, move) (0)
> +
> +/*
> + * Block access functions
> + */
> +
> +
> +
> +/* snapshot_ctl.c */
> +
> +
> +static inline int init_ext4_snapshot(void)
> +{
> +	return 0;
> +}
> +
> +static inline void exit_ext4_snapshot(void)
> +{
> +}
> +
> +
> +
> +
> +
> +#else /* CONFIG_EXT4_FS_SNAPSHOT */
> +
> +/* Snapshot NOP macros */
> +#define EXT4_SNAPSHOTS(sb) (0)
> +#define SNAPMAP_ISCOW(cmd)	(0)
> +#define SNAPMAP_ISMOVE(cmd)     (0)
> +#define SNAPMAP_ISSYNC(cmd)	(0)
> +#define IS_COWING(handle)	(0)
> +
> +#define ext4_snapshot_load(sb, es, ro) (0)
> +#define ext4_snapshot_destroy(sb)
> +#define init_ext4_snapshot() (0)
> +#define exit_ext4_snapshot()
> +#define ext4_snapshot_active(sbi) (0)
> +#define ext4_snapshot_file(inode) (0)
> +#define ext4_snapshot_should_move_data(inode) (0)
> +#define ext4_snapshot_test_excluded(handle, inode, block_to_free, count) (0)
> +#define ext4_snapshot_list(inode) (0)
> +#define ext4_snapshot_get_flags(ei, filp)
> +#define ext4_snapshot_set_flags(handle, inode, flags) (0)
> +#define ext4_snapshot_take(inode) (0)
> +#define ext4_snapshot_update(inode_i_sb, cleanup, zero) (0)
> +#define ext4_snapshot_has_active(sb) (NULL)
> +#define ext4_snapshot_get_bitmap_access(handle, sb, grp, bh) (0)
> +#define ext4_snapshot_get_write_access(handle, inode, bh) (0)
> +#define ext4_snapshot_get_create_access(handle, bh) (0)
> +#define ext4_snapshot_excluded(ac_inode) (0)
> +#define ext4_snapshot_get_delete_access(handle, inode, block, pcount) (0)
> +
> +#define ext4_snapshot_get_move_access(handle, inode, block, pcount, move) (0)
> +#define ext4_snapshot_start_pending_cow(sbh)
> +#define ext4_snapshot_end_pending_cow(sbh)
> +#define ext4_snapshot_is_active(inode)		(0)
> +#define ext4_snapshot_mow_in_tid(inode)		(1)
> +
> +#endif /* CONFIG_EXT4_FS_SNAPSHOT */
> +#endif	/* _LINUX_EXT4_SNAPSHOT_H */
> diff --git a/fs/ext4/snapshot_debug.h b/fs/ext4/snapshot_debug.h
> new file mode 100644
> index 0000000..e69de29
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 414167a..2c345d1 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -48,6 +48,7 @@
>  #include "xattr.h"
>  #include "acl.h"
>  #include "mballoc.h"
> +#include "snapshot.h"
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/ext4.h>
> @@ -2612,6 +2613,24 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
>  			return 0;
>  		}
>  	}
> +	/* Enforce snapshots requirements: */
> +	if (EXT4_SNAPSHOTS(sb)) {
> +		if (EXT4_HAS_INCOMPAT_FEATURE(sb,
> +					EXT4_FEATURE_INCOMPAT_META_BG|
> +					EXT4_FEATURE_INCOMPAT_64BIT)) {
> +			ext4_msg(sb, KERN_ERR,
> +				"has_snapshot feature cannot be mixed with "
> +				"features: meta_bg, 64bit");
> +			return 0;

So what if the fs is mounted as readonly and has those incompatible features
? Is it ok ?

> +		}
> +		if (EXT4_TEST_FLAGS(sb, EXT4_FLAGS_IS_SNAPSHOT)) {
> +			ext4_msg(sb, KERN_ERR,
> +				"A snapshot image must be mounted read-only. "
> +				"If this is an exported snapshot image, you "
> +				"must run fsck -xy to make it writable.");
> +			return 0;
> +		}
> +	}
>  	return 1;
>  }
>  
> @@ -3194,6 +3213,15 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>  
>  	blocksize = BLOCK_SIZE << le32_to_cpu(es->s_log_block_size);
>  
> +	/* Enforce snapshots blocksize == pagesize */
> +	if (EXT4_SNAPSHOTS(sb) && blocksize != PAGE_SIZE) {
> +		ext4_msg(sb, KERN_ERR,
> +				"snapshots require that filesystem blocksize "
> +				"(%d) be equal to system page size (%lu)",
> +				blocksize, PAGE_SIZE);
> +		goto failed_mount;
> +	}

I would rather see this test after the test for supported filesystem
blocksize. It is only logical to check for the superset first.

> +
>  	if (blocksize < EXT4_MIN_BLOCK_SIZE ||
>  	    blocksize > EXT4_MAX_BLOCK_SIZE) {
>  		ext4_msg(sb, KERN_ERR,
> @@ -3540,6 +3568,15 @@ no_journal:
>  		goto failed_mount_wq;
>  	}
>  
> +	/* Enforce journal ordered mode with snapshots */
> +	if (EXT4_SNAPSHOTS(sb) && !(sb->s_flags & MS_RDONLY) &&
> +		(!EXT4_SB(sb)->s_journal ||
> +		 test_opt(sb, DATA_FLAGS) != EXT4_MOUNT_ORDERED_DATA)) {
> +		ext4_msg(sb, KERN_ERR,
> +				"snapshots require journal ordered mode");
> +		goto failed_mount4;
> +	}
> +
>  	/*
>  	 * The jbd2_journal_load will have done any necessary log recovery,
>  	 * so we can safely mount the rest of the filesystem now.
> @@ -4878,10 +4915,15 @@ static int __init ext4_init_fs(void)
>  	err = register_filesystem(&ext4_fs_type);
>  	if (err)
>  		goto out;
> +	err = init_ext4_snapshot();
> +	if (err)
> +		goto out_fs;

Is it really necessary to init snapshots after the filesystem
registration ? I do not see reason why.

>  
>  	ext4_li_info = NULL;
>  	mutex_init(&ext4_li_mtx);
>  	return 0;
> +out_fs:
> +	unregister_filesystem(&ext4_fs_type);
>  out:
>  	unregister_as_ext2();
>  	unregister_as_ext3();
> @@ -4905,6 +4947,7 @@ out7:
>  
>  static void __exit ext4_exit_fs(void)
>  {
> +	exit_ext4_snapshot();
>  	ext4_destroy_lazyinit_thread();
>  	unregister_as_ext2();
>  	unregister_as_ext3();
> 

Thanks!
-Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux