Re: [Qemu-devel] [PATCH v2 00/11] blockjobs: Fix transactional race condition
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 1475272849-19990-1-git-send-email-js...@redhat.com
Subject: [Qemu-devel] [PATCH v2 00/11] blockjobs: Fix transactional race
condition
=== 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 show --no-patch --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
* [new tag] patchew/1475272849-19990-1-git-send-email-js...@redhat.com
-> patchew/1475272849-19990-1-git-send-email-js...@redhat.com
Switched to a new branch 'test'
d3bf4cf iotests: add transactional failure race test
ea3adad blockjob: refactor backup_start as backup_job_create
6b5e7c6 blockjob: add block_job_start
fc02e4d blockjob: add .start field
f34ba3c blockjob: add .clean property
6dc649d blockjobs: fix documentation
3e472b6 blockjobs: split interface into public/private
6cbfc4c blockjobs: Always use block_job_get_aio_context
b2ec7f6 Blockjobs: Internalize user_pause logic
e76a966 blockjob: centralize QMP event emissions
cb59e5e blockjob: fix dead pointer in txn list
=== OUTPUT BEGIN ===
Checking PATCH 1/11: blockjob: fix dead pointer in txn list...
Checking PATCH 2/11: blockjob: centralize QMP event emissions...
Checking PATCH 3/11: Blockjobs: Internalize user_pause logic...
Checking PATCH 4/11: blockjobs: Always use block_job_get_aio_context...
Checking PATCH 5/11: blockjobs: split interface into public/private...
ERROR: struct BlockJobDriver should normally be const
#301: FILE: include/block/blockjob.h:31:
+typedef struct BlockJobDriver BlockJobDriver;
ERROR: struct BlockJobDriver should normally be const
#535: FILE: include/block/blockjob_int.h:37:
+struct BlockJobDriver {
ERROR: space prohibited between function name and open parenthesis '('
#579: FILE: include/block/blockjob_int.h:81:
+void coroutine_fn (*pause)(BlockJob *job);
ERROR: space prohibited between function name and open parenthesis '('
#586: FILE: include/block/blockjob_int.h:88:
+void coroutine_fn (*resume)(BlockJob *job);
total: 4 errors, 0 warnings, 805 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 6/11: blockjobs: fix documentation...
Checking PATCH 7/11: blockjob: add .clean property...
Checking PATCH 8/11: blockjob: add .start field...
Checking PATCH 9/11: blockjob: add block_job_start...
Checking PATCH 10/11: blockjob: refactor backup_start as backup_job_create...
Checking PATCH 11/11: iotests: add transactional failure race test...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org
[Qemu-devel] [PATCH v2 00/11] blockjobs: Fix transactional race condition
There are a few problems with transactional job completion right now. First, if jobs complete so quickly they complete before remaining jobs get a chance to join the transaction, the completion mode can leave well known state and the QLIST can get corrupted and the transactional jobs can complete in batches or phases instead of all together. Second, if two or more jobs defer to the main loop at roughly the same time, it's possible for one job's cleanup to directly invoke the other job's cleanup from within the same thread, leading to a situation that will deadlock the entire transaction. Thanks to Vladimir for pointing out these modes of failure. This series also does a little digging into refactoring Jobs into public and private interfaces. It's somewhat unrelated, but it was easier to include this with this series than separate it out and send it later. This comprises patches 2-6. The actual fixes here are in patches 1 and 7-10. A new test to catch Vladimir's failure scenario is in patch 11. v2: - Lots of differences in patches 2-9. - Cancel should now work on an "unstarted" blockjob. - New refactoring patches. - Added "start" property for BlockJob Drivers. For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch job-manual-start https://github.com/jnsnow/qemu/tree/job-manual-start This version is tagged job-manual-start-v2: https://github.com/jnsnow/qemu/releases/tag/job-manual-start-v2 John Snow (10): blockjob: centralize QMP event emissions Blockjobs: Internalize user_pause logic blockjobs: Always use block_job_get_aio_context blockjobs: split interface into public/private blockjobs: fix documentation blockjob: add .clean property blockjob: add .start field blockjob: add block_job_start blockjob: refactor backup_start as backup_job_create iotests: add transactional failure race test Vladimir Sementsov-Ogievskiy (1): blockjob: fix dead pointer in txn list block/backup.c | 59 --- block/commit.c | 6 +- block/io.c | 6 +- block/mirror.c | 7 +- block/replication.c | 13 +- block/stream.c | 6 +- blockdev.c | 128 +++ blockjob.c | 72 +++-- include/block/block.h| 3 +- include/block/block_int.h| 29 ++-- include/block/blockjob.h | 345 +++- include/block/blockjob_int.h | 366 +++ qemu-img.c | 4 +- tests/qemu-iotests/124 | 91 +++ tests/qemu-iotests/124.out | 4 +- tests/test-blockjob-txn.c| 14 +- tests/test-blockjob.c| 2 +- 17 files changed, 688 insertions(+), 467 deletions(-) create mode 100644 include/block/blockjob_int.h -- 2.7.4
