On Tue, May 23, 2023 at 02:30:45PM +0530, Chandan Babu R wrote:
> In order to indicate the version of metadump files that they can work with,
> this commit renames read_header(), show_info() and restore() functions to
> read_header_v1(), show_info_v1() and restore_v1() respectively.
>
> Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
> ---
> mdrestore/xfs_mdrestore.c | 76 ++++++++++++++++++++++-----------------
> 1 file changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index 895e5cdab..5ec1a47b0 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -86,16 +86,26 @@ open_device(
> return fd;
> }
>
> -static void read_header(struct xfs_metablock *mb, FILE *src_f)
> +static void
> +read_header_v1(
> + void *header,
> + FILE *mdfp)
> {
> - if (fread(mb, sizeof(*mb), 1, src_f) != 1)
> + struct xfs_metablock *mb = header;
> +
> + if (fread(mb, sizeof(*mb), 1, mdfp) != 1)
> fatal("error reading from metadump file\n");
> if (mb->mb_magic != cpu_to_be32(XFS_MD_MAGIC_V1))
> fatal("specified file is not a metadata dump\n");
> }
>
> -static void show_info(struct xfs_metablock *mb, const char *mdfile)
> +static void
> +show_info_v1(
> + void *header,
> + const char *mdfile)
> {
> + struct xfs_metablock *mb = header;
> +
> if (mb->mb_info & XFS_METADUMP_INFO_FLAGS) {
> printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
> mdfile,
> @@ -107,24 +117,15 @@ static void show_info(struct xfs_metablock *mb, const char *mdfile)
> }
> }
>
> -/*
> - * restore() -- do the actual work to restore the metadump
> - *
> - * @src_f: A FILE pointer to the source metadump
> - * @dst_fd: the file descriptor for the target file
> - * @is_target_file: designates whether the target is a regular file
> - * @mbp: pointer to metadump's first xfs_metablock, read and verified by the caller
> - *
> - * src_f should be positioned just past a read the previously validated metablock
> - */
> static void
> -restore(
> - FILE *src_f,
> - int dst_fd,
> - int is_target_file,
> - const struct xfs_metablock *mbp)
> +restore_v1(
> + void *header,
> + FILE *mdfp,
> + int data_fd,
Umm. mdfp == "FILE * stream for reading the source" and "data_fd" == "fd
pointing to data device for writing the filesystem"?
I think I'd prefer md_fp and ddev_fd...
> + bool is_target_file)
> {
> - struct xfs_metablock *metablock; /* header + index + blocks */
> + struct xfs_metablock *mbp = header;
> + struct xfs_metablock *metablock;
> __be64 *block_index;
> char *block_buffer;
> int block_size;
> @@ -148,14 +149,15 @@ restore(
> block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));
> block_buffer = (char *)metablock + block_size;
>
> - if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1, src_f) != 1)
> + if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1,
> + mdfp) != 1)
> fatal("error reading from metadump file\n");
>
> if (block_index[0] != 0)
> fatal("first block is not the primary superblock\n");
>
>
> - if (fread(block_buffer, mb_count << mbp->mb_blocklog, 1, src_f) != 1)
> + if (fread(block_buffer, mb_count << mbp->mb_blocklog, 1, mdfp) != 1)
> fatal("error reading from metadump file\n");
>
> libxfs_sb_from_disk(&sb, (struct xfs_dsb *)block_buffer);
> @@ -178,7 +180,7 @@ restore(
> if (is_target_file) {
> /* ensure regular files are correctly sized */
>
> - if (ftruncate(dst_fd, sb.sb_dblocks * sb.sb_blocksize))
> + if (ftruncate(data_fd, sb.sb_dblocks * sb.sb_blocksize))
> fatal("cannot set filesystem image size: %s\n",
> strerror(errno));
> } else {
> @@ -188,7 +190,7 @@ restore(
> off64_t off;
>
> off = sb.sb_dblocks * sb.sb_blocksize - sizeof(lb);
> - if (pwrite(dst_fd, lb, sizeof(lb), off) < 0)
> + if (pwrite(data_fd, lb, sizeof(lb), off) < 0)
> fatal("failed to write last block, is target too "
> "small? (error: %s)\n", strerror(errno));
> }
> @@ -201,7 +203,7 @@ restore(
> print_progress("%lld MB read", bytes_read >> 20);
>
> for (cur_index = 0; cur_index < mb_count; cur_index++) {
> - if (pwrite(dst_fd, &block_buffer[cur_index <<
> + if (pwrite(data_fd, &block_buffer[cur_index <<
> mbp->mb_blocklog], block_size,
> be64_to_cpu(block_index[cur_index]) <<
> BBSHIFT) < 0)
> @@ -212,7 +214,7 @@ restore(
> if (mb_count < max_indices)
> break;
>
> - if (fread(metablock, block_size, 1, src_f) != 1)
> + if (fread(metablock, block_size, 1, mdfp) != 1)
> fatal("error reading from metadump file\n");
>
> mb_count = be16_to_cpu(metablock->mb_count);
> @@ -222,7 +224,7 @@ restore(
> fatal("bad block count: %u\n", mb_count);
>
> if (fread(block_buffer, mb_count << mbp->mb_blocklog,
> - 1, src_f) != 1)
> + 1, mdfp) != 1)
> fatal("error reading from metadump file\n");
>
> bytes_read += block_size + (mb_count << mbp->mb_blocklog);
> @@ -239,12 +241,18 @@ restore(
> offsetof(struct xfs_sb, sb_crc));
> }
>
> - if (pwrite(dst_fd, block_buffer, sb.sb_sectsize, 0) < 0)
> + if (pwrite(data_fd, block_buffer, sb.sb_sectsize, 0) < 0)
> fatal("error writing primary superblock: %s\n", strerror(errno));
>
> free(metablock);
> }
>
> +static struct mdrestore_ops mdrestore_ops_v1 = {
> + .read_header = read_header_v1,
> + .show_info = show_info_v1,
> + .restore = restore_v1,
> +};
> +
> static void
> usage(void)
> {
> @@ -294,9 +302,9 @@ main(
>
> /*
> * open source and test if this really is a dump. The first metadump
> - * block will be passed to restore() which will continue to read the
> - * file from this point. This avoids rewind the stream, which causes
> - * restore to fail when source was being read from stdin.
> + * block will be passed to mdrestore_ops->restore() which will continue
> + * to read the file from this point. This avoids rewind the stream,
> + * which causes restore to fail when source was being read from stdin.
> */
> if (strcmp(argv[optind], "-") == 0) {
> src_f = stdin;
> @@ -308,10 +316,12 @@ main(
> fatal("cannot open source dump file\n");
> }
>
> - read_header(&mb, src_f);
> + mdrestore.mdrops = &mdrestore_ops_v1;
> +
> + mdrestore.mdrops->read_header(&mb, src_f);
Starting to wonder if it's a mistake to continue to declare a struct
xfs_metablock directly on the stack in main() but I guess I'll see what
happens when you introduce the v2 code.
--D
>
> if (mdrestore.show_info) {
> - show_info(&mb, argv[optind]);
> + mdrestore.mdrops->show_info(&mb, argv[optind]);
>
> if (argc - optind == 1)
> exit(0);
> @@ -322,7 +332,7 @@ main(
> /* check and open target */
> dst_fd = open_device(argv[optind], &is_target_file);
>
> - restore(src_f, dst_fd, is_target_file, &mb);
> + mdrestore.mdrops->restore(&mb, src_f, dst_fd, is_target_file);
>
> close(dst_fd);
> if (src_f != stdin)
> --
> 2.39.1
>
