Change in vdsm[ovirt-3.5]: lvm: Check if device in VG before extend

Hello Fred Rolland, Allon Mureinik,

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

https://gerrit.ovirt.org/46651

to review the following change.

Change subject: lvm: Check if device in VG before extend
......................................................................

lvm: Check if device in VG before extend

Before extending a VG, VDSM needs to check that none of
the additional devices are not already part of the VG.

Without this protection, LVM will create a new PV with the
same device. After extending VG, its metadata will updated
with new PV. As a result, the VG will be partial.

pvs output after creating new PV with same device:
    # pvs
      WARNING: Device for PV 2fDjgH-TeKA-1Vy2-NMMv-GB9n-op0w-g2Nyrd not
               found or rejected by a filter.
      PV
      /dev/mapper/360014054bf6dd7d14fd40c9ac3b5dbe8
      unknown device

Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3
Backport-To: 3.6
Bug-Url: https://bugzilla.redhat.com/1265907
Signed-off-by: Fred Rolland 
Reviewed-on: https://gerrit.ovirt.org/45946
Continuous-Integration: Jenkins CI
Reviewed-by: Allon Mureinik 
Reviewed-by: Nir Soffer 
---
M vdsm/storage/lvm.py
1 file changed, 7 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/46651/1

diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index aa3c04b..7f2adf1 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -962,6 +962,13 @@
     pvs = [_fqpvname(pdev) for pdev in _normalizeargs(devices)]
     _checkpvsblksize(pvs, getVGBlockSizes(vgName))
     vg = _lvminfo.getVg(vgName)
+
+    member_pvs = set(vg.pv_name).intersection(pvs)
+    if member_pvs:
+        log.error("Cannot extend vg %s: pvs already belong to vg %s",
+                  vg.name, member_pvs)
+        raise se.VolumeGroupExtendError(vgName, pvs)
+
     # Format extension PVs as all the other already in the VG
     _initpvs(pvs, int(vg.vg_mda_size) / 2 ** 20, force)

[e-mail] has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 1:

* Update tracker::#1265907::OK
* Check Bug-Url::OK
* Check Public Bug::#1265907::OK, public bug
* Check Product::#1265907::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::#1265907::OK, correct target release 3.5.6
* Check merged to previous::OK, change not open on any previous branch

Freddy Rolland has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 1: Verified+1

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 1: Code-Review+1

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/46651/1//COMMIT_MSG
Commit Message:

Line 21:       /dev/mapper/360014054bf6dd7d14fd40c9ac3b5dbe8
Line 22:       unknown device
Line 23: 
Line 24: Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3
Line 25: Backport-To: 3.6
This should probably be removed, it's already a 3.5.z backport
Line 26: Bug-Url: https://bugzilla.redhat.com/1265907
Line 27: Signed-off-by: Fred Rolland 
Line 28: Reviewed-on: https://gerrit.ovirt.org/45946
Line 29: Continuous-Integration: Jenkins CI

[e-mail] has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 2:

* Update tracker::#1265907::OK
* Check Bug-Url::OK
* Check Public Bug::#1265907::OK, public bug
* Check Product::#1265907::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::#1265907::OK, correct target release 3.5.6
* Check merged to previous::OK, change not open on any previous branch

Freddy Rolland has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/46651/1//COMMIT_MSG
Commit Message:

Line 21:       /dev/mapper/360014054bf6dd7d14fd40c9ac3b5dbe8
Line 22:       unknown device
Line 23: 
Line 24: Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3
Line 25: Backport-To: 3.6
> This should probably be removed, it's already a 3.5.z backport

Done
Line 26: Bug-Url: https://bugzilla.redhat.com/1265907
Line 27: Signed-off-by: Fred Rolland 
Line 28: Reviewed-on: https://gerrit.ovirt.org/45946
Line 29: Continuous-Integration: Jenkins CI

Francesco Romani has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 2: Code-Review+2

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 2: Code-Review+1

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 2:

Can we move forward on this please?

Francesco Romani has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 2: Continuous-Integration+1

run CI tests manually.

Francesco Romani has submitted this change and it was merged.

Change subject: lvm: Check if device in VG before extend
......................................................................

lvm: Check if device in VG before extend

Before extending a VG, VDSM needs to check that none of
the additional devices are not already part of the VG.

Without this protection, LVM will create a new PV with the
same device. After extending VG, its metadata will updated
with new PV. As a result, the VG will be partial.

pvs output after creating new PV with same device:
    # pvs
      WARNING: Device for PV 2fDjgH-TeKA-1Vy2-NMMv-GB9n-op0w-g2Nyrd not
               found or rejected by a filter.
      PV
      /dev/mapper/360014054bf6dd7d14fd40c9ac3b5dbe8
      unknown device

Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3
Bug-Url: https://bugzilla.redhat.com/1265907
Signed-off-by: Fred Rolland 
Reviewed-on: https://gerrit.ovirt.org/45946
Continuous-Integration: Jenkins CI
Reviewed-by: Allon Mureinik 
Reviewed-by: Nir Soffer 
Reviewed-on: https://gerrit.ovirt.org/46651
Reviewed-by: Francesco Romani 
Continuous-Integration: Francesco Romani 
---
M vdsm/storage/lvm.py
1 file changed, 7 insertions(+), 0 deletions(-)

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

[e-mail] has posted comments on this change.

Change subject: lvm: Check if device in VG before extend
......................................................................

Patch Set 3:

* Update tracker::#1265907::OK
* Set MODIFIED::bug 1265907::::#1265907::::IGNORE, not oVirt prod but Red Hat Enterprise Virtualization Manager