aboutsummaryrefslogtreecommitdiff
path: root/nbd
AgeCommit message (Collapse)Author
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>
2016-06-16nbd: Detect servers that send unexpected error valuesEric Blake
Add some debugging to flag servers that are not compliant to the NBD protocol. This would have flagged the server bug fixed in commit c0301fcc. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alex Bligh <alex@alex.org.uk> Message-Id: <1463006384-7734-11-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Clean up ioctl handling of qemu-nbd -cEric Blake
The kernel ioctl() interface into NBD is limited to 'unsigned long'; we MUST pass in input with that type (and not int or size_t, as there may be platform ABIs where the wrong types promote incorrectly through var-args). Furthermore, on 32-bit platforms, the kernel is limited to a maximum export size of 2T (our BLKSIZE of 512 times a SIZE_BLOCKS constrained by 32 bit unsigned long). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1463006384-7734-8-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Group all Linux-specific ioctl code in one placeEric Blake
NBD ioctl()s are used to manage an NBD client session where initial handshake is done in userspace, but then the transmission phase is handed off to the kernel through a /dev/nbdX device. As such, all ioctls sent to the kernel on the /dev/nbdX fd belong in client.c; nbd_disconnect() was out-of-place in server.c. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1463006384-7734-7-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Reject unknown request flagsEric Blake
The NBD protocol says that clients should not send a command flag that has not been negotiated (whether by the client requesting an option during a handshake, or because we advertise support for the flag in response to NBD_OPT_EXPORT_NAME), and that servers should reject invalid flags with EINVAL. We were silently ignoring the flags instead. The client can't rely on our behavior, since it is their fault for passing the bad flag in the first place, but it's better to be robust up front than to possibly behave differently than the client was expecting with the attempted flag. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alex Bligh <alex@alex.org.uk> Message-Id: <1463006384-7734-6-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Improve server handling of bogus commandsEric Blake
We have a few bugs in how we handle invalid client commands: - A client can send an NBD_CMD_DISC where from + len overflows, convincing us to reply with an error and stay connected, even though the protocol requires us to silently disconnect. Fix by hoisting the special case sooner. - A client can send an NBD_CMD_WRITE where from + len overflows, where we reply to the client with EINVAL without consuming the payload; this will normally cause us to fail if the next thing read is not the right magic, but in rare cases, could cause us to interpret the data payload as valid commands and do things not requested by the client. Fix by adding a complete flag to track whether we are in sync or must disconnect. Furthermore, we have split the checks for bogus from/len across two functions, when it is easier to do it all at once. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1463006384-7734-5-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Quit server after any write errorEric Blake
We should never ignore failure from nbd_negotiate_send_rep(); if we are unable to write to the client, then it is not worth trying to continue the negotiation. Fortunately, the problem is not too severe - chances are that the errors being ignored here (mainly inability to write the reply to the client) are indications of a closed connection or something similar, which will also affect the next attempt to interact with the client and eventually reach a point where the errors are detected to end the loop. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1463006384-7734-4-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: More debug typo fixes, use correct formatsEric Blake
Clean up some debug message oddities missed earlier; this includes some typos, and recognizing that %d is not necessarily compatible with uint32_t. Also add a couple messages that I found useful while debugging things. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1463006384-7734-3-git-send-email-eblake@redhat.com> [Do not use PRIx16, clang complains. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Use BDRV_REQ_FUA for better FUA where supportedEric Blake
Rather than always flushing ourselves, let the block layer forward the FUA on to the underlying device - where all underlying layers also understand FUA, we are now more efficient; and where any underlying layer doesn't understand it, now the block layer takes care of the full flush fallback on our behalf. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1463006384-7734-2-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Don't use cpu_to_*w() functionsPeter Maydell
The cpu_to_*w() functions just compose a pointer dereference with a byteswap. Instead use st*_p(), which handles potential pointer misalignment and avoids the need to cast the pointer. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <1465575342-12146-1-git-send-email-peter.maydell@linaro.org> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-06-16nbd: Don't use *_to_cpup() functionsPeter Maydell
The *_to_cpup() functions are not very useful, as they simply do a pointer dereference and then a *_to_cpu(). Instead use either: * ld*_*_p(), if the data is at an address that might not be correctly aligned for the load * a local dereference and *_to_cpu(), if the pointer is the correct type and known to be correctly aligned Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <1465570836-22211-1-git-send-email-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-05-29nbd: Don't trim unrequested bytesEric Blake
Similar to commit df7b97ff, we are mishandling clients that give an unaligned NBD_CMD_TRIM request, and potentially trimming bytes that occur before their request; which in turn can cause potential unintended data loss (unlikely in practice, since most clients are sane and issue aligned trim requests). However, while we fixed read and write by switching to the byte interfaces of blk_, we don't yet have a byte interface for discard. On the other hand, trim is advisory, so rounding the user's request to simply ignore the first and last unaligned sectors (or the entire request, if it is sub-sector in length) is just fine. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1464173965-9694-1-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-05-19qemu-common: stop including qemu/bswap.h from qemu-common.hPaolo Bonzini
Move it to the actual users. There are still a few includes of qemu/bswap.h in headers; removing them is left for future work. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-05-18Fix some typos found by codespellStefan Weil
Signed-off-by: Stefan Weil <sw@weilnetz.de> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2016-05-12block: Allow BDRV_REQ_FUA through blk_pwrite()Eric Blake
We have several block drivers that understand BDRV_REQ_FUA, and emulate it in the block layer for the rest by a full flush. But without a way to actually request BDRV_REQ_FUA during a pass-through blk_pwrite(), FUA-aware block drivers like NBD are forced to repeat the emulation logic of a full flush regardless of whether the backend they are writing to could do it more efficiently. This patch just wires up a flags argument; followup patches will actually make use of it in the NBD driver and in qemu-io. Signed-off-by: Eric Blake <eblake@redhat.com> Acked-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-04-22nbd: Don't mishandle unaligned client requestsEric Blake
The NBD protocol does not (yet) force any alignment constraints on clients. Even though qemu NBD clients always send requests that are aligned to 512 bytes, we must be prepared for non-qemu clients that don't care about alignment (even if it means they are less efficient). Our use of blk_read() and blk_write() was silently operating on the wrong file offsets when the client made an unaligned request, corrupting the client's data (but as the client already has control over the file we are serving, I don't think it is a security hole, per se, just a data corruption bug). Note that in the case of NBD_CMD_READ, an unaligned length could cause us to return up to 511 bytes of uninitialized trailing garbage from blk_try_blockalign() - hopefully nothing sensitive from the heap's prior usage is ever leaked in that manner. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Tested-by: Kevin Wolf <kwolf@redhat.com> Message-id: 1461249750-31928-1-git-send-email-eblake@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2016-04-15nbd: Don't kill server on client that doesn't request TLSEric Blake
Upstream NBD documents (as of commit 4feebc95) that servers MAY choose to operate in a conditional mode, where it is up to the client whether to use TLS. For qemu's case, we want to always be in FORCEDTLS mode, because of the risk of man-in-the-middle attacks, and since we never export more than one device; likewise, the qemu client will ALWAYS send NBD_OPT_STARTTLS as its first option. But now that SELECTIVETLS servers exist, it is feasible to encounter a (non-qemu) client that is programmed to talk to such a server, and does not do NBD_OPT_STARTTLS first, but rather wants to probe if it can use a non-encrypted export. The NBD protocol documents that we should let such a client continue trying, on the grounds that maybe the client will get the hint to send NBD_OPT_STARTTLS, rather than immediately dropping the connection. Note that NBD_OPT_EXPORT_NAME is a special case: since it is the only option request that can't have an error return, we have to (continue to) drop the connection on that one; rather, what we are fixing here is that all other replies prior to TLS initiation tell the client NBD_REP_ERR_TLS_REQD, but keep the connection alive. Signed-off-by: Eric Blake <eblake@redhat.com> Message-id: 1460671343-18485-1-git-send-email-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2016-04-15nbd: Don't fail handshake on NBD_OPT_LIST descriptionsEric Blake
The NBD Protocol states that NBD_REP_SERVER may set 'length > sizeof(namelen) + namelen'; in which case the rest of the packet is a UTF-8 description of the export. While we don't know of any NBD servers that send this description yet, we had better consume the data so we don't choke when we start to talk to such a server. Also, a (buggy/malicious) server that replies with length < sizeof(namelen) would cause us to block waiting for bytes that the server is not sending, and one that replies with super-huge lengths could cause us to temporarily allocate up to 4G memory. Sanity check things before blindly reading incorrectly. Signed-off-by: Eric Blake <eblake@redhat.com> Message-id: 1460077777-31004-1-git-send-email-eblake@redhat.com Reviewed-by: Alex Bligh <alex@alex.org.uk> Signed-off-by: Max Reitz <mreitz@redhat.com>
2016-04-08nbd: do not hang nbd_wr_syncv if outside a coroutine and no available dataPaolo Bonzini
Until commit 1c778ef7 ("nbd: convert to using I/O channels for actual socket I/O", 2016-02-16), nbd_wr_sync returned -EAGAIN this scenario. nbd_reply_ready required these semantics because it has two conflicting requirements: 1) if a reply can be received on the socket, nbd_reply_ready needs to read the header outside coroutine context to identify _which_ coroutine to enter to process the rest of the reply 2) on the other hand, nbd_reply_ready can find a false positive if another thread (e.g. a VCPU thread running aio_poll) sneaks in and calls nbd_reply_ready too. In this case nbd_reply_ready does nothing and expects nbd_wr_syncv to return -EAGAIN. Currently, the solution to the first requirement is to wait in the very rare case of a read() that doesn't retrieve the reply header in its entirety; this is what nbd_wr_syncv does by calling qio_channel_wait(). However, the unconditional call to qio_channel_wait() breaks the second requirement. To fix this, the patch makes nbd_wr_syncv return -EAGAIN if done is zero, similar to the code before commit 1c778ef7. This is okay because NBD client-side negotiation is the only other case that calls nbd_wr_syncv outside a coroutine, and it places the socket in blocking mode. On the other hand, it is a bit unpleasant to put this in nbd_wr_syncv(), because the function is used by both client and server. The full fix would be to add a counter to NbdClientSession for how many bytes have been filled in s->reply. Then a reply can be filled by multiple separate invocations of nbd_reply_ready and the qio_channel_wait() call can be removed completely. Something to consider for 2.7... Reported-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-04-08nbd: Don't kill server when client requests unknown optionEric Blake
nbd-server.c currently fails to handle unsupported options properly. If during option haggling the client sends an unknown request, the server kills the connection instead of letting the client try to fall back to something older. This is precisely what advertising NBD_FLAG_FIXED_NEWSTYLE was supposed to fix. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1459982918-32229-1-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-04-08nbd: Fix NBD unsupported optionsAlex Bligh
nbd-client.c currently fails to handle unsupported options properly. If during option haggling the server finds an option that is unsupported, it returns an NBD_REP_ERR_UNSUP reply. According to nbd's proto.md, the format for such a reply should be: S: 64 bits, 0x3e889045565a9 (magic number for replies) S: 32 bits, the option as sent by the client to which this is a reply S: 32 bits, reply type (e.g., NBD_REP_ACK for successful completion, or NBD_REP_ERR_UNSUP to mark use of an option not known by this server S: 32 bits, length of the reply. This may be zero for some replies, in which case the next field is not sent S: any data as required by the reply (e.g., an export name in the case of NBD_REP_SERVER, or optional UTF-8 message for NBD_REP_ERR_*) However, in nbd-client.c, the reply type was being read, and if it contained an error, it was bailing out and issuing the next option request without first reading the length. This meant that the next option / handshake read had an extra 4 or more bytes of data in it. In practice, this makes Qemu incompatible with servers that do not support NBD_OPT_LIST. To verify this isn't an error in the specification or my reading of it, replies are sent by the reference implementation here: https://github.com/yoe/nbd/blob/66dfb35/nbd-server.c#L1232 and as is evident it always sends a 'datasize' (aka length) 32 bit word. Unsupported elements are replied to here: https://github.com/yoe/nbd/blob/66dfb35/nbd-server.c#L1371 Signed-off-by: Alex Bligh <alex@alex.org.uk> Message-Id: <1459882500-24316-1-git-send-email-alex@alex.org.uk> [rework to ALWAYS consume an optional UTF-8 message from the server] Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1459961962-18771-1-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-04-08nbd: Improve debug traces on little-endianEric Blake
Print debug tracing messages while data is still in native ordering, rather than after we've potentially swapped it into network order for transmission. Also, it's nice if the server mentions what it is replying, to correlate it to with what the client says it is receiving. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1459913704-19949-4-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-04-08nbd: Avoid bitrot in TRACE() usageEric Blake
The compiler is smart enough to optimize out 'if (0)', but won't type-check our printfs if they are hidden behind #if. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1459913704-19949-3-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-04-08nbd: Return correct error for write to read-only exportEric Blake
The NBD Protocol requires that servers should send EPERM for attempts to write (or trim) a read-only export. We were correct for TRIM (blk_co_discard() gave EPERM); but were manually setting EROFS which then got mapped to EINVAL over the wire on writes. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1459913704-19949-2-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-04-05nbd: Fix poor debug messageEric Blake
The client sends messages to the server, not itself. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1459459222-8637-3-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-03-22include/qemu/osdep.h: Don't include qapi/error.hMarkus Armbruster
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the Error typedef. Since then, we've moved to include qemu/osdep.h everywhere. Its file comment explains: "To avoid getting into possible circular include dependencies, this file should not include any other QEMU headers, with the exceptions of config-host.h, compiler.h, os-posix.h and os-win32.h, all of which are doing a similar job to this file and are under similar constraints." qapi/error.h doesn't do a similar job, and it doesn't adhere to similar constraints: it includes qapi-types.h. That's in excess of 100KiB of crap most .c files don't actually need. Add the typedef to qemu/typedefs.h, and include that instead of qapi/error.h. Include qapi/error.h in .c files that need it and don't get it now. Include qapi-types.h in qom/object.h for uint16List. Update scripts/clean-includes accordingly. Update it further to match reality: replace config.h by config-target.h, add sysemu/os-posix.h, sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h comment quoted above similarly. This reduces the number of objects depending on qapi/error.h from "all of them" to less than a third. Unfortunately, the number depending on qapi-types.h shrinks only a little. More work is needed for that one. Signed-off-by: Markus Armbruster <armbru@redhat.com> [Fix compilation without the spice devel packages. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-23all: Clean up includesPeter Maydell
Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Eric Blake <eblake@redhat.com>
2016-02-16nbd: implement TLS support in the protocol negotiationDaniel P. Berrange
This extends the NBD protocol handling code so that it is capable of negotiating TLS support during the connection setup. This involves requesting the STARTTLS protocol option before any other NBD options. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-14-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16nbd: use "" as a default export name if none providedDaniel P. Berrange
If the user does not provide an export name and the server is running the new style protocol, where export names are mandatory, use "" as the default export name if the user has not specified any. "" is defined in the NBD protocol as the default name to use in such scenarios. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-13-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16nbd: always query export list in fixed new style protocolDaniel P. Berrange
With the new style protocol, the NBD client will currenetly send NBD_OPT_EXPORT_NAME as the first (and indeed only) option it wants. The problem is that the NBD protocol spec does not allow for returning an error message with the NBD_OPT_EXPORT_NAME option. So if the server mandates use of TLS, the client will simply see an immediate connection close after issuing NBD_OPT_EXPORT_NAME which is not user friendly. To improve this situation, if we have the fixed new style protocol, we can sent NBD_OPT_LIST as the first option to query the list of server exports. We can check for our named export in this list and raise an error if it is not found, instead of going ahead and sending NBD_OPT_EXPORT_NAME with a name that we know will be rejected. This improves the error reporting both in the case that the server required TLS, and in the case that the client requested export name does not exist on the server. If the server does not support NBD_OPT_LIST, we just ignore that and carry on with NBD_OPT_EXPORT_NAME as before. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-12-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16nbd: make client request fixed new style if advertisedDaniel P. Berrange
If the server advertises support for the fixed new style negotiation, the client should in turn enable new style. This will allow the client to negotiate further NBD options besides the export name. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-10-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16nbd: make server compliant with fixed newstyle specDaniel P. Berrange
If the client does not request the fixed new style protocol, then we should only accept NBD_OPT_EXPORT_NAME. All other options are only valid when fixed new style has been activated. The qemu-nbd client doesn't currently request fixed new style protocol, but this change won't break qemu-nbd, because it fortunately only ever uses NBD_OPT_EXPORT_NAME, so was never triggering the non-compliant server behaviour. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-9-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16nbd: invert client logic for negotiating protocol versionDaniel P. Berrange
The nbd_receive_negotiate() method takes different code paths based on whether 'name == NULL', and then checks the expected protocol version in each branch. This patch inverts the logic, so that it takes different code paths based on what protocol version it receives and then checks if name is NULL or not as needed. This facilitates later code which allows the client to be capable of using the new style protocol regardless of whether an export name is listed or not. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-8-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16nbd: convert to using I/O channels for actual socket I/ODaniel P. Berrange
Now that all callers are converted to use I/O channels for initial connection setup, it is possible to switch the core NBD protocol handling core over to use QIOChannel APIs for actual sockets I/O. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-7-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-09nbd: avoid unaligned uint64_t storeJohn Snow
cpu_to_be64w can't be used to make unaligned stores, but stq_be_p can. Also, the st?_be_p takes a void* so it is more clearly suited to the case where you're writing into a byte buffer. Use the st?_be_p family of functions everywhere in nbd/server.c. Signed-off-by: John Snow <jsnow@redhat.com> [Changed to use st?_be_p everywhere. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-04all: Clean up includesPeter Maydell
Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1454089805-5470-16-git-send-email-peter.maydell@linaro.org
2016-02-02nbd: Switch from close to eject notifierMax Reitz
The NBD code uses the BDS close notifier to determine when a medium is ejected. However, now it should use the BB's BDS removal notifier for that instead of the BDS's close notifier. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-02-02nbd: client_close on error in nbd_co_client_startMax Reitz
Use client_close() if an error in nbd_co_client_start() occurs instead of manually inlining parts of it. This fixes an assertion error on the server side if nbd_negotiate() fails. Signed-off-by: Max Reitz <mreitz@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-01-26Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell
* chardev support for TLS and leak fix * NBD fix from Denis * condvar fix from Dave * kvm_stat and dump-guest-memory almost rewrite * mem-prealloc fix from Luiz * manpage style improvement # gpg: Signature made Tue 26 Jan 2016 14:58:18 GMT using RSA key ID 78C7AE83 # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" * remotes/bonzini/tags/for-upstream: (49 commits) scripts/dump-guest-memory.py: Fix module docstring scripts/dump-guest-memory.py: Introduce multi-arch support scripts/dump-guest-memory.py: Cleanup functions scripts/dump-guest-memory.py: Improve python 3 compatibility scripts/dump-guest-memory.py: Make methods functions scripts/dump-guest-memory.py: Move constants to the top nbd: add missed aio_context_acquire in nbd_export_new memory: exit when hugepage allocation fails if mem-prealloc cpus: use broadcast on qemu_pause_cond scripts/kvm/kvm_stat: Add optparse description scripts/kvm/kvm_stat: Add interactive filtering scripts/kvm/kvm_stat: Fixup filtering scripts/kvm/kvm_stat: Fix rlimit for unprivileged users scripts/kvm/kvm_stat: Read event values as u64 scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr scripts/kvm/kvm_stat: Fix output formatting scripts/kvm/kvm_stat: Make tui function a class scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS scripts/kvm/kvm_stat: Group arch specific data scripts/kvm/kvm_stat: Cleanup of Event class ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2016-01-26nbd: add missed aio_context_acquire in nbd_export_newDenis V. Lunev
blk_invalidate_cache() can call qcow2_invalidate_cache which performs IO inside. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Kevin Wolf <kwolf@redhat.com> CC: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1453273940-15382-3-git-send-email-den@openvz.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-20block: Rename BDRV_O_INCOMING to BDRV_O_INACTIVEKevin Wolf
Instead of covering only the state of images on the migration destination before the migration is completed, the flag will also cover the state of images on the migration source after completion. This common state implies that the image is technically still open, but no writes will happen and any cached contents will be reloaded from disk if and when the image leaves this state. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2016-01-15nbd-server: do not exit on failed memory allocationPaolo Bonzini
The amount of memory allocated in nbd_co_receive_request is driven by the NBD client (possibly a virtual machine). Parallel I/O can cause the server to allocate a large amount of memory; check for failures and return ENOMEM in that case. Cc: qemu-block@nongnu.org Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-15nbd-server: do not check request length except for reads and writesPaolo Bonzini
Only reads and writes need to allocate memory correspondent to the request length. Other requests can be sent to the storage without allocating any memory, and thus any request length is acceptable. Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com> Cc: qemu-block@nongnu.org Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>