[Qemu-devel] [PATCH v4 00/11] block: Rework bdrv_close_all()

[Max Reitz <mreitz@redhat.com>]

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