* [PULL 01/23] KVM: x86: do not fail if software breakpoint has already been removed
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
@ 2021-03-06 10:53 ` Paolo Bonzini
2021-03-06 10:53 ` [PULL 02/23] KVM: x86: deprecate -M kernel-irqchip=off except for -M isapc Paolo Bonzini
` (23 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Maxim Levitsky
If kvm_arch_remove_sw_breakpoint finds that a software breakpoint does not
have an INT3 instruction, it fails. This can happen if one sets a
software breakpoint in a kernel module and then reloads it. gdb then
thinks the breakpoint cannot be deleted and there is no way to add it
back.
Suggested-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/kvm/kvm.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 0b5755e42b..c8d61daf68 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -4352,8 +4352,13 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
{
uint8_t int3;
- if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0) || int3 != 0xcc ||
- cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) {
+ if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0)) {
+ return -EINVAL;
+ }
+ if (int3 != 0xcc) {
+ return 0;
+ }
+ if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) {
return -EINVAL;
}
return 0;
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 02/23] KVM: x86: deprecate -M kernel-irqchip=off except for -M isapc
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
2021-03-06 10:53 ` [PULL 01/23] KVM: x86: do not fail if software breakpoint has already been removed Paolo Bonzini
@ 2021-03-06 10:53 ` Paolo Bonzini
2021-03-06 10:53 ` [PULL 03/23] qga-vss: Use dynamic linking for GLib Paolo Bonzini
` (22 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Maxim Levitsky
The userspace local APIC is basically untested and does not support many
features such as TSC deadline timer, x2APIC or PV spinlocks. On the
other hand, the PIT and IOAPIC are okay as they are not tied to
the processor and are tested with -M kernel-irqchip=split.
Therefore, deprecate the local APIC and, with it, limit
-M kernel-irqchip=off to the ISA PC machine type, which does not
have a local APIC at all.
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/system/deprecated.rst | 7 +++++++
hw/intc/apic.c | 6 ++++++
2 files changed, 13 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 561c916da2..fcf0ca4068 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -153,6 +153,13 @@ The ``-writeconfig`` option is not able to serialize the entire contents
of the QEMU command line. It is thus considered a failed experiment
and deprecated, with no current replacement.
+Userspace local APIC with KVM (x86, since 6.0)
+''''''''''''''''''''''''''''''''''''''''''''''
+
+Using ``-M kernel-irqchip=off`` with x86 machine types that include a local
+APIC is deprecated. The ``split`` setting is supported, as is using
+``-M kernel-irqchip=off`` with the ISA PC machine type.
+
QEMU Machine Protocol (QMP) commands
------------------------------------
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 3ada22f427..f4f50f974e 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -25,6 +25,7 @@
#include "hw/intc/i8259.h"
#include "hw/pci/msi.h"
#include "qemu/host-utils.h"
+#include "sysemu/kvm.h"
#include "trace.h"
#include "hw/i386/apic-msidef.h"
#include "qapi/error.h"
@@ -875,6 +876,11 @@ static void apic_realize(DeviceState *dev, Error **errp)
return;
}
+ if (kvm_enabled()) {
+ warn_report("Userspace local APIC is deprecated for KVM.");
+ warn_report("Do not use kernel-irqchip except for the -M isapc machine type.");
+ }
+
memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, "apic-msi",
APIC_SPACE_SIZE);
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 03/23] qga-vss: Use dynamic linking for GLib
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
2021-03-06 10:53 ` [PULL 01/23] KVM: x86: do not fail if software breakpoint has already been removed Paolo Bonzini
2021-03-06 10:53 ` [PULL 02/23] KVM: x86: deprecate -M kernel-irqchip=off except for -M isapc Paolo Bonzini
@ 2021-03-06 10:53 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 04/23] chardev: add nodelay option Paolo Bonzini
` (21 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Kostiantyn Kostiuk, Marc-André Lureau
From: Kostiantyn Kostiuk <konstantin@daynix.com>
The current GLib version implements the DllMain function. DllMain is also
present in the provider.cpp code. So in the case of static linking, the
DllMain redefinition error occurs. For now, just switch to dynamic linking
and revert this patch when the issue will be solved.
See Glib issue for more details https://gitlab.gnome.org/GNOME/glib/-/issues/692
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qga/vss-win32/meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build
index 780c461432..90825edef3 100644
--- a/qga/vss-win32/meson.build
+++ b/qga/vss-win32/meson.build
@@ -1,5 +1,5 @@
if add_languages('cpp', required: false)
- glib_static = dependency('glib-2.0', static: true)
+ glib_dynamic = dependency('glib-2.0', static: false)
link_args = cc.get_supported_link_arguments(['-fstack-protector-all', '-fstack-protector-strong',
'-Wl,--add-stdcall-alias', '-Wl,--enable-stdcall-fixup'])
@@ -8,7 +8,7 @@ if add_languages('cpp', required: false)
cpp_args: ['-Wno-unknown-pragmas', '-Wno-delete-non-virtual-dtor', '-Wno-non-virtual-dtor'],
link_args: link_args,
vs_module_defs: 'qga-vss.def',
- dependencies: [glib_static, socket,
+ dependencies: [glib_dynamic, socket,
cc.find_library('ole32'),
cc.find_library('oleaut32'),
cc.find_library('shlwapi'),
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 04/23] chardev: add nodelay option
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (2 preceding siblings ...)
2021-03-06 10:53 ` [PULL 03/23] qga-vss: Use dynamic linking for GLib Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 05/23] qom: Check for wellformed id in user_creatable_add_type() Paolo Bonzini
` (20 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel
The "delay" option was introduced as a way to enable Nagle's algorithm
with ",nodelay". Since the short form for boolean options has now been
deprecated, introduce a more properly named "nodelay" option. The "delay"
option remains as an undocumented option.
"delay" and "nodelay" are mutually exclusive. Because the check is
done at consumption time, the code also rejects them if one of the
two is specified via -set.
Based-on: <20210226080526.651705-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
chardev/char-socket.c | 13 +++++++++++--
chardev/char.c | 3 +++
gdbstub.c | 2 +-
qemu-options.hx | 14 +++++++-------
4 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 06a37c0cc8..c8bced76b7 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1472,8 +1472,17 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
sock = backend->u.socket.data = g_new0(ChardevSocket, 1);
qemu_chr_parse_common(opts, qapi_ChardevSocket_base(sock));
- sock->has_nodelay = qemu_opt_get(opts, "delay");
- sock->nodelay = !qemu_opt_get_bool(opts, "delay", true);
+ if (qemu_opt_get(opts, "delay") && qemu_opt_get(opts, "nodelay")) {
+ error_setg(errp, "'delay' and 'nodelay' are mutually exclusive");
+ return;
+ }
+ sock->has_nodelay =
+ qemu_opt_get(opts, "delay") ||
+ qemu_opt_get(opts, "nodelay");
+ sock->nodelay =
+ !qemu_opt_get_bool(opts, "delay", true) ||
+ qemu_opt_get_bool(opts, "nodelay", false);
+
/*
* We have different default to QMP for 'server', hence
* we can't just check for existence of 'server'
diff --git a/chardev/char.c b/chardev/char.c
index 288efebd12..97cafd6849 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -867,6 +867,9 @@ QemuOptsList qemu_chardev_opts = {
},{
.name = "delay",
.type = QEMU_OPT_BOOL,
+ },{
+ .name = "nodelay",
+ .type = QEMU_OPT_BOOL,
},{
.name = "reconnect",
.type = QEMU_OPT_NUMBER,
diff --git a/gdbstub.c b/gdbstub.c
index 3ee40479b6..16d7c8f534 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3505,7 +3505,7 @@ int gdbserver_start(const char *device)
if (strstart(device, "tcp:", NULL)) {
/* enforce required TCP attributes */
snprintf(gdbstub_device_name, sizeof(gdbstub_device_name),
- "%s,wait=off,delay=off,server=on", device);
+ "%s,wait=off,nodelay=on,server=on", device);
device = gdbstub_device_name;
}
#ifndef _WIN32
diff --git a/qemu-options.hx b/qemu-options.hx
index 252db9357c..90801286c6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3033,7 +3033,7 @@ DEFHEADING(Character device options:)
DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
"-chardev help\n"
"-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
- "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,delay=on|off][,reconnect=seconds]\n"
+ "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off][,reconnect=seconds]\n"
" [,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds][,mux=on|off]\n"
" [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n"
"-chardev socket,id=id,path=path[,server=on|off][,wait=on|off][,telnet=on|off][,websocket=on|off][,reconnect=seconds]\n"
@@ -3184,7 +3184,7 @@ The available backends are:
TCP and unix socket options are given below:
- ``TCP options: port=port[,host=host][,to=to][,ipv4=on|off][,ipv6=on|off][,delay=on|off]``
+ ``TCP options: port=port[,host=host][,to=to][,ipv4=on|off][,ipv6=on|off][,nodelay=on|off]``
``host`` for a listening socket specifies the local address to
be bound. For a connecting socket species the remote host to
connect to. ``host`` is optional for listening sockets. If not
@@ -3204,7 +3204,7 @@ The available backends are:
or IPv6 must be used. If neither is specified the socket may
use either protocol.
- ``delay=on|off`` disables the Nagle algorithm.
+ ``nodelay=on|off`` disables the Nagle algorithm.
``unix options: path=path[,abstract=on|off][,tight=on|off]``
``path`` specifies the local path of the unix socket. ``path``
@@ -3593,13 +3593,13 @@ SRST
``telnet options:``
localhost 5555
- ``tcp:[host]:port[,server=on|off][,wait=on|off][,delay=on|off][,reconnect=seconds]``
+ ``tcp:[host]:port[,server=on|off][,wait=on|off][,nodelay=on|off][,reconnect=seconds]``
The TCP Net Console has two modes of operation. It can send the
serial I/O to a location or wait for a connection from a
location. By default the TCP Net Console is sent to host at the
port. If you use the ``server=on`` option QEMU will wait for a client
socket application to connect to the port before continuing,
- unless the ``wait=on|off`` option was specified. The ``delay=on|off``
+ unless the ``wait=on|off`` option was specified. The ``nodelay=on|off``
option disables the Nagle buffering algorithm. The ``reconnect=on``
option only applies if ``server=no`` is set, if the connection goes
down it will attempt to reconnect at the given interval. If host
@@ -3616,7 +3616,7 @@ SRST
``Example to not wait and listen on ip 192.168.0.100 port 4444``
-serial tcp:192.168.0.100:4444,server=on,wait=off
- ``telnet:host:port[,server=on|off][,wait=on|off][,delay=on|off]``
+ ``telnet:host:port[,server=on|off][,wait=on|off][,nodelay=on|off]``
The telnet protocol is used instead of raw tcp sockets. The
options work the same as if you had specified ``-serial tcp``.
The difference is that the port acts like a telnet server or
@@ -3626,7 +3626,7 @@ SRST
you do it with Control-] and then type "send break" followed by
pressing the enter key.
- ``websocket:host:port,server=on[,wait=on|off][,delay=on|off]``
+ ``websocket:host:port,server=on[,wait=on|off][,nodelay=on|off]``
The WebSocket protocol is used instead of raw tcp socket. The
port acts as a WebSocket server. Client mode is not supported.
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 05/23] qom: Check for wellformed id in user_creatable_add_type()
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (3 preceding siblings ...)
2021-03-06 10:54 ` [PULL 04/23] chardev: add nodelay option Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 06/23] vl.c: do not execute trace_init_backends() before daemonizing Paolo Bonzini
` (19 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf
From: Kevin Wolf <kwolf@redhat.com>
Most code paths for creating a user creatable object go through
QemuOpts, which ensures that the provided 'id' option is actually a
valid identifier.
However, there are some code paths that don't go through QemuOpts:
qemu-storage-daemon --object (since commit 8db1efd3) and QMP object-add
(since it was first introduced in commit cff8b2c6). We need to have the
same validity check for those, too.
This adds the check and makes it print the same error message as
QemuOpts on failure.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210302171623.49709-1-kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qom/object_interfaces.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 1e9ad6f08a..7661270b98 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -8,6 +8,7 @@
#include "qapi/qobject-input-visitor.h"
#include "qom/object_interfaces.h"
#include "qemu/help_option.h"
+#include "qemu/id.h"
#include "qemu/module.h"
#include "qemu/option.h"
#include "qapi/opts-visitor.h"
@@ -41,11 +42,19 @@ Object *user_creatable_add_type(const char *type, const char *id,
const QDict *qdict,
Visitor *v, Error **errp)
{
+ ERRP_GUARD();
Object *obj;
ObjectClass *klass;
const QDictEntry *e;
Error *local_err = NULL;
+ if (id != NULL && !id_wellformed(id)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
+ error_append_hint(errp, "Identifiers consist of letters, digits, "
+ "'-', '.', '_', starting with a letter.\n");
+ return NULL;
+ }
+
klass = object_class_by_name(type);
if (!klass) {
error_setg(errp, "invalid object type: %s", type);
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 06/23] vl.c: do not execute trace_init_backends() before daemonizing
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (4 preceding siblings ...)
2021-03-06 10:54 ` [PULL 05/23] qom: Check for wellformed id in user_creatable_add_type() Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 07/23] accel: kvm: Fix memory waste under mismatch page size Paolo Bonzini
` (18 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel Henrique Barboza, Stefan Hajnoczi
From: Daniel Henrique Barboza <danielhb413@gmail.com>
Commit v5.2.0-190-g0546c0609c ("vl: split various early command line
options to a separate function") moved the trace backend init code to
the qemu_process_early_options(). Which is now being called before
os_daemonize() via qemu_maybe_daemonize().
Turns out that this change of order causes a problem when executing
QEMU in daemon mode and with CONFIG_TRACE_SIMPLE. The trace thread
is now being created by the parent, and the parent is left waiting for
a trace file flush that was registered via st_init(). The result is
that the parent process never exits.
To reproduce, fire up a QEMU process with -daemonize and with
CONFIG_TRACE_SIMPLE enabled. Two QEMU process will be left in the
host:
$ sudo ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults \
-nographic -machine none,accel=kvm:tcg -daemonize
$ ps axf | grep qemu
529710 pts/3 S+ 0:00 | \_ grep --color=auto qemu
529697 ? Ssl 0:00 \_ ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm:tcg -daemonize
529699 ? Sl 0:00 \_ ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm:tcg -daemonize
The parent thread is hang in flush_trace_file:
$ sudo gdb ./x86_64-softmmu/qemu-system-x86_64 529697
(..)
(gdb) bt
#0 0x00007f9dac6a137d in syscall () at /lib64/libc.so.6
#1 0x00007f9dacc3c4f3 in g_cond_wait () at /lib64/libglib-2.0.so.0
#2 0x0000555d12f952da in flush_trace_file (wait=true) at ../trace/simple.c:140
#3 0x0000555d12f95b4c in st_flush_trace_buffer () at ../trace/simple.c:383
#4 0x00007f9dac5e43a7 in __run_exit_handlers () at /lib64/libc.so.6
#5 0x00007f9dac5e4550 in on_exit () at /lib64/libc.so.6
#6 0x0000555d12d454de in os_daemonize () at ../os-posix.c:255
#7 0x0000555d12d0bd5c in qemu_maybe_daemonize (pid_file=0x0) at ../softmmu/vl.c:2408
#8 0x0000555d12d0e566 in qemu_init (argc=8, argv=0x7fffc594d9b8, envp=0x7fffc594da00) at ../softmmu/vl.c:3459
#9 0x0000555d128edac1 in main (argc=8, argv=0x7fffc594d9b8, envp=0x7fffc594da00) at ../softmmu/main.c:49
(gdb)
Aside from the 'zombie' process in the host, this is directly impacting
Libvirt. Libvirt waits for the parent process to exit to be sure that the
QMP monitor is available in the daemonized process to fetch QEMU
capabilities, and as is now Libvirt hangs at daemon start waiting
for the parent thread to exit.
The fix is simple: just move the trace backend related code back to
be executed after daemonizing.
Fixes: 0546c0609cb5a8d90c1cbac8e0d64b5a048bbb19
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20210105181437.538366-2-danielhb413@gmail.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
softmmu/vl.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 10bd8a10a3..7e8110bd6e 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2361,11 +2361,6 @@ static void qemu_process_early_options(void)
cleanup_add_fd, NULL, &error_fatal);
#endif
- if (!trace_init_backends()) {
- exit(1);
- }
- trace_init_file();
-
/* Open the logfile at this point and set the log mask if necessary. */
qemu_set_log_filename(log_file, &error_fatal);
if (log_mask) {
@@ -3475,6 +3470,19 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_process_help_options();
qemu_maybe_daemonize(pid_file);
+ /*
+ * The trace backend must be initialized after daemonizing.
+ * trace_init_backends() will call st_init(), which will create the
+ * trace thread in the parent, and also register st_flush_trace_buffer()
+ * in atexit(). This function will force the parent to wait for the
+ * writeout thread to finish, which will not occur, and the parent
+ * process will be left in the host.
+ */
+ if (!trace_init_backends()) {
+ exit(1);
+ }
+ trace_init_file();
+
qemu_init_main_loop(&error_fatal);
cpu_timers_init();
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 07/23] accel: kvm: Fix memory waste under mismatch page size
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (5 preceding siblings ...)
2021-03-06 10:54 ` [PULL 06/23] vl.c: do not execute trace_init_backends() before daemonizing Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 08/23] accel: kvm: Add aligment assert for kvm_log_clear_one_slot Paolo Bonzini
` (17 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Andrew Jones, Keqian Zhu, Peter Xu
From: Keqian Zhu <zhukeqian1@huawei.com>
When handle dirty log, we face qemu_real_host_page_size and
TARGET_PAGE_SIZE. The first one is the granule of KVM dirty
bitmap, and the second one is the granule of QEMU dirty bitmap.
As qemu_real_host_page_size >= TARGET_PAGE_SIZE (kvm_init()
enforced it), misuse TARGET_PAGE_SIZE to init kvmslot dirty_bmap
may waste memory. For example, when qemu_real_host_page_size is
64K and TARGET_PAGE_SIZE is 4K, it wastes 93.75% (15/16) memory.
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20201217014941.22872-2-zhukeqian1@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/kvm/kvm-all.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 84c943fcdb..ffce83f1a7 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -598,8 +598,12 @@ static void kvm_memslot_init_dirty_bitmap(KVMSlot *mem)
* too, in most cases).
* So for now, let's align to 64 instead of HOST_LONG_BITS here, in
* a hope that sizeof(long) won't become >8 any time soon.
+ *
+ * Note: the granule of kvm dirty log is qemu_real_host_page_size.
+ * And mem->memory_size is aligned to it (otherwise this mem can't
+ * be registered to KVM).
*/
- hwaddr bitmap_size = ALIGN(((mem->memory_size) >> TARGET_PAGE_BITS),
+ hwaddr bitmap_size = ALIGN(mem->memory_size / qemu_real_host_page_size,
/*HOST_LONG_BITS*/ 64) / 8;
mem->dirty_bmap = g_malloc0(bitmap_size);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 08/23] accel: kvm: Add aligment assert for kvm_log_clear_one_slot
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (6 preceding siblings ...)
2021-03-06 10:54 ` [PULL 07/23] accel: kvm: Fix memory waste under mismatch page size Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 09/23] lsilogic: Use PCIDevice::exit instead of DeviceState::unrealize Paolo Bonzini
` (16 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Andrew Jones, Keqian Zhu, Peter Xu
From: Keqian Zhu <zhukeqian1@huawei.com>
The parameters start and size are transfered from QEMU memory
emulation layer. It can promise that they are TARGET_PAGE_SIZE
aligned. However, KVM needs they are qemu_real_page_size aligned.
Though no caller breaks this aligned requirement currently, we'd
better add an explicit assert to avoid future breaking.
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Acked-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-Id: <20201217014941.22872-3-zhukeqian1@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/kvm/kvm-all.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index ffce83f1a7..f88a52393f 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -673,6 +673,10 @@ out:
#define KVM_CLEAR_LOG_ALIGN (qemu_real_host_page_size << KVM_CLEAR_LOG_SHIFT)
#define KVM_CLEAR_LOG_MASK (-KVM_CLEAR_LOG_ALIGN)
+/*
+ * As the granule of kvm dirty log is qemu_real_host_page_size,
+ * @start and @size are expected and restricted to align to it.
+ */
static int kvm_log_clear_one_slot(KVMSlot *mem, int as_id, uint64_t start,
uint64_t size)
{
@@ -682,6 +686,9 @@ static int kvm_log_clear_one_slot(KVMSlot *mem, int as_id, uint64_t start,
unsigned long *bmap_clear = NULL, psize = qemu_real_host_page_size;
int ret;
+ /* Make sure start and size are qemu_real_host_page_size aligned */
+ assert(QEMU_IS_ALIGNED(start | size, psize));
+
/*
* We need to extend either the start or the size or both to
* satisfy the KVM interface requirement. Firstly, do the start
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 09/23] lsilogic: Use PCIDevice::exit instead of DeviceState::unrealize
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (7 preceding siblings ...)
2021-03-06 10:54 ` [PULL 08/23] accel: kvm: Add aligment assert for kvm_log_clear_one_slot Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 10/23] elf_ops: correct loading of 32 bit PVH kernel Paolo Bonzini
` (15 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Peng Liang, qemu-stable
From: Peng Liang <liangpeng10@huawei.com>
PCI_DEVICE has overwritten DeviceState::unrealize (pci_qdev_unrealize).
However, LSI53C895A, which is a subclass of PCI_DEVICE, overwrites it
again and doesn't save the parent's implementation so the PCI_DEVICE's
implementation of DeviceState::unrealize will never be called when
unrealize a LSI53C895A device. And it will lead to memory leak and
unplug failure.
For a PCI device, it's better to implement PCIDevice::exit instead of
DeviceState::unrealize. So let's change to use PCIDevice::exit.
Fixes: a8632434c7e9 ("lsi: implement I/O memory space for Memory Move instructions")
Cc: qemu-stable@nongnu.org
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Message-Id: <20210302133016.1221081-1-liangpeng10@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/lsi53c895a.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index a4e58580e4..e2c19180a0 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2312,7 +2312,7 @@ static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
scsi_bus_new(&s->bus, sizeof(s->bus), d, &lsi_scsi_info, NULL);
}
-static void lsi_scsi_unrealize(DeviceState *dev)
+static void lsi_scsi_exit(PCIDevice *dev)
{
LSIState *s = LSI53C895A(dev);
@@ -2325,11 +2325,11 @@ static void lsi_class_init(ObjectClass *klass, void *data)
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
k->realize = lsi_scsi_realize;
+ k->exit = lsi_scsi_exit;
k->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
k->device_id = PCI_DEVICE_ID_LSI_53C895A;
k->class_id = PCI_CLASS_STORAGE_SCSI;
k->subsystem_id = 0x1000;
- dc->unrealize = lsi_scsi_unrealize;
dc->reset = lsi_scsi_reset;
dc->vmsd = &vmstate_lsi_scsi;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 10/23] elf_ops: correct loading of 32 bit PVH kernel
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (8 preceding siblings ...)
2021-03-06 10:54 ` [PULL 09/23] lsilogic: Use PCIDevice::exit instead of DeviceState::unrealize Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 11/23] x86/pvh: extract only 4 bytes of start address for 32 bit kernels Paolo Bonzini
` (14 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel
Cc: David Edmondson, Philippe Mathieu-Daudé, Stefano Garzarella
From: David Edmondson <david.edmondson@oracle.com>
Because sizeof(struct elf64_note) == sizeof(struct elf32_note),
attempting to use the size of the currently defined struct elf_note as
a discriminator for whether the object being loaded is 64 bit in
load_elf() fails.
Instead, take advantage of the existing glue parameter SZ, which is
defined as 32 or 64 in the respective variants of load_elf().
Fixes: 696aa04c84c6 ("elf-ops.h: Add get_elf_note_type()")
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210302090315.3031492-2-david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/elf_ops.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 8e8436831d..78409ab34a 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -598,9 +598,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
nhdr = glue(get_elf_note_type, SZ)(nhdr, file_size, ph->p_align,
*(uint64_t *)translate_opaque);
if (nhdr != NULL) {
- bool is64 =
- sizeof(struct elf_note) == sizeof(struct elf64_note);
- elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64);
+ elf_note_fn((void *)nhdr, (void *)&ph->p_align, SZ == 64);
}
data = NULL;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 11/23] x86/pvh: extract only 4 bytes of start address for 32 bit kernels
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (9 preceding siblings ...)
2021-03-06 10:54 ` [PULL 10/23] elf_ops: correct loading of 32 bit PVH kernel Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 12/23] qemu-config: add error propagation to qemu_config_parse Paolo Bonzini
` (13 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: David Edmondson, Stefano Garzarella
From: David Edmondson <david.edmondson@oracle.com>
When loading the PVH start address from a 32 bit ELF note, extract
only the appropriate number of bytes.
Fixes: ab969087da65 ("pvh: Boot uncompressed kernel using direct boot ABI")
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210302090315.3031492-3-david.edmondson@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/i386/x86.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6329f90ef9..7865660e2c 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -690,6 +690,8 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64)
elf_note_data_addr =
((void *)nhdr64) + nhdr_size64 +
QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
+
+ pvh_start_addr = *elf_note_data_addr;
} else {
struct elf32_note *nhdr32 = (struct elf32_note *)arg1;
uint32_t nhdr_size32 = sizeof(struct elf32_note);
@@ -699,9 +701,9 @@ static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64)
elf_note_data_addr =
((void *)nhdr32) + nhdr_size32 +
QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
- }
- pvh_start_addr = *elf_note_data_addr;
+ pvh_start_addr = *(uint32_t *)elf_note_data_addr;
+ }
return pvh_start_addr;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 12/23] qemu-config: add error propagation to qemu_config_parse
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (10 preceding siblings ...)
2021-03-06 10:54 ` [PULL 11/23] x86/pvh: extract only 4 bytes of start address for 32 bit kernels Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-06-29 12:45 ` Peter Maydell
2021-03-06 10:54 ` [PULL 13/23] scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error codes Paolo Bonzini
` (12 subsequent siblings)
24 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Markus Armbruster
This enables some simplification of vl.c via error_fatal, and improves
error messages. Before:
$ ./qemu-system-x86_64 -readconfig .
qemu-system-x86_64: error reading file
qemu-system-x86_64: -readconfig .: read config .: Invalid argument
$ /usr/libexec/qemu-kvm -readconfig foo
qemu-kvm: -readconfig foo: read config foo: No such file or directory
After:
$ ./qemu-system-x86_64 -readconfig .
qemu-system-x86_64: -readconfig .: Cannot read config file: Is a directory
$ ./qemu-system-x86_64 -readconfig foo
qemu-system-x86_64: -readconfig foo: Could not open 'foo': No such file or directory
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210226170816.231173-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/blkdebug.c | 3 +--
include/qemu/config-file.h | 5 +++--
softmmu/vl.c | 29 +++++++++++------------------
util/qemu-config.c | 23 ++++++++++++-----------
4 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 5fe6172da9..7eaa8a28bf 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -279,9 +279,8 @@ static int read_config(BDRVBlkdebugState *s, const char *filename,
return -errno;
}
- ret = qemu_config_parse(f, config_groups, filename);
+ ret = qemu_config_parse(f, config_groups, filename, errp);
if (ret < 0) {
- error_setg(errp, "Could not parse blkdebug config file");
goto fail;
}
}
diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
index 29226107bd..8d3e53ae4d 100644
--- a/include/qemu/config-file.h
+++ b/include/qemu/config-file.h
@@ -11,9 +11,10 @@ void qemu_add_drive_opts(QemuOptsList *list);
int qemu_global_option(const char *str);
void qemu_config_write(FILE *fp);
-int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname,
+ Error **errp);
-int qemu_read_config_file(const char *filename);
+int qemu_read_config_file(const char *filename, Error **errp);
/* Parse QDict options as a replacement for a config file (allowing multiple
enumerated (0..(n-1)) configuration "sections") */
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 7e8110bd6e..2f4958db2a 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2062,17 +2062,19 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
return 0;
}
-static int qemu_read_default_config_file(void)
+static void qemu_read_default_config_file(Error **errp)
{
+ ERRP_GUARD();
int ret;
g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf");
- ret = qemu_read_config_file(file);
- if (ret < 0 && ret != -ENOENT) {
- return ret;
+ ret = qemu_read_config_file(file, errp);
+ if (ret < 0) {
+ if (ret == -ENOENT) {
+ error_free(*errp);
+ *errp = NULL;
+ }
}
-
- return 0;
}
static int qemu_set_option(const char *str)
@@ -2633,9 +2635,7 @@ void qemu_init(int argc, char **argv, char **envp)
}
if (userconfig) {
- if (qemu_read_default_config_file() < 0) {
- exit(1);
- }
+ qemu_read_default_config_file(&error_fatal);
}
/* second pass of option parsing */
@@ -3323,15 +3323,8 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_plugin_opt_parse(optarg, &plugin_list);
break;
case QEMU_OPTION_readconfig:
- {
- int ret = qemu_read_config_file(optarg);
- if (ret < 0) {
- error_report("read config %s: %s", optarg,
- strerror(-ret));
- exit(1);
- }
- break;
- }
+ qemu_read_config_file(optarg, &error_fatal);
+ break;
case QEMU_OPTION_spice:
olist = qemu_find_opts_err("spice", NULL);
if (!olist) {
diff --git a/util/qemu-config.c b/util/qemu-config.c
index e2a700b284..670bd6ebca 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -350,7 +350,7 @@ void qemu_config_write(FILE *fp)
}
/* Returns number of config groups on success, -errno on error */
-int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
{
char line[1024], group[64], id[64], arg[64], value[1024];
Location loc;
@@ -375,7 +375,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
/* group with id */
list = find_list(lists, group, &local_err);
if (local_err) {
- error_report_err(local_err);
+ error_propagate(errp, local_err);
goto out;
}
opts = qemu_opts_create(list, id, 1, NULL);
@@ -386,7 +386,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
/* group without id */
list = find_list(lists, group, &local_err);
if (local_err) {
- error_report_err(local_err);
+ error_propagate(errp, local_err);
goto out;
}
opts = qemu_opts_create(list, NULL, 0, &error_abort);
@@ -398,21 +398,21 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
sscanf(line, " %63s = \"\"", arg) == 1) {
/* arg = value */
if (opts == NULL) {
- error_report("no group defined");
+ error_setg(errp, "no group defined");
goto out;
}
- if (!qemu_opt_set(opts, arg, value, &local_err)) {
- error_report_err(local_err);
+ if (!qemu_opt_set(opts, arg, value, errp)) {
goto out;
}
continue;
}
- error_report("parse error");
+ error_setg(errp, "parse error");
goto out;
}
if (ferror(fp)) {
- error_report("error reading file");
- goto out;
+ loc_pop(&loc);
+ error_setg_errno(errp, errno, "Cannot read config file");
+ return res;
}
res = count;
out:
@@ -420,16 +420,17 @@ out:
return res;
}
-int qemu_read_config_file(const char *filename)
+int qemu_read_config_file(const char *filename, Error **errp)
{
FILE *f = fopen(filename, "r");
int ret;
if (f == NULL) {
+ error_setg_file_open(errp, errno, filename);
return -errno;
}
- ret = qemu_config_parse(f, vm_config_groups, filename);
+ ret = qemu_config_parse(f, vm_config_groups, filename, errp);
fclose(f);
return ret;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PULL 12/23] qemu-config: add error propagation to qemu_config_parse
2021-03-06 10:54 ` [PULL 12/23] qemu-config: add error propagation to qemu_config_parse Paolo Bonzini
@ 2021-06-29 12:45 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2021-06-29 12:45 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Philippe Mathieu-Daudé, QEMU Developers, Markus Armbruster
On Sat, 6 Mar 2021 at 10:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> This enables some simplification of vl.c via error_fatal, and improves
> error messages. Before:
>
> $ ./qemu-system-x86_64 -readconfig .
> qemu-system-x86_64: error reading file
> qemu-system-x86_64: -readconfig .: read config .: Invalid argument
> $ /usr/libexec/qemu-kvm -readconfig foo
> qemu-kvm: -readconfig foo: read config foo: No such file or directory
>
> After:
>
> $ ./qemu-system-x86_64 -readconfig .
> qemu-system-x86_64: -readconfig .: Cannot read config file: Is a directory
> $ ./qemu-system-x86_64 -readconfig foo
> qemu-system-x86_64: -readconfig foo: Could not open 'foo': No such file or directory
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Message-Id: <20210226170816.231173-1-pbonzini@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Hi; Coverity complains (CID 1457456) about a resource leak here:
> /* Returns number of config groups on success, -errno on error */
> -int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
> +int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
> {
> char line[1024], group[64], id[64], arg[64], value[1024];
> Location loc;
> @@ -375,7 +375,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
> /* group with id */
> list = find_list(lists, group, &local_err);
> if (local_err) {
> - error_report_err(local_err);
> + error_propagate(errp, local_err);
> goto out;
> }
> opts = qemu_opts_create(list, id, 1, NULL);
> @@ -386,7 +386,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
> /* group without id */
> list = find_list(lists, group, &local_err);
> if (local_err) {
> - error_report_err(local_err);
> + error_propagate(errp, local_err);
> goto out;
> }
> opts = qemu_opts_create(list, NULL, 0, &error_abort);
> @@ -398,21 +398,21 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
> sscanf(line, " %63s = \"\"", arg) == 1) {
> /* arg = value */
> if (opts == NULL) {
> - error_report("no group defined");
> + error_setg(errp, "no group defined");
> goto out;
> }
> - if (!qemu_opt_set(opts, arg, value, &local_err)) {
> - error_report_err(local_err);
> + if (!qemu_opt_set(opts, arg, value, errp)) {
> goto out;
> }
> continue;
> }
> - error_report("parse error");
> + error_setg(errp, "parse error");
> goto out;
> }
> if (ferror(fp)) {
> - error_report("error reading file");
> - goto out;
> + loc_pop(&loc);
> + error_setg_errno(errp, errno, "Cannot read config file");
Here we no longer 'goto out' and therefore we don't qobject_unref(qdict)
in this error path.
> + return res;
> }
> res = count;
> out:
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PULL 13/23] scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error codes
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (11 preceding siblings ...)
2021-03-06 10:54 ` [PULL 12/23] qemu-config: add error propagation to qemu_config_parse Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 14/23] scsi: Add mapping for generic SCSI_HOST status to sense codes Paolo Bonzini
` (11 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Hannes Reinecke
From: Hannes Reinecke <hare@suse.de>
We really should make a distinction between legitimate sense codes
(ie if one is running against an emulated block device or for
pass-through sense codes), and the intermediate errors generated
during processing of the command, which really are not sense codes
but refer to some specific internal status. And this internal
state is not necessarily linux-specific, but rather can refer to
the qemu implementation itself.
So rename the linux-only SG_ERR codes to SCSI_HOST codes and make
them available generally.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Message-Id: <20201116184041.60465-5-hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/scsi/utils.h | 23 ++++++++++++++++-------
scsi/utils.c | 6 +++---
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/include/scsi/utils.h b/include/scsi/utils.h
index ff7c7091b6..ddb22b56df 100644
--- a/include/scsi/utils.h
+++ b/include/scsi/utils.h
@@ -16,6 +16,22 @@ enum SCSIXferMode {
SCSI_XFER_TO_DEV, /* WRITE, MODE_SELECT, ... */
};
+enum SCSIHostStatus {
+ SCSI_HOST_OK,
+ SCSI_HOST_NO_LUN,
+ SCSI_HOST_BUSY,
+ SCSI_HOST_TIME_OUT,
+ SCSI_HOST_BAD_RESPONSE,
+ SCSI_HOST_ABORTED,
+ SCSI_HOST_ERROR = 0x07,
+ SCSI_HOST_RESET = 0x08,
+ SCSI_HOST_TRANSPORT_DISRUPTED = 0xe,
+ SCSI_HOST_TARGET_FAILURE = 0x10,
+ SCSI_HOST_RESERVATION_ERROR = 0x11,
+ SCSI_HOST_ALLOCATION_FAILURE = 0x12,
+ SCSI_HOST_MEDIUM_ERROR = 0x13,
+};
+
typedef struct SCSICommand {
uint8_t buf[SCSI_CMD_BUF_SIZE];
int len;
@@ -124,13 +140,6 @@ int scsi_cdb_length(uint8_t *buf);
#define SG_ERR_DRIVER_TIMEOUT 0x06
#define SG_ERR_DRIVER_SENSE 0x08
-#define SG_ERR_DID_OK 0x00
-#define SG_ERR_DID_NO_CONNECT 0x01
-#define SG_ERR_DID_BUS_BUSY 0x02
-#define SG_ERR_DID_TIME_OUT 0x03
-
-#define SG_ERR_DRIVER_SENSE 0x08
-
int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr,
SCSISense *sense);
#endif
diff --git a/scsi/utils.c b/scsi/utils.c
index 6b56e01002..4d994b6d56 100644
--- a/scsi/utils.c
+++ b/scsi/utils.c
@@ -612,9 +612,9 @@ int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr,
if (errno_value != 0) {
return scsi_sense_from_errno(errno_value, sense);
} else {
- if (io_hdr->host_status == SG_ERR_DID_NO_CONNECT ||
- io_hdr->host_status == SG_ERR_DID_BUS_BUSY ||
- io_hdr->host_status == SG_ERR_DID_TIME_OUT ||
+ if (io_hdr->host_status == SCSI_HOST_NO_LUN ||
+ io_hdr->host_status == SCSI_HOST_BUSY ||
+ io_hdr->host_status == SCSI_HOST_TIME_OUT ||
(io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT)) {
return BUSY;
} else if (io_hdr->host_status) {
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 14/23] scsi: Add mapping for generic SCSI_HOST status to sense codes
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (12 preceding siblings ...)
2021-03-06 10:54 ` [PULL 13/23] scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error codes Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 15/23] scsi-generic: do not snoop the output of failed commands Paolo Bonzini
` (10 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Hannes Reinecke
From: Hannes Reinecke <hare@suse.de>
As we don't have a driver-specific mapping (yet) we should provide
for a detailed mapping from host_status to SCSI sense codes.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Message-Id: <20201116184041.60465-6-hare@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/scsi/utils.h | 1 +
scsi/utils.c | 65 +++++++++++++++++++++++++++++++++++++++-----
2 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/include/scsi/utils.h b/include/scsi/utils.h
index ddb22b56df..9080d65e27 100644
--- a/include/scsi/utils.h
+++ b/include/scsi/utils.h
@@ -145,5 +145,6 @@ int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr,
#endif
int scsi_sense_from_errno(int errno_value, SCSISense *sense);
+int scsi_sense_from_host_status(uint8_t host_status, SCSISense *sense);
#endif
diff --git a/scsi/utils.c b/scsi/utils.c
index 4d994b6d56..28eb32746e 100644
--- a/scsi/utils.c
+++ b/scsi/utils.c
@@ -257,6 +257,21 @@ const struct SCSISense sense_code_LUN_COMM_FAILURE = {
.key = ABORTED_COMMAND, .asc = 0x08, .ascq = 0x00
};
+/* Command aborted, LUN does not respond to selection */
+const struct SCSISense sense_code_LUN_NOT_RESPONDING = {
+ .key = ABORTED_COMMAND, .asc = 0x05, .ascq = 0x00
+};
+
+/* Command aborted, Command Timeout during processing */
+const struct SCSISense sense_code_COMMAND_TIMEOUT = {
+ .key = ABORTED_COMMAND, .asc = 0x2e, .ascq = 0x02
+};
+
+/* Command aborted, Commands cleared by device server */
+const struct SCSISense sense_code_COMMAND_ABORTED = {
+ .key = ABORTED_COMMAND, .asc = 0x2f, .ascq = 0x02
+};
+
/* Medium Error, Unrecovered read error */
const struct SCSISense sense_code_READ_ERROR = {
.key = MEDIUM_ERROR, .asc = 0x11, .ascq = 0x00
@@ -605,6 +620,45 @@ int scsi_sense_from_errno(int errno_value, SCSISense *sense)
}
}
+int scsi_sense_from_host_status(uint8_t host_status,
+ SCSISense *sense)
+{
+ switch (host_status) {
+ case SCSI_HOST_NO_LUN:
+ *sense = SENSE_CODE(LUN_NOT_RESPONDING);
+ return CHECK_CONDITION;
+ case SCSI_HOST_BUSY:
+ return BUSY;
+ case SCSI_HOST_TIME_OUT:
+ *sense = SENSE_CODE(COMMAND_TIMEOUT);
+ return CHECK_CONDITION;
+ case SCSI_HOST_BAD_RESPONSE:
+ *sense = SENSE_CODE(LUN_COMM_FAILURE);
+ return CHECK_CONDITION;
+ case SCSI_HOST_ABORTED:
+ *sense = SENSE_CODE(COMMAND_ABORTED);
+ return CHECK_CONDITION;
+ case SCSI_HOST_RESET:
+ *sense = SENSE_CODE(RESET);
+ return CHECK_CONDITION;
+ case SCSI_HOST_TRANSPORT_DISRUPTED:
+ *sense = SENSE_CODE(I_T_NEXUS_LOSS);
+ return CHECK_CONDITION;
+ case SCSI_HOST_TARGET_FAILURE:
+ *sense = SENSE_CODE(TARGET_FAILURE);
+ return CHECK_CONDITION;
+ case SCSI_HOST_RESERVATION_ERROR:
+ return RESERVATION_CONFLICT;
+ case SCSI_HOST_ALLOCATION_FAILURE:
+ *sense = SENSE_CODE(SPACE_ALLOC_FAILED);
+ return CHECK_CONDITION;
+ case SCSI_HOST_MEDIUM_ERROR:
+ *sense = SENSE_CODE(READ_ERROR);
+ return CHECK_CONDITION;
+ }
+ return GOOD;
+}
+
#ifdef CONFIG_LINUX
int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr,
SCSISense *sense)
@@ -612,14 +666,11 @@ int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr,
if (errno_value != 0) {
return scsi_sense_from_errno(errno_value, sense);
} else {
- if (io_hdr->host_status == SCSI_HOST_NO_LUN ||
- io_hdr->host_status == SCSI_HOST_BUSY ||
- io_hdr->host_status == SCSI_HOST_TIME_OUT ||
- (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT)) {
+ int status = scsi_sense_from_host_status(io_hdr->host_status, sense);
+ if (status) {
+ return status;
+ } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
return BUSY;
- } else if (io_hdr->host_status) {
- *sense = SENSE_CODE(I_T_NEXUS_LOSS);
- return CHECK_CONDITION;
} else if (io_hdr->status) {
return io_hdr->status;
} else if (io_hdr->driver_status & SG_ERR_DRIVER_SENSE) {
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 15/23] scsi-generic: do not snoop the output of failed commands
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (13 preceding siblings ...)
2021-03-06 10:54 ` [PULL 14/23] scsi: Add mapping for generic SCSI_HOST status to sense codes Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 16/23] scsi: inline sg_io_sense_from_errno() into the callers Paolo Bonzini
` (9 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel
If a READ CAPACITY command would fail, for example s->qdev.blocksize would be
set to zero and cause a division by zero on the next use.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-generic.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index cf7e11cf44..f9fd2ccfdd 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -288,7 +288,10 @@ static void scsi_read_complete(void * opaque, int ret)
}
}
- if (len == 0) {
+ if (r->io_header.host_status != SCSI_HOST_OK ||
+ (r->io_header.driver_status & SG_ERR_DRIVER_TIMEOUT) ||
+ r->io_header.status != GOOD ||
+ len == 0) {
scsi_command_complete_noio(r, 0);
goto done;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 16/23] scsi: inline sg_io_sense_from_errno() into the callers.
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (14 preceding siblings ...)
2021-03-06 10:54 ` [PULL 15/23] scsi-generic: do not snoop the output of failed commands Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 17/23] scsi: move host_status handling into SCSI drivers Paolo Bonzini
` (8 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Hannes Reinecke
From: Hannes Reinecke <hare@suse.de>
Currently sg_io_sense_from_errno() converts the two input parameters
'errno' and 'io_hdr' into sense code and SCSI status. Having
split the function off into scsi_sense_from_errno() and
scsi_sense_from_host_status(), both of which are available generically,
we now inline the logic in the callers so that scsi-disk and
scsi-generic will be able to pass host_status to the HBA.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Message-Id: <20201116184041.60465-7-hare@suse.de>
[Put together from "scsi-disk: Add sg_io callback to evaluate status"
and what remains of "scsi: split sg_io_sense_from_errno() in two functions",
with many other fixes. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-disk.c | 47 +++++++++++++++++++++++++++++++++++++-----
hw/scsi/scsi-generic.c | 22 ++++++++++++++------
include/scsi/utils.h | 3 ---
scsi/qemu-pr-helper.c | 24 ++++++++++++++-------
scsi/utils.c | 23 ---------------------
5 files changed, 75 insertions(+), 44 deletions(-)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index a5a58d7db3..ceaf78b423 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -77,7 +77,6 @@ typedef struct SCSIDiskReq {
struct iovec iov;
QEMUIOVector qiov;
BlockAcctCookie acct;
- unsigned char *status;
} SCSIDiskReq;
#define SCSI_DISK_F_REMOVABLE 0
@@ -261,8 +260,6 @@ static bool scsi_disk_req_check_error(SCSIDiskReq *r, int ret, bool acct_failed)
if (ret < 0) {
return scsi_handle_rw_error(r, ret, acct_failed);
- } else if (r->status && *r->status) {
- return scsi_handle_rw_error(r, *r->status, acct_failed);
}
return false;
@@ -2697,8 +2694,47 @@ typedef struct SCSIBlockReq {
/* CDB passed to SG_IO. */
uint8_t cdb[16];
+ BlockCompletionFunc *cb;
+ void *cb_opaque;
} SCSIBlockReq;
+static void scsi_block_sgio_complete(void *opaque, int ret)
+{
+ SCSIBlockReq *req = (SCSIBlockReq *)opaque;
+ SCSIDiskReq *r = &req->req;
+ SCSIDevice *s = r->req.dev;
+ sg_io_hdr_t *io_hdr = &req->io_header;
+ SCSISense sense;
+
+ if (ret == 0) {
+ if (io_hdr->host_status != SCSI_HOST_OK) {
+ ret = scsi_sense_from_host_status(io_hdr->host_status, &sense);
+ if (ret == CHECK_CONDITION) {
+ scsi_req_build_sense(&r->req, sense);
+ }
+ } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
+ ret = BUSY;
+ } else {
+ ret = io_hdr->status;
+ }
+
+ if (ret > 0) {
+ aio_context_acquire(blk_get_aio_context(s->conf.blk));
+ if (scsi_handle_rw_error(r, ret, true)) {
+ aio_context_release(blk_get_aio_context(s->conf.blk));
+ scsi_req_unref(&r->req);
+ return;
+ }
+ aio_context_release(blk_get_aio_context(s->conf.blk));
+
+ /* Ignore error. */
+ ret = 0;
+ }
+ }
+
+ req->cb(req->cb_opaque, ret);
+}
+
static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
int64_t offset, QEMUIOVector *iov,
int direction,
@@ -2777,9 +2813,11 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
io_header->timeout = s->qdev.io_timeout * 1000;
io_header->usr_ptr = r;
io_header->flags |= SG_FLAG_DIRECT_IO;
+ req->cb = cb;
+ req->cb_opaque = opaque;
trace_scsi_disk_aio_sgio_command(r->req.tag, req->cdb[0], lba,
nb_logical_blocks, io_header->timeout);
- aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, cb, opaque);
+ aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, scsi_block_sgio_complete, req);
assert(aiocb != NULL);
return aiocb;
}
@@ -2893,7 +2931,6 @@ static int32_t scsi_block_dma_command(SCSIRequest *req, uint8_t *buf)
return 0;
}
- r->req.status = &r->io_header.status;
return scsi_disk_dma_command(req, buf);
}
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index f9fd2ccfdd..02b87819e5 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -75,6 +75,7 @@ static void scsi_command_complete_noio(SCSIGenericReq *r, int ret)
{
int status;
SCSISense sense;
+ sg_io_hdr_t *io_hdr = &r->io_header;
assert(r->req.aiocb == NULL);
@@ -82,15 +83,24 @@ static void scsi_command_complete_noio(SCSIGenericReq *r, int ret)
scsi_req_cancel_complete(&r->req);
goto done;
}
- status = sg_io_sense_from_errno(-ret, &r->io_header, &sense);
- if (status == CHECK_CONDITION) {
- if (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) {
- r->req.sense_len = r->io_header.sb_len_wr;
- } else {
+ if (ret < 0) {
+ status = scsi_sense_from_errno(-ret, &sense);
+ if (status == CHECK_CONDITION) {
+ scsi_req_build_sense(&r->req, sense);
+ }
+ } else if (io_hdr->host_status != SCSI_HOST_OK) {
+ status = scsi_sense_from_host_status(io_hdr->host_status, &sense);
+ if (status == CHECK_CONDITION) {
scsi_req_build_sense(&r->req, sense);
}
+ } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
+ status = BUSY;
+ } else {
+ status = io_hdr->status;
+ if (io_hdr->driver_status & SG_ERR_DRIVER_SENSE) {
+ r->req.sense_len = io_hdr->sb_len_wr;
+ }
}
-
trace_scsi_generic_command_complete_noio(r, r->req.tag, status);
scsi_req_complete(&r->req, status);
diff --git a/include/scsi/utils.h b/include/scsi/utils.h
index 9080d65e27..d5c8efa16e 100644
--- a/include/scsi/utils.h
+++ b/include/scsi/utils.h
@@ -139,9 +139,6 @@ int scsi_cdb_length(uint8_t *buf);
#ifdef CONFIG_LINUX
#define SG_ERR_DRIVER_TIMEOUT 0x06
#define SG_ERR_DRIVER_SENSE 0x08
-
-int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr,
- SCSISense *sense);
#endif
int scsi_sense_from_errno(int errno_value, SCSISense *sense);
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 2733d92f2d..7b9389b47b 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -149,19 +149,29 @@ static int do_sgio_worker(void *opaque)
io_hdr.dxferp = (char *)data->buf;
io_hdr.dxfer_len = data->sz;
ret = ioctl(data->fd, SG_IO, &io_hdr);
- status = sg_io_sense_from_errno(ret < 0 ? errno : 0, &io_hdr,
- &sense_code);
+
+ if (ret < 0) {
+ status = scsi_sense_from_errno(errno, &sense_code);
+ if (status == CHECK_CONDITION) {
+ scsi_build_sense(data->sense, sense_code);
+ }
+ } else if (io_hdr.host_status != SCSI_HOST_OK) {
+ status = scsi_sense_from_host_status(io_hdr.host_status, &sense_code);
+ if (status == CHECK_CONDITION) {
+ scsi_build_sense(data->sense, sense_code);
+ }
+ } else if (io_hdr.driver_status & SG_ERR_DRIVER_TIMEOUT) {
+ status = BUSY;
+ } else {
+ status = io_hdr.status;
+ }
+
if (status == GOOD) {
data->sz -= io_hdr.resid;
} else {
data->sz = 0;
}
- if (status == CHECK_CONDITION &&
- !(io_hdr.driver_status & SG_ERR_DRIVER_SENSE)) {
- scsi_build_sense(data->sense, sense_code);
- }
-
return status;
}
diff --git a/scsi/utils.c b/scsi/utils.c
index 28eb32746e..873e05aeaf 100644
--- a/scsi/utils.c
+++ b/scsi/utils.c
@@ -658,26 +658,3 @@ int scsi_sense_from_host_status(uint8_t host_status,
}
return GOOD;
}
-
-#ifdef CONFIG_LINUX
-int sg_io_sense_from_errno(int errno_value, struct sg_io_hdr *io_hdr,
- SCSISense *sense)
-{
- if (errno_value != 0) {
- return scsi_sense_from_errno(errno_value, sense);
- } else {
- int status = scsi_sense_from_host_status(io_hdr->host_status, sense);
- if (status) {
- return status;
- } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
- return BUSY;
- } else if (io_hdr->status) {
- return io_hdr->status;
- } else if (io_hdr->driver_status & SG_ERR_DRIVER_SENSE) {
- return CHECK_CONDITION;
- } else {
- return GOOD;
- }
- }
-}
-#endif
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 17/23] scsi: move host_status handling into SCSI drivers
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (15 preceding siblings ...)
2021-03-06 10:54 ` [PULL 16/23] scsi: inline sg_io_sense_from_errno() into the callers Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 18/23] qemu-option: do not suggest using the delay option Paolo Bonzini
` (7 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Hannes Reinecke
From: Hannes Reinecke <hare@suse.de>
Some SCSI drivers like virtio have an internal mapping for the
host_status. This patch moves the host_status translation into
the SCSI drivers to allow those drivers to set up the correct
values.
Signed-off-by: Hannes Reinecke <hare@suse.de>.
[Added default handling to avoid touching all drivers. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-bus.c | 33 ++++++++++++++++++++++++++++--
hw/scsi/scsi-disk.c | 12 +++++------
hw/scsi/scsi-generic.c | 6 ++----
hw/scsi/virtio-scsi.c | 46 ++++++++++++++++++++++++++++++++++++++++++
hw/scsi/vmw_pvscsi.c | 39 +++++++++++++++++++++++++++++++++++
include/hw/scsi/scsi.h | 5 ++++-
6 files changed, 128 insertions(+), 13 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index dc4141ec8d..2d674f94d7 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -692,6 +692,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
req->lun = lun;
req->hba_private = hba_private;
req->status = -1;
+ req->host_status = -1;
req->ops = reqops;
object_ref(OBJECT(d));
object_ref(OBJECT(qbus->parent));
@@ -1455,10 +1456,38 @@ void scsi_req_print(SCSIRequest *req)
}
}
+void scsi_req_complete_failed(SCSIRequest *req, int host_status)
+{
+ SCSISense sense;
+ int status;
+
+ assert(req->status == -1 && req->host_status == -1);
+ assert(req->ops != &reqops_unit_attention);
+
+ if (!req->bus->info->fail) {
+ status = scsi_sense_from_host_status(req->host_status, &sense);
+ if (status == CHECK_CONDITION) {
+ scsi_req_build_sense(req, sense);
+ }
+ scsi_req_complete(req, status);
+ return;
+ }
+
+ req->host_status = host_status;
+ scsi_req_ref(req);
+ scsi_req_dequeue(req);
+ req->bus->info->fail(req);
+
+ /* Cancelled requests might end up being completed instead of cancelled */
+ notifier_list_notify(&req->cancel_notifiers, req);
+ scsi_req_unref(req);
+}
+
void scsi_req_complete(SCSIRequest *req, int status)
{
- assert(req->status == -1);
+ assert(req->status == -1 && req->host_status == -1);
req->status = status;
+ req->host_status = SCSI_HOST_OK;
assert(req->sense_len <= sizeof(req->sense));
if (status == GOOD) {
@@ -1646,7 +1675,7 @@ static int put_scsi_requests(QEMUFile *f, void *pv, size_t size,
QTAILQ_FOREACH(req, &s->requests, next) {
assert(!req->io_canceled);
- assert(req->status == -1);
+ assert(req->status == -1 && req->host_status == -1);
assert(req->enqueued);
qemu_put_sbyte(f, req->retry ? 1 : 2);
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index ceaf78b423..bd7103cd0e 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2704,15 +2704,15 @@ static void scsi_block_sgio_complete(void *opaque, int ret)
SCSIDiskReq *r = &req->req;
SCSIDevice *s = r->req.dev;
sg_io_hdr_t *io_hdr = &req->io_header;
- SCSISense sense;
if (ret == 0) {
if (io_hdr->host_status != SCSI_HOST_OK) {
- ret = scsi_sense_from_host_status(io_hdr->host_status, &sense);
- if (ret == CHECK_CONDITION) {
- scsi_req_build_sense(&r->req, sense);
- }
- } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
+ scsi_req_complete_failed(&r->req, io_hdr->host_status);
+ scsi_req_unref(&r->req);
+ return;
+ }
+
+ if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
ret = BUSY;
} else {
ret = io_hdr->status;
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 02b87819e5..98c30c5d5c 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -89,10 +89,8 @@ static void scsi_command_complete_noio(SCSIGenericReq *r, int ret)
scsi_req_build_sense(&r->req, sense);
}
} else if (io_hdr->host_status != SCSI_HOST_OK) {
- status = scsi_sense_from_host_status(io_hdr->host_status, &sense);
- if (status == CHECK_CONDITION) {
- scsi_req_build_sense(&r->req, sense);
- }
+ scsi_req_complete_failed(&r->req, io_hdr->host_status);
+ goto done;
} else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {
status = BUSY;
} else {
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 358c0e70b0..6d80730287 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -500,6 +500,51 @@ static void virtio_scsi_complete_cmd_req(VirtIOSCSIReq *req)
virtio_scsi_complete_req(req);
}
+static void virtio_scsi_command_failed(SCSIRequest *r)
+{
+ VirtIOSCSIReq *req = r->hba_private;
+
+ if (r->io_canceled) {
+ return;
+ }
+
+ req->resp.cmd.status = GOOD;
+ switch (r->host_status) {
+ case SCSI_HOST_NO_LUN:
+ req->resp.cmd.response = VIRTIO_SCSI_S_INCORRECT_LUN;
+ break;
+ case SCSI_HOST_BUSY:
+ req->resp.cmd.response = VIRTIO_SCSI_S_BUSY;
+ break;
+ case SCSI_HOST_TIME_OUT:
+ case SCSI_HOST_ABORTED:
+ req->resp.cmd.response = VIRTIO_SCSI_S_ABORTED;
+ break;
+ case SCSI_HOST_BAD_RESPONSE:
+ req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
+ break;
+ case SCSI_HOST_RESET:
+ req->resp.cmd.response = VIRTIO_SCSI_S_RESET;
+ break;
+ case SCSI_HOST_TRANSPORT_DISRUPTED:
+ req->resp.cmd.response = VIRTIO_SCSI_S_TRANSPORT_FAILURE;
+ break;
+ case SCSI_HOST_TARGET_FAILURE:
+ req->resp.cmd.response = VIRTIO_SCSI_S_TARGET_FAILURE;
+ break;
+ case SCSI_HOST_RESERVATION_ERROR:
+ req->resp.cmd.response = VIRTIO_SCSI_S_NEXUS_FAILURE;
+ break;
+ case SCSI_HOST_ALLOCATION_FAILURE:
+ case SCSI_HOST_MEDIUM_ERROR:
+ case SCSI_HOST_ERROR:
+ default:
+ req->resp.cmd.response = VIRTIO_SCSI_S_FAILURE;
+ break;
+ }
+ virtio_scsi_complete_cmd_req(req);
+}
+
static void virtio_scsi_command_complete(SCSIRequest *r, size_t resid)
{
VirtIOSCSIReq *req = r->hba_private;
@@ -908,6 +953,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
.max_lun = VIRTIO_SCSI_MAX_LUN,
.complete = virtio_scsi_command_complete,
+ .fail = virtio_scsi_command_failed,
.cancel = virtio_scsi_request_cancelled,
.change = virtio_scsi_change,
.parse_cdb = virtio_scsi_parse_cdb,
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 0da378ed50..1f30cb020a 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -510,6 +510,44 @@ pvscsi_write_sense(PVSCSIRequest *r, uint8_t *sense, int len)
cpu_physical_memory_write(r->req.senseAddr, sense, r->cmp.senseLen);
}
+static void
+pvscsi_command_failed(SCSIRequest *req)
+{
+ PVSCSIRequest *pvscsi_req = req->hba_private;
+ PVSCSIState *s;
+
+ if (!pvscsi_req) {
+ trace_pvscsi_command_complete_not_found(req->tag);
+ return;
+ }
+ s = pvscsi_req->dev;
+
+ switch (req->host_status) {
+ case SCSI_HOST_NO_LUN:
+ pvscsi_req->cmp.hostStatus = BTSTAT_LUNMISMATCH;
+ break;
+ case SCSI_HOST_BUSY:
+ pvscsi_req->cmp.hostStatus = BTSTAT_ABORTQUEUE;
+ break;
+ case SCSI_HOST_TIME_OUT:
+ case SCSI_HOST_ABORTED:
+ pvscsi_req->cmp.hostStatus = BTSTAT_SENTRST;
+ break;
+ case SCSI_HOST_BAD_RESPONSE:
+ pvscsi_req->cmp.hostStatus = BTSTAT_SELTIMEO;
+ break;
+ case SCSI_HOST_RESET:
+ pvscsi_req->cmp.hostStatus = BTSTAT_BUSRESET;
+ break;
+ default:
+ pvscsi_req->cmp.hostStatus = BTSTAT_HASOFTWARE;
+ break;
+ }
+ pvscsi_req->cmp.scsiStatus = GOOD;
+ qemu_sglist_destroy(&pvscsi_req->sgl);
+ pvscsi_complete_request(s, pvscsi_req);
+}
+
static void
pvscsi_command_complete(SCSIRequest *req, size_t resid)
{
@@ -1103,6 +1141,7 @@ static const struct SCSIBusInfo pvscsi_scsi_info = {
.get_sg_list = pvscsi_get_sg_list,
.complete = pvscsi_command_complete,
.cancel = pvscsi_request_cancelled,
+ .fail = pvscsi_command_failed,
};
static void
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 5d992e6e1d..0b726bc78c 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -27,7 +27,8 @@ struct SCSIRequest {
uint32_t refcount;
uint32_t tag;
uint32_t lun;
- uint32_t status;
+ int16_t status;
+ int16_t host_status;
void *hba_private;
size_t resid;
SCSICommand cmd;
@@ -123,6 +124,7 @@ struct SCSIBusInfo {
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
void (*transfer_data)(SCSIRequest *req, uint32_t arg);
+ void (*fail)(SCSIRequest *req);
void (*complete)(SCSIRequest *req, size_t resid);
void (*cancel)(SCSIRequest *req);
void (*change)(SCSIBus *bus, SCSIDevice *dev, SCSISense sense);
@@ -177,6 +179,7 @@ void scsi_req_print(SCSIRequest *req);
void scsi_req_continue(SCSIRequest *req);
void scsi_req_data(SCSIRequest *req, int len);
void scsi_req_complete(SCSIRequest *req, int status);
+void scsi_req_complete_failed(SCSIRequest *req, int host_status);
uint8_t *scsi_req_get_buf(SCSIRequest *req);
int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len);
void scsi_req_cancel_complete(SCSIRequest *req);
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 18/23] qemu-option: do not suggest using the delay option
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (16 preceding siblings ...)
2021-03-06 10:54 ` [PULL 17/23] scsi: move host_status handling into SCSI drivers Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 19/23] build-sys: invoke ninja with -d keepdepfile Paolo Bonzini
` (6 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P . Berrangé
The "delay" option was a hack that was introduced to allow writing "nodelay".
We are adding a "nodelay" option to be used as "nodelay=on", so recommend it
instead of "delay".
This is quite ugly, but a proper deprecation of "delay"
cannot be done if QEMU starts suggesting it. Since it's the
only case I opted for this very much ad-hoc patch.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/system/deprecated.rst | 6 ++++++
util/qemu-option.c | 6 +++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index fcf0ca4068..cfabe69846 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -134,6 +134,12 @@ Boolean options such as ``share=on``/``share=off`` could be written
in short form as ``share`` and ``noshare``. This is now deprecated
and will cause a warning.
+``delay`` option for socket character devices (since 6.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
+rather than ``delay=off``.
+
``--enable-fips`` (since 6.0)
'''''''''''''''''''''''''''''
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 40564a12eb..9678d5b682 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -785,7 +785,11 @@ static const char *get_opt_name_value(const char *params,
}
if (!is_help && warn_on_flag) {
warn_report("short-form boolean option '%s%s' deprecated", prefix, *name);
- error_printf("Please use %s=%s instead\n", *name, *value);
+ if (g_str_equal(*name, "delay")) {
+ error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");
+ } else {
+ error_printf("Please use %s=%s instead\n", *name, *value);
+ }
}
}
} else {
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 19/23] build-sys: invoke ninja with -d keepdepfile
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (17 preceding siblings ...)
2021-03-06 10:54 ` [PULL 18/23] qemu-option: do not suggest using the delay option Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 20/23] meson: adjust timeouts for some slower tests Paolo Bonzini
` (5 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel
After reading the dependency file, ninja just deletes it, in the name
of cleanliness I guess. However this complicates debugging unnecessarily
compared to good old "-include *.d". Use the keepdepfile debugging
option to make it easier to see what is going on.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210215122103.63933-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index d7fb6b270e..bcbbec71a1 100644
--- a/Makefile
+++ b/Makefile
@@ -149,7 +149,7 @@ $(ninja-targets): run-ninja
# --output-sync line.
run-ninja: config-host.mak
ifneq ($(filter $(ninja-targets), $(ninja-cmd-goals)),)
- +$(quiet-@)$(if $(MAKE.nq),@:, $(NINJA) \
+ +$(quiet-@)$(if $(MAKE.nq),@:, $(NINJA) -d keepdepfile \
$(NINJAFLAGS) $(sort $(filter $(ninja-targets), $(ninja-cmd-goals))) | cat)
endif
endif
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 20/23] meson: adjust timeouts for some slower tests
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (18 preceding siblings ...)
2021-03-06 10:54 ` [PULL 19/23] build-sys: invoke ninja with -d keepdepfile Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 21/23] trace: fix "-trace file=..." Paolo Bonzini
` (4 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel
Adjust the timeouts for the benchmarks (Meson 0.57 allows 0 to mean
infinite) and for the longest running tests. These are the
times that I measured and the corresponding timeouts. For generic
qtests, the target that reported the longest runtime is included.
unit tests:
test-crypto-tlscredsx509 13.15s 45s
test-crypto-tlssession 14.12s 45s
qtests:
qos-test 21.26s 60s (i386)
ahci-test 22.18s 60s
pxe-test 26.51s 60s
boot-serial-test 28.02s 60s (sparc)
prom-env-test 28.86s 60s
bios-tables-test 50.17s 120s (aarch64)
test-hmp 57.15s 120s (aarch64)
npcm7xx_pwm-test 71.27s 150s
migration-test 97.09s 150s (aarch64)
qom-test 139.20s 240s (aarch64)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/fp/meson.build | 2 +-
tests/meson.build | 8 ++++++++
tests/qtest/meson.build | 15 +++++++++++++++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/tests/fp/meson.build b/tests/fp/meson.build
index 8d739c4d59..1c3eee9955 100644
--- a/tests/fp/meson.build
+++ b/tests/fp/meson.build
@@ -624,7 +624,7 @@ test('fp-test-mulAdd', fptest,
# no fptest_rounding_args
args: fptest_args +
['f16_mulAdd', 'f32_mulAdd', 'f64_mulAdd', 'f128_mulAdd'],
- suite: ['softfloat-slow', 'softfloat-ops-slow'], timeout: 60)
+ suite: ['softfloat-slow', 'softfloat-ops-slow'], timeout: 90)
fpbench = executable(
'fp-bench',
diff --git a/tests/meson.build b/tests/meson.build
index 7d7da6a636..656d211e25 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -237,6 +237,11 @@ test_env = environment()
test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
+slow_tests = {
+ 'test-crypto-tlscredsx509': 45,
+ 'test-crypto-tlssession': 45
+}
+
foreach test_name, extra: tests
src = [test_name + '.c']
deps = [qemuutil]
@@ -254,6 +259,8 @@ foreach test_name, extra: tests
env: test_env,
args: ['--tap', '-k'],
protocol: 'tap',
+ timeout: slow_tests.get(test_name, 30),
+ priority: slow_tests.get(test_name, 30),
suite: ['unit'])
endforeach
@@ -263,6 +270,7 @@ foreach bench_name, deps: benchs
benchmark(bench_name, exe,
args: ['--tap', '-k'],
protocol: 'tap',
+ timeout: 0,
suite: ['speed'])
endforeach
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index ba6ecaed32..8c79d5cc3b 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -4,6 +4,19 @@ if not config_host.has_key('CONFIG_POSIX')
subdir_done()
endif
+slow_qtests = {
+ 'ahci-test' : 60,
+ 'bios-tables-test' : 120,
+ 'boot-serial-test' : 60,
+ 'migration-test' : 150,
+ 'npcm7xx_pwm-test': 150,
+ 'prom-env-test' : 60,
+ 'pxe-test' : 60,
+ 'qos-test' : 60,
+ 'qom-test' : 300,
+ 'test-hmp' : 120,
+}
+
qtests_generic = [
'cdrom-test',
'device-introspect-test',
@@ -273,6 +286,8 @@ foreach dir : target_dirs
env: qtest_env,
args: ['--tap', '-k'],
protocol: 'tap',
+ timeout: slow_qtests.get(test, 30),
+ priority: slow_qtests.get(test, 30),
suite: ['qtest', 'qtest-' + target_base])
endforeach
endforeach
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 21/23] trace: fix "-trace file=..."
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (19 preceding siblings ...)
2021-03-06 10:54 ` [PULL 20/23] meson: adjust timeouts for some slower tests Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 22/23] trace: skip qemu_set_log_filename if no "-D" option was passed Paolo Bonzini
` (3 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, stefanha
Because trace_opt_parse always deletes the options it has parsed,
trace_init_file's call to qemu_find_opts_singleton always
creates an empty -trace option group. Therefore, the subsequent
qemu_opt_get(opts, "file") always returns NULL.
To fix this, save the last "-trace file=..." option in a global
variable and use it later in trace_init_file.
This is similar to what was done before commit 92eecfff32 ("trace:
remove argument from trace_init_file", 2020-11-11), except contained
within trace/control.c and without memory leaks.
Fixes: 92eecfff32 ("trace: remove argument from trace_init_file", 2020-11-11)
Cc: stefanha@redhat.com
Reported-by: armbru@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210209145759.141231-2-pbonzini@redhat.com>
---
trace/control.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/trace/control.c b/trace/control.c
index cd04dd4e0c..4be38e1af2 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -40,6 +40,7 @@ static size_t nevent_groups;
static uint32_t next_id;
static uint32_t next_vcpu_id;
static bool init_trace_on_startup;
+static char *trace_opts_file;
QemuOptsList qemu_trace_opts = {
.name = "trace",
@@ -224,10 +225,8 @@ static void trace_init_events(const char *fname)
void trace_init_file(void)
{
- QemuOpts *opts = qemu_find_opts_singleton("trace");
- const char *file = qemu_opt_get(opts, "file");
#ifdef CONFIG_TRACE_SIMPLE
- st_set_trace_file(file);
+ st_set_trace_file(trace_opts_file);
if (init_trace_on_startup) {
st_set_trace_file_enabled(true);
}
@@ -238,11 +237,11 @@ void trace_init_file(void)
* backend. However we should only override -D if we actually have
* something to override it with.
*/
- if (file) {
- qemu_set_log_filename(file, &error_fatal);
+ if (trace_opts_file) {
+ qemu_set_log_filename(trace_opts_file, &error_fatal);
}
#else
- if (file) {
+ if (trace_opts_file) {
fprintf(stderr, "error: --trace file=...: "
"option not supported by the selected tracing backends\n");
exit(1);
@@ -303,6 +302,8 @@ void trace_opt_parse(const char *optarg)
}
trace_init_events(qemu_opt_get(opts, "events"));
init_trace_on_startup = true;
+ g_free(trace_opts_file);
+ trace_opts_file = g_strdup(qemu_opt_get(opts, "file"));
qemu_opts_del(opts);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 22/23] trace: skip qemu_set_log_filename if no "-D" option was passed
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (20 preceding siblings ...)
2021-03-06 10:54 ` [PULL 21/23] trace: fix "-trace file=..." Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 10:54 ` [PULL 23/23] meson: Stop if cfi is enabled with system slirp Paolo Bonzini
` (2 subsequent siblings)
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
When the "simple" backend is not active but the "log" backend is,
both "-trace file=" and "-D" will result in a call to
qemu_set_log_filename. Unfortunately, QEMU was also calling
qemu_set_log_filename if "-D" was not passed, so the "-trace
file=" option had no effect and the tracepoints went back to
stderr.
Fortunately we can just skip qemu_set_log_filename in that case,
because the log backend will initialize itself just fine as soon
as qemu_set_log is called, also in qemu_process_early_options.
Cc: stefanha@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20210209145759.141231-3-pbonzini@redhat.com>
---
softmmu/vl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 2f4958db2a..ff488ea3e7 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2364,7 +2364,9 @@ static void qemu_process_early_options(void)
#endif
/* Open the logfile at this point and set the log mask if necessary. */
- qemu_set_log_filename(log_file, &error_fatal);
+ if (log_file) {
+ qemu_set_log_filename(log_file, &error_fatal);
+ }
if (log_mask) {
int mask;
mask = qemu_str_to_log_mask(log_mask);
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 23/23] meson: Stop if cfi is enabled with system slirp
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (21 preceding siblings ...)
2021-03-06 10:54 ` [PULL 22/23] trace: skip qemu_set_log_filename if no "-D" option was passed Paolo Bonzini
@ 2021-03-06 10:54 ` Paolo Bonzini
2021-03-06 11:15 ` [PULL 00/23] Misc patches for 2021-03-06 no-reply
2021-03-08 15:45 ` Peter Maydell
24 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2021-03-06 10:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniele Buono
From: Daniele Buono <dbuono@linux.vnet.ibm.com>
For CFI, we need to compile slirp as a static library together with qemu.
This is because we register slirp functions as callbacks for QEMU Timers.
When using a system-wide shared libslirp, the type information for the
callback is missing and the timer call produces a false positive with CFI.
With this patch, meson will stop if CFI is enabled with system-wide slirp.
In 6.1 we will introduce a new interface to slirp where the callback is
passed as an enum rather than a function pointer.
Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Message-Id: <20210304025939.9164-1-dbuono@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
meson.build | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/meson.build b/meson.build
index 07bc23129a..05fb125dc2 100644
--- a/meson.build
+++ b/meson.build
@@ -1574,6 +1574,18 @@ if have_system
endif
endif
+# For CFI, we need to compile slirp as a static library together with qemu.
+# This is because we register slirp functions as callbacks for QEMU Timers.
+# When using a system-wide shared libslirp, the type information for the
+# callback is missing and the timer call produces a false positive with CFI.
+#
+# Now that slirp_opt has been defined, check if the selected slirp is compatible
+# with control-flow integrity.
+if get_option('cfi') and slirp_opt == 'system'
+ error('Control-Flow Integrity is not compatible with system-wide slirp.' \
+ + ' Please configure with --enable-slirp=git')
+endif
+
fdt = not_found
fdt_opt = get_option('fdt')
if have_system
--
2.29.2
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PULL 00/23] Misc patches for 2021-03-06
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (22 preceding siblings ...)
2021-03-06 10:54 ` [PULL 23/23] meson: Stop if cfi is enabled with system slirp Paolo Bonzini
@ 2021-03-06 11:15 ` no-reply
2021-03-08 15:45 ` Peter Maydell
24 siblings, 0 replies; 27+ messages in thread
From: no-reply @ 2021-03-06 11:15 UTC (permalink / raw)
To: pbonzini; +Cc: qemu-devel
Patchew URL: https://patchew.org/QEMU/20210306105419.110503-1-pbonzini@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210306105419.110503-1-pbonzini@redhat.com
Subject: [PULL 00/23] Misc patches for 2021-03-06
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210306105419.110503-1-pbonzini@redhat.com -> patchew/20210306105419.110503-1-pbonzini@redhat.com
Switched to a new branch 'test'
e8ca3ae meson: Stop if cfi is enabled with system slirp
07dbcab trace: skip qemu_set_log_filename if no "-D" option was passed
f4066a9 trace: fix "-trace file=..."
e68de4e meson: adjust timeouts for some slower tests
77a9ebc build-sys: invoke ninja with -d keepdepfile
c0f4778 qemu-option: do not suggest using the delay option
e9e318f scsi: move host_status handling into SCSI drivers
fee64ec scsi: inline sg_io_sense_from_errno() into the callers.
162e7a7 scsi-generic: do not snoop the output of failed commands
c69f058 scsi: Add mapping for generic SCSI_HOST status to sense codes
3dc14e2 scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error codes
72d7f7c qemu-config: add error propagation to qemu_config_parse
39a40cc x86/pvh: extract only 4 bytes of start address for 32 bit kernels
7bc6f42 elf_ops: correct loading of 32 bit PVH kernel
eee7136 lsilogic: Use PCIDevice::exit instead of DeviceState::unrealize
9a5c9ce accel: kvm: Add aligment assert for kvm_log_clear_one_slot
d3955f4 accel: kvm: Fix memory waste under mismatch page size
52c0f08 vl.c: do not execute trace_init_backends() before daemonizing
bacc541 qom: Check for wellformed id in user_creatable_add_type()
4a7a94a chardev: add nodelay option
bbdceab qga-vss: Use dynamic linking for GLib
351c41a KVM: x86: deprecate -M kernel-irqchip=off except for -M isapc
713ab1d KVM: x86: do not fail if software breakpoint has already been removed
=== OUTPUT BEGIN ===
1/23 Checking commit 713ab1db9c9f (KVM: x86: do not fail if software breakpoint has already been removed)
2/23 Checking commit 351c41a16c9f (KVM: x86: deprecate -M kernel-irqchip=off except for -M isapc)
WARNING: line over 80 characters
#60: FILE: hw/intc/apic.c:881:
+ warn_report("Do not use kernel-irqchip except for the -M isapc machine type.");
total: 0 errors, 1 warnings, 31 lines checked
Patch 2/23 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/23 Checking commit bbdceab56d88 (qga-vss: Use dynamic linking for GLib)
4/23 Checking commit 4a7a94a4597c (chardev: add nodelay option)
5/23 Checking commit bacc5417205b (qom: Check for wellformed id in user_creatable_add_type())
6/23 Checking commit 52c0f083d216 (vl.c: do not execute trace_init_backends() before daemonizing)
7/23 Checking commit d3955f44d7fa (accel: kvm: Fix memory waste under mismatch page size)
8/23 Checking commit 9a5c9cea2bd4 (accel: kvm: Add aligment assert for kvm_log_clear_one_slot)
9/23 Checking commit eee713683833 (lsilogic: Use PCIDevice::exit instead of DeviceState::unrealize)
10/23 Checking commit 7bc6f4223f01 (elf_ops: correct loading of 32 bit PVH kernel)
11/23 Checking commit 39a40ccab60f (x86/pvh: extract only 4 bytes of start address for 32 bit kernels)
12/23 Checking commit 72d7f7ce1df7 (qemu-config: add error propagation to qemu_config_parse)
WARNING: line over 80 characters
#136: FILE: util/qemu-config.c:353:
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
total: 0 errors, 1 warnings, 144 lines checked
Patch 12/23 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/23 Checking commit 3dc14e2345fc (scsi: Rename linux-specific SG_ERR codes to generic SCSI_HOST error codes)
14/23 Checking commit c69f05834a13 (scsi: Add mapping for generic SCSI_HOST status to sense codes)
15/23 Checking commit 162e7a771b90 (scsi-generic: do not snoop the output of failed commands)
16/23 Checking commit fee64ec0ceb5 (scsi: inline sg_io_sense_from_errno() into the callers.)
ERROR: line over 90 characters
#107: FILE: hw/scsi/scsi-disk.c:2820:
+ aiocb = blk_aio_ioctl(s->qdev.conf.blk, SG_IO, io_header, scsi_block_sgio_complete, req);
total: 1 errors, 0 warnings, 166 lines checked
Patch 16/23 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/23 Checking commit e9e318f92d6c (scsi: move host_status handling into SCSI drivers)
18/23 Checking commit c0f4778ff786 (qemu-option: do not suggest using the delay option)
ERROR: line over 90 characters
#50: FILE: util/qemu-option.c:789:
+ error_printf("Please use nodelay=%s instead\n", prefix[0] ? "on" : "off");
total: 1 errors, 0 warnings, 24 lines checked
Patch 18/23 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
19/23 Checking commit 77a9ebc4bb31 (build-sys: invoke ninja with -d keepdepfile)
20/23 Checking commit e68de4e6792d (meson: adjust timeouts for some slower tests)
21/23 Checking commit f4066a9e3296 (trace: fix "-trace file=...")
22/23 Checking commit 07dbcab76e3b (trace: skip qemu_set_log_filename if no "-D" option was passed)
23/23 Checking commit e8ca3ae2f936 (meson: Stop if cfi is enabled with system slirp)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210306105419.110503-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PULL 00/23] Misc patches for 2021-03-06
2021-03-06 10:53 [PULL 00/23] Misc patches for 2021-03-06 Paolo Bonzini
` (23 preceding siblings ...)
2021-03-06 11:15 ` [PULL 00/23] Misc patches for 2021-03-06 no-reply
@ 2021-03-08 15:45 ` Peter Maydell
24 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2021-03-08 15:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On Sat, 6 Mar 2021 at 10:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit e586edcb410543768ef009eaa22a2d9dd4a53846:
>
> virtiofs: drop remapped security.capability xattr as needed (2021-03-04 10:26:16 +0000)
>
> are available in the Git repository at:
>
> https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to c715343fd96bcf93263fda38d81af815fdb5a7fa:
>
> meson: Stop if cfi is enabled with system slirp (2021-03-06 11:42:57 +0100)
>
> ----------------------------------------------------------------
> * fix tracing vs -daemonize (Daniel)
> * detect invalid CFI configuration (Daniele)
> * 32-bit PVH fix (David)
> * forward SCSI passthrough host-status to the SCSI HBA (Hannes)
> * detect ill-formed id in QMP object-add (Kevin)
> * miscellaneous bugfixes and cleanups (Keqian, Kostiantyn, myself, Peng Liang)
> * add nodelay option for chardev (myself)
> * deprecate -M kernel-irqchip=off on x86 (myself)
> * keep .d files (myself)
> * Fix -trace file (myself)
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread