[Qemu-commits] [qemu/qemu] e4918d: stream: Traverse graph after modification

[Richard Henderson]

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: e4918d9b7ddf4a5d8ab1d804c5bf17f625c298ff
      
https://github.com/qemu/qemu/commit/e4918d9b7ddf4a5d8ab1d804c5bf17f625c298ff
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block/stream.c

  Log Message:
  -----------
  stream: Traverse graph after modification

bdrv_cor_filter_drop() modifies the block graph.  That means that other
parties can also modify the block graph before it returns.  Therefore,
we cannot assume that the result of a graph traversal we did before
remains valid afterwards.

We should thus fetch `base` and `unfiltered_base` afterwards instead of
before.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211111120829.81329-2-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: 67fc2268877e18780347e234691f3f43fe991407
      
https://github.com/qemu/qemu/commit/67fc2268877e18780347e234691f3f43fe991407
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Manipulate children list in .attach/.detach

The children list is specific to BDS parents.  We should not modify it
in the general children modification code, but let BDS parents deal with
it in their .attach() and .detach() methods.

This also has the advantage that a BdrvChild is removed from the
children list before its .bs pointer can become NULL.  BDS parents
generally assume that their children's .bs pointer is never NULL, so
this is actually a bug fix.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211111120829.81329-3-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: fb6c0becd99b9d5eb24a3461732e0222386f9476
      
https://github.com/qemu/qemu/commit/fb6c0becd99b9d5eb24a3461732e0222386f9476
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Unite remove_empty_child and child_free

Now that bdrv_remove_empty_child() no longer removes the child from the
parent's children list but only checks that it is not in such a list, it
is only a wrapper around bdrv_child_free() that checks that the child is
empty and unused.  That should apply to all children that we free, so
put those checks into bdrv_child_free() and drop
bdrv_remove_empty_child().

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211111120829.81329-4-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: f57920cbd5f64222b8c8afde72d45c8dd07d467b
      
https://github.com/qemu/qemu/commit/f57920cbd5f64222b8c8afde72d45c8dd07d467b
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Drop detached child from ignore list

bdrv_attach_child_common_abort() restores the parent's AioContext.  To
do so, the child (which was supposed to be attached, but is now detached
again by this abort handler) is added to the ignore list for the
AioContext changing functions.

However, since we modify a BDS's children list in the BdrvChildClass's
.attach and .detach handlers, the child is already effectively detached
from the parent by this point.  We do not need to put it into the ignore
list.

Use this opportunity to clean up the empty line structure: Keep setting
the ignore list, invoking the AioContext function, and freeing the
ignore list in blocks separated by empty lines.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211111120829.81329-5-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: 9a9812d85d4b3f3f4af79fd4e3be7b78d690718c
      
https://github.com/qemu/qemu/commit/9a9812d85d4b3f3f4af79fd4e3be7b78d690718c
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Pass BdrvChild ** to replace_child_noperm

bdrv_replace_child_noperm() modifies BdrvChild.bs, and can potentially
set it to NULL.  That is dangerous, because BDS parents generally assume
that their children's .bs pointer is never NULL.  We therefore want to
let bdrv_replace_child_noperm() set the corresponding BdrvChild pointer
to NULL, too.

This patch lays the foundation for it by passing a BdrvChild ** pointer
to bdrv_replace_child_noperm() so that it can later use it to NULL the
BdrvChild pointer immediately after setting BdrvChild.bs to NULL.

(We will still need to undertake some intermediate steps, though.)

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211111120829.81329-6-hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: 8265c4db3f8eed17bb138d8b14477faa3fe0fd91
      
https://github.com/qemu/qemu/commit/8265c4db3f8eed17bb138d8b14477faa3fe0fd91
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Restructure remove_file_or_backing_child()

As of a future patch, bdrv_replace_child_tran() will take a BdrvChild **
pointer.  Prepare for that by getting such a pointer and using it where
applicable, and (dereferenced) as a parameter for
bdrv_replace_child_tran().

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211111120829.81329-7-hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: bc797eb37df68306fa10703ce00b952c0738c978
      
