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