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
Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
---
src/libvirt_private.syms | 1 -
src/storage/storage_backend.c | 162 ++++++++++++++++------------------------
src/util/util.c | 23 ++----
src/util/util.h | 3 -
4 files changed, 71 insertions(+), 118 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 21a65ad..8cf9c1a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -957,7 +957,6 @@ virPipeReadUntilEOF;
virRandom;
virRandomInitialize;
virRun;
-virRunWithHook;
virSetBlocking;
virSetCloseExec;
virSetInherit;
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 61d14b1..76dca7b 100644
--- 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,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",
@@ -926,13 +895,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 f169e54..89fc82b 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;
@@ -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 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list