Change in vdsm[master]: hba: Rescan using SCSI layer

danken at redhat.com danken at redhat.com
Sun Nov 23 20:20:48 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: hba: Rescan using SCSI layer
......................................................................


Patch Set 5: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/34245/5//COMMIT_MSG
Commit Message:

Line 22: This patch restores the missing scsi scan for fc_hosts, enabling discovery of
Line 23: new FC LUNs in the multiapth rescan flow.
Line 24: 
Line 25: Since scsi scan is a blocking operation, we perform the scan using external
Line 26: command line tool, ensuring that vdsm cannot get stuck. This tool can be also
vdsm cannot get stuck; but can scanning of one host block others?
Line 27: useful outside of vdsm.
Line 28: 
Line 29: In the fc-scan tool, we perform the scan in parallel, so blocking on
Line 30: unresponsive fc_host will not prevent discovery of new devices on other


http://gerrit.ovirt.org/#/c/34245/5/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:

Line 163: 
Line 164: EXT_VDSM_TOOL = os.path.join('@BINDIR@', 'vdsm-tool')
Line 165: 
Line 166: EXT_CURL_IMG_WRAP = '@LIBEXECDIR@/curl-img-wrap'
Line 167: EXT_FC_SCAN = '@LIBEXECDIR@/fc-scan'
Please use CommandPath or just the explicit path to your libexec. This constants module is bad since it exposes entities that should better be kept private to another module/subsystem.


http://gerrit.ovirt.org/#/c/34245/5/vdsm/storage/fc-scan
File vdsm/storage/fc-scan:

Line 66:             log.debug("Scanning %s", path)
Line 67:             start = time.time()
Line 68:             fd = os.open(path, os.O_WRONLY)
Line 69:             try:
Line 70:                 os.write(fd, "- - -")
If I recall correctly, we have seen this write causing the process to get into the D state, which means that concurrent threads for other hosts would block (possibly forever). Am I wrong?
Line 71:             finally:
Line 72:                 os.close(fd)
Line 73:             self.succeeded = True
Line 74:             elapsed = time.time() - start


Line 100:         sys.exit(0)
Line 101: 
Line 102:     scans = []
Line 103: 
Line 104:     for host in hosts:
have you considered using storage.misc.tmap()? there's a wide overlap with the suggested code.
Line 105:         s = Scan(host)
Line 106:         s.start()
Line 107:         scans.append(s)
Line 108: 


-- 
To view, visit http://gerrit.ovirt.org/34245
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7deb3047f1b75c4c65d59602b908835515290993
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer 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: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi 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