Yaniv Bronhaim has posted comments on this change.
Change subject: Fix problems with current implementation of forceIscsiRescan ......................................................................
Patch Set 1: (3 inline comments)
.................................................... File vdsm/storage/hsm.py Line 292: self.__validateLvmLockingType() Line 293: Line 294: def storageRefresh(): Line 295: lvm._lvminfo.bootstrap() Line 296: iscsiRescanTimeout = config.getint('irs', 'scsi_rescan_timeout') please explain somewhere where it is O.K to read from config and where it is not. Line 297: if (iscsiRescanTimeout < 0): Line 298: self.log.warning("scsi_rescan_timeout as an invalid value, " Line 299: "using the default: %d", Line 300: sdCache.iscsiRescanTimeout)
.................................................... File vdsm/storage/iscsi.py Line 395: t = threading.Thread(target=collectSlowProcs, args=(procs,)) Line 396: t.setDaemon(True) Line 397: t.start() Line 398: Line 399: I'll think about infra\misc function that can help here.. Line 400: def devIsiSCSI(dev): Line 401: hostdir = os.path.realpath(os.path.join("/sys/block", dev, "device/../../..")) Line 402: host = os.path.basename(hostdir) Line 403: iscsi_host = os.path.join(hostdir, constants.STRG_ISCSI_HOST, host)
.................................................... File vdsm/storage/sdc.py Line 63: self.__proxyCache = {} Line 64: self.__domainCache = {} Line 65: self.storage_repo = storage_repo Line 66: self.storageStale = True Line 67: self.iscsiRescanTimeout = 30 i don't understand if this timeout is the same timeout that hsm.storageRefresh needs to have...? if it is the same, why don't you use in both cases the same parameter? it can leads to conflict between the values
and i prefer hardcoded values to be constants.. its easier to read and change them Line 68: Line 69: def invalidateStorage(self): Line 70: self.storageStale = True Line 71: lvm.invalidateCache()
-- To view, visit http://gerrit.ovirt.org/8172 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ic4e7173086ba15c7706206c5ee1473ed6d334f9e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Barak Azulay bazulay@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com