Hi Karen,
Thanks for reviewing.
On Tue, 13 Jul 2010, Karen Tung wrote:
Hi Alok,
Here are my comments:
Section 1.1:
-------------
General comment about this section is that it needs to have a lot more detail
about what is changed and what is not changed by this work. There are bits
and pieces of what is different throughout the document, but I think it is
useful to summarize all the simlarities and differences in 1 single location.
For example, some of the things that I personally would find helpful is:
* User experience: is there any change? Will everything that's currently
supported continue to be supported? Will the user notice any difference
in using the DC app?
* A definition section that maps between old DC terminology to
new terminology would also be helpful.
Okay, I will add explicit details on what is
changing as a result of this design.
Section 1.2.3:
----------------
last sentence of the 2nd paragraph:
"The DOC will also be used to rollback to a checkpoint and
resume the build process from there.".
I believe that's how the engine will use the DOC to provide the stop/resume
functionalities. DC will not do that. So, I don't think it should
be mentioned here.
The DOC's rollback/snapshot functionality is
an indirect dependency for DC. The engine is
indeed going to be responsible for calling the
necessary DOC interfaces to rollback/snapshot.
I will rephrase the above so the intent is clearer.
Section 2.2
----------------
Since this is the specification for what DC will EVENTUALLY be. I think this
section should have a more specific title, such as "non-goals" for first
release of DC or something. The way things are sepecified right now makes it
look like those are the things that you will never do.
I considered that but doing it that way I think
could be confusing. If I don't call out a particular
feature to be a non-goal, I will be expected to outline
the design for the feature somewhere in the document
(regardless of whether design has been done on that
feature or not).
So, IMO it's clearer to state it this way and as we
enhance DC to provide more features, this design can
be updated accordingly at that time.
Section 2.3, The Data Object Cache bullet
-------------------------------------------
"interfaces to rollback and snapshot": Will those be directly used by DC?
If so, how? and for what purpose?
The interfaces won't actually be used by DC but they're
an indirect dependency. I'll re-word it.
Section 2.3, The Install Engine bullet
-------------------------------------------
What about the functionality to list checkpoints available for resume, and
stop/and resume?
I'll add that.
Section 3.1, 2nd paragraph, where it talks about the loggers
-------------------------------------------------------------
Current DC also display the content that would go into the simple log
by using a console logger. Is the new design going to provide that
functionality?
Yep, I'll add that as well.
Section 3.1, 4th paragraph
---------------------------
After the manifest is parsed and stored in DOC, before or after DC
registers the list of checkpoints, DC also needs to set the "dataset"
property in the engine so the engine knows which dataset it should snapshot
to provide the pause/resume capability
Wow, that's a glaring omission. Where is this specified
in the engine document?
Section 3.1, 6th paragraph
---------------------------
The DC app's role is to drive the image creation process. It should not
be aware of what checkpoints are being executed, and what they do.
So, I don't think this paragrah should belong here, since this section 3.1
is about what the DC app do, not what checkpoints do.
The fact is that it has to. Initially I had the same mindset
as you do in that I believe DC to be pretty much a "dumb"
app. Once we started doing some initial implementation, it
became clear that DC has to get into the business of knowing
exactly which checkpoints will be executed.
Consider the case of compression mechanism to be used. It
is set in the dc_spec section of the DC manifest even though
it gets consumed by one of the boot archive checkpoints. It
seems cleaner to not have the boot archive checkpoint know about
the dc_spec objects in the DOC if it could instead be specified
as part of a different object in the DOC.
Section 3.1, 8th paragraph
---------------------------
2 distinct topics are discussed in this paragraph. One topic is on how
DC and checkpoint retrieve data from DOC. The other topic is on how
DC handles interrupts. I think this paragraph should be split into 2.
Changed.
Section 3.1
-------------
- It is not discussed in this section, but I think a discussion on what
will DC do after it calls engine.execute() is needed. Will it block
and wait for that call to return? Will it provide a callback function
for the engine...etc...
I think it will just block for the call to return. I
will specify that in the document.
Section 3.1, the diagram:
--------------------------
As I have commented elsewhere, VMC is not an app, it is just one of
the output type of the DC app. Also, the order of the
"core application classes" should be listed to reflect the actual order.
Will change.
general comments for DC checkpoint sections: (section 3.5)
------------------------------
1) Some of the checkpoints are used "internal" to the DC implementation,
such as manifest_parser and target_instantiation. Others are
user specified checkpoints via the manifest. I think we should
make a distingtion between these checkpoints, and document how the
DC app treats them. For example, will the internal checkpoints
show up when you do "distro_const -l"? Will people be able to
resume from those...etc..
Actually I intend on exposing all the checkpoints via
the manifest including manifest_parser and target_instantiation.
Just seems cleaner that way and makes the build process
more observable.
That is why you see all the checkpoints listed in this
section.
2) For each the the checkpoints, I think it would be useful
to list the following information:
* checkpoint name - the name users use for the -r and -p arguments,
and the checkpoint name registered with the engine, if different.
* Name of the Checkpoint class that will implement the checkpoint
* If that checkpoint reads info from DOC or writes info to DOC,
those should be listed as well.
* If that checkpoint takes arguments, it would be useful to
list all the required or optional arguments it takes.
Good idea, will add. Some of this was mentioned by Darren
as well.
3) In many checkpoint descriptions, you mentioned that XXXX will have a
mod_name
of XXXXXX. For example, in section 3.5.3, it is mentioned that
"The checkpoint named 'transfer_ips' will have a mod_name
of the CUD transfer module and will....". What is this "mod_name"
value for?
The mod_name corresponds to the 'mod_name' for each
checkpoint in the execution of the DC manifest.
4) Quiet a few checkpoints tries to put all the general stuff into
a "parent" checkpoint, and then, have the media specific modifications
in a child checkpoint that will call the parent checkpoint's execute()
method. I am concerned about the fact that a checkpoint is trying
to act like an engine, and calling another checkpoint's execute() directly.
Just like our previous discussion of calling the transfer
module's execute() directly, a checkpoint that calls another's
checkpoint's execute() directly might make progress reporting inaccurate.
Good point, instead of calling execute(), I'll restructure
such that a different API is used.
For example, if the PrePkgImgMod.execute() reports progress to the
logger, the logger will call the engine to normalize the progress
information, and the engine won't know that PrePkgImgMod checkpoint
is running.
In such a case, I would expect for the sub-classes',
AIPrePkgImgMod/LiveCDPrePkgImgMod, get_progress_estimate
to account for time elapsed in calling the parent class
API.
Section 3.5.2
---------------
Currently, the boot_archive_archive script also calls libti to release
the mount point for the root archive libti created. What component will be
responsible for that in the new design?
The boot_archive_archive checkpoint will be responsible for
calling the appropriate TI API to release the boot_archive.
I'll add this to the doc.
3.5.5 boot_archive_initialize
------------------------------
The current boot_archive_initialize.py script also creates a few
directories after cpio is done. Where will those code be moved to?
That code is now part of boot_archive_configure.
3.5.7 plat_setup
------------------
Why not merge this script into the general boot archive configure script?
Just check for sparc, since this always has to be done for sparc.
Good point, I'll make the change.
3.7 User supplied checkpoints
-----------------------------
There's no discussion in the document about where all the checkpoints
we supplied will be stored. In the same token, I think we should discuss
whether there's any recommended spot for people to store user supplied
checkpoints. The engine provides the functionality to issue a warning
if it is loading checkpoints from any directory that's outside of sys.path.
Is the DC app going to utilize that functionality?
All the checkpoints will be delivered under (including
user specified ones):
/usr/lib/python2.6/vendor-packages/osol_install/distro_const/checkpoints
I'll add this to the doc.
3.10 Logging
--------------
It is not discussed, but is it implied that the simple and detail logs will
have the same file name that's being used now? Also, will new simple and
detail
logs be created for each run of the DC app?
Yes and yes.
I'll clarify this as well in the doc.
Thanks again, Karen!
Alok
_______________________________________________
caiman-discuss mailing list
caiman-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
