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 <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Vered Volansky <[email protected]> Gerrit-Reviewer: Yoav Kleinberger <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
