On Wed, Feb 11, 2026 at 20:20:40 +0000, Daniel P. Berrangé via Devel wrote:
> From: Daniel P. Berrangé <[email protected]>
> 
> The vircommand.c code will always log the argv about to
> be run, so logging it again in virfirewall.c is redundant.
> Removing the dupe avoids the repeated memory allocation
> from the array -> string conversion.

The virCommand infra does that with VIR_DEBUG, while this was doing it
with VIR_INFO priority. I'm okay with removing these but IMO should be
mentioned in the commit message.


> 
> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
>  src/util/virfirewall.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

[...]

> @@ -622,6 +618,7 @@ virFirewallCmdIptablesApply(virFirewall *firewall,
>              VIR_DEBUG("Ignoring error running command");
>              return 0;
>          } else {
> +            g_autofree char *cmdStr = virCommandToString(cmd, false);

Many operations on virCommand, including the whole call to
virCommandRunAsync() from within virCommandRun assert the 'has_error'
flag which in turn will prevent virCommandToString() returning the
command at this point.

Since virCommandRunAsync() captures errors from everything including
virExec this might actually make the errors useless.

virCommandToStringBuf could theoretically be made to skip the check with
an extra flag to avoid this behaviour so that we could be able to get
the command after trying to run it.

Reply via email to