Martin Polednik has uploaded a new change for review.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
vdsm: refactor buildConfDevices and getConfDevices into single function
Removes code duplication (flow should remain the same) - also removes getConfSmartcard call in buildConfDevices as this call is obsolete (smartcard was introduced later and is therefore not needed for backwards compatibility)
Change-Id: Ifd3a209967f97a55fe861c4446e8f93e1a1da08e Signed-off-by: Martin Polednik mpoledni@redhat.com --- M vdsm/vm.py 1 file changed, 25 insertions(+), 62 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/19348/1
diff --git a/vdsm/vm.py b/vdsm/vm.py index 92d274e..8957464 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -1854,30 +1854,6 @@ 'truesize': 0}) return removables
- def getConfDevices(self): - devices = {DISK_DEVICES: [], NIC_DEVICES: [], - SOUND_DEVICES: [], VIDEO_DEVICES: [], - CONTROLLER_DEVICES: [], GENERAL_DEVICES: [], - BALLOON_DEVICES: [], REDIR_DEVICES: [], - WATCHDOG_DEVICES: [], CONSOLE_DEVICES: [], - SMARTCARD_DEVICES: []} - 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]) - - # Avoid overriding the saved balloon target value on recovery. - if 'recover' not in self.conf: - for dev in devices[BALLOON_DEVICES]: - dev['target'] = int(self.conf.get('memSize')) * 1024 - return devices - def buildConfDevices(self): """ Return the "devices" section of this Vm's conf. @@ -1886,24 +1862,32 @@ # 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 = {} + devices = {DISK_DEVICES: [], + NIC_DEVICES: [], + SOUND_DEVICES: [], + VIDEO_DEVICES: [], + CONTROLLER_DEVICES: [], + GENERAL_DEVICES: [], + BALLOON_DEVICES: [], + REDIR_DEVICES: [], + WATCHDOG_DEVICES: [], + CONSOLE_DEVICES: [], + SMARTCARD_DEVICES: []} # Build devices structure - if self.conf.get('devices') is None: - with self._confLock: - self.conf['devices'] = [] + if self.conf.get('devices') is not None: + 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) + else: 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: - devices = self.getConfDevices()
# libvirt only support one watchdog device if len(devices[WATCHDOG_DEVICES]) > 1: @@ -1925,6 +1909,8 @@ self._normalizeDriveSharedAttribute(drv) 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. @@ -1964,17 +1950,6 @@ '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): """ @@ -2039,9 +2014,6 @@ # 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
@@ -2863,9 +2835,11 @@ 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]) # Update self.conf with updated devices # For old type vmParams, new 'devices' key will be @@ -2882,17 +2856,6 @@ # 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()
devMap = {DISK_DEVICES: Drive, NIC_DEVICES: NetworkInterfaceDevice,
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4500/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4419/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3603/ : SUCCESS
Martin Polednik has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 1: Verified+1
Michal Skrivanek has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 1: Code-Review+1
Antoni Segura Puimedon has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 1:
(1 comment)
Very very minor thing, It's not necessary to do for this patch.
.................................................... File vdsm/vm.py Line 1873: WATCHDOG_DEVICES: [], Line 1874: CONSOLE_DEVICES: [], Line 1875: SMARTCARD_DEVICES: []} Line 1876: # Build devices structure Line 1877: if self.conf.get('devices') is not None: Just a minor thing. It is generally more readable to have is None and start with what now is in the else and then do the first block. Line 1878: for dev in self.conf.get('devices'): Line 1879: try: Line 1880: devices[dev['type']].append(dev) Line 1881: except KeyError:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4789/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4865/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3980/ : SUCCESS
Martin Polednik has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 2: Verified+1
Ayal Baron has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 2:
(1 comment)
.................................................... File vdsm/vm.py Line 1856: 'index': 0, Line 1857: 'truesize': 0}) Line 1858: return removables Line 1859: Line 1860: def buildConfDevices(self): This patch needs to be rebased on top of http://gerrit.ovirt.org/#/c/1932 and that way the balloon part above disappearing would make sense. Line 1861: """ Line 1862: Return the "devices" section of this Vm's conf. Line 1863: If missing, create it according to old API. Line 1864: """
Martin Polednik has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 2: -Verified
(1 comment)
.................................................... File vdsm/vm.py Line 1856: 'index': 0, Line 1857: 'truesize': 0}) Line 1858: return removables Line 1859: Line 1860: def buildConfDevices(self): the balloon part is actually supposed to be here, the patches are designed to be self-contained Line 1861: """ Line 1862: Return the "devices" section of this Vm's conf. Line 1863: If missing, create it according to old API. Line 1864: """
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4992/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4105/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4915/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4449/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5251/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5329/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4556/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5356/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5435/ : SUCCESS
Federico Simoncelli has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 5: Code-Review+2
(1 comment)
.................................................... File vdsm/vm.py Line 1913: for drv in devices[DISK_DEVICES]: Line 1914: if isVdsmImage(drv): Line 1915: self._normalizeVdsmImg(drv) Line 1916: Line 1917: self.normalizeDrivesIndices(devices[DISK_DEVICES]) If I understand correctly this is here because it was called also in getConfDrives. Line 1918: Line 1919: # Preserve old behavior. Since libvirt add a memory balloon device Line 1920: # to all guests, we need to specifically request not to add it. Line 1921: self._normalizeBalloonDevice(devices[BALLOON_DEVICES])
Martin Polednik has posted comments on this change.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
Patch Set 5: Verified+1
(1 comment)
.................................................... File vdsm/vm.py Line 1913: for drv in devices[DISK_DEVICES]: Line 1914: if isVdsmImage(drv): Line 1915: self._normalizeVdsmImg(drv) Line 1916: Line 1917: self.normalizeDrivesIndices(devices[DISK_DEVICES]) correct Line 1918: Line 1919: # Preserve old behavior. Since libvirt add a memory balloon device Line 1920: # to all guests, we need to specifically request not to add it. Line 1921: self._normalizeBalloonDevice(devices[BALLOON_DEVICES])
Dan Kenigsberg has submitted this change and it was merged.
Change subject: vdsm: refactor buildConfDevices and getConfDevices into single function ......................................................................
vdsm: refactor buildConfDevices and getConfDevices into single function
Removes code duplication while flow should remain the same - also removes getConfSmartcard call in buildConfDevices as this call is obsolete (smartcard was introduced later and is therefore not needed for backwards compatibility)
Change-Id: Ifd3a209967f97a55fe861c4446e8f93e1a1da08e Signed-off-by: Martin Polednik mpoledni@redhat.com Reviewed-on: http://gerrit.ovirt.org/19348 Reviewed-by: Federico Simoncelli fsimonce@redhat.com --- M vdsm/vm.py 1 file changed, 14 insertions(+), 52 deletions(-)
Approvals: Federico Simoncelli: Looks good to me, approved Martin Polednik: Verified
vdsm-patches@lists.fedorahosted.org