Merge pull request #1230 from nikita-dubrovskii/s390x_iso_kargs

s390x: enable 'iso kargs' commands

docs/release-notes.md

@@ -8,6 +8,7 @@ nav_order: 8
 
 Major changes:
 
+- iso: Support kargs modification on s390x by using `kargs.json` if present
 
 Minor changes:
 

fixtures/iso/INDEX

@@ -48,3 +48,19 @@ embed-areas-2023-03.s390x.iso.xz
     features.json: installer-config, installer-config-directives,
         live-initrd-network
     installer-config-directives from 0.16.0
+
+embed-areas-2023-07.x86_64.iso.xz
+    igninfo.json pointing to ignition.img (no offset and no length)
+    kargs.json pointing to kargs areas and embedding defaults (with pad and end)
+    miniso.dat version 1
+    features.json: installer-config, installer-config-directives,
+        live-initrd-network
+    installer-config-directives from 0.16.0
+
+embed-areas-2023-07.s390x.iso.xz
+    igninfo.json pointing to cdboot.img (offset and length)
+    kargs.json pointing to kargs areas and embedding defaults (with pad and end)
+    miniso.dat version 1
+    features.json: installer-config, installer-config-directives,
+        live-initrd-network
+    installer-config-directives from 0.16.0

src/live/embed.rs

