In preparation to add more properties to the memory configuration like
maximum hotpluggable memory and whether virtio-mem devices should be
used.
This also allows to get rid of the cyclic include of PVE::QemuServer
in the memory module.
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
[FE: also convert new usage in get_derived_property
remove cyclic include of PVE::QemuServer
add commit message] Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
PVE::QemuServer::check_running() does both
PVE::QemuConfig::assert_config_exists_on_node()
PVE::QemuServer::Helpers::vm_running_locally()
The former one isn't needed here when doing hotplug, because the API
already assert that the VM config exists. It also would introduce a
new cyclic dependency between PVE::QemuServer::Memory <->
PVE::QemuConfig with the proposed virtio-mem patch set.
In preparation to remove the cyclic include of PVE::QemuServer in the
memory module.
fix #2816: restore: remove timeout when allocating disks
10 minutes is not long enough when disks are large and/or network
storages are used when preallocation is not disabled. The default is
metadata preallocation for qcow2, so there are still reports of the
issue [0][1]. If allocation really does not finish like the comment
describing the timeout feared, just let the user cancel it.
Also note that when restoring a PBS backup, there is no timeout for
disk allocation, and there don't seem to be any user complaints yet.
The 5 second timeout for receiving the config from vma is kept,
because certain corruptions in the VMA header can lead to the
operation hanging there.
There is no need for the $tmp variable before setting back the old
timeout, because that is at least one second, so we'll always be able
to set the $oldtimeout variable to undef in time in practice.
Currently, there shouldn't even be an outer timeout in the first
place, because the only call path leading to here is via the create
API (also used by qmrestore), both of which don't set a timeout.
Filip Schauer [Thu, 10 Aug 2023 09:20:36 +0000 (11:20 +0200)]
fix #3963: Skip TPM startup for template VMs
Skip the software TPM startup when starting a template VM for performing
a backup. This fixes an error that occurs when the TPM state disk is
write-protected.
Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
Quoting from QEMU commit 4271f40383 ("virtio-net: correctly report
maximum tx_queue_size value"):
> Maximum value for tx_queue_size depends on the backend type.
> 1024 for vDPA/vhost-user, 256 for all the others.
> So the parameter is silently ignored and ethtool reports a different
> value than the one provided by the user.
Indeed, for a non-vDPA/vhost-user netdev, the guest will see TX: 256
instead of the specified 1024 here. With the mentioned QEMU commit (in
master and will be part of 8.1), using 1024 will be a hard error:
> Invalid tx_queue_size (= 1024), must be a power of 2 between 256 and 256
Since neither vhost-user, nor vhost-vdpa netdev types are exposed by
Proxmox VE, just changing the limit to the correct 256 should be fine.
No obvious issue during live-migration found.
migration: alloc nbd disks: fix fall-back for remote live migration
While the comment sated
> # order of precedence, filtered by whether storage supports it:
> # 1. explicit requested format
> # 2. format of current volume
> # 3. default format of storage
the code did not fall back to the default format in the case of remote
migration, because the format was already set and the code used
> $format //= $defFormat;
This made remote migration from dir with qcow2 to e.g. LVM-thin fail.
Move extracting the format from the volume name to the call side for
local migration. This allows the logic here to be much simpler.
migration: alloc nbd disks: base format hint off source storage
Previously, qemu_img_format() was called with the target storage's
$scfg and the source storage's volume name.
This mismatch should only be relevant for certain special kinds of
storage plugins:
- no path, but does support an additional QEMU image format besides
'raw', in short NPAF.
- no path, volume name can match QEMU_FORMAT_RE, in short NPVM.
Note that all integrated plugins are neither NPAF nor NPVM.
Note that for NPAF plugins, qemu_img_format() already always returns
'raw' because there is no path. It's a bit unlikely such a plugin
exists, because there were no bug reports about qemu_img_format()
misbehaving there yet.
Let's go through the cases:
- If source and target storage both have or don't have a path,
qemu_img_format($scfg, $volname) returns the same for both $scfg's.
- If source storage has a path, but target storage does not, the
format hint was previously 'raw', but can only be more correct now
(being what the source image actually is):
- For non-NPAF targets, since we know there is no path, it follows
that 'raw' is the only supported QEMU image format.
- For NPAF targets, the format will be preserved now (if actually
supported).
- If source storage does not have a path, but target storage does, the
format hint will be 'raw' now.
- For non-NPVM sources, QEMU_FORMAT_RE didn't match when
qemu_img_format() was called with the target storage's $scfg, so
the hint also was 'raw' before this commit.
- For NPVM sources, qemu_img_format() might've guessed a format from
the source volume name when called with the target's $scfg before
this commit. If the target storage supports the previously guessed
format, it was preserved before this commit, but will not be
anymore. In theory, the guess might've also been wrong, and in
this case, this commit avoids the wrong guess.
To summarize, there is only one edge case with an exotic kind of third
party storage plugin where format preservation would be lost and in
another edge case, format preservation is gained.
In preparation to simplify the format fallback logic implementation.
Friedrich Weber [Mon, 13 Mar 2023 12:56:25 +0000 (13:56 +0100)]
fix: api: fix permission check for cloudinit drive update
Trying to regenerate a cloudinit drive as a non-root user via the API
currently throws a Perl error, as reported in the forum [1]. This is
due to a type mismatch in the permission check, where a string is
passed but an array is expected.
Friedrich Weber [Mon, 24 Jul 2023 11:33:48 +0000 (13:33 +0200)]
cloudinit: allow non-root users to set ciupgrade option
The new ciupgrade option was missing in $cloudinitoptions in
PVE::API2::Qemu, so $check_vm_modify_config_perm defaulted to
requiring root@pam for modifying the option. To fix this, add
ciupgrade to $cloudinitoptions. This also fixes an issue where
ciupgrade was missing in the output of `qm cloudinit pending`,
as it also relies on $cloudinitoptions.
This issue was originally reported in the forum [0].
Also add a comment to avoid similar issues when adding new options in
the future.
[0]: https://forum.proxmox.com/threads/131043/
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
fix #4620: cfg2cmd: drive device: correctly handle IDE for q35
Only unit 0 for IDE is supported with machine type q35. Currently,
QEMU will fail startup with machine type q35 with an error like
> Can't create IDE unit 1, bus supports only 1 units
when ide1 or ide3 is configured.
Make sure to keep backwards compat form migration by leaving ide0 and
ide2 fixed. Since starting with ide1 or ide3 never worked, they can be
moved to a controller with a higher ID without issue.
Reported in the community forum:
https://forum.proxmox.com/threads/124615/post-543127
https://forum.proxmox.com/threads/130815/
qmeventd: VMID from PID: don't fail immediately when encountering unexpected entry
While such entries shuoldn't exist in practice without manually
messing around, it's not hard to imagine, and there's still a good
chance the actual VMID can still be found.
It's still unexpected to encounter such entries so keep the log line.
but the order doesn't seem to be deterministic, so it can happen that
an entry like '11:devices:/qemu.slice' is the first to match the
'/qemu.slice' part, which previously made the code expect to find the
VMID.
To improve detection, as a first step, match the trailing slash too.
Reported in the community forum:
https://forum.proxmox.com/threads/129320/post-571654
vzdump: pbs: factor out getting and checking encryption keys
factor the common checks for disk-less and "normal" backups out into
its own helper, avoiding code duplication and ensuring that the
messages and checks stay in sync.
The use sites for key and master key are a bit clearer, as it all
just depends on them being defined or not.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
vzdump: reword "master-key but no encryption key" message
.. and make it use a warn level, which can then also mark the whole
task as potentially problematic as with a new enough pve-guest-common
the REST environment worker warn counters are then increased.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
our backup logs are still quite noise at the task start part so avoid
logging that the task is running with encryption enabled twice for
the master-key feature.
The definedness check on master_keyfile isn't required anymore, it
was never for the no-disk case, and for the standard case it isn't
since 781fb80 ("vzdump: error out for master-key backup but no QEMU
support")
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
vzdump: error out for master-key backup but no QEMU support
Our QEMU gained master-key support for Proxmox VE 6.4 with initial
QEMU 5.2.0 packaging in 0b8da68 ("add PBS master key support")
version.
As we're now two major releases in the future any VM needs to run
with a newer QEMU version we can just make this a hard-error, as
there really should be no use-case left. After all we only support
upgrading directly to the next major release, so one needs to do at
least a migration (or shutdown) of the VM to reboot the node for
upgrading to Proxmox VE 8, so the lowest QEMU version baseline is 6.0
for Proxmox VE 8 (i.e., the version from PVE 7.0).
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Aaron Lauterer [Mon, 19 Jun 2023 09:29:33 +0000 (11:29 +0200)]
migration: fail when aliased volume is detected
Aliased volids can lead to unexpected behavior in a migration.
An aliased volid can happen if we have two storage configurations,
pointing to the same place. The resulting 'path' for a disk image
will be the same.
Therefore, stop the migration in such a case.
The check works by comparing the path returned by the storage plugin.
We decided against checking the storages themselves being aliased. It is
not possible to infer that reliably from just the storage configuration
options alone.
Aaron Lauterer [Mon, 19 Jun 2023 09:29:31 +0000 (11:29 +0200)]
qemuserver: migration: test_volid: change attr name and ref handling
Since we don't scan all storages for matching disk images anymore for a
migration we don't have any images found via storage alone. They will be
referenced in the config somewhere.
Therefore, there is no need for the 'storage' ref.
The 'referenced_in_config' is not really needed and can apply to both,
attached and unused disk images.
Therefore the QemuServer::foreach_volid() will change the
'referenced_in_config' attribute to an 'is_attached' one that only
applies to disk images that are in the _main_ config part and are not
unused.
In QemuMigrate::scan_local_volumes() we can then quite easily map the
refs to each state, attached, unused, referenced_in_{pending,snapshot}.
The refs are mostly used for informational use to print out in the logs
why a disk image is part of the migration. Except for the 'attached' case.
In the future the extra step of the refs in QemuMigrate could probably
be streamlined even more.
Aaron Lauterer [Mon, 19 Jun 2023 09:29:30 +0000 (11:29 +0200)]
migration: only migrate disks used by the guest
When scanning all configured storages for disk images belonging to the
VM, the migration could easily fail if a storage is not available, but
enabled. That storage might not even be used by the VM at all.
By not scanning all storages and only looking at the disk images
referenced in the VM config, we can avoid unnecessary failures.
Some information that used to be provided by the storage scanning needs
to be fetched explicilty (size, format).
Behaviorally the biggest change is that unreferenced disk images will
not be migrated anymore. Only images referenced in the config will be
migrated.
Aaron Lauterer [Mon, 19 Jun 2023 09:29:29 +0000 (11:29 +0200)]
qemuserver: foreach_volid: always include pending disks
All calling sites except for QemuConfig.pm::get_replicatable_volumes()
already enabled it. Making it the non-configurable default results in a
change in the VM replication. Now a disk image only referenced in the
pending section will also be replicated.
Aaron Lauterer [Mon, 19 Jun 2023 09:29:28 +0000 (11:29 +0200)]
qemuserver: foreach_volid: include pending volumes
Make it possible to optionally iterate over disks in the pending section
of VMs, similar as to how snapshots are handled already.
This is for example useful in the migration if we don't want to rely on
the scanning of all storages.
All calling sites are adapted and enable it, except for
QemuConfig::get_replicatable_volumes as that would cause a change for
the replication if pending disks would be included.
The following lists the calling sites and if they should be fine with
the change (source [0]):
1. QemuMigrate: scan_local_volumes(): needed to include pending disk
images
2. API2/Qemu.pm: check_vm_disks_local() for migration precondition:
related to migration, so more consistent with pending
3. QemuConfig.pm: get_replicatable_volumes(): would change the behavior
of the replication, will not use it for now.
4. QemuServer.pm: get_vm_volumes(): is used multiple times by:
4a. vm_stop_cleanup() to deactivate/unmap: should also be fine with
including pending
4b. QemuMigrate.pm: in prepare(): part of migration, so more consistent
with pending
4c. QemuMigrate.pm: in phase3_cleanup() for deactivation: part of
migration, so more consistent with pending
Fiona Ebner [Wed, 21 Jun 2023 10:04:32 +0000 (12:04 +0200)]
cloudinit: restore previous default for package upgrades
Commit efa3355d ("fix #3428: cloudinit: add parameter for upgrade on
boot") changed the default, but this is a breaking change. The bug
report was only about making the option configurable.
The commit doesn't give an explicit reason for why, and arguably,
doing the upgrade is not an issue for most users. It also leads to a
different cloud-init instance ID, because of the different setting,
which in turn leads to ssh host key regeneration within the VM.
Reported-by: Friedrich Weber <f.weber@proxmox.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Thomas Lamprecht [Wed, 21 Jun 2023 07:13:50 +0000 (09:13 +0200)]
vm start: always reset any failed-state of the VM systemd scope
The scope can get into failed state for some issues like OOM kills of
the whole scope, in that case a user cannot re-start the VM until
they manually reset it.
Do this for now inline to avoid a pve-common bump as done in [0]
(location was suggested by me thinking we could maybe do it over
dbus, but as we have a stop command here already it probably doesn't
matters)
Dominik Csapak [Fri, 16 Jun 2023 13:05:24 +0000 (15:05 +0200)]
check local resources: extend for mapped resources
by adding them to their own list, saving the nodes where they are not
allowed, and return those on 'wantarray' so we don't break existing
callers that don't expect it.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Tested-By: Markus Frank <m.frank@proxmox.com>
Dominik Csapak [Fri, 16 Jun 2023 13:05:23 +0000 (15:05 +0200)]
enable cluster mapped PCI devices for guests
this patch allows configuring pci devices that are mapped via cluster
resource mapping when the user has 'Resource.Use' on the ACL path
'/mapping/pci/{ID}' (in addition to the usual required vm config
privileges)
When given multiple mappings in the config, we use them as alternatives
for the passthrough, and will select the first free one on startup.
It is using our regular pci reservation mechanism for regular devices and
we introduce a selection mechanism for mediated devices.
A few changes to the inner workings were required to make this work well:
* parse_hostpci now returns a different structure where we have a list
of lists (first level is for the different alternatives and second
level is for the different devices that should be passed through
together)
* factor out the 'parse_hostpci_devices' which parses each device from
the config and does some precondition checks
* reserve_pci_usage now behaves slightly different when trying to
reserve an device with the same VMID that's already reserved for,
since for checking which alternative we can use, we already must
reserve one (this means that qm showcmd can actually reserve devices,
albeit only for up to 10 seconds)
* configuring a mediated device on a multifunction device is not
supported anymore, and results in failure to start (previously, it
just chose the first device to do it). This is a breaking change
* configuring a single pci device twice on different hostpci slots now
fails during commandline generation instead on qemu start, so we had
to adapt one test where this occurred (it could never have worked
anyway)
* we now check permissions during clone/restore, meaning raw/real
devices can only be cloned/restored by root@pam from now on.
this is a breaking change.
Fixes #3574: Improve SR-IOV usability Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Tested-By: Markus Frank <m.frank@proxmox.com>
Dominik Csapak [Fri, 16 Jun 2023 13:05:22 +0000 (15:05 +0200)]
enable cluster mapped USB devices for guests
this patch allows configuring usb devices that are mapped via
cluster resource mapping when the user has 'Mapping.Use' on the ACL
path '/mapping/usb/{ID}' (in addition to the usual required vm config
privileges)
for now, this is only valid if there is exactly one mapping for the
host, since we don't track passed through usb devices yet
This now also checks permissions on clone/restore, meaning a
'non-mapped' device can only be cloned/restored as root@pam user.
That is a breaking change.
Refactor the checks for restoring into a sub, so we have central place
where we can add such checks
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Tested-By: Markus Frank <m.frank@proxmox.com>
Dominik Csapak [Fri, 16 Jun 2023 13:05:21 +0000 (15:05 +0200)]
usb: refactor usb code and move some into USB module
similar to how we handle the PCI module and format. This makes the
'verify_usb_device' method and format unnecessary since
we simply check the format with a regex.
while doing tihs, i noticed that we don't correctly check for the
case-insensitive variant for 'spice' during hotplug, so fix that too
With this we can also remove some parameters from the get_usb_devices
and get_usb_controllers functions
while were at it, refactor the permission checks for the usb config too
and use the new 'my sub' style for the functions
also make print_usbdevice_full parse the device itself, so we don't have
to do it in multiple places (especially in places where we don't see
that this is needed)
No functional change intended
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Tested-By: Markus Frank <m.frank@proxmox.com>
https://gitlab.com/x86-psABIs/x86-64-ABI/
https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg01592.html
"
In 2020, AMD, Intel, Red Hat, and SUSE worked together to define
three microarchitecture levels on top of the historical x86-64
baseline:
* x86-64: original x86_64 baseline instruction set
* x86-64-v2: vector instructions up to Streaming SIMD
Extensions 4.2 (SSE4.2) and Supplemental
Streaming SIMD Extensions 3 (SSSE3), the
POPCNT instruction, and CMPXCHG16B
* x86-64-v3: vector instructions up to AVX2, MOVBE,
and additional bit-manipulation instructions.
* x86-64-v4: vector instructions from some of the
AVX-512 variants.
"
This patch add new builtin model derivated from qemu64 model,
to be compatible between intel/amd.
mandatory flags from qemu-doc generator:
https://gitlab.com/qemu/qemu/-/blob/master/scripts/cpu-x86-uarch-abi.py
x86-64-v1 : I'm skipping it, as it's basicaly qemu64|kvm64 -vme,-cx16 for compat Opteron_G1 from 2004
so will use it as qemu64|kvm64 is higher are not working on opteron_g1 anyway
x86-64-v2 : Derived from qemu, +popcnt;+pni;+sse4.1;+sse4.2;+ssse3
min intel: Nehalem
min amd : Opteron_G3
x86-64-v2-AES : Derived from qemu, +aes;+popcnt;+pni;+sse4.1;+sse4.2;+ssse3
min intel: Westmere
min amd : Opteron_G3
x86-64-v3 : Derived from qemu64 +aes;+popcnt;+pni;+sse4.1;+sse4.2;+ssse3;+avx;+avx2;+bmi1;+bmi2;+f16c;+fma;+abm;+movbe+xsave
min intel: Haswell
min amd : EPYC_v1
x86-64-v4 : Derived from qemu64 +aes;+popcnt;+pni;+sse4.1;+sse4.2;+ssse3;+avx;+avx2;+bmi1;+bmi2;+f16c;+fma;+abm;+movbe;+xsave;+avx512f;+avx512bw;+avx512cd;+avx512dq;+avx512vl
restore: check bridge access when actual config is available
This was not only rather inefficient (getting the config from the
archive twice) but also wrong, as we can override options on restore,
so we can do the check only when the backed-up config and override
config got merged.
If this is to late from POV of volume deletion or the like, then the
issue is that those things happen to early, as we can only know what
to do with the actual target config, so destructive actions that
happen before that are wrong by design.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Fiona Ebner [Wed, 24 May 2023 13:56:50 +0000 (15:56 +0200)]
qmeventd: extract vmid from cgroup file instead of cmdline
This is the single remaining user of the id argument. The id argument
is a Proxmox-specific extension to QEMU, which we'd like to drop to
reduce our differences with upstream QEMU.
Fiona Ebner [Wed, 24 May 2023 13:56:49 +0000 (15:56 +0200)]
remove left-over mentions of to-be-dropped, outdated QMP commands
The commands snapshot-drive and delete-drive-snapshot have been unused
by qemu-server since commit eba2b721 ("use qemu's blockdev-snapshot
functions") and are now going to be dropped in our QEMU builds too, so
get rid of these left-overs.
Leo Nunner [Tue, 23 May 2023 07:14:19 +0000 (09:14 +0200)]
cloudinit: pass through hostname via fqdn field
If no FQDN is provided, we simply set it to the current hostname. This
ensures that the hostname *really* gets set, since we encountered an
issue on Fedora and CentOS based systems where no hostname got set at
all.
When there's no FQDN set in the cloudinit config, this leads to the
following entry:
Fiona Ebner [Tue, 30 May 2023 13:52:04 +0000 (15:52 +0200)]
fix #2315: api: have resize endpoint spawn a worker task
Similar to the corresponding endpoint for containers. Because disks
are involved, this can be a longer running operation, as is also
indicated by the 60 seconds timeout used in qemu_block_resize() which
is called by this endpoint.
Leo Nunner [Thu, 11 May 2023 08:03:17 +0000 (10:03 +0200)]
cloudinit: fix 'pending' api endpoint
This patch partially reverts commit 1b5706cd168fedc5e494e24300069ee4ff25761f,
by reintroducing the old format for return values (key, value, pending,
delete), but drops the "force-delete" return value. Right now, this
endpoint does not conform to its own format, because the return values
are as follows:
{
key => {
old => 'foo',
new => 'bar',
},
[…]
}
While the format specified is
[
{
key => 'baz',
old => 'foo',
new => 'bar',
},
[…]
]
This leads to the endpoint being broken when used through 'qm' and
'pvesh'. Using the API works fine, because the format doesn't get
verified there. Reverting this change brings the advantage that we can
also use PVE::GuestHelpers::format_pending when calling the endpoint
through qm again.
Leo Nunner [Thu, 4 May 2023 10:55:01 +0000 (12:55 +0200)]
fix #3428: cloudinit: add parameter for upgrade on boot
up until now, we did an automatic upgrade after the first boot in our
standard cloud-init config. This has been requested to be toggleable
several times [1][2]. With this patch, "package_upgrade" is disabled by
default, and needs to be enabled manually, diverging from the previous
behaviour.
these config keys only affect the cloudinit drive contents (and state of the
guest inside the VM), they are not used anywhere on the hypervisor side, so
they should not require VM.Config.Network (which allows a lot more, such as
changing vNIC VLAN tags or the bridges they are connected to).
block resize: avoid passing zero size to QMP command
Commit 7246e8f9 ("Set zero $size and continue if volume_resize()
returns false") mentions that this is needed for "some storages with
backing block devices to do online resize" and since this patch came
together [0] with pve-storage commit a4aee43 ("Fix RBD resize with
krbd option enabled."), it's safe to assume that RBD with krbd is
meant. But it should be the same situation for any external plugin
relying on the same behavior.
Other storages backed by block devices like LVM(-thin) and ZFS return
1 and the new size respectively, and the code is older than the above
mentioned commits. So really, the RBD plugin just should have returned
a positive value to be in-line with those and there should be no need
to pass 0 to the block_resize QMP command either.
Actually, it's a hack, because the block_resize QMP command does not
actually do special handling for the value 0. It's just that in the
case of a block device, QEMU won't try to resize it (and not fail for
shrinkage). But the size in the raw driver's BlockDriverState is
temporarily set to 0 (which is not nice), until the sector count is
refreshed, where raw_co_getlength is called, which queries the new
size and sets the size in the raw driver's BlockDriverState again as a
side effect. It's not known to cause any issues, but bdrv_getlength is
a coroutine wrapper starting from QEMU 8.0.0, and it's just better to
avoid setting a completely wrong value even temporarily. Just pass the
actually requested size like is done for LVM(thin) and ZFS.
Fiona Ebner [Mon, 22 May 2023 12:37:08 +0000 (14:37 +0200)]
tests: fix invoking migration tests with make
Even if between single quotes, the dollar sign needs to be escaped
here. Otherwise, there will be an error
> Search pattern not terminated at -e line 1.
and no migration tests would be run. The error did not lead to
aborting though, making it harder to notice.
Fixes: aac89f6c ("tests: avoid calling test script to get target names") Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>