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
