mock improvements

[BJ Dierkes]

On Oct 4, 2007, at 11:09 AM, Clark Williams wrote:
>
>
> I'd like to add code to do two things:
>
> 1. Find where we're exiting without umounting
> 2. Add some paranoia code in the startup logic to not screw up if  
> #1 fails (like is
> happening now).
>
> To that end, would you run one of your failure cases with mock -- 
> debug and send me
> the output?

Looking at this a bit further...  I can't recreate the issue using  
mock alone.  One of the issues I fixed in my app yesterday (after  
commenting on this thread) was that I wasn't properly terminating  
spawned child processes (i.e. mock).  Therefore, mock was still  
running a job at the time that I attempted to re-run it from my app.

That said, I think the main focus here is #2... ensuring that if the  
bind mounts from the host are indeed still mounted, divert from  
cleaning the chroot (or add logic to clean everything except those  
points that are mounted).

On top of this however, there does not appear to be anything in place  
to protect against two users (or the same user) using the same mock  
config file, and therefore the same /var/lib/mock/<chroot>.  If  
everyone knows how mock works they would use a --uniqueext for each  
build to protect from cleaning a chroot that is currently in use (and  
has host bind mounts). But that isn't likely, and isn't safe.

If you look at the 'def clean()' method, mock isn't even _umounting '/ 
dev' at all before it cleans the chroot:

===
     def clean(self):
         """clean out chroot with extreme prejudice :)"""
         self.state("clean")

         self.root_log('Cleaning Root')
         if os.path.exists('%s/%s' % (self.rootdir, 'proc')):
             self._umount('proc')
         if os.path.exists('%s/%s' % (self.rootdir, 'dev/pts')):
             self._umount('dev/pts')

         if os.path.exists(self.basedir):
             cmd = '%s -rf %s' % (self.config['rm'], self.basedir)
             (retval, output) = self.do(cmd)

             if retval != 0:
                 error(output)
                 if os.path.exists(self.rootdir):
                     raise RootError, "Failed to clean basedir, exiting"
===

Should be:

===
--- /home/wdierkes/mock 2007-10-03 20:01:03.000000000 -0500
+++ /usr/bin/mock       2007-10-04 15:05:14.000000000 -0500
@@ -193,6 +193,8 @@
              self._umount('proc')
          if os.path.exists('%s/%s' % (self.rootdir, 'dev/pts')):
              self._umount('dev/pts')
+        if os.path.exists('%s/%s' % (self.rootdir, 'dev')):
+            self._umount('dev')

          if os.path.exists(self.basedir):
              cmd = '%s -rf %s' % (self.config['rm'], self.basedir)
===


After adding this change, /dev no longer gets borked.  I've added  
this patch/comment to BZ 250985.

I would suggest adding some logic to say, determine if a build is  
currently in progress (using a specific config/chroot) and if so die  
out, but suggest the user add a --uniqueext.... or something to that  
effect.

Thanks.