Francesco Romani has submitted this change and it was merged.

Change subject: lvm: Fix error handling when resizing lvs
......................................................................


lvm: Fix error handling when resizing lvs

We depended on undocumented and wrong behavior in lvextend, returning
exit code 3 when lvextend failed because lv size is same or bigger then
the requested size. Exit code 3 means the command was invoked with
incorrect syntax. In EL 7.3, lvm changed the behavior and now lvextend
is returning 5 in this case:

    # lvextend --size 100m vg/lv; echo $?
      Rounding size to boundary between physical extents: 128.00 MiB.
      New size (1 extents) matches existing size (1 extents)
    5

This change caused lvextend to fail in many flows, since we tend to do
lot of unneeded resizes.

Using exit code 3 was also hiding real usage errors like this:

    # lvextend --no-such-option 100m vg/lv; echo $?
    lvextend: unrecognized option '--nosuchoption'
      Error during parsing of command line.
    3

When handling errors, we used to compare the requested size with the
free size on the vg instead of the additional size, raising misleading
exception. This check is also relevant only for extending lv, but we
raised it also when reducing lv.

Finally, if the op parameter was invalid, the lvm command would fail,
and we used to log or raise incorrect information about the error.

Changes in this patch:
- Replace the exit code check with lv size check
- Check sizes using extents matching lvm behavior
- Consider operation type when checking errors
- Raise clear error if used with invalid operation

Since we don't have a LogicalVolumeReduceError, we raise
LogicalVolumeExtendError also for reduce errors.

Change-Id: Id557df0b809f7ee141f2e9ea0e1c83084c71bb49
Bug-Url: https://bugzilla.redhat.com/1363734
Bug-Url: https://bugzilla.redhat.com/1364339
Bug-Url: https://bugzilla.redhat.com/1366176
Signed-off-by: Nir Soffer <nsof...@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/62338
Continuous-Integration: Jenkins CI
Reviewed-by: Freddy Rolland <froll...@redhat.com>
Reviewed-by: Allon Mureinik <amure...@redhat.com>
Reviewed-by: Adam Litke <ali...@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/62734
Tested-by: Allon Mureinik <amure...@redhat.com>
Reviewed-by: Francesco Romani <from...@redhat.com>
---
M vdsm/storage/lvm.py
1 file changed, 39 insertions(+), 15 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id557df0b809f7ee141f2e9ea0e1c83084c71bb49
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to