Hi Ginnie,
Apologies for the late response.
Page 4 - Section 4.1
Bullet point 2 - I think it's more correct to say "implement"
rather than to say "be a member of":
"Each chechpoint must implement the install engine's abstract..."
Page 4 - Section 4.2
Probably best to say:
"... reclocates ICTs to a new solaris_install.ict module."
I think the use of a '/' is incorrect here since it's not a file
path.
Pages 4-6 - Section 5.1 - SystemConfiguration
As Karen also said, I'm not sure that this belongs to this project
but should be part of the SC project to deliver. But maybe it's
more expedient to do it here with their co-operation.
I don't feel that the use of arguments in the __init__ is the
correct approach either, and would suggest creating a
"SystemConfigurationData" object which is what is stored in the
DOC.
This would need to be able to consume an SC profile from an XML
manifest, and also be able to generate the same.
It is this Data Object that you would then consume from the DOC as
parameters to the checkpoint. The source of the object then will
vary depending on the application - in AI's case it would come
from the AI Manifest, but in a UI case, the application would
create it and put in in the DOC.
As such, the methods you outline here, where appropriate, should
take the necessary parameters, e.g.:
_create_new_user(username, ... )
this will allow for better PyUnit testing but also then to have
the execute method really only be the one the deal with fetching
information from the DOC, and passing parameters to the various
utility methods.
If you look at many execute() methods, most will follow this style
of implementation. So in someway this comment applies to all
checkpoints.
Pages 6-7 - Section 5.2 BootConfiguration
I would suggest you talk to Niall about this, since I feel he's
doing much (but maybe not all) of this already through the
provision of the UEFI and GRUB support, which will provide
checkpoints. You seem to mention it at the end of the document,
but I would wonder why you still have it then?
It shouldn't be necessary to take a list of paths as parameters -
it should be possible to determine these from the DOC where TI
gets it's information about the disk layout - i.e. the "desired"
tree.
Similarly for the rpool, since it's a flag on the Zpool object
whether it "is_root" or not.
Do we really need to be able to access the bootenv.rc file? I
wonder whether these should just be the bootards in the GRUB menu.
Also that file is a 'private' interface for the eeprom command,
and whether it should be able to operate an alternative root (e.g.
eeprom -R /a ) rather than us accessing the file...?
Page 9 - Section 5.5
I wonder if this should really be required - is it not enough for
a BE to have the 'reconfigure' flag set and then it would just
create the devices?
I know this isn't specific to this document but I would be
interested in knowing if it's still required.
Page 9 - Sections 5.6 and 5.7
Can these not be combined in to a single checkpoint? They seem to
fit together.
Page 10 - Section 5.9
You should probably be using /var/run here, but there is a recent
PSARC case that is looking at changing this - but for now it's
/var/run rather than /tmp for any of these files.
You seem to be listing other arguments here which aren't listed in
the method definition: kbd_device, kbd_layout, keyboard_layout.
Are any of these values really expected to be modified any anyone
that runs the checkpoint? I would almost consider that these
should just be defined as class constants, or attributes maybe.
Thanks,
Darren.
On 13/12/2010 20:41, Ginnie Wray wrote:
> Hi -
>
> I've added a proposal for converting the ICTs to checkpoints to the
> caiman-docs repository.
>
> It can be downloaded from:
> http://src.opensolaris.org/source/xref/caiman/caiman-docs/ICT/ict_conversion.pdf
> or pulled from the caiman-docs source directly. Instructions are at:
> http://repo.opensolaris.org/info/repository.action?projectId=213&repositoryId=128
>
> I would like feedback by Thursday, December 30. If the holidays
> interfere with this, just let me know and I'll accomodate you.
>
> Thanks,
> ginnie
> _______________________________________________
> 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
