Re: [PATCH] mock _init _umountall

[Clark Williams]

On Wed, 30 Jun 2010 16:10:20 -0500
Paul B Schroeder <paulbsch(a)vbridges.com&gt; wrote:

>  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...
>  
>   
Thanks Paul, just wanted to let you know that we're not *completely*
ignoring you, just busy with release stuff. I'll merge this (or a
tweaked version of it) and get it out probably just after the 4th. 

Clark

>  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>
>  --
>  buildsys mailing list
>  buildsys(a)lists.fedoraproject.org
>  https://admin.fedoraproject.org/mailman/listinfo/buildsys