Re: [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

2024-02-16 Thread Jo-Philipp Wich

Hi Rafał,


Extend sysupgrade to check for disabled services, generate uci-defaults
script disabling them and include it in backup.

Cc: Christian Marangi 
Cc: Jo-Philipp Wich 
Cc: Jonas Gorski 
Signed-off-by: Rafał Miłecki 


Acked-by: Jo-Philipp Wich 


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


Re: [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

2024-02-15 Thread Rafał Miłecki

On 15.02.2024 18:46, Paul D wrote:

On 2024-02-15 15:42, Rafał Miłecki wrote:

On 14.02.2024 21:50, Paul D wrote:

Would services not do better to be tracked within uci and its config files in 
/etc/config?


Well, it's a part of a mess we have in our init/config code. It was only
last week that Jo was discussing it with Ansuel. In short we have:
1. Packages with no UCI config option for enabling/disabling
2. Packages with "enable"
3. Packages with "enabled"
4. Packages with "disable"
5. Packages with "disabled"

I'm pretty sure most users are used to enabling/disabling services
using UCI config option but not all of them have such possibility. That
is the case my change is meant to deal with.

An alternative would be to make sure every package uses "enabled" (or
"disabled") UCI option. Should we do that?



Is 'on' or 'off' in the config perhaps simpler? I think it avoids the 
linguistic differences of past participle vs present simple. (Sometimes this 
distinction is important to have.)

Otherwise 'en/disabled' is a good paradigm to encourage in configs.


Do you mean "on" and "off" as option values? How would you call such option? "enabled"? 
:P Something like "status" would be misleading again.

Or if you mean "on" and "off" as option names then what value should we assign?
uci set foo.config.on=1
?

No, I don't think "on" / "off" is a good option at all.

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


Re: [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

2024-02-15 Thread Paul D

On 2024-02-15 15:42, Rafał Miłecki wrote:

On 14.02.2024 21:50, Paul D wrote:
Would services not do better to be tracked within uci and its config 
files in /etc/config?


Well, it's a part of a mess we have in our init/config code. It was only
last week that Jo was discussing it with Ansuel. In short we have:
1. Packages with no UCI config option for enabling/disabling
2. Packages with "enable"
3. Packages with "enabled"
4. Packages with "disable"
5. Packages with "disabled"

I'm pretty sure most users are used to enabling/disabling services
using UCI config option but not all of them have such possibility. That
is the case my change is meant to deal with.

An alternative would be to make sure every package uses "enabled" (or
"disabled") UCI option. Should we do that?



Is 'on' or 'off' in the config perhaps simpler? I think it avoids the 
linguistic differences of past participle vs present simple. (Sometimes 
this distinction is important to have.)


Otherwise 'en/disabled' is a good paradigm to encourage in configs.



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


Re: [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

2024-02-15 Thread Eric via openwrt-devel
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.--- Begin Message ---
On Thursday, February 15th, 2024 at 06:42, Rafał Miłecki  
wrote:
> I'm pretty sure most users are used to enabling/disabling services
> using UCI config option but not all of them have such possibility. That
> is the case my change is meant to deal with.

Well, there was some user discussion on the forum a couple months back and
I've always felt 'service X enable/disable' was the right way to do things.
https://forum.openwrt.org/t/bug-service-status-is-not-saved-after-sysupgrade/179045

> An alternative would be to make sure every package uses "enabled" (or
> "disabled") UCI option. Should we do that?

I recently reworked the snort3 package and added "enabled" to its config.
I felt then (and still do) that this is sort of a "hack around a bug",
and would rather see it addressed as part of sysupgrade's backup/restore
process.

I'm a definite +1 on your current patch for sysupgrade, it looks like 
an elegant, unintrusive and robust means for dealing with service state.

Eric

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


Re: [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

2024-02-15 Thread Rafał Miłecki

On 14.02.2024 21:50, Paul D wrote:

Would services not do better to be tracked within uci and its config files in 
/etc/config?


Well, it's a part of a mess we have in our init/config code. It was only
last week that Jo was discussing it with Ansuel. In short we have:
1. Packages with no UCI config option for enabling/disabling
2. Packages with "enable"
3. Packages with "enabled"
4. Packages with "disable"
5. Packages with "disabled"

I'm pretty sure most users are used to enabling/disabling services
using UCI config option but not all of them have such possibility. That
is the case my change is meant to deal with.

An alternative would be to make sure every package uses "enabled" (or
"disabled") UCI option. Should we do that?



Or do changes to those files there risk triggering other procd actions to the 
services they dictate?


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


Re: [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

2024-02-14 Thread Paul D

(Not directly commenting on this change set)

Would services not do better to be tracked within uci and its config 
files in /etc/config?


Or do changes to those files there risk triggering other procd actions 
to the services they dictate?




On 2024-02-14 16:05, Rafał Miłecki wrote:

From: Rafał Miłecki 

Disabled services should be kept disabled after sysupgrade. This can be
easily handled using a proper uci-defaults script.

Extend sysupgrade to check for disabled services, generate uci-defaults
script disabling them and include it in backup.

Cc: Christian Marangi 
Cc: Jo-Philipp Wich 
Cc: Jonas Gorski 
Signed-off-by: Rafał Miłecki 
---
  package/base-files/files/sbin/sysupgrade | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/package/base-files/files/sbin/sysupgrade 
b/package/base-files/files/sbin/sysupgrade
index 1e09f65e07..b1ada062ed 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -273,6 +273,16 @@ create_backup_archive() {
\) | sed -e 's,.*/,,;s/\.control /\t/' > 
"$dir/${INSTALLED_PACKAGES}"
fi
  
+	mkdir -p $dir/etc/uci-defaults/

+   touch $dir/etc/uci-defaults/10_disable_services
+   for service in /etc/init.d/*; do
+   if ! $service enabled; then
+   echo "$service disable" >> 
$dir/etc/uci-defaults/10_disable_services
+   fi
+   done
+   echo "exit 0" >> $dir/etc/uci-defaults/10_disable_services
+   echo "/etc/uci-defaults/10_disable_services" >> "$CONFFILES"
+
v "Saving config files..."
sed -i 's/^\///' "$CONFFILES" # Drop leading slashes
[ "$VERBOSE" -gt 1 ] && TAR_V="v" || TAR_V=""



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


[PATCH] base-files: sysupgrade: include uci-defaults script disabling services

2024-02-14 Thread Rafał Miłecki
From: Rafał Miłecki 

Disabled services should be kept disabled after sysupgrade. This can be
easily handled using a proper uci-defaults script.

Extend sysupgrade to check for disabled services, generate uci-defaults
script disabling them and include it in backup.

Cc: Christian Marangi 
Cc: Jo-Philipp Wich 
Cc: Jonas Gorski 
Signed-off-by: Rafał Miłecki 
---
 package/base-files/files/sbin/sysupgrade | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/package/base-files/files/sbin/sysupgrade 
b/package/base-files/files/sbin/sysupgrade
index 1e09f65e07..b1ada062ed 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -273,6 +273,16 @@ create_backup_archive() {
\) | sed -e 's,.*/,,;s/\.control /\t/' > 
"$dir/${INSTALLED_PACKAGES}"
fi
 
+   mkdir -p $dir/etc/uci-defaults/
+   touch $dir/etc/uci-defaults/10_disable_services
+   for service in /etc/init.d/*; do
+   if ! $service enabled; then
+   echo "$service disable" >> 
$dir/etc/uci-defaults/10_disable_services
+   fi
+   done
+   echo "exit 0" >> $dir/etc/uci-defaults/10_disable_services
+   echo "/etc/uci-defaults/10_disable_services" >> "$CONFFILES"
+
v "Saving config files..."
sed -i 's/^\///' "$CONFFILES" # Drop leading slashes
[ "$VERBOSE" -gt 1 ] && TAR_V="v" || TAR_V=""
-- 
2.35.3


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