In qemu-server, we already allocate 2 * $mem_size + 500 MiB for driver
state (which was 32 MiB long ago according to git history). It seems
likely that the 30 MiB cutoff in the savevm-async implementation was
chosen based on that.
In bug #4476 [0], another issue caused the iteration to not make any
progress and the state file filled up all the way to the 30 MiB +
pending_size cutoff. Since the guest is not stopped immediately after
the check, it can still dirty some RAM and the current cutoff is not
enough for a reproducer VM (was done while bug #4476 still was not
fixed), dirtying memory with
> stress-ng -B 2 --bigheap-growth 64.0M'
After entering the final stage, savevm actually filled up the state
file completely, leading to an I/O error. It's probably the same
scenario as reported in the bug report, the error message was fixed in
commit a020815 ("savevm-async: fix function name in error message")
after the bug report.
If not for the bug, the cutoff will only be reached by a VM that's
dirtying RAM faster than can be written to the storage, so increase
the cutoff to 100 MiB to have a bigger chance to finish successfully,
while still trying to not increase downtime too much for
non-hibernation snapshots.
[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=4476
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
when pend_postcopy is large. By definition, pend_postcopy won't
decrease when iterating, so a value larger than the cutoff of 400000
would lead to essentially empty iterations, filling up the state file
until only 30 MiB + pending_size remain and the second half of the
check would trigger.
Avoid this, by not considering pend_postcopy for the cutoff to enter
the final phase.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
it ships files also shipped by our qemu package, switching from Debian qemu to
ours doesn't work without manual intervention otherwise..
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
between QEMU less than 7.2 and QEMU 7.2 without the fix (both
directions are affected).
As mentioned in the patch message, this fix itself will break
migration between QEMU 7.2 and QEMU 7.2 with the fix (in both
directions, if a virtio-rng device is attached), but this is fine,
because no pve-qemu-kvm package with QEMU 7.2 has been publicly
released yet.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Two for virtio-mem and one for vIOMMU. Both features are not yet
exposed in PVE's qemu-server, but planned to be added.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Avoids a patch and is required to compile when not all patches are
applied. No functional change is intended.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Commit d03e1b3 ("update submodule and patches to 7.2.0") argued that
slirp is not explicitly supported in PVE, but that is not true. In
qemu-server, user networking is supported (via CLI/API) when no bridge
is set on a virtual NIC. So slirp needs to stay to keep such NICs
working.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Otherwise, it depends on whether libslirp-devel is installed or not.
See the previous commit message for more context.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
User-facing breaking change:
The slirp submodule for user networking got removed. It would be
necessary to add the --enable-slirp option to the build and/or install
the appropriate library to continue building it. Since PVE is not
explicitly supporting it, it would require additionally installing the
libslirp0 package on all installations and there is *very* little
mention on the community forum when searching for "slirp" or
"netdev user", the plan is to only enable it again if there is some
real demand for it.
Notable changes:
* The big change for this release is the rework of job locking, using
a job mutex and introducing _locked() variants of job API functions
moving away from call-side AioContext locking. See (in the qemu
submodule) commit 6f592e5aca ("job.c: enable job lock/unlock and
remove Aiocontext locks") and previous commits for context.
Changes required for the backup patches:
* Use WITH_JOB_LOCK_GUARD() and call the _locked() variant of job
API functions where appropriate (many are only availalbe as
a _locked() variant).
* Remove acquiring/releasing AioContext around functions taking the
job mutex lock internally.
The patch introducing sequential transaction support for jobs needs
to temporarily unlock the job mutex to call job_start() when
starting the next job in the transaction.
* The zeroinit block driver now marks its child as primary.
The documentation in include/block/block-common.h states:
> Filter node has exactly one FILTERED|PRIMARY child, and may have
> other children which must not have these bits
Without this, an assert will trigger when copying to a zeroinit target
with qemu-img convert, because bdrv_child_cb_attach() expects any
non-PRIMARY child to be not FILTERED:
> qemu-img convert -n -p -f raw -O raw input.raw zeroinit:output.raw
> qemu-img: ../block.c:1476: bdrv_child_cb_attach: Assertion
> `!(child->role & BDRV_CHILD_FILTERED)' failed.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
albeit I was short from disarming that GLOBAL_STATE_CODE assert
completely, as its just bogus to assert that on runtime for a lot of
call sites, rather it should be verified on compilation (function
coloring with attributes and maybe a compiler plugin).
But, as this is already solved upstream lets take in that patch.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
fixes file restore, where we actively unlink the PID file of the
transient VM ourself after opening it - while we use it only for
tracking when the QEMU process itself has finished start up, it's
easier and cleaner to fix this regression now, than to rework that to
something that doesn't depends on the PID file at all.
Applying Fiona's patch as patch-patch tracked under extra, as I
expect that something similar to this gets accepted upstreamed.
Link: https://lists.proxmox.com/pipermail/pve-devel/2022-October/054448.html
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
The documentation in include/io/channel.h states that -1 or
QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply passing
along the return value from the blk-functions has the potential to
confuse the call sides. Non-blocking mode is not implemented
currently, so -1 it is.
The "return ret" was mistakenly left over from the previous
QEMUFileOps based implementation. Also, use error_setg_errno(), since
the blk(_co)_p{readv,writev} functions return errno codes.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
to be in-line with what other implementations in QEMU do. Commit
1d39c7098bbfa6862cb96066c4f8f6735ea397c5 mentions the EIO bit and
the function is expected to return 0 upon success (see other
implementations).
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Notable changes:
* The only big change is the switch to using a custom QIOChannel for
savevm-async, because the previously used QEMUFileOps was dropped.
Changes to the current implementation:
* Switch to vector based methods as required for an IO channel. For
short reads the passed-in IO vector is stuffed with zeroes at the
end, just to be sure.
* For reading: The documentation in include/io/channel.h states that
at least one byte should be read, so also error out when whe are
at the very end instead of returning 0.
* For reading: Fix off-by-one error when request goes beyond end.
The wrong code piece was:
if ((pos + size) > maxlen) {
size = maxlen - pos - 1;
}
Previously, the last byte would not be read. It's actually
possible to get a snapshot .raw file that has content all the way
up the final 512 byte (= BDRV_SECTOR_SIZE) boundary without any
trailing zero bytes (I wrote a script to do it).
Luckily, it didn't cause a real issue, because qemu_loadvm_state()
is not interested in the final (i.e. QEMU_VM_VMDESCRIPTION)
section. The buffer for reading it is simply freed up afterwards
and the function will assume that it read the whole section, even
if that's not the case.
* For writing: Make use of the generated blk_pwritev() wrapper
instead of manually wrapping the coroutine to simplify and save a
few lines.
* Adapt to changed interfaces for blk_{pread,pwrite}:
* a9262f551e ("block: Change blk_{pread,pwrite}() param order")
* 3b35d4542c ("block: Add a 'flags' param to blk_pread()")
* bf5b16fa40 ("block: Make blk_{pread,pwrite}() return 0 on success")
Those changes especially affected the qemu-img dd patches, because
the context also changed, but also some of our block drivers used
the functions.
* Drop qemu-common.h include: it got renamed after essentially
everything was moved to other headers. The only remaining user I
could find for things dropped from the header between 7.0 and 7.1
was qemu_get_vm_name() in the iscsi-initiatorname patch, but it
already includes the header to which the function was moved.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This version string can be queried with $BINARY --version as well as
the query-version QMP command.
Useful for qemu-server to be able to report the running QEMU version
exactly. Could also be used to version guard against features as an
alternative to the query-proxmox-support QMP command.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
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>
