diff mbox series

[V2,08/23] metadump: Introduce metadump v1 operations

Message ID 20230606092806.1604491-9-chandan.babu@oracle.com (mailing list archive)
State Superseded
Headers show
Series Metadump v2 | expand

Commit Message

Chandan Babu R June 6, 2023, 9:27 a.m. UTC
This commit moves functionality associated with writing metadump to disk into
a new function. It also renames metadump initialization, write and release
functions to reflect the fact that they work with v1 metadump files.

The metadump initialization, write and release functions are now invoked via
metadump_ops->init(), metadump_ops->write() and metadump_ops->release()
respectively.

Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
---
 db/metadump.c | 123 +++++++++++++++++++++++++-------------------------
 1 file changed, 61 insertions(+), 62 deletions(-)

Comments

Darrick J. Wong July 12, 2023, 5:10 p.m. UTC | #1
On Tue, Jun 06, 2023 at 02:57:51PM +0530, Chandan Babu R wrote:
> This commit moves functionality associated with writing metadump to disk into
> a new function. It also renames metadump initialization, write and release
> functions to reflect the fact that they work with v1 metadump files.
> 
> The metadump initialization, write and release functions are now invoked via
> metadump_ops->init(), metadump_ops->write() and metadump_ops->release()
> respectively.
> 
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> ---
>  db/metadump.c | 123 +++++++++++++++++++++++++-------------------------
>  1 file changed, 61 insertions(+), 62 deletions(-)
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index 266d3413..287e8f91 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -151,59 +151,6 @@ print_progress(const char *fmt, ...)
>  	metadump.progress_since_warning = true;
>  }
>  
> -/*
> - * A complete dump file will have a "zero" entry in the last index block,
> - * even if the dump is exactly aligned, the last index will be full of
> - * zeros. If the last index entry is non-zero, the dump is incomplete.
> - * Correspondingly, the last chunk will have a count < num_indices.
> - *
> - * Return 0 for success, -1 for failure.
> - */
> -
> -static int
> -write_index(void)
> -{
> -	struct xfs_metablock *metablock = metadump.metablock;
> -	/*
> -	 * write index block and following data blocks (streaming)
> -	 */
> -	metablock->mb_count = cpu_to_be16(metadump.cur_index);
> -	if (fwrite(metablock, (metadump.cur_index + 1) << BBSHIFT, 1,
> -			metadump.outf) != 1) {
> -		print_warning("error writing to target file");
> -		return -1;
> -	}
> -
> -	memset(metadump.block_index, 0, metadump.num_indices * sizeof(__be64));
> -	metadump.cur_index = 0;
> -	return 0;
> -}
> -
> -/*
> - * Return 0 for success, -errno for failure.
> - */
> -static int
> -write_buf_segment(
> -	char		*data,
> -	int64_t		off,
> -	int		len)
> -{
> -	int		i;
> -	int		ret;
> -
> -	for (i = 0; i < len; i++, off++, data += BBSIZE) {
> -		metadump.block_index[metadump.cur_index] = cpu_to_be64(off);
> -		memcpy(&metadump.block_buffer[metadump.cur_index << BBSHIFT],
> -			data, BBSIZE);
> -		if (++metadump.cur_index == metadump.num_indices) {
> -			ret = write_index();
> -			if (ret)
> -				return -EIO;
> -		}
> -	}
> -	return 0;
> -}
> -
>  /*
>   * we want to preserve the state of the metadata in the dump - whether it is
>   * intact or corrupt, so even if the buffer has a verifier attached to it we
> @@ -240,15 +187,16 @@ write_buf(
>  
>  	/* handle discontiguous buffers */
>  	if (!buf->bbmap) {
> -		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
> +		ret = metadump.mdops->write(buf->typ->typnm, buf->data, buf->bb,
> +				buf->blen);
>  		if (ret)
>  			return ret;
>  	} else {
>  		int	len = 0;
>  		for (i = 0; i < buf->bbmap->nmaps; i++) {
> -			ret = write_buf_segment(buf->data + BBTOB(len),
> -						buf->bbmap->b[i].bm_bn,
> -						buf->bbmap->b[i].bm_len);
> +			ret = metadump.mdops->write(buf->typ->typnm,
> +				buf->data + BBTOB(len), buf->bbmap->b[i].bm_bn,
> +				buf->bbmap->b[i].bm_len);
>  			if (ret)
>  				return ret;
>  			len += buf->bbmap->b[i].bm_len;
> @@ -3010,7 +2958,7 @@ done:
>  }
>  
>  static int
> -init_metadump(void)
> +init_metadump_v1(void)
>  {
>  	metadump.metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);
>  	if (metadump.metablock == NULL) {
> @@ -3051,12 +2999,61 @@ init_metadump(void)
>  	return 0;
>  }
>  
> +static int
> +end_write_metadump_v1(void)
> +{
> +	/*
> +	 * write index block and following data blocks (streaming)
> +	 */
> +	metadump.metablock->mb_count = cpu_to_be16(metadump.cur_index);
> +	if (fwrite(metadump.metablock, (metadump.cur_index + 1) << BBSHIFT, 1,
> +			metadump.outf) != 1) {
> +		print_warning("error writing to target file");
> +		return -1;
> +	}
> +
> +	memset(metadump.block_index, 0, metadump.num_indices * sizeof(__be64));
> +	metadump.cur_index = 0;
> +	return 0;
> +}
> +
> +static int
> +write_metadump_v1(
> +	enum typnm	type,
> +	char		*data,
> +	xfs_daddr_t	off,
> +	int		len)
> +{
> +	int		i;
> +	int		ret;
> +
> +	for (i = 0; i < len; i++, off++, data += BBSIZE) {
> +		metadump.block_index[metadump.cur_index] = cpu_to_be64(off);
> +		memcpy(&metadump.block_buffer[metadump.cur_index << BBSHIFT],
> +			data, BBSIZE);
> +		if (++metadump.cur_index == metadump.num_indices) {
> +			ret = end_write_metadump_v1();
> +			if (ret)
> +				return -EIO;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void
> -release_metadump(void)
> +release_metadump_v1(void)
>  {
>  	free(metadump.metablock);
>  }
>  
> +static struct metadump_ops metadump1_ops = {
> +	.init		= init_metadump_v1,
> +	.write		= write_metadump_v1,
> +	.end_write	= end_write_metadump_v1,
> +	.release	= release_metadump_v1,
> +};
> +
>  static int
>  metadump_f(
>  	int 		argc,
> @@ -3193,7 +3190,9 @@ metadump_f(
>  		}
>  	}
>  
> -	ret = init_metadump();
> +	metadump.mdops = &metadump1_ops;
> +
> +	ret = metadump.mdops->init();
>  	if (ret)
>  		goto out;
>  
> @@ -3216,7 +3215,7 @@ metadump_f(
>  
>  	/* write the remaining index */
>  	if (!exitcode)
> -		exitcode = write_index() < 0;
> +		exitcode = metadump.mdops->end_write() < 0;

Now that I see ->end_write in usage, I think it would be better named
->finish_dump or something like that.  It's only called once, right?

--D

>  
>  	if (metadump.progress_since_warning)
>  		fputc('\n', metadump.stdout_metadump ? stderr : stdout);
> @@ -3235,7 +3234,7 @@ metadump_f(
>  	while (iocur_sp > start_iocur_sp)
>  		pop_cur();
>  
> -	release_metadump();
> +	metadump.mdops->release();
>  
>  out:
>  	return 0;
> -- 
> 2.39.1
>
Chandan Babu R July 13, 2023, 4:36 a.m. UTC | #2
On Wed, Jul 12, 2023 at 10:10:09 AM -0700, Darrick J. Wong wrote:
> On Tue, Jun 06, 2023 at 02:57:51PM +0530, Chandan Babu R wrote:
>> This commit moves functionality associated with writing metadump to disk into
>> a new function. It also renames metadump initialization, write and release
>> functions to reflect the fact that they work with v1 metadump files.
>> 
>> The metadump initialization, write and release functions are now invoked via
>> metadump_ops->init(), metadump_ops->write() and metadump_ops->release()
>> respectively.
>> 
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>> ---
>>  db/metadump.c | 123 +++++++++++++++++++++++++-------------------------
>>  1 file changed, 61 insertions(+), 62 deletions(-)
>> 
>> diff --git a/db/metadump.c b/db/metadump.c
>> index 266d3413..287e8f91 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -151,59 +151,6 @@ print_progress(const char *fmt, ...)
>>  	metadump.progress_since_warning = true;
>>  }
>>  
>> -/*
>> - * A complete dump file will have a "zero" entry in the last index block,
>> - * even if the dump is exactly aligned, the last index will be full of
>> - * zeros. If the last index entry is non-zero, the dump is incomplete.
>> - * Correspondingly, the last chunk will have a count < num_indices.
>> - *
>> - * Return 0 for success, -1 for failure.
>> - */
>> -
>> -static int
>> -write_index(void)
>> -{
>> -	struct xfs_metablock *metablock = metadump.metablock;
>> -	/*
>> -	 * write index block and following data blocks (streaming)
>> -	 */
>> -	metablock->mb_count = cpu_to_be16(metadump.cur_index);
>> -	if (fwrite(metablock, (metadump.cur_index + 1) << BBSHIFT, 1,
>> -			metadump.outf) != 1) {
>> -		print_warning("error writing to target file");
>> -		return -1;
>> -	}
>> -
>> -	memset(metadump.block_index, 0, metadump.num_indices * sizeof(__be64));
>> -	metadump.cur_index = 0;
>> -	return 0;
>> -}
>> -
>> -/*
>> - * Return 0 for success, -errno for failure.
>> - */
>> -static int
>> -write_buf_segment(
>> -	char		*data,
>> -	int64_t		off,
>> -	int		len)
>> -{
>> -	int		i;
>> -	int		ret;
>> -
>> -	for (i = 0; i < len; i++, off++, data += BBSIZE) {
>> -		metadump.block_index[metadump.cur_index] = cpu_to_be64(off);
>> -		memcpy(&metadump.block_buffer[metadump.cur_index << BBSHIFT],
>> -			data, BBSIZE);
>> -		if (++metadump.cur_index == metadump.num_indices) {
>> -			ret = write_index();
>> -			if (ret)
>> -				return -EIO;
>> -		}
>> -	}
>> -	return 0;
>> -}
>> -
>>  /*
>>   * we want to preserve the state of the metadata in the dump - whether it is
>>   * intact or corrupt, so even if the buffer has a verifier attached to it we
>> @@ -240,15 +187,16 @@ write_buf(
>>  
>>  	/* handle discontiguous buffers */
>>  	if (!buf->bbmap) {
>> -		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
>> +		ret = metadump.mdops->write(buf->typ->typnm, buf->data, buf->bb,
>> +				buf->blen);
>>  		if (ret)
>>  			return ret;
>>  	} else {
>>  		int	len = 0;
>>  		for (i = 0; i < buf->bbmap->nmaps; i++) {
>> -			ret = write_buf_segment(buf->data + BBTOB(len),
>> -						buf->bbmap->b[i].bm_bn,
>> -						buf->bbmap->b[i].bm_len);
>> +			ret = metadump.mdops->write(buf->typ->typnm,
>> +				buf->data + BBTOB(len), buf->bbmap->b[i].bm_bn,
>> +				buf->bbmap->b[i].bm_len);
>>  			if (ret)
>>  				return ret;
>>  			len += buf->bbmap->b[i].bm_len;
>> @@ -3010,7 +2958,7 @@ done:
>>  }
>>  
>>  static int
>> -init_metadump(void)
>> +init_metadump_v1(void)
>>  {
>>  	metadump.metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);
>>  	if (metadump.metablock == NULL) {
>> @@ -3051,12 +2999,61 @@ init_metadump(void)
>>  	return 0;
>>  }
>>  
>> +static int
>> +end_write_metadump_v1(void)
>> +{
>> +	/*
>> +	 * write index block and following data blocks (streaming)
>> +	 */
>> +	metadump.metablock->mb_count = cpu_to_be16(metadump.cur_index);
>> +	if (fwrite(metadump.metablock, (metadump.cur_index + 1) << BBSHIFT, 1,
>> +			metadump.outf) != 1) {
>> +		print_warning("error writing to target file");
>> +		return -1;
>> +	}
>> +
>> +	memset(metadump.block_index, 0, metadump.num_indices * sizeof(__be64));
>> +	metadump.cur_index = 0;
>> +	return 0;
>> +}
>> +
>> +static int
>> +write_metadump_v1(
>> +	enum typnm	type,
>> +	char		*data,
>> +	xfs_daddr_t	off,
>> +	int		len)
>> +{
>> +	int		i;
>> +	int		ret;
>> +
>> +	for (i = 0; i < len; i++, off++, data += BBSIZE) {
>> +		metadump.block_index[metadump.cur_index] = cpu_to_be64(off);
>> +		memcpy(&metadump.block_buffer[metadump.cur_index << BBSHIFT],
>> +			data, BBSIZE);
>> +		if (++metadump.cur_index == metadump.num_indices) {
>> +			ret = end_write_metadump_v1();
>> +			if (ret)
>> +				return -EIO;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static void
>> -release_metadump(void)
>> +release_metadump_v1(void)
>>  {
>>  	free(metadump.metablock);
>>  }
>>  
>> +static struct metadump_ops metadump1_ops = {
>> +	.init		= init_metadump_v1,
>> +	.write		= write_metadump_v1,
>> +	.end_write	= end_write_metadump_v1,
>> +	.release	= release_metadump_v1,
>> +};
>> +
>>  static int
>>  metadump_f(
>>  	int 		argc,
>> @@ -3193,7 +3190,9 @@ metadump_f(
>>  		}
>>  	}
>>  
>> -	ret = init_metadump();
>> +	metadump.mdops = &metadump1_ops;
>> +
>> +	ret = metadump.mdops->init();
>>  	if (ret)
>>  		goto out;
>>  
>> @@ -3216,7 +3215,7 @@ metadump_f(
>>  
>>  	/* write the remaining index */
>>  	if (!exitcode)
>> -		exitcode = write_index() < 0;
>> +		exitcode = metadump.mdops->end_write() < 0;
>
> Now that I see ->end_write in usage, I think it would be better named
> ->finish_dump or something like that.  It's only called once, right?
>

finish_dump() is a much better name for the callback. I will make the change
before posting the next version of the patchset.
diff mbox series

Patch

diff --git a/db/metadump.c b/db/metadump.c
index 266d3413..287e8f91 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -151,59 +151,6 @@  print_progress(const char *fmt, ...)
 	metadump.progress_since_warning = true;
 }
 
-/*
- * A complete dump file will have a "zero" entry in the last index block,
- * even if the dump is exactly aligned, the last index will be full of
- * zeros. If the last index entry is non-zero, the dump is incomplete.
- * Correspondingly, the last chunk will have a count < num_indices.
- *
- * Return 0 for success, -1 for failure.
- */
-
-static int
-write_index(void)
-{
-	struct xfs_metablock *metablock = metadump.metablock;
-	/*
-	 * write index block and following data blocks (streaming)
-	 */
-	metablock->mb_count = cpu_to_be16(metadump.cur_index);
-	if (fwrite(metablock, (metadump.cur_index + 1) << BBSHIFT, 1,
-			metadump.outf) != 1) {
-		print_warning("error writing to target file");
-		return -1;
-	}
-
-	memset(metadump.block_index, 0, metadump.num_indices * sizeof(__be64));
-	metadump.cur_index = 0;
-	return 0;
-}
-
-/*
- * Return 0 for success, -errno for failure.
- */
-static int
-write_buf_segment(
-	char		*data,
-	int64_t		off,
-	int		len)
-{
-	int		i;
-	int		ret;
-
-	for (i = 0; i < len; i++, off++, data += BBSIZE) {
-		metadump.block_index[metadump.cur_index] = cpu_to_be64(off);
-		memcpy(&metadump.block_buffer[metadump.cur_index << BBSHIFT],
-			data, BBSIZE);
-		if (++metadump.cur_index == metadump.num_indices) {
-			ret = write_index();
-			if (ret)
-				return -EIO;
-		}
-	}
-	return 0;
-}
-
 /*
  * we want to preserve the state of the metadata in the dump - whether it is
  * intact or corrupt, so even if the buffer has a verifier attached to it we
@@ -240,15 +187,16 @@  write_buf(
 
 	/* handle discontiguous buffers */
 	if (!buf->bbmap) {
-		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
+		ret = metadump.mdops->write(buf->typ->typnm, buf->data, buf->bb,
+				buf->blen);
 		if (ret)
 			return ret;
 	} else {
 		int	len = 0;
 		for (i = 0; i < buf->bbmap->nmaps; i++) {
-			ret = write_buf_segment(buf->data + BBTOB(len),
-						buf->bbmap->b[i].bm_bn,
-						buf->bbmap->b[i].bm_len);
+			ret = metadump.mdops->write(buf->typ->typnm,
+				buf->data + BBTOB(len), buf->bbmap->b[i].bm_bn,
+				buf->bbmap->b[i].bm_len);
 			if (ret)
 				return ret;
 			len += buf->bbmap->b[i].bm_len;
@@ -3010,7 +2958,7 @@  done:
 }
 
 static int
-init_metadump(void)
+init_metadump_v1(void)
 {
 	metadump.metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);
 	if (metadump.metablock == NULL) {
@@ -3051,12 +2999,61 @@  init_metadump(void)
 	return 0;
 }
 
+static int
+end_write_metadump_v1(void)
+{
+	/*
+	 * write index block and following data blocks (streaming)
+	 */
+	metadump.metablock->mb_count = cpu_to_be16(metadump.cur_index);
+	if (fwrite(metadump.metablock, (metadump.cur_index + 1) << BBSHIFT, 1,
+			metadump.outf) != 1) {
+		print_warning("error writing to target file");
+		return -1;
+	}
+
+	memset(metadump.block_index, 0, metadump.num_indices * sizeof(__be64));
+	metadump.cur_index = 0;
+	return 0;
+}
+
+static int
+write_metadump_v1(
+	enum typnm	type,
+	char		*data,
+	xfs_daddr_t	off,
+	int		len)
+{
+	int		i;
+	int		ret;
+
+	for (i = 0; i < len; i++, off++, data += BBSIZE) {
+		metadump.block_index[metadump.cur_index] = cpu_to_be64(off);
+		memcpy(&metadump.block_buffer[metadump.cur_index << BBSHIFT],
+			data, BBSIZE);
+		if (++metadump.cur_index == metadump.num_indices) {
+			ret = end_write_metadump_v1();
+			if (ret)
+				return -EIO;
+		}
+	}
+
+	return 0;
+}
+
 static void
-release_metadump(void)
+release_metadump_v1(void)
 {
 	free(metadump.metablock);
 }
 
+static struct metadump_ops metadump1_ops = {
+	.init		= init_metadump_v1,
+	.write		= write_metadump_v1,
+	.end_write	= end_write_metadump_v1,
+	.release	= release_metadump_v1,
+};
+
 static int
 metadump_f(
 	int 		argc,
@@ -3193,7 +3190,9 @@  metadump_f(
 		}
 	}
 
-	ret = init_metadump();
+	metadump.mdops = &metadump1_ops;
+
+	ret = metadump.mdops->init();
 	if (ret)
 		goto out;
 
@@ -3216,7 +3215,7 @@  metadump_f(
 
 	/* write the remaining index */
 	if (!exitcode)
-		exitcode = write_index() < 0;
+		exitcode = metadump.mdops->end_write() < 0;
 
 	if (metadump.progress_since_warning)
 		fputc('\n', metadump.stdout_metadump ? stderr : stdout);
@@ -3235,7 +3234,7 @@  metadump_f(
 	while (iocur_sp > start_iocur_sp)
 		pop_cur();
 
-	release_metadump();
+	metadump.mdops->release();
 
 out:
 	return 0;