* 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list