Change in vdsm[master]: vm: drive: allow error policy fine tuning
nsoffer at redhat.com
nsoffer at redhat.com
Fri Mar 6 23:04:02 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: vm: drive: allow error policy fine tuning
......................................................................
Patch Set 8:
(6 comments)
https://gerrit.ovirt.org/#/c/38009/8/tests/vmStorageTests.py
File tests/vmStorageTests.py:
Line 272: io="threads" name="qemu" type="raw"/>
Line 273: """
Line 274:
Line 275: @permutations([['stop', 'stop'], ['report', 'report'],
Line 276: ['ignore', 'ignore'], ['enospace', 'enospace']])
We can simplify the permutations, as policy == expected:
@permuatations([["stop", "report", ...])
def test_xxx(self, policy):
...
Line 277: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 278: def test_supported_policy(self, policy, expected):
Line 279: conf = drive_config(errorPolicy=policy)
Line 280: drive = Drive({}, self.log, **conf)
Line 275: @permutations([['stop', 'stop'], ['report', 'report'],
Line 276: ['ignore', 'ignore'], ['enospace', 'enospace']])
Line 277: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 278: def test_supported_policy(self, policy, expected):
Line 279: conf = drive_config(errorPolicy=policy)
Actually this test the combination of propagateErrors='off' and the new policy.
Line 280: drive = Drive({}, self.log, **conf)
Line 281: xml = find_xml_element(drive.getXML().toxml(), './driver')
Line 282: self.assertXMLEqual(xml, self.EXPECTED_XML % expected)
Line 283:
Line 281: xml = find_xml_element(drive.getXML().toxml(), './driver')
Line 282: self.assertXMLEqual(xml, self.EXPECTED_XML % expected)
Line 283:
Line 284: @permutations([['stop', 'stop'], ['report', 'report'],
Line 285: ['ignore', 'ignore'], ['enospace', 'enospace']])
Same as previous test
Line 286: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 287: def test_supported_policy_override_legacy(self, policy, expected):
Line 288: conf = drive_config(errorPolicy=policy, propagateErrors='on')
Line 289: drive = Drive({}, self.log, **conf)
Line 287: def test_supported_policy_override_legacy(self, policy, expected):
Line 288: conf = drive_config(errorPolicy=policy, propagateErrors='on')
Line 289: drive = Drive({}, self.log, **conf)
Line 290: xml = find_xml_element(drive.getXML().toxml(), './driver')
Line 291: self.assertXMLEqual(xml, self.EXPECTED_XML % expected)
The first two tests are identical, we can replace them with one test using permutations:
@permutaions([
["stop", "on"], ["stop", "off"],
["enospace", "on"], ["enospace", "off"],
["report", "on"], ["report", "off"],
["ignore", "on"], ["ignore", "off"],
])
def test_policy(sefl, policy, propagateErrors):
...
Line 292:
Line 293: @permutations([[''], [None], [1], ['foobar']])
Line 294: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 295: def test_unsupported_policy(self, policy):
https://gerrit.ovirt.org/#/c/38009/8/vdsm/virt/vmdevices/storage.py
File vdsm/virt/vmdevices/storage.py:
Line 354: driverAttrs['type'] = 'raw'
Line 355:
Line 356: driverAttrs['cache'] = self.cache
Line 357:
Line 358: self._setDriverAttrErrorPolicy(driverAttrs)
This would be nicer and more testable:
driverAttrs['error_policy'] = self.errorPolicy
@property
def errorPolicy(self):
if hasattr('_errorPolicy'):
return self._errorPolicy
else if (self.propagateErrors == 'on' or
utils.tobool(self.propagateErrors)):
return "enospace"
else:
return "stop"
Line 359:
Line 360: diskelem.appendChildWithArgs('driver', **driverAttrs)
Line 361:
Line 362: if hasattr(self, 'specParams') and 'ioTune' in self.specParams:
Line 369: return diskelem
Line 370:
Line 371: def _setDriverAttrErrorPolicy(self, driverAttrs):
Line 372: if hasattr(self, 'errorPolicy'):
Line 373: if self.errorPolicy in _ERROR_POLICIES:
Lets validate when setting errorPolicy. A drive raising in getXML() is not very useful object.
@errorPolicy.setter
def errorPolicy(self, value):
if value not in _ERROR_POLICIES:
raise ...
self._errorPolicy = value
Line 374: driverAttrs['error_policy'] = self.errorPolicy
Line 375: else:
Line 376: raise ValueError("unsupported Error Policy '%s",
Line 377: self.errorPolicy)
--
To view, visit https://gerrit.ovirt.org/38009
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b1234ab15fc87ad75dfeae197d058eec23accbc
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Ravi Nori <rnori at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list