[Qemu-devel] [PULL 00/11] scsi, RCU, KVM, x86 changes for 2015-03-10

[Qemu-devel] [PULL 00/11] scsi, RCU, KVM, x86 changes for 2015-03-10

[Paolo Bonzini]

The following changes since commit 277263e1b320d759a760ba6c5ea75ec268f929e5:

  Merge remote-tracking branch 'remotes/agraf/tags/signed-ppc-for-upstream' into staging (2015-03-09 14:04:14 +0000)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to ac57622985220de064059971f9ccb00905e9bd04:

  x86: fix SS selector in SYSRET (2015-03-10 11:18:24 +0100)

----------------------------------------------------------------
- scsi: improvements to error reporting and conversion to realize,
  Coverity/sparse fix for iscsi driver
- RCU fallout: fix -daemonize and s390x system emulation
- KVM: kvm_stat improvements and new man page
- x86: SYSRET fix for VxWorks

----------------------------------------------------------------
Bill Paul (1):
      x86: fix SS selector in SYSRET

Markus Armbruster (4):
      scsi: Clean up duplicated error in legacy if=scsi code
      hw: Propagate errors through qdev_prop_set_drive()
      scsi: Improve error reporting for invalid drive property
      scsi: Convert remaining PCI HBAs to realize()

Paolo Bonzini (3):
      qemu-thread: do not use PTHREAD_MUTEX_ERRORCHECK
      rcu: handle forks safely
      cpus: initialize cpu->memory_dispatch

Stefan Hajnoczi (2):
      kvm_stat: add column headers to text UI
      kvm_stat: add kvm_stat.1 man page

Stefan Weil (1):
      iscsi: Fix check for username

 Makefile                         |  9 +++++++
 block/iscsi.c                    |  2 +-
 exec.c                           |  1 +
 hw/arm/vexpress.c                |  6 ++---
 hw/arm/virt.c                    |  6 ++---
 hw/block/pflash_cfi01.c          |  4 +--
 hw/block/pflash_cfi02.c          |  4 +--
 hw/core/qdev-properties-system.c | 22 ++++++++--------
 hw/scsi/esp-pci.c                | 28 ++++++++------------
 hw/scsi/lsi53c895a.c             | 13 +++-------
 hw/scsi/megasas.c                | 12 +++------
 hw/scsi/scsi-bus.c               |  6 ++---
 hw/usb/dev-storage.c             |  7 +++--
 include/hw/qdev-properties.h     |  4 +--
 scripts/kvm/kvm_stat             |  5 +++-
 scripts/kvm/kvm_stat.texi        | 55 ++++++++++++++++++++++++++++++++++++++++
 target-i386/seg_helper.c         |  4 +--
 util/qemu-thread-posix.c         |  6 +----
 util/rcu.c                       | 33 +++++++++++++++++++++++-
 19 files changed, 152 insertions(+), 75 deletions(-)
 create mode 100644 scripts/kvm/kvm_stat.texi
-- 
2.3.0

[Qemu-devel] [PULL 01/11] iscsi: Fix check for username

[Paolo Bonzini]

From: Stefan Weil <sw@weilnetz.de>

The variable user in struct iscsi_url is a character array, not a pointer.
Therefore its address will never be NULL.

clang reports this error:

block/iscsi.c:1329:20: warning:
 comparison of array 'iscsi_url->user' not equal to a null pointer
 is always true [-Wtautological-pointer-compare]

Reviewed-by: Peter Lieven <pl@kamp.de>
Acked-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Message-Id: <1425719670-5486-1-git-send-email-sw@weilnetz.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 1fa855a..3e34b1f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1326,7 +1326,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         goto out;
     }
 
