Display the executed command and failure message if a command failed to
execute.
Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
---
v2:
- addressing Eric Blake's comments
---
src/nwfilter/nwfilter_ebiptables_driver.c | 86 +++++++++++++++++-------------
1 file changed, 50 insertions(+), 36 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -65,10 +65,10 @@
#define CMD_DEF_PRE "cmd='"
#define CMD_DEF_POST "'"
#define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST
-#define CMD_EXEC "eval res=\\$\\(\"${cmd}\"\\)" CMD_SEPARATOR
+#define CMD_EXEC "eval res=\\$\\(\"${cmd} 2>&1\"\\)" CMD_SEPARATOR
#define CMD_STOPONERR(X) \
X ? "if [ $? -ne 0 ]; then" \
- " echo \"Failure to execute command '${cmd}'.\";" \
+ " echo \"Failure to execute command '${cmd}' : '${res}'.\";" \
" exit 1;" \
"fi" CMD_SEPARATOR \
: ""
@@ -2707,6 +2707,10 @@ ebiptablesDisplayRuleInstance(virConnect
* execute.
* @status: Pointer to an integer for returning the WEXITSTATUS of the
* commands executed via the script the was run.
+ * @outbuf: Optional pointer to a string that will hold the buffer with
+ * output of the executed command. The actual buffer holding
+ * the message will be newly allocated by this function and
+ * any passed in buffer freed first.
*
* Returns 0 in case of success, < 0 in case of an error. The returned
* value is NOT the result of running the commands inside the shell
@@ -2718,17 +2722,24 @@ ebiptablesDisplayRuleInstance(virConnect
*/
static int
ebiptablesExecCLI(virBufferPtr buf,
- int *status)
+ int *status, char **outbuf)
{
int rc = -1;
virCommandPtr cmd;
- *status = 0;
+ if (status)
+ *status = 0;
+
if (!virBufferError(buf) && !virBufferUse(buf))
return 0;
+ if (outbuf)
+ VIR_FREE(*outbuf);
+
cmd = virCommandNewArgList("/bin/sh", "-c", NULL);
virCommandAddArgBuffer(cmd, buf);
+ if (outbuf)
+ virCommandSetOutputBuffer(cmd, outbuf);
virMutexLock(&execCLIMutex);
@@ -3138,7 +3149,6 @@ ebtablesApplyBasicRules(const char *ifna
const unsigned char *macaddr)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- int cli_status;
char chain[MAX_CHAINNAME_LENGTH];
char chainPrefix = CHAINPREFIX_HOST_IN_TEMP;
char macaddr_str[VIR_MAC_STRING_BUFLEN];
@@ -3193,7 +3203,7 @@ ebtablesApplyBasicRules(const char *ifna
ebtablesLinkTmpRootChain(&buf, 1, ifname, 1);
ebtablesRenameTmpRootChain(&buf, 1, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
goto tear_down_tmpebchains;
return 0;
@@ -3229,7 +3239,6 @@ ebtablesApplyDHCPOnlyRules(const char *i
const char *dhcpserver)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- int cli_status;
char chain_in [MAX_CHAINNAME_LENGTH],
chain_out[MAX_CHAINNAME_LENGTH];
char macaddr_str[VIR_MAC_STRING_BUFLEN];
@@ -3309,7 +3318,7 @@ ebtablesApplyDHCPOnlyRules(const char *i
ebtablesRenameTmpRootChain(&buf, 1, ifname);
ebtablesRenameTmpRootChain(&buf, 0, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
goto tear_down_tmpebchains;
VIR_FREE(srcIPParam);
@@ -3342,7 +3351,6 @@ static int
ebtablesApplyDropAllRules(const char *ifname)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- int cli_status;
char chain_in [MAX_CHAINNAME_LENGTH],
chain_out[MAX_CHAINNAME_LENGTH];
@@ -3382,7 +3390,7 @@ ebtablesApplyDropAllRules(const char *if
ebtablesRenameTmpRootChain(&buf, 1, ifname);
ebtablesRenameTmpRootChain(&buf, 0, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
goto tear_down_tmpebchains;
return 0;
@@ -3425,7 +3433,7 @@ static int ebtablesCleanAll(const char *
ebtablesRemoveTmpRootChain(&buf, 1, ifname);
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
return 0;
}
@@ -3582,6 +3590,7 @@ ebiptablesApplyNewRules(virConnectPtr co
bool haveIp6tables = false;
ebiptablesRuleInstPtr ebtChains = NULL;
int nEbtChains = 0;
+ char *errmsg = NULL;
if (!chains_in_set || !chains_out_set) {
virReportOOMError();
@@ -3620,7 +3629,7 @@ ebiptablesApplyNewRules(virConnectPtr co
ebtablesRemoveTmpSubChains(&buf, ifname);
ebtablesRemoveTmpRootChain(&buf, 1, ifname);
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
}
/* create needed chains */
@@ -3635,7 +3644,7 @@ ebiptablesApplyNewRules(virConnectPtr co
qsort(&ebtChains[0], nEbtChains, sizeof(ebtChains[0]),
ebiptablesRuleOrderSort);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpebchains;
/* process ebtables commands; interleave commands from filters with
@@ -3669,7 +3678,7 @@ ebiptablesApplyNewRules(virConnectPtr co
ebtChains[j++].commandTemplate,
'A', -1, 1);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpebchains;
if (haveIptables) {
@@ -3678,17 +3687,17 @@ ebiptablesApplyNewRules(virConnectPtr co
iptablesCreateBaseChains(iptables_cmd_path, &buf);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpebchains;
iptablesCreateTmpRootChains(iptables_cmd_path, &buf, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpiptchains;
iptablesLinkTmpRootChains(iptables_cmd_path, &buf, ifname);
iptablesSetupVirtInPost(iptables_cmd_path, &buf, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpiptchains;
for (i = 0; i < nruleInstances; i++) {
@@ -3699,7 +3708,7 @@ ebiptablesApplyNewRules(virConnectPtr co
'A', -1, 1);
}
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpiptchains;
iptablesCheckBridgeNFCallEnabled(false);
@@ -3711,17 +3720,17 @@ ebiptablesApplyNewRules(virConnectPtr co
iptablesCreateBaseChains(ip6tables_cmd_path, &buf);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpiptchains;
iptablesCreateTmpRootChains(ip6tables_cmd_path, &buf, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpip6tchains;
iptablesLinkTmpRootChains(ip6tables_cmd_path, &buf, ifname);
iptablesSetupVirtInPost(ip6tables_cmd_path, &buf, ifname);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpip6tchains;
for (i = 0; i < nruleInstances; i++) {
@@ -3731,7 +3740,7 @@ ebiptablesApplyNewRules(virConnectPtr co
'A', -1, 1);
}
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_tmpip6tchains;
iptablesCheckBridgeNFCallEnabled(true);
@@ -3742,7 +3751,7 @@ ebiptablesApplyNewRules(virConnectPtr co
if (virHashSize(chains_out_set) != 0)
ebtablesLinkTmpRootChain(&buf, 0, ifname, 1);
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0)
+ if (ebiptablesExecCLI(&buf, NULL, &errmsg) < 0)
goto tear_down_ebsubchains_and_unlink;
virHashFree(chains_in_set);
@@ -3752,6 +3761,8 @@ ebiptablesApplyNewRules(virConnectPtr co
VIR_FREE(ebtChains[i].commandTemplate);
VIR_FREE(ebtChains);
+ VIR_FREE(errmsg);
+
return 0;
tear_down_ebsubchains_and_unlink:
@@ -3779,12 +3790,14 @@ tear_down_tmpebchains:
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
}
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
virNWFilterReportError(VIR_ERR_BUILD_FIREWALL,
_("Some rules could not be created for "
- "interface %s."),
- ifname);
+ "interface %s%s%s"),
+ ifname,
+ errmsg ? ": " : "",
+ errmsg ? errmsg : "");
exit_free_sets:
virHashFree(chains_in_set);
@@ -3794,6 +3807,8 @@ exit_free_sets:
VIR_FREE(ebtChains[i].commandTemplate);
VIR_FREE(ebtChains);
+ VIR_FREE(errmsg);
+
return 1;
}
@@ -3824,7 +3839,7 @@ ebiptablesTearNewRules(virConnectPtr con
ebtablesRemoveTmpRootChain(&buf, 0, ifname);
}
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
return 0;
}
@@ -3843,7 +3858,7 @@ ebiptablesTearOldRules(virConnectPtr con
iptablesRemoveRootChains(iptables_cmd_path, &buf, ifname);
iptablesRenameTmpRootChains(iptables_cmd_path, &buf, ifname);
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
}
if (ip6tables_cmd_path) {
@@ -3851,7 +3866,7 @@ ebiptablesTearOldRules(virConnectPtr con
iptablesRemoveRootChains(ip6tables_cmd_path, &buf, ifname);
iptablesRenameTmpRootChains(ip6tables_cmd_path, &buf, ifname);
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
}
if (ebtables_cmd_path) {
@@ -3865,7 +3880,7 @@ ebiptablesTearOldRules(virConnectPtr con
ebtablesRenameTmpSubAndRootChains(&buf, ifname);
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
}
return 0;
@@ -3902,7 +3917,7 @@ ebiptablesRemoveRules(virConnectPtr conn
'D', -1,
0);
- if (ebiptablesExecCLI(&buf, &cli_status))
+ if (ebiptablesExecCLI(&buf, &cli_status, NULL))
goto err_exit;
if (cli_status) {
@@ -3953,7 +3968,7 @@ ebiptablesAllTeardown(const char *ifname
ebtablesRemoveRootChain(&buf, 1, ifname);
ebtablesRemoveRootChain(&buf, 0, ifname);
}
- ebiptablesExecCLI(&buf, &cli_status);
+ ebiptablesExecCLI(&buf, &cli_status, NULL);
return 0;
}
@@ -3987,7 +4002,6 @@ static int
ebiptablesDriverInit(bool privileged)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- int cli_status;
if (!privileged)
return 0;
@@ -4008,7 +4022,7 @@ ebiptablesDriverInit(bool privileged)
ebtables_cmd_path, EBTABLES_DEFAULT_TABLE,
CMD_STOPONERR(1));
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status)
+ if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
VIR_FREE(ebtables_cmd_path);
}
@@ -4021,7 +4035,7 @@ ebiptablesDriverInit(bool privileged)
iptables_cmd_path,
CMD_STOPONERR(1));
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status)
+ if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
VIR_FREE(iptables_cmd_path);
}
@@ -4034,7 +4048,7 @@ ebiptablesDriverInit(bool privileged)
ip6tables_cmd_path,
CMD_STOPONERR(1));
- if (ebiptablesExecCLI(&buf, &cli_status) || cli_status)
+ if (ebiptablesExecCLI(&buf, NULL, NULL) < 0)
VIR_FREE(ip6tables_cmd_path);
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list