qemu-commits
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 9b6abd: qom-qmp-cmds: fix two memleaks in qmp

From: Peter Maydell
Subject: [Qemu-commits] [qemu/qemu] 9b6abd: qom-qmp-cmds: fix two memleaks in qmp_object_add
Date: Thu, 12 Mar 2020 10:45:13 -0700 
  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 9b6abdcfae4401f8078f883d6eca6463c0bfbd68
      
https://github.com/qemu/qemu/commit/9b6abdcfae4401f8078f883d6eca6463c0bfbd68
  Author: Pan Nengyuan <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M qom/qom-qmp-cmds.c

  Log Message:
  -----------
  qom-qmp-cmds: fix two memleaks in qmp_object_add

'type/id' forgot to free in qmp_object_add, this patch fix that.

The leak stack:
Direct leak of 84 byte(s) in 6 object(s) allocated from:
    #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
    #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
    #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92)
    #3 0x56344954e692 in qmp_object_add 
/mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:258
    #4 0x563449960f5a in do_qmp_dispatch 
/mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132
    #5 0x563449960f5a in qmp_dispatch 
/mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175
    #6 0x563449498a30 in monitor_qmp_dispatch 
/mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145
    #7 0x56344949a64f in monitor_qmp_bh_dispatcher 
/mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234
    #8 0x563449a92a3a in aio_bh_call 
/mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136

Direct leak of 54 byte(s) in 6 object(s) allocated from:
    #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
    #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
    #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92)
    #3 0x56344954e6c4 in qmp_object_add 
/mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:267
    #4 0x563449960f5a in do_qmp_dispatch 
/mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132
    #5 0x563449960f5a in qmp_dispatch 
/mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175
    #6 0x563449498a30 in monitor_qmp_dispatch 
/mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145
    #7 0x56344949a64f in monitor_qmp_bh_dispatcher 
/mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234
    #8 0x563449a92a3a in aio_bh_call 
/mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136

Fixes: 5f07c4d60d091320186e7b0edaf9ed2cc16b2d1e
Reported-by: Euler Robot <address@hidden>
Signed-off-by: Pan Nengyuan <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Daniel P. Berrangé <address@hidden>
Acked-by: Igor Mammedov <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: c7a0f2be8f95b220cdadbba9a9236eaf115951dc
      
https://github.com/qemu/qemu/commit/c7a0f2be8f95b220cdadbba9a9236eaf115951dc
  Author: Kevin Wolf <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M block.c
    M include/block/block_int.h

  Log Message:
  -----------
  block: Make bdrv_get_cumulative_perm() public

Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Peter Krempa <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: d29d3d1f80b3947fb26e7139645c83de66d146a9
      
https://github.com/qemu/qemu/commit/d29d3d1f80b3947fb26e7139645c83de66d146a9
  Author: Kevin Wolf <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M blockdev.c
    M tests/qemu-iotests/085.out

  Log Message:
  -----------
  block: Relax restrictions for blockdev-snapshot

blockdev-snapshot returned an error if the overlay was already in use,
which it defined as having any BlockBackend parent. This is in fact both
too strict (some parents can tolerate the change of visible data caused
by attaching a backing file) and too loose (some non-BlockBackend
parents may not be happy with it).

One important use case that is prevented by the too strict check is live
storage migration with blockdev-mirror. Here, the target node is
usually opened without a backing file so that the active layer is
mirrored while its backing chain can be copied in the background.

The backing chain should be attached to the mirror target node when
finalising the job, just before switching the users of the source node
to the new copy (at which point the mirror job still has a reference to
the node). drive-mirror did this automatically, but with blockdev-mirror
this is the job of the QMP client, so it needs a way to do this.

blockdev-snapshot is the obvious way, so this patch makes it work in
this scenario. The new condition is that no parent uses CONSISTENT_READ
permissions. This will ensure that the operation will still be blocked
when the node is attached to the guest device, so blockdev-snapshot
remains safe.

(For the sake of completeness, x-blockdev-reopen can be used to achieve
the same, however it is a big hammer, performs the graph change
completely unchecked and is still experimental. So even with the option
of using x-blockdev-reopen, there are reasons why blockdev-snapshot
should be able to perform this operation.)

Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Peter Krempa <address@hidden>
Tested-by: Peter Krempa <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: b31b532122ec6f68d17168449c034d2197bf96ec
      
https://github.com/qemu/qemu/commit/b31b532122ec6f68d17168449c034d2197bf96ec
  Author: Kevin Wolf <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

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

  Log Message:
  -----------
  iotests: Fix run_job() with use_log=False

The 'job-complete' QMP command should be run with qmp() rather than
qmp_log() if use_log=False is passed.

Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Peter Krempa <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 8bdee9f10eac2aefdcc5095feef756354c87bdec
      
https://github.com/qemu/qemu/commit/8bdee9f10eac2aefdcc5095feef756354c87bdec
  Author: Kevin Wolf <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M tests/qemu-iotests/155
    M tests/qemu-iotests/155.out

  Log Message:
  -----------
  iotests: Test mirror with temporarily disabled target backing file

