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

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



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


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