Re: [Qemu-devel] [PATCH v4 3/3] net: tap: replace snprintf with g_strdup_printf calls

[P J P]

+-- On Tue, 23 Jul 2019, Li Qiang wrote --+
| Stefan Hajnoczi  于2019年7月23日周二 下午9:03写道：
| > On Tue, Jul 23, 2019 at 04:17:54PM +0530, P J P wrote:
| > > -snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s",
| > > - helper, "--use-vnet", fd_buf, br_buf);
| > > +helper_cmd = g_strdup_printf("%s %s %s %s", helper,
| > > +"--use-vnet", fd_buf, br_buf ? br_buf : "");
| >
| > The change to the br_buf argument isn't covered in the commit
| > description.  Why did you change this, was it a bug, etc?
| 
| IIUC, if we pass the NULL argument in g_strdup_printf, the 'helper_cmd' will 
| contain the '(null)' char.

Yep, right.

| Reviewed-by: Li Qiang 

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [RFC v1 05/18] vfio/pci: add pasid alloc/free implementation

[Liu, Yi L]

> From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On Behalf
> Of David Gibson
> Sent: Tuesday, July 23, 2019 11:58 AM
> To: Liu, Yi L 
> Subject: Re: [RFC v1 05/18] vfio/pci: add pasid alloc/free implementation
> 
> On Mon, Jul 22, 2019 at 07:02:51AM +, Liu, Yi L wrote:
> > > From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org]
> > > On Behalf Of David Gibson
> > > Sent: Wednesday, July 17, 2019 11:07 AM
> > > To: Liu, Yi L 
> > > Subject: Re: [RFC v1 05/18] vfio/pci: add pasid alloc/free
> > > implementation
> > >
> > > On Tue, Jul 16, 2019 at 10:25:55AM +, Liu, Yi L wrote:
> > > > > From: kvm-ow...@vger.kernel.org
> > > > > [mailto:kvm-ow...@vger.kernel.org] On
> > > Behalf
> > > > > Of David Gibson
> > > > > Sent: Monday, July 15, 2019 10:55 AM
> > > > > To: Liu, Yi L 
> > > > > Subject: Re: [RFC v1 05/18] vfio/pci: add pasid alloc/free
> > > > > implementation
> > > > >
> > > > > On Fri, Jul 05, 2019 at 07:01:38PM +0800, Liu Yi L wrote:
> > > > > > This patch adds vfio implementation 
> > > > > > PCIPASIDOps.alloc_pasid/free_pasid().
> > > > > > These two functions are used to propagate guest pasid
> > > > > > allocation and free requests to host via vfio container ioctl.
> > > > >
> > > > > As I said in an earlier comment, I think doing this on the
> > > > > device is conceptually incorrect.  I think we need an explcit
> > > > > notion of an SVM context (i.e. the namespace in which all the
> > > > > PASIDs live) - which will IIUC usually be shared amongst
> > > > > multiple devices.  The create and free PASID requests should be on 
> > > > > that object.
> > > >
> > > > Actually, the allocation is not doing on this device. System wide,
> > > > it is done on a container. So not sure if it is the API interface
> > > > gives you a sense that this is done on device.
> > >
> > > Sorry, I should have been clearer.  I can see that at the VFIO level
> > > it is done on the container.  However the function here takes a bus
> > > and devfn, so this qemu internal interface is per-device, which
> > > doesn't really make sense.
> >
> > Got it. The reason here is to pass the bus and devfn info, so that
> > VFIO can figure out a container for the operation. So far in QEMU,
> > there is no good way to connect the vIOMMU emulator and VFIO regards
> > to SVM.
> 
> Right, and I think that's an indication that we're not modelling something in 
> qemu
> that we should be.
> 
> > hw/pci layer is a choice based on some previous discussion. But yes, I
> > agree with you that we may need to have an explicit notion for SVM. Do
> > you think it is good to introduce a new abstract layer for SVM (may
> > name as SVMContext).
> 
> I think so, yes.
> 
> If nothing else, I expect we'll need this concept if we ever want to be able 
> to
> implement SVM for emulated devices (which could be useful for debugging, even 
> if
> it's not something you'd do in production).
> 
> > The idea would be that vIOMMU maintain the SVMContext instances and
> > expose explicit interface for VFIO to get it. Then VFIO register
> > notifiers on to the SVMContext. When vIOMMU emulator wants to do PASID
> > alloc/free, it fires the corresponding notifier. After call into VFIO,
> > the notifier function itself figure out the container it is bound. In
> > this way, it's the duty of vIOMMU emulator to figure out a proper
> > notifier to fire. From interface point of view, it is no longer
> > per-device.
> 
> Exactly.

Cool, let me prepare another version with the ideas. Thanks for your
review. :-)

Regards,
Yi Liu

> > Also, it leaves the PASID management details to vIOMMU emulator as it
> > can be vendor specific. Does it make sense?
> > Also, I'd like to know if you have any other idea on it. That would
> > surely be helpful. :-)
> >
> > > > Also, curious on the SVM context
> > > > concept, do you mean it a per-VM context or a per-SVM usage context?
> > > > May you elaborate a little more. :-)
> > >
> > > Sorry, I'm struggling to find a good term for this.  By "context" I
> > > mean a namespace containing a bunch of PASID address spaces, those
> > > PASIDs are then visible to some group of devices.
> >
> > I see. May be the SVMContext instance above can include multiple PASID
> > address spaces. And again, I think this relationship should be
> > maintained in vIOMMU emulator.

Re: [Qemu-devel] [PATCH v3] qapi: add dirty-bitmaps to query-named-block-nodes result

[Markus Armbruster]

John Snow  writes:

> From: Vladimir Sementsov-Ogievskiy 
>
> Let's add a possibility to query dirty-bitmaps not only on root nodes.
> It is useful when dealing both with snapshots and incremental backups.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> [Added deprecation information. --js]
> Signed-off-by: John Snow 
> ---
>  block/qapi.c |  5 +
>  qapi/block-core.json |  6 +-
>  qemu-deprecated.texi | 12 
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 917435f022..15f1030264 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -79,6 +79,11 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
>  info->backing_file = g_strdup(bs->backing_file);
>  }
>  
> +if (!QLIST_EMPTY(>dirty_bitmaps)) {
> +info->has_dirty_bitmaps = true;
> +info->dirty_bitmaps = bdrv_query_dirty_bitmaps(bs);
> +}
> +
>  info->detect_zeroes = bs->detect_zeroes;
>  
>  if (blk && blk_get_public(blk)->throttle_group_member.throttle_state) {
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 0d43d4f37c..9210ae233d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -360,6 +360,9 @@
>  # @write_threshold: configured write threshold for the device.
>  #   0 if disabled. (Since 2.3)
>  #
> +# @dirty-bitmaps: dirty bitmaps information (only present if node
> +# has one or more dirty bitmaps) (Since 4.2)
> +#
>  # Since: 0.14.0
>  #
>  ##
> @@ -378,7 +381,7 @@
>  '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
>  '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
>  '*iops_size': 'int', '*group': 'str', 'cache': 
> 'BlockdevCacheInfo',
> -'write_threshold': 'int' } }
> +'write_threshold': 'int', '*dirty-bitmaps': ['BlockDirtyInfo'] } 
> }
>  
>  ##
>  # @BlockDeviceIoStatus:
> @@ -656,6 +659,7 @@
>  #
>  # @dirty-bitmaps: dirty bitmaps information (only present if the
>  # driver has one or more dirty bitmaps) (Since 2.0)
> +# Deprecated in 4.2; see BlockDirtyInfo instead.
>  #
>  # @io-status: @BlockDeviceIoStatus. Only present if the device
>  # supports it and the VM is configured to stop on errors
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index c90b08d553..6374b66546 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -134,6 +134,18 @@ The ``status'' field of the ``BlockDirtyInfo'' 
> structure, returned by
>  the query-block command is deprecated. Two new boolean fields,
>  ``recording'' and ``busy'' effectively replace it.
>  
> +@subsection query-block result field dirty-bitmaps (Since 4.2)
> +
> +The ``dirty-bitmaps`` field of the ``BlockInfo`` structure, returned by
> +the query-block command is itself now deprecated. The ``dirty-bitmaps``
> +field of the ``BlockDeviceInfo`` struct should be used instead, which is the
> +type of the ``inserted`` field in query-block replies, as well as the
> +type of array items in query-named-block-nodes.

Would the text be clearer if it talked only about commands, not about
types?

Here's my (laconic) try:

   @subsection query-block result field dirty-bitmaps (Since 4.2)

   In the result of query-block, member ``dirty-bitmaps'' has been moved
   into member ``inserted''.

Aside: same for existing @subsection query-block result field
dirty-bitmaps[i].status (since 4.0).

> +Since the ``dirty-bitmaps`` field is optionally present in both the old and
> +new locations, clients must use introspection to learn where to anticipate
> +the field if/when it does appear in command output.
> +

I find this hint a bit confusing.  Do we need it?

If yes, laconic me again:

   Clients should use introspection to learn whether ``dirty-bitmaps'' is
   in the new location.

>  @subsection query-cpus (since 2.12.0)
>  
>  The ``query-cpus'' command is replaced by the ``query-cpus-fast'' command.

[Qemu-devel] [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

[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/599958

Title:
  Timedrift problems with Win7: hpet missing time drift fixups

Status in QEMU:
  Expired

Bug description:
  We've been finding timedrift issues witth Win7 under qemu-kvm on our
  daily testing

  kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load FAIL1   Time 
drift too large after rest period: 38.63%
  kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot   FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
  kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migrationFAIL1   
Time drift too large at iteration 2: 3.08 seconds

  Steps to reproduce:

  timedrift.with_load

  1) Log into a guest.
  2) Take a time reading from the guest and host.
  3) Run load on the guest and host.
  4) Take a second time reading.
  5) Stop the load and rest for a while.
  6) Take a third time reading.
  7) If the drift immediately after load is higher than a user-
  specified value (in %), fail.
  If the drift after the rest period is higher than a user-specified value,
  fail.

  timedrift.with_migration

  1) Log into a guest.
  2) Take a time reading from the guest and host.
  3) Migrate the guest.
  4) Take a second time reading.
  5) If the drift (in seconds) is higher than a user specified value, fail.

  timedrift.with_reboot

  1) Log into a guest.
  2) Take a time reading from the guest and host.
  3) Reboot the guest.
  4) Take a second time reading.
  5) If the drift (in seconds) is higher than a user specified value, fail.

  This bug is to register those issues and keep an eye on them.

  Attached, some logs from the autotest tests executed on the guest

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

[Qemu-devel] [Bug 1585971] Re: Host system crashes on qemu with DMA remapping

[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/1585971

Title:
  Host system crashes on qemu with DMA remapping

Status in QEMU:
  Expired

Bug description:
  Hy,

  the host system crashes completely, when i try to pass an physical
  device without boot option intel_iommu=on set. In older kernel
  versions you didn't have to pass that option.

  I wonder if this can be easily checked by asking iommu state, avoiding
  a crash of the complete system.

  My data:
  cpu model: Intel(R) Core(TM) i7 CPU
  qemu version: 2.4.1-r2
  kernel version: 4.1.2 x86_64
  command line: 
  qemu-system-x86_64 -enable-kvm -drive 
file=/vms/prod/fw/fw.iso,if=virtio,format=raw -drive 
file=/vms/prod/fw/swap,if=virtio,format=raw -drive 
file=/vms/prod/fw/fwdata.iso,if=virtio,format=raw -m 512 -nographic -kernel 
/data/kernels/vmlinuz-2.6.36-gentoo-r8 -append "root=/dev/vda console=ttyS0 
earlyprintk=serial" -net nic,model=virtio,macaddr=DE:AD:BE:EF:2D:AD -net 
tap,ifname=tapfw0,script=/etc/qemu/qemu-ifup -device pci-assign,host=03:00.0

  There are also more detailed informations (if needed) here:
  https://forums.gentoo.org/viewtopic-p-7923976.html

  Thanks,
  Antonios.

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

Re: [Qemu-devel] [PATCH] pc-dimm: fix crash when invalid slot number is used

[David Gibson]

On Tue, Jul 23, 2019 at 12:08:59PM -0400, Igor Mammedov wrote:
> QEMU will crash with:
>   Segmentation fault (core dumped)
> when negative slot number is used, ex:
>   qemu-system-x86_64 -m 1G,maxmem=20G,slots=256 \
>   -object memory-backend-ram,id=mem1,size=1G \
>   -device pc-dimm,id=dimm1,memdev=mem1,slot=-2
> 
> fix it by checking that slot number is within valid range.
> 
> Signed-off-by: Igor Mammedov 

Reviewed-by: David Gibson 

> ---
>  hw/mem/pc-dimm.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index b1239fd0d3..29c785799c 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -38,6 +38,13 @@ void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState 
> *machine,
>  
>  slot = object_property_get_int(OBJECT(dimm), PC_DIMM_SLOT_PROP,
> _abort);
> +if ((slot < 0 || slot >= machine->ram_slots) &&
> + slot != PC_DIMM_UNASSIGNED_SLOT) {
> +error_setg(_err, "invalid slot number, valid range is [0-%"
> +   PRIu64 "]", machine->ram_slots - 1);
> +goto out;
> +}
> +
>  slot = pc_dimm_get_free_slot(slot == PC_DIMM_UNASSIGNED_SLOT ? NULL : 
> ,
>   machine->ram_slots, _err);
>  if (local_err) {

-- 
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] ppc/pnv: Generate phandle for the "interrupt-parent" property

[David Gibson]

On Tue, Jul 23, 2019 at 11:01:38AM +0200, Cédric Le Goater wrote:
> Devices such as the BT or serial devices require a valid
> "interrupt-parent" phandle in the device tree and it is currently
> empty (0x0). It was not a problem until now but since OpenFirmare
> started using a recent libdft (>= 1.4.7), petitboot fails to boot the
> system image with error :
> 
>dtc_resize: fdt_open_into returned FDT_ERR_BADMAGIC
> 
> Provide a phandle for the LPC bus.
> 
> Suggested-by: Greg Kurz 
> Signed-off-by: Cédric Le Goater 

I've applied this, since it looks to be correct.

But.. can you connect the dots for me in how this being missing
results in a BADMAGIC error??

> ---
>  hw/ppc/pnv.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 36f57479a1f5..2deceecdccb5 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -431,9 +431,14 @@ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
>  .fdt = fdt,
>  .offset = isa_offset,
>  };
> +uint32_t phandle;
>  
>  _FDT((fdt_setprop(fdt, isa_offset, "primary", NULL, 0)));
>  
> +phandle = qemu_fdt_alloc_phandle(fdt);
> +assert(phandle > 0);
> +_FDT((fdt_setprop_cell(fdt, isa_offset, "phandle", phandle)));
> +
>  /* ISA devices are not necessarily parented to the ISA bus so we
>   * can not use object_child_foreach() */
>  qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, 
> NULL,

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


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] migration/postcopy: cleanup multifd after postcopy finish

[Wei Yang]

In case we enable multifd, not forget to cleanup it.

Signed-off-by: Wei Yang 
---
 migration/savevm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index 0105068579..27eef72c9d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1738,6 +1738,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
 {
 MigrationIncomingState *mis = migration_incoming_get_current();
 QEMUFile *f = mis->from_src_file;
+Error *local_err = NULL;
 int load_res;
 
 migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
@@ -1800,6 +1801,9 @@ static void *postcopy_ram_listen_thread(void *opaque)
  */
 migration_incoming_state_destroy();
 qemu_loadvm_state_cleanup();
+if (multifd_load_cleanup(_err) != 0) {
+error_report_err(local_err);
+}
 
 rcu_unregister_thread();
 postcopy_state_set(POSTCOPY_INCOMING_END, NULL);
-- 
2.17.1

Re: [Qemu-devel] [PATCH 2/2] migration: extract ram_load_precopy

[Wei Yang]

On Tue, Jul 23, 2019 at 05:47:03PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> After cleanup, it would be clear to audience there are two cases
>> ram_load:
>> 
>>   * precopy
>>   * postcopy
>> 
>> And it is not necessary to check postcopy_running on each iteration for
>> precopy.
>> 
>> Signed-off-by: Wei Yang 
>> ---
>>  migration/ram.c | 73 +++--
>>  1 file changed, 46 insertions(+), 27 deletions(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 6bfdfae16e..5f6f07b255 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -4200,40 +4200,26 @@ static void colo_flush_ram_cache(void)
>>  trace_colo_flush_ram_cache_end();
>>  }
>>  
>> -static int ram_load(QEMUFile *f, void *opaque, int version_id)
>> +/**
>> + * ram_load_precopy: load a page in precopy case
>
>This comment is wrong - although I realise you copied it from the
>postcopy case; they don't just load a single page; they load 'pages'
>

Thanks for pointing out.

Actually, I got one confusion in these two load. Compare these two cases, I
found precopy would handle two more cases:

  * precopy:  RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
  RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE
  * postcopy: RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE

Why postcopy doesn't need to handle the other two cases? Function
ram_save_target_page() does the same thing in precopy and postcopy. I don't
find the reason the behavior differs. Would you mind giving me a hint?

>Other than that, I think it's OK, so:
>
>
>Reviewed-by: Dr. David Alan Gilbert 
>

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH] migration/multifd: multifd_load_cleanup() always return 0

[Wei Yang]

On Tue, Jul 23, 2019 at 04:39:08PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> multifd_load_cleanup() always return 0 and never use the errp, just
>> cleanup a little.
>> 
>> Signed-off-by: Wei Yang 
>
>I'd rather leave this; it wouldn't be that odd for us to add an error
>check there later.
>

Sounds reasonable :-)

>Dave

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH] migration/postcopy: use mis->bh instead of allocating a QEMUBH

[Wei Yang]

On Tue, Jul 23, 2019 at 06:23:53PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> For migration incoming side, it either quit in precopy or postcopy. It
>> is save to use the mis->bh for both instead of allocating a dedicated
>
>  'safe' not 'save'
>

oops,  thanks for pointing out.

>> QEMUBH for postcopy.
>> 
>> Signed-off-by: Wei Yang 
>
>Yes, I think that's OK; it's a little subtle.  Once postcopy is in 'run'
>then the main incoming code will exit before running it's bh, so you're
>right that it's free for us to use.
>
>
>Reviewed-by: Dr. David Alan Gilbert 
>
>> ---
>>  migration/savevm.c | 17 -
>>  1 file changed, 4 insertions(+), 13 deletions(-)
>> 
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 25fe7ea05a..0105068579 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1856,16 +1856,10 @@ static int 
>> loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
>>  return 0;
>>  }
>>  
>> -
>> -typedef struct {
>> -QEMUBH *bh;
>> -} HandleRunBhData;
>> -
>>  static void loadvm_postcopy_handle_run_bh(void *opaque)
>>  {
>>  Error *local_err = NULL;
>> -HandleRunBhData *data = opaque;
>> -MigrationIncomingState *mis = migration_incoming_get_current();
>> +MigrationIncomingState *mis = opaque;
>>  
>>  /* TODO we should move all of this lot into postcopy_ram.c or a shared 
>> code
>>   * in migration.c
>> @@ -1897,8 +1891,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
>>  runstate_set(RUN_STATE_PAUSED);
>>  }
>>  
>> -qemu_bh_delete(data->bh);
>> -g_free(data);
>> +qemu_bh_delete(mis->bh);
>>  }
>>  
>>  /* After all discards we can start running and asking for pages */
>> @@ -1906,7 +1899,6 @@ static int 
>> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
>>  {
>>  PostcopyState old_ps = POSTCOPY_INCOMING_LISTENING;
>>  PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING, 
>> _ps);
>> -HandleRunBhData *data;
>>  
>>  trace_loadvm_postcopy_handle_run();
>>  if (ps != old_ps) {
>> @@ -1914,9 +1906,8 @@ static int 
>> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
>>  return -1;
>>  }
>>  
>> -data = g_new(HandleRunBhData, 1);
>> -data->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, data);
>> -qemu_bh_schedule(data->bh);
>> +mis->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, mis);
>> +qemu_bh_schedule(mis->bh);
>>  
>>  /* We need to finish reading the stream from the package
>>   * and also stop reading anything more from the stream that loaded the
>> -- 
>> 2.17.1
>> 
>--
>Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

-- 
Wei Yang
Help you, Help me

[Qemu-devel] [Patch v2] migration/postcopy: make PostcopyDiscardState a static variable

[Wei Yang]

In postcopy-ram.c, we provide three functions to discard certain
RAMBlock range:

  * postcopy_discard_send_init()
  * postcopy_discard_send_range()
  * postcopy_discard_send_finish()

Currently, we allocate/deallocate PostcopyDiscardState for each RAMBlock
on sending discard information to destination. This is not necessary and
the same data area could be reused for each RAMBlock.

This patch defines PostcopyDiscardState a static variable. By doing so:

  1) avoid memory allocation and deallocation to the system
  2) avoid potential failure of memory allocation
  3) hide some details for their users

Signed-off-by: Wei Yang 

---
v2:
  * make it a static variable, suggested by Dave
---
 migration/postcopy-ram.c | 70 +---
 migration/postcopy-ram.h | 13 +++-
 migration/ram.c  | 30 +++--
 3 files changed, 46 insertions(+), 67 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 7b3e198538..cf2400b47e 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1375,22 +1375,16 @@ void 
postcopy_fault_thread_notify(MigrationIncomingState *mis)
  *   asking to discard individual ranges.
  *
  * @ms: The current migration state.
- * @offset: the bitmap offset of the named RAMBlock in the migration
- *   bitmap.
+ * @offset: the bitmap offset of the named RAMBlock in the migration bitmap.
  * @name: RAMBlock that discards will operate on.
- *
- * returns: a new PDS.
  */
-PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms,
- const char *name)
+static PostcopyDiscardState pds = {0};
+void postcopy_discard_send_init(MigrationState *ms, const char *name)
 {
-PostcopyDiscardState *res = g_malloc0(sizeof(PostcopyDiscardState));
-
-if (res) {
-res->ramblock_name = name;
-}
-
-return res;
+pds.ramblock_name = name;
+pds.cur_entry = 0;
+pds.nsentwords = 0;
+pds.nsentcmds = 0;
 }
 
 /**
@@ -1399,30 +1393,29 @@ PostcopyDiscardState 
*postcopy_discard_send_init(MigrationState *ms,
  *   be sent later.
  *
  * @ms: Current migration state.
- * @pds: Structure initialised by postcopy_discard_send_init().
  * @start,@length: a range of pages in the migration bitmap in the
  *   RAM block passed to postcopy_discard_send_init() (length=1 is one page)
  */
-void postcopy_discard_send_range(MigrationState *ms, PostcopyDiscardState *pds,
-unsigned long start, unsigned long length)
+void postcopy_discard_send_range(MigrationState *ms, unsigned long start,
+ unsigned long length)
 {
 size_t tp_size = qemu_target_page_size();
 /* Convert to byte offsets within the RAM block */
-pds->start_list[pds->cur_entry] = start  * tp_size;
-pds->length_list[pds->cur_entry] = length * tp_size;
-trace_postcopy_discard_send_range(pds->ramblock_name, start, length);
-pds->cur_entry++;
-pds->nsentwords++;
+pds.start_list[pds.cur_entry] = start  * tp_size;
+pds.length_list[pds.cur_entry] = length * tp_size;
+trace_postcopy_discard_send_range(pds.ramblock_name, start, length);
+pds.cur_entry++;
+pds.nsentwords++;
 
-if (pds->cur_entry == MAX_DISCARDS_PER_COMMAND) {
+if (pds.cur_entry == MAX_DISCARDS_PER_COMMAND) {
 /* Full set, ship it! */
 qemu_savevm_send_postcopy_ram_discard(ms->to_dst_file,
-  pds->ramblock_name,
-  pds->cur_entry,
-  pds->start_list,
-  pds->length_list);
-pds->nsentcmds++;
-pds->cur_entry = 0;
+  pds.ramblock_name,
+  pds.cur_entry,
+  pds.start_list,
+  pds.length_list);
+pds.nsentcmds++;
+pds.cur_entry = 0;
 }
 }
 
@@ -1431,24 +1424,21 @@ void postcopy_discard_send_range(MigrationState *ms, 
PostcopyDiscardState *pds,
  * bitmap code. Sends any outstanding discard messages, frees the PDS
  *
  * @ms: Current migration state.
- * @pds: Structure initialised by postcopy_discard_send_init().
  */
-void postcopy_discard_send_finish(MigrationState *ms, PostcopyDiscardState 
*pds)
+void postcopy_discard_send_finish(MigrationState *ms)
 {
 /* Anything unsent? */
-if (pds->cur_entry) {
+if (pds.cur_entry) {
 qemu_savevm_send_postcopy_ram_discard(ms->to_dst_file,
-  pds->ramblock_name,
-  pds->cur_entry,
-  pds->start_list,
-  pds->length_list);
-pds->nsentcmds++;
+  

Re: [Qemu-devel] [PATCH] migration/postcopy: use static PostcopyDiscardState instead of allocating it for each block

[Wei Yang]

On Tue, Jul 23, 2019 at 04:42:12PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> On Fri, Jul 19, 2019 at 06:41:28PM +0100, Dr. David Alan Gilbert wrote:
>> >* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> >> Even we need to do discard for each RAMBlock, we still can leverage the
>> >> same memory space to store the information.
>> >> 
>> >> By doing so, we avoid memory allocation and deallocation to the system
>> >> and also avoid potential failure of memory allocation which breaks the
>> >> migration.
>> >> 
>> >> Signed-off-by: Wei Yang 
>> >> ---
>> >>  migration/postcopy-ram.c | 16 +++-
>> >>  1 file changed, 7 insertions(+), 9 deletions(-)
>> >> 
>> >> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> >> index 9faacacc9e..2e6b076bb7 100644
>> >> --- a/migration/postcopy-ram.c
>> >> +++ b/migration/postcopy-ram.c
>> >> @@ -1377,8 +1377,7 @@ void 
>> >> postcopy_fault_thread_notify(MigrationIncomingState *mis)
>> >>   *   asking to discard individual ranges.
>> >>   *
>> >>   * @ms: The current migration state.
>> >> - * @offset: the bitmap offset of the named RAMBlock in the migration
>> >> - *   bitmap.
>> >> + * @offset: the bitmap offset of the named RAMBlock in the migration 
>> >> bitmap.
>> >>   * @name: RAMBlock that discards will operate on.
>> >>   *
>> >>   * returns: a new PDS.
>> >> @@ -1386,13 +1385,14 @@ void 
>> >> postcopy_fault_thread_notify(MigrationIncomingState *mis)
>> >>  PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms,
>> >>   const char *name)
>> >>  {
>> >> -PostcopyDiscardState *res = g_malloc0(sizeof(PostcopyDiscardState));
>> >> +static PostcopyDiscardState res = {0};
>> >
>> >Do you think it would be better to make this a static at the top of
>> >migration/postcopy-ram.c and then we could remove the pds parameters
>> >from postcopy_discard_send_range and friends?
>> 
>> Just took a look into this one. One problem is not all its friends are in
>> migration/postcopy-ram.c
>> 
>> For example, postcopy_chunk_hostpages_pass() is in migration/ram.c.
>
>But doesn't that just pass teh pds back to postcopy_discard_send_range
>which is in postcopy-ram ?

You are right, I didn't notice this.

>
>Dave
>

Re: [Qemu-devel] high-level view of packet processing for virtio NIC?

[Dongli Zhang]

Hi Chris,

On 7/24/19 12:18 AM, Chris Friesen wrote:
> Hi,
> 
> I'm looking for information on what the qemu architecture looks like for
> processing virtio network packets in a two-vCPU guest.
> 
> It looks like there's an IO thread doing a decent fraction of the work, 
> separate
> from the vCPU threads--is that correct?  There's no disk involved in this 
> case,
> purely network packet processing.
> 

I suggest use gdb to have a view.

To use gdb, the debug option should be enabled in source code.

# ./configure --target-list=x86_64-softmmu --enable-debug
# make -j8 > /dev/null

# sudo gdb ./x86_64-softmmu/qemu-system-x86_64

(gdb) run -machine pc,accel=kvm -vnc :0 -smp 4 -m 4096M -hda
/path/to/ubuntu1804.qcow2 -device virtio-net-pci,netdev=tapnet,mq=true,vectors=9
-netdev
tap,id=tapnet,ifname=tap0,script=/path/to/qemu-ifup,downscript=no,queues=4,vhost=off

gdb would be able to show which thread is running which function. To have a view
on RX path...

(gdb) b virtio_net_do_receive


Once the break point is hit...

Thread 1 "qemu-system-x86" hit Breakpoint 2, virtio_net_do_receive
(nc=0x57705d40, buf=0x5698b2f4 "", size=72) at
/home/zhang/kvm/debug/qemu-4.0.0/hw/net/virtio-net.c:1370
1370rcu_read_lock();
(gdb) bt
#0  virtio_net_do_receive (nc=0x57705d40, buf=0x5698b2f4 "", size=72) at
/home/zhang/kvm/debug/qemu-4.0.0/hw/net/virtio-net.c:1370
#1  0x558f6934 in virtio_net_receive (nc=0x57705d40,
buf=0x5698b2f4 "", size=72) at
/home/zhang/kvm/debug/qemu-4.0.0/hw/net/virtio-net.c:1978
#2  0x55c4208b in nc_sendv_compat (nc=0x57705d40,
iov=0x7fffddb0, iovcnt=1, flags=0) at net/net.c:706
#3  0x55c4214d in qemu_deliver_packet_iov (sender=0x5698ad00,
flags=0, iov=0x7fffddb0, iovcnt=1, opaque=0x57705d40) at net/net.c:734
#4  0x55c44bae in qemu_net_queue_deliver (queue=0x577060d0,
sender=0x5698ad00, flags=0, data=0x5698b2f4 "", size=72) at 
net/queue.c:164
#5  0x55c44cca in qemu_net_queue_send (queue=0x577060d0,
sender=0x5698ad00, flags=0, data=0x5698b2f4 "", size=72,
sent_cb=0x55c56829 ) at net/queue.c:199
#6  0x55c41ef3 in qemu_send_packet_async_with_flags
(sender=0x5698ad00, flags=0, buf=0x5698b2f4 "", size=72,
sent_cb=0x55c56829 ) at net/net.c:660
#7  0x55c41f2b in qemu_send_packet_async (sender=0x5698ad00,
buf=0x5698b2f4 "", size=72, sent_cb=0x55c56829 ) at
net/net.c:667
#8  0x55c56938 in tap_send (opaque=0x5698ad00) at net/tap.c:202
#9  0x55dee376 in aio_dispatch_handlers (ctx=0x568770d0) at
util/aio-posix.c:430
#10 0x55dee509 in aio_dispatch (ctx=0x568770d0) at 
util/aio-posix.c:461
#11 0x55de9a20 in aio_ctx_dispatch (source=0x568770d0, callback=0x0,
user_data=0x0) at util/async.c:261
#12 0x77570197 in g_main_context_dispatch () from
/lib/x86_64-linux-gnu/libglib-2.0.so.0
#13 0x55decdfc in glib_pollfds_poll () at util/main-loop.c:213
#14 0x55dece76 in os_host_main_loop_wait (timeout=198847612) at
util/main-loop.c:236
#15 0x55decf7b in main_loop_wait (nonblocking=0) at util/main-loop.c:512
#16 0x55a0beeb in main_loop () at vl.c:1970
#17 0x55a13230 in main (argc=15, argv=0x7fffe3a8,
envp=0x7fffe428) at vl.c:4604

