Change in vdsm[ovirt-3.6]: virt: graphics: avoid None in specParams

fromani at redhat.com fromani at redhat.com
Tue Sep 29 07:05:45 UTC 2015


Francesco Romani has uploaded a new change for review.

Change subject: virt: graphics: avoid None in specParams
......................................................................

virt: graphics: avoid None in specParams

In commit bbe8a03 we added a check to make sure
we use displayNetwork from global Vm configuration,
in the case the more specific setting was not sent
in the graphics device specParams.

One unfortunate side effect of this change is that
now None may end up in the graphics device specParams,
breaking XML-RPC communications.

We default (end recommend) JSON-RPC, but we still
support XML-RPC, and we use it as default in vdsClient,
so this must be avoided.

This patch reorganize the check to avoid the unwanted
injection of the None value.

Change-Id: I2a651c18c02c207f5497f22f0aad3d2967efd9a4
Backport-To: 3.6
Related-To: https://bugzilla.redhat.com/1261007
Signed-off-by: Francesco Romani <fromani at redhat.com>
---
M tests/deviceTests.py
M vdsm/virt/vmdevices/graphics.py
2 files changed, 42 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/72/46772/1

diff --git a/tests/deviceTests.py b/tests/deviceTests.py
index 3913a0f..a248277 100644
--- a/tests/deviceTests.py
+++ b/tests/deviceTests.py
@@ -518,6 +518,43 @@
         memory = vmdevices.core.Memory(self.conf, self.log, **params)
         self.assertXMLEqual(memory.getXML().toxml(), memoryXML)
 
+    def testGraphicsNoDisplayNetwork(self):
+        with fake.VM() as testvm:
+            graphDev = vmdevices.graphics.Graphics(
+                testvm.conf, testvm.log)
+
+            self.assertNotIn('displayNetwork', graphDev.specParams)
+
+    def testGraphicsDisplayNetworkFromSpecParams(self):
+        with fake.VM() as testvm:
+            graphDev = vmdevices.graphics.Graphics(
+                testvm.conf, testvm.log,
+                specParams={'displayNetwork': 'vmDisplaySpecParams'})
+
+            self.assertEqual(graphDev.specParams['displayNetwork'],
+                             'vmDisplaySpecParams')
+
+    def testGraphicsDisplayNetworkFromVmConf(self):
+        conf = {'displayNetwork': 'vmDisplayConf'}
+        conf.update(self.conf)
+        with fake.VM(conf) as testvm:
+            graphDev = vmdevices.graphics.Graphics(
+                testvm.conf, testvm.log)
+
+            self.assertEqual(graphDev.specParams['displayNetwork'],
+                             'vmDisplayConf')
+
+    def testGraphicsDisplayNetworkFromSpecParamsOverridesVmConf(self):
+        conf = {'displayNetwork': 'vmDisplayConf'}
+        conf.update(self.conf)
+        with fake.VM(conf) as testvm:
+            graphDev = vmdevices.graphics.Graphics(
+                testvm.conf, testvm.log,
+                specParams={'displayNetwork': 'vmDisplaySpecParams'})
+
+            self.assertEqual(graphDev.specParams['displayNetwork'],
+                             'vmDisplaySpecParams')
+
 
 class ConsoleTests(TestCaseBase):
 
diff --git a/vdsm/virt/vmdevices/graphics.py b/vdsm/virt/vmdevices/graphics.py
index c6ed06d..86301cd 100644
--- a/vdsm/virt/vmdevices/graphics.py
+++ b/vdsm/virt/vmdevices/graphics.py
@@ -59,9 +59,12 @@
 
         # It's possible that the network is specified vm's conf
         # and not in specParams. This is considered legacy.
-        self.specParams['displayNetwork'] = (
-            self.specParams.get('displayNetwork') or conf.get('displayNetwork')
+        displayNetwork = (
+            self.specParams.get('displayNetwork') or
+            conf.get('displayNetwork')
         )
+        if displayNetwork:
+            self.specParams['displayNetwork'] = displayNetwork
 
         self.specParams['displayIp'] = (
             _getNetworkIp(self.specParams.get('displayNetwork')))


-- 
To view, visit https://gerrit.ovirt.org/46772
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a651c18c02c207f5497f22f0aad3d2967efd9a4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani at redhat.com>


More information about the vdsm-patches mailing list