aboutsummaryrefslogtreecommitdiff
path: root/nbd
AgeCommit message (Collapse)AuthorFilesLines
2018-01-26qapi: add nbd-server-removeVladimir Sementsov-Ogievskiy1-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>
2018-01-17nbd/server: structurize option reply sendingVladimir Sementsov-Ogievskiy1-27/+13
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>
2018-01-17nbd/server: Add helper functions for parsing option payloadEric Blake1-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>
2018-01-17nbd/server: Add va_list form of nbd_negotiate_send_rep_err()Eric Blake1-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>
2018-01-17nbd/server: Better error for NBD_OPT_EXPORT_NAME failureEric Blake1-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>
2018-01-17nbd/server: refactor negotiation functions parametersVladimir Sementsov-Ogievskiy1-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>
2018-01-17nbd/server: Hoist nbd_reject_length() earlierEric Blake1-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>
2018-01-10nbd: rename nbd_option and nbd_opt_replyVladimir Sementsov-Ogievskiy1-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>
2018-01-09nbd/server: add additional assert to nbd_export_putVladimir Sementsov-Ogievskiy1-0/+6
This place is not obvious, nbd_export_close may theoretically reduce refcount to 0. It may happen if someone calls nbd_export_put on named export not through nbd_export_set_name when refcount is 1. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-Id: <20171207155102.66622-2-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2018-01-08nbd/server: Optimize final chunk of sparse readEric Blake1-6/+7
If we are careful to handle 0-length read requests correctly, we can optimize our sparse read to send the NBD_REPLY_FLAG_DONE bit on our last OFFSET_DATA or OFFSET_HOLE chunk rather than needing a separate chunk. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171107030912.23930-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2018-01-08nbd/server: Implement sparse reads atop structured replyEric Blake2-3/+76
The reason that NBD added structured reply in the first place was to allow for efficient reads of sparse files, by allowing the reply to include chunks to quickly communicate holes to the client without sending lots of zeroes over the wire. Time to implement this in the server; our client can already read such data. We can only skip holes insofar as the block layer can query them; and only if the client is okay with a fragmented request (if a client requests NBD_CMD_FLAG_DF and the entire read is a hole, we could technically return a single NBD_REPLY_TYPE_OFFSET_HOLE, but that's a fringe case not worth catering to here). Sadly, the control flow is a bit wonkier than I would have preferred, but it was minimally invasive to have a split in the action between a fragmented read (handled directly where we recognize NBD_CMD_READ with the right conditions, and sending multiple chunks) vs. a single read (handled at the end of nbd_trip, for both simple and structured replies, when we know there is only one thing being read). Likewise, I didn't make any effort to optimize the final chunk of a fragmented read to set the NBD_REPLY_FLAG_DONE, but unconditionally send that as a separate NBD_REPLY_TYPE_NONE. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171107030912.23930-2-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-11-28nbd/server: CVE-2017-15118 Stack smash on large export nameEric Blake1-0/+4
Introduced in commit f37708f6b8 (2.10). The NBD spec says a client can request export names up to 4096 bytes in length, even though they should not expect success on names longer than 256. However, qemu hard-codes the limit of 256, and fails to filter out a client that probes for a longer name; the result is a stack smash that can potentially give an attacker arbitrary control over the qemu process. The smash can be easily demonstrated with this client: $ qemu-io f raw nbd://localhost:10809/$(printf %3000d 1 | tr ' ' a) If the qemu NBD server binary (whether the standalone qemu-nbd, or the builtin server of QMP nbd-server-start) was compiled with -fstack-protector-strong, the ability to exploit the stack smash into arbitrary execution is a lot more difficult (but still theoretically possible to a determined attacker, perhaps in combination with other CVEs). Still, crashing a running qemu (and losing the VM) is bad enough, even if the attacker did not obtain full execution control. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com>
2017-11-28nbd/server: CVE-2017-15119 Reject options larger than 32MEric Blake1-0/+6
The NBD spec gives us permission to abruptly disconnect on clients that send outrageously large option requests, rather than having to spend the time reading to the end of the option. No real option request requires that much data anyways; and meanwhile, we already have the practice of abruptly dropping the connection on any client that sends NBD_CMD_WRITE with a payload larger than 32M. For comparison, nbdkit drops the connection on any request with more than 4096 bytes; however, that limit is probably too low (as the NBD spec states an export name can theoretically be up to 4096 bytes, which means a valid NBD_OPT_INFO could be even longer) - even if qemu doesn't permit exports longer than 256 bytes. It could be argued that a malicious client trying to get us to read nearly 4G of data on a bad request is a form of denial of service. In particular, if the server requires TLS, but a client that does not know the TLS credentials sends any option (other than NBD_OPT_STARTTLS or NBD_OPT_EXPORT_NAME) with a stated payload of nearly 4G, then the server was keeping the connection alive trying to read all the payload, tying up resources that it would rather be spending on a client that can get past the TLS handshake. Hence, this warranted a CVE. Present since at least 2.5 when handling known options, and made worse in 2.6 when fixing support for NBD_FLAG_C_FIXED_NEWSTYLE to handle unknown options. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com>
2017-11-17nbd/server: Fix error reporting for bad requestsEric Blake1-24/+12
The NBD spec says an attempt to NBD_CMD_TRIM on a read-only export should fail with EPERM, as a trim has the potential to change disk contents, but we were relying on the block layer to catch that for us, which might not always give the right error (and even if it does, it does not let us pass back a sane message for structured replies). The NBD spec says an attempt to NBD_CMD_WRITE_ZEROES out of bounds should fail with ENOSPC, not EINVAL. Our check for u64 offset + u32 length wraparound up front is pointless; nothing uses offset until after the second round of sanity checks, and we can just as easily ensure there is no wraparound by checking whether offset is in bounds (since a disk size cannot exceed off_t which is 63 bits, adding a 32-bit number for a valid offset can't overflow). Bonus: dropping the up-front check lets us keep the connection alive after NBD_CMD_WRITE, whereas before we would drop the connection (of course, any client sending a packet that would trigger the failure is already buggy, so it's also okay to drop the connection, but better quality-of-implementation never hurts). Solve all of these issues by some code motion and improved request validation. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171115213557.3548-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-11-17nbd/client: Don't hard-disconnect on ESHUTDOWN from serverEric Blake1-6/+0
The NBD spec says that a server may fail any transmission request with ESHUTDOWN when it is apparent that no further request from the client can be successfully honored. The client is supposed to then initiate a soft shutdown (wait for all remaining in-flight requests to be answered, then send NBD_CMD_DISC). However, since qemu's server never uses ESHUTDOWN errors, this code was mostly untested since its introduction in commit b6f5d3b5. More recently, I learned that nbdkit as the NBD server is able to send ESHUTDOWN errors, so I finally tested this code, and noticed that our client was special-casing ESHUTDOWN to cause a hard shutdown (immediate disconnect, with no NBD_CMD_DISC), but only if the server sends this error as a simple reply. Further investigation found that commit d2febedb introduced a regression where structured replies behave differently than simple replies - but that the structured reply behavior is more in line with the spec (even if we still lack code in nbd-client.c to properly quit sending further requests). So this patch reverts the portion of b6f5d3b5 that introduced an improper hard-disconnect special-case at the lower level, and leaves the future enhancement of a nicer soft-disconnect at the higher level for another day. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171113194857.13933-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-11-17nbd/client: Use error_prepend() correctlyEric Blake1-24/+26
When using error prepend(), it is necessary to end with a space in the format string; otherwise, messages come out incorrectly, such as when connecting to a socket that hangs up immediately: can't open device nbd://localhost:10809/: Failed to read dataUnexpected end-of-file before all bytes were read Originally botched in commit e44ed99d, then several more instances added in the meantime. Pre-existing and not fixed here: we are inconsistent on capitalization; some of our messages start with lower case, and others start with upper, although the use of error_prepend() is much nicer to read when all fragments consistently start with lower. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171113152424.25381-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
2017-11-09nbd/server: Fix structured read of length 0Eric Blake2-1/+21
The NBD spec was recently clarified to state that a read of length 0 should not be attempted by a compliant client; but that a server must still handle it correctly in an unspecified manner (that is, either a successful no-op or an error reply, but not a crash) [1]. However, it also implies that NBD_REPLY_TYPE_OFFSET_DATA must have a non-zero payload length, but our existing code was replying with a chunk that a picky client could reject as invalid because it was missing a payload (our own client implementation was recently patched to be that picky, after first fixing it to not send 0-length requests). We are already doing successful no-ops for 0-length writes and for non-structured reads; so for consistency, we want structured reply reads to also be a no-op. The easiest way to do this is to return a NBD_REPLY_TYPE_NONE chunk; this is best done via a new helper function (especially since future patches for other structured replies may benefit from using the same helper). [1] https://github.com/NetworkBlockDevice/nbd/commit/ee926037 Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171108215703.9295-8-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-11-09nbd: Fix struct name for structured readsEric Blake1-1/+1
A closer read of the NBD spec shows that a structured reply chunk for a hole is not quite identical to the prefix of a data chunk, because the hole has to also send a 32-bit size field. Although we do not yet send holes, we should fix the misleading information in our header and make it easier for a future patch to support sparse reads. Messed up in commit bae245d1. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171108215703.9295-5-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-11-09nbd/client: Nicer trace of structured replyEric Blake2-2/+4
It's useful to know which structured reply chunk is being processed. Missed in commit d2febedb. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171108215703.9295-4-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-11-08nbd/server: fix nbd_negotiate_handle_infoVladimir Sementsov-Ogievskiy1-1/+2
namelen should be here, length is unrelated, and always 0 at this point. Broken in introduction in commit f37708f6, but mostly harmless (replying with '' as the name does not violate protocol, and does not confuse qemu as the nbd client since our implementation does not ask for the name; but might confuse some other client that does ask for the name especially if the default export is different than the export name being queried). Adding an assert makes it obvious that we are not skipping any bytes in the client's message, as well as making it obvious that we were using the wrong variable. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> CC: qemu-stable@nongnu.org Message-Id: <20171101154204.27146-1-vsementsov@virtuozzo.com> [eblake: improve commit message, squash in assert addition] Signed-off-by: Eric Blake <eblake@redhat.com>
2017-10-30nbd: Minimal structured read for clientVladimir Sementsov-Ogievskiy2-1/+12
Minimal implementation: for structured error only error_report error message. Note that test 83 is now more verbose, because the implementation prints more warnings about unexpected communication errors; perhaps future patches should tone things down by using trace messages instead of traces, but the common case of successful communication is no noisier than before. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171027104037.8319-13-eblake@redhat.com>
2017-10-30nbd: Move nbd_read() to common headerEric Blake1-9/+0
An upcoming change to block/nbd-client.c will want to read the tail of a structured reply chunk directly from the wire. Move this function to make it easier. Based on a patch from Vladimir Sementsov-Ogievskiy. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171027104037.8319-12-eblake@redhat.com>
2017-10-30nbd/client: prepare nbd_receive_reply for structured replyVladimir Sementsov-Ogievskiy2-23/+84
In following patch nbd_receive_reply will be used both for simple and structured reply header receiving. NBDReply is altered into union of simple reply header and structured reply chunk header, simple error translation moved to block/nbd-client to be consistent with further structured reply error translation. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171027104037.8319-11-eblake@redhat.com>
2017-10-30nbd/client: refactor nbd_receive_starttlsVladimir Sementsov-Ogievskiy2-18/+42
Split out nbd_request_simple_option to be reused for structured reply option. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171027104037.8319-10-eblake@redhat.com>
2017-10-30nbd/server: Include human-readable message in structured errorsEric Blake2-8/+14
The NBD spec permits including a human-readable error string if structured replies are in force, so we might as well send the client the message that we logged on any error. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171027104037.8319-9-eblake@redhat.com>
2017-10-30nbd: Minimal structured read for serverVladimir Sementsov-Ogievskiy2-10/+99
Minimal implementation of structured read: one structured reply chunk, no segmentation. Minimal structured error implementation: no text message. Support DF flag, but just ignore it, as there is no segmentation any way. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171027104037.8319-8-eblake@redhat.com>
2017-10-30nbd/server: Refactor zero-length option checkEric Blake1-28/+46
Consolidate the response for a non-zero-length option payload into a new function, nbd_reject_length(). This check will also be used when introducing support for structured replies. Note that STARTTLS response differs based on time: if the connection is still unencrypted, we set fatal to true (a client that can't request TLS correctly may still think that we are ready to start the TLS handshake, so we must disconnect); while if the connection is already encrypted, the client is sending a bogus request but is no longer at risk of being confused by continuing the connection. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171027104037.8319-7-eblake@redhat.com> [eblake: correct return value on STARTTLS] Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-10-30nbd/server: Simplify nbd_negotiate_options loopEric Blake1-15/+4
Instead of making each caller check whether a transmission error occurred, we can sink a common error check to the end of the loop. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171027104037.8319-6-eblake@redhat.com> [eblake: squash in compiler warning fix] Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2017-10-30nbd/server: Report error for write to read-only exportEric Blake1-1/+2
When the server is read-only, we were already reporting an error message for NBD_CMD_WRITE_ZEROES, but failed to set errp for a similar NBD_CMD_WRITE. This will matter more once structured replies allow the server to propagate the errp information back to the client. While at it, use an error message that makes a bit more sense if viewed on the client side. Note that when using qemu-io to test qemu-nbd behavior, it is rather difficult to convince qemu-io to send protocol violations (such as a read beyond bounds), because we have a lot of active checking on the client side that a qemu-io request makes sense before it ever goes over the wire to the server. The case of a client attempting a write when the server is started as 'qemu-nbd -r' is one of the few places where we can easily test error path handling, without having to resort to hacking in known temporary bugs to either the server or client. [Maybe we want a future patch to the client to do up-front checking on writes to a read-only export, the way it does up-front bounds checking; but I don't see anything in the NBD spec that points to a protocol violation in our current behavior.] Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171027104037.8319-5-eblake@redhat.com>
2017-10-30nbd: Expose constants and structs for structured readEric Blake3-1/+30
Upcoming patches will implement the NBD structured reply extension [1] for both client and server roles. Declare the constants, structs, and lookup routines that will be valuable whether the server or client code is backported in isolation. This includes moving one constant from an internal header to the public header, as part of the structured read processing will be done in block/nbd-client.c rather than nbd/client.c. [1]https://github.com/NetworkBlockDevice/nbd/blob/extension-structured-reply/doc/proto.md Based on patches from Vladimir Sementsov-Ogievskiy. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171027104037.8319-4-eblake@redhat.com>
2017-10-30nbd: Move nbd_errno_to_system_errno() to public headerEric Blake4-45/+37
This is needed in preparation for structured reply handling, as we will be performing the translation from NBD error to system errno value higher in the stack at block/nbd-client.c. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171027104037.8319-3-eblake@redhat.com>
2017-10-30nbd: Include error names in trace messagesEric Blake5-4/+30
NBD errors were originally sent over the wire based on Linux errno values; but not all the world is Linux, and not all platforms share the same values. Since a number isn't very easy to decipher on all platforms, update the trace messages to include the name of NBD errors being sent/received over the wire. Tweak the trace messages to be at the point where we are using the NBD error, not the translation to the host errno values. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20171027104037.8319-2-eblake@redhat.com>
2017-10-13nbd: header constants indentingVladimir Sementsov-Ogievskiy1-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>
2017-10-13nbd/server: simplify reply transmissionVladimir Sementsov-Ogievskiy1-25/+24
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>
2017-10-13nbd/server: refactor nbd_co_send_simple_reply parametersVladimir Sementsov-Ogievskiy1-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>
2017-10-13nbd/server: do not use NBDReply structureVladimir Sementsov-Ogievskiy1-21/+15
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>
2017-10-12nbd/server: structurize simple reply header sendingVladimir Sementsov-Ogievskiy2-24/+14
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>
2017-10-12nbd: rename some simple-request related objects to be _simple_Vladimir Sementsov-Ogievskiy4-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>
2017-10-12NBD: use g_new() family of functionsMarc-André Lureau1-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>
2017-09-06nbd: Use new qio_channel_*_all() functionsEric Blake2-79/+7
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>
2017-08-30nbd/client: fix nbd_send_request to return intVladimir Sementsov-Ogievskiy1-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>
2017-08-30nbd/client: refactor nbd_receive_replyVladimir Sementsov-Ogievskiy1-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>
2017-08-30nbd/client: refactor nbd_read_eofVladimir Sementsov-Ogievskiy2-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>
2017-08-30nbd/client: fix nbd_opt_goVladimir Sementsov-Ogievskiy1-2/+0
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>
2017-08-15nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cacheKevin Wolf1-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>
2017-08-15nbd: Fix trace message for disconnectEric Blake1-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>
2017-08-01trace-events: fix code style: print 0x before hex numbersVladimir Sementsov-Ogievskiy1-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>
2017-07-28nbd: fix memory leak in nbd_opt_go()Philippe Mathieu-Daudé1-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>
2017-07-17nbd: Fix server reply to NBD_OPT_EXPORT_NAME of older clientsEric Blake2-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>
2017-07-17nbd: Trace client command being sentEric Blake2-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>