Change in vdsm[master]: vm: drive: allow error policy fine tuning
fromani at redhat.com
fromani at redhat.com
Tue Feb 24 22:43:42 UTC 2015
Francesco Romani has posted comments on this change.
Change subject: vm: drive: allow error policy fine tuning
......................................................................
Patch Set 4:
(8 comments)
https://gerrit.ovirt.org/#/c/38009/4//COMMIT_MSG
Commit Message:
Line 10: so this patch adds the missing bits in VDSM.
Line 11:
Line 12: We add a new Drive property, 'errorPolicy', which
Line 13: will superceed over the legacy, limited
Line 14: 'propagateErrors' if both given.
> This is not tested - right?
I've not yet run actual VMs with this patch.
Line 15:
Line 16: In case the new property is not given, code behaves
Line 17: like before.
Line 18:
https://gerrit.ovirt.org/#/c/38009/4/tests/vmStorageTests.py
File tests/vmStorageTests.py:
Line 265:
Line 266: @expandPermutations
Line 267: class ErrorPolicyTests(XMLTestCase):
Line 268:
Line 269: _xml_template = """
> We don't have to use _private names in a test class. Nobody is using this o
will just remove as discussed below
Line 270: <disk device="disk" snapshot="no" type="file">
Line 271: <source file="/path/to/volume"/>
Line 272: <target bus="virtio" dev="vda"/>
Line 273: <serial>54-a672-23e5b495a9ea</serial>
Line 273: <serial>54-a672-23e5b495a9ea</serial>
Line 274: <driver cache="none" error_policy="%s"
Line 275: io="threads" name="qemu" type="raw"/>
Line 276: </disk>
Line 277: """
> Using a template is fragile - if other unrelated code will change (e.g. one
will change to just find the driver element. Will post perleiminary patch to move code to testlib if needed.
Line 278:
Line 279: @permutations([['stop', 'stop'], ['report', 'report'],
Line 280: ['ignore', 'ignore'], ['enospace', 'enospace']])
Line 281: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 278:
Line 279: @permutations([['stop', 'stop'], ['report', 'report'],
Line 280: ['ignore', 'ignore'], ['enospace', 'enospace']])
Line 281: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 282: def test_set_supported_policy(self, policy, expected):
> test_set_supported -> test_supported?
will change
Line 283: conf = drive_config(errorPolicy=policy)
Line 284: drive = Drive({}, self.log, **conf)
Line 285: self.assertXMLEqual(drive.getXML().toxml(),
Line 286: self._xml_template % expected)
Line 286: self._xml_template % expected)
Line 287:
Line 288: @permutations([[''], [None], [1], ['foobar']])
Line 289: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 290: def test_set_unsupported_policy(self, policy):
> test_set_unsupported -> test_unsupported?
will change
Line 291: conf = drive_config(errorPolicy=policy)
Line 292: drive = Drive({}, self.log, **conf)
Line 293: self.assertRaises(ValueError, drive.getXML)
Line 294:
Line 293: self.assertRaises(ValueError, drive.getXML)
Line 294:
Line 295: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 296: def test_propagate_errors(self):
Line 297: # better explicit than implicit
> Unneeded comment.
will remove
Line 298: conf = drive_config(propagateErrors='on')
Line 299: drive = Drive({}, self.log, **conf)
Line 300: self.assertXMLEqual(drive.getXML().toxml(),
Line 301: self._xml_template % 'enospace')
Line 294:
Line 295: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 296: def test_propagate_errors(self):
Line 297: # better explicit than implicit
Line 298: conf = drive_config(propagateErrors='on')
> How errorPolicy and propagateErrors='on' mix? do we need tests for conflict
errorPolicy and propagateErrors allows both to ultimately control the XML error_policy domain. propagateErrors can be seen as a limited way, while errorPolicy is the new enhanced.
Older Engines will still send propagateErrors to tune the error_policy, so we have to support them
Line 299: drive = Drive({}, self.log, **conf)
Line 300: self.assertXMLEqual(drive.getXML().toxml(),
Line 301: self._xml_template % 'enospace')
Line 302:
Line 300: self.assertXMLEqual(drive.getXML().toxml(),
Line 301: self._xml_template % 'enospace')
Line 302:
Line 303: @MonkeyPatch(utils, 'isBlockDevice', lambda path: False)
Line 304: def test_not_propagate_errors(self):
> not -> do_not?
will change
Line 305: conf = drive_config(propagateErrors='off')
Line 306: drive = Drive({}, self.log, **conf)
Line 307: self.assertXMLEqual(drive.getXML().toxml(),
Line 308: self._xml_template % 'stop')
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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