Changes v0 -> v1:
 * Changed command-line format, now use blockdev specification to
   define vmstate image.
 * Don't deal with image creation in the tool, create externally.
 * Better block layer AIO handling in the load path.
 * Reduced fragmentation of the image backing file by using 'writtent-slice'
   bitmaps in RAM blocks. Zero block write is issued to a never written slice
   before the actual memory page write takes place.
 * Improved load performance in postcopy by using 'loaded-slice' bitmaps
   in RAM blocks.
 * Refactored error handling/messages.
 * Refactored namings.

This series is a kind of PoC for asynchronous snapshot reverting. It is
about external snapshots only and doesn't involve block devices. Thus, it's
mainly intended to be used with the new 'background-snapshot' migration
capability and otherwise standard QEMU migration mechanism.

The major ideas behind this first version were:
  * Make it compatible with 'exec:'-style migration - options can be create
    some separate tool or integrate into qemu-system.
  * Support asynchronous revert stage by using unaltered postcopy logic
    at destination. To do this, we should be capable of saving RAM pages
    so that any particular page can be directly addressed by it's block ID
    and page offset. Possible solutions here seem to be:
      use separate index (and storing it somewhere)
      create sparse file on host FS and address pages with file offset
      use QCOW2 (or other) image container with inherent sparsity support
  * Make image file dense on the host FS so we don't depend on
    copy/backup tools and how they deal with sparse files. Off course,
    there's some performance cost for this choice.
  * Make the code which is parsing unstructered format of migration stream,
    at least, not very sophisticated. Also, try to have minimum dependencies
    on QEMU migration code, both RAM and device.
  * Try to keep page save latencies small while not degrading migration
    bandwidth too much.

For this first version I decided not to integrate into main QEMU code but
create a separate tool. The main reason is that there's not too much migration
code that is target-specific and can be used in it's unmodified form. Also,
it's still not very clear how to make 'qemu-system' integration in terms of
command-line (or monitor/QMP?) interface extension.

For the storage format, QCOW2 as a container and large (1MB) cluster size seem
to be an optimal choice. Larger cluster is beneficial for performance particularly
in the case when image preallocation is disabled. Such cluster size does not result
in too high internal fragmentation level (~10% of space waste in most cases) yet
allows to reduce significantly the number of expensive cluster allocations.

A bit tricky part is dispatching QEMU migration stream cause it is mostly
unstructered and depends on configuration parameters like 'send-configuration'
and 'send-section-footer'. But, for the case with default values in migration
globals it seems that implemented dispatching code works well and won't have
compatibility issues in a reasonably long time frame.

I decided to keep RAM save path synchronous, anyhow it's better to use writeback
cache mode for the live snapshots cause of it's interleaving page address pattern.
Page coalescing buffer is used to merge contiguous pages to optimize block layer
writes.

Since for snapshot loading opening image file in cached mode would not do any good,
it implies that Linux native AIO and O_DIRECT mode is used in a common scenario.
AIO support in RAM loading path is implemented by using a ring of preallocated
fixed-sized buffers in such a way that there's always a number of outstanding block
requests anytime. It also ensures in-order request completion.

How to use:

**Save:**
* > qemu-img create -f qcow2 -o size=<2_x_ram_size>,cluster_size=1M,
           preallocation=off,refcount_bits=8 <image-filename>
* qemu> migrate_set_capability background-snapshot on
* qemu> migrate "exec:qemu-snapshot
           <image-filename>,cache.direct=off,file.aio=threads"

**Load:**
* Use 'qemu-system-* -incoming defer'
* qemu> migrate_incoming "exec:qemu-snapshot --revert
           <image-filename>,cache.direct=on,file.aio=native"

**Load with postcopy:**
* Use 'qemu-system-* -incoming defer'
* qemu> migrate_set_capability postcopy-ram on
* qemu> migrate_incoming "exec:qemu-snapshot --revert --postcopy=60
           <image-filename>,cache.direct=on,file.aio=native"

And yes, asynchronous revert works well only with SSD, not with rotational disk..

