[Issue #203] Add new pgBackup attribute 'content-crc', containing crc of backup_content.control file, to detect corruption

Author: gsmolk

src/backup.c

@@ -217,8 +217,6 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync)
 
 	if (prev_backup)
 	{
-		char		prev_backup_filelist_path[MAXPGPATH];
-
         if (parse_program_version(prev_backup->program_version) > parse_program_version(PROGRAM_VERSION))
             elog(ERROR, "pg_probackup binary version is %s, but backup %s version is %s. "
                         "pg_probackup do not guarantee to be forward compatible. "
@@ -227,10 +225,8 @@ do_backup_instance(PGconn *backup_conn, PGNodeInfo *nodeInfo, bool no_sync)
 
 		elog(INFO, "Parent backup: %s", base36enc(prev_backup->start_time));
 
-		join_path_components(prev_backup_filelist_path, prev_backup->root_dir,
-															DATABASE_FILE_LIST);
 		/* Files of previous backup needed by DELTA backup */
-		prev_backup_filelist = dir_read_file_list(NULL, NULL, prev_backup_filelist_path, FIO_BACKUP_HOST);
+		prev_backup_filelist = get_backup_filelist(prev_backup, true);
 
 		/* If lsn is not NULL, only pages with higher lsn will be copied. */
 		prev_backup_start_lsn = prev_backup->start_lsn;

src/catalog.c

@@ -538,17 +538,17 @@ catalog_get_backup_list(const char *instance_name, time_t requested_backup_id)
  * TODO this function only used once. Is it really needed?
  */
 parray *
-get_backup_filelist(pgBackup *backup)
+get_backup_filelist(pgBackup *backup, bool strict)
 {
 	parray		*files = NULL;
 	char		backup_filelist_path[MAXPGPATH];
 
 	join_path_components(backup_filelist_path, backup->root_dir, DATABASE_FILE_LIST);
-	files = dir_read_file_list(NULL, NULL, backup_filelist_path, FIO_BACKUP_HOST);
+	files = dir_read_file_list(NULL, NULL, backup_filelist_path, FIO_BACKUP_HOST, backup->content_crc);
 
 	/* redundant sanity? */
 	if (!files)
-		elog(ERROR, "Failed to get filelist for backup %s", base36enc(backup->start_time));
+		elog(strict ? ERROR : WARNING, "Failed to get file list for backup %s", base36enc(backup->start_time));
 
 	return files;
 }
@@ -1759,6 +1759,9 @@ pgBackupWriteControl(FILE *out, pgBackup *backup)
 	if (backup->note)
 		fio_fprintf(out, "note = '%s'\n", backup->note);
 
+	if (backup->content_crc != 0)
+		fio_fprintf(out, "content-crc = %u\n", backup->content_crc);
+
 }
 
 /*
@@ -1813,33 +1816,38 @@ write_backup_filelist(pgBackup *backup, parray *files, const char *root,
 					  parray *external_list, bool sync)
 {
 	FILE	   *out;
-	char		path[MAXPGPATH];
-	char		path_temp[MAXPGPATH];
+	char		control_path[MAXPGPATH];
+	char		control_path_temp[MAXPGPATH];
 	size_t		i = 0;
 	#define BUFFERSZ 1024*1024
 	char		*buf;
 	int64 		backup_size_on_disk = 0;
 	int64 		uncompressed_size_on_disk = 0;
 	int64 		wal_size_on_disk = 0;
 
-	join_path_components(path, backup->root_dir, DATABASE_FILE_LIST);
-	snprintf(path_temp, sizeof(path_temp), "%s.tmp", path);
+	join_path_components(control_path, backup->root_dir, DATABASE_FILE_LIST);
+	snprintf(control_path_temp, sizeof(control_path_temp), "%s.tmp", control_path);
 
-	out = fopen(path_temp, PG_BINARY_W);
+	out = fopen(control_path_temp, PG_BINARY_W);
 	if (out == NULL)
-		elog(ERROR, "Cannot open file list \"%s\": %s", path_temp,
+		elog(ERROR, "Cannot open file list \"%s\": %s", control_path_temp,
 			 strerror(errno));
 
-	if (chmod(path_temp, FILE_PERMISSION) == -1)
-		elog(ERROR, "Cannot change mode of \"%s\": %s", path_temp,
+	if (chmod(control_path_temp, FILE_PERMISSION) == -1)
+		elog(ERROR, "Cannot change mode of \"%s\": %s", control_path_temp,
 			 strerror(errno));
 
 	buf = pgut_malloc(BUFFERSZ);
 	setvbuf(out, buf, _IOFBF, BUFFERSZ);
 
+	if (sync)
+		INIT_FILE_CRC32(true, backup->content_crc);
+
 	/* print each file in the list */
 	for (i = 0; i < parray_num(files); i++)
 	{
+		int       len = 0;
+		char      line[BLCKSZ];
 		pgFile   *file = (pgFile *) parray_get(files, i);
 		char     *path = file->path; /* for streamed WAL files */
 
@@ -1873,7 +1881,7 @@ write_backup_filelist(pgBackup *backup, parray *files, const char *root,
 			(file->external_dir_num && external_list))
 				path = file->rel_path;
 
-		fprintf(out, "{\"path\":\"%s\", \"size\":\"" INT64_FORMAT "\", "
+		len = sprintf(line, "{\"path\":\"%s\", \"size\":\"" INT64_FORMAT "\", "
 					 "\"mode\":\"%u\", \"is_datafile\":\"%u\", "
 					 "\"is_cfs\":\"%u\", \"crc\":\"%u\", "
 					 "\"compress_alg\":\"%s\", \"external_dir_num\":\"%d\", "
@@ -1887,32 +1895,40 @@ write_backup_filelist(pgBackup *backup, parray *files, const char *root,
 					file->dbOid);
 
 		if (file->is_datafile)
-			fprintf(out, ",\"segno\":\"%d\"", file->segno);
+			len += sprintf(line+len, ",\"segno\":\"%d\"", file->segno);
 
 		if (file->linked)
-			fprintf(out, ",\"linked\":\"%s\"", file->linked);
+			len += sprintf(line+len, ",\"linked\":\"%s\"", file->linked);
 
 		if (file->n_blocks != BLOCKNUM_INVALID)
-			fprintf(out, ",\"n_blocks\":\"%i\"", file->n_blocks);
+			len += sprintf(line+len, ",\"n_blocks\":\"%i\"", file->n_blocks);
+
+		sprintf(line+len, "}\n");
 
-		fprintf(out, "}\n");
+		if (sync)
+			COMP_FILE_CRC32(true, backup->content_crc, line, strlen(line));
+
+		fprintf(out, "%s", line);
 	}
 
+	if (sync)
+		FIN_FILE_CRC32(true, backup->content_crc);
+
 	if (fflush(out) != 0)
 		elog(ERROR, "Cannot flush file list \"%s\": %s",
-			 path_temp, strerror(errno));
+			 control_path_temp, strerror(errno));
 
 	if (sync && fsync(fileno(out)) < 0)
 		elog(ERROR, "Cannot sync file list \"%s\": %s",
-			 path_temp, strerror(errno));
+			 control_path_temp, strerror(errno));
 
 	if (fclose(out) != 0)
 		elog(ERROR, "Cannot close file list \"%s\": %s",
-			 path_temp, strerror(errno));
+			 control_path_temp, strerror(errno));
 
