Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Peter Xu]

On Tue, Aug 22, 2017 at 12:15:19PM +0800, Fam Zheng wrote:
> On Tue, 08/22 10:56, Peter Xu wrote:
> > I haven't really encountered (c), but I think it's the migrate_cancel
> > command that matters, which should not need BQL as well.
> 
> There is bdrv_invalidate_cache_all() in migrate_cancel which clearly isn't 
> safe.
> Is that if block unreachable in this case? If so we should assert, otherwise
> this command is not okay to run without BQL.

Ah. I see.  Even if so, if that is the only usage of BQL, IMHO we can
still mark migrate_cancel as "without-bql=true", instead we take the
BQL before calling bdrv_invalidate_cache_all().  Then migrate_cancel
can be BQL-free at least when block migration is not active.

> 
> Generically, what guarantee the thread-safety of a qmp command when you decide
> BQL is not needed? In other words, how do you prove commands are safe without
> BQL? I think almost every command accesses global state, but lock-free data
> structures are rare AFAICT.

I would suggest we split the problem into at least three parts.  IMHO
we need to answer below questions one by one to know what we should do
next:

1. whether we can handle monitor commands outside iothread, or say, in
   an isolated thread?

   This is basically what patch 2 does, the "per-monitor threads".

   IMHO this is the very first question to ask.  So now I know that at
   least current code cannot do it.  We need to at least do something
   to remove/replace the assertion to make this happen.  Can we?  I
   don't really know the answer yet.  If this is undoable, we can skip
   question 2/3 below and may need to rethink on how to solve the
   problem that postcopy recovery encounters.

2. whether there is any monitor commands can run without BQL?

   This is basically what patch 3/5 does, one for QMP, one for HMP.

   If we can settle question 1, then we can possibly start consider
   this question.  This step does not really allow any command to run
   without BQL, but we need to know whether it's possible in general,
   and if possible, we provide a framework to allow QMP/HMP developers
   to specify that.  If you see patch 3/5, the default behavior is
   still taking the BQL for all commands.

   IMHO doing this whole thing is generally good in the sense that
   this is actually forcing ourselves to break the BQL into smaller
   locks.  Take the migration commands for example: migrate_incoming
   do not need BQL, and when we write codes around it we know that we
   don't need to think about thread-safety.  That's not good IMHO.  I
   think it's time we should start consider thread-safety always.
   Again, for migrate_incoming to do this, actually we'll possibly at
   least need a migration management lock (the smaller lock) to make
   sure e.g. the user is not running two migrate_incoming commands in
   parallel (after per-monitor threads, it can happen).  But it's
   better than BQL, because BQL is for sure too big, so even a guest
   page access (as long as it held the BQL) can block migration
   commands.

3. which monitor commands can be run without BQL?

   This is what patch 4/6 was doing.  It tries to move
   migrate_incoming command out as the first candidate BQL-free
   command.

   Yes it's hard to say which command can be run without BQL.  So we
   need to investigate, possibly modify existing codes to make sure
   it's thread-safe, prove validity, then we can add the new ones into
   the BQL-free list.

   If after evaluating the pros and cons, we found that one command
   can be put into BQL-free but not worth the time for working on it,
   we can also keep those commands under BQL.

I assume question 3 is the one you were asking, and I'd say we may
need to solve question 1/2 first.  If we are done with 1/2, we just
need to spend time on each command to prove whether it is doable to
let that command run without BQL, and whether it worths itself to move
the command out of BQL.  Then we decide.  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] Fwd: QEMU-KVM Source Code Installation.

[Fam Zheng]

On Tue, 08/22 10:39, Tushar Bhardwaj wrote:
> Respected Sir,
> 
> I studided many articles for QEMU-KVM source code installation which are
> confusing. Please guide me how to install QEMU-KVM with source code on
> UBUNTU linux distribution.

Please don't repeat what has already been answered:

https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg03334.html

Fam

Re: [Qemu-devel] [PATCH for-2.10] boot-serial-test: prefer tcg accelerator

[Thomas Huth]

On 22.08.2017 03:09, David Gibson wrote:
> On Wed, Aug 16, 2017 at 12:18:07PM +0100, Peter Maydell wrote:
>> On 16 August 2017 at 11:51, Paolo Bonzini  wrote:
>>> On 16/08/2017 10:26, Cornelia Huck wrote:
 Prefer to use the tcg accelarator if it is available: This is our only
 real smoke test for tcg, and fast enough to use it for that.
>>>
>>> I'm not sure this is required for 2.10.  Yes, it means the coverage from
>>> "make check" is worse, but that's it.
>>
>> Yes, I'd put it under "if we need to roll an rc4 anyway for
>> some more significant bug we might as well put this in too,
>> but it doesn't merit cutting rc4 by itself."
> 
> It does entirely break "make check" on a ppc host.  And that in turn
> has held up my testing cycle for a couple of ppc regressions from 2.9
> that I was hoping to squeeze in.  Does that change your calculations?

Uuh, right. The problem is that we can only run the "pseries" machine
with kvm-hv, but not the other ppc machines (and the boot-serial test
tries to start a couple of these). So the "accel=tcg:kvm" fix should
currently be the best way to ease the situation - since ppc does not
support "--disable-tcg" yet, it will always be tested with tcg.

But when we support "--disable-tcg" one day on ppc, too, we might have
to come up with a more clever solution here instead...

 Thomas




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] Fwd: QEMU-KVM Source Code Installation.

[Tushar Bhardwaj]

Respected Sir,

I studided many articles for QEMU-KVM source code installation which are
confusing. Please guide me how to install QEMU-KVM with source code on
UBUNTU linux distribution.

-- 

"Purity and Simplicity Reflects the Inner Beauty of soul"




*Thank you With Regards:Tushar Sharma,*


*IIT Kharagpur*





-- 

"Purity and Simplicity Reflects the Inner Beauty of soul"




*Thank you With Regards:Tushar Sharma,*


*IIT Kharagpur*

Re: [Qemu-devel] [PATCH] qcow2: allocate cluster_cache/cluster_data on demand

[Alexey Kardashevskiy]

On 19/08/17 12:46, Alexey Kardashevskiy wrote:
> On 19/08/17 01:18, Eric Blake wrote:
>> On 08/18/2017 08:31 AM, Stefan Hajnoczi wrote:
>>> Most qcow2 files are uncompressed so it is wasteful to allocate (32 + 1)
>>> * cluster_size + 512 bytes upfront.  Allocate s->cluster_cache and
>>> s->cluster_data when the first read operation is performance on a
>>> compressed cluster.
>>>
>>> The buffers are freed in .bdrv_close().  .bdrv_open() no longer has any
>>> code paths that can allocate these buffers, so remove the free functions
>>> in the error code path.
>>>
>>> Reported-by: Alexey Kardashevskiy 
>>> Cc: Kevin Wolf 
>>> Signed-off-by: Stefan Hajnoczi 
>>> ---
>>> Alexey: Does this improve your memory profiling results?
>>
>> Is this a regression from earlier versions? 
> 
> Hm, I have not thought about this.
> 
> So. I did bisect and this started happening from
> 9a4c0e220d8a4f82b5665d0ee95ef94d8e1509d5
> "hw/virtio-pci: fix virtio behaviour"
> 
> Before that, the very same command line would take less than 1GB of
> resident memory. That thing basically enforces virtio-1.0 for QEMU <=2.6
> which means that upstream with "-machine pseries-2.6" works fine (less than
> 1GB), "-machine pseries-2.7" does not (close to 7GB, sometime even 9GB).
> 
> Then I tried bisecting again, with
> "scsi=off,disable-modern=off,disable-legacy=on" on my 150 virtio-block
> devices, started from
> e266d421490e0 "virtio-pci: add flags to enable/disable legacy/modern" (it
> added the disable-modern switch) which uses 2GB of memory.
> 
> I ended up with ada434cd0b44 "virtio-pci: implement cfg capability".
> 
> Then I removed proxy->modern_as on v2.10.0-rc3 (see below) and got 1.5GB of
> used memory (yay!)
> 
> I do not really know how to reinterpret all of this, do you?


Anyone, ping? Should I move the conversation to the original thread? Any
hacks to try with libc?



> 
> 
> Note: 1GB..9GB numbers from below are the peak values from valgrind's
> massif. This is pretty much resident memory used by QEMU process. In my
> testing I did not enable KVM and I did not start the guest (i.e. used -S).
> 150 virtio-block devices, 2GB RAM for the guest.
> 
> Also, while bisecting, I only paid attention if it is 1..2GB or 6..9GB -
> all tests did fit these 2 ranges, for any given sha1 the amount of memory
> would be stable but among "good" commits it could change between 1GB and 2GB.
> 
> 
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 5d14bd6..7ad447a 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1783,6 +1783,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
> Error **errp)
> /* PCI BAR regions must be powers of 2 */
> pow2ceil(proxy->notify.offset + proxy->notify.size));
> 
> +#if 0
>  memory_region_init_alias(>modern_cfg,
>   OBJECT(proxy),
>   "virtio-pci-cfg",
> @@ -1791,7 +1792,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
> Error **errp)
>   memory_region_size(>modern_bar));
> 
>  address_space_init(>modern_as, >modern_cfg,
> "virtio-pci-cfg-as");
> -
> +#endif
>  if (proxy->disable_legacy == ON_OFF_AUTO_AUTO) {
>  proxy->disable_legacy = pcie_port ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
>  }
> @@ -1860,10 +1861,10 @@ static void virtio_pci_realize(PCIDevice *pci_dev,
> Error **errp)
> 
>  static void virtio_pci_exit(PCIDevice *pci_dev)
>  {
> -VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
> +//VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
> 
>  msix_uninit_exclusive_bar(pci_dev);
> -address_space_destroy(>modern_as);
> +//address_space_destroy(>modern_as);
>  }
> 
>  static void virtio_pci_reset(DeviceState *qdev)
> 
> 
> 
> 
> 
>> Likely, it is NOT -rc4
>> material, and thus can wait for 2.11; but it should be fine for -stable
>> as part of 2.10.1 down the road.
>>
>>> +++ b/block/qcow2-cluster.c
>>> @@ -1516,6 +1516,23 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
>>> uint64_t cluster_offset)
>>>  nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) 
>>> + 1;
>>>  sector_offset = coffset & 511;
>>>  csize = nb_csectors * 512 - sector_offset;
>>> +
>>> +/* Allocate buffers on first decompress operation, most images are
>>> + * uncompressed and the memory overhead can be avoided.  The 
>>> buffers
>>> + * are freed in .bdrv_close().
>>> + */
>>> +if (!s->cluster_data) {
>>> +/* one more sector for decompressed data alignment */
>>> +s->cluster_data = qemu_try_blockalign(bs->file->bs,
>>> +QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
>>> +if (!s->cluster_data) {
>>> +return -EIO;
>>
>> Is -ENOMEM any better than -EIO here?
>>
> 
> 


-- 
Alexey



signature.asc
Description: 

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[no-reply]

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 1503301464-27886-1-git-send-email-pet...@redhat.com
Subject: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/20170822042416.26758-1-da...@gibson.dropbear.id.au -> 
patchew/20170822042416.26758-1-da...@gibson.dropbear.id.au
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
Switched to a new branch 'test'
6c48e66427 migration: hmp: migrate_incoming don't need BQL
1ffc968231 hmp: support "without_bql"
42b3b3a711 migration: qmp: migrate_incoming don't need BQL
afbb938df1 QAPI: new QMP command option "without-bql"
047188cbc7 monitor: allow monitor to create thread to poll
a89bb1619b monitor: move skip_flush into monitor_data_init

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-vbkqrhlc/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-vbkqrhlc/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
git-1.7.1-8.el6.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.3-15.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc git glib2-devel libepoxy-devel libfdt-devel 
librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel spice-glib-devel 
spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=b29af1b43246
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=01;05;37;41:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.tbz=01;31:*.tbz2=01;31:*.bz=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=01;36:*.au=01;36:*.flac=01;36:*.mid=01;36:*.midi=01;36:*.mka=01;36:*.mp3=01;36:*.mpc=01;36:*.ogg=01;36:*.ra=01;36:*.wav=01;36:*.axa=01;36:*.oga=01;36:*.spx=01;36:*.xspf=01;36:
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin

[Qemu-devel] [PATCH v3 09/10] MAINTAINERS: Add tests/vm entry

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ccee28b12d..0ed607d003 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1881,6 +1881,7 @@ S: Maintained
 F: .travis.yml
 F: .shippable.yml
 F: tests/docker/
+F: tests/vm/
 W: https://travis-ci.org/qemu/qemu
 W: https://app.shippable.com/github/qemu/qemu
 W: http://patchew.org/QEMU/
-- 
2.13.5

[Qemu-devel] [PATCH v3 05/10] tests: Add FreeBSD image

[Fam Zheng]

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng 
---
 tests/vm/freebsd | 45 +
 1 file changed, 45 insertions(+)
 create mode 100755 tests/vm/freebsd

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
new file mode 100755
index 00..0e4eb037d7
--- /dev/null
+++ b/tests/vm/freebsd
@@ -0,0 +1,45 @@
+#!/usr/bin/env python
+#
+# FreeBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import subprocess
+import tempfile
+import time
+import basevm
+
+class FreeBSDVM(basevm.BaseVM):
+name = "freebsd"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d /var/tmp/qemu-test.XX);
+tar -xf /dev/vtbd1;
+./configure {configure_opts};
+gmake -j{jobs};
+gmake check;
+"""
+
+def build_image(self, img, rebuild=False):
+if os.path.exists(img) and not rebuild:
+return
+cimg = 
self._download_with_cache("http://download.patchew.org/freebsd.img.xz;,
+
sha256sum='adcb771549b37bc63826c501f05121a206ed3d9f55f49145908f7e1432d65891')
+img_tmp_xz = img + ".tmp.xz"
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+subprocess.check_call(["xz", "-df", img_tmp_xz])
+os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+sys.exit(basevm.main(FreeBSDVM))
-- 
2.13.5

[Qemu-devel] [PATCH v3 08/10] Makefile: Add rules to run vm tests

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 Makefile  |  2 ++
 configure |  2 +-
 tests/vm/Makefile.include | 40 
 3 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 tests/vm/Makefile.include

diff --git a/Makefile b/Makefile
index 81447b1f08..2798a5ca69 100644
--- a/Makefile
+++ b/Makefile
@@ -813,6 +813,7 @@ endif
 -include $(wildcard *.d tests/*.d)
 
 include $(SRC_PATH)/tests/docker/Makefile.include
+include $(SRC_PATH)/tests/vm/Makefile.include
 
 .PHONY: help
 help:
@@ -836,6 +837,7 @@ help:
@echo  'Test targets:'
@echo  '  check   - Run all tests (check-help for details)'
@echo  '  docker  - Help about targets running tests inside 
Docker containers'
+   @echo  '  vm-test - Help about targets running tests inside VM'
@echo  ''
@echo  'Documentation targets:'
@echo  '  html info pdf txt'
diff --git a/configure b/configure
index dd73cce62f..9a3052e9ad 100755
--- a/configure
+++ b/configure
@@ -6544,7 +6544,7 @@ if test "$ccache_cpp2" = "yes"; then
 fi
 
 # build tree in object directory in case the source is not in the current 
directory
-DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
+DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
 DIRS="$DIRS docs docs/interop fsdev"
 DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
 DIRS="$DIRS roms/seabios roms/vgabios"
diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
new file mode 100644
index 00..6e133ae2a6
--- /dev/null
+++ b/tests/vm/Makefile.include
@@ -0,0 +1,40 @@
+# Makefile for VM tests
+
+.PHONY: vm-build-all
+
+IMAGES := ubuntu.i386 freebsd netbsd openbsd
+IMAGE_FILES := $(patsubst %, tests/vm/%.img, $(IMAGES))
+
+.PRECIOUS: $(IMAGE_FILES)
+
+vm-test:
+   @echo "vm-test: Test QEMU in preconfigured virtual machines"
+   @echo
+   @echo "  vm-build-ubuntu.i386- Build QEMU in ubuntu i386 VM"
+   @echo "  vm-build-freebsd- Build QEMU in FreeBSD VM"
+   @echo "  vm-build-netbsd - Build QEMU in NetBSD VM"
+   @echo "  vm-build-freebsd- Build QEMU in OpenBSD VM"
+
+vm-build-all: $(addprefix vm-build-, $(IMAGES))
+
+tests/vm/%.img: $(SRC_PATH)/tests/vm/%
+   $(call quiet-command, \
+   $(SRC_PATH)/tests/vm/$* \
+   $(if $(V)$(DEBUG), --debug) \
+   --image "$@" \
+   --force \
+   --build-image $@, \
+   "  VM-IMAGE $*")
+
+
+# Build in VM $(IMAGE)
+vm-build-%: tests/vm/%.img
+   $(call quiet-command, \
+   $(SRC_PATH)/tests/vm/$* \
+   $(if $(V)$(DEBUG), --debug) \
+   $(if $(DEBUG), --interactive) \
+   $(if $(J),--jobs $(J)) \
+   --image "$<" \
+   --build-qemu $(SRC_PATH), \
+   "  VM-BUILD $*")
+
-- 
2.13.5

[Qemu-devel] [PATCH v3 07/10] tests: Add OpenBSD image

[Fam Zheng]

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng 
---
 tests/vm/openbsd | 46 ++
 1 file changed, 46 insertions(+)
 create mode 100755 tests/vm/openbsd

diff --git a/tests/vm/openbsd b/tests/vm/openbsd
new file mode 100755
index 00..b308aa252b
--- /dev/null
+++ b/tests/vm/openbsd
@@ -0,0 +1,46 @@
+#!/usr/bin/env python
+#
+# OpenBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import subprocess
+import tempfile
+import time
+import basevm
+
+class OpenBSDVM(basevm.BaseVM):
+name = "openbsd"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d /var/tmp/qemu-test.XX);
+tar -xf /dev/rsd1c;
+./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4 
--python=python2.7 {configure_opts};
+gmake -j{jobs};
+# XXX: "gmake check" seems to always hang or fail
+#gmake check;
+"""
+
+def build_image(self, img, rebuild=False):
+if os.path.exists(img) and not rebuild:
+return
+cimg = 
self._download_with_cache("http://download.patchew.org/openbsd.img.xz;,
+
sha256sum='8c6cedc483e602cfee5e04f0406c64eb99138495e8ca580bc0293bcf0640c1bf')
+img_tmp_xz = img + ".tmp.xz"
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+subprocess.check_call(["xz", "-df", img_tmp_xz])
+os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+sys.exit(basevm.main(OpenBSDVM))
-- 
2.13.5

[Qemu-devel] [PATCH v3 10/10] tests: Add README for vm tests

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 tests/vm/README | 63 +
 1 file changed, 63 insertions(+)
 create mode 100644 tests/vm/README

diff --git a/tests/vm/README b/tests/vm/README
new file mode 100644
index 00..a18b285507
--- /dev/null
+++ b/tests/vm/README
@@ -0,0 +1,63 @@
+=== VM test suite to run build in guests ===
+
+== Intro ==
+
+This test suite contains scripts that bootstrap various guest images that has
+necessary packages to build QEMU. The basic usage is documented in Makefile
+help which is displayed with "make vm-test".
+
+== Quick start ==
+
+Run "make vm-test" to list available make targets.
+
+== Manual invocation ==
+
+Each guest script is an executable script with the same uage. For example to
+work with the netbsd guest, use $QEMU_SRC/tests/vm/netbsd:
+
+$ cd $QEMU_SRC/tests/vm
+
+# To bootstrap the image
+$ ./netbsd --build-image --image /var/tmp/netbsd.img
+<...>
+
+# To run an arbitrary command in guest (the output will not be echoed 
unless
+# --debug is added)
+$ ./netbsd --debug --image /tmp/netbsd.img uname -a
+
+# To build QEMU in guest
+$ ./netbsd --debug --image /tmp/netbsd.img --build-qemu $QEMU_SRC
+
+# To get to an interactive shell
+$ ./netbsd --interactive --image /tmp/netbsd.img sh
+
+== Adding new guests ==
+
+Please look at existing guest scripts for how to add new guests.
+
+Most importantly, create a subclass of BaseVM and implement build_image()
+method and define BUILD_SCRIPT, then finally call basevm.main() from the
+scripts main().
+
+  - Usually in build_image(), a template image is downloaded from a predefined
+URL. BaseVM._download_with_cache() takes care of cache the checksum, so
+consider using it.
+
+  - Once the image is downloaded, users, SSH server and QEMU build deps should
+be set up:
+
+* Root password set to BaseVM.ROOT_PASS
+* User BaseVM.GUEST_USER is create, and password set to BaseVM.GUEST_PASS
+* SSH service is enabled and started on boot, BaseVM.SSH_PUB_KEY is added
+  to authorized_keys of both root and the normal user
+* DHCP client service is enabled and started on boot, so that it can
+  automatically configure the virtio-net-pci NIC and communicate with QEMU
+  user net (10.0.2.2)
+* Necessary packages are installed to untar the source tarball and build
+  QEMU
+
+  - Write a proper BUILD_SCRIPT template, which should be a shell script that
+untars a raw virtio-blk block device, which is the tarball data blob of the
+QEMU source tree, then configure/build it. Running "make check" is also
+recommended.
+
-- 
2.13.5

[Qemu-devel] [PATCH v3 06/10] tests: Add NetBSD image

[Fam Zheng]

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng 
Reviewed-by: Kamil Rytarowski 
---
 tests/vm/netbsd | 45 +
 1 file changed, 45 insertions(+)
 create mode 100755 tests/vm/netbsd

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
new file mode 100755
index 00..7d7dfe6586
--- /dev/null
+++ b/tests/vm/netbsd
@@ -0,0 +1,45 @@
+#!/usr/bin/env python
+#
+# NetBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import subprocess
+import tempfile
+import time
+import basevm
+
+class NetBSDVM(basevm.BaseVM):
+name = "netbsd"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d /var/tmp/qemu-test.XX);
+tar -xf /dev/ld1a;
+./configure --python=python2.7 {configure_opts};
+gmake -j{jobs};
+gmake check;
+"""
+
+def build_image(self, img, rebuild=False):
+if os.path.exists(img) and not rebuild:
+return
+cimg = 
self._download_with_cache("http://download.patchew.org/netbsd.img.xz;,
+ 
sha256sum='b633d565b0eac3d02015cd0c81440bd8a7a8df8512615ac1ee05d318be015732')
+img_tmp_xz = img + ".tmp.xz"
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+subprocess.check_call(["xz", "-df", img_tmp_xz])
+os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+sys.exit(basevm.main(NetBSDVM))
-- 
2.13.5

[Qemu-devel] [PATCH v3 04/10] tests: Add ubuntu.i386 image

[Fam Zheng]

This adds a 32bit guest.

The official LTS cloud image is downloaded and initialized with
cloud-init.

Signed-off-by: Fam Zheng 
---
 tests/vm/ubuntu.i386 | 88 
 1 file changed, 88 insertions(+)
 create mode 100755 tests/vm/ubuntu.i386

diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
new file mode 100755
index 00..b478a8a03f
--- /dev/null
+++ b/tests/vm/ubuntu.i386
@@ -0,0 +1,88 @@
+#!/usr/bin/env python
+#
+# Ubuntu i386 image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import subprocess
+import tempfile
+import time
+import basevm
+
+class UbuntuX86VM(basevm.BaseVM):
+name = "ubuntu.i386"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d);
+sudo chmod a+r /dev/vdb;
+tar -xf /dev/vdb;
+./configure {configure_opts};
+make -j{jobs};
+make check;
+"""
+
+def _gen_cloud_init_iso(self):
+cidir = self._tmpdir
+mdata = open(os.path.join(cidir, "meta-data"), "w")
+mdata.writelines(["instance-id: ubuntu-vm-0\n",
+ "local-hostname: ubuntu-guest\n"])
+mdata.close()
+udata = open(os.path.join(cidir, "user-data"), "w")
+udata.writelines(["#cloud-config\n",
+  "chpasswd:\n",
+  "  list: |\n",
+  "root:%s\n" % self.ROOT_PASS,
+  "%s:%s\n" % (self.GUEST_USER, self.GUEST_PASS),
+  "  expire: False\n",
+  "users:\n",
+  "  - name: %s\n" % self.GUEST_USER,
+  "sudo: ALL=(ALL) NOPASSWD:ALL\n",
+  "ssh-authorized-keys:\n",
+  "- %s\n" % basevm.SSH_PUB_KEY,
+  "  - name: root\n",
+  "ssh-authorized-keys:\n",
+  "- %s\n" % basevm.SSH_PUB_KEY])
+udata.close()
+subprocess.check_call(["genisoimage", "-output", "cloud-init.iso",
+   "-volid", "cidata", "-joliet", "-rock",
+   "user-data", "meta-data"],
+   cwd=cidir,
+   stdin=self._devnull, stdout=self._stdout,
+   stderr=self._stdout)
+return os.path.join(cidir, "cloud-init.iso")
+
+def build_image(self, img):
+cimg = 
self._download_with_cache("https://cloud-images.ubuntu.com/releases/16.04/release/ubuntu-16.04-server-cloudimg-i386-disk1.img;)
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp])
+subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
+self.boot(img_tmp, extra_args = ["-cdrom", self._gen_cloud_init_iso()])
+self.wait_ssh()
+self.ssh_root_check("touch /etc/cloud/cloud-init.disabled")
+self.ssh_root_check("apt-get update")
+self.ssh_root_check("apt-get install -y cloud-initramfs-growroot")
+# Don't check the status in case the guest hang up too quickly
+self.ssh_root("sync && reboot")
+time.sleep(5)
+self.wait_ssh()
+# The previous update sometimes doesn't survive a reboot, so do it 
again
+self.ssh_root_check("apt-get update")
+self.ssh_root_check("apt-get build-dep -y qemu")
+self.ssh_root_check("apt-get install -y libfdt-dev")
+self.ssh_root("poweroff")
+self.wait()
+os.rename(img_tmp, img)
+return 0
+
+if __name__ == "__main__":
+sys.exit(basevm.main(UbuntuX86VM))
-- 
2.13.5

[Qemu-devel] [PATCH v3 02/10] qemu.py: Add "wait()" method

[Fam Zheng]

Signed-off-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
---
 scripts/qemu.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 880e3e8219..153f2d1564 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -143,6 +143,13 @@ class QEMUMachine(object):
 self._post_shutdown()
 raise
 
+def wait(self):
+'''Wait for the VM to power off'''
+self._popen.wait()
+self._qmp.close()
+self._load_io_log()
+self._post_shutdown()
+
 def shutdown(self):
 '''Terminate the VM and clean up'''
 if self.is_running():
-- 
2.13.5

[Qemu-devel] [PATCH v3 03/10] tests: Add vm test lib

[Fam Zheng]

