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)
