Change in vdsm[master]: Revert "vdsm: refactor buildConfDevices and getConfDevices i...

nsoffer at redhat.com nsoffer at redhat.com
Sun Dec 1 14:55:50 UTC 2013


Nir Soffer has uploaded a new change for review.

Change subject: Revert "vdsm: refactor buildConfDevices and getConfDevices into single function"
......................................................................

Revert "vdsm: refactor buildConfDevices and getConfDevices into single function"

This reverts commit 93d09f6567e1dfef2397f83258998666d32c7475.

That commit removed Vm.getConfDevices without updating callers about it,
causing a failure in clientIF._recoverExistingVms. Also recovery process
seems to be broken after this commit, including vm unpausing, that
depends on successful recovery. When reverted, vm unpausing on startup
is working again.

Since this refactoring was not correct, and breaks a feature, it should
be done again more carefully.

Change-Id: I13d07d6de964ecbc7b518fc53adae0b26cef9658
Relates-To: http://gerrit.ovirt.org/21649
Bug-Url: https://bugzilla.redhat.com/1036358
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M vdsm/vm.py
1 file changed, 52 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/21900/1

diff --git a/vdsm/vm.py b/vdsm/vm.py
index b272ca2..295297d 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1884,30 +1884,47 @@
                 'truesize': 0})
         return removables
 
+    def getConfDevices(self):
+        devices = self._makeDeviceDict()
+        for dev in self.conf.get('devices'):
+            try:
+                devices[dev['type']].append(dev)
+            except KeyError:
+                # Unknown type device found
+                self.log.warn("Unknown type found, device: '%s' found", dev)
+                devices[GENERAL_DEVICES].append(dev)
+
+        # Update indices for drives devices
+        self.normalizeDrivesIndices(devices[DISK_DEVICES])
+
+        return devices
+
     def buildConfDevices(self):
         """
         Return the "devices" section of this Vm's conf.
         If missing, create it according to old API.
         """
-        devices = self._makeDeviceDict()
-
         # For BC we need to save previous behaviour for old type parameters.
         # The new/old type parameter will be distinguished
         # by existence/absence of the 'devices' key
+        devices = {}
+        # Build devices structure
         if self.conf.get('devices') is None:
+            with self._confLock:
+                self.conf['devices'] = []
             devices[DISK_DEVICES] = self.getConfDrives()
             devices[NIC_DEVICES] = self.getConfNetworkInterfaces()
             devices[SOUND_DEVICES] = self.getConfSound()
             devices[VIDEO_DEVICES] = self.getConfVideo()
             devices[CONTROLLER_DEVICES] = self.getConfController()
+            devices[GENERAL_DEVICES] = []
+            devices[BALLOON_DEVICES] = []
+            devices[WATCHDOG_DEVICES] = []
+            devices[SMARTCARD_DEVICES] = self.getConfSmartcard()
+            devices[REDIR_DEVICES] = []
+            devices[CONSOLE_DEVICES] = []
         else:
-            for dev in self.conf.get('devices'):
-                try:
-                    devices[dev['type']].append(dev)
-                except KeyError:
-                    self.log.warn("Unknown type found, device: '%s' "
-                                  "found", dev)
-                    devices[GENERAL_DEVICES].append(dev)
+            devices = self.getConfDevices()
 
         self._checkDeviceLimits(devices)
 
@@ -1915,8 +1932,6 @@
         for drv in devices[DISK_DEVICES]:
             if isVdsmImage(drv):
                 self._normalizeVdsmImg(drv)
-
-        self.normalizeDrivesIndices(devices[DISK_DEVICES])
 
         # Preserve old behavior. Since libvirt add a memory balloon device
         # to all guests, we need to specifically request not to add it.
@@ -1972,6 +1987,17 @@
                            'device': devType})
 
         return vcards
+
+    def getConfSmartcard(self):
+        """
+        Normalize smartcard device (now there is only one)
+        """
+        cards = []
+        if self.conf.get('smartcard'):
+            cards.append({'device': SMARTCARD_DEVICES,
+                          'specParams': {'mode': 'passthrough',
+                                         'type': 'spicevmc'}})
+        return cards
 
     def getConfSound(self):
         """
@@ -2036,6 +2062,9 @@
             # FIXME: For BC we have now two identical keys: iface = if
             # Till the day that conf will not returned as a status anymore.
             drv['iface'] = drv.get('iface') or drv.get('if', 'ide')
+
+        # Update indices for drives devices
+        self.normalizeDrivesIndices(confDrives)
 
         return confDrives
 
@@ -2929,11 +2958,9 @@
     def _run(self):
         self.log.info("VM wrapper has started")
         self.conf['smp'] = self.conf.get('smp', '1')
-        devices = self.buildConfDevices()
 
-        # TODO: In recover should loop over disks running on the VM because
-        # conf may be outdated if something happened during restart.
         if not 'recover' in self.conf:
+            devices = self.buildConfDevices()
             self.preparePaths(devices[DISK_DEVICES])
             self._prepareTransientDisks(devices[DISK_DEVICES])
             # Update self.conf with updated devices
@@ -2951,6 +2978,17 @@
             # So, to get proper device objects during VM recovery flow
             # we must to have updated conf before VM run
             self.saveState()
+        else:
+            # TODO: In recover should loop over disks running on the VM because
+            # conf may be outdated if something happened during restart.
+
+            # For BC we should to keep running VM run after vdsm upgrade.
+            # So, because this vm doesn't have normalize conf we need to build
+            # it in recovery flow
+            if not self.conf.get('devices'):
+                devices = self.buildConfDevices()
+            else:
+                devices = self.getConfDevices()
 
             for drive in devices[DISK_DEVICES]:
                 if drive['device'] == 'disk' and isVdsmImage(drive):


-- 
To view, visit http://gerrit.ovirt.org/21900
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I13d07d6de964ecbc7b518fc53adae0b26cef9658
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list