jan damborsky wrote:
> 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.
>
Actually, the check for NULL isn't necessary. If you look at the
nvlist_free() code, it checks for NULL and does the right thing. Either
way is fine with me, I just wanted to point this out.
sarah
****
>>
>>
>> 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.
>>>
>>
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
