From 7173d77e6da29ddc35bf8d87e20db9b16f06b26c Mon Sep 17 00:00:00 2001 From: Amul Sul Date: Wed, 14 Aug 2024 10:42:37 +0530 Subject: [PATCH v11 06/12] Refactor: split verify_backup_file() function and rename it. The function verify_backup_file() has now been renamed to verify_plain_backup_file() to make it clearer that it is specifically used for verifying files in a plain backup. Similarly, in a future patch, we would have a verify_tar_backup_file() function for verifying TAR backup files. In addition to that, moved the manifest entry verification code into a new function called verify_manifest_entry() so that it can be reused for tar backup verification. If verify_manifest_entry() doesn't find an entry, it reports an error as before and returns NULL to the caller. This is why a NULL check is added to should_verify_checksum(). --- src/bin/pg_verifybackup/pg_verifybackup.c | 58 +++++++++++++++-------- src/bin/pg_verifybackup/pg_verifybackup.h | 6 ++- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c index 3fcfb167217..5bfc98e7874 100644 --- a/src/bin/pg_verifybackup/pg_verifybackup.c +++ b/src/bin/pg_verifybackup/pg_verifybackup.c @@ -64,8 +64,8 @@ static void report_manifest_error(JsonManifestParseContext *context, static void verify_backup_directory(verifier_context *context, char *relpath, char *fullpath); -static void verify_backup_file(verifier_context *context, - char *relpath, char *fullpath); +static void verify_plain_backup_file(verifier_context *context, + char *relpath, char *fullpath); static void verify_control_file(const char *controlpath, uint64 manifest_system_identifier); static void report_extra_backup_files(verifier_context *context); @@ -570,7 +570,7 @@ verify_backup_directory(verifier_context *context, char *relpath, newrelpath = psprintf("%s/%s", relpath, filename); if (!should_ignore_relpath(context, newrelpath)) - verify_backup_file(context, newrelpath, newfullpath); + verify_plain_backup_file(context, newrelpath, newfullpath); pfree(newfullpath); pfree(newrelpath); @@ -591,7 +591,8 @@ verify_backup_directory(verifier_context *context, char *relpath, * verify_backup_directory. */ static void -verify_backup_file(verifier_context *context, char *relpath, char *fullpath) +verify_plain_backup_file(verifier_context *context, char *relpath, + char *fullpath) { struct stat sb; manifest_file *m; @@ -627,6 +628,32 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath) return; } + /* Check the backup manifest entry for this file. */ + m = verify_manifest_entry(context, relpath, sb.st_size); + + /* + * Validate the manifest system identifier, not available in manifest + * version 1. + */ + if (context->manifest->version != 1 && + strcmp(relpath, "global/pg_control") == 0 && + m != NULL && m->matched && !m->bad) + verify_control_file(fullpath, context->manifest->system_identifier); + + /* Update statistics for progress report, if necessary */ + if (show_progress && !context->skip_checksums && + should_verify_checksum(m)) + total_size += m->size; +} + +/* + * Verify file and its size entry in the manifest. + */ +manifest_file * +verify_manifest_entry(verifier_context *context, char *relpath, int64 filesize) +{ + manifest_file *m; + /* Check whether there's an entry in the manifest hash. */ m = manifest_files_lookup(context->manifest->files, relpath); if (m == NULL) @@ -634,40 +661,29 @@ verify_backup_file(verifier_context *context, char *relpath, char *fullpath) report_backup_error(context, "\"%s\" is present on disk but not in the manifest", relpath); - return; + return NULL; } /* Flag this entry as having been encountered in the filesystem. */ m->matched = true; /* Check that the size matches. */ - if (m->size != sb.st_size) + if (m->size != filesize) { report_backup_error(context, "\"%s\" has size %lld on disk but size %zu in the manifest", - relpath, (long long int) sb.st_size, m->size); + relpath, (long long int) filesize, m->size); m->bad = true; } - /* - * Validate the manifest system identifier, not available in manifest - * version 1. - */ - if (context->manifest->version != 1 && - strcmp(relpath, "global/pg_control") == 0) - verify_control_file(fullpath, context->manifest->system_identifier); - - /* Update statistics for progress report, if necessary */ - if (show_progress && !context->skip_checksums && - should_verify_checksum(m)) - total_size += m->size; - /* * We don't verify checksums at this stage. We first finish verifying that * we have the expected set of files with the expected sizes, and only * afterwards verify the checksums. That's because computing checksums may * take a while, and we'd like to report more obvious problems quickly. */ + + return m; } /* @@ -830,7 +846,7 @@ verify_file_checksum(verifier_context *context, manifest_file *m, /* * Double-check that we read the expected number of bytes from the file. - * Normally, a file size mismatch would be caught in verify_backup_file + * Normally, a file size mismatch would be caught in verify_manifest_entry * and this check would never be reached, but this provides additional * safety and clarity in the event of concurrent modifications or * filesystem misbehavior. diff --git a/src/bin/pg_verifybackup/pg_verifybackup.h b/src/bin/pg_verifybackup/pg_verifybackup.h index d8c566ed587..ff9476e356e 100644 --- a/src/bin/pg_verifybackup/pg_verifybackup.h +++ b/src/bin/pg_verifybackup/pg_verifybackup.h @@ -37,7 +37,8 @@ typedef struct manifest_file } manifest_file; #define should_verify_checksum(m) \ - (((m)->matched) && !((m)->bad) && (((m)->checksum_type) != CHECKSUM_TYPE_NONE)) + (((m) != NULL) && ((m)->matched) && !((m)->bad) && \ + (((m)->checksum_type) != CHECKSUM_TYPE_NONE)) /* * Define a hash table which we can use to store information about the files @@ -93,6 +94,9 @@ typedef struct verifier_context bool saw_any_error; } verifier_context; +extern manifest_file *verify_manifest_entry(verifier_context *context, + char *relpath, int64 filesize); + extern void report_backup_error(verifier_context *context, const char *pg_restrict fmt,...) pg_attribute_printf(2, 3); -- 2.18.0