[PATCH] base-files: sysupgrade: store status of system-services

2021-01-03 Thread Sven Roederer
When saving the list of installed pkgs, also store the status of the
system services. The list is created in the etc/backup folder also
and formated as:

/etc/init.d/ {enable|disable}

This way it can be sourced after sysupgrade, to restore the previous
state.

Signed-off-by: Sven Roederer 
---

Currently all services get enabled during image creation. This can cause
issues after upgrade with services explicitly disabled by the user.
With this created list sourced by a simple uci-defaults script the state
can be restored automatically.
Not including such a uci-defaults script by default, as currently the
stored package list is also not reinstalled.


 package/base-files/Makefile  |  2 +-
 package/base-files/files/sbin/sysupgrade | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/package/base-files/Makefile b/package/base-files/Makefile
index 0c612b73ca..fbcb694592 100644
--- a/package/base-files/Makefile
+++ b/package/base-files/Makefile
@@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/version.mk
 include $(INCLUDE_DIR)/feeds.mk
 
 PKG_NAME:=base-files
-PKG_RELEASE:=239
+PKG_RELEASE:=240
 PKG_FLAGS:=nonshared
 
 PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/ $(GENERIC_PLATFORM_DIR)/base-files/
diff --git a/package/base-files/files/sbin/sysupgrade 
b/package/base-files/files/sbin/sysupgrade
index 79927a2b5c..cadce36172 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -57,6 +57,7 @@ export CONFFILES=/tmp/sysupgrade.conffiles
 export CONF_TAR=/tmp/sysupgrade.tgz
 export ETCBACKUP_DIR=/etc/backup
 export INSTALLED_PACKAGES=${ETCBACKUP_DIR}/installed_packages.txt
+export SERVICE_STATUS=${ETCBACKUP_DIR}/service_status.txt
 
 IMAGE="$1"
 
