On Tue, 14 Apr 2009, William Schumann wrote: > Alok, > > Alok Aggarwal wrote: >> Hi William, >> >> On Thu, 9 Apr 2009, William Schumann wrote: >> >>> There are two problems addressed by this bug. >>> >>> 1) After initial AI installation, the swap slice is regarded as user data >>> in subsequent AI installations instead of scratch >>> 2) The swap slice space allocation in TI is not done with any awareness >>> of AI customization >>> >>> The fix involving making AI aware of 1 and for AI to perform 2 instead of >>> leaving everything to TI, which allocates the swap space for GUI. >> >> So, from a high level, is it fair to say that the policy with respect to >> preserving slices by default isn't being >> changed? > Yes, the policy for preserving slices by default does not change. >> And, the change you're making here is to create >> a swap slice if either s1 doesn't exist or s1 exists with >> a V_SWAP tag. Correct? > The decision of whether a swap slice is created depends upon the memory size > of the machine. >> >> Also now that 7718 has been reversed in the source code >> only machines with less than 700MB memory will see this >> problem on every other AI install after the *first* install. > I think it would fail *all* AI installs after the first one (that didn't > explicitly delete slice 1 per the workaround); otherwise, what you said is > true. Also, without the fix, problem 2) above still exists - if a swap > slice is called for, any slice customization or any existing slices which > should be preserved will be destroyed instead.
Okay, thanks for clarifying. Here are my comments - auto_install.c: Why is OM_ROOT being set for installs on slices other than slice zero? perform_slim_install.c: line 754: Nit - Sentence seems incomplete. " .. if swap slice needs to be created" perform_slim_install.c: line 2919: This function no longer returns a boolean_t ti_dm.c: lines 1503 - 1515: It seems that this comment should be made clearer to indicate under which conditions a swap slice would be created - a) If it is explicitly specified or, b) if the code figured out if the installation was being done on a low memory machine disk_slices.c: line 50: Why this change? disk_slices.c: line 365: How about check_slices_in_use() as the name instead? disk_slices.c: line 376: What is an exempt slice? disk_slices.c: line 398: What tag will be returned if none was set in the first place? Have you verified it is V_UNASSIGNED? disk_slices.c: line 406: Nit - I don't think the comment needs to include the type of this variable disk_slices.c: lines 430-432: Could you please expound on this? There seems to be implicit ordering here that the callers must maintain. In other words, this seems to be true only for AI installs and if auto-install were to be changed to reorder how it executes manifest actions, that could lead to problems here. disk_slices.c: line 607: Again this comment is not clear. Suggest rewording as "If default slice action is specified (i.e. no customizations), create a swap slice if deemed neccesary for the install to proceed (typically true only for low memory systems)" disk_slices.c: line 658: The thought of possible recursion in this function makes me queasy. This seems like a perfect candidate for some code refactoring in the future. disk_slices.c: The thing I'm still not clear about is whether AI installs done prior to this bug fix will have V_SWAP set for the swap slice. Will they? Alok