Dongli Zhang

Re: [Qemu-devel] [ANNOUNCE] QEMU 4.1.0-rc2 is now available

[Michael Roth]

Quoting Michael Roth (2019-07-23 17:12:31)
> Hello,
> 
> On behalf of the QEMU Team, I'd like to announce the availability of the
> third release candidate for the QEMU 4.1 release.  This release is meant
> for testing purposes and should not be used in a production environment.
> 
>   http://download.qemu-project.org/qemu-4.1.0-rc2.tar.xz
>   http://download.qemu-project.org/qemu-4.1.0-rc2.tar.xz.sig
> 
> You can help improve the quality of the QEMU 4.1 release by testing this
> release and reporting bugs on Launchpad:
> 
>   https://bugs.launchpad.net/qemu/
> 
> The release plan, as well a documented known issues for release
> candidates, are available at:
> 
>   http://wiki.qemu.org/Planning/4.1
> 
> Please add entries to the ChangeLog for the 4.1 release below:
> 
>   http://wiki.qemu.org/ChangeLog/4.1
> 
> Thank you to everyone involved!
> 
> Changes since rc1:

Please disregard prior changelog, this is the corrected one:

bf8b024372: Update version for v4.1.0-rc2 release (Peter Maydell)
98808c3d0c: tests/docker: Refresh APT cache before installing new packages on 
Debian (Philippe Mathieu-Daudé)
30edd9fa50: tests/qemu-iotests: Don't use 'seq' in the iotests (Thomas Huth)
4f01046172: tests/qemu-iotests/group: Remove some more tests from the "auto" 
group (Thomas Huth)
bf75d75275: tests/qemu-iotests/check: Allow tests without groups (Thomas Huth)
2b0c4fa13f: tests/docker: invoke the DEBUG shell with --noprofile/--norc (Alex 
Bennée)
3a37c13f8c: travis: enable travis_retry for check phase (Alex Bennée)
6d314626cc: hw/i386: also turn off VMMOUSE is VMPORT is disabled (Alex Bennée)
e54ecc70c3: NSIS: Add missing firmware blobs (Philippe Mathieu-Daudé)
5deb2de1d6: tests/docker: Let the test-mingw test generate a NSIS installer 
(Philippe Mathieu-Daudé)
b3ce38dcf9: buildsys: The NSIS Windows build requires qemu-nsis.bmp installed 
(Philippe Mathieu-Daudé)
f84f9f7584: buildsys: The NSIS Windows build requires the documentation 
installed (Philippe Mathieu-Daudé)
10d7166e42: tests/docker: Install texinfo in the Fedora image (Philippe 
Mathieu-Daudé)
a88819edde: tests/docker: Set the correct cross-PKG_CONFIG_PATH in the MXE 
images (Philippe Mathieu-Daudé)
c054f3f43a: tests/docker: Install the NSIS tools in the MinGW capable images 
(Philippe Mathieu-Daudé)
8d16a31028: tests/docker: Install Sphinx in the Debian images (Philippe 
Mathieu-Daudé)
7887289441: shippable: re-enable the windows cross builds (Alex Bennée)
67bd36beda: tests/dockerfiles: update the win cross builds to stretch (Alex 
Bennée)
601b5575e2: tests/migration-test: don't spam the logs when we fail (Alex Bennée)
56e89a310b: tests/docker: Install Ubuntu images noninteractively (Philippe 
Mathieu-Daudé)
8109b7986a: tests/docker: Install Sphinx in the Fedora image (Philippe 
Mathieu-Daudé)
6d7b9c8c06: tests/docker: Install Sphinx in the Ubuntu images (Philippe 
Mathieu-Daudé)
3c237b5c65: tests/docker: add test-misc for building tools & docs (Alex Bennée)
84963b5bbf: archive-source: also create a stash for submodules (Marc-André 
Lureau)
124e4cfaa4: hw/block/pflash_cfi02: Rewrite a fall through comment (Philippe 
Mathieu-Daudé)
3a283507c0: hw/block/pflash_cfi01: Add missing DeviceReset() handler (Philippe 
Mathieu-Daudé)
5122958258: target/mips: Fix emulation of MSA pack instructions on big endian 
hosts (Aleksandar Markovic)
45152d0502: target/mips: Add 'fall through' comments for handling nanoMips' 
SHXS, SWXS (Aleksandar Markovic)
43eaaaef0e: block: Only the main loop can change AioContexts (Max Reitz)
65181d6381: block: Dec. drained_end_counter before bdrv_wakeup (Max Reitz)
258867d1dc: block/nvme: don't touch the completion entries (Maxim Levitsky)
118d1b6a81: block/nvme: support larger that 512 bytes sector devices (Maxim 
Levitsky)
461bba04bf: block/nvme: fix doorbell stride (Maxim Levitsky)
ddb45afbfb: contrib/elf2dmp: Build download.o with CURL_CFLAGS (Peter Maydell)
a3e3b5217b: configure: Clarify URL to source downloads (Peter Maydell)
8f4821d77e: target/arm: Limit ID register assertions to TCG (Peter Maydell)
bc8c2ecfd0: hw/arm/fsl-imx6ul.c: Remove dead SMP-related code (Peter Maydell)
9bbb4ef991: target/arm: Add missing break statement for Hypervisor Trap 
Exception (Philippe Mathieu-Daudé)
d4b976c0a8: target/i386: sev: fix failed message typos (Jiri Slaby)
2924ab02c2: i386: indicate that 'pconfig' feature was removed intentionally 
(Denis V. Lunev)
5cf42b1c1f: nbd: Initialize reply on failure (Eric Blake)
21e709aa07: build-sys: do no support modules on Windows (Marc-André Lureau)
3bf5de5240: qmp: don't emit the RESET event on wakeup (Nicholas Piggin)
8072aae377: hmp: Print if memory section is registered with an accelerator 
(Alexey Kardashevskiy)
2f950b1e44: test-bitmap: add test for bitmap_set (Wei Yang)
1849f297f5: scsi-generic: Check sense key before request snooping and patching 
(Shin'ichiro Kawasaki)
68fa7ca015: vhost-user-scsi: Call virtio_scsi_common_unrealize() when device 
realize failed (Xie Yongji)
934443c37b: vhost-scsi: 

[Qemu-devel] [PATCH v3 3/4] block/ide/scsi: Set BLK_PERM_SUPPORT_ZONED

[Dmitry Fomichev]

Added a new boolean argument to blkconf_apply_backend_options()
to let the common block code know whether the chosen block
backend can handle zoned block devices or not.

blkconf_apply_backend_options() then sets BLK_PERM_SUPPORT_ZONED
permission accordingly. The raw code can then use this permission
to allow or deny opening a zone device by a particular block driver.

Signed-off-by: Dmitry Fomichev 
Acked-by: Paul Durrant 
---
 hw/block/block.c |  8 ++--
 hw/block/fdc.c   |  4 ++--
 hw/block/nvme.c  |  2 +-
 hw/block/virtio-blk.c|  2 +-
 hw/block/xen-block.c |  2 +-
 hw/ide/qdev.c|  2 +-
 hw/scsi/scsi-disk.c  | 13 +++--
 hw/scsi/scsi-generic.c   |  2 +-
 hw/usb/dev-storage.c |  2 +-
 include/hw/block/block.h |  3 ++-
 10 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index bf56c7612b..23fbe4d567 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -86,7 +86,8 @@ void blkconf_blocksizes(BlockConf *conf)
 }
 
 bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
-   bool resizable, Error **errp)
+   bool resizable, bool zoned_support,
+   Error **errp)
 {
 BlockBackend *blk = conf->blk;
 BlockdevOnError rerror, werror;
@@ -98,9 +99,12 @@ bool blkconf_apply_backend_options(BlockConf *conf, bool 
readonly,
 if (!readonly) {
 perm |= BLK_PERM_WRITE;
 }
+if (zoned_support) {
+perm |= BLK_PERM_SUPPORT_ZONED;
+}
 
 shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
-  BLK_PERM_GRAPH_MOD;
+  BLK_PERM_GRAPH_MOD | BLK_PERM_SUPPORT_ZONED;
 if (resizable) {
 shared_perm |= BLK_PERM_RESIZE;
 }
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 77af9979de..85efc80992 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -474,7 +474,7 @@ static void fd_change_cb(void *opaque, bool load, Error 
**errp)
 } else {
 if (!blkconf_apply_backend_options(drive->conf,
blk_is_read_only(drive->blk), false,
-   errp)) {
+   false, errp)) {
 return;
 }
 }
@@ -561,7 +561,7 @@ static void floppy_drive_realize(DeviceState *qdev, Error 
**errp)
 
 if (!blkconf_apply_backend_options(>conf,
blk_is_read_only(dev->conf.blk),
-   false, errp)) {
+   false, false, errp)) {
 return;
 }
 
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 36d6a8bb3a..71b35bf4e7 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1333,7 +1333,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 }
 blkconf_blocksizes(>conf);
 if (!blkconf_apply_backend_options(>conf, blk_is_read_only(n->conf.blk),
-   false, errp)) {
+   false, false, errp)) {
 return;
 }
 
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index cbb3729158..8894bdbb0c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1123,7 +1123,7 @@ static void virtio_blk_device_realize(DeviceState *dev, 
Error **errp)
 
 if (!blkconf_apply_backend_options(>conf,
blk_is_read_only(conf->conf.blk), true,
-   errp)) {
+   false, errp)) {
 return;
 }
 s->original_wce = blk_enable_write_cache(conf->conf.blk);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 69d73196e2..8ed5e9d832 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -228,7 +228,7 @@ static void xen_block_realize(XenDevice *xendev, Error 
**errp)
 }
 
 if (!blkconf_apply_backend_options(conf, blockdev->info & VDISK_READONLY,
-   true, errp)) {
+   true, false, errp)) {
 return;
 }
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 9d8502785d..c0b4a445e4 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -197,7 +197,7 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
kind, Error **errp)
 }
 }
 if (!blkconf_apply_backend_options(>conf, kind == IDE_CD,
-   kind != IDE_CD, errp)) {
+   kind != IDE_CD, false, errp)) {
 return;
 }
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 8e95e3e38d..f20815b1d7 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2315,7 +2315,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice 
*dev)
 }
 }
 
-static void scsi_realize(SCSIDevice *dev, Error **errp)
+static void 

[Qemu-devel] [PATCH v3 0/4] virtio/block: handle zoned backing devices

[Dmitry Fomichev]

Currently, attaching zoned block devices (i.e., storage devices
compliant to ZAC/ZBC standards) using several virtio methods doesn't
work properly as zoned devices appear as regular block devices at the
guest. This may cause unexpected i/o errors and, potentially, some
data corruption.

To be more precise, attaching a zoned device via virtio-pci-blk,
virtio-scsi-pci/scsi-disk or virtio-scsi-pci/scsi-hd demonstrates the
above behavior. The virtio-scsi-pci/scsi-block method works with a
recent patch. The virtio-scsi-pci/scsi-generic method also appears to
handle zoned devices without problems.

This patch set adds code to check if the backing device that is being
opened is a zoned Host Managed device. If this is the case, the patch
prohibits attaching such device for all use cases lacking proper
zoned support.

Host Aware zoned block devices are designed to work as regular block
devices at a guest system that does not support ZBD. Therefore, this
patch set doesn't prohibit attachment of Host Aware devices.

Considering that there is still a couple of different working ways
to attach a ZBD, this patch set provides a reasonable short-term
solution for this problem. What about long term?

It appears to be beneficial to add proper ZBD support to virtio-blk.
In order to support this use case properly, some virtio-blk protocol
changes will be necessary. They are needed to allow the host code to
propagate some ZBD properties that are required for virtio guest
driver to configure the guest block device as ZBD, such as zoned
device model, zone size and the total number of zones. Further, some
support needs to be added for REPORT ZONES command as well as for zone
operations, such as OPEN ZONE, CLOSE ZONE, FINISH ZONE and RESET ZONE.

These additions to the protocol are relatively straightforward, but
they need to be approved by the virtio TC and the whole process may
take some time.

ZBD support for virtio-scsi-pci/scsi-disk and virtio-scsi-pci/scsi-hd
does not seem as necessary. Users will be expected to attach zoned
block devices via virtio-scsi-pci/scsi-block instead.

This patch set contains some Linux-specific code. This code is
necessary to obtain Zoned Block Device model value from Linux sysfs.

History:

v1 -> v2:
- rework the code to be permission-based
- always allow Host Aware devices to be attached
- add fix for Host Aware attachments aka RCAP output snoop

v2 -> v3:
- drop the patch for RCAP output snoop - merged separately


Dmitry Fomichev (4):
  block: Add zoned device model property
  raw: Recognize zoned backing devices
  block/ide/scsi: Set BLK_PERM_SUPPORT_ZONED
  raw: Don't open ZBDs if backend can't handle them

 block.c   | 19 +
 block/file-posix.c| 88 +--
 block/raw-format.c|  8 
 hw/block/block.c  |  8 +++-
 hw/block/fdc.c|  4 +-
 hw/block/nvme.c   |  2 +-
 hw/block/virtio-blk.c |  2 +-
 hw/block/xen-block.c  |  2 +-
 hw/ide/qdev.c |  2 +-
 hw/scsi/scsi-disk.c   | 13 +++---
 hw/scsi/scsi-generic.c|  2 +-
 hw/usb/dev-storage.c  |  2 +-
 include/block/block.h | 21 +-
 include/block/block_int.h |  4 ++
 include/hw/block/block.h  |  3 +-
 15 files changed, 150 insertions(+), 30 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v3 1/4] block: Add zoned device model property

[Dmitry Fomichev]

This commit adds Zoned Device Model (as defined in T10 ZBC and
T13 ZAC standards) as a block driver property, along with some
useful access functions.

A new backend driver permission, BLK_PERM_SUPPORT_ZONED, is also
introduced. Only the drivers having this permission will be allowed
to open zoned block devices.

No code is added yet to initialize or check the value of this new
property, therefore this commit doesn't change any functionality.

Signed-off-by: Dmitry Fomichev 
---
 block.c   | 19 +++
 include/block/block.h | 21 -
 include/block/block_int.h |  4 
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index cbd8da5f3b..c717c4d6f5 100644
--- a/block.c
+++ b/block.c
@@ -4667,6 +4667,25 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t 
*nb_sectors_ptr)
 *nb_sectors_ptr = nb_sectors < 0 ? 0 : nb_sectors;
 }
 
+uint8_t bdrv_get_zoned_model(BlockDriverState *bs)
+{
+if (bs->drv->bdrv_get_zoned_info) {
+bs->drv->bdrv_get_zoned_info(bs);
+}
+
+return bs->bl.zoned_model;
+}
+
+uint8_t bdrv_is_zoned(BlockDriverState *bs)
+{
+/*
+ * Host Aware zone devices are supposed to be able to work
+ * just like regular block devices. Thus, we only consider
+ * Host Managed devices to be zoned here.
+ */
+return bdrv_get_zoned_model(bs) == BLK_ZONED_MODEL_HM;
+}
+
 bool bdrv_is_sg(BlockDriverState *bs)
 {
 return bs->sg;
diff --git a/include/block/block.h b/include/block/block.h
index 50a07c1c33..bd98933f67 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -266,18 +266,35 @@ enum {
  */
 BLK_PERM_GRAPH_MOD  = 0x10,
 
+/** This permission is required to open zoned block devices. */
+BLK_PERM_SUPPORT_ZONED  = 0x20,
+
 BLK_PERM_ALL= 0x1f,
 
 DEFAULT_PERM_PASSTHROUGH= BLK_PERM_CONSISTENT_READ
  | BLK_PERM_WRITE
  | BLK_PERM_WRITE_UNCHANGED
- | BLK_PERM_RESIZE,
+ | BLK_PERM_RESIZE
+ | BLK_PERM_SUPPORT_ZONED,
 
 DEFAULT_PERM_UNCHANGED  = BLK_PERM_ALL & ~DEFAULT_PERM_PASSTHROUGH,
 };
 
 char *bdrv_perm_names(uint64_t perm);
 
+/*
+ * Known zoned device models.
+ *
+ * TODO For a Linux host, it could be preferrable to include
+ * /usr/include/linux/blkzoned.h instead of defining ZBD-specific
+ * values here.
+ */
+enum blk_zoned_model {
+BLK_ZONED_MODEL_NONE, /* Regular block device */
+BLK_ZONED_MODEL_HA,   /* Host-aware zoned block device */
+BLK_ZONED_MODEL_HM,   /* Host-managed zoned block device */
+};
+
 /* disk I/O throttling */
 void bdrv_init(void);
 void bdrv_init_with_whitelist(void);
@@ -354,6 +371,8 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
 BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
BlockDriverState *in_bs, Error **errp);
 void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
+uint8_t bdrv_get_zoned_model(BlockDriverState *bs);
+uint8_t bdrv_is_zoned(BlockDriverState *bs);
 void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
 int bdrv_commit(BlockDriverState *bs);
 int bdrv_change_backing_file(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3aa1e832a8..52c5758a9d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -416,6 +416,7 @@ struct BlockDriver {
 bool (*bdrv_debug_is_suspended)(BlockDriverState *bs, const char *tag);
 
 void (*bdrv_refresh_limits)(BlockDriverState *bs, Error **errp);
+void (*bdrv_get_zoned_info)(BlockDriverState *bs);
 
 /*
  * Returns 1 if newly created images are guaranteed to contain only
@@ -614,6 +615,9 @@ typedef struct BlockLimits {
 
 /* maximum number of iovec elements */
 int max_iov;
+
+/* Zoned device model. Zero value indicates a regular block device */
+uint8_t zoned_model;
 } BlockLimits;
 
 typedef struct BdrvOpBlocker BdrvOpBlocker;
-- 
2.21.0

[Qemu-devel] [PATCH v3 2/4] raw: Recognize zoned backing devices

[Dmitry Fomichev]

The purpose of this patch is to recognize a zoned block device (ZBD)
when it is opened as a raw file. The new code initializes the zoned
model propery introduced by the previous commit.

This commit is Linux-specific as it gets the Zoned Block Device Model
value (none/host-managed/host-aware) from sysfs on the host.

In order to avoid code duplication in file-posix.c, a common helper
function is added to read values of sysfs entries under
/sys/block//queue. This way, the existing function that reads
the value of "max_segments" entry and the the new function that reads
"zoned" value both share the same helper code.

Signed-off-by: Dmitry Fomichev 
---
 block/file-posix.c | 74 ++
 block/raw-format.c |  8 +
 2 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 4479cc7ab4..e307cab7a4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1053,15 +1053,13 @@ static int sg_get_max_transfer_length(int fd)
 #endif
 }
 
-static int sg_get_max_segments(int fd)
+static int hdev_read_blk_queue_entry(int fd, const char *key,
+char *buf, int buf_len)
 {
 #ifdef CONFIG_LINUX
-char buf[32];
-const char *end;
 char *sysfspath = NULL;
 int ret;
 int sysfd = -1;
-long max_segments;
 struct stat st;
 
 if (fstat(fd, )) {
@@ -1069,23 +1067,45 @@ static int sg_get_max_segments(int fd)
 goto out;
 }
 
-sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/max_segments",
-major(st.st_rdev), minor(st.st_rdev));
+sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/%s",
+major(st.st_rdev), minor(st.st_rdev), key);
 sysfd = open(sysfspath, O_RDONLY);
 if (sysfd == -1) {
 ret = -errno;
 goto out;
 }
 do {
-ret = read(sysfd, buf, sizeof(buf) - 1);
+ret = read(sysfd, buf, buf_len - 1);
 } while (ret == -1 && errno == EINTR);
 if (ret < 0) {
 ret = -errno;
-goto out;
 } else if (ret == 0) {
 ret = -EIO;
+}
+out:
+if (sysfd != -1) {
+close(sysfd);
+}
+g_free(sysfspath);
+return ret;
+#else
+return -ENOTSUP;
+#endif
+}
+
+static int sg_get_max_segments(int fd)
+{
+#ifdef CONFIG_LINUX
+char buf[32];
+const char *end;
+int ret;
+long max_segments;
+
+ret = hdev_read_blk_queue_entry(fd, "max_segments", buf, sizeof(buf));
+if (ret < 0) {
 goto out;
 }
+
 buf[ret] = 0;
 /* The file is ended with '\n', pass 'end' to accept that. */
 ret = qemu_strtol(buf, , 10, _segments);
@@ -1094,10 +1114,33 @@ static int sg_get_max_segments(int fd)
 }
 
 out:
-if (sysfd != -1) {
-close(sysfd);
+return ret;
+#else
+return -ENOTSUP;
+#endif
+}
+
+static int hdev_get_zoned_model(int fd)
+{
+#ifdef CONFIG_LINUX
+char buf[32];
+int ret;
+
+ret = hdev_read_blk_queue_entry(fd, "zoned", buf, sizeof(buf));
+if (ret < 0) {
+ret = BLK_ZONED_MODEL_NONE;
+goto out;
 }
-g_free(sysfspath);
+
+buf[ret - 1] = 0;
+ret = BLK_ZONED_MODEL_NONE;
+if (strcmp(buf, "host-managed") == 0) {
+ret = BLK_ZONED_MODEL_HM;
+} else if (strcmp(buf, "host-aware") == 0) {
+ret = BLK_ZONED_MODEL_HA;
+}
+
+out:
 return ret;
 #else
 return -ENOTSUP;
@@ -1107,9 +1150,10 @@ out:
 static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
 {
 BDRVRawState *s = bs->opaque;
+int ret;
 
 if (bs->sg) {
-int ret = sg_get_max_transfer_length(s->fd);
+ret = sg_get_max_transfer_length(s->fd);
 
 if (ret > 0 && ret <= BDRV_REQUEST_MAX_BYTES) {
 bs->bl.max_transfer = pow2floor(ret);
@@ -1119,6 +1163,12 @@ static void raw_refresh_limits(BlockDriverState *bs, 
Error **errp)
 if (ret > 0) {
 bs->bl.max_transfer = MIN(bs->bl.max_transfer, ret * 
getpagesize());
 }
+
+}
+
+ret = hdev_get_zoned_model(s->fd);
+if (ret >= 0) {
+bs->bl.zoned_model = ret;
 }
 
 raw_probe_alignment(bs, s->fd, errp);
diff --git a/block/raw-format.c b/block/raw-format.c
index bffd424dd0..12c2a3f95d 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -369,6 +369,13 @@ static void raw_refresh_limits(BlockDriverState *bs, Error 
**errp)
 }
 }
 
+static void raw_get_zoned_info(BlockDriverState *bs)
+{
+if (!bs->probed) {
+bs->bl.zoned_model = bs->file->bs->bl.zoned_model;
+}
+}
+
 static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
 PreallocMode prealloc, Error **errp)
 {
@@ -572,6 +579,7 @@ BlockDriver bdrv_raw = {
 .bdrv_co_ioctl= _co_ioctl,
 .create_opts  = _create_opts,
 .bdrv_has_zero_init   = _has_zero_init,
+.bdrv_get_zoned_info  = _get_zoned_info,
 .strong_runtime_opts  = raw_strong_runtime_opts,
  

[Qemu-devel] [PATCH v3 4/4] raw: Don't open ZBDs if backend can't handle them

[Dmitry Fomichev]

Abort opening a zoned device as a raw file in case the chosen
block backend driver lacks proper support for this type of
storage.

Signed-off-by: Dmitry Fomichev 
---
 block/file-posix.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index e307cab7a4..507dba98c5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2870,6 +2870,20 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t 
perm, uint64_t shared,
 goto fail;
 }
 }
+
+/*
+ * If we are opening a zoned block device, check if the backend
+ * driver can properly handle such devices, abort if not.
+ */
+if (bdrv_is_zoned(bs) &&
+(shared & BLK_PERM_SUPPORT_ZONED) &&
+!(perm & BLK_PERM_SUPPORT_ZONED)) {
+error_setg(errp,
+   "block backend driver doesn't support HM zoned devices");
+ret = -ENOTSUP;
+goto fail;
+}
+
 return 0;
 
 fail:
-- 
2.21.0

[Qemu-devel] [ANNOUNCE] QEMU 4.1.0-rc2 is now available

[Michael Roth]

Hello,

On behalf of the QEMU Team, I'd like to announce the availability of the
third release candidate for the QEMU 4.1 release.  This release is meant
for testing purposes and should not be used in a production environment.

  http://download.qemu-project.org/qemu-4.1.0-rc2.tar.xz
  http://download.qemu-project.org/qemu-4.1.0-rc2.tar.xz.sig

You can help improve the quality of the QEMU 4.1 release by testing this
release and reporting bugs on Launchpad:

  https://bugs.launchpad.net/qemu/

The release plan, as well a documented known issues for release
candidates, are available at:

  http://wiki.qemu.org/Planning/4.1

Please add entries to the ChangeLog for the 4.1 release below:

  http://wiki.qemu.org/ChangeLog/4.1

Thank you to everyone involved!

Changes since rc1:

061b51e919: Update version for v4.0.0-rc2 release (Peter Maydell)
b4682a63f8: filemon: fix watch IDs to avoid potential wraparound issues (Daniel 
P. Berrangé)
ff3dc8fefe: filemon: ensure watch IDs are unique to QFileMonitor scope (Daniel 
P. Berrangé)
b26c3f9cbd: tests: refactor file monitor test to make it more understandable 
(Daniel P. Berrangé)
79b9d4bde7: accel: Unbreak accelerator fallback (Markus Armbruster)
0427b6257e: vl: Document dependencies hiding in global and compat props (Markus 
Armbruster)
daff7f0bbe: migration: Support adding migration blockers earlier (Markus 
Armbruster)
811f865271: Revert "migration: move only_migratable to MigrationState" (Markus 
Armbruster)
2fa23277d5: Revert "vl: Fix to create migration object before block backends 
again" (Markus Armbruster)
5cc8f9eb7a: qapi/migration.json: Rename COLOStatus last_mode to last-mode 
(Zhang Chen)
966c0d4932: qapi/migration.json: Fix ColoStatus member last_mode's version 
(Zhang Chen)
17f30eae12: vl: Fix error location of positional arguments (Markus Armbruster)
f18957b854: tests/qemu-iotests/235: Allow fallback to tcg (Thomas Huth)
d20ba603f2: block: test block-stream with a base node that is used by 
block-commit (Alberto Garcia)
20509c4b8b: block: freeze the backing chain earlier in stream_start() (Alberto 
Garcia)
0f0998f621: block: continue until base is found in bdrv_freeze_backing_chain() 
et al (Alberto Garcia)
696aaaed57: block/file-posix: do not fail on unlock bytes (Vladimir 
Sementsov-Ogievskiy)
38e694fcc9: tests/qemu-iotests: Remove redundant COPYING file (Thomas Huth)
de23e72bb7: block/gluster: limit the transfer size to 512 MiB (Stefano 
Garzarella)
a3d6ae2299: qemu-img: Enable BDRV_REQ_MAY_UNMAP in convert (Nir Soffer)
e0a59749ef: iotests: Fix test 200 on s390x without virtio-pci (Thomas Huth)
be1092afa0: audio: fix audio timer rate conversion bug (Volker Rümelin)
b396733df3: usb-mtp: remove usb_mtp_object_free_one (Bandan Das)
4bc1591681: usb-mtp: fix return status of delete (Bandan Das)
75d34eb98c: nbd/client: Trace server noncompliance on structured reads (Eric 
Blake)
b0245d6478: nbd/server: Advertise actual minimum block size (Eric Blake)
4841211e0d: block: Add bdrv_get_request_alignment() (Eric Blake)
9cf638508c: nbd/client: Support qemu-img convert from unaligned size (Eric 
Blake)
3add3ab782: nbd/client: Reject inaccessible tail of inconsistent server (Eric 
Blake)
5189e30b14: hw/usb/bus.c: Handle "no speed matched" case in usb_mask_to_str() 
(Peter Maydell)
28605a22f5: Revert "audio: fix pc speaker init" (Gerd Hoffmann)
a62a85ef5c: nbd/client: Report offsets in bdrv_block_status (Eric Blake)
7da537f70d: nbd/client: Lower min_block for block-status, unaligned size (Eric 
Blake)
e9dce9cb6e: iotests: Add 241 to test NBD on unaligned images (Eric Blake)
737d3f5244: nbd-client: Work around server BLOCK_STATUS misalignment at EOF 
(Eric Blake)
30065d1424: qemu-img: Gracefully shutdown when map can't finish (Eric Blake)
ebd82cd872: nbd: Permit simple error to NBD_CMD_BLOCK_STATUS (Eric Blake)
b29f3a3d2a: nbd: Don't lose server's error to NBD_CMD_BLOCK_STATUS (Eric Blake)
a39286dd61: nbd: Tolerate some server non-compliance in NBD_CMD_BLOCK_STATUS 
(Eric Blake)
2058c2ad26: qemu-img: Report bdrv_block_status failures (Eric Blake)
ab79237a15: net: tap: use qemu_set_nonblock (Li Qiang)
c6bf50ff72: MAINTAINERS: Update the latest email address (Zhang Chen)
157628d067: e1000: Delay flush queue when receive RCTL (yuchenlin)
fdec16e3c2: net/socket: learn to talk with a unix dgram socket (Marc-André 
Lureau)
7d5489e6d1: exec: Only count mapped memory backends for qemu_getrampagesize() 
(David Gibson)
273fef83f6: spapr/irq: Add XIVE sanity checks on non-P9 machines (Cédric Le 
Goater)
0a794529bd: spapr: Simplify handling of host-serial and host-model values 
(David Gibson)
3e5365b7aa: target/ppc: Fix QEMU crash with stxsdx (Greg Kurz)
15d68c5e1d: target/ppc: Improve comment of bcctr used for spectre v2 mitigation 
(Greg Kurz)
d0db7caddb: target/ppc: Consolidate 64-bit server processor detection in a 
helper (Greg Kurz)
fa200c95f7: target/ppc: Enable "decrement and test CTR" version of bcctr (Greg 
Kurz)
9acc95cdd3: target/ppc: Fix TCG temporary leaks in 

Re: [Qemu-devel] [RFC,v1] Namespace Management Support

[Keith Busch]

On Tue, Jul 02, 2019 at 10:39:36AM -0700, Matt Fitzpatrick wrote:
> Adding namespace management support to the nvme device. Namespace creation
> requires contiguous block space for a simple method of allocation.

I guess that means this won't handle creating a large namespace
from fragmented unallocated space after various create+delete
scenarios. Capping the create size to the max contiguous extent may not
be so bad, but it may be confusing when UNVMCAP exceeds the largest
possible namespace you can create when we have no good way to report
the max possible creation size.

[Qemu-devel] [Bug 1776920] Re: qemu-img convert on Mac OSX creates corrupt images

[Sven R]

Hi,

I recently ran into problems and after a long time trying to find out the cause 
landed here, I got in trouble using a CentOs Cloud image:
https://cloud.centos.org/centos/7/images/CentOS-7-x86_64-GenericCloud-1905.qcow2.xz
which extracts to a .qcow2 image with sha256 of:
b376afdc0150601f15e53516327d9832216e01a300fecfc302066e36e2ac2b39

image: CentOS-7-x86_64-GenericCloud-1905.qcow2
file format: qcow2
virtual size: 8.0G (8589934592 bytes)
disk size: 898M
cluster_size: 65536
Format specific information:
compat: 0.10
refcount bits: 16

I use this command on a Mac, OS X 10.13.6 (17G7024), qemu installed via brew:
qemu-img convert -f qcow2 -O vmdk -o 
adapter_type=lsilogic,subformat=streamOptimized,compat6=on -p 
CentOS-7-x86_64-GenericCloud-1905.qcow2 -S 0 result.vmdk


941359104 21 Jul 17:11 CentOS-7-x86_64-GenericCloud-1905.qcow2 - original image

Converting this gives these results:
214551040 23 Jul 20:45 conv_mac_v3_1_mit_s_0.vmdk - doesnt work, made on Mac 
(APFS) with -S 0 
402303488 23 Jul 20:50 conv_mac_v3_1_auf_exfat.vmdk - works and is bootable, 
made on same Mac, on external drive, exFAT formatted.
149743104 23 Jul 21:21 conv_mac_v4_0.vmdk - doesnt work, made on Mac (APFS) 
without -S 0 
214551040 23 Jul 21:20 conv_mac_v4_0mit_S0.vmdk - doesnt work, made on Mac 
(APFS) with -S 0 

converting on non-Mac also works fine:
402303488 23 Jul 18:48 conv_centos7_v1-5-3.vmdk - works and is bootable, made 
on Centos, qemu-img version 1.5.3

So it seems that '-S 0' didn't fix it for me, or is that only in the
development branch?

Best Regards

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

Title:
  qemu-img convert on Mac OSX creates corrupt images

Status in QEMU:
  New

Bug description:
  An image created by qemu-img create, then modified by another program
  is converted to bad/corrupt image when using convert sub command on
  Mac OSX. The same convert works on Linux. The version of qemu-img is
  2.12.

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

Re: [Qemu-devel] [PATCH for 4.2 0/3] require newer glib2 to enable autofree'ing of stack variables exiting scope

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190723154856.17348-1-berra...@redhat.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==13077==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-coroutine /basic/no-dangling-access
==13077==WARNING: ASan is ignoring requested __asan_handle_no_return: stack 
top: 0x7ffd239e2000; bottom 0x7f320f2f8000; size: 0x00cb146ea000 (872221155328)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 2 test-coroutine /basic/lifecycle
---
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==13111==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 9 fdc-test /x86_64/fdc/media_insert
PASS 10 fdc-test /x86_64/fdc/read_no_dma_1
PASS 14 test-aio /aio/timer/schedule
==13106==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
PASS 17 test-aio /aio-gsource/bh/schedule
---
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
PASS 1 test-aio-multithread /aio/multi/lifecycle
==13120==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 test-aio-multithread /aio/multi/mutex/contended
---
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==13154==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
==13165==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
PASS 2 ide-test /x86_64/ide/flush
PASS 1 test-throttle /throttle/leak_bucket
---
PASS 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
PASS 8 test-throttle /throttle/accounting
==13172==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 9 test-throttle /throttle/groups
PASS 10 test-throttle /throttle/config/enabled
PASS 11 test-throttle /throttle/config/conflicting
---
PASS 13 test-throttle /throttle/config/ranges
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
==13175==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
==13184==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==13186==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
==13257==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-thread-pool 

Re: [Qemu-devel] [Qemu-block] [PATCH] util/async: hold AioContext ref to prevent use-after-free

[Stefan Hajnoczi]

On Tue, Jul 23, 2019 at 8:06 PM Stefan Hajnoczi  wrote:
> So if co causes ctx to be freed then we're in trouble.  Fix this problem
> by holding a reference to ctx.

For QEMU 4.2.  I'm not aware of a way to trigger this bug in QEMU
proper.  This fix just makes tests/test-bdrv-drain more reliable.

Stefan

[Qemu-devel] [PATCH] util/async: hold AioContext ref to prevent use-after-free

[Stefan Hajnoczi]

The tests/test-bdrv-drain /bdrv-drain/iothread/drain test case does the
following:

1. The preadv coroutine calls aio_bh_schedule_oneshot() and then yields.
2. The one-shot BH executes in another AioContext.  All it does is call
   aio_co_wakeup(preadv_co).
3. The preadv coroutine is re-entered and returns.

There is a race condition in aio_co_wake() where the preadv coroutine
returns and the test case destroys the preadv IOThread.  aio_co_wake()
can still be running in the other AioContext and it performs an access
to the freed IOThread AioContext.

Here is the race in aio_co_schedule():

  QSLIST_INSERT_HEAD_ATOMIC(>scheduled_coroutines,
co, co_scheduled_next);
  <-- race: co may execute before we invoke qemu_bh_schedule()!
  qemu_bh_schedule(ctx->co_schedule_bh);

So if co causes ctx to be freed then we're in trouble.  Fix this problem
by holding a reference to ctx.

Signed-off-by: Stefan Hajnoczi 
---
 util/async.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/util/async.c b/util/async.c
index 8d2105729c..4e4c7af51e 100644
--- a/util/async.c
+++ b/util/async.c
@@ -459,9 +459,17 @@ void aio_co_schedule(AioContext *ctx, Coroutine *co)
 abort();
 }
 
+/* The coroutine might run and release the last ctx reference before we
+ * invoke qemu_bh_schedule().  Take a reference to keep ctx alive until
+ * we're done.
+ */
+aio_context_ref(ctx);
+
 QSLIST_INSERT_HEAD_ATOMIC(>scheduled_coroutines,
   co, co_scheduled_next);
 qemu_bh_schedule(ctx->co_schedule_bh);
+
+aio_context_unref(ctx);
 }
 
 void aio_co_wake(struct Coroutine *co)
-- 
2.21.0

Re: [Qemu-devel] [PATCH 01/36] i2c: Move typedef of bitbang_i2c_interface to i2c.h

[Thomas Huth]

On 23/07/2019 20.57, BALATON Zoltan wrote:
> On Tue, 23 Jul 2019, Michael Roth wrote:
>> From: BALATON Zoltan 
>>
>> Clang 3.4 considers duplicate typedef in ppc4xx_i2c.h and
>> bitbang_i2c.h an error even if they are identical. Move it to a common
>> place to allow building with this clang version.
>>
>> Reported-by: Thomas Huth 
>> Signed-off-by: BALATON Zoltan 
>> Acked-by: David Gibson 
>> Reviewed-by: Philippe Mathieu-Daudé 
>> Signed-off-by: Thomas Huth 
>> (cherry picked from commit 2b4c1125ac3db2734222ff43c25388a16aca4a99)
> 
> This was superseded by d718b7475 later so you may want to look at that
> instead but for stable branch I'm not sure what are your preferences.

You could also cherry-pick this commit here instead:

e6e90feedb706b1b92827a5977b37e1e8defb8ef
"configure: Add -Wno-typedef-redefinition to CFLAGS (for Clang)"

... this should fix this problem for good.

 Thomas

Re: [Qemu-devel] [PATCH 01/36] i2c: Move typedef of bitbang_i2c_interface to i2c.h

[BALATON Zoltan]

On Tue, 23 Jul 2019, Michael Roth wrote:

From: BALATON Zoltan 

Clang 3.4 considers duplicate typedef in ppc4xx_i2c.h and
bitbang_i2c.h an error even if they are identical. Move it to a common
place to allow building with this clang version.

Reported-by: Thomas Huth 
Signed-off-by: BALATON Zoltan 
Acked-by: David Gibson 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
(cherry picked from commit 2b4c1125ac3db2734222ff43c25388a16aca4a99)


This was superseded by d718b7475 later so you may want to look at that 
instead but for stable branch I'm not sure what are your preferences.


Regards,
BALATON Zoltan


Signed-off-by: Michael Roth 
---
hw/i2c/bitbang_i2c.h| 2 --
include/hw/i2c/i2c.h| 2 ++
include/hw/i2c/ppc4xx_i2c.h | 3 ---
3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/i2c/bitbang_i2c.h b/hw/i2c/bitbang_i2c.h
index 3a7126d5de..9443021710 100644
--- a/hw/i2c/bitbang_i2c.h
+++ b/hw/i2c/bitbang_i2c.h
@@ -3,8 +3,6 @@

#include "hw/i2c/i2c.h"

-typedef struct bitbang_i2c_interface bitbang_i2c_interface;
-
#define BITBANG_I2C_SDA 0
#define BITBANG_I2C_SCL 1

diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 5dc166158b..cf4c45a98f 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -82,6 +82,8 @@ int i2c_recv(I2CBus *bus);

DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr);

+typedef struct bitbang_i2c_interface bitbang_i2c_interface;
+
/* lm832x.c */
void lm832x_key_event(DeviceState *dev, int key, int state);

diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
index 0891a9c948..b3450bacf7 100644
--- a/include/hw/i2c/ppc4xx_i2c.h
+++ b/include/hw/i2c/ppc4xx_i2c.h
@@ -31,9 +31,6 @@
#include "hw/sysbus.h"
#include "hw/i2c/i2c.h"

-/* from hw/i2c/bitbang_i2c.h */
-typedef struct bitbang_i2c_interface bitbang_i2c_interface;
-
#define TYPE_PPC4xx_I2C "ppc4xx-i2c"
#define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj), TYPE_PPC4xx_I2C)

Re: [Qemu-devel] [PATCH 00/36] Patch Round-up for stable 3.1.1, freeze on 2019-07-29

[Michael Roth]

Quoting Aleksandar Markovic (2019-07-23 12:12:27)
> On Tue, Jul 23, 2019 at 7:04 PM Michael Roth 
> wrote:
> 
> > Hi everyone,
> >
> >
> > The following new patches are queued for QEMU stable v3.1.1:
> >
> >   https://github.com/mdroth/qemu/commits/stable-3.1-staging
> >
> >
> Hello, Michael.
> 
> There is usually a breakdown by modified files within a cover letter.
> 
> Why is it not present here?

Sorry meant to include it before hitting send. I've just sent it as a reply.

> 
> Sincerely,
> Aleksandar
> 
> 
> 
> > The release is planned for 2019-08-01:
> >
> >   https://wiki.qemu.org/Planning/3.1
> >
> > Please respond here or CC qemu-sta...@nongnu.org on any patches you
> > think should be included in the release.
> >
> > Note that this update falls outside the normal stable release support
> > window (~1 development cycle), but is being released now since it was
> > delayed from its intended release date.
> >
> > Thanks!
> >
> >
> >
> >
> 

Re: [Qemu-devel] [Qemu-stable] [PATCH 00/36] Patch Round-up for stable 3.1.1, freeze on 2019-07-29

[Michael Roth]

Quoting Michael Roth (2019-07-23 12:00:28)
> Hi everyone,  
> 
> 
> The following new patches are queued for QEMU stable v3.1.1:
> 
>   https://github.com/mdroth/qemu/commits/stable-3.1-staging
> 
> The release is planned for 2019-08-01:
> 
>   https://wiki.qemu.org/Planning/3.1
> 
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.
> 
> Note that this update falls outside the normal stable release support
> window (~1 development cycle), but is being released now since it was
> delayed from its intended release date.
> 
> Thanks!
> 

Forgot to include the usual patch list:


BALATON Zoltan (1):
  i2c: Move typedef of bitbang_i2c_interface to i2c.h

Christian Borntraeger (2):
  iotests: make 235 work on s390 (and others)
  s390x/cpumodel: ignore csske for expansion

Christophe Fergeau (1):
  json: Fix % handling when not interpolating

Corey Minyard (1):
  pc:piix4: Update smbus I/O space after a migration

Cornelia Huck (1):
  vfio-ap: flag as compatible with balloon

Dan Streetman (1):
  do not call vhost_net_cleanup() on running net from char user event

Daniel Henrique Barboza (1):
  qga: update docs with systemd suspend support info

Daniel P. Berrangé (1):
  qemu-img: fix error reporting for -object

Eric Blake (1):
  cutils: Fix size_to_str() on 32-bit platforms

Gerd Hoffmann (1):
  usb-mtp: use O_NOFOLLOW and O_CLOEXEC.

Janosch Frank (1):
  s390x: Return specification exception for unimplemented diag 308 subcodes

Kevin Wolf (3):
  block: Fix invalidate_cache error path for parent activation
  qcow2: Avoid COW during metadata preallocation
  block: Fix AioContext switch for bs->drv == NULL

Li Hangjing (1):
  vhost: fix vhost_log size overflow during migration

Liam Merwick (1):
  tpm_tis: fix loop that cancels any seizure by a lower locality

Marcel Apfelbaum (1):
  hw/rdma: another clang compilation fix

Mark Cave-Ayland (2):
  mac_oldworld: use node name instead of alias name for hd device in 
FWPathProvider
  mac_newworld: use node name instead of alias name for hd device in 
FWPathProvider

Max Reitz (3):
  iotests: Filter second BLOCK_JOB_ERROR from 229
  block/file-posix: Unaligned O_DIRECT block-status
  iotests: Test unaligned raw images with O_DIRECT

Michael Roth (1):
  qga-win: include glib when building VSS DLL

Paolo Bonzini (1):
  i386: remove the 'INTEL_PT' CPUID bit from named CPU models

Paul A. Clarke (1):
  Changes requirement for "vsubsbs" instruction

Peter Lieven (1):
  megasas: fix mapped frame size

Peter Maydell (2):
  linux-user: make pwrite64/pread64(fd, NULL, 0, offset) return 0
  exec.c: Don't reallocate IOMMUNotifiers that are in use

Robert Hoo (1):
  i386: remove the new CPUID 'PCONFIG' from Icelake-Server CPU model

Stefan Berger (2):
  tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid
  tpm: Make sure the locality received from backend is valid

Thomas Huth (1):
  hw/s390x: Fix bad mask in time2tod()

Thomas Petazzoni (1):
  configure: improve usbfs check

William Bowling (1):
  slirp: check sscanf result when emulating ident

Zheng Xiang (1):
  pcie: set link state inactive/active after hot unplug/plug

 block.c| 13 +++--
 block/file-posix.c | 16 
 block/qcow2.c  |  3 ++-
 configure  | 19 +--
 exec.c | 10 ++
 hw/acpi/piix4.c|  1 +
 hw/i2c/bitbang_i2c.h   |  2 --
 hw/pci/pcie.c  | 12 
 hw/ppc/mac_newworld.c  |  4 ++--
 hw/ppc/mac_oldworld.c  |  4 ++--
 hw/rdma/rdma_rm_defs.h |  4 ++--
 hw/scsi/megasas.c  |  2 +-
 hw/tpm/tpm_tis.c   |  8 ++--
 hw/usb/dev-mtp.c   | 13 +
 hw/vfio/ap.c   |  8 
 hw/virtio/vhost.c  | 10 ++
 include/hw/i2c/i2c.h   |  2 ++
 include/hw/i2c/ppc4xx_i2c.h|  3 ---
 include/hw/s390x/tod.h |  2 +-
 linux-user/syscall.c   | 22 ++
 net/vhost-user.c   |  1 -
 qemu-img.c | 26 +-
 qga/qapi-schema.json   | 25 ++---
 qga/vss-win32/Makefile.objs|  2 +-
 qobject/json-parser.c  | 10 ++
 slirp/tcp_subr.c   | 10 +-
 target/i386/cpu.c  | 11 ---
 target/ppc/translate/vmx-ops.inc.c |  2 +-
 target/s390x/cpu_models.c  |  2 ++
 target/s390x/diag.c|  2 +-
 

Re: [Qemu-devel] [RFC PATCH] pci: Use PCI aliases when determining device IOMMU address space

[Michael S. Tsirkin]

On Tue, Jul 23, 2019 at 11:26:18AM -0600, Alex Williamson wrote:
> On Mon, 1 Apr 2019 13:41:39 +
> "Singh, Brijesh"  wrote:
> 
> > Thanks for adding Alex.
> > 
> > Adding Suravee.
> > 
> > 
> > On 3/29/19 11:49 AM, Alex Williamson wrote:
> > > [Cc +Brijesh]
> > > 
> > > Hi Brijesh, will the change below require the IVRS to be updated to
> > > include aliases for all BDF ranges behind a conventional bridge?  I
> > > think the Linux code handles this regardless of the firmware provided
> > > aliases, but is it required per spec for the ACPI tables to include
> > > bridge aliases?  Thanks,
> > >   
> > 
> > We do need to includes aliases in ACPI table. We need to populate the
> > IVHD type 0x43 and 0x4 for alias range start and end. I believe host
> > IVRS would contain similar information.
> > 
> > Suravee, please correct me if I am missing something?
> 
> I finally found some time to investigate this a little further, yes the
> types mentioned are correct for defining start and end of an alias
> range.  The challenge here is that these entries require a DeviceID,
> which is defined as a BDF, AIUI.  The IVRS is created in QEMU, but bus
> numbers are defined by the guest firmware, and potentially redefined by
> the guest OS.  This makes it non-trivial to insert a few IVHDs into the
> IVRS to describe alias ranges.  I'm wondering if the solution here is
> to define a new linker-loader command that would instruct the guest to
> write a bus number byte to a given offset for a described device.
> These commands would be inserted before the checksum command, such that
> these bus number updates are calculated as part of the checksum.
> 
> I'm imagining the command format would need to be able to distinguish
> between the actual bus number of a described device, the secondary bus
> number of the device, and the subordinate bus number of the device.
> For describing the device, I'm envisioning stealing from the DMAR
> definition, which already includes a bus number invariant mechanism to
> describe a device, starting with a segment and root bus, follow a chain
> of devfns to get to the target device.  Therefore the guest firmware
> would follow the path to the described device, pick the desired bus
> number, and write it to the indicated table offset.
> 
> Does this seem like a reasonable approach?  Better ideas?  I'm not
> thrilled with the increased scope demanded by IVRS support, but so long
> as we have an AMD IOMMU model, I don't see how to avoid it.  Thanks,
> 
> Alex

We generally just got the bus #s as programmed into virtual
bridges/devices and had qemu program them into acpi
tables.

If guest os changes bus#s it's responsible for updating acpi
tables :)


> > > On Tue, 26 Mar 2019 16:55:19 -0600
> > > Alex Williamson  wrote:
> > >   
> > >> Conventional PCI buses pre-date requester IDs.  An IOMMU cannot
> > >> distinguish by devfn & bus between devices in a conventional PCI
> > >> topology and therefore we cannot assign them separate AddressSpaces.
> > >> By taking this requester ID aliasing into account, QEMU better matches
> > >> the bare metal behavior and restrictions, and enables shared
> > >> AddressSpace configurations that are otherwise not possible with
> > >> guest IOMMU support.
> > >>
> > >> For the latter case, given any example where an IOMMU group on the
> > >> host includes multiple devices:
> > >>
> > >>$ ls  /sys/kernel/iommu_groups/1/devices/
> > >>:00:01.0  :01:00.0  :01:00.1
> > >>
> > >> If we incorporate a vIOMMU into the VM configuration, we're restricted
> > >> that we can only assign one of the endpoints to the guest because a
> > >> second endpoint will attempt to use a different AddressSpace.  VFIO
> > >> only supports IOMMU group level granularity at the container level,
> > >> preventing this second endpoint from being assigned:
> > >>
> > >> qemu-system-x86_64 -machine q35... \
> > >>-device intel-iommu,intremap=on \
> > >>-device pcie-root-port,addr=1e.0,id=pcie.1 \
> > >>-device vfio-pci,host=1:00.0,bus=pcie.1,addr=0.0,multifunction=on \
> > >>-device vfio-pci,host=1:00.1,bus=pcie.1,addr=0.1
> > >>
> > >> qemu-system-x86_64: -device vfio-pci,host=1:00.1,bus=pcie.1,addr=0.1: 
> > >> vfio \
> > >> :01:00.1: group 1 used in multiple address spaces
> > >>
> > >> However, when QEMU incorporates proper aliasing, we can make use of a
> > >> PCIe-to-PCI bridge to mask the requester ID, resulting in a hack that
> > >> provides the downstream devices with the same AddressSpace, ex:
> > >>
> > >> qemu-system-x86_64 -machine q35... \
> > >>-device intel-iommu,intremap=on \
> > >>-device pcie-pci-bridge,addr=1e.0,id=pci.1 \
> > >>-device vfio-pci,host=1:00.0,bus=pci.1,addr=1.0,multifunction=on \
> > >>-device vfio-pci,host=1:00.1,bus=pci.1,addr=1.1
> > >>
> > >> While the utility of this hack may be limited, this AddressSpace
> > >> aliasing is the correct behavior for QEMU to emulate bare metal.
> > >>
> > >> Signed-off-by: 

Re: [Qemu-devel] [PATCH-for-4.2 2/2] target/riscv/pmp: Convert qemu_log_mask(LOG_TRACE) to trace events

[Alistair Francis]

On Tue, Jul 23, 2019 at 5:08 AM Philippe Mathieu-Daudé
 wrote:
>
> Use the always-compiled trace events, remove the now unused
> RISCV_DEBUG_PMP definition.
>
> Note pmpaddr_csr_read() could previously do out-of-bound accesses
> passing addr_index >= MAX_RISCV_PMPS.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/pmp.c| 31 ++-
>  target/riscv/trace-events |  6 ++
>  2 files changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index d836288cb4..d4f1007109 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -27,14 +27,7 @@
>  #include "qemu/log.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
> -
> -#define RISCV_DEBUG_PMP 0
> -#define PMP_DEBUG(fmt, ...)  
>   \
> -do { 
>   \
> -if (RISCV_DEBUG_PMP) {   
>   \
> -qemu_log_mask(LOG_TRACE, "%s: " fmt "\n", __func__, 
> ##__VA_ARGS__);\
> -}
>   \
> -} while (0)
> +#include "trace.h"
>
>  static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
>  uint8_t val);
> @@ -302,8 +295,7 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t 
> reg_index,
>  int i;
>  uint8_t cfg_val;
>
> -PMP_DEBUG("hart " TARGET_FMT_ld ": reg%d, val: 0x" TARGET_FMT_lx,
> -env->mhartid, reg_index, val);
> +trace_pmpcfg_csr_write(env->mhartid, reg_index, val);
>
>  if ((reg_index & 1) && (sizeof(target_ulong) == 8)) {
>  qemu_log_mask(LOG_GUEST_ERROR,
> @@ -332,9 +324,7 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t 
> reg_index)
>  val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i);
>  cfg_val |= (val << (i * 8));
>  }
> -
> -PMP_DEBUG("hart " TARGET_FMT_ld ": reg%d, val: 0x" TARGET_FMT_lx,
> -env->mhartid, reg_index, cfg_val);
> +trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val);
>
>  return cfg_val;
>  }
> @@ -346,9 +336,7 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t 
> reg_index)
>  void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
>  target_ulong val)
>  {
> -PMP_DEBUG("hart " TARGET_FMT_ld ": addr%d, val: 0x" TARGET_FMT_lx,
> -env->mhartid, addr_index, val);
> -
> +trace_pmpaddr_csr_write(env->mhartid, addr_index, val);
>  if (addr_index < MAX_RISCV_PMPS) {
>  if (!pmp_is_locked(env, addr_index)) {
>  env->pmp_state.pmp[addr_index].addr_reg = val;
> @@ -369,14 +357,15 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t 
> addr_index,
>   */
>  target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index)
>  {
> -PMP_DEBUG("hart " TARGET_FMT_ld ": addr%d, val: 0x" TARGET_FMT_lx,
> -env->mhartid, addr_index,
> -env->pmp_state.pmp[addr_index].addr_reg);
> +target_ulong val = 0;
> +
>  if (addr_index < MAX_RISCV_PMPS) {
> -return env->pmp_state.pmp[addr_index].addr_reg;
> +val = env->pmp_state.pmp[addr_index].addr_reg;
> +trace_pmpaddr_csr_read(env->mhartid, addr_index, val);
>  } else {
>  qemu_log_mask(LOG_GUEST_ERROR,
>"ignoring pmpaddr read - out of bounds\n");
> -return 0;
>  }
> +
> +return val;
>  }
> diff --git a/target/riscv/trace-events b/target/riscv/trace-events
> index 48af0373df..4b6c652ae9 100644
> --- a/target/riscv/trace-events
> +++ b/target/riscv/trace-events
> @@ -1,2 +1,8 @@
>  # target/riscv/cpu_helper.c
>  riscv_trap(uint64_t hartid, bool async, uint64_t cause, uint64_t epc, 
> uint64_t tval, const char *desc) "hart:%"PRId64", async:%d, cause:%"PRId64", 
> epc:0x%"PRIx64", tval:0x%"PRIx64", desc=%s"
> +
> +# pmp.c
> +pmpcfg_csr_read(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart %" 
> PRIu64 ": read reg%" PRIu32", val: 0x%" PRIx64
> +pmpcfg_csr_write(uint64_t mhartid, uint32_t reg_index, uint64_t val) "hart 
> %" PRIu64 ": write reg%" PRIu32", val: 0x%" PRIx64
> +pmpaddr_csr_read(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart 
> %" PRIu64 ": read addr%" PRIu32", val: 0x%" PRIx64
> +pmpaddr_csr_write(uint64_t mhartid, uint32_t addr_index, uint64_t val) "hart 
> %" PRIu64 ": write addr%" PRIu32", val: 0x%" PRIx64
> --
> 2.20.1
>
>

Re: [Qemu-devel] [PATCH-for-4.2 1/2] target/riscv/pmp: Restrict priviledged PMP to system-mode emulation

[Alistair Francis]

On Tue, Jul 23, 2019 at 5:08 AM Philippe Mathieu-Daudé
 wrote:
>
> The RISC-V Physical Memory Protection is restricted to privileged
> modes. Restrict its compilation to QEMU system builds.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/Makefile.objs | 3 ++-
>  target/riscv/pmp.c | 4 
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs
> index b1c79bc1d1..b754e4bf32 100644
> --- a/target/riscv/Makefile.objs
> +++ b/target/riscv/Makefile.objs
> @@ -1,4 +1,5 @@
> -obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o 
> gdbstub.o pmp.o
> +obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o 
> gdbstub.o
> +obj-$(CONFIG_SOFTMMU) += pmp.o
>
>  DECODETREE = $(SRC_PATH)/scripts/decodetree.py
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 958c7502a0..d836288cb4 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -28,8 +28,6 @@
>  #include "qapi/error.h"
>  #include "cpu.h"
>
> -#ifndef CONFIG_USER_ONLY
> -
>  #define RISCV_DEBUG_PMP 0
>  #define PMP_DEBUG(fmt, ...)  
>   \
>  do { 
>   \
> @@ -382,5 +380,3 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, 
> uint32_t addr_index)
>  return 0;
>  }
>  }
> -
> -#endif
> --
> 2.20.1
>
>

Re: [Qemu-devel] [PATCH v4 0/3] restrict bridge interface name to IFNAMSIZ

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190723104754.29324-1-ppan...@redhat.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==7880==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==7880==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7fffae69e000; bottom 0x7fd8498f8000; size: 0x002764da6000 (169195757568)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==7900==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
PASS 10 fdc-test /x86_64/fdc/read_no_dma_1
==7912==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
PASS 1 test-aio-multithread /aio/multi/lifecycle
==7918==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
==7946==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 1 ide-test /x86_64/ide/identify
==7963==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
PASS 2 ide-test /x86_64/ide/flush
---
PASS 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
PASS 8 test-throttle /throttle/accounting
==7971==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 9 test-throttle /throttle/groups
PASS 10 test-throttle /throttle/config/enabled
PASS 11 test-throttle /throttle/config/conflicting
---
PASS 13 test-throttle /throttle/config/ranges
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
==7973==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==7984==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
==7981==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
PASS 4 ide-test /x86_64/ide/bmdma/trim
==8055==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt

Re: [Qemu-devel] [PATCH v2 0/2] pc: mmap kernel (ELF image) and initrd

[Montes, Julio]

Stefano, Brilliant job!

I can confirm that with these patches the memory footprint is smaller
and the boot time is the same for kata

Here the results using kata metrics

https://pasteboard.co/Ipl06Q0.png
https://pasteboard.co/Ipl3p4d.png

Thanks

-
Julio


On Tue, 2019-07-23 at 16:04 +0200, Stefano Garzarella wrote:
> In order to reduce the memory footprint when PVH kernel and initrd
> are used, we map them into memory instead of reading them.
> In this way we can share them between multiple instances of QEMU.
> 
> v2:
> - Patch 1: used g_mapped_file_new_from_fd() with 'writeble' set to
> 'true',
>since we can modify the mapped buffer. [Paolo, Peter]
> 
> These are the results using a PVH kernel and initrd (cpio):
> - memory footprint (using smem) [MB]
> QEMU  before   now
> # instancesUSS  PSSUSS  PSS
>  1   102.0M   105.8M 102.3M   106.2M
>  294.6M   101.2M  72.3M90.1M
>  494.1M98.0M  72.0M81.5M
>  894.0M96.2M  71.8M76.9M
> 1693.9M95.1M  71.6M74.3M
> 
> Initrd size: 3.0M
> Kernel
> image size: 28M
> sections size [size -A -d vmlinux]:  18.9M
> 
> - boot time [ms]
>   before   now
>  qemu_init_end:   63.85   55.91
>  linux_start_kernel:  82.11 (+18.26)  74.51 (+18.60)
>  linux_start_user:   169.94 (+87.83) 159.06 (+84.56)
> 
> QEMU command used:
> ./qemu-system-x86_64 -bios /path/to/seabios/out/bios.bin -no-hpet \
> -machine
> q35,accel=kvm,kernel_irqchip,nvdimm,sata=off,smbus=off,vmport=off \
> -cpu host -m 1G -smp 1 -vga none -display none -no-user-config
> -nodefaults \
> -kernel /path/to/vmlinux -initrd /path/to/rootfs.cpio \
> -append 'root=/dev/mem0 ro console=hvc0 pci=lastbus=0 nosmap'
> 
> Stefano Garzarella (2):
>   elf-ops.h: Map into memory the ELF to load
>   hw/i386/pc: Map into memory the initrd
> 
>  hw/i386/pc.c | 15 ---
>  include/hw/elf_ops.h | 64 
> 
>  2 files changed, 46 insertions(+), 33 deletions(-)
> 

Re: [Qemu-devel] [PATCH v4 3/3] tests/migration: Add a test for auto converge

[Dr. David Alan Gilbert]

* Yury Kotov (yury-ko...@yandex-team.ru) wrote:
> Signed-off-by: Yury Kotov 

OK, I think that's worth a go; lets see how it does in heavy CI systems/

Dave

> ---
>  tests/migration-test.c | 103 -
>  1 file changed, 92 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index a4feb9545d..b783ae47b3 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -241,6 +241,17 @@ static int64_t read_ram_property_int(QTestState *who, 
> const char *property)
>  return result;
>  }
>  
> +static int64_t read_migrate_property_int(QTestState *who, const char 
> *property)
> +{
> +QDict *rsp_return;
> +int64_t result;
> +
> +rsp_return = migrate_query(who);
> +result = qdict_get_try_int(rsp_return, property, 0);
> +qobject_unref(rsp_return);
> +return result;
> +}
> +
>  static uint64_t get_migration_pass(QTestState *who)
>  {
>  return read_ram_property_int(who, "dirty-sync-count");
> @@ -255,20 +266,22 @@ static void read_blocktime(QTestState *who)
>  qobject_unref(rsp_return);
>  }
>  
> +static bool check_migration_status(QTestState *who, const char *status)
> +{
> +bool completed;
> +char *current_status;
> +
> +current_status = migrate_query_status(who);
> +completed = strcmp(current_status, status) == 0;
> +g_assert_cmpstr(current_status, !=, "failed");
> +g_free(current_status);
> +return completed;
> +}
> +
>  static void wait_for_migration_status(QTestState *who,
>const char *goal)
>  {
> -while (true) {
> -bool completed;
> -char *status;
> -
> -status = migrate_query_status(who);
> -completed = strcmp(status, goal) == 0;
> -g_assert_cmpstr(status, !=,  "failed");
> -g_free(status);
> -if (completed) {
> -return;
> -}
> +while (!check_migration_status(who, goal)) {
>  usleep(1000);
>  }
>  }
> @@ -1121,6 +1134,73 @@ static void test_migrate_fd_proto(void)
>  test_migrate_end(from, to, true);
>  }
>  
> +static void test_migrate_auto_converge(void)
> +{
> +char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> +QTestState *from, *to;
> +int64_t remaining, percentage;
> +
> +/*
> + * We want the test to be fast enough, but stable.
> + * Throttle percentages are chosen to cover all cases (init, increment, 
> max)
> + */
> +static const int64_t expected_pcts[] = { 0, 1, 51, 98 };
> +const int64_t max_bandwidth = 2; /* ~200Mb/s */
> +const int64_t downtime_limit = 50; /* 50ms */
> +/*
> + * We migrate through unix-socket (> 500Mb/s).
> + * Thus, expected migration speed ~= bandwidth limit (< 500Mb/s).
> + * So, we can predict expected_threshold
> + */
> +const int64_t expected_threshold = max_bandwidth * downtime_limit / 1000;
> +
> +if (test_migrate_start(, , uri, false, false)) {
> +return;
> +}
> +
> +migrate_set_capability(from, "auto-converge", true);
> +migrate_set_parameter_int(from, "cpu-throttle-initial", 
> expected_pcts[1]);
> +migrate_set_parameter_int(from, "cpu-throttle-increment",
> +  expected_pcts[2] - expected_pcts[1]);
> +migrate_set_parameter_int(from, "max-cpu-throttle", expected_pcts[3]);
> +
> +migrate_set_parameter_int(from, "max-bandwidth", max_bandwidth);
> +migrate_set_parameter_int(from, "downtime-limit", downtime_limit);
> +
> +/* To check remaining size after precopy */
> +migrate_set_capability(from, "pause-before-switchover", true);
> +
> +/* Wait for the first serial output from the source */
> +wait_for_serial("src_serial");
> +
> +migrate(from, uri, "{}");
> +
> +/*
> + * Wait for pre-switchover status to check last throttle percentage
> + * and remaining. These values will be zeroed later
> + */
> +wait_for_migration_status(from, "pre-switchover");
> +
> +/* We expect that migration can't converge without throttling */
> +percentage = read_migrate_property_int(from, "cpu-throttle-percentage");
> +g_assert_cmpint(percentage, >, 0);
> +
> +remaining = read_ram_property_int(from, "remaining");
> +g_assert_cmpint(remaining, <, expected_threshold);
> +
> +wait_command(from, "{ 'execute': 'migrate-continue' , 'arguments':"
> +   "  { 'state': 'pre-switchover' } }");
> +
> +qtest_qmp_eventwait(to, "RESUME");
> +
> +wait_for_serial("dest_serial");
> +wait_for_migration_complete(from);
> +
> +g_free(uri);
> +
> +test_migrate_end(from, to, true);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  char template[] = "/tmp/migration-test-XX";
> @@ -1176,6 +1256,7 @@ int main(int argc, char **argv)
>  /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
>  qtest_add_func("/migration/xbzrle/unix", 

Re: [Qemu-devel] [PATCH v4 3/3] tests/migration: Add a test for auto converge

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> * Yury Kotov (yury-ko...@yandex-team.ru) wrote:
> > Signed-off-by: Yury Kotov 
> 
> OK, I think that's worth a go; lets see how it does in heavy CI systems/
> 
> Dave

and I meant:


Reviewed-by: Dr. David Alan Gilbert 

> > ---
> >  tests/migration-test.c | 103 -
> >  1 file changed, 92 insertions(+), 11 deletions(-)
> > 
> > diff --git a/tests/migration-test.c b/tests/migration-test.c
> > index a4feb9545d..b783ae47b3 100644
> > --- a/tests/migration-test.c
> > +++ b/tests/migration-test.c
> > @@ -241,6 +241,17 @@ static int64_t read_ram_property_int(QTestState *who, 
> > const char *property)
> >  return result;
> >  }
> >  
> > +static int64_t read_migrate_property_int(QTestState *who, const char 
> > *property)
> > +{
> > +QDict *rsp_return;
> > +int64_t result;
> > +
> > +rsp_return = migrate_query(who);
> > +result = qdict_get_try_int(rsp_return, property, 0);
> > +qobject_unref(rsp_return);
> > +return result;
> > +}
> > +
> >  static uint64_t get_migration_pass(QTestState *who)
> >  {
> >  return read_ram_property_int(who, "dirty-sync-count");
> > @@ -255,20 +266,22 @@ static void read_blocktime(QTestState *who)
> >  qobject_unref(rsp_return);
> >  }
> >  
> > +static bool check_migration_status(QTestState *who, const char *status)
> > +{
> > +bool completed;
> > +char *current_status;
> > +
> > +current_status = migrate_query_status(who);
> > +completed = strcmp(current_status, status) == 0;
> > +g_assert_cmpstr(current_status, !=, "failed");
> > +g_free(current_status);
> > +return completed;
> > +}
> > +
> >  static void wait_for_migration_status(QTestState *who,
> >const char *goal)
> >  {
> > -while (true) {
> > -bool completed;
> > -char *status;
> > -
> > -status = migrate_query_status(who);
> > -completed = strcmp(status, goal) == 0;
> > -g_assert_cmpstr(status, !=,  "failed");
> > -g_free(status);
> > -if (completed) {
> > -return;
> > -}
> > +while (!check_migration_status(who, goal)) {
> >  usleep(1000);
> >  }
> >  }
> > @@ -1121,6 +1134,73 @@ static void test_migrate_fd_proto(void)
> >  test_migrate_end(from, to, true);
> >  }
> >  
> > +static void test_migrate_auto_converge(void)
> > +{
> > +char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> > +QTestState *from, *to;
> > +int64_t remaining, percentage;
> > +
> > +/*
> > + * We want the test to be fast enough, but stable.
> > + * Throttle percentages are chosen to cover all cases (init, 
> > increment, max)
> > + */
> > +static const int64_t expected_pcts[] = { 0, 1, 51, 98 };
> > +const int64_t max_bandwidth = 2; /* ~200Mb/s */
> > +const int64_t downtime_limit = 50; /* 50ms */
> > +/*
> > + * We migrate through unix-socket (> 500Mb/s).
> > + * Thus, expected migration speed ~= bandwidth limit (< 500Mb/s).
> > + * So, we can predict expected_threshold
> > + */
> > +const int64_t expected_threshold = max_bandwidth * downtime_limit / 
> > 1000;
> > +
> > +if (test_migrate_start(, , uri, false, false)) {
> > +return;
> > +}
> > +
> > +migrate_set_capability(from, "auto-converge", true);
> > +migrate_set_parameter_int(from, "cpu-throttle-initial", 
> > expected_pcts[1]);
> > +migrate_set_parameter_int(from, "cpu-throttle-increment",
> > +  expected_pcts[2] - expected_pcts[1]);
> > +migrate_set_parameter_int(from, "max-cpu-throttle", expected_pcts[3]);
> > +
> > +migrate_set_parameter_int(from, "max-bandwidth", max_bandwidth);
> > +migrate_set_parameter_int(from, "downtime-limit", downtime_limit);
> > +
> > +/* To check remaining size after precopy */
> > +migrate_set_capability(from, "pause-before-switchover", true);
> > +
> > +/* Wait for the first serial output from the source */
> > +wait_for_serial("src_serial");
> > +
> > +migrate(from, uri, "{}");
> > +
> > +/*
> > + * Wait for pre-switchover status to check last throttle percentage
> > + * and remaining. These values will be zeroed later
> > + */
> > +wait_for_migration_status(from, "pre-switchover");
> > +
> > +/* We expect that migration can't converge without throttling */
> > +percentage = read_migrate_property_int(from, 
> > "cpu-throttle-percentage");
> > +g_assert_cmpint(percentage, >, 0);
> > +
> > +remaining = read_ram_property_int(from, "remaining");
> > +g_assert_cmpint(remaining, <, expected_threshold);
> > +
> > +wait_command(from, "{ 'execute': 'migrate-continue' , 'arguments':"
> > +   "  { 'state': 'pre-switchover' } }");
> > +
> > +qtest_qmp_eventwait(to, "RESUME");
> > +
> > +wait_for_serial("dest_serial");
> > +

[Qemu-devel] [Bug 1837651] [NEW] -netdev socket uses 100% cpu on Windows host

[Randy Rhoads]

Public bug reported:

On Windows hosts, any `-netdev socket` option (tcp listen, tcp connect,
udp passing a fd) causes qemu to use 100% cpu. The guest still runs, but
only sluggishly.

A simple testcase is:

> qemu-system-i386.exe -netdev socket,listen=:8000,id=n

And, in another command prompt:

> echo foo | nc.exe localhost 8000

Where nc.exe is netcat.

Tested on qemu 3.1 (from https://qemu.weilnetz.de/w64/) and 4.0 (self
compiled).

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  -netdev socket uses 100% cpu on Windows host

Status in QEMU:
  New

Bug description:
  On Windows hosts, any `-netdev socket` option (tcp listen, tcp
  connect, udp passing a fd) causes qemu to use 100% cpu. The guest
  still runs, but only sluggishly.

  A simple testcase is:

  > qemu-system-i386.exe -netdev socket,listen=:8000,id=n

  And, in another command prompt:

  > echo foo | nc.exe localhost 8000

  Where nc.exe is netcat.

  Tested on qemu 3.1 (from https://qemu.weilnetz.de/w64/) and 4.0 (self
  compiled).

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

Re: [Qemu-devel] [PULL v4 00/23 for 4.1-rc2] testing updates

[Peter Maydell]

On Tue, 23 Jul 2019 at 17:23, Alex Bennée  wrote:
>
> The following changes since commit 4da6c0f9ebbdaaf4315c71ccd288d02b9087f8af:
>
>   Merge remote-tracking branch 
> 'remotes/amarkovic2/tags/mips-queue-jul-23-2019' into staging (2019-07-23 
> 12:49:39 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-testing-230719-4
>
> for you to fetch changes up to 98808c3d0c162aba93fe7840a34b54c4814332d4:
>
>   tests/docker: Refresh APT cache before installing new packages on Debian 
> (2019-07-23 17:19:11 +0100)
>
> 
> Final testing updates:
>
>   - docker sphinx updates
>   - windows build re-enabled in CI
>   - travis_retry for make check
>   - build fixes
>   - docker cache fixes
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [RFC PATCH] pci: Use PCI aliases when determining device IOMMU address space

[Alex Williamson]

On Mon, 1 Apr 2019 13:41:39 +
"Singh, Brijesh"  wrote:

> Thanks for adding Alex.
> 
> Adding Suravee.
> 
> 
> On 3/29/19 11:49 AM, Alex Williamson wrote:
> > [Cc +Brijesh]
> > 
> > Hi Brijesh, will the change below require the IVRS to be updated to
> > include aliases for all BDF ranges behind a conventional bridge?  I
> > think the Linux code handles this regardless of the firmware provided
> > aliases, but is it required per spec for the ACPI tables to include
> > bridge aliases?  Thanks,
> >   
> 
> We do need to includes aliases in ACPI table. We need to populate the
> IVHD type 0x43 and 0x4 for alias range start and end. I believe host
> IVRS would contain similar information.
> 
> Suravee, please correct me if I am missing something?

I finally found some time to investigate this a little further, yes the
types mentioned are correct for defining start and end of an alias
range.  The challenge here is that these entries require a DeviceID,
which is defined as a BDF, AIUI.  The IVRS is created in QEMU, but bus
numbers are defined by the guest firmware, and potentially redefined by
the guest OS.  This makes it non-trivial to insert a few IVHDs into the
IVRS to describe alias ranges.  I'm wondering if the solution here is
to define a new linker-loader command that would instruct the guest to
write a bus number byte to a given offset for a described device.
These commands would be inserted before the checksum command, such that
these bus number updates are calculated as part of the checksum.

I'm imagining the command format would need to be able to distinguish
between the actual bus number of a described device, the secondary bus
number of the device, and the subordinate bus number of the device.
For describing the device, I'm envisioning stealing from the DMAR
definition, which already includes a bus number invariant mechanism to
describe a device, starting with a segment and root bus, follow a chain
of devfns to get to the target device.  Therefore the guest firmware
would follow the path to the described device, pick the desired bus
number, and write it to the indicated table offset.

Does this seem like a reasonable approach?  Better ideas?  I'm not
thrilled with the increased scope demanded by IVRS support, but so long
as we have an AMD IOMMU model, I don't see how to avoid it.  Thanks,

Alex

> > On Tue, 26 Mar 2019 16:55:19 -0600
> > Alex Williamson  wrote:
> >   
> >> Conventional PCI buses pre-date requester IDs.  An IOMMU cannot
> >> distinguish by devfn & bus between devices in a conventional PCI
> >> topology and therefore we cannot assign them separate AddressSpaces.
> >> By taking this requester ID aliasing into account, QEMU better matches
> >> the bare metal behavior and restrictions, and enables shared
> >> AddressSpace configurations that are otherwise not possible with
> >> guest IOMMU support.
> >>
> >> For the latter case, given any example where an IOMMU group on the
> >> host includes multiple devices:
> >>
> >>$ ls  /sys/kernel/iommu_groups/1/devices/
> >>:00:01.0  :01:00.0  :01:00.1
> >>
> >> If we incorporate a vIOMMU into the VM configuration, we're restricted
> >> that we can only assign one of the endpoints to the guest because a
> >> second endpoint will attempt to use a different AddressSpace.  VFIO
> >> only supports IOMMU group level granularity at the container level,
> >> preventing this second endpoint from being assigned:
> >>
> >> qemu-system-x86_64 -machine q35... \
> >>-device intel-iommu,intremap=on \
> >>-device pcie-root-port,addr=1e.0,id=pcie.1 \
> >>-device vfio-pci,host=1:00.0,bus=pcie.1,addr=0.0,multifunction=on \
> >>-device vfio-pci,host=1:00.1,bus=pcie.1,addr=0.1
> >>
> >> qemu-system-x86_64: -device vfio-pci,host=1:00.1,bus=pcie.1,addr=0.1: vfio 
> >> \
> >> :01:00.1: group 1 used in multiple address spaces
> >>
> >> However, when QEMU incorporates proper aliasing, we can make use of a
> >> PCIe-to-PCI bridge to mask the requester ID, resulting in a hack that
> >> provides the downstream devices with the same AddressSpace, ex:
> >>
> >> qemu-system-x86_64 -machine q35... \
> >>-device intel-iommu,intremap=on \
> >>-device pcie-pci-bridge,addr=1e.0,id=pci.1 \
> >>-device vfio-pci,host=1:00.0,bus=pci.1,addr=1.0,multifunction=on \
> >>-device vfio-pci,host=1:00.1,bus=pci.1,addr=1.1
> >>
> >> While the utility of this hack may be limited, this AddressSpace
> >> aliasing is the correct behavior for QEMU to emulate bare metal.
> >>
> >> Signed-off-by: Alex Williamson 
> >> ---
> >>   hw/pci/pci.c |   33 +++--
> >>   1 file changed, 31 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index 35451c1e9987..38467e676f1f 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> >> @@ -2594,12 +2594,41 @@ AddressSpace 
> >> *pci_device_iommu_address_space(PCIDevice *dev)
> >>   {
> >>   PCIBus *bus = pci_get_bus(dev);
> >>   PCIBus 

Re: [Qemu-devel] [PATCH] migration/postcopy: use mis->bh instead of allocating a QEMUBH

[Dr. David Alan Gilbert]

* Wei Yang (richardw.y...@linux.intel.com) wrote:
> For migration incoming side, it either quit in precopy or postcopy. It
> is save to use the mis->bh for both instead of allocating a dedicated

  'safe' not 'save'

> QEMUBH for postcopy.
> 
> Signed-off-by: Wei Yang 

Yes, I think that's OK; it's a little subtle.  Once postcopy is in 'run'
then the main incoming code will exit before running it's bh, so you're
right that it's free for us to use.



Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/savevm.c | 17 -
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 25fe7ea05a..0105068579 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1856,16 +1856,10 @@ static int 
> loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
>  return 0;
>  }
>  
> -
> -typedef struct {
> -QEMUBH *bh;
> -} HandleRunBhData;
> -
>  static void loadvm_postcopy_handle_run_bh(void *opaque)
>  {
>  Error *local_err = NULL;
> -HandleRunBhData *data = opaque;
> -MigrationIncomingState *mis = migration_incoming_get_current();
> +MigrationIncomingState *mis = opaque;
>  
>  /* TODO we should move all of this lot into postcopy_ram.c or a shared 
> code
>   * in migration.c
> @@ -1897,8 +1891,7 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
>  runstate_set(RUN_STATE_PAUSED);
>  }
>  
> -qemu_bh_delete(data->bh);
> -g_free(data);
> +qemu_bh_delete(mis->bh);
>  }
>  
>  /* After all discards we can start running and asking for pages */
> @@ -1906,7 +1899,6 @@ static int 
> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
>  {
>  PostcopyState old_ps = POSTCOPY_INCOMING_LISTENING;
>  PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING, 
> _ps);
> -HandleRunBhData *data;
>  
>  trace_loadvm_postcopy_handle_run();
>  if (ps != old_ps) {
> @@ -1914,9 +1906,8 @@ static int 
> loadvm_postcopy_handle_run(MigrationIncomingState *mis)
>  return -1;
>  }
>  
> -data = g_new(HandleRunBhData, 1);
> -data->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, data);
> -qemu_bh_schedule(data->bh);
> +mis->bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, mis);
> +qemu_bh_schedule(mis->bh);
>  
>  /* We need to finish reading the stream from the package
>   * and also stop reading anything more from the stream that loaded the
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] tests/docker: Refresh APT cache before installing new packages on Debian

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190723141528.18023-1-phi...@redhat.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==8214==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==8214==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffcc3b39000; bottom 0x7f5993af8000; size: 0x00a330041000 (700885241856)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==8229==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==8246==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
PASS 1 test-aio-multithread /aio/multi/lifecycle
==8254==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
==8282==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 1 ide-test /x86_64/ide/identify
==8298==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
PASS 2 ide-test /x86_64/ide/flush
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
==8307==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 13 test-throttle /throttle/config/ranges
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
==8305==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==8316==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==8384==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
PASS 5 test-thread-pool /thread-pool/cancel
==8390==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
==8396==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-thread-pool /thread-pool/cancel-async

[Qemu-devel] [PATCH 23/36] mac_newworld: use node name instead of alias name for hd device in FWPathProvider

[Michael Roth]

From: Mark Cave-Ayland 

When using -drive to configure the hd drive for the New World machine, the node
name "disk" should be used instead of the "hd" alias.

Signed-off-by: Mark Cave-Ayland 
Message-Id: <20190307212058.4890-3-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: David Gibson 
(cherry picked from commit 31bc6fa7fa8124ff8fb08373f9402985c806919f)
Signed-off-by: Michael Roth 
---
 hw/ppc/mac_newworld.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 14273a123e..cc7626ce6a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -553,11 +553,11 @@ static char *core99_fw_dev_path(FWPathProvider *p, 
BusState *bus,
 return g_strdup("cdrom");
 }
 
-return g_strdup("hd");
+return g_strdup("disk");
 }
 
 if (!strcmp(object_get_typename(OBJECT(dev)), "ide-hd")) {
-return g_strdup("hd");
+return g_strdup("disk");
 }
 
 if (!strcmp(object_get_typename(OBJECT(dev)), "ide-cd")) {
-- 
2.17.1

Re: [Qemu-devel] [PATCH 00/36] Patch Round-up for stable 3.1.1, freeze on 2019-07-29

[Aleksandar Markovic]

On Tue, Jul 23, 2019 at 7:04 PM Michael Roth 
wrote:

> Hi everyone,
>
>
> The following new patches are queued for QEMU stable v3.1.1:
>
>   https://github.com/mdroth/qemu/commits/stable-3.1-staging
>
>
Hello, Michael.

There is usually a breakdown by modified files within a cover letter.

Why is it not present here?

Sincerely,
Aleksandar



> The release is planned for 2019-08-01:
>
>   https://wiki.qemu.org/Planning/3.1
>
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.
>
> Note that this update falls outside the normal stable release support
> window (~1 development cycle), but is being released now since it was
> delayed from its intended release date.
>
> Thanks!
>
>
>
>

[Qemu-devel] [PATCH 31/36] s390x/cpumodel: ignore csske for expansion

[Michael Roth]

From: Christian Borntraeger 

csske will be removed in a future machine. Ignore it for expanding the
cpu model. Otherwise qemu falls back to z9.

Signed-off-by: Christian Borntraeger 
Cc: qemu-sta...@nongnu.org
Reviewed-by: David Hildenbrand 
Message-Id: <20190429090250.7648-3-borntrae...@de.ibm.com>
Signed-off-by: Cornelia Huck 
(cherry picked from commit eaf6f642abf1d4d24791b70728d4068428fc4658)
Signed-off-by: Michael Roth 
---
 target/s390x/cpu_models.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7c253ff308..d4464a2ea6 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -1297,6 +1297,8 @@ static void init_ignored_base_feat(void)
  S390_FEAT_KM_TDEA_192,
  S390_FEAT_KIMD_SHA_1,
  S390_FEAT_KLMD_SHA_1,
+ /* CSSKE is deprecated on newer generations */
+ S390_FEAT_CONDITIONAL_SSKE,
 };
 int i;
 
-- 
2.17.1

[Qemu-devel] high-level view of packet processing for virtio NIC?

[Chris Friesen]

Hi,

I'm looking for information on what the qemu architecture looks like for 
processing virtio network packets in a two-vCPU guest.


It looks like there's an IO thread doing a decent fraction of the work, 
separate from the vCPU threads--is that correct?  There's no disk 
involved in this case, purely network packet processing.


Chris

[Qemu-devel] [PATCH 33/36] iotests: Filter second BLOCK_JOB_ERROR from 229

[Michael Roth]

From: Max Reitz 

Without this filter, this test sometimes fails.

Signed-off-by: Max Reitz 
Reviewed-by: John Snow 
Signed-off-by: Kevin Wolf 
(cherry picked from commit fff2388d5d9caecca6200455d0ab6d5e13f4e9bd)
Signed-off-by: Michael Roth 
---
 tests/qemu-iotests/229 | 6 +-
 tests/qemu-iotests/229.out | 1 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229
index 86602437ff..8ce0cd2939 100755
--- a/tests/qemu-iotests/229
+++ b/tests/qemu-iotests/229
@@ -82,11 +82,15 @@ echo
 echo '=== Force cancel job paused in error state  ==='
 echo
 
+# Filter out BLOCK_JOB_ERROR events because they may or may not occur.
+# Cancelling the job means resuming it for a bit before it is actually
+# aborted, and in that time it may or may not re-encounter the error.
 success_or_failure="y" _send_qemu_cmd $QEMU_HANDLE \
 "{'execute': 'block-job-cancel',
  'arguments': { 'device': 'testdisk',
 'force': true}}" \
- "BLOCK_JOB_CANCELLED" "Assertion"
+ "BLOCK_JOB_CANCELLED" "Assertion" \
+| grep -v '"BLOCK_JOB_ERROR"'
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/229.out b/tests/qemu-iotests/229.out
index 4c4112805f..a3eb33788a 100644
--- a/tests/qemu-iotests/229.out
+++ b/tests/qemu-iotests/229.out
@@ -17,7 +17,6 @@ wrote 2097152/2097152 bytes at offset 0
 
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
 {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_ERROR", "data": {"device": "testdisk", "operation": "write", 
"action": "stop"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "testdisk"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "testdisk", "len": 2097152, "offset": 
1048576, "speed": 0, "type": "mirror"}}
 *** done
-- 
2.17.1

[Qemu-devel] [PATCH 05/36] pc:piix4: Update smbus I/O space after a migration

[Michael Roth]

From: Corey Minyard 

Otherwise it won't be set up correctly and won't work after
miigration.

Signed-off-by: Corey Minyard 
Cc: Igor Mammedov 
Cc: qemu-sta...@nongnu.org
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 2b4e573c7c7b9a698ba6931ba456bbd8d3d8c84c)
Signed-off-by: Michael Roth 
---
 hw/acpi/piix4.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index e330f24c71..2f4dd03b83 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -173,6 +173,7 @@ static int vmstate_acpi_post_load(void *opaque, int 
version_id)
 PIIX4PMState *s = opaque;
 
 pm_io_space_update(s);
+smbus_io_space_update(s);
 return 0;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 02/36] iotests: make 235 work on s390 (and others)

[Michael Roth]

From: Christian Borntraeger 

"-machine pc" will not work all architectures. Lets fall back to the
default machine by not specifying it.

In addition we also need to specify -no-shutdown on s390 as qemu will
exit otherwise.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Christian Borntraeger 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
(cherry picked from commit 2c26e648e4350079b0c86a6627b2d3566c3709c0)
Signed-off-by: Michael Roth 
---
 tests/qemu-iotests/235 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235
index da044ed34e..d6edd97ab4 100755
--- a/tests/qemu-iotests/235
+++ b/tests/qemu-iotests/235
@@ -49,7 +49,9 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 
'preallocation=metadata', disk,
 str(size))
 
 vm = QEMUMachine(iotests.qemu_prog)
-vm.add_args('-machine', 'pc,accel=kvm')
+vm.add_args('-machine', 'accel=kvm')
+if iotests.qemu_default_machine == 's390-ccw-virtio':
+vm.add_args('-no-shutdown')
 vm.add_args('-drive', 'id=src,file=' + disk)
 vm.launch()
 
-- 
2.17.1

[Qemu-devel] [PATCH 04/36] pcie: set link state inactive/active after hot unplug/plug

[Michael Roth]

From: Zheng Xiang 

When VM boots from the latest version of linux kernel, after
hot-unpluging virtio-blk disks which are hotplugged into
pcie-root-port, the VM's dmesg log shows:

[  151.046242] pciehp :00:05.0:pcie004: pending interrupts 0x0001 from Slot 
Status
[  151.046365] pciehp :00:05.0:pcie004: Slot(0-3): Attention button pressed
[  151.046369] pciehp :00:05.0:pcie004: Slot(0-3): Powering off due to 
button press
[  151.046420] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  151.046425] pciehp :00:05.0:pcie004: pciehp_green_led_blink: SLOTCTRL a8 
write cmd 200
[  151.046464] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  151.046468] pciehp :00:05.0:pcie004: pciehp_set_attention_status: 
SLOTCTRL a8 write cmd c0
[  156.163421] pciehp :00:05.0:pcie004: pciehp_get_power_status: SLOTCTRL 
a8 value read 2f1
[  156.163427] pciehp :00:05.0:pcie004: pciehp_unconfigure_device: 
domain:bus:dev = :06:00
[  156.198736] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  156.198772] pciehp :00:05.0:pcie004: pciehp_power_off_slot: SLOTCTRL a8 
write cmd 400
[  157.224124] pciehp :00:05.0:pcie004: pending interrupts 0x0018 from Slot 
Status
[  157.224194] pciehp :00:05.0:pcie004: pciehp_green_led_off: SLOTCTRL a8 
write cmd 300
[  157.224220] pciehp :00:05.0:pcie004: pciehp_check_link_active: 
lnk_status = 2011
[  157.224223] pciehp :00:05.0:pcie004: Slot(0-3): Link Up
[  157.224233] pciehp :00:05.0:pcie004: pciehp_get_power_status: SLOTCTRL 
a8 value read 7f1
[  157.224281] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  157.224285] pciehp :00:05.0:pcie004: pciehp_power_on_slot: SLOTCTRL a8 
write cmd 0
[  157.224300] pciehp :00:05.0:pcie004: __pciehp_link_set: lnk_ctrl = 0
[  157.224336] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  157.224339] pciehp :00:05.0:pcie004: pciehp_green_led_blink: SLOTCTRL a8 
write cmd 200
[  159.739294] pci :06:00.0 id reading try 50 times with interval 20 ms to 
get 
[  159.739315] pciehp :00:05.0:pcie004: pciehp_check_link_status: 
lnk_status = 2011
[  159.739318] pciehp :00:05.0:pcie004: Failed to check link status
[  159.739371] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  159.739394] pciehp :00:05.0:pcie004: pciehp_power_off_slot: SLOTCTRL a8 
write cmd 400
[  160.771426] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  160.771452] pciehp :00:05.0:pcie004: pciehp_green_led_off: SLOTCTRL a8 
write cmd 300
[  160.771495] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  160.771499] pciehp :00:05.0:pcie004: pciehp_set_attention_status: 
SLOTCTRL a8 write cmd 40
[  160.771535] pciehp :00:05.0:pcie004: pending interrupts 0x0010 from Slot 
Status
[  160.771539] pciehp :00:05.0:pcie004: pciehp_green_led_off: SLOTCTRL a8 
write cmd 300

After analyzing the log information, it seems that qemu doesn't
change the Link Status from active to inactive after hot-unplug.
This results in the abnormal log after the linux kernel commit
d331710ea78fea merged.

Furthermore, If I hotplug the same virtio-blk disk after hot-unplug,
the virtio-blk would turn on and then back off.

So this patch set the Link Status inactive after hot-unplug and
active after hot-plug.

Signed-off-by: Zheng Xiang 
Signed-off-by: Zheng Xiang 
Cc: Wang Haibin 
Cc: qemu-sta...@nongnu.org
Reviewed-by: Marcel Apfelbaum 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 2f2b18f60bf17453b4c01197a9316615a3c1f1de)
Signed-off-by: Michael Roth 
---
 hw/pci/pcie.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 6c91bd44a0..66b73b87c8 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -345,6 +345,10 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, 
DeviceState *dev,
 if (!dev->hotplugged) {
 pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
PCI_EXP_SLTSTA_PDS);
+if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
+pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
+   PCI_EXP_LNKSTA_DLLLA);
+}
 return;
 }
 
@@ -355,6 +359,10 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, 
DeviceState *dev,
 if (pci_get_function_0(pci_dev)) {
 pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
PCI_EXP_SLTSTA_PDS);
+if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
+pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
+   PCI_EXP_LNKSTA_DLLLA);
+}
 pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
 PCI_EXP_HP_EV_PDC | 

[Qemu-devel] [PATCH 07/36] linux-user: make pwrite64/pread64(fd, NULL, 0, offset) return 0

[Michael Roth]

From: Peter Maydell 

Linux returns success if pwrite64() or pread64() are called with a
zero length NULL buffer, but QEMU was returning -TARGET_EFAULT.

This is the same bug that we fixed in commit 58cfa6c2e6eb51b23cc9
for the write syscall, and long before that in 38d840e6790c29f59
for the read syscall.

Fixes: https://bugs.launchpad.net/qemu/+bug/1810433

Signed-off-by: Peter Maydell 
Reviewed-by: Laurent Vivier 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190108184900.9654-1-peter.mayd...@linaro.org>
Signed-off-by: Laurent Vivier 
(cherry picked from commit 2bd3f8998e1e7dcd9afc29fab252fb9936f9e956)
Signed-off-by: Michael Roth 
---
 linux-user/syscall.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 280137da8c..b13a170e52 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9677,8 +9677,15 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 arg4 = arg5;
 arg5 = arg6;
 }
-if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
-return -TARGET_EFAULT;
+if (arg2 == 0 && arg3 == 0) {
+/* Special-case NULL buffer and zero length, which should succeed 
*/
+p = 0;
+} else {
+p = lock_user(VERIFY_WRITE, arg2, arg3, 0);
+if (!p) {
+return -TARGET_EFAULT;
+}
+}
 ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
 unlock_user(p, arg2, ret);
 return ret;
@@ -9687,8 +9694,15 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 arg4 = arg5;
 arg5 = arg6;
 }
-if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
-return -TARGET_EFAULT;
+if (arg2 == 0 && arg3 == 0) {
+/* Special-case NULL buffer and zero length, which should succeed 
*/
+p = 0;
+} else {
+p = lock_user(VERIFY_READ, arg2, arg3, 1);
+if (!p) {
+return -TARGET_EFAULT;
+}
+}
 ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
 unlock_user(p, arg2, 0);
 return ret;
-- 
2.17.1

[Qemu-devel] [PATCH 06/36] hw/s390x: Fix bad mask in time2tod()

[Michael Roth]

From: Thomas Huth 

Since "s390x/tcg: avoid overflows in time2tod/tod2time", the
time2tod() function tries to deal with the 9 uppermost bits in the
time value, but uses the wrong mask for this: 0xff80 should
be used instead of 0xff10 here.

Fixes: 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd
Cc: qemu-sta...@nongnu.org
Signed-off-by: Thomas Huth 
Message-Id: <1544792887-14575-1-git-send-email-th...@redhat.com>
Reviewed-by: David Hildenbrand 
[CH: tweaked commit message]
Signed-off-by: Cornelia Huck 
(cherry picked from commit aba7a5a2de3dba5917024df25441f715b9249e31)
Signed-off-by: Michael Roth 
---
 include/hw/s390x/tod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
index 413c0d7c02..442f45b2f5 100644
--- a/include/hw/s390x/tod.h
+++ b/include/hw/s390x/tod.h
@@ -50,7 +50,7 @@ typedef struct S390TODClass {
 /* Converts ns to s390's clock format */
 static inline uint64_t time2tod(uint64_t ns)
 {
-return (ns << 9) / 125 + (((ns & 0xff10ull) / 125) << 9);
+return (ns << 9) / 125 + (((ns & 0xff80ull) / 125) << 9);
 }
 
 /* Converts s390's clock format to ns */
-- 
2.17.1

[Qemu-devel] [PATCH 03/36] Changes requirement for "vsubsbs" instruction

[Michael Roth]

From: "Paul A. Clarke" 

Changes requirement for "vsubsbs" instruction, which has been supported
since ISA 2.03. (Please see section 5.9.1.2 of ISA 2.03)

Reported-by: Paul A. Clarke 
Signed-off-by: Paul A. Clarke 
Signed-off-by: Leonardo Bras 
Signed-off-by: David Gibson 
(cherry picked from commit fcfbc18d00b10335310c9665edd6e04f2d152be8)
Signed-off-by: Michael Roth 
---
 target/ppc/translate/vmx-ops.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/translate/vmx-ops.inc.c 
b/target/ppc/translate/vmx-ops.inc.c
index 139f80cb24..84e05fb827 100644
--- a/target/ppc/translate/vmx-ops.inc.c
+++ b/target/ppc/translate/vmx-ops.inc.c
@@ -143,7 +143,7 @@ GEN_VXFORM(vaddsws, 0, 14),
 GEN_VXFORM_DUAL(vsububs, bcdadd, 0, 24, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM(vsubuws, 0, 26),
-GEN_VXFORM_DUAL(vsubsbs, bcdtrunc, 0, 28, PPC_NONE, PPC2_ISA300),
+GEN_VXFORM_DUAL(vsubsbs, bcdtrunc, 0, 28, PPC_ALTIVEC, PPC2_ISA300),
 GEN_VXFORM(vsubshs, 0, 29),
 GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE),
 GEN_VXFORM_207(vadduqm, 0, 4),
-- 
2.17.1

[Qemu-devel] [PATCH 28/36] cutils: Fix size_to_str() on 32-bit platforms

[Michael Roth]

From: Eric Blake 

When extracting a human-readable size formatter, we changed 'uint64_t
div' pre-patch to 'unsigned long div' post-patch. Which breaks on
32-bit platforms, resulting in 'inf' instead of intended values larger
than 999GB.

Fixes: 22951aaa
CC: qemu-sta...@nongnu.org
Reported-by: Max Reitz 
Signed-off-by: Eric Blake 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
(cherry picked from commit 754da86714d550c3f995f11a2587395081362e0a)
Signed-off-by: Michael Roth 
---
 util/cutils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/cutils.c b/util/cutils.c
index 698bd315bd..e8c726a141 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -754,7 +754,7 @@ const char *qemu_ether_ntoa(const MACAddr *mac)
 char *size_to_str(uint64_t val)
 {
 static const char *suffixes[] = { "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei" };
-unsigned long div;
+uint64_t div;
 int i;
 
 /*
-- 
2.17.1

[Qemu-devel] [PATCH 08/36] s390x: Return specification exception for unimplemented diag 308 subcodes

[Michael Roth]

From: Janosch Frank 

The architecture specifies specification exceptions for all
unavailable subcodes.

The presence of subcodes is indicated by checking some query subcode.
For example 6 will indicate that 3-6 are available. So future systems
might call new subcodes to check for new features. This should not
trigger a hw error, instead we return the architectured specification
exception.

Signed-off-by: Janosch Frank 
Cc: qemu-sta...@nongnu.org
Message-Id: <2019013657.66195-3-fran...@linux.ibm.com>
Reviewed-by: Christian Borntraeger 
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
(cherry picked from commit 37dbd1f4d4805edcd18d94eb202bb3461b3cd52d)
Signed-off-by: Michael Roth 
---
 target/s390x/diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index acb0f3d4af..aafa740f61 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -130,7 +130,7 @@ out:
 }
 return;
 default:
-hw_error("Unhandled diag308 subcode %" PRIx64, subcode);
+s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
 break;
 }
 }
-- 
2.17.1

[Qemu-devel] [PATCH 35/36] iotests: Test unaligned raw images with O_DIRECT

[Michael Roth]

From: Max Reitz 

We already have 221 for accesses through the page cache, but it is
better to create a new file for O_DIRECT instead of integrating those
test cases into 221.  This way, we can make use of
_supported_cache_modes (and _default_cache_mode) so the test is
automatically skipped on filesystems that do not support O_DIRECT.

As part of the split, add _supported_cache_modes to 221.  With that, it
no longer fails when run with -c none or -c directsync.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
(cherry picked from commit 2fab30c80b33cdc6157c7efe6207e54b6835cf92)
*remove context dependencies on iotests not in 3.1
Signed-off-by: Michael Roth 
---
 tests/qemu-iotests/221 |  4 ++
 tests/qemu-iotests/253 | 84 ++
 tests/qemu-iotests/253.out | 14 +++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 103 insertions(+)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

diff --git a/tests/qemu-iotests/221 b/tests/qemu-iotests/221
index 06f48f1f23..66d7546e35 100755
--- a/tests/qemu-iotests/221
+++ b/tests/qemu-iotests/221
@@ -1,6 +1,7 @@
 #!/bin/bash
 #
 # Test qemu-img vs. unaligned images
+# (See also 253, which is the O_DIRECT version)
 #
 # Copyright (C) 2018 Red Hat, Inc.
 #
@@ -37,6 +38,9 @@ _supported_fmt raw
 _supported_proto file
 _supported_os Linux
 
+_default_cache_mode writeback
+_supported_cache_modes writeback writethrough unsafe
+
 echo
 echo "=== Check mapping of unaligned raw image ==="
 echo
diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
new file mode 100755
index 00..d88d5afa45
--- /dev/null
+++ b/tests/qemu-iotests/253
@@ -0,0 +1,84 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img vs. unaligned images; O_DIRECT version
+# (Originates from 221)
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+_default_cache_mode none
+_supported_cache_modes none directsync
+
+echo
+echo "=== Check mapping of unaligned raw image ==="
+echo
+
+# We do not know how large a physical sector is, but it is certainly
+# going to be a factor of 1 MB
+size=$((1 * 1024 * 1024 - 1))
+
+# qemu-img create rounds size up to BDRV_SECTOR_SIZE
+_make_test_img $size
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# so we resize it and check again
+truncate --size=$size "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# qemu-io with O_DIRECT always writes whole physical sectors.  Again,
+# we do not know how large a physical sector is, so we just start
+# writing from a 64 kB boundary, which should always be aligned.
+offset=$((1 * 1024 * 1024 - 64 * 1024))
+$QEMU_IO -c "w $offset $((size - offset))" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# Resize it and check again -- contrary to 221, we may not get partial
+# sectors here, so there should be only two areas (one zero, one
+# data).
+truncate --size=$size "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+"driver=$IMGFMT,file.driver=file,file.filename=$TEST_IMG,cache.direct=on" \
+| _filter_qemu_img_map
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
new file mode 100644
index 00..607c0baa0b
--- /dev/null
+++ b/tests/qemu-iotests/253.out
@@ -0,0 +1,14 @@
+QA output created by 253
+
+=== Check mapping of unaligned raw image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048575
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET}]
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET}]
+wrote 65535/65535 bytes 

[Qemu-devel] [PATCH 36/36] vhost: fix vhost_log size overflow during migration

[Michael Roth]

From: Li Hangjing 

When a guest which doesn't support multiqueue is migrated with a multi queues
vhost-user-blk deivce, a crash will occur like:

0 qemu_memfd_alloc (name=, size=562949953421312, 
seals=, fd=0x7f87171fe8b4, errp=0x7f87171fe8a8) at 
util/memfd.c:153
1 0x7f883559d7cf in vhost_log_alloc (size=70368744177664, share=true) at 
hw/virtio/vhost.c:186
2 0x7f88355a0758 in vhost_log_get (listener=0x7f8838bd7940, enable=1) at 
qemu-2-12/hw/virtio/vhost.c:211
3 vhost_dev_log_resize (listener=0x7f8838bd7940, enable=1) at 
hw/virtio/vhost.c:263
4 vhost_migration_log (listener=0x7f8838bd7940, enable=1) at 
hw/virtio/vhost.c:787
5 0x7f88355463d6 in memory_global_dirty_log_start () at memory.c:2503
6 0x7f8835550577 in ram_init_bitmaps (f=0x7f88384ce600, 
opaque=0x7f8836024098) at migration/ram.c:2173
7 ram_init_all (f=0x7f88384ce600, opaque=0x7f8836024098) at migration/ram.c:2192
8 ram_save_setup (f=0x7f88384ce600, opaque=0x7f8836024098) at 
migration/ram.c:2219
9 0x7f88357a419d in qemu_savevm_state_setup (f=0x7f88384ce600) at 
migration/savevm.c:1002
10 0x7f883579fc3e in migration_thread (opaque=0x7f8837530400) at 
migration/migration.c:2382
11 0x7f8832447893 in start_thread () from /lib64/libpthread.so.0
12 0x7f8832178bfd in clone () from /lib64/libc.so.6

This is because vhost_get_log_size() returns a overflowed vhost-log size.
In this function, it uses the uninitialized variable vqs->used_phys and
vqs->used_size to get the vhost-log size.

Signed-off-by: Li Hangjing 
Reviewed-by: Xie Yongji 
Reviewed-by: Chai Wen 
Message-Id: <20190603061524.24076-1-lihangj...@baidu.com>
Cc: qemu-sta...@nongnu.org
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 240e647a14df9677b3a501f7b8b870e40aac3fd5)
Signed-off-by: Michael Roth 
---
 hw/virtio/vhost.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 569c4053ea..be38b39242 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -131,6 +131,11 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
 }
 for (i = 0; i < dev->nvqs; ++i) {
 struct vhost_virtqueue *vq = dev->vqs + i;
+
+if (!vq->used_phys && !vq->used_size) {
+continue;
+}
+
 vhost_dev_sync_region(dev, section, start_addr, end_addr, 
vq->used_phys,
   range_get_last(vq->used_phys, vq->used_size));
 }
@@ -168,6 +173,11 @@ static uint64_t vhost_get_log_size(struct vhost_dev *dev)
 }
 for (i = 0; i < dev->nvqs; ++i) {
 struct vhost_virtqueue *vq = dev->vqs + i;
+
+if (!vq->used_phys && !vq->used_size) {
+continue;
+}
+
 uint64_t last = vq->used_phys + vq->used_size - 1;
 log_size = MAX(log_size, last / VHOST_LOG_CHUNK + 1);
 }
-- 
2.17.1

[Qemu-devel] [PATCH 24/36] qga: update docs with systemd suspend support info

[Michael Roth]

From: Daniel Henrique Barboza 

Commit 067927d62e ("qga: systemd hibernate/suspend/hybrid-sleep
support") failed to update qapi-schema.json after adding systemd
hibernate/suspend/hybrid-sleep capabilities to guest-suspend-* QGA
commands.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Eric Blake 
Signed-off-by: Michael Roth 
(cherry picked from commit bb6c8d407e49d7b805ac52fe46abf4d8d5262046)
Signed-off-by: Michael Roth 
---
 qga/qapi-schema.json | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index c6725b3ec8..61f66fc461 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -532,12 +532,12 @@
 #
 # Suspend guest to disk.
 #
-# This command tries to execute the scripts provided by the pm-utils package.
-# If it's not available, the suspend operation will be performed by manually
-# writing to a sysfs file.
+# This command attempts to suspend the guest using three strategies, in this
+# order:
 #
-# For the best results it's strongly recommended to have the pm-utils
-# package installed in the guest.
+# - systemd hibernate
+# - pm-utils (via pm-hibernate)
+# - manual write into sysfs
 #
 # This command does NOT return a response on success. There is a high chance
 # the command succeeded if the VM exits with a zero exit status or, when
@@ -560,12 +560,12 @@
 #
 # Suspend guest to ram.
 #
-# This command tries to execute the scripts provided by the pm-utils package.
-# If it's not available, the suspend operation will be performed by manually
-# writing to a sysfs file.
+# This command attempts to suspend the guest using three strategies, in this
+# order:
 #
-# For the best results it's strongly recommended to have the pm-utils
-# package installed in the guest.
+# - systemd suspend
+# - pm-utils (via pm-suspend)
+# - manual write into sysfs
 #
 # IMPORTANT: guest-suspend-ram requires QEMU to support the 'system_wakeup'
 # command.  Thus, it's *required* to query QEMU for the presence of the
@@ -592,7 +592,10 @@
 #
 # Save guest state to disk and suspend to ram.
 #
-# This command requires the pm-utils package to be installed in the guest.
+# This command attempts to suspend the guest by executing, in this order:
+#
+# - systemd hybrid-sleep
+# - pm-utils (via pm-suspend-hybrid)
 #
 # IMPORTANT: guest-suspend-hybrid requires QEMU to support the 'system_wakeup'
 # command.  Thus, it's *required* to query QEMU for the presence of the
-- 
2.17.1

[Qemu-devel] [PATCH 22/36] mac_oldworld: use node name instead of alias name for hd device in FWPathProvider

[Michael Roth]

From: Mark Cave-Ayland 

When using -drive to configure the hd drive for the Old World machine, the node
name "disk" should be used instead of the "hd" alias.

Signed-off-by: Mark Cave-Ayland 
Message-Id: <20190307212058.4890-2-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: David Gibson 
(cherry picked from commit 484d366e02732c8de6f92e53e2ee9bb93dd4ca23)
Signed-off-by: Michael Roth 
---
 hw/ppc/mac_oldworld.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 9891c325a9..a8c8ec77bc 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -404,11 +404,11 @@ static char *heathrow_fw_dev_path(FWPathProvider *p, 
BusState *bus,
 return g_strdup("cdrom");
 }
 
-return g_strdup("hd");
+return g_strdup("disk");
 }
 
 if (!strcmp(object_get_typename(OBJECT(dev)), "ide-hd")) {
-return g_strdup("hd");
+return g_strdup("disk");
 }
 
 if (!strcmp(object_get_typename(OBJECT(dev)), "ide-cd")) {
-- 
2.17.1

[Qemu-devel] [PATCH 27/36] qcow2: Avoid COW during metadata preallocation

[Michael Roth]

From: Kevin Wolf 

Limiting the allocation to INT_MAX bytes isn't particularly clever
because it means that the final cluster will be a partial cluster which
will be completed through a COW operation. This results in unnecessary
data read and write requests which lead to an unwanted non-sparse
filesystem block for metadata preallocation.

Align the maximum allocation size down to the cluster size to avoid this
situation.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
(cherry picked from commit f29fbf7c6b1c9a84f6931c1c222716fbe073e6e4)
*modified to avoid functional dependency on 93e32b3e
Signed-off-by: Michael Roth 
---
 block/qcow2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 991d6ac91b..6ddaf9a1f8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2590,6 +2590,7 @@ static int qcow2_set_up_encryption(BlockDriverState *bs,
 static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset,
uint64_t new_length)
 {
+BDRVQcow2State *s = bs->opaque;
 uint64_t bytes;
 uint64_t host_offset = 0;
 unsigned int cur_bytes;
@@ -2600,7 +2601,7 @@ static int coroutine_fn preallocate_co(BlockDriverState 
*bs, uint64_t offset,
 bytes = new_length - offset;
 
 while (bytes) {
-cur_bytes = MIN(bytes, INT_MAX);
+cur_bytes = MIN(bytes, QEMU_ALIGN_DOWN(INT_MAX, s->cluster_size));
 ret = qcow2_alloc_cluster_offset(bs, offset, _bytes,
  _offset, );
 if (ret < 0) {
-- 
2.17.1

[Qemu-devel] [PATCH 25/36] usb-mtp: use O_NOFOLLOW and O_CLOEXEC.

[Michael Roth]

From: Gerd Hoffmann 

Open files and directories with O_NOFOLLOW to avoid symlinks attacks.
While being at it also add O_CLOEXEC.

usb-mtp only handles regular files and directories and ignores
everything else, so users should not see a difference.

Because qemu ignores symlinks, carrying out a successful symlink attack
requires swapping an existing file or directory below rootdir for a
symlink and winning the race against the inotify notification to qemu.

Fixes: CVE-2018-16872
Cc: Prasad J Pandit 
Cc: Bandan Das 
Reported-by: Michael Hanselmann 
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Michael Hanselmann 
Message-id: 20181213122511.13853-1-kra...@redhat.com
(cherry picked from commit bab9df35ce73d1c8e19a37e2737717ea1c984dc1)
Signed-off-by: Michael Roth 
---
 hw/usb/dev-mtp.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 100b7171f4..36c43b8c20 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -653,13 +653,18 @@ static void usb_mtp_object_readdir(MTPState *s, MTPObject 
*o)
 {
 struct dirent *entry;
 DIR *dir;
+int fd;
 
 if (o->have_children) {
 return;
 }
 o->have_children = true;
 
-dir = opendir(o->path);
+fd = open(o->path, O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW);
+if (fd < 0) {
+return;
+}
+dir = fdopendir(fd);
 if (!dir) {
 return;
 }
@@ -1007,7 +1012,7 @@ static MTPData *usb_mtp_get_object(MTPState *s, 
MTPControl *c,
 
 trace_usb_mtp_op_get_object(s->dev.addr, o->handle, o->path);
 
-d->fd = open(o->path, O_RDONLY);
+d->fd = open(o->path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
 if (d->fd == -1) {
 usb_mtp_data_free(d);
 return NULL;
@@ -1031,7 +1036,7 @@ static MTPData *usb_mtp_get_partial_object(MTPState *s, 
MTPControl *c,
 c->argv[1], c->argv[2]);
 
 d = usb_mtp_data_alloc(c);
-d->fd = open(o->path, O_RDONLY);
+d->fd = open(o->path, O_RDONLY | O_CLOEXEC | O_NOFOLLOW);
 if (d->fd == -1) {
 usb_mtp_data_free(d);
 return NULL;
@@ -1658,7 +1663,7 @@ static void usb_mtp_write_data(MTPState *s)
  0, 0, 0, 0);
 goto done;
 }
-d->fd = open(path, O_CREAT | O_WRONLY, mask);
+d->fd = open(path, O_CREAT | O_WRONLY | O_CLOEXEC | O_NOFOLLOW, mask);
 if (d->fd == -1) {
 usb_mtp_queue_result(s, RES_STORE_FULL, d->trans,
  0, 0, 0, 0);
-- 
2.17.1

[Qemu-devel] [PATCH 32/36] megasas: fix mapped frame size

[Michael Roth]

From: Peter Lieven 

the current value of 1024 bytes (16 * MFI_FRAME_SIZE) we map is not enough to 
hold
the maximum number of scatter gather elements we advertise. We actually need a
maximum of 2048 bytes. This is 128 max sg elements * 16 bytes (sizeof (union 
mfi_sgl)).

Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Message-Id: <20190404121015.28634-1...@kamp.de>
Reviewed-by: Hannes Reinecke 
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 2e56fbc87f6ec3cd56c37b01d313abd502b80d61)
Signed-off-by: Michael Roth 
---
 hw/scsi/megasas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index a56317e026..5ad762de23 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -477,7 +477,7 @@ static MegasasCmd *megasas_enqueue_frame(MegasasState *s,
 {
 PCIDevice *pcid = PCI_DEVICE(s);
 MegasasCmd *cmd = NULL;
-int frame_size = MFI_FRAME_SIZE * 16;
+int frame_size = MEGASAS_MAX_SGE * sizeof(union mfi_sgl);
 hwaddr frame_size_p = frame_size;
 unsigned long index;
 
-- 
2.17.1

[Qemu-devel] [PATCH 26/36] qemu-img: fix error reporting for -object

[Michael Roth]

From: Daniel P. Berrangé 

Error reporting for user_creatable_add_opts_foreach was changed so that
it no longer called 'error_report_err' in:

  commit 7e1e0c11127bde81cff260fc6859690435c509d6
  Author: Markus Armbruster 
  Date:   Wed Oct 17 10:26:43 2018 +0200

qom: Clean up error reporting in user_creatable_add_opts_foreach()

Some callers were updated to pass in "_fatal" but all the ones in
qemu-img were left passing NULL. As a result all errors went to
/dev/null instead of being reported to the user.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Markus Armbruster 
Reviewed-by: Stefano Garzarella 
Signed-off-by: Kevin Wolf 
(cherry picked from commit 334c43e2c342e878311c66b4e62343f0a7c2c6be)
Signed-off-by: Michael Roth 
---
 qemu-img.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ad04f59565..8b4a371ed5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -503,7 +503,7 @@ static int img_create(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 goto fail;
 }
 
@@ -753,7 +753,7 @@ static int img_check(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 return 1;
 }
 
@@ -966,7 +966,7 @@ static int img_commit(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 return 1;
 }
 
@@ -1349,7 +1349,7 @@ static int img_compare(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 ret = 2;
 goto out4;
 }
@@ -2159,7 +2159,7 @@ static int img_convert(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 goto fail_getopt;
 }
 
@@ -2713,7 +2713,7 @@ static int img_info(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 return 1;
 }
 
@@ -2932,7 +2932,7 @@ static int img_map(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 return 1;
 }
 
@@ -3081,7 +3081,7 @@ static int img_snapshot(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 return 1;
 }
 
@@ -3241,7 +3241,7 @@ static int img_rebase(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 return 1;
 }
 
@@ -3621,7 +3621,7 @@ static int img_resize(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 return 1;
 }
 
@@ -3865,7 +3865,7 @@ static int img_amend(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 ret = -1;
 goto out_no_progress;
 }
@@ -4509,7 +4509,7 @@ static int img_dd(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 ret = -1;
 goto out;
 }
@@ -4786,7 +4786,7 @@ static int img_measure(int argc, char **argv)
 
 if (qemu_opts_foreach(_object_opts,
   user_creatable_add_opts_foreach,
-  NULL, NULL)) {
+  NULL, _fatal)) {
 goto out;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 34/36] block/file-posix: Unaligned O_DIRECT block-status

[Michael Roth]

From: Max Reitz 

Currently, qemu crashes whenever someone queries the block status of an
unaligned image tail of an O_DIRECT image:
$ echo > foo
$ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
Offset  Length  Mapped to   File
qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'
failed.

This is because bdrv_co_block_status() checks that the result returned
by the driver's implementation is aligned to the request_alignment, but
file-posix can fail to do so, which is actually mentioned in a comment
there: "[...] possibly including a partial sector at EOF".

Fix this by rounding up those partial sectors.

There are two possible alternative fixes:
(1) We could refuse to open unaligned image files with O_DIRECT
altogether.  That sounds reasonable until you realize that qcow2
does necessarily not fill up its metadata clusters, and that nobody
runs qemu-img create with O_DIRECT.  Therefore, unpreallocated qcow2
files usually have an unaligned image tail.

(2) bdrv_co_block_status() could ignore unaligned tails.  It actually
throws away everything past the EOF already, so that sounds
reasonable.
Unfortunately, the block layer knows file lengths only with a
granularity of BDRV_SECTOR_SIZE, so bdrv_co_block_status() usually
would have to guess whether its file length information is inexact
or whether the driver is broken.

Fixing what raw_co_block_status() returns is the safest thing to do.

There seems to be no other block driver that sets request_alignment and
does not make sure that it always returns aligned values.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
(cherry picked from commit 9c3db310ff0b7473272ae8dce5e04e2f8a825390)
Signed-off-by: Michael Roth 
---
 block/file-posix.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 07bbdab953..df467f510b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2461,6 +2461,8 @@ static int coroutine_fn 
raw_co_block_status(BlockDriverState *bs,
 off_t data = 0, hole = 0;
 int ret;
 
+assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
+
 ret = fd_open(bs);
 if (ret < 0) {
 return ret;
@@ -2486,6 +2488,20 @@ static int coroutine_fn 
raw_co_block_status(BlockDriverState *bs,
 /* On a data extent, compute bytes to the end of the extent,
  * possibly including a partial sector at EOF. */
 *pnum = MIN(bytes, hole - offset);
+
+/*
+ * We are not allowed to return partial sectors, though, so
+ * round up if necessary.
+ */
+if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {
+int64_t file_length = raw_getlength(bs);
+if (file_length > 0) {
+/* Ignore errors, this is just a safeguard */
+assert(hole == file_length);
+}
+*pnum = ROUND_UP(*pnum, bs->bl.request_alignment);
+}
+
 ret = BDRV_BLOCK_DATA;
 } else {
 /* On a hole, compute bytes to the beginning of the next extent.  */
-- 
2.17.1

[Qemu-devel] [PATCH 19/36] json: Fix % handling when not interpolating

[Michael Roth]

From: Christophe Fergeau 

Commit 8bca4613 added support for %% in json strings when interpolating,
but in doing so broke handling of % when not interpolating.

When parse_string() is fed a string token containing '%', it skips the
'%' regardless of ctxt->ap, i.e. even it's not interpolating.  If the
'%' is the string's last character, it fails an assertion.  Else, it
"merely" swallows the '%'.

Fix parse_string() to handle '%' specially only when interpolating.

To gauge the bug's impact, let's review non-interpolating users of this
parser, i.e. code passing NULL context to json_message_parser_init():

* tests/check-qjson.c, tests/test-qobject-input-visitor.c,
  tests/test-visitor-serialization.c

  Plenty of tests, but we still failed to cover the buggy case.

* monitor.c: QMP input

* qga/main.c: QGA input

* qobject_from_json():

  - qobject-input-visitor.c: JSON command line option arguments of
-display and -blockdev

Reproducer: -blockdev '{"%"}'

  - block.c: JSON pseudo-filenames starting with "json:"

Reproducer: https://bugzilla.redhat.com/show_bug.cgi?id=1668244#c3

  - block/rbd.c: JSON key pairs

Pseudo-filenames starting with "rbd:".

Command line, QMP and QGA input are trusted.

Filenames are trusted when they come from command line, QMP or HMP.
They are untrusted when they come from from image file headers.
Example: QCOW2 backing file name.  Note that this is *not* the security
boundary between host and guest.  It's the boundary between host and an
image file from an untrusted source.

Neither failing an assertion nor skipping a character in a filename of
your choice looks exploitable.  Note that we don't support compiling
with NDEBUG.

Fixes: 8bca4613e6cddd948895b8db3def05950463495b
Cc: qemu-sta...@nongnu.org
Signed-off-by: Christophe Fergeau 
Message-Id: <20190102140535.11512-1-cferg...@redhat.com>
Reviewed-by: Eric Blake 
Tested-by: Richard W.M. Jones 
[Commit message extended to discuss impact]
Signed-off-by: Markus Armbruster 
(cherry picked from commit bbc0586ced6e9ffdfd29d89fcc917b3d90ac3938)
Signed-off-by: Michael Roth 
---
 qobject/json-parser.c | 10 ++
 tests/check-qjson.c   |  5 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 5a840dfd86..53e91cb16b 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -208,11 +208,13 @@ static QString *parse_string(JSONParserContext *ctxt, 
JSONToken *token)
 }
 break;
 case '%':
-if (ctxt->ap && ptr[1] != '%') {
-parse_error(ctxt, token, "can't interpolate into string");
-goto out;
+if (ctxt->ap) {
+if (ptr[1] != '%') {
+parse_error(ctxt, token, "can't interpolate into string");
+goto out;
+}
+ptr++;
 }
-ptr++;
 /* fall through */
 default:
 cp = mod_utf8_codepoint(ptr, 6, );
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index d876a7a96e..fa2afccb0a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -175,6 +175,11 @@ static void utf8_string(void)
 "\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
 "\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
 "\\u03BA\\u1F79\\u03C3\\u03BC\\u03B5",
+},
+/* '%' character when not interpolating */
+{
+"100%",
+"100%",
 },
 /* 2  Boundary condition test cases */
 /* 2.1  First possible sequence of a certain length */
-- 
2.17.1

[Qemu-devel] [PATCH 01/36] i2c: Move typedef of bitbang_i2c_interface to i2c.h

[Michael Roth]

From: BALATON Zoltan 

Clang 3.4 considers duplicate typedef in ppc4xx_i2c.h and
bitbang_i2c.h an error even if they are identical. Move it to a common
place to allow building with this clang version.

Reported-by: Thomas Huth 
Signed-off-by: BALATON Zoltan 
Acked-by: David Gibson 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
(cherry picked from commit 2b4c1125ac3db2734222ff43c25388a16aca4a99)
Signed-off-by: Michael Roth 
---
 hw/i2c/bitbang_i2c.h| 2 --
 include/hw/i2c/i2c.h| 2 ++
 include/hw/i2c/ppc4xx_i2c.h | 3 ---
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/i2c/bitbang_i2c.h b/hw/i2c/bitbang_i2c.h
index 3a7126d5de..9443021710 100644
--- a/hw/i2c/bitbang_i2c.h
+++ b/hw/i2c/bitbang_i2c.h
@@ -3,8 +3,6 @@
 
 #include "hw/i2c/i2c.h"
 
-typedef struct bitbang_i2c_interface bitbang_i2c_interface;
-
 #define BITBANG_I2C_SDA 0
 #define BITBANG_I2C_SCL 1
 
diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h
index 5dc166158b..cf4c45a98f 100644
--- a/include/hw/i2c/i2c.h
+++ b/include/hw/i2c/i2c.h
@@ -82,6 +82,8 @@ int i2c_recv(I2CBus *bus);
 
 DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr);
 
+typedef struct bitbang_i2c_interface bitbang_i2c_interface;
+
 /* lm832x.c */
 void lm832x_key_event(DeviceState *dev, int key, int state);
 
diff --git a/include/hw/i2c/ppc4xx_i2c.h b/include/hw/i2c/ppc4xx_i2c.h
index 0891a9c948..b3450bacf7 100644
--- a/include/hw/i2c/ppc4xx_i2c.h
+++ b/include/hw/i2c/ppc4xx_i2c.h
@@ -31,9 +31,6 @@
 #include "hw/sysbus.h"
 #include "hw/i2c/i2c.h"
 
-/* from hw/i2c/bitbang_i2c.h */
-typedef struct bitbang_i2c_interface bitbang_i2c_interface;
-
 #define TYPE_PPC4xx_I2C "ppc4xx-i2c"
 #define PPC4xx_I2C(obj) OBJECT_CHECK(PPC4xxI2CState, (obj), TYPE_PPC4xx_I2C)
 
-- 
2.17.1

[Qemu-devel] [PATCH 29/36] block: Fix AioContext switch for bs->drv == NULL

[Michael Roth]

From: Kevin Wolf 

Even for block nodes with bs->drv == NULL, we can't just ignore a
bdrv_set_aio_context() call. Leaving the node in its old context can
mean that it's still in an iothread context in bdrv_close_all() during
shutdown, resulting in an attempted unlock of the AioContext lock which
we don't hold.

This is an example stack trace of a related crash:

 #0  0x759da57f in raise () at /lib64/libc.so.6
 #1  0x759c4895 in abort () at /lib64/libc.so.6
 #2  0x55b97b1e in error_exit (err=, 
msg=msg@entry=0x55d386d0 <__func__.19059> "qemu_mutex_unlock_impl") at 
util/qemu-thread-posix.c:36
 #3  0x55b97f7f in qemu_mutex_unlock_impl 
(mutex=mutex@entry=0x568002f0, file=file@entry=0x55d378df 
"util/async.c", line=line@entry=507) at util/qemu-thread-posix.c:97
 #4  0x55b92f55 in aio_context_release (ctx=ctx@entry=0x56800290) 
at util/async.c:507
 #5  0x55b05cf8 in bdrv_prwv_co (child=child@entry=0x7fffc80012f0, 
offset=offset@entry=131072, qiov=qiov@entry=0x7fffd4f0, 
is_write=is_write@entry=true, flags=flags@entry=0)
 at block/io.c:833
 #6  0x55b060a9 in bdrv_pwritev (qiov=0x7fffd4f0, offset=131072, 
child=0x7fffc80012f0) at block/io.c:990
 #7  0x55b060a9 in bdrv_pwrite (child=0x7fffc80012f0, offset=131072, 
buf=, bytes=) at block/io.c:990
 #8  0x55ae172b in qcow2_cache_entry_flush (bs=bs@entry=0x56810680, 
c=c@entry=0x568cc740, i=i@entry=0) at block/qcow2-cache.c:51
 #9  0x55ae18dd in qcow2_cache_write (bs=bs@entry=0x56810680, 
c=0x568cc740) at block/qcow2-cache.c:248
 #10 0x55ae15de in qcow2_cache_flush (bs=0x56810680, c=) at block/qcow2-cache.c:259
 #11 0x55ae16b1 in qcow2_cache_flush_dependency (c=0x568a1700, 
c=0x568a1700, bs=0x56810680) at block/qcow2-cache.c:194
 #12 0x55ae16b1 in qcow2_cache_entry_flush (bs=bs@entry=0x56810680, 
c=c@entry=0x568a1700, i=i@entry=0) at block/qcow2-cache.c:194
 #13 0x55ae18dd in qcow2_cache_write (bs=bs@entry=0x56810680, 
c=0x568a1700) at block/qcow2-cache.c:248
 #14 0x55ae15de in qcow2_cache_flush (bs=bs@entry=0x56810680, 
c=) at block/qcow2-cache.c:259
 #15 0x55ad242c in qcow2_inactivate (bs=bs@entry=0x56810680) at 
block/qcow2.c:2124
 #16 0x55ad2590 in qcow2_close (bs=0x56810680) at block/qcow2.c:2153
 #17 0x55ab0c62 in bdrv_close (bs=0x56810680) at block.c:3358
 #18 0x55ab0c62 in bdrv_delete (bs=0x56810680) at block.c:3542
 #19 0x55ab0c62 in bdrv_unref (bs=0x56810680) at block.c:4598
 #20 0x55af4d72 in blk_remove_bs (blk=blk@entry=0x568103d0) at 
block/block-backend.c:785
 #21 0x55af4dbb in blk_remove_all_bs () at block/block-backend.c:483
 #22 0x55aae02f in bdrv_close_all () at block.c:3412
 #23 0x557f9796 in main (argc=, argv=, 
envp=) at vl.c:4776

The reproducer I used is a qcow2 image on gluster volume, where the
virtual disk size (4 GB) is larger than the gluster volume size (64M),
so we can easily trigger an ENOSPC. This backend is assigned to a
virtio-blk device using an iothread, and then from the guest a
'dd if=/dev/zero of=/dev/vda bs=1G count=1' causes the VM to stop
because of an I/O error. qemu_gluster_co_flush_to_disk() sets
bs->drv = NULL on error, so when virtio-blk stops the dataplane, the
block nodes stay in the iothread AioContext. A 'quit' monitor command
issued from this paused state crashes the process.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1631227
Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Stefano Garzarella 
(cherry picked from commit 1bffe1ae7a7b707c3a14ea2ccd00d3609d3ce4d8)
*drop context dependency on e64f25f30b8
Signed-off-by: Michael Roth 
---
 block.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 1ec4512d1e..3405a652a3 100644
--- a/block.c
+++ b/block.c
@@ -5056,10 +5056,6 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
 BdrvAioNotifier *baf, *baf_tmp;
 BdrvChild *child;
 
-if (!bs->drv) {
-return;
-}
-
 assert(!bs->walking_aio_notifiers);
 bs->walking_aio_notifiers = true;
 QLIST_FOREACH_SAFE(baf, >aio_notifiers, list, baf_tmp) {
@@ -5074,7 +5070,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
  */
 bs->walking_aio_notifiers = false;
 
-if (bs->drv->bdrv_detach_aio_context) {
+if (bs->drv && bs->drv->bdrv_detach_aio_context) {
 bs->drv->bdrv_detach_aio_context(bs);
 }
 QLIST_FOREACH(child, >children, next) {
@@ -5090,16 +5086,12 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
 BdrvAioNotifier *ban, *ban_tmp;
 BdrvChild *child;
 
-if (!bs->drv) {
-return;
-}
-
 bs->aio_context = new_context;
 
 QLIST_FOREACH(child, >children, next) {
 bdrv_attach_aio_context(child->bs, 

[Qemu-devel] [PATCH 20/36] qga-win: include glib when building VSS DLL

[Michael Roth]

Commit 3ebee3b191e defined assert() as g_assert(), but when we build
the VSS DLL component of QGA (to handle fsfreeze) we do not include
glib, which results in breakage when building with VSS support enabled.

Fix this by including glib (along with the -lintl and -lws2_32
dependencies it brings).

Since the VSS DLL is built statically, this introduces an additional
dependency on static glib and supporting libs for the mingw environment
(possibly why we didn't include glib originally), but VSS support
already has very specific prerequisites so it shouldn't affect too many
build environments.

Since the VSS DLL code does use qemu/osdep.h, this should also help
avoid future breakages and possibly allow for some clean ups in current
VSS code.

Suggested-by: Daniel P. Berrangé 
Cc: Daniel P. Berrangé 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael Roth 
(cherry picked from commit 82a58d270c6fbbe2f2381224946340fd3814a273)
Signed-off-by: Michael Roth 
---
 qga/vss-win32/Makefile.objs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/vss-win32/Makefile.objs b/qga/vss-win32/Makefile.objs
index 23d08da225..dad9d1b0ba 100644
--- a/qga/vss-win32/Makefile.objs
+++ b/qga/vss-win32/Makefile.objs
@@ -5,7 +5,7 @@ qga-vss-dll-obj-y += requester.o provider.o install.o
 obj-qga-vss-dll-obj-y = $(addprefix $(obj)/, $(qga-vss-dll-obj-y))
 $(obj-qga-vss-dll-obj-y): QEMU_CXXFLAGS = $(filter-out -Wstrict-prototypes 
-Wmissing-prototypes -Wnested-externs -Wold-style-declaration 
-Wold-style-definition -Wredundant-decls -fstack-protector-all 
-fstack-protector-strong, $(QEMU_CFLAGS)) -Wno-unknown-pragmas 
-Wno-delete-non-virtual-dtor
 
-$(obj)/qga-vss.dll: LDFLAGS = -shared 
-Wl,--add-stdcall-alias,--enable-stdcall-fixup -lole32 -loleaut32 -lshlwapi 
-luuid -static
+$(obj)/qga-vss.dll: LDFLAGS = -shared 
-Wl,--add-stdcall-alias,--enable-stdcall-fixup -lglib-2.0 -lole32 -loleaut32 
-lshlwapi -luuid -lintl -lws2_32 -static
 $(obj)/qga-vss.dll: $(obj-qga-vss-dll-obj-y) $(SRC_PATH)/$(obj)/qga-vss.def
$(call quiet-command,$(CXX) -o $@ $(qga-vss-dll-obj-y) 
$(SRC_PATH)/qga/vss-win32/qga-vss.def $(CXXFLAGS) 
$(LDFLAGS),"LINK","$(TARGET_DIR)$@")
 
-- 
2.17.1

[Qemu-devel] [PATCH 18/36] i386: remove the 'INTEL_PT' CPUID bit from named CPU models

[Michael Roth]

From: Paolo Bonzini 

Processor tracing is not yet implemented for KVM and it will be an
opt in feature requiring a special module parameter.
Disable it, because it is wrong to enable it by default and
it is impossible that no one has ever used it.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 4c257911dcc7c4189768e9651755c849ce9db4e8)
*drop context dependency on ecb85fe48
Signed-off-by: Michael Roth 
---
 target/i386/cpu.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 83152204b5..40814719bb 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2493,8 +2493,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_MPX | CPUID_7_0_EBX_CLWB |
 CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ |
 CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
-CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT |
-CPUID_7_0_EBX_INTEL_PT,
+CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT,
 .features[FEAT_7_0_ECX] =
 CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE |
 CPUID_7_0_ECX_AVX512VNNI,
@@ -2546,7 +2545,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
 CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
 CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
-CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_MPX | CPUID_7_0_EBX_INTEL_PT,
+CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_MPX,
 .features[FEAT_7_0_ECX] =
 CPUID_7_0_ECX_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU |
 CPUID_7_0_ECX_OSPKE | CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
@@ -2604,8 +2603,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_MPX | CPUID_7_0_EBX_CLWB |
 CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ |
 CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
-CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT |
-CPUID_7_0_EBX_INTEL_PT,
+CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT,
 .features[FEAT_7_0_ECX] =
 CPUID_7_0_ECX_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU |
 CPUID_7_0_ECX_OSPKE | CPUID_7_0_ECX_VBMI2 | CPUID_7_0_ECX_GFNI |
-- 
2.17.1

[Qemu-devel] [PATCH 30/36] do not call vhost_net_cleanup() on running net from char user event

[Michael Roth]

From: Dan Streetman 

Buglink: https://launchpad.net/bugs/1823458

Currently, a user CHR_EVENT_CLOSED event will cause net_vhost_user_event()
to call vhost_user_cleanup(), which calls vhost_net_cleanup() for all
its queues.  However, vhost_net_cleanup() must never be called like
this for fully-initialized nets; when other code later calls
vhost_net_stop() - such as from virtio_net_vhost_status() - it will try
to access the already-cleaned-up fields and fail with assertion errors
or segfaults.

The vhost_net_cleanup() will eventually be called from
qemu_cleanup_net_client().

Signed-off-by: Dan Streetman 
Message-Id: <20190416184624.15397-3-dan.street...@canonical.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 6ab79a20af3a7b3bf610ba9aebb446a9f0b05930)
Signed-off-by: Michael Roth 
---
 net/vhost-user.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index a39f9c9974..afb5697acf 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -223,7 +223,6 @@ static void chr_closed_bh(void *opaque)
 s = DO_UPCAST(NetVhostUserState, nc, ncs[0]);
 
 qmp_set_link(name, false, );
-vhost_user_stop(queues, ncs);
 
 qemu_chr_fe_set_handlers(>chr, NULL, NULL, net_vhost_user_event,
  NULL, opaque, NULL, true);
-- 
2.17.1

[Qemu-devel] [PATCH 00/36] Patch Round-up for stable 3.1.1, freeze on 2019-07-29

[Michael Roth]

Hi everyone,
  

The following new patches are queued for QEMU stable v3.1.1:

  https://github.com/mdroth/qemu/commits/stable-3.1-staging

The release is planned for 2019-08-01:

  https://wiki.qemu.org/Planning/3.1

Please respond here or CC qemu-sta...@nongnu.org on any patches you
think should be included in the release.

Note that this update falls outside the normal stable release support
window (~1 development cycle), but is being released now since it was
delayed from its intended release date.

Thanks!

[Qemu-devel] [PATCH 10/36] tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid

[Michael Roth]

From: Stefan Berger 

Make sure that the new locality passed to tpm_tis_prep_abort()
is valid.

Add a comment to aborting_locty that it may be any locality, including
TPM_TIS_NO_LOCALITY.

Signed-off-by: Stefan Berger 
Reviewed-by: Marc-André Lureau 
(cherry picked from commit e92b63ea610201bd743343fc6b11e6c39c8d3515)
Signed-off-by: Michael Roth 
---
 hw/tpm/tpm_tis.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d9322692ee..9a795ce96c 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -263,7 +263,9 @@ static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, 
uint8_t newlocty)
 {
 uint8_t busy_locty;
 
-s->aborting_locty = locty;
+assert(TPM_TIS_IS_VALID_LOCTY(newlocty));
+
+s->aborting_locty = locty; /* may also be TPM_TIS_NO_LOCALITY */
 s->next_locty = newlocty;  /* locality after successful abort */
 
 /*
-- 
2.17.1

[Qemu-devel] [PATCH 16/36] vfio-ap: flag as compatible with balloon

[Michael Roth]

From: Cornelia Huck 

vfio-ap devices do not pin any pages in the host. Therefore, they
are compatible with memory ballooning.

Flag them as compatible, so both vfio-ap and a balloon can be
used simultaneously.

Cc: qemu-sta...@nongnu.org
Acked-by: Christian Borntraeger 
Tested-by: Tony Krowiak 
Reviewed-by: Halil Pasic 
Signed-off-by: Cornelia Huck 
(cherry picked from commit 1883e8fc8005e9ef452890a075bae98e8c432968)
Signed-off-by: Michael Roth 
---
 hw/vfio/ap.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 65de952f44..0a25f5e096 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -104,6 +104,14 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
 vapdev->vdev.name = g_strdup_printf("%s", mdevid);
 vapdev->vdev.dev = dev;
 
+/*
+ * vfio-ap devices operate in a way compatible with
+ * memory ballooning, as no pages are pinned in the host.
+ * This needs to be set before vfio_get_device() for vfio common to
+ * handle the balloon inhibitor.
+ */
+vapdev->vdev.balloon_allowed = true;
+
 ret = vfio_get_device(vfio_group, mdevid, >vdev, _err);
 if (ret) {
 goto out_get_dev_err;
-- 
2.17.1

[Qemu-devel] [PATCH 13/36] hw/rdma: another clang compilation fix

[Michael Roth]

From: Marcel Apfelbaum 

Configuring QEMU with:
   configure --target-list="x86_64-softmmu" --cc=clang --enable-pvrdma
Results in:
   qemu/hw/rdma/rdma_rm_defs.h:108:3: error: redefinition of typedef 
'RdmaDeviceResources' is a C11 feature [-Werror,-Wtypedef-redefinition]
   } RdmaDeviceResources;
 ^
   qemu/hw/rdma/rdma_backend_defs.h:24:36: note: previous definition is here
   typedef struct RdmaDeviceResources RdmaDeviceResources;

Fix by removing one of the 'typedef' definitions.

Signed-off-by: Marcel Apfelbaum 
Message-Id: <20190214154053.15050-1-marcel.apfelb...@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Kamal Heib 
Signed-off-by: Marcel Apfelbaum 
(cherry picked from commit 59f911938fbaa6a5eff1146c8a4d74e1c55ecc2b)
*drop context dep. on c2dd117b385
Signed-off-by: Michael Roth 
---
 hw/rdma/rdma_rm_defs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/rdma/rdma_rm_defs.h b/hw/rdma/rdma_rm_defs.h
index 7228151239..17ee8f2871 100644
--- a/hw/rdma/rdma_rm_defs.h
+++ b/hw/rdma/rdma_rm_defs.h
@@ -85,7 +85,7 @@ typedef struct RdmaRmPort {
 enum ibv_port_state state;
 } RdmaRmPort;
 
-typedef struct RdmaDeviceResources {
+struct RdmaDeviceResources {
 RdmaRmPort ports[MAX_PORTS];
 RdmaRmResTbl pd_tbl;
 RdmaRmResTbl mr_tbl;
@@ -94,6 +94,6 @@ typedef struct RdmaDeviceResources {
 RdmaRmResTbl cq_tbl;
 RdmaRmResTbl cqe_ctx_tbl;
 GHashTable *qp_hash; /* Keeps mapping between real and emulated */
-} RdmaDeviceResources;
+};
 
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH 14/36] slirp: check sscanf result when emulating ident

[Michael Roth]

From: William Bowling 

When emulating ident in tcp_emu, if the strchr checks passed but the
sscanf check failed, two uninitialized variables would be copied and
sent in the reply, so move this code inside the if(sscanf()) clause.

Signed-off-by: William Bowling 
Cc: qemu-sta...@nongnu.org
Cc: secal...@redhat.com
Message-Id: <1551476756-25749-1-git-send-email-w...@wbowling.info>
Signed-off-by: Samuel Thibault 
Reviewed-by: Philippe Mathieu-Daudé 
(cherry picked from commit d3222975c7d6cda9e25809dea05241188457b113)
Signed-off-by: Michael Roth 
---
 slirp/tcp_subr.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index fa61349cbb..c27e0d158d 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -657,12 +657,12 @@ tcp_emu(struct socket *so, struct mbuf *m)
break;
}
}
+   so_rcv->sb_cc = 
snprintf(so_rcv->sb_data,
+
so_rcv->sb_datalen,
+"%d,%d\r\n", 
n1, n2);
+   so_rcv->sb_rptr = so_rcv->sb_data;
+   so_rcv->sb_wptr = so_rcv->sb_data + 
so_rcv->sb_cc;
}
-so_rcv->sb_cc = snprintf(so_rcv->sb_data,
- so_rcv->sb_datalen,
- "%d,%d\r\n", n1, n2);
-   so_rcv->sb_rptr = so_rcv->sb_data;
-   so_rcv->sb_wptr = so_rcv->sb_data + 
so_rcv->sb_cc;
}
m_free(m);
return 0;
-- 
2.17.1

[Qemu-devel] [PATCH 15/36] tpm_tis: fix loop that cancels any seizure by a lower locality

[Michael Roth]

From: Liam Merwick 

In tpm_tis_mmio_write() if the requesting locality is seizing
access, any seizure by a lower locality is cancelled.  However the
loop doing the seizure had an off-by-one error and the locality
immediately preceding the requesting locality was not being cleared.
This is fixed by adjusting the test in the for loop to check the
localities up to the requesting locality.

Signed-off-by: Liam Merwick 
Reviewed-by: Stefan Berger 
Signed-off-by: Stefan Berger 
(cherry picked from commit 37b55d67c0f001b20b7831db3f9f24f1d453e1de)
Signed-off-by: Michael Roth 
---
 hw/tpm/tpm_tis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 1554026788..fb08b483bc 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -620,7 +620,7 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
 }
 
 /* cancel any seize by a lower locality */
-for (l = 0; l < locty - 1; l++) {
+for (l = 0; l < locty; l++) {
 s->loc[l].access &= ~TPM_TIS_ACCESS_SEIZE;
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 17/36] i386: remove the new CPUID 'PCONFIG' from Icelake-Server CPU model

[Michael Roth]

From: Robert Hoo 

PCONFIG is not available to guests; it must be specifically enabled
using the PCONFIG_ENABLE execution control.  Disable it, because
no one can ever use it.

Signed-off-by: Robert Hoo 
Message-Id: <1545227081-213696-2-git-send-email-robert...@linux.intel.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 76e5a4d58357b9d077afccf7f7c82e17f733b722)
Signed-off-by: Michael Roth 
---
 target/i386/cpu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f81d35e1f9..83152204b5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2613,8 +2613,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG |
 CPUID_7_0_ECX_AVX512_VPOPCNTDQ | CPUID_7_0_ECX_LA57,
 .features[FEAT_7_0_EDX] =
