Change in vdsm[master]: vm: reorganize the graphic devices lookup

Francesco Romani has uploaded a new change for review.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

vm: reorganize the graphic devices lookup

this patch extracts a method to gather and
return all the graphic devices of a Vm, either
new style (proper device) or legacy (reconstructed
from the display* params).

The correct order of the devices is preserved:
* first the one reconstructed from the legacy params,
  if any
* then the additional one

and proper locking (using _confLock) is used to avoid
races.

Change-Id: I231087647e515a1f310de45b659e512444bd0a1d
Signed-off-by: Francesco Romani 
---
M vdsm/virt/vm.py
1 file changed, 14 insertions(+), 9 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/96/27596/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 198cbce..445a750 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -2651,10 +2651,16 @@
         # updated in Vm._run (buildConfDevices and the following code).
         # easily reproduced with functional tests, but also possible
         # in the wild
-        for dev in self.conf.get('devices', []) + self.getConfGraphics():
+        for dev in self._getAllConfGraphicDevices():
+            stats.update(getInfo(dev))
+            break
+
+    def _getAllConfGraphicDevices(self):
+        with self._confLock:
+            devices = self.getConfGraphics() + self.conf.get('devices', [])
+        for dev in devices:
             if dev['type'] == GRAPHICS_DEVICES:
-                stats.update(getInfo(dev))
-                break
+                yield dev

def isMigrating(self):
         return self._migrationSourceThread.isAlive()
@@ -3016,8 +3022,9 @@
         for dev in devices.values():
             newDevices.extend(dev)

-        self.conf['devices'] = newDevices
-        self._cleanLegacyGraphics()
+        with self._confLock:
+            self.conf['devices'] = newDevices
+            self._cleanLegacyGraphics()

def _cleanLegacyGraphics(self):
         """
@@ -4389,10 +4396,8 @@

@property
     def hasSpice(self):
-        return any(
-            dev['device'] == 'spice'
-            for dev in self.conf.get('devices', self.getConfGraphics())
-            if dev['type'] == GRAPHICS_DEVICES)
+        return any(dev['device'] == 'spice'
+                   for dev in self._getAllConfGraphicDevices())

def _getPid(self):
         pid = '0'

Francesco Romani has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 1: Verified+1

Vetrified with http://gerrit.ovirt.org/#/c/26895/ , http://gerrit.ovirt.org/#/c/27595/ and http://gerrit.ovirt.org/#/c/27215/

Verification (all the following using unpatched engine 3.5.0 snapshot:
* creation flow: booted a VM, connected with spice
* recovery: booted a VM, restarted VDSM, connected with spice
* migration: migrated back and forth another host running VDSM 4.14.8.1
* hibernate:  booted a VM, restarted and connected with spice.
* verified the output of vdsClient
* cursory look at vdsm logs and to engine logs seems OK.

Found
Received a spice Device without an address when processing VM 025d59e3-f12c-41b8-be3f-de8dc22d23ce devices, skipping device: {port=5900, specParams={keyMap=en-us, copyPasteEnable=false, displayIp=0, fileTransferEnable=false}, device=spice, type=graphics}

but that is expected since the graphics device do not have PCI addresses.

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 1: Code-Review+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 1:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8766/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8902/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7976/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8823/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8959/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8033/ : SUCCESS

Francesco Romani has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 2:

rebased and simplified.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 3:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8864/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9000/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8074/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8914/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9051/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8124/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 5:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8976/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9117/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8188/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 6:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8996/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9137/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8208/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 7:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9003/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9144/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8215/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 8:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9038/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9179/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8250/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 9:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9066/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9207/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8278/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 10:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9112/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9255/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8324/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 11:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9137/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9280/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8349/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 12:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9147/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9290/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8359/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 13:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9188/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9331/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8400/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 14:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9271/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9414/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8483/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 15:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9576/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8644/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9430/ : SUCCESS

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 15: Code-Review+1

Nice

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 16:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9245/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10029/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/853/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10184/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5111/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3268/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 17:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10278/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11063/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1302/ : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11220/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 18:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10288/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11073/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1310/ : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11230/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 19:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10297/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11082/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1319/ : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11240/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Patch Set 20:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1432/ : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/10787/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11729/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11572/ : SUCCESS

Francesco Romani has abandoned this change.

Change subject: vm: reorganize the graphic devices lookup
......................................................................

Abandoned

seems no longer needed. Will be resumed if the need arises again.