Skip to content

fix: flush the image before importing a VM disk - #223

Open
michelroegl-brunner wants to merge 1 commit into
mainfrom
fix/vm-import-sync-before-zfs
Open

michelroegl-brunner wants to merge 1 commit into
mainfrom
fix/vm-import-sync-before-zfs

Conversation

@michelroegl-brunner

Copy link
Copy Markdown
Member

✍️ Description

vm_import_disk now flushes the image to disk (sync -f "$file", falling back to sync, best effort) before it runs qm disk import.

Images are usually freshly extracted or converted (xz, gzip, qemu-img, virt-customize), so gigabytes of dirty pages are still waiting to be written. On a default ZFS install the image (rpool/var-lib-vz) and the target zvol (rpool/data) are in the same pool, so the zvol create inside the import runs into Proxmox's zfs command timeout (zfs create ... failed: got timeout) while ZFS writes those pages back. The reporter of community-scripts/ProxmoxVE#18580 narrowed it down: a sleep or a sync -f after extraction fixes it, while a sleep before extraction does not.

community-scripts/ProxmoxVE#18581 patches only vm/haos-vm.sh. Every VM script goes through vm_import_disk (haos, pimox-haos, openwrt, opnsense, docker, ubuntu, unifi-os, vm-app.func), so fixing it here covers all of them at once.

  • pve/vm-core.func: flush before the import. A failed flush never blocks the import.
  • tests/vm-helpers.sh: the flush runs before qm disk import, and the import still runs when the flush fails.
  • docs/vm.md: helper table updated.

Not included: a retry on got timeout like OPNsense's pvesm alloc loop. By the time that timeout fires the zvol has already been created, so a retry would need cleanup first. That's only worth adding if the flush turns out not to be enough.

AI: Claude Opus 5.5 (low reasoning effort).

🔗 Related Issue

Fixes community-scripts/ProxmoxVE#18580
Supersedes community-scripts/ProxmoxVE#18581

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards.
  • Tested thoroughly – Changes work as expected (Proxmox VE and/or Incus, as applicable).
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.
  • API.txt regenerated – If a function signature or behavior changed, API.txt was updated to match.

Testing: tests/vm-helpers.sh passes for all vm_import_disk cases, including the two new ones. It has not been tried on a real ZFS host yet. Incus is unaffected because vm_import_disk is qm-only there. No API.txt covers pve/vm-core.func, so there is nothing to regenerate.


🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • ✨ New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing behavior in a way that may require updates in every downstream script.
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, docs/, CONTRIBUTING.md, or other docs.

🤖 AI Assistance (X in brackets)

If you used an AI tool (GitHub Copilot, Claude, ChatGPT, etc.) to write or generate any code in this PR, you must confirm compliance below.
Select exactly one option.

  • No AI used – Code was written without AI assistance.
  • AI was used – The output has been reviewed and corrected, and I take responsibility for it matching this repo's conventions.

Freshly extracted or converted images leave gigabytes of dirty pages. On a
default ZFS install the image (rpool/var-lib-vz) and the target zvol
(rpool/data) share a pool, so the zvol create inside qm disk import hits
Proxmox's zfs command timeout ("got timeout") while ZFS writes them back.

vm_import_disk now runs a best-effort sync -f on the image first, which
covers every VM script at once instead of patching them one by one.

Refs community-scripts/ProxmoxVE#18580, community-scripts/ProxmoxVE#18581
@github-actions

Copy link
Copy Markdown
Contributor

Try this branch

The engine and the scripts resolve independently, so a production script can
be run against the engine from this PR by setting one variable:

COMMUNITY_SCRIPTS_CORE_URL=https://raw.githubusercontent.com/community-scripts/core/fix/vm-import-sync-before-zfs \
bash -c "$(curl -fsSL https://raw.githubusercontent.com/community-scripts/ProxmoxVE/main/ct/debian.sh)"

Swap ct/debian.sh for whatever exercises the change.

This PR touches pve/, so it needs a Proxmox VE host.

Run a script from a fork as well
curl -fsSL https://raw.githubusercontent.com/community-scripts/core/fix/vm-import-sync-before-zfs/tools/run.sh |
  bash -s -- https://raw.githubusercontent.com/YOU/DevScripts/your-branch ct/debian.sh \
             https://raw.githubusercontent.com/community-scripts/core/fix/vm-import-sync-before-zfs

Note that run.sh is reached through a pipe, so the script it starts inherits
an exhausted stdin. Whiptail is fine — it opens /dev/tty — but a plain read
would see EOF. The single-variable form above does not have that problem.

Useful flags while testing

dev_mode=net logs every engine fetch with status and duration, which is the
quickest way to confirm the branch is really being used. dev_mode=keep stops a
failed build from deleting the container along with the evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[HAOS VM] Pending writes after image extraction cause ZFS disk import timeout

1 participant