Re: [Qemu-devel] kvm_intel fails to load on Conroe CPUs running Linux 4.12

[Gerhard Wiesinger]

On 10.11.2017 17:29, Paolo Bonzini wrote:

On 10/11/2017 16:33, Gerhard Wiesinger wrote:

Hello Paolo,

Any update for a new patch?

Yes,

https://marc.info/?l=kvm=150997149623548=2

Paolo


Works also for Fedora 26: 
https://koji.fedoraproject.org/koji/buildinfo?buildID=996781

kernel-4.13.12-200.fc26

Ready to release to 4.13.* upstream and 4.14 upstream kernels ...

https://bodhi.fedoraproject.org/updates/FEDORA-2017-abda708cee

Thnx.

Ciao,
Gerhard

Re: [Qemu-devel] [PATCH] linux-user: Support explicit targets for PowerPC

[no-reply]

Hi,

This series failed build test on s390x host. Please find the details below.

Message-id: 492245211.936616.1510023015797.javamail.zim...@xes-inc.com
Type: series
Subject: [Qemu-devel] [PATCH] linux-user: Support explicit targets for PowerPC

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/492245211.936616.1510023015797.javamail.zim...@xes-inc.com -> 
patchew/492245211.936616.1510023015797.javamail.zim...@xes-inc.com
Switched to a new branch 'test'
f1a6ed2 linux-user: Support explicit targets for PowerPC

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=81166
SHELL=/bin/sh
USER=fam
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
PATH=/usr/bin:/bin
PWD=/var/tmp/patchew-tester-tmp-x9mdrt3d/src
LANG=en_US.UTF-8
HOME=/home/fam
SHLVL=2
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
xz-libs-5.2.2-2.fc24.s390x
libxshmfence-1.2-3.fc24.s390x
giflib-4.1.6-15.fc24.s390x
trousers-lib-0.3.13-6.fc24.s390x
ncurses-base-6.0-6.20160709.fc25.noarch
gmp-6.1.1-1.fc25.s390x
libidn-1.33-1.fc25.s390x
slang-2.3.0-7.fc25.s390x
pkgconfig-0.29.1-1.fc25.s390x
alsa-lib-1.1.1-2.fc25.s390x
yum-metadata-parser-1.1.4-17.fc25.s390x
python3-slip-dbus-0.6.4-4.fc25.noarch
python2-cssselect-0.9.2-1.fc25.noarch
createrepo_c-libs-0.10.0-6.fc25.s390x
initscripts-9.69-1.fc25.s390x
parted-3.2-21.fc25.s390x
flex-2.6.0-3.fc25.s390x
colord-libs-1.3.4-1.fc25.s390x
python-osbs-client-0.33-3.fc25.noarch
perl-Pod-Simple-3.35-1.fc25.noarch
python2-simplejson-3.10.0-1.fc25.s390x
brltty-5.4-2.fc25.s390x
librados2-10.2.4-2.fc25.s390x
tcp_wrappers-7.6-83.fc25.s390x
libcephfs_jni1-10.2.4-2.fc25.s390x
nettle-devel-3.3-1.fc25.s390x
bzip2-devel-1.0.6-21.fc25.s390x
libuuid-2.28.2-2.fc25.s390x
python3-dnf-1.1.10-6.fc25.noarch
texlive-kpathsea-doc-svn41139-33.fc25.1.noarch
openssh-7.4p1-4.fc25.s390x
texlive-kpathsea-bin-svn40473-33.20160520.fc25.1.s390x
texlive-graphics-svn41015-33.fc25.1.noarch
texlive-dvipdfmx-def-svn40328-33.fc25.1.noarch
texlive-mfware-svn40768-33.fc25.1.noarch
texlive-texlive-scripts-svn41433-33.fc25.1.noarch
texlive-euro-svn22191.1.1-33.fc25.1.noarch
texlive-etex-svn37057.0-33.fc25.1.noarch
texlive-iftex-svn29654.0.2-33.fc25.1.noarch
texlive-palatino-svn31835.0-33.fc25.1.noarch
texlive-texlive-docindex-svn41430-33.fc25.1.noarch
texlive-xunicode-svn30466.0.981-33.fc25.1.noarch
texlive-koma-script-svn41508-33.fc25.1.noarch
texlive-pst-grad-svn15878.1.06-33.fc25.1.noarch
texlive-pst-blur-svn15878.2.0-33.fc25.1.noarch
texlive-jknapltx-svn19440.0-33.fc25.1.noarch
texinfo-6.1-4.fc25.s390x
openssl-devel-1.0.2k-1.fc25.s390x
jansson-2.10-2.fc25.s390x
fedora-repos-25-4.noarch
perl-Errno-1.25-387.fc25.s390x
acl-2.2.52-13.fc25.s390x
systemd-pam-231-17.fc25.s390x
NetworkManager-libnm-1.4.4-5.fc25.s390x
poppler-0.45.0-5.fc25.s390x
ccache-3.3.4-1.fc25.s390x
valgrind-3.12.0-9.fc25.s390x
perl-open-1.10-387.fc25.noarch
libgcc-6.4.1-1.fc25.s390x
libsoup-2.56.1-1.fc25.s390x
libstdc++-devel-6.4.1-1.fc25.s390x
libobjc-6.4.1-1.fc25.s390x
python2-rpm-4.13.0.1-2.fc25.s390x
python2-gluster-3.10.5-1.fc25.s390x
rpm-build-4.13.0.1-2.fc25.s390x
glibc-static-2.24-10.fc25.s390x
lz4-1.8.0-1.fc25.s390x
xapian-core-libs-1.2.24-1.fc25.s390x
elfutils-libelf-devel-0.169-1.fc25.s390x
nss-softokn-3.32.0-1.2.fc25.s390x
pango-1.40.9-1.fc25.s390x
glibc-debuginfo-common-2.24-10.fc25.s390x
libaio-0.3.110-6.fc24.s390x
libfontenc-1.1.3-3.fc24.s390x
lzo-2.08-8.fc24.s390x
isl-0.14-5.fc24.s390x
libXau-1.0.8-6.fc24.s390x
linux-atm-libs-2.5.1-14.fc24.s390x
libXext-1.3.3-4.fc24.s390x
libXxf86vm-1.1.4-3.fc24.s390x
bison-3.0.4-4.fc24.s390x
perl-srpm-macros-1-20.fc25.noarch
gawk-4.1.3-8.fc25.s390x
libwayland-client-1.12.0-1.fc25.s390x
perl-Exporter-5.72-366.fc25.noarch
perl-version-0.99.17-1.fc25.s390x
fftw-libs-double-3.3.5-3.fc25.s390x
libssh2-1.8.0-1.fc25.s390x
ModemManager-glib-1.6.4-1.fc25.s390x
newt-python3-0.52.19-2.fc25.s390x
python-munch-2.0.4-3.fc25.noarch
python-bugzilla-1.2.2-4.fc25.noarch
libedit-3.1-16.20160618cvs.fc25.s390x
createrepo_c-0.10.0-6.fc25.s390x
device-mapper-multipath-libs-0.4.9-83.fc25.s390x
yum-3.4.3-510.fc25.noarch
mozjs17-17.0.0-16.fc25.s390x
libselinux-2.5-13.fc25.s390x
python2-pyparsing-2.1.10-1.fc25.noarch
cairo-gobject-1.14.8-1.fc25.s390x
xorg-x11-proto-devel-7.7-20.fc25.noarch
brlapi-0.6.5-2.fc25.s390x

[Qemu-devel] [PATCH 1/2] tpm_tis: Return TPM_VERSION_UNSPEC in case of BE failure

[Stefan Berger]

In case the backend has a failure, such as the tpm_emulator's CMD_INIT
failing, the TIS goes into failure mode and does not respond to reads
or writes to MMIO registers. In this case we need to prevent the ACPI
table from being added and the straight-forward way is to indicate that
there's no known TPM version being used.

Signed-off-by: Stefan Berger 
---
 hw/tpm/tpm_tis.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index c0a0204..eca3374 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -970,6 +970,10 @@ static enum TPMVersion tpm_tis_get_tpm_version(TPMIf *ti)
 {
 TPMState *s = TPM(ti);
 
+if (tpm_backend_had_startup_error(s->be_driver)) {
+return TPM_VERSION_UNSPEC;
+}
+
 return tpm_backend_get_tpm_version(s->be_driver);
 }
 
-- 
2.5.5

[Qemu-devel] [PATCH 2/2] tpm_tis: Return 0 for every register in case of failure mode

[Stefan Berger]

Rather than returning ~0, return 0 for every register in case of
failure mode. The '0' is better to indicate that there's no device
there.

