On Mon, 8 Dec 2008, William Schumann wrote: > Alok, > I added TB and GB units and tested and updated the webrev. Thanks > William > Alok Aggarwal wrote: >> >> On Tue, 2 Dec 2008, William Schumann wrote: >> >>> Hey Alok, >>> I have been reading the AI XML parsing code. Is there an easy way of >>> launching the validator from a development system like indiana-build, >>> or do I need to do this on a client? >>> >>> I read a comment somewhere about using the DC validator in the >>> future. Anything new on this? >>> >>> I've been using the Jing validator (seems to work OK, uses Relax NG >>> schema file, but doesn't know about .defval.xml, I think) and the >>> xmloperator editor (supposed to validate against rng file, but buggy >>> for me). >>> >>> Also, xmloperator prepended: >>> <?xml version="1.0" encoding="UTF-8"?> >>> to ai_manifest.rng. It seems logical that we use UTF-8, since we're >>> international, but perhaps our character set for the manifest still >>> shouldn't take anything other than ASCII. What do you think? Probably >>> not so important, eh? >>> William >>> >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4460 >>> http://cr.opensolaris.org/~wmsch/bug-4460/ >>> >>> Added ability to specify partition size units of either sectors or >>> megabytes. >>> Created new Relax NG element: partition_size_units, defaulting to >>> megabytes >> >> So, I think in general we should also allow >> gigabytes and terabytes as the units since they're >> quite valid. Does it make sense to get rid of >> the sectors notation entirely? I mean do people >> really prefer this over mb, gb or tb? >> >> I also think applying this extension to slices >> at this time makes sense, it sounds kinda odd >> that partition units can be specified but slice >> units can't be. >> >> auto_install.c: line 310: I haven't looked at this >> closely but don't you want something like MB_TO_BLOCKS here? >> Or, is just a matter of naming the define that instead of >> BLOCKS_TO_MB? > You're right. There shouldn't be a constant here, but a formula. >> >> auto_parse.c: Why not just name the function >> ai_get_manifest_partition_units() to match what the >> rest of the code does? > Yeah, it's since there is so much code duplication. I wrote a generic > function that just took the element as a parameter. Is there a > particular reason why you broke it up into so many different functions? > If I can get you to see it my way, I'll make them all use the same > function. If you win, I'll change the new function name as you want. :) > > Thanks for this little pre-review. :) > wm
ai_manifest.rng: line 1, 24, 31: Why are these changes needed? ai_manifest.rng: Have you verified that if you specify any other value for the partition_size_units field, it fails during the validation phase? ai_manifest.xml: line 53: move this one line up so it's next to the partition_size field. auto_install.c: line 49, 50, 305: const not needed auto_install.h: line 75: CONVERT_UNIT_TO_TEXT may be more readable. auto_parse.c: line 405, 410, 415, 420: How are the capitalized versions even allowed given your schema? Alok
