Nir Soffer has uploaded a new change for review.
Change subject: lvm: Fail loudly if called with unexptected input. ......................................................................
lvm: Fail loudly if called with unexptected input.
When creating pvs with the force option, we are very carefull to accept only True. When using the jsonrpc transport, engine was sending "true" and "false", causing the call to fail misteiously.
Now we are also carefull about rejecting invlid input, making debugging easier.
Change-Id: If9e6754d4aa2efaf894a9309cfaa4595d710063b Signed-off-by: Nir Soffer nsoffer@redhat.com --- M vdsm/storage/lvm.py 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/37329/1
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py index aa3c04b..549839a 100644 --- a/vdsm/storage/lvm.py +++ b/vdsm/storage/lvm.py @@ -724,6 +724,11 @@ else: raise
+ # We must be very carefull here; any value execpt True or False is a user + # error. + if type(force) != bool: + raise ValueError("Invalid value for 'force': %r" % force) + if force is True: options = ("-y", "-ff") _initpvs_removeHolders()
automation@ovirt.org has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexptected input. ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Nir Soffer has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 2:
Spelling
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15441/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15272/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14484/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15442/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15273/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14485/ : FAILURE
Allon Mureinik has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/37329/2//COMMIT_MSG Commit Message:
Line 12: Line 13: Now we are also careful about rejecting invalid input, making debugging Line 14: easier. Line 15: Line 16: Change-Id: If9e6754d4aa2efaf894a9309cfaa4595d710063b Consider adding a Related-To: footer
automation@ovirt.org has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15458/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/15289/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14501/ : FAILURE
Allon Mureinik has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3: Code-Review+1
Federico Simoncelli has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/37329/3/vdsm/storage/lvm.py File vdsm/storage/lvm.py:
Line 724: else: Line 725: raise Line 726: Line 727: # We must be very carefull here; any value execpt True or False is a user Line 728: # error. If this comes from the xml/json rpc API it should be validated there. Internally we shouldn't start validating types. Line 729: if type(force) != bool: Line 730: raise ValueError("Invalid value for 'force': %r" % force) Line 731: Line 732: if force is True:
Nir Soffer has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/37329/3/vdsm/storage/lvm.py File vdsm/storage/lvm.py:
Line 724: else: Line 725: raise Line 726: Line 727: # We must be very carefull here; any value execpt True or False is a user Line 728: # error.
If this comes from the xml/json rpc API it should be validated there. Inter
I agree that we should not validate types in general, but in rare places we *require* certain types, we must validate them. Line 729: if type(force) != bool: Line 730: raise ValueError("Invalid value for 'force': %r" % force) Line 731: Line 732: if force is True:
Dan Kenigsberg has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/37329/3/vdsm/storage/lvm.py File vdsm/storage/lvm.py:
Line 731: Line 732: if force is True: Line 733: options = ("-y", "-ff") Line 734: _initpvs_removeHolders() Line 735: else: how about adding it after an
elif force is False:
? Line 736: options = tuple() Line 737: Line 738: rc, out, err = _createpv(devices, metadataSize, options) Line 739: _lvminfo._invalidatepvs(devices)
Nir Soffer has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/37329/3/vdsm/storage/lvm.py File vdsm/storage/lvm.py:
Line 731: Line 732: if force is True: Line 733: options = ("-y", "-ff") Line 734: _initpvs_removeHolders() Line 735: else:
how about adding it after an
It does not express the intent of the code.
The code is about:
1. Validate input 2. Handle True or False
And not about:
1. Handle one of the values True, False, Other Line 736: options = tuple() Line 737: Line 738: rc, out, err = _createpv(devices, metadataSize, options) Line 739: _lvminfo._invalidatepvs(devices)
Jenkins CI RO has abandoned this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Abandoned
Abandoned due to no activity - please restore if still relevant
Jenkins CI RO has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
Abandoned due to no activity - please restore if still relevant
gerrit-hooks has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 3:
* Update tracker: IGNORE, no Bug-Url found
Nir Soffer has restored this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Restored
Still needed
Jenkins CI RO has abandoned this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Abandoned
Abandoned due to no activity - please restore if still relevant
gerrit-hooks has posted comments on this change.
Change subject: lvm: Fail loudly if called with unexpected input ......................................................................
Patch Set 4:
* Update Tracker::#1185865::IGNORE, not relevant for Red Hat classification
vdsm-patches@lists.fedorahosted.org