[PATCH] remove usage of g_assert() in blockdev

David Zeuthen david at fubar.dk
Mon Nov 13 11:51:17 PST 2006


Danny Kukawka wrote:
> On Monday 13 November 2006 19:38, Andrey Borzenkov wrote:
>> On Monday 13 November 2006 21:14, David Zeuthen wrote:
>>> Danny Kukawka wrote:
> [...]
>>> Hmm, actually I think the use of the assert may be justified... but I
>>> can see this causing problems maybe if uevents are happening during the
>>> startup.. So I'd say OK to commit if you also use HAL_WARNING just
>>> before the 'goto error'.
>>>
>>> Can you describe a bit more the circumstances when this bug happens?
>>> E.g. what system, are uevents being triggered etc. etc.
>> https://bugs.freedesktop.org/show_bug.cgi?id=8210

And I think my reply for that still stands

  http://lists.freedesktop.org/archives/hal/2006-September/006272.html

E.g. "Don't do that right now". But see below.

> Hm ... didn't saw this bug as we get the problem at SUSE (but this is the 
> same, as it looks to me): https://bugzilla.novell.com/show_bug.cgi?id=217563
> Unfortunately the bug did not happen on every boot.
> 
> I think we don't need to exit/end the daemon only because we can't find a 
> device, exceptionally not if we handle such errors in a complete othere way 
> in the rest of the code. I can live with a error message and with missing one 
> device instead of exit without a comment. ;-)
> 
> I add a HAL_WARNING (or should we better use a HAL_ERROR ?) and refer in the 
> commit comment to #8210.

HAL_WARNING is fine I think.

Going forward we probably want to rethink how we do coldplug and ensure 
it's synchronized with the rest of the system. Which probably means 
recommending distributors to start HAL just before udev (without any 
sysfs coldplug) and then relying on udev's coldplugging code to emit the 
right uevents. But there are probably subtble issues to doing that; 
you'd have to ensure that all entries that HAL care about in sysfs 
actually triggers uevents. That's definitely not true today. Kay?

Starting HAL late and doing coldplug while other processes are loading 
drivers (cause uevents to be triggered) is just too dangerous and leads 
to bugs like these I think. Danny's fix is appropriate right now, it's 
probably better not to crash and just have one device less than to 
totally crash.

      David



More information about the hal mailing list