From 6c394f5746efce638d5eba3c6660bbe8d525d71a Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Tue, 6 Jun 2017 12:09:24 +1000 Subject: [PATCH] Pass all blockdevices to bootloader Currently we only export "image-block-device" which is the loopback device (/dev/loopX) for the underlying image. This is the device we install grub to (from inside the chroot ...) This is ok for x86, but is insufficient for some platforms like PPC which have a separate boot partition. They do not want to install to the loop device, but do things like dd special ELF files into special boot partitions. The first problem seems to be that in level1/partitioning.py we have a whole bunch of different paths that either call partprobe on the loop device, or kpartx. We have _all_part_devices_exist() that gates the kpartx for unknown reasons. We have detach_loopback() that does not seem to remove losetup created devices. I don't think this does cleanup if it uses kpartx correctly. It is extremley unclear what's going to be mapped where. This moves to us *only* using kpartx to map the partitions of the loop device. We will *not* call partprobe and create the /dev/loopXpN devices and will only have the devicemapper nodes kpartx creates. This seems to be best. Cleanup happens inside partitioning.py. practice. Deeper thinking about this, and more cleanup of the variables will be welcome. This adds "image-block-devices" (note the extra "s") which exports all the block devices with name and path. This is in a string format that can be eval'd to an array (you can't export arrays). This is then used in a follow-on (I0918e8df8797d6dbabf7af618989ab7f79ee9580) to pick the right partition on PPC. Change-Id: If8e33106b4104da2d56d7941ce96ffcb014907bc --- diskimage_builder/block_device/blockdevice.py | 24 +++-- .../block_device/level1/partition.py | 10 ++ .../block_device/level1/partitioning.py | 92 ++++++++----------- .../bootloader/finalise.d/50-bootloader | 7 +- diskimage_builder/lib/common-functions | 16 ---- diskimage_builder/lib/disk-image-create | 28 ++++-- 6 files changed, 89 insertions(+), 88 deletions(-) diff --git a/diskimage_builder/block_device/blockdevice.py b/diskimage_builder/block_device/blockdevice.py index 30c2172d..4e404bdd 100644 --- a/diskimage_builder/block_device/blockdevice.py +++ b/diskimage_builder/block_device/blockdevice.py @@ -307,18 +307,26 @@ class BlockDevice(object): # been dumped; i.e. after cmd_create() called. state = BlockDeviceState(self.state_json_file_name) - if symbol == 'image-block-partition': - # If there is no partition needed, pass back directly the - # image. - if 'root' in state['blockdev']: - print("%s" % state['blockdev']['root']['device']) - else: - print("%s" % state['blockdev']['image0']['device']) - return 0 + # The path to the .raw file for conversion if symbol == 'image-path': print("%s" % state['blockdev']['image0']['image']) return 0 + # This is the loopback device where the above image is setup + if symbol == 'image-block-device': + print("%s" % state['blockdev']['image0']['device']) + return 0 + + # Full list of created devices by name. Some bootloaders, for + # example, want to be able to see their boot partitions to + # copy things in. Intended to be read into a bash array + if symbol == 'image-block-devices': + out = "" + for k, v in state['blockdev'].items(): + out += " [%s]=%s " % (k, v['device']) + print(out) + return 0 + logger.error("Invalid symbol [%s] for getval", symbol) return 1 diff --git a/diskimage_builder/block_device/level1/partition.py b/diskimage_builder/block_device/level1/partition.py index c6f812c5..e91e1bf9 100644 --- a/diskimage_builder/block_device/level1/partition.py +++ b/diskimage_builder/block_device/level1/partition.py @@ -65,5 +65,15 @@ class PartitionNode(NodeBase): edge_from.append(self.prev_partition.name) return (edge_from, edge_to) + # These all call back to the parent "partitioning" object to do + # the real work. Every node calls it, but only one will succeed; + # see the gating we do in the parent function. + # + # XXX: A better model here would be for the parent object to a + # real node in the config graph, so it's create() gets called. + # These can then just be stubs. def create(self): self.partitioning.create() + + def cleanup(self): + self.partitioning.cleanup() diff --git a/diskimage_builder/block_device/level1/partitioning.py b/diskimage_builder/block_device/level1/partitioning.py index ff2b2bc7..f7fd07ce 100644 --- a/diskimage_builder/block_device/level1/partitioning.py +++ b/diskimage_builder/block_device/level1/partitioning.py @@ -15,8 +15,6 @@ import logging import os -from subprocess import CalledProcessError - from diskimage_builder.block_device.exception import \ BlockDeviceSetupException from diskimage_builder.block_device.level1.mbr import MBR @@ -45,6 +43,7 @@ class Partitioning(PluginBase): # within one object, there is the need to store a flag if the # creation of the partitions was already done. self.already_created = False + self.already_cleaned = False # Parameter check if 'base' not in config: @@ -94,52 +93,10 @@ class Partitioning(PluginBase): fd.seek(0, 2) return fd.tell() - def _all_part_devices_exist(self, expected_part_devices): - for part_device in expected_part_devices: - logger.debug("Checking if partition device [%s] exists", - part_device) - if not os.path.exists(part_device): - logger.info("Partition device [%s] does not exists", - part_device) - return False - logger.debug("Partition already exists [%s]", part_device) - return True - - def _notify_os_of_partition_changes(self, device_path, partition_devices): - """Notify of of partition table changes - - There is the need to call some programs to inform the operating - system of partition tables changes. - These calls are highly distribution and version specific. Here - a couple of different methods are used to get the best result. - """ - try: - exec_sudo(["partprobe", device_path]) - exec_sudo(["udevadm", "settle"]) - except CalledProcessError as e: - logger.info("Ignoring settling failure: %s", e) - pass - - if self._all_part_devices_exist(partition_devices): - return - # If running inside Docker, make our nodes manually, because udev - # will not be working. - if os.path.exists("/.dockerenv"): - # kpartx cannot run in sync mode in docker. - exec_sudo(["kpartx", "-av", device_path]) - exec_sudo(["dmsetup", "--noudevsync", "mknodes"]) - return - - exec_sudo(["kpartx", "-avs", device_path]) - + # not this is NOT a node and this is not called directly! The + # create() calls in the partition nodes this plugin has + # created are calling back into this. def create(self): - # not this is NOT a node and this is not called directly! The - # create() calls in the partition nodes this plugin has - # created are calling back into this. - image_path = self.state['blockdev'][self.base]['image'] - device_path = self.state['blockdev'][self.base]['device'] - logger.info("Creating partition on [%s] [%s]", self.base, image_path) - # This is a bit of a hack. Each of the partitions is actually # in the graph, so for every partition we get a create() call # as the walk happens. But we only need to create the @@ -147,10 +104,16 @@ class Partitioning(PluginBase): if self.already_created: logger.info("Not creating the partitions a second time.") return + self.already_created = True + + # the raw file on disk + image_path = self.state['blockdev'][self.base]['image'] + # the /dev/loopX device of the parent + device_path = self.state['blockdev'][self.base]['device'] + logger.info("Creating partition on [%s] [%s]", self.base, image_path) assert self.label == 'mbr' - partition_devices = set() disk_size = self._size_of_block_dev(image_path) with MBR(image_path, disk_size, self.align) as part_impl: for part_cfg in self.partitions: @@ -170,11 +133,36 @@ class Partitioning(PluginBase): part_size, part_type) logger.debug("Create partition [%s] [%d]", part_name, part_no) - partition_device_name = device_path + "p%d" % part_no + + # We're going to mount all partitions with kpartx + # below once we're done. So the device this partition + # will be seen at becomes "/dev/mapper/loop0pX" + assert device_path[:5] == "/dev/" + partition_device_name = "/dev/mapper/%sp%d" % \ + (device_path[5:], part_no) self.state['blockdev'][part_name] \ = {'device': partition_device_name} - partition_devices.add(partition_device_name) - self.already_created = True - self._notify_os_of_partition_changes(device_path, partition_devices) + # now all the partitions are created, get device-mapper to + # mount them + if not os.path.exists("/.dockerenv"): + exec_sudo(["kpartx", "-avs", device_path]) + else: + # If running inside Docker, make our nodes manually, + # because udev will not be working. kpartx cannot run in + # sync mode in docker. + exec_sudo(["kpartx", "-av", device_path]) + exec_sudo(["dmsetup", "--noudevsync", "mknodes"]) + return + + def cleanup(self): + # remove the partition mappings made for the parent + # block-device by create() above. this is called from the + # child PartitionNode umount/delete/cleanup. Thus every + # partition calls it, but we only want to do it once and our + # gate. + if not self.already_cleaned: + self.already_cleaned = True + exec_sudo(["kpartx", "-d", + self.state['blockdev'][self.base]['device']]) diff --git a/diskimage_builder/elements/bootloader/finalise.d/50-bootloader b/diskimage_builder/elements/bootloader/finalise.d/50-bootloader index 6b956f80..80472156 100755 --- a/diskimage_builder/elements/bootloader/finalise.d/50-bootloader +++ b/diskimage_builder/elements/bootloader/finalise.d/50-bootloader @@ -9,8 +9,11 @@ fi set -eu set -o pipefail -PART_DEV=$IMAGE_BLOCK_DEVICE -BOOT_DEV=$IMAGE_BLOCK_DEVICE_WITHOUT_PART +BOOT_DEV=$IMAGE_BLOCK_DEVICE + +# All available devices, handy for some bootloaders... +declare -A DEVICES +eval DEVICES=( $IMAGE_BLOCK_DEVICES ) function install_extlinux { install-packages -m bootloader extlinux diff --git a/diskimage_builder/lib/common-functions b/diskimage_builder/lib/common-functions index c99c4747..4be4bb67 100644 --- a/diskimage_builder/lib/common-functions +++ b/diskimage_builder/lib/common-functions @@ -239,22 +239,6 @@ function run_d() { check_break after-$1 bash } -function detach_loopback() { - local loopdev=$1 - - # Remove the map if it exists - # If setup on a rhel or derivative the map was created with kpartx not losetup - # and subsequently needs to be removed. - loopdev_name=$(echo $loopdev | sed 's/\/dev\///g') - - if sudo dmsetup ls | grep $loopdev_name; then - mapper_name=$(sudo dmsetup ls | grep $loopdev_name | awk '{ print $1 }') - sudo dmsetup --noudevsync remove $mapper_name - fi - - return 0 -} - function arg_to_elements() { for arg do IMAGE_ELEMENT="$IMAGE_ELEMENT $arg" ; done diff --git a/diskimage_builder/lib/disk-image-create b/diskimage_builder/lib/disk-image-create index 88c64ed3..ff3dcebe 100644 --- a/diskimage_builder/lib/disk-image-create +++ b/diskimage_builder/lib/disk-image-create @@ -413,21 +413,28 @@ if [ -z ${IMAGE_BLOCK_DEVICE} ] ; then # values to dib-block-device: using the YAML config and dib-block-device create - # It's called 'DEVICE' but it's the partition. - IMAGE_BLOCK_DEVICE=$(dib-block-device getval image-block-partition) + # This is the device (/dev/loopX). It's where to install the + # bootloader. + IMAGE_BLOCK_DEVICE=$(dib-block-device getval image-block-device) + export IMAGE_BLOCK_DEVICE + + # Similar to above, but all mounted devices. This is handy for + # some bootloaders that have multi-partition layouts and want to + # copy things to different places other than just + # IMAGE_BLOCK_DEVICE. "eval" this into an array as needed + IMAGE_BLOCK_DEVICES=$(dib-block-device getval image-block-devices) + export IMAGE_BLOCK_DEVICES # Write the fstab dib-block-device writefstab fi -export IMAGE_BLOCK_DEVICE + +# XXX: needed? LOOPDEV=${IMAGE_BLOCK_DEVICE} -IMAGE_BLOCK_DEVICE_WITHOUT_PART=$(echo ${IMAGE_BLOCK_DEVICE} \ - | sed -e "s|^\(.*loop[0-9]*\)p[0-9]*$|\1|g") -export IMAGE_BLOCK_DEVICE_WITHOUT_PART - -export EXTRA_DETACH="detach_loopback ${IMAGE_BLOCK_DEVICE_WITHOUT_PART}" -export EXTRA_UNMOUNT="dib-block-device cleanup" +# At this point, dib-block-device has created the raw image file +# (IMAGE_BLOCK_DEVICE) and mounted all the partitions under +# $TMP_BUILD_DIR/mnt for us. We can now copy into the final image. # 'mv' is not usable here - especially when a top level directory # has the same name as a mount point of a partition. If so, 'mv' @@ -480,14 +487,15 @@ fi # Unmount and cleanup the /mnt and /build subdirectories, to save # space before converting the image to some other format. +# XXX ? needed? export EXTRA_UNMOUNT="" unmount_image TMP_IMAGE_PATH=$(dib-block-device getval image-path) export TMP_IMAGE_PATH +# remove all mounts dib-block-device umount - dib-block-device cleanup cleanup_build_dir