New: [rhinstaller/blivet/pulls/161 master] Don't fail in storage.py:isNameValid when name is None

[dashea]

With ISM RAID-0 configured I get the following error when using blivet:

Traceback (most recent call last):
  File "list-devices.py", line 34, in <module>
    storage.reset()             # detect system storage configuration
  File "/usr/lib/python2.7/site-packages/blivet/__init__.py", line 372, in reset
    self.devicetree.populate(cleanupOnly=cleanupOnly)
  File "/usr/lib/python2.7/site-packages/blivet/devicetree.py", line 2113, in populate
    self._populate()
  File "/usr/lib/python2.7/site-packages/blivet/devicetree.py", line 2179, in _populate
    self.addUdevDevice(dev)
  File "/usr/lib/python2.7/site-packages/blivet/devicetree.py", line 1258, in addUdevDevice
    self.handleUdevDeviceFormat(info, device)
  File "/usr/lib/python2.7/site-packages/blivet/devicetree.py", line 1879, in handleUdevDeviceFormat
    self.handleUdevMDMemberFormat(info, device)
  File "/usr/lib/python2.7/site-packages/blivet/devicetree.py", line 1637, in handleUdevMDMemberFormat
    exists=True)
  File "/usr/lib/python2.7/site-packages/blivet/devices/md.py", line 87, in __init__
    sysfsPath=sysfsPath)
  File "/usr/lib/python2.7/site-packages/blivet/devices/container.py", line 63, in __init__
    super(ContainerDevice, self).__init__(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/blivet/devices/storage.py", line 106, in __init__
    super(StorageDevice, self).__init__(name, parents=parents)
  File "/usr/lib/python2.7/site-packages/blivet/devices/device.py", line 83, in __init__
    if not self.isNameValid(name):
  File "/usr/lib/python2.7/site-packages/blivet/devices/storage.py", line 787, in isNameValid
    if name.startswith("cciss/"):
AttributeError: 'NoneType' object has no attribute 'startswith'

mdadm --examine --brief /dev/sdc
ARRAY metadata=imsm UUID=17ca54e9:88114b9d:2072a87d:cc2c4c61
ARRAY /dev/md/Test0 container=17ca54e9:88114b9d:2072a87d:cc2c4c61 member=0 UUID=11cc7cbd:e42aabbf:d57faea8:baaa6425

md_info: {'MD_LEVEL': 'container', 'MD_UUID': '17ca54e9-8811-4b9d-2072-a87dcc2c4c61', 'MD_DEVICES': '2', 'MD_METADATA': 'imsm'}

Device name was not detected due to some error, but I think that None value should be considered as not valid name anyway.

[master 1/1] Don't fail in storage.py:isNameValid when name is None

[dashea]

From: Alexander Lakhin

---
 blivet/devices/storage.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py
index cf590cf..ac74853 100644
--- a/blivet/devices/storage.py
+++ b/blivet/devices/storage.py
@@ -745,6 +745,8 @@ def isNameValid(cls, name):
         # ...except some names *do* contain directory components, for this
         # is an imperfect world of joy and sorrow mingled. For cciss, split
         # the path into its components and do the real check on each piece
+        if name is None:
+            return False
         if name.startswith("cciss/"):
             return all(cls.isNameValid(n) for n in name.split('/'))

[rhinstaller/blivet/pulls/161 master] Don't fail in storage.py:isNameValid when name is None

[dashea]

Thanks for the patch.

Yes, that is a bad way to fail. But...

Let's assume that you are correct and that all subclasses of StorageDevice should have a name that is not None. Many subclasses of StorageDevice override StorageDevice.isNameValid w/out invoking it. So, your patch would introduce a behavior where sometimes a TypeError or AttributeError is raised, and sometimes False is returned. More  method surgery is needed to get this patch to work properly and get the intended result. Probably the actual checks on strings should be lowered into an internal method, like ```_isNameValid```, while ```isNameValid``` would only handle issues about None. It should also be noted that ```Device.isNameValid``` _might_ be the place to make this change, not ```StorageDevice.isNameValid```.

Alternatively, there may be some devices that are subset of StorageDevice that do not require a name that is not None. In that case, this patch would be wrong. I don't think that is true, but the possibility needs to be eliminated, especially before additional work is done.

@dwlehman: Is it the case that all StorageDevices should have a name that is not None when created?

[rhinstaller/blivet/pulls/161 master] Don't fail in storage.py:isNameValid when name is None

[dashea]

We'ld be interested in tracking down the reason for the missing name.

Could you give information about blivet version you're using and also accept a small patch which should set up logging and log some additional information about the md name? It should be possible to synthesize it from other data, and it is a mystery why that is not happening.

[rhinstaller/blivet/pulls/161 master] Don't fail in storage.py:isNameValid when name is None

[dashea]

Sure, I can accept a patch.
I use version 1.0 (pre libblockdev). I can't upgrade version just now due to system dependencies.
But looking at handleUdevMDMemberFormat in devicetree.py I see a possibility for md_name to be None. So Murphy's law is in action.

[rhinstaller/blivet/pulls/161 master] Don't fail in storage.py:isNameValid when name is None

[dashea]

There are some cases in which it is okay to pass a name of None to the constructor (BTRFSDevice, LoopDevice come to mind), but every device should have a name that is not None once the constructor has run.

I tend to think that the reason for not finding the array name is the end of this you should be chasing instead of changing the naming rules.