Re: block-level incremental backup

Here are some comments:

Or maybe we can just say:

"cannot verify checksum in file \"%s\"" if checksum requested, disable the

checksum and leave it to the following message:

+           ereport(WARNING,

+                   (errmsg("file size (%d) not in multiple of page size (%d), sending whole file",

+                           (int) cnt, BLCKSZ))); 

I think we should give the user hint from where he should be reading the input

lsn for incremental backup in the --help option as well as documentation?

Something like - "To take an incremental backup, please provide value of "--lsn"

as the "START WAL LOCATION" of previously taken full backup or incremental

backup from backup_lable file. 

pg_combinebackup:

+static bool made_new_outputdata = false;

+static bool found_existing_outputdata = false;

Both of these are global, I understand that we need them global so that they are

accessible in cleanup_directories_atexit(). But they are passed to

verify_dir_is_empty_or_create() as parameters, which I think is not needed.

Instead verify_dir_is_empty_or_create() can directly change the globals.

The current logic assumes the incremental backup directories are to be provided

as input in the serial order the backups were taken. This is bit confusing

unless clarified in pg_combinebackup help menu or documentation. I think we

should clarify it at both the places.

I think scan_directory() should be rather renamed as do_combinebackup().