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(-)
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
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
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
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
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
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
© 2016 - 2026 Red Hat, Inc.