[libvirt] [PATCH 04/16] storage_backend: Convert virRunWithHook usage to virCommand

Cole Robinson crobinso at redhat.com
Tue May 10 20:07:43 UTC 2011


virRunWithHook is now unused, so we can drop it. Tested w/ raw + qcow2
volume creation and copying.

Signed-off-by: Cole Robinson <crobinso at redhat.com>
---
 src/libvirt_private.syms      |    1 -
 src/storage/storage_backend.c |  151 +++++++++++++++--------------------------
 src/util/util.c               |   23 ++-----
 src/util/util.h               |    3 -
 4 files changed, 61 insertions(+), 117 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4c4f65d..a70fef6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -951,7 +951,6 @@ virPipeReadUntilEOF;
 virRandom;
 virRandomInitialize;
 virRun;
-virRunWithHook;
 virSetBlocking;
 virSetCloseExec;
 virSetInherit;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 97a7e4b..2acbf90 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -545,7 +545,7 @@ static int virStorageBuildSetUIDHook(void *data) {
 
 static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
                                               virStorageVolDefPtr vol,
-                                              const char **cmdargv) {
+                                              virCommandPtr cmd) {
     struct stat st;
     gid_t gid;
     uid_t uid;
@@ -557,21 +557,26 @@ static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
              && (vol->target.perms.uid != 0))
             || ((vol->target.perms.gid != -1)
                 && (vol->target.perms.gid != getgid())))) {
-        if (virRunWithHook(cmdargv,
-                           virStorageBuildSetUIDHook, vol, NULL) == 0) {
+
+        virCommandSetPreExecHook(cmd, virStorageBuildSetUIDHook, vol);
+
+        if (virCommandRun(cmd, NULL) == 0) {
             /* command was successfully run, check if the file was created */
             if (stat(vol->target.path, &st) >=0)
                 filecreated = 1;
         }
+
+        /* Unset the exec hook */
+        virCommandSetPreExecHook(cmd, NULL, NULL);
     }
+
     if (!filecreated) {
-        if (virRun(cmdargv, NULL) < 0) {
+        if (virCommandRun(cmd, NULL) < 0) {
             return -1;
         }
         if (stat(vol->target.path, &st) < 0) {
             virReportSystemError(errno,
-                                 _("%s failed to create %s"),
-                                 cmdargv[0], vol->target.path);
+                                 _("failed to create %s"), vol->target.path);
             return -1;
         }
     }
@@ -663,6 +668,8 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
     char *size = NULL;
     char *create_tool;
     int imgformat = -1;
+    virCommandPtr cmd = NULL;
+    bool do_encryption = (vol->target.encryption != NULL);
 
     const char *type = virStorageFileFormatTypeToString(vol->target.format);
     const char *backingType = vol->backingStore.path ?
@@ -735,7 +742,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
         }
     }
 
-    if (vol->target.encryption != NULL) {
+    if (do_encryption) {
         virStorageEncryptionPtr enc;
 
         if (vol->target.format != VIR_STORAGE_FILE_QCOW &&
@@ -786,114 +793,66 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
     if (imgformat < 0)
         goto cleanup;
 
+    cmd = virCommandNew(create_tool);
+
     if (inputvol) {
-        const char *imgargv[] = {
-            create_tool,
-            "convert",
-            "-f", inputType,
-            "-O", type,
-            inputPath,
-            vol->target.path,
-            NULL,
-            NULL,
-            NULL
-        };
-
-        if (vol->target.encryption != NULL) {
+        virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type,
+                             inputPath, vol->target.path, NULL);
+
+        if (do_encryption) {
             if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) {
-                imgargv[8] = "-o";
-                imgargv[9] = "encryption=on";
+                virCommandAddArgList(cmd, "-o", "encryption=on", NULL);
             } else {
-                imgargv[8] = "-e";
+                virCommandAddArg(cmd, "-e");
             }
         }
 
-        ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
     } else if (vol->backingStore.path) {
-        const char *imgargv[] = {
-            create_tool,
-            "create",
-            "-f", type,
-            "-b", vol->backingStore.path,
-            NULL,
-            NULL,
-            NULL,
-            NULL,
-            NULL,
-            NULL
-        };
-
-        char *optflag = NULL;
+        virCommandAddArgList(cmd, "create", "-f", type,
+                             "-b", vol->backingStore.path, NULL);
+
         switch (imgformat) {
         case QEMU_IMG_BACKING_FORMAT_FLAG:
-            imgargv[6] = "-F";
-            imgargv[7] = backingType;
-            imgargv[8] = vol->target.path;
-            imgargv[9] = size;
-            if (vol->target.encryption != NULL)
-                imgargv[10] = "-e";
+            virCommandAddArgList(cmd, "-F", backingType, vol->target.path,
+                                 size, NULL);
+
+            if (do_encryption)
+                virCommandAddArg(cmd, "-e");
             break;
 
         case QEMU_IMG_BACKING_FORMAT_OPTIONS:
-            if (virAsprintf(&optflag, "backing_fmt=%s", backingType) < 0) {
-                virReportOOMError();
-                goto cleanup;
-            }
-
-            if (vol->target.encryption != NULL) {
-                char *tmp = NULL;
-                if (virAsprintf(&tmp, "%s,%s", optflag, "encryption=on") < 0) {
-                    virReportOOMError();
-                    goto cleanup;
-                }
-                VIR_FREE(optflag);
-                optflag = tmp;
-            }
-
-            imgargv[6] = "-o";
-            imgargv[7] = optflag;
-            imgargv[8] = vol->target.path;
-            imgargv[9] = size;
+            virCommandAddArg(cmd, "-o");
+            virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType,
+                                   do_encryption ? ",encryption=on" : "");
+            virCommandAddArgList(cmd, vol->target.path, size, NULL);
+            break;
 
         default:
             VIR_INFO("Unable to set backing store format for %s with %s",
                      vol->target.path, create_tool);
-            imgargv[6] = vol->target.path;
-            imgargv[7] = size;
-            if (vol->target.encryption != NULL)
-                imgargv[8] = "-e";
-        }
 
-        ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
-        VIR_FREE(optflag);
+            virCommandAddArgList(cmd, vol->target.path, size, NULL);
+            if (do_encryption)
+                virCommandAddArg(cmd, "-e");
+        }
     } else {
-        /* The extra NULL field is for indicating encryption (-e). */
-        const char *imgargv[] = {
-            create_tool,
-            "create",
-            "-f", type,
-            vol->target.path,
-            size,
-            NULL,
-            NULL,
-            NULL
-        };
-
-        if (vol->target.encryption != NULL) {
+        virCommandAddArgList(cmd, "create", "-f", type,
+                             vol->target.path, size, NULL);
+
+        if (do_encryption) {
             if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) {
-                imgargv[6] = "-o";
-                imgargv[7] = "encryption=on";
+                virCommandAddArgList(cmd, "-o", "encryption=on", NULL);
             } else {
-                imgargv[6] = "-e";
+                virCommandAddArg(cmd, "-e");
             }
         }
-
-        ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
     }
 
