[Qemu-devel] [RFC PATCH 00/41] New op blocker system

Kevin Wolf posted 41 patches 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1487006583-24350-1-git-send-email-kwolf@redhat.com
Test checkpatch failed
Test docker passed
Test s390x passed
block.c                          | 615 ++++++++++++++++++++++++++++++++-------
block/backup.c                   |  21 +-
block/blkdebug.c                 |   2 +
block/blkreplay.c                |   1 +
block/blkverify.c                |   1 +
block/block-backend.c            |  95 +++++-
block/bochs.c                    |   7 +
block/cloop.c                    |   7 +
block/commit.c                   | 164 +++++++++--
block/crypto.c                   |   7 +
block/dmg.c                      |   7 +
block/io.c                       |  40 ++-
block/mirror.c                   | 160 ++++++++--
block/parallels.c                |   7 +
block/qcow.c                     |   7 +
block/qcow2.c                    |  19 +-
block/qed.c                      |  19 +-
block/quorum.c                   |  10 +-
block/raw-format.c               |   7 +
block/replication.c              |   7 +
block/stream.c                   |  36 ++-
block/vdi.c                      |   7 +
block/vhdx.c                     |   7 +
block/vmdk.c                     |   7 +
block/vpc.c                      |   7 +
block/vvfat.c                    |  13 +
blockdev.c                       |  27 +-
blockjob.c                       |  39 ++-
hmp.c                            |  33 ++-
hw/block/block.c                 |  21 +-
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              |  11 +-
hw/sd/sd.c                       |   8 +-
hw/usb/dev-storage.c             |   6 +-
include/block/block.h            |   5 +-
include/block/block_int.h        | 124 +++++++-
include/block/blockjob.h         |   4 +-
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 +-
qemu-img.c                       |   6 +-
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/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 +-
65 files changed, 1616 insertions(+), 282 deletions(-)
[Qemu-devel] [RFC PATCH 00/41] New op blocker system
Posted by Kevin Wolf 7 years, 1 month ago
This series is a first rough attempt at 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 still work left to do, but if we want to get this
(or the greatest part of it) into 2.9, it's probably time to start review and
discussion.

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 a subset of them, getting the permission first is
actually enforced with assertions. We can probably add more assertions to this.

The series doesn't remove the old op blockers yet, though in theory the new op
blockers should block everything they used to block. (In practice it's not
completely true yet, some monitor commands don't get the right permissions yet,
in particular related to resize/graph modification.)

Kevin Wolf (41):
  block: Attach bs->file only during .bdrv_open()
  block: Add op blocker permission constants
  block: Add Error argument to bdrv_attach_child()
  block: Let callers request permissions when attaching a child node
  tests: Use opened block node for block job tests
  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: 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_link_name()
  block: Include details on permission errors in message
  block: Add BdrvChildRole.stay_at_node
  blockjob: Add permissions to block_job_add_bdrv()
  block: Factor out bdrv_open_driver()
  block: Add bdrv_new_open_driver()
  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
  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.c                          | 615 ++++++++++++++++++++++++++++++++-------
 block/backup.c                   |  21 +-
 block/blkdebug.c                 |   2 +
 block/blkreplay.c                |   1 +
 block/blkverify.c                |   1 +
 block/block-backend.c            |  95 +++++-
 block/bochs.c                    |   7 +
 block/cloop.c                    |   7 +
 block/commit.c                   | 164 +++++++++--
 block/crypto.c                   |   7 +
 block/dmg.c                      |   7 +
 block/io.c                       |  40 ++-
 block/mirror.c                   | 160 ++++++++--
 block/parallels.c                |   7 +
 block/qcow.c                     |   7 +
 block/qcow2.c                    |  19 +-
 block/qed.c                      |  19 +-
 block/quorum.c                   |  10 +-
 block/raw-format.c               |   7 +
 block/replication.c              |   7 +
 block/stream.c                   |  36 ++-
 block/vdi.c                      |   7 +
 block/vhdx.c                     |   7 +
 block/vmdk.c                     |   7 +
 block/vpc.c                      |   7 +
 block/vvfat.c                    |  13 +
 blockdev.c                       |  27 +-
 blockjob.c                       |  39 ++-
 hmp.c                            |  33 ++-
 hw/block/block.c                 |  21 +-
 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              |  11 +-
 hw/sd/sd.c                       |   8 +-
 hw/usb/dev-storage.c             |   6 +-
 include/block/block.h            |   5 +-
 include/block/block_int.h        | 124 +++++++-
 include/block/blockjob.h         |   4 +-
 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 +-
 qemu-img.c                       |   6 +-
 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/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 +-
 65 files changed, 1616 insertions(+), 282 deletions(-)

-- 
1.8.3.1


Re: [Qemu-devel] [RFC PATCH 00/41] New op blocker system
Posted by no-reply@patchew.org 7 years, 1 month ago
Hi,

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

Subject: [Qemu-devel] [RFC PATCH 00/41] New op blocker system
Message-id: 1487006583-24350-1-git-send-email-kwolf@redhat.com
Type: series

=== 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/1486747506-15876-1-git-send-email-abologna@redhat.com -> patchew/1486747506-15876-1-git-send-email-abologna@redhat.com
 * [new tag]         patchew/1487006583-24350-1-git-send-email-kwolf@redhat.com -> patchew/1487006583-24350-1-git-send-email-kwolf@redhat.com
