The following changes since commit 848a6caa88b9f082c89c9b41afa975761262981d:

Merge tag 'migration-20230602-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-06-02 17:33:29 -0700)

https://gitlab.com/hreitz/qemu.git tags/pull-block-2023-06-05

for you to fetch changes up to 42a2890a76f4783cd1c212f27856edcf2b5e8a75:

qcow2: add discard-no-unref option (2023-06-05 13:15:42 +0200)

Block patches

- Fix padding of unaligned vectored requests to match the host alignment

for vectors with 1023 or 1024 buffers

- Refactor and fix bugs in parallels's image check functionality

- Add an option to the qcow2 driver to retain (qcow2-level) allocations

on discard requests from the guest (while still forwarding the discard

to the lower level and marking the range as zero)

Hanna Czenczek (4):

util/iov: Make qiov_slice() public

block: Collapse padded I/O vecs exceeding IOV_MAX

util/iov: Remove qemu_iovec_init_extended()

iotests/iov-padding: New test

Jean-Louis Dupond (1):

qcow2: add discard-no-unref option

qapi/block-core.json | 12 ++

block/qcow2.h | 3 +

include/qemu/iov.h | 8 +-

block/io.c | 166 ++++++++++++++++++--

block/parallels.c | 190 ++++++++++++++++-------

block/qcow2-cluster.c | 32 +++-

block/qcow2.c | 18 +++

util/iov.c | 89 ++---------

qemu-options.hx | 12 ++

tests/qemu-iotests/tests/iov-padding | 85 ++++++++++

tests/qemu-iotests/tests/iov-padding.out | 59 +++++++

11 files changed, 523 insertions(+), 151 deletions(-)

create mode 100755 tests/qemu-iotests/tests/iov-padding

create mode 100644 tests/qemu-iotests/tests/iov-padding.out

2.40.1

From: Jean-Louis Dupond <jean-louis@dupond.be>

When we for example have a sparse qcow2 image and discard: unmap is enabled,

there can be a lot of fragmentation in the image after some time. Especially on VM's

that do a lot of writes/deletes.

This causes the qcow2 image to grow even over 110% of its virtual size,

because the free gaps in the image get too small to allocate new

continuous clusters. So it allocates new space at the end of the image.

Disabling discard is not an option, as discard is needed to keep the

incremental backup size as low as possible. Without discard, the

incremental backups would become large, as qemu thinks it's just dirty

blocks but it doesn't know the blocks are unneeded.

So we need to avoid fragmentation but also 'empty' the unneeded blocks in

the image to have a small incremental backup.

In addition, we also want to send the discards further down the stack, so

the underlying blocks are still discarded.

Therefor we introduce a new qcow2 option "discard-no-unref".

When setting this option to true, discards will no longer have the qcow2

driver relinquish cluster allocations. Other than that, the request is

handled as normal: All clusters in range are marked as zero, and, if

pass-discard-request is true, it is passed further down the stack.

The only difference is that the now-zero clusters are preallocated

instead of being unallocated.

This will avoid fragmentation on the qcow2 image.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1621

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>

Message-Id: <20230605084523.34134-2-jean-louis@dupond.be>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>

qapi/block-core.json | 12 ++++++++++++

block/qcow2.h | 3 +++

block/qcow2-cluster.c | 32 ++++++++++++++++++++++++++++----

block/qcow2.c | 18 ++++++++++++++++++

qemu-options.hx | 12 ++++++++++++

5 files changed, 73 insertions(+), 4 deletions(-)

# @pass-discard-other: whether discard requests for the data source

# should be issued on other occasions where a cluster gets freed

#

+# @discard-no-unref: when enabled, discards from the guest will not cause

+# cluster allocations to be relinquished. This prevents qcow2 fragmentation

+# that would be caused by such discards. Besides potential

+# performance degradation, such fragmentation can lead to increased

+# allocation of clusters past the end of the image file,

+# resulting in image files whose file length can grow much larger

+# than their guest disk size would suggest.

+# If image file length is of concern (e.g. when storing qcow2

+# images directly on block devices), you should consider enabling

+# this option. (since 8.1)

# @overlap-check: which overlap checks to perform for writes to the

# image, defaults to 'cached' (since 2.2)

'*pass-discard-request': 'bool',

'*pass-discard-snapshot': 'bool',

'*pass-discard-other': 'bool',

+ '*discard-no-unref': 'bool',

'*overlap-check': 'Qcow2OverlapChecks',

'*cache-size': 'int',

'*l2-cache-size': 'int',

diff --git a/block/qcow2.h b/block/qcow2.h

--- a/block/qcow2.h

+++ b/block/qcow2.h

