Incorrect fsync handling in pg_basebackup's tar_finish

[Michael Paquier]

Hi all,

I was just looking at the code of pg_basebackup, and noticed that we
don't actually check if the two last empty blocks of any tar file
produced are correctly fsync'd or not:
@@ -957,7 +957,10 @@ tar_finish(void)

 /* sync the empty blocks as well, since they're after the last file */
 if (tar_data->sync)
-   fsync(tar_data->fd);
+   {
+       if (fsync(tar_data->fd) != 0)
+           return false;
+   }

That looks incorrect to me, hence shouldn't something like the attached
be done?  Magnus and others, any opinions?

Thanks,
--
Michael

diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index 331d0e7275..7867a56ee1 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -896,7 +896,7 @@ tar_finish(void)
 			return false;
 	}
 
-	/* A tarfile always ends with two empty  blocks */
+	/* A tarfile always ends with two empty blocks */
 	MemSet(zerobuf, 0, sizeof(zerobuf));
 	if (!tar_data->compression)
 	{
@@ -957,7 +957,10 @@ tar_finish(void)
 
 	/* sync the empty blocks as well, since they're after the last file */
 	if (tar_data->sync)
-		fsync(tar_data->fd);
+	{
+		if (fsync(tar_data->fd) != 0)
+			return false;
+	}
 
 	if (close(tar_data->fd) != 0)
 		return false;

signature.asc
Description: PGP signature