aboutsummaryrefslogtreecommitdiff
path: root/block/rbd.c
AgeCommit message (Collapse)AuthorFilesLines
2019-06-12Include qemu/module.h where needed, drop it from qemu-common.hMarkus Armbruster1-0/+1
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190523143508.25387-4-armbru@redhat.com> [Rebased with conflicts resolved automatically, except for hw/usb/dev-hub.c hw/misc/exynos4210_rng.c hw/misc/bcm2835_rng.c hw/misc/aspeed_scu.c hw/display/virtio-vga.c hw/arm/stm32f205_soc.c; ui/cocoa.m fixed up]
2019-02-25block: Add strong_runtime_opts to BlockDriverMax Reitz1-0/+14
This new field can be set by block drivers to list the runtime options they accept that may influence the contents of the respective BDS. As of a follow-up patch, this list will be used by the common bdrv_refresh_filename() implementation to decide which options to put into BDS.full_open_options (and consequently whether a JSON filename has to be created), thus freeing the drivers of having to implement that logic themselves. Additionally, this patch adds the field to all of the block drivers that need it and sets it accordingly. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-id: 20190201192935.18394-22-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-11-05block: Require auto-read-only for existing fallbacksKevin Wolf1-11/+4
Some block drivers have traditionally changed their node to read-only mode without asking the user. This behaviour has been marked deprecated since 2.11, expecting users to provide an explicit read-only=on option. Now that we have auto-read-only=on, enable these drivers to make use of the option. This is the only use of bdrv_set_read_only(), so we can make it a bit more specific and turn it into a bdrv_apply_auto_read_only() that is more convenient for drivers to use. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-11-05rbd: Close image in qemu_rbd_open() error pathKevin Wolf1-0/+1
Commit e2b8247a322 introduced an error path in qemu_rbd_open() after calling rbd_open(), but neglected to close the image again in this error path. The error path should contain everything that the regular close function qemu_rbd_close() contains. This adds the missing rbd_close() call. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-10-19block: Use warn_report() & friends to report warningsMarkus Armbruster1-2/+2
Calling error_report() in a function that takes an Error ** argument is suspicious. Convert a few that are actually warnings to warn_report(). While there, split warnings consisting of multiple sentences to conform to conventions spelled out in warn_report()'s contract, and improve a rather useless warning in sheepdog.c. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Lieven <pl@kamp.de> Cc: Liu Yuan <namei.unix@gmail.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20181017082702.5581-4-armbru@redhat.com> Drop changes to "without an explicit read-only=on" warnings, because there's a series removing them pending. Also drop a cc: to a former Sheepdog maintainer. Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2018-09-24block/rbd: Attempt to parse legacy filenamesJeff Cody1-2/+52
When we converted rbd to get rid of the older key/value-centric encoding format, we broke compatibility with image files with backing file strings encoded in the old format. This leaves a bit of an ugly conundrum, and a hacky solution. If the initial attempt to parse the "proper" options fails, it assumes that we may have an older key/value encoded filename. Fall back to attempting to parse the filename, and extract the required options from it. If that fails, pass along the original error message. We do not support mixed modern usage alongside legacy keyvalue pair usage. A deprecation warning has been added, although care should be taken when actually deprecating since the impact is not limited to commandline or qapi usage, but also opening existing images. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Message-id: 15b332e5432ad069441f7275a46080f465d789a0.1536704901.git.jcody@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2018-09-24block/rbd: pull out qemu_rbd_convert_optionsJeff Cody1-12/+24
Code movement to pull the conversion from Qdict to BlockdevOptionsRbd into a helper function. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Message-id: 5b49a980f2cde6610ab1df41bb0277d00b5db893.1536704901.git.jcody@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2018-06-29block: Convert .bdrv_truncate callback to coroutine_fnKevin Wolf1-3/+5
bdrv_truncate() is an operation that can block (even for a quite long time, depending on the PreallocMode) in I/O paths that shouldn't block. Convert it to a coroutine_fn so that we have the infrastructure for drivers to make their .bdrv_co_truncate implementation asynchronous. This change could potentially introduce new race conditions because bdrv_truncate() isn't necessarily executed atomically any more. Whether this is a problem needs to be evaluated for each block driver that supports truncate: * file-posix/win32, gluster, iscsi, nfs, rbd, ssh, sheepdog: The protocol drivers are trivially safe because they don't actually yield yet, so there is no change in behaviour. * copy-on-read, crypto, raw-format: Essentially just filter drivers that pass the request to a child node, no problem. * qcow2: The implementation modifies metadata, so it needs to hold s->lock to be safe with concurrent I/O requests. In order to avoid double locking, this requires pulling the locking out into preallocate_co() and using qcow2_write_caches() instead of bdrv_flush(). * qed: Does a single header update, this is fine without locking. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
2018-06-15rbd: New parameter key-secretMarkus Armbruster1-16/+25
Legacy -drive supports "password-secret" parameter that isn't available with -blockdev / blockdev-add. That's because we backed out our first try to provide it there due to interface design doubts, in commit 577d8c9a811, v2.9.0. This is the second try. It brings back the parameter, except it's named "key-secret" now. Let's review our reasons for backing out the first try, as stated in the commit message: * BlockdevOptionsRbd member @password-secret isn't actually a password, it's a key generated by Ceph. Addressed by the rename. * We're not sure where member @password-secret belongs (see the previous commit). See previous commit. * How @password-secret interacts with settings from a configuration file specified with @conf is undocumented. Not actually true, the documentation for @conf says "Values in the configuration file will be overridden by options specified via QAPI", and we've tested this. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-06-15rbd: New parameter auth-client-requiredMarkus Armbruster1-10/+32
Parameter auth-client-required lets you configure authentication methods. We tried to provide that in v2.9.0, but backed out due to interface design doubts (commit 464444fcc16). This commit is similar to what we backed out, but simpler: we use a list of enumeration values instead of a list of objects with a member of enumeration type. Let's review our reasons for backing out the first try, as stated in the commit message: * The implementation uses deprecated rados_conf_set() key "auth_supported". No biggie. Fixed: we use "auth-client-required". * 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. That fix is commit 2836284db60. This commit doesn't bring the bugs back. * BlockdevOptionsRbd member @password-secret applies only to authentication method cephx. Should it be a variant member of RbdAuthMethod? We've had time to ponder, and we decided to stick to the way Ceph configuration works: the key configured separately, and silently ignored if the authentication method doesn't use it. * 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? Likewise. * 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. Again, we decided to stick to the way Ceph configuration works, except we make auth-client-required a list of enumeration values instead of a string containing keywords separated by delimiters. * How BlockdevOptionsRbd member @auth-supported interacts with settings from a configuration file specified with @conf is undocumented. I suspect it's untested, too. Not actually true, the documentation for @conf says "Values in the configuration file will be overridden by options specified via QAPI", and we've tested this. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-06-15block: Factor out qobject_input_visitor_new_flat_confused()Markus Armbruster1-5/+2
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-06-15block: Fix -blockdev for certain non-string scalarsMarkus Armbruster1-1/+1
Configuration flows through the block subsystem in a rather peculiar way. Configuration made with -drive enters it as QemuOpts. Configuration made with -blockdev / blockdev-add enters it as QAPI type BlockdevOptions. The block subsystem uses QDict, QemuOpts and QAPI types internally. The precise flow is next to impossible to explain (I tried for this commit message, but gave up after wasting several hours). What I can explain is a flaw in the BlockDriver interface that leads to this bug: $ qemu-system-x86_64 -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234 qemu-system-x86_64: -blockdev node-name=n1,driver=nfs,server.type=inet,server.host=localhost,path=/foo/bar,user=1234: Internal error: parameter user invalid QMP blockdev-add is broken the same way. Here's what happens. The block layer passes configuration represented as flat QDict (with dotted keys) to BlockDriver methods .bdrv_file_open(). The QDict's members are typed according to the QAPI schema. nfs_file_open() converts it to QAPI type BlockdevOptionsNfs, with qdict_crumple() and a qobject input visitor. This visitor comes in two flavors. The plain flavor requires scalars to be typed according to the QAPI schema. That's the case here. The keyval flavor requires string scalars. That's not the case here. nfs_file_open() uses the latter, and promptly falls apart for members @user, @group, @tcp-syn-count, @readahead-size, @page-cache-size, @debug. Switching to the plain flavor would fix -blockdev, but break -drive, because there the scalars arrive in nfs_file_open() as strings. The proper fix would be to replace the QDict by QAPI type BlockdevOptions in the BlockDriver interface. Sadly, that's beyond my reach right now. Next best would be to fix the block layer to always pass correctly typed QDicts to the BlockDriver methods. Also beyond my reach. What I can do is throw another hack onto the pile: have nfs_file_open() convert all members to string, so use of the keyval flavor actually works, by replacing qdict_crumple() by new function qdict_crumple_for_keyval_qiv(). The pattern "pass result of qdict_crumple() to qobject_input_visitor_new_keyval()" occurs several times more: * qemu_rbd_open() Same issue as nfs_file_open(), but since BlockdevOptionsRbd has only string members, its only a latent bug. Fix it anyway. * parallels_co_create_opts(), qcow_co_create_opts(), qcow2_co_create_opts(), bdrv_qed_co_create_opts(), sd_co_create_opts(), vhdx_co_create_opts(), vpc_co_create_opts() These work, because they create the QDict with qemu_opts_to_qdict_filtered(), which creates only string scalars. The function sports a TODO comment asking for better typing; that's going to be fun. Use qdict_crumple_for_keyval_qiv() to be safe. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-06-15block: Add block-specific QDict headerMax Reitz1-0/+1
There are numerous QDict functions that have been introduced for and are used only by the block layer. Move their declarations into an own header file to reflect that. While qdict_extract_subqdict() is in fact used outside of the block layer (in util/qemu-config.c), it is still a function related very closely to how the block layer works with nested QDicts, namely by sometimes flattening them. Therefore, its declaration is put into this header as well and util/qemu-config.c includes it with a comment stating exactly which function it needs. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20180509165530.29561-7-mreitz@redhat.com> [Copyright note tweaked, superfluous includes dropped] Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-06-15rbd: Drop deprecated -drive parameter "filename"Markus Armbruster1-16/+0
Parameter "filename" is deprecated since commit 91589d9e5ca, v2.10.0. Time to get rid of it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-05-15rbd: Switch to byte-based callbacksEric Blake1-18/+22
We are gradually moving away from sector-based interfaces, towards byte-based. Make the change for the last few sector-based callbacks in the rbd driver. Note that the driver was already using byte-based calls for performing actual I/O, so this just gets rid of a round trip of scaling; however, as I don't know if RBD is tolerant of non-sector AIO operations, I went with the conservate approach of adding .bdrv_refresh_limits to override the block layer defaults back to the pre-patch value of 512. Signed-off-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-05-04qobject: Replace qobject_incref/QINCREF qobject_decref/QDECREFMarc-André Lureau1-7/+7
Now that we can safely call QOBJECT() on QObject * as well as its subtypes, we can have macros qobject_ref() / qobject_unref() that work everywhere instead of having to use QINCREF() / QDECREF() for QObject and qobject_incref() / qobject_decref() for its subtypes. The replacement is mechanical, except I broke a long line, and added a cast in monitor_qmp_cleanup_req_queue_locked(). Unlike qobject_decref(), qobject_unref() doesn't accept void *. Note that the new macros evaluate their argument exactly once, thus no need to shout them. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20180419150145.24795-4-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Rebased, semantic conflict resolved, commit message improved] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2018-04-04block/rbd: remove processed options from qdictJeff Cody1-0/+7
Commit 4bfb274 added some QAPIfication of option parsing in qemu_rbd_open(). We need to remove all the options we processed, otherwise in bdrv_open_inherit() we will think the remaining options are invalid. (This needs to go in 2.12 to avoid a regression that prevents rbd from being opened.) Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19qapi: Replace qobject_to_X(o) by qobject_to(X, o)Max Reitz1-4/+4
This patch was generated using the following Coccinelle script: @@ expression Obj; @@ ( - qobject_to_qnum(Obj) + qobject_to(QNum, Obj) | - qobject_to_qstring(Obj) + qobject_to(QString, Obj) | - qobject_to_qdict(Obj) + qobject_to(QDict, Obj) | - qobject_to_qlist(Obj) + qobject_to(QList, Obj) | - qobject_to_qbool(Obj) + qobject_to(QBool, Obj) ) and a bit of manual fix-up for overly long lines and three places in tests/check-qjson.c that Coccinelle did not find. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20180224154033.29559-4-mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: swap order from qobject_to(o, X), rebase to master, also a fix to latent false-positive compiler complaint about hw/i386/acpi-build.c] Signed-off-by: Eric Blake <eblake@redhat.com>
2018-03-09rbd: Use qemu_rbd_connect() in qemu_rbd_do_create()Kevin Wolf1-44/+10
This is almost exactly the same code. The differences are that qemu_rbd_connect() supports BlockdevOptionsRbd.server and that the cache mode is set explicitly. Supporting 'server' is a welcome new feature for image creation. Caching is disabled by default, so leave it that way. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-09rbd: Assign s->snap/image_name in qemu_rbd_open()Kevin Wolf1-9/+5
Now that the options are already available in qemu_rbd_open() and not only parsed in qemu_rbd_connect(), we can assign s->snap and s->image_name there instead of passing the fields by reference to qemu_rbd_connect(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-09rbd: Support .bdrv_co_createKevin Wolf1-50/+100
This adds the .bdrv_co_create driver callback to rbd, which enables image creation over QMP. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-09rbd: Pass BlockdevOptionsRbd to qemu_rbd_connect()Kevin Wolf1-60/+55
With the conversion to a QAPI options object, the function is now prepared to be used in a .bdrv_co_create implementation. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-09rbd: Remove non-schema options from runtime_optsKevin Wolf1-31/+24
Instead of the QemuOpts in qemu_rbd_connect(), we want to use QAPI objects. As a preparation, fetch those options directly from the QDict that .bdrv_open() supports in the rbd driver and that are not in the schema. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-09rbd: Factor out qemu_rbd_connect()Kevin Wolf1-40/+60
The code to establish an RBD connection is duplicated between open and create. In order to be able to share the code, factor out the code from qemu_rbd_open() as a first step. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-09rbd: Fix use after free in qemu_rbd_set_keypairs() error pathKevin Wolf1-1/+2
If we want to include the invalid option name in the error message, we can't free the string earlier than that. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-03-09block: convert bdrv_invalidate_cache callback to coroutine_fnPaolo Bonzini1-3/+3
QED's bdrv_invalidate_cache implementation would like to reuse functions that acquire/release the metadata locks. Call it from coroutine context to simplify the logic. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1516279431-30424-6-git-send-email-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-02block: rename .bdrv_create() to .bdrv_co_create_opts()Stefan Hajnoczi1-2/+4
BlockDriver->bdrv_create() has been called from coroutine context since commit 5b7e1542cfa41a281af9629d31cef03704d976e6 ("block: make bdrv_create adopt coroutine"). Make this explicit by renaming to .bdrv_co_create_opts() and add the coroutine_fn annotation. This makes it obvious to block driver authors that they may yield, use CoMutex, or other coroutine_fn APIs. bdrv_co_create is reserved for the QAPI-based version that Kevin is working on. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20170705102231.20711-2-stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-02-09Move include qemu/option.h from qemu-common.h to actual usersMarkus Armbruster1-0/+1
qemu-common.h includes qemu/option.h, but most places that include the former don't actually need the latter. Drop the include, and add it to the places that actually need it. While there, drop superfluous includes of both headers, and separate #include from file comment with a blank line. This cleanup makes the number of objects depending on qemu/option.h drop from 4545 (out of 4743) to 284 in my "build everything" tree. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-20-armbru@redhat.com> [Semantic conflict with commit bdd6a90a9e in block/nvme.c resolved]
2018-02-09Include qapi/qmp/qdict.h exactly where neededMarkus Armbruster1-0/+1
This cleanup makes the number of objects depending on qapi/qmp/qdict.h drop from 4550 (out of 4743) to 368 in my "build everything" tree. For qapi/qmp/qobject.h, the number drops from 4552 to 390. While there, separate #include from file comment with a blank line. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-13-armbru@redhat.com>
2018-02-09Include qapi/qmp/qlist.h exactly where neededMarkus Armbruster1-0/+1
This cleanup makes the number of objects depending on qapi/qmp/qlist.h drop from 4551 (out of 4743) to 16 in my "build everything" tree. While there, separate #include from file comment with a blank line. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-12-armbru@redhat.com>
2017-11-17block: Deprecate bdrv_set_read_only() and usersKevin Wolf1-4/+10
bdrv_set_read_only() is used by some block drivers to override the read-only option given by the user. This is not how read-only images generally work in QEMU: Instead of second guessing what the user really meant (which currently includes making an image read-only even if the user didn't only use the default, but explicitly said read-only=off), we should error out if we can't provide what the user requested. This adds deprecation warnings to all callers of bdrv_set_read_only() so that the behaviour can be corrected after the usual deprecation period. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-09-04qapi: Mechanically convert FOO_lookup[...] to FOO_str(...)Markus Armbruster1-1/+1
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1503564371-26090-14-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2017-07-14Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2017-07-13' ↵Peter Maydell1-3/+3
into staging Error reporting patches for 2017-07-13 # gpg: Signature made Thu 13 Jul 2017 12:55:45 BST # gpg: using RSA key 0x3870B400EB918653 # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653 * remotes/armbru/tags/pull-error-2017-07-13: Convert error_report*_err() to warn_report*_err() error: Implement the warn and free Error functions char-socket: Report TCP socket waiting as information Convert error_report() to warn_report() error: Functions to report warnings and informational messages util/qemu-error: Rename error_print_loc() to be more generic websock: Don't try to set *errp directly block: Don't try to set *errp directly xilinx: Fix latent error handling bug Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-07-13Convert error_report() to warn_report()Alistair Francis1-3/+3
Convert all uses of error_report("warning:"... to use warn_report() instead. This helps standardise on a single method of printing warnings to the user. All of the warnings were changed using these two commands: find ./* -type f -exec sed -i \ 's|error_report(".*warning[,:] |warn_report("|Ig' {} + Indentation fixed up manually afterwards. The test-qdev-global-props test case was manually updated to ensure that this patch passes make check (as the test cases are case sensitive). Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Suggested-by: Thomas Huth <thuth@redhat.com> Cc: Jeff Cody <jcody@redhat.com> Cc: Kevin Wolf <kwolf@redhat.com> Cc: Max Reitz <mreitz@redhat.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Lieven <pl@kamp.de> Cc: Josh Durgin <jdurgin@redhat.com> Cc: "Richard W.M. Jones" <rjones@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com> Cc: Richard Henderson <rth@twiddle.net> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> Cc: Greg Kurz <groug@kaod.org> Cc: Rob Herring <robh@kernel.org> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Peter Chubb <peter.chubb@nicta.com.au> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Marcel Apfelbaum <marcel@redhat.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: David Gibson <david@gibson.dropbear.id.au> Cc: Alexander Graf <agraf@suse.de> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: Cornelia Huck <cohuck@redhat.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: David Gibson <david@gibson.dropbear.id.au> Acked-by: Greg Kurz <groug@kaod.org> Acked-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed by: Peter Chubb <peter.chubb@data61.csiro.au> Acked-by: Max Reitz <mreitz@redhat.com> Acked-by: Marcel Apfelbaum <marcel@redhat.com> Message-Id: <e1cfa2cd47087c248dd24caca9c33d9af0c499b0.1499866456.git.alistair.francis@xilinx.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-07-11block: Add PreallocMode to BD.bdrv_truncate()Max Reitz1-1/+8
Add a PreallocMode parameter to the bdrv_truncate() function implemented by each block driver. Currently, we always pass PREALLOC_MODE_OFF and no driver accepts anything else. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20170613202107.10125-2-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-06-26block: change variable names in BlockDriverStateManos Pitsidianakis1-2/+2
Change the 'int count' parameter in *pwrite_zeros, *pdiscard related functions (and some others) to 'int bytes', as they both refer to bytes. This helps with code legibility. Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> Message-id: 20170609101808.13506-1-el13635@mail.ntua.gr Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-06-14block/rbd: enable filename option and parsingJeff Cody1-1/+21
When enabling option parsing and blockdev-add for rbd, we removed the 'filename' option. Unfortunately, this was a bit optimistic, as previous versions of QEMU allowed the use of the option in backing filenames via json. This means that without parsing this option, we cannot open existing images that used to work fine. See bug: https://bugzilla.redhat.com/show_bug.cgi?id=1457088 Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Message-id: 937dc9fde348d13311eb8e23444df3bc3190b612.1497444637.git.jcody@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2017-05-09qobject: Use simpler QDict/QList scalar insertion macrosEric Blake1-8/+8
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-04-28block: Add .bdrv_truncate() error messagesMax Reitz1-0/+1
Add missing error messages for the block driver implementations of .bdrv_truncate(); drop the generic one from block.c's bdrv_truncate(). Since one of these changes touches a mis-indented block in block/file-posix.c, this patch fixes that coding style issue along the way. Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20170328205129.15138-5-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-28block: Add errp to BD.bdrv_truncate()Max Reitz1-1/+1
Add an Error parameter to the block drivers' bdrv_truncate() interface. If a block driver does not set this in case of an error, the generic bdrv_truncate() implementation will do so. Where it is obvious, this patch also makes some block drivers set this value. Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20170328205129.15138-4-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-24block/rbd: Add support for reopen()Jeff Cody1-0/+21
This adds support for reopen in rbd, for changing between r/w and r/o. Note, that this is only a flag change, but we will block a change from r/o to r/w if we are using an RBD internal snapshot. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: d4e87539167ec6527d44c97b164eabcccf96e4f3.1491597120.git.jcody@redhat.com
2017-04-24block/rbd - update variable names to more apt namesJeff Cody1-16/+17
Update 'clientname' to be 'user', which tracks better with both the QAPI and rados variable naming. Update 'name' to be 'image_name', as it indicates the rbd image. Naming it 'image' would have been ideal, but we are using that for the rados_image_t value returned by rbd_open(). Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: b7ec1fb2e1cf36f9b6911631447a5b0422590b7d.1491597120.git.jcody@redhat.com
2017-04-24block: do not set BDS read_only if copy_on_read enabledJeff Cody1-1/+10
A few block drivers will set the BDS read_only flag from their .bdrv_open() function. This means the bs->read_only flag could be set after we enable copy_on_read, as the BDRV_O_COPY_ON_READ flag check occurs prior to the call to bdrv->bdrv_open(). This adds an error return to bdrv_set_read_only(), and an error will be return if we try to set the BDS to read_only while copy_on_read is enabled. This patch also changes the behavior of vvfat. Before, vvfat could override the drive 'readonly' flag with its own, internal 'rw' flag. For instance, this -drive parameter would result in a writable image: "-drive format=vvfat,dir=/tmp/vvfat,rw,if=virtio,readonly=on" This is not correct. Now, attempting to use the above -drive parameter will result in an error (i.e., 'rw' is incompatible with 'readonly=on'). Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 0c5b4c1cc2c651471b131f21376dfd5ea24d2196.1491597120.git.jcody@redhat.com
2017-04-24block: add bdrv_set_read_only() helper functionJeff Cody1-1/+1
We have a helper wrapper for checking for the BDS read_only flag, add a helper wrapper to set the read_only flag as well. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 9b18972d05f5fa2ac16c014f0af98d680553048d.1491597120.git.jcody@redhat.com
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>
generated by cgit v1.1 at 2024-11-03 14:58:16 +0000