aboutsummaryrefslogtreecommitdiff
path: root/qemu-nbd.c
AgeCommit message (Collapse)Author
2017-06-15qemu-nbd: Ignore SIGPIPEMax Reitz
qemu proper has done so for 13 years (8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have done so for four years (526eda14a68d5b3596be715505289b541288ef2a). Ignoring this signal is especially important in qemu-nbd because otherwise a client can easily take down the qemu-nbd server by dropping the connection when the server wants to send something, for example: $ qemu-nbd -x foo -f raw -t null-co:// & [1] 12726 $ qemu-io -c quit nbd://localhost/bar can't open device nbd://localhost/bar: No export with name 'bar' available [1] + 12726 broken pipe qemu-nbd -x foo -f raw -t null-co:// In this case, the client sends an NBD_OPT_ABORT and closes the connection (because it is not required to wait for a reply), but the server replies with an NBD_REP_ACK (because it is required to reply). Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20170611123714.31292-1-mreitz@redhat.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-05-09sockets: Limit SocketAddressLegacy to external interfacesMarkus Armbruster
SocketAddressLegacy is a simple union, and simple unions are awkward: they have their variant members wrapped in a "data" object on the wire, and require additional indirections in C. SocketAddress is the equivalent flat union. Convert all users of SocketAddressLegacy to SocketAddress, except for existing external interfaces. See also commit fce5d53..9445673 and 85a82e8..c5f1ae3. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1493192202-3184-7-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Minor editing accident fixed, commit message and a comment tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-05-09sockets: Rename SocketAddress to SocketAddressLegacyMarkus Armbruster
The next commit will rename SocketAddressFlat to SocketAddress, and the commit after that will replace most uses of SocketAddressLegacy by SocketAddress, replacing most of this commit's renames right back. Note that checkpatch emits a few "line over 80 characters" warnings. The long lines are all temporary; the SocketAddressLegacy replacement will shorten them again. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1493192202-3184-5-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-05-09qobject: Use simpler QDict/QList scalar insertion macrosEric Blake
We now have macros in place to make it less verbose to add a scalar to QDict and QList, so use them. Patch created mechanically via: spatch --sp-file scripts/coccinelle/qobject.cocci \ --macro-file scripts/cocci-macro-file.h --dir . --in-place then touched up manually to fix a couple of '?:' back to original spacing, as well as avoiding a long line in monitor.c. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20170427215821.19397-7-eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-03-19qemu-ga: obey LISTEN_PID when using systemd socket activationPaolo Bonzini
qemu-ga's socket activation support was not obeying the LISTEN_PID environment variable, which avoids that a process uses a socket-activation file descriptor meant for its parent. Mess can for example ensue if a process forks a children before consuming the socket-activation file descriptor and therefore setting O_CLOEXEC on it. Luckily, qemu-nbd also got socket activation code, and its copy does support LISTEN_PID. Some extra fixups are needed to ensure that the code can be used for both, but that's what this patch does. The main change is to replace get_listen_fds's "consume" argument with the FIRST_SOCKET_ACTIVATION_FD macro from the qemu-nbd code. Cc: "Richard W.M. Jones" <rjones@redhat.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-02-16qemu-nbd: Implement socket activation.Richard W.M. Jones
Socket activation (sometimes known as systemd socket activation) allows an Internet superserver to pass a pre-opened listening socket to the process, instead of having qemu-nbd open a socket itself. This is done via the LISTEN_FDS and LISTEN_PID environment variables, and a standard file descriptor range. This change partially implements socket activation for qemu-nbd. If the environment variables are set correctly, then socket activation will happen automatically, otherwise everything works as before. The limitation is that LISTEN_FDS must be 1. Signed-off-by: Richard W.M. Jones <rjones@redhat.com> Message-Id: <20170204100317.32425-2-rjones@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-24qemu-nbd: Add --fork optionMax Reitz
Using the --fork option, one can make qemu-nbd fork the worker process. The original process will exit on error of the worker or once the worker enters the main loop. Suggested-by: Sascha Silbe <silbe@linux.vnet.ibm.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-12trace: provide mechanism for registering trace eventsDaniel P. Berrange
Remove the notion of there being a single global array of trace events, by introducing a method for registering groups of events. The module_call_init() needs to be invoked at the start of any program that wants to make use of the trace support. Currently this covers system emulators qemu-nbd, qemu-img and qemu-io. [Squashed the following fix from Daniel P. Berrange <berrange@redhat.com>: linux-user/bsd-user: initialize trace events subsystem The bsd-user/linux-user programs make use of the CPU emulation code and this now requires that the trace events subsystem is enabled, otherwise it'll crash trying to allocate an empty trace events bitmap for the CPU object. --Stefan] Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1475588159-30598-14-git-send-email-berrange@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-10-06qemu-nbd: Shrink image size by specified offsetTomáš Golembiovský
When --offset is set the apparent device size has to be adjusted accordingly. Otherwise client may request read/write beyond the file end which would fail. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> Message-Id: <8a31654cb182932db78b95aae1e904fc2bd1c465.1475698895.git.tgolembi@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@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-06-28trace: enable tracing in qemu-nbdDenis V. Lunev
Please note, trace_init_backends() must be called in the final process, i.e. after daemonization. This is necessary to keep tracing thread in the proper process. Signed-off-by: Denis V. Lunev <den@openvz.org> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1466174654-30130-6-git-send-email-den@openvz.org CC: Paolo Bonzini <pbonzini@redhat.com> CC: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@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-20Use &error_fatal when initializing crypto on qemu-{img,io,nbd}Eduardo Habkost
In addition to making the code simpler, this will replace the long error messages: cannot initialize crypto: Unable to initialize GNUTLS library: [...] cannot initialize crypto: Unable to initialize gcrypt with shorter messages: Unable to initialize GNUTLS library: [...] Unable to initialize gcrypt Signed-off-by: Eduardo Habkost <ehabkost@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-12nbd: Switch to byte-based block accessEric Blake
Sector-based blk_read() should die; switch to byte-based blk_pread() instead. Add a constant for our magic number 512, to make it obvious that this size will NOT change even if BDRV_SECTOR_SIZE does, even though the two happen to be the same for now. Split assignments from conditionals to keep checkpatch.pl happy. Signed-off-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-04-28qom: -object error messages lost location, restore itMarkus Armbruster
qemu_opts_foreach() runs its callback with the error location set to the option's location. Any errors the callback reports use the option's location automatically. Commit 90998d5 moved the actual error reporting from "inside" qemu_opts_foreach() to after it. Here's a typical hunk: if (qemu_opts_foreach(qemu_find_opts("object"), - object_create, - object_create_initial, NULL)) { + user_creatable_add_opts_foreach, + object_create_initial, &err)) { + error_report_err(err); exit(1); } Before, object_create() reports from within qemu_opts_foreach(), using the option's location. Afterwards, we do it after qemu_opts_foreach(), using whatever location happens to be current there. Commonly a "none" location. This is because Error objects don't have location information. Problematic. Reproducer: $ qemu-system-x86_64 -nodefaults -display none -object secret,id=foo,foo=bar qemu-system-x86_64: Property '.foo' not found Note no location. This commit restores it: qemu-system-x86_64: -object secret,id=foo,foo=bar: Property '.foo' not found Note that the qemu_opts_foreach() bug just fixed could mask the bug here: if the location it leaves dangling hasn't been clobbered, yet, it's the correct one. Reported-by: Eric Blake <eblake@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1461767349-15329-4-git-send-email-armbru@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Paragraph on Error added to commit message]
2016-04-15nbd: fix assert() on qemu-nbd stopPavel Butsykin
From time to time qemu-nbd is crashing on the following assert: assert(state == TERMINATING); nbd_export_closed nbd_export_put main and the state at the moment of the crash is evaluated to TERMINATE. During shutdown process of the client the nbd_client_thread thread sends SIGTERM signal and the main thread calls the nbd_client_closed callback. If the SIGTERM callback will be executed after change the state to TERMINATING, then the state will once again be TERMINATE. To solve the issue, we must change the state to TERMINATE only if the state is RUNNING. In the other case we are shutting down already. Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com> Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Paolo Bonzini <pbonzini@redhat.com> Message-id: 1460629215-11567-1-git-send-email-den@openvz.org Signed-off-by: Max Reitz <mreitz@redhat.com>
2016-04-12block: initialize qcrypto API at startupDaniel P. Berrange
Any programs which call the qcrypto APIs should ensure that qcrypto_init() has been called before anything else which can use crypto. Essentially this means right at the start of the main method before initializing anything else. This is important because some versions of gnutls/gcrypt require explicit initialization before use. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Alex Bligh <alex@alex.org.uk> Tested-by: Alex Bligh <alex@alex.org.uk> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-04-08qemu-nbd: Document -x optionEric Blake
Commit 3d4b2f9c added -x to force qemu-nbd to use new-style negotiation, but while it documented it in the man page, it omitted docs in the --help output. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1459908128-11925-1-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-03-30qemu-nbd: Call blk_set_enable_write_cache() explicitlyKevin Wolf
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2016-03-22util: move declarations out of qemu-common.hVeronia Bahaa
Move declarations out of qemu-common.h for functions declared in utils/ files: e.g. include/qemu/path.h for utils/path.c. Move inline functions out of qemu-common.h and into new files (e.g. include/qemu/bcd.h) Signed-off-by: Veronia Bahaa <veroniabahaa@gmail.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-03-18qapi: Don't special-case simple union wrappersEric Blake
Simple unions were carrying a special case that hid their 'data' QMP member from the resulting C struct, via the hack method QAPISchemaObjectTypeVariant.simple_union_type(). But by using the work we started by unboxing flat union and alternate branches, coupled with the ability to visit the members of an implicit type, we can now expose the simple union's implicit type in qapi-types.h: | struct q_obj_ImageInfoSpecificQCow2_wrapper { | ImageInfoSpecificQCow2 *data; | }; | | struct q_obj_ImageInfoSpecificVmdk_wrapper { | ImageInfoSpecificVmdk *data; | }; ... | struct ImageInfoSpecific { | ImageInfoSpecificKind type; | union { /* union tag is @type */ | void *data; |- ImageInfoSpecificQCow2 *qcow2; |- ImageInfoSpecificVmdk *vmdk; |+ q_obj_ImageInfoSpecificQCow2_wrapper qcow2; |+ q_obj_ImageInfoSpecificVmdk_wrapper vmdk; | } u; | }; Doing this removes asymmetry between QAPI's QMP side and its C side (both sides now expose 'data'), and means that the treatment of a simple union as sugar for a flat union is now equivalent in both languages (previously the two approaches used a different layer of dereferencing, where the simple union could be converted to a flat union with equivalent C layout but different {} on the wire, or to an equivalent QMP wire form but with different C representation). Using the implicit type also lets us get rid of the simple_union_type() hack. Of course, now all clients of simple unions have to adjust from using su->u.member to using su->u.member.data; while this touches a number of files in the tree, some earlier cleanup patches helped minimize the change to the initialization of a temporary variable rather than every single member access. The generated qapi-visit.c code is also affected by the layout change: |@@ -7393,10 +7393,10 @@ void visit_type_ImageInfoSpecific_member | } | switch (obj->type) { | case IMAGE_INFO_SPECIFIC_KIND_QCOW2: |- visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2, &err); |+ visit_type_q_obj_ImageInfoSpecificQCow2_wrapper_members(v, &obj->u.qcow2, &err); | break; | case IMAGE_INFO_SPECIFIC_KIND_VMDK: |- visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk, &err); |+ visit_type_q_obj_ImageInfoSpecificVmdk_wrapper_members(v, &obj->u.vmdk, &err); | break; | default: | abort(); Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1458254921-17042-13-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-03-17blockdev: Split monitor reference from BB creationMax Reitz
Before this patch, blk_new() automatically assigned a name to the new BlockBackend and considered it referenced by the monitor. This patch removes the implicit monitor_add_blk() call from blk_new() (and consequently the monitor_remove_blk() call from blk_delete(), too) and thus blk_new() (and related functions) no longer take a BB name argument. In fact, there is only a single point where blk_new()/blk_new_open() is called and the new BB is monitor-owned, and that is in blockdev_init(). Besides thus relieving us from having to invent names for all of the BBs we use in qemu-img, this fixes a bug where qemu cannot create a new image if there already is a monitor-owned BB named "image". If a BB and its BDS tree are created in a single operation, as of this patch the BDS tree will be created before the BB is given a name (whereas it was the other way around before). This results in minor change to the output of iotest 087, whose reference output is amended accordingly. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-03-05util: Shorten references into SocketAddressEric Blake
An upcoming patch will alter how simple unions, like SocketAddress, are laid out, which will impact all lines of the form 'addr->u.XXX' (expanding it to the longer 'addr->u.XXX.data'). For better legibility in that patch, and less need for line wrapping, it's better to use a temporary variable to reduce the effect of a layout change to just the variable initializations, rather than every reference within a SocketAddress. Also, take advantage of some C99 initialization where it makes sense (simplifying g_new0() to g_new()). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1457021813-10704-7-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@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-22qemu-nbd: use no_argument/required_argument constantsDaniel P. Berrange
When declaring the 'struct option' array, use the standard constants no_argument/required_argument, instead of magic values 0 and 1. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-02-22qemu-nbd: don't overlap long option values with short optionsDaniel P. Berrange
When defining values for long options, the normal practice is to start numbering from 256, to avoid overlap with the range of valid values for short options. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-02-22qemu-nbd: allow specifying image as a set of options argsDaniel P. Berrange
Currently qemu-nbd allows an image filename to be passed on the command line, but unless using the JSON format, it does not have a way to set any options except the format eg qemu-nbd https://127.0.0.1/images/centos7.iso qemu-nbd /home/berrange/demo.qcow2 This adds a --image-opts arg that indicates that the positional filename should be interpreted as a full option string, not just a filename. qemu-nbd --image-opts driver=https,url=https://127.0.0.1/images,sslverify=off qemu-nbd --image-opts driver=file,filename=/home/berrange/demo.qcow2 This flag is mutually exclusive with the '-f' flag. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-02-16nbd: enable use of TLS with qemu-nbd serverDaniel P. Berrange
This modifies the qemu-nbd program so that it is possible to request the use of TLS with the server. It simply adds a new command line option --tls-creds which is used to provide the ID of a QCryptoTLSCreds object previously created via the --object command line option. For example qemu-nbd --object tls-creds-x509,id=tls0,endpoint=server,\ dir=/home/berrange/security/qemutls \ --tls-creds tls0 \ --exportname default TLS requires the new style NBD protocol, so if no export name is set (via --export-name), then we use the default NBD protocol export name "" TLS is only supported when using an IPv4/IPv6 socket listener. It is not possible to use with UNIX sockets, which includes when connecting the NBD server to a host device. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-16-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@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: allow setting of an export name for qemu-nbd serverDaniel P. Berrange
The qemu-nbd server currently always uses the old style protocol since it never sets any export name. This is a problem because future TLS support will require use of the new style protocol negotiation. This adds "--exportname NAME" / "-x NAME" arguments to qemu-nbd which allow the user to set an explicit export name. When an export name is set the server will always use the new style NBD protocol. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-11-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-16nbd: convert qemu-nbd server to use I/O channels for connection setupDaniel P. Berrange
This converts the qemu-nbd server to use the QIOChannelSocket class for initial listener socket setup and accepting of client connections. Actual I/O is still being performed against the socket file descriptor using the POSIX socket APIs. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-5-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-02-16qemu-nbd: add support for --object command line argDaniel P. Berrange
Allow creation of user creatable object types with qemu-nbd via a new --object command line arg. This will be used to supply passwords and/or encryption keys to the various block driver backends via the recently added 'secret' object type. # printf letmein > mypasswd.txt # qemu-nbd --object secret,id=sec0,file=mypasswd.txt \ ...other nbd args... Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-Id: <1455129674-17255-3-git-send-email-berrange@redhat.com> 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-01-15nbd: Always call "close_fn" in nbd_client_newFam Zheng
Rename the parameter "close" to "close_fn" to disambiguous with close(2). This unifies error handling paths of NBDClient allocation: nbd_client_new will shutdown the socket and call the "close_fn" callback if negotiation failed, so the caller don't need a different path than the normal close. The returned pointer is never used, make it void in preparation for the next patch. Signed-off-by: Fam Zheng <famz@redhat.com> Message-Id: <1452760863-25350-2-git-send-email-famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-13error: Clean up errors with embedded newlines (again)Markus Armbruster
The arguments of error_report() should yield a short error string without newlines. A few places try to print additional help after the error message by embedding newlines in the error string. That's nice, but let's do it the right way. Commit 474c213 cleaned up some, but they keep coming back. Offenders tracked down with the Coccinelle semantic patch from commit 312fd5f. Cc: Laszlo Ersek <lersek@redhat.com> Cc: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Markus Armbruster <armbru@pond.sub.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-01-13error: Strip trailing '\n' from error string arguments (again)Markus Armbruster
Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they keep coming back. Tracked down with the Coccinelle semantic patch from commit 312fd5f. Cc: Fam Zheng <famz@redhat.com> Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com> Cc: Dominik Dingel <dingel@linux.vnet.ibm.com> Cc: David Hildenbrand <dahi@linux.vnet.ibm.com> Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> Cc: Changchun Ouyang <changchun.ouyang@intel.com> Cc: zhanghailiang <zhang.zhanghailiang@huawei.com> Cc: Pavel Fedin <p.fedin@samsung.com> Signed-off-by: Markus Armbruster <armbru@pond.sub.org> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com> Acked-by: Bharata B Rao <bharata@linux.vnet.ibm.com> Acked-by: Fam Zheng <famz@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <1450452927-8346-17-git-send-email-armbru@redhat.com>
2016-01-13qemu-io qemu-nbd: Use error_report() etc. instead of fprintf()Markus Armbruster
Just three instances left. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <1450452927-8346-16-git-send-email-armbru@redhat.com>
2016-01-13error: Use error_reportf_err() where it makes obvious senseMarkus Armbruster
Done with this Coccinelle semantic patch @@ expression FMT, E, S; expression list ARGS; @@ - error_report(FMT, ARGS, error_get_pretty(E)); + error_reportf_err(E, FMT/*@@@*/, ARGS); ( - error_free(E); | exit(S); | abort(); ) followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping, because I can't figure out how to make Coccinelle transform strings. We now use the error whole instead of just its message obtained with error_get_pretty(). This avoids suppressing its hint (see commit 50b7b00), but I can't see how the errors touched in this commit could come with hints. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1450452927-8346-12-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2016-01-13qemu-nbd: Clean up "Failed to load snapshot" error messageMarkus Armbruster
bdrv_snapshot_load_tmp() sets an error and returns -errno on failure. We report both even though the error message is self-contained. Drop the redundant strerror(). While there: setting errno right before exit() is pointless, so drop that, too. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <1450452927-8346-8-git-send-email-armbru@redhat.com>
2016-01-13error: Use error_report_err() instead of ad hoc printsMarkus Armbruster
Unlike ad hoc prints, error_report_err() uses the error whole instead of just its message obtained with error_get_pretty(). This avoids suppressing its hint (see commit 50b7b00). Example: $ bld/ivshmem-server -l 42@ Parameter 'shm_size' expects a size You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes. The last line is new with this patch. While there, drop a "cannot parse shm size: " message prefix; it's redundant, because the error message proper is always of the form "Parameter 'shm_size' expects ...". Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <1450452927-8346-5-git-send-email-armbru@redhat.com>
2016-01-13error: Use error_report_err() where appropriate (again)Markus Armbruster
Same Coccinelle semantic patch as in commit 565f65d. We now use the original error whole instead of just its message obtained with error_get_pretty(). This avoids suppressing its hint (see commit 50b7b00), but I don't think the errors touched in this commit can come with hints. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <1450452927-8346-3-git-send-email-armbru@redhat.com>
2016-01-13qemu-nbd: Replace BSDism <err.h> by error_report()Markus Armbruster
Coccinelle semantic patch @@ expression E; expression list ARGS; @@ - errx(E, ARGS); + error_report(ARGS); + exit(E); @@ expression E, FMT; expression list ARGS; @@ - err(E, FMT, ARGS); + error_report(FMT /*": %s"*/, ARGS, strerror(errno)); + exit(E); followed by a replace of '"/*": %s"*/' by ' : %s"', because I can't figure out how to make Coccinelle transform strings. A few of the error messages touched have trailing newlines. They'll be stripped later in this series. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <1450452927-8346-2-git-send-email-armbru@redhat.com>