-    cleanup:
+    ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
+cleanup:
     VIR_FREE(size);
     VIR_FREE(create_tool);
+    virCommandFree(cmd);
 
     return ret;
 }
@@ -911,7 +870,8 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
 {
     int ret;
     char *size;
-    const char *imgargv[4];
+    char *create_tool;
+    virCommandPtr cmd;
 
     if (inputvol) {
         virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -945,13 +905,12 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
         return -1;
     }
 
-    imgargv[0] = virFindFileInPath("qcow-create");
-    imgargv[1] = size;
-    imgargv[2] = vol->target.path;
-    imgargv[3] = NULL;
+    create_tool = virFindFileInPath("qcow-create");
+    cmd = virCommandNewArgList(create_tool, size, vol->target.path, NULL);
 
-    ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
-    VIR_FREE(imgargv[0]);
+    ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
+    virCommandFree(cmd);
+    VIR_FREE(create_tool);
     VIR_FREE(size);
 
     return ret;
diff --git a/src/util/util.c b/src/util/util.c
index f860392..824e64e 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -787,10 +787,8 @@ virExec(const char *const*argv,
  * only if the command could not be run.
  */
 int
-virRunWithHook(const char *const*argv,
-               virExecHook hook,
-               void *data,
-               int *status) {
+virRun(const char *const*argv, int *status)
+{
     pid_t childpid;
     int exitstatus, execret, waitret;
     int ret = -1;
@@ -800,7 +798,7 @@ virRunWithHook(const char *const*argv,
 
     if ((execret = virExecWithHook(argv, NULL, NULL,
                              &childpid, -1, &outfd, &errfd,
-                             VIR_EXEC_NONE, hook, data, NULL)) < 0) {
+                             VIR_EXEC_NONE, NULL, NULL, NULL)) < 0) {
         ret = execret;
         goto error;
     }
@@ -864,17 +862,14 @@ int virSetInherit(int fd ATTRIBUTE_UNUSED, bool inherit ATTRIBUTE_UNUSED)
     return -1;
 }
 
-int
-virRunWithHook(const char *const *argv ATTRIBUTE_UNUSED,
-               virExecHook hook ATTRIBUTE_UNUSED,
-               void *data ATTRIBUTE_UNUSED,
-               int *status)
+virRun(const char *const *argv ATTRIBUTE_UNUSED,
+       int *status)
 {
     if (status)
         *status = ENOTSUP;
     else
         virUtilError(VIR_ERR_INTERNAL_ERROR,
-                     "%s", _("virRunWithHook is not implemented for WIN32"));
+                     "%s", _("virRun is not implemented for WIN32"));
     return -1;
 }
 
@@ -1011,12 +1006,6 @@ error:
     return -1;
 }
 
-int
-virRun(const char *const*argv,
-       int *status) {
-    return virRunWithHook(argv, NULL, NULL, status);
-}
-
 /* Like gnulib's fread_file, but read no more than the specified maximum
    number of bytes.  If the length of the input is <= max_len, and
    upon error while reading that data, it works just like fread_file.  */
diff --git a/src/util/util.h b/src/util/util.h
index 482294f..e2b8eb3 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -78,9 +78,6 @@ int virExec(const char *const*argv,
             int *errfd,
             int flags) ATTRIBUTE_RETURN_CHECK;
 int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK;
-int virRunWithHook(const char *const*argv,
-                   virExecHook hook, void *data,
-                   int *status) ATTRIBUTE_RETURN_CHECK;
 int virPipeReadUntilEOF(int outfd, int errfd,
                         char **outbuf, char **errbuf);
 int virFork(pid_t *pid);
-- 
1.7.4.4




More information about the libvir-list mailing list