-CPUID_7_0_EDX_PCONFIG | CPUID_7_0_EDX_SPEC_CTRL |
-CPUID_7_0_EDX_SPEC_CTRL_SSBD,
+CPUID_7_0_EDX_SPEC_CTRL | CPUID_7_0_EDX_SPEC_CTRL_SSBD,
 /* Missing: XSAVES (not supported by some Linux versions,
 * including v4.1 to v4.12).
 * KVM doesn't yet expose any XSAVES state save component,
-- 
2.17.1

[Qemu-devel] [PATCH 11/36] tpm: Make sure the locality received from backend is valid

[Michael Roth]

From: Stefan Berger 

Make sure that the locality passed from the backend to
tpm_tis_request_completed() is valid.

Signed-off-by: Stefan Berger 
Reviewed-by: Marc-André Lureau 
(cherry picked from commit a639f96111eadb3b8e3021fd3f27e2948ad1c640)
Signed-off-by: Michael Roth 
---
 hw/tpm/tpm_tis.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 9a795ce96c..1554026788 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -295,6 +295,8 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
 uint8_t locty = s->cmd.locty;
 uint8_t l;
 
+assert(TPM_TIS_IS_VALID_LOCTY(locty));
+
 if (s->cmd.selftest_done) {
 for (l = 0; l < TPM_TIS_NUM_LOCALITIES; l++) {
 s->loc[l].sts |= TPM_TIS_STS_SELFTEST_DONE;
-- 
2.17.1

[Qemu-devel] [PATCH 21/36] configure: improve usbfs check

[Michael Roth]

From: Thomas Petazzoni 

The current check to test if usbfs support should be compiled or not
solely relies on the presence of , without
actually checking that all definition used by Qemu are provided by
this header file.

With sufficiently old kernel headers,  may be
present, but some of the definitions needed by Qemu may not be
available.

This commit improves the check by building a small program that
actually tests whether the necessary definitions are available.

In addition, it fixes a bug where have_usbfs was set to "yes"
regardless of the result of the test.

Signed-off-by: Thomas Petazzoni 
Reviewed-by: Thomas Huth 
Message-Id: <20190213211827.20300-1-thomas.petazz...@bootlin.com>
Signed-off-by: Laurent Vivier 
(cherry picked from commit 96566d09aa105ee04cbc1c9539cf8a9a40e8e422)
Signed-off-by: Michael Roth 
---
 configure | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 0a3c6a72c3..85263e6b76 100755
--- a/configure
+++ b/configure
@@ -4203,10 +4203,25 @@ fi
 # check for usbfs
 have_usbfs=no
 if test "$linux_user" = "yes"; then
-  if check_include linux/usbdevice_fs.h; then
+  cat > $TMPC << EOF
+#include 
+
+#ifndef USBDEVFS_GET_CAPABILITIES
+#error "USBDEVFS_GET_CAPABILITIES undefined"
+#endif
+
+#ifndef USBDEVFS_DISCONNECT_CLAIM
+#error "USBDEVFS_DISCONNECT_CLAIM undefined"
+#endif
+
+int main(void)
+{
+return 0;
+}
+EOF
+  if compile_prog "" ""; then
 have_usbfs=yes
   fi
-  have_usbfs=yes
 fi
 
 # check for fallocate
-- 
2.17.1

[Qemu-devel] [PATCH 12/36] block: Fix invalidate_cache error path for parent activation

[Michael Roth]

From: Kevin Wolf 

bdrv_co_invalidate_cache() clears the BDRV_O_INACTIVE flag before
actually activating a node so that the correct permissions etc. are
taken. In case of errors, the flag must be restored so that the next
call to bdrv_co_invalidate_cache() retries activation.

Restoring the flag was missing in the error path for a failed
parent->role->activate() call. The consequence is that this attempt to
activate all images correctly fails because we still set errp, however
on the next attempt BDRV_O_INACTIVE is already clear, so we return
success without actually retrying the failed action.

An example where this is observable in practice is migration to a QEMU
instance that has a raw format block node attached to a guest device
with share-rw=off (the default) while another process holds
BLK_PERM_WRITE for the same image. In this case, all activation steps
before parent->role->activate() succeed because raw can tolerate other
writers to the image. Only the parent callback (in particular
blk_root_activate()) tries to implement the share-rw=on property and
requests exclusive write permissions. This fails when the migration
completes and correctly displays an error. However, a manual 'cont' will
incorrectly resume the VM without calling blk_root_activate() again.

This case is described in more detail in the following bug report:
https://bugzilla.redhat.com/show_bug.cgi?id=1531888

Fix this by correctly restoring the BDRV_O_INACTIVE flag in the error
path.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
Tested-by: Markus Armbruster 
Reviewed-by: Stefan Hajnoczi 
(cherry picked from commit 78fc3b3a26c145eebcdee992988644974b243a74)
Signed-off-by: Michael Roth 
---
 block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block.c b/block.c
index 811239ca23..1ec4512d1e 100644
--- a/block.c
+++ b/block.c
@@ -4553,6 +4553,7 @@ static void coroutine_fn 
bdrv_co_invalidate_cache(BlockDriverState *bs,
 if (parent->role->activate) {
 parent->role->activate(parent, _err);
 if (local_err) {
+bs->open_flags |= BDRV_O_INACTIVE;
 error_propagate(errp, local_err);
 return;
 }
-- 
2.17.1

[Qemu-devel] [PATCH 09/36] exec.c: Don't reallocate IOMMUNotifiers that are in use

[Michael Roth]

From: Peter Maydell 

The tcg_register_iommu_notifier() code has a GArray of
TCGIOMMUNotifier structs which it has registered by passing
memory_region_register_iommu_notifier() a pointer to the embedded
IOMMUNotifier field. Unfortunately, if we need to enlarge the
array via g_array_set_size() this can cause a realloc(), which
invalidates the pointer that memory_region_register_iommu_notifier()
put into the MemoryRegion's iommu_notify list. This can result
in segfaults.

Switch the GArray to holding pointers to the TCGIOMMUNotifier
structs, so that we can individually allocate and free them.

Cc: qemu-sta...@nongnu.org
Fixes: 1f871c5e6b0f30644a60a ("exec.c: Handle IOMMUs in 
address_space_translate_for_iotlb()")
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20190128174241.5860-1-peter.mayd...@linaro.org
(cherry picked from commit 5601be3b01d73e21c09331599e2ce62df016ff94)
Signed-off-by: Michael Roth 
---
 exec.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index bb6170dbff..df5571eb3d 100644
--- a/exec.c
+++ b/exec.c
@@ -664,7 +664,7 @@ static void tcg_register_iommu_notifier(CPUState *cpu,
 int i;
 
 for (i = 0; i < cpu->iommu_notifiers->len; i++) {
-notifier = _array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
+notifier = g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i);
 if (notifier->mr == mr && notifier->iommu_idx == iommu_idx) {
 break;
 }
@@ -672,7 +672,8 @@ static void tcg_register_iommu_notifier(CPUState *cpu,
 if (i == cpu->iommu_notifiers->len) {
 /* Not found, add a new entry at the end of the array */
 cpu->iommu_notifiers = g_array_set_size(cpu->iommu_notifiers, i + 1);
-notifier = _array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
+notifier = g_new0(TCGIOMMUNotifier, 1);
+g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i) = notifier;
 
 notifier->mr = mr;
 notifier->iommu_idx = iommu_idx;
@@ -704,8 +705,9 @@ static void tcg_iommu_free_notifier_list(CPUState *cpu)
 TCGIOMMUNotifier *notifier;
 
 for (i = 0; i < cpu->iommu_notifiers->len; i++) {
-notifier = _array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
+notifier = g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier *, i);
 memory_region_unregister_iommu_notifier(notifier->mr, >n);
+g_free(notifier);
 }
 g_array_free(cpu->iommu_notifiers, true);
 }
@@ -975,7 +977,7 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
 vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
 }
 
-cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier));
+cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier 
*));
 #endif
 }
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH v4 00/18] bitmaps: introduce 'bitmap' sync mode

[John Snow]

On 7/23/19 5:47 AM, Fabian Grünbichler wrote:
> On Mon, Jul 22, 2019 at 01:21:02PM -0400, John Snow wrote:
>>
>>
>> On 7/22/19 8:17 AM, Fabian Grünbichler wrote:
>>> On Tue, Jul 09, 2019 at 07:25:32PM -0400, John Snow wrote:
 This series adds a new "BITMAP" sync mode that is meant to replace the
 existing "INCREMENTAL" sync mode.

 This mode can have its behavior modified by issuing any of three bitmap 
 sync
 modes, passed as arguments to the job.

 The three bitmap sync modes are:
 - ON-SUCCESS: This is an alias for the old incremental mode. The bitmap is
   conditionally synchronized based on the return code of the 
 job
   upon completion.
 - NEVER: This is, effectively, the differential backup mode. It never 
 clears
  the bitmap, as the name suggests.
 - ALWAYS: Here is the new, exciting thing. The bitmap is always 
 synchronized,
   even on failure. On success, this is identical to incremental, 
 but
   on failure it clears only the bits that were copied successfully.
   This can be used to "resume" incremental backups from later 
 points
   in times.

 I wrote this series by accident on my way to implement incremental mode
 for mirror, but this happened first -- the problem is that Mirror mode
 uses its existing modes in a very particular way; and this was the best
 way to add bitmap support into the mirror job properly.

 [...]

 Future work:
 [..]
  - Add these modes to Mirror. (Done*, but needs tests.)
>>>
>>> are these mirror patches available somehwere for testing in combination
>>> with this series? your bitmaps branch does not seem to contain them ;)
>>>
>>> we've been experimenting with Ma Haocong's patch (v4 from February) to add
>>> "incremental"/differential sync to drive-mirror recently with positive
>>> results so far, and this sounds like it is another attempt at getting
>>> this properly integrated into Qemu.
>>>
>>
>> Not available quite yet; I added it in fairly hastily but haven't done
>> the testing I want to do yet, so I wouldn't feel comfortable sharing it
>> before I do my own due diligence on it. Give me a chance to polish it so
>> that the testing effort isn't wasted :)
> 
> fair enough, and no hurries :)
> 
>>
>> Can you share some of your use-cases for how you are using the
>> "incremental mirror" so far? It might be useful for the patch
>> justification if I can point to production use cases. (And good for
>> allocating time, too.)
> 
> it's basically the same use case that the original "incremental mirror"
> patch (series)[1] from two years ago had (no affiliation with the author
> though) - we have a guest disk replication feature for ZFS/zvols in a
> clustered hypervisor setting, and would like to re-use the already
> replicated disk state when live-migrating a VM. Qemu does not know
> anything about the replication, since it happens on the storage layer
> with zfs send/zfs receive. note that for VMs, we use zvols which are
> block devices backed by ZFS (or rather, ZFS datasets exposed as block
> devices), minus the file system part of regular ZFS datasets. from
> Qemu's PoV these (replicated) disks are just regular block devices (and not
> image-backed disks on a filesystem, or accessed via some special
> BlockDriver like Ceph's RBD images).
> 
> we currently support live migration
> 1) with disks on shared/distributed storage (easy enough)
> 2) with regular (non-replicated, local) disks (via nbd/drive-mirror)
> 3) with unused disks on the storage level (disks are not known to Qemu/the VM)
> 
> 1-3 can be mixed and matched arbitrarily in one guest, e.g. with one
> disk on a shared Ceph cluster, one disk that is not in use on an NFS
> share, and another disk on a local LVM-thin pool. 2) and 3) also allow
> switching the underlying storage on the fly, since they transfer the
> full disk (content) anyway.
> 
> we also support offline migration with shared, local, unused and/or
> replicated disks (all on the storage level with no involvement of Qemu).
> 
> as you can see there is a gap in the live-migration feature matrix: when
> replication is used, you either have to poweroff the VM to re-use the
> replication state (storage-only migration), or drop the replication
> state and do a full local-disk live-migration before re-creating the
> replication state from scratch (which is bad, since replication can have
> multiple target hosts, and re-establishing the whole disk can take a
> while if its big).
> 
> our basic approach is (currently) the following:
> 
> 1) get disk info
> 2) Qemu: add dirty bitmaps for currently used, replicated disks
> 3) storage/ZFS: do a regular replication of all replicated disks (used AND 
> unused)

I take it that the ZFS replication is not an ongoing process but
something that terminates, so you need QEMU to pick up the difference
that 

Re: [Qemu-devel] [PATCH 2/2] migration: extract ram_load_precopy

[Dr. David Alan Gilbert]

* Wei Yang (richardw.y...@linux.intel.com) wrote:
> After cleanup, it would be clear to audience there are two cases
> ram_load:
> 
>   * precopy
>   * postcopy
> 
> And it is not necessary to check postcopy_running on each iteration for
> precopy.
> 
> Signed-off-by: Wei Yang 
> ---
>  migration/ram.c | 73 +++--
>  1 file changed, 46 insertions(+), 27 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 6bfdfae16e..5f6f07b255 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4200,40 +4200,26 @@ static void colo_flush_ram_cache(void)
>  trace_colo_flush_ram_cache_end();
>  }
>  
> -static int ram_load(QEMUFile *f, void *opaque, int version_id)
> +/**
> + * ram_load_precopy: load a page in precopy case

This comment is wrong - although I realise you copied it from the
postcopy case; they don't just load a single page; they load 'pages'

Other than that, I think it's OK, so:


Reviewed-by: Dr. David Alan Gilbert 

> + * Returns 0 for success or -errno in case of error
> + *
> + * Called in precopy mode by ram_load().
> + * rcu_read_lock is taken prior to this being called.
> + *
> + * @f: QEMUFile where to send the data
> + */
> +static int ram_load_precopy(QEMUFile *f)
>  {
> -int flags = 0, ret = 0, invalid_flags = 0;
> -static uint64_t seq_iter;
> -int len = 0;
> -/*
> - * If system is running in postcopy mode, page inserts to host memory 
> must
> - * be atomic
> - */
> -bool postcopy_running = postcopy_is_running();
> +int flags = 0, ret = 0, invalid_flags = 0, len = 0;
>  /* ADVISE is earlier, it shows the source has the postcopy capability on 
> */
>  bool postcopy_advised = postcopy_is_advised();
> -
> -seq_iter++;
> -
> -if (version_id != 4) {
> -return -EINVAL;
> -}
> -
>  if (!migrate_use_compression()) {
>  invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE;
>  }
> -/* This RCU critical section can be very long running.
> - * When RCU reclaims in the code start to become numerous,
> - * it will be necessary to reduce the granularity of this
> - * critical section.
> - */
> -rcu_read_lock();
> -
> -if (postcopy_running) {
> -ret = ram_load_postcopy(f);
> -}
>  
> -while (!postcopy_running && !ret && !(flags & RAM_SAVE_FLAG_EOS)) {
> +while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
>  ram_addr_t addr, total_ram_bytes;
>  void *host = NULL;
>  uint8_t ch;
> @@ -4390,6 +4376,39 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>  }
>  }
>  
> +return ret;
> +}
> +
> +static int ram_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +int ret = 0;
> +static uint64_t seq_iter;
> +/*
> + * If system is running in postcopy mode, page inserts to host memory 
> must
> + * be atomic
> + */
> +bool postcopy_running = postcopy_is_running();
> +
> +seq_iter++;
> +
> +if (version_id != 4) {
> +return -EINVAL;
> +}
> +
> +/*
> + * This RCU critical section can be very long running.
> + * When RCU reclaims in the code start to become numerous,
> + * it will be necessary to reduce the granularity of this
> + * critical section.
> + */
> +rcu_read_lock();
> +
> +if (postcopy_running) {
> +ret = ram_load_postcopy(f);
> +} else {
> +ret = ram_load_precopy(f);
> +}
> +
>  ret |= wait_for_decompress_done();
>  rcu_read_unlock();
>  trace_ram_load_complete(ret, seq_iter);
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PULL v4 00/23 for 4.1-rc2] testing updates

[Alex Bennée]

The following changes since commit 4da6c0f9ebbdaaf4315c71ccd288d02b9087f8af:

  Merge remote-tracking branch 'remotes/amarkovic2/tags/mips-queue-jul-23-2019' 
into staging (2019-07-23 12:49:39 +0100)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-testing-230719-4

for you to fetch changes up to 98808c3d0c162aba93fe7840a34b54c4814332d4:

  tests/docker: Refresh APT cache before installing new packages on Debian 
(2019-07-23 17:19:11 +0100)


Final testing updates:

  - docker sphinx updates
  - windows build re-enabled in CI
  - travis_retry for make check
  - build fixes
  - docker cache fixes


Alex Bennée (7):
  tests/docker: add test-misc for building tools & docs
  tests/migration-test: don't spam the logs when we fail
  tests/dockerfiles: update the win cross builds to stretch
  shippable: re-enable the windows cross builds
  hw/i386: also turn off VMMOUSE is VMPORT is disabled
  travis: enable travis_retry for check phase
  tests/docker: invoke the DEBUG shell with --noprofile/--norc

Marc-André Lureau (1):
  archive-source: also create a stash for submodules

Philippe Mathieu-Daudé (12):
  tests/docker: Install Sphinx in the Ubuntu images
  tests/docker: Install Sphinx in the Fedora image
  tests/docker: Install Ubuntu images noninteractively
  tests/docker: Install Sphinx in the Debian images
  tests/docker: Install the NSIS tools in the MinGW capable images
  tests/docker: Set the correct cross-PKG_CONFIG_PATH in the MXE images
  tests/docker: Install texinfo in the Fedora image
  buildsys: The NSIS Windows build requires the documentation installed
  buildsys: The NSIS Windows build requires qemu-nsis.bmp installed
  tests/docker: Let the test-mingw test generate a NSIS installer
  NSIS: Add missing firmware blobs
  tests/docker: Refresh APT cache before installing new packages on Debian

Thomas Huth (3):
  tests/qemu-iotests/check: Allow tests without groups
  tests/qemu-iotests/group: Remove some more tests from the "auto" group
  tests/qemu-iotests: Don't use 'seq' in the iotests

 .shippable.yml |   9 +-
 .travis.yml|   2 +-
 Makefile   |   3 +-
 hw/i386/Kconfig|   4 +-
 qemu.nsi   |  11 ++
 scripts/archive-source.sh  |  18 ++--
 tests/docker/Makefile.include  |   6 +-
 tests/docker/dockerfiles/debian-alpha-cross.docker |   5 +-
 tests/docker/dockerfiles/debian-amd64.docker   |  20 ++--
 tests/docker/dockerfiles/debian-arm64-cross.docker |  16 +--
 tests/docker/dockerfiles/debian-armel-cross.docker |  13 +--
 tests/docker/dockerfiles/debian-armhf-cross.docker |  16 +--
 .../dockerfiles/debian-buster-arm64-cross.docker   |   5 +-
 tests/docker/dockerfiles/debian-hppa-cross.docker  |   5 +-
 tests/docker/dockerfiles/debian-m68k-cross.docker  |   5 +-
 tests/docker/dockerfiles/debian-mips-cross.docker  |  16 +--
 .../docker/dockerfiles/debian-mips64-cross.docker  |   5 +-
 .../dockerfiles/debian-mips64el-cross.docker   |  16 +--
 .../docker/dockerfiles/debian-mipsel-cross.docker  |  16 +--
 tests/docker/dockerfiles/debian-ports.docker   |   4 +-
 .../docker/dockerfiles/debian-powerpc-cross.docker |   5 +-
 tests/docker/dockerfiles/debian-ppc64-cross.docker |   5 +-
 .../docker/dockerfiles/debian-ppc64el-cross.docker |  14 +--
 .../docker/dockerfiles/debian-riscv64-cross.docker |   5 +-
 tests/docker/dockerfiles/debian-s390x-cross.docker |  15 +--
 tests/docker/dockerfiles/debian-sh4-cross.docker   |   5 +-
 tests/docker/dockerfiles/debian-sid.docker |   6 +-
 .../docker/dockerfiles/debian-sparc64-cross.docker |   5 +-
 tests/docker/dockerfiles/debian-win32-cross.docker |  13 ++-
 tests/docker/dockerfiles/debian-win64-cross.docker |  13 ++-
 .../docker/dockerfiles/debian-xtensa-cross.docker  |   4 +-
 tests/docker/dockerfiles/debian10.docker   |   7 +-
 tests/docker/dockerfiles/debian8.docker|   6 +-
 .../{debian8-mxe.docker => debian9-mxe.docker} |  15 +--
 tests/docker/dockerfiles/debian9.docker|   7 +-
 tests/docker/dockerfiles/fedora.docker |   4 +
 tests/docker/dockerfiles/ubuntu.docker |   3 +-
 tests/docker/dockerfiles/ubuntu1804.docker |   3 +-
 tests/docker/run   |   4 +-
 tests/docker/test-mingw|   4 +-
 tests/docker/test-misc |  22 
 tests/migration-test.c |  19 ++--
 tests/qemu-iotests/007 |   2 +-
 tests/qemu-iotests/011 |   2 +-
 

Re: [Qemu-devel] [PULL v3 for 4.1 00/23] testing updates

[Alex Bennée]

Alex Bennée  writes:

> The following changes since commit 4da6c0f9ebbdaaf4315c71ccd288d02b9087f8af:
>
>   Merge remote-tracking branch 
> 'remotes/amarkovic2/tags/mips-queue-jul-23-2019' into staging (2019-07-23 
> 12:49:39 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-testing-230719-3

scratch that, I missed a line continuation when manually applying the
apt-update patch to the debian windows image, v4 incoming.

--
Alex Bennée

[Qemu-devel] [Bug 1670170] Re: `qemu-system-sparc64 -M Niagara` Aborted (core dumped)

[Mark Cave-Ayland]

It's difficult to tell as you haven't posted your complete command line,
however it looks as if you're missing the path to the rom images:

https://qemu.weilnetz.de/doc/qemu-doc.html#Sparc64-System-emulator
https://wiki.qemu.org/Documentation/Platforms/SPARC#Compatibility


ATB,

Mark.

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

Title:
  `qemu-system-sparc64 -M Niagara` Aborted (core dumped)

Status in QEMU:
  Fix Released

Bug description:
  > qemu-system-sparc64 -M Niagara
  qemu: fatal: Trap 0x0064 while trap level (6) >= MAXTL (6), Error state
  pc: 4c80  npc: 4c84
  %g0-3:    
  %g4-7:    
  %o0-3:     
  %o4-7:     
  %l0-3: 07f0 01ff 01fff008  
  %l4-7:     
  %i0-3:     
  %i4-7:     
  %f00:     
  %f08:     
  %f16:     
  %f24:     
  %f32:     
  %f40:     
  %f48:     
  %f56:     
  pstate: 0414 ccr: 44 (icc: -Z-- xcc: -Z--) asi: 00 tl: 6 pil: 0
  cansave: 6 canrestore: 0 otherwin: 0 wstate: 0 cleanwin: 6 cwp: 7
  fsr:  y:  fprs: 

  Aborted (core dumped)

  > qemu-system-sparc64 -M help
  Supported machines are:
  Niagara  Sun4v platform, Niagara
  none empty machine
  sun4uSun4u platform (default)
  sun4vSun4v platform

  > qemu-system-sparc64 -version
  QEMU emulator version 2.8.0(Virtualization:Staging / SLE_12_SP2)

  from
  https://build.opensuse.org/package/show/Virtualization:Staging/qemu on
  openSUSE Leap 42.2.

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

Re: [Qemu-devel] [PATCH v2 1/2] configure: Define target access alignment in configure

[Aleksandar Markovic]

Hello, Tony

On Mon, Jul 22, 2019 at 5:27 AM  wrote:

> Move the define of target access alignment earlier from
> target/foo/cpu.h to configure.
>
>
It would be better if the commit message explained WHY the change is needed
and HOW it is achieved - not WHAT the change is.

Yours,
Aleksandar


> Signed-off-by: Tony Nguyen 
> ---
>  configure | 10 +-
>  include/exec/poison.h |  1 +
>  include/qom/cpu.h |  2 +-
>  target/alpha/cpu.h|  2 --
>  target/hppa/cpu.h |  1 -
>  target/mips/cpu.h |  2 --
>  target/sh4/cpu.h  |  2 --
>  target/sparc/cpu.h|  2 --
>  target/xtensa/cpu.h   |  2 --
>  tcg/tcg.c |  2 +-
>  tcg/tcg.h |  8 +---
>  11 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/configure b/configure
> index eb635c3..8316a16 100755
> --- a/configure
> +++ b/configure
> @@ -7424,8 +7424,13 @@ for target in $target_list; do
>  target_dir="$target"
>  config_target_mak=$target_dir/config-target.mak
>  target_name=$(echo $target | cut -d '-' -f 1)
> +target_aligned_only="no"
> +case "$target_name" in
> +
> alpha|hppa|mips64el|mips64|mipsel|mips|mipsn32|mipsn32el|sh4|sh4eb|sparc|sparc64|sparc32plus|xtensa|xtensaeb)
> +  target_aligned_only="yes"
> +  ;;
> +esac
>  target_bigendian="no"
> -
>  case "$target_name" in
>
>  
> armeb|aarch64_be|hppa|lm32|m68k|microblaze|mips|mipsn32|mips64|moxie|or1k|ppc|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
>target_bigendian=yes
> @@ -7710,6 +7715,9 @@ fi
>  if supported_whpx_target $target; then
>  echo "CONFIG_WHPX=y" >> $config_target_mak
>  fi
> +if test "$target_aligned_only" = "yes" ; then
> +  echo "TARGET_ALIGNED_ONLY=y" >> $config_target_mak
> +fi
>  if test "$target_bigendian" = "yes" ; then
>echo "TARGET_WORDS_BIGENDIAN=y" >> $config_target_mak
>  fi
> diff --git a/include/exec/poison.h b/include/exec/poison.h
> index b862320..955eb86 100644
> --- a/include/exec/poison.h
> +++ b/include/exec/poison.h
> @@ -35,6 +35,7 @@
>  #pragma GCC poison TARGET_UNICORE32
>  #pragma GCC poison TARGET_XTENSA
>
> +#pragma GCC poison TARGET_ALIGNED_ONLY
>  #pragma GCC poison TARGET_HAS_BFLT
>  #pragma GCC poison TARGET_NAME
>  #pragma GCC poison TARGET_SUPPORTS_MTTCG
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 5ee0046..9b50b73 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -89,7 +89,7 @@ struct TranslationBlock;
>   * @do_unassigned_access: Callback for unassigned access handling.
>   * (this is deprecated: new targets should use do_transaction_failed
> instead)
>   * @do_unaligned_access: Callback for unaligned access handling, if
> - * the target defines #ALIGNED_ONLY.
> + * the target defines #TARGET_ALIGNED_ONLY.
>   * @do_transaction_failed: Callback for handling failed memory
> transactions
>   * (ie bus faults or external aborts; not MMU faults)
>   * @virtio_is_big_endian: Callback to return %true if a CPU which supports
> diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
> index b3e8a82..16eb804 100644
> --- a/target/alpha/cpu.h
> +++ b/target/alpha/cpu.h
> @@ -23,8 +23,6 @@
>  #include "cpu-qom.h"
>  #include "exec/cpu-defs.h"
>
> -#define ALIGNED_ONLY
> -
>  /* Alpha processors have a weak memory model */
>  #define TCG_GUEST_DEFAULT_MO  (0)
>
> diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
> index aab251b..2be67c2 100644
> --- a/target/hppa/cpu.h
> +++ b/target/hppa/cpu.h
> @@ -30,7 +30,6 @@
> basis.  It's probably easier to fall back to a strong memory model.  */
>  #define TCG_GUEST_DEFAULT_MOTCG_MO_ALL
>
> -#define ALIGNED_ONLY
>  #define MMU_KERNEL_IDX   0
>  #define MMU_USER_IDX 3
>  #define MMU_PHYS_IDX 4
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 21c0615..c13cd4e 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -1,8 +1,6 @@
>  #ifndef MIPS_CPU_H
>  #define MIPS_CPU_H
>
> -#define ALIGNED_ONLY
> -
>  #include "cpu-qom.h"
>  #include "exec/cpu-defs.h"
>  #include "fpu/softfloat.h"
> diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h
> index aee733e..ecaa7a1 100644
> --- a/target/sh4/cpu.h
> +++ b/target/sh4/cpu.h
> @@ -23,8 +23,6 @@
>  #include "cpu-qom.h"
>  #include "exec/cpu-defs.h"
>
> -#define ALIGNED_ONLY
> -
>  /* CPU Subtypes */
>  #define SH_CPU_SH7750  (1 << 0)
>  #define SH_CPU_SH7750S (1 << 1)
> diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
> index 8ed2250..1406f0b 100644
> --- a/target/sparc/cpu.h
> +++ b/target/sparc/cpu.h
> @@ -5,8 +5,6 @@
>  #include "cpu-qom.h"
>  #include "exec/cpu-defs.h"
>
> -#define ALIGNED_ONLY
> -
>  #if !defined(TARGET_SPARC64)
>  #define TARGET_DPREGS 16
>  #else
> diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
> index 2c27713..0459243 100644
> --- a/target/xtensa/cpu.h
> +++ b/target/xtensa/cpu.h
> @@ -32,8 +32,6 @@
>  #include "exec/cpu-defs.h"
>  #include "xtensa-isa.h"
>
> -#define ALIGNED_ONLY
> -
>  /* Xtensa processors have a weak memory model */
>  #define TCG_GUEST_DEFAULT_MO  

Re: [Qemu-devel] [PATCH v2 2/2] configure: Cosmetic yes to "yes" for consistency

[Aleksandar Markovic]

Hi, Tony

On Mon, Jul 22, 2019 at 5:28 AM  wrote:

> Signed-off-by: Tony Nguyen 
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 8316a16..c07687c 100755
> --- a/configure
> +++ b/configure
> @@ -7433,7 +7433,7 @@ esac
>  target_bigendian="no"
>  case "$target_name" in
>
>  
> armeb|aarch64_be|hppa|lm32|m68k|microblaze|mips|mipsn32|mips64|moxie|or1k|ppc|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
> -  target_bigendian=yes
> +  target_bigendian="yes"
>;;
>  esac
>  target_softmmu="no"
> --
> 1.8.3.1
>
>
But, there are loads of similar cases (see 'grep "=yes" configure' output)
- what do we do to them? Perhaps not all should be corrected in a similar
way, but it looks to me that we should not limit this patch to just an
individual case. If we do the cleanup, let's do it to be complete or almost
complete. The same, of course, goes for "=no", and other variable
assignments.

Yours,
Aleksandar

$ grep "=yes" configure
export CCACHE_RECACHE=yes
git_update=yes
  --help|-h) show_help=yes
  --enable-git-update) git_update=yes
  --enable-debug-mutex) debug_mutex=yes
  --enable-libpmem) libpmem=yes
  *"-softmmu "*) softmmu=yes
have_broken_size_max=yes
  l2tpv3=yes
test "$vhost_user" = "" && vhost_user=yes
  test "$vhost_net_user" = "yes" && vhost_net=yes
  test "$vhost_kernel" = "yes" && vhost_net=yes
xen=yes
  xen=yes
  xen=yes
  xen=yes
  xen=yes
  xen=yes
  xen=yes
  xen=yes
  xen=yes
xen_pci_passthrough=yes
sparse=yes
have_x11=yes
need_x11=yes
gcrypt_hmac=yes
tasn1=yes
auth_pam=yes
have_ifaddrs_h=yes
sdl=yes
sdl_image=yes
need_x11=yes
vnc_sasl=yes
vnc_jpeg=yes
vnc_png=yes
xkbcommon=yes
vde=yes
netmap=yes
cap_ng=yes
brlapi=yes
iconv_found=yes
iconv=yes
curses_found=yes
curses=yes
curl=yes
bluez=yes
gio=yes
mpathpersist=yes
mpathpersist_new_api=yes
  mpathpersist=yes
cap=yes
  pthread=yes
  pthread=yes
  found=yes
  pthread_setname_np_w_tid=yes
  pthread_setname_np_wo_tid=yes
rbd=yes
libssh=yes
linux_aio=yes
tpm=yes
attr=yes
attr=yes
libattr=yes
  iovec=yes
  preadv=yes
  fdt_required=yes
  fdt=yes
opengl=yes
opengl_dmabuf=yes
need_x11=yes
  inotify=yes
  inotify1=yes
  pipe2=yes
  accept4=yes
  splice=yes
numa=yes
  signalfd=yes
  optreset=yes
  eventfd=yes
  memfd=yes
have_usbfs=yes
  fallocate=yes
  fallocate_punch_hole=yes
  fallocate_zero_range=yes
posix_fallocate=yes
  sync_file_range=yes
  fiemap=yes
  dup3=yes
  ppoll=yes
  prctl_pr_set_timerslack=yes
  epoll=yes
  epoll_create1=yes
  sendfile=yes
  timerfd=yes
  setns=yes
  clock_adjtime=yes
  syncfs=yes
docs=yes
  byteswap_h=yes
  bswap_h=yes
guest_agent_ntddscsi=yes
fdatasync=yes
madvise=yes
posix_madvise=yes
posix_memalign=yes
posix_syslog=yes
sem_timedwait=yes
strchrnul=yes
ucontext_works=yes
  coroutine_pool=yes
open_by_handle_at=yes
linux_magic_h=yes
pragma_diagnostic_available=yes
valgrind_h=yes
has_environ=yes
cpuid_h=yes
int128=yes
atomic128=yes
cmpxchg128=yes
  atomic64=yes
  vector16=yes
