Change in vdsm[master]: BZ#836161 - Pluralize lvm.changelv().
abaron at redhat.com
abaron at redhat.com
Mon Oct 15 09:40:55 UTC 2012
Ayal Baron has posted comments on this change.
Change subject: BZ#836161 - Pluralize lvm.changelv().
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(6 inline comments)
....................................................
File vdsm/storage/blockVolume.py
Line 646:
Line 647: lvNames = tuple(vol.volUUID for vol in volumes)
Line 648: #Assert volumes are writable. (Don't do this at home.)
Line 649: try:
Line 650: lvm.changelv(sdUUID, lvNames, ("--permission", "rw"))
why not move this into a variable?
Line 651: except se.StorageException, e:
Line 652: # Hope this only means that some volumes were already writable.
Line 653: pass
Line 654:
....................................................
File vdsm/storage/lvm.py
Line 729: """
Line 730: Change multiple attributes on multiple LVs.
Line 731:
Line 732: vg: VG name
Line 733: lvs: a single LV name or LV names iterable.
s/a single.*/iterable list of lvs/
Line 734: attrs: a (attr, value) tuple or a tuple of (attr, value) tuples.
Line 735:
Line 736: Note:
Line 737: You may activate an activated LV without error
Line 730: Change multiple attributes on multiple LVs.
Line 731:
Line 732: vg: VG name
Line 733: lvs: a single LV name or LV names iterable.
Line 734: attrs: a (attr, value) tuple or a tuple of (attr, value) tuples.
s/a (.*/an iterable of (attr, value) pairs), e.g. (('--available', 'y'), ('--permission', 'rw'))/
Line 735:
Line 736: Note:
Line 737: You may activate an activated LV without error
Line 738: but lvchange returns an error (RC=5) when activating rw if already rw
Line 738: but lvchange returns an error (RC=5) when activating rw if already rw
Line 739: """
Line 740:
Line 741: lvs = _normalizeargs(lvs)
Line 742: # If it fails or not we (may be) change the lv,
s/may be/maybe/
Line 743: # so we invalidate cache to reload these volumes on first occasion
Line 744: lvnames = tuple("%s/%s" % (vg, lv) for lv in lvs)
Line 745: cmd = ["lvchange"]
Line 746: cmd.extend(LVM_NOBACKUP)
Line 744: lvnames = tuple("%s/%s" % (vg, lv) for lv in lvs)
Line 745: cmd = ["lvchange"]
Line 746: cmd.extend(LVM_NOBACKUP)
Line 747: for attr in attrs:
Line 748: if type(attr) == type(''):
s/.*/isinstance(attr, types.StringTypes)/
Line 749: cmd.append(attr)
Line 750: else:
Line 751: cmd.extend(attr)
Line 752: cmd.extend(lvnames)
Line 745: cmd = ["lvchange"]
Line 746: cmd.extend(LVM_NOBACKUP)
Line 747: for attr in attrs:
Line 748: if type(attr) == type(''):
Line 749: cmd.append(attr)
you could do this in a single 'if' on attrs
Line 750: else:
Line 751: cmd.extend(attr)
Line 752: cmd.extend(lvnames)
Line 753: rc, out, err = _lvminfo.cmd(tuple(cmd))
--
To view, visit http://gerrit.ovirt.org/8289
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I22c4ef83f23c0004a5a1e6a8a7e55d13b56c4f50
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
More information about the vdsm-patches
mailing list