On Fri, 2009-11-20 at 21:04 +0100, Hans de Goede wrote:
> Hi,
>
> On 11/20/2009 06:59 PM, Jerry Vonau wrote:
> >> + if self._loopbackFile and os.path.exists(self._loopbackFile):
> >> + log.info("and exists and removing %s" % self._loopbackFile )
> >> + try:
> >> + os.unlink(self._loopbackFile)
> >> + except:
> >> + pass
> >> +
> >>
> >> Not sure what you are trying to do here, self._loopbackFile is None (so false)
> >> the first time we go through this code path, and if we go through this code
> >> path multiple times, it should be a nop the second time (see the
> >> check for this you removed above).
> >>
> > Before coping the running install.img, if there is self._loopbackFile
> > present (from a previously failed install attempt), remove it. Think the
> > original check made a bad assumption, this file may not be the same
> > version as what is need by the currently booted version of anaconda.
> > self.anaconda.backend.mountInstallImage is only called once from
> > yuminstall.py
> >
>
> Ah, this is a misunderstanding of what the check actually does, first of
> all it checks if we have generated a filename for storing install.img somewhere
> on a partition we are installing to / we are updating. If we have failed
> in a previous install and we need to transfer install.img, then that filename is
> not set yet, so we will transfer the current install.img, even if an older
> one is already in the same place.
>
> This check only stops us from transefering the image if:
> 1) the filename was already generated (so this is not the first run
> of this function this install)
> 2) a file with such name exists (iow the previous transfer attempt
> was correct).
>
> The current code for this is correct, and unless you have a really good reason
> to change it you should not.
>
Yup, I misunderstood, badly..
> >> + # This s/b the one that was copied into /tmp ....
> >> + # Why was this copied to ram when you have passed stage2=
> >> + # lets free the RAM
> >> + if os.path.exists("/tmp/install.img"):
> >> + log.info("OVERRIDE Using /tmp/install.img as stage2 image")
> >> + stage2img = "/tmp/install.img"
> >> + stage2ram = 1
> >> + else:
> >> + stage2img = installimg
> >>
> >> Ok, you've alsmost completely lost me here, I have some clue what
> >> you are trying to do, but it is completely unrelated to the previous parts
> >> of this patch.
> >>
> > No, it's related, you need to have the source for install.img, that can
> > live on cd/dvd media, or for hard-drive and net installs in /tmp. I want
> > to recover the ram from /tmp. It's best if we copy the one that is
> > currently mounted by anaconda.
> >
>
> It is related in that has to do with transfering the image, but it is not
> targeting the exact same purpose / fixing the exact same thing.
>
> >> Please make *ONE* change per patch, and add a lot more verbose description
> >> of the why, what and how of the patch. So put in the description:
> >>
> >> 1) What you are trying to fix
> >> 2) What is the current behaviour (if applicable in various scenarios)
> >> 3) What are you changing and how does this fix this.
> >> 4) How does this change impact other install scenarios ?
> >>
> > Sorry, should of made incremental patches, I'll break up the patch, to
> > be more readable with some revisions.
> >
>
> 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.
> 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.
> > 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,
Jerry
diff -up ./backend.py.orig ./backend.py
--- ./backend.py.orig 2009-11-21 19:22:34.000000000 -0600
+++ ./backend.py 2009-11-21 19:30:38.000000000 -0600
@@ -163,9 +163,13 @@ class AnacondaBackend:
return
free = anaconda.id.storage.fsFreeSpace
+ # revert this once "free" is fixed
+ # need to see what is returned
+ log.info("free = %s" %free )
+ # screw it just use / for now
self._loopbackFile = "%s%s/rhinstall-install.img" % (anaconda.rootPath,
- free[0][0])
-
+ "/")
+# free[0][0])
try:
win = anaconda.intf.waitWindow(_("Copying File"),
_("Transferring install image to hard drive"))
diff -up ./yuminstall.py.orig ./yuminstall.py
--- ./yuminstall.py.orig 2009-11-21 20:24:29.000000000 -0600
+++ ./yuminstall.py 2009-11-21 20:24:32.000000000 -0600
@@ -725,6 +725,13 @@ class AnacondaYum(YumSorter):
self.repos.setCacheDir(self.conf.cachedir)
+ if os.path.exists("/mnt/sysimage/boot/upgrade/install.img"):
+ log.info("REMOVING stage2 image from /mnt/sysimage/boot/upgrade")
+ try:
+ os.unlink ("/mnt/sysimage/boot/upgrade/install.img")
+ except:
+ log.warning("failed to clean /boot/upgrade")
+
def downloadHeader(self, po):
while True:
# retrying version of download header
diff -up ./yuminstall.py.orig ./yuminstall.py
--- ./yuminstall.py.orig 2009-11-21 05:16:04.000000000 -0600
+++ ./yuminstall.py 2009-11-21 16:36:47.000000000 -0600
@@ -625,6 +625,25 @@ class AnacondaYum(YumSorter):
def doConfigSetup(self, fn='/tmp/anaconda-yum.conf', root='/'):
+ # installs that don't use /mnt/stage2 hold the install.img on
+ # a tmpfs, free this ram if things are tight.
+ stage2img = "/tmp/install.img"
+
+ if os.path.exists(stage2img) and iutil.memInstalled() < isys.MIN_GUI_RAM:
+ log.info("%s exists and low memory" % stage2img )
+
+ # free up /tmp for more memory before yum is called,
+ try:
+ self.anaconda.backend.mountInstallImage(self.anaconda, stage2img)
+ except:
+ log.info("mountInstallImage failed")
+
+ try:
+ os.unlink(stage2img)
+ except:
+ log.info("clearing /tmp failed")
+ pass
+
if hasattr(self, "preconf"):
self.preconf.fn = fn
self.preconf.root = root
--- backend.py.orig2 2009-11-21 19:52:50.000000000 -0600
+++ backend.py 2009-11-21 19:53:23.000000000 -0600
@@ -156,11 +156,19 @@
if self._loopbackFile and os.path.exists(self._loopbackFile):
return
- # If we've booted off the first CD/DVD (so, not the boot.iso) then
- # copy the install.img to the filesystem and switch loopback devices
- # to there. Otherwise we won't be able to unmount and swap media.
- if not anaconda.mediaDevice or not os.path.exists(installimg):
- return
+ # remove me later..
+
+ # You get here currently only if your using cd's, based on
+ # yuminstall's run, mkeys.sort(mediasort), if len(mkeys) > 1.
+ # hd installs hold install.img in /tmp, while preupgrade might
+ # be forced to download install.img again. mediaDevice has alrealy
+ # been validated in yuminstall.py and you can't get here without
+ # mounting install.img, removed the check to enable first patch.
+
+ # If we've booted off the first CD (so, not the boot.iso or DVD)
+ # then copy the install.img to the filesystem and switch loopback
+ # devices to there. Otherwise we won't be able to unmount and
+ # swap media.
free = anaconda.id.storage.fsFreeSpace
# revert this once "free" is fixed
@@ -199,7 +207,12 @@
return 1
isys.lochangefd("/dev/loop0", self._loopbackFile)
- isys.umount("/mnt/stage2")
+
+ # http, ftp, hd installs don't mount /mnt/stage2
+ try:
+ isys.umount("/mnt/stage2")
+ except:
+ pass
def removeInstallImage(self):
if self._loopbackFile:
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
