Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[David Edmondson]

Eric Blake  writes:

> On 1/24/20 4:34 AM, David Edmondson wrote:
>> In many cases the target of a convert operation is a newly provisioned
>> target that the user knows is blank (filled with zeroes). In this
>> situation there is no requirement for qemu-img to wastefully zero out
>> the entire device.
>> 
>> Add a new option, --target-is-zero, allowing the user to indicate that
>> an existing target device is already zero filled.
>> 
>> Signed-off-by: David Edmondson 
>> ---
>>   qemu-img-cmds.hx |  4 ++--
>>   qemu-img.c   | 25 ++---
>>   qemu-img.texi|  4 
>>   3 files changed, 28 insertions(+), 5 deletions(-)
>
> I'm working up a patch series that tries to auto-set this flag without 
> user interaction where possible (for example, if lseek(fd, 0, SEEK_DATA) 
> returns EOF, or if fstat() reports 0 blocks allocated, or if qcow2 sees 
> no L2 tables allocated, or a proposed extension to NBD passes on the 
> same...).  I may rebase my series on top of your patch and tweak things 
> in yours accordingly.
>
> But as it stands, the idea makes sense to me; even if we add ways for 
> some images to efficiently report initial state (and our existing 
> bdrv_has_zero_init() is NOT such a method), there are enough other 
> scenarios where the knob will be the only way to let qemu-img know the 
> intent.

Having qemu-img figure things out on its own is obviously desirable, but
I agree that there are enough cases where this won't be possible and,
given the resulting performance improvement, it will still be useful to
allow the caller to force things.

>> +case OPTION_TARGET_IS_ZERO:
>> +/*
>> + * The user asserting that the target is blank has the
>> + * same effect as the target driver supporting zero
>> + * initialisation.
>
> Hmm. A git grep shows that 'initialization' has 200 hits, 
> 'initialisation' has only 29. But I think it's a US vs. UK thing, so I 
> don't care which spelling you use.

Yes, it's British English spelling. It was unconscious - I'll switch if
there is an existing policy.

> Reviewed-by: Eric Blake 

Thanks.

If the conversion of the documentation to rST is imminent then I'll wait
for that before submitting a followup with corresponding changes applied
to the new docs.

dme.
-- 
I'd come on over but I haven't got a raincoat.

Re: [PATCH v2 2/2] doc: Use @code rather than @var for options

[David Edmondson]

Eric Blake  writes:

> On 1/24/20 4:34 AM, David Edmondson wrote:
>> Texinfo defines @var for metasyntactic variables and such terms are
>> shown in upper-case or italics in the output of makeinfo. When
>> considering an option to a command, such as "-n", upper-casing is
>> undesirable as it may confuse the reader or be in conflict with the
>> equivalent upper-case option.
>> 
>> Replace the use of @var for options with @code to avoid this.
>> 
>> Signed-off-by: David Edmondson 
>> ---
>>   qemu-img.texi | 16 
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>
> Is this patch still needed given Peter's recent push to move to rST 
> documentation?

No, it would be obviated by those changes.

>> 
>> diff --git a/qemu-img.texi b/qemu-img.texi
>> index 3b6dfd8682..6b4a1ac961 100644
>> --- a/qemu-img.texi
>> +++ b/qemu-img.texi
>> @@ -74,13 +74,13 @@ keys.
>>   @item --image-opts
>>   Indicates that the source @var{filename} parameter is to be interpreted as 
>> a
>>   full option string, not a plain filename. This parameter is mutually
>> -exclusive with the @var{-f} parameter.
>> +exclusive with the @code{-f} parameter.
>>   
>
>
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org

dme.
-- 
Don't you know you're never going to get to France.

[Bug 1856837] Re: qemu 4.2.0 arm segmentation fault with gcc 9.2

[Fabian Godehardt]

Thanks, this helps a lot! We will now check the code again and see what
causes the behaviour.

Fabian

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

Title:
  qemu 4.2.0 arm  segmentation fault with gcc 9.2

Status in QEMU:
  New

Bug description:
  As discussed with f4bug yesterday on IRC here comes the bug
  description.

  I'm building/configured qemu-4.2.0 on an x86_64 (gcc (Debian
  6.3.0-18+deb9u1) 6.3.0 20170516) with target-list "arm-softmmu,arm-
  linux-user" and debug enabled. I use the arm-linux-user variant,
  "qemu-arm".

  Then i'm trying to cross-compile (arm gcc) an old version of googles
  v8 (as i need this version of the lib for binary compatibility) which
  uses qemu during build.

  It worked with gcc 5.4.0 but not with 9.2.0. I also tried with 6.5.0,
  7.4.0 and 8.3.0 but those are also causing the same segmentation
  fault.

  The executed command wich breaks qemu is:

   qemu-arm /tmp/build/out/arm.release/mksnapshot.arm --log-snapshot-
  positions --logfile
  /tmp/build/out/arm.release/obj.host/v8_snapshot/geni/snapshot.log
  --random-seed 314159265 /tmp/build/out/arm.release/obj.host/v8_snap

  The printed error message is:

  ARMv7=1 VFP3=1 VFP32DREGS=1 NEON=0 SUDIV=0 UNALIGNED_ACCESSES=1 
MOVW_MOVT_IMMEDIATE_LOADS=0 USE_EABI_HARDFLOAT=1
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped

  Calling qemu with gdb gives the following information:

   Thread 1 "qemu-arm" received signal SIGSEGV, Segmentation fault.
   0x55d63d11 in static_code_gen_buffer ()

  and

   (gdb) bt
   #0  0x55d63d11 in static_code_gen_buffer ()
   #1  0x55628d58 in cpu_tb_exec (itb=, 
cpu=0x57c33930) at 
   /tmp/build/qemu/accel/tcg/cpu-exec.c:172
   #2  cpu_loop_exec_tb (tb_exit=, last_tb=, tb=, 
   cpu=0x57c33930) at /tmp/build/qemu/accel/tcg/cpu-exec.c:618
   #3  cpu_exec (cpu=cpu@entry=0x57c2b660) at 
/tmp/build/qemu/accel/tcg/cpu-exec.c:731
   #4  0x55661578 in cpu_loop (env=0x57c33930) at 
/tmp/build/qemu/linux-user/arm/cpu_loop.c:219
  #5  0x555d6d76 in main (argc=, argv=, 
envp=) at /tmp/build/qemu/linux-user/main.c:865

  Calling qemu-arm with debug switch "-d in_asm,int,op_opt" shows the
  log in the attached file.

  Thanks for any hints!
  Fabian

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

Re: [PATCH v4 0/7] hw/arm/raspi: Run U-Boot on the raspi machines

[Philippe Mathieu-Daudé]

Hi Peter,

(Cc'ed Wainer from the Python part).

On 1/21/20 12:51 AM, Philippe Mathieu-Daudé wrote:

Following Laurent report:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg639950.html

The SYS_timer is already merged, see:
https://git.qemu.org/?p=qemu.git;a=commit;h=d05be883fc
"hw/timer/bcm2835: Add the BCM2835 SYS_timer"

The first patch should fix Laurent other issue.
Then few python patches are require to break into U-Boot console,
and the last patches add U-Boot tests for Raspi2 and Raspi3.

Laurent, if you successfully test U-Boot with this patchset again,
do you mind replying with a "Tested-by:" tag?

Regards,

Phil.

Since v3:
- rewrote '-smp' fix.
- tests use Debian 'trustable' u-boot.elf

previous feedbacks from Peter on v3:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg655415.html

v3: https://www.mail-archive.com/qemu-devel@nongnu.org/msg653807.html
Supersedes: <20191019234715.25750-1-f4...@amsat.org>

Philippe Mathieu-Daudé (7):
   hw/arm/raspi: Remove obsolete use of -smp to set the soc 'enabled-cpus'


While the first patch is reviewed by Alistair, the rest (acceptance 
tests) still requires an eye from Cleber/Eduardo.


Can you queue the first patch via your qemu-arm tree?

Thanks,

Phil.


   Acceptance tests: Extract _console_interaction()
   Acceptance tests: Add interrupt_interactive_console_until_pattern()
   python/qemu/machine: Allow to use other serial consoles than default
   tests/boot_linux_console: Test booting U-Boot on the Raspberry Pi 2
   tests/boot_linux_console: Test booting U-Boot on the Raspberry Pi 3
   tests/boot_linux_console: Tag Emcraft Smartfusion2 as running 'u-boot'

  hw/arm/raspi.c|  2 -
  python/qemu/machine.py|  9 +++-
  tests/acceptance/avocado_qemu/__init__.py | 59 +--
  tests/acceptance/boot_linux_console.py| 54 +
  4 files changed, 107 insertions(+), 17 deletions(-)

Re: [RFC PATCH] hw/arm/virt: Support NMI injection

[Gavin Shan]

[including more folks into the discussion]


On Fri, 17 Jan 2020 at 14:00, Peter Maydell  wrote:

On Thu, 19 Dec 2019 at 04:06, Gavin Shan  wrote:

This supports NMI injection for virtual machine and currently it's only
supported on GICv3 controller, which is emulated by qemu or host kernel.
The design is highlighted as below:

* The NMI is identified by its priority (0x20). In the guest (linux)
kernel, the GICC_PMR is set to 0x80, to block all interrupts except
the NMIs when the external interrupt is disabled. It means the FIQ
and IRQ bit in PSTATE isn't touched when the functionality (NMI) is
functional.
* LPIs aren't considered as NMIs because of their nature. It means NMI
is either SPI or PPI. Besides, the NMIs are injected in round-robin
fashion is there are multiple NMIs existing.
* When the GICv3 controller is emulated by qemu, the interrupt states
(e.g. enabled, priority) is fetched from the corresponding data struct
directly. However, we have to pause all CPUs to fetch the interrupt
states from host in advance if the GICv3 controller is emulated by
host.

The testing scenario is to tweak guest (linux) kernel where the pl011 SPI
can be enabled as NMI by request_nmi(). Check "/proc/interrupts" after injecting
several NMIs, to see if the interrupt count is increased or not. The result
is just as expected.



So, QEMU is trying to emulate actual hardware. None of this
looks to me like what GICv3 hardware does... If you want to
have the virt board send an interrupt, do it the usual way
by wiring up a qemu_irq from some device to the GIC, please.
(More generally, there is no concept of an "NMI" in the GIC;
there are just interrupts at varying possible guest-programmable
priority levels.)



Peter, I missed to read your reply in time and apologies for late response.

Yes, there is no concept of "NMI" in the GIC from hardware perspective.
However, NMI has been supported from the software by kernel commit
bc3c03ccb4641 ("arm64: Enable the support of pseudo-NMIs"). The NMIs
have higher priority than normal ones. NMIs are deliverable after
local_irq_disable() because the SYS_ICC_PMR_EL1 is tweaked so that
normal interrupts are masked only.

It's unclear about the purpose of "nmi" QMP/HMP command. It's why I
put a RFC tag. The command has been supported by multiple architects
including x86/ppc. However, they are having different behaviors. The
system will be restarted on ppc with this command, but a NMI is injected
through LAPIC on x86. So I'm not sure what architect (system reset on
ppc or injecting NMI on x86) aarch64 should follow.

Thanks,
Gavin

[PATCH v2 1/3] Fixed assert in vhost_user_set_mem_table_postcopy

[Raphael Norwitz]

The current vhost_user_set_mem_table_postcopy() implementation
populates each region of the VHOST_USER_SET_MEM_TABLE message without
first checking if there are more than VHOST_MEMORY_MAX_NREGIONS already
populated. This can cause memory corruption if too many regions are
added to the message during the postcopy step.

This change moves an existing assert up such that attempting to
construct a VHOST_USER_SET_MEM_TABLE message with too many memory
regions will gracefully bring down qemu instead of corrupting memory.

Signed-off-by: Raphael Norwitz 
Signed-off-by: Peter Turschmid 
---
 hw/virtio/vhost-user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 2e81f55..cce851a 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -443,6 +443,7 @@ static int vhost_user_set_mem_table_postcopy(struct 
vhost_dev *dev,
  &offset);
 fd = memory_region_get_fd(mr);
 if (fd > 0) {
+assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
 trace_vhost_user_set_mem_table_withfd(fd_num, mr->name,
   reg->memory_size,
   reg->guest_phys_addr,
@@ -455,7 +456,6 @@ static int vhost_user_set_mem_table_postcopy(struct 
vhost_dev *dev,
 msg.payload.memory.regions[fd_num].guest_phys_addr =
 reg->guest_phys_addr;
 msg.payload.memory.regions[fd_num].mmap_offset = offset;
-assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
 fds[fd_num++] = fd;
 } else {
 u->region_rb_offset[i] = 0;
-- 
1.8.3.1

[PATCH v2 2/3] Refactor vhost_user_set_mem_table functions

[Raphael Norwitz]

vhost_user_set_mem_table() and vhost_user_set_mem_table_postcopy() have
gotten convoluted, and have some identical code.

This change moves the logic populating the VhostUserMemory struct and
fds array from vhost_user_set_mem_table() and
vhost_user_set_mem_table_postcopy() to a new function,
vhost_user_fill_set_mem_table_msg().

No functionality is impacted.

Signed-off-by: Raphael Norwitz 
Signed-off-by: Peter Turschmid 
---
 hw/virtio/vhost-user.c | 143 +++--
 1 file changed, 67 insertions(+), 76 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cce851a..af83fdd 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -407,18 +407,79 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, 
uint64_t base,
 return 0;
 }
 
+static int vhost_user_fill_set_mem_table_msg(struct vhost_user *u,
+ struct vhost_dev *dev,
+ VhostUserMsg *msg,
+ int *fds, size_t *fd_num,
+ bool postcopy)
+{
+int i, fd;
+ram_addr_t offset;
+MemoryRegion *mr;
+struct vhost_memory_region *reg;
+
+msg->hdr.request = VHOST_USER_SET_MEM_TABLE;
+
+for (i = 0; i < dev->mem->nregions; ++i) {
+reg = dev->mem->regions + i;
+
+assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
+mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
+ &offset);
+fd = memory_region_get_fd(mr);
+if (fd > 0) {
+if (postcopy) {
+assert(*fd_num < VHOST_MEMORY_MAX_NREGIONS);
+trace_vhost_user_set_mem_table_withfd(*fd_num, mr->name,
+  reg->memory_size,
+  reg->guest_phys_addr,
+  reg->userspace_addr,
+  offset);
+u->region_rb_offset[i] = offset;
+u->region_rb[i] = mr->ram_block;
+} else if (*fd_num == VHOST_MEMORY_MAX_NREGIONS) {
+error_report("Failed preparing vhost-user memory table msg");
+return -1;
+}
+msg->payload.memory.regions[*fd_num].userspace_addr =
+reg->userspace_addr;
+msg->payload.memory.regions[*fd_num].memory_size =
+reg->memory_size;
+msg->payload.memory.regions[*fd_num].guest_phys_addr =
+reg->guest_phys_addr;
+msg->payload.memory.regions[*fd_num].mmap_offset = offset;
+fds[(*fd_num)++] = fd;
+} else if (postcopy) {
+u->region_rb_offset[i] = 0;
+u->region_rb[i] = NULL;
+}
+}
+
+msg->payload.memory.nregions = *fd_num;
+
+if (!*fd_num) {
+error_report("Failed initializing vhost-user memory map, "
+ "consider using -object memory-backend-file share=on");
+return -1;
+}
+
+msg->hdr.size = sizeof(msg->payload.memory.nregions);
+msg->hdr.size += sizeof(msg->payload.memory.padding);
+msg->hdr.size += *fd_num * sizeof(VhostUserMemoryRegion);
+
+return 1;
+}
+
 static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev,
  struct vhost_memory *mem)
 {
 struct vhost_user *u = dev->opaque;
 int fds[VHOST_MEMORY_MAX_NREGIONS];
-int i, fd;
 size_t fd_num = 0;
 VhostUserMsg msg_reply;
 int region_i, msg_i;
 
 VhostUserMsg msg = {
-.hdr.request = VHOST_USER_SET_MEM_TABLE,
 .hdr.flags = VHOST_USER_VERSION,
 };
 
@@ -433,48 +494,11 @@ static int vhost_user_set_mem_table_postcopy(struct 
vhost_dev *dev,
 u->region_rb_len = dev->mem->nregions;
 }
 
-for (i = 0; i < dev->mem->nregions; ++i) {
-struct vhost_memory_region *reg = dev->mem->regions + i;
-ram_addr_t offset;
-MemoryRegion *mr;
-
-assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
-mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
- &offset);
-fd = memory_region_get_fd(mr);
-if (fd > 0) {
-assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
-trace_vhost_user_set_mem_table_withfd(fd_num, mr->name,
-  reg->memory_size,
-  reg->guest_phys_addr,
-  reg->userspace_addr, offset);
-u->region_rb_offset[i] = offset;
-u->region_rb[i] = mr->ram_block;
-msg.payload.memory.regions[fd_num].userspace_addr =
-reg->userspace_addr;
-msg.pa

[PATCH v2 0/3] vhost-user: Lift Max Ram Slots Limitation

[Raphael Norwitz]

In QEMU today, a VM with a vhost-user device can hot add memory a
maximum of 8 times. See these threads, among others:

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01046.html
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01236.html

[2] https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg04656.html

This series introduces a new protocol feature
VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS which, when enabled, lifts the
restriction on the maximum number RAM slots imposed by vhost-user.

The patch consists of 3 changes:
1. Fixed assert in vhost_user_set_mem_table_postcopy:
   This is a bug fix in the postcopy migration path
2. Refactor vhost_user_set_mem_table functions:
   This is a non-functional change refractoring the
   vhost_user_set_mem_table and vhost_user_set_mem_table_postcopy
   functions such that the feature can be more cleanly added.
3. Lift max memory slots limit imposed by vhost-user:
   This change introduces the new protocol feature
   VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS.

The implementation details are explained in more detail in the commit
messages, but at a high level the new protocol feature works as follows:
- If the VHOST_USER_PROTCOL_F_CONFIGURE_SLOTS feature is enabled, QEMU will
  send multiple VHOST_USER_ADD_MEM_REG and VHOST_USER_REM_MEM_REG
  messages to map and unmap individual memory regions instead of one large
  VHOST_USER_SET_MEM_TABLE message containing all memory regions.
- The vhost-user struct maintains a ’shadow state’ of memory regions
  already sent to the guest. Each time vhost_user_set_mem_table is called,
  the shadow state is compared with the new device state. A
  VHOST_USER_REM_MEM_REG will be sent for each region in the shadow state
  not in the device state. Then, a VHOST_USER_ADD_MEM_REG will be sent
  for each region in the device state but not the shadow state. After
  these messages have been sent, the shadow state will be updated to
  reflect the new device state.

The VHOST_USER_SET_MEM_TABLE message was not reused because as the number of
regions grows, the message becomes very large. In practice, such large
messages caused problems (truncated messages) and in the past it seems the
community has opted for smaller fixed size messages where possible. VRINGs,
for example, are sent to the backend individually instead of in one massive
message.

Current Limitations:
- postcopy migration is not supported when the
  VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS has been negotiated. 
- VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS cannot be negotiated when
  VHOST_USER_PROTOCOL_F_REPLY_ACK has also been negotiated.

Both of these limitations are due to resource contraints. They are not
imposed for technical reasons.

Changes since V1:
* Kept the assert in vhost_user_set_mem_table_postcopy, but moved it
  to prevent corruption
* Made QEMU send a single VHOST_USER_GET_MAX_MEMSLOTS message at
  startup and cache the returned value so that QEMU does not need to
  query the backend every time vhost_backend_memslots_limit is called.

Best,
Raphael

Raphael Norwitz (3):
  Fixed assert in vhost_user_set_mem_table_postcopy
  Refactor vhost_user_set_mem_table functions
  Lift max memory slots limit imposed by vhost-user

 docs/interop/vhost-user.rst |  43 +
 hw/virtio/vhost-user.c  | 385 +---
 2 files changed, 336 insertions(+), 92 deletions(-)

-- 
1.8.3.1

[PATCH v2 3/3] Lift max memory slots limit imposed by vhost-user

[Raphael Norwitz]

The current vhost-user implementation in Qemu imposes a limit on the
maximum number of memory slots exposed to a VM using a vhost-user
device. This change provides a new protocol feature
VHOST_USER_F_CONFIGURE_SLOTS which, when enabled, lifts this limit and
allows a VM with a vhost-user device to expose a configurable number of
memory slots, up to the ACPI defined maximum. Existing backends which
do not support this protocol feature are unaffected.

This feature works by using three new messages,
VHOST_USER_GET_MAX_MEM_SLOTS, VHOST_USER_ADD_MEM_REG and
VHOST_USER_REM_MEM_REG. VHOST_USER_GET_MAX_MEM_SLOTS gets the
number of memory slots the backend is willing to accept when the
backend is initialized. Then, when the memory tables are set or updated,
a series of VHOST_USER_ADD_MEM_REG and VHOST_USER_REM_MEM_REG messages
are sent to transmit the regions to map and/or unmap instead of trying
to send all the regions in one fixed size VHOST_USER_SET_MEM_TABLE
message.

The vhost_user struct maintains a shadow state of the VM’s memory
regions. When the memory tables are modified, the
vhost_user_set_mem_table() function compares the new device memory state
to the shadow state and only sends regions which need to be unmapped or
mapped in. The regions which must be unmapped are sent first, followed
by the new regions to be mapped in. After all the messages have been
sent, the shadow state is set to the current virtual device state.

The current feature implementation does not work with postcopy migration
and cannot be enabled if the VHOST_USER_PROTOCOL_F_REPLY_ACK feature has
also been negotiated.

Signed-off-by: Raphael Norwitz 
Signed-off-by: Peter Turschmid 
Suggested-by: Mike Cui 
---
 docs/interop/vhost-user.rst |  43 
 hw/virtio/vhost-user.c  | 254 
 2 files changed, 275 insertions(+), 22 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 5f8b3a4..ae9acf2 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -786,6 +786,7 @@ Protocol features
   #define VHOST_USER_PROTOCOL_F_HOST_NOTIFIER  11
   #define VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD 12
   #define VHOST_USER_PROTOCOL_F_RESET_DEVICE   13
+  #define VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS   14
 
 Master message types
 
@@ -1205,6 +1206,48 @@ Master message types
   Only valid if the ``VHOST_USER_PROTOCOL_F_RESET_DEVICE`` protocol
   feature is set by the backend.
 
+``VHOST_USER_GET_MAX_MEM_SLOTS``
+  :id: 35
+  :equivalent ioctl: N/A
+  :slave payload: u64
+
+  When the VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS protocol feature has been
+  successfully negotiated, this message is submitted by master to the
+  slave. The slave should return the message with a u64 payload
+  containing the maximum number of memory slots for QEMU to expose to
+  the guest. This message is not supported with postcopy migration or if
+  the VHOST_USER_PROTOCOL_F_REPLY_ACK feature has also been negotiated.
+
+``VHOST_USER_ADD_MEM_REG``
+  :id: 36
+  :equivalent ioctl: N/A
+  :slave payload: memory region
+
+  When the VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS protocol feature has been
+  successfully negotiated, this message is submitted by master to the slave.
+  The message payload contains a memory region descriptor struct, describing
+  a region of guest memory which the slave device must map in. When the
+  VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS protocol feature has been successfully
+  negotiated, along with the VHOST_USER_REM_MEM_REG message, this message is
+  used to set and update the memory tables of the slave device. This message
+  is not supported with postcopy migration or if the
+  VHOST_USER_PROTOCOL_F_REPLY_ACK feature has also been negotiated.
+
+``VHOST_USER_REM_MEM_REG``
+  :id: 37
+  :equivalent ioctl: N/A
+  :slave payload: memory region
+
+  When the VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS protocol feature has been
+  successfully negotiated, this message is submitted by master to the slave.
+  The message payload contains a memory region descriptor struct, describing
+  a region of guest memory which the slave device must unmap. When the
+  VHOST_USER_PROTOCOL_F_CONFIGURE_SLOTS protocol feature has been successfully
+  negotiated, along with the VHOST_USER_ADD_MEM_REG message, this message is
+  used to set and update the memory tables of the slave device. This message
+  is not supported with postcopy migration or if the
+  VHOST_USER_PROTOCOL_F_REPLY_ACK feature has also been negotiated.
+
 Slave message types
 ---
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index af83fdd..fed6d02 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -35,11 +35,29 @@
 #include 
 #endif
 
-#define VHOST_MEMORY_MAX_NREGIONS8
+#define VHOST_MEMORY_LEGACY_NREGIONS8
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 #define VHOST_USER_SLAVE_MAX_FDS 8
 
 /*
+ * Set maximum number of RAM slots sup

[PULL 0/2] Ide patches

[John Snow]

The following changes since commit 105b07f1ba462ec48b27e5cb74ddf81c6a79364c:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200127' into 
staging (2020-01-27 13:02:36 +)

are available in the Git repository at:

  https://github.com/jnsnow/qemu.git tags/ide-pull-request

for you to fetch changes up to 59805ae92dfe4f67105e36b539d567caec4f8304:

  tests/ide-test: Create a single unit-test covering more PRDT cases 
(2020-01-27 17:07:31 -0500)


Pull request



Alexander Popov (2):
  ide: Fix incorrect handling of some PRDTs in ide_dma_cb()
  tests/ide-test: Create a single unit-test covering more PRDT cases

 hw/ide/core.c  |  30 +--
 tests/qtest/ide-test.c | 176 ++---
 2 files changed, 97 insertions(+), 109 deletions(-)

-- 
2.21.0

[PULL 2/2] tests/ide-test: Create a single unit-test covering more PRDT cases

[John Snow]

From: Alexander Popov 

Fuzzing the Linux kernel with syzkaller allowed to find how to crash qemu
using a special SCSI_IOCTL_SEND_COMMAND. It hits the assertion in
ide_dma_cb() introduced in the commit a718978ed58a in July 2015.
Currently this bug is not reproduced by the unit tests.

Let's improve the ide-test to cover more PRDT cases including one
that causes this particular qemu crash.

The test is developed according to the Programming Interface for
Bus Master IDE Controller (Revision 1.0 5/16/94).

Signed-off-by: Alexander Popov 
Message-id: 20191223175117.508990-3-alex.po...@linux.com
Signed-off-by: John Snow 
---
 tests/qtest/ide-test.c | 176 ++---
 1 file changed, 75 insertions(+), 101 deletions(-)

diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 0277e7d5a9..5cfd97f915 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -445,104 +445,81 @@ static void test_bmdma_trim(void)
 test_bmdma_teardown(qts);
 }
 
-static void test_bmdma_short_prdt(void)
+/*
+ * This test is developed according to the Programming Interface for
+ * Bus Master IDE Controller (Revision 1.0 5/16/94)
+ */
+static void test_bmdma_various_prdts(void)
 {
-QTestState *qts;
-QPCIDevice *dev;
-QPCIBar bmdma_bar, ide_bar;
-uint8_t status;
-
-PrdtEntry prdt[] = {
-{
-.addr = 0,
-.size = cpu_to_le32(0x10 | PRDT_EOT),
-},
-};
-
-qts = test_bmdma_setup();
-
-dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
-
-/* Normal request */
-status = send_dma_request(qts, CMD_READ_DMA, 0, 1,
-  prdt, ARRAY_SIZE(prdt), NULL);
-g_assert_cmphex(status, ==, 0);
-assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-
-/* Abort the request before it completes */
-status = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 1,
-  prdt, ARRAY_SIZE(prdt), NULL);
-g_assert_cmphex(status, ==, 0);
-assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-free_pci_device(dev);
-test_bmdma_teardown(qts);
-}
-
-static void test_bmdma_one_sector_short_prdt(void)
-{
-QTestState *qts;
-QPCIDevice *dev;
-QPCIBar bmdma_bar, ide_bar;
-uint8_t status;
-
-/* Read 2 sectors but only give 1 sector in PRDT */
-PrdtEntry prdt[] = {
-{
-.addr = 0,
-.size = cpu_to_le32(0x200 | PRDT_EOT),
-},
-};
-
-qts = test_bmdma_setup();
-
-dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
-
-/* Normal request */
-status = send_dma_request(qts, CMD_READ_DMA, 0, 2,
-  prdt, ARRAY_SIZE(prdt), NULL);
-g_assert_cmphex(status, ==, 0);
-assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-
-/* Abort the request before it completes */
-status = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 2,
-  prdt, ARRAY_SIZE(prdt), NULL);
-g_assert_cmphex(status, ==, 0);
-assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-free_pci_device(dev);
-test_bmdma_teardown(qts);
-}
-
-static void test_bmdma_long_prdt(void)
-{
-QTestState *qts;
-QPCIDevice *dev;
-QPCIBar bmdma_bar, ide_bar;
-uint8_t status;
-
-PrdtEntry prdt[] = {
-{
-.addr = 0,
-.size = cpu_to_le32(0x1000 | PRDT_EOT),
-},
-};
-
-qts = test_bmdma_setup();
-
-dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
-
-/* Normal request */
-status = send_dma_request(qts, CMD_READ_DMA, 0, 1,
-  prdt, ARRAY_SIZE(prdt), NULL);
-g_assert_cmphex(status, ==, BM_STS_ACTIVE | BM_STS_INTR);
-assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-
-/* Abort the request before it completes */
-status = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 1,
-  prdt, ARRAY_SIZE(prdt), NULL);
-g_assert_cmphex(status, ==, BM_STS_INTR);
-assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-free_pci_device(dev);
-test_bmdma_teardown(qts);
+int sectors = 0;
+uint32_t size = 0;
+
+for (sectors = 1; sectors <= 256; sectors *= 2) {
+QTestState *qts = NULL;
+QPCIDevice *dev = NULL;
+QPCIBar bmdma_bar, ide_bar;
+
+qts = test_bmdma_setup();
+dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
+
+for (size = 0; size < 65536; size += 256) {
+uint32_t req_size = sectors * 512;
+uint32_t prd_size = size & 0xfffe; /* bit 0 is always set to 0 */
+uint8_t ret = 0;
+uint8_t req_status = 0;
+uint8_t abort_req_status = 0;
+PrdtEntry prdt[] = {
+{
+.addr = 0,
+.size = cpu_to_le32(size | PRDT_EOT),
+},
+};
+
+   

[PULL 1/2] ide: Fix incorrect handling of some PRDTs in ide_dma_cb()

[John Snow]

From: Alexander Popov 

The commit a718978ed58a from July 2015 introduced the assertion which
implies that the size of successful DMA transfers handled in ide_dma_cb()
should be multiple of 512 (the size of a sector). But guest systems can
initiate DMA transfers that don't fit this requirement.

For fixing that let's check the number of bytes prepared for the transfer
by the prepare_buf() handler. The code in ide_dma_cb() must behave
according to the Programming Interface for Bus Master IDE Controller
(Revision 1.0 5/16/94):
1. If PRDs specified a smaller size than the IDE transfer
   size, then the Interrupt and Active bits in the Controller
   status register are not set (Error Condition).
2. If the size of the physical memory regions was equal to
   the IDE device transfer size, the Interrupt bit in the
   Controller status register is set to 1, Active bit is set to 0.
3. If PRDs specified a larger size than the IDE transfer size,
   the Interrupt and Active bits in the Controller status register
   are both set to 1.

Signed-off-by: Alexander Popov 
Reviewed-by: Kevin Wolf 
Message-id: 20191223175117.508990-2-alex.po...@linux.com
Signed-off-by: John Snow 
---
 hw/ide/core.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 754ff4dc34..8eb766 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -849,6 +849,7 @@ static void ide_dma_cb(void *opaque, int ret)
 int64_t sector_num;
 uint64_t offset;
 bool stay_active = false;
+int32_t prep_size = 0;
 
 if (ret == -EINVAL) {
 ide_dma_error(s);
@@ -863,13 +864,15 @@ static void ide_dma_cb(void *opaque, int ret)
 }
 }
 
-n = s->io_buffer_size >> 9;
-if (n > s->nsector) {
-/* The PRDs were longer than needed for this request. Shorten them so
- * we don't get a negative remainder. The Active bit must remain set
- * after the request completes. */
+if (s->io_buffer_size > s->nsector * 512) {
+/*
+ * The PRDs were longer than needed for this request.
+ * The Active bit must remain set after the request completes.
+ */
 n = s->nsector;
 stay_active = true;
+} else {
+n = s->io_buffer_size >> 9;
 }
 
 sector_num = ide_get_sector(s);
@@ -892,9 +895,20 @@ static void ide_dma_cb(void *opaque, int ret)
 n = s->nsector;
 s->io_buffer_index = 0;
 s->io_buffer_size = n * 512;
-if (s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size) < 512) {
-/* The PRDs were too short. Reset the Active bit, but don't raise an
- * interrupt. */
+prep_size = s->bus->dma->ops->prepare_buf(s->bus->dma, s->io_buffer_size);
+/* prepare_buf() must succeed and respect the limit */
+assert(prep_size >= 0 && prep_size <= n * 512);
+
+/*
+ * Now prep_size stores the number of bytes in the sglist, and
+ * s->io_buffer_size stores the number of bytes described by the PRDs.
+ */
+
+if (prep_size < n * 512) {
+/*
+ * The PRDs are too short for this request. Error condition!
+ * Reset the Active bit and don't raise the interrupt.
+ */
 s->status = READY_STAT | SEEK_STAT;
 dma_buf_commit(s, 0);
 goto eot;
-- 
2.21.0

Re: [PATCH v2] riscv: Add helper to make NaN-boxing for FP register

[Alistair Francis]

On Mon, Jan 27, 2020 at 4:37 PM Ian Jiang  wrote:
>
> The function that makes NaN-boxing when a 32-bit value is assigned
> to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
> Then it is applied in translating of the FLW instruction.
>
> Signed-off-by: Ian Jiang 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvf.inc.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvf.inc.c 
> b/target/riscv/insn_trans/trans_rvf.inc.c
> index e23cd639a6..3bfd8881e7 100644
> --- a/target/riscv/insn_trans/trans_rvf.inc.c
> +++ b/target/riscv/insn_trans/trans_rvf.inc.c
> @@ -23,6 +23,20 @@
>  return false;   \
>  } while (0)
>
> +/*
> + * RISC-V requires NaN-boxing of narrower width floating
> + * point values.  This applies when a 32-bit value is
> + * assigned to a 64-bit FP register.  Thus this does not
> + * apply when the RVD extension is not present.
> + */
> +static void gen_nanbox_fpr(DisasContext *ctx, int regno)
> +{
> +if (has_ext(ctx, RVD)) {
> +tcg_gen_ori_i64(cpu_fpr[regno], cpu_fpr[regno],
> +MAKE_64BIT_MASK(32, 32));
> +}
> +}
> +
>  static bool trans_flw(DisasContext *ctx, arg_flw *a)
>  {
>  TCGv t0 = tcg_temp_new();
> @@ -32,8 +46,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
>  tcg_gen_addi_tl(t0, t0, a->imm);
>
>  tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL);
> -/* RISC-V requires NaN-boxing of narrower width floating point values */
> -tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd], 0xULL);
> +gen_nanbox_fpr(ctx, a->rd);
>
>  tcg_temp_free(t0);
>  mark_fs_dirty(ctx);
> --
> 2.17.1
>
>

