Change in vdsm[master]: lvm: Allow multiple initial tags when creating an LV

nsoffer at redhat.com nsoffer at redhat.com
Wed Dec 2 14:53:55 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: lvm: Allow multiple initial tags when creating an LV
......................................................................


Patch Set 6: Code-Review-1

(2 comments)

Partial review.

https://gerrit.ovirt.org/#/c/44051/6/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 1069: #
Line 1070: 
Line 1071: 
Line 1072: def createLV(vgName, lvName, size, activate=True, contiguous=False,
Line 1073:              initialTags=[]):
Mutable default parameter?

Should be ()
Line 1074:     """
Line 1075:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1076:     """
Line 1077:     # WARNING! From man vgs:


Line 1083:     cont = {True: "y", False: "n"}[contiguous]
Line 1084:     cmd = ["lvcreate"]
Line 1085:     cmd.extend(LVM_NOBACKUP)
Line 1086:     cmd.extend(("--contiguous", cont, "--size", "%sm" % size))
Line 1087:     map(cmd.extend, [("--addtag", tag) for tag in initialTags])
Please use simpler code that make your intent clear:

    for tag in initialTags:
        cmd.extend(("--addtag", tag))
Line 1088:     cmd.extend(("--name", lvName, vgName))
Line 1089:     rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))
Line 1090: 
Line 1091:     if rc == 0:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2524f0127a7cb0eff5d7639b8ba1d916fd66af6a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list