On 06/03/11 14:08, Sue Sohn wrote:
On 06/ 2/11 03:28 PM, Jack Schwartz wrote:
Hi everyone.
Here is a webrev of the installadm manpage changes relevant to the
Derived
Manifests project.
Changes to installadm.1m also include some quick reformatting (so the
manpage
prints better). However, reformatting is not complete since Sue will
be doing an
overhaul with the ISIM project soon.
Once it is reviewed, I will push it to slim_source.
Also included, as an extra bonus :) is the aimanifest manpage. This
still in
progress but I'm close to being done with it. Please have a look and
let me know
if you have comments. I plan on giving this to Alta to work her magic
on it
before pushing it to the gate, since I'm the only one working on it.
http://cr.opensolaris.org/~schwartz/110602.1/webrev/index.html
Please submit any comments by Tuesday 6/7 COB.
Thanks for your time,
Jack
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
Hi Jack,
I just looked at the installadm man page.
General: all references to "install service" should say "installation
service"
16 Remove '...'
OK. Only one prop at a time...
29 Change:
<manifest/script_file> -> <manifest/script file>
or
<manifest/script_file> -> <manifest_file/script_file>
(to get across the idea that the manifest is a file)
OK.
Changed all manifest/script_file -> manifest/script file
Changed all manifest/script_name -> manifest/script name
35 Please add --options, such as -n|--service <svcname>.
This would apply to the other commands, but as I've already added them
in the isim gate, you don't really need to do it here.
64 Need to add the -o option for export:
-o|--output <pathname>
Thanks. Fixed.
287 remove '...'
Fixed.
289-292 How about simply:
Sets a property of a service to a specified value.
I'm not exactly sure of the intended meaning of the second sentence,
but I doubt it would apply to all properties.
This is what I had (minus the alias portions)- you're welcome to use
all or parts of it.
installadm set-service -o <prop>=<value> <svcname>
Sets a property of a service to a specified value.
-o|--option <prop>=<value>
Specifies the property and value to set.
prop=value can be:
default-manifest=<manifest/script name>
- default-manifest=<manifest/script name>
The default manifest of a service may not have
criteria actively associated with it while it is
the default. If one uses installadm set-service
to make a manifest the default, then its
associated criteria become "inactive", and are
not considered during manifest selection.
Inactive criteria are clearly indicated by the
installadm list command. If a different manifest
is later made the default, the criteria of the
formerly default manifest becomes active again.
<svcname>
Required: Specifies the name of the installation
service whose property is being set.
I changed all but the big paragraph for this subcommand. However, I
added a paragraph in the DESCRIPTION section explaining what a default
manifest or script is. Why explain repeatedly what a default is, when
explaining it once in the description section is most succinct and
clear, and all that's needed?
346-358 If you are going to add this level of detail for -m, you
should also add it for -p.
OK. Added the following to -p:
When -n is not specified, display an abbreviated
listing per service that includes the profile names.
When -n is specified, display the profiles for the
requested service along with their criteria.
424 Change inst_name to man_name
OK
433 refered (typo)
Fixed.
443 not your changes, but please remove extra '-c' inside of <>
Actually the command would imply one could specify many crit=value pairs
with a single -c, which isn't true. I removed the extra set of <> as
well as the extra -c, and added that "A command can specify -c multiple
times." I did this for set-criteria, add-manifest and create-profile
subcommands.
Processing this change pointed out an issue where if someone happens to
put multiple criteria in quotes:
... -c "arch=x86 mem=2048"
installadm treats this as arch = "x86 mem=2048"
or a single criteria "arch" with a value of "x86 mem=2048"
which may be confusing to people...
461 Use "ignored" instead of "inactive", since that is how it is
described elsewhere and in the list output
Thanks. Fixed.
471-472 How about: Any associated criteria or default status remain
with the manifest following the update.
Yes, that is clearer. Thanks.
474-483 I don't understand this wrt how update-manifest works. I
thought that update-manifest updates the contents of a given manifest,
specified by the required -m option. I didn't think the name would
change. We discussed this offline and you're looking into it, but I
think this section should probably be removed and -m should be
required (small code change)
OK, I think things are OK codewise, but I needed the change the
manpage. This command will *not* change the ai_instance/name attribute
of an existing manifest, but will only update the contents, which is
what is it supposed to do. The manpage now reads as follows:
The name of the manifest or script to be updated
is derived from (in order):
1) installadm update-manifest -m option, if
present
2) manifest name attribute, if present in the
manifest and it matches one in an existing
manifest, i.e.
<ai_instance name="inst_name">
3) manifest/script file name, if it matches the
manifest name attribute in an existing manifest,
or the name given by installadm list if it matches
the name of an existing script.
495 per above, this should likely be Required, not Optional
622 usage should match actual usage (different order of options, usage
has <output>, not <output_pathname> )
I'll change the usage, as output_pathname is clearer.
642 <output_pathname> again
636 It is confusing to mention profiles as part of -m text and to
mention manifests as part of line 652 text. Maybe you can remove the
"Any number of..." text from both -m and -p and add it the general
description.
I simplified both -m and -p descriptions by removing the "Any number
of..." text and saying that "A command can specify -m (or -p) multiple
times." The command's description already said that "At least one
manifest/script or profile must be provided."
646 need space after comma
Fixed.
963 - Check with Alta if this is the name to use.
Changed to "Installing Oracle Solaris 11 Systems" per Alta.
971 your installadm add-manifest command is missing the "-n
service_092910"
OK. Changed to:
(using installadm add-manifest -m my_manifest -n service_092910 ...)
The idea is to show only the relevant parts of the command to keep it
simple.
1182 stored in the file -> stored in the files
Fixed.
1206 install the changes -> then update the manifest in svc2
OK.
1211 remove "with you favorite editor"
OK.
1214 Remove backslash
Thanks.
I'll repost at the end of the code review.
Thanks again for your review.
Jack
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss