From 055c79fc58ddc0cf78d628ff896149de28c3a62d Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Mon, 21 Mar 2016 08:53:13 -0700 Subject: [PATCH] createhdds: use temp file correctly when uploading Summary: With the previous code, the temporary file wasn't closed before we try to upload its contents to the disk image. This seems to cause problems when run in Python 2; the file is truncated on upload. So rejig things a bit so we use tempfile.mkstemp, close the file after writing to it, then remove it ourselves after doing the upload. Tested with Python 2 and Python 3 that this creates a correct image. If something goes wrong at the wrong time the temp file will be left around, but hey, it's a temp file - they get cleaned up on system shutdown. So doesn't seem like a big deal. Test Plan: Check building the two images that involve a file upload in both Python 2 and Python 3, make sure the file is complete and correct in all cases. Reviewers: jskladan, garretraziel Reviewed By: garretraziel Subscribers: tflink Differential Revision: https://phab.qadevel.cloud.fedoraproject.org/D786 --- createhdds.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/createhdds.py b/createhdds.py index 7d948da..156398e 100755 --- a/createhdds.py +++ b/createhdds.py @@ -147,7 +147,8 @@ class GuestfsImage(object): for upload in self.uploads: # download the file to a temp file - borrowed from # fedora_openqa_schedule (which stole it from SO) - with tempfile.NamedTemporaryFile(prefix="createhdds") as dltmpfile: + (_, tmpfname) = tempfile.mkstemp(prefix="createhdds") + with open(tmpfname, 'wb') as tmpfh: resp = urlopen(upload['source']) while True: # This is the number of bytes to read between buffer @@ -155,12 +156,13 @@ class GuestfsImage(object): buff = resp.read(8192) if not buff: break - dltmpfile.write(buff) - # as with write, the dict must specify a target - # partition and location ('target') - partn = gfs.list_partitions()[int(upload['part'])-1] - gfs.mount(partn, "/") - gfs.upload(dltmpfile.name, upload['target']) + tmpfh.write(buff) + # as with write, the dict must specify a target + # partition and location ('target') + partn = gfs.list_partitions()[int(upload['part'])-1] + gfs.mount(partn, "/") + gfs.upload(tmpfname, upload['target']) + os.remove(tmpfname) # we're all done! rename to the correct name os.rename(tmpfile, self.filename) except: @@ -170,7 +172,8 @@ class GuestfsImage(object): raise finally: # whether things go right or wrong, we want to close the - # gfs instance + # gfs instance, and rwmj recommends 'shutdown()' too + gfs.shutdown() gfs.close()