From: Chandan Babu R <chandan.babu@oracle.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: cem@kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 09/24] metadump: Introduce metadump v1 operations
Date: Thu, 25 May 2023 19:49:32 +0530 [thread overview]
Message-ID: <87lehbjx2t.fsf@debian-BULLSEYE-live-builder-AMD64> (raw)
In-Reply-To: <20230523172513.GP11620@frogsfrogsfrogs>
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
>>
--
chandan
next prev parent reply other threads:[~2023-05-26 8:50 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-23 9:00 [PATCH 00/24] Metadump v2 Chandan Babu R
2023-05-23 9:00 ` [PATCH 01/24] metadump: Use boolean values true/false instead of 1/0 Chandan Babu R
2023-05-23 16:31 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 02/24] mdrestore: Fix logic used to check if target device is large enough Chandan Babu R
2023-05-23 16:32 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 03/24] metadump: Define and use struct metadump Chandan Babu R
2023-05-23 16:35 ` Darrick J. Wong
2023-05-24 4:50 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 04/24] metadump: Add initialization and release functions Chandan Babu R
2023-05-23 16:36 ` Darrick J. Wong
2023-05-24 5:03 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 05/24] set_cur: Add support to read from external log device Chandan Babu R
2023-05-23 16:48 ` Darrick J. Wong
2023-05-25 8:27 ` Chandan Babu R
2023-06-05 9:19 ` Chandan Babu R
2023-06-05 19:22 ` Darrick J. Wong
2023-06-06 4:47 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 06/24] metadump: Dump external log device contents Chandan Babu R
2023-05-23 17:02 ` Darrick J. Wong
2023-05-26 6:54 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 07/24] metadump: Postpone invocation of init_metadump() Chandan Babu R
2023-05-23 17:13 ` Darrick J. Wong
2023-05-25 8:45 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 08/24] metadump: Introduce struct metadump_ops Chandan Babu R
2023-05-23 17:15 ` Darrick J. Wong
2023-05-25 8:48 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 09/24] metadump: Introduce metadump v1 operations Chandan Babu R
2023-05-23 17:25 ` Darrick J. Wong
2023-05-25 14:19 ` Chandan Babu R [this message]
2023-06-02 14:34 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 10/24] metadump: Rename XFS_MD_MAGIC to XFS_MD_MAGIC_V1 Chandan Babu R
2023-05-23 17:27 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 11/24] metadump: Define metadump v2 ondisk format structures and macros Chandan Babu R
2023-05-23 17:34 ` Darrick J. Wong
2023-05-25 9:26 ` Chandan Babu R
2023-06-02 14:46 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 12/24] metadump: Define metadump ops for v2 format Chandan Babu R
2023-05-23 17:37 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 13/24] metadump: Add support for passing version option Chandan Babu R
2023-05-23 17:41 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 14/24] xfs_metadump.sh: " Chandan Babu R
2023-05-23 17:39 ` Darrick J. Wong
2023-05-25 9:31 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 15/24] xfs_metadump.8: Add description for the newly introduced -v option Chandan Babu R
2023-05-23 17:40 ` Darrick J. Wong
2023-05-25 10:04 ` Chandan Babu R
2023-06-02 14:58 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 16/24] mdrestore: Define and use struct mdrestore Chandan Babu R
2023-05-23 17:42 ` Darrick J. Wong
2023-05-26 8:38 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 17/24] mdrestore: Add open_device(), read_header() and show_info() functions Chandan Babu R
2023-05-23 17:44 ` Darrick J. Wong
2023-05-25 10:11 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 18/24] mdrestore: Introduce struct mdrestore_ops Chandan Babu R
2023-05-23 17:44 ` Darrick J. Wong
2023-05-25 10:34 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 19/24] mdrestore: Introduce mdrestore v1 operations Chandan Babu R
2023-05-23 17:48 ` Darrick J. Wong
2023-05-25 10:39 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 20/24] mdrestore: Detect metadump version from metadump image Chandan Babu R
2023-05-23 18:11 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 21/24] mdrestore: Extract target device size verification into a function Chandan Babu R
2023-05-23 18:07 ` Darrick J. Wong
2023-05-25 12:02 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 22/24] mdrestore: Define mdrestore ops for v2 format Chandan Babu R
2023-05-23 18:06 ` Darrick J. Wong
2023-05-25 12:10 ` Chandan Babu R
2023-06-02 15:01 ` Darrick J. Wong
2023-05-23 9:00 ` [PATCH 23/24] mdrestore: Add support for passing log device as an argument Chandan Babu R
2023-05-23 18:09 ` Darrick J. Wong
2023-05-25 13:43 ` Chandan Babu R
2023-06-02 15:02 ` Darrick J. Wong
2023-06-05 6:19 ` Chandan Babu R
2023-05-23 9:00 ` [PATCH 24/24] xfs_mdrestore.8: Add description for the newly introduced -l option Chandan Babu R
2023-05-23 18:10 ` Darrick J. Wong
2023-05-25 13:45 ` Chandan Babu R
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87lehbjx2t.fsf@debian-BULLSEYE-live-builder-AMD64 \
--to=chandan.babu@oracle.com \
--cc=cem@kernel.org \
--cc=djwong@kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox