On Sun, 2009-11-22 at 21:27 +0100, Hans de Goede wrote: > Hi, > > On 11/22/2009 05:51 AM, Jerry Vonau wrote: > > <snip> > > >> Ack, but your current split up is still not split up per purpose, I think > >> I understand what you are trying to achieve, so let me suggest a split: > >> > >> 1) A patch fixing the current copy code to use the fs with the most > >> free space instead of the least > >> > >> 2) A patch to not only do the image transfer in case of a multiple disk > >> cd / dvd install, but also in the case of a hdiso / net install, so as > >> to free up memory used by install.img (which will be another patch). > >> This patch should only do this when memory is tight! Doing this > >> always is bad, as it is useless on systems with tons of memory, and > >> could potentially even cause issues there with for example diskspace > >> > >> 3) A patch to actually achieve the freeing of memory 2)'s goal is by > >> unlinking the install.img from /tmp > >> > >> > > Think I got the flow you want for the patches now, I'm digging at the > > free variable for the first part now. That will take me some time as I > > don't have much to spare. What I have now is less that ideal, I would > > like see what is being returned, and just use / for the image till the > > above has being sorted out. At least for my testing, the rest of my > > patches depend on that one being in place. > > > > Oh, but that is easy, simply change: > self._loopbackFile = "%s%s/rhinstall-install.img" % (anaconda.rootPath, > free[0][0]) > to: > self._loopbackFile = "%s%s/rhinstall-install.img" % (anaconda.rootPath, > free[len(free)-1][0]) > Yea, I see that now, but I need to see what was being returned first ;-) > So that you use the last element of the list, instead of the first, so that > you get the biggest partition, your testing of this fix is much appreciated :) > That I can test quickly. I get back to you in a bit. (firing up vmm) > >> Notice that I'm not going with your suggested just always transfer > >> install.img approach. This is not acceptable IMHO as it causes unnecessary > >> slow down in many cases (net / disk install with plenty of ram, single dvd > >> install), and it has the potential to trigger issues in all these cases > >> which we would not hit if we did not do the transfer. > >> > >> IOW doing the transfer has a price: > >> 1) it consumes disk space which we may need > >> 2) it causes us to take more "steps" then if we don't and each step we > >> take can (and eventually will) cause issues, so avoiding extra steps where > >> possible is good. > >> 3) it causes a slowdown > >> > >> So since it is not free we should not do it unless there are good reasons > >> to, so far the only reason we had for doing this was a multiple cd/dvd > >> install, but I'm willing to agree that for things like a network install > >> on a low memory machine it would be a good thing to do too. > >> > > Cool, then the question is at what amount of ram should this kick in at, > > I went with MIN_GUI_RAM. > > > > That sounds reasonable. Maybe want to do use something like MIN_GUI_RAM + 100MB, > so as to have atleast MIN_GUI_RAM free when the install.img (which is approx > 100Mb) lives in RAM. > That is do-able. > >>> Like you stated above, you may have to change disks, you don't have to > >>> right now with a dvd, but how long before you need 2 DVDs? > >> > >> Quite long, there is no reason why we should not be able to fix a package > >> set for a compete desktop install on a single dvd for years to come. > >> > >>> The move to > >>> doConfigSetup is based on the fact that net and hdiso installs hold the > >>> install image in /tmp. In order to make that ram available for yum/rpm, > >>> I figured that is the earliest point to trigger that call to backend.py > >>> where that could now take place. I think the trade off of having the > >>> memory recovered, is a good one for using that space on the HD. > >> > >> The trade off is only a good one if memory is a scarce resource, see above. > >> > > Think I have that addressed in this round of patches against 12.46-2. > > > > Thanks for taking the time, > > You are welcome, thanks for contributing to anaconda. I hope that once you > get the hang of it you keep on contributing :) > > Here is a review of your latest set of patches: > > 1freefix.diff: > See above > > 2modcall.diff: > Please put the large comment in a commit msg, it would be great if you > could learn to use git. (Drop by on #anaconda during CET office hours and > I'll help you). Otherwise atleast learn to write commit messages, in a > .patch / .diff file you can put text above the > --- filename > +++ filename > I'm getting the hang of git, slowly but surely.. > Lines and patch (and other tools) will ignore this, please learn to put some > explanation of the patch there start with a single line summary, so for > example a good header for 2modcall.diff would be: > > ### > Remove unneeded check from mountInstallImage() > > This is a preparation patch for transferring install.img from /tmp to > disk in low memory network and hdiso install scenarios to free up > memory used by install.img under /tmp. > > Currently mountInstallImage only gets called when using cd's, based on > yuminstall's run, mkeys.sort(mediasort), if len(mkeys) > 1. > mediaDevice has already been validated in yuminstall.py and you can't > get here without mounting install.img, so there is no need for the > check this patch removes. > ### > > The second hunk of this patch should be part of the 3th patch (or separate) > > > 3setcall.diff: > > 1) This does not belong in doConfigSetup(), setup() itself or a new method > called from setup() would be a better place. > OK, I'll see what I come up with. > 2) Even if the mountInstallImage() fails you still unlink /tmp/install.img > Yea, rushed that part, should be checking for a return code before unlinking. > > 4boot.diff: > > Ah a new trick upi your sleef (atleast to me), nice one. But will this > work ? Does the loader (stage1) copy install.img from /boot/upgrade to > /tmp ? I'm asking because if it does not, then we will still have the > loopback mounted and the unlink will not free up the diskspace. > The harddrive method never used to until preupgrade came along, but it does now. > > We are getting somewhere, I think this will greatly improve certain > types of installs on low memory machines, and it will help with some > pre-upgrade issues, thanks! > At least if I don't have the time, the ideas are out there now. Thanks, Jerry _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
