summaryrefslogtreecommitdiff
path: root/block.c
AgeCommit message (Collapse)Author
2017-08-01block: fix leaks in bdrv_open_driver()Manos Pitsidianakis
bdrv_open_driver() is called in two places, bdrv_new_open_driver() and bdrv_open_common(). In the latter, failure cleanup in is in its caller, bdrv_open_inherit(), which unrefs the bs->file of the failed driver open if it exists. Let's move the bs->file cleanup to bdrv_open_driver() to take care of all callers and do not set bs->drv to NULL unless the driver's open function failed. When bs is destroyed by removing its last reference, it calls bdrv_close() which checks bs->drv to perform the needed cleanups and also call the driver's close function. Since it cleans up options and opaque we must take care not leave dangling pointers. The error paths in bdrv_open_driver() are now two: If open fails, drv->bdrv_close() should not be called. Unref the child if it exists, free what we allocated and set bs->drv to NULL. Return the error and let callers free their stuff. If open succeeds but we fail after, return the error and let callers unref and delete their bs, while cleaning up their allocations. Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-08-01block: fix dangling bs->explicit_options in block.cManos Pitsidianakis
In some error paths it is possible to QDECREF a freed dangling explicit_options, resulting in a heap overflow crash. For example bdrv_open_inherit()'s fail unrefs it, then calls bdrv_unref which calls bdrv_close which also unrefs it. Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-24block: Skip implicit nodes in query-block/blockstatsKevin Wolf
Commits 0db832f and 6cdbceb introduced the automatic insertion of filter nodes above the top layer of mirror and commit block jobs. The assumption made there was that since libvirt doesn't do node-level management of the block layer yet, it shouldn't be affected by added nodes. This is true as far as commands issued by libvirt are concerned. It only uses BlockBackend names to address nodes, so any operations it performs still operate on the root of the tree as intended. However, the assumption breaks down when you consider query commands, which return data for the wrong node now. These commands also return information on some child nodes (bs->file and/or bs->backing), which libvirt does make use of, and which refer to the wrong nodes, too. One of the consequences is that oVirt gets wrong information about the image size and stops the VM in response as long as a mirror or commit job is running: https://bugzilla.redhat.com/show_bug.cgi?id=1470634 This patch fixes the problem by hiding the implicit nodes created automatically by the mirror and commit block jobs in the output of query-block and BlockBackend-based query-blockstats as long as the user doesn't indicate that they are aware of those nodes by providing a node name for them in the QMP command to start the block job. The node-based commands query-named-block-nodes and query-blockstats with query-nodes=true still show all nodes, including implicit ones. This ensures that users that are capable of node-level management can still access the full information; users that only know BlockBackends won't use these commands. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Tested-by: Eric Blake <eblake@redhat.com>
2017-07-18qemu-img: Check for backing image if specified during createJohn Snow
Or, rather, force the open of a backing image if one was specified for creation. Using a similar -unsafe option as rebase, allow qemu-img to ignore the backing file validation if possible. It may not always be possible, as in the existing case when a filesize for the new image was not specified. This is accomplished by shifting around the conditionals in bdrv_img_create, such that a backing file is always opened unless we provide BDRV_O_NO_BACKING. qemu-img is adjusted to pass this new flag when -u is provided to create. Sorry for the heinous looking diffstat, but it's mostly whitespace. Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1213786 Signed-off-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-07-14Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2017-07-13' ↵Peter Maydell
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-13block: Don't try to set *errp directlyEduardo Habkost
Assigning directly to *errp is not valid, as errp may be NULL, &error_fatal, or &error_abort. Use error_propagate() instead. With this, there's no need to check if errp is NULL anymore, as error_propagate() and error_prepend() are able to handle that. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Max Reitz <mreitz@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <20170608133906.12737-3-ehabkost@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-07-11block: Add PreallocMode to bdrv_truncate()Max Reitz
For block drivers that just pass a truncate request to the underlying protocol, we can now pass the preallocation mode instead of aborting if it is not PREALLOC_MODE_OFF. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20170613202107.10125-3-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11block: Add PreallocMode to BD.bdrv_truncate()Max Reitz
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-07-11block: add bdrv_measure() APIStefan Hajnoczi
bdrv_measure() provides a conservative maximum for the size of a new image. This information is handy if storage needs to be allocated (e.g. a SAN or an LVM volume) ahead of time. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-id: 20170705125738.8777-2-stefanha@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11block: release persistent bitmaps on inactivateVladimir Sementsov-Ogievskiy
We should release them here to reload on invalidate cache. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20170628120530.31251-31-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11block: add bdrv_can_store_new_dirty_bitmapVladimir Sementsov-Ogievskiy
This will be needed to check some restrictions before making bitmap persistent in qmp-block-dirty-bitmap-add (this functionality will be added by future patch) Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20170628120530.31251-22-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11block: bdrv_close: release bitmaps after drv->bdrv_closeVladimir Sementsov-Ogievskiy
Release bitmaps after 'if (bs->drv) { ... }' block. This will allow format driver to save persistent bitmaps, which will appear in following commits. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20170628120530.31251-17-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11block: new bdrv_reopen_bitmaps_rw interfaceVladimir Sementsov-Ogievskiy
Add format driver handler, which should mark loaded read-only bitmaps as 'IN_USE' in the image and unset read_only field in corresponding BdrvDirtyBitmap's. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20170628120530.31251-14-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11block: refactor bdrv_reopen_commitVladimir Sementsov-Ogievskiy
Add bs local variable to simplify code. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20170628120530.31251-13-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11mirror: Fix inconsistent backing AioContext for after mirroringsochin.jiang
mirror_complete opens the backing chain, which should have the same AioContext as the top when using iothreads. Make the code guarantee this, which fixes a failed assertion in bdrv_attach_child. Signed-off-by: sochin.jiang <sochin.jiang@huawei.com> Message-id: 1498475064-39816-1-git-send-email-sochin.jiang@huawei.com [mreitz: Reworded commit message] Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-07-11block: remove all encryption handling APIsDaniel P. Berrange
Now that all encryption keys must be provided upfront via the QCryptoSecret API and associated block driver properties there is no need for any explicit encryption handling APIs in the block layer. Encryption can be handled transparently within the block driver. We only retain an API for querying whether an image is encrypted or not, since that is a potentially useful piece of metadata to report to the user. Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 20170623162419.26068-18-berrange@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-06-16block: introduce dirty_bitmap_mutexPaolo Bonzini
It protects only the list of dirty bitmaps; in the next patch we will also protect their content. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20170605123908.18777-15-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2017-06-16block: protect tracked_requests and flush_queue with reqs_lockPaolo Bonzini
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20170605123908.18777-14-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2017-06-16block: access write_gen with atomicsPaolo Bonzini
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20170605123908.18777-13-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2017-06-16block: access copy_on_read with atomic opsPaolo Bonzini
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20170605123908.18777-2-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2017-05-29block/file-*: *_parse_filename() and colonsMax Reitz
The file drivers' *_parse_filename() implementations just strip the optional protocol prefix off the filename. However, for e.g. "file:foo:bar", this would lead to "foo:bar" being stored as the BDS's filename which looks like it should be managed using the "foo" protocol. This is especially troublesome if you then try to resolve a backing filename based on "foo:bar". This issue can only occur if the stripped part is a relative filename ("file:/foo:bar" will be shortened to "/foo:bar" and having a slash before the first colon means that "/foo" is not recognized as a protocol part). Therefore, we can easily fix it by prepending "./" to such filenames. Before this patch: $ ./qemu-img create -f qcow2 backing.qcow2 64M Formatting 'backing.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ ./qemu-img create -f qcow2 -b backing.qcow2 file:top:image.qcow2 Formatting 'file:top:image.qcow2', fmt=qcow2 size=67108864 backing_file=backing.qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ ./qemu-io file:top:image.qcow2 can't open device file:top:image.qcow2: Could not open backing file: Unknown protocol 'top' After this patch: $ ./qemu-io file:top:image.qcow2 [no error] Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20170522195217.12991-3-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-05-29block: Fix backing paths for filenames with colonsMax Reitz
path_combine() naturally tries to preserve a protocol prefix. However, it recognizes such a prefix by scanning for the first colon; which is different from what path_has_protocol() does: There only is a protocol prefix if there is a colon before the first slash. A protocol prefix that is not recognized by path_has_protocol() is none, and should thus not be taken as one. Case in point, before this patch: $ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2 qemu-img: ./top:image.qcow2: Could not open './top:backing.qcow2': No such file or directory Afterwards: $ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2 qemu-img: ./top:image.qcow2: Could not open './backing.qcow2': No such file or directory Reported-by: yangyang <yangyang@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20170522195217.12991-2-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-05-11block: Fix write/resize permissions for inactive imagesKevin Wolf
Format drivers for inactive nodes don't need write/resize permissions on their bs->file and can share write/resize with another VM (in fact, this is the whole point of keeping images inactive). Represent this fact in the op blocker system, so that image locking does the right thing without special-casing inactive images. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-05-11block: Inactivate parents before childrenKevin Wolf
The proper order for inactivating block nodes is that first the parents get inactivated and then the children. If we do things in this order, we can assert that we didn't accidentally leave a parent activated when one of its child nodes is inactive. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-05-11block: Drop permissions when migration completesKevin Wolf
With image locking, permissions affect other qemu processes as well. We want to be sure that the destination can run, so let's drop permissions on the source when migration completes. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-05-11block: New BdrvChildRole.activate() for blk_resume_after_migration()Kevin Wolf
Instead of manually calling blk_resume_after_migration() in migration code after doing bdrv_invalidate_cache_all(), integrate the BlockBackend activation with cache invalidation into a single function. This is achieved with a new callback in BdrvChildRole that is called by bdrv_invalidate_cache_all(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2017-05-11block: Respect "force-share" in perm propagatingFam Zheng
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-05-11block: Add, parse and store "force-share" optionFam Zheng
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-05-11block: Make bdrv_perm_names publicFam Zheng
It can be used outside of block.c for making user friendly messages. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-05-09block: Simplify bdrv_append_temp_snapshot() logicEric Blake
Noticed while checking Coccinelle results. Naming a label 'out:' when it is only used on error paths is weird. Also, we had some dead stores to 'ret'. Meanwhile we know that snapshot_options is NULL on success and that QDECREF(NULL) is safe. So merge the two exit paths into one by careful control over bs_snapshot. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20170427215821.19397-8-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-05-08qobject: Drop useless QObject castsEric Blake
We have macros in place to make it less verbose to add a subtype of QObject to both QDict and QList. While we have made cleanups like this in the past (see commit fcfcd8ffc, for example), having it be automated by Coccinelle makes it easier to maintain. Patch created mechanically via: spatch --sp-file scripts/coccinelle/qobject.cocci \ --macro-file scripts/cocci-macro-file.h --dir . --in-place then I verified that no manual touchups were required. Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20170427215821.19397-5-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-04-28block: assert no image modification under BDRV_O_INACTIVEDenis V. Lunev
As long as BDRV_O_INACTIVE is set, the image file is only opened so we have a file descriptor for it. We're definitely not supposed to modify the image, it's still owned by the migration source. This commit is an addition to 09e0c771 but the assert() is added to bdrv_truncate(). Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Kevin Wolf <kwolf@redhat.com> CC: Max Reitz <mreitz@redhat.com> Message-id: 1491405505-31620-3-git-send-email-den@openvz.org Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-28block: Add .bdrv_truncate() error messagesMax Reitz
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 Reitz
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-28block: Add errp to b{lk,drv}_truncate()Max Reitz
For one thing, this allows us to drop the error message generation from qemu-img.c and blockdev.c and instead have it unified in bdrv_truncate(). Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20170328205129.15138-3-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2017-04-27block: An empty filename counts as no filenameMax Reitz
Reproducer: $ ./qemu-img info '' qemu-img: ./block.c:1008: bdrv_open_driver: Assertion `!drv->bdrv_needs_filename || bs->filename[0]' failed. [1] 26105 abort (core dumped) ./qemu-img info '' This patch fixes this to be: $ ./qemu-img info '' qemu-img: Could not open '': The 'file' block driver requires a file name Cc: qemu-stable <qemu-stable@nongnu.org> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-04-27Revert "block/io: Comment out permission assertions"Max Reitz
This reverts commit e3e0003a8f6570aba1421ef99a0b383a43371a74. This commit was necessary for the 2.9 release because we were unable to fix the underlying issue(s) in time. However, we will be for 2.10. Signed-off-by: Max Reitz <mreitz@redhat.com> Acked-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-04-24block: use bdrv_can_set_read_only() during reopenJeff Cody
Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 00aed7ffdd7be4b9ed9ce1007d50028a72b34ebe.1491597120.git.jcody@redhat.com
2017-04-24block: introduce bdrv_can_set_read_only()Jeff Cody
Introduce check function for setting read_only flags. Will return < 0 on error, with appropriate Error value set. Does not alter any flags. Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: e2bba34ac3bc76a0c42adc390413f358ae0566e8.1491597120.git.jcody@redhat.com
2017-04-24block: code movementJeff Cody
Move bdrv_is_read_only() up with its friends. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Message-id: 73b2399459760c32506f9407efb9dddb3a2789de.1491597120.git.jcody@redhat.com
2017-04-24block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_onlyJeff Cody
The BDRV_O_ALLOW_RDWR flag allows / prohibits the changing of the BDS 'read_only' state, but there are a few places where it is ignored. In the bdrv_set_read_only() helper, make sure to honor the flag. Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: be2e5fb2d285cbece2b6d06bed54a6f56520d251.1491597120.git.jcody@redhat.com
2017-04-24block: do not set BDS read_only if copy_on_read enabledJeff Cody
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 Cody
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-24block: Make errp the last parameter of bdrv_img_createFam Zheng
Signed-off-by: Fam Zheng <famz@redhat.com> Message-Id: <20170421122710.15373-6-famz@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-04-11block/io: Comment out permission assertionsMax Reitz
In case of block migration, there may be writes to BlockBackends that do not have the write permission taken. Before this issue is fixed (which is not going to happen in 2.9), we therefore cannot assert that this is the case. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20170411145050.31290-1-mreitz@redhat.com Tested-by: Laurent Vivier <lvivier@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2017-04-11block: Introduce bdrv_coroutine_enterFam Zheng
Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2017-04-11block: Quiesce old aio context during bdrv_set_aio_contextFam Zheng
The fact that the bs->aio_context is changing can confuse the dataplane iothread, because of the now fine granularity aio context lock. bdrv_drain should rather be a bdrv_drained_begin/end pair, but since bs->aio_context is changing, we can just use aio_disable_external and bdrv_parent_drained_begin. Reported-by: Ed Swierk <eswierk@skyportsystems.com> Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2017-04-07block: Assert attached child node has right aio contextFam Zheng
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2017-04-03block: Document -drive problematic code and bugsMarkus Armbruster
-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>