On Tue, Apr 23, 2013 at 03:46:09PM +0200, Peter Krempa wrote:
> Decrease size of qemuBuildGraphicsCommandLine() by splitting out
> spice-related code into qemuBuildGraphicsVNCCommandLine().
I guess you mean "VNC-related code" here.
Christophe
>
> This patch also fixes 2 possible memory leaks on error path in the code
> that was split-out. The buffer containing the already generated options
> and a listen address string could be leaked.
>
> Also break a few very long lines and reflow code that fits now.
> ---
> src/qemu/qemu_command.c | 244 +++++++++++++++++++++++++-----------------------
> 1 file changed, 125 insertions(+), 119 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d6d782c..66b2ba8 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5419,6 +5419,130 @@ cleanup:
>
>
> static int
> +qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
> + virCommandPtr cmd,
> + virDomainDefPtr def,
> + virQEMUCapsPtr qemuCaps,
> + virDomainGraphicsDefPtr graphics)
> +{
> + virBuffer opt = VIR_BUFFER_INITIALIZER;
> + const char *listenNetwork;
> + const char *listenAddr = NULL;
> + char *netAddr = NULL;
> + bool escapeAddr;
> + int ret;
> +
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("vnc graphics are not supported with this QEMU"));
> + goto error;
> + }
> +
> + if (graphics->data.vnc.socket || cfg->vncAutoUnixSocket) {
> + if (!graphics->data.vnc.socket &&
> + virAsprintf(&graphics->data.vnc.socket,
> + "%s/%s.vnc", cfg->libDir, def->name) == -1)
> + goto no_memory;
> +
> + virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket);
> +
> + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
> + switch (virDomainGraphicsListenGetType(graphics, 0)) {
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
> + break;
> +
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
> + if (!listenNetwork)
> + break;
> + ret = networkGetNetworkAddress(listenNetwork, &netAddr);
> + if (ret <= -2) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + "%s", _("network-based listen not possible, "
> + "network driver not present"));
> + goto error;
> + }
> + if (ret < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("listen network '%s' had no usable address"),
> + listenNetwork);
> + goto error;
> + }
> + listenAddr = netAddr;
> + /* store the address we found in the <graphics> element so it will
> + * show up in status. */
> + if (virDomainGraphicsListenSetAddress(graphics, 0,
> + listenAddr, -1, false) < 0)
> + goto error;
> + break;
> + }
> +
> + if (!listenAddr)
> + listenAddr = cfg->vncListen;
> +
> + escapeAddr = strchr(listenAddr, ':') != NULL;
> + if (escapeAddr)
> + virBufferAsprintf(&opt, "[%s]", listenAddr);
> + else
> + virBufferAdd(&opt, listenAddr, -1);
> + virBufferAsprintf(&opt, ":%d",
> + graphics->data.vnc.port - 5900);
> +
> + VIR_FREE(netAddr);
> + } else {
> + virBufferAsprintf(&opt, "%d",
> + graphics->data.vnc.port - 5900);
> + }
> +
> + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
> + if (graphics->data.vnc.auth.passwd || cfg->vncPassword)
> + virBufferAddLit(&opt, ",password");
> +
> + if (cfg->vncTLS) {
> + virBufferAddLit(&opt, ",tls");
> + if (cfg->vncTLSx509verify)
> + virBufferAsprintf(&opt, ",x509verify=%s", cfg->vncTLSx509certdir);
> + else
> + virBufferAsprintf(&opt, ",x509=%s", cfg->vncTLSx509certdir);
> + }
> +
> + if (cfg->vncSASL) {
> + virBufferAddLit(&opt, ",sasl");
> +
> + if (cfg->vncSASLdir)
> + virCommandAddEnvPair(cmd, "SASL_CONF_DIR", cfg->vncSASLdir);
> +
> + /* TODO: Support ACLs later */
> + }
> + }
> +
> + virCommandAddArg(cmd, "-vnc");
> + virCommandAddArgBuffer(cmd, &opt);
> + if (graphics->data.vnc.keymap)
> + virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, NULL);
> +
> + /* Unless user requested it, set the audio backend to none, to
> + * prevent it opening the host OS audio devices, since that causes
> + * security issues and might not work when using VNC.
> + */
> + if (cfg->vncAllowHostAudio)
> + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
> + else
> + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> +
> + return 0;
> +
> +no_memory:
> + virReportOOMError();
> +error:
> + VIR_FREE(netAddr);
> + virBufferFreeAndReset(&opt);
> + return -1;
> +}
> +
> +
> +static int
> qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> virCommandPtr cmd,
> virQEMUCapsPtr qemuCaps,
> @@ -5600,124 +5724,8 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
> virDomainGraphicsDefPtr graphics)
> {
> if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> - virBuffer opt = VIR_BUFFER_INITIALIZER;
> -
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("vnc graphics are not supported with this QEMU"));
> + if (qemuBuildGraphicsVNCCommandLine(cfg, cmd, def, qemuCaps, graphics) < 0)
> goto error;
> - }
> -
> - if (graphics->data.vnc.socket ||
> - cfg->vncAutoUnixSocket) {
> -
> - if (!graphics->data.vnc.socket &&
> - virAsprintf(&graphics->data.vnc.socket,
> - "%s/%s.vnc", cfg->libDir, def->name) == -1) {
> - goto no_memory;
> - }
> -
> - virBufferAsprintf(&opt, "unix:%s",
> - graphics->data.vnc.socket);
> -
> - } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
> - const char *listenNetwork;
> - const char *listenAddr = NULL;
> - char *netAddr = NULL;
> - bool escapeAddr;
> - int ret;
> -
> - switch (virDomainGraphicsListenGetType(graphics, 0)) {
> - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> - listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
> - break;
> -
> - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> - listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
> - if (!listenNetwork)
> - break;
> - ret = networkGetNetworkAddress(listenNetwork, &netAddr);
> - if (ret <= -2) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - "%s", _("network-based listen not possible, "
> - "network driver not present"));
> - goto error;
> - }
> - if (ret < 0) {
> - virReportError(VIR_ERR_XML_ERROR,
> - _("listen network '%s' had no usable address"),
> - listenNetwork);
> - goto error;
> - }
> - listenAddr = netAddr;
> - /* store the address we found in the <graphics> element so it will
> - * show up in status. */
> - if (virDomainGraphicsListenSetAddress(graphics, 0,
> - listenAddr, -1, false) < 0)
> - goto error;
> - break;
> - }
> -
> - if (!listenAddr)
> - listenAddr = cfg->vncListen;
> -
> - escapeAddr = strchr(listenAddr, ':') != NULL;
> - if (escapeAddr)
> - virBufferAsprintf(&opt, "[%s]", listenAddr);
> - else
> - virBufferAdd(&opt, listenAddr, -1);
> - virBufferAsprintf(&opt, ":%d",
> - graphics->data.vnc.port - 5900);
> -
> - VIR_FREE(netAddr);
> - } else {
> - virBufferAsprintf(&opt, "%d",
> - graphics->data.vnc.port - 5900);
> - }
> -
> - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
> - if (graphics->data.vnc.auth.passwd ||
> - cfg->vncPassword)
> - virBufferAddLit(&opt, ",password");
> -
> - if (cfg->vncTLS) {
> - virBufferAddLit(&opt, ",tls");
> - if (cfg->vncTLSx509verify) {
> - virBufferAsprintf(&opt, ",x509verify=%s",
> - cfg->vncTLSx509certdir);
> - } else {
> - virBufferAsprintf(&opt, ",x509=%s",
> - cfg->vncTLSx509certdir);
> - }
> - }
> -
> - if (cfg->vncSASL) {
> - virBufferAddLit(&opt, ",sasl");
> -
> - if (cfg->vncSASLdir)
> - virCommandAddEnvPair(cmd, "SASL_CONF_DIR",
> - cfg->vncSASLdir);
> -
> - /* TODO: Support ACLs later */
> - }
> - }
> -
> - virCommandAddArg(cmd, "-vnc");
> - virCommandAddArgBuffer(cmd, &opt);
> - if (graphics->data.vnc.keymap) {
> - virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap,
> - NULL);
> - }
> -
> - /* Unless user requested it, set the audio backend to none, to
> - * prevent it opening the host OS audio devices, since that causes
> - * security issues and might not work when using VNC.
> - */
> - if (cfg->vncAllowHostAudio) {
> - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
> - } else {
> - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
> - }
> } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_0_10) &&
> !virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL)) {
> @@ -5761,8 +5769,6 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
>
> return 0;
>
> -no_memory:
> - virReportOOMError();
> error:
> return -1;
> }
> --
> 1.8.2.1
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
pgpyoW25I6xJw.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list