[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,

On 05/04/2010 04:45 PM, David Cantrell wrote:

<snip>

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

This chunk seems to not belong in this patch.

Oh yeah, I had that and then removed it, but not here.  Fixed.

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

First, sorry about the messed up indentation, it seems our
mailers don't like each other.

Another reason I wish that ReviewBoard would be completed on fedorahosted.

Why not write:
+ spec = deviceNameToDiskByPath(self.name)
Instead of:
+ spec = deviceNameToDiskByPath(devicePathToName(self.path))

Forgot about self.name.  Fixed.

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:


This should be:
bypath = os.path.basename(deviceNameToDiskByPath(device.name))

(and drop the setting of path)

Fixed.  I also updated the initcb line a few down from this bypath line so it
uses 'bypath' instead of 'path' (that was the only reason I kept the path
variable).

OK, third revision.  Batting average is pretty low now.  :)


[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/devices.py    |   24 +++++-------------------
 storage/devicetree.py |    5 ++---
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/storage/devices.py b/storage/devices.py
index 399802b..7e23b9e 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(self.name)
         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..1ed73c1 100644
- --- a/storage/devicetree.py
+++ b/storage/devicetree.py
@@ -1356,15 +1356,14 @@ class DeviceTree(object):
         if self.zeroMbr:
             initcb = lambda: True
         else:
- -            path = device.path
             description = device.description or device.model
- -            bypath = os.path.basename(deviceNameToDiskByPath(path))
+            bypath = os.path.basename(deviceNameToDiskByPath(device.name))
             if bypath:
                 details = "\n\nDevice details:\n%s" % (bypath,)
             else:
                 details = ""

- -            initcb = lambda: self.intf.questionInitializeDisk(path,
+            initcb = lambda: self.intf.questionInitializeDisk(bypath,
                                                               description,
                                                               device.size,
                                                               details)

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

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

iEYEARECAAYFAkvgPGMACgkQ5hsjjIy1VkmdXwCgryHOsCATiPOztd5mEUt6U3ed
jbEAmwWd9DeSXM9z1J0WBlYe63wgQDkV
=2XJR
-----END PGP SIGNATURE-----


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