Commit Graph

532 Commits

Author SHA1 Message Date
Fabian Ebner
0d2db08414 prefer storage_check_enabled over storage_check_node
storage_check_enabled simply checks for the 'disable' option and then calls
storage_check_node.

While not strictly necessary for a second call where only the storage differs,
e.g. in case of clone, it is more future-proof: if support for a target storage
is added at some point, it might be easy to miss adapting the call.

For the migration checks, the situation is improved by now always catching
disabled (target) storages.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2021-06-21 11:17:48 +02:00
Fabian Ebner
16e66777a0 vm destroy: do not remove unreferenced disks by default
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Reviewed-by: Stefan Reiter <s.reiter@proxmox.com>
2021-06-08 14:56:39 +02:00
Lorenz Stechauner
1cb23b87b4 api: clone: sort vm disks to keep numbers consistent
reported by user in forum:
https://forum.proxmox.com/threads/problem-when-copying-template-with-2-discs.89851/

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
2021-05-26 17:32:52 +02:00
Thomas Lamprecht
b973806ef1 api: restore: better error messages
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-04-06 10:20:26 +02:00
Thomas Lamprecht
a0e27afb5e api: restore: start and live-restore do not conflict
if live-restore is set then the VM is actually started before, so we
can just skip it..

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-04-06 10:12:49 +02:00
Stefan Reiter
26731a3c15 enable live-restore for PBS
Enables live-restore functionality using the 'alloc-track' QEMU driver.
This allows starting a VM immediately when restoring from a PBS
snapshot. The snapshot is mounted into the VM, so it can boot from that,
while guest reads and a 'block-stream' job handle the restore in the
background.

If an error occurs, the VM is deleted and all data written during the
restore is lost.

The VM remains locked during the restore, which automatically prohibits
any modifications to the config while restoring. Some modifications
might potentially be safe, however, this is experimental enough that I
believe this would cause more bad stuff(tm) than actually satisfy any
use cases.

Pool handling is slightly adjusted so the VM can be added to the pool
before the restore starts.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
2021-04-01 10:58:13 +02:00
Fabian Ebner
bd61033e30 api: migrate: fix variable name
Commit abff03211f switched to iterating over the
values instead of the keys, but didn't update the variable name. Use target_sid,
because target is already in use for the target node.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2021-03-24 06:51:40 +01:00
Thomas Lamprecht
0761e6194a improve windows VM version pinning on VM creation
unify code paths to ensure more consistent behavior, especially on
future changes.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-03-12 10:00:46 +01:00
Fabian Ebner
0761ee013f api: create_vm: check serial and usb permissions
The existing check_vm_modify_config_perm doesn't do so anymore, but
the check only got re-added to the modify/delete paths. See commits
165be267eb and
e30f75c571 for context.

In the future, it might make sense to generalise the
check_vm_modify_config_perm and have it not only take keys, but both
new and old values, and use that generalised function everywhere.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-03-05 21:15:23 +01:00
Thomas Lamprecht
4dd1e83c75 always pin windows VMs to a machine version by default
A fix for violating a important standard for booting[0] in recently
packaged QEMU 5.2 surfaced some issues with Windows based VMs in our
forum[1], which seem to be quite sensitive for such changes (it seems
they derive lots of their device assignment from ACPI).
User visible effects are loss of any network configuration due to
windows thinking it was swapped with a new one, and starts with a
fresh config - this is mostly problematic for setups with static
address assignment.

There may be lots of other, more subtle, effects and the PVE admin is
also not always the VM admin, so we really need to avoid such
negative effects. Do this by pinning the version of any windows based
VMs to either the minimum of (5.1, kvm-version) for existing VMs or
the kvm-version at time of VM creation for new ones.

There are patches in pve-manager for user to be able to change the
pinned version themself in the webinterface, so this can now also get
adapted more easily if there surface any other issues (with new or
old version) in the future.

0: https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
1: https://forum.proxmox.com/threads/warning-latest-patch-just-broke-all-my-windows-vms-6-3-4-patch-inside.84915/page-2#post-373331

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-03-05 20:46:46 +01:00
Aaron Lauterer
ee9255601e API: update_vm_api: check for CDROM on disk delete
Since CDRoms and disks share the same config keys, we need to check if
it actually is a CDRom and then check the permissions accordingly.

Otherwise it is possible for someone without VM.Config.CDROM
permissions, but with VM.Config.Disk permissions to remove a CD drive
while being unable to create a CDRom drive.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
2021-02-22 17:06:10 +01:00
Dominik Csapak
b08c37c363 fix #2788: do not resume vms after backup if they were paused before
by checking if the vm is paused at the beginning and skipping the
resume now we also skip the qga freeze/thaw (which cannot work if the
vm is paused)

moved the 'vm_is_paused' sub from the api to PVE/QemuServer.pm so it
is available everywhere we need it.

since a suspend backup would pause the vm anyway, we can skip that
step also

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Fabian Ebner <f.ebner@proxmox.com>
2021-01-26 18:41:11 +01:00
Thomas Lamprecht
99676a6c1a api: destroy VM: fixup parameter description language
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-01-26 18:40:20 +01:00
Thomas Lamprecht
47f9f50b75 style fix: missing trailing comma
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-01-25 16:24:52 +01:00
Thomas Lamprecht
7585466269 vm destroy: allow opt-out of purging unreferenced disks
Since an old change released with a version bump on 2009-09-07, we
search all enabled storages for VMID maching volumes on VM removal
and purge those too.

This has multiple pitfalls and may be quite unexpected for some
users.

It can make problems when:
* on recovery a VM is created, before disks are reattached the admin
  notices some settings issues and chooses to just recreate the VM;
  but during destroying the dummy VM all related disks get destroyed
  unconditionally which may result in data loss. This actually
  happened and is the original reason for the decision to change
  this.

* a storage is shared between PVE instance (between a set of clusters
  and/or single nodes), while this is against our rules it may still
  come as a surprise if destroying a VM on node A may destroy
  unrelated and unreferenced disks on the unrelated node B without
  asking or allowing to avoid that.

As this the removal of matching but unreferenced disks can result in
permanent data loss (up to the last backup) and may be to subtle and
unforgiving, allow to opt-out of it.

In the long run we want to make this opt-in, but that is an API
change and so needs to wait for next major release. But, we can adapt
the GUI already to make it opt-in there, catching most of the cases.

side-note: CT do not have this behavior at all

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2021-01-25 15:22:46 +01:00
Thomas Lamprecht
e00319af4d api: adapt VM destroy description
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-12-15 14:49:41 +01:00
Dominic Jäger
3eecc92525 qm destroy: Extend --purge description
Add replication jobs & HA. This makes the enumeration complete.

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
2020-12-15 14:46:14 +01:00
Thomas Lamprecht
1b987638a8 api: cleanup code format of clone_disk call
showing off it's monstrosity of a method signature, needs to be
cleaned up in a followup commit

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-11-25 14:18:23 +01:00
Thomas Lamprecht
a2af1bbe89 add and use get_qga_key
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-11-25 14:18:23 +01:00
Fabian Ebner
68980d6626 Repeat check for replication target in locked section
No need to warn twice, so the warning from the outside check
was removed.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-11-09 10:08:22 +01:00
Thomas Lamprecht
009cbf61bc api: factor out common vm_is_paused code in private helper
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-10-16 17:51:24 +02:00
Thomas Lamprecht
11c601e9ad api: fix conditional variable declaration
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-10-16 17:51:05 +02:00
Thomas Lamprecht
d1c1af4b02 tree wide cleanup of s/return undef/return/
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-10-16 16:20:05 +02:00
Mira Limbeck
f8c4b2c5ba fix VM clone from snapshot with cloudinit disk
All volumes contained in $vollist are activated. In this case a snapshot
of the volume. For cloudinit disks no snapshots are created so don't add
it to the list of volumes to activate as it otherwise fails with no
logical volume found.

Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
2020-10-16 15:39:43 +02:00
Stefan Reiter
078c109fae api: add handling for new boot order format
The API is updated to handle the deprecation correctly, i.e. when
updating the 'order' attribute, the old 'legacy' (default_key) values
are removed (would now be ignored anyway).

When removing a device that is in the bootorder list, it will be removed
from the aforementioned. Note that non-existing devices in the list will
not cause an error - they will simply be ignored - but it's still nice
to not have them in there.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
2020-10-14 12:30:50 +02:00
Stefan Reiter
2141a802b8 fix #3010: add 'bootorder' parameter for better control of boot devices
(also fixes #3011)

Deprecates the old-style 'boot' and 'bootdisk' options by adding a new
'order=' subproperty to 'boot'.

This allows a user to specify more than one disk in the boot order,
helping with newer versions of SeaBIOS/OVMF where disks without a
bootindex won't be initialized at all (breaks soft-raid and some LVM
setups).

This also allows specifying a bootindex for USB and hostpci devices,
which was not possible before. Floppy boot support is not supported in
the new model, but I doubt that will be a problem (AFAICT we can't even
attach floppy disks to a VM?).

Default behaviour is intended to stay the same, i.e. while new VMs will
receive the new 'order' property, it will be set so the VM starts the
same as before (using get_default_bootorder).

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
2020-10-14 12:30:50 +02:00
Fabian Ebner
aa491a6ecd Fix #2728: die/warn if target is not a replication target when live-migrating
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Tested-by: Aaron Lauterer <a.lauterer@proxmox.com>
2020-07-24 07:18:08 +02:00
Mira Limbeck
fc701af740 close #2741: add VM.Config.Cloudinit permission
This allows setting ciuser, cipassword and all other cloudinit settings that
are not part of the network without VM.Config.Network permissions.
Keep VM.Config.Network still as fallback so custom roles that add
VM.Config.Network but not VM.Config.Cloudinit don't break.

Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
2020-06-26 09:41:43 +02:00
Fabian Grünbichler
a4e128a9a9 gen_rand_chars: handle errors properly
should not really happen on modern systems, but random_bytes just
returns false if it fails to generate random bytes, in which case we
want to die instead of returning an empty 'random' string.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-06-22 12:03:01 +02:00
Thomas Lamprecht
3c5bdde815 vncproxy: allow to request a generated VNC password
We used the VNC API $ticket as password for VNC, but QEMU limits the
password to the first 8 chars and ignores the rest[0].
As our tickets start with a static string (e.g., "PVE") the entropy
was a bit limited.

For Proxmox VE this does not matters much as the noVNC viewer
provided by has to go always over the API call, and so a valid
ticket and correct permissions for the requested VM are enforced
anyway.

This patch helps external users, which often use NoVNC-Websockify,
circumventing the API and relying solely on the VNC password to avoid
snooping on VNC sessions.

A 'generate-password' parameter is added, if set a password from good
entropy (using libopenssl) is generated.

For simplicity of mapping random bits to ranges we extract 6 bit of
entropy per character and add the integer value of '!' (first
printable ASCII char) to that. This way we get 64^8 possibilities,
which even with millions of guesses per second one would need years
of guessing and mostly just DDOS the server with websocket upgrade
requests.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

Tested-By: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-By: Dominik Csapak <d.csapak@proxmox.com>
2020-06-22 12:01:55 +02:00
Thomas Lamprecht
b63f34b8a1 fix indentation
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-06-18 11:26:36 +02:00
Thomas Lamprecht
326007b250 followup whitespace cleanup
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-06-17 14:05:49 +02:00
Fabian Grünbichler
d7856be59e properly parse 'vga' for termproxy
'vga' is a property string, we can't just assume it starts with the default key's value here either.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-06-17 14:04:26 +02:00
Fabian Grünbichler
d3efae29f5 fix #2787: properly parse 'vga' for vncproxy
'vga' is a property string, we can't just assume it starts with the
default key's value.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-06-17 14:04:26 +02:00
Fabian Grünbichler
35171ddb0d fix #2774: add early check for non-managed volumes
when checking whether a to-be-added drive's and the VM's replication
status are matching. otherwise, we end up in a failing generic
'parse_volume_id' with no mention of the actual reason.

adding 'replicate=0' to the new drive string fixes the underlying issue
with and without this patch, so this is just a cosmetic/usability
improvement.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-06-02 11:46:08 +02:00
Fabian Grünbichler
e3d3194446 create_disks: fix uninitialized warning
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-06-02 11:46:08 +02:00
Stefan Reiter
6e72f90bbb api: check Sys.Audit permissions when setting a custom CPU model
Explicitly allows changing other properties than the cputype, even if
the currently set cputype is not accessible by the user. This way, an
administrator can assign a custom CPU type to a VM for a less privileged
user without breaking edit functionality for them.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
2020-05-06 16:48:08 +02:00
Fabian Grünbichler
45fd77bbe3 clone: use new config_lock_shared
and move the lock call and decision logic closer together

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Fabian Ebner <f.ebner@proxmox.com>
2020-05-05 11:41:55 +02:00
Fabian Ebner
e29e5be6ef Revert "resize_vm: request new size from storage after resizing"
This reverts commit b5490d8a98.

When resizing a volume of a running VM, a qmp block_resize command
is issued. This is non-blocking, so the size on the storage immediately
after issuing the command might still be the old one.

This is part of the issue reported in bug #2621.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-05-04 17:35:21 +02:00
Fabian Grünbichler
a4c7029dd8 api/destroy: repeat early checks after lock
to protect against concurrent changes

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-05-04 17:33:53 +02:00
Fabian Grünbichler
4fb85adc65 api/resume: make nocheck root-only
this is only used for migration via 'qm mtunnel', regular users should
never need to resume a VM that does not logically belong to the node it
is running on

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-04-27 13:07:54 +02:00
Fabian Ebner
bcd5401acc Allow specifying targetstorage for offline migration
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-04-08 22:11:54 +02:00
Fabian Ebner
912792e245 Switch to using foreach_volume instead of foreach_drive
It was necessary to move foreach_volid back to QemuServer.pm

In VZDump/QemuServer.pm and QemuMigrate.pm the dependency on
QemuConfig.pm was already there, just the explicit "use" was missing.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2020-04-08 22:11:54 +02:00
Stefan Reiter
ea1c21108f Include "-cpu" parameter with snapshots/suspend
Just like with live-migration, custom CPU models might change after a
snapshot has been taken (or a VM suspended), which would lead to a
different QEMU invocation on rollback/resume.

Save the "-cpu" argument as a new "runningcpu" option into the VM conf
akin to "runningmachine" and use as override during rollback/resume.

No functional change with non-custom CPU types intended.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
2020-04-07 17:27:58 +02:00
Stefan Reiter
58c64ad5d9 Include "-cpu" parameter with live-migration
This is required to support custom CPU models, since the
"cpu-models.conf" file is not versioned, and can be changed while a VM
using a custom model is running. Changing the file in such a state can
lead to a different "-cpu" argument on the receiving side.

This patch fixes this by passing the entire "-cpu" option (extracted
from /proc/.../cmdline) as a "qm start" parameter. Note that this is
only done if the VM to migrate is using a custom model (which we can
check just fine, since the <vmid>.conf *is* versioned with pending
changes), thus not breaking any live-migration directionality.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
2020-04-07 17:27:58 +02:00
Thomas Lamprecht
e214cda8f5 followup: s/targetstorage map/storage map/ for parameter exceptions
to reduce duplicated info in the error slightly

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-04-07 17:20:37 +02:00
Thomas Lamprecht
abff03211f followup: refactor loop to use perls values
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2020-04-07 17:15:28 +02:00
Fabian Grünbichler
aea447bb9d migrate: check storage permissions and content type
both where previously missing. the existing 'check_storage_access'
helper is not applicable here since it operates on a full set of VM
config options, not just storage IDs.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-04-07 17:07:23 +02:00
Fabian Grünbichler
bf8fc5a307 migrate: allow arbitrary source->target storage maps
the syntax is backwards compatible, providing a single storage ID or '1'
works like before. the new helper ensures consistent behaviour at all
call sites.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-04-02 17:47:14 +02:00
Fabian Grünbichler
e5fd1c6591 restore: parse volid with $noerr
otherwise VMA files passed in as paths instead of as volids don't
work anymore.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2020-04-02 15:14:42 +02:00