From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adolf Belka To: development@lists.ipfire.org Subject: Re: [PATCH 2/3] backup: Create tarball in one pass Date: Fri, 03 Dec 2021 14:54:05 +0100 Message-ID: <0d5f8563-2023-6a78-a197-dfcf9ed6095c@ipfire.org> In-Reply-To: <3BCAA893-D04F-4688-880B-C925843EC2B6@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5931239946386166064==" List-Id: --===============5931239946386166064== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Michael, On 02/12/2021 16:48, Michael Tremer wrote: > Hello Adolf, >=20 > Thank you for reviewing my patch :) >=20 >> On 2 Dec 2021, at 13:42, Adolf Belka wrote: >> >> Hi Michael, >> >> I split the tar into two stages in May of this year as a fix for Bug #1262= 6. >> >> If the tar/gzip is done in one go then any files in the include.user file = that match a pattern in the global exclude will always be ignored as the --ex= clude-from has precedence over any file lists at the end of the command. >=20 > I should have reached out because it wasn=E2=80=99t quite clear to me why t= his was split into two operations. >=20 > I would have considered this the expected behaviour, because what is exclud= ed is excluded. Clearly you expected something else to happen which I would a= lso consider a valid expectation. It wasn't me that highlighted that effect but Paul Simmons in Bug 11320=20 - Umbrella bug for backup system, comment 1. I took that and created Bug=20 12626. I have not had any problems myself with the user backup as it was=20 previously running. >=20 >> If this patch is implemented then I would edit the wiki to specify that us= er files, wanting to be backed up by adding to the include.user file, should = not be in any of the directories listed in the global exclude file and must n= ot have a .tmp extension. >=20 > Is there any use-case where this would become a big problem? Is there anyth= ing the user would include that has previously been globally excluded? This question would need to be asked to Paul Simmons as he raised the=20 bug in the first case. I don't know if he is included in the=20 dewvlopment(a)lists.ipfire.org, if not then the simplest would be to add=20 his name to this email series and see if we get a response. The issues that have been raised of the backup in the iso not working=20 and the amount of disk space used with the temporary storage of the tar=20 files instead of going direct to gzip would seem to be good reasons to=20 revert the change I did anyway and then later, depending on the response=20 from Paul Simmons, we can discuss further if a use case that needs to be=20 addressed is identified. Does that seem a reasonable approach to this. Regards, Adolf. >=20 > -Michael >=20 >> >> >> Regards, >> >> Adolf. >> >> >> On 02/12/2021 13:37, Michael Tremer wrote: >>> This patch is changing the behaviour of the backup script so that it >>> creates one tarball and compresses it in one go. >>> >>> This will save storing the original tarball on disk before compressing >>> it which on my test system requires significant disk space. >>> >>> This patch also solves a bug where the backup file included with the ISO >>> image could not be extracted because it was not gzip-compressed when it >>> was expected to be. >>> >>> Signed-off-by: Michael Tremer >>> --- >>> config/backup/backup.pl | 15 ++++----------- >>> 1 file changed, 4 insertions(+), 11 deletions(-) >>> >>> diff --git a/config/backup/backup.pl b/config/backup/backup.pl >>> index 0b47af2d6..bed5952de 100644 >>> --- a/config/backup/backup.pl >>> +++ b/config/backup/backup.pl >>> @@ -58,20 +58,13 @@ make_backup() { >>> done >>> # Backup using global exclude/include definitions >>> - tar cvf "${filename}" \ >>> + tar cvfz "${filename}" \ >>> --exclude-from=3D"/var/ipfire/backup/exclude" \ >>> - $(process_includes "/var/ipfire/backup/include") \ >>> - "$@" >>> - >>> - # Backup using user exclude/include definitions and append to global ba= ckup >>> - tar rvf "${filename}" \ >>> --exclude-from=3D"/var/ipfire/backup/exclude.user" \ >>> + $(process_includes "/var/ipfire/backup/include") \ >>> $(process_includes "/var/ipfire/backup/include.user") \ >>> "$@" >>> - # gzip the combined global/user backup and use .ipf suffix >>> - gzip --suffix .ipf "${filename}" >>> - >>> return 0 >>> } >>> @@ -215,7 +208,7 @@ main() { >>> local filename=3D"${1}" >>> if [ -z "${filename}" ]; then >>> - filename=3D"/var/ipfire/backup/${NOW}" >>> + filename=3D"/var/ipfire/backup/${NOW}.ipf" >>> fi >>> make_backup "${filename}" $(find_logfiles) >>> @@ -225,7 +218,7 @@ main() { >>> local filename=3D"${1}" >>> if [ -z "${filename}" ]; then >>> - filename=3D"/var/ipfire/backup/${NOW}" >>> + filename=3D"/var/ipfire/backup/${NOW}.ipf" >>> fi >>> make_backup "${filename}" >> >> --=20 >> Sent from my laptop >=20 --=20 Sent from my laptop --===============5931239946386166064==--