| Message ID | 20190523160104.21258-1-kwolf@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | block: AioContext management, part 2 | expand |
Patchew URL: https://patchew.org/QEMU/20190523160104.21258-1-kwolf@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH 00/15] block: AioContext management, part 2
Type: series
Message-id: 20190523160104.21258-1-kwolf@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
6ca27a5 block: Remove bdrv_set_aio_context()
984ba6d test-bdrv-drain: Use bdrv_try_set_aio_context()
b054561 iotests: Attach new devices to node in non-default iothread
98252b3 virtio-scsi-test: Test attaching new overlay with iothreads
7f6cd93 block: Remove wrong bdrv_set_aio_context() calls
0419720 blockdev: Use bdrv_try_set_aio_context() for monitor commands
26a1acf block: Move node without parents to main AioContext
cd2b0d7 test-block-iothread: BlockBackend AioContext across root node change
5a4c60c test-block-iothread: Test adding parent to iothread node
1767a79 block: Adjust AioContexts when attaching nodes
16a9345 scsi-disk: Use qdev_prop_drive_iothread
cc6b17c block: Add qdev_prop_drive_iothread property type
2e30704 block: Add BlockBackend.ctx
15a3e30 block: Add Error to blk_set_aio_context()
88715f0 test-block-iothread: Check filter node in test_propagate_mirror
=== OUTPUT BEGIN ===
1/15 Checking commit 88715f0ee897 (test-block-iothread: Check filter node in test_propagate_mirror)
2/15 Checking commit 15a3e308457c (block: Add Error to blk_set_aio_context())
WARNING: Block comments use a leading /* on a separate line
#104: FILE: hw/block/dataplane/virtio-blk.c:289:
+ /* Drain and try to switch bs back to the QEMU main loop. If other users
WARNING: Block comments use a trailing */ on a separate line
#105: FILE: hw/block/dataplane/virtio-blk.c:290:
+ * keep the BlockBackend in the iothread, that's ok */
total: 0 errors, 2 warnings, 259 lines checked
Patch 2/15 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/15 Checking commit 2e30704b8e57 (block: Add BlockBackend.ctx)
WARNING: Block comments use a leading /* on a separate line
#96: FILE: block/block-backend.c:1865:
+ /* FIXME The AioContext of bs and blk can be inconsistent. For the moment,
WARNING: Block comments use a trailing */ on a separate line
#97: FILE: block/block-backend.c:1866:
+ * we prefer the one of bs for compatibility. */
WARNING: Block comments use a leading /* on a separate line
#405: FILE: hw/core/qdev-properties-system.c:83:
+ /* BlockBackends of devices start in the main context and are only
WARNING: Block comments use a trailing */ on a separate line
#406: FILE: hw/core/qdev-properties-system.c:84:
+ * later moved into another context if the device supports that. */
WARNING: line over 80 characters
#647: FILE: tests/test-block-backend.c:40:
+ BlockBackend *blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
WARNING: line over 80 characters
#656: FILE: tests/test-block-backend.c:56:
+ BlockBackend *blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
total: 0 errors, 6 warnings, 533 lines checked
Patch 3/15 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/15 Checking commit cc6b17c2c35e (block: Add qdev_prop_drive_iothread property type)
ERROR: Macros with complex values should be enclosed in parenthesis
#133: FILE: include/hw/block/block.h:61:
+#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
+ DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
+ DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)
total: 1 errors, 0 warnings, 107 lines checked
Patch 4/15 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/15 Checking commit 16a934533950 (scsi-disk: Use qdev_prop_drive_iothread)
ERROR: Macros with complex values should be enclosed in parenthesis
#49: FILE: hw/scsi/scsi-disk.c:2939:
+#define DEFINE_SCSI_DISK_PROPERTIES() \
+ DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk), \
+ DEFINE_BLOCK_PROPERTIES_BASE(SCSIDiskState, qdev.conf), \
+ DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
+ DEFINE_PROP_STRING("ver", SCSIDiskState, version), \
+ DEFINE_PROP_STRING("serial", SCSIDiskState, serial), \
+ DEFINE_PROP_STRING("vendor", SCSIDiskState, vendor), \
+ DEFINE_PROP_STRING("product", SCSIDiskState, product), \
DEFINE_PROP_STRING("device_id", SCSIDiskState, device_id)
total: 1 errors, 0 warnings, 85 lines checked
Patch 5/15 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/15 Checking commit 1767a7917e97 (block: Adjust AioContexts when attaching nodes)
WARNING: Block comments use a leading /* on a separate line
#27: FILE: block.c:2246:
+/* The caller must hold the AioContext lock @child_bs, but not that of @ctx
WARNING: Block comments use a trailing */ on a separate line
#28: FILE: block.c:2247:
+ * (unless @child_bs is already in @ctx). */
WARNING: Block comments use a leading /* on a separate line
#45: FILE: block.c:2275:
+ /* If the AioContexts don't match, first try to move the subtree of
WARNING: Block comments use a trailing */ on a separate line
#47: FILE: block.c:2277:
+ * try moving the parent into the AioContext of child_bs instead. */
WARNING: Block comments use a leading /* on a separate line
#76: FILE: block.c:2306:
+/* If @parent_bs and @child_bs are in different AioContexts, the caller must
WARNING: Block comments use a trailing */ on a separate line
#77: FILE: block.c:2307:
+ * hold the AioContext lock for @child_bs, but not for @parent_bs. */
total: 0 errors, 6 warnings, 149 lines checked
Patch 6/15 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/15 Checking commit 5a4c60c72bfc (test-block-iothread: Test adding parent to iothread node)
8/15 Checking commit cd2b0d787e1b (test-block-iothread: BlockBackend AioContext across root node change)
9/15 Checking commit 26a1acffbe16 (block: Move node without parents to main AioContext)
WARNING: Block comments use a leading /* on a separate line
#23: FILE: block.c:2239:
+ /* When the parent requiring a non-default AioContext is removed, the
WARNING: Block comments use a trailing */ on a separate line
#24: FILE: block.c:2240:
+ * node moves back to the main AioContext */
total: 0 errors, 2 warnings, 28 lines checked
Patch 9/15 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/15 Checking commit 0419720e7eb2 (blockdev: Use bdrv_try_set_aio_context() for monitor commands)
11/15 Checking commit 7f6cd93a5e95 (block: Remove wrong bdrv_set_aio_context() calls)
12/15 Checking commit 98252b3fe05c (virtio-scsi-test: Test attaching new overlay with iothreads)
13/15 Checking commit b0545619fcbb (iotests: Attach new devices to node in non-default iothread)
14/15 Checking commit 984ba6d265e1 (test-bdrv-drain: Use bdrv_try_set_aio_context())
15/15 Checking commit 6ca27a5484fa (block: Remove bdrv_set_aio_context())
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190523160104.21258-1-kwolf@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Am 23.05.2019 um 18:00 hat Kevin Wolf geschrieben: > Recently, a few bugs were reported that resulted from an inconsistent > state regarding AioContexts. Block nodes can end up in different > contexts than their users expect - the AioContext of a node can even > change under the feet of a device with no way for the device to forbid > this. We recently added a few basic checks to scsi-disk and virtio-blk, > but they are by far not enough. > > Part 1 solved the first half of the problem and made sure that > AioContext changes are propagated through the graph as necessary, so > that a bdrv_set_aio_context() correctly pulls everything that uses the > node into the right context. > > This is part 2 that fixes the second half: Attaching new child nodes > where the parent and child are already in different AioContexts. This > operation may only succeed if we can move one of the node into the > context of the other node. > > After applying this series, unchecked bdrv_set_aio_context() doesn't > exist any more and all users call functions that make sure that the > AioContext assignments across the graph stays consistent. Applied to the block branch. Kevin