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])
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 :)
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.
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
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.
2) Even if the mountInstallImage() fails you still unlink /tmp/install.img
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.
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!
Regards,
Hans
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
