Commit Graph

1448 Commits

Author SHA1 Message Date
Fiona Ebner
219719aada foreach volid helper: make $pending parameter behave like a boolean
Avoids potential future mistake with passing in an explicit 0.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-06-21 12:48:11 +02:00
Aaron Lauterer
6e9c4929be 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.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
2023-06-21 12:48:11 +02:00
Aaron Lauterer
0b7a0b78db 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.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
2023-06-21 12:48:11 +02:00
Aaron Lauterer
6328c554c1 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

[0] https://lists.proxmox.com/pipermail/pve-devel/2023-May/056868.html

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
2023-06-21 12:48:11 +02:00
Fiona Ebner
ec11b92abb 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>
2023-06-21 12:40:58 +02:00
Fiona Ebner
178c355ded schema: cloudinit: document default for ciupgrade
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-06-21 12:40:58 +02:00
Thomas Lamprecht
3d79cf5536 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)

[0]: https://lists.proxmox.com/pipermail/pve-devel/2023-June/057770.html

Originally-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-21 09:14:47 +02:00
Thomas Lamprecht
728404c03d vm start: factor out silencing systemd stop-scope command
will be reused in the next commit

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-21 09:13:04 +02:00
Fabian Grünbichler
621edb2b65 restore: extend permissions checks
to allow early checking of the merged config, if the backup archive
passed in is a proper volume where extraction is possible.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-06-20 19:42:48 +02:00
Dominik Csapak
a52eb3c4e9 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>
2023-06-19 07:21:07 +02:00
Dominik Csapak
9b71c34d61 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>
2023-06-16 16:24:02 +02:00
Dominik Csapak
e3971865b4 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>
2023-06-16 16:24:02 +02:00
Dominik Csapak
0cf8d56c6d 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>
2023-06-16 16:24:02 +02:00
Fiona Ebner
5854213953 status: fix description of qmpstatus property
Using the word 'agent' is highly confusing here as there is no QMP
agent and thus wrongly suggests that the value is related to the
guest agent[0].

[0]: https://forum.proxmox.com/threads/123590/post-537716

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-06-14 13:33:56 +02:00
Thomas Lamprecht
951714ea92 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>
2023-06-08 17:50:50 +02:00
Thomas Lamprecht
d6deb7f6bb move helper to check bridge access out of api
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-08 17:50:50 +02:00
Fiona Ebner
d62bdac593 fast plug options: add migrate_downtime and migrate_speed
for convenience. These options do not influence the QEMU instance
directly, but are only used for migration, so no need to keep them in
pending.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-06-07 18:37:51 +02:00
Fiona Ebner
f68910a05f fast plug options: order alphabetically
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-06-07 18:37:51 +02:00
Leo Nunner
efa3355d3b 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.

