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.
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. > > 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 >>>> >>> >> >
