remote migration: fix online migration via API clients
As reported in the community forum [0], when a remote migration
request comes in via an API client, the -T flag for Perl is set, so an
insecure dependency in a call like unlink() in forward_unix_socket()
will fail with:
> failed to write forwarding command - Insecure dependency in unlink while running with -T switch
To fix it, untaint the problematic socket addresses coming from the
remote side. Require that all sockets are below '/run/qemu-server/'
and end with '.migrate' with the main socket being matched more
strictly. This allows extensions in the future while still being quite
strict.
move helper to check running QEMU version out of the 'Machine' module
The version of the running QEMU binary is not related to the machine
version and so it's a bit confusing to have the helper in the
'Machine' module. It cannot live in the 'Helpers' module, because that
would lead to a cyclic inclusion Helpers <-> Monitor. Thus,
'QMPHelpers' is chosen as the new home.
This can be prevented by using the 'write-blocking' copy mode (also
called active mode) for the mirror. However, with active mode, the
guest write speed is limited by the synchronous writes to the mirror
target. For this reason, a way to start out in the faster 'background'
mode and later switch to active mode was introduced in QEMU 8.2.
The switch is done once the mirror job for all drives is ready to be
completed to reduce the time spent where guest IO is limited.
The loop waiting for actively-synced to become true is not an endless
loop: Once the remaining dirty parts have been mirrored by the
background iteration, the actively-synced flag will be set. Because
the 'block-job-change' QMP command already succeeded, new writes will
be done synchronously to the target and thus not lead to new dirty
parts. If the job fails or vanishes (shouldn't actually happen,
because auto-dismiss is false), the loop will be exited and the error
propagated.
Reported rarely, but steadily over the years:
https://forum.proxmox.com/threads/78954/post-353651
https://forum.proxmox.com/threads/78954/post-380015
https://forum.proxmox.com/threads/100020/post-431660
https://forum.proxmox.com/threads/111831/post-482425
https://forum.proxmox.com/threads/111831/post-499807
https://forum.proxmox.com/threads/137849/
drive mirror: prevent wrongly logging success when completion fails differently
Currently, when completing a drive mirror job, only errors matching
"cannot be completed" will be handled. Other errors are ignored and
a wrong message that the job was completed successfully will be
printed to the log. An instance of this popped up in the community
forum [0].
The QMP command used for completing the job is either
'block-job-complete' or 'block-job-cancel'. The former causes the VM
to switch to the target drive, the latter doesn't, e.g. migration uses
the latter to not switch the source instance over to the target drive.
The 'block-job-cancel' command doesn't even have the same "cannot be
completed" message, but returns immediately.
The timeout for both 'block-job-cancel' and 'block-job-complete' is
set to 10 minutes in the QMPClient module, which should be enough.
Mira Limbeck [Tue, 30 Jul 2024 15:15:39 +0000 (17:15 +0200)]
fix 4493: cloud-init: fix generated Windows config
Cloudbase-Init, a cloud-init reimplementation for Windows, supports only
a subset of the configuration options of cloud-init. Some features
depend on support by the Metadata Service (ConfigDrive2 here) and have
further limitations [0].
To support a basic setup the following changes were made:
- password is saved as plaintext for any Windows guests (ostype)
- DNS servers are added to each of the interfaces
- SSH public keys are passed via metadata
Network and metadata generation for Cloudbase-Init is separate from the
default ConfigDrive2 one so as to not interfere with any other OSes that
depend on the current ConfigDrive2 implementation.
DNS search domains were removed because Cloudbase-Init's ENI parser
doesn't handle it at all.
The password set via `cipassword` is used for the Admin user configured
in the cloudbase-init.conf in the guest while the `ciuser` parameter is
ignored. The Admin user has to be set in the cloudbase-init.conf file
instead.
Specifying a different user does not work.
For the password to work the `ostype` needs to be any Windows variant
before `cipassword` is set. Otherwise the password will be encrypted and
the encrypted password used as plaintext password in the guest.
The `citype` needs to be `configdrive2`, which is the default for
Windows guests, for the generated configs to be compatible with
Cloudbase-Init.
As reported in the community forum [0], after migration, the VM might
not immediately be able to respond to QMP commands, which means the VM
could fail to resume and stay in paused state on the target.
The reason is that activating the block drives in QEMU can take a bit
of time. For example, it might be necessary to invalidate the caches
(where for raw devices a flush might be needed) and the request
alignment and size of the block device needs to be queried.
In [0], an external Ceph cluster with krbd is used, and the initial
read to the block device after migration, for probing the request
alignment, takes a bit over 10 seconds[1]. Use 60 seconds as the new
timeout to be on the safe side for the future.
All callers are inside workers or via the 'qm' CLI command, so bumping
beyond 30 seconds is fine.
Stefan Hanreich [Tue, 23 Jul 2024 14:24:07 +0000 (16:24 +0200)]
fix #5619: honor link-down setting when hot-plugging NIC
When detaching and attaching the network device on update, the
link_down setting is not considered and the network device always gets
attached to the guest - even if link_down is set.
Fixes: 3f14f206 ("nic online bridge/vlan change: link disconnect/reconnect") Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
fix #5528: override cgroup methods to call systemd via dbus
Systemd reapplies its known values on reload, so we cannot simply call
into PVE::CGroup. Call systemd's SetUnitProperties method via dbus
instead.
The hotplug and startup code also calculated different values, as one
operated within systemd's value framework (documented in
systemd.resource-control(5)) and one worked with cgroup values
(distinguishing between cgroup v1 and v2 manually).
This is now unified by overriding `change_cpu_quota()` and
`change_cpu_shares()` via `PVE::QemuServer::CGroup` which now takes
systemd-based values and sends those directly via dbus.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
fix #5574: api: fix permission check for 'spice' usb port
With the last change in the permission check, I accidentally broke the
check for 'spice' host value, since in the if/elsif/else this will fall
through to the else case which was only intended for when neither 'host'
nor 'mapping' was set.
This made 'spice' only settable by root@pam since there we return early.
To fix this, move the spice check into the 'host' branch, but only error
out in case it's not spice.
Fixes: e3971865 (enable cluster mapped USB devices for guests) Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Stefan Hanreich [Wed, 17 Jul 2024 13:06:02 +0000 (15:06 +0200)]
config: net: avoid duplicate ipam entries on nic update
Updating the NIC of a VM when the following conditions were met:
* VM is turned off
* NIC is on a bridge that uses automatic dhcp
* Leave bridge unchanged
led to duplicate IPAM entries for the same network device.
This is due to the fact that the add_next_free_cidr always ran on
applying pending network changes.
Now we only add a new ipam entry if either:
* the value of the bridge or mac address changed
* the network device has been newly added
This way no duplicate IPAM entries should get created.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Dominik Csapak [Thu, 6 Jun 2024 09:22:05 +0000 (11:22 +0200)]
stop cleanup: remove unnecessary tpmstate cleanup
tpmstate0 is already included in `get_vm_volumes`, and our only storage
plugin that has unmap_volume implemented is the RBDPlugin, where we call
unmap in `deactivate_volume`. So it's already ummapped by the
`deactivate_volumes` calls above.
For third-party storage plugins, it's natural to expect that
deactivate_volume() would also remove a mapping for the volume just
like RBDPlugin does.
While there is an explicit map_volume() call in start_swtpm(), a
third-party plugin might expect an explicit unmap_volume() call too.
However, the order of calls right now is
1. activate_volume()
2. map_volume()
3. deactivate_volume()
4. unmap_volume()
Which seems like it could cause problems already for third-party
plugins relying on an explicit unmap call.
All that said, it is unlikely that a third-party plugin breaks. If it
really happens, it can be discussed/adapted to the actual needs still.
Fiona Ebner [Tue, 28 May 2024 08:50:03 +0000 (10:50 +0200)]
migration: handle replication: remove outdated and inaccurate check for QEMU version
In Proxmox VE 8, the oldest supported QEMU version is 8.0, so a
check for version 4.2 is not required anymore. The check was also
wrong, because it checked the installed version and not the currently
running one.
Fiona Ebner [Fri, 14 Jun 2024 11:29:46 +0000 (13:29 +0200)]
autocomplete: backup: also list archives from PBS storages and without compressor extension
While archives with unknown or undetermined subtype could be shown,
this is only for autocompletion, so users can still specify those
manually if required.
fix #5572: avoid warning about uninitialized value when cloning cloudinit disk
Some callers like the move disk API endpoint do not pass an explicit
completion argument. This is not an issue in general, because
qemu_drive_mirror_monitor() defaults to 'complete'. However, there was
a string comparision for the cloudinit case that can trigger a warning
about the value being uninitialized.
mirror: do not auto-dismiss to allow getting error message from job
upon failure. Otherwise, the job would disappear too quickly from the
job list and cannot be queried for the actual error anymore.
Relevant part of the error in actual examples (note that the fact that
it's a mirror job is already mentioned earlier in the full error, with
"block job (mirror) error:"):
Before:
> 'mirror' has been cancelled
> 'mirror' has been cancelled
After:
> Source and target image have different sizes (io-status: ok)
> No space left on device (io-status: ok)
blockjob: anticipate jobs with auto-dismiss=false for better error messages and detection
When auto-dismiss=true (the default), a failed job can disappear very
quickly from the job list and there might not be any chance to see the
error in the result of 'query-block-jobs'. For jobs with $completion
being 'auto', like 'block-stream', it couldn't even be detected that
the job failed.
Jobs with auto-dismiss=false on the other hand, will wait in
'concluded' state until manually dismissed. For those, it will be
possible to query the error if the job failed.
There doesn't seem to be a way to have only failed jobs stay around,
e.g. something like auto-dismiss=on-success.
Planned to be used for the 'drive-mirror' and 'block-stream' jobs
initially.
Dominik Csapak [Thu, 20 Jun 2024 10:09:22 +0000 (12:09 +0200)]
fix #3352: templates: minimize config when starting templates
templates can only be started in context of a pbs backup, and there we
don't need or want to use most of the config, since they cannot be
started normally anyway.
We minimize the config by copying some specific relevant options (see
the comments for why the options were chosen) and all disk
configurations.
Since we change the qemu commandline for templates, we now have to adapt
the tests involving templates.
Without this, users can get into a situation where the template cannot
be backed up when there are some resources not available (such as cpu
cores, kvm, pci devices, etc.) even if the backup process does not need
them.
This change has some nice side effects, such as we don't need to
allocate the full amount of memory anymore for templates that have a
hostpci device configured, the configured bridges don't have to exist,
etc.
Fiona Ebner [Thu, 27 Jun 2024 11:03:17 +0000 (13:03 +0200)]
api: update vm: prohibit changing version of TPM state
After the TPM state has been created (to be precise, initialized by
swtpm) it is not possible to change the version anymore. Doing so will
lead to failure starting the associated VM. While documented in the
description, it's better to enforce this via API.
Fiona Ebner [Thu, 27 Jun 2024 11:03:16 +0000 (13:03 +0200)]
drive: tpm: fix default version in schema
Since the check in start_swtpm() only checks for an explicitly
configured v2.0 to opt-in to version 2, the actual default is v1.2
and not v2.0 like the schema stated.
Of course, it would be nicer to have the default be v2.0, but changing
the check to use that default would break any TPM state without an
explicitly configured version.
There doesn't seem to be any code beside start_swtpm() accessing the
version.
Fixes: f9dde219 ("fix #3075: add TPM v1.2 and v2.0 support via swtpm") Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Fiona Ebner [Tue, 18 Jun 2024 07:52:47 +0000 (09:52 +0200)]
vma restore: improve timeout error messages
The generic "got timeout" message cannot be associated to a certain
code path and also isn't very user-friendly. Use dedicated messages
for each stage and also suggest why the timeout for reading the header
might have happened, i.e. because it was corrupted.
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
The default timeout for HMP commands is 5 seconds.
While it should be rather fast to attach a new drive to QEMU, a busy
system might take longer, so future-proof and increase to 60 seconds.
On the other hand, detaching a drive needs to complete any pending IO
on it, so use the same 10 minutes timeout that's used for
drive-related QMP commands.
Fiona Ebner [Fri, 3 May 2024 11:19:53 +0000 (13:19 +0200)]
cli: qm: increase timeout for monitor commands to 30 seconds
The default timeout is 5 seconds, but some HMP commands (e.g.
disk-related ones) might take longer than that. It's still an
interactive session, so use 30 seconds for now. Should there be any
user-complains about frequent timeouts, it could still be increased
further.
Fiona Ebner [Fri, 3 May 2024 11:19:52 +0000 (13:19 +0200)]
vzdump: increase timeout for attaching drives to 60 seconds
The default timeout for HMP commands is 5 seconds and while it should
be rather fast to attach a new drive to QEMU, a system can be very
busy during backup, so future-proof and increase to 60 seconds.
Fiona Ebner [Fri, 3 May 2024 11:19:51 +0000 (13:19 +0200)]
api: human monitor: increase timeout to 25 seconds
The default timeout is 5 seconds, but some HMP commands (e.g.
disk-related ones) might take longer than that. The API call is
synchronous, so has to complete within 30 seconds, and since there is
no other costly operation, use 25 seconds.
Fiona Ebner [Fri, 3 May 2024 11:19:50 +0000 (13:19 +0200)]
monitor: allow passing timeout for a HMP command
Passing the timeout key with an explicit value of undef is fine,
because both the absence of the timeout key and an explicit value of
undef will lead to $timeout being undef in the qmp_cmd() function.
In preparation to increase the timeout for certain (e.g. disk-related)
HMP commands.
Fiona Ebner [Tue, 14 May 2024 14:11:13 +0000 (16:11 +0200)]
suspend: continue cleanup even if savevm-end QMP command fails
The savevm-end command also fails when no snapshot operation was
started before. In particular, this is the case when savevm-start
failed early, because of unmigratable devices.
Avoid potentially leaving an orphaned volume and snasphot-related
configuration keys around by continuing with cleanup instead.
Fiona Ebner [Fri, 3 May 2024 12:01:35 +0000 (14:01 +0200)]
schema: fix description of migrate_downtime parameter
Since commit 865ef132 ("implement dynamic migration_downtime") the
migration downtime will be automatically increased when migration
cannot converge at the very end. Update the description to reflect
reality.
Filip Schauer [Wed, 24 Apr 2024 09:14:33 +0000 (11:14 +0200)]
cpu config: fix get_cpu_bitness always reverting to default cpu type
This fixes the broken prevention of starting a VM with a 32-bit CPU
using a 64-bit OVMF (UEFI) BIOS.
Fixes: 89d5b1c9 ("prevent starting a 32-bit VM using a 64-bit OVMF BIOS") Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
[FE: add Fixes trailer, add prefix to title] Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Fiona Ebner [Tue, 10 Oct 2023 08:57:03 +0000 (10:57 +0200)]
qmeventd: also treat 'prelaunch' and 'suspended' states as active
Otherwise, a VM in those states would be terminated after a backup
in handle_qmp_return() with QMP 'quit', which is pretty bad in case
of the 'suspended' state.
Does not change the fact that a VM started in prelaunch mode for
backup is terminated later (that is handled by the Perl code).
Thomas Lamprecht [Sat, 20 Apr 2024 10:23:33 +0000 (12:23 +0200)]
api: create vm: fix missing import for serializing machine type
The machine handling was transformed into a full fledged property
string with a (sub) format, but the single call-site for print_machine
was seemingly not tested, as this could have never worked due to a
missing import of the print_property_string helper.
Fixes: 8082eb8 ("config: define machine schema as property-string") Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Hanreich [Fri, 19 Apr 2024 09:42:34 +0000 (11:42 +0200)]
firewall: add handling for new nft firewall
When the nftables firewall is enabled, we do not need to create
firewall bridges.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
[ TL: use a more meaningful variable name and add a comment ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Management for fleecing images is implemented here. If the fleecing
option is set, for each disk (except EFI disk and TPM state) a new
fleecing image is allocated on the configured fleecing storage (same
storage as original disk by default). The disk is attached to QEMU
with the 'size' parameter, because the block node in QEMU has to be
the exact same size and the newly allocated image might be bigger if
the storage has a coarser allocation or rounded up. After backup, the
disks are detached and removed from the storage.
If the storage supports qcow2, use that as the fleecing image format.
This allows saving some space even on storages that do not properly
support discard, like, for example, older versions of NFS.
Since there can be multiple volumes with the same volume name on
different storages, the fleecing image's name cannot be just based on
the original volume's name. The schema vm-ID-fleece-N(.FORMAT) with N
incrementing for each disk is used.
Partially inspired by the existing handling of the TPM state image
during backup.
clone disk: prevent 'uninitialized value' warning for unused check
since commit 1f743141 (fix #1905: Allow moving unused disks)
we want to check the source drive name for 'unused', but in case of
importing a volume from the 'import' content type (e.g. from esxi),
there is no source drive name. So we have to first check if it's
defined.
The previous wording made it sound like all "visible" tasks were
aborted, which is not the case: A user with Sys.Audit but without
Sys.Modify may see a task that was started by a different user, but
overrule-shutdown would not abort the task.
Change wording to better reflect that not all visible tasks may be
aborted.
Also, add a full-stop that was previously missing.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
Friedrich Weber [Fri, 12 Apr 2024 14:15:51 +0000 (16:15 +0200)]
fix #4474: qemu api: add overrule-shutdown parameter to stop endpoint
The new `overrule-shutdown` parameter is boolean and defaults to 0. If
it is 1, all active `qmshutdown` tasks for the same VM (which are
visible to the user/token) are aborted before attempting to stop the
VM.
Passing `overrule-shutdown=1` is forbidden for HA resources.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
Filip Schauer [Mon, 19 Feb 2024 11:11:39 +0000 (12:11 +0100)]
fix #1905: Allow moving unused disks
In the past, moving unused disks to another storage was prohibited due
to oversights in the handling of unused disks. This commit rectifies
this limitation by allowing the movement of unused disks.
Historical context:
* 16 Sep 2010 r5164 qemu-server/pve2: The disknames sub was removed.
* 17 Sep 2010 r5170 qemu-server/pve2: Unused disks were introduced.
* 28 Jan 2011 r5461 qemu-server/pve2: The same disknames sub that was
removed in r5164 was brought back. Since unused disks were not around
yet in r5164 the disknames sub did not consider unused disks.
* 6-8 Aug 2012 c1175c92..f91b2e45 qemu-server.git: Disk resize was
introduced. In commit c1175c92 in sub qemu_block_resize unused disks
were not taken into account and in commit 2f48a4f5 (8 Aug 2012) the
resize API call was changed to only allow disks matching the ones in
the disknames sub. Since sub disknames did not contain any unused
disks, those were not allowed at all in the resize API call.
* 27 May 2013 586bfa78 qemu-server.git: Disk move was introduced. The
API call implementation borrowed heavily from disk resize, including
the behaviour of not taking unused disks into account. Thus, unused
disk could not be moved, which persists to this day.
In summary, this behaviour was introduced because the handling of unused
disks was overlooked and it was never changed.
There is no inherent reason why unused disks should be restricted from
being moved to another storage. These disks cannot use the
qemu_drive_mirror, but they can still be moved with qemu_img_convert,
the same way as any other disk of a stopped VM.
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
Markus Frank [Thu, 11 Apr 2024 10:48:20 +0000 (12:48 +0200)]
fix #3784: config: Parameter for guest vIOMMU + test-cases
vIOMMU enables the option to passthrough pci devices to L2 VMs in L1
VMs via Nested Virtualisation and adds an extra isolation.
Uses the new property-string from the "config: define machine schema
as property-string"-commit to add the viommu option to the machine
parameter.
Currently there are two vIOMMU implementation in QEMU to choose:
intel or virtio
Virtio-iommu is more recent but less used in production than intel-iommu.
The assert_valid_machine_property function prevents using intel-iommu with
i440fx.
Signed-off-by: Markus Frank <m.frank@proxmox.com>
[ TL: tiny coding style fix to extract variable inside if expr ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Hannes Duerr [Wed, 10 Apr 2024 11:17:29 +0000 (13:17 +0200)]
fix #5363: cloudinit: make creation of scsi cloudinit discs possible again
Upon obtaining the device type, a check is performed to determine if it
is a CD drive. It is important to note that Cloudinit drives are always
assigned as CD drives. If the drive has not yet been allocated, the test
will fail due to the unset cd attribute.
To avoid this, an explicit check is now performed to determine if it is
a Cloudinit drive that has not yet been assigned.
Fixes: d1feab4 ("fix #4957: add vendor and product information passthrough for SCSI-Disks") Signed-off-by: Hannes Duerr <h.duerr@proxmox.com>
Filip Schauer [Mon, 11 Mar 2024 10:13:08 +0000 (11:13 +0100)]
cpu config: die on hotplug of non x86_64 CPUs
When attempting a CPU hotplug on an architecture other than x86_64, die
with a clean error instead of attempting a hotplug with a known
non-working device command line. Also move the corresponding FIXME up to
the error.
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
Thomas Lamprecht [Thu, 14 Mar 2024 08:06:22 +0000 (09:06 +0100)]
qm: add VM import command
Add a command that can be used together with volumes from the new
'import' content type of storage plugins.
For now only the new ESXi exposes that content type, but in the long
run its planned to migrate over the existing OVF/OVA infra and extend
it so that it will replace the 'ovfimport' command.
Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: split out to separate commit and add message, fix completing
VMID to propose unused ones, note explicitly when in dry-run mode ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
instead of a "pbs-backing" parameter we now have a
"live-restore-backing" parameter containing the `-blockdev` arg and
its name, which also means we print the blockdev earlier
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Thomas Lamprecht [Sun, 10 Mar 2024 17:12:21 +0000 (18:12 +0100)]
config: pending network: avoid undef-warning on old/new comparison
A network device of a VM does not necessarily has to be connected to
an actual bridge, so when a new pending value is set we need to use
the undef-safe compare helpers when checking if there was a change
between old and new value, as otherwise one gets ugly "use of
uninitialized value in string ne" warnings.
cpu config: implement is_native_arch locally for now
could be a better fit in PVE::Tools, like proposed by Filip, but OTOH.
Tools is already crowded as is, so wait if we need it on more places
outside of qemu-server.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Fiona Ebner [Fri, 9 Feb 2024 12:14:25 +0000 (13:14 +0100)]
QMP client: remove unnecessary question mark from comment
There might've been a question back when it got first added in commit 9d689077 ("use long timeouts for snapshot monitor command"). But
nowadays, the value is well-established. Changing it would affect
quite a few operations, so that should not be done without good
reason and is likely better done for the specific operation.
Filip Schauer [Wed, 21 Feb 2024 14:33:16 +0000 (15:33 +0100)]
cpu config: Unify the default value for 'kvm'
Make the default value for 'kvm' consistent, taking into account
whether the VM will run on the same CPU architecture as the host.
This would be a breaking change to CPU hotplug for VMs with a
different CPU architecture running on an x86_64 host, as in this case
the default CPU type for CPU hotplug changes from 'kvm64' to 'qemu64'.
However, CPU hotplug of non x86_64 architectures is not supported
anyway, so this is not a breaking change after all.
It should be noted that this change does alter the CPU hotplug
behaviour when emulating an x86_64 CPU on a non-x86_64 host. This is
however not officially supported in Proxmox VE.
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
Filip Schauer [Wed, 21 Feb 2024 14:33:14 +0000 (15:33 +0100)]
prevent starting a 32-bit VM using a 64-bit OVMF BIOS
Instead of starting a VM with a 32-bit CPU type and a 64-bit OVMF image,
throw an error before starting the VM telling the user that OVMF is not
supported on 32-bit CPU types.
To obtain a list of 32-bit CPU types, refer to the builtin_x86_defs in
target/i386/cpu.c of QEMU. Exclude any entries that have the long mode
feature (CPUID_EXT2_LM).
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
Dominik Csapak [Thu, 7 Mar 2024 09:33:37 +0000 (10:33 +0100)]
mediated device pass-through: fix race condition on VM reboot
When rebooting a VM from PVE (via CLI/API), the reboot code is called
under a guest lock, which creates a reboot request, shuts down the VM
and then calls the regular cleanup code, which includes the mdev
cleanup.
In parallel, the qmeventd observes that the VM process has gone, and
starts 'qm cleanup' which is (among other tasks) also starts the VM
again if a reboot from the PVE side is pending.
The qmeventd synchronizes this through a lock on the guest, with a
default timeout of 10 seconds.
Since we currently also always wait 10 seconds for the NVIDIA driver
to clean up the mdev, this creates a race condition for the cleanup
lock. IOW., when the call to `qm cleanup` starts before we started to
sleep for 10 seconds, it will not be able to acquire its lock and not
start the vm again.
To avoid the race condition in practice, do two things:
* increase the timeout in `qm cleanup` to 60 seconds.
Technically this still might run into a timeout, as we can configure
up to 16 mediated devices with each delaying 10 seconds in the worst
case, but realistically most users won't configure more than two or
three of them, if even that.
* change the hard-coded `sleep 10` to a loop sleeping for 1 second
each before checking the state again. This shortens the timeout when
the NVIDIA driver did not require the full 10s to finish the
clean-up.
Further, add a bit of logging, so one can properly see in the task log
what is happening at which point in time.
Fixes: 49c51a60 (pci: workaround nvidia driver issue on mdev cleanup) Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com>
[ TL: change warn to print, reword commit message ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
api: clone vm: comment and style clean-up deactivation error-handling
Make the post-if check for the target not already running more
prominent by using a full if block.
Also comment on why we ignore the error here, while the commit
changing that explained it well, this is one of the things that might
be better of with a in-code comment (as doing the deactivation is
described as important here, so one might wonder why the code
continues if that fails)
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Hannes Duerr [Wed, 6 Mar 2024 14:08:34 +0000 (15:08 +0100)]
fix #1734: clone VM: if deactivation fails demote error to warning
When a template with disks on LVM is cloned to another node, the
volumes are first activated, then cloned and deactivated again after
cloning.
However, if clones of this template are now created in parallel to
other nodes, it can happen that one of the tasks can no longer
deactivate the logical volume because it is still in use. The reason
for this is that we use a shared lock.
Since the failed deactivation does not necessarily have consequences,
we downgrade the error to a warning, which means that the clone tasks
will continue to be completed successfully.
Signed-off-by: Hannes Duerr <h.duerr@proxmox.com> Tested-by: Friedrich Weber <f.weber@proxmox.com>
Fiona Ebner [Wed, 31 Jan 2024 10:53:59 +0000 (11:53 +0100)]
api: fix using import-from with SCSI disks
by fixing the SCSI feature compatibility check helper. The helper is
also called for disks using import-from, so it has to use the extended
schema when parsing the drive.
Fixes: d1feab4a ("fix #4957: add vendor and product information passthrough for SCSI-Disks") Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
PVE::Storage::path() neither activates the storage of the passed-in volume, nor
does it ensure that the returned value is actually a file or block device, so
this actually fixes two issues. PVE::Storage::abs_filesystem_path() actually
takes care of both, while still calling path() under the hood (since $volid
here is always a proper volid, unless we change the cicustom schema at some
point in the future).