qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 3a75a8: qcow2: Assert that host cluster offse


From: Peter Maydell
Subject: [Qemu-commits] [qemu/qemu] 3a75a8: qcow2: Assert that host cluster offsets fit in L2 ...
Date: Thu, 06 Feb 2020 11:00:17 -0800

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 3a75a870ef51f79c2b5f7fc8f7756f9efb500a14
      
https://github.com/qemu/qemu/commit/3a75a870ef51f79c2b5f7fc8f7756f9efb500a14
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/qcow2-cluster.c

  Log Message:
  -----------
  qcow2: Assert that host cluster offsets fit in L2 table entries

The standard cluster descriptor in L2 table entries has a field to
store the host cluster offset. When we need to get that offset from an
entry we use L2E_OFFSET_MASK to ensure that we only use the bits that
belong to that field.

But while that mask is used every time we read from an L2 entry, it
is never used when we write to it. Due to the QCOW_MAX_CLUSTER_OFFSET
limit set in the cluster allocation code QEMU can never produce
offsets that don't fit in that field so any such offset would indicate
a bug in QEMU.

Compressed cluster descriptors contain two fields (host cluster offset
and size of the compressed data) and the situation with them is
similar. In this case the masks are not constant but are stored in the
csize_mask and cluster_offset_mask fields of BDRVQcow2State.

Signed-off-by: Alberto Garcia <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: e2a7423a119660f33abcbbf4b7827a81ef23872f
      
https://github.com/qemu/qemu/commit/e2a7423a119660f33abcbbf4b7827a81ef23872f
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Use a GString in bdrv_perm_names()

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 <address@hidden>
Message-id: address@hidden
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: cb8956144ccaccf23d5cc4167677e2c84fa5a9f8
      
https://github.com/qemu/qemu/commit/cb8956144ccaccf23d5cc4167677e2c84fa5a9f8
  Author: Pan Nengyuan <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: fix memleaks in bdrv_refresh_filename

If we call the qmp 'query-block' while qemu is working on
'block-commit', it will cause memleaks, the memory leak stack is as
follow:

Indirect leak of 12360 byte(s) in 3 object(s) allocated from:
    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
    #3 0x55ea956cd043 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6427
    #4 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #5 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #6 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #7 0x55ea958818ea in bdrv_block_device_info 
/mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:56
    #8 0x55ea958879de in bdrv_query_info 
/mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:392
    #9 0x55ea9588b58f in qmp_query_block 
/mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:578
    #10 0x55ea95567392 in qmp_marshal_query_block 
qapi/qapi-commands-block-core.c:95

Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
    #0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
    #3 0x55ea956cd043 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6427
    #4 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #5 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
    #6 0x55ea9569f301 in bdrv_backing_attach 
/mnt/sdb/qemu-4.2.0-rc0/block.c:1064
    #7 0x55ea956a99dd in bdrv_replace_child_noperm 
/mnt/sdb/qemu-4.2.0-rc0/block.c:2283
    #8 0x55ea956b9b53 in bdrv_replace_node /mnt/sdb/qemu-4.2.0-rc0/block.c:4196
    #9 0x55ea956b9e49 in bdrv_append /mnt/sdb/qemu-4.2.0-rc0/block.c:4236
    #10 0x55ea958c3472 in commit_start 
/mnt/sdb/qemu-4.2.0-rc0/block/commit.c:306
    #11 0x55ea94b68ab0 in qmp_block_commit 
/mnt/sdb/qemu-4.2.0-rc0/blockdev.c:3459
    #12 0x55ea9556a7a7 in qmp_marshal_block_commit 
qapi/qapi-commands-block-core.c:407

Fixes: bb808d5f5c0978828a974d547e6032402c339555
Reported-by: Euler Robot <address@hidden>
Signed-off-by: Pan Nengyuan <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: 7cdca2e2337e82953495b907c60df3115a268428
      
https://github.com/qemu/qemu/commit/7cdca2e2337e82953495b907c60df3115a268428
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/qcow2.c

  Log Message:
  -----------
  qcow2: Use a GString in report_unsupported_feature()

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é <address@hidden>
Signed-off-by: Alberto Garcia <address@hidden>
Reviewed-by: Alex Bennée <address@hidden>
Message-id: address@hidden
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Reviewed-by: Stefano Garzarella <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 72b2903056afc629b0e4d28b3a7d6bb3c7b36136
      
https://github.com/qemu/qemu/commit/72b2903056afc629b0e4d28b3a7d6bb3c7b36136
  Author: John Snow <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M tests/qemu-iotests/iotests.py

  Log Message:
  -----------
  iotests: remove 'linux' from default supported platforms

