storage_backend: Convert virRunWithHook usage to virCommand

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

v2:
    Use opaque data to skip hook second time around
    Simply command building

v3:
    Drop explicit FindFileInPath

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index acd5af3d15b5fceb48af92f0661f8e6dfd8be723..7b6151c6abcb7e335f5ae15e87839f572fb3b343 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -967,7 +967,6 @@ virPipeReadUntilEOF;
 virRandom;
 virRandomInitialize;
 virRun;
-virRunWithHook;
 virSetBlocking;
 virSetCloseExec;
 virSetInherit;

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 61d14b17a61d4691314be3775a80b26d79b8b0d2..f90425a22194507d1b7072db7b4630b4678ca644 100644 (file)
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -523,8 +523,17 @@ cleanup:
     return ret;
 }
 
+struct hookdata {
+    virStorageVolDefPtr vol;
+    bool skip;
+};
+
 static int virStorageBuildSetUIDHook(void *data) {
-    virStorageVolDefPtr vol = data;
+    struct hookdata *tmp = data;
+    virStorageVolDefPtr vol = tmp->vol;
+
+    if (tmp->skip)
+        return 0;
 
     if ((vol->target.perms.gid != -1)
         && (setgid(vol->target.perms.gid) != 0)) {
@@ -545,11 +554,12 @@ 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;
     int filecreated = 0;
+    struct hookdata data = {vol, false};
 
     if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
         && (((getuid() == 0)
@@ -557,21 +567,25 @@ 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, &data);
+
+        if (virCommandRun(cmd, NULL) == 0) {
             /* command was successfully run, check if the file was created */
             if (stat(vol->target.path, &st) >=0)
                 filecreated = 1;
         }
     }
+
+    data.skip = true;
+
     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;
         }
     }
@@ -644,6 +658,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 ?
@@ -716,7 +732,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
         }
     }
 
-    if (vol->target.encryption != NULL) {
+    if (do_encryption) {
         virStorageEncryptionPtr enc;
 
         if (vol->target.format != VIR_STORAGE_FILE_QCOW &&
@@ -767,114 +783,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;
 }
@@ -892,7 +860,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
 {
     int ret;
     char *size;
-    const char *imgargv[4];
+    virCommandPtr cmd;
 
     if (inputvol) {
         virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -926,13 +894,10 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
         return -1;
     }
 
-    imgargv[0] = virFindFileInPath("qcow-create");
-    imgargv[1] = size;
-    imgargv[2] = vol->target.path;
-    imgargv[3] = NULL;
+    cmd = virCommandNewArgList("qcow-create", size, vol->target.path, NULL);
 
-    ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
-    VIR_FREE(imgargv[0]);
+    ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
+    virCommandFree(cmd);
     VIR_FREE(size);
 
     return ret;

diff --git a/src/util/util.c b/src/util/util.c
index f169e54831b1df8f6eb795e377ab61fb7faede51..89fc82b53cdf468bd3fbbb58cc31d5c9bb328b72 100644 (file)
--- 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;
@@ -807,7 +805,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;
     }
@@ -871,17 +869,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;
 }
 
@@ -1018,12 +1013,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 482294f303d5e8c819a141b73bd91d411339b236..e2b8eb361a4c905a05878453b99040594f668d6e 100644 (file)
--- 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);