[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