Re: [PATCH 3/7] Make storage.devices.deviceNameToDiskByPath() more robust.

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

 



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

On Wed, 14 Oct 2009, Steffen Maier wrote:

One comment below.

On 10/08/2009 09:28 AM, David Cantrell wrote:
Have deviceNameToDiskByPath() return the full device path rather than
just the basename path.  Have it check to see if the entry in
/dev/disk/by-path is actually a symlink, then readlink that.  Otherwise
just take the basename of the entry.  Also fix a problem where passing
it a full device path would not match (e.g., '/dev/dasdb1').  Take the
basename of deviceName and match that against the readlink() value.
---
 storage/devices.py |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/storage/devices.py b/storage/devices.py
index 348d2fd..9d257e2 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -154,10 +154,18 @@ def deviceNameToDiskByPath(deviceName=None):
     if not os.path.isdir(bypath):
         return ""

+    deviceName = os.path.basename(deviceName)
+
     for path in os.listdir(bypath):

With many devices this linear search may become a performance problem.
I was wondering, if one could just use something that you already had
for a different purpose in your "[PATCH] Add dracutSetupString() method
to ZFCPDiskDevice (#526354)". Take info.get("symlinks") since it's in
the udev-db anyway already. Search for a match of "disk/by-path" and
then you have it.

That works for the call from storage/devicetree.py, but the call from
PartitionDevice.fstabSpec won't have the 'info' hash at that point.  I suppose
deviceNameToDiskByPath() could be extended to use the info hash if it's a
parameter, but I'm not sure it's a performance problem yet.  I did these rough
tests:

1) Ran deviceNameToDiskByPath() for /dev/sda1 on my laptop.  The by-path
directory on my laptop has these entries:

pci-0000:00:1a.7-usb-0:1.4:1.0-scsi-0:0:0:0@
pci-0000:00:1a.7-usb-0:1.4:1.0-scsi-0:0:0:0-part1@
pci-0000:00:1f.2-scsi-0:0:0:0@
pci-0000:00:1f.2-scsi-0:0:0:0-part1@
pci-0000:00:1f.2-scsi-0:0:0:0-part2@
pci-0000:00:1f.2-scsi-1:0:0:0@

Here are the /usr/bin/time[1] results:

    real time: 0:00.07
    CPU-seconds in kernel mode: 0.00
    CPU-seconds in user mode: 0.01

2) Created a fake by-path style directory with 32766 symlinks pointing to
different files in ../../../usr/bin.  Since there are only 2403 files in my
/usr/bin, some repeat, but the idea is more or less the same.  And access here
is slower since I'm pointing to actual things on a filesystem whereas sysfs is
in memory.  I ran a timed test of deviceNameToDiskByPath() using my fake
by-path tree and had it look for 'sda32766'.  Here's what I got:

    real time: 0:00.89
    CPU-seconds in kernel mode: 0.42
    CPU-seconds in user mode: 0.42

This test isn't perfect since #2 is looking at an actual on-disk filesystem.
It's also worth noting that I'm on an SSD, so it's even slower than a regular
disk.

And given that there are currently only two calls to deviceNameToDiskByPath(),
I'd prefer to keep the code in anaconda simple rather than make it behave
differently based on the caller.

We could modify the caller in storage/devicetree.py, but that would require
pulling in the info hash (which is there), but then we're counting on the
symlinks hash member.  And, as you've pointed out, we should just read the
trees and get the information ourselves.  I'd rather keep things the way they
are for now.  If this becomes a major performance problem, we can discuss a
redesign.

[1] I used the following time command:
/usr/bin/time -f \
"real time: %E\nCPU-seconds in kernel mode: %S\nCPU-seconds in user mode: %U" \
./test-bypath

-        target = os.path.basename(os.readlink(bypath + '/' + path))
+        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 path
+            return entry

     return ""


- -- David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI

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

iEYEARECAAYFAkrXoRYACgkQ5hsjjIy1Vkl4XACgqgnjdlxRR+kDUvg239HyTzZA
r3UAn0IWImZZJFJ+aiHMPjxjN/C5fw6g
=4mUp
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux