Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA-142595: Non-block devices causing IndexError #203

Closed
wants to merge 3 commits into from

Conversation

wangyanbn
Copy link
Contributor

Some device (e.g. a scsi security manager device.) has no subdir
"block/" , this will cause _extract_dev_name "IndexError: list index
out of range" error.

The following is the traceback info:
run this cmd: xe sr-probe type=lvmohba
There was an SR backend failure.
status: non-zero exit
stdout:
stderr: Traceback (most recent call last):
File "/opt/xensource/sm/LVMoHBASR", line 239, in ?
SRCommand.run(LVHDoHBASR, DRIVER_INFO)
File "/opt/xensource/sm/SRCommand.py", line 343, in run
sr = driver(cmd, cmd.sr_uuid)
File "/opt/xensource/sm/SR.py", line 139, in init
self.load(sr_uuid)
File "/opt/xensource/sm/LVMoHBASR", line 98, in load
print >>sys.stderr,self.hbasr.print_devs()
File "/opt/xensource/sm/HBASR.py", line 242, in print_devs
self._init_hbadict()
File "/opt/xensource/sm/HBASR.py", line 63, in _init_hbadict
dict = devscan.adapters(filterstr=self.type)
File "/opt/xensource/sm/devscan.py", line 75, in adapters
(dev, entry) = _extract_dev(dir, proc, id, lun)
File "/opt/xensource/sm/devscan.py", line 235, in _extract_dev
dev = _extract_dev_name(device_dir)
File "/opt/xensource/sm/devscan.py", line 226, in _extract_dev_name
dev = glob.glob(os.path.join(device_dir, 'block/*'))[0]
IndexError: list index out of range

Signed-off-by: wangyanbn wangyanbin@dayang.com.cn

Fix _extract_dev_name(device_dir) bug:
some device dir has no subdir "block/" , this will cause "IndexError: list index out of range".
In my test environment, LUN 0 has no "block/" subdir, but other LUNs are OK.
@xen-git
Copy link

xen-git commented Aug 8, 2014

Can one of the admins verify this patch?

1 similar comment
@xen-git
Copy link

xen-git commented Aug 8, 2014

Can one of the admins verify this patch?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) when pulling 7cd0e74 on wangyanbn:master into bce4501 on xapi-project:master.

@matelakat
Copy link
Contributor

@wangyanbn - could you cover the new code with unit tests?

@wangyanbn
Copy link
Contributor Author

@matelakat , the current unit tests can cover the normal code execute path, but not the special path.
To cover all the new added code, we may need modify the test_devscan.py and testlib.py files.
I am not familiar with sm module and python , so it may be difficult for me. But I will try it at next week if I have time.
Thanks for you quick response!

@germanop
Copy link
Contributor

germanop commented Aug 8, 2014

@matelakat , providing unit tests is a plus. Especially when the code is touching internal parts of a function,
the unit test should be already there, covering the function itself.
@wangyanbn, what kernel are you using? Can you make an example of a block device that is
not in /sys/block?

@wangyanbn
Copy link
Contributor Author

@matelakat and @germanop , I am using Creedence alpha4. Today I found that special LUN is a security manager device, not a block device. It is in the "/sys/class/fc_transport" dir and has no 'block' subdir :
[root@DVP1 ~]# ls /sys/class/fc_transport/target7:0:0/device/|grep 7:
7:0:0:0
7:0:0:11
7:0:0:12
7:0:0:2
7:0:0:22
7:0:0:33
7:0:0:44
[root@DVP1 ~]# ls /sys/class/fc_transport/target7:0:0/device/7:0:0:0/block
ls: /sys/class/fc_transport/target7:0:0/device/7:0:0:0/block: No such file or directory
[root@DVP1 ~]# ls /sys/class/fc_transport/target7:0:0/device/7:0:0:11/block
sde
[root@DVP1 ~]# cat /sys/class/fc_transport/target7:0:0/device/7:0:0:0/type
13
[root@DVP1 ~]# cat /sys/class/fc_transport/target7:0:0/device/7:0:0:11/type
0
[root@DVP1 ~]# ls /sys/class/scsi_disk|grep 7:
7:0:0:11
7:0:0:12
7:0:0:2
7:0:0:22
7:0:0:33
7:0:0:44
[root@DVP1 ~]# uname -a
Linux DVP1 3.10.0+1 #1 SMP Mon Jun 30 06:56:24 EDT 2014 x86_64 x86_64 x86_64 GNU/Linux
ps: about SCSI Peripheral Device Type
00h Direct-access block device (e.g., magnetic disk)
13h Security manager device
see http://en.wikipedia.org/wiki/SCSI_Peripheral_Device_Type

@matelakat
Copy link
Contributor