-	if (rename(path_temp, path) < 0)
+	if (rename(control_path_temp, control_path) < 0)
 		elog(ERROR, "Cannot rename file \"%s\" to \"%s\": %s",
-			 path_temp, path, strerror(errno));
+			 control_path_temp, control_path, strerror(errno));
 
 	/* use extra variable to avoid reset of previous data_bytes value in case of error */
 	backup->data_bytes = backup_size_on_disk;
@@ -1975,6 +1991,7 @@ readBackupControlFile(const char *path)
 		{'s', 0, "primary-conninfo",	&backup->primary_conninfo, SOURCE_FILE_STRICT},
 		{'s', 0, "external-dirs",		&backup->external_dir_str, SOURCE_FILE_STRICT},
 		{'s', 0, "note",				&backup->note, SOURCE_FILE_STRICT},
+		{'u', 0, "content-crc",			&backup->content_crc, SOURCE_FILE_STRICT},
 		{0}
 	};
 
@@ -2242,6 +2259,7 @@ pgBackupInit(pgBackup *backup)
 	backup->root_dir = NULL;
 	backup->files = NULL;
 	backup->note = NULL;
+	backup->content_crc = 0;
 
 }
 

src/data.c

@@ -1462,7 +1462,7 @@ check_data_file(ConnectionArgs *arguments, pgFile *file,
 		 */
 		if (errno == ENOENT)
 		{
-			elog(LOG, "File \"%s\" is not found", file->path);
+			elog(LOG, "File \"%s\" is not found", from_fullpath);
 			return true;
 		}
 
@@ -1472,7 +1472,7 @@ check_data_file(ConnectionArgs *arguments, pgFile *file,
 	}
 
 	if (file->size % BLCKSZ != 0)
-		elog(WARNING, "File: \"%s\", invalid file size %zu", file->path, file->size);
+		elog(WARNING, "File: \"%s\", invalid file size %zu", from_fullpath, file->size);
 
 	/*
 	 * Compute expected number of blocks in the file.
@@ -1508,28 +1508,28 @@ check_data_file(ConnectionArgs *arguments, pgFile *file,
 
 /* Valiate pages of datafile in backup one by one */
 bool
-check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
-				 uint32 backup_version)
+check_file_pages(pgFile *file, const char *fullpath, XLogRecPtr stop_lsn,
+				 uint32 checksum_version, uint32 backup_version)
 {
 	size_t		read_len = 0;
 	bool		is_valid = true;
 	FILE		*in;
 	pg_crc32	crc;
 	bool		use_crc32c = backup_version <= 20021 || backup_version >= 20025;
 
-	elog(VERBOSE, "Validate relation blocks for file \"%s\"", file->path);
+	elog(VERBOSE, "Validate relation blocks for file \"%s\"", fullpath);
 
-	in = fopen(file->path, PG_BINARY_R);
+	in = fopen(fullpath, PG_BINARY_R);
 	if (in == NULL)
 	{
 		if (errno == ENOENT)
 		{
-			elog(WARNING, "File \"%s\" is not found", file->path);
+			elog(WARNING, "File \"%s\" is not found", fullpath);
 			return false;
 		}
 
 		elog(ERROR, "Cannot open file \"%s\": %s",
-			 file->path, strerror(errno));
+			 fullpath, strerror(errno));
 	}
 
 	/* calc CRC of backup file */