@@ -162,10 +162,20 @@ struct Region {
     pub contents: Vec<u8>,
     #[serde(skip_serializing)]
     pub modified: bool,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pad: Option<char>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    end: Option<char>,
 }
 
 impl Region {
-    pub fn read(file: &mut File, offset: u64, length: usize) -> Result<Self> {
+    pub fn read(
+        file: &mut File,
+        offset: u64,
+        length: usize,
+        pad: Option<char>,
+        end: Option<char>,
+    ) -> Result<Self> {
         let mut contents = vec![0; length];
         file.seek(SeekFrom::Start(offset))
             .with_context(|| format!("seeking to offset {offset}"))?;
@@ -176,6 +186,8 @@ impl Region {
             length,
             contents,
             modified: false,
+            pad,
+            end,
         })
     }
 
@@ -280,6 +292,10 @@ struct KargEmbedInfo {
 struct KargEmbedLocation {
     path: String,
     offset: u64,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pad: Option<char>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    end: Option<char>,
 }
 
 impl KargEmbedInfo {
@@ -361,6 +377,8 @@ impl KargEmbedAreas {
                     iso.as_file()?,
                     iso_file.address.as_offset() + loc.offset,
                     info.size,
+                    loc.pad,
+                    loc.end,
                 )
                 .context("reading kargs embed area")?,
             );
@@ -381,6 +399,8 @@ impl KargEmbedAreas {
             file,
             32768 - COREOS_INITRD_HEADER_SIZE - COREOS_KARG_EMBED_AREA_HEADER_SIZE,
             COREOS_KARG_EMBED_AREA_HEADER_SIZE as usize,
+            None,
+            None,
         )
         .context("reading karg embed header")?;
         let mut header = &region.contents[..];
@@ -407,7 +427,8 @@ impl KargEmbedAreas {
 
         // default kargs
         let offset = header.get_u64_le();
-        let default_region = Region::read(file, offset, length).context("reading default kargs")?;
+        let default_region =
+            Region::read(file, offset, length, None, None).context("reading default kargs")?;
         let default = Self::parse(&default_region)?;
 
         // writable regions
@@ -417,7 +438,10 @@ impl KargEmbedAreas {
             if offset == 0 {
                 break;
             }
-            regions.push(Region::read(file, offset, length).context("reading kargs embed area")?);
+            regions.push(
+                Region::read(file, offset, length, None, None)
+                    .context("reading kargs embed area")?,
+            );
         }
 
         Some(Self::build(length, default, regions)).transpose()
@@ -454,7 +478,8 @@ impl KargEmbedAreas {
     fn parse(region: &Region) -> Result<String> {
         Ok(String::from_utf8(region.contents.clone())
             .context("invalid UTF-8 in karg area")?
-            .trim_end_matches('#')
+            .trim_end_matches(region.pad.unwrap_or('#'))
+            .trim_end_matches(region.end.unwrap_or('\n'))
             .trim()
             .into())
     }
@@ -469,17 +494,20 @@ impl KargEmbedAreas {
 
     pub fn set_kargs(&mut self, kargs: &str) -> Result<()> {
         let unformatted = kargs.trim();
-        let formatted = unformatted.to_string() + "\n";
-        if formatted.len() > self.length {
+        if unformatted.len() >= self.length {
             bail!(
                 "kargs too large for area: {} vs {}",
-                formatted.len(),
+                unformatted.len() + 1,
                 self.length
             );
         }
-        let mut contents = vec![b'#'; self.length];
-        contents[..formatted.len()].copy_from_slice(formatted.as_bytes());
+
         for region in &mut self.regions {
+            let mut formatted = unformatted.to_string();
+            formatted.push(region.end.unwrap_or('\n'));
+            let pad = region.pad.unwrap_or('#');
+            let mut contents = vec![pad as u8; self.length];
+            contents[..formatted.len()].copy_from_slice(formatted.as_bytes());
             region.contents = contents.clone();
             region.modified = true;
         }
@@ -538,7 +566,7 @@ impl InitrdEmbedArea {
             .length
             .unwrap_or(f.length as usize - file_offset as usize);
         // read (checks offset/length as a side effect)
-        let mut region = Region::read(iso.as_file()?, iso_offset, length)
+        let mut region = Region::read(iso.as_file()?, iso_offset, length, None, None)
             .context("reading initrd embed area")?;
         let initrd = if region.contents.iter().any(|v| *v != 0) {
             Initrd::from_reader(&*region.contents).context("decoding initrd embed area")?

tests/images.sh

@@ -29,6 +29,8 @@ fixtures=(
     embed-areas-2022-09.iso.xz
     embed-areas-2023-03.x86_64.iso.xz
     embed-areas-2023-03.s390x.iso.xz
+    embed-areas-2023-07.x86_64.iso.xz
+    embed-areas-2023-07.s390x.iso.xz
 )
 
 msg() {

tests/images/iso-extract-minimal-iso.sh

@@ -22,8 +22,8 @@ grepq() {
 iso=$1; shift
 iso=$(realpath "${iso}")
 
-if [[ "${iso}" == *.s390x.* ]]; then
-    echo "Skipped; minimal ISO not supported on s390x"
+if [[ "${iso}" == *2023-03.s390x.* ]]; then
+    echo "Skipped; minimal ISO not supported on s390x before July 2023"
     exit 0
 fi
 

tests/images/iso-kargs.sh

@@ -22,9 +22,13 @@ grepq() {
 iso=$1; shift
 iso=$(realpath "${iso}")
 
+s390x=
 if [[ "${iso}" == *.s390x.* ]]; then
-    echo "Skipped; kargs not supported on s390x"
-    exit 0
+    s390x=1
+    if [[ "${iso}" == *2023-03.s390x.* ]]; then
+        echo "Skipped; kargs not supported on s390x before July 2023"
+        exit 0
+    fi
 fi
 
 tmpd=$(mktemp -d)
@@ -120,9 +124,22 @@ offset=$(coreos-installer dev show iso --kargs "${iso}" | jq -r .kargs[0].offset
 length=$(coreos-installer dev show iso --kargs "${iso}" | jq -r .kargs[0].length)
 expected_args="$(coreos-installer iso kargs show --default "${iso}") foobar=val"
 expected_args_len="$(echo -n "${expected_args}" | wc -c)"
-filler="$(printf '%*s' $((${length} - ${expected_args_len} - 1)) | tr ' ' '#')"
-if ! echo -en "${expected_args}\n${filler}" | cmp -s <(dd if=${iso} skip=${offset} count=${length} bs=1 status=none) -; then
-    fatal "Failed to manually round-trip kargs"
+if [[ -n "${s390x}" ]]; then
+    # There is no filler or newline character on s390x's kargs line, but null bytes:
+    #    $ hexdump -C -s 138457 -n 896 builds/latest/s390x/*.iso
+    #    00021cd9  69 67 6e 69 74 69 6f 6e  2e 70 6c 61 74 66 6f 72  |ignition.platfor|
+    #    00021ce9  6d 2e 69 64 3d 6d 65 74  61 6c 00 00 00 00 00 00  |m.id=metal......|
+    # Create temp file with kargs and nulls and compare it to bytes of the iso image:
+    echo -n "${expected_args}" > cmdline.s390x
+    truncate -s "${length}" cmdline.s390x
+    if ! cmp -s <(dd if=${iso} skip=${offset} count=${length} bs=1 status=none) cmdline.s390x; then
+        fatal "Failed to manually round-trip kargs"
+    fi
+else
+    filler="$(printf '%*s' $((${length} - ${expected_args_len} - 1)) | tr ' ' '#')"
+    if ! echo -en "${expected_args}\n${filler}" | cmp -s <(dd if=${iso} skip=${offset} count=${length} bs=1 status=none) -; then
+        fatal "Failed to manually round-trip kargs"
+    fi
 fi
 
 # Done