On 02/03/2010 08:37 AM, Ales Kozumplik wrote:
> You are right and I didn't consider the nokill option much. Your patch
> is completely disregarding doKill, but suppose we used this version
> instead:
Sorry, my fault, I accidently dropped doKill which we need of course.
> Here's what happens with any rebootAction and doKill=0:
> :: the program goes into the endless loop in performDelayedReboot()
> :: user presses cad, sigint handler is triggered
> :: it calls getKillPolicy() that tells it to call shutDown() with doKill=0 again
> :: endless loop in performDelayedReboot() again
Exactly, I just strictly interpreted "nokill" to never actually REBOOT.
:) This might not meet user expectation.
> I agree that with 'nokill' we should always do DELAYED_REBOOT. What we
> need to make it work correctly though is a mechanism to remember that we
> called performDelayedReboot() already and are ready to do normal
> shutdown.
Sure, if you want the second DELAYED_REBOOT to actually trigger the
reboot independent of "nokill", then you need to remember that it has
been called earlier already (I think that may be what the old code did,
which had a sigint handler in init to catch CAD during install and a
sigint handler in shutdown which would catch CAD from inside delayed
reboot on loader/anaconda fail out).
I don't know the originally intended nokill semantics well enough.
> There's no nice solution: having the signitHandler() called in
> the middle we can't pass this information around as a parameter. Maybe
> we can use a static variable:
>
> void shutDown(int doKill, reboot_action rebootAction)
> {
> static int reentered = 0;
>
> if (reentered) {
> performUnmounts();
> performTerminations();
> performReboot(rebootAction);
> }
> reentered = 1;
> if (rebootAction == DELAYED_REBOOT || !doKill) {
I had a hard time formally proofing that this is in fact equivalent to
(rebootAction != DELAYED_REBOOT && doKill)
plus reversing then and else case, i.e. negating the if clause.
Maybe I was just too used to (rebootAction != DELAYED_REBOOT && doKill)
because it describes the probably most common case and I understood that
better. Also the previous notation with calling performDelayedReboot
unconditionally after the if statement absolutely made sure, we never
exit init, not even if performReboot didn't work. But then again, I'm
not sure if performReboot can ever fail.
Apart from my nit-picking, it looks good. :)
> performDelayedReboot();
> } else {
> performUnmounts();
> performTerminations();
> performReboot(rebootAction);
> }
> exit(0);
> }
Do we also need the fix for rhel6-branch because of bug 533198?
Steffen
Linux on System z Development
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
