[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH 3/4] Make deviceNameToDiskByPath check udev info instead of sysfs



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 4 May 2010, Hans de Goede wrote:

Hi,

Some more comments from reviewing patch 4 in this set.

On 05/04/2010 06:46 AM, David Cantrell wrote:
Rather than scanning /dev/disk/by-path directly, check the udev database
for the PATH_ID or ID_PATH or whatever it's called that gives us the
devlink name that would appear in /dev/disk/by-path.  Sometimes the
links in /dev/disk/by-path do not appear because you have them already
in /dev/disk/by-id (or not, who knows?).  At any rate, stop checking the
/dev/disk/by-path directory manually and just ask udev.

Related: rhbz#560702
---
  storage/devices.py |   24 ++++++------------------
  1 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/storage/devices.py b/storage/devices.py
index 399802b..5761d05 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -147,28 +147,16 @@ def devicePathToName(devicePath):


  def deviceNameToDiskByPath(deviceName=None):
-    bypath = '/dev/disk/by-path'
-
      if not deviceName:
          return ""
+    else:
+        deviceName = os.path.basename(deviceName)

You cannot simply convert a path to a name this way, instead
you should use devicePathToName, or better fix all callers
of deviceNameToDiskByPath to pass in a name as the function
name suggests. You're already doing this for the
usage in dasd.py, this only leaves one usage in
devices.py PartitionDevice.fstabSpec() and one usage
in devicetree.py to fixup.


Thanks for the review.  I've made those changes.  Here is the revised patch:


[PATCH] Make deviceNameToDiskByPath check udev info instead of sysfs

Rather than scanning /dev/disk/by-path directly, check the udev database
for the PATH_ID or ID_PATH or whatever it's called that gives us the
devlink name that would appear in /dev/disk/by-path.  Sometimes the
links in /dev/disk/by-path do not appear because you have them already
in /dev/disk/by-id (or not, who knows?).  At any rate, stop checking the
/dev/disk/by-path directory manually and just ask udev.

Related: rhbz#560702
- ---
 storage/dasd.py       |    1 +
 storage/devices.py    |   24 +++++-------------------
 storage/devicetree.py |    2 +-
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/storage/dasd.py b/storage/dasd.py
index 6c9241f..dc6fdc4 100644
- --- a/storage/dasd.py
+++ b/storage/dasd.py
@@ -23,6 +23,7 @@ import iutil
 import sys
 import os
 from storage.errors import DasdFormatError
+from storage.devices import devicePathToName
 from storage.devices import deviceNameToDiskByPath
 from constants import *
 from flags import flags
diff --git a/storage/devices.py b/storage/devices.py
index 399802b..e7f196d 100644
- --- a/storage/devices.py
+++ b/storage/devices.py
@@ -147,28 +147,14 @@ def devicePathToName(devicePath):


 def deviceNameToDiskByPath(deviceName=None):
- -    bypath = '/dev/disk/by-path'
- -
     if not deviceName:
         return ""

- -    if not os.path.isdir(bypath):
- -        return ""
- -
- -    deviceName = os.path.basename(deviceName)
- -
- -    for path in os.listdir(bypath):
- -        entry = bypath + '/' + path
- -
- -        if os.path.islink(entry):
- -            target = os.path.basename(os.readlink(entry))
- -        else:
- -            target = os.path.basename(entry)
- -
- -        if target == deviceName:
- -            return entry
+    for dev in udev_get_block_devices():
+        if udev_device_get_name(dev) == deviceName:
+            return udev_device_get_path(dev)

- -    return ""
+    return deviceName


 class Device(object):
@@ -1100,7 +1086,7 @@ class PartitionDevice(StorageDevice):
     def fstabSpec(self):
         spec = self.path
         if self.disk and self.disk.type == 'dasd':
- -            spec = deviceNameToDiskByPath(self.path)
+            spec = deviceNameToDiskByPath(devicePathToName(self.path))
         elif self.format and self.format.uuid:
             spec = "UUID=%s" % self.format.uuid
         return spec
diff --git a/storage/devicetree.py b/storage/devicetree.py
index c8b7249..7f9ef46 100644
- --- a/storage/devicetree.py
+++ b/storage/devicetree.py
@@ -1356,7 +1356,7 @@ class DeviceTree(object):
         if self.zeroMbr:
             initcb = lambda: True
         else:
- -            path = device.path
+            path = devicePathToName(device.path)
             description = device.description or device.model
             bypath = os.path.basename(deviceNameToDiskByPath(path))
             if bypath:

- -- David Cantrell <dcantrell redhat com>
Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkvgMvMACgkQ5hsjjIy1VknpoQCgrPfAwkGHBykLkviB0zUDWTEM
UnsAn2dCw9UwJLXYegmUDFv024XQwP0F
=/GDi
-----END PGP SIGNATURE-----


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]