[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 3/3] Port hooks and iptables code to new command execution APIs



This proof of concept shows how two existing uses of virExec
and virRun can be ported to the new virCommand APIs, and how
much simpler the code becomes
---
 src/util/hooks.c    |  221 +++-----------------------------------------------
 src/util/iptables.c |   70 +++-------------
 2 files changed, 27 insertions(+), 264 deletions(-)

diff --git a/src/util/hooks.c b/src/util/hooks.c
index dec9223..d03a76e 100644
--- a/src/util/hooks.c
+++ b/src/util/hooks.c
@@ -36,6 +36,7 @@
 #include "conf/domain_conf.h"
 #include "logging.h"
 #include "memory.h"
+#include "command.h"
 
 #define VIR_FROM_THIS VIR_FROM_HOOK
 
@@ -188,36 +189,15 @@ virHookPresent(int driver) {
  * Returns: 0 if the execution succeeded, 1 if the script was not found or
  *          invalid parameters, and -1 if script returned an error
  */
-#ifdef WIN32
-int
-virHookCall(int driver ATTRIBUTE_UNUSED,
-            const char *id ATTRIBUTE_UNUSED,
-            int op ATTRIBUTE_UNUSED,
-            int sub_op ATTRIBUTE_UNUSED,
-            const char *extra ATTRIBUTE_UNUSED,
-            const char *input ATTRIBUTE_UNUSED) {
-    virReportSystemError(ENOSYS, "%s",
-                         _("spawning hooks not supported on this platform"));
-    return -1;
-}
-#else
 int
 virHookCall(int driver, const char *id, int op, int sub_op, const char *extra,
             const char *input) {
-    int ret, waitret, exitstatus, i;
+    int ret;
     char *path;
-    int argc = 0, arga = 0;
-    const char **argv = NULL;
-    int envc = 0, enva = 0;
-    const char **env = NULL;
+    virCommandPtr cmd;
     const char *drvstr;
     const char *opstr;
     const char *subopstr;
-    pid_t pid;
-    int outfd = -1, errfd = -1;
-    int pipefd[2] = { -1, -1};
-    char *outbuf = NULL;
-    char *errbuf = NULL;
 
     if ((driver < VIR_HOOK_DRIVER_DAEMON) ||
         (driver >= VIR_HOOK_DRIVER_LAST))
@@ -269,195 +249,22 @@ virHookCall(int driver, const char *id, int op, int sub_op, const char *extra,
         return(-1);
     }
 
-    /*
-     * Convenience macros borrowed from qemudBuildCommandLine()
-     */
-# define ADD_ARG_SPACE                                                   \
-    do {                                                                \
-        if (argc == arga) {                                             \
-            arga += 10;                                                 \
-            if (VIR_REALLOC_N(argv, arga) < 0)                          \
-                goto no_memory;                                         \
-        }                                                               \
-    } while (0)
-
-# define ADD_ARG(thisarg)                                                \
-    do {                                                                \
-        ADD_ARG_SPACE;                                                  \
-        argv[argc++] = thisarg;                                         \
-    } while (0)
-
-# define ADD_ARG_LIT(thisarg)                                            \
-    do {                                                                \
-        ADD_ARG_SPACE;                                                  \
-        if ((argv[argc++] = strdup(thisarg)) == NULL)                   \
-            goto no_memory;                                             \
-    } while (0)
-
-# define ADD_ENV_SPACE                                                   \
-    do {                                                                \
-        if (envc == enva) {                                             \
-            enva += 10;                                                 \
-            if (VIR_REALLOC_N(env, enva) < 0)                           \
-                goto no_memory;                                         \
-        }                                                               \
-    } while (0)
-
-# define ADD_ENV(thisarg)                                                \
-    do {                                                                \
-        ADD_ENV_SPACE;                                                  \
-        env[envc++] = thisarg;                                          \
-    } while (0)
-
-# define ADD_ENV_LIT(thisarg)                                            \
-    do {                                                                \
-        ADD_ENV_SPACE;                                                  \
-        if ((env[envc++] = strdup(thisarg)) == NULL)                    \
-            goto no_memory;                                             \
-    } while (0)
-
-# define ADD_ENV_PAIR(envname, val)                                      \
-    do {                                                                \
-        char *envval;                                                   \
-        ADD_ENV_SPACE;                                                  \
-        if (virAsprintf(&envval, "%s=%s", envname, val) < 0)            \
-            goto no_memory;                                             \
-        env[envc++] = envval;                                           \
-    } while (0)
-
-# define ADD_ENV_COPY(envname)                                           \
-    do {                                                                \
-        char *val = getenv(envname);                                    \
-        if (val != NULL) {                                              \
-            ADD_ENV_PAIR(envname, val);                                 \
-        }                                                               \
-    } while (0)
-
-    ADD_ENV_LIT("LC_ALL=C");
-
-    ADD_ENV_COPY("LD_PRELOAD");
-    ADD_ENV_COPY("LD_LIBRARY_PATH");
-    ADD_ENV_COPY("PATH");
-    ADD_ENV_COPY("HOME");
-    ADD_ENV_COPY("USER");
-    ADD_ENV_COPY("LOGNAME");
-    ADD_ENV_COPY("TMPDIR");
-    ADD_ENV(NULL);
-
-    ADD_ARG_LIT(path);
-    ADD_ARG_LIT(id);
-    ADD_ARG_LIT(opstr);
-    ADD_ARG_LIT(subopstr);
-
-    ADD_ARG_LIT(extra);
-    ADD_ARG(NULL);
-
-    /* pass any optional input on the script stdin */
-    if (input != NULL) {
-        if (pipe(pipefd) < -1) {
-            virReportSystemError(errno, "%s",
-                             _("unable to create pipe for hook input"));
-            ret = 1;
-            goto cleanup;
-        }
-        if (safewrite(pipefd[1], input, strlen(input)) < 0) {
-            virReportSystemError(errno, "%s",
-                             _("unable to write to pipe for hook input"));
-            ret = 1;
-            goto cleanup;
-        }
-        ret = virExec(argv, env, NULL, &pid, pipefd[0], &outfd, &errfd,
-                      VIR_EXEC_NONE | VIR_EXEC_NONBLOCK);
-        if (close(pipefd[1]) < 0) {
-            virReportSystemError(errno, "%s",
-                             _("unable to close pipe for hook input"));
-        }
-        pipefd[1] = -1;
-    } else {
-        ret = virExec(argv, env, NULL, &pid, -1, &outfd, &errfd,
-                      VIR_EXEC_NONE | VIR_EXEC_NONBLOCK);
-    }
-    if (ret < 0) {
-        virHookReportError(VIR_ERR_HOOK_SCRIPT_FAILED,
-                           _("Failed to execute %s hook script"),
-                           path);
-        ret = 1;
-        goto cleanup;
-    }
+    cmd = virCommandNew(path);
 
-    /*
-     * we are interested in the error log if any and make sure the
-     * script doesn't block on stdout/stderr descriptors being full
-     * stdout can be useful for debug too.
-     */
-    if (virPipeReadUntilEOF(outfd, errfd, &outbuf, &errbuf) < 0) {
-        virReportSystemError(errno, _("cannot wait for '%s'"), path);
-        while (waitpid(pid, &exitstatus, 0) == -1 && errno == EINTR)
-            ;
-        ret = 1;
-        goto cleanup;
-    }
+    virCommandAddEnvPassCommon(cmd);
 
-    if (outbuf)
-        VIR_DEBUG("Command stdout: %s", outbuf);
-    if (errbuf)
-        VIR_DEBUG("Command stderr: %s", errbuf);
-
-    while ((waitret = waitpid(pid, &exitstatus, 0) == -1) &&
-           (errno == EINTR));
-    if (waitret == -1) {
-        virReportSystemError(errno, _("Failed to wait for '%s'"), path);
-        ret = 1;
-        goto cleanup;
-    }
-    if (exitstatus != 0) {
-        virHookReportError(VIR_ERR_HOOK_SCRIPT_FAILED,
-                           _("Hook script %s %s failed with error code %d:%s"),
-                           path, drvstr, exitstatus, errbuf);
-        ret = -1;
-    }
+    virCommandAddArg(cmd, id);
+    virCommandAddArg(cmd, opstr);
+    virCommandAddArg(cmd, subopstr);
+    virCommandAddArg(cmd, extra);
 
-cleanup:
-    if (pipefd[0] >= 0) {
-        if (close(pipefd[0]) < 0) {
-            virReportSystemError(errno, "%s",
-                             _("unable to close pipe for hook input"));
-        }
-    }
-    if (pipefd[1] >= 0) {
-        if (close(pipefd[1]) < 0) {
-            virReportSystemError(errno, "%s",
-                             _("unable to close pipe for hook input"));
-        }
-    }
-    if (argv) {
-        for (i = 0 ; i < argc ; i++)
-            VIR_FREE((argv)[i]);
-        VIR_FREE(argv);
-    }
-    if (env) {
-        for (i = 0 ; i < envc ; i++)
-            VIR_FREE((env)[i]);
-        VIR_FREE(env);
-    }
-    VIR_FREE(outbuf);
-    VIR_FREE(errbuf);
-    VIR_FREE(path);
+    virCommandSetInputBuffer(cmd, input);
 
-    return(ret);
+    ret = virCommandRun(cmd, NULL);
 
-no_memory:
-    virReportOOMError();
+    virCommandFree(cmd);
 
-    goto cleanup;
+    VIR_FREE(path);
 
-# undef ADD_ARG
-# undef ADD_ARG_LIT
-# undef ADD_ARG_SPACE
-# undef ADD_USBDISK
-# undef ADD_ENV
-# undef ADD_ENV_COPY
-# undef ADD_ENV_LIT
-# undef ADD_ENV_SPACE
+    return ret;
 }
-#endif
diff --git a/src/util/iptables.c b/src/util/iptables.c
index 4f95a02..7196ebe 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -39,7 +39,7 @@
 
 #include "internal.h"
 #include "iptables.h"
-#include "util.h"
+#include "command.h"
 #include "memory.h"
 #include "virterror_internal.h"
 #include "logging.h"
@@ -96,69 +96,25 @@ static int ATTRIBUTE_SENTINEL
 iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...)
 {
     va_list args;
-    int retval = ENOMEM;
-    const char **argv;
+    int ret;
+    virCommandPtr cmd;
     const char *s;
-    int n;
 
-    n = 1 + /* /sbin/iptables  */
-        2 + /*   --table foo   */
-        2 + /*   --insert bar  */
-        1;  /*   arg           */
+    cmd = virCommandNew(IPTABLES_PATH);
+    virCommandAddArg(cmd, "--table");
+    virCommandAddArg(cmd, rules->table);
+    virCommandAddArg(cmd, action == ADD ? "--insert" : "--delete");
+    virCommandAddArg(cmd, rules->chain);
+    virCommandAddArg(cmd, arg);
 
     va_start(args, arg);
-    while (va_arg(args, const char *))
-        n++;
-
-    va_end(args);
-
-    if (VIR_ALLOC_N(argv, n + 1) < 0)
-        goto error;
-
-    n = 0;
-
-    if (!(argv[n++] = strdup(IPTABLES_PATH)))
-        goto error;
-
-    if (!(argv[n++] = strdup("--table")))
-        goto error;
-
-    if (!(argv[n++] = strdup(rules->table)))
-        goto error;
-
-    if (!(argv[n++] = strdup(action == ADD ? "--insert" : "--delete")))
-        goto error;
-
-    if (!(argv[n++] = strdup(rules->chain)))
-        goto error;
-
-    if (!(argv[n++] = strdup(arg)))
-        goto error;
-
-    va_start(args, arg);
-
     while ((s = va_arg(args, const char *)))
-        if (!(argv[n++] = strdup(s)))
-            goto error;
-
+        virCommandAddArg(cmd, s);
     va_end(args);
 
-    if (virRun(argv, NULL) < 0) {
-        retval = errno;
-        goto error;
-    }
-
-    retval = 0;
-
- error:
-    if (argv) {
-        n = 0;
-        while (argv[n])
-            VIR_FREE(argv[n++]);
-        VIR_FREE(argv);
-    }
-
-    return retval;
+    ret = virCommandRun(cmd, NULL);
+    virCommandFree(cmd);
+    return ret;
 }
 
 /**
-- 
1.6.6.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]