The immediately visible effect of this patch is that it fixes committing
a temporary snapshot to its backing file. Previously, it would fail with
a "permission denied" error because bdrv_inherited_flags() forced the
backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
The bigger problem this revealed is that the original open flags must
actually only be applied to the temporary snapshot, and the original
image file must be treated as a backing file of the temporary snapshot
and get the right flags for that.
Reported-by: Jan Kiszka <jan.kiszka@web.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
* remotes/qmp-unstable/queue/qmp: (38 commits)
Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()"
qapi: Document optional arguments' backwards compatibility
qmp: use valid JSON in transaction example
qmp: Don't use error_is_set() to suppress additional errors
dump: Drop pointless error_is_set(), DumpState member errp
qemu-option: Clean up fragile use of error_is_set()
qga: Drop superfluous error_is_set()
qga: Clean up fragile use of error_is_set()
qapi: Clean up fragile use of error_is_set()
tests/qapi-schema: Drop superfluous error_is_set()
qapi: Drop redundant, unclean error_is_set()
hmp: Guard against misuse of hmp_handle_error()
qga: Use return values instead of error_is_set(errp)
error: Consistently name Error ** objects errp, and not err
qmp: Consistently name Error ** objects errp, and not err
qga: Consistently name Error ** objects errp, and not err
qmp hmp: Consistently name Error * objects err, and not errp
pci-assign: assigned_initfn(): set monitor error in common error handler
pci-assign: propagate errors from assign_intx()
pci-assign: propagate errors from assign_device()
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This reverts commit 25a7017555.
Turns out the argument *can* be null: QEMU now segfaults if it
receives an invalid parameter via a qmp command instead of throwing an
error.
For example:
{ "execute": "blockdev-add",
"arguments": { "options" : { "driver": "invalid-driver" } } }
CC: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Our example should use the correct quotes to match what someone
could actually pass over the wire.
* qmp-commands.hx: Use correct JSON quotes.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Using error_is_set(errp) that way can sweep programming errors under
the carpet when we get called incorrectly with an error set.
encrypted_bdrv_it() does it, because there's no way to make
bdrv_iterate() break its loop. Actually safe, because qmp_cont()
clears the error before the loop. Clean it up anyway: replace
bdrv_iterate() by bdrv_next(), break the loop on error.
Replace both occurrences, for consistency.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
In qmp_dump_guest_memory(), the error must be clear on entry, and we
always bail out after setting it, directly or via dump_init().
Therefore, both error_is_set() are always false. Drop them.
DumpState member errp is now write-only. Drop it, too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Using error_is_set(ERRP) to find out whether to bail out due to
previous error 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).
The error_is_set(state->errp) in qemu_opts_from_qdict_1() is merely
fragile, because the callers never pass state argument with null
state->errp.
Make the code more robust and more obviously correct: test
*state->errp directly.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
acquire_privilege(), execute_async() and check_suspend_mode() do
nothing when called with an error set. Callers shouldn't do that, and
no caller does. Drop the superfluous tests.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
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).
The error_is_set(errp) in the guest agent command handler functions
are merely fragile, because all chall chains (do_qmp_dispatch() via
the generated marshalling functions) pass a non-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>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
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).
The error_is_set(errp) in do_qmp_dispatch() is merely fragile, because
the caller never 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>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
visit_type_TestStruct() does nothing when called with an error set.
Callers shouldn't do that, and no caller does. Drop the superfluous
test.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
do_qmp_dispatch()'s test for qmp_dispatch_check_obj() failure examines
both the return value and the error object. The latter part is
unclean; it works only when do_qmp_dispatch()'s caller passes a
non-null errp argument. That's the case, but it's not locally
obvious. Unclean.
Cleanup would be easy enough, but since the unclean code is also
redundant, let's just drop it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Null errp argument makes no sense. Assert it's not null, to make this
explicit, and guard against misuse. All current callers pass non-null
errp.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Using error_is_set(errp) to check whether a function call failed is
fragile: it breaks when errp is null. ga_get_fd_handle() and
guest_file_handle_add() don't return a useful value when they fail,
but that's just stupid. Fix that, and check them instead. As far
as I can tell, errp can't be null there, but this is more robust and
more obviously correct.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This will return cluster_size and needs_compressed_writes to caller, if all the
extents have the same value (or there's only one extent). Otherwise return
-ENOTSUP.
cluster_size is only reported for sparse formats.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Add a wrapper function to support "compressed" path in qemu-img convert.
Only support streamOptimized subformat case for now (num_extents == 1
and extent compression is true).
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
If target block driver forces compression, qemu-img convert needs to
write by cluster size as well as "-c" option.
Particularly, this applies for converting to VMDK streamOptimized
format.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
After the URL has been parsed make sure the server part is valid in
order to avoid a segmentation fault when calling nfs_mount().
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The help message for qemu-img lists the supported block formats, of
which there are 27 as of version 2.0.50. The formats are printed in
the order of their driver's position in a linked list, which appears
random. This patch prints the formats in sorted order, making it
easier to read and to find a specific format in the list.
[Added suggestions from Fam Zheng <famz@redhat.com> to declare variables
at the top of the scope in help() and to omit explicit cast for void*
opaque.
--Stefan]
Signed-off-by: Mike Day <ncmike@ncultra.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Currently, QEMU's iotests rely on /usr/bin/env to start the correct
Python (that is, at least Python 2.4, but not 3). On systems where
Python 3 is the default, the user has no clean way of making the iotests
use the correct binary.
This commit makes the iotests use the Python selected by configure.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
If the very first allocation has a length of 0, the free_cluster_index
is still 0 after the for loop, which means that subtracting one from it
will underflow and signal an invalid range of clusters by returning
-EFBIG. However, there is no such range, as its length is 0.
Fix this by preventing underflows on free_cluster_index during the
check.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Among the callers, only assigned_initfn() should set the monitor's stored
error. Other callers may run in contexts where the monitor's stored error
makes no sense. For example:
assigned_dev_pci_write_config()
assigned_dev_update_msix()
assign_intx()
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Also, change the return type to "void"; the function is static (with a
sole caller) and the negative errno values are not distinguished from each
other.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
The return type is also changed from "int" to "void", because it was used
in a success vs. failure sense only (the caller didn't distinguish error
codes from each other, and even assigned_dev_register_msix_mmio() masked
mmap()'s errno values with a common -EFAULT).
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Propagate any errors while adding PCI capabilities to
assigned_device_pci_cap_init(). We'll continue the propagation upwards
when assigned_device_pci_cap_init() becomes a leaf itself (when none of
its callees will report errors internally any longer when detecting and
returning them).
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
... and rebase pci_add_capability() to it.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Rename check_irqchip_in_kernel() to verify_irqchip_in_kernel(), so that
the name reflects our expectation better. Rather than returning a bool,
make it do nothing or set an Error.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
get_real_id() has two thin wrappers (and no other callers),
get_real_vendor_id() and get_real_device_id(); it's easiest to convert
them in one fell swoop.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This allows us to report the entire error with one error_report() call,
easing future error propagation.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Propagate any errors in monitor fd handling up to get_real_device(), and
report them there. We'll continue the propagation upwards when
get_real_device() becomes a leaf itself (when none of its callees will
report errors internally any longer when detecting and returning an
error).
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
eviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
and rebase monitor_handle_fd_param() to it. (Note that this will slightly
change the behavior when the qemu_parse_fd() branch is selected and it
fails: we now report (and in case of QMP, set) the error immediately,
rather than allowing the caller to set its own error message (if any)).
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
qemu_parse_fd() used to handle at least the following strings incorrectly:
o "-2": simply let through
o "2147483648": returned as LONG_MAX==INT_MAX on ILP32 (with ERANGE
ignored); implementation-defined behavior on LP64
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
strtosz_suffix() might return negative error, this patch fixes
the error handling.
This patch also changes to handle error in the if statement
rather than handle success specially, this will make this use
of strtosz_suffix consistent with all other uses.
Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
The primitive uses JSON syntax, and include paths are relative to the file using the directive:
{ 'include': 'path/to/file.json' }
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Use an explicit input file on the command-line instead of reading from standard
input.
It also outputs the proper file name when there's an error.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Current Makefile system allows using foo.o-cflags variables to store
object-specific CFLAGS. Convert some usages of old syntax
(using QEMU_CFLAGS += construct) to the new syntax.
Do not touch multifile modules for now, as build system isn't ready for this.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
