This series is the first part of implementing the new op blocker system
whose design was agreed on quite a while ago, but proved a bit tricky to
implement in places. There is more work to do to fully replace the old op
blocker system, but realistically we don't have that much time until the 2.9
freeze. So let's merge this series to complement the traditional op blockers
and plan with a second part for the 2.10 timeframe.

The basic idea is that every user of a block node (including things outside the
block layer that go through a BlockBackend, and also other block nodes that
hold references to it) has to declare which low-level operations/permissions it
needs and which operation it allows other users to perform on the same node.
Depending on these declarations, conflicts are avoided by returning an error
for attempts to attach a conflicting user to the same node.

After this series, all users request permissions, and hopefully all of the
permissions they need. For writes and resize, getting the permission first is
actually enforced with assertions. Asserting it for consistent reads is in
theory doable, but would mean introducing a request flag that tell us that
inconsistent reads are okay - and that in all block drivers to recursively
propagate this flag through the backing chain. It might not be worth it.

As stated above, the series doesn't remove the old op blockers yet, though in
theory the new op blockers should block everything that needs to be blocked.
In practice, the read/write/resize blockers should be okay, but
BLK_PERM_GRAPH_MOD isn't to be taken too seriously at the moment. It isn't
really applied consistently and doesn't do much useful yet. Making proper use
of it is left for the part 2 series.

Kevin Wolf (54):
  blockdev: Use BlockBackend to resize in qmp_block_resize()
  qcow2: Use BB for resizing in qcow2_amend_options()
  mirror: Resize active commit base in mirror_run()
  block: Pass BdrvChild to bdrv_truncate()
  block: Attach bs->file only during .bdrv_open()
  block: Factor out bdrv_open_child_bs()
  block: Use BlockBackend for image probing
  block: Factor out bdrv_open_driver()
  block: Add bdrv_new_open_driver()
  vvfat: Use opened node as backing file
  tests: Use opened block node for block job tests
  block: Add op blocker permission constants
  block: Add Error argument to bdrv_attach_child()
  block: Let callers request permissions when attaching a child node
  block: Involve block drivers in permission granting
  block: Default .bdrv_child_perm() for filter drivers
  block: Request child permissions in filter drivers
  block: Default .bdrv_child_perm() for format drivers
  block: Request child permissions in format drivers
  vvfat: Implement .bdrv_child_perm()
  block: Require .bdrv_child_perm() with child nodes
  block: Request real permissions in bdrv_attach_child()
  block: Add permissions to BlockBackend
  block: Add permissions to blk_new()
  block: Add error parameter to blk_insert_bs()
  block: Add BDRV_O_RESIZE for blk_new_open()
  block: Request real permissions in blk_new_open()
  block: Allow error return in BlockDevOps.change_media_cb()
  hw/block: Request permissions
  hw/block: Introduce share-rw qdev property
  blockjob: Add permissions to block_job_create()
  block: Add BdrvChildRole.get_parent_desc()
  block: Include details on permission errors in message
  block: Add BdrvChildRole.stay_at_node
  blockjob: Add permissions to block_job_add_bdrv()
  commit: Use real permissions in commit block job
  commit: Use real permissions for HMP 'commit'
  backup: Use real permissions in backup block job
  block: Fix pending requests check in bdrv_append()
  block: BdrvChildRole.attach/detach() callbacks
  block: Allow backing file links in change_parent_backing_link()
  mirror: Use real permissions in mirror/active commit block job
  stream: Use real permissions in streaming block job
  mirror: Add filter-node-name to blockdev-mirror
  commit: Add filter-node-name to block-commit
  hmp: Request permissions in qemu-io
  migration/block: Use real permissions
  nbd/server: Use real permissions for NBD exports
  tests: Remove FIXME comments
  block: Pass BdrvChild to bdrv_aligned_preadv/pwritev
  block: Assertions for write permissions
  block: Assertions for resize permission
  block: Add Error parameter to bdrv_set_backing_hd()
  block: Add Error parameter to bdrv_append()

 block.c                          | 803 +++++++++++++++++++++++++++++++--------
 block/backup.c                   |  22 +-
 block/blkdebug.c                 |   4 +-
 block/blkreplay.c                |   1 +
 block/blkverify.c                |   1 +
 block/block-backend.c            | 118 +++++-
 block/bochs.c                    |   7 +
 block/cloop.c                    |   7 +
 block/commit.c                   | 168 +++++++-
 block/crypto.c                   |   9 +-
 block/dmg.c                      |   7 +
 block/io.c                       |  40 +-
 block/mirror.c                   | 243 +++++++++---
 block/parallels.c                |  18 +-
 block/qcow.c                     |  14 +-
 block/qcow2-refcount.c           |   2 +-
 block/qcow2.c                    |  38 +-
 block/qed.c                      |  22 +-
 block/quorum.c                   |  11 +-
 block/raw-format.c               |   9 +-
 block/replication.c              |   9 +-
 block/stream.c                   |  45 ++-
 block/vdi.c                      |  10 +-
 block/vhdx-log.c                 |   2 +-
 block/vhdx.c                     |  12 +-
 block/vmdk.c                     |  13 +-
 block/vpc.c                      |  10 +-
 block/vvfat.c                    |  34 +-
 blockdev.c                       |  74 +++-
 blockjob.c                       |  48 ++-
 hmp.c                            |  33 +-
 hw/block/block.c                 |  24 +-
 hw/block/fdc.c                   |  28 +-
 hw/block/m25p80.c                |   8 +
 hw/block/nand.c                  |   7 +
 hw/block/nvme.c                  |   8 +-
 hw/block/onenand.c               |   7 +
 hw/block/pflash_cfi01.c          |  18 +-
 hw/block/pflash_cfi02.c          |  19 +-
 hw/block/virtio-blk.c            |   8 +-
 hw/core/qdev-properties-system.c |   9 +-
 hw/ide/core.c                    |   2 +-
 hw/ide/qdev.c                    |   8 +-
 hw/nvram/spapr_nvram.c           |   8 +
 hw/scsi/scsi-disk.c              |  12 +-
 hw/sd/sd.c                       |   8 +-
 hw/usb/dev-storage.c             |   6 +-
 include/block/block.h            |  49 ++-
 include/block/block_int.h        | 126 +++++-
 include/block/blockjob.h         |   5 +-
 include/block/blockjob_int.h     |   4 +-
 include/hw/block/block.h         |   7 +-
 include/qemu-io.h                |   1 +
 include/sysemu/block-backend.h   |   9 +-
 migration/block.c                |  20 +-
 nbd/server.c                     |  16 +-
 qapi/block-core.json             |  16 +-
 qemu-img.c                       |  12 +-
 qemu-io-cmds.c                   |  28 ++
 tests/qemu-iotests/051.out       |   4 +-
 tests/qemu-iotests/051.pc.out    |  10 +-
 tests/qemu-iotests/055           |  11 +-
 tests/qemu-iotests/085.out       |   2 +-
 tests/qemu-iotests/141           |   2 +-
 tests/qemu-iotests/141.out       |   4 +-
 tests/qemu-iotests/172.out       |  53 +++
 tests/test-blockjob-txn.c        |  12 +-
 tests/test-blockjob.c            |  16 +-
 tests/test-throttle.c            |   7 +-
 69 files changed, 2013 insertions(+), 415 deletions(-)