Signed-off-by: Stefan Berger 
---
 hw/tpm/tpm_tis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index eca3374..d0bdd96 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -519,7 +519,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
 uint8_t v;
 
 if (tpm_backend_had_startup_error(s->be_driver)) {
-return val;
+return 0;
 }
 
 switch (offset) {
-- 
2.5.5

[Qemu-devel] [PATCH 0/2] tpm: Handle failure mode of backend device better

[Stefan Berger]

The following two patches fix the case that the backend device, e.g.,
tpm_emulator, could not be initialized and the TIS frontend needs to
go into failure mode.

   Stefan

Stefan Berger (2):
  tpm_tis: Return TPM_VERSION_UNSPEC in case of BE failure
  tpm_tis: Return 0 for every register in case of failure mode

 hw/tpm/tpm_tis.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

-- 
2.5.5

Re: [Qemu-devel] [Qemu-block] [PATCH] block: all I/O should be completed before removing throttle timers.

[l00284672]

ok，thanks ！


On 2017/11/10 23:33, Stefan Hajnoczi wrote:

On Sat, Oct 21, 2017 at 01:34:00PM +0800, Zhengui Li wrote:

From: Zhengui 

In blk_remove_bs, all I/O should be completed before removing throttle
timers. If there has inflight I/O, removing throttle timers here will
cause the inflight I/O never return.
This patch add bdrv_drained_begin before throttle_timers_detach_aio_context
to let all I/O completed before removing throttle timers.

Signed-off-by: Zhengui 
---
  block/block-backend.c | 4 
  1 file changed, 4 insertions(+)

Hi Zhengui,
Sorry it took so long to get thism merged!

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

I moved the declaration of bs as suggested by Berto when I merged the
patch.

Stefan


<>

Re: [Qemu-devel] [PATCH v2] linux-user: fix is_proc_myself to check the paths via realpath

[Zach Riggle]

I wrote up a quick example to show that this should work specifically for
/proc/self/exe:

#define _GNU_SOURCE
#include 
#include 
#include 
#include 
int main(int argc, char** argv) {
int fd = open("/proc/self/exe", O_NOFOLLOW | O_PATH);
system("ls -la /proc/$PPID/fd/");
}


*Zach Riggle*

On Fri, Nov 10, 2017 at 7:47 PM, Zach Riggle  wrote:

> Good catch.  Relying on realpath() for *exe* does cause issues.
>
> A better general solution (which handles the "exe" case) is to use open(2)
> with O_PATH | O_NOFOLLOW for the candidate path (e.g. /proc/self/exe) and
> to do the same for the path we're testing along with readlink().
>
> If, in the process of link resolution via the readlink() loop, we end up
> with the same path as our candidate, we can return true.  This avoids the
> need to rely on any libc implementation of realpath(), since we're just
> relying on the host kernel.
>
>
> *Zach Riggle*
>
> On Fri, Nov 10, 2017 at 5:44 PM, Laurent Vivier  wrote:
>
>> Le 25/10/2017 à 05:34, Zach Riggle a écrit :
>> > Previously, it was possible to get a handle to the "real" /proc/self/mem
>> > by creating a symlink to it and opening the symlink, or opening e.g.
>> > "./mem" after chdir'ing to "/proc/self".
>> >
>> > $ ln -s /proc/self self
>> > $ cat self/maps
>> > 6000-602bc000 r-xp  fc:01 270375
>>  /usr/bin/qemu-arm-static
>> > 604bc000-6050f000 rw-p 002bc000 fc:01 270375
>>  /usr/bin/qemu-arm-static
>> > ...
>> >
>> > Signed-off-by: Zach Riggle 
>> > ---
>> >  linux-user/syscall.c | 47 --
>> -
>> >  1 file changed, 28 insertions(+), 19 deletions(-)
>> >
>> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> > index 9bf901fa11..6c1f28a1f7 100644
>> > --- a/linux-user/syscall.c
>> > +++ b/linux-user/syscall.c
>> > @@ -7496,26 +7496,35 @@ static int open_self_auxv(void *cpu_env, int fd)
>> >
>> >  static int is_proc_myself(const char *filename, const char *entry)
>> >  {
>> > -if (!strncmp(filename, "/proc/", strlen("/proc/"))) {
>> > -filename += strlen("/proc/");
>> > -if (!strncmp(filename, "self/", strlen("self/"))) {
>> > -filename += strlen("self/");
>> > -} else if (*filename >= '1' && *filename <= '9') {
>> > -char myself[80];
>> > -snprintf(myself, sizeof(myself), "%d/", getpid());
>> > -if (!strncmp(filename, myself, strlen(myself))) {
>> > -filename += strlen(myself);
>> > -} else {
>> > -return 0;
>> > -}
>> > -} else {
>> > -return 0;
>> > -}
>> > -if (!strcmp(filename, entry)) {
>> > -return 1;
>> > -}
>> > +char proc_self_entry[PATH_MAX + 1];
>> > +char proc_self_entry_realpath[PATH_MAX + 1];
>> > +char filename_realpath[PATH_MAX + 1];
>> > +
>> > +if (PATH_MAX < snprintf(proc_self_entry,
>> > +sizeof(proc_self_entry),
>> > +"/proc/self/%s",
>> > +entry)) {
>> > +/* Full path to "entry" is too long to fit in the buffer */
>> > +return 0;
>> >  }
>> > -return 0;
>> > +
>> > +if (!realpath(filename, filename_realpath)) {
>> > +/* File does not exist, or can't be canonicalized */
>> > +return 0;
>> > +}
>> > +
>> > +if (!realpath(proc_self_entry, proc_self_entry_realpath)) {
>> > +/* Procfs entry does not exist */
>> > +return 0;
>> > +}
>> > +
>> > +if (strcmp(filename_realpath, proc_self_entry_realpath) != 0) {
>> > +/* Paths are different */
>>
>> I think it doesn't work with /proc/map/exe (or other soft link in
>> /proc/self) as realpath will give the path of the executable and not the
>> path inside /proc so it will be true for any process with the same
>> executable (which in our case is qemu for all).
>>
>> Thanks,
>> Laurent
>>
>>
>

Re: [Qemu-devel] [PATCH v2] linux-user: fix is_proc_myself to check the paths via realpath

[Zach Riggle]

Good catch.  Relying on realpath() for *exe* does cause issues.

A better general solution (which handles the "exe" case) is to use open(2)
with O_PATH | O_NOFOLLOW for the candidate path (e.g. /proc/self/exe) and
to do the same for the path we're testing along with readlink().

If, in the process of link resolution via the readlink() loop, we end up
with the same path as our candidate, we can return true.  This avoids the
need to rely on any libc implementation of realpath(), since we're just
relying on the host kernel.


*Zach Riggle*

On Fri, Nov 10, 2017 at 5:44 PM, Laurent Vivier  wrote:

> Le 25/10/2017 à 05:34, Zach Riggle a écrit :
> > Previously, it was possible to get a handle to the "real" /proc/self/mem
> > by creating a symlink to it and opening the symlink, or opening e.g.
> > "./mem" after chdir'ing to "/proc/self".
> >
> > $ ln -s /proc/self self
> > $ cat self/maps
> > 6000-602bc000 r-xp  fc:01 270375
>  /usr/bin/qemu-arm-static
> > 604bc000-6050f000 rw-p 002bc000 fc:01 270375
>  /usr/bin/qemu-arm-static
> > ...
> >
> > Signed-off-by: Zach Riggle 
> > ---
> >  linux-user/syscall.c | 47 --
> -
> >  1 file changed, 28 insertions(+), 19 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 9bf901fa11..6c1f28a1f7 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -7496,26 +7496,35 @@ static int open_self_auxv(void *cpu_env, int fd)
> >
> >  static int is_proc_myself(const char *filename, const char *entry)
> >  {
> > -if (!strncmp(filename, "/proc/", strlen("/proc/"))) {
> > -filename += strlen("/proc/");
> > -if (!strncmp(filename, "self/", strlen("self/"))) {
> > -filename += strlen("self/");
> > -} else if (*filename >= '1' && *filename <= '9') {
> > -char myself[80];
> > -snprintf(myself, sizeof(myself), "%d/", getpid());
> > -if (!strncmp(filename, myself, strlen(myself))) {
> > -filename += strlen(myself);
> > -} else {
> > -return 0;
> > -}
> > -} else {
> > -return 0;
> > -}
> > -if (!strcmp(filename, entry)) {
> > -return 1;
> > -}
> > +char proc_self_entry[PATH_MAX + 1];
> > +char proc_self_entry_realpath[PATH_MAX + 1];
> > +char filename_realpath[PATH_MAX + 1];
> > +
> > +if (PATH_MAX < snprintf(proc_self_entry,
> > +sizeof(proc_self_entry),
> > +"/proc/self/%s",
> > +entry)) {
> > +/* Full path to "entry" is too long to fit in the buffer */
> > +return 0;
> >  }
> > -return 0;
> > +
> > +if (!realpath(filename, filename_realpath)) {
> > +/* File does not exist, or can't be canonicalized */
> > +return 0;
> > +}
> > +
> > +if (!realpath(proc_self_entry, proc_self_entry_realpath)) {
> > +/* Procfs entry does not exist */
> > +return 0;
> > +}
> > +
> > +if (strcmp(filename_realpath, proc_self_entry_realpath) != 0) {
> > +/* Paths are different */
>
> I think it doesn't work with /proc/map/exe (or other soft link in
> /proc/self) as realpath will give the path of the executable and not the
> path inside /proc so it will be true for any process with the same
> executable (which in our case is qemu for all).
>
> Thanks,
> Laurent
>
>

Re: [Qemu-devel] Yet another git submodule rant

[Alexey Kardashevskiy]

On 11/11/17 01:22, Daniel P. Berrange wrote:
> On Sat, Nov 11, 2017 at 12:46:36AM +1100, Alexey Kardashevskiy wrote:
>> On 10/11/17 21:41, Daniel P. Berrange wrote:
>>> On Fri, Nov 10, 2017 at 09:35:54PM +1100, Alexey Kardashevskiy wrote:
 On 09/11/17 00:01, Daniel P. Berrange wrote:
> On Wed, Nov 08, 2017 at 09:26:01AM -0300, Philippe Mathieu-Daudé wrote:
>> On 11/08/2017 06:57 AM, Thomas Huth wrote:
>>>
>>> That automatic git submodule stuff now broke my workflow again. I
>>> usually keep the git repository on my laptop and then simply rsync the
>>> sources (without .git directories) to my target machine to compile it
>>> there. Used to work great for years. Now it's broken, the build process
>>> complains:
>>>
>>> GIT submodule checkout is out of date. Please run
>>>   scripts/git-submodule.sh update
>>> from the source directory checkout /home/thuth/devel/qemu
>>>
>>> Running "scripts/git-submodule.sh update" did not fix the issue at all -
>>> I first had to tinker with it for a while to find out that I simply have
>>> to delete ".git-submodule-status" in my git tree to fix the issue.
>>>
>>> I've got the feeling that all this submodule crap is constantly causing
>>> pain ... do we really need this? Can't we find another solution instead?
>>> Or at least stop modifying files automatically in the $SRC_PATH ?
>>
>> Also yesterday on IRC:
>>
>>  [...] I downloaded the qemu source from git and tried to compile
>> it. I am getting this:
>>
>> ./configure --static && make && sudo make install
>>  CC  ui/input-keymap.o
>> ui/input-keymap.c:8:10: fatal error: ui/input-keymap-linux-to-qcode.c:
>> No such file or directory
>
> I had a pull request merged yesterday later afternoon which possibly
> would address that problem, though hard hard to say for certain.

 wow, already? :(

 I still wonder why do not we checkout submodules into the build directory
 and why .git-submodule-status is not there too...
>>>
>>> That simply isn't the way submodules work, they are inherently part of
>>> the source tree, and the status file reflects that too.
>>
>> Sorry, I am missing the point here. What precisely does prevent us from
>> checking out the required modules to the build directory and build them
>> there? git provides a submodule repository url and sha1 for the current
>> qemu branch.
> 
> The build directory should never contain any of your version controlled
> source, as that will get irretrievably lost when the build dir is purged.

I am not suggesting editing submodule sources from the build directory, I
am suggesting not building them from the source directory, in order to keep
them in sync, Makefile can rsync or "git push" them to the build directory.
Yeah, fairly ugly but still more correct than the current solution.

And again, .git-submodule-status is not a source file, what is it doing in
$SRC_PATH?

Now I cannot compile qemu pretty much 50% of time because of that
enhancement. In the today's episode:

ssh aikhostos2 cd /home/aik/pbuild/qemu-aikhostos2-ppc64/ ;
/home/aik/p/qemu/configure --target-list=ppc64-softmmu
--source-path=/home/aik/p/qemu/ --enable-debug --enable-debug-info
--disable-werror --disable-git-update --enable-trace-backend=log

All good. One detail:
capstone  git

ssh aikhostos2 make -C /home/aik/pbuild/qemu-aikhostos2-ppc64/ -j24

make: Entering directory `/home/aik/pbuild/qemu-aikhostos2-ppc64'
  GEN ppc64-softmmu/config-devices.mak.tmp
mkdir -p dtc/libfdt
mkdir -p dtc/tests
  GEN config-host.h
  GEN qemu-options.def
make[1]: *** No rule to make target
`/home/aik/pbuild/qemu-aikhostos2-ppc64/capstone/libcapstone.a'.  Stop.
make: *** [subdir-capstone] Error 2
make: *** Waiting for unfinished jobs
  GEN ppc64-softmmu/config-devices.mak


How, why, where are all these nice warning messages? Ah, there they are
(added 'set -x' to the script):

+ substat=.git-submodule-status
+ command=status
+ shift
+ maybe_modules='ui/keycodemapdb dtc capstone'
+ test -z git
+ modules=
+ for m in '$maybe_modules'
+ git submodule status ui/keycodemapdb
+ test 128 = 0
+ echo 'warn: ignoring non-existent submodule ui/keycodemapdb'
+ for m in '$maybe_modules'
+ git submodule status dtc
+ test 128 = 0:m
+ echo 'warn: ignoring non-existent submodule dtc'
+ for m in '$maybe_modules'
+ git submodule status capstone
+ test 128 = 0
+ echo 'warn: ignoring non-existent submodule capstone'
+ test -n 'ui/keycodemapdb dtc capstone'
+ test -e .git
+ case "$command" in
+ test -z 'ui/keycodemapdb dtc capstone'
+ test -f .git-submodule-status
+ exit 1


I assume that 'warn' is not printed because of Makefile's 'quiet-command'.

And this fails because on the server the source directory is created with
'git worktree' which points to bare git repo outside of the source tree and
the git on that build machine has no idea about worktrees (the repo 

[Qemu-devel] [Bug 1731588] [NEW] qemu-system-arm black screen and keyboard not detected

[Kevin]

Public bug reported:

Hi guys,

I try to emulate FreeRTOS with this guide : 
http://wiki.csie.ncku.edu.tw/embedded/Lab32
But, the keys on my keyboard are not taken into account. 
 - Command line : qemu_stm32/arm-softmmu/qemu-system-arm -M stm32-p103 -monitor 
stdio -kernel build/main.bin -semihosting
If I try to add usb flag with : qemu_stm32/arm-softmmu/qemu-system-arm -M 
stm32-p103 -monitor stdio -kernel build/main.bin -usb -device 
usb-host,hostbus=1,hostaddr=1 -show-curso
I obtain this message "qemu-system-arm: -device usb-host,hostbus=1,hostaddr=1: 
'usb-host' is not a valid device model name"

My second option is try with the latest version of qemu with this
command line : "qemu-system-arm -M lm3s6965evb -monitor stdio -kernel
build/main.bin -semihosting" but I obtain a black screen.

Could someone guide me on this problem ?

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: blackscreen cortex-m3 freertos keyboard keys qemu qemu-system-arm

** Tags added: blackscreen cortex-m3 freertos keyboard keys qemu qemu-
system-arm

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

Title:
  qemu-system-arm black screen and keyboard not detected

Status in QEMU:
  New

Bug description:
  Hi guys,

  I try to emulate FreeRTOS with this guide : 
http://wiki.csie.ncku.edu.tw/embedded/Lab32
  But, the keys on my keyboard are not taken into account. 
   - Command line : qemu_stm32/arm-softmmu/qemu-system-arm -M stm32-p103 
-monitor stdio -kernel build/main.bin -semihosting
  If I try to add usb flag with : qemu_stm32/arm-softmmu/qemu-system-arm -M 
stm32-p103 -monitor stdio -kernel build/main.bin -usb -device 
usb-host,hostbus=1,hostaddr=1 -show-curso
  I obtain this message "qemu-system-arm: -device 
usb-host,hostbus=1,hostaddr=1: 'usb-host' is not a valid device model name"

  My second option is try with the latest version of qemu with this
  command line : "qemu-system-arm -M lm3s6965evb -monitor stdio -kernel
  build/main.bin -semihosting" but I obtain a black screen.

  Could someone guide me on this problem ?

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

Re: [Qemu-devel] Yet another git submodule rant

[Alexey Kardashevskiy]

On 11/11/17 01:01, Peter Maydell wrote:
> On 10 November 2017 at 13:46, Alexey Kardashevskiy  wrote:
>> And it will still be
>> better than changing the $SRC_PATH when a user specifically asked not to do
>> that by calling "./configure --source-path='.
> 
> I'm not terribly happy with the submodule stuff either, but
> that configure rune is not making use of a documented
> or supported feature. --source-path is for telling configure
> where the source code is. Passing it an empty string should
> probably be rejected as an error.


It is not empty, I just cut an example too much :) The script looks like:

cd /home/aik/pbuild/qemu-garrison2-ppc64/
/home/aik/p/qemu/configure --target-list=ppc64-softmmu
--source-path=/home/aik/p/qemu/



-- 
Alexey

Re: [Qemu-devel] [PATCH v2 for-2.11] block: Make bdrv_next() keep strong references

[Paolo Bonzini]

On 10/11/2017 18:25, Max Reitz wrote:
>  if (bs) {
> +bdrv_ref(bs);
> +bdrv_unref(old_bs);
>  return bs;
>  }

Maybe instead goto...

>  it->phase = BDRV_NEXT_MONITOR_OWNED;
> +} else {
> +old_bs = it->bs;
>  }
>  
>  /* Then return the monitor-owned BDSes without a BB attached. Ignore all
> @@ -467,18 +483,46 @@ BlockDriverState *bdrv_next(BdrvNextIterator *it)
>  bs = it->bs;
>  } while (bs && bdrv_has_blk(bs));

... here?

Paolo

> +if (bs) {
> +bdrv_ref(bs);
> +}
> +bdrv_unref(old_bs);
> +
>  return bs;

[Qemu-devel] UT Austin Virtualization project

[william lin]

Hi all,

My name is William Lin. I am an undergrad student at UT Austin and for our
virtualization class we have to contribute to a open source repo related to
virtualization. I am working in a group of two with about a month of time.
We are both comfortable with virtualization concepts and systems
programming. If there are any ideas or suggestions please let me know it
will be much appreciated. We are looking for any features/issues that would
take a team of two about a month to complete.

Getting the code accepted is not a requirement for our project but is extra
credit and would be preferred.

Thanks,
Will

Re: [Qemu-devel] [PATCH v2] linux-user: fix is_proc_myself to check the paths via realpath

[Laurent Vivier]

Le 25/10/2017 à 05:34, Zach Riggle a écrit :
> Previously, it was possible to get a handle to the "real" /proc/self/mem
> by creating a symlink to it and opening the symlink, or opening e.g.
> "./mem" after chdir'ing to "/proc/self".
> 
> $ ln -s /proc/self self
> $ cat self/maps
> 6000-602bc000 r-xp  fc:01 270375 
> /usr/bin/qemu-arm-static
> 604bc000-6050f000 rw-p 002bc000 fc:01 270375 
> /usr/bin/qemu-arm-static
> ...
> 
> Signed-off-by: Zach Riggle 
> ---
>  linux-user/syscall.c | 47 ---
>  1 file changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9bf901fa11..6c1f28a1f7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -7496,26 +7496,35 @@ static int open_self_auxv(void *cpu_env, int fd)
> 
>  static int is_proc_myself(const char *filename, const char *entry)
>  {
> -if (!strncmp(filename, "/proc/", strlen("/proc/"))) {
> -filename += strlen("/proc/");
> -if (!strncmp(filename, "self/", strlen("self/"))) {
> -filename += strlen("self/");
> -} else if (*filename >= '1' && *filename <= '9') {
> -char myself[80];
> -snprintf(myself, sizeof(myself), "%d/", getpid());
> -if (!strncmp(filename, myself, strlen(myself))) {
> -filename += strlen(myself);
> -} else {
> -return 0;
> -}
> -} else {
> -return 0;
> -}
> -if (!strcmp(filename, entry)) {
> -return 1;
> -}
> +char proc_self_entry[PATH_MAX + 1];
> +char proc_self_entry_realpath[PATH_MAX + 1];
> +char filename_realpath[PATH_MAX + 1];
> +
> +if (PATH_MAX < snprintf(proc_self_entry,
> +sizeof(proc_self_entry),
> +"/proc/self/%s",
> +entry)) {
> +/* Full path to "entry" is too long to fit in the buffer */
> +return 0;
>  }
> -return 0;
> +
> +if (!realpath(filename, filename_realpath)) {
> +/* File does not exist, or can't be canonicalized */
> +return 0;
> +}
> +
> +if (!realpath(proc_self_entry, proc_self_entry_realpath)) {
> +/* Procfs entry does not exist */
> +return 0;
> +}
> +
> +if (strcmp(filename_realpath, proc_self_entry_realpath) != 0) {
> +/* Paths are different */

I think it doesn't work with /proc/map/exe (or other soft link in
/proc/self) as realpath will give the path of the executable and not the
path inside /proc so it will be true for any process with the same
executable (which in our case is qemu for all).

Thanks,
Laurent

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Robin H. Johnson]

TL;DR: pass '-vga none' with -nographic, or redirect the screen
somewhere!

I ended up digging into this after it was mentioned by smoser. The bug
is invalid because of a bad assumption in the QEMU inputs. smoser's
workaround of usb=off removes USB as a workaround.

The kernel, OpenFirmware, and QEMU are behaving correctly, but the
original command-line hides the screen output.

If you have both a supported* keyboard AND screen, then OpenFirmware/SLOF sets 
OF stdout to be the screen.
This causes the OF output of 'No console specified using screen & keyboard'

If you are missing either a keyboard OR screen, then OF sets stdout to be 
hvterm (serial).
This causes the OF output of 'No console specified using hvterm'

https://git.qemu.org/?p=SLOF.git;a=blob;f=board-
qemu/slof/OF.fs;h=4e04b840d5e6ff6c39191939e1a4f70f3d169d5d;hb=HEAD#l215

Which devices are available depends on the QEMU commandline...
Base QEMU defaults to '-vga std'.
QEMU '-machine pseries' defaults to enabling USB.


Re supported keyboards: The OpenFirmware/SLOF only seems to support USB 
keyboards at this time (virtio keyboard not supported).

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Invalid
Status in linux package in Ubuntu:
  Invalid
Status in livecd-rootfs package in Ubuntu:
  Invalid
Status in qemu package in Ubuntu:
  Invalid

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  

Re: [Qemu-devel] [PATCH for 2.11 5/5] hw: add .min_cpus and .default_cpus fields to machine_class

[Alistair Francis]

On Fri, Nov 10, 2017 at 11:53 AM, Emilio G. Cota  wrote:
> max_cpus needs to be an upper bound on the number of vCPUs
> initialized; otherwise TCG region initialization breaks.
>
> Some boards initialize a hard-coded number of vCPUs, which is not
> captured by the global max_cpus and therefore breaks TCG initialization.
> Fix it by adding the .min_cpus field to machine_class.
>
> This commit also changes some user-facing behaviour: we now die if
> -smp is below this hard-coded vCPU minimum instead of silently
> ignoring the passed -smp value (sometimes announcing this by printing
> a warning). However, the introduction of .default_cpus lessens the
> likelihood that users will notice this: if -smp isn't set, we now
> assign the value in .default_cpus to both smp_cpus and max_cpus. IOW,
> if a user does not set -smp, they always get a correct number of vCPUs.
>
> This change fixes 3468b59 ("tcg: enable multiple TCG contexts in
> softmmu", 2017-10-24), which broke TCG initialization for some
> ARM boards.
>
> Fixes: 3468b59e18b179bc63c7ce934de912dfa9596122
> Reported-by: Thomas Huth 
> Suggested-by: Peter Maydell 
> Signed-off-by: Emilio G. Cota 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  include/hw/boards.h |  5 +
>  hw/arm/exynos4_boards.c | 12 
>  hw/arm/raspi.c  |  2 ++
>  hw/arm/xlnx-zcu102.c|  2 ++
>  vl.c| 21 ++---
>  5 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 191a5b3..62f160e 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -102,6 +102,9 @@ typedef struct {
>
>  /**
>   * MachineClass:
> + * @max_cpus: maximum number of CPUs supported. Default: 1
> + * @min_cpus: minimum number of CPUs supported. Default: 1
> + * @default_cpus: number of CPUs instantiated if none are specified. 
> Default: 1
>   * @get_hotplug_handler: this function is called during bus-less
>   *device hotplug. If defined it returns pointer to an instance
>   *of HotplugHandler object, which handles hotplug operation
> @@ -167,6 +170,8 @@ struct MachineClass {
>  BlockInterfaceType block_default_type;
>  int units_per_default_bus;
>  int max_cpus;
> +int min_cpus;
> +int default_cpus;
>  unsigned int no_serial:1,
>  no_parallel:1,
>  use_virtcon:1,
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index f1441ec..750162c 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -27,7 +27,6 @@
>  #include "qemu-common.h"
>  #include "cpu.h"
>  #include "sysemu/sysemu.h"
> -#include "sysemu/qtest.h"
>  #include "hw/sysbus.h"
>  #include "net/net.h"
>  #include "hw/arm/arm.h"
> @@ -129,13 +128,6 @@ exynos4_boards_init_common(MachineState *machine,
> Exynos4BoardType board_type)
>  {
>  Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
> -MachineClass *mc = MACHINE_GET_CLASS(machine);
> -
> -if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
> -error_report("%s board supports only %d CPU cores, ignoring smp_cpus"
> - " value",
> - mc->name, EXYNOS4210_NCPUS);
> -}
>
>  exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type];
>  exynos4_board_binfo.board_id = exynos4_board_id[board_type];
> @@ -189,6 +181,8 @@ static void nuri_class_init(ObjectClass *oc, void *data)
>  mc->desc = "Samsung NURI board (Exynos4210)";
>  mc->init = nuri_init;
>  mc->max_cpus = EXYNOS4210_NCPUS;
> +mc->min_cpus = EXYNOS4210_NCPUS;
> +mc->default_cpus = EXYNOS4210_NCPUS;
>  mc->ignore_memory_transaction_failures = true;
>  }
>
> @@ -205,6 +199,8 @@ static void smdkc210_class_init(ObjectClass *oc, void 
> *data)
>  mc->desc = "Samsung SMDKC210 board (Exynos4210)";
>  mc->init = smdkc210_init;
>  mc->max_cpus = EXYNOS4210_NCPUS;
> +mc->min_cpus = EXYNOS4210_NCPUS;
> +mc->default_cpus = EXYNOS4210_NCPUS;
>  mc->ignore_memory_transaction_failures = true;
>  }
>
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 5941c9f..cd5fa8c 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -167,6 +167,8 @@ static void raspi2_machine_init(MachineClass *mc)
>  mc->no_floppy = 1;
>  mc->no_cdrom = 1;
>  mc->max_cpus = BCM2836_NCPUS;
> +mc->min_cpus = BCM2836_NCPUS;
> +mc->default_cpus = BCM2836_NCPUS;
>  mc->default_ram_size = 1024 * 1024 * 1024;
>  mc->ignore_memory_transaction_failures = true;
>  };
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index 190eb69..9631a53 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -189,6 +189,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass 
> *oc, void *data)
>  mc->units_per_default_bus = 1;
>  mc->ignore_memory_transaction_failures = 

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/1] Add 8-byte wide AMD flash support, partial interleaving

[Paolo Bonzini]

On 10/11/2017 21:25, Mike Nawrocki wrote:
> This patch set does a few things. First, it switches the AMD CFI flash MMIO
> operations from the old MMIO API to the new one. Second, it enables 8-byte 
> wide
> flash arrays. Finally, it adds flash interleaving using the "device-width" and
> "max-device-width" properties, using the same interface as pflash_cfi01.c. 
> Much
> of the code was taken and adapted from that file.

This unfortunately is not a patch set, it's a patch that does many
things.  You should split it in three parts, according to the three
things you've mentioned in the message above.

Paolo

Re: [Qemu-devel] [PATCH for 2.11 1/5] qom: move CPUClass.tcg_initialize to a global

[Alistair Francis]

On Fri, Nov 10, 2017 at 12:23 PM, Eduardo Habkost  wrote:
> On Fri, Nov 10, 2017 at 02:53:42PM -0500, Emilio G. Cota wrote:
>> 55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
>> introduces a per-CPUClass bool that we check so that the target CPU
>> is initialized for TCG only once. This works well except when
>> we end up creating more than one CPUClass, in which case we end
>> up incorrectly initializing TCG more than once, i.e. once for
>> each CPUClass.
>>
>> This can be replicated with:
>>   $ aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -smp 6 \
>>   -global driver=xlnx,,zynqmp,property=has_rpu,value=on
>> In this case the class name of the "RPUs" is prefixed by "cortex-r5-",
>> whereas the "regular" CPUs are prefixed by "cortex-a53-". This
>> results in two CPUClass instances being created.
>>
>> Fix it by introducing a static variable, so that only the first
>> target CPU being initialized will initialize the target-dependent
>> part of TCG, regardless of CPUClass instances.
>>
>> Fixes: 55c3ceef61fcf06fc98ddc752b7cce788ce7680b
>> Signed-off-by: Emilio G. Cota 
>
> Reviewed-by: Eduardo Habkost 

Reviewed-by: Alistair Francis 
Tested-by: Alistair Francis 

Alistair

>
> --
> Eduardo
>

Re: [Qemu-devel] [PATCH for 2.11 4/5] xlnx-zcu102: Specify the max number of CPUs for the EP108

[Alistair Francis]

On Fri, Nov 10, 2017 at 11:53 AM, Emilio G. Cota  wrote:
> Just like the zcu102, the ep108 can instantiate several CPUs.
>
> Signed-off-by: Emilio G. Cota 

I completely missed this, thanks for the patch.

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/arm/xlnx-zcu102.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index a23..190eb69 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -188,6 +188,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass 
> *oc, void *data)
>  mc->block_default_type = IF_IDE;
>  mc->units_per_default_bus = 1;
>  mc->ignore_memory_transaction_failures = true;
> +mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
>  }
>
>  static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
> --
> 2.7.4
>
>

Re: [Qemu-devel] [PATCH v8 02/14] block/dirty-bitmap: add locked version of bdrv_release_dirty_bitmap

[John Snow]

On 10/30/2017 12:32 PM, Vladimir Sementsov-Ogievskiy wrote:
> It is needed to realize bdrv_dirty_bitmap_release_successor in
> the following patch.
> 

OK, but...

> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/dirty-bitmap.c | 25 -
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 81adbeb6d4..981f99d362 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -326,13 +326,13 @@ static bool bdrv_dirty_bitmap_has_name(BdrvDirtyBitmap 
> *bitmap)
>  return !!bdrv_dirty_bitmap_name(bitmap);
>  }
>  
> -/* Called with BQL taken.  */
> -static void bdrv_do_release_matching_dirty_bitmap(
> +/* Called within bdrv_dirty_bitmap_lock..unlock */

...Add this so it will compile:

__attribute__((__unused__))
> +static void bdrv_do_release_matching_dirty_bitmap_locked(
>  BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
>  bool (*cond)(BdrvDirtyBitmap *bitmap))
>  {
>  BdrvDirtyBitmap *bm, *next;
> -bdrv_dirty_bitmaps_lock(bs);
> +
>  QLIST_FOREACH_SAFE(bm, >dirty_bitmaps, list, next) {
>  if ((!bitmap || bm == bitmap) && (!cond || cond(bm))) {
>  assert(!bm->active_iterators);
> @@ -344,18 +344,33 @@ static void bdrv_do_release_matching_dirty_bitmap(
>  g_free(bm);
>  
>  if (bitmap) {
> -goto out;
> +return;
>  }
>  }
>  }
> +
>  if (bitmap) {
>  abort();
>  }

Do we have any style guide rules on using abort() instead of assert()?
The rest of this function uses assert, and it'd be less lines to simply
write:

assert(!bitmap);

which I think might also carry better semantic information for coverity
beyond an actual runtime conditional branch.

(I think. Please correct me if I am wrong, I'm a little hazy on this.)

> +}
>  
> -out:
> +/* Called with BQL taken.  */
> +static void bdrv_do_release_matching_dirty_bitmap(
> +BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
> +bool (*cond)(BdrvDirtyBitmap *bitmap))
> +{
> +bdrv_dirty_bitmaps_lock(bs);
> +bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, cond);
>  bdrv_dirty_bitmaps_unlock(bs);
>  }
>  
> +/* Called within bdrv_dirty_bitmap_lock..unlock */
> +static void bdrv_release_dirty_bitmap_locked(BlockDriverState *bs,
> + BdrvDirtyBitmap *bitmap)
> +{
> +bdrv_do_release_matching_dirty_bitmap_locked(bs, bitmap, NULL);
> +}
> +
>  /* Called with BQL taken.  */
>  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
>  {
> 

If you agree with those two changes, you may add:

Reviewed-by: John Snow 

[Qemu-devel] [PATCH for-2.12 4/4] blockdev: Mark BD-{remove, insert}-medium stable

[Max Reitz]

Now that iotest 093 test proves that the throttling configuration
survives a blockdev-remove-medium/blockdev-insert-medium pair, the
original reason for declaring these commands experimental is gone
(see commit 6e0abc251dd4f8eba1f53656dfede12e5840e83b).

Signed-off-by: Max Reitz 
---
 qapi/block-core.json   | 32 +---
 blockdev.c |  6 +++---
 tests/ahci-test.c  |  4 ++--
 tests/qemu-iotests/093 |  6 +++---
 tests/qemu-iotests/118 | 20 ++--
 tests/qemu-iotests/139 |  2 +-
 6 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index cf3f941999..8e6842e23e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3397,7 +3397,7 @@
 '*id': 'str' } }
 
 ##
-# @x-blockdev-remove-medium:
+# @blockdev-remove-medium:
 #
 # Removes a medium (a block driver state tree) from a block device. That block
 # device's tray must currently be open (unless there is no attached guest
@@ -3405,16 +3405,13 @@
 #
 # If the tray is open and there is no medium inserted, this will be a no-op.
 #
-# @id: The name or QOM path of the guest device (since: 2.8)
-#
-# Note: This command is still a work in progress and is considered 
experimental.
-# Stay away from it unless you want to help with its development.
+# @id: The name or QOM path of the guest device
 #
-# Since: 2.5
+# Since: 2.12
 #
 # Example:
 #
-# -> { "execute": "x-blockdev-remove-medium",
+# -> { "execute": "blockdev-remove-medium",
 #  "arguments": { "id": "ide0-1-0" } }
 #
 # <- { "error": { "class": "GenericError",
@@ -3432,30 +3429,27 @@
 #
 # <- { "return": {} }
 #
-# -> { "execute": "x-blockdev-remove-medium",
+# -> { "execute": "blockdev-remove-medium",
 #  "arguments": { "id": "ide0-1-0" } }
 #
 # <- { "return": {} }
 #
 ##
-{ 'command': 'x-blockdev-remove-medium',
+{ 'command': 'blockdev-remove-medium',
   'data': { 'id': 'str' } }
 
 ##
-# @x-blockdev-insert-medium:
+# @blockdev-insert-medium:
 #
 # Inserts a medium (a block driver state tree) into a block device. That block
 # device's tray must currently be open (unless there is no attached guest
 # device) and there must be no medium inserted already.
 #
-# @id:The name or QOM path of the guest device (since: 2.8)
+# @id:The name or QOM path of the guest device
 #
 # @node-name: name of a node in the block driver state graph
 #
-# Note: This command is still a work in progress and is considered 
experimental.
-# Stay away from it unless you want to help with its development.
-#
-# Since: 2.5
+# Since: 2.12
 #
 # Example:
 #
@@ -3467,14 +3461,14 @@
 #"filename": "fedora.iso" } } }
 # <- { "return": {} }
 #
-# -> { "execute": "x-blockdev-insert-medium",
+# -> { "execute": "blockdev-insert-medium",
 #  "arguments": { "id": "ide0-1-0",
 # "node-name": "node0" } }
 #
 # <- { "return": {} }
 #
 ##
-{ 'command': 'x-blockdev-insert-medium',
+{ 'command': 'blockdev-insert-medium',
   'data': { 'id': 'str',
 'node-name': 'str'} }
 
@@ -3503,8 +3497,8 @@
 #
 # Changes the medium inserted into a block device by ejecting the current 
medium
 # and loading a new image file which is inserted as the new medium (this 
command
-# combines blockdev-open-tray, x-blockdev-remove-medium,
-# x-blockdev-insert-medium and blockdev-close-tray).
+# combines blockdev-open-tray, blockdev-remove-medium, blockdev-insert-medium
+# and blockdev-close-tray).
 #
 # @device:  Block device name (deprecated, use @id instead)
 #
diff --git a/blockdev.c b/blockdev.c
index a74224e8f8..fb93283334 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2441,7 +2441,7 @@ out:
 aio_context_release(aio_context);
 }
 
-void qmp_x_blockdev_remove_medium(const char *id, Error **errp)
+void qmp_blockdev_remove_medium(const char *id, Error **errp)
 {
 blockdev_remove_medium(false, NULL, true, id, errp);
 }
@@ -2519,8 +2519,8 @@ static void blockdev_insert_medium(bool has_device, const 
char *device,
 qmp_blockdev_insert_anon_medium(blk, bs, errp);
 }
 
-void qmp_x_blockdev_insert_medium(const char *id, const char *node_name,
-  Error **errp)
+void qmp_blockdev_insert_medium(const char *id, const char *node_name,
+Error **errp)
 {
 blockdev_insert_medium(false, NULL, true, id, node_name, errp);
 }
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 3635ea6d41..3934e62ef7 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1596,7 +1596,7 @@ static void test_atapi_tray(void)
 rsp = qmp_receive();
 QDECREF(rsp);
 
-qmp_discard_response("{'execute': 'x-blockdev-remove-medium', "
+qmp_discard_response("{'execute': 'blockdev-remove-medium', "
  "'arguments': {'id': 'cd0'}}");
 
 /* Test the tray without a medium */
@@ -1612,7 +1612,7 @@ static void test_atapi_tray(void)
  

[Qemu-devel] [PATCH for-2.12 1/4] iotests: Make BD-{remove, insert}-medium use @id

[Max Reitz]

In some cases, these commands still use the deprecated @device
parameter.  Fix that so we can later drop that parameter from their
interface.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/118 | 184 +++--
 tests/qemu-iotests/155 |  60 
 2 files changed, 113 insertions(+), 131 deletions(-)

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index 8a9e838c90..ca6965d23c 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -28,6 +28,14 @@ from iotests import qemu_img
 old_img = os.path.join(iotests.test_dir, 'test0.img')
 new_img = os.path.join(iotests.test_dir, 'test1.img')
 
+def interface_to_device_name(interface):
+if interface == 'ide':
+return 'ide-cd'
+elif interface == 'floppy':
+return 'floppy'
+else:
+return None
+
 class ChangeBaseClass(iotests.QMPTestCase):
 has_opened = False
 has_closed = False
@@ -63,7 +71,7 @@ class ChangeBaseClass(iotests.QMPTestCase):
 
 class GeneralChangeTestsBaseClass(ChangeBaseClass):
 
-device_name = None
+device_name = 'qdev0'
 
 def test_change(self):
 result = self.vm.qmp('change', device='drive0', target=new_img,
@@ -79,14 +87,9 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
 self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
 
 def test_blockdev_change_medium(self):
-if self.device_name is not None:
-result = self.vm.qmp('blockdev-change-medium',
- id=self.device_name, filename=new_img,
- format=iotests.imgfmt)
-else:
-result = self.vm.qmp('blockdev-change-medium',
- device='drive0', filename=new_img,
- format=iotests.imgfmt)
+result = self.vm.qmp('blockdev-change-medium',
+ id=self.device_name, filename=new_img,
+ format=iotests.imgfmt)
 
 self.assert_qmp(result, 'return', {})
 
@@ -99,10 +102,7 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
 self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
 
 def test_eject(self):
-if self.device_name is not None:
-result = self.vm.qmp('eject', id=self.device_name, force=True)
-else:
-result = self.vm.qmp('eject', device='drive0', force=True)
+result = self.vm.qmp('eject', id=self.device_name, force=True)
 self.assert_qmp(result, 'return', {})
 
 self.wait_for_open()
@@ -113,10 +113,7 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
 self.assert_qmp_absent(result, 'return[0]/inserted')
 
 def test_tray_eject_change(self):
-if self.device_name is not None:
-result = self.vm.qmp('eject', id=self.device_name, force=True)
-else:
-result = self.vm.qmp('eject', device='drive0', force=True)
+result = self.vm.qmp('eject', id=self.device_name, force=True)
 self.assert_qmp(result, 'return', {})
 
 self.wait_for_open()
@@ -126,12 +123,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
 self.assert_qmp(result, 'return[0]/tray_open', True)
 self.assert_qmp_absent(result, 'return[0]/inserted')
 
-if self.device_name is not None:
-result = self.vm.qmp('blockdev-change-medium', id=self.device_name,
- filename=new_img, format=iotests.imgfmt)
-else:
-result = self.vm.qmp('blockdev-change-medium', device='drive0',
- filename=new_img, format=iotests.imgfmt)
+result = self.vm.qmp('blockdev-change-medium', id=self.device_name,
+ filename=new_img, format=iotests.imgfmt)
 self.assert_qmp(result, 'return', {})
 
 self.wait_for_close()
@@ -142,12 +135,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
 self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
 
 def test_tray_open_close(self):
-if self.device_name is not None:
-result = self.vm.qmp('blockdev-open-tray',
- id=self.device_name, force=True)
-else:
-result = self.vm.qmp('blockdev-open-tray',
- device='drive0', force=True)
+result = self.vm.qmp('blockdev-open-tray',
+ id=self.device_name, force=True)
 self.assert_qmp(result, 'return', {})
 
 self.wait_for_open()
@@ -160,10 +149,7 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
 else:
 self.assert_qmp(result, 'return[0]/inserted/image/filename', 
old_img)
 
-if self.device_name is not None:
-result = self.vm.qmp('blockdev-close-tray', id=self.device_name)
-else:
-result = 

[Qemu-devel] [PATCH for-2.12 3/4] blockdev: Drop BD-{remove, insert}-medium's @device

[Max Reitz]

This is an incompatible change, which is fine as the commands are
experimental.

Signed-off-by: Max Reitz 
---
 qapi/block-core.json | 10 ++
 blockdev.c   | 30 +++---
 2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 76bf50f813..cf3f941999 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3405,8 +3405,6 @@
 #
 # If the tray is open and there is no medium inserted, this will be a no-op.
 #
-# @device: Block device name (deprecated, use @id instead)
-#
 # @id: The name or QOM path of the guest device (since: 2.8)
 #
 # Note: This command is still a work in progress and is considered 
experimental.
@@ -3441,8 +3439,7 @@
 #
 ##
 { 'command': 'x-blockdev-remove-medium',
-  'data': { '*device': 'str',
-'*id': 'str' } }
+  'data': { 'id': 'str' } }
 
 ##
 # @x-blockdev-insert-medium:
@@ -3451,8 +3448,6 @@
 # device's tray must currently be open (unless there is no attached guest
 # device) and there must be no medium inserted already.
 #
-# @device:Block device name (deprecated, use @id instead)
-#
 # @id:The name or QOM path of the guest device (since: 2.8)
 #
 # @node-name: name of a node in the block driver state graph
@@ -3480,8 +3475,7 @@
 #
 ##
 { 'command': 'x-blockdev-insert-medium',
-  'data': { '*device': 'str',
-'*id': 'str',
+  'data': { 'id': 'str',
 'node-name': 'str'} }
 
 
diff --git a/blockdev.c b/blockdev.c
index 56a6b24a0b..a74224e8f8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -59,6 +59,11 @@ static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
 
 static int do_open_tray(const char *blk_name, const char *qdev_id,
 bool force, Error **errp);
+static void blockdev_remove_medium(bool has_device, const char *device,
+   bool has_id, const char *id, Error **errp);
+static void blockdev_insert_medium(bool has_device, const char *device,
+   bool has_id, const char *id,
+   const char *node_name, Error **errp);
 
 static const char *const if_name[IF_COUNT] = {
 [IF_NONE] = "none",
@@ -2256,7 +2261,7 @@ void qmp_eject(bool has_device, const char *device,
 }
 error_free(local_err);
 
-qmp_x_blockdev_remove_medium(has_device, device, has_id, id, errp);
+blockdev_remove_medium(has_device, device, has_id, id, errp);
 }
 
 void qmp_block_passwd(bool has_device, const char *device,
@@ -2379,8 +2384,8 @@ void qmp_blockdev_close_tray(bool has_device, const char 
*device,
 }
 }
 
-void qmp_x_blockdev_remove_medium(bool has_device, const char *device,
-  bool has_id, const char *id, Error **errp)
+static void blockdev_remove_medium(bool has_device, const char *device,
+   bool has_id, const char *id, Error **errp)
 {
 BlockBackend *blk;
 BlockDriverState *bs;
@@ -2436,6 +2441,11 @@ out:
 aio_context_release(aio_context);
 }
 
+void qmp_x_blockdev_remove_medium(const char *id, Error **errp)
+{
+blockdev_remove_medium(false, NULL, true, id, errp);
+}
+
 static void qmp_blockdev_insert_anon_medium(BlockBackend *blk,
 BlockDriverState *bs, Error **errp)
 {
@@ -2481,9 +2491,9 @@ static void qmp_blockdev_insert_anon_medium(BlockBackend 
*blk,
 }
 }
 
-void qmp_x_blockdev_insert_medium(bool has_device, const char *device,
-  bool has_id, const char *id,
-  const char *node_name, Error **errp)
+static void blockdev_insert_medium(bool has_device, const char *device,
+   bool has_id, const char *id,
+   const char *node_name, Error **errp)
 {
 BlockBackend *blk;
 BlockDriverState *bs;
@@ -2509,6 +2519,12 @@ void qmp_x_blockdev_insert_medium(bool has_device, const 
char *device,
 qmp_blockdev_insert_anon_medium(blk, bs, errp);
 }
 
+void qmp_x_blockdev_insert_medium(const char *id, const char *node_name,
+  Error **errp)
+{
+blockdev_insert_medium(false, NULL, true, id, node_name, errp);
+}
+
 void qmp_blockdev_change_medium(bool has_device, const char *device,
 bool has_id, const char *id,
 const char *filename,
@@ -2583,7 +2599,7 @@ void qmp_blockdev_change_medium(bool has_device, const 
char *device,
 error_free(err);
 err = NULL;
 
-qmp_x_blockdev_remove_medium(has_device, device, has_id, id, );
+blockdev_remove_medium(has_device, device, has_id, id, );
 if (err) {
 error_propagate(errp, err);
 goto fail;
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 2/4] tests/ahci: Switch tray and medium commands to @id

[Max Reitz]

Currently, the tray and medium commands in the AHCI test use the
deprecated @device parameter.  This patch switches all invocations over
to use @id.

Signed-off-by: Max Reitz 
---
 tests/ahci-test.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index cb84edc8fb..3635ea6d41 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1578,9 +1578,9 @@ static void test_atapi_tray(void)
 QDict *rsp;
 
 fd = prepare_iso(iso_size, , );
-ahci = ahci_boot_and_enable("-drive if=none,id=drive0,file=%s,format=raw "
+ahci = ahci_boot_and_enable("-blockdev 
node-name=drive0,driver=file,filename=%s "
 "-M q35 "
-"-device ide-cd,drive=drive0 ", iso);
+"-device ide-cd,id=cd0,drive=drive0 ", iso);
 port = ahci_port_select(ahci);
 
 ahci_atapi_eject(ahci, port);
@@ -1591,13 +1591,13 @@ static void test_atapi_tray(void)
 
 /* Remove media */
 qmp_async("{'execute': 'blockdev-open-tray', "
-   "'arguments': {'device': 'drive0'}}");
+   "'arguments': {'id': 'cd0'}}");
 atapi_wait_tray(true);
 rsp = qmp_receive();
 QDECREF(rsp);
 
 qmp_discard_response("{'execute': 'x-blockdev-remove-medium', "
- "'arguments': {'device': 'drive0'}}");
+ "'arguments': {'id': 'cd0'}}");
 
 /* Test the tray without a medium */
 ahci_atapi_load(ahci, port);
@@ -1613,12 +1613,12 @@ static void test_atapi_tray(void)
 "'file': { 'driver': 'file', "
   "'filename': %s }}}", iso);
 qmp_discard_response("{'execute': 'x-blockdev-insert-medium',"
-  "'arguments': { 'device': 'drive0', "
+  "'arguments': { 'id': 'cd0', "
  "'node-name': 'node0' }}");
 
 /* Again, the event shows up first */
 qmp_async("{'execute': 'blockdev-close-tray', "
-   "'arguments': {'device': 'drive0'}}");
+   "'arguments': {'id': 'cd0'}}");
 atapi_wait_tray(false);
 rsp = qmp_receive();
 QDECREF(rsp);
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 0/4] blockdev: Mark BD-{remove, insert}-medium stable

[Max Reitz]

Berto's "Test I/O limits with removable media" patch proves that
throttling survives a blockdev-remove-medium/blockdev-insert-medium pair
now, so let's mark them stable (because that was the reason they were
considered experimental, see commit
6e0abc251dd4f8eba1f53656dfede12e5840e83b for more).

But before we do that, let's use the chance and drop the @device
parameter.


Based-on: 
("Fix throttling crashes in BlockBackend with no BlockDriverState",
 because of the test case added there)


Max Reitz (4):
  iotests: Make BD-{remove,insert}-medium use @id
  tests/ahci: Switch tray and medium commands to @id
  blockdev: Drop BD-{remove,insert}-medium's @device
  blockdev: Mark BD-{remove,insert}-medium stable

 qapi/block-core.json   |  42 ---
 blockdev.c |  30 ++--
 tests/ahci-test.c  |  16 ++---
 tests/qemu-iotests/093 |   6 +-
 tests/qemu-iotests/118 | 184 +++--
 tests/qemu-iotests/139 |   2 +-
 tests/qemu-iotests/155 |  60 
 7 files changed, 163 insertions(+), 177 deletions(-)

-- 
2.13.6

Re: [Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

[Alberto Garcia]

On Fri 10 Nov 2017 11:08:20 PM CET, Max Reitz  wrote:
>> I just noticed a typo in the commit message:
>> 
>>> There'a a couple of problems with this:
>> 
>> "There's a couple"
>> 
>> If there's no v2 of this series you can correct this when committing.
>
> Well, the issue is that it's going to be interesting to commit this
> because it depends on Stefan's tree now...

I meant whoever commits this, Stefan in this case I guess :-)

Berto

Re: [Qemu-devel] [PATCH for-2.12 2/3] block: Handle null backing link

[Max Reitz]

On 2017-11-10 23:22, Eric Blake wrote:
> On 11/10/2017 04:13 PM, Max Reitz wrote:
>> Instead of converting all "backing": null instances into "backing": "",
>> handle a null value directly in bdrv_open_inherit().
>>
>> This enables explicitly null backing links for json:{} filenames.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  block.c|  2 +-
>>  blockdev.c | 14 --
>>  tests/qemu-iotests/089 | 20 
>>  tests/qemu-iotests/089.out |  8 
>>  4 files changed, 29 insertions(+), 15 deletions(-)
>>
> 
>> @@ -3899,19 +3898,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
>> **errp)
>>  
>>  qdict_flatten(qdict);
>>  
>> -/*
>> - * Rewrite "backing": null to "backing": ""
>> - * TODO Rewrite "" to null instead, and perhaps not even here
>> - */
> 
> Nice that the TODO told you what to do :)

Well, not really, because I disagree that it needs to be rewritten at
all.  I think we just need to deprecate and later disallow "backing":
"", which would absolve us from all of the rewriting trouble.

This code was added here because Markus needed to allow "backing": null
in a hurry (as far as I remember), so he added it centrally here instead
of checking how many places there are that evaluate "backing": "".

I should maybe have said that patch 3 is more of an RFC.  I'm not sure
whether other people agree that "backing": "" should be deprecated --
and if not, we would have to rewrite it still.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.12 2/3] block: Handle null backing link

[Eric Blake]

On 11/10/2017 04:13 PM, Max Reitz wrote:
> Instead of converting all "backing": null instances into "backing": "",
> handle a null value directly in bdrv_open_inherit().
> 
> This enables explicitly null backing links for json:{} filenames.
> 
> Signed-off-by: Max Reitz 
> ---
>  block.c|  2 +-
>  blockdev.c | 14 --
>  tests/qemu-iotests/089 | 20 
>  tests/qemu-iotests/089.out |  8 
>  4 files changed, 29 insertions(+), 15 deletions(-)
> 

> @@ -3899,19 +3898,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
> **errp)
>  
>  qdict_flatten(qdict);
>  
> -/*
> - * Rewrite "backing": null to "backing": ""
> - * TODO Rewrite "" to null instead, and perhaps not even here
> - */

Nice that the TODO told you what to do :)

Reviewed-by: Eric Blake 

-- 
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] [PATCH 3/3] qemu-iotests: Test I/O limits with removable media

