Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove tmp image if the download operation fails #1140

Closed
wants to merge 5 commits into from
Closed

Remove tmp image if the download operation fails #1140

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7747700
Exit early if the image already exists
enescakir Jan 31, 2024
1a4cd40
Finalize the image download with File.rename
enescakir Jan 31, 2024
7d4fc8a
Add force flag to download image even if exists
enescakir Feb 2, 2024
9e3702b
Allow only one concurrent image download with flock
enescakir Jan 31, 2024
b48eb69
Use force flag in the boot image download prog
enescakir Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 34 additions & 34 deletions rhizome/host/lib/vm_setup.rb
View file
Open in desktop
Expand Up @@ -470,43 +470,43 @@ def download_boot_image(boot_image, custom_url: nil, ca_path: nil)

download = urls.fetch(boot_image) || custom_url
image_path = vp.image_path(boot_image)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the commit message and breaking this out; reviewing it with ?w=1 in github was great.

unless File.exist?(image_path)
fail "Must provide custom_url for #{boot_image} image" if download.nil?
FileUtils.mkdir_p vp.image_root

# If image URL has query parameter such as SAS token, File.extname returns
# it too. We need to remove them and only get extension.
image_ext = File.extname(URI.parse(download).path)
initial_format = case image_ext
when ".qcow2", ".img"
"qcow2"
when ".vhd"
"vpc"
when ".raw"
"raw"
else
fail "Unsupported boot_image format: #{image_ext}"
end

# Use of File::EXCL provokes a crash rather than a race
# condition if two VMs are lazily getting their images at the
# same time.
temp_path = File.join(vp.image_root, boot_image + image_ext + ".tmp")
ca_arg = ca_path ? " --cacert #{ca_path.shellescape}" : ""
File.open(temp_path, File::RDWR | File::CREAT | File::EXCL, 0o644) do
r "curl -f -L10 -o #{temp_path.shellescape} #{download.shellescape}#{ca_arg}"
end
return if File.exist?(image_path)

fail "Must provide custom_url for #{boot_image} image" if download.nil?
FileUtils.mkdir_p vp.image_root

# If image URL has query parameter such as SAS token, File.extname returns
# it too. We need to remove them and only get extension.
image_ext = File.extname(URI.parse(download).path)
initial_format = case image_ext
when ".qcow2", ".img"
"qcow2"
when ".vhd"
"vpc"
when ".raw"
"raw"
else
fail "Unsupported boot_image format: #{image_ext}"
end

if initial_format == "raw"
File.rename(temp_path, image_path)
else
# Images are presumed to be atomically renamed into the path,
# i.e. no partial images will be passed to qemu-image.
r "qemu-img convert -p -f #{initial_format.shellescape} -O raw #{temp_path.shellescape} #{image_path.shellescape}"
end
# Use of File::EXCL provokes a crash rather than a race
# condition if two VMs are lazily getting their images at the
# same time.
temp_path = File.join(vp.image_root, boot_image + image_ext + ".tmp")
ca_arg = ca_path ? " --cacert #{ca_path.shellescape}" : ""
File.open(temp_path, File::RDWR | File::CREAT | File::EXCL, 0o644) do
r "curl -f -L10 -o #{temp_path.shellescape} #{download.shellescape}#{ca_arg}"
end

rm_if_exists(temp_path)
if initial_format == "raw"
File.rename(temp_path, image_path)
else
# Images are presumed to be atomically renamed into the path,
# i.e. no partial images will be passed to qemu-image.
r "qemu-img convert -p -f #{initial_format.shellescape} -O raw #{temp_path.shellescape} #{image_path.shellescape}"
end

rm_if_exists(temp_path)
end

# Unnecessary if host has this set before creating the netns, but
Expand Down