@wangyanbn - Thanks for the info. Is there any chance that you have the traceback with the IndexError in it? If so, could you please add it as a comment here? Thanks.

@matelakat
Copy link
Contributor

@wangyanbn - regarding to the unit tests, let me help you by doing some refactoring on the framework, so you can easily unit test the changes. Will get back to you soon.

@wangyanbn
Copy link
Contributor Author

@matelakat , here is the traceback info;
[root@DVP1 /]# xe sr-probe type=lvmohba
There was an SR backend failure.
status: non-zero exit
stdout:
stderr: Traceback (most recent call last):
File "/opt/xensource/sm/LVMoHBASR", line 239, in ?
SRCommand.run(LVHDoHBASR, DRIVER_INFO)
File "/opt/xensource/sm/SRCommand.py", line 343, in run
sr = driver(cmd, cmd.sr_uuid)
File "/opt/xensource/sm/SR.py", line 139, in init
self.load(sr_uuid)
File "/opt/xensource/sm/LVMoHBASR", line 98, in load
print >>sys.stderr,self.hbasr.print_devs()
File "/opt/xensource/sm/HBASR.py", line 242, in print_devs
self._init_hbadict()
File "/opt/xensource/sm/HBASR.py", line 63, in _init_hbadict
dict = devscan.adapters(filterstr=self.type)
File "/opt/xensource/sm/devscan.py", line 75, in adapters
(dev, entry) = _extract_dev(dir, proc, id, lun)
File "/opt/xensource/sm/devscan.py", line 235, in _extract_dev
dev = _extract_dev_name(device_dir)
File "/opt/xensource/sm/devscan.py", line 226, in _extract_dev_name
dev = glob.glob(os.path.join(device_dir, 'block/*'))[0]
IndexError: list index out of range

@matelakat
Copy link
Contributor

@wangyanbn - thanks for your work, again. I prepared the changes that will let us unit test your changes, see them in #209 and #208 Other than that, could you please amend your commit for me:

  • refer to our internal ticket: CA-142595
  • please add the Signed-off-by tag to your commit message.
  • could you please make sure that the first line of your commit contains a reference in our system, and the whole message looks something like this:
CA-142595: Non-block devices causing IndexError

...  your description here, lines wrapped at 72 chars

Signed-off-by: ... your details ...

Thank you for your cooperation and good work, if you need some guidance on how to write the actual test cases, let me know.

@wangyanbn wangyanbn changed the title Update devscan.py CA-142595: Non-block devices causing IndexError Aug 18, 2014
Some device (e.g. a scsi security manager device.) has no subdir
"block/" , this will cause _extract_dev_name "IndexError: list index
out of range" error.
The following is the traceback info:
run this cmd: xe sr-probe type=lvmohba
There was an SR backend failure.
status: non-zero exit
stdout:
stderr: Traceback (most recent call last):
File "/opt/xensource/sm/LVMoHBASR", line 239, in ?
SRCommand.run(LVHDoHBASR, DRIVER_INFO)
File "/opt/xensource/sm/SRCommand.py", line 343, in run
sr = driver(cmd, cmd.sr_uuid)
File "/opt/xensource/sm/SR.py", line 139, in init
self.load(sr_uuid)
File "/opt/xensource/sm/LVMoHBASR", line 98, in load
print >>sys.stderr,self.hbasr.print_devs()
File "/opt/xensource/sm/HBASR.py", line 242, in print_devs
self._init_hbadict()
File "/opt/xensource/sm/HBASR.py", line 63, in _init_hbadict
dict = devscan.adapters(filterstr=self.type)
File "/opt/xensource/sm/devscan.py", line 75, in adapters
(dev, entry) = _extract_dev(dir, proc, id, lun)
File "/opt/xensource/sm/devscan.py", line 235, in _extract_dev
dev = _extract_dev_name(device_dir)
File "/opt/xensource/sm/devscan.py", line 226, in _extract_dev_name
dev = glob.glob(os.path.join(device_dir, 'block/*'))[0]
IndexError: list index out of range

Signed-off-by: Wang Yanbin wangyanbin@dayang.com.cn
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) when pulling e2ce98a on wangyanbn:master into bce4501 on xapi-project:master.

@wangyanbn
Copy link
Contributor Author

@matelakat , I have amended the commit and pushed into the repository. It became a new commit , and I must make a merge before push it. Is it right?

@fquesnel
Copy link

@wangyanbn , we are currently reviewing our code to check that there is no side effect.
Hopefully, we will be able to merge your pull request soon.
Thank you for your contribution.

@matelakat
Copy link
Contributor

@wangyanbn - Thank you for your contribution, your changes have been merged to xs64bit via 41aeac9
At some point we'll merge it to master as well.

@matelakat matelakat closed this Sep 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants