[Qemu-devel] [PULL 0/7] Trivial patches for 2017-08-14

[Qemu-devel] [PULL 0/7] Trivial patches for 2017-08-14

[Michael Tokarev]

There are just a few small thigs, I've omitted larger series for now.

The following changes since commit 9db6ffc76676731a25a5538ab71e8ca6ac234f80:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2017-08-11 15:11:50 +0100)

are available in the git repository at:

  git://git.corpit.ru/qemu.git tags/trivial-patches-fetch

for you to fetch changes up to a808c0865b720e22ca2929ec3d362d4610fbad51:

  hw/misc/mmio_interface: Return after error_setg() to avoid crash (2017-08-14 13:06:54 +0300)

----------------------------------------------------------------
trivial patches for 2017-08-14

----------------------------------------------------------------
Cleber Rosa (3):
      qemu-iotests: get rid of _full_imgproto_details()
      qemu-iotests: remove commented out variables
      qemu-iotests: remove comment about root privileges requirement

Eduardo Otubo (1):
      unicore32: abort when entering "x 0" on the monitor

Eric Blake (1):
      libqtest: Fix typo in comments

Thomas Huth (2):
      qemu-doc: Fix "-net van" typo
      hw/misc/mmio_interface: Return after error_setg() to avoid crash

 hw/misc/mmio_interface.c     |  2 ++
 qemu-doc.texi                |  2 +-
 target/unicore32/softmmu.c   |  8 ++++----
 tests/libqtest.h             |  8 ++++----
 tests/qemu-iotests/check     | 13 +------------
 tests/qemu-iotests/common.rc |  5 -----
 6 files changed, 12 insertions(+), 26 deletions(-)

[Qemu-devel] [PULL 1/7] unicore32: abort when entering "x 0" on the monitor

[Michael Tokarev]

From: Eduardo Otubo <otubo@redhat.com>

Starting Qemu with "qemu-system-unicore32 -M puv3,accel=qtest -S -nographic"
and entering "x 0 " at the monitor prompt leads to abort():

  $ ./unicore32-softmmu/qemu-system-unicore32 -M puv3,accel=qtest -S -nographic
  QEMU 2.9.90 monitor - type 'help' for more information
  (qemu) x 0
  qemu: fatal: uc32_cpu_get_phys_page_debug not supported yet

  R00=00000000 R01=00000000 R02=00000000 R03=00000000
  R04=00000000 R05=00000000 R06=00000000 R07=00000000
  R08=00000000 R09=00000000 R10=00000000 R11=00000000
  R12=00000000 R13=00000000 R14=00000000 R15=00000000
  R16=00000000 R17=00000000 R18=00000000 R19=00000000
  R20=00000000 R21=00000000 R22=00000000 R23=00000000
  R24=00000000 R25=00000000 R26=00000000 R27=00000000
  R28=00000000 R29=00000000 R30=00000000 R31=03000000
  PSR=40000013 -Z-- PRIV
  Aborted (core dumped)

This happens because uc32_cpu_get_phys_page_debug() is not implemented
yet, this is a temporary workaround to avoid the crash.

Signed-off-by: Eduardo Otubo <otubo@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 target/unicore32/softmmu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/unicore32/softmmu.c b/target/unicore32/softmmu.c
index e7152e72e0..d8d76968f3 100644
--- a/target/unicore32/softmmu.c
+++ b/target/unicore32/softmmu.c
@@ -15,6 +15,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
+#include "qemu/error-report.h"
 
 #undef DEBUG_UC32
 
@@ -271,8 +272,7 @@ int uc32_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
 
 hwaddr uc32_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
 {
-    UniCore32CPU *cpu = UNICORE32_CPU(cs);
-
-    cpu_abort(CPU(cpu), "%s not supported yet\n", __func__);
-    return addr;
+    error_report("function uc32_cpu_get_phys_page_debug not "
+                    "implemented, aborting");
+    return -1;
 }
-- 
2.11.0

[Qemu-devel] [PULL 2/7] libqtest: Fix typo in comments

[Michael Tokarev]

From: Eric Blake <eblake@redhat.com>

s/continuosly/continuously/

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 tests/libqtest.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/libqtest.h b/tests/libqtest.h
index 38bc1e9953..3ae570927a 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -117,7 +117,7 @@ QDict *qtest_qmp_receive(QTestState *s);
  * @s: #QTestState instance to operate on.
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  */
 void qtest_qmp_eventwait(QTestState *s, const char *event);
 
