Karen,
Alok wanted me to update and comment on the grub_setup.py checkpoint.
Responses inline.
--------------
usr/src/cmd/distro_const/checkpoints/grub_setup.py
--------------
- Lines 85-90: Instead of having hard coded defaults here, I think
it is better for them to be defined as constants in the class so
they are easy to find and change in the future.
Fixed to be class constants.
- Line 85-90: I noticed that you didn't assign a default value for
self.grub_mods.title. I think it would be easist to check that
no special title is specified here, and retrieve the value from
/etc/release.
Then, use a flag to keep track of whether a special title is specified.
That would avoid the 2 places later that has the duplicate code to
get the value from /etc/release.
Good catch. Fixed to check for the grub title / /etc/release in the
parse_doc() method.
- Question: in existing DC, there's an entry in the manifest called
<title> within the <grub_menu_modifications> section.
The value specified in <title> is used for populating
all the grub menu entries. So, if I want to do the same
thing with the new DC and manifest, I would just do
the following:
<grub_mods min_mem="0" title="myentry" default_entry="0" timeout="5">
</grub_mods>
Note that I didn't specify a <grub_entry> because I don't want another
grub entry. Have we tested this case to make sure it works?
If it does, I think we need to put better comment in the manifests than
what we have now. The way it looks right now, it looks like I will
have to
specify a <grub_entry>
Verified by hand that it works. Updated the manifests to reflect that
the <grub_entry> elements are optional.
- Line 145-162: In the update_img_info_path() function,
the GRUB_TITLE line is always written to the .image_info file.
The algorithm in the current DC grub_setup.py file only
writes the GRUB_TITLE line to the .image_info file if a special
title is specified. If I remember right, there's special logic
in installadm(1M) to deal with the GRUB_TITLE line being present or not.
Please check installadm(1M) to make sure this new behavior is
OK or change it back to the way it is done in the current DC.
Fixed to mirror the logic in old grub_setup.py.
- LiveCDGrubSetup.build_entries() function: What happened to all
the "special" entries that are in
the existing all_lang_slim_cd_x86.xml? Such as the livessh=enable
and all the entries to support accessibility. I checked the
dc_livecd.xml, and don't see them there either.
Added the missing ssh entries to the build_entries()
- LiveCDGrubSetup class: for the the Live CD, the accessibility
entries MUST be the last entry on the grub menu. I don't see any
special code in this class to deal with that.
Fixed along with the previous comment.
- Line 236-256: AIGrubSetup.update_img_info_path().
The code to get the grub title from etc/release is duplicate.
Furthermore, as with the comment above, the GRUB_TITLE should
not always be written out to the file.
Fixed as stated above.
- Line 253: Need to remove this comment?? :-)
Zap. It's gone.
Thanks Karen!
-Drew
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
