On Sat, 2008-01-05 at 00:11 +0000, Daniel P. Berrange wrote:
> On Fri, Jan 04, 2008 at 03:57:31PM +0000, Mark McLoughlin wrote:
> > The next patch requires iptablesSpawn() higher up in
> > the file. This patch just moves the code around; there
> > is no functional change.
>
> The util.c file has a virExec function, although its a little more
> cumbersome to use than iptablesSpawn. So for the storage APIs I've
> got a new virRun() function in util.c which is very similar to your
> iptablesSpawn code. How about we just make iptables use the virRun
> method in the attached patch ?
Okay, the only thing lost with this is that when using WITH_ERRORS,
iptables error spew would go to libvirtd stderr, now it goes
to /dev/null. No big deal.
(I'll fold the attached patch into the "use utils.[ch]" patch)
Cheers,
Mark.
Index: libvirt/src/iptables.c
===================================================================
--- libvirt.orig/src/iptables.c 2008-01-07 09:49:37.000000000 +0000
+++ libvirt.orig/src/iptables.c 2008-01-07 09:49:37.000000000 +0000
@@ -53,11 +53,6 @@ enum {
REMOVE
};
-enum {
- WITH_ERRORS = 0,
- NO_ERRORS
-};
-
typedef struct
{
char *rule;
@@ -294,54 +289,11 @@ iptRulesNew(const char *table,
}
static int
-iptablesSpawn(int errors, char * const *argv)
-{
- pid_t pid, ret;
- int status;
- int null = -1;
-
- if (errors == NO_ERRORS && (null = open(_PATH_DEVNULL, O_RDONLY)) < 0)
- return errno;
-
- pid = fork();
- if (pid == -1) {
- if (errors == NO_ERRORS)
- close(null);
- return errno;
- }
-
- if (pid == 0) { /* child */
- if (errors == NO_ERRORS) {
- dup2(null, STDIN_FILENO);
- dup2(null, STDOUT_FILENO);
- dup2(null, STDERR_FILENO);
- close(null);
- }
-
- execvp(argv[0], argv);
-
- _exit (1);
- }
-
- if (errors == NO_ERRORS)
- close(null);
-
- while ((ret = waitpid(pid, &status, 0) == -1) && errno == EINTR);
- if (ret == -1)
- return errno;
-
- if (errors == NO_ERRORS)
- return 0;
- else
- return (WIFEXITED(status) && WEXITSTATUS(status) == 0) ? 0 : EINVAL;
-}
-
-static int
iptablesAddRemoveChain(iptRules *rules, int action)
{
char **argv;
int retval = ENOMEM;
- int n;
+ int n, status;
n = 1 + /* /sbin/iptables */
2 + /* --table foo */
@@ -367,7 +319,10 @@ iptablesAddRemoveChain(iptRules *rules,
if (!(argv[n++] = strdup(rules->chain)))
goto error;
- retval = iptablesSpawn(NO_ERRORS, argv);
+ if (virRun(NULL, argv, &status) < 0)
+ retval = errno;
+
+ retval = 0;
error:
if (argv) {
@@ -455,8 +410,10 @@ iptablesAddRemoveRule(iptRules *rules, i
(retval = iptablesAddRemoveChain(rules, action)))
goto error;
- if ((retval = iptablesSpawn(WITH_ERRORS, argv)))
+ if (virRun(NULL, argv, NULL) < 0) {
+ retval = errno;
goto error;
+ }
if (action == REMOVE &&
(retval = iptablesAddRemoveChain(rules, action)))
@@ -546,9 +503,9 @@ iptRulesReload(iptRules *rules)
orig = rule->argv[rule->flipflop];
rule->argv[rule->flipflop] = (char *) "--delete";
- if ((retval = iptablesSpawn(WITH_ERRORS, rule->argv)))
+ if (virRun(NULL, rule->argv, NULL) < 0)
qemudLog(QEMUD_WARN, "Failed to remove iptables rule '%s' from chain '%s' in table '%s': %s",
- rule->rule, rules->chain, rules->table, strerror(retval));
+ rule->rule, rules->chain, rules->table, strerror(errno));
rule->argv[rule->flipflop] = orig;
}
@@ -559,9 +516,9 @@ iptRulesReload(iptRules *rules)
rules->chain, rules->table, strerror(retval));
for (i = 0; i < rules->nrules; i++)
- if ((retval = iptablesSpawn(WITH_ERRORS, rules->rules[i].argv)))
+ if (virRun(NULL, rules->rules[i].argv, NULL) < 0)
qemudLog(QEMUD_WARN, "Failed to add iptables rule '%s' to chain '%s' in table '%s': %s",
- rules->rules[i].rule, rules->chain, rules->table, strerror(retval));
+ rules->rules[i].rule, rules->chain, rules->table, strerror(errno));
}
/**
--
Libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