Re: [PATCH] riscv: Add helper to make NaN-boxing for FP register

[Ian Jiang]

The patch description is modified and a new version is committed.
--
Ian Jiang

Richard Henderson  于2020年1月28日周二 上午1:38写道：
>
> On 1/27/20 6:10 AM, Ian Jiang wrote:
> > The function that makes NaN-boxing when a 32-bit value is assigned
> > to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
> > Then it is applied in translating of the FLW instruction.
> >
> > This also applies for other instructions when the RVD extension is
> > present, such as FMV.W.W, FADD.S, FSUB.S and so on.
>
> I wouldn't mention this yet, as it begs the question of what you are doing
> about it.  Which is nothing, yet, since that will apply to a follow-up patch.
>
>
> >
> > Signed-off-by: Ian Jiang 
> > ---
> >  target/riscv/insn_trans/trans_rvf.inc.c | 17 +++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
>
> Reviewed-by: Richard Henderson 
>
>
> r~

Re: Making QEMU easier for management tools and applications

[John Snow]

On 1/27/20 5:38 PM, Paolo Bonzini wrote:
> 
> 
> Il lun 27 gen 2020, 21:11 John Snow  > ha scritto:
> 
> 
> > ./qemu-core < {
>     "machine": "Q35",
>     "memory": "2GiB",
>     "accel": "kvm"
> }
> EOF
> 
> 
> And now you have to keep all the syntactic sugar that is in vl.c. I

Didn't mean to suggest literal values, I just grabbed some from the top
of my head. Let's pretend I didn't use sugared ones. This is a
distraction from the point.

The point is: you can specify JSON on the command line, as stdin to QEMU
as a here document. No config file needed.

> don't think a redesign of -readconfig should accept anything less
> verbose than
> 
> - machine:
>     type: q35
>     ram:
>        type: memory-backend-hostmem
>        size: 2G
> - accel:
>   - type: kvm
> 
> And this is not even taking into account disks.
> 

Though, point taken, even minimal configurations might get too cumbersome.

> I like the idea of refactoring all the syntactic sugar into a pre-pass
> on command line options. This is not an entirely new idea,
> see https://www.mail-archive.com/qemu-devel@nongnu.org/msg35024.html.

This could be very good, as long as the transformation was knowable in
some way. (Documented, modeled in the schema somehow, observable
post-transformation, whichever.)

(wow, 2010.)

> 
> I am afraid that this thread is derailing a bit, with lots of pipe
> dreams and no actionable items. How do we get it back in track?
> 

That depends on what your opinion of "on track" is. If there are
specific angles you find interesting, it would be easiest to start a
thread around the framing you find productive.

--js

[PATCH v2] riscv: Add helper to make NaN-boxing for FP register

[Ian Jiang]

The function that makes NaN-boxing when a 32-bit value is assigned
to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
Then it is applied in translating of the FLW instruction.

Signed-off-by: Ian Jiang 
---
 target/riscv/insn_trans/trans_rvf.inc.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvf.inc.c 
b/target/riscv/insn_trans/trans_rvf.inc.c
index e23cd639a6..3bfd8881e7 100644
--- a/target/riscv/insn_trans/trans_rvf.inc.c
+++ b/target/riscv/insn_trans/trans_rvf.inc.c
@@ -23,6 +23,20 @@
 return false;   \
 } while (0)
 
+/*
+ * RISC-V requires NaN-boxing of narrower width floating
+ * point values.  This applies when a 32-bit value is
+ * assigned to a 64-bit FP register.  Thus this does not
+ * apply when the RVD extension is not present.
+ */
+static void gen_nanbox_fpr(DisasContext *ctx, int regno)
+{
+if (has_ext(ctx, RVD)) {
+tcg_gen_ori_i64(cpu_fpr[regno], cpu_fpr[regno],
+MAKE_64BIT_MASK(32, 32));
+}
+}
+
 static bool trans_flw(DisasContext *ctx, arg_flw *a)
 {
 TCGv t0 = tcg_temp_new();
@@ -32,8 +46,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
 tcg_gen_addi_tl(t0, t0, a->imm);
 
 tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL);
-/* RISC-V requires NaN-boxing of narrower width floating point values */
-tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd], 0xULL);
+gen_nanbox_fpr(ctx, a->rd);
 
 tcg_temp_free(t0);
 mark_fs_dirty(ctx);
-- 
2.17.1

Re: [PATCH v3 00/14] Fixes for DP8393X SONIC device emulation

[Finn Thain]

On Mon, 20 Jan 2020, Finn Thain wrote:

> Hi All,
> 
> There are bugs in the emulated dp8393x device that can stop packet
> reception in a Linux/m68k guest (q800 machine).
> 
> With a Linux/mips guest (magnum machine), the driver fails to probe
> the dp8393x device.
> 
> With a NetBSD/arc 5.1 guest (magnum machine), the bugs in the device
> can be fatal to the guest kernel.
> 
> Whilst debugging the device, I found that the receiver algorithm
> differs from the one described in the National Semiconductor
> datasheet.
> 
> This patch series resolves these bugs.
> 

Ironically, given the recent driver fixes in the Linux kernel, the bugs in 
stock QEMU make it possible to remotely trigger a kernel Oops (much like 
the NetBSD crash). This patch series is needed to resolve those issues.

> Note that the mainline Linux sonic driver also has bugs.
> I have fixed the bugs that I know of in a series of patches at,
> https://github.com/fthain/linux/commits/mac68k
> ---
> Changed since v1:
>  - Minor revisions as described in commit logs.
>  - Dropped patches 4/10 and 7/10.
>  - Added 5 new patches.
> 
> Changed since v2:
>  - Minor revisions as described in commit logs.
>  - Dropped patch 13/13.
>  - Added 2 new patches.
> 
> 
> Finn Thain (14):
>   dp8393x: Mask EOL bit from descriptor addresses
>   dp8393x: Always use 32-bit accesses
>   dp8393x: Clean up endianness hacks
>   dp8393x: Have dp8393x_receive() return the packet size
>   dp8393x: Update LLFA and CRDA registers from rx descriptor
>   dp8393x: Clear RRRA command register bit only when appropriate
>   dp8393x: Implement packet size limit and RBAE interrupt
>   dp8393x: Don't clobber packet checksum
>   dp8393x: Use long-word-aligned RRA pointers in 32-bit mode
>   dp8393x: Pad frames to word or long word boundary
>   dp8393x: Clear descriptor in_use field to release packet
>   dp8393x: Always update RRA pointers and sequence numbers
>   dp8393x: Don't reset Silicon Revision register
>   dp8393x: Don't stop reception upon RBE interrupt assertion
> 
>  hw/net/dp8393x.c | 205 +++
>  1 file changed, 137 insertions(+), 68 deletions(-)
> 
> 

Re: [PATCH] riscv: Add helper to make NaN-boxing for FP register

[Alistair Francis]

On Mon, Jan 27, 2020 at 6:11 AM Ian Jiang  wrote:
>
> The function that makes NaN-boxing when a 32-bit value is assigned
> to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
> Then it is applied in translating of the FLW instruction.
>
> This also applies for other instructions when the RVD extension is
> present, such as FMV.W.W, FADD.S, FSUB.S and so on.
>
> Signed-off-by: Ian Jiang 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvf.inc.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvf.inc.c 
> b/target/riscv/insn_trans/trans_rvf.inc.c
> index e23cd639a6..3bfd8881e7 100644
> --- a/target/riscv/insn_trans/trans_rvf.inc.c
> +++ b/target/riscv/insn_trans/trans_rvf.inc.c
> @@ -23,6 +23,20 @@
>  return false;   \
>  } while (0)
>
> +/*
> + * RISC-V requires NaN-boxing of narrower width floating
> + * point values.  This applies when a 32-bit value is
> + * assigned to a 64-bit FP register.  Thus this does not
> + * apply when the RVD extension is not present.
> + */
> +static void gen_nanbox_fpr(DisasContext *ctx, int regno)
> +{
> +if (has_ext(ctx, RVD)) {
> +tcg_gen_ori_i64(cpu_fpr[regno], cpu_fpr[regno],
> +MAKE_64BIT_MASK(32, 32));
> +}
> +}
> +
>  static bool trans_flw(DisasContext *ctx, arg_flw *a)
>  {
>  TCGv t0 = tcg_temp_new();
> @@ -32,8 +46,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a)
>  tcg_gen_addi_tl(t0, t0, a->imm);
>
>  tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL);
> -/* RISC-V requires NaN-boxing of narrower width floating point values */
> -tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd], 0xULL);
> +gen_nanbox_fpr(ctx, a->rd);
>
>  tcg_temp_free(t0);
>  mark_fs_dirty(ctx);
> --
> 2.17.1
>
>

Re: [PATCH v3 00/14] Fixes for DP8393X SONIC device emulation

[Laurent Vivier]

Le 19/01/2020 à 23:59, Finn Thain a écrit :
> Hi All,
> 
> There are bugs in the emulated dp8393x device that can stop packet
> reception in a Linux/m68k guest (q800 machine).
> 
> With a Linux/mips guest (magnum machine), the driver fails to probe
> the dp8393x device.
> 
> With a NetBSD/arc 5.1 guest (magnum machine), the bugs in the device
> can be fatal to the guest kernel.
> 
> Whilst debugging the device, I found that the receiver algorithm
> differs from the one described in the National Semiconductor
> datasheet.
> 
> This patch series resolves these bugs.
> 
> Note that the mainline Linux sonic driver also has bugs.
> I have fixed the bugs that I know of in a series of patches at,
> https://github.com/fthain/linux/commits/mac68k
> ---
> Changed since v1:
>  - Minor revisions as described in commit logs.
>  - Dropped patches 4/10 and 7/10.
>  - Added 5 new patches.
> 
> Changed since v2:
>  - Minor revisions as described in commit logs.
>  - Dropped patch 13/13.
>  - Added 2 new patches.
> 
> 
> Finn Thain (14):
>   dp8393x: Mask EOL bit from descriptor addresses
>   dp8393x: Always use 32-bit accesses
>   dp8393x: Clean up endianness hacks
>   dp8393x: Have dp8393x_receive() return the packet size
>   dp8393x: Update LLFA and CRDA registers from rx descriptor
>   dp8393x: Clear RRRA command register bit only when appropriate
>   dp8393x: Implement packet size limit and RBAE interrupt
>   dp8393x: Don't clobber packet checksum
>   dp8393x: Use long-word-aligned RRA pointers in 32-bit mode
>   dp8393x: Pad frames to word or long word boundary
>   dp8393x: Clear descriptor in_use field to release packet
>   dp8393x: Always update RRA pointers and sequence numbers
>   dp8393x: Don't reset Silicon Revision register
>   dp8393x: Don't stop reception upon RBE interrupt assertion
> 
>  hw/net/dp8393x.c | 205 +++
>  1 file changed, 137 insertions(+), 68 deletions(-)
> 

For the series with q800 machine type and kernel 5.5.0:

Tested-by: Laurent Vivier 

Re: [PATCH v4 29/40] target/arm: Flush tlb for ASID changes in EL2&0 translation regime

[Richard Henderson]

On 12/6/19 9:05 AM, Peter Maydell wrote:
> On Tue, 3 Dec 2019 at 02:30, Richard Henderson
>  wrote:
>>
>> Since we only support a single ASID, flush the tlb when it changes.
>>
>> Signed-off-by: Richard Henderson 
>> ---
>>  target/arm/helper.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 9df55a8d6b..2a4d4c2c0d 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -3740,6 +3740,15 @@ static void vmsa_ttbr_write(CPUARMState *env, const 
>> ARMCPRegInfo *ri,
>>  static void vmsa_tcr_ttbr_el2_write(CPUARMState *env, const ARMCPRegInfo 
>> *ri,
>>  uint64_t value)
>>  {
>> +/*
>> + * If we are running with E2&0 regime, then the ASID is active.
>> + * Flush if that changes.
>> + */
>> +if ((arm_hcr_el2_eff(env) & HCR_E2H) &&
>> +extract64(raw_read(env, ri) ^ value, 48, 16)) {
>> +tlb_flush_by_mmuidx(env_cpu(env),
>> +ARMMMUIdxBit_EL20_2 | ARMMMUIdxBit_EL20_0);
>> +}
>>  raw_write(env, ri, value);
>>  }
> 
> For the existing EL1 setup we have separate write functions
> for TTBR registers and for TCR_EL1 (vmsa_tcr_el1_write()
> and vmsa_ttbr_write()), rather than a single one, and they
> don't do the same thing. Why do we use a single writefn
> here? It looks particularly odd because we're actually looking
> at the value written here.

Yes, Laurent noticed the same problem wrt patch 4.
Fixed.


r~

Re: [PATCH] .travis.yml: Drop superfluous use of --python=python3 parameter

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> As we require Python3 since commit ddf9069963, we don't need to
> explicit it with the --python=/usr/bin/python3 configure option.
>
> Reported-by: Thomas Huth 
> Signed-off-by: Philippe Mathieu-Daudé 

Queued to testing/next, thanks.

> ---
> Cc: Wainer dos Santos Moschetta 
> Cc: Eduardo Habkost 
> Cc: Cleber Rosa 
> ---
>  .travis.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 6c1038a0f1..ee93180283 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -268,7 +268,7 @@ matrix:
>  
>  # Acceptance (Functional) tests
>  - env:
> -- CONFIG="--python=/usr/bin/python3 
> --target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,sparc-softmmu"
> +- 
> CONFIG="--target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,sparc-softmmu"
>  - TEST_CMD="make check-acceptance"
>after_script:
>  - python3 -c 'import json; r = 
> json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) 
> for t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat


-- 
Alex Bennée

Re: [PATCH v2] riscv/virt: Add syscon reboot and poweroff DT nodes

[Alistair Francis]

On Wed, Jan 22, 2020 at 11:18 PM Anup Patel  wrote:
>
> The SiFive test device found on virt machine can be used by
> generic syscon reboot and poweroff drivers available in Linux
> kernel.
>
> This patch updates FDT generation in virt machine so that
> Linux kernel can probe and use generic syscon drivers.
>
> Signed-off-by: Anup Patel 

Reviewed-by: Alistair Francis 

Alistair

> ---
> Changes since v1:
>  - Rebased on latest QEMU master commit 
> 3e08b2b9cb64bff2b73fa9128c0e49bfcde0dd40
> ---
>  hw/riscv/virt.c | 26 ++
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index c44b865959..6d682f8a78 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -182,11 +182,10 @@ static void create_fdt(RISCVVirtState *s, const struct 
> MemmapEntry *memmap,
>  uint64_t mem_size, const char *cmdline)
>  {
>  void *fdt;
> -int cpu;
> +int cpu, i;
>  uint32_t *cells;
>  char *nodename;
> -uint32_t plic_phandle, phandle = 1;
> -int i;
> +uint32_t plic_phandle, test_phandle, phandle = 1;
>  hwaddr flashsize = virt_memmap[VIRT_FLASH].size / 2;
>  hwaddr flashbase = virt_memmap[VIRT_FLASH].base;
>
> @@ -356,16 +355,35 @@ static void create_fdt(RISCVVirtState *s, const struct 
> MemmapEntry *memmap,
>  create_pcie_irq_map(fdt, nodename, plic_phandle);
>  g_free(nodename);
>
> +test_phandle = phandle++;
>  nodename = g_strdup_printf("/test@%lx",
>  (long)memmap[VIRT_TEST].base);
>  qemu_fdt_add_subnode(fdt, nodename);
>  {
> -const char compat[] = "sifive,test1\0sifive,test0";
> +const char compat[] = "sifive,test1\0sifive,test0\0syscon";
>  qemu_fdt_setprop(fdt, nodename, "compatible", compat, 
> sizeof(compat));
>  }
>  qemu_fdt_setprop_cells(fdt, nodename, "reg",
>  0x0, memmap[VIRT_TEST].base,
>  0x0, memmap[VIRT_TEST].size);
> +qemu_fdt_setprop_cell(fdt, nodename, "phandle", test_phandle);
> +test_phandle = qemu_fdt_get_phandle(fdt, nodename);
> +g_free(nodename);
> +
> +nodename = g_strdup_printf("/reboot");
> +qemu_fdt_add_subnode(fdt, nodename);
> +qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-reboot");
> +qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> +qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> +qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_RESET);
> +g_free(nodename);
> +
> +nodename = g_strdup_printf("/poweroff");
> +qemu_fdt_add_subnode(fdt, nodename);
> +qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
> +qemu_fdt_setprop_cell(fdt, nodename, "regmap", test_phandle);
> +qemu_fdt_setprop_cell(fdt, nodename, "offset", 0x0);
> +qemu_fdt_setprop_cell(fdt, nodename, "value", FINISHER_PASS);
>  g_free(nodename);
>
>  nodename = g_strdup_printf("/uart@%lx",
> --
> 2.17.1
>
>

Re: [PATCH v6 2/2] s390: diagnose 318 info reset and migration support

[Collin Walling]

On 1/27/20 12:35 PM, Cornelia Huck wrote:
> On Mon, 27 Jan 2020 11:39:02 -0500
> Collin Walling  wrote:
> 
>> On 1/27/20 6:47 AM, Cornelia Huck wrote:
>>> On Fri, 24 Jan 2020 17:14:04 -0500
>>> Collin Walling  wrote:
>>>   

[...]


 The availability of this instruction is determined by byte 134, bit 0
 of the Read Info block. This coincidentally expands into the space used  
>>>
>>> "SCLP Read Info"
>>>   
 for CPU entries by taking away one byte, which means VMs running with
 the diag318 capability will not be able to retrieve information regarding
 the 248th CPU. This will not effect performance, and VMs can still be
 ran with 248 CPUs.  
>>>
>>> Are there other ways in which that might affect guests? I assume Linux
>>> can deal with it? Is it ok architecture-wise?
>>>
>>> In any case, should go into the patch description :)
>>>   
>>
>> Same as above. I'll try to provide more information regarding what happens
>> here in my next reply.
> 
> I think you can lift some stuff from the cover letter.
> 

Here's what I found out:

Each CPU entry holds info regarding the CPU's address / ID as well as an 
indication of the availability of certain CPU features. With these patches,
we lose a CPU entry for one CPU (essentially what would be the CPU at the
tail-end of the list). This CPU exists, but is essentially in limbo... the
machine cannot access any information regarding it.

So, a VM can run with the original N max CPUs, but in reality we can only
utilize n-1. 

>>


[...]


-- 
Respectfully,
- Collin Walling

Re: Making QEMU easier for management tools and applications

[Paolo Bonzini]

Il lun 27 gen 2020, 21:11 John Snow  ha scritto:

>
> > ./qemu-core < {
> "machine": "Q35",
> "memory": "2GiB",
> "accel": "kvm"
> }
> EOF
>

And now you have to keep all the syntactic sugar that is in vl.c. I don't
think a redesign of -readconfig should accept anything less verbose than

- machine:
type: q35
ram:
   type: memory-backend-hostmem
   size: 2G
- accel:
  - type: kvm

And this is not even taking into account disks.

I like the idea of refactoring all the syntactic sugar into a pre-pass on
command line options. This is not an entirely new idea, see
https://www.mail-archive.com/qemu-devel@nongnu.org/msg35024.html.

I am afraid that this thread is derailing a bit, with lots of pipe dreams
and no actionable items. How do we get it back in track?

Paolo


> No file required, cooperates with readline, avoids crunchy,
> hard-to-maintain CLI syntax. Directly and easily translatable to a
> stored-file configuration. All configuration and documentation is
> centralized via QAPI.
>
> A little worse to type manually, yes. Maybe not bad /enough/ for me to
> want to rescue the CLI which prevents full QAPI-fication and a working
> configuration file.
>
> Arguably, a well documented configuration schema will be much easier to
> browse, discover, and use than a labyrinthine CLI with many stub
> definitions whose options are not exposed in the documentation.
>
> (The argument here is: It's a little harder and a little longer to type,
> but the benefits from the schema organization may improve productivity
> of using QEMU directly instead of harming it.)
>
> --js
>
>

Re: [PATCH v2 1/2] qemu-img: Add --target-is-zero to convert

[Eric Blake]

On 1/24/20 4:34 AM, David Edmondson wrote:

In many cases the target of a convert operation is a newly provisioned
target that the user knows is blank (filled with zeroes). In this
situation there is no requirement for qemu-img to wastefully zero out
the entire device.

Add a new option, --target-is-zero, allowing the user to indicate that
an existing target device is already zero filled.

Signed-off-by: David Edmondson 
---
  qemu-img-cmds.hx |  4 ++--
  qemu-img.c   | 25 ++---
  qemu-img.texi|  4 
  3 files changed, 28 insertions(+), 5 deletions(-)


I'm working up a patch series that tries to auto-set this flag without 
user interaction where possible (for example, if lseek(fd, 0, SEEK_DATA) 
returns EOF, or if fstat() reports 0 blocks allocated, or if qcow2 sees 
no L2 tables allocated, or a proposed extension to NBD passes on the 
same...).  I may rebase my series on top of your patch and tweak things 
in yours accordingly.


But as it stands, the idea makes sense to me; even if we add ways for 
some images to efficiently report initial state (and our existing 
bdrv_has_zero_init() is NOT such a method), there are enough other 
scenarios where the knob will be the only way to let qemu-img know the 
intent.




+case OPTION_TARGET_IS_ZERO:
+/*
+ * The user asserting that the target is blank has the
+ * same effect as the target driver supporting zero
+ * initialisation.


Hmm. A git grep shows that 'initialization' has 200 hits, 
'initialisation' has only 29. But I think it's a US vs. UK thing, so I 
don't care which spelling you use.




@@ -2247,6 +2256,11 @@ static int img_convert(int argc, char **argv)
  warn_report("This will become an error in future QEMU versions.");
  }
  
+if (s.has_zero_init && !skip_create) {

+error_report("--target-is-zero requires use of -n flag");
+goto fail_getopt;
+}
+


Makes sense, although we could perhaps relax it to also work even when 
the -n flag is supplied IF the destination image supports my proposal 
for a new status bit set when an image is known to be opened with all 
zero content.



  s.src_num = argc - optind - 1;
  out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL;
  
@@ -2380,6 +2394,11 @@ static int img_convert(int argc, char **argv)

  }
  s.target_has_backing = (bool) out_baseimg;
  
+if (s.has_zero_init && s.target_has_backing) {

+error_report("Cannot use --target-is-zero with a backing file");
+goto out;
+}
+


Makes sense, although we could perhaps relax it to also work even when 
there is a backing file IF the backing file supports my proposal for a 
new status bit set when an image is known to be opened with all zero 
content.


As my patch proposal is still not submitted, I'm fine if yours lands as-is:

Reviewed-by: Eric Blake 

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

[PULL 17/19] multifd: Split multifd code into its own file

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/Makefile.objs |   1 +
 migration/migration.c   |   1 +
 migration/multifd.c | 899 
 migration/multifd.h | 139 ++
 migration/ram.c | 988 +---
 migration/ram.h |   7 -
 6 files changed, 1041 insertions(+), 994 deletions(-)
 create mode 100644 migration/multifd.c
 create mode 100644 migration/multifd.h

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index a4f3bafd86..d3623d5f9b 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -7,6 +7,7 @@ common-obj-y += qemu-file-channel.o
 common-obj-y += xbzrle.o postcopy-ram.o
 common-obj-y += qjson.o
 common-obj-y += block-dirty-bitmap.o
+common-obj-y += multifd.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
 
diff --git a/migration/migration.c b/migration/migration.c
index 9b24849651..cd72bb6e9a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -53,6 +53,7 @@
 #include "monitor/monitor.h"
 #include "net/announce.h"
 #include "qemu/queue.h"
+#include "multifd.h"
 
 #define MAX_THROTTLE  (32 << 20)  /* Migration transfer speed throttling */
 
diff --git a/migration/multifd.c b/migration/multifd.c
new file mode 100644
index 00..b3e8ae9bcc
--- /dev/null
+++ b/migration/multifd.c
@@ -0,0 +1,899 @@
+/*
+ * Multifd common code
+ *
+ * Copyright (c) 2019-2020 Red Hat Inc
+ *
+ * Authors:
+ *  Juan Quintela 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/rcu.h"
+#include "exec/target_page.h"
+#include "sysemu/sysemu.h"
+#include "exec/ramblock.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "ram.h"
+#include "migration.h"
+#include "socket.h"
+#include "qemu-file.h"
+#include "trace.h"
+#include "multifd.h"
+
+/* Multiple fd's */
+
+#define MULTIFD_MAGIC 0x11223344U
+#define MULTIFD_VERSION 1
+
+typedef struct {
+uint32_t magic;
+uint32_t version;
+unsigned char uuid[16]; /* QemuUUID */
+uint8_t id;
+uint8_t unused1[7]; /* Reserved for future use */
+uint64_t unused2[4];/* Reserved for future use */
+} __attribute__((packed)) MultiFDInit_t;
+
+static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
+{
+MultiFDInit_t msg = {};
+int ret;
+
+msg.magic = cpu_to_be32(MULTIFD_MAGIC);
+msg.version = cpu_to_be32(MULTIFD_VERSION);
+msg.id = p->id;
+memcpy(msg.uuid, &qemu_uuid.data, sizeof(msg.uuid));
+
+ret = qio_channel_write_all(p->c, (char *)&msg, sizeof(msg), errp);
+if (ret != 0) {
+return -1;
+}
+return 0;
+}
+
+static int multifd_recv_initial_packet(QIOChannel *c, Error **errp)
+{
+MultiFDInit_t msg;
+int ret;
+
+ret = qio_channel_read_all(c, (char *)&msg, sizeof(msg), errp);
+if (ret != 0) {
+return -1;
+}
+
+msg.magic = be32_to_cpu(msg.magic);
+msg.version = be32_to_cpu(msg.version);
+
+if (msg.magic != MULTIFD_MAGIC) {
+error_setg(errp, "multifd: received packet magic %x "
+   "expected %x", msg.magic, MULTIFD_MAGIC);
+return -1;
+}
+
+if (msg.version != MULTIFD_VERSION) {
+error_setg(errp, "multifd: received packet version %d "
+   "expected %d", msg.version, MULTIFD_VERSION);
+return -1;
+}
+
+if (memcmp(msg.uuid, &qemu_uuid, sizeof(qemu_uuid))) {
+char *uuid = qemu_uuid_unparse_strdup(&qemu_uuid);
+char *msg_uuid = qemu_uuid_unparse_strdup((const QemuUUID *)msg.uuid);
+
+error_setg(errp, "multifd: received uuid '%s' and expected "
+   "uuid '%s' for channel %hhd", msg_uuid, uuid, msg.id);
+g_free(uuid);
+g_free(msg_uuid);
+return -1;
+}
+
+if (msg.id > migrate_multifd_channels()) {
+error_setg(errp, "multifd: received channel version %d "
+   "expected %d", msg.version, MULTIFD_VERSION);
+return -1;
+}
+
+return msg.id;
+}
+
+static MultiFDPages_t *multifd_pages_init(size_t size)
+{
+MultiFDPages_t *pages = g_new0(MultiFDPages_t, 1);
+
+pages->allocated = size;
+pages->iov = g_new0(struct iovec, size);
+pages->offset = g_new0(ram_addr_t, size);
+
+return pages;
+}
+
+static void multifd_pages_clear(MultiFDPages_t *pages)
+{
+pages->used = 0;
+pages->allocated = 0;
+pages->packet_num = 0;
+pages->block = NULL;
+g_free(pages->iov);
+pages->iov = NULL;
+g_free(pages->offset);
+pages->offset = NULL;
+g_free(pages);
+}
+
+static void multifd_send_fill_packet(MultiFDSendParams *p)
+{
+MultiFDPacket_t *packet = p->packet;
+int i;
+
+packet->flags = cpu_to_be32(p->flags);
+packet->pages_alloc = cpu_to_be32(p->pages->allocated);
+packet->pages_used = cpu_to_be32(p->pages->used);
+packet->n

[PULL 14/19] multifd: Make multifd_save_setup() get an Error parameter

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 

---

We can't trust that error_in is not NULL.  Create a local_error.
---
 migration/migration.c | 4 +++-
 migration/ram.c   | 2 +-
 migration/ram.h   | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 77768fb2c7..d478f832ea 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3367,6 +3367,7 @@ static void *migration_thread(void *opaque)
 
 void migrate_fd_connect(MigrationState *s, Error *error_in)
 {
+Error *local_err = NULL;
 int64_t rate_limit;
 bool resume = s->state == MIGRATION_STATUS_POSTCOPY_PAUSED;
 
@@ -3415,7 +3416,8 @@ void migrate_fd_connect(MigrationState *s, Error 
*error_in)
 return;
 }
 
-if (multifd_save_setup() != 0) {
+if (multifd_save_setup(&local_err) != 0) {
+error_report_err(local_err);
 migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
   MIGRATION_STATUS_FAILED);
 migrate_fd_cleanup(s);
diff --git a/migration/ram.c b/migration/ram.c
index 12b76b7841..78483247ad 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1252,7 +1252,7 @@ static void multifd_new_send_channel_async(QIOTask *task, 
gpointer opaque)
 }
 }
 
-int multifd_save_setup(void)
+int multifd_save_setup(Error **errp)
 {
 int thread_count;
 uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
diff --git a/migration/ram.h b/migration/ram.h
index bd0eee79b6..da22a417ea 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -41,7 +41,7 @@ int xbzrle_cache_resize(int64_t new_size, Error **errp);
 uint64_t ram_bytes_remaining(void);
 uint64_t ram_bytes_total(void);
 
-int multifd_save_setup(void);
+int multifd_save_setup(Error **errp);
 void multifd_save_cleanup(void);
 int multifd_load_setup(void);
 int multifd_load_cleanup(Error **errp);
-- 
2.24.1

[PULL 16/19] multifd: Add multifd-method parameter

[Juan Quintela]

This will store the compression method to use.  We start with none.

Signed-off-by: Juan Quintela 
Reviewed-by: Markus Armbruster 
Reviewed-by: Dr. David Alan Gilbert 

---
Rename it to NONE
Fix typos (dave)
We don't need to chek values returned by visit_type_MultifdCompress (markus)
Fix yet more typos (wei)
Rename multifd-compress to multifd-method
---
 hw/core/qdev-properties.c| 13 +
 include/hw/qdev-properties.h |  3 +++
 migration/migration.c| 13 +
 monitor/hmp-cmds.c   | 13 +
 qapi/migration.json  | 30 +++---
 tests/qtest/migration-test.c | 14 ++
 6 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 7f93bfeb88..4442844d37 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -8,6 +8,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qemu/ctype.h"
 #include "qemu/error-report.h"
+#include "qapi/qapi-types-migration.h"
 #include "hw/block/block.h"
 #include "net/hub.h"
 #include "qapi/visitor.h"
@@ -639,6 +640,18 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
 .set_default_value = set_default_value_enum,
 };
 
+/* --- MultiFDMethod --- */
+
+const PropertyInfo qdev_prop_multifd_method = {
+.name = "MultiFDMethod",
+.description = "multifd_method values, "
+   "none",
+.enum_table = &MultiFDMethod_lookup,
+.get = get_enum,
+.set = set_enum,
+.set_default_value = set_default_value_enum,
+};
+
 /* --- pci address --- */
 
 /*
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 906e697c58..6871b075a6 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -20,6 +20,7 @@ extern const PropertyInfo qdev_prop_chr;
 extern const PropertyInfo qdev_prop_tpm;
 extern const PropertyInfo qdev_prop_macaddr;
 extern const PropertyInfo qdev_prop_on_off_auto;
+extern const PropertyInfo qdev_prop_multifd_method;
 extern const PropertyInfo qdev_prop_losttickpolicy;
 extern const PropertyInfo qdev_prop_blockdev_on_error;
 extern const PropertyInfo qdev_prop_bios_chs_trans;
@@ -184,6 +185,8 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
 DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
 #define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \
 DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_on_off_auto, OnOffAuto)
+#define DEFINE_PROP_MULTIFD_METHOD(_n, _s, _f, _d) \
+DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_method, MultiFDMethod)
 #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
 DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
 LostTickPolicy)
diff --git a/migration/migration.c b/migration/migration.c
index adc7d08e93..9b24849651 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -87,6 +87,7 @@
 /* The delay time (in ms) between two COLO checkpoints */
 #define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
 #define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
+#define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_NONE
 
 /* Background transfer rate for postcopy, 0 means unlimited, note
  * that page requests can still exceed this limit.
@@ -797,6 +798,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
**errp)
 params->block_incremental = s->parameters.block_incremental;
 params->has_multifd_channels = true;
 params->multifd_channels = s->parameters.multifd_channels;
+params->has_multifd_method = true;
+params->multifd_method = s->parameters.multifd_method;
 params->has_xbzrle_cache_size = true;
 params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
 params->has_max_postcopy_bandwidth = true;
@@ -1314,6 +1317,9 @@ static void 
migrate_params_test_apply(MigrateSetParameters *params,
 if (params->has_multifd_channels) {
 dest->multifd_channels = params->multifd_channels;
 }
+if (params->has_multifd_method) {
+dest->multifd_method = params->multifd_method;
+}
 if (params->has_xbzrle_cache_size) {
 dest->xbzrle_cache_size = params->xbzrle_cache_size;
 }
@@ -1410,6 +1416,9 @@ static void migrate_params_apply(MigrateSetParameters 
*params, Error **errp)
 if (params->has_multifd_channels) {
 s->parameters.multifd_channels = params->multifd_channels;
 }
+if (params->has_multifd_method) {
+s->parameters.multifd_method = params->multifd_method;
+}
 if (params->has_xbzrle_cache_size) {
 s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
 xbzrle_cache_resize(params->xbzrle_cache_size, errp);
@@ -3514,6 +3523,9 @@ static Property migration_properties[] = {
 DEFINE_PROP_UINT8("multifd-channels", MigrationState,
   parameters.multifd_channels,
   DEFAULT_MIGRATE_MULTIFD_CHANNELS),
+DEFINE_PROP_MULTIFD_METHOD("multifd-method", MigrationState,
+  parameters.mul

[PULL 19/19] migration/compress: compress QEMUFile is not writable

[Juan Quintela]

From: Wei Yang 

We open a file with empty_ops for compress QEMUFile, which means this is
not writable.

Signed-off-by: Wei Yang 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/qemu-file.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index bbb2b63927..1c3a358a14 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -764,11 +764,8 @@ static int qemu_compress_data(z_stream *stream, uint8_t 
*dest, size_t dest_len,
 /* Compress size bytes of data start at p and store the compressed
  * data to the buffer of f.
  *
- * When f is not writable, return -1 if f has no space to save the
- * compressed data.
- * When f is wirtable and it has no space to save the compressed data,
- * do fflush first, if f still has no space to save the compressed
- * data, return -1.
+ * Since the file is dummy file with empty_ops, return -1 if f has no space to
+ * save the compressed data.
  */
 ssize_t qemu_put_compression_data(QEMUFile *f, z_stream *stream,
   const uint8_t *p, size_t size)
@@ -776,14 +773,7 @@ ssize_t qemu_put_compression_data(QEMUFile *f, z_stream 
*stream,
 ssize_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t);
 
 if (blen < compressBound(size)) {
-if (!qemu_file_is_writable(f)) {
-return -1;
-}
-qemu_fflush(f);
-blen = IO_BUF_SIZE - sizeof(int32_t);
-if (blen < compressBound(size)) {
-return -1;
-}
+return -1;
 }
 
 blen = qemu_compress_data(stream, f->buf + f->buf_index + sizeof(int32_t),
-- 
2.24.1

[PULL 13/19] migration: Make checkpatch happy with comments

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 8f04b5ab3a..12b76b7841 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1320,10 +1320,12 @@ static void multifd_recv_terminate_threads(Error *err)
 
 qemu_mutex_lock(&p->mutex);
 p->quit = true;
-/* We could arrive here for two reasons:
-   - normal quit, i.e. everything went fine, just finished
-   - error quit: We close the channels so the channel threads
- finish the qio_channel_read_all_eof() */
+/*
+ * We could arrive here for two reasons:
+ *  - normal quit, i.e. everything went fine, just finished
+ *  - error quit: We close the channels so the channel threads
+ *finish the qio_channel_read_all_eof()
+ */
 if (p->c) {
 qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
 }
-- 
2.24.1

[PULL 18/19] migration: Simplify get_qlist

[Juan Quintela]

From: Eric Auger 

Instead of inserting read elements at the head and
then reversing the list, it is simpler to add
each element after the previous one. Introduce
QLIST_RAW_INSERT_AFTER helper and use it in
get_qlist().

Signed-off-by: Eric Auger 
Suggested-by: Juan Quintela 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 include/qemu/queue.h  | 19 ++-
 migration/vmstate-types.c | 10 +++---
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index 4d4554a7ce..19425f973f 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -515,6 +515,12 @@ union {
 \
  (elm);
\
  (elm) = *QLIST_RAW_NEXT(elm, entry))
 
+#define QLIST_RAW_INSERT_AFTER(head, prev, elem, entry) do {   
\
+*QLIST_RAW_NEXT(prev, entry) = elem;   
\
+*QLIST_RAW_PREVIOUS(elem, entry) = QLIST_RAW_NEXT(prev, entry);
\
+*QLIST_RAW_NEXT(elem, entry) = NULL;   
\
+} while (0)
+
 #define QLIST_RAW_INSERT_HEAD(head, elm, entry) do {   
\
 void *first = *QLIST_RAW_FIRST(head);  
\
 *QLIST_RAW_FIRST(head) = elm;  
\
@@ -527,17 +533,4 @@ union {
 \
 }  
\
 } while (0)
 
-#define QLIST_RAW_REVERSE(head, elm, entry) do {   
\
-void *iter = *QLIST_RAW_FIRST(head), *prev = NULL, *next;  
\
-while (iter) { 
\
-next = *QLIST_RAW_NEXT(iter, entry);   
\
-*QLIST_RAW_PREVIOUS(iter, entry) = QLIST_RAW_NEXT(next, entry);
\
-*QLIST_RAW_NEXT(iter, entry) = prev;   
\
-prev = iter;   
\
-iter = next;   
\
-}  
\
-*QLIST_RAW_FIRST(head) = prev; 
\
-*QLIST_RAW_PREVIOUS(prev, entry) = QLIST_RAW_FIRST(head);  
\
-} while (0)
-
 #endif /* QEMU_SYS_QUEUE_H */
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index 1eee36773a..35e784c9d9 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -879,7 +879,7 @@ static int get_qlist(QEMUFile *f, void *pv, size_t 
unused_size,
 /* offset of the QLIST entry in a QLIST element */
 size_t entry_offset = field->start;
 int version_id = field->version_id;
-void *elm;
+void *elm, *prev = NULL;
 
 trace_get_qlist(field->name, vmsd->name, vmsd->version_id);
 if (version_id > vmsd->version_id) {
@@ -900,9 +900,13 @@ static int get_qlist(QEMUFile *f, void *pv, size_t 
unused_size,
 g_free(elm);
 return ret;
 }
-QLIST_RAW_INSERT_HEAD(pv, elm, entry_offset);
+if (!prev) {
+QLIST_RAW_INSERT_HEAD(pv, elm, entry_offset);
+} else {
+QLIST_RAW_INSERT_AFTER(pv, prev, elm, entry_offset);
+}
+prev = elm;
 }
-QLIST_RAW_REVERSE(pv, elm, entry_offset);
 trace_get_qlist_end(field->name, vmsd->name);
 
 return ret;
-- 
2.24.1

[PULL 10/19] multifd: multifd_queue_page only needs the qemufile

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 602e9ca5a0..ad9cc3e7bd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -974,7 +974,7 @@ static int multifd_send_pages(QEMUFile *f)
 return 1;
 }
 
-static int multifd_queue_page(RAMState *rs, RAMBlock *block, ram_addr_t offset)
+static int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset)
 {
 MultiFDPages_t *pages = multifd_send_state->pages;
 
@@ -993,12 +993,12 @@ static int multifd_queue_page(RAMState *rs, RAMBlock 
*block, ram_addr_t offset)
 }
 }
 
-if (multifd_send_pages(rs->f) < 0) {
+if (multifd_send_pages(f) < 0) {
 return -1;
 }
 
 if (pages->block != block) {
-return  multifd_queue_page(rs, block, offset);
+return  multifd_queue_page(f, block, offset);
 }
 
 return 1;
@@ -2136,7 +2136,7 @@ static int ram_save_page(RAMState *rs, PageSearchStatus 
*pss, bool last_stage)
 static int ram_save_multifd_page(RAMState *rs, RAMBlock *block,
  ram_addr_t offset)
 {
-if (multifd_queue_page(rs, block, offset) < 0) {
+if (multifd_queue_page(rs->f, block, offset) < 0) {
 return -1;
 }
 ram_counters.normal++;
-- 
2.24.1

[PULL 08/19] ram_addr: Split RAMBlock definition

[Juan Quintela]

We need some of the fields without having to poison everything else.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 MAINTAINERS |  1 +
 include/exec/ram_addr.h | 40 +-
 include/exec/ramblock.h | 64 +
 3 files changed, 66 insertions(+), 39 deletions(-)
 create mode 100644 include/exec/ramblock.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f6511d5120..b570a24d4d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1973,6 +1973,7 @@ F: ioport.c
 F: include/exec/memop.h
 F: include/exec/memory.h
 F: include/exec/ram_addr.h
+F: include/exec/ramblock.h
 F: memory.c
 F: include/exec/memory-internal.h
 F: exec.c
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 5adebb0bc7..5e59a3d8d7 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -24,45 +24,7 @@
 #include "hw/xen/xen.h"
 #include "sysemu/tcg.h"
 #include "exec/ramlist.h"
-
-struct RAMBlock {
-struct rcu_head rcu;
-struct MemoryRegion *mr;
-uint8_t *host;
-uint8_t *colo_cache; /* For colo, VM's ram cache */
-ram_addr_t offset;
-ram_addr_t used_length;
-ram_addr_t max_length;
-void (*resized)(const char*, uint64_t length, void *host);
-uint32_t flags;
-/* Protected by iothread lock.  */
-char idstr[256];
-/* RCU-enabled, writes protected by the ramlist lock */
-QLIST_ENTRY(RAMBlock) next;
-QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
-int fd;
-size_t page_size;
-/* dirty bitmap used during migration */
-unsigned long *bmap;
-/* bitmap of already received pages in postcopy */
-unsigned long *receivedmap;
-
-/*
- * bitmap to track already cleared dirty bitmap.  When the bit is
- * set, it means the corresponding memory chunk needs a log-clear.
- * Set this up to non-NULL to enable the capability to postpone
- * and split clearing of dirty bitmap on the remote node (e.g.,
- * KVM).  The bitmap will be set only when doing global sync.
- *
- * NOTE: this bitmap is different comparing to the other bitmaps
- * in that one bit can represent multiple guest pages (which is
- * decided by the `clear_bmap_shift' variable below).  On
- * destination side, this should always be NULL, and the variable
- * `clear_bmap_shift' is meaningless.
- */
-unsigned long *clear_bmap;
-uint8_t clear_bmap_shift;
-};
+#include "exec/ramblock.h"
 
 /**
  * clear_bmap_size: calculate clear bitmap size
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
new file mode 100644
index 00..07d50864d8
--- /dev/null
+++ b/include/exec/ramblock.h
@@ -0,0 +1,64 @@
+/*
+ * Declarations for cpu physical memory functions
+ *
+ * Copyright 2011 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ *  Avi Kivity 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ *
+ */
+
+/*
+ * This header is for use by exec.c and memory.c ONLY.  Do not include it.
+ * The functions declared here will be removed soon.
+ */
+
+#ifndef QEMU_EXEC_RAMBLOCK_H
+#define QEMU_EXEC_RAMBLOCK_H
+
+#ifndef CONFIG_USER_ONLY
+#include "cpu-common.h"
+
+struct RAMBlock {
+struct rcu_head rcu;
+struct MemoryRegion *mr;
+uint8_t *host;
+uint8_t *colo_cache; /* For colo, VM's ram cache */
+ram_addr_t offset;
+ram_addr_t used_length;
+ram_addr_t max_length;
+void (*resized)(const char*, uint64_t length, void *host);
+uint32_t flags;
+/* Protected by iothread lock.  */
+char idstr[256];
+/* RCU-enabled, writes protected by the ramlist lock */
+QLIST_ENTRY(RAMBlock) next;
+QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
+int fd;
+size_t page_size;
+/* dirty bitmap used during migration */
+unsigned long *bmap;
+/* bitmap of already received pages in postcopy */
+unsigned long *receivedmap;
+
+/*
+ * bitmap to track already cleared dirty bitmap.  When the bit is
+ * set, it means the corresponding memory chunk needs a log-clear.
+ * Set this up to non-NULL to enable the capability to postpone
+ * and split clearing of dirty bitmap on the remote node (e.g.,
+ * KVM).  The bitmap will be set only when doing global sync.
+ *
+ * NOTE: this bitmap is different comparing to the other bitmaps
+ * in that one bit can represent multiple guest pages (which is
+ * decided by the `clear_bmap_shift' variable below).  On
+ * destination side, this should always be NULL, and the variable
+ * `clear_bmap_shift' is meaningless.
+ */
+unsigned long *clear_bmap;
+uint8_t clear_bmap_shift;
+};
+#endif
+#endif
-- 
2.24.1

[PULL 15/19] multifd: Make multifd_load_setup() get an Error parameter

[Juan Quintela]

We need to change the full chain to pass the Error parameter.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 

---

Always use a local_err, and in case of error propagate/print it as needed.
---
 migration/migration.c | 35 +--
 migration/migration.h |  2 +-
 migration/ram.c   |  2 +-
 migration/ram.h   |  2 +-
 migration/rdma.c  |  2 +-
 5 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index d478f832ea..adc7d08e93 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -518,13 +518,23 @@ fail:
 exit(EXIT_FAILURE);
 }
 
-static void migration_incoming_setup(QEMUFile *f)
+/**
+ * @migration_incoming_setup: Setup incoming migration
+ *
+ * Returns 0 for no error or 1 for error
+ *
+ * @f: file for main migration channel
+ * @errp: where to put errors
+ */
+static int migration_incoming_setup(QEMUFile *f, Error **errp)
 {
 MigrationIncomingState *mis = migration_incoming_get_current();
+Error *local_err = NULL;
 
-if (multifd_load_setup() != 0) {
+if (multifd_load_setup(&local_err) != 0) {
 /* We haven't been able to create multifd threads
nothing better to do */
+error_report_err(local_err);
 exit(EXIT_FAILURE);
 }
 
@@ -532,6 +542,7 @@ static void migration_incoming_setup(QEMUFile *f)
 mis->from_src_file = f;
 }
 qemu_file_set_blocking(f, false);
+return 0;
 }
 
 void migration_incoming_process(void)
@@ -572,19 +583,27 @@ static bool postcopy_try_recover(QEMUFile *f)
 return false;
 }
 
-void migration_fd_process_incoming(QEMUFile *f)
+void migration_fd_process_incoming(QEMUFile *f, Error **errp)
 {
+Error *local_err = NULL;
+
 if (postcopy_try_recover(f)) {
 return;
 }
 
-migration_incoming_setup(f);
+if (migration_incoming_setup(f, &local_err)) {
+if (local_err) {
+error_propagate(errp, local_err);
+}
+return;
+}
 migration_incoming_process();
 }
 
 void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp)
 {
 MigrationIncomingState *mis = migration_incoming_get_current();
+Error *local_err = NULL;
 bool start_migration;
 
 if (!mis->from_src_file) {
@@ -596,7 +615,12 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error 
**errp)
 return;
 }
 
-migration_incoming_setup(f);
+if (migration_incoming_setup(f, &local_err)) {
+if (local_err) {
+error_propagate(errp, local_err);
+}
+return;
+}
 
 /*
  * Common migration only needs one channel, so we can start
@@ -604,7 +628,6 @@ void migration_ioc_process_incoming(QIOChannel *ioc, Error 
**errp)
  */
 start_migration = !migrate_use_multifd();
 } else {
-Error *local_err = NULL;
 /* Multiple connections */
 assert(migrate_use_multifd());
 start_migration = multifd_recv_new_channel(ioc, &local_err);
diff --git a/migration/migration.h b/migration/migration.h
index 44b1d56929..8473ddfc88 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -265,7 +265,7 @@ struct MigrationState
 
 void migrate_set_state(int *state, int old_state, int new_state);
 
-void migration_fd_process_incoming(QEMUFile *f);
+void migration_fd_process_incoming(QEMUFile *f, Error **errp);
 void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp);
 void migration_incoming_process(void);
 
diff --git a/migration/ram.c b/migration/ram.c
index 78483247ad..3abd41ad33 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1474,7 +1474,7 @@ static void *multifd_recv_thread(void *opaque)
 return NULL;
 }
 
-int multifd_load_setup(void)
+int multifd_load_setup(Error **errp)
 {
 int thread_count;
 uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
diff --git a/migration/ram.h b/migration/ram.h
index da22a417ea..42be471d52 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -43,7 +43,7 @@ uint64_t ram_bytes_total(void);
 
 int multifd_save_setup(Error **errp);
 void multifd_save_cleanup(void);
-int multifd_load_setup(void);
+int multifd_load_setup(Error **errp);
 int multifd_load_cleanup(Error **errp);
 bool multifd_recv_all_channels_created(void);
 bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp);
diff --git a/migration/rdma.c b/migration/rdma.c
index e241dcb992..2379b8345b 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4004,7 +4004,7 @@ static void rdma_accept_incoming_migration(void *opaque)
 }
 
 rdma->migration_started_on_destination = 1;
-migration_fd_process_incoming(f);
+migration_fd_process_incoming(f, errp);
 }
 
 void rdma_start_incoming_migration(const char *host_port, Error **errp)
-- 
2.24.1

[PULL 11/19] multifd: multifd_send_sync_main only needs the qemufile

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index ad9cc3e7bd..152e9cf46d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1082,7 +1082,7 @@ void multifd_save_cleanup(void)
 multifd_send_state = NULL;
 }
 
-static void multifd_send_sync_main(RAMState *rs)
+static void multifd_send_sync_main(QEMUFile *f)
 {
 int i;
 
@@ -1090,7 +1090,7 @@ static void multifd_send_sync_main(RAMState *rs)
 return;
 }
 if (multifd_send_state->pages->used) {
-if (multifd_send_pages(rs->f) < 0) {
+if (multifd_send_pages(f) < 0) {
 error_report("%s: multifd_send_pages fail", __func__);
 return;
 }
@@ -,7 +,7 @@ static void multifd_send_sync_main(RAMState *rs)
 p->packet_num = multifd_send_state->packet_num++;
 p->flags |= MULTIFD_FLAG_SYNC;
 p->pending_job++;
-qemu_file_update_transfer(rs->f, p->packet_len);
+qemu_file_update_transfer(f, p->packet_len);
 ram_counters.multifd_bytes += p->packet_len;
 ram_counters.transferred += p->packet_len;
 qemu_mutex_unlock(&p->mutex);
@@ -3434,7 +3434,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 ram_control_before_iterate(f, RAM_CONTROL_SETUP);
 ram_control_after_iterate(f, RAM_CONTROL_SETUP);
 
-multifd_send_sync_main(*rsp);
+multifd_send_sync_main(f);
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 qemu_fflush(f);
 
@@ -3534,7 +3534,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 out:
 if (ret >= 0
 && migration_is_setup_or_active(migrate_get_current()->state)) {
-multifd_send_sync_main(rs);
+multifd_send_sync_main(rs->f);
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 qemu_fflush(f);
 ram_counters.transferred += 8;
@@ -3593,7 +3593,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
 }
 
 if (ret >= 0) {
-multifd_send_sync_main(rs);
+multifd_send_sync_main(rs->f);
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 qemu_fflush(f);
 }
-- 
2.24.1

[PULL 02/19] multifd: Make sure that we don't do any IO after an error

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index d2208b5534..f95d656c26 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3445,7 +3445,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 {
 RAMState **temp = opaque;
 RAMState *rs = *temp;
-int ret;
+int ret = 0;
 int i;
 int64_t t0;
 int done = 0;
@@ -3524,12 +3524,14 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 ram_control_after_iterate(f, RAM_CONTROL_ROUND);
 
 out:
-multifd_send_sync_main(rs);
-qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
-qemu_fflush(f);
-ram_counters.transferred += 8;
+if (ret >= 0) {
+multifd_send_sync_main(rs);
+qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+qemu_fflush(f);
+ram_counters.transferred += 8;
 
-ret = qemu_file_get_error(f);
+ret = qemu_file_get_error(f);
+}
 if (ret < 0) {
 return ret;
 }
@@ -3581,9 +3583,11 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
 ram_control_after_iterate(f, RAM_CONTROL_FINISH);
 }
 
-multifd_send_sync_main(rs);
-qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
-qemu_fflush(f);
+if (ret >= 0) {
+multifd_send_sync_main(rs);
+qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+qemu_fflush(f);
+}
 
 return ret;
 }
-- 
2.24.1

[PULL 07/19] migration/multifd: fix nullptr access in multifd_send_terminate_threads

[Juan Quintela]

From: Zhimin Feng 

If the multifd_send_threads is not created when migration is failed,
multifd_save_cleanup would be called twice. In this senario, the
multifd_send_state is accessed after it has been released, the result
is that the source VM is crashing down.

Here is the coredump stack:
Program received signal SIGSEGV, Segmentation fault.
0x5629333a78ef in multifd_send_terminate_threads (err=err@entry=0x0) at 
migration/ram.c:1012
1012MultiFDSendParams *p = &multifd_send_state->params[i];
#0  0x5629333a78ef in multifd_send_terminate_threads 
(err=err@entry=0x0) at migration/ram.c:1012
#1  0x5629333ab8a9 in multifd_save_cleanup () at migration/ram.c:1028
#2  0x5629333abaea in multifd_new_send_channel_async 
(task=0x562935450e70, opaque=) at migration/ram.c:1202
#3  0x56293373a562 in qio_task_complete 
(task=task@entry=0x562935450e70) at io/task.c:196
#4  0x56293373a6e0 in qio_task_thread_result (opaque=0x562935450e70) at 
io/task.c:111
#5  0x7f475d4d75a7 in g_idle_dispatch () from 
/usr/lib64/libglib-2.0.so.0
#6  0x7f475d4da9a9 in g_main_context_dispatch () from 
/usr/lib64/libglib-2.0.so.0
#7  0x562933785b33 in glib_pollfds_poll () at util/main-loop.c:219
#8  os_host_main_loop_wait (timeout=) at util/main-loop.c:242
#9  main_loop_wait (nonblocking=nonblocking@entry=0) at util/main-loop.c:518
#10 0x5629334c5acf in main_loop () at vl.c:1810
#11 0x56293334d7bb in main (argc=, argv=, 
envp=) at vl.c:4471

If the multifd_send_threads is not created when migration is failed.
In this senario, we don't call multifd_save_cleanup in 
multifd_new_send_channel_async.

Signed-off-by: Zhimin Feng 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 3fd7fdffcf..82c7edb083 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1233,7 +1233,15 @@ static void multifd_new_send_channel_async(QIOTask 
*task, gpointer opaque)
 trace_multifd_new_send_channel_async(p->id);
 if (qio_task_propagate_error(task, &local_err)) {
 migrate_set_error(migrate_get_current(), local_err);
-multifd_save_cleanup();
+/* Error happen, we need to tell who pay attention to me */
+qemu_sem_post(&multifd_send_state->channels_ready);
+qemu_sem_post(&p->sem_sync);
+/*
+ * Although multifd_send_thread is not created, but main migration
+ * thread neet to judge whether it is running, so we need to mark
+ * its status.
+ */
+p->quit = true;
 } else {
 p->c = QIO_CHANNEL(sioc);
 qio_channel_set_delay(p->c, false);
-- 
2.24.1

[PULL 12/19] multifd: Use qemu_target_page_size()

[Juan Quintela]

We will make it cpu independent.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 152e9cf46d..8f04b5ab3a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -882,14 +882,14 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams 
*p, Error **errp)
 for (i = 0; i < p->pages->used; i++) {
 uint64_t offset = be64_to_cpu(packet->offset[i]);
 
-if (offset > (block->used_length - TARGET_PAGE_SIZE)) {
+if (offset > (block->used_length - qemu_target_page_size())) {
 error_setg(errp, "multifd: offset too long %" PRIu64
" (max " RAM_ADDR_FMT ")",
offset, block->max_length);
 return -1;
 }
 p->pages->iov[i].iov_base = block->host + offset;
-p->pages->iov[i].iov_len = TARGET_PAGE_SIZE;
+p->pages->iov[i].iov_len = qemu_target_page_size();
 }
 
 return 0;
@@ -964,7 +964,8 @@ static int multifd_send_pages(QEMUFile *f)
 p->packet_num = multifd_send_state->packet_num++;
 multifd_send_state->pages = p->pages;
 p->pages = pages;
-transferred = ((uint64_t) pages->used) * TARGET_PAGE_SIZE + p->packet_len;
+transferred = ((uint64_t) pages->used) * qemu_target_page_size()
++ p->packet_len;
 qemu_file_update_transfer(f, transferred);
 ram_counters.multifd_bytes += transferred;
 ram_counters.transferred += transferred;;
@@ -985,7 +986,7 @@ static int multifd_queue_page(QEMUFile *f, RAMBlock *block, 
ram_addr_t offset)
 if (pages->block == block) {
 pages->offset[pages->used] = offset;
 pages->iov[pages->used].iov_base = block->host + offset;
-pages->iov[pages->used].iov_len = TARGET_PAGE_SIZE;
+pages->iov[pages->used].iov_len = qemu_target_page_size();
 pages->used++;
 
 if (pages->used < pages->allocated) {
-- 
2.24.1

[PULL 05/19] migration-test: Make sure that multifd and cancel works

[Juan Quintela]

Test that this sequence works:

- launch source
- launch target
- start migration
- cancel migration
- relaunch target
- do migration again

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 

---

- Wait for 1st trhead to move to cancelled before launching second
  migration
- Add 'to2' parameter to diferentiate 1st and second target.
- Use g_free() instead of free()
---
 tests/qtest/migration-test.c | 112 ++-
 1 file changed, 111 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b6a74a05ce..cf27ebbc9d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -424,6 +424,14 @@ static void migrate_recover(QTestState *who, const char 
*uri)
 qobject_unref(rsp);
 }
 
+static void migrate_cancel(QTestState *who)
+{
+QDict *rsp;
+
+rsp = wait_command(who, "{ 'execute': 'migrate_cancel' }");
+qobject_unref(rsp);
+}
+
 static void migrate_set_capability(QTestState *who, const char *capability,
bool value)
 {
@@ -456,6 +464,8 @@ static void migrate_postcopy_start(QTestState *from, 
QTestState *to)
 typedef struct {
 bool hide_stderr;
 bool use_shmem;
+/* only launch the target process */
+bool only_target;
 char *opts_source;
 char *opts_target;
 } MigrateStart;
@@ -571,7 +581,9 @@ static int test_migrate_start(QTestState **from, QTestState 
**to,
  arch_source, shmem_opts, args->opts_source,
  ignore_stderr);
 g_free(arch_source);
-*from = qtest_init(cmd_source);
+if (!args->only_target) {
+*from = qtest_init(cmd_source);
+}
 g_free(cmd_source);
 
 cmd_target = g_strdup_printf("-accel kvm -accel tcg%s%s "
@@ -1294,6 +1306,103 @@ static void test_multifd_tcp(void)
 g_free(uri);
 }
 
+/*
+ * This test does:
+ *  source   target
+ *   migrate_incoming
+ * migrate
+ * migrate_cancel
+ *   launch another target
+ * migrate
+ *
+ *  And see that it works
+ */
+
+static void test_multifd_tcp_cancel(void)
+{
+MigrateStart *args = migrate_start_new();
+QTestState *from, *to, *to2;
+QDict *rsp;
+char *uri;
+
+args->hide_stderr = true;
+
+if (test_migrate_start(&from, &to, "defer", args)) {
+return;
+}
+
+/*
+ * We want to pick a speed slow enough that the test completes
+ * quickly, but that it doesn't complete precopy even on a slow
+ * machine, so also set the downtime.
+ */
+/* 1 ms should make it not converge*/
+migrate_set_parameter_int(from, "downtime-limit", 1);
+/* 300MB/s */
+migrate_set_parameter_int(from, "max-bandwidth", 3000);
+
+migrate_set_parameter_int(from, "multifd-channels", 16);
+migrate_set_parameter_int(to, "multifd-channels", 16);
+
+migrate_set_capability(from, "multifd", "true");
+migrate_set_capability(to, "multifd", "true");
+
+/* Start incoming migration from the 1st socket */
+rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
+   "  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
+qobject_unref(rsp);
+
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+uri = migrate_get_socket_address(to, "socket-address");
+
+migrate_qmp(from, uri, "{}");
+
+wait_for_migration_pass(from);
+
+migrate_cancel(from);
+
+args = migrate_start_new();
+args->only_target = true;
+
+if (test_migrate_start(&from, &to2, "defer", args)) {
+return;
+}
+
+migrate_set_parameter_int(to2, "multifd-channels", 16);
+
+migrate_set_capability(to2, "multifd", "true");
+
+/* Start incoming migration from the 1st socket */
+rsp = wait_command(to2, "{ 'execute': 'migrate-incoming',"
+"  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
+qobject_unref(rsp);
+
+uri = migrate_get_socket_address(to2, "socket-address");
+
+wait_for_migration_status(from, "cancelled", NULL);
+
+/* 300ms it should converge */
+migrate_set_parameter_int(from, "downtime-limit", 300);
+/* 1GB/s */
+migrate_set_parameter_int(from, "max-bandwidth", 10);
+
+migrate_qmp(from, uri, "{}");
+
+wait_for_migration_pass(from);
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+qtest_qmp_eventwait(to2, "RESUME");
+
+wait_for_serial("dest_serial");
+wait_for_migration_complete(from);
+test_migrate_end(from, to2, true);
+g_free(uri);
+}
+
 int main(int argc, char **argv)
 {
 char template[] = "/tmp/migration-test-XX";
@@ -1359,6 +1468,7 @@ int main(int argc, char **argv)
 
 qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
 qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
+qtest_add_func("/migration/multifd/tcp/canc

[PULL 03/19] qemu-file: Don't do IO after shutdown

[Juan Quintela]

Be sure that we are not doing neither read/write after shutdown of the
QEMUFile.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 

---

Set error in case that there is none (dave)
---
 migration/qemu-file.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 26fb25ddc1..bbb2b63927 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -53,6 +53,8 @@ struct QEMUFile {
 
 int last_error;
 Error *last_error_obj;
+/* has the file has been shutdown */
+bool shutdown;
 };
 
 /*
@@ -61,10 +63,18 @@ struct QEMUFile {
  */
 int qemu_file_shutdown(QEMUFile *f)
 {
+int ret;
+
+f->shutdown = true;
 if (!f->ops->shut_down) {
 return -ENOSYS;
 }
-return f->ops->shut_down(f->opaque, true, true, NULL);
+ret = f->ops->shut_down(f->opaque, true, true, NULL);
+
+if (!f->last_error) {
+qemu_file_set_error(f, -EIO);
+}
+return ret;
 }
 
 /*
@@ -214,6 +224,9 @@ void qemu_fflush(QEMUFile *f)
 return;
 }
 
+if (f->shutdown) {
+return;
+}
 if (f->iovcnt > 0) {
 expect = iov_size(f->iov, f->iovcnt);
 ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt, f->pos,
@@ -328,6 +341,10 @@ static ssize_t qemu_fill_buffer(QEMUFile *f)
 f->buf_index = 0;
 f->buf_size = pending;
 
+if (f->shutdown) {
+return 0;
+}
+
 len = f->ops->get_buffer(f->opaque, f->buf + pending, f->pos,
  IO_BUF_SIZE - pending, &local_error);
 if (len > 0) {
@@ -642,6 +659,9 @@ int64_t qemu_ftell(QEMUFile *f)
 
 int qemu_file_rate_limit(QEMUFile *f)
 {
+if (f->shutdown) {
+return 1;
+}
 if (qemu_file_get_error(f)) {
 return 1;
 }
-- 
2.24.1

[PULL 06/19] migration: Create migration_is_running()

[Juan Quintela]

This function returns true if we are in the middle of a migration.
It is like migration_is_setup_or_active() with CANCELLING and COLO.
Adapt all callers that are needed.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 

---

Fix typo.  Thanks Denis.
---
 migration/migration.c | 29 -
 migration/migration.h |  1 +
 migration/savevm.c|  4 +---
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index efd5350e84..77768fb2c7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -829,6 +829,27 @@ bool migration_is_setup_or_active(int state)
 }
 }
 
+bool migration_is_running(int state)
+{
+switch (state) {
+case MIGRATION_STATUS_ACTIVE:
+case MIGRATION_STATUS_POSTCOPY_ACTIVE:
+case MIGRATION_STATUS_POSTCOPY_PAUSED:
+case MIGRATION_STATUS_POSTCOPY_RECOVER:
+case MIGRATION_STATUS_SETUP:
+case MIGRATION_STATUS_PRE_SWITCHOVER:
+case MIGRATION_STATUS_DEVICE:
+case MIGRATION_STATUS_WAIT_UNPLUG:
+case MIGRATION_STATUS_CANCELLING:
+case MIGRATION_STATUS_COLO:
+return true;
+
+default:
+return false;
+
+}
+}
+
 static void populate_time_info(MigrationInfo *info, MigrationState *s)
 {
 info->has_status = true;
@@ -1077,7 +1098,7 @@ void 
qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
 MigrationCapabilityStatusList *cap;
 bool cap_list[MIGRATION_CAPABILITY__MAX];
 
-if (migration_is_setup_or_active(s->state)) {
+if (migration_is_running(s->state)) {
 error_setg(errp, QERR_MIGRATION_ACTIVE);
 return;
 }
@@ -1590,7 +1611,7 @@ static void migrate_fd_cancel(MigrationState *s)
 
 do {
 old_state = s->state;
-if (!migration_is_setup_or_active(old_state)) {
+if (!migration_is_running(old_state)) {
 break;
 }
 /* If the migration is paused, kick it out of the pause */
@@ -1888,9 +1909,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, 
bool blk_inc,
 return true;
 }
 
-if (migration_is_setup_or_active(s->state) ||
-s->state == MIGRATION_STATUS_CANCELLING ||
-s->state == MIGRATION_STATUS_COLO) {
+if (migration_is_running(s->state)) {
 error_setg(errp, QERR_MIGRATION_ACTIVE);
 return false;
 }
diff --git a/migration/migration.h b/migration/migration.h
index aa9ff6f27b..44b1d56929 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -279,6 +279,7 @@ void migrate_fd_error(MigrationState *s, const Error 
*error);
 void migrate_fd_connect(MigrationState *s, Error *error_in);
 
 bool migration_is_setup_or_active(int state);
+bool migration_is_running(int state);
 
 void migrate_init(MigrationState *s);
 bool migration_is_blocked(Error **errp);
diff --git a/migration/savevm.c b/migration/savevm.c
index adfdca26ac..f19cb9ec7a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1531,9 +1531,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
 MigrationState *ms = migrate_get_current();
 MigrationStatus status;
 
-if (migration_is_setup_or_active(ms->state) ||
-ms->state == MIGRATION_STATUS_CANCELLING ||
-ms->state == MIGRATION_STATUS_COLO) {
+if (migration_is_running(ms->state)) {
 error_setg(errp, QERR_MIGRATION_ACTIVE);
 return -EINVAL;
 }
-- 
2.24.1

[PULL 09/19] multifd: multifd_send_pages only needs the qemufile

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 82c7edb083..602e9ca5a0 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -929,7 +929,7 @@ struct {
  * false.
  */
 
-static int multifd_send_pages(RAMState *rs)
+static int multifd_send_pages(QEMUFile *f)
 {
 int i;
 static int next_channel;
@@ -965,7 +965,7 @@ static int multifd_send_pages(RAMState *rs)
 multifd_send_state->pages = p->pages;
 p->pages = pages;
 transferred = ((uint64_t) pages->used) * TARGET_PAGE_SIZE + p->packet_len;
-qemu_file_update_transfer(rs->f, transferred);
+qemu_file_update_transfer(f, transferred);
 ram_counters.multifd_bytes += transferred;
 ram_counters.transferred += transferred;;
 qemu_mutex_unlock(&p->mutex);
@@ -993,7 +993,7 @@ static int multifd_queue_page(RAMState *rs, RAMBlock 
*block, ram_addr_t offset)
 }
 }
 
-if (multifd_send_pages(rs) < 0) {
+if (multifd_send_pages(rs->f) < 0) {
 return -1;
 }
 
@@ -1090,7 +1090,7 @@ static void multifd_send_sync_main(RAMState *rs)
 return;
 }
 if (multifd_send_state->pages->used) {
-if (multifd_send_pages(rs) < 0) {
+if (multifd_send_pages(rs->f) < 0) {
 error_report("%s: multifd_send_pages fail", __func__);
 return;
 }
-- 
2.24.1

[PULL 01/19] migration-test: Use g_free() instead of free()

[Juan Quintela]

Signed-off-by: Juan Quintela 
Reviewed-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
---
 tests/qtest/migration-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 26e2e77289..b6a74a05ce 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1291,7 +1291,7 @@ static void test_multifd_tcp(void)
 wait_for_serial("dest_serial");
 wait_for_migration_complete(from);
 test_migrate_end(from, to, true);
-free(uri);
+g_free(uri);
 }
 
 int main(int argc, char **argv)
-- 
2.24.1

[PULL 00/19] 10 next patches

[Juan Quintela]

The following changes since commit 105b07f1ba462ec48b27e5cb74ddf81c6a79364c:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200127' into 
staging (2020-01-27 13:02:36 +)

are available in the Git repository at:

  https://github.com/juanquintela/qemu.git tags/10_next-pull-request

for you to fetch changes up to 3189f80ee7b44c968796e63f81c92c915fccdb08:

  migration/compress: compress QEMUFile is not writable (2020-01-27 20:47:24 
+0100)


Migration pull request

This pull request include:
- simplify get_qlist (eric)
- fix null in multifd_send_terminate_threads (zhimin)
- small fix for compress (wei)
- migrate multifd + cancel fixes (juan)
- migrate compression: the bits that are reviewed (juan)



Eric Auger (1):
  migration: Simplify get_qlist

Juan Quintela (16):
  migration-test: Use g_free() instead of free()
  multifd: Make sure that we don't do any IO after an error
  qemu-file: Don't do IO after shutdown
  migration: Don't send data if we have stopped
  migration-test: Make sure that multifd and cancel works
  migration: Create migration_is_running()
  ram_addr: Split RAMBlock definition
  multifd: multifd_send_pages only needs the qemufile
  multifd: multifd_queue_page only needs the qemufile
  multifd: multifd_send_sync_main only needs the qemufile
  multifd: Use qemu_target_page_size()
  migration: Make checkpatch happy with comments
  multifd: Make multifd_save_setup() get an Error parameter
  multifd: Make multifd_load_setup() get an Error parameter
  multifd: Add multifd-method parameter
  multifd: Split multifd code into its own file

Wei Yang (1):
  migration/compress: compress QEMUFile is not writable

Zhimin Feng (1):
  migration/multifd: fix nullptr access in
multifd_send_terminate_threads

 MAINTAINERS  |1 +
 hw/core/qdev-properties.c|   13 +
 include/exec/ram_addr.h  |   40 +-
 include/exec/ramblock.h  |   64 +++
 include/hw/qdev-properties.h |3 +
 include/qemu/queue.h |   19 +-
 migration/Makefile.objs  |1 +
 migration/migration.c|   82 ++-
 migration/migration.h|3 +-
 migration/multifd.c  |  899 ++
 migration/multifd.h  |  139 +
 migration/qemu-file.c|   38 +-
 migration/ram.c  | 1004 +-
 migration/ram.h  |7 -
 migration/rdma.c |2 +-
 migration/savevm.c   |4 +-
 migration/vmstate-types.c|   10 +-
 monitor/hmp-cmds.c   |   13 +
 qapi/migration.json  |   30 +-
 tests/qtest/migration-test.c |  126 -
 20 files changed, 1410 insertions(+), 1088 deletions(-)
 create mode 100644 include/exec/ramblock.h
 create mode 100644 migration/multifd.c
 create mode 100644 migration/multifd.h

-- 
2.24.1

[PULL 04/19] migration: Don't send data if we have stopped

[Juan Quintela]

If we do a cancel, we got out without one error, but we can't do the
rest of the output as in a normal situation.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 migration/ram.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index f95d656c26..3fd7fdffcf 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3524,7 +3524,8 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 ram_control_after_iterate(f, RAM_CONTROL_ROUND);
 
 out:
-if (ret >= 0) {
+if (ret >= 0
+&& migration_is_setup_or_active(migrate_get_current()->state)) {
 multifd_send_sync_main(rs);
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 qemu_fflush(f);
-- 
2.24.1

Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic consistency checks

[Robert Henry]

This proposed text sounds good. Better English is to say "concepts" rather than 
"conceptions".

My plugin currently allocates its own unique user data structure on every call 
to the instrumentation-time callback.  This piece of user data captures the 
transient data presented from qemu.   What's missing from the interface is a 
callback when qemu can guarantee that the run-time callback will never be 
called again with this piece of user data.  At that point I would free my piece 
of user data.

I'm not too worried about this memory loss, yet.  I ran ubuntu for 3 days on 
qemu+plugins and only observed a tolerable growth in qemu's memory consumption.

From: Alex Bennée 
Sent: Friday, January 24, 2020 11:44 AM
To: Robert Henry 
Cc: Laurent Desnogues ; qemu-devel@nongnu.org 

Subject: Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic 
consistency checks


Robert Henry  writes:

> I found at least one problem with my plugin.
>
> I was assuming that the insn data from
>   struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> could be passed into qemu_plugin_register_vcpu_insn_exec_cb both as the 1st 
> argument AND as the user data last argument.  This assumed that insn would 
> persist and be unique from when qemu_plugin_register_vcpu_insn_exec_cb was 
> called to when the execution-time callback (vcpu_insn_exec_before) was called.
>
> This assumption is not true.
>
> I now capture pieces of *insn into my own persistent data structure, and pass 
> that in as void *udata, my problems went away.
>
> I think this lack of persistence of insn should be documented, or
> treated as a bug to be fixed.

I thought I had done this but it turns out I only mentioned it for
hwaddr:

  /*
   * qemu_plugin_get_hwaddr():
   * @vaddr: the virtual address of the memory operation
   *
   * For system emulation returns a qemu_plugin_hwaddr handle to query
   * details about the actual physical address backing the virtual
   * address. For linux-user guests it just returns NULL.
   *
   * This handle is *only* valid for the duration of the callback. Any
   * information about the handle should be recovered before the
   * callback returns.
   */

But the concept of handle lifetime is true for all the handles. I
propose something like this for the docs:


--8<---cut here---start->8---
docs/devel: document query handle lifetimes

I forgot to document the lifetime of handles in the developer
documentation. Do so now.

Signed-off-by: Alex Bennée 

1 file changed, 11 insertions(+), 2 deletions(-)
docs/devel/tcg-plugins.rst | 13 +++--

modified   docs/devel/tcg-plugins.rst
@@ -51,8 +51,17 @@ about how QEMU's translation works to the plugins. While 
there are
 conceptions such as translation time and translation blocks the
 details are opaque to plugins. The plugin is able to query select
 details of instructions and system configuration only through the
-exported *qemu_plugin* functions. The types used to describe
-instructions and events are opaque to the plugins themselves.
+exported *qemu_plugin* functions.
+
+Query Handle Lifetime
+-
+
+Each callback provides an opaque anonymous information handle which
+can usually be further queried to find out information about a
+translation, instruction or operation. The handles themselves are only
+valid during the lifetime of the callback so it is important that any
+information that is needed is extracted during the callback and saved
+by the plugin.

 Usage
 =

--8<---cut here---end--->8---

--
Alex Bennée

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Robert Foley]

On Mon, 27 Jan 2020 at 15:07, Alex Bennée  wrote:
> Robert Foley  writes:
>
> > Hi Drew,
> >
> > On Mon, 27 Jan 2020 at 12:27, Andrew Jones  wrote:
> >
> >> >
> >> > I suppose we could check the version of QEMU and use the above
> >> > defaults only for earlier versions of QEMU.
> >> > This is something we will probably move to aarch64vm.py since it is 
> >> > common.
> >>
> >> What versions of QEMU do these tests *have* to support? Because we could
> >> just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
> >> 'max' is indeed the nicest selection for using the same command line on
> >> KVM (gicv2 and gicv3 hosts) and TCG.
> >
> > I believe these test scripts which build/launch the VM have to support
> > the older version of QEMU since
> > this is the version of QEMU currently used when these VMs are
> > launched.  I don't know the history on
> > this, but it seems intentional that we use one older/different version
> > of QEMU to launch the VM,
>
> Well we defer to the system QEMU as it should be stable. It can be
> overridden with the QEMU environment variable which worked well enough
> when we only had VMs of one architecture. Perhaps we needs a new way to
> say "use the appropriate QEMU from this build"?

Good idea.  This is a pretty common use case and it makes sense to
make it easy to use.
I will add some support for this in my patch series.

Thanks & Regards,
-Rob
>
> > while we test the 'current' build of QEMU inside the VM.
> > It also seems like a 'nice to have' to automatically support the
> > latest version where we could
> > use max as you pointed out.
> >
> > Thanks & Regards,
> > -Rob
>
>
> --
> Alex Bennée

Re: [PATCH v3 00/13] RFC: [for 5.0]: HMP monitor handlers cleanups

[John Snow]

On 1/27/20 3:43 PM, Peter Krempa wrote:
> On Mon, Jan 27, 2020 at 14:39:02 -0500, John Snow wrote:
>>
>>
>> On 1/27/20 5:36 AM, Maxim Levitsky wrote:
>>> This patch series is bunch of cleanups
>>> to the hmp monitor code.
>>>
>>> This series only touched blockdev related hmp handlers.
>>>
>>> No functional changes expected other that
>>> light error message changes by the last patch.
>>>
>>> This was inspired by this bugzilla:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1719169
>>>
>>> Basically some users still parse hmp error messages,
>>> and they would like to have them prefixed with 'Error:'
>>>
>>
>> HMP isn't meant to be parsed. It's explicitly *not* API or ABI. I do
>> like consistency in my UIs (it's useful for human eyes, too), but I'd
>> like to know more about the request.
> 
> That's true as long as there's an stable replacement ... see below.
> 

Thanks for the context!

>>
>> Is this request coming from libvirt? Can we wean them off of this
>> interface? What do they need as a replacement?
> 
> There are 5 commands that libvirt still has HMP interfaces for:
> 
> drive_add
> drive_del
> 
> savevm
> loadvm
> delvm
> 
> From upstream point of view there's no value in adding the 'error'
> prefix to drive_add/drive_del as libvirt now uses blockdev-add/del QMP
> command instead which have implicit error propagation.
> 

As thought.

> There are no replacements for the internal snapshot commands, but they
> reported the 'error' prefix for some time even before this series.
> 
> Said that, please don't break savevm/loadvm/delvm until a QMP
> replacement is added.
> 

Yes, noted. I wonder where userfaultfd write support is these days...

> The bug was reported at the time when libvirt didn't use blockdev yet,
> but at this point it's pointless from our side. This wouldn't even fix
> the scenario when old (pre-5.10) libvirt would use new qemu because the
> drive-add handler never checked the error prefix.
> 
> [1] 
> https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_monitor_text.c;h=9135a11f0a3aae718c86bb199112fba8d16d4d80;hb=HEAD
> 

Thank you for the report from libvirtville :)

--js

Re: Making QEMU easier for management tools and applications

[John Snow]

On 1/27/20 9:35 AM, Kevin Wolf wrote:
> Am 24.01.2020 um 10:50 hat Daniel P. Berrangé geschrieben:
>> On Thu, Jan 23, 2020 at 04:07:09PM -0500, John Snow wrote:
>>> Well, sure. The context of this email was qmp-shell though, which is
>>> meant to help facilitate the entry of JSON commands so that you *can*
>>> indeed just forego the CLI/HMP entirely.
>>>
>>> If you are of the opinion that every user of QEMU should be copy/pasting
>>> JSON straight into a socket and we should delete qmp-shell, that's
>>> certainly a fine opinion.
>>
>> I think part of the pain of qmp-shell comes from the very fact that
>> it is trying to be an interactive shell. This points people towards
>> interactively typing in the commands, which is horrific when you get
>> anywhere near the JSON, or even dot-notation traditional commands.
>>
>> If it was just a qmp-client that was single shot, we'd encourage
>> people to create the JSON in a sensible way - vim/emacs/whatever.
> 
> I don't see how this is sensible. QMP commands are something that I
> reuse even less than VM configurations, so creating a one-off file for
> each would take me a lot more time and I would still have to type the
> same JSON code that I have to type with -qmp stdio.
> 
> The reason it is and should be an interactive shell is that I'm
> interacting with it. Switching back and forth between a text editor and
> a shell to actually send the command to QEMU would make things only even
> more cumbersome than they already are.
> 
>> Bash/dash/zsh/$whatever is their interactive shell, with massively
>> more features than qmp-shell. You have command history, autocomplete,
>> conditional and looping constructs, and everything a normal shell
>> offers.
> 
> If I wanted to program a QMP client, I would use Python. For me,
> conditionals and loops are completely out of scope for a QMP shell. I
> just want an easy way to tell QEMU to do something specific.
> 
> A command history already exists for qmp-shell. It's better than bash
> because it doesn't mix QMP history with whatever else I do on my
> computer.
> 
> Autocomplete in qmp-shell doesn't exist, as far as I know, but if
> implemented, it could be a lot more useful than bash completion because
> it could offer key completion based on the QMP schema.
> 

It does have tab completion for command names, but it does not know
about or remember argument fields. It does not have autocomplete or
typing hints like FiSH or bash ^r.

I would like to change this, actually, by making the docstrings in QAPI
schema a first class citizen of the spec and allowing them to be
introspectable via the socket directly.

(I.e., you can get the list of arguments and the docstrings that
accompany them over the wire so you can display it in the client.)

Problem I'm having with qmp-shell is, like Kevin says below ...

> This is in fact a big part of the problem that qmp-shell really needs to
> solve before it can replace HMP: How to make writing commands at least
> almost as simple as with HMP. If I can just press tab a few times to
> cycle through the available options for the command, that would already
> be a massive improvement over writing JSON manually (which you would
> still have to do with your text-file based approach, without any
> QMP-specific support).
> 

... I can't figure out how to make writing commands simple.

When you have a "simple" command, the abstraction works OK; you can type
key=val pairs and go about your way.

As soon as you have anything nested, the gossamer-thin illusion is
destroyed. I investigated making this a little easier by adding a parser
that could read directly from stdin and would allow multi-line JSON
inputs as arguments.

(Like the python shell does it, for example: When you have a dictionary
opening brace, it lets you continue to the next line.)

I was a little disheartened that most JSON parsers in python expect to
consume buffered objects and generally consume the entire buffer -- it
didn't seem to play nice with the idea of wanting to parse from STDIN
directly.


So:

- I think qmp-shell is worth having, especially if polished
(autocomplete, docstrings, argument hints, etc).

- Kevin mentioned getting this into the GTK shell. I think that would be
great, as a step to help phase out HMP.

- I think getting rid of HMP is good because I don't care for the idea
of supporting two monitor protocols. One schema, one parser, one truth.

- I do, however, like the idea of having a non-rigorous monitor that
lets us get away with murder when we need to. HMP is useful for
debugging, prototypes and other things where the rigor and permanence of
a QAPI schema feels too burdensome.

- So: maybe a turbocharged qmp-shell can offer some similar kinds of
convenience commands that are build on top of real QMP. Sugar logic and
other fanciful things could be implemented there in qmp-shell as
extensions. You'd get a stock pile of them with your QEMU install that
help you do certain tasks quickly and trivially.

- Given

Re: [PATCH] tests/acceptance: Add a test for the canon-a1100 machine

[Philippe Mathieu-Daudé]

Hey Wainer,

On 1/27/20 6:45 PM, Wainer dos Santos Moschetta wrote:

On 1/27/20 1:41 PM, Philippe Mathieu-Daudé wrote:

On 1/27/20 4:39 PM, Thomas Huth wrote:

On 27/01/2020 16.18, Philippe Mathieu-Daudé wrote:

On 1/27/20 3:41 PM, Thomas Huth wrote:

The canon-a1100 machine can be used with the Barebox firmware. The
QEMU Advent Calendar 2018 features a pre-compiled image which we
can use for testing.

Signed-off-by: Thomas Huth 
---
   tests/acceptance/machine_arm_canon-a1100.py | 33 
+



What is the reason for not adding this case in boot_linux_console suite?


Because there are too many tests in this file and it became hardly 
maintainable. Also it is easier to add a 'F:' entry in the MAINTAINERS 
file to each machine section.



   1 file changed, 33 insertions(+)
   create mode 100644 tests/acceptance/machine_arm_canon-a1100.py

diff --git a/tests/acceptance/machine_arm_canon-a1100.py
b/tests/acceptance/machine_arm_canon-a1100.py
new file mode 100644
index 00..3888168451
--- /dev/null
+++ b/tests/acceptance/machine_arm_canon-a1100.py
@@ -0,0 +1,33 @@
+# Functional test that boots the canon-a1100 machine with firmware
+#
+# Copyright (c) 2020 Red Hat, Inc.
+#
+# Author:
+#  Thomas Huth 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado_qemu import Test
+from avocado_qemu import wait_for_console_pattern
+from avocado.utils import archive
+
+class CanonA1100Machine(Test):
+
+    timeout = 90
+
+    def test_arm_canona1100(self):
+    """
+    :avocado: tags=arch:arm
+    :avocado: tags=machine:canon-a1100


To the maintainer taking this, please add:

    :avocado: tags=pflash_cfi02


Should there be a "device:" between the "=" and the device name? At
least I can see some other files using "device:" for similar tags...


Ah yes you are right, it is clearer.



Notice that avocado_qemu won't automatically convert that tag into 
QEMU's -device option, If that is the intention...


That could be useful, but currently my usage is 'avocado run -t 
device:pcnet32' to run all tests using the pcnet32 network device.


I have pflash tests which I plan to use the same way.

This is a hint to other maintainers, who don't have to look at each test 
to find the set of tests that suits them.


IOW "As a maintainer of the device:pflash I'm interested to run all 
tests using this device, and while they pass I won't look at them".


(This is how I expect maintainers to use the acceptance tests when I add 
some).

Re: [PATCH v3 00/13] RFC: [for 5.0]: HMP monitor handlers cleanups

[Peter Krempa]

On Mon, Jan 27, 2020 at 14:39:02 -0500, John Snow wrote:
> 
> 
> On 1/27/20 5:36 AM, Maxim Levitsky wrote:
> > This patch series is bunch of cleanups
> > to the hmp monitor code.
> > 
> > This series only touched blockdev related hmp handlers.
> > 
> > No functional changes expected other that
> > light error message changes by the last patch.
> > 
> > This was inspired by this bugzilla:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1719169
> > 
> > Basically some users still parse hmp error messages,
> > and they would like to have them prefixed with 'Error:'
> > 
> 
> HMP isn't meant to be parsed. It's explicitly *not* API or ABI. I do
> like consistency in my UIs (it's useful for human eyes, too), but I'd
> like to know more about the request.

That's true as long as there's an stable replacement ... see below.

> 
> Is this request coming from libvirt? Can we wean them off of this
> interface? What do they need as a replacement?

There are 5 commands that libvirt still has HMP interfaces for:

drive_add
drive_del

savevm
loadvm
delvm

>From upstream point of view there's no value in adding the 'error'
prefix to drive_add/drive_del as libvirt now uses blockdev-add/del QMP
command instead which have implicit error propagation.

There are no replacements for the internal snapshot commands, but they
reported the 'error' prefix for some time even before this series.

Said that, please don't break savevm/loadvm/delvm until a QMP
replacement is added.

The bug was reported at the time when libvirt didn't use blockdev yet,
but at this point it's pointless from our side. This wouldn't even fix
the scenario when old (pre-5.10) libvirt would use new qemu because the
drive-add handler never checked the error prefix.

[1] 
https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_monitor_text.c;h=9135a11f0a3aae718c86bb199112fba8d16d4d80;hb=HEAD

Re: [PATCH 0/9] more audio fixes and improvements

[Volker Rümelin]

> On 23/01/2020 07:41, Volker Rümelin wrote:
>
>> The first two patches "audio: fix audio_generic_write" and
>> "audio: fix audio_generic_read" are only compile tested. The
>> code is only reachable from the DirectSound backend with the
>> mixing-engine off. I don't know if it is reachable at all.
>> I can't test because I don't have a Windows computer.
>>
>> Volker Rümelin (9):
>>   audio: fix audio_generic_write
>>   audio: fix audio_generic_read
>>   paaudio: remove unused variables
>>   audio: prevent SIGSEGV in AUD_get_buffer_size_out
>>   audio: fix bug 1858488
>>   ossaudio: prevent SIGSEGV in oss_enable_out
>>   ossaudio: prevent SIGPFE in oss_write
>>   ossaudio: disable poll mode can't be reached
>>   audio: audio_generic_get_buffer_in should honor *size
>>
>>  audio/alsaaudio.c |  1 +
>>  audio/audio.c | 77 
>> ++-
>>  audio/audio_int.h |  4 +--
>>  audio/coreaudio.c |  7 +++--
>>  audio/noaudio.c   |  1 +
>>  audio/ossaudio.c  | 28 
>>  audio/paaudio.c   |  6 ++---
>>  audio/sdlaudio.c  |  7 +++--
>>  audio/wavaudio.c  |  1 +
>>  9 files changed, 71 insertions(+), 61 deletions(-)
> Thanks for your patches! I've had reports from some of the PPC emulation guys 
> that
> the switch to the new audio API broke the coreaudio backend on Macs (see
> https://bugs.launchpad.net/qemu/+bug/1859916) and I've pointed them towards 
> this
> patchset, but sadly it still doesn't appear to fix the issue.
>
> Have you seen any issues with the coreaudio backend during your testing?

Hi Mark,

I didn't test coreaudio. I don't have a Mac. One of my patches changes 
coreaudio.c 
because I renamed a function.

With best regards,
Volker

> ATB,
>
> Mark.

Re: Making QEMU easier for management tools and applications

[Dr. David Alan Gilbert]

* Kevin Wolf (kw...@redhat.com) wrote:
> Am 24.01.2020 um 10:50 hat Daniel P. Berrangé geschrieben:
> > On Thu, Jan 23, 2020 at 04:07:09PM -0500, John Snow wrote:
> > > Well, sure. The context of this email was qmp-shell though, which is
> > > meant to help facilitate the entry of JSON commands so that you *can*
> > > indeed just forego the CLI/HMP entirely.
> > > 
> > > If you are of the opinion that every user of QEMU should be copy/pasting
> > > JSON straight into a socket and we should delete qmp-shell, that's
> > > certainly a fine opinion.
> > 
> > I think part of the pain of qmp-shell comes from the very fact that
> > it is trying to be an interactive shell. This points people towards
> > interactively typing in the commands, which is horrific when you get
> > anywhere near the JSON, or even dot-notation traditional commands.
> > 
> > If it was just a qmp-client that was single shot, we'd encourage
> > people to create the JSON in a sensible way - vim/emacs/whatever.
> 
> I don't see how this is sensible. QMP commands are something that I
> reuse even less than VM configurations, so creating a one-off file for
> each would take me a lot more time and I would still have to type the
> same JSON code that I have to type with -qmp stdio.
> 
> The reason it is and should be an interactive shell is that I'm
> interacting with it. Switching back and forth between a text editor and
> a shell to actually send the command to QEMU would make things only even
> more cumbersome than they already are.
> 
> > Bash/dash/zsh/$whatever is their interactive shell, with massively
> > more features than qmp-shell. You have command history, autocomplete,
> > conditional and looping constructs, and everything a normal shell
> > offers.
> 
> If I wanted to program a QMP client, I would use Python. For me,
> conditionals and loops are completely out of scope for a QMP shell. I
> just want an easy way to tell QEMU to do something specific.
> 
> A command history already exists for qmp-shell. It's better than bash
> because it doesn't mix QMP history with whatever else I do on my
> computer.
> 
> Autocomplete in qmp-shell doesn't exist, as far as I know, but if
> implemented, it could be a lot more useful than bash completion because
> it could offer key completion based on the QMP schema.
> 
> This is in fact a big part of the problem that qmp-shell really needs to
> solve before it can replace HMP: How to make writing commands at least
> almost as simple as with HMP. If I can just press tab a few times to
> cycle through the available options for the command, that would already
> be a massive improvement over writing JSON manually (which you would
> still have to do with your text-file based approach, without any
> QMP-specific support).

Doing all that in a python process (i.e. an actual python shell with a
bunch of qemu commands added) seems easyish.

> The other part that it needs to solve is how to be available by default
> without specifying anything on the command line. Basically, if I press
> Ctrl-Alt-2, I want to get to a monitor shell. If that shell is
> implemented internally or by an external Python process, I don't mind.

That is a harder part. (I rarely use Ctrl-Alt-2 actually; I mostly
use HMP on stdin).

Dave

> > The only strong reason for qmp-shell to be interactive would be if
> > the initial protoocl handshake was too slow. I can't see that being
> > a problem with QMP.
> 
> Speed would be the least of my concerns. This is about manual use, and
> it already takes me a while to type in my commands.
> 
> > Example usage:
> > 
> > 1. Launch the QEMU runtime for the desired target
> > 
> >  $ qemu-runtime-x86_64 myvm.sock
> > 
> > 2. Load the configuration to define the VM
> > 
> >$ cat myvm.yaml
> >commands:
> >  - machine_declare:
> >  name: pc-q35-5.0
> >  ...
> >  - blockdev_add:
> >  ...
> >  - device_add:
> >  ...
> >  - blockdev_add:
> >  ...
> >  - device_add:
> >  ...
> >$ qemu-client myvm.sock myvm.yaml
> > 
> > 
> > 3. Hotplug a disk
> > 
> >$ cat mynewdisk.yaml
> >commands:
> >  - blockdev_add:
> >  ...
> >  - device_add:
> >  ...
> >$ qemu-client myvm.sock mynewdisk.yaml
> > 
> > 
> > 3. Hotunplug a disk
> > 
> >$ cat myolddisk.yaml
> >commands:
> >  - device_del:
> >  ...
> >  - blockdev_del:
> >  ...
> >$ qemu-client myvm.sock myolddisk.yaml
> 
> Just to compare, this is what the human user oriented flow looks like
> today:
> 
> 1. qemu-system-x86_64 -M pc-q35-5.0 -drive if=virtio,... -cdrom ...
> 
> 2. 
>(qemu) drive_add ...
>
> 
> 3. 
>(qemu) device_del ...
>
> 
> This is what we're competing with, and honestly I don't see how your
> qemu-runtime-*/qemu-client based flow comes even close to it in terms of
> usability.
> 
> QMP, JSON and YAML may be nice machine interfaces, but having nice
> machine interfaces doesn't mean that 

Re: Making QEMU easier for management tools and applications

[Dr. David Alan Gilbert]

* Kevin Wolf (kw...@redhat.com) wrote:
> Am 25.01.2020 um 11:18 hat Markus Armbruster geschrieben:
> > Kevin Wolf  writes:
> > 
> > > Am 24.01.2020 um 11:27 hat Daniel P. Berrangé geschrieben:
> > >> This is finally something I'd consider to be on a par with the
> > >> original QEMU syntax, before we added hierarchical data. You
> > >> have the minimal possible amount of syntax here. No commas,
> > >> no quotes, no curly brackets, etc.
> > >
> > > This seems to have the same problems as the QEMU command line (how do
> > > you distinguish strings from ints, from bools, from null?).
> > 
> > True: YAML provides only string scalars.
> > 
> > TOML provides strings, integers, floats, booleans, and several flavors
> > of time.  It lacks null.
> > 
> > > It's
> > > basically just a pretty-printed version of it with the consequence that
> > > it needs to be stored in an external file and there is no reasonable way
> > > to keep it in my shell history.
> > 
> > There is a reasonable way to keep it in my file system, though.  I find
> > that decidedly superior.
> 
> That depends a lot on your use case.
> 
> If you have a long-lived production VM that you always run with the same
> configuration, then yes, having a config file for it in the file system
> is what you probably want. Currently, for this case, people directly
> using QEMU tend to write a script that contains the command line. I
> think I do have such scripts somewhere, but their number is very small.
> 
> My common case is short-lived VMs with configurations that change very
> often between QEMU invocations. Here the command line is decidedly
> superior.

I can imagine you could do something similar to gdb's --eval-command
option which lets you pass a command to be executed at startup; gdb's
syntax is a bit painful but it feels like that could be fixed, e.g. I
have:

gdb --eval-command='set pagination off' --eval-command='set confirm no' 
--eval-command='handle SIGPIPE print pas
s nostop' --eval-command='handle SIGBUS print pass nostop' --eval-command=r 
--eval-command='thread apply all bt full'
 --eval-command='info proc mappings' --eval-command='info threads' 
--eval-command=q --args $QEMUTODEBUG "$@"

in a script.

Dave

> Requiring me to create a file in the file system each time and to
> remember deleting it after I'm done feels about as convenient as a *nix
> shell that doesn't accept parameters for commands on the command line,
> but instead requires you to write a one-off script first and then run
> that.
> 
> Kevin
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: Making QEMU easier for management tools and applications

[John Snow]

On 1/27/20 6:56 AM, Kevin Wolf wrote:
> Am 25.01.2020 um 11:18 hat Markus Armbruster geschrieben:
>> Kevin Wolf  writes:
>>
>>> Am 24.01.2020 um 11:27 hat Daniel P. Berrangé geschrieben:
 This is finally something I'd consider to be on a par with the
 original QEMU syntax, before we added hierarchical data. You
 have the minimal possible amount of syntax here. No commas,
 no quotes, no curly brackets, etc.
>>>
>>> This seems to have the same problems as the QEMU command line (how do
>>> you distinguish strings from ints, from bools, from null?).
>>
>> True: YAML provides only string scalars.
>>
>> TOML provides strings, integers, floats, booleans, and several flavors
>> of time.  It lacks null.
>>
>>> It's
>>> basically just a pretty-printed version of it with the consequence that
>>> it needs to be stored in an external file and there is no reasonable way
>>> to keep it in my shell history.
>>
>> There is a reasonable way to keep it in my file system, though.  I find
>> that decidedly superior.
> 
> That depends a lot on your use case.
> 
> If you have a long-lived production VM that you always run with the same
> configuration, then yes, having a config file for it in the file system
> is what you probably want. Currently, for this case, people directly
> using QEMU tend to write a script that contains the command line. I
> think I do have such scripts somewhere, but their number is very small.
> 
> My common case is short-lived VMs with configurations that change very
> often between QEMU invocations. Here the command line is decidedly
> superior.
> 
> Requiring me to create a file in the file system each time and to
> remember deleting it after I'm done feels about as convenient as a *nix
> shell that doesn't accept parameters for commands on the command line,
> but instead requires you to write a one-off script first and then run
> that.
> 
> Kevin
> 

> ./qemu-core <

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Alex Bennée]

Robert Foley  writes:

> Hi Drew,
>
> On Mon, 27 Jan 2020 at 12:27, Andrew Jones  wrote:
>
>> >
>> > I suppose we could check the version of QEMU and use the above
>> > defaults only for earlier versions of QEMU.
>> > This is something we will probably move to aarch64vm.py since it is common.
>>
>> What versions of QEMU do these tests *have* to support? Because we could
>> just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
>> 'max' is indeed the nicest selection for using the same command line on
>> KVM (gicv2 and gicv3 hosts) and TCG.
>
> I believe these test scripts which build/launch the VM have to support
> the older version of QEMU since
> this is the version of QEMU currently used when these VMs are
> launched.  I don't know the history on
> this, but it seems intentional that we use one older/different version
> of QEMU to launch the VM,

Well we defer to the system QEMU as it should be stable. It can be
overridden with the QEMU environment variable which worked well enough
when we only had VMs of one architecture. Perhaps we needs a new way to
say "use the appropriate QEMU from this build"?

> while we test the 'current' build of QEMU inside the VM.
> It also seems like a 'nice to have' to automatically support the
> latest version where we could
> use max as you pointed out.
>
> Thanks & Regards,
> -Rob


--
Alex Bennée

Re: [PULL 111/111] virtiofsd: add some options to the help message

[Dr. David Alan Gilbert]

* Christophe de Dinechin (dinec...@redhat.com) wrote:
> 
> Dr. David Alan Gilbert (git) writes:
> 
> > From: Masayoshi Mizuma 
> >
> > Add following options to the help message:
> > - cache
> > - flock|no_flock
> > - norace
> > - posix_lock|no_posix_lock
> > - readdirplus|no_readdirplus
> > - timeout
> > - writeback|no_writeback
> > - xattr|no_xattr
> >
> > Signed-off-by: Masayoshi Mizuma 
> >
> > dgilbert: Split cache, norace, posix_lock, readdirplus off
> >   into our own earlier patches that added the options
> >
> > Reviewed-by: Dr. David Alan Gilbert 
> > Reviewed-by: Misono Tomohiro 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  tools/virtiofsd/helper.c | 10 +-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> > index f98d8f2eb2..0801cf752c 100644
> > --- a/tools/virtiofsd/helper.c
> > +++ b/tools/virtiofsd/helper.c
> > @@ -148,6 +148,8 @@ void fuse_cmdline_help(void)
> > "-o cache=cache mode. could be one of 
> > \"auto, "
> > "always, none\"\n"
> > "   default: auto\n"
> > +   "-o flock|no_flock  enable/disable flock\n"
> > +   "   default: no_flock\n"
> > "-o log_level=   log level, default to 
> > \"info\"\n"
> > "   level could be one of \"debug, "
> > "info, warn, err\"\n"
> > @@ -163,7 +165,13 @@ void fuse_cmdline_help(void)
> > "   enable/disable readirplus\n"
> > "   default: readdirplus except 
> > with "
> > "cache=none\n"
> > -  );
> > +   "-o timeout=I/O timeout (second)\n"
> 
> s/second/seconds/ ? (Not sure, I'm not a native speaker)

Yes, it should.

Dave

> > +   "   default: depends on cache= 
> > option.\n"
> > +   "-o writeback|no_writeback  enable/disable writeback 
> > cache\n"
> > +   "   default: no_writeback\n"
> > +   "-o xattr|no_xattr  enable/disable xattr\n"
> > +   "   default: no_xattr\n"
> > +   );
> >  }
> >
> >  static int fuse_helper_opt_proc(void *data, const char *arg, int key,
> 
> 
> --
> Cheers,
> Christophe de Dinechin (IRC c3d)
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH v1 12/13] tests/docker: re-enable cross-compiling for x86_64 hosts

[Philippe Mathieu-Daudé]

On Mon, Jan 27, 2020 at 8:38 PM Philippe Mathieu-Daudé
 wrote:
> On 1/24/20 9:40 PM, Alex Bennée wrote:
> > Now we have moved everything around we can re-enable the builds for
> > x86_64. Thanks to the descriptive names we are able to sort out QEMU
> > build and tests build dockerfiles and ensure the correct debian
> > dependencies.
> >
> > Move the fedora, mxe and travis dockerfiles into the x86_64 directory
> > as they cannot be used on other architectures.
> >
> > Signed-off-by: Alex Bennée 
> > ---
> >   .../dockerfiles.x86_64/Makefile.include   | 26 +++
> >   .../debain10-x86_64-qemu-build.docker |  1 +
> >   .../debian10-alpha-build-tests.docker |  1 +
> >   .../debian10-amd64-build-qemu.docker  |  1 +
> >   .../debian10-arm64-build-qemu.docker  |  1 +
> >   .../debian10-armel-build-qemu.docker  |  1 +
> >   .../debian10-armhf-build-qemu.docker  |  1 +
> >   .../debian10-hppa-build-tests.docker  |  1 +
> >   .../debian10-m68k-build-tests.docker  |  1 +
> >   .../debian10-mips-build-qemu.docker   |  1 +
> >   .../debian10-mips64-build-tests.docker|  1 +
> >   .../debian10-mips64el-build-qemu.docker   |  1 +
> >   .../debian10-mipsel-build-qemu.docker |  1 +
> >   .../debian10-native-qemu-build.docker |  1 +
> >   .../debian10-powerpc-build-tests.docker   |  1 +
> >   .../debian10-ppc64-build-tests.docker |  1 +
> >   .../debian10-ppc64el-build-qemu.docker|  1 +
> >   .../debian10-riscv64-build-tests.docker   |  1 +
> >   .../debian10-s390x-build-qemu.docker  |  1 +
> >   .../debian10-sh4-build-tests.docker   |  1 +
> >   .../debian10-sparc64-build-tests.docker   |  1 +
> >   .../debian9-mxe-win32-build-qemu.docker   |  0
> >   .../debian9-mxe-win64-build-qemu.docker   |  0
> >   .../debian9-mxe.docker|  0
> >   .../debian9-tricore-build-tests.docker|  1 +
> >   .../debian9-xtensa-build-tests.docker |  1 +
> >   .../fedora-cris-build-tests.docker|  1 +
> >   .../fedora-i386-build-tests.docker|  1 +
> >   .../travis.docker |  0
> >   29 files changed, 50 insertions(+)
> >   create mode 100644 tests/docker/dockerfiles.x86_64/Makefile.include
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debain10-x86_64-qemu-build.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-alpha-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-amd64-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-arm64-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-armel-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-armhf-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-hppa-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-m68k-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-mips-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-mips64-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-mips64el-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-mipsel-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-native-qemu-build.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-powerpc-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-ppc64-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-ppc64el-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-riscv64-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-s390x-build-qemu.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-sh4-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian10-sparc64-build-tests.docker
> >   rename tests/docker/{dockerfiles.cross => 
> > dockerfiles.x86_64}/debian9-mxe-win32-build-qemu.docker (100%)
> >   rename tests/docker/{dockerfiles.cross => 
> > dockerfiles.x86_64}/debian9-mxe-win64-build-qemu.docker (100%)
> >   rename tests/docker/{dockerfiles => 
> > dockerfiles.x86_64}/debian9-mxe.docker (100%)
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian9-tricore-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/debian9-xtensa-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/fedora-cris-build-tests.docker
> >   create mode 12 
> > tests/docker/dockerfiles.x86_64/fedora-i386-build-tests.docker
> >   rename tests/docker/{docke

Re: [PATCH v1 10/13] tests/docker: add debian10-native-qemu-build

[Philippe Mathieu-Daudé]

On 1/24/20 9:40 PM, Alex Bennée wrote:

This is a new dockerfile which can build the native architecture QEMU
on a Debian 10 based image whatever architecture that may be.

Signed-off-by: Alex Bennée 
---
  .../dockerfiles/debian10-native-qemu-build.docker | 15 +++
  1 file changed, 15 insertions(+)
  create mode 100644 tests/docker/dockerfiles/debian10-native-qemu-build.docker

diff --git a/tests/docker/dockerfiles/debian10-native-qemu-build.docker 
b/tests/docker/dockerfiles/debian10-native-qemu-build.docker
new file mode 100644
index 00..71bd2b1d83
--- /dev/null
+++ b/tests/docker/dockerfiles/debian10-native-qemu-build.docker
@@ -0,0 +1,15 @@
+#
+# Debain Native Build
+#
+# This docker target builds on the Debian Buster base image. It is
+# deliberatly architecture agnostic as it can build on any Debian
+# supported architecture.
+#
+FROM qemu:debian10
+MAINTAINER Alex Bennée 
+
+# We use --arch-only as not all hosts have what's needed to build all
+# the binpkg's that come with QEMU (roms etc).
+RUN apt update && \
+DEBIAN_FRONTEND=noninteractive eatmydata \
+apt build-dep --arch-only -yy qemu



Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

Re: Making QEMU easier for management tools and applications

[John Snow]

On 1/25/20 6:41 AM, Paolo Bonzini wrote:
> On 20/01/20 10:55, Stefan Hajnoczi wrote:
>>>
>>> [1] https://qemu.readthedocs.io/en/latest/interop/live-block-operations.html
>> John and I discussed async events in the past.  qmp-shell currently uses
>> the input() built-in function.  If we modify it with a
>> select(2)/poll(2)-style function that monitors both stdin and the QMP
>> socket then it could print QMP events as soon as they are received.
> 
> I think it should be rewritten using async/await.  A simple example:
> 
> import asyncio
> import sys
> from concurrent.futures import ThreadPoolExecutor
> 
> async def ainput(prompt: str = ""):
> with ThreadPoolExecutor(1, "ainput") as executor:
> return (await asyncio.get_event_loop().run_in_executor(
> executor, sys.stdin.readline
> )).rstrip()
> 
> async def numbers():
> i = 1
> while True:
> print(i)
> i = i + 1
> await asyncio.sleep(1)
> 
> async def main():
> name = await ainput("What's your name? ")
> print("Hello, {}!".format(name))
> 
> asyncio.get_event_loop().create_task(numbers())
> asyncio.get_event_loop().run_until_complete(main())
> 
> This would be a great Summer of Code project.  Even an autocompletion
> interface using readline should be possible.
> 
> Paolo
> 

I wrote an async version at one point; I had problems integrating
asyncio with readline functionality.

--js

Re: [PATCH v1 12/13] tests/docker: re-enable cross-compiling for x86_64 hosts

[Philippe Mathieu-Daudé]

On 1/24/20 9:40 PM, Alex Bennée wrote:

Now we have moved everything around we can re-enable the builds for
x86_64. Thanks to the descriptive names we are able to sort out QEMU
build and tests build dockerfiles and ensure the correct debian
dependencies.

Move the fedora, mxe and travis dockerfiles into the x86_64 directory
as they cannot be used on other architectures.

Signed-off-by: Alex Bennée 
---
  .../dockerfiles.x86_64/Makefile.include   | 26 +++
  .../debain10-x86_64-qemu-build.docker |  1 +
  .../debian10-alpha-build-tests.docker |  1 +
  .../debian10-amd64-build-qemu.docker  |  1 +
  .../debian10-arm64-build-qemu.docker  |  1 +
  .../debian10-armel-build-qemu.docker  |  1 +
  .../debian10-armhf-build-qemu.docker  |  1 +
  .../debian10-hppa-build-tests.docker  |  1 +
  .../debian10-m68k-build-tests.docker  |  1 +
  .../debian10-mips-build-qemu.docker   |  1 +
  .../debian10-mips64-build-tests.docker|  1 +
  .../debian10-mips64el-build-qemu.docker   |  1 +
  .../debian10-mipsel-build-qemu.docker |  1 +
  .../debian10-native-qemu-build.docker |  1 +
  .../debian10-powerpc-build-tests.docker   |  1 +
  .../debian10-ppc64-build-tests.docker |  1 +
  .../debian10-ppc64el-build-qemu.docker|  1 +
  .../debian10-riscv64-build-tests.docker   |  1 +
  .../debian10-s390x-build-qemu.docker  |  1 +
  .../debian10-sh4-build-tests.docker   |  1 +
  .../debian10-sparc64-build-tests.docker   |  1 +
  .../debian9-mxe-win32-build-qemu.docker   |  0
  .../debian9-mxe-win64-build-qemu.docker   |  0
  .../debian9-mxe.docker|  0
  .../debian9-tricore-build-tests.docker|  1 +
  .../debian9-xtensa-build-tests.docker |  1 +
  .../fedora-cris-build-tests.docker|  1 +
  .../fedora-i386-build-tests.docker|  1 +
  .../travis.docker |  0
  29 files changed, 50 insertions(+)
  create mode 100644 tests/docker/dockerfiles.x86_64/Makefile.include
  create mode 12 
tests/docker/dockerfiles.x86_64/debain10-x86_64-qemu-build.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-alpha-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-amd64-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-arm64-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-armel-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-armhf-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-hppa-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-m68k-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-mips-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-mips64-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-mips64el-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-mipsel-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-native-qemu-build.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-powerpc-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-ppc64-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-ppc64el-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-riscv64-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-s390x-build-qemu.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-sh4-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian10-sparc64-build-tests.docker
  rename tests/docker/{dockerfiles.cross => 
dockerfiles.x86_64}/debian9-mxe-win32-build-qemu.docker (100%)
  rename tests/docker/{dockerfiles.cross => 
dockerfiles.x86_64}/debian9-mxe-win64-build-qemu.docker (100%)
  rename tests/docker/{dockerfiles => dockerfiles.x86_64}/debian9-mxe.docker 
(100%)
  create mode 12 
tests/docker/dockerfiles.x86_64/debian9-tricore-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/debian9-xtensa-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/fedora-cris-build-tests.docker
  create mode 12 
tests/docker/dockerfiles.x86_64/fedora-i386-build-tests.docker
  rename tests/docker/{dockerfiles => dockerfiles.x86_64}/travis.docker (100%)

diff --git a/tests/docker/dockerfiles.x86_64/Makefile.include 
b/tests/docker/dockerfiles.x86_64/Makefile.include
new file mode 100644
index 00..6237eb500e
--- /dev/null
+++ b/tests/docker/dockerfiles.x86_64/Makefile.include
@@ -0,0 +1,26 @@
+# -*- Mode: makefile -*-
+#
+# x86_64 Containers
+#
+# This architecture has by far the largest number of cross compilers
+# enabled for it.
+

Re: [PATCH v3 00/13] RFC: [for 5.0]: HMP monitor handlers cleanups

[John Snow]

On 1/27/20 5:36 AM, Maxim Levitsky wrote:
> This patch series is bunch of cleanups
> to the hmp monitor code.
> 
> This series only touched blockdev related hmp handlers.
> 
> No functional changes expected other that
> light error message changes by the last patch.
> 
> This was inspired by this bugzilla:
> https://bugzilla.redhat.com/show_bug.cgi?id=1719169
> 
> Basically some users still parse hmp error messages,
> and they would like to have them prefixed with 'Error:'
> 

HMP isn't meant to be parsed. It's explicitly *not* API or ABI. I do
like consistency in my UIs (it's useful for human eyes, too), but I'd
like to know more about the request.

Is this request coming from libvirt? Can we wean them off of this
interface? What do they need as a replacement?

(Is blockdev not enough?)

--js

Re: [PULL 016/111] virtiofsd: Add options for virtio

[Dr. David Alan Gilbert]

* Christophe de Dinechin (dinec...@redhat.com) wrote:
> 
> Dr. David Alan Gilbert (git) writes:
> 
> > From: "Dr. David Alan Gilbert" 
> >
> > Add options to specify parameters for virtio-fs paths, i.e.
> >
> >./virtiofsd -o vhost_user_socket=/tmp/vhostqemu
> 
> Shouldn't that be --socket-path=/tmp/vhostqemu now?

Oops yes it should, that was the old syntax

> >
> > Signed-off-by: Dr. David Alan Gilbert 
> > Reviewed-by: Misono Tomohiro 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  tools/virtiofsd/fuse_i.h|  1 +
> >  tools/virtiofsd/fuse_lowlevel.c | 11 ---
> >  tools/virtiofsd/helper.c| 14 +++---
> >  3 files changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
> > index bae06992e0..26b1a7da88 100644
> > --- a/tools/virtiofsd/fuse_i.h
> > +++ b/tools/virtiofsd/fuse_i.h
> > @@ -63,6 +63,7 @@ struct fuse_session {
> >  struct fuse_notify_req notify_list;
> >  size_t bufsize;
> >  int error;
> > +char *vu_socket_path;
> >  };
> >
> >  struct fuse_chan {
> > diff --git a/tools/virtiofsd/fuse_lowlevel.c 
> > b/tools/virtiofsd/fuse_lowlevel.c
> > index 8552cfb8af..17e8718283 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.c
> > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > @@ -2115,8 +2115,11 @@ reply_err:
> >  }
> >
> >  static const struct fuse_opt fuse_ll_opts[] = {
> > -LL_OPTION("debug", debug, 1), LL_OPTION("-d", debug, 1),
> > -LL_OPTION("--debug", debug, 1), LL_OPTION("allow_root", deny_others, 
> > 1),
> > +LL_OPTION("debug", debug, 1),
> > +LL_OPTION("-d", debug, 1),
> > +LL_OPTION("--debug", debug, 1),
> > +LL_OPTION("allow_root", deny_others, 1),
> > +LL_OPTION("--socket-path=%s", vu_socket_path, 0),
> >  FUSE_OPT_END
> >  };
> >
> > @@ -2132,7 +2135,9 @@ void fuse_lowlevel_help(void)
> >   * These are not all options, but the ones that are
> >   * potentially of interest to an end-user
> >   */
> > -printf("-o allow_root  allow access by root\n");
> > +printf(
> > +"-o allow_root  allow access by root\n"
> > +"--socket-path=PATH path for the vhost-user socket\n");
> >  }
> >
> >  void fuse_session_destroy(struct fuse_session *se)
> > diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> > index 9333691525..676032e71f 100644
> > --- a/tools/virtiofsd/helper.c
> > +++ b/tools/virtiofsd/helper.c
> > @@ -127,13 +127,13 @@ static const struct fuse_opt conn_info_opt_spec[] = {
> >
> >  void fuse_cmdline_help(void)
> >  {
> > -printf(
> > -"-h   --helpprint help\n"
> > -"-V   --version print version\n"
> > -"-d   -o debug  enable debug output (implies -f)\n"
> > -"-f foreground operation\n"
> > -"-o max_idle_threadsthe maximum number of idle worker 
> > threads\n"
> > -"   allowed (default: 10)\n");
> > +printf("-h   --helpprint help\n"
> > +   "-V   --version print version\n"
> > +   "-d   -o debug  enable debug output (implies 
> > -f)\n"
> > +   "-f foreground operation\n"
> > +   "-o max_idle_threadsthe maximum number of idle 
> > worker "
> > +   "threads\n"
> > +   "   allowed (default: 10)\n");
> >  }
> >
> >  static int fuse_helper_opt_proc(void *data, const char *arg, int key,
> 
> 
> --
> Cheers,
> Christophe de Dinechin (IRC c3d)
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH v2 2/2] doc: Use @code rather than @var for options

[Eric Blake]

On 1/24/20 4:34 AM, David Edmondson wrote:

Texinfo defines @var for metasyntactic variables and such terms are
shown in upper-case or italics in the output of makeinfo. When
considering an option to a command, such as "-n", upper-casing is
undesirable as it may confuse the reader or be in conflict with the
equivalent upper-case option.

Replace the use of @var for options with @code to avoid this.

Signed-off-by: David Edmondson 
---
  qemu-img.texi | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)


Is this patch still needed given Peter's recent push to move to rST 
documentation?




diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6dfd8682..6b4a1ac961 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -74,13 +74,13 @@ keys.
  @item --image-opts
  Indicates that the source @var{filename} parameter is to be interpreted as a
  full option string, not a plain filename. This parameter is mutually
-exclusive with the @var{-f} parameter.
+exclusive with the @code{-f} parameter.
  



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

Re: [PATCH v3 1/4] tests/acceptance: avocado_qemu: Introduce the 'accel' test parameter

[Wainer dos Santos Moschetta]

On 1/24/20 7:36 AM, Philippe Mathieu-Daudé wrote:

On 1/22/20 2:27 AM, Wainer dos Santos Moschetta wrote:

The test case may need to boot the VM with an accelerator that
isn't actually enabled on the QEMU binary and/or present in the host. In
this case the test behavior is undefined, and the best course of
action is to skip its execution.

This change introduced the 'accel' parameter (and the handler of
tag with same name) used to indicate the test case requires a
given accelerator available. It was implemented a mechanism to
skip the test case if the accelerator is not available. Moreover,
  the QEMU -accel argument is set automatically to any VM
launched if the parameter is present.

Signed-off-by: Wainer dos Santos Moschetta 
---
  docs/devel/testing.rst    | 16 
  tests/acceptance/avocado_qemu/__init__.py | 23 +++
  2 files changed, 39 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index ab5be0c729..d17d0e90aa 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -759,6 +759,17 @@ name.  If one is not given explicitly, it will 
either be set to

  ``None``, or, if the test is tagged with one (and only one)
  ``:avocado: tags=machine:VALUE`` tag, it will be set to ``VALUE``.
  +accel
+~
+The accelerator that will be set to all QEMUMachine instances created
+by the test.
+
+The ``accel`` attribute will be set to the test parameter of the same
+name.  If one is not given explicitly, it will either be set to
+``None``, or, if the test is tagged with one (and only one)
+``:avocado: tags=accel:VALUE`` tag, it will be set to ``VALUE``. 
Currently

+``VALUE`` should be either ``kvm`` or ``tcg``.
+
  qemu_bin
  
  @@ -800,6 +811,11 @@ machine
  The machine type that will be set to all QEMUMachine instances created
  by the test.
  +accel
+~
+The accelerator that will be set to all QEMUMachine instances created
+by the test. In case the accelerator is not available (both QEMU
+binary and the host system are checked) then the test is canceled.
    qemu_bin
  
diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py

index 6618ea67c1..c83a75ccbc 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -20,6 +20,7 @@ SRC_ROOT_DIR = 
os.path.join(os.path.dirname(__file__), '..', '..', '..')

  sys.path.append(os.path.join(SRC_ROOT_DIR, 'python'))
    from qemu.machine import QEMUMachine
+from qemu.accel import kvm_available, tcg_available
    def is_readable_executable_file(path):
  return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
@@ -111,6 +112,8 @@ class Test(avocado.Test):
    def setUp(self):
  self._vms = {}
+    # VM argumments that are mapped from parameters
+    self._param_to_vm_args = []
    self.arch = self.params.get('arch',
default=self._get_unique_tag_val('arch'))
@@ -124,10 +127,30 @@ class Test(avocado.Test):
  if self.qemu_bin is None:
  self.cancel("No QEMU binary defined or found in the 
source tree")

  +    self.accel = self.params.get('accel',
+ default=self._get_unique_tag_val('accel'))
+    if self.accel:
+    avail = False
+    if self.accel == 'kvm':
+    if kvm_available(self.arch, self.qemu_bin):
+    avail = True
+    elif self.accel == 'tcg':
+    if tcg_available(self.qemu_bin):
+    avail = True
+    else:
+    self.cancel("Unknown accelerator: %s" % self.accel)
+
+    if avail:
+    self._param_to_vm_args.extend(['-accel', self.accel])
+    else:
+    self.cancel("%s is not available" % self.accel)


Why refuse to test the other accelerators?

Isn't it better to QMP-ask QEMU which accelerator it supports, and 
SKIP if it isn't available?



python/qemu/accel.py:list_accel(qemu_bin) can be used for that end. For 
example:


if self.accel not in list_accel(self.qemu_bin):

    self.cancel("%s is not available" % self.accel)

However checking the support only on QEMU's binary may be very weak 
(take KVM as an example). I implemented checkers for kvm and tcg on 
python/qemu/accel.py. Given that I have zero knowledge on the other 
accelerators,  I simply did not touch on them.


That said, IMHO it needs to implement checkers for those others 
accelerator before they get reliably handled by avocado_qemu 
automatically. And test writers can still run tests with those 
accelerators as long as 'accel' tag is not used.


What do you think Philippe?

Thanks, good point!

- Wainer






+
  def _new_vm(self, *args):
  vm = QEMUMachine(self.qemu_bin, sock_dir=tempfile.mkdtemp())
  if args:
  vm.add_args(*args)
+    if self._param_to_vm_args:
+    vm.add_args(*self._param_to_vm_args)
  return vm
    @property

Re: [PATCH v1 02/13] tests/docker: better handle symlinked libs

[Philippe Mathieu-Daudé]

On 1/24/20 9:40 PM, Alex Bennée wrote:

When we are copying we want to ensure we grab the first
resolution (the found in path section). However even that binary might
be a symlink so lets make sure we chase the symlinks to copy the right
binary to where it can be found.

Signed-off-by: Alex Bennée 
---
  tests/docker/docker.py | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 31d8adf836..7dfca63fe4 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -109,7 +109,7 @@ def _get_so_libs(executable):
  ensure theright data is copied."""
  
  libs = []

-ldd_re = re.compile(r"(/.*/)(\S*)")
+ldd_re = re.compile(r"(?:\S+ => )?(\S*) \(:?0x[0-9a-f]+\)")
  try:
  ldd_output = subprocess.check_output(["ldd", 
executable]).decode('utf-8')
  for line in ldd_output.split("\n"):
@@ -145,7 +145,8 @@ def _copy_binary_with_libs(src, bin_dest, dest_dir):
  if libs:
  for l in libs:
  so_path = os.path.dirname(l)
-_copy_with_mkdir(l, dest_dir, so_path)
+real_l = os.path.realpath(l)
+_copy_with_mkdir(real_l, dest_dir, so_path)
  
  
  def _check_binfmt_misc(executable):




Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

Re: [PULL] RISC-V Patches for the 5.0 Soft Freeze, Part 1

[Palmer Dabbelt]

On Fri, 24 Jan 2020 04:35:14 PST (-0800), Peter Maydell wrote:

On Thu, 23 Jan 2020 at 18:43, Palmer Dabbelt  wrote:

On Thu, 23 Jan 2020 06:38:07 PST (-0800), Peter Maydell wrote:
> Hi. This pull request doesn't seem to be signed with the GPG
> key that I have on record for you...

When I moved to Google I got a Yubikey and made new subkeys for it.  If I
understand correctly the new subkeys should be signed by my main key, but maybe
that didn't make it to your keyring?  I see

$ gpg --list-keys pal...@dabbelt.com
pub   rsa4096 2017-06-06 [SC] [expires: 2027-11-13]
  00CE76D1834960DFCE886DF8EF4CA1502CCBAB41
uid   [ultimate] Palmer Dabbelt 
uid   [ultimate] Palmer Dabbelt 
sub   rsa4096 2017-06-06 [E]
sub   rsa4096 2019-11-26 [S] [expires: 2024-11-24]
sub   rsa4096 2019-11-26 [A] [expires: 2024-11-24]
sub   rsa4096 2019-11-26 [E] [expires: 2024-11-24]


Yeah, I have those. I think I must have fumbled something
when I retried the pullreq after doing a refresh of your
gpg key, because I just did a retry now and it's fine.
(I'm just running the pull through my tests now.)


Thanks!

Re: [PULL 00/11] target/hppa patch queue

[Richard Henderson]

On 1/27/20 11:01 AM, Richard Henderson wrote:
> Version 4 fixes trivial conflicts with 
> 
> commit 4f67d30b5e74e060b8dbe10528829b47345cd6e8
> Author: Marc-André Lureau 
> Date:   Fri Jan 10 19:30:32 2020 +0400
> 
> qdev: set properties with device_class_set_props()

Ho hum, missed "PULL" in the subject.


r~



> The following changes since commit 105b07f1ba462ec48b27e5cb74ddf81c6a79364c:
> 
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200127' into 
> staging (2020-01-27 13:02:36 +)
> 
> are available in the Git repository at:
> 
>   https://github.com/rth7680/qemu.git tags/pull-pa-20200127
> 
> for you to fetch changes up to b1af755c33bf0d690553a5ccd93689dfd15a98e8:
> 
>   target/hppa: Allow, but diagnose, LDCW aligned only mod 4 (2020-01-27 
> 10:49:51 -0800)
> 
> 
> Improve LASI emulation
> Add Artist graphics
> Fix main memory allocation
> Improve LDCW emulation wrt real hw
> 
> 
> Helge Deller (3):
>   hw/hppa/dino.c: Improve emulation of Dino PCI chip
>   hppa: Add support for LASI chip with i82596 NIC
>   hppa: Switch to tulip NIC by default
> 
> Philippe Mathieu-Daudé (3):
>   hw/hppa/machine: Correctly check the firmware is in PDC range
>   hw/hppa/machine: Restrict the total memory size to 3GB
>   hw/hppa/machine: Map the PDC memory region with higher priority
> 
> Richard Henderson (1):
>   target/hppa: Allow, but diagnose, LDCW aligned only mod 4
> 
> Sven Schnelle (4):
>   ps2: accept 'Set Key Make and Break' commands
>   hppa: add emulation of LASI PS2 controllers
>   seabios-hppa: update to latest version
>   hppa: Add emulation of Artist graphics
> 
>  hw/hppa/hppa_hardware.h|1 +
>  hw/hppa/hppa_sys.h |2 +
>  hw/net/i82596.h|   55 ++
>  include/hw/input/lasips2.h |   16 +
>  include/hw/input/ps2.h |1 +
>  include/hw/net/lasi_82596.h|   29 +
>  target/hppa/helper.h   |2 +
>  hw/display/artist.c| 1454 
> 
>  hw/hppa/dino.c |   97 ++-
>  hw/hppa/lasi.c |  368 ++
>  hw/hppa/machine.c  |   33 +-
>  hw/input/lasips2.c |  291 
>  hw/input/ps2.c |   15 +
>  hw/net/i82596.c|  734 
>  hw/net/lasi_i82596.c   |  188 ++
>  target/hppa/op_helper.c|9 +
>  target/hppa/translate.c|   15 +-
>  tests/qtest/boot-serial-test.c |3 +-
>  MAINTAINERS|4 +-
>  hw/display/Kconfig |4 +
>  hw/display/Makefile.objs   |1 +
>  hw/display/trace-events|9 +
>  hw/hppa/Kconfig|3 +
>  hw/hppa/Makefile.objs  |2 +-
>  hw/hppa/trace-events   |   10 +
>  hw/input/Kconfig   |3 +
>  hw/input/Makefile.objs |1 +
>  hw/input/trace-events  |5 +
>  hw/net/Kconfig |7 +
>  hw/net/Makefile.objs   |2 +
>  hw/net/trace-events|   13 +
>  pc-bios/hppa-firmware.img  |  Bin 783724 -> 766136 bytes
>  roms/seabios-hppa  |2 +-
>  33 files changed, 3351 insertions(+), 28 deletions(-)
>  create mode 100644 hw/net/i82596.h
>  create mode 100644 include/hw/input/lasips2.h
>  create mode 100644 include/hw/net/lasi_82596.h
>  create mode 100644 hw/display/artist.c
>  create mode 100644 hw/hppa/lasi.c
>  create mode 100644 hw/input/lasips2.c
>  create mode 100644 hw/net/i82596.c
>  create mode 100644 hw/net/lasi_i82596.c
> 

Re: Integrating QOM into QAPI

[Christophe de Dinechin]

> On 26 Jan 2020, at 16:04, Peter Maydell  wrote:
> 
> On Sun, 26 Jan 2020 at 08:10, Christophe de Dinechin
>  wrote:
>> I’m still puzzled as to why anybody would switch to something like
>> GObject when there is C++.
> 
> I'm fairly strongly against using C++.

Just to be clear, so am I ;-)

> C++'s language design
> is an "everything including the kitchen sink, lots of "this
> is here for back compat but it's a bear trap", lots of new
> stuff arriving all the time.

Actually, the new stuff is not that bad, overall.

I do agree C++ is an overly complicated language, and that in
practice, there is zero chance of qemu moving to it. But that does
not invalidate my point that creating a class in C++ is easier
than creating a class in any C-based macro-heavy reinvention
of basic OO concepts.

(I write this after having read Paolo’s response, which points
out IMO better reasons for GObject, which I will discuss there).

> It's just too big to keep in
> your head all at once. C has its faults, absolutely, but at
> least it tries to be a reasonably sized vaguely coherent
> language.
> 
> You'd have more luck persuading me we should move to Rust:
> at least then we'd get some clear benefits (no more buffer
> overrun security bugs) for the upheaval :-)

This is largely a myth as soon as you need to do “your own stuff”.
Example: CVE-2019-18960, https://seclists.org/oss-sec/2019/q4/141.

> 
> thanks
> -- PMM
> 

Re: Integrating QOM into QAPI

[Christophe de Dinechin]

> On 26 Jan 2020, at 10:11, Marc-André Lureau  
> wrote:
> 
> Hi
> 
> On Sun, Jan 26, 2020 at 9:10 AM Christophe de Dinechin
>  wrote:
>> 
>> 
>> 
>>> On 21 Jan 2020, at 16:11, Marc-André Lureau  
>>> wrote:
>>> 
>>> Hi
>>> 
>>> On Tue, Jan 21, 2020 at 7:01 PM Markus Armbruster  wrote:
 
 Daniel P. Berrangé  writes:
 
> On Tue, Jan 21, 2020 at 02:36:17PM +0100, Markus Armbruster wrote:
>> Marc-André Lureau  writes:
>> 
>>> Hi
>>> 
>>> On Tue, Jan 21, 2020 at 3:32 PM Stefan Hajnoczi  
>>> wrote:
 
 On Tue, Jan 21, 2020 at 06:42:47AM +0100, Markus Armbruster wrote:
> Stefan Hajnoczi  writes:
> 
>> On Wed, Jan 15, 2020 at 01:15:17PM +0100, Markus Armbruster wrote:
>>> Christophe de Dinechin  writes:
> On 15 Jan 2020, at 10:20, Markus Armbruster  
> wrote:
>>> * qemuMonitorJSONSetIOThread() uses it to control iothread's 
>>> properties
>>> poll-max-ns, poll-grow, poll-shrink.  Their use with -object is
>>> documented (in qemu-options.hx), their use with qom-set is not.
>> 
>> I'm happy to use a different interface.
>> 
>> Writing a boilerplate "iothread-set-poll-params" QMP command in C 
>> would
>> be a step backwards.
> 
> No argument.
> 
>> Maybe the QAPI code generator could map something like this:
>> 
>> { 'command': 'iothread-set-poll-params',
>>   'data': {
>>   'id': 'str',
>>   '*max-ns': 'uint64',
>>   '*grow': 'uint64',
>>   '*shrink': 'uint64'
>>   },
>>   'map-to-qom-set': 'IOThread'
>> }
>> 
>> And turn it into QOM accessors on the IOThread object.
> 
> I think a generic "set this configuration to that value" command is 
> just
> fine.  qom-set fails on several counts, though:
> 
> * Tolerable: qom-set is not actually generic, it applies only to QOM.
> 
> * qom-set lets you set tons of stuff that is not meant to be changed 
> at
> run time.  If it breaks your guest, you get to keep the pieces.
> 
> * There is virtually no documentation on what can be set to what 
> values,
> and their semantics.
> 
> In its current state, QOM is a user interface superfund site.
 
 Thoughts about a solution:
 
 Static QOM properties should be declared via QAPI instead of
 imperatively via QOM APIs.  That way they are introspectable and type
 information is present in the schema.
 
 The QAPI code generator could emit a function that is callable from
 .class_init().  This eliminates the need to manually call
 object_class_property_add().
>> 
>> We need to make up our minds what exactly we want generated.  Then we
>> can design the QAPI language, and code up the generator.
>> 
>> Skeleton QOM type, to help with the discussion:
>> 
>>   #define TYPE_FOO "foo"
>> 
>>   #define FOO(obj) OBJECT_CHECK(Foo, (obj), TYPE_FOO)
>>   #define FOO_CLASS(klass) \
>>   OBJECT_CLASS_CHECK(FooClass, (klass), TYPE_FOO)
>>   #define FOO_GET_CLASS(obj) \
>>   OBJECT_GET_CLASS(FooClass, (obj), TYPE_FOO)
>> 
>>   typedef FooClass {
>>   ParentClass parent_class;
>>   ... // hand-written per-class state
>>   }
>> 
>>   struct Chardev {
>>   ParentObject parent_obj;
>>   ... // hand-written instance (per-object) state
>>   };
>> 
>>   static const TypeInfo char_type_info = {
>>   .name = TYPE_FOO,
>>   .parent = TYPE_OBJECT,
>>   .instance_size = sizeof(Foo),
>>   .instance_init = ...,   // methods to initialize
>>   .instance_post_init = ...,  // and finalize instances,
>>   .instance_finalize = ...,   // all optional
>>   .abstract = ...,// true or false (d'oh)
>>   .class_size = sizeof(FooClass),
>>   .class_init = ...,  // methods to initialize
>>   .class_base_init = ..., // classes, optional
>>   .class_data = ...,  // extra argument for them
>>   .interfaces = ...
>>   };
>> 
>> There's substantial boilerplate, with plenty of hand-written code in the
>> gaps.  What of the boilerplate do we plan to generate?  How do we plan
>> to fill the gaps, if any?
> 
> FWIW, even without a QOM generator, we can do waaay better on the
> amount of boilerplate needed for QOM without very much work. It just
> needs a few convenience macros writing.
> 
> QOM is not GObject, but is heavily inspired by it and so looking at
> GObject gives us a

Re: Integrating QOM into QAPI

[Christophe de Dinechin]

> On 26 Jan 2020, at 17:47, Paolo Bonzini  wrote:
> 
> On 26/01/20 10:11, Marc-André Lureau wrote:
>>> I’m still puzzled as to why anybody would switch to something like
>>> GObject when there is C++.
>> C++ is another level of complexity.
>> 
>> Replacing QOM with GObject would mainly bring us a more solid type
>> system with better tooling/features, gobject-introspection support,
>> and remove the burden of having our own OO from QEMU code base.
> 
> In fact, C++ doesn't solve any of the problems that either QOM or
> GObject try to solve.  (Neither does Rust for that matter).

It does not solve all of them _easily_. I do believe that any solution
to these problems can be engineered in C++, if only because C++
is essentially a superset of C. The question is whether the result can
be made easier / safer to use and generally more elegant. I believe
that the answer is yes, see proof at end of mail.

However, before going further, glib offers way more than gobject.
So there’s that too… And I’m not saying migrating to C++ is a good
idea, I’m just trying to evaluate the various options fairly.


> Nevertheless, there is no stupid question, only stupid answers, and I
> think Christophe's remark is an example of a common misconception.  In
> the hope of not making this a stupid answer, let my try to formulate
> succinctly what I think the differences are between QOM, GObject and the
> C++ object model:

Thank you for this remarkably non-stupid answer ;-) You have really
isolated, in very clear terms, the essence of the discussion.

> 
> - the C++ object model (at least "old-style" C++ with virtual functions
> and the like) provides you with _the intersection_ of what QOM and
> GObject try to solve.  This is what Marc-André calls "OO", and it's
> essentially virtual functions and dynamic casts.  It's a relatively
> small part of both QOM and GObject, and unfortunately a wheel that
> almost every large C program ends up reinventing.

This was the part I was pointing to in my initial comment.

C++ solves that basic “OO” stuff well, and goes a little beyond QOM
or GObject in offering IMO many other benefits, e.g. wrt type safety.

> 
> - Marc-André also described above what GObject provides: a fully
> introspectable type system and the tools so that _libraries_ can define
> _types that will be used from multiple programming languages_.

This kind of things has existed for a very long time. CORBA dates back
to 1991. Also, I’m not sure how important multiple programming languages
are for the qemu use-case. I believe that what matters more is remotely
accessible objects (e.g. over a socket), which in turn makes it almost
trivial to call from another language as long as you accept some kind
of serialization / deserialization process along the way.

GObject only seems marginally better for the “in same process” use case,
in the sense that it makes “objects” that can be used from any language,
indeed, but at the cost of being somewhat foreign and weird in all languages,
including C.

Look at the GClosure marshalling, for example, and compare it with the
example I give you at end of this email, and you tell me which one looks
easier and safer to use.

> 
> - QOM also provides a fully introspectable type system, but with a
> different focus: it's so that _objects_ can expose _properties that will
> be accessed from multiple channels_.

Exposing the properties and making them introspectable are the
fundamental feature we need to discuss. So agreement here.

What you have not explained to my satisfaction
yet is how GObject is a better starting point for re-creating a new
externally-accessible API than some kind of wrapper built in C++
and taking advantage of modern C++ features.


> Everything else in both GObject and QOM follows from this core purpose,
> and the differences between the two follow from the differences.  For
> example:
> 
> - GObject's focus on multiple programming languages:
> gobject-introspection, GClosure, support for non-object types (scalar
> and GBoxed)

How much of that is actually useful to create a new usable qemu API?

> 
> - QOM's focus on objects: dynamic properties, object tree, all types are
> classes

That, to me, looks fundamental, since not having it would require
a total re-architecture of the rest of qemu.

But it looks also somewhat trivial to implement in C++.
For example, obj[“id”] could return a PropertyAccessor
that lets you read or write the object knowing that it is of type
T, so you could write:

if (my_object[“id”] < 3) // Automatically checks the type

or 

my_object[“id”] = 42;

The latter would call PropertyAccessor::operator=(int), which
in turn would check if property “id” exists in my_object, if it has type
“int”, and so on.

Implementation-wise, a simple std::map of BaseProperty pointers,
where each would be an instance of some

template
class Property : public BaseProperty
{
operator T(); // get
T& op

[PATCH 00/11] target/hppa patch queue

[Richard Henderson]

Version 4 fixes trivial conflicts with 

commit 4f67d30b5e74e060b8dbe10528829b47345cd6e8
Author: Marc-André Lureau 
Date:   Fri Jan 10 19:30:32 2020 +0400

qdev: set properties with device_class_set_props()


r~


The following changes since commit 105b07f1ba462ec48b27e5cb74ddf81c6a79364c:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200127' into 
staging (2020-01-27 13:02:36 +)

are available in the Git repository at:

  https://github.com/rth7680/qemu.git tags/pull-pa-20200127

for you to fetch changes up to b1af755c33bf0d690553a5ccd93689dfd15a98e8:

  target/hppa: Allow, but diagnose, LDCW aligned only mod 4 (2020-01-27 
10:49:51 -0800)


Improve LASI emulation
Add Artist graphics
Fix main memory allocation
Improve LDCW emulation wrt real hw


Helge Deller (3):
  hw/hppa/dino.c: Improve emulation of Dino PCI chip
  hppa: Add support for LASI chip with i82596 NIC
  hppa: Switch to tulip NIC by default

Philippe Mathieu-Daudé (3):
  hw/hppa/machine: Correctly check the firmware is in PDC range
  hw/hppa/machine: Restrict the total memory size to 3GB
  hw/hppa/machine: Map the PDC memory region with higher priority

Richard Henderson (1):
  target/hppa: Allow, but diagnose, LDCW aligned only mod 4

Sven Schnelle (4):
  ps2: accept 'Set Key Make and Break' commands
  hppa: add emulation of LASI PS2 controllers
  seabios-hppa: update to latest version
  hppa: Add emulation of Artist graphics

 hw/hppa/hppa_hardware.h|1 +
 hw/hppa/hppa_sys.h |2 +
 hw/net/i82596.h|   55 ++
 include/hw/input/lasips2.h |   16 +
 include/hw/input/ps2.h |1 +
 include/hw/net/lasi_82596.h|   29 +
 target/hppa/helper.h   |2 +
 hw/display/artist.c| 1454 
 hw/hppa/dino.c |   97 ++-
 hw/hppa/lasi.c |  368 ++
 hw/hppa/machine.c  |   33 +-
 hw/input/lasips2.c |  291 
 hw/input/ps2.c |   15 +
 hw/net/i82596.c|  734 
 hw/net/lasi_i82596.c   |  188 ++
 target/hppa/op_helper.c|9 +
 target/hppa/translate.c|   15 +-
 tests/qtest/boot-serial-test.c |3 +-
 MAINTAINERS|4 +-
 hw/display/Kconfig |4 +
 hw/display/Makefile.objs   |1 +
 hw/display/trace-events|9 +
 hw/hppa/Kconfig|3 +
 hw/hppa/Makefile.objs  |2 +-
 hw/hppa/trace-events   |   10 +
 hw/input/Kconfig   |3 +
 hw/input/Makefile.objs |1 +
 hw/input/trace-events  |5 +
 hw/net/Kconfig |7 +
 hw/net/Makefile.objs   |2 +
 hw/net/trace-events|   13 +
 pc-bios/hppa-firmware.img  |  Bin 783724 -> 766136 bytes
 roms/seabios-hppa  |2 +-
 33 files changed, 3351 insertions(+), 28 deletions(-)
 create mode 100644 hw/net/i82596.h
 create mode 100644 include/hw/input/lasips2.h
 create mode 100644 include/hw/net/lasi_82596.h
 create mode 100644 hw/display/artist.c
 create mode 100644 hw/hppa/lasi.c
 create mode 100644 hw/input/lasips2.c
 create mode 100644 hw/net/i82596.c
 create mode 100644 hw/net/lasi_i82596.c

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Robert Foley]

Hi Drew,

On Mon, 27 Jan 2020 at 12:27, Andrew Jones  wrote:

> >
> > I suppose we could check the version of QEMU and use the above
> > defaults only for earlier versions of QEMU.
> > This is something we will probably move to aarch64vm.py since it is common.
>
> What versions of QEMU do these tests *have* to support? Because we could
> just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
> 'max' is indeed the nicest selection for using the same command line on
> KVM (gicv2 and gicv3 hosts) and TCG.

I believe these test scripts which build/launch the VM have to support
the older version of QEMU since
this is the version of QEMU currently used when these VMs are
launched.  I don't know the history on
this, but it seems intentional that we use one older/different version
of QEMU to launch the VM,
while we test the 'current' build of QEMU inside the VM.
It also seems like a 'nice to have' to automatically support the
latest version where we could
use max as you pointed out.

Thanks & Regards,
-Rob

Re: [qemu-web PATCH v2] Add "Security Process" information to the main website

[Paolo Bonzini]

On 27/01/20 11:00, Thomas Huth wrote:
> On 23/01/2020 20.43, Eric Blake wrote:
>> On 1/23/20 11:11 AM, Thomas Huth wrote:
>>> One reporter of a security issue recently complained that it might not
>>> be the best idea to store our "Security Process" in the Wiki. Well, while
>>> the page in the Wiki is protected (so that only some few people can edit
>>> it), it is still possible that someone might find a bug in the Wiki
>>> software to alter the page contents...
>>> Anyway, it looks more trustworthy if we present the "Security Process"
>>> information in the static website instead. Thus this patch adds the
>>> information from the wiki to the Jekyll-based website now.
>>>
>>> Signed-off-by: Thomas Huth 
>>> ---
>>>   v2: Improved some sentences as suggested by Paolo
>>>
>>
>>> +### Publication embargo
>>> +
>>> +As a security issue reported, that is not already publically disclosed
>>
>> publicly
>>
>>> +elsewhere, has an embargo date assigned and communicated to reporter.
>>> Embargo
>>
>> Reads awkwardly. I'd suggest:
>>
>> If a security issue is reported that is not already publicly disclosed,
>> an embargo date may be assigned and communicated to the reporter.
> 
> Ok, thanks, I've added your suggestions and pushed the changes now to
> the website.
> 
> To the people on CC: ... could someone please update the wiki page
> (https://wiki.qemu.org/SecurityProcess) to point to
> https://www.qemu.org/contribute/security-process/ instead? ... I don't
> have write access to that page, so I can not do that on my own.

Done, I will add a server-side redirect when I have some time.

Paolo

Re: [PATCH v6 2/2] s390: diagnose 318 info reset and migration support

[Collin Walling]

On 1/27/20 1:21 PM, Collin Walling wrote:
> On 1/27/20 12:55 PM, David Hildenbrand wrote:
>> On 27.01.20 18:29, Cornelia Huck wrote:
>>> On Mon, 27 Jan 2020 18:09:11 +0100
>>> David Hildenbrand  wrote:
>>>
>>> +static void s390_diag318_reset(DeviceState *dev)
>>> +{
>>> +if (kvm_enabled())
>>> +kvm_s390_set_diag318_info(0);
>>> +}
>>> +
>>> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +dc->reset = s390_diag318_reset;
>>> +dc->vmsd = &vmstate_diag318;
>>> +dc->hotpluggable = false;
>>> +/* Reason: Created automatically during machine instantiation */
>>> +dc->user_creatable = false;
>>> +}
>>> +
>>> +static const TypeInfo s390_diag318_info = {
>>> +.class_init = s390_diag318_class_init,
>>> +.parent = TYPE_DEVICE,
>>> +.name = TYPE_S390_DIAG318,
>>> +.instance_size = sizeof(DIAG318State),
>>> +};
>>> +
>>> +static void s390_diag318_register_types(void)
>>> +{
>>> +type_register_static(&s390_diag318_info);
>>> +}  
>>
>> Do we really need a new device? Can't we simply glue that extended state
>> to the machine state?
>>  
>> -> target/s390x/machine.c  
>>  
>
> Those VM States relate to the CPU state... does it make sense to store the
> diag318 info in a CPU state? (It doesn't seem necessary to store / migrate
> this info for each CPU).  

 I'm sorry, I was looking at the wrong file ...

>
> Should we store this in the S390CcwMachineState? Or perhaps create a 
> generic
> S390MachineState for information that needs to be stored once and migrated
> once?  

 ... I actually thought we have something like this already. Personally,
 I think that would make sense. At least spapr seems to have something
 like this already (hw/ppc/spapr.c:spapr_machine_init().

 @Conny?
>>>
>>> What are you referring to? I only see the one with the FIXME in front
>>> of it...
>>
>> That's the one I mean. The fixme states something about qdev ... but
>> AFAIK that's only applicable if TYPE_DEVICE is involved. So maybe right
>> now there is no other way than registering the vmstate directly.
>>
> 
> Hmm okay. I'll take a look at how spapr does it. I think I've registered a
> vmstate via register_savevm_live() in an earlier version, but had difficulties
> figuring out where to store the data. I'll revisit this approach.
> 
> Thanks for the feedback!
> 

Err perhaps not entirely in this manner...

docs/devel/migration.rst declares the register_savevm_live() function as the
"legacy way" of doing things. I'll have to see how other VMStateDescriptions
are modeled. I think vmstate_register() is what I want.

Sorry for the confusion.

-- 
Respectfully,
- Collin Walling

Re: [GSoC/Outreachy QEMU project proposal] Measure and Analyze QEMU Performance

[Wainer dos Santos Moschetta]

On 1/21/20 12:07 PM, Aleksandar Markovic wrote:

On Mon, Jan 20, 2020 at 3:51 PM Stefan Hajnoczi  wrote:

On Sat, Jan 18, 2020 at 03:08:37PM +0100, Aleksandar Markovic wrote:

3) The community will be given all devised performance measurement methods in 
the form of easily reproducible step-by-step setup and execution procedures.

Tracking performance is a good idea and something that has not been done
upstream yet.

Thanks for the interest, Stefan!


  A few questions:

  * Will benchmarks be run automatically (e.g. nightly or weekly) on
someone's hardware or does every TCG architecture maintainer need to
run them manually for themselves?

If the community wants it, definitely yes. Once the methodology is
developed, it should be straightforward to setup nightly and/or weekly
benchmarks - that could definitely include sending mails with reports
to the entire list or just individuals or subgroups. The recipient
choice is just a matter or having decent criteria about
appropriateness of information within the message (e.g. not to flood
the list with the data most people are not really interested).

For linux-user tests, they are typically very quick, and nightly tests
are quite feasible to run. On someone hardware, of course, and
consistently always on the same hardware, if possible. If it makes
sense, one could setup multiple test beds with a variety of hardware
setups.

For system mode tests, I knoe they are much more difficult to
automate, and, on top of that, there could be greater risk of
hangs/crashes Also, considering the number of machines we support,
those tests could consume much more time - perhaps even one day would
not be sufficient, if we have many machines and boot/shutdown
variants. For these reason, perhaps weekly executions would be more
appropriate for them, and, in general, given greater complexity, the
expectation from system-mode performance tests should be better kept
quite low for now.


  * Where will the benchmark result history be stored?


If emailing is set up, the results could be reconstructed from emails.
But, yes, it would be better if the result history is kept somewhere
on an internet-connected file server



If you eventually choose Gitlab CI for weekly/nightly executions then 
results can be simply archived [1].


Also it can be attached machines in Gitlab CI then running the 
system-mode experiment always on same environment.


[1] https://docs.gitlab.com/ee/user/project/pipelines/job_artifacts.html

IMHO, it is a very good GSoC proposal.

- Wainer



Yours,
Aleksandar


Stefan

Re: [PATCH v1 12/13] tests/docker: re-enable cross-compiling for x86_64 hosts

[Alex Bennée]

Richard Henderson  writes:

> On 1/24/20 10:40 AM, Alex Bennée wrote:
>> +../dockerfiles.cross/fedora-i386-build-qemu.docker
>> \ No newline at end of file
>
> Lots of no trailing newlines.  Probably not intentional?

I think that's just a vagary of the git symlink representation.


-- 
Alex Bennée

Re: [PATCH v6 2/2] s390: diagnose 318 info reset and migration support

[Collin Walling]

On 1/27/20 12:55 PM, David Hildenbrand wrote:
> On 27.01.20 18:29, Cornelia Huck wrote:
>> On Mon, 27 Jan 2020 18:09:11 +0100
>> David Hildenbrand  wrote:
>>
>> +static void s390_diag318_reset(DeviceState *dev)
>> +{
>> +if (kvm_enabled())
>> +kvm_s390_set_diag318_info(0);
>> +}
>> +
>> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
>> +{
>> +DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +dc->reset = s390_diag318_reset;
>> +dc->vmsd = &vmstate_diag318;
>> +dc->hotpluggable = false;
>> +/* Reason: Created automatically during machine instantiation */
>> +dc->user_creatable = false;
>> +}
>> +
>> +static const TypeInfo s390_diag318_info = {
>> +.class_init = s390_diag318_class_init,
>> +.parent = TYPE_DEVICE,
>> +.name = TYPE_S390_DIAG318,
>> +.instance_size = sizeof(DIAG318State),
>> +};
>> +
>> +static void s390_diag318_register_types(void)
>> +{
>> +type_register_static(&s390_diag318_info);
>> +}  
>
> Do we really need a new device? Can't we simply glue that extended state
> to the machine state?
>  
> -> target/s390x/machine.c  
>  

 Those VM States relate to the CPU state... does it make sense to store the
 diag318 info in a CPU state? (It doesn't seem necessary to store / migrate
 this info for each CPU).  
>>>
>>> I'm sorry, I was looking at the wrong file ...
>>>

 Should we store this in the S390CcwMachineState? Or perhaps create a 
 generic
 S390MachineState for information that needs to be stored once and migrated
 once?  
>>>
>>> ... I actually thought we have something like this already. Personally,
>>> I think that would make sense. At least spapr seems to have something
>>> like this already (hw/ppc/spapr.c:spapr_machine_init().
>>>
>>> @Conny?
>>
>> What are you referring to? I only see the one with the FIXME in front
>> of it...
> 
> That's the one I mean. The fixme states something about qdev ... but
> AFAIK that's only applicable if TYPE_DEVICE is involved. So maybe right
> now there is no other way than registering the vmstate directly.
> 

Hmm okay. I'll take a look at how spapr does it. I think I've registered a
vmstate via register_savevm_live() in an earlier version, but had difficulties
figuring out where to store the data. I'll revisit this approach.

Thanks for the feedback!

-- 
Respectfully,
- Collin Walling

Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

[Paolo Bonzini]

On 27/01/20 14:26, Olaf Hering wrote:
>> That's years away, so ideally libxl would have migrated away from
>> xenfv before that.  For now, sticking to a fixed version as in
>> Olaf's patch is a good stopgap measure.
> Is there a way to inspect a running qemu process to see what version
> it is? I assume one thing is to poke at /proc/$PID/cmdline and make
> some guesses. Would a running qemu report what pc-i440fx it supports?

Yes, via QMP. For example on QEMU 3.1 with "-M pc" you would get:

{"execute":"qom-get",
 "arguments":{"path":"/machine", "property":"type"}}
{"return": "pc-i440fx-3.1-machine"}

So libxl would start QEMU with "-M pc,accel=xen -device xen-platform"
when _not_ migrating, but on the destination of live migration it would
query the machine type and use "-Mpc-i440fx-3.1,accel=xen -device
xen-platform".

A cleaner possibility is to do {"execute": "query-machines"} and search
the result for an entry like

{"hotpluggable-cpus": true,
 "name": "pc-i440fx-3.1", "is-default": true, "cpu-max": 255,
 "alias": "pc"}

i.e. the name corresponding to the entry with "alias": "pc" would be
used on the destination.

Thanks,

Paolo

> With such info an enlightened libxl might be able construct a
> compatible commandline for the receiving host.

[PULL 13/13] iscsi: Don't access non-existent scsi_lba_status_descriptor

[Kevin Wolf]

In iscsi_co_block_status(), we may have received num_descriptors == 0
from the iscsi server. Therefore, we can't unconditionally access
lbas->descriptors[0]. Add the missing check.

Signed-off-by: Kevin Wolf 
Reviewed-by: Felipe Franciosi 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: John Snow 
Reviewed-by: Peter Lieven 
---
 block/iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index cbd57294ab..c8feaa2f0e 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -753,7 +753,7 @@ retry:
 }
 
 lbas = scsi_datain_unmarshall(iTask.task);
-if (lbas == NULL) {
+if (lbas == NULL || lbas->num_descriptors == 0) {
 ret = -EIO;
 goto out_unlock;
 }
-- 
2.20.1

[PULL 07/13] block/backup-top: Don't acquire context while dropping top

[Kevin Wolf]

From: Sergio Lopez 

All paths that lead to bdrv_backup_top_drop(), except for the call
from backup_clean(), imply that the BDS AioContext has already been
acquired, so doing it there too can potentially lead to QEMU hanging
on AIO_WAIT_WHILE().

An easy way to trigger this situation is by issuing a two actions
transaction, with a proper and a bogus blockdev-backup, so the second
one will trigger a rollback. This will trigger a hang with an stack
trace like this one:

 #0  0x7fb680c75016 in __GI_ppoll (fds=0x55e74580f7c0, nfds=1, 
timeout=,
 timeout@entry=0x0, sigmask=sigmask@entry=0x0) at 
../sysdeps/unix/sysv/linux/ppoll.c:39
 #1  0x55e743386e09 in ppoll (__ss=0x0, __timeout=0x0, __nfds=, __fds=)
 at /usr/include/bits/poll2.h:77
 #2  0x55e743386e09 in qemu_poll_ns
 (fds=, nfds=, timeout=) at 
util/qemu-timer.c:336
 #3  0x55e743388dc4 in aio_poll (ctx=0x55e7458925d0, 
blocking=blocking@entry=true)
 at util/aio-posix.c:669
 #4  0x55e743305dea in bdrv_flush (bs=bs@entry=0x55e74593c0d0) at 
block/io.c:2878
 #5  0x55e7432be58e in bdrv_close (bs=0x55e74593c0d0) at block.c:4017
 #6  0x55e7432be58e in bdrv_delete (bs=) at block.c:4262
 #7  0x55e7432be58e in bdrv_unref (bs=bs@entry=0x55e74593c0d0) at 
block.c:5644
 #8  0x55e743316b9b in bdrv_backup_top_drop (bs=bs@entry=0x55e74593c0d0) at 
block/backup-top.c:273
 #9  0x55e74331461f in backup_job_create
 (job_id=0x0, bs=bs@entry=0x55e7458d5820, 
target=target@entry=0x55e74589f640, speed=0, sync_mode=MIRROR_SYNC_MODE_FULL, 
sync_bitmap=sync_bitmap@entry=0x0, bitmap_mode=BITMAP_SYNC_MODE_ON_SUCCESS, 
compress=false, filter_node_name=0x0, on_source_error=BLOCKDEV_ON_ERROR_REPORT, 
on_target_error=BLOCKDEV_ON_ERROR_REPORT, creation_flags=0, cb=0x0, opaque=0x0, 
txn=0x0, errp=0x7ffddfd1efb0) at block/backup.c:478
 #10 0x55e74315bc52 in do_backup_common
 (backup=backup@entry=0x55e746c066d0, bs=bs@entry=0x55e7458d5820, 
target_bs=target_bs@entry=0x55e74589f640, 
aio_context=aio_context@entry=0x55e7458a91e0, txn=txn@entry=0x0, 
errp=errp@entry=0x7ffddfd1efb0)
 at blockdev.c:3580
 #11 0x55e74315c37c in do_blockdev_backup
 (backup=backup@entry=0x55e746c066d0, txn=0x0, 
errp=errp@entry=0x7ffddfd1efb0)
 at 
/usr/src/debug/qemu-kvm-4.2.0-2.module+el8.2.0+5135+ed3b2489.x86_64/./qapi/qapi-types-block-core.h:1492
 #12 0x55e74315c449 in blockdev_backup_prepare (common=0x55e746a8de90, 
errp=0x7ffddfd1f018)
 at blockdev.c:1885
 #13 0x55e743160152 in qmp_transaction
 (dev_list=, has_props=, 
props=0x55e7467fe2c0, errp=errp@entry=0x7ffddfd1f088) at blockdev.c:2340
 #14 0x55e743287ff5 in qmp_marshal_transaction
 (args=, ret=, errp=0x7ffddfd1f0f8)
 at qapi/qapi-commands-transaction.c:44
 #15 0x55e74333de6c in do_qmp_dispatch
 (errp=0x7ffddfd1f0f0, allow_oob=, request=, 
cmds=0x55e743c28d60 ) at qapi/qmp-dispatch.c:132
 #16 0x55e74333de6c in qmp_dispatch
 (cmds=0x55e743c28d60 , request=, 
allow_oob=)
 at qapi/qmp-dispatch.c:175
 #17 0x55e74325c061 in monitor_qmp_dispatch (mon=0x55e745908030, 
req=)
 at monitor/qmp.c:145
 #18 0x55e74325c6fa in monitor_qmp_bh_dispatcher (data=) at 
monitor/qmp.c:234
 #19 0x55e743385866 in aio_bh_call (bh=0x55e745807ae0) at util/async.c:117
 #20 0x55e743385866 in aio_bh_poll (ctx=ctx@entry=0x55e7458067a0) at 
util/async.c:117
 #21 0x55e743388c54 in aio_dispatch (ctx=0x55e7458067a0) at 
util/aio-posix.c:459
 #22 0x55e743385742 in aio_ctx_dispatch
 (source=, callback=, user_data=) at util/async.c:260
 #23 0x7fb68543e67d in g_main_dispatch (context=0x55e745893a40) at 
gmain.c:3176
 #24 0x7fb68543e67d in g_main_context_dispatch 
(context=context@entry=0x55e745893a40) at gmain.c:3829
 #25 0x55e743387d08 in glib_pollfds_poll () at util/main-loop.c:219
 #26 0x55e743387d08 in os_host_main_loop_wait (timeout=) at 
util/main-loop.c:242
 #27 0x55e743387d08 in main_loop_wait (nonblocking=) at 
util/main-loop.c:518
 #28 0x55e74316a3c1 in main_loop () at vl.c:1828
 #29 0x55e743016a72 in main (argc=, argv=, 
envp=)
 at vl.c:4504

Fix this by not acquiring the AioContext there, and ensuring all paths
leading to it have it already acquired (backup_clean()).

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1782111
Signed-off-by: Sergio Lopez 
Signed-off-by: Kevin Wolf 
---
 block/backup-top.c | 5 -
 block/backup.c | 3 +++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/block/backup-top.c b/block/backup-top.c
index 818d3f26b4..b8d863ff08 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -255,9 +255,6 @@ append_failed:
 void bdrv_backup_top_drop(BlockDriverState *bs)
 {
 BDRVBackupTopState *s = bs->opaque;
-AioContext *aio_context = bdrv_get_aio_context(bs);
-
-aio_context_acquire(aio_context);
 
 bdrv_drained_begin(bs);
 
@@ -271,6 +268,4 @@ void bdrv_backup_top_drop(BlockDriverState *bs)
 bdrv_drained_end(bs);
 
 bdrv_unref

[PULL 12/13] iscsi: Cap block count from GET LBA STATUS (CVE-2020-1711)

[Kevin Wolf]

From: Felipe Franciosi 

When querying an iSCSI server for the provisioning status of blocks (via
GET LBA STATUS), Qemu only validates that the response descriptor zero's
LBA matches the one requested. Given the SCSI spec allows servers to
respond with the status of blocks beyond the end of the LUN, Qemu may
have its heap corrupted by clearing/setting too many bits at the end of
its allocmap for the LUN.

A malicious guest in control of the iSCSI server could carefully program
Qemu's heap (by selectively setting the bitmap) and then smash it.

This limits the number of bits that iscsi_co_block_status() will try to
update in the allocmap so it can't overflow the bitmap.

Fixes: CVE-2020-1711
Cc: qemu-sta...@nongnu.org
Signed-off-by: Felipe Franciosi 
Signed-off-by: Peter Turschmid 
Signed-off-by: Raphael Norwitz 
Signed-off-by: Kevin Wolf 
---
 block/iscsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 2aea7e3f13..cbd57294ab 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -701,7 +701,7 @@ static int coroutine_fn 
iscsi_co_block_status(BlockDriverState *bs,
 struct scsi_get_lba_status *lbas = NULL;
 struct scsi_lba_status_descriptor *lbasd = NULL;
 struct IscsiTask iTask;
-uint64_t lba;
+uint64_t lba, max_bytes;
 int ret;
 
 iscsi_co_init_iscsitask(iscsilun, &iTask);
@@ -721,6 +721,7 @@ static int coroutine_fn 
iscsi_co_block_status(BlockDriverState *bs,
 }
 
 lba = offset / iscsilun->block_size;
+max_bytes = (iscsilun->num_blocks - lba) * iscsilun->block_size;
 
 qemu_mutex_lock(&iscsilun->mutex);
 retry:
@@ -764,7 +765,7 @@ retry:
 goto out_unlock;
 }
 
-*pnum = (int64_t) lbasd->num_blocks * iscsilun->block_size;
+*pnum = MIN((int64_t) lbasd->num_blocks * iscsilun->block_size, max_bytes);
 
 if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
 lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
-- 
2.20.1

[PULL 06/13] blockdev: honor bdrv_try_set_aio_context() context requirements

[Kevin Wolf]

From: Sergio Lopez 

bdrv_try_set_aio_context() requires that the old context is held, and
the new context is not held. Fix all the occurrences where it's not
done this way.

Suggested-by: Max Reitz 
Signed-off-by: Sergio Lopez 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 68 +++---
 1 file changed, 60 insertions(+), 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 152a0f7454..1dacbc20ec 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1535,6 +1535,7 @@ static void external_snapshot_prepare(BlkActionState 
*common,
  DO_UPCAST(ExternalSnapshotState, common, common);
 TransactionAction *action = common->action;
 AioContext *aio_context;
+AioContext *old_context;
 int ret;
 
 /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
@@ -1675,7 +1676,16 @@ static void external_snapshot_prepare(BlkActionState 
*common,
 goto out;
 }
 
+/* Honor bdrv_try_set_aio_context() context acquisition requirements. */
+old_context = bdrv_get_aio_context(state->new_bs);
+aio_context_release(aio_context);
+aio_context_acquire(old_context);
+
 ret = bdrv_try_set_aio_context(state->new_bs, aio_context, errp);
+
+aio_context_release(old_context);
+aio_context_acquire(aio_context);
+
 if (ret < 0) {
 goto out;
 }
@@ -1775,11 +1785,13 @@ static void drive_backup_prepare(BlkActionState 
*common, Error **errp)
 BlockDriverState *target_bs;
 BlockDriverState *source = NULL;
 AioContext *aio_context;
+AioContext *old_context;
 QDict *options;
 Error *local_err = NULL;
 int flags;
 int64_t size;
 bool set_backing_hd = false;
+int ret;
 
 assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
 backup = common->action->u.drive_backup.data;
@@ -1868,6 +1880,21 @@ static void drive_backup_prepare(BlkActionState *common, 
Error **errp)
 goto out;
 }
 
+/* Honor bdrv_try_set_aio_context() context acquisition requirements. */
+old_context = bdrv_get_aio_context(target_bs);
+aio_context_release(aio_context);
+aio_context_acquire(old_context);
+
+ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
+if (ret < 0) {
+bdrv_unref(target_bs);
+aio_context_release(old_context);
+return;
+}
+
+aio_context_release(old_context);
+aio_context_acquire(aio_context);
+
 if (set_backing_hd) {
 bdrv_set_backing_hd(target_bs, source, &local_err);
 if (local_err) {
@@ -1947,6 +1974,8 @@ static void blockdev_backup_prepare(BlkActionState 
*common, Error **errp)
 BlockDriverState *bs;
 BlockDriverState *target_bs;
 AioContext *aio_context;
+AioContext *old_context;
+int ret;
 
 assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
 backup = common->action->u.blockdev_backup.data;
@@ -1961,7 +1990,18 @@ static void blockdev_backup_prepare(BlkActionState 
*common, Error **errp)
 return;
 }
 
+/* Honor bdrv_try_set_aio_context() context acquisition requirements. */
 aio_context = bdrv_get_aio_context(bs);
+old_context = bdrv_get_aio_context(target_bs);
+aio_context_acquire(old_context);
+
+ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
+if (ret < 0) {
+aio_context_release(old_context);
+return;
+}
+
+aio_context_release(old_context);
 aio_context_acquire(aio_context);
 state->bs = bs;
 
@@ -3562,7 +3602,6 @@ static BlockJob *do_backup_common(BackupCommon *backup,
 BlockJob *job = NULL;
 BdrvDirtyBitmap *bmap = NULL;
 int job_flags = JOB_DEFAULT;
-int ret;
 
 if (!backup->has_speed) {
 backup->speed = 0;
@@ -3586,11 +3625,6 @@ static BlockJob *do_backup_common(BackupCommon *backup,
 backup->compress = false;
 }
 
-ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
-if (ret < 0) {
-return NULL;
-}
-
 if ((backup->sync == MIRROR_SYNC_MODE_BITMAP) ||
 (backup->sync == MIRROR_SYNC_MODE_INCREMENTAL)) {
 /* done before desugaring 'incremental' to print the right message */
@@ -3825,6 +3859,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
 BlockDriverState *bs;
 BlockDriverState *source, *target_bs;
 AioContext *aio_context;
+AioContext *old_context;
 BlockMirrorBackingMode backing_mode;
 Error *local_err = NULL;
 QDict *options = NULL;
@@ -3937,12 +3972,22 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
(arg->mode == NEW_IMAGE_MODE_EXISTING ||
 !bdrv_has_zero_init(target_bs)));
 
+
+/* Honor bdrv_try_set_aio_context() context acquisition requirements. */
+old_context = bdrv_get_aio_context(target_bs);
+aio_context_release(aio_context);
+aio_context_acquire(old_context);
+
 ret = bdrv_try_set_aio_context(target_bs, aio_context

[PULL 08/13] blockdev: Acquire AioContext on dirty bitmap functions

[Kevin Wolf]

From: Sergio Lopez 

Dirty map addition and removal functions are not acquiring to BDS
AioContext, while they may call to code that expects it to be
acquired.

This may trigger a crash with a stack trace like this one:

 #0  0x7f0ef146370f in __GI_raise (sig=sig@entry=6)
 at ../sysdeps/unix/sysv/linux/raise.c:50
 #1  0x7f0ef144db25 in __GI_abort () at abort.c:79
 #2  0x565022294dce in error_exit
 (err=, msg=msg@entry=0x56502243a730 <__func__.16350> 
"qemu_mutex_unlock_impl") at util/qemu-thread-posix.c:36
 #3  0x5650222950ba in qemu_mutex_unlock_impl
 (mutex=mutex@entry=0x5650244b0240, file=file@entry=0x565022439adf 
"util/async.c", line=line@entry=526) at util/qemu-thread-posix.c:108
 #4  0x565022290029 in aio_context_release
 (ctx=ctx@entry=0x5650244b01e0) at util/async.c:526
 #5  0x56502221cd08 in bdrv_can_store_new_dirty_bitmap
 (bs=bs@entry=0x5650244dc820, name=name@entry=0x56502481d360 "bitmap1", 
granularity=granularity@entry=65536, errp=errp@entry=0x7fff22831718)
 at block/dirty-bitmap.c:542
 #6  0x56502206ae53 in qmp_block_dirty_bitmap_add
 (errp=0x7fff22831718, disabled=false, has_disabled=, 
persistent=, has_persistent=true, granularity=65536, 
has_granularity=, name=0x56502481d360 "bitmap1", node=) at blockdev.c:2894
 #7  0x56502206ae53 in qmp_block_dirty_bitmap_add
 (node=, name=0x56502481d360 "bitmap1", 
has_granularity=, granularity=, 
has_persistent=true, persistent=, has_disabled=false, 
disabled=false, errp=0x7fff22831718) at blockdev.c:2856
 #8  0x5650221847a3 in qmp_marshal_block_dirty_bitmap_add
 (args=, ret=, errp=0x7fff22831798)
 at qapi/qapi-commands-block-core.c:651
 #9  0x565022247e6c in do_qmp_dispatch
 (errp=0x7fff22831790, allow_oob=, request=, 
cmds=0x565022b32d60 ) at qapi/qmp-dispatch.c:132
 #10 0x565022247e6c in qmp_dispatch
 (cmds=0x565022b32d60 , request=, 
allow_oob=) at qapi/qmp-dispatch.c:175
 #11 0x565022166061 in monitor_qmp_dispatch
 (mon=0x56502450faa0, req=) at monitor/qmp.c:145
 #12 0x5650221666fa in monitor_qmp_bh_dispatcher
 (data=) at monitor/qmp.c:234
 #13 0x56502228f866 in aio_bh_call (bh=0x56502440eae0)
 at util/async.c:117
 #14 0x56502228f866 in aio_bh_poll (ctx=ctx@entry=0x56502440d7a0)
 at util/async.c:117
 #15 0x565022292c54 in aio_dispatch (ctx=0x56502440d7a0)
 at util/aio-posix.c:459
 #16 0x56502228f742 in aio_ctx_dispatch
 (source=, callback=, user_data=) at util/async.c:260
 #17 0x7f0ef5ce667d in g_main_dispatch (context=0x56502449aa40)
 at gmain.c:3176
 #18 0x7f0ef5ce667d in g_main_context_dispatch
 (context=context@entry=0x56502449aa40) at gmain.c:3829
 #19 0x565022291d08 in glib_pollfds_poll () at util/main-loop.c:219
 #20 0x565022291d08 in os_host_main_loop_wait
 (timeout=) at util/main-loop.c:242
 #21 0x565022291d08 in main_loop_wait (nonblocking=)
 at util/main-loop.c:518
 #22 0x5650220743c1 in main_loop () at vl.c:1828
 #23 0x565021f20a72 in main
 (argc=, argv=, envp=)
 at vl.c:4504

Fix this by acquiring the AioContext at qmp_block_dirty_bitmap_add()
and qmp_block_dirty_bitmap_add().

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1782175
Signed-off-by: Sergio Lopez 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1dacbc20ec..d4ef6cd452 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2984,6 +2984,7 @@ void qmp_block_dirty_bitmap_add(const char *node, const 
char *name,
 {
 BlockDriverState *bs;
 BdrvDirtyBitmap *bitmap;
+AioContext *aio_context;
 
 if (!name || name[0] == '\0') {
 error_setg(errp, "Bitmap name cannot be empty");
@@ -2995,11 +2996,14 @@ void qmp_block_dirty_bitmap_add(const char *node, const 
char *name,
 return;
 }
 
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
 if (has_granularity) {
 if (granularity < 512 || !is_power_of_2(granularity)) {
 error_setg(errp, "Granularity must be power of 2 "
  "and at least 512");
-return;
+goto out;
 }
 } else {
 /* Default to cluster size, if available: */
@@ -3017,12 +3021,12 @@ void qmp_block_dirty_bitmap_add(const char *node, const 
char *name,
 if (persistent &&
 !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
 {
-return;
+goto out;
 }
 
 bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
 if (bitmap == NULL) {
-return;
+goto out;
 }
 
 if (disabled) {
@@ -3030,6 +3034,9 @@ void qmp_block_dirty_bitmap_add(const char *node, const 
char *name,
 }
 
 bdrv_dirty_bitmap_set_persistence(bitmap, persistent);
+
+out:
+aio_context_release(aio_context);
 }
 
 static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
@@ -

[PULL 05/13] blockdev: unify qmp_blockdev_backup and blockdev-backup transaction paths

[Kevin Wolf]

From: Sergio Lopez 

Issuing a blockdev-backup from qmp_blockdev_backup takes a slightly
different path than when it's issued from a transaction. In the code,
this is manifested as some redundancy between do_blockdev_backup() and
blockdev_backup_prepare().

This change unifies both paths, merging do_blockdev_backup() and
blockdev_backup_prepare(), and changing qmp_blockdev_backup() to
create a transaction instead of calling do_backup_common() direcly.

As a side-effect, now qmp_blockdev_backup() is executed inside a
drained section, as it happens when creating a blockdev-backup
transaction. This change is visible from the user's perspective, as
the job gets paused and immediately resumed before starting the actual
work.

Signed-off-by: Sergio Lopez 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 60 --
 1 file changed, 13 insertions(+), 47 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5e85fc042e..152a0f7454 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1940,16 +1940,13 @@ typedef struct BlockdevBackupState {
 BlockJob *job;
 } BlockdevBackupState;
 
-static BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
-Error **errp);
-
 static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
 {
 BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
 BlockdevBackup *backup;
-BlockDriverState *bs, *target;
+BlockDriverState *bs;
+BlockDriverState *target_bs;
 AioContext *aio_context;
-Error *local_err = NULL;
 
 assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
 backup = common->action->u.blockdev_backup.data;
@@ -1959,8 +1956,8 @@ static void blockdev_backup_prepare(BlkActionState 
*common, Error **errp)
 return;
 }
 
-target = bdrv_lookup_bs(backup->target, backup->target, errp);
-if (!target) {
+target_bs = bdrv_lookup_bs(backup->target, backup->target, errp);
+if (!target_bs) {
 return;
 }
 
@@ -1971,13 +1968,10 @@ static void blockdev_backup_prepare(BlkActionState 
*common, Error **errp)
 /* Paired with .clean() */
 bdrv_drained_begin(state->bs);
 
-state->job = do_blockdev_backup(backup, common->block_job_txn, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-goto out;
-}
+state->job = do_backup_common(qapi_BlockdevBackup_base(backup),
+  bs, target_bs, aio_context,
+  common->block_job_txn, errp);
 
-out:
 aio_context_release(aio_context);
 }
 
@@ -3695,41 +3689,13 @@ XDbgBlockGraph *qmp_x_debug_query_block_graph(Error 
**errp)
 return bdrv_get_xdbg_block_graph(errp);
 }
 
-BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
- Error **errp)
+void qmp_blockdev_backup(BlockdevBackup *backup, Error **errp)
 {
-BlockDriverState *bs;
-BlockDriverState *target_bs;
-AioContext *aio_context;
-BlockJob *job;
-
-bs = bdrv_lookup_bs(backup->device, backup->device, errp);
-if (!bs) {
-return NULL;
-}
-
-target_bs = bdrv_lookup_bs(backup->target, backup->target, errp);
-if (!target_bs) {
-return NULL;
-}
-
-aio_context = bdrv_get_aio_context(bs);
-aio_context_acquire(aio_context);
-
-job = do_backup_common(qapi_BlockdevBackup_base(backup),
-   bs, target_bs, aio_context, txn, errp);
-
-aio_context_release(aio_context);
-return job;
-}
-
-void qmp_blockdev_backup(BlockdevBackup *arg, Error **errp)
-{
-BlockJob *job;
-job = do_blockdev_backup(arg, NULL, errp);
-if (job) {
-job_start(&job->job);
-}
+TransactionAction action = {
+.type = TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP,
+.u.blockdev_backup.data = backup,
+};
+blockdev_do_action(&action, errp);
 }
 
 /* Parameter check and block job starting for drive mirroring.
-- 
2.20.1

[PULL 10/13] iotests: Test handling of AioContexts with some blockdev actions

[Kevin Wolf]

From: Sergio Lopez 

Includes the following tests:

 - Adding a dirty bitmap.
   * RHBZ: 1782175

 - Starting a drive-mirror to an NBD-backed target.
   * RHBZ: 1746217, 1773517

 - Aborting an external snapshot transaction.
   * RHBZ: 1779036

 - Aborting a blockdev backup transaction.
   * RHBZ: 1782111

For each one of them, a VM with a number of disks running in an
IOThread AioContext is used.

Signed-off-by: Sergio Lopez 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/281 | 247 +
 tests/qemu-iotests/281.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 253 insertions(+)
 create mode 100755 tests/qemu-iotests/281
 create mode 100644 tests/qemu-iotests/281.out

diff --git a/tests/qemu-iotests/281 b/tests/qemu-iotests/281
new file mode 100755
index 00..269d583b2c
--- /dev/null
+++ b/tests/qemu-iotests/281
@@ -0,0 +1,247 @@
+#!/usr/bin/env python
+#
+# Test cases for blockdev + IOThread interactions
+#
+# 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 .
+#
+
+import os
+import iotests
+from iotests import qemu_img
+
+image_len = 64 * 1024 * 1024
+
+# Test for RHBZ#1782175
+class TestDirtyBitmapIOThread(iotests.QMPTestCase):
+drive0_img = os.path.join(iotests.test_dir, 'drive0.img')
+images = { 'drive0': drive0_img }
+
+def setUp(self):
+for name in self.images:
+qemu_img('create', '-f', iotests.imgfmt,
+ self.images[name], str(image_len))
+
+self.vm = iotests.VM()
+self.vm.add_object('iothread,id=iothread0')
+
+for name in self.images:
+self.vm.add_blockdev('driver=file,filename=%s,node-name=file_%s'
+ % (self.images[name], name))
+self.vm.add_blockdev('driver=qcow2,file=file_%s,node-name=%s'
+ % (name, name))
+
+self.vm.launch()
+self.vm.qmp('x-blockdev-set-iothread',
+node_name='drive0', iothread='iothread0',
+force=True)
+
+def tearDown(self):
+self.vm.shutdown()
+for name in self.images:
+os.remove(self.images[name])
+
+def test_add_dirty_bitmap(self):
+result = self.vm.qmp(
+'block-dirty-bitmap-add',
+node='drive0',
+name='bitmap1',
+persistent=True,
+)
+
+self.assert_qmp(result, 'return', {})
+
+
+# Test for RHBZ#1746217 & RHBZ#1773517
+class TestNBDMirrorIOThread(iotests.QMPTestCase):
+nbd_sock = os.path.join(iotests.sock_dir, 'nbd.sock')
+drive0_img = os.path.join(iotests.test_dir, 'drive0.img')
+mirror_img = os.path.join(iotests.test_dir, 'mirror.img')
+images = { 'drive0': drive0_img, 'mirror': mirror_img }
+
+def setUp(self):
+for name in self.images:
+qemu_img('create', '-f', iotests.imgfmt,
+ self.images[name], str(image_len))
+
+self.vm_src = iotests.VM(path_suffix='src')
+self.vm_src.add_object('iothread,id=iothread0')
+self.vm_src.add_blockdev('driver=file,filename=%s,node-name=file0'
+  % (self.drive0_img))
+self.vm_src.add_blockdev('driver=qcow2,file=file0,node-name=drive0')
+self.vm_src.launch()
+self.vm_src.qmp('x-blockdev-set-iothread',
+node_name='drive0', iothread='iothread0',
+force=True)
+
+self.vm_tgt = iotests.VM(path_suffix='tgt')
+self.vm_tgt.add_object('iothread,id=iothread0')
+self.vm_tgt.add_blockdev('driver=file,filename=%s,node-name=file0'
+  % (self.mirror_img))
+self.vm_tgt.add_blockdev('driver=qcow2,file=file0,node-name=drive0')
+self.vm_tgt.launch()
+self.vm_tgt.qmp('x-blockdev-set-iothread',
+node_name='drive0', iothread='iothread0',
+force=True)
+
+def tearDown(self):
+self.vm_src.shutdown()
+self.vm_tgt.shutdown()
+for name in self.images:
+os.remove(self.images[name])
+
+def test_nbd_mirror(self):
+result = self.vm_tgt.qmp(
+'nbd-server-start',
+addr={
+'type': 'unix',
+'data': { 'path': self.nbd_sock }
+}
+)
+self.assert_qmp(r

[PULL 01/13] iotests.py: Let wait_migration wait even more

[Kevin Wolf]

From: Max Reitz 

The "migration completed" event may be sent (on the source, to be
specific) before the migration is actually completed, so the VM runstate
will still be "finish-migrate" instead of "postmigrate".  So ask the
users of VM.wait_migration() to specify the final runstate they desire
and then poll the VM until it has reached that state.  (This should be
over very quickly, so busy polling is fine.)

Without this patch, I see intermittent failures in the new iotest 280
under high system load.  I have not yet seen such failures with other
iotests that use VM.wait_migration() and query-status afterwards, but
maybe they just occur even more rarely, or it is because they also wait
on the destination VM to be running.

Signed-off-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/iotests.py | 6 +-
 tests/qemu-iotests/234| 8 
 tests/qemu-iotests/262| 4 ++--
 tests/qemu-iotests/280| 2 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 13fd8b5cd2..0b62c42851 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -668,12 +668,16 @@ class VM(qtest.QEMUQtestMachine):
 }
 ]))
 
-def wait_migration(self):
+def wait_migration(self, expect_runstate):
 while True:
 event = self.event_wait('MIGRATION')
 log(event, filters=[filter_qmp_event])
 if event['data']['status'] == 'completed':
 break
+# The event may occur in finish-migrate, so wait for the expected
+# post-migration runstate
+while self.qmp('query-status')['return']['status'] != expect_runstate:
+pass
 
 def node_info(self, node_name):
 nodes = self.qmp('query-named-block-nodes')
diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234
index 34c818c485..59a7f949ec 100755
--- a/tests/qemu-iotests/234
+++ b/tests/qemu-iotests/234
@@ -69,9 +69,9 @@ with iotests.FilePath('img') as img_path, \
 iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo_a)))
 with iotests.Timeout(3, 'Migration does not complete'):
 # Wait for the source first (which includes setup=setup)
-vm_a.wait_migration()
+vm_a.wait_migration('postmigrate')
 # Wait for the destination second (which does not)
-vm_b.wait_migration()
+vm_b.wait_migration('running')
 
 iotests.log(vm_a.qmp('query-migrate')['return']['status'])
 iotests.log(vm_b.qmp('query-migrate')['return']['status'])
@@ -98,9 +98,9 @@ with iotests.FilePath('img') as img_path, \
 iotests.log(vm_b.qmp('migrate', uri='exec:cat >%s' % (fifo_b)))
 with iotests.Timeout(3, 'Migration does not complete'):
 # Wait for the source first (which includes setup=setup)
-vm_b.wait_migration()
+vm_b.wait_migration('postmigrate')
 # Wait for the destination second (which does not)
-vm_a.wait_migration()
+vm_a.wait_migration('running')
 
 iotests.log(vm_a.qmp('query-migrate')['return']['status'])
 iotests.log(vm_b.qmp('query-migrate')['return']['status'])
diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
index 0963daa806..bbcb5260a6 100755
--- a/tests/qemu-iotests/262
+++ b/tests/qemu-iotests/262
@@ -71,9 +71,9 @@ with iotests.FilePath('img') as img_path, \
 iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo)))
 with iotests.Timeout(3, 'Migration does not complete'):
 # Wait for the source first (which includes setup=setup)
-vm_a.wait_migration()
+vm_a.wait_migration('postmigrate')
 # Wait for the destination second (which does not)
-vm_b.wait_migration()
+vm_b.wait_migration('running')
 
 iotests.log(vm_a.qmp('query-migrate')['return']['status'])
 iotests.log(vm_b.qmp('query-migrate')['return']['status'])
diff --git a/tests/qemu-iotests/280 b/tests/qemu-iotests/280
index 0b1fa8e1d8..85e9114c5e 100755
--- a/tests/qemu-iotests/280
+++ b/tests/qemu-iotests/280
@@ -45,7 +45,7 @@ with iotests.FilePath('base') as base_path , \
 vm.qmp_log('migrate', uri='exec:cat > /dev/null')
 
 with iotests.Timeout(3, 'Migration does not complete'):
-vm.wait_migration()
+vm.wait_migration('postmigrate')
 
 iotests.log('\nVM is now stopped:')
 iotests.log(vm.qmp('query-migrate')['return']['status'])
-- 
2.20.1

[PULL 09/13] blockdev: Return bs to the proper context on snapshot abort

[Kevin Wolf]

From: Sergio Lopez 

external_snapshot_abort() calls to bdrv_set_backing_hd(), which
returns state->old_bs to the main AioContext, as it's intended to be
used then the BDS is going to be released. As that's not the case when
aborting an external snapshot, return it to the AioContext it was
before the call.

This issue can be triggered by issuing a transaction with two actions,
a proper blockdev-snapshot-sync and a bogus one, so the second will
trigger a transaction abort. This results in a crash with an stack
trace like this one:

 #0  0x7fa1048b28df in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:50
 #1  0x7fa10489ccf5 in __GI_abort () at abort.c:79
 #2  0x7fa10489cbc9 in __assert_fail_base
 (fmt=0x7fa104a03300 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
assertion=0x5572240b44d8 "bdrv_get_aio_context(old_bs) == 
bdrv_get_aio_context(new_bs)", file=0x557224014d30 "block.c", line=2240, 
function=) at assert.c:92
 #3  0x7fa1048aae96 in __GI___assert_fail
 (assertion=assertion@entry=0x5572240b44d8 "bdrv_get_aio_context(old_bs) == 
bdrv_get_aio_context(new_bs)", file=file@entry=0x557224014d30 "block.c", 
line=line@entry=2240, function=function@entry=0x5572240b5d60 
<__PRETTY_FUNCTION__.31620> "bdrv_replace_child_noperm") at assert.c:101
 #4  0x557223e631f8 in bdrv_replace_child_noperm (child=0x557225b9c980, 
new_bs=new_bs@entry=0x557225c42e40) at block.c:2240
 #5  0x557223e68be7 in bdrv_replace_node (from=0x557226951a60, 
to=0x557225c42e40, errp=0x5572247d6138 ) at block.c:4196
 #6  0x557223d069c4 in external_snapshot_abort (common=0x557225d7e170) at 
blockdev.c:1731
 #7  0x557223d069c4 in external_snapshot_abort (common=0x557225d7e170) at 
blockdev.c:1717
 #8  0x557223d09013 in qmp_transaction (dev_list=, 
has_props=, props=0x557225cc7d70, 
errp=errp@entry=0x7ffe704c0c98) at blockdev.c:2360
 #9  0x557223e32085 in qmp_marshal_transaction (args=, 
ret=, errp=0x7ffe704c0d08) at qapi/qapi-commands-transaction.c:44
 #10 0x557223ee798c in do_qmp_dispatch (errp=0x7ffe704c0d00, 
allow_oob=, request=, cmds=0x5572247d3cc0 
) at qapi/qmp-dispatch.c:132
 #11 0x557223ee798c in qmp_dispatch (cmds=0x5572247d3cc0 , 
request=, allow_oob=) at qapi/qmp-dispatch.c:175
 #12 0x557223e06141 in monitor_qmp_dispatch (mon=0x557225c69ff0, 
req=) at monitor/qmp.c:120
 #13 0x557223e0678a in monitor_qmp_bh_dispatcher (data=) at 
monitor/qmp.c:209
 #14 0x557223f2f366 in aio_bh_call (bh=0x557225b9dc60) at util/async.c:117
 #15 0x557223f2f366 in aio_bh_poll (ctx=ctx@entry=0x557225b9c840) at 
util/async.c:117
 #16 0x557223f32754 in aio_dispatch (ctx=0x557225b9c840) at 
util/aio-posix.c:459
 #17 0x557223f2f242 in aio_ctx_dispatch (source=, 
callback=, user_data=) at util/async.c:260
 #18 0x7fa10913467d in g_main_dispatch (context=0x557225c28e80) at 
gmain.c:3176
 #19 0x7fa10913467d in g_main_context_dispatch 
(context=context@entry=0x557225c28e80) at gmain.c:3829
 #20 0x557223f31808 in glib_pollfds_poll () at util/main-loop.c:219
 #21 0x557223f31808 in os_host_main_loop_wait (timeout=) at 
util/main-loop.c:242
 #22 0x557223f31808 in main_loop_wait (nonblocking=) at 
util/main-loop.c:518
 #23 0x557223d13201 in main_loop () at vl.c:1828
 #24 0x557223bbfb82 in main (argc=, argv=, 
envp=) at vl.c:4504

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1779036
Signed-off-by: Sergio Lopez 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index d4ef6cd452..4cd9a58d36 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1731,6 +1731,8 @@ static void external_snapshot_abort(BlkActionState 
*common)
 if (state->new_bs) {
 if (state->overlay_appended) {
 AioContext *aio_context;
+AioContext *tmp_context;
+int ret;
 
 aio_context = bdrv_get_aio_context(state->old_bs);
 aio_context_acquire(aio_context);
@@ -1738,6 +1740,25 @@ static void external_snapshot_abort(BlkActionState 
*common)
 bdrv_ref(state->old_bs);   /* we can't let bdrv_set_backind_hd()
   close state->old_bs; we need it */
 bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
+
+/*
+ * The call to bdrv_set_backing_hd() above returns state->old_bs to
+ * the main AioContext. As we're still going to be using it, return
+ * it to the AioContext it was before.
+ */
+tmp_context = bdrv_get_aio_context(state->old_bs);
+if (aio_context != tmp_context) {
+aio_context_release(aio_context);
+aio_context_acquire(tmp_context);
+
+ret = bdrv_try_set_aio_context(state->old_bs,
+   aio_context, NULL);
+assert(ret == 0);
+
+aio_context_release(tmp_conte

[PULL 03/13] blockdev: fix coding style issues in drive_backup_prepare

[Kevin Wolf]

From: Sergio Lopez 

Fix a couple of minor coding style issues in drive_backup_prepare.

Signed-off-by: Sergio Lopez 
Reviewed-by: Max Reitz 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8e029e9c01..553e315972 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3620,7 +3620,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, 
JobTxn *txn,
 
 if (!backup->has_format) {
 backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
- NULL : (char*) bs->drv->format_name;
+ NULL : (char *) bs->drv->format_name;
 }
 
 /* Early check to avoid creating target */
@@ -3630,8 +3630,10 @@ static BlockJob *do_drive_backup(DriveBackup *backup, 
JobTxn *txn,
 
 flags = bs->open_flags | BDRV_O_RDWR;
 
-/* See if we have a backing HD we can use to create our new image
- * on top of. */
+/*
+ * See if we have a backing HD we can use to create our new image
+ * on top of.
+ */
 if (backup->sync == MIRROR_SYNC_MODE_TOP) {
 source = backing_bs(bs);
 if (!source) {
-- 
2.20.1

[PULL 02/13] iotests: Add more "skip_if_unsupported" statements to the python tests

[Kevin Wolf]

From: Thomas Huth 

The python code already contains a possibility to skip tests if the
corresponding driver is not available in the qemu binary - use it
in more spots to avoid that the tests are failing if the driver has
been disabled.

While we're at it, we can now also remove some of the old checks that
were using iotests.supports_quorum() - and which were apparently not
working as expected since the tests aborted instead of being skipped
when "quorum" was missing in the QEMU binary.

Signed-off-by: Thomas Huth 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/030 |  4 +---
 tests/qemu-iotests/040 |  2 ++
 tests/qemu-iotests/041 | 39 +++
 tests/qemu-iotests/245 |  2 ++
 4 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index be35bde06f..0990681c1e 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -530,6 +530,7 @@ class TestQuorum(iotests.QMPTestCase):
 children = []
 backing = []
 
+@iotests.skip_if_unsupported(['quorum'])
 def setUp(self):
 opts = ['driver=quorum', 'vote-threshold=2']
 
@@ -560,9 +561,6 @@ class TestQuorum(iotests.QMPTestCase):
 os.remove(img)
 
 def test_stream_quorum(self):
-if not iotests.supports_quorum():
-return
-
 self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', 
self.children[0]),
 qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', 
self.backing[0]),
 'image file map matches backing file before 
streaming')
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 762ad1ebcb..74f62c3c4a 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -106,6 +106,7 @@ class TestSingleDrive(ImageCommitTestCase):
 self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 
524288', backing_img).find("verification failed"))
 self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 
524288', backing_img).find("verification failed"))
 
+@iotests.skip_if_unsupported(['throttle'])
 def test_commit_with_filter_and_quit(self):
 result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
 self.assert_qmp(result, 'return', {})
@@ -125,6 +126,7 @@ class TestSingleDrive(ImageCommitTestCase):
 self.has_quit = True
 
 # Same as above, but this time we add the filter after starting the job
+@iotests.skip_if_unsupported(['throttle'])
 def test_commit_plus_filter_and_quit(self):
 result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
 self.assert_qmp(result, 'return', {})
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index d7be30b62b..c07437fda1 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -871,6 +871,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
 image_len = 1 * 1024 * 1024 # MB
 IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
 
+@iotests.skip_if_unsupported(['quorum'])
 def setUp(self):
 self.vm = iotests.VM()
 
@@ -891,9 +892,8 @@ class TestRepairQuorum(iotests.QMPTestCase):
 #assemble the quorum block device from the individual files
 args = { "driver": "quorum", "node-name": "quorum0",
  "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] }
-if iotests.supports_quorum():
-result = self.vm.qmp("blockdev-add", **args)
-self.assert_qmp(result, 'return', {})
+result = self.vm.qmp("blockdev-add", **args)
+self.assert_qmp(result, 'return', {})
 
 
 def tearDown(self):
@@ -906,9 +906,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
 pass
 
 def test_complete(self):
-if not iotests.supports_quorum():
-return
-
 self.assert_no_active_block_jobs()
 
 result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
@@ -925,9 +922,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
 'target image does not match source after mirroring')
 
 def test_cancel(self):
-if not iotests.supports_quorum():
-return
-
 self.assert_no_active_block_jobs()
 
 result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
@@ -942,9 +936,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
 self.vm.shutdown()
 
 def test_cancel_after_ready(self):
-if not iotests.supports_quorum():
-return
-
 self.assert_no_active_block_jobs()
 
 result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
@@ -961,9 +952,6 @@ class TestRepairQuorum(iotests.QMPTestCase):
 'target image does not match source after mirroring')
 
 def test_pause(self):
-if not iotests.supports_quorum():
-return
-
 self.assert_no_active_block_jobs()
 
 result = self.vm.qm

[PULL 11/13] block/backup: fix memory leak in bdrv_backup_top_append()

[Kevin Wolf]

From: Eiichi Tsukata 

bdrv_open_driver() allocates bs->opaque according to drv->instance_size.
There is no need to allocate it and overwrite opaque in
bdrv_backup_top_append().

Reproducer:

  $ QTEST_QEMU_BINARY=./x86_64-softmmu/qemu-system-x86_64 valgrind -q 
--leak-check=full tests/test-replication -p /replication/secondary/start
  ==29792== 24 bytes in 1 blocks are definitely lost in loss record 52 of 226
  ==29792==at 0x483AB1A: calloc (vg_replace_malloc.c:762)
  ==29792==by 0x4B07CE0: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.7)
  ==29792==by 0x12BAB9: bdrv_open_driver (block.c:1289)
  ==29792==by 0x12BEA9: bdrv_new_open_driver (block.c:1359)
  ==29792==by 0x1D15CB: bdrv_backup_top_append (backup-top.c:190)
  ==29792==by 0x1CC11A: backup_job_create (backup.c:439)
  ==29792==by 0x1CD542: replication_start (replication.c:544)
  ==29792==by 0x1401B9: replication_start_all (replication.c:52)
  ==29792==by 0x128B50: test_secondary_start (test-replication.c:427)
  ...

Fixes: 7df7868b9640 ("block: introduce backup-top filter driver")
Signed-off-by: Eiichi Tsukata 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Kevin Wolf 
---
 block/backup-top.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/backup-top.c b/block/backup-top.c
index b8d863ff08..9aed2eb4c0 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -196,7 +196,7 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState 
*source,
 }
 
 top->total_sectors = source->total_sectors;
-top->opaque = state = g_new0(BDRVBackupTopState, 1);
+state = top->opaque;
 
 bdrv_ref(target);
 state->target = bdrv_attach_child(top, target, "target", &child_file, 
errp);
-- 
2.20.1

[PULL 00/13] Block layer patches

[Kevin Wolf]

The following changes since commit 105b07f1ba462ec48b27e5cb74ddf81c6a79364c:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200127' into 
staging (2020-01-27 13:02:36 +)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 5fbf1d56c24018772e900a40a0955175ff82f35c:

  iscsi: Don't access non-existent scsi_lba_status_descriptor (2020-01-27 
17:19:53 +0100)


Block layer patches:

- iscsi: Cap block count from GET LBA STATUS (CVE-2020-1711)
- AioContext fixes in QMP commands for backup and bitmaps
- iotests fixes


Eiichi Tsukata (1):
  block/backup: fix memory leak in bdrv_backup_top_append()

Felipe Franciosi (1):
  iscsi: Cap block count from GET LBA STATUS (CVE-2020-1711)

Kevin Wolf (1):
  iscsi: Don't access non-existent scsi_lba_status_descriptor

Max Reitz (1):
  iotests.py: Let wait_migration wait even more

Sergio Lopez (8):
  blockdev: fix coding style issues in drive_backup_prepare
  blockdev: unify qmp_drive_backup and drive-backup transaction paths
  blockdev: unify qmp_blockdev_backup and blockdev-backup transaction paths
  blockdev: honor bdrv_try_set_aio_context() context requirements
  block/backup-top: Don't acquire context while dropping top
  blockdev: Acquire AioContext on dirty bitmap functions
  blockdev: Return bs to the proper context on snapshot abort
  iotests: Test handling of AioContexts with some blockdev actions

Thomas Huth (1):
  iotests: Add more "skip_if_unsupported" statements to the python tests

 block/backup-top.c|   7 +-
 block/backup.c|   3 +
 block/iscsi.c |   7 +-
 blockdev.c| 393 +++---
 tests/qemu-iotests/iotests.py |   6 +-
 tests/qemu-iotests/030|   4 +-
 tests/qemu-iotests/040|   2 +
 tests/qemu-iotests/041|  39 +
 tests/qemu-iotests/141.out|   2 +
 tests/qemu-iotests/185.out|   2 +
 tests/qemu-iotests/219|   7 +-
 tests/qemu-iotests/219.out|   8 +
 tests/qemu-iotests/234|   8 +-
 tests/qemu-iotests/245|   2 +
 tests/qemu-iotests/262|   4 +-
 tests/qemu-iotests/280|   2 +-
 tests/qemu-iotests/281| 247 ++
 tests/qemu-iotests/281.out|   5 +
 tests/qemu-iotests/group  |   1 +
 19 files changed, 510 insertions(+), 239 deletions(-)
 create mode 100755 tests/qemu-iotests/281
 create mode 100644 tests/qemu-iotests/281.out

[PULL 04/13] blockdev: unify qmp_drive_backup and drive-backup transaction paths

[Kevin Wolf]

From: Sergio Lopez 

Issuing a drive-backup from qmp_drive_backup takes a slightly
different path than when it's issued from a transaction. In the code,
this is manifested as some redundancy between do_drive_backup() and
drive_backup_prepare().

This change unifies both paths, merging do_drive_backup() and
drive_backup_prepare(), and changing qmp_drive_backup() to create a
transaction instead of calling do_backup_common() direcly.

As a side-effect, now qmp_drive_backup() is executed inside a drained
section, as it happens when creating a drive-backup transaction. This
change is visible from the user's perspective, as the job gets paused
and immediately resumed before starting the actual work.

Also fix tests 141, 185 and 219 to cope with the extra
JOB_STATUS_CHANGE lines.

Signed-off-by: Sergio Lopez 
Reviewed-by: Kevin Wolf 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 224 +
 tests/qemu-iotests/141.out |   2 +
 tests/qemu-iotests/185.out |   2 +
 tests/qemu-iotests/219 |   7 +-
 tests/qemu-iotests/219.out |   8 ++
 5 files changed, 117 insertions(+), 126 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 553e315972..5e85fc042e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1761,39 +1761,128 @@ typedef struct DriveBackupState {
 BlockJob *job;
 } DriveBackupState;
 
-static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
-Error **errp);
+static BlockJob *do_backup_common(BackupCommon *backup,
+  BlockDriverState *bs,
+  BlockDriverState *target_bs,
+  AioContext *aio_context,
+  JobTxn *txn, Error **errp);
 
 static void drive_backup_prepare(BlkActionState *common, Error **errp)
 {
 DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
-BlockDriverState *bs;
 DriveBackup *backup;
+BlockDriverState *bs;
+BlockDriverState *target_bs;
+BlockDriverState *source = NULL;
 AioContext *aio_context;
+QDict *options;
 Error *local_err = NULL;
+int flags;
+int64_t size;
+bool set_backing_hd = false;
 
 assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
 backup = common->action->u.drive_backup.data;
 
+if (!backup->has_mode) {
+backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+}
+
 bs = bdrv_lookup_bs(backup->device, backup->device, errp);
 if (!bs) {
 return;
 }
 
+if (!bs->drv) {
+error_setg(errp, "Device has no medium");
+return;
+}
+
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
 /* Paired with .clean() */
 bdrv_drained_begin(bs);
 
-state->bs = bs;
+if (!backup->has_format) {
+backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
+ NULL : (char *) bs->drv->format_name;
+}
+
+/* Early check to avoid creating target */
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+goto out;
+}
+
+flags = bs->open_flags | BDRV_O_RDWR;
+
+/*
+ * See if we have a backing HD we can use to create our new image
+ * on top of.
+ */
+if (backup->sync == MIRROR_SYNC_MODE_TOP) {
+source = backing_bs(bs);
+if (!source) {
+backup->sync = MIRROR_SYNC_MODE_FULL;
+}
+}
+if (backup->sync == MIRROR_SYNC_MODE_NONE) {
+source = bs;
+flags |= BDRV_O_NO_BACKING;
+set_backing_hd = true;
+}
+
+size = bdrv_getlength(bs);
+if (size < 0) {
+error_setg_errno(errp, -size, "bdrv_getlength failed");
+goto out;
+}
+
+if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
+assert(backup->format);
+if (source) {
+bdrv_refresh_filename(source);
+bdrv_img_create(backup->target, backup->format, source->filename,
+source->drv->format_name, NULL,
+size, flags, false, &local_err);
+} else {
+bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL,
+size, flags, false, &local_err);
+}
+}
 
-state->job = do_drive_backup(backup, common->block_job_txn, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto out;
 }
 
+options = qdict_new();
+qdict_put_str(options, "discard", "unmap");
+qdict_put_str(options, "detect-zeroes", "unmap");
+if (backup->format) {
+qdict_put_str(options, "driver", backup->format);
+}
+
+target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
+if (!target_bs) {
+goto out;
+}
+
+if (set_backing_hd) {
+bdrv_set_backing_hd(target_bs, source, &local_err);
+if (local_err) {
+goto unref;
+}
+}
+
+state->bs = bs;
+
+ 

Re: [PATCH v6 2/2] s390: diagnose 318 info reset and migration support

[David Hildenbrand]

On 27.01.20 18:29, Cornelia Huck wrote:
> On Mon, 27 Jan 2020 18:09:11 +0100
> David Hildenbrand  wrote:
> 
> +static void s390_diag318_reset(DeviceState *dev)
> +{
> +if (kvm_enabled())
> +kvm_s390_set_diag318_info(0);
> +}
> +
> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +dc->reset = s390_diag318_reset;
> +dc->vmsd = &vmstate_diag318;
> +dc->hotpluggable = false;
> +/* Reason: Created automatically during machine instantiation */
> +dc->user_creatable = false;
> +}
> +
> +static const TypeInfo s390_diag318_info = {
> +.class_init = s390_diag318_class_init,
> +.parent = TYPE_DEVICE,
> +.name = TYPE_S390_DIAG318,
> +.instance_size = sizeof(DIAG318State),
> +};
> +
> +static void s390_diag318_register_types(void)
> +{
> +type_register_static(&s390_diag318_info);
> +}  

 Do we really need a new device? Can't we simply glue that extended state
 to the machine state?
  
 -> target/s390x/machine.c  
  
>>>
>>> Those VM States relate to the CPU state... does it make sense to store the
>>> diag318 info in a CPU state? (It doesn't seem necessary to store / migrate
>>> this info for each CPU).  
>>
>> I'm sorry, I was looking at the wrong file ...
>>
>>>
>>> Should we store this in the S390CcwMachineState? Or perhaps create a generic
>>> S390MachineState for information that needs to be stored once and migrated
>>> once?  
>>
>> ... I actually thought we have something like this already. Personally,
>> I think that would make sense. At least spapr seems to have something
>> like this already (hw/ppc/spapr.c:spapr_machine_init().
>>
>> @Conny?
> 
> What are you referring to? I only see the one with the FIXME in front
> of it...

That's the one I mean. The fixme states something about qdev ... but
AFAIK that's only applicable if TYPE_DEVICE is involved. So maybe right
now there is no other way than registering the vmstate directly.

-- 
Thanks,

David / dhildenb

Re: [PATCH] tests/acceptance: Add a test for the canon-a1100 machine

[Wainer dos Santos Moschetta]

On 1/27/20 1:41 PM, Philippe Mathieu-Daudé wrote:

On 1/27/20 4:39 PM, Thomas Huth wrote:

On 27/01/2020 16.18, Philippe Mathieu-Daudé wrote:

On 1/27/20 3:41 PM, Thomas Huth wrote:

The canon-a1100 machine can be used with the Barebox firmware. The
QEMU Advent Calendar 2018 features a pre-compiled image which we
can use for testing.

Signed-off-by: Thomas Huth 
---
   tests/acceptance/machine_arm_canon-a1100.py | 33 
+



What is the reason for not adding this case in boot_linux_console suite?



   1 file changed, 33 insertions(+)
   create mode 100644 tests/acceptance/machine_arm_canon-a1100.py

diff --git a/tests/acceptance/machine_arm_canon-a1100.py
b/tests/acceptance/machine_arm_canon-a1100.py
new file mode 100644
index 00..3888168451
--- /dev/null
+++ b/tests/acceptance/machine_arm_canon-a1100.py
@@ -0,0 +1,33 @@
+# Functional test that boots the canon-a1100 machine with firmware
+#
+# Copyright (c) 2020 Red Hat, Inc.
+#
+# Author:
+#  Thomas Huth 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado_qemu import Test
+from avocado_qemu import wait_for_console_pattern
+from avocado.utils import archive
+
+class CanonA1100Machine(Test):
+
+    timeout = 90
+
+    def test_arm_canona1100(self):
+    """
+    :avocado: tags=arch:arm
+    :avocado: tags=machine:canon-a1100


To the maintainer taking this, please add:

    :avocado: tags=pflash_cfi02


Should there be a "device:" between the "=" and the device name? At
least I can see some other files using "device:" for similar tags...


Ah yes you are right, it is clearer.



Notice that avocado_qemu won't automatically convert that tag into 
QEMU's -device option, If that is the intention...


Thanks!

- Wainer

Re: [PATCH] riscv: Add helper to make NaN-boxing for FP register

[Richard Henderson]

On 1/27/20 6:10 AM, Ian Jiang wrote:
> The function that makes NaN-boxing when a 32-bit value is assigned
> to a 64-bit FP register is split out to a helper gen_nanbox_fpr().
> Then it is applied in translating of the FLW instruction.
> 
> This also applies for other instructions when the RVD extension is
> present, such as FMV.W.W, FADD.S, FSUB.S and so on.

I wouldn't mention this yet, as it begs the question of what you are doing
about it.  Which is nothing, yet, since that will apply to a follow-up patch.


> 
> Signed-off-by: Ian Jiang 
> ---
>  target/riscv/insn_trans/trans_rvf.inc.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [PATCH v6 2/2] s390: diagnose 318 info reset and migration support

[Cornelia Huck]

On Mon, 27 Jan 2020 11:39:02 -0500
Collin Walling  wrote:

> On 1/27/20 6:47 AM, Cornelia Huck wrote:
> > On Fri, 24 Jan 2020 17:14:04 -0500
> > Collin Walling  wrote:
> >   
> >> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
> >> be intercepted by SIE and handled via KVM. Let's introduce some
> >> functions to communicate between QEMU and KVM via ioctls. These
> >> will be used to get/set the diag318 information.  
> > 
> > Do you want to give a hint what diag 318 actually does?
> >   
> 
> For the sake of completeness, I'll have to get back to you on this.
> 
> >>
> >> The availability of this instruction is determined by byte 134, bit 0
> >> of the Read Info block. This coincidentally expands into the space used  
> > 
> > "SCLP Read Info"
> >   
> >> for CPU entries by taking away one byte, which means VMs running with
> >> the diag318 capability will not be able to retrieve information regarding
> >> the 248th CPU. This will not effect performance, and VMs can still be
> >> ran with 248 CPUs.  
> > 
> > Are there other ways in which that might affect guests? I assume Linux
> > can deal with it? Is it ok architecture-wise?
> > 
> > In any case, should go into the patch description :)
> >   
> 
> Same as above. I'll try to provide more information regarding what happens
> here in my next reply.

I think you can lift some stuff from the cover letter.

> 
> >>
> >> In order to simplify the migration and system reset requirements of
> >> the diag318 data, let's introduce it as a device class and include
> >> a VMStateDescription.
> >>
> >> Diag318 is set to 0 during modified clear and load normal resets.  
> > 
> > What exactly is set to 0? Stored values?
> >   
> 
> Correct. "The stored values set by DIAG318 are reset to 0 during..."

Sounds good.

> 
> >>
> >> Signed-off-by: Collin Walling 
> >> ---
> >>  hw/s390x/Makefile.objs  |  1 +
> >>  hw/s390x/diag318.c  | 85 
> >> +
> >>  hw/s390x/diag318.h  | 40 +
> >>  hw/s390x/s390-virtio-ccw.c  | 17 
> >>  hw/s390x/sclp.c | 13 ++
> >>  include/hw/s390x/sclp.h |  2 +
> >>  target/s390x/cpu_features.h |  1 +
> >>  target/s390x/cpu_features_def.inc.h |  3 ++
> >>  target/s390x/gen-features.c |  1 +
> >>  target/s390x/kvm-stub.c | 10 +
> >>  target/s390x/kvm.c  | 29 +
> >>  target/s390x/kvm_s390x.h|  2 +
> >>  12 files changed, 204 insertions(+)
> >>  create mode 100644 hw/s390x/diag318.c
> >>  create mode 100644 hw/s390x/diag318.h
> >>  
> > (...)  

> >> +static bool diag318_needed(void *opaque)
> >> +{
> >> +return kvm_enabled() ? s390_has_feat(S390_FEAT_DIAG318) : 0;  
> > 
> > Why do you need to guard this with kvm_enabled()? If tcg does not
> > enable the feature, we should be fine; and if it emulates this in the
> > future, we probably need to migrate something anyway.
> >   
> 
> Your explanation makes sense. My thoughts were to not even bother
> registering the state description if KVM isn't enabled (but I guess
> that thinking would mean that the other kvm_enabled fencing would
> be redundant? Doh.)

My thinking was along the lines how easy it would be to do some tcg
implementation (not sure if that even makes sense.)

> 
> I'll fix this.
> 

> >> @@ -294,6 +307,9 @@ static void ccw_init(MachineState *machine)
> >>  
> >>  /* init the TOD clock */
> >>  s390_init_tod();
> >> +
> >> +/* init object used for migrating diag318 info */
> >> +s390_init_diag318();  
> > 
> > Doesn't that device do a bit more than 'migrating' info?
> > 
> > Also, it seems a bit useless unless you're running with kvm and the
> > feature bit switched on...
> >   
> 
> Right... I think this whole "diag318 device" thing needs some rethinking.
> 
> I made a comment on David's response regarding where to but the 
> VMStateDescription
> code for diag318. Perhaps including the related information within the 
> S390MachineState
> would be better? (I'm not sure).

Replied to David's mail.


> >> @@ -37,10 +39,19 @@ static void prepare_cpu_entries(SCLPDevice *sclp, 
> >> CPUEntry *entry, int *count)
> >>  {
> >>  MachineState *ms = MACHINE(qdev_get_machine());
> >>  uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 };
> >> +int max_entries;
> >>  int i;
> >>  
> >> +/* Calculate the max number of CPU entries that can be stored in the 
> >> SCCB */
> >> +max_entries = (SCCB_SIZE - offsetof(ReadInfo, entries)) / 
> >> sizeof(CPUEntry);
> >> +
> >>  s390_get_feat_block(S390_FEAT_TYPE_SCLP_CPU, features);
> >>  for (i = 0, *count = 0; i < ms->possible_cpus->len; i++) {
> >> +if (*count == max_entries) {
> >> +warn_report("Configuration only supports a max of %d CPU 
> >> entries.",
> >> +max_entries);  
> > 
> > IIUC, this only moans during Read Info... but you co

Re: [PATCH] tests/acceptance: Add boot tests for some of the QEMU advent calendar images

[Wainer dos Santos Moschetta]

On 1/24/20 3:03 PM, Thomas Huth wrote:

The 2018 edition of the QEMU advent calendar 2018 featured Linux images
for various non-x86 machines. We can use them for a boot tests in our
acceptance test suite.

Let's also make sure that we build the corresponding machines in Travis,
and while we're there, drop the superfluous --python parameter (python3
is now the only supported version anyway).

Signed-off-by: Thomas Huth 
---
  .travis.yml|  2 +-
  tests/acceptance/boot_linux_console.py | 96 ++
  2 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 6c1038a0f1..73ca12c921 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -268,7 +268,7 @@ matrix:
  
  # Acceptance (Functional) tests

  - env:
-- CONFIG="--python=/usr/bin/python3 
--target-list=x86_64-softmmu,mips-softmmu,mips64el-softmmu,aarch64-softmmu,arm-softmmu,s390x-softmmu,alpha-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,sparc-softmmu"
+- 
CONFIG="--target-list=aarch64-softmmu,alpha-softmmu,arm-softmmu,m68k-softmmu,microblaze-softmmu,mips-softmmu,mips64el-softmmu,nios2-softmmu,or1k-softmmu,ppc-softmmu,ppc64-softmmu,s390x-softmmu,sparc-softmmu,x86_64-softmmu,xtensa-softmmu"
  - TEST_CMD="make check-acceptance"
after_script:
  - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for 
t in r["tests"] if t["status"] not in ("PASS", "SKIP")]' | xargs cat
diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index e03add2989..f7ac2a3a59 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -584,3 +584,99 @@ class BootLinuxConsole(Test):
  self.wait_for_console_pattern(console_pattern)
  console_pattern = 'No filesystem could mount root'
  self.wait_for_console_pattern(console_pattern)
+
+def do_test_advcal_2018(self, day, tar_hash, kernel_name):
+tar_url = ('https://www.qemu-advent-calendar.org'
+   '/2018/download/day' + day + '.tar.xz')
+file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
+archive.extract(file_path, self.workdir)
+self.vm.set_console()
+self.vm.add_args('-kernel',
+ self.workdir + '/day' + day + '/' + kernel_name)
+self.vm.launch()
+self.wait_for_console_pattern('QEMU advent calendar')
+
+def test_arm_vexpressa9(self):
+"""
+:avocado: tags=arch:arm
+:avocado: tags=machine:vexpress-a9
+"""
+tar_hash = '32b7677ce8b6f1471fb0059865f451169934245b'
+self.vm.add_args('-dtb', self.workdir + '/day16/vexpress-v2p-ca9.dtb')
+self.do_test_advcal_2018('16', tar_hash, 'winter.zImage')
+
+def test_m68k_mcf5208evb(self):
+"""
+:avocado: tags=arch:m68k
+:avocado: tags=machine:mcf5208evb
+"""
+tar_hash = 'ac688fd00561a2b6ce1359f9ff6aa2b98c9a570c'
+self.do_test_advcal_2018('07', tar_hash, 'sanity-clause.elf')
+
+def test_microblaze_s3adsp1800(self):
+"""
+:avocado: tags=arch:microblaze
+:avocado: tags=machine:petalogix-s3adsp1800
+"""
+tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
+self.do_test_advcal_2018('17', tar_hash, 'ballerina.bin')
+
+def test_or1k_sim(self):
+"""
+:avocado: tags=arch:or1k
+:avocado: tags=machine:or1k-sim
+"""
+tar_hash = '20334cdaf386108c530ff0badaecc955693027dd'
+self.do_test_advcal_2018('20', tar_hash, 'vmlinux')
+
+def test_nios2_10m50(self):
+"""
+:avocado: tags=arch:nios2
+:avocado: tags=machine:10m50-ghrd
+"""
+tar_hash = 'e4251141726c412ac0407c5a6bceefbbff018918'
+self.do_test_advcal_2018('14', tar_hash, 'vmlinux.elf')
+
+def test_ppc64_e500(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:ppce500
+"""
+tar_hash = '6951d86d644b302898da2fd701739c9406527fe1'
+self.vm.add_args('-cpu', 'e5500')
+self.do_test_advcal_2018('19', tar_hash, 'uImage')
+
+def test_ppc_g3beige(self):
+"""
+:avocado: tags=arch:ppc
+:avocado: tags=machine:g3beige
+"""
+tar_hash = 'e0b872a5eb8fdc5bed19bd43ffe863900ebcedfc'
+self.vm.add_args('-M', 'graphics=off')
+self.do_test_advcal_2018('15', tar_hash, 'invaders.elf')


Hi Thomas, let me check one thing...

The VM will be launched as:



ppc-softmmu/qemu-system-ppc -display none -vga none -chardev 
socket,id=mon,path=/tmp/tmpvdokyvs3/qemu-41146-monitor.sock -mon 
chardev=mon,mode=control -machine g3beige -chardev 
socket,id=console,path=/tmp/tmpvdokyvs3/qemu-41146-console.sock,server,nowait 
-serial chardev:console -M graphics=off -kernel 
/tmp/avocado_g3uccfo5/avocado_job_61gglyz3/02-tests_accept

Re: [PATCH v6 2/2] s390: diagnose 318 info reset and migration support

[Cornelia Huck]

On Mon, 27 Jan 2020 18:09:11 +0100
David Hildenbrand  wrote:

> >>> +static void s390_diag318_reset(DeviceState *dev)
> >>> +{
> >>> +if (kvm_enabled())
> >>> +kvm_s390_set_diag318_info(0);
> >>> +}
> >>> +
> >>> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
> >>> +{
> >>> +DeviceClass *dc = DEVICE_CLASS(klass);
> >>> +
> >>> +dc->reset = s390_diag318_reset;
> >>> +dc->vmsd = &vmstate_diag318;
> >>> +dc->hotpluggable = false;
> >>> +/* Reason: Created automatically during machine instantiation */
> >>> +dc->user_creatable = false;
> >>> +}
> >>> +
> >>> +static const TypeInfo s390_diag318_info = {
> >>> +.class_init = s390_diag318_class_init,
> >>> +.parent = TYPE_DEVICE,
> >>> +.name = TYPE_S390_DIAG318,
> >>> +.instance_size = sizeof(DIAG318State),
> >>> +};
> >>> +
> >>> +static void s390_diag318_register_types(void)
> >>> +{
> >>> +type_register_static(&s390_diag318_info);
> >>> +}  
> >>
> >> Do we really need a new device? Can't we simply glue that extended state
> >> to the machine state?
> >>  
> >> -> target/s390x/machine.c  
> >>  
> > 
> > Those VM States relate to the CPU state... does it make sense to store the
> > diag318 info in a CPU state? (It doesn't seem necessary to store / migrate
> > this info for each CPU).  
> 
> I'm sorry, I was looking at the wrong file ...
> 
> > 
> > Should we store this in the S390CcwMachineState? Or perhaps create a generic
> > S390MachineState for information that needs to be stored once and migrated
> > once?  
> 
> ... I actually thought we have something like this already. Personally,
> I think that would make sense. At least spapr seems to have something
> like this already (hw/ppc/spapr.c:spapr_machine_init().
> 
> @Conny?

What are you referring to? I only see the one with the FIXME in front
of it...

> 
> [...]
> > 
> > How about we introduce a union in the ReadInfo struct. Something like:
> > 
> > union {
> > uint8_t  byte_134;
> > struct CPUEntry entries[0];
> > } x;  
> 
> Or drop the "entries" pointer completely and introduce
> 
> static int cpu_entries_offset(void)
> {
> /*
>  * When we have to indicate features in byte 134, we have to move
>  * the start of the cpu entries.
>  */
> if (s390_has_feat(S390_FEAT_DIAG318)) {
> return 144;
> }
> return 128;
> }
> 
> struct CPUEntry *cpu_entries(ReadInfo *ri)
> {
> return (struct CPUEntry *)((void *)ri + cpu_entries_offset());
> }
> 
> unsigned int cpu_entries)count(ReadInfo *ri)
> {
> return (SCCB_SIZE - cpu_entries_offset()) / sizeof(CPUEntry);
> }
> 
> etc. (might take some tweaking to make it compile) and a comment for the
> struct. Not sure what's better. Having two struct CPUEntry entries[0] is
> also confusing.

I think that version may end up looking better.

Re: [PATCH 7/8] tests/vm: Added a new script for ubuntu.aarch64.

[Andrew Jones]

On Mon, Jan 27, 2020 at 11:47:35AM -0500, Robert Foley wrote:
> On Mon, 27 Jan 2020 at 10:01, Alex Bennée  wrote:
> > >  vm-boot-ssh-%: $(IMAGES_DIR)/%.img
> > >   $(call quiet-command, \
> > > - $(SRC_PATH)/tests/vm/$* \
> > > + $(PYTHON) $(SRC_PATH)/tests/vm/$* \
> >
> > This seems like it should be in a different patch.
> 
> Good point, will move it to a different patch.
> 
> > > +
> > > +DEFAULT_CONFIG = {
> > > +'cpu'  : "max",
> > > +'machine'  : "virt,gic-version=max",
> >
> > According to virt.c:
> >
> >   Valid values are 2, 3 and host
> >
> > but max should work on TCG. However we need a more recent QEMU than my
> > system one for it to work. Otherwise you see:
> >
> >   DEBUG:qemu.machine:Error launching VM
> 
> Good point.  We were trying to avoid having different values for KVM
> vs TCG, which we
> could do with the latest QEMU.
> We will update this to make sure this works with older versions of QEMU as 
> well.
> 
> On my system I have qemu 2.11.1 installed by default.
> It seems that we will need the following defaults based on our environment.
> 
> For KVM we end up with the below args since max cpu and max
> gic-version is not available.
> kvm:  -cpu host -machine virt,gic-version=host
> 
> For TCG max cpu is also not available: qemu-system-aarch64: unable to
> find CPU model 'max',
> so we pick cortex-a57.
> TCG: -cpu cortex-a57 -machine virt,gic-version=3
> 
> I suppose we could check the version of QEMU and use the above
> defaults only for earlier versions of QEMU.
> This is something we will probably move to aarch64vm.py since it is common.

What versions of QEMU do these tests *have* to support? Because we could
just skip the tests for QEMU that doesn't support cpu=max,gic-version=max.
'max' is indeed the nicest selection for using the same command line on
KVM (gicv2 and gicv3 hosts) and TCG.

Thanks,
drew