@@ -1553,7 +1553,7 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
 
 		if (ferror(in))
 			elog(ERROR, "Cannot read header of block %u of \"%s\": %s",
-					 blknum, file->path, strerror(errno));
+					 blknum, fullpath, strerror(errno));
 
 		if (read_len != sizeof(header))
 		{
@@ -1562,25 +1562,25 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
 			else if (read_len != 0 && feof(in))
 				elog(WARNING,
 					 "Odd size page found at block %u of \"%s\"",
-					 blknum, file->path);
+					 blknum, fullpath);
 			else
 				elog(WARNING, "Cannot read header of block %u of \"%s\": %s",
-					 blknum, file->path, strerror(errno));
+					 blknum, fullpath, strerror(errno));
 			return false;
 		}
 
 		COMP_FILE_CRC32(use_crc32c, crc, &header, read_len);
 
 		if (header.block == 0 && header.compressed_size == 0)
 		{
-			elog(VERBOSE, "Skip empty block of \"%s\"", file->path);
+			elog(VERBOSE, "Skip empty block of \"%s\"", fullpath);
 			continue;
 		}
 
 		if (header.block < blknum)
 		{
 			elog(WARNING, "Backup is broken at block %u of \"%s\"",
-				 blknum, file->path);
+				 blknum, fullpath);
 			return false;
 		}
 
@@ -1589,7 +1589,7 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
 		if (header.compressed_size == PageIsTruncated)
 		{
 			elog(LOG, "Block %u of \"%s\" is truncated",
-				 blknum, file->path);
+				 blknum, fullpath);
 			continue;
 		}
 
