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