-    if (iscsi_url->user != NULL) {
+    if (iscsi_url->user[0] != '\0') {
         ret = iscsi_set_initiator_username_pwd(iscsi, iscsi_url->user,
                                               iscsi_url->passwd);
         if (ret != 0) {
-- 
2.3.0

[Qemu-devel] [PULL 02/11] kvm_stat: add column headers to text UI

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

The curses user interface shows both the accumulated total and the
current event counts.  Add column headers so it's clear what the numbers
mean.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Ademar Reis <areis@redhat.com>
Reviewed-by: Wei Huang <wei@redhat.com>
Message-Id: <1425338947-10296-2-git-send-email-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/kvm/kvm_stat | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index c65cabd..7e5d256 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -519,7 +519,10 @@ def tui(screen, stats):
     def refresh(sleeptime):
         screen.erase()
         screen.addstr(0, 0, 'kvm statistics')
-        row = 2
+        screen.addstr(2, 1, 'Event')
+        screen.addstr(2, 1 + label_width + number_width - len('Total'), 'Total')
+        screen.addstr(2, 1 + label_width + number_width + 8 - len('Current'), 'Current')
+        row = 3
         s = stats.get()
         def sortkey(x):
             if s[x][1]:
-- 
2.3.0

[Qemu-devel] [PULL 03/11] kvm_stat: add kvm_stat.1 man page

[Paolo Bonzini]

From: Stefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Ademar Reis <areis@redhat.com>
Reviewed-by: Wei Huang <wei@redhat.com>
Message-Id: <1425338947-10296-3-git-send-email-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile                  |  9 ++++++++
 scripts/kvm/kvm_stat.texi | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 scripts/kvm/kvm_stat.texi

diff --git a/Makefile b/Makefile
index d92d4cd..884b59d 100644
--- a/Makefile
+++ b/Makefile
@@ -84,6 +84,9 @@ HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qmp-commands.txt
+ifdef CONFIG_LINUX
+DOCS+=kvm_stat.1
+endif
 ifdef CONFIG_VIRTFS
 DOCS+=fsdev/virtfs-proxy-helper.1
 endif
@@ -490,6 +493,12 @@ qemu-nbd.8: qemu-nbd.texi
 	  $(POD2MAN) --section=8 --center=" " --release=" " qemu-nbd.pod > $@, \
 	  "  GEN   $@")
 
+kvm_stat.1: scripts/kvm/kvm_stat.texi
+	$(call quiet-command, \
+	  perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< kvm_stat.pod && \
+	  $(POD2MAN) --section=1 --center=" " --release=" " kvm_stat.pod > $@, \
+	  "  GEN   $@")
+
 dvi: qemu-doc.dvi qemu-tech.dvi
 html: qemu-doc.html qemu-tech.html
 info: qemu-doc.info qemu-tech.info
diff --git a/scripts/kvm/kvm_stat.texi b/scripts/kvm/kvm_stat.texi
new file mode 100644
index 0000000..6ce00d8
--- /dev/null
+++ b/scripts/kvm/kvm_stat.texi
@@ -0,0 +1,55 @@
+@example
+@c man begin SYNOPSIS
+usage: kvm_stat [OPTION]...
+@c man end
+@end example
+
+@c man begin DESCRIPTION
+
+kvm_stat prints counts of KVM kernel module trace events.  These events signify
+state transitions such as guest mode entry and exit.
+
+This tool is useful for observing guest behavior from the host perspective.
+Often conclusions about performance or buggy behavior can be drawn from the
+output.
+
+The set of KVM kernel module trace events may be specific to the kernel version
+or architecture.  It is best to check the KVM kernel module source code for the
+meaning of events.
+
+Note that trace events are counted globally across all running guests.
+
+@c man end
+
+@c man begin OPTIONS
+@table @option
+@item -1, --once, --batch
+  run in batch mode for one second
+@item -l, --log
+  run in logging mode (like vmstat)
+@item -t, --tracepoints
+  retrieve statistics from tracepoints
+@item -d, --debugfs
+  retrieve statistics from debugfs
+@item -f, --fields=@var{fields}
+  fields to display (regex)
+@item -h, --help
+  show help message
+@end table
+
+@c man end
+
+@ignore
+
+@setfilename kvm_stat
+@settitle Report KVM kernel module event counters.
+
+@c man begin AUTHOR
+Stefan Hajnoczi <stefanha@redhat.com>
+@c man end
+
+@c man begin SEEALSO
+perf(1), trace-cmd(1)
+@c man end
+
+@end ignore
-- 
2.3.0

[Qemu-devel] [PULL 04/11] qemu-thread: do not use PTHREAD_MUTEX_ERRORCHECK

[Paolo Bonzini]

PTHREAD_MUTEX_ERRORCHECK is completely broken with respect to fork.
The way to safely do fork is to bring all threads to a quiescent
state by acquiring locks (either in callers---as we do for the
iothread mutex---or using pthread_atfork's prepare callbacks)
and then release them in the child.

The problem is that releasing error-checking locks in the child
fails under glibc with EPERM, because the mutex stores a different
owner tid than the duplicated thread in the child process.  We
could make it work for locks acquired via pthread_atfork, by
recreating the mutex in the child instead of unlocking it
(we know that there are no other threads that could have taken
the mutex; but when the lock is acquired in fork's caller
that would not be possible.

The simplest solution is just to forgo error checking.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/qemu-thread-posix.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 50a29d8..ba67cec 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -51,12 +51,8 @@ static void error_exit(int err, const char *msg)
 void qemu_mutex_init(QemuMutex *mutex)
 {
     int err;
-    pthread_mutexattr_t mutexattr;
 
-    pthread_mutexattr_init(&mutexattr);
-    pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_ERRORCHECK);
-    err = pthread_mutex_init(&mutex->lock, &mutexattr);
-    pthread_mutexattr_destroy(&mutexattr);
+    err = pthread_mutex_init(&mutex->lock, NULL);
     if (err)
         error_exit(err, __func__);
 }
-- 
2.3.0

[Qemu-devel] [PULL 05/11] rcu: handle forks safely

[Paolo Bonzini]

After forking, only the calling thread is duplicated in the child process.
The call_rcu thread has to be recreated in the child.  Exploit the fact
that only one thread exists (same as when constructors run), and just redo
the entire initialization to ensure the threads are in the proper state.

The only additional things to do are emptying the list of threads
registered with RCU, and unlocking the lock that was taken in the prepare
callback (implementations are allowed to fail pthread_mutex_init()
if the mutex is still locked).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/rcu.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/util/rcu.c b/util/rcu.c
index bd73b8e..27802a4 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -283,7 +283,7 @@ void rcu_unregister_thread(void)
     qemu_mutex_unlock(&rcu_gp_lock);
 }
 
-static void __attribute__((__constructor__)) rcu_init(void)
+static void rcu_init_complete(void)
 {
     QemuThread thread;
 
@@ -291,8 +291,39 @@ static void __attribute__((__constructor__)) rcu_init(void)
     qemu_event_init(&rcu_gp_event, true);
 
     qemu_event_init(&rcu_call_ready_event, false);
+
+    /* The caller is assumed to have iothread lock, so the call_rcu thread
+     * must have been quiescent even after forking, just recreate it.
+     */
     qemu_thread_create(&thread, "call_rcu", call_rcu_thread,
                        NULL, QEMU_THREAD_DETACHED);
 
     rcu_register_thread();
 }
+
+#ifdef CONFIG_POSIX
+static void rcu_init_lock(void)
+{
+    qemu_mutex_lock(&rcu_gp_lock);
+}
+
+static void rcu_init_unlock(void)
+{
+    qemu_mutex_unlock(&rcu_gp_lock);
+}
+
+static void rcu_init_child(void)
+{
+    qemu_mutex_unlock(&rcu_gp_lock);
+    memset(&registry, 0, sizeof(registry));
+    rcu_init_complete();
+}
+#endif
+
+static void __attribute__((__constructor__)) rcu_init(void)
+{
+#ifdef CONFIG_POSIX
+    pthread_atfork(rcu_init_lock, rcu_init_unlock, rcu_init_child);
+#endif
+    rcu_init_complete();
+}
-- 
2.3.0

[Qemu-devel] [PULL 06/11] cpus: initialize cpu->memory_dispatch

[Paolo Bonzini]

This fixes a NULL pointer dereference in s390x-softmmu.

On pretty much all other architectures, creating an MMIO region calls
cpu_reload_memory_map.  On s390, however, there are no MMIO regions
and everything is done via hypercalls.

Fixes: 9d82b5a792236db31a75b9db5c93af69ac07c7c5
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/exec.c b/exec.c
index 6a5adab..a6cb4a2 100644
--- a/exec.c
+++ b/exec.c
@@ -548,6 +548,7 @@ void cpu_exec_init(CPUArchState *env)
 #ifndef CONFIG_USER_ONLY
     cpu->as = &address_space_memory;
     cpu->thread_id = qemu_get_thread_id();
+    cpu_reload_memory_map(cpu);
 #endif
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
 #if defined(CONFIG_USER_ONLY)
-- 
2.3.0

[Qemu-devel] [PULL 07/11] scsi: Clean up duplicated error in legacy if=scsi code

[Paolo Bonzini]

From: Markus Armbruster <armbru@redhat.com>

Commit a818a4b changed scsi_bus_legacy_handle_cmdline() to report
errors from scsi_bus_legacy_add_drive() with error_report() in
addition to returning them.  That's inappropriate.

Two kinds of callers:

1. realize methods (devices "esp", "virtio-scsi-device" and
   "spapr-vscsi")

   The error object gets passed up the call chain until it gets
   reported again and freed.

   Example:

   $ qemu-system-arm -M virt -S -display none \
   > -drive if=scsi,id=foo,bus=1,file=tmp.qcow2 \
   > -device nec-usb-xhci -device usb-storage,drive=foo \
   > -device virtio-scsi-pci
   qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Property 'scsi-disk.drive' can't take value 'foo', it's in use
   qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Setting drive property failed
   qemu-system-arm: -device virtio-scsi-pci: Setting drive property failed
   qemu-system-arm: -device virtio-scsi-pci: Device initialization failed
   qemu-system-arm: -device virtio-scsi-pci: Device 'virtio-scsi-pci' could not be initialized

   The second message in this error cascade comes from
   scsi_bus_legacy_handle_cmdline().  The error object then gets
   passed up to the qdev_init() called from
   virtio_scsi_pci_init_pci(), which reports it again.

2. init methods (devices "am53c974", "dc390", "lsi53c895a",
   "lsi53c810", "megasas", "megasas-gen2")

   init methods need to report their errors with qerror_report().
   These don't.  The inappropriate error_report() papers over the bug.

   error_report() isn't the same as qerror_report() in QMP context,
   but this can't actually happen: QMP can still only hot-plug, and
   callers call scsi_bus_legacy_handle_cmdline() only on cold-plug.
   Except for sysbus_esp_realize(), but that can't be hot-plugged at
   all, as far as I can tell.

Fix the init methods and drop the inappropriate error_report() in
scsi_bus_legacy_handle_cmdline().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1425925048-15482-2-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/esp-pci.c    | 2 ++
 hw/scsi/lsi53c895a.c | 3 ++-
 hw/scsi/megasas.c    | 2 ++
 hw/scsi/scsi-bus.c   | 1 -
 4 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 00b7297..a75fcfa 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -28,6 +28,7 @@
 #include "hw/scsi/esp.h"
 #include "trace.h"
 #include "qemu/log.h"
+#include "qapi/qmp/qerror.h"
 
 #define TYPE_AM53C974_DEVICE "am53c974"
 
@@ -369,6 +370,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
     if (!d->hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus, &err);
         if (err != NULL) {
+            qerror_report_err(err);
             error_free(err);
             return -1;
         }
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index db7d4b8..4ffab03 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -19,7 +19,7 @@
 #include "hw/pci/pci.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/dma.h"
-#include "qemu/error-report.h"
+#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_LSI
 //#define DEBUG_LSI_REG
@@ -2119,6 +2119,7 @@ static int lsi_scsi_init(PCIDevice *dev)
     if (!d->hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus, &err);
         if (err != NULL) {
+            qerror_report_err(err);
             error_free(err);
             return -1;
         }
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 4852237..69ffdbd 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -28,6 +28,7 @@
 #include "hw/scsi/scsi.h"
 #include "block/scsi.h"
 #include "trace.h"
+#include "qapi/qmp/qerror.h"
 
 #include "mfi.h"
 
@@ -2409,6 +2410,7 @@ static int megasas_scsi_init(PCIDevice *dev)
     if (!d->hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus, &err);
         if (err != NULL) {
+            qerror_report_err(err);
             error_free(err);
             return -1;
         }
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index dca9576..f8de569 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -273,7 +273,6 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, Error **errp)
         scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo),
                                   unit, false, -1, NULL, &err);
         if (err != NULL) {
-            error_report("%s", error_get_pretty(err));
             error_propagate(errp, err);
             break;
         }
-- 
2.3.0

[Qemu-devel] [PULL 08/11] hw: Propagate errors through qdev_prop_set_drive()

[Paolo Bonzini]

From: Markus Armbruster <armbru@redhat.com>

Three kinds of callers:

1. On failure, report the error and abort

   Passing &error_abort does the job.  No functional change.

2. On failure, report the error and exit()

   This is qdev_prop_set_drive_nofail().  Error reporting moves from
   qdev_prop_set_drive() to its caller.  Because hiding away the error
   in the monitor right before exit() isn't helpful, replace
   qerror_report_err() by error_report_err().  Shouldn't make a
   difference, because qdev_prop_set_drive_nofail() should never be
   used in QMP context.

3. On failure, report the error and recover

   This is usb_msd_init() and scsi_bus_legacy_add_drive().  Error
   reporting and freeing the error object moves from
   qdev_prop_set_drive() to its callers.

   Because usb_msd_init() can't run in QMP context, replace
   qerror_report_err() by error_report_err() there.

   No functional change.

   scsi_bus_legacy_add_drive() calling qerror_report_err() is of
   course inappropriate, but this commit merely makes it more obvious.
   The next one will clean it up.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1425925048-15482-3-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/arm/vexpress.c                |  6 +++---
 hw/arm/virt.c                    |  6 +++---
 hw/block/pflash_cfi01.c          |  4 ++--
 hw/block/pflash_cfi02.c          |  4 ++--
 hw/core/qdev-properties-system.c | 22 ++++++++++------------
 hw/scsi/scsi-bus.c               |  6 +++++-
 hw/usb/dev-storage.c             |  7 +++++--
 include/hw/qdev-properties.h     |  4 ++--
 8 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 5933454..8496c16 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -515,9 +515,9 @@ static pflash_t *ve_pflash_cfi01_register(hwaddr base, const char *name,
 {
     DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
 
-    if (di && qdev_prop_set_drive(dev, "drive",
-                                  blk_by_legacy_dinfo(di))) {
-        abort();
+    if (di) {
+        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(di),
+                            &error_abort);
     }
 
     qdev_prop_set_uint32(dev, "num-blocks",
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 69f51ac..93b7605 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -522,9 +522,9 @@ static void create_one_flash(const char *name, hwaddr flashbase,
     DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
     const uint64_t sectorlength = 256 * 1024;
 
-    if (dinfo && qdev_prop_set_drive(dev, "drive",
-                                     blk_by_legacy_dinfo(dinfo))) {
-        abort();
+    if (dinfo) {
+        qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+                            &error_abort);
     }
 
     qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 89d380e..d282695 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -969,8 +969,8 @@ pflash_t *pflash_cfi01_register(hwaddr base,
 {
     DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
 
-    if (blk && qdev_prop_set_drive(dev, "drive", blk)) {
-        abort();
+    if (blk) {
+        qdev_prop_set_drive(dev, "drive", blk, &error_abort);
     }
     qdev_prop_set_uint32(dev, "num-blocks", nb_blocs);
     qdev_prop_set_uint64(dev, "sector-length", sector_len);
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 389b4aa..074a005 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -773,8 +773,8 @@ pflash_t *pflash_cfi02_register(hwaddr base,
 {
     DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH02);
 
-    if (blk && qdev_prop_set_drive(dev, "drive", blk)) {
-        abort();
+    if (blk) {
+        qdev_prop_set_drive(dev, "drive", blk, &error_abort);
     }
     qdev_prop_set_uint32(dev, "num-blocks", nb_blocs);
     qdev_prop_set_uint32(dev, "sector-length", sector_len);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index a2e44bd..c413226 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -341,27 +341,25 @@ PropertyInfo qdev_prop_vlan = {
     .set   = set_vlan,
 };
 
-int qdev_prop_set_drive(DeviceState *dev, const char *name,
-                        BlockBackend *value)
+void qdev_prop_set_drive(DeviceState *dev, const char *name,
+                         BlockBackend *value, Error **errp)
 {
-    Error *err = NULL;
-    object_property_set_str(OBJECT(dev),
-                            value ? blk_name(value) : "", name, &err);
-    if (err) {
-        qerror_report_err(err);
-        error_free(err);
-        return -1;
-    }
-    return 0;
+    object_property_set_str(OBJECT(dev), value ? blk_name(value) : "",
+                            name, errp);
 }
 
 void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
                                 BlockBackend *value)
 {
-    if (qdev_prop_set_drive(dev, name, value) < 0) {
+    Error *err = NULL;
+
+    qdev_prop_set_drive(dev, name, value, &err);
+    if (err) {
+        error_report_err(err);
         exit(1);
     }
 }
+
 void qdev_prop_set_chr(DeviceState *dev, const char *name,
                        CharDriverState *value)
 {
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f8de569..61c595f 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -7,6 +7,7 @@
 #include "sysemu/blockdev.h"
 #include "trace.h"
 #include "sysemu/dma.h"
+#include "qapi/qmp/qerror.h"
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
@@ -242,7 +243,10 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
     if (serial && object_property_find(OBJECT(dev), "serial", NULL)) {
         qdev_prop_set_string(dev, "serial", serial);
     }
-    if (qdev_prop_set_drive(dev, "drive", blk) < 0) {
+    qdev_prop_set_drive(dev, "drive", blk, &err);
+    if (err) {
+        qerror_report_err(err);
+        error_free(err);
         error_setg(errp, "Setting drive property failed");
         object_unparent(OBJECT(dev));
         return NULL;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 65d9aa6..3f5b32d 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -663,6 +663,7 @@ static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
 static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
 {
     static int nr=0;
+    Error *err = NULL;
     char id[8];
     QemuOpts *opts;
     DriveInfo *dinfo;
@@ -706,8 +707,10 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
 
     /* create guest device */
     dev = usb_create(bus, "usb-storage");
-    if (qdev_prop_set_drive(&dev->qdev, "drive",
-                            blk_by_legacy_dinfo(dinfo)) < 0) {
+    qdev_prop_set_drive(&dev->qdev, "drive", blk_by_legacy_dinfo(dinfo),
+                        &err);
+    if (err) {
+        error_report_err(err);
         object_unparent(OBJECT(dev));
         return NULL;
     }
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 57ee363..2d47c0c 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -168,8 +168,8 @@ void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
 void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value);
 void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value);
 void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value);
-int qdev_prop_set_drive(DeviceState *dev, const char *name,
-                        BlockBackend *value) QEMU_WARN_UNUSED_RESULT;
+void qdev_prop_set_drive(DeviceState *dev, const char *name,
+                         BlockBackend *value, Error **errp);
 void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
                                 BlockBackend *value);
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
-- 
2.3.0

[Qemu-devel] [PULL 09/11] scsi: Improve error reporting for invalid drive property

[Paolo Bonzini]

From: Markus Armbruster <armbru@redhat.com>

When setting "realized" fails, scsi_bus_legacy_add_drive() passes the
error to qerror_report_err(), then returns an unspecific "Setting
drive property failed" error, which is reported further up the call
chain.

Example:

    $ qemu-system-x86_64 -nodefaults -S -display none \
    > -drive if=scsi,id=foo,file=tmp.qcow2 -global isa-fdc.driveA=foo
    qemu-system-x86_64: -drive if=scsi,id=foo,file=tmp.qcow2: Property 'scsi-disk.drive' can't take value 'foo', it's in use
    qemu-system-x86_64: Setting drive property failed
    qemu-system-x86_64: Initialization of device lsi53c895a failed: Device initialization failed

Clean up the obvious way: simply return the original error to the
caller.  Gets rid of the second message in the above error cascade.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1425925048-15482-4-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 61c595f..bd2c0e4 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -7,7 +7,6 @@
 #include "sysemu/blockdev.h"
 #include "trace.h"
 #include "sysemu/dma.h"
-#include "qapi/qmp/qerror.h"
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
@@ -245,9 +244,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
     }
     qdev_prop_set_drive(dev, "drive", blk, &err);
     if (err) {
-        qerror_report_err(err);
-        error_free(err);
-        error_setg(errp, "Setting drive property failed");
+        error_propagate(errp, err);
         object_unparent(OBJECT(dev));
         return NULL;
     }
-- 
2.3.0

[Qemu-devel] [PULL 10/11] scsi: Convert remaining PCI HBAs to realize()

[Paolo Bonzini]

From: Markus Armbruster <armbru@redhat.com>

These are "am53c974", "dc390", "lsi53c895a", "lsi53c810", "megasas",
"megasas-gen2".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1425925048-15482-5-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/esp-pci.c    | 30 +++++++++++-------------------
 hw/scsi/lsi53c895a.c | 14 +++-----------
 hw/scsi/megasas.c    | 14 +++-----------
 3 files changed, 17 insertions(+), 41 deletions(-)

diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index a75fcfa..8d2242d 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -28,7 +28,6 @@
 #include "hw/scsi/esp.h"
 #include "trace.h"
 #include "qemu/log.h"
-#include "qapi/qmp/qerror.h"
 
 #define TYPE_AM53C974_DEVICE "am53c974"
 
@@ -343,13 +342,12 @@ static const struct SCSIBusInfo esp_pci_scsi_info = {
     .cancel = esp_request_cancelled,
 };
 
-static int esp_pci_scsi_init(PCIDevice *dev)
+static void esp_pci_scsi_realize(PCIDevice *dev, Error **errp)
 {
     PCIESPState *pci = PCI_ESP(dev);
     DeviceState *d = DEVICE(dev);
     ESPState *s = &pci->esp;
     uint8_t *pci_conf;
-    Error *err = NULL;
 
     pci_conf = dev->config;
 
@@ -368,14 +366,8 @@ static int esp_pci_scsi_init(PCIDevice *dev)
 
     scsi_bus_new(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info, NULL);
     if (!d->hotplugged) {
-        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
-        if (err != NULL) {
-            qerror_report_err(err);
-            error_free(err);
-            return -1;
-        }
+        scsi_bus_legacy_handle_cmdline(&s->bus, errp);
     }
-    return 0;
 }
 
 static void esp_pci_scsi_uninit(PCIDevice *d)
@@ -390,7 +382,7 @@ static void esp_pci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = esp_pci_scsi_init;
+    k->realize = esp_pci_scsi_realize;
     k->exit = esp_pci_scsi_uninit;
     k->vendor_id = PCI_VENDOR_ID_AMD;
     k->device_id = PCI_DEVICE_ID_AMD_SCSI;
@@ -468,17 +460,19 @@ static void dc390_write_config(PCIDevice *dev,
     }
 }
 
-static int dc390_scsi_init(PCIDevice *dev)
+static void dc390_scsi_realize(PCIDevice *dev, Error **errp)
 {
     DC390State *pci = DC390(dev);
+    Error *err = NULL;
     uint8_t *contents;
     uint16_t chksum = 0;
-    int i, ret;
+    int i;
 
     /* init base class */
-    ret = esp_pci_scsi_init(dev);
-    if (ret < 0) {
-        return ret;
+    esp_pci_scsi_realize(dev, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
     }
 
     /* EEPROM */
@@ -505,8 +499,6 @@ static int dc390_scsi_init(PCIDevice *dev)
     chksum = 0x1234 - chksum;
     contents[EE_CHKSUM1] = chksum & 0xff;
     contents[EE_CHKSUM2] = chksum >> 8;
-
-    return 0;
 }
 
 static void dc390_class_init(ObjectClass *klass, void *data)
@@ -514,7 +506,7 @@ static void dc390_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = dc390_scsi_init;
+    k->realize = dc390_scsi_realize;
     k->config_read = dc390_read_config;
     k->config_write = dc390_write_config;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 4ffab03..c5b0cc5 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -19,7 +19,6 @@
 #include "hw/pci/pci.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/dma.h"
-#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_LSI
 //#define DEBUG_LSI_REG
@@ -2089,12 +2088,11 @@ static const struct SCSIBusInfo lsi_scsi_info = {
     .cancel = lsi_request_cancelled
 };
 
-static int lsi_scsi_init(PCIDevice *dev)
+static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
 {
     LSIState *s = LSI53C895A(dev);
     DeviceState *d = DEVICE(dev);
     uint8_t *pci_conf;
-    Error *err = NULL;
 
     pci_conf = dev->config;
 
@@ -2117,14 +2115,8 @@ static int lsi_scsi_init(PCIDevice *dev)
 
     scsi_bus_new(&s->bus, sizeof(s->bus), d, &lsi_scsi_info, NULL);
     if (!d->hotplugged) {
-        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
-        if (err != NULL) {
-            qerror_report_err(err);
-            error_free(err);
-            return -1;
-        }
+        scsi_bus_legacy_handle_cmdline(&s->bus, errp);
     }
-    return 0;
 }
 
 static void lsi_class_init(ObjectClass *klass, void *data)
@@ -2132,7 +2124,7 @@ static void lsi_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = lsi_scsi_init;
+    k->realize = lsi_scsi_realize;
     k->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
     k->device_id = PCI_DEVICE_ID_LSI_53C895A;
     k->class_id = PCI_CLASS_STORAGE_SCSI;
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 69ffdbd..bf83b65 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -28,7 +28,6 @@
 #include "hw/scsi/scsi.h"
 #include "block/scsi.h"
 #include "trace.h"
-#include "qapi/qmp/qerror.h"
 
 #include "mfi.h"
 
@@ -2321,14 +2320,13 @@ static const struct SCSIBusInfo megasas_scsi_info = {
     .cancel = megasas_command_cancel,
 };
 
-static int megasas_scsi_init(PCIDevice *dev)
+static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
 {
     DeviceState *d = DEVICE(dev);
     MegasasState *s = MEGASAS(dev);
     MegasasBaseClass *b = MEGASAS_DEVICE_GET_CLASS(s);
     uint8_t *pci_conf;
     int i, bar_type;
-    Error *err = NULL;
 
     pci_conf = dev->config;
 
@@ -2408,14 +2406,8 @@ static int megasas_scsi_init(PCIDevice *dev)
     scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
                  &megasas_scsi_info, NULL);
     if (!d->hotplugged) {
-        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
-        if (err != NULL) {
-            qerror_report_err(err);
-            error_free(err);
-            return -1;
-        }
+        scsi_bus_legacy_handle_cmdline(&s->bus, errp);
     }
-    return 0;
 }
 
 static void
@@ -2509,7 +2501,7 @@ static void megasas_class_init(ObjectClass *oc, void *data)
     MegasasBaseClass *e = MEGASAS_DEVICE_CLASS(oc);
     const MegasasInfo *info = data;
 
-    pc->init = megasas_scsi_init;
+    pc->realize = megasas_scsi_realize;
     pc->exit = megasas_scsi_uninit;
     pc->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
     pc->device_id = info->device_id;
-- 
2.3.0

[Qemu-devel] [PULL 11/11] x86: fix SS selector in SYSRET

[Paolo Bonzini]

From: Bill Paul <wpaul@windriver.com>

According to my reading of the Intel documentation, the SYSRET instruction
is supposed to force the RPL bits of the %ss register to 3 when returning
to user mode. The actual sequence is:

SS.Selector <-- (IA32_STAR[63:48]+8) OR 3; (* RPL forced to 3 *)

However, the code in helper_sysret() leaves them at 0 (in other words, the "OR
3" part of the above sequence is missing). It does set the privilege level
bits of %cs correctly though.

This has caused me trouble with some of my VxWorks development: code that runs
okay on real hardware will crash on QEMU, unless I apply the patch below.

Signed-off-by: Bill Paul <wpaul@windriver.com>
Message-Id: <201503091548.01462.wpaul@windriver.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/seg_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index fa374d0..2bc757a 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -1043,7 +1043,7 @@ void helper_sysret(CPUX86State *env, int dflag)
                                    DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK);
             env->eip = (uint32_t)env->regs[R_ECX];
         }
-        cpu_x86_load_seg_cache(env, R_SS, selector + 8,
+        cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3,
                                0, 0xffffffff,
                                DESC_G_MASK | DESC_B_MASK | DESC_P_MASK |
                                DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
@@ -1056,7 +1056,7 @@ void helper_sysret(CPUX86State *env, int dflag)
                                DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
                                DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK);
         env->eip = (uint32_t)env->regs[R_ECX];
-        cpu_x86_load_seg_cache(env, R_SS, selector + 8,
+        cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3,
                                0, 0xffffffff,
                                DESC_G_MASK | DESC_B_MASK | DESC_P_MASK |
                                DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
-- 
2.3.0

Re: [Qemu-devel] [PULL 00/11] scsi, RCU, KVM, x86 changes for 2015-03-10

[Peter Maydell]

On 10 March 2015 at 10:37, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 277263e1b320d759a760ba6c5ea75ec268f929e5:
>
>   Merge remote-tracking branch 'remotes/agraf/tags/signed-ppc-for-upstream' into staging (2015-03-09 14:04:14 +0000)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to ac57622985220de064059971f9ccb00905e9bd04:
>
>   x86: fix SS selector in SYSRET (2015-03-10 11:18:24 +0100)
>
> ----------------------------------------------------------------
> - scsi: improvements to error reporting and conversion to realize,
>   Coverity/sparse fix for iscsi driver
> - RCU fallout: fix -daemonize and s390x system emulation
> - KVM: kvm_stat improvements and new man page
> - x86: SYSRET fix for VxWorks
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM