On Wed, Jul 12, 2023 at 11:10:03 AM -0700, Darrick J. Wong wrote:
> On Tue, Jun 06, 2023 at 02:58:05PM +0530, Chandan Babu R wrote:
>> This commit adds functionality to restore metadump stored in v2 format.
>>
>> Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
>> ---
>> mdrestore/xfs_mdrestore.c | 251 +++++++++++++++++++++++++++++++++++---
>> 1 file changed, 233 insertions(+), 18 deletions(-)
>>
>> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
>> index c395ae90..7b484071 100644
>> --- a/mdrestore/xfs_mdrestore.c
>> +++ b/mdrestore/xfs_mdrestore.c
>> @@ -12,7 +12,8 @@ struct mdrestore_ops {
>> void (*read_header)(void *header, FILE *md_fp);
>> void (*show_info)(void *header, const char *md_file);
>> void (*restore)(void *header, FILE *md_fp, int ddev_fd,
>> - bool is_target_file);
>> + bool is_data_target_file, int logdev_fd,
>> + bool is_log_target_file);
>> };
>>
>> static struct mdrestore {
>> @@ -20,6 +21,7 @@ static struct mdrestore {
>> bool show_progress;
>> bool show_info;
>> bool progress_since_warning;
>> + bool external_log;
>> } mdrestore;
>>
>> static void
>> @@ -143,10 +145,12 @@ show_info_v1(
>>
>> static void
>> restore_v1(
>> - void *header,
>> - FILE *md_fp,
>> - int ddev_fd,
>> - bool is_target_file)
>> + void *header,
>> + FILE *md_fp,
>> + int ddev_fd,
>> + bool is_data_target_file,
>
> Why does the indent level change here...
>
>> + int logdev_fd,
>> + bool is_log_target_file)
>> {
>> struct xfs_metablock *metablock;
>> struct xfs_metablock *mbp;
>
> ...but not here?
>
Sorry, I will fix this.
>> @@ -203,7 +207,7 @@ restore_v1(
>>
>> ((struct xfs_dsb*)block_buffer)->sb_inprogress = 1;
>>
>> - verify_device_size(ddev_fd, is_target_file, sb.sb_dblocks,
>> + verify_device_size(ddev_fd, is_data_target_file, sb.sb_dblocks,
>> sb.sb_blocksize);
>>
>> bytes_read = 0;
>> @@ -264,6 +268,195 @@ static struct mdrestore_ops mdrestore_ops_v1 = {
>> .restore = restore_v1,
>> };
>>
>> +static void
>> +read_header_v2(
>> + void *header,
>> + FILE *md_fp)
>> +{
>> + struct xfs_metadump_header *xmh = header;
>> + bool want_external_log;
>> +
>> + xmh->xmh_magic = cpu_to_be32(XFS_MD_MAGIC_V2);
>> +
>> + if (fread((uint8_t *)xmh + sizeof(xmh->xmh_magic),
>> + sizeof(*xmh) - sizeof(xmh->xmh_magic), 1, md_fp) != 1)
>> + fatal("error reading from metadump file\n");
>> +
>> + want_external_log = !!(be32_to_cpu(xmh->xmh_incompat_flags) &
>> + XFS_MD2_INCOMPAT_EXTERNALLOG);
>> +
>> + if (want_external_log && !mdrestore.external_log)
>> + fatal("External Log device is required\n");
>> +}
>> +
>> +static void
>> +show_info_v2(
>> + void *header,
>> + const char *md_file)
>> +{
>> + struct xfs_metadump_header *xmh;
>> + uint32_t incompat_flags;
>> +
>> + xmh = header;
>> + incompat_flags = be32_to_cpu(xmh->xmh_incompat_flags);
>> +
>> + printf("%s: %sobfuscated, %s log, external log contents are %sdumped, %s metadata blocks,\n",
>> + md_file,
>> + incompat_flags & XFS_MD2_INCOMPAT_OBFUSCATED ? "":"not ",
>> + incompat_flags & XFS_MD2_INCOMPAT_DIRTYLOG ? "dirty":"clean",
>> + incompat_flags & XFS_MD2_INCOMPAT_EXTERNALLOG ? "":"not ",
>> + incompat_flags & XFS_MD2_INCOMPAT_FULLBLOCKS ? "full":"zeroed");
>> +}
>> +
>> +#define MDR_IO_BUF_SIZE (8 * 1024 * 1024)
>> +
>> +static void
>> +dump_meta_extent(
>
> Aren't we restoring here? And not dumping?
>
You are right. I will rename the function to restore_meta_extent().
>> + FILE *md_fp,
>> + int dev_fd,
>> + char *device,
>> + void *buf,
>> + uint64_t offset,
>> + int len)
>> +{
>> + int io_size;
>> +
>> + io_size = min(len, MDR_IO_BUF_SIZE);
>> +
>> + do {
>> + if (fread(buf, io_size, 1, md_fp) != 1)
>> + fatal("error reading from metadump file\n");
>> + if (pwrite(dev_fd, buf, io_size, offset) < 0)
>> + fatal("error writing to %s device at offset %llu: %s\n",
>> + device, offset, strerror(errno));
>> + len -= io_size;
>> + offset += io_size;
>> +
>> + io_size = min(len, io_size);
>> + } while (len);
>> +}
>> +
>> +static void
>> +restore_v2(
>> + void *header,
>> + FILE *md_fp,
>> + int ddev_fd,
>> + bool is_data_target_file,
>> + int logdev_fd,
>> + bool is_log_target_file)
>> +{
>> + struct xfs_sb sb;
>> + struct xfs_meta_extent xme;
>> + char *block_buffer;
>> + int64_t bytes_read;
>> + uint64_t offset;
>> + int len;
>> +
>> + block_buffer = malloc(MDR_IO_BUF_SIZE);
>> + if (block_buffer == NULL)
>> + fatal("Unable to allocate input buffer memory\n");
>> +
>> + if (fread(&xme, sizeof(xme), 1, md_fp) != 1)
>> + fatal("error reading from metadump file\n");
>> +
>> + if (xme.xme_addr != 0 || xme.xme_len == 1)
>> + fatal("Invalid superblock disk address/length\n");
>
> Shouldn't we check that xme_addr points to XME_ADDR_DATA_DEVICE?
>
Yes, you are right. I will add the check.
>> + len = BBTOB(be32_to_cpu(xme.xme_len));
>> +
>> + if (fread(block_buffer, len, 1, md_fp) != 1)
>> + fatal("error reading from metadump file\n");
>> +
>> + libxfs_sb_from_disk(&sb, (struct xfs_dsb *)block_buffer);
>> +
>> + if (sb.sb_magicnum != XFS_SB_MAGIC)
>> + fatal("bad magic number for primary superblock\n");
>> +
>> + ((struct xfs_dsb *)block_buffer)->sb_inprogress = 1;
>> +
>> + verify_device_size(ddev_fd, is_data_target_file, sb.sb_dblocks,
>> + sb.sb_blocksize);
>> +
>> + if (sb.sb_logstart == 0) {
>> + ASSERT(mdrestore.external_log == true);
>
> This should be more graceful to users:
>
> if (!mdrestore.external_log)
> fatal("Filesystem has external log but -l not specified.\n");
mdrestore.external_log is set to true only when the user passes the -l
option. Also, read_header_v2() would have already verified if the metadump
file contains data captured from an external log device and that an external
log device was indeed passed to the restore program. It should be impossible
to have mdrestore.external_log set to false at this point in
restore_v2(). Hence, I think a call to ASSERT() is more appropriate.
--
chandan
