aboutsummaryrefslogtreecommitdiff
path: root/block/rbd.c
AgeCommit message (Collapse)AuthorFilesLines
2017-04-03block: Document -drive problematic code and bugsMarkus Armbruster1-0/+6
-blockdev and blockdev_add convert their arguments via QObject to BlockdevOptions for qmp_blockdev_add(), which converts them back to QObject, then to a flattened QDict. The QDict's members are typed according to the QAPI schema. -drive converts its argument via QemuOpts to a (flat) QDict. This QDict's members are all QString. Thus, the QType of a flat QDict member depends on whether it comes from -drive or -blockdev/blockdev_add, except when the QAPI type maps to QString, which is the case for 'str' and enumeration types. The block layer core extracts generic configuration from the flat QDict, and the block driver extracts driver-specific configuration. Both commonly do so by converting (parts of) the flat QDict to QemuOpts, which turns all values into strings. Not exactly elegant, but correct. However, A few places access the flat QDict directly: * Most of them access members that are always QString. Correct. * bdrv_open_inherit() accesses a boolean, carefully. Correct. * nfs_config() uses a QObject input visitor. Correct only because the visited type contains nothing but QStrings. * nbd_config() and ssh_config() use a QObject input visitor, and the visited types contain non-QStrings: InetSocketAddress members @numeric, @to, @ipv4, @ipv6. -drive works as long as you don't try to use them (they're all optional). @to is ignored anyway. Reproducer: -drive driver=ssh,server.host=h,server.port=22,server.ipv4,path=p -drive driver=nbd,server.type=inet,server.data.host=h,server.data.port=22,server.data.ipv4 both fail with "Invalid parameter type for 'data.ipv4', expected: boolean" Add suitable comments to all these places. Mark the buggy ones FIXME. "Fortunately", -drive's driver-specific options are entirely undocumented. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-id: 1490895797-29094-5-git-send-email-armbru@redhat.com [mreitz: Fixed two typos] Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-03-31rbd: Fix regression in legacy key/values containing escaped :Eric Blake1-43/+44
Commit c7cacb3 accidentally broke legacy key-value parsing through pseudo-filename parsing of -drive file=rbd://..., for any key that contains an escaped ':'. Such a key is surprisingly common, thanks to mon_host specifying a 'host:port' string. The break happens because passing things from QDict through QemuOpts back to another QDict requires that we pack our parsed key/value pairs into a string, and then reparse that string, but the intermediate string that we created ("key1=value1:key2=value2") lost the \: escaping that was present in the original, so that we could no longer see which : were used as separators vs. those used as part of the original input. Fix it by collecting the key/value pairs through a QList, and sending that list on a round trip through a JSON QString (as in '["key1","value1","key2","value2"]') on its way through QemuOpts, rather than hand-rolling our own string. Since the string is only handled internally, this was faster than creating a full-blown struct of '[{"key1":"value1"},{"key2":"value2"}]', and safer at guaranteeing order compared to '{"key1":"value1","key2":"value2"}'. It would be nicer if we didn't have to round-trip through QemuOpts in the first place, but that's a much bigger task for later. Reproducer: ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ -drive 'file=rbd:volumes/volume-ea141b5c-cdb3-4765-910d-e7008b209a70'\ ':id=compute:key=AQAVkvxXAAAAABAA9ZxWFYdRmV+DSwKr7BKKXg=='\ ':auth_supported=cephx\;none:mon_host=192.168.1.2\:6789'\ ',format=raw,if=none,id=drive-virtio-disk0,'\ 'serial=ea141b5c-cdb3-4765-910d-e7008b209a70,cache=writeback' Even without an RBD setup, this serves a test of whether we get the incorrect parser error of: qemu-system-x86_64: -drive file=rbd:...cache=writeback: conf option 6789 has no value or the correct behavior of hanging while trying to connect to the requested mon_host of 192.168.1.2:6789. Reported-by: Alexandru Avadanii <Alexandru.Avadanii@enea.com> Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20170331152730.12514-1-eblake@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Fix bugs around -drive parameter "server"Markus Armbruster1-95/+32
qemu_rbd_open() takes option parameters as a flattened QDict, with keys of the form server.%d.host, server.%d.port, where %d counts up from zero. qemu_rbd_array_opts() extracts these values as follows. First, it calls qdict_array_entries() to find the list's length. For each list element, it formats the list's key prefix (e.g. "server.0."), then creates a new QDict holding the options with that key prefix, then converts that to a QemuOpts, so it can finally get the member values from there. If there's one surefire way to make code using QDict more awkward, it's creating more of them and mixing in QemuOpts for good measure. The extraction of keys starting with server.%d into another QDict makes us ignore parameters like server.0.neither-host-nor-port silently. The conversion to QemuOpts abuses runtime_opts, as described a few commits ago. Rewrite to simply get the values straight from the options QDict. Fixes -drive not to crash when server.*.* are present, but server.*.host is absent. Fixes -drive to reject invalid server.*.*. Permits cleaning up runtime_opts. Do that, and fix -drive to reject bogus parameters host and port instead of silently ignoring them. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-11-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Revert -blockdev and -drive parameter auth-supportedMarkus Armbruster1-28/+3
This reverts half of commit 0a55679. We're having second thoughts on the QAPI schema (and thus the external interface), and haven't reached consensus, yet. Issues include: * The implementation uses deprecated rados_conf_set() key "auth_supported". No biggie. * The implementation makes -drive silently ignore invalid parameters "auth" and "auth-supported.*.X" where X isn't "auth". Fixable (in fact I'm going to fix similar bugs around parameter server), so again no biggie. * BlockdevOptionsRbd member @password-secret applies only to authentication method cephx. Should it be a variant member of RbdAuthMethod? * BlockdevOptionsRbd member @user could apply to both methods cephx and none, but I'm not sure it's actually used with none. If it isn't, should it be a variant member of RbdAuthMethod? * The client offers a *set* of authentication methods, not a list. Should the methods be optional members of BlockdevOptionsRbd instead of members of list @auth-supported? The latter begs the question what multiple entries for the same method mean. Trivial question now that RbdAuthMethod contains nothing but @type, but less so when RbdAuthMethod acquires other members, such the ones discussed above. * How BlockdevOptionsRbd member @auth-supported interacts with settings from a configuration file specified with @conf is undocumented. I suspect it's untested, too. Let's avoid painting ourselves into a corner now, and revert the feature for 2.9. Note that users can still configure authentication methods with a configuration file. They probably do that anyway if they use Ceph outside QEMU as well. Further note that this doesn't affect use of key "auth-supported" in -drive file=rbd:...:key=value. qemu_rbd_array_opts()'s parameter @type now must be RBD_MON_HOST, which is silly. This will be cleaned up shortly. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-9-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Clean up qemu_rbd_create()'s detour through QemuOptsMarkus Armbruster1-15/+5
The conversion from QDict to QemuOpts is pointless. Simply get the stuff straight from the QDict. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-8-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Clean up runtime_opts, fix -drive to reject filenameMarkus Armbruster1-14/+24
runtime_opts is used for three different purposes: * qemu_rbd_open() uses it to accept options it recognizes, such as "pool" and "image". Other .bdrv_open() methods do it similarly. * qemu_rbd_open() accepts additional list-valued options auth-supported and server, with the help of qemu_rbd_array_opts(). The list elements are again dictionaries. qemu_rbd_array_opts() uses runtime_opts to accept their members. Thus, runtime_opts contains recognized sub-sub-options "auth", "host", "port" in addition to recognized options. No other block driver does that. * qemu_rbd_create() uses it to convert the QDict produced by qemu_rbd_parse_filename() to QemuOpts. No other block driver does that. The keys produced by qemu_rbd_parse_filename() are "pool", "image", "snapshot", "conf", "user" and "keyvalue-pairs". qemu_rbd_open() accepts these, so no additional ones here. This is a confusing mess. Dates back to commit 0f9d252. First step to clean it up is documenting runtime_opts.desc[]: * Reorder entries to match the QAPI schema, like we do in other block drivers. * Document why the schema's "server" and "auth-supported" aren't in .desc[]. * Document why "keyvalue-pairs", "host", "port" and "auth" are in .desc[], but not the schema. * Delete "filename", because none of the three users actually uses it. This fixes -drive to reject parameter filename instead of silently ignoring it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-7-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Don't accept -drive driver=rbd, keyvalue-pairs=...Markus Armbruster1-4/+8
The way we communicate extra key-value pairs from qemu_rbd_parse_filename() to qemu_rbd_open() exposes option parameter "keyvalue-pairs" on the command line. It's not wanted there. Hack: rename the parameter to "=keyvalue-pairs" to make it inaccessible. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-6-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Clean up after the previous commitMarkus Armbruster1-15/+9
This code in qemu_rbd_parse_filename() found_str = qemu_rbd_next_tok(p, '\0', &p); p = found_str; has no effect. Drop it, and simplify qemu_rbd_next_tok(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-5-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Don't limit length of parameter valuesMarkus Armbruster1-77/+14
We laboriously enforce that parameter values are between one and some arbitrary limit in length. Only RBD_MAX_IMAGE_NAME_SIZE comes from librbd.h, and I'm not sure it applies. Where the other limits come from is unclear. Drop the length checking. The limits librbd actually imposes must be checked by librbd anyway. There's one minor complication: BDRVRBDState member name is a fixed-size array. Depends on the length limit. Make it a pointer to a dynamically allocated string. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-4-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-28rbd: Fix to cleanly reject -drive without pool or imageMarkus Armbruster1-3/+7
qemu_rbd_open() neglects to check pool and image are present. Missing image is caught by rbd_open(), but missing pool crashes. Reproducer: $ qemu-system-x86_64 -nodefaults -drive driver=rbd,id=rbd,image=i,... terminate called after throwing an instance of 'std::logic_error' what(): basic_string::_M_construct null not valid Aborted (core dumped) where ... is a working server.0.{host,port} configuration. Doesn't affect -drive with file=..., because qemu_rbd_parse_filename() always sets both pool and image. Doesn't affect -blockdev, because pool and image are mandatory in the QAPI schema. Fix by adding the missing checks. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1490691368-32099-3-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-03-01block/rbd: add support for 'mon_host', 'auth_supported' via QAPIJeff Cody1-0/+135
This adds support for three additional options that may be specified by QAPI in blockdev-add: server: host, port auth method: either 'cephx' or 'none' The "server" and "auth-supported" QAPI parameters are arrays. To conform with the rados API, the array items are join as a single string with a ';' character as a delimiter when setting the configuration values. Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-02-28block/rbd: parse all options via bdrv_parse_filenameJeff Cody1-148/+151
Get rid of qemu_rbd_parsename in favor of bdrv_parse_filename. This simplifies a lot of the parsing as well, as we can treat everything a bit simpler since nonexistent options are simply NULL pointers instead of empty strings. An important item to note: Ceph has many extra option values that can be specified as key/value pairs. This was handled previously in the driver by extracting the values that the QEMU driver cared about, and then blindly passing all extra options to rbd after splitting them into key/value pairs, and cleaning up any special character escaping. The practice is continued in this patch; there is an option "keyvalue-pairs" that is populated with all the key/value pairs that the QEMU driver does not care about. These key/value pairs will override any settings in the 'conf' configuration file, just as they did before. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-02-28block/rbd: add all the currently supported runtime_optsJeff Cody1-19/+49
This adds all the currently supported runtime opts, which are the options as parsed from the filename. All of these options are explicitly checked for during during runtime, with an exception to the "keyvalue-pairs" option. This option contains all the key/value pairs that the QEMU rbd driver merely unescapes, and passes along blindly to rados. This option is a "legacy" option, and will not be exposed in the QAPI or available for introspection. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-02-28block/rbd: don't copy strings in qemu_rbd_next_tok()Jeff Cody1-35/+64
This patch is prep work for parsing options for .bdrv_parse_filename, and using QDict options. The function qemu_rbd_next_tok() searched for various key/value pairs, and copied them into buffers. This will soon be an unnecessary extra step, so we will now return found strings by reference only, and offload the responsibility for safely handling/coping these strings to the caller. This also cleans up error handling some, as the callers now rely on the Error object to determine if there is a parse error. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-02-24RBD: Add support readv,writev for rbdtianqing1-24/+56
Rbd can do readv and writev directly, so wo do not need to transform iov to buf or vice versa any more. Signed-off-by: tianqing <tianqing@unitedstack.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01rbd: make the code more readableXiubo Li1-13/+12
Make it a bit clearer and more readable. Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1476519973-6436-1-git-send-email-lixiubo@cmss.chinamobile.com CC: John Snow <jsnow@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-10-23rbd: shift byte count as a 64-bit valuePaolo Bonzini1-2/+2
Otherwise, reads of more than 2GB fail. Until commit 7bbca9e290a9c7c217b5a24fc6094e91e54bd05d, reads of 2^41 bytes succeeded at least theoretically. In fact, pdiscard ought to receive a 64-bit integer as the count for the same reason. Reported by Coverity. Fixes: 7bbca9e290a9c7c217b5a24fc6094e91e54bd05d Cc: qemu-stable@nongnu.org Cc: kwolf@redhat.com Cc: eblake@redhat.com Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-10-07block: use aio_bh_schedule_oneshotPaolo Bonzini1-6/+2
This simplifies bottom half handlers by removing calls to qemu_bh_delete and thus removing the need to stash the bottom half pointer in the opaque datum. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-07-20block: Convert .bdrv_aio_discard() to byte-basedEric Blake1-8/+7
Another step towards byte-based interfaces everywhere. Replace the sector-based driver callback .bdrv_aio_discard() with a new byte-based .bdrv_aio_pdiscard(). Only raw-posix and RBD drivers are affected, so it was not worth splitting into multiple patches. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1468624988-423-9-git-send-email-eblake@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-07-20rbd: Switch rbd_start_aio() to byte-basedEric Blake1-9/+9
The internal function converts to byte-based before calling into RBD code; hoist the conversion to the callers so that callers can then be switched to byte-based themselves. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1468624988-423-8-git-send-email-eblake@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-06-20coccinelle: Remove unnecessary variables for function return valueEduardo Habkost1-3/+1
Use Coccinelle script to replace 'ret = E; return ret' with 'return E'. The script will do the substitution only when the function return type and variable type are the same. Manual fixups: * audio/audio.c: coding style of "read (...)" and "write (...)" * block/qcow2-cluster.c: wrap line to make it shorter * block/qcow2-refcount.c: change indentation of wrapped line * target-tricore/op_helper.c: fix coding style of "remainder|quotient" * target-mips/dsp_helper.c: reverted changes because I don't want to argue about checkpatch.pl * ui/qemu-pixman.c: fix line indentation * block/rbd.c: restore blank line between declarations and statements Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <1465855078-19435-4-git-send-email-ehabkost@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Unused Coccinelle rule name dropped along with a redundant comment; whitespace touched up in block/qcow2-cluster.c; stale commit message paragraph deleted] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-06-16rbd:change error_setg() to error_setg_errno()Vikhyat Umrao1-15/+23
Ceph RBD block driver does not use error_setg_errno() where it is possible to use. This patch replaces error_setg() from error_setg_errno(). Signed-off-by: Vikhyat Umrao <vumrao@redhat.com> Message-id: 1462780319-5796-1-git-send-email-vumrao@redhat.com Reviewed-by: Josh Durgin <jdurgin@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2016-03-22util: move declarations out of qemu-common.hVeronia Bahaa1-1/+1
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 Armbruster1-0/+1
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-29rbd: add support for getting password from QCryptoSecret objectDaniel P. Berrange1-0/+47
Currently RBD passwords must be provided on the command line via $QEMU -drive file=rbd:pool/image:id=myname:\ key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\ auth_supported=cephx This is insecure because the key is visible in the OS process listing. This adds support for an 'password-secret' parameter in the RBD parameters that can be used with the QCryptoSecret object to provide the password via a file: echo "QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=" > poolkey.b64 $QEMU -object secret,id=secret0,file=poolkey.b64,format=base64 \ -drive driver=rbd,filename=rbd:pool/image:id=myname:\ auth_supported=cephx,password-secret=secret0 Reviewed-by: Josh Durgin <jdurgin@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1453385961-10718-2-git-send-email-berrange@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-01-20block: Clean up includesPeter Maydell1-1/+1
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> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-07-14rbd: fix ceph settings precedenceJosh Durgin1-8/+24
Apply the ceph settings from a config file before any ceph settings from the command line. Since the ceph config file location may be specified on the command line, parse it once to read the config file, and do a second pass to apply the rest of the command line ceph options. Signed-off-by: Josh Durgin <jdurgin@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-07-14rbd: make qemu's cache setting override any ceph settingJosh Durgin1-12/+12
To be safe, when cache=none is used ceph settings should not be able to override it to turn on caching. This was previously possible with rbd_cache=true in the rbd device configuration or a ceph configuration file. Similarly, rbd settings could have turned off caching when qemu requested it, although this would just be a performance problem. Fix this by changing rbd's cache setting to match qemu after all other ceph settings have been applied. Signed-off-by: Josh Durgin <jdurgin@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-07-14rbd: remove unused constants and fieldsJosh Durgin1-10/+0
RBDAIOCB.status was only used for cancel, which was removed in 7691e24dbebb46658e89b3f950fda6ec78bbb823. RBDAIOCB.sector_num was never used. RADOSCB.done and rcbid were never used. RBD_FD* are obsolete since the pipe was removed in e04fb07fd1676e9facd7f3f878c1bbe03bccd26b. Signed-off-by: Josh Durgin <jdurgin@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-04-28Convert (ffs(val) - 1) to ctz32(val)Stefan Hajnoczi1-1/+1
This commit was generated mechanically by coccinelle from the following semantic patch: @@ expression val; @@ - (ffs(val) - 1) + ctz32(val) The call sites have been audited to ensure the ffs(0) - 1 == -1 case never occurs (due to input validation, asserts, etc). Therefore we don't need to worry about the fact that ctz32(0) == 32. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1427124571-28598-5-git-send-email-stefanha@redhat.com Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-12block/rbd: fix memory leakGonglei1-4/+4
Variable local_err going out of scope leaks the storage it points to. Cc: Markus Armbruster <armbru@redhat.com> Signed-off-by: Gonglei <arei.gonglei@huawei.com> Reviewed-by: Amos Kong <akong@redhat.com> Message-id: 1417674851-6248-1-git-send-email-arei.gonglei@huawei.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03rbd: Add support for bdrv_invalidate_cacheAdam Crume1-0/+15
This fixes Ceph issue 2467: ttp://tracker.ceph.com/issues/2467 [Dropped return r in void function as suggested by Josh Durgin <josh.durgin@inktank.com>. --Stefan] Signed-off-by: Adam Crume <adamcrume@gmail.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1412880272-3154-1-git-send-email-adamcrume@gmail.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-10-20block: Rename BlockDriverCompletionFunc to BlockCompletionFuncMarkus Armbruster1-5/+5
I'll use it with block backends shortly, and the name is going to fit badly there. It's a block layer thing anyway, not just a block driver thing. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-10-20block: Rename BlockDriverAIOCB* to BlockAIOCB*Markus Armbruster1-28/+28
I'll use BlockDriverAIOCB with block backends shortly, and the name is going to fit badly there. It's a block layer thing anyway, not just a block driver thing. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-09-22block: Rename qemu_aio_release -> qemu_aio_unrefFam Zheng1-2/+2
Suggested-by: Benoît Canet <benoit.canet@irqsave.net> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-09-22rbd: Drop rbd_aiocb_info.cancelFam Zheng1-22/+1
And also drop the now unused "cancelled" field. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-09-12block: round up file size to nearest sectorHu Tao1-1/+2
Currently the file size requested by user is rounded down to nearest sector, causing the actual file size could be a bit less than the size user requested. Since some formats (like qcow2) record virtual disk size in bytes, this can make the last few bytes cannot be accessed. This patch fixes it by rounding up file size to nearest sector so that the actual file size is no less than the requested file size. Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-08-20block: Use g_new() & friends to avoid multiplying sizesMarkus Armbruster1-1/+1
g_new(T, n) is safer than g_malloc(sizeof(*v) * n) for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. Perhaps a conversion to g_malloc_n() would be neater in places, but that's merely four years old, and we can't use such newfangled stuff. This commit only touches allocations with size arguments of the form sizeof(T), plus two that use 4 instead of sizeof(uint32_t). We can make the others safe by converting to g_malloc_n() when it becomes available to us in a couple of years. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-08-20block: Use g_new() & friends where that makes obvious senseMarkus Armbruster1-2/+2
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. Patch created with Coccinelle, with two manual changes on top: * Add const to bdrv_iterate_format() to keep the types straight * Convert the allocation in bdrv_drop_intermediate(), which Coccinelle inexplicably misses Coccinelle semantic patch: @@ type T; @@ -g_malloc(sizeof(T)) +g_new(T, 1) @@ type T; @@ -g_try_malloc(sizeof(T)) +g_try_new(T, 1) @@ type T; @@ -g_malloc0(sizeof(T)) +g_new0(T, 1) @@ type T; @@ -g_try_malloc0(sizeof(T)) +g_try_new0(T, 1) @@ type T; expression n; @@ -g_malloc(sizeof(T) * (n)) +g_new(T, n) @@ type T; expression n; @@ -g_try_malloc(sizeof(T) * (n)) +g_try_new(T, n) @@ type T; expression n; @@ -g_malloc0(sizeof(T) * (n)) +g_new0(T, n) @@ type T; expression n; @@ -g_try_malloc0(sizeof(T) * (n)) +g_try_new0(T, n) @@ type T; expression p, n; @@ -g_realloc(p, sizeof(T) * (n)) +g_renew(T, p, n) @@ type T; expression p, n; @@ -g_try_realloc(p, sizeof(T) * (n)) +g_try_renew(T, p, n) Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-08-15rbd: Handle failure for potentially large allocationsKevin Wolf1-2/+5
Some code in the block layer makes potentially huge allocations. Failure is not completely unexpected there, so avoid aborting qemu and handle out-of-memory situations gracefully. This patch addresses the allocations in the rbd block driver. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-16cleanup QEMUOptionParameterChunyan Liu1-1/+1
Now that all backend drivers are using QemuOpts, remove all QEMUOptionParameter related codes. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> Signed-off-by: Chunyan Liu <cyliu@suse.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-16rbd.c: replace QEMUOptionParameter with QemuOptsChunyan Liu1-33/+30
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> Signed-off-by: Chunyan Liu <cyliu@suse.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-06rbd: Fix leaks in rbd_start_aio() error pathKevin Wolf1-1/+4
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Josh Durgin <josh.durgin@inktank.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-04rbd: use BlockDriverState's AioContextStefan Hajnoczi1-2/+3
Drop the assumption that we're using the main AioContext. Convert qemu_bh_new() to aio_bh_new() and qemu_aio_wait() to aio_poll(). The .bdrv_detach_aio_context() and .bdrv_attach_aio_context() interfaces are not needed since no fd handlers, timers, or BHs stay registered when requests have been drained. Cc: Josh Durgin <josh.durgin@inktank.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
2014-05-28block/rbd: Propagate errors to open and create methodsMarkus Armbruster1-32/+39
Completes the conversion to Error started in commit 015a103^..d5124c0. Cc: Josh Durgin <josh.durgin@inktank.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-02-17Use error_is_set() only when necessaryMarkus Armbruster1-1/+1
error_is_set(&var) is the same as var != NULL, but it takes whole-program analysis to figure that out. Unnecessarily hard for optimizers, static checkers, and human readers. Dumb it down to obvious. Gets rid of several dozen Coverity false positives. Note that the obvious form is already used in many places. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Andreas Färber <afaerber@suse.de> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-01-22rbd: switch from pipe to QEMUBH completion notificationStefan Hajnoczi1-108/+22
rbd callbacks are called from non-QEMU threads. Up until now a pipe was used to signal completion back to the QEMU iothread. The pipe writer code handles EAGAIN using select(2). The select(2) API is not scalable since fd_set size is static. FD_SET() can write beyond the end of fd_set if the file descriptor number is too high. (QEMU's main loop uses poll(2) to avoid this issue with select(2).) Since the pipe itself is quite clumsy to use and QEMUBH is now thread-safe, just schedule a BH from the rbd callback function. This way we can simplify I/O completion in addition to eliminating the potential FD_SET() crash when file descriptor numbers become too high. Crash scenario: QEMU already has 1024 file descriptors open. Hotplug an rbd drive and get the pipe writer to take the select(2) code path. Reviewed-by: Josh Durgin <josh.durgin@inktank.com> Tested-by: Josh Durgin <josh.durgin@inktank.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-01-06qemu-option: Remove qemu_opts_create_nofailPeter Crosthwaite1-1/+1
This is a boiler-plate _nofail variant of qemu_opts_create. Remove and use error_abort in call sites. null/0 arguments needs to be added for the id and fail_if_exists fields in affected callsites due to argument inconsistency between the normal and no_fail variants. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2013-09-25rbd: avoid qemu_rbd_snap_list() memory leaksStefan Hajnoczi1-1/+2
When there are no snapshots qemu_rbd_snap_list() returns 0 and the snapshot table pointer is NULL. Don't forget to free the snaps buffer we allocated for librbd rbd_snap_list(). When the function succeeds don't forget to free the snaps buffer after calling rbd_snap_list_end(). Cc: qemu-stable@nongnu.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-09-25block: introduce BlockDriver.bdrv_needs_filename to enable some drivers.Benoît Canet1-0/+1
Some drivers will have driver specifics options but no filename. This new bool allow the block layer to treat them correctly. The .bdrv_needs_filename is set in drivers not having .bdrv_parse_filename and not having .bdrv_open. The first exception to this rule will be the quorum driver. Signed-off-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>