Re: [Qemu-devel] [PATCH 5/5] commit: Remove overlay_bs

[Eric Blake]

On 09/25/2017 07:28 AM, Kevin Wolf wrote:
> We don't need to make any assumptions about the graph layout above the
> top node of the commit operation any more. Remove the use of
> bdrv_find_overlay() and related variables from the commit job code.
> 
> bdrv_drop_intermediate() doesn't use the 'active' parameter any more, so
> we can just drop it.
> 
> The overlay node was previously added to the block job to get a
> BLK_PERM_GRAPH_MOD. We really need to respect those permissions in
> bdrv_drop_intermediate() now, but as long as we haven't figured out yet
> how BLK_PERM_GRAPH_MOD is actually supposed to work, just leave a TODO
> comment there.
> 
> With this change, it is now possible to perform another block job on an
> overlay node without conflicts. qemu-iotests 030 is changed accordingly.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  include/block/block.h  |  3 +--
>  block.c|  6 +++--
>  block/commit.c | 62 
> --
>  tests/qemu-iotests/030 |  4 
>  4 files changed, 20 insertions(+), 55 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] [Qemu-stable] [PATCH] migration: disable auto-converge during bulk block migration

[Michael Roth]

Quoting Peter Lieven (2017-09-21 07:32:32)
> auto-converge and block migration currently do not play well together.
> During block migration the auto-converge logic detects that ram
> migration makes no progress and thus throttles down the vm until
> it nearly stalls completely. Avoid this by disabling the throttling
> logic during the bulk phase of the block migration.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Peter Lieven 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  migration/block.c | 5 +
>  migration/block.h | 7 +++
>  migration/ram.c   | 3 ++-
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/block.c b/migration/block.c
> index 9171f60..606ad4d 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -161,6 +161,11 @@ int blk_mig_active(void)
>  return !QSIMPLEQ_EMPTY(_mig_state.bmds_list);
>  }
> 
> +int blk_mig_bulk_active(void)
> +{
> +return blk_mig_active() && !block_mig_state.bulk_completed;
> +}
> +
>  uint64_t blk_mig_bytes_transferred(void)
>  {
>  BlkMigDevState *bmds;
> diff --git a/migration/block.h b/migration/block.h
> index 22ebe94..3178609 100644
> --- a/migration/block.h
> +++ b/migration/block.h
> @@ -16,6 +16,7 @@
> 
>  #ifdef CONFIG_LIVE_BLOCK_MIGRATION
>  int blk_mig_active(void);
> +int blk_mig_bulk_active(void);
>  uint64_t blk_mig_bytes_transferred(void);
>  uint64_t blk_mig_bytes_remaining(void);
>  uint64_t blk_mig_bytes_total(void);
> @@ -25,6 +26,12 @@ static inline int blk_mig_active(void)
>  {
>  return false;
>  }
> +
> +static inline int blk_mig_bulk_active(void)
> +{
> +return false;
> +}
> +
>  static inline uint64_t blk_mig_bytes_transferred(void)
>  {
>  return 0;
> diff --git a/migration/ram.c b/migration/ram.c
> index e18b3e2..720470e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -46,6 +46,7 @@
>  #include "exec/ram_addr.h"
>  #include "qemu/rcu_queue.h"
>  #include "migration/colo.h"
> +#include "migration/block.h"
> 
>  /***/
>  /* ram save/restore */
> @@ -623,7 +624,7 @@ static void migration_bitmap_sync(RAMState *rs)
>  / (end_time - rs->time_last_bitmap_sync);
>  bytes_xfer_now = ram_counters.transferred;
> 
> -if (migrate_auto_converge()) {
> +if (migrate_auto_converge() && !blk_mig_bulk_active()) {
>  /* The following detection logic can be refined later. For now:
> Check to see if the dirtied bytes is 50% more than the approx.
> amount of bytes that just got transferred since the last time 
> we
> -- 
> 1.9.1
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: move registration of "host" CPU core type to machine code

[Greg Kurz]

On Mon, 25 Sep 2017 17:48:57 +0200
Greg Kurz  wrote:

> On Mon, 25 Sep 2017 15:41:34 +0200
> Igor Mammedov  wrote:
> 
> > On Mon, 25 Sep 2017 11:47:33 +0200
> > Greg Kurz  wrote:
> >   
> > > The CPU core abstraction belongs to the machine code. This also gets
> > > rid of some code duplication.
> > > 
> > > Signed-off-by: Greg Kurz 
> > > ---
> > > 
> > > hw/ppc/spapr_cpu_core.h is also included elsewhere in target/ppc/kvm.c
> > > but this is already handled by the following cleanup patch:
> > > 
> > > https://patchwork.ozlabs.org/patch/817598/
> > > ---
> > >  hw/ppc/spapr.c  |4 
> > >  hw/ppc/spapr_cpu_core.c |   34 ++
> > >  include/hw/ppc/spapr_cpu_core.h |2 +-
> > >  target/ppc/kvm.c|   12 
> > >  4 files changed, 27 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 0ce3ec87ac59..e82c8532ffb0 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2349,6 +2349,10 @@ static void ppc_spapr_init(MachineState *machine)
> > >  }
> > >  
> > >  /* init CPUs */
> > > +if (kvm_enabled()) {
> > > +spapr_cpu_core_register_host_type();
> > > +}
> > why don't we create it statically in hw/ppc/spapr_cpu_core.c
> > like it's done in x86, i.e.
> > 
> >   static void x86_cpu_register_types(void)  
> >
> >   { 
> >
> >   ...  
> >   #ifdef CONFIG_KVM 
> >
> >   type_register_static(_x86_cpu_type_info);
> >
> >   #endif
> >
> >   } 
> >   type_init(x86_cpu_register_types)
> > 
> > and do the same for host CPU as well?
> >   
> 
> Hi Igor,
> 
> Not sure yet why we use dynamic types, but I'd be glad to dig a bit more.

So the problem is that it was decided to make the host CPU class a
subclass of the host's CPU model, and this requires all the CPU model
classes to be registered beforehand.

commit 5ba4576b858c0d6056f59abb7e17a2b63f7905f3
Author: Andreas Färber 
Date:   Sat Feb 23 11:22:12 2013 +

target-ppc: Make host CPU a subclass of the host's CPU model

This avoids assigning individual class fields and contributors
forgetting to add field assignments in KVM-only code.

ppc_cpu_class_find_by_pvr() requires the CPU model classes to be
registered, so defer host CPU type registration to kvm_arch_init().

Only register the host CPU type if there is a class with matching PVR.
This lets us drop error handling from instance_init.

Signed-off-by: Andreas Färber 
Signed-off-by: Alexander Graf 

I can't think of an alternate way to do this. Any suggestion ?

> Maybe I'll do this in a followup patch though, since the present patch
> is mostly about not referencing sPAPRCPUCore from the KVM code anymore.
> 
> > my gripe with 'dynamic' types is that it creates problems
> > if one needs to access type information before type is created.
> > In my use-case it's before machine_init() is called but
> > after kvm is initialized, so I was sort of 'fine' with way
> > it's currently handled /i.e. still too much convoluted with
> > aliases redefinition and all but sort of 'manageable' /.
> > 
> > However I'd very much prefer static type info so it
> > could be used regardless of place/time it's accessed.
> >   
> 
> I agree that it would be better.
> 
> > I can add a couple patches to that effect into upcomming
> > cpu_model removal series, if that's ok for PPC folks.
> >   
> 
> Oh, and can you share an url to see how it looks like ?
> 
> Cheers,
> 
> --
> Greg
> 
> >   
> > > +
> > >  if (machine->cpu_model == NULL) {
> > >  machine->cpu_model = kvm_enabled() ? "host" : 
> > > smc->tcg_default_cpu;
> > >  }
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index c08ee7571a50..6e224ba029ec 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -317,7 +317,7 @@ static Property spapr_cpu_core_properties[] = {
> > >  DEFINE_PROP_END_OF_LIST()
> > >  };
> > >  
> > > -void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> > > +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> > >  {
> > >  DeviceClass *dc = DEVICE_CLASS(oc);
> > >  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
> > > @@ -337,6 +337,20 @@ static const TypeInfo spapr_cpu_core_type_info = {
> > >  .class_size = sizeof(sPAPRCPUCoreClass),
> > >  };
> > >  
> > > +static void spapr_cpu_core_register_type(const char *model_name)
> > > +{
> > > +TypeInfo type_info = {
> > > +

Re: [Qemu-devel] [PULL 00/31] Trivial patches for 2017-09-25

[Peter Maydell]

On 24 September 2017 at 22:22, Michael Tokarev  wrote:
> This is a collection of trivial stuff collected for quite some time.
> It includes various stuff, and just one series from
> Philippe Mathieu-Daudé (MAINTAINERS update), - other his series are
> in the works.
>
> Thanks,
>
> /mjt
>
> The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2017-09-23 12:55:40 +0100)
>
> are available in the git repository at:
>
>   git://git.corpit.ru/qemu.git tags/trivial-patches-fetch
>
> for you to fetch changes up to 97fb016a2aae686098f01d1c2dc194ed0f8e1c36:
>
>   hw/isa/pc87312: Mark the device with user_creatable = false (2017-09-25 
> 00:09:11 +0300)
>
> 
> trivial patches for 2017-09-25

This fails 'make check' on most of my configs:

  GTESTER check-qtest-ppc64
qemu-system-ppc64: -object
filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0:
Device 'qtest-bn0' not found
Broken pipe
GTester: last random seed: R02Sb816ff80b7d08ef6a5328ff373d8cd65

GLib-CRITICAL **: void g_hook_destroy_link(GHookList *, GHook *):
assertion `hook != NULL' failed
aborting...
  GTESTER check-qtest-ppcemb
  GTESTER check-qtest-s390x
qemu-system-s390x: -object
filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0:
Device 'qtest-bn0' not found
Broken pipe
GTester: last random seed: R02S61c57a88e229192da22afbf90262d768

GLib-CRITICAL **: void g_hook_destroy_link(GHookList *, GHook *):
assertion `hook != NULL' failed
aborting...
  GTESTER check-qtest-sh4
  GTESTER check-qtest-sh4eb
  GTESTER check-qtest-sparc
  GTESTER check-qtest-sparc64
  GTESTER check-qtest-tricore
  GTESTER check-qtest-unicore32
  GTESTER check-qtest-x86_64
qemu-system-x86_64: -object
filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0:
Device 'qtest-bn0' not found
Broken pipe
GTester: last random seed: R02S2210aede2be05fdbaceb0c1e23378915

GLib-CRITICAL **: void g_hook_destroy_link(GHookList *, GHook *):
assertion `hook != NULL' failed
aborting...
  GTESTER check-qtest-xtensa


thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 14/27] s390x/kvm: factor out storing of adtl CPU status

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Called from SIGP code to be factored out, so let's move it. Add a
> FIXME for TCG code in the future.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.c   | 29 +
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c  | 30 +-
>  3 files changed, 31 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 14/27] s390x/kvm: factor out storing of adtl CPU status

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Called from SIGP code to be factored out, so let's move it. Add a
> FIXME for TCG code in the future.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.c   | 29 +
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c  | 30 +-
>  3 files changed, 31 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 15/27] s390x/kvm: drop two debug prints

[Richard Henderson]

On 09/18/2017 09:00 AM, David Hildenbrand wrote:
> Preparation for moving it out of kvm.c.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/kvm.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 0/8] Remove some of the fprintf(stderr, "*

[no-reply]

Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH v1 0/8]  Remove some of the fprintf(stderr, "*
Message-id: cover.1506384414.git.alistair.fran...@xilinx.com

=== 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
 - [tag update]  patchew/20170913160333.23622-1-ebl...@redhat.com -> 
patchew/20170913160333.23622-1-ebl...@redhat.com
 - [tag update]  
patchew/20170919150313.10833-1-richard.hender...@linaro.org -> 
patchew/20170919150313.10833-1-richard.hender...@linaro.org
 - [tag update]  patchew/20170925082913.22089-1-f...@redhat.com -> 
patchew/20170925082913.22089-1-f...@redhat.com
 - [tag update]  patchew/20170925145526.32690-1-ebl...@redhat.com -> 
patchew/20170925145526.32690-1-ebl...@redhat.com
 * [new tag] patchew/cover.1506384414.git.alistair.fran...@xilinx.com 
-> patchew/cover.1506384414.git.alistair.fran...@xilinx.com
Switched to a new branch 'test'
8d5b2ea target: Replace fprintf(stderr, "*\n" with error_report()
d8106c7 tcg: Replace fprintf(stderr, "*\n" with error_report()
ce4a0a7 ui: Replace fprintf(stderr, "*\n" with error_report()
05fbef2 util: Replace fprintf(stderr, "*\n" with error_report()
79f557c block: Replace fprintf(stderr, "*\n" with error_report()
b6097a9 hw: Replace fprintf(stderr, "*\n" with error_report()
2149242 tests: Replace fprintf(stderr, "*\n" with error_report()
e646215 Replace all occurances of __FUNCTION__ with __func__

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=19724
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-lzr53l_5/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

Re: [Qemu-devel] [PATCH] block/qcow2-bitmap: fix use of uninitialized pointer

[Max Reitz]

On 2017-09-22 16:43, Vladimir Sementsov-Ogievskiy wrote:
> Without initialization to zero dirty_bitmap field may be not zero
> for a bitmap which should not be stored and
> qcow2_store_persistent_dirty_bitmaps will erroneously call
> store_bitmap for it which leads to SYGSEGV on bdrv_dirty_bitmap_name.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-bitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, fixed the commit message and applied it to my block branch:

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

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 00/12] Patch Round-up for stable 2.10.1, freeze on 2017-09-27

[Michael Roth]

Quoting Michael Roth (2017-09-19 19:45:09)
> Hi everyone,
> 
> The following new patches are queued for QEMU stable v2.10.1:
> 
>   https://github.com/mdroth/qemu/commits/stable-2.10-staging
> 
> The release is planned for 2017-10-02:
> 
>   https://wiki.qemu.org/Planning/2.10

Thank you for the suggestions. The following additional patches are now queued:

  s390x/ais: for 2.10 stable: disable ais facility (Christian Borntraeger)
  9pfs: check the size of transport buffer before marshaling (Jan Dakinevich)
  9pfs: fix name_to_path assertion in v9fs_complete_rename() (Jan Dakinevich)
  9pfs: fix readdir() for 9p2000.u (Jan Dakinevich)
  console: fix dpy_gfx_replace_surface assert (Gerd Hoffmann)
  ide: ahci: unparent children buses before freeing their memory (Igor Mammedov)
  hw/ide/microdrive: Mark the dscm1 device with user_creatable = false 
(Thomas Huth)
  hw/arm/aspeed_soc: Mark devices as user_creatable = false (Thomas Huth)
  hw/arm/digic: Mark device with user_creatable = false (Thomas Huth)
  s390x/ipl: The s390-ipl device is not hot-pluggable (Thomas Huth)
  watchdog/wdt_diag288: Mark diag288 watchdog as non-hotpluggable (Thomas Huth)
  multiboot: validate multiboot header address values (Prasad J Pandit)
  vga: stop passing pointers to vga_draw_line* functions (Gerd Hoffmann)
  vga: fix display update region calculation (split screen) (Gerd Hoffmann)

The following patches are tagged for inclusion, but still awaiting pull
requests:

  block/qcow2-bitmap: fix use of uninitialized pointer
  migration: disable auto-converge during bulk block migration
  accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)
  virtio/vhost: reset dev->log after syncing
  block/throttle-groups.c: allocate RestartData on the heap
  throttle-groups: update tg->any_timer_armed[] on detach

> 
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.
> 
> Testing/feedback is greatly appreciated.
> 
> Thanks!
> 
> 
> Alex Williamson (1):
>   vhost: Release memory references on cleanup
> 
> Farhan Ali (1):
>   s390-ccw: Fix alignment for CCW1
> 
> Greg Kurz (1):
>   virtfs: error out gracefully when mandatory suboptions are missing
> 
> Hannes Reinecke (1):
>   scsi-bus: correct responses for INQUIRY and REQUEST SENSE
> 
> Marc-André Lureau (2):
>   libvhost-user: support resuming vq->last_avail_idx based on used_idx
>   vhost-user-bridge: fix resume regression (since 2.9)
> 
> Pavel Butsykin (1):
>   qcow2: move qcow2_store_persistent_dirty_bitmaps() before cache flushing
> 
> Peter Maydell (1):
>   mps2-an511: Fix wiring of UART overflow interrupt lines
> 
> Pranith Kumar (1):
>   arm_gicv3_kvm: Fix compile warning
> 
> Richard Henderson (1):
>   target/arm: Fix aa64 ldp register writeback
> 
> Samuel Thibault (1):
>   slirp: fix clearing ifq_so from pending packets
> 
> Thomas Huth (1):
>   hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable 
> = false
> 
>  block/qcow2.c | 16 +++---
>  contrib/libvhost-user/libvhost-user.c | 13 
>  contrib/libvhost-user/libvhost-user.h |  7 +++
>  hw/arm/allwinner-a10.c|  2 ++
>  hw/arm/mps2.c |  4 ++--
>  hw/intc/arm_gicv3_kvm.c   |  2 +-
>  hw/scsi/scsi-bus.c| 29 ++
>  hw/virtio/vhost.c |  4 
>  pc-bios/s390-ccw/cio.h|  2 +-
>  scripts/device-crash-test |  1 -
>  slirp/socket.c| 39 
> +--
>  target/arm/translate-a64.c| 29 +++---
>  tests/vhost-user-bridge.c |  7 +++
>  vl.c  | 16 --
>  14 files changed, 120 insertions(+), 51 deletions(-)
> 
> 
> 

Re: [Qemu-devel] [PATCH] accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)

[Michael Roth]

Quoting Alex Bennée (2017-09-21 06:06:25)
> The mmio path (see exec.c:prepare_mmio_access) already protects itself
> against recursive locking and it makes sense to do the same for
> io_readx/writex. Otherwise any helper running in the BQL context will
> assert when it attempts to write to device memory as in the case of
> the bug report.
> 
> Signed-off-by: Alex Bennée 
> CC: Richard Jones 
> CC: Paolo Bonzini 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  accel/tcg/cputlb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e72415a882..bcbcc4db6c 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -765,7 +765,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
> 
>  cpu->mem_io_vaddr = addr;
> 
> -if (mr->global_locking) {
> +if (mr->global_locking && !qemu_mutex_iothread_locked()) {
>  qemu_mutex_lock_iothread();
>  locked = true;
>  }
> @@ -800,7 +800,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
>  cpu->mem_io_vaddr = addr;
>  cpu->mem_io_pc = retaddr;
> 
> -if (mr->global_locking) {
> +if (mr->global_locking && !qemu_mutex_iothread_locked()) {
>  qemu_mutex_lock_iothread();
>  locked = true;
>  }
> -- 
> 2.14.1
> 
> 

Re: [Qemu-devel] [PATCH v2] hw/sd: fix out-of-bounds check for multi block reads

[Alistair Francis]

On Mon, Sep 25, 2017 at 3:38 PM, Peter Maydell  wrote:
> On 25 September 2017 at 22:16, Alistair Francis  wrote:
>> On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
>>  wrote:
>>> Alistair, were you planning to provide a reviewed-by: for this
>>> patch (or did you have more review comments on it)?
>>
>> Ah woops, this slipped through. Looks fine to me then.
>>
>> Reviewed-by: Alistair Francis 
>
> Cheers. Should we cc:stable on this too? I'm inclined towards 'yes'.

Yeah, I don't see a reason not to.

Thanks,
Alistair

>
> thanks
> -- PMM

[Qemu-devel] [PATCH v2 4/6] linux-user: refactor socket.h for sparc

[Carlo Marcelo Arenas Belón]

fixes SOL_SOCKET and SO_LINGER and all other values that didn't match the
default (SO_PASSSEC being the exception as it only changed base)

TARGET_SOCK_{NONBLOCK,CLOEXEC} updated to match the values for the header:
arch/sparc/include/uapi/asm/fcntl.h

Signed-off-by: Carlo Marcelo Arenas Belón 
Reviewed-by: Laurent Vivier 
---
 linux-user/socket.h |  46 ++--
 linux-user/sparc/sockbits.h | 104 
 2 files changed, 107 insertions(+), 43 deletions(-)
 create mode 100644 linux-user/sparc/sockbits.h

diff --git a/linux-user/socket.h b/linux-user/socket.h
index 036270a6e4..dfa692286b 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -4,50 +4,10 @@
 #include "alpha/sockbits.h"
 #elif defined(TARGET_HPPA)
 #include "hppa/sockbits.h"
+#elif defined(TARGET_SPARC)
+#include "sparc/sockbits.h"
 #else
 
-#if defined(TARGET_SPARC)
-/** sock_type - Socket types
- *
- * Please notice that for binary compat reasons SPARC has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-
-#define ARCH_HAS_SOCKET_TYPES  1
-
-enum sock_type {
-   TARGET_SOCK_STREAM  = 1,
-   TARGET_SOCK_DGRAM   = 2,
-   TARGET_SOCK_RAW = 3,
-   TARGET_SOCK_RDM = 4,
-   TARGET_SOCK_SEQPACKET   = 5,
-   TARGET_SOCK_DCCP= 6,
-   TARGET_SOCK_PACKET  = 10,
-   TARGET_SOCK_CLOEXEC = 02000,
-   TARGET_SOCK_NONBLOCK= 04,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK0xf  /* Covers up to TARGET_SOCK_MAX-1. */
-
-#define TARGET_SO_PASSSEC31
-#else
-#define TARGET_SO_PASSSEC34
-#endif
-
 /* For setsockopt(2) */
 #define TARGET_SOL_SOCKET  1
 
@@ -103,11 +63,11 @@
 
 #define TARGET_SO_PEERSEC  31
 
+#define TARGET_SO_PASSSEC  34
 #endif
 
 #ifndef ARCH_HAS_SOCKET_TYPES
 /** sock_type - Socket types - default values
- *
  *
  * @SOCK_STREAM - stream (connection) socket
  * @SOCK_DGRAM - datagram (conn.less) socket
diff --git a/linux-user/sparc/sockbits.h b/linux-user/sparc/sockbits.h
new file mode 100644
index 00..d51ae5f84f
--- /dev/null
+++ b/linux-user/sparc/sockbits.h
@@ -0,0 +1,104 @@
+#define TARGET_SOL_SOCKET0x
+
+#define TARGET_SO_DEBUG  0x0001
+#define TARGET_SO_PASSCRED   0x0002
+#define TARGET_SO_REUSEADDR  0x0004
+#define TARGET_SO_KEEPALIVE  0x0008
+#define TARGET_SO_DONTROUTE  0x0010
+#define TARGET_SO_BROADCAST  0x0020
+#define TARGET_SO_PEERCRED   0x0040
+#define TARGET_SO_LINGER 0x0080
+#define TARGET_SO_OOBINLINE  0x0100
+#define TARGET_SO_REUSEPORT  0x0200
+#define TARGET_SO_BSDCOMPAT  0x0400
+#define TARGET_SO_RCVLOWAT   0x0800
+#define TARGET_SO_SNDLOWAT   0x1000
+#define TARGET_SO_RCVTIMEO   0x2000
+#define TARGET_SO_SNDTIMEO   0x4000
+#define TARGET_SO_ACCEPTCONN 0x8000
+#define TARGET_SO_SNDBUF 0x1001
+#define TARGET_SO_RCVBUF 0x1002
+#define TARGET_SO_SNDBUFFORCE0x100a
+#define TARGET_SO_RCVBUFFORCE0x100b
+#define TARGET_SO_ERROR  0x1007
+#define TARGET_SO_TYPE   0x1008
+#define TARGET_SO_PROTOCOL   0x1028
+#define TARGET_SO_DOMAIN 0x1029
+#define TARGET_SO_NO_CHECK   0x000b
+#define TARGET_SO_PRIORITY   0x000c
+#define TARGET_SO_BINDTODEVICE   0x000d
+#define TARGET_SO_ATTACH_FILTER  0x001a
+#define TARGET_SO_DETACH_FILTER  0x001b
+#define TARGET_SO_GET_FILTER TARGET_SO_ATTACH_FILTER
+#define TARGET_SO_PEERNAME   0x001c
+#define TARGET_SO_TIMESTAMP  0x001d
+#define 

[Qemu-devel] [PATCH v1 8/8] target: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Some lines where then manually tweaked to pass checkpatch.

Signed-off-by: Alistair Francis 
Cc: "Edgar E. Iglesias" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcelo Tosatti 
Cc: Michael Walle 
Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Cc: Christian Borntraeger 
Cc: Cornelia Huck 
Cc: Guan Xuetao 
Cc: qemu-...@nongnu.org
Cc: qemu-...@nongnu.org
---

 target/arm/arm-powerctl.c|  5 +++--
 target/arm/arm-semi.c|  3 ++-
 target/arm/helper.c  |  4 ++--
 target/arm/kvm.c | 16 ++---
 target/arm/kvm32.c   |  2 +-
 target/arm/kvm64.c   |  2 +-
 target/arm/translate-a64.c   |  4 ++--
 target/arm/translate.c   |  2 +-
 target/cris/helper.c |  2 +-
 target/cris/translate.c  |  2 +-
 target/i386/hax-all.c| 52 +--
 target/i386/hax-darwin.c | 26 +++---
 target/i386/hax-mem.c|  4 ++--
 target/i386/hax-windows.c| 42 +--
 target/i386/kvm.c| 38 +++
 target/i386/misc_helper.c| 12 +-
 target/lm32/op_helper.c  |  4 ++--
 target/mips/mips-semi.c  |  3 ++-
 target/mips/translate.c  |  2 +-
 target/ppc/excp_helper.c |  4 ++--
 target/ppc/kvm.c | 36 +++---
 target/ppc/mmu-hash64.c  |  2 +-
 target/ppc/mmu_helper.c  |  2 +-
 target/ppc/translate.c   | 20 -
 target/ppc/translate_init.c  | 53 ++--
 target/s390x/kvm.c   | 20 -
 target/s390x/misc_helper.c   |  2 +-
 target/sh4/translate.c   |  4 ++--
 target/unicore32/translate.c |  4 ++--
 29 files changed, 188 insertions(+), 184 deletions(-)

diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index 25207cb850..2d56d5d579 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -9,6 +9,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "cpu.h"
 #include "cpu-qom.h"
 #include "internals.h"
@@ -24,7 +25,7 @@
 #define DPRINTF(fmt, args...) \
 do { \
 if (DEBUG_ARM_POWERCTL) { \
-fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \
+error_report("[ARM]%s: " fmt , __func__, ##args); \
 } \
 } while (0)
 
@@ -32,7 +33,7 @@ CPUState *arm_get_cpu_by_id(uint64_t id)
 {
 CPUState *cpu;
 
-DPRINTF("cpu %" PRId64 "\n", id);
+DPRINTF("cpu %" PRId64 "", id);
 
 CPU_FOREACH(cpu) {
 ARMCPU *armcpu = ARM_CPU(cpu);
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 7cac8734c7..f8f12102f1 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 
 #include "cpu.h"
 #include "exec/semihost.h"
@@ -649,7 +650,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
 }
 /* fall through -- invalid for A32/T32 */
 default:
-fprintf(stderr, "qemu: Unsupported SemiHosting SWI 0x%02x\n", nr);
+error_report("qemu: Unsupported 

[Qemu-devel] [PATCH v1 6/8] ui: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Peter Maydell 
Cc: Gerd Hoffmann 
---

 ui/cocoa.m |  6 +++---
 ui/console.c   |  2 +-
 ui/curses.c|  2 +-
 ui/cursor.c| 10 +-
 ui/gtk.c   | 10 +-
 ui/input-linux.c   |  3 ++-
 ui/keymaps.c   |  4 ++--
 ui/sdl.c   | 14 +++---
 ui/sdl2.c  |  8 
 ui/sdl_zoom.c  |  3 ++-
 ui/shader.c|  8 
 ui/spice-display.c | 10 +-
 ui/vnc-enc-tight.c |  4 ++--
 ui/vnc-enc-zlib.c  |  4 ++--
 ui/vnc-enc-zrle.c  |  4 ++--
 ui/vnc.c   |  2 +-
 16 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 93e56d0518..62c021c5d3 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -235,7 +235,7 @@ const int mac_to_qkeycode_map[] = {
 static int cocoa_keycode_to_qemu(int keycode)
 {
 if (ARRAY_SIZE(mac_to_qkeycode_map) <= keycode) {
-fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
+error_report("(cocoa) warning unknown keycode 0x%x", keycode);
 return 0;
 }
 return mac_to_qkeycode_map[keycode];
@@ -908,7 +908,7 @@ QemuCocoaView *cocoaView;
 // create a view and add it to the window
 cocoaView = [[QemuCocoaView alloc] initWithFrame:NSMakeRect(0.0, 0.0, 
640.0, 480.0)];
 if(!cocoaView) {
-fprintf(stderr, "(cocoa) can't create a view\n");
+error_report("(cocoa) can't create a view");
 exit(1);
 }
 
@@ -917,7 +917,7 @@ QemuCocoaView *cocoaView;
 
styleMask:NSWindowStyleMaskTitled|NSWindowStyleMaskMiniaturizable|NSWindowStyleMaskClosable
 backing:NSBackingStoreBuffered defer:NO];
 if(!normalWindow) {
-fprintf(stderr, "(cocoa) can't create window\n");
+error_report("(cocoa) can't create window");
 exit(1);
 }
 [normalWindow setAcceptsMouseMovedEvents:YES];
diff --git a/ui/console.c b/ui/console.c
index b82c27960a..56d0ebcb50 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1417,7 +1417,7 @@ void register_displaychangelistener(DisplayChangeListener 
*dcl)
 /* display has opengl support */
 assert(dcl->con);
 if (dcl->con->gl) {
-fprintf(stderr, "can't register two opengl displays (%s, %s)\n",
+error_report("can't register two opengl displays (%s, %s)",
 dcl->ops->dpy_name, dcl->con->gl->ops->dpy_name);
 exit(1);
 }
diff --git a/ui/curses.c b/ui/curses.c
index 03cefdf470..06784ec7f0 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -423,7 +423,7 @@ void curses_display_init(DisplayState *ds, int full_screen)
 {
 #ifndef _WIN32
 if (!isatty(1)) {
-fprintf(stderr, "We need a terminal output\n");
+error_report("We need a terminal output");
 exit(1);
 }
 #endif
diff --git a/ui/cursor.c b/ui/cursor.c
index f3da0cee79..9d847031ec 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -18,12 +18,12 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
 /* parse header line: width, height, #colors, #chars */
 if (sscanf(xpm[line], "%u %u %u %u",
, , , ) != 4) {
-fprintf(stderr, "%s: header parse error: \"%s\"\n",
-__func__, xpm[line]);
+error_report("%s: header parse 

Re: [Qemu-devel] [PATCH v10 18/20] qcow2: Switch store_bitmap_data() to byte-based iteration

[John Snow]

On 09/25/2017 10:55 AM, Eric Blake wrote:
> Now that we have adjusted the majority of the calls this function
> makes to be byte-based, it is easier to read the code if it makes
> passes over the image using bytes rather than sectors.
> 
> iotests 165 was rather weak - on a default 64k-cluster image, where
> bitmap granularity also defaults to 64k bytes, a single cluster of
> the bitmap table thus covers (64*1024*8) bits which each cover 64k
> bytes, or 32G of image space.  But the test only uses a 1G image,
> so it cannot trigger any more than one loop of the code in
> store_bitmap_data(); and it was writing to the first cluster.  In
> order to test that we are properly aligning which portions of the
> bitmap are being written to the file, we really want to test a case
> where the first dirty bit returned by bdrv_dirty_iter_next() is not
> aligned to the start of a cluster, which we can do by modifying the
> test to write data that doesn't happen to fall in the first cluster
> of the image.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy
> 
> ---
> v10: no change, add R-b
> v9: update iotests to show why aligning down is needed [Kevin], R-b dropped
> v8: no change
> v7: rebase to earlier change, make rounding of offset obvious (no semantic
> change, so R-b kept) [Kevin]
> v5-v6: no change
> v4: new patch
> ---
>  block/qcow2-bitmap.c   | 31 ---
>  tests/qemu-iotests/165 |  2 +-
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 692ce0de88..df957c66d5 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1072,10 +1072,9 @@ static uint64_t *store_bitmap_data(BlockDriverState 
> *bs,
>  {
>  int ret;
>  BDRVQcow2State *s = bs->opaque;
> -int64_t sector;
> -uint64_t limit, sbc;
> +int64_t offset;
> +uint64_t limit;
>  uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
> -uint64_t bm_sectors = DIV_ROUND_UP(bm_size, BDRV_SECTOR_SIZE);
>  const char *bm_name = bdrv_dirty_bitmap_name(bitmap);
>  uint8_t *buf = NULL;
>  BdrvDirtyBitmapIter *dbi;
> @@ -1100,18 +1099,22 @@ static uint64_t *store_bitmap_data(BlockDriverState 
> *bs,
>  dbi = bdrv_dirty_iter_new(bitmap);
>  buf = g_malloc(s->cluster_size);
>  limit = bytes_covered_by_bitmap_cluster(s, bitmap);
> -sbc = limit >> BDRV_SECTOR_BITS;
>  assert(DIV_ROUND_UP(bm_size, limit) == tb_size);
> 
> -while ((sector = bdrv_dirty_iter_next(dbi) >> BDRV_SECTOR_BITS) >= 0) {
> -uint64_t cluster = sector / sbc;
> +while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
> +uint64_t cluster = offset / limit;
>  uint64_t end, write_size;
>  int64_t off;
> 
> -sector = cluster * sbc;
> -end = MIN(bm_sectors, sector + sbc);
> -write_size = bdrv_dirty_bitmap_serialization_size(bitmap,
> -sector * BDRV_SECTOR_SIZE, (end - sector) * BDRV_SECTOR_SIZE);
> +/*
> + * We found the first dirty offset, but want to write out the
> + * entire cluster of the bitmap that includes that offset,
> + * including any leading zero bits.
> + */
> +offset = QEMU_ALIGN_DOWN(offset, limit);
> +end = MIN(bm_size, offset + limit);
> +write_size = bdrv_dirty_bitmap_serialization_size(bitmap, offset,
> +  end - offset);
>  assert(write_size <= s->cluster_size);
> 
>  off = qcow2_alloc_clusters(bs, s->cluster_size);
> @@ -1123,9 +1126,7 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs,
>  }
>  tb[cluster] = off;
> 
> -bdrv_dirty_bitmap_serialize_part(bitmap, buf,
> - sector * BDRV_SECTOR_SIZE,
> - (end - sector) * BDRV_SECTOR_SIZE);
> +bdrv_dirty_bitmap_serialize_part(bitmap, buf, offset, end - offset);
>  if (write_size < s->cluster_size) {
>  memset(buf + write_size, 0, s->cluster_size - write_size);
>  }
> @@ -1143,11 +1144,11 @@ static uint64_t *store_bitmap_data(BlockDriverState 
> *bs,
>  goto fail;
>  }
> 
> -if (end >= bm_sectors) {
> +if (end >= bm_size) {
>  break;
>  }
> 
> -bdrv_set_dirty_iter(dbi, end * BDRV_SECTOR_SIZE);
> +bdrv_set_dirty_iter(dbi, end);
>  }
> 
>  *bitmap_table_size = tb_size;
> diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
> index 74d7b79a0b..a3932db3de 100755
> --- a/tests/qemu-iotests/165
> +++ b/tests/qemu-iotests/165
> @@ -27,7 +27,7 @@ disk = os.path.join(iotests.test_dir, 'disk')
>  disk_size = 0x4000 # 1G
> 
>  # regions for qemu_io: (start, count) in bytes
> -regions1 = ((0,0x10),
> +regions1 = ((0x0fff00, 0x1),
>  (0x20, 

Re: [Qemu-devel] [PATCH 2/8] block/nbd-client: refactor nbd_co_receive_reply

[Eric Blake]

On 09/25/2017 08:57 AM, Vladimir Sementsov-Ogievskiy wrote:
> Pass handle parameter directly, as the whole request isn't needed.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/nbd-client.c | 8 
>  1 file changed, 4 insertions(+), 4 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 8/8] nbd: Minimal structured read for client

[Eric Blake]

On 09/25/2017 08:58 AM, Vladimir Sementsov-Ogievskiy wrote:
> Minimal implementation: drop most of additional error information.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/nbd-client.h  |   2 +
>  include/block/nbd.h |  15 -
>  block/nbd-client.c  |  97 +-
>  nbd/client.c| 169 
> +++-
>  4 files changed, 249 insertions(+), 34 deletions(-)
> 

> +++ b/include/block/nbd.h
> @@ -57,11 +57,17 @@ struct NBDRequest {
>  };
>  typedef struct NBDRequest NBDRequest;
>  
> -struct NBDReply {
> +typedef struct NBDReply {
> +bool simple;
>  uint64_t handle;
>  uint32_t error;
> -};
> -typedef struct NBDReply NBDReply;
> +
> +uint16_t flags;
> +uint16_t type;
> +uint32_t tail_length;
> +uint64_t offset;
> +uint32_t hole_size;
> +} NBDReply;

This feels like it should be a discriminated union, rather than a struct
containing fields that are only sometimes valid...

>  
>  #define NBD_SREP_TYPE_NONE  0
>  #define NBD_SREP_TYPE_OFFSET_DATA   1
> +#define NBD_SREP_TYPE_OFFSET_HOLE   2
>  #define NBD_SREP_TYPE_ERROR NBD_SREP_ERR(1)
> +#define NBD_SREP_TYPE_ERROR_OFFSET  NBD_SREP_ERR(2)

...especially since there is more than one type of SREP packet layout.

I also wonder why you are defining constants in a piecemeal fashion,
rather than all at once (even if your minimal server implementation
doesn't send a particular constant, there's no harm in defining them all
up front).

> +++ b/block/nbd-client.c
> @@ -179,9 +179,10 @@ err:
>  return rc;
>  }
>  
> -static int nbd_co_receive_reply(NBDClientSession *s,
> -uint64_t handle,
> -QEMUIOVector *qiov)
> +static int nbd_co_receive_1_reply_or_chunk(NBDClientSession *s,

Long name, and unusual to mix in "1" instead of "one".  Would it be
better to name it nbd_co_receive_chunk, where we declare that a simple
reply is (roughly) the same amount of effort as a chunk in a structured
reply?

> +   uint64_t handle,
> +   bool *cont,
> +   QEMUIOVector *qiov)
>  {

No documentation of the function?

>  int ret;
>  int i = HANDLE_TO_INDEX(s, handle);
> @@ -191,29 +192,95 @@ static int nbd_co_receive_reply(NBDClientSession *s,
>  qemu_coroutine_yield();
>  s->requests[i].receiving = false;
>  if (!s->ioc || s->quit) {
> -ret = -EIO;
> -} else {
> -assert(s->reply.handle == handle);
> -ret = -s->reply.error;
> -if (qiov && s->reply.error == 0) {
> +*cont = false;
> +return -EIO;
> +}
> +
> +assert(s->reply.handle == handle);
> +*cont = !(s->reply.simple || (s->reply.flags & NBD_SREP_FLAG_DONE));

We need to validate that the server is not sending us SREP chunks unless
we negotiated them.  I'm thinking it might be better to do it here
(maybe you did it somewhere else, but I haven't seen it yet; I'm
reviewing the patch in textual order rather than the order in which
things are called).

> +ret = -s->reply.error;
> +if (ret < 0) {
> +goto out;
> +}
> +
> +if (s->reply.simple) {
> +if (qiov) {
>  if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
> -  NULL) < 0) {
> -ret = -EIO;
> -s->quit = true;
> +  NULL) < 0)
> +{
> +goto fatal;
>  }
>  }
> +goto out;
> +}
>  
> -/* Tell the read handler to read another header.  */
> -s->reply.handle = 0;
> +/* here we deal with successful structured reply */
> +switch (s->reply.type) {
> +QEMUIOVector sub_qiov;
> +case NBD_SREP_TYPE_OFFSET_DATA:

This is putting a LOT of smarts directly into the receive routine.
Here's where I was previously wondering (and I think Paolo as well)
whether it might be better to split the efforts: the generic function
reads off the chunk information and any payload, but a per-command
callback function then parses the chunks.  Especially since the
definition of the chunks differs on a per-command basis (yes, the NBD
spec will try to not reuse an SREP chunk type across multiple commands
unless the semantics are similar, but that's a bit more fragile).  This
particularly matters given my statement above that you want a
discriminated union, rather than a struct that contains unused fields,
for handling different SREP chunk types.

My review has to pause here for now...


-- 
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] hw/sd: fix out-of-bounds check for multi block reads

[Peter Maydell]

On 25 September 2017 at 22:16, Alistair Francis  wrote:
> On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
>  wrote:
>> Alistair, were you planning to provide a reviewed-by: for this
>> patch (or did you have more review comments on it)?
>
> Ah woops, this slipped through. Looks fine to me then.
>
> Reviewed-by: Alistair Francis 

Cheers. Should we cc:stable on this too? I'm inclined towards 'yes'.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 01/27] s390x: raise CPU hotplug irq after really hotplugged

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Let's move it into the machine, so we trigger the IRQ after setting
> ms->possible_cpus (which SCLP uses to construct the list of
> online CPUs).
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-virtio-ccw.c | 4 
>  target/s390x/cpu.c | 8 
>  2 files changed, 4 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v1 5/8] util: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Kevin Wolf 
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Stefan Weil 
Cc: qemu-bl...@nongnu.org
---

 util/aio-posix.c | 5 +++--
 util/coroutine-sigaltstack.c | 2 +-
 util/error.c | 2 +-
 util/main-loop.c | 2 +-
 util/mmap-alloc.c| 3 ++-
 util/module.c| 6 +++---
 util/osdep.c | 4 ++--
 util/oslib-posix.c   | 3 ++-
 util/oslib-win32.c   | 3 ++-
 util/qemu-coroutine.c| 5 +++--
 util/qemu-progress.c | 3 ++-
 util/qemu-thread-posix.c | 5 +++--
 util/qemu-thread-win32.c | 5 +++--
 util/qemu-timer-common.c | 3 ++-
 util/qht.c   | 2 +-
 15 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 2d51239ec6..fe4772b4a9 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -15,6 +15,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
+#include "qemu/error-report.h"
 #include "block/block.h"
 #include "qemu/rcu_queue.h"
 #include "qemu/sockets.h"
@@ -696,8 +697,8 @@ void aio_context_setup(AioContext *ctx)
 {
 /* TODO remove this in final patch submission */
 if (getenv("QEMU_AIO_POLL_MAX_NS")) {
-fprintf(stderr, "The QEMU_AIO_POLL_MAX_NS environment variable has "
-"been replaced with -object iothread,poll-max-ns=NUM\n");
+error_report("The QEMU_AIO_POLL_MAX_NS environment variable has "
+"been replaced with -object iothread,poll-max-ns=NUM");
 exit(1);
 }
 
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index f6fc49a0e5..96a01c2c88 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -80,7 +80,7 @@ static void __attribute__((constructor)) coroutine_init(void)
 
 ret = pthread_key_create(_state_key, qemu_coroutine_thread_cleanup);
 if (ret != 0) {
-fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
+error_report("unable to create leader key: %s", strerror(errno));
 abort();
 }
 }
diff --git a/util/error.c b/util/error.c
index 3efdd69162..e423368ca0 100644
--- a/util/error.c
+++ b/util/error.c
@@ -32,7 +32,7 @@ Error *error_fatal;
 static void error_handle_fatal(Error **errp, Error *err)
 {
 if (errp == _abort) {
-fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
+error_report("Unexpected error in %s() at %s:%d:",
 err->func, err->src, err->line);
 error_report_err(err);
 abort();
diff --git a/util/main-loop.c b/util/main-loop.c
index 7558eb5f53..d8369716b2 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -95,7 +95,7 @@ static int qemu_signal_init(void)
 sigdelset(, SIG_IPI);
 sigfd = qemu_signalfd();
 if (sigfd == -1) {
-fprintf(stderr, "failed to create signalfd\n");
+error_report("failed to create signalfd");
 return -errno;
 }
 
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 3ec029a9ea..11887aac69 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qemu/mmap-alloc.h"
 #include "qemu/host-utils.h"
 
@@ -51,7 +52,7 @@ size_t 

Re: [Qemu-devel] [PATCH] throttle-groups: update tg->any_timer_armed[] on detach

[Michael Roth]

Quoting Stefan Hajnoczi (2017-09-19 10:50:25)
> Clear tg->any_timer_armed[] when throttling timers are destroy during
> AioContext attach/detach.  Failure to do so causes throttling to hang
> because we believe the timer is already scheduled!
> 
> The following was broken at least since QEMU 2.10.0 with -drive
> iops=100:
> 
>   $ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000
>   (qemu) stop
>   (qemu) cont
>   ...I/O is stuck...
> 
> Reported-by: Yongxue Hong 
> Signed-off-by: Stefan Hajnoczi 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  block/throttle-groups.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index 6ba992c8d7..2bfd03faa0 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -592,7 +592,20 @@ void 
> throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
>  void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
>  {
>  ThrottleTimers *tt = >throttle_timers;
> +ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
> +
>  throttle_timers_detach_aio_context(tt);
> +
> +/* Forget about these timers, they have been destroyed */
> +qemu_mutex_lock(>lock);
> +if (tg->tokens[0] == tgm) {
> +tg->any_timer_armed[0] = false;
> +}
> +if (tg->tokens[1] == tgm) {
> +tg->any_timer_armed[1] = false;
> +}
> +qemu_mutex_unlock(>lock);
> +
>  tgm->aio_context = NULL;
>  }
> 
> -- 
> 2.13.5
> 
> 

Re: [Qemu-devel] [PATCH v10 00/20] make dirty-bitmap byte-based

[John Snow]

On 09/25/2017 10:55 AM, Eric Blake wrote:
> There are patches floating around to add NBD_CMD_BLOCK_STATUS,
> but NBD wants to report status on byte granularity (even if the
> reporting will probably be naturally aligned to sectors or even
> much higher levels).  I've therefore started the task of
> converting our block status code to report at a byte granularity
> rather than sectors.
> 
> Now that 2.11 is open, I'm rebasing/reposting the remaining patches.
> 
> The overall conversion currently looks like:
> part 1: bdrv_is_allocated (merged in 2.10, commit 51b0a488)
> part 2: dirty-bitmap (this series, v9 was here [1])
> part 3: bdrv_get_block_status (v4 is posted [2] and is mostly reviewed)
> part 4: .bdrv_co_block_status (v3 is posted [3], but needs review)
> 
> Available as a tag at:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-dirty-v10
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05387.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03543.html
> [3] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03812.html
> 
> Since v9:
> - another try at patch 5 [John]
> - add R-b where appropriate
> 
> 001/20:[] [--] 'block: Make bdrv_img_create() size selection easier to 
> read'
> 002/20:[] [--] 'hbitmap: Rename serialization_granularity to 
> serialization_align'
> 003/20:[] [--] 'qcow2: Ensure bitmap serialization is aligned'
> 004/20:[] [--] 'dirty-bitmap: Drop unused functions'
> 005/20:[0003] [FC] 'dirty-bitmap: Avoid size query failure during truncate'
> 006/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_size() to report 
> bytes'
> 007/20:[] [--] 'dirty-bitmap: Track bitmap size by bytes'
> 008/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to 
> take bytes'
> 009/20:[] [--] 'qcow2: Switch sectors_covered_by_bitmap_cluster() to 
> byte-based'
> 010/20:[] [--] 'dirty-bitmap: Set iterator start by offset, not sector'
> 011/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_iter_next() to report 
> byte offset'
> 012/20:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_count() to report 
> bytes'
> 013/20:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_locked() to take 
> bytes'
> 014/20:[] [--] 'dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use 
> bytes'
> 015/20:[] [--] 'mirror: Switch mirror_dirty_init() to byte-based 
> iteration'
> 016/20:[] [--] 'qcow2: Switch qcow2_measure() to byte-based iteration'
> 017/20:[] [--] 'qcow2: Switch load_bitmap_data() to byte-based iteration'
> 018/20:[] [--] 'qcow2: Switch store_bitmap_data() to byte-based iteration'
> 019/20:[] [--] 'dirty-bitmap: Switch bdrv_set_dirty() to bytes'
> 020/20:[] [--] 'dirty-bitmap: Convert internal hbitmap size/granularity'
> 
> Eric Blake (20):
>   block: Make bdrv_img_create() size selection easier to read
>   hbitmap: Rename serialization_granularity to serialization_align
>   qcow2: Ensure bitmap serialization is aligned
>   dirty-bitmap: Drop unused functions
>   dirty-bitmap: Avoid size query failure during truncate
>   dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes
>   dirty-bitmap: Track bitmap size by bytes
>   dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to take bytes
>   qcow2: Switch sectors_covered_by_bitmap_cluster() to byte-based
>   dirty-bitmap: Set iterator start by offset, not sector
>   dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset
>   dirty-bitmap: Change bdrv_get_dirty_count() to report bytes
>   dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes
>   dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes
>   mirror: Switch mirror_dirty_init() to byte-based iteration
>   qcow2: Switch qcow2_measure() to byte-based iteration
>   qcow2: Switch load_bitmap_data() to byte-based iteration
>   qcow2: Switch store_bitmap_data() to byte-based iteration
>   dirty-bitmap: Switch bdrv_set_dirty() to bytes
>   dirty-bitmap: Convert internal hbitmap size/granularity
> 
>  include/block/block_int.h|   2 +-
>  include/block/dirty-bitmap.h |  43 ++
>  include/qemu/hbitmap.h   |   8 +--
>  block/io.c   |   6 +-
>  block.c  |  18 --
>  block/backup.c   |   7 +--
>  block/dirty-bitmap.c | 134 
> ++-
>  block/mirror.c   |  79 +++--
>  block/qcow2-bitmap.c |  62 +++-
>  block/qcow2.c|  22 ---
>  migration/block.c|  12 ++--
>  tests/test-hbitmap.c |  10 ++--
>  util/hbitmap.c   |   8 +--
>  tests/qemu-iotests/165   |   2 +-
>  14 files changed, 174 insertions(+), 239 deletions(-)
> 

Tested-by: John Snow 

(Just in case I had to merge it, I ran tests.)

[Qemu-devel] [PATCH v2 5/6] linux-user: update default socket.h

[Carlo Marcelo Arenas Belón]

enable SO_REUSEPORT as a sideeffect and add SO_GET_FILTER alias

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 linux-user/socket.h | 59 +++--
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/linux-user/socket.h b/linux-user/socket.h
index dfa692286b..6f49255b5f 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -27,7 +27,7 @@
 #define TARGET_SO_PRIORITY 12
 #define TARGET_SO_LINGER   13
 #define TARGET_SO_BSDCOMPAT14
-/* To add :#define TARGET_SO_REUSEPORT 15 */
+#define TARGET_SO_REUSEPORT15
 #if defined(TARGET_PPC)
 #define TARGET_SO_RCVLOWAT 16
 #define TARGET_SO_SNDLOWAT 17
@@ -49,21 +49,58 @@
 #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT23
 #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK  24
 
-#define TARGET_SO_BINDTODEVICE 25
+#define TARGET_SO_BINDTODEVICE25
 
 /* Socket filtering */
-#define TARGET_SO_ATTACH_FILTER26
-#define TARGET_SO_DETACH_FILTER27
+#define TARGET_SO_ATTACH_FILTER   26
+#define TARGET_SO_DETACH_FILTER   27
+#define TARGET_SO_GET_FILTER  TARGET_SO_ATTACH_FILTER
 
-#define TARGET_SO_PEERNAME 28
-#define TARGET_SO_TIMESTAMP29
-#define TARGET_SCM_TIMESTAMP   TARGET_SO_TIMESTAMP
+#define TARGET_SO_PEERNAME28
+#define TARGET_SO_TIMESTAMP   29
+#define TARGET_SCM_TIMESTAMP  TARGET_SO_TIMESTAMP
 
-#define TARGET_SO_ACCEPTCONN   30
+#define TARGET_SO_ACCEPTCONN  30
 
-#define TARGET_SO_PEERSEC  31
+#define TARGET_SO_PEERSEC 31
+#define TARGET_SO_PASSSEC 34
+#define TARGET_SO_TIMESTAMPNS 35
+#define TARGET_SCM_TIMESTAMPNSTARGET_SO_TIMESTAMPNS
+
+#define TARGET_SO_MARK36
+
+#define TARGET_SO_TIMESTAMPING37
+#define TARGET_SCM_TIMESTAMPING   TARGET_SO_TIMESTAMPING
+
+#define TARGET_SO_PROTOCOL38
+#define TARGET_SO_DOMAIN  39
+
+#define TARGET_SO_RXQ_OVFL40
+
+#define TARGET_SO_WIFI_STATUS 41
+#define TARGET_SCM_WIFI_STATUSTARGET_SO_WIFI_STATUS
+#define TARGET_SO_PEEK_OFF42
+
+#define TARGET_SO_NOFCS   43
+#define TARGET_SO_LOCK_FILTER 44
+#define TARGET_SO_SELECT_ERR_QUEUE45
+#define TARGET_SO_BUSY_POLL   46
+#define TARGET_SO_MAX_PACING_RATE 47
+#define TARGET_SO_BPF_EXTENSIONS  48
+#define TARGET_SO_INCOMING_CPU49
+#define TARGET_SO_ATTACH_BPF  50
+#define TARGET_SO_DETACH_BPF  TARGET_SO_DETACH_FILTER
+#define TARGET_SO_ATTACH_REUSEPORT_CBPF   51
+#define TARGET_SO_ATTACH_REUSEPORT_EBPF   52
+#define TARGET_SO_CNX_ADVICE  53
+#define TARGET_SCM_TIMESTAMPING_OPT_STATS 54
+#define TARGET_SO_MEMINFO 55
+#define TARGET_SO_INCOMING_NAPI_ID56
+#define TARGET_SO_COOKIE  57
+#define TARGET_SCM_TIMESTAMPING_PKTINFO   58
+#define TARGET_SO_PEERGROUPS  59
+#define TARGET_SO_ZEROCOPY60
 
-#define TARGET_SO_PASSSEC  34
 #endif
 
 #ifndef ARCH_HAS_SOCKET_TYPES
@@ -94,6 +131,6 @@
 };
 
 #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK0xf  /* Covers up to TARGET_SOCK_MAX-1. */
+#define TARGET_SOCK_TYPE_MASK  0xf  /* Covers up to TARGET_SOCK_MAX - 1. */
 
 #endif
-- 
2.14.1

Re: [Qemu-devel] [PATCH v1 09/27] target/s390x: factor out handling of WAIT PSW into handle_wait()

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> This will now also detect crashes under TCG. We can directly use
> cpu->env.psw.addr instead of kvm_run, as we do a
> cpu_synchronize_state().
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.c   | 28 ++--
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c  | 15 +--
>  3 files changed, 24 insertions(+), 20 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 11/27] s390x/kvm: generalize SIGP stop and restart interrupt injection

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Preparation for factoring it out into !kvm code.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/internal.h  |  2 ++
>  target/s390x/interrupt.c | 20 
>  target/s390x/kvm-stub.c  |  8 
>  target/s390x/kvm.c   | 33 +
>  target/s390x/kvm_s390x.h |  2 ++
>  5 files changed, 53 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH] docker: Fix test-mingw

[Alistair Francis]

On Mon, Sep 25, 2017 at 1:29 AM, Fam Zheng  wrote:
> Feature "dtc" is explicitly required by test-mingw, but is not detected
> by the run script since we switched to archive-source.sh in b7f404201e4.
> Since it isn't available in the Fedora image which runs this test on
> patchew, the way we get dtc is still from submodule.
>
> archive-source.sh takes care of bundling the submodule files already, so
> what we need to do is just checking if files are there. Makefile is
> chosen because it is one that is unlikely to get renamed in the future.
>
> Signed-off-by: Fam Zheng 

Reviewed-by: Alistair Francis 

Thanks,
Alistair

> ---
>  tests/docker/run | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tests/docker/run b/tests/docker/run
> index c8f940de15..0fd2f358ce 100755
> --- a/tests/docker/run
> +++ b/tests/docker/run
> @@ -31,6 +31,9 @@ mkdir -p $TEST_DIR/{src,build,install}
>
>  # Extract the source tarballs
>  tar -C $TEST_DIR/src -xf $BASE/qemu.tar || prep_fail "Failed to untar source"
> +if test -f $TEST_DIR/src/Makefile; then
> +export FEATURES="$FEATURES dtc"
> +fi
>
>  if test -n "$SHOW_ENV"; then
>  if test -f /packages.txt; then
> --
> 2.13.5
>
>

[Qemu-devel] [PATCH v1 0/8] Remove some of the fprintf(stderr, "*

[Alistair Francis]

Continue on improving QEMUs logging/error messages by removing more
fprintf()'s.

Unfortunatley my Coccinelle skills aren't that great so it's all done in
some nasty regex and a little bit of manual work.



Alistair Francis (8):
  Replace all occurances of __FUNCTION__ with __func__
  tests: Replace fprintf(stderr, "*\n" with error_report()
  hw: Replace fprintf(stderr, "*\n" with error_report()
  block: Replace fprintf(stderr, "*\n" with error_report()
  util: Replace fprintf(stderr, "*\n" with error_report()
  ui: Replace fprintf(stderr, "*\n" with error_report()
  tcg: Replace fprintf(stderr, "*\n" with error_report()
  target: Replace fprintf(stderr, "*\n" with error_report()

 audio/audio_int.h  |   2 +-
 block/file-posix.c |   6 +-
 block/file-win32.c |   3 +-
 block/linux-aio.c  |   5 +-
 block/parallels.c  |   7 ++-
 block/qcow2-cluster.c  |   2 +-
 block/qcow2-refcount.c |  95 +++
 block/qcow2.c  |   8 +--
 block/quorum.c |   5 +-
 block/ssh.c|   4 +-
 block/vdi.c|  14 +++--
 block/vpc.c|   5 +-
 block/vvfat.c  |  99 ++---
 cpus.c |   8 +--
 exec.c |  14 ++---
 hw/arm/armv7m.c|   2 +-
 hw/arm/boot.c  |  34 +--
 hw/arm/gumstix.c   |  13 +++--
 hw/arm/mainstone.c |   7 ++-
 hw/arm/musicpal.c  |   2 +-
 hw/arm/nseries.c   |   2 +-
 hw/arm/omap1.c |  45 +++
 hw/arm/omap2.c |  23 
 hw/arm/omap_sx1.c  |   6 +-
 hw/arm/palm.c  |  20 +++
 hw/arm/pxa2xx.c|  53 +-
 hw/arm/pxa2xx_gpio.c   |   6 +-
 hw/arm/pxa2xx_pic.c|   4 +-
 hw/arm/stellaris.c |   3 +-
 hw/arm/tosa.c  |  25 +
 hw/arm/versatilepb.c   |   2 +-
 hw/arm/vexpress.c  |   8 +--
 hw/arm/z2.c|   6 +-
 hw/audio/hda-codec.c   |  10 ++--
 hw/audio/intel-hda.c   |  28 +-
 hw/audio/wm8750.c  |   4 +-
 hw/block/dataplane/virtio-blk.c|   6 +-
 hw/block/nand.c|   4 +-
 hw/block/onenand.c |  16 +++---
 hw/block/tc58128.c |  44 +++
 hw/bt/core.c   |  15 ++---
 hw/bt/hci-csr.c|  17 +++---
 hw/bt/hci.c|  48 
 hw/bt/hid.c|   4 +-
 hw/bt/l2cap.c  |  49 
 hw/bt/sdp.c|  11 ++--
 hw/char/exynos4210_uart.c  |   6 +-
 hw/char/mcf_uart.c |   5 +-
 hw/char/sh_serial.c|   9 +--
 hw/core/loader.c   |  31 ++-
 hw/core/ptimer.c   |   7 ++-
 hw/cris/axis_dev88.c   |   3 +-
 hw/cris/boot.c |   5 +-
 hw/display/blizzard.c  |  28 +-
 hw/display/omap_dss.c  |  16 +++---
 hw/display/pl110.c |   2 +-
 hw/display/pxa2xx_lcd.c|  14 ++---
 hw/display/qxl-render.c|   9 ++-
 hw/display/qxl.c   |  10 ++--
 hw/display/qxl.h   |   2 +-
 hw/display/tc6393xb.c  |  38 +++--
 hw/display/virtio-gpu-3d.c |   4 +-
 hw/display/vmware_vga.c|  22 
 hw/display/xenfb.c |   2 +-
 hw/dma/omap_dma.c  |  38 +++--
 hw/dma/pxa2xx_dma.c|  14 ++---
 hw/dma/soc_dma.c   |  37 ++--
 hw/gpio/max7310.c  |   8 +--
 hw/gpio/omap_gpio.c|   2 +-
 hw/i2c/omap_i2c.c  |  10 ++--
 hw/i386/kvm/apic.c |   9 +--
 hw/i386/kvm/clock.c|   7 ++-
 hw/i386/kvm/i8254.c|   7 ++-
 hw/i386/kvm/i8259.c|   5 +-
 hw/i386/kvm/ioapic.c   |   5 +-
 hw/i386/multiboot.c|  21 +++
 hw/i386/pc.c   |  18 +++---
 hw/i386/pc_piix.c  |   2 +-
 hw/i386/pc_sysfw.c |   5 +-
 hw/i386/xen/xen-hvm.c  |  32 ++-
 hw/i386/xen/xen-mapcache.c |  12 ++--
 hw/i386/xen/xen_apic.c |   2 

[Qemu-devel] [PULL 0/1] tcg patch queue

[Richard Henderson]

Just one patch, but should make stable 2.10.1 this week.


r~


The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2017-09-23 12:55:40 +0100)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20170925

for you to fetch changes up to 8b81253332b5a3f3c67b6462f39caef47a00dd29:

  accel/tcg/cputlb: avoid recursive BQL (fixes #1706296) (2017-09-25 11:23:30 
-0700)


BQL bug fix


Alex Bennée (1):
  accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)

 accel/tcg/cputlb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

[Qemu-devel] [PULL 1/1] accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)

[Richard Henderson]

From: Alex Bennée 

The mmio path (see exec.c:prepare_mmio_access) already protects itself
against recursive locking and it makes sense to do the same for
io_readx/writex. Otherwise any helper running in the BQL context will
assert when it attempts to write to device memory as in the case of
the bug report.

Reviewed-by: Peter Maydell 
Signed-off-by: Alex Bennée 
CC: Richard Jones 
CC: Paolo Bonzini 
CC: qemu-sta...@nongnu.org
Message-Id: <20170921110625.9500-1-alex.ben...@linaro.org>
Signed-off-by: Richard Henderson 
---
 accel/tcg/cputlb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e72415a882..bcbcc4db6c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -765,7 +765,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 
 cpu->mem_io_vaddr = addr;
 
-if (mr->global_locking) {
+if (mr->global_locking && !qemu_mutex_iothread_locked()) {
 qemu_mutex_lock_iothread();
 locked = true;
 }
@@ -800,7 +800,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 cpu->mem_io_vaddr = addr;
 cpu->mem_io_pc = retaddr;
 
-if (mr->global_locking) {
+if (mr->global_locking && !qemu_mutex_iothread_locked()) {
 qemu_mutex_lock_iothread();
 locked = true;
 }
-- 
2.13.5

Re: [Qemu-devel] xen/disk: don't leak stack data via response ring

[Stefano Stabellini]

On Sun, 24 Sep 2017, Michael Tokarev wrote:
> 23.09.2017 19:05, Michael Tokarev wrote:
> > 28.06.2017 01:04, Stefano Stabellini wrote:
> >> Rather than constructing a local structure instance on the stack, fill
> >> the fields directly on the shared ring, just like other (Linux)
> >> backends do. Build on the fact that all response structure flavors are
> >> actually identical (aside from alignment and padding at the end).
> >>
> >> This is XSA-216.
> >>
> >> Reported by: Anthony Perard 
> >> Signed-off-by: Jan Beulich 
> >> Signed-off-by: Stefano Stabellini 
> >> Acked-by: Anthony PERARD 
> > 
> > Reportedly, after this patch, HVM DomUs running with qemu-system-i386
> > (note i386, not x86_64), are leaking memory and host is running out of
> > memory rather fast.  See for example https://bugs.debian.org/871702
> 
> Looks like this is a false alarm, the problem actually is with
> 04bf2526ce87f21b32c9acba1c5518708c243ad0 (exec: use qemu_ram_ptr_length
> to access guest ram) without f5aa69bdc3418773f26747ca282c291519626ece
> (exec: Add lock parameter to qemu_ram_ptr_length).
> 
> I applied only 04bf2526ce87f to 2.8, without realizing that we also
> need f5aa69bdc3418).
> 
> Now when I try to backport f5aa69bdc3418 to 2.8 (on top of 04bf2526ce87f),
> I face an interesting logic without also applying 1ff7c5986a515d2d936eba0
> (xen/mapcache: store dma information in revmapcache entries for debugging),
> the arguments for xen_map_cache in qemu_ram_ptr_length() in these two patches
> are quite fun.. :)

Sorry about that. Let me know if you need any help with those backport.
Thank you!

- Stefano

Re: [Qemu-devel] [PATCH v2] hw/sd: fix out-of-bounds check for multi block reads

[Peter Maydell]

On 25 September 2017 at 23:53, Alistair Francis  wrote:
> On Mon, Sep 25, 2017 at 3:38 PM, Peter Maydell  
> wrote:
>> On 25 September 2017 at 22:16, Alistair Francis  wrote:
>>> On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
>>>  wrote:
 Alistair, were you planning to provide a reviewed-by: for this
 patch (or did you have more review comments on it)?
>>>
>>> Ah woops, this slipped through. Looks fine to me then.
>>>
>>> Reviewed-by: Alistair Francis 
>>
>> Cheers. Should we cc:stable on this too? I'm inclined towards 'yes'.
>
> Yeah, I don't see a reason not to.

Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v1 05/27] s390/tcg: turn INTERRUPT_EXT into a mask

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> +} else if (env->pending_int | INTERRUPT_EXT_FLOATING) {

Surely & here.

Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 10/27] s390x/kvm: pass ipb directly into handle_sigp()

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> No need to pass kvm_run.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/kvm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 08/27] s390x/tcg: a CPU cannot switch state due to an interrupt

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Going to OPERATING here looks wrong. A CPU should even never be
> !OPERATING at this point. Unhalting will already be done in
> cpu_handle_halt() if there is work, so we can drop this statement
> completely.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/excp_helper.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 16/27] s390x/kvm: factor out SIGP code into sigp.c

[Richard Henderson]

On 09/18/2017 09:00 AM, David Hildenbrand wrote:
> We want to use the same code base for TCG, so let's cleanly factor it
> out.
> 
> The sigp mutex is currently not really needed, as everything is
> protected by the iothread mutex. But this could change later, so leave
> it in place and initialize it properly from common code.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-virtio-ccw.c |   3 +
>  target/s390x/Makefile.objs |   1 +
>  target/s390x/cpu.c |   8 -
>  target/s390x/cpu.h |   6 +-
>  target/s390x/internal.h|   4 +
>  target/s390x/kvm-stub.c|   5 -
>  target/s390x/kvm.c | 349 +-
>  target/s390x/kvm_s390x.h   |   1 -
>  target/s390x/sigp.c| 366 
> +
>  target/s390x/trace-events  |   4 +-
>  10 files changed, 388 insertions(+), 359 deletions(-)
>  create mode 100644 target/s390x/sigp.c


Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v1 7/8] tcg: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Paolo Bonzini 
Cc: Peter Crosthwaite 
Cc: Richard Henderson 
Cc: Stefan Weil 
---

 cpus.c   |  8 
 exec.c   | 14 +++---
 tcg/optimize.c   |  8 
 tcg/tcg.c|  2 +-
 tcg/tcg.h|  3 ++-
 tcg/tci.c|  2 +-
 tcg/tci/tcg-target.inc.c |  4 ++--
 vl.c |  2 +-
 8 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/cpus.c b/cpus.c
index c9a624003a..784cee4848 100644
--- a/cpus.c
+++ b/cpus.c
@@ -258,7 +258,7 @@ int64_t cpu_get_icount_raw(void)
 
 if (cpu && cpu->running) {
 if (!cpu->can_do_io) {
-fprintf(stderr, "Bad icount read\n");
+error_report("Bad icount read");
 exit(1);
 }
 /* Take into account what has run */
@@ -1113,7 +1113,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 
 r = kvm_init_vcpu(cpu);
 if (r < 0) {
-fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
+error_report("kvm_init_vcpu failed: %s", strerror(-r));
 exit(1);
 }
 
@@ -1143,7 +1143,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 static void *qemu_dummy_cpu_thread_fn(void *arg)
 {
 #ifdef _WIN32
-fprintf(stderr, "qtest is not supported under Windows\n");
+error_report("qtest is not supported under Windows");
 exit(1);
 #else
 CPUState *cpu = arg;
@@ -1525,7 +1525,7 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
 #else /* _WIN32 */
 if (!qemu_cpu_is_self(cpu)) {
 if (!QueueUserAPC(dummy_apc_func, cpu->hThread, 0)) {
-fprintf(stderr, "%s: QueueUserAPC failed with error %lu\n",
+error_report("%s: QueueUserAPC failed with error %lu",
 __func__, GetLastError());
 exit(1);
 }
diff --git a/exec.c b/exec.c
index 7a80460725..f71b714b10 100644
--- a/exec.c
+++ b/exec.c
@@ -1045,7 +1045,7 @@ static RAMBlock *qemu_get_ram_block(ram_addr_t addr)
 }
 }
 
-fprintf(stderr, "Bad ram offset %" PRIx64 "\n", (uint64_t)addr);
+error_report("Bad ram offset %" PRIx64 "", (uint64_t)addr);
 abort();
 
 found:
@@ -1658,7 +1658,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
 }
 
 if (offset == RAM_ADDR_MAX) {
-fprintf(stderr, "Failed to find gap of requested size: %" PRIu64 "\n",
+error_report("Failed to find gap of requested size: %" PRIu64 "",
 (uint64_t)size);
 abort();
 }
@@ -1688,8 +1688,8 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t 
size)
 ret = qemu_madvise(addr, size, QEMU_MADV_DONTDUMP);
 if (ret) {
 perror("qemu_madvise");
-fprintf(stderr, "madvise doesn't support MADV_DONTDUMP, "
-"but dump_guest_core=off specified\n");
+error_report("madvise doesn't support MADV_DONTDUMP, "
+"but dump_guest_core=off specified");
 }
 }
 }
@@ -1725,7 +1725,7 @@ void qemu_ram_set_idstr(RAMBlock *new_block, const char 
*name, DeviceState *dev)
 RAMBLOCK_FOREACH(block) {
 if (block != new_block &&
 !strcmp(block->idstr, new_block->idstr)) {
-fprintf(stderr, "RAMBlock 

[Qemu-devel] [PATCH v1 4/8] block: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Stefan Weil 
Cc: Stefan Hajnoczi 
Cc: "Denis V. Lunev" 
Cc: Alberto Garcia 
Cc: "Richard W.M. Jones" 
Cc: Jeff Cody 
Cc: qemu-bl...@nongnu.org
---

 block/file-posix.c |  6 +--
 block/file-win32.c |  3 +-
 block/linux-aio.c  |  5 ++-
 block/parallels.c  |  7 ++--
 block/qcow2-cluster.c  |  2 +-
 block/qcow2-refcount.c | 95 
 block/qcow2.c  |  8 ++--
 block/quorum.c |  5 ++-
 block/ssh.c|  4 +-
 block/vdi.c| 14 ---
 block/vpc.c|  5 ++-
 block/vvfat.c  | 99 --
 12 files changed, 136 insertions(+), 117 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index ab12a2b591..2ea7a689cd 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -198,7 +198,7 @@ static int raw_normalize_devicepath(const char **filename)
 fname = *filename;
 dp = strrchr(fname, '/');
 if (lstat(fname, ) < 0) {
-fprintf(stderr, "%s: stat failed: %s\n",
+error_report("%s: stat failed: %s",
 fname, strerror(errno));
 return -errno;
 }
@@ -215,7 +215,7 @@ static int raw_normalize_devicepath(const char **filename)
 }
 fprintf(stderr, "%s is a block device", fname);
 *filename = namebuf;
-fprintf(stderr, ", using %s\n", *filename);
+error_report(", using %s", *filename);
 
 return 0;
 }
@@ -1499,7 +1499,7 @@ static int aio_worker(void *arg)
 ret = handle_aiocb_write_zeroes(aiocb);
 break;
 default:
-fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
+error_report("invalid aio request (0x%x)", aiocb->aio_type);
 ret = -EINVAL;
 break;
 }
diff --git a/block/file-win32.c b/block/file-win32.c
index 9e02214a69..a2ed9d2e55 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
 #include "block/block_int.h"
@@ -131,7 +132,7 @@ static int aio_worker(void *arg)
 }
 break;
 default:
-fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
+error_report("invalid aio request (0x%x)", aiocb->aio_type);
 ret = -EINVAL;
 break;
 }
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 88b8d55ec7..435c2ae47e 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -8,6 +8,7 @@
  * See the COPYING file in the top-level directory.
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "block/aio.h"
 #include "qemu/queue.h"
@@ -389,7 +390,7 @@ static int laio_do_submit(int fd, struct qemu_laiocb 
*laiocb, off_t offset,
break;
 /* Currently Linux kernel does not support other operations */
 default:
-fprintf(stderr, "%s: invalid AIO request type 0x%x.\n",
+error_report("%s: invalid AIO request type 0x%x.",
 __func__, type);
 return -EIO;
 }
@@ -499,7 +500,7 @@ void laio_cleanup(LinuxAioState *s)
 event_notifier_cleanup(>e);
 
 if (io_destroy(s->ctx) != 0) {
-

Re: [Qemu-devel] [PATCH 11/18] hbitmap: Add @advance param to hbitmap_iter_next()

[Max Reitz]

On 2017-09-25 17:38, Vladimir Sementsov-Ogievskiy wrote:
> 13.09.2017 21:19, Max Reitz wrote:
>> This new parameter allows the caller to just query the next dirty
>> position without moving the iterator.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>   include/qemu/hbitmap.h |  4 +++-
>>   block/dirty-bitmap.c   |  2 +-
>>   tests/test-hbitmap.c   | 26 +-
>>   util/hbitmap.c | 10 +++---
>>   4 files changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
>> index d3a74a21fc..6a52575ad5 100644
>> --- a/include/qemu/hbitmap.h
>> +++ b/include/qemu/hbitmap.h
>> @@ -316,11 +316,13 @@ void hbitmap_free_meta(HBitmap *hb);
>>   /**
>>    * hbitmap_iter_next:
>>    * @hbi: HBitmapIter to operate on.
>> + * @advance: If true, advance the iterator.  Otherwise, the next call
>> + *   of this function will return the same result.
> 
> it's not quit right, as hbitmap iterator allows concurrent resetting of
> bits, and in
> this case next call may return some other result. (see f63ea4e92bad1db)

Ah, right!  I think it should still be useful for what I (currently)
need in patch 12, I would just need a different description then.

(Like "...will return the same result (if that position is still dirty).")

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] hw/sd: fix out-of-bounds check for multi block reads

[Alistair Francis]

On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
 wrote:
> On 20 September 2017 at 07:19, Michael Olbrich  
> wrote:
>> On Tue, Sep 19, 2017 at 05:09:51PM -0700, Alistair Francis wrote:
>>> On Tue, Sep 19, 2017 at 1:23 AM, Michael Olbrich
>>>  wrote:
>>> > On Mon, Sep 18, 2017 at 02:28:26PM -0700, Alistair Francis wrote:
>>> >> On Sat, Sep 16, 2017 at 3:35 AM, Michael Olbrich
>>> >>  wrote:
>>> >> >  hw/sd/sd.c | 12 ++--
>>> >> >  1 file changed, 6 insertions(+), 6 deletions(-)
>>> >> >
>>> >> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> >> > index ba47bff4db80..35347a5bbcde 100644
>>> >> > --- a/hw/sd/sd.c
>>> >> > +++ b/hw/sd/sd.c
>>> >> > @@ -1797,8 +1797,13 @@ uint8_t sd_read_data(SDState *sd)
>>> >> >  break;
>>> >> >
>>> >> >  case 18:   /* CMD18:  READ_MULTIPLE_BLOCK */
>>> >> > -if (sd->data_offset == 0)
>>> >> > +if (sd->data_offset == 0) {
>>> >> > +if (sd->data_start + io_len > sd->size) {
>>> >> > +sd->card_status |= ADDRESS_ERROR;
>>> >> > +return 0x00;
>>> >> > +}
>>> >>
>>> >> Why move it inside the if (sd->data_offset == 0) and not just below
>>> >> the ret = sd->data[sd->data_offset ++] ?
>>> >>
>>> >> >  BLK_READ_BLOCK(sd->data_start, io_len);
>>> >
>>> > Mostly because of the line above. This copies the full block from the
>>> > backend storage to sd->data, so we need to make sure that the data is
>>> > actually available to fill sd->data, not if it's ok to access a certain
>>> > byte within sd->data.
>>>
>>> Doesn't this mean that the check is only done for the first block
>>> then? When data_offset is 0.
>>
>> No, data_offset is reset at the end of the block.
>> [...]
>
> Alistair, were you planning to provide a reviewed-by: for this
> patch (or did you have more review comments on it)?

Ah woops, this slipped through. Looks fine to me then.

Reviewed-by: Alistair Francis 

Thanks,
Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PULL 0/3] slirp updates

[Peter Maydell]

On 24 September 2017 at 19:08, Samuel Thibault
 wrote:
> warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2017-09-23 12:55:40 +0100)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 13146a83951e045c810c37c5c11c2a016ebc0663:
>
>   slirp: Add a special case for the NULL socket (2017-09-24 20:04:09 +0200)
>
> 
> slirp updates
>
> 
> Dr. David Alan Gilbert (1):
>   slirp: Add explanation for hostfwd parsing failure
>
> Kevin Cernekee (2):
>   slirp: Fix intermittent send queue hangs on a socket
>   slirp: Add a special case for the NULL socket

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v1 03/27] target/s390x: get rid of next_core_id

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
>  /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
> -cs->cpu_index = env->core_id;
> +cs->cpu_index = cpu->env.core_id;
> +#endif

Any reason not to drop core_id entirely in favour of cpu_index?
(Since cpu_index itself is generic and can't be dropped.)


r~

Re: [Qemu-devel] [PATCH v1 04/27] s390x: introduce and use S390_MAX_CPUS

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Will be handy in the next patches.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-virtio-ccw.c | 2 +-
>  target/s390x/cpu.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 12/18] block/dirty-bitmap: Add bdrv_dirty_iter_next_area

[Max Reitz]

On 2017-09-25 17:49, Vladimir Sementsov-Ogievskiy wrote:
> I have a patch on list, which adds hbitmap_next_zero function, it may help
> https://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg00809.html

Hmmm.  Sounds good, but (1) I would need to directly access the bitmap
instead of the iterator, and (2) I would still need to clear the whole
in the iterator...

It does sound tempting because I could drop the previous patch, then
(and thus wouldn't have to worry about concurrent resetting), but I
don't think the whole implementation would be simpler.

I'll think about it, but thanks for pointing it out in any case!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] virtio/vhost: reset dev->log after syncing

[Michael Roth]

Quoting Felipe Franciosi (2017-09-20 13:53:06)
> vhost_log_put() is called to decomission the dirty log between qemu and
> a vhost device when stopping the device. Such a call can happen from
> migration_completion().
> 
> Present code sets dev->log_size to zero too early in vhost_log_put(),
> causing the sync check to always return false. As a consequence, the
> last pass on the dirty bitmap never happens at the end of migration.
> 
> If a vhost device was busy (writing to guest memory) until the last
> moments before vhost_virtqueue_stop(), this error will result in guest
> memory corruption (at least) following migrations.
> 
> Signed-off-by: Felipe Franciosi 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  hw/virtio/vhost.c |5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 5fd69f0..ddc42f0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -375,8 +375,6 @@ static void vhost_log_put(struct vhost_dev *dev, bool 
> sync)
>  if (!log) {
>  return;
>  }
> -dev->log = NULL;
> -dev->log_size = 0;
> 
>  --log->refcnt;
>  if (log->refcnt == 0) {
> @@ -396,6 +394,9 @@ static void vhost_log_put(struct vhost_dev *dev, bool 
> sync)
> 
>  g_free(log);
>  }
> +
> +dev->log = NULL;
> +dev->log_size = 0;
>  }
> 
>  static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
> -- 
> 1.7.1
> 
> 

Re: [Qemu-devel] [PATCH v10 05/20] dirty-bitmap: Avoid size query failure during truncate

[John Snow]

On 09/25/2017 10:55 AM, Eric Blake wrote:
> We've previously fixed several places where we failed to account
> for possible errors from bdrv_nb_sectors().  Fix another one by
> making bdrv_dirty_bitmap_truncate() take the new size from the
> caller instead of querying itself; then adjust the sole caller
> bdrv_truncate() to pass the size just determined by a successful
> resize, or to skip the bitmap resize on failure, thus avoiding
> sizing the bitmaps to -1.  This also fixes a bug where not all
> failure paths in bdrv_truncate() would set errp.
> 
> Note that bdrv_truncate() is still a bit awkward.  We may want
> to revisit it later and clean up things to better guarantee that
> a resize attempt either fails cleanly up front, or cannot fail
> after guest-visible changes have been made (if temporary changes
> are made, then they need to be cleanly rolled back).  But that
> is a task for another day; for now, the goal is the bare minimum
> fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v10: always resize bitmap as before [John], enhance commit message to
> point out errp bugfix [Vladimir]
> v9: skip only bdrv_dirty_bitmap_truncate on error [Fam]
> v8: retitle and rework to avoid possibility of secondary failure [John]
> v7: new patch [Kevin]
> ---
>  include/block/dirty-bitmap.h |  2 +-
>  block.c  | 16 +++-
>  block/dirty-bitmap.c |  6 +++---
>  3 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 8fd842eac9..7a27590047 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>  void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
>  bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>  bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>  bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
> diff --git a/block.c b/block.c
> index 528cda7b2c..ef5af81f66 100644
> --- a/block.c
> +++ b/block.c
> @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
> PreallocMode prealloc,
>  assert(!(bs->open_flags & BDRV_O_INACTIVE));
> 
>  ret = drv->bdrv_truncate(bs, offset, prealloc, errp);
> -if (ret == 0) {
> -ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> -bdrv_dirty_bitmap_truncate(bs);
> -bdrv_parent_cb_resize(bs);
> -atomic_inc(>write_gen);
> +if (ret < 0) {
> +return ret;
>  }
> +ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> +if (ret < 0) {
> +error_setg_errno(errp, -ret, "Could not refresh total sector count");
> +} else {
> +offset = bs->total_sectors * BDRV_SECTOR_SIZE;
> +}
> +bdrv_dirty_bitmap_truncate(bs, offset);
> +bdrv_parent_cb_resize(bs);
> +atomic_inc(>write_gen);
>  return ret;
>  }
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 42a55e4a4b..ee164fb518 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -1,7 +1,7 @@
>  /*
>   * Block Dirty Bitmap
>   *
> - * Copyright (c) 2016 Red Hat. Inc
> + * Copyright (c) 2016-2017 Red Hat. Inc
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>   * of this software and associated documentation files (the "Software"), to 
> deal
> @@ -302,10 +302,10 @@ BdrvDirtyBitmap 
> *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
>   * Truncates _all_ bitmaps attached to a BDS.
>   * Called with BQL taken.
>   */
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes)
>  {
>  BdrvDirtyBitmap *bitmap;
> -uint64_t size = bdrv_nb_sectors(bs);
> +int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE);
> 
>  bdrv_dirty_bitmaps_lock(bs);
>  QLIST_FOREACH(bitmap, >dirty_bitmaps, list) {
> 

I *THINK* this is the most correct we can do for now...

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH] block/throttle-groups.c: allocate RestartData on the heap

[Michael Roth]

Quoting Manos Pitsidianakis (2017-09-18 15:25:29)
> RestartData is the opaque data of the throttle_group_restart_queue_entry
> coroutine. By being stack allocated, it isn't available anymore if
> aio_co_enter schedules the coroutine with a bottom halve and runs after
> throttle_group_restart_queue returns.
> 
> Signed-off-by: Manos Pitsidianakis 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  block/throttle-groups.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index 6ba992c8d7..b291a88481 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -403,17 +403,19 @@ static void coroutine_fn 
> throttle_group_restart_queue_entry(void *opaque)
>  schedule_next_request(tgm, is_write);
>  qemu_mutex_unlock(>lock);
>  }
> +
> +g_free(data);
>  }
> 
>  static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool 
> is_write)
>  {
>  Coroutine *co;
> -RestartData rd = {
> -.tgm = tgm,
> -.is_write = is_write
> -};
> +RestartData *rd = g_new0(RestartData, 1);
> 
> -co = qemu_coroutine_create(throttle_group_restart_queue_entry, );
> +rd->tgm = tgm;
> +rd->is_write = is_write;
> +
> +co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd);
>  aio_co_enter(tgm->aio_context, co);
>  }
> 
> -- 
> 2.11.0
> 
> 

Re: [Qemu-devel] [PATCH] block/qcow2-bitmap: fix use of uninitialized pointer

[Michael Roth]

Quoting Vladimir Sementsov-Ogievskiy (2017-09-22 09:43:53)
> Without initialization to zero dirty_bitmap field may be not zero
> for a bitmap which should not be stored and
> qcow2_store_persistent_dirty_bitmaps will erroneously call
> store_bitmap for it which leads to SYGSEGV on bdrv_dirty_bitmap_name.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  block/qcow2-bitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index e8d3bdbd6e..14f41d0427 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -602,7 +602,7 @@ static Qcow2BitmapList *bitmap_list_load(BlockDriverState 
> *bs, uint64_t offset,
>  goto fail;
>  }
> 
> -bm = g_new(Qcow2Bitmap, 1);
> +bm = g_new0(Qcow2Bitmap, 1);
>  bm->table.offset = e->bitmap_table_offset;
>  bm->table.size = e->bitmap_table_size;
>  bm->flags = e->flags;
> -- 
> 2.11.1
> 
> 

Re: [Qemu-devel] [PATCH v2 2/2] qcow2: truncate the tail of the image file after shrinking the image

[John Snow]

On 09/22/2017 05:39 AM, Pavel Butsykin wrote:
> Now after shrinking the image, at the end of the image file, there might be a
> tail that probably will never be used. So we can find the last used cluster 
> and
> cut the tail.
> 
> Signed-off-by: Pavel Butsykin 


Reviewed-by: John Snow 

[Qemu-devel] [PATCH v2 6/6] linux_user: consolidate sock_type

[Carlo Marcelo Arenas Belón]

remove unnecessary sock_type enum and other unused surrounding code to
allow for per arch sockbits to mirror better linux headers for maintenance

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 linux-user/alpha/sockbits.h | 36 
 linux-user/hppa/sockbits.h  | 30 ---
 linux-user/mips/sockbits.h  | 35 ---
 linux-user/socket.h | 58 ++---
 linux-user/sparc/sockbits.h | 35 ---
 5 files changed, 29 insertions(+), 165 deletions(-)

diff --git a/linux-user/alpha/sockbits.h b/linux-user/alpha/sockbits.h
index 768579a1f7..defdb806ea 100644
--- a/linux-user/alpha/sockbits.h
+++ b/linux-user/alpha/sockbits.h
@@ -66,39 +66,3 @@
 #define TARGET_SCM_TIMESTAMPING_PKTINFO 58
 #define TARGET_SO_PEERGROUPS59
 #define TARGET_SO_ZEROCOPY  60
-
-/** sock_type - Socket types
- *
- * Please notice that for binary compat reasons ALPHA has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-
-enum sock_type {
-TARGET_SOCK_STREAM  = 1,
-TARGET_SOCK_DGRAM   = 2,
-TARGET_SOCK_RAW = 3,
-TARGET_SOCK_RDM = 4,
-TARGET_SOCK_SEQPACKET   = 5,
-TARGET_SOCK_DCCP= 6,
-TARGET_SOCK_PACKET  = 10,
-TARGET_SOCK_CLOEXEC = 01000,
-TARGET_SOCK_NONBLOCK= 0x4000,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK 0xf  /* Covers up to TARGET_SOCK_MAX - 1. */
-
-#define ARCH_HAS_SOCKET_TYPES 1
diff --git a/linux-user/hppa/sockbits.h b/linux-user/hppa/sockbits.h
index 3dab31a76a..32f81357d6 100644
--- a/linux-user/hppa/sockbits.h
+++ b/linux-user/hppa/sockbits.h
@@ -71,33 +71,3 @@
 #define TARGET_SO_PEERGROUPS0x4034
 #define TARGET_SO_ZEROCOPY  0x4035
 
-/** sock_type - Socket types - default values
- *
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-enum sock_type {
-TARGET_SOCK_STREAM= 1,
-TARGET_SOCK_DGRAM = 2,
-TARGET_SOCK_RAW   = 3,
-TARGET_SOCK_RDM   = 4,
-TARGET_SOCK_SEQPACKET = 5,
-TARGET_SOCK_DCCP  = 6,
-TARGET_SOCK_PACKET= 10,
-TARGET_SOCK_CLOEXEC   = 01000,
-TARGET_SOCK_NONBLOCK  = 0x4000,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK 0xf  /* Covers up to TARGET_SOCK_MAX - 1. */
-
-#define ARCH_HAS_SOCKET_TYPES 1
diff --git a/linux-user/mips/sockbits.h b/linux-user/mips/sockbits.h
index 6d8ea8aba2..fa8062391d 100644
--- a/linux-user/mips/sockbits.h
+++ b/linux-user/mips/sockbits.h
@@ -70,38 +70,3 @@
 #define TARGET_SCM_TIMESTAMPING_PKTINFO 58
 #define TARGET_SO_PEERGROUPS59
 #define TARGET_SO_ZEROCOPY  60
-
-/** sock_type - Socket types
- *
- * Please notice that for binary compat reasons MIPS has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-enum sock_type {
-TARGET_SOCK_DGRAM   = 1,
-TARGET_SOCK_STREAM  = 2,
-TARGET_SOCK_RAW = 3,
-TARGET_SOCK_RDM = 4,
-TARGET_SOCK_SEQPACKET   = 5,
-TARGET_SOCK_DCCP= 6,
-

Re: [Qemu-devel] [PATCH v1 07/27] s390x/tcg: STOPPED cpus can never wake up

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Interrupts can't wake such CPUs up. SIGP from other CPUs has to be used
> to toggle the state.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.c | 6 ++
>  1 file changed, 6 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 0/7] mac_dbdma: tidy-up and QOMify

[David Gibson]

On Sun, Sep 24, 2017 at 03:47:39PM +0100, Mark Cave-Ayland wrote:
> Whilst looking at implementing another DBDMA device for the Mac machines
> I noticed a couple of things: firstly there were some unused fields still
> in DBDMAState, and secondly the existing code still used global functions
> to register DMA channels and handle the relationship between macio IDE and
> DBDMA.
> 
> This patchset removes the now-unused fields from DBDMA state, QOMifys the
> DBDMA device, uses a QOM object link to allow the macio IDE object to
> reference the DBDMA device, and then finally removes the global DBDMA_*
> functions substituting them instead for QOM methods.
> 
> Note: this patchset does not apply to master but on top of David's
> ppc-for-2.11 branch since there are merge conflicts with my previous
> patchset. Hopefully the Based-On line below is enough to keep patchew
> happy, even though it wasn't the final version applied to the ppc-for-2.11
> branch.
> 
> Signed-off-by: Mark Cave-Ayland 
> Based-on: 1505668548-16616-1-git-send-email-mark.cave-ayl...@ilande.co.uk 
> (ppc: more Mac-related fixups)

I've applied 1-5/7.  There are a couple of details I'm not 100%
convinced on, but it's still better than what was there before.  6 & 7
I'm still thinking about.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 6/8] arm: Support Capstone in disas_set_info

[Alex Bennée]

Richard Henderson  writes:

> Cc: qemu-...@nongnu.org
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 
Tested-by: Alex Bennée 

And BTW better than libvixl at least w.r.t wfi.

> ---
>  disas.c  |  3 +++
>  target/arm/cpu.c | 21 ++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 42fae735ee..ea295f9cfc 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -451,6 +451,7 @@ void disas(FILE *out, void *code, unsigned long size)
>  print_insn = print_insn_ppc;
>  #elif defined(__aarch64__) && defined(CONFIG_ARM_A64_DIS)
>  print_insn = print_insn_arm_a64;
> +s.info.cap_arch = CS_ARCH_ARM64;
>  #elif defined(__alpha__)
>  print_insn = print_insn_alpha;
>  #elif defined(__sparc__)
> @@ -458,6 +459,8 @@ void disas(FILE *out, void *code, unsigned long size)
>  s.info.mach = bfd_mach_sparc_v9b;
>  #elif defined(__arm__)
>  print_insn = print_insn_arm;
> +s.info.cap_arch = CS_ARCH_ARM;
> +/* TCG only generates code for arm mode.  */
>  #elif defined(__MIPSEB__)
>  print_insn = print_insn_big_mips;
>  #elif defined(__MIPSEL__)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 412e94c7ad..53320709ac 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -33,6 +33,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/hw_accel.h"
>  #include "kvm_arm.h"
> +#include "disas/capstone.h"
>
>  static void arm_cpu_set_pc(CPUState *cs, vaddr value)
>  {
> @@ -482,10 +483,24 @@ static void arm_disas_set_info(CPUState *cpu, 
> disassemble_info *info)
>  #if defined(CONFIG_ARM_A64_DIS)
>  info->print_insn = print_insn_arm_a64;
>  #endif
> -} else if (env->thumb) {
> -info->print_insn = print_insn_thumb1;
> +info->cap_arch = CS_ARCH_ARM64;
>  } else {
> -info->print_insn = print_insn_arm;
> +int cap_mode;
> +if (env->thumb) {
> +info->print_insn = print_insn_thumb1;
> +cap_mode = CS_MODE_THUMB;
> +} else {
> +info->print_insn = print_insn_arm;
> +cap_mode = CS_MODE_ARM;
> +}
> +if (arm_feature(env, ARM_FEATURE_V8)) {
> +cap_mode |= CS_MODE_V8;
> +}
> +if (arm_feature(env, ARM_FEATURE_M)) {
> +cap_mode |= CS_MODE_MCLASS;
> +}
> +info->cap_arch = CS_ARCH_ARM;
> +info->cap_mode = cap_mode;
>  }
>  if (bswap_code(arm_sctlr_b(env))) {
>  #ifdef TARGET_WORDS_BIGENDIAN


--
Alex Bennée

[Qemu-devel] [PATCH v1 1/8] Replace all occurances of __FUNCTION__ with __func__

[Alistair Francis]

Replace all occurs of __FUNCTION__ except for the check in checkpatch
with the non GCC specific __func__.

One line in hcd-musb.c was manually tweaked to pass checkpatch.

Signed-off-by: Alistair Francis 
Cc: Gerd Hoffmann 
Cc: Andrzej Zaborowski 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: John Snow 
Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Cc: Peter Crosthwaite 
Cc: Stefan Hajnoczi  (supporter:Block
Cc: Fam Zheng  (supporter:Block
Cc: Juan Quintela 
Cc: "Dr. David Alan Gilbert" 
Cc: qemu-...@nongnu.org
Cc: qemu-bl...@nongnu.org
Cc: xen-de...@lists.xenproject.org
---

 audio/audio_int.h  |  2 +-
 hw/arm/nseries.c   |  2 +-
 hw/arm/omap1.c | 42 +-
 hw/arm/omap2.c | 12 ++--
 hw/arm/palm.c  | 14 +++---
 hw/arm/pxa2xx.c| 46 +++---
 hw/arm/pxa2xx_gpio.c   |  6 +++---
 hw/arm/pxa2xx_pic.c|  4 ++--
 hw/arm/tosa.c  | 10 +-
 hw/audio/hda-codec.c   | 10 +-
 hw/audio/intel-hda.c   | 28 ++--
 hw/audio/wm8750.c  |  4 ++--
 hw/block/nand.c|  4 ++--
 hw/block/onenand.c |  8 
 hw/bt/core.c   | 10 +-
 hw/bt/hci-csr.c| 14 +++---
 hw/bt/hci.c| 26 +-
 hw/bt/hid.c|  2 +-
 hw/bt/l2cap.c  | 22 +++---
 hw/bt/sdp.c|  6 +++---
 hw/display/blizzard.c  | 18 +-
 hw/display/omap_dss.c  |  6 +++---
 hw/display/pxa2xx_lcd.c| 14 +++---
 hw/display/qxl-render.c|  6 +++---
 hw/display/qxl.h   |  2 +-
 hw/display/tc6393xb.c  |  2 +-
 hw/display/xenfb.c |  2 +-
 hw/dma/omap_dma.c  | 26 +-
 hw/dma/pxa2xx_dma.c| 14 +++---
 hw/gpio/max7310.c  |  8 
 hw/gpio/omap_gpio.c|  2 +-
 hw/i2c/omap_i2c.c  |  6 +++---
 hw/ide/ahci.c  |  2 +-
 hw/ide/microdrive.c|  4 ++--
 hw/input/lm832x.c  |  6 +++---
 hw/input/pxa2xx_keypad.c   |  6 +++---
 hw/input/tsc2005.c |  8 
 hw/input/tsc210x.c |  4 ++--
 hw/intc/omap_intc.c|  2 +-
 hw/isa/vt82c686.c  |  2 +-
 hw/mips/gt64xxx_pci.c  |  2 +-
 hw/misc/cbus.c | 12 ++--
 hw/misc/omap_clk.c |  4 ++--
 hw/misc/omap_gpmc.c|  6 +++---
 hw/misc/omap_l4.c  |  4 ++--
 hw/misc/omap_sdrc.c|  2 +-
 hw/misc/omap_tap.c |  6 +++---
 hw/misc/tmp105.c   |  2 +-
 hw/pci-host/bonito.c   |  2 +-
 hw/sd/pxa2xx_mmci.c|  6 +++---
 hw/ssi/omap_spi.c  |  6 +++---
 hw/timer/omap_gptimer.c|  6 +++---
 hw/timer/twl92230.c|  6 +++---
 hw/usb/desc.c  |  2 +-
 hw/usb/dev-bluetooth.c |  4 ++--
 hw/usb/hcd-musb.c  |  4 ++--
 hw/usb/tusb6010.c  | 14 +++---
 hw/xenpv/xen_domainbuild.c | 16 
 hw/xenpv/xen_machine_pv.c  |  2 +-
 include/hw/arm/omap.h  | 10 +-
 include/hw/arm/sharpsl.h   |  2 +-
 memory_mapping.c   |  2 +-
 migration/block.c  |  4 ++--
 ui/cursor.c|  6 +++---
 ui/spice-display.c |  4 ++--
 65 files changed, 273 insertions(+), 273 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index 5bcb1c60e1..543b1bd8d5 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -253,7 +253,7 @@ static inline int audio_ring_dist (int dst, int src, int 
len)
 #define AUDIO_STRINGIFY(n) AUDIO_STRINGIFY_(n)
 
 #if defined _MSC_VER || defined __GNUC__
-#define AUDIO_FUNC __FUNCTION__
+#define AUDIO_FUNC __func__
 #else
 #define AUDIO_FUNC __FILE__ ":" AUDIO_STRINGIFY (__LINE__)
 #endif
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 58005b6619..32687afced 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -463,7 +463,7 @@ static uint32_t mipid_txrx(void *opaque, uint32_t cmd, int 
len)
 uint8_t ret;
 
 if (len > 9) {
-hw_error("%s: FIXME: bad SPI word width %i\n", __FUNCTION__, len);
+hw_error("%s: FIXME: bad SPI word width %i\n", __func__, len);
 }
 
 if (s->p >= ARRAY_SIZE(s->resp)) {
diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index b3e7625130..1388200191 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -999,7 +999,7 @@ static uint64_t omap_id_read(void *opaque, hwaddr addr,
 case omap1510:
 return 0x03310115;
 default:
-hw_error("%s: bad mpu model\n", __FUNCTION__);
+hw_error("%s: bad mpu model\n", __func__);
 }
 break;
 

Re: [Qemu-devel] [PATCH 1/2] fprintf-fn.h uses GCC_FMT, but doesn't include the definition of it

[Daniel Loffgren]

I am working on getting ppc-darwin-user working again, and this was one of the 
many things preventing it from compiling. Your explanation sounds correct. I 
believe it was the lack of osdep.h in the right .c files, since I added osdep.h 
to the tops of all of the necessary files well after this change (I was fixing 
things in order of compiler failures). I dropped this commit from my branch and 
it didn’t break. So, feel free to disregard this.

> On Sep 25, 2017, at 9:18 AM, Eric Blake  wrote:
> 
> On 09/24/2017 08:02 PM, Daniel Loffgren wrote:
>> 
>> Signed-off-by: Daniel Loffgren 
> 
> Hmm - you've identified a file with no listed maintainer.  But
> qemu-trivial does seem like the right place to include it.
> 
> Generally, we like patches to call out the topic that it is touching;
> also, the subject line should focus on the what, while the body focuses
> on the why.  So a better commit message might be:
> 
> maint: Make fprintf-fn.h self-contained
> 
> Include the necessary headers so that GCC_FMT_ATTR is defined regardless
> of what client files use fprintf-fn.h.
> 
> 
> However, after saying that, I think your patch is not needed.  Per
> HACKING, _all_ .c files must include osdep.h first, and osdep.h already
> includes compiler.h, therefore, any .c file that uses fprintf-fn.h
> already has GCC_FMT_ATTR in scope by the time it gets to the
> fprintf_function typedef.  If you ran into a situation where you had a
> compile failure, please post more details of what failed for you, in
> case the problem was you forgetting to use osdep.h.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

Re: [Qemu-devel] [PATCH 2/2] thunk.h uses TARGET_ABI_BITS without including abitypes.h

[Daniel Loffgren]

> On Sep 25, 2017, at 9:10 AM, Eric Blake  wrote:
> 
> On 09/24/2017 08:02 PM, Daniel Loffgren wrote:
>> 
>> Signed-off-by: Daniel Loffgren > >
>> ---
>> include/exec/user/thunk.h | 1 +
>> 1 file changed, 1 insertion(+)
> 
> meta-comment: your patch is titled 2/2, but was sent as its own
> top-level thread (it is missing In-Reply-To: and References: headers).
> When sending 2 patches as a series, it is important to include a 0/2
> cover letter, and to properly thread things so that both 1/2 and 2/2 are
> in-reply-to the 0/2 cover letter.  'git send-email' is probably the
> easiest way to get this to work.  More patch submission hints at:
> 
> https://wiki.qemu.org/Contribute/SubmitAPatch 
> 

Oops, sorry about that! I used 'git format-patch' to get this email from a set 
of two commits, and sent each as-is. This commit can totally stand alone.

>> 
>> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
>> index f19ef4b230..12b5449d8c 100644
>> --- a/include/exec/user/thunk.h
>> +++ b/include/exec/user/thunk.h
>> @@ -19,6 +19,7 @@
>> #ifndef THUNK_H
>> #define THUNK_H
>> 
>> +#include "abitypes.h"
>> #include "cpu.h"
>> 
>> /* types enums definitions */
>> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org  | libvirt.org 
> 

[Qemu-devel] [PATCH v3 2/4] iothread: provide helpers for internal use

[Peter Xu]

IOThread is a general framework that contains IO loop environment and a
real thread behind.  It's also good to be used internally inside qemu.
Provide some helpers for it to create iothreads to be used internally.

Put all the internal used iothreads into the internal object container.

Signed-off-by: Peter Xu 
---
 include/sysemu/iothread.h |  8 
 iothread.c| 16 
 2 files changed, 24 insertions(+)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index d2985b3..b07663f 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -46,4 +46,12 @@ AioContext *iothread_get_aio_context(IOThread *iothread);
 void iothread_stop_all(void);
 GMainContext *iothread_get_g_main_context(IOThread *iothread);
 
+/*
+ * Helpers used to allocate iothreads for internal use.  These
+ * iothreads will not be seen by monitor clients when query using
+ * "query-iothreads".
+ */
+IOThread *iothread_create(const char *id, Error **errp);
+void iothread_destroy(IOThread *iothread);
+
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index 44c8944..33f996e 100644
--- a/iothread.c
+++ b/iothread.c
@@ -354,3 +354,19 @@ GMainContext *iothread_get_g_main_context(IOThread 
*iothread)
 
 return iothread->worker_context;
 }
+
+IOThread *iothread_create(const char *id, Error **errp)
+{
+Object *obj;
+
+obj = object_new_with_props(TYPE_IOTHREAD,
+object_get_internal_root(),
+id, errp, NULL);
+
+return IOTHREAD(obj);
+}
+
+void iothread_destroy(IOThread *iothread)
+{
+object_unparent(OBJECT(iothread));
+}
-- 
2.7.4

[Qemu-devel] [PATCH v3 1/4] qom: provide root container for internal objs

[Peter Xu]

We have object_get_objects_root() to keep user created objects, however
no place for objects that will be used internally.  Create such a
container for internal objects.

CC: Andreas Färber 
CC: Markus Armbruster 
CC: Paolo Bonzini 
Suggested-by: Daniel P. Berrange 
Signed-off-by: Peter Xu 
---
 include/qom/object.h | 10 ++
 qom/object.c | 11 +++
 2 files changed, 21 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index f3e5cff..f567052 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1214,6 +1214,16 @@ Object *object_get_root(void);
 Object *object_get_objects_root(void);
 
 /**
+ * object_get_internal_root:
+ *
+ * Get the container object that holds internally used object
+ * instances. This is the object at path "/internal-objects"
+ *
+ * Returns: the internal object container
+ */
+Object *object_get_internal_root(void);
+
+/**
  * object_get_canonical_path_component:
  *
  * Returns: The final component in the object's canonical path.  The canonical
diff --git a/qom/object.c b/qom/object.c
index 3e18537..6a7bd92 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1370,6 +1370,17 @@ Object *object_get_objects_root(void)
 return container_get(object_get_root(), "/objects");
 }
 
+Object *object_get_internal_root(void)
+{
+static Object *internal_root;
+
+if (!internal_root) {
+internal_root = object_new("container");
+}
+
+return internal_root;
+}
+
 static void object_get_child_property(Object *obj, Visitor *v,
   const char *name, void *opaque,
   Error **errp)
-- 
2.7.4

[Qemu-devel] [PATCH v3 0/4] iothread: allow to create internal iothreads

[Peter Xu]

v3:
- pick up r-bs (one missing for Fam's on last patch)
- fix patch 1 to create isolated internal container

v2:
- add one patch to provide object_get_internal_root() [Daniel]
- patch 2: use the new object_get_internal_root()
- patch 3: fix commit message, "reentrant" is wrongly used by me. it
  should be "called multiple times"; move iothread->ctx check into
  iothread_stop() [Fam]
- patch 4: add one paragraph in commit message, mention about the glib
  issue. [Fam]

When trying to support monitor OOB (out-of-band) commands, I found
that the monitor IO thread I did looks just like iothread.  It would
be best if I can use iothread directly.  However it seems that it was
mostly used by "-object iothread" before but not friendly to internal
usages.  This series tries to export essential functions to do it.

Also, I think patch 2 also fixes a bug in iothread_stop().

Please review. Thanks.

Peter Xu (4):
  qom: provide root container for internal objs
  iothread: provide helpers for internal use
  iothread: export iothread_stop()
  iothread: delay the context release to finalize

 include/qom/object.h  | 10 ++
 include/sysemu/iothread.h |  9 +
 iothread.c| 46 --
 qom/object.c  | 11 +++
 4 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default

[Bharata B Rao]

On Fri, Sep 22, 2017 at 11:16:34AM +0200, Thomas Huth wrote:
> Historically we've marked all devices as hotpluggable by default. However,
> most devices are not hotpluggable, and you also need a HotplugHandler to
> support these devices. So if the user tries to "device_add" or "device_del"
> such a non-hotpluggable device during runtime, either nothing really usable
> happens, or QEMU even crashes/aborts unexpectedly (see for example commit
> 84ebd3e8c7d4fe955b - "Mark diag288 watchdog as non-hotpluggable").
> So let's change this dangerous default behaviour and mark the devices as
> non-hotpluggable by default. Certain parent devices classes which are known
> as hotpluggable (e.g. PCI, USB, etc.) are marked with "hotpluggable = true",
> so that devices that are derived from these classes continue to work as
> expected.

I see that the discussion has moved on, but want to note here that
CPU hotplug on pseries breaks with this patch.

(qemu) device_add host-spapr-cpu-core,core-id=8,id=core8
Device 'host-powerpc64-cpu' does not support hotplugging

(qemu) device_add POWER8E_v2.1-spapr-cpu-core,id=core8,core-id=8
Device 'POWER8E_v2.1-powerpc64-cpu' does not support hotplugging

Hope I am not missing anything.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:16PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Route the errors from vsmtate_save_state back up through
> vmstate_save and out to the normal device state path.
> That's the normal error path done.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:17PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> vmstate_save is called in a few places, and vmstate_save_state is
> called in lots of places.

vmstate_save() should have been processed with previous patch?

> 
> Route error returns from the easier cases back up;  there are lots
> of more complex cases where their own error paths need fixing.
> 
> Signed-off-by: Dr. David Alan Gilbert 

The patch content wires quite a few vmstate_save_state() callers, so
the patch itself makes sense to me.

Reviewed-by: Peter Xu 

> ---
>  hw/display/virtio-gpu.c|  4 +---
>  hw/virtio/virtio.c | 13 +++--
>  include/hw/virtio/virtio.h |  2 +-
>  migration/vmstate-types.c  | 11 ---
>  tests/test-vmstate.c   |  9 ++---
>  5 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 3a8f1e1a2d..2becbfda59 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1050,9 +1050,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, 
> size_t size,
>  }
>  qemu_put_be32(f, 0); /* end of list */
>  
> -vmstate_save_state(f, _virtio_gpu_scanouts, g, NULL);
> -
> -return 0;
> +return vmstate_save_state(f, _virtio_gpu_scanouts, g, NULL);
>  }
>  
>  static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 3129d25c00..311929e9df 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1897,7 +1897,7 @@ static const VMStateDescription vmstate_virtio = {
>  }
>  };
>  
> -void virtio_save(VirtIODevice *vdev, QEMUFile *f)
> +int virtio_save(VirtIODevice *vdev, QEMUFile *f)
>  {
>  BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> @@ -1947,20 +1947,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>  }
>  
>  if (vdc->vmsd) {
> -vmstate_save_state(f, vdc->vmsd, vdev, NULL);
> +int ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL);
> +if (ret) {
> +return ret;
> +}
>  }
>  
>  /* Subsections */
> -vmstate_save_state(f, _virtio, vdev, NULL);
> +return vmstate_save_state(f, _virtio, vdev, NULL);
>  }
>  
>  /* A wrapper for use as a VMState .put function */
>  static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,
>VMStateField *field, QJSON *vmdesc)
>  {
> -virtio_save(VIRTIO_DEVICE(opaque), f);
> -
> -return 0;
> +return virtio_save(VIRTIO_DEVICE(opaque), f);
>  }
>  
>  /* A wrapper for use as a VMState .get function */
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 80c45c321e..5abada6966 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -188,7 +188,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
> int *in_bytes,
>  void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq);
>  void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
>  
> -void virtio_save(VirtIODevice *vdev, QEMUFile *f);
> +int virtio_save(VirtIODevice *vdev, QEMUFile *f);
>  
>  extern const VMStateInfo virtio_vmstate_info;
>  
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index c056c98bdb..48184c380d 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -550,13 +550,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size, 
> VMStateField *field,
>  {
>  const VMStateDescription *vmsd = field->vmsd;
>  void *tmp = g_malloc(size);
> +int ret;
>  
>  /* Writes the parent field which is at the start of the tmp */
>  *(void **)tmp = pv;
> -vmstate_save_state(f, vmsd, tmp, vmdesc);
> +ret = vmstate_save_state(f, vmsd, tmp, vmdesc);
>  g_free(tmp);
>  
> -return 0;
> +return ret;
>  }
>  
>  const VMStateInfo vmstate_info_tmp = {
> @@ -657,12 +658,16 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t 
> unused_size,
>  /* offset of the QTAILQ entry in a QTAILQ element*/
>  size_t entry_offset = field->start;
>  void *elm;
> +int ret;
>  
>  trace_put_qtailq(vmsd->name, vmsd->version_id);
>  
>  QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {
>  qemu_put_byte(f, true);
> -vmstate_save_state(f, vmsd, elm, vmdesc);
> +ret = vmstate_save_state(f, vmsd, elm, vmdesc);
> +if (ret) {
> +return ret;
> +}
>  }
>  qemu_put_byte(f, false);
>  
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index e643ac662b..087844b6c8 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -70,7 +70,8 @@ static void save_vmstate(const VMStateDescription *desc, 
> void *obj)
>  QEMUFile *f = open_test_file(true);
>  
>  /* Save file with vmstate */
> -

Re: [Qemu-devel] [PATCH] spapr: move registration of "host" CPU core type to machine code

[David Gibson]

On Mon, Sep 25, 2017 at 11:47:33AM +0200, Greg Kurz wrote:
> The CPU core abstraction belongs to the machine code. This also gets
> rid of some code duplication.
> 
> Signed-off-by: Greg Kurz 
> ---
> 
> hw/ppc/spapr_cpu_core.h is also included elsewhere in target/ppc/kvm.c
> but this is already handled by the following cleanup patch:

I don't really see what the advantage of this is.  As others have
pointed out it leads to the host type being registered very late,
which could cause problems.

> 
> https://patchwork.ozlabs.org/patch/817598/
> ---
>  hw/ppc/spapr.c  |4 
>  hw/ppc/spapr_cpu_core.c |   34 ++
>  include/hw/ppc/spapr_cpu_core.h |2 +-
>  target/ppc/kvm.c|   12 
>  4 files changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0ce3ec87ac59..e82c8532ffb0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2349,6 +2349,10 @@ static void ppc_spapr_init(MachineState *machine)
>  }
>  
>  /* init CPUs */
> +if (kvm_enabled()) {
> +spapr_cpu_core_register_host_type();
> +}
> +
>  if (machine->cpu_model == NULL) {
>  machine->cpu_model = kvm_enabled() ? "host" : smc->tcg_default_cpu;
>  }
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index c08ee7571a50..6e224ba029ec 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -317,7 +317,7 @@ static Property spapr_cpu_core_properties[] = {
>  DEFINE_PROP_END_OF_LIST()
>  };
>  
> -void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(oc);
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
> @@ -337,6 +337,20 @@ static const TypeInfo spapr_cpu_core_type_info = {
>  .class_size = sizeof(sPAPRCPUCoreClass),
>  };
>  
> +static void spapr_cpu_core_register_type(const char *model_name)
> +{
> +TypeInfo type_info = {
> +.parent = TYPE_SPAPR_CPU_CORE,
> +.instance_size = sizeof(sPAPRCPUCore),
> +.class_init = spapr_cpu_core_class_init,
> +.class_data = (void *) model_name,
> +};
> +
> +type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, model_name);
> +type_register(_info);
> +g_free((void *)type_info.name);
> +}
> +
>  static void spapr_cpu_core_register_types(void)
>  {
>  int i;
> @@ -344,18 +358,14 @@ static void spapr_cpu_core_register_types(void)
>  type_register_static(_cpu_core_type_info);
>  
>  for (i = 0; i < ARRAY_SIZE(spapr_core_models); i++) {
> -TypeInfo type_info = {
> -.parent = TYPE_SPAPR_CPU_CORE,
> -.instance_size = sizeof(sPAPRCPUCore),
> -.class_init = spapr_cpu_core_class_init,
> -.class_data = (void *) spapr_core_models[i],
> -};
> -
> -type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE,
> - spapr_core_models[i]);
> -type_register(_info);
> -g_free((void *)type_info.name);
> +spapr_cpu_core_register_type(spapr_core_models[i]);
>  }
> +
> +}
> +
> +void spapr_cpu_core_register_host_type(void)
> +{
> +spapr_cpu_core_register_type("host");
>  }
>  
>  type_init(spapr_cpu_core_register_types)
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 93051e9ecf56..e3e906343048 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -36,5 +36,5 @@ typedef struct sPAPRCPUCoreClass {
>  } sPAPRCPUCoreClass;
>  
>  char *spapr_get_cpu_core_type(const char *model);
> -void spapr_cpu_core_class_init(ObjectClass *oc, void *data);
> +void spapr_cpu_core_register_host_type(void);
>  #endif
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 5b281b2f1b6d..8dd80993ec9e 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -37,7 +37,6 @@
>  #include "hw/sysbus.h"
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
> -#include "hw/ppc/spapr_cpu_core.h"
>  #include "hw/ppc/ppc.h"
>  #include "sysemu/watchdog.h"
>  #include "trace.h"
> @@ -2502,17 +2501,6 @@ static int kvm_ppc_register_host_cpu_type(void)
>  oc = object_class_by_name(type_info.name);
>  g_assert(oc);
>  
> -#if defined(TARGET_PPC64)
> -type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host");
> -type_info.parent = TYPE_SPAPR_CPU_CORE,
> -type_info.instance_size = sizeof(sPAPRCPUCore);
> -type_info.instance_init = NULL;
> -type_info.class_init = spapr_cpu_core_class_init;
> -type_info.class_data = (void *) "host";
> -type_register(_info);
> -g_free((void *)type_info.name);
> -#endif
> -
>  /*
>   * Update generic CPU family class alias (e.g. on a POWER8NVL host,
>   * we want "POWER8" to be a "family" alias that points to the current
> 

-- 
David 

Re: [Qemu-devel] [PATCH 6/7] mac_dbdma: change DBDMA_register_channel to a MAC_DBDMA type method

[David Gibson]

On Sun, Sep 24, 2017 at 03:47:45PM +0100, Mark Cave-Ayland wrote:
> Using this we can change the MACIO_IDE instance to register the channel
> itself via a type method instead of requiring a separate
> DBDMA_register_channel() function.
> 
> As a consequence of this it is now possible to remove the old external
> macio_ide_register_dma() function.
> 
> Signed-off-by: Mark Cave-Ayland 

Ok, two concerns about this.

First, you've added the function pointer to the instance, not to the
class, which is not how a QOM method would normally be done.

More generally, though, why is a method preferable to a plain
function?  AFAICT it's not plausible that there will ever be more than
one implementation of the method.

Same comments apply to patch 7/7.

> ---
>  hw/ide/macio.c |   12 ++--
>  hw/misc/macio/mac_dbdma.c  |9 +
>  hw/misc/macio/macio.c  |1 -
>  include/hw/ppc/mac_dbdma.h |9 -
>  4 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index ce194c6..b296017 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -411,12 +411,18 @@ static const IDEDMAOps dbdma_ops = {
>  static void macio_ide_realizefn(DeviceState *dev, Error **errp)
>  {
>  MACIOIDEState *s = MACIO_IDE(dev);
> +DBDMAState *dbdma;
>  
>  ide_init2(>bus, s->ide_irq);
>  
>  /* Register DMA callbacks */
>  s->dma.ops = _ops;
>  s->bus.dma = >dma;
> +
> +/* Register DBDMA channel */
> +dbdma = MAC_DBDMA(object_property_get_link(OBJECT(dev), "dbdma", errp));
> +dbdma->register_channel(dbdma, s->channel, s->dma_irq,
> +pmac_ide_transfer, pmac_ide_flush, s);
>  }
>  
>  static void pmac_ide_irq(void *opaque, int n, int level)
> @@ -497,10 +503,4 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo 
> **hd_table)
>  }
>  }
>  
> -void macio_ide_register_dma(MACIOIDEState *s)
> -{
> -DBDMA_register_channel(s->dbdma, s->channel, s->dma_irq,
> -   pmac_ide_transfer, pmac_ide_flush, s);
> -}
> -
>  type_init(macio_ide_register_types)
> diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c
> index 0eddf2e..addb97d 100644
> --- a/hw/misc/macio/mac_dbdma.c
> +++ b/hw/misc/macio/mac_dbdma.c
> @@ -557,11 +557,10 @@ void DBDMA_kick(DBDMAState *dbdma)
>  qemu_bh_schedule(dbdma->bh);
>  }
>  
> -void DBDMA_register_channel(void *dbdma, int nchan, qemu_irq irq,
> -DBDMA_rw rw, DBDMA_flush flush,
> -void *opaque)
> +static void
> +dbdma_register_channel(DBDMAState *s, int nchan, qemu_irq irq,
> +   DBDMA_rw rw, DBDMA_flush flush, void *opaque)
>  {
> -DBDMAState *s = dbdma;
>  DBDMA_channel *ch = >channels[nchan];
>  
>  DBDMA_DPRINTFCH(ch, "DBDMA_register_channel 0x%x\n", nchan);
> @@ -903,6 +902,8 @@ static void mac_dbdma_init(Object *obj)
>  
>  memory_region_init_io(>mem, obj, _ops, s, "dbdma", 0x1000);
>  sysbus_init_mmio(sbd, >mem);
> +
> +s->register_channel = dbdma_register_channel;
>  }
>  
>  static void mac_dbdma_realize(DeviceState *dev, Error **errp)
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 9aa7e75..51a 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -161,7 +161,6 @@ static void macio_realize_ide(MacIOState *s, 
> MACIOIDEState *ide,
>  sysbus_connect_irq(sysbus_dev, 1, irq1);
>  qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
>  object_property_set_link(OBJECT(ide), OBJECT(s->dbdma), "dbdma", errp);
> -macio_ide_register_dma(ide);
>  
>  object_property_set_bool(OBJECT(ide), true, "realized", errp);
>  }
> diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h
> index 26cc469..d6a38c5 100644
> --- a/include/hw/ppc/mac_dbdma.h
> +++ b/include/hw/ppc/mac_dbdma.h
> @@ -160,19 +160,18 @@ typedef struct DBDMA_channel {
>  dbdma_cmd current;
>  } DBDMA_channel;
>  
> -typedef struct {
> +typedef struct DBDMAState {
>  SysBusDevice parent_obj;
>  
>  MemoryRegion mem;
>  DBDMA_channel channels[DBDMA_CHANNELS];
>  QEMUBH *bh;
> +
> +void (*register_channel)(struct DBDMAState *s, int nchan, qemu_irq irq,
> + DBDMA_rw rw, DBDMA_flush flush, void *opaque);
>  } DBDMAState;
>  
>  /* Externally callable functions */
> -
> -void DBDMA_register_channel(void *dbdma, int nchan, qemu_irq irq,
> -DBDMA_rw rw, DBDMA_flush flush,
> -void *opaque);
>  void DBDMA_kick(DBDMAState *dbdma);
>  
>  #define TYPE_MAC_DBDMA "mac-dbdma"

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH v2 01/21] ppc/xive: introduce a skeleton for the sPAPR XIVE interrupt controller

[David Gibson]

On Fri, Sep 22, 2017 at 02:42:07PM +0200, Cédric Le Goater wrote:
> On 09/22/2017 01:00 PM, David Gibson wrote:
> > On Tue, Sep 19, 2017 at 03:15:44PM +0200, Cédric Le Goater wrote:
> >> On 09/19/2017 04:27 AM, David Gibson wrote:
> >>> On Mon, Sep 11, 2017 at 07:12:15PM +0200, Cédric Le Goater wrote:
>  Start with a couple of attributes for the XIVE sPAPR controller
>  model. The number of provisionned IRQ is necessary to size the
>  different internal XIVE tables, the number of CPUs is also.
> 
>  Signed-off-by: Cédric Le Goater 
> >>>
> >>> [snip]
> >>>
>  +static void spapr_xive_realize(DeviceState *dev, Error **errp)
>  +{
>  +sPAPRXive *xive = SPAPR_XIVE(dev);
>  +
>  +if (!xive->nr_targets) {
>  +error_setg(errp, "Number of interrupt targets needs to be 
>  greater 0");
>  +return;
>  +}
>  +/* We need to be able to allocate at least the IPIs */
>  +if (!xive->nr_irqs || xive->nr_irqs < xive->nr_targets) {
>  +error_setg(errp, "Number of interrupts too small");
>  +return;
>  +}
>  +}
>  +
>  +static Property spapr_xive_properties[] = {
>  +DEFINE_PROP_UINT32("nr-irqs", sPAPRXive, nr_irqs, 0),
>  +DEFINE_PROP_UINT32("nr-targets", sPAPRXive, nr_targets, 0),
> >>>
> >>> I'm a bit uneasy about the number of targets having to be set in
> >>> advance: this can make life awkward when CPUs are hotplugged.  I know
> >>> there's something similar in xics, but it has caused some hassles, and
> >>> we're starting to move away from it.
> >>>
> >>> Do you really need this?
> >>>
> >>
> >> Some of the internal table size depend on the number of cpus 
> >> defined for the machine.
> > 
> > Which ones?  My impression was that there needed to be at least #cpus
> > * #priority-levels EQs, but there could be more than that, 
> 
> euh no, not in spapr mode at least. There are 8 queues per cpu.

Ok.

> > so it was no longer as tightly bound to the number if "interrupt servers"> 
> > as xics.
> 
> ah. I think I see what you mean, that we could allocate them on the 
> fly when needed by some hcalls ?

Not at hcall time, no, but at cpu hot(un)plug time I was wondering if we
could (de)allocate them then.

> The other place where I use the nr_targets is to provision the 
> IRQ numbers for the IPIs but that could probably be done in some 
> other way, specially it there is a IRQ allocator at the machine
> level.

Hm, ok.

> 
> C.  
> >> When the sPAPRXive object is instantiated, 
> >> we use xics_max_server_number() to get the max number of cpus
> >> provisioned.
> >>
> >> C.
> >>
> > 
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] linux-user: remove duplicate break in syscall

[no-reply]

Hi,

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

Type: series
Message-id: 20170926041420.32673-1-care...@gmail.com
Subject: [Qemu-devel] [PATCH] linux-user: remove duplicate break in syscall

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20170925112917.21340-1-dgilb...@redhat.com 
-> patchew/20170925112917.21340-1-dgilb...@redhat.com
 t [tag update]patchew/20170925145526.32690-1-ebl...@redhat.com -> 
patchew/20170925145526.32690-1-ebl...@redhat.com
 * [new tag]   patchew/20170926041420.32673-1-care...@gmail.com -> 
patchew/20170926041420.32673-1-care...@gmail.com
Switched to a new branch 'test'
bf097ed029 linux-user: remove duplicate break in syscall

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

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

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

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:12PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Modify the pre_save method on VMStateDescription to return an int
> rather than void so that it potentially can fail.
> 
> Changed zillions of devices to make them return 0; the only
> case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already
> had an error_report/return case.
> 
> Note: If you add an error exit in your pre_save you must emit
> an error_report to say why.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v1 3/8] hw: Replace fprintf(stderr, "*\n" with error_report()

[Thomas Huth]

On 26.09.2017 02:08, Alistair Francis wrote:
> Replace a large number of the fprintf(stderr, "*\n" calls with
> error_report(). The functions were renamed with these commands and then
> compiler issues where manually fixed.
> 
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
> \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> 
> Some lines where then manually tweaked to pass checkpatch.
> 
> Signed-off-by: Alistair Francis 
> Cc: Andrzej Zaborowski 
> Cc: Jan Kiszka 
> Cc: Stefan Hajnoczi 
> Cc: Paolo Bonzini 
> Cc: Thomas Huth 
> Cc: Gerd Hoffmann 
> Cc: "Michael S. Tsirkin" 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: John Snow 
> Cc: Christian Borntraeger 
> Cc: Cornelia Huck 
> Cc: Alexander Graf 
> Cc: Michael Walle 
> Cc: Paul Burton 
> Cc: Aurelien Jarno 
> Cc: Yongbok Kim 
> Cc: "Hervé Poussineau" 
> Cc: Anthony Green 
> Cc: Jason Wang 
> Cc: Chris Wulff 
> Cc: Marek Vasut 
> Cc: Jia Liu 
> Cc: Stafford Horne 
> Cc: Marcel Apfelbaum 
> Cc: Magnus Damm 
> Cc: Fabien Chouteau 
> Cc: Mark Cave-Ayland 
> Cc: Artyom Tarasenko 
> Cc: qemu-...@nongnu.org
> Cc: qemu-bl...@nongnu.org
> Cc: xen-de...@lists.xenproject.org
> Cc: qemu-...@nongnu.org
> ---
> 
>  hw/arm/armv7m.c |  2 +-
>  hw/arm/boot.c   | 34 +--
>  hw/arm/gumstix.c| 13 
>  hw/arm/mainstone.c  |  7 ++--
>  hw/arm/musicpal.c   |  2 +-
>  hw/arm/omap1.c  |  5 +--
>  hw/arm/omap2.c  | 21 ++--
>  hw/arm/omap_sx1.c   |  6 ++--
>  hw/arm/palm.c   | 10 +++---
>  hw/arm/pxa2xx.c |  7 ++--
>  hw/arm/stellaris.c  |  3 +-
>  hw/arm/tosa.c   | 17 +-
>  hw/arm/versatilepb.c|  2 +-
>  hw/arm/vexpress.c   |  8 ++---
>  hw/arm/z2.c |  6 ++--
>  hw/block/dataplane/virtio-blk.c |  6 ++--
>  hw/block/onenand.c  |  8 ++---
>  hw/block/tc58128.c  | 44 -
>  hw/bt/core.c| 15 +
>  hw/bt/hci-csr.c | 17 +-
>  hw/bt/hci.c | 30 -
>  hw/bt/hid.c |  2 +-
>  hw/bt/l2cap.c   | 47 ++-
>  hw/bt/sdp.c |  7 ++--
>  hw/char/exynos4210_uart.c   |  6 ++--
>  hw/char/mcf_uart.c  |  5 +--
>  hw/char/sh_serial.c |  9 +++---
>  hw/core/loader.c| 31 +-
>  hw/core/ptimer.c|  7 ++--
>  hw/cris/axis_dev88.c|  3 +-
>  hw/cris/boot.c  |  5 +--
>  hw/display/blizzard.c   | 20 ++--
>  hw/display/omap_dss.c   | 14 
>  hw/display/pl110.c  |  2 +-
>  hw/display/pxa2xx_lcd.c |  2 +-
>  

Re: [Qemu-devel] [PATCH 2/2] thunk.h uses TARGET_ABI_BITS without including abitypes.h

[Daniel Loffgren]

I am attempting to get ppc-darwin-user in a good working state again, and this 
broke one of the .c files that included it. I figured this change should be 
made regardless of my branch, and would help reduce the size of my branch for 
future rebasing.

> On Sep 25, 2017, at 9:37 AM, Thomas Huth  wrote:
> 
> 
> Did this cause any trouble? ... one of two sentences in the patch
> description would be nice, I think.
> 
> Thomas
> 
> 
> On 25.09.2017 03:02, Daniel Loffgren wrote:
>> 
>> Signed-off-by: Daniel Loffgren 
>> ---
>> include/exec/user/thunk.h | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
>> index f19ef4b230..12b5449d8c 100644
>> --- a/include/exec/user/thunk.h
>> +++ b/include/exec/user/thunk.h
>> @@ -19,6 +19,7 @@
>> #ifndef THUNK_H
>> #define THUNK_H
>> 
>> +#include "abitypes.h"
>> #include "cpu.h"
>> 
>> /* types enums definitions */
>> 
> 

Re: [Qemu-devel] [RFC 0/6] initial plugin support

[Thomas Huth]

On 06.09.2017 22:28, Emilio G. Cota wrote:
> Related threads:
>   [PATCH 00/13] instrument: Add basic event instrumentation
>   Date: Mon, 24 Jul 2017 20:02:24 +0300
>   https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg07419.html
> and
>   [PATCH v4 00/20] instrument: Add basic event instrumentation
>   Date:   Wed, 6 Sep 2017 20:22:41 +0300
>   https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg07419.html
> 
> This set does something similar to the instrumentation patches by Lluis,
> but with a different implementation (and for now less events).
> 
> My focus has been on working on the skeleton of a (pseudo) stable API,
> as Stefan requested. Of course more events would have to be added, but
> before spending more time on this I'd like to get some feedback on the
> core of the design. Patch 2 has all the details.

Sorry for my ignorance, but if you send a patch series like this, could
you please elaborate a little bit more on the topic what this all is
about? In this cover letter, you basically give only some pointers about
other patch series and point the reader to patch 2, but also patch 2
does not really have a proper *description* of what this is really all
about. Sure, it's about plugins, but what kind of plugins? Audio? Video?
CPU? Everything? If you send RFC, you should properly describe your
vision first, and maybe give some examples, before you jump into the
details.

 Thanks,
  Thomas

[Qemu-devel] [Bug 1354727] Re: build error with GCC 4.9

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  build error with GCC 4.9

Status in QEMU:
  Expired

Bug description:
  % gcc --version
  gcc (GCC) 4.9.1

  latest development version on git

  xen-hvm.c: In function ‘xen_hvm_init’:
  xen-hvm.c:1008:41: error: ‘HVM_PARAM_IOREQ_PFN’ undeclared (first use in this 
function)
   xc_get_hvm_param(xen_xc, xen_domid, HVM_PARAM_IOREQ_PFN, _pfn);

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

[Qemu-devel] [PATCH v3 4/4] iothread: delay the context release to finalize

[Peter Xu]

When gcontext is used with iothread, the context will be destroyed
during iothread_stop().  That's not good since sometimes we would like
to keep the resources until iothread is destroyed, but we may want to
stop the thread before that point.

Delay the destruction of gcontext to iothread finalize.  Then we can do:

  iothread_stop(thread);
  some_cleanup_on_resources();
  iothread_destroy(thread);

We may need this patch if we want to run chardev IOs in iothreads and
hopefully clean them up correctly.  For more specific information,
please see 2b316774f6 ("qemu-char: do not operate on sources from
finalize callbacks").

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
 iothread.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/iothread.c b/iothread.c
index b3c092b..27a4288 100644
--- a/iothread.c
+++ b/iothread.c
@@ -71,8 +71,6 @@ static void *iothread_run(void *opaque)
 g_main_loop_unref(loop);
 
 g_main_context_pop_thread_default(iothread->worker_context);
-g_main_context_unref(iothread->worker_context);
-iothread->worker_context = NULL;
 }
 }
 
@@ -117,6 +115,10 @@ static void iothread_instance_finalize(Object *obj)
 IOThread *iothread = IOTHREAD(obj);
 
 iothread_stop(iothread);
+if (iothread->worker_context) {
+g_main_context_unref(iothread->worker_context);
+iothread->worker_context = NULL;
+}
 qemu_cond_destroy(>init_done_cond);
 qemu_mutex_destroy(>init_done_lock);
 if (!iothread->ctx) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 2/8] tests: Replace fprintf(stderr, "*\n" with error_report()

[Emilio G. Cota]

On Mon, Sep 25, 2017 at 17:08:48 -0700, Alistair Francis wrote:
> diff --git a/tests/atomic_add-bench.c b/tests/atomic_add-bench.c
> index caa1e8e689..41ba1600c0 100644
> --- a/tests/atomic_add-bench.c
> +++ b/tests/atomic_add-bench.c
> @@ -29,7 +29,7 @@ static const char commands_string[] =
>  static void usage_complete(char *argv[])
>  {
>  fprintf(stderr, "Usage: %s [options]\n", argv[0]);
> -fprintf(stderr, "options:\n%s\n", commands_string);
> +fprintf(stderr, "options:\n%s", commands_string);
>  }

We do want that trailing \n, unless we move it to commands_string.

Also, I think using error_report here would be confusing -- this is a standalone
test program with as little QEMU-specific knowledge as possible (QemuThreads
are used for portability); using error_report here is confusing (this is not
an error).

> diff --git a/tests/check-qlit b/tests/check-qlit
> new file mode 100755
> index 
> ..950429524e3eb07e6daed1fe01caad0f5d554809
> GIT binary patch
> literal 272776
> zcmeEvdwf*Ywf~vPB$){zGeCghJ;($So{10*LNEgfoIs*MKvBRDLV(l`3b5QKOS6

? I don't know what this is, I don't seem to have this binary in my
checked out tree.

(snips thousands of lines)

> diff --git a/tests/qht-bench.c b/tests/qht-bench.c
> index 11c1cec766..2637d601a9 100644
> --- a/tests/qht-bench.c
> +++ b/tests/qht-bench.c
> @@ -5,6 +5,7 @@
>   *   See the COPYING file in the top-level directory.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>  #include "qemu/processor.h"
>  #include "qemu/atomic.h"
>  #include "qemu/qht.h"
> @@ -89,7 +90,7 @@ static const char commands_string[] =
>  static void usage_complete(int argc, char *argv[])
>  {
>  fprintf(stderr, "Usage: %s [options]\n", argv[0]);
> -fprintf(stderr, "options:\n%s\n", commands_string);
> +fprintf(stderr, "options:\n%s", commands_string);

Same as above: this removes the necessary trailing \n.

>  exit(-1);
>  }
>  
> @@ -328,7 +329,7 @@ static void htable_init(void)
>  retries++;
>  }
>  }
> -fprintf(stderr, " populated after %zu retries\n", retries);
> +error_report(" populated after %zu retries", retries);
>  }

ditto -- I'd rather keep fprintf(stderr) here, it's less confusing.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH v10 18/20] qcow2: Switch store_bitmap_data() to byte-based iteration

[Fam Zheng]

On Mon, 09/25 09:55, Eric Blake wrote:
> Now that we have adjusted the majority of the calls this function
> makes to be byte-based, it is easier to read the code if it makes
> passes over the image using bytes rather than sectors.
> 
> iotests 165 was rather weak - on a default 64k-cluster image, where
> bitmap granularity also defaults to 64k bytes, a single cluster of
> the bitmap table thus covers (64*1024*8) bits which each cover 64k
> bytes, or 32G of image space.  But the test only uses a 1G image,
> so it cannot trigger any more than one loop of the code in
> store_bitmap_data(); and it was writing to the first cluster.  In
> order to test that we are properly aligning which portions of the
> bitmap are being written to the file, we really want to test a case
> where the first dirty bit returned by bdrv_dirty_iter_next() is not
> aligned to the start of a cluster, which we can do by modifying the
> test to write data that doesn't happen to fall in the first cluster
> of the image.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:15PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Route the errors from vmstate_subsection_save up through
> vmstate_subsection_save (and back down, all rather recursive).

I guess here one of the "vmstate_subsection_save" should be replaced
by "vmstate_save_state"? :-)

> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default

[Thomas Huth]

On 26.09.2017 04:59, Eduardo Habkost wrote:
> On Mon, Sep 25, 2017 at 07:05:26PM +0100, Peter Maydell wrote:
>> On 25 September 2017 at 18:59, Eduardo Habkost  wrote:
>>> Finding the full list of devices that can be instantiated
>>> internally at hotplug-time sounds tricky.
>>
>> If we just diff "list of devices marked hotplug before this patch"
>> against "list of devices marked hotplug after this patch" how
>> big is the list? Can we just eyeball it to see what needs
>> to be specialcased?
> 
> So, the full list quite big, ~1800 device types are affected by
> this patch:
> 
> https://gist.github.com/ehabkost/bd8e25c6811ac81d947ad8ad5b557f5c#file-dev-types-diff-json
> 
> If we ignore the "-cpu" classes, there ~640 affected device
> types.
> 
> However, if we look only at the direct children of TYPE_DEVICE,
> we have:

OK, thanks a lot for that list! But do you think that we can assume that
the devices which are not direct children of TYPE_DEVICE can not be
hotplugged internally during runtime? ... I currently don't think so
(but at least they are good candidates which need to be examined more
carefully).

But anyway, how can we continue here? Set hotpluggable = true on all 640
(or even all 1800) affected devices? That sounds very, very ugly to me.
Maybe we should just do it for the virtio-*-device devices and the
others that break during "make check" (btw. sorry for not noticing this
... I normally run "make check" regularly, but this time I apparently
missed to run it for aarch64), and if we later notice some more
problems, we know that we lack a "make check" test for that case, too,
and we should add such a test? That would at least eventually improve
our test coverage a little bit, I guess...

 Thomas

Re: [Qemu-devel] [PATCH v3 1/4] qom: provide root container for internal objs

[Fam Zheng]

On Tue, 09/26 12:52, Peter Xu wrote:
> We have object_get_objects_root() to keep user created objects, however
> no place for objects that will be used internally.  Create such a
> container for internal objects.
> 
> CC: Andreas Färber 
> CC: Markus Armbruster 
> CC: Paolo Bonzini 
> Suggested-by: Daniel P. Berrange 
> Signed-off-by: Peter Xu 
> ---
>  include/qom/object.h | 10 ++
>  qom/object.c | 11 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f3e5cff..f567052 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1214,6 +1214,16 @@ Object *object_get_root(void);
>  Object *object_get_objects_root(void);
>  
>  /**
> + * object_get_internal_root:
> + *
> + * Get the container object that holds internally used object
> + * instances. This is the object at path "/internal-objects"

Does this comment need update?

> + *
> + * Returns: the internal object container
> + */
> +Object *object_get_internal_root(void);
> +
> +/**
>   * object_get_canonical_path_component:
>   *
>   * Returns: The final component in the object's canonical path.  The 
> canonical
> diff --git a/qom/object.c b/qom/object.c
> index 3e18537..6a7bd92 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1370,6 +1370,17 @@ Object *object_get_objects_root(void)
>  return container_get(object_get_root(), "/objects");
>  }
>  
> +Object *object_get_internal_root(void)
> +{
> +static Object *internal_root;
> +
> +if (!internal_root) {
> +internal_root = object_new("container");
> +}
> +
> +return internal_root;
> +}
> +
>  static void object_get_child_property(Object *obj, Visitor *v,
>const char *name, void *opaque,
>Error **errp)
> -- 
> 2.7.4
> 

Fam

Re: [Qemu-devel] [PATCH v3 2/4] iothread: provide helpers for internal use

[Fam Zheng]

On Tue, 09/26 12:52, Peter Xu wrote:
> IOThread is a general framework that contains IO loop environment and a
> real thread behind.  It's also good to be used internally inside qemu.
> Provide some helpers for it to create iothreads to be used internally.
> 
> Put all the internal used iothreads into the internal object container.
> 
> Signed-off-by: Peter Xu 
> ---
>  include/sysemu/iothread.h |  8 
>  iothread.c| 16 
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> index d2985b3..b07663f 100644
> --- a/include/sysemu/iothread.h
> +++ b/include/sysemu/iothread.h
> @@ -46,4 +46,12 @@ AioContext *iothread_get_aio_context(IOThread *iothread);
>  void iothread_stop_all(void);
>  GMainContext *iothread_get_g_main_context(IOThread *iothread);
>  
> +/*
> + * Helpers used to allocate iothreads for internal use.  These
> + * iothreads will not be seen by monitor clients when query using
> + * "query-iothreads".
> + */
> +IOThread *iothread_create(const char *id, Error **errp);
> +void iothread_destroy(IOThread *iothread);
> +
>  #endif /* IOTHREAD_H */
> diff --git a/iothread.c b/iothread.c
> index 44c8944..33f996e 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -354,3 +354,19 @@ GMainContext *iothread_get_g_main_context(IOThread 
> *iothread)
>  
>  return iothread->worker_context;
>  }
> +
> +IOThread *iothread_create(const char *id, Error **errp)
> +{
> +Object *obj;
> +
> +obj = object_new_with_props(TYPE_IOTHREAD,
> +object_get_internal_root(),
> +id, errp, NULL);
> +
> +return IOTHREAD(obj);
> +}
> +
> +void iothread_destroy(IOThread *iothread)
> +{
> +object_unparent(OBJECT(iothread));
> +}
> -- 
> 2.7.4
> 

Reviewed-by: Fam Zheng 

[Qemu-devel] A glib warning encountered with internal iothread

[Peter Xu]

Hi,

Generally speaking this is a question about glib, but I would like to
see how the list sees this before throwing this question to glib
community in case I made severe mistake.

I encountered one glib warning when start to use internal iothread:

(qemu-system-x86_64:19925): GLib-CRITICAL **: g_source_remove_poll: assertion 
'!SOURCE_DESTROYED (source)' failed

This will be triggered as long as we create one private iothread and
enables gcontext of it (by calling iothread_get_g_main_context() at
least once on the private iothread).

The warning is generated when quitting QEMU in following path (please
ignore unknown function names, they only appear in a private tree, but
the logic is the same):

#0  0x7ff0c2bb4180 in g_source_remove_poll () at /lib64/libglib-2.0.so.0
#1  0x5634b0fe8bc9 in aio_set_fd_handler (ctx=0x5634b365ca90, fd=22, 
is_external=false, io_read=0x0, io_write=0x0, io_poll=0x0, 
opaque=0x5634b365cb3c) at /root/git/qemu/util/aio-posix.c:226
#2  0x5634b0fe8ebc in aio_set_event_notifier (ctx=0x5634b365ca90, 
notifier=0x5634b365cb3c, is_external=false, io_read=0x0, io_poll=0x0) at 
/root/git/qemu/util/aio-posix.c:299
#3  0x5634b0fe5174 in aio_ctx_finalize (source=0x5634b365ca90) at 
/root/git/qemu/util/async.c:296
#4  0x7ff0c2bb39a6 in g_source_unref_internal () at /lib64/libglib-2.0.so.0
#5  0x5634b0fe5757 in aio_context_unref (ctx=0x5634b365ca90) at 
/root/git/qemu/util/async.c:484
#6  0x5634b0c39642 in iothread_instance_finalize (obj=0x5634b36cfa40) at 
/root/git/qemu/iothread.c:127
#7  0x5634b0ede36e in object_deinit (obj=0x5634b36cfa40, 
type=0x5634b3543c60) at /root/git/qemu/qom/object.c:453
#8  0x5634b0ede3e0 in object_finalize (data=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:467
#9  0x5634b0edf361 in object_unref (obj=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:902
#10 0x5634b0ee0607 in object_finalize_child_property (obj=0x5634b369d3b0, 
name=0x5634b36eb900 "monitor_io_thr", opaque=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:1407
#11 0x5634b0ede25d in object_property_del_child (obj=0x5634b369d3b0, 
child=0x5634b36cfa40, errp=0x0) at /root/git/qemu/qom/object.c:427
#12 0x5634b0ede33d in object_unparent (obj=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:446
#13 0x5634b0c39f44 in iothread_destroy (iothread=0x5634b36cfa40) at 
/root/git/qemu/iothread.c:383
#14 0x5634b0ae920f in monitor_io_thread_destroy () at 
/root/git/qemu/monitor.c:4416
#15 0x5634b0ae9302 in monitor_cleanup () at /root/git/qemu/monitor.c:4439
#16 0x5634b0c496cb in main (argc=17, argv=0x7ffcc5b88918, 
envp=0x7ffcc5b889a8) at /root/git/qemu/vl.c:4889

