From e8d05c978da774b7abbbc38dfcc00b9b582fdf72 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 10 Dec 2010 11:33:53 -0700 Subject: [PATCH] command: ease use with virBuffer, and fix qemu leak * 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. --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 42 ++++++++-------------------------- src/util/command.c | 49 ++++++++++++++++++++++++++++++++++++++++ src/util/command.h | 17 ++++++++++++++ 4 files changed, 77 insertions(+), 33 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c5a114ce9e..b24ca70b99 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 fb0b29a244..f4b524edd5 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,12 +4753,8 @@ qemudBuildCommandLine(virConnectPtr conn, } } - if (virBufferError(&rbd_hosts)) { - virBufferFreeAndReset(&rbd_hosts); - goto no_memory; - } if (has_rbd_hosts) - virCommandAddEnvPair(cmd, "CEPH_ARGS", virBufferContentAndReset(&rbd_hosts)); + virCommandAddEnvBuffer(cmd, &rbd_hosts); if (qemuCmdFlags & QEMUD_CMD_FLAG_FSDEV) { for (i = 0 ; i < def->nfss ; i++) { @@ -5079,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) @@ -5118,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); @@ -5168,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", @@ -5211,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); @@ -5516,6 +5491,7 @@ qemudBuildCommandLine(virConnectPtr conn, error: for (i = 0; i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); + virBufferFreeAndReset(&rbd_hosts); virCommandFree(cmd); return NULL; } diff --git a/src/util/command.c b/src/util/command.c index 5e2b19aff7..f9d475e16f 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -314,6 +314,31 @@ virCommandAddEnvString(virCommandPtr cmd, const char *str) } +/* + * 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) +{ + if (!cmd || cmd->has_error) { + virBufferFreeAndReset(buf); + 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 @@ -380,6 +405,30 @@ virCommandAddArg(virCommandPtr cmd, const char *val) } +/* + * 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) +{ + if (!cmd || cmd->has_error) { + virBufferFreeAndReset(buf); + 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 */ diff --git a/src/util/command.h b/src/util/command.h index 9b04e68119..59d0ee3ebd 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 @@ -128,6 +138,13 @@ void virCommandAddEnvPassCommon(virCommandPtr cmd); 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 */ -- 2.47.2