Seth,
The changes look OK to me now.
-ethan
On 09/30/11 10:32, Seth Goldberg wrote:
On Sep 30, 2011, at 10:05 AM, Ethan Quach wrote:
Seth,
Mostly questions below ...
legacygrub.py
------------------
881 - Just making sure here, but does this need a corresponding
'else' clause?
> 885 - Same question.
Yes; I just modified that code to catch the scenario where rpool
is set but there is no findroot directive in a preexisting entry (that
is not currently used by any pybootmgmt consumers). The resulting
block is now:
elif inst.rpool is not None:
self._debug('No bootfs property - using rpool=%s' %
inst.rpool)
# If there's a findroot command that's already part of the
# menu entry, use its partition hint
if inst._menulst_entity is not None:
# findroot's arguments come in two forms:
# SIGNATURE | (SIGNATURE,partition1[,slice])
findroot_orig =
inst._menulst_entity.find_command('findroot')
if findroot_orig is not None:
findroot_orig_args =
''.join(findroot_orig.get_args())
if findroot_orig_args.startswith('('):
findroot_components =
findroot_orig_args.split(',')
if len(findroot_components) >= 2:
self._debug('reusing findroot hint:
%s' % findroot_components[1:])
findroot_cmd += ' (pool_' + inst.rpool
findroot_cmd = ','.join([findroot_cmd]
+ findroot_components[1:])
if findroot_cmd == 'findroot':
findroot_cmd += ' pool_' + inst.rpool
So the last 'if findroot_cmd == 'findroot'' catches all the else
cases inside the if inst._menulst_entity is not None block as well as
the case when there is no _menulst_entity.
Thanks for finding that corner case :).
923 - At this point, we expect at least one of bootfs_cmd or
findroot_cmd to have been updated and hence 'prepend_cmds' should be
non-empty here correct? Would it be worth it to check against that
and raise BootmgmtIncompleteBootConfigError() if not?
Not necessarily -- this function also supports creating menu entries
for UFS boot instances (currently that just means the USB image), so
no exception should be generated.
menu.lst.py:
---------------
123 - "should *be* the first"
Fixed!
127,134 - if idx ends up being -1 at these places, do we need to do
anything else or is simply falling through and appending the command
to the end of the entry what we want/expect to do?
Falling through and doing the append is what we want.
Thanks. I've uploaded an incremental webrev based on your and
DarrenK's comments here:
https://cr.opensolaris.org/action/browse/caiman/sethg/7096624-incr-1/webrev-incr-1/
Thanks,
--S
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
