Change in vdsm[master]: virt: add device setup and teardown

mpolednik at redhat.com mpolednik at redhat.com
Wed May 4 07:46:01 UTC 2016


Martin Polednik has posted comments on this change.

Change subject: virt: add device setup and teardown
......................................................................


Patch Set 14:

(3 comments)

https://gerrit.ovirt.org/#/c/55135/14/tests/vmTests.py
File tests/vmTests.py:

Line 1114: 
Line 1115:         with fake.VM(self.conf, create_device_objects=True) as testvm:
Line 1116:             testvm._devices[hwclass.GENERAL] = devices
Line 1117:             self.assertRaises(ExpectedError, testvm._setup_devices)
Line 1118:             self.assertEqual(devices[0].state, fake.SETUP)
> In abstract, I prefer to do teardown if setup fails. I think it is better t
We have reverted tearing down host devices at all.

I suppose storage/console devices could give us a bigger hint in this regard.
Line 1119:             self.assertEqual(devices[1].state, fake.CREATED)
Line 1120:             self.assertEqual(devices[2].state, fake.CREATED)
Line 1121: 
Line 1122:     def test_device_setup_fail_second(self):


https://gerrit.ovirt.org/#/c/55135/14/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 1719:             for dev_object in dev_objects[:]:
Line 1720:                 try:
Line 1721:                     dev_object.teardown()
Line 1722:                 except Exception:
Line 1723:                     self.log.exception("Failed to teardown device")
> This repeats the code in _teardown_setup_devices, with different log - aren
These functions should be merged.
Line 1724: 
Line 1725:     def _cleanupRecoveryFile(self):
Line 1726:         self._recovery_file.cleanup()
Line 1727: 


Line 1867:         """
Line 1868:         done = []
Line 1869: 
Line 1870:         for dev_objects in self._devices.values():
Line 1871:             for dev_object in dev_objects[:]:
> This code is repeated in both setup_devices and here.
I don't believe this adds anything but confusion unless we move the rest of code to _iter_devices.
Line 1872:                 try:
Line 1873:                     dev_object.setup()
Line 1874:                 except Exception:
Line 1875:                     self.log.exception("Failed to setup device %s",


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f99b855de43cff693b99b6873a835b7ad56db1b
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list