0001 seems mostly OK, but I don't like some of these new function names. I see you've named them so that they are case-consistent with the name of the struct member that they affect, but I don't think that's a good criterion. I propose
SetrunningBackups -> XLogBackupSetRunning() ResetXLogBackupActivity -> XLogBackupNotRunning() // or maybe SetNotRunning, or ResetRunning? I prefer the one above SetlastBackupStart -> XLogBackupSetLastStart() GetlastFpwDisableRecPtr -> XLogGetLastFPWDisableRecptr() GetminRecoveryPoint -> XLogGetMinRecoveryPoint() I wouldn't say in the xlog_internal.h comment that these new functions are for xlogbackup.c to use. The API definition doesn't have to concern itself with that. Maybe one day xlogrecovery.c or some other xlog*.c would like to call those functions, and then the comment becomes a lie; and what for? 0002 is where the interesting stuff happens. I have not reviewed that part with any care, but it appears that set_backup_state is pretty much useless. Let's get rid of it instead of moving it. Which also means that we shouldn't introduce reset_backup_status in 0001, I suppose. I think xlogfuncs.c is content with having just get_backup_status(). Speaking of which -- I'm not sure we really want to do 0003. xlogfuncs.c is not a big file, the functions are not complex, and there are no interesting interactions in those functions with the internals (other than get_backup_status). I see that Michael advised the same. I propose we keep those functions where they are. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "I suspect most samba developers are already technically insane... Of course, since many of them are Australians, you can't tell." (L. Torvalds)
