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
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to