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

[Laine Stump]

On 07/21/2011 07:40 PM, Eric Blake wrote:
> 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?

Not yet, but before the end of Friday.

>
>>           } 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.
>

Okay, I fixed those up, but am waiting to push until the new 
prerequisite patch is pushed (changing the returns of functions in 
util/interface.c to be < 0 on error).