sosreport / sos

A unified tool for collecting system logs and other debug information
http://sos.rtfd.org
GNU General Public License v2.0
508 stars 543 forks source link

Iterator abstractions (hbas, disks, cpus, network interfaces, bridges, etc.) #148

Closed bmr-cymru closed 4 years ago

bmr-cymru commented 11 years ago

Iterate over $some_system_objects is becoming a fairly common idiom in sos plug-in code. E.g. the filesys and block plug-ins contain very similar code for iterating over whole-disk block devices as found in /sys/block:

        if os.path.isdir("/sys/block"):
            for disk in os.listdir("/sys/block"):
                if disk in [ ".",  ".." ] or disk.startswith("ram"):
                    continue
                disk_path = os.path.join('/dev/', disk)
                self.add_cmd_output("udevadm info -ap /sys/block/%s" % (disk))
                self.add_cmd_output("parted -s %s print" % (disk_path))
                self.add_cmd_output("fdisk -l %s" % disk_path)

and:

        if os.path.isdir(sys_block):
            for disk in os.listdir(sys_block):
                if disk.startswith("sd") or disk.startswith("hd"):
                    disk_path = os.path.join(dev_path, disk)
                    self.add_cmd_output("hdparm %s" % disk_path)
                    self.add_cmd_output("smartctl -a %s" % disk_path)

These two especially are doing essentially the same thing: iterating over all entries in /sys/block and filtering by name.

It seems like it would be useful to provide a common abstraction for these uses rather than allowing multiple hand-rolled versions to proliferate.

bmr-cymru commented 11 years ago

Similar thing from kernel.py:

        try:
            modules = os.listdir(self.sys_module)
            self.add_cmd_output("modinfo " + " ".join(modules))
        except OSError:
            self.soslog.error("could not list %s" % self.sys_module)

This time it's iterating over modules but it's the same basic idiom.

pierg75 commented 11 years ago

Just for me to learn, how would you tackle this problem?

What about creating a parent class that has a "iterate" method usable by all children classes? The method will simply return a list with all the entries in a dir.

jhjaggars commented 11 years ago

seems like all that you might need is a utility function that wraps os.listdir with an optional filter function?

def sos_listdir(path, filter_=None):
    try:
        return filter(filter_, os.listdir(path))
    except OSError:
        return ()

Then you could use it like:

for path in sos_listdir("/sys/block", lambda d: not d.startswith("ram")):
    # do things
TurboTurtle commented 4 years ago

This is generally handled by a2720ae - which today has support for block devices and the framework for other devices. We'd just need to expand this as we see fit going forward for E.G. NICs.