Clean up left-over fleecing images before the guest is migrated to a
different node and they'd really become orphaned.
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-16-f.ebner@proxmox.com
By recording the allocated fleecing images in the VM config, they
are not immediately orphaned, should a hard error occur during
backup that prevents cleanup.
They are attempted to be cleaned up during the next backup run.
In the cleanup helper, check if fleecing images are still attached in
QEMU and detach them. This allows recovering from more failure
scenarios. However, to avoid a deadlock, a left-over backup job needs
to be canceled first. While canceling a left-over backup already
happens when cleanup is done for a subsquent backup, it is required
for other cases that like cleanup before migration (to be added in a
following commit).
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-15-f.ebner@proxmox.com
Currently, a duplicate section will quietly override the previous
instance of the section with the same identifier. Keep the current
behavior of preferring later entries, but issue a warning or die when
parsing strictly.
The entry for 'pending' in the result needs to start out as undefined
for the check to also work in presence of empty sections. Avoid
changing the returned value itself, by making sure to initialize the
entry before returning.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-12-f.ebner@proxmox.com
Collect special sections below a common 'special-sections' key in
preparation to introduce a new special section.
The special 'cloudinit' section was added in the top-level of the
configuration structure, but it's cleaner to group special sections
more similar to snapshots.
The 'cloudinit' key was already initialized, so having the new
'special-sections' key be always initialized should not cause issues
after checking and adapting all usages of 'cloudinit' which this patch
attempts to do.
Add compat handling for remote migration which might receive the
configuration hash from a node that does not yet have the changes.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-10-f.ebner@proxmox.com
Also log an informational message just like for pending and snapshot
sections.
Add a comment about it to parse_vm_config() in the hope that the
behavior is noted when introducing a future special section.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-9-f.ebner@proxmox.com
Otherwise, a snapshot with a name that includes "pending" will be
misinterpreted as the pending section.
Only affects the warning messages, but still confusing.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-8-f.ebner@proxmox.com
Currently, keys in an unknown section will be interpreted as still
belonging to the last section and might erroneously overwrite values
in that way. Explicitly ignore unknown sections to avoid this while
warning the user.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-7-f.ebner@proxmox.com
While unknown sections do lead to an error in strict mode, in
non-strict mode the line is just skipped, meaning that key-value
pairs from the unknown section will override the key-value pairs from
the previous section.
Fixed by the next commit.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-6-f.ebner@proxmox.com
There are checks for custom parsing behavior inside certain sections
relying only on the section name. While the name 'pending' cannot be
used by snapshots, the name 'cloudinit' can. Introduce an associated
section type to make the checks precise.
The test was not added in a separate commit, because it would fail
when writing the config before the fix, and failure in writing is
never expected by the test script. So there is no easy way to
highlight just the difference in behavior together with the fix and
the git history should stay bisectable.
Compare with the verify-snapshot.conf testcase without the actual fix
applied to see the difference.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-5-f.ebner@proxmox.com
Tests for parsing and writing VM configuration files. The parsing part
is already covered by the config2command test too, but that only
focuses on the main section, not other section types and does not also
test parsing in strict mode.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-4-f.ebner@proxmox.com
The duplication is there, because two independet fixes for a test
failure where applied, namely commits:
75c430ce ("test: unbreak restore_config_test")
cc1cdadb ("test: fix restore config test as unprivileged user")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.proxmox.com/20250127112923.31703-3-f.ebner@proxmox.com
Commit c8ed1ac2 ("api: create disks: live import: use format from
storage layer") broke live import from an OVA containing a disk,
because a combined format like 'ova+vmdk' was used for the live-import
disk mapping, leading to failure:
> invalid format 'ova+vmdk' for 'scsi0' mapping
There was also an informational message about the confusion earlier:
> file_size_info: '/mnt/pve/dir/images/135/vm-135-disk-0.vmdk': \
> falling back to 'raw' from unknown format 'ova+vmdk
Fixes: c8ed1ac2 ("api: create disks: live import: use format from storage layer")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
While no problem is known with having an empty machine string in the
configuration and it was already possible setting an empty machine
manually via 'qm set', the behavior changed because of commit 919e69d0
("machine: add check_and_pin_machine_string() helper") and there is
potential for problematic edge cases. Restore the previous behavior.
Pinning is also required when there is no given machine type, so the
call to check_and_pin_machine_string() should stay unconditional.
For update, pinning was recently added by commit 7a9570f3 ("api:
update vm config: pin machine version when switching to windows os
type"), so bring that in-line with the behavior for create too.
Another idea would've been to just return the default machine in
check_and_pin_machine_string(), but that would also be a change in
behavior. In particular, the default depends on the arch, so an empty
machine type will pick the default machine for the currently
configured arch even when the arch is later changed.
Reported-by: Daniel Herzig <d.herzig@proxmox.com>
Fixes: 919e69d0 ("machine: add check_and_pin_machine_string() helper")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
While the path for changing a CD-ROM medium was already consistent
with the path for an already inserted CD-ROM at VM start, i.e. the one
from print_drive_commandline_full(), this makes that fact more
explicit. While at it, make sure the format is also consistent with
how it is determined in print_drive_commandline_full(). Do the same
for cloud-init drives, which often are in non-raw format.
Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Friedrich Weber <f.weber@proxmox.com>
To allow re-using it for CD-ROM hotplug.
No functional change intended.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Friedrich Weber <f.weber@proxmox.com>
This was the only user of List::Util, so move that too.
No functional change intended.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Friedrich Weber <f.weber@proxmox.com>
During virtual machine creation, the machine version is pinned when
the guest OS is Windows. The same should be done when the guest OS
type is newly set to Windows for consistency.
NOTE RFC
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Extract the logic for guest OS-type dependent machine version pinning
into a dedicated helper, so it can be re-used.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Starting from QEMU creation version 9.1, pin to the creation version
instead. Support for machine version 5.1 is expected to drop with QEMU
11.1 and it would still be good to handle Windows VMs that do not have
explicit machine version for whatever reason. For example, explicitly
setting the machine without a version on the CLI/API after creation is
one way to end up with such a machine.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Like this, it can be used by modules that cannot depend on
QemuServer.pm without creating a cyclic dependency.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
There are already other places where 'aarch64' and 'x86_64' are
checked to be the only valid architectures, for example, the
get_command_for_arch() helper, so the new error scenario for an
unknown arch should not cause any regressions.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Add an export, since the function is rather commonly used (in
particular inlined in function calls, where prefixing with the module
name would hurt readability) and there won't be much potential for
confusion name-wise.
This was the only user of stat(), so remove the File::stat include.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Cannot use the is_native_arch() helper inside the function anymore,
to avoid a cyclic dependency between the 'CPUConfig' and 'Helpers'
modules, inline it.
While at it, improve the variable name for the mapping.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
The commit 37a1f42a introduced a conversion step for newly allocated
volume disks to base volumes for VM templates. This wrongly excluded
efidisk and tmpstate volumes from this conversion, even though those
should be converted to base volumes too.
Therefore, include efidisks and tpmstate volumes to be converted to base
volumes when newly allocating them.
Suggested-by: Fiona Ebner <f.ebner@proxmox.com>
Fixes: 37a1f42a ("fix #5301: convert added volume disks to base images for templates")
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Fixes a small typo and uses the same wording as used in pve-container's
description of the `mem` property.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Checking only vm configuration for choose the shutdown method causes it
to always fail, after reaching the timeout, if the qemu agent option in
the vm configuration is enabled but the agent is not installed and
active in the guest.
As I seen in the windows vm the agent also crashes in some cases, so
shutdown don't fail only if qemu guest agent is not installed or not
started.
Added check that agent is active when choosing agent shutdown method to
avoid certain shutdown failure in those cases.
Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
[FE: do not set flag to suppress warning when agent is not running]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
when creating new ones, we already force raw as format, but adding existing
volumes as tpmstate0 had no such checks.
Suggested-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