Switched to a new branch 'test'
74e9065 block: Assertions for write permissions
e39ccbe block: Pass BdrvChild to bdrv_aligned_preadv/pwritev
7b6003a tests: Remove FIXME comments
779ef72 nbd/server: Use real permissions for NBD exports
2a88fb5 migration/block: Use real permissions
b7208e1 hmp: Request permissions in qemu-io
2fd0afd stream: Use real permissions in streaming block job
d1bf1e8 mirror: Use real permissions in mirror/active commit block job
b53e0a7 block: Allow backing file links in change_parent_backing_link()
04ce3a6 block: BdrvChildRole.attach/detach() callbacks
d27507e block: Fix pending requests check in bdrv_append()
c28f58a backup: Use real permissions in backup block job
9c179f9 commit: Use real permissions for HMP 'commit'
d0affd9 commit: Use real permissions in commit block job
fc0d620 block: Add bdrv_new_open_driver()
c4670f2 block: Factor out bdrv_open_driver()
a0b9c4e blockjob: Add permissions to block_job_add_bdrv()
7ae2792 block: Add BdrvChildRole.stay_at_node
6790ced block: Include details on permission errors in message
135cb50 block: Add BdrvChildRole.get_link_name()
aee95b7 blockjob: Add permissions to block_job_create()
e65d733 hw/block: Introduce share-rw qdev property
abf8d2f hw/block: Request permissions
016d652 block: Allow error return in BlockDevOps.change_media_cb()
f0ce22b block: Request real permissions in blk_new_open()
0560a02 block: Add error parameter to blk_insert_bs()
616c5f5 block: Add permissions to blk_new()
9d7081b block: Add permissions to BlockBackend
53aa264 block: Request real permissions in bdrv_attach_child()
2dc4523 block: Require .bdrv_child_perm() with child nodes
a1cb388 vvfat: Implement .bdrv_child_perm()
c3c7960 block: Request child permissions in format drivers
49ff6dd block: Default .bdrv_child_perm() for format drivers
126dedf block: Request child permissions in filter drivers
6eebee7 block: Default .bdrv_child_perm() for filter drivers
017216c block: Involve block drivers in permission granting
18c9ee8 tests: Use opened block node for block job tests
aa32b84 block: Let callers request permissions when attaching a child node
7abb303 block: Add Error argument to bdrv_attach_child()
0a3b05b block: Add op blocker permission constants
e693833 block: Attach bs->file only during .bdrv_open()

=== OUTPUT BEGIN ===
Checking PATCH 1/41: block: Attach bs->file only during .bdrv_open()...
Checking PATCH 2/41: block: Add op blocker permission constants...
Checking PATCH 3/41: block: Add Error argument to bdrv_attach_child()...
Checking PATCH 4/41: block: Let callers request permissions when attaching a child node...
Checking PATCH 5/41: tests: Use opened block node for block job tests...
Checking PATCH 6/41: block: Involve block drivers in permission granting...
ERROR: "foo* bar" should be "foo *bar"
#228: FILE: include/block/block_int.h:357:
+     void (*bdrv_child_perm)(BlockDriverState* bs, BdrvChild *c,

total: 1 errors, 0 warnings, 214 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/41: block: Default .bdrv_child_perm() for filter drivers...
Checking PATCH 8/41: block: Request child permissions in filter drivers...
Checking PATCH 9/41: block: Default .bdrv_child_perm() for format drivers...
Checking PATCH 10/41: block: Request child permissions in format drivers...
Checking PATCH 11/41: vvfat: Implement .bdrv_child_perm()...
Checking PATCH 12/41: block: Require .bdrv_child_perm() with child nodes...
Checking PATCH 13/41: block: Request real permissions in bdrv_attach_child()...
Checking PATCH 14/41: block: Add permissions to BlockBackend...
Checking PATCH 15/41: block: Add permissions to blk_new()...
Checking PATCH 16/41: block: Add error parameter to blk_insert_bs()...
Checking PATCH 17/41: block: Request real permissions in blk_new_open()...
Checking PATCH 18/41: block: Allow error return in BlockDevOps.change_media_cb()...
Checking PATCH 19/41: hw/block: Request permissions...
Checking PATCH 20/41: hw/block: Introduce share-rw qdev property...
WARNING: line over 80 characters
#67: 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, 413 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 21/41: blockjob: Add permissions to block_job_create()...
Checking PATCH 22/41: block: Add BdrvChildRole.get_link_name()...
Checking PATCH 23/41: block: Include details on permission errors in message...
Checking PATCH 24/41: block: Add BdrvChildRole.stay_at_node...
Checking PATCH 25/41: blockjob: Add permissions to block_job_add_bdrv()...
Checking PATCH 26/41: block: Factor out bdrv_open_driver()...
Checking PATCH 27/41: block: Add bdrv_new_open_driver()...
Checking PATCH 28/41: commit: Use real permissions in commit block job...
Checking PATCH 29/41: commit: Use real permissions for HMP 'commit'...
Checking PATCH 30/41: backup: Use real permissions in backup block job...
Checking PATCH 31/41: block: Fix pending requests check in bdrv_append()...
Checking PATCH 32/41: block: BdrvChildRole.attach/detach() callbacks...
Checking PATCH 33/41: block: Allow backing file links in change_parent_backing_link()...
Checking PATCH 34/41: mirror: Use real permissions in mirror/active commit block job...
WARNING: line over 80 characters
#275: FILE: block/mirror.c:1124:
+            /* XXX BLK_PERM_WRITE needs to be allowed so we don't block ourselves

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

total: 0 errors, 2 warnings, 311 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 35/41: stream: Use real permissions in streaming block job...
Checking PATCH 36/41: hmp: Request permissions in qemu-io...
Checking PATCH 37/41: migration/block: Use real permissions...
Checking PATCH 38/41: nbd/server: Use real permissions for NBD exports...
Checking PATCH 39/41: tests: Remove FIXME comments...
Checking PATCH 40/41: block: Pass BdrvChild to bdrv_aligned_preadv/pwritev...
Checking PATCH 41/41: block: Assertions for write permissions...
=== 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