Re: [Qemu-devel] [PATCH v3 08/38] 9pfs: Fix CLI parsing crash on error
On Tue, 16 Oct 2018 19:41:28 +0200
Markus Armbruster wrote:
> Calling error_report() in a function that takes an Error ** argument
> is suspicious. 9p-handle.c's handle_parse_opts() does that, and then
> fails without setting an error. Wrong. Its caller crashes when it
> tries to report the error:
>
> $ qemu-system-x86_64 -nodefaults -fsdev id=foo,fsdriver=handle
> qemu-system-x86_64: -fsdev id=foo,fsdriver=handle: warning: handle
> backend is deprecated
> qemu-system-x86_64: -fsdev id=foo,fsdriver=handle: fsdev: No path
> specified
> Segmentation fault (core dumped)
>
> Screwed up when commit 91cda4e8f37 (v2.12.0) converted the function to
> Error. Fix by calling error_setg() instead of error_report().
>
> Fixes: 91cda4e8f372602795e3a2f4bd2e3adaf9f82255
> Cc: Greg Kurz
> Signed-off-by: Markus Armbruster
> Acked-by: Greg Kurz
> ---
Hi Markus,
FWIW you had a Reviewed-by from Eric.
https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg03297.html
Cheers,
--
Greg
> hw/9pfs/9p-handle.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c
> index f3641dbe4a..3465b1ef30 100644
> --- a/hw/9pfs/9p-handle.c
> +++ b/hw/9pfs/9p-handle.c
> @@ -19,6 +19,7 @@
> #include
> #include
> #include
> +#include "qapi/error.h"
> #include "qemu/xattr.h"
> #include "qemu/cutils.h"
> #include "qemu/error-report.h"
> @@ -655,12 +656,13 @@ static int handle_parse_opts(QemuOpts *opts,
> FsDriverEntry *fse, Error **errp)
> warn_report("handle backend is deprecated");
>
> if (sec_model) {
> -error_report("Invalid argument security_model specified with handle
> fsdriver");
> +error_setg(errp,
> + "Invalid argument security_model specified with handle
> fsdriver");
> return -1;
> }
>
> if (!path) {
> -error_report("fsdev: No path specified");
> +error_setg(errp, "fsdev: No path specified");
> return -1;
> }
> fse->path = g_strdup(path);
Re: [Qemu-devel] [PATCH] qapi: add info about reset to SHUTDOWN event
On 10/8/18 3:19 PM, Dominik Csapak wrote:
when '-no-reboot' is set, it is interesting if the guest was originally
shutdown or reset, so save and return that info
Signed-off-by: Dominik Csapak
---
qapi/run-state.json | 5 -
vl.c| 5 -
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 332e44897b..ec1769777d 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -107,6 +107,9 @@
# a guest-initiated ACPI shutdown request or other hardware-specific action)
# rather than a host request (such as sending qemu a SIGINT). (since 2.10)
#
+# @was_reset: If true, the shutdown was actually a reset, but no-reboot
+# was set, so it got converted to a shutdown
+#
# Note: If the command-line option "-no-shutdown" has been specified, qemu
will
# not exit, and a STOP event will eventually follow the SHUTDOWN event
#
@@ -118,7 +121,7 @@
# "timestamp": { "seconds": 1267040730, "microseconds": 682951 } }
#
##
-{ 'event': 'SHUTDOWN', 'data': { 'guest': 'bool' } }
+{ 'event': 'SHUTDOWN', 'data': { 'guest': 'bool', 'was_reset': 'bool' } }
##
# @POWERDOWN:
diff --git a/vl.c b/vl.c
index 4e25c78bff..66e29cb830 100644
--- a/vl.c
+++ b/vl.c
@@ -1524,6 +1524,7 @@ void vm_state_notify(int running, RunState state)
static ShutdownCause reset_requested;
static ShutdownCause shutdown_requested;
+static bool shutdown_was_reset;
static int shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
@@ -1681,6 +1682,7 @@ void qemu_system_guest_panicked(GuestPanicInformation
*info)
void qemu_system_reset_request(ShutdownCause reason)
{
if (no_reboot && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
+shutdown_was_reset = true;
shutdown_requested = reason;
} else {
reset_requested = reason;
@@ -1807,7 +1809,8 @@ static bool main_loop_should_exit(void)
request = qemu_shutdown_requested();
if (request) {
qemu_kill_report();
-qapi_event_send_shutdown(shutdown_caused_by_guest(request));
+qapi_event_send_shutdown(shutdown_caused_by_guest(request),
+ shutdown_was_reset);
if (no_shutdown) {
vm_stop(RUN_STATE_SHUTDOWN);
} else {
hi,
any comment on this?
Re: [Qemu-devel] [PATCH 1/4] ptr_ring: port ptr_ring from linux kernel to QEMU
On 16/10/2018 18:40, Emilio G. Cota wrote: >> +#define SMP_CACHE_BYTES 64 >> +#define cacheline_aligned_in_smp \ >> +__attribute__((__aligned__(SMP_CACHE_BYTES))) > You could use QEMU_ALIGNED() here. > >> + >> +#define WRITE_ONCE(ptr, val) \ >> +(*((volatile typeof(ptr) *)(&(ptr))) = (val)) >> +#define READ_ONCE(ptr) (*((volatile typeof(ptr) *)(&(ptr > Why not atomic_read/set, like in the rest of the QEMU code base? Or even atomic_rcu_read/atomic_rcu_set, which includes the necessary barriers. Also, please do not use __ identifiers in QEMU code. cacheline_aligned_in_smp can become just QEMU_ALIGNED(SMP_CACHE_BYTES). Paolo
Re: [Qemu-devel] [PATCH RFC v5 1/7] Fix segmentation fault when qemu_signal_init fails
Sorry for the late reply! Omitted this one..
On 10/12/2018 09:26 PM, Markus Armbruster wrote:
Fei Li writes:
On 10/12/2018 03:56 PM, Markus Armbruster wrote:
Fei Li writes:
On 10/11/2018 06:02 PM, Markus Armbruster wrote:
Fei Li writes:
Currently, when qemu_signal_init() fails it only returns a non-zero
value but without propagating any Error. But its callers need a
non-null err when runs error_report_err(err), or else 0->msg occurs.
The bug is in qemu_init_main_loop():
ret = qemu_signal_init();
if (ret) {
return ret;
}
Fails without setting an error, unlike the other failures. Its callers
crash then.
Thanks!
Consider working that into your commit message.
Ok. I'd like to reword as follows:
Currently in qemu_init_main_loop(), qemu_signal_init() fails without
setting an error
like the other failures. Its callers crash then when runs
error_report_err(err).
Polishing the English a bit:
When qemu_signal_init() fails in qemu_init_main_loop(), we return
without setting an error. Its callers crash then when they try to
report the error with error_report_err().
Thanks. :)
To avoid such segmentation fault, add a new Error parameter to make
the call trace to propagate the err to the final caller.
Signed-off-by: Fei Li
Reviewed-by: Fam Zheng
---
include/qemu/osdep.h | 2 +-
util/compatfd.c | 9 ++---
util/main-loop.c | 9 -
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4f8559e550..f1f56763a0 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
additional fields in the future) */
};
-int qemu_signalfd(const sigset_t *mask);
+int qemu_signalfd(const sigset_t *mask, Error **errp);
void sigaction_invoke(struct sigaction *action,
struct qemu_signalfd_siginfo *info);
#endif
diff --git a/util/compatfd.c b/util/compatfd.c
index 980bd33e52..d3ed890405 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -16,6 +16,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "qemu/thread.h"
+#include "qapi/error.h"
#include
@@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
}
}
-static int qemu_signalfd_compat(const sigset_t *mask)
+static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
{
struct sigfd_compat_info *info;
QemuThread thread;
@@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
info = malloc(sizeof(*info));
if (info == NULL) {
+error_setg(errp, "Failed to allocate signalfd memory");
errno = ENOMEM;
return -1;
}
if (pipe(fds) == -1) {
+error_setg(errp, "Failed to create signalfd pipe");
free(info);
return -1;
}
@@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
return fds[0];
}
-int qemu_signalfd(const sigset_t *mask)
+int qemu_signalfd(const sigset_t *mask, Error **errp)
{
#if defined(CONFIG_SIGNALFD)
int ret;
@@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
}
#endif
-return qemu_signalfd_compat(mask);
+return qemu_signalfd_compat(mask, errp);
}
I think this takes the Error conversion too far.
qemu_signalfd() is like the signalfd() system call, only portable, and
setting FD_CLOEXEC. In particular, it reports failure just like a
system call: it sets errno and returns -1. I'd prefer to keep it that
way. Instead...
diff --git a/util/main-loop.c b/util/main-loop.c
index affe0403c5..9671b6d226 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
}
}
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
{
int sigfd;
sigset_t set;
@@ -94,9 +94,8 @@ static int qemu_signal_init(void)
pthread_sigmask(SIG_BLOCK, &set, NULL);
sigdelset(&set, SIG_IPI);
-sigfd = qemu_signalfd(&set);
+sigfd = qemu_signalfd(&set, errp);
if (sigfd == -1) {
-fprintf(stderr, "failed to create signalfd\n");
return -errno;
}
... change this function so:
pthread_sigmask(SIG_BLOCK, &set, NULL);
sigdelset(&set, SIG_IPI);
sigfd = qemu_signalfd(&set);
if (sigfd == -1) {
-fprintf(stderr, "failed to create signalfd\n");
+error_setg_errno(errp, errno, "failed to create signalfd");
return -errno;
}
Does this make sense?
Yes, it looks more concise if we only have this patch, but triggers
one errno-set
issue after applying patch 7/7, which adds a Error **errp parameter for
qemu_thread_create() to let its callers handle the error instead of
exit() directly
to add the robustness.
Re: [Qemu-devel] [PATCH v3 08/38] 9pfs: Fix CLI parsing crash on error
Greg Kurz writes: > On Tue, 16 Oct 2018 19:41:28 +0200 > Markus Armbruster wrote: > >> Calling error_report() in a function that takes an Error ** argument >> is suspicious. 9p-handle.c's handle_parse_opts() does that, and then >> fails without setting an error. Wrong. Its caller crashes when it >> tries to report the error: >> >> $ qemu-system-x86_64 -nodefaults -fsdev id=foo,fsdriver=handle >> qemu-system-x86_64: -fsdev id=foo,fsdriver=handle: warning: handle >> backend is deprecated >> qemu-system-x86_64: -fsdev id=foo,fsdriver=handle: fsdev: No path >> specified >> Segmentation fault (core dumped) >> >> Screwed up when commit 91cda4e8f37 (v2.12.0) converted the function to >> Error. Fix by calling error_setg() instead of error_report(). >> >> Fixes: 91cda4e8f372602795e3a2f4bd2e3adaf9f82255 >> Cc: Greg Kurz >> Signed-off-by: Markus Armbruster >> Acked-by: Greg Kurz >> --- > > Hi Markus, > > FWIW you had a Reviewed-by from Eric. > > https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg03297.html Fixed in v4. Thanks!
[Qemu-devel] [PATCH v2 0/4] Introduce attributes for timers subsystem and remove QEMU_CLOCK_VIRTUAL_EXT clock type
Recent patches from series [PATCH v6] "Fixing record/replay and adding reverse
debugging" introduced new clock type QEMU_CLOCK_VIRTUAL_EXT and replaced
virtual timers in some external subsystems with it.
This resulted in small change to existing behavior, which I consider to be
unacceptable.
Processing of virtual timers, belonging to new clock type, was kicked off to
main loop, which made them asynchronous with vCPU thread and, in icount mode,
with whole guest execution. This breaks expected determinism in
non-record/replay icount mode of emulation where these "external subsystems"
are isolated from host (i.e. they external only to guest core, not to emulation
environment).
Example for slirp ("user" backend for network device):
User runs qemu in icount mode with rtc clock=vm without any external
communication interfaces but with "-netdev user,restrict=on". It expects
deterministic execution, because network services are emulated inside qemu and
isolated from host. There are no reasons to get reply from DHCP server with
different delay or something like that.
These series of patches revert those commits and reimplement their
modifications in a better way.
Current implementation of timers/clock processing is confusing (at least for
me) because of exceptions from design concept behind them, which already
introduced by icount mode (which adds QEMU_CLOCK_VIRTUAL_RT). Adding
QEMU_CLOCK_VIRTUAL_EXT just made things even more complicated. I consider these
"alternative" virtual clocks to be some kind of hacks being convinient only to
authors of relevant qemu features. Lets don't touch fundamental clock types and
keep them orthogonal to special cases of timers handling.
As far as I understand, original intention of author was just to make
optimization in replay log by avoiding storing extra events which don't change
guest state directly. Indeed, for example, ipv6 icmp timer in slirp does things
which external to guest core and ends with sending network packet to guest, but
record/replay will anyway catch event, corresponding to packet reception in
guest network frontend, and store it to replay log, so there are no need in
making checkpoint for corresponding clock when that timer fires.
If so, then we just need to skip checkpoints for clock values, when only these
specific timers are run. It is individual timers which are specific, not clock.
Adding some kind of attribute/flag/property to individual timer allows any
special qemu feature (such as record/replay) to inspect it in any place of code
and handle as needed. This design acheives less dependencies, more transparency
and has more intuitive and clear sense. For record/replay feature it's acheived
with 'EXTERNAL' attribute. The only drawback is that it required to add one
extra mutex unlock/lock pair for virtual clock type in timerlist_run_timers()
function (see patch 3), but it's being added only when qemu runs in
record/replay mode.
Finally, this patch series optimizes checkpointing for all other clocks it
applies to.
v2 changes:
- timers/aio interface refactored and improved, addition to couroutines
removed (as Paolo Bonzini suggested)
- fixed wrong reimplementation in qemu-timer.c caused race conditions
- added bonus patch which optimizes checkpointing for other clocks
P.S. I've tried to test record/replay with slirp, but in replay mode qemu
stucks at guest linux boot after "Configuring network interfaces..." message,
where DHCP communication takes place. It's broken in a same way both in master
and master with reverted commits being fixed.
P.S.2. I wasn't able to test these patches on purely clean master branch
because of bugs https://bugs.launchpad.net/qemu/+bug/1790460 and
https://bugs.launchpad.net/qemu/+bug/1795369, which workarounded by several
not-accepted (yet?) modifications.
Artem Pisarenko (4):
Revert some patches from recent [PATCH v6] "Fixing record/replay and
adding reverse debugging"
Introduce attributes to qemu timer subsystem
Restores record/replay behavior related to special virtual clock
processing for timers used in external subsystems.
Optimize record/replay checkpointing for all clocks it applies to
include/block/aio.h | 57 +++---
include/qemu/timer.h | 143 +-
slirp/ip6_icmp.c | 9 +--
tests/ptimer-test-stubs.c | 13 +++--
ui/input.c| 9 +--
util/qemu-timer.c | 100 ++--
6 files changed, 219 insertions(+), 112 deletions(-)
--
2.7.4
[Qemu-devel] [PATCH v2 1/4] Revert some patches from recent [PATCH v6] "Fixing record/replay and adding reverse debugging"
That patch series introduced new virtual clock type for use in external
subsystems. It breaks desired behavior in non-record/replay usage scenarios.
This reverts commit 87f4fe7653baf55b5c2f2753fe6003f473c07342.
This reverts commit 775a412bf83f6bc0c5c02091ee06cf649b34c593.
This reverts commit 9888091404a702d7ec79d51b088d994b9fc121bd.
Signed-off-by: Artem Pisarenko
---
include/qemu/timer.h | 9 -
slirp/ip6_icmp.c | 7 +++
ui/input.c | 8
util/qemu-timer.c| 2 --
4 files changed, 7 insertions(+), 19 deletions(-)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index a005ed2..39ea907 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -42,14 +42,6 @@
* In icount mode, this clock counts nanoseconds while the virtual
* machine is running. It is used to increase @QEMU_CLOCK_VIRTUAL
* while the CPUs are sleeping and thus not executing instructions.
- *
- * @QEMU_CLOCK_VIRTUAL_EXT: virtual clock for external subsystems
- *
- * The virtual clock only runs during the emulation. It stops
- * when the virtual machine is stopped. The timers for this clock
- * do not recorded in rr mode, therefore this clock could be used
- * for the subsystems that operate outside the guest core.
- *
*/
typedef enum {
@@ -57,7 +49,6 @@ typedef enum {
QEMU_CLOCK_VIRTUAL = 1,
QEMU_CLOCK_HOST = 2,
QEMU_CLOCK_VIRTUAL_RT = 3,
-QEMU_CLOCK_VIRTUAL_EXT = 4,
QEMU_CLOCK_MAX
} QEMUClockType;
diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
index 3f41187..ee333d0 100644
--- a/slirp/ip6_icmp.c
+++ b/slirp/ip6_icmp.c
@@ -17,7 +17,7 @@ static void ra_timer_handler(void *opaque)
{
Slirp *slirp = opaque;
timer_mod(slirp->ra_timer,
- qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_EXT) + NDP_Interval);
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + NDP_Interval);
ndp_send_ra(slirp);
}
@@ -27,10 +27,9 @@ void icmp6_init(Slirp *slirp)
return;
}
-slirp->ra_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL_EXT,
- ra_timer_handler, slirp);
+slirp->ra_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, ra_timer_handler,
slirp);
timer_mod(slirp->ra_timer,
- qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_EXT) + NDP_Interval);
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + NDP_Interval);
}
void icmp6_cleanup(Slirp *slirp)
diff --git a/ui/input.c b/ui/input.c
index dd7f6d7..51b1019 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -271,7 +271,7 @@ static void qemu_input_queue_process(void *opaque)
item = QTAILQ_FIRST(queue);
switch (item->type) {
case QEMU_INPUT_QUEUE_DELAY:
-timer_mod(item->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_EXT)
+timer_mod(item->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)
+ item->delay_ms);
return;
case QEMU_INPUT_QUEUE_EVENT:
@@ -301,7 +301,7 @@ static void qemu_input_queue_delay(struct
QemuInputEventQueueHead *queue,
queue_count++;
if (start_timer) {
-timer_mod(item->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_EXT)
+timer_mod(item->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)
+ item->delay_ms);
}
}
@@ -448,8 +448,8 @@ void qemu_input_event_send_key_delay(uint32_t delay_ms)
}
if (!kbd_timer) {
-kbd_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL_EXT,
- qemu_input_queue_process, &kbd_queue);
+kbd_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, qemu_input_queue_process,
+ &kbd_queue);
}
if (queue_count < queue_limit) {
qemu_input_queue_delay(&kbd_queue, kbd_timer,
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index eb60d8f..86bfe84 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -496,7 +496,6 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
switch (timer_list->clock->type) {
case QEMU_CLOCK_REALTIME:
-case QEMU_CLOCK_VIRTUAL_EXT:
break;
default:
case QEMU_CLOCK_VIRTUAL:
@@ -598,7 +597,6 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
return get_clock();
default:
case QEMU_CLOCK_VIRTUAL:
-case QEMU_CLOCK_VIRTUAL_EXT:
if (use_icount) {
return cpu_get_icount();
} else {
--
2.7.4
[Qemu-devel] [PATCH v2 2/4] Introduce attributes to qemu timer subsystem
Attributes are simple flags, associated with individual timers for their whole
lifetime.
They intended to be used to mark individual timers for special handling by
various qemu features operating at qemu core level.
New/init functions family in timer interface updated and refactored (new
'attribute' argument added, timer_list replaced with timer_list_group+type
combinations, comments improved to avoid info duplication).
Also existing aio interface extended with attribute-enabled variants of
functions, which create/initialize timers.
Signed-off-by: Artem Pisarenko
---
Notes:
v2:
- timer creation/initialize functions reworked and and their unnecessary
variants removed (as Paolo Bonzini suggested)
- also their comments improved to avoid info duplication
include/block/aio.h | 57 ++---
include/qemu/timer.h | 128 ++
tests/ptimer-test-stubs.c | 13 +++--
util/qemu-timer.c | 18 +--
4 files changed, 146 insertions(+), 70 deletions(-)
diff --git a/include/block/aio.h b/include/block/aio.h
index f08630c..fce9d48 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -388,6 +388,31 @@ struct LinuxAioState *aio_setup_linux_aio(AioContext *ctx,
Error **errp);
struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);
/**
+ * aio_timer_new_with_attrs:
+ * @ctx: the aio context
+ * @type: the clock type
+ * @scale: the scale
+ * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to
assign
+ * @cb: the callback to call on timer expiry
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Allocate a new timer (with attributes) attached to the context @ctx.
+ * The function is responsible for memory allocation.
+ *
+ * The preferred interface is aio_timer_init or aio_timer_init_with_attrs.
+ * Use that unless you really need dynamic memory allocation.
+ *
+ * Returns: a pointer to the new timer
+ */
+static inline QEMUTimer *aio_timer_new_with_attrs(AioContext *ctx,
+ QEMUClockType type,
+ int scale, int attributes,
+ QEMUTimerCB *cb, void
*opaque)
+{
+return timer_new_full(&ctx->tlg, type, scale, attributes, cb, opaque);
+}
+
+/**
* aio_timer_new:
* @ctx: the aio context
* @type: the clock type
@@ -396,10 +421,7 @@ struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);
* @opaque: the opaque pointer to pass to the callback
*
* Allocate a new timer attached to the context @ctx.
- * The function is responsible for memory allocation.
- *
- * The preferred interface is aio_timer_init. Use that
- * unless you really need dynamic memory allocation.
+ * See aio_timer_new_with_attrs for details.
*
* Returns: a pointer to the new timer
*/
@@ -407,7 +429,28 @@ static inline QEMUTimer *aio_timer_new(AioContext *ctx,
QEMUClockType type,
int scale,
QEMUTimerCB *cb, void *opaque)
{
-return timer_new_tl(ctx->tlg.tl[type], scale, cb, opaque);
+return timer_new_full(&ctx->tlg, type, scale, 0, cb, opaque);
+}
+
+/**
+ * aio_timer_init_with_attrs:
+ * @ctx: the aio context
+ * @ts: the timer
+ * @type: the clock type
+ * @scale: the scale
+ * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to
assign
+ * @cb: the callback to call on timer expiry
+ * @opaque: the opaque pointer to pass to the callback
+ *
+ * Initialise a new timer (with attributes) attached to the context @ctx.
+ * The caller is responsible for memory allocation.
+ */
+static inline void aio_timer_init_with_attrs(AioContext *ctx,
+ QEMUTimer *ts, QEMUClockType type,
+ int scale, int attributes,
+ QEMUTimerCB *cb, void *opaque)
+{
+timer_init_full(ts, &ctx->tlg, type, scale, attributes, cb, opaque);
}
/**
@@ -420,14 +463,14 @@ static inline QEMUTimer *aio_timer_new(AioContext *ctx,
QEMUClockType type,
* @opaque: the opaque pointer to pass to the callback
*
* Initialise a new timer attached to the context @ctx.
- * The caller is responsible for memory allocation.
+ * See aio_timer_init_with_attrs for details.
*/
static inline void aio_timer_init(AioContext *ctx,
QEMUTimer *ts, QEMUClockType type,
int scale,
QEMUTimerCB *cb, void *opaque)
{
-timer_init_tl(ts, ctx->tlg.tl[type], scale, cb, opaque);
+timer_init_full(ts, &ctx->tlg, type, scale, 0, cb, opaque);
}
/**
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 39ea907..e225ad4 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -52,6 +52,28 @@ typedef enum {
QEMU_CLOCK_MAX
} QEMUClockType;
+/**
+ * QEMU Ti
[Qemu-devel] [PATCH v4 12/38] migration: Fix !replay_can_snapshot() error handling
Calling error_report() in a function that takes an Error ** argument
is suspicious. save_snapshot() and load_snapshot() do that, and then
fail without setting an error. Wrong. The HMP commands survive this
unscathed, since hmp_handle_error() does nothing when no error has
been set. Callers main() (on behalf of -loadvm) and
replay_vmstate_init() crash, but I'm not sure the error is possible
there.
Screwed up when commit 377b21ccea1 (v2.12.0) added incorrect error
handling right next to correct examples. Fix by calling error_setg()
instead of error_report().
Fixes: 377b21ccea1755a8b0dae822c29567c58dda6939
Cc: Paolo Bonzini
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
---
migration/savevm.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 2d10e45582..5f8eb38676 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2414,8 +2414,8 @@ int save_snapshot(const char *name, Error **errp)
AioContext *aio_context;
if (!replay_can_snapshot()) {
-error_report("Record/replay does not allow making snapshot "
- "right now. Try once more later.");
+error_setg(errp, "Record/replay does not allow making snapshot "
+ "right now. Try once more later.");
return ret;
}
@@ -2611,8 +2611,8 @@ int load_snapshot(const char *name, Error **errp)
MigrationIncomingState *mis = migration_incoming_get_current();
if (!replay_can_snapshot()) {
-error_report("Record/replay does not allow loading snapshot "
- "right now. Try once more later.");
+error_setg(errp, "Record/replay does not allow loading snapshot "
+ "right now. Try once more later.");
return -EINVAL;
}
--
2.17.1
[Qemu-devel] [PATCH v4 03/38] block: Use warn_report() & friends to report warnings
Calling error_report() in a function that takes an Error ** argument
is suspicious. Convert a few that are actually warnings to
warn_report().
While there, split warnings consisting of multiple sentences to
conform to conventions spelled out in warn_report()'s contract, and
improve a rather useless warning in sheepdog.c.
Cc: Kevin Wolf
Cc: Ronnie Sahlberg
Cc: Paolo Bonzini
Cc: Peter Lieven
Cc: Hitoshi Mitake
Cc: Liu Yuan
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
block/bochs.c| 8
block/cloop.c| 8
block/dmg.c | 8
block/iscsi.c| 2 +-
block/rbd.c | 12 ++--
block/sheepdog.c | 2 +-
block/vvfat.c| 8
7 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/block/bochs.c b/block/bochs.c
index 50c630047b..36c1b45bd2 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -112,10 +112,10 @@ static int bochs_open(BlockDriverState *bs, QDict
*options, int flags,
}
if (!bdrv_is_read_only(bs)) {
-error_report("Opening bochs images without an explicit read-only=on "
- "option is deprecated. Future versions will refuse to "
- "open the image instead of automatically marking the "
- "image read-only.");
+warn_report("Opening bochs images without an explicit read-only=on "
+"option is deprecated");
+error_printf("Future versions may refuse to open the image "
+ "instead of automatically marking it read-only.\n");
ret = bdrv_set_read_only(bs, true, errp); /* no write support yet */
if (ret < 0) {
return ret;
diff --git a/block/cloop.c b/block/cloop.c
index 2be68987bd..a558e67cb0 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -74,10 +74,10 @@ static int cloop_open(BlockDriverState *bs, QDict *options,
int flags,
}
if (!bdrv_is_read_only(bs)) {
-error_report("Opening cloop images without an explicit read-only=on "
- "option is deprecated. Future versions will refuse to "
- "open the image instead of automatically marking the "
- "image read-only.");
+warn_report("Opening cloop images without an explicit read-only=on "
+"option is deprecated");
+error_printf("Future versions may refuse to open the image "
+ "instead of automatically marking it read-only.\n");
ret = bdrv_set_read_only(bs, true, errp);
if (ret < 0) {
return ret;
diff --git a/block/dmg.c b/block/dmg.c
index c9b3c519c4..9fb814460d 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -420,10 +420,10 @@ static int dmg_open(BlockDriverState *bs, QDict *options,
int flags,
}
if (!bdrv_is_read_only(bs)) {
-error_report("Opening dmg images without an explicit read-only=on "
- "option is deprecated. Future versions will refuse to "
- "open the image instead of automatically marking the "
- "image read-only.");
+warn_report("Opening dmg images without an explicit read-only=on "
+"option is deprecated");
+error_printf("Future versions may refuse to open the image "
+ "instead of automatically marking it read-only.\n");
ret = bdrv_set_read_only(bs, true, errp);
if (ret < 0) {
return ret;
diff --git a/block/iscsi.c b/block/iscsi.c
index bb69faf34a..73998c2860 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1844,7 +1844,7 @@ static int iscsi_open(BlockDriverState *bs, QDict
*options, int flags,
iscsi_set_timeout(iscsi, timeout);
#else
if (timeout) {
-error_report("iSCSI: ignoring timeout value for libiscsi <1.15.0");
+warn_report("iSCSI: ignoring timeout value for libiscsi <1.15.0");
}
#endif
diff --git a/block/rbd.c b/block/rbd.c
index 014c68d629..6e26bac170 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -750,8 +750,8 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
/* Take care whenever deciding to actually deprecate; once this ability
* is removed, we will not be able to open any images with
legacy-styled
* backing image strings. */
-error_report("RBD options encoded in the filename as keyvalue pairs "
- "is deprecated");
+warn_report("RBD options encoded in the filename as keyvalue pairs "
+"is deprecated");
}
/* Remove the processed options from the QDict (the visitor processes
@@ -781,10 +781,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
* leave as-is */
if (s->snap != NULL) {
if (!bdrv_is_read_only(bs)) {
-error_report("Opening rbd snapshots without an explicit "
- "read-only=on option is deprecated. Future versions
[Qemu-devel] [PATCH v2 4/4] Optimize record/replay checkpointing for all clocks it applies to
Removes redundant checkpoints in replay log when there are no expired timers in
timers list, associated with corresponding clock (i.e. no rr events associated
with current clock value).
This also improves performance in rr mode.
Signed-off-by: Artem Pisarenko
---
include/qemu/timer.h | 2 +-
util/qemu-timer.c| 62 +---
2 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 8e3f236..bd627dc 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -70,7 +70,7 @@ typedef enum {
* QEMU_TIMER_ATTR_EXTERNAL: drives external subsystem
*
* Timers with this attribute do not recorded in rr mode, therefore it could be
- * used for the subsystems that operate outside the guest core. Applicable only
+ * used for the subsystems that operate outside the guest core. Relevant only
* with virtual clock type.
*/
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index e9a0f00..7315ca1 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -495,6 +495,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
QEMUTimerCB *cb;
void *opaque;
bool need_replay_checkpoint = false;
+ReplayCheckpoint replay_checkpoint_id;
if (!atomic_read(&timer_list->active_timers)) {
return false;
@@ -505,43 +506,40 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
goto out;
}
-switch (timer_list->clock->type) {
-case QEMU_CLOCK_REALTIME:
-break;
-default:
-case QEMU_CLOCK_VIRTUAL:
-if (replay_mode != REPLAY_MODE_NONE) {
-/* Checkpoint for virtual clock is redundant in cases where
- * it's being triggered with only non-EXTERNAL timers, because
- * these timers don't change guest state directly.
- * Since it has conditional dependence on specific timers, it is
- * subject to race conditions and requires special handling.
- * See below.
- */
+if (replay_mode != REPLAY_MODE_NONE) {
+/* Postpone actual checkpointing to timer list processing
+ * to properly check if we actually need it.
+ */
+switch (timer_list->clock->type) {
+case QEMU_CLOCK_VIRTUAL:
need_replay_checkpoint = true;
+replay_checkpoint_id = CHECKPOINT_CLOCK_VIRTUAL;
+break;
+case QEMU_CLOCK_HOST:
+need_replay_checkpoint = true;
+replay_checkpoint_id = CHECKPOINT_CLOCK_HOST;
+break;
+case QEMU_CLOCK_VIRTUAL_RT:
+need_replay_checkpoint = true;
+replay_checkpoint_id = CHECKPOINT_CLOCK_VIRTUAL_RT;
+break;
+default:
+break;
}
-break;
-case QEMU_CLOCK_HOST:
-if (!replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
-goto out;
-}
-break;
-case QEMU_CLOCK_VIRTUAL_RT:
-if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
-goto out;
-}
-break;
}
/*
- * Extract expired timers from active timers list and and process them.
+ * Extract expired timers from active timers list and and process them,
+ * taking into account checkpointing required in rr mode.
*
- * In rr mode we need "filtered" checkpointing for virtual clock.
- * Checkpoint must be replayed before any non-EXTERNAL timer has been
- * processed and only one time (virtual clock value stays same). But these
- * timers may appear in the timers list while it being processed, so this
- * must be checked until we finally decide that "no timers left - we are
- * done".
+ * Checkpoint must be replayed before any timer has been processed
+ * and only one time. But new timers may appear in the timers list while
+ * it's being processed, so this must be checked until we finally decide
+ * that "no timers left - we are done" (to avoid skipping checkpoint due to
+ * possible races).
+ * Also checkpoint for virtual clock is redundant in cases where it's being
+ * triggered with only non-EXTERNAL timers, because these timers don't
+ * change guest state directly.
*/
current_time = qemu_clock_get_ns(timer_list->clock->type);
qemu_mutex_lock(&timer_list->active_timers_lock);
@@ -557,7 +555,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
/* once we got here, checkpoint clock only once */
need_replay_checkpoint = false;
qemu_mutex_unlock(&timer_list->active_timers_lock);
-if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
+if (!replay_checkpoint(replay_checkpoint_id)) {
goto out;
}
qemu_mutex_lock(&timer_list->active_timers_lock);
--
2.7.4
[Qemu-devel] [PATCH v2 3/4] Restores record/replay behavior related to special virtual clock processing for timers used in external subsystems.
Adds EXTERNAL attribute definition to qemu timers subsystem and assigns it to
virtual clock timers, used in slirp (ICMP IPv6) and ui (key queue).
Virtual clock processing in rr mode reimplemented using this attribute.
Fixes: 87f4fe7653baf55b5c2f2753fe6003f473c07342
Fixes: 775a412bf83f6bc0c5c02091ee06cf649b34c593
Fixes: 9888091404a702d7ec79d51b088d994b9fc121bd
Signed-off-by: Artem Pisarenko
---
Notes:
v2: fixes race condition and reimplements synchronization between
checkpointing and timers processing in qemu-timer.c
include/qemu/timer.h | 12 +---
slirp/ip6_icmp.c | 4 +++-
ui/input.c | 5 +++--
util/qemu-timer.c| 50 +++---
4 files changed, 58 insertions(+), 13 deletions(-)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index e225ad4..8e3f236 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -65,14 +65,20 @@ typedef enum {
* instead each attribute in bit set accessed with QEMU_TIMER_ATTR_ macro,
* where is a unique part of attribute identifier.
*
- * No attributes defined currently.
+ * The following attributes are available:
+ *
+ * QEMU_TIMER_ATTR_EXTERNAL: drives external subsystem
+ *
+ * Timers with this attribute do not recorded in rr mode, therefore it could be
+ * used for the subsystems that operate outside the guest core. Applicable only
+ * with virtual clock type.
*/
typedef enum {
-QEMU_TIMER_ATTRBIT__NONE
+QEMU_TIMER_ATTRBIT_EXTERNAL,
} QEMUTimerAttrBit;
-#define QEMU_TIMER_ATTR__NONE (1 << QEMU_TIMER_ATTRBIT__NONE)
+#define QEMU_TIMER_ATTR_EXTERNAL (1 << QEMU_TIMER_ATTRBIT_EXTERNAL)
typedef struct QEMUTimerList QEMUTimerList;
diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
index ee333d0..cd1e0b9 100644
--- a/slirp/ip6_icmp.c
+++ b/slirp/ip6_icmp.c
@@ -27,7 +27,9 @@ void icmp6_init(Slirp *slirp)
return;
}
-slirp->ra_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, ra_timer_handler,
slirp);
+slirp->ra_timer = timer_new_full(NULL, QEMU_CLOCK_VIRTUAL,
+ SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
+ ra_timer_handler, slirp);
timer_mod(slirp->ra_timer,
qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + NDP_Interval);
}
diff --git a/ui/input.c b/ui/input.c
index 51b1019..7c9a410 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -448,8 +448,9 @@ void qemu_input_event_send_key_delay(uint32_t delay_ms)
}
if (!kbd_timer) {
-kbd_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, qemu_input_queue_process,
- &kbd_queue);
+kbd_timer = timer_new_full(NULL, QEMU_CLOCK_VIRTUAL,
+ SCALE_MS, QEMU_TIMER_ATTR_EXTERNAL,
+ qemu_input_queue_process, &kbd_queue);
}
if (queue_count < queue_limit) {
qemu_input_queue_delay(&kbd_queue, kbd_timer,
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 2046b68..e9a0f00 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -494,6 +494,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
bool progress = false;
QEMUTimerCB *cb;
void *opaque;
+bool need_replay_checkpoint = false;
if (!atomic_read(&timer_list->active_timers)) {
return false;
@@ -509,8 +510,15 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
break;
default:
case QEMU_CLOCK_VIRTUAL:
-if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
-goto out;
+if (replay_mode != REPLAY_MODE_NONE) {
+/* Checkpoint for virtual clock is redundant in cases where
+ * it's being triggered with only non-EXTERNAL timers, because
+ * these timers don't change guest state directly.
+ * Since it has conditional dependence on specific timers, it is
+ * subject to race conditions and requires special handling.
+ * See below.
+ */
+need_replay_checkpoint = true;
}
break;
case QEMU_CLOCK_HOST:
@@ -525,13 +533,38 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
break;
}
+/*
+ * Extract expired timers from active timers list and and process them.
+ *
+ * In rr mode we need "filtered" checkpointing for virtual clock.
+ * Checkpoint must be replayed before any non-EXTERNAL timer has been
+ * processed and only one time (virtual clock value stays same). But these
+ * timers may appear in the timers list while it being processed, so this
+ * must be checked until we finally decide that "no timers left - we are
+ * done".
+ */
current_time = qemu_clock_get_ns(timer_list->clock->type);
-for(;;) {
-qemu_mutex_lock(&timer_list->active_timers_lock);
-ts = timer_list->active_timers;
+qemu_mutex_lock(&timer_list->active_timers_lock);
+while ((ts = timer_list->active_timers)) {
if (!t
qemu-devel@nongnu.org
>From include/qapi/error.h:
* Pass an existing error to the caller with the message modified:
* error_propagate(errp, err);
* error_prepend(errp, "Could not frobnicate '%s': ", name);
Fei Li pointed out that doing error_propagate() first doesn't work
well when @errp is &error_fatal or &error_abort: the error_prepend()
is never reached.
Since I doubt fixing the documentation will stop people from getting
it wrong, introduce error_propagate_prepend(), in the hope that it
lures people away from using its constituents in the wrong order.
Update the instructions in error.h accordingly.
Convert existing error_prepend() next to error_propagate to
error_propagate_prepend(). If any of these get reached with
&error_fatal or &error_abort, the error messages improve. I didn't
check whether that's the case anywhere.
Cc: Fei Li
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Eric Blake
---
block.c | 6 +++---
block/qcow2.c | 4 ++--
block/qed.c | 4 ++--
hw/9pfs/9p-local.c | 4 ++--
hw/intc/xics.c | 15 +--
hw/ppc/pnv_core.c | 4 ++--
hw/ppc/spapr_pci.c | 7 +++
hw/timer/aspeed_timer.c | 3 +--
hw/usb/bus.c| 5 +++--
hw/vfio/pci.c | 3 +--
include/qapi/error.h| 14 ++
migration/migration.c | 12 ++--
util/error.c| 13 +
13 files changed, 61 insertions(+), 33 deletions(-)
diff --git a/block.c b/block.c
index 7710b399a3..5d51419d21 100644
--- a/block.c
+++ b/block.c
@@ -4697,9 +4697,9 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType
op, Error **errp)
assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
if (!QLIST_EMPTY(&bs->op_blockers[op])) {
blocker = QLIST_FIRST(&bs->op_blockers[op]);
-error_propagate(errp, error_copy(blocker->reason));
-error_prepend(errp, "Node '%s' is busy: ",
- bdrv_get_device_or_node_name(bs));
+error_propagate_prepend(errp, error_copy(blocker->reason),
+"Node '%s' is busy: ",
+bdrv_get_device_or_node_name(bs));
return true;
}
return false;
diff --git a/block/qcow2.c b/block/qcow2.c
index 7277feda13..4f8d2fa7bd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2208,8 +2208,8 @@ static void coroutine_fn
qcow2_co_invalidate_cache(BlockDriverState *bs,
qemu_co_mutex_unlock(&s->lock);
qobject_unref(options);
if (local_err) {
-error_propagate(errp, local_err);
-error_prepend(errp, "Could not reopen qcow2 layer: ");
+error_propagate_prepend(errp, local_err,
+"Could not reopen qcow2 layer: ");
bs->drv = NULL;
return;
} else if (ret < 0) {
diff --git a/block/qed.c b/block/qed.c
index 689ea9d4d5..9377c0b7ad 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1606,8 +1606,8 @@ static void coroutine_fn
bdrv_qed_co_invalidate_cache(BlockDriverState *bs,
ret = bdrv_qed_do_open(bs, NULL, bs->open_flags, &local_err);
qemu_co_mutex_unlock(&s->table_lock);
if (local_err) {
-error_propagate(errp, local_err);
-error_prepend(errp, "Could not reopen qed layer: ");
+error_propagate_prepend(errp, local_err,
+"Could not reopen qed layer: ");
return;
} else if (ret < 0) {
error_setg_errno(errp, -ret, "Could not reopen qed layer");
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index c30f4f26bd..08e673a79c 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1509,8 +1509,8 @@ static int local_parse_opts(QemuOpts *opts, FsDriverEntry
*fse, Error **errp)
fsdev_throttle_parse_opts(opts, &fse->fst, &local_err);
if (local_err) {
-error_propagate(errp, local_err);
-error_prepend(errp, "invalid throttle configuration: ");
+error_propagate_prepend(errp, local_err,
+"invalid throttle configuration: ");
return -1;
}
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index c90c893228..406efee064 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -320,8 +320,9 @@ static void icp_realize(DeviceState *dev, Error **errp)
obj = object_property_get_link(OBJECT(dev), ICP_PROP_XICS, &err);
if (!obj) {
-error_propagate(errp, err);
-error_prepend(errp, "required link '" ICP_PROP_XICS "' not found: ");
+error_propagate_prepend(errp, err,
+"required link '" ICP_PROP_XICS
+"' not found: ");
return;
}
@@ -329,8 +330,9 @@ static void icp_realize(DeviceState *dev, Error **errp)
obj = object_property_get_link(OBJECT(dev), ICP_PROP_CPU, &err);
if (!obj) {
-error_propagate(errp, err);
-error_prepend(errp, "required link '" ICP_PROP_CPU "' not found: ");
+erro
[Qemu-devel] [PATCH v4 08/38] 9pfs: Fix CLI parsing crash on error
Calling error_report() in a function that takes an Error ** argument
is suspicious. 9p-handle.c's handle_parse_opts() does that, and then
fails without setting an error. Wrong. Its caller crashes when it
tries to report the error:
$ qemu-system-x86_64 -nodefaults -fsdev id=foo,fsdriver=handle
qemu-system-x86_64: -fsdev id=foo,fsdriver=handle: warning: handle backend
is deprecated
qemu-system-x86_64: -fsdev id=foo,fsdriver=handle: fsdev: No path specified
Segmentation fault (core dumped)
Screwed up when commit 91cda4e8f37 (v2.12.0) converted the function to
Error. Fix by calling error_setg() instead of error_report().
Fixes: 91cda4e8f372602795e3a2f4bd2e3adaf9f82255
Cc: Greg Kurz
Signed-off-by: Markus Armbruster
Acked-by: Greg Kurz
Reviewed-by: Eric Blake
---
hw/9pfs/9p-handle.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c
index f3641dbe4a..3465b1ef30 100644
--- a/hw/9pfs/9p-handle.c
+++ b/hw/9pfs/9p-handle.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include "qapi/error.h"
#include "qemu/xattr.h"
#include "qemu/cutils.h"
#include "qemu/error-report.h"
@@ -655,12 +656,13 @@ static int handle_parse_opts(QemuOpts *opts,
FsDriverEntry *fse, Error **errp)
warn_report("handle backend is deprecated");
if (sec_model) {
-error_report("Invalid argument security_model specified with handle
fsdriver");
+error_setg(errp,
+ "Invalid argument security_model specified with handle
fsdriver");
return -1;
}
if (!path) {
-error_report("fsdev: No path specified");
+error_setg(errp, "fsdev: No path specified");
return -1;
}
fse->path = g_strdup(path);
--
2.17.1
[Qemu-devel] [PATCH v4 18/38] vl: Clean up error reporting in parse_add_fd()
Calling error_report() in a function that takes an Error ** argument
is suspicious. parse_add_fd() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
Also change call of cleanup_add_fd(), which can't fail, for symmetry.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
---
vl.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/vl.c b/vl.c
index 643121e5d4..2ef066ad61 100644
--- a/vl.c
+++ b/vl.c
@@ -1059,12 +1059,12 @@ static int parse_add_fd(void *opaque, QemuOpts *opts,
Error **errp)
fd_opaque = qemu_opt_get(opts, "opaque");
if (fd < 0) {
-error_report("fd option is required and must be non-negative");
+error_setg(errp, "fd option is required and must be non-negative");
return -1;
}
if (fd <= STDERR_FILENO) {
-error_report("fd cannot be a standard I/O stream");
+error_setg(errp, "fd cannot be a standard I/O stream");
return -1;
}
@@ -1074,12 +1074,12 @@ static int parse_add_fd(void *opaque, QemuOpts *opts,
Error **errp)
*/
flags = fcntl(fd, F_GETFD);
if (flags == -1 || (flags & FD_CLOEXEC)) {
-error_report("fd is not valid or already in use");
+error_setg(errp, "fd is not valid or already in use");
return -1;
}
if (fdset_id < 0) {
-error_report("set option is required and must be non-negative");
+error_setg(errp, "set option is required and must be non-negative");
return -1;
}
@@ -1092,7 +1092,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts,
Error **errp)
}
#endif
if (dupfd == -1) {
-error_report("error duplicating fd: %s", strerror(errno));
+error_setg(errp, "error duplicating fd: %s", strerror(errno));
return -1;
}
@@ -3983,15 +3983,11 @@ int main(int argc, char **argv, char **envp)
}
#ifndef _WIN32
-if (qemu_opts_foreach(qemu_find_opts("add-fd"),
- parse_add_fd, NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("add-fd"),
+ parse_add_fd, NULL, &error_fatal);
-if (qemu_opts_foreach(qemu_find_opts("add-fd"),
- cleanup_add_fd, NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("add-fd"),
+ cleanup_add_fd, NULL, &error_fatal);
#endif
current_machine = MACHINE(object_new(object_class_get_name(
--
2.17.1
[Qemu-devel] [PATCH v4 16/38] xen/pt: Fix incomplete conversion to realize()
The conversion of "xen-pci-passthrough" to realize() (commit
5a11d0f7549, v2.6.0) neglected to convert the xen_pt_config_init()
error path. If xen_pt_config_init() fails, xen_pt_realize() reports
the error, then returns success without completing its job. I don't
know the exact impact, but it can't be good.
Belatedly convert the error path.
Fixes: 5a11d0f7549e24a10e178a9dc8ff5e698031d9a6
Cc: Stefano Stabellini
Cc: Anthony Perard
Signed-off-by: Markus Armbruster
Acked-by: Anthony PERARD
---
hw/xen/xen_pt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index e5a6eff44f..f1f3a3727c 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -830,7 +830,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
xen_pt_config_init(s, &err);
if (err) {
error_append_hint(&err, "PCI Config space initialisation failed");
-error_report_err(err);
+error_propagate(errp, err);
rc = -1;
goto err_out;
}
--
2.17.1
[Qemu-devel] [PATCH v4 14/38] net/socket: Fix invalid socket type error handling
Calling error_report() in a function that takes an Error ** argument
is suspicious. net_socket_fd_init() does that, and then fails without
setting an error. Wrong. I didn't analyze how exactly this can
break. A caller that reports the error on failure would crash.
Broken when commit c37f0bb1d0d (v2.11.0) converted the function to
Error. Fix by calling error_setg() instead of error_report().
Fixes: c37f0bb1d0d24e3a6b5f4659bb305913dcb798a6
Cc: Jason Wang
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
---
net/socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 6917fbcbf5..90ef3517be 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -453,8 +453,8 @@ static NetSocketState *net_socket_fd_init(NetClientState
*peer,
case SOCK_STREAM:
return net_socket_fd_init_stream(peer, model, name, fd, is_connected);
default:
-error_report("socket type=%d for fd=%d must be either"
- " SOCK_DGRAM or SOCK_STREAM", so_type, fd);
+error_setg(errp, "socket type=%d for fd=%d must be either"
+ " SOCK_DGRAM or SOCK_STREAM", so_type, fd);
closesocket(fd);
}
return NULL;
--
2.17.1
[Qemu-devel] [PATCH v4 00/38] Replace some unwise uses of error_report() & friends
Calling error_report() or similar in a function that takes an Error ** argument is suspicious. Fix a number of instances that are actually wrong. Clean up a few more that are merely fragile / bad examples. v4: * PATCH 36-37: Unbreak qemu-iotests [Eric] v3: * PATCH 27: Drop useless error message prefix [Gerd] * PATCH 36-38: New v2: * PATCH 01: New * PATCH 02: Commit hash corrected in commit message [Eric] * PATCH 13: Error messages tidied up [Marc-André] * PATCH 15: Commit message tidied up [Eduardo H.] * PATCH 16: Unwanted string split fixed [Marc-André, Eduardo O.] * PATCH 20: Commit message corrected [Marc-André] * PATCH 25-26: New * PATCH 27: Rebased, commit message corrected * PATCH 30: Commit message tidied up [Eric] * PATCH 32: Less confusing commit message, help fixed [Max] * PATCH 34: New Fei Li (1): ui: Convert vnc_display_init(), init_keyboard_layout() to Error Markus Armbruster (37): error: Fix use of error_prepend() with &error_fatal, &error_abort Use error_fatal to simplify obvious fatal errors (again) block: Use warn_report() & friends to report warnings cpus hw target: Use warn_report() & friends to report warnings vfio: Use warn_report() & friends to report warnings vfio: Clean up error reporting after previous commit char: Use error_printf() to print help and such 9pfs: Fix CLI parsing crash on error pc: Fix machine property nvdimm-persistence error handling ioapic: Fix error handling in realize() smbios: Clean up error handling in smbios_add() migration: Fix !replay_can_snapshot() error handling l2tpv3: Improve -netdev/netdev_add/-net/... error reporting net/socket: Fix invalid socket type error handling numa: Fix QMP command set-numa-node error handling xen/pt: Fix incomplete conversion to realize() seccomp: Clean up error reporting in parse_sandbox() vl: Clean up error reporting in parse_add_fd() qom: Clean up error reporting in user_creatable_add_opts_foreach() vl: Clean up error reporting in chardev_init_func() vl: Clean up error reporting in machine_set_property() vl: Clean up error reporting in mon_init_func() vl: Clean up error reporting in parse_fw_cfg() vl: Clean up error reporting in device_init_func() ui/keymaps: Fix handling of erroneous include files vnc: Clean up error reporting in vnc_init_func() numa: Clean up error reporting in parse_numa() tpm: Clean up error reporting in tpm_init_tpmdev() spice: Clean up error reporting in add_channel() fsdev: Clean up error reporting in qemu_fsdev_add() vl: Assert drive_new() does not fail in default_drive() blockdev: Convert drive_new() to Error vl: Fix exit status for -drive format=help vl: Simplify call of parse_name() block: Clean up bdrv_img_create()'s error reporting raw: Convert a warning to warn_report() vpc: Fail open on bad header checksum block.c | 9 +- block/bochs.c| 8 +- block/cloop.c| 8 +- block/dmg.c | 8 +- block/iscsi.c| 2 +- block/qcow2.c| 4 +- block/qed.c | 4 +- block/raw-format.c | 17 +-- block/rbd.c | 12 +- block/sheepdog.c | 2 +- block/vpc.c | 8 +- block/vvfat.c| 8 +- blockdev.c | 27 ++--- chardev/char-pty.c | 2 +- chardev/char.c | 2 +- cpus.c | 8 +- device-hotplug.c | 5 +- fsdev/qemu-fsdev-dummy.c | 2 +- fsdev/qemu-fsdev.c | 12 +- fsdev/qemu-fsdev.h | 2 +- hw/9pfs/9p-handle.c | 6 +- hw/9pfs/9p-local.c | 4 +- hw/9pfs/xen-9p-backend.c | 7 +- hw/display/cg3.c | 2 +- hw/display/tcx.c | 2 +- hw/i386/pc.c | 5 +- hw/intc/ioapic.c | 8 +- hw/intc/xics.c | 15 ++- hw/intc/xics_kvm.c | 7 +- hw/misc/ivshmem.c| 4 +- hw/net/virtio-net.c | 8 +- hw/ppc/pnv_core.c| 4 +- hw/ppc/spapr_pci.c | 7 +- hw/smbios/smbios.c | 90 ++- hw/timer/aspeed_timer.c | 3 +- hw/usb/bus.c | 5 +- hw/vfio/pci-quirks.c | 4 +- hw/vfio/pci.c| 25 ++-- hw/vfio/platform.c | 6 +- hw/virtio/virtio-pci.c | 4 +- hw/xen/xen_pt.c | 2 +- include/hw/vfio/vfio-common.h
[Qemu-devel] [PATCH v4 06/38] vfio: Clean up error reporting after previous commit
The previous commit changed vfio's warning messages from
vfio warning: DEV-NAME: Could not frobnicate
to
warning: vfio DEV-NAME: Could not frobnicate
To match this change, change error messages from
vfio error: DEV-NAME: On fire
to
vfio DEV-NAME: On fire
Note the loss of "error". If we think marking error messages that way
is a good idea, we should mark *all* error messages, i.e. make
error_report() print it.
Cc: Alex Williamson
Signed-off-by: Markus Armbruster
Acked-by: Alex Williamson
---
hw/vfio/pci-quirks.c | 4 ++--
hw/vfio/pci.c | 8
hw/vfio/platform.c| 2 +-
include/hw/vfio/vfio-common.h | 1 -
4 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 481fd08df7..eae31c74d6 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1670,7 +1670,7 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice
*vdev, int nr)
* but also no point in us enabling VGA if disabled in hardware.
*/
if (!(gmch & 0x2) && !vdev->vga && vfio_populate_vga(vdev, &err)) {
-error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
+error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
error_report("IGD device %s failed to enable VGA access, "
"legacy mode disabled", vdev->vbasedev.name);
goto out;
@@ -1696,7 +1696,7 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice
*vdev, int nr)
ret = vfio_pci_igd_opregion_init(vdev, opregion, &err);
if (ret) {
error_append_hint(&err, "IGD legacy mode disabled\n");
-error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
+error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
goto out;
}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 0c3d245932..5c7bd96984 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -745,7 +745,7 @@ static void vfio_msi_disable_common(VFIOPCIDevice *vdev)
vfio_intx_enable(vdev, &err);
if (err) {
-error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
+error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
}
}
@@ -2196,7 +2196,7 @@ static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
vfio_intx_enable(vdev, &err);
if (err) {
-error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
+error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
}
for (nr = 0; nr < PCI_NUM_REGIONS - 1; ++nr) {
@@ -2830,7 +2830,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
if (stat(vdev->vbasedev.sysfsdev, &st) < 0) {
error_setg_errno(errp, errno, "no such host device");
-error_prepend(errp, ERR_PREFIX, vdev->vbasedev.sysfsdev);
+error_prepend(errp, VFIO_MSG_PREFIX, vdev->vbasedev.sysfsdev);
return;
}
@@ -3085,7 +3085,7 @@ out_teardown:
vfio_teardown_msi(vdev);
vfio_bars_exit(vdev);
error:
-error_prepend(errp, ERR_PREFIX, vdev->vbasedev.name);
+error_prepend(errp, VFIO_MSG_PREFIX, vdev->vbasedev.name);
}
static void vfio_instance_finalize(Object *obj)
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index baf236ae79..398db38f14 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -690,7 +690,7 @@ out:
}
if (vdev->vbasedev.name) {
-error_prepend(errp, ERR_PREFIX, vdev->vbasedev.name);
+error_prepend(errp, VFIO_MSG_PREFIX, vdev->vbasedev.name);
} else {
error_prepend(errp, "vfio error: ");
}
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 1d7630345b..1b434d02f6 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -31,7 +31,6 @@
#include
#endif
-#define ERR_PREFIX "vfio error: %s: "
#define VFIO_MSG_PREFIX "vfio %s: "
enum {
--
2.17.1
[Qemu-devel] [PATCH v4 24/38] vl: Clean up error reporting in device_init_func()
Calling error_report() in a function that takes an Error ** argument
is suspicious. device_init_func() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
vl.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index c5bc54f72b..c053117028 100644
--- a/vl.c
+++ b/vl.c
@@ -,12 +,10 @@ static int device_help_func(void *opaque, QemuOpts
*opts, Error **errp)
static int device_init_func(void *opaque, QemuOpts *opts, Error **errp)
{
-Error *err = NULL;
DeviceState *dev;
-dev = qdev_device_add(opts, &err);
+dev = qdev_device_add(opts, errp);
if (!dev) {
-error_report_err(err);
return -1;
}
object_unref(OBJECT(dev));
@@ -4491,10 +4489,8 @@ int main(int argc, char **argv, char **envp)
/* init generic devices */
rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
-if (qemu_opts_foreach(qemu_find_opts("device"),
- device_init_func, NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("device"),
+ device_init_func, NULL, &error_fatal);
cpu_synchronize_all_post_init();
--
2.17.1
[Qemu-devel] [PATCH v4 02/38] Use error_fatal to simplify obvious fatal errors (again)
Add a slight improvement of the Coccinelle semantic patch from commit
007b06578ab, and use it to clean up. It leaves dead Error * variables
behind, cleaned up manually.
Cc: David Gibson
Cc: Alexander Graf
Cc: Eric Blake
Cc: Paolo Bonzini
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Acked-by: David Gibson
---
hw/intc/xics_kvm.c | 7 +--
qemu-nbd.c | 6 +-
scripts/coccinelle/use-error_fatal.cocci | 20
vl.c | 7 +--
4 files changed, 23 insertions(+), 17 deletions(-)
create mode 100644 scripts/coccinelle/use-error_fatal.cocci
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 30c3769a20..e8fa9a53ae 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -198,17 +198,12 @@ static void ics_get_kvm_state(ICSState *ics)
{
uint64_t state;
int i;
-Error *local_err = NULL;
for (i = 0; i < ics->nr_irqs; i++) {
ICSIRQState *irq = &ics->irqs[i];
kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
- i + ics->offset, &state, false, &local_err);
-if (local_err) {
-error_report_err(local_err);
-exit(1);
-}
+ i + ics->offset, &state, false, &error_fatal);
irq->server = state & KVM_XICS_DESTINATION_MASK;
irq->saved_priority = (state >> KVM_XICS_PRIORITY_SHIFT)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index e76fe3082a..7874bc973c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1002,11 +1002,7 @@ int main(int argc, char **argv)
}
exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, nbd_export_closed,
- writethrough, NULL, &local_err);
-if (!exp) {
-error_report_err(local_err);
-exit(EXIT_FAILURE);
-}
+ writethrough, NULL, &error_fatal);
nbd_export_set_name(exp, export_name);
nbd_export_set_description(exp, export_description);
diff --git a/scripts/coccinelle/use-error_fatal.cocci
b/scripts/coccinelle/use-error_fatal.cocci
new file mode 100644
index 00..10fff0aec4
--- /dev/null
+++ b/scripts/coccinelle/use-error_fatal.cocci
@@ -0,0 +1,20 @@
+@@
+type T;
+identifier FUN, RET;
+expression list ARGS;
+expression ERR, EC, FAIL;
+@@
+(
+-T RET = FUN(ARGS, &ERR);
++T RET = FUN(ARGS, &error_fatal);
+|
+-RET = FUN(ARGS, &ERR);
++RET = FUN(ARGS, &error_fatal);
+|
+-FUN(ARGS, &ERR);
++FUN(ARGS, &error_fatal);
+)
+-if (FAIL) {
+-error_report_err(ERR);
+-exit(EC);
+-}
diff --git a/vl.c b/vl.c
index 4e25c78bff..ff50199e64 100644
--- a/vl.c
+++ b/vl.c
@@ -2002,15 +2002,10 @@ static void select_vgahw(const char *p)
static void parse_display_qapi(const char *optarg)
{
-Error *err = NULL;
DisplayOptions *opts;
Visitor *v;
-v = qobject_input_visitor_new_str(optarg, "type", &err);
-if (!v) {
-error_report_err(err);
-exit(1);
-}
+v = qobject_input_visitor_new_str(optarg, "type", &error_fatal);
visit_type_DisplayOptions(v, NULL, &opts, &error_fatal);
QAPI_CLONE_MEMBERS(DisplayOptions, &dpy, opts);
--
2.17.1
[Qemu-devel] [PATCH v4 21/38] vl: Clean up error reporting in machine_set_property()
Calling error_report() in a function that takes an Error ** argument
is suspicious. machine_set_property() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
---
vl.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/vl.c b/vl.c
index aa03192019..c3c17106bf 100644
--- a/vl.c
+++ b/vl.c
@@ -2676,7 +2676,7 @@ static int machine_set_property(void *opaque,
g_free(qom_name);
if (local_err) {
-error_report_err(local_err);
+error_propagate(errp, local_err);
return -1;
}
@@ -4248,11 +4248,8 @@ int main(int argc, char **argv, char **envp)
}
machine_opts = qemu_get_machine_opts();
-if (qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
- NULL)) {
-object_unref(OBJECT(current_machine));
-exit(1);
-}
+qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
+ &error_fatal);
configure_accelerator(current_machine);
--
2.17.1
[Qemu-devel] [PATCH v4 30/38] spice: Clean up error reporting in add_channel()
Calling error_report() in a function that takes an Error ** argument
is suspicious. add_channel() does that, and then exit()s. Its caller
main(), via qemu_opts_foreach(), is fine with it, but clean it up
anyway.
Cc: Gerd Hoffmann
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
---
ui/spice-core.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a4fbbc3898..ebaae24643 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -597,9 +597,9 @@ static int add_channel(void *opaque, const char *name,
const char *value,
if (strcmp(name, "tls-channel") == 0) {
int *tls_port = opaque;
if (!*tls_port) {
-error_report("spice: tried to setup tls-channel"
- " without specifying a TLS port");
-exit(1);
+error_setg(errp, "spice: tried to setup tls-channel"
+ " without specifying a TLS port");
+return -1;
}
security = SPICE_CHANNEL_SECURITY_SSL;
}
@@ -615,8 +615,9 @@ static int add_channel(void *opaque, const char *name,
const char *value,
rc = spice_server_set_channel_security(spice_server, value, security);
}
if (rc != 0) {
-error_report("spice: failed to set channel security for %s", value);
-exit(1);
+error_setg(errp, "spice: failed to set channel security for %s",
+ value);
+return -1;
}
return 0;
}
@@ -787,7 +788,7 @@ void qemu_spice_init(void)
spice_server_set_playback_compression
(spice_server, qemu_opt_get_bool(opts, "playback-compression", 1));
-qemu_opt_foreach(opts, add_channel, &tls_port, NULL);
+qemu_opt_foreach(opts, add_channel, &tls_port, &error_fatal);
spice_server_set_name(spice_server, qemu_name);
spice_server_set_uuid(spice_server, (unsigned char *)&qemu_uuid);
--
2.17.1
[Qemu-devel] [PATCH v4 22/38] vl: Clean up error reporting in mon_init_func()
Calling error_report() in a function that takes an Error ** argument
is suspicious. mon_init_func() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
---
vl.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/vl.c b/vl.c
index c3c17106bf..cd8e8a6586 100644
--- a/vl.c
+++ b/vl.c
@@ -2270,8 +2270,8 @@ static int mon_init_func(void *opaque, QemuOpts *opts,
Error **errp)
} else if (strcmp(mode, "control") == 0) {
flags = MONITOR_USE_CONTROL;
} else {
-error_report("unknown monitor mode \"%s\"", mode);
-exit(1);
+error_setg(errp, "unknown monitor mode \"%s\"", mode);
+return -1;
}
if (qemu_opt_get_bool(opts, "pretty", 0))
@@ -2285,8 +2285,8 @@ static int mon_init_func(void *opaque, QemuOpts *opts,
Error **errp)
chardev = qemu_opt_get(opts, "chardev");
chr = qemu_chr_find(chardev);
if (chr == NULL) {
-error_report("chardev \"%s\" not found", chardev);
-exit(1);
+error_setg(errp, "chardev \"%s\" not found", chardev);
+return -1;
}
monitor_init(chr, flags);
@@ -4412,10 +4412,8 @@ int main(int argc, char **argv, char **envp)
default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
-if (qemu_opts_foreach(qemu_find_opts("mon"),
- mon_init_func, NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("mon"),
+ mon_init_func, NULL, &error_fatal);
if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)
exit(1);
--
2.17.1
[Qemu-devel] [PATCH v4 11/38] smbios: Clean up error handling in smbios_add()
Calling error_report() in a function that takes an Error ** argument
is suspicious. smbios_entry_add() does that, and then exit()s. It
also passes &error_fatal to qemu_opts_validate(). Both wrong, but
currently harmless, as its only caller passes &error_fatal. Messed up
in commit 1007a37e208. Clean it up.
Cc: Paolo Bonzini
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Acked-by: Paolo Bonzini
---
hw/smbios/smbios.c | 90 +++---
1 file changed, 62 insertions(+), 28 deletions(-)
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index a27e54b2fa..920939454e 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -950,6 +950,7 @@ static void save_opt_list(size_t *ndest, const char ***dest,
void smbios_entry_add(QemuOpts *opts, Error **errp)
{
+Error *err = NULL;
const char *val;
assert(!smbios_immutable);
@@ -960,12 +961,16 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
int size;
struct smbios_table *table; /* legacy mode only */
-qemu_opts_validate(opts, qemu_smbios_file_opts, &error_fatal);
+qemu_opts_validate(opts, qemu_smbios_file_opts, &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
size = get_image_size(val);
if (size == -1 || size < sizeof(struct smbios_structure_header)) {
-error_report("Cannot read SMBIOS file %s", val);
-exit(1);
+error_setg(errp, "Cannot read SMBIOS file %s", val);
+return;
}
/*
@@ -978,14 +983,15 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
smbios_tables_len);
if (load_image(val, (uint8_t *)header) != size) {
-error_report("Failed to load SMBIOS file %s", val);
-exit(1);
+error_setg(errp, "Failed to load SMBIOS file %s", val);
+return;
}
if (test_bit(header->type, have_fields_bitmap)) {
-error_report("can't load type %d struct, fields already
specified!",
- header->type);
-exit(1);
+error_setg(errp,
+ "can't load type %d struct, fields already specified!",
+ header->type);
+return;
}
set_bit(header->type, have_binfile_bitmap);
@@ -1030,19 +1036,23 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
unsigned long type = strtoul(val, NULL, 0);
if (type > SMBIOS_MAX_TYPE) {
-error_report("out of range!");
-exit(1);
+error_setg(errp, "out of range!");
+return;
}
if (test_bit(type, have_binfile_bitmap)) {
-error_report("can't add fields, binary file already loaded!");
-exit(1);
+error_setg(errp, "can't add fields, binary file already loaded!");
+return;
}
set_bit(type, have_fields_bitmap);
switch (type) {
case 0:
-qemu_opts_validate(opts, qemu_smbios_type0_opts, &error_fatal);
+qemu_opts_validate(opts, qemu_smbios_type0_opts, &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
save_opt(&type0.vendor, opts, "vendor");
save_opt(&type0.version, opts, "version");
save_opt(&type0.date, opts, "date");
@@ -1051,14 +1061,18 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
val = qemu_opt_get(opts, "release");
if (val) {
if (sscanf(val, "%hhu.%hhu", &type0.major, &type0.minor) != 2)
{
-error_report("Invalid release");
-exit(1);
+error_setg(errp, "Invalid release");
+return;
}
type0.have_major_minor = true;
}
return;
case 1:
-qemu_opts_validate(opts, qemu_smbios_type1_opts, &error_fatal);
+qemu_opts_validate(opts, qemu_smbios_type1_opts, &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
save_opt(&type1.manufacturer, opts, "manufacturer");
save_opt(&type1.product, opts, "product");
save_opt(&type1.version, opts, "version");
@@ -1069,14 +1083,18 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
val = qemu_opt_get(opts, "uuid");
if (val) {
if (qemu_uuid_parse(val, &qemu_uuid) != 0) {
-error_report("Invalid UUID");
-exit(1);
+error_setg(errp, "Invalid UUID");
+return;
}
qemu_uuid_set = true;
}
return;
case 2:
-qemu_opts_validate
[Qemu-devel] [PATCH v4 35/38] vl: Simplify call of parse_name()
main() checks for parse_name() failure even though it can't actually
fail. That's okay. Simplify it to check by passing &error_fatal,
like the other users of qemu_opts_foreach().
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
vl.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index 89520d8007..01d71371c2 100644
--- a/vl.c
+++ b/vl.c
@@ -3973,10 +3973,8 @@ int main(int argc, char **argv, char **envp)
}
#endif
-if (qemu_opts_foreach(qemu_find_opts("name"),
- parse_name, NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("name"),
+ parse_name, NULL, &error_fatal);
#ifndef _WIN32
qemu_opts_foreach(qemu_find_opts("add-fd"),
--
2.17.1
[Qemu-devel] [PATCH v4 33/38] blockdev: Convert drive_new() to Error
Calling error_report() from within a function that takes an Error **
argument is suspicious. drive_new() calls error_report() even though
it can run within drive_init_func(), which takes an Error ** argument.
drive_init_func()'s caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway:
* Convert drive_new() to Error
* Update add_init_drive() to report the error received from
drive_new()
* Make main() pass &error_fatal through qemu_opts_foreach(),
drive_init_func() to drive_new()
* Make default_drive() pass &error_abort through qemu_opts_foreach(),
drive_init_func() to drive_new()
Cc: Kevin Wolf
Cc: Max Reitz
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Reviewed-by: Max Reitz
Reviewed-by: Philippe Mathieu-Daudé
---
blockdev.c| 27 ++-
device-hotplug.c | 5 -
include/sysemu/blockdev.h | 3 ++-
vl.c | 8
4 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index a8755bd908..574adbcb7f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -759,7 +759,8 @@ QemuOptsList qemu_legacy_drive_opts = {
},
};
-DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
+DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type,
+ Error **errp)
{
const char *value;
BlockBackend *blk;
@@ -808,7 +809,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
qemu_opt_rename(all_opts, opt_renames[i].from, opt_renames[i].to,
&local_err);
if (local_err) {
-error_report_err(local_err);
+error_propagate(errp, local_err);
return NULL;
}
}
@@ -819,7 +820,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
bool writethrough;
if (bdrv_parse_cache_mode(value, &flags, &writethrough) != 0) {
-error_report("invalid cache option");
+error_setg(errp, "invalid cache option");
return NULL;
}
@@ -847,7 +848,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
&error_abort);
qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err);
if (local_err) {
-error_report_err(local_err);
+error_propagate(errp, local_err);
goto fail;
}
@@ -860,7 +861,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
media = MEDIA_CDROM;
read_only = true;
} else {
-error_report("'%s' invalid media", value);
+error_setg(errp, "'%s' invalid media", value);
goto fail;
}
}
@@ -885,7 +886,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
type++) {
}
if (type == IF_COUNT) {
-error_report("unsupported bus type '%s'", value);
+error_setg(errp, "unsupported bus type '%s'", value);
goto fail;
}
} else {
@@ -902,7 +903,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
if (index != -1) {
if (bus_id != 0 || unit_id != -1) {
-error_report("index cannot be used with bus and unit");
+error_setg(errp, "index cannot be used with bus and unit");
goto fail;
}
bus_id = drive_index_to_bus_id(type, index);
@@ -921,13 +922,13 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
}
if (max_devs && unit_id >= max_devs) {
-error_report("unit %d too big (max is %d)", unit_id, max_devs - 1);
+error_setg(errp, "unit %d too big (max is %d)", unit_id, max_devs - 1);
goto fail;
}
if (drive_get(type, bus_id, unit_id) != NULL) {
-error_report("drive with bus=%d, unit=%d (index=%d) exists",
- bus_id, unit_id, index);
+error_setg(errp, "drive with bus=%d, unit=%d (index=%d) exists",
+ bus_id, unit_id, index);
goto fail;
}
@@ -970,7 +971,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
if (werror != NULL) {
if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO &&
type != IF_NONE) {
-error_report("werror is not supported by this bus type");
+error_setg(errp, "werror is not supported by this bus type");
goto fail;
}
qdict_put_str(bs_opts, "werror", werror);
@@ -980,7 +981,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType
block_default_type)
if (rerror != NULL) {
if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI &&
type != IF_NONE) {
-error_report("rerror is not suppor
[Qemu-devel] [PATCH v4 34/38] vl: Fix exit status for -drive format=help
Signed-off-by: Markus Armbruster
---
vl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 22beca29d1..89520d8007 100644
--- a/vl.c
+++ b/vl.c
@@ -4397,7 +4397,7 @@ int main(int argc, char **argv, char **envp)
if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
&machine_class->block_default_type, &error_fatal)) {
/* We printed help */
-exit(1);
+exit(0);
}
default_drive(default_cdrom, snapshot, machine_class->block_default_type,
2,
--
2.17.1
[Qemu-devel] [PATCH v4 07/38] char: Use error_printf() to print help and such
Calling error_report() in a function that takes an Error ** argument
is suspicious. Convert a few that are actually help and such to
error_printf().
Improves output of -chardev help from
qemu-system-x86_64: -chardev help: Available chardev backend types:
serial
...
to
Available chardev backend types:
serial
...
Cc: Paolo Bonzini
Cc: "Marc-André Lureau"
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
chardev/char-pty.c | 2 +-
chardev/char.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index e8d9a53476..f681d637c1 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -259,7 +259,7 @@ static void char_pty_open(Chardev *chr,
qemu_set_nonblock(master_fd);
chr->filename = g_strdup_printf("pty:%s", pty_name);
-error_report("char device redirected to %s (label %s)",
+error_printf("char device redirected to %s (label %s)\n",
pty_name, chr->label);
s = PTY_CHARDEV(chr);
diff --git a/chardev/char.c b/chardev/char.c
index e115166995..7f07a1bfbd 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -634,7 +634,7 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error
**errp)
chardev_name_foreach(help_string_append, str);
-error_report("Available chardev backend types: %s", str->str);
+error_printf("Available chardev backend types: %s\n", str->str);
g_string_free(str, true);
return NULL;
}
--
2.17.1
[Qemu-devel] [PATCH v4 15/38] numa: Fix QMP command set-numa-node error handling
Calling error_report() in a function that takes an Error ** argument
is suspicious. parse_numa_node() does that, and then exit()s. It
also passes &error_fatal to machine_set_cpu_numa_node(). Both wrong.
Attempting to configure numa when the machine doesn't support it kills
the VM:
$ qemu-system-x86_64 -nodefaults -S -display none -M none -preconfig -qmp
stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3},
"package": "v3.0.0-837-gc5e4e49258"}, "capabilities": []}}
{"execute": "qmp_capabilities"}
{"return": {}}
{"execute": "set-numa-node", "arguments": {"type": "node"}}
NUMA is not supported by this machine-type
$ echo $?
1
Messed up when commit 64c2a8f6d3f and 7c88e65d9e9 (v2.10.0) added
incorrect error handling right next to correct examples. Latent bug
until commit f3be67812c2 (v3.0.0) made it accessible via QMP. Fairly
harmless in practice, because it's limited to RUN_STATE_PRECONFIG.
The fix is obvious: replace error_report(); exit() by error_setg();
return.
This affects parse_numa_node()'s other caller
numa_complete_configuration(): since it ignores errors, the "NUMA is
not supported by this machine-type" is now ignored, too. But that
error is as unexpected there as any other. Change it to abort on
error instead.
Fixes: f3be67812c226162f86ce92634bd913714445420
Cc: Igor Mammedov
Signed-off-by: Markus Armbruster
Reviewed-by: Igor Mammedov
---
numa.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/numa.c b/numa.c
index 81542d4ebb..1d7c49ad43 100644
--- a/numa.c
+++ b/numa.c
@@ -60,6 +60,7 @@ NodeInfo numa_info[MAX_NODES];
static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
Error **errp)
{
+Error *err = NULL;
uint16_t nodenr;
uint16List *cpus = NULL;
MachineClass *mc = MACHINE_GET_CLASS(ms);
@@ -82,8 +83,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions
*node,
}
if (!mc->cpu_index_to_instance_props || !mc->get_default_cpu_node_id) {
-error_report("NUMA is not supported by this machine-type");
-exit(1);
+error_setg(errp, "NUMA is not supported by this machine-type");
+return;
}
for (cpus = node->cpus; cpus; cpus = cpus->next) {
CpuInstanceProperties props;
@@ -97,7 +98,11 @@ static void parse_numa_node(MachineState *ms,
NumaNodeOptions *node,
props = mc->cpu_index_to_instance_props(ms, cpus->value);
props.node_id = nodenr;
props.has_node_id = true;
-machine_set_cpu_numa_node(ms, &props, &error_fatal);
+machine_set_cpu_numa_node(ms, &props, &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
}
if (node->has_mem && node->has_memdev) {
@@ -367,7 +372,7 @@ void numa_complete_configuration(MachineState *ms)
if (ms->ram_slots > 0 && nb_numa_nodes == 0 &&
mc->auto_enable_numa_with_memhp) {
NumaNodeOptions node = { };
-parse_numa_node(ms, &node, NULL);
+parse_numa_node(ms, &node, &error_abort);
}
assert(max_numa_nodeid <= MAX_NODES);
--
2.17.1
[Qemu-devel] [PATCH v4 09/38] pc: Fix machine property nvdimm-persistence error handling
Calling error_report() in a function that takes an Error ** argument
is suspicious. pc.c's pc_machine_set_nvdimm_persistence() does that,
and then exit()s. Wrong. Attempting to set machine property
nvdimm-persistence to a bad value instantly kills the VM:
$ qemu-system-x86_64 -nodefaults -S -display none -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3},
"package": "v3.0.0-837-gc5e4e49258"}, "capabilities": []}}
{"execute": "qmp_capabilities"}
{"return": {}}
{"execute": "qom-set", "arguments": {"path": "/machine", "property":
"nvdimm-persistence", "value": "instadeath"}}
-machine nvdimm-persistence=instadeath: unsupported option
$ echo $?
1
Broken when commit 11c39b5cd96 (v3.0.0) replaced error_propagate();
return by error_report(); exit() instead of error_setg(); return. Fix
that.
Fixes: 11c39b5cd966ddc067a1ca0c5392ec9b666c45b7
Cc: "Michael S. Tsirkin"
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
---
hw/i386/pc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index cd5029c149..eab8572f2a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2209,8 +2209,9 @@ static void pc_machine_set_nvdimm_persistence(Object
*obj, const char *value,
else if (strcmp(value, "mem-ctrl") == 0)
nvdimm_state->persistence = 2;
else {
-error_report("-machine nvdimm-persistence=%s: unsupported option",
value);
-exit(EXIT_FAILURE);
+error_setg(errp, "-machine nvdimm-persistence=%s: unsupported option",
+ value);
+return;
}
g_free(nvdimm_state->persistence_string);
--
2.17.1
[Qemu-devel] [PATCH v4 13/38] l2tpv3: Improve -netdev/netdev_add/-net/... error reporting
When -netdev l2tpv3 fails, it first reports a specific error, then a
generic one, like this:
$ qemu-system-x86_64 -netdev l2tpv3,id=foo,src=,dst=,txsession=1
qemu-system-x86_64: -netdev l2tpv3,id=foo,src=,dst=,txsession=1:
l2tpv3_open : could not resolve src, errno = Name or service not known
qemu-system-x86_64: Device 'l2tpv3' could not be initialized
With the command line, the messages go to stderr. In HMP, they go to
the monitor. In QMP, the second one becomes the error reply, and the
first one goes to stderr.
Convert net_init_tap() to Error. This suppresses the unwanted second
message, and makes the specific error the QMP error reply.
Cc: Jason Wang
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
---
net/l2tpv3.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 6745b78990..81db24dc8c 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -28,6 +28,7 @@
#include
#include "net/net.h"
#include "clients.h"
+#include "qapi/error.h"
#include "qemu-common.h"
#include "qemu/error-report.h"
#include "qemu/option.h"
@@ -528,7 +529,6 @@ int net_init_l2tpv3(const Netdev *netdev,
const char *name,
NetClientState *peer, Error **errp)
{
-/* FIXME error_setg(errp, ...) on failure */
const NetdevL2TPv3Options *l2tpv3;
NetL2TPV3State *s;
NetClientState *nc;
@@ -555,7 +555,7 @@ int net_init_l2tpv3(const Netdev *netdev,
}
if ((l2tpv3->has_offset) && (l2tpv3->offset > 256)) {
-error_report("l2tpv3_open : offset must be less than 256 bytes");
+error_setg(errp, "offset must be less than 256 bytes");
goto outerr;
}
@@ -563,6 +563,8 @@ int net_init_l2tpv3(const Netdev *netdev,
if (l2tpv3->has_rxcookie && l2tpv3->has_txcookie) {
s->cookie = true;
} else {
+error_setg(errp,
+ "require both 'rxcookie' and 'txcookie' or neither");
goto outerr;
}
} else {
@@ -578,7 +580,7 @@ int net_init_l2tpv3(const Netdev *netdev,
if (l2tpv3->has_udp && l2tpv3->udp) {
s->udp = true;
if (!(l2tpv3->has_srcport && l2tpv3->has_dstport)) {
-error_report("l2tpv3_open : need both src and dst port for udp");
+error_setg(errp, "need both src and dst port for udp");
goto outerr;
} else {
srcport = l2tpv3->srcport;
@@ -639,20 +641,19 @@ int net_init_l2tpv3(const Netdev *netdev,
gairet = getaddrinfo(l2tpv3->src, srcport, &hints, &result);
if ((gairet != 0) || (result == NULL)) {
-error_report(
-"l2tpv3_open : could not resolve src, errno = %s",
-gai_strerror(gairet)
-);
+error_setg(errp, "could not resolve src, errno = %s",
+ gai_strerror(gairet));
goto outerr;
}
fd = socket(result->ai_family, result->ai_socktype, result->ai_protocol);
if (fd == -1) {
fd = -errno;
-error_report("l2tpv3_open : socket creation failed, errno = %d", -fd);
+error_setg(errp, "socket creation failed, errno = %d",
+ -fd);
goto outerr;
}
if (bind(fd, (struct sockaddr *) result->ai_addr, result->ai_addrlen)) {
-error_report("l2tpv3_open : could not bind socket err=%i", errno);
+error_setg(errp, "could not bind socket err=%i", errno);
goto outerr;
}
if (result) {
@@ -677,10 +678,8 @@ int net_init_l2tpv3(const Netdev *netdev,
result = NULL;
gairet = getaddrinfo(l2tpv3->dst, dstport, &hints, &result);
if ((gairet != 0) || (result == NULL)) {
-error_report(
-"l2tpv3_open : could not resolve dst, error = %s",
-gai_strerror(gairet)
-);
+error_setg(errp, "could not resolve dst, error = %s",
+ gai_strerror(gairet));
goto outerr;
}
--
2.17.1
[Qemu-devel] [PATCH v4 32/38] vl: Assert drive_new() does not fail in default_drive()
If creating (empty) default drives fails, it's a bug. Therefore,
assert() is more appropriate than exit(1).
Cc: Kevin Wolf
Cc: Max Reitz
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Reviewed-by: Max Reitz
---
vl.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/vl.c b/vl.c
index 2a8ef05aef..65366b961e 100644
--- a/vl.c
+++ b/vl.c
@@ -1156,9 +1156,7 @@ static void default_drive(int enable, int snapshot,
BlockInterfaceType type,
}
dinfo = drive_new(opts, type);
-if (!dinfo) {
-exit(1);
-}
+assert(dinfo);
dinfo->is_default = true;
}
--
2.17.1
[Qemu-devel] [PATCH v4 31/38] fsdev: Clean up error reporting in qemu_fsdev_add()
Calling error_report() from within a function that takes an Error **
argument is suspicious. qemu_fsdev_add() does that, and its caller
fsdev_init_func() then fails without setting an error. Its caller
main(), via qemu_opts_foreach(), is fine with it, but clean it up
anyway.
Cc: Greg Kurz
Signed-off-by: Markus Armbruster
Acked-by: Greg Kurz
---
fsdev/qemu-fsdev-dummy.c | 2 +-
fsdev/qemu-fsdev.c | 12 +---
fsdev/qemu-fsdev.h | 2 +-
hw/9pfs/xen-9p-backend.c | 7 ++-
vl.c | 8 +++-
5 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 6dc0fbc4c4..489cd29081 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -15,7 +15,7 @@
#include "qemu/config-file.h"
#include "qemu/module.h"
-int qemu_fsdev_add(QemuOpts *opts)
+int qemu_fsdev_add(QemuOpts *opts, Error **errp)
{
return 0;
}
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 8a4afbffbd..7a3b87cc9e 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -30,7 +30,7 @@ static FsDriverTable FsDrivers[] = {
{ .name = "proxy", .ops = &proxy_ops},
};
-int qemu_fsdev_add(QemuOpts *opts)
+int qemu_fsdev_add(QemuOpts *opts, Error **errp)
{
int i;
struct FsDriverListEntry *fsle;
@@ -38,10 +38,9 @@ int qemu_fsdev_add(QemuOpts *opts)
const char *fsdriver = qemu_opt_get(opts, "fsdriver");
const char *writeout = qemu_opt_get(opts, "writeout");
bool ro = qemu_opt_get_bool(opts, "readonly", 0);
-Error *local_err = NULL;
if (!fsdev_id) {
-error_report("fsdev: No id specified");
+error_setg(errp, "fsdev: No id specified");
return -1;
}
@@ -53,11 +52,11 @@ int qemu_fsdev_add(QemuOpts *opts)
}
if (i == ARRAY_SIZE(FsDrivers)) {
-error_report("fsdev: fsdriver %s not found", fsdriver);
+error_setg(errp, "fsdev: fsdriver %s not found", fsdriver);
return -1;
}
} else {
-error_report("fsdev: No fsdriver specified");
+error_setg(errp, "fsdev: No fsdriver specified");
return -1;
}
@@ -76,8 +75,7 @@ int qemu_fsdev_add(QemuOpts *opts)
}
if (fsle->fse.ops->parse_opts) {
-if (fsle->fse.ops->parse_opts(opts, &fsle->fse, &local_err)) {
-error_report_err(local_err);
+if (fsle->fse.ops->parse_opts(opts, &fsle->fse, errp)) {
g_free(fsle->fse.fsdev_id);
g_free(fsle);
return -1;
diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
index 65e4b1cfab..d9716b4144 100644
--- a/fsdev/qemu-fsdev.h
+++ b/fsdev/qemu-fsdev.h
@@ -38,7 +38,7 @@ typedef struct FsDriverListEntry {
QTAILQ_ENTRY(FsDriverListEntry) next;
} FsDriverListEntry;
-int qemu_fsdev_add(QemuOpts *opts);
+int qemu_fsdev_add(QemuOpts *opts, Error **errp);
FsDriverEntry *get_fsdev_fsentry(char *id);
extern FileOperations local_ops;
extern FileOperations handle_ops;
diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 6026780f95..3f54a21c76 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -14,6 +14,7 @@
#include "hw/9pfs/9p.h"
#include "hw/xen/xen_backend.h"
#include "hw/9pfs/xen-9pfs.h"
+#include "qapi/error.h"
#include "qemu/config-file.h"
#include "qemu/option.h"
#include "fsdev/qemu-fsdev.h"
@@ -355,6 +356,7 @@ static int xen_9pfs_free(struct XenDevice *xendev)
static int xen_9pfs_connect(struct XenDevice *xendev)
{
+Error *err = NULL;
int i;
Xen9pfsDev *xen_9pdev = container_of(xendev, Xen9pfsDev, xendev);
V9fsState *s = &xen_9pdev->state;
@@ -452,7 +454,10 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
qemu_opt_set(fsdev, "path", xen_9pdev->path, NULL);
qemu_opt_set(fsdev, "security_model", xen_9pdev->security_model, NULL);
qemu_opts_set_id(fsdev, s->fsconf.fsdev_id);
-qemu_fsdev_add(fsdev);
+qemu_fsdev_add(fsdev, &err);
+if (err) {
+error_report_err(err);
+}
v9fs_device_realize_common(s, &xen_9p_transport, NULL);
return 0;
diff --git a/vl.c b/vl.c
index abfe991ed6..2a8ef05aef 100644
--- a/vl.c
+++ b/vl.c
@@ -2249,7 +2249,7 @@ static int chardev_init_func(void *opaque, QemuOpts
*opts, Error **errp)
#ifdef CONFIG_VIRTFS
static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
{
-return qemu_fsdev_add(opts);
+return qemu_fsdev_add(opts, errp);
}
#endif
@@ -4235,10 +4235,8 @@ int main(int argc, char **argv, char **envp)
chardev_init_func, NULL, &error_fatal);
#ifdef CONFIG_VIRTFS
-if (qemu_opts_foreach(qemu_find_opts("fsdev"),
- fsdev_init_func, NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("fsdev"),
+ fsdev_init_func, NULL, &error_fatal);
#endif
if (qemu_opts_foreach(qemu_find_opts("device"),
--
2.17.1
[Qemu-devel] [PATCH v4 26/38] ui: Convert vnc_display_init(), init_keyboard_layout() to Error
From: Fei Li
Signed-off-by: Fei Li
Cc: Gerd Hoffmann
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
include/ui/console.h | 2 +-
ui/curses.c | 6 +++---
ui/keymaps.c | 11 ++-
ui/keymaps.h | 2 +-
ui/sdl.c | 6 +++---
ui/vnc.c | 15 ++-
6 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index fb969caf70..c17803c530 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
/* vnc.c */
-void vnc_display_init(const char *id);
+void vnc_display_init(const char *id, Error **errp);
void vnc_display_open(const char *id, Error **errp);
void vnc_display_add_client(const char *id, int csock, bool skipauth);
int vnc_display_password(const char *id, const char *password);
diff --git a/ui/curses.c b/ui/curses.c
index 59d819fd4d..f4e7a12f74 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -28,6 +28,7 @@
#include
#endif
+#include "qapi/error.h"
#include "qemu-common.h"
#include "ui/console.h"
#include "ui/input.h"
@@ -421,9 +422,8 @@ static void curses_keyboard_setup(void)
keyboard_layout = "en-us";
#endif
if(keyboard_layout) {
-kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout);
-if (!kbd_layout)
-exit(1);
+kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout,
+ &error_fatal);
}
}
diff --git a/ui/keymaps.c b/ui/keymaps.c
index b05fb028dc..085889b555 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -27,6 +27,7 @@
#include "sysemu/sysemu.h"
#include "trace.h"
#include "qemu/error-report.h"
+#include "qapi/error.h"
struct keysym2code {
uint32_t count;
@@ -81,7 +82,7 @@ static void add_keysym(char *line, int keysym, int keycode,
kbd_layout_t *k)
static int parse_keyboard_layout(kbd_layout_t *k,
const name2keysym_t *table,
- const char *language)
+ const char *language, Error **errp)
{
int ret;
FILE *f;
@@ -95,7 +96,7 @@ static int parse_keyboard_layout(kbd_layout_t *k,
f = filename ? fopen(filename, "r") : NULL;
g_free(filename);
if (!f) {
-fprintf(stderr, "Could not read keymap file: '%s'\n", language);
+error_setg(errp, "could not read keymap file: '%s'", language);
return -1;
}
@@ -114,7 +115,7 @@ static int parse_keyboard_layout(kbd_layout_t *k,
continue;
}
if (!strncmp(line, "include ", 8)) {
-if (parse_keyboard_layout(k, table, line + 8) < 0) {
+if (parse_keyboard_layout(k, table, line + 8, errp) < 0) {
ret = -1;
goto out;
}
@@ -172,13 +173,13 @@ out:
kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
- const char *language)
+ const char *language, Error **errp)
{
kbd_layout_t *k;
k = g_new0(kbd_layout_t, 1);
k->hash = g_hash_table_new(NULL, NULL);
-if (parse_keyboard_layout(k, table, language) < 0) {
+if (parse_keyboard_layout(k, table, language, errp) < 0) {
g_hash_table_unref(k->hash);
g_free(k);
return NULL;
diff --git a/ui/keymaps.h b/ui/keymaps.h
index 0693588225..98213a4191 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -53,7 +53,7 @@ typedef struct {
typedef struct kbd_layout_t kbd_layout_t;
kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
- const char *language);
+ const char *language, Error **errp);
int keysym2scancode(kbd_layout_t *k, int keysym,
bool shift, bool altgr, bool ctrl);
int keycode_is_keypad(kbd_layout_t *k, int keycode);
diff --git a/ui/sdl.c b/ui/sdl.c
index a5fd503c25..190b16f575 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -29,6 +29,7 @@
#include
#include
+#include "qapi/error.h"
#include "qemu-common.h"
#include "qemu/cutils.h"
#include "ui/console.h"
@@ -917,9 +918,8 @@ static void sdl1_display_init(DisplayState *ds,
DisplayOptions *o)
keyboard_layout = "en-us";
#endif
if(keyboard_layout) {
-kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout);
-if (!kbd_layout)
-exit(1);
+kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout,
+ &error_fatal);
}
g_printerr("Running QEMU with SDL 1.2 is deprecated, and will be removed\n"
diff --git a/ui/vnc.c b/ui/vnc.c
index cf221c83cc..98e3d3b1d8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3205,7 +3205,7 @@ static const DisplayChangeListenerOps dcl_ops = {
.dpy_cursor_define= vnc_dp
[Qemu-devel] [PATCH v4 29/38] tpm: Clean up error reporting in tpm_init_tpmdev()
Calling error_report() in a function that takes an Error ** argument
is suspicious. tpm_init_tpmdev() does that, and then fails without
setting an error. Its caller main(), via tpm_init() and
qemu_opts_foreach(), is fine with it, but clean it up anyway.
Cc: Stefan Berger
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
Reviewed-by: Stefan Berger
---
include/sysemu/tpm.h | 2 +-
stubs/tpm.c | 3 +--
tpm.c| 22 +-
vl.c | 4 +---
4 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 9ae1ab6da3..17a97ed77a 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -16,7 +16,7 @@
#include "qom/object.h"
int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
-int tpm_init(void);
+void tpm_init(void);
void tpm_cleanup(void);
typedef enum TPMVersion {
diff --git a/stubs/tpm.c b/stubs/tpm.c
index 6729bc8517..80939cd3db 100644
--- a/stubs/tpm.c
+++ b/stubs/tpm.c
@@ -9,9 +9,8 @@
#include "qapi/qapi-commands-tpm.h"
#include "sysemu/tpm.h"
-int tpm_init(void)
+void tpm_init(void)
{
-return 0;
}
void tpm_cleanup(void)
diff --git a/tpm.c b/tpm.c
index 93031723ad..9c9e20bbb7 100644
--- a/tpm.c
+++ b/tpm.c
@@ -89,19 +89,19 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts,
Error **errp)
int i;
if (!QLIST_EMPTY(&tpm_backends)) {
-error_report("Only one TPM is allowed.");
+error_setg(errp, "Only one TPM is allowed.");
return 1;
}
id = qemu_opts_id(opts);
if (id == NULL) {
-error_report(QERR_MISSING_PARAMETER, "id");
+error_setg(errp, QERR_MISSING_PARAMETER, "id");
return 1;
}
value = qemu_opt_get(opts, "type");
if (!value) {
-error_report(QERR_MISSING_PARAMETER, "type");
+error_setg(errp, QERR_MISSING_PARAMETER, "type");
tpm_display_backend_drivers();
return 1;
}
@@ -109,8 +109,8 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts,
Error **errp)
i = qapi_enum_parse(&TpmType_lookup, value, -1, NULL);
be = i >= 0 ? tpm_be_find_by_type(i) : NULL;
if (be == NULL) {
-error_report(QERR_INVALID_PARAMETER_VALUE,
- "type", "a TPM backend type");
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
+ "a TPM backend type");
tpm_display_backend_drivers();
return 1;
}
@@ -118,7 +118,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts,
Error **errp)
/* validate backend specific opts */
qemu_opts_validate(opts, be->opts, &local_err);
if (local_err) {
-error_report_err(local_err);
+error_propagate(errp, local_err);
return 1;
}
@@ -151,14 +151,10 @@ void tpm_cleanup(void)
* Initialize the TPM. Process the tpmdev command line options describing the
* TPM backend.
*/
-int tpm_init(void)
+void tpm_init(void)
{
-if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
- tpm_init_tpmdev, NULL, NULL)) {
-return -1;
-}
-
-return 0;
+qemu_opts_foreach(qemu_find_opts("tpmdev"),
+ tpm_init_tpmdev, NULL, &error_fatal);
}
/*
diff --git a/vl.c b/vl.c
index 8e0006d49c..abfe991ed6 100644
--- a/vl.c
+++ b/vl.c
@@ -4359,9 +4359,7 @@ int main(int argc, char **argv, char **envp)
user_creatable_add_opts_foreach,
object_create_delayed, &error_fatal);
-if (tpm_init() < 0) {
-exit(1);
-}
+tpm_init();
/* init the bluetooth world */
if (foreach_device_config(DEV_BT, bt_parse))
--
2.17.1
[Qemu-devel] [PATCH v4 20/38] vl: Clean up error reporting in chardev_init_func()
Calling error_report() in a function that takes an Error ** argument
is suspicious. chardev_init_func() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
---
vl.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/vl.c b/vl.c
index bb18f2d995..aa03192019 100644
--- a/vl.c
+++ b/vl.c
@@ -2239,7 +2239,7 @@ static int chardev_init_func(void *opaque, QemuOpts
*opts, Error **errp)
if (!qemu_chr_new_from_opts(opts, &local_err)) {
if (local_err) {
-error_report_err(local_err);
+error_propagate(errp, local_err);
return -1;
}
exit(0);
@@ -4232,10 +4232,8 @@ int main(int argc, char **argv, char **envp)
user_creatable_add_opts_foreach,
object_create_initial, &error_fatal);
-if (qemu_opts_foreach(qemu_find_opts("chardev"),
- chardev_init_func, NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("chardev"),
+ chardev_init_func, NULL, &error_fatal);
#ifdef CONFIG_VIRTFS
if (qemu_opts_foreach(qemu_find_opts("fsdev"),
--
2.17.1
[Qemu-devel] [PATCH v4 36/38] block: Clean up bdrv_img_create()'s error reporting
bdrv_img_create() takes an Error ** argument and uses it in the
conventional way, except for one place: when qemu_opts_do_parse()
fails, it first reports its error to stderr or the HMP monitor with
error_report_err(), then error_setg()'s a generic error.
When the caller reports that second error similarly, this produces two
consecutive error messages on stderr or the HMP monitor.
When the caller does something else with it, such as send it via QMP,
the first error still goes to stderr or the HMP monitor. Fortunately,
no such caller exists.
Simply use the first error as is. Update expected output of
qemu-iotest 049 accordingly.
Cc: Kevin Wolf
Cc: Max Reitz
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
---
block.c| 3 ---
tests/qemu-iotests/049.out | 12
2 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index 5d51419d21..08d64cdc61 100644
--- a/block.c
+++ b/block.c
@@ -4803,9 +4803,6 @@ void bdrv_img_create(const char *filename, const char
*fmt,
if (options) {
qemu_opts_do_parse(opts, options, NULL, &local_err);
if (local_err) {
-error_report_err(local_err);
-local_err = NULL;
-error_setg(errp, "Invalid options for file format '%s'", fmt);
goto out;
}
}
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index 0871bff564..6b505408dd 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -95,35 +95,31 @@ qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1024
qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2
-qemu-img: Value '-1024' is out of range for parameter 'size'
-qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
+qemu-img: TEST_DIR/t.qcow2: Value '-1024' is out of range for parameter 'size'
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k
qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2
-qemu-img: Value '-1k' is out of range for parameter 'size'
-qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
+qemu-img: TEST_DIR/t.qcow2: Value '-1k' is out of range for parameter 'size'
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte
qemu-img: Invalid image size specified! You may use k, M, G, T, P or E
suffixes for
qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
qemu-img create -f qcow2 -o size=1kilobyte TEST_DIR/t.qcow2
-qemu-img: Parameter 'size' expects a non-negative number below 2^64
+qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number
below 2^64
Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta-
and exabytes, respectively.
-qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- foobar
qemu-img: Invalid image size specified! You may use k, M, G, T, P or E
suffixes for
qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2
-qemu-img: Parameter 'size' expects a non-negative number below 2^64
+qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number
below 2^64
Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta-
and exabytes, respectively.
-qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
== Check correct interpretation of suffixes for cluster size ==
--
2.17.1
[Qemu-devel] [PATCH v4 37/38] raw: Convert a warning to warn_report()
Convert the warning about dangerous automatic probing of raw images to
warn_report(). Split its text to conform to conventions spelled out
in warn_report()'s contract.
Update expected output of qemu-iotest 109 accordingly. Update
qemu-iotest 099's output filtering to keep filtering out the warning.
Cc: Kevin Wolf
Cc: Max Reitz
Signed-off-by: Markus Armbruster
---
block/raw-format.c | 17 ++--
tests/qemu-iotests/099 | 3 ++-
tests/qemu-iotests/109.out | 55 +++---
3 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/block/raw-format.c b/block/raw-format.c
index 6f6dc99b2c..d65fd0ffce 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -29,6 +29,7 @@
#include "qemu/osdep.h"
#include "block/block_int.h"
#include "qapi/error.h"
+#include "qemu/error-report.h"
#include "qemu/option.h"
typedef struct BDRVRawState {
@@ -436,14 +437,14 @@ static int raw_open(BlockDriverState *bs, QDict *options,
int flags,
bs->file->bs->supported_zero_flags);
if (bs->probed && !bdrv_is_read_only(bs)) {
-fprintf(stderr,
-"WARNING: Image format was not specified for '%s' and probing "
-"guessed raw.\n"
-" Automatically detecting the format is dangerous for "
-"raw images, write operations on block 0 will be restricted.\n"
-" Specify the 'raw' format explicitly to remove the "
-"restrictions.\n",
-bs->file->bs->filename);
+warn_report("Image format was not specified for '%s' and probing "
+"guessed raw",
+bs->file->bs->filename);
+error_printf("Automatically detecting the format is dangerous for "
+ "raw images, write\n"
+ "operations on block 0 will be restricted.\n"
+ "Specify the 'raw' format explicitly to remove the "
+ "restrictions.\n");
}
ret = raw_read_options(options, bs, s, errp);
diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
index caaf58eee5..f2a62448d8 100755
--- a/tests/qemu-iotests/099
+++ b/tests/qemu-iotests/099
@@ -57,7 +57,8 @@ function run_qemu()
# which is how we can extract it)
do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_imgfmt | _filter_qmp \
| _filter_qemu | grep "drv0" \
-| sed -e 's/^.*"file": "\(\(\\"\|[^"]\)*\)".*$/\1/' -e 's/\\"/"/g'
+| sed -e 's/^.*"file": "\(\(\\"\|[^"]\)*\)".*$/\1/' -e 's/\\"/"/g' \
+ -e '/probing guessed raw/d'
}
function test_qemu()
diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
index ad0ee6fb48..99add53271 100644
--- a/tests/qemu-iotests/109.out
+++ b/tests/qemu-iotests/109.out
@@ -5,8 +5,9 @@ QA output created by 109
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
{"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing
guessed raw.
- Automatically detecting the format is dangerous for raw images, write
operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing
guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -43,8 +44,9 @@ Images are identical.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
{"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing
guessed raw.
- Automatically detecting the format is dangerous for raw images, write
operations on block 0 will be restricted.
+warning: Image format was not specified for 'TEST_DIR/t.raw' and probing
guessed raw
+Automatically detecting the format is dangerous for raw images, write
+operations on block 0 will be restricted.
Specify the 'raw' format explicitly to remove the restrictions.
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "src"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "src"}}
@@ -81,8 +83,9 @@ Images are identical.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
{"return": {}}
-WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing
guessed raw.
- Automati
[Qemu-devel] [PATCH v4 27/38] vnc: Clean up error reporting in vnc_init_func()
Calling error_report() in a function that takes an Error ** argument
is suspicious. vnc_init_func() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
While there, drop a "Failed to start VNC server: " error message
prefix that doesn't really add value.
Cc: Gerd Hoffmann
Signed-off-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
ui/vnc.c | 8
vl.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 98e3d3b1d8..0c1b477425 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4082,13 +4082,13 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error
**errp)
assert(id);
vnc_display_init(id, &local_err);
if (local_err) {
-error_report_err(local_err);
-exit(1);
+error_propagate(errp, local_err);
+return -1;
}
vnc_display_open(id, &local_err);
if (local_err != NULL) {
-error_reportf_err(local_err, "Failed to start VNC server: ");
-exit(1);
+error_propagate(errp, local_err);
+return -1;
}
return 0;
}
diff --git a/vl.c b/vl.c
index c053117028..8e0006d49c 100644
--- a/vl.c
+++ b/vl.c
@@ -4526,7 +4526,7 @@ int main(int argc, char **argv, char **envp)
/* init remote displays */
#ifdef CONFIG_VNC
qemu_opts_foreach(qemu_find_opts("vnc"),
- vnc_init_func, NULL, NULL);
+ vnc_init_func, NULL, &error_fatal);
#endif
if (using_spice) {
--
2.17.1
[Qemu-devel] [PATCH v4 10/38] ioapic: Fix error handling in realize()
Calling error_report() in a function that takes an Error ** argument
is suspicious. ioapic_realize() does that, and then exit()s.
Currently mostly harmless, as the device cannot be hot-plugged.
Fixes: 20fd4b7b6d9282fe0cb83601f1821f31bd257458
Cc: Peter Xu
Signed-off-by: Markus Armbruster
Reviewed-by: Peter Xu
Reviewed-by: Marc-André Lureau
---
hw/intc/ioapic.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index b6896ac4ce..4e529729b4 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -21,7 +21,7 @@
*/
#include "qemu/osdep.h"
-#include "qemu/error-report.h"
+#include "qapi/error.h"
#include "monitor/monitor.h"
#include "hw/hw.h"
#include "hw/i386/pc.h"
@@ -393,9 +393,9 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
IOAPICCommonState *s = IOAPIC_COMMON(dev);
if (s->version != 0x11 && s->version != 0x20) {
-error_report("IOAPIC only supports version 0x11 or 0x20 "
- "(default: 0x%x).", IOAPIC_VER_DEF);
-exit(1);
+error_setg(errp, "IOAPIC only supports version 0x11 or 0x20 "
+ "(default: 0x%x).", IOAPIC_VER_DEF);
+return;
}
memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
--
2.17.1
[Qemu-devel] [PATCH v4 05/38] vfio: Use warn_report() & friends to report warnings
The vfio code reports warnings like
error_report(WARN_PREFIX "Could not frobnicate", DEV-NAME);
where WARN_PREFIX is defined so the message comes out as
vfio warning: DEV-NAME: Could not frobnicate
This usage predates the introduction of warn_report() & friends in
commit 97f40301f1d. It's time to convert to that interface. Since
these functions already prefix the message with "warning: ", replace
WARN_PREFIX by VFIO_MSG_PREFIX, so the messages come out like
warning: vfio DEV-NAME: Could not frobnicate
The next commit will replace ERR_PREFIX.
Cc: Alex Williamson
Signed-off-by: Markus Armbruster
Acked-by: Alex Williamson
Reviewed-by: Eric Blake
---
hw/vfio/pci.c | 14 +++---
hw/vfio/platform.c| 4 ++--
include/hw/vfio/vfio-common.h | 2 +-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4404c28360..0c3d245932 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -252,7 +252,7 @@ static void vfio_intx_update(PCIDevice *pdev)
vfio_intx_enable_kvm(vdev, &err);
if (err) {
-error_reportf_err(err, WARN_PREFIX, vdev->vbasedev.name);
+warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
}
/* Re-enable the interrupt in cased we missed an EOI */
@@ -317,7 +317,7 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev, Error
**errp)
vfio_intx_enable_kvm(vdev, &err);
if (err) {
-error_reportf_err(err, WARN_PREFIX, vdev->vbasedev.name);
+warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
}
vdev->interrupt = VFIO_INT_INTx;
@@ -1557,7 +1557,7 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos,
Error **errp)
&err);
if (ret < 0) {
if (ret == -ENOTSUP) {
-error_report_err(err);
+warn_report_err(err);
return 0;
}
@@ -2590,9 +2590,9 @@ static void vfio_populate_device(VFIOPCIDevice *vdev,
Error **errp)
} else if (irq_info.count == 1) {
vdev->pci_aer = true;
} else {
-error_report(WARN_PREFIX
- "Could not enable error recovery for the device",
- vbasedev->name);
+warn_report(VFIO_MSG_PREFIX
+"Could not enable error recovery for the device",
+vbasedev->name);
}
}
@@ -2717,7 +2717,7 @@ static void vfio_req_notifier_handler(void *opaque)
qdev_unplug(&vdev->pdev.qdev, &err);
if (err) {
-error_reportf_err(err, WARN_PREFIX, vdev->vbasedev.name);
+warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
}
}
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 64c1af653d..baf236ae79 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -679,8 +679,8 @@ static void vfio_platform_realize(DeviceState *dev, Error
**errp)
for (i = 0; i < vbasedev->num_regions; i++) {
if (vfio_region_mmap(vdev->regions[i])) {
-error_report("%s mmap unsupported. Performance may be slow",
- memory_region_name(vdev->regions[i]->mem));
+warn_report("%s mmap unsupported, performance may be slow",
+memory_region_name(vdev->regions[i]->mem));
}
sysbus_init_mmio(sbdev, vdev->regions[i]->mem);
}
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index e46a28910a..1d7630345b 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -32,7 +32,7 @@
#endif
#define ERR_PREFIX "vfio error: %s: "
-#define WARN_PREFIX "vfio warning: %s: "
+#define VFIO_MSG_PREFIX "vfio %s: "
enum {
VFIO_DEVICE_TYPE_PCI = 0,
--
2.17.1
[Qemu-devel] [PATCH v4 04/38] cpus hw target: Use warn_report() & friends to report warnings
Calling error_report() in a function that takes an Error ** argument
is suspicious. Convert a few that are actually warnings to
warn_report().
While there, split a warning consisting of multiple sentences to
conform to conventions spelled out in warn_report()'s contract.
Cc: Alex Bennée
Cc: Mark Cave-Ayland
Cc: Alex Williamson
Cc: Fam Zheng
Cc: Wei Huang
Cc: David Gibson
Signed-off-by: Markus Armbruster
Acked-by: David Gibson
Reviewed-by: Alex Bennée
---
cpus.c | 8
hw/display/cg3.c| 2 +-
hw/display/tcx.c| 2 +-
hw/misc/ivshmem.c | 4 ++--
hw/net/virtio-net.c | 8
hw/virtio/virtio-pci.c | 4 ++--
target/i386/cpu.c | 17 +
target/ppc/translate_init.inc.c | 4 ++--
8 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/cpus.c b/cpus.c
index 361678e459..7804071872 100644
--- a/cpus.c
+++ b/cpus.c
@@ -211,12 +211,12 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
error_setg(errp, "No MTTCG when icount is enabled");
} else {
#ifndef TARGET_SUPPORTS_MTTCG
-error_report("Guest not yet converted to MTTCG - "
- "you may get unexpected results");
+warn_report("Guest not yet converted to MTTCG - "
+"you may get unexpected results");
#endif
if (!check_tcg_memory_orders_compatible()) {
-error_report("Guest expects a stronger memory ordering "
- "than the host provides");
+warn_report("Guest expects a stronger memory ordering "
+"than the host provides");
error_printf("This may cause strange/hard to debug
errors\n");
}
mttcg_enabled = true;
diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index 1c199ab369..e50d97e48c 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -307,7 +307,7 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
ret = load_image_mr(fcode_filename, &s->rom);
g_free(fcode_filename);
if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
-error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
+warn_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
}
}
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index b2786ee8d0..66f2459226 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -823,7 +823,7 @@ static void tcx_realizefn(DeviceState *dev, Error **errp)
ret = load_image_mr(fcode_filename, &s->rom);
g_free(fcode_filename);
if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
-error_report("tcx: could not load prom '%s'", TCX_ROM_FILE);
+warn_report("tcx: could not load prom '%s'", TCX_ROM_FILE);
}
}
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 8cb17b9dd4..f88910e55c 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -1288,8 +1288,8 @@ static void ivshmem_realize(PCIDevice *dev, Error **errp)
IVShmemState *s = IVSHMEM_COMMON(dev);
if (!qtest_enabled()) {
-error_report("ivshmem is deprecated, please use ivshmem-plain"
- " or ivshmem-doorbell instead");
+warn_report("ivshmem is deprecated, please use ivshmem-plain"
+" or ivshmem-doorbell instead");
}
if (qemu_chr_fe_backend_connected(&s->server_chr) + !!s->shmobj != 1) {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4bdd5b8532..385b1a03e9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2020,10 +2020,10 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
if (n->net_conf.tx && strcmp(n->net_conf.tx, "timer")
&& strcmp(n->net_conf.tx, "bh")) {
-error_report("virtio-net: "
- "Unknown option tx=%s, valid options: \"timer\" \"bh\"",
- n->net_conf.tx);
-error_report("Defaulting to \"bh\"");
+warn_report("virtio-net: "
+"Unknown option tx=%s, valid options: \"timer\" \"bh\"",
+n->net_conf.tx);
+error_printf("Defaulting to \"bh\"");
}
n->net_conf.tx_queue_size = MIN(virtio_net_max_tx_queue_size(n),
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 3a01fe90f0..a954799267 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1683,8 +1683,8 @@ static void virtio_pci_device_plugged(DeviceState *d,
Error **errp)
if (err) {
/* Notice when a system that supports MSIx can't initialize it */
if (err != -ENOTSUP) {
-error_report("unable to init msix vectors to %" PRIu32,
- proxy->nvectors);
+warn_report("unable to init msix vectors to %" PRIu32,
[Qemu-devel] [PATCH v4 38/38] vpc: Fail open on bad header checksum
vpc_open() merely prints a warning when it finds a bad header
checksum. Turn that into a hard error.
Cc: Kevin Wolf
Signed-off-by: Markus Armbruster
---
block/vpc.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index bf294abfa7..1729c0cb44 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -284,9 +284,11 @@ static int vpc_open(BlockDriverState *bs, QDict *options,
int flags,
checksum = be32_to_cpu(footer->checksum);
footer->checksum = 0;
-if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum)
-fprintf(stderr, "block-vpc: The header checksum of '%s' is "
-"incorrect.\n", bs->filename);
+if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum) {
+error_setg(errp, "incorrect header checksum");
+ret = -EINVAL;
+goto fail;
+}
/* Write 'checksum' back to footer, or else will leave it with zero. */
footer->checksum = cpu_to_be32(checksum);
--
2.17.1
[Qemu-devel] [PATCH v4 19/38] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Calling error_report() in a function that takes an Error ** argument
is suspicious. user_creatable_add_opts_foreach() does that, and then
fails without setting an error. Its caller main(), via
qemu_opts_foreach(), is fine with it, but clean it up anyway.
Cc: Daniel P. Berrangé
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Reviewed-by: Marc-André Lureau
---
qemu-io.c | 8 +++-
qemu-nbd.c | 8 +++-
qom/object_interfaces.c | 4 +---
vl.c| 16 ++--
4 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 13829f5e21..6df7731af4 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -620,11 +620,9 @@ int main(int argc, char **argv)
exit(1);
}
-if (qemu_opts_foreach(&qemu_object_opts,
- user_creatable_add_opts_foreach,
- NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &error_fatal);
if (!trace_init_backends()) {
exit(1);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 7874bc973c..ca7109652e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -766,11 +766,9 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
-if (qemu_opts_foreach(&qemu_object_opts,
- user_creatable_add_opts_foreach,
- NULL, NULL)) {
-exit(EXIT_FAILURE);
-}
+qemu_opts_foreach(&qemu_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, &error_fatal);
if (!trace_init_backends()) {
exit(1);
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 941fd63afd..97b79b48bb 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -143,7 +143,6 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts
*opts, Error **errp)
{
bool (*type_opt_predicate)(const char *, QemuOpts *) = opaque;
Object *obj = NULL;
-Error *err = NULL;
const char *type;
type = qemu_opt_get(opts, "qom-type");
@@ -152,9 +151,8 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts
*opts, Error **errp)
return 0;
}
-obj = user_creatable_add_opts(opts, &err);
+obj = user_creatable_add_opts(opts, errp);
if (!obj) {
-error_report_err(err);
return -1;
}
object_unref(obj);
diff --git a/vl.c b/vl.c
index 2ef066ad61..bb18f2d995 100644
--- a/vl.c
+++ b/vl.c
@@ -4228,11 +4228,9 @@ int main(int argc, char **argv, char **envp)
page_size_init();
socket_init();
-if (qemu_opts_foreach(qemu_find_opts("object"),
- user_creatable_add_opts_foreach,
- object_create_initial, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("object"),
+ user_creatable_add_opts_foreach,
+ object_create_initial, &error_fatal);
if (qemu_opts_foreach(qemu_find_opts("chardev"),
chardev_init_func, NULL, NULL)) {
@@ -4363,11 +4361,9 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
-if (qemu_opts_foreach(qemu_find_opts("object"),
- user_creatable_add_opts_foreach,
- object_create_delayed, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("object"),
+ user_creatable_add_opts_foreach,
+ object_create_delayed, &error_fatal);
if (tpm_init() < 0) {
exit(1);
--
2.17.1
[Qemu-devel] [PATCH 0/5] target/i386: introduce coalesced pio
Coalesced pio is base on coalesced mmio and can be used for some port
like rtc port, pci-host config port and so on.
Specially in case of rtc as coalesced pio, some versions of windows guest
access rtc frequently because of rtc as system tick. guest access rtc like
this: write register index to 0x70, then write or read data from 0x71.
writing 0x70 port is just as index and do nothing else. So we can use
coalesced pio to handle this scene to reduce VM-EXIT time.
When it starts and closes the virtual machine, it will access pci-host config
port or virtio-pci config port frequently. So setting these port as
coalesced pio can reduce startup and shutdown time. In qemu I just realize
piixfx's pci-host, it is convenient for other pci-host type implementations.
without my patch, get the vm-exit time of accessing rtc 0x70 and piix 0xcf8
using perf tools: (guest OS : windows 7 64bit)
IO Port Access Samples Samples% Time% Min Time Max Time Avg time
0x70:POUT86 30.99%74.59% 9us 29us10.75us (+- 3.41%)
0xcf8:POUT 1119 2.60% 2.12% 2.79us56.83us 3.41us (+- 2.23%)
with my patch
IO Port Access Samples Samples% Time% Min Time Max Time Avg time
0x70:POUT 10632.02%29.47%0us 10us 1.57us (+- 7.38%)
0xcf8:POUT 10651.67% 0.28% 0.41us65.44us 0.66us (+- 10.55%)
These are just qemu's patches, another patches are for kernel.
Changes v4 --> v5:
update kvm header, improve compatibility.
Changes v3 --> v4
modify coalesced_mmio_{add|del} to coalesced_io_{add|del}
delete unnecessary macro define
Peng Hao (5):
target-i386: add rtc 0x70 port as coalesced_pio
target-i386: add i440fx 0xcf8 port as coalesced_pio
target-i386: add q35 0xcf8 port as coalesced_pio
target/i386: add coalesced pio support
target-i386: add coalesced_pio API
accel/kvm/kvm-all.c | 60 +++
hw/pci-host/piix.c| 4
hw/pci-host/q35.c | 4
hw/timer/mc146818rtc.c| 8 +++
include/exec/memory.h | 4 ++--
linux-headers/linux/kvm.h | 9 ++-
memory.c | 4 ++--
7 files changed, 83 insertions(+), 10 deletions(-)
--
1.8.3.1
[Qemu-devel] [PATCH v4 23/38] vl: Clean up error reporting in parse_fw_cfg()
Calling error_report() in a function that takes an Error ** argument
is suspicious. parse_fw_cfg() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
---
vl.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/vl.c b/vl.c
index cd8e8a6586..c5bc54f72b 100644
--- a/vl.c
+++ b/vl.c
@@ -2174,7 +2174,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
Error **errp)
FWCfgState *fw_cfg = (FWCfgState *) opaque;
if (fw_cfg == NULL) {
-error_report("fw_cfg device not available");
+error_setg(errp, "fw_cfg device not available");
return -1;
}
name = qemu_opt_get(opts, "name");
@@ -2183,15 +2183,16 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
Error **errp)
/* we need name and either a file or the content string */
if (!(nonempty_str(name) && (nonempty_str(file) || nonempty_str(str {
-error_report("invalid argument(s)");
+error_setg(errp, "invalid argument(s)");
return -1;
}
if (nonempty_str(file) && nonempty_str(str)) {
-error_report("file and string are mutually exclusive");
+error_setg(errp, "file and string are mutually exclusive");
return -1;
}
if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
-error_report("name too long (max. %d char)", FW_CFG_MAX_FILE_PATH - 1);
+error_setg(errp, "name too long (max. %d char)",
+ FW_CFG_MAX_FILE_PATH - 1);
return -1;
}
if (strncmp(name, "opt/", 4) != 0) {
@@ -2203,7 +2204,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts,
Error **errp)
buf = g_memdup(str, size);
} else {
if (!g_file_get_contents(file, &buf, &size, NULL)) {
-error_report("can't load %s", file);
+error_setg(errp, "can't load %s", file);
return -1;
}
}
@@ -4476,10 +4477,8 @@ int main(int argc, char **argv, char **envp)
hax_sync_vcpus();
}
-if (qemu_opts_foreach(qemu_find_opts("fw_cfg"),
- parse_fw_cfg, fw_cfg_find(), NULL) != 0) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("fw_cfg"),
+ parse_fw_cfg, fw_cfg_find(), &error_fatal);
/* init USB devices */
if (machine_usb(current_machine)) {
--
2.17.1
[Qemu-devel] [PATCH V6 5/5] target-i386: add q35 0xcf8 port as coalesced_pio
Signed-off-by: Peng Hao --- hw/pci-host/q35.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 02f9576..8ce1e09 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -51,6 +51,10 @@ static void q35_host_realize(DeviceState *dev, Error **errp) sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem); sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4); +/* register q35 0xcf8 port as coalesced pio */ +memory_region_set_flush_coalesced(&pci->data_mem); +memory_region_add_coalescing(&pci->conf_mem, 0, 4); + pci->bus = pci_root_bus_new(DEVICE(s), "pcie.0", s->mch.pci_address_space, s->mch.address_space_io, -- 1.8.3.1
[Qemu-devel] [PATCH V6 4/5] target-i386: add i440fx 0xcf8 port as coalesced_pio
Signed-off-by: Peng Hao --- hw/pci-host/piix.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 0e60834..da73743 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -327,6 +327,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, Error **errp) sysbus_add_io(sbd, 0xcfc, &s->data_mem); sysbus_init_ioports(sbd, 0xcfc, 4); + +/* register i440fx 0xcf8 port as coalesced pio */ +memory_region_set_flush_coalesced(&s->data_mem); +memory_region_add_coalescing(&s->conf_mem, 0, 4); } static void i440fx_realize(PCIDevice *dev, Error **errp) -- 1.8.3.1
[Qemu-devel] [PATCH v4 28/38] numa: Clean up error reporting in parse_numa()
Calling error_report() in a function that takes an Error ** argument
is suspicious. parse_numa() does that, and then fails without setting
an error. Its caller main(), via qemu_opts_foreach(), is fine with
it, but clean it up anyway.
While there, give parse_numa() internal linkage.
Cc: Eduardo Habkost
Signed-off-by: Markus Armbruster
Reviewed-by: Eduardo Habkost
---
include/sysemu/numa.h | 1 -
numa.c| 8 +++-
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index 7a0ae751aa..21713b7e2f 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -22,7 +22,6 @@ struct NumaNodeMem {
};
extern NodeInfo numa_info[MAX_NODES];
-int parse_numa(void *opaque, QemuOpts *opts, Error **errp);
void parse_numa_opts(MachineState *ms);
void numa_complete_configuration(MachineState *ms);
void query_numa_node_mem(NumaNodeMem node_mem[]);
diff --git a/numa.c b/numa.c
index 1d7c49ad43..50ec016013 100644
--- a/numa.c
+++ b/numa.c
@@ -215,7 +215,7 @@ end:
error_propagate(errp, err);
}
-int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
+static int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
{
NumaOptions *object = NULL;
MachineState *ms = MACHINE(opaque);
@@ -239,7 +239,7 @@ int parse_numa(void *opaque, QemuOpts *opts, Error **errp)
end:
qapi_free_NumaOptions(object);
if (err) {
-error_report_err(err);
+error_propagate(errp, err);
return -1;
}
@@ -444,9 +444,7 @@ void numa_complete_configuration(MachineState *ms)
void parse_numa_opts(MachineState *ms)
{
-if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, &error_fatal);
}
void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
--
2.17.1
[Qemu-devel] [PATCH V6 3/5] target-i386: add rtc 0x70 port as coalesced_pio
Signed-off-by: Peng Hao
---
hw/timer/mc146818rtc.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index acee47d..808a212 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -70,6 +70,7 @@ typedef struct RTCState {
ISADevice parent_obj;
MemoryRegion io;
+MemoryRegion coalesced_io;
uint8_t cmos_data[128];
uint8_t cmos_index;
int32_t base_year;
@@ -990,6 +991,13 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2);
isa_register_ioport(isadev, &s->io, base);
+/* register rtc 0x70 port as coalesced_pio */
+memory_region_set_flush_coalesced(&s->io);
+memory_region_init_io(&s->coalesced_io, OBJECT(s), &cmos_ops,
+ s, "rtc1", 1);
+isa_register_ioport(isadev, &s->coalesced_io, base);
+memory_region_add_coalescing(&s->coalesced_io, 0, 1);
+
qdev_set_legacy_instance_id(dev, base, 3);
qemu_register_reset(rtc_reset, s);
--
1.8.3.1
Re: [Qemu-devel] [Qemu-block] [PATCH v2 1/8] block: Update flags in bdrv_set_read_only()
On Fri 12 Oct 2018 01:55:25 PM CEST, Kevin Wolf wrote: > To fully change the read-only state of a node, we must not only change > bs->read_only, but also update bs->open_flags. > > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto
[Qemu-devel] [PATCH v4 25/38] ui/keymaps: Fix handling of erroneous include files
While errors in the keyboard layout named with -k are fatal, errors in
included files are reported, but otherwise ignored:
$ cat worst
include bad
include worse
$ ls -l bad worse
ls: cannot access 'bad': No such file or directory
ls: cannot access 'worse': No such file or directory
$ qemu-system-x86_64 -nodefaults -S -monitor stdio -display vnc=:0 -k bad
QEMU 3.0.50 monitor - type 'help' for more information
(qemu) Could not read keymap file: 'bad'
$ qemu-system-x86_64 -nodefaults -S -monitor stdio -display vnc=:0 -k worst
QEMU 3.0.50 monitor - type 'help' for more information
(qemu) Could not read keymap file: 'bad'
Could not read keymap file: 'worse'
Fix that.
Note that parse_keyboard_layout() allocates the keymap, except when
it's parsing an include file. To keep error handling simple, move the
memory management to its caller init_keyboard_layout().
Cc: Gerd Hoffmann
Signed-off-by: Markus Armbruster
---
ui/keymaps.c | 35 +++
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 43fe604724..b05fb028dc 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -79,10 +79,11 @@ static void add_keysym(char *line, int keysym, int keycode,
kbd_layout_t *k)
trace_keymap_add(keysym, keycode, line);
}
-static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
- const char *language,
- kbd_layout_t *k)
+static int parse_keyboard_layout(kbd_layout_t *k,
+ const name2keysym_t *table,
+ const char *language)
{
+int ret;
FILE *f;
char * filename;
char line[1024];
@@ -95,12 +96,7 @@ static kbd_layout_t *parse_keyboard_layout(const
name2keysym_t *table,
g_free(filename);
if (!f) {
fprintf(stderr, "Could not read keymap file: '%s'\n", language);
-return NULL;
-}
-
-if (!k) {
-k = g_new0(kbd_layout_t, 1);
-k->hash = g_hash_table_new(NULL, NULL);
+return -1;
}
for(;;) {
@@ -118,7 +114,10 @@ static kbd_layout_t *parse_keyboard_layout(const
name2keysym_t *table,
continue;
}
if (!strncmp(line, "include ", 8)) {
-parse_keyboard_layout(table, line + 8, k);
+if (parse_keyboard_layout(k, table, line + 8) < 0) {
+ret = -1;
+goto out;
+}
} else {
int offset = 0;
while (line[offset] != 0 &&
@@ -164,15 +163,27 @@ static kbd_layout_t *parse_keyboard_layout(const
name2keysym_t *table,
}
}
}
+
+ret = 0;
+out:
fclose(f);
-return k;
+return ret;
}
kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
const char *language)
{
-return parse_keyboard_layout(table, language, NULL);
+kbd_layout_t *k;
+
+k = g_new0(kbd_layout_t, 1);
+k->hash = g_hash_table_new(NULL, NULL);
+if (parse_keyboard_layout(k, table, language) < 0) {
+g_hash_table_unref(k->hash);
+g_free(k);
+return NULL;
+}
+return k;
}
--
2.17.1
[Qemu-devel] [PATCH V6 2/5] target-i386 : add coalesced_pio API
the primary API realization.
Signed-off-by: Peng Hao
Reviewed-by: Eduardo Habkost
---
accel/kvm/kvm-all.c | 56 ---
include/exec/memory.h | 4 ++--
memory.c | 4 ++--
3 files changed, 57 insertions(+), 7 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 29d208d..7724055 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -79,6 +79,7 @@ struct KVMState
int fd;
int vmfd;
int coalesced_mmio;
+int coalesced_pio;
struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
bool coalesced_flush_in_progress;
int vcpu_events;
@@ -560,6 +561,45 @@ static void kvm_uncoalesce_mmio_region(MemoryListener
*listener,
}
}
+static void kvm_coalesce_pio_add(MemoryListener *listener,
+MemoryRegionSection *section,
+hwaddr start, hwaddr size)
+{
+KVMState *s = kvm_state;
+
+if (s->coalesced_pio) {
+struct kvm_coalesced_mmio_zone zone;
+
+zone.addr = start;
+zone.size = size;
+zone.pio = 1;
+
+(void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
+}
+}
+
+static void kvm_coalesce_pio_del(MemoryListener *listener,
+MemoryRegionSection *section,
+hwaddr start, hwaddr size)
+{
+KVMState *s = kvm_state;
+
+if (s->coalesced_pio) {
+struct kvm_coalesced_mmio_zone zone;
+
+zone.addr = start;
+zone.size = size;
+zone.pio = 1;
+
+(void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
+ }
+}
+
+static MemoryListener kvm_coalesced_pio_listener = {
+.coalesced_io_add = kvm_coalesce_pio_add,
+.coalesced_io_del = kvm_coalesce_pio_del,
+};
+
int kvm_check_extension(KVMState *s, unsigned int extension)
{
int ret;
@@ -1616,6 +1656,8 @@ static int kvm_init(MachineState *ms)
}
s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
+s->coalesced_pio = s->coalesced_mmio &&
+ kvm_check_extension(s, KVM_CAP_COALESCED_PIO);
#ifdef KVM_CAP_VCPU_EVENTS
s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
@@ -1686,13 +1728,15 @@ static int kvm_init(MachineState *ms)
s->memory_listener.listener.eventfd_add = kvm_mem_ioeventfd_add;
s->memory_listener.listener.eventfd_del = kvm_mem_ioeventfd_del;
}
-s->memory_listener.listener.coalesced_mmio_add = kvm_coalesce_mmio_region;
-s->memory_listener.listener.coalesced_mmio_del =
kvm_uncoalesce_mmio_region;
+s->memory_listener.listener.coalesced_io_add = kvm_coalesce_mmio_region;
+s->memory_listener.listener.coalesced_io_del = kvm_uncoalesce_mmio_region;
kvm_memory_listener_register(s, &s->memory_listener,
&address_space_memory, 0);
memory_listener_register(&kvm_io_listener,
&address_space_io);
+memory_listener_register(&kvm_coalesced_pio_listener,
+ &address_space_io);
s->many_ioeventfds = kvm_check_many_ioeventfds();
@@ -1778,7 +1822,13 @@ void kvm_flush_coalesced_mmio_buffer(void)
ent = &ring->coalesced_mmio[ring->first];
-cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
+if (ent->pio == 1) {
+address_space_rw(&address_space_io, ent->phys_addr,
+ MEMTXATTRS_UNSPECIFIED, ent->data,
+ ent->len, true);
+} else {
+cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
+}
smp_wmb();
ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
}
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 3a427aa..667466b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -419,9 +419,9 @@ struct MemoryListener {
bool match_data, uint64_t data, EventNotifier *e);
void (*eventfd_del)(MemoryListener *listener, MemoryRegionSection *section,
bool match_data, uint64_t data, EventNotifier *e);
-void (*coalesced_mmio_add)(MemoryListener *listener, MemoryRegionSection
*section,
+void (*coalesced_io_add)(MemoryListener *listener, MemoryRegionSection
*section,
hwaddr addr, hwaddr len);
-void (*coalesced_mmio_del)(MemoryListener *listener, MemoryRegionSection
*section,
+void (*coalesced_io_del)(MemoryListener *listener, MemoryRegionSection
*section,
hwaddr addr, hwaddr len);
/* Lower = earlier (during add), later (during del) */
unsigned priority;
diff --git a/memory.c b/memory.c
index d852f11..51204aa 100644
--- a/memory.c
+++ b/memory.c
@@ -2129,7 +2129,7 @@ static void
memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
[Qemu-devel] [PATCH v4 17/38] seccomp: Clean up error reporting in parse_sandbox()
Calling error_report() in a function that takes an Error ** argument
is suspicious. parse_sandbox() does that, and then fails without
setting an error. Its caller main(), via qemu_opts_foreach(), is fine
with it, but clean it up anyway.
Cc: Eduardo Otubo
Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
Acked-by: Eduardo Otubo
---
qemu-seccomp.c | 18 +-
vl.c | 4 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 1baa5c69ed..5c73e6ad05 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -12,11 +12,12 @@
* Contributions after 2012-01-13 are licensed under the terms of the
* GNU GPL, version 2 or (at your option) any later version.
*/
+
#include "qemu/osdep.h"
+#include "qapi/error.h"
#include "qemu/config-file.h"
#include "qemu/option.h"
#include "qemu/module.h"
-#include "qemu/error-report.h"
#include
#include
#include "sysemu/seccomp.h"
@@ -190,7 +191,7 @@ int parse_sandbox(void *opaque, QemuOpts *opts, Error
**errp)
* to provide a little bit of consistency for
* the command line */
} else {
-error_report("invalid argument for obsolete");
+error_setg(errp, "invalid argument for obsolete");
return -1;
}
}
@@ -205,14 +206,13 @@ int parse_sandbox(void *opaque, QemuOpts *opts, Error
**errp)
/* calling prctl directly because we're
* not sure if host has CAP_SYS_ADMIN set*/
if (prctl(PR_SET_NO_NEW_PRIVS, 1)) {
-error_report("failed to set no_new_privs "
- "aborting");
+error_setg(errp, "failed to set no_new_privs aborting");
return -1;
}
} else if (g_str_equal(value, "allow")) {
/* default value */
} else {
-error_report("invalid argument for elevateprivileges");
+error_setg(errp, "invalid argument for elevateprivileges");
return -1;
}
}
@@ -224,7 +224,7 @@ int parse_sandbox(void *opaque, QemuOpts *opts, Error
**errp)
} else if (g_str_equal(value, "allow")) {
/* default value */
} else {
-error_report("invalid argument for spawn");
+error_setg(errp, "invalid argument for spawn");
return -1;
}
}
@@ -236,14 +236,14 @@ int parse_sandbox(void *opaque, QemuOpts *opts, Error
**errp)
} else if (g_str_equal(value, "allow")) {
/* default value */
} else {
-error_report("invalid argument for resourcecontrol");
+error_setg(errp, "invalid argument for resourcecontrol");
return -1;
}
}
if (seccomp_start(seccomp_opts) < 0) {
-error_report("failed to install seccomp syscall filter "
- "in the kernel");
+error_setg(errp, "failed to install seccomp syscall filter "
+ "in the kernel");
return -1;
}
}
diff --git a/vl.c b/vl.c
index ff50199e64..643121e5d4 100644
--- a/vl.c
+++ b/vl.c
@@ -3972,8 +3972,8 @@ int main(int argc, char **argv, char **envp)
#ifdef CONFIG_SECCOMP
olist = qemu_find_opts_err("sandbox", NULL);
-if (olist && qemu_opts_foreach(olist, parse_sandbox, NULL, NULL)) {
-exit(1);
+if (olist) {
+qemu_opts_foreach(olist, parse_sandbox, NULL, &error_fatal);
}
#endif
--
2.17.1
Re: [Qemu-devel] [RFC PATCH v1 1/4] VFIO KABI for migration interface
On Tue, Oct 16, 2018 at 11:42:35PM +0530, Kirti Wankhede wrote: > - Added vfio_device_migration_info structure to use interact with vendor > driver. There is no such thing as a 'vendor driver' in Linux - all drivers ate treated equal. And I don't see any single driver supporting this yet, so you really should submit your noveau or whatever patches first.
Re: [Qemu-devel] [RFC PATCH v1 0/4] Add migration support for VFIO device
On Tue, 16 Oct 2018 23:42:34 +0530 Kirti Wankhede wrote: > Add migration support for VFIO device I'd love to take a deeper look at this; but sadly, I'm currently low on spare time, and therefore will only add some general remarks. > > This Patch set include patches as below: > - Define KABI for VFIO device for migration support. > - Generic migration functionality for VFIO device. > * This patch set adds functionality only for PCI devices, but can be > extended to other VFIO devices. I've been thinking about how best to add migration to vfio-ccw; it might be quite different from what we do for vfio-pci, but I hope that we can still use some common infrastructure. Basically, we probably need something like the following: - Wait until outstanding channel programs are finished, and don't allow new ones. Don't report back to userspace (or maybe do; we just don't want to fence of too many new requests.) - Collect subchannel and related state, copy it over. That probably fits in with the interface you're proposing. - Re-setup on the other side and get going again. The problem here might be state that the vfio-ccw driver can't be aware of (like replaying storage server configurations). Maybe we can send CRWs (notifications) to the guest and leverage the existing suspend/resume driver code for channel devices. For vfio-ap, I frankly have no idea how migration will work, given the setup with the matrix device and the interesting information in the SIE control blocks (but maybe it just can replay from the info in the matrix device?) Keys etc. are likely to be the bigger problem here. Cc:ing some s390 folks for awareness. > * Added all the basic functions required for pre-copy, stop-and-copy and > resume phases of migration. > * Added state change notifier and from that notifier function, VFIO > device's state changed is conveyed to VFIO vendor driver. I assume that this will, in general, be transparent for the guest; but maybe there'll be need for some interaction for special cases? > * During save setup phase and resume/load setup phase, migration region > is queried from vendor driver and is mmaped by QEMU. This region is > used to read/write data from and to vendor driver. > * .save_live_pending, .save_live_iterate and .is_active_iterate are > implemented to use QEMU's functionality of iteration during pre-copy > phase. > * In .save_live_complete_precopy, that is in stop-and-copy phase, > iteration to read data from vendor driver is implemented till pending > bytes returned by vendor driver are not zero. > * .save_cleanup and .load_cleanup are implemented to unmap migration > region that was setup duing setup phase. > * Added function to get dirty pages bitmap from vendor driver. > - Add vfio_listerner_log_sync to mark dirty pages. > - Make VFIO PCI device migration capable. > > Thanks, > Kirti > > Kirti Wankhede (4): > VFIO KABI for migration interface > Add migration functions for VFIO devices > Add vfio_listerner_log_sync to mark dirty pages > Make vfio-pci device migration capable. > > hw/vfio/Makefile.objs | 2 +- > hw/vfio/common.c | 32 ++ > hw/vfio/migration.c | 716 > ++ > hw/vfio/pci.c | 13 +- > include/hw/vfio/vfio-common.h | 23 ++ > linux-headers/linux/vfio.h| 91 ++ > 6 files changed, 869 insertions(+), 8 deletions(-) > create mode 100644 hw/vfio/migration.c >
[Qemu-devel] [PATCH V6 1/5] target/i386 : add coalesced pio support
add coalesced_pio's struct and KVM_CAP_COALESCED_PIO header.
Signed-off-by: Peng Hao
---
accel/kvm/kvm-all.c | 4 ++--
linux-headers/linux/kvm.h | 9 -
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index de12f78..29d208d 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -537,7 +537,7 @@ static void kvm_coalesce_mmio_region(MemoryListener
*listener,
zone.addr = start;
zone.size = size;
-zone.pad = 0;
+zone.pio = 0;
(void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
}
@@ -554,7 +554,7 @@ static void kvm_uncoalesce_mmio_region(MemoryListener
*listener,
zone.addr = start;
zone.size = size;
-zone.pad = 0;
+zone.pio = 0;
(void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
}
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 83ba4eb..b5d4289 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -420,13 +420,19 @@ struct kvm_run {
struct kvm_coalesced_mmio_zone {
__u64 addr;
__u32 size;
- __u32 pad;
+ union {
+ __u32 pad;
+ __u32 pio;
+ };
};
struct kvm_coalesced_mmio {
__u64 phys_addr;
__u32 len;
+ unino {
__u32 pad;
+ __u32 pio;
+ };
__u8 data[8];
};
@@ -953,6 +959,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_NESTED_STATE 157
#define KVM_CAP_ARM_INJECT_SERROR_ESR 158
#define KVM_CAP_MSR_PLATFORM_INFO 159
+#define KVM_CAP_COALESCED_PIO 160
#ifdef KVM_CAP_IRQ_ROUTING
--
1.8.3.1
[Qemu-devel] [PATCH] Revert "icount: remove obsolete warp call"
QEMU running with options "-icount,sleep=off -rtc clock=vm" doesn't
execute emulation at maximum possible speed.
Target virtual clock may run faster or slower than realtime clock by N
times, where N value depends on various unrelated conditions (i.e. random
from the user point of view), or possibly the target hangs completely.
Bisection shows commit 281b2201e4 ("icount: remove obsolete warp call",
2016-03-15) to be the culprit, revert it.
Suggested-by: Artem Pisarenko
Signed-off-by: Paolo Bonzini
---
cpus.c | 4
1 file changed, 4 insertions(+)
diff --git a/cpus.c b/cpus.c
index 361678e459..2872e7e37c 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1224,6 +1224,10 @@ static void qemu_tcg_rr_wait_io_event(CPUState *cpu)
{
while (all_cpu_threads_idle()) {
stop_tcg_kick_timer();
+
+/* Start accounting real time to the virtual clock if the CPUs
+ are idle. */
+qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
}
--
2.17.1
Re: [Qemu-devel] [RFC PATCH v1 2/4] Add migration functions for VFIO devices
On Tue, 16 Oct 2018 23:42:36 +0530
Kirti Wankhede wrote:
> - Migration function are implemented for VFIO_DEVICE_TYPE_PCI device.
> - Added SaveVMHandlers and implemented all basic functions required for live
> migration.
> - Added VM state change handler to know running or stopped state of VM.
> - Added migration state change notifier to get notification on migration state
> change. This state is translated to VFIO device state and conveyed to vendor
> driver.
>
> Signed-off-by: Kirti Wankhede
> Reviewed-by: Neo Jia
> ---
> hw/vfio/Makefile.objs | 2 +-
> hw/vfio/migration.c | 716
> ++
> include/hw/vfio/vfio-common.h | 23 ++
> 3 files changed, 740 insertions(+), 1 deletion(-)
> create mode 100644 hw/vfio/migration.c
>
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index a2e7a0a7cf02..6206ad47e90e 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -1,6 +1,6 @@
> ifeq ($(CONFIG_LINUX), y)
> obj-$(CONFIG_SOFTMMU) += common.o
> -obj-$(CONFIG_PCI) += pci.o pci-quirks.o display.o
> +obj-$(CONFIG_PCI) += pci.o pci-quirks.o display.o migration.o
I don't think you want to make this explicitly pci-specific, but build
in a proper split of common infrastructure and pci handling from the
beginning.
> obj-$(CONFIG_VFIO_CCW) += ccw.o
> obj-$(CONFIG_SOFTMMU) += platform.o
> obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> new file mode 100644
> index ..8a4f515226e0
> --- /dev/null
> +++ b/hw/vfio/migration.c
(...)
> +static int vfio_save_device_config_state(QEMUFile *f, void *opaque)
> +{
> +VFIODevice *vbasedev = opaque;
> +
> +qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_STATE);
> +
> +if (vbasedev->type == VFIO_DEVICE_TYPE_PCI) {
> +VFIOPCIDevice *vdev = container_of(vbasedev, VFIOPCIDevice,
> vbasedev);
> +PCIDevice *pdev = &vdev->pdev;
> +uint32_t msi_flags, msi_addr_lo, msi_addr_hi = 0, msi_data;
> +bool msi_64bit;
> +int i;
> +
> +for (i = 0; i < PCI_ROM_SLOT; i++) {
> +uint32_t bar;
> +
> +bar = pci_default_read_config(pdev, PCI_BASE_ADDRESS_0 + i * 4,
> 4);
> +qemu_put_be32(f, bar);
> +}
> +
> +msi_flags = pci_default_read_config(pdev,
> +pdev->msi_cap + PCI_MSI_FLAGS,
> 2);
> +msi_64bit = (msi_flags & PCI_MSI_FLAGS_64BIT);
> +
> +msi_addr_lo = pci_default_read_config(pdev,
> + pdev->msi_cap + PCI_MSI_ADDRESS_LO,
> 4);
> +qemu_put_be32(f, msi_addr_lo);
> +
> +if (msi_64bit) {
> +msi_addr_hi = pci_default_read_config(pdev,
> +pdev->msi_cap +
> PCI_MSI_ADDRESS_HI,
> +4);
> +}
> +qemu_put_be32(f, msi_addr_hi);
> +
> +msi_data = pci_default_read_config(pdev,
> +pdev->msi_cap + (msi_64bit ? PCI_MSI_DATA_64 :
> PCI_MSI_DATA_32),
> +2);
> +qemu_put_be32(f, msi_data);
> +}
This is an example of a function that should go into a pci-specific
file. Especially as config space is not a universal concept.
> +qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
> +
> +return qemu_file_get_error(f);
> +}
Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/4] Introduce attributes to qemu timer subsystem
On Wed, Oct 17, 2018 at 02:24:19PM +0600, Artem Pisarenko wrote:
> Attributes are simple flags, associated with individual timers for their
> whole lifetime.
> They intended to be used to mark individual timers for special handling by
> various qemu features operating at qemu core level.
I'm worried that this sentence suggests various parts of QEMU will stash
state in ts->attributes. That's messy and they shouldn't do this. Make
the field private to qemu-timer.c.
Attributes should only affect qemu-timer.c behavior. Other parts of
QEMU should not act differently based on timer attributes (i.e. checking
bits). If they need to do that then it suggests something isn't
properly encapsulated in qemu-timer.c.
> diff --git a/include/block/aio.h b/include/block/aio.h
> index f08630c..fce9d48 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -388,6 +388,31 @@ struct LinuxAioState *aio_setup_linux_aio(AioContext
> *ctx, Error **errp);
> struct LinuxAioState *aio_get_linux_aio(AioContext *ctx);
>
> /**
> + * aio_timer_new_with_attrs:
> + * @ctx: the aio context
> + * @type: the clock type
> + * @scale: the scale
> + * @attributes: 0, or one to multiple OR'ed QEMU_TIMER_ATTR(id) values to
> assign
Further down in this patch the notation is QEMU_TIMER_ATTR_, which I
think is clearer because QEMU_TIMER_ATTR(id) looks like a (non-existent)
macro. Please use the QEMU_TIMER_ATTR_ notation consistently.
> +/**
> + * QEMU Timer attributes:
> + *
> + * An individual timer may be assigned with one or multiple attributes when
> + * initialized.
> + * Attribute is a static flag, meaning that timer has corresponding property.
> + * Attributes are defined in QEMUTimerAttrBit enum and encoded to bit set,
> + * which used to initialize timer, stored to 'attributes' member and can be
> + * retrieved externally with timer_get_attributes() call.
> + * Values of QEMUTimerAttrBit aren't used directly,
> + * instead each attribute in bit set accessed with QEMU_TIMER_ATTR_
> macro,
> + * where is a unique part of attribute identifier.
> + *
> + * No attributes defined currently.
> + */
> +
> +typedef enum {
> +QEMU_TIMER_ATTRBIT__NONE
> +} QEMUTimerAttrBit;
> +
> +#define QEMU_TIMER_ATTR__NONE (1 << QEMU_TIMER_ATTRBIT__NONE)
What is the purpose of this bit? I guess it's just here as a
placeholder because no real bits have been defined yet. Hopefully the
next patch removes it (/* This placeholder is removed in the next patch
*/ would be a nice way to document this for reviewers).
The enum isn't needed and makes debugging harder since the bit number is
implicit in the enum ordering. This alternative is clearer and more
concise:
#define QEMU_TIMER_ATTR_foo BIT(n)
signature.asc
Description: PGP signature
Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/4] Introduce attributes to qemu timer subsystem
On 17/10/2018 11:12, Stefan Hajnoczi wrote: >> Attributes are simple flags, associated with individual timers for their >> whole lifetime. >> They intended to be used to mark individual timers for special handling by >> various qemu features operating at qemu core level. > I'm worried that this sentence suggests various parts of QEMU will stash > state in ts->attributes. That's messy and they shouldn't do this. Make > the field private to qemu-timer.c. Yes, the contents of the fields are private. Are you suggesting a different wording for the commit message or the "QEMU Timer attributes" doc comment, or something more than that? Possibly removing timer_get_attributes altogether? Paolo signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] virtio: Provide version-specific variants of virtio PCI devices
On Fri, Oct 12, 2018 at 11:54:35PM -0300, Eduardo Habkost wrote: > This patch adds separate device types for each of those virtio > device flavors: > > - virtio-*-pci: the existing multi-purpose device types > - Configurable using `disable-legacy` and `disable-modern` > properties > - Legacy driver support is automatically enabled/disabled > depending on the bus where it is plugged > - Supports Conventional PCI and PCI Express buses > (but Conventional PCI is incompatible with > disable-legacy=off) > - Changes PCI vendor/device IDs at runtime > - virtio-*-pci-0.9: legacy virtio device > - Supports Conventional PCI buses only, because > it has a PIO BAR > - virtio-*-pci-1.0-transitional: virtio-1.0 device supporting legacy drivers > - Supports Conventional PCI buses only, because > it has a PIO BAR > - virtio-*-pci-1.0: modern-only > - Supports both Conventional PCI and PCI Express buses If new device types are being created, is it time to decouple the VIRTIO PCI transport from the actual VIRTIO device (blk, net, scsi) like we already have with virtio-mmio? -device virtio-pci-1.0,id=virtio-pci-0 -device virtio-blk,bus=virtio-pci-0,drive=drive0,serial=mydisk That way we avoid lots of boilerplate code and an explosion of new device types. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v5 0/3] Bootstrap Python venv and acceptance/functional tests
On Tue, Oct 16, 2018 at 01:50:39PM -0400, Cleber Rosa wrote: > TL;DR > = > > Allow acceptance tests to be run with `make check-acceptance`. > > Details > === > > This introduces a Python virtual environment that will be setup within > the QEMU build directory, that will contain the exact environment that > tests may require. Looks useful! Acked-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] hw/acpi/nvdimm: Don't take address of fields in packed structs
On Tue, Oct 16, 2018 at 06:52:36PM +0100, Peter Maydell wrote: > Taking the address of a field in a packed struct is a bad idea, because > it might not be actually aligned enough for that pointer type (and > thus cause a crash on dereference on some host architectures). Newer > versions of clang warn about this. Avoid the bug by not using the > "modify in place" byte swapping functions. > > Patch produced with scripts/coccinelle/inplace-byteswaps.cocci. > > Signed-off-by: Peter Maydell > --- > Automatically generated patch, tested with "make check" only. > > hw/acpi/nvdimm.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[Qemu-devel] [PATCH 0/1] Add PKU/OSPKE on Skylake-Server CPU model
This patch adds PKU/OSPKE on Skylake-Server CPU model Tao Xu (1): i386: Add PKU/OSPKE on Skylake-Server CPU model target/i386/cpu.c | 4 1 file changed, 4 insertions(+) -- 2.17.1
[Qemu-devel] [PATCH 1/1] i386: Add PKU/OSPKE on Skylake-Server CPU model
As the release document ref below link (page 13):
https://software.intel.com/sites/default/files/managed/c5/15/\
architecture-instruction-set-extensions-programming-reference.pdf
PKU is supported in Skylake Server (Only Server) and later, and
on Intel(R) Xeon(R) Processor Scalable Family. OSPKE is to reads
the value of PKRU (Instruction of PKU) into EAX and clears EDX.
So PKU/OSPKE are supposed to be in Skylake-Server CPU model.
And PKU/OSPKE 's CPUID has been exposed to QEMU. But PKU/OSPKE
can't be find in Skylake-Server CPU model in the code.
So this patch will fix PKU/OSPKE this issue in Skylake-Server
CPU model.
Signed-off-by: Tao Xu
---
target/i386/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f24295e6e4..6ecd28c8a2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2322,6 +2322,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ |
CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT,
+.features[FEAT_7_0_ECX] =
+CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE,
/* Missing: XSAVES (not supported by some Linux versions,
* including v4.1 to v4.12).
* KVM doesn't yet expose any XSAVES state save component,
@@ -2372,6 +2374,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ |
CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
CPUID_7_0_EBX_AVX512VL,
+.features[FEAT_7_0_ECX] =
+CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE,
/* Missing: XSAVES (not supported by some Linux versions,
* including v4.1 to v4.12).
* KVM doesn't yet expose any XSAVES state save component,
--
2.17.1
Re: [Qemu-devel] [Qemu-block] [PATCH] block/vhdx: Don't take address of fields in packed structs
On Tue, Oct 16, 2018 at 06:09:38PM +0100, Peter Maydell wrote: > Taking the address of a field in a packed struct is a bad idea, because > it might not be actually aligned enough for that pointer type (and > thus cause a crash on dereference on some host architectures). Newer > versions of clang warn about this. Avoid the bug by not using the > "modify in place" byte swapping functions. > > There are a few places where the in-place swap function is > used on something other than a packed struct field; we convert > those anyway, for consistency. > > Patch produced with scripts/coccinelle/inplace-byteswaps.cocci. > > Signed-off-by: Peter Maydell > --- > Usual disclaimer: produced with "make check" only, but purely > automated conversion should be safe. > > block/vhdx.h| 12 ++--- > block/vhdx-endian.c | 118 ++-- > block/vhdx-log.c| 4 +- > block/vhdx.c| 18 +++ > 4 files changed, 76 insertions(+), 76 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [Qemu-block] [PATCH] block/vdi: Don't take address of fields in packed structs
On Tue, Oct 16, 2018 at 06:25:03PM +0100, Peter Maydell wrote: > Taking the address of a field in a packed struct is a bad idea, because > it might not be actually aligned enough for that pointer type (and > thus cause a crash on dereference on some host architectures). Newer > versions of clang warn about this. Avoid the bug by not using the > "modify in place" byte swapping functions. > > There are a few places where the in-place swap function is > used on something other than a packed struct field; we convert > those anyway, for consistency. > > Patch produced with scripts/coccinelle/inplace-byteswaps.cocci. > > There are other places where we take the address of a packed member > in this file for other purposes than passing it to a byteswap > function (all the calls to qemu_uuid_*()); we leave those for now. > > Signed-off-by: Peter Maydell > --- > Another "tested with make check" auto-conversion patch. In this > case, as noted above, it doesn't fix all the warnings for the file, > but we might as well put the easy part of the fix in. I'm not sure > what to do with the qemu_uuid_*() calls. Something like > QemuUUID uuid_link = header->uuid_link; > and then using "qemu_uuid_is_null(uuid_link)" rather than I would take this route. (I think you mean qemu_uuid_is_null(&uuid_link).) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] Revert "icount: remove obsolete warp call"
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> QEMU running with options "-icount,sleep=off -rtc clock=vm" doesn't
> execute emulation at maximum possible speed.
> Target virtual clock may run faster or slower than realtime clock by N
> times, where N value depends on various unrelated conditions (i.e. random
> from the user point of view), or possibly the target hangs completely.
> Bisection shows commit 281b2201e4 ("icount: remove obsolete warp call",
> 2016-03-15) to be the culprit, revert it.
>
> Suggested-by: Artem Pisarenko
> Signed-off-by: Paolo Bonzini
> ---
> cpus.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/cpus.c b/cpus.c
> index 361678e459..2872e7e37c 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1224,6 +1224,10 @@ static void qemu_tcg_rr_wait_io_event(CPUState *cpu)
> {
> while (all_cpu_threads_idle()) {
> stop_tcg_kick_timer();
> +
> +/* Start accounting real time to the virtual clock if the CPUs
> + are idle. */
> +qemu_clock_warp(QEMU_CLOCK_VIRTUAL);
Can't apply, because this function does not exist anymore.
> qemu_cond_wait(cpu->halt_cond, &qemu_global_mutex);
> }
>
> --
> 2.17.1
Pavel Dovgalyuk
Re: [Qemu-devel] [PATCH] pckbd: Convert DPRINTF->trace
On Tue, Oct 16, 2018 at 12:22:32PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > Signed-off-by: Dr. David Alan Gilbert > --- > hw/input/pckbd.c | 19 ++- > hw/input/trace-events | 7 +++ > 2 files changed, 13 insertions(+), 13 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[Qemu-devel] [PATCH 0/3] Modern shell scripting (use $() instead of ``)
Various shell files contain a mix between obsolete `` and modern $(); It would be nice to convert to using $() everywhere. On https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02920.html I just replaced `` in scripts dir, so this series is a thorough cleanup of all obsolete `` in the source tree. Cc: phi...@redhat.com Cc: peter.mayd...@linaro.org Cc: th...@redhat.com Cc: s...@weilnetz.de Cc: kw...@redhat.com Cc: mre...@redhat.com Cc: alex.ben...@linaro.org Cc: f...@redhat.com Mao Zhongyi (3): qemu-iotests: Modern shellscripting (use $() instead of ``) debian-bootstrap.pre: Modern shell scripting (use $() instead of ``) po/Makefile: Modern shell scripting (use $() instead of ``) po/Makefile | 2 +- tests/docker/dockerfiles/debian-bootstrap.pre | 4 +- tests/qemu-iotests/001| 4 +- tests/qemu-iotests/002| 4 +- tests/qemu-iotests/003| 4 +- tests/qemu-iotests/004| 4 +- tests/qemu-iotests/005| 4 +- tests/qemu-iotests/007| 6 +- tests/qemu-iotests/008| 4 +- tests/qemu-iotests/009| 4 +- tests/qemu-iotests/010| 4 +- tests/qemu-iotests/011| 6 +- tests/qemu-iotests/012| 4 +- tests/qemu-iotests/013| 4 +- tests/qemu-iotests/014| 6 +- tests/qemu-iotests/015| 4 +- tests/qemu-iotests/017| 4 +- tests/qemu-iotests/018| 4 +- tests/qemu-iotests/019| 4 +- tests/qemu-iotests/020| 4 +- tests/qemu-iotests/021| 4 +- tests/qemu-iotests/022| 4 +- tests/qemu-iotests/023| 4 +- tests/qemu-iotests/024| 4 +- tests/qemu-iotests/025| 4 +- tests/qemu-iotests/026| 4 +- tests/qemu-iotests/027| 4 +- tests/qemu-iotests/028| 4 +- tests/qemu-iotests/029| 4 +- tests/qemu-iotests/031| 4 +- tests/qemu-iotests/032| 4 +- tests/qemu-iotests/033| 4 +- tests/qemu-iotests/034| 4 +- tests/qemu-iotests/035| 4 +- tests/qemu-iotests/036| 4 +- tests/qemu-iotests/037| 4 +- tests/qemu-iotests/038| 4 +- tests/qemu-iotests/039| 4 +- tests/qemu-iotests/042| 4 +- tests/qemu-iotests/043| 4 +- tests/qemu-iotests/046| 4 +- tests/qemu-iotests/047| 4 +- tests/qemu-iotests/048| 2 +- tests/qemu-iotests/049| 4 +- tests/qemu-iotests/050| 4 +- tests/qemu-iotests/051| 4 +- tests/qemu-iotests/052| 4 +- tests/qemu-iotests/053| 4 +- tests/qemu-iotests/054| 4 +- tests/qemu-iotests/058| 4 +- tests/qemu-iotests/059| 4 +- tests/qemu-iotests/061| 4 +- tests/qemu-iotests/062| 4 +- tests/qemu-iotests/063| 4 +- tests/qemu-iotests/064| 4 +- tests/qemu-iotests/067| 4 +- tests/qemu-iotests/070| 4 +- tests/qemu-iotests/073| 4 +- tests/qemu-iotests/074| 2 +- tests/qemu-iotests/075| 4 +- tests/qemu-iotests/076| 4 +- tests/qemu-iotests/077| 4 +- tests/qemu-iotests/078| 4 +- tests/qemu-iotests/079| 4 +- tests/qemu-iotests/080| 4 +- tests/qemu-iotests/081| 4 +- tests/qemu-iotests/082| 4 +- tests/qemu-iotests/083| 4 +- tests/qemu-iotests/084| 4 +- tests/qemu-iotests/085| 4 +- tests/qemu-iotests/086| 4 +- tests/qemu-iotests/087| 4 +- tests/qemu-iotests/088| 4 +- tests/qemu-iotests/091| 4 +- tests/qemu-iotests/092| 4 +- tests/qemu-iotests/095| 4 +- tests/qemu-iotests/101| 4 +- tests/qemu-iotests/104| 4 +- t
[Qemu-devel] [PATCH 3/3] po/Makefile: Modern shell scripting (use $() instead of ``)
Various shell files contain a mix between obsolete `` and modern $(); It would be nice to convert to using $() everywhere. Cc: phi...@redhat.com Cc: peter.mayd...@linaro.org Cc: th...@redhat.com Cc: s...@weilnetz.de Signed-off-by: Mao Zhongyi --- po/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/Makefile b/po/Makefile index e47e262ee6..10605e8eb3 100644 --- a/po/Makefile +++ b/po/Makefile @@ -36,7 +36,7 @@ clean: install: $(OBJS) for obj in $(OBJS); do \ - base=`basename $$obj .mo`; \ + base=$(basename $$obj .mo); \ $(INSTALL) -d $(DESTDIR)$(prefix)/share/locale/$$base/LC_MESSAGES; \ $(INSTALL) -m644 $$obj $(DESTDIR)$(prefix)/share/locale/$$base/LC_MESSAGES/qemu.mo; \ done -- 2.17.1
[Qemu-devel] [PATCH 2/3] debian-bootstrap.pre: Modern shell scripting (use $() instead of ``)
Various shell files contain a mix between obsolete `` and modern $(); It would be nice to convert to using $() everywhere. Cc: alex.ben...@linaro.org Cc: f...@redhat.com Cc: phi...@redhat.com Signed-off-by: Mao Zhongyi --- tests/docker/dockerfiles/debian-bootstrap.pre | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/docker/dockerfiles/debian-bootstrap.pre b/tests/docker/dockerfiles/debian-bootstrap.pre index 3b0ef95374..c164778c30 100755 --- a/tests/docker/dockerfiles/debian-bootstrap.pre +++ b/tests/docker/dockerfiles/debian-bootstrap.pre @@ -2,7 +2,7 @@ # # Simple wrapper for debootstrap, run in the docker build context # -FAKEROOT=`which fakeroot 2> /dev/null` +FAKEROOT=$(which fakeroot 2> /dev/null) # debootstrap < 1.0.67 generates empty sources.list, see Debian#732255 MIN_DEBOOTSTRAP_VERSION=1.0.67 @@ -52,7 +52,7 @@ fi if [ -z $DEBOOTSTRAP_DIR ]; then NEED_DEBOOTSTRAP=false -DEBOOTSTRAP=`which debootstrap 2> /dev/null` +DEBOOTSTRAP=$(which debootstrap 2> /dev/null) if [ -z $DEBOOTSTRAP ]; then echo "No debootstrap installed, attempting to install from SCM" NEED_DEBOOTSTRAP=true -- 2.17.1
[Qemu-devel] [PATCH 1/3] qemu-iotests: Modern shellscripting (use $() instead of ``)
Various shell files contain a mix between obsolete `` and modern $(); It would be nice to convert to using $() everywhere. `pwd` and `basename $0` are in 231 files under directory tests/qemu-iotests, so replaced it with the following: sed -i 's/`pwd`/$(pwd)/g' $(git grep -l "\`pwd\`") sed -i 's/`basename $0`/$(basename $0)/g' $(git grep -l "basename \$0") A small amount of the rest is manually modified. Cc: kw...@redhat.com Cc: mre...@redhat.com Cc: qemu-bl...@nongnu.org Signed-off-by: Mao Zhongyi --- tests/qemu-iotests/001| 4 +- tests/qemu-iotests/002| 4 +- tests/qemu-iotests/003| 4 +- tests/qemu-iotests/004| 4 +- tests/qemu-iotests/005| 4 +- tests/qemu-iotests/007| 6 +-- tests/qemu-iotests/008| 4 +- tests/qemu-iotests/009| 4 +- tests/qemu-iotests/010| 4 +- tests/qemu-iotests/011| 6 +-- tests/qemu-iotests/012| 4 +- tests/qemu-iotests/013| 4 +- tests/qemu-iotests/014| 6 +-- tests/qemu-iotests/015| 4 +- tests/qemu-iotests/017| 4 +- tests/qemu-iotests/018| 4 +- tests/qemu-iotests/019| 4 +- tests/qemu-iotests/020| 4 +- tests/qemu-iotests/021| 4 +- tests/qemu-iotests/022| 4 +- tests/qemu-iotests/023| 4 +- tests/qemu-iotests/024| 4 +- tests/qemu-iotests/025| 4 +- tests/qemu-iotests/026| 4 +- tests/qemu-iotests/027| 4 +- tests/qemu-iotests/028| 4 +- tests/qemu-iotests/029| 4 +- tests/qemu-iotests/031| 4 +- tests/qemu-iotests/032| 4 +- tests/qemu-iotests/033| 4 +- tests/qemu-iotests/034| 4 +- tests/qemu-iotests/035| 4 +- tests/qemu-iotests/036| 4 +- tests/qemu-iotests/037| 4 +- tests/qemu-iotests/038| 4 +- tests/qemu-iotests/039| 4 +- tests/qemu-iotests/042| 4 +- tests/qemu-iotests/043| 4 +- tests/qemu-iotests/046| 4 +- tests/qemu-iotests/047| 4 +- tests/qemu-iotests/048| 2 +- tests/qemu-iotests/049| 4 +- tests/qemu-iotests/050| 4 +- tests/qemu-iotests/051| 4 +- tests/qemu-iotests/052| 4 +- tests/qemu-iotests/053| 4 +- tests/qemu-iotests/054| 4 +- tests/qemu-iotests/058| 4 +- tests/qemu-iotests/059| 4 +- tests/qemu-iotests/061| 4 +- tests/qemu-iotests/062| 4 +- tests/qemu-iotests/063| 4 +- tests/qemu-iotests/064| 4 +- tests/qemu-iotests/067| 4 +- tests/qemu-iotests/070| 4 +- tests/qemu-iotests/073| 4 +- tests/qemu-iotests/074| 2 +- tests/qemu-iotests/075| 4 +- tests/qemu-iotests/076| 4 +- tests/qemu-iotests/077| 4 +- tests/qemu-iotests/078| 4 +- tests/qemu-iotests/079| 4 +- tests/qemu-iotests/080| 4 +- tests/qemu-iotests/081| 4 +- tests/qemu-iotests/082| 4 +- tests/qemu-iotests/083| 4 +- tests/qemu-iotests/084| 4 +- tests/qemu-iotests/085| 4 +- tests/qemu-iotests/086| 4 +- tests/qemu-iotests/087| 4 +- tests/qemu-iotests/088| 4 +- tests/qemu-iotests/091| 4 +- tests/qemu-iotests/092| 4 +- tests/qemu-iotests/095| 4 +- tests/qemu-iotests/101| 4 +- tests/qemu-iotests/104| 4 +- tests/qemu-iotests/105| 4 +- tests/qemu-iotests/116| 4 +- tests/qemu-iotests/128| 4 +- tests/qemu-iotests/131| 4 +- tests/qemu-iotests/133| 4 +- tests/qemu-iotests/134| 4 +- tests/qemu-iotests/135| 4 +- tests/qemu-iotests/142| 4 +- tests/qemu-iotests/144| 4 +- tests/qemu-iotests/145| 4 +- tests/qemu-iotests/146| 4 +- tests/qemu-iotests/154| 4 +- tests/qemu-iotests/158| 4 +- tests/qemu-iotests/171| 4 +- tests/qemu-iotests/172| 4 +- tests/qemu-iotests/173| 4 +- tests/qemu-iotests/174| 4 +- tests/qemu-iotests/175| 4 +- tests/qemu-iotests/177| 4 +- tests/qemu-iotests/178| 4 +- tests/qemu-iotests/181| 4 +- tests/qemu-iotests/183| 4 +- tests/qemu-iotests/184| 4 +- tests/qemu-iotests/185| 4 +- tests/qemu-iotests/186| 4 +- tests/qemu-iotests/187| 4 +- tests/qemu-iotests/188| 4 +- tests/qemu-iotests/189| 4 +- tests/qemu
Re: [Qemu-devel] modern virtio on HVF
On Tue, Oct 16, 2018 at 06:27:12PM +0300, Roman Bolshakov wrote: > Hello dear subscribers, > > I'm running Linux in QEMU on macOS with hvf accel enabled and having an > issue that is very similar to the KVM bug in nested KVM environments, > where KVM is run under another hypervisor: > https://bugs.launchpad.net/qemu/+bug/1636217 > > > The symptomps are the same as in the bug above. udev hangs unless: > * -machine type=pc-i440fx-X, where X <=2.6 is used > * -accel tcg is used > * -global virtio-pci.disable-modern=on is specified > > The issue was briefly noted on packer mailing list: > https://groups.google.com/forum/#!topic/packer-tool/je2D0LRhWj0 > > If I send Magic SysRQ-t to the VM, I can notice virtio_pci hangs > indefinetly in vp_reset: > [ 48.604482] systemd-udevd D0 121106 0x0100 > [ 48.608093] Call Trace: > [ 48.609701] ? __schedule+0x292/0x880 > [ 48.612076] schedule+0x32/0x80 > [ 48.614189] schedule_timeout+0x15e/0x300 > [ 48.616840] ? call_timer_fn+0x140/0x140 > [ 48.619375] msleep+0x2a/0x40 > [ 48.621284] vp_reset+0x27/0x50 [virtio_pci] > [ 48.624185] register_virtio_device+0x71/0x100 [virtio] > [ 48.627689] virtio_pci_probe+0xad/0x120 [virtio_pci] > [ 48.630825] local_pci_probe+0x44/0xa0 > [ 48.633357] pci_device_probe+0x127/0x140 > [ 48.636085] driver_probe_device+0x297/0x450 > [ 48.638876] __driver_attach+0xd9/0xe0 > [ 48.641484] ? driver_probe_device+0x450/0x450 > [ 48.644393] bus_for_each_dev+0x5a/0x90 > [ 48.646879] bus_add_driver+0x41/0x260 > [ 48.649279] driver_register+0x5b/0xd0 > [ 48.651703] ? 0xc00ac000 > [ 48.653994] do_one_initcall+0x50/0x1b0 > [ 48.656496] do_init_module+0x5a/0x1fa > [ 48.659001] load_module+0x1557/0x1ed0 > [ 48.661507] ? m_show+0x1b0/0x1b0 > [ 48.663725] ? security_capable+0x47/0x60 > [ 48.666435] SYSC_finit_module+0x80/0xb0 > [ 48.669021] do_syscall_64+0x74/0x150 > [ 48.671222] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 > [ 48.674565] RIP: 0033:0x7f71dac73139 > [ 48.677050] RSP: 002b:7ffdcfd35058 EFLAGS: 0246 ORIG_RAX: > 0139 > [ 48.681997] RAX: ffda RBX: 55d25bd01500 RCX: > 7f71dac73139 > [ 48.686608] RDX: RSI: 7f71db5af83d RDI: > 000f > [ 48.691191] RBP: 7f71db5af83d R08: R09: > > [ 48.695750] R10: 000f R11: 0246 R12: > 0002 > [ 48.700568] R13: 55d25bd039b0 R14: R15: > 03938700 > > It looks like virtio backend doesn't return 0 device status after > vp_iowrite8 and vp_reset blocks udev: > while (vp_ioread8(&vp_dev->common->device_status)) > msleep(1) > > What could be the cause of the issue? > Any advices how to triage it are appreciated. I wonder what happened in virtio_pci_probe() -> virtio_pci_modern_probe(). For example, were the BARs properly set up? For starters you can debug the QEMU process to check if virtio_pci_common_read/write() get called for this device (disable all other virtio devices to make life easy). If these functions aren't being called then the guest either got the address wrong or dispatch isn't working for some other reason (hvf?). Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] hw/acpi/nvdimm: Don't take address of fields in packed structs
On 16/10/2018 19:52, Peter Maydell wrote:
> Taking the address of a field in a packed struct is a bad idea, because
> it might not be actually aligned enough for that pointer type (and
> thus cause a crash on dereference on some host architectures). Newer
> versions of clang warn about this. Avoid the bug by not using the
> "modify in place" byte swapping functions.
>
> Patch produced with scripts/coccinelle/inplace-byteswaps.cocci.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> Automatically generated patch, tested with "make check" only.
>
> hw/acpi/nvdimm.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 27eeb6609f5..e53b2cb6819 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -581,7 +581,7 @@ static void nvdimm_dsm_func_read_fit(AcpiNVDIMMState
> *state, NvdimmDsmIn *in,
> int size;
>
> read_fit = (NvdimmFuncReadFITIn *)in->arg3;
> -le32_to_cpus(&read_fit->offset);
> +read_fit->offset = le32_to_cpu(read_fit->offset);
>
> fit = fit_buf->fit;
>
> @@ -742,8 +742,8 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice
> *nvdimm, NvdimmDsmIn *in,
> int size;
>
> get_label_data = (NvdimmFuncGetLabelDataIn *)in->arg3;
> -le32_to_cpus(&get_label_data->offset);
> -le32_to_cpus(&get_label_data->length);
> +get_label_data->offset = le32_to_cpu(get_label_data->offset);
> +get_label_data->length = le32_to_cpu(get_label_data->length);
>
> nvdimm_debug("Read Label Data: offset %#x length %#x.\n",
> get_label_data->offset, get_label_data->length);
> @@ -781,8 +781,8 @@ static void nvdimm_dsm_set_label_data(NVDIMMDevice
> *nvdimm, NvdimmDsmIn *in,
>
> set_label_data = (NvdimmFuncSetLabelDataIn *)in->arg3;
>
> -le32_to_cpus(&set_label_data->offset);
> -le32_to_cpus(&set_label_data->length);
> +set_label_data->offset = le32_to_cpu(set_label_data->offset);
> +set_label_data->length = le32_to_cpu(set_label_data->length);
>
> nvdimm_debug("Write Label Data: offset %#x length %#x.\n",
> set_label_data->offset, set_label_data->length);
> @@ -877,9 +877,9 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val,
> unsigned size)
> in = g_new(NvdimmDsmIn, 1);
> cpu_physical_memory_read(dsm_mem_addr, in, sizeof(*in));
>
> -le32_to_cpus(&in->revision);
> -le32_to_cpus(&in->function);
> -le32_to_cpus(&in->handle);
> +in->revision = le32_to_cpu(in->revision);
> +in->function = le32_to_cpu(in->function);
> +in->handle = le32_to_cpu(in->handle);
>
> nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
> in->handle, in->function);
>
Re: [Qemu-devel] [PATCH] Revert "icount: remove obsolete warp call"
See my last comment in bug report. This kind of modification, even adapted to changed function name, doesn't solve issue. I thought long time that it does, but once I catched qemu with a hang. And of course, I wasn't able to reproduce it. So it just better hides issue. Take a look at alternative solution from QBox: https://git.greensocs.com/qemu/qbox/commit/a8ed106032e375e715a531d6e93e4d9ec295dbdb I didn't catched fail with it (yet). -- С уважением, Артем Писаренко
Re: [Qemu-devel] [Qemu-block] [PATCH] migration/block-dirty-bitmap: fix Coverity CID1390625
On Tue, Oct 16, 2018 at 04:20:18PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Theoretically possible that we finish the skipping loop with bs = NULL
> and the following code will crash trying to dereference it. Fix that.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> migration/block-dirty-bitmap.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index 477826330c..6de808f95f 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -288,6 +288,10 @@ static int init_dirty_bitmap_migration(void)
> bs = backing_bs(bs);
> }
>
> +if (!bs || bs->implicit) {
> +continue;
> +}
> +
> for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
> bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
> {
Previous discussion:
http://qemu.11.n7.nabble.com/PATCH-migration-Appease-coverity-skip-empty-block-trees-td582504.html
I've CCed John so he can take a look.
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 3/3] po/Makefile: Modern shell scripting (use $() instead of ``)
On 2018-10-17 11:44, Mao Zhongyi wrote: > Various shell files contain a mix between obsolete `` > and modern $(); It would be nice to convert to using $() > everywhere. > > Cc: phi...@redhat.com > Cc: peter.mayd...@linaro.org > Cc: th...@redhat.com > Cc: s...@weilnetz.de > > Signed-off-by: Mao Zhongyi > --- > po/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/po/Makefile b/po/Makefile > index e47e262ee6..10605e8eb3 100644 > --- a/po/Makefile > +++ b/po/Makefile > @@ -36,7 +36,7 @@ clean: > > install: $(OBJS) > for obj in $(OBJS); do \ > - base=`basename $$obj .mo`; \ > + base=$(basename $$obj .mo); \ You're changing a Makefile here, so you need to "escape" the "$" by doubling it: base=$$(basename $$obj .mo); \ Thomas
Re: [Qemu-devel] [RFC 5/5] virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size
I am pretty sure that you can create misleading warnings in case you
migrate at the wrong time. (migrate while half the 64k page is inflated,
on the new host the other part is inflated - a warning when switching to
another 64k page).
>>>
>>> Yes we can get bogus warnings across migration with this. I was
>>> considering that an acceptable price, but I'm open to better
>>> alternatives.
>>
>> Is maybe reporting a warning on a 64k host when realizing the better
>> approach than on every inflation?
>>
>> "host page size does not match virtio-balloon page size. If the guest
>> has a different page size than the host, inflating the balloon might not
>> effectively free up memory."
>
> That might work - I'll see what I can come up with. One complication
> is that theoretically at least, you can have multiple host page sizes
> (main memory in normal pages, a DIMM in hugepages). That makes the
> condition on which the warning should be issued a bit fiddly to work
> out.
I assume issuing a warning on these strange systems would not hurt after
all. ("there is a chance this might not work")
>
>> Or reporting a warning whenever changing the balloon target size.
>
> I'm not sure what you mean by this.
I mean when setting e.g. qmp_balloon() to something != 0. This avoids a
warning when a virtio-balloon device is silently created (e.g. by
libvirt?) but never used.
Checking in virtio_balloon_to_target would be sufficient I guess.
>
>>
>>>
> +free(balloon->pbp);
> +balloon->pbp = NULL;
> }
>
> -ram_block_discard_range(rb, ram_offset, rb_page_size);
> -/* We ignore errors from ram_block_discard_range(), because it has
> - * already reported them, and failing to discard a balloon page is
> - * not fatal */
> +if (!balloon->pbp) {
> +/* Starting on a new host page */
> +size_t bitlen = BITS_TO_LONGS(subpages) * sizeof(unsigned long);
> +balloon->pbp = g_malloc0(sizeof(PartiallyBalloonedPage) +
> bitlen);
> +balloon->pbp->rb = rb;
> +balloon->pbp->base = host_page_base;
> +}
> +
> +bitmap_set(balloon->pbp->bitmap,
> + (ram_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE,
> + subpages);
> +
> +if (bitmap_full(balloon->pbp->bitmap, subpages)) {
> +/* We've accumulated a full host page, we can actually discard
> + * it now */
> +
> +ram_block_discard_range(rb, balloon->pbp->base, rb_page_size);
> +/* We ignore errors from ram_block_discard_range(), because it
> + * has already reported them, and failing to discard a balloon
> + * page is not fatal */
> +
> +free(balloon->pbp);
> +balloon->pbp = NULL;
> +}
> }
No, not a fan of this approach.
>>>
>>> I can see why, but I really can't see what else to do without breaking
>>> existing, supported, working (albeit by accident) setups.
>>
>> Is there any reason to use this more complicated "allow random freeing"
>> approach over a simplistic sequential freeing I propose? Then we can
>> simply migrate the last freed page and should be fine.
>
> Well.. your approach is probably simpler in terms of the calculations
> that need to be done, though only very slightly. I think my approach
> is conceptually clearer though, since we're explicitly checking for
> exactly the condition we need, rather than something we thing should
> match up with that condition.
I prefer to keep it simple where possible. We expect sequential freeing,
so it's easy to implement with only one additional uint64_t that can be
easily migrated. Having to use bitmaps + alloc/free is not really needed.
If you insist, at least try to get rid of the malloc to e.g. simplify
migration. (otherwise, please add freeing code on unrealize(), I guess
you are missing that right now)
--
Thanks,
David / dhildenb
Re: [Qemu-devel] When it's okay to treat OOM as fatal?
On Tue, Oct 16, 2018 at 03:01:29PM +0200, Markus Armbruster wrote: > Anything that pages commonly becomes unusable long before > allocations fail. Anything that overcommits will send you a (commonly > lethal) signal instead. Anything that tries handling OOM gracefully, > and manages to dodge both these bullets somehow, will commonly get it > wrong and crash. In the block layer blk_try_blockalign() (previously qemu_try_blockalign()) is used because significant amounts of memory can be allocated by the untrusted guest or untrusted disk image files. I think the error handling is reasonable in those cases: 1. QEMU startup or disk hotplug fail with a nice error message OR 2. An I/O request is failed (ultimately just EIO error reporting but it's better than killing the QEMU process!) I'm pretty sure ENOMEM errors are possible even when memory overcommit is enabled. My thinking has been to use g_new() for small QEMU-internal structures and g_try_new() for large amounts of memory allocated in response to untrusted inputs. (Untrusted inputs must never be used for unbounded allocation sizes but those bounded sizes can still be large.) Stefan signature.asc Description: PGP signature
[Qemu-devel] [PULL 00/15] qtest and misc patches
Hi Peter, the following changes since commit dddb37495b844270088e68e3bf30b764d48d863f: Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20181015.0' into staging (2018-10-15 18:44:04 +0100) are available in the git repository at: https://gitlab.com/huth/qemu.git tags/pull-request-2018-10-17 for you to fetch changes up to 7fc527cdc9cfd139dee1d90e3220a884229a7698: configure: remove glib_subprocess check (2018-10-17 09:01:41 +0200) - Updates for qtest entries in test/Makefile.include - Simple updates for some shell scripts - Misc simple patches for files without regular subsystem pull requests Eric Blake (1): tests: Prevent more accidental test disabling John Arbuckle (1): qemu-common.h: update copyright date to 2018 Liu Yuan (1): MAINTAINERS: update block/sheepdog maintainers Mao Zhongyi (3): archive-source.sh: Modern shell scripting (use $() instead of ``) git-submodule.sh: Modern shell scripting (use $() instead of ``) show-fixed-bugs.sh: Modern shell scripting (use $() instead of ``) Marc-André Lureau (1): configure: remove glib_subprocess check Paolo Bonzini (1): tests: remove gcov-files- variables Philippe Mathieu-Daudé (2): gdbstub: Remove unused include mailmap: Fix Reimar Döffinger name Thomas Huth (5): target/cris/translate: Get rid of qemu_log_separate() qemu/compiler: Wrap __attribute__((flatten)) in a macro hw/core/generic-loader: Set a category for the generic-loader device cpu: Provide a proper prototype for target_words_bigendian() in a header hw/core/generic-loader: Compile only once, not for each target .mailmap | 3 +- MAINTAINERS| 1 - configure | 6 -- docs/devel/testing.rst | 4 +- exec.c | 5 -- fpu/softfloat.c| 39 + gdbstub.c | 1 - hw/core/Makefile.objs | 2 +- hw/core/generic-loader.c | 7 +-- hw/virtio/virtio.c | 1 - include/qemu-common.h | 2 +- include/qemu/compiler.h| 15 + include/qom/cpu.h | 11 qom/cpu.c | 1 - scripts/archive-source.sh | 4 +- scripts/git-submodule.sh | 4 +- scripts/show-fixed-bugs.sh | 10 ++-- target/cris/translate.c| 6 +- tests/Makefile.include | 140 - 19 files changed, 69 insertions(+), 193 deletions(-)
[Qemu-devel] [PULL 10/15] mailmap: Fix Reimar Döffinger name
From: Philippe Mathieu-Daudé This probably happened when interpreting the utf8 name as latin1. Fixes dbbaaff6867 and f4e94dfefb6. Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- .mailmap | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 2c2b9b1..6f2ff22 100644 --- a/.mailmap +++ b/.mailmap @@ -33,5 +33,6 @@ Justin Terry (VM) Justin Terry (VM) via Qemu-devel +Reimar Döffinger -- 1.8.3.1
Re: [Qemu-devel] [PATCH] call HotplugHandler->plug() as the last step in device realization
On Tue, Oct 16, 2018 at 03:33:40PM +0200, Igor Mammedov wrote: > When [2] was fixed it was agreed that adding and calling post_plug() > callback after device_reset() was low risk approach to hotfix issue > right before release. So it was merged instead of moving already > existing plug() callback after device_reset() is called which would > be more risky and require all plug() callbacks audit. > > Looking at the current plug() callbacks, it doesn't seem that moving > plug() callback after device_reset() is breaking anything, so here > goes agreed upon [3] proper fix which essentially reverts [1][2] > and moves plug() callback after device_reset(). > This way devices always comes to plug() stage, after it's been fully > initialized (including being reset), which fixes race condition [2] > without need for an extra post_plug() callback. > > 1. (25e897881 "qdev: add HotplugHandler->post_plug() callback") > 2. (8449bcf94 "virtio-scsi: fix hotplug ->reset() vs event race") > 3. https://www.mail-archive.com/qemu-devel@nongnu.org/msg549915.html > > Signed-off-by: Igor Mammedov > --- > TODO: > remove usage of Error** from plug() callback, we need to factor out > pre_plug part from plug() callbacks, before proceeding with it. > DavidH has recently finished it for pc-dimm/memory_devices, cpus > mostly have pre_plug parts factored out, but there still are parts > that could fail so it needs some more work to eliminate failure points > from plug() callbacks. Meanwhile, I'll plan to treat other misc > handlers (pci[e]/acpi/usb/...) and introduce pre_plug() where > necessary. > --- > include/hw/hotplug.h | 11 --- > hw/core/hotplug.c | 10 -- > hw/core/qdev.c| 16 ++-- > hw/scsi/virtio-scsi.c | 11 +-- > 4 files changed, 7 insertions(+), 41 deletions(-) Thanks. I think we're okay from the virtio-scsi perspective. I have not audited all devices. Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[Qemu-devel] [PULL 04/15] tests: remove gcov-files- variables
From: Paolo Bonzini
Commit 31d2dda ("build-system: remove per-test GCOV reporting", 2018-06-20)
removed users of the variables, since those uses can be replaced by a simple
overall report produced by gcovr. However, the variables were never removed.
Do it now.
Signed-off-by: Paolo Bonzini
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Alex Bennée
[thuth: Fixed up contextual conflicts with the patch from Eric]
Signed-off-by: Thomas Huth
---
docs/devel/testing.rst | 4 +-
tests/Makefile.include | 118 -
2 files changed, 1 insertion(+), 121 deletions(-)
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 727c401..fcfad87 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -43,15 +43,13 @@ add a new unit test:
3. Add the test to ``tests/Makefile.include``. First, name the unit test
program and add it to ``$(check-unit-y)``; then add a rule to build the
- executable. Optionally, you can add a magical variable to support ``gcov``.
- For example:
+ executable. For example:
.. code::
check-unit-y += tests/foo-test$(EXESUF)
tests/foo-test$(EXESUF): tests/foo-test.o $(test-util-obj-y)
...
- gcov-files-foo-test-y = util/foo.c
Since unit tests don't require environment variables, the simplest way to debug
a unit test failure is often directly invoking it or even running it under
diff --git a/tests/Makefile.include b/tests/Makefile.include
index b68cbb3..7fe8578 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -39,106 +39,61 @@ SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \
$(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak)))
check-unit-y += tests/check-qdict$(EXESUF)
-gcov-files-check-qdict-y = qobject/qdict.c
check-unit-y += tests/check-block-qdict$(EXESUF)
-gcov-files-check-block-qdict-y = qobject/block-qdict.c
check-unit-y += tests/test-char$(EXESUF)
-gcov-files-check-qdict-y = chardev/char.c
check-unit-y += tests/check-qnum$(EXESUF)
-gcov-files-check-qnum-y = qobject/qnum.c
check-unit-y += tests/check-qstring$(EXESUF)
-gcov-files-check-qstring-y = qobject/qstring.c
check-unit-y += tests/check-qlist$(EXESUF)
-gcov-files-check-qlist-y = qobject/qlist.c
check-unit-y += tests/check-qnull$(EXESUF)
-gcov-files-check-qnull-y = qobject/qnull.c
check-unit-y += tests/check-qobject$(EXESUF)
check-unit-y += tests/check-qjson$(EXESUF)
-gcov-files-check-qjson-y = qobject/qjson.c
check-unit-y += tests/check-qlit$(EXESUF)
-gcov-files-check-qlit-y = qobject/qlit.c
check-unit-y += tests/test-qobject-output-visitor$(EXESUF)
-gcov-files-test-qobject-output-visitor-y = qapi/qobject-output-visitor.c
check-unit-y += tests/test-clone-visitor$(EXESUF)
-gcov-files-test-clone-visitor-y = qapi/qapi-clone-visitor.c
check-unit-y += tests/test-qobject-input-visitor$(EXESUF)
-gcov-files-test-qobject-input-visitor-y = qapi/qobject-input-visitor.c
check-unit-y += tests/test-qmp-cmds$(EXESUF)
-gcov-files-test-qmp-cmds-y = qapi/qmp-dispatch.c
check-unit-y += tests/test-string-input-visitor$(EXESUF)
-gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c
check-unit-y += tests/test-string-output-visitor$(EXESUF)
-gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c
check-unit-y += tests/test-qmp-event$(EXESUF)
-gcov-files-test-qmp-event-y += qapi/qmp-event.c
check-unit-y += tests/test-opts-visitor$(EXESUF)
-gcov-files-test-opts-visitor-y = qapi/opts-visitor.c
check-unit-y += tests/test-coroutine$(EXESUF)
-gcov-files-test-coroutine-y = coroutine-$(CONFIG_COROUTINE_BACKEND).c
check-unit-y += tests/test-visitor-serialization$(EXESUF)
check-unit-y += tests/test-iov$(EXESUF)
-gcov-files-test-iov-y = util/iov.c
check-unit-y += tests/test-aio$(EXESUF)
-gcov-files-test-aio-y = util/async.c util/qemu-timer.o
-gcov-files-test-aio-$(CONFIG_WIN32) += util/aio-win32.c
-gcov-files-test-aio-$(CONFIG_POSIX) += util/aio-posix.c
check-unit-y += tests/test-aio-multithread$(EXESUF)
-gcov-files-test-aio-multithread-y = $(gcov-files-test-aio-y)
-gcov-files-test-aio-multithread-y += util/qemu-coroutine.c tests/iothread.c
check-unit-y += tests/test-throttle$(EXESUF)
check-unit-y += tests/test-thread-pool$(EXESUF)
-gcov-files-test-thread-pool-y = thread-pool.c
-gcov-files-test-hbitmap-y = util/hbitmap.c
check-unit-y += tests/test-hbitmap$(EXESUF)
-gcov-files-test-hbitmap-y = blockjob.c
check-unit-y += tests/test-bdrv-drain$(EXESUF)
check-unit-y += tests/test-blockjob$(EXESUF)
check-unit-y += tests/test-blockjob-txn$(EXESUF)
check-unit-y += tests/test-block-backend$(EXESUF)
check-unit-y += tests/test-x86-cpuid$(EXESUF)
# all code tested by test-x86-cpuid is inside topology.h
-gcov-files-test-x86-cpuid-y =
ifeq ($(CONFIG_SOFTMMU),y)
check-unit-y += tests/test-xbzrle$(EXESUF)
-gcov-files-test-xbzrle-y = migration/xbzrle.c
check-unit-$(CONFIG_POSIX) += tests/test-vmstate$(EXESUF)
endif
check-unit-y += tests/test-cutils$(EXESUF)
-gcov-files-test-cutils-y
[Qemu-devel] [PULL 09/15] show-fixed-bugs.sh: Modern shell scripting (use $() instead of ``)
From: Mao Zhongyi
Various shell files contain a mix between obsolete ``
and modern $(); It would be nice to convert to using $()
everywhere.
Signed-off-by: Mao Zhongyi
Reviewed-by: Thomas Huth
Signed-off-by: Thomas Huth
---
scripts/show-fixed-bugs.sh | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/scripts/show-fixed-bugs.sh b/scripts/show-fixed-bugs.sh
index 36f3068..a095a4d 100755
--- a/scripts/show-fixed-bugs.sh
+++ b/scripts/show-fixed-bugs.sh
@@ -23,10 +23,10 @@ while getopts "s:e:cbh" opt; do
done
if [ "x$start" = "x" ]; then
-start=`git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1`
+start=$(git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1)
fi
if [ "x$end" = "x" ]; then
-end=`git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 1`
+end=$(git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 1)
fi
if [ "x$start" = "x" ] || [ "x$end" = "x" ]; then
@@ -38,9 +38,9 @@ fi
echo "Searching git log for bugs in the range $start..$end"
urlstr='https://bugs.launchpad.net/\(bugs\|qemu/+bug\)/'
-bug_urls=`git log $start..$end \
+bug_urls=$(git log $start..$end \
| sed -n '\,'"$urlstr"', s,\(.*\)\('"$urlstr"'\)\([0-9]*\).*,\2\4,p' \
- | sort -u`
+ | sort -u)
echo Found bug URLs:
for i in $bug_urls ; do echo " $i" ; done
@@ -68,7 +68,7 @@ elif [ "x$show_in_browser" = "x1" ]; then
bugbrowser=xdg-open
elif command -v gnome-open >/dev/null 2>&1; then
bugbrowser=gnome-open
-elif [ "`uname`" = "Darwin" ]; then
+elif [ "$(uname)" = "Darwin" ]; then
bugbrowser=open
elif command -v sensible-browser >/dev/null 2>&1; then
bugbrowser=sensible-browser
--
1.8.3.1
Re: [Qemu-devel] [PATCH] target/arm : add pvpanic mmio device
Hi Peng,
On 17/10/2018 11:23, Peng Hao wrote:
> Add pvpanic mmio device that is similar to x86's pvpanic device.
>
> Signed-off-by: Peng Hao
> ---
> default-configs/arm-softmmu.mak | 2 +-
> hw/arm/virt.c | 21
> hw/misc/Makefile.objs | 1 +
> hw/misc/pvpanic-mmio.c | 76
> +
> include/hw/arm/virt.h | 1 +
> include/hw/misc/pvpanic-mmio.h | 12 +++
> 6 files changed, 112 insertions(+), 1 deletion(-)
> create mode 100644 hw/misc/pvpanic-mmio.c
> create mode 100644 include/hw/misc/pvpanic-mmio.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 2420491..4713c92 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -43,7 +43,7 @@ CONFIG_USB_MUSB=y
> CONFIG_USB_EHCI_SYSBUS=y
> CONFIG_PLATFORM_BUS=y
> CONFIG_VIRTIO_MMIO=y
> -
> +CONFIG_PVPANIC_MMIO=y
> CONFIG_ARM11MPCORE=y
> CONFIG_A9MPCORE=y
> CONFIG_A15MPCORE=y
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a472566..ab41128 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -140,6 +140,7 @@ static const MemMapEntry a15memmap[] = {
> [VIRT_UART] = { 0x0900, 0x1000 },
> [VIRT_RTC] ={ 0x0901, 0x1000 },
> [VIRT_FW_CFG] = { 0x0902, 0x0018 },
> +[VIRT_PVPANIC_MMIO] = { 0x09020018, 0x0002 },
> [VIRT_GPIO] = { 0x0903, 0x1000 },
> [VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
> [VIRT_SMMU] = { 0x0905, 0x0002 },
> @@ -798,6 +799,24 @@ static void create_gpio(const VirtMachineState *vms,
> qemu_irq *pic)
> g_free(nodename);
> }
>
> +static void create_pvpanic_device(const VirtMachineState *vms)
> +{
> +char *nodename;
> +hwaddr base = vms->memmap[VIRT_PVPANIC_MMIO].base;
> +hwaddr size = vms->memmap[VIRT_PVPANIC_MMIO].size;
> +
> +sysbus_create_simple("pvpanic-mmio", base, NULL);
> +
> +nodename = g_strdup_printf("/pvpanic-mmio@%" PRIx64, base);
> +qemu_fdt_add_subnode(vms->fdt, nodename);
> +qemu_fdt_setprop_string(vms->fdt, nodename,
> +"compatible", "pvpanic,mmio");
> +qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
> + 2, base, 2, size);
> +g_free(nodename);
> +
> +}
> +
> static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic)
> {
> int i;
> @@ -1544,6 +1563,8 @@ static void machvirt_init(MachineState *machine)
>
> create_pcie(vms, pic);
>
> +create_pvpanic_device(vms);
> +
> create_gpio(vms, pic);
>
> /* Create mmio transports, so the user can create virtio backends
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 6d50b03..6326260 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -71,6 +71,7 @@ obj-$(CONFIG_IOTKIT_SYSCTL) += iotkit-sysctl.o
> obj-$(CONFIG_IOTKIT_SYSINFO) += iotkit-sysinfo.o
>
> obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
> obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
> obj-$(CONFIG_AUX) += auxbus.o
> obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
> diff --git a/hw/misc/pvpanic-mmio.c b/hw/misc/pvpanic-mmio.c
> new file mode 100644
> index 000..c7f373e
> --- /dev/null
> +++ b/hw/misc/pvpanic-mmio.c
> @@ -0,0 +1,76 @@
> +#include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/log.h"
> +#include "hw/misc/pvpanic-mmio.h"
> +
> +#define PVPANIC_MMIO_FEAT_CRASHED 0
> +
> +#define PVPANIC_MMIO_CRASHED(1 << PVPANIC_MMIO_FEAT_CRASHED)
> +
> +static void handle_mmio_event(int event)
> +{
> +static bool logged;
> +
> +if (event & ~PVPANIC_MMIO_CRASHED && !logged) {
> +qemu_log_mask(LOG_GUEST_ERROR, "pvpanic-mmio: unknown event %#x.\n",
> event);
> +logged = true;
> +}
> +
> +if (event & PVPANIC_MMIO_CRASHED) {
> +qemu_system_guest_panicked(NULL);
> +return;
> +}
It would be easier to maintain a single pvpanic device. There is no
improvement here, it is the same handler than 'pvpanic.c'.
The current pvpanic device is not x86-only, it only implements the
ioport API.
If you want to use the mmio API, please add it there.
Basically you don't have to write any more code that in this patch, but
just move it in the pvpanic.c file.
Thanks,
Phil.
> +}
> +
> +static uint64_t pvpanic_mmio_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +return -1;
> +}
> +
> +static void pvpanic_mmio_write(void *opaque, hwaddr addr, uint64_t value,
> + unsigned size)
> +{
> + handle_mmio_event(value);
> +}
> +
> +static const MemoryRegionOps pvpanic_mmio_ops = {
> +.read = pvpanic_mmio_read,
> +.write = pvpanic_mmio_write,
> +.impl = {
> +.min_access_size = 1,
> +.max_access_size = 2,
> +},
[Qemu-devel] [PULL 03/15] tests: Prevent more accidental test disabling
From: Eric Blake GNU make is perfectly happy to use 'check-FOO-y += bar' to initialize check-FOO-y. (GNU Automake strictly insists that you cannot use += until after an initial = per variable, but thankfully we aren't using automake). As we have had more than one instance where copy-and-paste of 'check-FOO-y = bar' from a first test under category FOO into an additional test, which ends up disabling the first (see commits 992159c7 and 4429532b), it's better to just always use the form that survives copy-and-paste, even for categories that don't currently add more than one test. Done with s/^\(check-[a-z]*-y \)=/\1+=/g Signed-off-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/Makefile.include | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 5eadfd5..b68cbb3 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -38,7 +38,7 @@ $(SRC_PATH)/scripts/qapi-gen.py SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \ $(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak))) -check-unit-y = tests/check-qdict$(EXESUF) +check-unit-y += tests/check-qdict$(EXESUF) gcov-files-check-qdict-y = qobject/qdict.c check-unit-y += tests/check-block-qdict$(EXESUF) gcov-files-check-block-qdict-y = qobject/block-qdict.c @@ -181,7 +181,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh # All QTests for now are POSIX-only, but the dependencies are # really in libqtest, not in the testcases themselves. -check-qtest-generic-y = tests/qmp-test$(EXESUF) +check-qtest-generic-y += tests/qmp-test$(EXESUF) gcov-files-generic-y = monitor.c qapi/qmp-dispatch.c check-qtest-generic-y += tests/qmp-cmd-test$(EXESUF) @@ -324,13 +324,13 @@ check-qtest-x86_64-$(CONFIG_SDHCI) += tests/sdhci-test$(EXESUF) gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y)) -check-qtest-alpha-y = tests/boot-serial-test$(EXESUF) +check-qtest-alpha-y += tests/boot-serial-test$(EXESUF) -check-qtest-hppa-y = tests/boot-serial-test$(EXESUF) +check-qtest-hppa-y += tests/boot-serial-test$(EXESUF) check-qtest-m68k-y = tests/boot-serial-test$(EXESUF) -check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF) +check-qtest-microblaze-y += tests/boot-serial-test$(EXESUF) check-qtest-mips-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF) @@ -338,7 +338,7 @@ check-qtest-mips64-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF) check-qtest-mips64el-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF) -check-qtest-moxie-y = tests/boot-serial-test$(EXESUF) +check-qtest-moxie-y += tests/boot-serial-test$(EXESUF) check-qtest-ppc-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF) check-qtest-ppc-y += tests/boot-order-test$(EXESUF) @@ -348,7 +348,7 @@ check-qtest-ppc-y += tests/boot-serial-test$(EXESUF) check-qtest-ppc-y += tests/m48t59-test$(EXESUF) gcov-files-ppc-y += hw/timer/m48t59.c -check-qtest-ppc64-y = $(check-qtest-ppc-y) +check-qtest-ppc64-y += $(check-qtest-ppc-y) gcov-files-ppc64-y = $(subst ppc-softmmu/,ppc64-softmmu/,$(gcov-files-ppc-y)) check-qtest-ppc64-y += tests/spapr-phb-test$(EXESUF) gcov-files-ppc64-y += ppc64-softmmu/hw/ppc/spapr_pci.c @@ -377,7 +377,7 @@ check-qtest-sh4-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF) check-qtest-sh4eb-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF) -check-qtest-sparc-y = tests/prom-env-test$(EXESUF) +check-qtest-sparc-y += tests/prom-env-test$(EXESUF) check-qtest-sparc-y += tests/m48t59-test$(EXESUF) gcov-files-sparc-y = hw/timer/m48t59.c check-qtest-sparc-y += tests/boot-serial-test$(EXESUF) @@ -386,7 +386,7 @@ check-qtest-sparc64-$(CONFIG_ISA_TESTDEV) = tests/endianness-test$(EXESUF) check-qtest-sparc64-y += tests/prom-env-test$(EXESUF) check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF) -check-qtest-arm-y = tests/tmp105-test$(EXESUF) +check-qtest-arm-y += tests/tmp105-test$(EXESUF) check-qtest-arm-y += tests/pca9552-test$(EXESUF) check-qtest-arm-y += tests/ds1338-test$(EXESUF) check-qtest-arm-y += tests/m25p80-test$(EXESUF) @@ -404,9 +404,9 @@ check-qtest-aarch64-$(CONFIG_SDHCI) += tests/sdhci-test$(EXESUF) check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF) check-qtest-aarch64-y += tests/migration-test$(EXESUF) -check-qtest-microblazeel-y = $(check-qtest-microblaze-y) +check-qtest-microblazeel-y += $(check-qtest-microblaze-y) -check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) +check-qtest-xtensaeb-y += $(check-qtest-xtensa-y) check-qtest-s390x-y = tests/boot-serial-test$(EXESUF) check-qtest-s390x-$(CONFIG_SLIRP) += tests/pxe-test$(EXESUF) -- 1.8.3.1
[Qemu-devel] [PULL 06/15] MAINTAINERS: update block/sheepdog maintainers
From: Liu Yuan E-mail to one of block/sheepdog maintainers Mitake Hitoshi bounces : unknown user: "mitake.hitoshi" and no current address is known. So just remove it. Signed-off-by: Liu Yuan Reviewed-by: Markus Armbruster Signed-off-by: Thomas Huth --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index fb81b3a..40672c4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2023,7 +2023,6 @@ F: block/rbd.c T: git git://github.com/codyprime/qemu-kvm-jtc.git block Sheepdog -M: Hitoshi Mitake M: Liu Yuan M: Jeff Cody L: qemu-bl...@nongnu.org -- 1.8.3.1
[Qemu-devel] [PULL 12/15] hw/core/generic-loader: Set a category for the generic-loader device
Each device that is instantiatable by the users should be marked with
a category. Since the generic-loader does not fit anywhere else, put
it into the MISC category.
Reviewed-by: Alistair Francis
Reviewed-by: Cornelia Huck
Signed-off-by: Thomas Huth
---
hw/core/generic-loader.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index fde32cb..be29ae1 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -204,6 +204,7 @@ static void generic_loader_class_init(ObjectClass *klass,
void *data)
dc->unrealize = generic_loader_unrealize;
dc->props = generic_loader_props;
dc->desc = "Generic Loader";
+set_bit(DEVICE_CATEGORY_MISC, dc->categories);
}
static TypeInfo generic_loader_info = {
--
1.8.3.1
[Qemu-devel] [PULL 15/15] configure: remove glib_subprocess check
From: Marc-André Lureau This should have been removed as part of commit 692fbdf9f4c6f6bafd0b3a4d4f94973effd3bbae. Signed-off-by: Marc-André Lureau Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- configure | 6 -- 1 file changed, 6 deletions(-) diff --git a/configure b/configure index 8af2be9..9138af3 100755 --- a/configure +++ b/configure @@ -3531,12 +3531,6 @@ if ! compile_prog "$CFLAGS" "$LIBS" ; then "build target" fi -# g_test_trap_subprocess added in 2.38. Used by some tests. -glib_subprocess=yes -if ! $pkg_config --atleast-version=2.38 glib-2.0; then -glib_subprocess=no -fi - # Silence clang 3.5.0 warnings about glib attribute __alloc_size__ usage cat > $TMPC << EOF #include -- 1.8.3.1
[Qemu-devel] [PULL 11/15] qemu/compiler: Wrap __attribute__((flatten)) in a macro
Older versions of Clang (before 3.5) and GCC (before 4.1) do not
support the "__attribute__((flatten))" yet. We don't care about
such old versions of GCC anymore, but since Clang 3.4 is still
used in EPEL for RHEL7 / CentOS 7, we should not use this attribute
directly but with a wrapper macro instead.
Reviewed-by: Alex Bennée
Reviewed-by: Richard Henderson
Signed-off-by: Thomas Huth
---
fpu/softfloat.c | 39 +++
include/qemu/compiler.h | 15 +++
2 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 46ae206..e1eef95 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -726,8 +726,7 @@ static FloatParts addsub_floats(FloatParts a, FloatParts b,
bool subtract,
* IEC/IEEE Standard for Binary Floating-Point Arithmetic.
*/
-float16 __attribute__((flatten)) float16_add(float16 a, float16 b,
- float_status *status)
+float16 QEMU_FLATTEN float16_add(float16 a, float16 b, float_status *status)
{
FloatParts pa = float16_unpack_canonical(a, status);
FloatParts pb = float16_unpack_canonical(b, status);
@@ -736,8 +735,7 @@ float16 __attribute__((flatten)) float16_add(float16 a,
float16 b,
return float16_round_pack_canonical(pr, status);
}
-float32 __attribute__((flatten)) float32_add(float32 a, float32 b,
- float_status *status)
+float32 QEMU_FLATTEN float32_add(float32 a, float32 b, float_status *status)
{
FloatParts pa = float32_unpack_canonical(a, status);
FloatParts pb = float32_unpack_canonical(b, status);
@@ -746,8 +744,7 @@ float32 __attribute__((flatten)) float32_add(float32 a,
float32 b,
return float32_round_pack_canonical(pr, status);
}
-float64 __attribute__((flatten)) float64_add(float64 a, float64 b,
- float_status *status)
+float64 QEMU_FLATTEN float64_add(float64 a, float64 b, float_status *status)
{
FloatParts pa = float64_unpack_canonical(a, status);
FloatParts pb = float64_unpack_canonical(b, status);
@@ -756,8 +753,7 @@ float64 __attribute__((flatten)) float64_add(float64 a,
float64 b,
return float64_round_pack_canonical(pr, status);
}
-float16 __attribute__((flatten)) float16_sub(float16 a, float16 b,
- float_status *status)
+float16 QEMU_FLATTEN float16_sub(float16 a, float16 b, float_status *status)
{
FloatParts pa = float16_unpack_canonical(a, status);
FloatParts pb = float16_unpack_canonical(b, status);
@@ -766,8 +762,7 @@ float16 __attribute__((flatten)) float16_sub(float16 a,
float16 b,
return float16_round_pack_canonical(pr, status);
}
-float32 __attribute__((flatten)) float32_sub(float32 a, float32 b,
- float_status *status)
+float32 QEMU_FLATTEN float32_sub(float32 a, float32 b, float_status *status)
{
FloatParts pa = float32_unpack_canonical(a, status);
FloatParts pb = float32_unpack_canonical(b, status);
@@ -776,8 +771,7 @@ float32 __attribute__((flatten)) float32_sub(float32 a,
float32 b,
return float32_round_pack_canonical(pr, status);
}
-float64 __attribute__((flatten)) float64_sub(float64 a, float64 b,
- float_status *status)
+float64 QEMU_FLATTEN float64_sub(float64 a, float64 b, float_status *status)
{
FloatParts pa = float64_unpack_canonical(a, status);
FloatParts pb = float64_unpack_canonical(b, status);
@@ -835,8 +829,7 @@ static FloatParts mul_floats(FloatParts a, FloatParts b,
float_status *s)
g_assert_not_reached();
}
-float16 __attribute__((flatten)) float16_mul(float16 a, float16 b,
- float_status *status)
+float16 QEMU_FLATTEN float16_mul(float16 a, float16 b, float_status *status)
{
FloatParts pa = float16_unpack_canonical(a, status);
FloatParts pb = float16_unpack_canonical(b, status);
@@ -845,8 +838,7 @@ float16 __attribute__((flatten)) float16_mul(float16 a,
float16 b,
return float16_round_pack_canonical(pr, status);
}
-float32 __attribute__((flatten)) float32_mul(float32 a, float32 b,
- float_status *status)
+float32 QEMU_FLATTEN float32_mul(float32 a, float32 b, float_status *status)
{
FloatParts pa = float32_unpack_canonical(a, status);
FloatParts pb = float32_unpack_canonical(b, status);
@@ -855,8 +847,7 @@ float32 __attribute__((flatten)) float32_mul(float32 a,
float32 b,
return float32_round_pack_canonical(pr, status);
}
-float64 __attribute__((flatten)) float64_mul(float64 a, float64 b,
- float_status *status)
+float64 QEMU_FLATTEN float64_mul(float64 a, float64 b, float_status *status)
{
FloatParts pa = float64_unpack_canonical(a, status);
FloatParts pb = float64_unpack_canonical(b, sta
[Qemu-devel] [PULL 14/15] hw/core/generic-loader: Compile only once, not for each target
The generic-loader is currently compiled target specific due to one
single "#ifdef TARGET_WORDS_BIGENDIAN" in the file. We have already a
function called target_words_bigendian() for this instead, so we can
put the generic-loader into common-obj to save some compilation time.
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Laszlo Ersek
Reviewed-by: Alistair Francis
Signed-off-by: Thomas Huth
---
hw/core/Makefile.objs| 2 +-
hw/core/generic-loader.c | 6 +-
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index eb88ca9..b736ce2 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -20,6 +20,6 @@ common-obj-$(CONFIG_SOFTMMU) += register.o
common-obj-$(CONFIG_SOFTMMU) += or-irq.o
common-obj-$(CONFIG_SOFTMMU) += split-irq.o
common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
+common-obj-$(CONFIG_SOFTMMU) += generic-loader.o
-obj-$(CONFIG_SOFTMMU) += generic-loader.o
obj-$(CONFIG_SOFTMMU) += null-machine.o
diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index be29ae1..fbae05f 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -130,11 +130,7 @@ static void generic_loader_realize(DeviceState *dev, Error
**errp)
s->cpu = first_cpu;
}
-#ifdef TARGET_WORDS_BIGENDIAN
-big_endian = 1;
-#else
-big_endian = 0;
-#endif
+big_endian = target_words_bigendian();
if (s->file) {
AddressSpace *as = s->cpu ? s->cpu->as : NULL;
--
1.8.3.1
[Qemu-devel] [PULL 02/15] target/cris/translate: Get rid of qemu_log_separate()
The gen_BUG() function calls already cpu_abort(), which prints the
information to stderr and the log already. So instead of additionally
printing the dc->pc via fprintf() and qemu_log here, too, we can
simply pass this information to cpu_abort() instead.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Thomas Huth
---
target/cris/translate.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/cris/translate.c b/target/cris/translate.c
index 4ae1c04..11b2c11 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -137,11 +137,7 @@ typedef struct DisasContext {
static void gen_BUG(DisasContext *dc, const char *file, int line)
{
-fprintf(stderr, "BUG: pc=%x %s %d\n", dc->pc, file, line);
-if (qemu_log_separate()) {
-qemu_log("BUG: pc=%x %s %d\n", dc->pc, file, line);
-}
-cpu_abort(CPU(dc->cpu), "%s:%d\n", file, line);
+cpu_abort(CPU(dc->cpu), "%s:%d pc=%x\n", file, line, dc->pc);
}
static const char *regnames_v32[] =
--
1.8.3.1
Re: [Qemu-devel] [PATCH] Acceptance tests: host arch to target arch name mapping
Hi Cleber,
On 17/10/2018 01:22, Cleber Rosa wrote:
> The host arch name is not always the target arch name, so it's
> necessary to have a mapping.
>
> The configure scripts contains what is the authoritative and failproof
> mapping, but, reusing it is not straightforward, so it's replicated in
> the acceptance tests supporting code.
>
> Signed-off-by: Cleber Rosa
> ---
> configure | 2 ++
> tests/acceptance/avocado_qemu/__init__.py | 23 +++
> 2 files changed, 25 insertions(+)
>
> diff --git a/configure b/configure
> index 8af2be959f..e029b756d4 100755
> --- a/configure
> +++ b/configure
> @@ -6992,6 +6992,8 @@ TARGET_ARCH="$target_name"
> TARGET_BASE_ARCH=""
> TARGET_ABI_DIR=""
>
> +# When updating target_name => TARGET_ARCH, please also update the
> +# HOST_TARGET_ARCH mapping in tests/acceptance/avocado_qemu/__init__.py
> case "$target_name" in
>i386)
> mttcg="yes"
> diff --git a/tests/acceptance/avocado_qemu/__init__.py
> b/tests/acceptance/avocado_qemu/__init__.py
> index 1e54fd5932..d9bc4736ec 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
I'd put this in scripts/qemu.py
> @@ -19,6 +19,28 @@ sys.path.append(os.path.join(SRC_ROOT_DIR, 'scripts'))
>
> from qemu import QEMUMachine
>
> +
> +#: Mapping of host arch names to target arch names. It's expected that the
> +#: arch identification on the host, using os.uname()[4], would return the
> +#: key (LHS). The QEMU target name, and consequently the target binary,
> would
> +#: be based on the name on the value (RHS).
> +HOST_TARGET_ARCH = {
> +'armeb': 'arm',
> +'aarch64_be': 'aarch64',
Since you add this, I'd start directly with an exhaustive list:
'aarch64_be': {'arch': 'aarch64', 'endian': 'big', 'wordsize':
64, 'abi' = 'arm'},
> +'microblazeel': 'microblaze',
> +'mipsel': 'mips',
> +'mipsn32el' : 'mips64',
> +'mips64el': 'mips64',
> +'or1k': 'openrisc',
> +'ppc64le': 'ppc64',
> +'ppc64abi32': 'ppc64',
> +'riscv64': 'riscv',
> +'sh4eb': 'sh4',
> +'sparc32plus': 'sparc64',
> +'xtensaeb': 'xtensa'
> +}
Then a function such:
def target_normalize(key, arch):
return table[arch][key]
> +
> +
> def is_readable_executable_file(path):
> return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>
> @@ -29,6 +51,7 @@ def pick_default_qemu_bin():
> directory or in the source tree root directory.
> """
> arch = os.uname()[4]
> +arch = HOST_TARGET_ARCH.get(arch, arch)
arch = target_normalize('arch', os.uname()[4])
> qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
>"qemu-system-%s" % arch)
> if is_readable_executable_file(qemu_bin_relative_path):
>
What do you think?
Thanks,
Phil.
[Qemu-devel] [PULL 01/15] qemu-common.h: update copyright date to 2018
From: John Arbuckle
Currently the copyright date is set to 2017. Update the date to say
2018.
Signed-off-by: John Arbuckle
Reviewed-by: Stefan Weil
Signed-off-by: Thomas Huth
---
include/qemu-common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 85f4749..ed60ba2 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -17,7 +17,7 @@
#define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
/* Copyright string for -version arguments, About dialogs, etc */
-#define QEMU_COPYRIGHT "Copyright (c) 2003-2017 " \
+#define QEMU_COPYRIGHT "Copyright (c) 2003-2018 " \
"Fabrice Bellard and the QEMU Project developers"
/* Bug reporting information for --help arguments, About dialogs, etc */
--
1.8.3.1
[Qemu-devel] [PULL 07/15] archive-source.sh: Modern shell scripting (use $() instead of ``)
From: Mao Zhongyi
Various shell files contain a mix between obsolete ``
and modern $(); It would be nice to convert to using $()
everywhere.
Signed-off-by: Mao Zhongyi
Reviewed-by: Thomas Huth
Signed-off-by: Thomas Huth
---
scripts/archive-source.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 4e63774..62bd225 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -18,7 +18,7 @@ if test $# -lt 1; then
error "Usage: $0 "
fi
-tar_file=`realpath "$1"`
+tar_file=$(realpath "$1")
list_file="${tar_file}.list"
vroot_dir="${tar_file}.vroot"
@@ -34,7 +34,7 @@ if git diff-index --quiet HEAD -- &>/dev/null
then
HEAD=HEAD
else
-HEAD=`git stash create`
+HEAD=$(git stash create)
fi
git clone --shared . "$vroot_dir"
test $? -ne 0 && error "failed to clone into '$vroot_dir'"
--
1.8.3.1
[Qemu-devel] [PULL 05/15] gdbstub: Remove unused include
From: Philippe Mathieu-Daudé Signed-off-by: Philippe Mathieu-Daudé Tested-by: Thomas Huth Signed-off-by: Thomas Huth --- gdbstub.c | 1 - 1 file changed, 1 deletion(-) diff --git a/gdbstub.c b/gdbstub.c index c8478de..c4e4f9f 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -20,7 +20,6 @@ #include "qapi/error.h" #include "qemu/error-report.h" #include "qemu/cutils.h" -#include "cpu.h" #include "trace-root.h" #ifdef CONFIG_USER_ONLY #include "qemu.h" -- 1.8.3.1
Re: [Qemu-devel] [PATCH] Revert "icount: remove obsolete warp call"
On 17/10/2018 11:53, Artem Pisarenko wrote: > See my last comment in bug report. This kind of modification, even > adapted to changed function name, doesn't solve issue. > I thought long time that it does, but once I catched qemu with a hang. > And of course, I wasn't able to reproduce it. So it just better hides issue. > Take a look at alternative solution from > QBox: > https://git.greensocs.com/qemu/qbox/commit/a8ed106032e375e715a531d6e93e4d9ec295dbdb > I didn't catched fail with it (yet). Makes sense (and the patch I submitted was stupid). Clement, can you submit the patch from qbox with Signed-off-by? Thanks, Paolo
