[Ocfs2-tools-devel] [PATCH 2/2] libocfs2: Make allocating a specific cluster a clean API

[Sunil Mushran]

Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>

You may want to clean up these warnings though.

backup_super.c: In function ‘ocfs2_clear_backup_super_list’:
backup_super.c:63: warning: unused variable ‘loaded’
backup_super.c:62: warning: unused variable ‘bm_blk’
backup_super.c: In function ‘ocfs2_set_backup_super_list’:
backup_super.c:115: warning: unused variable ‘bm_blk’


Joel Becker wrote:
> The backup superblock code hand-builds the necessary operations to load
> the cluster allocator and set the backup superblock clusters.  Let's
> create clean APIs for this in libocfs2/alloc.c and have backup_super.c
> use them.
>
> While we're at it, set_backup_super() used to free the cluster allocator
> when it was done.  That doesn't fit with the library's standard "cache
> that allocator" behavior.  So don't free it.
>
> Signed-off-by: Joel Becker <joel.becker at oracle.com>
> ---
>  include/ocfs2/ocfs2.h   |    3 ++
>  libocfs2/alloc.c        |   39 +++++++++++++++++++++++++++
>  libocfs2/backup_super.c |   67 ++++++----------------------------------------
>  3 files changed, 51 insertions(+), 58 deletions(-)
>
> diff --git a/include/ocfs2/ocfs2.h b/include/ocfs2/ocfs2.h
> index 68ba4f5..e30d31d 100644
> --- a/include/ocfs2/ocfs2.h
> +++ b/include/ocfs2/ocfs2.h
> @@ -511,6 +511,9 @@ errcode_t ocfs2_new_clusters(ocfs2_filesys *fs,
>  			     uint32_t requested,
>  			     uint64_t *start_blkno,
>  			     uint32_t *clusters_found);
> +errcode_t ocfs2_test_cluster_allocated(ocfs2_filesys *fs, uint32_t cpos,
> +				       int *is_allocated);
> +errcode_t ocfs2_new_specific_cluster(ocfs2_filesys *fs, uint32_t cpos);
>  errcode_t ocfs2_free_clusters(ocfs2_filesys *fs,
>  			      uint32_t len,
>  			      uint64_t start_blkno);
> diff --git a/libocfs2/alloc.c b/libocfs2/alloc.c
> index 03b3091..2597c2f 100644
> --- a/libocfs2/alloc.c
> +++ b/libocfs2/alloc.c
> @@ -491,6 +491,45 @@ out:
>  	return ret;
>  }
>  
> +errcode_t ocfs2_test_cluster_allocated(ocfs2_filesys *fs, uint32_t cpos,
> +				       int *is_allocated)
> +{
> +	errcode_t ret;
> +	ret = ocfs2_load_allocator(fs, GLOBAL_BITMAP_SYSTEM_INODE,
> +				   0, &fs->fs_cluster_alloc);
> +	if (!ret) {
> +		ret = ocfs2_chain_test(fs, fs->fs_cluster_alloc, cpos,
> +				       is_allocated);
> +	}
> +
> +	return ret;
> +}
> +
> +errcode_t ocfs2_new_specific_cluster(ocfs2_filesys *fs, uint32_t cpos)
> +{
> +	errcode_t ret;
> +	int allocatedp = 0;
> +
> +	/* Loads the allocator if we need it */
> +	ret = ocfs2_test_cluster_allocated(fs, cpos, &allocatedp);
> +	if (ret)
> +		goto out;
> +
> +	if (allocatedp) {
> +		ret = OCFS2_ET_BIT_NOT_FOUND;
> +		goto out;
> +	}
> +
> +	ocfs2_chain_force_val(fs, fs->fs_cluster_alloc, cpos, 1, NULL);
> +	ret = ocfs2_write_chain_allocator(fs, fs->fs_cluster_alloc);
> +	if (ret)
> +		ocfs2_free_clusters(fs, 1,
> +				    ocfs2_blocks_to_clusters(fs, cpos));
> +
> +out:
> +	return ret;
> +}
> +
>  errcode_t ocfs2_free_clusters(ocfs2_filesys *fs,
>  			      uint32_t len,
>  			      uint64_t start_blkno)
> diff --git a/libocfs2/backup_super.c b/libocfs2/backup_super.c
> index 84b0449..4538b84 100644
> --- a/libocfs2/backup_super.c
> +++ b/libocfs2/backup_super.c
> @@ -76,49 +76,23 @@ errcode_t ocfs2_clear_backup_super_list(ocfs2_filesys *fs,
>  				      OCFS2_FEATURE_COMPAT_BACKUP_SB))
>  		goto bail;
>  
> -	if (!fs->fs_cluster_alloc) {
> -		loaded = 1;
> -		ret = ocfs2_lookup_system_inode(fs, GLOBAL_BITMAP_SYSTEM_INODE,
> -						0, &bm_blk);
> -		if (ret)
> -			goto bail;
> -
> -		ret = ocfs2_read_cached_inode(fs, bm_blk, &fs->fs_cluster_alloc);
> -		if (ret)
> -			goto bail;
> -
> -		ret = ocfs2_load_chain_allocator(fs, fs->fs_cluster_alloc);
> -		if (ret)
> -			goto bail;
> -	}
>  
>  	for (i = 0; i < len; i++) {
> -		ret = ocfs2_chain_free(fs,
> -				       fs->fs_cluster_alloc,
> -				       ocfs2_blocks_to_clusters(fs,
> -								blocks[i]));
> -		/* Ignore the 'bit was already free' error */
> -		if (ret &&
> -		    (ret != OCFS2_ET_FREEING_UNALLOCATED_REGION))
> -			goto bail;
> +		ret = ocfs2_free_clusters(fs, 1, blocks[i]);
> +		if (ret)
> +			break;
>  	}
>  
> -	ret = ocfs2_write_chain_allocator(fs, fs->fs_cluster_alloc);
> -
>  bail:
> -	if (fs->fs_cluster_alloc && loaded) {
> -		ocfs2_free_cached_inode(fs, fs->fs_cluster_alloc);
> -		fs->fs_cluster_alloc = NULL;
> -	}
>  	return ret;
>  }
>  
> -static errcode_t check_cluster(ocfs2_bitmap *bitmap, uint64_t bit)
> +static errcode_t check_cluster(ocfs2_filesys *fs, uint32_t cpos)
>  {
>  	errcode_t ret;
>  	int val;
>  
> -	ret = ocfs2_bitmap_test(bitmap, bit, &val);
> +	ret = ocfs2_test_cluster_allocated(fs, cpos, &val);
>  	if (ret)
>  		goto bail;
>  
> @@ -139,35 +113,18 @@ errcode_t ocfs2_set_backup_super_list(ocfs2_filesys *fs,
>  	errcode_t ret = 0;
>  	char *buf = NULL;
>  	uint64_t bm_blk, *blkno = blocks;
> -	int val, loaded = 0;
>  	uint32_t cluster, bpc = fs->fs_clustersize / fs->fs_blocksize;
>  
>  	if (!len || !blocks || !*blocks)
>  		goto bail;
>  	len = ocfs2_min(len,(size_t)OCFS2_MAX_BACKUP_SUPERBLOCKS);
>  
> -	if (!fs->fs_cluster_alloc) {
> -		loaded = 1;
> -		ret = ocfs2_lookup_system_inode(fs, GLOBAL_BITMAP_SYSTEM_INODE,
> -						0, &bm_blk);
> -		if (ret)
> -			goto bail;
> -
> -		ret = ocfs2_read_cached_inode(fs, bm_blk, &fs->fs_cluster_alloc);
> -		if (ret)
> -			goto bail;
> -
> -		ret = ocfs2_load_chain_allocator(fs, fs->fs_cluster_alloc);
> -		if (ret)
> -			goto bail;
> -	}
> -
>  	if (!OCFS2_HAS_COMPAT_FEATURE(OCFS2_RAW_SB(fs->fs_super),
>  				      OCFS2_FEATURE_COMPAT_BACKUP_SB)) {
>  		/* check all the blkno to see whether it is used. */
>  		for (i = 0; i < len; i++, blkno++) {
> -			ret = check_cluster(fs->fs_cluster_alloc->ci_chains,
> -					ocfs2_blocks_to_clusters(fs, *blkno));
> +			ret = check_cluster(fs,
> +					    ocfs2_blocks_to_clusters(fs, *blkno));
>  			if (ret)
>  				goto bail;
>  		}
> @@ -191,20 +148,14 @@ errcode_t ocfs2_set_backup_super_list(ocfs2_filesys *fs,
>  	if (ret)
>  		goto bail;
>  
> +	/* We just tested the clusters, so the allocation can't fail */
>  	blkno = blocks;
>  	for (i = 0; i < len; i++, blkno++)
> -		ocfs2_bitmap_set(fs->fs_cluster_alloc->ci_chains,
> -				 ocfs2_blocks_to_clusters(fs, *blkno), &val);
> -
> -	ret = ocfs2_write_chain_allocator(fs, fs->fs_cluster_alloc);
> +		ocfs2_new_specific_cluster(fs, ocfs2_blocks_to_clusters(fs, *blkno));
>  
>  bail:
>  	if (buf)
>  		ocfs2_free(&buf);
> -	if (fs->fs_cluster_alloc && loaded) {
> -		ocfs2_free_cached_inode(fs, fs->fs_cluster_alloc);
> -		fs->fs_cluster_alloc = NULL;
> -	}
>  	return ret;
>  }
>  
>