Hey Tim, These are really just nits:
libbe_priv.h - Can you move the prototype definition for be_activate_current_be() under line 145. be_activate.c - Can you move the be_activate_current_be() function to be somewhere before line 226. Should we move be_is_active_on_boot() into be_activate.c as well? -ethan Tim Knitter wrote: > Dave and Evan, > > I addressed both your comments in the latest webrev. Please take a look > > http://cr.opensolaris.org/~tsk/1868_slim/ > > Thanks > Tim > > Dave Miner wrote: > >> Tim Knitter wrote: >> >>> Dave Miner wrote: >>> >>>> Tim S. Knitter wrote: >>>> >>>>> Can someone please review the following? >>>>> >>>>> 1868 beadm destroy creates an empty grub menu >>>>> >>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=1868 >>>>> http://cr.opensolaris.org/~tsk/1868_slim/ >>>>> >>>>> >>>> The fix seems a little problematic yet. The sequencing you've chosen >>>> here means that if we fail to activate the current menu item (which, >>>> though fairly unlikely, is certainly possible), then we still end up >>>> with a GRUB menu without an active entry. I'd rather we did things >>>> in an order that made that not possible. >>>> >>>> >>> I fixed this in the latest webrev. If you could verify when you can >>> find a spare moment, I'd appreciate it. >>> >>> >> Two things: >> >> - I found it odd that be_activate_current_be is off in the be_utils.c >> rather than in be_activate.c. Any particular reason it's there? >> >> - It seems like beadm perhaps should print a message noting which be >> will be active, just so the user realizes this and can correct if >> desired. Or, perhaps have the confirmation prompt that's put up note >> this case. >> >> Dave >> > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
