[pve-devel] [RFC manager/lxc-syscalld/container 0/4] avoid using generic runtime directory name for pve-lxc-syscalld

[pve-devel] [RFC manager/lxc-syscalld/container 0/4] avoid using generic runtime directory name for pve-lxc-syscalld

[Fiona Ebner]

This is intended for PVE 9.

The pve-lxc-syscalld systemd service currently uses /run/pve as a
runtime directory. This means, that when the service is restarted, the
directory will be recreated. But the /run/pve directory is not just
used as the runtime directory of this service, but also for other
things, e.g. storage tunnel and mtunnel sockets, container stderr logs
as well as pull metric cache and lock, which will be lost when the
service is restarted.

Versioned Breaks needed:

new pve-lxc-syscalld breaks old pve-container: when the experimental
'mknod' feature is used, which requires specifying the new socket path

new pve-lxc-syscalld breaks old pve-manager: /run/pve is not
automatically created in the context of pull metrics yet

I've never used systemd tmpfiles.d before, so that requires extra
scrutiny 0:)

manager:

Fiona Ebner (1):
  add tpmfiles.d config to create /run/pve directory

 configs/Makefile          | 1 +
 configs/pve-tmpfiles.conf | 2 ++
 2 files changed, 3 insertions(+)
 create mode 100644 configs/pve-tmpfiles.conf


pve-lxc-syscalld:

Fiona Ebner (2):
  service: avoid using generic runtime directory name
  d/postinst: create link to new socket location on upgrade

 debian/postinst                 | 19 +++++++++++++++++++
 etc/pve-lxc-syscalld.service.in |  4 ++--
 2 files changed, 21 insertions(+), 2 deletions(-)
 create mode 100644 debian/postinst


pve-container:

Fiona Ebner (1):
  seccomp config: adapt to new lxc-syscalld runtime directory

 src/PVE/LXC.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)


Summary over all repositories:
  5 files changed, 33 insertions(+), 3 deletions(-)

-- 
Generated by git-murpp 0.5.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager 1/4] add tpmfiles.d config to create /run/pve directory

[Fiona Ebner]

The pve-lxc-syscalld systemd service currently uses /run/pve as a
runtime directory. This means, that when the service is restarted, the
directory will be recreated. But the /run/pve directory is not just
used as the runtime directory of this service, but also for other
things, e.g. storage tunnel and mtunnel sockets, container stderr logs
as well as pull metric cache and lock, which will be lost when the
service is restarted.

The plan is to give the service its own runtime directory that is only
used for that purpose and nothing else. However, this means the
/run/pve directory will not get created automatically anymore (e.g.
pull metric relies on the existence already). Add this tmpfiles.d
configuration to create it automatically again. Note that the
permissions/owner are different now. As the runtime directory, it was
created with 0755 root:root. This tmpfiles.conf configuration
aligns the permissions/owner with the ones /run/pve-cluster has, i.e.
0750 root:www-data.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

We could also opt for 0750 root:root, not sure.

 configs/Makefile          | 1 +
 configs/pve-tmpfiles.conf | 2 ++
 2 files changed, 3 insertions(+)
 create mode 100644 configs/pve-tmpfiles.conf

diff --git a/configs/Makefile b/configs/Makefile
index fa586e28..36f4f75a 100644
--- a/configs/Makefile
+++ b/configs/Makefile
@@ -14,6 +14,7 @@ install: country.dat vzdump.conf pve-sources.list pve-initramfs.conf pve-blackli
 	install -D -m 0644 pve-initramfs.conf $(DESTDIR)/etc/initramfs-tools/conf.d/pve-initramfs.conf
 	install -D -m 0644 country.dat $(DESTDIR)/usr/share/$(PACKAGE)/country.dat
 	install -D -m 0644 proxmox-ve-default.link $(DESTDIR)/usr/lib/systemd/network/99-default.link.d/proxmox-mac-address-policy.conf
+	install -D -m 0644 pve-tmpfiles.conf $(DESTDIR)/usr/lib/tmpfiles.d/pve-tmpfiles.conf
 
 clean:
 	rm -f country.dat
