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
