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

fromani at redhat.com fromani at redhat.com
Wed May 4 06:58:55 UTC 2016


Francesco Romani has posted comments on this change.

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


Patch Set 14:

(2 comments)

partial review

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)
> Now we have the decide - if setup fails, do we teardown the device?
In abstract, I prefer to do teardown if setup fails. I think it is better to clean up after ourselves and avoid partially done stuff.
However, I'm thinking about actual real usecases trying to figure out what's best in practice. The most complex devices we handle are host devices, so let's take in account first what's best for them.

Martin, I believe that for host devices it is indeed better to do teardown if setup fails, right? Am I remembering right?
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):


Line 1167:             self.assertEqual(devices[1].state, fake.TEARDOWN)
Line 1168:             self.assertEqual(devices[2].state, fake.TEARDOWN)
Line 1169: 
Line 1170:     def test_device_teardown_fail_all(self):
Line 1171:         devices = [fake.Device('device_{}'.format(i), fail_teardown=True)
> fail_teardown=UnexpectedError
Yes please do that
Line 1172:                    for i in range(3)]
Line 1173: 
Line 1174:         with fake.VM(self.conf, create_device_objects=True) as testvm:
Line 1175:             testvm._devices[hwclass.GENERAL] = devices


-- 
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