Display the executed command and failure message if a command failed to execute. Signed-off-by: Stefan Berger --- src/nwfilter/nwfilter_ebiptables_driver.c | 82 ++++++++++++++++++------------ 1 file changed, 50 insertions(+), 32 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 @@ -63,10 +63,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 \ : "" @@ -2785,12 +2785,16 @@ err_exit: */ static int ebiptablesExecCLI(virBufferPtr buf, - int *status) + int *status, char **errbuf) { char *cmds; char *filename; int rc; const char *argv[] = {NULL, NULL}; + virCommandPtr cmd; + + if (errbuf) + VIR_FREE(*errbuf); if (virBufferError(buf)) { virReportOOMError(); @@ -2817,7 +2821,13 @@ ebiptablesExecCLI(virBufferPtr buf, virMutexLock(&execCLIMutex); - rc = virRun(argv, status); + cmd = virCommandNewArgs(argv); + if (errbuf) + virCommandSetOutputBuffer(cmd, errbuf); + + rc = virCommandRun(cmd, status); + + virCommandFree(cmd); virMutexUnlock(&execCLIMutex); @@ -2831,6 +2841,8 @@ ebiptablesExecCLI(virBufferPtr buf, } VIR_DEBUG("rc = %d, status = %d", rc, *status); + if (errbuf && *errbuf) + VIR_DEBUG("error message: %s", *errbuf); unlink(filename); @@ -3285,7 +3297,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, &cli_status, NULL) || cli_status != 0) goto tear_down_tmpebchains; return 0; @@ -3401,7 +3413,7 @@ ebtablesApplyDHCPOnlyRules(const char *i ebtablesRenameTmpRootChain(&buf, 1, ifname); ebtablesRenameTmpRootChain(&buf, 0, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status != 0) goto tear_down_tmpebchains; VIR_FREE(srcIPParam); @@ -3474,7 +3486,7 @@ ebtablesApplyDropAllRules(const char *if ebtablesRenameTmpRootChain(&buf, 1, ifname); ebtablesRenameTmpRootChain(&buf, 0, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status != 0) goto tear_down_tmpebchains; return 0; @@ -3517,7 +3529,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; } @@ -3677,6 +3689,7 @@ ebiptablesApplyNewRules(virConnectPtr co bool haveIp6tables = false; ebiptablesRuleInstPtr ebtChains = NULL; int nEbtChains = 0; + char *errmsg = NULL; if (!chains_in_set || !chains_out_set) { virReportOOMError(); @@ -3715,7 +3728,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 */ @@ -3730,7 +3743,7 @@ ebiptablesApplyNewRules(virConnectPtr co qsort(&ebtChains[0], nEbtChains, sizeof(ebtChains[0]), ebiptablesRuleOrderSort); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpebchains; /* process ebtables commands; interleave commands from filters with @@ -3764,7 +3777,7 @@ ebiptablesApplyNewRules(virConnectPtr co ebtChains[j++].commandTemplate, 'A', -1, 1); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpebchains; if (haveIptables) { @@ -3773,17 +3786,17 @@ ebiptablesApplyNewRules(virConnectPtr co iptablesCreateBaseChains(iptables_cmd_path, &buf); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpebchains; iptablesCreateTmpRootChains(iptables_cmd_path, &buf, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 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, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpiptchains; for (i = 0; i < nruleInstances; i++) { @@ -3794,7 +3807,7 @@ ebiptablesApplyNewRules(virConnectPtr co 'A', -1, 1); } - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpiptchains; iptablesCheckBridgeNFCallEnabled(false); @@ -3806,17 +3819,17 @@ ebiptablesApplyNewRules(virConnectPtr co iptablesCreateBaseChains(ip6tables_cmd_path, &buf); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpiptchains; iptablesCreateTmpRootChains(ip6tables_cmd_path, &buf, ifname); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 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, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpip6tchains; for (i = 0; i < nruleInstances; i++) { @@ -3826,7 +3839,7 @@ ebiptablesApplyNewRules(virConnectPtr co 'A', -1, 1); } - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) + if (ebiptablesExecCLI(&buf, &cli_status, &errmsg) || cli_status != 0) goto tear_down_tmpip6tchains; iptablesCheckBridgeNFCallEnabled(true); @@ -3837,7 +3850,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, &cli_status, &errmsg) || cli_status != 0) goto tear_down_ebsubchains_and_unlink; virHashFree(chains_in_set); @@ -3847,6 +3860,8 @@ ebiptablesApplyNewRules(virConnectPtr co VIR_FREE(ebtChains[i].commandTemplate); VIR_FREE(ebtChains); + VIR_FREE(errmsg); + return 0; tear_down_ebsubchains_and_unlink: @@ -3874,12 +3889,13 @@ 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"), + ifname, + errmsg ? errmsg : ""); exit_free_sets: virHashFree(chains_in_set); @@ -3889,6 +3905,8 @@ exit_free_sets: VIR_FREE(ebtChains[i].commandTemplate); VIR_FREE(ebtChains); + VIR_FREE(errmsg); + return 1; } @@ -3919,7 +3937,7 @@ ebiptablesTearNewRules(virConnectPtr con ebtablesRemoveTmpRootChain(&buf, 0, ifname); } - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); return 0; } @@ -3938,7 +3956,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) { @@ -3946,7 +3964,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) { @@ -3960,7 +3978,7 @@ ebiptablesTearOldRules(virConnectPtr con ebtablesRenameTmpSubAndRootChains(&buf, ifname); - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); } return 0; @@ -3997,7 +4015,7 @@ ebiptablesRemoveRules(virConnectPtr conn 'D', -1, 0); - if (ebiptablesExecCLI(&buf, &cli_status)) + if (ebiptablesExecCLI(&buf, &cli_status, NULL)) goto err_exit; if (cli_status) { @@ -4048,7 +4066,7 @@ ebiptablesAllTeardown(const char *ifname ebtablesRemoveRootChain(&buf, 1, ifname); ebtablesRemoveRootChain(&buf, 0, ifname); } - ebiptablesExecCLI(&buf, &cli_status); + ebiptablesExecCLI(&buf, &cli_status, NULL); return 0; } @@ -4103,7 +4121,7 @@ ebiptablesDriverInit(bool privileged) ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, CMD_STOPONERR(1)); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status) + if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status) VIR_FREE(ebtables_cmd_path); } @@ -4116,7 +4134,7 @@ ebiptablesDriverInit(bool privileged) iptables_cmd_path, CMD_STOPONERR(1)); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status) + if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status) VIR_FREE(iptables_cmd_path); } @@ -4129,7 +4147,7 @@ ebiptablesDriverInit(bool privileged) ip6tables_cmd_path, CMD_STOPONERR(1)); - if (ebiptablesExecCLI(&buf, &cli_status) || cli_status) + if (ebiptablesExecCLI(&buf, &cli_status, NULL) || cli_status) VIR_FREE(ip6tables_cmd_path); }