[1] https://forum.proxmox.com/threads/how-to-prevent-automatic-apt-upgrade-during-the-first-boot-with-cloud-init.68472/
[2] https://forum.proxmox.com/threads/cloud-init-ohne-package-upgrade.123841/

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
2023-06-07 18:25:46 +02:00
Fiona Ebner
bda7ccb1c9 schema: avoid using deprecated -no-hpet in example for 'args' property
instead use a recent example that served as a workaround in #4625.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 17:35:41 +02:00
Fiona Ebner
17bacc2182 cfg2cmd: replace deprecated no-hpet option with hpet=off machine flag
like the deprecation message printed by QEMU suggests.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 17:35:41 +02:00
Fiona Ebner
0f704640be cfg2cmd: replace deprecated no-acpi option with acpi=off machine flag
like the deprecation message printed by QEMU suggests.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 17:35:41 +02:00
Fiona Ebner
e35eb8766b cfg2cmd: use actual backend names instead of removed tty and paraport aliases
As described in:
https://qemu-project.gitlab.io/qemu/about/removed-features.html#chardev-backend-aliases-tty-and-parport-removed-in-8-0

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-06-07 17:35:41 +02:00
Fiona Ebner
2e4357c537 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.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2017-January/025060.html

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-06-06 19:42:16 +02:00
Christian Ebner
bb547dcbd6 net: Skip and warn of interfaces without bridge
Handle and warn about network interfaces which are not attached to
any bridge because the user actively removed it from the VM config.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
2023-06-06 18:26:27 +02:00
Fiona Ebner
e4263214b8 disable SMM check: always return false for virt machine type
There is no 'smm' flag for the 'virt' machine type.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-05-15 11:09:33 +02:00
Fiona Ebner
efa3aa2496 avoid list context for volume_size_info calls
With the recent pve-storage commit d70d814 ("api: fix get content call response
type for RBD/ZFS/iSCSI volumes"), the volume_size_info call for RBD in
list context is much slower than before (from a quick test, about twice as long
without snapshots, even longer with snapshots and untested, but when using an
external cluster with image not having the fast-diff feature, it should be worse
still) and thus increases the likelihood to run into timeouts here.

None of the callers here actually need the more expensive call, so just
avoid calling in list context.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-03-21 10:37:40 +01:00
Dominik Csapak
49c51a60db pci: workaround nvidia driver issue on mdev cleanup
in some nvidia grid drivers (e.g. 14.4 and 15.x), their kernel module
tries to clean up the mdev device when the vm is shutdown and if it
cannot do that (e.g. becaues we already cleaned it up), their removal
process cancels with an error such that the vgpu does still exist inside
their book-keeping, but can't be used/recreated/freed until a reboot.

since there seems no obvious way to detect if thats the case besides
either parsing dmesg (which is racy), or the nvidia kernel module
version(which i'd rather not do), we simply test the pci device vendor
for nvidia and add a 10s sleep. that should give the driver enough time
to clean up and we will not find the path anymore and skip the cleanup.

This way, it works with both the newer and older versions of the driver
(some of the older drivers are LTS releases, so they're still
supported).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2023-03-16 09:08:34 +01:00
Dominik Csapak
c8bd54e9a8 fix #4553: nvidia vgpu: reuse smbios uuid for '-uuid' parameter
instead of using the mdev uuid. The nvidia driver does not actually care
that it's the same as the mdev, and in qemu the uuid parameter
overwrites the smbios1 uuid internally, so we should have been reusing
that in the first place.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2023-03-16 09:08:27 +01:00
Fiona Ebner
79f5ca393a clone: remove outdated TODO about bandwidth limit
Respecting bandwidth limit for offline clone was implemented by commit
56d16f16 ("fix #4249: make image clone or conversion respect bandwidth
limit"). It's still not respected for EFI disks, but those are small,
so just ignore it.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-24 08:56:28 +01:00
Leo Nunner
56d16f169c fix #4249: make image clone or conversion respect bandwidth limit
Previously, cloning a stopped VM didn't respect bwlimit. Passing the -r
(ratelimit) parameter to qemu-img convert fixes this issue.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
 [ T: reword subject line slightly ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-02-23 17:09:51 +01:00
Alexandre Derumier
62fdcfd4cf update network dev: MTU is not hot-pluggable
Avoid pretending that a MTU change on a existing network device gets
applied live to a running VM.

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
 [ T: reworded and expanded commit message slightly ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-02-23 16:37:34 +01:00
Christoph Heiss
93e21fd230 vzdump: Add VM QGA option to skip fs-freeze/-thaw on backup
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
2023-02-23 16:34:10 +01:00
Matthias Heiserer
1183c8f1a0 ovmf efi disk: ignore efitype parameter for ARM VMs
Required because there's one single efi for ARM, and the 2m/4m
difference doesn't seem to apply.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
 [ T: move description to format and reword subject ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-02-23 16:29:57 +01:00
Fiona Ebner
72a5a17610 swtpm: use start time as prefix for logging
to be able to distinguish different invocations.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-23 12:53:25 +01:00
Fiona Ebner
b2e9c4d322 swtpm: enable logging
AFAICT, previously, errors from swtpm would not show up in any logs,
because they were just printed to the stderr of the daemonized
invocation here.

The 'truncate' option is not used, so that the log is not immediately
lost when a new instance is started. This increases the chance that
the relevant errors are still present when requesting the log from a
user.

Log level 1 contains the most relevant errors and seems to be quiet
for working-as-expected invocations. Log level 2 already includes
logging full TPM commands, some of which are 1024 bytes long. Thus,
log level 1 was chosen.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-23 12:53:25 +01:00
Fiona Ebner
cb64a64339 start: make not being able to set polling interval for ballooning non-critical
The guest will be running, so it's misleading to fail the start task
here. Also ensures that we clean up the hibernation state upon resume
even if there is an error here, which did not happen previously[0].

[0]: https://forum.proxmox.com/threads/123159/

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-23 11:42:40 +01:00
Alexandre Derumier
dafb728cb8 memory: remove calls to parse_hotplug_features
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
[FE: style: avoid overly long lines]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-15 14:34:25 +01:00
Fiona Ebner
8fbae1dc8f fix #4525: clone disk: disallow mirror if it might cause problems with io_uring
The target of the drive-mirror operation is opened with (essentially)
the same flags as the source in QEMU, in particular whether io_uring
should be used is inherited.

But io_uring currently causes problems in combination with certain
storage types, sometimes even leading to crashes (LVM with Linux 6.1).
Just disallow live cloning of drives when the source uses io_uring and
the target storage is not ready for it. There is one exception, namely
when source and target storage are the same. In that case, just assume
it will keep working for the target.

Migration does not seem to be affected, because there, the target VM
opens the images with the checked aio setting and then NBD exports of
those are used as the targets for mirroring.

It can be that the default determined for the source is not what's
actually used, because after a drive-mirror to a storage with a
different default, it will still use the default from the old storage.
Unfortunately, aio doesn't seem to be part of the 'query-block' QMP
command's result, so just tolerate this edge case.

The check can be removed if either
1. drive-mirror learns to open the target with a different aio setting
or more ideally
2. there are no more bad storages for io_uring.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-14 10:09:04 +01:00
Fiona Ebner
b7071d6c00 drive commandline: factor out determining direct cache usage into helper
In preparation to re-use it for a check for live disk cloning.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-14 10:09:04 +01:00
Fiona Ebner
eec9f9fe00 drive commandline: factor out checks if io_uring is allowed by default
while getting rid of the double negation.

In preparation to re-use the check for live disk cloning.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-14 10:09:04 +01:00
Fiona Ebner
ea7c3b39b2 hotplug: disk: mark aio as non-hotpluggable
Previously, changing aio would be applied to the configuration, but
the drive would still be using the old setting.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-02-14 10:09:04 +01:00
Alexandre Derumier
39c074fe23 qemu_memory_hotplug: remove unused $opt arg
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
2023-02-03 11:23:47 +01:00
Thomas Lamprecht
ea0bc51427 schema: OS type: note that the l26 type is also compatible with Linux 6.x
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-01-30 11:02:53 +01:00
Fiona Ebner
252e2624dc schema: memory: be precise that unit is binary prefix
In the web UI, this was fixed years ago by pve-manager commit c11c4a40
("fix #1631: change units to binary prefix").

Quickly checked with the 'query-memory-size-summary' QMP command that
this is actually the case.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-01-24 17:09:52 +01:00
Thomas Lamprecht
eba285f594 cloud init schema: fix indentation and overly long wording
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-01-19 11:57:06 +01:00
Noel Ullreich
f78c9b6b44 fix #4378: standardized error for ovmf files
The error messages for missing OVMF_CODE and OVMF_VARS files were
inconsistent as well as the error for the missing base var file not
telling you the expected path.

Signed-off-by: Noel Ullreich <n.ullreich@proxmox.com>
2023-01-18 11:03:11 +01:00
Thomas Lamprecht
4044ae1fc3 vm start: fix code style for balloon enabling
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-01-16 13:46:29 +01:00
Fiona Ebner
83f04be3d5 migration: nbd export: switch away from deprecated QMP command
The 'nbd-server-add' QMP command has been deprecated since QEMU 5.2 in
favor of a more general 'block-export-add'.

When using 'nbd-server-add', QEMU internally converts the parameters
and calls blk_exp_add() which is also used by 'block-export-add'. It
does one more thing, namely calling nbd_export_set_on_eject_blk() to
auto-remove the export from the server when the backing drive goes
away. But that behavior is not needed in our case, stopping the NBD
server removes the exports anyways.

It was checked with a debugger that the parameters to blk_exp_add()
are still the same after this change. Well, the block node names are
autogenerated and not consistent across invocations.

The alternative to using 'query-block' would be specifying a
predictable 'node-name' for our '-drive' commandline. It's not that
difficult for this use case, but in general one needs to be careful
(e.g. it can't be specified for an empty CD drive, but would need to
be set when inserting a CD later). Querying the actual 'node-name'
seemed a bit more future-proof.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-01-13 14:04:39 +01:00
Thomas Lamprecht
c3d151080a cdrom path: code reduction
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2023-01-13 13:52:39 +01:00
Stefan Sterz
490b730854 cd rom handling: refactor cd rom path helper function
to stop returning results of assignments

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
2023-01-13 13:20:35 +01:00
Stefan Sterz
259470ee41 cd rom handling: return a clearer error when there is no cd rom drive
when a vm is configured to use a physical cd rom drive but there is no
such drive a cryptic "uninitialized value" error is thrown. this is
due to `$path` being undefined in `sub print_drive_commandline_full`.
warn that no cd rom drive is available instead.

note that the error was cosmetic as the vm would start just fine.

forum thread: https://forum.proxmox.com/threads/119592/

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
2023-01-13 13:20:35 +01:00
Stefan Hanreich
30fdf99cff fix #4358: destroy_vm: Ignore 'suspended' lock when destroying VM
Since we can now differentiate between 'suspended' and 'suspending',
it is possible to ignore the 'suspended' lock when destroying a VM.
It shouldn't matter whether the VM is locked because of hibernation
when you want to remove it. Therefore we can safely ignore the lock.
2023-01-11 10:59:32 +01:00
Fiona Ebner
b3a3e92962 fix #4435: devices list: avoid error for undefined value
When $d->{'pci_bridge'}->{devices} is undef, @-dereferencing it will
die with:
> Can't use an undefined value as an ARRAY reference

This can happen (at least) when the VM is in 'prelaunch' state. The
QAPI definition for '@PciBridgeInfo' also declares the 'devices'
member as optional.

Before commit 721624b ("collect device list for nested pci-bridges"),
there was no issue, because $d->{'pci_bridge'}->{devices} was used in
foreach, so auto-vivified if undef.

Fixes: f721624b ("collect device list for nested pci-bridges")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-01-11 10:14:44 +01:00
Fiona Ebner
7bd9abd243 tree-wide: switch to official spelling of QEMU in descriptions/messages
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-12-20 10:26:41 +01:00
Thomas Lamprecht
3d07669cf1 ovmf cmd assembly: rework now that it's in a separate method
We can now do a few things that would be not really possible, or at
least mess with readability when this was still mostly inline
config2command, shaves of quite a few lines of code.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-12-12 11:43:19 +01:00
Thomas Lamprecht
2ceb59d4b1 ovmf cmd assembly: reorder arguments
in preparation of reworking the new separate method for OVMF cmd
assembly, do this in a separate very targeted commit to make it more
clear that the next reworking-commit doesn't messes with our tests at
all.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-12-12 11:41:50 +01:00
Fiona Ebner
b7d80c7905 cfg2cmd: factor out ovmf drives printing
No functional change is intended.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-12-06 16:56:30 +01:00
Fabian Grünbichler
ad9e347c46 fix #4372: fix vm_resume migration callback
the fix for the recently introduced requirement of loading the VM config while
migrating was incomplete, since the vmlist node value could already be out of
date by the time load_config is called.

extend the fallback behaviour even further, by doing the following sequence:
- try regular load_config (likely case, rename already fully processed)
- if it fails, get node from vmlist, and load_config using that
- it that fails, invalidate the PVE::Cluster cache, retry regular load_config

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-11-30 16:21:39 +01:00
Fabian Grünbichler
a20dc58a1b explain 'nocheck' in more places
was only explained in git history and vm_stop, add comments in other
relevant places to avoid future breakage.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-11-21 13:42:52 +01:00
Fabian Grünbichler
270bfff2e1 vm_resume: fix nocheck/migrate handling
it's not deterministic whether the rename/move of the VM config
triggered on the source side of a migration is already visible on the
target side when vm_resume is executed. check the vmlist for the node
where the config is currently located if $nocheck is set - it is now
needed to add the forwarding DB entries to the bridge.

this fixes an issue on busier or slower clusters, where pmxcfs hasn't
yet processed the rename, and resuming would fail with an error about
the config not existing.

Reported-by: Dominik Csapak <d.csapak@proxmox.com>

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-11-21 13:42:52 +01:00
Thomas Lamprecht
fe62da4f97 fdb: only manage FDB entries for Linux bridges for now
we need to handle OVS setups differently, so for now just ignore it
there (behavior as it was in 7.2)

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-20 16:03:45 +01:00
Thomas Lamprecht
f81c9843c3 fdb: pull out bridge variable
no semantic change intended

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-20 16:03:17 +01:00
Fabian Grünbichler
eef93bc590 migrate: add remote migration handling
remote migration uses a websocket connection to a task worker running on
the target node instead of commands via SSH to control the migration.
this websocket tunnel is started earlier than the SSH tunnel, and allows
adding UNIX-socket forwarding over additional websocket connections
on-demand.

the main differences to regular intra-cluster migration are:
- source VM config and disks are only removed upon request via --delete
- shared storages are treated like local storages, since we can't
assume they are shared across clusters (with potentical to extend this
by marking storages as shared)
- NBD migrated disks are explicitly pre-allocated on the target node via
tunnel command before starting the target VM instance
- in addition to storages, network bridges and the VMID itself is
transformed via a user defined mapping
- all commands and migration data streams are sent via a WS tunnel proxy
- pending changes and snapshots are discarded on the target side (for
  the time being)

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-11-17 15:21:39 +01:00
Fabian Grünbichler
05b2a4ae9c migrate: refactor remote VM/tunnel start
no semantic changes intended, except for:
- no longer passing the main migration UNIX socket to SSH twice for
forwarding
- dropping the 'unix:' prefix in start_remote_tunnel's timeout error message

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-11-17 15:21:39 +01:00
Fabian Grünbichler
7a24c98af6 pending: fix typo in variable name
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-11-17 15:21:39 +01:00
Fabian Grünbichler
e97bbbb64d pending changes: allow skipping cloud-init
in case of remote migration, we use the `update_vm_api` helper for
checking permissions on the incoming config. this would also cause an
incoming cloud-init image to be overwritten, since the VM is not running
yet at this point.

provide a parameter which can be set by an incoming *remote* migration
to avoid having inconsistent cloud init images on the source and target
side.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-11-17 15:21:39 +01:00
Wolfgang Bumiller
9660e606ee fixup delayed cloudinit hotplug
cloudinit generation needs to see the cloudinit drive so we
need to pass a config with it already updated

Fixes: 4b785da1a9 ("delay cloudinit generation in hotplug")
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-11-17 15:16:21 +01:00
Wolfgang Bumiller
a540985120 rework cloudinit_record_changed logic
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-11-17 14:35:27 +01:00
Thomas Lamprecht
c229961ad3 parse config: do not validate informative values in cloud init section
Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-17 13:19:19 +01:00
Wolfgang Bumiller
d29483147d don't call 'cleanup_config' the cloudinit section
It performs schema valdiation (and normalization).

We only ever write values into it which came from an
already validated config, and we also add an additional
"added" key which is not covered by the schema, so this
would fail.

Simply skip it.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-11-16 18:17:07 +01:00
Wolfgang Bumiller
f16cf6c37d record cloud-init changes in the cloudinit section
introducing an 'added' value in the cloudinit section for
values which have not been present when the cloudinit image
has been generated

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-11-16 18:17:07 +01:00
Wolfgang Bumiller
4b785da1a9 delay cloudinit generation in hotplug
Hotpluggieg generated a cloudinit image based on old values
in order to attach the device and later update it again, but
the update was only done if cloudinit hotplug was enabled.
This is weird, let's not.

Also introduce 'apply_cloudinit_config' which also write the
config, which, as it turns out, is the only thing we
actually need anyway, currently.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-11-16 18:17:07 +01:00
Wolfgang Bumiller
3de134ef4a Revert "cloudinit: avoid unsafe write of VM config"
This reverts commit b137c30c3a.

In preparation of fixing the special:cloudinig section.
2022-11-16 18:16:56 +01:00
Leo Nunner
e0e036e1ba fix #4284: add read-only to non-hotpluggable disk options
Changing the read-only status of a disk is not possible through QMP, so
it needs to be exempt from the hotpluggable values as to notify the
user.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
2022-11-16 13:16:53 +01:00
Thomas Lamprecht
1e1d6f589c write config: only write out special cloudinit config if the vm has it
this is only for the current in use CI stuff, not the actual cloud
init config itself.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-16 12:50:58 +01:00
Alexandre Derumier
6622226553 net: increase max queues to 64
max supported queues tx + rx = 256, so 128 for combined
https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg03917.html

But from above link it also seems that x86 only supports 80 pairs in
practice, so for now "only" quadruple the limit to 64 and see if we
get user feedback for more requested.

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
 [ T: reduce from 128 to 64 and add short rationale for that ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-16 12:14:46 +01:00
Thomas Lamprecht
cf364f9574 indentation fix
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-16 12:08:05 +01:00
Thomas Lamprecht
b137c30c3a cloudinit: avoid unsafe write of VM config
there's no guarantee that we're locked here and it also produces
unnecessary extra IO in most cases.

While at it also avoid that a special:cloudinit section is added on
start to *every* VM, which caused another bug to trigger (see prev.
commit) and is just odd for users that ain't using cloudinit

Note in two call sites that we may need to write the config indeed
out there on the caller side.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-16 12:03:53 +01:00
Thomas Lamprecht
eb9923f9b9 config: fix dropping description on parsing special cloud init section
we now always write out a new clouding special section on start (to
be fixed) independent of any cloudinit drive/config configured or
not, and thus always run into that section after a VM started with
the new qemu-server installed, which in turn set the description
always to undef.

Fixes: 95a5135 ("cloudinit: add cloudinit section for current generated config.")
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-16 11:41:34 +01:00
Thomas Lamprecht
cbfc9d753f parse config: factor out finishing reading the description comment
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-16 11:23:01 +01:00
Thomas Lamprecht
74fe3d9a7b config to command: avoid line bloat, keep cmd definition near initial pushes
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-15 08:34:47 +01:00
Thomas Lamprecht
326704e73f affinity: add actual example to description
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-15 08:33:04 +01:00
Thomas Lamprecht
1a67f99959 add fixme comment to replace duplicate nodename cache
that function also caches the value, and it recently was changed to
be importable, so we can just import and drop this once a new enough
pve-common is available.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-15 07:27:12 +01:00
Alexandre Derumier
620d6b328f virtio-net: increase defaults rx|tx-queue-size to 1024
This is reducing packet drop on high pps, and also needed for dpdk.

Redhat already have use it by default in rhev and his openstack platform too
since 2019.

I'm using it in production since 6 months, I don't have seen performance regression.

fix: (which ask for custom option, but setting it by default seem fine for me)

https://bugzilla.proxmox.com/show_bug.cgi?id=1546
https://bugzilla.proxmox.com/show_bug.cgi?id=2349
Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
2022-11-13 16:42:23 +01:00
Alexandre Derumier
0c03a39035 fix #4296: virtio-net: enable packed queues for qemu 7.1
virtio 1.1 have improve virtio multiqueue performance,
with a new implementation called "packed queues".

https://www.redhat.com/en/blog/packed-virtqueue-how-reduce-overhead-virtio
https://archive.fosdem.org/2018/schedule/event/virtio/attachments/slides/2167/export/events/attachments/virtio/slides/2167/fosdem_virtio1_1.pdf

This patch enable it by default for qemu 7.1

This don't break old guests with old virtio 1.0 drivers,
as virtio device/devices are forward/backward compatible.

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
2022-11-13 16:42:00 +01:00
Alexandre Derumier
73ed64967e migration : add del_nets_bridge_fdb
at the end of a live migration, we need to remove old mac entries
on source host (vm is not yet stopped), before resume vm on target host

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
 [T: resolve conflicts and rework on apply ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-13 14:56:57 +01:00
Thomas Lamprecht
4ddd2ca293 net devs: avoid registering MAC to fdb if not static
In theory we can have a config with netX records that do not specify
a `macaddr` property, we just auto-generate on in config2cmd for
startup transitively, but don't save that explicitly back to the
config; so while we could parse the /proc/$pid/cmdline or try to get
the info from QMP (not fully straight forward) it seems rather a
hassle; especially if one has in mind that this cannot happen via the
API FWICT; as there a "deletion" *saves* a newly auto generated value
out to the config, same with clone of a VM and restore of a backup.

So, in basically all reasonable cases we got the `macaddr` available,
but if we don't it makes no sense to add a FDB variable for a *newly*
generated one by the parse_net call, as the VM won't use that (well,
at least if one doesn't get "lucky" and it randomly re-generates the
same as on startup), so allow telling parse_net to skip auto
generating MACs and use that in the add-fdb-entries helper

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-13 14:56:57 +01:00
Thomas Lamprecht
1b5ba4ddc6 net devs: code cleanup new fdb mac add helper
reduce a level of indentation and modernize slightly

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-13 14:56:57 +01:00
Alexandre Derumier
21947fea42 net devs: register vNIC mac to FDB on start/resume
On plain VM start (no live migration), we can simply add MAC address
into the fdb. In case of a live migration, we add the mac address
just before the resume.

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
2022-11-13 14:55:26 +01:00
Dominik Csapak
c60cad61a0 fix #3271: USB: allow usb hotplugging for modern guests
same as with the extended support for more usb devices, allow
hotplugging for guests that can use the qemu-xhci controller which
require a machine type >= 7.1 and a ostype l26 or windows > 7

if no usb device was passed through on startup, dynamically add
the xhci controller (and remove if the last usb device is unplugged)
so that live migration is still possible

much of the usb hotplug code was already there, but it still needed
a few adaptions, for example we have to add a chardev when adding
a spice redir port (that gets automatically removed when the
usb-redir device gets removed)

since the spice devices use the id 'usbredirdevX' instead of 'usbX', we
have to manually map that a bit around

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2022-11-10 17:02:34 +01:00
Dominik Csapak
0c3d18ef13 USB: increase max usb devices to 14 for newer machine version and ostype
for machine versions >= 7.1 and ostype linux or windows > 7, we use the
qemu-xhci controller where we have up to 14 usable ports, so make them
available to the user

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2022-11-10 17:02:34 +01:00
Dominik Csapak
4862922a2b fix #4324: USB: use qemu-xhci for machine versions >= 7.1
going by reports in the forum (e.g. [0]) and semi-official qemu
information[1], we should prefer qemu-xhci over nec-usb-xhci

for compatibility purposes, we guard that behind the machine version,
so that guests with a fixed version don't suddenly have a different usb
controller after a reboot (which could potentially break some hardcoded
guest configs)

0: https://forum.proxmox.com/threads/proxmox-usb-connect-disconnect-loop.117063/
1: https://www.kraxel.org/blog/2018/08/qemu-usb-tips/

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2022-11-10 17:02:34 +01:00
Dominik Csapak
238af88edc move 'windows_version' to Helpers
to avoid a cyclic dependency when we want to use that in PVE::QemuServer::USB

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2022-11-10 17:02:34 +01:00
Dominik Csapak
2b938c7d88 print_tabletdevice_full: make use of $q35 variable
just outside of context, we already save the result from
machine_type_is_q35 into the $q35 variable, but never use it.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2022-11-10 17:02:34 +01:00
Thomas Lamprecht
faf72d6cbf pci: cleanup pci: unregister mdev directly inline
not worth the hassle of a break/depends cycle

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-10 17:02:34 +01:00
Wolfgang Bumiller
c963efc882 use full path for /usr/bin/taskset
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-11-10 11:15:43 +01:00
Daniel Bowder
8387039819 fix #3593: add affinity to qemu
Reuse the PVE::CpuSet to validate cpuset formatting.
Add new qemu property called 'affinity' to store the cpuset.
Push taskset command in front of kvm if 'affinity' is set.

Signed-off-by: Daniel Bowder <daniel@bowdernet.com>
2022-11-10 09:39:28 +01:00
Dominik Csapak
1b189121fc vm start/stop: cleanup passed-through pci devices in more situations
if the preparing of PCI devices or the start of the VM fails, we need
to cleanup the PCI devices (reservations *and* mdevs), or else it
might happen that there are leftovers which must be manually removed.

to include also mdevs now, refactor the cleanup code from
'vm_stop_cleanup' into it's own function, and call that instead of
only 'remove_pci_reservation'

also simplifies the code, such that it now removes all PCI ids
reserved for that VMID, since we cannot have multiple VMs with the
same VMID anyway

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-09 08:49:45 +01:00
Matthias Heiserer
d80ad18c67 fix #3890 - GUI: warn for unlikely iothread config
Previously, only a plaintext line in the task log showed something was off.
Now, the GUI will show it as a warning.

Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
2022-11-08 17:49:51 +01:00