Hi Karen,
Thanks for the review. Response below....
On 17/03/2012 04:16, Karen Tung wrote:
Hi Niall,
Everything looks good. I have 1 small nit:
usr/src/cmd/distro_const/checkpoints/create_iso.py:
- Small nit: would it be better to define uefi_args in line 166 instead
of line 157 since it might not be used if uefi_eltorito is None.
Normally I agree with you. In this particular case, the ordering of the
arguments to mkisofs is extremely important, so I declare them
in their proper order to illustrate what the command line invocation
looks like, and comment on it to emphasise the importance of the
precise ordering of arguments. I think it would be less obvious if I
were to declare them in isolation at line 166.
Does that seem reasonable to you?
Thanks,
Niall
Thanks,
--Karen
On 03/16/12 08:55 AM, Niall Power wrote:
Hi,
Can I get a sanity check for this webrev for distro constructor.
This has already been reviewed as part of the overall project webrev
for UEFI support, but we have an opportunity to integrate a UEFI and
GRUB2 compatible distro-constructor before the actual delivery of
UEFI and GRUB2 in build 14.
So what I have done is extract a subset of the full webrev that
delivers just the distro constructor code changes. The separation is
quite clean and easily isolated from the rest of the code base. There
was no recoding necessary.
I'd like to ask for a quick sanity check however from folks who
previously reviewed the distro-constructor and boot checkpoint pieces
as part of my original webrev for UEFI.
I've tested this webrev to confirm that it
- builds a GRUB2 BIOS/UEFI based hybrid ISO image (confirmed that
said image boots)
- builds existing legacy GRUB based BIOS ISO images
- confirmed that existing installadm can import both the ISO and AI
pkg repo image for the legacy GRUB image
- confirmed that the usbcopy and usbgen function with both the legacy
GRUB and GRUB2 based ISO images and boot correctly for BIOS and UEFI
(for GRUB2)
Pointer to DC UEFI webrev:
http://jurassic.us.oracle.com/~npower/webrevs/webrev-dc-uefi-2012-03-16/
Pointer to overall slim_uefi webrev from which this sub-webrev is
taken from:
http://jurassic.us.oracle.com/~npower/webrevs/webrev-slim-uefi-2012-03-16/
Thanks!
Niall
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
