From e79ba14bcd0f19418b1bb76f469d9666033854ac Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:46:47 -0300 Subject: [PATCH 1/5] refactor(xcat-test): move the vmothersetting machine type logic into a helper reg_linux_diskless_installation_flat sets machine:invalid, a valid machine type and then no machine type in vmothersetting with three long inline commands. The bats test for them cut the commands out of the case file with a regex. Move the three commands unchanged into commoncmd/vm_machine_type.sh and call it from the case. The bats test now sources the helper and runs the case's three steps against a stubbed lsdef and chdef. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- .../testcase/commoncmd/vm_machine_type.sh | 51 +++++++++ .../reg_linux_diskless_installation_flat | 6 +- .../diskless_flat_vmothersetting_machine.bats | 102 ------------------ xCAT-test/bats/vm_machine_type.bats | 81 ++++++++++++++ 4 files changed, 135 insertions(+), 105 deletions(-) create mode 100755 xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh delete mode 100644 xCAT-test/bats/diskless_flat_vmothersetting_machine.bats create mode 100644 xCAT-test/bats/vm_machine_type.bats diff --git a/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh b/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh new file mode 100755 index 000000000..1f53678cc --- /dev/null +++ b/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh @@ -0,0 +1,51 @@ +#!/bin/bash +# vm_machine_type.sh - set and clear the KVM machine type in vmothersetting of a node. +# +# Usage: +# vm_machine_type.sh invalid add machine:invalid, so the VM cannot start +# vm_machine_type.sh apply replace machine:invalid with a valid type +# vm_machine_type.sh restore remove the valid type again + +vm_machine_type_for_arch() +{ + local arch="$1" str3 + if [[ "$arch" =~ "ppc64" ]]; then str3="machine:pseries-rhel7.6.0"; elif [[ "$arch" =~ "x86_64" ]]; then str3="machine:pc"; elif [[ "$arch" =~ "riscv64" ]]; then str3="machine:virt"; elif [[ "$arch" =~ "aarch64" ]]; then str3="machine:virt";fi + printf '%s\n' "$str3" +} + +vm_machine_type_invalid() +{ + local node="$1" str1 str2 str3 str4 + str1=`lsdef $node | grep vmothersetting | cut -d '=' -f 2`;str2=";"; str3="machine:invalid"; if [ -z $str1 ]; then str4=$str3; else str4=$str1$str2$str3;fi; chdef $node vmothersetting=$str4 +} + +vm_machine_type_apply() +{ + local node="$1" arch="$2" str1 str2 str3 str4 str5 + str1=`lsdef $node | grep vmothersetting | cut -d '=' -f 2`;str2="machine:invalid"; str3=$(vm_machine_type_for_arch "$arch"); if [ "$str1" == "$str2" ]; then str5=$str3; else str4=`echo $str1 | sed -e "s/$str2//"`;str5=$str4$str3;fi; chdef $node vmothersetting=$str5 +} + +vm_machine_type_restore() +{ + local node="$1" arch="$2" str1 str2 str3 str4 + str1=`lsdef $node | grep vmothersetting | cut -d '=' -f 2`;str2=";"; str3=$(vm_machine_type_for_arch "$arch"); if [ "$str1" == "$str3" ]; then chdef $node vmothersetting=; else str4=`echo $str1 | sed -e "s/$str2$str3//"`; chdef $node vmothersetting=$str4;fi +} + +vm_machine_type_main() +{ + local action="$1" + shift + case "$action" in + invalid) vm_machine_type_invalid "$@" ;; + apply) vm_machine_type_apply "$@" ;; + restore) vm_machine_type_restore "$@" ;; + *) + echo "usage: vm_machine_type.sh {invalid |apply |restore }" >&2 + return 2 + ;; + esac +} + +if [ "${BASH_SOURCE[0]}" = "$0" ]; then + vm_machine_type_main "$@" +fi diff --git a/xCAT-test/autotest/testcase/installation/reg_linux_diskless_installation_flat b/xCAT-test/autotest/testcase/installation/reg_linux_diskless_installation_flat index d1b3f77f9..b73672192 100644 --- a/xCAT-test/autotest/testcase/installation/reg_linux_diskless_installation_flat +++ b/xCAT-test/autotest/testcase/installation/reg_linux_diskless_installation_flat @@ -46,7 +46,7 @@ cmd:rm -rf /tmp/mountoutput cmd:packimage __GETNODEATTR($$CN,os)__-__GETNODEATTR($$CN,arch)__-netboot-compute check:rc==0 -cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then str1=`lsdef $$CN | grep vmothersetting | cut -d '=' -f 2`;str2=";"; str3="machine:invalid"; if [ -z $str1 ]; then str4=$str3; else str4=$str1$str2$str3;fi; chdef $$CN vmothersetting=$str4; fi +cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then /opt/xcat/share/xcat/tools/autotest/testcase/commoncmd/vm_machine_type.sh invalid $$CN; fi check:rc==0 cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then str=`lsdef $$CN | grep vmothersetting`; if [[ $str =~ "invalid" ]]; then exit 0; else exit 1; fi; fi check:rc==0 @@ -57,7 +57,7 @@ check:rc==0 cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then str=`/opt/xcat/share/xcat/tools/autotest/testcase/commoncmd/retry_install.sh $$CN __GETNODEATTR($$CN,os)__-__GETNODEATTR($$CN,arch)__-netboot-compute 1`; if [[ $str =~ "failed" ]]; then exit 0; else exit 1; fi; fi check:rc==0 -cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then str1=`lsdef $$CN | grep vmothersetting | cut -d '=' -f 2`;str2="machine:invalid"; if [[ "__GETNODEATTR($$CN,arch)__" =~ "ppc64" ]]; then str3="machine:pseries-rhel7.6.0"; elif [[ "__GETNODEATTR($$CN,arch)__" =~ "x86_64" ]]; then str3="machine:pc"; elif [[ "__GETNODEATTR($$CN,arch)__" =~ "riscv64" ]]; then str3="machine:virt"; elif [[ "__GETNODEATTR($$CN,arch)__" =~ "aarch64" ]]; then str3="machine:virt";fi; if [ "$str1" == "$str2" ]; then str5=$str3; else str4=`echo $str1 | sed -e "s/$str2//"`;str5=$str4$str3;fi; chdef $$CN vmothersetting=$str5; fi +cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then /opt/xcat/share/xcat/tools/autotest/testcase/commoncmd/vm_machine_type.sh apply $$CN __GETNODEATTR($$CN,arch)__; fi check:rc==0 cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then str=`lsdef $$CN | grep vmothersetting`; if [[ $str =~ "machine" ]]; then exit 0; else exit 1; fi; fi check:rc==0 @@ -68,7 +68,7 @@ check:rc==0 cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then str=`/opt/xcat/share/xcat/tools/autotest/testcase/commoncmd/retry_install.sh $$CN __GETNODEATTR($$CN,os)__-__GETNODEATTR($$CN,arch)__-netboot-compute`; echo $str; if [[ $str =~ "The provision succeeded" ]]; then exit 0; else exit 1; fi; fi check:rc==0 -cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then str1=`lsdef $$CN | grep vmothersetting | cut -d '=' -f 2`;str2=";"; if [[ "__GETNODEATTR($$CN,arch)__" =~ "ppc64" ]]; then str3="machine:pseries-rhel7.6.0"; elif [[ "__GETNODEATTR($$CN,arch)__" =~ "x86_64" ]]; then str3="machine:pc"; elif [[ "__GETNODEATTR($$CN,arch)__" =~ "riscv64" ]]; then str3="machine:virt"; elif [[ "__GETNODEATTR($$CN,arch)__" =~ "aarch64" ]]; then str3="machine:virt";fi; if [ "$str1" == "$str3" ]; then chdef $$CN vmothersetting=; else str4=`echo $str1 | sed -e "s/$str2$str3//"`; chdef $$CN vmothersetting=$str4;fi; fi +cmd:if [ "__GETNODEATTR($$CN,mgt)__" == "kvm" ]; then /opt/xcat/share/xcat/tools/autotest/testcase/commoncmd/vm_machine_type.sh restore $$CN __GETNODEATTR($$CN,arch)__; fi check:rc==0 cmd:if [ "__GETNODEATTR($$CN,arch)__" != "ppc64" -a "__GETNODEATTR($$CN,mgt)__" != "ipmi" -a "__GETNODEATTR($$CN,mgt)__" != "openbmc" ];then echo "CN node $$CN is a VM, mgt is __GETNODEATTR($$CN,mgt)__, starting to recreate the vm"; echo "rpower $$CN off"; rpower $$CN off; sleep 3; echo "rpower $$CN stat"; rpower $$CN stat; var=`expr substr "__GETNODEATTR($$CN,vmstorage)__" 1 3`; echo "The disk type of $$CN is $var"; if [ "$var" = "phy" ]; then echo "mkvm $$CN"; mkvm $$CN; echo "rmvm $$CN -f -p"; rmvm $$CN -f -p; echo "mkvm $$CN"; mkvm $$CN; exit $?; elif [ "$var" = "dir" ]; then echo "mkvm $$CN -s 30G -f"; mkvm $$CN -s 30G -f; echo "rmvm $$CN -f -p"; rmvm $$CN -f -p; echo "mkvm $$CN -s 30G -f"; mkvm $$CN -s 30G -f; exit $?; elif ["$var" = "nfs" -o "$var" = "lvm" ];then echo "Need to fix disk type $var"; exit 2; else echo "Unsupported disk type $var"; exit 3;fi;else echo "CN node $$CN is not a VM; do not need to recreate it";fi check:rc==0 diff --git a/xCAT-test/bats/diskless_flat_vmothersetting_machine.bats b/xCAT-test/bats/diskless_flat_vmothersetting_machine.bats deleted file mode 100644 index 1246f0334..000000000 --- a/xCAT-test/bats/diskless_flat_vmothersetting_machine.bats +++ /dev/null @@ -1,102 +0,0 @@ -#!/usr/bin/env bats -# -# reg_linux_diskless_installation_flat corrupts the KVM machine type of the compute node, checks -# that the node fails to boot, and then restores it. The restore reads a machine type from a -# ladder that names ppc64 and x86_64 only, so on any other architecture it writes an empty -# vmothersetting and the check after it fails. -# -# The two commands are lifted out of the case file and RUN, with lsdef and chdef shadowed, so -# the assertions read the value the case would write. The extraction fails the test when it -# stops matching, so a rewrite fails loudly instead of covering nothing. - -load 'helpers/shell_source' - -setup() -{ - CASE="$(repo_path 'xCAT-test/autotest/testcase/installation/reg_linux_diskless_installation_flat')" - [ -r "$CASE" ] || skip "$CASE is required" - export CASE -} - -# The command that restores the machine type, and the one that removes it again afterwards. -restore_command() -{ - extract_first_matching_line "$CASE" \ - '^cmd:.*str2="machine:invalid".*chdef [$][$]CN vmothersetting=[$]str5' -} - -remove_command() -{ - extract_first_matching_line "$CASE" \ - '^cmd:.*str2=";".*~ "ppc64".*chdef [$][$]CN vmothersetting=' -} - -# Render one command the way xcattest does, then run it with lsdef and chdef shadowed. bash -# resolves a function ahead of PATH, so the case's own backticks read the stub. -# -# Sets OUT to everything the command printed and WRITTEN to the value it gave chdef. -run_case_command() -{ - local cmd="$1" arch="$2" lsdef_value="$3" - - cmd="${cmd#cmd:}" - cmd="${cmd//__GETNODEATTR(\$\$CN,arch)__/$arch}" - cmd="${cmd//__GETNODEATTR(\$\$CN,mgt)__/kvm}" - cmd="${cmd//\$\$CN/cn1}" - - OUT="$(bash -c "lsdef() { echo ' vmothersetting=$lsdef_value'; } -chdef() { echo \"CHDEF:[\$*]\"; } -$cmd" 2>&1)" || true - # The brackets keep an empty value apart from no call at all: the cleanup is meant to write - # an empty vmothersetting, and a command that never reaches chdef must not read as that. - CHDEF_CALLS="$(grep -c '^CHDEF:' <<<"$OUT" || true)" - WRITTEN="$(sed -n 's/^CHDEF:\[cn1 vmothersetting=\(.*\)\]$/\1/p' <<<"$OUT")" -} - -# The machine type each architecture must end up with. riscv64 guests run the qemu "virt" -# machine; kvm.pm sets it in guest_arch_profile. -# -# The restore writes the machine type; the cleanup after it takes the same machine type away -# again and leaves every other setting. Both read the same ladder, so both are checked against -# the same value. -assert_arch() -{ - local arch="$1" machine="$2" restore remove - - restore="$(restore_command)" - remove="$(remove_command)" - - # The node carries the corrupt value only. - run_case_command "$restore" "$arch" 'machine:invalid' - [ "$CHDEF_CALLS" -eq 1 ] - [ "$WRITTEN" = "machine:$machine" ] - - # The node carries a setting of its own beside the corrupt value. - run_case_command "$restore" "$arch" 'cpumode:host-passthrough;machine:invalid' - [ "$CHDEF_CALLS" -eq 1 ] - [ "$WRITTEN" = "cpumode:host-passthrough;machine:$machine" ] - - # The cleanup, with nothing but the machine type to remove. - run_case_command "$remove" "$arch" "machine:$machine" - [ "$(grep -c 'unary operator expected' <<<"$OUT")" -eq 0 ] - [ "$CHDEF_CALLS" -eq 1 ] - [ "$WRITTEN" = "" ] - - # The cleanup, with a setting of its own that must survive it. - run_case_command "$remove" "$arch" "cpumode:host-passthrough;machine:$machine" - [ "$(grep -c 'unary operator expected' <<<"$OUT")" -eq 0 ] - [ "$CHDEF_CALLS" -eq 1 ] - [ "$WRITTEN" = "cpumode:host-passthrough" ] -} - -@test "ppc64le: the restore writes the machine type and the cleanup takes it away" { - assert_arch ppc64le pseries-rhel7.6.0 -} - -@test "x86_64: the restore writes the machine type and the cleanup takes it away" { - assert_arch x86_64 pc -} - -@test "riscv64: the restore writes the machine type and the cleanup takes it away" { - assert_arch riscv64 virt -} diff --git a/xCAT-test/bats/vm_machine_type.bats b/xCAT-test/bats/vm_machine_type.bats new file mode 100644 index 000000000..14aa0cd5c --- /dev/null +++ b/xCAT-test/bats/vm_machine_type.bats @@ -0,0 +1,81 @@ +#!/usr/bin/env bats +# vm_machine_type.sh sets machine:invalid, then a valid machine type, then clears it again. +# lsdef and chdef are shell functions that read and write vmothersetting of node cn1 in a file. + +load 'helpers/shell_source' + +setup() +{ + source "$(require_repo_file 'xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh')" + VMOTHER="${BATS_TEST_TMPDIR}/vmothersetting" + export VM_MACHINE_TYPE_STATE_DIR="${BATS_TEST_TMPDIR}/state" + mkdir -p "$VM_MACHINE_TYPE_STATE_DIR" +} + +# lsdef prints no line for an attribute without a value. +lsdef() +{ + printf 'Object name: %s\n' "$1" + if [ -s "$VMOTHER" ]; then + printf ' vmothersetting=%s\n' "$(cat "$VMOTHER")" + fi +} + +chdef() +{ + [ "$1" = cn1 ] || return 1 + case "$2" in + vmothersetting=*) printf '%s' "${2#vmothersetting=}" >"$VMOTHER" ;; + *) return 1 ;; + esac + CHDEF_CALLS=$((CHDEF_CALLS + 1)) +} + +set_vmother() +{ + printf '%s' "$1" >"$VMOTHER" +} + +vmother() +{ + cat "$VMOTHER" +} + +# assert_arch -- the value after each step of the case. +assert_arch() +{ + local arch="$1" machine="$2" original="$3" sep='' + [ -z "$original" ] || sep=';' + CHDEF_CALLS=0 + + set_vmother "$original" + vm_machine_type_main invalid cn1 + [ "$(vmother)" = "${original}${sep}machine:invalid" ] + + vm_machine_type_main apply cn1 "$arch" + [ "$(vmother)" = "${original}${sep}machine:$machine" ] + + vm_machine_type_main restore cn1 "$arch" + [ "$(vmother)" = "$original" ] + [ "$CHDEF_CALLS" -eq 3 ] +} + +@test "ppc64le: the case sets and clears pseries-rhel7.6.0" { + assert_arch ppc64le pseries-rhel7.6.0 '' + assert_arch ppc64le pseries-rhel7.6.0 'cpumode:host-passthrough' +} + +@test "x86_64: the case sets and clears pc" { + assert_arch x86_64 pc '' + assert_arch x86_64 pc 'cpumode:host-passthrough' +} + +@test "riscv64: the case sets and clears virt" { + assert_arch riscv64 virt '' + assert_arch riscv64 virt 'cpumode:host-passthrough' +} + +@test "aarch64: the case sets and clears virt" { + assert_arch aarch64 virt '' + assert_arch aarch64 virt 'cpumode:host-passthrough' +} From 5eeedd41ad77e6ecba0ca0cf08878088a4e0185c Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:47:27 -0300 Subject: [PATCH 2/5] test(xcat-test): vm_machine_type.sh restores the original vmothersetting vm_machine_type.sh removes the machine type it added with a substring sed. When vmothersetting already names a versioned machine type, the sed cuts the first match out of the user's own value: cpumode:host-passthrough;machine:pc-q35-9.2 comes back as cpumode:host-passthrough-q35-9.2;machine:pc. Run the three steps of the case and require the exact original value at the end, for an empty value, machine:invalid, versioned x86_64 and riscv64 machine types, and several settings. Also require that apply keeps a machine type the node already names. Three round trips and the apply test fail on the current helper. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/bats/vm_machine_type.bats | 52 +++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/xCAT-test/bats/vm_machine_type.bats b/xCAT-test/bats/vm_machine_type.bats index 14aa0cd5c..cbf184d48 100644 --- a/xCAT-test/bats/vm_machine_type.bats +++ b/xCAT-test/bats/vm_machine_type.bats @@ -79,3 +79,55 @@ assert_arch() assert_arch aarch64 virt '' assert_arch aarch64 virt 'cpumode:host-passthrough' } + +# assert_round_trip -- the case ends with the exact value it started with. +assert_round_trip() +{ + local arch="$1" original="$2" machine + + set_vmother "$original" + vm_machine_type_main invalid cn1 + # kvm.pm uses the last machine: field of vmothersetting. + machine="$(vmother | tr ';' '\n' | grep '^machine:' | tail -n 1)" + [ "$machine" = "machine:invalid" ] + + vm_machine_type_main apply cn1 "$arch" + vm_machine_type_main restore cn1 "$arch" + [ "$(vmother)" = "$original" ] + [ -z "$(ls -A "$VM_MACHINE_TYPE_STATE_DIR")" ] +} + +@test "round trip: no vmothersetting" { + assert_round_trip x86_64 '' +} + +@test "round trip: machine:invalid" { + assert_round_trip x86_64 'machine:invalid' +} + +@test "round trip: cpumode:host-passthrough" { + assert_round_trip x86_64 'cpumode:host-passthrough' +} + +@test "round trip: x86_64 with a versioned machine type" { + assert_round_trip x86_64 'cpumode:host-passthrough;machine:pc-q35-9.2' +} + +@test "round trip: riscv64 with a versioned machine type" { + assert_round_trip riscv64 'machine:virt-9.2' +} + +@test "round trip: riscv64 with a versioned machine type after another setting" { + assert_round_trip riscv64 'cpumode:host-passthrough;machine:virt-9.2' +} + +@test "round trip: several settings" { + assert_round_trip ppc64le 'vcpupin:0-3;membind:0;devpassthrough:pci_0000_01_00_0,pci_0000_02_00_0;cpumode:host-model' +} + +@test "apply keeps a machine type the node already names" { + set_vmother 'cpumode:host-passthrough;machine:pc-q35-9.2' + vm_machine_type_main invalid cn1 + vm_machine_type_main apply cn1 x86_64 + [ "$(vmother)" = 'cpumode:host-passthrough;machine:pc-q35-9.2' ] +} From 3265574c1e75e168b2b1458287ef46760fdb6b28 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:48:03 -0300 Subject: [PATCH 3/5] fix(xcat-test): the installation cases corrupt a versioned machine type in vmothersetting reg_linux_diskless_installation_flat adds a machine type to vmothersetting and removes it again with a substring sed. When the node already names a versioned machine type, the sed removes the first match from the user's own value: cpumode:host-passthrough;machine:pc-q35-9.2 comes back as cpumode:host-passthrough-q35-9.2;machine:pc. The same happens on riscv64 with machine:virt-9.2. vm_machine_type.sh now saves the exact vmothersetting before it adds machine:invalid, and restore writes that value back and deletes the saved copy. An empty value clears the attribute. apply keeps a machine type that the node already names and adds the architecture default only when there is none. restore fails when no value was saved, and apply fails for an architecture without a default. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- .../testcase/commoncmd/vm_machine_type.sh | 79 ++++++++++++++++--- xCAT-test/bats/vm_machine_type.bats | 17 ++++ 2 files changed, 83 insertions(+), 13 deletions(-) diff --git a/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh b/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh index 1f53678cc..fbc929bd0 100755 --- a/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh +++ b/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh @@ -2,33 +2,86 @@ # vm_machine_type.sh - set and clear the KVM machine type in vmothersetting of a node. # # Usage: -# vm_machine_type.sh invalid add machine:invalid, so the VM cannot start -# vm_machine_type.sh apply replace machine:invalid with a valid type -# vm_machine_type.sh restore remove the valid type again +# vm_machine_type.sh invalid save vmothersetting, then add machine:invalid +# vm_machine_type.sh apply write the saved value with a valid machine type +# vm_machine_type.sh restore [] write the saved value back and delete it +# +# The saved value is in $VM_MACHINE_TYPE_STATE_DIR (default /tmp), one file per node. + +vm_machine_type_state() +{ + printf '%s/xcattest-%s-vmothersetting\n' "${VM_MACHINE_TYPE_STATE_DIR:-/tmp}" "$1" +} vm_machine_type_for_arch() { - local arch="$1" str3 - if [[ "$arch" =~ "ppc64" ]]; then str3="machine:pseries-rhel7.6.0"; elif [[ "$arch" =~ "x86_64" ]]; then str3="machine:pc"; elif [[ "$arch" =~ "riscv64" ]]; then str3="machine:virt"; elif [[ "$arch" =~ "aarch64" ]]; then str3="machine:virt";fi - printf '%s\n' "$str3" + case "$1" in + ppc64*) echo 'machine:pseries-rhel7.6.0' ;; + x86_64) echo 'machine:pc' ;; + riscv64) echo 'machine:virt' ;; + aarch64) echo 'machine:virt' ;; + *) return 1 ;; + esac +} + +vm_machine_type_get() +{ + lsdef "$1" -i vmothersetting | sed -n 's/^[[:space:]]*vmothersetting=//p' +} + +vm_machine_type_saved() +{ + local state + state="$(vm_machine_type_state "$1")" + if [ ! -f "$state" ]; then + echo "vm_machine_type.sh: no saved vmothersetting for $1 in $state" >&2 + return 1 + fi + cat "$state" +} + +# join -- join two vmothersetting values with ";". +vm_machine_type_join() +{ + if [ -z "$1" ]; then + printf '%s\n' "$2" + else + printf '%s;%s\n' "$1" "$2" + fi } vm_machine_type_invalid() { - local node="$1" str1 str2 str3 str4 - str1=`lsdef $node | grep vmothersetting | cut -d '=' -f 2`;str2=";"; str3="machine:invalid"; if [ -z $str1 ]; then str4=$str3; else str4=$str1$str2$str3;fi; chdef $node vmothersetting=$str4 + local node="$1" original + original="$(vm_machine_type_get "$node")" + printf '%s' "$original" >"$(vm_machine_type_state "$node")" || return 1 + # kvm.pm uses the last machine: setting. + chdef "$node" "vmothersetting=$(vm_machine_type_join "$original" 'machine:invalid')" } vm_machine_type_apply() { - local node="$1" arch="$2" str1 str2 str3 str4 str5 - str1=`lsdef $node | grep vmothersetting | cut -d '=' -f 2`;str2="machine:invalid"; str3=$(vm_machine_type_for_arch "$arch"); if [ "$str1" == "$str2" ]; then str5=$str3; else str4=`echo $str1 | sed -e "s/$str2//"`;str5=$str4$str3;fi; chdef $node vmothersetting=$str5 + local node="$1" arch="$2" original machine + original="$(vm_machine_type_saved "$node")" || return 1 + # A machine type the node already names stays in place. + if [[ ";$original" == *";machine:"* ]]; then + chdef "$node" "vmothersetting=$original" + return + fi + if ! machine="$(vm_machine_type_for_arch "$arch")"; then + echo "vm_machine_type.sh: no machine type for arch '$arch'" >&2 + return 1 + fi + chdef "$node" "vmothersetting=$(vm_machine_type_join "$original" "$machine")" } vm_machine_type_restore() { - local node="$1" arch="$2" str1 str2 str3 str4 - str1=`lsdef $node | grep vmothersetting | cut -d '=' -f 2`;str2=";"; str3=$(vm_machine_type_for_arch "$arch"); if [ "$str1" == "$str3" ]; then chdef $node vmothersetting=; else str4=`echo $str1 | sed -e "s/$str2$str3//"`; chdef $node vmothersetting=$str4;fi + local node="$1" original + original="$(vm_machine_type_saved "$node")" || return 1 + # An empty value makes chdef delete the attribute. + chdef "$node" "vmothersetting=$original" || return 1 + rm -f "$(vm_machine_type_state "$node")" } vm_machine_type_main() @@ -40,7 +93,7 @@ vm_machine_type_main() apply) vm_machine_type_apply "$@" ;; restore) vm_machine_type_restore "$@" ;; *) - echo "usage: vm_machine_type.sh {invalid |apply |restore }" >&2 + echo "usage: vm_machine_type.sh {invalid |apply |restore []}" >&2 return 2 ;; esac diff --git a/xCAT-test/bats/vm_machine_type.bats b/xCAT-test/bats/vm_machine_type.bats index cbf184d48..4545f95cd 100644 --- a/xCAT-test/bats/vm_machine_type.bats +++ b/xCAT-test/bats/vm_machine_type.bats @@ -131,3 +131,20 @@ assert_round_trip() vm_machine_type_main apply cn1 x86_64 [ "$(vmother)" = 'cpumode:host-passthrough;machine:pc-q35-9.2' ] } + +@test "restore without a saved value fails and leaves vmothersetting alone" { + set_vmother 'cpumode:host-passthrough;machine:pc' + run vm_machine_type_main restore cn1 x86_64 + [ "$status" -ne 0 ] + [[ "$output" == *"no saved vmothersetting for cn1"* ]] + [ "$(vmother)" = 'cpumode:host-passthrough;machine:pc' ] +} + +@test "apply fails for an architecture without a machine type" { + set_vmother '' + vm_machine_type_main invalid cn1 + run vm_machine_type_main apply cn1 s390x + [ "$status" -ne 0 ] + [[ "$output" == *"no machine type for arch 's390x'"* ]] + [ "$(vmother)" = 'machine:invalid' ] +} From c9e00745d1f38ce667c4718bbff51f34a7800e6c Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Mon, 12 Oct 2026 10:58:44 -0300 Subject: [PATCH 4/5] test(xcat-test): vm_machine_type.sh overwrites its snapshot and saves a failed lsdef vm_machine_type.sh invalid saves an empty vmothersetting when lsdef fails, so the next restore deletes the configuration of the node. A second invalid replaces the saved value with one that holds machine:invalid. The script writes the state file at a predictable path and follows a symbolic link there. Add tests for a failed lsdef, a repeated invalid, a snapshot left by an earlier run, a symbolic link as the state file or the state directory, and a state directory that other users can write. Add tests for an empty value that lsdef read and for a restore that fails. The first group fails on the current script. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/bats/vm_machine_type.bats | 117 +++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/xCAT-test/bats/vm_machine_type.bats b/xCAT-test/bats/vm_machine_type.bats index 4545f95cd..22480c09e 100644 --- a/xCAT-test/bats/vm_machine_type.bats +++ b/xCAT-test/bats/vm_machine_type.bats @@ -9,12 +9,20 @@ setup() source "$(require_repo_file 'xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh')" VMOTHER="${BATS_TEST_TMPDIR}/vmothersetting" export VM_MACHINE_TYPE_STATE_DIR="${BATS_TEST_TMPDIR}/state" - mkdir -p "$VM_MACHINE_TYPE_STATE_DIR" + mkdir -m 0700 "$VM_MACHINE_TYPE_STATE_DIR" + STATE="${VM_MACHINE_TYPE_STATE_DIR}/xcattest-cn1-vmothersetting" + LSDEF_FAIL=0 + CHDEF_FAIL=0 + CHDEF_CALLS=0 } # lsdef prints no line for an attribute without a value. lsdef() { + if [ "$LSDEF_FAIL" -ne 0 ]; then + echo "Error: [mn]: Could not find an object named '$1' of type 'node'." >&2 + return 1 + fi printf 'Object name: %s\n' "$1" if [ -s "$VMOTHER" ]; then printf ' vmothersetting=%s\n' "$(cat "$VMOTHER")" @@ -24,6 +32,7 @@ lsdef() chdef() { [ "$1" = cn1 ] || return 1 + [ "$CHDEF_FAIL" -eq 0 ] || return 1 case "$2" in vmothersetting=*) printf '%s' "${2#vmothersetting=}" >"$VMOTHER" ;; *) return 1 ;; @@ -148,3 +157,109 @@ assert_round_trip() [[ "$output" == *"no machine type for arch 's390x'"* ]] [ "$(vmother)" = 'machine:invalid' ] } + +@test "invalid saves an empty vmothersetting that lsdef read" { + set_vmother '' + vm_machine_type_main invalid cn1 + [ -f "$STATE" ] + [ ! -s "$STATE" ] + [ "$(vmother)" = 'machine:invalid' ] +} + +@test "invalid creates a private state directory" { + rmdir "$VM_MACHINE_TYPE_STATE_DIR" + vm_machine_type_main invalid cn1 + [ -d "$VM_MACHINE_TYPE_STATE_DIR" ] + [ -z "$(find "$VM_MACHINE_TYPE_STATE_DIR" -maxdepth 0 -perm /077)" ] +} + +@test "invalid fails without a snapshot or chdef when lsdef fails" { + set_vmother 'cpumode:host-passthrough' + LSDEF_FAIL=1 + run vm_machine_type_main invalid cn1 + [ "$status" -ne 0 ] + [[ "$output" == *"cannot read vmothersetting of cn1"* ]] + [ ! -e "$STATE" ] + [ "$CHDEF_CALLS" -eq 0 ] + [ "$(vmother)" = 'cpumode:host-passthrough' ] +} + +@test "a second invalid is refused and keeps the first snapshot" { + set_vmother 'cpumode:host-passthrough' + vm_machine_type_main invalid cn1 + run vm_machine_type_main invalid cn1 + [ "$status" -ne 0 ] + [[ "$output" == *"saved vmothersetting for cn1 already exists"* ]] + [ "$(cat "$STATE")" = 'cpumode:host-passthrough' ] + [ "$(vmother)" = 'cpumode:host-passthrough;machine:invalid' ] + [ "$CHDEF_CALLS" -eq 1 ] + + vm_machine_type_main apply cn1 x86_64 + vm_machine_type_main restore cn1 x86_64 + [ "$(vmother)" = 'cpumode:host-passthrough' ] +} + +@test "invalid refuses a snapshot left by an earlier run" { + set_vmother 'cpumode:host-passthrough' + printf '%s' 'machine:stale' >"$STATE" + run vm_machine_type_main invalid cn1 + [ "$status" -ne 0 ] + [[ "$output" == *"saved vmothersetting for cn1 already exists"* ]] + [ "$(cat "$STATE")" = 'machine:stale' ] + [ "$(vmother)" = 'cpumode:host-passthrough' ] + [ "$CHDEF_CALLS" -eq 0 ] +} + +@test "invalid refuses a symbolic link as the state file" { + set_vmother 'cpumode:host-passthrough' + ln -s "${BATS_TEST_TMPDIR}/target" "$STATE" + run vm_machine_type_main invalid cn1 + [ "$status" -ne 0 ] + [ ! -e "${BATS_TEST_TMPDIR}/target" ] + [ "$(vmother)" = 'cpumode:host-passthrough' ] + [ "$CHDEF_CALLS" -eq 0 ] +} + +@test "restore refuses a symbolic link as the state file" { + set_vmother 'cpumode:host-passthrough;machine:pc' + printf '%s' 'machine:other' >"${BATS_TEST_TMPDIR}/target" + ln -s "${BATS_TEST_TMPDIR}/target" "$STATE" + run vm_machine_type_main restore cn1 x86_64 + [ "$status" -ne 0 ] + [[ "$output" == *"is not a regular file"* ]] + [ "$(vmother)" = 'cpumode:host-passthrough;machine:pc' ] + [ "$CHDEF_CALLS" -eq 0 ] +} + +@test "invalid refuses a symbolic link as the state directory" { + set_vmother 'cpumode:host-passthrough' + rmdir "$VM_MACHINE_TYPE_STATE_DIR" + mkdir -m 0700 "${BATS_TEST_TMPDIR}/real" + ln -s "${BATS_TEST_TMPDIR}/real" "$VM_MACHINE_TYPE_STATE_DIR" + run vm_machine_type_main invalid cn1 + [ "$status" -ne 0 ] + [[ "$output" == *"is not a private directory"* ]] + [ -z "$(ls -A "${BATS_TEST_TMPDIR}/real")" ] + [ "$(vmother)" = 'cpumode:host-passthrough' ] + [ "$CHDEF_CALLS" -eq 0 ] +} + +@test "invalid refuses a state directory that other users can write" { + set_vmother 'cpumode:host-passthrough' + chmod 0777 "$VM_MACHINE_TYPE_STATE_DIR" + run vm_machine_type_main invalid cn1 + [ "$status" -ne 0 ] + [[ "$output" == *"is not a private directory"* ]] + [ -z "$(ls -A "$VM_MACHINE_TYPE_STATE_DIR")" ] + [ "$CHDEF_CALLS" -eq 0 ] +} + +@test "restore keeps the snapshot when chdef fails" { + set_vmother 'cpumode:host-passthrough' + vm_machine_type_main invalid cn1 + CHDEF_FAIL=1 + run vm_machine_type_main restore cn1 x86_64 + [ "$status" -ne 0 ] + [ "$(cat "$STATE")" = 'cpumode:host-passthrough' ] + [ "$(vmother)" = 'cpumode:host-passthrough;machine:invalid' ] +} From 0e1c3d0808f06a5955a2df63a69cdc7f868a0c08 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Mon, 12 Oct 2026 10:59:20 -0300 Subject: [PATCH 5/5] fix(xcat-test): vm_machine_type.sh overwrites its snapshot and saves a failed lsdef vm_machine_type.sh invalid ignored the exit status of lsdef. When lsdef failed, it saved an empty vmothersetting, and the next restore deleted the configuration of the node. A second invalid replaced the saved value with one that holds machine:invalid. The state file was a predictable /tmp path that the script truncated and followed through a symbolic link. invalid now stops before it saves or changes anything when lsdef fails, or when a saved value already exists. An empty value that lsdef read is still saved. The script creates the state file with O_EXCL in a directory that the current user owns with mode 0700. The default directory is /tmp/xcattest-vmothersetting.. The script refuses a symbolic link, a directory that other users can open, and a file it does not own. When chdef fails in invalid, the script deletes the new snapshot. A failed restore still keeps it. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- .../testcase/commoncmd/vm_machine_type.sh | 57 ++++++++++++++++--- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh b/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh index fbc929bd0..412a16c77 100755 --- a/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh +++ b/xCAT-test/autotest/testcase/commoncmd/vm_machine_type.sh @@ -6,11 +6,31 @@ # vm_machine_type.sh apply write the saved value with a valid machine type # vm_machine_type.sh restore [] write the saved value back and delete it # -# The saved value is in $VM_MACHINE_TYPE_STATE_DIR (default /tmp), one file per node. +# The saved value is in $VM_MACHINE_TYPE_STATE_DIR, one file per node. The +# default directory is /tmp/xcattest-vmothersetting.. + +vm_machine_type_dir() +{ + printf '%s\n' "${VM_MACHINE_TYPE_STATE_DIR:-/tmp/xcattest-vmothersetting.$(id -u)}" +} vm_machine_type_state() { - printf '%s/xcattest-%s-vmothersetting\n' "${VM_MACHINE_TYPE_STATE_DIR:-/tmp}" "$1" + printf '%s/xcattest-%s-vmothersetting\n' "$(vm_machine_type_dir)" "$1" +} + +# private_dir -- create the state directory, or accept it only when it is a +# directory that the current user owns and nobody else can open. +vm_machine_type_private_dir() +{ + local dir + dir="$(vm_machine_type_dir)" + [ -e "$dir" ] || [ -L "$dir" ] || mkdir -m 0700 "$dir" 2>/dev/null + if [ -L "$dir" ] || [ ! -d "$dir" ] || [ ! -O "$dir" ] || + [ -n "$(find "$dir" -maxdepth 0 -perm /077)" ]; then + echo "vm_machine_type.sh: $dir is not a private directory of $(id -un)" >&2 + return 1 + fi } vm_machine_type_for_arch() @@ -26,17 +46,27 @@ vm_machine_type_for_arch() vm_machine_type_get() { - lsdef "$1" -i vmothersetting | sed -n 's/^[[:space:]]*vmothersetting=//p' + local out + if ! out="$(lsdef "$1" -i vmothersetting)"; then + echo "vm_machine_type.sh: cannot read vmothersetting of $1" >&2 + return 1 + fi + printf '%s\n' "$out" | sed -n 's/^[[:space:]]*vmothersetting=//p' } vm_machine_type_saved() { local state + vm_machine_type_private_dir || return 1 state="$(vm_machine_type_state "$1")" - if [ ! -f "$state" ]; then + if [ ! -e "$state" ] && [ ! -L "$state" ]; then echo "vm_machine_type.sh: no saved vmothersetting for $1 in $state" >&2 return 1 fi + if [ -L "$state" ] || [ ! -f "$state" ] || [ ! -O "$state" ]; then + echo "vm_machine_type.sh: $state is not a regular file of $(id -un)" >&2 + return 1 + fi cat "$state" } @@ -52,11 +82,22 @@ vm_machine_type_join() vm_machine_type_invalid() { - local node="$1" original - original="$(vm_machine_type_get "$node")" - printf '%s' "$original" >"$(vm_machine_type_state "$node")" || return 1 + local node="$1" state original + vm_machine_type_private_dir || return 1 + state="$(vm_machine_type_state "$node")" + # A second invalid would save machine:invalid as the original value. + if [ -e "$state" ] || [ -L "$state" ]; then + echo "vm_machine_type.sh: a saved vmothersetting for $node already exists in $state" >&2 + return 1 + fi + original="$(vm_machine_type_get "$node")" || return 1 + # noclobber makes bash create the file with O_EXCL. + (set -o noclobber && printf '%s' "$original" >"$state") || return 1 # kvm.pm uses the last machine: setting. - chdef "$node" "vmothersetting=$(vm_machine_type_join "$original" 'machine:invalid')" + if ! chdef "$node" "vmothersetting=$(vm_machine_type_join "$original" 'machine:invalid')"; then + rm -f "$state" + return 1 + fi } vm_machine_type_apply()