diff --git a/configs/pve-tmpfiles.conf b/configs/pve-tmpfiles.conf
new file mode 100644
index 00000000..01c3275b
--- /dev/null
+++ b/configs/pve-tmpfiles.conf
@@ -0,0 +1,2 @@
+#Type Path     Mode User Group    Age Argument
+d     /run/pve 0750 root www-data -   -
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager 1/4] add tpmfiles.d config to create /run/pve directory

[Thomas Lamprecht]

Am 13.05.25 um 12:56 schrieb Fiona Ebner:
> The pve-lxc-syscalld systemd service currently uses /run/pve as a
> runtime directory. This means, that when the service is restarted, the
> directory will be recreated. But the /run/pve directory is not just
> used as the runtime directory of this service, but also for other
> things, e.g. storage tunnel and mtunnel sockets, container stderr logs
> as well as pull metric cache and lock, which will be lost when the
> service is restarted.
> 
> The plan is to give the service its own runtime directory that is only
> used for that purpose and nothing else. However, this means the
> /run/pve directory will not get created automatically anymore (e.g.
> pull metric relies on the existence already). Add this tmpfiles.d
> configuration to create it automatically again. Note that the
> permissions/owner are different now. As the runtime directory, it was
> created with 0755 root:root. This tmpfiles.conf configuration
> aligns the permissions/owner with the ones /run/pve-cluster has, i.e.
> 0750 root:www-data.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> We could also opt for 0750 root:root, not sure.

Would indeed better match the currently used /run/pve ownership.

> 
>  configs/Makefile          | 1 +
>  configs/pve-tmpfiles.conf | 2 ++
>  2 files changed, 3 insertions(+)
>  create mode 100644 configs/pve-tmpfiles.conf
> 
> diff --git a/configs/Makefile b/configs/Makefile
> index fa586e28..36f4f75a 100644
> --- a/configs/Makefile
> +++ b/configs/Makefile
> @@ -14,6 +14,7 @@ install: country.dat vzdump.conf pve-sources.list pve-initramfs.conf pve-blackli
>  	install -D -m 0644 pve-initramfs.conf $(DESTDIR)/etc/initramfs-tools/conf.d/pve-initramfs.conf
>  	install -D -m 0644 country.dat $(DESTDIR)/usr/share/$(PACKAGE)/country.dat
>  	install -D -m 0644 proxmox-ve-default.link $(DESTDIR)/usr/lib/systemd/network/99-default.link.d/proxmox-mac-address-policy.conf
> +	install -D -m 0644 pve-tmpfiles.conf $(DESTDIR)/usr/lib/tmpfiles.d/pve-tmpfiles.conf

You can use dh_installtmpfiles [0] for this and just add the relevant config in
a "debian/package.tmpfiles" file. With debhelper compat level 13 that helper
will be run by default [1], and as level 13 is the recommended level for Trixie,
I plan to switch all packages over to that anyway.

0: https://manpages.debian.org/trixie/debhelper/dh_installtmpfiles.1.en.html
1: https://manpages.debian.org/testing/debhelper/debhelper-compat-upgrade-checklist.7.en.html#v13

>  
>  clean:
>  	rm -f country.dat
> diff --git a/configs/pve-tmpfiles.conf b/configs/pve-tmpfiles.conf
> new file mode 100644
> index 00000000..01c3275b
> --- /dev/null
> +++ b/configs/pve-tmpfiles.conf
> @@ -0,0 +1,2 @@
> +#Type Path     Mode User Group    Age Argument
> +d     /run/pve 0750 root www-data -   -




_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager 1/4] add tpmfiles.d config to create /run/pve directory

[Fiona Ebner]

Am 14.05.25 um 20:08 schrieb Thomas Lamprecht:
> Am 13.05.25 um 12:56 schrieb Fiona Ebner:
>> The pve-lxc-syscalld systemd service currently uses /run/pve as a
>> runtime directory. This means, that when the service is restarted, the
>> directory will be recreated. But the /run/pve directory is not just
>> used as the runtime directory of this service, but also for other
>> things, e.g. storage tunnel and mtunnel sockets, container stderr logs
>> as well as pull metric cache and lock, which will be lost when the
>> service is restarted.
>>
>> The plan is to give the service its own runtime directory that is only
>> used for that purpose and nothing else. However, this means the
>> /run/pve directory will not get created automatically anymore (e.g.
>> pull metric relies on the existence already). Add this tmpfiles.d
>> configuration to create it automatically again. Note that the
>> permissions/owner are different now. As the runtime directory, it was
>> created with 0755 root:root. This tmpfiles.conf configuration
>> aligns the permissions/owner with the ones /run/pve-cluster has, i.e.
>> 0750 root:www-data.
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> We could also opt for 0750 root:root, not sure.
> 
> Would indeed better match the currently used /run/pve ownership.

Okay, then I'll go for this, can always be extended to www-data if the
need arises in the future.

>>
>>  configs/Makefile          | 1 +
>>  configs/pve-tmpfiles.conf | 2 ++
>>  2 files changed, 3 insertions(+)
>>  create mode 100644 configs/pve-tmpfiles.conf
>>
>> diff --git a/configs/Makefile b/configs/Makefile
>> index fa586e28..36f4f75a 100644
>> --- a/configs/Makefile
>> +++ b/configs/Makefile
>> @@ -14,6 +14,7 @@ install: country.dat vzdump.conf pve-sources.list pve-initramfs.conf pve-blackli
>>  	install -D -m 0644 pve-initramfs.conf $(DESTDIR)/etc/initramfs-tools/conf.d/pve-initramfs.conf
>>  	install -D -m 0644 country.dat $(DESTDIR)/usr/share/$(PACKAGE)/country.dat
>>  	install -D -m 0644 proxmox-ve-default.link $(DESTDIR)/usr/lib/systemd/network/99-default.link.d/proxmox-mac-address-policy.conf
>> +	install -D -m 0644 pve-tmpfiles.conf $(DESTDIR)/usr/lib/tmpfiles.d/pve-tmpfiles.conf
> 
> You can use dh_installtmpfiles [0] for this and just add the relevant config in
> a "debian/package.tmpfiles" file. With debhelper compat level 13 that helper
> will be run by default [1], and as level 13 is the recommended level for Trixie,
> I plan to switch all packages over to that anyway.
> 
> 0: https://manpages.debian.org/trixie/debhelper/dh_installtmpfiles.1.en.html
> 1: https://manpages.debian.org/testing/debhelper/debhelper-compat-upgrade-checklist.7.en.html#v13
> 

Good to know! Will change it in v2.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-lxc-syscalld 2/4] service: avoid using generic runtime directory name

[Fiona Ebner]

When the service is restarted, the directory will be recreated. The
issue is that the /run/pve directory is not just used as the runtime
directory of this service, but also for other things, e.g. storage
tunnel and mtunnel sockets and container stderr logs as well as pull
metrics, which will be lost when the service is restarted.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Versioned breaks for pve-container and pve-manager needed.

 etc/pve-lxc-syscalld.service.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/etc/pve-lxc-syscalld.service.in b/etc/pve-lxc-syscalld.service.in
index be076a7..0557fe6 100644
--- a/etc/pve-lxc-syscalld.service.in
+++ b/etc/pve-lxc-syscalld.service.in
@@ -4,8 +4,8 @@ Before=pve-guests.service
 
 [Service]
 Type=notify
-ExecStart=%LIBEXECDIR%/pve-lxc-syscalld/pve-lxc-syscalld --system /run/pve/lxc-syscalld.sock
-RuntimeDirectory=pve
+ExecStart=%LIBEXECDIR%/pve-lxc-syscalld/pve-lxc-syscalld --system /run/pve-lxc-syscalld/lxc-syscalld.sock
+RuntimeDirectory=pve-lxc-syscalld
 Restart=on-failure
 
 [Install]
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-lxc-syscalld 2/4] service: avoid using generic runtime directory name

[Thomas Lamprecht]

Am 13.05.25 um 12:56 schrieb Fiona Ebner:
> When the service is restarted, the directory will be recreated. The
> issue is that the /run/pve directory is not just used as the runtime
> directory of this service, but also for other things, e.g. storage
> tunnel and mtunnel sockets and container stderr logs as well as pull
> metrics, which will be lost when the service is restarted.

Is there much gained by moving the socket path or would it be enough to
drop the RundtimeDirectory completely or do we want to rely on the
clean-up being done by systemd? Which probably is an OK enough reason,
I'd think..

btw., to name all options, there would be the RuntimeDirectoryPreserve=
property for systemd services (documented in man systemd.exec) which
we could set to `restart` to handle just updates or `yes` to never clean
it up; besides obviously a system reboot clearing any tmpfs, that is.

> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Versioned breaks for pve-container and pve-manager needed.
> 
>  etc/pve-lxc-syscalld.service.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/etc/pve-lxc-syscalld.service.in b/etc/pve-lxc-syscalld.service.in
> index be076a7..0557fe6 100644
> --- a/etc/pve-lxc-syscalld.service.in
> +++ b/etc/pve-lxc-syscalld.service.in
> @@ -4,8 +4,8 @@ Before=pve-guests.service
>  
>  [Service]
>  Type=notify
> -ExecStart=%LIBEXECDIR%/pve-lxc-syscalld/pve-lxc-syscalld --system /run/pve/lxc-syscalld.sock
> -RuntimeDirectory=pve
> +ExecStart=%LIBEXECDIR%/pve-lxc-syscalld/pve-lxc-syscalld --system /run/pve-lxc-syscalld/lxc-syscalld.sock

If we go for a dedicated directory we could drop the redundancy in the
file name, e.g.

/run/pve-lxc-syscalld/sock

or

/run/pve-lxc-syscalld/socket

> +RuntimeDirectory=pve-lxc-syscalld
>  Restart=on-failure
>  
>  [Install]



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-lxc-syscalld 2/4] service: avoid using generic runtime directory name

[Fiona Ebner]

Am 14.05.25 um 16:33 schrieb Thomas Lamprecht:
> Am 13.05.25 um 12:56 schrieb Fiona Ebner:
>> When the service is restarted, the directory will be recreated. The
>> issue is that the /run/pve directory is not just used as the runtime
>> directory of this service, but also for other things, e.g. storage
>> tunnel and mtunnel sockets and container stderr logs as well as pull
>> metrics, which will be lost when the service is restarted.
> 
> Is there much gained by moving the socket path or would it be enough to
> drop the RundtimeDirectory completely or do we want to rely on the
> clean-up being done by systemd? Which probably is an OK enough reason,
> I'd think..

Yes, the cleanup aspect is there and it seems nicer to have a dedicated
directory IMHO. It would avoid the need to adapt pve-container and the
postinst snippet, but I'd still like to go for the "dedicated
RuntimeDirectory" approach.

> btw., to name all options, there would be the RuntimeDirectoryPreserve=
> property for systemd services (documented in man systemd.exec) which
> we could set to `restart` to handle just updates or `yes` to never clean
> it up; besides obviously a system reboot clearing any tmpfs, that is.

That option would not help with the awkward dependency/reliance on the
service to create the /run/pve directory in the first place. And even if
it's created by the tmpfiles.d snippet in pve-manager first, it will get
overwritten (with root:root 0755 permissions) when its first used as the
service's runtime directory.

>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> Versioned breaks for pve-container and pve-manager needed.
>>
>>  etc/pve-lxc-syscalld.service.in | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/etc/pve-lxc-syscalld.service.in b/etc/pve-lxc-syscalld.service.in
>> index be076a7..0557fe6 100644
>> --- a/etc/pve-lxc-syscalld.service.in
>> +++ b/etc/pve-lxc-syscalld.service.in
>> @@ -4,8 +4,8 @@ Before=pve-guests.service
>>  
>>  [Service]
>>  Type=notify
>> -ExecStart=%LIBEXECDIR%/pve-lxc-syscalld/pve-lxc-syscalld --system /run/pve/lxc-syscalld.sock
>> -RuntimeDirectory=pve
>> +ExecStart=%LIBEXECDIR%/pve-lxc-syscalld/pve-lxc-syscalld --system /run/pve-lxc-syscalld/lxc-syscalld.sock
> 
> If we go for a dedicated directory we could drop the redundancy in the
> file name, e.g.
> 
> /run/pve-lxc-syscalld/sock
> 
> or
> 
> /run/pve-lxc-syscalld/socket

Okay, I'll go for the latter in v2.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-lxc-syscalld 2/4] service: avoid using generic runtime directory name

[Thomas Lamprecht]