@@ -XXX,XX +XXX,XX @@

#define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request"

#define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot"

#define QCOW2_OPT_DISCARD_OTHER "pass-discard-other"

+#define QCOW2_OPT_DISCARD_NO_UNREF "discard-no-unref"

#define QCOW2_OPT_OVERLAP "overlap-check"

#define QCOW2_OPT_OVERLAP_TEMPLATE "overlap-check.template"

#define QCOW2_OPT_OVERLAP_MAIN_HEADER "overlap-check.main-header"

@@ -XXX,XX +XXX,XX @@ typedef struct BDRVQcow2State {

bool discard_passthrough[QCOW2_DISCARD_MAX];

+ bool discard_no_unref;

int overlap_check; /* bitmask of Qcow2MetadataOverlap values */

bool signaled_corruption;

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-cluster.c

+++ b/block/qcow2-cluster.c

@@ -XXX,XX +XXX,XX @@ static int discard_in_l2_slice(BlockDriverState *bs, uint64_t offset,

uint64_t new_l2_bitmap = old_l2_bitmap;

QCow2ClusterType cluster_type =

qcow2_get_cluster_type(bs, old_l2_entry);

+ bool keep_reference = (cluster_type != QCOW2_CLUSTER_COMPRESSED) &&

+ !full_discard &&

+ (s->discard_no_unref &&

+ type == QCOW2_DISCARD_REQUEST);

/*

* If full_discard is true, the cluster should not read back as zeroes,

@@ -XXX,XX +XXX,XX @@ static int discard_in_l2_slice(BlockDriverState *bs, uint64_t offset,

new_l2_entry = new_l2_bitmap = 0;

} else if (bs->backing || qcow2_cluster_is_allocated(cluster_type)) {

if (has_subclusters(s)) {

- new_l2_entry = 0;

+ if (keep_reference) {

+ new_l2_entry = old_l2_entry;

+ } else {

+ new_l2_entry = 0;

+ }

new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;

} else {

- new_l2_entry = s->qcow_version >= 3 ? QCOW_OFLAG_ZERO : 0;

+ if (s->qcow_version >= 3) {

+ if (keep_reference) {

+ new_l2_entry |= QCOW_OFLAG_ZERO;

+ } else {

+ new_l2_entry = QCOW_OFLAG_ZERO;

+ }

+ } else {

+ new_l2_entry = 0;

+ }

}

@@ -XXX,XX +XXX,XX @@ static int discard_in_l2_slice(BlockDriverState *bs, uint64_t offset,

if (has_subclusters(s)) {

set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap);

}

- /* Then decrease the refcount */

- qcow2_free_any_cluster(bs, old_l2_entry, type);

+ if (!keep_reference) {

+ /* Then decrease the refcount */

+ qcow2_free_any_cluster(bs, old_l2_entry, type);

+ } else if (s->discard_passthrough[type] &&

+ (cluster_type == QCOW2_CLUSTER_NORMAL ||

+ cluster_type == QCOW2_CLUSTER_ZERO_ALLOC)) {

+ /* If we keep the reference, pass on the discard still */

+ bdrv_pdiscard(s->data_file, old_l2_entry & L2E_OFFSET_MASK,

+ s->cluster_size);

+ }

qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);

diff --git a/block/qcow2.c b/block/qcow2.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ static const char *const mutable_opts[] = {

QCOW2_OPT_DISCARD_REQUEST,

QCOW2_OPT_DISCARD_SNAPSHOT,

QCOW2_OPT_DISCARD_OTHER,

+ QCOW2_OPT_DISCARD_NO_UNREF,

QCOW2_OPT_OVERLAP,

QCOW2_OPT_OVERLAP_TEMPLATE,

QCOW2_OPT_OVERLAP_MAIN_HEADER,

@@ -XXX,XX +XXX,XX @@ static QemuOptsList qcow2_runtime_opts = {

.type = QEMU_OPT_BOOL,

.help = "Generate discard requests when other clusters are freed",

},

+ {

+ .name = QCOW2_OPT_DISCARD_NO_UNREF,

+ .type = QEMU_OPT_BOOL,

+ .help = "Do not unreference discarded clusters",

+ },

{

.name = QCOW2_OPT_OVERLAP,

.type = QEMU_OPT_STRING,

@@ -XXX,XX +XXX,XX @@ typedef struct Qcow2ReopenState {

bool use_lazy_refcounts;

int overlap_check;

bool discard_passthrough[QCOW2_DISCARD_MAX];

+ bool discard_no_unref;

uint64_t cache_clean_interval;

QCryptoBlockOpenOptions *crypto_opts; /* Disk encryption runtime options */

} Qcow2ReopenState;

@@ -XXX,XX +XXX,XX @@ static int qcow2_update_options_prepare(BlockDriverState *bs,

r->discard_passthrough[QCOW2_DISCARD_OTHER] =

qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false);

+ r->discard_no_unref = qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_NO_UNREF,

+ false);

+ if (r->discard_no_unref && s->qcow_version < 3) {

+ error_setg(errp,

+ "discard-no-unref is only supported since qcow2 version 3");

+ ret = -EINVAL;

+ goto fail;

+ }

switch (s->crypt_method_header) {

case QCOW_CRYPT_NONE:

if (encryptfmt) {

@@ -XXX,XX +XXX,XX @@ static void qcow2_update_options_commit(BlockDriverState *bs,

s->discard_passthrough[i] = r->discard_passthrough[i];

}

+ s->discard_no_unref = r->discard_no_unref;

if (s->cache_clean_interval != r->cache_clean_interval) {

cache_clean_timer_del(bs);

s->cache_clean_interval = r->cache_clean_interval;

diff --git a/qemu-options.hx b/qemu-options.hx

index XXXXXXX..XXXXXXX 100644

--- a/qemu-options.hx

+++ b/qemu-options.hx

@@ -XXX,XX +XXX,XX @@ SRST

issued on other occasions where a cluster gets freed

(on/off; default: off)

+ ``discard-no-unref``

+ When enabled, discards from the guest will not cause cluster

+ allocations to be relinquished. This prevents qcow2 fragmentation

+ that would be caused by such discards. Besides potential

+ performance degradation, such fragmentation can lead to increased

+ allocation of clusters past the end of the image file,

+ resulting in image files whose file length can grow much larger

+ than their guest disk size would suggest.

+ If image file length is of concern (e.g. when storing qcow2

+ images directly on block devices), you should consider enabling

+ this option.

``overlap-check``

Which overlap checks to perform for writes to the image

(none/constant/cached/all; default: cached). For details or

2.40.1

From: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

This helper will be reused in next patches during parallels_co_check

rework to simplify its code.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Reviewed-by: Denis V. Lunev <den@openvz.org>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Message-Id: <20230424093147.197643-5-alexander.ivanov@virtuozzo.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>

block/parallels.c | 11 ++++++++---

1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c

--- a/block/parallels.c

+++ b/block/parallels.c

@@ -XXX,XX +XXX,XX @@ static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,

return start_off;

+static void parallels_set_bat_entry(BDRVParallelsState *s,

+ uint32_t index, uint32_t offset)

+{

+ s->bat_bitmap[index] = cpu_to_le32(offset);

+ bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1);

+}

+

static int64_t coroutine_fn GRAPH_RDLOCK

allocate_clusters(BlockDriverState *bs, int64_t sector_num,

int nb_sectors, int *pnum)

@@ -XXX,XX +XXX,XX @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,

for (i = 0; i < to_allocate; i++) {

- s->bat_bitmap[idx + i] = cpu_to_le32(s->data_end / s->off_multiplier);

+ parallels_set_bat_entry(s, idx + i, s->data_end / s->off_multiplier);

s->data_end += s->tracks;

- bitmap_set(s->bat_dirty_bmap,

- bat_entry_off(idx + i) / s->bat_dirty_block, 1);

return bat2sect(s, idx) + sector_num % s->tracks;

2.40.1

From: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Don't let high_off be more than the file size even if we don't fix the

image.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Reviewed-by: Denis V. Lunev <den@openvz.org>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Message-Id: <20230424093147.197643-3-alexander.ivanov@virtuozzo.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>

block/parallels.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c

--- a/block/parallels.c

+++ b/block/parallels.c

@@ -XXX,XX +XXX,XX @@ parallels_co_check(BlockDriverState *bs, BdrvCheckResult *res,

fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);

res->corruptions++;

if (fix & BDRV_FIX_ERRORS) {

- prev_off = 0;

s->bat_bitmap[i] = 0;

res->corruptions_fixed++;

flush_bat = true;

- continue;

}

+ prev_off = 0;

+ continue;

res->bfi.allocated_clusters++;

2.40.1

From: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

We will add more and more checks so we need a better code structure

in parallels_co_check. Let each check performs in a separate loop

in a separate helper.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Reviewed-by: Denis V. Lunev <den@openvz.org>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Message-Id: <20230424093147.197643-7-alexander.ivanov@virtuozzo.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>

block/parallels.c | 31 +++++++++++++++++++++----------

1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c

--- a/block/parallels.c

+++ b/block/parallels.c

@@ -XXX,XX +XXX,XX @@ parallels_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors,

return ret;