It's on the path during destruction of AioContext, where we called
g_source_remove_poll() in the destructor.  When reach here, AioContext
has been detached from the gcontext already, so the assertion is
triggered.

I'll copy some code for easier reference from glib:

void
g_source_remove_poll (GSource *source,
  GPollFD *fd)
{
  GMainContext *context;
  
  g_return_if_fail (source != NULL);
  g_return_if_fail (fd != NULL);
  g_return_if_fail (!SOURCE_DESTROYED (source)); < this assertion fails
  
  context = source->context;

  if (context)
LOCK_CONTEXT (context);
  
  source->poll_fds = g_slist_remove (source->poll_fds, fd);

  if (context)
{
  if (!SOURCE_BLOCKED (source))
g_main_context_remove_poll_unlocked (context, fd);
  UNLOCK_CONTEXT (context);
}
}

I'm translating this assertion failure into: "when removing one
GSource from the polled GSource array, the GSource should still be
attached to its GMainContext".  But does this really matter?  Why
can't we remove one GSource from the poll array even if the source is
detached already?  After all if we see following code in
g_source_remove_poll() we have already taken special care when
source->context == NULL happens.

Any thoughts?  TIA.

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v1 18/27] s390x/tcg: implement SIGP SENSE RUNNING STATUS

[David Hildenbrand]

On 25.09.2017 14:47, Thomas Huth wrote:
> On 18.09.2017 18:00, David Hildenbrand wrote:
>> Preparation for TCG, for KVM is this is completely handled in the
>> kernel.
>>
>> Signed-off-by: David Hildenbrand 
>> ---
>>  target/s390x/cpu.h  |  2 ++
>>  target/s390x/sigp.c | 25 +
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 5d03802c7d..5aa755d7b5 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -594,6 +594,7 @@ struct sysib_322 {
>>  #define SIGP_SET_PREFIX0x0d
>>  #define SIGP_STORE_STATUS_ADDR 0x0e
>>  #define SIGP_SET_ARCH  0x12
>> +#define SIGP_SENSE_RUNNING 0x15
>>  #define SIGP_STORE_ADTL_STATUS 0x17
>>  
>>  /* SIGP condition codes */
>> @@ -604,6 +605,7 @@ struct sysib_322 {
>>  
>>  /* SIGP status bits */
>>  #define SIGP_STAT_EQUIPMENT_CHECK   0x8000UL
>> +#define SIGP_STAT_NOT_RUNNING   0x0400UL
>>  #define SIGP_STAT_INCORRECT_STATE   0x0200UL
>>  #define SIGP_STAT_INVALID_PARAMETER 0x0100UL
>>  #define SIGP_STAT_EXT_CALL_PENDING  0x0080UL
>> diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
>> index 9587c3d319..c57312b743 100644
>> --- a/target/s390x/sigp.c
>> +++ b/target/s390x/sigp.c
>> @@ -234,6 +234,28 @@ static void sigp_set_prefix(CPUState *cs, 
>> run_on_cpu_data arg)
>>  si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>>  }
>>  
>> +static void sigp_sense_running(S390CPU *dst_cpu, SigpInfo *si)
>> +{
>> +if (!tcg_enabled()) {
>> +/* handled in KVM */
>> +set_sigp_status(si, SIGP_STAT_INVALID_ORDER);
>> +return;
>> +}
> 
> If we're sure that this is always handled in the kernel, I think you
> could simply do a "g_assert(tcg_enabled())" here instead?
> 
>  Thomas
> 

This keeps existing behavior and does not crash the guest. Therefore I
decided to not use a g_assert().

Especially, kernels throw every SIGP order to user space that they don't
understand. So e.g. a SIGP SENSE RUNNING could end up here for older
kernels.

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

[Richard Henderson]

On 09/25/2017 06:29 AM, Dr. David Alan Gilbert (git) wrote:
> Modify the pre_save method on VMStateDescription to return an int
> rather than void so that it potentially can fail.

What does the int value signify?  Why not a bool?


r~

[Qemu-devel] [PATCH 6/8] nbd: Minimal structured read for server

[Vladimir Sementsov-Ogievskiy]

Minimal implementation of structured read: one structured reply chunk,
no segmentation.
Minimal structured error implementation: no text message.
Support DF flag, but just ignore it, as there is no segmentation any
way.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/nbd.h |  31 
 nbd/nbd-internal.h  |   1 +
 nbd/server.c| 100 
 3 files changed, 126 insertions(+), 6 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index a6df5ce8b5..314f2f9bbc 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -69,6 +69,25 @@ typedef struct NBDSimpleReply {
 uint64_t handle;
 } QEMU_PACKED NBDSimpleReply;
 
+typedef struct NBDStructuredReplyChunk {
+uint32_t magic;  /* NBD_STRUCTURED_REPLY_MAGIC */
+uint16_t flags;  /* combination of NBD_SREP_FLAG_* */
+uint16_t type;   /* NBD_SREP_TYPE_* */
+uint64_t handle; /* request handle */
+uint32_t length; /* length of payload */
+} QEMU_PACKED NBDStructuredReplyChunk;
+
+typedef struct NBDStructuredRead {
+NBDStructuredReplyChunk h;
+uint64_t offset;
+} QEMU_PACKED NBDStructuredRead;
+
+typedef struct NBDStructuredError {
+NBDStructuredReplyChunk h;
+uint32_t error;
+uint16_t message_length;
+} QEMU_PACKED NBDStructuredError;
+
 /* Transmission (export) flags: sent from server to client during handshake,
but describe what will happen during transmission */
 #define NBD_FLAG_HAS_FLAGS (1 << 0) /* Flags are there */
@@ -79,6 +98,7 @@ typedef struct NBDSimpleReply {
rotational media */
 #define NBD_FLAG_SEND_TRIM (1 << 5) /* Send TRIM (discard) */
 #define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6) /* Send WRITE_ZEROES */
+#define NBD_FLAG_SEND_DF   (1 << 7) /* Send DF (Do not Fragment) */
 
 /* New-style handshake (global) flags, sent from server to client, and
control what will happen during handshake phase. */
@@ -125,6 +145,7 @@ typedef struct NBDSimpleReply {
 /* Request flags, sent from client to server during transmission phase */
 #define NBD_CMD_FLAG_FUA(1 << 0) /* 'force unit access' during write */
 #define NBD_CMD_FLAG_NO_HOLE(1 << 1) /* don't punch hole on zero run */
+#define NBD_CMD_FLAG_DF (1 << 2) /* don't fragment structured read */
 
 /* Supported request types */
 enum {
@@ -149,6 +170,16 @@ enum {
  * aren't overflowing some other buffer. */
 #define NBD_MAX_NAME_SIZE 256
 
+/* Structured reply flags */
+#define NBD_SREP_FLAG_DONE  (1 << 0) /* This reply-chunk is last */
+
+/* Structured reply types */
+#define NBD_SREP_ERR(value) ((1 << 15) | (value))
+
+#define NBD_SREP_TYPE_NONE  0
+#define NBD_SREP_TYPE_OFFSET_DATA   1
+#define NBD_SREP_TYPE_ERROR NBD_SREP_ERR(1)
+
 /* Details collected by NBD_OPT_EXPORT_NAME and NBD_OPT_GO */
 struct NBDExportInfo {
 /* Set by client before nbd_receive_negotiate() */
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index d96c9cc7fd..6b0d1183ba 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -48,6 +48,7 @@
 
 #define NBD_REQUEST_MAGIC   0x25609513
 #define NBD_SIMPLE_REPLY_MAGIC  0x67446698
+#define NBD_STRUCTURED_REPLY_MAGIC  0x668e33ef
 #define NBD_OPTS_MAGIC  0x49484156454F5054LL
 #define NBD_CLIENT_MAGIC0x420281861253LL
 #define NBD_REP_MAGIC   0x0003e889045565a9LL
diff --git a/nbd/server.c b/nbd/server.c
index 57d5598e0f..0af94a293d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -98,6 +98,8 @@ struct NBDClient {
 QTAILQ_ENTRY(NBDClient) next;
 int nb_requests;
 bool closing;
+
+bool structured_reply;
 };
 
 /* That's all folks */
@@ -760,6 +762,20 @@ static int nbd_negotiate_options(NBDClient *client, 
uint16_t myflags,
 return ret;
 }
 break;
+
+case NBD_OPT_STRUCTURED_REPLY:
+if (client->structured_reply) {
+error_setg(errp, "Double negotiation of structured reply");
+return -EINVAL;
+}
+ret = nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option,
+ errp);
+if (ret < 0) {
+return ret;
+}
+client->structured_reply = true;
+break;
+
 default:
 if (nbd_drop(client->ioc, length, errp) < 0) {
 return -EIO;
@@ -1233,6 +1249,61 @@ static int nbd_co_send_simple_reply(NBDClient *client,
 return nbd_co_send_iov(client, iov, size ? 2 : 1, errp);
 }
 
+static inline void set_be_chunk(NBDStructuredReplyChunk *chunk, uint16_t flags,
+uint16_t type, uint64_t handle, uint32_t 
length)
+{
+stl_be_p(>magic, NBD_STRUCTURED_REPLY_MAGIC);
+stw_be_p(>flags, flags);
+

[Qemu-devel] [PATCH 8/8] nbd: Minimal structured read for client

[Vladimir Sementsov-Ogievskiy]

Minimal implementation: drop most of additional error information.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h  |   2 +
 include/block/nbd.h |  15 -
 block/nbd-client.c  |  97 +-
 nbd/client.c| 169 +++-
 4 files changed, 249 insertions(+), 34 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index b435754b82..9e178de510 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -35,6 +35,8 @@ typedef struct NBDClientSession {
 NBDClientRequest requests[MAX_NBD_REQUESTS];
 NBDReply reply;
 bool quit;
+
+bool structured_reply;
 } NBDClientSession;
 
 NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 314f2f9bbc..7604e80c49 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -57,11 +57,17 @@ struct NBDRequest {
 };
 typedef struct NBDRequest NBDRequest;
 
-struct NBDReply {
+typedef struct NBDReply {
+bool simple;
 uint64_t handle;
 uint32_t error;
-};
-typedef struct NBDReply NBDReply;
+
+uint16_t flags;
+uint16_t type;
+uint32_t tail_length;
+uint64_t offset;
+uint32_t hole_size;
+} NBDReply;
 
 typedef struct NBDSimpleReply {
 uint32_t magic;  /* NBD_SIMPLE_REPLY_MAGIC */
@@ -178,12 +184,15 @@ enum {
 
 #define NBD_SREP_TYPE_NONE  0
 #define NBD_SREP_TYPE_OFFSET_DATA   1
+#define NBD_SREP_TYPE_OFFSET_HOLE   2
 #define NBD_SREP_TYPE_ERROR NBD_SREP_ERR(1)
+#define NBD_SREP_TYPE_ERROR_OFFSET  NBD_SREP_ERR(2)
 
 /* Details collected by NBD_OPT_EXPORT_NAME and NBD_OPT_GO */
 struct NBDExportInfo {
 /* Set by client before nbd_receive_negotiate() */
 bool request_sizes;
+bool structured_reply;
 /* Set by server results during nbd_receive_negotiate() */
 uint64_t size;
 uint16_t flags;
diff --git a/block/nbd-client.c b/block/nbd-client.c
index e4f0c789f4..bdf9299bb9 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -179,9 +179,10 @@ err:
 return rc;
 }
 
-static int nbd_co_receive_reply(NBDClientSession *s,
-uint64_t handle,
-QEMUIOVector *qiov)
+static int nbd_co_receive_1_reply_or_chunk(NBDClientSession *s,
+   uint64_t handle,
+   bool *cont,
+   QEMUIOVector *qiov)
 {
 int ret;
 int i = HANDLE_TO_INDEX(s, handle);
@@ -191,29 +192,95 @@ static int nbd_co_receive_reply(NBDClientSession *s,
 qemu_coroutine_yield();
 s->requests[i].receiving = false;
 if (!s->ioc || s->quit) {
-ret = -EIO;
-} else {
-assert(s->reply.handle == handle);
-ret = -s->reply.error;
-if (qiov && s->reply.error == 0) {
+*cont = false;
+return -EIO;
+}
+
+assert(s->reply.handle == handle);
+*cont = !(s->reply.simple || (s->reply.flags & NBD_SREP_FLAG_DONE));
+ret = -s->reply.error;
+if (ret < 0) {
+goto out;
+}
+
+if (s->reply.simple) {
+if (qiov) {
 if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
-  NULL) < 0) {
-ret = -EIO;
-s->quit = true;
+  NULL) < 0)
+{
+goto fatal;
 }
 }
+goto out;
+}
 
-/* Tell the read handler to read another header.  */
-s->reply.handle = 0;
+/* here we deal with successful structured reply */
+switch (s->reply.type) {
+QEMUIOVector sub_qiov;
+case NBD_SREP_TYPE_OFFSET_DATA:
+if (!qiov || s->reply.offset + s->reply.tail_length > qiov->size) {
+goto fatal;
+}
+qemu_iovec_init(_qiov, qiov->niov);
+qemu_iovec_concat(_qiov, qiov, s->reply.offset,
+  s->reply.tail_length);
+ret = qio_channel_readv_all(s->ioc, sub_qiov.iov, sub_qiov.niov, NULL);
+qemu_iovec_destroy(_qiov);
+if (ret < 0) {
+goto fatal;
+}
+assert(ret == 0);
+break;
+case NBD_SREP_TYPE_OFFSET_HOLE:
+if (!qiov || s->reply.offset + s->reply.hole_size > qiov->size) {
+goto fatal;
+}
+qemu_iovec_memset(qiov, s->reply.offset, 0, s->reply.hole_size);
+break;
+case NBD_SREP_TYPE_NONE:
+break;
+default:
+goto fatal;
 }
 
-s->requests[i].coroutine = NULL;
+out:
+/* For assert at loop start in nbd_read_reply_entry */
+s->reply.handle = 0;
+
+if (s->read_reply_co) {
+aio_co_wake(s->read_reply_co);
+}
+
+return ret;
 
-/* Kick the read_reply_co to get the next reply.  */
+fatal:
+/* protocol or ioc failure */
+*cont = false;
+s->quit = true;
 if 

[Qemu-devel] [PATCH 1/8] block/nbd-client: assert qiov len once in nbd_co_request

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 9d1e154feb..88fd10270e 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -156,7 +156,6 @@ static int nbd_co_send_request(BlockDriverState *bs,
 qio_channel_set_cork(s->ioc, true);
 rc = nbd_send_request(s->ioc, request);
 if (rc >= 0 && !s->quit) {
-assert(request->len == iov_size(qiov->iov, qiov->niov));
 if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
NULL) < 0) {
 rc = -EIO;
@@ -197,7 +196,6 @@ static int nbd_co_receive_reply(NBDClientSession *s,
 assert(s->reply.handle == request->handle);
 ret = -s->reply.error;
 if (qiov && s->reply.error == 0) {
-assert(request->len == iov_size(qiov->iov, qiov->niov));
 if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
   NULL) < 0) {
 ret = -EIO;
@@ -233,6 +231,7 @@ static int nbd_co_request(BlockDriverState *bs,
 
 assert(!qiov || request->type == NBD_CMD_WRITE ||
request->type == NBD_CMD_READ);
+assert(!qiov || request->len == iov_size(qiov->iov, qiov->niov));
 ret = nbd_co_send_request(bs, request,
   request->type == NBD_CMD_WRITE ? qiov : NULL);
 if (ret < 0) {
-- 
2.11.1

Re: [Qemu-devel] [PATCH 27/34] hw/pci: declare pci_nic_init_nofail() in "hw/net/pci.h"

[Marcel Apfelbaum]

Hi Philippe,

It seems the patch is doing much more than
what is mentioned in the subject.

On 22/09/2017 19:01, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/pci/pci_internal.h | 16 +++
  include/hw/net/pci.h  | 20 +
  include/hw/pci/pci.h  |  4 ---
  hw/arm/virt.c |  1 +
  hw/mips/mips_malta.c  |  1 +
  hw/pci/pci.c  | 67 ++--
  hw/pci/pci_nic.c  | 77 +++
  hw/ppc/e500.c |  1 +
  hw/ppc/spapr.c|  1 +
  hw/pci/Makefile.objs  |  1 +
  10 files changed, 120 insertions(+), 69 deletions(-)
  create mode 100644 hw/pci/pci_internal.h
  create mode 100644 include/hw/net/pci.h
  create mode 100644 hw/pci/pci_nic.c

diff --git a/hw/pci/pci_internal.h b/hw/pci/pci_internal.h
new file mode 100644
index 00..d967468767
--- /dev/null
+++ b/hw/pci/pci_internal.h
@@ -0,0 +1,16 @@
+/*
+ * QEMU PCI internal
+ *
+ * Copyright (c) 2005 Fabrice Bellard
+ * > + * This work is licensed under the terms of the GNU GPL, version 2 

or later.

+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_HW_PCI_INTERNAL_H
+#define QEMU_HW_PCI_INTERNAL_H
+
+#include "hw/pci/pci_bus.h"
+
+PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr);
+
+#endif


Why don't add the function directly to hw/pci/pci_nic.c ?
We have only one calling site anyway.

I have nothing against adding an internal H file, but then
we would need to add there *all* the functions used
only by the pci sub-system. And that may be out of the scope
of your series.





diff --git a/include/hw/net/pci.h b/include/hw/net/pci.h
new file mode 100644
index 00..529591b7f3
--- /dev/null
+++ b/include/hw/net/pci.h
@@ -0,0 +1,20 @@
+/*
+ * QEMU network devices
+ *
+ * Copyright (c) 2005 Fabrice Bellard
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_HW_NET_PCI_H
+#define QEMU_HW_NET_PCI_H
+
+#include "net/net.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
+
+PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
+   const char *default_model,
+   const char *default_devaddr);
+


Can you add a short explanation on how the split
helps removing i386/pc dependency from non-PC world?
- it is not straight forward, at least for me.


Thanks,
Marcel


+#endif
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index aa7ef9cf69..6a0f7b5472 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -422,10 +422,6 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
PCIINTxRoutingNotifier notifier);
  void pci_device_reset(PCIDevice *dev);
  
-PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,

-   const char *default_model,
-   const char *default_devaddr);
-
  PCIDevice *pci_vga_init(PCIBus *bus);
  
  int pci_bus_num(PCIBus *s);

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9e18b410d7..39fab3acb9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -35,6 +35,7 @@
  #include "hw/arm/primecell.h"
  #include "hw/arm/virt.h"
  #include "hw/devices.h"
+#include "hw/net/pci.h"
  #include "net/net.h"
  #include "sysemu/block-backend.h"
  #include "sysemu/device_tree.h"
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 6945fa47c3..fb6a2f9363 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -48,6 +48,7 @@
  #include "hw/timer/mc146818rtc.h"
  #include "hw/input/i8042.h"
  #include "hw/timer/i8254.h"
+#include "hw/net/pci.h"
  #include "sysemu/blockdev.h"
  #include "exec/address-spaces.h"
  #include "hw/sysbus.h" /* SysBusDevice */
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 1e6fb88eba..9b678c8fd0 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -28,7 +28,6 @@
  #include "hw/pci/pci_bus.h"
  #include "hw/pci/pci_host.h"
  #include "monitor/monitor.h"
-#include "net/net.h"
  #include "sysemu/sysemu.h"
  #include "hw/loader.h"
  #include "qemu/error-report.h"
@@ -41,6 +40,7 @@
  #include "hw/hotplug.h"
  #include "hw/boards.h"
  #include "qemu/cutils.h"
+#include "pci_internal.h"
  
  //#define DEBUG_PCI

  #ifdef DEBUG_PCI
@@ -671,8 +671,7 @@ static int pci_parse_devaddr(const char *addr, int *domp, 
int *busp,
  return 0;
  }
  
-static PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root,

- const char *devaddr)
+PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr)
  {
  int dom, bus;
  unsigned slot;
@@ -1812,68 +1811,6 @@ PciInfoList *qmp_query_pci(Error **errp)
  return head;
  }
  
-static const char * const pci_nic_models[] = {

-"ne2k_pci",
-"i82551",
-"i82557b",
-"i82559er",
-"rtl8139",
-"e1000",
-"pcnet",

Re: [Qemu-devel] [PATCH 0/2] s390/z14: extended TOD-clock support

[no-reply]

Hi,

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

Type: series
Message-id: 20170925102302.60587-1-borntrae...@de.ibm.com
Subject: [Qemu-devel] [PATCH 0/2] s390/z14: extended TOD-clock support

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

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

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

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
96c6906c93 s390/kvm: Support for get/set of extended TOD-Clock for guest
f4ecf143f0 linux-headers: Update linux headers for extended TOD-Clock

=== OUTPUT BEGIN ===
Checking PATCH 1/2: linux-headers: Update linux headers for extended 
TOD-Clock...
Checking PATCH 2/2: s390/kvm: Support for get/set of extended TOD-Clock for 
guest...
ERROR: Error messages should not contain newlines
#34: FILE: hw/s390x/s390-virtio-ccw.c:217:
+ "s390_get_clock returned error %d.\n", r);

total: 1 errors, 0 warnings, 134 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH v2 2/2] xen: dont try setting max grants multiple times

[Anthony PERARD]

On Fri, Sep 22, 2017 at 02:07:25PM +0200, Juergen Gross wrote:
> Trying to call xengnttab_set_max_grants() with the same file handle
> might fail on some kernels, as this operation is allowed only once.
> 
> This is a problem for the qdisk backend as blk_connect() can be
> called multiple times for a domain, e.g. in case grub-xen is being
> used to boot it.
> 
> So instead of letting the generic backend code open the gnttab device
> do it in blk_connect() and close it again in blk_disconnect.
> 
> Signed-off-by: Juergen Gross 

Acked-by: Anthony PERARD 

Thanks.

> ---
> V2:
> - always call blk_disconnect() from blk_free() in order to have the
>   gnttab device node closed (Anthony Perard)
> ---
>  hw/block/xen_disk.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 62506e3167..e431bd89e8 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -1220,6 +1220,12 @@ static int blk_connect(struct XenDevice *xendev)
>  /* Add on the number needed for the ring pages */
>  max_grants += blkdev->nr_ring_ref;
>  
> +blkdev->xendev.gnttabdev = xengnttab_open(NULL, 0);
> +if (blkdev->xendev.gnttabdev == NULL) {
> +xen_pv_printf(xendev, 0, "xengnttab_open failed: %s\n",
> +  strerror(errno));
> +return -1;
> +}
>  if (xengnttab_set_max_grants(blkdev->xendev.gnttabdev, max_grants)) {
>  xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
>strerror(errno));
> @@ -1327,6 +1333,11 @@ static void blk_disconnect(struct XenDevice *xendev)
>  }
>  blkdev->feature_persistent = false;
>  }
> +
> +if (blkdev->xendev.gnttabdev) {
> +xengnttab_close(blkdev->xendev.gnttabdev);
> +blkdev->xendev.gnttabdev = NULL;
> +}
>  }
>  
>  static int blk_free(struct XenDevice *xendev)
> @@ -1334,9 +1345,7 @@ static int blk_free(struct XenDevice *xendev)
>  struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, 
> xendev);
>  struct ioreq *ioreq;
>  
> -if (blkdev->blk || blkdev->sring) {
> -blk_disconnect(xendev);
> -}
> +blk_disconnect(xendev);
>  
>  while (!QLIST_EMPTY(>freelist)) {
>  ioreq = QLIST_FIRST(>freelist);
> @@ -1363,7 +1372,6 @@ static void blk_event(struct XenDevice *xendev)
>  
>  struct XenDevOps xen_blkdev_ops = {
>  .size   = sizeof(struct XenBlkDev),
> -.flags  = DEVOPS_FLAG_NEED_GNTDEV,
>  .alloc  = blk_alloc,
>  .init   = blk_init,
>  .initialise= blk_connect,
> -- 
> 2.12.3
> 

-- 
Anthony PERARD

[Qemu-devel] [PATCH v10 01/20] block: Make bdrv_img_create() size selection easier to read

[Eric Blake]

All callers of bdrv_img_create() pass in a size, or -1 to read the
size from the backing file.  We then set that size as the QemuOpt
default, which means we will reuse that default rather than the
final parameter to qemu_opt_get_size() several lines later.  But
it is rather confusing to read subsequent checks of 'size == -1'
when it looks (without seeing the full context) like size defaults
to 0; it also doesn't help that a size of 0 is valid (for some
formats).

Rework the logic to make things more legible.

Signed-off-by: Eric Blake 
Reviewed-by: John Snow 
Reviewed-by: Kevin Wolf 
Reviewed-by: Fam Zheng 

---
v6: Combine into a series rather than being a standalone patch (more for
ease of tracking than for being on topic)
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 5c65fac672..528cda7b2c 100644
--- a/block.c
+++ b/block.c
@@ -4488,7 +4488,7 @@ void bdrv_img_create(const char *filename, const char 
*fmt,

 /* The size for the image must always be specified, unless we have a 
backing
  * file and we have not been forbidden from opening it. */
-size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
+size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, img_size);
 if (backing_file && !(flags & BDRV_O_NO_BACKING)) {
 BlockDriverState *bs;
 char *full_backing = g_new0(char, PATH_MAX);
-- 
2.13.5

[Qemu-devel] [PATCH v10 00/20] make dirty-bitmap byte-based

[Eric Blake]

There are patches floating around to add NBD_CMD_BLOCK_STATUS,
but NBD wants to report status on byte granularity (even if the
reporting will probably be naturally aligned to sectors or even
much higher levels).  I've therefore started the task of
converting our block status code to report at a byte granularity
rather than sectors.

Now that 2.11 is open, I'm rebasing/reposting the remaining patches.

The overall conversion currently looks like:
part 1: bdrv_is_allocated (merged in 2.10, commit 51b0a488)
part 2: dirty-bitmap (this series, v9 was here [1])
part 3: bdrv_get_block_status (v4 is posted [2] and is mostly reviewed)
part 4: .bdrv_co_block_status (v3 is posted [3], but needs review)

Available as a tag at:
git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-dirty-v10

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05387.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03543.html
[3] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03812.html

Since v9:
- another try at patch 5 [John]
- add R-b where appropriate

001/20:[] [--] 'block: Make bdrv_img_create() size selection easier to read'
002/20:[] [--] 'hbitmap: Rename serialization_granularity to 
serialization_align'
003/20:[] [--] 'qcow2: Ensure bitmap serialization is aligned'
004/20:[] [--] 'dirty-bitmap: Drop unused functions'
005/20:[0003] [FC] 'dirty-bitmap: Avoid size query failure during truncate'
006/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_size() to report 
bytes'
007/20:[] [--] 'dirty-bitmap: Track bitmap size by bytes'
008/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to 
take bytes'
009/20:[] [--] 'qcow2: Switch sectors_covered_by_bitmap_cluster() to 
byte-based'
010/20:[] [--] 'dirty-bitmap: Set iterator start by offset, not sector'
011/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_iter_next() to report byte 
offset'
012/20:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_count() to report bytes'
013/20:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes'
014/20:[] [--] 'dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use 
bytes'
015/20:[] [--] 'mirror: Switch mirror_dirty_init() to byte-based iteration'
016/20:[] [--] 'qcow2: Switch qcow2_measure() to byte-based iteration'
017/20:[] [--] 'qcow2: Switch load_bitmap_data() to byte-based iteration'
018/20:[] [--] 'qcow2: Switch store_bitmap_data() to byte-based iteration'
019/20:[] [--] 'dirty-bitmap: Switch bdrv_set_dirty() to bytes'
020/20:[] [--] 'dirty-bitmap: Convert internal hbitmap size/granularity'

Eric Blake (20):
  block: Make bdrv_img_create() size selection easier to read
  hbitmap: Rename serialization_granularity to serialization_align
  qcow2: Ensure bitmap serialization is aligned
  dirty-bitmap: Drop unused functions
  dirty-bitmap: Avoid size query failure during truncate
  dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes
  dirty-bitmap: Track bitmap size by bytes
  dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to take bytes
  qcow2: Switch sectors_covered_by_bitmap_cluster() to byte-based
  dirty-bitmap: Set iterator start by offset, not sector
  dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset
  dirty-bitmap: Change bdrv_get_dirty_count() to report bytes
  dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes
  dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes
  mirror: Switch mirror_dirty_init() to byte-based iteration
  qcow2: Switch qcow2_measure() to byte-based iteration
  qcow2: Switch load_bitmap_data() to byte-based iteration
  qcow2: Switch store_bitmap_data() to byte-based iteration
  dirty-bitmap: Switch bdrv_set_dirty() to bytes
  dirty-bitmap: Convert internal hbitmap size/granularity

 include/block/block_int.h|   2 +-
 include/block/dirty-bitmap.h |  43 ++
 include/qemu/hbitmap.h   |   8 +--
 block/io.c   |   6 +-
 block.c  |  18 --
 block/backup.c   |   7 +--
 block/dirty-bitmap.c | 134 ++-
 block/mirror.c   |  79 +++--
 block/qcow2-bitmap.c |  62 +++-
 block/qcow2.c|  22 ---
 migration/block.c|  12 ++--
 tests/test-hbitmap.c |  10 ++--
 util/hbitmap.c   |   8 +--
 tests/qemu-iotests/165   |   2 +-
 14 files changed, 174 insertions(+), 239 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH] iotests: fix 181: enable postcopy-ram capability on target

[Vladimir Sementsov-Ogievskiy]

Migration capabilities should be enabled on both source and
destination qemu processes.

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

Hi all.

After my patch

commit 58110f0acb1a33e2bc60a2f1b26d2690a92e8a14
Author: Vladimir Sementsov-Ogievskiy 
Date:   Mon Jul 10 19:30:16 2017 +0300

migration: split common postcopy out of ram postcopy

test 181 becomes broken. Sorry for that.

Actually, test 181 itself should be fixed, so, here is a fix.
The problem was noted by Kevin, and it touches also some problems
with image locking, which I don't see with my older kernel, so,
please check is this patch fixes all problems.

 tests/qemu-iotests/181 | 2 ++
 tests/qemu-iotests/181.out | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/181 b/tests/qemu-iotests/181
index 0333dda0e3..0c91e8f9de 100755
--- a/tests/qemu-iotests/181
+++ b/tests/qemu-iotests/181
@@ -93,7 +93,9 @@ echo
 
 # Slow down migration so much that it definitely won't finish before we can
 # switch to postcopy
+# Enable postcopy-ram capability both on source and destination
 silent=yes
+_send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)"
 _send_qemu_cmd $src 'migrate_set_speed 4k' "(qemu)"
 _send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
 _send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
diff --git a/tests/qemu-iotests/181.out b/tests/qemu-iotests/181.out
index 6534ba2a76..d58c6a9dab 100644
--- a/tests/qemu-iotests/181.out
+++ b/tests/qemu-iotests/181.out
@@ -20,7 +20,6 @@ read 65536/65536 bytes at offset 0
 
 === Do some I/O on the destination ===
 
-QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qemu-io disk "read -P 0x55 0 64k"
 read 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
2.11.1

[Qemu-devel] [PATCH v10 04/20] dirty-bitmap: Drop unused functions

[Eric Blake]

We had several functions that no one is currently using, and which
use sector-based interfaces.  I'm trying to convert towards byte-based
interfaces, so it's easier to just drop the unused functions:

bdrv_dirty_bitmap_get_meta
bdrv_dirty_bitmap_get_meta_locked
bdrv_dirty_bitmap_reset_meta
bdrv_dirty_bitmap_meta_granularity

Signed-off-by: Eric Blake 
Reviewed-by: John Snow 
Reviewed-by: Kevin Wolf 
Reviewed-by: Fam Zheng 

---
v5: no change
v4: rebase to Vladimir's persistent bitmaps (bdrv_dirty_bitmap_size now
in use), dropped R-b
v3: rebase to upstream changes (bdrv_dirty_bitmap_get_meta_locked was
added in b64bd51e with no clients), kept R-b
v2: tweak commit message based on review, no code change
---
 include/block/dirty-bitmap.h | 10 --
 block/dirty-bitmap.c | 44 
 2 files changed, 54 deletions(-)

diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index a79a58d2c3..8fd842eac9 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -34,7 +34,6 @@ void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
 uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap);
-uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
 const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
@@ -44,15 +43,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int64_t nr_sectors);
 void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int64_t nr_sectors);
-int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
-   BdrvDirtyBitmap *bitmap, int64_t sector,
-   int nb_sectors);
-int bdrv_dirty_bitmap_get_meta_locked(BlockDriverState *bs,
-  BdrvDirtyBitmap *bitmap, int64_t sector,
-  int nb_sectors);
-void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
-  BdrvDirtyBitmap *bitmap, int64_t sector,
-  int nb_sectors);
 BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new(BdrvDirtyBitmap *bitmap);
 BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
  uint64_t first_sector);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 0490ca3aff..42a55e4a4b 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -173,45 +173,6 @@ void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap 
*bitmap)
 qemu_mutex_unlock(bitmap->mutex);
 }

-int bdrv_dirty_bitmap_get_meta_locked(BlockDriverState *bs,
-  BdrvDirtyBitmap *bitmap, int64_t sector,
-  int nb_sectors)
-{
-uint64_t i;
-int sectors_per_bit = 1 << hbitmap_granularity(bitmap->meta);
-
-/* To optimize: we can make hbitmap to internally check the range in a
- * coarse level, or at least do it word by word. */
-for (i = sector; i < sector + nb_sectors; i += sectors_per_bit) {
-if (hbitmap_get(bitmap->meta, i)) {
-return true;
-}
-}
-return false;
-}
-
-int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
-   BdrvDirtyBitmap *bitmap, int64_t sector,
-   int nb_sectors)
-{
-bool dirty;
-
-qemu_mutex_lock(bitmap->mutex);
-dirty = bdrv_dirty_bitmap_get_meta_locked(bs, bitmap, sector, nb_sectors);
-qemu_mutex_unlock(bitmap->mutex);
-
-return dirty;
-}
-
-void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
-  BdrvDirtyBitmap *bitmap, int64_t sector,
-  int nb_sectors)
-{
-qemu_mutex_lock(bitmap->mutex);
-hbitmap_reset(bitmap->meta, sector, nb_sectors);
-qemu_mutex_unlock(bitmap->mutex);
-}
-
 int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
 {
 return bitmap->size;
@@ -511,11 +472,6 @@ uint32_t bdrv_dirty_bitmap_granularity(const 
BdrvDirtyBitmap *bitmap)
 return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
 }

-uint32_t bdrv_dirty_bitmap_meta_granularity(BdrvDirtyBitmap *bitmap)
-{
-return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->meta);
-}
-
 BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
  uint64_t first_sector)
 {
-- 
2.13.5

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

[Richard Henderson]

On 09/25/2017 06:25 AM, Dr. David Alan Gilbert wrote:
>> Why not a bool?
> 
> Consistency with pre_load, post_load, put and get methods
> that are already int.

Fair enough, thanks.


r~

[Qemu-devel] [PULL 1/4] nbd-client: Use correct macro parenthesization

[Eric Blake]

If 'bs' is a complex expression, we were only casting the front half
rather than the full expression.  Luckily, none of the callers were
passing bad arguments, but it's better to be robust up front.

Signed-off-by: Eric Blake 
Message-Id: <20170918214649.17550-1-ebl...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Hajnoczi 
---
 block/nbd-client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index ee7f758e68..cc05e73c2d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -31,8 +31,8 @@
 #include "qapi/error.h"
 #include "nbd-client.h"

-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
-#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))
+#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
+#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))

 static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
 {
-- 
2.13.5

[Qemu-devel] [PULL 4/4] block/nbd-client: nbd_co_send_request: fix return code

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy 

It's incorrect to return success rc >= 0 if we skip qio_channel_writev_all()
call due to s->quit.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Eric Blake 
Message-Id: <20170920124507.18841-4-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake 
---
 block/nbd-client.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index ac93c4c0d0..72651dcdb1 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -161,6 +161,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
NULL) < 0) {
 rc = -EIO;
 }
+} else if (rc >= 0) {
+rc = -EIO;
 }
 qio_channel_set_cork(s->ioc, false);
 } else {
-- 
2.13.5

Re: [Qemu-devel] [PATCH v9 05/20] dirty-bitmap: Avoid size query failure during truncate

[Eric Blake]

On 09/23/2017 07:04 AM, Vladimir Sementsov-Ogievskiy wrote:
> 19.09.2017 23:18, Eric Blake wrote:
>> We've previously fixed several places where we failed to account
>> for possible errors from bdrv_nb_sectors().  Fix another one by
>> making bdrv_dirty_bitmap_truncate() take the new size from the
>> caller instead of querying itself; then adjust the sole caller
>> bdrv_truncate() to pass the size just determined by a successful
>> resize, or to skip the bitmap resize on failure, thus avoiding
>> sizing the bitmaps to -1.
>>
>> Signed-off-by: Eric Blake 
>>
>> ---
>> v9: skip only bdrv_dirty_bitmap_truncate on error [Fam]
>> v8: retitle and rework to avoid possibility of secondary failure [John]
>> v7: new patch [Kevin]
>> ---
>>   include/block/dirty-bitmap.h |  2 +-
>>   block.c  | 15 ++-
>>   block/dirty-bitmap.c |  6 +++---
>>   3 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index 8fd842eac9..7a27590047 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter
>> *iter);
>>   void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
>>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>>   int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
>> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
> 
> why not uint64_t as in following patches?

Because off_t is signed, so you can never have more than 2^63 (and NOT
2^64) bytes for your size anyways.  The following patches use int64_t,
rather than uint64_t, both because of off_t, and because it leaves room
for returning negative values on error.

> 
>>   bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>>   bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>>   bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
>> diff --git a/block.c b/block.c
>> index ee6a48976e..89261a7a53 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3450,12 +3450,17 @@ int bdrv_truncate(BdrvChild *child, int64_t
>> offset, PreallocMode prealloc,
>>   assert(!(bs->open_flags & BDRV_O_INACTIVE));
>>
>>   ret = drv->bdrv_truncate(bs, offset, prealloc, errp);
>> -    if (ret == 0) {
>> -    ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
>> -    bdrv_dirty_bitmap_truncate(bs);
>> -    bdrv_parent_cb_resize(bs);
>> -    atomic_inc(>write_gen);
>> +    if (ret < 0) {
>> +    return ret;
>>   }
>> +    ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
>> +    if (ret < 0) {
>> +    error_setg_errno(errp, -ret, "Could not refresh total sector
>> count");
> 
> looks like a separate bug - we didn't set errp with <0 return value

Yes, it was a pre-existing bug.  If I have to respin, I can update the
commit message to mention it.

> 
> Looks like this all needs more work to make it really correct and safe
> (reading last John's comment)..
> And this patch don't really relate to the series, so if it will be the
> only obstacle for merging, can we
> merge all other patches first? I'll then rebase dirty bitmap migration
> series on master..

But it does relate, because I have to do something to avoid calling a
failing bdrv_nb_sectors/bdrv_getlength.

-- 
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] [PULL 3/4] block/nbd-client: simplify check in nbd_co_receive_reply

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy 

If we are woken up from while() loop in nbd_read_reply_entry
handles must be equal. If we are woken up from
nbd_recv_coroutines_wake_all s->quit must be true, so we do
not need checking handles equality.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Eric Blake 
Message-Id: <20170920124507.18841-3-vsement...@virtuozzo.com>
Signed-off-by: Eric Blake 
---
 block/nbd-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 5168a2cab6..ac93c4c0d0 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -189,9 +189,10 @@ static int nbd_co_receive_reply(NBDClientSession *s,
 s->requests[i].receiving = true;
 qemu_coroutine_yield();
 s->requests[i].receiving = false;
-if (s->reply.handle != request->handle || !s->ioc || s->quit) {
+if (!s->ioc || s->quit) {
 ret = -EIO;
 } else {
+assert(s->reply.handle == request->handle);
 ret = -s->reply.error;
 if (qiov && s->reply.error == 0) {
 assert(request->len == iov_size(qiov->iov, qiov->niov));
-- 
2.13.5

[Qemu-devel] [PATCH 2/8] block/nbd-client: refactor nbd_co_receive_reply

[Vladimir Sementsov-Ogievskiy]

Pass handle parameter directly, as the whole request isn't needed.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 88fd10270e..e4f0c789f4 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -180,11 +180,11 @@ err:
 }
 
 static int nbd_co_receive_reply(NBDClientSession *s,
-NBDRequest *request,
+uint64_t handle,
 QEMUIOVector *qiov)
 {
 int ret;
-int i = HANDLE_TO_INDEX(s, request->handle);
+int i = HANDLE_TO_INDEX(s, handle);
 
 /* Wait until we're woken up by nbd_read_reply_entry.  */
 s->requests[i].receiving = true;
@@ -193,7 +193,7 @@ static int nbd_co_receive_reply(NBDClientSession *s,
 if (!s->ioc || s->quit) {
 ret = -EIO;
 } else {
-assert(s->reply.handle == request->handle);
+assert(s->reply.handle == handle);
 ret = -s->reply.error;
 if (qiov && s->reply.error == 0) {
 if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
@@ -238,7 +238,7 @@ static int nbd_co_request(BlockDriverState *bs,
 return ret;
 }
 
-return nbd_co_receive_reply(client, request,
+return nbd_co_receive_reply(client, request->handle,
 request->type == NBD_CMD_READ ? qiov : NULL);
 }
 
-- 
2.11.1

[Qemu-devel] [PATCH 7/8] nbd/client: refactor nbd_receive_starttls

[Vladimir Sementsov-Ogievskiy]

Split out nbd_receive_simple_option to be reused for structured reply
option.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/client.c | 64 
 nbd/trace-events |  7 ---
 2 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index cd5a2c80ac..51ae492e92 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -540,35 +540,63 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
 }
 }
 
-static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
-QCryptoTLSCreds *tlscreds,
-const char *hostname, Error **errp)
+/* nbd_request_simple_option
+ * return 1 for successful negotiation,
+ *0 if operation is unsupported,
+ *-1 with errp set for any other error
+ */
+static int nbd_request_simple_option(QIOChannel *ioc, int opt, Error **errp)
 {
 nbd_opt_reply reply;
-QIOChannelTLS *tioc;
-struct NBDTLSHandshakeData data = { 0 };
 
-trace_nbd_receive_starttls_request();
-if (nbd_send_option_request(ioc, NBD_OPT_STARTTLS, 0, NULL, errp) < 0) {
-return NULL;
+trace_nbd_receive_simple_option_request(opt, nbd_opt_lookup(opt));
+if (nbd_send_option_request(ioc, opt, 0, NULL, errp) < 0) {
+return -1;
 }
 
-trace_nbd_receive_starttls_reply();
-if (nbd_receive_option_reply(ioc, NBD_OPT_STARTTLS, , errp) < 0) {
-return NULL;
+trace_nbd_receive_simple_option_reply(opt, nbd_opt_lookup(opt));
+if (nbd_receive_option_reply(ioc, opt, , errp) < 0) {
+return -1;
 }
 
-if (reply.type != NBD_REP_ACK) {
-error_setg(errp, "Server rejected request to start TLS %" PRIx32,
-   reply.type);
+if (reply.length != 0) {
+error_setg(errp, "Option %d ('%s') response length is %" PRIu32
+   " (it should be zero)", opt, nbd_opt_lookup(opt),
+   reply.length);
 nbd_send_opt_abort(ioc);
-return NULL;
+return -1;
 }
 
-if (reply.length != 0) {
-error_setg(errp, "Start TLS response was not zero %" PRIu32,
-   reply.length);
+if (reply.type == NBD_REP_ERR_UNSUP) {
+return 1;
+}
+
+if (reply.type != NBD_REP_ACK) {
+error_setg(errp, "Server rejected request for option %d (%s) "
+   "with reply %" PRIx32 " (%s)", opt, nbd_opt_lookup(opt),
+   reply.type, nbd_rep_lookup(reply.type));
 nbd_send_opt_abort(ioc);
+return -1;
+}
+
+trace_nbd_receive_simple_option_approved(opt, nbd_opt_lookup(opt));
+return 0;
+}
+
+static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
+QCryptoTLSCreds *tlscreds,
+const char *hostname, Error **errp)
+{
+int ret;
+QIOChannelTLS *tioc;
+struct NBDTLSHandshakeData data = { 0 };
+
+ret = nbd_request_simple_option(ioc, NBD_OPT_STARTTLS, errp);
+if (ret <= 0) {
+if (ret == 0) {
+error_setg(errp, "Server don't support STARTTLS option");
+nbd_send_opt_abort(ioc);
+}
 return NULL;
 }
 
diff --git a/nbd/trace-events b/nbd/trace-events
index 48a4f27682..ea44e6963f 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -9,9 +9,10 @@ nbd_opt_go_info_unknown(int info, const char *name) "Ignoring 
unknown info %d (%
 nbd_opt_go_info_block_size(uint32_t minimum, uint32_t preferred, uint32_t 
maximum) "Block sizes are 0x%" PRIx32 ", 0x%" PRIx32 ", 0x%" PRIx32
 nbd_receive_query_exports_start(const char *wantname) "Querying export list 
for '%s'"
 nbd_receive_query_exports_success(const char *wantname) "Found desired export 
name '%s'"
-nbd_receive_starttls_request(void) "Requesting TLS from server"
-nbd_receive_starttls_reply(void) "Getting TLS reply from server"
-nbd_receive_starttls_new_client(void) "TLS request approved, setting up TLS"
+nbd_receive_simple_option_request(int opt, const char *name) "Requesting 
option %d (%s) from server"
+nbd_receive_simple_option_reply(int opt, const char *name) "Getting reply for 
option %d (%s) from server"
+nbd_receive_simple_option_approved(int opt, const char *name) "Option %d (%s) 
approved"
+nbd_receive_starttls_new_client(void) "Setting up TLS"
 nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake"
 nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving 
negotiation tlscreds=%p hostname=%s"
 nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
-- 
2.11.1

[Qemu-devel] [PATCH v10 06/20] dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes

[Eric Blake]

We're already reporting bytes for bdrv_dirty_bitmap_granularity();
mixing bytes and sectors in our return values is a recipe for
confusion.  A later cleanup will convert dirty bitmap internals
to be entirely byte-based, but in the meantime, we should report
the bitmap size in bytes.

The only external caller in qcow2-bitmap.c is temporarily more verbose
(because it is still using sector-based math), but will later be
switched to track progress by bytes instead of sectors.

Signed-off-by: Eric Blake 
Reviewed-by: John Snow 
Reviewed-by: Kevin Wolf 
Reviewed-by: Fam Zheng 

---
v8: no change
v7: split external from internal change [Kevin], drop R-b
v6: no change
v5: fix bdrv_dirty_bitmap_truncate [John], drop R-b
v4: retitle from "Track size in bytes", rebase to persistent bitmaps,
round up when converting bytes to sectors
v3: no change
v2: tweak commit message, no code change
---
 block/dirty-bitmap.c |  2 +-
 block/qcow2-bitmap.c | 14 --
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index ee164fb518..75af32 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -175,7 +175,7 @@ void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)

 int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
 {
-return bitmap->size;
+return bitmap->size * BDRV_SECTOR_SIZE;
 }

 const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index b3ee4c794a..65122e9ae1 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -295,10 +295,11 @@ static int load_bitmap_data(BlockDriverState *bs,
 BDRVQcow2State *s = bs->opaque;
 uint64_t sector, sbc;
 uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
+uint64_t bm_sectors = DIV_ROUND_UP(bm_size, BDRV_SECTOR_SIZE);
 uint8_t *buf = NULL;
 uint64_t i, tab_size =
 size_to_clusters(s,
-bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size));
+bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_sectors));

 if (tab_size != bitmap_table_size || tab_size > BME_MAX_TABLE_SIZE) {
 return -EINVAL;
@@ -307,7 +308,7 @@ static int load_bitmap_data(BlockDriverState *bs,
 buf = g_malloc(s->cluster_size);
 sbc = sectors_covered_by_bitmap_cluster(s, bitmap);
 for (i = 0, sector = 0; i < tab_size; ++i, sector += sbc) {
-uint64_t count = MIN(bm_size - sector, sbc);
+uint64_t count = MIN(bm_sectors - sector, sbc);
 uint64_t entry = bitmap_table[i];
 uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK;

@@ -1077,13 +1078,14 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs,
 int64_t sector;
 uint64_t sbc;
 uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
+uint64_t bm_sectors = DIV_ROUND_UP(bm_size, BDRV_SECTOR_SIZE);
 const char *bm_name = bdrv_dirty_bitmap_name(bitmap);
 uint8_t *buf = NULL;
 BdrvDirtyBitmapIter *dbi;
 uint64_t *tb;
 uint64_t tb_size =
 size_to_clusters(s,
-bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size));
+bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_sectors));

 if (tb_size > BME_MAX_TABLE_SIZE ||
 tb_size * s->cluster_size > BME_MAX_PHYS_SIZE)
@@ -1101,7 +1103,7 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs,
 dbi = bdrv_dirty_iter_new(bitmap, 0);
 buf = g_malloc(s->cluster_size);
 sbc = sectors_covered_by_bitmap_cluster(s, bitmap);
-assert(DIV_ROUND_UP(bm_size, sbc) == tb_size);
+assert(DIV_ROUND_UP(bm_sectors, sbc) == tb_size);

 while ((sector = bdrv_dirty_iter_next(dbi)) != -1) {
 uint64_t cluster = sector / sbc;
@@ -1109,7 +,7 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs,
 int64_t off;

 sector = cluster * sbc;
-end = MIN(bm_size, sector + sbc);
+end = MIN(bm_sectors, sector + sbc);
 write_size =
 bdrv_dirty_bitmap_serialization_size(bitmap, sector, end - sector);
 assert(write_size <= s->cluster_size);
@@ -1141,7 +1143,7 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs,
 goto fail;
 }

-if (end >= bm_size) {
+if (end >= bm_sectors) {
 break;
 }

-- 
2.13.5

[Qemu-devel] [PATCH v10 07/20] dirty-bitmap: Track bitmap size by bytes

[Eric Blake]

We are still using an internal hbitmap that tracks a size in sectors,
with the granularity scaled down accordingly, because it lets us
use a shortcut for our iterators which are currently sector-based.
But there's no reason we can't track the dirty bitmap size in bytes,
since it is (mostly) an internal-only variable (remember, the size
is how many bytes are covered by the bitmap, not how many bytes the
bitmap occupies).  A later cleanup will convert dirty bitmap
internals to be entirely byte-based, eliminating the intermediate
sector rounding added here; and technically, since bdrv_getlength()
already rounds up to sectors, our use of DIV_ROUND_UP is more for
theoretical completeness than for any actual rounding.

Use is_power_of_2() while at it, instead of open-coding that.

Signed-off-by: Eric Blake 
Reviewed-by: John Snow 
Reviewed-by: Kevin Wolf 
Reviewed-by: Fam Zheng 

---
v8: rebase to earlier truncate changes, R-b kept
v7: split external from internal [Kevin], drop R-b
v6: no change
v5: fix bdrv_dirty_bitmap_truncate [John], drop R-b
v4: retitle from "Track size in bytes", rebase to persistent bitmaps,
round up when converting bytes to sectors
v3: no change
v2: tweak commit message, no code change
---
 block/dirty-bitmap.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 75af32..6d32554b4e 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -42,7 +42,7 @@ struct BdrvDirtyBitmap {
 HBitmap *meta;  /* Meta dirty bitmap */
 BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
 char *name; /* Optional non-empty unique ID */
-int64_t size;   /* Size of the bitmap (Number of sectors) */
+int64_t size;   /* Size of the bitmap, in bytes */
 bool disabled;  /* Bitmap is disabled. It ignores all writes to
the device */
 int active_iterators;   /* How many iterators are active */
@@ -115,17 +115,14 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 {
 int64_t bitmap_size;
 BdrvDirtyBitmap *bitmap;
-uint32_t sector_granularity;

-assert((granularity & (granularity - 1)) == 0);
+assert(is_power_of_2(granularity) && granularity >= BDRV_SECTOR_SIZE);

 if (name && bdrv_find_dirty_bitmap(bs, name)) {
 error_setg(errp, "Bitmap already exists: %s", name);
 return NULL;
 }
-sector_granularity = granularity >> BDRV_SECTOR_BITS;
-assert(sector_granularity);
-bitmap_size = bdrv_nb_sectors(bs);
+bitmap_size = bdrv_getlength(bs);
 if (bitmap_size < 0) {
 error_setg_errno(errp, -bitmap_size, "could not get length of device");
 errno = -bitmap_size;
@@ -133,7 +130,12 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState 
*bs,
 }
 bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->mutex = >dirty_bitmap_mutex;
-bitmap->bitmap = hbitmap_alloc(bitmap_size, ctz32(sector_granularity));
+/*
+ * TODO - let hbitmap track full granularity. For now, it is tracking
+ * only sector granularity, as a shortcut for our iterators.
+ */
+bitmap->bitmap = hbitmap_alloc(DIV_ROUND_UP(bitmap_size, BDRV_SECTOR_SIZE),
+   ctz32(granularity) - BDRV_SECTOR_BITS);
 bitmap->size = bitmap_size;
 bitmap->name = g_strdup(name);
 bitmap->disabled = false;
@@ -175,7 +177,7 @@ void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)

 int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
 {
-return bitmap->size * BDRV_SECTOR_SIZE;
+return bitmap->size;
 }

 const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
@@ -305,14 +307,13 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
 void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes)
 {
 BdrvDirtyBitmap *bitmap;
-int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE);

 bdrv_dirty_bitmaps_lock(bs);
 QLIST_FOREACH(bitmap, >dirty_bitmaps, list) {
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
 assert(!bitmap->active_iterators);
-hbitmap_truncate(bitmap->bitmap, size);
-bitmap->size = size;
+hbitmap_truncate(bitmap->bitmap, DIV_ROUND_UP(bytes, 
BDRV_SECTOR_SIZE));
+bitmap->size = bytes;
 }
 bdrv_dirty_bitmaps_unlock(bs);
 }
@@ -549,7 +550,8 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, 
HBitmap **out)
 hbitmap_reset_all(bitmap->bitmap);
 } else {
 HBitmap *backup = bitmap->bitmap;
-bitmap->bitmap = hbitmap_alloc(bitmap->size,
+bitmap->bitmap = hbitmap_alloc(DIV_ROUND_UP(bitmap->size,
+BDRV_SECTOR_SIZE),
hbitmap_granularity(backup));