Dave,
Thanks for the review. Comments/questions inline.
On 03/18/10 03:45 PM, Dave Miner wrote:
> Jean, some comments/questions:
>
> Generally, where will the specific names for the parameters from the
> object cache referenced here be specified (if it's not a future
> version of this spec, then which one would it be)?
>
It really should be in this doc. In fact, I believe the structure will
be clearer if I also include what the xml would look like.
> 3.1 Mention towards the end of this section of "common methods like
> src, dst" but no further discussion in the document.
That was leftover from the previous version and should have been removed.
>
> 3.3 The table here seems to be using ARC interface classifications,
> but the terminology is not current. I'm also unclear as to why the
> SVR4 interface would be classified differently than the others.
The SVR4 interface shouldn't have been different. I'll look up the new
interface classifications and update accordingly.
>
> 3.4.1
> What is a "data cache node" (in other words, can you be more specific
> about the class that this is based on the draft terminology from the
> Data Object Cache work)? Is the application responsible for passing
> it in, or does the engine do so?
The engine is responsible for the call to create_transfer. The data
cache node is the child in the data cache that
is specific to this transfer/checkpoint. I'll work on making this
clearer and with words consistent with DOC.
>
> What about a common interface for obtaining size requirements for the
> bits to be installed?
My thought was that the get_progress_estimate method was the place where
this functionality was required.
Since this is TBD I didn't want to go deep and have it all be wrong
although I will if you'd like.
Or are you thinking a more general interface than that? i.e. we would
need the functionality in other places?
> How would the application obtain the lists of files that are
> referenced here? Would it make sense for the class to be able to do
> this itself by defining an interface that the media might provide?
I wasn't clear. I meant a file containing this list. My thought was
that the application would keep a file that listed the files to be
transferred within it. That way the list is associated with the
application and easily modified by the developer.
>
> I'm not clear on whether, for something that looks like the live CD,
> there's a single cpio transfer invocation, or multiple. The file list
> noted here seems to indicate there might be multiple, but the use case
> in 3.6 seems to indicate a single one.
I think this is an area where showing an xml tree would really help. The
application could make 1 call to
the checkpoint cpio transfer module. With the cpio transfer module,
there may or may not be multiple calls
to cpio. Something like this:
<transfer>
<file_list_file = "/tmp/file.txt"/>
</transfer>
<transfer>
<file_list_file = "/tmp/file2.txt"/>
<cpio_args = "-pdm"/>
</transf-
In this case, the application would make one call to cpio.execute()
which would read the xml data and perform 2 actual
cpio calls, one to tranfer the files in /tmp/file.txt with the default
cpio_args, the other to transfer the files in
/tmp/file2.txt with specifc cpio_args.
It is possible to have this:
<transfer>
<file_list_file = "/tmp/file.txt"/>
<file_list_file = "/tmp/file2.txt"/>
</transfer>
Again, I think it would be simpler to have the cpio.execute() call
perform 2 cpio calls for this case instead of merging
the files. Not sure if this falls within leaving it up to the
implementer or should be called out.
>
> WRT to the do_clobber functionality here, would it be a good idea to
> define an interface here for that specific implementation to be
> associated with the media instead of embedded in the class?
Yeah I think it would. To make sure we're communicating, something like:
<transfer>
<clobber> True</clobber>
......
Is that in line with what you're thinking?
>
> Would it be a good idea for the dryrun to allow for copying to
> /dev/null rather than not executing at all?
Interesting thought and I could see how it would be useful to have data
somewhere for the checkpoints downstream.
So if I'm given a destination of /rpool/dc then instead of installing
the packages there, I would
install to /dev/null. Then do the next checkpoints just "know" to look
in /dev/null? Do I modify the destination to be
/dev/null?
>
> 3.4.4.2
> I'll re-express here the concern I had with the first draft, which is
> that any parameters you specifically define here are potentially
> duplicating (and thus potentially diverging from) portions of the pkg
> API. Would it make sense to instead expose access to the pkg
> ImageInterface (or any other classes) and allow the application to
> directly manipulate properties of those classes? I haven't spent much
> time looking at the specifics to see if this would work or not, but it
> seems important to consider.
The current API doesn't appear to have attributes as part of the
ImageInterface class that can be accessed to
set the options. They are arguments to the method calls. I'll look into
your suggest of using dictionaries to implement
the options.
Jean
