From a929d4ae21269e83f5cdc00e6f6bb2ba4dececaa Mon Sep 17 00:00:00 2001 From: MickLesk <47820557+MickLesk@users.noreply.github.com> Date: Mon, 31 Aug 2026 08:35:39 +0200 Subject: [PATCH 1/2] Lift the settings summary and the mode fork into vm-core Every migrated VM script still carried two blocks that were not its own. The default-or-advanced fork was byte for byte identical in all of them, and the fifteen-line settings summary agreed on thirteen lines -- the two that differed, the CPU model label and the app name, are both derivable from variables the script has already set. vm_echo_default_settings computes the labels rather than taking them, so a script can no longer print "Host" while CPU_TYPE says otherwise. That was not hypothetical: the summary text and the variable were separate edits. vm_start_script resolves default_settings and advanced_settings when it runs, so each script still supplies its own. Nothing existing is changed -- the ten scripts that define start_script themselves keep overriding it. Caught while testing: vm_machine_type_label takes the type as an argument rather than reading MACHINE_TYPE, so calling it bare reported i440fx for every machine, q35 included. Verified both ways round now, along with the cache and CPU labels. --- pve/vm-core.func | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/pve/vm-core.func b/pve/vm-core.func index 511f2f2..e209c1b 100644 --- a/pve/vm-core.func +++ b/pve/vm-core.func @@ -733,6 +733,68 @@ vm_choose_settings_mode() { whiptail --backtitle "Proxmox VE Helper Scripts" --title "SETTINGS" --yesno "$message" --no-button Advanced "$height" "$width" } +# ------------------------------------------------------------------------------ +# vm_echo_default_settings() +# +# Prints the settings summary from the variables default_settings has just set. +# Every VM script wrote this block itself and they agreed on thirteen of the +# fifteen lines; the two that differed were the CPU model label and the app +# name, both derivable. The labels are computed rather than passed, so a script +# cannot print "Host" while CPU_TYPE says otherwise. +# ------------------------------------------------------------------------------ +vm_echo_default_settings() { + local cpu_label="KVM64" + [[ "${CPU_TYPE:-}" == *host* ]] && cpu_label="Host" + + local cache_label="None" + [[ -n "${DISK_CACHE:-}" ]] && cache_label="Write Through" + + # vm_machine_type_label takes the type as an argument, not from the + # environment, so MACHINE_TYPE has to be handed over explicitly -- calling it + # bare silently reported i440fx for every machine. + local machine_label + machine_label="$(vm_machine_type_label "${MACHINE_TYPE:-i440fx}")" + + echo -e "${CONTAINERID}${BOLD}${DGN}Virtual Machine ID: ${BGN}${VMID}${CL}" + echo -e "${CONTAINERTYPE}${BOLD}${DGN}Machine Type: ${BGN}${machine_label}${CL}" + echo -e "${DISKSIZE}${BOLD}${DGN}Disk Size: ${BGN}${DISK_SIZE}${CL}" + echo -e "${DISKSIZE}${BOLD}${DGN}Disk Cache: ${BGN}${cache_label}${CL}" + echo -e "${HOSTNAME}${BOLD}${DGN}Hostname: ${BGN}${HN}${CL}" + echo -e "${OS}${BOLD}${DGN}CPU Model: ${BGN}${cpu_label}${CL}" + echo -e "${CPUCORE}${BOLD}${DGN}CPU Cores: ${BGN}${CORE_COUNT}${CL}" + echo -e "${RAMSIZE}${BOLD}${DGN}RAM Size: ${BGN}${RAM_SIZE} MiB${CL}" + echo -e "${BRIDGE}${BOLD}${DGN}Bridge: ${BGN}${BRG}${CL}" + echo -e "${MACADDRESS}${BOLD}${DGN}MAC Address: ${BGN}${MAC}${CL}" + echo -e "${VLANTAG}${BOLD}${DGN}VLAN: ${BGN}${VLAN:-Default}${CL}" + echo -e "${DEFAULT}${BOLD}${DGN}Interface MTU Size: ${BGN}${MTU:-Default}${CL}" + echo -e "${GATEWAY}${BOLD}${DGN}Start VM when completed: ${BGN}${START_VM}${CL}" + echo -e "${CREATING}${BOLD}${DGN}Creating a ${APP:-${NSAPP}} VM using the above default settings${CL}" +} + +# ------------------------------------------------------------------------------ +# vm_start_script() +# +# The default-or-advanced fork, which was byte for byte the same in every VM +# script that had been migrated. default_settings and advanced_settings are +# resolved when this runs rather than when it is defined, so each script still +# supplies its own. +# +# Arguments: the dialog message, and optionally its height and width -- a script +# that lists what its defaults are needs a taller box than one asking a bare +# question. +# ------------------------------------------------------------------------------ +vm_start_script() { + if vm_choose_settings_mode "$@"; then + header_info + echo -e "${DEFAULT}${BOLD}${BL}Using Default Settings${CL}" + default_settings + else + header_info + echo -e "${ADVANCED}${BOLD}${RD}Using Advanced Settings${CL}" + advanced_settings + fi +} + vm_confirm_advanced_settings() { local message="$1" local height="${2:-10}" From 36f6c3a2af4495ca02a187cd84ffc537398e8813 Mon Sep 17 00:00:00 2001 From: MickLesk <47820557+MickLesk@users.noreply.github.com> Date: Mon, 31 Aug 2026 08:49:26 +0200 Subject: [PATCH 2/2] Stop load_functions running the checks it was meant to provide Three faults, all visible in one run of the CachyOS VM: 101 curl: (22) The requested URL returned error: 404 vm-core.func: line 124: _cs_clear: command not found load_functions ended with six bare names -- get_valid_nextid, cleanup_vmid, cleanup, check_root, pve_check, arch_check -- left behind when their bodies moved out of it. Loading the engine therefore ran them: the stray 101 is a VM ID printed by get_valid_nextid, and cleanup tore down a temp dir before there was one. Every script calls those itself afterwards, in its own order. The same residue is in debian-vm.sh and k3s-vm.sh in ProxmoxVED. _cs_clear had five callers here and no definition. core/core.func owns it, but a VM script never loads that file -- it carries its own colours and icons -- so header_info died on the first line it drew. get_header let curl print its 404. A script whose banner the generator has not produced yet is the normal case, and header_info already handles it by drawing nothing; it just did so loudly. The empty file curl leaves behind is removed too, since it would satisfy the cache check forever after. --- pve/vm-core.func | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/pve/vm-core.func b/pve/vm-core.func index e209c1b..e1da599 100644 --- a/pve/vm-core.func +++ b/pve/vm-core.func @@ -66,6 +66,12 @@ load_api_functions() { fi } +# Only the groups that set variables. The six names that used to follow +# shell_check were left behind when their definitions moved out of this +# function, so loading the engine ran them: get_valid_nextid printed a bare VM +# ID, cleanup tore down the temp dir before there was one, and check_root and +# friends ran before the script had said what it wanted. Every script calls +# those itself, in its own order. load_functions() { [[ -n "${__FUNCTIONS_LOADED:-}" ]] && return __FUNCTIONS_LOADED=1 @@ -76,12 +82,17 @@ load_functions() { default_vars set_std_mode shell_check - get_valid_nextid - cleanup_vmid - cleanup - check_root - pve_check - arch_check +} + +# core/core.func owns this, but a VM script never loads that file -- it carries +# its own colours and icons. `clear` is part of ncurses and missing from minimal +# images, where a bare call returns 127 and errexit ends the run. +_cs_clear() { + if command -v clear >/dev/null 2>&1; then + clear + else + printf '\033[H\033[2J\033[3J' + fi } load_cloud_init_functions() { @@ -106,8 +117,11 @@ get_header() { mkdir -p "$(dirname "$local_header_path")" + # A script whose banner the generator has not produced yet 404s here, which is + # expected and not worth printing at the user. header_info draws nothing. if [ ! -s "$local_header_path" ]; then - if ! curl -fsSL "$header_url" -o "$local_header_path"; then + if ! curl -fsSL "$header_url" -o "$local_header_path" 2>/dev/null; then + rm -f "$local_header_path" return 1 fi fi