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 instead eject the BDS trees from
all BlockBackends and then close the monitor-owned BDS trees, which are
the only BDSs without a BB. In effect, all BDSs are closed just by
getting closed automatically due to their reference count becoming 0.
The benefit over the approach taken in v1 and v2 is that in device
models we often cannot simply drop the reference to a BB because there
may be some user which we forgot about. By ejecting the BDS trees from
the BB, the BB itself becomes unusable, but in a clean way (it will
return errors when accessed, but nothing will crash). Also, it is much
simpler (no reference tracking necessary).
The only disadvantage (I can see) is that the BBs are leaked; but this
does not matter because the qemu process is about to exit anyway.
This series depends on v2 of my series
"blockdev: BlockBackend and media" (or any later version), and on my
patch "virtio-scsi: Allocate op blocker reason before blocking".
v4:
- Patch 1: Simpler regex for removing "nbd:" lines [Fam]
- Patch 2: Made the non-redirection optional instead of mandatory; still
ugly, but at least much less invasive
- Patch 3: Additional line due to the mechanism introduced in patch 2
being optional
- Patch 5:
- Embedded the Notifier objects in VirtIOBlockDataPlane and NBDExport
[Fam]
- Put the notifiers for the block devices on a virtio-scsi bus into a
list, because there can be more than one device [Fam]
To demonstrate the difference, try:
$ x86_64-softmmu/qemu-system-x86_64 \
-object iothread,id=thr -device virtio-scsi-pci,iothread=thr \
-drive if=none,file=foo.qcow2,format=qcow2,id=foo \
-device scsi-hd,drive=foo \
-drive if=none,file=bar.qcow2,format=qcow2,id=bar \
-device scsi-hd,drive=bar
With v3, the !s->blocker assertion in
virtio_scsi_set_up_op_blockers() will fail. With v4, everything
works fine.
git-backport-diff against v3:
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/11:[0002] [FC] 'iotests: Move _filter_nbd into common.filter'
002/11:[down] 'iotests: Make redirecting qemu's stderr optional'
003/11:[0001] [FC] 'iotests: Add test for eject under NBD server'
004/11:[----] [--] 'quorum: Fix close path'
005/11:[0159] [FC] 'block: Move BDS close notifiers into BB'
006/11:[----] [--] 'block: Use blk_remove_bs() in blk_delete()'
007/11:[----] [--] 'blockdev: Use blk_remove_bs() in do_drive_del()'
008/11:[----] [--] 'block: Make bdrv_close() static'
009/11:[----] [--] 'blockdev: Keep track of monitor-owned BDS'
010/11:[----] [--] 'block: Eject BDS tree from BB at bdrv_close_all()'
011/11:[----] [--] 'iotests: Add test for multiple BB on BDS tree'
Max Reitz (11):
iotests: Move _filter_nbd into common.filter
iotests: Make redirecting qemu's stderr optional
iotests: Add test for eject under NBD server
quorum: Fix close path
block: Move BDS close notifiers into BB
block: Use blk_remove_bs() in blk_delete()
blockdev: Use blk_remove_bs() in do_drive_del()
block: Make bdrv_close() static
blockdev: Keep track of monitor-owned BDS
block: Eject BDS tree from BB at bdrv_close_all()
iotests: Add test for multiple BB on BDS tree
block.c | 25 +++-------
block/block-backend.c | 41 ++++++++++++----
block/quorum.c | 3 +-
blockdev-nbd.c | 36 +-------------
blockdev.c | 24 +++++++--
hw/block/dataplane/virtio-blk.c | 77 ++++++++++++++++++++++-------
hw/scsi/virtio-scsi.c | 87 ++++++++++++++++++++++++++++++--
include/block/block.h | 2 -
include/block/block_int.h | 6 ++-
include/hw/virtio/virtio-scsi.h | 10 ++++
include/sysemu/block-backend.h | 4 +-
nbd.c | 13 +++++
stubs/Makefile.objs | 1 +
stubs/blockdev-close-all-bdrv-states.c | 5 ++
tests/qemu-iotests/083 | 13 +----
tests/qemu-iotests/083.out | 10 ----
tests/qemu-iotests/096 | 90 ++++++++++++++++++++++++++++++++++
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 | 12 ++++-
tests/qemu-iotests/group | 2 +
23 files changed, 470 insertions(+), 119 deletions(-)
create mode 100644 stubs/blockdev-close-all-bdrv-states.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
