From: Joel Becker <Joel.Becker@oracle.com>
Date: Wed, 28 Nov 2007 14:11:53 -0800
Subject: [Cluster-devel] Re: [PATCH] Fix qdiskd device scanning logic
In-Reply-To: <474DD889.6030801@ubuntu.com>
References: <474DD889.6030801@ubuntu.com>
Message-ID: <20071128221153.GE22605@tasint.org>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

On Wed, Nov 28, 2007 at 10:07:21PM +0100, Fabio Massimo Di Nitto wrote:
> 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.

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

> - scan /dev only to match maj/min and gather the device name.

	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.

> - never dig or attempt to check hidden files.

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