* [PATCH 1/3] backup: Use filename as specified on console @ 2021-12-02 12:37 Michael Tremer 2021-12-02 12:37 ` [PATCH 2/3] backup: Create tarball in one pass Michael Tremer 2021-12-02 12:37 ` [PATCH 3/3] backup: Fork ISO job into the background in CGI script Michael Tremer 0 siblings, 2 replies; 7+ messages in thread From: Michael Tremer @ 2021-12-02 12:37 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 480 bytes --] Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org> --- config/backup/backup.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/backup/backup.pl b/config/backup/backup.pl index 8dffbf2e1..0b47af2d6 100644 --- a/config/backup/backup.pl +++ b/config/backup/backup.pl @@ -238,7 +238,7 @@ main() { filename="/tmp/restore.ipf" fi - restore_backup "/tmp/restore.ipf" + restore_backup "${filename}" ;; addonbackup) -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] backup: Create tarball in one pass 2021-12-02 12:37 [PATCH 1/3] backup: Use filename as specified on console Michael Tremer @ 2021-12-02 12:37 ` Michael Tremer 2021-12-02 13:42 ` Adolf Belka 2021-12-02 12:37 ` [PATCH 3/3] backup: Fork ISO job into the background in CGI script Michael Tremer 1 sibling, 1 reply; 7+ messages in thread From: Michael Tremer @ 2021-12-02 12:37 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 1858 bytes --] 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 <michael.tremer(a)ipfire.org> --- 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="/var/ipfire/backup/exclude" \ - $(process_includes "/var/ipfire/backup/include") \ - "$@" - - # Backup using user exclude/include definitions and append to global backup - tar rvf "${filename}" \ --exclude-from="/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="${1}" if [ -z "${filename}" ]; then - filename="/var/ipfire/backup/${NOW}" + filename="/var/ipfire/backup/${NOW}.ipf" fi make_backup "${filename}" $(find_logfiles) @@ -225,7 +218,7 @@ main() { local filename="${1}" if [ -z "${filename}" ]; then - filename="/var/ipfire/backup/${NOW}" + filename="/var/ipfire/backup/${NOW}.ipf" fi make_backup "${filename}" -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] backup: Create tarball in one pass 2021-12-02 12:37 ` [PATCH 2/3] backup: Create tarball in one pass Michael Tremer @ 2021-12-02 13:42 ` Adolf Belka 2021-12-02 15:48 ` Michael Tremer 0 siblings, 1 reply; 7+ messages in thread From: Adolf Belka @ 2021-12-02 13:42 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 2738 bytes --] Hi Michael, I split the tar into two stages in May of this year as a fix for Bug #12626. 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 --exclude-from has precedence over any file lists at the end of the command. If this patch is implemented then I would edit the wiki to specify that user 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 not have a .tmp extension. 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 <michael.tremer(a)ipfire.org> > --- > 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="/var/ipfire/backup/exclude" \ > - $(process_includes "/var/ipfire/backup/include") \ > - "$@" > - > - # Backup using user exclude/include definitions and append to global backup > - tar rvf "${filename}" \ > --exclude-from="/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="${1}" > > if [ -z "${filename}" ]; then > - filename="/var/ipfire/backup/${NOW}" > + filename="/var/ipfire/backup/${NOW}.ipf" > fi > > make_backup "${filename}" $(find_logfiles) > @@ -225,7 +218,7 @@ main() { > local filename="${1}" > > if [ -z "${filename}" ]; then > - filename="/var/ipfire/backup/${NOW}" > + filename="/var/ipfire/backup/${NOW}.ipf" > fi > > make_backup "${filename}" -- Sent from my laptop ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] backup: Create tarball in one pass 2021-12-02 13:42 ` Adolf Belka @ 2021-12-02 15:48 ` Michael Tremer 2021-12-03 13:54 ` Adolf Belka 0 siblings, 1 reply; 7+ messages in thread From: Michael Tremer @ 2021-12-02 15:48 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 3361 bytes --] Hello Adolf, Thank you for reviewing my patch :) > On 2 Dec 2021, at 13:42, Adolf Belka <adolf.belka(a)ipfire.org> wrote: > > Hi Michael, > > I split the tar into two stages in May of this year as a fix for Bug #12626. > > 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 --exclude-from has precedence over any file lists at the end of the command. I should have reached out because it wasn’t quite clear to me why this was split into two operations. I would have considered this the expected behaviour, because what is excluded is excluded. Clearly you expected something else to happen which I would also consider a valid expectation. > If this patch is implemented then I would edit the wiki to specify that user 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 not have a .tmp extension. Is there any use-case where this would become a big problem? Is there anything the user would include that has previously been globally excluded? -Michael > > > 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 <michael.tremer(a)ipfire.org> >> --- >> 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="/var/ipfire/backup/exclude" \ >> - $(process_includes "/var/ipfire/backup/include") \ >> - "$@" >> - >> - # Backup using user exclude/include definitions and append to global backup >> - tar rvf "${filename}" \ >> --exclude-from="/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="${1}" >> if [ -z "${filename}" ]; then >> - filename="/var/ipfire/backup/${NOW}" >> + filename="/var/ipfire/backup/${NOW}.ipf" >> fi >> make_backup "${filename}" $(find_logfiles) >> @@ -225,7 +218,7 @@ main() { >> local filename="${1}" >> if [ -z "${filename}" ]; then >> - filename="/var/ipfire/backup/${NOW}" >> + filename="/var/ipfire/backup/${NOW}.ipf" >> fi >> make_backup "${filename}" > > -- > Sent from my laptop ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] backup: Create tarball in one pass 2021-12-02 15:48 ` Michael Tremer @ 2021-12-03 13:54 ` Adolf Belka 2021-12-06 18:40 ` Paul Simmons 0 siblings, 1 reply; 7+ messages in thread From: Adolf Belka @ 2021-12-03 13:54 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 4537 bytes --] Hi Michael, On 02/12/2021 16:48, Michael Tremer wrote: > Hello Adolf, > > Thank you for reviewing my patch :) > >> On 2 Dec 2021, at 13:42, Adolf Belka <adolf.belka(a)ipfire.org> wrote: >> >> Hi Michael, >> >> I split the tar into two stages in May of this year as a fix for Bug #12626. >> >> 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 --exclude-from has precedence over any file lists at the end of the command. > > I should have reached out because it wasn’t quite clear to me why this was split into two operations. > > I would have considered this the expected behaviour, because what is excluded is excluded. Clearly you expected something else to happen which I would also consider a valid expectation. It wasn't me that highlighted that effect but Paul Simmons in Bug 11320 - Umbrella bug for backup system, comment 1. I took that and created Bug 12626. I have not had any problems myself with the user backup as it was previously running. > >> If this patch is implemented then I would edit the wiki to specify that user 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 not have a .tmp extension. > > Is there any use-case where this would become a big problem? Is there anything the user would include that has previously been globally excluded? This question would need to be asked to Paul Simmons as he raised the bug in the first case. I don't know if he is included in the dewvlopment(a)lists.ipfire.org, if not then the simplest would be to add 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 and the amount of disk space used with the temporary storage of the tar files instead of going direct to gzip would seem to be good reasons to revert the change I did anyway and then later, depending on the response from Paul Simmons, we can discuss further if a use case that needs to be addressed is identified. Does that seem a reasonable approach to this. Regards, Adolf. > > -Michael > >> >> >> 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 <michael.tremer(a)ipfire.org> >>> --- >>> 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="/var/ipfire/backup/exclude" \ >>> - $(process_includes "/var/ipfire/backup/include") \ >>> - "$@" >>> - >>> - # Backup using user exclude/include definitions and append to global backup >>> - tar rvf "${filename}" \ >>> --exclude-from="/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="${1}" >>> if [ -z "${filename}" ]; then >>> - filename="/var/ipfire/backup/${NOW}" >>> + filename="/var/ipfire/backup/${NOW}.ipf" >>> fi >>> make_backup "${filename}" $(find_logfiles) >>> @@ -225,7 +218,7 @@ main() { >>> local filename="${1}" >>> if [ -z "${filename}" ]; then >>> - filename="/var/ipfire/backup/${NOW}" >>> + filename="/var/ipfire/backup/${NOW}.ipf" >>> fi >>> make_backup "${filename}" >> >> -- >> Sent from my laptop > -- Sent from my laptop ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] backup: Create tarball in one pass 2021-12-03 13:54 ` Adolf Belka @ 2021-12-06 18:40 ` Paul Simmons 0 siblings, 0 replies; 7+ messages in thread From: Paul Simmons @ 2021-12-06 18:40 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 5423 bytes --] On 12/3/21 07:54, Adolf Belka wrote: > Hi Michael, > > On 02/12/2021 16:48, Michael Tremer wrote: >> Hello Adolf, >> >> Thank you for reviewing my patch :) >> >>> On 2 Dec 2021, at 13:42, Adolf Belka <adolf.belka(a)ipfire.org> wrote: >>> >>> Hi Michael, >>> >>> I split the tar into two stages in May of this year as a fix for Bug >>> #12626. >>> >>> 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 --exclude-from has precedence over any file lists at >>> the end of the command. >> >> I should have reached out because it wasn’t quite clear to me why >> this was split into two operations. >> >> I would have considered this the expected behaviour, because what is >> excluded is excluded. Clearly you expected something else to happen >> which I would also consider a valid expectation. > It wasn't me that highlighted that effect but Paul Simmons in Bug > 11320 - Umbrella bug for backup system, comment 1. I took that and > created Bug 12626. I have not had any problems myself with the user > backup as it was previously running. >> >>> If this patch is implemented then I would edit the wiki to specify >>> that user 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 not have a .tmp extension. >> >> Is there any use-case where this would become a big problem? Is there >> anything the user would include that has previously been globally >> excluded? > This question would need to be asked to Paul Simmons as he raised the > bug in the first case. I don't know if he is included in the > dewvlopment(a)lists.ipfire.org, if not then the simplest would be to add > 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 > and the amount of disk space used with the temporary storage of the > tar files instead of going direct to gzip would seem to be good > reasons to revert the change I did anyway and then later, depending on > the response from Paul Simmons, we can discuss further if a use case > that needs to be addressed is identified. > > Does that seem a reasonable approach to this. > > Regards, > Adolf. >> >> -Michael >> >>> >>> >>> 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 <michael.tremer(a)ipfire.org> >>>> --- >>>> 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="/var/ipfire/backup/exclude" \ >>>> - $(process_includes "/var/ipfire/backup/include") \ >>>> - "$@" >>>> - >>>> - # Backup using user exclude/include definitions and append to >>>> global backup >>>> - tar rvf "${filename}" \ >>>> --exclude-from="/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="${1}" >>>> if [ -z "${filename}" ]; then >>>> - filename="/var/ipfire/backup/${NOW}" >>>> + filename="/var/ipfire/backup/${NOW}.ipf" >>>> fi >>>> make_backup "${filename}" $(find_logfiles) >>>> @@ -225,7 +218,7 @@ main() { >>>> local filename="${1}" >>>> if [ -z "${filename}" ]; then >>>> - filename="/var/ipfire/backup/${NOW}" >>>> + filename="/var/ipfire/backup/${NOW}.ipf" >>>> fi >>>> make_backup "${filename}" >>> >>> -- >>> Sent from my laptop >> > Howdy, y'all. I have an alternate solution for my use case. I've created a pseudo addon backup file which includes files that were omitted by the original global include / exclude evaluations. Thank you, Paul Simmons ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] backup: Fork ISO job into the background in CGI script 2021-12-02 12:37 [PATCH 1/3] backup: Use filename as specified on console Michael Tremer 2021-12-02 12:37 ` [PATCH 2/3] backup: Create tarball in one pass Michael Tremer @ 2021-12-02 12:37 ` Michael Tremer 1 sibling, 0 replies; 7+ messages in thread From: Michael Tremer @ 2021-12-02 12:37 UTC (permalink / raw) To: development [-- Attachment #1: Type: text/plain, Size: 1349 bytes --] In order to be able to run the ISO command on command line it is helpful that the script does not go into background halfway through the process. We should rather start it as a background job straight from the CGI script. Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org> --- config/backup/backup.pl | 2 +- html/cgi-bin/backup.cgi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/backup/backup.pl b/config/backup/backup.pl index bed5952de..afd8d1663 100644 --- a/config/backup/backup.pl +++ b/config/backup/backup.pl @@ -247,7 +247,7 @@ main() { local filename="/var/ipfire/backup/${NOW}.ipf" if make_backup "${filename}"; then - /usr/local/bin/backupiso "${NOW}" & + /usr/local/bin/backupiso "${NOW}" fi ;; diff --git a/html/cgi-bin/backup.cgi b/html/cgi-bin/backup.cgi index 84c015314..1b1ccb4a8 100644 --- a/html/cgi-bin/backup.cgi +++ b/html/cgi-bin/backup.cgi @@ -119,7 +119,7 @@ if ( $cgiparams{'ACTION'} eq "backup" ) } elsif ( $cgiparams{'BACKUPLOGS'} eq "exclude" ) { &General::system("/usr/local/bin/backupctrl", "exclude"); } elsif ( $cgiparams{'BACKUPLOGS'} eq "iso" ) { - &General::system("/usr/local/bin/backupctrl", "iso"); + &General::system_background("/usr/local/bin/backupctrl", "iso"); } } if ( $cgiparams{'ACTION'} eq "addonbackup" ) -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-06 18:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-02 12:37 [PATCH 1/3] backup: Use filename as specified on console Michael Tremer 2021-12-02 12:37 ` [PATCH 2/3] backup: Create tarball in one pass Michael Tremer 2021-12-02 13:42 ` Adolf Belka 2021-12-02 15:48 ` Michael Tremer 2021-12-03 13:54 ` Adolf Belka 2021-12-06 18:40 ` Paul Simmons 2021-12-02 12:37 ` [PATCH 3/3] backup: Fork ISO job into the background in CGI script Michael Tremer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox