Re: [pve-devel] [PATCH lxcfs 1/1] d/rules: Allow passing options to the lxcfs systemd unit

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

Hi!

sorry once for that it took so long to get back to you!

with the nits below addressed, consider this

Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

it would be nice if the systemd unit change (and potentially the default
file?) could also be submitted for upstream inclusion, so that we can
reduce this delta again, and non-Proxmox distributions could also
benefit from it :)

On May 13, 2024 4:21 pm, Antonio Huete Jimenez wrote:
> Fixes: https://forum.proxmox.com/threads/45724
> Signed-off-by: Antonio Huete <tuxillo@quantumachine.net>

it seems we don't yet have a signed CLA from you on record, please see

https://pve.proxmox.com/wiki/Developer_Documentation#Software_License_and_Copyright

> ---
>  debian/changelog                        |  6 ++++++
>  debian/lxcfs.default                    |  4 ++++
>  debian/patches/allow-passing-opts.patch | 16 ++++++++++++++++
>  debian/patches/series                   |  1 +
>  debian/rules                            |  3 +++
>  5 files changed, 30 insertions(+)
>  create mode 100644 debian/lxcfs.default
>  create mode 100644 debian/patches/allow-passing-opts.patch
> 
> diff --git a/debian/changelog b/debian/changelog
> index 1643126..67ec130 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,9 @@
> +lxcfs (6.0.0-pve3) bookworm; urgency=medium
> +
> +  * Allow passing opts to the lxcfs systemd unit
> +
> + -- Proxmox Support Team <support@proxmox.com>  Mon, 12 May 2024 08:01:01 +0200
> +

nit: we don't usually do this as part of a patch submission, since cutting
releases is usually done after accumulating a few fixes, and it also
means the patch doesn't apply if the changelog got bumped in the
meantime.

>  lxcfs (6.0.0-pve2) bookworm; urgency=medium
>  
>    * don't treat reload failure as fatal in postinst
> diff --git a/debian/lxcfs.default b/debian/lxcfs.default
> new file mode 100644
> index 0000000..1d3bc09
> --- /dev/null
> +++ b/debian/lxcfs.default
> @@ -0,0 +1,4 @@
> +# Default settings for lxcfs.
> +
> +# lxcfs specific options
> +LXCFS_OPTS=""

we could maybe add the load-related option as an example here, since it
will probably be the main use case..

> diff --git a/debian/patches/allow-passing-opts.patch b/debian/patches/allow-passing-opts.patch
> new file mode 100644
> index 0000000..487d061
> --- /dev/null
> +++ b/debian/patches/allow-passing-opts.patch
> @@ -0,0 +1,16 @@
> +Index: lxcfs-6.0.0/config/init/systemd/lxcfs.service.in
> +===================================================================
> +--- new.orig/config/init/systemd/lxcfs.service.in
> ++++ new/config/init/systemd/lxcfs.service.in
> +@@ -5,9 +5,10 @@ Before=lxc.service
> + Documentation=man:lxcfs(1)
> + 
> + [Service]
> ++EnvironmentFile=-/etc/default/lxcfs
> + OOMScoreAdjust=-1000
> + ExecStartPre=/bin/mkdir -p {{LXCFSTARGETDIR}}
> +-ExecStart=/usr/bin/lxcfs {{LXCFSTARGETDIR}}
> ++ExecStart=/usr/bin/lxcfs ${LXCFS_OPTS} {{LXCFSTARGETDIR}}
> + KillMode=process
> + Restart=on-failure
> + ExecStopPost=-/bin/fusermount -u {{LXCFSTARGETDIR}}
> diff --git a/debian/patches/series b/debian/patches/series
> index bf650b4..cc36fad 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -1 +1,2 @@
> +allow-passing-opts.patch
>  do-not-start-without-lxcfs.patch
> diff --git a/debian/rules b/debian/rules
> index c2e6dbb..ad0e0cc 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -18,5 +18,8 @@ override_dh_auto_configure:
>  override_dh_auto_install:
>  	dh_auto_install --destdir=debian/lxcfs
>  
> +override_dh_installinit:
> +	dh_installinit -p lxcfs --no-stop-on-upgrade
> +

this shouldn't be needed (and in fact, leaving it out produces the exact
same package)

>  override_dh_installsystemd:
>  	dh_installsystemd -p lxcfs --no-start --no-stop-on-upgrade lxcfs.service
> -- 
> 2.42.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


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