The newly tested scenario is a common live storage migration scenario:
The target node is opened without a backing file so that the active
layer is mirrored while its backing chain can be copied in the
background.

The backing chain should be attached to the mirror target node when
finalising the job, just before switching the users of the source node
to the new copy (at which point the mirror job still has a reference to
the node). drive-mirror did this automatically, but with blockdev-mirror
this is the job of the QMP client.

This patch adds test cases for two ways to achieve the desired result,
using either x-blockdev-reopen or blockdev-snapshot.

Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Peter Krempa <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 30dd65f307b647eef8156c4a33bd007823ef85cb
      
https://github.com/qemu/qemu/commit/30dd65f307b647eef8156c4a33bd007823ef85cb
  Author: Kevin Wolf <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M block.c
    M blockdev.c

  Log Message:
  -----------
  block: Fix cross-AioContext blockdev-snapshot

external_snapshot_prepare() tries to move the overlay to the AioContext
of the backing file (the snapshotted node). However, it's possible that
this doesn't work, but the backing file can instead be moved to the
overlay's AioContext (e.g. opening the backing chain for a mirror
target).

bdrv_append() already indirectly uses bdrv_attach_node(), which takes
care to move nodes to make sure they use the same AioContext and which
tries both directions.

So the problem has a simple fix: Just delete the unnecessary extra
bdrv_try_set_aio_context() call in external_snapshot_prepare() and
instead assert in bdrv_append() that both nodes were indeed moved to the
same AioContext.

Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Tested-by: Peter Krempa <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 6a5f6403a11307794ec79d277a065c137cfc12b2
      
https://github.com/qemu/qemu/commit/6a5f6403a11307794ec79d277a065c137cfc12b2
  Author: Kevin Wolf <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M tests/qemu-iotests/155
    M tests/qemu-iotests/155.out

  Log Message:
  -----------
  iotests: Add iothread cases to 155

This patch adds test cases for attaching the backing chain to a mirror
job target right before finalising the job, where the image is in a
non-mainloop AioContext (i.e. the backing chain needs to be moved to the
AioContext of the mirror target).

This requires switching the test case from virtio-blk to virtio-scsi
because virtio-blk only actually starts using the iothreads when the
guest driver initialises the device (which never happens in a test case
without a guest OS). virtio-scsi always keeps its block nodes in the
AioContext of the the requested iothread without guest interaction.

Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Peter Krempa <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: c6bdc312f30d5c7326aa2fdca3e0f98c15eb541a
      
https://github.com/qemu/qemu/commit/c6bdc312f30d5c7326aa2fdca3e0f98c15eb541a
  Author: Peter Krempa <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M qapi/block-core.json

  Log Message:
  -----------
  qapi: Add '@allow-write-only-overlay' feature for 'blockdev-snapshot'

Anounce that 'blockdev-snapshot' command's permissions allow changing
of the backing file if the 'consistent_read' permission is not required.

This is useful for libvirt to allow late opening of the backing chain
during a blockdev-mirror.

Signed-off-by: Peter Krempa <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 6e1da96b946a1a38d3517564af4e2f572c3ccf4b
      
https://github.com/qemu/qemu/commit/6e1da96b946a1a38d3517564af4e2f572c3ccf4b
  Author: Philippe Mathieu-Daudé <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M tests/Makefile.include
    M tests/qtest/Makefile.include

  Log Message:
  -----------
  tests/qemu-iotests: Fix socket_scm_helper build path

The socket_scm_helper path got corrupted during the mechanical
refactor moving the qtests files into their own sub-directory.

Fixes: 1e8a1fae7 ("test: Move qtests to a separate directory")
Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 9bffae14df879255329473a7bd578643af2d4c9c
      
https://github.com/qemu/qemu/commit/9bffae14df879255329473a7bd578643af2d4c9c
  Author: Daniel Henrique Barboza <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M block/file-posix.c
    M include/block/block_int.h

  Log Message:
  -----------
  block: introducing 'bdrv_co_delete_file' interface

Adding to Block Drivers the capability of being able to clean up
its created files can be useful in certain situations. For the
LUKS driver, for instance, a failure in one of its authentication
steps can leave files in the host that weren't there before.

This patch adds the 'bdrv_co_delete_file' interface to block
drivers and add it to the 'file' driver in file-posix.c. The
implementation is given by 'raw_co_delete_file'.

Suggested-by: Daniel P. Berrangé <address@hidden>
Signed-off-by: Daniel Henrique Barboza <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: e1d7f8bb1ec0c6911dcea81641ce6139dbded02d
      
https://github.com/qemu/qemu/commit/e1d7f8bb1ec0c6911dcea81641ce6139dbded02d
  Author: Daniel Henrique Barboza <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M block.c
    M include/block/block.h

  Log Message:
  -----------
  block.c: adding bdrv_co_delete_file

Using the new 'bdrv_co_delete_file' interface, a pure co_routine function
'bdrv_co_delete_file' inside block.c can can be used in a way similar of
the existing bdrv_create_file to to clean up a created file.

We're creating a pure co_routine because the only caller of
'bdrv_co_delete_file' will be already in co_routine context, thus there
is no need to add all the machinery to check for qemu_in_coroutine() and
create a separated co_routine to do the job.

