summaryrefslogtreecommitdiffstats
path: root/nbd
Commit message (Collapse)AuthorAgeFilesLines
* nbd: header constants indentingVladimir Sementsov-Ogievskiy2017-10-131-17/+17
| | | | | | | | | Prepare indenting for the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171012095319.136610-9-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: simplify reply transmissionVladimir Sementsov-Ogievskiy2017-10-131-25/+24Star
| | | | | | | | | | Send qiov via qio_channel_writev_all instead of calling nbd_write twice with a cork. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171012095319.136610-8-vsementsov@virtuozzo.com> [eblake: rebase to tweaks earlier in series] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: refactor nbd_co_send_simple_reply parametersVladimir Sementsov-Ogievskiy2017-10-131-6/+7
| | | | | | | | | Pass client and buffer (*data) parameters directly, to make the function consistent with further structured reply sending functions. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171012095319.136610-7-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: do not use NBDReply structureVladimir Sementsov-Ogievskiy2017-10-131-21/+15Star
| | | | | | | | | | NBDReply structure will be upgraded in future patches to handle both simple and structured replies and will be used only in the client Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171012095319.136610-6-vsementsov@virtuozzo.com> [eblake: rebase to tweaks earlier in series] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: structurize simple reply header sendingVladimir Sementsov-Ogievskiy2017-10-122-24/+14Star
| | | | | | | | | | | Use packed structure instead of pointer arithmetics. Also, merge two redundant traces into one. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171012095319.136610-5-vsementsov@virtuozzo.com> [eblake: tweak and mention impact on traces, fix errp usage] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: rename some simple-request related objects to be _simple_Vladimir Sementsov-Ogievskiy2017-10-124-10/+10
| | | | | | | | | | To be consistent when their _structured_ analogs will be introduced. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171012095319.136610-4-vsementsov@virtuozzo.com> [eblake: also tweak trace message contents] Signed-off-by: Eric Blake <eblake@redhat.com>
* NBD: use g_new() family of functionsMarc-André Lureau2017-10-121-2/+2
| | | | | | Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20171006235023.11952-22-f4bug@amsat.org> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: Use new qio_channel_*_all() functionsEric Blake2017-09-062-79/+7Star
| | | | | | | | | | Rather than open-coding our own read/write-all functions, we can make use of the recently-added qio code. It slightly changes the error message in one of the iotests. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170905191114.5959-4-eblake@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
* nbd/client: fix nbd_send_request to return intVladimir Sementsov-Ogievskiy2017-08-301-1/+1
| | | | | | | | | | Fix nbd_send_request to return int, as it returns a return value of nbd_write (which is int), and the only user of nbd_send_request's return value (nbd_co_send_request) consider it as int too. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170804151440.320927-5-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/client: refactor nbd_receive_replyVladimir Sementsov-Ogievskiy2017-08-301-3/+9
| | | | | | | | | | | Refactor nbd_receive_reply to return 1 on success, 0 on eof, when no data was read and <0 for other cases, because returned size of read data is not actually used. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170804151440.320927-4-vsementsov@virtuozzo.com> [eblake: tweak function comments] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/client: refactor nbd_read_eofVladimir Sementsov-Ogievskiy2017-08-302-14/+24
| | | | | | | | | | | Refactor nbd_read_eof to return 1 on success, 0 on eof, when no data was read and <0 for other cases, because returned size of read data is not actually used. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170804151440.320927-3-vsementsov@virtuozzo.com> [eblake: tweak function comments, rebase to test 083 enhancements] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/client: fix nbd_opt_goVladimir Sementsov-Ogievskiy2017-08-301-2/+0Star
| | | | | | | | | | Do not send NBD_OPT_ABORT to the broken server. After sending NBD_REP_ACK on NBD_OPT_GO server is most probably in transmission phase, when option sending is finished. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170804151440.320927-2-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cacheKevin Wolf2017-08-151-9/+11
| | | | | | | | | | | The "inactive" state of BDS affects whether the permissions can be granted, we must call bdrv_invalidate_cache before bdrv_set_perm to support "-incoming defer" case. Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Signed-off-by: Fam Zheng <famz@redhat.com> Message-Id: <20170815130740.31229-3-famz@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: Fix trace message for disconnectEric Blake2017-08-151-1/+1
| | | | | | | | NBD_CMD_DISC is a disconnect request, not a data discard request. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170811015749.20365-1-eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
* trace-events: fix code style: print 0x before hex numbersVladimir Sementsov-Ogievskiy2017-08-011-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The only exception are groups of numers separated by symbols '.', ' ', ':', '/', like 'ab.09.7d'. This patch is made by the following: > find . -name trace-events | xargs python script.py where script.py is the following python script: ========================= #!/usr/bin/env python import sys import re import fileinput rhex = '%[-+ *.0-9]*(?:[hljztL]|ll|hh)?(?:x|X|"\s*PRI[xX][^"]*"?)' rgroup = re.compile('((?:' + rhex + '[.:/ ])+' + rhex + ')') rbad = re.compile('(?<!0x)' + rhex) files = sys.argv[1:] for fname in files: for line in fileinput.input(fname, inplace=True): arr = re.split(rgroup, line) for i in range(0, len(arr), 2): arr[i] = re.sub(rbad, '0x\g<0>', arr[i]) sys.stdout.write(''.join(arr)) ========================= Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Cornelia Huck <cohuck@redhat.com> Message-id: 20170731160135.12101-5-vsementsov@virtuozzo.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
* nbd: fix memory leak in nbd_opt_go()Philippe Mathieu-Daudé2017-07-281-3/+5
| | | | | | | | | | | nbd/client.c:385:12: warning: Potential leak of memory pointed to by 'buf' Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170727024224.22900-5-f4bug@amsat.org> [introduced in commit 8ecaeae8] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: Fix server reply to NBD_OPT_EXPORT_NAME of older clientsEric Blake2017-07-182-10/+16
| | | | | | | | | | | | | | | | | | | | | | | | | A typo in commit 23e099c set the size of buf[] used in response to NBD_OPT_EXPORT_NAME according to the length needed for old-style negotiation (4 bytes of flag information) instead of the intended 2 bytes used in new style. If the client doesn't enable NBD_FLAG_C_NO_ZEROES, then the server sends two bytes too many, and is then out of sync in response to the client's next command (the bug is masked when modern qemu is the client, since we enable the no zeroes flag). While touching this code, add some more defines to nbd_internal.h rather than having quite so many magic numbers in the .c; also, use "" initialization rather than memset(), and tweak the oldstyle negotiation to better match the spec description of the layout (since the spec is big-endian, skipping two bytes as 0 followed by writing a 2-byte flag is the same as writing a zero-extended 4-byte flag), to make it a bit easier to follow compared to the spec. [checkpatch.pl has some false positives in the comments] Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170717192635.17880-3-eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com>
* nbd: Trace client command being sentEric Blake2017-07-182-2/+3
| | | | | | | | | Make the client trace slightly more legible by including the name of the command being sent. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-Id: <20170717192635.17880-2-eblake@redhat.com>
* nbd: Fix iotests failure due to changed client error messageEric Blake2017-07-171-3/+2Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 8ecaeae8 changed the way the client requests an NBD export, and in the process also changed the resulting error message when the export is not present, breaking a couple of iotests. The error message is now directly given by the server (a failed NBD_OPT_GO) instead of implied by the client (after exhausting NBD_OPT_LIST), but looking at the testsuite changes, it proves worthwhile to reword the error message to be slightly less verbose (as this is one particular error message likely to be hit by a user). Note that the error message is now sensitive to which binary is running the server as well as the client (since the expected output is replaying a message received from the server - for that matter, it depends on a server new enough to understand NBD_OPT_GO); in general iotests are run on client and server from the same source code base so the default setup will pass; but if it proves problematic for people overriding QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, and QEMU_NBD_PROG to point across multiple builds for cross-version integration testing, we may have to later tweak or sanitize the output somehow. Reported-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170717142310.17048-1-eblake@redhat.com> Tested-by: John Snow <jsnow@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com>
* nbd: Implement NBD_INFO_BLOCK_SIZE on clientEric Blake2017-07-142-12/+70
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The upstream NBD Protocol has defined a new extension to allow the server to advertise block sizes to the client, as well as a way for the client to inform the server whether it intends to obey block sizes. When using the block layer as the client, we will obey block sizes; but when used as 'qemu-nbd -c' to hand off to the kernel nbd module as the client, we are still waiting for the kernel to implement a way for us to learn if it will honor block sizes (perhaps by an addition to sysfs, rather than an ioctl), as well as any way to tell the kernel what additional block sizes to obey (NBD_SET_BLKSIZE appears to be accurate for the minimum size, but preferred and maximum sizes would probably be new ioctl()s), so until then, we need to make our request for block sizes conditional. When using ioctl(NBD_SET_BLKSIZE) to hand off to the kernel, use the minimum block size as the sector size if it is larger than 512, which also has the nice effect of cooperating with (non-qemu) servers that don't do read-modify-write when exposing a block device with 4k sectors; it might also allow us to visit a file larger than 2T on a 32-bit kernel. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-10-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Implement NBD_INFO_BLOCK_SIZE on serverEric Blake2017-07-142-4/+45
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The upstream NBD Protocol has defined a new extension to allow the server to advertise block sizes to the client, as well as a way for the client to inform the server that it intends to obey block sizes. Thanks to a recent fix (commit df7b97ff), our real minimum transfer size is always 1 (the block layer takes care of read-modify-write on our behalf), but we're still more efficient if we advertise 512 when the client supports it, as follows: - OPT_INFO, but no NBD_INFO_BLOCK_SIZE: advertise 512, then fail with NBD_REP_ERR_BLOCK_SIZE_REQD; client is free to try something else since we don't disconnect - OPT_INFO with NBD_INFO_BLOCK_SIZE: advertise 512 - OPT_GO, but no NBD_INFO_BLOCK_SIZE: advertise 1 - OPT_GO with NBD_INFO_BLOCK_SIZE: advertise 512 We can also advertise the optimum block size (presumably the cluster size, when exporting a qcow2 file), and our absolute maximum transfer size of 32M, to help newer clients avoid EINVAL failures or abrupt disconnects on oversize requests. We do not reject clients for using the older NBD_OPT_EXPORT_NAME; we are no worse off for those clients than we used to be. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-9-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Implement NBD_OPT_GO on clientEric Blake2017-07-143-2/+130
| | | | | | | | | | | | | | | | | | | | | | | | | | NBD_OPT_EXPORT_NAME is lousy: per the NBD protocol, any failure requires the server to close the connection rather than report an error to us. Therefore, upstream NBD recently added NBD_OPT_GO as the improved version of the option that does what we want [1]: it reports sane errors on failures, and on success provides at least as much info as NBD_OPT_EXPORT_NAME. [1] https://github.com/NetworkBlockDevice/nbd/blob/extension-info/doc/proto.md This is a first cut at use of the information types. Note that we do not need to use NBD_OPT_INFO, and that use of NBD_OPT_GO means we no longer have to use NBD_OPT_LIST to learn whether a server requires TLS (this requires servers that gracefully handle unknown NBD_OPT, many servers prior to qemu 2.5 were buggy, but I have patched qemu, upstream nbd, and nbdkit in the meantime, in part because of interoperability testing with this patch). We still fall back to NBD_OPT_LIST when NBD_OPT_GO is not supported on the server, as it is still one last chance for a nicer error message. Later patches will use further info, like NBD_INFO_BLOCK_SIZE. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-8-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Implement NBD_OPT_GO on serverEric Blake2017-07-142-3/+179
| | | | | | | | | | | | | | | | | | | | | NBD_OPT_EXPORT_NAME is lousy: per the NBD protocol, any failure requires us to close the connection rather than report an error. Therefore, upstream NBD recently added NBD_OPT_GO as the improved version of the option that does what we want [1], along with NBD_OPT_INFO that returns the same information but does not transition to transmission phase. [1] https://github.com/NetworkBlockDevice/nbd/blob/extension-info/doc/proto.md This is a first cut at the information types, and only passes the same information already available through NBD_OPT_LIST and NBD_OPT_EXPORT_NAME; items like NBD_INFO_BLOCK_SIZE (and thus any use of NBD_REP_ERR_BLOCK_SIZE_REQD) are intentionally left for later patches. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-7-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Refactor reply to NBD_OPT_EXPORT_NAMEEric Blake2017-07-141-22/+28
| | | | | | | | | | | Reply directly in nbd_negotiate_handle_export_name(), rather than waiting until nbd_negotiate_options() completes. This will make it easier to implement NBD_OPT_GO. Pass additional parameters around, rather than stashing things inside NBDClient. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-6-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Simplify trace of client flags in negotiationEric Blake2017-07-142-7/+3Star
| | | | | | | | | Simplify the tracing of client flags in the server, and return -EINVAL instead of -EIO if we successfully read but don't like those flags. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-5-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Expose and debug more NBD constantsEric Blake2017-07-145-37/+147
| | | | | | | | | | | The NBD protocol has several constants defined in various extensions that we are about to implement. Expose them to the code, along with an easy way to map various constants to strings during diagnostic messages. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-4-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Don't bother tracing an NBD_OPT_ABORT response failureEric Blake2017-07-142-12/+4Star
| | | | | | | | | We really don't care if our spec-compliant reply to NBD_OPT_ABORT was received, so shave off some lines of code by not even tracing it. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-3-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: Create struct for tracking export infoEric Blake2017-07-141-22/+22
| | | | | | | | | | | | | The NBD Protocol is introducing some additional information about exports, such as minimum request size and alignment, as well as an advertised maximum request size. It will be easier to feed this information back to the block layer if we gather all the information into a struct, rather than adding yet more pointer parameters during negotiation. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707203049.534-2-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: use generic trace subsystem instead of TRACE macroVladimir Sementsov-Ogievskiy2017-07-104-95/+119
| | | | | | | | | | | | | | | | | | Let NBD use the trace mechanisms already present in qemu. Now you can use the -trace optino of qemu, or the -T/--trace option of qemu-img, qemu-io, and qemu-nbd, to select nbd traces. For qemu, the QMP commands trace-event-{get,set}-state can also toggle tracing on the fly. Example: qemu-nbd --trace 'nbd_*' <image file> # enables all nbd traces Recompilation with CFLAGS=-DDEBUG_NBD is no more needed, furthermore, DEBUG_NBD macro is removed from the code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-11-vsementsov@virtuozzo.com> [eblake: minor tweaks to a couple of traces] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: refactor tracingVladimir Sementsov-Ogievskiy2017-07-102-24/+9Star
| | | | | | | | Reorganize traces: move, reword, add information, drop extra ones. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-10-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: rename clientflags var in nbd_negotiate_optionsVladimir Sementsov-Ogievskiy2017-07-101-19/+19
| | | | | | | | | | Rename 'clientflags' to just 'option'. This variable has nothing to do with flags, but is a single integer representing the option requested by the client. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-9-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: fix TRACE in nbd_negotiate_send_rep_lenVladimir Sementsov-Ogievskiy2017-07-101-1/+1
| | | | | | | | Fix wrong order of TRACE arguments. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-8-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/client: refactor TRACE of NBD_MAGICVladimir Sementsov-Ogievskiy2017-07-101-9/+2Star
| | | | | | | | | We are going to switch from TRACE macro to trace points, this TRACE complicates things, this patch simplifies it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-7-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/common: nbd_tls_handshake: remove extra TRACEVladimir Sementsov-Ogievskiy2017-07-101-3/+1Star
| | | | | | | | | Error is propagated to the caller, TRACE is not needed. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707152918.23086-6-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: add errp to nbd_send_reply()Vladimir Sementsov-Ogievskiy2017-07-101-8/+9
| | | | | | | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707152918.23086-5-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: use errp instead of LOGVladimir Sementsov-Ogievskiy2017-07-101-106/+160
| | | | | | | | Move to modern errp scheme from just LOGging errors. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-4-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: refactor nbd_negotiateVladimir Sementsov-Ogievskiy2017-07-101-9/+3Star
| | | | | | | | | | | | Combine two successive "if (oldStyle) {...} else {...}" into one. Block "if (client->tlscreds)" under "if (oldStyle)" is unreachable, as we have "oldStyle = client->exp != NULL && !client->tlscreds;". So, delete this block. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-3-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORTVladimir Sementsov-Ogievskiy2017-07-101-5/+15
| | | | | | | | | | | | | Separate the case when a client sends NBD_OPT_ABORT from all other errors. It will be needed for the following patch, where errors will be reported. This particular case is not actually an error - it honestly follows the NBD protocol. Therefore it should not be reported like an error. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170707152918.23086-2-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: refactor nbd_tripVladimir Sementsov-Ogievskiy2017-06-151-33/+20Star
| | | | | | | | | | | - do not use 'goto error_reply' outside a switch to jump into the middle of the switch's default case label - reduce code duplication Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-13-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: rename rc to retVladimir Sementsov-Ogievskiy2017-06-151-19/+19
| | | | | | | | | | For consistency use 'ret' name for saving return code everywhere in the file. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-12-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: get rid of fail: return rcVladimir Sementsov-Ogievskiy2017-06-151-16/+12Star
| | | | | | | | | | "goto fail" error handling scheme is not needed for just returning error code. Better is return it immediately. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-11-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: nbd_negotiate: fix error pathVladimir Sementsov-Ogievskiy2017-06-151-1/+2
| | | | | | | | | | Current code will return 0 on this nbd_write fail, as rc is 0 after successful nbd_negotiate_options. Fix this. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-10-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: remove NBDClientNewDataVladimir Sementsov-Ogievskiy2017-06-151-18/+7Star
| | | | | | | | | | | "co" field of NBDClientNewData has never been used, all the way back to its declaration in commit 1a6245a5. So let's just use client pointer instead of extra structure. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-9-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: refactor nbd_co_receive_requestVladimir Sementsov-Ogievskiy2017-06-151-28/+13Star
| | | | | | | | | | | Move function tail, about receiving next request out of the function. Error path is simplified and nbd_co_receive_request becomes more corresponding to its name. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-8-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: get rid of EAGAIN dead codeVladimir Sementsov-Ogievskiy2017-06-151-11/+7Star
| | | | | | | | | For now nbd_read never returns EAGAIN. So, don't handle it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-7-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: refactor nbd_co_send_replyVladimir Sementsov-Ogievskiy2017-06-151-4/+4
| | | | | | | | | As nbd_write never returns value > 0, we can get rid of extra ret. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-6-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: get rid of ssize_tVladimir Sementsov-Ogievskiy2017-06-151-10/+8Star
| | | | | | | | | Now nbd_read and friends return int, so get rid of ssize_t. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-5-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd/server: get rid of nbd_negotiate_read and friendsVladimir Sementsov-Ogievskiy2017-06-151-85/+22Star
| | | | | | | | | | | | | | | | | | | Functions nbd_negotiate_{read,write,drop_sync} were introduced in 1a6245a5b, when nbd_rwv (was nbd_wr_sync) was working through qemu_co_sendv_recvv (the path is nbd_wr_sync -> qemu_co_{recv/send} -> qemu_co_send_recv -> qemu_co_sendv_recvv), which just yields, without setting any handlers. But starting from ff82911cd nbd_rwv (was nbd_wr_syncv) works through qio_channel_yield() which sets handlers, so watchers are redundant in nbd_negotiate_{read,write,drop_sync}, then, let's just use nbd_{read,write,drop} functions. Functions nbd_{read,write,drop} has errp parameter, which is unused in this patch. This will be fixed later. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-4-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: make nbd_drop publicVladimir Sementsov-Ogievskiy2017-06-153-26/+28
| | | | | | | | | Following commit will reuse it for nbd server too. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170602150150.258222-3-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* nbd: rename read_sync and friendsVladimir Sementsov-Ogievskiy2017-06-154-46/+42Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename nbd_wr_syncv -> nbd_rwv read_sync -> nbd_read read_sync_eof -> nbd_read_eof write_sync -> nbd_write drop_sync -> nbd_drop 1. nbd_ prefix read_sync and write_sync are already shared, so it is good to have a namespace prefix. drop_sync will be shared, and read_sync_eof is related to read_sync, so let's rename them all. 2. _sync suffix _sync is related to the fact that nbd_wr_syncv doesn't return if a write to socket returns EAGAIN. The first implementation of nbd_wr_syncv (was wr_sync in 7a5ca8648b) just loops while getting EAGAIN, the current implementation yields in this case. Why we want to get rid of it: - it is normal for r/w functions to be synchronous, so having an additional suffix for it looks redundant (contrariwise, we have _aio suffix for async functions) - _sync suffix in block layer is used when function does flush (so using it for other thing is confusing a bit) - keep function names short after adding nbd_ prefix 3. for nbd_wr_syncv let's use more common notation 'rw' Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170602150150.258222-2-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>