Hi Niall.

I hope you are feeling better...

I have no further comments except as noted below.

    Thanks,
    Jack

On 04/27/11 06:54 AM, Niall Power wrote:
Hi Jack,

Thank you very much for the review. Responses below...


Hi Niall.

I reviewed group 3 and a few extra files:

Group 3:
========
usr/src/cmd/distro_const/manifest/dc_ai_x86.xml
-----------------------------------------------
76-78: There is no magnifier entry in this file, so
this comment can be
removed.
Fixed.
It's still there in the cud-boot-1-vs-4 review.

usr/src/cmd/distro_const/manifest/dc_text_x86.xml
-------------------------------------------------
77-79:  There is no magnifier entry in this file, so
this comment can be
removed.
Fixed.
It still shows up in the cud-boot-1-vs-4 review.
additional files:
=================
boot.py
-------
Not sure if this is the right place for this, but
would it be a good
idea to emit a warning of kernel_args are provided in
a SPARC manifest?
I think it might be appropriate. It's not just kernel_args that
are not supported however. timeout isn't either nor are
boot_entry objects as a whole. At the moment they are just
silently ignored (and not included in the DC sparc manifests).
As per Dave's comments I have updated the boot.dtd comments
to clarify matters.
While it is true they are updated in boot.dtd, boot.dtd is no longer getting delivered. Looks like it has been replaced by boot_mods.dtd, and the new file is missing the updated comment (line ~76).
I had intended to catch BootmgmtUnsupportedPropertyError
via pybootmgmt's SPARC backend when setting these properties but since we
don't have SPARC support I had to bolt on a SPARC solution in a bit of a hurry 
and
bypass the methods that process boot_mods and boot_entries. Processing them
is going to further entangle the currently bolted on SPARC support  so I'd 
rather not
do that unless you feel strongly on the matter?
OK. I suggest filing a P4 bug so this doesn't get dropped on the floor in the future.

    Thanks,
    Jack
Thanks again!

Niall

      Thanks,
Jack

On 04/20/11 06:51 PM, Niall Power wrote:
Hi all,

I'm looking for a few brave souls to review my
implementation of a boot checkpoint based on
pybootmgmgt. If you'd like to volunteer but get
around to it later, please let me know when
you could be available to review.

The changes aren't huge mostly tweaks here and
there, with the bulk of the code contained in
usr/src/lib/install_boot/boot.py (about 1100 LOC)
and boot_spec.py (about 250 LOC) so I'd be
very grateful for any comments received by April
27/28th. I'll even buy you a pint :-)
The changes can be logically grouped together as
follows if you want to look at a subset of the
changes:
1) Boot schema definitions and Data Object Cache
support
usr/src/lib/install_manifest/dtd/Makefile
usr/src/lib/install_manifest/dtd/boot.dtd
usr/src/lib/install_manifest/dtd/dc.dtd
usr/src/lib/install_boot/boot_spec.py
usr/src/lib/install_boot/test/test_boot_spec.py
(Tests)
2) Boot checkpoint implementation
usr/src/lib/install_boot/__init__.py
usr/src/lib/install_boot/boot.py
usr/src/lib/install_boot/test/test_boot.py (Tests)

3) Distro Constructor changes to use new boot-setup
checkpoint instead of grub-setup for X86
usr/src/cmd/distro_const/__init__.py
usr/src/cmd/distro_const/checkpoints/create_iso.py
usr/src/cmd/distro_const/manifest/dc_ai_x86.xml
usr/src/cmd/distro_const/manifest/dc_livecd.xml
usr/src/cmd/distro_const/manifest/dc_text_x86.xml

4) Packaging and makefile changes to slim_source
repo
usr/src/Makefile.master
usr/src/Targetdirs
usr/src/lib/Makefile
usr/src/lib/Makefile.targ
usr/src/lib/install_boot/Makefile
usr/src/lib/install_manifest/dtd/Makefile
usr/src/pkg/manifests/system-library-install.mf


Webrev location:
http://cr.opensolaris.org/~niall/cud-boot/

Thanks,
Niall
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-di
scuss


_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to