Jack Schwartz wrote: > 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. Whew. Thanks.
Jean > > 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 >>>>>> >>>>> >>>> >>> >> >
