Change in vdsm[master]: lvm: Add convertLV function in storage.lvm module

nsoffer at redhat.com nsoffer at redhat.com
Wed Mar 11 08:56:43 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: lvm: Add convertLV function in storage.lvm module
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/38557/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 818:     if rc != 0 and len(out) < 1:
Line 819:         raise se.StorageException("%d %s %s\n%s/%s" % (rc, out, err, vg, lvs))
Line 820: 
Line 821: 
Line 822: def convertLV(attrs):
Please use only kwargs for options, no raw values.
Line 823:     """
Line 824:     Convert a logical volume from linear to mirror or snapshot 
Line 825: 
Line 826:     attrs: an iterable of (attr, value) pairs),


Line 833:         cmd.extend(attr)
Line 834:     else:
Line 835:         # (("--aa", "v1"), ("--ab", "v2"))
Line 836:         for attr in attrs:
Line 837:             cmd.extend(attr)
And no multiple arguments types. This duplicate horrible code that I want to kill.
Line 838:     rc, out, err = _lvminfo.cmd(tuple(cmd))
Line 839:     if rc != 0 and len(out) < 1:
Line 840:         raise se.StorageException("%d %s %s" % (rc, out, err))
Line 841: 


Line 834:     else:
Line 835:         # (("--aa", "v1"), ("--ab", "v2"))
Line 836:         for attr in attrs:
Line 837:             cmd.extend(attr)
Line 838:     rc, out, err = _lvminfo.cmd(tuple(cmd))
Why convert cmd to tuple? the caller should not care about the type.
Line 839:     if rc != 0 and len(out) < 1:
Line 840:         raise se.StorageException("%d %s %s" % (rc, out, err))
Line 841: 
Line 842: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I80fe74ed9e61f58f20bd60be46109cce9b06a69c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list