From 8e8893a3cdd06d37bbcfa3ee7bef1a81caa9110f Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Fri, 27 Apr 2007 14:39:11 +0100 Subject: [PATCH] * Fix fd handling to work around Python's habit of closing files you specify in subprocess.Popen. * Error handling bugfixes: say except (IOError,OSError) everywhere rather than just one of those two; correct harmless bug in gpg key generation error handling. * Make adt-virt-null work properly (VirtSubProc runs `down' with a single argument, so down must be sh -c and not []). * In VirtSubProc close spurious copy of plumbing pipe, which prevents certain hangs during error situations. * Xen cleanup script runs dmsetup info / dmsetup remove several times with some sleeps because xm destroy is not properly instantaneous. --- debian/changelog | 16 ++++++++++++++++ runner/adt-run | 25 +++++++++++-------------- virt-subproc/VirtSubproc.py | 29 +++++++++++++++++------------ virt-subproc/adt-virt-null | 2 +- xen/cleanup | 11 +++++++++-- 5 files changed, 54 insertions(+), 29 deletions(-) diff --git a/debian/changelog b/debian/changelog index c541b88..3d4bec5 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,19 @@ +autopkgtest (0.8.1feisty1~iwj) feisty-updates; urgency=low + + * Fix fd handling to work around Python's habit of closing files + you specify in subprocess.Popen. + * Error handling bugfixes: say except (IOError,OSError) everywhere + rather than just one of those two; correct harmless bug in gpg key + generation error handling. + * Make adt-virt-null work properly (VirtSubProc runs `down' with + a single argument, so down must be sh -c and not []). + * In VirtSubProc close spurious copy of plumbing pipe, which prevents + certain hangs during error situations. + * Xen cleanup script runs dmsetup info / dmsetup remove several times + with some sleeps because xm destroy is not properly instantaneous. + + -- Ian Jackson Fri, 27 Apr 2007 14:38:49 +0100 + autopkgtest (0.8.1) feisty; urgency=low * Call dmsetup remove repeatedly instead of messing with udevsettle. diff --git a/runner/adt-run b/runner/adt-run index 573fd06..b33c5ba 100755 --- a/runner/adt-run +++ b/runner/adt-run @@ -114,7 +114,7 @@ class Errplumb: ep.stream = open('/dev/null','w') ep._sp = None elif count == 1: - if to_stderr: ep.stream = sys.stderr + if to_stderr: ep.stream = os.dup(2) else: ep.stream = trace_stream ep._sp = None else: @@ -124,6 +124,9 @@ class Errplumb: ep.stream = ep._sp.stdin def wait(ep): if ep._sp is None: return + if type(ep.stream) == type(2): + os.close(ep.stream) + ep.stream = () ep._sp.stdin.close() rc = ep._sp.wait() if rc: bomb('stderr plumbing tee(1) failed, exit code %d' % rc) @@ -180,7 +183,7 @@ class Unsupported: def mkdir_okexist(pathname, mode=02755): try: os.mkdir(pathname, mode) - except OSError, oe: + except (IOError,OSError), oe: if oe.errno != errno.EEXIST: raise def rmtree(what, pathname): @@ -1115,7 +1118,7 @@ def read_control(act, tree, control_override): tree, 'debian/tests/control') try: control = open(control_af.read(), 'r') - except OSError, oe: + except (IOError,OSError), oe: if oe[0] != errno.ENOENT: raise return [] @@ -1256,7 +1259,7 @@ class Binaries: for x in ['pubring','secring']: os.stat(opts.gnupghome + '/' + x + '.gpg') ok = True - except OSError, oe: + except (IOError,OSError), oe: if oe.errno != errno.ENOENT: raise if ok: b._debug('no key generation needed') @@ -1286,14 +1289,8 @@ END gpg --homedir="$1" --batch --gen-key key-gen-params ''' cmdl = ['sh','-ec',script,'x',opts.gnupghome] - rc = subprocess_cooked(cmdl, dbg=(genkey,script))[0] - if rc: - try: - f = open(opts.gnupghome+'/key-gen-log') - tp = file.read() - except OSError, e: tp = e - pstderr(tp) - bomb('key generation failed, code %d' % rc) + rc = subprocess_cooked(cmdl, dbg=('genkey',script))[0] + if rc: bomb('key generation failed, code %d' % rc) def apt_configs(b): return { @@ -1353,11 +1350,11 @@ END dest = RelativeOutputFile('binaries--'+leafname, b.dir, leafname) try: os.remove(dest.write()) - except OSError, oe: + except (IOError,OSError), oe: if oe.errno != errno.ENOENT: raise e try: os.link(af.read(), dest.write()) - except OSError, oe: + except (IOError,OSError), oe: if oe.errno != errno.EXDEV: raise e shutil.copy(af.read(), dest) diff --git a/virt-subproc/VirtSubproc.py b/virt-subproc/VirtSubproc.py index 8a4216c..6c17f52 100644 --- a/virt-subproc/VirtSubproc.py +++ b/virt-subproc/VirtSubproc.py @@ -151,9 +151,8 @@ def down_python_script(gobody, functions=''): debug("+P ...\n"+script) scripte = urllib.quote(script) - cmdl = down + ['python','-c', - "'import urllib; s = urllib.unquote(%s); exec s'" % - ('"%s"' % scripte)] + cmdl = down + ["exec python -c 'import urllib; s = urllib.unquote(%s);" + " exec s'" % ('"%s"' % scripte)] return cmdl def cmd_execute(c, ce): @@ -205,18 +204,23 @@ def cmd_execute(c, ce): " mode = status.st_mode | 0111\n" " os.chmod(c0, mode)\n" " try: os.execvp(c0, cmd)\n" - " except OSError, e:\n" + " except (IOError,OSError), e:\n" " print >>sys.stderr, \"%s: %s\" % (\n" " (c0, os.strerror(e.errno)))\n" " os._exit(127)\n") cmdl = down_python_script(gobody) + stdout_copy = None try: - (status, out) = execute_raw('sub-python', None, timeout, - cmdl, stdout=stdout, stdin=devnull_read, - stderr=subprocess.PIPE) - except Timeout: - raise FailedCmd(['timeout']) + if type(stdout) == type(2): stdout_copy = os.dup(stdout) + try: + (status, out) = execute_raw('sub-python', None, + timeout, cmdl, stdout=stdout_copy, + stdin=devnull_read, stderr=subprocess.PIPE) + except Timeout: + raise FailedCmd(['timeout']) + finally: + if stdout_copy is not None: os.close(stdout_copy) if out: bomb("sub-python unexpected produced stdout" " visible to us `%s'" % out) @@ -257,11 +261,11 @@ def copyupdown(c, ce, upp): gobody = " dir = urllib.unquote('%s')\n" % sde[iremote] if upp: try: os.mkdir(sd[ilocal]) - except OSError, oe: + except (IOError,OSError), oe: if oe.errno != errno.EEXIST: raise else: gobody += (" try: os.mkdir(dir)\n" - " except OSError, oe:\n" + " except (IOError,OSError), oe:\n" " if oe.errno != errno.EEXIST: raise\n") gobody +=( " os.chdir(dir)\n" " tarcmd = 'tar -f -'.split()\n") @@ -289,11 +293,12 @@ def copyupdown(c, ce, upp): debug(" +> %s" % string.join(cmdls[1])) subprocs[1] = subprocess.Popen(cmdls[1], stdin=subprocs[0].stdout, stdout=deststdout, preexec_fn=preexecfn) + subprocs[0].stdout.close() timeout_start(copy_timeout) for sdn in [1,0]: debug(" +"+"<>"[sdn]+"?"); status = subprocs[sdn].wait() - if status: + if not (status==0 or (sdn==0 and status==-13)): timeout_stop() bomb("%s %s failed, status %d" % (wh, ['source','destination'][sdn], status)) diff --git a/virt-subproc/adt-virt-null b/virt-subproc/adt-virt-null index e1a887b..f515286 100755 --- a/virt-subproc/adt-virt-null +++ b/virt-subproc/adt-virt-null @@ -52,7 +52,7 @@ def parse_args(): if os.getuid()==0: capabilities.append('root-on-testbed') - vsp.down = [] + vsp.down = ['sh','-ec'] def hook_open(): global downtmp diff --git a/xen/cleanup b/xen/cleanup index 5c1028c..9ef7163 100755 --- a/xen/cleanup +++ b/xen/cleanup @@ -5,10 +5,17 @@ test $nonoptargs = 0 || fail "non-option arguments not allowed" mkdir -p $adt_play $snap -try () { printf "%s\n" "- $*"; "$@" >/dev/null 2>&1 ||:; } +try_es () { printf "%s\n" "- $*"; "$@" >/dev/null 2>&1; } +try () { try_es "$@" ||:; } n=0 try xm destroy $adt_xmname 2>/dev/null try umount $lvm_baselv_namepath -try dmsetup remove $adt_devmapper_cowdev +sleeptime=0 +while try_es dmsetup info $adt_devmapper_cowdev; do + try dmsetup remove $adt_devmapper_cowdev + [ $sleeptime -le 5 ] || fail "timed out trying dmsetup info/remove" + sleep $sleeptime + sleeptime=$(( $sleeptime + 1 )) +done rm -f $lvm_fslink_ptr -- 2.30.2