The following changes since commit 418fa86dd465b4fd8394373cf83db8fa65d7611c:

Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-040220-1' into staging (2020-02-04 18:55:06 +0000)

https://github.com/XanClic/qemu.git tags/pull-block-2020-02-06

for you to fetch changes up to a541fcc27c98b96da187c7d4573f3270f3ddd283:

iotests: add test for backup-top failure on permission activation (2020-02-06 13:47:45 +0100)

Block patches:

- Drop BDRV_SECTOR_SIZE from qcow2

- Allow Python iotests to be added to the auto group

(and add some)

- Fix for the backup job

- Fix memleak in bdrv_refresh_filename()

- Use GStrings in two places for greater efficiency (than manually

handling string allocation)

John Snow (1):

iotests: remove 'linux' from default supported platforms

Pan Nengyuan (1):

block: fix memleaks in bdrv_refresh_filename

Thomas Huth (5):

iotests: Test 041 only works on certain systems

iotests: Test 183 does not work on macOS and OpenBSD

iotests: Check for the availability of the required devices in 267 and

127

iotests: Skip Python-based tests if QEMU does not support virtio-blk

iotests: Enable more tests in the 'auto' group to improve test

coverage

Vladimir Sementsov-Ogievskiy (2):

block/backup-top: fix failure path

iotests: add test for backup-top failure on permission activation

block.c | 12 +++--

block/backup-top.c | 21 ++++----

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

block/qcow2-refcount.c | 2 +-

block/qcow2-snapshot.c | 3 +-

block/qcow2.c | 46 ++++++++----------

tests/qemu-iotests/041 | 3 +-

tests/qemu-iotests/127 | 2 +

tests/qemu-iotests/183 | 1 +

tests/qemu-iotests/267 | 2 +

tests/qemu-iotests/283 | 92 +++++++++++++++++++++++++++++++++++

tests/qemu-iotests/283.out | 8 +++

tests/qemu-iotests/check | 12 ++++-

tests/qemu-iotests/common.rc | 14 ++++++

tests/qemu-iotests/group | 15 +++---

tests/qemu-iotests/iotests.py | 16 ++++--

16 files changed, 220 insertions(+), 73 deletions(-)

create mode 100644 tests/qemu-iotests/283

create mode 100644 tests/qemu-iotests/283.out

2.24.1

From: Alberto Garcia <berto@igalia.com>

qemu-img's convert_co_copy_range() operates at the sector level and

block_copy() operates at the cluster level so this condition is always

true, but it is not necessary to restrict this here, so let's leave it

to the driver implementation return an error if there is any.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Message-id: a4264aaee656910c84161a2965f7a501437379ca.1579374329.git.berto@igalia.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

block/qcow2.c | 4 ----

1 file changed, 4 deletions(-)

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

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_from(BlockDriverState *bs,

case QCOW2_CLUSTER_NORMAL:

child = s->data_file;

copy_offset += offset_into_cluster(s, src_offset);

- if ((copy_offset & 511) != 0) {

- ret = -EIO;

- goto out;

- }

break;

default:

2.24.1

From: Alberto Garcia <berto@igalia.com>

This replaces all remaining instances in the qcow2 code.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Message-id: b5f74b606c2d9873b12d29acdb7fd498029c4025.1579374329.git.berto@igalia.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

block/qcow2.c | 8 +++++---

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

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

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)

/* Validate options and set default values */

if (!QEMU_IS_ALIGNED(qcow2_opts->size, BDRV_SECTOR_SIZE)) {

- error_setg(errp, "Image size must be a multiple of 512 bytes");

+ error_setg(errp, "Image size must be a multiple of %u bytes",

+ (unsigned) BDRV_SECTOR_SIZE);

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,

return -ENOTSUP;

- if (offset & 511) {

- error_setg(errp, "The new size must be a multiple of 512");

+ if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {

+ error_setg(errp, "The new size must be a multiple of %u",

+ (unsigned) BDRV_SECTOR_SIZE);

return -EINVAL;

}

2.24.1

From: Alberto Garcia <berto@igalia.com>

qcow2_alloc_cluster_offset() and qcow2_get_cluster_offset() always

return offsets that are cluster-aligned so don't just check that they

are sector-aligned.

The check in qcow2_co_preadv_task() is also replaced by an assertion

for the same reason.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: 558ba339965f858bede4c73ce3f50f0c0493597d.1579374329.git.berto@igalia.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

block/qcow2.c | 9 +++------

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

From: Alberto Garcia <berto@igalia.com>

The L1 table is read from disk using the byte-based bdrv_pread() and

is never accessed beyond its last element, so there's no need to

allocate more memory than that.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: b2e27214ec7b03a585931bcf383ee1ac3a641a10.1579374329.git.berto@igalia.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

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

block/qcow2-refcount.c | 2 +-

block/qcow2-snapshot.c | 3 +--

block/qcow2.c | 2 +-

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

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

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

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

@@ -XXX,XX +XXX,XX @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,

#endif

new_l1_size2 = sizeof(uint64_t) * new_l1_size;

- new_l1_table = qemu_try_blockalign(bs->file->bs,

- ROUND_UP(new_l1_size2, 512));

+ new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_size2);

if (new_l1_table == NULL) {

return -ENOMEM;

- memset(new_l1_table, 0, ROUND_UP(new_l1_size2, 512));

+ memset(new_l1_table, 0, new_l1_size2);

if (s->l1_size) {

memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));

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

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

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

@@ -XXX,XX +XXX,XX @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,

* l1_table_offset when it is the current s->l1_table_offset! Be careful

* when changing this! */

if (l1_table_offset != s->l1_table_offset) {

- l1_table = g_try_malloc0(ROUND_UP(l1_size2, 512));

+ l1_table = g_try_malloc0(l1_size2);

if (l1_size2 && l1_table == NULL) {

ret = -ENOMEM;

goto fail;

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

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-snapshot.c

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

@@ -XXX,XX +XXX,XX @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,

return ret;

}

new_l1_bytes = sn->l1_size * sizeof(uint64_t);

- new_l1_table = qemu_try_blockalign(bs->file->bs,

- ROUND_UP(new_l1_bytes, 512));

+ new_l1_table = qemu_try_blockalign(bs->file->bs, new_l1_bytes);

if (new_l1_table == NULL) {

return -ENOMEM;

}

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 int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,

if (s->l1_size > 0) {

s->l1_table = qemu_try_blockalign(bs->file->bs,

- ROUND_UP(s->l1_size * sizeof(uint64_t), 512));

+ s->l1_size * sizeof(uint64_t));

if (s->l1_table == NULL) {

error_setg(errp, "Could not allocate L1 table");

ret = -ENOMEM;

2.24.1

From: Thomas Huth <thuth@redhat.com>

According to Kevin, tests 030, 040 and 041 are among the most valuable

tests that we have, so we should always run them if possible, even if

they take a little bit longer.

According to Max, it would be good to have a test for iothreads and

migration. 127 and 256 seem to be good candidates for iothreads. For

migration, let's enable 181 and 203 (which also tests iothreads).

(091 would be a good candidate for migration, too, but Alex Bennée

reported that this test fails on ZFS file systems, so it can't be

included yet)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Thomas Huth <thuth@redhat.com>

Message-id: 20200121095205.26323-7-thuth@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/group | 14 +++++++-------

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

From: Thomas Huth <thuth@redhat.com>

We are going to enable some of the python-based tests in the "auto" group,

and these tests require virtio-blk to work properly. Running iotests

without virtio-blk likely does not make too much sense anyway, so instead

of adding a check for the availability of virtio-blk to each and every

test (which does not sound very appealing), let's rather add a check for

this a central spot in the "check" script instead (so that it is still

possible to run "make check" for qemu-system-tricore for example).

Signed-off-by: Thomas Huth <thuth@redhat.com>

Message-id: 20200121095205.26323-6-thuth@redhat.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

tests/qemu-iotests/check | 12 ++++++++++--

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

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/check

+++ b/tests/qemu-iotests/check

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

python_usable=false

if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'

then

- python_usable=true

+ # Our python framework also requires virtio-blk

+ if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1

+ then

+ python_usable=true

+ else

+ python_unusable_because="Missing virtio-blk in QEMU binary"

+ fi

+else

+ python_unusable_because="Unsupported Python version"

fi

default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')

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

run_command="$PYTHON $seq"

else

run_command="false"

- echo "Unsupported Python version" > $seq.notrun

+ echo "$python_unusable_because" > $seq.notrun

fi

else

run_command="./$seq"

2.24.1

From: Alberto Garcia <berto@igalia.com>

This is a bit more efficient than having to allocate and free memory

for each item.

The default size (60) is enough for all the existing incompatible

features or the "Unknown incompatible feature" message.

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Signed-off-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Message-id: 20200115135626.19442-1-berto@igalia.com

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

block/qcow2.c | 23 +++++++++++------------

1 file changed, 11 insertions(+), 12 deletions(-)

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

--- a/block/qcow2.c

+++ b/block/qcow2.c

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

static void report_unsupported_feature(Error **errp, Qcow2Feature *table,

uint64_t mask)

- char *features = g_strdup("");

- char *old;

+ g_autoptr(GString) features = g_string_sized_new(60);

while (table && table->name[0] != '\0') {

if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {

if (mask & (1ULL << table->bit)) {

- old = features;

- features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",

- table->name);

- g_free(old);

+ if (features->len > 0) {

+ g_string_append(features, ", ");

+ }

+ g_string_append_printf(features, "%.46s", table->name);

mask &= ~(1ULL << table->bit);

@@ -XXX,XX +XXX,XX @@ static void report_unsupported_feature(Error **errp, Qcow2Feature *table,

if (mask) {

- old = features;

- features = g_strdup_printf("%s%sUnknown incompatible feature: %" PRIx64,

- old, *old ? ", " : "", mask);

- g_free(old);

+ if (features->len > 0) {

+ g_string_append(features, ", ");

+ }

+ g_string_append_printf(features,

+ "Unknown incompatible feature: %" PRIx64, mask);

- error_setg(errp, "Unsupported qcow2 feature(s): %s", features);

- g_free(features);

+ error_setg(errp, "Unsupported qcow2 feature(s): %s", features->str);

/*

2.24.1

From: Alberto Garcia <berto@igalia.com>

When updating an L1 entry the qcow2 driver writes a (512-byte) sector

worth of data to avoid a read-modify-write cycle. Instead of always

writing 512 bytes we should follow the alignment requirements of the

storage backend.

(the only exception is when the alignment is larger than the cluster

size because then we could be overwriting data after the L1 table)

Signed-off-by: Alberto Garcia <berto@igalia.com>

Message-id: 71f34d4ae4b367b32fb36134acbf4f4f7ee681f4.1579374329.git.berto@igalia.com

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

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

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

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

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

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

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

- * Writes one sector of the L1 table to the disk (can't update single entries

- * and we really don't want bdrv_pread to perform a read-modify-write)

+ * Writes an L1 entry to disk (note that depending on the alignment

+ * requirements this function may write more that just one entry in

+ * order to prevent bdrv_pwrite from performing a read-modify-write)

-#define L1_ENTRIES_PER_SECTOR (512 / 8)

int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)

BDRVQcow2State *s = bs->opaque;

- uint64_t buf[L1_ENTRIES_PER_SECTOR] = { 0 };

int l1_start_index;

int i, ret;

+ int bufsize = MAX(sizeof(uint64_t),

+ MIN(bs->file->bs->bl.request_alignment, s->cluster_size));

+ int nentries = bufsize / sizeof(uint64_t);

+ g_autofree uint64_t *buf = g_try_new0(uint64_t, nentries);

- l1_start_index = l1_index & ~(L1_ENTRIES_PER_SECTOR - 1);

- for (i = 0; i < L1_ENTRIES_PER_SECTOR && l1_start_index + i < s->l1_size;

- i++)

- {

+ if (buf == NULL) {

+ return -ENOMEM;

+ }

+

+ l1_start_index = QEMU_ALIGN_DOWN(l1_index, nentries);

+ for (i = 0; i < MIN(nentries, s->l1_size - l1_start_index); i++) {

buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]);

}

ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1,

- s->l1_table_offset + 8 * l1_start_index, sizeof(buf), false);

+ s->l1_table_offset + 8 * l1_start_index, bufsize, false);

@@ -XXX,XX +XXX,XX @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index)

BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);

ret = bdrv_pwrite_sync(bs->file,

s->l1_table_offset + 8 * l1_start_index,

- buf, sizeof(buf));

+ buf, bufsize);

2.24.1

From: Alberto Garcia <berto@igalia.com>

This is a bit more efficient than having to allocate and free memory

for each new permission.

The default size (30) is enough for "consistent read, write, resize".

Signed-off-by: Alberto Garcia <berto@igalia.com>

Message-id: 20200110171518.22168-1-berto@igalia.com

Signed-off-by: Max Reitz <mreitz@redhat.com>

block.c | 11 ++++++-----

1 file changed, 6 insertions(+), 5 deletions(-)

@@ -XXX,XX +XXX,XX @@ char *bdrv_perm_names(uint64_t perm)

{ 0, NULL }

};

- char *result = g_strdup("");

+ GString *result = g_string_sized_new(30);

struct perm_name *p;

for (p = permissions; p->name; p++) {

if (perm & p->perm) {

- char *old = result;

- result = g_strdup_printf("%s%s%s", old, *old ? ", " : "", p->name);

- g_free(old);

+ if (result->len > 0) {

+ g_string_append(result, ", ");

+ }

+ g_string_append(result, p->name);

}

}

- return result;

+ return g_string_free(result, FALSE);