[Qemu-devel] [PATCH v3 00/13] nbd minimal structured read

Vladimir Sementsov-Ogievskiy posted 13 patches 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171012095319.136610-1-vsementsov@virtuozzo.com
Test checkpatch failed
Test docker passed
Test s390x passed
There is a newer version of this series
include/block/nbd.h                      | 132 +++++++++-
nbd/nbd-internal.h                       |  56 ++---
block/nbd-client.c                       | 401 ++++++++++++++++++++++++++++---
nbd/client.c                             | 205 ++++++++++------
nbd/server.c                             | 189 ++++++++++-----
nbd/trace-events                         |  12 +-
tests/qemu-iotests/nbd-fault-injector.py |   4 +-
7 files changed, 784 insertions(+), 215 deletions(-)
[Qemu-devel] [PATCH v3 00/13] nbd minimal structured read
Posted by Vladimir Sementsov-Ogievskiy 6 years, 6 months ago
Minimally implement nbd structured read extension.

v3:

clone: tag up-nbd-minimal-structured-read-v3 from https://src.openvz.org/scm/~vsementsov/qemu.git
online: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=up-nbd-minimal-structured-read-v3

03: rename  nbd_co_send_reply here too, remove r-b
04-07: splitted nbd server refactoring, with some changes
08: add Eric's r-b
13: rewrite receiving loops by Paolo's suggestion


v2:

01: improve assertion (Eric)
02: add Eric's r-b
06: define NBD_SREP_TYPE_OFFSET_HOLE and NBD_SREP_TYPE_ERROR_OFFSET
    here (Eric)
07: fix return value of nbd_request_simple_option
08-09: new patches
10: mostly rewritten

Vladimir Sementsov-Ogievskiy (13):
  block/nbd-client: assert qiov len once in nbd_co_request
  block/nbd-client: refactor nbd_co_receive_reply
  nbd: rename some simple-request related objects to be _simple_
  nbd/server: structurize simple reply header sending
  nbd/server: do not use NBDReply structure
  nbd/server: refactor nbd_co_send_simple_reply parameters
  nbd-server: simplify reply transmission
  nbd: header constants indenting
  nbd: Minimal structured read for server
  nbd/client: refactor nbd_receive_starttls
  nbd: share some nbd entities to be reused in block/nbd-client.c
  nbd/client: prepare nbd_receive_reply for structured reply
  nbd: Minimal structured read for client

 include/block/nbd.h                      | 132 +++++++++-
 nbd/nbd-internal.h                       |  56 ++---
 block/nbd-client.c                       | 401 ++++++++++++++++++++++++++++---
 nbd/client.c                             | 205 ++++++++++------
 nbd/server.c                             | 189 ++++++++++-----
 nbd/trace-events                         |  12 +-
 tests/qemu-iotests/nbd-fault-injector.py |   4 +-
 7 files changed, 784 insertions(+), 215 deletions(-)

-- 
2.11.1


Re: [Qemu-devel] [PATCH v3 00/13] nbd minimal structured read
Posted by no-reply@patchew.org 6 years, 6 months ago
Hi,

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

Type: series
Message-id: 20171012095319.136610-1-vsementsov@virtuozzo.com
Subject: [Qemu-devel] [PATCH v3 00/13] nbd minimal structured read

=== 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
 t [tag update]            patchew/20171005151313.32015-1-cohuck@redhat.com -> patchew/20171005151313.32015-1-cohuck@redhat.com
Switched to a new branch 'test'
803c96aeca nbd: Minimal structured read for client
55b2692ff6 nbd/client: prepare nbd_receive_reply for structured reply
3155f99d3a nbd: share some nbd entities to be reused in block/nbd-client.c
cf17e2f26c nbd/client: refactor nbd_receive_starttls
7934ecd6e3 nbd: Minimal structured read for server
abfc671d5e nbd: header constants indenting
1c2944d9d1 nbd-server: simplify reply transmission
fe87c0f68c nbd/server: refactor nbd_co_send_simple_reply parameters
a9de77d411 nbd/server: do not use NBDReply structure
691f470797 nbd/server: structurize simple reply header sending
7f1652db0f nbd: rename some simple-request related objects to be _simple_
ab2e6ba2f6 block/nbd-client: refactor nbd_co_receive_reply
c5baea9e86 block/nbd-client: assert qiov len once in nbd_co_request

=== OUTPUT BEGIN ===
Checking PATCH 1/13: block/nbd-client: assert qiov len once in nbd_co_request...
Checking PATCH 2/13: block/nbd-client: refactor nbd_co_receive_reply...
Checking PATCH 3/13: nbd: rename some simple-request related objects to be _simple_...
Checking PATCH 4/13: nbd/server: structurize simple reply header sending...
Checking PATCH 5/13: nbd/server: do not use NBDReply structure...
Checking PATCH 6/13: nbd/server: refactor nbd_co_send_simple_reply parameters...
Checking PATCH 7/13: nbd-server: simplify reply transmission...
Checking PATCH 8/13: nbd: header constants indenting...
Checking PATCH 9/13: nbd: Minimal structured read for server...
Checking PATCH 10/13: nbd/client: refactor nbd_receive_starttls...
Checking PATCH 11/13: nbd: share some nbd entities to be reused in block/nbd-client.c...
ERROR: return of an errno should typically be -ve (return -EPERM)
#46: FILE: include/block/nbd.h:206:
+        return EPERM;

ERROR: return of an errno should typically be -ve (return -EIO)
#48: FILE: include/block/nbd.h:208:
+        return EIO;

ERROR: return of an errno should typically be -ve (return -ENOMEM)
#50: FILE: include/block/nbd.h:210:
+        return ENOMEM;

ERROR: return of an errno should typically be -ve (return -ENOSPC)
#52: FILE: include/block/nbd.h:212:
+        return ENOSPC;

ERROR: return of an errno should typically be -ve (return -ESHUTDOWN)
#54: FILE: include/block/nbd.h:214:
+        return ESHUTDOWN;

ERROR: return of an errno should typically be -ve (return -EINVAL)
#56: FILE: include/block/nbd.h:216:
+        return EINVAL;

ERROR: return of an errno should typically be -ve (return -EINVAL)
#59: FILE: include/block/nbd.h:219:
+    return EINVAL;

total: 7 errors, 0 warnings, 149 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 12/13: nbd/client: prepare nbd_receive_reply for structured reply...
Checking PATCH 13/13: nbd: Minimal structured read for client...
=== 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] [PATCH v3 00/13] nbd minimal structured read
Posted by Vladimir Sementsov-Ogievskiy 6 years, 6 months ago
This is a code movement, no changes.

12.10.2017 13:49, no-reply@patchew.org wrote:
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20171012095319.136610-1-vsementsov@virtuozzo.com
> Subject: [Qemu-devel] [PATCH v3 00/13] nbd minimal structured read
>
> === 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
>   t [tag update]            patchew/20171005151313.32015-1-cohuck@redhat.com -> patchew/20171005151313.32015-1-cohuck@redhat.com
> Switched to a new branch 'test'
> 803c96aeca nbd: Minimal structured read for client
> 55b2692ff6 nbd/client: prepare nbd_receive_reply for structured reply
> 3155f99d3a nbd: share some nbd entities to be reused in block/nbd-client.c
> cf17e2f26c nbd/client: refactor nbd_receive_starttls
> 7934ecd6e3 nbd: Minimal structured read for server
> abfc671d5e nbd: header constants indenting
> 1c2944d9d1 nbd-server: simplify reply transmission
> fe87c0f68c nbd/server: refactor nbd_co_send_simple_reply parameters
> a9de77d411 nbd/server: do not use NBDReply structure
> 691f470797 nbd/server: structurize simple reply header sending
> 7f1652db0f nbd: rename some simple-request related objects to be _simple_
> ab2e6ba2f6 block/nbd-client: refactor nbd_co_receive_reply
> c5baea9e86 block/nbd-client: assert qiov len once in nbd_co_request
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/13: block/nbd-client: assert qiov len once in nbd_co_request...
> Checking PATCH 2/13: block/nbd-client: refactor nbd_co_receive_reply...
> Checking PATCH 3/13: nbd: rename some simple-request related objects to be _simple_...
> Checking PATCH 4/13: nbd/server: structurize simple reply header sending...
> Checking PATCH 5/13: nbd/server: do not use NBDReply structure...
> Checking PATCH 6/13: nbd/server: refactor nbd_co_send_simple_reply parameters...
> Checking PATCH 7/13: nbd-server: simplify reply transmission...
> Checking PATCH 8/13: nbd: header constants indenting...
> Checking PATCH 9/13: nbd: Minimal structured read for server...
> Checking PATCH 10/13: nbd/client: refactor nbd_receive_starttls...
> Checking PATCH 11/13: nbd: share some nbd entities to be reused in block/nbd-client.c...
> ERROR: return of an errno should typically be -ve (return -EPERM)
> #46: FILE: include/block/nbd.h:206:
> +        return EPERM;
>
> ERROR: return of an errno should typically be -ve (return -EIO)
> #48: FILE: include/block/nbd.h:208:
> +        return EIO;
>
> ERROR: return of an errno should typically be -ve (return -ENOMEM)
> #50: FILE: include/block/nbd.h:210:
> +        return ENOMEM;
>
> ERROR: return of an errno should typically be -ve (return -ENOSPC)
> #52: FILE: include/block/nbd.h:212:
> +        return ENOSPC;
>
> ERROR: return of an errno should typically be -ve (return -ESHUTDOWN)
> #54: FILE: include/block/nbd.h:214:
> +        return ESHUTDOWN;
>
> ERROR: return of an errno should typically be -ve (return -EINVAL)
> #56: FILE: include/block/nbd.h:216:
> +        return EINVAL;
>
> ERROR: return of an errno should typically be -ve (return -EINVAL)
> #59: FILE: include/block/nbd.h:219:
> +    return EINVAL;
>
> total: 7 errors, 0 warnings, 149 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 12/13: nbd/client: prepare nbd_receive_reply for structured reply...
> Checking PATCH 13/13: nbd: Minimal structured read for client...
> === 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


