Change in vdsm[ovirt-3.6]: lvm: Separate lv reduce and extend

[automation]

gerrit-hooks has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 3:

* #1363734::Update tracker: OK
* #1364339::Update tracker: OK
* #1366176::Update tracker: OK
* Set MODIFIED::bug 1363734#1363734OK
* Set MODIFIED::bug 1363734bug 1364339#1364339::SKIPPED, tm_suffix 
'4.0.4' does not match branch_suffix '3.6'.
* Set MODIFIED::bug 1363734bug 1364339bug 1366176#1366176::SKIPPED, 
tm_suffix '4.0.4' does not match branch_suffix '3.6'.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[fromani]

Francesco Romani has submitted this change and it was merged.

Change subject: lvm: Separate lv reduce and extend
..


lvm: Separate lv reduce and extend

Reducing and extending logical volumes were mixed in a horrible and
buggy mess in _resizeLV. Now we have a much simpler and clear function
for each operation.

Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
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 
Reviewed-on: https://gerrit.ovirt.org/62369
Reviewed-by: Freddy Rolland 
Reviewed-by: Allon Mureinik 
Reviewed-by: Adam Litke 
Continuous-Integration: Jenkins CI
Reviewed-on: https://gerrit.ovirt.org/62735
Tested-by: Allon Mureinik 
Reviewed-by: Francesco Romani 
---
M vdsm/storage/lvm.py
1 file changed, 41 insertions(+), 42 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/62735
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
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-3.6]: lvm: Separate lv reduce and extend

[fromani]

Francesco Romani has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[amureini]

Allon Mureinik has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 2: Verified+1

Marking VERIFIED as per automation tier 1 run.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 2: Code-Review+1

We did not plan to backport this, but I would like to have the same code in 3.6 
to make it easier to maintain.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[automation]

gerrit-hooks has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 2: -Verified

* #1363734::Update tracker: OK
* #1364339::Update tracker: OK
* #1366176::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1363734::OK, public bug
* Check Public Bug::#1364339::OK, public bug
* Check Public Bug::#1366176::OK, public bug
* Check Product::#1363734::OK, Correct classification oVirt
* Check Product::#1364339::OK, Correct classification oVirt
* Check Product::#1366176::OK, Correct classification oVirt
* Check TM::#1363734::OK, correct target milestone ovirt-3.6.9
* Check TM::#1364339::ERROR, wrong target milestone for stable branch, 
ovirt-4.0.4 should match ^.*3.6.*
* Check TM::#1366176::ERROR, wrong target milestone for stable branch, 
ovirt-4.0.4 should match ^.*3.6.*
* Check merged to previous::OK, change not open on any previous branch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[automation]

gerrit-hooks has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found

* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch
* Check merged to previous::OK, change not open on any previous branch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[fromani]

Francesco Romani has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 1: Code-Review+1

No Bug-Url? Also, please verify on branch.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[fromani]

Francesco Romani has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 1:

Rerun-Hooks: all

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Francesco Romani 
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-3.6]: lvm: Separate lv reduce and extend

[frolland]

Freddy Rolland has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
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-3.6]: lvm: Separate lv reduce and extend

[automation]

gerrit-hooks has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 1: Verified-1

* Update tracker: IGNORE, no Bug-Url found

* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch
* Check merged to previous::WARN, Still open on branches ovirt-4.0

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
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-3.6]: lvm: Separate lv reduce and extend

[alitke]

Hello Nir Soffer, Freddy Rolland,

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

https://gerrit.ovirt.org/62735

to review the following change.

Change subject: lvm: Separate lv reduce and extend
..

lvm: Separate lv reduce and extend

Reducing and extending logical volumes were mixed in a horrible and
buggy mess in _resizeLV. Now we have a much simpler and clear function
for each operation.

Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Signed-off-by: Nir Soffer 
Reviewed-on: https://gerrit.ovirt.org/62369
Reviewed-by: Freddy Rolland 
Reviewed-by: Allon Mureinik 
Reviewed-by: Adam Litke 
Continuous-Integration: Jenkins CI
---
M vdsm/storage/lvm.py
1 file changed, 41 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/62735/1

diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 7717622..7ccf110 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -1143,19 +1143,10 @@
 raise se.CannotRemoveLogicalVolume(vgName, str(lvNames))
 
 
-# TODO: kill this, inline into extendLV and reduceLV.
-def _resizeLV(op, vgName, lvName, size):
-"""
-Size units: MB (1024 ** 2 = 2 ** 20)B.
-"""
-# WARNING! From man vgs:
-# All sizes are output in these units: (h)uman-readable, (b)ytes,
-# (s)ectors,(k)ilobytes, (m)egabytes, (g)igabytes, (t)erabytes,
-# (p)etabytes, (e)xabytes.
-# Capitalise to use multiples of 1000 (S.I.) instead of 1024.
-cmd = (op,) + LVM_NOBACKUP
-cmd += ("--size", "%sm" % (size,), "%s/%s" % (vgName, lvName))
-rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))
+def extendLV(vgName, lvName, size_mb):
+cmd = ("lvextend",) + LVM_NOBACKUP
+cmd += ("--size", "%sm" % (size_mb,), "%s/%s" % (vgName, lvName))
+rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName,)))
 if rc != 0:
 # Since this runs only on the SPM, assume that cached vg and lv
 # metadata is correct.
@@ -1165,46 +1156,54 @@
 # Convert sizes to extents to match lvm behavior.
 extent_size = int(vg.extent_size)
 lv_extents = int(lv.size) // extent_size
-requested_size = size * constants.MEGAB
+requested_size = size_mb * constants.MEGAB
 requested_extents = (requested_size + extent_size - 1) // extent_size
 
-if op == "lvextend":
-if lv_extents >= requested_extents:
-log.debug("LV %s/%s already extended (extents=%d, "
-  "requested=%d)",
-  vgName, lvName, lv_extents, requested_extents)
-return
+if lv_extents >= requested_extents:
+log.debug("LV %s/%s already extended (extents=%d, requested=%d)",
+  vgName, lvName, lv_extents, requested_extents)
+return
 
-needed_extents = requested_extents - lv_extents
-free_extents = int(vg.free_count)
-if free_extents < needed_extents:
-raise se.VolumeGroupSizeError(
-"Not enough free extents to extend LV %s/%s (free=%d, "
-"needed=%d)"
-% (vgName, lvName, needed_extents, free_extents))
-elif op == "lvreduce":
-if lv_extents <= requested_extents:
-# May happen since callers do not calculate size in extents.
-log.debug("LV %s/%s already reduced (extents=%d, "
-  "requested=%d)",
-  vgName, lvName, lv_extents, requested_extents)
-return
-else:
-raise RuntimeError("Invalid resize operation %r" % op)
+needed_extents = requested_extents - lv_extents
+free_extents = int(vg.free_count)
+if free_extents < needed_extents:
+raise se.VolumeGroupSizeError(
+"Not enough free extents for extending LV %s/%s (free=%d, "
+"needed=%d)"
+% (vgName, lvName, free_extents, needed_extents))
 
-# TODO: add and raise LogicalVolumeReduceError for lvreduce
-raise se.LogicalVolumeExtendError(vgName, lvName, "%sM" % (size, ))
+raise se.LogicalVolumeExtendError(vgName, lvName, "%sm" % (size_mb,))
 
 _lvminfo._invalidatevgs(vgName)
 _lvminfo._invalidatelvs(vgName, lvName)
 
 
-def extendLV(vgName, lvName, size):
-_resizeLV("lvextend", vgName, lvName, size)
+def reduceLV(vgName, lvName, size_mb):
+cmd = ("lvreduce",) + LVM_NOBACKUP
+cmd += ("--size", "%sm" % (size_mb,), "%s/%s" % (vgName, lvName))
+rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName,)))
+if rc != 0:
+# Since this runs only on the SPM, assume that cached vg and lv
+# metadata is correct.
+vg = getVG(vgName)
+lv = getLV(vgName, lvName)
 
+#