verify_platform will check an explicit whitelist and blacklist instead.
The default will now be assumed to be allowed to run anywhere.

For tests that do not specify their platforms explicitly, this has the effect of
enabling these tests on non-linux platforms. For tests that always specified
linux explicitly, there is no change.

For Python tests on FreeBSD at least; only seven python tests fail:
045 147 149 169 194 199 211

045 and 149 appear to be misconfigurations,
147 and 194 are the AF_UNIX path too long error,
169 and 199 are bitmap migration bugs, and
211 is a bug that shows up on Linux platforms, too.

This is at least good evidence that these tests are not Linux-only. If
they aren't suitable for other platforms, they should be disabled on a
per-platform basis as appropriate.

Therefore, let's switch these on and deal with the failures.

Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: 877d18f2aa240672333f44651e67fd4ebb94eeb8
      
https://github.com/qemu/qemu/commit/877d18f2aa240672333f44651e67fd4ebb94eeb8
  Author: Thomas Huth <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M tests/qemu-iotests/041

  Log Message:
  -----------
  iotests: Test 041 only works on certain systems

041 works fine on Linux, FreeBSD, NetBSD and OpenBSD, but fails on macOS.
Let's mark it as only supported on the systems where we know that it is
working fine.

Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: Thomas Huth <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: 30ad36f55f4734ea663ddc21ea0910f2c7b4423c
      
https://github.com/qemu/qemu/commit/30ad36f55f4734ea663ddc21ea0910f2c7b4423c
  Author: Thomas Huth <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M tests/qemu-iotests/183

  Log Message:
  -----------
  iotests: Test 183 does not work on macOS and OpenBSD

In the long run, we might want to add test 183 to the "auto" group
(but it still fails occasionally, so we cannot do that yet). However,
when running 183 in Cirrus-CI on macOS, or with our vm-build-openbsd
target, it currently always fails with an "Timeout waiting for return
on handle 0" error.

Let's mark it as supported only on systems where the test is working
most of the time (i.e. Linux, FreeBSD and NetBSD).

Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: Thomas Huth <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: 9bdabfbe722e4d47892dfea17ae4c1670e54123b
      
https://github.com/qemu/qemu/commit/9bdabfbe722e4d47892dfea17ae4c1670e54123b
  Author: Thomas Huth <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M tests/qemu-iotests/127
    M tests/qemu-iotests/267
    M tests/qemu-iotests/common.rc

  Log Message:
  -----------
  iotests: Check for the availability of the required devices in 267 and 127

We are going to enable 127 in the "auto" group, but it only works if
virtio-scsi and scsi-hd are available - which is not the case with
QEMU binaries like qemu-system-tricore for example, so we need a
proper check for the availability of these devices here.

A very similar problem exists in iotest 267 - it has been added to
the "auto" group already, but requires virtio-blk and thus currently
fails with qemu-system-tricore for example. Let's also add aproper
check there.

Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: Thomas Huth <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: cd2058289bbe0437c2818f69a8c25a7618e906bd
      
https://github.com/qemu/qemu/commit/cd2058289bbe0437c2818f69a8c25a7618e906bd
  Author: Thomas Huth <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M tests/qemu-iotests/check

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

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 <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: ce95a15e4249e6b0c014e7af708a0a8e65cecfc5
      
https://github.com/qemu/qemu/commit/ce95a15e4249e6b0c014e7af708a0a8e65cecfc5
  Author: Thomas Huth <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M tests/qemu-iotests/group

  Log Message:
  -----------
  iotests: Enable more tests in the 'auto' group to improve test coverage

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 <address@hidden>
Signed-off-by: Thomas Huth <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: ef97d608c7f069331d8141d1d59df715f4a3beaf
      
https://github.com/qemu/qemu/commit/ef97d608c7f069331d8141d1d59df715f4a3beaf
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/qcow2-cluster.c
    M block/qcow2-refcount.c
    M block/qcow2-snapshot.c
    M block/qcow2.c

  Log Message:
  -----------
  qcow2: Don't round the L1 table allocation up to the sector size

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 <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: 344ffea951aa4aa4259f24c885ba00e768083f09
      
https://github.com/qemu/qemu/commit/344ffea951aa4aa4259f24c885ba00e768083f09
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/qcow2.c

  Log Message:
  -----------
  qcow2: Tighten cluster_offset alignment assertions

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 <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: da86f8cbad30ed3819ee5fd19a3b19291459c768
      
https://github.com/qemu/qemu/commit/da86f8cbad30ed3819ee5fd19a3b19291459c768
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/qcow2-cluster.c

  Log Message:
  -----------
  qcow2: Use bs->bl.request_alignment when updating an L1 entry

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 <address@hidden>
Message-id: address@hidden
Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 25ae71db55ebb06bfa501f17dabb96ff3b34921b
      
https://github.com/qemu/qemu/commit/25ae71db55ebb06bfa501f17dabb96ff3b34921b
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/qcow2.c

  Log Message:
  -----------
  qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()

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 <address@hidden>
Message-id: address@hidden
Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 3afea40243d7ccba7b569daa4dc7b174e594f792
      
https://github.com/qemu/qemu/commit/3afea40243d7ccba7b569daa4dc7b174e594f792
  Author: Alberto Garcia <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/qcow2.c

  Log Message:
  -----------
  qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value

This replaces all remaining instances in the qcow2 code.

Signed-off-by: Alberto Garcia <address@hidden>
Message-id: address@hidden
Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: Max Reitz <address@hidden>


  Commit: 0df62f45c1de6c020f1e6fba4eeafd248209b003
      
https://github.com/qemu/qemu/commit/0df62f45c1de6c020f1e6fba4eeafd248209b003
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block/backup-top.c

  Log Message:
  -----------
  block/backup-top: fix failure path

We can't access top after call bdrv_backup_top_drop, as it is already
freed at this time.

Also, no needs to unref target child by hand, it will be unrefed on
bdrv_close() automatically.

So, just do bdrv_backup_top_drop if append succeed and one bdrv_unref
otherwise.

Note, that in !appended case bdrv_unref(top) moved into drained section
on source. It doesn't really matter, but just for code simplicity.

Fixes: 7df7868b96404
Cc: address@hidden # v4.2.0
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: a541fcc27c98b96da187c7d4573f3270f3ddd283
      
https://github.com/qemu/qemu/commit/a541fcc27c98b96da187c7d4573f3270f3ddd283
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    A tests/qemu-iotests/283
    A tests/qemu-iotests/283.out
    M tests/qemu-iotests/group

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

This test checks that bug is really fixed by previous commit.

Cc: address@hidden # v4.2.0
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>


  Commit: 863d2ed5823f90c42dcd481687cc99cbc9c4a17c
      
https://github.com/qemu/qemu/commit/863d2ed5823f90c42dcd481687cc99cbc9c4a17c
  Author: Peter Maydell <address@hidden>
  Date:   2020-02-06 (Thu, 06 Feb 2020)

  Changed paths:
    M block.c
    M block/backup-top.c
    M block/qcow2-cluster.c
    M block/qcow2-refcount.c
    M block/qcow2-snapshot.c
    M block/qcow2.c
    M tests/qemu-iotests/041
    M tests/qemu-iotests/127
    M tests/qemu-iotests/183
    M tests/qemu-iotests/267
    A tests/qemu-iotests/283
    A tests/qemu-iotests/283.out
    M tests/qemu-iotests/check
    M tests/qemu-iotests/common.rc
    M tests/qemu-iotests/group
    M tests/qemu-iotests/iotests.py

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-02-06' 
into staging

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)

# gpg: Signature made Thu 06 Feb 2020 12:50:14 GMT
# gpg:                using RSA key 91BEB60A30DB3E8857D11829F407DB0061D5CF40
# gpg:                issuer "address@hidden"
# gpg: Good signature from "Max Reitz <address@hidden>" [full]
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* remotes/maxreitz/tags/pull-block-2020-02-06:
  iotests: add test for backup-top failure on permission activation
  block/backup-top: fix failure path
  qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value
  qcow2: Don't require aligned offsets in qcow2_co_copy_range_from()
  qcow2: Use bs->bl.request_alignment when updating an L1 entry
  qcow2: Tighten cluster_offset alignment assertions
  qcow2: Don't round the L1 table allocation up to the sector size
  iotests: Enable more tests in the 'auto' group to improve test coverage
  iotests: Skip Python-based tests if QEMU does not support virtio-blk
  iotests: Check for the availability of the required devices in 267 and 127
  iotests: Test 183 does not work on macOS and OpenBSD
  iotests: Test 041 only works on certain systems
  iotests: remove 'linux' from default supported platforms
  qcow2: Use a GString in report_unsupported_feature()
  block: fix memleaks in bdrv_refresh_filename
  block: Use a GString in bdrv_perm_names()
  qcow2: Assert that host cluster offsets fit in L2 table entries

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/2021b7c9716c...863d2ed5823f



reply via email to

[Prev in Thread] Current Thread [Next in Thread]