-- 
Best regards,
Vladimir


Re: [Qemu-devel] [PATCH v3 00/13] nbd minimal structured read
Posted by Eric Blake 6 years, 6 months ago
On 10/12/2017 06:15 AM, Vladimir Sementsov-Ogievskiy wrote:
> This is a code movement, no changes.
> 

>> Checking PATCH 11/13: nbd: share some nbd entities to be reused in
>> block/nbd-client.c...
>> ERROR: return of an errno should typically be -ve (return -EPERM)
>> #46: FILE: include/block/nbd.h:206:
>> +        return EPERM;

Indeed.  We could avoid the false positive by doing:

case ...:
    ret = EPERM;
    break;
...
return ret;

to hide the positive errno value, but I'm also fine ignoring checkpatch
on this one.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 00/13] nbd minimal structured read
Posted by Eric Blake 6 years, 6 months ago
On 10/12/2017 04:53 AM, Vladimir Sementsov-Ogievskiy wrote:
> Minimally implement nbd structured read extension.
> 
> v3:
> 
> clone: tag up-nbd-minimal-structured-read-v3 from https://src.openvz.org/scm/~vsementsov/qemu.git
> online: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=up-nbd-minimal-structured-read-v3
> 
> 03: rename  nbd_co_send_reply here too, remove r-b
> 04-07: splitted nbd server refactoring, with some changes

English does not have 'splitted'; 'split' is one of those irregular
verbs that is the same form for present, past, and past participle tenses.

I've pushed 1-7 with tweaks to my NBD staging queue:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd

Let me know if I need to redo any of the tweaks I made to your work.

> 08: add Eric's r-b
> 13: rewrite receiving loops by Paolo's suggestion
> 

and I will resume review of the rest tomorrow.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 00/13] nbd minimal structured read
Posted by Vladimir Sementsov-Ogievskiy 6 years, 6 months ago
13.10.2017 01:39, Eric Blake wrote:
> On 10/12/2017 04:53 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Minimally implement nbd structured read extension.
>>
>> v3:
>>
>> clone: tag up-nbd-minimal-structured-read-v3 from https://src.openvz.org/scm/~vsementsov/qemu.git
>> online: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=up-nbd-minimal-structured-read-v3
>>
>> 03: rename  nbd_co_send_reply here too, remove r-b
>> 04-07: splitted nbd server refactoring, with some changes
> English does not have 'splitted'; 'split' is one of those irregular
> verbs that is the same form for present, past, and past participle tenses.
>
> I've pushed 1-7 with tweaks to my NBD staging queue:
> http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd
>
> Let me know if I need to redo any of the tweaks I made to your work.

I've checked, and I agree with all your changes, thank you for this work!

>
>> 08: add Eric's r-b
>> 13: rewrite receiving loops by Paolo's suggestion
>>
> and I will resume review of the rest tomorrow.
>


-- 
Best regards,
Vladimir