Fam Zheng [Wed, 13 Jan 2016 08:37:41 +0000 (16:37 +0800)]
qemu-img: Speed up comparing empty/zero images
Two empty raw files are always compared by actually reading data even if
there is no data, because BDRV_BLOCK_ZERO is considered "allocated" in
bdrv_is_allocated_above(). That is inefficient.
Use bdrv_get_block_status_above() for more information, and skip the
consecutive zero sectors.
This brings a huge speed up in comparing sparse/empty raw images:
$ qemu-img create a 1G
$ time ~/build/master/bin/qemu-img compare a a
Images are identical.
block/raw-posix: avoid bogus fixup for cylinders on DASD disks
large volume DASD that have > 64k cylinders do claim to have
0xFFFE cylinders as special value in the old 16 bit field. We
want to pass this "token" along to the guest, instead of
calculating the real number. Otherwise qemu might fail with
"cyls must be between 1 and 65535"
Kevin Wolf [Mon, 11 Jan 2016 18:07:50 +0000 (19:07 +0100)]
block: Fix .bdrv_open flags
bdrv_common_open() modified bs->open_flags after inferring the set of
options to pass to the driver's .bdrv_open callback. This means that the
cache options were correctly set in bs->open_flags (and therefore
correctly displayed in 'info block'), but the image would actually be
opened with the default cache mode instead.
This patch removes the flags parameter to bdrv_common_open() (except for
BDRV_O_NO_BACKING it's the same as bs->open_flags anyway, and having two
names for the same thing is confusing), and moves the assignment of
open_flags down to immediately before calling into the block drivers. In
all other places, bs->open_flags is now used consistently.
Andreas Färber [Mon, 18 Jan 2016 17:19:35 +0000 (18:19 +0100)]
MAINTAINERS: Fix sPAPR entry heading
get_maintainers.pl does not handle parenthesis in maintenance areas well
in connection with list emails (here: [email protected]).
Resolve a recurring CC issue breaking git-send-email by reverting part
of commit 085eb217dfb3ee12e7985c11f71f8a038394735a ("Add David Gibson
for sPAPR in MAINTAINERS file").
Paolo Bonzini [Mon, 19 Oct 2015 11:11:39 +0000 (13:11 +0200)]
qdev: Free QemuOpts when the QOM path goes away
Otherwise there is a race where the DEVICE_DELETED event has been sent but
attempts to reuse the ID will fail.
Note that similar races exist for other QemuOpts, which this patch
does not attempt to fix.
For example, if the device is a block device, then unplugging it also
deletes its backend. However, this backend's get deleted in
drive_info_del(), which is only called when properties are
destroyed. Just like device_finalize(), drive_info_del() is called
some time after DEVICE_DELETED is sent. A separate patch series has
been sent to plug this other bug. Character devices also have yet to
be fixed.
Currently the ObjectProperty iterator API works as follows:
ObjectPropertyIterator *iter;
iter = object_property_iter_init(obj);
while ((prop = object_property_iter_next(iter))) {
...
}
object_property_iter_free(iter);
This has the benefit that the ObjectPropertyIterator struct
can be opaque, but has the downside that callers need to
explicitly call a free function. It is also not in keeping
with iterator style used elsewhere in QEMU/GLib2.
This patch changes the API to use stack allocation instead:
ObjectPropertyIterator iter;
object_property_iter_init(&iter, obj);
while ((prop = object_property_iter_next(&iter))) {
...
}
qom: Allow properties to be registered against classes
When there are many instances of a given class, registering
properties against the instance is wasteful of resources. The
majority of objects have a statically defined list of possible
properties, so most of the properties are easily registerable
against the class. Only those properties which are conditionally
registered at runtime need be recorded against the klass.
Registering properties against classes also makes it possible
to provide static introspection of QOM - currently introspection
is only possible after creating an instance of a class, which
severely limits its usefulness.
This impl only supports simple scalar properties. It does not
attempt to allow child object / link object properties against
the class. There are ways to support those too, but it would
make this patch more complicated, so it is left as an exercise
for the future.
There is no equivalent to object_property_del() provided, since
classes must be immutable once they are defined.
Peter Maydell [Mon, 7 Dec 2015 16:23:43 +0000 (16:23 +0000)]
scripts: Add new clean-includes script to fix C include directives
Add a new scripts/clean-includes, which can be used to automatically
ensure that a C source file includes qemu/osdep.h first and doesn't
then include any headers which osdep.h provides already.
Similarly to the commit 764eb39d1b6 fixing VNC+SASL+QXL, when starting
QEMU with SPICE but no SASL, and at the same time VNC with SASL, then
spice_server_init() will get called without a previous call to
spice_server_set_sasl_appname(), which will cause cyrus-sasl to
try to use /etc/sasl2/spice.conf (spice-server uses "spice" as its
default appname) rather than the expected /etc/sasl2/qemu.conf.
This commit unconditionally calls spice_server_set_sasl_appname()
before calling spice_server_init() in order to use the correct appname
even if SPICE without SASL was requested on qemu command line.
This pointer should be cleared in vnc_display_close()
otherwise a use-after-free can happen when when using the
old style 'x509' and 'tls' options rather than a persistent
tls-creds -object, by issuing monitor commands to change
the vnc server like so:
Start with: -vnc unix:test.socket,x509,tls
Then use the following monitor command:
change vnc unix:test.socket
After this the pointer is still set but invalid and a crash
can be triggered for instance by issuing the same command a
second time which will try to object_unparent() the same
pointer again.
Paolo Bonzini [Thu, 17 Dec 2015 12:47:02 +0000 (13:47 +0100)]
gtk: implement set_echo
Even without line editing, this makes -qmp vc more pleasant with the
GTK+ backend. The only issue is that set_echo is invoked very early,
long before a vc is actually associated with a VirtualConsole. To work
around this, create a temporary VirtualConsole until then.
Peter Maydell [Mon, 18 Jan 2016 09:33:36 +0000 (09:33 +0000)]
Merge remote-tracking branch 'remotes/mcayland/tags/qemu-sparc-signed' into staging
qemu-sparc update
# gpg: Signature made Sat 16 Jan 2016 12:32:06 GMT using RSA key ID AE0F321F
# gpg: Good signature from "Mark Cave-Ayland <[email protected]>"
* remotes/mcayland/tags/qemu-sparc-signed:
target-sparc: Migrate CWP and PIL for SPARC64
target-sparc: Use VMState arrays for SPARC64 TLB/MMU state
target-sparc: Convert to VMStateDescription
target-sparc: Don't flush TLB in cpu_load function
target-sparc: Split cpu_put_psr into side-effect and no-side-effect parts
vmstate: define vmstate_info_uinttl
vmstate: Introduce VMSTATE_VARRAY_MULTPLY
vmstate: introduce CPU_DoubleU arrays
Peter Maydell [Mon, 11 Jan 2016 12:40:28 +0000 (12:40 +0000)]
target-sparc: Migrate CWP and PIL for SPARC64
In SPARC32 the env->cwp and env->psrpil state is part of the PSR
register, and gets migrated as part of that register.
In SPARC64 this state is in separate CWP and PIL registers, but we
were not doing anything to migrate those.
Add the missing fields to the migration vmstate (which is a
migration break, but without these fields migration is completely
broken anyway).
This change means that trying a save/load of a SPARC64 target at
the boot rom prompt now produces a system which at least responds
to keyboard input after the restore.
Peter Maydell [Mon, 11 Jan 2016 12:40:27 +0000 (12:40 +0000)]
target-sparc: Use VMState arrays for SPARC64 TLB/MMU state
Use VMState arrays for SPARC64 TLB/MMU state. This is
a migration-break for SPARC64 (but not for SPARC32),
which is acceptable because currently migration does not
work for any SPARC64 machines due to the lack of any migration
of interrupt controller state.
Juan Quintela [Mon, 11 Jan 2016 12:40:26 +0000 (12:40 +0000)]
target-sparc: Convert to VMStateDescription
Convert the SPARC CPU from cpu_load/save functions to VMStateDescription.
We preserve migration compatibility with the previous version
(required for SPARC32 but not necessarily for SPARC64).
Signed-off-by: Juan Quintela <[email protected]>
[PMM:
* Rebase and update to apply to master
* VMSTATE_STRUCT_POINTER now takes type, not pointer-to-type
* QEMUTimer* are migrated via VMSTATE_TIMER_PTR
* Put CPUTimer vmstate struct inside TARGET_SPARC64 ifdef
* Convert handling of PSR to use a vmstate_psr, like Alpha and ARM
] Signed-off-by: Peter Maydell <[email protected]> Signed-off-by: Mark Cave-Ayland <[email protected]>
Peter Maydell [Mon, 11 Jan 2016 12:40:24 +0000 (12:40 +0000)]
target-sparc: Split cpu_put_psr into side-effect and no-side-effect parts
For inbound migration we really want to be able to set the PSR without
having any side effects, but cpu_put_psr() calls cpu_check_irqs() which
might try to deliver CPU interrupts. Split cpu_put_psr() into the
no-side-effect and side-effect parts.
This includes reordering the cpu_check_irqs() to the end of cpu_put_psr(),
because that function may actually end up calling cpu_interrupt(), which
does not seem like a good thing to happen in the middle of updating the PSR.
Juan Quintela [Mon, 11 Jan 2016 12:40:23 +0000 (12:40 +0000)]
vmstate: define vmstate_info_uinttl
We are going to define arrays of this type, so we need the integer type.
Signed-off-by: Juan Quintela <[email protected]>
[PMM: updated to apply on current QEMU; renamed to 'uinttl'
rather than 'uinttls' to match other vmstate naming] Signed-off-by: Peter Maydell <[email protected]> Signed-off-by: Mark Cave-Ayland <[email protected]>
Juan Quintela [Mon, 11 Jan 2016 12:40:21 +0000 (12:40 +0000)]
vmstate: introduce CPU_DoubleU arrays
Add vmstate support for migrating arrays of CPU_DoubleU via
VMSTATE_CPUDOUBLE_ARRAY.
Signed-off-by: Juan Quintela <[email protected]>
[PMM: rebased, since files have all moved since 2012;
added VMSTATE_CPUDOUBLE_ARRAY_V for consistency with FLOAT64] Signed-off-by: Peter Maydell <[email protected]> Signed-off-by: Mark Cave-Ayland <[email protected]>
# gpg: Signature made Fri 15 Jan 2016 17:58:28 GMT using RSA key ID 78C7AE83
# gpg: Good signature from "Paolo Bonzini <[email protected]>"
# gpg: aka "Paolo Bonzini <[email protected]>"
* remotes/bonzini/tags/for-upstream:
qemu-char: do not leak QemuMutex when freeing a character device
qemu-char: add logfile facility to all chardev backends
nbd-server: do not exit on failed memory allocation
nbd-server: do not check request length except for reads and writes
nbd-server: Coroutine based negotiation
nbd: Split nbd.c
nbd: Always call "close_fn" in nbd_client_new
SCSI device: fix to incomplete QOMify
iscsi: send readcapacity10 when readcapacity16 failed
qemu-char: delete send_all/recv_all helper methods
vmw_pvscsi: x-disable-pcie, x-old-pci-configuration back-compat props are 2.5 specific
scsi: initialise info object with appropriate size
i386: avoid null pointer dereference
target-i386: do not duplicate page protection checks
scsi: revert change to scsi_req_cancel_async and add assertions
qemu-char: add logfile facility to all chardev backends
Typically a UNIX guest OS will log boot messages to a serial
port in addition to any graphical console. An admin user
may also wish to use the serial port for an interactive
console. A virtualization management system may wish to
collect system boot messages by logging the serial port,
but also wish to allow admins interactive access.
Currently providing such a feature forces the mgmt app
to either provide 2 separate serial ports, one for
logging boot messages and one for interactive console
login, or to proxy all output via a separate service
that can multiplex the two needs onto one serial port.
While both are valid approaches, they each have their
own downsides. The former causes confusion and extra
setup work for VM admins creating disk images. The latter
places an extra burden to re-implement much of the QEMU
chardev backends logic in libvirt or even higher level
mgmt apps and adds extra hops in the data transfer path.
A simpler approach that is satisfactory for many use
cases is to allow the QEMU chardev backends to have a
"logfile" property associated with them.
This patch introduces a 'ChardevCommon' struct which
is setup as a base for all the ChardevBackend types.
Ideally this would be registered directly as a base
against ChardevBackend, rather than each type, but
the QAPI generator doesn't allow that since the
ChardevBackend is a non-discriminated union. The
ChardevCommon struct provides the optional 'logfile'
parameter, as well as 'logappend' which controls
whether QEMU truncates or appends (default truncate).
Paolo Bonzini [Thu, 7 Jan 2016 13:34:13 +0000 (14:34 +0100)]
nbd-server: do not exit on failed memory allocation
The amount of memory allocated in nbd_co_receive_request is driven by the
NBD client (possibly a virtual machine). Parallel I/O can cause the
server to allocate a large amount of memory; check for failures and
return ENOMEM in that case.
Paolo Bonzini [Thu, 7 Jan 2016 13:32:42 +0000 (14:32 +0100)]
nbd-server: do not check request length except for reads and writes
Only reads and writes need to allocate memory correspondent to the
request length. Other requests can be sent to the storage without
allocating any memory, and thus any request length is acceptable.
Fam Zheng [Thu, 14 Jan 2016 08:41:03 +0000 (16:41 +0800)]
nbd-server: Coroutine based negotiation
Create a coroutine in nbd_client_new, so that nbd_send_negotiate doesn't
need qemu_set_block().
Handlers need to be set temporarily for csock fd in case the coroutine
yields during I/O.
With this, if the other end disappears in the middle of the negotiation,
we don't block the whole event loop.
To make the code clearer, unify all function names that belong to
negotiate, so they are less likely to be misused. This is important
because we rely on negotiation staying in main loop, as commented in
nbd_negotiate_read/write().
Fam Zheng [Thu, 14 Jan 2016 08:41:01 +0000 (16:41 +0800)]
nbd: Always call "close_fn" in nbd_client_new
Rename the parameter "close" to "close_fn" to disambiguous with
close(2).
This unifies error handling paths of NBDClient allocation:
nbd_client_new will shutdown the socket and call the "close_fn" callback
if negotiation failed, so the caller don't need a different path than
the normal close.
The returned pointer is never used, make it void in preparation for the
next patch.
Zhu Lingshan [Tue, 29 Dec 2015 03:32:14 +0000 (11:32 +0800)]
iscsi: send readcapacity10 when readcapacity16 failed
When play with Dell MD3000 target, for sure it
is a TYPE_DISK, but readcapacity16 would fail.
Then we find that readcapacity10 succeeded. It
looks like the target just support readcapacity10
even through it is a TYPE_DISK or have some
TYPE_ROM characteristics.
This patch can give a chance to send
readcapacity16 when readcapacity10 failed.
This patch is not harmful to original pathes
The qemu-char.c contains two helper methods send_all
and recv_all. These are in fact declared in sockets.h
so ought to have been in util/qemu-sockets.c. For added
fun the impl of recv_all is completely missing on Win32.
Fortunately there is only a single caller of these
methods, the TPM passthrough code, which is only
ever compiled on Linux. With only a single caller
these helpers are not compelling enough to keep so
inline them in the TPM code, avoiding the need to
fix the missing recv_all on Win32.
P J P [Mon, 21 Dec 2015 09:43:13 +0000 (15:13 +0530)]
scsi: initialise info object with appropriate size
While processing controller 'CTRL_GET_INFO' command, the routine
'megasas_ctrl_get_info' overflows the '&info' object size. Use its
appropriate size to null initialise it.
P J P [Fri, 18 Dec 2015 06:05:07 +0000 (11:35 +0530)]
i386: avoid null pointer dereference
Hello,
A null pointer dereference issue was reported by Mr Ling Liu, CC'd here. It
occurs while doing I/O port write operations via hmp interface. In that,
'current_cpu' remains null as it is not called from cpu_exec loop, which
results in the said issue.
Below is a proposed (tested)patch to fix this issue; Does it look okay?
When I/O port write operation is called from hmp interface,
'current_cpu' remains null, as it is not called from cpu_exec()
loop. This leads to a null pointer dereference in vapic_write
routine. Add check to avoid it.
Paolo Bonzini [Tue, 17 Nov 2015 16:09:33 +0000 (17:09 +0100)]
target-i386: do not duplicate page protection checks
x86_cpu_handle_mmu_fault is currently checking twice for writability
and executability of pages; the first time to decide whether to
trigger a page fault, the second time to compute the "prot" argument
to tlb_set_page_with_attrs.
Reorganize code so that first "prot" is computed, then it is used
to check whether to raise a page fault, then finally PROT_WRITE is
removed if the D bit will have to be set.
Paolo Bonzini [Fri, 18 Dec 2015 08:54:53 +0000 (09:54 +0100)]
scsi: revert change to scsi_req_cancel_async and add assertions
Fam Zheng noticed that the change in commit 36896bf ("scsi: always call
notifier on async cancellation", 2015-12-16) could cause a leak of
the request; scsi_req_cancel_async now calls scsi_req_ref
multiple times for multiple cancellations, but there is only
one call to scsi_req_cancel_complete.
So revert the patch and instead assert that the problematic case (a call
to scsi_req_cancel_async after the aiocb has been completed) cannot
happen.
Peter Maydell [Fri, 15 Jan 2016 15:49:43 +0000 (15:49 +0000)]
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20160115' into staging
target-arm queue:
* use the right MMU index when handling unaligned accesses
* xlnx-zynqmp: Add support for high DDR memory regions
* target-arm: support QMP dump-guest-memory
* ARM: virt: Don't generate RTC ACPI device when using UEFI
* remotes/pmaydell/tags/pull-target-arm-20160115:
ARM: virt: Don't generate RTC ACPI device when using UEFI
target-arm: dump-guest-memory: add vfp notes for arm
elf: add arm note types
target-arm: dump-guest-memory: add prfpreg notes for aarch64
target-arm: support QMP dump-guest-memory
dump: allow target to set the physical base
dump: allow target to set the page size
dump: qemunotes aren't commonly needed
qapi-schema: dump-guest-memory: Improve text
xlnx-zynqmp: Add support for high DDR memory regions
target-arm: Use the right MMU index in arm_regime_using_lpae_format
Shannon Zhao [Fri, 15 Jan 2016 14:11:31 +0000 (22:11 +0800)]
ARM: virt: Don't generate RTC ACPI device when using UEFI
When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
While UEFI can use libfdt to disable the RTC device node in the DTB that
it passes to the OS, it cannot modify AML. Therefore, we won't generate
the RTC ACPI device at all when using UEFI.
Andrew Jones [Mon, 11 Jan 2016 19:56:25 +0000 (20:56 +0100)]
target-arm: dump-guest-memory: add vfp notes for arm
gdb won't actually dump these with 'info all-registers' since
it first tries to confirm that it should by checking the VFP
hwcap in the .auxv note. Well, we don't generate an .auxv note.
Andrew Jones [Mon, 11 Jan 2016 19:56:21 +0000 (20:56 +0100)]
dump: allow target to set the physical base
crash assumes the physical base in the kdump subheader of
makedumpfile formatted dumps is correct. Zero is not correct
for all architectures, so allow it to be changed.
Andrew Jones [Mon, 11 Jan 2016 19:56:20 +0000 (20:56 +0100)]
dump: allow target to set the page size
This is necessary for targets that don't have TARGET_PAGE_SIZE ==
real-target-page-size. The target should set the page size to the
correct one, if known, or, if not known, to the maximum page size
it supports.
Andrew Jones [Mon, 11 Jan 2016 19:56:19 +0000 (20:56 +0100)]
dump: qemunotes aren't commonly needed
Only one of three architectures implementing qmp-dump-guest-memory write
qemu notes. And, another architecture (arm/aarch64) is coming, which
won't use them either. Make the common implementation truly common.
Alistair Francis [Tue, 12 Jan 2016 22:39:18 +0000 (14:39 -0800)]
xlnx-zynqmp: Add support for high DDR memory regions
The Xilinx ZynqMP SoC and EP108 board supports three memory regions:
- A 2GB region starting at 0
- A 32GB region starting at 32GB
- A 256GB region starting at 768GB
This patch adds support for the first two memory regions, which is
automatically created based on the size specified by the QEMU memory
command line argument.
On hardware the physical memory region is one continuous region, it is then
mapped into the three different regions by the DDRC. As we don't model the
DDRC this is done at startup by QEMU. The board creates the memory region and
then passes that memory region to the SoC. The SoC then maps the memory
regions.
Alvise Rigo [Fri, 15 Jan 2016 10:37:42 +0000 (11:37 +0100)]
target-arm: Use the right MMU index in arm_regime_using_lpae_format
arm_regime_using_lpae_format checks whether the LPAE extension is used
for stage 1 translation regimes. MMU indexes not exclusively of a stage 1
regime won't work with this method.
In case of ARMMMUIdx_S12NSE0 or ARMMMUIdx_S12NSE1, offset these values
by ARMMMUIdx_S1NSE0 to get the right index indicating a stage 1
translation regime.
Rename also the function to arm_s1_regime_using_lpae_format and update
the comments to reflect the change.
Commit 8acc216b956 attempted to silence some sign-compare
warnings in libvixl by adding -Wno-sign-compare to the CFLAGS
for the relevant objects. Unfortunately it was ineffective
because it was placed before $(QEMU_CFLAGS), so the -Wall in
the general flags overrode -Wno-sign-compare rather than
vice-versa. Reorder the flags so the warning suppression works.
Thanks to Franz-Josef Haider <[email protected]>
for pointing out what was wrong with the original patch.
Peter Maydell [Thu, 14 Jan 2016 13:07:38 +0000 (13:07 +0000)]
Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2016-01-13' into staging
Error reporting patches for 2016-01-13
# gpg: Signature made Wed 13 Jan 2016 14:21:48 GMT using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <[email protected]>"
# gpg: aka "Markus Armbruster <[email protected]>"
* remotes/armbru/tags/pull-error-2016-01-13: (41 commits)
checkpatch: Detect newlines in error_report and other error functions
error: Consistently name Error * objects err, and not errp
s390/sclp: Simplify control flow in sclp_realize()
hw/s390x: Rename local variables Error *l_err to just err
error: Clean up errors with embedded newlines (again)
vhdx: Fix "log that needs to be replayed" error message
pci-assign: Clean up "Failed to assign" error messages
vmdk: Clean up "Invalid extent lines" error message
vmdk: Clean up control flow in vmdk_parse_extents() a bit
error: Strip trailing '\n' from error string arguments (again)
qemu-io qemu-nbd: Use error_report() etc. instead of fprintf()
migration: Use error_reportf_err() instead of monitor_printf()
spapr: Use error_reportf_err()
error: Use error_prepend() where it makes obvious sense
error: Use error_reportf_err() where it makes obvious sense
error: Don't decorate original error message when adding to it
error: New error_prepend(), error_reportf_err()
test-throttle: Simplify qemu_init_main_loop() error handling
qemu-nbd: Clean up "Failed to load snapshot" error message
block: Clean up "Could not create temporary overlay" error message
...
* remotes/amit-migration/tags/migration-for-2.6-1:
multithread decompression: Avoid one copy
Use qemu_get_buffer_in_place for xbzrle data
Migration: Emit event at start of pass
Postcopy: Send events/change state on incoming side
migration: Add state records for migration incoming
migration: Export migrate_set_state()
Jason J. Herne [Fri, 11 Dec 2015 18:30:42 +0000 (13:30 -0500)]
checkpatch: Detect newlines in error_report and other error functions
We don't want newlines embedded in error messages. This seems to be a common
problem with new code so let's try to catch it with checkpatch.
This will not catch cases where newlines are inserted into the middle of an
existing multi-line statement. But those cases should be rare.
Signed-off-by: Jason J. Herne <[email protected]>
Message-Id: <1449858642[email protected]>
[Rephrased "Error function text" to "Error messages", dropped
error_vprintf, error_printf, error_printf from $qemu_error_funcs,
because they may legitimately print newlines] Signed-off-by: Markus Armbruster <[email protected]>
error: Clean up errors with embedded newlines (again)
The arguments of error_report() should yield a short error string
without newlines.
A few places try to print additional help after the error message by
embedding newlines in the error string. That's nice, but let's do it
the right way. Commit 474c213 cleaned up some, but they keep coming
back. Offenders tracked down with the Coccinelle semantic patch from
commit 312fd5f.
vhdx: Fix "log that needs to be replayed" error message
The arguments of error_setg_errno() should yield a short error string
without newlines.
Here, we try to append additional help to the error message by
embedding newlines in the error string. That's nice, but it's doesn't
play nicely with the errno part. tests/qemu-iotests/070.out shows the
resulting mess:
can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed. To replay the log, execute:
qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx': Operation not permitted
Switch to error_setg() and error_append_hint(). Result:
can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed
To replay the log, run:
qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx'
pci-assign: Clean up "Failed to assign" error messages
The arguments of error_setg() & friends should yield a short error
string without newlines.
Two places try to append additional help to the error message by
embedding newlines in the error string. That's nice, but let's do it
the right way, with error_append_hint().
error: Strip trailing '\n' from error string arguments (again)
Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
keep coming back. Tracked down with the Coccinelle semantic patch
from commit 312fd5f.
migration: Use error_reportf_err() instead of monitor_printf()
Both error_reportf_err() and monitor_printf() print to the same
destination when monitor_printf() is used correctly, i.e. within an
HMP monitor. Elsewhere, monitor_printf() does nothing, while
error_reportf_err() reports to stderr.
Both changed functions are HMP command handlers. These should only
run within an HMP monitor.
Unlike monitor_printf(), error_reportf_err() uses the error whole
instead of just its message obtained with error_get_pretty(). This
avoids suppressing its hint (see commit 50b7b00), but I don't think
the errors touched in this commit can come with hints.
followed by manual cleanup, first because I can't figure out how to
make Coccinelle transform strings, and second to get rid of now
superfluous error_propagate().
We now use or propagate the original error whole instead of just its
message obtained with error_get_pretty(). This avoids suppressing its
hint (see commit 50b7b00), but I can't see how the errors touched in
this commit could come with hints. It also improves the message
printed with &error_abort when we screw up (see commit 1e9b65b).
followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
because I can't figure out how to make Coccinelle transform strings.
We now use the error whole instead of just its message obtained with
error_get_pretty(). This avoids suppressing its hint (see commit 50b7b00), but I can't see how the errors touched in this commit could
come with hints.
This is suboptimal, because it loses err's hint (if any). Moreover,
when errp is &error_abort or is subsequently propagated to
&error_abort, the abort message points to the place where we last
added to the error, not to the place where it originated.
To avoid these issues, provide means to add to an error's message in
place:
The code looks like it tries to check for both qemu_init_main_loop()
and qemu_get_aio_context() failure in one conditional. In fact,
qemu_get_aio_context() can fail only after qemu_init_main_loop()
failed.
Simplify accordingly: check for qemu_init_main_loop() error directly,
without bothering to improve its error message. Call
qemu_get_aio_context() only when qemu_get_aio_context() succeeded. It
can't fail then, so no need to check.
qemu-nbd: Clean up "Failed to load snapshot" error message
bdrv_snapshot_load_tmp() sets an error and returns -errno on failure.
We report both even though the error message is self-contained. Drop
the redundant strerror().
While there: setting errno right before exit() is pointless, so drop
that, too.
block: Clean up "Could not create temporary overlay" error message
bdrv_create() sets an error and returns -errno on failure. When the
latter is interesting, the error is created with error_setg_errno().
bdrv_append_temp_snapshot() uses the error's message to create a new
one with error_setg_errno(). This adds a strerror() that is either
uninteresting or duplicate. Use error_setg() instead.
error: Use error_report_err() instead of ad hoc prints
Unlike ad hoc prints, error_report_err() uses the error whole instead
of just its message obtained with error_get_pretty(). This avoids
suppressing its hint (see commit 50b7b00). Example:
$ bld/ivshmem-server -l 42@
Parameter 'shm_size' expects a size
You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes.
The last line is new with this patch.
While there, drop a "cannot parse shm size: " message prefix; it's
redundant, because the error message proper is always of the form
"Parameter 'shm_size' expects ...".
error: Use error_report_err() instead of monitor_printf()
Both error_report_err() and monitor_printf() print to the same
destination when monitor_printf() is used correctly, i.e. within an
HMP monitor. Elsewhere, monitor_printf() does nothing, while
error_report_err() reports to stderr.
Most changed functions are HMP command handlers. These should only
run within an HMP monitor. The one exception is bdrv_password_cb(),
which should also only run within an HMP monitor.
Four command handlers prefix the error message with the command name:
balloon, migrate_set_capability, migrate_set_parameter, migrate.
Pointless, drop.
Unlike monitor_printf(), error_report_err() uses the error whole
instead of just its message obtained with error_get_pretty(). This
avoids suppressing its hint (see commit 50b7b00). Example:
(qemu) device_add ivshmem,id=666
Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.
Try "help device_add" for more information
The "Identifiers consist of..." line is new with this patch.
Coccinelle semantic patch:
@@
expression M, E;
@@
- monitor_printf(M, "%s\n", error_get_pretty(E));
- error_free(E);
+ error_report_err(E);
@r1@
expression M, E;
format F;
position p;
@@
- monitor_printf(M, "...%@F@\n", error_get_pretty(E));@p
- error_free(E);
+ error_report_err(E);
@script:python@
p << r1.p;
@@
print "%s:%s:%s: prefix dropped" % (p[0].file, p[0].line, p[0].column)
error: Use error_report_err() where appropriate (again)
Same Coccinelle semantic patch as in commit 565f65d.
We now use the original error whole instead of just its message
obtained with error_get_pretty(). This avoids suppressing its hint
(see commit 50b7b00), but I don't think the errors touched in this
commit can come with hints.
audio: Clean up inappropriate and unreachable use of hw_error()
audio_init() should not use hw_error(), because dumping CPU registers
is unhelpful there, and aborting is wrong, because it can be called
called from an audio device's realize() method.
The two uses of hw_error() come from commit 0d9acba:
* When qemu_new_timer() fails. It couldn't fail back then, and it
can't fail now. Drop the unreachable error handling.
* When no_audio_driver can't be initialized. It couldn't fail back
then, and it can't fail now. Replace the error handling by an
assertion.
isa_bus_irqs(), isa_create() and isa_try_create() call hw_error() when
passed a null bus. Use of hw_error() has always been questionable,
because these are used only during machine initialization, and
printing CPU registers isn't useful there.
Since the previous commit, passing a null bus is a programming error.
Drop the hw_error() and simply let it crash.
We can have at most one ISA bus. If you try to create another one,
isa_bus_new() complains to stderr and returns null.
isa_bus_new() is called in two contexts, machine's init() and device's
realize() methods. Since complaining to stderr is not proper in the
latter context, convert isa_bus_new() to Error.
Machine's init():
* mips_jazz_init(), called from the init() methods of machines
"magnum" and "pica"
* mips_r4k_init(), the init() method of machine "mips"
* pc_init1() called from the init() methods of non-q35 PC machines
* typhoon_init(), called from clipper_init(), the init() method of
machine "clipper"
These callers always create the first ISA bus, hence isa_bus_new()
can't fail. Simply pass &error_abort.
Device's realize():
* i82378_realize(), of PCI device "i82378"
* ich9_lpc_realize(), of PCI device "ICH9-LPC"
* pci_ebus_realize(), of PCI device "ebus"
* piix3_realize(), of PCI device "pci-piix3", abstract parent of
"PIIX3" and "PIIX3-xen"
* piix4_realize(), of PCI device "PIIX4"
* vt82c686b_realize(), of PCI device "VT82C686B"
Propagate the error. Note that these devices are typically created
only by machine init() methods with qdev_init_nofail() or similar. If
we screwed up and created an ISA bus before that call, we now give up
right away. Before, we'd hobble on, and typically die in
isa_bus_irqs(). Similar if someone finds a way to hot-plug one of
these critters.
sysbus: Don't use hw_error() in machine_init_done_notifiers
platform_bus_map_irq() and platform_bus_map_mmio() use hw_error() to
fail. They run in machine_init_done_notifiers, via
platform_bus_init_notify() and link_sysbus_device(). Printing CPU
registers is not helpful there.
Replace hw_error() by error_report(); exit(1). If these are
programming errors, it should be replaced by an assertion instead.
While there, observe that both functions always return 0, and
link_sysbus_device() ignores the return value. Change them to void.
virt_set_gic_version() calls exit(1) when passed an invalid property
value. Property setters are not supposed to do that. Screwed up in
commit b92ad39. Harmless, because the property belongs to a machine.
Set an error object instead.
error: Don't append a newline when printing the error hint
Since commit 50b7b00, we have error_append_hint() to conveniently
accumulate Error member @hint. error_report_err() prints it with a
newline appended. Consequently, users of error_append_hint() need to
know whether theirs is the final line of the hint to decide whether it
needs a newline. Not a nice interface.
Change error_report_err() to print just the hint, and the (still few)
users of error_append_hint() to add the required newline.
Device realize() methods aren't supposed to call hw_error(), they
should set an error and fail cleanly. Blindly doing that would be
easy enough, but then realize() would fail without undoing its side
effects. Just mark it FIXME for now.
omap: Don't use hw_error() in device init() methods
Device init() methods aren't supposed to call hw_error(), they should
report the error and fail cleanly. Do that.
The errors are all device misconfiguration. All callers use
qdev_init_nofail(), so this patch merely converts hw_error() crashes
into &error_abort crashes. Improvement, because now it crashes closer
to where the misconfiguration bug would be, and a few more bad
examples of hw_error() use are gone.
hw: Don't use hw_error() for machine initialization errors
Printing CPU registers is not helpful during machine initialization.
Moreover, these are straightforward configuration or "can get
resources" errors, so dumping core isn't appropriate either. Replace
hw_error() by error_report(); exit(1). Matches how we report these
errors in other machine initializations.
projects / qemu.git / log