Suggested-by: Daniel P. Berrangé <address@hidden>
Signed-off-by: Daniel Henrique Barboza <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 1bba30da24e1124ceeb0693c81382a0d77e20ca5
      
https://github.com/qemu/qemu/commit/1bba30da24e1124ceeb0693c81382a0d77e20ca5
  Author: Daniel Henrique Barboza <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

  Changed paths:
    M block/crypto.c

  Log Message:
  -----------
  crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails

When using a non-UTF8 secret to create a volume using qemu-img, the
following error happens:

$ qemu-img create -f luks --object 
secret,id=vol_1_encrypt0,file=vol_resize_pool.vol_1.secret.qzVQrI -o 
key-secret=vol_1_encrypt0 /var/tmp/pool_target/vol_1 10240K

Formatting '/var/tmp/pool_target/vol_1', fmt=luks size=10485760 
key-secret=vol_1_encrypt0
qemu-img: /var/tmp/pool_target/vol_1: Data from secret vol_1_encrypt0 is not 
valid UTF-8

However, the created file '/var/tmp/pool_target/vol_1' is left behind in the
file system after the failure. This behavior can be observed when creating
the volume using Libvirt, via 'virsh vol-create', and then getting "volume
target path already exist" errors when trying to re-create the volume.

The volume file is created inside block_crypto_co_create_opts_luks(), in
block/crypto.c. If the bdrv_create_file() call is successful but any
succeeding step fails*, the existing 'fail' label does not take into
account the created file, leaving it behind.

This patch changes block_crypto_co_create_opts_luks() to delete
'filename' in case of failure. A failure in this point means that
the volume is now truncated/corrupted, so even if 'filename' was an
existing volume before calling qemu-img, it is now unusable. Deleting
the file it is not much worse than leaving it in the filesystem in
this scenario, and we don't have to deal with checking the file
pre-existence in the code.

* in our case, block_crypto_co_create_generic calls qcrypto_block_create,
which calls qcrypto_block_luks_create, and this function fails when
calling qcrypto_secret_lookup_as_utf8.

Reported-by: Srikanth Aithal <address@hidden>
Suggested-by: Kevin Wolf <address@hidden>
Signed-off-by: Daniel Henrique Barboza <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 8bb3b023f2055054ee119cb45b42d2b14be7fc8a
      
https://github.com/qemu/qemu/commit/8bb3b023f2055054ee119cb45b42d2b14be7fc8a
  Author: Daniel Henrique Barboza <address@hidden>
  Date:   2020-03-11 (Wed, 11 Mar 2020)

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

  Log Message:
  -----------
  qemu-iotests: adding LUKS cleanup for non-UTF8 secret error

This patch adds a new test file to exercise the case where
qemu-img fails to complete for the LUKS format when a non-UTF8
secret is used.

Signed-off-by: Daniel Henrique Barboza <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 49780a582d8bcedf098237f8997214c8424124be
      
https://github.com/qemu/qemu/commit/49780a582d8bcedf098237f8997214c8424124be
  Author: Peter Maydell <address@hidden>
  Date:   2020-03-12 (Thu, 12 Mar 2020)

  Changed paths:
    M block.c
    M block/crypto.c
    M block/file-posix.c
    M blockdev.c
    M include/block/block.h
    M include/block/block_int.h
    M qapi/block-core.json
    M qom/qom-qmp-cmds.c
    M tests/Makefile.include
    M tests/qemu-iotests/085.out
    M tests/qemu-iotests/155
    M tests/qemu-iotests/155.out
    A tests/qemu-iotests/282
    A tests/qemu-iotests/282.out
    M tests/qemu-iotests/group
    M tests/qemu-iotests/iotests.py
    M tests/qtest/Makefile.include

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging

Block layer patches:

- Relax restrictions for blockdev-snapshot (allows libvirt to do live
  storage migration with blockdev-mirror)
- luks: Delete created files when block_crypto_co_create_opts_luks fails
- Fix memleaks in qmp_object_add

# gpg: Signature made Wed 11 Mar 2020 15:38:59 GMT
# gpg:                using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <address@hidden>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream:
  qemu-iotests: adding LUKS cleanup for non-UTF8 secret error
  crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails
  block.c: adding bdrv_co_delete_file
  block: introducing 'bdrv_co_delete_file' interface
  tests/qemu-iotests: Fix socket_scm_helper build path
  qapi: Add '@allow-write-only-overlay' feature for 'blockdev-snapshot'
  iotests: Add iothread cases to 155
  block: Fix cross-AioContext blockdev-snapshot
  iotests: Test mirror with temporarily disabled target backing file
  iotests: Fix run_job() with use_log=False
  block: Relax restrictions for blockdev-snapshot
  block: Make bdrv_get_cumulative_perm() public
  qom-qmp-cmds: fix two memleaks in qmp_object_add

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


Compare: https://github.com/qemu/qemu/compare/10b114008acc...49780a582d8b
reply via email to
[Prev in Thread] Current Thread [Next in Thread]