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.
--
Fedora-buildsys-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/fedora-buildsys-list
