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
This commit is contained in:
Adam Williamson 2016-03-21 08:53:13 -07:00
parent 326dd08e25
commit 055c79fc58

View File

@ -147,7 +147,8 @@ class GuestfsImage(object):
for upload in self.uploads: for upload in self.uploads:
# download the file to a temp file - borrowed from # download the file to a temp file - borrowed from
# fedora_openqa_schedule (which stole it from SO) # 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']) resp = urlopen(upload['source'])
while True: while True:
# This is the number of bytes to read between buffer # This is the number of bytes to read between buffer
@ -155,12 +156,13 @@ class GuestfsImage(object):
buff = resp.read(8192) buff = resp.read(8192)
if not buff: if not buff:
break break
dltmpfile.write(buff) tmpfh.write(buff)
# as with write, the dict must specify a target # as with write, the dict must specify a target
# partition and location ('target') # partition and location ('target')
partn = gfs.list_partitions()[int(upload['part'])-1] partn = gfs.list_partitions()[int(upload['part'])-1]
gfs.mount(partn, "/") gfs.mount(partn, "/")
gfs.upload(dltmpfile.name, upload['target']) gfs.upload(tmpfname, upload['target'])
os.remove(tmpfname)
# we're all done! rename to the correct name # we're all done! rename to the correct name
os.rename(tmpfile, self.filename) os.rename(tmpfile, self.filename)
except: except:
@ -170,7 +172,8 @@ class GuestfsImage(object):
raise raise
finally: finally:
# whether things go right or wrong, we want to close the # whether things go right or wrong, we want to close the
# gfs instance # gfs instance, and rwmj recommends 'shutdown()' too
gfs.shutdown()
gfs.close() gfs.close()