diff mbox series

[09/24] metadump: Introduce metadump v1 operations

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

Commit Message

Chandan Babu R May 23, 2023, 9 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(), metadump_ops->write_metadump() and
metadump_ops->release_metadump() respectively.

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

Comments

Darrick J. Wong May 23, 2023, 5:25 p.m. UTC | #1
On Tue, May 23, 2023 at 02:30:35PM +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(), metadump_ops->write_metadump() and
> metadump_ops->release_metadump() respectively.
> 
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> ---
>  db/metadump.c | 124 +++++++++++++++++++++++++-------------------------
>  1 file changed, 61 insertions(+), 63 deletions(-)
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index 56d8c3bdf..7265f73ec 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -135,59 +135,6 @@ print_progress(const char *fmt, ...)
>  	metadump.progress_since_warning = 1;
>  }
>  
> -/*
> - * 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
> @@ -224,15 +171,16 @@ write_buf(
>  
>  	/* handle discontiguous buffers */
>  	if (!buf->bbmap) {
> -		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
> +		ret = metadump.mdops->write_metadump(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_metadump(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;
> @@ -2994,7 +2942,7 @@ done:
>  }
>  
>  static int
> -init_metadump(void)
> +init_metadump_v1(void)
>  {
>  	metadump.metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);
>  	if (metadump.metablock == NULL) {
> @@ -3035,12 +2983,60 @@ 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,
> +	int64_t		off,

This really ought to be an xfs_daddr_t, right?

> +	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);

Wondering if this ought to be called ->record_segment or something, since
it's not really writing anything to disk, merely copying it to the index
buffer.

> +		if (++metadump.cur_index == metadump.num_indices) {
> +			ret = end_write_metadump_v1();
> +			if (ret)
> +				return -EIO;

This is "generic" code for "Have we filled up the index table?  If so,
then write the index block the indexed data".  Shouldn't it go in
write_buf?  And then write_buf does something like:

	while (len > 0) {
		segment_len = min(len, metadump.num_indices - metadump.cur_index);

		metadump.ops->record_segment(type, buf, daddr, segment_len);

		metadump.cur_index += segment_len;
		if (metadump.cur_index == metadump.num_indices) {
			metadump.ops->write_index(...);
			metadump.cur_index = 0;
		}

		len -= segment_len;
		daddr += segment_len;
		buf += (segment_len << 9);
	}

	if (metadump.cur_index)
		metadump.ops->write_index(...);
	metadump.cur_index = 0;

--D

> +		}
> +	}
> +
> +        return 0;
> +}
> +
>  static void
> -release_metadump(void)
> +release_metadump_v1(void)
>  {
>  	free(metadump.metablock);
>  }
>  
> +static struct metadump_ops metadump1_ops = {
> +	.init_metadump = init_metadump_v1,
> +	.write_metadump = write_metadump_v1,
> +	.end_write_metadump = end_write_metadump_v1,
> +	.release_metadump = release_metadump_v1,
> +};
> +
>  static int
>  metadump_f(
>  	int 		argc,
> @@ -3182,9 +3178,11 @@ metadump_f(
>  		}
>  	}
>  
> -	ret = init_metadump();
> +	metadump.mdops = &metadump1_ops;
> +
> +	ret = metadump.mdops->init_metadump();
>  	if (ret)
> -		return 0;
> +		goto out;
>  
>  	exitcode = 0;
>  
> @@ -3206,7 +3204,7 @@ metadump_f(
>  
>  	/* write the remaining index */
>  	if (!exitcode)
> -		exitcode = write_index() < 0;
> +		exitcode = metadump.mdops->end_write_metadump() < 0;
>  
>  	if (metadump.progress_since_warning)
>  		fputc('\n', metadump.stdout_metadump ? stderr : stdout);
> @@ -3225,7 +3223,7 @@ metadump_f(
>  	while (iocur_sp > start_iocur_sp)
>  		pop_cur();
>  
> -	release_metadump();
> +	metadump.mdops->release_metadump();
>  
>  out:
>  	return 0;
> -- 
> 2.39.1
>
Chandan Babu R May 25, 2023, 2:19 p.m. UTC | #2
On Tue, May 23, 2023 at 10:25:13 AM -0700, Darrick J. Wong wrote:
> On Tue, May 23, 2023 at 02:30:35PM +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(), metadump_ops->write_metadump() and
>> metadump_ops->release_metadump() respectively.
>> 
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>> ---
>>  db/metadump.c | 124 +++++++++++++++++++++++++-------------------------
>>  1 file changed, 61 insertions(+), 63 deletions(-)
>> 
>> diff --git a/db/metadump.c b/db/metadump.c
>> index 56d8c3bdf..7265f73ec 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -135,59 +135,6 @@ print_progress(const char *fmt, ...)
>>  	metadump.progress_since_warning = 1;
>>  }
>>  
>> -/*
>> - * 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
>> @@ -224,15 +171,16 @@ write_buf(
>>  
>>  	/* handle discontiguous buffers */
>>  	if (!buf->bbmap) {
>> -		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
>> +		ret = metadump.mdops->write_metadump(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_metadump(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;
>> @@ -2994,7 +2942,7 @@ done:
>>  }
>>  
>>  static int
>> -init_metadump(void)
>> +init_metadump_v1(void)
>>  {
>>  	metadump.metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);
>>  	if (metadump.metablock == NULL) {
>> @@ -3035,12 +2983,60 @@ 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,
>> +	int64_t		off,
>
> This really ought to be an xfs_daddr_t, right?
>

Yes, you are right. I will make the required change.

>> +	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);
>
> Wondering if this ought to be called ->record_segment or something, since
> it's not really writing anything to disk, merely copying it to the index
> buffer.
>
>> +		if (++metadump.cur_index == metadump.num_indices) {
>> +			ret = end_write_metadump_v1();
>> +			if (ret)
>> +				return -EIO;
>
> This is "generic" code for "Have we filled up the index table?  If so,
> then write the index block the indexed data".  Shouldn't it go in
> write_buf?  And then write_buf does something like:
>
> 	while (len > 0) {
> 		segment_len = min(len, metadump.num_indices - metadump.cur_index);
>
> 		metadump.ops->record_segment(type, buf, daddr, segment_len);
>
> 		metadump.cur_index += segment_len;
> 		if (metadump.cur_index == metadump.num_indices) {
> 			metadump.ops->write_index(...);
> 			metadump.cur_index = 0;
> 		}
>
> 		len -= segment_len;
> 		daddr += segment_len;
> 		buf += (segment_len << 9);
> 	}
>
> 	if (metadump.cur_index)
> 		metadump.ops->write_index(...);
> 	metadump.cur_index = 0;
>

The above change would require write_buf() to know about the internals of
metadump v1 format. This change can be performed as long as the metadump
supports only the v1 format. However, supporting the v2 format later requires
that write_buf() invoke version specific functions via function pointers and
to not perform any other version specific operation (e.g. checking to see if
the v1 format's in-memory index is filled) on its own.

>> +		}
>> +	}
>> +
>> +        return 0;
>> +}
>> +
>>  static void
>> -release_metadump(void)
>> +release_metadump_v1(void)
>>  {
>>  	free(metadump.metablock);
>>  }
>>  
>> +static struct metadump_ops metadump1_ops = {
>> +	.init_metadump = init_metadump_v1,
>> +	.write_metadump = write_metadump_v1,
>> +	.end_write_metadump = end_write_metadump_v1,
>> +	.release_metadump = release_metadump_v1,
>> +};
>> +
>>  static int
>>  metadump_f(
>>  	int 		argc,
>> @@ -3182,9 +3178,11 @@ metadump_f(
>>  		}
>>  	}
>>  
>> -	ret = init_metadump();
>> +	metadump.mdops = &metadump1_ops;
>> +
>> +	ret = metadump.mdops->init_metadump();
>>  	if (ret)
>> -		return 0;
>> +		goto out;
>>  
>>  	exitcode = 0;
>>  
>> @@ -3206,7 +3204,7 @@ metadump_f(
>>  
>>  	/* write the remaining index */
>>  	if (!exitcode)
>> -		exitcode = write_index() < 0;
>> +		exitcode = metadump.mdops->end_write_metadump() < 0;
>>  
>>  	if (metadump.progress_since_warning)
>>  		fputc('\n', metadump.stdout_metadump ? stderr : stdout);
>> @@ -3225,7 +3223,7 @@ metadump_f(
>>  	while (iocur_sp > start_iocur_sp)
>>  		pop_cur();
>>  
>> -	release_metadump();
>> +	metadump.mdops->release_metadump();
>>  
>>  out:
>>  	return 0;
>> -- 
>> 2.39.1
>>
Darrick J. Wong June 2, 2023, 2:34 p.m. UTC | #3
On Thu, May 25, 2023 at 07:49:32PM +0530, Chandan Babu R wrote:
> On Tue, May 23, 2023 at 10:25:13 AM -0700, Darrick J. Wong wrote:
> > On Tue, May 23, 2023 at 02:30:35PM +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(), metadump_ops->write_metadump() and
> >> metadump_ops->release_metadump() respectively.
> >> 
> >> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> >> ---
> >>  db/metadump.c | 124 +++++++++++++++++++++++++-------------------------
> >>  1 file changed, 61 insertions(+), 63 deletions(-)
> >> 
> >> diff --git a/db/metadump.c b/db/metadump.c
> >> index 56d8c3bdf..7265f73ec 100644
> >> --- a/db/metadump.c
> >> +++ b/db/metadump.c
> >> @@ -135,59 +135,6 @@ print_progress(const char *fmt, ...)
> >>  	metadump.progress_since_warning = 1;
> >>  }
> >>  
> >> -/*
> >> - * 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
> >> @@ -224,15 +171,16 @@ write_buf(
> >>  
> >>  	/* handle discontiguous buffers */
> >>  	if (!buf->bbmap) {
> >> -		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
> >> +		ret = metadump.mdops->write_metadump(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_metadump(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;
> >> @@ -2994,7 +2942,7 @@ done:
> >>  }
> >>  
> >>  static int
> >> -init_metadump(void)
> >> +init_metadump_v1(void)
> >>  {
> >>  	metadump.metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);
> >>  	if (metadump.metablock == NULL) {
> >> @@ -3035,12 +2983,60 @@ 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,
> >> +	int64_t		off,
> >
> > This really ought to be an xfs_daddr_t, right?
> >
> 
> Yes, you are right. I will make the required change.
> 
> >> +	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);
> >
> > Wondering if this ought to be called ->record_segment or something, since
> > it's not really writing anything to disk, merely copying it to the index
> > buffer.
> >
> >> +		if (++metadump.cur_index == metadump.num_indices) {
> >> +			ret = end_write_metadump_v1();
> >> +			if (ret)
> >> +				return -EIO;
> >
> > This is "generic" code for "Have we filled up the index table?  If so,
> > then write the index block the indexed data".  Shouldn't it go in
> > write_buf?  And then write_buf does something like:
> >
> > 	while (len > 0) {
> > 		segment_len = min(len, metadump.num_indices - metadump.cur_index);
> >
> > 		metadump.ops->record_segment(type, buf, daddr, segment_len);
> >
> > 		metadump.cur_index += segment_len;
> > 		if (metadump.cur_index == metadump.num_indices) {
> > 			metadump.ops->write_index(...);
> > 			metadump.cur_index = 0;
> > 		}
> >
> > 		len -= segment_len;
> > 		daddr += segment_len;
> > 		buf += (segment_len << 9);
> > 	}
> >
> > 	if (metadump.cur_index)
> > 		metadump.ops->write_index(...);
> > 	metadump.cur_index = 0;
> >
> 
> The above change would require write_buf() to know about the internals of
> metadump v1 format. This change can be performed as long as the metadump
> supports only the v1 format. However, supporting the v2 format later requires
> that write_buf() invoke version specific functions via function pointers and
> to not perform any other version specific operation (e.g. checking to see if
> the v1 format's in-memory index is filled) on its own.

Ah, right.  I made that comment before you pointed out that v2
interleaves meta_extent records with dump data instead of having
separate index blocks.  Question withdrawn.

--D

> >> +		}
> >> +	}
> >> +
> >> +        return 0;
> >> +}
> >> +
> >>  static void
> >> -release_metadump(void)
> >> +release_metadump_v1(void)
> >>  {
> >>  	free(metadump.metablock);
> >>  }
> >>  
> >> +static struct metadump_ops metadump1_ops = {
> >> +	.init_metadump = init_metadump_v1,
> >> +	.write_metadump = write_metadump_v1,
> >> +	.end_write_metadump = end_write_metadump_v1,
> >> +	.release_metadump = release_metadump_v1,
> >> +};
> >> +
> >>  static int
> >>  metadump_f(
> >>  	int 		argc,
> >> @@ -3182,9 +3178,11 @@ metadump_f(
> >>  		}
> >>  	}
> >>  
> >> -	ret = init_metadump();
> >> +	metadump.mdops = &metadump1_ops;
> >> +
> >> +	ret = metadump.mdops->init_metadump();
> >>  	if (ret)
> >> -		return 0;
> >> +		goto out;
> >>  
> >>  	exitcode = 0;
> >>  
> >> @@ -3206,7 +3204,7 @@ metadump_f(
> >>  
> >>  	/* write the remaining index */
> >>  	if (!exitcode)
> >> -		exitcode = write_index() < 0;
> >> +		exitcode = metadump.mdops->end_write_metadump() < 0;
> >>  
> >>  	if (metadump.progress_since_warning)
> >>  		fputc('\n', metadump.stdout_metadump ? stderr : stdout);
> >> @@ -3225,7 +3223,7 @@ metadump_f(
> >>  	while (iocur_sp > start_iocur_sp)
> >>  		pop_cur();
> >>  
> >> -	release_metadump();
> >> +	metadump.mdops->release_metadump();
> >>  
> >>  out:
> >>  	return 0;
> >> -- 
> >> 2.39.1
> >> 
> 
> 
> -- 
> chandan
diff mbox series

Patch

diff --git a/db/metadump.c b/db/metadump.c
index 56d8c3bdf..7265f73ec 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -135,59 +135,6 @@  print_progress(const char *fmt, ...)
 	metadump.progress_since_warning = 1;
 }
 
-/*
- * 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
@@ -224,15 +171,16 @@  write_buf(
 
 	/* handle discontiguous buffers */
 	if (!buf->bbmap) {
-		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
+		ret = metadump.mdops->write_metadump(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_metadump(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;
@@ -2994,7 +2942,7 @@  done:
 }
 
 static int
-init_metadump(void)
+init_metadump_v1(void)
 {
 	metadump.metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);
 	if (metadump.metablock == NULL) {
@@ -3035,12 +2983,60 @@  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,
+	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 = 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_metadump = init_metadump_v1,
+	.write_metadump = write_metadump_v1,
+	.end_write_metadump = end_write_metadump_v1,
+	.release_metadump = release_metadump_v1,
+};
+
 static int
 metadump_f(
 	int 		argc,
@@ -3182,9 +3178,11 @@  metadump_f(
 		}
 	}
 
-	ret = init_metadump();
+	metadump.mdops = &metadump1_ops;
+
+	ret = metadump.mdops->init_metadump();
 	if (ret)
-		return 0;
+		goto out;
 
 	exitcode = 0;
 
@@ -3206,7 +3204,7 @@  metadump_f(
 
 	/* write the remaining index */
 	if (!exitcode)
-		exitcode = write_index() < 0;
+		exitcode = metadump.mdops->end_write_metadump() < 0;
 
 	if (metadump.progress_since_warning)
 		fputc('\n', metadump.stdout_metadump ? stderr : stdout);
@@ -3225,7 +3223,7 @@  metadump_f(
 	while (iocur_sp > start_iocur_sp)
 		pop_cur();
 
-	release_metadump();
+	metadump.mdops->release_metadump();
 
 out:
 	return 0;