1:18 p.m.
* src/util/command.h (virCommandAddArgBuffer)
(virCommandAddEnvBuffer): New prototypes.
* src/util/command.c (virCommandAddArgBuffer)
(virCommandAddEnvBuffer): Implement them.
* src/libvirt_private.syms (command.h): Export them.
* src/qemu/qemu_conf.c (qemudBuildCommandLine): Use them, plugging
a memory leak on rbd_hosts in the process.
---
Transferring malloc'd contents from one struct to another can be
tricky; hopefully this is easy enough to understand and hard enough to
abuse to warrant the syntactic shorthand that it provides to
qemu_conf. It helps that this is a transfer from one robust wrapper
to another; since we do NOT want something more direct, like:
/* Add malloc'd str to cmd, transferring ownership of who should free it */
void virCommandTransferArg(virCommandPtr cmd, char *str);
because that just invites bugs from freeing in the wrong place.
src/libvirt_private.syms | 2 ++
src/qemu/qemu_conf.c | 46 +++++++++-------------------------------------
src/util/command.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/util/command.h | 17 +++++++++++++++++
4 files changed, 71 insertions(+), 37 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c5a114c..b24ca70 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -85,10 +85,12 @@ virCgroupSetSwapHardLimit;
# command.h
virCommandAddArg;
+virCommandAddArgBuffer;
virCommandAddArgFormat;
virCommandAddArgList;
virCommandAddArgPair;
virCommandAddArgSet;
+virCommandAddEnvBuffer;
virCommandAddEnvPair;
virCommandAddEnvPass;
virCommandAddEnvPassCommon;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 4f19037..f4b524e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -4466,7 +4466,6 @@ qemudBuildCommandLine(virConnectPtr conn,
}
if (def->os.nBootDevs) {
virBuffer boot_buf = VIR_BUFFER_INITIALIZER;
- char *bootstr;
virCommandAddArg(cmd, "-boot");
boot[def->os.nBootDevs] = '\0';
@@ -4481,14 +4480,7 @@ qemudBuildCommandLine(virConnectPtr conn,
virBufferVSprintf(&boot_buf, "%s", boot);
}
- if (virBufferError(&boot_buf)) {
- virReportOOMError();
- goto error;
- }
-
- bootstr = virBufferContentAndReset(&boot_buf);
- virCommandAddArg(cmd, bootstr);
- VIR_FREE(bootstr);
+ virCommandAddArgBuffer(cmd, &boot_buf);
}
if (def->os.kernel)
@@ -4622,7 +4614,7 @@ qemudBuildCommandLine(virConnectPtr conn,
disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD) {
for (j = 0 ; j < disk->nhosts ; j++) {
if (!has_rbd_hosts) {
- virBufferAddLit(&rbd_hosts, "-m ");
+ virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m ");
has_rbd_hosts = true;
} else {
virBufferAddLit(&rbd_hosts, ",");
@@ -4727,7 +4719,7 @@ qemudBuildCommandLine(virConnectPtr conn,
snprintf(file, PATH_MAX, "rbd:%s,", disk->src);
for (j = 0 ; j < disk->nhosts ; j++) {
if (!has_rbd_hosts) {
- virBufferAddLit(&rbd_hosts, "-m ");
+ virBufferAddLit(&rbd_hosts, "CEPH_ARGS=-m ");
has_rbd_hosts = true;
} else {
virBufferAddLit(&rbd_hosts, ",");
@@ -4761,15 +4753,8 @@ qemudBuildCommandLine(virConnectPtr conn,
}
}
- if (virBufferError(&rbd_hosts)) {
- virBufferFreeAndReset(&rbd_hosts);
- goto no_memory;
- }
- if (has_rbd_hosts) {
- char *optstr = virBufferContentAndReset(&rbd_hosts);
- virCommandAddEnvPair(cmd, "CEPH_ARGS", optstr);
- VIR_FREE(optstr);
- }
+ if (has_rbd_hosts)
+ virCommandAddEnvBuffer(cmd, &rbd_hosts);
if (qemuCmdFlags & QEMUD_CMD_FLAG_FSDEV) {
for (i = 0 ; i < def->nfss ; i++) {
@@ -5082,7 +5067,6 @@ qemudBuildCommandLine(virConnectPtr conn,
if ((def->ngraphics == 1) &&
def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
virBuffer opt = VIR_BUFFER_INITIALIZER;
- char *optstr;
if (qemuCmdFlags & QEMUD_CMD_FLAG_VNC_COLON) {
if (def->graphics[0]->data.vnc.listenAddr)
@@ -5121,15 +5105,9 @@ qemudBuildCommandLine(virConnectPtr conn,
virBufferVSprintf(&opt, "%d",
def->graphics[0]->data.vnc.port - 5900);
}
- if (virBufferError(&opt)) {
- virBufferFreeAndReset(&opt);
- goto no_memory;
- }
-
- optstr = virBufferContentAndReset(&opt);
- virCommandAddArgList(cmd, "-vnc", optstr, NULL);
- VIR_FREE(optstr);
+ virCommandAddArg(cmd, "-vnc");
+ virCommandAddArgBuffer(cmd, &opt);
if (def->graphics[0]->data.vnc.keymap) {
virCommandAddArgList(cmd, "-k",
def->graphics[0]->data.vnc.keymap,
NULL);
@@ -5171,7 +5149,6 @@ qemudBuildCommandLine(virConnectPtr conn,
} else if ((def->ngraphics == 1) &&
def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
virBuffer opt = VIR_BUFFER_INITIALIZER;
- char *optstr;
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_SPICE)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -5214,13 +5191,8 @@ qemudBuildCommandLine(virConnectPtr conn,
}
}
- if (virBufferError(&opt))
- goto no_memory;
-
- optstr = virBufferContentAndReset(&opt);
-
- virCommandAddArgList(cmd, "-spice", optstr, NULL);
- VIR_FREE(optstr);
+ virCommandAddArg(cmd, "-spice");
+ virCommandAddArgBuffer(cmd, &opt);
if (def->graphics[0]->data.spice.keymap)
virCommandAddArgList(cmd, "-k",
def->graphics[0]->data.spice.keymap, NULL);
diff --git a/src/util/command.c b/src/util/command.c
index 5e2b19a..90c0d3a 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -315,6 +315,28 @@ virCommandAddEnvString(virCommandPtr cmd, const char *str)
/*
+ * Convert a buffer containing preformatted name=value into an
+ * environment variable of the child
+ */
+void
+virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf)
+{
+ if (!cmd || cmd->has_error)
+ return;
+
+ /* env plus trailing NULL. */
+ if (virBufferError(buf) ||
+ VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 1 + 1) < 0) {
+ cmd->has_error = ENOMEM;
+ virBufferFreeAndReset(buf);
+ return;
+ }
+
+ cmd->env[cmd->nenv++] = virBufferContentAndReset(buf);
+}
+
+
+/*
* Pass an environment variable to the child
* using current process' value
*/
@@ -381,6 +403,27 @@ virCommandAddArg(virCommandPtr cmd, const char *val)
/*
+ * Convert a buffer into a command line argument to the child
+ */
+void
+virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr buf)
+{
+ if (!cmd || cmd->has_error)
+ return;
+
+ /* Arg plus trailing NULL. */
+ if (virBufferError(buf) ||
+ VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
+ cmd->has_error = ENOMEM;
+ virBufferFreeAndReset(buf);
+ return;
+ }
+
+ cmd->args[cmd->nargs++] = virBufferContentAndReset(buf);
+}
+
+
+/*
* Add a command line argument created by a printf-style format
*/
void
diff --git a/src/util/command.h b/src/util/command.h
index 9b04e68..59d0ee3 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -24,6 +24,7 @@
# include "internal.h"
# include "util.h"
+# include "buf.h"
typedef struct _virCommand virCommand;
typedef virCommand *virCommandPtr;
@@ -110,6 +111,15 @@ void virCommandAddEnvPair(virCommandPtr cmd,
*/
void virCommandAddEnvString(virCommandPtr cmd,
const char *str) ATTRIBUTE_NONNULL(2);
+
+/*
+ * Convert a buffer containing preformatted name=value into an
+ * environment variable of the child.
+ * Correctly transfers memory errors or contents from buf to cmd.
+ */
+void virCommandAddEnvBuffer(virCommandPtr cmd,
+ virBufferPtr buf);
+
/*
* Pass an environment variable to the child
* using current process' value
@@ -129,6 +139,13 @@ void virCommandAddArg(virCommandPtr cmd,
const char *val) ATTRIBUTE_NONNULL(2);
/*
+ * Convert a buffer into a command line argument to the child.
+ * Correctly transfers memory errors or contents from buf to cmd.
+ */
+void virCommandAddArgBuffer(virCommandPtr cmd,
+ virBufferPtr buf);
+
+/*
* Add a command line argument created by a printf-style format
*/
void virCommandAddArgFormat(virCommandPtr cmd,
--
1.7.3.2