> Also rewrote the baseudev.py to use the bindings instead of parsing > the udev db file > --- > baseudev.py | 66 ++++++---------------- > pyudev.py | 180 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 197 insertions(+), 49 deletions(-) Will pyudev.py eventually be moving into another package, like perhaps in the udev package or one of its subpackages? > @@ -43,20 +44,24 @@ def udev_get_device(sysfs_path): > log.debug("%s does not exist" % sysfs_path) > return None > > - db_entry = sysfs_path.replace("/", "\\x2f") > - db_root = "/dev/.udev/db" > - db_path = os.path.normpath("%s/%s" % (db_root, db_entry)) > - if not os.access(db_path, os.R_OK): > - log.debug("db entry %s does not exist" % db_path) > - return None > + udev = pyudev.Udev() > + # XXX we remove the /sys part when enumerating devices, > + # so we have to prepend it when creating the device > + udev_device = udev.create_device("/sys" + sysfs_path) > + udev.teardown() > + > + dev = udev_device.properties > + > + if dev: > + # remove the /dev/ part from beginning of devnode and set as name > + dev["name"] = udev_device.devnode[5:] > + > + dev["symlinks"] = dev["DEVLINKS"].split() > + dev["sysfs_path"] = sysfs_path > > - entry = open(db_path).read() > - dev = udev_parse_entry(entry) > - if dev.has_key("name"): > - dev['sysfs_path'] = sysfs_path > + # now add in the contents of the uevent file since they're handy > dev = udev_parse_uevent_file(dev) > > - # now add in the contents of the uevent file since they're handy > return dev > > def udev_get_devices(deviceClass="block"): We could potentially be calling udev_get_device() on several hundreds or even thousands of devices, which is a whole lot of: pyudev.Udev() udev.create_device() udev.teardown() Can we instead only call pyudev.Udev() once and stash an instance to the object as a global in baseudev.py? I'm concerned that we're going to be introducing another big slowdown here in device discovery on very large configurations. > +# XXX this one may need some tweaking... > +def find_library(name): > + libdirs = ("/lib64", "/lib") > + > + for dir in libdirs: > + files = fnmatch.filter(os.listdir(dir), "*%s*" % name) > + files = [os.path.join(dir, file) for file in files] > + > + if files: > + break > + > + if files: > + return files[0] > + else: > + return None Would be nice to search the $LD_LIBRARY_PATH, since we use that to do updates.img stuff: def find_library(name): env = os.environ.get("LD_LIBRARY_PATH") if env: libdirs = env.split(":") else: libdirs = ["/lib64", "/lib"] for dir in libdirs: ... > +class UdevDevice(object): If you really want to be fancy, you can add some special methods to UdevDevice so people don't have to go through the properties dict to access udev properties. You could do something like the following: def __getattr__(self, name): return self.properties[name] Then it's just a simple matter of calling it like this: udev_device = udev.create_device("/sys" + sysfs_path) print udev_device["symlinks"] print udev_device["busted"] The first print will return udev_device.properties["symlinks"]. The second will raise AttributeError, like you'd expect. I think doing something like this makes for a much nicer API and hides some of the implementation details. More information at: http://docs.python.org/reference/datamodel.html?highlight=__get__#customizing-attribute-access > +class Udev(object): > + > + def __init__(self): > + self.udev = libudev_udev_new() > + > + def create_device(self, sysfs_path): > + return UdevDevice(self.udev, sysfs_path) > + > + def scan_devices(self, subsystem=None): > + enumerate = libudev_udev_enumerate_new(self.udev) scan_devices could take a while, I suppose. Is it worth making it return an iterator so it doesn't have to grab everything all at once and can instead grab each individual device as needed? Or am I just making things too complicated now? > + # add the match subsystem > + if subsystem is not None: > + rc = libudev_udev_enumerate_add_match_subsystem(enumerate, subsystem) > + if not rc == 0: > + print >> sys.stderr, "error: unable to add the match subsystem" > + libudev_udev_enumerate_unref(enumerate) > + return [] print is a function in python3 (also python2.6?). For future compatibility, you'll want to: print("error: whatever", file=sys.stderr) One final minor point: This is a fairly low level module in anaconda. Whenever we add new things like this, it's good to think about how it could be automatically tested so we have more trust in what gets built on top of it. Have you given any thought to this yet?' Overall, looks interesting. - Chris _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list