Hi Jan,
Thank you for making the changes.
Sundar's question about invoking ls_init() made me realize one more issue.
usr/src/cmd/auto-install/auto_install.c
----------------------------------------
1628 if (ls_init_attr != NULL)
I believe this check, done on line 1628 for ls_init_attr != NULL, should
also be done before line:
1623 nvlist_free(ls_init_attr);
Other than that things look good to me.
I see no need for another round of code review after you address this.
Thanks you. Joe
jan damborsky wrote:
> Hi Joe,
>
> thank you very much for your comments.
> Please see my response in line.
> I have updated the webrev with all changes
> incorporated.
>
> Jan
>
>
> On 04/03/09 22:34, Joseph J VLcek wrote:
>>
>> +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
>>
>>
>> usr/src/cmd/auto-install/auto_install.c
>>
>>
>> Issue 1:
>> --------
>>
>> Missing "break; between lines 1582 and 1583
>>
>> 1581 case 'v': /* debug verbose mode enabled */
>> 1582 enable_debug_mode(B_TRUE);
>> break;
>> 1583 }
>
> Thanks for catching this !
>
>>
>>
>> +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
>>
>>
>> usr/src/cmd/slim-install/svc/live-fs-root
>>
>> Nit/suggestion: Move both the install.conf file name and file spec to
>> variables.
>>
>> Perhaps change from:
>> 48 INSTALL_CONF=/tmp/install.conf
>>
>> To:
>> INSTALL_CONF_FILE="install.conf"
>> INSTALL_CONF_SPEC="/tmp/${INSTALL_CONF_FILE}"
>>
>> Then you could:
>> From:
>> 315 # download the install.conf file to get the service name
>> for SPARC
>> 316 if [ "$ISA_INFO" = "sparc" ]; then
>> 317 install_conf="$url/install.conf"
>> 318 /usr/bin/wget $install_conf -O $INSTALL_CONF > \
>>
>> and
>>
>> 373 AI_ENABLE_SSH=`/usr/bin/grep "^livessh"
>> $INSTALL_CONF |
>>
>>
>> To:
>> # download the install configuration file to get the
>> service name for SPARC
>> if [ "$ISA_INFO" = "sparc" ]; then
>> install_conf="$url/${INSTALL_CONF_FILE}"
>> /usr/bin/wget $install_conf -O $INSTALL_CONF_SPEC >\
>> and
>>
>> AI_ENABLE_SSH=`/usr/bin/grep "^livessh"
>> $INSTALL_CONF_SPEC |
>
> Done.
>
>>
>>
>> +-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
>>
>>
>> usr/src/lib/libict_pymod/ict.py
>>
>> It's not clear to me why this needs to change.
>> Please explain?
>
> '2>&1' redirected stderr to stdout which caused debug messages
> to go to stdout (after debug mode was enabled by setting BE_PRINT_ERR
> to true in live-fs-root script). Since stdout is passed to the parser
> for searching root dataset, debug messages got lost and confused the
> parser.
>
