Hi Joe,
On 04/06/09 13:57, Joseph J VLcek wrote:
> 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);
You are right - good point. I have added that check.
>
>
> Other than that things look good to me.
>
> I see no need for another round of code review after you address this.
Thanks a lot for review !
Jan
>
> 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.
>>
>
