Looks good. John
Sent from my iPad On Apr 20, 2012, at 5:05 AM, Nirmal Agarwal <[email protected]> wrote: > Hi John, Jesse > > Ethan pointed me out that find_network[_*] functions are not called anymore. > I verified the same and couldn't find calls to these function. > So I have removed the functions from installadm-common.sh. > Please find the updated webrev : > > webrev : > > https://cr.opensolaris.org/action/browse/caiman/nirmal27/6996539-rev1/webrev.rev1/ > > webrev diff : > > https://cr.opensolaris.org/action/browse/caiman/nirmal27/6996539-rev1-diff/webrev.rev1-diff/ > > I have tested the following operations with the new packages. > --> create-service > --> create-service -i -c > --> create-service in multi-home network > --> create-client > --> delete-client > --> delete-service > > Thanks > Nirmal > > On 04/10/12 22:21, John Fischer wrote: >> Looks good. >> >> John >> >> On 04/ 6/12 04:36 AM, Nirmal Agarwal wrote: >>> Hi John >>> >>> Thanks for the review. >>> I have fixed the issues you raised. Please find the webrevs : >>> >>> webrev : >>> https://cr.opensolaris.org/action/browse/caiman/nirmal27/6996539-2/webrev/original/ >>> >>> >>> Diff : >>> https://cr.opensolaris.org/action/browse/caiman/nirmal27/6996539-2-diff/webrev/differential/ >>> >>> >>> As of now I can't find tests for ai_get_manifest.py. >>> I am not sure about the test for it. Please advice for the same. >>> >>> Regards >>> Nirmal >>> >>> On 04/06/12 01:33, John Fischer wrote: >>>> Nirmal, >>>> >>>> Some comments below. Do we need to add any tests to check the >>>> changes within ai_get_manifest.py? >>>> >>>> Thanks, >>>> >>>> John >>>> >>>> >>>> usr/src/cmd/installadm/installadm-common.sh >>>> >>>> - In python we use spaces within the shell scripts we use actual tabs. >>>> Please correct the indentation below: >>>> >>>> 891 # Iterate through the interfaces to figure what the possible >>>> 892 # networks are (in case this is a multi-homed server). >>>> 893 # For each network, calculate its network address and compare it >>>> 894 # with network address of the given IP address. >>>> 895 $IPADM show-addr -p -o ADDROBJ,ADDR|grep -v "^lo" | \ >>>> 896 while read addr_info ; do >>>> 896 while read addr_info ; do >>>> 897 >>>> 898 # remove the interface info >>>> 899 addr=`echo $addr_info|cut -d":" -f2` >>>> 900 #separate ip and mask >>>> 901 t_ipaddr=`echo $addr|cut -d"/" -f1` >>>> 902 bits=`echo $addr|cut -d"/" -f2` >>>> 903 >>>> 904 # get network of this interface >>>> 905 if_network=$(calculate_net_addr ${t_ipaddr}/$bits) >>>> 906 if [[ -z $if_network ]]; then >>>> >>>> - Should the above code factor out the IPv6 interfaces as these will not >>>> parse correctly to get the t_ipaddr and bits lines? Also should tunnels >>>> be parsed out for similar reasons? >>>> >>>> usr/src/auto-install/ai_get_manifest.py >>>> >>>> - Please add a space between the # and Network in the below code: >>>> >>>> 56 #Network commands >>>> 57 IPADM = "/usr/sbin/ipadm" >>>> 58 DLADM = "/usr/sbin/dladm" >>>> >>>> - Please reword the log message: >>>> >>>> 395 AIGM_LOG.post(AILog.AI_DBGLVL_ERR, >>>> 396 "Could not obtain name of valid network interface") >>>> >>>> to "Could not obtain a valid network interface name." Or something >>>> similar. >>>> >>>> On 04/ 5/12 11:05 AM, Nirmal Agarwal wrote: >>>>> Hi all >>>>> >>>>> A gentle reminder, I would appreciate if someone can review this fix. >>>>> >>>>> >>>>> Thanks >>>>> Nirmal >>>>> >>>>> >>>>> On 4/3/2012 5:28 PM, Nirmal Agarwal wrote: >>>>>> Hi all >>>>>> >>>>>> Can I please get a code review for CR 6996539. >>>>>> >>>>>> 6996539 Convert AI to use ipadm instead of ifconfig >>>>>> >>>>>> >>>>>> Webrev : >>>>>> https://cr.opensolaris.org/action/browse/caiman/nirmal27/6996539/webrev/ >>>>>> >>>>>> >>>>>> I have removed get_ip_netmask() and find_network_nmask() from >>>>>> installadm-common.sh. I couldn't find reference to them in the gate. >>>>>> Let me know if I missed something. >>>>>> >>>>>> Pep8 is clean and pylint output is unchanged. >>>>>> >>>>>> Testing : >>>>>> >>>>>> -> Created custom image and tested client boots and sets the >>>>>> criteria correctly. >>>>>> >>>>>> --> installed the installadm packages from nightly and created >>>>>> a service. >>>>>> >>>>>> Let me know if I need to run some other tests. >>>>>> >>>>>> Thanks >>>>>> Nirmal >>>>>> _______________________________________________ >>>>>> caiman-discuss mailing list >>>>>> [email protected] >>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>>> >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> [email protected] >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>> >> > _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
