Re: Add system identifier to backup manifest

On Fri, Mar 1, 2024 at 11:28 AM Michael Paquier <michael@paquier.xyz> wrote:

> On Mon, Feb 19, 2024 at 12:06:19PM +0530, Amul Sul wrote:
> > Agreed, now they will have an error as _could not read file "<DataDir>":
> Is
> > a directory_. But, IIUC, that what usually happens with the dev version,
> and
> > the extension needs to be updated for compatibility with the newer
> version for
> > the same reason.
>
> I was reading through the remaining pieces of the patch set, and are
> you sure that there is a need for 0002 at all?  The only reason why
> get_dir_controlfile() is introduced is to be able to get the contents
> of a control file with a full path to it, and not a data folder.  Now,
> if I look closely, with 0002~0004 applied, the only two callers of
> get_controlfile() are pg_combinebackup.c and pg_verifybackup.c.  Both
> of them have an access to the backup directories, which point to the
> root of the data folder.  pg_combinebackup can continue to use
> backup_dirs[i].  pg_verifybackup has an access to the backup directory
> in the context data, if I'm reading this right, so you could just use
> that in verify_system_identifier().
>

Yes, you are correct. Both the current caller of get_controlfile() has
access to the root directory.

I have dropped the 0002 patch -- I don't have a very strong opinion to
refactor
get_controlfile() apart from saying that it might be good to have both
versions :) .

Regards,
Amul