[libvirt] [PATCHv2 4/4] qemu: support the listenNetwork attribute in <graphics>

[Eric Blake]

On 07/20/2011 02:11 AM, Laine Stump wrote:
> The domain XML now understands an attribute named "listenNetwork" in
> the<graphics>  element, and the network driver has an internal API
> that will turn a network name into an IP address, so the final logical
> step is to put the glue into the qemu driver such that when it is
> starting up a domain, if it finds "listenNetwork" in the XML, it will
> call the network driver to get an associated IP address, and tell qemu
> to listen for vnc (or spice) on that address rather than the default
> (localhost).

>
> Since this is the commit that turns on the new functionality, I've
> included the doc changes here.

Yay.  Did you ever finish the other doc changes you were promising?

>           } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) {
> -            const char *addr = def->graphics[0]->data.vnc.listenAddr ?
> -                def->graphics[0]->data.vnc.listenAddr :
> -                driver->vncListen;
> -            bool escapeAddr = strchr(addr, ':') != NULL;
> +            int ret;
> +            char *addr;

Uninitialized...

> +            bool freeAddr = false;
> +            bool escapeAddr;
> +
> +            if (def->graphics[0]->data.vnc.listenAddr) {
> +                addr = def->graphics[0]->data.vnc.listenAddr;
> +            } else if (def->graphics[0]->data.vnc.listenNetwork) {
> +                ret = networkGetNetworkAddress(def->graphics[0]->data.vnc.listenNetwork,
> +&addr);

and if networkGetNetworkAddress is stubbed out, addr remains 
uninitialized...

> +                if (ret<= -2) {
> +                    qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                    "%s", _("listenNetwork not supported, network driver not present"));
> +                }
> +                if (!addr) {

Oops - jump depending on uninit data - valgrind will call you on that! 
Not to mention that it looks odd to use ret <= -2, then !addr.  I would 
have expected ret <= -2, then ret < 0, for consistency.  Simplest fix 
would be to add the missing "goto error;" statement in the ret <= -2 clause.

> +        } else if (def->graphics[0]->data.spice.listenNetwork) {
> +            int ret;
> +            char *addr;
> +
> +            ret = networkGetNetworkAddress(def->graphics[0]->data.spice.listenNetwork,
> +&addr);
> +            if (ret<= -2) {
> +                qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                "%s", _("listenNetwork not supported, network driver not present"));
> +            }
> +            if (!addr) {

Same problem with addr.

ACK with those fixed.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org