Am 15.05.25 um 10:41 schrieb Fiona Ebner:
> Am 14.05.25 um 16:33 schrieb Thomas Lamprecht:
>> Am 13.05.25 um 12:56 schrieb Fiona Ebner:
>>> When the service is restarted, the directory will be recreated. The
>>> issue is that the /run/pve directory is not just used as the runtime
>>> directory of this service, but also for other things, e.g. storage
>>> tunnel and mtunnel sockets and container stderr logs as well as pull
>>> metrics, which will be lost when the service is restarted.
>> Is there much gained by moving the socket path or would it be enough to
>> drop the RundtimeDirectory completely or do we want to rely on the
>> clean-up being done by systemd? Which probably is an OK enough reason,
>> I'd think..
>
> Yes, the cleanup aspect is there and it seems nicer to have a dedicated
> directory IMHO. It would avoid the need to adapt pve-container and the
> postinst snippet, but I'd still like to go for the "dedicated
> RuntimeDirectory" approach.

Alright with me.

>> btw., to name all options, there would be the RuntimeDirectoryPreserve=
>> property for systemd services (documented in man systemd.exec) which
>> we could set to `restart` to handle just updates or `yes` to never clean
>> it up; besides obviously a system reboot clearing any tmpfs, that is.
>
> That option would not help with the awkward dependency/reliance on the
> service to create the /run/pve directory in the first place. And even if
> it's created by the tmpfiles.d snippet in pve-manager first, it will get
> overwritten (with root:root 0755 permissions) when its first used as the
> service's runtime directory.

Fair enough, then let's go with your proposed approach.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-lxc-syscalld 3/4] d/postinst: create link to new socket location on upgrade

[Fiona Ebner]

This allows containers started with a configuration using the old
socket path to continue calling in to the pve-lxc-syscalld, except for
a brief time window after the new runtime dir is used before the
postinst script runs. However, such a time window was/is already
present during service restart (and thus during package upgrade),
since pve-lxc-syscalld doesn't currently implement graceful reloading.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 debian/postinst | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 debian/postinst

diff --git a/debian/postinst b/debian/postinst
new file mode 100644
index 0000000..97abc0c
--- /dev/null
+++ b/debian/postinst
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+set -e
+
+#DEBHELPER#
+
+case "$1" in
+  configure)
+    if test -n "$2"; then
+      # TODO: remove once PVE 10.0 is released
+      if dpkg --compare-versions "$2" 'lt' '1.3.1'; then
+        ln -s -f /run/pve-lxc-syscalld/lxc-syscalld.sock /run/pve/lxc-syscalld.sock
+      fi
+    fi
+    ;;
+
+esac
+
+exit 0
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH container 4/4] seccomp config: adapt to new lxc-syscalld runtime directory

[Fiona Ebner]

The lxc-syscalld now uses a different runtime directory. Its old
runtime directory was /run/pve, which was also used for other things,
e.g. storage tunnel and mtunnel sockets and container stderr logs as
well as pull metrics. The fact that it would be recreated on service
restart is problematic, so the runtime directory was changed.

Note that this configuration is only used for containers with the
experimental 'mknod' feature enabled.

For already running containers, a symbolic link is put into place by
the new version of pve-lxc-syscalld, but newly started ones should
always use the new socket path as soon as it is available. Only use
the old socket path if the old version of pve-lxc-syscalld is still
used. The heuristic to check this is:
1. the new socket path doesn't exist
2. the old socket path exists
3. the old socket path is not a symbolic link

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/LXC.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 2b9f0cf..c42fdde 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -525,7 +525,15 @@ sub make_seccomp_config {
 	    die "'mknod' feature requested, but kernel too old (found $kernel, required >= 5.3)\n";
 	}
 
-	$raw_conf .= "lxc.seccomp.notify.proxy = unix:/run/pve/lxc-syscalld.sock\n";
+	# TODO PVE 10 - always use new socket path
+	my $old_socket_path = '/run/pve/lxc-syscalld.sock';
+	my $new_socket_path = '/run/pve-lxc-syscalld/lxc-syscalld.sock';
+
+	if (!-e $new_socket_path && -e $old_socket_path && !-l $old_socket_path) {
+	    $raw_conf .= "lxc.seccomp.notify.proxy = unix:$old_socket_path\n";
+	} else {
+	    $raw_conf .= "lxc.seccomp.notify.proxy = unix:$new_socket_path\n";
+	}
 	$raw_conf .= "lxc.seccomp.notify.cookie = $vmid\n";
 
 	$rules->{mknod} = [
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel