- 10 7月, 2017 10 次提交
-
-
Let NBD use the trace mechanisms already present in qemu. Now you can use the -trace optino of qemu, or the -T/--trace option of qemu-img, qemu-io, and qemu-nbd, to select nbd traces. For qemu, the QMP commands trace-event-{get,set}-state can also toggle tracing on the fly. Example: qemu-nbd --trace 'nbd_*' <image file> # enables all nbd traces Recompilation with CFLAGS=-DDEBUG_NBD is no more needed, furthermore, DEBUG_NBD macro is removed from the code. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-11-vsementsov@virtuozzo.com> [eblake: minor tweaks to a couple of traces] Signed-off-by: NEric Blake <eblake@redhat.com>
-
Reorganize traces: move, reword, add information, drop extra ones. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-10-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
Rename 'clientflags' to just 'option'. This variable has nothing to do with flags, but is a single integer representing the option requested by the client. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-9-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
Fix wrong order of TRACE arguments. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-8-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
We are going to switch from TRACE macro to trace points, this TRACE complicates things, this patch simplifies it. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-7-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
Error is propagated to the caller, TRACE is not needed. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170707152918.23086-6-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170707152918.23086-5-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
Move to modern errp scheme from just LOGging errors. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-4-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
Combine two successive "if (oldStyle) {...} else {...}" into one. Block "if (client->tlscreds)" under "if (oldStyle)" is unreachable, as we have "oldStyle = client->exp != NULL && !client->tlscreds;". So, delete this block. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170707152918.23086-3-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
Separate the case when a client sends NBD_OPT_ABORT from all other errors. It will be needed for the following patch, where errors will be reported. This particular case is not actually an error - it honestly follows the NBD protocol. Therefore it should not be reported like an error. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170707152918.23086-2-vsementsov@virtuozzo.com> Signed-off-by: NEric Blake <eblake@redhat.com>
-
- 15 6月, 2017 13 次提交
-
-
- do not use 'goto error_reply' outside a switch to jump into the middle of the switch's default case label - reduce code duplication Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-13-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
For consistency use 'ret' name for saving return code everywhere in the file. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-12-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
"goto fail" error handling scheme is not needed for just returning error code. Better is return it immediately. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-11-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
Current code will return 0 on this nbd_write fail, as rc is 0 after successful nbd_negotiate_options. Fix this. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-10-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
"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: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-9-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
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: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-8-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
For now nbd_read never returns EAGAIN. So, don't handle it. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-7-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
As nbd_write never returns value > 0, we can get rid of extra ret. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-6-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
Now nbd_read and friends return int, so get rid of ssize_t. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-5-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
Functions nbd_negotiate_{read,write,drop_sync} were introduced in 1a6245a5, 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 ff82911c 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: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: NEric Blake <eblake@redhat.com> Message-Id: <20170602150150.258222-4-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
Following commit will reuse it for nbd server too. Reviewed-by: NEric Blake <eblake@redhat.com> Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170602150150.258222-3-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
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 7a5ca864) 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: NEric Blake <eblake@redhat.com> Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170602150150.258222-2-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
由 Eric 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 ee7d7aab 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 1a6245a5 (a segfault before we could even assert); the (masked) assertion was cleaned up in d3780c2d (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=1451614Signed-off-by: NEric Blake <eblake@redhat.com> Message-Id: <20170608222617.20376-1-eblake@redhat.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
- 08 6月, 2017 1 次提交
-
-
由 Eric 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=1451614Signed-off-by: NEric Blake <eblake@redhat.com> Message-Id: <20170527030421.28366-1-eblake@redhat.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
- 07 6月, 2017 5 次提交
-
-
Move to modern errp scheme from just LOGging errors. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170526110913.89098-1-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
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: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-5-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
Will be used in following patch to provide actual error message in some cases. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-4-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
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: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-3-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
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: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170516094533.6160-2-vsementsov@virtuozzo.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
- 27 3月, 2017 1 次提交
-
-
由 Paolo 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: NMax Reitz <mreitz@redhat.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com> Message-id: 20170314111157.14464-1-pbonzini@redhat.com Reviewed-by: NMax Reitz <mreitz@redhat.com> Signed-off-by: NMax Reitz <mreitz@redhat.com>
-
- 14 3月, 2017 1 次提交
-
-
Comparison symbol is misused. It may lead to memory corruption. Introduced in commit 7d3123e1. Signed-off-by: NVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20170203154757.36140-6-vsementsov@virtuozzo.com> [eblake: add CVE details, update conditional] Signed-off-by: NEric Blake <eblake@redhat.com> Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20170307151627.27212-1-eblake@redhat.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
- 01 3月, 2017 3 次提交
-
-
由 Kevin 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: NKevin Wolf <kwolf@redhat.com> Reviewed-by: NMax Reitz <mreitz@redhat.com> Acked-by: NFam Zheng <famz@redhat.com>
-
由 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: NKevin Wolf <kwolf@redhat.com> Reviewed-by: NMax Reitz <mreitz@redhat.com> Acked-by: NFam Zheng <famz@redhat.com>
-
由 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: NKevin Wolf <kwolf@redhat.com> Reviewed-by: NMax Reitz <mreitz@redhat.com> Acked-by: NFam Zheng <famz@redhat.com>
-
- 21 2月, 2017 1 次提交
-
-
由 Paolo 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: NStefan Hajnoczi <stefanha@redhat.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com> Reviewed-by: NFam Zheng <famz@redhat.com> Reviewed-by: NDaniel P. Berrange <berrange@redhat.com> Message-id: 20170213135235.12274-8-pbonzini@redhat.com Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
-
- 23 1月, 2017 1 次提交
-
-
由 Daniel 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: NEric Blake <eblake@redhat.com> Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
-
- 04 1月, 2017 1 次提交
-
-
由 Stefan 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: NStefan Hajnoczi <stefanha@redhat.com> Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com> Message-id: 20161201192652.9509-3-stefanha@redhat.com Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
-
- 10 11月, 2016 1 次提交
-
-
由 Eric Blake 提交于
Commit 7d3123e1 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: NMax Reitz <mreitz@redhat.com> Signed-off-by: NEric Blake <eblake@redhat.com> Message-Id: <1478551093-32757-1-git-send-email-eblake@redhat.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
- 02 11月, 2016 2 次提交
-
-
由 Eric 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: NEric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-16-git-send-email-eblake@redhat.com> Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
-
由 Eric 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: NEric 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: NPaolo Bonzini <pbonzini@redhat.com>
-