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

[Joel Becker]

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;
 }
 
-- 
1.5.6.5