Saggi Mizrahi has uploaded a new change for review.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Rescan FC when rescanning for new multipath devices
Change-Id: Idec939222676a24452e8825b36db68839bfd2bbc Signed-off-by: Saggi Mizrahi smizrahi@redhat.com --- M lib/vdsm/utils.py M tests/utilsTests.py M vdsm/storage/Makefile.am A vdsm/storage/fc.py M vdsm/storage/multipath.py 5 files changed, 61 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/19539/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index f06cad2..e273514 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -793,6 +793,24 @@ return functools.partial(self.__call__, obj)
+def tee_async(data, outputFiles=[], append=False): + cmd = ["tee"] + if append: + cmd.append("--append") + + cmd.extend(outputFiles) + proc = execCmd(cmd, sync=False) + proc.stdin.write(data) + + def parser(rc, out, err): + if rc != 0: + raise OSError(-1, err) + + return out + + return AsyncProcessOperation(proc, parser) + + def validateMinimalKeySet(dictionary, reqParams): if not all(key in dictionary for key in reqParams): raise ValueError diff --git a/tests/utilsTests.py b/tests/utilsTests.py index 656c121..5b83ec0 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -18,11 +18,14 @@ # Refer to the README and COPYING files for full details of the license # import errno +import time +import tempfile +import shutil +import os
from testrunner import VdsmTestCase as TestCaseBase from vdsm import utils from storage import misc -import time
class RetryTests(TestCaseBase): @@ -63,6 +66,33 @@ sproc.wait()
+class TeeTests(TestCaseBase): + def setUp(self): + self.dir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.dir) + + def test(self): + d = self.dir + n = 10 + files = [] + for i in range(n): + files.append(os.path.join(d, "%d.tmp" % i)) + + data = "hello" + res = utils.tee_async(data, files).result() + self.assertEquals((data, None), res) + for fname in files: + with open(fname, "r") as f: + self.assertEquals(data, f.read()) + + def testError(self): + res, err = utils.tee_async("hello", ["/"]).result() + self.assertEquals(res, None) + self.assertNotEquals(err, None) + + class CommandPathTests(TestCaseBase): def testExisting(self): cp = utils.CommandPath('sh', 'utter nonsense', '/bin/sh') diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am index 2d3e9a6..a367c9b 100644 --- a/vdsm/storage/Makefile.am +++ b/vdsm/storage/Makefile.am @@ -33,6 +33,7 @@ fileSD.py \ fileUtils.py \ fileVolume.py \ + fc.py \ fuser.py \ glusterSD.py \ glusterVolume.py \ diff --git a/vdsm/storage/fc.py b/vdsm/storage/fc.py new file mode 100644 index 0000000..96aa710 --- /dev/null +++ b/vdsm/storage/fc.py @@ -0,0 +1,8 @@ +from glob import glob +from vdsm import utils + + +def rescan(): + """Issue command to discover new ports if the fc driver supports it""" + # Use tee to make sure we don't do that IO directly + utils.tee_async("1", glob("/sys/class/fc_host/host*/issue_lip")) diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py index 6d6fcbd..076bfca 100644 --- a/vdsm/storage/multipath.py +++ b/vdsm/storage/multipath.py @@ -36,6 +36,7 @@ import iscsi import supervdsm import devicemapper +import fc
import storage_exception as se
@@ -106,7 +107,8 @@ Should only be called from hsm._rescanDevices() """
- # First ask iSCSI to rescan all its sessions + # First ask fc and iSCSI to rescan all of their sessions + fc.rescan() iscsi.rescan()
# Now let multipath daemon pick up new devices
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4608/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4530/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3723/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4613/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4535/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3728/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4618/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4540/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3733/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3796/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4681/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4605/ : SUCCESS
Yaniv Bronhaim has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 4: Code-Review-1
guess it should be rebased over http://gerrit.ovirt.org/#/c/19254/
Yaniv Bronhaim has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 4: -Code-Review
sorry, didn't follow it right
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4723/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4799/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3914/ : SUCCESS
Ayal Baron has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 5:
(2 comments)
.................................................... File lib/vdsm/utils.py Line 798: if append: Line 799: cmd.append("--append") Line 800: Line 801: cmd.extend(outputFiles) Line 802: proc = execCmd(cmd, sync=False) I thought you disapprove of direct usage of execCmd? Line 803: proc.stdin.write(data) Line 804: Line 805: def parser(rc, out, err): Line 806: if rc != 0:
.................................................... File vdsm/storage/multipath.py Line 107: Should only be called from hsm._rescanDevices() Line 108: """ Line 109: Line 110: # First ask fc and iSCSI to rescan all of their sessions Line 111: fc.rescan() I thought the premise of the previous patch is that iscsiadm -m session -R already does this? Line 112: iscsi.rescan() Line 113: Line 114: # Now let multipath daemon pick up new devices Line 115:
Saggi Mizrahi has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 5:
(2 comments)
.................................................... File lib/vdsm/utils.py Line 798: if append: Line 799: cmd.append("--append") Line 800: Line 801: cmd.extend(outputFiles) Line 802: proc = execCmd(cmd, sync=False) This is *the* wrapper of tee. So here it's fine. As to splitting up execCmd(), this will happen in the near future. Line 803: proc.stdin.write(data) Line 804: Line 805: def parser(rc, out, err): Line 806: if rc != 0:
.................................................... File vdsm/storage/multipath.py Line 107: Should only be called from hsm._rescanDevices() Line 108: """ Line 109: Line 110: # First ask fc and iSCSI to rescan all of their sessions Line 111: fc.rescan() No, it does the iscsi rescan not fc lip reset. It makes sense when you realize that iscsiadm is the iscsi administration tool and not the fc administration tool. Line 112: iscsi.rescan() Line 113: Line 114: # Now let multipath daemon pick up new devices Line 115:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 6: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4763/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4839/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3954/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_install_rpm_sanity_gerrit/143/ : FAILURE
Ayal Baron has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 5:
(1 comment)
.................................................... File vdsm/storage/multipath.py Line 107: Should only be called from hsm._rescanDevices() Line 108: """ Line 109: Line 110: # First ask fc and iSCSI to rescan all of their sessions Line 111: fc.rescan() in this case this patch set will probably cause a regression as I believe it is missing rescan of all non fc / iscsi scsi devices.
Also, issuing a lip will cause all I/O to be suspended which "- - -" avoids iirc and is the reason we avoided lip to begin with
Possibly we should just replace all of this with /usr/bin/rescan-scsi-bus.sh and differentiate between flows (i.e. not rescan everything every time, but have different calls depending on use case). Line 112: iscsi.rescan() Line 113: Line 114: # Now let multipath daemon pick up new devices Line 115:
Ayal Baron has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 6: Code-Review-1
Saggi Mizrahi has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 5:
(1 comment)
.................................................... File vdsm/storage/multipath.py Line 107: Should only be called from hsm._rescanDevices() Line 108: """ Line 109: Line 110: # First ask fc and iSCSI to rescan all of their sessions Line 111: fc.rescan() As I said "- - -" and issue_lip are two different things. The former is for iscsi the latter is for fc.
Also fc.rescan is completely async so it wouldn't block iscsi rescans.
doing issue_lip will suspend all IO on the HBA since it causes a bus reset. The suspend doesn't effect all IO in the system. Which means that paths that don't use FC are unaffected.
I agree it's problematic but we call multipath.rescan() in only 3 places.
1) sdc.refreshStorage() 2) scanDevicesVisibility 3) formatStorageDomain
In all those cases we do want to be 100% sure that our view of the storage topology is up to date.
We could move to a "rescan on miss" strategy for 2 and 3. I don't know how much getDeviceList() and sdc.refreshStorage() is hammered though.
In any case, I agree that this needs further discussion. ---- I disagree with a move to rescan-scsi-bus.sh since it was meant to be used by human administrators and has different priorities and goals from our own. If we keep things in VDSM it'll be much easier to optimize and synchronize when necessary. Line 112: iscsi.rescan() Line 113: Line 114: # Now let multipath daemon pick up new devices Line 115:
Ayal Baron has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 5:
(1 comment)
.................................................... File vdsm/storage/multipath.py Line 107: Should only be called from hsm._rescanDevices() Line 108: """ Line 109: Line 110: # First ask fc and iSCSI to rescan all of their sessions Line 111: fc.rescan() rescan on miss is definitely warranted in this situation. of course the i/o is suspended only on these paths. These paths may be used by running VMs and issuing the lip is quite problematic here. refreshStorage is actually the place that worries me the most for this operation since it is used in too many flows and is the last place I would like to have it. Line 112: iscsi.rescan() Line 113: Line 114: # Now let multipath daemon pick up new devices Line 115:
Itamar Heim has posted comments on this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Patch Set 6:
ping
Itamar Heim has abandoned this change.
Change subject: Rescan FC when rescanning for new multipath devices ......................................................................
Abandoned
no activity, restore if relevant
vdsm-patches@lists.fedorahosted.org