summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* MAINTAINERS: Document QOMAndreas Färber2014-05-051-0/+11
| | | | | | | | | Invented by Anthony. Maintained through my qom-next tree lately. Cc: Anthony Liguori <aliguori@amazon.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Acked-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Andreas Färber <afaerber@suse.de>
* arm: Clean up fragile use of error_is_set() in realize() methodsMarkus Armbruster2014-05-053-6/+12
| | | | | | | | | | | | | | | | | | | Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit 84d18f0). I guess the error_is_set(errp) in the DeviceClass realize() methods are merely fragile right now, because I can't find a call chain that passes a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Acked-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Andreas Färber <afaerber@suse.de>
* qom: Clean up fragile use of error_is_set() in set() methodsMarkus Armbruster2014-05-056-24/+45
| | | | | | | | | | | | | | | | | | Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit 84d18f0). I guess the error_is_set(errp) in the ObjectProperty set() methods are merely fragile right now, because I can't find a call chain that passes a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* hw: Consistently name Error ** objects errp, and not errMarkus Armbruster2014-05-053-14/+14
| | | | | Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* hw: Consistently name Error * objects err, and not errpMarkus Armbruster2014-05-053-21/+21
| | | | | Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* machine: Remove QEMUMachine indirection from MachineClassMarcel Apfelbaum2014-05-055-87/+50Star
| | | | | | | | No need to go through qemu_machine field. Use MachineClass fields directly. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* machine: Replace QEMUMachine by MachineClass in accelerator configurationMarcel Apfelbaum2014-05-0511-17/+17
| | | | | | | This minimizes QEMUMachine usage, as part of machine QOM-ification. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* vl.c: Replace QEMUMachine with MachineClass in QEMUMachineInitArgsMarcel Apfelbaum2014-05-052-3/+4
| | | | | | | | | QEMUMachine's fields are already in MachineClass. We can safely make the switch because we copy them in machine_class_init() and spapr_machine_class_init(). Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* machine: Copy QEMUMachine's fields to MachineClassMarcel Apfelbaum2014-05-053-0/+71
| | | | | | | | | | | In order to eliminate the QEMUMachine indirection, add its fields directly to MachineClass. Do not yet remove qemu_machine field because it is still in use by sPAPR. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> [AF: Copied fields for sPAPR, too] Signed-off-by: Andreas Färber <afaerber@suse.de>
* machine: Remove obsoleted field from QEMUMachineMarcel Apfelbaum2014-05-051-1/+0Star
| | | | | | | | | | | | | | | | This field shouldn't be used any more since we adopted the QOM way of iterating over the types. The commit that obsoleted it is: commit 261747f176f6f2d88f8268aaebfdd1a1afe887e2 vl: Use MachineClass instead of global QEMUMachine list The machine registration flow is refactored to use the QOM functionality. Instead of linking the machines into a list, each machine has a type and the types can be traversed in the QOM way. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* qdev: Fix crash by validating the object typeAmos Kong2014-05-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | QEMU crashed when I try to list device parameters and the driver name is actually an available bus name. # qemu -device virtio-pci-bus,? # qemu -device virtio-bus,? # qemu -device virtio-serial-bus,? qdev-monitor.c:212:qdev_device_help: Object 0x7fd932f50620 is not an instance of type device Aborted (core dumped) We can also reproduce this bug by adding device from monitor, so it's worth to fix the crash. (qemu) device_add virtio-serial-bus qdev-monitor.c:491:qdev_device_add: Object 0x7f5e89530920 is not an instance of type device Aborted (core dumped) Cc: qemu-stable@nongnu.org Signed-off-by: Amos Kong <akong@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de>
* Merge remote-tracking branch ↵Peter Maydell2014-05-025-48/+99
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 'remotes/pmaydell/tags/pull-target-arm-20140501' into staging target-arm queue: * implement XScale cache lockdown cp15 ops * fix v7M CPUID base register * implement WFE and YIELD as yields for A64 * fix A64 "BLR LR" * support Cortex-A57 in virt machine model * a few other minor AArch64 bugfixes # gpg: Signature made Thu 01 May 2014 15:42:17 BST using RSA key ID 14360CDE # gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>" * remotes/pmaydell/tags/pull-target-arm-20140501: hw/arm/virt: Add support for Cortex-A57 hw/arm/virt: Put GIC register banks on 64K boundaries hw/arm/virt: Create the GIC ourselves rather than (ab)using a15mpcore_priv target-arm: Correct a comment refering to EL0 target-arm: A64: Fix a typo when declaring TLBI ops target-arm: A64: Handle blr lr target-arm: Make vbar_write 64bit friendly on 32bit hosts target-arm: implement WFE/YIELD as a yield for AArch64 armv7m_nvic: fix CPUID Base Register target-arm: Implement XScale cache lockdown operations as NOPs Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * hw/arm/virt: Add support for Cortex-A57Peter Maydell2014-05-011-0/+5
| | | | | | | | | | | | | | Support the Cortex-A57 in the virt machine model. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1398362083-17737-4-git-send-email-peter.maydell@linaro.org
| * hw/arm/virt: Put GIC register banks on 64K boundariesPeter Maydell2014-05-011-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | For an AArch64 CPU which supports 64K pages, having the GIC register banks at 4K offsets is potentially awkward. Move them out to being at 64K offsets. (This is harmless for AArch32 CPUs and for AArch64 CPUs with 4K pages, so it is simpler to use the same offsets everywhere than to try to use 64K offsets only for AArch64 host CPUs.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1398362083-17737-3-git-send-email-peter.maydell@linaro.org
| * hw/arm/virt: Create the GIC ourselves rather than (ab)using a15mpcore_privPeter Maydell2014-05-011-29/+53
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rather than having the virt machine model create an a15mpcore_priv device regardless of the actual CPU type in order to instantiate the GIC, move to having the machine model create the GIC directly. This corresponds to a system which uses a standalone GIC (eg the GIC-400) rather than the one built in to the CPU core. The primary motivation for this is to support the Cortex-A57, which for a KVM configuration will use a GICv2, which is not built into the CPU. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1398362083-17737-2-git-send-email-peter.maydell@linaro.org
| * target-arm: Correct a comment refering to EL0Edgar E. Iglesias2014-05-011-1/+1
| | | | | | | | | | | | Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Message-id: 1398926097-28097-5-git-send-email-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * target-arm: A64: Fix a typo when declaring TLBI opsEdgar E. Iglesias2014-05-011-12/+12
| | | | | | | | | | | | | | | | | | | | Harmless typo as opc1 defaults to zero and opc2 gets re-declared to its correct value. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 1398926097-28097-4-git-send-email-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * target-arm: A64: Handle blr lrEdgar E. Iglesias2014-05-011-1/+2
| | | | | | | | | | | | | | | | | | | | | | For linked branches, updates to the link register happen conceptually after the read of the branch target register. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Cc: qemu-stable@nongnu.org Message-id: 1398926097-28097-3-git-send-email-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * target-arm: Make vbar_write 64bit friendly on 32bit hostsEdgar E. Iglesias2014-05-011-1/+1
| | | | | | | | | | | | | | Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 1398926097-28097-2-git-send-email-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * target-arm: implement WFE/YIELD as a yield for AArch64Rob Herring2014-05-011-0/+6
| | | | | | | | | | | | | | | | | | Like was done for AArch32 for WFE, implement both WFE and YIELD as a yield operation. This speeds up multi-core system emulation. Signed-off-by: Rob Herring <rob.herring@linaro.org> Message-id: 1397588401-20366-1-git-send-email-robherring2@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * armv7m_nvic: fix CPUID Base RegisterRabin Vincent2014-05-011-1/+1
| | | | | | | | | | | | | | | | | | cp15.c0_cpuid is never initialized for ARMv7-M; take the value directly from cpu->midr instead. Signed-off-by: Rabin Vincent <rabin@rab.in> Message-id: 1398036308-32166-1-git-send-email-rabin@rab.in Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * target-arm: Implement XScale cache lockdown operations as NOPsPeter Maydell2014-05-011-0/+15
| | | | | | | | | | | | | | | | | | | | | | XScale defines some implementation-specific coprocessor registers for doing cache lockdown operations. Since QEMU doesn't model a cache no proper implementation is possible, but NOP out the registers so that guest code like u-boot that tries to use them doesn't crash. Reported-by: <prqek@centrum.cz> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
* | Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into stagingPeter Maydell2014-05-0224-171/+355
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Block patches # gpg: Signature made Wed 30 Apr 2014 19:19:32 BST using RSA key ID C88F2FD6 # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" * remotes/kevin/tags/for-upstream: (31 commits) curl: Fix hang reading from slow connections curl: Ensure all informationals are checked for completion curl: Eliminate unnecessary use of curl_multi_socket_all curl: Remove unnecessary explicit calls to internal event handler curl: Remove erroneous sleep waiting for curl completion curl: Fix return from curl_read_cb with invalid state curl: Remove unnecessary use of goto curl: Fix long line block/vdi: Error out immediately in vdi_create() block/bochs: Fix error handling for seek_to_sector() qcow2: Check min_size in qcow2_grow_l1_table() qcow2: Catch bdrv_getlength() error block: Use correct width in format strings qcow2: Avoid overflow in alloc_clusters_noref() block: Use error_abort in bdrv_image_info_specific_dump() block: Fix open_flags in bdrv_reopen() Revert "block: another bdrv_append fix" block: Unlink temporary files in raw-posix/win32 block: Remove BDRV_O_COPY_ON_READ for bs->file block: Create bdrv_backing_flags() ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
| * | curl: Fix hang reading from slow connectionsMatthew Booth2014-04-301-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When receiving a new aio read request, we first look for an existing transaction whose range will cover the read request by the time it completes. However, we weren't checking that the existing transaction was still active. If it had timed out, we were adding the request to a transaction which would never complete and had already been cancelled, resulting in a hang. Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | curl: Ensure all informationals are checked for completionMatthew Booth2014-04-301-30/+23Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | According to the documentation, the correct way to ensure all informationals have been returned by curl_multi_info_read is to loop until it returns NULL. Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | curl: Eliminate unnecessary use of curl_multi_socket_allMatthew Booth2014-04-301-10/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | curl_multi_socket_all is a deprecated catch-all which checks for activities on all open curl sockets. We have enough information from the event loop to check only the sockets with activity. This change removes use of curl_multi_socket_all in favour of curl_multi_socket_action called with the relevant handle. At the same time, it also ensures that the driver only checks for completion of read operations after reading from a socket, rather than both reading and writing. Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | curl: Remove unnecessary explicit calls to internal event handlerMatthew Booth2014-04-301-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove calls to curl_multi_do where the relevant handles are already registered to the event loop. Ensure that we kick off socket handling with CURL_SOCKET_TIMEOUT after adding a new handle. Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | curl: Remove erroneous sleep waiting for curl completionMatthew Booth2014-04-301-2/+1Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The driver will not start more than a fixed number of curl sessions. If it needs more, it must wait for the completion of an existing one. The driver was sleeping, which will prevent the main loop from running, and therefore the event it's waiting on. It was also directly calling its internal handler rather than waiting on existing registered handlers to be called from the main loop. This change causes it simply to wait for a period of time whilst allowing the main loop to execute. Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | curl: Fix return from curl_read_cb with invalid stateMatthew Booth2014-04-301-2/+1Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A curl write callback is supposed to return the number of bytes it handled. curl_read_cb would have erroneously reported it had handled all bytes in the event that the internal curl state was invalid. Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | curl: Remove unnecessary use of gotoMatthew Booth2014-04-301-28/+27Star
| | | | | | | | | | | | | | | | | | | | | | | | This isn't any of the usually acceptable uses of goto. Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | curl: Fix long lineMatthew Booth2014-04-301-1/+2
| | | | | | | | | | | | | | | | | | Signed-off-by: Matthew Booth <mbooth@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | block/vdi: Error out immediately in vdi_create()Max Reitz2014-04-301-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, if an error occurs during the part of vdi_create() which actually writes the image, the function stores -errno, but continues anyway. Instead of trying to write data which (if it can be written at all) does not make any sense without the operations before succeeding (e.g., writing the image header), just error out immediately. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Stefan Weil <sw@weilnetz.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | block/bochs: Fix error handling for seek_to_sector()Max Reitz2014-04-301-9/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, seek_to_sector() returns -1 both for errors and unallocated sectors, resulting in silent errors. As 0 is an invalid offset of data clusters (bitmap_offset is greater than 0 because s->data_offset is greater than 0), just return 0 for unallocated sectors and -errno in case of error. This should then be propagated by bochs_read(), the sole user of seek_to_sector(). That function also has a case of "return -1 in case of error", which is fixed by this patch as well. bochs_read() is called by bochs_co_read() which passes the return value through, therefore it is indeed correct for bochs_read() to return -errno. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | qcow2: Check min_size in qcow2_grow_l1_table()Max Reitz2014-04-301-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | First, new_l1_size is an int64_t, whereas min_size is a uint64_t. Therefore, during the loop which adjusts new_l1_size until it equals or exceeds min_size, new_l1_size might overflow and become negative. The comparison in the loop condition however will take it as an unsigned value (because min_size is unsigned) and therefore recognize it as exceeding min_size. Therefore, the loop is left with a negative new_l1_size, which is not correct. This could be fixed by making new_l1_size uint64_t. On the other hand, however, by doing this, the while loop may take forever. If min_size is e.g. UINT64_MAX, it will take new_l1_size probably multiple overflows to reach the exact same value (if it reaches it at all). Then, right after the loop, new_l1_size will be recognized as being too big anyway. Both problems require a ridiculously high min_size value, which is very unlikely to occur; but both problems are also simply avoided by checking whether min_size is sane before calculating new_l1_size (which should still be checked separately, though). Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | qcow2: Catch bdrv_getlength() errorMax Reitz2014-04-301-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | The call to bdrv_getlength() from qcow2_check_refcounts() may result in an error. Check this and abort if necessary. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | block: Use correct width in format stringsMax Reitz2014-04-306-24/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of blindly relying on a normal integer having a width of 32 bits (which is a pretty good assumption, but we should not rely on it if there is no need), use the correct format string macros. This does not touch DEBUG output. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | qcow2: Avoid overflow in alloc_clusters_noref()Max Reitz2014-04-301-0/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | alloc_clusters_noref() stores the cluster index in a uint64_t. However, offsets are often represented as int64_t (as for example the return value of alloc_clusters_noref() itself demonstrates). Therefore, we should make sure all offsets in the allocated range of clusters are representable using int64_t without overflows. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | block: Use error_abort in bdrv_image_info_specific_dump()Max Reitz2014-04-301-2/+1Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, bdrv_image_info_specific_dump() uses an error variable for visit_type_ImageInfoSpecific, but ignores the result. As this function is used here with an output visitor to transform the ImageInfoSpecific object to a generic QDict, an error should actually be impossible. It is however better to assert that this is indeed the case. This is done by this patch using error_abort instead of an unused local Error variable. Signed-off-by: Max Reitz <mreitz@redhat.com> Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | block: Fix open_flags in bdrv_reopen()Kevin Wolf2014-04-301-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | Revert "block: another bdrv_append fix"Kevin Wolf2014-04-301-1/+0Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | block: Unlink temporary files in raw-posix/win32Kevin Wolf2014-04-305-28/+18Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | block: Remove BDRV_O_COPY_ON_READ for bs->fileKevin Wolf2014-04-301-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | block: Create bdrv_backing_flags()Kevin Wolf2014-04-301-6/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | block: Create bdrv_inherited_flags()Kevin Wolf2014-04-301-2/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | iotests: Discarding compressed clusters on qcow2Max Reitz2014-04-293-0/+74
| | | | | | | | | | | | | | | | | | | | | | | | Add a test which discards a compressed cluster on qcow2. This should work without any problems. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | qcow2: Fix discardMax Reitz2014-04-291-8/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | discard_single_l2() should not implement its own version of qcow2_get_cluster_type(), but rather rely on this already existing function. By doing so, it will work for compressed clusters as well (which it did not so far). Also, rename "old_offset" to "old_l2_entry", as both are quite different (and the value is indeed of the latter kind). Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | block: qemu-iotests: make test 019 and 086 work with spaced pathnamesJeff Cody2014-04-292-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Both tests 019 and 086 need proper quotations to work with pathnames that contain spaces. Reviewed-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | block: qemu-iotests - fix image cleanup when using spaced pathnamesJeff Cody2014-04-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The _rm_test_img() function in common.rc did not quote the image file, which left droppings in the scratch directory (and performed a potentially unsafe rm -f). This adds the necessary quotes. Reviewed-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | mirror: Check for bdrv_get_info resultFam Zheng2014-04-291-1/+4
| | | | | | | | | | | | | | | | | | | | | bdrv_get_info could fail. Add check before using the returned value. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| * | mirror: Fix resource leak when bdrv_getlength failsFam Zheng2014-04-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | The direct return will skip releasing of all the resouces at immediate_exit, don't miss that. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
generated by cgit v1.2.3-55-g7522 (git 2.39.0) at 2024-12-05 02:48:21 +0100