To ensure the new Confidential VM (CVM) images that got the Secure
Management Mode (SMM) disabled are available.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Make it more noticeable if a efidisk will be ignored, as this means
one changed a existing VM that already had a efidisk entry to SEV-SNP,
as adding an efidisk for such VMs with SEV-SNP enabled is blocked.
While at it fix indentation and drop the duplicated $arch variable,
that's already declared as required here at the top of the method.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This patch is for enabling AMD SEV-SNP support.
Where applicable, it extends support for existing SEV(-ES) variables
to SEV-SNP. This means that it retains no-debug and kernel-hashes
options, but the no-key-sharing option is removed.
The default policy value is identical to QEMU’s, and the therefore
required option has been added to configure SMT support.
The code was tested by running a VM without SEV, with SEV, SEV-ES,
SEV-SNP. Each configuration was tested with and without an EFI disk
attached. For SEV-enabled configurations it was also verified that the
kernel actually used the respective feature.
Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
Tested-by: Markus Frank <m.frank@proxmox.com>
Reviewed-by: Markus Frank <m.frank@proxmox.com>
Convert policy calculation to use shift operators and OR operation
instead of binary numbers and addition.
Signed-off-by: Philipp Giersfeld <philipp.giersfeld@canarybit.eu>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Markus Frank <m.frank@proxmox.com>
Reviewed-by: Markus Frank <m.frank@proxmox.com>
The format was dropped in QEMU binary version 2.2 with commit
550830f935 ("block: delete cow block driver").
Very old backups might still include this format as a hint (the data
in the backup is present in raw/chunk format in any case), but that is
not an issue. Restore already checks that the target storage supports
a given format and defaults to the default format of the storage if
the hint does not apply.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Acked-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Showing a final transfer log line helps with identifying what was
actually transferred. E.g., it could happen that the VFIO state was
only transferred in the last iteration. In such a case we would not
see that information at all.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Show the transferred VFIO state (when there is one), but since there
is no total here, so we can't show that, just what was transferred up
until now.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
So that we can show a proper warning in the migrate dialog and check it
in the bulk migrate precondition check.
The returned 'unavailable_storages' and 'unavailable-resources' should
be the same as before, but we now always return (not_)allowed_nodes
too.
To make the code a bit easier to read, reorganize how we construct the
(not_)allowed_nodes properties.
Also add a note that we want to redesign the return values here for a
easier to consume interface.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Set the 'enable-migration' QEMU command-line flag to on for
live-migration marked mapped devices, as the default is 'auto', but
for those which are marked as capable for live-migration, we want to
explicitly enable that, so that we get an early error on start if the
driver does not support live-migration.
With that we can drop such devices from being a migration blocker.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[TL: squash and re-order similar changes together]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Switch the underlying check_local_resource method to return a hash and
adapt call sites to that change. Return that new hash in the API as
new return property for the frontend to have more info to check.
This is in preparation for enabling live-migration for VMs with
mappings that declare being capable of them.
Originally-by: Dominik Csapak <d.csapak@proxmox.com>
[TL: split out from unrelated changes and fix return schema]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Do not mark mapped devices as local resources for the offline
migration case, there it's only relevant that they have a mapping
configured on the target node, which is a different check.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
By passing the mapping config to assert_valid, not only the specific
mapping.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This will fail with the new checks for mdev when we don't have the
correct config, as a device that has mediated devices, should have
'mdev' set in the mapping config
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
we cleaned up extracted images, but logged it even for non extracted
ones. Only log this when we actually cleaned anything up.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
we currently only call deactivate_volumes, but we actually want to call
the whole vm_stop_cleanup, since that is not invoked by the vm_stop
above (we cannot parse the config anymore) and might do other cleanups
we also want to do (like mdev cleanup).
For this to work properly we have to clone the original config at the
beginning, since we might modify the volids.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
and keep it the same for all current callers as before by setting the
additional 'noerr' parameter to '1'.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
in our schema for 'vga' the type is optional, so a config like
vga: memory=64
is valid. When checking the type we have to check if the type is defined
before accessing it.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[FE: add 'termproxy/vncproxy' prefix to commit title]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
When the VM is running, after deleting the tpmstate from the
configuration, the TPM drive will still be active, because it is used
by swtpm and QEMU will still have the TPM socket.
Also, a subsequent live migration will fail, because the
commandline for the target instance does not match the one for the
source:
> kvm: Unknown ramblock "tpm-ppi", cannot accept migration
For efidisk, hotunplug would already fail with
> can't unplug device 'efidisk0'
but it's better to catch this early and queue the change as pending
instead.
For modifying the 'tpmstate0' and 'efidisk0' options, there are
already checks to disallow live changes.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Remove variable `$size`, which is not used here and likely a copy-paste
redundancy from the `create_disks` subroutine.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Asserts whether the target storage supports storing VM images before
cloning a VM and its volumes to the target storage.
Without the check in place, a VMs volumes can be cloned to a storage,
which does not support VM images, but won't be able to start since any
attached volume must be stored on a supported storage.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Asserts whether the target storage supports storing VM images before
moving a VM volume to the target storage.
Without the check in place, a VM volume can be moved to a storage, which
does not support VM images, but won't be able to start since any
attached volume must be stored on a supported storage.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
Tests whether when running `config_to_command` it will correctly fail
with an error message that a volume cannot be used if the underlying
storage does not support its content type.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
[FE: add 'content' key to storage hash to be slightly more realistic]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
attempting to write such a config is already a bug, and execution should not
continue in this case. very often a write of the config will be followed by
reloading it, expecting changes to be persisted and possibly missing
re-checking of the original reason for marking the config as NoWrite.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
instead of blessing a passed-in config and returning it, explicitly only bless
without returning to make it more obvious to callers that this affects the
argument.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
For PBS, templates are started in 'prelaunch' mode for backup and the
same can be done for VMA for consistency.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Previously, the template's configuration was used as-is for the rest
of handling the VM start even if config_to_command() uses a minimized
configuration to build the command. This can lead to issues with a
network device with the 'link_down' flag set, because the network
device will not be present, but the start handling will still issue a
QMP command for it, leading to a failed backup operation.
Use the minimized configuration for the whole start-up handling to
avoid such issues.
Use the special QemuConfig::NoWrite class to safeguard against
accidentally writing out the temporarily modified config.
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
To be used in the context of template backup, where a minimized
temporary configuration is created to start the VM in 'prelaunch'
mode. Issue a warning, so that code paths where this happens will be
noted and can be evaluated and adapted.
Since the code currently doesn't use blessed config objects, special
dispatching is needed to potentially defer to the new child class in
the write_config() method.
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
A failure during enforce_vm_running_for_backup() would result in
restore_vm_power_state() not being called, but the VM might already
have been started before the failure. In particular, this could happen
in the context of bug #6007 [0], where the 'set_link' QMP command
fails right after VM start.
If the failure happens before successful start, there will be an
additional error message issued by restore_vm_power_state() (that the
VM is not running). This could be avoided by returning early if the VM
is not running anymore, but that would mean not warning about it in
other scenarios where it is not expected and keeping track of whether
the VM was actually started or not does not seem to be worth it just
for avoiding that error message in this edge case.
[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=6007
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Template-VMs might be started in 'prelaunch' state when creating a
backup, but they must never be actually started fully, to avoid
modifying their disks.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Fixes device hotplug in combination with QEMU 9.2.
QEMU commit be93fd5372 ("qdev-monitor: avoid QemuOpts in QMP device_add")
notes:
> This patch changes the behavior of QMP device_add but not HMP
> device_add. QMP clients that sent incorrectly typed device_add QMP
> commands no longer work. This is a breaking change but clients should be
> using the correct types already.
The qemu_deviceadd() helper does not have the required type
information right now, so switch to using HMP, which still behaves the
same when passing a device commandline string. QEMU commit be93fd5372
fixes passing in complex properties via JSON, but the qemu_deviceadd()
helper never uses any such, as it already only received a string (and
naively split it up).
Use HMP for 'device_del' too, simply to keep the qemu_deviceadd() and
qemu_devicedel() helpers consistent.
Switching back to QMP using the correct types in the JSON can still be
done later. Unfortunately, 'qmp-query-schema' does not provide
device-specific types, so another way is needed.
A timeout of 25 seconds is used rather then relying on the low default
like before, since device plug operations require actions by the guest
kernel and might require IO. Device plug is often an interactive
operation, so a too high timeout could lead to bad UX. For now stay a
few seconds under the default timeout of 30 seconds of our web UI's
API request handler. Should specific devices need a higher timeout, it
can still be increased further for them in the future.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
[TL: reduce timeout from 30s to 25s to ensure sync API requests
(without a task worker) do not run into the frontend timeout]
Signed-off-by: Thomas Lamprecht <t.lamprecht@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>
