public inbox for pve-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal