Glenn Lagasse wrote:
> Hi Sundar,
>
> Thanks for the comments! I know Joe responded to the comments about the
> finalizer scripts in particular. I'll address the DC specific ones.
>
> * sundar Yamunachari (sundar.yamunachari at sun.com) wrote:
>
>> Glenn Lagasse wrote:
>>
>>> The VMC project is proud to present it's code for your review. You may
>>> access the webrev at:
>>>
>>> http://cr.opensolaris.org/~glagasse/slim_vmc/
>>>
>>> I'm also attaching the DC log file for a VMC run as well as the console
>>> output for the same run for the curious as to what this looks like in
>>> practice.
>>>
>>> The timeout for providing review comments is COB October 23rd, 2009. If
>>> you need more time, please let me know.
>>>
>>> Thanks!
>>>
>>> ------------------------------------------------------------------------
>>>
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>
>> Glenn,
>>
>> usr/src/cmd/distro_const/DC-manifest.rng:
>>
>> Is there a reason why vmc fields are commented in the schema files
>> (lines 65-67, 299-305). Are these definitions are not used or not
>> needed?
>>
>
> I answered this for Jean. I'll include the response I sent her here.
>
> Somehow I missed this when I was reviewing the webrev prior to publish.
> There actually aren't any vmimg_params for VMC. Everything is set as
> arguments to the VMC finalizer scripts. So the block 52,73 should look
> like:
>
> <attribute name="name"/>
>
> <!-- General distro-constructor parameters -->
> <ref name="nm_distro_constr_params"/>
>
> ! <optional>
> ! <!-- Parameters for building live
> image -->
> <ref name="nm_img_params"/>
> + </optional>
>
> The effect of which makes nm_img_params optional for all manifests since
> it's not needed for VMC manifests (and there was no good way I could
> come up with to make nm_img_params mandatory for the AI/SLIM manifests
> but not for VMC manifests).
>
> That said, Keith raises an interesting thought in that in order to allow
> manifest validation of a sort to actually occur we might want to include
> an empty vm_img_params. I need to think about this some more. At the
> moment, what I said to Jean is what I'm planning to do.
>
okay.
>
>> DC_defs.py:
>>
>> You got me confused here. Changing DISTRO_PARAMS -> IMG_PARAMS and
>> IMG_PARAMS->DISTRO_PARAMS ;-)
>>
>
> Sorry about that :-) Since I had to move elements around in those
> sections, that also necessitated changing some things here.
>
>
>> usr/src/cmd/distro_const/utils/im_pop.py:
>>
>> General comments: The pkg set-authority is changed to set-publisher.
>> Do we need to convert authority to published for this project?
>>
>
> No. That will get cleaned up at a later date.
>
okay.
>
>> 80: Why you have a 'pass' here?
>>
>
> This code came from distro_const.py initially. Apparently it wasn't
> considered a real problem if the .image_info file wasn't created during
> image construction. If we're going to keep that behaviour, it should be
> return instead of pass. I think however that we probably should stop
> the build if the .image_info file can't be created. I'll talk to Karen
> about it.
>
okay and let me know what you decide.
>
>> 107: Can you add some comments to the function 'dc_ips_unset_auth'
>>
>
> I can. Apparently it was an oversight when this was originally written.
>
>
>> 180, 188, 197, 208, 220: Comments the functions in those lines
>>
>
> More existing code that didn't have initial comments. I didn't write
> any of this, but I can take a stab at it.
>
>
>> 315: publisher is used here and authority is used in all other places
>>
>
> Good catch. I'll go ahead and change the publicly displayed authority
> strings to publish (since that's what it's called now). I'm not going
> to fully re-work everything at this time however.
>
> Thanks Sundar!
>
>
Thanks,
Sundar
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20091022/f8abd047/attachment.html>
