Matt, Looks perfect! :)
Luis Matt Keenan wrote: > Ethan/Luis, > > Thanks for the review. > > I've updated comment, and re-generated webrev. > http://cr.opensolaris.org/~mattman/bug-10115-12960/ > > cheers > > Matt > > > On 03/29/10 03:19 PM, Luis de Bethencourt wrote: >> Hi Ethan and Matt, >> >> I agree with Ethan. The second part of the comment says: >> >> 2596 + * There is an assumption that for this function to be called >> there >> 2597 + * is enough space for at least MIN_SWAP_SIZE + MIN_DUMP_SIZE. >> >> >> But this doesn't seam to be true in the code, since the function is >> there to check the possiblity of not having enough space for the request >> and returning (OM_FAILURE) if this happens. >> >> My two cents, >> Luis >> >> >> >> Ethan Quach wrote: >>> Matt, >>> >>> Looks good. Just a comment nit at perform_slim_insall.c >>> starting at line 2592. With your latest changes, this >>> comment isn't accurate anymore. >>> >>> >>> thanks, >>> -ethan >>> >>> >>> On 03/24/10 07:38, Matt Keenan wrote: >>>> Code review request for following two bugs : >>>> 10115 - AI ignores desired swap listed in ai manifest >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=10115 >>>> >>>> 12960 - Should be possible to define dump ZVOL size in AI manifest >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=12960 >>>> >>>> Webrev : >>>> http://cr.opensolaris.org/~mattman/bug-10115-12960/ >>>> >>>> 2nd round for review, made following updates after review : >>>> - Changed ai manifest definition names, to make more readable : >>>> <ai_device_swap> to <ai_swap_device> >>>> <ai_device_dump> to <ai_dump_device> >>>> >>>> - If not enough disk space to create the specified swap/dump then >>>> install will fail. >>>> >>>> Both bugs were addressed together as they affect same code paths, >>>> plan is to push to tip only. >>>> >>>> 10115 >>>> ----- >>>> AI manifest currently provides specification for <ai_swap_size>, >>>> however the contents of this element had never been implemented, it >>>> was simply ignored. >>>> Solution, was to read <ai_swap_size> from manifest, if defined and >>>> pass through to liborchestrator via nvlist as new nvpair. >>>> >>>> 12960 >>>> ----- >>>> This is a request to allow the specification of dump size in AI >>>> manifest. >>>> Solution, was to define new section in AI manifest <ai_dump_device> >>>> which can >>>> contain new element <ai_dump_size>. Parse this element in AI and pass >>>> through to liborchestrator in the same manner as swap size. >>>> >>>> >>>> In both cases for swap and dump size, a specification of "0" size is >>>> indicative that the user does not want a swap/dump zvol created. >>>> >>>> The only instance where this is ignored is in the case of swap where >>>> a system memory is low e.g. < 700MB, then a default swap slice of 512 >>>> is created (see disk_slices.c) >>>> >>>> Swap and dump sizes are always optional, if neither are specified, >>>> then the original algorithm for calculating swap and dump size is >>>> used, which is >>>> simply RAM/2. >>>> >>>> If swap and/or dump is specified of > 0, and there is insufficient >>>> disk space available to create the requested zvol(s), the install >>>> will fail. >>>> >>>> >>>> Following testing was carried out. >>>> >>>> Machine : X86 Vbox Client >>>> Disk Size : 16GB (16001 MB) >>>> Ram Size : 1GB (1024 MB) >>>> Recommended Software Size : ((4096 * 1.2) * 2) + 2048 = 11878 MB >>>> Available disk space for swap/dump : (16001 MB - 11878 MB) = 4123 MB >>>> >>>> Following scenarios were tested : >>>> >>>> <ai_swap_size> <ai_dump_size> Swap Created Dump Created >>>> --------------- ---------------- --------------- ------------ >>>> Not Specified Not Specified 512 511 >>>> 0 Not Specified Not Created 511 >>>> 250 Not Specified 250 511 >>>> 1024 Not Specified 1024 511 >>>> 4000 Not Specified Install fail, not enough disk space >>>> Not Specified 0 512 Not Created >>>> Not Specified 250 512 250 >>>> Not Specified 1024 512 1024 >>>> Not Specified 4000 Install fail, not enough disk space >>>> 0 0 Not Created Not Created >>>> 250 0 250 Not Created >>>> 1500 0 1500 Not Created >>>> 4000 0 4000 Not Created >>>> 5000 0 Install fail, not enough disk space >>>> 0 250 Not Created 256 >>>> 0 1500 Not Created 1500 >>>> 0 4000 Not Created 4000 >>>> 0 5000 Install fail, not enough disk space >>>> 1024 1024 1024 1024 >>>> 3000 3000 Install fail, not enough disk space >>>> 1024 4000 Install fail, not enough disk space >>>> 4000 1024 Install fail, not enough disk space >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
