Change in vdsm[master]: tests: new functional tests for vdsm storage
nsoffer at redhat.com
nsoffer at redhat.com
Mon Sep 8 19:33:26 UTC 2014
Nir Soffer has posted comments on this change.
Change subject: tests: new functional tests for vdsm storage
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.ovirt.org/#/c/32496/4/tests/functional/testlib/storagebackends/iscsi.py
File tests/functional/testlib/storagebackends/iscsi.py:
Line 63: self._testDirectory = tempfile.mkdtemp()
Line 64: self._storageFile = os.path.join(self._testDirectory, 'testfile')
Line 65: self._fileioBackstore = self.randomName('backfile')
Line 66: logging.info('using %s, %s' % (self._fileioBackstore, self._storageFile))
Line 67: self._targetcli('/backstores/fileio create %s %s 10G' % (self._fileioBackstore, self._storageFile))
We should separate the server driver from the backend, so it will be easier to create the version using tgtd, that we need for testing on el6.
Line 68: self._targetcli('/iscsi create %s' % self._iqn)
Line 69: self._targetcli('/iscsi/%s/tpg1/luns create /backstores/fileio/%s' % (self._iqn, self._fileioBackstore))
Line 70: self._targetcli('/iscsi/%s/tpg1 set attribute authentication=0 demo_mode_write_protect=0' % self._iqn)
Line 71: self._targetcli('/iscsi/%s/tpg1 set attribute generate_node_acls=1 cache_dynamic_acls=1' % self._iqn)
Line 65: self._fileioBackstore = self.randomName('backfile')
Line 66: logging.info('using %s, %s' % (self._fileioBackstore, self._storageFile))
Line 67: self._targetcli('/backstores/fileio create %s %s 10G' % (self._fileioBackstore, self._storageFile))
Line 68: self._targetcli('/iscsi create %s' % self._iqn)
Line 69: self._targetcli('/iscsi/%s/tpg1/luns create /backstores/fileio/%s' % (self._iqn, self._fileioBackstore))
This is unreadable, specially in gerrit.
This should be:
self.driver.create_lun(self._iqn, self.tpg, self._disk)
The targetcli driver should make a command line from the arguments and run it:
def create_lun(self, iqn, tgp, disk):
path = '/iscsi/%s/%s' % (iqn, tpg)
backstore = 'backstores/fileio/%s' % disk
self._run(path, 'create', backstore)
Using strings does not make code more readable. Annoying lists are actually better for readability when command lines becomes long.
Line 70: self._targetcli('/iscsi/%s/tpg1 set attribute authentication=0 demo_mode_write_protect=0' % self._iqn)
Line 71: self._targetcli('/iscsi/%s/tpg1 set attribute generate_node_acls=1 cache_dynamic_acls=1' % self._iqn)
Line 72: return self, Verify(self._iqn, self._volumeGroup, self.vdsm, self._volumeID)
Line 73:
--
To view, visit http://gerrit.ovirt.org/32496
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1703e7c1dc223ff707775865cd14c7dd62314caf
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yoav Kleinberger <ykleinbe at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: Yoav Kleinberger <ykleinbe 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