qemu-commits
[Top][All Lists]
Advanced

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

[Qemu-commits] [qemu/qemu] 8d3dd0: stream: Traverse graph after modifica


From: Richard Henderson
Subject: [Qemu-commits] [qemu/qemu] 8d3dd0: stream: Traverse graph after modification
Date: Tue, 16 Nov 2021 06:45:05 -0800

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 8d3dd037d947ce96c42c376ef0bb5e5c6ef96cbe
      
https://github.com/qemu/qemu/commit/8d3dd037d947ce96c42c376ef0bb5e5c6ef96cbe
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-2-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: a225369bce684c01095be52e1014e3fa91dec210
      
https://github.com/qemu/qemu/commit/a225369bce684c01095be52e1014e3fa91dec210
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-3-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 04c9c3a52c21088e7039956e5a635250fe3eaee6
      
https://github.com/qemu/qemu/commit/04c9c3a52c21088e7039956e5a635250fe3eaee6
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-4-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 265180614189b969058f5e507c8964fc7dfd422f
      
https://github.com/qemu/qemu/commit/265180614189b969058f5e507c8964fc7dfd422f
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-5-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: be64bbb0149748f3999c49b13976aafb8330ea86
      
https://github.com/qemu/qemu/commit/be64bbb0149748f3999c49b13976aafb8330ea86
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-6-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 562bda8bb41879eeda0bd484dd3d55134579b28e
      
https://github.com/qemu/qemu/commit/562bda8bb41879eeda0bd484dd3d55134579b28e
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-7-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 079bff693bc47bf69fb131f87a03c1689e48ed55
      
https://github.com/qemu/qemu/commit/079bff693bc47bf69fb131f87a03c1689e48ed55
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-8-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 82b54cf51656bf3cd5ed1ac549e8a1085a0e3290
      
https://github.com/qemu/qemu/commit/82b54cf51656bf3cd5ed1ac549e8a1085a0e3290
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-9-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: b0a9f6fed3d80de610dcd04a7e66f9f30a04174f
      
https://github.com/qemu/qemu/commit/b0a9f6fed3d80de610dcd04a7e66f9f30a04174f
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-10-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 16e29cc050516ac0915f0db9226079b17dcbcc51
      
https://github.com/qemu/qemu/commit/16e29cc050516ac0915f0db9226079b17dcbcc51
  Author: Hanna Reitz <hreitz@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-11-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 4d8b0f0a95361b1a3888f3eb3f76b27420383753
      
https://github.com/qemu/qemu/commit/4d8b0f0a95361b1a3888f3eb3f76b27420383753
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-12-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: c9d4e42a8febe4db22eed8463087b38c3c74fd4c
      
https://github.com/qemu/qemu/commit/c9d4e42a8febe4db22eed8463087b38c3c74fd4c
  Author: Stefan Hajnoczi <stefanha@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-14-kwolf@redhat.com>
Signed-off-by: Hanna Reitz <hreitz@redhat.com>


  Commit: 5dbd0ce115c7720268e653fe928bb6a0c1314a80
      
https://github.com/qemu/qemu/commit/5dbd0ce115c7720268e653fe928bb6a0c1314a80
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   2021-11-16 (Tue, 16 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>
Message-Id: <20211115145409.176785-13-kwolf@redhat.com>
[hreitz: Fix iotest 142 for block sizes greater than 512 by operating on
         a file with a size of 1 MB]
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Message-Id: <20211116101431.105252-1-hreitz@redhat.com>


  Commit: 871c71b1bad2d2647641500603a2236869135c7f
      
https://github.com/qemu/qemu/commit/871c71b1bad2d2647641500603a2236869135c7f
  Author: Richard Henderson <richard.henderson@linaro.org>
  Date:   2021-11-16 (Tue, 16 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 tests/qemu-iotests/030
    M tests/qemu-iotests/142
    M tests/qemu-iotests/142.out
    M util/transactions.c

  Log Message:
  -----------
  Merge tag 'pull-block-2021-11-16' of https://gitlab.com/hreitz/qemu into 
staging

Block patches for 6.2.0-rc1:
- 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 Tue 16 Nov 2021 01:57:03 PM CET
# gpg:                using RSA key CB62D7A0EE3829E45F004D34A1FA40D098019CDF
# gpg:                issuer "hreitz@redhat.com"
# gpg: Good signature from "Hanna Reitz <hreitz@redhat.com>" [marginal]
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg:          It is not certain that the signature belongs to the owner.
# Primary key fingerprint: CB62 D7A0 EE38 29E4 5F00  4D34 A1FA 40D0 9801 9CDF

* tag 'pull-block-2021-11-16' of https://gitlab.com/hreitz/qemu:
  file-posix: Fix alignment after reopen changing O_DIRECT
  softmmu/qdev-monitor: fix use-after-free in qdev_set_id()
  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/9f0f846465d4...871c71b1bad2



reply via email to

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