Some performance stats:
* SATA SSD drive with ~500/450 MB/s sequantial read/write and ~60K IOPS max.
* 220 MB/s average save rate (depends on workload).
* 440 MB/s average load rate in precopy.
* 260 MB/s average load rate in postcopy.


Andrey Gruzdev (7):
  migration/snapshot: Introduce qemu-snapshot tool
  migration/snapshot: Introduce qemu_ftell2() routine
  migration/snapshot: Move RAM_SAVE_FLAG_xxx defines to migration/ram.h
  migration/snapshot: Block layer AIO support in qemu-snapshot
  migration/snapshot: Implementation of qemu-snapshot save path
  migration/snapshot: Implementation of qemu-snapshot load path
  migration/snapshot: Implementation of qemu-snapshot load path in
    postcopy mode

 include/qemu-snapshot.h |  155 ++++
 meson.build             |    2 +
 migration/qemu-file.c   |    6 +
 migration/qemu-file.h   |    1 +
 migration/ram.c         |   16 -
 migration/ram.h         |   16 +
 qemu-snapshot-io.c      |  266 ++++++
 qemu-snapshot-vm.c      | 1881 +++++++++++++++++++++++++++++++++++++++
 qemu-snapshot.c         |  554 ++++++++++++
 9 files changed, 2881 insertions(+), 16 deletions(-)
 create mode 100644 include/qemu-snapshot.h
 create mode 100644 qemu-snapshot-io.c
 create mode 100644 qemu-snapshot-vm.c
 create mode 100644 qemu-snapshot.c

-- 
2.27.0

Patchew URL: https://patchew.org/QEMU/20210512192619.537268-1-andrey.gruzdev@virtuozzo.com/



Hi,

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

Type: series
Message-id: 20210512192619.537268-1-andrey.gruzdev@virtuozzo.com
Subject: [RFC PATCH v1 0/7] migration/snapshot: External snapshot utility

=== 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
From https://github.com/patchew-project/qemu
   3158964..3e9f48b  master     -> master
 - [tag update]      patchew/20210511163151.45167-1-kwolf@redhat.com -> patchew/20210511163151.45167-1-kwolf@redhat.com
 * [new tag]         patchew/20210512192619.537268-1-andrey.gruzdev@virtuozzo.com -> patchew/20210512192619.537268-1-andrey.gruzdev@virtuozzo.com
Switched to a new branch 'test'
87d9eb7 migration/snapshot: Implementation of qemu-snapshot load path in postcopy mode
dece241 migration/snapshot: Implementation of qemu-snapshot load path
cbaa5b9 migration/snapshot: Implementation of qemu-snapshot save path
f9dd5c5 migration/snapshot: Block layer AIO support in qemu-snapshot
ed9b73d migration/snapshot: Move RAM_SAVE_FLAG_xxx defines to migration/ram.h
92f9263 migration/snapshot: Introduce qemu_ftell2() routine
32c347a migration/snapshot: Introduce qemu-snapshot tool

=== OUTPUT BEGIN ===
1/7 Checking commit 32c347a19ef4 (migration/snapshot: Introduce qemu-snapshot tool)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100644

WARNING: line over 80 characters
#227: FILE: qemu-snapshot.c:63:
+    int postcopy_percent;       /* Start postcopy after % of normal pages loaded */

WARNING: line over 80 characters
#274: FILE: qemu-snapshot.c:110:
+static BlockBackend *image_open_opts(const char *optstr, QDict *options, int flags)

