On 11/ 9/10 08:46 PM, Karen Tung wrote:
> Hi Darren,
>
> Please see my response inline.
>
> On 11/ 9/10 09:32 AM, Darren Kenny wrote:
>> Hi Karen,
>>
>> Thanks for the review.
>>
>> On 10/25/10 11:55 PM, Karen Tung wrote:
>>> Hi Darren,
>>>
>>> Very nice and clear spec, I do have a few comments:
>>>
>>> - The engine provides the ability to stop/resume. I know that's probably
>>> not a normal use case for the AI application. Is there any plan to
>>> provide that as a private feature somehow for the AI developers or
>>> those debugging AI to save sometime?
>> It's certainly very likely since I'm sure Matt and myself could use it too,
>> should I be documenting this in the design, or just in a README?
>>
> I asked that question because I was wondering whether you are
> going to provide some sort of "interface" to use the stop/resume feature.
> I guess if it is something that others can use as well, it would be
> better to document it in a README or something that the users will
> read, since it is unlikely users will be reading the design spec.
Ok, I'll use a README to document such features.
>
>>> - Section 3.3: dry-run: will it be supported at all? If so, how?
>> Forgot about dry-run - certainly if checkpoints support it - but I wonder how
>> much is feasible in dry-run mode only when doing an install - especially
>> around
>> the TI stage...
>>
>> AFAIR some checkpoints (via the Engine) support dry-run, so it should be
>> possible to use that, shouldn't it?
> The engine passes the dry-run flag specified by the application down to the
> checkpoints. It is up to the checkpoint to interpret and provide the
> dry run support.
I'll add a dry-run flag to the application, but each checkpoint will need to
ensure that they handle the flag appropriately.
>
>> Still, it's a developer feature rather than end-user, isn't it?
> I am not sure whether you can qualify that as an end-user or
> a developer feature. Dry-run is a feature that was requested
> by the test team. I imagine that will be a very useful feature
> for developers as well. Might also be useful for debugging. For example,
> if I am trying to figure out why a certain disk is selected, but
> I don't want the data on the disk to be destroyed.
Agreed.
>> I may look at adding a 'developer' appendix if it's useful - what do you
>> think?
>>
> IMO, we need to distinguish between supported interfaces,
> and unsupported interfaces. It is up to the user, regardless developers
> or end users, to use those interfaces. Then, it will be useful to show
> some use cases of how developers and end users might use these
> support and/or unsupported interfaces to do their work.
True. Supported interfaces will be documented in the design, and visible in the
usage, etc.
I will document the unsupported interfaces in the README.
>>> - Section 3.3: "Run the install" section, will progress be reported as a
>>> percentage? Or
>>> people just see progress because they see something posted in the log file?
>>> Is the AI app going to register a progress handler with the logger to
>>> pro-actively
>>> get the progress and log it?
>> Good question, and Dave also raised this (due to some bugs in this area).
>>
>> I've not figured out how to report progress, other than the current way
>> which is
>> via the log file, which is the current solution.
>>
>> I will probably send out an e-mail to discuss this further.
> OK.
>>> - Section 3.3.1.1: "Register checkpoints" bullet. It would be useful
>>> to list the exact list of checkpoints and their names used in the AI app
>>> in the design spec
>>> somewhere to make sure that the if we ever change the names for
>>> <software> in the manifest,
>>> we know what might get affected in the code.
>> I've been trying to explicitly avoid the need to expose this.
> Well, since this is the design specification, I thought it would be a
> fair place
> to put it. That way, the reviewers of the design can confirm that the
> right set of checkpoints are used in the right order...etc...
>
> I completely understand that this is not going to be an interface for
> the users.
> So, we won't put it in any user documentations or any configuration files.
That's fine then, as long as it's not seen as part of the API :)
>> While I know the Transfer Software nodes need a matching checkpoint, I've
>> been
>> talking with Ginnie and Jean about having a Transfer factory method that
>> would
>> generate the checkpoint from the software node - that way the name (if
>> provided)
>> in the manifest doesn't have to match the internal name of the checkpoint,
>> rather it's likely that the checkpoint will do the opposite and match the
>> name
>> in the manifest...
>>
>> Also, there is not requirement to have a name in the manifest, so that's also
>> something to be looked at - and generating dummy names if none is specified.
>>
> Good idea.. I don't really like matching Software node names and checkpoint
> names either, but that was the best solution we came up with earlier.
> I am glad that we are revisiting that again.
Me too.
>>> - Section 3.3.1.1: "Execute the install" bullet. This indicates that all
>>> checkpoints will be executed without interruption by the engine.
>>> Will the checkpoints be executed with stop-on-error equal to true?
>>> It might be useful for stop-on-error to be false for some of the ICT
>>> related checkpoints.
>>
>> That's interesting - I would have thought that it better to fail totally than
>> continue in an AI case, I'll mention it anyway.
>>
> When we worked on the text installer earlier this year, I remember we
> had to make a change to handle cases where we allow the installation
> to complete despite some non-critical ICT failures.
> So, I think this will be an interesting area to consider, not just for
> AI, but
> for the other installers as well.
>
That's good to know.
>>> - 3.3.1.2: Why is the AIFileHandler Class needed? You can just add a
>>> new FileHandler pointing to a location other
>>> than where the default log is, if you don't want to use the default log.
>> Agreed, we just copied this from DC :)
>>
>> But after looking close, there is good reason for it in DC since the output
>> directory might not exist, so it caches logged messages until it does
>> exist...
>>
>> So to sum up, this doesn't apply for us, so removed it.
>>
> OK.
>>> - 3.4.2: second bullet, 2nd sentence. This talks about checking every
>>> <software> node
>>> has a corresponding checkpoint. For DC, it is about consistency,
>>> because the DC user
>>> specifies both the checkpoint name and the software node name. So, we
>>> want to make sure
>>> they are the same. For AI, the user only have control on the<software>
>>> node in the manifest.
>>> They should not have knowledge or control over the name that's used for
>>> the checkpoints.
>>> For the AI app, I believe it's important to make sure they are
>>> consistent in case either the
>>> AI app or the manifest is updated in the future, and things get out of sync.
>> As mentioned above, we are looking at ways of not having to care about the
>> name
>> of the<software> node in the non-DC applications. If this works we don't
>> need
>> to worry about there being a need to match the names in the user's XML
>> provided,
>> it'll be taken care of internally - and by the Transfer module.
>>
>> I'll put in some text to describe this proposed changes.
>>
> OK.
>>> - 3.4.3, first bullet, "DeviceDriverUpdateLive". I asked Jack, he told
>>> me that the result
>>> of "search all" gets saved somewhere after it is applied to the live image.
>>> That way, after install, the same drivers will get installed to the new
>>> BE without
>>> having to do the search again. Will these results be saved in the DOC
>>> somewhere?
>> The exact details of that I've not figured out, I do know that they are
>> remembered from the Live BE install - the DOC would be the correct place to
>> put
>> it ideally, but that will depend on what the current DDU python code does.
>>
>> I've added this to the document here too.
>>
> OK.
>>> - Section 3.4.8.2: which class will be registered with the engine?
>>> BootConfigurationBaseCheckpoint
>>> which will make decision based on whether the architecture is x86 or
>>> Sparc? or
>>> the actual BootConfigurationSPARC and BootconfigurionX86 checkpoint will
>>> be registered?
>> The one registered would be the one that is appropriate.
>>
>> The call would be something like:
>>
>> (cp_path, cp_class_name) = BootConfiguration.get_checkpoint_info()
>> engine.register_checkpoint("boot-config", cp_path, cp_class_name)
>>
>> So the get_checkpoint_info() method would get the appropriate one.
>>
> Sounds good.
>>> - Section 3.4.8.5: Just curious, are we going to use the manifest-writer
>>> in anyway for this?
>> At the moment, no - I'd like to focus on the replication of existing
>> functionality, but it's certainly something we can look at later.
>>
> OK.
>
Thanks,
Darren.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
