Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-29 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: lvm: Never hide lvm errors
..


Patch Set 2:

* update_tracker: OK
* Set MODIFIED::bug 1358348#1358348::SKIPPED, tm_suffix '' does not match 
branch_suffix '4.0'.

-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-29 Thread fromani 
Francesco Romani has submitted this change and it was merged.

Change subject: lvm: Never hide lvm errors
..


lvm: Never hide lvm errors

In changelv, if lvm command has failed but stdout is not empty, the
error was ignored silently. This may hide errors when deactivating lvs
leading to stale lvs, or hide other errors.

This code was introduced in 2009 with the first version of the lvm
module, and there is no documentation why ignoring errors when stdout is
not empty is needed.

Testing lvm show that using verbose mode (-v) will write to stdout and
cause errors to be hidden. I could not find lvm command that requires
this strange treatment, and no other lvm command is using it.

This patch removes the stdout empty check, raising if lvchange return
non-zero exit code.

Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Bug-Url: https://bugzilla.redhat.com/1358348
Signed-off-by: Nir Soffer 
Reviewed-on: https://gerrit.ovirt.org/61287
Reviewed-by: Allon Mureinik 
Reviewed-by: Adam Litke 
Continuous-Integration: Jenkins CI
Reviewed-on: https://gerrit.ovirt.org/62738
Reviewed-by: Freddy Rolland 
Tested-by: Maor Lipchuk 
Reviewed-by: Francesco Romani 
---
M vdsm/storage/lvm.py
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Francesco Romani: Looks good to me, approved
  Freddy Rolland: Looks good to me, but someone else must approve
  Maor Lipchuk: Verified



-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-29 Thread fromani 
Francesco Romani has posted comments on this change.

Change subject: lvm: Never hide lvm errors
..


Patch Set 1: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-29 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: lvm: Never hide lvm errors
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-28 Thread mlipchuk 
Maor Lipchuk has posted comments on this change.

Change subject: lvm: Never hide lvm errors
..


Patch Set 1: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-24 Thread frolland 
Freddy Rolland has posted comments on this change.

Change subject: lvm: Never hide lvm errors
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-23 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: lvm: Never hide lvm errors
..


Patch Set 1:

* update_tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1358348::ERROR, private bug
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::OK, change not open on any previous branch

-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: lvm: Never hide lvm errors

2016-08-23 Thread alitke 
Hello Nir Soffer,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/62738

to review the following change.

Change subject: lvm: Never hide lvm errors
..

lvm: Never hide lvm errors

In changelv, if lvm command has failed but stdout is not empty, the
error was ignored silently. This may hide errors when deactivating lvs
leading to stale lvs, or hide other errors.

This code was introduced in 2009 with the first version of the lvm
module, and there is no documentation why ignoring errors when stdout is
not empty is needed.

Testing lvm show that using verbose mode (-v) will write to stdout and
cause errors to be hidden. I could not find lvm command that requires
this strange treatment, and no other lvm command is using it.

This patch removes the stdout empty check, raising if lvchange return
non-zero exit code.

Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Bug-Url: https://bugzilla.redhat.com/1358348
Signed-off-by: Nir Soffer 
Reviewed-on: https://gerrit.ovirt.org/61287
Reviewed-by: Allon Mureinik 
Reviewed-by: Adam Litke 
Continuous-Integration: Jenkins CI
---
M vdsm/storage/lvm.py
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/62738/1

diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 03cf211..6fb7760 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -813,7 +813,7 @@
 cmd.extend(lvnames)
 rc, out, err = _lvminfo.cmd(tuple(cmd), _lvminfo._getVGDevs((vg, )))
 _lvminfo._invalidatelvs(vg, lvs)
-if rc != 0 and len(out) < 1:
+if rc != 0:
 raise se.StorageException("%d %s %s\n%s/%s" % (rc, out, err, vg, lvs))
 
 


-- 
To view, visit https://gerrit.ovirt.org/62738
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ce160ecddebfc903fb7bc00ba5a69e63f2cc996
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Nir Soffer 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org