[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PATCH v6 for-2.6 00/15] block: Rework bdrv_close_all()
From: |
Max Reitz |
Subject: |
[Qemu-block] [PATCH v6 for-2.6 00/15] block: Rework bdrv_close_all() |
Date: |
Wed, 4 Nov 2015 19:57:32 +0100 |
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.
Note that the approach taken here leaks all BlockBackends. This does not
really matter, however, since qemu is about to exit anyway.
v5 is here (yes, it has been a while):
http://lists.nongnu.org/archive/html/qemu-block/2015-03/msg00044.html
v6:
- Patch 1: Added (leakage of a BDS in a drive-backup error case, becomes
apparent with this series)
- Patch 2: Added (leakage of a BDS when a block job failed to be
created)
- Patch 3: Added (we hacked our way around this so far, but now we have
to fix it, see the commit message)
- Patch 5: Rebase conflicts
- Patch 6: Renamed test from 096 to 140 (rebase conflict)
- Patch 7:
- Contextual rebase conflicts
- Rebase conflict in a comment being removed
- Dropping the reference to exp->blk has been moved from
nbd_export_close() to nbd_export_put(), so the notifier_remove()
call must be moved there, too
- Patch 8: blk_remove_bs() no longer silently ignores blk->bs being
NULL, therefore we have to keep it wrapped in if (blk->bs)
- Patch 9:
- The bdrv_drain_all() and bdrv_flush() calls have been removed from
hmp_drive_del() already, so we don't need to do it anymore.
- As in patch 8, blk_remove_bs() must stay in the if (blk->bs) block
- Patch 11: Do not move the list entry in all_bdrv_states in
bdrv_move_feature_fields(); every BDS is part of that list, so there
is absolutely no point in swapping their positions here
- Patch 12:
- Do not move the list entry in the list of monitor-owned BDS in
bdrv_move_feature_fields(); if a BDS is owned by the monitor, that
will not change by putting another BDS on top of it or by putting it
on top of another BDS
- Add test to qmp_x_blockdev_del() whether the BDS to be deleted (if a
node is to be deleted) is actually monitor-owned
- If qmp_x_blockdev_del() unrefs a node, it must be dropped from the
list of monitor-owned BDS
- Patch 13: Enclose blk_remove_bs() in an if (blk->bs) block
(just like in patches 8 and 9)
- Patch 14:
- One semi-contextual conflict in which an empty line addition is
dropped, because now there already is an empty line
- Changes to bdrv_close_all():
- Add a bdrv_drain_all() at the beginning; it may not do anything,
but it looked like a reasonable thing to do to me
- Add a comment what the loop is for
- A block job will not immediately release its BDS reference once it
is canceled (maybe it did back in March, but now it doesn't), so
we need to call aio_poll() repeatedly until the reference is
dropped
git-backport-diff against v5:
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/15:[down] 'blockdev: Add missing bdrv_unref() in drive-backup'
002/15:[down] 'blockjob: Call bdrv_unref() on creation error'
003/15:[down] 'block: Release dirty bitmaps in bdrv_close()'
004/15:[----] [--] 'iotests: Move _filter_nbd into common.filter'
005/15:[0009] [FC] 'iotests: Make redirecting qemu's stderr optional'
006/15:[0004] [FC] 'iotests: Add test for eject under NBD server'
007/15:[0007] [FC] 'block: Move BDS close notifiers into BB'
008/15:[0004] [FC] 'block: Use blk_remove_bs() in blk_delete()'
009/15:[0006] [FC] 'blockdev: Use blk_remove_bs() in do_drive_del()'
010/15:[----] [-C] 'block: Make bdrv_close() static'
011/15:[0003] [FC] 'block: Add list of all BlockDriverStates'
012/15:[0009] [FC] 'blockdev: Keep track of monitor-owned BDS'
013/15:[0004] [FC] 'block: Add blk_remove_all_bs()'
014/15:[0025] [FC] 'block: Rewrite bdrv_close_all()'
015/15:[----] [-C] 'iotests: Add test for multiple BB on BDS tree'
Max Reitz (15):
blockdev: Add missing bdrv_unref() in drive-backup
blockjob: Call bdrv_unref() on creation error
block: Release dirty bitmaps in bdrv_close()
iotests: Move _filter_nbd into common.filter
iotests: Make redirecting qemu's stderr optional
iotests: Add test for eject under NBD server
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
block: Add list of all BlockDriverStates
blockdev: Keep track of monitor-owned BDS
block: Add blk_remove_all_bs()
block: Rewrite bdrv_close_all()
iotests: Add test for multiple BB on BDS tree
block.c | 83 ++++++++++++++++++++++++-------
block/block-backend.c | 41 +++++++++++++---
blockdev-nbd.c | 37 +-------------
blockdev.c | 28 ++++++++++-
blockjob.c | 1 +
hw/block/dataplane/virtio-blk.c | 77 ++++++++++++++++++++++-------
hw/scsi/virtio-scsi.c | 59 ++++++++++++++++++++++
include/block/block.h | 2 -
include/block/block_int.h | 8 ++-
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/117 | 86 ++++++++++++++++++++++++++++++++
tests/qemu-iotests/117.out | 14 ++++++
tests/qemu-iotests/140 | 90 ++++++++++++++++++++++++++++++++++
tests/qemu-iotests/140.out | 16 ++++++
tests/qemu-iotests/common.filter | 12 +++++
tests/qemu-iotests/common.qemu | 15 +++++-
tests/qemu-iotests/group | 2 +
23 files changed, 518 insertions(+), 109 deletions(-)
create mode 100644 stubs/blockdev-close-all-bdrv-states.c
create mode 100755 tests/qemu-iotests/117
create mode 100644 tests/qemu-iotests/117.out
create mode 100755 tests/qemu-iotests/140
create mode 100644 tests/qemu-iotests/140.out
--
2.6.2
- [Qemu-block] [PATCH v6 for-2.6 00/15] block: Rework bdrv_close_all(),
Max Reitz <=