aboutsummaryrefslogtreecommitdiff
path: root/nbd
AgeCommit message (Collapse)Author
2017-06-15nbd/server: rename rc to retVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd/server: get rid of fail: return rcVladimir Sementsov-Ogievskiy
"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>
2017-06-15nbd/server: nbd_negotiate: fix error pathVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd/server: remove NBDClientNewDataVladimir Sementsov-Ogievskiy
"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>
2017-06-15nbd/server: refactor nbd_co_receive_requestVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd/server: get rid of EAGAIN dead codeVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd/server: refactor nbd_co_send_replyVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd/server: get rid of ssize_tVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd/server: get rid of nbd_negotiate_read and friendsVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd: make nbd_drop publicVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd: rename read_sync and friendsVladimir Sementsov-Ogievskiy
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>
2017-06-15nbd: Fix regression on resiliency to port scanEric Blake
Back in qemu 2.5, qemu-nbd was immune to port probes (a transient server would not quit, regardless of how many probe connections came and went, until a connection actually negotiated). But we broke that in commit ee7d7aa when removing the return value to nbd_client_new(), although that patch also introduced a bug causing an assertion failure on a client that fails negotiation. We then made it worse during refactoring in commit 1a6245a (a segfault before we could even assert); the (masked) assertion was cleaned up in d3780c2 (still in 2.6), and just recently we finally fixed the segfault ("nbd: Fully intialize client in case of failed negotiation"). But that still means that ever since we added TLS support to qemu-nbd, we have been vulnerable to an ill-timed port-scan being able to cause a denial of service by taking down qemu-nbd before a real client has a chance to connect. Since negotiation is now handled asynchronously via coroutines, we no longer have a synchronous point of return by re-adding a return value to nbd_client_new(). So this patch instead wires things up to pass the negotiation status through the close_fn callback function. Simple test across two terminals: $ qemu-nbd -f raw -p 30001 file $ nmap 127.0.0.1 -p 30001 && \ qemu-io -c 'r 0 512' -f raw nbd://localhost:30001 Note that this patch does not change what constitutes successful negotiation (thus, a client must enter transmission phase before that client can be considered as a reason to terminate the server when the connection ends). Perhaps we may want to tweak things in a later patch to also treat a client that uses NBD_OPT_ABORT as being a 'successful' negotiation (the client correctly talked the NBD protocol, and informed us it was not going to use our export after all), but that's a discussion for another day. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451614 Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170608222617.20376-1-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-06-07nbd: Fully initialize client in case of failed negotiationEric Blake
If a non-NBD client connects to qemu-nbd, we would end up with a SIGSEGV in nbd_client_put() because we were trying to unregister the client's association to the export, even though we skipped inserting the client into that list. Easy trigger in two terminals: $ qemu-nbd -p 30001 --format=raw file $ nmap 127.0.0.1 -p 30001 nmap claims that it thinks it connected to a pago-services1 server (which probably means nmap could be updated to learn the NBD protocol and give a more accurate diagnosis of the open port - but that's not our problem), then terminates immediately, so our call to nbd_negotiate() fails. The fix is to reorder nbd_co_client_start() to ensure that all initialization occurs before we ever try talking to a client in nbd_negotiate(), so that the teardown sequence on negotiation failure doesn't fault while dereferencing a half-initialized object. While debugging this, I also noticed that nbd_update_server_watch() called by nbd_client_closed() was still adding a channel to accept the next client, even when the state was no longer RUNNING. That is fixed by making nbd_can_accept() pay attention to the current state. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451614 Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170527030421.28366-1-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-06-06nbd/client.c: use errp instead of LOGVladimir Sementsov-Ogievskiy
Move to modern errp scheme from just LOGging errors. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170526110913.89098-1-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-06-06nbd: add errp to read_sync, write_sync and drop_syncVladimir Sementsov-Ogievskiy
There a lot of calls of these functions, which already have errp, which they are filling themselves. On the other hand, nbd_wr_syncv has errp parameter too, so it would be great to connect them. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-5-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-06-06nbd: add errp parameter to nbd_wr_syncv()Vladimir Sementsov-Ogievskiy
Will be used in following patch to provide actual error message in some cases. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-4-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-06-06nbd: read_sync and friends: return 0 on successVladimir Sementsov-Ogievskiy
functions read_sync, drop_sync, write_sync, and also nbd_negotiate_write, nbd_negotiate_read, nbd_negotiate_drop_sync returns number of processed bytes. But what this number can be, except requested number of bytes? Actually, underlying nbd_wr_syncv function returns a value >= 0 and != requested_bytes only on eof on read operation. So, firstly, it is impossible on write (let's add an assert) and on read it actually means, that communication is broken (except nbd_receive_reply, see below). Most of callers operate like this: if (func(..., size) != size) { /* error path */ } , i.e.: 1. They are not interested in partial success 2. Extra duplications in code (especially bad are duplications of magic numbers) 3. User doesn't see actual error message, as return code is lost. (this patch doesn't fix this point, but it makes fixing easier) Several callers handles ret >= 0 and != requested-size separately, by just returning EINVAL in this case. This patch makes read_sync and friends return EINVAL in this case, so final behavior is the same. And only one caller - nbd_receive_reply() does something not so obvious. It returns EINVAL for ret > 0 and != requested-size, like previous group, but for ret == 0 it returns 0. The only caller of nbd_receive_reply() - nbd_read_reply_entry() handles ret == 0 in the same way as ret < 0, so for now it doesn't matter. However, in following commits error path handling will be improved and we'll need to distinguish success from fail in this case too. So, this patch adds separate helper for this case - read_sync_eof. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-3-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-06-06nbd: strict nbd_wr_syncvVladimir Sementsov-Ogievskiy
nbd_wr_syncv is called either from coroutine or from client negotiation code, when socket is in blocking mode. So, -EAGAIN is impossible. Furthermore, EAGAIN is confusing, as, what to read/write again? With EAGAIN as a return code we don't know how much data is already read or written by the function, so in case of EAGAIN the whole communication is broken. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-2-vsementsov@virtuozzo.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-03-27nbd-client: fix handling of hungup connectionsPaolo Bonzini
After the switch to reading replies in a coroutine, nothing is reentering pending receive coroutines if the connection hangs. Move nbd_recv_coroutines_enter_all to the reply read coroutine, which is the place where hangups are detected. nbd_teardown_connection can simply wait for the reply read coroutine to detect the hangup and clean up after itself. This wouldn't be enough though because nbd_receive_reply returns 0 (rather than -EPIPE or similar) when reading from a hung connection. Fix the return value check in nbd_read_reply_entry. This fixes qemu-iotests 083. Reported-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20170314111157.14464-1-pbonzini@redhat.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-03-14nbd/client: fix drop_sync [CVE-2017-2630]Vladimir Sementsov-Ogievskiy
Comparison symbol is misused. It may lead to memory corruption. Introduced in commit 7d3123e. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170203154757.36140-6-vsementsov@virtuozzo.com> [eblake: add CVE details, update conditional] Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20170307151627.27212-1-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-02-28nbd/server: Use real permissions for NBD exportsKevin Wolf
NBD can't cope with device size changes, so resize must be forbidden, but otherwise we can tolerate anything. Depending on whether the export is writable or not, we only require consistent reads and writes. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Acked-by: Fam Zheng <famz@redhat.com>
2017-02-28block: Add error parameter to blk_insert_bs()Kevin Wolf
Now that blk_insert_bs() requests the BlockBackend permissions for the node it attaches to, it can fail. Instead of aborting, pass the errors to the callers. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Acked-by: Fam Zheng <famz@redhat.com>
2017-02-28block: Add permissions to blk_new()Kevin Wolf
We want every user to be specific about the permissions it needs, so we'll pass the initial permissions as parameters to blk_new(). A user only needs to call blk_set_perm() if it wants to change the permissions after the fact. The permissions are stored in the BlockBackend and applied whenever a BlockDriverState should be attached in blk_insert_bs(). This does not include actually choosing the right set of permissions everywhere yet. Instead, the usual FIXME comment is added to each place and will be addressed in individual patches. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Acked-by: Fam Zheng <famz@redhat.com>
2017-02-21nbd: convert to use qio_channel_yieldPaolo Bonzini
In the client, read the reply headers from a coroutine, switching the read side between the "read header" coroutine and the I/O coroutine that reads the body of the reply. In the server, if the server can read more requests it will create a new "read request" coroutine as soon as a request has been read. Otherwise, the new coroutine is created in nbd_request_put. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 20170213135235.12274-8-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2017-01-23io: change the QIOTask callback signatureDaniel P. Berrange
Currently the QIOTaskFunc signature takes an Object * for the source, and an Error * for any error. We also need to be able to provide a result pointer. Rather than continue to add parameters to QIOTaskFunc, remove the existing ones and simply pass the QIOTask object instead. This has methods to access all the other data items required in the callback impl. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2017-01-03aio: add AioPollFn and io_poll() interfaceStefan Hajnoczi
The new AioPollFn io_poll() argument to aio_set_fd_handler() and aio_set_event_handler() is used in the next patch. Keep this code change separate due to the number of files it touches. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20161201192652.9509-3-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-11-10nbd: Don't inf-loop on early EOFEric Blake
Commit 7d3123e converted a single read_sync() into a while loop that assumed that read_sync() would either make progress or give an error. But when the server hangs up early, the client sees EOF (a read_sync() of 0) and never makes progress, which in turn caused qemu-iotest './check -nbd 83' to go into an infinite loop. Rework the loop to accomodate reads cut short by EOF. Reported-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1478551093-32757-1-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Implement NBD_CMD_WRITE_ZEROES on serverEric Blake
Upstream NBD protocol recently added the ability to efficiently write zeroes without having to send the zeroes over the wire, along with a flag to control whether the client wants to allow a hole. Note that when it comes to requiring full allocation, vs. permitting optimizations, the NBD spec intentionally picked a different sense for the flag; the rules in qemu are: MAY_UNMAP == 0: must write zeroes MAY_UNMAP == 1: may use holes if reads will see zeroes while in NBD, the rules are: FLAG_NO_HOLE == 1: must write zeroes FLAG_NO_HOLE == 0: may use holes if reads will see zeroes In all cases, the 'may use holes' scenario is optional (the server need not use a hole, and must not use a hole if subsequent reads would not see zeroes). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-16-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Improve server handling of shutdown requestsEric Blake
NBD commit 6d34500b clarified how clients and servers are supposed to behave before closing a connection. It added NBD_REP_ERR_SHUTDOWN (for the server to announce it is about to go away during option haggling, so the client should quit sending NBD_OPT_* other than NBD_OPT_ABORT) and ESHUTDOWN (for the server to announce it is about to go away during transmission, so the client should quit sending NBD_CMD_* other than NBD_CMD_DISC). It also clarified that NBD_OPT_ABORT gets a reply, while NBD_CMD_DISC does not. This patch merely adds the missing reply to NBD_OPT_ABORT and teaches the client to recognize server errors. Actually teaching the server to send NBD_REP_ERR_SHUTDOWN or ESHUTDOWN would require knowing that the server has been requested to shut down soon (maybe we could do that by installing a SIGINT handler in qemu-nbd, which transitions from RUNNING to a new state that waits for the client to react, rather than just out-right quitting - but that's a bigger task for another day). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-15-git-send-email-eblake@redhat.com> [Move dummy ESHUTDOWN to include/qemu/osdep.h. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Refactor conversion to errno to silence checkpatchEric Blake
Checkpatch complains that 'return EINVAL' is usually wrong (since we tend to favor 'return -EINVAL'). But it is a false positive for nbd_errno_to_system_errno(). Since NBD may add future defined wire values, refactor the code to keep checkpatch happy. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-14-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Support shorter handshakeEric Blake
The NBD Protocol allows the server and client to mutually agree on a shorter handshake (omit the 124 bytes of reserved 0), via the server advertising NBD_FLAG_NO_ZEROES and the client acknowledging with NBD_FLAG_C_NO_ZEROES (only possible in newstyle, whether or not it is fixed newstyle). It doesn't shave much off the wire, but we might as well implement it. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alex Bligh <alex@alex.org.uk> Message-Id: <1476469998-28592-13-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Less allocation during NBD_OPT_LISTEric Blake
Since we know that the maximum name we are willing to accept is small enough to stack-allocate, rework the iteration over NBD_OPT_LIST responses to reuse a stack buffer rather than allocating every time. Furthermore, we don't even have to allocate if we know the server's length doesn't match what we are searching for. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-12-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Let client skip portions of server replyEric Blake
The server has a nice helper function nbd_negotiate_drop_sync() which lets it easily ignore fluff from the client (such as the payload to an unknown option request). We can't quite make it common, since it depends on nbd_negotiate_read() which handles coroutine magic, but we can copy the idea into the client where we have places where we want to ignore data (such as the description tacked on the end of NBD_REP_SERVER). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-11-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Let server know when client gives up negotiationEric Blake
The NBD spec says that a client should send NBD_OPT_ABORT rather than just dropping the connection, if the client doesn't like something the server sent during option negotiation. This is a best-effort attempt only, and can only be done in places where we know the server is still in sync with what we've sent, whether or not we've read everything the server has sent. Technically, the server then has to reply with NBD_REP_ACK, but it's not worth complicating the client to wait around for that reply. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-10-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Share common option-sending code in clientEric Blake
Rather than open-coding each option request, it's easier to have common helper functions do the work. That in turn requires having convenient packed types for handling option requests and replies. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-9-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Send message along with server NBD_REP_ERR errorsEric Blake
The NBD Protocol allows us to send human-readable messages along with any NBD_REP_ERR error during option negotiation; make use of this fact for clients that know what to do with our message. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-8-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Share common reply-sending code in serverEric Blake
Rather than open-coding NBD_REP_SERVER, reuse the code we already have by adding a length parameter. Additionally, the refactoring will make adding NBD_OPT_GO in a later patch easier. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-7-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Rename struct nbd_request and nbd_replyEric Blake
Our coding convention prefers CamelCase names, and we already have other existing structs with NBDFoo naming. Let's be consistent, before later patches add even more structs. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-6-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Rename NBDRequest to NBDRequestDataEric Blake
We have both 'struct NBDRequest' and 'struct nbd_request'; making it confusing to see which does what. Furthermore, we want to rename nbd_request to align with our normal CamelCase naming conventions. So, rename the struct which is used to associate the data received during request callbacks, while leaving the shorter name for the description of the request sent over the wire in the NBD protocol. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-4-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Treat flags vs. command type as separate fieldsEric Blake
Current upstream NBD documents that requests have a 16-bit flags, followed by a 16-bit type integer; although older versions mentioned only a 32-bit field with masking to find flags. Since the protocol is in network order (big-endian over the wire), the ABI is unchanged; but dealing with the flags as a separate field rather than masking will make it easier to add support for upcoming NBD extensions that increase the number of both flags and commands. Improve some comments in nbd.h based on the current upstream NBD protocol (https://github.com/yoe/nbd/blob/master/doc/proto.md), and touch some nearby code to keep checkpatch.pl happy. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-3-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Add qemu-nbd -D for human-readable descriptionEric Blake
The NBD protocol allows servers to advertise a human-readable description alongside an export name during NBD_OPT_LIST. Add an option to pass through the user's string to the NBD client. Doing this also makes it easier to test commit 200650d4, which is the client counterpart of receiving the description. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-2-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-10-27nbd: set name for all I/O channels createdDaniel P. Berrange
Ensure that all I/O channels created for NBD are given names to distinguish their respective roles. Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2016-09-05nbd-server: Use a separate BlockBackendKevin Wolf
The builtin NBD server uses its own BlockBackend now instead of reusing the monitor/guest device one. This means that it has its own writethrough setting now. The builtin NBD server always uses writeback caching now regardless of whether the guest device has WCE enabled. qemu-nbd respects the cache mode given on the command line. We still need to keep a reference to the monitor BB because we put an eject notifier on it, but we don't use it for any I/O. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2016-08-03nbd: Limit nbdflags to 16 bitsEric Blake
Rather than asserting that nbdflags is within range, just give it the correct type to begin with :) nbdflags corresponds to the per-export portion of NBD Protocol "transmission flags", which is 16 bits in response to NBD_OPT_EXPORT_NAME and NBD_OPT_GO. Furthermore, upstream NBD has never passed the global flags to the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually tried to OR the global flags with the transmission flags, with the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9 caused all earlier NBD 3.x clients to treat every export as read-only; NBD 3.10 and later intentionally clip things to 16 bits to pass only transmission flags). Qemu should follow suit, since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior during transmission. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1469129688-22848-3-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-08-03nbd: Fix bad flag detection on serverEric Blake
Commit ab7c548e added a check for invalid flags, but used an early return on error instead of properly going through the cleanup label. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1469129688-22848-2-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-07-20block: Convert BB interface to byte-based discardsEric Blake
Change sector-based blk_discard(), blk_co_discard(), and blk_aio_discard() to instead be byte-based blk_pdiscard(), blk_co_pdiscard(), and blk_aio_pdiscard(). NBD gets a lot simpler now that ignoring the unaligned portion of a byte-based discard request is handled under the hood by the block layer. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1468624988-423-6-git-send-email-eblake@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-07-20nbd: Drop unused offset parameterEric Blake
Now that NBD relies on the block layer to fragment things, we no longer need to track an offset argument for which fragment of a request we are actually servicing. While at it, use true and false instead of 0 and 1 for a bool parameter. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 1468607524-19021-6-git-send-email-eblake@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-07-13coroutine: move entry argument to qemu_coroutine_createPaolo Bonzini
In practice the entry argument is always known at creation time, and it is confusing that sometimes qemu_coroutine_enter is used with a non-NULL argument to re-enter a coroutine (this happens in block/sheepdog.c and tests/test-coroutine.c). So pass the opaque value at creation time, for consistency with e.g. aio_bh_new. Mostly done with the following semantic patch: @ entry1 @ expression entry, arg, co; @@ - co = qemu_coroutine_create(entry); + co = qemu_coroutine_create(entry, arg); ... - qemu_coroutine_enter(co, arg); + qemu_coroutine_enter(co); @ entry2 @ expression entry, arg; identifier co; @@ - Coroutine *co = qemu_coroutine_create(entry); + Coroutine *co = qemu_coroutine_create(entry, arg); ... - qemu_coroutine_enter(co, arg); + qemu_coroutine_enter(co); @ entry3 @ expression entry, arg; @@ - qemu_coroutine_enter(qemu_coroutine_create(entry), arg); + qemu_coroutine_enter(qemu_coroutine_create(entry, arg)); @ reentry @ expression co; @@ - qemu_coroutine_enter(co, NULL); + qemu_coroutine_enter(co); except for the aforementioned few places where the semantic patch stumbled (as expected) and for test_co_queue, which would otherwise produce an uninitialized variable warning. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-06-17nbd/client.c: Correct trace format stringPeter Maydell
The trace format string in nbd_send_request uses PRIu16 for request->type, but request->type is a uint32_t. This provokes compiler warnings on the OSX clang. Use PRIu32 instead. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1466167331-17063-1-git-send-email-peter.maydell@linaro.org
2016-06-16nbd: Avoid magic number for NBD max name sizeEric Blake
Declare a constant and use that when determining if an export name fits within the constraints we are willing to support. Note that upstream NBD recently documented that clients MUST support export names of 256 bytes (not including trailing NUL), and SHOULD support names up to 4096 bytes. 4096 is a bit big (we would lose benefits of stack-allocation of a name array), and we already have other limits in place (for example, qcow2 snapshot names are clamped around 1024). So for now, just stick to the required minimum, as that's easier to audit than a full-scale support for larger names. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1463006384-7734-12-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-29 05:34:02 +0000