ERROR: switch and case should be at the same indent
#414: FILE: qemu-snapshot.c:250:
+        switch (c) {
+            case '?':
[...]
+            case 'h':
[...]
+            case 'V':
[...]
+            case 'T':
[...]
+            case 'r':
[...]
+            case OPTION_POSTCOPY:
[...]
+            case OPTION_PAGE_SIZE:
[...]
+            case OPTION_URI:
[...]
+            default:

ERROR: trailing whitespace
#436: FILE: qemu-snapshot.c:272:
+                $

ERROR: consider using qemu_strtol in preference to strtol
#449: FILE: qemu-snapshot.c:285:
+                postcopy_percent = strtol(optarg, &r, 10);

WARNING: line over 80 characters
#450: FILE: qemu-snapshot.c:286:
+                if (*r != '\0' || postcopy_percent < 0 || postcopy_percent > 100) {

ERROR: consider using qemu_strtol in preference to strtol
#468: FILE: qemu-snapshot.c:304:
+                target_page_size = strtol(optarg, &r, 0);

WARNING: line over 80 characters
#469: FILE: qemu-snapshot.c:305:
+                if (*r != '\0' || (target_page_size & (target_page_size - 1)) != 0 ||

ERROR: space required after that ',' (ctx:VxV)
#494: FILE: qemu-snapshot.c:330:
+                    fd = strtol(p, &r,10);
                                      ^

ERROR: consider using qemu_strtol in preference to strtol
#494: FILE: qemu-snapshot.c:330:
+                    fd = strtol(p, &r,10);

total: 6 errors, 5 warnings, 563 lines checked

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

2/7 Checking commit 92f92637b84c (migration/snapshot: Introduce qemu_ftell2() routine)
3/7 Checking commit ed9b73da773a (migration/snapshot: Move RAM_SAVE_FLAG_xxx defines to migration/ram.h)
WARNING: Block comments use a leading /* on a separate line
#51: FILE: migration/ram.h:36:
+/* RAM_SAVE_FLAG_ZERO used to be named RAM_SAVE_FLAG_COMPRESS, it

total: 0 errors, 1 warnings, 44 lines checked

Patch 3/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/7 Checking commit f9dd5c542600 (migration/snapshot: Block layer AIO support in qemu-snapshot)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#81: 
new file mode 100644

total: 0 errors, 1 warnings, 314 lines checked

Patch 4/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/7 Checking commit cbaa5b9bd2a6 (migration/snapshot: Implementation of qemu-snapshot save path)
WARNING: line over 80 characters
#554: FILE: qemu-snapshot-vm.c:507:
+            error_report("RAM page with unknown combination of flags 0x%x", flags);

WARNING: line over 80 characters
#648: FILE: qemu-snapshot-vm.c:601:
+                         (size_t)(ssize_t) (VMSTATE_HEADER_EOF_OFFSET - eof_pos));

WARNING: line over 80 characters
#832: FILE: qemu-snapshot-vm.c:783:
+             * Normally we will never get here since the ending part of migration

total: 0 errors, 3 warnings, 901 lines checked

Patch 5/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit dece2418c08d (migration/snapshot: Implementation of qemu-snapshot load path)
ERROR: trailing whitespace
#296: FILE: qemu-snapshot-vm.c:996:
+        $

ERROR: switch and case should be at the same indent
#435: FILE: qemu-snapshot-vm.c:1135:
+        switch (flags) {
+            case RAM_SAVE_FLAG_MEM_SIZE:
[...]
+            case RAM_SAVE_FLAG_EOS:
[...]
+            default:

WARNING: line over 80 characters
#446: FILE: qemu-snapshot-vm.c:1146:
+                error_report("Unknown combination of RAM page flags 0x%x", flags);

WARNING: line over 80 characters
#765: FILE: qemu-snapshot.c:258:
+    s->aio_ring = aio_ring_new(ram_load_aio_co, AIO_RING_SIZE, AIO_RING_INFLIGHT);

total: 2 errors, 2 warnings, 741 lines checked

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

7/7 Checking commit 87d9eb785185 (migration/snapshot: Implementation of qemu-snapshot load path in postcopy mode)
WARNING: line over 80 characters
#517: FILE: qemu-snapshot-vm.c:1272:
+    send_command(s->f_fd, MIG_CMD_PACKAGED, sizeof(length), (uint8_t *) &length);

WARNING: line over 80 characters
#603: FILE: qemu-snapshot-vm.c:1836:
+        if (!res && s->postcopy && !s->in_postcopy && is_postcopy_switchover(s)) {

total: 0 errors, 2 warnings, 628 lines checked

Patch 7/7 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/20210512192619.537268-1-andrey.gruzdev@virtuozzo.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com