[PATCH] mock _init _umountall

[Paul B Schroeder]

Here is an updated patch.  I found that _umountall was unmounting in the same order of the mounts and this was also causing some issues.  _umountall now unwinds the mounts by executing the umount commands in reverse order of the mounts.
I also went ahead an created a trac ticket:  https://fedorahosted.org/mock/ticket/8

Cheers...Paul...


diff --git a/py/mock/backend.py b/py/mock/backend.py
index ca06f59..bad33b5 100644
--- a/py/mock/backend.py
+++ b/py/mock/backend.py
@@ -282,24 +282,24 @@ class Root(object):
             self._mountall()
             if self.chrootWasCleaned:
                 self._yum(self.chroot_setup_cmd, returnOutput=1)
-        finally:
-            self._umountall()
 
-        # create user
-        self._makeBuildUser()
+            # create user
+            self._makeBuildUser()
 
-        # create rpmbuild dir
-        self._buildDirSetup()
+            # create rpmbuild dir
+            self._buildDirSetup()
 
-        # set up timezone to match host
-        localtimedir = self.makeChrootPath('etc')
-        localtimepath = self.makeChrootPath('etc', 'localtime')
-        if os.path.exists(localtimepath):
-            os.remove(localtimepath)
-        shutil.copy2('/etc/localtime', localtimedir)
+            # set up timezone to match host
+            localtimedir = self.makeChrootPath('etc')
+            localtimepath = self.makeChrootPath('etc', 'localtime')
+            if os.path.exists(localtimepath):
+                os.remove(localtimepath)
+            shutil.copy2('/etc/localtime', localtimedir)
 
-        # done with init
-        self._callHooks('postinit')
+            # done with init
+            self._callHooks('postinit')
+        finally:
+            self._umountall()
 
     decorate(traceLog())
     def _setupDev(self):
@@ -589,7 +589,10 @@ class Root(object):
     decorate(traceLog())
     def _umountall(self):
         """umount all mounted chroot fs."""
-        for cmd in self.umountCmds:
+        # Unwind mounts by umounting in the opposite order of the mounts
+        umountCmds = self.umountCmds
+        umountCmds.reverse()
+        for cmd in umountCmds:
             self.root_log.debug(cmd)
             mock.util.do(cmd, raiseExc=0, shell=True)
 


On 06/25/2010 03:11 PM, Paul B Schroeder wrote:
> Hello all...
> 
> Recently, I was doing some work which needed a more extensive /dev than the mock internal dev setup.  So I tried doing this in site-defaults.cfg:
> config_opts['internal_dev_setup'] = False
> config_opts['plugin_conf']['bind_mount_enable'] = True
> config_opts['plugin_conf']['bind_mount_opts']['dirs'].append(('/dev', '/dev' ))
> 
> And discovered that mock chroot init was failing due _umountall getting called (in _init) before _makeBuildUser.  In _makeBuildUser there is a perl command which, for whatever reason, would fail due to /dev/null not being available at that point.
> 
> At any rate, the following patch seems to take care of the issue.  And it would seem to be a better way to handle this in general as I would expect _umountall to be the last method called.
> 
> It would be awesome if this or something similar could be applied..
> 
> Thanks...Paul...
> 
> 
> diff --git a/py/mock/backend.py b/py/mock/backend.py
> index ca06f59..9df438b 100644
> --- a/py/mock/backend.py
> +++ b/py/mock/backend.py
> @@ -282,24 +282,24 @@ class Root(object):
>               self._mountall()
>               if self.chrootWasCleaned:
>                   self._yum(self.chroot_setup_cmd, returnOutput=1)
> -        finally:
> -            self._umountall()
> 
> -        # create user
> -        self._makeBuildUser()
> +            # create user
> +            self._makeBuildUser()
> 
> -        # create rpmbuild dir
> -        self._buildDirSetup()
> +            # create rpmbuild dir
> +            self._buildDirSetup()
> 
> -        # set up timezone to match host
> -        localtimedir = self.makeChrootPath('etc')
> -        localtimepath = self.makeChrootPath('etc', 'localtime')
> -        if os.path.exists(localtimepath):
> -            os.remove(localtimepath)
> -        shutil.copy2('/etc/localtime', localtimedir)
> +            # set up timezone to match host
> +            localtimedir = self.makeChrootPath('etc')
> +            localtimepath = self.makeChrootPath('etc', 'localtime')
> +            if os.path.exists(localtimepath):
> +                os.remove(localtimepath)
> +            shutil.copy2('/etc/localtime', localtimedir)
> 
> -        # done with init
> -        self._callHooks('postinit')
> +            # done with init
> +            self._callHooks('postinit')
> +        finally:
> +            self._umountall()
> 
>       decorate(traceLog())
>       def _setupDev(self):
> 
> 
> 
> 

-- 
---
Paul B Schroeder
<paulbsch "at" vbridges "dot" com>