Change in vdsm[master]: vm: drive: allow error policy fine tuning
fromani at redhat.com
fromani at redhat.com
Mon Mar 9 07:38:04 UTC 2015
Francesco Romani has posted comments on this change.
Change subject: vm: drive: allow error policy fine tuning
......................................................................
Patch Set 8:
(7 comments)
https://gerrit.ovirt.org/#/c/38009/8//COMMIT_MSG
Commit Message:
Line 11: the guest OS, pausing the VM only if disk space was exhausted.
Line 12:
Line 13: This approach was nice and simple, but sometimes just too limiting
Line 14: and some users asked for more fine-grained drive error policy
Line 15: control.
> Currently we support only "stop" and "enospace", and we have mechanism for
This is a very valid point, but better to continue this conversation on BZ (as you already begun to do).
Meantime, I will update this patch as you suggested.
If we settle to just export 'propagateErrors' on Engine I'll drop this patch completely.
Line 16:
Line 17: To allow that, we add a new Drive property, 'errorPolicy', which
Line 18: will superceed over the legacy, limited 'propagateErrors' tunable,
Line 19: in the case both are given.
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:
Done
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 pol
will rename. Or you want it dropped?
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
will rename in the next submission
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
will merge
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:
indeed nicer. Will change.
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
Indeed nicer. Will change.
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: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce 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