Re: [PATCH] improve device probing heuristics

[Andrew Clausen]

Hi Andreas,

> > Obviously, this means you have to traverse the
> > entire cache, but don't you need to do this
> > anyway?
> > 
> > (And iterating the cache should be cheap?)
> 
> Well, libblkid still needs a class of functions that
> I was talking about
> above, namely, to find all of the block devices on
> that system.  Before
> it has a cache, it has to have a list of devices to
> add to the cache.

So we can't assume the cache has already been
populated, by, say, initscripts?

> That code is replicated in so many places, because
> you can't guarantee
> that /proc/partitions is available, nor that it
> holds all of the block
> devices that are actually accessible - only those
> that have already
> been detected by the kernel.  It could very well be
> that for iSCSI or
> such that it is blkid which is telling the kernel
> about the existence of a device.

Yep.

> Yes, maybe I could add a void * (or maybe even a
> list, in case there are
> multiple layers like foo->libparted->libblkid) to
> each device struct which
> the calling application can use to associate private
> data with the device.

Sounds like a good idea.  Maybe I should do
the same with libparted.
 
> > Either that, or a "get_next" function...
> 
> Either is fine.  The above is what I'm used to from
> e2fsprogs, but I'm flexible.

I prefer get_next, because it's more convienient
for for loops (C doesn't have lambda functions!)

> > I think I prefer the following:
> > * each device has a bit-field of:
> >    - BLKID_PHYSICAL   (or BLKID_LOWLEVEL)
> >    - BLKID_STALE   (entry invalidated?)
> >    - BLKID_REMOVABLE
> 
> Well, depending on how you set up the scanning code,
> these could be
> either input flags or output flags.  On input they
> mean "I'm only
> interested in getting devices with the given
> property", and on output
> they mean "the device has this property".

The problem is: on input, each flag has one
of 3 states: don't care, must-be-0, must-be-1.

So, I think code like:

for (dev = blkid_first; dev; dev = blkid_next (dev)) {
   if (dev->flags & BLKID_REMOVABLE)
      continue;
   ...
}

...looks nice.

Your objection is: you want to be able to control
which device gets "deeply" probed (i.e. go hunting
for signatures)

Perhaps each device should have yet-another-flag:
BLKID_PROBED

If this flag isn't set, then you have a subset
of the information you would otherwise have
available... this subset should be cheap to get
from the OS.  (Is it easy to define such a
subset?)

Anyway, you can call blkid_probe() on the
device if you want more info...

There should be a function blkid_find_all()
that finds all devices without probing them.
(This should be cheap?)

> > This is another field...?
> > (dev->bus?)
> 
> Yes, it could be.  Some of this information is
> encoded in the device
> name and/or major/minor numbers, but it would also
> be nice for some
> applications to not care.  Again, this could be
> either an input and/or
> an output parameter.

Indeed... we don't want apps to have to parse
anything.
 
> > In the end, you need it because the user wants to
> see a list of
> > devices, and click on the one she wants.
> 
> Well, I'm not so much worried about that.  Most of
> the code I'm writing
> is not directly interfacing with a user, but is
> helper code for other
> apps (mostly consolidating the duplicated and
> slightly mutated code from
> many programs into a single place).

Right, but it's good to see why it's useful ;)

> > Does this ever gain you anything?
> > I think it boils down to: 'How "internal" are
> > the internals?'.  If the internals are so
> > internal that you need to hide them, you
> > probably got the ontology ("being") of the
> > object wrong.  i.e. the wrong fields in the
> > struct.
> 
> Well, I worked on libpng for a long time and we
> eventually moved to a
> model where the struct is supposed to be totally
> opaque and all data
> is gotten from accessor functions (even if they are
> just returning a
> single field in the struct).

Ah, this is one exception... you probably need
to use fancy data-structures + lots of bit-bashing, 
because you're trying to save space.  So, in
this case you're like to change HOW you're
going to represent the same thing.

OTOH, here, this is unlikely to happen.  If
it is going to change, then it's likely to
be accompanied by a change in philosophy, in
which case we should break compatibility anyway.

Obviously, a religious issue, hehehe...

> Well, I had thought there were tools/libs in the
> installers which could give you a list of, say, 
> disks, serial ports, etc.  Maybe their code is too
> huge for some needs, or maybe it is dying to use
> "the" block device iteration library, who knows.

There's kudzu, which could be worth a look...

Andrew



__________________________________________________
Do You Yahoo!?
Send FREE Valentine eCards with Yahoo! Greetings!
http://greetings.yahoo.com