Subject: Replace as much external scripting with calls to virRun This patch replaces as much external scripting as possible with calls to virRun(). The structure of the code is maintained as much as possible and what previously ran an external script now executes the commands one after the other using virRun(). Commands whose return value may not indicate failure terminate the execution due to a string 'STOPONERROR' being found in the 'batch'. Signed-off-by: Stefan Berger --- src/nwfilter/nwfilter_ebiptables_driver.c | 199 +++++++++++++++++++++++++++++- 1 file changed, 198 insertions(+), 1 deletion(-) 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 @@ -23,6 +23,7 @@ #include +#include #include #include "internal.h" @@ -49,19 +50,17 @@ #define CHAINPREFIX_HOST_OUT_TEMP 'P' +#define STOPONERROR_STR "STOPONERROR" + #define CMD_SEPARATOR "\n" -#define CMD_DEF_PRE "cmd=\"" -#define CMD_DEF_POST "\"" +#define CMD_DEF_PRE "" +#define CMD_DEF_POST "" #define CMD_DEF(X) CMD_DEF_PRE X CMD_DEF_POST -#define CMD_EXEC "res=`${cmd}`" CMD_SEPARATOR +#define CMD_EXEC "" #define CMD_STOPONERR(X) \ - X ? "if [ $? -ne 0 ]; then" \ - " echo \"Failure to execute command '${cmd}'.\";" \ - " exit 1;" \ - "fi" CMD_SEPARATOR \ + X ? STOPONERROR_STR CMD_SEPARATOR \ : "" - #define EBTABLES_CMD EBTABLES_PATH #define IPTABLES_CMD IPTABLES_PATH #define BASH_CMD BASH_PATH @@ -330,8 +329,7 @@ static int iptablesLinkIPTablesBaseChain virBufferPtr buf, const char *udchain, const char *syschain, - unsigned int pos, - int stopOnError) + unsigned int pos) { virBufferVSprintf(buf, "res=$(" @@ -342,13 +340,19 @@ static int iptablesLinkIPTablesBaseChain "else\n" " r=$(echo $res | " GAWK_CMD " '{print $1}')\n" " if [ \"${r}\" != \"%d\" ]; then\n" - " " CMD_DEF(IPTABLES_CMD " -I %s %d -j %s") CMD_SEPARATOR - " " CMD_EXEC - " %s" + " cmd=\"" IPTABLES_CMD " -I %s %d -j %s\"" CMD_SEPARATOR + " res=`${cmd}`" CMD_SEPARATOR + " if [ $? -ne 0 ]; then\n" + " echo \"Failure to execute command '${cmd}'.\";\n" + " exit 1;\n" + " fi\n" " let r=r+1\n" - " " CMD_DEF(IPTABLES_CMD " -D %s ${r}") CMD_SEPARATOR - " " CMD_EXEC - " %s" + " cmd=\"" IPTABLES_CMD " -D %s ${r}\"" CMD_SEPARATOR + " res=`${cmd}`" CMD_SEPARATOR + " if [ $? -ne 0 ]; then\n" + " echo \"Failure to execute command '${cmd}'.\";\n" + " exit 1;\n" + " fi\n" " fi\n" "fi\n", @@ -359,10 +363,8 @@ static int iptablesLinkIPTablesBaseChain pos, syschain, pos, udchain, - CMD_STOPONERR(stopOnError), - syschain, - CMD_STOPONERR(stopOnError)); + syschain); return 0; } @@ -375,13 +377,13 @@ static int iptablesCreateBaseChains(virC IPTABLES_CMD " -N " VIRT_IN_POST_CHAIN CMD_SEPARATOR IPTABLES_CMD " -N " HOST_IN_CHAIN CMD_SEPARATOR); iptablesLinkIPTablesBaseChain(conn, buf, - VIRT_IN_CHAIN , "FORWARD", 1, 1); + VIRT_IN_CHAIN , "FORWARD", 1); iptablesLinkIPTablesBaseChain(conn, buf, - VIRT_OUT_CHAIN , "FORWARD", 2, 1); + VIRT_OUT_CHAIN , "FORWARD", 2); iptablesLinkIPTablesBaseChain(conn, buf, - VIRT_IN_POST_CHAIN, "FORWARD", 3, 1); + VIRT_IN_POST_CHAIN, "FORWARD", 3); iptablesLinkIPTablesBaseChain(conn, buf, - HOST_IN_CHAIN , "INPUT" , 1, 1); + HOST_IN_CHAIN , "INPUT" , 1); return 0; } @@ -558,16 +560,18 @@ iptablesSetupVirtInPost(virConnectPtr co virBufferVSprintf(buf, "res=$(" IPTABLES_CMD " -L " VIRT_IN_POST_CHAIN " | grep \"\\%s %s\")\n" - "if [ \"${res}\" == \"\" ]; then " - CMD_DEF(IPTABLES_CMD + "if [ \"${res}\" == \"\" ]; then\n" + " cmd=\"" IPTABLES_CMD " -A " VIRT_IN_POST_CHAIN - " %s %s -j ACCEPT") CMD_SEPARATOR - CMD_EXEC - "%s" + " %s %s -j ACCEPT\"" CMD_SEPARATOR + " res=`${cmd}`" CMD_SEPARATOR + " if [ $? -ne 0 ]; then\n" + " echo \"Failure to execute command '${cmd}'.\";\n" + " exit 1;\n" + " fi\n" "fi\n", PHYSDEV_IN, ifname, - match, ifname, - CMD_STOPONERR(1)); + match, ifname); return 0; } @@ -1959,6 +1963,110 @@ err_exit: } +static int +ebiptablesExecScript(virConnectPtr conn, + virBufferPtr buf, + int *status) +{ + int rc = 0; + char *cmds; + char *filename; + const char *argv[] = {NULL, NULL}; + + if (virBufferError(buf)) { + virReportOOMError(); + virBufferFreeAndReset(buf); + return 1; + } + + *status = 0; + + cmds = virBufferContentAndReset(buf); + + VIR_DEBUG("%s", cmds); + + if (!cmds) + return 0; + + filename = ebiptablesWriteToTempFile(conn, cmds); + VIR_FREE(cmds); + + if (!filename) + return 1; + + argv[0] = filename; + rc = virRun(argv, status); + + *status >>= 8; + + VIR_DEBUG("rc = %d, status = %d\n",rc, *status); + + unlink(filename); + + VIR_FREE(filename); + + return rc; +} + + +static const char ** +virStringToArgv(const char *line) { + int i,c = 0; + const char *next = line, *beg; + const char **ret; + + while (1) { + while (isspace(*next)) + next++; + if (*next == 0) + break; + while (*next && !isspace(*next)) + next++; + c++; + } + + c++; + + if (VIR_ALLOC_N(ret, c) < 0) { + virReportOOMError(); + return NULL; + } + + c = 0; + + next = line; + while (1) { + while (isspace(*next)) + next++; + if (*next == 0) + break; + beg = next; + while (*next && !isspace(*next)) + next++; + ret[c] = strndup(beg, next-beg); + if (!ret[c]) + goto err_exit; + c++; + } + return ret; + + err_exit: + for (i = 0; i < c; i++) + VIR_FREE(ret[c]); + VIR_FREE(ret); + return NULL; +} + + +static void +virFreeStrArray(const char ** arr) { + int i = 0; + while (arr[i]) + VIR_FREE(arr[i++]); + VIR_FREE(arr); +} + + /** * ebiptablesExecCLI: * @conn : pointer to virConnect object @@ -1975,14 +2083,16 @@ err_exit: * script and return the status of the execution. */ static int -ebiptablesExecCLI(virConnectPtr conn, +ebiptablesExecCLI(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferPtr buf, int *status) { + int rc = 0; char *cmds; - char *filename; - int rc; - const char *argv[] = {NULL, NULL}; + char *cmd, *next; + const char **argv; + int cmd_sep_strlen = strlen(CMD_SEPARATOR); + int stoponerr_strlen = strlen(STOPONERROR_STR); if (virBufferError(buf)) { virReportOOMError(); @@ -1999,24 +2109,47 @@ ebiptablesExecCLI(virConnectPtr conn, if (!cmds) return 0; - filename = ebiptablesWriteToTempFile(conn, cmds); - VIR_FREE(cmds); - - if (!filename) - return 1; + cmd = cmds; + while (cmd) { + next = strstr(cmd, CMD_SEPARATOR); + if (next) { + *next = 0; + next += cmd_sep_strlen; + } + argv = virStringToArgv(cmd); - argv[0] = filename; - rc = virRun(argv, status); + if (!argv) + return 1; - *status >>= 8; + rc = virRun(argv, status); - VIR_DEBUG("rc = %d, status = %d\n",rc, *status); + virFreeStrArray(argv); + if (rc) + return rc; - unlink(filename); + VIR_DEBUG("cmd=%s rc=%d status=%d\n",cmd, rc, *status >> 8); - VIR_FREE(filename); + while (isspace(*next)) + next++; - return rc; + if (STREQLEN(next, STOPONERROR_STR, stoponerr_strlen)) { + if (*status) { + *status >>= 8; + return rc; + } + next = strstr(next, CMD_SEPARATOR); + if (next) + next += cmd_sep_strlen; + } + if (next) { + while (isspace(*next)) + next++; + } + if (!next || *next == 0) + break; + cmd = next; + } + return 0; } @@ -2454,9 +2587,13 @@ ebiptablesApplyNewRules(virConnectPtr co iptablesUnlinkTmpRootChains(conn, &buf, ifname); iptablesRemoveTmpRootChains(conn, &buf, ifname); + if (ebiptablesExecCLI(conn, &buf, &cli_status)) + goto tear_down_tmpebchains; + iptablesCreateBaseChains(conn, &buf); - if (ebiptablesExecCLI(conn, &buf, &cli_status) || cli_status != 0) + if (ebiptablesExecScript(conn, &buf, &cli_status) || + cli_status != 0) goto tear_down_tmpebchains; iptablesCreateTmpRootChains(conn, &buf, ifname); @@ -2465,10 +2602,15 @@ ebiptablesApplyNewRules(virConnectPtr co goto tear_down_tmpiptchains; iptablesLinkTmpRootChains(conn, &buf, ifname); - iptablesSetupVirtInPost(conn, &buf, ifname); + if (ebiptablesExecCLI(conn, &buf, &cli_status) || cli_status != 0) goto tear_down_tmpiptchains; + iptablesSetupVirtInPost(conn, &buf, ifname); + + if (ebiptablesExecScript(conn, &buf, &cli_status) || cli_status != 0) + goto tear_down_tmpiptchains; + for (i = 0; i < nruleInstances; i++) { if (inst[i]->ruleType == RT_IPTABLES) iptablesInstCommand(conn, &buf,