@@ -228,6 +229,7 @@ do_save_conffiles() {
 
if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
+   echo "${SERVICE_STATUS}" >> "$CONFFILES"
mkdir -p "$ETCBACKUP_DIR"
# Avoid touching filesystem on each backup
RAMFS="$(mktemp -d -t sysupgrade.XX)"
@@ -245,6 +247,15 @@ do_save_conffiles() {
\( -exec test -f /overlay/upper/{} \; -exec echo {} 
overlay \; \) -o \
\( -exec echo {} unknown \; \) \
\) | sed -e 's,.*/,,;s/\.control /\t/' > 
${INSTALLED_PACKAGES}
+
+   # Format: /etc/init.d/servicename {enable,disable}
+   rm -f ${SERVICE_STATUS}
+   for service in /etc/init.d/* ; do \
+   ${service} enabled && \
+   echo >> ${SERVICE_STATUS} "$service" "enable" 
|| \
+   echo >> ${SERVICE_STATUS} "$service" "disable" \
+   ; \
+   done
fi
 
v "Saving config files..."
-- 
2.20.1


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-09 Thread Stijn Segers

Hi,

Op zondag 3 januari 2021 om 23u14 schreef Sven Roederer 
:

When saving the list of installed pkgs, also store the status of the
system services. The list is created in the etc/backup folder also
and formated as:

/etc/init.d/ {enable|disable}

This way it can be sourced after sysupgrade, to restore the previous
state.

Signed-off-by: Sven Roederer 
---

Currently all services get enabled during image creation. This can 
cause

issues after upgrade with services explicitly disabled by the user.
With this created list sourced by a simple uci-defaults script the 
state

can be restored automatically.
Not including such a uci-defaults script by default, as currently the
stored package list is also not reinstalled.


 package/base-files/Makefile  |  2 +-
 package/base-files/files/sbin/sysupgrade | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/package/base-files/Makefile b/package/base-files/Makefile
index 0c612b73ca..fbcb694592 100644
--- a/package/base-files/Makefile
+++ b/package/base-files/Makefile
@@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/version.mk
 include $(INCLUDE_DIR)/feeds.mk

 PKG_NAME:=base-files
-PKG_RELEASE:=239
+PKG_RELEASE:=240
 PKG_FLAGS:=nonshared

 PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/ 
$(GENERIC_PLATFORM_DIR)/base-files/
diff --git a/package/base-files/files/sbin/sysupgrade 
b/package/base-files/files/sbin/sysupgrade

index 79927a2b5c..cadce36172 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -57,6 +57,7 @@ export CONFFILES=/tmp/sysupgrade.conffiles
 export CONF_TAR=/tmp/sysupgrade.tgz
 export ETCBACKUP_DIR=/etc/backup
 export INSTALLED_PACKAGES=${ETCBACKUP_DIR}/installed_packages.txt
+export SERVICE_STATUS=${ETCBACKUP_DIR}/service_status.txt

 IMAGE="$1"

@@ -228,6 +229,7 @@ do_save_conffiles() {

if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
+   echo "${SERVICE_STATUS}" >> "$CONFFILES"
mkdir -p "$ETCBACKUP_DIR"



Am I reading this correctly and is this only keeping track of service 
status if you tell sysupgrade to save packages? What's the rationale 
behind that?


I have a personal build with all packages preinstalled, so I don't need 
that. Would like to keep track of service status though. Can those two 
things be entangled?


Cheers

Stijn


# Avoid touching filesystem on each backup
RAMFS="$(mktemp -d -t sysupgrade.XX)"
@@ -245,6 +247,15 @@ do_save_conffiles() {
 			\( -exec test -f /overlay/upper/{} \; -exec echo {} overlay \; \) 
-o \

\( -exec echo {} unknown \; \) \
\) | sed -e 's,.*/,,;s/\.control /\t/' > 
${INSTALLED_PACKAGES}
+
+   # Format: /etc/init.d/servicename {enable,disable}
+   rm -f ${SERVICE_STATUS}
+   for service in /etc/init.d/* ; do \
+   ${service} enabled && \
+   echo >> ${SERVICE_STATUS} "$service" "enable" 
|| \
+   echo >> ${SERVICE_STATUS} "$service" "disable" \
+   ; \
+   done
fi

v "Saving config files..."
--
2.20.1


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel




___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-09 Thread Reiner Karlsberg

Am 09.01.2021 um 13:28 schrieb Stijn Segers:

> Currently all services get enabled during image creation. This can cause
> issues after upgrade with services explicitly disabled by the user.
> With this created list sourced by a simple uci-defaults script the state
> can be restored automatically.

Pls, do _NOT_ make this default.
There might be other entities, besides me, who already have
implemented methods to work around unnecessary enabled services.
I.e. to disable service in uci-defaults, like I do.
Auto-restore service state will break existing code.

In general, I do _NOT_ consider it a good idea, to more or less
silently modify existing functionality.
Which happems often enough in openwrt, unfortunately.

Best would be, to make this new behaviour optional. Do another change for LuCI, 
and then this is done.

In worst case, pls provide an option for sysupgrade _NOT_
to keep service state.

Cheers,

Reiner


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-09 Thread Daniel Golle
On Sat, Jan 09, 2021 at 01:56:13PM +0200, Reiner Karlsberg wrote:
> Am 09.01.2021 um 13:28 schrieb Stijn Segers:
> 
> > Currently all services get enabled during image creation. This can cause
> > issues after upgrade with services explicitly disabled by the user.
> > With this created list sourced by a simple uci-defaults script the state
> > can be restored automatically.
> 
> Pls, do _NOT_ make this default.
> There might be other entities, besides me, who already have
> implemented methods to work around unnecessary enabled services.
> I.e. to disable service in uci-defaults, like I do.
> Auto-restore service state will break existing code.

Also note that ImageBuilder allows to generate images having
certain services installed but disabled using the
DISABLED_SERVICES=" ..." parameter.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-09 Thread Paul Spooren




On So, Jan 3, 2021 at 23:14, Sven Roederer  
wrote:

When saving the list of installed pkgs, also store the status of the
system services. The list is created in the etc/backup folder also
and formated as:

/etc/init.d/ {enable|disable}

This way it can be sourced after sysupgrade, to restore the previous
state.

Signed-off-by: Sven Roederer 
---

Currently all services get enabled during image creation. This can 
cause

issues after upgrade with services explicitly disabled by the user.
With this created list sourced by a simple uci-defaults script the 
state

can be restored automatically.
Not including such a uci-defaults script by default, as currently the
stored package list is also not reinstalled.


 package/base-files/Makefile  |  2 +-
 package/base-files/files/sbin/sysupgrade | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/package/base-files/Makefile b/package/base-files/Makefile
index 0c612b73ca..fbcb694592 100644
--- a/package/base-files/Makefile
+++ b/package/base-files/Makefile
@@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/version.mk
 include $(INCLUDE_DIR)/feeds.mk

 PKG_NAME:=base-files
-PKG_RELEASE:=239
+PKG_RELEASE:=240
 PKG_FLAGS:=nonshared

 PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/ 
$(GENERIC_PLATFORM_DIR)/base-files/
diff --git a/package/base-files/files/sbin/sysupgrade 
b/package/base-files/files/sbin/sysupgrade

index 79927a2b5c..cadce36172 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -57,6 +57,7 @@ export CONFFILES=/tmp/sysupgrade.conffiles
 export CONF_TAR=/tmp/sysupgrade.tgz
 export ETCBACKUP_DIR=/etc/backup
 export INSTALLED_PACKAGES=${ETCBACKUP_DIR}/installed_packages.txt
+export SERVICE_STATUS=${ETCBACKUP_DIR}/service_status.txt

 IMAGE="$1"

@@ -228,6 +229,7 @@ do_save_conffiles() {

if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
+   echo "${SERVICE_STATUS}" >> "$CONFFILES"
mkdir -p "$ETCBACKUP_DIR"
# Avoid touching filesystem on each backup
RAMFS="$(mktemp -d -t sysupgrade.XX)"
@@ -245,6 +247,15 @@ do_save_conffiles() {
 			\( -exec test -f /overlay/upper/{} \; -exec echo {} overlay \; \) 
-o \

\( -exec echo {} unknown \; \) \
\) | sed -e 's,.*/,,;s/\.control /\t/' > 
${INSTALLED_PACKAGES}
+
+   # Format: /etc/init.d/servicename {enable,disable}
+   rm -f ${SERVICE_STATUS}
+   for service in /etc/init.d/* ; do \
+   ${service} enabled && \
+   echo >> ${SERVICE_STATUS} "$service" "enable" 
|| \
+   echo >> ${SERVICE_STATUS} "$service" "disable" \
+   ; \
+   done
fi

v "Saving config files..."
--
2.20.1



I like the idea of this patch but would agree with Stijn that it should 
be it's own parameter.


@Reiner could you collaborate with your oot solution? This feature 
seems like something that should exists per default, not requiring 
everyone to come up with an individual implementation.


@Daniel that's convenient for custom images but by default I may just 
want to upgrade with an upstream image. Even a custom image like 
generated by ASU doesn't support service settings.






___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-09 Thread Alberto Bursi




On 09/01/21 12:56, Reiner Karlsberg wrote:

Am 09.01.2021 um 13:28 schrieb Stijn Segers:

 > Currently all services get enabled during image creation. This can cause
 > issues after upgrade with services explicitly disabled by the user.
 > With this created list sourced by a simple uci-defaults script the state
 > can be restored automatically.

Pls, do _NOT_ make this default.
There might be other entities, besides me, who already have
implemented methods to work around unnecessary enabled services.
I.e. to disable service in uci-defaults, like I do.
Auto-restore service state will break existing code.

In general, I do _NOT_ consider it a good idea, to more or less
silently modify existing functionality.
Which happems often enough in openwrt, unfortunately.

Best would be, to make this new behaviour optional. Do another change 
for LuCI, and then this is done.


In worst case, pls provide an option for sysupgrade _NOT_
to keep service state.

Cheers,

Reiner





When you or some other entity decided to keep your workaround downstream 
instead of contributing it you accepted the risk of someone else 
eventually fixing it in a way you don't like, or in a moment you don't like.


I disagree with making this optional just to save you and whatever 
others had their own local workaround some trouble, if you have a bunch 
of downstream patches it's on you to deal with it, you can't complain to 
the main project and ask to stop development because it's increasing 
your workload.


I think what you think is "worst case", is the only acceptable thing, 
making it active by default and adding a setting to disable this 
function. Although it's useful only for your specific usecase (i.e. the 
ones that actually have their own workaround in place). If this works 
there is no need to disable it and re-invent the wheel.


-Alberto

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-09 Thread Alberto Bursi



On 09/01/21 12:28, Stijn Segers wrote:

Hi,

Op zondag 3 januari 2021 om 23u14 schreef Sven Roederer 
:

When saving the list of installed pkgs, also store the status of the
system services. The list is created in the etc/backup folder also
and formated as:

/etc/init.d/ {enable|disable}

This way it can be sourced after sysupgrade, to restore the previous
state.

Signed-off-by: Sven Roederer 
---

Currently all services get enabled during image creation. This can cause
issues after upgrade with services explicitly disabled by the user.
With this created list sourced by a simple uci-defaults script the state
can be restored automatically.
Not including such a uci-defaults script by default, as currently the
stored package list is also not reinstalled.


 package/base-files/Makefile  |  2 +-
 package/base-files/files/sbin/sysupgrade | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/package/base-files/Makefile b/package/base-files/Makefile
index 0c612b73ca..fbcb694592 100644
--- a/package/base-files/Makefile
+++ b/package/base-files/Makefile
@@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/version.mk
 include $(INCLUDE_DIR)/feeds.mk

 PKG_NAME:=base-files
-PKG_RELEASE:=239
+PKG_RELEASE:=240
 PKG_FLAGS:=nonshared

 PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/ $(GENERIC_PLATFORM_DIR)/base-files/
diff --git a/package/base-files/files/sbin/sysupgrade 
b/package/base-files/files/sbin/sysupgrade

index 79927a2b5c..cadce36172 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -57,6 +57,7 @@ export CONFFILES=/tmp/sysupgrade.conffiles
 export CONF_TAR=/tmp/sysupgrade.tgz
 export ETCBACKUP_DIR=/etc/backup
 export INSTALLED_PACKAGES=${ETCBACKUP_DIR}/installed_packages.txt
+export SERVICE_STATUS=${ETCBACKUP_DIR}/service_status.txt

 IMAGE="$1"

@@ -228,6 +229,7 @@ do_save_conffiles() {

 if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
 echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
+    echo "${SERVICE_STATUS}" >> "$CONFFILES"
 mkdir -p "$ETCBACKUP_DIR"



Am I reading this correctly and is this only keeping track of service 
status if you tell sysupgrade to save packages? What's the rationale 
behind that?


I have a personal build with all packages preinstalled, so I don't need 
that. Would like to keep track of service status though. Can those two 
things be entangled?




Same. I would personally like this as default sysupgrade procedure, as 
that's what makes most sense imho.
If I have disabled a service it makes sense that after a firmware 
upgrade it remains disabled.


-Alberto

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-09 Thread Reiner Karlsberg

Am 10.01.2021 um 03:32 schrieb Alberto Bursi:
>
>
> On 09/01/21 12:56, Reiner Karlsberg wrote:
>> Am 09.01.2021 um 13:28 schrieb Stijn Segers:
>>
>>  > Currently all services get enabled during image creation. This can cause
>>  > issues after upgrade with services explicitly disabled by the user.
>>  > With this created list sourced by a simple uci-defaults script the state
>>  > can be restored automatically.
>>
>> Pls, do _NOT_ make this default.
>> There might be other entities, besides me, who already have
>> implemented methods to work around unnecessary enabled services.
>> I.e. to disable service in uci-defaults, like I do.
>> Auto-restore service state will break existing code.
>>
>> In general, I do _NOT_ consider it a good idea, to more or less
>> silently modify existing functionality.
>> Which happems often enough in openwrt, unfortunately.
>>
>> Best would be, to make this new behaviour optional. Do another change for 
LuCI, and then this is done.
>>
>> In worst case, pls provide an option for sysupgrade _NOT_
>> to keep service state.
>>
>> Cheers,
>>
>> Reiner
>>
>>
>
>
> When you or some other entity decided to keep your workaround downstream 
instead of contributing it you accepted the
> risk of someone else eventually fixing it in a way you don't like, or in a 
moment you don't like.
You can fix a bug, to modify existing code really to fulfil assumed 
functionality.
But we are talking about implementation of new (or modified) functionality here.

>
> I disagree with making this optional just to save you and whatever others had 
their own local workaround some trouble,
> if you have a bunch of downstream patches it's on you to deal with it, you 
can't complain to the main project and ask to
> stop development because it's increasing your workload.>
> I think what you think is "worst case", is the only acceptable thing, making 
it active by default and adding a setting
> to disable this function. Although it's useful only for your specific usecase 
(i.e. the ones that actually have their
> own workaround in place). If this works there is no need to disable it and 
re-invent the wheel.
In this particular case, there is already the "-n" option available for 
sysupgrade. Not to keep the settings.
The proposed functionality would affect this well-known (?) existing function.
A reasonable compromise would be, to extend the functionality of "-n": In case "-n" is used, this also means, _NOT_ to 
save the service states.


However, in general, already half a century ago I knew the concept of 
"modularity" in software engineering.
Which also means, to keep existing functionality in case of changes. You can also call it 
"backward compatibility".

Cheers,

Reiner
>
> -Alberto
>
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Andre Heider

On 10/01/2021 02:37, Alberto Bursi wrote:



On 09/01/21 12:28, Stijn Segers wrote:

Hi,

Op zondag 3 januari 2021 om 23u14 schreef Sven Roederer 
:

When saving the list of installed pkgs, also store the status of the
system services. The list is created in the etc/backup folder also
and formated as:

/etc/init.d/ {enable|disable}

This way it can be sourced after sysupgrade, to restore the previous
state.

Signed-off-by: Sven Roederer 
---

Currently all services get enabled during image creation. This can cause
issues after upgrade with services explicitly disabled by the user.
With this created list sourced by a simple uci-defaults script the state
can be restored automatically.
Not including such a uci-defaults script by default, as currently the
stored package list is also not reinstalled.


 package/base-files/Makefile  |  2 +-
 package/base-files/files/sbin/sysupgrade | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/package/base-files/Makefile b/package/base-files/Makefile
index 0c612b73ca..fbcb694592 100644
--- a/package/base-files/Makefile
+++ b/package/base-files/Makefile
@@ -12,7 +12,7 @@ include $(INCLUDE_DIR)/version.mk
 include $(INCLUDE_DIR)/feeds.mk

 PKG_NAME:=base-files
-PKG_RELEASE:=239
+PKG_RELEASE:=240
 PKG_FLAGS:=nonshared

 PKG_FILE_DEPENDS:=$(PLATFORM_DIR)/ $(GENERIC_PLATFORM_DIR)/base-files/
diff --git a/package/base-files/files/sbin/sysupgrade 
b/package/base-files/files/sbin/sysupgrade

index 79927a2b5c..cadce36172 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -57,6 +57,7 @@ export CONFFILES=/tmp/sysupgrade.conffiles
 export CONF_TAR=/tmp/sysupgrade.tgz
 export ETCBACKUP_DIR=/etc/backup
 export INSTALLED_PACKAGES=${ETCBACKUP_DIR}/installed_packages.txt
+export SERVICE_STATUS=${ETCBACKUP_DIR}/service_status.txt

 IMAGE="$1"

@@ -228,6 +229,7 @@ do_save_conffiles() {

 if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
 echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
+    echo "${SERVICE_STATUS}" >> "$CONFFILES"
 mkdir -p "$ETCBACKUP_DIR"



Am I reading this correctly and is this only keeping track of service 
status if you tell sysupgrade to save packages? What's the rationale 
behind that?


I have a personal build with all packages preinstalled, so I don't 
need that. Would like to keep track of service status though. Can 
those two things be entangled?




Same. I would personally like this as default sysupgrade procedure, as 
that's what makes most sense imho.
If I have disabled a service it makes sense that after a firmware 
upgrade it remains disabled.


Seconded.

Use case: Two identical routers for which I build a single image 
including all the packages I required. One has a workround to disable 
dnsmasq in rc.local, because it's always enabled after a sysupgrade and 
acts as a additional interfering dhcp server (I can probably configure 
it otherwise, but I don't require that instance at all).


Thanks,
Andre

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Alberto Bursi



On 10/01/21 08:26, Reiner Karlsberg wrote:

Am 10.01.2021 um 03:32 schrieb Alberto Bursi:
 >
 >
 > On 09/01/21 12:56, Reiner Karlsberg wrote:
 >> Am 09.01.2021 um 13:28 schrieb Stijn Segers:
 >>
 >>  > Currently all services get enabled during image creation. This 
can cause

 >>  > issues after upgrade with services explicitly disabled by the user.
 >>  > With this created list sourced by a simple uci-defaults script 
the state

 >>  > can be restored automatically.
 >>
 >> Pls, do _NOT_ make this default.
 >> There might be other entities, besides me, who already have
 >> implemented methods to work around unnecessary enabled services.
 >> I.e. to disable service in uci-defaults, like I do.
 >> Auto-restore service state will break existing code.
 >>
 >> In general, I do _NOT_ consider it a good idea, to more or less
 >> silently modify existing functionality.
 >> Which happems often enough in openwrt, unfortunately.
 >>
 >> Best would be, to make this new behaviour optional. Do another 
change for LuCI, and then this is done.

 >>
 >> In worst case, pls provide an option for sysupgrade _NOT_
 >> to keep service state.
 >>
 >> Cheers,
 >>
 >> Reiner
 >>
 >>
 >
 >
 > When you or some other entity decided to keep your workaround 
downstream instead of contributing it you accepted the
 > risk of someone else eventually fixing it in a way you don't like, or 
in a moment you don't like.
You can fix a bug, to modify existing code really to fulfil assumed 
functionality.
But we are talking about implementation of new (or modified) 
functionality here.


not saving state of services on sysupgrade is a bug, at least imho.

What is the rationale for not saving service status on sysupgrade, 
beyond "not break my current custom downstream stuff" though?


Do you have a usecase where you are doing significantly more complex 
things that cannot be just migrated to a new standard where service 
status is preserved with a normal sysupgrade?




 >
 > I disagree with making this optional just to save you and whatever 
others had their own local workaround some trouble,
 > if you have a bunch of downstream patches it's on you to deal with 
it, you can't complain to the main project and ask to

 > stop development because it's increasing your workload.>
 > I think what you think is "worst case", is the only acceptable thing, 
making it active by default and adding a setting
 > to disable this function. Although it's useful only for your specific 
usecase (i.e. the ones that actually have their
 > own workaround in place). If this works there is no need to disable 
it and re-invent the wheel.
In this particular case, there is already the "-n" option available for 
sysupgrade. Not to keep the settings.
The proposed functionality would affect this well-known (?) existing 
function.


This makes me think you misread the proposed change.

At the moment it's adding this functionality under the "-k" option (the 
"save packages" function), everything else is unaffected.


Using "-n" always overrides all other backup options. It means sysupgade 
will not write the "backup config tar" file at all.
So it does not matter what additional things are backed up by default or 
what other options you add to sysupgrade call, if you use "-n" nothing 
gets backed up.

This does not change, and nobody wants or wanted to change the "-n" option.

There are just 2 people (me, Andrew Heider) that would like to see 
saving service status done by default when sysupgrading, and other 2 
people that would like it in its own setting option (Stjin Segers and 
Paul Spooren).





However, in general, already half a century ago I knew the concept of 
"modularity" in software engineering.
Which also means, to keep existing functionality in case of changes. You 
can also call it "backward compatibility".


Yeah but if a default is changed, the only "backward compatibility" that 
can be done is adding an option to "run in legacy mode", which is what I 
said was acceptable imho, but strange.


Others asked to place this in its own option instead to add it to an 
existing option.




Cheers,

Reiner
 >
 > -Alberto
 >
 > ___
 > openwrt-devel mailing list
 > openwrt-devel@lists.openwrt.org
 > https://lists.openwrt.org/mailman/listinfo/openwrt-devel

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Luiz Angelo Daros de Luca
> When saving the list of installed pkgs, also store the status of the
> system services. The list is created in the etc/backup folder also
> and formated as:
>
> /etc/init.d/ {enable|disable}
>
> This way it can be sourced after sysupgrade, to restore the previous
> state.

I also liked the idea. I didn't like the use of -k option (save a list
of installed packages). I would either create a new option for it,
enable it when -c or -o is in use, or simply enable it by default.

I also think that ${SERVICE_STATUS} could be executed by default after
the reboot. If the user asked to preserve service status, it could be
implicitly assumed that the user wants to restore it after the backup
is applied. The problem is that you cannot write directly to
/etc/uci-defaults without touching the flash on each backup. The
/etc/backup is a good solution but you need to change when it is
mounted, and not only when '-k' is selected.

I would create a new /etc/uci-defaults/01-post-restore.sh provided by
base-files (and not created by a backup) that would read a possible
/etc/backup/post-restore.sh script. There you could place your
enable/disable calls.

Regards,

Luiz Angelo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Sven Roederer
Am Samstag, 9. Januar 2021, 12:28:31 CET schrieb Stijn Segers:
> > @@ -228,6 +229,7 @@ do_save_conffiles() {
> > 
> > if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
> > echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
> > +   echo "${SERVICE_STATUS}" >> "$CONFFILES"
> > mkdir -p "$ETCBACKUP_DIR"
> 
> Am I reading this correctly and is this only keeping track of service
> status if you tell sysupgrade to save packages? What's the rationale
> behind that?
> 
> I have a personal build with all packages preinstalled, so I don't need
> that. Would like to keep track of service status though. Can those two
> things be entangled?
> 

Stijn,

my intention was to not change the current behavior by default, so an extra 
switch or extending an existing switch looked like the way. I've choosen the 
lazy one, based on "when the user is storing the packages-list, he is for sure 
interested in the services".
But I'm happy to add a separate switch to sysupgrade. Any preference of the 
letter? What about using "-s"?

Sven



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Sven Roederer
Am Sonntag, 10. Januar 2021, 09:47:27 CET schrieb Andre Heider:
> > Same. I would personally like this as default sysupgrade procedure, as
> > that's what makes most sense imho.
> > If I have disabled a service it makes sense that after a firmware
> > upgrade it remains disabled.
> 
> Seconded.
> 
> Use case: Two identical routers for which I build a single image
> including all the packages I required. One has a workround to disable
> dnsmasq in rc.local, because it's always enabled after a sysupgrade and
> acts as a additional interfering dhcp server (I can probably configure
> it otherwise, but I don't require that instance at all).

That's quite the situation we have here with our common images. Users during 
initial setup getting some services disabled based on their choices. As the 
services shall remain inside the image (reconfiguration) the services-state 
should be kept.

Sven



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Paul Spooren




On So, Jan 10, 2021 at 22:40, Sven Roederer  
wrote:

Am Sonntag, 10. Januar 2021, 09:47:27 CET schrieb Andre Heider:
 > Same. I would personally like this as default sysupgrade 
procedure, as

 > that's what makes most sense imho.
 > If I have disabled a service it makes sense that after a firmware
 > upgrade it remains disabled.

 Seconded.

 Use case: Two identical routers for which I build a single image
 including all the packages I required. One has a workround to 
disable
 dnsmasq in rc.local, because it's always enabled after a sysupgrade 
and
 acts as a additional interfering dhcp server (I can probably 
configure

 it otherwise, but I don't require that instance at all).


That's quite the situation we have here with our common images. Users 
during
initial setup getting some services disabled based on their choices. 
As the
services shall remain inside the image (reconfiguration) the 
services-state

should be kept.

Sven


Do you have a follow up patch that does the service disabling after a 
reboot? I'd be curious about the implementation.


I'm in favor to have this by default unless disabled via -n. It's 
certainly less "invasive" than an automatic `opkg reinstall packages` 
step.






___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel




___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Stijn Segers

Hi Sven,

Op zondag 10 januari 2021 om 22u28 schreef Sven Roederer 
:

Am Samstag, 9. Januar 2021, 12:28:31 CET schrieb Stijn Segers:

 > @@ -228,6 +229,7 @@ do_save_conffiles() {
 >
 > if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
 > echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
 > +   echo "${SERVICE_STATUS}" >> "$CONFFILES"
 > mkdir -p "$ETCBACKUP_DIR"

 Am I reading this correctly and is this only keeping track of 
service

 status if you tell sysupgrade to save packages? What's the rationale
 behind that?

 I have a personal build with all packages preinstalled, so I don't 
need
 that. Would like to keep track of service status though. Can those 
two

 things be entangled?



Stijn,

my intention was to not change the current behavior by default, so an 
extra
switch or extending an existing switch looked like the way. I've 
choosen the
lazy one, based on "when the user is storing the packages-list, he is 
for sure

interested in the services".
But I'm happy to add a separate switch to sysupgrade. Any preference 
of the

letter? What about using "-s"?

Sven


Yes, that's still free and the most intuitive I think.

Thanks!

Stijn





___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel




___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Alberto Bursi



On 10/01/21 22:50, Stijn Segers wrote:

Hi Sven,

Op zondag 10 januari 2021 om 22u28 schreef Sven Roederer 
:

Am Samstag, 9. Januar 2021, 12:28:31 CET schrieb Stijn Segers:

 > @@ -228,6 +229,7 @@ do_save_conffiles() {
 >
 > if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
 > echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
 > +   echo "${SERVICE_STATUS}" >> "$CONFFILES"
 > mkdir -p "$ETCBACKUP_DIR"

 Am I reading this correctly and is this only keeping track of service
 status if you tell sysupgrade to save packages? What's the rationale
 behind that?

 I have a personal build with all packages preinstalled, so I don't need
 that. Would like to keep track of service status though. Can those two
 things be entangled?



Stijn,

my intention was to not change the current behavior by default, so an 
extra
switch or extending an existing switch looked like the way. I've 
choosen the
lazy one, based on "when the user is storing the packages-list, he is 
for sure

interested in the services".
But I'm happy to add a separate switch to sysupgrade. Any preference 
of the

letter? What about using "-s"?

Sven


Yes, that's still free and the most intuitive I think.

Thanks!

Stijn



Since we (me, Andre Heider and Paul Spooren) are discussing/asking about 
enabling this by default, do you have any opinion on that?


-Alberto

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-10 Thread Alberto Bursi




On 10/01/21 22:40, Sven Roederer wrote:

Am Sonntag, 10. Januar 2021, 09:47:27 CET schrieb Andre Heider:

Same. I would personally like this as default sysupgrade procedure, as
that's what makes most sense imho.
If I have disabled a service it makes sense that after a firmware
upgrade it remains disabled.


Seconded.

Use case: Two identical routers for which I build a single image
including all the packages I required. One has a workround to disable
dnsmasq in rc.local, because it's always enabled after a sysupgrade and
acts as a additional interfering dhcp server (I can probably configure
it otherwise, but I don't require that instance at all).


That's quite the situation we have here with our common images. Users during
initial setup getting some services disabled based on their choices. As the
services shall remain inside the image (reconfiguration) the services-state
should be kept.

Sven




I don't understand what you are saying here. Can you explain?

-Alberto

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


RE: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-11 Thread Adrian Schmutzler
> There are just 2 people (me, Andrew Heider) that would like to see saving
> service status done by default when sysupgrading, and other 2 people that
> would like it in its own setting option (Stjin Segers and Paul Spooren).

+1 for saving service status by default. This has always annoyed me when 
working with "default" images and actually for me it was expected behavior 
until I found out it is not happening.
This is a very relevant behavior/feature affecting many of our "standard" 
users, e.g. when using OpenWrt for "Dump AP" setups where you disable DHCP etc. 
I'm sure a two-digit percentage of users setting up their device like that 
won't even be aware that they suddenly have a DHCP running again after upgrade.

Of course, if adding an option to _disable_ is fairly easy, we should do so.

Best

Adrian 


openpgp-digital-signature.asc
Description: PGP signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-11 Thread Eneas U de Queiroz
+1
I agree 100% with Adrian on this one.  Enable by default, add option
to disable.  Disabled services are, intuitively, part of the
configuration being saved.  So, it should not be saved when '-n' is
given.  I may be stretching things a bit, but I would consider this a
fix, not a feature change ;-).

Cheers,

Eneas

On Mon, Jan 11, 2021 at 9:48 AM Adrian Schmutzler
 wrote:
>
> > There are just 2 people (me, Andrew Heider) that would like to see saving
> > service status done by default when sysupgrading, and other 2 people that
> > would like it in its own setting option (Stjin Segers and Paul Spooren).
>
> +1 for saving service status by default. This has always annoyed me when 
> working with "default" images and actually for me it was expected behavior 
> until I found out it is not happening.
> This is a very relevant behavior/feature affecting many of our "standard" 
> users, e.g. when using OpenWrt for "Dump AP" setups where you disable DHCP 
> etc. I'm sure a two-digit percentage of users setting up their device like 
> that won't even be aware that they suddenly have a DHCP running again after 
> upgrade.
>
> Of course, if adding an option to _disable_ is fairly easy, we should do so.
>
> Best
>
> Adrian
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-11 Thread Stijn Segers
Hi Alberto,

Alberto Bursi  schreef op 11 januari 2021 03:56:23 
CET:
>
>
>On 10/01/21 22:50, Stijn Segers wrote:
>> Hi Sven,
>> 
>> Op zondag 10 januari 2021 om 22u28 schreef Sven Roederer 
>> :
>>> Am Samstag, 9. Januar 2021, 12:28:31 CET schrieb Stijn Segers:
  > @@ -228,6 +229,7 @@ do_save_conffiles() {
  >
  > if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
  > echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
  > +   echo "${SERVICE_STATUS}" >> "$CONFFILES"
  > mkdir -p "$ETCBACKUP_DIR"

  Am I reading this correctly and is this only keeping track of service
  status if you tell sysupgrade to save packages? What's the rationale
  behind that?

  I have a personal build with all packages preinstalled, so I don't need
  that. Would like to keep track of service status though. Can those two
  things be entangled?

>>>
>>> Stijn,
>>>
>>> my intention was to not change the current behavior by default, so an 
>>> extra
>>> switch or extending an existing switch looked like the way. I've 
>>> choosen the
>>> lazy one, based on "when the user is storing the packages-list, he is 
>>> for sure
>>> interested in the services".
>>> But I'm happy to add a separate switch to sysupgrade. Any preference 
>>> of the
>>> letter? What about using "-s"?
>>>
>>> Sven
>> 
>> Yes, that's still free and the most intuitive I think.
>> 
>> Thanks!
>> 
>> Stijn
>> 
>
>Since we (me, Andre Heider and Paul Spooren) are discussing/asking about 
>enabling this by default, do you have any opinion on that?
>
>-Alberto

That would be great, as Adrian pointed out it's something you'd expect would be 
saved when you tell sysupgrade to keep settings.

So +1 from me on making it default. All my AP/testing devices now have 
scripting to disable e.g. DHCP and DNS again after an upgrade.

Stijn
>
>___
>openwrt-devel mailing list
>openwrt-devel@lists.openwrt.org
>https://lists.openwrt.org/mailman/listinfo/openwrt-devel

-- 
Verstuurd vanaf mijn Android apparaat met K-9 Mail. Excuseer mijn beknoptheid.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-11 Thread Martin Schiller

On 2021-01-11 14:38, Stijn Segers wrote:

Hi Alberto,

Alberto Bursi  schreef op 11 januari 2021
03:56:23 CET:



On 10/01/21 22:50, Stijn Segers wrote:

Hi Sven,

Op zondag 10 januari 2021 om 22u28 schreef Sven Roederer
:

Am Samstag, 9. Januar 2021, 12:28:31 CET schrieb Stijn Segers:

 > @@ -228,6 +229,7 @@ do_save_conffiles() {
 >
 > if [ "$SAVE_INSTALLED_PKGS" -eq 1 ]; then
 > echo "${INSTALLED_PACKAGES}" >> "$CONFFILES"
 > +   echo "${SERVICE_STATUS}" >> "$CONFFILES"
 > mkdir -p "$ETCBACKUP_DIR"

 Am I reading this correctly and is this only keeping track of 
service
 status if you tell sysupgrade to save packages? What's the 
rationale

 behind that?

 I have a personal build with all packages preinstalled, so I don't 
need
 that. Would like to keep track of service status though. Can those 
two

 things be entangled?



Stijn,

my intention was to not change the current behavior by default, so 
an

extra
switch or extending an existing switch looked like the way. I've
choosen the
lazy one, based on "when the user is storing the packages-list, he 
is

for sure
interested in the services".
But I'm happy to add a separate switch to sysupgrade. Any preference
of the
letter? What about using "-s"?

Sven


Yes, that's still free and the most intuitive I think.

Thanks!

Stijn



Since we (me, Andre Heider and Paul Spooren) are discussing/asking 
about

enabling this by default, do you have any opinion on that?

-Alberto


That would be great, as Adrian pointed out it's something you'd expect
would be saved when you tell sysupgrade to keep settings.

So +1 from me on making it default. All my AP/testing devices now have
scripting to disable e.g. DHCP and DNS again after an upgrade.

Stijn


I am also in favor of saving the service status by default.
This is the expected behavior as long as you don't use the '-n' switch.

Currently we use our own patchset for this, but I would prefer to get
an upstream solution.

Martin

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] base-files: sysupgrade: store status of system-services

2021-01-11 Thread Sven Roederer
Am Montag, 11. Januar 2021, 03:59:51 CET schrieb Alberto Bursi:
> On 10/01/21 22:40, Sven Roederer wrote:
> > Am Sonntag, 10. Januar 2021, 09:47:27 CET schrieb Andre Heider:
> >>> Same. I would personally like this as default sysupgrade procedure, as
> >>> that's what makes most sense imho.
> >>> If I have disabled a service it makes sense that after a firmware
> >>> upgrade it remains disabled.
> >> 
> >> Seconded.
> >> 
> >> Use case: Two identical routers for which I build a single image
> >> including all the packages I required. One has a workround to disable
> >> dnsmasq in rc.local, because it's always enabled after a sysupgrade and
> >> acts as a additional interfering dhcp server (I can probably configure
> >> it otherwise, but I don't require that instance at all).
> > 
> > That's quite the situation we have here with our common images. Users
> > during initial setup getting some services disabled based on their
> > choices. As the services shall remain inside the image (reconfiguration)
> > the services-state should be kept.
> > 
> > Sven
> 
> I don't understand what you are saying here. Can you explain?
> 

Alberto,

it's just the story why the current behavior annoyed me. 
Same as you wrote: a common image for multiple routers which have all relevant 
services installed.during setup our wizard disables some services, which 
should remain in this state.

Sven



___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel