On Wed, Jul 12, 2023 at 10:46:09 AM -0700, Darrick J. Wong wrote:
> On Tue, Jun 06, 2023 at 02:58:00PM +0530, Chandan Babu R wrote:
>> This commit moves functionality associated with opening the target device,
>> reading metadump header information and printing information about the
>> metadump into their respective functions. There are no functional changes made
>> by this commit.
>>
>> Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
>> ---
>> mdrestore/xfs_mdrestore.c | 141 +++++++++++++++++++++++---------------
>> 1 file changed, 85 insertions(+), 56 deletions(-)
>>
>> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
>> index 2a9527b9..61e06598 100644
>> --- a/mdrestore/xfs_mdrestore.c
>> +++ b/mdrestore/xfs_mdrestore.c
>> @@ -6,6 +6,7 @@
>>
>> #include "libxfs.h"
>> #include "xfs_metadump.h"
>> +#include <libfrog/platform.h>
>>
>> static struct mdrestore {
>> bool show_progress;
>> @@ -40,8 +41,72 @@ print_progress(const char *fmt, ...)
>> mdrestore.progress_since_warning = true;
>> }
>>
>> +static int
>> +open_device(
>> + char *path,
>> + bool *is_file)
>> +{
>> + struct stat statbuf;
>> + int open_flags;
>> + int fd;
>> +
>> + open_flags = O_RDWR;
>> + *is_file = false;
>> +
>> + if (stat(path, &statbuf) < 0) {
>> + /* ok, assume it's a file and create it */
>> + open_flags |= O_CREAT;
>> + *is_file = true;
>> + } else if (S_ISREG(statbuf.st_mode)) {
>> + open_flags |= O_TRUNC;
>> + *is_file = true;
>> + } else {
>> + /*
>> + * check to make sure a filesystem isn't mounted on the device
>> + */
>> + if (platform_check_ismounted(path, NULL, &statbuf, 0))
>
> } else if (platform_check_ismounted(...)) ?
>
Ok.
>> + fatal("a filesystem is mounted on target device \"%s\","
>> + " cannot restore to a mounted filesystem.\n",
>> + path);
>> + }
>> +
>> + fd = open(path, open_flags, 0644);
>> + if (fd < 0)
>> + fatal("couldn't open \"%s\"\n", path);
>> +
>> + return fd;
>> +}
>> +
>> +static void
>> +read_header(
>> + struct xfs_metablock *mb,
>> + FILE *md_fp)
>> +{
>> + mb->mb_magic = cpu_to_be32(XFS_MD_MAGIC_V1);
>> +
>> + if (fread((uint8_t *)mb + sizeof(mb->mb_magic),
>> + sizeof(*mb) - sizeof(mb->mb_magic), 1, md_fp) != 1)
>> + fatal("error reading from metadump file\n");
>> +}
>> +
>> +static void
>> +show_info(
>> + struct xfs_metablock *mb,
>> + const char *md_file)
>> +{
>> + if (mb->mb_info & XFS_METADUMP_INFO_FLAGS) {
>> + printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
>> + md_file,
>> + mb->mb_info & XFS_METADUMP_OBFUSCATED ? "":"not ",
>> + mb->mb_info & XFS_METADUMP_DIRTYLOG ? "dirty":"clean",
>> + mb->mb_info & XFS_METADUMP_FULLBLOCKS ? "full":"zeroed");
>> + } else {
>> + printf("%s: no informational flags present\n", md_file);
>> + }
>> +}
>> +
>> /*
>> - * perform_restore() -- do the actual work to restore the metadump
>> + * 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
>> @@ -51,9 +116,9 @@ print_progress(const char *fmt, ...)
>> * src_f should be positioned just past a read the previously validated metablock
>> */
>> static void
>> -perform_restore(
>> - FILE *src_f,
>> - int dst_fd,
>> +restore(
>> + FILE *md_fp,
>> + int ddev_fd,
>> int is_target_file,
>> const struct xfs_metablock *mbp)
>> {
>> @@ -81,14 +146,15 @@ perform_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,
>> + md_fp) != 1)
>
> Something I forgot to comment on in previous patches: Please don't
> indent the second line of an if test at the same level as the if body.
> It's much harder for me to distinguish the two. Compare:
>
> if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1,
> md_fp) != 1)
> fatal("error reading from metadump file\n");
> vs:
>
> if (fread(block_index, block_size - sizeof(struct xfs_metablock), 1,
> md_fp) != 1)
> fatal("error reading from metadump file\n");
>
> Also, previous patches have done things like:
>
> if (foocondition &&
> barcondition &&
> bazcondition)
> dosomething();
>
> vs.
>
> if (foocondition &&
> barcondition &&
> bazcondition)
> dosomething();
>
> The code structure is easier to see at a glance, right? That's why the
> xfs and kernel style guidelines ask for distinct indentation levels.
> Please go back and fix all of that in the new code you're adding.
Sorry, I will fix the indentation issues across the patchset.
--
chandan
