Re: [PATCH 1/4] libext2fs: Support for orphan file feature

[Jan Kara <jack@suse.cz>]

On Fri 22-05-15 10:35:21, Darrick J. Wong wrote:
> On Fri, May 22, 2015 at 01:28:54PM +0200, Jan Kara wrote:
...
> > diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> > index 28c46701da29..1e303d5d59ca 100644
> > --- a/lib/ext2fs/ext2fs.h
> > +++ b/lib/ext2fs/ext2fs.h
> > @@ -555,7 +555,8 @@ typedef struct ext2_icount *ext2_icount_t;
> >  					 EXT2_FEATURE_COMPAT_RESIZE_INODE|\
> >  					 EXT2_FEATURE_COMPAT_DIR_INDEX|\
> >  					 EXT2_FEATURE_COMPAT_EXT_ATTR|\
> > -					 EXT4_FEATURE_COMPAT_SPARSE_SUPER2)
> > +					 EXT4_FEATURE_COMPAT_SPARSE_SUPER2|\
> > +					 EXT4_FEATURE_COMPAT_ORPHAN_FILE)
> >  
> >  #ifdef CONFIG_MMP
> >  #define EXT4_LIB_INCOMPAT_MMP		EXT4_FEATURE_INCOMPAT_MMP
> > @@ -589,7 +590,8 @@ typedef struct ext2_icount *ext2_icount_t;
> >  					 EXT4_FEATURE_RO_COMPAT_BIGALLOC|\
> >  					 EXT4_LIB_RO_COMPAT_QUOTA|\
> >  					 EXT4_FEATURE_RO_COMPAT_METADATA_CSUM|\
> > -					 EXT4_FEATURE_RO_COMPAT_READONLY)
> > +					 EXT4_FEATURE_RO_COMPAT_READONLY|\
> > +					 EXT4_FEATURE_RO_COMPAT_ORPHAN_PRESENT)
> >  
> >  /*
> >   * These features are only allowed if EXT2_FLAG_SOFTSUPP_FEATURES is passed
> > @@ -1512,6 +1514,19 @@ errcode_t ext2fs_get_data_io(ext2_filsys fs, io_channel *old_io);
> >  errcode_t ext2fs_set_data_io(ext2_filsys fs, io_channel new_io);
> >  errcode_t ext2fs_rewrite_to_io(ext2_filsys fs, io_channel new_io);
> >  
> > +/* orphan.c */
> > +/*
> > + * Minimum orphan file size (it must be at least 1 block and smaller one isn't
> > + * very useful).
> > + */
> > +#define EXT4_MIN_ORPHAN_FILE_SIZE 16384
> 
> What about 64k block size?  I guess it's fine not to use the whole block in
> this (non-default) configuration.

We round up file size to be a multiple of fs block size.

> > +struct mkorphan_info {
> > +	char *buf;
> > +	char *zerobuf;
> > +	blk_t num_blocks;
> > +	blk_t alloc_blocks;
> > +	errcode_t err;
> > +};
> > +
> > +static int mkorphan_proc(ext2_filsys	fs,
> > +			 blk64_t	*blocknr,
> > +			 e2_blkcnt_t	blockcnt,
> > +			 blk64_t	ref_block EXT2FS_ATTR((unused)),
> > +			 int		ref_offset EXT2FS_ATTR((unused)),
> > +			 void		*priv_data)
> > +{
> > +	struct mkorphan_info *oi = (struct mkorphan_info *)priv_data;
> > +	blk64_t new_blk;
> > +	errcode_t err;
> > +
> > +	err = ext2fs_new_block2(fs, 0, 0, &new_blk);
> 
> Hm.  I think this breaks the cluster allocation rules, since this allocates
> a new block for every logical block inside a cluster.

Yes, I didn't think of cluster allocation. Frankly, I have just mirrored
what a code creating journal does. If this is wrong, then that code needs
fixing as well ;)

> Hopefully ext2fs_fallocate will land soon, then we can get rid of open-coding
> file block allocation like this.  You'll still have to have the iterate3 loop
> to write out the appropriate block footer, but iirc the library call can be
> told to allocate written extents without zeroing the blocks, precisely for
> cases like these.

So should I wait for that or do you have any pointer to code which does
it correctly?

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR