Stefan Reiter [Mon, 8 Mar 2021 15:32:30 +0000 (16:32 +0100)]
vzdump: increase PBS 'backup' QMP call timeout
Commit "a941bbd0 client: raise HTTP_TIMEOUT to 120s" in proxmox-backup
did the same, however, we would now still fail after 60 seconds since
the QMP call would time out.
Increase the timeout here to the same +5 seconds to give some time to
receive a response, so if the HTTP call in proxmox-backup times out, we
can still get a useful error message instead of timing out the QMP call
too.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
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>
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.
Fabian Ebner [Mon, 1 Mar 2021 12:02:23 +0000 (13:02 +0100)]
fix #3324: clone disk: use larger blocksize for EFI disk
Moving to Ceph is very slow when bs=1. Instead, use a larger block size in
combination with the (currently) PVE-specific osize option to specify the
desired output size.
Stefan Reiter [Thu, 18 Feb 2021 13:52:34 +0000 (14:52 +0100)]
snapshot-test: mock query-savevm better
Otherwise the new printing functions produce warnings about undefined
numbers. These stats are guaranteed to be returned by real QEMU, so mock
them with some sensible values.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Aaron Lauterer [Fri, 12 Feb 2021 15:57:50 +0000 (16:57 +0100)]
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.
Stefan Reiter [Tue, 9 Feb 2021 15:24:07 +0000 (16:24 +0100)]
qmeventd: explicitly close() pidfds
In most circumstances a pidfd gets closed automatically once the child
dies, and that *should* be guaranteed by us calling SIGKILL - however,
it seems that sometimes that doesn't happen, leading to leaked file
descriptors[0].
Also add a small note to verbose mode showing when the late-cleanup
actually happens, helped during debug.
Fabian Ebner [Mon, 8 Feb 2021 14:26:03 +0000 (15:26 +0100)]
test: migrate: correctly mock storage module
by fixing a typo. Since cfs_read_file within the storage module was not mocked,
the tests could fail on some setups. Now that get_bandwidth_limit is mocked,
cfs_read_file is not called anymore, but still mock it too for good measure and
to make it more future-proof.
Stefan Reiter [Mon, 18 Jan 2021 13:07:52 +0000 (14:07 +0100)]
anchor CPU flag regex to avoid arbitrary flag suffixes
Previously one could specify a CPU flag like 'pcidfoobar' and it would
be accepted, even though we attempt to filter VM-only flags for
security. AFAICT none of the flags we allow can be turned into any
others just by appending text, but better safe than sorry.
Reported-by: Oguz Bektas <o.bektas@proxmox.com> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Thomas Lamprecht [Mon, 25 Jan 2021 14:49:39 +0000 (15:49 +0100)]
vm destroy: destroy also unusedX config entries
this was previously covered by the "lets destroy ever disk which
matches the VMID" feature we disarmed a bit.
As unused disks are referenced in the config, it is not subtle to
destroy them (and we always did in the past) so fix that regression
again for explicitly referenced but unused disks.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Mon, 25 Jan 2021 14:06:19 +0000 (15:06 +0100)]
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>
Thomas Lamprecht [Mon, 25 Jan 2021 13:58:55 +0000 (14:58 +0100)]
qm: import ovf: removed all imported disks on error
While the do_import method cleans up the current disk it was
importing on any error the following cases are not handled:
* multiple disks, first few succeed then one fails, only the last
failed one was taken care of before this patch
* error after the import disk loop was not handled
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Mira Limbeck [Fri, 15 Jan 2021 11:49:24 +0000 (12:49 +0100)]
fix drive-mirror completion with cloudinit
On clone_vm when cloning the disks while the VM is running, we use
drive-mirror. We skip completion until the last disk, but with a
cloudinit disk there's no drive-mirror and so no completion done. If it
is the last disk in the hash, we never complete the drive-mirror jobs
and no further cloning is possible as there are already active jobs
using the disks.
To fix it we have to call qemu_drive_mirror_monitor directly in the case
of cloudinit when completion is requested and there are jobs defined.
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
Fabian Ebner [Mon, 11 Jan 2021 12:34:09 +0000 (13:34 +0100)]
tests: mock storage locking for migration tests
by doing it in a local directory instead of /var/lock/pve-manager, which is
used by the installed/non-test PVE code. This also covers the shared case,
which will become relevant after fixing #3229 (currently migration doesn't
touch disks on shared storages).
Reported-by: Stefan Reiter <s.reiter@proxmox.com> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Fabian Ebner [Thu, 17 Dec 2020 09:37:02 +0000 (10:37 +0100)]
tests: allow running migration tests in parallel
It's not easily possible to use separate JSON files for the test configuration,
because part of it is generated with perl code. While this could be encoded too,
it seems cleaner to use the "run a single test by specifing the name"
functionality while adding a way for make to obtain a list of the test names.
Each test has (and needs) its own directory now, meaning the log files do not
need to be renamed anymore.
Mira Limbeck [Wed, 2 Dec 2020 12:50:25 +0000 (13:50 +0100)]
fix cloning/restoring of cloudinit disks in raw format
We only added the format extension when it was not 'raw'. But on file level
storages we always require it. To fix this, always add the format
extension if the storage provides the 'path' property.
This is the same logic we use in create_disks for cloudinit disks.
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
Fabian Ebner [Tue, 1 Dec 2020 12:07:02 +0000 (13:07 +0100)]
create test environment for migration
and the associated parts for 'qm start'.
Each test will first populate the MigrationTest/run directory
with the relevant configuration files and files keeping track of the
state of everything necessary. Second, the mock-script for migration
is executed, which in turn will execute the 'qm start' mock-script
(if it's an online test that gets far enough). The scripts will simulate
a migration and update the relevant files in the MigrationTest/run directory.
Finally, the main test script will evaluate the state.
The main checks are the volume IDs on the source and target and the VM
configuration itself. Additional checks are the vm_status and expected_calls,
keeping track if certain calls have been made.
The rationale behind creating two mock-scripts is two-fold:
1. It removes the need to hard code responses for the tunnel
and to recycle logic for determining and allocating migration volumes.
Some of that logic already happens in the API part, so it was necessary
to mock the whole CLI-Handler.
2. It allows testing the code relevant for migration in 'qm start' as well,
and it should even be possible to test different versions of the
mock-scripts against each other. With a bit of extra work and things
like 'git worktree', it might even be possible to automate this.
The helper get_patched config is useful to change pre-defined configuration
files on the fly, avoiding the new to explicitly define whole configurations to
test for something in many cases.
Fabian Ebner [Mon, 7 Dec 2020 12:15:15 +0000 (13:15 +0100)]
clone_disk: fix offline clone of efidisk
by partially reverting 4df98f2f14348d0ed57529c4c04a1b5ffb840055 and fixing the
line-length issue differently. The commit didn't update two later usages of
$size, breaking copying the efidisk. The other usage as a parameter to
qemu_img_convert() is luckily only cosmetic, for progress output.
Fabian Ebner [Thu, 29 Oct 2020 13:31:32 +0000 (14:31 +0100)]
don't migrate replicated VM whose replication job is marked for removal
while it didn't actually fail, we probably want to avoid the behavior:
With remove_job=full:
* run_replication called during migration causes the replicated volumes to
be removed
* migration continues by fully copying all volumes
With remove_job=local:
* run_replication called during migration causes the job (and local
replication snapshots) to be removed
* migration continues by fully copying all volumes and renaming them to
avoid collision with the still existing remote volumes
Fabian Ebner [Thu, 29 Oct 2020 13:31:31 +0000 (14:31 +0100)]
fix checks for transfering replication state/switching job target
In some cases $self->{replicated_volumes} will be auto-vivified
to {} by checks like
next if $self->{replicated_volumes}->{$volid}
and then {} would evaluate to true in a boolean context.
Now the replication information is retrieved once in prepare,
and used to decide whether to make the calls or not.
Stefan Reiter [Mon, 19 Oct 2020 12:18:41 +0000 (14:18 +0200)]
fix vm_resume and allow vm_start with QMP status 'shutdown'
When the VM is in status 'shutdown', i.e. after the guest issues a
powerdown while a backup is running, QEMU requires a 'system_reset' to
be issued before 'cont' can boot the guest again.
Additionally, when the VM has been powered down during a backup, the
logically correct call would be a 'vm_start', so automatically vm_resume
from vm_start in case this situation occurs. This also means the GUI can
cope with this almost unchanged.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Stefan Reiter [Mon, 19 Oct 2020 12:18:39 +0000 (14:18 +0200)]
vzdump: use dirty bitmap for not running VMs too
Now that VMs can be started during a backup, it makes sense to create a
dirty bitmap in these cases too, since the VM might be resumed and thus
continue running normally even after the backup is done.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Stefan Reiter [Mon, 19 Oct 2020 12:18:38 +0000 (14:18 +0200)]
vzdump: connect to qmeventd for duration of backup
Connect and send the vmid of the VM being backed up. This prevents
qmeventd from SIGTERMing the underlying QEMU instance, even if the guest
shuts itself down, until we close the socket connection (in cleanup,
which happens on success and abort, or if we crash the file handle will
be closed as well).
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Stefan Reiter [Mon, 19 Oct 2020 12:18:37 +0000 (14:18 +0200)]
qmeventd: add last-ditch effort SIGKILL cleanup
'alarm' is used to schedule an additionaly cleanup round 5 seconds after
sending SIGTERM via terminate_client. This then sends SIGKILL via a
pidfd (if supported by the kernel) or directly via kill, making sure
that the QEMU process is *really* dead and won't be left behind in an
undetermined state.
This shouldn't be an issue under normal circumstances, but can help
avoid dead processes lying around if QEMU hangs after SIGTERM.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Stefan Reiter [Mon, 19 Oct 2020 12:18:36 +0000 (14:18 +0200)]
qmeventd: add handling for -no-shutdown QEMU instances
We take care of killing QEMU processes when a guest shuts down manually.
QEMU will not exit itself, if started with -no-shutdown, but it will
still emit a "SHUTDOWN" event, which we await and then send SIGTERM.
This additionally allows us to handle backups in such situations. A
vzdump instance will connect to our socket and identify itself as such
in the handshake, sending along a VMID which will be marked as backing
up until the file handle is closed.
When a SHUTDOWN event is received while the VM is backing up, we do not
kill the VM. And when the vzdump handle is closed, we check if the
guest has started up since, and only if it's determined to still be
turned off, we then finally kill QEMU.
We cannot wait for QEMU directly to finish the backup (i.e. with
query-backup), as that would kill the VM too fast for vzdump to send the
last 'query-backup' to mark the backup as successful and done.
For handling 'query-status' messages sent to QEMU, a state-machine-esque
protocol is implemented into the Client struct (ClientState). This is
necessary, since QMP works asynchronously, and results arrive on the
same channel as events and even the handshake.
For referencing QEMU Clients from vzdump messages, they are kept in a
hash table. This requires linking against glib.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Dominik Csapak [Mon, 19 Oct 2020 14:11:38 +0000 (16:11 +0200)]
partially fix #3056: try to cancel backup without uuid
if the 'backup' qmp call itself times out or fails, we still want to
try to cancel the backup, else it can happen that there is still
a backup running even when vzdump thinks it was canceled
qapi docs says that backup cancel always returns success, even
if no backup is running
since we hold a global and a per vm lock for the backup, this should be
ok, since we should not reach this code without that lock
Stefan Reiter [Thu, 22 Oct 2020 15:34:20 +0000 (17:34 +0200)]
migrate: enable dirty-bitmap migration
We query QEMU if it's safe before enabling it, as on versions without
the necessary patches it not only would be useless, but can actually
lead to hangs.
PBS state is always migrated, as it's a small amount of data anyway, so
we don't need to set a specific flag for it.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Stefan Reiter [Wed, 21 Oct 2020 09:00:25 +0000 (11:00 +0200)]
bootorder: don't print empty 'order=' property
Specifying 'boot: order=' was intended to be used for an empty bootorder
(i.e. no boot devices), but as it turns out our format parser doesn't
like empty '-list' properties if they are nested in a subformat.
Fixing this in JSONSchema sounds like a risky move, so instead just
write 'boot: ' (without 'order=') to indicate an empty bootorder. The
rest of the code handles it just fine, as this was valid before too.
Incidentally also fixes a bug where you couldn't create a new VM without
any disks if no explicit 'boot' property was specified (i.e. a simple
'qm create 100' without any parameters would fail).
Reported-by: Dominic Jäger <d.jaeger@proxmox.com> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Thomas Lamprecht [Fri, 16 Oct 2020 15:53:28 +0000 (17:53 +0200)]
PCI: use warnings/strict and fix setting $vga from config2command
fixes commit 74c17b7a23c8a953d1dcec9bd53449d71d88cd5d which moved
this code here, but forgot to pass $vga ref, as the module was not
using warning nor strict mode this was not caught..
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
projects / qemu-server.git / log