https://github.com/qemu/qemu/commit/bc797eb37df68306fa10703ce00b952c0738c978
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M include/qemu/transactions.h
    M util/transactions.c

  Log Message:
  -----------
  transactions: Invoke clean() after everything else

Invoke the transaction drivers' .clean() methods only after all
.commit() or .abort() handlers are done.

This makes it easier to have nested transactions where the top-level
transactions pass objects to lower transactions that the latter can
still use throughout their commit/abort phases, while the top-level
transaction keeps a reference that is released in its .clean() method.

(Before this commit, that is also possible, but the top-level
transaction would need to take care to invoke tran_add() before the
lower-level transaction does.  This commit makes the ordering
irrelevant, which is just a bit nicer.)

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211111120829.81329-8-hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: 444a4bcf28d233f7835b4d8f17fc276c2589a986
      
https://github.com/qemu/qemu/commit/444a4bcf28d233f7835b4d8f17fc276c2589a986
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Let replace_child_tran keep indirect pointer

As of a future commit, bdrv_replace_child_noperm() will clear the
indirect BdrvChild pointer passed to it if the new child BDS is NULL.
bdrv_replace_child_tran() will want to let it do that, but revert this
change in its abort handler.  For that, we need to have it receive a
BdrvChild ** pointer, too, and keep it stored in the
BdrvReplaceChildState object that we attach to the transaction.

Note that we do not need to store it in the BdrvReplaceChildState when
new_bs is not NULL, because then there is nothing to revert.  This is
important so that bdrv_replace_node_noperm() can pass a pointer to a
loop-local variable to bdrv_replace_child_tran() without worrying that
this pointer will outlive one loop iteration.

(Of course, for that to work, bdrv_replace_node_noperm() and in turn
bdrv_replace_node() and its relatives may not be called with a NULL @to
node.  Luckily, they already are not, but now we should assert this.)

bdrv_remove_file_or_backing_child() on the other hand needs to ensure
that the indirect pointer it passes will stay valid for the duration of
the transaction.  Ensure this by keeping a strong reference to the BDS
whose &bs->backing or &bs->file it passes to bdrv_replace_child_tran(),
and giving up that reference only in the transaction .clean() handler.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211111120829.81329-9-hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: ced051059221b32868f11bc3b38e7e5c8adc324c
      
https://github.com/qemu/qemu/commit/ced051059221b32868f11bc3b38e7e5c8adc324c
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Let replace_child_noperm free children

In most of the block layer, especially when traversing down from other
BlockDriverStates, we assume that BdrvChild.bs can never be NULL.  When
it becomes NULL, it is expected that the corresponding BdrvChild pointer
also becomes NULL and the BdrvChild object is freed.

Therefore, once bdrv_replace_child_noperm() sets the BdrvChild.bs
pointer to NULL, it should also immediately set the corresponding
BdrvChild pointer (like bs->file or bs->backing) to NULL.

In that context, it also makes sense for this function to free the
child.  Sometimes we cannot do so, though, because it is called in a
transactional context where the caller might still want to reinstate the
child in the abort branch (and free it only on commit), so this behavior
has to remain optional.

In bdrv_replace_child_tran()'s abort handler, we now rely on the fact
that the BdrvChild passed to bdrv_replace_child_tran() must have had a
non-NULL .bs pointer initially.  Make a note of that and assert it.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211111120829.81329-10-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: a43d94143071d85fe850f450369c6ee4f2976ff0
      
https://github.com/qemu/qemu/commit/a43d94143071d85fe850f450369c6ee4f2976ff0
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M tests/qemu-iotests/030

  Log Message:
  -----------
  iotests/030: Unthrottle parallel jobs in reverse

See the comment for why this is necessary.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211111120829.81329-11-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: f750d21c285fb950fce4615c808ccacb223ae216
      
https://github.com/qemu/qemu/commit/f750d21c285fb950fce4615c808ccacb223ae216
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M docs/about/deprecated.rst

  Log Message:
  -----------
  docs: Deprecate incorrectly typed device_add arguments

While introducing a non-QemuOpts code path for device creation for JSON
-device, we noticed that QMP device_add doesn't check its input
correctly (accepting arguments that should have been rejected), and that
users may be relying on this behaviour (libvirt did until it was fixed
recently).

Let's use a deprecation period before we fix this bug in QEMU to avoid
nasty surprises for users.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20211111143530.18985-1-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: b3f7c5613d873f9625f3e18863cd94d50b0a824c
      
https://github.com/qemu/qemu/commit/b3f7c5613d873f9625f3e18863cd94d50b0a824c
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block/file-posix.c
    M tests/qemu-iotests/142
    M tests/qemu-iotests/142.out

  Log Message:
  -----------
  file-posix: Fix alignment after reopen changing O_DIRECT

At the end of a reopen, we already call bdrv_refresh_limits(), which
should update bs->request_alignment according to the new file
descriptor. However, raw_probe_alignment() relies on s->needs_alignment
and just uses 1 if it isn't set. We neglected to update this field, so
starting with cache=writeback and then reopening with cache=none means
that we get an incorrect bs->request_alignment == 1 and unaligned
requests fail instead of being automatically aligned.

Fix this by recalculating s->needs_alignment in raw_refresh_limits()
before calling raw_probe_alignment().

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20211104113109.56336-1-kwolf@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: 7461272c5f6032436ef9032c091c0118539483e4
      
https://github.com/qemu/qemu/commit/7461272c5f6032436ef9032c091c0118539483e4
  Author: Stefan Hajnoczi <stefanha@redhat.com>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M softmmu/qdev-monitor.c

  Log Message:
  -----------
  softmmu/qdev-monitor: fix use-after-free in qdev_set_id()

Reported by Coverity (CID 1465222).

Fixes: 4a1d937796de0fecd8b22d7dbebf87f38e8282fd ("softmmu/qdev-monitor: add 
error handling in qdev_set_id")
Cc: Damien Hedde <damien.hedde@greensocs.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20211102163342.31162-1-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>


  Commit: 385e0da4da06f2f87c7b2420ccecc75e00c3118f
      
https://github.com/qemu/qemu/commit/385e0da4da06f2f87c7b2420ccecc75e00c3118f
  Author: Richard Henderson <richard.henderson@linaro.org>
  Date:   2021-11-15 (Mon, 15 Nov 2021)

  Changed paths:
    M block.c
    M block/file-posix.c
    M block/stream.c
    M docs/about/deprecated.rst
    M include/qemu/transactions.h
    M softmmu/qdev-monitor.c
    M tests/qemu-iotests/030
    M tests/qemu-iotests/142
    M tests/qemu-iotests/142.out
    M util/transactions.c

  Log Message:
  -----------
  Merge tag 'for-upstream' of git://repo.or.cz/qemu/kevin into staging

Block layer patches

- Fixes to image streaming job and block layer reconfiguration to make
  iotest 030 pass again
- docs: Deprecate incorrectly typed device_add arguments
- file-posix: Fix alignment after reopen changing O_DIRECT

# gpg: Signature made Mon 15 Nov 2021 03:49:52 PM CET
# gpg:                using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg:                issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]

* tag 'for-upstream' of git://repo.or.cz/qemu/kevin:
  softmmu/qdev-monitor: fix use-after-free in qdev_set_id()
  file-posix: Fix alignment after reopen changing O_DIRECT
  docs: Deprecate incorrectly typed device_add arguments
  iotests/030: Unthrottle parallel jobs in reverse
  block: Let replace_child_noperm free children
  block: Let replace_child_tran keep indirect pointer
  transactions: Invoke clean() after everything else
  block: Restructure remove_file_or_backing_child()
  block: Pass BdrvChild ** to replace_child_noperm
  block: Drop detached child from ignore list
  block: Unite remove_empty_child and child_free
  block: Manipulate children list in .attach/.detach
  stream: Traverse graph after modification

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Compare: https://github.com/qemu/qemu/compare/42f6c9179be4...385e0da4da06