@@ -1600,7 +1600,7 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
 		if (read_len != MAXALIGN(header.compressed_size))
 		{
 			elog(WARNING, "Cannot read block %u of \"%s\" read %zu of %d",
-				blknum, file->path, read_len, header.compressed_size);
+				blknum, fullpath, read_len, header.compressed_size);
 			return false;
 		}
 
@@ -1620,7 +1620,7 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
 											  &errormsg);
 			if (uncompressed_size < 0 && errormsg != NULL)
 				elog(WARNING, "An error occured during decompressing block %u of file \"%s\": %s",
-					 blknum, file->path, errormsg);
+					 blknum, fullpath, errormsg);
 
 			if (uncompressed_size != BLCKSZ)
 			{
@@ -1630,7 +1630,7 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
 					continue;
 				}
 				elog(WARNING, "Page of file \"%s\" uncompressed to %d bytes. != BLCKSZ",
-					 file->path, uncompressed_size);
+						fullpath, uncompressed_size);
 				return false;
 			}
 
@@ -1676,7 +1676,7 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version,
 	if (crc != file->crc)
 	{
 		elog(WARNING, "Invalid CRC of backup file \"%s\": %X. Expected %X",
-				file->path, crc, file->crc);
+				fullpath, crc, file->crc);
 		is_valid = false;
 	}
 

src/dir.c

@@ -1512,12 +1512,13 @@ get_control_value(const char *str, const char *name,
  */
 parray *
 dir_read_file_list(const char *root, const char *external_prefix,
-				   const char *file_txt, fio_location location)
+				   const char *file_txt, fio_location location, pg_crc32 expected_crc)
 {
-	FILE   *fp;
-	parray *files;
-	char	buf[MAXPGPATH * 2];
-	char    stdio_buf[STDIO_BUFSIZE];
+	FILE    *fp;
+	parray  *files;
+	char     buf[BLCKSZ];
+	char     stdio_buf[STDIO_BUFSIZE];
+	pg_crc32 content_crc = 0;
 
 	fp = fio_open_stream(file_txt, location);
 	if (fp == NULL)
@@ -1529,6 +1530,8 @@ dir_read_file_list(const char *root, const char *external_prefix,
 
 	files = parray_new();
 
+	INIT_FILE_CRC32(true, content_crc);
+
 	while (fgets(buf, lengthof(buf), fp))
 	{
 		char		path[MAXPGPATH];
@@ -1546,6 +1549,8 @@ dir_read_file_list(const char *root, const char *external_prefix,
 					dbOid;		/* used for partial restore */
 		pgFile	   *file;
 
+		COMP_FILE_CRC32(true, content_crc, buf, strlen(buf));
+
 		get_control_value(buf, "path", path, NULL, true);
 		get_control_value(buf, "size", NULL, &write_size, true);
 		get_control_value(buf, "mode", NULL, &mode, true);
@@ -1598,10 +1603,21 @@ dir_read_file_list(const char *root, const char *external_prefix,
 		parray_append(files, file);
 	}
 
+	FIN_FILE_CRC32(true, content_crc);
+
 	if (ferror(fp))
 		elog(ERROR, "Failed to read from file: \"%s\"", file_txt);
 
 	fio_close_stream(fp);
+
+	if (expected_crc != 0 &&
+		expected_crc != content_crc)
+	{
+		elog(WARNING, "Invalid CRC of backup control file '%s': %u. Expected: %u",
+				file_txt, content_crc, expected_crc);
+		return NULL;
+	}
+
 	return files;
 }
 

src/merge.c

@@ -565,13 +565,9 @@ merge_chain(parray *parent_chain, pgBackup *full_backup, pgBackup *dest_backup)
 	 */
 	for (i = parray_num(parent_chain) - 1; i >= 0; i--)
 	{
-		char		control_file[MAXPGPATH];
-
 		pgBackup   *backup = (pgBackup *) parray_get(parent_chain, i);
 
-		join_path_components(control_file, backup->root_dir, DATABASE_FILE_LIST);
-		backup->files = dir_read_file_list(NULL, NULL, control_file, FIO_BACKUP_HOST);
-
+		backup->files = get_backup_filelist(backup, true);
 		parray_qsort(backup->files, pgFileCompareRelPathWithExternal);
 
 		/* Set MERGING status for every member of the chain */