Sandro Bonazzola has posted comments on this change. Change subject: packaging: setup: iSCSI support ......................................................................
Patch Set 11: (7 comments) http://gerrit.ovirt.org/#/c/26501/11/src/bin/hosted-engine.in File src/bin/hosted-engine.in: Line 268: echo "Storage type not supported: ${domainType}" Line 269: exit 1 Line 270: fi Line 271: echo "Connecting Storage Server" Line 272: if [ "${domainType}" == "iscsi" ] ; then missing ! here Line 273: ${VDSCOMMAND} connectStorageServer \ Line 274: ${storageType} \ Line 275: ${spUUID} \ Line 276: connection=${storage},iqn=,portal=,user=kvm,password=,id=${connectionUUID},port=,protocol_version=${protocol_version} 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 Done 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? Done 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 good point. removing the whole path searching, iqn is granted to exist by iscsi_get_device call. No need to check again here. 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') ? Done 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? I don't know how multipathing works and I'm not sure if hosted-engine setup must support that too. Can you provide a test case? 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 http://gerrit.ovirt.org/#/c/26501/11/src/plugins/ovirt-hosted-engine-setup/storage/storage.py File src/plugins/ovirt-hosted-engine-setup/storage/storage.py: Line 821: ): Line 822: # vdsmd has been restarted, we need to reconnect in any case. Line 823: self._storageServerConnection() Line 824: if self.domain_exists: Line 825: self.logger.info(_('Connecting Storage Domain')) > Perhaps 'Connected to Storage Domain' Done Line 826: else: Line 827: self.logger.info(_('Creating Storage Domain')) Line 828: self._createStorageDomain() Line 829: elif self.storageType in ( -- 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 <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Greg Padgett <[email protected]> Gerrit-Reviewer: Jiří Moskovčák <[email protected]> Gerrit-Reviewer: Lev Veyde <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
