[PATCH v4 0/4] block: seriously improve savevm performance

Denis V. Lunev posted 4 patches 3 years, 11 months ago
Test FreeBSD passed
Test asan passed
Test docker-quick@centos7 passed
Test checkpatch failed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200616162035.29857-1-den@openvz.org
Maintainers: Kevin Wolf <kwolf@redhat.com>, Juan Quintela <quintela@redhat.com>, Fam Zheng <fam@euphon.net>, Stefan Hajnoczi <stefanha@redhat.com>, Max Reitz <mreitz@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
There is a newer version of this series
[PATCH v4 0/4] block: seriously improve savevm performance
Posted by Denis V. Lunev 3 years, 11 months ago
This series do standard basic things:
- it creates intermediate buffer for all writes from QEMU migration code
  to QCOW2 image,
- this buffer is sent to disk asynchronously, allowing several writes to
  run in parallel.

In general, migration code is fantastically inefficent (by observation),
buffers are not aligned and sent with arbitrary pieces, a lot of time
less than 100 bytes at a chunk, which results in read-modify-write
operations with non-cached operations. It should also be noted that all
operations are performed into unallocated image blocks, which also suffer
due to partial writes to such new clusters.

This patch series is an implementation of idea discussed in the RFC
posted by Denis Plotnikov
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg01925.html
Results with this series over NVME are better than original code
                original     rfc    this
cached:          1.79s      2.38s   1.27s
non-cached:      3.29s      1.31s   0.81s

Changes from v3:
- rebased to master
- added patch 3 which removes aio_task_pool_wait_one()
- added R-By to patch 1
- patch 4 is rewritten via bdrv_run_co
- error path in blk_save_vmstate() is rewritten to call bdrv_flush_vmstate
  unconditionally
- added some comments
- fixes initialization in bdrv_co_vmstate_save_task_entry as suggested

Changes from v2:
- code moved from QCOW2 level to generic block level
- created bdrv_flush_vmstate helper to fix 022, 029 tests
- added recursive for bs->file in bdrv_co_flush_vmstate (fix 267)
- fixed blk_save_vmstate helper
- fixed coroutine wait as Vladimir suggested with waiting fixes from me

Changes from v1:
- patchew warning fixed
- fixed validation that only 1 waiter is allowed in patch 1

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <fam@euphon.net>
CC: Juan Quintela <quintela@redhat.com>
CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
CC: Denis Plotnikov <dplotnikov@virtuozzo.com>


Re: [PATCH v4 0/4] block: seriously improve savevm performance
Posted by no-reply@patchew.org 3 years, 11 months ago
Patchew URL: https://patchew.org/QEMU/20200616162035.29857-1-den@openvz.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v4 0/4] block: seriously improve savevm performance
Type: series
Message-id: 20200616162035.29857-1-den@openvz.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
551826e block/io: improve savevm performance
81a93fd block, migration: add bdrv_flush_vmstate helper
05e3395 block/aio_task: drop aio_task_pool_wait_one() helper
61e0ce3 block/aio_task: allow start/wait task from any coroutine
d04a716 migration/savevm: respect qemu_fclose() error code in save_snapshot()

=== OUTPUT BEGIN ===
1/5 Checking commit d04a7160d2c0 (migration/savevm: respect qemu_fclose() error code in save_snapshot())
2/5 Checking commit 61e0ce32d320 (block/aio_task: allow start/wait task from any coroutine)
3/5 Checking commit 05e339500af8 (block/aio_task: drop aio_task_pool_wait_one() helper)
4/5 Checking commit 81a93fdf74a9 (block, migration: add bdrv_flush_vmstate helper)
WARNING: Block comments use a leading /* on a separate line
#93: FILE: include/block/block.h:575:
+/* bdrv_flush_vmstate() is mandatory to commit vmstate changes if

WARNING: Block comments use * on subsequent lines
#94: FILE: include/block/block.h:576:
+/* bdrv_flush_vmstate() is mandatory to commit vmstate changes if
+   bdrv_save_vmstate() was ever called */

WARNING: Block comments use a trailing */ on a separate line
#94: FILE: include/block/block.h:576:
+   bdrv_save_vmstate() was ever called */

ERROR: braces {} are necessary for all arms of this statement
#108: FILE: migration/savevm.c:154:
+    if (err < 0)
[...]

total: 1 errors, 3 warnings, 60 lines checked

Patch 4/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/5 Checking commit 551826e4ae66 (block/io: improve savevm performance)
WARNING: Block comments use a leading /* on a separate line
#132: FILE: block/io.c:2711:
+        /* Caller is responsible for cleanup. We should block all further

WARNING: Block comments use a trailing */ on a separate line
#133: FILE: block/io.c:2712:
+         * save operations for this exact state */

total: 0 errors, 2 warnings, 179 lines checked

Patch 5/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200616162035.29857-1-den@openvz.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com