-- 
1.8.3.1

Am 21.02.2017 um 15:57 hat Kevin Wolf geschrieben:
> This series is the first part of implementing the new op blocker system
> whose design was agreed on quite a while ago, but proved a bit tricky to
> implement in places. There is more work to do to fully replace the old op
> blocker system, but realistically we don't have that much time until the 2.9
> freeze. So let's merge this series to complement the traditional op blockers
> and plan with a second part for the 2.10 timeframe.
> 
> The basic idea is that every user of a block node (including things outside the
> block layer that go through a BlockBackend, and also other block nodes that
> hold references to it) has to declare which low-level operations/permissions it
> needs and which operation it allows other users to perform on the same node.
> Depending on these declarations, conflicts are avoided by returning an error
> for attempts to attach a conflicting user to the same node.
> 
> After this series, all users request permissions, and hopefully all of the
> permissions they need. For writes and resize, getting the permission first is
> actually enforced with assertions. Asserting it for consistent reads is in
> theory doable, but would mean introducing a request flag that tell us that
> inconsistent reads are okay - and that in all block drivers to recursively
> propagate this flag through the backing chain. It might not be worth it.
> 
> As stated above, the series doesn't remove the old op blockers yet, though in
> theory the new op blockers should block everything that needs to be blocked.
> In practice, the read/write/resize blockers should be okay, but
> BLK_PERM_GRAPH_MOD isn't to be taken too seriously at the moment. It isn't
> really applied consistently and doesn't do much useful yet. Making proper use
> of it is left for the part 2 series.

In order to keep the pull request next week at least a bit smaller, I
applied the already reviewed patches 1-11 to the block branch (i.e.
the preparations before the actual op blocker patches start).

Kevin

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH 00/54] New op blocker system, part 1
Message-id: 1487689130-30373-1-git-send-email-kwolf@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1487363905-9480-1-git-send-email-armbru@redhat.com -> patchew/1487363905-9480-1-git-send-email-armbru@redhat.com
 * [new tag]         patchew/1487689130-30373-1-git-send-email-kwolf@redhat.com -> patchew/1487689130-30373-1-git-send-email-kwolf@redhat.com
 - [tag update]      patchew/20170221115512.21918-1-berrange@redhat.com -> patchew/20170221115512.21918-1-berrange@redhat.com
 - [tag update]      patchew/20170221141451.28305-1-marcandre.lureau@redhat.com -> patchew/20170221141451.28305-1-marcandre.lureau@redhat.com
Switched to a new branch 'test'
6250670 block: Add Error parameter to bdrv_append()
81ad909 block: Add Error parameter to bdrv_set_backing_hd()
7279c72 block: Assertions for resize permission
75eb5f4 block: Assertions for write permissions
eb72651 block: Pass BdrvChild to bdrv_aligned_preadv/pwritev
5ed6aa6 tests: Remove FIXME comments
decb6e0 nbd/server: Use real permissions for NBD exports
ca8c8f5 migration/block: Use real permissions
50e1e34 hmp: Request permissions in qemu-io
a839e2e commit: Add filter-node-name to block-commit
66a7bce mirror: Add filter-node-name to blockdev-mirror
202f494 stream: Use real permissions in streaming block job
25bd3cd mirror: Use real permissions in mirror/active commit block job
51ac8c3 block: Allow backing file links in change_parent_backing_link()
23f65d8 block: BdrvChildRole.attach/detach() callbacks
4e76bef block: Fix pending requests check in bdrv_append()
c7b4ae6 backup: Use real permissions in backup block job
4ba5d90 commit: Use real permissions for HMP 'commit'
c096907 commit: Use real permissions in commit block job
2fba186 blockjob: Add permissions to block_job_add_bdrv()
d07ff30 block: Add BdrvChildRole.stay_at_node
ce4d7a7 block: Include details on permission errors in message
12ffb27 block: Add BdrvChildRole.get_parent_desc()
c2ce609 blockjob: Add permissions to block_job_create()
62d3e5e hw/block: Introduce share-rw qdev property
ce2b431 hw/block: Request permissions
960aefe block: Allow error return in BlockDevOps.change_media_cb()
b8f2ec6 block: Request real permissions in blk_new_open()
1a14589 block: Add BDRV_O_RESIZE for blk_new_open()
099cca9 block: Add error parameter to blk_insert_bs()
6b7f8a8 block: Add permissions to blk_new()
3fab508 block: Add permissions to BlockBackend
9dc8540 block: Request real permissions in bdrv_attach_child()
36ab94b block: Require .bdrv_child_perm() with child nodes
f38b810 vvfat: Implement .bdrv_child_perm()
71e6c39 block: Request child permissions in format drivers
743f766 block: Default .bdrv_child_perm() for format drivers
2b0de16 block: Request child permissions in filter drivers
a360de8 block: Default .bdrv_child_perm() for filter drivers
cf4c576 block: Involve block drivers in permission granting
6105af3 block: Let callers request permissions when attaching a child node
edfea57 block: Add Error argument to bdrv_attach_child()
74330bd block: Add op blocker permission constants
daee2fb tests: Use opened block node for block job tests
a81f5d6 vvfat: Use opened node as backing file
64be2da block: Add bdrv_new_open_driver()
8e74a4f block: Factor out bdrv_open_driver()
7da4b8f block: Use BlockBackend for image probing
210ae8d block: Factor out bdrv_open_child_bs()
856867b block: Attach bs->file only during .bdrv_open()
d5f1724 block: Pass BdrvChild to bdrv_truncate()
8a0dd79 mirror: Resize active commit base in mirror_run()
a2f0a85 qcow2: Use BB for resizing in qcow2_amend_options()
0dcd9f3 blockdev: Use BlockBackend to resize in qmp_block_resize()

=== OUTPUT BEGIN ===
Checking PATCH 1/54: blockdev: Use BlockBackend to resize in qmp_block_resize()...
Checking PATCH 2/54: qcow2: Use BB for resizing in qcow2_amend_options()...
Checking PATCH 3/54: mirror: Resize active commit base in mirror_run()...
Checking PATCH 4/54: block: Pass BdrvChild to bdrv_truncate()...
Checking PATCH 5/54: block: Attach bs->file only during .bdrv_open()...
Checking PATCH 6/54: block: Factor out bdrv_open_child_bs()...
ERROR: "foo* bar" should be "foo *bar"
#41: FILE: block.c:1551:
+                   BlockDriverState* parent, const BdrvChildRole *child_role,

ERROR: "foo* bar" should be "foo *bar"
#78: FILE: block.c:1602:
+                           BlockDriverState* parent,

total: 2 errors, 0 warnings, 76 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 7/54: block: Use BlockBackend for image probing...
Checking PATCH 8/54: block: Factor out bdrv_open_driver()...
Checking PATCH 9/54: block: Add bdrv_new_open_driver()...
Checking PATCH 10/54: vvfat: Use opened node as backing file...
ERROR: "(foo*)" should be "(foo *)"
#32: FILE: block/vvfat.c:2971:
+    .instance_size      = sizeof(void*),

ERROR: "(foo**)" should be "(foo **)"
#43: FILE: block/vvfat.c:3042:
+    *(void**) backing->opaque = s;

total: 2 errors, 0 warnings, 25 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 11/54: tests: Use opened block node for block job tests...
Checking PATCH 12/54: block: Add op blocker permission constants...
Checking PATCH 13/54: block: Add Error argument to bdrv_attach_child()...
Checking PATCH 14/54: block: Let callers request permissions when attaching a child node...
Checking PATCH 15/54: block: Involve block drivers in permission granting...
Checking PATCH 16/54: block: Default .bdrv_child_perm() for filter drivers...
Checking PATCH 17/54: block: Request child permissions in filter drivers...
Checking PATCH 18/54: block: Default .bdrv_child_perm() for format drivers...
Checking PATCH 19/54: block: Request child permissions in format drivers...
Checking PATCH 20/54: vvfat: Implement .bdrv_child_perm()...
Checking PATCH 21/54: block: Require .bdrv_child_perm() with child nodes...
Checking PATCH 22/54: block: Request real permissions in bdrv_attach_child()...
Checking PATCH 23/54: block: Add permissions to BlockBackend...
Checking PATCH 24/54: block: Add permissions to blk_new()...
Checking PATCH 25/54: block: Add error parameter to blk_insert_bs()...
Checking PATCH 26/54: block: Add BDRV_O_RESIZE for blk_new_open()...
WARNING: line over 80 characters
#155: FILE: include/block/block.h:85:
+#define BDRV_O_RESIZE      0x0004 /* requests permission for resizing the node */

total: 0 errors, 1 warnings, 96 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 27/54: block: Request real permissions in blk_new_open()...
Checking PATCH 28/54: block: Allow error return in BlockDevOps.change_media_cb()...
Checking PATCH 29/54: hw/block: Request permissions...
Checking PATCH 30/54: hw/block: Introduce share-rw qdev property...
WARNING: line over 80 characters
#56: FILE: include/hw/block/block.h:57:
+    DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, ON_OFF_AUTO_AUTO), \

total: 0 errors, 1 warnings, 403 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 31/54: blockjob: Add permissions to block_job_create()...
Checking PATCH 32/54: block: Add BdrvChildRole.get_parent_desc()...
Checking PATCH 33/54: block: Include details on permission errors in message...
Checking PATCH 34/54: block: Add BdrvChildRole.stay_at_node...
Checking PATCH 35/54: blockjob: Add permissions to block_job_add_bdrv()...
Checking PATCH 36/54: commit: Use real permissions in commit block job...
Checking PATCH 37/54: commit: Use real permissions for HMP 'commit'...
Checking PATCH 38/54: backup: Use real permissions in backup block job...
Checking PATCH 39/54: block: Fix pending requests check in bdrv_append()...
Checking PATCH 40/54: block: BdrvChildRole.attach/detach() callbacks...
Checking PATCH 41/54: block: Allow backing file links in change_parent_backing_link()...
Checking PATCH 42/54: mirror: Use real permissions in mirror/active commit block job...
WARNING: line over 80 characters
#277: FILE: block/mirror.c:1152:
+            /* XXX BLK_PERM_WRITE needs to be allowed so we don't block ourselves

WARNING: line over 80 characters
#278: FILE: block/mirror.c:1153:
+             * at s->base. The other options would be a second filter driver above

total: 0 errors, 2 warnings, 313 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 43/54: stream: Use real permissions in streaming block job...
Checking PATCH 44/54: mirror: Add filter-node-name to blockdev-mirror...
Checking PATCH 45/54: commit: Add filter-node-name to block-commit...
Checking PATCH 46/54: hmp: Request permissions in qemu-io...
Checking PATCH 47/54: migration/block: Use real permissions...
Checking PATCH 48/54: nbd/server: Use real permissions for NBD exports...
Checking PATCH 49/54: tests: Remove FIXME comments...
Checking PATCH 50/54: block: Pass BdrvChild to bdrv_aligned_preadv/pwritev...
Checking PATCH 51/54: block: Assertions for write permissions...
Checking PATCH 52/54: block: Assertions for resize permission...
Checking PATCH 53/54: block: Add Error parameter to bdrv_set_backing_hd()...
Checking PATCH 54/54: block: Add Error parameter to bdrv_append()...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org