[Qemu-devel] [PATCH v3 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll

Paolo Bonzini posted 7 patches 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170515100059.15795-1-pbonzini@redhat.com
Test checkpatch failed
Test docker passed
Test s390x passed
block/curl.c | 217 +++++++++++++++++++++++++++++++++--------------------------
1 file changed, 121 insertions(+), 96 deletions(-)
[Qemu-devel] [PATCH v3 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll
Posted by Paolo Bonzini 6 years, 11 months ago
Compared to v2, this silences checkpatch and correctly destroy the mutex on
exiting from curl_open with an error.

Paolo

Paolo Bonzini (7):
  curl: strengthen assertion in curl_clean_state
  curl: never invoke callbacks with s->mutex held
  curl: avoid recursive locking of BDRVCURLState mutex
  curl: split curl_find_state/curl_init_state
  curl: convert CURLAIOCB to byte values
  curl: convert readv to coroutines
  curl: do not do aio_poll when waiting for a free CURLState

 block/curl.c | 217 +++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 121 insertions(+), 96 deletions(-)

-- 
2.12.2


Re: [Qemu-devel] [PATCH v3 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll
Posted by no-reply@patchew.org 6 years, 11 months ago
Hi,

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

Subject: [Qemu-devel] [PATCH v3 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll
Message-id: 20170515100059.15795-1-pbonzini@redhat.com
Type: series

=== 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

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
 * [new tag]         patchew/20170515104543.32044-1-kraxel@redhat.com -> patchew/20170515104543.32044-1-kraxel@redhat.com
Switched to a new branch 'test'
e926adb curl: do not do aio_poll when waiting for a free CURLState
4eebc68 curl: convert readv to coroutines
f4b49bc curl: convert CURLAIOCB to byte values
610980d curl: split curl_find_state/curl_init_state
c5ca4dc curl: avoid recursive locking of BDRVCURLState mutex
3ace9ce curl: never invoke callbacks with s->mutex held
b31fcbf curl: strengthen assertion in curl_clean_state

=== OUTPUT BEGIN ===
Checking PATCH 1/7: curl: strengthen assertion in curl_clean_state...
ERROR: spaces required around that '=' (ctx:VxV)
#24: FILE: block/curl.c:536:
+    for (j=0; j < CURL_NUM_ACB; j++) {
           ^

total: 1 errors, 0 warnings, 11 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 2/7: curl: never invoke callbacks with s->mutex held...
Checking PATCH 3/7: curl: avoid recursive locking of BDRVCURLState mutex...
Checking PATCH 4/7: curl: split curl_find_state/curl_init_state...
ERROR: spaces required around that '=' (ctx:VxV)
#40: FILE: block/curl.c:463:
+    for (i=0; i < CURL_NUM_STATES; i++) {
           ^

total: 1 errors, 0 warnings, 92 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 5/7: curl: convert CURLAIOCB to byte values...
Checking PATCH 6/7: curl: convert readv to coroutines...
Checking PATCH 7/7: curl: do not do aio_poll when waiting for a free CURLState...
=== 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
Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll
Posted by Max Reitz 6 years, 11 months ago
On 2017-05-15 12:00, Paolo Bonzini wrote:
> Compared to v2, this silences checkpatch and correctly destroy the mutex on
> exiting from curl_open with an error.
> 
> Paolo
> 
> Paolo Bonzini (7):
>   curl: strengthen assertion in curl_clean_state
>   curl: never invoke callbacks with s->mutex held
>   curl: avoid recursive locking of BDRVCURLState mutex
>   curl: split curl_find_state/curl_init_state
>   curl: convert CURLAIOCB to byte values
>   curl: convert readv to coroutines
>   curl: do not do aio_poll when waiting for a free CURLState
> 
>  block/curl.c | 217 +++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 121 insertions(+), 96 deletions(-)

Modulo checkpatch:

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH v3 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll
Posted by Jeff Cody 6 years, 11 months ago
On Mon, May 15, 2017 at 12:00:52PM +0200, Paolo Bonzini wrote:
> Compared to v2, this silences checkpatch and correctly destroy the mutex on
> exiting from curl_open with an error.
> 
> Paolo
> 
> Paolo Bonzini (7):
>   curl: strengthen assertion in curl_clean_state
>   curl: never invoke callbacks with s->mutex held
>   curl: avoid recursive locking of BDRVCURLState mutex
>   curl: split curl_find_state/curl_init_state
>   curl: convert CURLAIOCB to byte values
>   curl: convert readv to coroutines
>   curl: do not do aio_poll when waiting for a free CURLState
> 
>  block/curl.c | 217 +++++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 121 insertions(+), 96 deletions(-)
> 
> -- 
> 2.12.2
> 

Thanks,

I fixed up the coding styling issues in patch 1 and 4.

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff

Re: [Qemu-devel] [PATCH v3 0/7] curl: locking cleanups/fixes, coroutine conversion, remove aio_poll
Posted by Richard W.M. Jones 6 years, 11 months ago
On Mon, May 15, 2017 at 12:00:52PM +0200, Paolo Bonzini wrote:
> Compared to v2, this silences checkpatch and correctly destroy the mutex on
> exiting from curl_open with an error.

FWIW I hit the curl bug again and tested the current patch
series, and it also fixes the bug (as expected).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW