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.

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

Create disk images without root #381

Merged
merged 3 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 19 additions & 21 deletions lib/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def __init__(self, arch, external_sources, early_preseed, repo_path):
self.external_sources = external_sources
self.repo_path = repo_path
self.source_manifest = self.get_source_manifest(not self.external_sources)
self.early_source_manifest = self.get_source_manifest(True)
self.target_dir = None
self.external_dir = None

Expand Down Expand Up @@ -59,26 +60,16 @@ def prepare(self, target, using_kernel=False, kernel_bootstrap=False, target_siz
# argument matrix ... or we could just use ext3 instead which
# is effectively universally the same
if kernel_bootstrap:
init_path = os.path.join(self.target_dir, 'init')
self.target_dir = os.path.join(self.target_dir, 'init')
os.mkdir(self.target_dir)

os.mkdir(init_path)
self.target_dir = init_path

if self.repo_path or self.external_sources:
target.add_disk("external", filesystem="ext3")
target.mount_disk("external", "external")
else:
if not self.repo_path and not self.external_sources:
self.external_dir = os.path.join(self.target_dir, 'external')
elif using_kernel:
self.target_dir = os.path.join(self.target_dir, 'disk')
target.add_disk("disk",
filesystem="ext3",
size=(str(target_size) + "M") if target_size else "16G",
bootable=True)
target.mount_disk("disk", "disk")
self.external_dir = os.path.join(self.target_dir, 'external')

os.makedirs(self.external_dir, exist_ok=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove exist_ok here? I'm pretty sure this will break CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any situation where that directory can exist at that point? Previously, it would exist when the "external" image was mounted, but it is not longer the case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It exists at the beginning the latter phases of GitHub CI, which is split up into multiple phases to avoid hitting GitHub's time limits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the prepare function (the one modified here) is only called for the first step, later steps use the reuse function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like I was actually mistaken about the purpose of this.

But it is indeed necessary - when building with kernel-bootstrap and external sources, /external will already exist when we get here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still don't see how can the directory exist.

But it is indeed necessary - when building with kernel-bootstrap and external sources, /external will already exist when we get here.

That was true when the external image was mounted, which already created the directory. But now only the init directory will exist at that point.

os.makedirs(self.external_dir)

if self.early_preseed:
# Extract tar containing preseed
Expand All @@ -103,10 +94,16 @@ def prepare(self, target, using_kernel=False, kernel_bootstrap=False, target_siz
if kernel_bootstrap:
self.create_builder_hex0_disk_image(self.target_dir + '.img', target_size)

if kernel_bootstrap and (self.external_sources or self.repo_path):
target.umount_disk('external')
if self.repo_path or self.external_sources:
mkfs_args = ['-d', os.path.join(target.path, 'external')]
target.add_disk("external", filesystem="ext3", mkfs_args=mkfs_args)
elif using_kernel:
target.umount_disk('disk')
mkfs_args = ['-d', os.path.join(target.path, 'disk')]
target.add_disk("disk",
filesystem="ext3",
size=(str(target_size) + "M") if target_size else "16G",
bootable=True,
mkfs_args=mkfs_args)

def steps(self):
"""Copy in steps."""
Expand Down Expand Up @@ -163,9 +160,10 @@ def create_fiwix_file_list(self):

def distfiles(self):
"""Copy in distfiles"""
def copy_no_network_distfiles(out):
def copy_no_network_distfiles(out, early):
# Note that "no disk" implies "no network" for kernel bootstrap mode
for file in self.source_manifest:
manifest = self.early_source_manifest if early else self.source_manifest
for file in manifest:
file = file[3].strip()
shutil.copy2(os.path.join(self.distfiles_dir, file),
os.path.join(out, file))
Expand All @@ -175,13 +173,13 @@ def copy_no_network_distfiles(out):

if early_distfile_dir != main_distfile_dir:
os.makedirs(early_distfile_dir, exist_ok=True)
copy_no_network_distfiles(early_distfile_dir)
copy_no_network_distfiles(early_distfile_dir, True)

if self.external_sources:
shutil.copytree(self.distfiles_dir, main_distfile_dir, dirs_exist_ok=True)
else:
os.mkdir(main_distfile_dir)
copy_no_network_distfiles(main_distfile_dir)
copy_no_network_distfiles(main_distfile_dir, False)

@staticmethod
def output_dir(srcfs_file, dirpath):
Expand Down
46 changes: 8 additions & 38 deletions lib/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
"""

import enum
import getpass
import os

from lib.utils import mount, umount, create_disk, run_as_root
from lib.utils import mount, create_disk

class TargetType(enum.Enum):
"""Different types of target dirs we can have"""
Expand All @@ -24,7 +23,6 @@ class Target:
"""

_disks = {}
_disk_filesystems = {}
_mountpoints = {}

def __init__(self, path="target"):
Expand All @@ -34,15 +32,6 @@ def __init__(self, path="target"):
if not os.path.exists(self.path):
os.mkdir(self.path)

def __del__(self):
for path in self._mountpoints:
print(f"Unmounting {path}")
umount(path)

for disk in self._disks.values():
print(f"Detaching {disk}")
run_as_root("losetup", "-d", disk)

def tmpfs(self, size="8G"):
"""Mount a tmpfs"""
print(f"Mounting tmpfs on {self.path}")
Expand All @@ -59,32 +48,13 @@ def add_disk(self,
mkfs_args=None):
"""Add a disk"""
disk_path = os.path.join(self.path, f"{name}.img")
self._disks[name] = create_disk(disk_path,
tabletype,
filesystem,
size,
bootable,
mkfs_args)
self._disk_filesystems[name] = filesystem
# Allow executing user to access it
run_as_root("chown", getpass.getuser(), self._disks[name])

def mount_disk(self, name, mountpoint=None):
"""Mount the disk"""
if mountpoint is None:
mountpoint = f"{name}_mnt"
mountpoint = os.path.join(self.path, mountpoint)
os.mkdir(mountpoint)
mount(self._disks[name] + "p1", mountpoint, self._disk_filesystems[name])
# Allow executing user to access it
run_as_root("chown", getpass.getuser(), mountpoint)
self._mountpoints[name] = mountpoint
return mountpoint

def umount_disk(self, name):
"""Unmount a disk"""
umount(self._mountpoints[name])
del self._mountpoints[name]
create_disk(disk_path,
tabletype,
filesystem,
size,
bootable,
mkfs_args)
self._disks[name] = disk_path

def get_disk(self, name):
"""Get the path to a device of a disk"""
Expand Down
13 changes: 5 additions & 8 deletions lib/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,13 @@ def create_disk(image, disk_type, fs_type, size, bootable=False, mkfs_args=None)
if mkfs_args is None:
mkfs_args = []
run('truncate', '-s', size, image)
# First find the device we will use, then actually use it
loop_dev = run_as_root('losetup', '-f', capture_output=True).stdout.decode().strip()
run_as_root('losetup', '-P', loop_dev, image)
# Create the partition
if disk_type != "none":
run_as_root('parted', '--script', image, 'mklabel', disk_type, 'mkpart',
'primary', fs_type, '1GiB' if bootable else '1MiB', '100%')
run_as_root('partprobe', loop_dev)
run_as_root('mkfs.' + fs_type, loop_dev + "p1", *mkfs_args)
return loop_dev
# 1 GiB if bootable, 1 MiB otherwise
offset = str(1024 * 1024 * (1024 if bootable else 1))
run('parted', '--script', image, 'mklabel', disk_type, 'mkpart',
'primary', fs_type, offset + 'B', '100%')
run('mkfs.' + fs_type, image, '-E', 'offset=' + offset, *mkfs_args)

def mount(source, target, fs_type, options='', **kwargs):
"""Mount filesystem"""
Expand Down
4 changes: 3 additions & 1 deletion steps/jump/move_disk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ while ! dd if=/dev/${DISK} of=/dev/null bs=512 count=1; do
done

# Create partition if it doesn't exist
if [ $(($(stat -c "%Lr" "/dev/${DISK}") % 8)) -eq 0 ]; then
# 'stat -c "%T"' prints the minor device type in hexadecimal.
# The decimal version (with "%Lr") is not available in this version of stat.
if [ $((0x$(stat -c "%T" "/dev/${DISK}") % 8)) -eq 0 ]; then
echo "Creating partition table..."
# Start at 1GiB, use -S32 -H64 to align to MiB rather than cylinder boundary
echo "2097152;" | sfdisk -uS -S32 -H64 --force "/dev/${DISK}"
Expand Down