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@xxxxxxxxxx>
>> ---
>> 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.
--
chandan
