Re: [Qemu-block] [Qemu-devel] [PULL 00/41] Block layer patches

[Kevin Wolf]

Am 15.03.2018 um 18:55 hat John Snow geschrieben:
> 
> 
> On 03/15/2018 12:56 PM, Kevin Wolf wrote:
> > Am 15.03.2018 um 17:42 hat Peter Maydell geschrieben:
> >> On 13 March 2018 at 16:17, Kevin Wolf  wrote:
> >>> The following changes since commit 
> >>> 22ef7ba8e8ce7fef297549b3defcac333742b804:
> >>>
> >>>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
> >>> into staging (2018-03-13 11:42:45 +)
> >>>
> >>> are available in the git repository at:
> >>>
> >>>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >>>
> >>> for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
> >>>
> >>>   block/mirror: change the semantic of 'force' of block-job-cancel 
> >>> (2018-03-13 16:54:47 +0100)
> >>>
> >>> 
> >>> Block layer patches
> >>>
> >>> 
> >>
> >> I get a compile failure here on some hosts:
> >>
> >> /home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
> >> /home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
> >> uninitialized in this function [-Werror=maybe-uninitialized]
> >>  return rc;
> >>  ^
> >>
> >> I guess the compiler can't always figure out whether there is
> >> guaranteed to be at least one thing in the list ?
> > 
> > I think so.
> > 
> > John, I'll just modify your patch 'blockjobs: add prepare callback' to
> > initialise rc = 0 in this function and send a v2 pull request.
> > 
> > Kevin
> > 
> 
> Oh, interesting. I guess technically the list COULD be empty and that'd
> be perfectly valid. I wonder why my compiler doesn't complain?
> 
> Anyway, initializing to zero seems like the correct behavior, thanks.

You only call block_job_txn_apply() in the context of completing a
specific job, which has to be in its transaction, so I don't think the
list can actually be empty.

Not sure how the compiler is able to infer that, though.

Kevin

Re: [Qemu-block] [Qemu-devel] [PULL 00/41] Block layer patches

[John Snow]

On 03/15/2018 12:56 PM, Kevin Wolf wrote:
> Am 15.03.2018 um 17:42 hat Peter Maydell geschrieben:
>> On 13 March 2018 at 16:17, Kevin Wolf  wrote:
>>> The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:
>>>
>>>   Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' 
>>> into staging (2018-03-13 11:42:45 +)
>>>
>>> are available in the git repository at:
>>>
>>>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>>>
>>> for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:
>>>
>>>   block/mirror: change the semantic of 'force' of block-job-cancel 
>>> (2018-03-13 16:54:47 +0100)
>>>
>>> 
>>> Block layer patches
>>>
>>> 
>>
>> I get a compile failure here on some hosts:
>>
>> /home/pm215/qemu/blockjob.c: In function ‘block_job_txn_apply.isra.8’:
>> /home/pm215/qemu/blockjob.c:524:5: error: ‘rc’ may be used
>> uninitialized in this function [-Werror=maybe-uninitialized]
>>  return rc;
>>  ^
>>
>> I guess the compiler can't always figure out whether there is
>> guaranteed to be at least one thing in the list ?
> 
> I think so.
> 
> John, I'll just modify your patch 'blockjobs: add prepare callback' to
> initialise rc = 0 in this function and send a v2 pull request.
> 
> Kevin
> 

Oh, interesting. I guess technically the list COULD be empty and that'd
be perfectly valid. I wonder why my compiler doesn't complain?

Anyway, initializing to zero seems like the correct behavior, thanks.

--js

Re: [Qemu-block] [Qemu-devel] [PULL 00/41] Block layer patches

[no-reply]

Hi,

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

Type: series
Message-id: 20180313161803.1814-1-kw...@redhat.com
Subject: [Qemu-devel] [PULL 00/41] Block layer patches

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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
 t [tag update]patchew/20180307082512.14203-1-be...@igalia.com -> 
patchew/20180307082512.14203-1-be...@igalia.com
 t [tag update]
patchew/20180313153458.26822-1-peter.mayd...@linaro.org -> 
patchew/20180313153458.26822-1-peter.mayd...@linaro.org
 * [new tag]   patchew/20180313161803.1814-1-kw...@redhat.com -> 
patchew/20180313161803.1814-1-kw...@redhat.com
Switched to a new branch 'test'
73ceee78e6 block/mirror: change the semantic of 'force' of block-job-cancel
7363482c0a vpc: Require aligned size in .bdrv_co_create
2882d5ed9b vpc: Support .bdrv_co_create
fc6a997d9c vhdx: Support .bdrv_co_create
20ef05f192 vdi: Make comments consistent with other drivers
459ee653e4 qed: Support .bdrv_co_create
8bba4791b7 qcow: Support .bdrv_co_create
f64c119db2 qemu-iotests: Enable write tests for parallels
2080c0a1ab parallels: Support .bdrv_co_create
9faa105c59 iotests: Add regression test for commit base locking
6a296d9cfe block: Fix flags in reopen queue
781f48c549 vdi: Implement .bdrv_co_create
4cab0e18bb vdi: Move file creation to vdi_co_create_opts
891969da22 vdi: Pull option parsing from vdi_co_create
49280fb721 qemu-iotests: Test luks QMP image creation
0a4d72fa21 luks: Catch integer overflow for huge sizes
6fda8b9a38 luks: Turn invalid assertion into check
c88dc7ac6e luks: Support .bdrv_co_create
681d5dff50 luks: Create block_crypto_co_create_generic()
d641340c5a luks: Separate image file creation from formatting
8cae2fd8e8 tests/test-blockjob: test cancellations
1390b7c37d iotests: test manual job dismissal
1ad1823194 blockjobs: Expose manual property
55441ac858 blockjobs: add block-job-finalize
4a6e1bfbb0 blockjobs: add PENDING status and event
ca394bb9c1 blockjobs: add waiting status
92017bd151 blockjobs: add prepare callback
ef89eb33ad blockjobs: add block_job_txn_apply function
225d9d25ba blockjobs: add commit, abort, clean helpers
2de3034128 blockjobs: ensure abort is called for cancelled jobs
37ef0263ce blockjobs: add block_job_dismiss
b59095a50b blockjobs: add NULL state
4c49f9fa27 blockjobs: add CONCLUDED state
7a4f169154 blockjobs: add ABORTING state
ac30b7288c blockjobs: add block_job_verb permission table
068f7c2061 iotests: add pause_wait
8c386101bf blockjobs: add state transition table
d8d8ffcb3d blockjobs: add status enum
99b5fa3cf0 Blockjobs: documentation touchup
318dc73f7e blockjobs: model single jobs as transactions
020497053e blockjobs: fix set-speed kick

=== OUTPUT BEGIN ===
Checking PATCH 1/41: blockjobs: fix set-speed kick...
Checking PATCH 2/41: blockjobs: model single jobs as transactions...
Checking PATCH 3/41: Blockjobs: documentation touchup...
Checking PATCH 4/41: blockjobs: add status enum...
Checking PATCH 5/41: blockjobs: add state transition table...
ERROR: space prohibited before open square bracket '['
#82: FILE: blockjob.c:48:
+/* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0},

ERROR: space prohibited before open square bracket '['
#83: FILE: blockjob.c:49:
+/* C: */ [BLOCK_JOB_STATUS_CREATED]   = {0, 0, 1, 0, 0, 0},

ERROR: space prohibited before open square bracket '['
#84: FILE: blockjob.c:50:
+/* R: */ [BLOCK_JOB_STATUS_RUNNING]   = {0, 0, 0, 1, 1, 0},

ERROR: space prohibited before open square bracket '['
#85: FILE: blockjob.c:51:
+/* P: */ [BLOCK_JOB_STATUS_PAUSED]= {0, 0, 1, 0, 0, 0},

ERROR: space prohibited before open square bracket '['
#86: FILE: blockjob.c:52:
+/* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1},

ERROR: space prohibited before open square bracket '['
#87: FILE: blockjob.c:53:
+/* S: */ [BLOCK_JOB_STATUS_STANDBY]   = {0, 0, 0, 0, 1, 0},

total: 6 errors, 0 warnings, 88 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/41: iotests: add pause_wait...
Checking PATCH 7/41: blockjobs: add block_job_verb permission table...
Checking PATCH 8/41: blockjobs: add ABORTING state...
ERROR: space prohibited before open square bracket '['
#65: FILE: blockjob.c:48:
+/* U: */