aboutsummaryrefslogtreecommitdiff
path: root/qemu-img.c
AgeCommit message (Collapse)Author
2015-04-28qemu-img convert: Rewrite copying logicKevin Wolf
The implementation of qemu-img convert is (a) messy, (b) buggy, and (c) less efficient than possible. The changes required to beat some sense into it are massive enough that incremental changes would only make my and the reviewers' life harder. So throw it away and reimplement it from scratch. Let me give some examples what I mean by messy, buggy and inefficient: (a) The copying logic of qemu-img convert has two separate branches for compressed and normal target images, which roughly do the same - except for a little code that handles actual differences between compressed and uncompressed images, and much more code that implements just a different set of optimisations and bugs. This is unnecessary code duplication, and makes the code for compressed output (unsurprisingly) suffer from bitrot. The code for uncompressed ouput is run twice to count the the total length for the progress bar. In the first run it just takes a shortcut and runs only half the loop, and when it's done, it toggles a boolean, jumps out of the loop with a backwards goto and starts over. Works, but pretty is something different. (b) Converting while keeping a backing file (-B option) is broken in several ways. This includes not writing to the image file if the input has zero clusters or data filled with zeros (ignoring that the backing file will be visible instead). It also doesn't correctly limit every iteration of the copy loop to sectors of the same status so that too many sectors may be copied to in the target image. For -B this gives an unexpected result, for other images it just does more work than necessary. Conversion with a compressed target completely ignores any target backing file. (c) qemu-img convert skips reading and writing an area if it knows from metadata that copying isn't needed (except for the bug mentioned above that ignores a status change in some cases). It does, however, read from the source even if it knows that it will read zeros, and then search for non-zero bytes in the read buffer, if it's possible that a write might be needed. This reimplementation of the copying core reorganises the code to remove the duplication and have a much more obvious code flow, by essentially splitting the copy iteration loop into three parts: 1. Find the number of contiguous sectors of the same status at the current offset (This can also be called in a separate loop before the copying loop in order to determine the total sectors for the progress bar.) 2. Read sectors. If the status implies that there is no data there to read (zero or unallocated cluster), don't do anything. 3. Write sectors depending on the status. If it's data, write it. If we want the backing file to be visible (with -B), don't write it. If it's zeroed, skip it if you can, otherwise use bdrv_write_zeroes() to optimise the write at least where possible. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2015-03-16qemu-img: Avoid qerror_report_err() outside QMP handlers, againMarkus Armbruster
qerror_report_err() is a transitional interface to help with converting existing monitor commands to QMP. It should not be used elsewhere. Replace by error_report_err(). Commit 6936f29 cleaned that up in qemu-img.c, but two calls have crept in since. Take care of them the same way. 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-16qemu-img: Fix convert, amend error messages for unknown optionsMarkus Armbruster
Message quality regressed in commit dc523cd. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-03-10fix GCC 5.0.0 logical-not-parentheses warningsRadim Krčmář
man gcc: Warn about logical not used on the left hand side operand of a comparison. This option does not warn if the RHS operand is of a boolean type. By preferring bool over int where sensible, but without modifying any depending code, make GCC happy in cases like this, qemu-img.c: In function ‘compare_sectors’: qemu-img.c:992:39: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses] if (!!memcmp(buf1, buf2, 512) != res) { hw/ide/core.c:1836 doesn't throw an error, assert(!!s->error == !!(s->status & ERR_STAT)); even thought the second operand is int (and first hunk of this patch has a very similar case), maybe GCC developers still have a little faith in C programmers. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2015-03-10block: remove superfluous '\n' around error_report/error_setgGonglei
Signed-off-by: Gonglei <arei.gonglei@huawei.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
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-26qemu-img: Suppress unhelpful extra errors in convert, resizeMarkus Armbruster
add_old_style_options() for img_convert() and img_resize() use 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. Reproducer: $ qemu-img convert -B raw in.img out.img qemu-img: Invalid parameter 'backing_file' qemu-img: Backing file not supported for file format 'raw' 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-26Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2015-02-18' ↵Peter Maydell
into staging Clean up around error_get_pretty(), qerror_report_err() # gpg: Signature made Wed Feb 18 10:10:07 2015 GMT using RSA key ID EB918653 # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" * remotes/armbru/tags/pull-error-2015-02-18: qemu-char: Avoid qerror_report_err() outside QMP command handlers qemu-img: Avoid qerror_report_err() outside QMP command handlers vl: Avoid qerror_report_err() outside QMP command handlers tpm: Avoid qerror_report_err() outside QMP command handlers numa: Avoid qerror_report_err() outside QMP command handlers net: Avoid qerror_report_err() outside QMP command handlers monitor: Avoid qerror_report_err() outside QMP command handlers monitor: Clean up around monitor_handle_fd_param() error: Use error_report_err() where appropriate error: New convenience function error_report_err() vhost-scsi: Improve error reporting for invalid vhostfd Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-02-18qemu-img: Avoid qerror_report_err() outside QMP command handlersMarkus Armbruster
qerror_report_err() is a transitional interface to help with converting existing monitor commands to QMP. It should not be used elsewhere. Replace by error_report_err(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2015-02-18error: Use error_report_err() where appropriateMarkus Armbruster
Coccinelle semantic patch: @@ expression E; @@ - error_report("%s", error_get_pretty(E)); - error_free(E); + error_report_err(E); @@ expression E, S; @@ - error_report("%s", error_get_pretty(E)); + error_report_err(E); ( exit(S); | abort(); ) Trivial manual touch-ups in block/sheepdog.c. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2015-02-16qemu-img: Use BlockBackend as far as possibleMax Reitz
Although qemu-img already creates BlockBackends, it does not do accesses to the images through them. This patch converts all of the bdrv_* calls for which this is currently possible to blk_* calls. Most of the remaining calls will probably stay bdrv_* calls because they really do operate on the BDS level instead of the BB level. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1423162705-32065-10-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-02-16qemu-img: Use blk_new_open() in img_rebase()Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1423162705-32065-9-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-02-16qemu-img: Use blk_new_open() in img_open()Max Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1423162705-32065-8-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-16qemu-img: Fix qemu-img convert -nMax Reitz
If -n is specified, it does not matter whether the output format and protocol support image creation; building the creation options should simply be skipped. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1423666727-20777-2-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-02-06qemu-img: Add QEMU_PKGVERSION to QEMU_IMG_VERSIONDon Slutz
This is the same way vl.c handles this. Signed-off-by: Don Slutz <dslutz@verizon.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2015-01-23block: update string sizes for filename,backing_file,exact_filenameJeff Cody
The string field entries 'filename', 'backing_file', and 'exact_filename' in the BlockDriverState struct are defined as 1024 bytes. However, many places that use these values accept a maximum of PATH_MAX bytes, so we have a mixture of 1024 byte and PATH_MAX byte allocations. This patch makes the BlockDriverStruct field string sizes match usage. This patch also does a few fixes related to the size that needs to happen now: * the block qapi driver is updated to use PATH_MAX bytes * the qcow and qcow2 drivers have an additional safety check * the block vvfat driver is updated to use PATH_MAX bytes for the size of backing_file, for systems where PATH_MAX is < 1024 bytes. * qemu-img uses PATH_MAX rather than 1024. These instances were not changed to be dynamically allocated, however, as the extra temporary 3K in stack usage for qemu-img does not seem worrisome. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-12-10qemu-img: Check create_opts before image amendmentMax Reitz
The image options which can be amended are described by the .create_opts field for every driver. This field must therefore be non-NULL so that anything can be amended in the first place. Check that this holds true before going into qemu_opts_create() (because if .create_opts is NULL, the create_opts pointer in img_amend() will be NULL after qemu_opts_append()). 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-10qemu-img: 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 for the target image in qemu-img convert, which is important so that the create_opts pointer in img_convert() 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-11-06qemu-img: Omit error_report() after img_open()Max Reitz
img_open() already prints an error if the operation failed, so there should not be another error_report() afterwards. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-11-03qemu-img: Fix insignificant memleakMax Reitz
As soon as options is set in img_amend(), it needs to be freed before the function returns. This leak is rather insignificant, as qemu-img will exit subsequently anyway, but there's no point in not fixing it. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Benoît Canet <benoit.canet@nodalink.com> Message-id: 1414404776-4919-4-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03qemu-img: Add progress output for amendMax Reitz
Now that bdrv_amend_options() supports a status callback, use it to display a progress report. 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-3-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <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-03qemu-img: Specify backing file for commitMax Reitz
Introduce a new parameter for qemu-img commit which may be used to explicitly specify the backing file into which an image should be committed if the backing chain has more than a single layer. [Applied Eric Blake's qemu-img.texi documentation rewording --Stefan] Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1414159063-25977-12-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03qemu-img: Enable progress output for commitMax Reitz
Implement progress output for the commit command by querying the progress of the block job. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-id: 1414159063-25977-11-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03qemu-img: Empty image after commitMax Reitz
After the top image has been committed, it should be emptied unless specified otherwise. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-id: 1414159063-25977-10-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-11-03qemu-img: Implement commit like QMPMax Reitz
qemu-img should use QMP commands whenever possible in order to ensure feature completeness of both online and offline image operations. As qemu-img itself has no access to QMP (since this would basically require just everything being linked into qemu-img), imitate QMP's implementation of block-commit by using commit_active_start() and then waiting for the block job to finish. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1414159063-25977-9-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-10-23qemu-img: Print error if check failedMax Reitz
Currently, if bdrv_check() fails either by returning -errno or having check_errors set, qemu-img check just exits with 1 after having told the user that there were no errors on the image. This is bad. Instead of printing the check result if there were internal errors which were so bad that bdrv_check() could not even complete with 0 as a return value, qemu-img check should inform the user about the error. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
2014-10-20block: Make BlockBackend own its BlockDriverStateMarkus Armbruster
On BlockBackend destruction, unref its BlockDriverState. Replaces the callers' unrefs. This turns the pointer from BlockBackend to BlockDriverState into a strong reference, managed with bdrv_ref() / bdrv_unref(). The back-pointer remains weak. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-10-20block: Connect BlockBackend to BlockDriverStateMarkus Armbruster
Convenience function blk_new_with_bs() creates a BlockBackend with its BlockDriverState. Callers have to unref both. The commit after next will relieve them of the need to unref the BlockDriverState. Complication: due to the silly way drive_del works, we need a way to hide a BlockBackend, just like bdrv_make_anon(). To emphasize its "special" status, give the function a suitably off-putting name: blk_hide_on_behalf_of_do_drive_del(). Unfortunately, hiding turns the BlockBackend's name into the empty string. Can't avoid that without breaking the blk->bs->device_name equals blk->name invariant. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-10-20block: New BlockBackendMarkus Armbruster
A block device consists of a frontend device model and a backend. A block backend has a tree of block drivers doing the actual work. The tree is managed by the block layer. We currently use a single abstraction BlockDriverState both for tree nodes and the backend as a whole. Drawbacks: * Its API includes both stuff that makes sense only at the block backend level (root of the tree) and stuff that's only for use within the block layer. This makes the API bigger and more complex than necessary. Moreover, it's not obvious which interfaces are meant for device models, and which really aren't. * Since device models keep a reference to their backend, the backend object can't just be destroyed. But for media change, we need to replace the tree. Our solution is to make the BlockDriverState generic, with actual driver state in a separate object, pointed to by member opaque. That lets us replace the tree by deinitializing and reinitializing its root. This special need of the root makes the data structure awkward everywhere in the tree. The general plan is to separate the APIs into "block backend", for use by device models, monitor and whatever other code dealing with block backends, and "block driver", for use by the block layer and whatever other code (if any) dealing with trees and tree nodes. Code dealing with block backends, device models in particular, should become completely oblivious of BlockDriverState. This should let us clean up both APIs, and the tree data structures. This commit is a first step. It creates a minimal "block backend" API: type BlockBackend and functions to create, destroy and find them. BlockBackend objects are created and destroyed exactly when root BlockDriverState objects are created and destroyed. "Root" in the sense of "in bdrv_states". They're not yet used for anything; that'll come shortly. A root BlockDriverState is created with bdrv_new_root(), so where to create a BlockBackend is obvious. Where these roots get destroyed isn't always as obvious. It is obvious in qemu-img.c, qemu-io.c and qemu-nbd.c, and in error paths of blockdev_init(), blk_connect(). That leaves destruction of objects successfully created by blockdev_init() and blk_connect(). blockdev_init() is used only by drive_new() and qmp_blockdev_add(). Objects created by the latter are currently indestructible (see commit 48f364d "blockdev: Refuse to drive_del something added with blockdev-add" and commit 2d246f0 "blockdev: Introduce DriveInfo.enable_auto_del"). Objects created by the former get destroyed by drive_del(). Objects created by blk_connect() get destroyed by blk_disconnect(). BlockBackend is reference-counted. Its reference count never exceeds one so far, but that's going to change. In drive_del(), the BB's reference count is surely one now. The BDS's reference count is greater than one when something else is holding a reference, such as a block job. In this case, the BB is destroyed right away, but the BDS lives on until all extra references get dropped. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-10-20block: Split bdrv_new_root() off bdrv_new()Markus Armbruster
Creating an anonymous BDS can't fail. Make that obvious. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Benoît Canet <benoit.canet@nodalink.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-10-03block: Drop superfluous conditionals around qemu_opts_del()Markus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1411999675-14533-1-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-09-25block: Validate node-nameKevin Wolf
The device_name of a BlockDriverState is currently checked because it is always used as a QemuOpts ID and qemu_opts_create() checks whether such IDs are wellformed. node-name is supposed to share the same namespace, but it isn't checked currently. This patch adds explicit checks both for device_name and node-name so that the same rules will still apply even if QemuOpts won't be used any more at some point. qemu-img used to use names with spaces in them, which isn't allowed any more. Replace them with underscores. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-09-22async: aio_context_new(): Handle event_notifier_init failureChrysostomos Nanakos
On a system with a low limit of open files the initialization of the event notifier could fail and QEMU exits without printing any error information to the user. The problem can be easily reproduced by enforcing a low limit of open files and start QEMU with enough I/O threads to hit this limit. The same problem raises, without the creation of I/O threads, while QEMU initializes the main event loop by enforcing an even lower limit of open files. This commit adds an error message on failure: # qemu [...] -object iothread,id=iothread0 -object iothread,id=iothread1 qemu: Failed to initialize event notifier: Too many open files in system Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-09-08qemu-img: clarify src_cache option documentationStefan Hajnoczi
The source cache option takes the same values as the cache option. The documentation reads a little strange because it starts with "In contrast the src_cache option ...". The fact that this is comparing with the previous documented option (the 'cache' option) is implicit. Readers may be confused, especially if they jump to src_cache without reading cache documentation first. Suggested-by: Jeff Nelson <jenelson@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-08-29qemu-img: always goto out in img_snapshot() error pathsStefan Hajnoczi
The out label has the qemu_progress_end() and other cleanup calls. Always goto out in error paths so the cleanup happens. These error paths now return 1 instead of -1. Note that bdrv_unref(NULL) is safe. We just need to initialize bs to NULL at the top of the function. We can now remove the obsolete bs_old_backing = NULL and bs_new_backing = NULL for safe mode. Originally it was necessary in commit 3e85c6fd ("qemu-img rebase") but became useless in commit c2abcce ("qemu-img: avoid calling exit(1) to release resources properly") because the variables are already initialized during declaration. Reported-by: John Snow <jsnow@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-08-29qemu-img: fix img_compare() flags error pathStefan Hajnoczi
If img_compare() fails to parse the cache flags the goto out3 code path will call qemu_progress_end(). Make sure we actually call qemu_progress_init() first. Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-08-29qemu-img: fix img_commit() error return valueStefan Hajnoczi
The img_commit() return value is a process exit code. Use 1 for failure instead of -1. The other failure paths in this function already use 1. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2014-08-28Revert "qemu-img: sort block formats in help message"Stefan Hajnoczi
This reverts commit 1a443c1b8b4314d365e82bddeb1de5b4b1c15fb3 and the later commit 395071a76328189f50c778f4dee6dabb90503dd9. GSequence was introduced in glib 2.14. RHEL 5 fails to compile since it uses glib 2.12.3. Now that bdrv_iterate_format() invokes the iteration callback in sorted order these commits are unnecessary. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
2014-08-22qemu-img: Allow cache mode specification for amendMax Reitz
qemu-img amend may extensively modify the target image, depending on the options to be amended (e.g. conversion to qcow2 compat level 0.10 from 1.1 for an image with many unallocated zero clusters). Therefore it makes sense to allow the user to specify the cache mode to be used. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-08-22qemu-img: Allow source cache mode specificationMax Reitz
Many qemu-img subcommands only read the source file(s) once. For these use cases, a full write-back cache is unnecessary and mainly clutters host cache memory. Though this is generally no concern as cache memory is freely available and can be scaled by the host OS, it may become a concern with thin provisioning. For these cases, it makes sense to allow users to freely specify the source cache mode (e.g. use no cache at all). This commit adds a new switch (-T) for the qemu-img subcommands check, compare, convert and rebase to specify the cache to be used for source images (the backing file in case of rebase). Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-08-15Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into stagingPeter Maydell
Block patches # gpg: Signature made Fri 15 Aug 2014 14:07:42 BST using RSA key ID C88F2FD6 # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" * remotes/kevin/tags/for-upstream: (59 commits) block: Catch !bs->drv in bdrv_check() iotests: Add test for image header overlap qcow2: Catch !*host_offset for data allocation qcow2: Return useful error code in refcount_init() mirror: Handle failure for potentially large allocations vpc: Handle failure for potentially large allocations vmdk: Handle failure for potentially large allocations vhdx: Handle failure for potentially large allocations vdi: Handle failure for potentially large allocations rbd: Handle failure for potentially large allocations raw-win32: Handle failure for potentially large allocations raw-posix: Handle failure for potentially large allocations qed: Handle failure for potentially large allocations qcow2: Handle failure for potentially large allocations qcow1: Handle failure for potentially large allocations parallels: Handle failure for potentially large allocations nfs: Handle failure for potentially large allocations iscsi: Handle failure for potentially large allocations dmg: Handle failure for potentially large allocations curl: Handle failure for potentially large allocations ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2014-08-15block: Avoid bdrv_get_geometry() where errors should be detectedMarkus Armbruster
bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or bdrv_getlength() instead where that's obviously inappropriate. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-08-15qemu-img: Make img_convert() get image size just once per imageMarkus Armbruster
Chiefly so I don't have to do the error checking in quadruplicate in the next commit. Moreover, replacing the frequently updated bs_sectors by an array assigned just once makes the code easier to understand. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-08-15block: Use bdrv_nb_sectors() in img_convert()Markus Armbruster
Instead of bdrv_getlength(). Replace variable output_length by output_sectors. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Benoit Canet <benoit@irqsave.net> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-08-09qemu-img: Check getchar() return value in read_password() for WIN32Chen Gang
getchar() is a standard c library function which may return with failure (e.g. -1), so like another platforms, also need check it under WIN32. And make the related code match current qemu code styles, too. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2014-07-18qemu-img: Remove redundancy "ret = -1"Chen Gang
In this case, 'ret' is already '-1', so need not do it again. Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>