getauxval=yes
ccache_cpp2=yes
have_copy_file_range=yes
have_fsxattr=yes
have_membarrier=yes
have_membarrier=yes
have_rtnetlink=yes
have_af_vsock=yes
have_afalg=yes
have_sysmacros=yes
vxhs=yes
have_static_assert=yes
have_utmpx=yes
have_getrandom=yes
  have_asan=yes
  have_ubsan=yes
  have_asan_iface_h=yes
  have_asan_iface_fiber=yes
  ivshmem=yes
  virtfs=yes
  mpath=yes
  guest_agent=yes
guest_agent_msi=yes
echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak
  echo "BUILD_DOCS=yes" >> $config_host_mak
  echo "INSTALL_BLOBS=yes" >> $config_host_mak
  target_bigendian=yes
mttcg=yes
mttcg=yes
mttcg=yes
mttcg=yes
mttcg=yes

[Qemu-devel] [PATCH] pc-dimm: fix crash when invalid slot number is used

[Igor Mammedov]

QEMU will crash with:
  Segmentation fault (core dumped)
when negative slot number is used, ex:
  qemu-system-x86_64 -m 1G,maxmem=20G,slots=256 \
  -object memory-backend-ram,id=mem1,size=1G \
  -device pc-dimm,id=dimm1,memdev=mem1,slot=-2

fix it by checking that slot number is within valid range.

Signed-off-by: Igor Mammedov 
---
 hw/mem/pc-dimm.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index b1239fd0d3..29c785799c 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -38,6 +38,13 @@ void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState 
*machine,
 
 slot = object_property_get_int(OBJECT(dimm), PC_DIMM_SLOT_PROP,
_abort);
+if ((slot < 0 || slot >= machine->ram_slots) &&
+ slot != PC_DIMM_UNASSIGNED_SLOT) {
+error_setg(_err, "invalid slot number, valid range is [0-%"
+   PRIu64 "]", machine->ram_slots - 1);
+goto out;
+}
+
 slot = pc_dimm_get_free_slot(slot == PC_DIMM_UNASSIGNED_SLOT ? NULL : 
,
  machine->ram_slots, _err);
 if (local_err) {
-- 
2.18.1

Re: [Qemu-devel] [PATCH for 4.2 1/3] glib: bump min required glib library version to 2.48

[Daniel P . Berrangé]

On Tue, Jul 23, 2019 at 06:02:43PM +0200, Thomas Huth wrote:
> On 23/07/2019 17.48, Daniel P. Berrangé wrote:
> > Per supported platforms doc[1], the various min glib on relevant distros is:
> > 
> >   RHEL-8: 2.56.1
> >   RHEL-7: 2.50.3
> >   Debian (Buster): 2.58.3
> >   Debian (Stretch): 2.50.3
> >   OpenBSD (Ports): 2.58.3
> >   FreeBSD (Ports): 2.56.3
> >   OpenSUSE Leap 15: 2.54.3
> >   SLE12-SP2: 2.48.2
> >   Ubuntu (Xenial): 2.48.0
> >   macOS (Homebrew): 2.56.0
> > 
> > This suggests that a minimum glib of 2.48 is a reasonable target.
> > 
> > Compared to the previous version bump in
> > 
> >   commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
> >   Author: Daniel P. Berrangé 
> >   Date:   Fri May 4 15:34:46 2018 +0100
> > 
> > glib: bump min required glib library version to 2.40
> > 
> > This will result in us dropping support for Debian Jessie and
> > Ubuntu 14.04.
> > 
> > As per the commit message 14.04 was already outside our list
> > of supported build platforms and an exception was only made
> > because one of the build hosts used during merge testing was
> > stuck on 14.04.
> > 
> > Debian Jessie is justified to drop because we only aim to
> > support at most 2 major versions of Debian at any time. This
> > means Buster and Stretch at this time.
> > 
> > The g_strv_contains compat code is dropped as this API is
> > present since 2.44
> > 
> > The g_assert_cmpmem compat code is dropped as this API is
> > present since 2.46
> > 
> > [1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
> > 
> > Reviewed-by: Thomas Huth 
> 
> Sorry, my memory fails ... when did I review this patch?

Doh, sorry - cut & paste screwup. I'll drop this line.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for 4.2 1/3] glib: bump min required glib library version to 2.48

[Thomas Huth]

On 23/07/2019 17.48, Daniel P. Berrangé wrote:
> Per supported platforms doc[1], the various min glib on relevant distros is:
> 
>   RHEL-8: 2.56.1
>   RHEL-7: 2.50.3
>   Debian (Buster): 2.58.3
>   Debian (Stretch): 2.50.3
>   OpenBSD (Ports): 2.58.3
>   FreeBSD (Ports): 2.56.3
>   OpenSUSE Leap 15: 2.54.3
>   SLE12-SP2: 2.48.2
>   Ubuntu (Xenial): 2.48.0
>   macOS (Homebrew): 2.56.0
> 
> This suggests that a minimum glib of 2.48 is a reasonable target.
> 
> Compared to the previous version bump in
> 
>   commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
>   Author: Daniel P. Berrangé 
>   Date:   Fri May 4 15:34:46 2018 +0100
> 
> glib: bump min required glib library version to 2.40
> 
> This will result in us dropping support for Debian Jessie and
> Ubuntu 14.04.
> 
> As per the commit message 14.04 was already outside our list
> of supported build platforms and an exception was only made
> because one of the build hosts used during merge testing was
> stuck on 14.04.
> 
> Debian Jessie is justified to drop because we only aim to
> support at most 2 major versions of Debian at any time. This
> means Buster and Stretch at this time.
> 
> The g_strv_contains compat code is dropped as this API is
> present since 2.44
> 
> The g_assert_cmpmem compat code is dropped as this API is
> present since 2.46
> 
> [1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
> 
> Reviewed-by: Thomas Huth 

Sorry, my memory fails ... when did I review this patch?

 Thomas

Re: [Qemu-devel] [PULL for 4.1-rc2 00/23] testing updates (green CI!)

[Thomas Huth]

On 23/07/2019 15.01, Peter Maydell wrote:
> On Tue, 23 Jul 2019 at 13:58, Alex Bennée  wrote:
>>
>>
>> Peter Maydell  writes:
>>
>>> On Tue, 23 Jul 2019 at 13:16, Philippe Mathieu-Daudé  
>>> wrote:

 On 7/23/19 1:46 PM, Alex Bennée wrote:
>> I see Thomas Huth has a patch on-list for that, but this
>> didn't manifest as a problem before this pullreq.
>
> OK, I'll add it and rebuild the PR.

 But Thomas got another error later...
>>>
>>> If we're not sure what all the needed fixes are we
>>> should probably just drop the change that starts
>>> running the iotests under 'make check'. Otherwise this
>>> pullreq is going to miss rc2, and it's too big to go into rc3.

I think OpenBSD was the final thing that is not working. I've checked
freebsd and macOS and they were working fine for me there. The netbsd
image does not have "bash", so the tests are skipped there.

>> I'm just double checking now - it does seem OpenBSD is very slow even
>> with 18 cores assigned.
> 
> We should avoid enabling "very slow" tests in make check too...

It's not the iotests that are slow here, it's the whole openbsd image -
even compiling is way slower than natively... it feels like it is
ignoring the multiple cores and only running single-threaded.

 Thomas

[Qemu-devel] [PATCH for 4.2 3/3] crypto: use auto cleanup for many stack variables

[Daniel P . Berrangé]

Simplify cleanup paths by using glib's auto cleanup macros for stack
variables, allowing several goto jumps / labels to be eliminated.

Signed-off-by: Daniel P. Berrangé 
---
 crypto/afsplit.c  | 28 +---
 crypto/block-luks.c   | 74 +--
 crypto/block.c| 15 +++--
 crypto/pbkdf.c|  5 +--
 crypto/secret.c   |  9 +++---
 crypto/tlscredsanon.c | 16 --
 crypto/tlscredspsk.c  |  5 ++-
 crypto/tlscredsx509.c | 16 +++---
 8 files changed, 53 insertions(+), 115 deletions(-)

diff --git a/crypto/afsplit.c b/crypto/afsplit.c
index 328d68c96b..b1a5a20899 100644
--- a/crypto/afsplit.c
+++ b/crypto/afsplit.c
@@ -58,7 +58,7 @@ static int qcrypto_afsplit_hash(QCryptoHashAlgorithm hash,
 }
 
 for (i = 0; i < hashcount; i++) {
-uint8_t *out = NULL;
+g_autofree uint8_t *out = NULL;
 size_t outlen = 0;
 uint32_t iv = cpu_to_be32(i);
 struct iovec in[] = {
@@ -79,7 +79,6 @@ static int qcrypto_afsplit_hash(QCryptoHashAlgorithm hash,
 assert(outlen == digestlen);
 memcpy(block + (i * digestlen), out,
(i == (hashcount - 1)) ? finallen : digestlen);
-g_free(out);
 }
 
 return 0;
@@ -93,13 +92,12 @@ int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
uint8_t *out,
Error **errp)
 {
-uint8_t *block = g_new0(uint8_t, blocklen);
+g_autofree uint8_t *block = g_new0(uint8_t, blocklen);
 size_t i;
-int ret = -1;
 
 for (i = 0; i < (stripes - 1); i++) {
 if (qcrypto_random_bytes(out + (i * blocklen), blocklen, errp) < 0) {
-goto cleanup;
+return -1;
 }
 
 qcrypto_afsplit_xor(blocklen,
@@ -108,18 +106,14 @@ int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
 block);
 if (qcrypto_afsplit_hash(hash, blocklen, block,
  errp) < 0) {
-goto cleanup;
+return -1;
 }
 }
 qcrypto_afsplit_xor(blocklen,
 in,
 block,
 out + (i * blocklen));
-ret = 0;
-
- cleanup:
-g_free(block);
-return ret;
+return 0;
 }
 
 
@@ -130,9 +124,8 @@ int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
uint8_t *out,
Error **errp)
 {
-uint8_t *block = g_new0(uint8_t, blocklen);
+g_autofree uint8_t *block = g_new0(uint8_t, blocklen);
 size_t i;
-int ret = -1;
 
 for (i = 0; i < (stripes - 1); i++) {
 qcrypto_afsplit_xor(blocklen,
@@ -141,7 +134,7 @@ int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
 block);
 if (qcrypto_afsplit_hash(hash, blocklen, block,
  errp) < 0) {
-goto cleanup;
+return -1;
 }
 }
 
@@ -149,10 +142,5 @@ int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
 in + (i * blocklen),
 block,
 out);
-
-ret = 0;
-
- cleanup:
-g_free(block);
-return ret;
+return 0;
 }
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 409ab50f20..c8e0a0caa4 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -425,14 +425,13 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
 Error **errp)
 {
 QCryptoBlockLUKS *luks = block->opaque;
-uint8_t *splitkey;
+g_autofree uint8_t *splitkey = NULL;
 size_t splitkeylen;
-uint8_t *possiblekey;
-int ret = -1;
+g_autofree uint8_t *possiblekey = NULL;
 ssize_t rv;
-QCryptoCipher *cipher = NULL;
+g_autoptr(QCryptoCipher) cipher = NULL;
 uint8_t keydigest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
-QCryptoIVGen *ivgen = NULL;
+g_autoptr(QCryptoIVGen) ivgen = NULL;
 size_t niv;
 
 if (slot->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED) {
@@ -456,7 +455,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
slot->iterations,
possiblekey, masterkeylen,
errp) < 0) {
-goto cleanup;
+return -1;
 }
 
 /*
@@ -472,7 +471,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
   opaque,
   errp);
 if (rv < 0) {
-goto cleanup;
+return -1;
 }
 
 
@@ -482,7 +481,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
 possiblekey, masterkeylen,
 errp);
 if (!cipher) {
-goto cleanup;
+return -1;
 }
 
 niv = qcrypto_cipher_get_iv_len(cipheralg,
@@ -493,7 +492,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
   possiblekey, masterkeylen,
   errp);
 if (!ivgen) {
-goto cleanup;
+

[Qemu-devel] [PATCH for 4.2 1/3] glib: bump min required glib library version to 2.48

[Daniel P . Berrangé]

Per supported platforms doc[1], the various min glib on relevant distros is:

  RHEL-8: 2.56.1
  RHEL-7: 2.50.3
  Debian (Buster): 2.58.3
  Debian (Stretch): 2.50.3
  OpenBSD (Ports): 2.58.3
  FreeBSD (Ports): 2.56.3
  OpenSUSE Leap 15: 2.54.3
  SLE12-SP2: 2.48.2
  Ubuntu (Xenial): 2.48.0
  macOS (Homebrew): 2.56.0

This suggests that a minimum glib of 2.48 is a reasonable target.

Compared to the previous version bump in

  commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
  Author: Daniel P. Berrangé 
  Date:   Fri May 4 15:34:46 2018 +0100

glib: bump min required glib library version to 2.40

This will result in us dropping support for Debian Jessie and
Ubuntu 14.04.

As per the commit message 14.04 was already outside our list
of supported build platforms and an exception was only made
because one of the build hosts used during merge testing was
stuck on 14.04.

Debian Jessie is justified to drop because we only aim to
support at most 2 major versions of Debian at any time. This
means Buster and Stretch at this time.

The g_strv_contains compat code is dropped as this API is
present since 2.44

The g_assert_cmpmem compat code is dropped as this API is
present since 2.46

[1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms

Reviewed-by: Thomas Huth 
Signed-off-by: Daniel P. Berrangé 
---
 configure |  2 +-
 crypto/hmac-glib.c|  5 -
 include/glib-compat.h | 42 ++
 3 files changed, 3 insertions(+), 46 deletions(-)

diff --git a/configure b/configure
index 714e7fb6a1..22dc9c41db 100755
--- a/configure
+++ b/configure
@@ -3636,7 +3636,7 @@ fi
 ##
 # glib support probe
 
-glib_req_ver=2.40
+glib_req_ver=2.48
 glib_modules=gthread-2.0
 if test "$modules" = yes; then
 glib_modules="$glib_modules gmodule-export-2.0"
diff --git a/crypto/hmac-glib.c b/crypto/hmac-glib.c
index 7df627329d..509bbc74c2 100644
--- a/crypto/hmac-glib.c
+++ b/crypto/hmac-glib.c
@@ -21,12 +21,7 @@ static int qcrypto_hmac_alg_map[QCRYPTO_HASH_ALG__MAX] = {
 [QCRYPTO_HASH_ALG_MD5] = G_CHECKSUM_MD5,
 [QCRYPTO_HASH_ALG_SHA1] = G_CHECKSUM_SHA1,
 [QCRYPTO_HASH_ALG_SHA256] = G_CHECKSUM_SHA256,
-/* Support for HMAC SHA-512 in GLib 2.42 */
-#if GLIB_CHECK_VERSION(2, 42, 0)
 [QCRYPTO_HASH_ALG_SHA512] = G_CHECKSUM_SHA512,
-#else
-[QCRYPTO_HASH_ALG_SHA512] = -1,
-#endif
 [QCRYPTO_HASH_ALG_SHA224] = -1,
 [QCRYPTO_HASH_ALG_SHA384] = -1,
 [QCRYPTO_HASH_ALG_RIPEMD160] = -1,
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 1291628e09..0b0ec76299 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -19,12 +19,12 @@
 /* Ask for warnings for anything that was marked deprecated in
  * the defined version, or before. It is a candidate for rewrite.
  */
-#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
+#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_48
 
 /* Ask for warnings if code tries to use function that did not
  * exist in the defined version. These risk breaking builds
  */
-#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_40
+#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_48
 
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
@@ -63,26 +63,6 @@
  * without generating warnings.
  */
 
-static inline gboolean g_strv_contains_qemu(const gchar *const *strv,
-const gchar *str)
-{
-#if GLIB_CHECK_VERSION(2, 44, 0)
-return g_strv_contains(strv, str);
-#else
-g_return_val_if_fail(strv != NULL, FALSE);
-g_return_val_if_fail(str != NULL, FALSE);
-
-for (; *strv != NULL; strv++) {
-if (g_str_equal(str, *strv)) {
-return TRUE;
-}
-}
-
-return FALSE;
-#endif
-}
-#define g_strv_contains(a, b) g_strv_contains_qemu(a, b)
-
 #if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
 /*
  * g_poll has a problem on Windows when using
@@ -92,24 +72,6 @@ static inline gboolean g_strv_contains_qemu(const gchar 
*const *strv,
 gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
 #endif
 
-
-#ifndef g_assert_cmpmem
-#define g_assert_cmpmem(m1, l1, m2, l2)
\
-do {   
\
-gconstpointer __m1 = m1, __m2 = m2;
\
-int __l1 = l1, __l2 = l2;  
\
-if (__l1 != __l2) {
\
-g_assertion_message_cmpnum(
\
-G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   
\
-#l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==",   
\
-__l2, 'i');
\
-} else if (memcmp(__m1, __m2, __l1) != 0) {
\
-

[Qemu-devel] [PATCH for 4.2 2/3] crypto: define cleanup functions for use with g_autoptr

[Daniel P . Berrangé]

Allow crypto structs to be used with g_autoptr, avoiding the need to
explicitly call XXX_free() functions when variables go out of scope on
the stack.

Signed-off-by: Daniel P. Berrangé 
---
 include/crypto/block.h  | 2 ++
 include/crypto/cipher.h | 2 ++
 include/crypto/hmac.h   | 2 ++
 include/crypto/ivgen.h  | 2 ++
 include/crypto/tlssession.h | 2 ++
 5 files changed, 10 insertions(+)

diff --git a/include/crypto/block.h b/include/crypto/block.h
index fe12899831..d49d2c2da9 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -268,4 +268,6 @@ uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block);
  */
 void qcrypto_block_free(QCryptoBlock *block);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoBlock, qcrypto_block_free)
+
 #endif /* QCRYPTO_BLOCK_H */
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index cac90b410c..5928e5ecc7 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -170,6 +170,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm 
alg,
  */
 void qcrypto_cipher_free(QCryptoCipher *cipher);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoCipher, qcrypto_cipher_free)
+
 /**
  * qcrypto_cipher_encrypt:
  * @cipher: the cipher object
diff --git a/include/crypto/hmac.h b/include/crypto/hmac.h
index aa3c97a2ff..ad4d778416 100644
--- a/include/crypto/hmac.h
+++ b/include/crypto/hmac.h
@@ -65,6 +65,8 @@ QCryptoHmac *qcrypto_hmac_new(QCryptoHashAlgorithm alg,
  */
 void qcrypto_hmac_free(QCryptoHmac *hmac);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoHmac, qcrypto_hmac_free)
+
 /**
  * qcrypto_hmac_bytesv:
  * @hmac: the hmac object
diff --git a/include/crypto/ivgen.h b/include/crypto/ivgen.h
index 9b4a62f7bb..e41521519c 100644
--- a/include/crypto/ivgen.h
+++ b/include/crypto/ivgen.h
@@ -203,4 +203,6 @@ QCryptoHashAlgorithm qcrypto_ivgen_get_hash(QCryptoIVGen 
*ivgen);
  */
 void qcrypto_ivgen_free(QCryptoIVGen *ivgen);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoIVGen, qcrypto_ivgen_free)
+
 #endif /* QCRYPTO_IVGEN_H */
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index 816300cdcc..e01e1a9dc2 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -160,6 +160,8 @@ QCryptoTLSSession *qcrypto_tls_session_new(QCryptoTLSCreds 
*creds,
  */
 void qcrypto_tls_session_free(QCryptoTLSSession *sess);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoTLSSession, qcrypto_tls_session_free)
+
 /**
  * qcrypto_tls_session_check_credentials:
  * @sess: the TLS session object
-- 
2.21.0

[Qemu-devel] [PATCH for 4.2 0/3] require newer glib2 to enable autofree'ing of stack variables exiting scope

[Daniel P . Berrangé]

Both GCC and CLang support a C extension attribute((cleanup)) which
allows you to define a function that is invoked when a stack variable
exits scope. This typically used to free the memory allocated to it,
though you're not restricted to this. For example it could be used to
unlock a mutex.

We could use that functionality now, but the syntax is a bit ugly in
plain C. Since version 2.44 of GLib, there have been a few macros to
make it more friendly to use - g_autofree, g_autoptr and
G_DEFINE_AUTOPTR_CLEANUP_FUNC.

  https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html

  https://blogs.gnome.org/desrt/2015/01/30/g_autoptr/

The key selling point is that it simplifies the cleanup code paths,
often eliminating the need to goto cleanup labels. This improves
the readability of the code and makes it less likely that you'll
leak memory accidentally.

Inspired by seeing it added to glib, and used in systemd, Libvirt
finally got around to adopting this in Feb 2019. Overall our
experience with it has been favourable/positive, with the code
simplification being very nice.

The main caveats with it are

 - Only works with GCC or CLang. We don't care as those are
   the only two compilers we declare support for.

 - You must always initialize the variables when declared
   to ensure predictable behaviour when they leave scope.
   Chances are most methods with goto jumps for cleanup
   are doing this already

 - You must not directly return the value that's assigned
   to a auto-cleaned variable. You must steal the pointer
   in some way. ie

BAD:
g_autofree char *wibble = g_strdup("wibble")

return wibble;

GOOD:
g_autofree char *wibble = g_strdup("wibble")
...
return g_steal_pointer(wibble);

g_steal_pointer is an inline function which simply copies
the pointer to a new variable, and sets the original variable
to NULL, thus avoiding cleanup.

I've illustrated the usage by converting a bunch of the crypto code in
QEMU to use auto cleanup.

Daniel P. Berrangé (3):
  glib: bump min required glib library version to 2.48
  crypto: define cleanup functions for use with g_autoptr
  crypto: use auto cleanup for many stack variables

 configure   |  2 +-
 crypto/afsplit.c| 28 --
 crypto/block-luks.c | 74 +++--
 crypto/block.c  | 15 +++-
 crypto/hmac-glib.c  |  5 ---
 crypto/pbkdf.c  |  5 +--
 crypto/secret.c |  9 ++---
 crypto/tlscredsanon.c   | 16 +++-
 crypto/tlscredspsk.c|  5 +--
 crypto/tlscredsx509.c   | 16 +++-
 include/crypto/block.h  |  2 +
 include/crypto/cipher.h |  2 +
 include/crypto/hmac.h   |  2 +
 include/crypto/ivgen.h  |  2 +
 include/crypto/tlssession.h |  2 +
 include/glib-compat.h   | 42 +
 16 files changed, 66 insertions(+), 161 deletions(-)

-- 
2.21.0

Re: [Qemu-devel] [PATCH v4 3/3] net: tap: replace snprintf with g_strdup_printf calls

[Li Qiang]

Stefan Hajnoczi  于2019年7月23日周二 下午9:03写道：

> On Tue, Jul 23, 2019 at 04:17:54PM +0530, P J P wrote:
> > -snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s",
> > - helper, "--use-vnet", fd_buf, br_buf);
> > +helper_cmd = g_strdup_printf("%s %s %s %s", helper,
> > +"--use-vnet", fd_buf, br_buf ? br_buf : "");
>
> The change to the br_buf argument isn't covered in the commit
> description.  Why did you change this, was it a bug, etc?
>


IIUC, if we pass the NULL argument in g_strdup_printf, the 'helper_cmd'
will contain the '(null)' char.
If we pass "" to g_strdup_printf, there is nothing in 'helper_cmd'. The
original is like this.
So here Prasad has to check the 'fd_buf'.

So:

Reviewed-by: Li Qiang 


Thanks,
Li Qiang

Re: [Qemu-devel] [PATCH 1/2] migration: return -EINVAL directly when version_id mismatch

[Dr. David Alan Gilbert]

* Wei Yang (richardw.y...@linux.intel.com) wrote:
> It is not reasonable to continue when version_id mismatch.
> 
> Signed-off-by: Wei Yang 
> ---
>  migration/ram.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 7e34c82a72..6bfdfae16e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -4216,7 +4216,7 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>  seq_iter++;
>  
>  if (version_id != 4) {
> -ret = -EINVAL;
> +return -EINVAL;
>  }

Oh yes that's nice; it's before the lock gets taken so we can just
return


Reviewed-by: Dr. David Alan Gilbert 

>  if (!migrate_use_compression()) {
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration/postcopy: use static PostcopyDiscardState instead of allocating it for each block

[Dr. David Alan Gilbert]

* Wei Yang (richardw.y...@linux.intel.com) wrote:
> On Fri, Jul 19, 2019 at 06:41:28PM +0100, Dr. David Alan Gilbert wrote:
> >* Wei Yang (richardw.y...@linux.intel.com) wrote:
> >> Even we need to do discard for each RAMBlock, we still can leverage the
> >> same memory space to store the information.
> >> 
> >> By doing so, we avoid memory allocation and deallocation to the system
> >> and also avoid potential failure of memory allocation which breaks the
> >> migration.
> >> 
> >> Signed-off-by: Wei Yang 
> >> ---
> >>  migration/postcopy-ram.c | 16 +++-
> >>  1 file changed, 7 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> >> index 9faacacc9e..2e6b076bb7 100644
> >> --- a/migration/postcopy-ram.c
> >> +++ b/migration/postcopy-ram.c
> >> @@ -1377,8 +1377,7 @@ void 
> >> postcopy_fault_thread_notify(MigrationIncomingState *mis)
> >>   *   asking to discard individual ranges.
> >>   *
> >>   * @ms: The current migration state.
> >> - * @offset: the bitmap offset of the named RAMBlock in the migration
> >> - *   bitmap.
> >> + * @offset: the bitmap offset of the named RAMBlock in the migration 
> >> bitmap.
> >>   * @name: RAMBlock that discards will operate on.
> >>   *
> >>   * returns: a new PDS.
> >> @@ -1386,13 +1385,14 @@ void 
> >> postcopy_fault_thread_notify(MigrationIncomingState *mis)
> >>  PostcopyDiscardState *postcopy_discard_send_init(MigrationState *ms,
> >>   const char *name)
> >>  {
> >> -PostcopyDiscardState *res = g_malloc0(sizeof(PostcopyDiscardState));
> >> +static PostcopyDiscardState res = {0};
> >
> >Do you think it would be better to make this a static at the top of
> >migration/postcopy-ram.c and then we could remove the pds parameters
> >from postcopy_discard_send_range and friends?
> 
> Just took a look into this one. One problem is not all its friends are in
> migration/postcopy-ram.c
> 
> For example, postcopy_chunk_hostpages_pass() is in migration/ram.c.

But doesn't that just pass teh pds back to postcopy_discard_send_range
which is in postcopy-ram ?

Dave

> Which way do you prefer?
> 
> >If there's only one pds then we don't need to pass the pointer around.
> >
> >Dave
> >
> >> -if (res) {
> >> -res->ramblock_name = name;
> >> -}
> >> +res.ramblock_name = name;
> >> +res.cur_entry = 0;
> >> +res.nsentwords = 0;
> >> +res.nsentcmds = 0;
> >>  
> >> -return res;
> >> +return 
> >>  }
> >>  
> >>  /**
> >> @@ -1449,8 +1449,6 @@ void postcopy_discard_send_finish(MigrationState 
> >> *ms, PostcopyDiscardState *pds)
> >>  
> >>  trace_postcopy_discard_send_finish(pds->ramblock_name, 
> >> pds->nsentwords,
> >> pds->nsentcmds);
> >> -
> >> -g_free(pds);
> >>  }
> >>  
> >>  /*
> >> -- 
> >> 2.17.1
> >> 
> >--
> >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> 
> -- 
> Wei Yang
> Help you, Help me
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration/multifd: multifd_load_cleanup() always return 0

[Dr. David Alan Gilbert]

* Wei Yang (richardw.y...@linux.intel.com) wrote:
> multifd_load_cleanup() always return 0 and never use the errp, just
> cleanup a little.
> 
> Signed-off-by: Wei Yang 

I'd rather leave this; it wouldn't be that odd for us to add an error
check there later.

Dave

> ---
>  migration/migration.c | 9 ++---
>  migration/ram.c   | 7 +++
>  migration/ram.h   | 2 +-
>  3 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 27ca10122f..38d2fdfa73 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -399,10 +399,7 @@ static void process_incoming_migration_bh(void *opaque)
>   */
>  qemu_announce_self(>announce_timer, migrate_announce_params());
>  
> -if (multifd_load_cleanup(_err) != 0) {
> -error_report_err(local_err);
> -autostart = false;
> -}
> +multifd_load_cleanup();
>  /* If global state section was not received or we are in running
> state, we need to obey autostart. Any other state is set with
> runstate_set. */
> @@ -506,9 +503,7 @@ fail:
>  migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
>MIGRATION_STATUS_FAILED);
>  qemu_fclose(mis->from_src_file);
> -if (multifd_load_cleanup(_err) != 0) {
> -error_report_err(local_err);
> -}
> +multifd_load_cleanup();
>  exit(EXIT_FAILURE);
>  }
>  
> diff --git a/migration/ram.c b/migration/ram.c
> index 66792568e2..7d30f8484c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1243,13 +1243,12 @@ static void multifd_recv_terminate_threads(Error *err)
>  }
>  }
>  
> -int multifd_load_cleanup(Error **errp)
> +void multifd_load_cleanup(void)
>  {
>  int i;
> -int ret = 0;
>  
>  if (!migrate_use_multifd()) {
> -return 0;
> +return;
>  }
>  multifd_recv_terminate_threads(NULL);
>  for (i = 0; i < migrate_multifd_channels(); i++) {
> @@ -1276,7 +1275,7 @@ int multifd_load_cleanup(Error **errp)
>  g_free(multifd_recv_state);
>  multifd_recv_state = NULL;
>  
> -return ret;
> +return;
>  }
>  
>  static void multifd_recv_sync_main(void)
> diff --git a/migration/ram.h b/migration/ram.h
> index e0a499b956..6fdd449899 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -44,7 +44,7 @@ uint64_t ram_bytes_total(void);
>  int multifd_save_setup(void);
>  void multifd_save_cleanup(void);
>  int multifd_load_setup(void);
> -int multifd_load_cleanup(Error **errp);
> +void multifd_load_cleanup(void);
>  bool multifd_recv_all_channels_created(void);
>  bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp);
>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK