[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/7] qemu: Split out code to generate VNC command line



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

Attachment: pgpBALEEDgwGj.pgp
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]