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

s390x: add support for IBM Z DASD disks #153

Merged
merged 3 commits into from
Jul 10, 2020
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
7 changes: 7 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ rust:
- beta
- nightly

arch:
- amd64
- s390x

matrix:
allow_failures:
- rust: nightly
# VM apparently has broken network access
- rust: 1.41.0
arch: s390x
Copy link
Member

Choose a reason for hiding this comment

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

Wait, what? /me reads https://docs.travis-ci.com/user/multi-cpu-architectures/.

Travis CI can test on amd64, ppc64le (IBM Power CPUs), s390x (IBM Z CPUs) and arm64 (run on ARMv8 compliant CPUs) if the operating system is Linux.

Whoa, didn't realize this! We could use that in cosa at least to get some multi-arch coverage, even if just of ./build.sh (though definitely worth checking if it also has virt support, but likely not).


env:
global:
Expand Down
104 changes: 104 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ path = "src/main.rs"
[profile.release]
lto = true

[target.'cfg(target_arch = "s390x")'.dependencies]
gptman = { version = "^0.6", default-features = false }

[dependencies]
bincode = "^1.3"
byte-unit = "^4.0"
Expand Down
19 changes: 13 additions & 6 deletions scripts/coreos-installer-service
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ PERSIST_KERNEL_NET_PARAMS=("ipv6.disable" "net.ifnames" "net.naming-scheme")
# List from https://www.mankier.com/7/dracut.cmdline#Description-Network
PERSIST_DRACUT_NET_PARAMS=("ip" "ifname" "rd.route" "bootdev" "BOOTIF" "rd.bootif" "nameserver" "rd.peerdns" "biosdevname" "vlan" "bond" "team" "bridge" "rd.net.timeout.carrier" "coreos.no_persist_ip")

# IBM S390X params to persist
PERSIST_S390X_PARAMS=("rd.dasd" "rd.zfcp" "rd.znet" "zfcp.allow_lun_scan" "cio_ignore")
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, do we really need this? There is a push in general to not add support for more kargs than we already have and instead direct users towards scripting coreos-installer via Ignition: #124.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand it:

  • rd.dasd activates a DASD device, without which a DASD boot device won't show up
  • rd.zfcp activates a Fibre Channel device, without which an FC boot device won't show up
  • rd.znet activates a network device, without which it won't be available from the initramfs
  • zfcp.allow_lun_scan might be needed to configure activation of certain FC devices
  • cio_ignore tells the kernel not to attach to specified devices on boot

So I suspect they might all be needed. @nikita-dubrovskii, is that correct? Any additional details?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I suspect they might all be needed.

Exactly, we have to persist those kargs.


args=("install")

cmdline=( $(</proc/cmdline) )
Expand Down Expand Up @@ -61,19 +64,24 @@ if [ -n "${ignition_url}" -a "${ignition_url}" != "skip" ]; then
args+=("--ignition-url" "${ignition_url}" "--insecure-ignition")
fi

# First-boot kernel arguments. Filter out any args that aren't in our
# list of networking arguments to forward and store them in $firstboot_args
# Forward whitelisted kernel arguments to the installed system. We have
# separate sets of whitelists for first-boot kargs and persistent kargs.
# Only pass --firstboot-kargs if additional networking options have been
# specified, since the default in coreos-assembler specifies
# `rd.neednet=1 ip=dhcp` when no options are persisted.
firstboot_args=""
for item in "${cmdline[@]}"; do
for param in "${PERSIST_KERNEL_NET_PARAMS[@]}" "${PERSIST_DRACUT_NET_PARAMS[@]}"; do
if [[ $item =~ ^$param(=.*)?$ ]]; then
firstboot_args+="${item} "
fi
done
for param in "${PERSIST_S390X_PARAMS[@]}"; do
if [[ $item =~ ^$param(=.*)?$ ]]; then
args+=("--append-karg" "${item}")
fi
done
done
# Only pass firstboot-kargs if additional networking options have been
# specified, since the default in ignition-dracut specifies
# `rd.neednet=1 ip=dhcp` when no options are persisted
if [ -n "${firstboot_args}" ]; then
args+=("--firstboot-args" "rd.neednet=1 ${firstboot_args}")
fi
Expand All @@ -94,4 +102,3 @@ udevadm settle
# Install
echo "coreos-installer ${args[@]}"
coreos-installer "${args[@]}"

29 changes: 13 additions & 16 deletions src/blockdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use nix::{errno::Errno, mount};
use regex::Regex;
use std::collections::HashMap;
use std::convert::TryInto;
use std::fs::{metadata, read_dir, read_link, read_to_string, remove_dir, File, OpenOptions};
use std::fs::{metadata, read_dir, read_to_string, remove_dir, File, OpenOptions};
use std::num::{NonZeroU32, NonZeroU64};
use std::os::linux::fs::MetadataExt;
use std::os::raw::c_int;
Expand All @@ -30,6 +30,7 @@ use std::thread::sleep;
use std::time::Duration;

use crate::errors::*;
use crate::io::resolve_link;

#[derive(Debug)]
pub struct Disk {
Expand Down Expand Up @@ -374,21 +375,17 @@ impl Partition {
// Now assume a kpartx "partition", where the path is a symlink to
// an unpartitioned DM device node.
// /sys/block/dm-1
match read_link(Path::new(&self.path)) {
Ok(target) => {
let devdir = basedir.join(
Path::new(&target)
.file_name()
.chain_err(|| format!("target {} has no filename", self.path))?,
);
if devdir.exists() {
return Ok(devdir);
}
let (target, is_link) = resolve_link(&self.path)?;
if is_link {
let devdir = basedir.join(
target
.file_name()
.chain_err(|| format!("target {} has no filename", target.display()))?,
);
if devdir.exists() {
return Ok(devdir);
}
// ignore if not symlink
Err(e) if e.kind() == std::io::ErrorKind::InvalidInput => (),
Err(e) => return Err(e).chain_err(|| format!("reading link {}", self.path)),
};
}

// Give up
bail!(
Expand Down Expand Up @@ -572,7 +569,7 @@ mod ioctl {
ioctl_read!(blkgetsize64, 0x12, 114, libc::size_t);
}

fn udev_settle() -> Result<()> {
pub fn udev_settle() -> Result<()> {
// "udevadm settle" silently no-ops if the udev socket is missing, and
// then lsblk can't find partition labels. Catch this early.
if !Path::new("/run/udev/control").exists() {
Expand Down
52 changes: 38 additions & 14 deletions src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,14 @@ fn write_image_and_sig(

// download and verify image
// don't check sector size
write_image(source, &mut dest, decompress, None)?;
write_image(
source,
&mut dest,
path,
image_copy_default,
decompress,
None,
)?;

// write signature, if relevant
if let (false, Some(signature)) = (decompress, source.signature.as_ref()) {
Expand All @@ -164,12 +171,17 @@ fn write_image_and_sig(
}

/// Copy the image to disk and verify its signature.
pub fn write_image(
pub fn write_image<F>(
source: &mut ImageSource,
dest: &mut File,
dest_path: &Path,
image_copy: F,
decompress: bool,
expected_sector_size: Option<NonZeroU32>,
) -> Result<()> {
) -> Result<()>
where
F: FnOnce(&[u8], &mut dyn Read, &mut File, &Path) -> Result<()>,
{
// wrap source for GPG verification
let mut verify_reader: Box<dyn Read> = {
if let Some(signature) = source.signature.as_ref() {
Expand Down Expand Up @@ -269,6 +281,27 @@ pub fn write_image(
}
}

// call the callback to copy the image
image_copy(&first_mb, &mut decompress_reader, dest, dest_path)?;

// flush
dest.flush().chain_err(|| "flushing data to disk")?;
dest.sync_all().chain_err(|| "syncing data to disk")?;

// if we reported progress using CRs, log final newline
if stderr_is_tty {
eprintln!();
}

Ok(())
}

pub fn image_copy_default(
first_mb: &[u8],
source: &mut dyn Read,
dest: &mut File,
_dest_path: &Path,
) -> Result<()> {
// Cache the first MiB and write zeroes to dest instead. This ensures
// that the disk image can't be used accidentally before its GPG signature
// is verified.
Expand All @@ -287,23 +320,14 @@ pub fn write_image(
// sparse-copy the image, falling back to non-sparse copy if hardware
// acceleration is unavailable. But BLKZEROOUT doesn't support
// BLKDEV_ZERO_NOFALLBACK, so we'd risk gigabytes of redundant I/O.
copy(&mut decompress_reader, dest).chain_err(|| "decoding and writing image")?;
copy(source, dest).chain_err(|| "decoding and writing image")?;

// verify_reader has now checked the signature, so fill in the first MiB
dest.seek(SeekFrom::Start(0))
.chain_err(|| "seeking to start of disk")?;
dest.write_all(&first_mb)
dest.write_all(first_mb)
.chain_err(|| "writing to first MiB of disk")?;

// flush
dest.flush().chain_err(|| "flushing data to disk")?;
dest.sync_all().chain_err(|| "syncing data to disk")?;

// if we reported progress using CRs, log final newline
if stderr_is_tty {
eprintln!();
}

Ok(())
}

Expand Down
Loading