Change in vdsm[master]: lvm: ignore skipped clustered vgs

nsoffer at redhat.com nsoffer at redhat.com
Thu Oct 17 14:29:18 UTC 2013


Nir Soffer has posted comments on this change.

Change subject: lvm: ignore skipped clustered vgs
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

This patch may be great but it does not explain why it is needed, not in the commit message or code. And it will possibly break the installation again, adding dependency on newer versions of lvm.

....................................................
Commit Message
Line 3: AuthorDate: 2013-10-16 11:38:25 -0400
Line 4: Commit:     Federico Simoncelli <fsimonce at redhat.com>
Line 5: CommitDate: 2013-10-16 11:39:55 -0400
Line 6: 
Line 7: lvm: ignore skipped clustered vgs
Why? What is the context of this change?
Line 8: 
Line 9: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=820982
Line 10: Change-Id: Idcabb5c1c19106f0db1385787251d951e12ab0cf


....................................................
File vdsm.spec.in
Line 158: Requires: policycoreutils >= 2.0.83-19.30
Line 159: Requires: policycoreutils-python >= 2.0.83-19.30
Line 160: Requires: python-pthreading
Line 161: Requires: selinux-policy-targeted >= 3.7.19-195.el6.13
Line 162: Requires: lvm2 >= 2.02.100-5
Is it available now?
Line 163: Requires: logrotate < 3.8.0
Line 164: %endif
Line 165: %else
Line 166: %if 0%{?fedora} >= 19


Line 186: Requires: sanlock >= 2.4-2, sanlock-python
Line 187: Requires: policycoreutils-python
Line 188: Requires: sed >= 4.2.1-10
Line 189: Requires: ed
Line 190: Requires: lvm2 >= 2.02.103
Not sure what do you mean by this comment. Is this version available now?
Line 191: 
Line 192: %if 0%{?fedora} >= 18
Line 193: %if 0%{?fedora} >= 19
Line 194: Requires: selinux-policy-targeted >= 3.12.1-71


....................................................
File vdsm/storage/lvm.py
Line 80: 
Line 81: SEPARATOR = "|"
Line 82: LVM_NOBACKUP = ("--autobackup", "n")
Line 83: LVM_FLAGS = ("--noheadings", "--units", "b", "--nosuffix", "--separator",
Line 84:              SEPARATOR, "--ignoreskippedcluster")
Using this option only if available should be better, unless this code will never run with older lvm.
Line 85: 
Line 86: PV_PREFIX = "/dev/mapper"
Line 87: # Assuming there are no spaces in the PV name
Line 88: re_pvName = re.compile(PV_PREFIX + '[^\s\"]+', re.MULTILINE)


-- 
To view, visit http://gerrit.ovirt.org/20248
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcabb5c1c19106f0db1385787251d951e12ab0cf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list