Yedidyah Bar David has posted comments on this change. Change subject: packaging: setup: iSCSI support ......................................................................
Patch Set 11: Code-Review+1 (5 comments) Seems ok, to the small extent I understand this code and the vdsm calls. Minor comments inside. http://gerrit.ovirt.org/#/c/26501/11/src/bin/hosted-engine.in File src/bin/hosted-engine.in: Line 277: else Line 278: ${VDSCOMMAND} connectStorageServer \ Line 279: ${storageType} \ Line 280: ${spUUID} \ Line 281: connection=${storage},iqn=${iqn},portal=${portal},user=${user},password=${password},id=${connectionUUID},port=${port} quotes? especially for password Line 282: fi Line 283: } Line 284: Line 285: cmd_start_pool() { http://gerrit.ovirt.org/#/c/26501/11/src/plugins/ovirt-hosted-engine-setup/storage/iscsi.py File src/plugins/ovirt-hosted-engine-setup/storage/iscsi.py: Line 231: ] Line 232: ) Line 233: if res['status']['code'] != 0: Line 234: raise RuntimeError(devices['status']['message']) Line 235: retry -= 1 Perhaps add some delay here? Line 236: return iscsi_device Line 237: Line 238: def _validate_domain(self, target): Line 239: device = self._iscsi_get_device( Line 249: ) Line 250: path = None Line 251: for path in device['pathlist']: Line 252: if path['iqn'] == target: Line 253: break What do you need 'path' for? You only use path['iqn'] which (if the loop is "correct") == target Line 254: size_mb = int(device['capacity']) / pow(2, 20) Line 255: self.logger.debug( Line 256: 'Available space on {iqn} is {space}Mb'.format( Line 257: iqn=path['iqn'], Line 367: except RuntimeError as e: Line 368: self.logger.debug('exception', exc_info=True) Line 369: self.logger.error(e) Line 370: if not self._interactive: Line 371: raise RuntimeError('Cannot access to iSCSI portal') _('Cannot access iSCSI portal') ? Line 372: self.environment[ohostedcons.StorageEnv.ISCSI_IP_ADDR] = address Line 373: self.environment[ohostedcons.StorageEnv.ISCSI_PORT] = port Line 374: self.environment[ohostedcons.StorageEnv.ISCSI_USER] = user Line 375: self.environment[ohostedcons.StorageEnv.ISCSI_PASSWORD] = password Line 434: ]: Line 435: self.environment[ Line 436: ohostedcons.StorageEnv.ISCSI_PORTAL Line 437: ] = path['portal'] Line 438: break Did you test this with multipathing? How will it work then? Line 439: except (ValueError, KeyError) as e: Line 440: self.logger.debug('exception', exc_info=True) Line 441: self.logger.error(_('Cannot detect iSCSI portal')) Line 442: raise e -- To view, visit http://gerrit.ovirt.org/26501 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ide79a130b730d95d3545dcfb80b1b01dfeaf2b12 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-hosted-engine-setup Gerrit-Branch: master Gerrit-Owner: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Jiří Moskovčák <jmosk...@redhat.com> Gerrit-Reviewer: Lev Veyde <lve...@gmail.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
