summaryrefslogtreecommitdiffstats
path: root/nbd
Commit message (Collapse)AuthorAgeFilesLines
...
* nbd/client: Drop pointless buf variableEric Blake2019-01-052-16/+9Star
| | | | | | | | | | | | | | | | | | | | | There's no need to read into a temporary buffer (oversized since commit 7d3123e1) followed by a byteswap into a uint64_t to check for a magic number via memcmp(), when the code immediately below demonstrates reading into the uint64_t then byteswapping in place and checking for a magic number via integer math. What's more, having a different error message when the server's first reply byte is 0 is unusual - it's no different from any other wrong magic number, and we already detected short reads. That whole strlen() issue has been present and useless since commit 1d45f8b5 in 2010; perhaps it was leftover debugging (since the correct magic number happens to be ASCII)? Make the error messages more consistent and detailed while touching things. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20181215135324.152629-9-eblake@redhat.com>
* qemu-nbd: Fail earlier for -c/-d on non-linuxEric Blake2019-01-051-17/+1Star
| | | | | | | | | | | | | | | | | | | | | Connecting to a /dev/nbdN device is a Linux-specific action. We were already masking -c and -d from 'qemu-nbd --help' on non-linux. However, while -d fails with a sensible error message, it took hunting through a couple of files to prove that. What's more, the code for -c doesn't fail until after it has created a pthread and tried to open a device - possibly even printing an error message with %m on a non-Linux platform in spite of the comment that %m is glibc-specific. Make the failure happen sooner, then get rid of stubs that are no longer needed because of the early exits. While at it: tweak the blank newlines in --help output to be consistent, whether or not built on Linux. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20181215135324.152629-7-eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* nbd/client: More consistent error messagesEric Blake2019-01-051-9/+12
| | | | | | | | | | | | | Consolidate on using decimal (not hex), on outputting the option reply name (not just value), and a consistent comma between clauses, when the client reports protocol discrepancies from the server. While it won't affect normal operation, it makes debugging additions easier. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20181215135324.152629-6-eblake@redhat.com>
* nbd/client: Trace all server option error messagesEric Blake2019-01-052-0/+3
| | | | | | | | | | | | Not all servers send free-form text alongside option error replies, but for servers that do (such as qemu), we pass the server's message as a hint alongside our own error reporting. However, it would also be useful to trace such server messages, since we can't guarantee how the hint may be consumed. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20181218225714.284495-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* nbd: publish _lookup functionsVladimir Sementsov-Ogievskiy2019-01-051-5/+0Star
| | | | | | | | | | | | | These functions are used for formatting pretty trace points. We are going to add some in block/nbd-client, so, let's publish all these functions at once. Note, that nbd_reply_type_lookup is already published, and constants, "named" by these functions live in include/block/nbd.h too. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20181102151152.288399-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: Advertise all contexts in response to bare LISTEric Blake2018-11-301-0/+1
| | | | | | | | | | | | The NBD spec, and even our code comment, says that if the client asks for NBD_OPT_LIST_META_CONTEXT with 0 queries, then we should reply with (a possibly-compressed representation of) ALL contexts that we are willing to let them try. But commit 3d068aff forgot to advertise qemu:dirty-bitmap:FOO. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20181130023232.3079982-2-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* nbd/server: Ignore write errors when replying to NBD_OPT_ABORTEric Blake2018-11-191-4/+8
| | | | | | | | | | | | | Commit 37ec36f6 intentionally ignores errors when trying to reply to an NBD_OPT_ABORT request for plaintext clients, but did not make the same change for a TLS server. Since NBD_OPT_ABORT is documented as being a potential for an EPIPE when the client hangs up without waiting for our reply, we don't need to pollute the server's output with that failure. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20181117223221.2198751-1-eblake@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
* nbd: fix whitespace in server error messageDaniel P. Berrangé2018-11-191-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | A space was missing after the option number was printed: Option 0x8not permitted before TLS becomes Option 0x8 not permitted before TLS This fixes commit 3668328303429f3bc93ab3365c66331600b06a2d Author: Eric Blake <eblake@redhat.com> Date: Fri Oct 14 13:33:09 2016 -0500 nbd: Send message along with server NBD_REP_ERR errors Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20181116155325.22428-2-berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: move lone space to next line] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: forbid use of frozen bitmapsJohn Snow2018-10-291-2/+2
| | | | | | | | | | Whether it's "locked" or "frozen", it's in use and should not be allowed for the purposes of this operation. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20181002230218.13949-7-jsnow@redhat.com Signed-off-by: John Snow <jsnow@redhat.com>
* nbd/server: drop old-style negotiationVladimir Sementsov-Ogievskiy2018-10-031-38/+15Star
| | | | | | | | | | | | After the previous commit, nbd_client_new's first parameter is always NULL. Let's drop it with all corresponding old-style negotiation code path which is unreachable now. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20181003170228.95973-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: re-wrap short line] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: fix NBD_CMD_CACHEVladimir Sementsov-Ogievskiy2018-10-031-1/+2
| | | | | | | | | | | | | | We should not go to structured-read branch on CACHE command, fix that. Bug introduced in bc37b06a5cde24 "nbd/server: introduce NBD_CMD_CACHE" with the whole feature and affects 3.0.0 release. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> CC: qemu-stable@nongnu.org Message-Id: <20181003144738.70670-1-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: commit message typo fix] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: Don't take address of fields in packed structsPeter Maydell2018-10-032-34/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. This patch was produced with the following spatch script: @@ expression E; @@ -be16_to_cpus(&E); +E = be16_to_cpu(E); @@ expression E; @@ -be32_to_cpus(&E); +E = be32_to_cpu(E); @@ expression E; @@ -be64_to_cpus(&E); +E = be64_to_cpu(E); @@ expression E; @@ -cpu_to_be16s(&E); +E = cpu_to_be16(E); @@ expression E; @@ -cpu_to_be32s(&E); +E = cpu_to_be32(E); @@ expression E; @@ -cpu_to_be64s(&E); +E = cpu_to_be64(E); Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20180927164200.15097-1-peter.maydell@linaro.org> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: rebase, and squash in missed changes] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: send more than one extent of base:allocation contextVladimir Sementsov-Ogievskiy2018-09-271-19/+60
| | | | | | | | | | This is necessary for efficient block-status export, for clients which support it. (qemu is not yet such a client, but could become one.) Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180704112302.471456-3-vsementsov@virtuozzo.com> [eblake: grammar tweaks] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: fix bitmap exportVladimir Sementsov-Ogievskiy2018-09-261-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | bitmap_to_extents function is broken: it switches dirty variable after every iteration, however it can process only part of dirty (or zero) area during one iteration in case when this area is too large for one extent. Fortunately, the bug doesn't produce wrong extent flags: it just inserts a zero-length extent between sequential extents representing large dirty (or zero) area. However, zero-length extents are forbidden by the NBD protocol. So, a careful client should consider such a reply as a server fault, while a less-careful will likely ignore zero-length extents. The bug can only be triggered by a client that requests block status for nearly 4G at once (a request of 4G and larger is impossible per the protocol, and requests smaller than 4G less the bitmap granularity cause the loop to quit iterating rather than revisit the tail of the large area); it also cannot trigger if the client used the NBD_CMD_FLAG_REQ_ONE flag. Since qemu 3.0 as client (using the x-dirty-bitmap extension) always passes the flag, it is immune; and we are not aware of other open-source clients that know how to request qemu:dirty-bitmap:FOO contexts. Clients that want to avoid the bug could cap block status requests to a smaller length, such as 2G or 3G. Fix this by more careful handling of dirty variable. Bug was introduced in 3d068aff16 "nbd/server: implement dirty bitmap export", with the whole function. and is present in v3.0.0 release. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180914165116.23182-1-vsementsov@virtuozzo.com> CC: qemu-stable@nongnu.org Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: improved commit message] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: fix nbd_co_send_block_statusVladimir Sementsov-Ogievskiy2018-07-081-2/+3
| | | | | | | | | | | | Call nbd_co_send_extents() with correct length parameter (extent.length may be smaller than original length). Also, switch length parameter type to uint32_t, to correspond with request->len and similar nbd_co_send_bitmap(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180704112302.471456-2-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/client: Add x-dirty-bitmap to query bitmap from serverEric Blake2018-07-021-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | In order to test that the NBD server is properly advertising dirty bitmaps, we need a bare minimum client that can request and read the context. Since feature freeze for 3.0 is imminent, this is the smallest workable patch, which replaces the qemu block status report with the results of the NBD server's dirty bitmap (making it very easy to use 'qemu-img map --output=json' to learn where the dirty portions are). Note that the NBD protocol defines a dirty section with the same bit but opposite sense that normal "base:allocation" uses to report an allocated section; so in qemu-img map output, "data":true corresponds to clean, "data":false corresponds to dirty. A more complete solution that allows dirty bitmaps to be queried at the same time as normal block status will be required before this addition can lose the x- prefix. Until then, the fact that this replaces normal status with dirty status means actions like 'qemu-img convert' will likely misbehave due to treating dirty regions of the file as if they are unallocated. The next patch adds an iotest to exercise this new code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180702191458.28741-2-eblake@redhat.com>
* nbd/server: Fix dirty bitmap logic regressionEric Blake2018-07-021-1/+1
| | | | | | | | | | | | | | | | | | In my hurry to fix a build failure, I introduced a logic bug. The assertion conditional is backwards, meaning that qemu will now abort instead of reporting dirty bitmap status. The bug can only be tickled by an NBD client using an exported dirty bitmap (which is still an experimental QMP command), so it's not the end of the world for supported usage (and neither 'make check' nor qemu-iotests fails); but it also shows that we really want qemu-io support for reading dirty bitmaps if only so that I can add iotests coverage to prevent future brown-bag-of-shame events like this one. Fixes: 45eb6fb6 Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180622153509.375130-1-eblake@redhat.com>
* nbd/server: Silence gcc false positiveEric Blake2018-06-221-1/+2
| | | | | | | | | | | | The code has a while() loop that always initialized 'end', and the loop always executes at least once (as evidenced by the assert() just prior to the loop). But some versions of gcc still complain that 'end' is used uninitialized, so silence them. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20180622125814.345274-1-eblake@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
* nbd/server: introduce NBD_CMD_CACHEVladimir Sementsov-Ogievskiy2018-06-212-4/+9
| | | | | | | | | | | Handle nbd CACHE command. Just do read, without sending read data back. Cache mechanism should be done by exported node driver chain. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180413143156.11409-1-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: fix two missing case labels in switch statements] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: implement dirty bitmap exportVladimir Sementsov-Ogievskiy2018-06-212-23/+256
| | | | | | | | | | | | | | | | Handle a new NBD meta namespace: "qemu", and corresponding queries: "qemu:dirty-bitmap:<export bitmap name>". With the new metadata context negotiated, BLOCK_STATUS query will reply with dirty-bitmap data, converted to extents. The new public function nbd_export_bitmap selects which bitmap to export. For now, only one bitmap may be exported. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180609151758.17343-5-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: wording tweaks, minor cleanups, additional tracing] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: add nbd_meta_empty_or_pattern helperVladimir Sementsov-Ogievskiy2018-06-211-27/+62
| | | | | | | | | | | | Add nbd_meta_pattern() and nbd_meta_empty_or_pattern() helpers for metadata query parsing. nbd_meta_pattern() will be reused for the "qemu" namespace in following patches. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180609151758.17343-4-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: comment tweaks] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: refactor NBDExportMetaContextsVladimir Sementsov-Ogievskiy2018-06-211-12/+11Star
| | | | | | | | | | | | Use NBDExport pointer instead of just export name: there is no need to store a duplicated name in the struct; moreover, NBDExport will be used further. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180609151758.17343-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: commit message grammar tweak] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: fix traceVladimir Sementsov-Ogievskiy2018-06-211-4/+10
| | | | | | | | | | | Return code = 1 doesn't mean that we parsed base:allocation. Use correct traces in both -parsed and -skipped cases. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180609151758.17343-2-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: comment tweaks] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: Reject 0-length block status requestEric Blake2018-06-211-0/+4
| | | | | | | | | | | | | The NBD spec says that behavior is unspecified if the client requests 0 length for block status; but since the structured reply is documenting as returning a non-zero length, it's easier to just diagnose this with an EINVAL error than to figure out what to return. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180621124937.166549-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* nbd/client: Fix error messages during NBD_INFO_BLOCK_SIZEEric Blake2018-05-041-4/+10
| | | | | | | | | | | | A missing space makes for poor error messages, and sizes can't go negative. Also, we missed diagnosing a server that sends a maximum block size less than the minimum. Fixes: 081dd1fe CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180501154654.943782-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* nbd/client: fix nbd_negotiate_simple_meta_contextVladimir Sementsov-Ogievskiy2018-05-041-2/+2
| | | | | | | | | | | | | | | | | | | | Initialize received variable. Otherwise, is is possible for server to answer without any contexts, but we will set context_id to something random (received_id is not initialized too) and return 1, which is wrong. To solve it, just initialize received to false. Initialize received_id too, just to make all possible checkers happy. Bug was introduced in 78a33ab58782efdb206de14 "nbd: BLOCK_STATUS for standard get_block_status function: client part" with the whole function. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180427142002.21930-2-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: trace meta context negotiationEric Blake2018-04-023-0/+16
| | | | | | | | | | Having a more detailed log of the interaction between client and server is invaluable in debugging how meta context negotiation actually works. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180330130950.1931229-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* nbd/client: Correctly handle bad server REP_META_CONTEXTEric Blake2018-04-021-7/+21
| | | | | | | | | | | | | | | | | | | | | It's never a good idea to blindly read for size bytes as returned by the server without first validating that the size is within bounds; a malicious or buggy server could cause us to hang or get out of sync from reading further messages. It may be smarter to try and teach the client to cope with unexpected context ids by silently ignoring them instead of hanging up on the server, but for now, if the server doesn't reply with exactly the one context we expect, it's easier to just give up - however, if we give up for any reason other than an I/O failure, we might as well try to politely tell the server we are quitting rather than continuing. Fix some typos in the process. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180329231837.1914680-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
* nbd: BLOCK_STATUS for standard get_block_status function: client partVladimir Sementsov-Ogievskiy2018-03-131-0/+117
| | | | | | | | | | | | | Minimal realization: only one extent in server answer is supported. Flag NBD_CMD_FLAG_REQ_ONE is used to force this behavior. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180312152126.286890-6-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: grammar tweaks, fix min_block check and 32-bit cap, use -1 instead of errno on failure in nbd_negotiate_simple_meta_context, ensure that block status makes progress on success] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: BLOCK_STATUS for standard get_block_status function: server partVladimir Sementsov-Ogievskiy2018-03-131-0/+311
| | | | | | | | | | | | Minimal realization: only one extent in server answer is supported. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180312152126.286890-4-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: tweak whitespace, move constant from .h to .c, improve logic of check_meta_export_name, simplify nbd_negotiate_options by doing more in nbd_negotiate_meta_queries] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: add nbd_read_opt_name helperVladimir Sementsov-Ogievskiy2018-03-131-10/+43
| | | | | | | | | | | | | | | Add helper to read name in format: uint32 len (<= NBD_MAX_NAME_SIZE) len bytes string (not 0-terminated) The helper will be reused in following patch. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180312152126.286890-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: grammar fixes, actually check error] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: add nbd_opt_invalid helperVladimir Sementsov-Ogievskiy2018-03-131-14/+36
| | | | | | | | | | NBD_REP_ERR_INVALID is often parameter to nbd_opt_drop and it would be used more in following patches. So, let's add a helper. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20180312152126.286890-2-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: Honor FUA request on NBD_CMD_TRIMEric Blake2018-03-131-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The NBD spec states that since trim requests can affect disk contents, then they should allow for FUA semantics just like writes for ensuring the disk has settled before returning. As bdrv_[co_]pdiscard() does not support a flags argument, we can't pass FUA down the block layer stack, and must therefore emulate it with a flush at the NBD layer. Note that in all reality, generic well-behaved clients will never send TRIM+FUA (in fact, qemu as a client never does, and we have no intention to plumb flags into bdrv_pdiscard). This is because the NBD protocol states that it is unspecified to READ a trimmed area (you might read stale data, all zeroes, or even random unrelated data) without first rewriting it, and even the experimental BLOCK_STATUS extension states that TRIM need not affect reported status. Thus, in the general case, a client cannot tell the difference between an arbitrary server that ignores TRIM, a server that had a power outage without flushing to disk, and a server that actually affected the disk before returning; so waiting for the trim actions to flush to disk makes little sense. However, for a specific client and server pair, where the client knows the server treats TRIM'd areas as guaranteed reads-zero, waiting for a flush makes sense, hence why the protocol documents that FUA is valid on trim. So, even though the NBD protocol doesn't have a way for the server to advertise what effects (if any) TRIM will actually have, and thus any client that relies on specific effects is probably in error, we can at least support a client that requests TRIM+FUA. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180307225732.155835-1-eblake@redhat.com>
* nbd/server: refactor nbd_trip: split out nbd_handle_requestVladimir Sementsov-Ogievskiy2018-03-131-62/+66
| | | | | | | | | | Split out request handling logic. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-6-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: touch up blank line placement] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: refactor nbd_trip: cmd_read and generic replyVladimir Sementsov-Ogievskiy2018-03-131-81/+95
| | | | | | | | | | | | | | | | | | | | | | | | | nbd_trip has difficult logic when sending replies: it tries to use one code path for all replies. It is ok for simple replies, but is not comfortable for structured replies. Also, two types of error (and corresponding messages in local_err) - fatal (leading to disconnect) and not-fatal (just to be sent to the client) are difficult to follow. To make things a bit clearer, the following is done: - split CMD_READ logic to separate function. It is the most difficult command for now, and it is definitely cramped inside nbd_trip. Also, it is difficult to follow CMD_READ logic, shared between "case NBD_CMD_READ" and "if"s under "reply:" label. - create separate helper function nbd_send_generic_reply() and use it both in new nbd_do_cmd_read and for other commands in nbd_trip instead of common code-path under "reply:" label in nbd_trip. The helper supports an error message, so logic with local_err in nbd_trip is simplified. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-5-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: grammar tweaks and blank line placement] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: fix: check client->closing before sending replyVladimir Sementsov-Ogievskiy2018-03-131-8/+9
| | | | | | | | | | | | | | | | | | | | | Since the unchanged code has just set client->recv_coroutine to NULL before calling nbd_client_receive_next_request(), we are spawning a new coroutine unconditionally, but the first thing that coroutine will do is check for client->closing, making it a no-op if we have already detected that the client is going away. Furthermore, for any error other than EIO (where we disconnect, which itself sets client->closing), if the client has already gone away, we'll probably encounter EIO later in the function and attempt disconnect at that point. Logically, as soon as we know the connection is closing, there is no need to try a likely-to-fail a response or spawn a no-op coroutine. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-4-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: squash in further reordering: hoist check before spawning next coroutine, and document rationale in commit message] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: fix sparse readVladimir Sementsov-Ogievskiy2018-03-131-3/+14
| | | | | | | | | | | | | | | | | In case of io error in nbd_co_send_sparse_read we should not "goto reply:", as it was a fatal error and the common behavior is to disconnect in this case. We should not try to send the client an additional error reply, since we already hit a channel-io error on our previous attempt to send one. Fix this by handling block-status error in nbd_co_send_sparse_read, so nbd_co_send_sparse_read fails only on io error. Then just skip common "reply:" code path in nbd_trip. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-3-vsementsov@virtuozzo.com> [eblake: grammar tweaks] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: move nbd_co_send_structured_error upVladimir Sementsov-Ogievskiy2018-03-131-24/+24
| | | | | | | | | To be reused in nbd_co_send_sparse_read() in the following patch. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-2-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* qio: non-default context for TLS handshakePeter Xu2018-03-062-0/+2
| | | | | | | | | A new parameter "context" is added to qio_channel_tls_handshake() is to allow the TLS to be run on a non-default context. Still, no functional change. Signed-off-by: Peter Xu <peterx@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
* nbd/client: fix error messages in nbd_handle_reply_errVladimir Sementsov-Ogievskiy2018-03-013-18/+18
| | | | | | | | | | | | | | 1. NBD_REP_ERR_INVALID is not only about length, so, make message more general 2. hex format is not very good: it's hard to read something like "option a (set meta context)", so switch to dec. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <1518702707-7077-6-git-send-email-vsementsov@virtuozzo.com> [eblake: expand scope of patch: ALL uses of nbd_opt_lookup and nbd_rep_lookup are now decimal] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd: BLOCK_STATUS constantsVladimir Sementsov-Ogievskiy2018-03-011-0/+10
| | | | | | | | | | | | | Expose the new constants and structs that will be used by both server and client implementations of NBD_CMD_BLOCK_STATUS (the command is currently experimental at https://github.com/NetworkBlockDevice/nbd/blob/extension-blockstatus/doc/proto.md but will hopefully be stabilized soon). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <1518702707-7077-4-git-send-email-vsementsov@virtuozzo.com> [eblake: split from larger patch on server implementation] Signed-off-by: Eric Blake <eblake@redhat.com>
* Include qapi/error.h exactly where neededMarkus Armbruster2018-02-091-1/+0Star
| | | | | | | | | | | | | | This cleanup makes the number of objects depending on qapi/error.h drop from 1910 (out of 4743) to 1612 in my "build everything" tree. While there, separate #include from file comment with a blank line, and drop a useless comment on why qemu/osdep.h is included first. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-5-armbru@redhat.com> [Semantic conflict with commit 34e304e975 resolved, OSX breakage fixed]
* qapi: add nbd-server-removeVladimir Sementsov-Ogievskiy2018-01-261-0/+13
| | | | | | | | | | | | | Add command for removing an export. It is needed for cases when we don't want to keep the export after the operation on it was completed. The other example is a temporary node, created with blockdev-add. If we want to delete it we should firstly remove any corresponding NBD export. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180119135719.24745-3-vsementsov@virtuozzo.com> [eblake: drop dead nb_clients code] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: structurize option reply sendingVladimir Sementsov-Ogievskiy2018-01-181-27/+13Star
| | | | | | | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171122101958.17065-6-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: Add helper functions for parsing option payloadEric Blake2018-01-181-60/+62
| | | | | | | | | | | | | | | | | | | | | Rather than making every callsite perform length sanity checks and error reporting, add the helper functions nbd_opt_read() and nbd_opt_drop() that use the length stored in the client struct; also add an assertion that optlen is 0 before any option (ie. any previous option was fully handled), complementing the assertion added in an earlier patch that optlen is 0 after all negotiation completes. Note that the call in nbd_negotiate_handle_export_name() does not use the new helper (in part because the server cannot reply to NBD_OPT_EXPORT_NAME - it either succeeds or the connection drops). Based on patches by Vladimir Sementsov-Ogievskiy. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180110230825.18321-6-eblake@redhat.com>
* nbd/server: Add va_list form of nbd_negotiate_send_rep_err()Eric Blake2018-01-181-6/+18
| | | | | | | | | | This will be useful for the next patch. Based on a patch by Vladimir Sementsov-Ogievskiy Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180110230825.18321-5-eblake@redhat.com>
* nbd/server: Better error for NBD_OPT_EXPORT_NAME failureEric Blake2018-01-181-1/+1
| | | | | | | | | | | When a client abruptly disconnects before we've finished reading the name sent with NBD_OPT_EXPORT_NAME, we are better off logging the failure as EIO (we can't communicate with the client), rather than EINVAL (the client sent bogus data). Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180110230825.18321-4-eblake@redhat.com>
* nbd/server: refactor negotiation functions parametersVladimir Sementsov-Ogievskiy2018-01-181-84/+84
| | | | | | | | | | | | | | | | | | | | | Instead of passing currently negotiating option and its length to many of negotiation functions let's just store them on NBDClient struct to be state-variables of negotiation phase. This unifies semantics of negotiation functions and allows tracking changes of remaining option length in future patches. Asssert that optlen is back to 0 after negotiation (including old-style connections which don't negotiate), although we need more patches before we can assert optlen is 0 between options during negotiation. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171122101958.17065-2-vsementsov@virtuozzo.com> [eblake: rebase, commit message tweak, assert !optlen after negotiation completes] Signed-off-by: Eric Blake <eblake@redhat.com>
* nbd/server: Hoist nbd_reject_length() earlierEric Blake2018-01-181-29/+29
| | | | | | | | | | No semantic change, but will make it easier for an upcoming patch to refactor code without having to add forward declarations. Fix a poor comment while at it. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180110230825.18321-2-eblake@redhat.com>
* nbd: rename nbd_option and nbd_opt_replyVladimir Sementsov-Ogievskiy2018-01-101-6/+6
| | | | | | | | | | Rename nbd_option and nbd_opt_reply to NBDOption and NBDOptionReply to correspond to Qemu coding style and other structures here. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171122101958.17065-5-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
generated by cgit v1.2.3-55-g7522 (git 2.39.0) at 2024-12-18 11:57:47 +0100