Bug 1312270

Description of problem:
get_size_for_disk_name() of python-rtslib in RHEL7.2 can not handle 10+ over number of partitions and it causes targetcli calltrace.

Version-Release number of selected component (if applicable):
python-rtslib-2.1.fb57-3.el7.noarch

How reproducible:
Always

Steps to Reproduce:
1. Make partitions which its name is sda10 or more
2. Create backstores as block with sda10
3. ls in /backstores/block

Actual results:
targetcli causes following backtrace.
~~~
utils.py:90:fread:IOError: [Errno 2] No such file or directory: '/sys/block/sda1/sda10/size'

Traceback (most recent call last):
  File "/bin/targetcli", line 121, in <module>
    main()
  File "/bin/targetcli", line 111, in main
    shell.run_interactive()
  File "/usr/lib/python2.7/site-packages/configshell_fb/shell.py", line 894, in run_interactive
    self._cli_loop()
  File "/usr/lib/python2.7/site-packages/configshell_fb/shell.py", line 723, in _cli_loop
    self.run_cmdline(cmdline)
  File "/usr/lib/python2.7/site-packages/configshell_fb/shell.py", line 837, in run_cmdline
    self._execute_command(path, command, pparams, kparams)
  File "/usr/lib/python2.7/site-packages/configshell_fb/shell.py", line 812, in _execute_command
    result = target.execute_command(command, pparams, kparams)
  File "/usr/lib/python2.7/site-packages/configshell_fb/node.py", line 1411, in execute_command
    return method(*pparams, **kparams)
  File "/usr/lib/python2.7/site-packages/configshell_fb/node.py", line 710, in ui_command_ls
    tree = self._render_tree(target, depth=depth)
  File "/usr/lib/python2.7/site-packages/configshell_fb/node.py", line 861, in _render_tree
    + self._render_tree(children[i], margin, depth)
  File "/usr/lib/python2.7/site-packages/configshell_fb/node.py", line 765, in _render_tree
    (description, is_healthy) = root.summary()
  File "/usr/lib/python2.7/site-packages/targetcli/ui_backstore.py", line 540, in summary
    return ("%s (%s) %s%s %s" % (so.udev_path, bytes_to_human(so.size),
  File "/usr/lib/python2.7/site-packages/rtslib_fb/tcm.py", line 703, in _get_size
    return get_size_for_disk_name(self._parse_info('device')) * int(self._parse_info('SectorSize'))
  File "/usr/lib/python2.7/site-packages/rtslib_fb/utils.py", line 162, in get_size_for_disk_name
    return get_size("/sys/block/%s/%s" % (disk, m.group()), True)
  File "/usr/lib/python2.7/site-packages/rtslib_fb/utils.py", line 141, in get_size
    sect_size = int(fread("%s/size" % path))
  File "/usr/lib/python2.7/site-packages/rtslib_fb/utils.py", line 90, in fread
    with open(path, 'r') as file_fd:
IOError: [Errno 2] No such file or directory: '/sys/block/sda1/sda10/size'

Local variables in innermost frame:
path: '/sys/block/sda1/sda10/size'
~~~

Expected results:
targetcli properly handle the comand and device file and does not crash.

Additional info:
I checked targetcli with pdb 

~~~
(Pdb) bt
  /usr/lib64/python2.7/pdb.py(1314)main()
-> pdb._runscript(mainpyfile)
  /usr/lib64/python2.7/pdb.py(1233)_runscript()
-> self.run(statement)
  /usr/lib64/python2.7/bdb.py(400)run()
-> exec cmd in globals, locals
  <string>(1)<module>()
  /usr/bin/targetcli(19)<module>()
-> '''
  /usr/bin/targetcli(110)main()
-> shell.run_interactive()
  /usr/lib/python2.7/site-packages/configshell_fb/shell.py(894)run_interactive()
-> self._cli_loop()
  /usr/lib/python2.7/site-packages/configshell_fb/shell.py(723)_cli_loop()
-> self.run_cmdline(cmdline)
  /usr/lib/python2.7/site-packages/configshell_fb/shell.py(837)run_cmdline()
-> self._execute_command(path, command, pparams, kparams)
  /usr/lib/python2.7/site-packages/configshell_fb/shell.py(812)_execute_command()
-> result = target.execute_command(command, pparams, kparams)
  /usr/lib/python2.7/site-packages/configshell_fb/node.py(1411)execute_command()
-> return method(*pparams, **kparams)
  /usr/lib/python2.7/site-packages/configshell_fb/node.py(710)ui_command_ls()
-> tree = self._render_tree(target, depth=depth)
  /usr/lib/python2.7/site-packages/configshell_fb/node.py(861)_render_tree()
-> + self._render_tree(children[i], margin, depth)
  /usr/lib/python2.7/site-packages/configshell_fb/node.py(765)_render_tree()
-> (description, is_healthy) = root.summary()
  /usr/lib/python2.7/site-packages/targetcli/ui_backstore.py(540)summary()
-> return ("%s (%s) %s%s %s" % (so.udev_path, bytes_to_human(so.size),
  /usr/lib/python2.7/site-packages/rtslib_fb/tcm.py(703)_get_size()
-> return get_size_for_disk_name(self._parse_info('device')) * int(self._parse_info('SectorSize'))
  /usr/lib/python2.7/site-packages/rtslib_fb/utils.py(162)get_size_for_disk_name()
-> return get_size("/sys/block/%s/%s" % (disk, m.group()), True)
  /usr/lib/python2.7/site-packages/rtslib_fb/utils.py(141)get_size()
-> sect_size = int(fread("%s/size" % path))
> /usr/lib/python2.7/site-packages/rtslib_fb/utils.py(90)fread()
-> with open(path, 'r') as file_fd:
~~~

~~~
(Pdb) up
> /usr/lib/python2.7/site-packages/rtslib_fb/utils.py(162)get_size_for_disk_name()
-> return get_size("/sys/block/%s/%s" % (disk, m.group()), True)
(Pdb) p disk
'sda1'
(Pdb) p m.groups
~~~
At this line, disk parameter is expected as 'sda1' but actual result is 'sda10'

~~~
def get_size_for_disk_name(name):
    '''
    @param name: a kernel disk name, as found in /proc/partitions
    @type name: string
    @return: The size in logical blocks of a disk-type block device.
    '''

    # size is in 512-byte sectors, we want to return number of logical blocks
    def get_size(path, is_partition=False):
        sect_size = int(fread("%s/size" % path))
        if is_partition:
            path = os.path.split(path)[0]
        logical_block_size = int(fread("%s/queue/logical_block_size" % path))
        return sect_size / (logical_block_size / 512)

    # Disk names can include '/' (e.g. 'cciss/c0d0') but these are changed to
    # '!' when listed in /sys/block.
    name = name.replace("/", "!")

    try:
        return get_size("/sys/block/%s" % name)
    except IOError:
        # Maybe it's a partition?
        m = re.search(r'^([a-z0-9_\-!]+)(\d+)$', name)
        if m:
            # If disk name ends with a digit, Linux sticks a 'p' between it and
            # the partition number in the blockdev name.
            disk = m.groups()[0]
            if disk[-1] == 'p' and disk[-2].isdigit():
                disk = disk[:-1]
            return get_size("/sys/block/%s/%s" % (disk, m.group()), True)
        else:
            raise
~~~

re.search() line causes this issue.
~~~
(Pdb) p m.groups()
('sda1', '0')
(Pdb) p m.group(0)
'sda10'
~~~

I checked upstream code and find the fix for this issue.

~~~
commit 290738c20213a9fabff1d0a0d8eb726706473aad
Author: Andy Grover <agrover>
Date:   Fri Nov 6 09:13:35 2015 -0800

    Fix regex in get_size_for_disk_name
    
    Would break with 'sda10'. We need to non-greedily match the first subgroup.
    
    Signed-off-by: Andy Grover <agrover>

diff --git a/rtslib/utils.py b/rtslib/utils.py
index 5531e8f..79a4d9d 100644
--- a/rtslib/utils.py
+++ b/rtslib/utils.py
@@ -152,7 +152,7 @@ def get_size_for_disk_name(name):
         return get_size("/sys/block/%s" % name)
     except IOError:
         # Maybe it's a partition?
-        m = re.search(r'^([a-z0-9_\-!]+)(\d+)$', name)
+        m = re.search(r'^([a-z0-9_\-!]+?)(\d+)$', name)
         if m:
             # If disk name ends with a digit, Linux sticks a 'p' between it and
             # the partition number in the blockdev name.
~~~

Could you merge the fix into RHEL7?

Regards,
Keigo