Hi Alok, please see my response below.
Thank you, Jan Alok Aggarwal wrote: > Hi Jan, > > Thanks a lot for taking the time to review the > change. Please see my comments inline. > > On Mon, 13 Oct 2008, jan damborsky wrote: > >> Hi Alok, >> >> please see my comments below - I have gone through >> all the files - those not referred look good to me. >> >> Thank you, >> Jan >> cmd/auto-install/auto_install.c >> 409 - I think &api should be passed to ai_get_manifest_partition_info() >> (one more level of indirection should be added), otherwise value of >> pointer to partition configuration is lost and illegal memory access >> might occur on 410,417 >> >> 423 - ditto It seems the change was not probably propagated, since looking at the pushed source code, it is not there. >> 574 - With respect to defining location of IPS repository, >> will specifying of IPS mirror be supported ? I think it >> might be useful for people who run their own local IPS >> content mirrors. > > It will be. Jean just pushed the ability to specify IPS mirrors > as an argument to the transfer module like Friday or over the > weekend. If we don't get to it over the next couple of weeks, > I suspect we will get to it shortly after 2008.11 ships. ok. Thanks ! > >> cmd/auto-install/auto_parse.c >> ----------------------------- >> >> 503 ptr2 = strrchr(ptr, '\"'); >> -> >> 503 ptr2 = strrchr(ptr, '\''); It seems this change was not integrated. >> >> 522 - For 2008.11 release, will be setting of 'default locale' >> supported ? I am asking, since it is not searched/recognized >> in auto_parse_sc_manifest(). > > We won't be supporting it. ok. Thanks for clarifying this. > > >> lib/libict/ict.c >> ---------------- >> >> 449,535,558 - I am not sure that it will work, since it seems >> that these configuration changes (locale, hostname, >> nodename) >> are not propagated to '/etc/*' files on running system >> according to values defined in SC manifest. > > So, right now the SC manifest doesn't allow for setting > of any of these values. Even though setting the locale > isn't going to be supported for 2008.11, I think setting > the hostname should be supported. I'll look into it. ok. >> 830-833 - this check should be done on 817 This change was not pushed and it seems it might be causing regression when installing from LiveCD. I have filed following bug to track this: 3899 LiveCD installation fails with SUNWinstall built from tip 272:6ffbbcf6242a http://defect.opensolaris.org/bz/show_bug.cgi?id=3899 >> >> Alok Aggarwal wrote: >>> This a request for the review of the automated installer >>> (aka the AI engine). The webrev can be found here - >>> >>> http://cr.opensolaris.org/~aalok/ai-engine-complete >>> >>> Some of the pieces that will be delivered as part of this >>> changeset include - >>> >>> - a new auto-install command >>> - liborchestrator changes to support IPS based >>> installs >>> - install-finish/ICT changes to support automated >>> installs >>> >>> The changeset also includes some changes to the transfer >>> module that were the subject of discussion a few weeks >>> back. I plan on putting these back as is and fixing them >>> in the coming week (i.e. well before 2008.11 ships) (really). >>> >>> A brief overview of the mechanics >>> --------------------------------- >>> >>> The AI service choosing engine (delivered as part of defect >>> 3478) obtains a combined manifest from the AI webserver. >>> The combined manifest includes the AI manifest (where to >>> install, which slices/partitions to modify, etc) and the >>> SC (system configuration) manifest (username, password, root >>> password, timezone, etc). >>> >>> The auto-install program (aka AI engine) is then invoked >>> with this combined manifest as an argument. The installer >>> first splits the manifest into separate AI and SC manifests. >>> It then validates the AI manifest using the defaults validator >>> (found in $SRC/lib/install_utils/ManifestServ) and subsequently >>> lays out the manifest in an in-memory tree. Based on the AI >>> manifest specification it goes off and modifies whatever slices/ >>> partitions need to be modified. Then it builds up the installation >>> arguments (target disk, username, password, etc) based on the >>> manifest specification and invokes liborchestrator to install >>> the target. install-finish runs in the end of provide the >>> finishing touches. >>> >>> Please review the changes and provide your feedback. >>> >>> Thanks, >>> Alok >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >>
