Message ID | 20200616162035.29857-1-den@openvz.org |
---|---|
Headers | show |
Series | block: seriously improve savevm performance | expand |
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
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>