all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH proxmox-firewall 1/2] firewall: wait for nft process
@ 2024-04-19 13:00 Stefan Hanreich
  2024-04-19 13:00 ` [pve-devel] [PATCH proxmox-firewall 2/2] firewall: improve systemd unit file Stefan Hanreich
  2024-04-19 17:45 ` [pve-devel] applied-series: [PATCH proxmox-firewall 1/2] firewall: wait for nft process Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Hanreich @ 2024-04-19 13:00 UTC (permalink / raw)
  To: pve-devel

NftClient never waits for the child process to terminate leading to
defunct leftover processes.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 proxmox-nftables/src/client.rs | 38 ++++++++--------------------------
 1 file changed, 9 insertions(+), 29 deletions(-)

diff --git a/proxmox-nftables/src/client.rs b/proxmox-nftables/src/client.rs
index 69e464b..eaa3dd2 100644
--- a/proxmox-nftables/src/client.rs
+++ b/proxmox-nftables/src/client.rs
@@ -36,35 +36,15 @@ impl NftClient {
             return Err(NftError::from(error));
         };
 
-        let mut error_output = String::new();
-
-        match child
-            .stderr
-            .take()
-            .expect("can get stderr")
-            .read_to_string(&mut error_output)
-        {
-            Ok(_) if !error_output.is_empty() => {
-                return Err(NftError::Command(error_output));
-            }
-            Err(error) => {
-                return Err(NftError::from(error));
-            }
-            _ => (),
-        };
-
-        let mut output = String::new();
-
-        if let Err(error) = child
-            .stdout
-            .take()
-            .expect("can get stdout")
-            .read_to_string(&mut output)
-        {
-            return Err(NftError::from(error));
-        };
-
-        Ok(output)
+        let output = child.wait_with_output().map_err(NftError::from)?;
+
+        if output.status.success() {
+            Ok(String::from_utf8(output.stdout).expect("output is valid utf-8"))
+        } else {
+            Err(NftError::Command(
+                String::from_utf8(output.stderr).expect("output is valid utf-8"),
+            ))
+        }
     }
 
     pub fn run_json_commands(commands: &Commands) -> Result<Option<CommandOutput>, NftError> {
-- 
2.39.2


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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] [PATCH proxmox-firewall 2/2] firewall: improve systemd unit file
  2024-04-19 13:00 [pve-devel] [PATCH proxmox-firewall 1/2] firewall: wait for nft process Stefan Hanreich
@ 2024-04-19 13:00 ` Stefan Hanreich
  2024-04-19 17:45 ` [pve-devel] applied-series: [PATCH proxmox-firewall 1/2] firewall: wait for nft process Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hanreich @ 2024-04-19 13:00 UTC (permalink / raw)
  To: pve-devel

Explicitly mark the service as simple and remove the PIDFile
attribute, which doesn't do anything with simple services.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 debian/proxmox-firewall.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/proxmox-firewall.service b/debian/proxmox-firewall.service
index ad2324b..c2dc903 100644
--- a/debian/proxmox-firewall.service
+++ b/debian/proxmox-firewall.service
@@ -5,7 +5,7 @@ After=pvefw-logger.service pve-cluster.service network.target systemd-modules-lo
 
 [Service]
 ExecStart=/usr/libexec/proxmox/proxmox-firewall
-PIDFile=/run/proxmox-firewall.pid
+Type=simple
 Environment="RUST_LOG_STYLE=SYSTEMD"
 Environment="RUST_LOG=warn"
 
-- 
2.39.2


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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [pve-devel] applied-series: [PATCH proxmox-firewall 1/2] firewall: wait for nft process
  2024-04-19 13:00 [pve-devel] [PATCH proxmox-firewall 1/2] firewall: wait for nft process Stefan Hanreich
  2024-04-19 13:00 ` [pve-devel] [PATCH proxmox-firewall 2/2] firewall: improve systemd unit file Stefan Hanreich
@ 2024-04-19 17:45 ` Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2024-04-19 17:45 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

Am 19/04/2024 um 15:00 schrieb Stefan Hanreich:
> NftClient never waits for the child process to terminate leading to
> defunct leftover processes.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  proxmox-nftables/src/client.rs | 38 ++++++++--------------------------
>  1 file changed, 9 insertions(+), 29 deletions(-)
> 
>

applied both patches, thanks!


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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-04-19 17:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-19 13:00 [pve-devel] [PATCH proxmox-firewall 1/2] firewall: wait for nft process Stefan Hanreich
2024-04-19 13:00 ` [pve-devel] [PATCH proxmox-firewall 2/2] firewall: improve systemd unit file Stefan Hanreich
2024-04-19 17:45 ` [pve-devel] applied-series: [PATCH proxmox-firewall 1/2] firewall: wait for nft process Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal