Hi Jean.
On 10/27/08 11:22, Jean McCormack wrote:
> Jack Schwartz wrote:
>> HI Jean.
>>
>> ai_x86_image.xml:
>>
>> 36: Rather than say the "resume_from field", please say "resume_from
>> attribute field of checkpoint_enable" so people know how to specify
>> that field..
> OK.
>>
>> distro_const.py:
>>
>> 228: Nit: IMO calling DC_verify_resume_step() where it was and at
>> line 152+ targets the calls to the specific cases it validates.
>> Where it is now, tthere is an explicit test needed (227) and it is
>> possible that DC_verify_resume_step() gets called extra times. Not a
>> big deal, but not as clean as it could be either.
> The problem is, calling it at line 152+ is a bug. A -r or -R overrides
> the manifest file. So if you call it before you determine if you have
> a -r or -R you will hit the case where the manifest file is illegal
> but the -r is legal and that's the overriding value you wish.
OK.
>
> Why would it get called extra times on line 228? It gets called once
> only. For the stepno that is in the resume_step field. That will be
> the step you will actually resume from.
Oops. You're right. I was thinking it was called as part of the loop
that iterates through the options.
Looks good to me.
Thanks,
Jack
>
>
>>
>> all_lang_slim_cd.xml:
>>
>> same comment as for ai_x86_image.xml
> OK
>>
>> slim_cd.xml:
>>
>> same comment as for ai_x86_image.xml
> OK.
>>
>> Thanks,
>> Jack
>>
>>
>>
>> On 10/27/08 10:26, Jean McCormack wrote:
>>> Code review updated.
>>> http://cr.opensolaris.org/~jeanm/slim_4127_3/
>>>
>>> Tested: manifest file resume_from not there, valid, invalid, names
>>> and numbers
>>> -r valid, invalid names and numbers
>>> -R
>>> Mixing -r and manifest file with manifest file resume_from
>>> valid and invalid and not there and -r valid and invalid.
>>> Mixing -R and manifest file with manifest file
>>> resume_from valid and invalid and not there.
>>>
>>> Jean
>>>
>>>
>>> Jack Schwartz wrote:
>>>> Hi Jean.
>>>>
>>>> I agree with both of Karen's points, and have one of my own as well:
>>>>
>>>> usr/src/cmd/distro_const/distro_const.py:
>>>>
>>>> When -r is specified on the commandline, DC_verify_resume_step()
>>>> verifies that the given step is valid. If -r is not specified on
>>>> the commandline but a resume step is given in the manifest, I don't
>>>> see DC_verify_resume_step() getting called. Add at line 152+ ?
>>>>
>>>> Thanks,
>>>> Jack
>>>>
>>>> On 10/26/08 18:36, Jean McCormack wrote:
>>>>> Can Jack and Karen please review the following
>>>>>
>>>>>
>>>>> Defect:
>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4127
>>>>>
>>>>> Webrev:
>>>>> http://cr.opensolaris.org/~jeanm/slim_4127/
>>>>>
>>>>>
>>>>> Jean
>>>>> _______________________________________________
>>>>> caiman-discuss mailing list
>>>>> caiman-discuss at opensolaris.org
>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>>
>>>>
>>>
>>
>