This is the common code to implement a "VM test" to

  1) Download and initialize a pre-defined VM that has necessary
  dependencies to build QEMU and SSH access.

  2) Archive $SRC_PATH to a .tar file.

  3) Boot the VM, and pass the source tar file to the guest.

  4) SSH into the VM, untar the source tarball, build from the source.

Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py | 278 +
 1 file changed, 278 insertions(+)
 create mode 100755 tests/vm/basevm.py

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
new file mode 100755
index 00..7eab737aa8
--- /dev/null
+++ b/tests/vm/basevm.py
@@ -0,0 +1,278 @@
+#!/usr/bin/env python
+#
+# VM testing base class
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import time
+import datetime
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
+from qemu import QEMUMachine
+import subprocess
+import hashlib
+import optparse
+import atexit
+import tempfile
+import shutil
+import multiprocessing
+import traceback
+
+SSH_KEY = """\
+-BEGIN RSA PRIVATE KEY-
+MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
+coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
+RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
+7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
+Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
+cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
+7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
+R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
+rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
+cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
+vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
+XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
+j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
+T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
+XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
+JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
+CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
+bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
+eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
+BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
+CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
+D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
+VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
+5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
+-END RSA PRIVATE KEY-
+"""
+SSH_PUB_KEY = """\
+ssh-rsa 
B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
 qemu-vm-key
+"""
+
+class BaseVM(object):
+GUEST_USER = "qemu"
+GUEST_PASS = "qemupass"
+ROOT_PASS = "qemupass"
+
+# The script to run in the guest that builds QEMU
+BUILD_SCRIPT = ""
+# The guest name, to be overridden by subclasses
+name = "#base"
+def __init__(self, debug=False, vcpus=None):
+self._guest = None
+self._tmpdir = tempfile.mkdtemp(prefix="qemu-vm-")
+atexit.register(shutil.rmtree, self._tmpdir)
+
+self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
+open(self._ssh_key_file, "w").write(SSH_KEY)
+subprocess.check_call(["chmod", "600", self._ssh_key_file])
+
+self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
+open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
+
+self.debug = debug
+self._stderr = sys.stderr
+self._devnull = open("/dev/null", "w")
+if self.debug:
+self._stdout = sys.stdout
+else:
+self._stdout = self._devnull
+self._args = [ \
+"-nodefaults", "-enable-kvm", "-m", "2G",
+"-cpu", "host",
+"-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",
+"-device", "virtio-net-pci,netdev=vnet",
+"-vnc", ":0,to=20",
+"-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
+if vcpus:
+self._args += ["-smp", str(vcpus)]
+
+self._data_args = []
+
+def _download_with_cache(self, url, sha256sum=None):
+def check_sha256sum(fname):
+if 

[Qemu-devel] [PATCH v3 00/10] tests: Add VM based build tests (for non-x86_64 and/or non-Linux)

[Fam Zheng]

v3: Drop RFC.
Add Stefan's and Kamil's reviewed-bys.
Use optparse. [Stefan]
Drop the VGA patch. [Paolo, Stefan]
Improve exit/exit code/doc. [Stefan]
Drop unused line from basevm.py. [Stefan]
Drop "--target-list" form Makefile.
More intelligent '-j'.
Add README. [Stefan]

v2: - Add docstring. [Stefan]
- Call self._load_io_lod. [Stefan]
- Use "info usernet" and dynamic ssh_port forwarding. [Stefan]
- Add image checksum.
- Use os.rename() and os.makedirs(). [Stefan]
- Fix NetBSD URL. [Kamil]

Build tests in one 32 bit Linux guest and three BSD images are defined in this
series. This is a more managable way than the manually maintained virtual
machines in patchew. Also, one big advantage of ephemeral VMs over long running
guests is the reduced RAM usage of host, which makes it possible to have one
host test all these BSD variants and probably more.

The BSD guest templates are manually prepared following

https://wiki.qemu.org/Hosts/BSD

as it is not easy to automate. (The ideal approach is like the ubuntu.i386
script, which configures the guest on top of an official released image, fully
automatically.)

Need for help: "gmake check" in the added OpenBSD image fails with -ENOMEM
errors, even if I change "-m 2G" to "-m 8G" when starting VM. Ideas? And there
is a warning from ./configure about OpenBSD going to be unsupported in coming
releases, is it still the case?

Fam

Fam Zheng (10):
  gitignore: Ignore vm test images
  qemu.py: Add "wait()" method
  tests: Add vm test lib
  tests: Add ubuntu.i386 image
  tests: Add FreeBSD image
  tests: Add NetBSD image
  tests: Add OpenBSD image
  Makefile: Add rules to run vm tests
  MAINTAINERS: Add tests/vm entry
  tests: Add README for vm tests

 .gitignore|   2 +
 MAINTAINERS   |   1 +
 Makefile  |   2 +
 configure |   2 +-
 scripts/qemu.py   |   7 ++
 tests/vm/Makefile.include |  40 +++
 tests/vm/README   |  63 +++
 tests/vm/basevm.py| 278 ++
 tests/vm/freebsd  |  45 
 tests/vm/netbsd   |  45 
 tests/vm/openbsd  |  46 
 tests/vm/ubuntu.i386  |  88 +++
 12 files changed, 618 insertions(+), 1 deletion(-)
 create mode 100644 tests/vm/Makefile.include
 create mode 100644 tests/vm/README
 create mode 100755 tests/vm/basevm.py
 create mode 100755 tests/vm/freebsd
 create mode 100755 tests/vm/netbsd
 create mode 100755 tests/vm/openbsd
 create mode 100755 tests/vm/ubuntu.i386

-- 
2.13.5

[Qemu-devel] [PATCH v3 01/10] gitignore: Ignore vm test images

[Fam Zheng]

Signed-off-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index cf65316863..693e2f3009 100644
--- a/.gitignore
+++ b/.gitignore
@@ -52,6 +52,8 @@
 /vscclient
 /vhost-user-scsi
 /fsdev/virtfs-proxy-helper
+/tests/vm/*.img
+/tests/vm/*.tmp
 *.[1-9]
 *.a
 *.aux
-- 
2.13.5

[Qemu-devel] [PULL 6/7] hw/ppc/spapr_rtc: Mark the RTC device with user_creatable = false

[David Gibson]

From: Thomas Huth 

QEMU currently aborts unexpectedly when a user tries to do something
like this:

$ qemu-system-ppc64 -nographic -S -nodefaults -monitor stdio
QEMU 2.9.92 monitor - type 'help' for more information
(qemu) device_add spapr-rtc,id=spapr-rtc
(qemu) device_del spapr-rtc
**
ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
Aborted (core dumped)

The RTC device is not meant to be hot-pluggable - it's an internal
device only and it even should not be possible to create it a
second time with the "-device" parameter, so let's mark this
with "user_creatable = false".

Signed-off-by: Thomas Huth 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_rtc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/spapr_rtc.c b/hw/ppc/spapr_rtc.c
index 00a4e4c717..9ec3078691 100644
--- a/hw/ppc/spapr_rtc.c
+++ b/hw/ppc/spapr_rtc.c
@@ -164,6 +164,8 @@ static void spapr_rtc_class_init(ObjectClass *oc, void 
*data)
 
 dc->realize = spapr_rtc_realize;
 dc->vmsd = _spapr_rtc;
+/* Reason: This is an internal device only for handling the hypercalls */
+dc->user_creatable = false;
 
 spapr_rtas_register(RTAS_GET_TIME_OF_DAY, "get-time-of-day",
 rtas_get_time_of_day);
-- 
2.13.5

[Qemu-devel] [PULL 5/7] hw/ppc/spapr: Fix segfault when instantiating a 'pc-dimm' without 'memdev'

[David Gibson]

From: Thomas Huth 

QEMU currently crashes when trying to use a 'pc-dimm' on the pseries
machine without specifying its 'memdev' property. This happens because
pc_dimm_get_memory_region() does not check whether the 'memdev' property
has properly been set by the user. Looking closer at this function, it's
also obvious that it is using _abort to call another function - and
this is bad in a function that is used in the hot-plugging calling chain
since this can also cause QEMU to exit unexpectedly.

So let's fix these issues in a proper way now: Add a "Error **errp"
parameter to pc_dimm_get_memory_region() which we use in case the 'memdev'
property has not been set by the user, and which we can use instead of
the _abort, and change the callers of get_memory_region() to make
use of this "errp" parameter for proper error checking.

Signed-off-by: Thomas Huth 
Reviewed-by: Igor Mammedov 
Signed-off-by: David Gibson 
---
 hw/i386/pc.c | 14 --
 hw/mem/nvdimm.c  |  2 +-
 hw/mem/pc-dimm.c | 14 +++---
 hw/ppc/spapr.c   | 42 ++
 include/hw/mem/pc-dimm.h |  2 +-
 5 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 59435390ba..21081041d5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1691,10 +1691,15 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
 PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
 PCDIMMDevice *dimm = PC_DIMM(dev);
 PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
-MemoryRegion *mr = ddc->get_memory_region(dimm);
+MemoryRegion *mr;
 uint64_t align = TARGET_PAGE_SIZE;
 bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
 
+mr = ddc->get_memory_region(dimm, _err);
+if (local_err) {
+goto out;
+}
+
 if (memory_region_get_alignment(mr) && pcmc->enforce_aligned_dimm) {
 align = memory_region_get_alignment(mr);
 }
@@ -1758,10 +1763,15 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
 PCMachineState *pcms = PC_MACHINE(hotplug_dev);
 PCDIMMDevice *dimm = PC_DIMM(dev);
 PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
-MemoryRegion *mr = ddc->get_memory_region(dimm);
+MemoryRegion *mr;
 HotplugHandlerClass *hhc;
 Error *local_err = NULL;
 
+mr = ddc->get_memory_region(dimm, _err);
+if (local_err) {
+goto out;
+}
+
 hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
 hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, _err);
 
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index db896b0bb6..952fce5ec8 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -71,7 +71,7 @@ static void nvdimm_init(Object *obj)
 NULL, NULL);
 }
 
-static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm)
+static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp)
 {
 NVDIMMDevice *nvdimm = NVDIMM(dimm);
 
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index ea67b461c2..bdf6649083 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -363,7 +363,10 @@ static void pc_dimm_get_size(Object *obj, Visitor *v, 
const char *name,
 PCDIMMDevice *dimm = PC_DIMM(obj);
 PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj);
 
-mr = ddc->get_memory_region(dimm);
+mr = ddc->get_memory_region(dimm, errp);
+if (!mr) {
+return;
+}
 value = memory_region_size(mr);
 
 visit_type_uint64(v, name, , errp);
@@ -411,9 +414,14 @@ static void pc_dimm_unrealize(DeviceState *dev, Error 
**errp)
 host_memory_backend_set_mapped(dimm->hostmem, false);
 }
 
-static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
+static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm, Error 
**errp)
 {
-return host_memory_backend_get_memory(dimm->hostmem, _abort);
+if (!dimm->hostmem) {
+error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property must be set");
+return NULL;
+}
+
+return host_memory_backend_get_memory(dimm->hostmem, errp);
 }
 
 static MemoryRegion *pc_dimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f7a19720dc..cec441cbf4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2772,10 +2772,15 @@ static void spapr_memory_plug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
 PCDIMMDevice *dimm = PC_DIMM(dev);
 PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
-MemoryRegion *mr = ddc->get_memory_region(dimm);
-uint64_t align = memory_region_get_alignment(mr);
-uint64_t size = memory_region_size(mr);
-uint64_t addr;
+MemoryRegion *mr;
+uint64_t align, size, addr;
+
+mr = ddc->get_memory_region(dimm, _err);
+if (local_err) {
+goto out;
+}
+align = memory_region_get_alignment(mr);
+size = 

[Qemu-devel] [PULL 7/7] hw/ppc/spapr_iommu: Fix crash when removing the "spapr-tce-table" device

[David Gibson]

From: Thomas Huth 

QEMU currently aborts unexpectedly when the user tries to add and
remove a "spapr-tce-table" device:

$ qemu-system-ppc64 -nographic -S -nodefaults -monitor stdio
QEMU 2.9.92 monitor - type 'help' for more information
(qemu) device_add spapr-tce-table,id=x
(qemu) device_del x
**
ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
Aborted (core dumped)

The device should not be accessable for the users at all, it's just
used internally, so mark it with user_creatable = false.

Signed-off-by: Thomas Huth 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_iommu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index e614621a83..ed2d53559a 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -618,6 +618,8 @@ static void spapr_tce_table_class_init(ObjectClass *klass, 
void *data)
 dc->init = spapr_tce_table_realize;
 dc->reset = spapr_tce_reset;
 dc->unrealize = spapr_tce_table_unrealize;
+/* Reason: This is just an internal device for handling the hypercalls */
+dc->user_creatable = false;
 
 QLIST_INIT(_tce_tables);
 
-- 
2.13.5

[Qemu-devel] [PULL 4/7] spapr: Allow configure-connector to be called multiple times

[David Gibson]

From: Bharata B Rao 

In case of in-kernel memory hot unplug, when the guest is not able
to remove all the LMBs that are requested for removal, it will add back
any LMBs that have been successfully removed. The DR Connectors of
these LMBs wouldn't have been unconfigured and hence the addition of
these LMBs will result in configure-connector call being issued on
LMB DR connectors that are already in configured state. Such
configure-connector calls will fail resulting in a DIMM which is
partially unplugged.

This however worked till recently before we overhauled the DRC
implementation in QEMU. Commit 9d4c0f4f0a71e: "spapr: Consolidate
DRC state variables" is the first commit where this problem shows up
as per git bisect.

Ideally guest shouldn't be issuing configure-connector call on an
already configured DR connector. However for now, work around this in
QEMU by allowing configure-connector to be called multiple times for
all types of DR connectors.

Signed-off-by: Bharata B Rao 
[dwg: Corrected buglet that would have initialized fdt pointers ready
 for reading on a device not present at reset]
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_drc.c | 30 +++---
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 5260b5d363..605697d8bd 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -442,12 +442,17 @@ void spapr_drc_reset(sPAPRDRConnector *drc)
 if (drc->dev) {
 /* A device present at reset is ready to go, same as coldplugged */
 drc->state = drck->ready_state;
+/*
+ * Ensure that we are able to send the FDT fragment again
+ * via configure-connector call if the guest requests.
+ */
+drc->ccs_offset = drc->fdt_start_offset;
+drc->ccs_depth = 0;
 } else {
 drc->state = drck->empty_state;
+drc->ccs_offset = -1;
+drc->ccs_depth = -1;
 }
-
-drc->ccs_offset = -1;
-drc->ccs_depth = -1;
 }
 
 static void drc_reset(void *opaque)
@@ -1071,8 +1076,14 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
 }
 
 if ((drc->state != SPAPR_DRC_STATE_LOGICAL_UNISOLATE)
-&& (drc->state != SPAPR_DRC_STATE_PHYSICAL_UNISOLATE)) {
-/* Need to unisolate the device before configuring */
+&& (drc->state != SPAPR_DRC_STATE_PHYSICAL_UNISOLATE)
+&& (drc->state != SPAPR_DRC_STATE_LOGICAL_CONFIGURED)
+&& (drc->state != SPAPR_DRC_STATE_PHYSICAL_CONFIGURED)) {
+/*
+ * Need to unisolate the device before configuring
+ * or it should already be in configured state to
+ * allow configure-connector be called repeatedly.
+ */
 rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
 goto out;
 }
@@ -1108,8 +1119,13 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
 /* done sending the device tree, move to configured state */
 trace_spapr_drc_set_configured(drc_index);
 drc->state = drck->ready_state;
-drc->ccs_offset = -1;
-drc->ccs_depth = -1;
+/*
+ * Ensure that we are able to send the FDT fragment
+ * again via configure-connector call if the guest requests.
+ */
+drc->ccs_offset = drc->fdt_start_offset;
+drc->ccs_depth = 0;
+fdt_offset_next = drc->fdt_start_offset;
 resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
 } else {
 resp = SPAPR_DR_CC_RESPONSE_PREV_PARENT;
-- 
2.13.5

Re: [Qemu-devel] Any release-critical issues for 2.10 ?

[David Gibson]

On Mon, Aug 21, 2017 at 11:51:00AM +0100, Peter Maydell wrote:
> Last call -- are there any release critical issues for 2.10?
> If not then I'll tag the current rc3 as the final release tomorrow,
> so please reply here to let me know if there's anything we should
> be considering as needing an rc4.

Uh.. kind of, yeah.  I have some real-guest-affecting regressions from
2.9 that I have fixes for.  Sorry they're so late - that's partly my
fault, but then they were delayed further as I tried to do my usual
pre-pull tests by the change in boot-serial-test which breaks make
check on a ppc host.

I'm just finishing up my final tests now, then I'll send the pull
request.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [PULL 2/7] target/ppc: 'PVR != host PVR' in KVM_SET_SREGS workaround

[David Gibson]

From: Daniel Henrique Barboza 

Commit d5fc133eed ("ppc: Rework CPU compatibility testing
across migration") changed the way cpu_post_load behaves with
the PVR setting, causing an unexpected bug in KVM-HV migrations
between hosts that are compatible (POWER8 and POWER8E, for example).
Even with pvr_match() returning true, the guest freezes right after
cpu_post_load. The reason is that the guest kernel can't handle a
different PVR value other that the running host in KVM_SET_SREGS.

In [1] it was discussed the possibility of a new KVM capability
that would indicate that the guest kernel can handle a different
PVR in KVM_SET_SREGS. Even if such feature is implemented, there is
still the problem with older kernels that will not have this capability
and will fail to migrate.

This patch implements a workaround for that scenario. If running
with KVM, check if the guest kernel does not have the capability
(named here as 'cap_ppc_pvr_compat'). If it doesn't, calls
kvmppc_is_pr() to see if the guest is running in KVM-HV. If all this
happens, set env->spr[SPR_PVR] to the same value as the current
host PVR. This ensures that we allow migrations with 'close enough'
PVRs to still work in KVM-HV but also makes the code ready for
this new KVM capability when it is done.

A new function called 'kvmppc_pvr_workaround_required' was created
to encapsulate the conditions said above and to avoid calling too
many kvm.c internals inside cpu_post_load.

[1] https://lists.gnu.org/archive/html/qemu-ppc/2017-06/msg00503.html

Signed-off-by: Daniel Henrique Barboza 
Signed-off-by: David Gibson 
---
 target/ppc/kvm.c | 34 ++
 target/ppc/kvm_ppc.h |  1 +
 target/ppc/machine.c | 22 ++
 3 files changed, 57 insertions(+)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 85713795de..fb3ee4351a 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -90,6 +90,7 @@ static int cap_htm; /* Hardware transactional 
memory support */
 static int cap_mmu_radix;
 static int cap_mmu_hash_v3;
 static int cap_resize_hpt;
+static int cap_ppc_pvr_compat;
 
 static uint32_t debug_inst_opcode;
 
@@ -147,6 +148,13 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);
 cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);
 cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);
+/*
+ * Note: setting it to false because there is not such capability
+ * in KVM at this moment.
+ *
+ * TODO: call kvm_vm_check_extension() with the right capability
+ * after the kernel starts implementing it.*/
+cap_ppc_pvr_compat = false;
 
 if (!cap_interrupt_level) {
 fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
@@ -2785,3 +2793,29 @@ void kvmppc_update_sdr1(target_ulong sdr1)
 run_on_cpu(cs, kvmppc_pivot_hpt_cpu, RUN_ON_CPU_TARGET_PTR(sdr1));
 }
 }
+
+/*
+ * This is a helper function to detect a post migration scenario
+ * in which a guest, running as KVM-HV, freezes in cpu_post_load because
+ * the guest kernel can't handle a PVR value other than the actual host
+ * PVR in KVM_SET_SREGS, even if pvr_match() returns true.
+ *
+ * If we don't have cap_ppc_pvr_compat and we're not running in PR
+ * (so, we're HV), return true. The workaround itself is done in
+ * cpu_post_load.
+ *
+ * The order here is important: we'll only check for KVM PR as a
+ * fallback if the guest kernel can't handle the situation itself.
+ * We need to avoid as much as possible querying the running KVM type
+ * in QEMU level.
+ */
+bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
+{
+CPUState *cs = CPU(cpu);
+
+if (cap_ppc_pvr_compat) {
+return false;
+}
+
+return !kvmppc_is_pr(cs->kvm_state);
+}
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 6bc6fb3e2d..381afe6240 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -67,6 +67,7 @@ void kvmppc_check_papr_resize_hpt(Error **errp);
 int kvmppc_resize_hpt_prepare(PowerPCCPU *cpu, target_ulong flags, int shift);
 int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift);
 void kvmppc_update_sdr1(target_ulong sdr1);
+bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
 
 bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);
 
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index abe0a1cdf0..e36b7100cb 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -9,6 +9,7 @@
 #include "mmu-hash64.h"
 #include "migration/cpu.h"
 #include "qapi/error.h"
+#include "kvm_ppc.h"
 
 static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
 {
@@ -249,6 +250,27 @@ static int cpu_post_load(void *opaque, int version_id)
 }
 }
 
+/*
+ * If we're running with KVM HV, there is a chance that the guest
+ 

[Qemu-devel] [PULL 0/7] ppc-for-2.10 queue 20170822

[David Gibson]

The following changes since commit 1f296733876434118fd766cfef5eb6f29ecab6a8:

  Update version for v2.10.0-rc3 release (2017-08-15 18:53:31 +0100)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.10-20170822

for you to fetch changes up to d3234e2851f1630c695c681beac1e87ac0881260:

  hw/ppc/spapr_iommu: Fix crash when removing the "spapr-tce-table" device 
(2017-08-22 11:11:30 +1000)


ppc patch queue 2017-08-22

Last minute ppc related fixes for qemu-2.10.  I'm not sure if these
are critical enough to prompt another rc, but I'm submitting them for
consideration.

First, is Cornelia's fix for 480bc11e6 which meant "make check" would
always fail on a ppc host.  Tracking that down delayed submission of
the rest of these patches, sorry.

The rest are all fairly important bugfixes for qemu crashes or guest
behaviour regression on ppc.  Patches 2-4 specifically are fixes for
regressions from qemu-2.9, caused by the compatibility mode and
hotplug handling cleanups for the pseries machine type.


Bharata B Rao (1):
  spapr: Allow configure-connector to be called multiple times

Cornelia Huck (1):
  boot-serial-test: prefer tcg accelerator

Daniel Henrique Barboza (1):
  target/ppc: 'PVR != host PVR' in KVM_SET_SREGS workaround

Greg Kurz (1):
  ppc: fix ppc_set_compat() with KVM PR

Thomas Huth (3):
  hw/ppc/spapr: Fix segfault when instantiating a 'pc-dimm' without 'memdev'
  hw/ppc/spapr_rtc: Mark the RTC device with user_creatable = false
  hw/ppc/spapr_iommu: Fix crash when removing the "spapr-tce-table" device

 hw/i386/pc.c | 14 --
 hw/mem/nvdimm.c  |  2 +-
 hw/mem/pc-dimm.c | 14 +++---
 hw/ppc/spapr.c   | 42 ++
 hw/ppc/spapr_drc.c   | 30 +++---
 hw/ppc/spapr_iommu.c |  2 ++
 hw/ppc/spapr_rtc.c   |  2 ++
 include/hw/mem/pc-dimm.h |  2 +-
 target/ppc/compat.c  |  9 +
 target/ppc/kvm.c | 34 ++
 target/ppc/kvm_ppc.h |  1 +
 target/ppc/machine.c | 22 ++
 tests/boot-serial-test.c |  6 +-
 13 files changed, 149 insertions(+), 31 deletions(-)

[Qemu-devel] [PULL 3/7] ppc: fix ppc_set_compat() with KVM PR

[David Gibson]

From: Greg Kurz 

When running in KVM PR mode, kvmppc_set_compat() always fail because the
current PR implementation doesn't handle KVM_REG_PPC_ARCH_COMPAT. Now that
the machine code inconditionally calls ppc_set_compat_all() at reset time
to restore the compat mode default value (commit 66d5c492dd3a9), it is
impossible to start a guest with PR:

qemu-system-ppc64: Unable to set CPU compatibility mode in KVM:
 Invalid argument

A tentative patch [1] was recently sent by Suraj to address the issue, but
it would prevent the compat mode to be turned off on reset. And we really
don't want to explicitely check for KVM PR. During the patch's review,
David suggested that we should only call the KVM ioctl() if the compat
PVR changes. This allows at least to run with KVM PR, provided no compat
mode is requested from the command line (which should be the case when
running PR nested). This is what this patch does.

While here, we also fix the side effect where KVM would fail but we would
change the CPU state in QEMU anyway.

[1] http://patchwork.ozlabs.org/patch/782039/

Signed-off-by: Greg Kurz 
Reviewed-by: Suraj Jitindar Singh 
Signed-off-by: David Gibson 
---
 target/ppc/compat.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/ppc/compat.c b/target/ppc/compat.c
index f1b67faa97..f8729fe46d 100644
--- a/target/ppc/compat.c
+++ b/target/ppc/compat.c
@@ -140,16 +140,17 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, 
Error **errp)
 
 cpu_synchronize_state(CPU(cpu));
 
-cpu->compat_pvr = compat_pvr;
-env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
-
-if (kvm_enabled()) {
+if (kvm_enabled() && cpu->compat_pvr != compat_pvr) {
 int ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
 if (ret < 0) {
 error_setg_errno(errp, -ret,
  "Unable to set CPU compatibility mode in KVM");
+return;
 }
 }
+
+cpu->compat_pvr = compat_pvr;
+env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
 }
 
 typedef struct {
-- 
2.13.5

[Qemu-devel] [PULL 1/7] boot-serial-test: prefer tcg accelerator

[David Gibson]

From: Cornelia Huck 

Prefer to use the tcg accelarator if it is available: This is our only
real smoke test for tcg, and fast enough to use it for that.

Fixes: 480bc11e6 ("boot-serial-test: fallback to kvm accelerator")
Reported-by: Richard Henderson 
Signed-off-by: Cornelia Huck 
Signed-off-by: David Gibson 
---
 tests/boot-serial-test.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index a8ca877168..b95c5e74ea 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -78,7 +78,11 @@ static void test_machine(const void *data)
 fd = mkstemp(tmpname);
 g_assert(fd != -1);
 
-args = g_strdup_printf("-M %s,accel=kvm:tcg "
+/*
+ * Make sure that this test uses tcg if available: It is used as a
+ * fast-enough smoketest for that.
+ */
+args = g_strdup_printf("-M %s,accel=tcg:kvm "
"-chardev file,id=serial0,path=%s "
"-no-shutdown -serial chardev:serial0 %s",
test->machine, tmpname, test->extra);
-- 
2.13.5

Re: [Qemu-devel] [PATCH] audio: intel-hda: do not use old_mmio accesses

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20170821211320.7026-1-mtpa...@gmail.com
Subject: [Qemu-devel] [PATCH] audio: intel-hda: do not use old_mmio accesses

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
87ea375b1e audio: intel-hda: do not use old_mmio accesses

=== OUTPUT BEGIN ===
Checking PATCH 1/1: audio: intel-hda: do not use old_mmio accesses...
WARNING: line over 80 characters
#19: FILE: hw/audio/intel-hda.c:1046:
+static void intel_hda_mmio_write(void *opaque, hwaddr addr, uint64_t val, 
unsigned size)

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 1 warnings, 75 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Fam Zheng]

On Tue, 08/22 10:56, Peter Xu wrote:
> I haven't really encountered (c), but I think it's the migrate_cancel
> command that matters, which should not need BQL as well.

There is bdrv_invalidate_cache_all() in migrate_cancel which clearly isn't safe.
Is that if block unreachable in this case? If so we should assert, otherwise
this command is not okay to run without BQL.

Generically, what guarantee the thread-safety of a qmp command when you decide
BQL is not needed? In other words, how do you prove commands are safe without
BQL? I think almost every command accesses global state, but lock-free data
structures are rare AFAICT.

Fam

Re: [Qemu-devel] [Qemu-arm] [PATCH 4/8] boards.h: Define new flag ignore_memory_transaction_failures

[Philippe Mathieu-Daudé]

Hi Peter,

On 08/17/2017 07:25 AM, Peter Maydell wrote:

On 5 August 2017 at 11:13, Peter Maydell  wrote:

On 4 August 2017 at 20:23, Richard Henderson
 wrote:

On 08/04/2017 11:09 AM, Philippe Mathieu-Daudé wrote:

Since create_unimplemented_device() register overlapped with low priority, why
not register it as default device directly, over the whole address space?


That's a good suggestion.  It makes more sense to me than adding a flag on the
MachineClass.


Yeah, I did think about implementing it that way, but...