[Max Reitz]

On 2017-11-10 19:54, Alberto Garcia wrote:
> This test hotplugs a CD drive to a VM and checks that I/O limits can
> be set only when the drive has media inserted and that they are kept
> when the media is replaced.
> 
> This also tests the removal of a device with valid I/O limits set but
> no media inserted. This involves deleting and disabling the limits
> of a BlockBackend without BlockDriverState, a scenario that has been
> crashing until the fixes from the last couple of patches.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  tests/qemu-iotests/093 | 62 
> ++
>  tests/qemu-iotests/093.out |  4 +--
>  2 files changed, 64 insertions(+), 2 deletions(-)

By the way, I just noticed that this test tests that
x-blockdev-remove-medium and x-blockdev-insert-medium do not destroy
throttling information -- which is exactly why those commands had been
declared experimental in the first place.  So I guess this means we can
drop the x- now. :-)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()

[Eric Blake]

On 11/10/2017 04:13 PM, Max Reitz wrote:
> Signed-off-by: Max Reitz 
> ---
>  include/qapi/qmp/qdict.h |  1 +
>  qobject/qdict.c  | 10 ++
>  2 files changed, 11 insertions(+)
> 

Reviewed-by: Eric Blake 

-- 
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] [PATCH for-2.12 3/3] block: Deprecate "backing": ""

[Eric Blake]

On 11/10/2017 04:13 PM, Max Reitz wrote:
> We have a clear replacement, so let's deprecate it.
> 
> Signed-off-by: Max Reitz 
> ---
>  qapi/block-core.json | 4 ++--
>  block.c  | 4 
>  qemu-doc.texi| 7 +++
>  qemu-options.hx  | 4 ++--
>  4 files changed, 15 insertions(+), 4 deletions(-)

Dan has more details on the proper documentation to tweak when declaring
something deprecated.  So this patch is incomplete, but what you have
makes sense.

-- 
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] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

[Max Reitz]

On 2017-11-10 23:15, Eric Blake wrote:
> On 11/10/2017 04:00 PM, Max Reitz wrote:
>>> Trying to understand this: we have a double corruption, because we
>>> encountered a refblock that points outside of the image, but fixing the
>>> refblock in turn encounters a second refblock that points within the
>>> image but to an unaligned area.
>>
>> No, it's the very same.  As far as I've seen it, the repair function
>> tries to fix the "refblock is outside image" error by resizing the image
>> so the refblock is inside the image.  However, the subsequent
>> bdrv_truncate() detects the alignment corruption, too, and thus marks
>> the image corrupt.
> 
> Is resizing the image to be larger always a wise thing compared to just
> rebuilding the refcount?  If I stick a large enough out-of-image value
> in the table, can I cause a denial-of-service by making qemu try to
> allocate petabytes of storage just to bring it into range?

But it's just a qcow2 resize (with no preallocation), so nothing will be
allocated.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

[Eric Blake]

On 11/10/2017 04:00 PM, Max Reitz wrote:
>> Trying to understand this: we have a double corruption, because we
>> encountered a refblock that points outside of the image, but fixing the
>> refblock in turn encounters a second refblock that points within the
>> image but to an unaligned area.
> 
> No, it's the very same.  As far as I've seen it, the repair function
> tries to fix the "refblock is outside image" error by resizing the image
> so the refblock is inside the image.  However, the subsequent
> bdrv_truncate() detects the alignment corruption, too, and thus marks
> the image corrupt.

Is resizing the image to be larger always a wise thing compared to just
rebuilding the refcount?  If I stick a large enough out-of-image value
in the table, can I cause a denial-of-service by making qemu try to
allocate petabytes of storage just to bring it into range?

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH for-2.12 2/3] block: Handle null backing link

[Max Reitz]

Instead of converting all "backing": null instances into "backing": "",
handle a null value directly in bdrv_open_inherit().

This enables explicitly null backing links for json:{} filenames.

Signed-off-by: Max Reitz 
---
 block.c|  2 +-
 blockdev.c | 14 --
 tests/qemu-iotests/089 | 20 
 tests/qemu-iotests/089.out |  8 
 4 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 85427df577..bc92ddd5a0 100644
--- a/block.c
+++ b/block.c
@@ -2558,7 +2558,7 @@ static BlockDriverState *bdrv_open_inherit(const char 
*filename,
 
 /* See cautionary note on accessing @options above */
 backing = qdict_get_try_str(options, "backing");
-if (backing && *backing == '\0') {
+if (qdict_is_qnull(options, "backing") || (backing && *backing == '\0')) {
 flags |= BDRV_O_NO_BACKING;
 qdict_del(options, "backing");
 }
diff --git a/blockdev.c b/blockdev.c
index 56a6b24a0b..99ef618b39 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3885,7 +3885,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
**errp)
 QObject *obj;
 Visitor *v = qobject_output_visitor_new();
 QDict *qdict;
-const QDictEntry *ent;
 Error *local_err = NULL;
 
 visit_type_BlockdevOptions(v, NULL, , _err);
@@ -3899,19 +3898,6 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
**errp)
 
 qdict_flatten(qdict);
 
-/*
- * Rewrite "backing": null to "backing": ""
- * TODO Rewrite "" to null instead, and perhaps not even here
- */
-for (ent = qdict_first(qdict); ent; ent = qdict_next(qdict, ent)) {
-char *dot = strrchr(ent->key, '.');
-
-if (!strcmp(dot ? dot + 1 : ent->key, "backing")
-&& qobject_type(ent->value) == QTYPE_QNULL) {
-qdict_put(qdict, ent->key, qstring_new());
-}
-}
-
 if (!qdict_get_try_str(qdict, "node-name")) {
 error_setg(errp, "'node-name' must be specified for the root node");
 goto fail;
diff --git a/tests/qemu-iotests/089 b/tests/qemu-iotests/089
index 9bfe2307b3..832eac1248 100755
--- a/tests/qemu-iotests/089
+++ b/tests/qemu-iotests/089
@@ -82,6 +82,26 @@ $QEMU_IO_PROG --cache $CACHEMODE \
 $QEMU_IO -c 'read -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
 
 
+echo
+echo "=== Testing correct handling of 'backing':null ==="
+echo
+
+_make_test_img -b "$TEST_IMG.base" $IMG_SIZE
+
+# This should read 42
+$QEMU_IO -c 'read -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
+
+# This should read 0
+$QEMU_IO -c 'read -P 0 0 512' "json:{\
+'driver': '$IMGFMT',
+'file': {
+'driver': 'file',
+'filename': '$TEST_IMG'
+},
+'backing': null
+}" | _filter_qemu_io
+
+
 # Taken from test 071
 echo
 echo "=== Testing blkdebug ==="
diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out
index 18f5fdda7a..607a9c6d9c 100644
--- a/tests/qemu-iotests/089.out
+++ b/tests/qemu-iotests/089.out
@@ -19,6 +19,14 @@ Pattern verification failed at offset 0, 512 bytes
 read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+=== Testing correct handling of 'backing':null ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
backing_file=TEST_DIR/t.IMGFMT.base
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
 === Testing blkdebug ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()

[Max Reitz]

Signed-off-by: Max Reitz 
---
 include/qapi/qmp/qdict.h |  1 +
 qobject/qdict.c  | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index fc218e7be6..c65ebfc748 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -76,6 +76,7 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
   int64_t def_value);
 bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
+bool qdict_is_qnull(const QDict *qdict, const char *key);
 
 void qdict_copy_default(QDict *dst, QDict *src, const char *key);
 void qdict_set_default_str(QDict *dst, const char *key, const char *val);
diff --git a/qobject/qdict.c b/qobject/qdict.c
index e8f15f1132..a032ea629a 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -294,6 +294,16 @@ const char *qdict_get_try_str(const QDict *qdict, const 
char *key)
 }
 
 /**
+ * qdict_is_qnull(): Return true if the value for 'key' is QNull
+ */
+bool qdict_is_qnull(const QDict *qdict, const char *key)
+{
+QObject *value = qdict_get(qdict, key);
+
+return value && value->type == QTYPE_QNULL;
+}
+
+/**
  * qdict_iter(): Iterate over all the dictionary's stored values.
  *
  * This function allows the user to provide an iterator, which will be
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 3/3] block: Deprecate "backing": ""

[Max Reitz]

We have a clear replacement, so let's deprecate it.

Signed-off-by: Max Reitz 
---
 qapi/block-core.json | 4 ++--
 block.c  | 4 
 qemu-doc.texi| 7 +++
 qemu-options.hx  | 4 ++--
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 76bf50f813..dfe4d3650c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1090,7 +1090,7 @@
 # @overlay: reference to the existing block device that will become
 #   the overlay of @node, as part of creating the snapshot.
 #   It must not have a current backing file (this can be
-#   achieved by passing "backing": "" to blockdev-add).
+#   achieved by passing "backing": null to blockdev-add).
 #
 # Since: 2.5
 ##
@@ -1238,7 +1238,7 @@
 # "node-name": "node1534",
 # "file": { "driver": "file",
 #   "filename": "hd1.qcow2" },
-# "backing": "" } }
+# "backing": null } }
 #
 # <- { "return": {} }
 #
diff --git a/block.c b/block.c
index bc92ddd5a0..463c4de25b 100644
--- a/block.c
+++ b/block.c
@@ -2559,6 +2559,10 @@ static BlockDriverState *bdrv_open_inherit(const char 
*filename,
 /* See cautionary note on accessing @options above */
 backing = qdict_get_try_str(options, "backing");
 if (qdict_is_qnull(options, "backing") || (backing && *backing == '\0')) {
+if (backing) {
+warn_report("Use of \"backing\": \"\" is deprecated; "
+"use \"backing\": null instead");
+}
 flags |= BDRV_O_NO_BACKING;
 qdict_del(options, "backing");
 }
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 8c10956a66..8f57d9ad21 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2537,6 +2537,13 @@ or ``ivshmem-doorbell`` device types.
 The ``spapr-pci-vfio-host-bridge'' device type is replaced by
 the ``spapr-pci-host-bridge'' device type.
 
+@section Block device options
+
+@subsection "backing": "" (since 2.12.0)
+
+In order to prevent QEMU from automatically opening an image's backing
+chain, use ``"backing": null'' instead.
+
 @node License
 @appendix License
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 3728e9b4dd..0ee1a04d00 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -731,8 +731,8 @@ Reference to or definition of the data source block driver 
node
 
 @item backing
 Reference to or definition of the backing file block device (default is taken
-from the image file). It is allowed to pass an empty string here in order to
-disable the default backing file.
+from the image file). It is allowed to pass @code{null} here in order to 
disable
+the default backing file.
 
 @item lazy-refcounts
 Whether to enable the lazy refcounts feature (on/off; default is taken from the
-- 
2.13.6

[Qemu-devel] [PATCH for-2.12 0/3] block: Handle null backing link

[Max Reitz]

Currently, we try to rewrite every occurrence of "backing": null into
"backing": "" in qmp_blockdev_add().  However, that breaks using the
same "backing": null construction in json:{} file names (which do not go
through qmp_blockdev_add()).  Currently, these then just behave as if
the option has not been specified.

Since there is actually only one place where we evaluate the @backing
option to find out whether not to use a backing file, we can instead
just check for null there.  It doesn't matter that this changes the
runtime state of the option from "" to null, because nobody really does
anything with that runtime state anyway (except put it into qemu again,
but qemu doesn't care whether it's "" or null).

And in the future, it's much better if we get it to be null in that
runtime state sooner than later -- see patch 3. :-)


Max Reitz (3):
  qapi: Add qdict_is_null()
  block: Handle null backing link
  block: Deprecate "backing": ""

 qapi/block-core.json   |  4 ++--
 include/qapi/qmp/qdict.h   |  1 +
 block.c|  6 +-
 blockdev.c | 14 --
 qobject/qdict.c| 10 ++
 qemu-doc.texi  |  7 +++
 qemu-options.hx|  4 ++--
 tests/qemu-iotests/089 | 20 
 tests/qemu-iotests/089.out |  8 
 9 files changed, 55 insertions(+), 19 deletions(-)

-- 
2.13.6

Re: [Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

[Max Reitz]

On 2017-11-10 23:06, Alberto Garcia wrote:
> On Fri 10 Nov 2017 07:54:47 PM CET, Alberto Garcia  wrote:
> 
> I just noticed a typo in the commit message:
> 
>> There'a a couple of problems with this:
> 
> "There's a couple"
> 
> If there's no v2 of this series you can correct this when committing.

Well, the issue is that it's going to be interesting to commit this
because it depends on Stefan's tree now...

Maybe this series can go through his as well?

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

[Alberto Garcia]

On Fri 10 Nov 2017 07:54:47 PM CET, Alberto Garcia  wrote:

I just noticed a typo in the commit message:

> There'a a couple of problems with this:

"There's a couple"

If there's no v2 of this series you can correct this when committing.

Berto

Re: [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

[Max Reitz]

On 2017-11-10 22:54, Eric Blake wrote:
> On 11/10/2017 02:31 PM, Max Reitz wrote:
>> Instead of using an assertion, it is better to emit a corruption event
>> here.  Checking all offsets for correct alignment can be tedious and it
>> is easily possible to forget to do so.  qcow2_cache_do_get() is a
>> function every L2 and refblock access has to go through, so this is a
>> good central point to add such a check.
>>
>> And for good measure, let us also add an assertion that the offset is
>> non-zero.  Making this a corruption event is not feasible, because a
>> zero offset usually means something special (such as the cluster is
>> unused), so all callers should be checking this anyway.  If they do not,
>> it is their fault, hence the assertion here.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  block/qcow2-cache.c| 21 +
>>  tests/qemu-iotests/060 | 21 +
>>  tests/qemu-iotests/060.out | 29 +
>>  3 files changed, 71 insertions(+)
>>
> 
>> +--- Repairing ---
>> +Repairing refcount block 1 is outside image
>> +ERROR refcount block 2 is not cluster aligned; refcount table entry 
>> corrupted
>> +qcow2: Marking image as corrupt: Refblock offset 0x200 unaligned (reftable 
>> index: 0x2); further corruption events will be suppressed
>> +Can't get refcount for cluster 1048576: Input/output error
> 
> Trying to understand this: we have a double corruption, because we
> encountered a refblock that points outside of the image, but fixing the
> refblock in turn encounters a second refblock that points within the
> image but to an unaligned area.

No, it's the very same.  As far as I've seen it, the repair function
tries to fix the "refblock is outside image" error by resizing the image
so the refblock is inside the image.  However, the subsequent
bdrv_truncate() detects the alignment corruption, too, and thus marks
the image corrupt.

The check function itself never marks the image corrupt because it's
doing its best to fix it. :-)

(And the only point in marking an image corrupt is to force the user to
repair it.)

And that's also the reason why we need to invoke the repair twice: On
the first run the check function notices that the image is so broken we
need to create new refcount structures, so it does that.  But it cannot
free the old structures (or something) because bs->drv == NULL by now.
And then it cannot be run a second time because !bs->drv.

So you need to manually invoke it a second time, and then it goes over
the newly created refcount structures which are then fixed up.

> Of course, you should never encounter these bad refblocks in normal
> usage, but when it comes to dealing with untrusted images, being robust
> is always worth it.
> 
> Reviewed-by: Eric Blake 

Thanks!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

[Eric Blake]

On 11/10/2017 02:31 PM, Max Reitz wrote:
> Instead of using an assertion, it is better to emit a corruption event
> here.  Checking all offsets for correct alignment can be tedious and it
> is easily possible to forget to do so.  qcow2_cache_do_get() is a
> function every L2 and refblock access has to go through, so this is a
> good central point to add such a check.
> 
> And for good measure, let us also add an assertion that the offset is
> non-zero.  Making this a corruption event is not feasible, because a
> zero offset usually means something special (such as the cluster is
> unused), so all callers should be checking this anyway.  If they do not,
> it is their fault, hence the assertion here.
> 
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2-cache.c| 21 +
>  tests/qemu-iotests/060 | 21 +
>  tests/qemu-iotests/060.out | 29 +
>  3 files changed, 71 insertions(+)
> 

> +--- Repairing ---
> +Repairing refcount block 1 is outside image
> +ERROR refcount block 2 is not cluster aligned; refcount table entry corrupted
> +qcow2: Marking image as corrupt: Refblock offset 0x200 unaligned (reftable 
> index: 0x2); further corruption events will be suppressed
> +Can't get refcount for cluster 1048576: Input/output error

Trying to understand this: we have a double corruption, because we
encountered a refblock that points outside of the image, but fixing the
refblock in turn encounters a second refblock that points within the
image but to an unaligned area.

Of course, you should never encounter these bad refblocks in normal
usage, but when it comes to dealing with untrusted images, being robust
is always worth it.

Reviewed-by: Eric Blake 

-- 
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] [PATCH for-2.11 4/5] qcow2: Add bounds check to get_refblock_offset()

[Eric Blake]

On 11/10/2017 02:31 PM, Max Reitz wrote:
> Reported-by: R. Nageswara Sastry 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1728661
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2.h  |  6 --
>  block/qcow2-refcount.c | 26 +-
>  tests/qemu-iotests/060 | 46 
> ++
>  tests/qemu-iotests/060.out | 22 ++
>  4 files changed, 93 insertions(+), 7 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
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] [PATCH for-2.11 3/5] block: Guard against NULL bs->drv

[Eric Blake]

On 11/10/2017 02:31 PM, Max Reitz wrote:
> We currently do not guard everywhere against a NULL bs->drv where we
> should be doing so.  Most of the places fixed here just do not care
> about that case at all.
> 
> Some care implicitly, e.g. through a prior function call to
> bdrv_getlength() which would always fail for an ejected BDS.  Add an
> assert there to make it more obvious.
> 
> Other places seem to care, but do so insufficiently: Freeing clusters in
> a qcow2 image is an error-free operation, but it may leave the image in
> an unusable state anyway.  Giving qcow2_free_clusters() an error code is
> not really viable, it is much easier to note that bs->drv may be NULL
> even after a successful driver call.  This concerns bdrv_co_flush(), and
> the way the check is added to bdrv_co_pdiscard() (in every iteration
> instead of only once).
> 
> Finally, some places employ at least an assert(bs->drv); somewhere, that
> may be reasonable (such as in the reopen code), but in
> bdrv_has_zero_init(), it is definitely not.  Returning 0 there in case
> of an ejected BDS saves us much headache instead.
> 
> Reported-by: R. Nageswara Sastry 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1728660
> Signed-off-by: Max Reitz 
> ---

> +++ b/block/replication.c

>  
> +if (!s->hidden_disk->bs->drv) {
> +error_setg(errp, "Hidden disk %s is ejected",
> +   s->hidden_disk->bs->node_name);
> +return;
> +}

How would the hidden disk ever be ejected?  Could this be an assert instead?

But what you have is equally safe, so
Reviewed-by: Eric Blake 

-- 
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] [PATCH v2] linux-user: fix is_proc_myself to check the paths via realpath

[Zach Riggle]

Day 17 Ping :)



*Zach Riggle*

On Tue, Nov 7, 2017 at 2:06 PM, Riku Voipio  wrote:

> Hi,
>
> On Mon, Nov 06, 2017 at 08:17:44PM +, Zach Riggle wrote:
> > Ping! What needs to be done to move this forward? My current
> implementation
> > is compatible with musl.
>
> I'll have a look at it soon.
>
> Riku
>
> > On Thu, Nov 2, 2017 at 12:36 PM Peter Maydell 
> > wrote:
> >
> > > On 28 October 2017 at 06:14, Eric Blake  wrote:
> > > > By definition, in linux-user, we ARE using glibc; therefore, you are
> > > > free to use all GNU extensions.
> > >
> > > Don't we also support musl libc? I forget...
> > >
> > > thanks
> > > -- PMM
> > >
>

Re: [Qemu-devel] [PATCH] iotests: Add test for failing qemu-img commit

[Max Reitz]

On 2017-06-16 15:58, Max Reitz wrote:
> Signed-off-by: Max Reitz 
> ---
> In order to pass, this depends on "fix: avoid an infinite loop or a
> dangling pointer problem in img_commit"
> (http://lists.nongnu.org/archive/html/qemu-block/2017-06/msg00443.html)
> and on the "block: Don't compare strings in bdrv_reopen_prepare()"
> series
> (http://lists.nongnu.org/archive/html/qemu-block/2017-06/msg00424.html).
> ---
>  tests/qemu-iotests/020 | 27 +++
>  tests/qemu-iotests/020.out | 17 +
>  2 files changed, 44 insertions(+)

Finally applied to my block tree:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.11 2/5] qcow2: Unaligned zero cluster in handle_alloc()

[Eric Blake]

On 11/10/2017 02:31 PM, Max Reitz wrote:
> We should check whether the cluster offset we are about to use is
> actually valid; that is, whether it is aligned to cluster boundaries.
> 
> Reported-by: R. Nageswara Sastry 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1728643
> Buglink: https://bugs.launchpad.net/qemu/+bug/1728657
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2-cluster.c  | 13 -
>  tests/qemu-iotests/060 | 16 
>  tests/qemu-iotests/060.out | 10 ++
>  3 files changed, 38 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake 

-- 
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] [PATCH for-2.11 1/5] qcow2: check_errors are fatal

[Eric Blake]

On 11/10/2017 02:31 PM, Max Reitz wrote:
> When trying to repair a dirty image, qcow2_check() may apparently
> succeed (no really fatal error occurred that would prevent the check
> from continuing), but if check_errors in the result object is non-zero,
> we cannot trust the image to be usable.
> 
> Reported-by: R. Nageswara Sastry 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1728639
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2.c  |  5 -
>  tests/qemu-iotests/060 | 20 
>  tests/qemu-iotests/060.out | 23 +++
>  3 files changed, 47 insertions(+), 1 deletion(-)
> 
Reviewed-by: Eric Blake 

-- 
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] [PATCH for-2.12 1/1] qcow2: Repair unaligned preallocated zero clusters

[Eric Blake]

On 11/10/2017 02:37 PM, Max Reitz wrote:
> We can easily repair unaligned preallocated zero clusters by discarding
> them, so why not do it?
> 
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2-refcount.c | 70 
> ++
>  tests/qemu-iotests/060 |  3 +-
>  tests/qemu-iotests/060.out |  9 ++
>  3 files changed, 69 insertions(+), 13 deletions(-)
> 

>  
> +/* Correct offsets are cluster aligned */
> +if (offset_into_cluster(s, offset)) {
> +if (qcow2_get_cluster_type(l2_entry) ==
> +QCOW2_CLUSTER_ZERO_ALLOC)
> +{
> +fprintf(stderr, "%s offset=%" PRIx64 ": Preallocated 
> zero "
> +"cluster is not properly aligned; L2 entry "
> +"corrupted.\n",
> +fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR",
> +offset);

Is fprintf() still the right thing to be using?

> @@ -1586,13 +1637,6 @@ static int check_refcounts_l2(BlockDriverState *bs, 
> BdrvCheckResult *res,
>  if (ret < 0) {
>  goto fail;
>  }
> -
> -/* Correct offsets are cluster aligned */
> -if (offset_into_cluster(s, offset)) {
> -fprintf(stderr, "ERROR offset=%" PRIx64 ": Cluster is not "
> -"properly aligned; L2 entry corrupted.\n", offset);

But it's no worse than what you are refactoring.

Reviewed-by: Eric Blake 

-- 
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] [PATCH for-2.11 0/5] qcow2: Fixes for corrupted images

[Max Reitz]

On 2017-11-10 21:31, Max Reitz wrote:
> This series contains fixes for another batch of qcow2-related crashes
> reported on Launchpad by Nageswara (the first batch was
> http://lists.nongnu.org/archive/html/qemu-block/2017-11/msg00082.html by
> Berto).
> 
> Patch 4 fixes an out-of-bounds array access in memory which is not
> really a security issue for multiple reasons (really, at most you can
> read eight bytes from somewhere with an extremely high chance of
> crashing qemu and requiring the user to invoke a block_resize shrinking
> the qcow2 image (and also reset some bit in the image from 1 to 0, but
> only if the overlap checks don't catch you)), but most importantly that
> code hasn't been in 2.10, so we're fine.
> 
> 
> Max Reitz (5):
>   qcow2: check_errors are fatal
>   qcow2: Unaligned zero cluster in handle_alloc()
>   block: Guard against NULL bs->drv
>   qcow2: Add bounds check to get_refblock_offset()
>   qcow2: Refuse to get unaligned offsets from cache
> 
>  block/qcow2.h  |   6 ---
>  block.c|  19 ++-
>  block/io.c |  36 +
>  block/qapi.c   |   8 ++-
>  block/qcow2-cache.c|  21 
>  block/qcow2-cluster.c  |  13 -
>  block/qcow2-refcount.c |  26 +-
>  block/qcow2.c  |   5 +-
>  block/replication.c|  15 ++
>  block/vvfat.c  |   2 +-
>  tests/qemu-iotests/060 | 125 
> +
>  tests/qemu-iotests/060.out | 115 +
>  12 files changed, 379 insertions(+), 12 deletions(-)

I see that Patchew complains, so let's try:

Based-on: 

And let's see whether it can handle the recursive dependency...

(Letting Patchew base something on git branches would be nice O:-))


Also note my follow-up patch "qcow2: Repair unaligned preallocated zero
clusters" which fixes the TODO added in patch 2.

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH for-2.12 0/1] qcow2: Repair unaligned preallocated zero clusters

[Max Reitz]

This is a follow-up to patch 2 of my "qcow2: Unaligned zero cluster in
handle_alloc()" series.  That patch adds a way to correctly deal with
such clusters, this patch here adds a way to repair them.

Naturally, this patch is therefore based on that series:

Based-on: <20171110203111.7666-1-mre...@redhat.com>


Max Reitz (1):
  qcow2: Repair unaligned preallocated zero clusters

 block/qcow2-refcount.c | 70 ++
 tests/qemu-iotests/060 |  3 +-
 tests/qemu-iotests/060.out |  9 ++
 3 files changed, 69 insertions(+), 13 deletions(-)

-- 
2.13.6

Re: [Qemu-devel] [PATCH] virtio: fix descriptor counting in virtqueue_pop

[Michael S. Tsirkin]

On Fri, Nov 10, 2017 at 03:41:37PM +, Stefan Hajnoczi wrote:
> On Thu, Oct 05, 2017 at 08:03:35PM +0200, Alexandre DERUMIER wrote:
> > Hi,
> > 
> > has somebody reviewed this patch ?
> > 
> > I'm also able de reproduce the vm crash like the proxmox user.
> > This patch is fixing it for me too.
> 
> This patch should go through Michael Tsirkin's tree.  I have pinged him
> separately in case this email thread got buried in his inbox.
> 
> Stefan

Isn't this upstream?
I see it as commit 37ef70be6af7e9f2a6f852c68f74bd98dac2664b there.


> > 
> > Regards,
> > 
> > Alexandre
> > 
> > 
> > - Mail original -
> > De: "Wolfgang Bumiller" 
> > À: "qemu-devel" 
> > Cc: "pbonzini" , "Michael S. Tsirkin" 
> > Envoyé: Mercredi 20 Septembre 2017 08:09:33
> > Objet: [Qemu-devel] [PATCH] virtio: fix descriptor counting in virtqueue_pop
> > 
> > While changing the s/g list allocation, commit 3b3b0628 
> > also changed the descriptor counting to count iovec entries 
> > as split by cpu_physical_memory_map(). Previously only the 
> > actual descriptor entries were counted and the split into 
> > the iovec happened afterwards in virtqueue_map(). 
> > Count the entries again instead to avoid erroneous 
> > "Looped descriptor" errors. 
> > 
> > Reported-by: Hans Middelhoek  
> > Link: https://forum.proxmox.com/threads/vm-crash-with-memory-hotplug.35904/ 
> > Fixes: 3b3b0628217e ("virtio: slim down allocation of VirtQueueElements") 
> > Signed-off-by: Wolfgang Bumiller  
> > --- 
> > hw/virtio/virtio.c | 6 +++--- 
> > 1 file changed, 3 insertions(+), 3 deletions(-) 
> > 
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c 
> > index 890b4d7eb7..33bb770177 100644 
> > --- a/hw/virtio/virtio.c 
> > +++ b/hw/virtio/virtio.c 
> > @@ -834,7 +834,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) 
> > int64_t len; 
> > VirtIODevice *vdev = vq->vdev; 
> > VirtQueueElement *elem = NULL; 
> > - unsigned out_num, in_num; 
> > + unsigned out_num, in_num, elem_entries; 
> > hwaddr addr[VIRTQUEUE_MAX_SIZE]; 
> > struct iovec iov[VIRTQUEUE_MAX_SIZE]; 
> > VRingDesc desc; 
> > @@ -852,7 +852,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) 
> > smp_rmb(); 
> > 
> > /* When we start there are none of either input nor output. */ 
> > - out_num = in_num = 0; 
> > + out_num = in_num = elem_entries = 0; 
> > 
> > max = vq->vring.num; 
> > 
> > @@ -922,7 +922,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) 
> > } 
> > 
> > /* If we've got too many, that implies a descriptor loop. */ 
> > - if ((in_num + out_num) > max) { 
> > + if (++elem_entries > max) { 
> > virtio_error(vdev, "Looped descriptor"); 
> > goto err_undo_map; 
> > } 
> > -- 
> > 2.11.0 
> > 
> > 

[Qemu-devel] [PATCH for-2.12 1/1] qcow2: Repair unaligned preallocated zero clusters

[Max Reitz]

We can easily repair unaligned preallocated zero clusters by discarding
them, so why not do it?

Signed-off-by: Max Reitz 
---
 block/qcow2-refcount.c | 70 ++
 tests/qemu-iotests/060 |  3 +-
 tests/qemu-iotests/060.out |  9 ++
 3 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3de1ab51ba..92701ab7af 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1508,7 +1508,7 @@ enum {
 static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
   void **refcount_table,
   int64_t *refcount_table_size, int64_t l2_offset,
-  int flags)
+  int flags, BdrvCheckMode fix)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l2_table, l2_entry;
@@ -1579,6 +1579,57 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 next_contiguous_offset = offset + s->cluster_size;
 }
 
+/* Correct offsets are cluster aligned */
+if (offset_into_cluster(s, offset)) {
+if (qcow2_get_cluster_type(l2_entry) ==
+QCOW2_CLUSTER_ZERO_ALLOC)
+{
+fprintf(stderr, "%s offset=%" PRIx64 ": Preallocated zero "
+"cluster is not properly aligned; L2 entry "
+"corrupted.\n",
+fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR",
+offset);
+if (fix & BDRV_FIX_ERRORS) {
+uint64_t l2e_offset =
+l2_offset + (uint64_t)i * sizeof(uint64_t);
+
+l2_entry = QCOW_OFLAG_ZERO;
+l2_table[i] = cpu_to_be64(l2_entry);
+ret = qcow2_pre_write_overlap_check(bs,
+QCOW2_OL_ACTIVE_L2 | QCOW2_OL_INACTIVE_L2,
+l2e_offset, sizeof(uint64_t));
+if (ret < 0) {
+fprintf(stderr, "ERROR: Overlap check failed\n");
+res->check_errors++;
+/* Something is seriously wrong, so abort checking
+ * this L2 table */
+goto fail;
+}
+
+ret = bdrv_pwrite_sync(bs->file, l2e_offset,
+   _table[i], sizeof(uint64_t));
+if (ret < 0) {
+fprintf(stderr, "ERROR: Failed to overwrite L2 "
+"table entry: %s\n", strerror(-ret));
+res->check_errors++;
+/* Do not abort, continue checking the rest of this
+ * L2 table's entries */
+} else {
+res->corruptions_fixed++;
+/* Skip marking the cluster as used
+ * (it is unused now) */
+continue;
+}
+} else {
+res->corruptions++;
+}
+} else {
+fprintf(stderr, "ERROR offset=%" PRIx64 ": Data cluster is 
"
+"not properly aligned; L2 entry corrupted.\n", offset);
+res->corruptions++;
+}
+}
+
 /* Mark cluster as used */
 ret = qcow2_inc_refcounts_imrt(bs, res,
refcount_table, refcount_table_size,
@@ -1586,13 +1637,6 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 if (ret < 0) {
 goto fail;
 }
-
-/* Correct offsets are cluster aligned */
-if (offset_into_cluster(s, offset)) {
-fprintf(stderr, "ERROR offset=%" PRIx64 ": Cluster is not "
-"properly aligned; L2 entry corrupted.\n", offset);
-res->corruptions++;
-}
 break;
 }
 
@@ -1626,7 +1670,7 @@ static int check_refcounts_l1(BlockDriverState *bs,
   void **refcount_table,
   int64_t *refcount_table_size,
   int64_t l1_table_offset, int l1_size,
-  int flags)
+  int flags, BdrvCheckMode fix)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t *l1_table = NULL, l2_offset, l1_size2;
@@ -1681,7 +1725,8 @@ static int check_refcounts_l1(BlockDriverState *bs,
 
 /* Process and check L2 entries */
 ret = check_refcounts_l2(bs, res, 

[Qemu-devel] [PATCH for-2.11 3/5] block: Guard against NULL bs->drv

[Max Reitz]

We currently do not guard everywhere against a NULL bs->drv where we
should be doing so.  Most of the places fixed here just do not care
about that case at all.

Some care implicitly, e.g. through a prior function call to
bdrv_getlength() which would always fail for an ejected BDS.  Add an
assert there to make it more obvious.

Other places seem to care, but do so insufficiently: Freeing clusters in
a qcow2 image is an error-free operation, but it may leave the image in
an unusable state anyway.  Giving qcow2_free_clusters() an error code is
not really viable, it is much easier to note that bs->drv may be NULL
even after a successful driver call.  This concerns bdrv_co_flush(), and
the way the check is added to bdrv_co_pdiscard() (in every iteration
instead of only once).

Finally, some places employ at least an assert(bs->drv); somewhere, that
may be reasonable (such as in the reopen code), but in
bdrv_has_zero_init(), it is definitely not.  Returning 0 there in case
of an ejected BDS saves us much headache instead.

Reported-by: R. Nageswara Sastry 
Buglink: https://bugs.launchpad.net/qemu/+bug/1728660
Signed-off-by: Max Reitz 
---
 block.c| 19 ++-
 block/io.c | 36 
 block/qapi.c   |  8 +++-
 block/replication.c| 15 +++
 block/vvfat.c  |  2 +-
 tests/qemu-iotests/060 | 22 ++
 tests/qemu-iotests/060.out | 31 +++
 7 files changed, 130 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index cec4e9a8d6..dce20ed5e7 100644
--- a/block.c
+++ b/block.c
@@ -720,6 +720,10 @@ static int refresh_total_sectors(BlockDriverState *bs, 
int64_t hint)
 {
 BlockDriver *drv = bs->drv;
 
+if (!drv) {
+return -ENOMEDIUM;
+}
+
 /* Do not attempt drv->bdrv_getlength() on scsi-generic devices */
 if (bdrv_is_sg(bs))
 return 0;
@@ -3431,6 +3435,10 @@ int bdrv_change_backing_file(BlockDriverState *bs,
 BlockDriver *drv = bs->drv;
 int ret;
 
+if (!drv) {
+return -ENOMEDIUM;
+}
+
 /* Backing file format doesn't make sense without a backing file */
 if (backing_fmt && !backing_file) {
 return -EINVAL;
@@ -3916,7 +3924,9 @@ int bdrv_has_zero_init_1(BlockDriverState *bs)
 
 int bdrv_has_zero_init(BlockDriverState *bs)
 {
-assert(bs->drv);
+if (!bs->drv) {
+return 0;
+}
 
 /* If BS is a copy on write image, it is initialized to
the contents of the base image, which may not be zeroes.  */
@@ -4245,6 +4255,10 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs,
 BdrvChild *child, *parent;
 int ret;
 
+if (!bs->drv) {
+return -ENOMEDIUM;
+}
+
 if (!setting_flag && bs->drv->bdrv_inactivate) {
 ret = bs->drv->bdrv_inactivate(bs);
 if (ret < 0) {
@@ -4780,6 +4794,9 @@ void bdrv_remove_aio_context_notifier(BlockDriverState 
*bs,
 int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb, void *cb_opaque)
 {
+if (!bs->drv) {
+return -ENOMEDIUM;
+}
 if (!bs->drv->bdrv_amend_options) {
 return -ENOTSUP;
 }
diff --git a/block/io.c b/block/io.c
index 3d5ef2cabe..4fdf93a014 100644
--- a/block/io.c
+++ b/block/io.c
@@ -853,6 +853,10 @@ static int coroutine_fn 
bdrv_driver_preadv(BlockDriverState *bs,
 
 assert(!(flags & ~BDRV_REQ_MASK));
 
+if (!drv) {
+return -ENOMEDIUM;
+}
+
 if (drv->bdrv_co_preadv) {
 return drv->bdrv_co_preadv(bs, offset, bytes, qiov, flags);
 }
@@ -894,6 +898,10 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 
 assert(!(flags & ~BDRV_REQ_MASK));
 
+if (!drv) {
+return -ENOMEDIUM;
+}
+
 if (drv->bdrv_co_pwritev) {
 ret = drv->bdrv_co_pwritev(bs, offset, bytes, qiov,
flags & bs->supported_write_flags);
@@ -945,6 +953,10 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, 
uint64_t offset,
 {
 BlockDriver *drv = bs->drv;
 
+if (!drv) {
+return -ENOMEDIUM;
+}
+
 if (!drv->bdrv_co_pwritev_compressed) {
 return -ENOTSUP;
 }
@@ -975,6 +987,10 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild 
*child,
 BDRV_REQUEST_MAX_BYTES);
 unsigned int progress = 0;
 
+if (!drv) {
+return -ENOMEDIUM;
+}
+
 /* FIXME We cannot require callers to have write permissions when all they
  * are doing is a read request. If we did things right, write permissions
  * would be obtained anyway, but internally by the copy-on-read code. As
@@ -1291,6 +1307,10 @@ static int coroutine_fn 
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
 bs->bl.request_alignment);
 int max_transfer = 

[Qemu-devel] [PATCH v1] migration/ram.c: do not set 'postcopy_running' in POSTCOPY_INCOMING_END

[Daniel Henrique Barboza]

When migrating a VM with 'migrate_set_capability postcopy-ram on'
a postcopy_state is set during the process, ending up with the
state POSTCOPY_INCOMING_END when the migration is over. This
postcopy_state is taken into account inside ram_load to check
how it will load the memory pages. This same ram_load is called when
in a loadvm command.

Inside ram_load, the logic to see if we're at postcopy_running state
is:

postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING

postcopy_state_get() returns this enum type:

typedef enum {
POSTCOPY_INCOMING_NONE = 0,
POSTCOPY_INCOMING_ADVISE,
POSTCOPY_INCOMING_DISCARD,
POSTCOPY_INCOMING_LISTENING,
POSTCOPY_INCOMING_RUNNING,
POSTCOPY_INCOMING_END
} PostcopyState;

In the case where ram_load is executed and postcopy_state is
POSTCOPY_INCOMING_END, postcopy_running will be set to 'true' and
ram_load will behave like a postcopy is in progress. This scenario isn't
achievable in a migration but it is reproducible when executing
savevm/loadvm after migrating with 'postcopy-ram on', causing loadvm
to fail with Error -22:

Source:

(qemu) migrate_set_capability postcopy-ram on
(qemu) migrate tcp:127.0.0.1:

Dest:

(qemu) migrate_set_capability postcopy-ram on
(qemu)
ubuntu1704-intel login:
Ubuntu 17.04 ubuntu1704-intel ttyS0

ubuntu1704-intel login: (qemu)
(qemu) savevm test1
(qemu) loadvm test1
Unknown combination of migration flags: 0x4 (postcopy mode)
error while loading state for instance 0x0 of device 'ram'
Error -22 while loading VM state
(qemu)

This patch fixes this problem by changing a bit the semantics
of postcopy_running inside ram_load, verifying first if
we're not in the POSTCOPY_INCOMING_END state. In this case,
postcopy_running is set to 'false'.

Signed-off-by: Daniel Henrique Barboza 
---
 migration/ram.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 8620aa400a..43ed719668 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2803,13 +2803,21 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 int flags = 0, ret = 0, invalid_flags = 0;
 static uint64_t seq_iter;
 int len = 0;
-/*
- * If system is running in postcopy mode, page inserts to host memory must
- * be atomic
- */
-bool postcopy_running = postcopy_state_get() >= 
POSTCOPY_INCOMING_LISTENING;
-/* ADVISE is earlier, it shows the source has the postcopy capability on */
-bool postcopy_advised = postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE;
+bool postcopy_advised = false, postcopy_running = false;
+uint8_t postcopy_state = postcopy_state_get();
+
+if (postcopy_state != POSTCOPY_INCOMING_END) {
+/*
+ * If system is running in postcopy mode, page inserts to host memory
+ * must be atomic
+ */
+postcopy_running = postcopy_state >= POSTCOPY_INCOMING_LISTENING;
+
+/* ADVISE is earlier, it shows the source has the postcopy
+ * capability on
+ */
+postcopy_advised = postcopy_state >= POSTCOPY_INCOMING_ADVISE;
+}
 
 seq_iter++;
 
-- 
2.13.6

Re: [Qemu-devel] [PATCH 3/3] qemu-iotests: Test I/O limits with removable media

[Max Reitz]

On 2017-11-10 19:54, Alberto Garcia wrote:
> This test hotplugs a CD drive to a VM and checks that I/O limits can
> be set only when the drive has media inserted and that they are kept
> when the media is replaced.
> 
> This also tests the removal of a device with valid I/O limits set but
> no media inserted. This involves deleting and disabling the limits
> of a BlockBackend without BlockDriverState, a scenario that has been
> crashing until the fixes from the last couple of patches.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  tests/qemu-iotests/093 | 62 
> ++
>  tests/qemu-iotests/093.out |  4 +--
>  2 files changed, 64 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for 2.11 5/5] hw: add .min_cpus and .default_cpus fields to machine_class

[Eduardo Habkost]

On Fri, Nov 10, 2017 at 02:53:46PM -0500, Emilio G. Cota wrote:
> max_cpus needs to be an upper bound on the number of vCPUs
> initialized; otherwise TCG region initialization breaks.
> 
> Some boards initialize a hard-coded number of vCPUs, which is not
> captured by the global max_cpus and therefore breaks TCG initialization.
> Fix it by adding the .min_cpus field to machine_class.
> 
> This commit also changes some user-facing behaviour: we now die if
> -smp is below this hard-coded vCPU minimum instead of silently
> ignoring the passed -smp value (sometimes announcing this by printing
> a warning). However, the introduction of .default_cpus lessens the
> likelihood that users will notice this: if -smp isn't set, we now
> assign the value in .default_cpus to both smp_cpus and max_cpus. IOW,
> if a user does not set -smp, they always get a correct number of vCPUs.
> 
> This change fixes 3468b59 ("tcg: enable multiple TCG contexts in
> softmmu", 2017-10-24), which broke TCG initialization for some
> ARM boards.
> 
> Fixes: 3468b59e18b179bc63c7ce934de912dfa9596122
> Reported-by: Thomas Huth 
> Suggested-by: Peter Maydell 
> Signed-off-by: Emilio G. Cota 
> ---
>  include/hw/boards.h |  5 +
>  hw/arm/exynos4_boards.c | 12 
>  hw/arm/raspi.c  |  2 ++
>  hw/arm/xlnx-zcu102.c|  2 ++
>  vl.c| 21 ++---
>  5 files changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 191a5b3..62f160e 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -102,6 +102,9 @@ typedef struct {
>  
>  /**
>   * MachineClass:
> + * @max_cpus: maximum number of CPUs supported. Default: 1
> + * @min_cpus: minimum number of CPUs supported. Default: 1
> + * @default_cpus: number of CPUs instantiated if none are specified. 
> Default: 1
>   * @get_hotplug_handler: this function is called during bus-less
>   *device hotplug. If defined it returns pointer to an instance
>   *of HotplugHandler object, which handles hotplug operation
> @@ -167,6 +170,8 @@ struct MachineClass {
>  BlockInterfaceType block_default_type;
>  int units_per_default_bus;
>  int max_cpus;
> +int min_cpus;
> +int default_cpus;
>  unsigned int no_serial:1,
>  no_parallel:1,
>  use_virtcon:1,
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index f1441ec..750162c 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -27,7 +27,6 @@
>  #include "qemu-common.h"
>  #include "cpu.h"
>  #include "sysemu/sysemu.h"
> -#include "sysemu/qtest.h"
>  #include "hw/sysbus.h"
>  #include "net/net.h"
>  #include "hw/arm/arm.h"
> @@ -129,13 +128,6 @@ exynos4_boards_init_common(MachineState *machine,
> Exynos4BoardType board_type)
>  {
>  Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
> -MachineClass *mc = MACHINE_GET_CLASS(machine);
> -
> -if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
> -error_report("%s board supports only %d CPU cores, ignoring smp_cpus"
> - " value",
> - mc->name, EXYNOS4210_NCPUS);
> -}
>  
>  exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type];
>  exynos4_board_binfo.board_id = exynos4_board_id[board_type];
> @@ -189,6 +181,8 @@ static void nuri_class_init(ObjectClass *oc, void *data)
>  mc->desc = "Samsung NURI board (Exynos4210)";
>  mc->init = nuri_init;
>  mc->max_cpus = EXYNOS4210_NCPUS;
> +mc->min_cpus = EXYNOS4210_NCPUS;
> +mc->default_cpus = EXYNOS4210_NCPUS;
>  mc->ignore_memory_transaction_failures = true;
>  }
>  
> @@ -205,6 +199,8 @@ static void smdkc210_class_init(ObjectClass *oc, void 
> *data)
>  mc->desc = "Samsung SMDKC210 board (Exynos4210)";
>  mc->init = smdkc210_init;
>  mc->max_cpus = EXYNOS4210_NCPUS;
> +mc->min_cpus = EXYNOS4210_NCPUS;
> +mc->default_cpus = EXYNOS4210_NCPUS;
>  mc->ignore_memory_transaction_failures = true;
>  }
>  
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 5941c9f..cd5fa8c 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -167,6 +167,8 @@ static void raspi2_machine_init(MachineClass *mc)
>  mc->no_floppy = 1;
>  mc->no_cdrom = 1;
>  mc->max_cpus = BCM2836_NCPUS;
> +mc->min_cpus = BCM2836_NCPUS;
> +mc->default_cpus = BCM2836_NCPUS;
>  mc->default_ram_size = 1024 * 1024 * 1024;
>  mc->ignore_memory_transaction_failures = true;
>  };
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index 190eb69..9631a53 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -189,6 +189,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass 
> *oc, void *data)
>  mc->units_per_default_bus = 1;
>  mc->ignore_memory_transaction_failures = true;
>  mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + 

[Qemu-devel] [PATCH for-2.11 2/5] qcow2: Unaligned zero cluster in handle_alloc()

[Max Reitz]

We should check whether the cluster offset we are about to use is
actually valid; that is, whether it is aligned to cluster boundaries.

Reported-by: R. Nageswara Sastry 
Buglink: https://bugs.launchpad.net/qemu/+bug/1728643
Buglink: https://bugs.launchpad.net/qemu/+bug/1728657
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c  | 13 -
 tests/qemu-iotests/060 | 16 
 tests/qemu-iotests/060.out | 10 ++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 2e072ed155..a3fec27bf9 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1308,10 +1308,21 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 (!*host_offset ||
  start_of_cluster(s, *host_offset) == (entry & L2E_OFFSET_MASK)))
 {
+int preallocated_nb_clusters;
+
+if (offset_into_cluster(s, entry & L2E_OFFSET_MASK)) {
+qcow2_signal_corruption(bs, true, -1, -1, "Preallocated zero "
+"cluster offset %#llx unaligned (guest "
+"offset: %#" PRIx64 ")",
+entry & L2E_OFFSET_MASK, guest_offset);
+ret = -EIO;
+goto fail;
+}
+
 /* Try to reuse preallocated zero clusters; contiguous normal clusters
  * would be fine, too, but count_cow_clusters() above has limited
  * nb_clusters already to a range of COW clusters */
-int preallocated_nb_clusters =
+preallocated_nb_clusters =
 count_contiguous_clusters(nb_clusters, s->cluster_size,
   _table[l2_index], QCOW_OFLAG_COPIED);
 assert(preallocated_nb_clusters > 0);
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 56bdf1ee2e..49bc89df38 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -321,6 +321,22 @@ echo '--- Repairing ---'
 # because the image was already marked corrupt by that point
 _check_test_img -r all
 
+echo
+echo "=== Writing to an unaligned preallocated zero cluster ==="
+echo
+
+_make_test_img 64M
+
+# Allocate the L2 table
+$QEMU_IO -c "write 0 64k" -c "discard 0 64k" "$TEST_IMG" | _filter_qemu_io
+# Pretend there is a preallocated zero cluster somewhere inside the
+# image header
+poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x00\x2a\x01"
+# Let's write to it!
+$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
+
+# Can't repair this yet (TODO: We can just deallocate the cluster)
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index f013fe73c0..c583076808 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -307,4 +307,14 @@ The following inconsistencies were found and repaired:
 
 Double checking the fixed image now...
 No errors were found on the image.
+
+=== Writing to an unaligned preallocated zero cluster ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+discard 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qcow2: Marking image as corrupt: Preallocated zero cluster offset 0x2a00 
unaligned (guest offset: 0); further corruption events will be suppressed
+write failed: Input/output error
 *** done
-- 
2.13.6

[Qemu-devel] [PATCH for-2.11 4/5] qcow2: Add bounds check to get_refblock_offset()

[Max Reitz]

Reported-by: R. Nageswara Sastry 
Buglink: https://bugs.launchpad.net/qemu/+bug/1728661
Signed-off-by: Max Reitz 
---
 block/qcow2.h  |  6 --
 block/qcow2-refcount.c | 26 +-
 tests/qemu-iotests/060 | 46 ++
 tests/qemu-iotests/060.out | 22 ++
 4 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 782a206ecb..6f0ff15dd0 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -527,12 +527,6 @@ uint32_t offset_to_reftable_index(BDRVQcow2State *s, 
uint64_t offset)
 return offset >> (s->refcount_block_bits + s->cluster_bits);
 }
 
-static inline uint64_t get_refblock_offset(BDRVQcow2State *s, uint64_t offset)
-{
-uint32_t index = offset_to_reftable_index(s, offset);
-return s->refcount_table[index] & REFT_OFFSET_MASK;
-}
-
 /* qcow2.c functions */
 int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
   int64_t sector_num, int nb_sectors);
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 60b8eef3e8..3de1ab51ba 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -3077,16 +3077,40 @@ done:
 return ret;
 }
 
+static int64_t get_refblock_offset(BlockDriverState *bs, uint64_t offset)
+{
+BDRVQcow2State *s = bs->opaque;
+uint32_t index = offset_to_reftable_index(s, offset);
+int64_t covering_refblock_offset = 0;
+
+if (index < s->refcount_table_size) {
+covering_refblock_offset = s->refcount_table[index] & REFT_OFFSET_MASK;
+}
+if (!covering_refblock_offset) {
+qcow2_signal_corruption(bs, true, -1, -1, "Refblock at %#" PRIx64 " is 
"
+"not covered by the refcount structures",
+offset);
+return -EIO;
+}
+
+return covering_refblock_offset;
+}
+
 static int qcow2_discard_refcount_block(BlockDriverState *bs,
 uint64_t discard_block_offs)
 {
 BDRVQcow2State *s = bs->opaque;
-uint64_t refblock_offs = get_refblock_offset(s, discard_block_offs);
+int64_t refblock_offs;
 uint64_t cluster_index = discard_block_offs >> s->cluster_bits;
 uint32_t block_index = cluster_index & (s->refcount_block_size - 1);
 void *refblock;
 int ret;
 
+refblock_offs = get_refblock_offset(bs, discard_block_offs);
+if (refblock_offs < 0) {
+return refblock_offs;
+}
+
 assert(discard_block_offs != 0);
 
 ret = qcow2_cache_get(bs, s->refcount_block_cache, refblock_offs,
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 44141f6243..c230696b3a 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -359,6 +359,52 @@ echo '--- Repairing ---'
 _check_test_img -q -r all
 _check_test_img -r all
 
+echo
+echo "=== Discarding an out-of-bounds refblock ==="
+echo
+
+_make_test_img 64M
+
+# Pretend there's a refblock really up high
+poke_file "$TEST_IMG" "$(($rt_offset+8))" "\x00\xff\xff\xff\x00\x00\x00\x00"
+# Let's try to shrink the qcow2 image so that the block driver tries
+# to discard that refblock (and see what happens!)
+$QEMU_IMG resize --shrink "$TEST_IMG" 32M
+
+echo '--- Checking and retrying ---'
+# Image should not be resized
+_img_info | grep 'virtual size'
+# But it should pass this check, because the "partial" resize has
+# already overwritten refblocks past the end
+_check_test_img -r all
+# So let's try again
+$QEMU_IMG resize --shrink "$TEST_IMG" 32M
+_img_info | grep 'virtual size'
+
+echo
+echo "=== Discarding a non-covered in-bounds refblock ==="
+echo
+
+IMGOPTS='refcount_bits=1' _make_test_img 64M
+
+# Pretend there's a refblock somewhere where there is no refblock to
+# cover it (but the covering refblock has a valid index in the
+# reftable)
+# Every refblock covers 65536 * 8 * 65536 = 32 GB, so we have to point
+# to 0x10__ (64G) to point to the third refblock
+poke_file "$TEST_IMG" "$(($rt_offset+8))" "\x00\x00\x00\x10\x00\x00\x00\x00"
+$QEMU_IMG resize --shrink "$TEST_IMG" 32M
+
+echo '--- Checking and retrying ---'
+# Image should not be resized
+_img_info | grep 'virtual size'
+# But it should pass this check, because the "partial" resize has
+# already overwritten refblocks past the end
+_check_test_img -r all
+# So let's try again
+$QEMU_IMG resize --shrink "$TEST_IMG" 32M
+_img_info | grep 'virtual size'
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 07dfdcac99..358e54cdc9 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -348,4 +348,26 @@ The following inconsistencies were found and repaired:
 
 Double checking the fixed image now...
 No errors were found on the image.
+
+=== Discarding an out-of-bounds refblock ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qcow2: Marking image as 

[Qemu-devel] [PATCH for-2.11 1/5] qcow2: check_errors are fatal

[Max Reitz]

When trying to repair a dirty image, qcow2_check() may apparently
succeed (no really fatal error occurred that would prevent the check
from continuing), but if check_errors in the result object is non-zero,
we cannot trust the image to be usable.

Reported-by: R. Nageswara Sastry 
Buglink: https://bugs.launchpad.net/qemu/+bug/1728639
Signed-off-by: Max Reitz 
---
 block/qcow2.c  |  5 -
 tests/qemu-iotests/060 | 20 
 tests/qemu-iotests/060.out | 23 +++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 92e5d548e3..d4fcb0250d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1475,7 +1475,10 @@ static int qcow2_do_open(BlockDriverState *bs, QDict 
*options, int flags,
 BdrvCheckResult result = {0};
 
 ret = qcow2_check(bs, , BDRV_FIX_ERRORS | BDRV_FIX_LEAKS);
-if (ret < 0) {
+if (ret < 0 || result.check_errors) {
+if (ret >= 0) {
+ret = -EIO;
+}
 error_setg_errno(errp, -ret, "Could not repair dirty image");
 goto fail;
 }
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index fae08b03bf..56bdf1ee2e 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -301,6 +301,26 @@ _make_test_img 64M
 poke_file "$TEST_IMG" "48""\x00\x00\x00\x00\x00\x00\x00\x00"
 $QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
 
+echo
+echo "=== Testing dirty corrupt image ==="
+echo
+
+_make_test_img 64M
+
+# Let the refblock appear unaligned
+poke_file "$TEST_IMG" "$rt_offset""\x00\x00\x00\x00\xff\xff\x2a\x00"
+# Mark the image dirty, thus forcing an automatic check when opening it
+poke_file "$TEST_IMG" 72 "\x00\x00\x00\x00\x00\x00\x00\x01"
+# Open the image (qemu should refuse to do so)
+$QEMU_IO -c close "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
+
+echo '--- Repairing ---'
+
+# The actual repair should have happened (because of the dirty bit),
+# but some cleanup may have failed (like freeing the old reftable)
+# because the image was already marked corrupt by that point
+_check_test_img -r all
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 62c22701b8..f013fe73c0 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -284,4 +284,27 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table at 
offset 0; further corruption events will be suppressed
 write failed: Input/output error
+
+=== Testing dirty corrupt image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+ERROR refcount block 0 is not cluster aligned; refcount table entry corrupted
+IMGFMT: Marking image as corrupt: Refblock offset 0x2a00 unaligned 
(reftable index: 0); further corruption events will be suppressed
+Can't get refcount for cluster 0: Input/output error
+Can't get refcount for cluster 1: Input/output error
+Can't get refcount for cluster 2: Input/output error
+Can't get refcount for cluster 3: Input/output error
+Rebuilding refcount structure
+Repairing cluster 1 refcount=1 reference=0
+can't open device TEST_DIR/t.IMGFMT: Could not repair dirty image: 
Input/output error
+--- Repairing ---
+Leaked cluster 1 refcount=1 reference=0
+Repairing cluster 1 refcount=1 reference=0
+The following inconsistencies were found and repaired:
+
+1 leaked clusters
+0 corruptions
+
+Double checking the fixed image now...
+No errors were found on the image.
 *** done
-- 
2.13.6

[Qemu-devel] [PATCH for-2.11 0/5] qcow2: Fixes for corrupted images

[Max Reitz]

This series contains fixes for another batch of qcow2-related crashes
reported on Launchpad by Nageswara (the first batch was
http://lists.nongnu.org/archive/html/qemu-block/2017-11/msg00082.html by
Berto).

Patch 4 fixes an out-of-bounds array access in memory which is not
really a security issue for multiple reasons (really, at most you can
read eight bytes from somewhere with an extremely high chance of
crashing qemu and requiring the user to invoke a block_resize shrinking
the qcow2 image (and also reset some bit in the image from 1 to 0, but
only if the overlap checks don't catch you)), but most importantly that
code hasn't been in 2.10, so we're fine.


Max Reitz (5):
  qcow2: check_errors are fatal
  qcow2: Unaligned zero cluster in handle_alloc()
  block: Guard against NULL bs->drv
  qcow2: Add bounds check to get_refblock_offset()
  qcow2: Refuse to get unaligned offsets from cache

 block/qcow2.h  |   6 ---
 block.c|  19 ++-
 block/io.c |  36 +
 block/qapi.c   |   8 ++-
 block/qcow2-cache.c|  21 
 block/qcow2-cluster.c  |  13 -
 block/qcow2-refcount.c |  26 +-
 block/qcow2.c  |   5 +-
 block/replication.c|  15 ++
 block/vvfat.c  |   2 +-
 tests/qemu-iotests/060 | 125 +
 tests/qemu-iotests/060.out | 115 +
 12 files changed, 379 insertions(+), 12 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

[Max Reitz]

Instead of using an assertion, it is better to emit a corruption event
here.  Checking all offsets for correct alignment can be tedious and it
is easily possible to forget to do so.  qcow2_cache_do_get() is a
function every L2 and refblock access has to go through, so this is a
good central point to add such a check.

And for good measure, let us also add an assertion that the offset is
non-zero.  Making this a corruption event is not feasible, because a
zero offset usually means something special (such as the cluster is
unused), so all callers should be checking this anyway.  If they do not,
it is their fault, hence the assertion here.

Signed-off-by: Max Reitz 
---
 block/qcow2-cache.c| 21 +
 tests/qemu-iotests/060 | 21 +
 tests/qemu-iotests/060.out | 29 +
 3 files changed, 71 insertions(+)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 75746a7f43..a5baaba0ff 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -62,6 +62,18 @@ static inline int qcow2_cache_get_table_idx(BlockDriverState 
*bs,
 return idx;
 }
 
+static inline const char *qcow2_cache_get_name(BDRVQcow2State *s, Qcow2Cache 
*c)
+{
+if (c == s->refcount_block_cache) {
+return "refcount block";
+} else if (c == s->l2_table_cache) {
+return "L2 table";
+} else {
+/* Do not abort, because this is not critical */
+return "unknown";
+}
+}
+
 static void qcow2_cache_table_release(BlockDriverState *bs, Qcow2Cache *c,
   int i, int num_tables)
 {
@@ -314,9 +326,18 @@ static int qcow2_cache_do_get(BlockDriverState *bs, 
Qcow2Cache *c,
 uint64_t min_lru_counter = UINT64_MAX;
 int min_lru_index = -1;
 
+assert(offset != 0);
+
 trace_qcow2_cache_get(qemu_coroutine_self(), c == s->l2_table_cache,
   offset, read_from_disk);
 
+if (offset_into_cluster(s, offset)) {
+qcow2_signal_corruption(bs, true, -1, -1, "Cannot get entry from %s "
+"cache: Offset %#" PRIx64 " is unaligned",
+qcow2_cache_get_name(s, c), offset);
+return -EIO;
+}
+
 /* Check if the table is already cached */
 i = lookup_index = (offset / s->cluster_size * 4) % c->size;
 do {
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index c230696b3a..1eca09417b 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -405,6 +405,27 @@ _check_test_img -r all
 $QEMU_IMG resize --shrink "$TEST_IMG" 32M
 _img_info | grep 'virtual size'
 
+echo
+echo "=== Discarding a refblock covered by an unaligned refblock ==="
+echo
+
+IMGOPTS='refcount_bits=1' _make_test_img 64M
+
+# Same as above
+poke_file "$TEST_IMG" "$(($rt_offset+8))" "\x00\x00\x00\x10\x00\x00\x00\x00"
+# But now we actually "create" an unaligned third refblock
+poke_file "$TEST_IMG" "$(($rt_offset+16))" "\x00\x00\x00\x00\x00\x00\x02\x00"
+$QEMU_IMG resize --shrink "$TEST_IMG" 32M
+
+echo '--- Repairing ---'
+# Fails the first repair because the corruption prevents the check
+# function from double-checking
+# (Using -q for the first invocation, because otherwise the
+#  double-check error message appears above the summary for some
+#  reason -- so let's just hide the summary)
+_check_test_img -q -r all
+_check_test_img -r all
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 358e54cdc9..56f5eb15d8 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -370,4 +370,33 @@ virtual size: 64M (67108864 bytes)
 No errors were found on the image.
 Image resized.
 virtual size: 32M (33554432 bytes)
+
+=== Discarding a refblock covered by an unaligned refblock ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qcow2: Marking image as corrupt: Cannot get entry from refcount block cache: 
Offset 0x200 is unaligned; further corruption events will be suppressed
+qemu-img: Failed to discard unused refblocks: Input/output error
+--- Repairing ---
+Repairing refcount block 1 is outside image
+ERROR refcount block 2 is not cluster aligned; refcount table entry corrupted
+qcow2: Marking image as corrupt: Refblock offset 0x200 unaligned (reftable 
index: 0x2); further corruption events will be suppressed
+Can't get refcount for cluster 1048576: Input/output error
+Rebuilding refcount structure
+Repairing cluster 1 refcount=1 reference=0
+Repairing cluster 2 refcount=1 reference=0
+Repairing cluster 1048576 refcount=1 reference=0
+qemu-img: Check failed: No medium found
+Leaked cluster 1 refcount=1 reference=0
+Leaked cluster 2 refcount=1 reference=0
+Leaked cluster 1048576 refcount=1 reference=0
+Repairing cluster 1 refcount=1 reference=0
+Repairing cluster 2 refcount=1 reference=0
+Repairing cluster 1048576 refcount=1 reference=0
+The following inconsistencies were found and 

Re: [Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

[Max Reitz]

On 2017-11-10 19:54, Alberto Garcia wrote:
> If a BlockBackend has I/O limits set then its ThrottleGroupMember
> structure uses the AioContext from its attached BlockDriverState.
> Those two contexts must be kept in sync manually. This is not
> ideal and will be fixed in the future by removing the throttling
> configuration from the BlockBackend and storing it in an implicit
> filter node instead, but for now we have to live with this.
> 
> When you remove the BlockDriverState from the backend then the
> throttle timers are destroyed. If a new BlockDriverState is later
> inserted then they are created again using the new AioContext.
> 
> There'a a couple of problems with this:
> 
>a) The code manipulates the timers directly, leaving the
>   ThrottleGroupMember.aio_context field in an inconsisent state.
> 
>b) If you remove the I/O limits (e.g by destroying the backend)
>   when the timers are gone then throttle_group_unregister_tgm()
>   will attempt to destroy them again, crashing QEMU.
> 
> While b) could be fixed easily by allowing the timers to be freed
> twice, this would result in a situation in which we can no longer
> guarantee that a valid ThrottleState has a valid AioContext and
> timers.
> 
> This patch ensures that the timers and AioContext are always valid
> when I/O limits are set, regardless of whether the BlockBackend has a
> BlockDriverState inserted or not.
> 
> Reported-by: sochin jiang 
> Signed-off-by: Alberto Garcia 
> ---
>  block/block-backend.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 1/1] Add 8-byte access, interleaving to AMD CFI devices

[Mike Nawrocki]

This adds 8-byte wide access support to AMD CFI flash devices.
Additionally, it migrates the MMIO operations from old_mmio to the new
API. Finally, it mirrors the interleaving support already in place in
pflash_cfi01.c, using the max_device_width and device_width properties.

Signed-off-by: Mike Nawrocki 
---
 hw/block/pflash_cfi02.c | 491 +---
 1 file changed, 337 insertions(+), 154 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index c81ddd3a99..1571148f14 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -18,7 +18,7 @@
  */
 
 /*
- * For now, this code can emulate flashes of 1, 2 or 4 bytes width.
+ * For now, this code can emulate flashes of 1, 2, 4, or 8 bytes width.
  * Supported commands/modes are:
  * - flash read
  * - flash write
@@ -28,11 +28,14 @@
  * - unlock bypass command
  * - CFI queries
  *
- * It does not support flash interleaving.
  * It does not implement boot blocs with reduced size
  * It does not implement software data protection as found in many real chips
  * It does not implement erase suspend/resume commands
  * It does not implement multiple sectors erase
+ *
+ * Flash interleaving is partially supported using the device_width and
+ * max_device_width properties, as in pflash_cfi01.c
+ * Issuing commands to individual members of the flash array is not supported.
  */
 
 #include "qemu/osdep.h"
@@ -40,6 +43,7 @@
 #include "hw/block/flash.h"
 #include "qapi/error.h"
 #include "qemu/timer.h"
+#include "qemu/bitops.h"
 #include "sysemu/block-backend.h"
 #include "exec/address-spaces.h"
 #include "qemu/host-utils.h"
@@ -69,7 +73,9 @@ struct pflash_t {
 uint32_t nb_blocs;
 uint32_t chip_len;
 uint8_t mappings;
-uint8_t width;
+uint8_t bank_width;
+uint8_t device_width; /* If 0, device width not specified. */
+uint8_t max_device_width;  /* max device width in bytes */
 uint8_t be;
 int wcycle; /* if 0, the flash is read normally */
 int bypass;
@@ -138,12 +144,190 @@ static void pflash_timer (void *opaque)
 pfl->cmd = 0;
 }
 
-static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
- int width, int be)
+static uint64_t pflash_cfi_query(pflash_t *pfl, hwaddr offset)
 {
+int i;
+uint64_t resp = 0;
 hwaddr boff;
-uint32_t ret;
-uint8_t *p;
+
+/* Adjust incoming offset to match expected device-width
+ * addressing. CFI query addresses are always specified in terms of
+ * the maximum supported width of the device.  This means that x8
+ * devices and x8/x16 devices in x8 mode behave differently.  For
+ * devices that are not used at their max width, we will be
+ * provided with addresses that use higher address bits than
+ * expected (based on the max width), so we will shift them lower
+ * so that they will match the addresses used when
+ * device_width==max_device_width.
+ */
+boff = offset >> (ctz32(pfl->bank_width) +
+  ctz32(pfl->max_device_width) - ctz32(pfl->device_width));
+
+if (boff > pfl->cfi_len) {
+return 0;
+}
+/* Now we will construct the CFI response generated by a single
+ * device, then replicate that for all devices that make up the
+ * bus.  For wide parts used in x8 mode, CFI query responses
+ * are different than native byte-wide parts.
+ */
+resp = pfl->cfi_table[boff];
+if (pfl->device_width != pfl->max_device_width) {
+/* The only case currently supported is x8 mode for a
+ * wider part.
+ */
+if (pfl->device_width != 1 || pfl->bank_width > 8) {
+DPRINTF("%s: Unsupported device configuration: "
+"device_width=%d, max_device_width=%d\n",
+__func__, pfl->device_width,
+pfl->max_device_width);
+return 0;
+}
+/* CFI query data is repeated, rather than zero padded for
+ * wide devices used in x8 mode.
+ */
+for (i = 1; i < pfl->max_device_width; i++) {
+resp = deposit64(resp, 8 * i, 8, pfl->cfi_table[boff]);
+}
+}
+/* Replicate responses for each device in bank. */
+if (pfl->device_width < pfl->bank_width) {
+for (i = pfl->device_width;
+ i < pfl->bank_width; i += pfl->device_width) {
+resp = deposit64(resp, 8 * i, 8 * pfl->device_width, resp);
+}
+}
+
+return resp;
+}
+
+
+
+static uint64_t _flash_read(pflash_t *pfl, hwaddr offset,
+int width, int be)
+{
+/* Flash area read */
+uint64_t ret = 0;
+uint8_t *p = pfl->storage;
+
+switch (width) {
+case 1:
+ret = p[offset];
+/* DPRINTF("%s: data offset %08x %02x\n", __func__, offset, ret); */
+break;
+case 2:
+if (be) {
+ret = p[offset] << 8;
+ret |= p[offset 

[Qemu-devel] [PATCH v2 0/1] Add 8-byte wide AMD flash support, partial interleaving

[Mike Nawrocki]

This patch set does a few things. First, it switches the AMD CFI flash MMIO
operations from the old MMIO API to the new one. Second, it enables 8-byte wide
flash arrays. Finally, it adds flash interleaving using the "device-width" and
"max-device-width" properties, using the same interface as pflash_cfi01.c. Much
of the code was taken and adapted from that file.

Version 1 of the patch set changed the flash register function (and all
usages), version 2 localizes changes to the pflash_cfi02.c file.

Mike Nawrocki (1):
  Add 8-byte access, interleaving to AMD CFI devices

 hw/block/pflash_cfi02.c | 491 +---
 1 file changed, 337 insertions(+), 154 deletions(-)

-- 
2.14.2

Re: [Qemu-devel] [PATCH for 2.11 1/5] qom: move CPUClass.tcg_initialize to a global

[Eduardo Habkost]

On Fri, Nov 10, 2017 at 02:53:42PM -0500, Emilio G. Cota wrote:
> 55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
> introduces a per-CPUClass bool that we check so that the target CPU
> is initialized for TCG only once. This works well except when
> we end up creating more than one CPUClass, in which case we end
> up incorrectly initializing TCG more than once, i.e. once for
> each CPUClass.
> 
> This can be replicated with:
>   $ aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -smp 6 \
>   -global driver=xlnx,,zynqmp,property=has_rpu,value=on
> In this case the class name of the "RPUs" is prefixed by "cortex-r5-",
> whereas the "regular" CPUs are prefixed by "cortex-a53-". This
> results in two CPUClass instances being created.
> 
> Fix it by introducing a static variable, so that only the first
> target CPU being initialized will initialize the target-dependent
> part of TCG, regardless of CPUClass instances.
> 
> Fixes: 55c3ceef61fcf06fc98ddc752b7cce788ce7680b
> Signed-off-by: Emilio G. Cota 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/3] block: Check for inserted BlockDriverState in blk_io_limits_disable()

[Max Reitz]

On 2017-11-10 19:54, Alberto Garcia wrote:
> When you set I/O limits using block_set_io_throttle or the command
> line throttling.* options they are kept in the BlockBackend regardless
> of whether a BlockDriverState is attached to the backend or not.
> 
> Therefore when removing the limits using blk_io_limits_disable() we
> need to check if there's a BDS before attempting to drain it, else it
> will crash QEMU. This can be reproduced very easily using HMP:
> 
>  (qemu) drive_add 0 if=none,throttling.iops-total=5000
>  (qemu) drive_del none0
> 
> Reported-by: sochin jiang 
> Signed-off-by: Alberto Garcia 
> ---
>  block/block-backend.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/3] ivshmem: Don't update non-existent MSI routes

[geoff--- via Qemu-devel]

Thanks Ladi, I had not yet had time to dig into these, this patch set 
resolves all issues I was aware of.


Tested-by: Geoffrey McRae 

On 2017-11-11 04:34, Ladi Prosek wrote:
As of commit 660c97eef6f8 ("ivshmem: use kvm irqfd for msi 
notifications"),

QEMU crashes with:

  kvm_irqchip_commit_routes: Assertion `ret == 0' failed.

if the ivshmem device is configured with more vectors than what the 
server

supports. This is caused by the ivshmem_vector_unmask() being called on
vectors that have not been initialized by ivshmem_add_kvm_msi_virq().

This commit fixes it by adding a simple check to the mask and unmask
callbacks.

Note that the opposite mismatch, if the server supplies more vectors 
than
what the device is configured for, is already handled and leads to 
output

like:

  Too many eventfd received, device has 1 vectors

Fixes: 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications")
Signed-off-by: Ladi Prosek 
---
 hw/misc/ivshmem.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index a5a46827fe..6e46669744 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -317,6 +317,10 @@ static int ivshmem_vector_unmask(PCIDevice *dev,
unsigned vector,
 int ret;

 IVSHMEM_DPRINTF("vector unmask %p %d\n", dev, vector);
+if (!v->pdev) {
+error_report("ivshmem: vector %d route does not exist", 
vector);

+return -EINVAL;
+}

 ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg, dev);
 if (ret < 0) {
@@ -331,12 +335,16 @@ static void ivshmem_vector_mask(PCIDevice *dev,
unsigned vector)
 {
 IVShmemState *s = IVSHMEM_COMMON(dev);
 EventNotifier *n = >peers[s->vm_id].eventfds[vector];
+MSIVector *v = >msi_vectors[vector];
 int ret;

 IVSHMEM_DPRINTF("vector mask %p %d\n", dev, vector);
+if (!v->pdev) {
+error_report("ivshmem: vector %d route does not exist", 
vector);

+return;
+}

-ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n,
-
s->msi_vectors[vector].virq);
+ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, 
v->virq);

 if (ret != 0) {
 error_report("remove_irqfd_notifier_gsi failed");
 }

Re: [Qemu-devel] [PATCH] iotests: test clearing unknown autoclear_features by qcow2

[Kevin Wolf]

Am 10.11.2017 um 18:54 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Test clearing unknown autoclear_features by qcow2 on incoming
> migration.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> Hi all!
> 
> This patch shows degradation, added in 2.10 in commit
> 
> commit 9c5e6594f15b7364624a3ad40306c396c93a2145
> Author: Kevin Wolf 
> Date:   Thu May 4 18:52:40 2017 +0200
> 
> block: Fix write/resize permissions for inactive images
> 
> The problem:
> When on incoming migration with shared storage we reopen image to RW mode
> we call bdrv_invalidate_cache it firstly call drv->bdrv_invalidate_cache and
> only after it, through "parent->role->activate(parent, _err);" we set
> appropriate RW permission.
> 
> Because of this, if drv->bdrv_invalidate_cache wants to write something
> (image is RW and BDRV_O_INACTIVE is not set) it goes into
> "bdrv_aligned_pwritev: Assertion `child->perm & BLK_PERM_WRITE' failed"
> 
> One case, when qcow2_invalidate_cache wants to write
> something - when it wants to clear some unknown autoclear features. So,
> here is a test for it.
> 
> Another case is when we try to migrate persistent dirty bitmaps through
> shared storage, on invalidate_cache qcow2 will try to set DIRTY bit in
> all loaded bitmaps.
> 
> Kevin, what do you think? I understand that operation order should be changed
> somehow in bdrv_invalidate_cache, but I'm not sure about how 
> "parent->role->.."
> things works and can we safely move this part from function end to the middle.

I don't think you need to move the parent->role->activate() callback,
but just the bdrv_set_perm() so that we request the correct permissions
for operation without the BDRV_O_INACTIVE flag.

The following seems to work for me (including a fix for the test case,
we don't seem to get a RESUME event). I'm not sure about the error paths
yet. We should probably try do undo the permission changes there. I can
have a closer look into this next week.

Kevin


diff --git a/block.c b/block.c
index edc5bb9f9b..f530b630b6 100644
--- a/block.c
+++ b/block.c
@@ -4178,7 +4178,17 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+/* Update permissions, they may differ for inactive nodes */
 bs->open_flags &= ~BDRV_O_INACTIVE;
+bdrv_get_cumulative_perm(bs, , _perm);
+ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, _err);
+if (ret < 0) {
+bs->open_flags |= BDRV_O_INACTIVE;
+error_propagate(errp, local_err);
+return;
+}
+bdrv_set_perm(bs, perm, shared_perm);
+
 if (bs->drv->bdrv_invalidate_cache) {
 bs->drv->bdrv_invalidate_cache(bs, _err);
 if (local_err) {
@@ -4195,16 +4205,6 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error 
**errp)
 return;
 }
 
-/* Update permissions, they may differ for inactive nodes */
-bdrv_get_cumulative_perm(bs, , _perm);
-ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, _err);
-if (ret < 0) {
-bs->open_flags |= BDRV_O_INACTIVE;
-error_propagate(errp, local_err);
-return;
-}
-bdrv_set_perm(bs, perm, shared_perm);
-
 QLIST_FOREACH(parent, >parents, next_parent) {
 if (parent->role->activate) {
 parent->role->activate(parent, _err);
diff --git a/tests/qemu-iotests/196 b/tests/qemu-iotests/196
index 9ffbc723c2..4116ebc92b 100755
--- a/tests/qemu-iotests/196
+++ b/tests/qemu-iotests/196
@@ -53,7 +53,10 @@ class TestInvalidateAutoclear(iotests.QMPTestCase):
 f.write(b'\xff')
 
 self.vm_b.launch()
-self.assertNotEqual(self.vm_b.event_wait("RESUME"), None)
+while True:
+result = self.vm_b.qmp('query-status')
+if result['return']['status'] == 'running':
+break
 
 with open(disk, 'rb') as f:
 f.seek(88)

[Qemu-devel] [PATCH for 2.11 3/5] xlnx-zcu102: Add an info message deprecating the EP108

[Emilio G. Cota]

From: Alistair Francis 

The EP108 was an early access development board that is no longer used.
Add an info message to convert any users to the ZCU102 instead. On QEMU
they are both identical.

This patch also updated the qemu-doc.texi file to indicate that the
EP108 has been deprecated.

Signed-off-by: Alistair Francis 
Reviewed-by: Emilio G. Cota 
---
 qemu-doc.texi| 7 +++
 hw/arm/xlnx-zcu102.c | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 8c10956..d383ac4 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2537,6 +2537,13 @@ or ``ivshmem-doorbell`` device types.
 The ``spapr-pci-vfio-host-bridge'' device type is replaced by
 the ``spapr-pci-host-bridge'' device type.
 
+@section System emulator machines
+
+@subsection Xilinx EP108 (since 2.11.0)
+
+The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
+The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
+
 @node License
 @appendix License
 
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 7ec03da..a23 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -164,6 +164,9 @@ static void xlnx_ep108_init(MachineState *machine)
 {
 XlnxZCU102 *s = EP108_MACHINE(machine);
 
+info_report("The Xilinx EP108 machine is deprecated, please use the "
+"ZCU102 machine instead. It has the same features supported.");
+
 xlnx_zynqmp_init(s, machine);
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH for 2.11 1/5] qom: move CPUClass.tcg_initialize to a global

[Emilio G. Cota]

55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
introduces a per-CPUClass bool that we check so that the target CPU
is initialized for TCG only once. This works well except when
we end up creating more than one CPUClass, in which case we end
up incorrectly initializing TCG more than once, i.e. once for
each CPUClass.

This can be replicated with:
  $ aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -smp 6 \
  -global driver=xlnx,,zynqmp,property=has_rpu,value=on
In this case the class name of the "RPUs" is prefixed by "cortex-r5-",
whereas the "regular" CPUs are prefixed by "cortex-a53-". This
results in two CPUClass instances being created.

Fix it by introducing a static variable, so that only the first
target CPU being initialized will initialize the target-dependent
part of TCG, regardless of CPUClass instances.

Fixes: 55c3ceef61fcf06fc98ddc752b7cce788ce7680b
Signed-off-by: Emilio G. Cota 
---
 include/qom/cpu.h | 1 -
 exec.c| 5 +++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index fa4b0c9..c2fa151 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -209,7 +209,6 @@ typedef struct CPUClass {
 /* Keep non-pointer data at the end to minimize holes.  */
 int gdb_num_core_regs;
 bool gdb_stop_before_watchpoint;
-bool tcg_initialized;
 } CPUClass;
 
 #ifdef HOST_WORDS_BIGENDIAN
diff --git a/exec.c b/exec.c
index 97a24a8..8b579c0 100644
--- a/exec.c
+++ b/exec.c
@@ -792,11 +792,12 @@ void cpu_exec_initfn(CPUState *cpu)
 void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
+static bool tcg_target_initialized;
 
 cpu_list_add(cpu);
 
-if (tcg_enabled() && !cc->tcg_initialized) {
-cc->tcg_initialized = true;
+if (tcg_enabled() && !tcg_target_initialized) {
+tcg_target_initialized = true;
 cc->tcg_initialize();
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH for 2.11 4/5] xlnx-zcu102: Specify the max number of CPUs for the EP108

[Emilio G. Cota]

Just like the zcu102, the ep108 can instantiate several CPUs.

Signed-off-by: Emilio G. Cota 
---
 hw/arm/xlnx-zcu102.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index a23..190eb69 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -188,6 +188,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, 
void *data)
 mc->block_default_type = IF_IDE;
 mc->units_per_default_bus = 1;
 mc->ignore_memory_transaction_failures = true;
+mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
 }
 
 static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
-- 
2.7.4

[Qemu-devel] [PATCH for 2.11 0/5] TCG/ARM fixes for 2.11

[Emilio G. Cota]

Some MachineClass changes to fix TCG initialization of some
ARM boards for 2.11. This was originally reported by Thomas Huth in [1],
where Peter suggested a way to fix it. Further discussion in
another thread [2] followed up on this.

As a result of that follow-up discussion we also got some Zynq changes from
Alistair [3], which I'm including here since in order to test them
we need the first patch in this series.

Thanks,

Emilio

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg00078.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg00502.html
[3] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01842.html

Alistair Francis (2):
  xlnx-zynqmp: Properly support the smp command line option
  xlnx-zcu102: Add an info message deprecating the EP108

Emilio G. Cota (3):
  qom: move CPUClass.tcg_initialize to a global
  xlnx-zcu102: Specify the max number of CPUs for the EP108
  hw: add .min_cpus and .default_cpus fields to machine_class

 qemu-doc.texi   |  7 +++
 include/hw/boards.h |  5 +
 include/qom/cpu.h   |  1 -
 exec.c  |  5 +++--
 hw/arm/exynos4_boards.c | 12 
 hw/arm/raspi.c  |  2 ++
 hw/arm/xlnx-zcu102.c|  9 -
 hw/arm/xlnx-zynqmp.c| 26 --
 vl.c| 21 ++---
 9 files changed, 63 insertions(+), 25 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH for 2.11 5/5] hw: add .min_cpus and .default_cpus fields to machine_class

[Emilio G. Cota]

max_cpus needs to be an upper bound on the number of vCPUs
initialized; otherwise TCG region initialization breaks.

Some boards initialize a hard-coded number of vCPUs, which is not
captured by the global max_cpus and therefore breaks TCG initialization.
Fix it by adding the .min_cpus field to machine_class.

This commit also changes some user-facing behaviour: we now die if
-smp is below this hard-coded vCPU minimum instead of silently
ignoring the passed -smp value (sometimes announcing this by printing
a warning). However, the introduction of .default_cpus lessens the
likelihood that users will notice this: if -smp isn't set, we now
assign the value in .default_cpus to both smp_cpus and max_cpus. IOW,
if a user does not set -smp, they always get a correct number of vCPUs.

This change fixes 3468b59 ("tcg: enable multiple TCG contexts in
softmmu", 2017-10-24), which broke TCG initialization for some
ARM boards.

Fixes: 3468b59e18b179bc63c7ce934de912dfa9596122
Reported-by: Thomas Huth 
Suggested-by: Peter Maydell 
Signed-off-by: Emilio G. Cota 
---
 include/hw/boards.h |  5 +
 hw/arm/exynos4_boards.c | 12 
 hw/arm/raspi.c  |  2 ++
 hw/arm/xlnx-zcu102.c|  2 ++
 vl.c| 21 ++---
 5 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 191a5b3..62f160e 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -102,6 +102,9 @@ typedef struct {
 
 /**
  * MachineClass:
+ * @max_cpus: maximum number of CPUs supported. Default: 1
+ * @min_cpus: minimum number of CPUs supported. Default: 1
+ * @default_cpus: number of CPUs instantiated if none are specified. Default: 1
  * @get_hotplug_handler: this function is called during bus-less
  *device hotplug. If defined it returns pointer to an instance
  *of HotplugHandler object, which handles hotplug operation
@@ -167,6 +170,8 @@ struct MachineClass {
 BlockInterfaceType block_default_type;
 int units_per_default_bus;
 int max_cpus;
+int min_cpus;
+int default_cpus;
 unsigned int no_serial:1,
 no_parallel:1,
 use_virtcon:1,
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index f1441ec..750162c 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -27,7 +27,6 @@
 #include "qemu-common.h"
 #include "cpu.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/qtest.h"
 #include "hw/sysbus.h"
 #include "net/net.h"
 #include "hw/arm/arm.h"
@@ -129,13 +128,6 @@ exynos4_boards_init_common(MachineState *machine,
Exynos4BoardType board_type)
 {
 Exynos4BoardState *s = g_new(Exynos4BoardState, 1);
-MachineClass *mc = MACHINE_GET_CLASS(machine);
-
-if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
-error_report("%s board supports only %d CPU cores, ignoring smp_cpus"
- " value",
- mc->name, EXYNOS4210_NCPUS);
-}
 
 exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type];
 exynos4_board_binfo.board_id = exynos4_board_id[board_type];
@@ -189,6 +181,8 @@ static void nuri_class_init(ObjectClass *oc, void *data)
 mc->desc = "Samsung NURI board (Exynos4210)";
 mc->init = nuri_init;
 mc->max_cpus = EXYNOS4210_NCPUS;
+mc->min_cpus = EXYNOS4210_NCPUS;
+mc->default_cpus = EXYNOS4210_NCPUS;
 mc->ignore_memory_transaction_failures = true;
 }
 
@@ -205,6 +199,8 @@ static void smdkc210_class_init(ObjectClass *oc, void *data)
 mc->desc = "Samsung SMDKC210 board (Exynos4210)";
 mc->init = smdkc210_init;
 mc->max_cpus = EXYNOS4210_NCPUS;
+mc->min_cpus = EXYNOS4210_NCPUS;
+mc->default_cpus = EXYNOS4210_NCPUS;
 mc->ignore_memory_transaction_failures = true;
 }
 
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 5941c9f..cd5fa8c 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -167,6 +167,8 @@ static void raspi2_machine_init(MachineClass *mc)
 mc->no_floppy = 1;
 mc->no_cdrom = 1;
 mc->max_cpus = BCM2836_NCPUS;
+mc->min_cpus = BCM2836_NCPUS;
+mc->default_cpus = BCM2836_NCPUS;
 mc->default_ram_size = 1024 * 1024 * 1024;
 mc->ignore_memory_transaction_failures = true;
 };
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 190eb69..9631a53 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -189,6 +189,7 @@ static void xlnx_ep108_machine_class_init(ObjectClass *oc, 
void *data)
 mc->units_per_default_bus = 1;
 mc->ignore_memory_transaction_failures = true;
 mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
+mc->default_cpus = XLNX_ZYNQMP_NUM_APU_CPUS;
 }
 
 static const TypeInfo xlnx_ep108_machine_init_typeinfo = {
@@ -246,6 +247,7 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, 
void *data)
 mc->units_per_default_bus = 1;
 mc->ignore_memory_transaction_failures = true;
   

[Qemu-devel] [PATCH for 2.11 2/5] xlnx-zynqmp: Properly support the smp command line option

[Emilio G. Cota]

From: Alistair Francis 

Allow the -smp command line option to control the number of CPUs we
create.

Signed-off-by: Alistair Francis 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Emilio G. Cota 
Tested-by: Emilio G. Cota 
---
 hw/arm/xlnx-zcu102.c |  3 ++-
 hw/arm/xlnx-zynqmp.c | 26 --
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index e2d15a1..7ec03da 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -235,7 +235,8 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc, 
void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 
-mc->desc = "Xilinx ZynqMP ZCU102 board";
+mc->desc = "Xilinx ZynqMP ZCU102 board with 4xA53s and 2xR5s based on " \
+   "the value of smp";
 mc->init = xlnx_zcu102_init;
 mc->block_default_type = IF_IDE;
 mc->units_per_default_bus = 1;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index d4b6560..c707c66 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -98,8 +98,9 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const 
char *boot_cpu,
 {
 Error *err = NULL;
 int i;
+int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, 
XLNX_ZYNQMP_NUM_RPU_CPUS);
 
-for (i = 0; i < XLNX_ZYNQMP_NUM_RPU_CPUS; i++) {
+for (i = 0; i < num_rpus; i++) {
 char *name;
 
 object_initialize(>rpu_cpu[i], sizeof(s->rpu_cpu[i]),
@@ -132,8 +133,9 @@ static void xlnx_zynqmp_init(Object *obj)
 {
 XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
 int i;
+int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 
-for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+for (i = 0; i < num_apus; i++) {
 object_initialize(>apu_cpu[i], sizeof(s->apu_cpu[i]),
   "cortex-a53-" TYPE_ARM_CPU);
 object_property_add_child(obj, "apu-cpu[*]", OBJECT(>apu_cpu[i]),
@@ -182,6 +184,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 MemoryRegion *system_memory = get_system_memory();
 uint8_t i;
 uint64_t ram_size;
+int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
 ram_addr_t ddr_low_size, ddr_high_size;
 qemu_irq gic_spi[GIC_NUM_SPI_INTR];
@@ -233,10 +236,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 
 qdev_prop_set_uint32(DEVICE(>gic), "num-irq", GIC_NUM_SPI_INTR + 32);
 qdev_prop_set_uint32(DEVICE(>gic), "revision", 2);
-qdev_prop_set_uint32(DEVICE(>gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
+qdev_prop_set_uint32(DEVICE(>gic), "num-cpu", num_apus);
 
 /* Realize APUs before realizing the GIC. KVM requires this.  */
-for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+for (i = 0; i < num_apus; i++) {
 char *name;
 
 object_property_set_int(OBJECT(>apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
@@ -292,7 +295,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 }
 }
 
-for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+for (i = 0; i < num_apus; i++) {
 qemu_irq irq;
 
 sysbus_connect_irq(SYS_BUS_DEVICE(>gic), i,
@@ -307,11 +310,14 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 }
 
 if (s->has_rpu) {
-xlnx_zynqmp_create_rpu(s, boot_cpu, );
-if (err) {
-error_propagate(errp, err);
-return;
-}
+info_report("The 'has_rpu' property is no longer required, to use the "
+"RPUs just use -smp 6.");
+}
+
+xlnx_zynqmp_create_rpu(s, boot_cpu, );
+if (err) {
+error_propagate(errp, err);
+return;
 }
 
 if (!s->boot_cpu_ptr) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH] linux-user, s390x: ignore OS ABI value in ELF header

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH] linux-user, s390x: ignore OS ABI value in ELF 
header
Type: series
Message-id: 20171110194935.17541-1-laur...@vivier.eu

=== 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
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/20171110194935.17541-1-laur...@vivier.eu -> 
patchew/20171110194935.17541-1-laur...@vivier.eu
Switched to a new branch 'test'
07d00cea5e linux-user, s390x: ignore OS ABI value in ELF header

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user, s390x: ignore OS ABI value in ELF header...
ERROR: line over 90 characters
#39: FILE: scripts/qemu-binfmt-conf.sh:88:
+s390x_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'

total: 1 errors, 0 warnings, 8 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

[Qemu-devel] [PATCH] linux-user, s390x: ignore OS ABI value in ELF header

[Laurent Vivier]

I have this error:
bash: /sbin/ldconfig: cannot execute binary file: Exec format error

because /sbin/ldconfig is:
ELF 64-bit MSB executable, IBM S/390, version 1 (GNU/Linux),
statically linked, for GNU/Linux 3.2.0,
BuildID[sha1]=90b64604014aafac9c1a0623b1cf447281d1a382, stripped

OS ABI is GNU/linux

"/bin/ls" works well:

ELF 64-bit MSB shared object, IBM S/390, version 1 (SYSV), dynamically linked,
interpreter /lib/ld64.so.1, for GNU/Linux 3.2.0,
BuildID[sha1]=be9b19143d4657678846f6e5277383071fc1059a, stripped

OS ABI is SYSV

To be able to execute ldconfig, this patch modifies s390x binfmt mask
to ignore the OS ABI value (EI_OSABI, byte 7).

Signed-off-by: Laurent Vivier 
---
 scripts/qemu-binfmt-conf.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 8afc3eb5bb..e2e1b7544d 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -85,7 +85,7 @@ 
sh4eb_mask='\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff
 sh4eb_family=sh4
 
 
s390x_magic='\x7fELF\x02\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x16'
-s390x_mask='\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
+s390x_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
 s390x_family=s390x
 
 
aarch64_magic='\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\xb7\x00'
-- 
2.13.6

Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class

[Emilio G. Cota]

On Tue, Nov 07, 2017 at 18:15:45 -0200, Eduardo Habkost wrote:
> On Fri, Nov 03, 2017 at 02:47:33PM -0400, Emilio G. Cota wrote:
> > @@ -4330,12 +4330,34 @@ int main(int argc, char **argv, char **envp)
> >  smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
> >  
> >  machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to 
> > UP */
> > +machine_class->min_cpus = machine_class->min_cpus ?: 1;
> > +machine_class->default_cpus = machine_class->default_cpus ?: 1;
> > +
> > +/* if -smp is not set, default to mc->default_cpus */
> > +if (!smp_cpus) {
> > +smp_cpus = machine_class->default_cpus;
> > +max_cpus = machine_class->default_cpus;
> > +}
> 
> I suggest doing this before smp_parse(), so any validation of
> smp_cpus inside smp_parse will apply to the value we're setting
> here (e.g. the replay_add_blocker() call in smp_parse() will
> work).
(snip)
> > +if (max_cpus < machine_class->min_cpus) {
> 
> smp_parse() already ensures max_cpus >= smp_cpus, and you are
> already checking if smp_cpus < machine_class->min_cpus above.  Is
> it really possible to trigger this error message?
> 
> Except for that, the patch looks good to me.

Both very good points! I've modified the patch accordingly.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH V4] hw/pci-host: Fix x86 Host Bridges 64bit PCI hole

[Marcel Apfelbaum]

On 10/11/2017 11:26, Laszlo Ersek wrote:

Hi Marcel,

On 11/09/17 18:27, Marcel Apfelbaum wrote:

Currently there is no MMIO range over 4G
reserved for PCI hotplug. Since the 32bit PCI hole
depends on the number of cold-plugged PCI devices
and other factors, it is very possible is too small
to hotplug PCI devices with large BARs.

Fix it by reserving 2G for I4400FX chipset
in order to comply with older Win32 Guest OSes
and 32G for Q35 chipset.

Even if the new defaults of pci-hole64-size will appear in
"info qtree" also for older machines, the property was
not implemented so no changes will be visible to guests.

Note this is a regression since prev QEMU versions had
some range reserved for 64bit PCI hotplug.

Reviewed-by: Laszlo Ersek 
Reviewed-by: Gerd Hoffmann 
Signed-off-by: Marcel Apfelbaum 
---

V3 -> V4:
  - Addressed Laszlo's comments:
 - Added defines for pci-hole64 default size props.
 - Rounded the hole64_end to 1G
 - Moved some info to commit message


Looks good to me, but a new variable's name is a bit misleading:


  - Addressed Michael's comments:
 - Added more comments.
  - I kept Gerd's "review-by" tag since no functional changes were made.

V2 -> V3:
  - Addressed Gerd's and others comments and re-enabled the pci-hole64-size
property defaulting it to 2G for I440FX and 32g for Q35.
  - Even if the new defaults of pci-hole64-size will appear in "info qtree"
also for older machines, the property was not implemented so
no changes will be visible to guests.

V1 -> V2:
  Addressed Igor's comments:
 - aligned the hole64 start to 1Gb
  (I think all the computations took care of it already,
   but it can't hurt)
 - Init compat props to "off" instead of "false"

  hw/i386/pc.c  | 22 ++
  hw/pci-host/piix.c| 32 ++--
  hw/pci-host/q35.c | 35 ---
  include/hw/i386/pc.h  | 10 +-
  include/hw/pci-host/q35.h |  1 +
  5 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e11a65b545..fafe5ba5cd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1448,6 +1448,28 @@ void pc_memory_init(PCMachineState *pcms,
  pcms->ioapic_as = _space_memory;
  }
  
+/*

+ * The 64bit pci hole starts after "above 4G RAM" and
+ * potentially the space reserved for memory hotplug.
+ */
+uint64_t pc_pci_hole64_start(void)
+{
+PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+uint64_t hole64_start = 0;
+
+if (pcmc->has_reserved_memory && pcms->hotplug_memory.base) {
+hole64_start = pcms->hotplug_memory.base;
+if (!pcmc->broken_reserved_end) {
+hole64_start += memory_region_size(>hotplug_memory.mr);
+}
+} else {
+hole64_start = 0x1ULL + pcms->above_4g_mem_size;
+}
+
+return ROUND_UP(hole64_start, 1ULL << 30);
+}
+
  qemu_irq pc_allocate_cpu_irq(void)
  {
  return qemu_allocate_irq(pic_irq_request, NULL, 0);
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index a7e2256870..f689c31d12 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -50,6 +50,7 @@ typedef struct I440FXState {
  PCIHostState parent_obj;
  Range pci_hole;
  uint64_t pci_hole64_size;
+bool pci_hole64_fix;
  uint32_t short_root_bus;
  } I440FXState;
  
@@ -112,6 +113,9 @@ struct PCII440FXState {

  #define I440FX_PAM_SIZE 7
  #define I440FX_SMRAM0x72
  
+/* Keep it 2G to comply with older win32 guests */

+#define I440FX_PCI_HOST_HOLE64_SIZE_DEFAULT (1ULL << 31)
+
  /* Older coreboot versions (4.0 and older) read a config register that doesn't
   * exist in real hardware, to get the RAM size from QEMU.
   */
@@ -238,29 +242,52 @@ static void i440fx_pcihost_get_pci_hole_end(Object *obj, 
Visitor *v,
  visit_type_uint32(v, name, , errp);
  }
  
+/*

+ * The 64bit PCI hole start is set by the Guest firmware
+ * as the address of the first 64bit PCI MEM resource.
+ * If no PCI device has resources on the 64bit area,
+ * the 64bit PCI hole will start after "over 4G RAM" and the
+ * reserved space for memory hotplug if any.
+ */
  static void i440fx_pcihost_get_pci_hole64_start(Object *obj, Visitor *v,
  const char *name,
  void *opaque, Error **errp)
  {
  PCIHostState *h = PCI_HOST_BRIDGE(obj);
+I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
  Range w64;
  uint64_t value;
  
  pci_bus_get_w64_range(h->bus, );

  value = range_is_empty() ? 0 : range_lob();
+if (!value && s->pci_hole64_fix) {
+value = pc_pci_hole64_start();
+}
  visit_type_uint64(v, name, , errp);
  }
  
+/*

+ * The 64bit PCI hole end is set by the Guest firmware
+ * as the address of the last 64bit PCI MEM resource.
+ * Then 

[Qemu-devel] [PATCH 3/3] qemu-iotests: Test I/O limits with removable media

[Alberto Garcia]

This test hotplugs a CD drive to a VM and checks that I/O limits can
be set only when the drive has media inserted and that they are kept
when the media is replaced.

This also tests the removal of a device with valid I/O limits set but
no media inserted. This involves deleting and disabling the limits
of a BlockBackend without BlockDriverState, a scenario that has been
crashing until the fixes from the last couple of patches.

Signed-off-by: Alberto Garcia 
---
 tests/qemu-iotests/093 | 62 ++
 tests/qemu-iotests/093.out |  4 +--
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index ef3997206b..7862f2ba94 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -308,6 +308,68 @@ class ThrottleTestGroupNames(iotests.QMPTestCase):
 groupname = "group%d" % i
 self.verify_name(devname, groupname)
 
+class ThrottleTestRemovableMedia(iotests.QMPTestCase):
+def setUp(self):
+self.vm = iotests.VM()
+if iotests.qemu_default_machine == 's390-ccw-virtio':
+self.vm.add_device("virtio-scsi-ccw,id=virtio-scsi")
+else:
+self.vm.add_device("virtio-scsi-pci,id=virtio-scsi")
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+
+def test_removable_media(self):
+# Add a couple of dummy nodes named cd0 and cd1
+result = self.vm.qmp("blockdev-add", driver = "null-aio",
+ node_name = "cd0")
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp("blockdev-add", driver = "null-aio",
+ node_name = "cd1")
+self.assert_qmp(result, 'return', {})
+
+# Attach a CD drive with cd0 inserted
+result = self.vm.qmp("device_add", driver = "scsi-cd",
+ id = "dev0", drive = "cd0")
+self.assert_qmp(result, 'return', {})
+
+# Set I/O limits
+args = { "id": "dev0", "iops": 100, "iops_rd": 0, "iops_wr": 0,
+"bps":  50,  "bps_rd": 0,  "bps_wr": 0 }
+result = self.vm.qmp("block_set_io_throttle", conv_keys = False, 
**args)
+self.assert_qmp(result, 'return', {})
+
+# Check that the I/O limits have been set
+result = self.vm.qmp("query-block")
+self.assert_qmp(result, 'return[0]/inserted/iops', 100)
+self.assert_qmp(result, 'return[0]/inserted/bps',   50)
+
+# Now eject cd0 and insert cd1
+result = self.vm.qmp("blockdev-open-tray", id = 'dev0')
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp("x-blockdev-remove-medium", id = 'dev0')
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp("x-blockdev-insert-medium", id = 'dev0', 
node_name = 'cd1')
+self.assert_qmp(result, 'return', {})
+
+# Check that the I/O limits are still the same
+result = self.vm.qmp("query-block")
+self.assert_qmp(result, 'return[0]/inserted/iops', 100)
+self.assert_qmp(result, 'return[0]/inserted/bps',   50)
+
+# Eject cd1
+result = self.vm.qmp("x-blockdev-remove-medium", id = 'dev0')
+self.assert_qmp(result, 'return', {})
+
+# Check that we can't set limits if the device has no medium
+result = self.vm.qmp("block_set_io_throttle", conv_keys = False, 
**args)
+self.assert_qmp(result, 'error/class', 'GenericError')
+
+# Remove the CD drive
+result = self.vm.qmp("device_del", id = 'dev0')
+self.assert_qmp(result, 'return', {})
+
 
 if __name__ == '__main__':
 iotests.main(supported_fmts=["raw"])
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
index 2f7d3902f2..594c16f49f 100644
--- a/tests/qemu-iotests/093.out
+++ b/tests/qemu-iotests/093.out
@@ -1,5 +1,5 @@
-...
+
 --
-Ran 7 tests
+Ran 8 tests
 
 OK
-- 
2.11.0

[Qemu-devel] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

[Alberto Garcia]

If a BlockBackend has I/O limits set then its ThrottleGroupMember
structure uses the AioContext from its attached BlockDriverState.
Those two contexts must be kept in sync manually. This is not
ideal and will be fixed in the future by removing the throttling
configuration from the BlockBackend and storing it in an implicit
filter node instead, but for now we have to live with this.

When you remove the BlockDriverState from the backend then the
throttle timers are destroyed. If a new BlockDriverState is later
inserted then they are created again using the new AioContext.

There'a a couple of problems with this:

   a) The code manipulates the timers directly, leaving the
  ThrottleGroupMember.aio_context field in an inconsisent state.

   b) If you remove the I/O limits (e.g by destroying the backend)
  when the timers are gone then throttle_group_unregister_tgm()
  will attempt to destroy them again, crashing QEMU.

While b) could be fixed easily by allowing the timers to be freed
twice, this would result in a situation in which we can no longer
guarantee that a valid ThrottleState has a valid AioContext and
timers.

This patch ensures that the timers and AioContext are always valid
when I/O limits are set, regardless of whether the BlockBackend has a
BlockDriverState inserted or not.

Reported-by: sochin jiang 
Signed-off-by: Alberto Garcia 
---
 block/block-backend.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 193a080096..38db6e639b 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -655,15 +655,15 @@ BlockBackend *blk_by_public(BlockBackendPublic *public)
  */
 void blk_remove_bs(BlockBackend *blk)
 {
+ThrottleGroupMember *tgm = >public.throttle_group_member;
 BlockDriverState *bs;
-ThrottleTimers *tt;
 
 notifier_list_notify(>remove_bs_notifiers, blk);
-if (blk->public.throttle_group_member.throttle_state) {
-tt = >public.throttle_group_member.throttle_timers;
+if (tgm->throttle_state) {
 bs = blk_bs(blk);
 bdrv_drained_begin(bs);
-throttle_timers_detach_aio_context(tt);
+throttle_group_detach_aio_context(tgm);
+throttle_group_attach_aio_context(tgm, qemu_get_aio_context());
 bdrv_drained_end(bs);
 }
 
@@ -678,6 +678,7 @@ void blk_remove_bs(BlockBackend *blk)
  */
 int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
 {
+ThrottleGroupMember *tgm = >public.throttle_group_member;
 blk->root = bdrv_root_attach_child(bs, "root", _root,
blk->perm, blk->shared_perm, blk, errp);
 if (blk->root == NULL) {
@@ -686,10 +687,9 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, 
Error **errp)
 bdrv_ref(bs);
 
 notifier_list_notify(>insert_bs_notifiers, blk);
-if (blk->public.throttle_group_member.throttle_state) {
-throttle_timers_attach_aio_context(
->public.throttle_group_member.throttle_timers,
-bdrv_get_aio_context(bs));
+if (tgm->throttle_state) {
+throttle_group_detach_aio_context(tgm);
+throttle_group_attach_aio_context(tgm, bdrv_get_aio_context(bs));
 }
 
 return 0;
-- 
2.11.0

[Qemu-devel] [PATCH 1/3] block: Check for inserted BlockDriverState in blk_io_limits_disable()

[Alberto Garcia]

When you set I/O limits using block_set_io_throttle or the command
line throttling.* options they are kept in the BlockBackend regardless
of whether a BlockDriverState is attached to the backend or not.

Therefore when removing the limits using blk_io_limits_disable() we
need to check if there's a BDS before attempting to drain it, else it
will crash QEMU. This can be reproduced very easily using HMP:

 (qemu) drive_add 0 if=none,throttling.iops-total=5000
 (qemu) drive_del none0

Reported-by: sochin jiang 
Signed-off-by: Alberto Garcia 
---
 block/block-backend.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 18e543780d..193a080096 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1978,10 +1978,16 @@ void blk_set_io_limits(BlockBackend *blk, 
ThrottleConfig *cfg)
 
 void blk_io_limits_disable(BlockBackend *blk)
 {
-assert(blk->public.throttle_group_member.throttle_state);
-bdrv_drained_begin(blk_bs(blk));
-throttle_group_unregister_tgm(>public.throttle_group_member);
-bdrv_drained_end(blk_bs(blk));
+BlockDriverState *bs = blk_bs(blk);
+ThrottleGroupMember *tgm = >public.throttle_group_member;
+assert(tgm->throttle_state);
+if (bs) {
+bdrv_drained_begin(bs);
+}
+throttle_group_unregister_tgm(tgm);
+if (bs) {
+bdrv_drained_end(bs);
+}
 }
 
 /* should be called before blk_set_io_limits if a limit is set */
-- 
2.11.0

[Qemu-devel] [PATCH 0/3] Fix throttling crashes in BlockBackend with no BlockDriverState

[Alberto Garcia]

Hi,

this series fixes the problems reported by Sochin Jiang in
BlockBackend when there's a valid throttling configuration but the BDS
has been removed.

The patches apply on top of Li Zhengui's "all I/O should be completed
before removing throttle timers" and I tested this on top of Stefan's
block branch (commit 900276cf24589596296d3d099fe609ad5c182ac9).

Regards,

Berto

Alberto Garcia (3):
  block: Check for inserted BlockDriverState in blk_io_limits_disable()
  block: Leave valid throttle timers when removing a BDS from a backend
  qemu-iotests: Test I/O limits with removable media

 block/block-backend.c  | 30 +-
 tests/qemu-iotests/093 | 62 ++
 tests/qemu-iotests/093.out |  4 +--
 3 files changed, 82 insertions(+), 14 deletions(-)

-- 
2.11.0

Re: [Qemu-devel] [PATCH for 2.11 v2 0/2] Xilinx ZCU102 fixes for 2.11

[Emilio G. Cota]

On Thu, Nov 09, 2017 at 13:49:30 -0800, Alistair Francis wrote:
> These are two small fixes for 2.11.
> 
> V2:
>  - Update qemu-doc.texi
> 
> Alistair Francis (2):
>   xlnx-zynqmp: Properly support the smp command line option
>   xlnx-zcu102: Add an info message deprecating the EP108

Peter: please do not merge this yet!

Given that these two changes require another patch to be tested,
I am going to include these two patches in an upcoming series
with the required patch added first.

BTW I'll also add my R-b tags plus the max_cpus update for the ep108.

Emilio

Re: [Qemu-devel] [PATCH for 2.11 v2 2/2] xlnx-zcu102: Add an info message deprecating the EP108

[Emilio G. Cota]

On Fri, Nov 10, 2017 at 13:07:22 -0500, Emilio G. Cota wrote:
> On Thu, Nov 09, 2017 at 13:49:35 -0800, Alistair Francis wrote:
> > The EP108 was an early access development board that is no longer used.
> > Add an info message to convert any users to the ZCU102 instead. On QEMU
> > they are both identical.
> > 
> > This patch also updated the qemu-doc.texi file to indicate that the
> > EP108 has been deprecated.
> > 
> > Signed-off-by: Alistair Francis 
> 
> Reviewed-by: Emilio G. Cota 
> 
> Note to potential reviewers: This patch is meant to be applied after the
> patch that updates ep108's max_cpus from 1 to NR_APUS + NR_RPUS, for
> otherwise the message added here won't be seen unless -smp == 1.
> This other patch is already in target-arm.next:
>  https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg06537.html

Correction: that patch does updates the zcu102, not the ep108.

I think we should update max_cpus for ep108 as well.

Emilio

Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/5] iotests: Make 083 less flaky

[Eric Blake]

On 11/10/2017 11:29 AM, Max Reitz wrote:

 It seems that in this patch you're indenting with spaces but this file
 uses tabs.
>>>
>>> Yes, but tabs are wrong. :-)
>>
>> I actually agree with you, but don't mix them in the file :-)
> 
> I can whistle and say here, too, that Eric liked it. O:-)

I don't really pay attention to which files have pre-existing TABs.
You're right that preserving whole-file TABs is a bit nicer from
consistency than reformatting a file wholesale; but then you have to
tell checkpatch that preserving TABs was intentional.  Mixed mode
indentation is not as consistent, but at least keeps checkpatch happy
without effort, and may make it easier for a patch down the road to
finally do wholesale conversion of the rest of the file to avoid TABs.

So when it comes to a file with existing TABs, I'm okay whether the
patch preserves TABs (with documentation that it is doing so
intentionally) or switches to mixed-mode spaces.

-- 
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] [PATCH 0/1] qcow2: Check that corrupted images can be repaired in iotest 060

[Max Reitz]

On 2017-11-08 13:13, Alberto Garcia wrote:
> Hi,
> 
> I sent the 'Misc qcow2 corruption checks' series the other day, and
> Kevin suggested that we check that the corrupted images can be
> repaired using qemu-img.
> 
> This patch extends the tests that I wrote in order to do just
> that. Since the series is already in Max's branch I decided to write
> this as a follow-up patch, but if you prefer I can resend it instead.
> 
> Regards,
> 
> Berto

Thanks, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for 2.11 v2 1/2] xlnx-zynqmp: Properly support the smp command line option

[Emilio G. Cota]

On Thu, Nov 09, 2017 at 13:49:33 -0800, Alistair Francis wrote:
> Allow the -smp command line option to control the number of CPUs we
> create.
> 
> Signed-off-by: Alistair Francis 
> Reviewed-by: Eduardo Habkost 

Reviewed-by: Emilio G. Cota 
Tested-by: Emilio G. Cota 

As I said in the other thread, this is much better than fiddling with
MachineClass -- thanks!

I'll send now a proper series with the fixes we discussed in the other
thread:
  https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg00502.html

Emilio

Re: [Qemu-devel] [PATCH 1/3] ivshmem: Don't update non-existent MSI routes

[Marc-André Lureau]

- Original Message -
> As of commit 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications"),
> QEMU crashes with:
> 
>   kvm_irqchip_commit_routes: Assertion `ret == 0' failed.
> 
> if the ivshmem device is configured with more vectors than what the server
> supports. This is caused by the ivshmem_vector_unmask() being called on
> vectors that have not been initialized by ivshmem_add_kvm_msi_virq().
> 
> This commit fixes it by adding a simple check to the mask and unmask
> callbacks.
> 
> Note that the opposite mismatch, if the server supplies more vectors than
> what the device is configured for, is already handled and leads to output
> like:
> 
>   Too many eventfd received, device has 1 vectors
> 
> Fixes: 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications")
> Signed-off-by: Ladi Prosek 

Reviewed-by: Marc-André Lureau 

> ---
>  hw/misc/ivshmem.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index a5a46827fe..6e46669744 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -317,6 +317,10 @@ static int ivshmem_vector_unmask(PCIDevice *dev,
> unsigned vector,
>  int ret;
>  
>  IVSHMEM_DPRINTF("vector unmask %p %d\n", dev, vector);
> +if (!v->pdev) {
> +error_report("ivshmem: vector %d route does not exist", vector);
> +return -EINVAL;
> +}
>  
>  ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg, dev);
>  if (ret < 0) {
> @@ -331,12 +335,16 @@ static void ivshmem_vector_mask(PCIDevice *dev,
> unsigned vector)
>  {
>  IVShmemState *s = IVSHMEM_COMMON(dev);
>  EventNotifier *n = >peers[s->vm_id].eventfds[vector];
> +MSIVector *v = >msi_vectors[vector];
>  int ret;
>  
>  IVSHMEM_DPRINTF("vector mask %p %d\n", dev, vector);
> +if (!v->pdev) {
> +error_report("ivshmem: vector %d route does not exist", vector);
> +return;
> +}
>  
> -ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n,
> -
> s->msi_vectors[vector].virq);
> +ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, v->virq);
>  if (ret != 0) {
>  error_report("remove_irqfd_notifier_gsi failed");
>  }
> --
> 2.13.5
> 
> 

Re: [Qemu-devel] [PATCH for 2.11 v2 2/2] xlnx-zcu102: Add an info message deprecating the EP108

[Emilio G. Cota]

On Thu, Nov 09, 2017 at 13:49:35 -0800, Alistair Francis wrote:
> The EP108 was an early access development board that is no longer used.
> Add an info message to convert any users to the ZCU102 instead. On QEMU
> they are both identical.
> 
> This patch also updated the qemu-doc.texi file to indicate that the
> EP108 has been deprecated.
> 
> Signed-off-by: Alistair Francis 

Reviewed-by: Emilio G. Cota 

Note to potential reviewers: This patch is meant to be applied after the
patch that updates ep108's max_cpus from 1 to NR_APUS + NR_RPUS, for
otherwise the message added here won't be seen unless -smp == 1.
This other patch is already in target-arm.next:
 https://lists.gnu.org/archive/html/qemu-devel/2017-10/msg06537.html

Emilio

Re: [Qemu-devel] [PATCH v6 0/6] block: Don't compare strings in bdrv_reopen_prepare()

[Max Reitz]

On 2017-11-10 18:47, Kevin Wolf wrote:
> Am 10.11.2017 um 18:36 hat Max Reitz geschrieben:
>> On 2017-11-10 10:16, Markus Armbruster wrote:
>>> Max Reitz  writes:
>>>
 bdrv_reopen_prepare() assumes that all BDS options are strings, which is
 not necessarily correct. This series introduces a new qobject_is_equal()
 function which can be used to test whether any options have changed,
 independently of their type.
>>>
>>> Series looks ready to me.  It touches QAPI to achieve its purpose in the
>>> block layer; I'd be fine with merging it via a block tree.
>>
>> Thanks!  So now it's my problem to figure out whether this is 2.11
>> material...? :-)
> 
> The test case in patch 5 segfaults without the series. Why would it not
> be a bug fix (= 2.11 material)?

Because it adds a whole lot of QAPI code, and the segfault is a clear
NULL pointer dereference which you can only get through HMP.  But then
again, the only place where the new code is used is from the place of
the bug fix itself, so I guess no regressions are possible.

So if it is a bug fix, why have you not applied it? :-)


Applied to my block branch for 2.11:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] iotests: test clearing unknown autoclear_features by qcow2

[Vladimir Sementsov-Ogievskiy]

Test clearing unknown autoclear_features by qcow2 on incoming
migration.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi all!

This patch shows degradation, added in 2.10 in commit

commit 9c5e6594f15b7364624a3ad40306c396c93a2145
Author: Kevin Wolf 
Date:   Thu May 4 18:52:40 2017 +0200

block: Fix write/resize permissions for inactive images

The problem:
When on incoming migration with shared storage we reopen image to RW mode
we call bdrv_invalidate_cache it firstly call drv->bdrv_invalidate_cache and
only after it, through "parent->role->activate(parent, _err);" we set
appropriate RW permission.

Because of this, if drv->bdrv_invalidate_cache wants to write something
(image is RW and BDRV_O_INACTIVE is not set) it goes into
"bdrv_aligned_pwritev: Assertion `child->perm & BLK_PERM_WRITE' failed"

One case, when qcow2_invalidate_cache wants to write
something - when it wants to clear some unknown autoclear features. So,
here is a test for it.

Another case is when we try to migrate persistent dirty bitmaps through
shared storage, on invalidate_cache qcow2 will try to set DIRTY bit in
all loaded bitmaps.

Kevin, what do you think? I understand that operation order should be changed
somehow in bdrv_invalidate_cache, but I'm not sure about how "parent->role->.."
things works and can we safely move this part from function end to the middle.

 tests/qemu-iotests/196 | 63 ++
 tests/qemu-iotests/196.out |  5 
 tests/qemu-iotests/group   |  1 +
 3 files changed, 69 insertions(+)
 create mode 100755 tests/qemu-iotests/196
 create mode 100644 tests/qemu-iotests/196.out

diff --git a/tests/qemu-iotests/196 b/tests/qemu-iotests/196
new file mode 100755
index 00..9ffbc723c2
--- /dev/null
+++ b/tests/qemu-iotests/196
@@ -0,0 +1,63 @@
+#!/usr/bin/env python
+#
+# Test clearing unknown autoclear_features flag by qcow2 after
+# migration. This test mimics migration to older qemu.
+#
+# Copyright (c) 2017 Parallels International GmbH
+#
+# 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.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import iotests
+from iotests import qemu_img
+
+disk = os.path.join(iotests.test_dir, 'disk')
+migfile = os.path.join(iotests.test_dir, 'migfile')
+
+class TestInvalidateAutoclear(iotests.QMPTestCase):
+
+def tearDown(self):
+self.vm_a.shutdown()
+self.vm_b.shutdown()
+os.remove(disk)
+os.remove(migfile)
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, disk, '1M')
+
+self.vm_a = iotests.VM(path_suffix='a').add_drive(disk)
+self.vm_a.launch()
+
+self.vm_b = iotests.VM(path_suffix='b').add_drive(disk)
+self.vm_b.add_incoming("exec: cat '" + migfile + "'")
+
+def test_migration(self):
+result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
+self.assert_qmp(result, 'return', {});
+self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
+
+with open(disk, 'r+b') as f:
+f.seek(88) # first byte of autoclear_features field
+f.write(b'\xff')
+
+self.vm_b.launch()
+self.assertNotEqual(self.vm_b.event_wait("RESUME"), None)
+
+with open(disk, 'rb') as f:
+f.seek(88)
+self.assertEqual(f.read(1), b'\x00')
+
+if __name__ == '__main__':
+iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/196.out b/tests/qemu-iotests/196.out
new file mode 100644
index 00..ae1213e6f8
--- /dev/null
+++ b/tests/qemu-iotests/196.out
@@ -0,0 +1,5 @@
+.
+--
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 24e5ad1b79..78d01d9516 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -193,4 +193,5 @@
 192 rw auto quick
 194 rw auto migration quick
 195 rw auto quick
+196 rw auto quick
 197 rw auto quick
-- 
2.11.1

Re: [Qemu-devel] Intermittent hang of iotest 194 (bdrv_drain_all after non-shared storage migration)

[Max Reitz]

On 2017-11-10 03:36, Fam Zheng wrote:
> On Thu, 11/09 20:31, Max Reitz wrote:
>> On 2017-11-09 16:30, Fam Zheng wrote:
>>> On Thu, 11/09 16:14, Max Reitz wrote:

[...]

 *sigh*

 OK, I'll look into it...
>>>
>>> OK, I'll let you.. Just one more thing: could it relate to the 
>>> use-after-free
>>> bug reported on block_job_defer_to_main_loop()?
>>>
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01144.html
>>
>> Thanks for the heads-up; I think it's a different issue, though.
>>
>> What appears to be happening is that the mirror job completes and then
>> drains its BDS.  While that is happening, a bdrv_drain_all() comes in
>> from block_migration_cleanup().
>>
>> That now tries to drain the mirror node.  However, that node cannot be
>> drained until the job is truly gone now, so that is what's happening:
>> mirror_exit() is called, it cleans up, destroys the mirror node, and
>> returns.
>>
>> Now bdrv_drain_all() can go on, specifically the BDRV_POLL_WHILE() on
>> the mirror node.  However, oops, that node is gone now...  So that's
>> where the issue seems to be. :-/
>>
>> Maybe all that we need to do is wrap the bdrv_drain_recurse() call in
>> bdrv_drain_all_begin() in a bdrv_ref()/bdrv_unref() pair?  Having run
>> 194 for a couple of minutes, that seems to indeed work -- until it dies
>> because of an invalid BB pointer in bdrv_next().  I guess that is
>> because bdrv_next() does not guard against deleted BDSs.
>>
>> Copying all BDS into an own list (in both bdrv_drain_all_begin() and
>> bdrv_drain_all_end()), with a strong reference to every single one, and
>> then draining them really seems to work, though.  (Survived 9000
>> iterations, that seems good enough for something that usually fails
>> after, like, 5.)
> 
> Yes, that makes sense. I'm curious if the patch in
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01649.html
> 
> would also work?

No, unfortunately it did not.

(Or maybe fortunately so, since that means I didn't do a whole lot of
work for nothing :-))

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v6 0/6] block: Don't compare strings in bdrv_reopen_prepare()

[Kevin Wolf]

Am 10.11.2017 um 18:36 hat Max Reitz geschrieben:
> On 2017-11-10 10:16, Markus Armbruster wrote:
> > Max Reitz  writes:
> > 
> >> bdrv_reopen_prepare() assumes that all BDS options are strings, which is
> >> not necessarily correct. This series introduces a new qobject_is_equal()
> >> function which can be used to test whether any options have changed,
> >> independently of their type.
> > 
> > Series looks ready to me.  It touches QAPI to achieve its purpose in the
> > block layer; I'd be fine with merging it via a block tree.
> 
> Thanks!  So now it's my problem to figure out whether this is 2.11
> material...? :-)

The test case in patch 5 segfaults without the series. Why would it not
be a bug fix (= 2.11 material)?

Kevin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v6 0/6] block: Don't compare strings in bdrv_reopen_prepare()

[Max Reitz]

On 2017-11-10 10:16, Markus Armbruster wrote:
> Max Reitz  writes:
> 
>> bdrv_reopen_prepare() assumes that all BDS options are strings, which is
>> not necessarily correct. This series introduces a new qobject_is_equal()
>> function which can be used to test whether any options have changed,
>> independently of their type.
> 
> Series looks ready to me.  It touches QAPI to achieve its purpose in the
> block layer; I'd be fine with merging it via a block tree.

Thanks!  So now it's my problem to figure out whether this is 2.11
material...? :-)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [U-Boot] Support of latest qemux86-64

[Anton Gerasimov]

Hooray, changing SYS_CAR_ADDR to 0x1 in arch/x86/cpu/qemu/Kconfig
does the trick. Bin, what do you think about it?

Best regards,
Anton Gerasimov

On 11/10/2017 06:25 PM, Anton Gerasimov wrote:
> Yes, apparently 0xdfffc is in ROM area for QEMU (0xc -- 0xe,
> defined in include/hw/loader.h). The next thing to figure out is why
> u-boot uses it as a stack area.
>
> Best regards,
> Anton Gerasimov
>
> On 11/10/2017 06:04 PM, Anton Gerasimov wrote:
>> New guess:
>>
>> in the most safe configuration of u-boot (CONFIG_SMP=n, lacpi disabled)
>> with Igor's patch applied `qemu-system-i386 -bios /path/to/uboot.rom`
>> fails on the first 'ret' instruction. GDB shows that memory at $esp
>> (0xdfffc at the entrance to board_init_f_mem) and everything around it
>> is zero despite 'call' and 'push' instructions executed. If you go one
>> commit before the breaking one it works fine, stuff gets put onto stack.
>> Could it that be that stack itself is in this 'readonly' area?
>>
>> Thanks,
>> Anton Gerasimov
>>
>> On 11/09/2017 02:58 AM, Bin Meng wrote:
>>> On Wed, Nov 8, 2017 at 9:05 PM, Anton Gerasimov
>>>  wrote:
 Adding Igor Mammedov to the loop.

>>> Really add Igor Mammedov.
>>>
>>> Igor, can you help look at this?
>>>
 On 11/08/2017 01:59 PM, Anton Gerasimov wrote:
> To whoever might be interested: I've bisected qemu and the breaking
> commit is 208fa0e43645edd0b0d8f838857dfc79daff40a8 (pc: make 'pc.rom'
> readonly when machine has PCI enabled). It's just three lines added,
> I'll paste the whole patch here. Not quite sure what can we do here 
> though.
>
>
>   diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>   index 22e16031b0..59435390ba 100644
>   --- a/hw/i386/pc.c
>   +++ b/hw/i386/pc.c
>   @@ -1443,6 +1443,9 @@ void pc_memory_init(PCMachineState *pcms,
>option_rom_mr = g_malloc(sizeof(*option_rom_mr));
>memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
>   _fatal);
>   +if (pcmc->pci_enabled) {
>   +memory_region_set_readonly(option_rom_mr, true);
>   +}
>memory_region_add_subregion_overlap(rom_memory,
>PC_ROM_MIN_VGA,
>option_rom_mr,
>
>
>>> Regards,
>>> Bin


-- 
Anton Gerasimov, ATS Advanced Telematic Systems GmbH
Kantstrasse 162, 10623 Berlin
Managing Directors: Dirk Pöschl, Armin G. Schmidt
Register Court: HRB 151501 B, Amtsgericht Charlottenburg

[Qemu-devel] [PATCH 2/3] ivshmem: Always remove irqfd notifiers

[Ladi Prosek]

As of commit 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications"),
QEMU crashes with:

ivshmem: msix_set_vector_notifiers failed
msix_unset_vector_notifiers: Assertion `dev->msix_vector_use_notifier && 
dev->msix_vector_release_notifier' failed.

if MSI-X is repeatedly enabled and disabled on the ivshmem device, for example
by loading and unloading the Windows ivshmem driver. This is because
msix_unset_vector_notifiers() doesn't call any of the release notifier callbacks
since MSI-X is already disabled at that point (msix_enabled() returning false
is how this transition is detected in the first place). Thus 
ivshmem_vector_mask()
doesn't run and when MSI-X is subsequently enabled again ivshmem_vector_unmask()
fails.

This is fixed by keeping track of unmasked vectors and making sure that
ivshmem_vector_mask() always runs on MSI-X disable.

Fixes: 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications")
Signed-off-by: Ladi Prosek 
---
 hw/misc/ivshmem.c | 30 --
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 6e46669744..493a5030a1 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -77,6 +77,7 @@ typedef struct Peer {
 typedef struct MSIVector {
 PCIDevice *pdev;
 int virq;
+bool unmasked;
 } MSIVector;
 
 typedef struct IVShmemState {
@@ -321,6 +322,7 @@ static int ivshmem_vector_unmask(PCIDevice *dev, unsigned 
vector,
 error_report("ivshmem: vector %d route does not exist", vector);
 return -EINVAL;
 }
+assert(!v->unmasked);
 
 ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg, dev);
 if (ret < 0) {
@@ -328,7 +330,11 @@ static int ivshmem_vector_unmask(PCIDevice *dev, unsigned 
vector,
 }
 kvm_irqchip_commit_routes(kvm_state);
 
-return kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, v->virq);
+ret = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, n, NULL, v->virq);
+if (ret == 0) {
+v->unmasked = true;
+}
+return ret;
 }
 
 static void ivshmem_vector_mask(PCIDevice *dev, unsigned vector)
@@ -343,9 +349,12 @@ static void ivshmem_vector_mask(PCIDevice *dev, unsigned 
vector)
 error_report("ivshmem: vector %d route does not exist", vector);
 return;
 }
+assert(v->unmasked);
 
 ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, v->virq);
-if (ret != 0) {
+if (ret == 0) {
+v->unmasked = false;
+} else {
 error_report("remove_irqfd_notifier_gsi failed");
 }
 }
@@ -817,11 +826,20 @@ static void ivshmem_disable_irqfd(IVShmemState *s)
 PCIDevice *pdev = PCI_DEVICE(s);
 int i;
 
-for (i = 0; i < s->peers[s->vm_id].nb_eventfds; i++) {
-ivshmem_remove_kvm_msi_virq(s, i);
-}
-
 msix_unset_vector_notifiers(pdev);
+
+for (i = 0; i < s->peers[s->vm_id].nb_eventfds; i++) {
+/*
+ * MSI-X is already disabled here so msix_unset_vector_notifiers
+ * didn't call our release notifier. Do it now to keep our masks and
+ * unmasks balanced.
+ */
+if (s->msi_vectors[i].unmasked) {
+ivshmem_vector_mask(pdev, i);
+}
+ivshmem_remove_kvm_msi_virq(s, i);
+}
+
 }
 
 static void ivshmem_write_config(PCIDevice *pdev, uint32_t address,
-- 
2.13.5

[Qemu-devel] [PATCH 3/3] ivshmem: Improve MSI irqfd error handling

[Ladi Prosek]

Adds a rollback path to ivshmem_enable_irqfd() and fixes
ivshmem_disable_irqfd() to bail if irqfd has not been enabled.

Signed-off-by: Ladi Prosek 
---
 hw/misc/ivshmem.c | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 493a5030a1..ff07a94691 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -784,6 +784,20 @@ static int ivshmem_setup_interrupts(IVShmemState *s, Error 
**errp)
 return 0;
 }
 
+static void ivshmem_remove_kvm_msi_virq(IVShmemState *s, int vector)
+{
+IVSHMEM_DPRINTF("ivshmem_remove_kvm_msi_virq vector:%d\n", vector);
+
+if (s->msi_vectors[vector].pdev == NULL) {
+return;
+}
+
+/* it was cleaned when masked in the frontend. */
+kvm_irqchip_release_virq(kvm_state, s->msi_vectors[vector].virq);
+
+s->msi_vectors[vector].pdev = NULL;
+}
+
 static void ivshmem_enable_irqfd(IVShmemState *s)
 {
 PCIDevice *pdev = PCI_DEVICE(s);
@@ -795,7 +809,7 @@ static void ivshmem_enable_irqfd(IVShmemState *s)
 ivshmem_add_kvm_msi_virq(s, i, );
 if (err) {
 error_report_err(err);
-/* TODO do we need to handle the error? */
+goto undo;
 }
 }
 
@@ -804,21 +818,14 @@ static void ivshmem_enable_irqfd(IVShmemState *s)
   ivshmem_vector_mask,
   ivshmem_vector_poll)) {
 error_report("ivshmem: msix_set_vector_notifiers failed");
+goto undo;
 }
-}
+return;
 
-static void ivshmem_remove_kvm_msi_virq(IVShmemState *s, int vector)
-{
-IVSHMEM_DPRINTF("ivshmem_remove_kvm_msi_virq vector:%d\n", vector);
-
-if (s->msi_vectors[vector].pdev == NULL) {
-return;
+undo:
+while (--i >= 0) {
+ivshmem_remove_kvm_msi_virq(s, i);
 }
-
-/* it was cleaned when masked in the frontend. */
-kvm_irqchip_release_virq(kvm_state, s->msi_vectors[vector].virq);
-
-s->msi_vectors[vector].pdev = NULL;
 }
 
 static void ivshmem_disable_irqfd(IVShmemState *s)
@@ -826,6 +833,10 @@ static void ivshmem_disable_irqfd(IVShmemState *s)
 PCIDevice *pdev = PCI_DEVICE(s);
 int i;
 
+if (!pdev->msix_vector_use_notifier) {
+return;
+}
+
 msix_unset_vector_notifiers(pdev);
 
 for (i = 0; i < s->peers[s->vm_id].nb_eventfds; i++) {
-- 
2.13.5

[Qemu-devel] [PATCH 0/3] ivshmem: MSI bug fixes

[Ladi Prosek]

Fixes bugs in the ivshmem device implementation uncovered with the new
Windows ivshmem driver:
https://github.com/virtio-win/kvm-guest-drivers-windows/tree/master/ivshmem

Ladi Prosek (3):
  ivshmem: Don't update non-existent MSI routes
  ivshmem: Always remove irqfd notifiers
  ivshmem: Improve MSI irqfd error handling

 hw/misc/ivshmem.c | 75 +--
 1 file changed, 56 insertions(+), 19 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH 1/3] ivshmem: Don't update non-existent MSI routes

[Ladi Prosek]

As of commit 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications"),
QEMU crashes with:

  kvm_irqchip_commit_routes: Assertion `ret == 0' failed.

if the ivshmem device is configured with more vectors than what the server
supports. This is caused by the ivshmem_vector_unmask() being called on
vectors that have not been initialized by ivshmem_add_kvm_msi_virq().

This commit fixes it by adding a simple check to the mask and unmask
callbacks.

Note that the opposite mismatch, if the server supplies more vectors than
what the device is configured for, is already handled and leads to output
like:

  Too many eventfd received, device has 1 vectors

Fixes: 660c97eef6f8 ("ivshmem: use kvm irqfd for msi notifications")
Signed-off-by: Ladi Prosek 
---
 hw/misc/ivshmem.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index a5a46827fe..6e46669744 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -317,6 +317,10 @@ static int ivshmem_vector_unmask(PCIDevice *dev, unsigned 
vector,
 int ret;
 
 IVSHMEM_DPRINTF("vector unmask %p %d\n", dev, vector);
+if (!v->pdev) {
+error_report("ivshmem: vector %d route does not exist", vector);
+return -EINVAL;
+}
 
 ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg, dev);
 if (ret < 0) {
@@ -331,12 +335,16 @@ static void ivshmem_vector_mask(PCIDevice *dev, unsigned 
vector)
 {
 IVShmemState *s = IVSHMEM_COMMON(dev);
 EventNotifier *n = >peers[s->vm_id].eventfds[vector];
+MSIVector *v = >msi_vectors[vector];
 int ret;
 
 IVSHMEM_DPRINTF("vector mask %p %d\n", dev, vector);
+if (!v->pdev) {
+error_report("ivshmem: vector %d route does not exist", vector);
+return;
+}
 
-ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n,
-s->msi_vectors[vector].virq);
+ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n, v->virq);
 if (ret != 0) {
 error_report("remove_irqfd_notifier_gsi failed");
 }
-- 
2.13.5

Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/5] iotests: Make 083 less flaky

[Max Reitz]

On 2017-11-10 16:51, Alberto Garcia wrote:
> On Fri 10 Nov 2017 04:18:15 PM CET, Max Reitz wrote:
>> On 2017-11-10 11:02, Alberto Garcia wrote:
>>> On Thu 09 Nov 2017 09:30:24 PM CET, Max Reitz wrote:
 +echo > "$TEST_DIR/nbd-fault-injector.out"
$PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" 
 "$TEST_DIR/nbd-fault-injector.conf" >"$TEST_DIR/nbd-fault-injector.out" 
 2>&1 &
>>>
>>> It seems that in this patch you're indenting with spaces but this file
>>> uses tabs.
>>
>> Yes, but tabs are wrong. :-)
> 
> I actually agree with you, but don't mix them in the file :-)

I can whistle and say here, too, that Eric liked it. O:-)

And I really don't want to use tabs in the second hunk of this patch, as
it's broken over two lines.

Max



signature.asc
Description: OpenPGP digital signature