@@ -126,7 +126,7 @@ void qtest_qmp_eventwait(QTestState *s, const char *event);
  * @s: #QTestState instance to operate on.
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  * Returns a copy of the event for further investigation.
  */
 QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
@@ -571,7 +571,7 @@ static inline QDict *qmp_receive(void)
  * qmp_eventwait:
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  */
 static inline void qmp_eventwait(const char *event)
 {
@@ -582,7 +582,7 @@ static inline void qmp_eventwait(const char *event)
  * qmp_eventwait_ref:
  * @s: #event event to wait for.
  *
- * Continuosly polls for QMP responses until it receives the desired event.
+ * Continuously polls for QMP responses until it receives the desired event.
  * Returns a copy of the event for further investigation.
  */
 static inline QDict *qmp_eventwait_ref(const char *event)
-- 
2.11.0

[Qemu-devel] [PULL 3/7] qemu-doc: Fix "-net van" typo

[Michael Tokarev]

From: Thomas Huth <thuth@redhat.com>

While Andrew S. Tanenbaum has a point by saying "Never underestimate the
bandwidth of a station wagon full of tapes hurtling down the highway",
we don't support that way of transportation in QEMU yet, so replace the
typo with the correct word "vlan".

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 qemu-doc.texi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index aeb7bc52f5..4fc6b6f077 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -3243,7 +3243,7 @@ the ``-netdev user,guestfwd=ARGS'' argument instead.
 
 @subsection -net vlan (since 2.9.0)
 
-The ``-net van=NN'' argument is partially replaced with the
+The ``-net vlan=NN'' argument is partially replaced with the
 new ``-netdev'' argument. The remaining use cases will no
 longer be directly supported in QEMU.
 
-- 
2.11.0

[Qemu-devel] [PULL 4/7] qemu-iotests: get rid of _full_imgproto_details()

[Michael Tokarev]

From: Cleber Rosa <crosa@redhat.com>

Although this function is used, its implementation does nothing
besides echoing a variable name.  There's no need to wrap this
functionality in a function, and based on the one usage it has, it's
not even required to adhere to a convention or code style.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 tests/qemu-iotests/check     | 3 +--
 tests/qemu-iotests/common.rc | 5 -----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 2a55ec9ada..7a2e0d0119 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -228,7 +228,6 @@ rm -f check.full
 [ -f $TIMESTAMP_FILE ] || touch $TIMESTAMP_FILE
 
 FULL_IMGFMT_DETAILS=`_full_imgfmt_details`
-FULL_IMGPROTO_DETAILS=`_full_imgproto_details`
 FULL_HOST_DETAILS=`_full_platform_details`
 #FULL_MKFS_OPTIONS=`_scratch_mkfs_options`
 #FULL_MOUNT_OPTIONS=`_scratch_mount_options`
@@ -239,7 +238,7 @@ QEMU_IMG      -- "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS
 QEMU_IO       -- "$QEMU_IO_PROG" $QEMU_IO_OPTIONS
 QEMU_NBD      -- "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS
 IMGFMT        -- $FULL_IMGFMT_DETAILS
-IMGPROTO      -- $FULL_IMGPROTO_DETAILS
+IMGPROTO      -- $IMGPROTO
 PLATFORM      -- $FULL_HOST_DETAILS
 TEST_DIR      -- $TEST_DIR
 SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index bfbc80e5f6..8d486dbeb4 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -482,11 +482,6 @@ _full_imgfmt_details()
     fi
 }
 
-_full_imgproto_details()
-{
-    echo "$IMGPROTO"
-}
-
 _full_platform_details()
 {
     os=`uname -s`
-- 
2.11.0

[Qemu-devel] [PULL 5/7] qemu-iotests: remove commented out variables

[Michael Tokarev]

From: Cleber Rosa <crosa@redhat.com>

The variables FULL_MKFS_OPTIONS and FULL_MOUNT_OPTIONS are commented
out, never used, and even refer to functions that do exist.  The last
time these were touched was around 8 years ago, so I guess it's safe
to assume outputting such information on test execution is still on the
radar.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 tests/qemu-iotests/check | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 7a2e0d0119..437ef65320 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -229,8 +229,6 @@ rm -f check.full
 
 FULL_IMGFMT_DETAILS=`_full_imgfmt_details`
 FULL_HOST_DETAILS=`_full_platform_details`
-#FULL_MKFS_OPTIONS=`_scratch_mkfs_options`
-#FULL_MOUNT_OPTIONS=`_scratch_mount_options`
 
 cat <<EOF
 QEMU          -- "$QEMU_PROG" $QEMU_OPTIONS
@@ -244,8 +242,6 @@ TEST_DIR      -- $TEST_DIR
 SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
 
 EOF
-#MKFS_OPTIONS  -- $FULL_MKFS_OPTIONS
-#MOUNT_OPTIONS -- $FULL_MOUNT_OPTIONS
 
 seq="check"
 
-- 
2.11.0

[Qemu-devel] [PULL 6/7] qemu-iotests: remove comment about root privileges requirement

[Michael Tokarev]

From: Cleber Rosa <crosa@redhat.com>

The check script contains a commented out root user requirement,
probably because of its xfstests heritage.  This requirement doesn't
apply to qemu-iotests, so it better be gone.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 tests/qemu-iotests/check | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 437ef65320..d504b6e455 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -123,12 +123,6 @@ fi
 # we need common
 . "$source_iotests/common"
 
-#if [ `id -u` -ne 0 ]
-#then
-#    echo "check: QA must be run as root"
-#    exit 1
-#fi
-
 TIMESTAMP_FILE=check.time-$IMGPROTO-$IMGFMT
 
 tmp="${TEST_DIR}"/$$
-- 
2.11.0

[Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

[Michael Tokarev]

From: Thomas Huth <thuth@redhat.com>

QEMU currently abort()s if the user tries to specify the mmio_interface
device without parameters:

x86_64-softmmu/qemu-system-x86_64 -nographic -device mmio_interface
qemu-system-x86_64: /home/thuth/devel/qemu/util/error.c:57: error_setv:
 Assertion `*errp == ((void *)0)' failed.
Aborted (core dumped)

This happens because the realize function is trying to set the errp
twice in this case. After setting an error, the realize function
should immediately return instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 hw/misc/mmio_interface.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c
index 6f004d2bab..da154e5c95 100644
--- a/hw/misc/mmio_interface.c
+++ b/hw/misc/mmio_interface.c
@@ -63,10 +63,12 @@ static void mmio_interface_realize(DeviceState *dev, Error **errp)
 
     if (!s->host_ptr) {
         error_setg(errp, "host_ptr property must be set");
+        return;
     }
 
     if (!s->subregion) {
         error_setg(errp, "subregion property must be set");
+        return;
     }
 
     memory_region_init_ram_ptr(&s->ram_mem, OBJECT(s), "ram",
-- 
2.11.0

Re: [Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

[Peter Maydell]

On 14 August 2017 at 12:07, Michael Tokarev <mjt@tls.msk.ru> wrote:
> From: Thomas Huth <thuth@redhat.com>
>
> QEMU currently abort()s if the user tries to specify the mmio_interface
> device without parameters:
>
> x86_64-softmmu/qemu-system-x86_64 -nographic -device mmio_interface
> qemu-system-x86_64: /home/thuth/devel/qemu/util/error.c:57: error_setv:
>  Assertion `*errp == ((void *)0)' failed.
> Aborted (core dumped)
>
> This happens because the realize function is trying to set the errp
> twice in this case. After setting an error, the realize function
> should immediately return instead.

It seems like it should be an error to permit this to be
created from the command line at all -- the device is intended
only as an internal implementation detail of the memory system,
and it has a PROP_PTR property which can't be sensibly set
from the command line.

This patch is a correct fix for an immediate problem, but we should disable
using this via -device somehow.

thanks
-- PMM

Re: [Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

[Thomas Huth]

On 14.08.2017 13:45, Peter Maydell wrote:
> On 14 August 2017 at 12:07, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> QEMU currently abort()s if the user tries to specify the mmio_interface
>> device without parameters:
>>
>> x86_64-softmmu/qemu-system-x86_64 -nographic -device mmio_interface
>> qemu-system-x86_64: /home/thuth/devel/qemu/util/error.c:57: error_setv:
>>  Assertion `*errp == ((void *)0)' failed.
>> Aborted (core dumped)
>>
>> This happens because the realize function is trying to set the errp
>> twice in this case. After setting an error, the realize function
>> should immediately return instead.
> 
> It seems like it should be an error to permit this to be
> created from the command line at all
That's also what thought first ... but the commit message of commit
7cc2298c46a6afa4f4ff7e5cd262809c782d701b says that it "can be
hotplugged/hotunplugged" ? ... that's confusing ...

Frederic, I think some more comments in the header of the file would be
really useful here.

 Thomas

Re: [Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

[Peter Maydell]

On 14 August 2017 at 12:52, Thomas Huth <thuth@redhat.com> wrote:
> On 14.08.2017 13:45, Peter Maydell wrote:
>> It seems like it should be an error to permit this to be
>> created from the command line at all

> That's also what thought first ... but the commit message of commit
> 7cc2298c46a6afa4f4ff7e5cd262809c782d701b says that it "can be
> hotplugged/hotunplugged" ? ... that's confusing ...

It means that memory.c creates and deletes instances of the
device on demand, not that it's hotplugged in the "controlled by
the user simulating PCI or other hotplug" sense.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] Trivial patches for 2017-08-14

[Peter Maydell]

On 14 August 2017 at 12:07, Michael Tokarev <mjt@tls.msk.ru> wrote:
> There are just a few small thigs, I've omitted larger series for now.
>
> The following changes since commit 9db6ffc76676731a25a5538ab71e8ca6ac234f80:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2017-08-11 15:11:50 +0100)
>
> are available in the git repository at:
>
>   git://git.corpit.ru/qemu.git tags/trivial-patches-fetch
>
> for you to fetch changes up to a808c0865b720e22ca2929ec3d362d4610fbad51:
>
>   hw/misc/mmio_interface: Return after error_setg() to avoid crash (2017-08-14 13:06:54 +0300)
>
> ----------------------------------------------------------------
> trivial patches for 2017-08-14
>
> ----------------------------------------------------------------
> Cleber Rosa (3):
>       qemu-iotests: get rid of _full_imgproto_details()
>       qemu-iotests: remove commented out variables
>       qemu-iotests: remove comment about root privileges requirement
>
> Eduardo Otubo (1):
>       unicore32: abort when entering "x 0" on the monitor
>
> Eric Blake (1):
>       libqtest: Fix typo in comments
>
> Thomas Huth (2):
>       qemu-doc: Fix "-net van" typo
>       hw/misc/mmio_interface: Return after error_setg() to avoid crash

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

[Igor Mammedov]

On Mon, 14 Aug 2017 12:45:16 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 14 August 2017 at 12:07, Michael Tokarev <mjt@tls.msk.ru> wrote:
> > From: Thomas Huth <thuth@redhat.com>
> >
> > QEMU currently abort()s if the user tries to specify the mmio_interface
> > device without parameters:
> >
> > x86_64-softmmu/qemu-system-x86_64 -nographic -device mmio_interface
> > qemu-system-x86_64: /home/thuth/devel/qemu/util/error.c:57: error_setv:
> >  Assertion `*errp == ((void *)0)' failed.
> > Aborted (core dumped)
> >
> > This happens because the realize function is trying to set the errp
> > twice in this case. After setting an error, the realize function
> > should immediately return instead.  
> 
> It seems like it should be an error to permit this to be
> created from the command line at all -- the device is intended
> only as an internal implementation detail of the memory system,
> and it has a PROP_PTR property which can't be sensibly set
> from the command line.
> 
> This patch is a correct fix for an immediate problem, but we should disable
> using this via -device somehow.

Setting DeviceClass::user_creatable to false should prevent creating device
with device_add interface. See: qdev_get_device_class()

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

[KONRAD Frederic]

On 08/14/2017 01:55 PM, Peter Maydell wrote:
> On 14 August 2017 at 12:52, Thomas Huth <thuth@redhat.com> wrote:
>> On 14.08.2017 13:45, Peter Maydell wrote:
>>> It seems like it should be an error to permit this to be
>>> created from the command line at all
> 
>> That's also what thought first ... but the commit message of commit
>> 7cc2298c46a6afa4f4ff7e5cd262809c782d701b says that it "can be
>> hotplugged/hotunplugged" ? ... that's confusing ...
> 
> It means that memory.c creates and deletes instances of the
> device on demand, not that it's hotplugged in the "controlled by
> the user simulating PCI or other hotplug" sense.
> 
> thanks
> -- PMM
> 

Sorry, missed that I changed my email address recently.
I'll add some docs and fix the things we mentioned for 2.11.

For the context:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg446748.html

Thanks,
Fred