John,
usr/src/cmd/installadm/Makefile:
98, 105: The target 'all' defined twice. Can you combine them?
usr/src/cmd/installadm/installadm.c:
994-1006: The comment indicates that a new error message is not printed
if the return code is 1 but the code at line 1000 checks for return
value '256'. Which is correct?
usr/src/cmd/installadm/installadm.h:
55: Just a nit -- move the right side of the definition so that it is
aligned with existing definitions.
usr/src/cmd/installadm/list.py:
General: It is assumed that tftp directory is /tftpboot and it may not
be true. This directory is defined with tftp service in /etc/inetd.conf.
The default value is /tftpboot but it could be some other directory too.
So please check /etc/inetd.conf for the tftp directory. Check
usr/src/cmd/delete_service.py:findTFTProot() for more information.
64-67: Why we need a version number for list?
68: Can you change name --> service (or service name) so that it will be
clearer
50-89: In line 60, the usage indicates that there is an option '-h' but
the parser doesn't process '-h'
91-109: installadm.c uses the existence of platform/sun4v or
platform/i86pc to determine the architecture. Please use the same method
to find the architecture of the image. platform/{arch} is more stable
than boot/multiboot and also it will be consistent with the rest of
installadm code.
154: get_manifest_name has two input args but the comment in 156
indicates there are 3 input.
164: Assumes that the port is always 5 characters (digits). This may be
true now but may fail if it changes. Use the method at line 143.
184: The comment at 156 indicates that the functions returns void where
as it returns service dictionary.
215: same comment as 164 above
211, 223: Width is initialized to 0 twice
236: same comment as 164 above
340: Only x86 information is stored in menu.lst. The sparc information
under /etc/netboot directory. If you are looking for install_service and
install_svc_address for sparc, they are under $image/install.conf. Also
in sparc, these values are service specific rather than the client
specific and a bug 7152 filed for this issue. I think it is better to
store this information in SMF install service when a service/client is
created and retrieve it from there for list.
393: The comment above applies to get_clients(). The sparc clients are
under /etc/netboot.
485: This may not be the right check since the service menu.lst is also
in the same directory. If there is an X86 service, the directory should
exist.
usr/src/man/installadm.1m.txt:
The diff doesn't look right. The old one is txt and new one is troff?
Thanks,
Sundar
John Fischer wrote:
> All,
>
> Updated webrev for this set of changes at:
>
> http://cr.opensolaris.org/~johnfisc/list
>
> These changes include feedback during a phone conversation from Ethan
> as well as the installadm man page change to document the list
> subcommand.
>
> Thanks,
>
> John
>
> John Fischer wrote:
>> Ethan, Clay and Sundar,
>>
>> When you get a chance can you review my webrev at:
>>
>> http://cr.opensolaris.org/~johnfisc/list
>>
>> These changes are for the installadm list subcommand fix and
>> will address:
>>
>> 4330 - installadm list should show which server provide the install
>> services.
>> 4113 - nice to have an option to show what service the client is
>> using
>> 4298 - installadm list -n should give different output if there is
>> no custom manifest
>> 4597 - installadm list: expect usage statement when giving service
>> name without "-n" flag.
>> 4646 - list: showing added manifest for a non running service.
>> 5300 - list: does not show informational message for non running
>> service.
>> 8496 - list: no verbiage indicating that a service does not exist
>> when a non-existent service is given.
>> 8529 - 'installadm list' command lists same service three times
>> 6811 - list: should have similar output between list and list -n
>> 8015 - list-manifests output is hard to read
>> 9094 - installadm list: prints colons for empty MAC fields, and
>> doesn't account for colons or periods in MAC field's width
>> 4175 - install list error slips out
>>
>> The changed and added files are:
>>
>> usr/src/cmd/installadm/Makefile
>> usr/src/pkgdefs/SUNWinstalladm-tools/prototype_com
>> usr/src/cmd/installadm/installadm.c
>> usr/src/cmd/installadm/installadm.h
>> usr/src/cmd/installadm/list.py
>>
>> These changes also include a change to the Makefile and prototype_com
>> file for the delete_service and delete_client to be consistent with
>> the other python scripts within the /usr/lib/installadm directory.
>>
>> The remote service listing is being postponed due to a timing issue
>> with multiple remote services that grows with the number of services
>> within the domain.
>>
>> Thanks,
>>
>> John
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
