Without this change, it's necessary to send a second savevm-end QMP
command after aborting a snaphsot, before a new savevm-start QMP
command can succeed.
In process_savevm_finalize(), no longer set an error in the abort
scenario. If there already is another error, there's no need to
override it. If canceling was done intentionally, qmp_savevm_end()
is responsible for setting the state now.
Reported-by: Mira Limbeck <m.limbeck@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Reported in the community forum[0].
For 6.1.0, there were a few changes to the coroutine-sleep API, but
the adaptations in f376b2b ("update and rebase to QEMU v6.1.0") made
a mistake.
Currently, target_close_wait is NULL when passed to
qemu_co_sleep_ns_wakeable(), which further passes it to
qemu_co_sleep(), but there, it is dereferenced when trying to access
the 'to_wake' member:
> Thread 1 "kvm" received signal SIGSEGV, Segmentation fault.
> qemu_co_sleep (w=0x0) at ../util/qemu-coroutine-sleep.c:57
To fix it, create a proper struct and pass its address instead. Also
call qemu_co_sleep_wake unconditionally, because the NULL check (for
the 'to_wake' member) is done inside the function itself.
This patch is based on what the QEMU commits introducing the changes
to the coroutine-sleep API did to the callers in QEMU:
eaee072085 ("coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing")
29a6ea24eb ("coroutine-sleep: replace QemuCoSleepState pointer with struct in the API")
[0]: https://forum.proxmox.com/threads/112130/
Tested-by: Mira Limbeck <m.limbeck@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
For the io_uring patch, it's not very clear which configurations can
trigger it, but it should be rather uncommon. See qemu commit
be6a166fde652589761cf70471bcde623e9bd72a for a bit more information.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
coming in via qemu-stable (except for the vdmk fix, which was tagged
for-7.0 on the qemu-devel list, but didn't make it into the release).
Also took the chance to switch the gluster fix to the version that
made it into upstream.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
See QEMU commits 9e8be4c546ce8469ca9702715bf8f198d604b685 and
a5730b8bd3675f484ed0eacea052452048eeb35d for more information.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Only very minor changes needed:
* Most patches in extra (or some version of them) are part of 7.0.0.
* aio_set_fd_handler got an extra parameter, but can just pass NULL
like we did for the related 'poll' parameter. See QEMU commit
826cc32423db2a99d184dbf4f507c737d7e7a4ae for more.
* Add include for qemu/memalign.h in vma.c and vma-writer.c.
* Add reverts for fixups of already reverted 0347a8fd4c ("block/rbd:
implement bdrv_co_block_status") that came in with 7.0.0. Those
fixups are not enough, see Proxmox bugzilla #4047.
* Two trivial context changes for bitmap-mirror patches.
* block_int.h got split up into multiple headers.
* Some context changes in configure and meson.build.
* Used the oppurtunity to squash fixup of bdrv_backuo_dump_create typo
in a later patch into the patch introducing the function (had to
move code to new header during rebase).
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Introduce a new map line for skipping a certain drive, of the form
skip=drive-scsi0
Since in PVE, most archives are compressed and piped to vma for
restore, it's not easily possible to skip reads.
For the reader, a new skip flag for VmaRestoreState is added and the
target is allowed to be NULL if skip is specified when registering.
If
the skip flag is set, no writes will be made as well as no check for
duplicate clusters. Therefore, the flag is not set for verify.
Originally-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Acked-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
It pulls in a lot of stuff via the libglx0 -> libglx-mesa0 dependency
chain, so only suggest it for now to avoid installing it in the
installer or via common "PVE on-top Debian" installations, VirGL
integration is experimental after all and we may drop/replace it with
the vulkan based venus one, once available (Debian 12?).
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This is necessary for multi-disk backups where not all jobs are
immediately started after they are created. QEMU commit
06e0a9c16405c0a4c1eca33cf286cc04c42066a2 did already part of the work,
ensuring that new writes after job creation don't pass through to the
backup, but not yet for the MIRROR_SYNC_MODE_BITMAP case which is used
for PBS.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
was always disabled in our clean builds, this now also avoids
auto-enabling it on "dirty" build hosts
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Notable changes:
* bdrv_co_p{discard,readv,writev,write_zeroes} function signatures
changed, to using int64_t for offsets/bytes and some still had int
rather than BrdvRequestFlags for the flags.
* job_cancel_sync now has a force parameter. Commit messages in
73895f3838cd7fdaf185cf1dbc47be58844a966f
4cfb3f05627ad82af473e7f7ae113c3884cd04e3
sound like using force=true makes more sense.
* Added 3 patches coming in via qemu-stable tag, most important one is
to work around a librbd issue.
* Added another 3 patches from qemu-devel to fix issue leading to
crash when live migrating with iothread.
* cluster_size calculation helper changed (see patch pve/0026).
* QAPI's if conditionals now use 'CONFIG_FOO' rather than
'defined(CONFIG_FOO)'
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Otherwise, the header might already get written by a coroutine and
registering further streams will fail after that.
Also adds a missing g_list_free call for the other GList that's used.
Reported in the community forum:
https://forum.proxmox.com/threads/104744/
Reproducer script (increase beyond 30 if the issue isn't triggered yet):
> #!/usr/bin/perl
>
> my $dir = "./vma-create-bug";
> mkdir $dir;
>
> my $archive_path = "$dir/vzdump-qemu-104-2202_02_02-00_00_00.vma";
> unlink $archive_path;
>
> my $cmd = "vma create $archive_path -v";
> for (my $i = 0; $i < 30; $i++) {
> system("truncate -s 1M $dir/drive-virtio$i.img");
> $cmd .= " drive-virtio$i=$dir/drive-virtio$i.img";
> }
> system($cmd);
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
which fixes the bad commit 18473467d55a20d643b6c9b3a52de42f705b4d35
that was tracked down via bisecting, and has a Cc for qemu-stable as
well.
Issue was easy enough to reproduce with a single virtio-block disk
using a few runs of dd if=/dev/urandom of=file bs=1M count=1000
Commit cc071629539dc1f303175a7e2d4ab854c0a8b20f upstream.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
both of them depend on generated header files, so we have to specify
them as sources. Otherwise, it happens (at least on some machines)
that they will be compiled before the headers are generated, aborting
the build.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
libguestfs starts their helper VMs with `-machine accel=..` without a
machine type, and our pve version suffix handling would segfault in that
case. there might be other scripted use cases that are affected as well.
this regression was introduced with the rebase of our patch set on top
of 6.1.0
Fixes: f376b2b9e2
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Very clean rebase, only the +pve version handling needed manual fixing.
Drops two applied patches from extra/ and adds one new from upstream
(extra/0001*, fixes VNC over unix sockets) as well as 3 of my own for
allowing password changes on custom VNC displays again (as seen and
reviewed upstream, but not yet applied).
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
same as the initial version sent to qemu-devel, it won't be the final
fix we plan to upstream but it should be enough band-aid to
workaround how PVE uses the QMP.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
[ Thomas: add a bit reasoning to commit message body ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This drops debian/patches/pve/0005-PVE-Config-smm_available-false.patch
(and renumbers the remaining patches)
From what I could gather, this patch was originally added
due to issues with old kernels. Now we have users which
seem to run into issues *with* the patch.
All this does is toggle an option, and it's available via a
qemu CLI option anyway, so if dropping this patch causes
issues for some people we can just add an option to
qemu-server & UI control smm explicitly.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Cc: Alexandre Derumier <aderumier@odiso.com>
Tested-by: Stefan Reiter <s.reiter@proxmox.com>
Linux SCSI can throw spurious -EAGAIN in some corner cases in its
completion path, which will end up being the result in the completed
io_uring request.
Resubmitting such requests should allow block jobs to complete, even
if such spurious errors are encountered.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Mostly minor changes, bigger ones summarized:
* QEMU's internal backup code now uses a new async system, which allows
parallel requests - the default max_workers settings is 64, I chose
less, since 64 put enough stress on QEMU that the guest became
practically unusable during the backup, and 16 still shows quite a
nice measureable performance improvement. Little code changes for us
though.
* 'malformed' QAPI parameters/functions are now a build error (i.e.
using '_' vs '-'), I chose to just whitelist our calls in the name of
backwards compatibility.
* monitor OOB race fix now uses the upstream variant, cherry-picked from
origin/master since it's not in 6.0 by default
* last patch fixes a bug with snapshot rollback related to the new yank
system
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
ran into this when live-restoring a backup configured for IO-threads,
got the good ol':
> qemu: qemu_mutex_unlock_impl: Operation not permitted
error.
Checking out the history of the related bdrv_backup_top_drop(*bs)
method, we can see that it used to do the AIO context acquiring too,
but in the backup path this was problematic and was changed to be
higher up in the call path in a upstream series from Stefan[0].
That said, this is a completely different code path and it is safe to
do so here. We always run from the main threads's AIO context here
and we call it only indirectly once, guarded by checking for
`s->drop_state == DropNone` and set `s->drop_state = DropRequested`
shortly before we schedule the track_drop() in a bh.
[0]: https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg09139.html
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Reads just nicer with a drain begin *and* end call. Also clearing the
backing link of the alloc track BDS makes it closer to
bdrv_backup_top_drop() with which this driver has a bit in common.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
cherry-picked cleanly from 6.0 development tree, fixes an issue with
resizing RBD drives (and reportedly also on krbd or potentially other
storage backends) with iothreads.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
See added patches for more info, overview:
0044: slightly increase PBS performance by reducing allocations
0045: slightly increase block-stream performance for Ceph
0046: don't crash with block-stream on RBD
0047: add alloc-track driver for live restore
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Saving dirty bitmaps from our savevm-async code didn't work, since we
use a coroutine which holds the iothread mutex already (upstream savevm
is sync, migration uses a thread). Release the mutex before calling the
one function that (according to it's documentation) requires the lock to
*not* be held: qemu_savevm_state_pending.
Additionally, loading dirty bitmaps requires a call to
dirty_bitmap_mig_before_vm_start after "loadvm", which the upstream
savevm does explicitly afterwards - do that too.
This is exposed via the query-proxmox-support property
"pbs-dirty-bitmap-savevm".
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
...instead of having them in the middle of the backup related patches.
These might (hopefully) become upstream at some point as well.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Use blk_unref to drop the last reference, which will close the block
backend and flush all caches and outstanding writes.
This is especially important for restoring to Ceph, as the userspace
librbd caches will not be flushed if the application exits immediately,
leading to potentially incomplete restores.
Reported-by: Eneko Lacunza <elacunza@binovo.es>
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
it ships a symbol file now, so it can be auto-generated based on the
build-dep and usage.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Lots of patches touched and some slight changes to the build process
since QEMU switched to meson as their build system. Functionality-wise
very little rebasing required.
New patches introduced:
* pve/0058: to fix VMA backups and clean up some code in general with
new 5.2 features now available to us (namely coroutine-enabled QMP).
* extra/0002: don't build man pages for guest agent when disabled
* extra/0003: fix live-migration with hugepages
* 0017 and 0018 are adjusted to fix snapshot abort and improve
snap performance a bit
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Fixes vma restore when the target is an iSCSI storage which expects that
initiatorname. Also avoids the need to always explicitly set the initiatorname
in PVE code, thus fixing moving efidisks from and to such iSCSI storages.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
jemalloc does not play nice with our Rust library (proxmox-backup-qemu),
specifically it never releases memory allocated from Rust to the OS.
This leads to a problem with larger caches (e.g. for the PBS block driver).
It appears to be related to this GitHub issue:
https://github.com/jemalloc/jemalloc/issues/1398
The background_thread solution seems weirdly hacky, so let's disable
jemalloc entirely for now.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
bump build-dependency on libproxmox-backup-qemu0-dev with version query
support
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
The idea in general is to migrate all the state, which is small for
us, in a single step once. But, QEMU only calls save state if we
return active true.
Hardcoding is-active to return true, like done initially, makes the
migration freeze, as QEMU thinks this is never done, and only stops
calling us and finishes after a few seconds.
So, add a state with an "active" boolean, set to true when
initializing a migration, and set it to false when the state was
saved.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
With the transaction patches, patch 0026-PVE-Backup-modify-job-api.patch
is no longer necessary, so drop it and rebase all following patches on
top.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Prints QEMU errors that occur *after* the "-daemonize" fork to the
systemd journal, instead of pushing them into /dev/null like before.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
this is needed for template backups with PBS until we have the backup
equivalent of 'pbs-restore'.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
No major semantic changes, mostly just deprecations and changed function
signatures. Drop the extra/ patches, as they have been applied upstream.
The added extra/ patch was accepted upstream[0] but has not been picked
up for 5.1. It is required for non-4M aligned backups to work with PBS.
[0] https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg01671.html
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
fixes an assertion failure in qemu network packet processing, which can
lead to DOS'ing the qemu process on the host. this affects 'e1000e' and
'vmxnet3' network devices.
patch is cherry-picked from the commit mentioned in the oss-security email.
more info on oss-security [0]
[0]: https://www.openwall.com/lists/oss-security/2020/08/10/1
Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
"PVE backup: rename incremental to use-dirty-bitmap" merged two
variables (use_dirty_bitmap and incremental) into one, but they served
two different purposes. Rename the original use_dirty_bitmap to
"expect_only_dirty" so the new one doesn't conflict, and rework "PVE:
use proxmox_backup_check_incremental" around that semantic.
In practice, this had the effect that only one disk at a time would
have a bitmap added, as after the first "use_dirty_bitmap" would be set
to one and the rest would behave as if the QMP parameter of the same
name was unset.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
* rename "incremental" param to "use-dirty-bitmap", avoids confusion
as the backup can be incrementally also with that param set to
false.
* use new proxmox_backup_check_incremental
* fix setting dirty counter and adapt to new connect API semantic
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
to ensure we have the new 'incremental' parameter for the
proxmox_backup_register_image and proxmox_backup_register_image_async
functions available
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Add trailing newline to two error messages, and drop an extra
unconditional `qdict_put_str(options, "driver", format);`
Besides that it's just formatting.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Move qemu_savevm_state_{header,setup} into the main loop and
the rest of the iteration into a coroutine. The former need
to lock the iothread (and we can't unlock it in the
coroutine), and the latter can't deal with being in a
separate thread, so a coroutine it must be.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Release the lock on qemu_coroutine_yield, so coroutines don't
deadlock.
Originally-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
the first two patches were mistakenly left out during the 4.2 qemu
rebase.
also adds another patch for issue CVE-2019-14378 (heap-based BOF)
Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
The long overdue nice rebase+cleanup was done by Dietmar
Originally-by: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
original commits and email can be found here[0]
A out-of-bounds heap buffer access issue was found in the SLiRP
networking implementation of the QEMU emulator. It occurs in tcp_emu()
routine while emulating IRC and other protocols due to unsafe usage of
snprintf(3) function.
A user/process could use this flaw to crash the Qemu process on the host
resulting in DoS or potentially execute arbitrary code with privileges
of the QEMU process on the host.
[0]: https://seclists.org/oss-sec/2020/q1/64
Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
oss-security email can be found here[0]
upstream commit here[1]
this effects our vncproxy. dominik and me tested if the issue is present
on our branch and it appears that it is.
in essence when we disconnect from a vnc connection, the memory isn't
free'd afterwards which causes the qemu process to use more and more
memory with each disconnect, which could lead to a dos scenario.
we tested the patch and it seems to mitigate the problem.
[0]: https://seclists.org/oss-sec/2020/q1/105
[1]: https://git.qemu.org/?p=qemu.git;a=commitdiff;h=6bf21f3d83e95bcc4ba35a7a07cc6655e8b010b0
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
* Fix VMA tool build
* Change PVE code to new blockjob API
* Acquire missing lock for block_job_add_bdrv
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>