Fix qdiskd device scanning logic

Hi Lon,

the original idea of scanning /dev was not too bad but after porting it to ocfs2-tools and brainstorming again with the guys, we found a few things that could be done better.

Let's change a direct scan of /dev into:

- scan /sys/block first for well.. block devices.
- sysfs exports enough info for us to filter out removable devices like cdroms that we don't care to even check and to gather maj/min of each device.
- scan /dev only to match maj/min and gather the device name.
- never dig or attempt to check hidden files.

Please review or apply.

Joel: once applied on top, this is basically the same code that would land in ocfs2-tools.

Thanks
Fabio

I'm going to make him an offer he can't refuse.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix_proc_part_scan.diff
Type: application/pgp-encrypted
Size: 5980 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20071128/a166c95f/attachment.pgp>

We need to check /sys/block/<xxx>/device/media too. A removable
cdrom should be skipped. A removable USB/FW drive? Perhaps not.

You should do this up front to create a mapping table.
Currently, it will re-recurse /dev for each device found in /sys. While the data will be cached in the kernel, it's much better to have a lookup table in your code. You also save all checking on non-block devices in additional paths. If a device appears after your table was built, you just rescan.
Also, it needs better handling of "I want a list of all
devices". It currently only does "print out in a static way". ocfs2-tools wants a list of all devices. Sometimes we want "all devices", sometimes "all ocfs2 devices", sometimes "all non-ocfs2 devices".
Wouldn't we be better off with a scan API that you call without
the path prefix (it knows how to find /sys/block and /dev), but with a callback function? That function can take the path in /dev, is_removable, the media type, and a void*. It can then decide on whether the block device is valid or not, adding to the void* as it pleases. The return code from the callback would specify "<0 == error, 0 = continue, 1 = quit". Then a simple "lookup label" find would use the callback to check the label against the void*. When it matches, it fills in the void* with the device name and returns 1. That sort of thing.

Joel

"When ideas fail, words come in very handy." 
         - Goethe

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Ok, we will need to assume that if we can't find device/media or device/type, it is a good one. Same way as removable work as it is not present in all block devices (see ram/loop for example).

Ok, i understand why you are asking this and it is perfectly doable. Generally I think that it is a bit over engineered for what we need to achieve and it adds some extra complexity to the code, such as caching of /dev (that is racy), invalidate the cache and rescan.
I will poke around and see how complex this become using your approach and we will see how it goes.

Right, but this is something you can abstract from the scanning function itself into a higher level. To know what is an ocfs2 device, you still need to scan all of them to build a list in the first place.

So I think the scanning function (also given your points below) should just really scan for valid block devices (all of them) and stop there. Whatever you do with the result change from apps to apps and IMHO should be done one level up in the stack where you can do caching of the results, invalidate the cache and ask for a rescan (etc..).

We can just use the callback in scandir to achieve the same. I didn't want to use it before because it makes the code a bit less readable IMHO but i was discussing the exact same issue with Lon :)

Fabio

I'm going to make him an offer he can't refuse.