aboutsummaryrefslogtreecommitdiff
path: root/block.c
AgeCommit message (Collapse)Author
2014-07-18block: Add Error argument to bdrv_refresh_limits()Kevin Wolf
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-07-14block: Assert qiov length matches request lengthKevin Wolf
At least raw-posix relies on this because it can allocate bounce buffers based on the request length, but access it using all of the qiov entries later. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-07-14block: Make qiov match the request size until EOFKevin Wolf
If a read request goes across EOF, the block driver sees a shortened request that stops at EOF (the rest is memsetted in block.c), however the original qiov was used for this request. This patch makes the qiov size match the request size, avoiding a potential buffer overflow in raw-posix. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-07-09block: prefer aio_poll to qemu_aio_waitPaolo Bonzini
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-07-09block: Fix bdrv_is_allocated() return valueKevin Wolf
bdrv_is_allocated() should return either 0 or 1 in successful cases. We're lucky that currently, the callers that rely on this (e.g. because they check for ret == 1) don't seem to break badly. They just might skip some optimisation or in the case of qemu-io 'map' print separate lines where a single line would suffice. In theory, a wrong allocation status could lead to image corruption with certain operations, so let's fix this quickly. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-07-07block: block: introduce APIs for submitting IO as a batchMing Lei
This patch introduces three APIs so that following patches can support queuing I/O requests and submitting them as a batch for improving I/O performance. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Ming Lei <ming.lei@canonical.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-07-01block: extend block-commit to accept a string for the backing fileJeff Cody
On some image chains, QEMU may not always be able to resolve the filenames properly, when updating the backing file of an image after a block commit. For instance, certain relative pathnames may fail, or drives may have been specified originally by file descriptor (e.g. /dev/fd/???), or a relative protocol pathname may have been used. In these instances, QEMU may lack the information to be able to make the correct choice, but the user or management layer most likely does have that knowledge. With this extension to the block-commit api, the user is able to change the backing file of the overlay image as part of the block-commit operation. This allows the change to be 'safe', in the sense that if the attempt to write the overlay image metadata fails, then the block-commit operation returns failure, without disrupting the guest. If the commit top is the active layer, then specifying the backing file string will be treated as an error (there is no overlay image to modify in that case). If a backing file string is not specified in the command, the backing file string to use is determined in the same manner as it was previously. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-07-01block: add helper function to determine if a BDS is in a chainJeff Cody
This is a small helper function, to determine if 'base' is in the chain of BlockDriverState 'top'. It returns true if it is in the chain, and false otherwise. If either argument is NULL, it will also return false. Reviewed-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-07-01block: simplify bdrv_find_base() and bdrv_find_overlay()Jeff Cody
This simplifies the function bdrv_find_overlay(). With this change, bdrv_find_base() is just a subset of usage of bdrv_find_overlay(), so this also takes advantage of that. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-27block.c: Don't return success for bdrv_append_temp_snapshot() failureChen Gang
When failure occurs, 'ret' need be set, or may return 0 to indicate success. Previously, an error was set in errp, but 0 was returned anyway. So let bdrv_append_temp_snapshot() return an error code and use that for the bdrv_open() return value. Also, error_propagate() need be called only one time within a function. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-06-27block: Add replaces argument to drive-mirrorBenoƮt Canet
drive-mirror will bdrv_swap the new BDS named node-name with the one pointed by replaces when the mirroring is finished. Signed-off-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-06-27block: check for RESIZE blocker in the QMP command, not bdrv_truncate()Jeff Cody
If we check for the RESIZE blocker in bdrv_truncate(), that means a commit will fail if the overlay layer is larger than the base, due to the backing blocker. This is a regression in behavior from 2.0; currently, commit will try to grow the size of the base image to match the overlay size, if the overlay size is larger. By moving this into the QMP command qmp_block_resize(), it allows usage of bdrv_truncate() within block jobs. Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-06-26block: Remove a special case for protocolsKevin Wolf
The only semantic change is that bs->open_flags gets BDRV_O_PROTOCOL set now. This isn't useful, but it doesn't hurt either. The code that was previously skipped by 'goto done' is automatically disabled because protocol drivers don't support backing files (and if they did, this would probably be a fix) and can't have snapshot_flags set. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-26block: Catch backing files assigned to non-COW driversKevin Wolf
Since we parse backing.* options to add a backing file from the command line when the driver didn't assign one, it has been possible to have a backing file for e.g. raw images (it just was never accessed). This is obvious nonsense and should be rejected. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-26block: Remove second bdrv_open() recursionKevin Wolf
This recursion was introduced in commit 505d7583 in order to allow nesting image formats. It only ever takes effect when the user explicitly specifies a driver name and that driver isn't suitable for the protocol level. We can check this earlier in bdrv_open() and if the explicitly requested driver is a format driver, clear BDRV_O_PROTOCOL so that another bs->file layer is opened. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-26block: Inline bdrv_file_open()Kevin Wolf
It doesn't do much any more, we can move the code to bdrv_open() now. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-26block: Use common driver selection code for bdrv_open_file()Kevin Wolf
This moves the bdrv_open_file() call a bit down so that it can use the bdrv_open() code that selects the right block driver. The code between the old and the new call site is either common code (the error message for an unknown driver has been unified now) or doesn't run with cleared BDRV_O_PROTOCOL (added an if block in one place, whereas the right path was already asserted in another place) Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net>
2014-06-26block: Always pass driver name through options QDictKevin Wolf
The "driver" entry in the options QDict is now only missing if we're opening an image with format probing. We also catch cases now where both the drv argument and a "driver" option is specified, e.g. by specifying -drive format=qcow2,driver=raw Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-26block: Move json: parsing to bdrv_fill_options()Kevin Wolf
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-26block: Move bdrv_fill_options() call to bdrv_open()Kevin Wolf
bs->options now contains the modified version of the options. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-26block: Create bdrv_fill_options()Kevin Wolf
The idea of bdrv_fill_options() is to convert every parameter for opening images, in particular the filename and flags, to entries in the options QDict. This patch starts with moving the filename parsing and driver probing part from bdrv_file_open() to the new function. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2014-06-24block.c: Remove useless 'buf' variableChen Gang
'buf' is not used actually, so remove it and related snprintf() statement. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2014-06-23qapi event: convert BLOCK_IO_ERROR and BLOCK_JOB_ERRORWenchao Xia
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-06-23qapi event: convert DEVICE_TRAY_MOVEDWenchao Xia
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-06-23qapi: adjust existing definesWenchao Xia
In order to let event defines use existing types later, instead of redefine new ones, some old type defines for spice and vnc are changed, and BlockErrorAction is moved from block.h to qapi schema. Note that BlockErrorAction is not merged with BlockdevOnError. At this point, VncInfo is not made a child of VncBasicInfo, because VncBasicInfo has mandatory fields where VncInfo makes them optional. Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-06-23block: asynchronously stop the VM on I/O errorsPaolo Bonzini
With virtio-blk dataplane, I/O errors might occur while QEMU is not in the main I/O thread. However, it's invalid to call vm_stop when we're neither in a VCPU thread nor in the main I/O thread, even if we were to take the iothread mutex around it. To avoid this problem, we can raise a request to the main I/O thread, similar to what QEMU does when vm_stop is called from a CPU thread. We know that bdrv_error_action is called from an AIO callback, and the moment at which the callback will fire is not well-defined; it depends on the moment at which the disk or OS finishes the operation, which can happen at any time. Note that QEMU is certainly not in a CPU thread and we do not need to call cpu_stop_current() like vm_stop() does. However, we need to ensure that any action taken by management will result in correct detection of the error _and_ a running VM. In particular: - the event must be raised after the iostatus has been set, so that "info block" will return an iostatus that matches the event. - the VM must be stopped after the iostatus has been set, so that "info block" will return an iostatus that matches the runstate. The ordering between the STOP and BLOCK_IO_ERROR events is preserved; BLOCK_IO_ERROR is documented to come first. This makes bdrv_error_action() thread safe (assuming QMP events are, which is attacked by a separate series). Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-16cleanup QEMUOptionParameterChunyan Liu
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-16change block layer to support both QemuOpts and QEMUOptionParamterChunyan Liu
Change block layer to support both QemuOpts and QEMUOptionParameter. After this patch, it will change backend drivers one by one. At the end, QEMUOptionParameter will be removed and only QemuOpts is kept. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> Signed-off-by: Chunyan Liu <cyliu@suse.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-04throttle: add throttle_detach/attach_aio_context()Stefan Hajnoczi
Block I/O throttling uses timers and currently always adds them to the main loop. Throttling will break if bdrv_set_aio_context() is used to move a BlockDriverState to a different AioContext. This patch adds throttle_detach/attach_aio_context() interfaces so the throttling timers and uses them to move timers to the new AioContext. Note that bdrv_set_aio_context() already drains all requests so we're sure no throttled requests are pending. The test cases need to be updated since the throttle_init() interface has changed. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net>
2014-06-04block: add bdrv_set_aio_context()Stefan Hajnoczi
Up until now all BlockDriverState instances have used the QEMU main loop for fd handlers, timers, and BHs. This is not scalable on SMP guests and hosts so we need to move to a model with multiple event loops on different host CPUs. bdrv_set_aio_context() assigns the AioContext event loop to use for a particular BlockDriverState. It first detaches the entire BlockDriverState graph from the current AioContext and then attaches to the new AioContext. This function will be used by virtio-blk data-plane to assign a BlockDriverState to its IOThread AioContext. Make bdrv_aio_set_context() public since data-plane should not include block_int.h. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-04block: acquire AioContext in bdrv_drain_all()Stefan Hajnoczi
Modify bdrv_drain_all() to take into account that BlockDriverState instances may be running in different AioContexts. This patch changes the implementation of bdrv_drain_all() while preserving the semantics. Previously kicking throttled requests and checking for pending requests were done across all BlockDriverState instances in sequence. Now we process each BlockDriverState in turn, making sure to acquire and release its AioContext. This prevents race conditions between the thread executing bdrv_drain_all() and the thread running the AioContext. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-04block: acquire AioContext in bdrv_*_all()Stefan Hajnoczi
bdrv_close_all(), bdrv_commit_all(), bdrv_flush_all(), bdrv_invalidate_cache_all(), and bdrv_clear_incoming_migration_all() are called by main loop code and touch all BlockDriverState instances. Some BlockDriverState instances may be running in another AioContext. Make sure to acquire the AioContext before closing the BlockDriverState. This will protect against race conditions once virtio-blk data-plane is using the BlockDriverState from another AioContext event loop. Note that this patch does not convert bdrv_drain_all() yet since that conversion is non-trivial. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-06-04block: use BlockDriverState AioContextStefan Hajnoczi
Drop the assumption that we're using the main AioContext. Convert qemu_aio_wait() to aio_poll() and qemu_bh_new() to aio_bh_new() so the BlockDriverState AioContext is used. Note there is still one qemu_aio_wait() left in bdrv_create() but we do not have a BlockDriverState there and only main loop code invokes this function. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-05-30block: Plug memory leak on brv_open_image() error pathMarkus Armbruster
Introduced in commit da557a. Spotted by Coverity. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-05-28block: Drop redundant bdrv_refresh_limitsFam Zheng
The above bdrv_set_backing_hd already does this. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-05-28block: Add backing_blocker in BlockDriverStateFam Zheng
This makes use of op_blocker and blocks all the operations except for commit target, on each BlockDriverState->backing_hd. The asserts for op_blocker in bdrv_swap are removed because with this change, the target of block commit has at least the backing blocker of its child, so the assertion is not true. Callers should do their check. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-05-28block: Use bdrv_set_backing_hd everywhereFam Zheng
We need to handle the coming backing_blocker properly, so don't open code the assignment, instead, call bdrv_set_backing_hd to change backing_hd. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-05-28block: Add bdrv_set_backing_hd()Fam Zheng
This is the common but non-trivial steps to assign or change the backing_hd of BDS. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-05-28block: Replace in_use with operation blockerFam Zheng
This drops BlockDriverState.in_use with op_blockers: - Call bdrv_op_block_all in place of bdrv_set_in_use(bs, 1). - Call bdrv_op_unblock_all in place of bdrv_set_in_use(bs, 0). - Check bdrv_op_is_blocked() in place of bdrv_in_use(bs). The specific types are used, e.g. in place of starting block backup, bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP, ...). There is one exception in block_job_create, where bdrv_op_blocker_is_empty() is used, because we don't know the operation type here. This doesn't matter because in a few commits away we will drop the check and move it to callers that _do_ know the type. - Check bdrv_op_blocker_is_empty() in place of assert(!bs->in_use). Note: there is only bdrv_op_block_all and bdrv_op_unblock_all callers at this moment. So although the checks are specific to op types, this changes can still be seen as identical logic with previously with in_use. The difference is error message are improved because of blocker error info. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-05-28block: Introduce op_blockers to BlockDriverStateFam Zheng
BlockDriverState.op_blockers is an array of lists with BLOCK_OP_TYPE_MAX elements. Each list is a list of blockers of an operation type (BlockOpType), that marks this BDS as currently blocked for a certain type of operation with reason errors stored in the list. The rule of usage is: * BDS user who wants to take an operation should check if there's any blocker of the type with bdrv_op_is_blocked(). * BDS user who wants to block certain types of operation, should call bdrv_op_block (or bdrv_op_block_all to block all types of operations, which is similar to the existing bdrv_set_in_use()). * A blocker is only referenced by op_blockers, so the lifecycle is managed by caller, and shouldn't be lost until unblock, so typically a caller does these: - Allocate a blocker with error_setg or similar, call bdrv_op_block() to block some operations. - Hold the blocker, do his job. - Unblock operations that it blocked, with the same reason pointer passed to bdrv_op_unblock(). - Release the blocker with error_free(). Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-05-19block: optimize zero writes with bdrv_write_zeroesPeter Lieven
this patch tries to optimize zero write requests by automatically using bdrv_write_zeroes if it is supported by the format. This significantly speeds up file system initialization and should speed zero write test used to test backend storage performance. I ran the following 2 tests on my internal SSD with a 50G QCOW2 container and on an attached iSCSI storage. a) mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 /dev/vdX QCOW2 [off] [on] [unmap] ----- runtime: 14secs 1.1secs 1.1secs filesize: 937M 18M 18M iSCSI [off] [on] [unmap] ---- runtime: 9.3s 0.9s 0.9s b) dd if=/dev/zero of=/dev/vdX bs=1M oflag=direct QCOW2 [off] [on] [unmap] ----- runtime: 246secs 18secs 18secs filesize: 51G 192K 192K throughput: 203M/s 2.3G/s 2.3G/s iSCSI* [off] [on] [unmap] ---- runtime: 8mins 45secs 33secs throughput: 106M/s 1.2G/s 1.6G/s allocated: 100% 100% 0% * The storage was connected via an 1Gbit interface. It seems to internally handle writing zeroes via WRITESAME16 very fast. Signed-off-by: Peter Lieven <pl@kamp.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-05-19block: Allow JSON filenamesMax Reitz
If the filename given to bdrv_open() is prefixed with "json:", parse the rest as a JSON object and merge the result into the options QDict. If there are conflicts, the options QDict takes precedence. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-05-19block: Fix bdrv_is_allocated() for short backing filesKevin Wolf
bdrv_is_allocated() shouldn't return true for sectors that are unallocated, but after the end of a short backing file, even though such sectors are (correctly) marked as containing zeros. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-05-09block: Fix open flags with BDRV_O_SNAPSHOTKevin Wolf
The immediately visible effect of this patch is that it fixes committing a temporary snapshot to its backing file. Previously, it would fail with a "permission denied" error because bdrv_inherited_flags() forced the backing file to be read-only, ignoring the r/w reopen of bdrv_commit(). The bigger problem this revealed is that the original open flags must actually only be applied to the temporary snapshot, and the original image file must be treated as a backing file of the temporary snapshot and get the right flags for that. Reported-by: Jan Kiszka <jan.kiszka@web.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-04-30block: Fix open_flags in bdrv_reopen()Kevin Wolf
Use the same function as bdrv_open() for determining what the right flags for bs->file are. Without doing this, a reopen means that bs->file loses BDRV_O_CACHE_WB or BDRV_O_UNMAP if bs doesn't have it as well. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-04-30Revert "block: another bdrv_append fix"Kevin Wolf
This reverts commit 3a389e7926750cba5c83f662b1941888b2bebc04. The commit was wrong and what it tried to fix just works today without any change. What the commit tried to fix: When creating live snapshots, the new image file is opened with BDRV_O_NO_BACKING because the whole backing chain is already opened. It is then appended to the chain using bdrv_append(). The result of this was that the image had a backing file, but BDRV_O_NO_BACKING was still set. This is obviously inconsistent. There used to be some places in qemu that closed and image and then opened it again, with its old flags (a bdrv_open()/close() sequence involves reopening the whole backing file chain, too). In this case the BDRV_O_NO_BACKING flag meant that the backing chain wasn't reopened and only the top layer was left. (Most, but not all of these places are replaced by bdrv_reopen() today, which doesn't touch the backing files at all.) Other places that looked at bs->open_flags weren't interested in BDRV_O_NO_BACKING, so no breakage there. What it actually did: The commit moved the BDRV_O_NO_BACKING away to the backing file. Because the bdrv_open()/close() sequences only looked at the flags of the top level BlockDriverState and used it for the whole chain, the flag didn't hurt there any more. Obviously, it is still inconsistent because the backing file may have another backing file, but without practical impact. At the same time, it swapped all other flags. This is practically irrelevant as long as live snapshots only allow opening the new layer with the same flags as the old top layer. It still doesn't make any sense, and it is a time bomb that explodes as soon as the flags can differ. bdrv_append_temp_snapshot() is such a case: It adds the new flag BDRV_O_TEMPORARY for the temporary snapshot. The swapping of commit 3a389e79 results in the following nonsensical configuration: bs->open_flags: BDRV_O_TEMPORARY cleared bs->file->open_flags: BDRV_O_TEMPORARY set bs->backing_hd->open_flags: BDRV_O_TEMPORARY set bs->backing_hd->file->open_flags: BDRV_O_TEMPORARY cleared We're still lucky because the format layer ignores the flag and the protocol layer happens to get the right value, but sooner or later this is bound to go wrong... What the right fix would have been: Simply clear the BDRV_O_NO_BACKING flag when the BlockDriverState is appended to an existing backing file chain, because now it does have a backing file. Commit 4ddc07ca already implemented this silently in bdrv_append(), so we don't have to come up with a new fix. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-04-30block: Unlink temporary files in raw-posix/win32Kevin Wolf
Instead of having unlink() calls in the generic block layer, where we aren't even guarateed to have a file name, move them to those block drivers that are actually used and that always have a filename. Gets us rid of some #ifdefs as well. The patch also converts bs->is_temporary to a new BDRV_O_TEMPORARY open flag so that it is inherited in the protocol layer and the raw-posix and raw-win32 drivers can unlink the file. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-04-30block: Remove BDRV_O_COPY_ON_READ for bs->fileKevin Wolf
Copy on Read makes sense on the format level where backing files are implemented, but it's not required on the protocol level. While it shouldn't actively break anything to have COR enabled on both layers, needless serialisation and allocation checks may impact performance. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-04-30block: Create bdrv_backing_flags()Kevin Wolf
Instead of manipulation flags inline, move the derivation of the flags of a backing file into a new function next to the existing functions that derive flags for bs->file and for the block driver open function. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-04-30block: Create bdrv_inherited_flags()Kevin Wolf
Instead of having bdrv_open_flags() as a function that creates flags for several unrelated places and then adding open-coded flags on top, create a new function that derives the flags for bs->file from the flags for bs. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>