Change in vdsm[master]: client: add the domainsMap in connectStoragePool

nsoffer at redhat.com nsoffer at redhat.com
Tue Feb 18 17:21:54 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: client: add the domainsMap in connectStoragePool
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/24491/2/client/vdsClient.py
File client/vdsClient.py:

Line 878:         spUUID = args[0]
Line 879:         ID = int(args[1])
Line 880:         scsi_key = args[2]
Line 881:         master = args[3] if len(args) > 3 else BLANK_UUID
Line 882:         master_ver = int(args[4]) if len(args) > 4 else -1
The previous style is more consistent and flexible, since we use the same style for any option, and we can easily add more code when needed, while the new style support only case where no processing or very little processing is needed.

But which style is better is not the topic of this change, but adding new optional parameter. Why not keep the old code and just add the new 4 lines needed for the new parameter, and refactor it later in another patch?
Line 883: 
Line 884:         if len(args) > 5:
Line 885:             domainsMap = self._parseDomainsMap(args[5])
Line 886:             pool = self.s.connectStoragePool(


Line 887:                 spUUID, ID, scsi_key, master, master_ver, domainsMap)
Line 888:         else:
Line 889:             pool = self.s.connectStoragePool(
Line 890:                 spUUID, ID, scsi_key, master, master_ver)
Line 891: 
The old code was better, first setting the options, then one line creating the pool.

    if len(args) > 5:
        domainsMap = self._parseDomainsMap(args[5])
    else:
        domainsMap = None

    pool = self.s.connectStoragePool(
        spUUID, ID, scsi_key, master, master_ver, domainsMap)
Line 892:         if pool['status']['code']:
Line 893:             return pool['status']['code'], pool['status']['message']
Line 894: 
Line 895:         return 0, ''


Line 890:                 spUUID, ID, scsi_key, master, master_ver)
Line 891: 
Line 892:         if pool['status']['code']:
Line 893:             return pool['status']['code'], pool['status']['message']
Line 894: 
Another unrelated change.
Line 895:         return 0, ''
Line 896: 
Line 897:     def disconnectStoragePool(self, args):
Line 898:         spUUID = args[0]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c3037456ac045b7ac03043bb2f46ccd48b67045
Gerrit-PatchSet: 2
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: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list