That wouldn't handle the case of a device model directly
returning a MEMTX_ERROR, or a transaction dispatched to
a memory region whose MemoryRegionOps valid settings
prohibit it (eg byte accesses to a word-access-only device),
or accesses to a MemoryRegion that was created by passing
a NULL MemoryRegionOps pointer to memory_region_init_io
(I dunno why you'd do that but some code does).

In short, there are lots of ways the memory subsystem might
end up returning a transaction error -- this mechanism
ensures that none of them start generating exceptions
when they previously did not, and is (I hope) easy to
review in the sense of being sure that it does what it
intends to do without the need to audit a lot of corner
cases.


So, this question (should we have a board flag to disable reporting
of tx failures to the CPU hook, or use unimplemented_device as a
sort of background region) seems to be the main unanswered question
for this series. I think (as outlined above) that the board flag
is simpler and safer; are people happy for me to put this series
in target-arm.next with that approach, or should I rethink this bit?
As remarked previously in this thread, the current QEMU behavior on 
transaction error isn't always matching real hardware.

Matching correctly throwing errors is likely to break various current users.

If we are worried about being backward compatible, defaulting background 
region to unimp() won't throw any transaction error.


Since the default is no transaction error, users who expect hardware 
error can implement the correct behavior, per region/bus transaction errors.


I'm somehow afraid that "ignore_memory_transaction_failures" ends up 
like the "cannot_instantiate_with_device_add_yet" flag - a hard to 
remove kludge outliving his purpose.


Anyway I'm not unhappy with this approach, but I'd be very happy to have 
unimp() covering the whole background region.


Regards,

Phil.

Re: [Qemu-devel] [PATCH v3 2/5] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls

[David Gibson]

On Mon, Aug 21, 2017 at 06:05:34PM +0530, Aravinda Prasad wrote:
> 
> 
> On Thursday 17 August 2017 07:09 AM, David Gibson wrote:
> > What's with the extra spaces in the subject line?
> 
> I don't see any. Can you pls point out?

I see "ibm, nmi-register" and "ibm, nmi-interlock"

> 
> > 
> > On Wed, Aug 16, 2017 at 02:42:21PM +0530, Aravinda Prasad wrote:
> >> This patch adds support in QEMU to handle "ibm,nmi-register"
> >> and "ibm,nmi-interlock" RTAS calls.
> >>
> >> The machine check notification address is saved when the
> >> OS issues "ibm,nmi-register" RTAS call.
> >>
> >> This patch also handles the case when multiple processors
> >> experience machine check at or about the same time by
> >> handling "ibm,nmi-interlock" call. In such cases, as per
> >> PAPR, subsequent processors serialize waiting for the first
> >> processor to issue the "ibm,nmi-interlock" call. The second
> >> processor waits till the first processor, which also
> >> received a machine check error, is done reading the error
> >> log. The first processor issues "ibm,nmi-interlock" call
> >> when the error log is consumed. This patch implements the
> >> releasing part of the error-log while subsequent patch
> >> (which builds error log) handles the locking part.
> >>
> >> Signed-off-by: Aravinda Prasad 
> >> ---
> >>  hw/ppc/spapr.c |8 
> >>  hw/ppc/spapr_rtas.c|   35 +++
> >>  include/hw/ppc/spapr.h |   10 +-
> >>  3 files changed, 52 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 2a3e53d..0bb2c4a 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -1441,6 +1441,11 @@ static void ppc_spapr_reset(void)
> >>  first_ppc_cpu->env.nip = SPAPR_ENTRY_POINT;
> >>  
> >>  spapr->cas_reboot = false;
> >> +
> >> +spapr->mc_in_progress = false;
> >> +spapr->guest_machine_check_addr = 0;
> >> +qemu_cond_destroy(>mc_delivery_cond);
> >> +qemu_cond_init(>mc_delivery_cond);
> >>  }
> >>  
> >>  static void spapr_create_nvram(sPAPRMachineState *spapr)
> >> @@ -2491,6 +2496,9 @@ static void ppc_spapr_init(MachineState *machine)
> >>  
> >>  kvmppc_spapr_enable_inkernel_multitce();
> >>  }
> >> +
> >> +spapr->mc_in_progress = false;
> >> +qemu_cond_init(>mc_delivery_cond);
> >>  }
> >>  
> >>  static int spapr_kvm_type(const char *vm_type)
> >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >> index 94a2799..2f3c47b 100644
> >> --- a/hw/ppc/spapr_rtas.c
> >> +++ b/hw/ppc/spapr_rtas.c
> >> @@ -348,6 +348,37 @@ static void rtas_get_power_level(PowerPCCPU *cpu, 
> >> sPAPRMachineState *spapr,
> >>  rtas_st(rets, 1, 100);
> >>  }
> >>  
> >> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> >> +  sPAPRMachineState *spapr,
> >> +  uint32_t token, uint32_t nargs,
> >> +  target_ulong args,
> >> +  uint32_t nret, target_ulong rets)
> >> +{
> >> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
> >> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >> +}
> >> +
> >> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
> >> +   sPAPRMachineState *spapr,
> >> +   uint32_t token, uint32_t nargs,
> >> +   target_ulong args,
> >> +   uint32_t nret, target_ulong rets)
> >> +{
> >> +if (!spapr->guest_machine_check_addr) {
> >> +/* NMI register not called */
> >> +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> >> +} else {
> >> +/*
> >> + * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> >> + * hence unset mc_in_progress.
> >> + */
> >> +spapr->mc_in_progress = false;
> >> +qemu_cond_signal(>mc_delivery_cond);
> >> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >> +}
> >> +}
> >> +
> >> +
> >>  static struct rtas_call {
> >>  const char *name;
> >>  spapr_rtas_fn fn;
> >> @@ -489,6 +520,10 @@ static void core_rtas_register_types(void)
> >>  rtas_set_power_level);
> >>  spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
> >>  rtas_get_power_level);
> >> +spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
> >> +rtas_ibm_nmi_register);
> >> +spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
> >> +rtas_ibm_nmi_interlock);
> >>  }
> >>  
> >>  type_init(core_rtas_register_types)
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index 46012b3..eee8d33 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -123,6 +123,12 @@ struct sPAPRMachineState {
> >>   * occurs during the unplug process. */
> >>  QTAILQ_HEAD(, 

Re: [Qemu-devel] [RFC PATCH 12/12] ppc: Add aCube Sam460ex board

[David Gibson]

On Fri, Aug 18, 2017 at 02:46:42PM +0200, BALATON Zoltan wrote:
> On Fri, 18 Aug 2017, David Gibson wrote:
> > On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:
> > > Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
> > > This is not a full implementation yet with a lot of components still
> > > missing but enough to start a Linux kernel and the U-Boot firmware.
> > > 
> > > Signed-off-by: François Revol 
> > > Signed-off-by: BALATON Zoltan 
> > 
> > There are a *lot* of devices defined here.  Most of them look like
> > they belong to the SoC, not the board (since they use DCRs), so it
> > doesn't really make sense to define them in a board file.  It would
> > also make it easier to review if they were split up into separate
> > patches.
> 
> I thought it's simpler to review a series with 12 reasonably sized patches
> than one with twice as many which only modify a few lines here and there
> each. Also adding a lot of code scattered in hw directories is probably less
> clear than having them all at one place. But of course each approach can be
> reasoned. I thought this might have to be split up but I've left it one
> place for now for first review to get some advice on what's preferred.

Well, it depends on the content of the patches.  If splitting it up
means a lot of looking between patches to make sense of what's going
on then that's certainly not good.  But if the small patches are
independent of each other and can be assessed on their own merits,
then that usually makes it easier.

In this case the various new 440 devices should be pretty much
independent of each other, so I think splitting is the better option.

> Maybe I should put things that belong to the SoC in ppc440_uc.c (similar to
> ppc405uc.c we already have) and move common devices used by both to
> ppc4xx_devs.c (which already seems to serve that purpose). If more cleanup
> is needed that could be done separately afterwards, I don't think it's a
> good idea to mix in too much cleanup now to keep patches relatively simple.
> (I already have some moving around included as clean up patches but I'd like
> to focus on actual functions than clean up at this point).
> 
> Does putting these devices from board code to ppc440_uc.c sound
> acceptable?

That'd be ok - though again, I'd prefer to see each device as a
separate patch.  It would probably be preferable to put each device in
a separate file as well though, unless they're _really_ tiny.  Nothing
inherently wrong with small .c files, if they're still more or less
self contained.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 1/5] ppc: spapr: Register and handle HCALL to receive updated RTAS region

[David Gibson]

On Mon, Aug 21, 2017 at 03:12:49PM +0530, Aravinda Prasad wrote:
> 
> 
> On Thursday 17 August 2017 07:04 AM, David Gibson wrote:
> > On Wed, Aug 16, 2017 at 02:42:13PM +0530, Aravinda Prasad wrote:
> >> Receive updates from SLOF about the updated rtas-base.
> >> A separate patch for SLOF [1] adds functionality to invoke
> >> a private HCALL whenever OS issues instantiate-rtas with
> >> a new rtas-base.
> >>
> >> This is required as QEMU needs to know the updated rtas-base
> >> as it allocates error reporting structure in RTAS space upon
> >> a machine check exception.
> >>
> >> [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-August/120386.html
> >>
> >> Signed-off-by: Aravinda Prasad 
> >> Reviewed-by: David Gibson 
> > 
> > Actually, I take back this R-b, see below.
> > 
> > In any case I'm not willing to apply the patches which depend on this
> > until the corresponding SLOF update is merged as well.
> 
> As Nikunj mentioned, SLOF updates are already merged.

In qemu as well as SLOF master, ok, good.  Commit message could do
with updating to reflect that.

> >>  hw/ppc/spapr_hcall.c   |8 
> >>  include/hw/ppc/spapr.h |4 +++-
> >>  2 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 72ea5a8..e66c72e 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1062,6 +1062,13 @@ static target_ulong h_rtas(PowerPCCPU *cpu, 
> >> sPAPRMachineState *spapr,
> >> nret, rtas_r3 + 12 + 4*nargs);
> >>  }
> >>  
> >> +static target_ulong h_rtas_update(PowerPCCPU *cpu, sPAPRMachineState 
> >> *spapr,
> >> +  target_ulong opcode, target_ulong *args)
> >> +{
> >> +spapr->rtas_addr = args[0];
> >> +return 0;
> >> +}
> >> +
> >>  static target_ulong h_logical_load(PowerPCCPU *cpu, sPAPRMachineState 
> >> *spapr,
> >> target_ulong opcode, target_ulong 
> >> *args)
> >>  {
> >> @@ -1717,6 +1724,7 @@ static void hypercall_register_types(void)
> >>  
> >>  /* qemu/KVM-PPC specific hcalls */
> >>  spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
> >> +spapr_register_hypercall(KVMPPC_H_RTAS_UPDATE, h_rtas_update);
> >>  
> >>  /* ibm,client-architecture-support support */
> >>  spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
> >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >> index 2a303a7..46012b3 100644
> >> --- a/include/hw/ppc/spapr.h
> >> +++ b/include/hw/ppc/spapr.h
> >> @@ -90,6 +90,7 @@ struct sPAPRMachineState {
> >>  
> >>  hwaddr rma_size;
> >>  int vrma_adjust;
> >> +hwaddr rtas_addr;
> > 
> > This can now change at runtime, which means it needs to be migrated -
> > that's not happening in your patches yet.
> 
> Yes. Need a bit of help in understanding the migration process.
> 
> As rtas_addr is updated by SLOF, I think we need to modify SLOF to issue
> KVMPPC_H_RTAS_UPDATE HCALL with the new rtas_addr during migration. But
> I am not sure if SLOF is notified of migrations.

Uh.. no.  By the time you're migrating chances are SLOF isn't even
running any more, and it wouldn't make sense for it to be aware of
migration anyway.

Instead we need to add the rtas_addr field to the vmstate information
for the spapr machine object.  However, we can't just add it plain,
because that would break backwards migration.  Instead we'll need to
add another sub-vmstate which will migrate rtas_addr if it differs
from the default value.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0

[Dou Liyang]

Currently, Using the fisrt node without memory on the machine makes
QEMU unhappy. With this example command line:
  ... \
  -m 1024M,slots=4,maxmem=32G \
  -numa node,nodeid=0 \
  -numa node,mem=1024M,nodeid=1 \
  -numa node,nodeid=2 \
  -numa node,nodeid=3 \
Guest reports "No NUMA configuration found" and the NUMA topology is
wrong.

This is because when QEMU builds ACPI SRAT, it regards node0 as the
default node to deal with the memory hole(640K-1M). this means the
node0 must have some memory(>1M), but, actually it can have no
memory.

Fix this problem by replace the node0 with the first node which has
memory on it. Add a new function for each node. Also do some cleanup.

Signed-off-by: Dou Liyang 
---
 hw/i386/acpi-build.c | 78 +---
 1 file changed, 50 insertions(+), 28 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 98dd424..f93d712 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2318,15 +2318,43 @@ build_tpm2(GArray *table_data, BIOSLinker *linker)
  (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }
 
+static uint64_t
+build_srat_node_entry(GArray *table_data, PCMachineState *pcms,
+int i, uint64_t mem_base, uint64_t mem_len)
+{
+AcpiSratMemoryAffinity *numamem;
+uint64_t next_base;
+
+next_base = mem_base + mem_len;
+
+/* Cut out the ACPI_PCI hole */
+if (mem_base <= pcms->below_4g_mem_size &&
+next_base > pcms->below_4g_mem_size) {
+mem_len -= next_base - pcms->below_4g_mem_size;
+if (mem_len > 0) {
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, mem_base, mem_len, i,
+  MEM_AFFINITY_ENABLED);
+}
+mem_base = 1ULL << 32;
+mem_len = next_base - pcms->below_4g_mem_size;
+next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+}
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, mem_base, mem_len, i,
+  MEM_AFFINITY_ENABLED);
+return next_base;
+}
+
 static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
 AcpiSystemResourceAffinityTable *srat;
 AcpiSratMemoryAffinity *numamem;
 
-int i;
+int i, node;
 int srat_start, numa_start, slots;
-uint64_t mem_len, mem_base, next_base;
+uint64_t mem_len, mem_base;
 MachineClass *mc = MACHINE_GET_CLASS(machine);
 const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
 PCMachineState *pcms = PC_MACHINE(machine);
@@ -2370,36 +2398,30 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 /* the memory map is a bit tricky, it contains at least one hole
  * from 640k-1M and possibly another one from 3.5G-4G.
  */
-next_base = 0;
+
 numa_start = table_data->len;
 
-numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, 0, 640 * 1024, 0, MEM_AFFINITY_ENABLED);
-next_base = 1024 * 1024;
-for (i = 1; i < pcms->numa_nodes + 1; ++i) {
-mem_base = next_base;
-mem_len = pcms->node_mem[i - 1];
-if (i == 1) {
-mem_len -= 1024 * 1024;
+/* get the first node which has memory and map the hole from 640K-1M */
+for (node = 0; node < pcms->numa_nodes; node++) {
+if (pcms->node_mem[node] != 0) {
+break;
 }
-next_base = mem_base + mem_len;
-
-/* Cut out the ACPI_PCI hole */
-if (mem_base <= pcms->below_4g_mem_size &&
-next_base > pcms->below_4g_mem_size) {
-mem_len -= next_base - pcms->below_4g_mem_size;
-if (mem_len > 0) {
-numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, mem_base, mem_len, i - 1,
-  MEM_AFFINITY_ENABLED);
-}
-mem_base = 1ULL << 32;
-mem_len = next_base - pcms->below_4g_mem_size;
-next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+}
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, 0, 640 * 1024, node, MEM_AFFINITY_ENABLED);
+
+/* map the rest of memory from 1M */
+mem_base = 1024 * 1024;
+mem_len = pcms->node_mem[node] - mem_base;
+mem_base = build_srat_node_entry(table_data, pcms, node,
+mem_base, mem_len);
+
+for (i = 0; i < pcms->numa_nodes; i++) {
+if (i == node) {
+continue;
 }
-numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, mem_base, mem_len, i - 1,
-  MEM_AFFINITY_ENABLED);
+mem_base = build_srat_node_entry(table_data, pcms, i,
+mem_base, 

[Qemu-devel] [PATCH v4 0/2] hw/acpi-build: Fix ACPI SRAT Memory Affinity building

[Dou Liyang]

V3 --> v4:
  -add a new testcase.

This patchset fixs an ACPI building bug which caused by no RAM
in the first NUAM node. and also add a new testcase for the bug.

Dou Liyang (2):
  hw/acpi-build: Fix SRAT memory building when there is no memory in
node0
  ACPI/unit-test: Add a new testcase for RAM allocation in numa node

 hw/i386/acpi-build.c  |  78 ++
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
 tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
 tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
 tests/bios-tables-test.c  |  30 +
 8 files changed, 80 insertions(+), 28 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

-- 
2.5.5

[Qemu-devel] [PATCH v4 2/2] ACPI/unit-test: Add a new testcase for RAM allocation in numa node

[Dou Liyang]

As QEMU supports the memory-less node, it is possible that there is
no RAM in the first numa node(also be called as node0). eg:
  ... \
  -m 128,slots=3,maxmem=1G \
  -numa node -numa node,mem=128M \

But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
table. Only fixing it is not enough.

Add a testcase for this situation to make sure the ACPI table is
correct for guest.

Suggested-by: Eduardo Habkost 
Signed-off-by: Dou Liyang 
---
 tests/acpi-test-data/pc/DSDT.numamem  | Bin 0 -> 6463 bytes
 tests/acpi-test-data/pc/SLIT.numamem  | Bin 0 -> 48 bytes
 tests/acpi-test-data/pc/SRAT.numamem  | Bin 0 -> 264 bytes
 tests/acpi-test-data/q35/DSDT.numamem | Bin 0 -> 9147 bytes
 tests/acpi-test-data/q35/SLIT.numamem | Bin 0 -> 48 bytes
 tests/acpi-test-data/q35/SRAT.numamem | Bin 0 -> 264 bytes
 tests/bios-tables-test.c  |  30 ++
 7 files changed, 30 insertions(+)
 create mode 100644 tests/acpi-test-data/pc/DSDT.numamem
 create mode 100644 tests/acpi-test-data/pc/SLIT.numamem
 create mode 100644 tests/acpi-test-data/pc/SRAT.numamem
 create mode 100644 tests/acpi-test-data/q35/DSDT.numamem
 create mode 100644 tests/acpi-test-data/q35/SLIT.numamem
 create mode 100644 tests/acpi-test-data/q35/SRAT.numamem

diff --git a/tests/acpi-test-data/pc/DSDT.numamem 
b/tests/acpi-test-data/pc/DSDT.numamem
new file mode 100644
index 
..53f6d5824359ff0ca20179b19b3b5cb79f017f82
GIT binary patch
literal 6463
zcmcgw-EJGl6`tiTrR9*4meR(St)wR8{4{A|^T&}Bv}jH4QX(xbwdPW;i#5unETdGA
z)(a(G22h;b1+0tqjy8C0KSKKmd4%*8QZ(^Z)bH$aD2CDk$wf;*t2uMN{mwZv
zXU@#5>6p#moMTMdFKrkVCsVp*8z%ZB#u+k|NOp)9N$)Jr#N!8yp
zOPg!b-#Xr3J@3QMJgM!ottZ-}t+xN^LvM=_=>C?^IW@H9mQ!lE-6h+oX4O`uYNm=`
zaanB@%?49jn^jZNEH%WG)rwti3XlX4)NrF>H!YT8?5ppSOmQD*Brn`7*UgOGFk2aY
zrR6k>%%>jDr>^$L9@o5n>dT(TdS3GAXu$fjU-sJUFYOfj*MH=%V;?1G@m_8
zAYza+g|R)Ry>f}XA$Q~J(cc4g*WLD_JAP
zJq~wY{rx3kp*I<;TxQcXyIhypJ4`l;)R2u5{%OXA%d#*`Y;O0hM$-UkIAlo-7Wuo#
zUs#iT<})p}%%nAGm+i9H)E;xYSJzAC2rkQdA{doXpuvkC^O%IUw%IoRFUJtC+kMU2
z*c`n$w=nsl%HzvSBVbWoHI30EP7gg=;)`q2H}W?!Q`V5Jji(~x@oDNOf(Jyu3BYkX!+`bhprR@LQ$z@M^WY*;xlsBtOGKtV$j5=HY|el9b0
zSqo@Zi6%cm!($^J%xEM}?0F14DtNStdYnWDw&66TBzwwLq%a+)hnaB?obq)n
zMfE9VRFVXvlr3L}qExpUQc?>QBp9Xorj>D}Qq5Rr`YX0kz8-Tgl5}+BcSz~)Njr>Q
zcVDCK0n_SOLZz0r?fllk+p1J7seC#A%rHb3w`Zu1!1^7nV!Ta3%>PG9TY
z0VsiGQ>(9=Y`gz*?~c?@_u5<;bvtbP@ytjn0+jCE;jvLqY1ku=dJMMhf3mm5HHL-D
z9({@3

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Peter Xu]

On Tue, Aug 22, 2017 at 10:15:56AM +0800, Fam Zheng wrote:
> On Mon, 08/21 18:28, Dr. David Alan Gilbert wrote:
> > > It's not much more than asserting qemu_mutex_iothread_locked(), the 
> > > problem is
> > > the new monitor thread breaks certain assumptions that was true.
> > > 
> > > What is interesting in this is that block layer's nested aio_poll() now 
> > > not only
> > > run in the main thread but also in the monitor thread. Bugs may hide 
> > > there.  :)
> > > 
> > > That's why I suggested a "safe by default" strategy.
> > 
> > OK, that's going to need some more flags somewhere; we've now
> > effectively got three types of command:
> >a) Commands that can only run in the main thread
> >b) Commands that can run in other monitor threads, but must have the bql
> >c) Commands that can run in other monitor threads but don't take the
> >bql
> > 
> >The class (a) that you point out are a pain; arguably if we have to
> > split them up then perhaps we should initially only allow (c).

Good to know that this series may break something; That's really what
I want to know for this RFC post. :-)

I agree with Dave that if we assume (a) unchangable, then the only way
to go is only allow (c) in threaded monitors.

However before that, I am curious about whether we can replace the
assertion with something else like locks?  And how hard would it be?

(Paolo?)

> > 
> > > One step back, is it possible to "unblock" main thread even upon network 
> > > issue?
> > > What is the scenario that causes main thread hang? Is there a backtrace?
> > 
> > There are at least 3 scenarious I know of:
> > 
> >   a) Postcopy: An IO operation takes the lock and accesses guest memory;
> >  the guest memory is missing due to userfault'd memory.
> >  Unfortunately the network connection to the source happens to fail;
> >   so we never receive that page and the thread stays stuck in the 
> > userfault.
> >  We can't issue a recovery command to reopen a network connection
> >  because the monitor is blocked.
> >   b) Postcopy: A monitor command either accesses guest memory or has
> >  to wait on another thread that is doing; e.g. info cpu  waits
> >  for the CPU threads to exit the loop, but they might be blocked
> >  waiting on userfault.
> >   c) COLO or migration: The network fails during the critical bit
> >  at the end of migration when we have the bql held.  You can't
> >  issue a migration_cancel or a colo-failover via the monitor
> >  because it's blocked.
> 
> Thanks for explainaing!
> 
> What commands are in class (c)? From the cover letter it seems 
> migrate-incoming
> is the only one in mind, I'm not sure how it resolves any of the three
> scenarios?

I haven't really encountered (c), but I think it's the migrate_cancel
command that matters, which should not need BQL as well.  For (a) and
(b), they should both need the other migrate_incoming command.

Thanks,

> 
> > 
> > There are other advantages of being able to do bql'less commands;
> > things like an 'info status' or the like should be doable without bql,
> > so just avoding taking the bql when the management layer is doing
> > stuff (or alternatively getting faster replies on management)
> > are both useful.
> 
> Agreed. It is very useful not just for migration.
> 
> Fam

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v6 5/7] qemu.py: use os.path.null instead of /dev/null

[Philippe Mathieu-Daudé]

On 07/31/2017 05:51 AM, Amador Pahim wrote:

For increased portability, let's use os.path.devnull.

Signed-off-by: Amador Pahim 


Reviewed-by: Philippe Mathieu-Daudé 


---
  scripts/qemu.py | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 9434ccc30b..d313c6d4db 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -80,7 +80,7 @@ class QEMUMachine(object):
  fd_param = ["%s" % self._socket_scm_helper,
  "%d" % self._qmp.get_sock_fd(),
  "%s" % fd_file_path]
-devnull = open('/dev/null', 'rb')
+devnull = open(os.path.devnull, 'rb')
  p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
   stderr=sys.stderr)
  return p.wait()
@@ -137,7 +137,7 @@ class QEMUMachine(object):
  
  def launch(self):

  '''Launch the VM and establish a QMP connection'''
-devnull = open('/dev/null', 'rb')
+devnull = open(os.path.devnull, 'rb')
  qemulog = open(self._qemu_log_path, 'wb')
  try:
  self._pre_launch()

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Fam Zheng]

On Mon, 08/21 18:28, Dr. David Alan Gilbert wrote:
> > It's not much more than asserting qemu_mutex_iothread_locked(), the problem 
> > is
> > the new monitor thread breaks certain assumptions that was true.
> > 
> > What is interesting in this is that block layer's nested aio_poll() now not 
> > only
> > run in the main thread but also in the monitor thread. Bugs may hide there. 
> >  :)
> > 
> > That's why I suggested a "safe by default" strategy.
> 
> OK, that's going to need some more flags somewhere; we've now
> effectively got three types of command:
>a) Commands that can only run in the main thread
>b) Commands that can run in other monitor threads, but must have the bql
>c) Commands that can run in other monitor threads but don't take the
>bql
> 
>The class (a) that you point out are a pain; arguably if we have to
> split them up then perhaps we should initially only allow (c).
> 
> > One step back, is it possible to "unblock" main thread even upon network 
> > issue?
> > What is the scenario that causes main thread hang? Is there a backtrace?
> 
> There are at least 3 scenarious I know of:
> 
>   a) Postcopy: An IO operation takes the lock and accesses guest memory;
>  the guest memory is missing due to userfault'd memory.
>  Unfortunately the network connection to the source happens to fail;
>   so we never receive that page and the thread stays stuck in the 
> userfault.
>  We can't issue a recovery command to reopen a network connection
>  because the monitor is blocked.
>   b) Postcopy: A monitor command either accesses guest memory or has
>  to wait on another thread that is doing; e.g. info cpu  waits
>  for the CPU threads to exit the loop, but they might be blocked
>  waiting on userfault.
>   c) COLO or migration: The network fails during the critical bit
>  at the end of migration when we have the bql held.  You can't
>  issue a migration_cancel or a colo-failover via the monitor
>  because it's blocked.

Thanks for explainaing!

What commands are in class (c)? From the cover letter it seems migrate-incoming
is the only one in mind, I'm not sure how it resolves any of the three
scenarios?

> 
> There are other advantages of being able to do bql'less commands;
> things like an 'info status' or the like should be doable without bql,
> so just avoding taking the bql when the management layer is doing
> stuff (or alternatively getting faster replies on management)
> are both useful.

Agreed. It is very useful not just for migration.

Fam

Re: [Qemu-devel] [PATCH] spapr: Add ibm, processor-storage-keys property to CPU DT node

[David Gibson]

On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
> LoPAPR says:
> 
> “ibm,processor-storage-keys”
> 
> property name indicating the number of virtual storage keys supported
> by the processor described by this node.
> 
> prop-encoded-array: Consists of two cells encoded as with encode-int.
> The first cell represents the number of virtual storage keys supported
> for data accesses while the second cell represents the number of
> virtual storage keys supported for instruction accesses. The cell value
> of zero indicates that no storage keys are supported for the access
> type.
> 
> pHyp provides the property above but there's a bug in P8 firmware where the
> second cell is zero even though POWER8 supports instruction access keys.
> This bug will be fixed for P9.
> 
> Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.
> 
> Signed-off-by: Thiago Jung Bauermann 

Regardless of anything else, this clearly won't go in until 2.11 opens.



> ---
> 
> The sysfs files are provided by this patch for Linux:
> 
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-August/162005.html
> 
> I realize that this patch can't be committed before the Linux one goes in,
> but I'd appreciate feedback so that it will be ready by the time the kernel
> side is accepted.
> 
>  hw/ppc/spapr.c | 76 
> ++
>  include/hw/ppc/spapr.h |  6 
>  2 files changed, 82 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f7a19720dcdf..a665e4d830f7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -605,6 +605,80 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> *fdt, int offset,
>pcc->radix_page_info->count *
>sizeof(radix_AP_encodings[0];
>  }
> +
> +if (spapr->storage_keys) {
> +uint32_t val[2];
> +
> +val[0] = cpu_to_be32(spapr->storage_keys);
> +val[1] = spapr->insn_keys ? val[0] : 0;
> +
> +_FDT(fdt_setprop(fdt, offset, "ibm,processor-storage-keys",
> + val, sizeof(val)));
> +}
> +}
> +
> +#define SYSFS_PROT_KEYS_PATH "/sys/kernel/mm/protection_keys/"
> +#define SYSFS_USABLE_STORAGE_KEYS SYSFS_PROT_KEYS_PATH "usable_keys"
> +#define SYSFS_DISABLE_EXEC_KEYS SYSFS_PROT_KEYS_PATH 
> "disable_execute_supported"
> +
> +static void setup_storage_keys(CPUPPCState *env, sPAPRMachineState *spapr)
> +{
> +if (!(env->mmu_model & POWERPC_MMU_AMR))
> +return;
> +
> +if (kvm_enabled()) {
> +char buf[sizeof("false\n")];
> +uint32_t keys;
> +FILE *fd;

For starters, reasonably complex kvm-only code like this should go
into target/ppc/kvm.c.

But, there's a more fundamental problem.  Automatically adjusting
guest visible parameters based on the host's capabilities is really
problematic: if you migrate to a host with different capabilities
what's usable suddenly changes, but the guest can't know.

The usual way to deal with this is instead to have explicit machine
parameters to control the value, and check if those are possible on
the host.  It's then up to the user and/or management layer to set the
parameters suitable to allow migration between all machines that they
care about.


> +/*
> + * With KVM, we allow the guest to use the keys which the kernel 
> tells
> + * us are available.
> + */
> +
> +fd = fopen(SYSFS_USABLE_STORAGE_KEYS, "r");
> +if (!fd) {
> +error_report("%s: open %s failed", __func__,
> + SYSFS_USABLE_STORAGE_KEYS);
> +return;
> +}
> +
> +if (fscanf(fd, "%u", ) != 1) {
> +error_report("%s: error reading %s", __func__,
> + SYSFS_USABLE_STORAGE_KEYS);
> +fclose(fd);
> +return;
> +}
> +
> +fclose(fd);
> +
> +/* Now find out whether the keys can be used for instruction access. 
> */
> +
> +fd = fopen(SYSFS_DISABLE_EXEC_KEYS, "r");
> +if (!fd) {
> +error_report("%s: open %s failed", __func__,
> + SYSFS_USABLE_STORAGE_KEYS);
> +return;
> +}
> +
> +if (!fread(buf, 1, sizeof(buf), fd)) {
> +error_report("%s: error reading %s", __func__,
> + SYSFS_DISABLE_EXEC_KEYS);
> +fclose(fd);
> +return;
> +}
> +
> +fclose(fd);
> +
> +spapr->storage_keys = keys;
> +spapr->insn_keys = !strncmp(buf, "true\n", sizeof(buf));
> +} else {
> +/* Without KVM, all keys provided by the architecture are available. 
> */
> +spapr->storage_keys = 32;
> +
> +/* POWER7 doesn't support instruction access keys. */
> +spapr->insn_keys = POWERPC_MMU_VER(env->mmu_model) != 
> POWERPC_MMU_VER_2_06;
> +}
>  }
> 

Re: [Qemu-devel] [PATCH for-2.10] boot-serial-test: prefer tcg accelerator

[David Gibson]

On Wed, Aug 16, 2017 at 12:18:07PM +0100, Peter Maydell wrote:
> On 16 August 2017 at 11:51, Paolo Bonzini  wrote:
> > On 16/08/2017 10:26, Cornelia Huck wrote:
> >> Prefer to use the tcg accelarator if it is available: This is our only
> >> real smoke test for tcg, and fast enough to use it for that.
> >
> > I'm not sure this is required for 2.10.  Yes, it means the coverage from
> > "make check" is worse, but that's it.
> 
> Yes, I'd put it under "if we need to roll an rc4 anyway for
> some more significant bug we might as well put this in too,
> but it doesn't merit cutting rc4 by itself."

It does entirely break "make check" on a ppc host.  And that in turn
has held up my testing cycle for a couple of ppc regressions from 2.9
that I was hoping to squeeze in.  Does that change your calculations?

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] filter-mirror: segfault when specifying non existent device

[Zhang Chen]

On 08/21/2017 11:50 PM, Eduardo Otubo wrote:

When using filter-mirror like the example below where the interface
'ndev0' does not exist on the host, QEMU crashes into segmentation
fault.

  $ qemu-system-x86_64 -S -machine pc -netdev user,id=ndev0 -object 
filter-mirror,id=test-object,netdev=ndev0

This happens because the function filter_mirror_setup() does not checks
if the device actually exists and still keep on processing calling
qemu_chr_find(). This patch fixes this issue.

Signed-off-by: Eduardo Otubo 


Looks good for me.

Reviewed-by: Zhang Chen

Thanks
Zhang Chen


---
  net/filter-mirror.c | 14 +++---
  1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 90e2c92337..e18a4b16a0 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -213,14 +213,22 @@ static void filter_mirror_setup(NetFilterState *nf, Error 
**errp)
  MirrorState *s = FILTER_MIRROR(nf);
  Chardev *chr;
  
+if (s->outdev == NULL) {

+goto err;
+}
+
  chr = qemu_chr_find(s->outdev);
+
  if (chr == NULL) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-  "Device '%s' not found", s->outdev);
-return;
+goto err;
  }
  
  qemu_chr_fe_init(>chr_out, chr, errp);

+
+err:
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found",
+  nf->netdev_id);
+return;
  }
  
  static void redirector_rs_finalize(SocketReadState *rs)


--
Thanks
Zhang Chen

[Qemu-devel] 答复: Re: 答复: Re: [PATCH] vhost: don't set vring call fd to -1 invhost_virtqueue_start for vhost-user

[lu.zhipeng]

>On 2017年08月21日 11:28, lu.zhip...@zte.com.cn wrote:


>>   if (!vdev->use_guest_notifier_mask) {>>> >/* TODO: check and 
>> handle errors. */>>> >  
>> -- set right  callfd>>  }>  if 
>> (k->query_guest_notifiers &&>>  
>> k->query_guest_notifiers(qbus->parent) &&>>  
>> virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {>>  file.fd = 
>> -1>>  r = dev->vhost_ops->vhost_set_vring_call(dev, )>> 
>> ---set call fd to -1>>  if (r) {>>  goto 
>> fail_vector>>  }>>  }>>>   So   the callfd of  ovs+dpdk in host  
>> is -1.  it  can't be able  to interact with the guest.>The queue does 
>> not even an vector, so it won't generate any interrupt. >Why callfd is still 
>> needed?




it's a bug of my vhost-user backend. Thanks.




为了让您的VPlat虚拟机故障和docker故障得到高效的处理，请上报故障到: $VPlat技术支持。


芦志朋 luzhipeng






IT开发工程师 IT Development
Engineer
操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R＆D Institute/System Product









四川省成都市天府大道中段800号
E: lu.zhip...@zte.com.cn 
www.zte.com.cn










原始邮件



发件人： 
收件人：芦志朋10108272
抄送人：  
日 期 ：2017年08月21日 11:35
主 题 ：Re: [Qemu-devel] 答复: Re: [PATCH] vhost: don't set vring call fd to -1 
invhost_virtqueue_start for vhost-user







On 2017年08月21日 11:28, lu.zhip...@zte.com.cn wrote:
>   if (!vdev->use_guest_notifier_mask) {
>
>  /* TODO: check and handle errors. */
>
>  vhost_virtqueue_mask(dev, vdev, idx, false)
>
>  -- set right  callfd
>
>  }
>
>
>
>
>  if (k->query_guest_notifiers &&
>
>  k->query_guest_notifiers(qbus->parent) &&
>
>  virtio_queue_vector(vdev, idx) == VIRTIO_NO_VECTOR) {
>
>  file.fd = -1
>
>  r = dev->vhost_ops->vhost_set_vring_call(dev, )
>
> ---set call fd to -1
>
>  if (r) {
>
>  goto fail_vector
>
>  }
>
>  }
>
>   So   the callfd of  ovs+dpdk in host  is -1.  it  can't be able  to 
> interact with the guest.
>
>

The queue does not even an vector, so it won't generate any interrupt. 
Why callfd is still needed?

Thanks

Re: [Qemu-devel] [PATCH v7 03/16] migration: split common postcopy out of ram postcopy

[John Snow]

On 07/11/2017 09:38 AM, Vladimir Sementsov-Ogievskiy wrote:
> 11.07.2017 16:06, Dr. David Alan Gilbert wrote:
>> * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote:
>>> Split common postcopy staff from ram postcopy staff.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> I'm OK with this; I'm not sure I'd have bothered making the ping's
>> dependent on it being RAM.
>>
>> (These first few are pretty much a separable series).
> 
> It would be grate if you (or Juan?) can take them separately.
> 

Yes please. I don't think this ever happened, did it? Can we split off
1-3 and re-roll?

Re: [Qemu-devel] [PATCH v7 0/4] Add shrink image for qcow2

[John Snow]

On 08/17/2017 05:15 AM, Pavel Butsykin wrote:
> This patch add shrinking of the image file for qcow2. As a result, this allows
> us to reduce the virtual image size and free up space on the disk without
> copying the image. Image can be fragmented and shrink is done by punching 
> holes
> in the image file.
> 
> # ./qemu-img create -f qcow2 image.qcow2 4G
> Formatting 'image.qcow2', fmt=qcow2 size=4294967296 encryption=off 
> cluster_size=65536 lazy_refcounts=off refcount_bits=16
> 
> # ./qemu-io -c "write -P 0x22 0 1G" image.qcow2
> wrote 1073741824/1073741824 bytes at offset 0
> 1 GiB, 1 ops; 0:00:04.59 (222.886 MiB/sec and 0.2177 ops/sec)
> 
> # ./qemu-img resize image.qcow2 512M
> warning: qemu-img: Shrinking an image will delete all data beyond the 
> shrunken image's end. Before performing such an operation, make sure there is 
> no important data there.
> error: qemu-img: Use the --shrink option to perform a shrink operation.
> 
> # ./qemu-img resize --shrink image.qcow2 128M
> Image resized.
> 
> # ./qemu-img info image.qcow2
> image: image.qcow2
> file format: qcow2
> virtual size: 128M (134217728 bytes)
> disk size: 128M
> cluster_size: 65536
> Format specific information:
> compat: 1.1
> lazy refcounts: false
> refcount bits: 16
> corrupt: false
> 
> # du -h image.qcow2
> 129Mimage.qcow2
> 

It looks sane to me, and already has a full set of R-Bs from Max. Are we
waiting for Kevin?

It looks like in Vladimir's series we sidestepped the problem of what
happens if we resize with persistent bitmaps: we deny the operation,
regardless of if we are shrinking, growing, or have bitmaps that are
read-only, frozen, or what-have-you.

It shouldn't be too hard to add soon, but this is fine for now.
(I think for adding it we just need to make sure the bitmaps aren't
frozen and are not read-only, and the implementation bitmap structure
can already handle truncation in either direction just fine.)

Anyway;

Reviewed-by: John Snow 


> Changes from v1:
> - add --shrink flag for qemu-img resize
> - add qcow2_cache_discard
> - simplify qcow2_shrink_l1_table() to reduce the likelihood of image 
> corruption
> - add new qemu-iotests for shrinking images
> 
> Changes from v2:
> - replace qprintf() on error_report() (1)
> - rewrite warning messages (1)
> - enforce --shrink flag for all formats except raw (1)
> - split qcow2_cache_discard() (2)
> - minor fixes according to comments (3)
> - rewrite the last part of qcow2_shrink_reftable() to avoid
>   qcow2_free_clusters() calls inside (3)
> - improve test for shrinking image (4)
> 
> Changes from v3:
> - rebase on "Implement a warning_report function" Alistair's patch-set (1)
> - spelling fixes (1)
> - the man page fix according to the discussion (1)
> - add call qcow2_signal_corruption() in case of image corruption (3)
> 
> Changes from v4:
> - rebase on https://github.com/XanClic/qemu/commits/block Max's block branch
> 
> Changes from v5:
> - the condition refcount == 0 should be enough to evict the l2/refcount 
> cluster
>   from the cache (2)
> - overwrite the l1/refcount table in memory with zeros, even if overwriting 
> the
>   l1/refcount table on disk has failed (3)
> - replace g_try_malloc() on g_malloc() for allocation reftable_tmp (3)
> 
> Changes from v6:
> - rebase on master 1f29673387
> 
> Pavel Butsykin (4):
>   qemu-img: add --shrink flag for resize
>   qcow2: add qcow2_cache_discard
>   qcow2: add shrink image support
>   qemu-iotests: add shrinking image test
> 
>  block/qcow2-cache.c|  26 +++
>  block/qcow2-cluster.c  |  50 +
>  block/qcow2-refcount.c | 140 -
>  block/qcow2.c  |  43 +---
>  block/qcow2.h  |  17 +
>  qapi/block-core.json   |   3 +-
>  qemu-img-cmds.hx   |   4 +-
>  qemu-img.c |  23 ++
>  qemu-img.texi  |   6 +-
>  tests/qemu-iotests/102 |   4 +-
>  tests/qemu-iotests/163 | 170 
> +
>  tests/qemu-iotests/163.out |   5 ++
>  tests/qemu-iotests/group   |   1 +
>  13 files changed, 475 insertions(+), 17 deletions(-)
>  create mode 100644 tests/qemu-iotests/163
>  create mode 100644 tests/qemu-iotests/163.out
> 

Re: [Qemu-devel] [PATCH 2/2 v3] xenfb: Add [feature|request]-raw-pointer

[Stefano Stabellini]

On Wed, 26 Jul 2017, Owen Smith wrote:
> Writes "feature-raw-pointer" during init to indicate the backend
> can pass raw unscaled values for absolute axes to the frontend.
> Frontends set "request-raw-pointer" to indicate the backend should
> not attempt to scale absolute values to console size.
> "request-raw-pointer" is only valid if "request-abs-pointer" is
> also set. Raw unscaled pointer values are in the range [0, 0x7fff]
> 
> "feature-raw-pointer" and "request-raw-pointer" added to Xen
> header in commit 7868654ff7fe5e4a2eeae2b277644fa884a5031e
> 
> Signed-off-by: Owen Smith 

Reviewed-by: Stefano Stabellini 


> ---
>  hw/display/xenfb.c | 37 -
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index e412753..f397f9a 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -50,6 +50,7 @@ struct common {
>  struct XenInput {
>  struct common c;
>  int abs_pointer_wanted; /* Whether guest supports absolute pointer */
> +int raw_pointer_wanted; /* Whether guest supports raw (unscaled) pointer 
> */
>  QemuInputHandlerState *qkbd;
>  QemuInputHandlerState *qmou;
>  int mouse_axes[INPUT_AXIS__MAX];
> @@ -380,21 +381,23 @@ static void xenfb_mouse_sync(DeviceState *dev)
>  in->abs_pointer_wanted);
>  
>  if (in->abs_pointer_wanted) {
> -QemuConsole *con = qemu_console_lookup_by_index(0);
> -DisplaySurface *surface;
> -int dw, dh;
> -
> -if (!con) {
> -xen_pv_printf(>c.xendev, 0, "No QEMU console available");
> -return;
> -}
> +if (!in->raw_pointer_wanted) {
> +QemuConsole *con = qemu_console_lookup_by_index(0);
> +DisplaySurface *surface;
> +int dw, dh;
> +
> +if (!con) {
> +xen_pv_printf(>c.xendev, 0, "No QEMU console available");
> +return;
> +}
>  
> -surface = qemu_console_surface(con);
> -dw = surface_width(surface);
> -dh = surface_height(surface);
> +surface = qemu_console_surface(con);
> +dw = surface_width(surface);
> +dh = surface_height(surface);
>  
> -dx = dx * (dw - 1) / 0x7fff;
> -dy = dy * (dh - 1) / 0x7fff;
> +dx = dx * (dw - 1) / 0x7fff;
> +dy = dy * (dh - 1) / 0x7fff;
> +}
>  
>  xenfb_send_position(in, dx, dy, dz);
>  } else {
> @@ -428,6 +431,7 @@ static QemuInputHandler xenfb_rel_mouse = {
>  static int input_init(struct XenDevice *xendev)
>  {
>  xenstore_write_be_int(xendev, "feature-abs-pointer", 1);
> +xenstore_write_be_int(xendev, "feature-raw-pointer", 1);
>  return 0;
>  }
>  
> @@ -451,6 +455,13 @@ static void input_connected(struct XenDevice *xendev)
>   >abs_pointer_wanted) == -1) {
>  in->abs_pointer_wanted = 0;
>  }
> +if (xenstore_read_fe_int(xendev, "request-raw-pointer",
> + >raw_pointer_wanted) == -1) {
> +in->raw_pointer_wanted = 0;
> +}
> +if (in->raw_pointer_wanted && !in->abs_pointer_wanted) {
> +xen_pv_printf(xendev, 0, "raw pointer set without absolute 
> pointer.");
> +}
>  
>  if (in->qkbd) {
>  qemu_input_handler_unregister(in->qkbd);
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 1/2 v3] xenfb: Use Input Handlers directly

[Stefano Stabellini]

Anthony,

The code looks good. I tested this patch with Linux guests and seems to
work OK, can you also confirm?

One comment below.


On Wed, 26 Jul 2017, Owen Smith wrote:
> Avoid the unneccessary calls through the input-legacy.c file by
> using the qemu_input_handler_*() calls directly. This did require
> reworking the event and sync handlers and a direct mapping from
> QEMU's qcodes to linux KEY_* identifiers required by the ring
> protocol. Removes the scancode2linux mapping, and supporting
> documention.
> 
> Signed-off-by: Owen Smith 
> ---
>  hw/display/xenfb.c | 401 
> +
>  1 file changed, 247 insertions(+), 154 deletions(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index df8b78f..e412753 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -27,6 +27,7 @@
>  #include "qemu/osdep.h"
>  
>  #include "hw/hw.h"
> +#include "ui/input.h"
>  #include "ui/console.h"
>  #include "hw/xen/xen_backend.h"
>  
> @@ -37,9 +38,7 @@
>  
>  #include "trace.h"
>  
> -#ifndef BTN_LEFT
> -#define BTN_LEFT 0x110 /* from  */
> -#endif
> +#include "standard-headers/linux/input.h"
>  
>  /*  */
>  
> @@ -51,9 +50,10 @@ struct common {
>  struct XenInput {
>  struct common c;
>  int abs_pointer_wanted; /* Whether guest supports absolute pointer */
> -int button_state;   /* Last seen pointer button state */
> -int extended;
> -QEMUPutMouseEntry *qmouse;
> +QemuInputHandlerState *qkbd;
> +QemuInputHandlerState *qmou;
> +int mouse_axes[INPUT_AXIS__MAX];
> +int mouse_wheel;
>  };
>  
>  #define UP_QUEUE 8
> @@ -120,77 +120,125 @@ static void common_unbind(struct common *c)
>  
>  /*  */
>  
> -#if 0
> -/*
> - * These two tables are not needed any more, but left in here
> - * intentionally as documentation, to show how scancode2linux[]
> - * was generated.
> - *
> - * Tables to map from scancode to Linux input layer keycode.
> - * Scancodes are hardware-specific.  These maps assumes a
> - * standard AT or PS/2 keyboard which is what QEMU feeds us.
> - */
> -const unsigned char atkbd_set2_keycode[512] = {
> -
> - 0, 67, 65, 63, 61, 59, 60, 88,  0, 68, 66, 64, 62, 15, 41,117,
> - 0, 56, 42, 93, 29, 16,  2,  0,  0,  0, 44, 31, 30, 17,  3,  0,
> - 0, 46, 45, 32, 18,  5,  4, 95,  0, 57, 47, 33, 20, 19,  6,183,
> - 0, 49, 48, 35, 34, 21,  7,184,  0,  0, 50, 36, 22,  8,  9,185,
> - 0, 51, 37, 23, 24, 11, 10,  0,  0, 52, 53, 38, 39, 25, 12,  0,
> - 0, 89, 40,  0, 26, 13,  0,  0, 58, 54, 28, 27,  0, 43,  0, 85,
> - 0, 86, 91, 90, 92,  0, 14, 94,  0, 79,124, 75, 71,121,  0,  0,
> -82, 83, 80, 76, 77, 72,  1, 69, 87, 78, 81, 74, 55, 73, 70, 99,
> -
> -  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
> -217,100,255,  0, 97,165,  0,  0,156,  0,  0,  0,  0,  0,  0,125,
> -173,114,  0,113,  0,  0,  0,126,128,  0,  0,140,  0,  0,  0,127,
> -159,  0,115,  0,164,  0,  0,116,158,  0,150,166,  0,  0,  0,142,
> -157,  0,  0,  0,  0,  0,  0,  0,155,  0, 98,  0,  0,163,  0,  0,
> -226,  0,  0,  0,  0,  0,  0,  0,  0,255, 96,  0,  0,  0,143,  0,
> -  0,  0,  0,  0,  0,  0,  0,  0,  0,107,  0,105,102,  0,  0,112,
> -110,111,108,112,106,103,  0,119,  0,118,109,  0, 99,104,119,  0,
> -
> -};
> -
> -const unsigned char atkbd_unxlate_table[128] = {
> -
> -  0,118, 22, 30, 38, 37, 46, 54, 61, 62, 70, 69, 78, 85,102, 13,
> - 21, 29, 36, 45, 44, 53, 60, 67, 68, 77, 84, 91, 90, 20, 28, 27,
> - 35, 43, 52, 51, 59, 66, 75, 76, 82, 14, 18, 93, 26, 34, 33, 42,
> - 50, 49, 58, 65, 73, 74, 89,124, 17, 41, 88,  5,  6,  4, 12,  3,
> - 11,  2, 10,  1,  9,119,126,108,117,125,123,107,115,116,121,105,
> -114,122,112,113,127, 96, 97,120,  7, 15, 23, 31, 39, 47, 55, 63,
> - 71, 79, 86, 94,  8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 87,111,
> - 19, 25, 57, 81, 83, 92, 95, 98, 99,100,101,103,104,106,109,110
> -
> +static const unsigned int keymap_qcode[Q_KEY_CODE__MAX] = {
> +[Q_KEY_CODE_ESC] = KEY_ESC,
> +[Q_KEY_CODE_1]   = KEY_1,
> +[Q_KEY_CODE_2]   = KEY_2,
> +[Q_KEY_CODE_3]   = KEY_3,
> +[Q_KEY_CODE_4]   = KEY_4,
> +[Q_KEY_CODE_5]   = KEY_5,
> +[Q_KEY_CODE_6]   = KEY_6,
> +[Q_KEY_CODE_7]   = KEY_7,
> +[Q_KEY_CODE_8]   = KEY_8,
> +[Q_KEY_CODE_9]   = KEY_9,
> +[Q_KEY_CODE_0]   = KEY_0,
> +[Q_KEY_CODE_MINUS]   = KEY_MINUS,
> +[Q_KEY_CODE_EQUAL]   = KEY_EQUAL,
> +[Q_KEY_CODE_BACKSPACE]   = KEY_BACKSPACE,
> +
> +[Q_KEY_CODE_TAB] = KEY_TAB,
> +[Q_KEY_CODE_Q]   = KEY_Q,
> +[Q_KEY_CODE_W]  

Re: [Qemu-devel] [PATCH] xen: Emit RTC_CHANGE upon TIMEOFFSET ioreq

[Stefano Stabellini]

On Mon, 21 Aug 2017, Ross Lagerwall wrote:
> When the guest writes to the RTC, Xen emulates it and broadcasts a
> TIMEOFFSET ioreq. Emit an RTC_CHANGE QMP message when this happens
> rather than ignoring it so that something useful can be done with the
> information.

Are there any handlers of the RTC_CHANGE QMP message today? What happens
if there are no handlers?

In other words, does this patch change the existing behavior? If so,
please describe, otherwise, please state that there are no behavioral
changes.


> Signed-off-by: Ross Lagerwall 
> ---
>  hw/i386/xen/xen-hvm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index d9ccd5d..ffd20dc 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -16,6 +16,7 @@
>  #include "hw/i386/apic-msidef.h"
>  #include "hw/xen/xen_common.h"
>  #include "hw/xen/xen_backend.h"
> +#include "qapi-event.h"
>  #include "qmp-commands.h"
>  
>  #include "qemu/error-report.h"
> @@ -967,6 +968,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
>  handle_vmport_ioreq(state, req);
>  break;
>  case IOREQ_TYPE_TIMEOFFSET:
> +qapi_event_send_rtc_change((int64_t)req->data, _abort);
>  break;
>  case IOREQ_TYPE_INVALIDATE:
>  xen_invalidate_map_cache();
> -- 
> 2.9.5
> 

Re: [Qemu-devel] [PATCH v6 00/10] qemu.py: Pylint/style fixes

[John Snow]

On 08/18/2017 10:26 AM, Lukáš Doktor wrote:
> Hello guys,
> 
> I'm reading the available python modules to exercise qemu and while reading 
> them
> I fixed some issues that caught my attention. It usually starts with a simple
> pylint/docstring fixes and slowly graduates to more controversial ones so I'm
> open to suggestion to remove some of them.
> 
> Kind regards,
> Lukáš
> 
> Changes in v2
> - Squashed 2nd and 10th patches into 2nd one
> - Use repr() in MonitorResponseError's description
> - Improved commit message of the 6th patch
> - Two tweaks to docstrings changed in the 6th patch
> - Also updated qmp-shell to use new-style super calls (7th patch)
> - Fixed the accidental rename of qmp `cmd_id` (kept the id->cmd_id patch)
> - Changed the style of the style-fix in the 10th commit
> 
> Changes in v3
> - Don't use repr in the 5th patch in MonitorResponseError
> 
> Changes in v4
> - Use correct git base (remove unwanted commits)
> 
> Changes in v5
> - Avoid bool comparison
> - Change report to return in one docstring
> - Removed the unnecessary spaces around single-line docstring
> 
> Changes in v6
> - Bunch of docstring tweaks by Markus Armbruster
> - Line break in <80 chars
> - result dict => response dict
> - Removed the "event_match" rename
> 

Looks like all ten patches have an R-B despite changes; but it looks
like nothing particularly major was changed anyway.

Does this fall under Markus's jurisdiction?

(Well, except for qtest.py which seemingly has double-extra-no
maintainer...!)

--js

Re: [Qemu-devel] [RFC v4 08/13] ide: enumerate_slots implementation

[John Snow]

On 08/18/2017 12:57 PM, Eduardo Habkost wrote:
> On Wed, Aug 16, 2017 at 05:46:18PM -0400, John Snow wrote:
>>
>>
>> On 08/14/2017 05:57 PM, Eduardo Habkost wrote:
>>> Example output when using "-machine q35":
>>>
>>>   {
>>> "available": true,
>>> "count": 1,
>>> "device-types": [
>>>   "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>>   { "option": "unit", "values": 0 },
>>>   { "option": "bus", "values": "ide.2" }
>>> ],
>>> "opts-complete": true
>>>   }
>>>   {
>>> "available": false,
>>> "count": 1,
>>> "device": "/machine/unattached/device[19]",
>>> "device-types": [
>>>   "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>>   { "option": "unit", "values": 1 },
>>>   { "option": "bus", "values": "ide.2" } ],
>>> "opts-complete": true
>>>   }
>>>   {
>>> "available": true,
>>> "count": 10,
>>> "device-types": [
>>>   "ide-device"
>>> ],
>>> "hotpluggable": false,
>>> "opts": [
>>>   { "option": "unit", "values": [ [ 0, 1 ] ] },
>>
>> Hm, these unit values aren't really correct -- we do not support
>> primary/secondary semantics for IDE buses on the AHCI device. I guess
>> they technically exist, but you cannot use them for anything.
>>
>> Should I do something to "disable" or otherwise hide the unusable
>> secondary unit slots for AHCI devices?
> 
> If the device is already rejecting -device ...,unit=1, then the
> bug is in my implementation of enumerate_devices.  Maybe it
> should just look at IDEBus::max_units to find that out?
> 

jhuston@probe (master) ~/s/q/b/git>
./x86_64-softmmu/qemu-system-x86_64 \
-M q35 \
-nodefaults \
-device ich9-ahci,id=ahci0 \
-drive id=bar,file=/media/ext/img/f25.qcow2,if=none \
-device ide-hd,id=foo,bus=ahci0.0,drive=bar,unit=1 \
-vga std

qemu-system-x86_64: -device ide-hd,id=foo,bus=ahci0.0,drive=bar,unit=1:
Can't create IDE unit 1, bus supports only 1 units
qemu-system-x86_64: -device ide-hd,id=foo,bus=ahci0.0,drive=bar,unit=1:
Device initialization failed.

based on;

IDEBus.max_units

as seen in:

hw/ide/qdev.c line 93.


Seems a bit like a hack on IDE's end -- but if your enumerate devices
code has a call-in inside of the IDE layer, please do check the
IDEBus::max_units property.

--js

Re: [Qemu-devel] [PATCH 09/15] hw/ide: Emulate SiI3112 SATA controller

[John Snow]

On 08/20/2017 01:23 PM, BALATON Zoltan wrote:
> This is a common generic PCI SATA conroller that is also used in PCs
> but more importantly guests running on the Sam460ex board prefer this
> card and have a driver for it (unlike for other SATA controllers
> already emulated).
> 

Oh, interesting. This is basically an alternative to the PCI BMDMA
interface and not really an alternative to AHCI. It doesn't really seem
to use any of the SATA-specific interfaces to SATA drives (cough, not
that we currently emulate a difference in QEMU... ATA and SATA both are
simply IDEState*) so this really seems like another PCI IDE interface
akin to the PCI BMDMA adapter we already have.

It's just that guests will think they're using SATA, except... not. Not
a big deal, *I think*...

...It isn't a problem that our support for NCQ commands is tied to AHCI,
is it? Some of our current "SATA" support is tied very directly to AHCI
(see is_ncq() in ahci.c) -- is that relevant here?

> Signed-off-by: BALATON Zoltan 

I'd like to invite you to create a Sam460ex MAINTAINERS stanza, add
yourself, and add hw/ide/sii3112.c to that stanza.

--js

> ---
>  hw/ide/Makefile.objs |   1 +
>  hw/ide/sii3112.c | 369 
> +++
>  2 files changed, 370 insertions(+)
>  create mode 100644 hw/ide/sii3112.c
> 
> diff --git a/hw/ide/Makefile.objs b/hw/ide/Makefile.objs
> index 729e9bd..76f3d6d 100644
> --- a/hw/ide/Makefile.objs
> +++ b/hw/ide/Makefile.objs
> @@ -10,3 +10,4 @@ common-obj-$(CONFIG_IDE_VIA) += via.o
>  common-obj-$(CONFIG_MICRODRIVE) += microdrive.o
>  common-obj-$(CONFIG_AHCI) += ahci.o
>  common-obj-$(CONFIG_AHCI) += ich.o
> +common-obj-$(CONFIG_IDE_SII3112) += sii3112.o
> diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
> new file mode 100644
> index 000..9ec8cd1
> --- /dev/null
> +++ b/hw/ide/sii3112.c
> @@ -0,0 +1,369 @@
> +/*
> + * QEMU SiI3112A PCI to Serial ATA Controller Emulation
> + *
> + * Copyright (C) 2017 BALATON Zoltan 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +/* For documentation on this and similar cards see:
> + * http://wiki.osdev.org/User:Quok/Silicon_Image_Datasheets
> + */

Thank you so much for including docs!

> +
> +#include 
> +#include 
> +
> +#ifdef DEBUG_IDE
> +#define DPRINTF(fmt, ...) fprintf(stderr, fmt, ## __VA_ARGS__);
> +#else
> +#define DPRINTF(fmt, ...)
> +#endif /* DEBUG */
> +

Please use trace events instead of DPRINTF; I'm in the process of
removing them from the rest of IDE.

> +#define TYPE_SII3112_PCI "sii3112"
> +#define SII3112_PCI(obj) OBJECT_CHECK(SiI3112PCIState, (obj), \
> + TYPE_SII3112_PCI)
> +
> +typedef struct SiI3112Regs {
> +uint32_t confstat;
> +uint32_t scontrol;
> +uint16_t sien;
> +uint8_t swdata;
> +} SiI3112Regs;
> +
> +typedef struct SiI3112PCIState {
> +PCIIDEState i;
> +MemoryRegion mmio;
> +SiI3112Regs regs[2];
> +} SiI3112PCIState;
> +
> +static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
> +unsigned int size)
> +{
> +SiI3112PCIState *d = opaque;
> +uint64_t val = 0;
> +
> +switch (addr) {
> +case 0x00:
> +val = d->i.bmdma[0].cmd;
> +break;
> +case 0x01:
> +val = d->regs[0].swdata;
> +break;
> +case 0x02:
> +val = d->i.bmdma[0].status;
> +break;
> +case 0x03:
> +val = 0;
> +break;
> +case 0x04 ... 0x07:
> +val = bmdma_addr_ioport_ops.read(>i.bmdma[0], addr - 4, size);
> +break;
> +case 0x08:
> +val = d->i.bmdma[1].cmd;
> +break;
> +case 0x09:
> +val = d->regs[1].swdata;
> +break;
> +case 0x0a:
> +val = d->i.bmdma[1].status;
> +break;
> +case 0x0b:
> +val = 0;
> +break;
> +case 0x0c ... 0x0f:
> +val = bmdma_addr_ioport_ops.read(>i.bmdma[1], addr - 12, size);
> +break;
> +case 0x10:
> +val = d->i.bmdma[0].cmd;
> +val |= (d->regs[0].confstat & (1UL << 11) ? (1 << 4) : 0); 
> /*SATAINT0*/
> +val |= (d->regs[1].confstat & (1UL << 11) ? (1 << 6) : 0); 
> /*SATAINT1*/
> +val |= (d->i.bmdma[1].status & BM_STATUS_INT ? (1 << 14) : 0);
> +val |= d->i.bmdma[0].status << 16;
> +val |= d->i.bmdma[1].status << 24;
> +break;
> +case 0x18:
> +val = d->i.bmdma[1].cmd;
> +val |= (d->regs[1].confstat & (1UL << 11) ? (1 << 4) : 0);
> +val |= d->i.bmdma[1].status << 16;
> +break;
> +case 0x80 ... 0x87:
> +if (size == 1) {
> +val = ide_ioport_read(>i.bus[0], addr - 0x80);
> +} else if (addr == 0x80) {
> +val = (size == 2) ? ide_data_readw(>i.bus[0], 0) :
> +ide_data_readl(>i.bus[0], 0);
> +} else {
> + 

[Qemu-devel] [PATCH] audio: intel-hda: do not use old_mmio accesses

[Matt Parker]

intel-hda is still using the old_mmio accessors for io.
This updates the device to use .read and .write accessors instead.
---
 hw/audio/intel-hda.c | 56 +---
 1 file changed, 9 insertions(+), 47 deletions(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 06acc98f7b..c0f002f744 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1043,66 +1043,28 @@ static void intel_hda_regs_reset(IntelHDAState *d)
 
 /* - */
 
-static void intel_hda_mmio_writeb(void *opaque, hwaddr addr, uint32_t val)
+static void intel_hda_mmio_write(void *opaque, hwaddr addr, uint64_t val, 
unsigned size)
 {
 IntelHDAState *d = opaque;
 const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
 
-intel_hda_reg_write(d, reg, val, 0xff);
+intel_hda_reg_write(d, reg, val, (1UL << (size * 8)) - 1);
 }
 
-static void intel_hda_mmio_writew(void *opaque, hwaddr addr, uint32_t val)
+static uint64_t intel_hda_mmio_read(void *opaque, hwaddr addr, unsigned size)
 {
 IntelHDAState *d = opaque;
 const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
 
-intel_hda_reg_write(d, reg, val, 0x);
-}
-
-static void intel_hda_mmio_writel(void *opaque, hwaddr addr, uint32_t val)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-intel_hda_reg_write(d, reg, val, 0x);
-}
-
-static uint32_t intel_hda_mmio_readb(void *opaque, hwaddr addr)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-return intel_hda_reg_read(d, reg, 0xff);
-}
-
-static uint32_t intel_hda_mmio_readw(void *opaque, hwaddr addr)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-return intel_hda_reg_read(d, reg, 0x);
-}
-
-static uint32_t intel_hda_mmio_readl(void *opaque, hwaddr addr)
-{
-IntelHDAState *d = opaque;
-const IntelHDAReg *reg = intel_hda_reg_find(d, addr);
-
-return intel_hda_reg_read(d, reg, 0x);
+return intel_hda_reg_read(d, reg, (1UL << (size * 8)) - 1);
 }
 
 static const MemoryRegionOps intel_hda_mmio_ops = {
-.old_mmio = {
-.read = {
-intel_hda_mmio_readb,
-intel_hda_mmio_readw,
-intel_hda_mmio_readl,
-},
-.write = {
-intel_hda_mmio_writeb,
-intel_hda_mmio_writew,
-intel_hda_mmio_writel,
-},
+.read = intel_hda_mmio_read,
+.write = intel_hda_mmio_write,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 4,
 },
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
-- 
2.13.2

[Qemu-devel] [PATCH 0/2] ui/cocoa.m: enable guest to see control-alt key combinations

[John Arbuckle]

Currently if the user needs to send a control-alt key combination, he or she 
was either out of luck or had to rely on the monitor's sendkey command to do 
so. With this patch the user can now directly send control-alt key 
combinations. This is great for Windows guest that may need the 
control-alt-delete key combination. 

John Arbuckle (2):
  move ungrab to ctrl-alt-g
  send ctrl-alt key combinations to guest if not used by QEMU

 ui/cocoa.m | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

-- 
2.11.0 (Apple Git-81)

[Qemu-devel] [PATCH 2/2] ui/cocoa.m: send ctrl-alt key combinations to guest if not used by QEMU

[John Arbuckle]

Send control-alt key combinations to the guest if not used by the user 
interface.

Signed-off-by: John Arbuckle 
---
 ui/cocoa.m | 8 
 1 file changed, 8 insertions(+)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index d3e7907103..6920ea38aa 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -634,6 +634,14 @@ - (void) handleEvent:(NSEvent *)event
 case Q_KEY_CODE_G:
 [self ungrabMouse];
 break;
+
+// send to the guest
+default:
+if (qemu_console_is_graphic(NULL)) {
+qemu_input_event_send_key_qcode(dcl->con, keycode,
+true);
+}
+break;
 }
 
 // handle keys for graphic console
-- 
2.11.0 (Apple Git-81)

[Qemu-devel] [PATCH 1/2] ui/cocoa.m: move ungrab to ctrl-alt-g

[John Arbuckle]

Currently the cocoa user interface relys on the user pushing control-alt to 
ungrab the mouse. This is patch changes the key combination to control-alt-g to 
be in line with the GTK user interface. 

Signed-off-by: John Arbuckle 
---
 ui/cocoa.m | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 93e56d0518..d3e7907103 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -609,10 +609,6 @@ - (void) handleEvent:(NSEvent *)event
 }
 }
 
-// release Mouse grab when pressing ctrl+alt
-if (([event modifierFlags] & NSEventModifierFlagControl) && 
([event modifierFlags] & NSEventModifierFlagOption)) {
-[self ungrabMouse];
-}
 break;
 case NSEventTypeKeyDown:
 keycode = cocoa_keycode_to_qemu([event keyCode]);
@@ -625,7 +621,7 @@ - (void) handleEvent:(NSEvent *)event
 
 // default
 
-// handle control + alt Key Combos (ctrl+alt is reserved for QEMU)
+// handle control + alt Key Combos (ctrl+alt+[1..9,g] is reserved 
for QEMU)
 if (([event modifierFlags] & NSEventModifierFlagControl) && 
([event modifierFlags] & NSEventModifierFlagOption)) {
 switch (keycode) {
 
@@ -633,6 +629,11 @@ - (void) handleEvent:(NSEvent *)event
 case Q_KEY_CODE_1 ... Q_KEY_CODE_9: // '1' to '9' keys
 console_select(keycode - 11);
 break;
+
+// release the mouse grab
+case Q_KEY_CODE_G:
+[self ungrabMouse];
+break;
 }
 
 // handle keys for graphic console
@@ -806,9 +807,9 @@ - (void) grabMouse
 
 if (!isFullscreen) {
 if (qemu_name)
-[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - 
(Press ctrl + alt to release Mouse)", qemu_name]];
+[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - 
(Press ctrl + alt + g to release Mouse)", qemu_name]];
 else
-[normalWindow setTitle:@"QEMU - (Press ctrl + alt to release 
Mouse)"];
+[normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release 
Mouse)"];
 }
 [self hideCursor];
 if (!isAbsoluteEnabled) {
-- 
2.11.0 (Apple Git-81)

Re: [Qemu-devel] [PATCH v14 3/5] virtio-balloon: VIRTIO_BALLOON_F_SG

[Michael S. Tsirkin]

On Fri, Aug 18, 2017 at 03:39:27PM +0800, Wei Wang wrote:
> On 08/18/2017 10:22 AM, Michael S. Tsirkin wrote:
> > +static void send_balloon_page_sg(struct virtio_balloon *vb,
> > +struct virtqueue *vq,
> > +void *addr,
> > +uint32_t size)
> > +{
> > +   unsigned int len;
> > +   int ret;
> > +
> > +   do {
> > +   ret = add_one_sg(vq, addr, size);
> > +   virtqueue_kick(vq);
> > +   wait_event(vb->acked, virtqueue_get_buf(vq, ));
> > +   /*
> > +* It is uncommon to see the vq is full, because the sg is sent
> > +* one by one and the device is able to handle it in time. But
> > +* if that happens, we go back to retry after an entry gets
> > +* released.
> > +*/
> > Why send one by one though? Why not batch some s/gs and wait for all
> > of them to be completed? If memory if fragmented, waiting every time is
> > worse than what we have now (VIRTIO_BALLOON_ARRAY_PFNS_MAX at a time).
> > 
> 
> OK, I'll do batching in some fashion.
> 
> 
> Best,
> Wei
> 
> 

btw you need to address the build errors that kbot has found.

-- 
MST

Re: [Qemu-devel] [PATCH V2 2/3] xen-pt: bind/unbind interrupt remapping format MSI

[Stefano Stabellini]

On Tue, 15 Aug 2017, Lan Tianyu wrote:
> Hi Anthony:
> 
> On 2017年08月12日 02:04, Anthony PERARD wrote:
> > On Wed, Aug 09, 2017 at 04:51:21PM -0400, Lan Tianyu wrote:
> >> From: Chao Gao 
> >>
> >> If a vIOMMU is exposed to guest, guest will configure the msi to remapping
> >> format. The original code isn't suitable to the new format. A new pair
> >> bind/unbind interfaces are added for this usage. This patch recognizes
> >> this case and uses new interfaces to bind/unbind msi.
> >>
> >> Signed-off-by: Chao Gao 
> >> Signed-off-by: Lan Tianyu 
> > 
> > Reviewed-by: Anthony PERARD 
> > 
> > That patch series can be applied once the Xen side patches are merged.
> > 
> 
> Great. Thanks for your review.

Lan,

Could you please add a reference to the Xen side patch series in the
description of this patch? That way, it is easier to check what is the
dependecy. In fact, it would be even better to add a reference to the
commit id, once the patches are committed to xen.git.

Thanks,

Stefano

Re: [Qemu-devel] virtio: network: lost tcp/ip packets

[Stefan Priebe - Profihost AG]

Hello,

does nobody have an idea?

Greets,
Stefam

Am 18.08.2017 um 16:40 schrieb Stefan Priebe - Profihost AG:
> Hello,
> 
> i've a problem with two VMs running on the SAME host machine using qemu
> 2.7.1 or 2.9.0 and vhost_net + virtio.
> 
> Sometimes TCP packets going from machine a to machine b are simply lost.
> I see them in VM A using tcpdump going out but they never come in on
> machine B. Both machines have iptables and stuff turned off.
> 
> On the host both VMs network interfaces are connected to the same bridge.
> 
> Any ideas? May be a known bug?
> 
> Host and Guest Kernel is an OpenSuSE 42.3 kernel based on vanilla 4.4.82.
> 
> Thanks!
> 
> Greets,
> Stefan
> 

[Qemu-devel] [PATCH] spapr: Add ibm, processor-storage-keys property to CPU DT node

[Thiago Jung Bauermann]

LoPAPR says:

“ibm,processor-storage-keys”

property name indicating the number of virtual storage keys supported
by the processor described by this node.

prop-encoded-array: Consists of two cells encoded as with encode-int.
The first cell represents the number of virtual storage keys supported
for data accesses while the second cell represents the number of
virtual storage keys supported for instruction accesses. The cell value
of zero indicates that no storage keys are supported for the access
type.

pHyp provides the property above but there's a bug in P8 firmware where the
second cell is zero even though POWER8 supports instruction access keys.
This bug will be fixed for P9.

Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.

Signed-off-by: Thiago Jung Bauermann 
---

The sysfs files are provided by this patch for Linux:

https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-August/162005.html

I realize that this patch can't be committed before the Linux one goes in,
but I'd appreciate feedback so that it will be ready by the time the kernel
side is accepted.

 hw/ppc/spapr.c | 76 ++
 include/hw/ppc/spapr.h |  6 
 2 files changed, 82 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f7a19720dcdf..a665e4d830f7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -605,6 +605,80 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, 
int offset,
   pcc->radix_page_info->count *
   sizeof(radix_AP_encodings[0];
 }
+
+if (spapr->storage_keys) {
+uint32_t val[2];
+
+val[0] = cpu_to_be32(spapr->storage_keys);
+val[1] = spapr->insn_keys ? val[0] : 0;
+
+_FDT(fdt_setprop(fdt, offset, "ibm,processor-storage-keys",
+ val, sizeof(val)));
+}
+}
+
+#define SYSFS_PROT_KEYS_PATH "/sys/kernel/mm/protection_keys/"
+#define SYSFS_USABLE_STORAGE_KEYS SYSFS_PROT_KEYS_PATH "usable_keys"
+#define SYSFS_DISABLE_EXEC_KEYS SYSFS_PROT_KEYS_PATH 
"disable_execute_supported"
+
+static void setup_storage_keys(CPUPPCState *env, sPAPRMachineState *spapr)
+{
+if (!(env->mmu_model & POWERPC_MMU_AMR))
+return;
+
+if (kvm_enabled()) {
+char buf[sizeof("false\n")];
+uint32_t keys;
+FILE *fd;
+
+/*
+ * With KVM, we allow the guest to use the keys which the kernel tells
+ * us are available.
+ */
+
+fd = fopen(SYSFS_USABLE_STORAGE_KEYS, "r");
+if (!fd) {
+error_report("%s: open %s failed", __func__,
+ SYSFS_USABLE_STORAGE_KEYS);
+return;
+}
+
+if (fscanf(fd, "%u", ) != 1) {
+error_report("%s: error reading %s", __func__,
+ SYSFS_USABLE_STORAGE_KEYS);
+fclose(fd);
+return;
+}
+
+fclose(fd);
+
+/* Now find out whether the keys can be used for instruction access. */
+
+fd = fopen(SYSFS_DISABLE_EXEC_KEYS, "r");
+if (!fd) {
+error_report("%s: open %s failed", __func__,
+ SYSFS_USABLE_STORAGE_KEYS);
+return;
+}
+
+if (!fread(buf, 1, sizeof(buf), fd)) {
+error_report("%s: error reading %s", __func__,
+ SYSFS_DISABLE_EXEC_KEYS);
+fclose(fd);
+return;
+}
+
+fclose(fd);
+
+spapr->storage_keys = keys;
+spapr->insn_keys = !strncmp(buf, "true\n", sizeof(buf));
+} else {
+/* Without KVM, all keys provided by the architecture are available. */
+spapr->storage_keys = 32;
+
+/* POWER7 doesn't support instruction access keys. */
+spapr->insn_keys = POWERPC_MMU_VER(env->mmu_model) != 
POWERPC_MMU_VER_2_06;
+}
 }
 
 static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
@@ -619,6 +693,8 @@ static void spapr_populate_cpus_dt_node(void *fdt, 
sPAPRMachineState *spapr)
 _FDT((fdt_setprop_cell(fdt, cpus_offset, "#address-cells", 0x1)));
 _FDT((fdt_setprop_cell(fdt, cpus_offset, "#size-cells", 0x0)));
 
+setup_storage_keys(_CPU(first_cpu)->env, spapr);
+
 /*
  * We walk the CPUs in reverse order to ensure that CPU DT nodes
  * created by fdt_add_subnode() end up in the right order in FDT
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 2a303a705c17..15af12010779 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -122,6 +122,12 @@ struct sPAPRMachineState {
  * occurs during the unplug process. */
 QTAILQ_HEAD(, sPAPRDIMMState) pending_dimm_unplugs;
 
+/* Number of processor storage keys available to the guest. */
+uint32_t storage_keys;
+
+/* Whether storage keys can control instruction access. */
+bool insn_keys;
+
 /*< public >*/

Re: [Qemu-devel] [Qemu-block] [PATCH] nbd-client: fix hang after server closes connection

[Eric Blake]

On 08/21/2017 11:27 AM, Stefan Hajnoczi wrote:
> On Mon, Aug 21, 2017 at 5:15 PM, Stefan Hajnoczi  wrote:
>>   (qemu) quit
>>   ...hang...
> 
> By the way, the same issue is present in QEMU 2.9.  This is not a 2.10
> regression.

Most likely, though, it IS a regression introduced in 2.9 and not
present in 2.8.  I'll add this to my list of 2.11 NBD patches.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] xhci: move command stop and command abort flag check to the case when the crcr_low register is set

[Jaroslav Jindrák]

Hi,

that was an error on my part, I was debugging the non-working command ring
abort/stop functionality and as
the xhci driver implementation I was working on used crcr_low and crcr_high
as two separate registers it did not
hit me it was actually my way of accessing them that caused the abort/stop
to not work.

Thank you very much for the correction and I apologise for disturbing you,
hope I didn't eat too much of your time.

Have a nice day,
Jaroslav Jindrak

S pozdravem,
Jaroslav Jindrak

On 21 August 2017 at 14:23, Gerd Hoffmann  wrote:

> On Tue, 2017-08-01 at 01:48 +0200, Jaroslav Jindrák wrote:
> > Hello,
> >
> > I'd like to submit a patch to the xhci subsystem of QEMU. Currently,
> > when the command stop or command abort flags in the crcr_low register
> > are set, nothing happens. This is because the part of the code that
> > tests those two flags (and performs command ring abort/stop) is in
> > the crcr_high case. This error has a simple workaround - after
> > writing to the crcr_low register with either of these two flags set,
> > one can write the value of crcr_high to crcr_high, so I
> > assume this fix does not have that big of a priority, but a driver
> > that follows the specification strictly would misbehave in this kind
> > of situation (stopping/aborting the command ring).
>
> Specs says (section 5.1, Register Conventions):
>
> 
> If the xHC supports 64-bit addressing (AC64 = ‘1’), then software
> should write registers containing 64-bit address fields using only
> Qword accesses. If a system is incapable of issuing Qword accesses,
> then writes to the 64-bit address fields shall be performed using 2
> Dword accesses; low Dword-first, high-Dword second.
> 
>
> So I think the guest must write both crcr_low and crcr_high, and the
> qemu behavior is correct.
>
> Are there any guests which actually have problems?
>
> cheers,
>   Gerd
>
>

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Dr. David Alan Gilbert]

* Fam Zheng (f...@redhat.com) wrote:
> On Mon, 08/21 16:36, Dr. David Alan Gilbert wrote:
> > * Fam Zheng (f...@redhat.com) wrote:
> > > On Mon, 08/21 18:05, Peter Xu wrote:
> > > > On Mon, Aug 21, 2017 at 04:58:51PM +0800, Fam Zheng wrote:
> > > > > On Mon, 08/21 15:44, Peter Xu wrote:
> > > > > > This is an extended work for migration postcopy recovery. This 
> > > > > > series
> > > > > > is tested with the following series to make sure it solves the 
> > > > > > monitor
> > > > > > hang problem that we have encountered for postcopy recovery:
> > > > > > 
> > > > > >   [RFC 00/29] Migration: postcopy failure recovery
> > > > > >   [RFC 0/6] migration: re-use migrate_incoming for postcopy recovery
> > > > > > 
> > > > > > The root problem is that, monitor commands are all handled in main
> > > > > > loop thread now, no matter how many monitors we specify. And, if 
> > > > > > main
> > > > > > loop thread hangs due to some reason, all monitors will be stuck.
> > > > > > This can be done in reversed order as well: if any of the monitor
> > > > > > hangs, it will hang the main loop, and the rest of the monitors (if
> > > > > > there is any).
> > > > > > 
> > > > > > That affects postcopy recovery, since the recovery requires user 
> > > > > > input
> > > > > > on destination side.  If monitors hang, the destination VM dies and
> > > > > > lose hope for even a final recovery.
> > > > > > 
> > > > > > So, sometimes we need to make sure the monitor be alive, at least 
> > > > > > one
> > > > > > of them.
> > > > > > 
> > > > > > The whole idea of this series is that instead if handling monitor
> > > > > > commands all in main loop thread, we do it separately in per-monitor
> > > > > > threads.  Then, even if main loop thread hangs at any point by any
> > > > > > reason, per-monitor thread can still survive.  Further, we add hint 
> > > > > > in
> > > > > > QMP/HMP to show whether a command can be executed without QMP, if 
> > > > > > so,
> > > > > > we avoid taking BQL when running that command.  It greatly reduced
> > > > > > contention of BQL.  Now the only user of that new parameter 
> > > > > > (currently
> > > > > > I call it "without-bql") is "migrate-incoming" command, which is the
> > > > > > only command to rescue a paused postcopy migration.
> > > > > > 
> > > > > > However, even with the series, it does not mean that per-monitor
> > > > > > threads will never hang.  One example is that we can still run "info
> > > > > > vcpus" in per-monitor threads during a paused postcopy (in that 
> > > > > > state,
> > > > > > page faults are never handled, and "info cpus" will never return 
> > > > > > since
> > > > > > it tries to sync every vcpus).  So to make sure it does not hang, we
> > > > > > not only need the per-monitor thread, the user should be careful as
> > > > > > well on how to use it.
> > > > > 
> > > > > I think this is like saying we expect the user to understand the 
> > > > > internals of
> > > > > QEMU, unless the "rules" are clearly documented.  Taking this into 
> > > > > account,
> > > > > does it make sense to make the per-monitor thread only allow BQL-free 
> > > > > commands?
> > > > 
> > > > I don't think users need to know the internals - they just need to be
> > > > careful on using them.  Just take the example of "info cpus": during
> > > > paused postcopy it will hang, but IMHO it does not mean that it's
> > > > illegal for user to send that command.  It's "by-design" that it'll be
> > > > stuck if one of the vcpus is stuck somewhere; it's just not the
> > > > correct way to use it when the monitor is prepared for postcopy
> > > > recovery.
> > > 
> > > They still need to know "what" is the correct way to use the monitor, and 
> > > what
> > > I'm saying is there doesn't seem to be an easy way for users to know 
> > > exactly
> > > what is correct. See below.
> > > 
> > > > 
> > > > And IMHO we should not treat threaded monitors special - it should be
> > > > exactly the same monitor service when used with main loop thread.  It
> > > > just has its own thread to handle the requests, so it is less
> > > > dependent on main loop thread, and that's all.
> > > 
> > > It's not that simple, I think all non-trivial commands need very careful 
> > > audit
> > > before assuming they're safe. For example many block related commands
> > > (qmp_trasaction, for example) indirectly calls BDRV_POLL_WHILE(), which, 
> > > if
> > > called from a per-monitor thread, will enter the else branch then fail 
> > > the first
> > > assert.
> > 
> > OK, that's interesting - I'd assumed that as long as we actually held
> > the bql we were reasonably safe.
> > Can you explain what that assert is actually asserting?
> 
> It's not much more than asserting qemu_mutex_iothread_locked(), the problem is
> the new monitor thread breaks certain assumptions that was true.
> 
> What is interesting in this is that block layer's nested aio_poll() now not 
> only
> run in the main thread but also in the monitor thread. 

Re: [Qemu-devel] [PATCH v4 0/4] scsi-block: Support werror/rerror

[Paolo Bonzini]

- Original Message -
> From: "Fam Zheng" 
> To: qemu-devel@nongnu.org
> Cc: "Paolo Bonzini" 
> Sent: Monday, August 21, 2017 4:10:04 PM
> Subject: [PATCH v4 0/4] scsi-block: Support werror/rerror
> 
> v4: Handle more sense keys/ascqs. [Paolo]
> Add scsi_sense_buf_to_errno to util/scsi.c. [Paolo]
> Shorten the comment notes on SCSI constants.
> 
> v3: Enhance and reuse iscsi sense data translation. [Paolo]
> 
> First refactor and enhance the code in iscsi for sense data translation, then
> modify error handing of scsi-block and expose the qdev properties.
> 
> Fam Zheng (4):
>   scsi: Refactor scsi sense interpreting code
>   scsi: Improve scsi_sense_to_errno
>   scsi: Introduce scsi_sense_buf_to_errno
>   scsi-block: Support rerror/werror

Looks good, thanks!

Paolo

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Fam Zheng]

On Mon, 08/21 16:36, Dr. David Alan Gilbert wrote:
> * Fam Zheng (f...@redhat.com) wrote:
> > On Mon, 08/21 18:05, Peter Xu wrote:
> > > On Mon, Aug 21, 2017 at 04:58:51PM +0800, Fam Zheng wrote:
> > > > On Mon, 08/21 15:44, Peter Xu wrote:
> > > > > This is an extended work for migration postcopy recovery. This series
> > > > > is tested with the following series to make sure it solves the monitor
> > > > > hang problem that we have encountered for postcopy recovery:
> > > > > 
> > > > >   [RFC 00/29] Migration: postcopy failure recovery
> > > > >   [RFC 0/6] migration: re-use migrate_incoming for postcopy recovery
> > > > > 
> > > > > The root problem is that, monitor commands are all handled in main
> > > > > loop thread now, no matter how many monitors we specify. And, if main
> > > > > loop thread hangs due to some reason, all monitors will be stuck.
> > > > > This can be done in reversed order as well: if any of the monitor
> > > > > hangs, it will hang the main loop, and the rest of the monitors (if
> > > > > there is any).
> > > > > 
> > > > > That affects postcopy recovery, since the recovery requires user input
> > > > > on destination side.  If monitors hang, the destination VM dies and
> > > > > lose hope for even a final recovery.
> > > > > 
> > > > > So, sometimes we need to make sure the monitor be alive, at least one
> > > > > of them.
> > > > > 
> > > > > The whole idea of this series is that instead if handling monitor
> > > > > commands all in main loop thread, we do it separately in per-monitor
> > > > > threads.  Then, even if main loop thread hangs at any point by any
> > > > > reason, per-monitor thread can still survive.  Further, we add hint in
> > > > > QMP/HMP to show whether a command can be executed without QMP, if so,
> > > > > we avoid taking BQL when running that command.  It greatly reduced
> > > > > contention of BQL.  Now the only user of that new parameter (currently
> > > > > I call it "without-bql") is "migrate-incoming" command, which is the
> > > > > only command to rescue a paused postcopy migration.
> > > > > 
> > > > > However, even with the series, it does not mean that per-monitor
> > > > > threads will never hang.  One example is that we can still run "info
> > > > > vcpus" in per-monitor threads during a paused postcopy (in that state,
> > > > > page faults are never handled, and "info cpus" will never return since
> > > > > it tries to sync every vcpus).  So to make sure it does not hang, we
> > > > > not only need the per-monitor thread, the user should be careful as
> > > > > well on how to use it.
> > > > 
> > > > I think this is like saying we expect the user to understand the 
> > > > internals of
> > > > QEMU, unless the "rules" are clearly documented.  Taking this into 
> > > > account,
> > > > does it make sense to make the per-monitor thread only allow BQL-free 
> > > > commands?
> > > 
> > > I don't think users need to know the internals - they just need to be
> > > careful on using them.  Just take the example of "info cpus": during
> > > paused postcopy it will hang, but IMHO it does not mean that it's
> > > illegal for user to send that command.  It's "by-design" that it'll be
> > > stuck if one of the vcpus is stuck somewhere; it's just not the
> > > correct way to use it when the monitor is prepared for postcopy
> > > recovery.
> > 
> > They still need to know "what" is the correct way to use the monitor, and 
> > what
> > I'm saying is there doesn't seem to be an easy way for users to know exactly
> > what is correct. See below.
> > 
> > > 
> > > And IMHO we should not treat threaded monitors special - it should be
> > > exactly the same monitor service when used with main loop thread.  It
> > > just has its own thread to handle the requests, so it is less
> > > dependent on main loop thread, and that's all.
> > 
> > It's not that simple, I think all non-trivial commands need very careful 
> > audit
> > before assuming they're safe. For example many block related commands
> > (qmp_trasaction, for example) indirectly calls BDRV_POLL_WHILE(), which, if
> > called from a per-monitor thread, will enter the else branch then fail the 
> > first
> > assert.
> 
> OK, that's interesting - I'd assumed that as long as we actually held
> the bql we were reasonably safe.
> Can you explain what that assert is actually asserting?

It's not much more than asserting qemu_mutex_iothread_locked(), the problem is
the new monitor thread breaks certain assumptions that was true.

What is interesting in this is that block layer's nested aio_poll() now not only
run in the main thread but also in the monitor thread. Bugs may hide there.  :)

That's why I suggested a "safe by default" strategy.


One step back, is it possible to "unblock" main thread even upon network issue?
What is the scenario that causes main thread hang? Is there a backtrace?

Fam

Re: [Qemu-devel] [Qemu-block] [PATCH] nbd-client: fix hang after server closes connection

[Stefan Hajnoczi]

On Mon, Aug 21, 2017 at 5:15 PM, Stefan Hajnoczi  wrote:
>   (qemu) quit
>   ...hang...

By the way, the same issue is present in QEMU 2.9.  This is not a 2.10
regression.

Stefan

Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions

[Halil Pasic]

On 08/21/2017 04:58 PM, Pierre Morel wrote:
> On 21/08/2017 11:16, Cornelia Huck wrote:
>> If we do not provide zpci, pci reconfiguration via sclp is not available
>> either. Don't indicate it in the sclp facilities and return an invalid
>> command if the guest tries to issue pci configure/deconfigure.
>>
>> Reviewed-by: Thomas Huth 
>> Signed-off-by: Cornelia Huck 
>> ---
>>   hw/s390x/sclp.c | 19 +++
>>   1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 9253dbbc64..d0104cd784 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -59,6 +59,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>   int rnsize, rnmax;
>>   int slots = MIN(machine->ram_slots, s390_get_memslot_count(kvm_state));
>>   IplParameterBlock *ipib = s390_ipl_get_iplb();
>> +uint64_t sclp_facilities = SCLP_HAS_CPU_INFO;
>>
>>   CPU_FOREACH(cpu) {
>>   cpu_count++;
>> @@ -79,8 +80,10 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>
>>   prepare_cpu_entries(sclp, read_info->entries, cpu_count);
>>
>> -read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>> -SCLP_HAS_PCI_RECONFIG);
>> +if (s390_has_feat(S390_FEAT_ZPCI)) {
>> +sclp_facilities |= SCLP_HAS_PCI_RECONFIG;
>> +}
>> +read_info->facilities = cpu_to_be64(sclp_facilities);
>>
>>   /* Memory Hotplug is only supported for the ccw machine type */
>>   if (mhd) {
>> @@ -385,10 +388,18 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, 
>> uint32_t code)
>>   sclp_c->unassign_storage(sclp, sccb);
>>   break;
>>   case SCLP_CMDW_CONFIGURE_PCI:
>> -s390_pci_sclp_configure(sccb);
>> +if (s390_has_feat(S390_FEAT_ZPCI)) {
>> +s390_pci_sclp_configure(sccb);
>> +} else {
>> +sccb->h.response_code = 
>> cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> 
> Hello Conny,
> 
> I find more logical if the response would be 0x06F0 : "Adapter type in SCCB 
> not recognized"
> Since we could have more than one adapter type... someday.
> then the SCLP command would be valid but not the adapter type.

I agree, 01F0 does not seem to be the correct answer, based on
the AR as it seems to be tied to the command code.

Verifying what the machine does would be great though -- frankly
I cant tell with absolute confidence what's the right thing to
do based on my understanding of the AR. 


> 
> However, I will try to find a real hardware to test this since I noticed that 
> my logic sometime... :) .
> 
> Another point is that don't you think that this test on S390_FEAT_ZPCI better 
> belong to the dedicated PCI code inside of the s390_pci_sclp_configure().
>

I'm fine either way. If I imagine having a lots of adapter types, then I
would expect a switch or a jumptable on the type before handling control
to the pci specific function. In this case statically not supported types
would probably get caught by the default branch of the switch and for a
jumptable it could even handle the dynamic case (based on the facilities)
trivially. In short both approaches can make sense.

Regards,
Halil

> best regards,
> 
> Pierre
> 
> 
>> +}
>>   break;
>>   case SCLP_CMDW_DECONFIGURE_PCI:
>> -s390_pci_sclp_deconfigure(sccb);
>> +if (s390_has_feat(S390_FEAT_ZPCI)) {
>> +s390_pci_sclp_deconfigure(sccb);
>> +} else {
>> +sccb->h.response_code = 
>> cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
>> +}
>>   break;
>>   default:
>>   efc->command_handler(ef, sccb, code);
>>
> 
> 

Re: [Qemu-devel] [PATCH v4 2/5] qapi: Add qobject_is_equal()

[Max Reitz]

On 2017-08-14 11:07, Markus Armbruster wrote:
> Max Reitz  writes:
> 
>> On 2017-07-11 13:33, Markus Armbruster wrote:
>>> Max Reitz  writes:
>>>
 First of all, OK, you don't want QNum(42.0) to equal QNum(42) at all (at
 least not right now and in the foreseeable future).
 You're the maintainer, so you decide, so I'll go along with it. :-)

 Now, let's follow up with my therefore rather useless commentary:

 (Feel free to disregard, because honestly, I can see how replying to
 most of the points I'm asking isn't really worth the time...)
>>>
>>> When I use the authority entrusted to maintainers, I feel obliged to at
>>> least explain my reasoning.  Besides, putting my reasoning in words
>>> tends to lead me to new insights.
>>
>> And I am indeed very grateful for that. :-)
>>
 On 2017-07-10 11:17, Markus Armbruster wrote:
> Max Reitz  writes:
>
>> On 2017-07-06 16:30, Markus Armbruster wrote:
>>
>> [...]
>>
> The only way to add unsigned integers without breaking QMP compatibility
> is to make them interchangeable with signed integers.  That doesn't mean
> you get to make floating-point numbers interchangeable with integers
> now.

 Again, begs the question why QNum covers floating point numbers then and
 why this very fact is not documented in qnum.c.
>>>
>>> What kind of documentation would you like to see?
>>
>> It would be good to note that the QNum type is not meant to be a
>> completely uniform way to handle JSON numbers (e.g. if the user provides
>> something with a decimal point but you need an integer, QNum will not do
>> that conversion for you).
>>
>> It is (English indirect speech is broken badly) just meant to
>> encapsulate the different variants a number can be represented in, but
>> you're still generally supposed to read it out the way it was put in
>> (exceptions apply, see signed/unsigned and qnum_get_double()).
> 
> Can we distill this into text that could become an actual patch?  Let me
> try.
> 
> QNum encapsulates how our dialect of JSON fills in the blanks left
> by the JSON specification (RFC 7159) regarding numbers.
> 
> Conceptually, we treat number as an abstract type with three
> concrete subtypes: floating-point, signed integer, unsigned integer.
> QNum implements this a discriminated union of double, int64_t,
> uint64_t.
> 
> The JSON parser picks the subtype as follows.  If the number has a
> decimal point or an exponent, it is floating-point.  Else if it fits
> into int64_t, it's signed integer.  Else if it first into uint64_t,
> it's unsigned integer.  Else it's floating-point.
> 
> Any number can serve as double: qnum_get_double() converts under the
> hood.
> 
> An integer can serve as signed / unsigned integer as long as it is
> in range: qnum_get_try_int() / qnum_get_try_uint() check range and
> convert under the hood.
> 
> What do you think?

Sounds very good to me, thanks!

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] nbd-client: fix hang after server closes connection

[Stefan Hajnoczi]

Commit 72b6ffc76653214b69a94a7b1643ff80df134486 ("nbd-client: Fix
regression when server sends garbage") improved NBD client behavior when
the connection enters a broken state.

The following still does not behave as expected:

  $ qemu-nbd -p 1234 -x drive0 -f qcow2 test.qcow2
  $ qemu-system-x86_64 -M accel=kvm -m 1G \
-drive if=virtio,id=drive0,file=nbd://localhost:1234/drive0,format=raw
  $ pkill qemu-nbd
  (qemu) quit
  ...hang...

QEMU should be able to quit even when the connection was previously
closed by the NBD server.  Currently the nbd_read_reply_entry()
coroutine terminates without letting in-flight requests know that the
connection is dead.

This patch flags the connection as dead so in-flight requests can
complete.

Reported-by: Longxiang Lyu 
Cc: Eric Blake 
Cc: Paolo Bonzini 
Signed-off-by: Stefan Hajnoczi 
---
 block/nbd-client.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 422ecb4307..5a5fe02015 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -80,6 +80,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 error_report_err(local_err);
 }
 if (ret <= 0) {
+s->quit = true;
 break;
 }
 
@@ -107,9 +108,6 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 qemu_coroutine_yield();
 }
 
-if (ret < 0) {
-s->quit = true;
-}
 nbd_recv_coroutines_enter_all(s);
 s->read_reply_co = NULL;
 }
-- 
2.13.5

Re: [Qemu-devel] AVMF & OVMF blobs in QEMU tree???

[Laszlo Ersek]

On 08/21/17 16:03, Alex Bennée wrote:
> 
> Gerd Hoffmann  writes:
> 
>> On Mon, 2017-08-07 at 18:51 +0200, Laszlo Ersek wrote:
>>> On 08/07/17 16:40, Peter Maydell wrote:
 On 7 August 2017 at 15:31, Igor Mammedov 
 wrote:
> As I recall there were issues with FAT driver licensing in edk2,
> but I've heard there were some changes in that regard.
>
> Is there any other reasons why we are not putting subj.
> in QEMU tree like we do with SeaBIOS and other roms?

 I suspect the primary answer is "nobody who's willing to
 maintain, test and update the resulting binary blobs has
 stepped forward to say they want to do so" :-)

 (I think that shipping them in the QEMU tree would be
 nice but is principally a convenience for our direct
 users, since distros are going to want to build their
 own ROM blobs from source anyway.)
>>>
>>> I agree that OVMF and ArmVirtQemu firmware binaries (and matching
>>> varstore templates, likely compressed) should be bundled with QEMU.
>>> There are no license-related reasons left that would prevent this.
>>>
>>> Please let us discuss this when Gerd returns from vacation. (CC'ing
>>> Gerd.)
>>
>> slighly oldish wip branch:
>> https://www.kraxel.org/cgit/qemu/log/?h=work/edk2
>>
>> Related question (as the edk2 blobs are pretty big):  Do we want commit
>> this to the qemu repo directly?   Or should we create a qemu-firmware
>> repo for the precompiled blobs and hook it up as submodule?
> 
> How big?

The ArmVirtQemu firmware binary, and its matching varstore template,
need to be padded to 64MB each, after the edk2 build process completes,
before the fw binary, and a variable store created from the varstore
template, can be passed to QEMU.

So, "pretty big".

Thanks
Laszlo

> Things like github tend to get picky with blobs over 50Mb and
> git isn't really set-up for large binary bits hence things like git-lfs.
> Unfortunately there seems to be multiple solutions to the blobs in git
> problem.
> 
> We should also take some care to reproducibility so they can be updated
> by someone other than the original author.
> 
>>
>> cheers,
>>   Gerd
> 
> 
> --
> Alex Bennée
> 

Re: [Qemu-devel] [PATCH v4 02/10] kvm: remove hard dependency on pci

[Pierre Morel]

On 21/08/2017 11:16, Cornelia Huck wrote:

The msi routing code in kvm calls some pci functions: provide
some stubs to enable builds without pci.

Also, to make this more obvious, guard them via a pci_available boolean
(which also can be reused in other places).

Fixes: e1d4fb2de ("kvm-irqchip: x86: add msi route notify fn")
Fixes: 767a554a0 ("kvm-all: Pass requester ID to MSI routing functions")
Signed-off-by: Cornelia Huck 
---
  accel/kvm/kvm-all.c  |  6 +++---
  hw/pci/pci-stub.c| 15 +++
  hw/pci/pci.c |  2 ++
  include/hw/pci/pci.h |  2 ++
  4 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 46ce479dc3..f85553a851 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, 
PCIDevice *dev)
  int virq;
  MSIMessage msg = {0, 0};

-if (dev) {
+if (pci_available && dev) {
  msg = pci_get_msi_message(dev, vector);
  }


Hi Conny,

I did not find a case where pci_available is false and dev is true.
but anyway, sure is sure.




@@ -1271,7 +1271,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, 
PCIDevice *dev)
  kroute.u.msi.address_lo = (uint32_t)msg.address;
  kroute.u.msi.address_hi = msg.address >> 32;
  kroute.u.msi.data = le32_to_cpu(msg.data);
-if (kvm_msi_devid_required()) {
+if (pci_available && kvm_msi_devid_required()) {
  kroute.flags = KVM_MSI_VALID_DEVID;
  kroute.u.msi.devid = pci_requester_id(dev);
  }
@@ -1309,7 +1309,7 @@ int kvm_irqchip_update_msi_route(KVMState *s, int virq, 
MSIMessage msg,
  kroute.u.msi.address_lo = (uint32_t)msg.address;
  kroute.u.msi.address_hi = msg.address >> 32;
  kroute.u.msi.data = le32_to_cpu(msg.data);
-if (kvm_msi_devid_required()) {
+if (pci_available && kvm_msi_devid_required()) {
  kroute.flags = KVM_MSI_VALID_DEVID;
  kroute.u.msi.devid = pci_requester_id(dev);
  }
diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index ecad664946..ace43821ca 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -23,10 +23,12 @@
  #include "monitor/monitor.h"
  #include "qapi/qmp/qerror.h"
  #include "hw/pci/pci.h"
+#include "hw/pci/msi.h"


I think you forgot that...


  #include "qmp-commands.h"
  #include "hw/pci/msi.h"


...you already have it included here. Didn't you ?



  bool msi_nonbroken;
+bool pci_available;

  PciInfoList *qmp_query_pci(Error **errp)
  {
@@ -38,3 +40,16 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict 
*qdict)
  {
  monitor_printf(mon, "PCI devices not supported\n");
  }
+
+/* kvm-all wants this */
+MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
+{
+g_assert(false);
+return (MSIMessage){};
+}
+
+uint16_t pci_requester_id(PCIDevice *dev)
+{
+g_assert(false);
+return 0;
+}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 258fbe51e2..26f346db2c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -49,6 +49,8 @@
  # define PCI_DPRINTF(format, ...)   do { } while (0)
  #endif

+bool pci_available = true;
+
  static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
  static char *pcibus_get_dev_path(DeviceState *dev);
  static char *pcibus_get_fw_dev_path(DeviceState *dev);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e598b095eb..8bb6449dd7 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -10,6 +10,8 @@

  #include "hw/pci/pcie.h"

+extern bool pci_available;
+
  /* PCI bus */

  #define PCI_DEVFN(slot, func)   slot) & 0x1f) << 3) | ((func) & 0x07))



otherwise LGTM

Regards,

Pierre


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

[Qemu-devel] [PATCH] filter-mirror: segfault when specifying non existent device

[Eduardo Otubo]

When using filter-mirror like the example below where the interface
'ndev0' does not exist on the host, QEMU crashes into segmentation
fault.

 $ qemu-system-x86_64 -S -machine pc -netdev user,id=ndev0 -object 
filter-mirror,id=test-object,netdev=ndev0

This happens because the function filter_mirror_setup() does not checks
if the device actually exists and still keep on processing calling
qemu_chr_find(). This patch fixes this issue.

Signed-off-by: Eduardo Otubo 
---
 net/filter-mirror.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 90e2c92337..e18a4b16a0 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -213,14 +213,22 @@ static void filter_mirror_setup(NetFilterState *nf, Error 
**errp)
 MirrorState *s = FILTER_MIRROR(nf);
 Chardev *chr;
 
+if (s->outdev == NULL) {
+goto err;
+}
+
 chr = qemu_chr_find(s->outdev);
+
 if (chr == NULL) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-  "Device '%s' not found", s->outdev);
-return;
+goto err;
 }
 
 qemu_chr_fe_init(>chr_out, chr, errp);
+
+err:
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found",
+  nf->netdev_id);
+return;
 }
 
 static void redirector_rs_finalize(SocketReadState *rs)
-- 
2.13.5

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[Dr. David Alan Gilbert]

oh yeh you want to tell gdb to ignore SIGUSR1, something like: 
  handle SIGUSR1 nostop noprint pass

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1711602

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1711602/+subscriptions

Re: [Qemu-devel] [PATCH v2 3/6] block: require job-id when device is a node name

[Manos Pitsidianakis]

On Mon, Aug 21, 2017 at 05:05:30PM +0200, Alberto Garcia wrote:

On Wed 09 Aug 2017 04:02:53 PM CEST, Manos Pitsidianakis wrote:

@@ -622,20 +622,14 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 return NULL;
 }

-if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
-job_id = bdrv_get_device_name(bs);
-if (!*job_id) {
-error_setg(errp, "An explicit job ID is required for this node");
-return NULL;
-}
-}
-
-if (job_id) {
-if (flags & BLOCK_JOB_INTERNAL) {
+if (flags & BLOCK_JOB_INTERNAL) {
+if (job_id) {
 error_setg(errp, "Cannot specify job ID for internal block job");
 return NULL;
 }


When BLOCK_JOB_INTERNAL is set, then job_id must be NULL...


-
+} else {
+/* Require job-id. */
+assert(job_id);
 if (!id_wellformed(job_id)) {
 error_setg(errp, "Invalid job ID '%s'", job_id);
 return NULL;
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index f13ad05c0d..ff906808a6 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -112,8 +112,7 @@ struct BlockJobDriver {

 /**
  * block_job_create:
- * @job_id: The id of the newly-created job, or %NULL to have one
- * generated automatically.
+ * @job_id: The id of the newly-created job, must be non %NULL.


...but here you say that it must not be NULL.

And since job_id can be NULL in some cases I think I'd replace the
assert(job_id) that you added to block_job_create() with a normal
pointer check + error_setg().


@@ -93,9 +93,6 @@ static void test_job_ids(void)
 blk[1] = create_blk("drive1");
 blk[2] = create_blk("drive2");

-/* No job ID provided and the block backend has no name */
-job[0] = do_test_id(blk[0], NULL, false);
-


If you decide to handle NULL ids by returning and error instead of
asserting, we should keep a couple of tests for that scenario.

Berto



Thanks, I will change that. What cases are you thinking of besides 
internal jobs though? And should documentation of block_job_create 
reflect that internal jobs have job_id == NULL?


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Dr. David Alan Gilbert]

* Fam Zheng (f...@redhat.com) wrote:
> On Mon, 08/21 18:05, Peter Xu wrote:
> > On Mon, Aug 21, 2017 at 04:58:51PM +0800, Fam Zheng wrote:
> > > On Mon, 08/21 15:44, Peter Xu wrote:
> > > > This is an extended work for migration postcopy recovery. This series
> > > > is tested with the following series to make sure it solves the monitor
> > > > hang problem that we have encountered for postcopy recovery:
> > > > 
> > > >   [RFC 00/29] Migration: postcopy failure recovery
> > > >   [RFC 0/6] migration: re-use migrate_incoming for postcopy recovery
> > > > 
> > > > The root problem is that, monitor commands are all handled in main
> > > > loop thread now, no matter how many monitors we specify. And, if main
> > > > loop thread hangs due to some reason, all monitors will be stuck.
> > > > This can be done in reversed order as well: if any of the monitor
> > > > hangs, it will hang the main loop, and the rest of the monitors (if
> > > > there is any).
> > > > 
> > > > That affects postcopy recovery, since the recovery requires user input
> > > > on destination side.  If monitors hang, the destination VM dies and
> > > > lose hope for even a final recovery.
> > > > 
> > > > So, sometimes we need to make sure the monitor be alive, at least one
> > > > of them.
> > > > 
> > > > The whole idea of this series is that instead if handling monitor
> > > > commands all in main loop thread, we do it separately in per-monitor
> > > > threads.  Then, even if main loop thread hangs at any point by any
> > > > reason, per-monitor thread can still survive.  Further, we add hint in
> > > > QMP/HMP to show whether a command can be executed without QMP, if so,
> > > > we avoid taking BQL when running that command.  It greatly reduced
> > > > contention of BQL.  Now the only user of that new parameter (currently
> > > > I call it "without-bql") is "migrate-incoming" command, which is the
> > > > only command to rescue a paused postcopy migration.
> > > > 
> > > > However, even with the series, it does not mean that per-monitor
> > > > threads will never hang.  One example is that we can still run "info
> > > > vcpus" in per-monitor threads during a paused postcopy (in that state,
> > > > page faults are never handled, and "info cpus" will never return since
> > > > it tries to sync every vcpus).  So to make sure it does not hang, we
> > > > not only need the per-monitor thread, the user should be careful as
> > > > well on how to use it.
> > > 
> > > I think this is like saying we expect the user to understand the 
> > > internals of
> > > QEMU, unless the "rules" are clearly documented.  Taking this into 
> > > account,
> > > does it make sense to make the per-monitor thread only allow BQL-free 
> > > commands?
> > 
> > I don't think users need to know the internals - they just need to be
> > careful on using them.  Just take the example of "info cpus": during
> > paused postcopy it will hang, but IMHO it does not mean that it's
> > illegal for user to send that command.  It's "by-design" that it'll be
> > stuck if one of the vcpus is stuck somewhere; it's just not the
> > correct way to use it when the monitor is prepared for postcopy
> > recovery.
> 
> They still need to know "what" is the correct way to use the monitor, and what
> I'm saying is there doesn't seem to be an easy way for users to know exactly
> what is correct. See below.
> 
> > 
> > And IMHO we should not treat threaded monitors special - it should be
> > exactly the same monitor service when used with main loop thread.  It
> > just has its own thread to handle the requests, so it is less
> > dependent on main loop thread, and that's all.
> 
> It's not that simple, I think all non-trivial commands need very careful audit
> before assuming they're safe. For example many block related commands
> (qmp_trasaction, for example) indirectly calls BDRV_POLL_WHILE(), which, if
> called from a per-monitor thread, will enter the else branch then fail the 
> first
> assert.

OK, that's interesting - I'd assumed that as long as we actually held
the bql we were reasonably safe.
Can you explain what that assert is actually asserting?

Dave

> Fam
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 09/10] s390x/kvm: msi route fixup for non-pci

[Halil Pasic]

On 08/21/2017 05:17 PM, Thomas Huth wrote:
> On 21.08.2017 17:10, Halil Pasic wrote:
> [...]
>> The situation is just complicated by the fact that there is code which
>> relies on assert(true) asserting for correctness (e.g. virtio goes so far
>> to make builds with normal asserts disabled fail). Thus for me it's hard
>> to assume that the assertion is guaranteed to be disabled in production.
> 
> FYI: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg03608.html
> 
>  Thomas
> 

Thanks, I've missed that. With that assumed it becomes either
assert(false) or return -ENODEV but not both.

Regards,
Halil 

Re: [Qemu-devel] [PATCH v2 3/6] block: require job-id when device is a node name

[Alberto Garcia]

On Wed 09 Aug 2017 04:02:53 PM CEST, Manos Pitsidianakis wrote:
> @@ -622,20 +622,14 @@ void *block_job_create(const char *job_id, const 
> BlockJobDriver *driver,
>  return NULL;
>  }
>  
> -if (job_id == NULL && !(flags & BLOCK_JOB_INTERNAL)) {
> -job_id = bdrv_get_device_name(bs);
> -if (!*job_id) {
> -error_setg(errp, "An explicit job ID is required for this node");
> -return NULL;
> -}
> -}
> -
> -if (job_id) {
> -if (flags & BLOCK_JOB_INTERNAL) {
> +if (flags & BLOCK_JOB_INTERNAL) {
> +if (job_id) {
>  error_setg(errp, "Cannot specify job ID for internal block job");
>  return NULL;
>  }

When BLOCK_JOB_INTERNAL is set, then job_id must be NULL...

> -
> +} else {
> +/* Require job-id. */
> +assert(job_id);
>  if (!id_wellformed(job_id)) {
>  error_setg(errp, "Invalid job ID '%s'", job_id);
>  return NULL;
> diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
> index f13ad05c0d..ff906808a6 100644
> --- a/include/block/blockjob_int.h
> +++ b/include/block/blockjob_int.h
> @@ -112,8 +112,7 @@ struct BlockJobDriver {
>  
>  /**
>   * block_job_create:
> - * @job_id: The id of the newly-created job, or %NULL to have one
> - * generated automatically.
> + * @job_id: The id of the newly-created job, must be non %NULL.

...but here you say that it must not be NULL.

And since job_id can be NULL in some cases I think I'd replace the
assert(job_id) that you added to block_job_create() with a normal
pointer check + error_setg().

> @@ -93,9 +93,6 @@ static void test_job_ids(void)
>  blk[1] = create_blk("drive1");
>  blk[2] = create_blk("drive2");
>  
> -/* No job ID provided and the block backend has no name */
> -job[0] = do_test_id(blk[0], NULL, false);
> -

If you decide to handle NULL ids by returning and error instead of
asserting, we should keep a couple of tests for that scenario.

Berto

Re: [Qemu-devel] [PATCH v4 09/10] s390x/kvm: msi route fixup for non-pci

[Thomas Huth]

On 21.08.2017 17:10, Halil Pasic wrote:
[...]
> The situation is just complicated by the fact that there is code which
> relies on assert(true) asserting for correctness (e.g. virtio goes so far
> to make builds with normal asserts disabled fail). Thus for me it's hard
> to assume that the assertion is guaranteed to be disabled in production.

FYI: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg03608.html

 Thomas

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[ChristianEhrhardt]

As expected by David when I trace on process_incoming_migration_co which prints 
the "readable" error I see the error pop up on "qemu_loadvm_state"
It appears as "Thread 4 "CPU 0/KVM" received signal SIGUSR1" and similar which 
is just the break down of the guest.


Diving "into" qemu_loadvm_state reveals that it gets until 
"cpu_synchronize_all_pre_loadvm".
In qemu_loadvm_state none of the initial checks fail.
Then the "ret = vmstate_load_state(f, _configuration, _state, 
0);" seems to work fine was well.
It seems reproducible in "cpu_synchronize_all_pre_loadvm" where the crash 
happens.

I can catch the incoming qemu easily with:
$ while ! pid=$(pidof qemu-system-x86_64); do /bin/true; done; gdb --pid ${pid}
# Then in gdb break on "cpu_synchronize_all_pre_loadvm"
# And when I step over it I the next thing I see is the "beginning of the end" 
for the process
Thread 4 "CPU 0/KVM" received signal SIGUSR1, User defined signal 1.
[Switching to Thread 0x7f418136e700 (LWP 3887)]
__lll_lock_wait () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135

The guest only has one vcpu, so CPU_FOREACH(cpu) is not much of a loop.
Looking down that path I tracked it to along:
cpu_synchronize_all_pre_loadvm -> cpu_synchronize_pre_loadvm -> 
kvm_cpu_synchronize_pre_loadvm -> do_run_on_cpu

Here it queues the function "do_kvm_cpu_synchronize_pre_loadvm" onto the vcpu.
That is done via queue_work_on_cpu(cpu, ); which in turn uses eventually 
"qemu_cpu_kick_thread(cpu);"
That seems to trigger the first SIGUSR1

Following that I get the breakpoint that I set at 
"do_kvm_cpu_synchronize_pre_loadvm".
The actual function only sets "cpu->vcpu_dirty = true;" and works.

On the way out from there, there is a "qemu_kvm_wait_io_event" which
leads to the next SIGUSR1.

Going on I see another "do_run_on_cpu" with "vapic_do_enable_tpr_reporting" as 
the function.
I set a breakpoint on that as well but took a full CPUstate before going on:
p *cpu
$4 = {parent_obj = {parent_obj = {class = 0x5ffe7170c0, free = 0x7f62328f15a0 
, properties = 0x5ffe736e40, ref = 1, 
  parent = 0x5ffe726160}, id = 0x0, realized = true, pending_deleted_event 
= false, opts = 0x0, hotplugged = 0, parent_bus = 0x0, gpios = {
  lh_first = 0x0}, child_bus = {lh_first = 0x0}, num_child_bus = 0, 
instance_id_alias = -1, alias_required_for_version = 0}, nr_cores = 1, 
  nr_threads = 1, thread = 0x5ffe803cd0, thread_id = 8498, running = false, 
has_waiter = false, halt_cond = 0x5ffe803cf0, thread_kicked = true, 
  created = true, stop = false, stopped = true, unplug = false, crash_occurred 
= false, exit_request = false, interrupt_request = 4, 
  singlestep_enabled = 0, icount_budget = 0, icount_extra = 0, jmp_env = 
{{__jmpbuf = {0, 0, 0, 0, 0, 0, 0, 0}, __mask_was_saved = 0, 
  __saved_mask = {__val = {0 , work_mutex = {lock = 
{__data = {__lock = 0, __count = 0, __owner = 0, __nusers = 0, 
__kind = 0, __spins = 0, __elision = 0, __list = {__prev = 0x0, __next 
= 0x0}}, __size = '\000' , __align = 0}, 
initialized = true}, queued_work_first = 0x5fffefc990, queued_work_last = 
0x5fffefc990, cpu_ases = 0x5ffe803c10, num_ases = 1, 
  as = 0x5ffe7f9690, memory = 0x5ffe725bd0, env_ptr = 0x5ffe7e44c0, 
tb_jmp_cache = {0x0 }, gdb_regs = 0x0, 
  gdb_num_regs = 57, gdb_num_g_regs = 57, node = {tqe_next = 0x0, tqe_prev = 
0x5ffc1783f0 }, breakpoints = {tqh_first = 0x0, 
tqh_last = 0x5ffe7e4430}, watchpoints = {tqh_first = 0x0, tqh_last = 
0x5ffe7e4440}, watchpoint_hit = 0x0, opaque = 0x0, mem_io_pc = 0, 
  mem_io_vaddr = 0, kvm_fd = 19, kvm_state = 0x5ffe7357a0, kvm_run = 
0x7f62374bc000, trace_dstate_delayed = {0}, trace_dstate = {0}, 
  cpu_index = 0, halted = 1, can_do_io = 1, exception_index = -1, vcpu_dirty = 
true, throttle_thread_scheduled = false, icount_decr = {u32 = 0, 
u16 = {low = 0, high = 0}}, hax_vcpu = 0x0, pending_tlb_flush = 7}

Continuing I hit the "vapic_do_enable_tpr_reporting" with qemu still
running.

Things go on, the next candidate for "do_run_on_cpu" is "kvm_apic_put"
Another SIGUSR1 to get that kicked it seems.
"kvm_apic_put" breakpoint is reached after that kick.

Next for "do_run_on_cpu"  is "do_kvm_cpu_synchronize_post_init". And
that triggered the fourth SIGUSR1. Before I only saw four, hopefully
that is the same with so much breakpoints.

Checked the cpu state again:
1880static void do_kvm_cpu_synchronize_post_init(CPUState *cpu, 
run_on_cpu_data arg)
1881{
1882kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
1883cpu->vcpu_dirty = false;
1884}
1885
(gdb) p cpu
$5 = (CPUState *) 0x5ffe7dc230
(gdb) p *cpu
$6 = {parent_obj = {parent_obj = {class = 0x5ffe7170c0, free = 0x7f62328f15a0 
, properties = 0x5ffe736e40, ref = 1, 
  parent = 0x5ffe726160}, id = 0x0, realized = true, pending_deleted_event 
= false, opts = 0x0, hotplugged = 0, parent_bus = 0x0, gpios = {
  lh_first = 0x0}, child_bus = {lh_first = 0x0}, num_child_bus = 0, 
instance_id_alias = -1, 

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[ChristianEhrhardt]

After this I was trying to start closer to the issue, so I put a break on 
"process_incoming_migration_co" (to skip over much of the initial setup).
Once that was hit I added "qemu_kvm_cpu_thread_fn" and "qemu_kvm_wait_io_event".

Of course when I try that the other functions do not trigger.
Maybe it is partially influenced by the debugging itself and/or the timing 
changes it causes.

I'll check what else I can find with slightly different debugging, but
so much as an update for now.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1711602

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1711602/+subscriptions

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[ChristianEhrhardt]

Since the qemu "lives" in that time I can try to debug what happens.
With strace to sniff where things could be I see right before the end:
 0.000203 recvmsg(27, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="", 
iov_len=32768}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_CMSG_CLOEXEC}, 
MSG_CMSG_CLOEXEC) = 0 <0.14>
 0.49 futex(0xca65dacf4, FUTEX_CMP_REQUEUE_PRIVATE, 1, 2147483647, 
0xca4785a80, 20) = 1 <0.16>
 0.38 getpid()  = 29750 <0.23>
 0.11 tgkill(29750, 29760, SIGUSR1) = 0 <0.30>
 0.12 futex(0xca4785a80, FUTEX_WAKE_PRIVATE, 1) = 1 <0.48>
 0.10 futex(0xca47b46e4, FUTEX_WAIT_PRIVATE, 19, NULL) = 0 <0.002215>
 0.32 sendmsg(21, {msg_name=NULL, msg_namelen=0, 
msg_iov=[{iov_base="{\"timestamp\": {\"seconds\": 1503322067, \"microseconds\": 
613178}, \"event\": \"MIGRATION\", \"data\": {\"status\": \"failed\"}}\r\n", 
iov_len=116}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, 0) = 116 <0.24>
 0.74 write(2, "2017-08-21T13:27:47.613276Z qemu-system-x86_64: load of 
migration failed: Input/output error\n", 93) = 93 <0.22>
 0.55 close(27) = 0 <0.90>

Now 29750 is the main process/tgid and 29760 is the third process started on 
the migration.
It is the one that does the vcpu ioctl's so I assume this is just the one 
representing the vpu.
Well gdb should be more useful so looking with that.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1711602

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1711602/+subscriptions

[Qemu-devel] [Bug 1711602] Re: --copy-storage-all failing with qemu 2.10

[ChristianEhrhardt]

Hi David,
confirming the red-herring on the cpu feature - I had a build without runnign 
over the weekend so this was easy to test - and still the migration fails.

I have about 7 seconds from kicking off the migration until the sync seems to 
pass its first phase and then qemu is exiting (at least that is what libvirt 
thinks):
  "closed without SHUTDOWN event; assuming the domain crashed"

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1711602

Title:
  --copy-storage-all failing with qemu 2.10

Status in QEMU:
  New
Status in libvirt package in Ubuntu:
  Confirmed
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  We fixed an issue around disk locking already in regard to qemu-nbd
  [1], but there still seem to be issues.

  $ virsh migrate --live --copy-storage-all kvmguest-artful-normal 
qemu+ssh://10.22.69.196/system
  error: internal error: qemu unexpectedly closed the monitor: 
2017-08-18T12:10:29.800397Z qemu-system-x86_64: -chardev pty,id=charserial0: 
char device redirected to /dev/pts/0 (label charserial0)
  2017-08-18T12:10:48.545776Z qemu-system-x86_64: load of migration failed: 
Input/output error

  Source libvirt log for the guest:
  2017-08-18 12:09:08.251+: initiating migration
  2017-08-18T12:09:08.809023Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer
  2017-08-18T12:09:08.809481Z qemu-system-x86_64: Unable to read from socket: 
Connection reset by peer

  Target libvirt log for the guest:
  2017-08-18T12:09:08.730911Z qemu-system-x86_64: load of migration failed: 
Input/output error
  2017-08-18 12:09:09.010+: shutting down, reason=crashed

  Given the timing it seems that the actual copy now works (it is busy ~10 
seconds on my environment which would be the copy).
  Also we don't see the old errors we saw before, but afterwards on the actual 
take-over it fails.

  Dmesg has no related denials as often apparmor is in the mix.

  Need to check libvirt logs of source [2] and target [3] in Detail.

  [1]: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02200.html
  [2]: http://paste.ubuntu.com/25339356/
  [3]: http://paste.ubuntu.com/25339358/

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1711602/+subscriptions

Re: [Qemu-devel] [PATCH v4 09/10] s390x/kvm: msi route fixup for non-pci

[Halil Pasic]

On 08/21/2017 02:13 PM, Cornelia Huck wrote:
> On Mon, 21 Aug 2017 14:00:15 +0200
> Halil Pasic  wrote:
> 
>> On 08/21/2017 11:16 AM, Cornelia Huck wrote:
>>> If we don't provide pci, we cannot have a pci device for which we
>>> have to translate to adapter routes: just return -ENODEV.
>>>
>>> Signed-off-by: Cornelia Huck 
>>> ---
>>>  target/s390x/kvm.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index 9de165d8b1..d8db1cbf6e 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -2424,6 +2424,12 @@ int kvm_arch_fixup_msi_route(struct 
>>> kvm_irq_routing_entry *route,
>>>  uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
>>>  uint32_t vec = data & ZPCI_MSI_VEC_MASK;
>>>
>>> +if (!s390_has_feat(S390_FEAT_ZPCI)) {
>>> +/* How can we get here without pci enabled? */
>>> +g_assert(false);  
>>
>> You don't tell us about the g_assert in the commit message.
>> Do you expect G_DISABLE_ASSERT being defined for production 
>> builds. I've grepped for G_DISABLE_ASSERT and found nothing.
> 
> AFAIK this is set by distribution builds. I've also noticed that mingw
> builds treat (g_)assert() as if code flow continues, but I don't know
> whether asserts do anything there at all.
> 

After thinking some more,  I would prefer the the commit message being
modified so, that it's clear, what we really want is the assert; or this
assert being dropped or replaced with some kind of warning/tracing.

I assume no production QEMU should ever return -ENODEV here (and if it
does, it's due to an QEMU bug). So the assert is there to communicate
with the devel, and just in case if the client code fails to fulfill
their part of the contract.  In this case we shall fail fast and hard,
and no such QEMU should ship. The return -ENODEV is then 'just in case'
on the square -- a failsafe for the failsafe (which does make sense
to me).

On the contrary if we assume this is a legit error condition (and a part
of the contract) then according to HACKING 7.2 Handling errors we shall
not call exit() or abort() to handle an error that can be triggered by
the guest in particular and operation related errors in general. Makes
perfect sense to me.

Pierre helped me, kvm_arch_fixup_msi_route is guest triggered and also
considering this particular case killing off the QEMU, and the guest does
not seem like the lesser evil.

The situation is just complicated by the fact that there is code which
relies on assert(true) asserting for correctness (e.g. virtio goes so far
to make builds with normal asserts disabled fail). Thus for me it's hard
to assume that the assertion is guaranteed to be disabled in production.

But if it ain't disabled than it calls abort() which we should not
do (HACKING and IMHO common sense).

TL;DR

I'm for modifying the commit message so it tells us about
the assert.

>>
>> And why g_assert over assert (again no guidance in HACKING
>> mostly asking for my own learning)?
> 
> I do recall a recent(ish) discussion, but not the details. Anyway,
> using glib interfaces seems more consistent.

We can't live with NDEBUG is another reason for using g_assert (makes
my preferred solution work).

Regards,
Halil

[..]

Re: [Qemu-devel] [PATCH v2 0/3] s390x: diag-related things

[Cornelia Huck]

On Fri, 18 Aug 2017 13:48:41 +0200
Cornelia Huck  wrote:

> ...which are a fix in tcg for diag handling and diag288 watchdog changes.
> 
> v1->v2: just reorder patches for less churn [Thomas]
> 
> Cornelia Huck (2):
>   s390x/tcg: specification exception for unknown diag
>   s390x: wire up diag288 in tcg
> 
> Thomas Huth (1):
>   watchdog/wdt_diag288: Mark diag288 watchdog as non-hotpluggable
> 
>  hw/watchdog/wdt_diag288.c  | 1 +
>  target/s390x/misc_helper.c | 6 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 

Queued to s390-next.

Re: [Qemu-devel] [PATCH v4 07/10] s390x/sclp: properly guard pci-specific functions

[Pierre Morel]

On 21/08/2017 11:16, Cornelia Huck wrote:

If we do not provide zpci, pci reconfiguration via sclp is not available
either. Don't indicate it in the sclp facilities and return an invalid
command if the guest tries to issue pci configure/deconfigure.

Reviewed-by: Thomas Huth 
Signed-off-by: Cornelia Huck 
---
  hw/s390x/sclp.c | 19 +++
  1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 9253dbbc64..d0104cd784 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -59,6 +59,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
  int rnsize, rnmax;
  int slots = MIN(machine->ram_slots, s390_get_memslot_count(kvm_state));
  IplParameterBlock *ipib = s390_ipl_get_iplb();
+uint64_t sclp_facilities = SCLP_HAS_CPU_INFO;

  CPU_FOREACH(cpu) {
  cpu_count++;
@@ -79,8 +80,10 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)

  prepare_cpu_entries(sclp, read_info->entries, cpu_count);

-read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
-SCLP_HAS_PCI_RECONFIG);
+if (s390_has_feat(S390_FEAT_ZPCI)) {
+sclp_facilities |= SCLP_HAS_PCI_RECONFIG;
+}
+read_info->facilities = cpu_to_be64(sclp_facilities);

  /* Memory Hotplug is only supported for the ccw machine type */
  if (mhd) {
@@ -385,10 +388,18 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, 
uint32_t code)
  sclp_c->unassign_storage(sclp, sccb);
  break;
  case SCLP_CMDW_CONFIGURE_PCI:
-s390_pci_sclp_configure(sccb);
+if (s390_has_feat(S390_FEAT_ZPCI)) {
+s390_pci_sclp_configure(sccb);
+} else {
+sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);


Hello Conny,

I find more logical if the response would be 0x06F0 : "Adapter type in 
SCCB not recognized"

Since we could have more than one adapter type... someday.
then the SCLP command would be valid but not the adapter type.

However, I will try to find a real hardware to test this since I noticed 
that my logic sometime... :) .


Another point is that don't you think that this test on S390_FEAT_ZPCI 
better belong to the dedicated PCI code inside of the 
s390_pci_sclp_configure().


best regards,

Pierre



+}
  break;
  case SCLP_CMDW_DECONFIGURE_PCI:
-s390_pci_sclp_deconfigure(sccb);
+if (s390_has_feat(S390_FEAT_ZPCI)) {
+s390_pci_sclp_deconfigure(sccb);
+} else {
+sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+}
  break;
  default:
  efc->command_handler(ef, sccb, code);




--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

Re: [Qemu-devel] [PATCH v2 1/6] block: skip implicit nodes in snapshots, blockjobs

[Alberto Garcia]

On Tue 15 Aug 2017 03:50:12 PM CEST, Manos Pitsidianakis wrote:
>>> +static inline BlockDriverState *child_bs(BlockDriverState *bs)
>>> +{
>>> +BdrvChild *child = QLIST_FIRST(>children);
>>> +assert(child && !QLIST_NEXT(child, next));
>>> +return child->bs;
>>> +}
>>
>>This aborts if the bs has a number of children != 1. That's not
>>something that I would expect from a function named like that.
>>
>>Considering that you're only using it in bdrv_get_first_explicit(),
>>why don't you simply move the code there?
>
> It felt useful to have a function that also returns file->bs (we have 
> backing_bs() already) instead of doing backing_bs(bs) ? : file_bs(bs)
>>
>>The other question is of course whether we can rely for the future on
>>the assumption that implicit nodes only have one children.
>
> This is only to get either bs->backing or bs->file (we can't have both
> anyway).

You can have other children (see Quorum for example).

Berto

Re: [Qemu-devel] [PATCH] scripts: Support building with Python 3

[Markus Armbruster]

David Michael  writes:

> This allows building with "./configure --python=python3", where
> the python3 program is at least version 3.6.  It preserves
> compatibility with Python 2.  The changes include:
>
>   - Avoiding "print" usage
>   - Using bytes with files opened in binary mode
>   - Switching .iteritems() to .items()
>   - Adding fallback imports for functions moved to other modules
>
> Signed-off-by: David Michael 
> ---
>
> Hi,
>
> I've been applying these changes when building on Fedora 26, which does
> not include any Python 2 packages by default.  It was tested with Python
> 2.7 and 3.6.
>
> I just saw the list of scripts that need updating on the mailing list,
> and this doesn't cover all of them, but it is enough to build a binary
> for running virtual machines with KVM.  Maybe it is still useful as a
> starting point.
>
> Thanks.
>
> David
>
>  configure|  6 --
>  scripts/qapi.py  | 31 ---
>  scripts/qapi2texi.py | 10 +-
>  scripts/signrom.py   |  4 ++--
>  4 files changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/configure b/configure
> index dd73cce..09f4d68 100755
> --- a/configure
> +++ b/configure
> @@ -1548,9 +1548,11 @@ fi
>  
>  # Note that if the Python conditional here evaluates True we will exit
>  # with status 1 which is a shell 'false' value.
> -if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6) or 
> sys.version_info >= (3,))'; then
> +if ! $python -c 'import sys; sys.exit(sys.version_info >= (3,) and 
> sys.version_info < (3,6))'; then
> +  error_exit "Cannot use '$python', Python 3.6 or later is required." \
> +  "Use --python=/path/to/python3 to specify a supported Python 3."
> +elif ! $python -c 'import sys; sys.exit(sys.version_info < (2,6))'; then
>error_exit "Cannot use '$python', Python 2.6 or later is required." \
> -  "Note that Python 3 or later is not yet supported." \
>"Use --python=/path/to/python to specify a supported Python."
>  fi
>  

Hmm.  If we detect a Python 2 that is too old, we ask the user to
provide a sufficiently new Python 2.  We don't ask him for a
sufficiently new Python 3, even though that would work, too.  Should we
tweak the error message a bit?

> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 8aa2775..6450998 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -15,9 +15,11 @@ import errno
>  import getopt
>  import os
>  import re
> -import string
>  import sys
> -from ordereddict import OrderedDict
> +try:
> +from collections import OrderedDict
> +except ImportError:
> +from ordereddict import OrderedDict
>  
>  builtin_types = {
>  'null': 'QTYPE_QNULL',
> @@ -252,7 +254,7 @@ class QAPIDoc(object):
> "'Returns:' is only valid for commands")
>  
>  def check(self):
> -bogus = [name for name, section in self.args.iteritems()
> +bogus = [name for name, section in self.args.items()
>   if not section.member]
>  if bogus:
>  raise QAPISemError(

Efficiency loss with Python 2.  Not critical, just sad.

> @@ -308,7 +310,7 @@ class QAPISchemaParser(object):
>  if not isinstance(pragma, dict):
>  raise QAPISemError(
>  info, "Value of 'pragma' must be a dictionary")
> -for name, value in pragma.iteritems():
> +for name, value in pragma.items():
>  self._pragma(name, value, info)
>  else:
>  expr_elem = {'expr': expr,
> @@ -1574,7 +1576,7 @@ class QAPISchema(object):
>  
>  def _make_members(self, data, info):
>  return [self._make_member(key, value, info)
> -for (key, value) in data.iteritems()]
> +for (key, value) in data.items()]
>  
>  def _def_struct_type(self, expr, info, doc):
>  name = expr['struct']
> @@ -1606,11 +1608,11 @@ class QAPISchema(object):
>  name, info, doc, 'base', self._make_members(base, info)))
>  if tag_name:
>  variants = [self._make_variant(key, value)
> -for (key, value) in data.iteritems()]
> +for (key, value) in data.items()]
>  members = []
>  else:
>  variants = [self._make_simple_variant(key, value, info)
> -for (key, value) in data.iteritems()]
> +for (key, value) in data.items()]
>  typ = self._make_implicit_enum_type(name, info,
>  [v.name for v in variants])
>  tag_member = QAPISchemaObjectTypeMember('type', typ, False)
> @@ -1625,7 +1627,7 @@ class QAPISchema(object):
>  name = expr['alternate']
>  data = expr['data']
>  variants = [self._make_variant(key, value)
> -for (key, value) in 

Re: [Qemu-devel] [PATCH v6 5/6] block: add throttle block filter driver

[Alberto Garcia]

On Mon 21 Aug 2017 11:31:49 AM CEST, Manos Pitsidianakis wrote:
> block/throttle.c uses existing I/O throttle infrastructure inside a
> block filter driver. I/O operations are intercepted in the filter's
> read/write coroutines, and referred to block/throttle-groups.c
>
> The driver can be used with the syntax
> -drive driver=throttle,file.filename=foo.qcow2, \
> limits.iops-total=...
>
> which creates an anonymous group with the specified limits, or

Weren't we going to forbid anonymous groups? :-?

Berto

[Qemu-devel] [PATCH v4 4/4] scsi-block: Support rerror/werror

[Fam Zheng]

This makes the werror/rerror options available on the scsi-block device,
to allow user specify error handling policy similar to scsi-hd.

Signed-off-by: Fam Zheng 
---
 hw/scsi/scsi-disk.c | 38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 5f1e5e8070..cf38f93b9d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -39,6 +39,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 #include "hw/block/block.h"
 #include "sysemu/dma.h"
 #include "qemu/cutils.h"
+#include "scsi/scsi.h"
 
 #ifdef __linux
 #include 
@@ -106,7 +107,7 @@ typedef struct SCSIDiskState
 bool tray_locked;
 } SCSIDiskState;
 
-static int scsi_handle_rw_error(SCSIDiskReq *r, int error, bool acct_failed);
+static bool scsi_handle_rw_error(SCSIDiskReq *r, int error, bool acct_failed);
 
 static void scsi_free_request(SCSIRequest *req)
 {
@@ -184,19 +185,10 @@ static bool scsi_disk_req_check_error(SCSIDiskReq *r, int 
ret, bool acct_failed)
 return true;
 }
 
-if (ret < 0) {
+if (ret < 0 || (r->status && *r->status)) {
 return scsi_handle_rw_error(r, -ret, acct_failed);
 }
 
-if (r->status && *r->status) {
-if (acct_failed) {
-SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-block_acct_failed(blk_get_stats(s->qdev.conf.blk), >acct);
-}
-scsi_req_complete(>req, *r->status);
-return true;
-}
-
 return false;
 }
 
@@ -422,13 +414,13 @@ static void scsi_read_data(SCSIRequest *req)
 }
 
 /*
- * scsi_handle_rw_error has two return values.  0 means that the error
- * must be ignored, 1 means that the error has been processed and the
+ * scsi_handle_rw_error has two return values.  False means that the error
+ * must be ignored, true means that the error has been processed and the
  * caller should not do anything else for this request.  Note that
  * scsi_handle_rw_error always manages its reference counts, independent
  * of the return value.
  */
-static int scsi_handle_rw_error(SCSIDiskReq *r, int error, bool acct_failed)
+static bool scsi_handle_rw_error(SCSIDiskReq *r, int error, bool acct_failed)
 {
 bool is_read = (r->req.cmd.mode == SCSI_XFER_FROM_DEV);
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
@@ -440,6 +432,11 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, 
bool acct_failed)
 block_acct_failed(blk_get_stats(s->qdev.conf.blk), >acct);
 }
 switch (error) {
+case 0:
+/* The command has run, no need to fake sense.  */
+assert(r->status && *r->status);
+scsi_req_complete(>req, *r->status);
+break;
 case ENOMEDIUM:
 scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
 break;
@@ -457,6 +454,18 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, 
bool acct_failed)
 break;
 }
 }
+if (!error) {
+assert(r->status && *r->status);
+error = scsi_sense_buf_to_errno(r->req.sense, sizeof(r->req.sense));
+
+if (error == ECANCELED || error == EAGAIN || error == ENOTCONN ||
+error == 0)  {
+/* These errors are handled by guest. */
+scsi_req_complete(>req, *r->status);
+return true;
+}
+}
+
 blk_error_action(s->qdev.conf.blk, action, is_read, error);
 if (action == BLOCK_ERROR_ACTION_STOP) {
 scsi_req_retry(>req);
@@ -2972,6 +2981,7 @@ static const TypeInfo scsi_cd_info = {
 
 #ifdef __linux__
 static Property scsi_block_properties[] = {
+DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
 DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_PROP_END_OF_LIST(),
 };
-- 
2.13.5

[Qemu-devel] [PATCH v4 2/4] scsi: Improve scsi_sense_to_errno

[Fam Zheng]

Tweak the errno mapping to return more accurate/appropriate values.

Signed-off-by: Fam Zheng 
---
 util/scsi.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/util/scsi.c b/util/scsi.c
index a6710799fc..472eb5bea5 100644
--- a/util/scsi.c
+++ b/util/scsi.c
@@ -18,13 +18,16 @@
 int scsi_sense_to_errno(int key, int asc, int ascq)
 {
 switch (key) {
-case 0x02: /* NOT READY */
-return EBUSY;
-case 0x07: /* DATA PROTECTION */
-return EACCES;
+case 0x00: /* NO SENSE */
+case 0x01: /* RECOVERED ERROR */
+case 0x06: /* UNIT ATTENTION */
+/* These sense keys are not errors */
+return 0;
 case 0x0b: /* COMMAND ABORTED */
 return ECANCELED;
+case 0x02: /* NOT READY */
 case 0x05: /* ILLEGAL REQUEST */
+case 0x07: /* DATA PROTECTION */
 /* Parse ASCQ */
 break;
 default:
@@ -37,6 +40,7 @@ int scsi_sense_to_errno(int key, int asc, int ascq)
 case 0x2600: /* INVALID FIELD IN PARAMETER LIST */
 return EINVAL;
 case 0x2100: /* LBA OUT OF RANGE */
+case 0x2707: /* SPACE ALLOC FAILED */
 return ENOSPC;
 case 0x2500: /* LOGICAL UNIT NOT SUPPORTED */
 return ENOTSUP;
@@ -46,6 +50,10 @@ int scsi_sense_to_errno(int key, int asc, int ascq)
 return ENOMEDIUM;
 case 0x2700: /* WRITE PROTECTED */
 return EACCES;
+case 0x0401: /* NOT READY, IN PROGRESS OF BECOMING READY */
+return EAGAIN;
+case 0x0402: /* NOT READY, INITIALIZING COMMAND REQUIRED */
+return ENOTCONN;
 default:
 return EIO;
 }
-- 
2.13.5

[Qemu-devel] [PATCH v4 3/4] scsi: Introduce scsi_sense_buf_to_errno

[Fam Zheng]

This recognizes the "fixed" and "descriptor" format sense data, extracts
the sense key/asc/ascq fields then converts them to an errno.

Signed-off-by: Fam Zheng 
---
 include/scsi/scsi.h |  1 +
 util/scsi.c | 30 ++
 2 files changed, 31 insertions(+)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index f894ace4bf..fe330385d8 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -15,5 +15,6 @@
 #define QEMU_SCSI_H
 
 int scsi_sense_to_errno(int key, int asc, int ascq);
+int scsi_sense_buf_to_errno(const uint8_t *sense, size_t sense_size);
 
 #endif
diff --git a/util/scsi.c b/util/scsi.c
index 472eb5bea5..472293d59b 100644
--- a/util/scsi.c
+++ b/util/scsi.c
@@ -58,3 +58,33 @@ int scsi_sense_to_errno(int key, int asc, int ascq)
 return EIO;
 }
 }
+
+int scsi_sense_buf_to_errno(const uint8_t *sense, size_t sense_size)
+{
+int key, asc, ascq;
+if (sense_size < 1) {
+return EIO;
+}
+switch (sense[0]) {
+case 0x70: /* Fixed format sense data. */
+if (sense_size < 14) {
+return EIO;
+}
+key = sense[2] & 0xF;
+asc = sense[12];
+ascq = sense[13];
+break;
+case 0x72: /* Descriptor format sense data. */
+if (sense_size < 4) {
+return EIO;
+}
+key = sense[1] & 0xF;
+asc = sense[2];
+ascq = sense[3];
+break;
+default:
+return EIO;
+break;
+}
+return scsi_sense_to_errno(key, asc, ascq);
+}
-- 
2.13.5

[Qemu-devel] [PATCH v4 1/4] scsi: Refactor scsi sense interpreting code

[Fam Zheng]

So that it can be reused outside of iscsi.c.

Also update MAINTAINERS to include the new files in SCSI section.

Signed-off-by: Fam Zheng 
---
 MAINTAINERS |  2 ++
 block/iscsi.c   | 45 -
 include/scsi/scsi.h | 19 +++
 util/Makefile.objs  |  1 +
 util/scsi.c | 52 
 5 files changed, 78 insertions(+), 41 deletions(-)
 create mode 100644 include/scsi/scsi.h
 create mode 100644 util/scsi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ccee28b12d..2a4e5036ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -969,7 +969,9 @@ SCSI
 M: Paolo Bonzini 
 S: Supported
 F: include/hw/scsi/*
+F: include/scsi/*
 F: hw/scsi/*
+F: util/scsi*
 F: tests/virtio-scsi-test.c
 T: git git://github.com/bonzini/qemu.git scsi-next
 
diff --git a/block/iscsi.c b/block/iscsi.c
index d557c99668..4bed63cd6d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -40,6 +40,7 @@
 #include "qmp-commands.h"
 #include "qapi/qmp/qstring.h"
 #include "crypto/secret.h"
+#include "scsi/scsi.h"
 
 #include 
 #include 
@@ -209,47 +210,9 @@ static inline unsigned exp_random(double mean)
 
 static int iscsi_translate_sense(struct scsi_sense *sense)
 {
-int ret;
-
-switch (sense->key) {
-case SCSI_SENSE_NOT_READY:
-return -EBUSY;
-case SCSI_SENSE_DATA_PROTECTION:
-return -EACCES;
-case SCSI_SENSE_COMMAND_ABORTED:
-return -ECANCELED;
-case SCSI_SENSE_ILLEGAL_REQUEST:
-/* Parse ASCQ */
-break;
-default:
-return -EIO;
-}
-switch (sense->ascq) {
-case SCSI_SENSE_ASCQ_PARAMETER_LIST_LENGTH_ERROR:
-case SCSI_SENSE_ASCQ_INVALID_OPERATION_CODE:
-case SCSI_SENSE_ASCQ_INVALID_FIELD_IN_CDB:
-case SCSI_SENSE_ASCQ_INVALID_FIELD_IN_PARAMETER_LIST:
-ret = -EINVAL;
-break;
-case SCSI_SENSE_ASCQ_LBA_OUT_OF_RANGE:
-ret = -ENOSPC;
-break;
-case SCSI_SENSE_ASCQ_LOGICAL_UNIT_NOT_SUPPORTED:
-ret = -ENOTSUP;
-break;
-case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT:
-case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT_TRAY_CLOSED:
-case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT_TRAY_OPEN:
-ret = -ENOMEDIUM;
-break;
-case SCSI_SENSE_ASCQ_WRITE_PROTECTED:
-ret = -EACCES;
-break;
-default:
-ret = -EIO;
-break;
-}
-return ret;
+return - scsi_sense_to_errno(sense->key,
+ (sense->ascq & 0xFF00) >> 8,
+ sense->ascq & 0xFF);
 }
 
 /* Called (via iscsi_service) with QemuMutex held.  */
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
new file mode 100644
index 00..f894ace4bf
--- /dev/null
+++ b/include/scsi/scsi.h
@@ -0,0 +1,19 @@
+/*
+ *  SCSI helpers
+ *
+ *  Copyright 2017 Red Hat, Inc.
+ *
+ *  Authors:
+ *   Fam Zheng 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+#ifndef QEMU_SCSI_H
+#define QEMU_SCSI_H
+
+int scsi_sense_to_errno(int key, int asc, int ascq);
+
+#endif
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 50a55ecc75..c9e6c493d3 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -45,3 +45,4 @@ util-obj-y += qht.o
 util-obj-y += range.o
 util-obj-y += stats64.o
 util-obj-y += systemd.o
+util-obj-y += scsi.o
diff --git a/util/scsi.c b/util/scsi.c
new file mode 100644
index 00..a6710799fc
--- /dev/null
+++ b/util/scsi.c
@@ -0,0 +1,52 @@
+/*
+ *  SCSI helpers
+ *
+ *  Copyright 2017 Red Hat, Inc.
+ *
+ *  Authors:
+ *   Fam Zheng 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "scsi/scsi.h"
+
+int scsi_sense_to_errno(int key, int asc, int ascq)
+{
+switch (key) {
+case 0x02: /* NOT READY */
+return EBUSY;
+case 0x07: /* DATA PROTECTION */
+return EACCES;
+case 0x0b: /* COMMAND ABORTED */
+return ECANCELED;
+case 0x05: /* ILLEGAL REQUEST */
+/* Parse ASCQ */
+break;
+default:
+return EIO;
+}
+switch ((asc << 8) | ascq) {
+case 0x1a00: /* PARAMETER LIST LENGTH ERROR */
+case 0x2000: /* INVALID OPERATION CODE */
+case 0x2400: /* INVALID FIELD IN CDB */
+case 0x2600: /* INVALID FIELD IN PARAMETER LIST */
+return EINVAL;
+case 0x2100: /* LBA OUT OF RANGE */
+return ENOSPC;
+case 0x2500: /* LOGICAL UNIT NOT SUPPORTED */
+return ENOTSUP;
+case 0x3a00: /* MEDIUM NOT PRESENT */
+

[Qemu-devel] [PATCH v4 0/4] scsi-block: Support werror/rerror

[Fam Zheng]

v4: Handle more sense keys/ascqs. [Paolo]
Add scsi_sense_buf_to_errno to util/scsi.c. [Paolo]
Shorten the comment notes on SCSI constants.

v3: Enhance and reuse iscsi sense data translation. [Paolo]

First refactor and enhance the code in iscsi for sense data translation, then
modify error handing of scsi-block and expose the qdev properties.

Fam Zheng (4):
  scsi: Refactor scsi sense interpreting code
  scsi: Improve scsi_sense_to_errno
  scsi: Introduce scsi_sense_buf_to_errno
  scsi-block: Support rerror/werror

 MAINTAINERS |  2 ++
 block/iscsi.c   | 45 +++
 hw/scsi/scsi-disk.c | 38 +-
 include/scsi/scsi.h | 20 
 util/Makefile.objs  |  1 +
 util/scsi.c | 90 +
 6 files changed, 141 insertions(+), 55 deletions(-)
 create mode 100644 include/scsi/scsi.h
 create mode 100644 util/scsi.c

-- 
2.13.5

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Dr. David Alan Gilbert]

* Fam Zheng (f...@redhat.com) wrote:
> On Mon, 08/21 11:17, Dr. David Alan Gilbert wrote:
> > From previous discussions we've had, one suggestion was to have some
> > type of 'safe' command; once issued in a thread, the monitor thread
> > would only allow other lock-free commands to be issued; it stops any
> > accidents of them issuing unsafe commands.
> 
> I'm not sure I understand. If the 'safe' command is not issued, users are
> allowed to do unsafe things? What are the possible consequences of those
> 'unsafe' commands? Errors/hangs/crashes?

With or without the safe command no command could cause a crash.
However, a command might try and take the bql and block waiting for it.
With the 'safe' command only those commands that were declared as
not-wanting the lock would be allowed.

Dave

> Fam
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Fam Zheng]

On Mon, 08/21 11:17, Dr. David Alan Gilbert wrote:
> From previous discussions we've had, one suggestion was to have some
> type of 'safe' command; once issued in a thread, the monitor thread
> would only allow other lock-free commands to be issued; it stops any
> accidents of them issuing unsafe commands.

I'm not sure I understand. If the 'safe' command is not issued, users are
allowed to do unsafe things? What are the possible consequences of those
'unsafe' commands? Errors/hangs/crashes?

Fam

Re: [Qemu-devel] AVMF & OVMF blobs in QEMU tree???

[Alex Bennée]

Gerd Hoffmann  writes:

> On Mon, 2017-08-07 at 18:51 +0200, Laszlo Ersek wrote:
>> On 08/07/17 16:40, Peter Maydell wrote:
>> > On 7 August 2017 at 15:31, Igor Mammedov 
>> > wrote:
>> > > As I recall there were issues with FAT driver licensing in edk2,
>> > > but I've heard there were some changes in that regard.
>> > >
>> > > Is there any other reasons why we are not putting subj.
>> > > in QEMU tree like we do with SeaBIOS and other roms?
>> >
>> > I suspect the primary answer is "nobody who's willing to
>> > maintain, test and update the resulting binary blobs has
>> > stepped forward to say they want to do so" :-)
>> >
>> > (I think that shipping them in the QEMU tree would be
>> > nice but is principally a convenience for our direct
>> > users, since distros are going to want to build their
>> > own ROM blobs from source anyway.)
>>
>> I agree that OVMF and ArmVirtQemu firmware binaries (and matching
>> varstore templates, likely compressed) should be bundled with QEMU.
>> There are no license-related reasons left that would prevent this.
>>
>> Please let us discuss this when Gerd returns from vacation. (CC'ing
>> Gerd.)
>
> slighly oldish wip branch:
> https://www.kraxel.org/cgit/qemu/log/?h=work/edk2
>
> Related question (as the edk2 blobs are pretty big):  Do we want commit
> this to the qemu repo directly?   Or should we create a qemu-firmware
> repo for the precompiled blobs and hook it up as submodule?

How big? Things like github tend to get picky with blobs over 50Mb and
git isn't really set-up for large binary bits hence things like git-lfs.
Unfortunately there seems to be multiple solutions to the blobs in git
problem.

We should also take some care to reproducibility so they can be updated
by someone other than the original author.

>
> cheers,
>   Gerd


--
Alex Bennée

[Qemu-devel] [PATCH] xen: Emit RTC_CHANGE upon TIMEOFFSET ioreq

[Ross Lagerwall]

When the guest writes to the RTC, Xen emulates it and broadcasts a
TIMEOFFSET ioreq. Emit an RTC_CHANGE QMP message when this happens
rather than ignoring it so that something useful can be done with the
information.

Signed-off-by: Ross Lagerwall 
---
 hw/i386/xen/xen-hvm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index d9ccd5d..ffd20dc 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -16,6 +16,7 @@
 #include "hw/i386/apic-msidef.h"
 #include "hw/xen/xen_common.h"
 #include "hw/xen/xen_backend.h"
+#include "qapi-event.h"
 #include "qmp-commands.h"
 
 #include "qemu/error-report.h"
@@ -967,6 +968,7 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)
 handle_vmport_ioreq(state, req);
 break;
 case IOREQ_TYPE_TIMEOFFSET:
+qapi_event_send_rtc_change((int64_t)req->data, _abort);
 break;
 case IOREQ_TYPE_INVALIDATE:
 xen_invalidate_map_cache();
-- 
2.9.5

[Qemu-devel] [PATCH v2] qcow2: allocate cluster_cache/cluster_data on demand

[Stefan Hajnoczi]

Most qcow2 files are uncompressed so it is wasteful to allocate (32 + 1)
* cluster_size + 512 bytes upfront.  Allocate s->cluster_cache and
s->cluster_data when the first read operation is performance on a
compressed cluster.

The buffers are freed in .bdrv_close().  .bdrv_open() no longer has any
code paths that can allocate these buffers, so remove the free functions
in the error code path.

This patch can result in significant memory savings when many qcow2
disks are attached or backing file chains are long:

Before 12.81% (1,023,193,088B)
After   5.36% (393,893,888B)

Reported-by: Alexey Kardashevskiy 
Tested-by: Alexey Kardashevskiy 
Cc: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
v2:
 * Changed EIO to ENOMEM [Eric]
 * Added Alexey's Tested-by
---
 block/qcow2-cluster.c | 17 +
 block/qcow2.c | 12 
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..8538533102 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1516,6 +1516,23 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
uint64_t cluster_offset)
 nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1;
 sector_offset = coffset & 511;
 csize = nb_csectors * 512 - sector_offset;
+
+/* Allocate buffers on first decompress operation, most images are
+ * uncompressed and the memory overhead can be avoided.  The buffers
+ * are freed in .bdrv_close().
+ */
+if (!s->cluster_data) {
+/* one more sector for decompressed data alignment */
+s->cluster_data = qemu_try_blockalign(bs->file->bs,
+QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
+if (!s->cluster_data) {
+return -ENOMEM;
+}
+}
+if (!s->cluster_cache) {
+s->cluster_cache = g_malloc(s->cluster_size);
+}
+
 BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED);
 ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data,
 nb_csectors);
diff --git a/block/qcow2.c b/block/qcow2.c
index 40ba26c111..0ac201910a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1360,16 +1360,6 @@ static int qcow2_do_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
-s->cluster_cache = g_malloc(s->cluster_size);
-/* one more sector for decompressed data alignment */
-s->cluster_data = qemu_try_blockalign(bs->file->bs, QCOW_MAX_CRYPT_CLUSTERS
-* s->cluster_size + 512);
-if (s->cluster_data == NULL) {
-error_setg(errp, "Could not allocate temporary cluster buffer");
-ret = -ENOMEM;
-goto fail;
-}
-
 s->cluster_cache_offset = -1;
 s->flags = flags;
 
@@ -1507,8 +1497,6 @@ static int qcow2_do_open(BlockDriverState *bs, QDict 
*options, int flags,
 if (s->refcount_block_cache) {
 qcow2_cache_destroy(bs, s->refcount_block_cache);
 }
-g_free(s->cluster_cache);
-qemu_vfree(s->cluster_data);
 qcrypto_block_free(s->crypto);
 qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
 return ret;
-- 
2.13.5

Re: [Qemu-devel] [RFC 0/6] monitor: allow per-monitor thread

[Fam Zheng]

On Mon, 08/21 18:05, Peter Xu wrote:
> On Mon, Aug 21, 2017 at 04:58:51PM +0800, Fam Zheng wrote:
> > On Mon, 08/21 15:44, Peter Xu wrote:
> > > This is an extended work for migration postcopy recovery. This series
> > > is tested with the following series to make sure it solves the monitor
> > > hang problem that we have encountered for postcopy recovery:
> > > 
> > >   [RFC 00/29] Migration: postcopy failure recovery
> > >   [RFC 0/6] migration: re-use migrate_incoming for postcopy recovery
> > > 
> > > The root problem is that, monitor commands are all handled in main
> > > loop thread now, no matter how many monitors we specify. And, if main
> > > loop thread hangs due to some reason, all monitors will be stuck.
> > > This can be done in reversed order as well: if any of the monitor
> > > hangs, it will hang the main loop, and the rest of the monitors (if
> > > there is any).
> > > 
> > > That affects postcopy recovery, since the recovery requires user input
> > > on destination side.  If monitors hang, the destination VM dies and
> > > lose hope for even a final recovery.
> > > 
> > > So, sometimes we need to make sure the monitor be alive, at least one
> > > of them.
> > > 
> > > The whole idea of this series is that instead if handling monitor
> > > commands all in main loop thread, we do it separately in per-monitor
> > > threads.  Then, even if main loop thread hangs at any point by any
> > > reason, per-monitor thread can still survive.  Further, we add hint in
> > > QMP/HMP to show whether a command can be executed without QMP, if so,
> > > we avoid taking BQL when running that command.  It greatly reduced
> > > contention of BQL.  Now the only user of that new parameter (currently
> > > I call it "without-bql") is "migrate-incoming" command, which is the
> > > only command to rescue a paused postcopy migration.
> > > 
> > > However, even with the series, it does not mean that per-monitor
> > > threads will never hang.  One example is that we can still run "info
> > > vcpus" in per-monitor threads during a paused postcopy (in that state,
> > > page faults are never handled, and "info cpus" will never return since
> > > it tries to sync every vcpus).  So to make sure it does not hang, we
> > > not only need the per-monitor thread, the user should be careful as
> > > well on how to use it.
> > 
> > I think this is like saying we expect the user to understand the internals 
> > of
> > QEMU, unless the "rules" are clearly documented.  Taking this into account,
> > does it make sense to make the per-monitor thread only allow BQL-free 
> > commands?
> 
> I don't think users need to know the internals - they just need to be
> careful on using them.  Just take the example of "info cpus": during
> paused postcopy it will hang, but IMHO it does not mean that it's
> illegal for user to send that command.  It's "by-design" that it'll be
> stuck if one of the vcpus is stuck somewhere; it's just not the
> correct way to use it when the monitor is prepared for postcopy
> recovery.

They still need to know "what" is the correct way to use the monitor, and what
I'm saying is there doesn't seem to be an easy way for users to know exactly
what is correct. See below.

> 
> And IMHO we should not treat threaded monitors special - it should be
> exactly the same monitor service when used with main loop thread.  It
> just has its own thread to handle the requests, so it is less
> dependent on main loop thread, and that's all.

It's not that simple, I think all non-trivial commands need very careful audit
before assuming they're safe. For example many block related commands
(qmp_trasaction, for example) indirectly calls BDRV_POLL_WHILE(), which, if
called from a per-monitor thread, will enter the else branch then fail the first
assert.

Fam

Re: [Qemu-devel] [PATCH] qcow2: allocate cluster_cache/cluster_data on demand

[Stefan Hajnoczi]

On Fri, Aug 18, 2017 at 10:18:37AM -0500, Eric Blake wrote:
> On 08/18/2017 08:31 AM, Stefan Hajnoczi wrote:
> > Most qcow2 files are uncompressed so it is wasteful to allocate (32 + 1)
> > * cluster_size + 512 bytes upfront.  Allocate s->cluster_cache and
> > s->cluster_data when the first read operation is performance on a
> > compressed cluster.
> > 
> > The buffers are freed in .bdrv_close().  .bdrv_open() no longer has any
> > code paths that can allocate these buffers, so remove the free functions
> > in the error code path.
> > 
> > Reported-by: Alexey Kardashevskiy 
> > Cc: Kevin Wolf 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> > Alexey: Does this improve your memory profiling results?
> 
> Is this a regression from earlier versions?  Likely, it is NOT -rc4
> material, and thus can wait for 2.11; but it should be fine for -stable
> as part of 2.10.1 down the road.

It's not a regression.  s->cluster_data was always allocated upfront in
.bdrv_open().

> > +++ b/block/qcow2-cluster.c
> > @@ -1516,6 +1516,23 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
> > uint64_t cluster_offset)
> >  nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) 
> > + 1;
> >  sector_offset = coffset & 511;
> >  csize = nb_csectors * 512 - sector_offset;
> > +
> > +/* Allocate buffers on first decompress operation, most images are
> > + * uncompressed and the memory overhead can be avoided.  The 
> > buffers
> > + * are freed in .bdrv_close().
> > + */
> > +if (!s->cluster_data) {
> > +/* one more sector for decompressed data alignment */
> > +s->cluster_data = qemu_try_blockalign(bs->file->bs,
> > +QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
> > +if (!s->cluster_data) {
> > +return -EIO;
> 
> Is -ENOMEM any better than -EIO here?

There is another instance of -ENOMEM in qcow2.c so it seems reasonable
to use the more specific ENOMEM error code.

I'll resend the patch.

Stefan

Re: [Qemu-devel] AVMF & OVMF blobs in QEMU tree???

[Peter Maydell]

On 21 August 2017 at 12:58, Gerd Hoffmann  wrote:
> Related question (as the edk2 blobs are pretty big):  Do we want commit
> this to the qemu repo directly?   Or should we create a qemu-firmware
> repo for the precompiled blobs and hook it up as submodule?

Having all our precompiled blobs be in a submodule would
maybe be handy for properly keeping them separate from the
QEMU code -- would that be useful for downstream distro
packagers?

In any case this seems like a good opportunity to write down
exactly what our policy about ROM blobs is (eg where they
are, how we update them, requirement for source, whether
we allow source to be in a submodule not on qemu.org...)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/15] Convert over to use keycodemapdb

[Gerd Hoffmann]

On Thu, 2017-08-10 at 16:55 +0100, Daniel P. Berrange wrote:
> The keycodemap project[1] provides a database mapping between
> many different keysym/keycode/scancode sets, along with a
> tool to generate mapping/lookup tables in various programming
> languages. It is already used by GTK-VNC, SPICE-GTK and
> libvirt.
> 
> This series enables its use in QEMU, thus fixing a great
> many bugs/ommissions in the 15+ key mapping tables people
> have manually written for QEMU.

Looks good on a quick look.  I'll have a closer look and test once I'm
done processing my vacation email backlog.

One thing for now:  I think we should not require the keycodemapdb at
build time.   Instead commit the generated files to git so the build
works without the submodule, then update the generated files each time
the submodule is updated (simliar to firmware blobs/submodules).

cheers,
  Gerd