* [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all()
@ 2015-02-09 18:38 Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 01/22] iotests: Move _filter_nbd into common.filter Max Reitz
` (22 more replies)
0 siblings, 23 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Currently, bdrv_close_all() force-closes all BDSs with a BlockBackend,
which can lead to data corruption (see the iotest added in the final
patch of this series) and is most certainly very ugly.
This series reworks bdrv_close_all() to notify all owners of a
BlockBackend that they should release their reference (and additionally
the monitor releases all its references to BB-less BDSs). This way,
force-closing becomes unnecessary.
Also, blk_hide_on_behalf_of_do_drive_del() is removed. Yay!
This series depends on v2 of my series
"blockdev: BlockBackend and media" (or any later version), and on my
series "nbd: Drop BDS backpointer" (on the patch
"iotests: Add 'wait' functionality to _cleanup_qemu").
v2:
- Dropped old patches 1 (squashed into
"blockdev: BlockBackend and media") and 20 (put into
"nbd: Drop BDS backpointer")
- Patch 1: Added (required for patch 3)
- Patch 2: Added (required for patch 3); if someone has a better
solution, I'd be very grateful
- Patch 3: Added a test which ensures that if the BDS an NBD server is
attached to through a BB is ejected, that NBD server is stopped
- Patch 5: Replaces the old patch 5 (which removed the BDS close
notifiers completely); as pointed out by Paolo, we do need the
notifiers as eject notifiers for NBD, so this patch replaces the
per-BDS close notifiers by an eject notifier for the BB which is
called whenever blk_remove_bs() is called and removes a BDS tree from
the BB
- Patch 7: Fixed up to fit onto patch 5
- Patch 11: s/namespaces/namespace/ [Eric]
- Patch 14: Rebase conflict due to the new bdrv_add_key() function
git-backport-diff against v1:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/22:[down] 'iotests: Move _filter_nbd into common.filter'
002/22:[down] 'iotests: Do not redirect qemu's stderr'
003/22:[down] 'iotests: Add test for eject under NBD server'
004/22:[----] [-C] 'quorum: Fix close path'
005/22:[down] 'block: Move BDS close notifiers into BB'
006/22:[----] [--] 'block: Add bdrv_close_all() notifiers'
007/22:[0075] [FC] 'block: Add bdrv_close_all() handlers'
008/22:[----] [-C] 'block: Use blk_remove_bs() in blk_delete()'
009/22:[----] [--] 'blockdev: Use blk_remove_bs() in do_drive_del()'
010/22:[----] [--] 'block: Make bdrv_close() static'
011/22:[0002] [FC] 'block: Add blk_name_taken()'
012/22:[----] [--] 'block: Add blk_next_inserted()'
013/22:[----] [--] 'block: Add blk_commit_all() and blk_invalidate_cache_all()'
014/22:[0009] [FC] 'block: Use BlockBackend more'
015/22:[----] [--] 'blockdev: Add list of monitor-owned BlockBackends'
016/22:[----] [--] 'blockdev: Remove blk_hide_on_behalf_of_do_drive_del()'
017/22:[----] [--] 'block: Make bdrv_drain_one() public'
018/22:[----] [-C] 'block: Move some bdrv_*_all() functions to BB'
019/22:[----] [--] 'block: Remove bdrv_states'
020/22:[----] [--] 'blockdev: Keep track of monitor-owned BDS'
021/22:[----] [--] 'block: Strip down bdrv_close_all()'
022/22:[----] [--] 'iotests: Add test for multiple BB on BDS tree'
Max Reitz (22):
iotests: Move _filter_nbd into common.filter
iotests: Do not redirect qemu's stderr
iotests: Add test for eject under NBD server
quorum: Fix close path
block: Move BDS close notifiers into BB
block: Add bdrv_close_all() notifiers
block: Add bdrv_close_all() handlers
block: Use blk_remove_bs() in blk_delete()
blockdev: Use blk_remove_bs() in do_drive_del()
block: Make bdrv_close() static
block: Add blk_name_taken()
block: Add blk_next_inserted()
block: Add blk_commit_all() and blk_invalidate_cache_all()
block: Use BlockBackend more
blockdev: Add list of monitor-owned BlockBackends
blockdev: Remove blk_hide_on_behalf_of_do_drive_del()
block: Make bdrv_drain_one() public
block: Move some bdrv_*_all() functions to BB
block: Remove bdrv_states
blockdev: Keep track of monitor-owned BDS
block: Strip down bdrv_close_all()
iotests: Add test for multiple BB on BDS tree
block.c | 168 ++++------------------
block/block-backend.c | 301 +++++++++++++++++++++++++++++++--------
block/qapi.c | 13 +-
block/quorum.c | 3 +-
block/snapshot.c | 3 +-
blockdev-nbd.c | 36 +----
blockdev.c | 121 +++++++++++++---
cpus.c | 7 +-
device-hotplug.c | 2 +-
hw/block/xen_disk.c | 16 ++-
hw/ide/piix.c | 1 -
include/block/block.h | 9 +-
include/block/block_int.h | 8 +-
include/sysemu/block-backend.h | 24 ++--
include/sysemu/blockdev.h | 3 +
migration/block.c | 10 +-
migration/migration.c | 4 +-
monitor.c | 13 +-
nbd.c | 59 +++++++-
qemu-char.c | 3 +-
qemu-img.c | 39 ++---
qemu-io.c | 8 +-
qemu-nbd.c | 6 +-
qmp.c | 9 +-
savevm.c | 66 +++++----
stubs/Makefile.objs | 2 +-
stubs/bdrv-commit-all.c | 7 -
stubs/blk-commit-all.c | 7 +
tests/qemu-iotests/083 | 13 +-
tests/qemu-iotests/083.out | 10 --
tests/qemu-iotests/091 | 3 +-
tests/qemu-iotests/096 | 89 ++++++++++++
tests/qemu-iotests/096.out | 16 +++
tests/qemu-iotests/117 | 86 +++++++++++
tests/qemu-iotests/117.out | 14 ++
tests/qemu-iotests/common.filter | 12 ++
tests/qemu-iotests/common.qemu | 1 -
tests/qemu-iotests/group | 2 +
xen-mapcache.c | 3 +-
39 files changed, 807 insertions(+), 390 deletions(-)
delete mode 100644 stubs/bdrv-commit-all.c
create mode 100644 stubs/blk-commit-all.c
create mode 100755 tests/qemu-iotests/096
create mode 100644 tests/qemu-iotests/096.out
create mode 100755 tests/qemu-iotests/117
create mode 100644 tests/qemu-iotests/117.out
--
2.1.0
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 01/22] iotests: Move _filter_nbd into common.filter
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-16 19:31 ` Eric Blake
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 02/22] iotests: Do not redirect qemu's stderr Max Reitz
` (21 subsequent siblings)
22 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
_filter_nbd can be useful for other NBD tests, too, therefore it should
reside in common.filter, and it should support URLs of the "nbd://"
format and export names.
The NBD log lines ("/your/source/dir/nbd.c:function():line: error")
should not be converted to empty lines but removed altogether.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/083 | 13 +------------
tests/qemu-iotests/083.out | 10 ----------
tests/qemu-iotests/common.filter | 12 ++++++++++++
3 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
index 1b2d3f1..aa99278 100755
--- a/tests/qemu-iotests/083
+++ b/tests/qemu-iotests/083
@@ -49,17 +49,6 @@ wait_for_tcp_port() {
done
}
-filter_nbd() {
- # nbd.c error messages contain function names and line numbers that are prone
- # to change. Message ordering depends on timing between send and receive
- # callbacks sometimes, making them unreliable.
- #
- # Filter out the TCP port number since this changes between runs.
- sed -e 's#^.*nbd\.c:.*##g' \
- -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \
- -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#'
-}
-
check_disconnect() {
event=$1
when=$2
@@ -84,7 +73,7 @@ EOF
$PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null &
wait_for_tcp_port "127\\.0\\.0\\.1:$port"
- $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | filter_nbd
+ $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | _filter_nbd
echo
}
diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
index 8c1441b..5c9141b 100644
--- a/tests/qemu-iotests/083.out
+++ b/tests/qemu-iotests/083.out
@@ -51,7 +51,6 @@ no file open, try 'help open'
=== Check disconnect after neg2 ===
-
read failed: Input/output error
=== Check disconnect 8 neg2 ===
@@ -66,42 +65,34 @@ no file open, try 'help open'
=== Check disconnect before request ===
-
read failed: Input/output error
=== Check disconnect after request ===
-
read failed: Input/output error
=== Check disconnect before reply ===
-
read failed: Input/output error
=== Check disconnect after reply ===
-
read failed: Input/output error
=== Check disconnect 4 reply ===
-
read failed: Input/output error
=== Check disconnect 8 reply ===
-
read failed: Input/output error
=== Check disconnect before data ===
-
read failed: Input/output error
=== Check disconnect after data ===
-
read 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -132,7 +123,6 @@ no file open, try 'help open'
=== Check disconnect after neg-classic ===
-
read failed: Input/output error
*** done
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 06e1bb0..6e95474 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -224,5 +224,17 @@ _filter_qemu_img_map()
-e 's/Mapped to *//' | _filter_testdir | _filter_imgfmt
}
+_filter_nbd()
+{
+ # nbd.c error messages contain function names and line numbers that are
+ # prone to change. Message ordering depends on timing between send and
+ # receive callbacks sometimes, making them unreliable.
+ #
+ # Filter out the TCP port number since this changes between runs.
+ sed -e '/^.*nbd\.c:.*/d' \
+ -e 's#nbd:\(//\)\?127\.0\.0\.1:[0-9]*#nbd:\1127.0.0.1:PORT#g' \
+ -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#'
+}
+
# make sure this script returns success
true
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 02/22] iotests: Do not redirect qemu's stderr
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 01/22] iotests: Move _filter_nbd into common.filter Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 03/22] iotests: Add test for eject under NBD server Max Reitz
` (20 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Redirecting qemu's stderr to stdout makes working with the stderr output
difficult due to the other file descriptor magic performed in
_launch_qemu ("ambiguous redirect").
There is no harm in leaving stderr on stderr, so do it.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
If someone has a better solution, especially regarding the redirection
to a subshell here (test 091) and in the next patch, I'd be very
grateful. All of my efforts to pipe the output of the _launch_qemu
function resulted in said error ("ambiguous redirect"), so I had to keep
it on stderr and I did not find another way to pipe stderr to another
program.
---
tests/qemu-iotests/091 | 3 ++-
tests/qemu-iotests/common.qemu | 1 -
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index 32bbd56..caea1ce 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -68,7 +68,8 @@ echo
echo === Starting QEMU VM2 ===
echo
_launch_qemu -drive file="${TEST_IMG}",cache=${CACHEMODE},id=disk \
- -incoming "exec: cat '${MIG_FIFO}'"
+ -incoming "exec: cat '${MIG_FIFO}'" \
+ 2> >(grep -v 'cat: write error')
h2=$QEMU_HANDLE
echo
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index 4e1996c..5f10c1e 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -155,7 +155,6 @@ function _launch_qemu()
"${QEMU}" -nographic -serial none ${comm} -machine accel=qtest "${@}" \
>"${fifo_out}" \
- 2>&1 \
<"${fifo_in}" &
QEMU_PID[${_QEMU_HANDLE}]=$!
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 03/22] iotests: Add test for eject under NBD server
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 01/22] iotests: Move _filter_nbd into common.filter Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 02/22] iotests: Do not redirect qemu's stderr Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 04/22] quorum: Fix close path Max Reitz
` (19 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
This patch adds a test for ejecting the BlockBackend an NBD server is
connected to (the NBD server is supposed to stop).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/096 | 89 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/096.out | 16 +++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 106 insertions(+)
create mode 100755 tests/qemu-iotests/096
create mode 100644 tests/qemu-iotests/096.out
diff --git a/tests/qemu-iotests/096 b/tests/qemu-iotests/096
new file mode 100755
index 0000000..eba144e
--- /dev/null
+++ b/tests/qemu-iotests/096
@@ -0,0 +1,89 @@
+#!/bin/bash
+#
+# Test case for ejecting a BB with an NBD server attached to it
+#
+# Copyright (C) 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+_make_test_img 64k
+
+$QEMU_IO -c 'write -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+_launch_qemu -drive if=ide,media=cdrom,id=drv,file="$TEST_IMG",format=$IMGFMT \
+ 2> >(_filter_nbd)
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'qmp_capabilities' }" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'nbd-server-start',
+ 'arguments': { 'addr': { 'type': 'inet',
+ 'data': { 'host': '127.0.0.1',
+ 'port': '10809' }}}}" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'nbd-server-add',
+ 'arguments': { 'device': 'drv' }}" \
+ 'return'
+
+$QEMU_IO_PROG -f raw -c 'read -P 42 0 64k' 'nbd://127.0.0.1:10809/drv' 2>&1 \
+ | _filter_qemu_io | _filter_nbd
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'eject',
+ 'arguments': { 'device': 'drv' }}" \
+ 'return'
+
+$QEMU_IO_PROG -f raw -c close 'nbd://127.0.0.1:10809/drv' 2>&1 \
+ | _filter_qemu_io | _filter_nbd
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'quit' }" \
+ 'return'
+
+wait=1 _cleanup_qemu
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/096.out b/tests/qemu-iotests/096.out
new file mode 100644
index 0000000..cc10e51
--- /dev/null
+++ b/tests/qemu-iotests/096.out
@@ -0,0 +1,16 @@
+QA output created by 096
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"return": {}}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "drv", "tray-open": true}}
+{"return": {}}
+qemu-io: can't open device nbd://127.0.0.1:PORT/drv: Failed to read export length
+no file open, try 'help open'
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1716269..1508d9b 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -101,6 +101,7 @@
092 rw auto quick
094 rw auto quick
095 rw auto quick
+096 rw auto quick
097 rw auto backing
098 rw auto backing quick
099 rw auto quick
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 04/22] quorum: Fix close path
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (2 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 03/22] iotests: Add test for eject under NBD server Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 05/22] block: Move BDS close notifiers into BB Max Reitz
` (18 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
bdrv_unref() can lead to bdrv_close(), which in turn will result in
bdrv_drain_all(). This function will later be called blk_drain_all() and
iterate only over the BlockBackends for which blk_is_inserted() holds
true; therefore, bdrv_is_inserted() and thus quorum_is_inserted() will
probably be called.
This patch makes quorum_is_inserted() aware of the fact that some
children may have been closed already.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/quorum.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/quorum.c b/block/quorum.c
index 7a75cea..5ae2398 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1005,6 +1005,7 @@ static void quorum_close(BlockDriverState *bs)
for (i = 0; i < s->num_children; i++) {
bdrv_unref(s->bs[i]);
+ s->bs[i] = NULL;
}
g_free(s->bs);
@@ -1070,7 +1071,7 @@ static bool quorum_is_inserted(BlockDriverState *bs)
int i;
for (i = 0; i < s->num_children; i++) {
- if (!bdrv_is_inserted(s->bs[i])) {
+ if (s->bs[i] && !bdrv_is_inserted(s->bs[i])) {
return false;
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 05/22] block: Move BDS close notifiers into BB
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (3 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 04/22] quorum: Fix close path Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 06/22] block: Add bdrv_close_all() notifiers Max Reitz
` (17 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
The only remaining user of the BDS close notifiers is NBD which uses
them to determine when a BDS tree is being ejected. This patch removes
the BDS-level close notifiers and adds a notifier list to the
BlockBackend structure that is invoked whenever a BDS is removed. While
it might make sense to have a similar list for the "insert BDS tree"
event, there would be no users so far, so no such list is added here.
Note that the mixture of "eject" and "close" in the new NBD code is
intentional; notifiers to be called on bdrv_close_all() will be added
later, and those will reuse the BDRVCloseNotifier type.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 7 -------
block/block-backend.c | 11 +++++++----
blockdev-nbd.c | 36 +-----------------------------------
include/block/block.h | 1 -
include/block/block_int.h | 2 --
include/sysemu/block-backend.h | 2 +-
nbd.c | 35 +++++++++++++++++++++++++++++++++++
7 files changed, 44 insertions(+), 50 deletions(-)
diff --git a/block.c b/block.c
index 99d9955..6ad80fe 100644
--- a/block.c
+++ b/block.c
@@ -371,7 +371,6 @@ BlockDriverState *bdrv_new(void)
for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
QLIST_INIT(&bs->op_blockers[i]);
}
- notifier_list_init(&bs->close_notifiers);
notifier_with_return_list_init(&bs->before_write_notifiers);
qemu_co_queue_init(&bs->throttled_reqs[0]);
qemu_co_queue_init(&bs->throttled_reqs[1]);
@@ -381,11 +380,6 @@ BlockDriverState *bdrv_new(void)
return bs;
}
-void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
-{
- notifier_list_add(&bs->close_notifiers, notify);
-}
-
BlockDriver *bdrv_find_format(const char *format_name)
{
BlockDriver *drv1;
@@ -1864,7 +1858,6 @@ void bdrv_close(BlockDriverState *bs)
bdrv_drain_all(); /* complete I/O */
bdrv_flush(bs);
bdrv_drain_all(); /* in case flush left pending I/O */
- notifier_list_notify(&bs->close_notifiers, bs);
if (bs->drv) {
if (bs->backing_hd) {
diff --git a/block/block-backend.c b/block/block-backend.c
index ad0af67..6d02184 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -47,6 +47,8 @@ struct BlockBackend {
BlockdevOnError on_read_error, on_write_error;
bool iostatus_enabled;
BlockDeviceIoStatus iostatus;
+
+ NotifierList remove_bs_notifiers;
};
typedef struct BlockBackendAIOCB {
@@ -97,6 +99,7 @@ BlockBackend *blk_new(const char *name, Error **errp)
blk = g_new0(BlockBackend, 1);
blk->name = g_strdup(name);
blk->refcnt = 1;
+ notifier_list_init(&blk->remove_bs_notifiers);
QTAILQ_INSERT_TAIL(&blk_backends, blk, link);
return blk;
}
@@ -320,6 +323,8 @@ void blk_remove_bs(BlockBackend *blk)
return;
}
+ notifier_list_notify(&blk->remove_bs_notifiers, blk);
+
blk_update_root_state(blk);
bdrv_unref(blk->bs);
@@ -1067,11 +1072,9 @@ void blk_remove_aio_context_notifier(BlockBackend *blk,
}
}
-void blk_add_close_notifier(BlockBackend *blk, Notifier *notify)
+void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify)
{
- if (blk->bs) {
- bdrv_add_close_notifier(blk->bs, notify);
- }
+ notifier_list_add(&blk->remove_bs_notifiers, notify);
}
void blk_io_plug(BlockBackend *blk)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 22e95d1..eb5f9a0 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -47,36 +47,11 @@ void qmp_nbd_server_start(SocketAddress *addr, Error **errp)
}
}
-/* Hook into the BlockDriverState notifiers to close the export when
- * the file is closed.
- */
-typedef struct NBDCloseNotifier {
- Notifier n;
- NBDExport *exp;
- QTAILQ_ENTRY(NBDCloseNotifier) next;
-} NBDCloseNotifier;
-
-static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
- QTAILQ_HEAD_INITIALIZER(close_notifiers);
-
-static void nbd_close_notifier(Notifier *n, void *data)
-{
- NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
-
- notifier_remove(&cn->n);
- QTAILQ_REMOVE(&close_notifiers, cn, next);
-
- nbd_export_close(cn->exp);
- nbd_export_put(cn->exp);
- g_free(cn);
-}
-
void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
Error **errp)
{
BlockBackend *blk;
NBDExport *exp;
- NBDCloseNotifier *n;
if (server_fd == -1) {
error_setg(errp, "NBD server not running");
@@ -108,20 +83,11 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
exp = nbd_export_new(blk, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, NULL);
nbd_export_set_name(exp, device);
-
- n = g_new0(NBDCloseNotifier, 1);
- n->n.notify = nbd_close_notifier;
- n->exp = exp;
- blk_add_close_notifier(blk, &n->n);
- QTAILQ_INSERT_TAIL(&close_notifiers, n, next);
}
void qmp_nbd_server_stop(Error **errp)
{
- while (!QTAILQ_EMPTY(&close_notifiers)) {
- NBDCloseNotifier *cn = QTAILQ_FIRST(&close_notifiers);
- nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp));
- }
+ nbd_export_close_all();
if (server_fd != -1) {
qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
diff --git a/include/block/block.h b/include/block/block.h
index 9e33c54..0708a48 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -196,7 +196,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
void bdrv_reopen_commit(BDRVReopenState *reopen_state);
void bdrv_reopen_abort(BDRVReopenState *reopen_state);
void bdrv_close(BlockDriverState *bs);
-void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify);
int bdrv_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8a40309..376ca2f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -350,8 +350,6 @@ struct BlockDriverState {
BlockDriverState *backing_hd;
BlockDriverState *file;
- NotifierList close_notifiers;
-
/* Callback before write request is processed */
NotifierWithReturnList before_write_notifiers;
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index e234aca..dc1e217 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -158,7 +158,7 @@ void blk_remove_aio_context_notifier(BlockBackend *blk,
void *),
void (*detach_aio_context)(void *),
void *opaque);
-void blk_add_close_notifier(BlockBackend *blk, Notifier *notify);
+void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify);
void blk_io_plug(BlockBackend *blk);
void blk_io_unplug(BlockBackend *blk);
BlockAcctStats *blk_get_stats(BlockBackend *blk);
diff --git a/nbd.c b/nbd.c
index 71159af..02ee686 100644
--- a/nbd.c
+++ b/nbd.c
@@ -964,9 +964,25 @@ static void blk_aio_detach(void *opaque)
exp->ctx = NULL;
}
+typedef struct NBDCloseNotifier {
+ Notifier n;
+ NBDExport *exp;
+ QTAILQ_ENTRY(NBDCloseNotifier) next;
+} NBDCloseNotifier;
+
+static QTAILQ_HEAD(, NBDCloseNotifier) eject_notifiers =
+ QTAILQ_HEAD_INITIALIZER(eject_notifiers);
+
+static void nbd_close_notifier(Notifier *n, void *data)
+{
+ NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
+ nbd_export_close(cn->exp);
+}
+
NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
uint32_t nbdflags, void (*close)(NBDExport *))
{
+ NBDCloseNotifier *eject_notifier = g_new0(NBDCloseNotifier, 1);
NBDExport *exp = g_malloc0(sizeof(NBDExport));
exp->refcount = 1;
QTAILQ_INIT(&exp->clients);
@@ -978,6 +994,13 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
exp->ctx = blk_get_aio_context(blk);
blk_ref(blk);
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
+
+ eject_notifier->n.notify = nbd_close_notifier;
+ eject_notifier->exp = exp;
+ QTAILQ_INSERT_TAIL(&eject_notifiers, eject_notifier, next);
+
+ blk_add_remove_bs_notifier(blk, &eject_notifier->n);
+
/*
* NBD exports are used for non-shared storage migration. Make sure
* that BDRV_O_INCOMING is cleared and the image is ready for write
@@ -1022,6 +1045,7 @@ void nbd_export_set_name(NBDExport *exp, const char *name)
void nbd_export_close(NBDExport *exp)
{
+ NBDCloseNotifier *eject_notifier;
NBDClient *client, *next;
nbd_export_get(exp);
@@ -1031,6 +1055,17 @@ void nbd_export_close(NBDExport *exp)
nbd_export_set_name(exp, NULL);
nbd_export_put(exp);
if (exp->blk) {
+ QTAILQ_FOREACH(eject_notifier, &eject_notifiers, next) {
+ if (eject_notifier->exp == exp) {
+ break;
+ }
+ }
+ assert(eject_notifier);
+
+ notifier_remove(&eject_notifier->n);
+ QTAILQ_REMOVE(&eject_notifiers, eject_notifier, next);
+ g_free(eject_notifier);
+
blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
blk_aio_detach, exp);
blk_unref(exp->blk);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 06/22] block: Add bdrv_close_all() notifiers
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (4 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 05/22] block: Move BDS close notifiers into BB Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 07/22] block: Add bdrv_close_all() handlers Max Reitz
` (16 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
This adds a list of notifiers to be invoked on bdrv_close_all().
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 10 ++++++++++
include/block/block.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/block.c b/block.c
index 6ad80fe..143c7cc 100644
--- a/block.c
+++ b/block.c
@@ -104,6 +104,9 @@ static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
+static NotifierList close_all_notifiers =
+ NOTIFIER_LIST_INITIALIZER(close_all_notifiers);
+
#ifdef _WIN32
static int is_windows_drive_prefix(const char *filename)
{
@@ -1907,6 +1910,8 @@ void bdrv_close_all(void)
{
BlockDriverState *bs;
+ notifier_list_notify(&close_all_notifiers, NULL);
+
QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
AioContext *aio_context = bdrv_get_aio_context(bs);
@@ -1916,6 +1921,11 @@ void bdrv_close_all(void)
}
}
+void bdrv_add_close_all_notifier(Notifier *notifier)
+{
+ notifier_list_add(&close_all_notifiers, notifier);
+}
+
/* Check if any requests are in-flight (including throttled requests) */
static bool bdrv_requests_pending(BlockDriverState *bs)
{
diff --git a/include/block/block.h b/include/block/block.h
index 0708a48..79efccb 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -543,4 +543,6 @@ void bdrv_io_plug(BlockDriverState *bs);
void bdrv_io_unplug(BlockDriverState *bs);
void bdrv_flush_io_queue(BlockDriverState *bs);
+void bdrv_add_close_all_notifier(Notifier *notifier);
+
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 07/22] block: Add bdrv_close_all() handlers
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (5 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 06/22] block: Add bdrv_close_all() notifiers Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-13 23:02 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 08/22] block: Use blk_remove_bs() in blk_delete() Max Reitz
` (15 subsequent siblings)
22 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Every time a reference to a BlockBackend is taken, a notifier for
bdrv_close_all() has to be deposited so the reference holder can
relinquish its reference when bdrv_close_all() is called. That notifier
should be revoked on a bdrv_unref() call.
Add a Notifier * parameter to all the functions changing the reference
count of a BlockBackend: blk_new(), blk_new_with_bs(), blk_new_open(),
blk_ref() and blk_unref().
By dropping the manual reference handling in hw/block/xen_disk.c, the
blk_unref() in hw/ide/piix.c can be dropped as well.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 69 ++++++++++++++++++++++++++++++++++++------
blockdev.c | 62 ++++++++++++++++++++++++++++++++++---
device-hotplug.c | 2 +-
hw/block/xen_disk.c | 16 +++++++---
hw/ide/piix.c | 1 -
include/sysemu/block-backend.h | 13 +++++---
include/sysemu/blockdev.h | 3 ++
nbd.c | 26 ++++++++++++++--
qemu-img.c | 39 ++++++++++++------------
qemu-io.c | 6 ++--
qemu-nbd.c | 6 ++--
11 files changed, 189 insertions(+), 54 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 6d02184..98f4af9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -76,7 +76,8 @@ static QTAILQ_HEAD(, BlockBackend) blk_backends =
* Store an error through @errp on failure, unless it's null.
* Return the new BlockBackend on success, null on failure.
*/
-BlockBackend *blk_new(const char *name, Error **errp)
+BlockBackend *blk_new(const char *name, Notifier *close_all_notifier,
+ Error **errp)
{
BlockBackend *blk;
@@ -100,6 +101,9 @@ BlockBackend *blk_new(const char *name, Error **errp)
blk->name = g_strdup(name);
blk->refcnt = 1;
notifier_list_init(&blk->remove_bs_notifiers);
+ if (close_all_notifier) {
+ bdrv_add_close_all_notifier(close_all_notifier);
+ }
QTAILQ_INSERT_TAIL(&blk_backends, blk, link);
return blk;
}
@@ -108,12 +112,13 @@ BlockBackend *blk_new(const char *name, Error **errp)
* Create a new BlockBackend with a new BlockDriverState attached.
* Otherwise just like blk_new(), which see.
*/
-BlockBackend *blk_new_with_bs(const char *name, Error **errp)
+BlockBackend *blk_new_with_bs(const char *name, Notifier *close_all_notifier,
+ Error **errp)
{
BlockBackend *blk;
BlockDriverState *bs;
- blk = blk_new(name, errp);
+ blk = blk_new(name, close_all_notifier, errp);
if (!blk) {
return NULL;
}
@@ -138,12 +143,12 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp)
*/
BlockBackend *blk_new_open(const char *name, const char *filename,
const char *reference, QDict *options, int flags,
- Error **errp)
+ Notifier *close_all_notifier, Error **errp)
{
BlockBackend *blk;
int ret;
- blk = blk_new_with_bs(name, errp);
+ blk = blk_new_with_bs(name, close_all_notifier, errp);
if (!blk) {
QDECREF(options);
return NULL;
@@ -151,7 +156,7 @@ BlockBackend *blk_new_open(const char *name, const char *filename,
ret = bdrv_open(&blk->bs, filename, reference, options, flags, NULL, errp);
if (ret < 0) {
- blk_unref(blk);
+ blk_unref(blk, close_all_notifier);
return NULL;
}
@@ -191,9 +196,13 @@ static void drive_info_del(DriveInfo *dinfo)
* Increment @blk's reference count.
* @blk must not be null.
*/
-void blk_ref(BlockBackend *blk)
+void blk_ref(BlockBackend *blk, Notifier *close_all_notifier)
{
blk->refcnt++;
+
+ if (close_all_notifier) {
+ bdrv_add_close_all_notifier(close_all_notifier);
+ }
}
/*
@@ -201,8 +210,12 @@ void blk_ref(BlockBackend *blk)
* If this drops it to zero, destroy @blk.
* For convenience, do nothing if @blk is null.
*/
-void blk_unref(BlockBackend *blk)
+void blk_unref(BlockBackend *blk, Notifier *close_all_notifier)
{
+ if (close_all_notifier) {
+ notifier_remove(close_all_notifier);
+ }
+
if (blk) {
assert(blk->refcnt > 0);
if (!--blk->refcnt) {
@@ -352,6 +365,21 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
bs->blk = blk;
}
+typedef struct DevCloseAllNotifier {
+ Notifier n;
+ BlockBackend *blk;
+ QTAILQ_ENTRY(DevCloseAllNotifier) next;
+} DevCloseAllNotifier;
+
+static QTAILQ_HEAD(, DevCloseAllNotifier) close_all_notifiers =
+ QTAILQ_HEAD_INITIALIZER(close_all_notifiers);
+
+static void dev_close_all_notifier(Notifier *n, void *data)
+{
+ DevCloseAllNotifier *can = DO_UPCAST(DevCloseAllNotifier, n, n);
+ blk_detach_dev(can->blk, can->blk->dev);
+}
+
/*
* Attach device model @dev to @blk.
* Return 0 on success, -EBUSY when a device model is attached already.
@@ -359,10 +387,19 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
int blk_attach_dev(BlockBackend *blk, void *dev)
/* TODO change to DeviceState *dev when all users are qdevified */
{
+ DevCloseAllNotifier *can;
+
if (blk->dev) {
return -EBUSY;
}
- blk_ref(blk);
+
+ can = g_new0(DevCloseAllNotifier, 1);
+ can->n.notify = dev_close_all_notifier;
+ can->blk = blk;
+
+ QTAILQ_INSERT_TAIL(&close_all_notifiers, can, next);
+
+ blk_ref(blk, &can->n);
blk->dev = dev;
blk_iostatus_reset(blk);
return 0;
@@ -387,12 +424,24 @@ void blk_attach_dev_nofail(BlockBackend *blk, void *dev)
void blk_detach_dev(BlockBackend *blk, void *dev)
/* TODO change to DeviceState *dev when all users are qdevified */
{
+ DevCloseAllNotifier *can;
+
assert(blk->dev == dev);
blk->dev = NULL;
blk->dev_ops = NULL;
blk->dev_opaque = NULL;
blk->guest_block_size = 512;
- blk_unref(blk);
+
+ QTAILQ_FOREACH(can, &close_all_notifiers, next) {
+ if (can->blk == blk) {
+ break;
+ }
+ }
+ assert(can);
+
+ blk_unref(blk, &can->n);
+ QTAILQ_REMOVE(&close_all_notifiers, can, next);
+ g_free(can);
}
/*
diff --git a/blockdev.c b/blockdev.c
index f198be6..75baf16 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -47,6 +47,15 @@
#include "trace.h"
#include "sysemu/arch_init.h"
+typedef struct BlockdevCloseAllNotifier {
+ Notifier n;
+ BlockBackend *blk;
+ QTAILQ_ENTRY(BlockdevCloseAllNotifier) next;
+} BlockdevCloseAllNotifier;
+
+static QTAILQ_HEAD(, BlockdevCloseAllNotifier) close_all_notifiers =
+ QTAILQ_HEAD_INITIALIZER(close_all_notifiers);
+
static const char *const if_name[IF_COUNT] = {
[IF_NONE] = "none",
[IF_IDE] = "ide",
@@ -78,6 +87,37 @@ static int if_max_devs[IF_COUNT] = {
[IF_SCSI] = 7,
};
+static void monitor_blk_unref_with_can(BlockBackend *blk,
+ BlockdevCloseAllNotifier *can)
+{
+ if (can) {
+ assert(can->blk == blk);
+ } else {
+ QTAILQ_FOREACH(can, &close_all_notifiers, next) {
+ if (can->blk == blk) {
+ break;
+ }
+ }
+ assert(can);
+ }
+
+ blk_unref(blk, &can->n);
+ QTAILQ_REMOVE(&close_all_notifiers, can, next);
+ g_free(can);
+}
+
+void monitor_blk_unref(BlockBackend *blk)
+{
+ monitor_blk_unref_with_can(blk, NULL);
+}
+
+static void blockdev_close_all_notifier(Notifier *n, void *data)
+{
+ BlockdevCloseAllNotifier *can = DO_UPCAST(BlockdevCloseAllNotifier, n, n);
+
+ monitor_blk_unref_with_can(can->blk, can);
+}
+
/**
* Boards may call this to offer board-by-board overrides
* of the default, global values.
@@ -140,7 +180,7 @@ void blockdev_auto_del(BlockBackend *blk)
DriveInfo *dinfo = blk_legacy_dinfo(blk);
if (dinfo && dinfo->auto_del) {
- blk_unref(blk);
+ monitor_blk_unref(blk);
}
}
@@ -460,6 +500,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
bool has_driver_specific_opts;
BlockdevDetectZeroesOptions detect_zeroes =
BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
+ BlockdevCloseAllNotifier *can;
/* Check common options by copying from bs_opts to opts, all other options
* stay in bs_opts for processing by bdrv_open(). */
@@ -536,14 +577,21 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
}
/* init */
+ can = g_new0(BlockdevCloseAllNotifier, 1);
+ can->n.notify = blockdev_close_all_notifier;
+
if ((!file || !*file) && !has_driver_specific_opts) {
BlockBackendRootState *blk_rs;
- blk = blk_new(qemu_opts_id(opts), errp);
+ blk = blk_new(qemu_opts_id(opts), &can->n, errp);
if (!blk) {
+ g_free(can);
goto early_err;
}
+ can->blk = blk;
+ QTAILQ_INSERT_TAIL(&close_all_notifiers, can, next);
+
blk_rs = blk_get_root_state(blk);
blk_rs->open_flags = bdrv_flags;
blk_rs->read_only = !(bdrv_flags & BDRV_O_RDWR);
@@ -561,12 +609,16 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
}
blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags,
- errp);
+ &can->n, errp);
if (!blk) {
+ g_free(can);
goto err_no_bs_opts;
}
bs = blk_bs(blk);
+ can->blk = blk;
+ QTAILQ_INSERT_TAIL(&close_all_notifiers, can, next);
+
bs->detect_zeroes = detect_zeroes;
/* disk I/O throttling */
@@ -2246,7 +2298,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
blk_set_on_error(blk, BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT);
} else {
- blk_unref(blk);
+ monitor_blk_unref(blk);
}
aio_context_release(aio_context);
@@ -3174,7 +3226,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
if (bs && bdrv_key_required(bs)) {
if (blk) {
- blk_unref(blk);
+ monitor_blk_unref(blk);
} else {
bdrv_unref(bs);
}
diff --git a/device-hotplug.c b/device-hotplug.c
index 9e38cc4..aa05a71 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -77,6 +77,6 @@ void drive_hot_add(Monitor *mon, const QDict *qdict)
err:
if (dinfo) {
- blk_unref(blk_by_legacy_dinfo(dinfo));
+ monitor_blk_unref(blk_by_legacy_dinfo(dinfo));
}
}
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 2f59b5b..4916ddf 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -908,8 +908,10 @@ static int blk_connect(struct XenDevice *xendev)
/* setup via xenbus -> create new block driver instance */
xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
+ /* No need to give a close_all_notifier because blk_attach_dev_nofail()
+ * and blk_detach_dev() will keep track of our reference */
blkdev->blk = blk_new_open(blkdev->dev, blkdev->filename, NULL, options,
- qflags, &local_err);
+ qflags, NULL, &local_err);
if (!blkdev->blk) {
xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
error_get_pretty(local_err));
@@ -925,11 +927,16 @@ static int blk_connect(struct XenDevice *xendev)
blkdev->blk = NULL;
return -1;
}
- /* blkdev->blk is not create by us, we get a reference
- * so we can blk_unref() unconditionally */
- blk_ref(blkdev->blk);
}
+
blk_attach_dev_nofail(blkdev->blk, blkdev);
+ if (!blkdev->dinfo) {
+ /* blkdev->blk has just been created; blk_attach_dev_nofail() counts
+ * the reference blkdev->blk, so we do not have to keep track (which
+ * allows us to ignore bdrv_close_all()) */
+ blk_unref(blkdev->blk, NULL);
+ }
+
blkdev->file_size = blk_getlength(blkdev->blk);
if (blkdev->file_size < 0) {
BlockDriverState *bs = blk_bs(blkdev->blk);
@@ -1032,7 +1039,6 @@ static void blk_disconnect(struct XenDevice *xendev)
if (blkdev->blk) {
blk_detach_dev(blkdev->blk, blkdev);
- blk_unref(blkdev->blk);
blkdev->blk = NULL;
}
xen_be_unbind_evtchn(&blkdev->xendev);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index b0172fb..becf0f5 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -184,7 +184,6 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev)
blk_detach_dev(blk, ds);
}
pci_ide->bus[di->bus].ifs[di->unit].blk = NULL;
- blk_unref(blk);
}
}
qdev_reset_all(DEVICE(dev));
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index dc1e217..9840fef 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -13,6 +13,7 @@
#ifndef BLOCK_BACKEND_H
#define BLOCK_BACKEND_H
+#include "qemu/notify.h"
#include "qemu/typedefs.h"
#include "qapi/error.h"
@@ -60,13 +61,15 @@ typedef struct BlockDevOps {
void (*resize_cb)(void *opaque);
} BlockDevOps;
-BlockBackend *blk_new(const char *name, Error **errp);
-BlockBackend *blk_new_with_bs(const char *name, Error **errp);
+BlockBackend *blk_new(const char *name, Notifier *close_all_notifier,
+ Error **errp);
+BlockBackend *blk_new_with_bs(const char *name, Notifier *close_all_notifier,
+ Error **errp);
BlockBackend *blk_new_open(const char *name, const char *filename,
const char *reference, QDict *options, int flags,
- Error **errp);
-void blk_ref(BlockBackend *blk);
-void blk_unref(BlockBackend *blk);
+ Notifier *close_all_notifier, Error **errp);
+void blk_ref(BlockBackend *blk, Notifier *close_all_notifier);
+void blk_unref(BlockBackend *blk, Notifier *close_all_notifier);
const char *blk_name(BlockBackend *blk);
BlockBackend *blk_by_name(const char *name);
BlockBackend *blk_next(BlockBackend *blk);
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 2a34332..b6091e0 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -67,4 +67,7 @@ DriveInfo *add_init_drive(const char *opts);
void do_commit(Monitor *mon, const QDict *qdict);
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+
+void monitor_blk_unref(BlockBackend *blk);
+
#endif
diff --git a/nbd.c b/nbd.c
index 02ee686..9c44c0f 100644
--- a/nbd.c
+++ b/nbd.c
@@ -973,6 +973,9 @@ typedef struct NBDCloseNotifier {
static QTAILQ_HEAD(, NBDCloseNotifier) eject_notifiers =
QTAILQ_HEAD_INITIALIZER(eject_notifiers);
+static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
+ QTAILQ_HEAD_INITIALIZER(close_notifiers);
+
static void nbd_close_notifier(Notifier *n, void *data)
{
NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
@@ -983,6 +986,7 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
uint32_t nbdflags, void (*close)(NBDExport *))
{
NBDCloseNotifier *eject_notifier = g_new0(NBDCloseNotifier, 1);
+ NBDCloseNotifier *close_notifier = g_new0(NBDCloseNotifier, 1);
NBDExport *exp = g_malloc0(sizeof(NBDExport));
exp->refcount = 1;
QTAILQ_INIT(&exp->clients);
@@ -992,7 +996,12 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
exp->size = size == -1 ? blk_getlength(blk) : size;
exp->close = close;
exp->ctx = blk_get_aio_context(blk);
- blk_ref(blk);
+
+ close_notifier->n.notify = nbd_close_notifier;
+ close_notifier->exp = exp;
+ QTAILQ_INSERT_TAIL(&close_notifiers, close_notifier, next);
+
+ blk_ref(blk, &close_notifier->n);
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
eject_notifier->n.notify = nbd_close_notifier;
@@ -1045,7 +1054,7 @@ void nbd_export_set_name(NBDExport *exp, const char *name)
void nbd_export_close(NBDExport *exp)
{
- NBDCloseNotifier *eject_notifier;
+ NBDCloseNotifier *eject_notifier, *close_notifier;
NBDClient *client, *next;
nbd_export_get(exp);
@@ -1054,6 +1063,7 @@ void nbd_export_close(NBDExport *exp)
}
nbd_export_set_name(exp, NULL);
nbd_export_put(exp);
+
if (exp->blk) {
QTAILQ_FOREACH(eject_notifier, &eject_notifiers, next) {
if (eject_notifier->exp == exp) {
@@ -1066,10 +1076,20 @@ void nbd_export_close(NBDExport *exp)
QTAILQ_REMOVE(&eject_notifiers, eject_notifier, next);
g_free(eject_notifier);
+ QTAILQ_FOREACH(close_notifier, &close_notifiers, next) {
+ if (close_notifier->exp == exp) {
+ break;
+ }
+ }
+ assert(close_notifier);
+
blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
blk_aio_detach, exp);
- blk_unref(exp->blk);
+ blk_unref(exp->blk, &close_notifier->n);
exp->blk = NULL;
+
+ QTAILQ_REMOVE(&close_notifiers, close_notifier, next);
+ g_free(close_notifier);
}
}
diff --git a/qemu-img.c b/qemu-img.c
index 77fd4e4..56a528e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -302,7 +302,7 @@ static BlockBackend *img_open(const char *id, const char *filename,
qdict_put(options, "driver", qstring_from_str(fmt));
}
- blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
+ blk = blk_new_open(id, filename, NULL, options, flags, NULL, &local_err);
if (!blk) {
error_report("Could not open '%s': %s", filename,
error_get_pretty(local_err));
@@ -324,7 +324,7 @@ static BlockBackend *img_open(const char *id, const char *filename,
}
return blk;
fail:
- blk_unref(blk);
+ blk_unref(blk, NULL);
return NULL;
}
@@ -713,7 +713,7 @@ static int img_check(int argc, char **argv)
fail:
qapi_free_ImageCheck(check);
- blk_unref(blk);
+ blk_unref(blk, NULL);
return ret;
}
@@ -880,7 +880,7 @@ unref_backing:
done:
qemu_progress_end();
- blk_unref(blk);
+ blk_unref(blk, NULL);
if (local_err) {
qerror_report_err(local_err);
@@ -1292,9 +1292,9 @@ static int img_compare(int argc, char **argv)
out:
qemu_vfree(buf1);
qemu_vfree(buf2);
- blk_unref(blk2);
+ blk_unref(blk2, NULL);
out2:
- blk_unref(blk1);
+ blk_unref(blk1, NULL);
out3:
qemu_progress_end();
return ret;
@@ -1862,11 +1862,11 @@ out:
qemu_opts_free(create_opts);
qemu_vfree(buf);
qemu_opts_del(sn_opts);
- blk_unref(out_blk);
+ blk_unref(out_blk, NULL);
g_free(bs);
if (blk) {
for (bs_i = 0; bs_i < bs_n; bs_i++) {
- blk_unref(blk[bs_i]);
+ blk_unref(blk[bs_i], NULL);
}
g_free(blk);
}
@@ -2001,7 +2001,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
if (err) {
error_report("%s", error_get_pretty(err));
error_free(err);
- blk_unref(blk);
+ blk_unref(blk, NULL);
goto err;
}
@@ -2010,7 +2010,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
*last = elem;
last = &elem->next;
- blk_unref(blk);
+ blk_unref(blk, NULL);
filename = fmt = NULL;
if (chain) {
@@ -2298,7 +2298,7 @@ static int img_map(int argc, char **argv)
dump_map_entry(output_format, &curr, NULL);
out:
- blk_unref(blk);
+ blk_unref(blk, NULL);
return ret < 0;
}
@@ -2422,7 +2422,7 @@ static int img_snapshot(int argc, char **argv)
}
/* Cleanup */
- blk_unref(blk);
+ blk_unref(blk, NULL);
if (ret) {
return 1;
}
@@ -2546,7 +2546,7 @@ static int img_rebase(int argc, char **argv)
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
blk_old_backing = blk_new_open("old_backing", backing_name, NULL,
- options, src_flags, &local_err);
+ options, src_flags, NULL, &local_err);
if (!blk_old_backing) {
error_report("Could not open old backing file '%s': %s",
backing_name, error_get_pretty(local_err));
@@ -2563,7 +2563,8 @@ static int img_rebase(int argc, char **argv)
}
blk_new_backing = blk_new_open("new_backing", out_baseimg, NULL,
- options, src_flags, &local_err);
+ options, src_flags, NULL,
+ &local_err);
if (!blk_new_backing) {
error_report("Could not open new backing file '%s': %s",
out_baseimg, error_get_pretty(local_err));
@@ -2736,11 +2737,11 @@ out:
qemu_progress_end();
/* Cleanup */
if (!unsafe) {
- blk_unref(blk_old_backing);
- blk_unref(blk_new_backing);
+ blk_unref(blk_old_backing, NULL);
+ blk_unref(blk_new_backing, NULL);
}
- blk_unref(blk);
+ blk_unref(blk, NULL);
if (ret) {
return 1;
}
@@ -2863,7 +2864,7 @@ static int img_resize(int argc, char **argv)
break;
}
out:
- blk_unref(blk);
+ blk_unref(blk, NULL);
if (ret) {
return 1;
}
@@ -3001,7 +3002,7 @@ static int img_amend(int argc, char **argv)
out:
qemu_progress_end();
- blk_unref(blk);
+ blk_unref(blk, NULL);
qemu_opts_del(opts);
qemu_opts_free(create_opts);
g_free(options);
diff --git a/qemu-io.c b/qemu-io.c
index 4a3e719..9c0ad37 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -37,7 +37,7 @@ static ReadLineState *readline_state;
static int close_f(BlockBackend *blk, int argc, char **argv)
{
- blk_unref(qemuio_blk);
+ blk_unref(qemuio_blk, NULL);
qemuio_blk = NULL;
return 0;
}
@@ -59,7 +59,7 @@ static int openfile(char *name, int flags, QDict *opts)
return 1;
}
- qemuio_blk = blk_new_open("hda", name, NULL, opts, flags, &local_err);
+ qemuio_blk = blk_new_open("hda", name, NULL, opts, flags, NULL, &local_err);
if (!qemuio_blk) {
fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
name ? " device " : "", name ?: "",
@@ -474,7 +474,7 @@ int main(int argc, char **argv)
*/
bdrv_drain_all();
- blk_unref(qemuio_blk);
+ blk_unref(qemuio_blk, NULL);
g_free(readline_state);
return 0;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ac1e5d6..5e6e4b4 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -694,7 +694,8 @@ int main(int argc, char **argv)
}
srcpath = argv[optind];
- blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
+ /* the reference will be passed on nbd_export_new() */
+ blk = blk_new_open("hda", srcpath, NULL, options, flags, NULL, &local_err);
if (!blk) {
errx(EXIT_FAILURE, "Failed to blk_new_open '%s': %s", argv[optind],
error_get_pretty(local_err));
@@ -728,7 +729,9 @@ int main(int argc, char **argv)
}
}
+ /* nbd_export_new() takes the reference to blk */
exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed);
+ blk_unref(blk, NULL);
if (sockpath) {
fd = unix_socket_incoming(sockpath);
@@ -773,7 +776,6 @@ int main(int argc, char **argv)
}
} while (state != TERMINATED);
- blk_unref(blk);
if (sockpath) {
unlink(sockpath);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 08/22] block: Use blk_remove_bs() in blk_delete()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (6 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 07/22] block: Add bdrv_close_all() handlers Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 09/22] blockdev: Use blk_remove_bs() in do_drive_del() Max Reitz
` (14 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/block-backend.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 98f4af9..bcad1dc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -167,12 +167,7 @@ static void blk_delete(BlockBackend *blk)
{
assert(!blk->refcnt);
assert(!blk->dev);
- if (blk->bs) {
- assert(blk->bs->blk == blk);
- blk->bs->blk = NULL;
- bdrv_unref(blk->bs);
- blk->bs = NULL;
- }
+ blk_remove_bs(blk);
/* Avoid double-remove after blk_hide_on_behalf_of_do_drive_del() */
if (blk->name[0]) {
QTAILQ_REMOVE(&blk_backends, blk, link);
@@ -336,6 +331,8 @@ void blk_remove_bs(BlockBackend *blk)
return;
}
+ assert(blk->bs->blk == blk);
+
notifier_list_notify(&blk->remove_bs_notifiers, blk);
blk_update_root_state(blk);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 09/22] blockdev: Use blk_remove_bs() in do_drive_del()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (7 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 08/22] block: Use blk_remove_bs() in blk_delete() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 10/22] block: Make bdrv_close() static Max Reitz
` (13 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
blockdev.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 75baf16..3dfb5a2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2281,11 +2281,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
/* quiesce block driver; prevent further io */
- bdrv_drain_all();
- if (bs) {
- bdrv_flush(bs);
- bdrv_close(bs);
- }
+ blk_remove_bs(blk);
/* if we have a device attached to this BlockDriverState
* then we need to make the drive anonymous until the device
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 10/22] block: Make bdrv_close() static
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (8 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 09/22] blockdev: Use blk_remove_bs() in do_drive_del() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 11/22] block: Add blk_name_taken() Max Reitz
` (12 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
There are no users of bdrv_close() left, except for one of bdrv_open()'s
failure paths, bdrv_close_all() and bdrv_delete(), and that is good.
Make bdrv_close() static so nobody makes the mistake of directly using
bdrv_close() again.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 4 +++-
include/block/block.h | 1 -
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 143c7cc..1590cb3 100644
--- a/block.c
+++ b/block.c
@@ -107,6 +107,8 @@ static int use_bdrv_whitelist;
static NotifierList close_all_notifiers =
NOTIFIER_LIST_INITIALIZER(close_all_notifiers);
+static void bdrv_close(BlockDriverState *bs);
+
#ifdef _WIN32
static int is_windows_drive_prefix(const char *filename)
{
@@ -1851,7 +1853,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
}
-void bdrv_close(BlockDriverState *bs)
+static void bdrv_close(BlockDriverState *bs)
{
BdrvAioNotifier *ban, *ban_next;
diff --git a/include/block/block.h b/include/block/block.h
index 79efccb..8274dbd 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -195,7 +195,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue, Error **errp);
void bdrv_reopen_commit(BDRVReopenState *reopen_state);
void bdrv_reopen_abort(BDRVReopenState *reopen_state);
-void bdrv_close(BlockDriverState *bs);
int bdrv_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 11/22] block: Add blk_name_taken()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (9 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 10/22] block: Make bdrv_close() static Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 12/22] block: Add blk_next_inserted() Max Reitz
` (11 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
There may be BlockBackends which are not returned by blk_by_name(), but
do exist and have a name. blk_name_taken() allows testing whether a
specific name is in use already, independent of whether the BlockBackend
with that name is accessible through blk_by_name().
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block.c | 4 ++--
block/block-backend.c | 19 ++++++++++++++++++-
include/sysemu/block-backend.h | 1 +
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 1590cb3..5bf74b7 100644
--- a/block.c
+++ b/block.c
@@ -899,8 +899,8 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
return;
}
- /* takes care of avoiding namespaces collisions */
- if (blk_by_name(node_name)) {
+ /* takes care of avoiding namespace collisions */
+ if (blk_name_taken(node_name)) {
error_setg(errp, "node-name=%s is conflicting with a device id",
node_name);
return;
diff --git a/block/block-backend.c b/block/block-backend.c
index bcad1dc..c54792d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -86,7 +86,7 @@ BlockBackend *blk_new(const char *name, Notifier *close_all_notifier,
error_setg(errp, "Invalid device name");
return NULL;
}
- if (blk_by_name(name)) {
+ if (blk_name_taken(name)) {
error_setg(errp, "Device with id '%s' already exists", name);
return NULL;
}
@@ -262,6 +262,23 @@ BlockBackend *blk_by_name(const char *name)
}
/*
+ * This function should be used to check whether a certain BlockBackend name is
+ * already taken; blk_by_name() will only search in the list of monitor-owned
+ * BlockBackends which is not necessarily complete.
+ */
+bool blk_name_taken(const char *name)
+{
+ BlockBackend *blk;
+
+ QTAILQ_FOREACH(blk, &blk_backends, link) {
+ if (!strcmp(name, blk->name)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/*
* Return the BlockDriverState attached to @blk if any, else null.
*/
BlockDriverState *blk_bs(BlockBackend *blk)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 9840fef..3790c9b 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -72,6 +72,7 @@ void blk_ref(BlockBackend *blk, Notifier *close_all_notifier);
void blk_unref(BlockBackend *blk, Notifier *close_all_notifier);
const char *blk_name(BlockBackend *blk);
BlockBackend *blk_by_name(const char *name);
+bool blk_name_taken(const char *name);
BlockBackend *blk_next(BlockBackend *blk);
BlockDriverState *blk_bs(BlockBackend *blk);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 12/22] block: Add blk_next_inserted()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (10 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 11/22] block: Add blk_name_taken() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 13/22] block: Add blk_commit_all() and blk_invalidate_cache_all() Max Reitz
` (10 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
This function skips to the next BlockBackend for which blk_is_inserted()
is true.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/block-backend.c | 15 +++++++++++++++
include/sysemu/block-backend.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index c54792d..5505bb8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -235,6 +235,21 @@ BlockBackend *blk_next(BlockBackend *blk)
}
/*
+ * Like blk_next(), but skips all non-inserted BlockBackends (that is,
+ * BlockBackends for which blk_is_inserted() returns false)
+ */
+BlockBackend *blk_next_inserted(BlockBackend *blk)
+{
+ while ((blk = blk_next(blk)) != NULL) {
+ if (blk_is_inserted(blk)) {
+ break;
+ }
+ }
+
+ return blk;
+}
+
+/*
* Return @blk's name, a non-null string.
* Wart: the name is empty iff @blk has been hidden with
* blk_hide_on_behalf_of_do_drive_del().
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 3790c9b..1772db7 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -74,6 +74,7 @@ const char *blk_name(BlockBackend *blk);
BlockBackend *blk_by_name(const char *name);
bool blk_name_taken(const char *name);
BlockBackend *blk_next(BlockBackend *blk);
+BlockBackend *blk_next_inserted(BlockBackend *blk);
BlockDriverState *blk_bs(BlockBackend *blk);
void blk_remove_bs(BlockBackend *blk);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 13/22] block: Add blk_commit_all() and blk_invalidate_cache_all()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (11 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 12/22] block: Add blk_next_inserted() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 14/22] block: Use BlockBackend more Max Reitz
` (9 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
These functions will be changed to iterate through the BDS trees as
defined by the BlockBackends instead of the list of root BDS, therefore
prepare moving their code to the BlockBackend level.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/block-backend.c | 10 ++++++++++
include/sysemu/block-backend.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 5505bb8..3c611c1 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1261,3 +1261,13 @@ BlockBackendRootState *blk_get_root_state(BlockBackend *blk)
{
return &blk->root_state;
}
+
+int blk_commit_all(void)
+{
+ return bdrv_commit_all();
+}
+
+void blk_invalidate_cache_all(Error **errp)
+{
+ bdrv_invalidate_cache_all(errp);
+}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 1772db7..4773878 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -128,7 +128,9 @@ int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
int blk_co_flush(BlockBackend *blk);
int blk_flush(BlockBackend *blk);
int blk_flush_all(void);
+int blk_commit_all(void);
void blk_drain_all(void);
+void blk_invalidate_cache_all(Error **errp);
void blk_set_on_error(BlockBackend *blk, BlockdevOnError on_read_error,
BlockdevOnError on_write_error);
BlockdevOnError blk_get_on_error(BlockBackend *blk, bool is_read);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 14/22] block: Use BlockBackend more
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (12 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 13/22] block: Add blk_commit_all() and blk_invalidate_cache_all() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 15/22] blockdev: Add list of monitor-owned BlockBackends Max Reitz
` (8 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Replace bdrv_drain_all(), bdrv_commmit_all(), bdrv_flush_all(),
bdrv_invalidate_cache_all(), bdrv_next() and occurrences of bdrv_states
by their BlockBackend equivalents.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 22 ++++++++---------
block/block-backend.c | 8 +++----
block/qapi.c | 13 ++++++++--
block/snapshot.c | 3 ++-
blockdev.c | 14 ++++++-----
cpus.c | 7 +++---
migration/block.c | 10 +++++---
migration/migration.c | 4 ++--
monitor.c | 13 ++++++----
qemu-char.c | 3 ++-
qemu-io.c | 2 +-
qmp.c | 9 ++++---
savevm.c | 66 ++++++++++++++++++++++++++++++---------------------
xen-mapcache.c | 3 ++-
14 files changed, 105 insertions(+), 72 deletions(-)
diff --git a/block.c b/block.c
index 5bf74b7..8502957 100644
--- a/block.c
+++ b/block.c
@@ -1690,7 +1690,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
assert(bs_queue != NULL);
- bdrv_drain_all();
+ blk_drain_all();
QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {
@@ -1860,9 +1860,9 @@ static void bdrv_close(BlockDriverState *bs)
if (bs->job) {
block_job_cancel_sync(bs->job);
}
- bdrv_drain_all(); /* complete I/O */
+ blk_drain_all(); /* complete I/O */
bdrv_flush(bs);
- bdrv_drain_all(); /* in case flush left pending I/O */
+ blk_drain_all(); /* in case flush left pending I/O */
if (bs->drv) {
if (bs->backing_hd) {
@@ -1910,15 +1910,15 @@ static void bdrv_close(BlockDriverState *bs)
void bdrv_close_all(void)
{
- BlockDriverState *bs;
+ BlockBackend *blk = NULL;
notifier_list_notify(&close_all_notifiers, NULL);
- QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ AioContext *aio_context = blk_get_aio_context(blk);
aio_context_acquire(aio_context);
- bdrv_close(bs);
+ bdrv_close(blk_bs(blk));
aio_context_release(aio_context);
}
}
@@ -5718,7 +5718,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
{
- bdrv_drain_all(); /* ensure there are no in-flight requests */
+ blk_drain_all(); /* ensure there are no in-flight requests */
bdrv_detach_aio_context(bs);
@@ -5821,14 +5821,14 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
*/
bool bdrv_is_first_non_filter(BlockDriverState *candidate)
{
- BlockDriverState *bs;
+ BlockBackend *blk = NULL;
/* walk down the bs forest recursively */
- QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
+ while ((blk = blk_next_inserted(blk)) != NULL) {
bool perm;
/* try to recurse in this top level bs */
- perm = bdrv_recurse_is_first_non_filter(bs, candidate);
+ perm = bdrv_recurse_is_first_non_filter(blk_bs(blk), candidate);
/* candidate is the first non filter */
if (perm) {
diff --git a/block/block-backend.c b/block/block-backend.c
index 3c611c1..1a411c2 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -265,10 +265,10 @@ const char *blk_name(BlockBackend *blk)
*/
BlockBackend *blk_by_name(const char *name)
{
- BlockBackend *blk;
+ BlockBackend *blk = NULL;
assert(name);
- QTAILQ_FOREACH(blk, &blk_backends, link) {
+ while ((blk = blk_next(blk)) != NULL) {
if (!strcmp(name, blk->name)) {
return blk;
}
@@ -326,9 +326,9 @@ DriveInfo *blk_set_legacy_dinfo(BlockBackend *blk, DriveInfo *dinfo)
*/
BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo)
{
- BlockBackend *blk;
+ BlockBackend *blk = NULL;
- QTAILQ_FOREACH(blk, &blk_backends, link) {
+ while ((blk = blk_next(blk)) != NULL) {
if (blk->legacy_dinfo == dinfo) {
return blk;
}
diff --git a/block/qapi.c b/block/qapi.c
index 43b0be2..0d97d6f 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -395,15 +395,24 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
{
BlockStatsList *head = NULL, **p_next = &head;
BlockDriverState *bs = NULL;
+ BlockBackend *blk = NULL;
/* Just to be safe if query_nodes is not always initialized */
query_nodes = has_query_nodes && query_nodes;
- while ((bs = query_nodes ? bdrv_next_node(bs) : bdrv_next(bs))) {
+ while (query_nodes ? (bs = bdrv_next_node(bs)) != NULL
+ : (blk = blk_next_inserted(blk)) != NULL)
+ {
BlockStatsList *info = g_malloc0(sizeof(*info));
- AioContext *ctx = bdrv_get_aio_context(bs);
+ AioContext *ctx = blk ? blk_get_aio_context(blk)
+ : bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
+
+ if (blk) {
+ bs = blk_bs(blk);
+ }
+
info->value = bdrv_query_stats(bs, !query_nodes);
aio_context_release(ctx);
diff --git a/block/snapshot.c b/block/snapshot.c
index 698e1a1..e92e2d4 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -24,6 +24,7 @@
#include "block/snapshot.h"
#include "block/block_int.h"
+#include "sysemu/block-backend.h"
QemuOptsList internal_snapshot_opts = {
.name = "snapshot",
@@ -238,7 +239,7 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
}
/* drain all pending i/o before deleting snapshot */
- bdrv_drain_all();
+ blk_drain_all();
if (drv->bdrv_snapshot_delete) {
return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
diff --git a/blockdev.c b/blockdev.c
index 3dfb5a2..8f21c4a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1156,7 +1156,7 @@ void do_commit(Monitor *mon, const QDict *qdict)
int ret;
if (!strcmp(device, "all")) {
- ret = bdrv_commit_all();
+ ret = blk_commit_all();
} else {
bs = bdrv_find(device);
if (!bs) {
@@ -1830,7 +1830,7 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
QSIMPLEQ_INIT(&snap_bdrv_states);
/* drain all i/o before any operations */
- bdrv_drain_all();
+ blk_drain_all();
/* We don't do anything in this loop that commits us to the operations */
while (NULL != dev_entry) {
@@ -2337,7 +2337,7 @@ void qmp_block_resize(bool has_device, const char *device,
}
/* complete all in-flight operations before resizing the device */
- bdrv_drain_all();
+ blk_drain_all();
ret = bdrv_truncate(bs, size);
switch (ret) {
@@ -2504,7 +2504,7 @@ void qmp_block_commit(const char *device,
bs = blk_bs(blk);
/* drain all i/o before commits */
- bdrv_drain_all();
+ blk_drain_all();
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, errp)) {
goto out;
@@ -3238,12 +3238,14 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
{
BlockJobInfoList *head = NULL, **p_next = &head;
BlockDriverState *bs;
+ BlockBackend *blk = NULL;
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ AioContext *aio_context = blk_get_aio_context(blk);
aio_context_acquire(aio_context);
+ bs = blk_bs(blk);
if (bs->job) {
BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
elem->value = block_job_query(bs->job);
diff --git a/cpus.c b/cpus.c
index 0cdd1d7..7b4aba1 100644
--- a/cpus.c
+++ b/cpus.c
@@ -28,6 +28,7 @@
#include "monitor/monitor.h"
#include "qapi/qmp/qerror.h"
#include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
#include "exec/gdbstub.h"
#include "sysemu/dma.h"
#include "sysemu/kvm.h"
@@ -603,8 +604,8 @@ static int do_vm_stop(RunState state)
qapi_event_send_stop(&error_abort);
}
- bdrv_drain_all();
- ret = bdrv_flush_all();
+ blk_drain_all();
+ ret = blk_flush_all();
return ret;
}
@@ -1302,7 +1303,7 @@ int vm_stop_force_state(RunState state)
runstate_set(state);
/* Make sure to return an error if the flush in a previous vm_stop()
* failed. */
- return bdrv_flush_all();
+ return blk_flush_all();
}
}
diff --git a/migration/block.c b/migration/block.c
index 0c76106..ec435db 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -23,6 +23,7 @@
#include "migration/block.h"
#include "migration/migration.h"
#include "sysemu/blockdev.h"
+#include "sysemu/block-backend.h"
#include <assert.h>
#define BLOCK_SIZE (1 << 20)
@@ -348,6 +349,7 @@ static void unset_dirty_tracking(void)
static void init_blk_migration(QEMUFile *f)
{
BlockDriverState *bs;
+ BlockBackend *blk = NULL;
BlkMigDevState *bmds;
int64_t sectors;
@@ -359,7 +361,9 @@ static void init_blk_migration(QEMUFile *f)
block_mig_state.bulk_completed = 0;
block_mig_state.zero_blocks = migrate_zero_blocks();
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ bs = blk_bs(blk);
+
if (bdrv_is_read_only(bs)) {
continue;
}
@@ -456,7 +460,7 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
blk_mig_lock();
if (bmds_aio_inflight(bmds, sector)) {
blk_mig_unlock();
- bdrv_drain_all();
+ blk_drain_all();
} else {
blk_mig_unlock();
}
@@ -596,7 +600,7 @@ static void blk_mig_cleanup(void)
BlkMigDevState *bmds;
BlkMigBlock *blk;
- bdrv_drain_all();
+ blk_drain_all();
unset_dirty_tracking();
diff --git a/migration/migration.c b/migration/migration.c
index b3adbc6..f08cd6d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -19,7 +19,7 @@
#include "monitor/monitor.h"
#include "migration/qemu-file.h"
#include "sysemu/sysemu.h"
-#include "block/block.h"
+#include "sysemu/block-backend.h"
#include "qemu/sockets.h"
#include "migration/block.h"
#include "qemu/thread.h"
@@ -104,7 +104,7 @@ static void process_incoming_migration_co(void *opaque)
qemu_announce_self();
/* Make sure all file formats flush their mutable metadata */
- bdrv_invalidate_cache_all(&local_err);
+ blk_invalidate_cache_all(&local_err);
if (local_err) {
qerror_report_err(local_err);
error_free(local_err);
diff --git a/monitor.c b/monitor.c
index c3cc060..0c9d34c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -40,6 +40,7 @@
#include "ui/console.h"
#include "ui/input.h"
#include "sysemu/blockdev.h"
+#include "sysemu/block-backend.h"
#include "audio/audio.h"
#include "disas/disas.h"
#include "sysemu/balloon.h"
@@ -4617,13 +4618,15 @@ void host_net_remove_completion(ReadLineState *rs, int nb_args, const char *str)
static void vm_completion(ReadLineState *rs, const char *str)
{
size_t len;
- BlockDriverState *bs = NULL;
+ BlockDriverState *bs;
+ BlockBackend *blk = NULL;
len = strlen(str);
readline_set_completion_index(rs, len);
- while ((bs = bdrv_next(bs))) {
+ while ((blk = blk_next_inserted(blk)) != NULL) {
SnapshotInfoList *snapshots, *snapshot;
+ bs = blk_bs(blk);
if (!bdrv_can_snapshot(bs)) {
continue;
}
@@ -4670,7 +4673,7 @@ static void monitor_find_completion_by_table(Monitor *mon,
int i;
const char *ptype, *str, *name;
const mon_cmd_t *cmd;
- BlockDriverState *bs;
+ BlockBackend *blk = NULL;
if (nb_args <= 1) {
/* command completion */
@@ -4723,8 +4726,8 @@ static void monitor_find_completion_by_table(Monitor *mon,
case 'B':
/* block device name completion */
readline_set_completion_index(mon->rs, strlen(str));
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
- name = bdrv_get_device_name(bs);
+ while ((blk = blk_next(blk)) != NULL) {
+ name = blk_name(blk);
if (str[0] == '\0' ||
!strncmp(name, str, strlen(str))) {
readline_add_completion(mon->rs, name);
diff --git a/qemu-char.c b/qemu-char.c
index 98d4342..4d3fb60 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -24,6 +24,7 @@
#include "qemu-common.h"
#include "monitor/monitor.h"
#include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
#include "qemu/timer.h"
#include "sysemu/char.h"
#include "hw/usb.h"
@@ -523,7 +524,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
break;
}
case 's':
- bdrv_commit_all();
+ blk_commit_all();
break;
case 'b':
qemu_chr_be_event(chr, CHR_EVENT_BREAK);
diff --git a/qemu-io.c b/qemu-io.c
index 9c0ad37..bbacb06 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -472,7 +472,7 @@ int main(int argc, char **argv)
/*
* Make sure all outstanding requests complete before the program exits.
*/
- bdrv_drain_all();
+ blk_drain_all();
blk_unref(qemuio_blk, NULL);
g_free(readline_state);
diff --git a/qmp.c b/qmp.c
index 2e381a1..d68ed19 100644
--- a/qmp.c
+++ b/qmp.c
@@ -156,8 +156,7 @@ SpiceInfo *qmp_query_spice(Error **errp)
void qmp_cont(Error **errp)
{
Error *local_err = NULL;
- BlockBackend *blk;
- BlockDriverState *bs;
+ BlockBackend *blk = NULL;
if (runstate_needs_reset()) {
error_setg(errp, "Resetting the Virtual Machine is required");
@@ -166,11 +165,11 @@ void qmp_cont(Error **errp)
return;
}
- for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
+ while ((blk = blk_next(blk)) != NULL) {
blk_iostatus_reset(blk);
}
- for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
- bdrv_add_key(bs, NULL, &local_err);
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ bdrv_add_key(blk_bs(blk), NULL, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
diff --git a/savevm.c b/savevm.c
index 8040766..29c71b5 100644
--- a/savevm.c
+++ b/savevm.c
@@ -30,6 +30,7 @@
#include "net/net.h"
#include "monitor/monitor.h"
#include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
#include "qemu/timer.h"
#include "audio/audio.h"
#include "migration/migration.h"
@@ -1053,10 +1054,10 @@ out:
static BlockDriverState *find_vmstate_bs(void)
{
- BlockDriverState *bs = NULL;
- while ((bs = bdrv_next(bs))) {
- if (bdrv_can_snapshot(bs)) {
- return bs;
+ BlockBackend *blk = NULL;
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ if (bdrv_can_snapshot(blk_bs(blk))) {
+ return blk_bs(blk);
}
}
return NULL;
@@ -1068,11 +1069,13 @@ static BlockDriverState *find_vmstate_bs(void)
static int del_existing_snapshots(Monitor *mon, const char *name)
{
BlockDriverState *bs;
+ BlockBackend *blk = NULL;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
Error *err = NULL;
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ bs = blk_bs(blk);
+
if (bdrv_can_snapshot(bs) &&
bdrv_snapshot_find(bs, snapshot, name) >= 0) {
bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
@@ -1094,6 +1097,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
void do_savevm(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
+ BlockBackend *blk = NULL;
QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
int ret;
QEMUFile *f;
@@ -1104,16 +1108,14 @@ void do_savevm(Monitor *mon, const QDict *qdict)
const char *name = qdict_get_try_str(qdict, "name");
/* Verify if there is a device that doesn't support snapshots and is writable */
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
-
- if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ if (blk_is_read_only(blk)) {
continue;
}
- if (!bdrv_can_snapshot(bs)) {
+ if (!bdrv_can_snapshot(blk_bs(blk))) {
monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n",
- bdrv_get_device_name(bs));
+ blk_name(blk));
return;
}
}
@@ -1170,15 +1172,17 @@ void do_savevm(Monitor *mon, const QDict *qdict)
/* create the snapshots */
- bs1 = NULL;
- while ((bs1 = bdrv_next(bs1))) {
+ blk = NULL;
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ bs1 = blk_bs(blk);
+
if (bdrv_can_snapshot(bs1)) {
/* Write VM state size only to the image that contains the state */
sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
ret = bdrv_snapshot_create(bs1, sn);
if (ret < 0) {
monitor_printf(mon, "Error while creating snapshot on '%s'\n",
- bdrv_get_device_name(bs1));
+ blk_name(blk));
}
}
}
@@ -1217,6 +1221,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
int load_vmstate(const char *name)
{
+ BlockBackend *blk;
BlockDriverState *bs, *bs_vm_state;
QEMUSnapshotInfo sn;
QEMUFile *f;
@@ -1240,12 +1245,12 @@ int load_vmstate(const char *name)
/* Verify if there is any device that doesn't support snapshots and is
writable and check if the requested snapshot is available too. */
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
-
- if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+ blk = NULL;
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ if (blk_is_read_only(blk)) {
continue;
}
+ bs = blk_bs(blk);
if (!bdrv_can_snapshot(bs)) {
error_report("Device '%s' is writable but does not support snapshots.",
@@ -1262,10 +1267,12 @@ int load_vmstate(const char *name)
}
/* Flush all IO requests so they don't interfere with the new state. */
- bdrv_drain_all();
+ blk_drain_all();
+
+ blk = NULL;
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ bs = blk_bs(blk);
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs)) {
ret = bdrv_snapshot_goto(bs, name);
if (ret < 0) {
@@ -1298,6 +1305,7 @@ int load_vmstate(const char *name)
void do_delvm(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs;
+ BlockBackend *blk = NULL;
Error *err;
const char *name = qdict_get_str(qdict, "name");
@@ -1306,8 +1314,9 @@ void do_delvm(Monitor *mon, const QDict *qdict)
return;
}
- bs = NULL;
- while ((bs = bdrv_next(bs))) {
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ bs = blk_bs(blk);
+
if (bdrv_can_snapshot(bs)) {
err = NULL;
bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
@@ -1315,7 +1324,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
monitor_printf(mon,
"Error while deleting snapshot on device '%s':"
" %s\n",
- bdrv_get_device_name(bs),
+ blk_name(blk),
error_get_pretty(err));
error_free(err);
}
@@ -1326,6 +1335,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
void do_info_snapshots(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
+ BlockBackend *blk;
QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
int nb_sns, i, ret, available;
int total;
@@ -1353,9 +1363,11 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
for (i = 0; i < nb_sns; i++) {
sn = &sn_tab[i];
available = 1;
- bs1 = NULL;
+ blk = NULL;
+
+ while ((blk = blk_next_inserted(blk)) != NULL) {
+ bs1 = blk_bs(blk);
- while ((bs1 = bdrv_next(bs1))) {
if (bdrv_can_snapshot(bs1) && bs1 != bs) {
ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str);
if (ret < 0) {
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 8cefd0c..1a98a69 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -14,6 +14,7 @@
#include "hw/xen/xen_backend.h"
#include "sysemu/blockdev.h"
+#include "sysemu/block-backend.h"
#include "qemu/bitmap.h"
#include <xen/hvm/params.h>
@@ -421,7 +422,7 @@ void xen_invalidate_map_cache(void)
MapCacheRev *reventry;
/* Flush pending AIO before destroying the mapcache */
- bdrv_drain_all();
+ blk_drain_all();
mapcache_lock();
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 15/22] blockdev: Add list of monitor-owned BlockBackends
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (13 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 14/22] block: Use BlockBackend more Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 16/22] blockdev: Remove blk_hide_on_behalf_of_do_drive_del() Max Reitz
` (7 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
The monitor does hold references to some BlockBackends so it should have
a list of those BBs; blk_backends is a different list, as it contains
references to all BBs (after a follow-up patch, that is), and that
should not be changed because we do need such a list.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 27 ++++++++++++++++++++++++++-
blockdev.c | 14 ++++++++++++++
include/sysemu/block-backend.h | 2 ++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 1a411c2..208c16c 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -28,6 +28,7 @@ struct BlockBackend {
BlockDriverState *bs;
DriveInfo *legacy_dinfo; /* null unless created by drive_new() */
QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */
+ QTAILQ_ENTRY(BlockBackend) monitor_link; /* for monitor_block_backends */
void *dev; /* attached device model, if any */
/* TODO change to DeviceState when all users are qdevified */
@@ -69,6 +70,11 @@ static void drive_info_del(DriveInfo *dinfo);
static QTAILQ_HEAD(, BlockBackend) blk_backends =
QTAILQ_HEAD_INITIALIZER(blk_backends);
+/* All BlockBackends referenced by the monitor and which are iterated through by
+ * blk_next() */
+static QTAILQ_HEAD(, BlockBackend) monitor_block_backends =
+ QTAILQ_HEAD_INITIALIZER(monitor_block_backends);
+
/*
* Create a new BlockBackend with @name, with a reference count of one.
* @name must not be null or empty.
@@ -231,7 +237,8 @@ void blk_unref(BlockBackend *blk, Notifier *close_all_notifier)
*/
BlockBackend *blk_next(BlockBackend *blk)
{
- return blk ? QTAILQ_NEXT(blk, link) : QTAILQ_FIRST(&blk_backends);
+ return blk ? QTAILQ_NEXT(blk, monitor_link)
+ : QTAILQ_FIRST(&monitor_block_backends);
}
/*
@@ -250,6 +257,24 @@ BlockBackend *blk_next_inserted(BlockBackend *blk)
}
/*
+ * Add a BlockBackend into the list of backends referenced by the monitor.
+ * Strictly for use by blockdev.c.
+ */
+void monitor_add_blk(BlockBackend *blk)
+{
+ QTAILQ_INSERT_TAIL(&monitor_block_backends, blk, monitor_link);
+}
+
+/*
+ * Remove a BlockBackend from the list of backends referenced by the monitor.
+ * Strictly for use by blockdev.c.
+ */
+void monitor_remove_blk(BlockBackend *blk)
+{
+ QTAILQ_REMOVE(&monitor_block_backends, blk, monitor_link);
+}
+
+/*
* Return @blk's name, a non-null string.
* Wart: the name is empty iff @blk has been hidden with
* blk_hide_on_behalf_of_do_drive_del().
diff --git a/blockdev.c b/blockdev.c
index 8f21c4a..7f1ad42 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -90,6 +90,8 @@ static int if_max_devs[IF_COUNT] = {
static void monitor_blk_unref_with_can(BlockBackend *blk,
BlockdevCloseAllNotifier *can)
{
+ BlockBackend *blk1 = NULL;
+
if (can) {
assert(can->blk == blk);
} else {
@@ -101,6 +103,15 @@ static void monitor_blk_unref_with_can(BlockBackend *blk,
assert(can);
}
+ /* Thanks to do_drive_del() magic, the BlockBackend is not necessarily
+ * listed at this point */
+ while ((blk1 = blk_next(blk1)) != NULL) {
+ if (blk1 == blk) {
+ monitor_remove_blk(blk);
+ break;
+ }
+ }
+
blk_unref(blk, &can->n);
QTAILQ_REMOVE(&close_all_notifiers, can, next);
g_free(can);
@@ -634,6 +645,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
blk_set_on_error(blk, on_read_error, on_write_error);
+ monitor_add_blk(blk);
+
err_no_bs_opts:
qemu_opts_del(opts);
return blk;
@@ -2290,6 +2303,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
*/
if (blk_get_attached_dev(blk)) {
blk_hide_on_behalf_of_do_drive_del(blk);
+ monitor_remove_blk(blk);
/* Further I/O must not pause the guest */
blk_set_on_error(blk, BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 4773878..5effc8f 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -75,6 +75,8 @@ BlockBackend *blk_by_name(const char *name);
bool blk_name_taken(const char *name);
BlockBackend *blk_next(BlockBackend *blk);
BlockBackend *blk_next_inserted(BlockBackend *blk);
+void monitor_add_blk(BlockBackend *blk);
+void monitor_remove_blk(BlockBackend *blk);
BlockDriverState *blk_bs(BlockBackend *blk);
void blk_remove_bs(BlockBackend *blk);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 16/22] blockdev: Remove blk_hide_on_behalf_of_do_drive_del()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (14 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 15/22] blockdev: Add list of monitor-owned BlockBackends Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 17/22] block: Make bdrv_drain_one() public Max Reitz
` (6 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
This function first removed the BlockBackend from the blk_backends list
and cleared its name so it would no longer be found by blk_name(); since
blk_next() now iterates through monitor_block_backends (which the BB is
removed from in do_drive_del()), this is no longer necessary.
Second, bdrv_make_anon() was called on the BDS. This was intended for
cases where the BDS was owned by that BB alone; in which case the BDS
will no longer exist at this point thanks to the blk_remove_bs() in
do_drive_del().
Therefore, this function does nothing useful anymore. Remove it.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/block-backend.c | 25 ++-----------------------
blockdev.c | 1 -
include/sysemu/block-backend.h | 2 --
3 files changed, 2 insertions(+), 26 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 208c16c..ead1dfc 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -66,7 +66,7 @@ static const AIOCBInfo block_backend_aiocb_info = {
static void drive_info_del(DriveInfo *dinfo);
-/* All the BlockBackends (except for hidden ones) */
+/* All the BlockBackends */
static QTAILQ_HEAD(, BlockBackend) blk_backends =
QTAILQ_HEAD_INITIALIZER(blk_backends);
@@ -174,10 +174,7 @@ static void blk_delete(BlockBackend *blk)
assert(!blk->refcnt);
assert(!blk->dev);
blk_remove_bs(blk);
- /* Avoid double-remove after blk_hide_on_behalf_of_do_drive_del() */
- if (blk->name[0]) {
- QTAILQ_REMOVE(&blk_backends, blk, link);
- }
+ QTAILQ_REMOVE(&blk_backends, blk, link);
g_free(blk->name);
drive_info_del(blk->legacy_dinfo);
g_free(blk);
@@ -362,24 +359,6 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo)
}
/*
- * Hide @blk.
- * @blk must not have been hidden already.
- * Make attached BlockDriverState, if any, anonymous.
- * Once hidden, @blk is invisible to all functions that don't receive
- * it as argument. For example, blk_by_name() won't return it.
- * Strictly for use by do_drive_del().
- * TODO get rid of it!
- */
-void blk_hide_on_behalf_of_do_drive_del(BlockBackend *blk)
-{
- QTAILQ_REMOVE(&blk_backends, blk, link);
- blk->name[0] = 0;
- if (blk->bs) {
- bdrv_make_anon(blk->bs);
- }
-}
-
-/*
* Disassociates the currently associated BlockDriverState from @blk.
*/
void blk_remove_bs(BlockBackend *blk)
diff --git a/blockdev.c b/blockdev.c
index 7f1ad42..a6230e0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2302,7 +2302,6 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
* then we can just get rid of the block driver state right here.
*/
if (blk_get_attached_dev(blk)) {
- blk_hide_on_behalf_of_do_drive_del(blk);
monitor_remove_blk(blk);
/* Further I/O must not pause the guest */
blk_set_on_error(blk, BLOCKDEV_ON_ERROR_REPORT,
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 5effc8f..de54ffa 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -82,8 +82,6 @@ BlockDriverState *blk_bs(BlockBackend *blk);
void blk_remove_bs(BlockBackend *blk);
void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs);
-void blk_hide_on_behalf_of_do_drive_del(BlockBackend *blk);
-
void blk_iostatus_enable(BlockBackend *blk);
bool blk_iostatus_is_enabled(const BlockBackend *blk);
BlockDeviceIoStatus blk_iostatus(const BlockBackend *blk);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 17/22] block: Make bdrv_drain_one() public
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (15 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 16/22] blockdev: Remove blk_hide_on_behalf_of_do_drive_del() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 18/22] block: Move some bdrv_*_all() functions to BB Max Reitz
` (5 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
We will need it in block/block-backend.c.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 2 +-
include/block/block_int.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 8502957..3f3258c 100644
--- a/block.c
+++ b/block.c
@@ -1949,7 +1949,7 @@ static bool bdrv_requests_pending(BlockDriverState *bs)
return false;
}
-static bool bdrv_drain_one(BlockDriverState *bs)
+bool bdrv_drain_one(BlockDriverState *bs)
{
bool bs_busy;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 376ca2f..49d4fdd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -428,6 +428,8 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
void bdrv_set_io_limits(BlockDriverState *bs,
ThrottleConfig *cfg);
+bool bdrv_drain_one(BlockDriverState *bs);
+
/**
* bdrv_add_before_write_notifier:
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 18/22] block: Move some bdrv_*_all() functions to BB
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (16 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 17/22] block: Make bdrv_drain_one() public Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 19/22] block: Remove bdrv_states Max Reitz
` (4 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Move bdrv_drain_all(), bdrv_commit_all(), bdrv_flush_all() and
bdrv_invalidate_cache_all() to BB.
The only operation left is bdrv_close_all(), which cannot be moved to
the BB because it should not only close all BBs, but also all
monitor-owned BDSs.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 97 --------------------------------------------
block/block-backend.c | 104 ++++++++++++++++++++++++++++++++++++++++++------
include/block/block.h | 4 --
stubs/Makefile.objs | 2 +-
stubs/bdrv-commit-all.c | 7 ----
stubs/blk-commit-all.c | 7 ++++
6 files changed, 100 insertions(+), 121 deletions(-)
delete mode 100644 stubs/bdrv-commit-all.c
create mode 100644 stubs/blk-commit-all.c
diff --git a/block.c b/block.c
index 3f3258c..94eb4e0 100644
--- a/block.c
+++ b/block.c
@@ -1977,37 +1977,6 @@ void bdrv_drain(BlockDriverState *bs)
}
}
-/*
- * Wait for pending requests to complete across all BlockDriverStates
- *
- * This function does not flush data to disk, use bdrv_flush_all() for that
- * after calling this function.
- *
- * Note that completion of an asynchronous I/O operation can trigger any
- * number of other I/O operations on other devices---for example a coroutine
- * can be arbitrarily complex and a constant flow of I/O can come until the
- * coroutine is complete. Because of this, it is not possible to have a
- * function to drain a single device's I/O queue.
- */
-void bdrv_drain_all(void)
-{
- /* Always run first iteration so any pending completion BHs run */
- bool busy = true;
- BlockDriverState *bs;
-
- while (busy) {
- busy = false;
-
- QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
-
- aio_context_acquire(aio_context);
- busy |= bdrv_drain_one(bs);
- aio_context_release(aio_context);
- }
- }
-}
-
/* make a BlockDriverState anonymous by removing from bdrv_state and
* graph_bdrv_state list.
Also, NULL terminate the device_name to prevent double remove */
@@ -2300,26 +2269,6 @@ ro_cleanup:
return ret;
}
-int bdrv_commit_all(void)
-{
- BlockDriverState *bs;
-
- QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
-
- aio_context_acquire(aio_context);
- if (bs->drv && bs->backing_hd) {
- int ret = bdrv_commit(bs);
- if (ret < 0) {
- aio_context_release(aio_context);
- return ret;
- }
- }
- aio_context_release(aio_context);
- }
- return 0;
-}
-
/**
* Remove an active request from the tracked requests list
*
@@ -3794,14 +3743,6 @@ BlockDriverState *bdrv_next_node(BlockDriverState *bs)
return QTAILQ_NEXT(bs, node_list);
}
-BlockDriverState *bdrv_next(BlockDriverState *bs)
-{
- if (!bs) {
- return QTAILQ_FIRST(&bdrv_states);
- }
- return QTAILQ_NEXT(bs, device_list);
-}
-
const char *bdrv_get_node_name(const BlockDriverState *bs)
{
return bs->node_name;
@@ -3818,26 +3759,6 @@ int bdrv_get_flags(BlockDriverState *bs)
return bs->open_flags;
}
-int bdrv_flush_all(void)
-{
- BlockDriverState *bs;
- int result = 0;
-
- QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
- int ret;
-
- aio_context_acquire(aio_context);
- ret = bdrv_flush(bs);
- if (ret < 0 && !result) {
- result = ret;
- }
- aio_context_release(aio_context);
- }
-
- return result;
-}
-
int bdrv_has_zero_init_1(BlockDriverState *bs)
{
return 1;
@@ -4997,24 +4918,6 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
}
}
-void bdrv_invalidate_cache_all(Error **errp)
-{
- BlockDriverState *bs;
- Error *local_err = NULL;
-
- QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
- AioContext *aio_context = bdrv_get_aio_context(bs);
-
- aio_context_acquire(aio_context);
- bdrv_invalidate_cache(bs, &local_err);
- aio_context_release(aio_context);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
- }
-}
-
int bdrv_flush(BlockDriverState *bs)
{
Coroutine *co;
diff --git a/block/block-backend.c b/block/block-backend.c
index ead1dfc..e4a4675 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -372,6 +372,9 @@ void blk_remove_bs(BlockBackend *blk)
notifier_list_notify(&blk->remove_bs_notifiers, blk);
blk_update_root_state(blk);
+ /* After this function, the BDS may longer be accessible to blk_*_all()
+ * functions */
+ blk_drain_all();
bdrv_unref(blk->bs);
blk->bs->blk = NULL;
@@ -897,16 +900,6 @@ int blk_flush(BlockBackend *blk)
return bdrv_flush(blk->bs);
}
-int blk_flush_all(void)
-{
- return bdrv_flush_all();
-}
-
-void blk_drain_all(void)
-{
- bdrv_drain_all();
-}
-
void blk_set_on_error(BlockBackend *blk, BlockdevOnError on_read_error,
BlockdevOnError on_write_error)
{
@@ -1266,12 +1259,99 @@ BlockBackendRootState *blk_get_root_state(BlockBackend *blk)
return &blk->root_state;
}
+/*
+ * Wait for pending requests to complete across all BlockBackends
+ *
+ * This function does not flush data to disk, use blk_flush_all() for that
+ * after calling this function.
+ *
+ * Note that completion of an asynchronous I/O operation can trigger any
+ * number of other I/O operations on other devices---for example a coroutine
+ * can be arbitrarily complex and a constant flow of I/O can come until the
+ * coroutine is complete. Because of this, it is not possible to have a
+ * function to drain a single device's I/O queue.
+ */
+void blk_drain_all(void)
+{
+ /* Always run first iteration so any pending completion BHs run */
+ bool busy = true;
+ BlockBackend *blk;
+
+ while (busy) {
+ busy = false;
+
+ QTAILQ_FOREACH(blk, &blk_backends, link) {
+ AioContext *aio_context = blk_get_aio_context(blk);
+
+ if (!blk_is_inserted(blk)) {
+ continue;
+ }
+
+ aio_context_acquire(aio_context);
+ busy |= bdrv_drain_one(blk->bs);
+ aio_context_release(aio_context);
+ }
+ }
+}
+
int blk_commit_all(void)
{
- return bdrv_commit_all();
+ BlockBackend *blk;
+
+ QTAILQ_FOREACH(blk, &blk_backends, link) {
+ AioContext *aio_context = blk_get_aio_context(blk);
+
+ aio_context_acquire(aio_context);
+ if (blk_is_available(blk) && blk->bs->drv && blk->bs->backing_hd) {
+ int ret = bdrv_commit(blk->bs);
+ if (ret < 0) {
+ aio_context_release(aio_context);
+ return ret;
+ }
+ }
+ aio_context_release(aio_context);
+ }
+ return 0;
+}
+
+int blk_flush_all(void)
+{
+ BlockBackend *blk;
+ int result = 0;
+
+ QTAILQ_FOREACH(blk, &blk_backends, link) {
+ AioContext *aio_context = blk_get_aio_context(blk);
+ int ret;
+
+ aio_context_acquire(aio_context);
+ if (blk_is_inserted(blk)) {
+ ret = blk_flush(blk);
+ if (ret < 0 && !result) {
+ result = ret;
+ }
+ }
+ aio_context_release(aio_context);
+ }
+
+ return result;
}
void blk_invalidate_cache_all(Error **errp)
{
- bdrv_invalidate_cache_all(errp);
+ BlockBackend *blk;
+ Error *local_err = NULL;
+
+ QTAILQ_FOREACH(blk, &blk_backends, link) {
+ AioContext *aio_context = blk_get_aio_context(blk);
+
+ aio_context_acquire(aio_context);
+ if (blk_is_inserted(blk)) {
+ blk_invalidate_cache(blk, &local_err);
+ }
+ aio_context_release(aio_context);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ }
}
diff --git a/include/block/block.h b/include/block/block.h
index 8274dbd..2df5bc5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -239,7 +239,6 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
int bdrv_commit(BlockDriverState *bs);
-int bdrv_commit_all(void);
int bdrv_change_backing_file(BlockDriverState *bs,
const char *backing_file, const char *backing_fmt);
void bdrv_register(BlockDriver *bdrv);
@@ -325,15 +324,12 @@ BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
/* Invalidate any cached metadata used by image formats */
void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp);
-void bdrv_invalidate_cache_all(Error **errp);
/* Ensure contents are flushed to disk. */
int bdrv_flush(BlockDriverState *bs);
int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
-int bdrv_flush_all(void);
void bdrv_close_all(void);
void bdrv_drain(BlockDriverState *bs);
-void bdrv_drain_all(void);
int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 5e347d0..56eb2e8 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,5 +1,5 @@
stub-obj-y += arch-query-cpu-def.o
-stub-obj-y += bdrv-commit-all.o
+stub-obj-y += blk-commit-all.o
stub-obj-y += chr-baum-init.o
stub-obj-y += chr-msmouse.o
stub-obj-y += chr-testdev.o
diff --git a/stubs/bdrv-commit-all.c b/stubs/bdrv-commit-all.c
deleted file mode 100644
index a8e0a95..0000000
--- a/stubs/bdrv-commit-all.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "block/block.h"
-
-int bdrv_commit_all(void)
-{
- return 0;
-}
diff --git a/stubs/blk-commit-all.c b/stubs/blk-commit-all.c
new file mode 100644
index 0000000..90b59e5
--- /dev/null
+++ b/stubs/blk-commit-all.c
@@ -0,0 +1,7 @@
+#include "qemu-common.h"
+#include "sysemu/block-backend.h"
+
+int blk_commit_all(void)
+{
+ return 0;
+}
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 19/22] block: Remove bdrv_states
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (17 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 18/22] block: Move some bdrv_*_all() functions to BB Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 20/22] blockdev: Keep track of monitor-owned BDS Max Reitz
` (3 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
Every entry in this list should be a root BDS and as such either be
anchored to a BlockBackend or be owned by the monitor.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 21 +--------------------
include/block/block.h | 1 -
include/block/block_int.h | 2 --
3 files changed, 1 insertion(+), 23 deletions(-)
diff --git a/block.c b/block.c
index 94eb4e0..cbe333d 100644
--- a/block.c
+++ b/block.c
@@ -88,9 +88,6 @@ static void coroutine_fn bdrv_co_do_rw(void *opaque);
static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, BdrvRequestFlags flags);
-static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
- QTAILQ_HEAD_INITIALIZER(bdrv_states);
-
static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
@@ -360,10 +357,7 @@ void bdrv_register(BlockDriver *bdrv)
BlockDriverState *bdrv_new_root(void)
{
- BlockDriverState *bs = bdrv_new();
-
- QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
- return bs;
+ return bdrv_new();
}
BlockDriverState *bdrv_new(void)
@@ -1982,17 +1976,6 @@ void bdrv_drain(BlockDriverState *bs)
Also, NULL terminate the device_name to prevent double remove */
void bdrv_make_anon(BlockDriverState *bs)
{
- /*
- * Take care to remove bs from bdrv_states only when it's actually
- * in it. Note that bs->device_list.tqe_prev is initially null,
- * and gets set to non-null by QTAILQ_INSERT_TAIL(). Establish
- * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by
- * resetting it to null on remove.
- */
- if (bs->device_list.tqe_prev) {
- QTAILQ_REMOVE(&bdrv_states, bs, device_list);
- bs->device_list.tqe_prev = NULL;
- }
if (bs->node_name[0] != '\0') {
QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
}
@@ -2033,8 +2016,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
/* job */
bs_dest->job = bs_src->job;
- /* keep the same entry in bdrv_states */
- bs_dest->device_list = bs_src->device_list;
bs_dest->blk = bs_src->blk;
memcpy(bs_dest->op_blockers, bs_src->op_blockers,
diff --git a/include/block/block.h b/include/block/block.h
index 2df5bc5..84a90b6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -361,7 +361,6 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
Error **errp);
bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base);
BlockDriverState *bdrv_next_node(BlockDriverState *bs);
-BlockDriverState *bdrv_next(BlockDriverState *bs);
int bdrv_is_encrypted(BlockDriverState *bs);
int bdrv_key_required(BlockDriverState *bs);
int bdrv_set_key(BlockDriverState *bs, const char *key);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 49d4fdd..2e9e3e3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -380,8 +380,6 @@ struct BlockDriverState {
char node_name[32];
/* element of the list of named nodes building the graph */
QTAILQ_ENTRY(BlockDriverState) node_list;
- /* element of the list of "drives" the guest sees */
- QTAILQ_ENTRY(BlockDriverState) device_list;
QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
int refcnt;
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 20/22] blockdev: Keep track of monitor-owned BDS
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (18 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 19/22] block: Remove bdrv_states Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 21/22] block: Strip down bdrv_close_all() Max Reitz
` (2 subsequent siblings)
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
...and release the reference on bdrv_close_all().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 2 ++
blockdev.c | 24 ++++++++++++++++++++++++
include/block/block_int.h | 2 ++
3 files changed, 28 insertions(+)
diff --git a/block.c b/block.c
index cbe333d..1ab0e29 100644
--- a/block.c
+++ b/block.c
@@ -2016,6 +2016,8 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
/* job */
bs_dest->job = bs_src->job;
+ /* keep the same entry in the list of monitor-owned BDS */
+ bs_dest->monitor_list = bs_src->monitor_list;
bs_dest->blk = bs_src->blk;
memcpy(bs_dest->op_blockers, bs_src->op_blockers,
diff --git a/blockdev.c b/blockdev.c
index a6230e0..9a6e645 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -56,6 +56,11 @@ typedef struct BlockdevCloseAllNotifier {
static QTAILQ_HEAD(, BlockdevCloseAllNotifier) close_all_notifiers =
QTAILQ_HEAD_INITIALIZER(close_all_notifiers);
+static Notifier *close_all_bdrv_states_notifier;
+
+static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
+ QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states);
+
static const char *const if_name[IF_COUNT] = {
[IF_NONE] = "none",
[IF_IDE] = "ide",
@@ -129,6 +134,17 @@ static void blockdev_close_all_notifier(Notifier *n, void *data)
monitor_blk_unref_with_can(can->blk, can);
}
+static void blockdev_close_all_bdrv_states(Notifier *n, void *data)
+{
+ BlockDriverState *bds, *next_bds;
+
+ QTAILQ_FOREACH_SAFE(bds, &monitor_bdrv_states, monitor_list, next_bds) {
+ bdrv_unref(bds);
+ }
+
+ g_free(n);
+}
+
/**
* Boards may call this to offer board-by-board overrides
* of the default, global values.
@@ -3231,6 +3247,14 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
if (!bs) {
goto fail;
}
+
+ QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
+ if (!close_all_bdrv_states_notifier) {
+ close_all_bdrv_states_notifier = g_new0(Notifier, 1);
+ close_all_bdrv_states_notifier->notify =
+ blockdev_close_all_bdrv_states;
+ bdrv_add_close_all_notifier(close_all_bdrv_states_notifier);
+ }
}
if (bs && bdrv_key_required(bs)) {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2e9e3e3..8dff7f8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -380,6 +380,8 @@ struct BlockDriverState {
char node_name[32];
/* element of the list of named nodes building the graph */
QTAILQ_ENTRY(BlockDriverState) node_list;
+ /* element of the list of monitor-owned BDS */
+ QTAILQ_ENTRY(BlockDriverState) monitor_list;
QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
int refcnt;
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 21/22] block: Strip down bdrv_close_all()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (19 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 20/22] blockdev: Keep track of monitor-owned BDS Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 22/22] iotests: Add test for multiple BB on BDS tree Max Reitz
2015-02-10 8:23 ` [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Paolo Bonzini
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
References to all BlockBackends and BlockDriverStates are now released
by their respective holders; force-closing them is no longer necessary.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 11 +----------
block/block-backend.c | 8 ++++++++
include/sysemu/block-backend.h | 1 +
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index 1ab0e29..68d772e 100644
--- a/block.c
+++ b/block.c
@@ -1904,17 +1904,8 @@ static void bdrv_close(BlockDriverState *bs)
void bdrv_close_all(void)
{
- BlockBackend *blk = NULL;
-
notifier_list_notify(&close_all_notifiers, NULL);
-
- while ((blk = blk_next_inserted(blk)) != NULL) {
- AioContext *aio_context = blk_get_aio_context(blk);
-
- aio_context_acquire(aio_context);
- bdrv_close(blk_bs(blk));
- aio_context_release(aio_context);
- }
+ blk_close_all_finalize();
}
void bdrv_add_close_all_notifier(Notifier *notifier)
diff --git a/block/block-backend.c b/block/block-backend.c
index e4a4675..287dcf3 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -223,6 +223,14 @@ void blk_unref(BlockBackend *blk, Notifier *close_all_notifier)
}
/*
+ * For use by bdrv_close_all()
+ */
+void blk_close_all_finalize(void)
+{
+ assert(QTAILQ_EMPTY(&blk_backends));
+}
+
+/*
* Return the BlockBackend after @blk.
* If @blk is null, return the first one.
* Else, return @blk's next sibling, which may be null.
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index de54ffa..a4cd28f 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -70,6 +70,7 @@ BlockBackend *blk_new_open(const char *name, const char *filename,
Notifier *close_all_notifier, Error **errp);
void blk_ref(BlockBackend *blk, Notifier *close_all_notifier);
void blk_unref(BlockBackend *blk, Notifier *close_all_notifier);
+void blk_close_all_finalize(void);
const char *blk_name(BlockBackend *blk);
BlockBackend *blk_by_name(const char *name);
bool blk_name_taken(const char *name);
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH v2 22/22] iotests: Add test for multiple BB on BDS tree
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (20 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 21/22] block: Strip down bdrv_close_all() Max Reitz
@ 2015-02-09 18:38 ` Max Reitz
2015-02-10 8:23 ` [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Paolo Bonzini
22 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-09 18:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
This adds a test for having multiple BlockBackends in one BDS tree. In
this case, there is one BB for the protocol BDS and one BB for the
format BDS in a simple two-BDS tree (with the protocol BDS and BB added
first).
When bdrv_close_all() is executed, no cached data from any BDS should be
lost; the protocol BDS may not be closed until the format BDS is closed.
Otherwise, metadata updates may be lost.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/117 | 86 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/117.out | 14 ++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 101 insertions(+)
create mode 100755 tests/qemu-iotests/117
create mode 100644 tests/qemu-iotests/117.out
diff --git a/tests/qemu-iotests/117 b/tests/qemu-iotests/117
new file mode 100755
index 0000000..7881fc7
--- /dev/null
+++ b/tests/qemu-iotests/117
@@ -0,0 +1,86 @@
+#!/bin/bash
+#
+# Test case for shared BDS between backend trees
+#
+# Copyright (C) 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+_make_test_img 64k
+
+_launch_qemu
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'qmp_capabilities' }" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'blockdev-add',
+ 'arguments': { 'options': { 'id': 'protocol',
+ 'driver': 'file',
+ 'filename': '$TEST_IMG' } } }" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'blockdev-add',
+ 'arguments': { 'options': { 'id': 'format',
+ 'driver': '$IMGFMT',
+ 'file': 'protocol' } } }" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'human-monitor-command',
+ 'arguments': { 'command-line': 'qemu-io format \"write -P 42 0 64k\"' } }" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{ 'execute': 'quit' }" \
+ 'return'
+
+wait=1 _cleanup_qemu
+
+_check_test_img
+
+$QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/117.out b/tests/qemu-iotests/117.out
new file mode 100644
index 0000000..f52dc1a
--- /dev/null
+++ b/tests/qemu-iotests/117.out
@@ -0,0 +1,14 @@
+QA output created by 117
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
+{"return": {}}
+{"return": {}}
+{"return": {}}
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+No errors were found on the image.
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1508d9b..86a9cd4 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -119,4 +119,5 @@
113 rw auto quick
114 rw auto quick
116 rw auto quick
+117 rw auto
118 rw auto
--
2.1.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all()
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
` (21 preceding siblings ...)
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 22/22] iotests: Add test for multiple BB on BDS tree Max Reitz
@ 2015-02-10 8:23 ` Paolo Bonzini
22 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2015-02-10 8:23 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
On 09/02/2015 19:38, Max Reitz wrote:
> Currently, bdrv_close_all() force-closes all BDSs with a BlockBackend,
> which can lead to data corruption (see the iotest added in the final
> patch of this series) and is most certainly very ugly.
>
> This series reworks bdrv_close_all() to notify all owners of a
> BlockBackend that they should release their reference (and additionally
> the monitor releases all its references to BB-less BDSs). This way,
> force-closing becomes unnecessary.
The NBD parts look good.
Paolo
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v2 07/22] block: Add bdrv_close_all() handlers
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 07/22] block: Add bdrv_close_all() handlers Max Reitz
@ 2015-02-13 23:02 ` Max Reitz
0 siblings, 0 replies; 26+ messages in thread
From: Max Reitz @ 2015-02-13 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi
On 2015-02-09 at 13:38, Max Reitz wrote:
> Every time a reference to a BlockBackend is taken, a notifier for
> bdrv_close_all() has to be deposited so the reference holder can
> relinquish its reference when bdrv_close_all() is called. That notifier
> should be revoked on a bdrv_unref() call.
>
> Add a Notifier * parameter to all the functions changing the reference
> count of a BlockBackend: blk_new(), blk_new_with_bs(), blk_new_open(),
> blk_ref() and blk_unref().
>
> By dropping the manual reference handling in hw/block/xen_disk.c, the
> blk_unref() in hw/ide/piix.c can be dropped as well.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/block-backend.c | 69 ++++++++++++++++++++++++++++++++++++------
> blockdev.c | 62 ++++++++++++++++++++++++++++++++++---
> device-hotplug.c | 2 +-
> hw/block/xen_disk.c | 16 +++++++---
> hw/ide/piix.c | 1 -
> include/sysemu/block-backend.h | 13 +++++---
> include/sysemu/blockdev.h | 3 ++
> nbd.c | 26 ++++++++++++++--
> qemu-img.c | 39 ++++++++++++------------
> qemu-io.c | 6 ++--
> qemu-nbd.c | 6 ++--
> 11 files changed, 189 insertions(+), 54 deletions(-)
[snip]
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index ac1e5d6..5e6e4b4 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -694,7 +694,8 @@ int main(int argc, char **argv)
> }
>
> srcpath = argv[optind];
> - blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
> + /* the reference will be passed on nbd_export_new() */
> + blk = blk_new_open("hda", srcpath, NULL, options, flags, NULL, &local_err);
> if (!blk) {
> errx(EXIT_FAILURE, "Failed to blk_new_open '%s': %s", argv[optind],
> error_get_pretty(local_err));
> @@ -728,7 +729,9 @@ int main(int argc, char **argv)
> }
> }
>
> + /* nbd_export_new() takes the reference to blk */
> exp = nbd_export_new(blk, dev_offset, fd_size, nbdflags, nbd_export_closed);
> + blk_unref(blk, NULL);
>
> if (sockpath) {
> fd = unix_socket_incoming(sockpath);
> @@ -773,7 +776,6 @@ int main(int argc, char **argv)
> }
> } while (state != TERMINATED);
>
> - blk_unref(blk);
> if (sockpath) {
> unlink(sockpath);
> }
Because qemu-nbd does not install a handler for relinquishing its
reference, it has to make sure it drops that reference before a
bdrv_close_all() occurs. Therefore, err() and errx() calls after
blk_new_open() (except for the if (!blk) path) and before blk_unref(blk,
NULL) need to be preceded by a blk_unref(blk, NULL).
I'll fix it in v3.
Max
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH v2 01/22] iotests: Move _filter_nbd into common.filter
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 01/22] iotests: Move _filter_nbd into common.filter Max Reitz
@ 2015-02-16 19:31 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2015-02-16 19:31 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 793 bytes --]
On 02/09/2015 11:38 AM, Max Reitz wrote:
> _filter_nbd can be useful for other NBD tests, too, therefore it should
> reside in common.filter, and it should support URLs of the "nbd://"
> format and export names.
>
> The NBD log lines ("/your/source/dir/nbd.c:function():line: error")
> should not be converted to empty lines but removed altogether.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/083 | 13 +------------
> tests/qemu-iotests/083.out | 10 ----------
> tests/qemu-iotests/common.filter | 12 ++++++++++++
> 3 files changed, 13 insertions(+), 22 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2015-02-16 19:31 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-09 18:38 [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 01/22] iotests: Move _filter_nbd into common.filter Max Reitz
2015-02-16 19:31 ` Eric Blake
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 02/22] iotests: Do not redirect qemu's stderr Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 03/22] iotests: Add test for eject under NBD server Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 04/22] quorum: Fix close path Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 05/22] block: Move BDS close notifiers into BB Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 06/22] block: Add bdrv_close_all() notifiers Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 07/22] block: Add bdrv_close_all() handlers Max Reitz
2015-02-13 23:02 ` Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 08/22] block: Use blk_remove_bs() in blk_delete() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 09/22] blockdev: Use blk_remove_bs() in do_drive_del() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 10/22] block: Make bdrv_close() static Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 11/22] block: Add blk_name_taken() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 12/22] block: Add blk_next_inserted() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 13/22] block: Add blk_commit_all() and blk_invalidate_cache_all() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 14/22] block: Use BlockBackend more Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 15/22] blockdev: Add list of monitor-owned BlockBackends Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 16/22] blockdev: Remove blk_hide_on_behalf_of_do_drive_del() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 17/22] block: Make bdrv_drain_one() public Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 18/22] block: Move some bdrv_*_all() functions to BB Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 19/22] block: Remove bdrv_states Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 20/22] blockdev: Keep track of monitor-owned BDS Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 21/22] block: Strip down bdrv_close_all() Max Reitz
2015-02-09 18:38 ` [Qemu-devel] [PATCH v2 22/22] iotests: Add test for multiple BB on BDS tree Max Reitz
2015-02-10 8:23 ` [Qemu-devel] [PATCH v2 00/22] block: Rework bdrv_close_all() Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).