aboutsummaryrefslogtreecommitdiff
path: root/block.c
AgeCommit message (Collapse)Author
2015-07-29block: Initialize local_err in bdrv_append_temp_snapshotFam Zheng
Cc: qemu-stable@nongnu.org Signed-off-by: Fam Zheng <famz@redhat.com> Message-id: 1436156684-16526-1-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> (cherry picked from commit c2e0dbbfd7265eb9a7170ab195d8f9f8a1cbd1af) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
2015-07-29block: Fix dirty bitmap in bdrv_co_discardFam Zheng
Unsetting dirty globally with discard is not very correct. The discard may zero out sectors (depending on can_write_zeroes_with_unmap), we should replicate this change to destination side to make sure that the guest sees the same data. Calling bdrv_reset_dirty also troubles mirror job because the hbitmap iterator doesn't expect unsetting of bits after current position. So let's do it the opposite way which fixes both problems: set the dirty bits if we are to discard it. Reported-by: wangxiaolong@ucloud.cn Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> (cherry picked from commit 508249952c0ea7472c62e17bf8132295dab4912d) Conflicts: block/io.c * applied manually to avoid dependency on 61007b316 * squashed in 6e82e4b bdrv_reset_dirty() is static in 2.3.0 and becomes unused as of this patch Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
2015-07-29block: Add bdrv_get_block_status_aboveFam Zheng
Like bdrv_is_allocated_above, this function follows the backing chain until seeing BDRV_BLOCK_ALLOCATED. Base is not included. Reimplement bdrv_is_allocated on top. [Initialized bdrv_co_get_block_status_above() ret to 0 to silence mingw64 compiler warning about the unitialized variable. assert(bs != base) prevents that case but I suppose the program could be compiled with -DNDEBUG. --Stefan] Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> (cherry picked from commit ba3f0e2545c365ebe1dbddb0e53058710d41881e) Conflicts: block/io.c * applied manually to avoid dependency on 61007b316 Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
2015-07-29block: Fix NULL deference for unaligned write if qiov is NULLFam Zheng
For zero write, callers pass in NULL qiov (qemu-io "write -z" or scsi-disk "write same"). Commit fc3959e466 fixed bdrv_co_write_zeroes which is the common case for this bug, but it still exists in bdrv_aio_write_zeroes. A simpler fix would be in bdrv_co_do_pwritev which is the NULL dereference point and covers both cases. So don't access it in bdrv_co_do_pwritev in this case, use three aligned writes. [Initialize ret to 0 in bdrv_co_do_zero_pwritev() to avoid uninitialized variable warning with gcc 4.9.2. --Stefan] Signed-off-by: Fam Zheng <famz@redhat.com> Message-id: 1431522721-3266-3-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> (cherry picked from commit 9eeb6dd1b27bd57eb4e3869290e87feac8e8b226) Conflicts: block/io.c * moved hunks into corresponding location in block.c due to lack of 61007b316 in v2.3.0 Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
2015-07-29Revert "block: Fix unaligned zero write"Michael Roth
This reverts commit fc3959e4669a1c2149b91ccb05101cfc7ae1fc05. From upstream commit d01c07f: This reverts commit fc3959e4669a1c2149b91ccb05101cfc7ae1fc05. The core write code already handles the case, so remove this duplication. Because commit 61007b316 moved the touched code from block.c to block/io.c, the change is manually reverted. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> v2.3.0 does not contain 61007b316 so we can revert the change directly. Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
2015-03-27block: Fix unaligned zero writeFam Zheng
If the zero write is not aligned, bdrv_co_do_pwritev will segfault because of accessing to the NULL qiov passed in by bdrv_co_write_zeroes. Fix this by allocating a local qiov in bdrv_co_do_pwritev if the request is not aligned. (In this case the padding iovs are necessary anyway, so it doesn't hurt.) Also add a check at the end of bdrv_co_do_pwritev to clear the zero flag if padding is involved. Signed-off-by: Fam Zheng <famz@redhat.com> Message-id: 1427160230-4489-2-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-03-16block: Drop bdrv_findFam Zheng
All callers are converted, so drop it. Signed-off-by: Fam Zheng <famz@redhat.com> Message-id: 1425296209-1476-5-git-send-email-famz@redhat.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2015-03-16block: Deprecate QCOW/QCOW2 encryptionMarkus Armbruster
We've steered users away from QCOW/QCOW2 encryption for a while, because it's a flawed design (commit 136cd19 Describe flaws in qcow/qcow2 encryption in the docs). In addition to flawed crypto, we have comically bad usability, and plain old bugs. Let me show you. = Example images = I'm going to use a raw image as backing file, and two QCOW2 images, one encrypted, and one not: $ qemu-img create -f raw backing.img 4m Formatting 'backing.img', fmt=raw size=4194304 $ qemu-img create -f qcow2 -o encryption,backing_file=backing.img,backing_fmt=raw geheim.qcow2 4m Formatting 'geheim.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=on cluster_size=65536 lazy_refcounts=off $ qemu-img create -f qcow2 -o backing_file=backing.img,backing_fmt=raw normal.qcow2 4m Formatting 'normal.qcow2', fmt=qcow2 size=4194304 backing_file='backing.img' backing_fmt='raw' encryption=off cluster_size=65536 lazy_refcounts=off = Usability issues = == Confusing startup == When no image is encrypted, and you don't give -S, QEMU starts the guest immediately: $ qemu-system-x86_64 -nodefaults -display none -monitor stdio normal.qcow2 QEMU 2.2.50 monitor - type 'help' for more information (qemu) info status VM status: running But as soon as there's an encrypted image in play, the guest is *not* started, with no notification whatsoever: $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2 QEMU 2.2.50 monitor - type 'help' for more information (qemu) info status VM status: paused (prelaunch) If the user figured out that he needs to type "cont" to enter his keys, the confusion enters the next level: "cont" asks for at most *one* key. If more are needed, it then silently does nothing. The user has to type "cont" once per encrypted image: $ qemu-system-x86_64 -nodefaults -display none -monitor stdio -drive if=none,file=geheim.qcow2 -drive if=none,file=geheim.qcow2 QEMU 2.2.50 monitor - type 'help' for more information (qemu) info status VM status: paused (prelaunch) (qemu) c none0 (geheim.qcow2) is encrypted. Password: ****** (qemu) info status VM status: paused (prelaunch) (qemu) c none1 (geheim.qcow2) is encrypted. Password: ****** (qemu) info status VM status: running == Incorrect passwords not caught == All existing encryption schemes give you the GIGO treatment: garbage password in, garbage data out. Guests usually refuse to mount garbage, but other usage is prone to data loss. == Need to stop the guest to add an encrypted image == $ qemu-system-x86_64 -nodefaults -display none -monitor stdio QEMU 2.2.50 monitor - type 'help' for more information (qemu) info status VM status: running (qemu) drive_add "" if=none,file=geheim.qcow2 Guest must be stopped for opening of encrypted image (qemu) stop (qemu) drive_add "" if=none,file=geheim.qcow2 OK Commit c3adb58 added this restriction. Before, we could expose images lacking an encryption key to guests, with potentially catastrophic results. See also "Use without key is not always caught". = Bugs = == Use without key is not always caught == Encrypted images can be in an intermediate state "opened, but no key". The weird startup behavior and the need to stop the guest are there to ensure the guest isn't exposed to that state. But other things still are! * drive_backup $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2 QEMU 2.2.50 monitor - type 'help' for more information (qemu) drive_backup -f ide0-hd0 out.img raw Formatting 'out.img', fmt=raw size=4194304 I guess this writes encrypted data to raw image out.img. Good luck with figuring out how to decrypt that again. * commit $ qemu-system-x86_64 -nodefaults -display none -monitor stdio geheim.qcow2 QEMU 2.2.50 monitor - type 'help' for more information (qemu) commit ide0-hd0 I guess this writes encrypted data into the unencrypted raw backing image, effectively destroying it. == QMP device_add of usb-storage fails when it shouldn't == When the image is encrypted, device_add creates the device, defers actually attaching it to when the key becomes available, then fails. This is wrong. device_add must either create the device and succeed, or do nothing and fail. $ qemu-system-x86_64 -nodefaults -display none -usb -qmp stdio -drive if=none,id=foo,file=geheim.qcow2 {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}} { "execute": "qmp_capabilities" } {"return": {}} { "execute": "device_add", "arguments": { "driver": "usb-storage", "id": "bar", "drive": "foo" } } {"error": {"class": "DeviceEncrypted", "desc": "'foo' (geheim.qcow2) is encrypted"}} {"execute":"device_del","arguments": { "id": "bar" } } {"timestamp": {"seconds": 1426003440, "microseconds": 237181}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/bar/bar.0/legacy[0]"}} {"timestamp": {"seconds": 1426003440, "microseconds": 238231}, "event": "DEVICE_DELETED", "data": {"device": "bar", "path": "/machine/peripheral/bar"}} {"return": {}} This stuff is worse than useless, it's a trap for users. If people become sufficiently interested in encrypted images to contribute a cryptographically sane implementation for QCOW2 (or whatever other format), then rewriting the necessary support around it from scratch will likely be easier and yield better results than fixing up the existing mess. Let's deprecate the mess now, drop it after a grace period, and move on. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-03-10block: add bdrv functions for geometry and blocksizeEkaterina Tumanova
Add driver functions for geometry and blocksize detection Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1424087278-49393-2-git-send-email-tumanova@linux.vnet.ibm.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-02-26qemu-img: Suppress unhelpful extra errors in convert, amendMarkus Armbruster
img_convert() and img_amend() use qemu_opts_do_parse(), which reports errors with qerror_report_err(). Its error messages aren't helpful here, the caller reports one that actually makes sense. Reproducer: $ qemu-img convert -o backing_format=raw in.img out.img qemu-img: Invalid parameter 'backing_format' qemu-img: Invalid options for file format 'raw' To fix, propagate errors through qemu_opts_do_parse(). This lifts the error reporting into callers. Drop it from img_convert() and img_amend(), keep it in qemu_chr_parse_compat(), bdrv_img_create(). Since I'm touching qemu_opts_do_parse() anyway, write a function comment for it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2015-02-26QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix useMarkus Armbruster
qemu_opt_set() is a wrapper around qemu_opt_set() that reports the error with qerror_report_err(). Most of its users assume the function can't fail. Make them use qemu_opt_set_err() with &error_abort, so that should the assumption ever break, it'll break noisily. Just two users remain, in util/qemu-config.c. Switch them to qemu_opt_set_err() as well, then rename qemu_opt_set_err() to qemu_opt_set(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2015-02-26block: Suppress unhelpful extra errors in bdrv_img_create()Markus Armbruster
bdrv_img_create() uses qemu_opt_set(), which reports errors with qerror_report_err(). Its error messages aren't helpful here, the caller reports one that actually makes sense. I don't know how to trigger the error conditions, though. Switch to qemu_opt_set_err() to get rid of the unwanted messages. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2015-02-26QemuOpts: Convert qemu_opt_set_number() to Error, fix its useMarkus Armbruster
Return the Error object instead of reporting it with qerror_report_err(). Change callers that assume the function can't fail to pass &error_abort, so that should the assumption ever break, it'll break noisily. Turns out all callers outside its unit test assume that. We could drop the Error ** argument, but that would make the interface less regular, so don't. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2015-02-16block: Keep bdrv_check*_request()'s return valueMax Reitz
Do not throw away the value returned by bdrv_check_request() and bdrv_check_byte_request(). Fix up some coding style issues in the proximity of the affected hunks. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-id: 1423162705-32065-17-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-02-16block: Remove "growable" from BDSMax Reitz
Now that request clamping is done in the BlockBackend, the "growable" field can be removed from the BlockDriverState. All BDSs are now treated as being "growable" (that is, they are allowed to grow; they are not necessarily actually able to). Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1423162705-32065-16-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-02-16block: Add Error parameter to bdrv_find_protocol()Max Reitz
The argument given to bdrv_find_protocol() is just a file name, which makes it difficult for the caller to reconstruct what protocol bdrv_find_protocol() was hoping to find. This patch adds an Error parameter to that function to solve this issue. Suggested-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1423162705-32065-4-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-02-06block: Eliminate silly QERR_ macros used for encryption keysMarkus Armbruster
The QERR_ macros are leftovers from the days of "rich" error objects. They're used with error_set() and qerror_report(), and expand into the first *two* arguments. This trickiness has become pointless. Clean up QERR_DEVICE_ENCRYPTED and QERR_DEVICE_NOT_ENCRYPTED. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1422524221-8566-5-git-send-email-armbru@redhat.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2015-02-06block: New bdrv_add_key(), convert monitor to use itMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1422524221-8566-4-git-send-email-armbru@redhat.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2015-02-06block: introduce BDRV_REQUEST_MAX_SECTORSPeter Lieven
we check and adjust request sizes at several places with sometimes inconsistent checks or default values: INT_MAX INT_MAX >> BDRV_SECTOR_BITS UINT_MAX >> BDRV_SECTOR_BITS SIZE_MAX >> BDRV_SECTOR_BITS This patches introdocues a macro for the maximal allowed sectors per request and uses it at several places. Signed-off-by: Peter Lieven <pl@kamp.de> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-02-06block: add accounting for merged requestsPeter Lieven
Signed-off-by: Peter Lieven <pl@kamp.de> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-02-06block: change default for discard and write zeroes to INT_MAXPeter Lieven
do not trim requests if the driver does not supply a limit through BlockLimits. For write zeroes we still keep a limit for the unsupported path to avoid allocating a big bounce buffer. Suggested-by: Kevin Wolf <kwolf@redhat.com> Suggested-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Peter Lieven <pl@kamp.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-01-23block: remove unused variable in bdrv_commitJeff Cody
As Stefan pointed out, the variable 'filename' in bdrv_commit is unused, despite being maintained in previous patches. With this patch, get rid of the variable for good. Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-01-13block: Split BLOCK_OP_TYPE_COMMIT to BLOCK_OP_TYPE_COMMIT_{SOURCE, TARGET}Fam Zheng
Like BLOCK_OP_TYPE_BACKUP_SOURCE and BLOCK_OP_TYPE_BACKUP_TARGET, block-commit involves two asymmetric devices. This change is not user-visible (yet), because commit only works with device names. But once we enable backing reference in blockdev-add, or specifying node-name in block-commit command, we don't want the user to start two commit jobs on the same backing chain, which will corrupt things because of the final bdrv_swap. Before we have per category blockers, splitting this type is still better. [Resolved virtio-blk dataplane conflict by replacing BLOCK_OP_TYPE_COMMIT with both BLOCK_OP_TYPE_COMMIT_{SOURCE, TARGET}. They are safe since the block job runs in the same AioContext as the dataplane IOThread. --Stefan] Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-01-13block: limited request size in write zeroes unsupported pathPeter Lieven
If bs->bl.max_write_zeroes is large and we end up in the unsupported path we might allocate a lot of memory for the iovector and/or even generate an oversized requests. Fix this by limiting the request by the minimum of the reported maximum transfer size or 16MB (32768 sectors). Reported-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Peter Lieven <pl@kamp.de> Reviewed-by: Denis V. Lunev <den@openvz.org> Message-id: 1420457389-16332-1-git-send-email-pl@kamp.de Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-01-13block: fix spoiling all dirty bitmaps by mirror and migrationVladimir Sementsov-Ogievskiy
Mirror and migration use dirty bitmaps for their purposes, and since commit [block: per caller dirty bitmap] they use their own bitmaps, not the global one. But they use old functions bdrv_set_dirty and bdrv_reset_dirty, which change all dirty bitmaps. Named dirty bitmaps series by Fam and Snow are affected: mirroring and migration will spoil all (not related to this mirroring or migration) named dirty bitmaps. This patch fixes this by adding bdrv_set_dirty_bitmap and bdrv_reset_dirty_bitmap, which change concrete bitmap. Also, to prevent such mistakes in future, old functions bdrv_(set,reset)_dirty are made static, for internal block usage. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com> CC: John Snow <jsnow@redhat.com> CC: Fam Zheng <famz@redhat.com> CC: Denis V. Lunev <den@openvz.org> CC: Stefan Hajnoczi <stefanha@redhat.com> CC: Kevin Wolf <kwolf@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-id: 1417081246-3593-1-git-send-email-vsementsov@parallels.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2015-01-13block: Relative backing file for image creationMax Reitz
Relative backing filenames are always relative to the backed image's directory; the same applies to image creation. Therefore, if the backing file has to be opened for determining its size (in case the size has not been explicitly specified) its filename should be interpreted relative to the new image's base directory and not relative to qemu's working directory. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-01-13block: JSON filenames and relative backing filesMax Reitz
When using a relative backing file name, qemu needs to know the directory of the top image file. For JSON filenames, such a directory cannot be easily determined (e.g. how do you determine the directory of a qcow2 BDS directly on top of a quorum BDS?). Therefore, do not allow relative filenames for the backing file of BDSs only having a JSON filename. Furthermore, BDS::exact_filename should be used whenever possible. If BDS::filename is not equal to BDS::exact_filename, the former will always be a JSON object. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-01-13block: Get full backing filename from stringMax Reitz
Introduce bdrv_get_full_backing_filename_from_filename(), a function which takes the name of the backed file and a potentially relative backing filename to produce the full (absolute) backing filename. Use this function from bdrv_get_full_backing_filename(). Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-01-13block: do not allocate an iovec per read of a growable/zero_after_eof BDSPaolo Bonzini
Most reads do not go past the end of the file, and they can use the input QEMUIOVector instead of creating one. This removes the qemu_iovec_* functions from the profile. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-12block: Don't add trailing space in "Formating..." messageFam Zheng
Change the message printing code to output a separator for each option string before it instead of after, then we don't one more extra ' ' in the end. To update qemu-iotests output files, most of the times one would just copy the *.out.bad to *.out. With this change we will not have the space disliked by checkpatch.pl. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-id: 1418110684-19528-3-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-12-12vmdk: Fix error for JSON descriptor file namesMax Reitz
If vmdk blindly tries to use path_combine() using bs->file->filename as the base file name, this will result in a bad error message for JSON file names when calling bdrv_open(). It is better to only try bs->file->exact_filename; if that is empty, bs->file->filename will be useless for path_combine() and an error should be emitted (containing bs->file->filename because desc_file_path (which is bs->file->exact_filename) is empty). Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-id: 1417615043-26174-2-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-12-10block: Check create_opts before image creationMax Reitz
If a driver supports image creation, it needs to set the .create_opts field. We can use that to make sure .create_opts for both drivers involved is not NULL in bdrv_img_create(), which is important so that the create_opts pointer in that function is not NULL after the qemu_opts_append() calls and when going into qemu_opts_create(). Cc: qemu-stable@nongnu.org Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10block: Omit bdrv_find_format for essential driversMax Reitz
We can always assume raw, file and qcow2 being available; so do not use bdrv_find_format() to locate their BlockDriver objects but statically reference the respective objects. Cc: qemu-stable@nongnu.org Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10block: Don't probe for unknown backing file formatKevin Wolf
If a qcow2 image specifies a backing file format that doesn't correspond to any format driver that qemu knows, we shouldn't fall back to probing, but simply error out. Not looking up the backing file driver in bdrv_open_backing_file(), but just filling in the "driver" option if it isn't there moves us closer to the goal of having everything in QDict options and gets us the error handling of bdrv_open(), which correctly refuses unknown drivers. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1416935562-7760-4-git-send-email-kwolf@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10raw: Prohibit dangerous writes for probed imagesKevin Wolf
If the user neglects to specify the image format, QEMU probes the image to guess it automatically, for convenience. Relying on format probing is insecure for raw images (CVE-2008-2004). If the guest writes a suitable header to the device, the next probe will recognize a format chosen by the guest. A malicious guest can abuse this to gain access to host files, e.g. by crafting a QCOW2 header with backing file /etc/shadow. Commit 1e72d3b (April 2008) provided -drive parameter format to let users disable probing. Commit f965509 (March 2009) extended QCOW2 to optionally store the backing file format, to let users disable backing file probing. QED has had a flag to suppress probing since the beginning (2010), set whenever a raw backing file is assigned. All of these additions that allow to avoid format probing have to be specified explicitly. The default still allows the attack. In order to fix this, commit 79368c8 (July 2010) put probed raw images in a restricted mode, in which they wouldn't be able to overwrite the first few bytes of the image so that they would identify as a different image. If a write to the first sector would write one of the signatures of another driver, qemu would instead zero out the first four bytes. This patch was later reverted in commit 8b33d9e (September 2010) because it didn't get the handling of unaligned qiov members right. Today's block layer that is based on coroutines and has qiov utility functions makes it much easier to get this functionality right, so this patch implements it. The other differences of this patch to the old one are that it doesn't silently write something different than the guest requested by zeroing out some bytes (it fails the request instead) and that it doesn't maintain a list of signatures in the raw driver (it calls the usual probe function instead). Note that this change doesn't introduce new breakage for false positive cases where the guest legitimately writes data into the first sector that matches the signatures of an image format (e.g. for nested virt): These cases were broken before, only the failure mode changes from corruption after the next restart (when the wrong format is probed) to failing the problematic write request. Also note that like in the original patch, the restrictions only apply if the image format has been guessed by probing. Explicitly specifying a format allows guests to write anything they like. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1416497234-29880-8-git-send-email-kwolf@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10block: Read only one sector for format probingKevin Wolf
The only image format driver that even potentially accesses anything after 512 bytes in its bdrv_probe() implementation is VMDK, which reads a plain-text descriptor file. In practice, the field it's looking for seems to come first and will be well within the first 512 bytes, too. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1416497234-29880-7-git-send-email-kwolf@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10block: Factor bdrv_probe_all() out of find_image_format()Markus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1416497234-29880-6-git-send-email-kwolf@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10block: Add bdrv_get_node_nameFam Zheng
This returns the node name of a BDS. Remove the TODO comment and expect the callers to be explicit. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10block: Add bdrv_next_nodeFam Zheng
Similar to bdrv_next, this traverses through graph_bdrv_states. Will be useful to enumerate all the named nodes. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-11-14block: Fix max nb_sectors in bdrv_make_zeroFam Zheng
In bdrv_rw_co we report -EINVAL for nb_sectors > INT_MAX / BDRV_SECTOR_SIZE, so a caller shouldn't exceed it. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-id: 1415603264-21497-1-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-11Merge remote-tracking branch ↵Peter Maydell
'remotes/mjt/tags/pull-trivial-patches-2014-11-11' into staging trivial patches for 2014-11-11 # gpg: Signature made Tue 11 Nov 2014 14:38:39 GMT using RSA key ID A4C3D7DB # gpg: Good signature from "Michael Tokarev <mjt@tls.msk.ru>" # gpg: aka "Michael Tokarev <mjt@corpit.ru>" # gpg: aka "Michael Tokarev <mjt@debian.org>" * remotes/mjt/tags/pull-trivial-patches-2014-11-11: block: Fix comment for bdrv_co_get_block_status sysbus: Correct SYSTEM_BUS(obj) defines target-i386: cpu: keeping function parameters alignment on new line xen-hvm: Remove redundant variable 'xstate' coroutine-sigaltstack: Change jmp_buf to sigjmp_buf pc-bios: petalogix-s3adsp1800.dtb: Use 'xlnx, xps-ethernetlite-2.00.a' instead of 'xlnx, xps-ethernetlite-2.00.b' gdbstub: Add a missing case of signal number translation in gdbstub numa: make 'info numa' take into account hotplugged memory slirp/smbd: modify/set several parameters in generated smbd.conf qemu-doc.texi: fix typos in x509 examples icc_bus: fix typo ICC_BRIGDE -> ICC_BRIDGE Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2014-11-11block: Fix comment for bdrv_co_get_block_statusFam Zheng
It returns more information than binary, fix the comment. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2014-11-06block: Propagate error in bdrv_img_create()Max Reitz
If the specified backing file could not be opened, do not generate a new error message which contains the message which has been generated by bdrv_open(), but just propagate the latter. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Peter Lieven <pl@kamp.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-11-03block: let mirror blockjob run in BDS AioContextStefan Hajnoczi
The mirror block job must run in the BlockDriverState AioContext so that it works with dataplane. Acquire the AioContext in blockdev.c so starting the block job is safe. Note that to_replace is treated separately from other BlockDriverStates in that it does not need to be in the same AioContext. Explicitly acquire/release to_replace's AioContext when accessing it. The completion code in block/mirror.c must perform BDS graph manipulation and bdrv_reopen() from the main loop. Use block_job_defer_to_main_loop() to achieve that. The bdrv_drain_all() call is not allowed outside the main loop since it could lead to lock ordering problems. Use bdrv_drain(bs) instead because we have acquired the AioContext so nothing else can sneak in I/O. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1413889440-32577-10-git-send-email-stefanha@redhat.com
2014-11-03block: add bdrv_drain()Stefan Hajnoczi
Now that op blockers are in use, we can ensure that no other sources are generating I/O on a BlockDriverState. Therefore it is possible to drain requests for a single BDS. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 1413889440-32577-7-git-send-email-stefanha@redhat.com
2014-11-03block: Add status callback to bdrv_amend_options()Max Reitz
Depending on the changed options and the image format, bdrv_amend_options() may take a significant amount of time. In these cases, a way to be informed about the operation's status is desirable. Since the operation is rather complex and may fundamentally change the image, implementing it as AIO or a coroutine does not seem feasible. On the other hand, implementing it as a block job would be significantly more difficult than a simple callback and would not add benefits other than progress report to the amending operation, because it should not actually be run as a block job at all. A callback may not be very pretty, but it's very easy to implement and perfectly fits its purpose here. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Benoît Canet <benoit.canet@nodalink.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-id: 1414404776-4919-2-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03block.c: Fix type of IoOperationType variable in send_qmp_error_event()Peter Maydell
The local variable 'ac' in send_qmp_error_event() is declared with the wrong type, which causes clang to complain when it is initialized and again when it is used: block.c:3655:20: warning: implicit conversion from enumeration type 'enum IoOperationType' to different enumeration type 'BlockErrorAction' (aka 'enum BlockErrorAction') [-Wenum-conversion] ac = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE; ~ ^~~~~~~~~~~~~~~~~~~~~~ block.c:3655:45: warning: implicit conversion from enumeration type 'enum IoOperationType' to different enumeration type 'BlockErrorAction' (aka 'enum BlockErrorAction') [-Wenum-conversion] ac = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE; ~ ^~~~~~~~~~~~~~~~~~~~~~~ block.c:3656:62: warning: implicit conversion from enumeration type 'BlockErrorAction' (aka 'enum BlockErrorAction') to different enumeration type 'IoOperationType' (aka 'enum IoOperationType') [-Wenum-conversion] qapi_event_send_block_io_error(bdrv_get_device_name(bs), ac, action, ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~ Correct the type to IoOperationType, and rename the variable to 'optype' to match its correct type. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com> Message-id: 1412969583-21045-1-git-send-email-peter.maydell@linaro.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03block: avoid creating oversized writes in multiwrite_mergePeter Lieven
Signed-off-by: Peter Lieven <pl@kamp.de> Reviewed-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03BlockLimits: introduce max_transfer_lengthPeter Lieven
Signed-off-by: Peter Lieven <pl@kamp.de> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-10-23block: Respect underlying file's EOFMax Reitz
When falling through to the underlying file in bdrv_co_get_block_status(), if it returns that the query offset is beyond the file end (by setting *pnum to 0), return the range to be zero and do not let the number of sectors for which information could be obtained be overwritten. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-19 05:56:13 +0000