Re: [Qemu-devel] [Bug] cirrus_vga: qemu abort at booting when configure vgamem_mb <= 2

[Gerd Hoffmann]

On Fr, 2014-05-09 at 03:47 +, Gonglei (Arei) wrote:
> Hi, Gerd
> 
> The issue consequentially occur, I have tested various qemu versions, 
> including the current qemu.git. 

Missing sanity check.  It's not valid.  We mimic existing hardware here,
and the cirrus card emulated has 4 MB video memory.  So ideally we
should just use that and be done with it.  Problem is qemu has
historically used 8 or 16 mb vga memory for cirrus, and simply changing
it will break live migration.

So cirrus should accept 4 MB (correct value), 8 MB and 16 MB (for
backward compatibility) and reject everything else.  Patches are
welcome ;)

If you want reduce the qemu memory footprint use stdvga instead which
should handle memory sized from 1 MB to 256 MB just fine.

cheers,
  Gerd

[Qemu-devel] [Bug 1316115] Re: linux-user qemu-arm NEON support

[Christopher Horler]

I didn't test it on real hardware yet - but I resolved the issue and
found the root cause last night:

This perhaps should have been more obvious to me in the beginning, but "readelf 
-l" shows a program header similar to this:
  INTERP 0x00394600 0x00394600 0x00394600
 0x001c 0x001c  R  10
  [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]


This triggered a 9 month old memory of me fixing the Qt4.8 project file (used 
to create the Makefile) to ensure the correct loader (program interpreter).

Meanwhile, upstream made this patch in Qt5 - which I don't want, when I
revert it and implement what I had before I get the expected result
under qemu - it runs.

https://qt.gitorious.org/qt/qtbase/commit/b2a45e02a23fcbc9db29d700e2abaf627a1fdedf

(the !cross_compile causes the variables not to be set, my own patch for
Qt 4.8 was setting these from buildroot / patch)

In the default unpatched case for a cross-compiled build, the shared
library is not directly executable because the entry point and
interpreter define never get set (eliminating the code that outputs the
desired specific version information!)

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

Title:
  linux-user qemu-arm NEON support

Status in QEMU:
  New

Bug description:
  I was reading the mailing list and saw NEON support in QEmu was making
  progress.

  Is it not supported in user mode?  or am I running into something else
  here?  (I've tried to include some what may be useful information)

  using qemu from git (last commits as below):
  fdaad47 Merge remote-tracking branch 
'remotes/pmaydell/tags/pull-target-arm-20140501' into staging
  e50bf23 Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into 
staging
  c090c10 Merge remote-tracking branch 'remotes/cohuck/tags/kvm_cap_helpers' 
into staging

  (for completeness I should point out this is not actually
  libQtCore.so.4.6.2 - the SONAME shows libQt5Core.so.5).

  chorler@linux-foxtrot:~/projects/src/CustomFirmware> qemu-arm -L ./root 
./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2 
  qemu: unhandled CPU exception 0x2 - aborting
  R00= R01=f6c84fdd R02= R03=
  R04= R05= R06= R07=
  R08= R09= R10=f6ff9d80 R11=
  R12= R13=f6c84d90 R14= R15=f6cdef74
  PSR=0010  A usr32
  qemu: uncaught target signal 6 (Aborted) - core dumped
  Aborted

  
  chorler@linux-foxtrot:~/projects/src/CustomFirmware> 
arm-linux-gnueabihf-readelf -A 
./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2 
  Attribute Section: aeabi
  File Attributes
Tag_CPU_name: "7-A"
Tag_CPU_arch: v7
Tag_CPU_arch_profile: Application
Tag_ARM_ISA_use: Yes
Tag_THUMB_ISA_use: Thumb-2
Tag_FP_arch: VFPv3
Tag_Advanced_SIMD_arch: NEONv1
Tag_ABI_PCS_wchar_t: 4
Tag_ABI_FP_denormal: Needed
Tag_ABI_FP_exceptions: Needed
Tag_ABI_FP_number_model: IEEE 754
Tag_ABI_align_needed: 8-byte
Tag_ABI_align_preserved: 8-byte, except leaf SP
Tag_ABI_enum_size: int
Tag_ABI_HardFP_use: SP and DP
Tag_ABI_VFP_args: VFP registers
Tag_ABI_optimization_goals: Aggressive Speed
Tag_CPU_unaligned_access: v6
Tag_DIV_use: Not allowed


  chorler@linux-foxtrot:~/projects/src/CustomFirmware> gdb qemu-arm
  GNU gdb (GDB; openSUSE 13.1) 7.6.50.20130731-cvs
  Copyright (C) 2013 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later 
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
  and "show warranty" for details.
  This GDB was configured as "x86_64-suse-linux".
  Type "show configuration" for configuration details.
  For bug reporting instructions, please see:
  .
  Find the GDB manual and other documentation resources online at:
  .
  For help, type "help".
  Type "apropos word" to search for commands related to "word".
  ..
  Reading symbols from /home/chorler/projects/bin/qemu-arm...done.
  (gdb) list main.c:685
  680
  681 for(;;) {
  682 cpu_exec_start(cs);
  683 trapnr = cpu_arm_exec(env);
  684 cpu_exec_end(cs);
  685 switch(trapnr) {
  686 case EXCP_UDEF:
  687 {
  688 TaskState *ts = cs->opaque;
  689 uint32_t opcode;
  (gdb) break main.c:685
  Breakpoint 3 at 0x60059773: file 
/home/chorler/projects/src/qemu/linux-user/main.c, line 685.
  (gdb) run -L ./root 
./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2
  Starting program: /home/chorler/projects/bin/qemu-arm -L ./root 
.

Re: [Qemu-devel] [PATCH] preallocate memory after NUMA policy configuration

[Hu Tao]

On Fri, May 09, 2014 at 03:24:52AM -0300, Marcelo Tosatti wrote:
> 
> (note: applies on top of patchset of current thread)
> 
> Preallocate memory after the NUMA policy has been instantiated.
> This is necessary to guarantee memory is allocated with 
> specified NUMA policy in place.
> 
> Signed-off-by: Marcelo Tosatti 
> 
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index d3f8476..3cfa8a0 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -293,9 +293,6 @@ host_memory_backend_memory_init(UserCreatable *uc, Error 
> **errp)
>  if (!backend->dump) {
>  qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP);
>  }
> -if (backend->prealloc) {
> -os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
> -}
>  
>  #ifdef CONFIG_NUMA
>  unsigned long maxnode = find_last_bit(backend->host_nodes, MAX_NODES);
> @@ -310,6 +307,9 @@ host_memory_backend_memory_init(UserCreatable *uc, Error 
> **errp)
>  return;
>  }
>  #endif
> +if (backend->prealloc) {
> +os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
> +}
>  }
>  
>  MemoryRegion *

Thanks, I'll squash it in patch 25.

Regards,
Hu Tao

[Qemu-devel] [PATCH] preallocate memory after NUMA policy configuration

[Marcelo Tosatti]

(note: applies on top of patchset of current thread)

Preallocate memory after the NUMA policy has been instantiated.
This is necessary to guarantee memory is allocated with 
specified NUMA policy in place.

Signed-off-by: Marcelo Tosatti 

diff --git a/backends/hostmem.c b/backends/hostmem.c
index d3f8476..3cfa8a0 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -293,9 +293,6 @@ host_memory_backend_memory_init(UserCreatable *uc, Error 
**errp)
 if (!backend->dump) {
 qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP);
 }
-if (backend->prealloc) {
-os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
-}
 
 #ifdef CONFIG_NUMA
 unsigned long maxnode = find_last_bit(backend->host_nodes, MAX_NODES);
@@ -310,6 +307,9 @@ host_memory_backend_memory_init(UserCreatable *uc, Error 
**errp)
 return;
 }
 #endif
+if (backend->prealloc) {
+os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
+}
 }
 
 MemoryRegion *

Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent.

[Markus Armbruster]

Benoît Canet  writes:

> The Thursday 08 May 2014 à 20:30:33 (+0200), Markus Armbruster wrote :
[...]
>> There are two reasons to detect cycles.  The technical one is preventing
>> infinite expansion.  No longer applies with idempotent include.  The
>> other, more abstract one is rejecting nonsensical use of the include
>> directive.  I think that one still applies.
[...]
>> > @@ -102,17 +102,16 @@ class QAPISchema:
>> >  'Expected a file name (string), 
>> > got: %s'
>> >  % include)
>> >  include_path = os.path.join(self.input_dir, include)
>> > -if any(include_path == elem[1]
>> > -   for elem in self.include_hist):
>> > -raise QAPIExprError(expr_info, "Inclusion loop for %s"
>> > -% include)
>> > +# make include idempotent by skipping further includes
>> > +if include_path in [x[1] for x in  include_hist]:
>> > +continue
>> 
>> Loses cycle detection.
>
> It simply also skip cycle includes. If cycle are skipped then cannot do no
> harm.

Your argument is based exclusively on the technical reason to detect
cycles: cycles need to be caught because they cause infinite recursion.
Since there is no infinite recursion with idempotent include, cycles are
just fine.

I'm arguing from a more abstract point of view: cycles should be
rejected because they're nonsensical.  The fact that they can cause
infinite recursion is an implementation detail.  Even without infinite
recursion, they're just as nonsensical as ever.

[...]

Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration

[Gonglei (Arei)]

Hi,

> -Original Message-
> From: Michael R. Hines [mailto:mrhi...@linux.vnet.ibm.com]
> Sent: Tuesday, April 01, 2014 8:42 AM
> To: Gonglei (Arei); qemu-devel@nongnu.org
> Cc: Huangweidong (C); quint...@redhat.com; dgilb...@redhat.com;
> owass...@redhat.com; mrhi...@us.ibm.com; Moyuxiang;
> pbonz...@redhat.com
> Subject: Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration
> 
> On 03/29/2014 03:39 PM, arei.gong...@huawei.com wrote:
> > From: Mo Yuxiang 
> >
> > If the networking break or there's something wrong with rdma
> > device(ib0 with no IP) during rdma migration, the main_loop of
> > qemu will be blocked in rdma_destroy_id. I add rdma_ack_cm_event
> > to fix this bug.
> >
> > Signed-off-by: Mo Yuxiang 
> > Signed-off-by: Gonglei 
> > ---
> >   migration-rdma.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/migration-rdma.c b/migration-rdma.c
> > index eeb4302..f60749b 100644
> > --- a/migration-rdma.c
> > +++ b/migration-rdma.c
> > @@ -949,6 +949,7 @@ route:
> >   ERROR(errp, "result not equal to event_addr_resolved %s",
> >   rdma_event_str(cm_event->event));
> >   perror("rdma_resolve_addr");
> > +rdma_ack_cm_event(cm_event);
> >   ret = -EINVAL;
> >   goto err_resolve_get_addr;
> >   }
> 
> Reviewed-by: Michael R. Hines 
> 
> Good catch. =) That's an obvious bug. It looks like I need
> to do a much better job of "kill -9" inside the regression
> testing scripts - probably i should try killing the migration
> prematurely at different periods just to be sure there are
> no more places where the connection state is not getting
> cleaned up..
> 
> - Michael
> 
Michael, do you have a plan to pull this patch to master? Thanks.

Best regards,
-Gonglei

[Qemu-devel] [Bug] cirrus_vga: qemu abort at booting when configure vgamem_mb <= 2

[Gonglei (Arei)]

Hi, Gerd

The issue consequentially occur, I have tested various qemu versions, 
including the current qemu.git. 

Any ideas? Thanks.

The command line:

./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name sles \ 
-boot c -drive file=/mnt/sdb/gonglei/image/sles.img -vnc 0.0.0.0:10 -monitor \
stdio -device cirrus-vga,id=video0,vgamem_mb=2 


The backtrace:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x72785700 (LWP 13966)]
0x556ceb09 in cirrus_vga_mem_write (opaque=0x5638e888, addr=0, 
mem_value=0, size=1) at hw/display/cirrus_vga.c:2039
2039*(s->vga.vram_ptr + bank_offset) = mem_value;
(gdb) bt
#0  0x556ceb09 in cirrus_vga_mem_write (opaque=0x5638e888, addr=0, 
mem_value=0, size=1) at hw/display/cirrus_vga.c:2039
#1  0x558bc394 in memory_region_write_accessor (mr=0x5639f5a8, 
addr=0, value=0x727848a8, size=1, shift=0, mask=255)
at /mnt/sdb/gonglei/code/qemu/memory.c:441
#2  0x558bc4d0 in access_with_adjusted_size (addr=0, 
value=0x727848a8, size=2, access_size_min=1, access_size_max=1, 
access=0x558bc30b , mr=0x5639f5a8) at 
/mnt/sdb/gonglei/code/qemu/memory.c:478
#3  0x558bf331 in memory_region_dispatch_write (mr=0x5639f5a8, 
addr=0, data=0, size=2)
at /mnt/sdb/gonglei/code/qemu/memory.c:985
#4  0x558c2b47 in io_mem_write (mr=0x5639f5a8, addr=0, val=0, 
size=2) at /mnt/sdb/gonglei/code/qemu/memory.c:1744
#5  0x55847c5b in address_space_rw (as=0x561e8600 
, addr=655360, buf=0x77ff4030 "", len=2, 
is_write=true) at /mnt/sdb/gonglei/code/qemu/exec.c:2029
#6  0x558480c2 in cpu_physical_memory_rw (addr=655360, 
buf=0x77ff4030 "", len=2, is_write=1)
at /mnt/sdb/gonglei/code/qemu/exec.c:2103
#7  0x558b90c7 in cpu_physical_memory_write (addr=655360, 
buf=0x77ff4030, len=2)
at /mnt/sdb/gonglei/code/qemu/include/exec/cpu-common.h:68
#8  0x558b9025 in kvm_flush_coalesced_mmio_buffer () at 
/mnt/sdb/gonglei/code/qemu/kvm-all.c:1607
#9  0x55844c08 in qemu_flush_coalesced_mmio_buffer () at 
/mnt/sdb/gonglei/code/qemu/exec.c:976
#10 0x558bc34a in memory_region_write_accessor (mr=0x5639f5a8, 
addr=170, value=0x72784b58, size=1, shift=0, mask=
255) at /mnt/sdb/gonglei/code/qemu/memory.c:437
#11 0x558bc4d0 in access_with_adjusted_size (addr=170, 
value=0x72784b58, size=2, access_size_min=1, access_size_max=1, 
access=0x558bc30b , mr=0x5639f5a8) at 
/mnt/sdb/gonglei/code/qemu/memory.c:478
#12 0x558bf331 in memory_region_dispatch_write (mr=0x5639f5a8, 
addr=170, data=0, size=2)
at /mnt/sdb/gonglei/code/qemu/memory.c:985
#13 0x558c2b47 in io_mem_write (mr=0x5639f5a8, addr=170, val=0, 
size=2) at /mnt/sdb/gonglei/code/qemu/memory.c:1744
#14 0x55847c5b in address_space_rw (as=0x561e8600 
, addr=655530, buf=0x77ff2028 "", len=2, 
is_write=true) at /mnt/sdb/gonglei/code/qemu/exec.c:2029
#15 0x558480c2 in cpu_physical_memory_rw (addr=655530, 
buf=0x77ff2028 "", len=2, is_write=1)
at /mnt/sdb/gonglei/code/qemu/exec.c:2103
#16 0x558b940b in kvm_cpu_exec (cpu=0x562a7aa0) at 
/mnt/sdb/gonglei/code/qemu/kvm-all.c:1704
#17 0x55838de2 in qemu_kvm_cpu_thread_fn (arg=0x562a7aa0) at 
/mnt/sdb/gonglei/code/qemu/cpus.c:873
#18 0x759337f6 in start_thread () from /lib64/libpthread.so.0
#19 0x7568f09d in clone () from /lib64/libc.so.6
#20 0x in ?? ()


Best regards,
-Gonglei

Re: [Qemu-devel] virtio-serial-pci very expensive during live migration

[Chris Friesen]

On 05/08/2014 07:44 PM, ChenLiang wrote:

> Hi,
> I have test the patch at the qemu.git, qemu crashed when vm is booting.
> 
> the backtrace is:
> 
> Program received signal SIGABRT, Aborted.
> [Switching to Thread 0x7f6bf67f9700 (LWP 9740)]
> 0x7f6bfacb2b55 in raise () from /lib64/libc.so.6
> (gdb) bt
> #0  0x7f6bfacb2b55 in raise () from /lib64/libc.so.6
> #1  0x7f6bfacb4131 in abort () from /lib64/libc.so.6
> #2  0x7f6bfd51047c in kvm_io_ioeventfd_del (listener=
>  0x7f6bfd9ffee0 , section=0x7f6bf67f87c0, 
> match_data=true, data=
>  0, e=0x7f697930) at /tmp/qemu/kvm-all.c:879
> #3  0x7f6bfd5163b5 in address_space_add_del_ioeventfds (as=
>  0x7f6bfde3d6e0 , fds_new=0x0, fds_new_nb=0, fds_old=
>  0x7f6bfdfd8ce0, fds_old_nb=1) at /tmp/qemu/memory.c:628
> #4  0x7f6bfd51698e in address_space_update_ioeventfds (as=
>  0x7f6bfde3d6e0 ) at /tmp/qemu/memory.c:687
> #5  0x7f6bfd517949 in address_space_update_topology (as=
>  0x7f6bfde3d6e0 ) at /tmp/qemu/memory.c:780
> #6  0x7f6bfd517a68 in memory_region_transaction_commit ()
>  at /tmp/qemu/memory.c:800
> #7  0x7f6bfd3e2942 in virtio_pci_stop_ioeventfd (proxy=0x7f6bfdfde080)
>  at hw/virtio/virtio-pci.c:270


Hah...I attached gdb to the source qemu this time and got basically the same 
thing.
This is with the stable-1.4 branch plus the patch, using "-machine accel=kvm".

#0  0x7f4e08e1e9e9 in raise () from /lib64/libc.so.6
#1  0x7f4e08e200f8 in abort () from /lib64/libc.so.6
#2  0x7f4e0b30e8a4 in kvm_io_ioeventfd_del (listener=, 
section=0x7f4e080f2c00, match_data=, data=0, e=) 
at /home/cfriesen/devel/qemu/kvm-all.c:804
#3  0x7f4e0b311f60 in address_space_add_del_ioeventfds (fds_old_nb=64, 
fds_old=0x7f4df8004b40, fds_new_nb=0, fds_new=0x0, as=0x7f4e0bfb9fe0 
) at /home/cfriesen/devel/qemu/memory.c:603
#4  address_space_update_ioeventfds (as=0x7f4e0bfb9fe0 ) at 
/home/cfriesen/devel/qemu/memory.c:649
#5  address_space_update_topology (as=0x7f4e0bfb9fe0 ) at 
/home/cfriesen/devel/qemu/memory.c:730
#6  memory_region_transaction_commit () at 
/home/cfriesen/devel/qemu/memory.c:750
#7  0x7f4e0b255cb5 in virtio_pci_stop_ioeventfd (proxy=0x7f4e0d3266a0) at 
hw/virtio-pci.c:259
#8  0x7f4e0b3086d0 in virtio_vmstate_change (opaque=0x7f4e0d31c370, 
running=, state=) at 
/home/cfriesen/devel/qemu/hw/virtio.c:914
#9  0x7f4e0b2bdf5b in vm_state_notify (running=running@entry=0, 
state=state@entry=RUN_STATE_FINISH_MIGRATE) at vl.c:1674
#10 0x7f4e0b2c3f62 in do_vm_stop (state=RUN_STATE_FINISH_MIGRATE) at 
/home/cfriesen/devel/qemu/cpus.c:446
#11 vm_stop (state=state@entry=RUN_STATE_FINISH_MIGRATE) at 
/home/cfriesen/devel/qemu/cpus.c:1080
#12 0x7f4e0b25e5c5 in buffered_file_thread (opaque=0x7f4e0b79a640 
) at migration.c:707
#13 0x7f4e09cacc53 in start_thread () from /lib64/libpthread.so.0
#14 0x7f4e08ededbd in clone () from /lib64/libc.so.6

Chris

Re: [Qemu-devel] My OS hangup in KVM for some reasons, how can I debug?

[Jun Koi]

On Thu, May 8, 2014 at 4:28 PM, Jun Koi  wrote:

> Hi,
>
> I have an weird OS that I am trying to boot in KVM. however, it just hang
> in the middle, without a good reason.
>
> The same OS boots fine in physical machine, and this OS comes with no
> source code.
>
> There must be a bug somewhere in KVM, so I am wondering how people debug
> deal with such a case to find and fix bugs in KVM?
>
> Same thing like this happened before with some new OSes (such as Windows
> 8), and somebody always came up with a quick fix, so I am wondering what
> the magic is here.
>
>
Any help, please?

thanks.
Jun

Re: [Qemu-devel] [PATCH 1/6] xics: add flags for interrupts

[Alexey Kardashevskiy]

On 05/07/2014 11:09 PM, Mike Day wrote:
> 
>>  
>>  for (i = 0; i < ics->nr_irqs; i++) {
>>  /* FIXME: filter by server#? */
>> -if (ics->islsi[i]) {
>> +if (ics->irqs[i].flags & XICS_FLAGS_LSI) {
>>  resend_lsi(ics, i);
> 
> Not part of your patch, but I'm curious about this FIXME (there's an
> identical FIXME in resend_msi). Has this proved to be a problem?

ics_resend -> resend_lsi() -> icp_irq() and the last one delivers interrupt
to the correct server. I should probably remove that FIXME or I am missing
something here.

> With
> these patches you could have many unallocated interrupts in array AFTER
> the last allocated interrupt, correct?

I would have unallocated interrupts before this patch too.

> In that case, the loop would
> continue beyond the last allocated interrupt for no purpose. 

For LSI we check the type, if it is not, it is assumed MSI and then
resend_msi() would check for XICS_STATUS_REJECTED which is not set for
non-allocated interrupt so we are safe.

But since I can tell now if it is allocated or not, I will fix it to call
resend_msi() on only if irq is allocated as MSI.

> There are
> a couple ways to mitigate this type of situation by using alternative
> data structures to inform the loop traversal. I don't know if it is
> worth the effort, though.

Here I lost you :)

btw I just realized that in patch#2 it should be xics_find_source instead
of xics_find_server. There are many interrupt servers already and just one
interrupt source (we could have many like one per PHB or something like
that but we are not there yet), this is what I meant.


> 
> 
>> +/* @flags == 0 measn the interrupt is not allocated */
>> +#define XICS_FLAGS_LSI 0x1
>> +#define XICS_FLAGS_MSI 0x2
> 
> (nit) typo in the above comment
> 
> Mike
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH v3.1 00/31] NUMA series, and hostmem improvements

[Hu Tao]

On Thu, May 08, 2014 at 04:51:56PM +0200, Paolo Bonzini wrote:
> Il 06/05/2014 11:27, Hu Tao ha scritto:
> > This series includes work on QOMifying the memory backends.
> > the idea is to delegate all properties of the memory backend to
> > a new QOM class hierarchy, in which the concrete classes
> > are hostmem-ram and hostmem-file.  The backend is passed to the
> > machine via "-numa node,memdev=foo" where "foo" is the id of the
> > backend object.
> 
> Hello,
> 
> I noticed now that if you have the host-nodes property set Linux
> requires you to set a policy other than "default" too.  If you don't,
> the mbind system call fails.
> 
> What about squashing something like this?
> 
> Paolo
> 
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index d3f8476..a0a3111 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -299,12 +299,23 @@ host_memory_backend_memory_init(UserCreatable *uc, 
> Error **errp)
>  
>  #ifdef CONFIG_NUMA
>  unsigned long maxnode = find_last_bit(backend->host_nodes, MAX_NODES);
> +unsigned policy = backend->policy;
> +
> +/* Linux does not accept MPOL_DEFAULT with nonzero bitmap, but
> + * "-object memory-ram,size=128M,hostnodes=0,policy=bind" is a
> + * bit of a mouthful.  So if the host_nodes bitmap is nonzero,
> + * pick the BIND policy.
> + */
> +if (find_first_bit(backend->host_nodes, MAX_NODES) != MAX_NODES &&
> +policy == MPOL_DEFAULT) {
> +policy = MPOL_BIND;
> +}

Looks reasonable to me. Thanks!

>  
>  /* This is a workaround for a long standing bug in Linux'
>   * mbind implementation, which cuts off the last specified
>   * node.
>   */
> -if (mbind(ptr, sz, backend->policy, backend->host_nodes, maxnode + 2, 
> 0)) {
> +if (mbind(ptr, sz, policy, backend->host_nodes, maxnode + 2, 0)) {
>  error_setg_errno(errp, errno,
>   "cannot bind memory to host NUMA nodes");

Re: [Qemu-devel] Question about RAM migration flags

[ChenLiang]

On 2014/5/9 7:08, Peter Lieven wrote:

> Hi,
> 
> while working on ram migration and reading through the code I realized that
> qemu does not stop loading a saved VM or rejecting an incoming migration
> if there is a flag in the stream that it does not understand. An unknown flag
> is simply ignored.
> 
> In the block migration code there is a catch at the end complaining about
> unknown flags, but in RAM migration there isn't.
> 
> Is this on purpose or an error?
> 
> Peter
> 
> 
> 


IMO, the flag in ram_load always is correct if the code of ram_save is correct.
Of course, it will be much rubost to do it as block migration.

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

[Rusty Russell]

Alexander Graf  writes:
> On 05/07/2014 12:19 PM, Greg Kurz wrote:
> The uglyness about the current_cpu bit is that devices are usually not 
> supposed to know about the cpu accesses come from usually. But then 
> again devices shouldn't know about the endianness of a cpu either so I 
> guess it's ok to breach layers here.
>
> Rusty, do you have strong feelings either way?

My only strong feeling is that qemu is horrible to get patches into.

At this point, I really don't care :(

Rusty.

Re: [Qemu-devel] [PATCH] kvmclock: Ensure time in migration never goes backward

[Marcelo Tosatti]

On Thu, May 08, 2014 at 09:21:29AM +0200, Alexander Graf wrote:
> 
> On 08.05.14 03:33, Marcelo Tosatti wrote:
> >On Mon, May 05, 2014 at 03:51:22PM +0200, Alexander Graf wrote:
> >>When we migrate we ask the kernel about its current belief on what the guest
> >>time would be. However, I've seen cases where the kvmclock guest structure
> >>indicates a time more recent than the kvm returned time.
> >>
> >>To make sure we never go backwards, calculate what the guest would have seen
> >>as time at the point of migration and use that value instead of the kernel
> >>returned one when it's more recent.
> >>
> >>While this doesn't fix the underlying issue that the kernel's view of time
> >>is skewed, it allows us to safely migrate guests even from sources that are
> >>known broken.
> >>
> >>Signed-off-by: Alexander Graf 
> >OK Alexander better move this logic to the kernel, in KVM_GET_CLOCK.
> >
> >Otherwise every user of KVM_GET_CLOCK would have to apply the
> >workaround.
> 
> Well, the breakage occurs on the *source* of things. So if I have
> 100 VMs running, I'm pretty sure one of them gets hit by this bug.
> 
> If I put the workaround in the kernel, I have to take the chance to
> break some of those 100 VMs to get things rolling. If I put it in
> QEMU, I can live with a broken migration source.
> 
> This gets even worse when you want to phase out unfixed host kernels.
> 
> 
> Alex

Alex,

Unability to upgrade systems is not an excuse to fix the bug in the
wrong place. 

I'll send a kernel patch.

Re: [Qemu-devel] [PATCH 6/6] xics: implement xics_ics_free()

[Alexey Kardashevskiy]

On 05/08/2014 10:07 PM, Alexander Graf wrote:
> On 05/07/2014 08:01 AM, Alexey Kardashevskiy wrote:
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>   hw/intc/xics.c| 24 
>>   include/hw/ppc/xics.h |  1 +
>>   trace-events  |  2 ++
>>   3 files changed, 27 insertions(+)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index faf304c..2316519 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -755,6 +755,30 @@ int xics_alloc_block(XICSState *icp, int server, int
>> num, bool lsi, bool align)
>>   return first + ics->offset;
>>   }
>>   +static void ics_free(ICSState *ics, int srcno, int num)
>> +{
>> +int i;
>> +
>> +for (i = srcno; i < srcno + num; ++i) {
>> +if (ICS_IRQ_FREE(ics, i)) {
>> +trace_xics_ics_free_warn(ics - ics->icp->ics, i + ics->offset);
>> +}
>> +memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
> 
> Why does this do something different from ics_reset?


ics_reset() resets the interrupt state which still remains allocated and
assigned to some device. ics_free() clears the descriptor and this
interrupt may be now returned if someone asks for a new interrupt (pci
hotplug, intx->msi switch).


> Alex
> 
>> +}
>> +}
>> +
>> +void xics_free(XICSState *icp, int server, int irq, int num)
>> +{
>> +ICSState *ics = &icp->ics[server];
>> +
>> +assert(server == 0);
>> +
>> +trace_xics_ics_free(ics - icp->ics, irq, num);
>> +if (server >= 0) {
>> +ics_free(ics, irq - ics->offset, num);
>> +}
>> +}
>> +
>>   /*
>>* Guest interfaces
>>*/
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 0f01a21..e5d8f47 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -158,6 +158,7 @@ struct ICSIRQState {
>>   qemu_irq xics_get_qirq(XICSState *icp, int irq);
>>   int xics_alloc(XICSState *icp, int server, int irq_hint, bool lsi);
>>   int xics_alloc_block(XICSState *icp, int server, int num, bool lsi,
>> bool align);
>> +void xics_free(XICSState *icp, int server, int irq, int num);
>> void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
>>   diff --git a/trace-events b/trace-events
>> index 8cf8fb2..5ca126c 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -1179,6 +1179,8 @@ xics_alloc(int server, int irq) "server#%d, irq %d"
>>   xics_alloc_failed_hint(int server, int irq) "server#%d, irq %d is
>> already in use"
>>   xics_alloc_failed_no_left(int server) "server#%d, no irq left"
>>   xics_alloc_block(int server, int first, int num, bool lsi, int align)
>> "server#%d, first irq %d, %d irqs, lsi=%d, alignnum %d"
>> +xics_ics_free(int server, int irq, int num) "server#%d, first irq %d, %d
>> irqs"
>> +xics_ics_free_warn(int server, int irq) "server#%d, irq %d is already free"
>> # hw/ppc/spapr_iommu.c
>>   spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t
>> ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
> 


-- 
Alexey

[Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module?

[Gonglei (Arei)]

Hi,

Vhost devices need to do VHOST_SET_MEM_TABLE ioctl in vhost_dev_start()
to tell vhost kernel modules GPA to HVA memory mappings, which consume is 
expensively. 
The reason is same as KVM_SET_GSI_ROUTING ioctl. That is, in ioctl processing, 
kmod and vhost calls synchronize_rcu() to wait for grace period to free old 
memory.
 
In KVM_SET_GSI_ROUTING case, we cannot simply change synchronize_rcu to 
call_rcu, 
since this may leads to DOS attacks if guest VM keeps setting IRQ affinity.
 
In VHOST_SET_MEM_TABLE case, I wonder if we can change synchronize_rcu() to 
call_rcu(), 
i.e., is it possible to trigger DOS attack in guest? There are some cases QEMU 
would do 
VHOST_SET_MEM_TABLE ioctl, like VM start/reboot/attach vhost devices, and RAM 
memory 
regions in system memory address space change. 

And I'd like to know if guest activities could lead to RAM memory regions 
change?
 
Can you give me some advices? Thanks!


Best regards,
-Gonglei

Re: [Qemu-devel] virtio-serial-pci very expensive during live migration

[ChenLiang]

On 2014/5/8 22:34, Paolo Bonzini wrote:

> Il 08/05/2014 16:31, Chris Friesen ha scritto:
>>
>>
>> The fact remains that qemu crashes when I apply the patch.  I also tried
>> patching it as below in virtio_pci_vmstate_change().  That would allow
>> the VM to boot, but it would crash when I tried to do a live migration.
> 
> Can you give us your command line and a backtrace?
> 
> Paolo
> 
> 
> 

Hi,
I have test the patch at the qemu.git, qemu crashed when vm is booting.

the backtrace is:

Program received signal SIGABRT, Aborted.
[Switching to Thread 0x7f6bf67f9700 (LWP 9740)]
0x7f6bfacb2b55 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x7f6bfacb2b55 in raise () from /lib64/libc.so.6
#1  0x7f6bfacb4131 in abort () from /lib64/libc.so.6
#2  0x7f6bfd51047c in kvm_io_ioeventfd_del (listener=
0x7f6bfd9ffee0 , section=0x7f6bf67f87c0, match_data=true, 
data=
0, e=0x7f697930) at /tmp/qemu/kvm-all.c:879
#3  0x7f6bfd5163b5 in address_space_add_del_ioeventfds (as=
0x7f6bfde3d6e0 , fds_new=0x0, fds_new_nb=0, fds_old=
0x7f6bfdfd8ce0, fds_old_nb=1) at /tmp/qemu/memory.c:628
#4  0x7f6bfd51698e in address_space_update_ioeventfds (as=
0x7f6bfde3d6e0 ) at /tmp/qemu/memory.c:687
#5  0x7f6bfd517949 in address_space_update_topology (as=
0x7f6bfde3d6e0 ) at /tmp/qemu/memory.c:780
#6  0x7f6bfd517a68 in memory_region_transaction_commit ()
at /tmp/qemu/memory.c:800
#7  0x7f6bfd3e2942 in virtio_pci_stop_ioeventfd (proxy=0x7f6bfdfde080)
at hw/virtio/virtio-pci.c:270
#8  0x7f6bfd3e2aaa in virtio_ioport_write (opaque=0x7f6bfdfde080, addr=18, 
val=
0) at hw/virtio/virtio-pci.c:309
#9  0x7f6bfd3e2ff3 in virtio_pci_config_write (opaque=0x7f6bfdfde080, 
addr=18,
val=0, size=1) at hw/virtio/virtio-pci.c:436
#10 0x7f6bfd515368 in memory_region_write_accessor (mr=0x7f6bfdfde818, 
addr=18,
value=0x7f6bf67f8b68, size=1, shift=0, mask=255) at /tmp/qemu/memory.c:441
#11 0x7f6bfd5154a4 in access_with_adjusted_size (addr=18, 
value=0x7f6bf67f8b68,
size=1, access_size_min=1, access_size_max=4, access=
0x7f6bfd5152df , mr=0x7f6bfdfde818)
at /tmp/qemu/memory.c:478
#12 0x7f6bfd518305 in memory_region_dispatch_write (mr=0x7f6bfdfde818, 
addr=18,
data=0, size=1) at /tmp/qemu/memory.c:985
#13 0x7f6bfd51bb1b in io_mem_write (mr=0x7f6bfdfde818, addr=18, val=0, 
size=1)
at /tmp/qemu/memory.c:1744
#14 0x7f6bfd4a0c67 in address_space_rw (as=0x7f6bfde3d6e0 
,
addr=49170, buf=0x7f6bfd189000 "", len=1, is_write=true)
at /tmp/qemu/exec.c:2034
#15 0x7f6bfd511e06 in kvm_handle_io (port=49170, data=0x7f6bfd189000, 
direction=
1, size=1, count=1) at /tmp/qemu/kvm-all.c:1558
#16 0x7f6bfd5123aa in kvm_cpu_exec (cpu=0x7f6bfdf54d50)
at /tmp/qemu/kvm-all.c:1695
#17 0x7f6bfd491db6 in qemu_kvm_cpu_thread_fn (arg=0x7f6bfdf54d50)
at /tmp/qemu/cpus.c:873
#18 0x7f6bfafff7f6 in start_thread () from /lib64/libpthread.so.0
#19 0x7f6bfad5b09d in clone () from /lib64/libc.so.6
#20 0x in ?? ()


the commandline is:
LC_ALL=C PATH=/bin:/sbin:/usr/bin:/usr/sbin HOME=/ QEMU_AUDIO_DRV=none 
/tmp/qemu/x86_
64-softmmu/qemu-system-x86_64 -name cl_suse -S -machine 
pc-i440fx-1.5,accel=kvm,usb=o
ff -m 10240 -realtime mlock=off -smp 4,sockets=4,cores=1,threads=1 -uuid 
5a09315c-d31
4-49a5-aa51-2168a71bf82d -no-user-config -nodefaults -chardev 
socket,id=charmonitor,p
ath=/var/lib/libvirt/qemu/cl_suse.monitor,server,nowait -mon 
chardev=charmonitor,id=m
onitor,mode=control -rtc base=utc -no-hpet -no-shutdown -device 
piix3-usb-uhci,id=usb
,bus=pci.0,addr=0x1.0x2 -drive 
file=/mnt/sdb/cl/cl_sles11sp3.img,if=none,id=drive-vir
tio-disk0,format=raw,cache=none,aio=native -device 
virtio-blk-pci,scsi=off,bus=pci.0,
addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -chardev 
pty,id=charser
ial0 -device isa-serial,chardev=charserial0,id=serial0 -device 
usb-tablet,id=input0 -
vnc 0.0.0.0:0 -device cirrus-vga,id=video0,vgamem_mb=9,bus=pci.0,addr=0x2 
-device vir
tio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6

Re: [Qemu-devel] [PATCH 1/4] kvm: Add set_one_reg/get_one_reg helpers

[Alexey Kardashevskiy]

On 05/08/2014 10:27 PM, Alexander Graf wrote:
> On 04/03/2014 03:14 PM, Alexey Kardashevskiy wrote:
>> This adds QEMU wrappers for KVM_SET_ONE_REG/KVM_GET_ONE_REG ioctls.
>>
>> Signed-off-by: Alexey Kardashevskiy 
> 
> Please merge with the s390 variant of this.


How? Why?

I posted mine 19 september 2013. What you refer to is from April this year.
How is s390 patch better than mine? Did I "ping" not often enough? Not funny :(

Do you want me to move kvm_set_one_reg/kvm_get_one_reg from
target-s390x/kvm.c to kvm-all.c?


-- 
Alexey

Re: [Qemu-devel] QEMU 2.1 release schedule?

[Michael Roth]

Quoting Peter Maydell (2014-05-08 08:48:01)
> On 28 April 2014 16:38, Peter Maydell  wrote:
> > Thanks to everybody who helped with getting QEMU 2.0 released.
> > The traditional reward for a job well done is another job, which
> > means we should probably work out what the release schedule for
> > 2.1 is going to be.
> >
> > We started 2.1's development phase on 17th April, which means that
> > for a standard 3 month release we're looking at mid to late July
> > release. (Of course if we stick to 3 month releases then the
> > slippage on 2.0 means we'll only have 3 releases this year, not
> > 4; do we care?)
> >
> > I felt we were a bit too aggressive in the schedule this
> > time around, and didn't really leave enough hard-freeze time
> > for producing, testing and stabilizing the release candidates.
> > I think we should probably aim for something more like what
> > 2.0 actually ended up with, rather than our proposed dates
> > (see http://wiki.qemu.org/Planning/2.0).
> 
> Paolo suggested on IRC:
> 
> > soft freeze mid june, hard freeze beginning of july,
> > release end of july or beginning of august?
> 
> Which works for me. Some concrete dates:
>  * 17 June: softfreeze and rc0
>[ 2 week softfreeze period ]
>  * 1 July: hardfreeze
>[ 4 week hardfreeze period, aiming for
>  rc1, rc2, rc3 at about weekly intervals ]
>  * 29 July: release
> 
> Michael: does that schedule work for you, given you'd
> be doing the tarball-rolling parts?

Looks good, don't see any issues with the proposed schedule on my end.

> 
> thanks
> -- PMM

Re: [Qemu-devel] virtio-serial-pci very expensive during live migration

[Chris Friesen]

On 05/08/2014 09:40 AM, Chris Friesen wrote:

On 05/08/2014 07:47 AM, Amit Shah wrote:


Chris, I just tried a simple test this way:

./x86_64-softmmu/qemu-system-x86_64 -device virtio-serial-pci -device
virtserialport -S -monitor stdio -nographic

and it didn't crash for me.  This was with qemu.git.  Perhaps you can
try in a similar way.


I just tried it with the "stable-1.4" branch from upstream with my first
patch added on.



Anyway, it seems to boot up okay, which is better than what I was
getting before.



Turns out I spoke too soon.  With the patch applied, it boots, but if I 
try to do a live migration both the source and destination crash.  This 
happens for both the master branch as well as the stable-1.4 branch.


If I back out the patch, it works fine.  If I leave the patch in and 
disable kvm acceleration it works fine.



I'm running the source as

/tmp/qemu-system-x86_64-upstream  -machine accel=kvm -m 1000 
test-cgcs-guest.img -device virtio-serial -chardev 
socket,path=/tmp/foo,server,nowait,id=foo -device 
virtserialport,chardev=foo,name=myfoo -monitor stdio



and the dest as

/tmp/qemu-system-x86_64-upstream  -machine accel=kvm -m 1000 
test-cgcs-guest.img -device virtio-serial -chardev 
socket,path=/tmp/foo,server,nowait,id=foo -device 
virtserialport,chardev=foo,name=myfoo -monitor stdio -incoming tcp:0:



and I'm triggering the migration with

migrate -d tcp:localhost:



On the dest side monitor I get:

(qemu) qemu: warning: error while loading state section id 3
load of migration failed



I managed to get gdb working, but it's not very helpful.  With gdb 
attached to the destination process I just get:


[Thread 0x7f760f6dc700 (LWP 15328) exited]
[Inferior 1 (process 15326) exited normally]

Re: [Qemu-devel] [PATCH v3 18/25] rbd: use BlockDriverState's AioContext

[Josh Durgin]

On 05/08/2014 07:34 AM, Stefan Hajnoczi wrote:

Drop the assumption that we're using the main AioContext.  Convert
qemu_bh_new() to aio_bh_new() and qemu_aio_wait() to aio_poll().

The .bdrv_detach_aio_context() and .bdrv_attach_aio_context() interfaces
are not needed since no fd handlers, timers, or BHs stay registered when
requests have been drained.

Cc: Josh Durgin 
Signed-off-by: Stefan Hajnoczi 
---
  block/rbd.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index dbc79f4..41f7bdc 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -548,7 +548,7 @@ static void qemu_rbd_aio_cancel(BlockDriverAIOCB *blockacb)
  acb->cancelled = 1;

  while (acb->status == -EINPROGRESS) {
-qemu_aio_wait();
+aio_poll(bdrv_get_aio_context(acb->common.bs), true);
  }

  qemu_aio_release(acb);
@@ -581,7 +581,8 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB 
*rcb)
  rcb->ret = rbd_aio_get_return_value(c);
  rbd_aio_release(c);

-acb->bh = qemu_bh_new(rbd_finish_bh, rcb);
+acb->bh = aio_bh_new(bdrv_get_aio_context(acb->common.bs),
+ rbd_finish_bh, rcb);
  qemu_bh_schedule(acb->bh);
  }


Assuming bdrv_get_aio_context() continues to be safe to call from a
non-qemu thread:

Reviewed-by: Josh Durgin 

[Qemu-devel] [RFC PATCH] migration: reintroduce skipped zero pages

[Peter Lieven]

commit f1c72795a introduced skipping of all zero pages during
bulk phase of ram migration. In theory this should have worked,
however the underlying assumption that the memory of target VM
is totally empty (zeroed) was wrong. Altough qemu accepts an incoming
migration BIOS, ROMs or tables are set up. If e.g. a ROM differs
between source and target we get memory corruption if a page
is zero at the source and not at the target. Therefore the
original patch was reverted later on.

This patch now reintroduces the feature to skip zero pages.
However, this time it has to be explicitely turned on through
a migration capability which should only be enabled if both
source and destination support it.

The feature especially makes sense if you expect a significant portion
of zero pages while bandwidth or disk space is limited.
Because even if a zero page is compressed we still transfer 9 bytes for
each page.

Signed-off-by: Peter Lieven 
---
 arch_init.c   |   44 +
 include/migration/migration.h |2 +-
 migration.c   |9 +
 qapi-schema.json  |   11 ---
 4 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 995f56d..2579302 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -123,7 +123,8 @@ static uint64_t bitmap_sync_count;
 #define RAM_SAVE_FLAG_EOS  0x10
 #define RAM_SAVE_FLAG_CONTINUE 0x20
 #define RAM_SAVE_FLAG_XBZRLE   0x40
-/* 0x80 is reserved in migration.h start with 0x100 next */
+/* 0x80 is reserved in migration.h */
+#define RAM_SAVE_FLAG_ZERO_TARGET 0x100
 
 static struct defconfig_file {
 const char *filename;
@@ -575,8 +576,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
 MemoryRegion *mr;
 ram_addr_t current_addr;
 
-if (!block)
+if (!block) {
 block = QTAILQ_FIRST(&ram_list.blocks);
+}
 
 while (true) {
 mr = block->mr;
@@ -619,11 +621,16 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
 }
 }
 } else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
-acct_info.dup_pages++;
-bytes_sent = save_block_hdr(f, block, offset, cont,
-RAM_SAVE_FLAG_COMPRESS);
-qemu_put_byte(f, 0);
-bytes_sent++;
+if (!ram_bulk_stage || !migrate_skip_zero_pages()) {
+acct_info.dup_pages++;
+bytes_sent = save_block_hdr(f, block, offset, cont,
+RAM_SAVE_FLAG_COMPRESS);
+qemu_put_byte(f, 0);
+bytes_sent++;
+} else {
+acct_info.skipped_pages++;
+bytes_sent = 0;
+}
 /* Must let xbzrle know, otherwise a previous (now 0'd) cached
  * page would be stale
  */
@@ -752,6 +759,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 {
 RAMBlock *block;
 int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps */
+uint64_t flags = 0;
 
 mig_throttle_on = false;
 dirty_rate_high_cnt = 0;
@@ -812,7 +820,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 migration_bitmap_sync();
 qemu_mutex_unlock_iothread();
 
-qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
+if (migrate_skip_zero_pages()) {
+flags |= RAM_SAVE_FLAG_ZERO_TARGET;
+}
+
+qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE | flags);
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
 qemu_put_byte(f, strlen(block->idstr));
@@ -1082,6 +1094,22 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 goto done;
 }
 
+/* ensure that the target memory is really zero initialized
+ * so we can skip zero pages in the bulk phase.
+ * TODO: Ideally this should not be needed since we mmap the
+ * target memory, however machine individual code may still
+ * load BIOS, ROMs etc. altough we await an incoming migration.
+ * (see commit 9ef051e5) */
+if (flags & RAM_SAVE_FLAG_ZERO_TARGET) {
+ram_addr_t offset = 0;
+void *base = memory_region_get_ram_ptr(block->mr);
+for (; offset < block->length; offset += TARGET_PAGE_SIZE) 
{
+if (!is_zero_range(base + offset, TARGET_PAGE_SIZE)) {
+memset(base + offset, 0x00, TARGET_PAGE_SIZE);
+}
+}
+}
+
 total_ram_bytes -= length;
 }
 }
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 3cb5ba8..4320f6a 100644
--- a/include/migration/migration.h
+

[Qemu-devel] Question about RAM migration flags

[Peter Lieven]

Hi,

while working on ram migration and reading through the code I realized that
qemu does not stop loading a saved VM or rejecting an incoming migration
if there is a flag in the stream that it does not understand. An unknown flag
is simply ignored.

In the block migration code there is a catch at the end complaining about
unknown flags, but in RAM migration there isn't.

Is this on purpose or an error?

Peter

Re: [Qemu-devel] [SeaBIOS] seabios release?

[Kevin O'Connor]

On Wed, May 07, 2014 at 12:33:35PM +0200, Gerd Hoffmann wrote:
> On Mi, 2014-04-23 at 16:41 +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > I think it's time to start planning the next seabios release.  First,
> > because a bunch of changes have piled up in master.  And second, because
> > of the smbios changes.  They are a step forward in making seabios less
> > dependent on qemu internals and I want have that in qemu soonish.
> > 
> > So, how about this plan:
> > 
> >   (1) merge smbios patches in qemu (patchset is close to final now).
> >   (2) merge smbios patch in seabios
> 
> We are here now ...
> 
> >   (3) start freeze
> >   (4) tag release candidate (early may is realistic for that I think).
> 
> ... so it's time kick freeze + release candidate I guess ;)
> 
> Are there any pending patches for the next release which should better
> make it into the release candidate?

I just sent a few fixes to the mailing list (xhci msleep, romlayout.o
rebuild, int 1589 fix).  I think these should go into the final
release.  Unless there are comments on the above patches I'll push
them on Monday.

I think we can then also start the feature freeze on Monday.

-Kevin

[Qemu-devel] [PULL 04/38] qapi: Add a primitive to include other files from a QAPI schema file

[Luiz Capitulino]

From: Lluís Vilanova 

The primitive uses JSON syntax, and include paths are relative to the file 
using the directive:

  { 'include': 'path/to/file.json' }

Signed-off-by: Lluís Vilanova 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 docs/qapi-code-gen.txt | 11 +
 scripts/qapi.py| 64 --
 tests/Makefile |  5 ++-
 tests/qapi-schema/include-before-err.err   |  1 +
 tests/qapi-schema/include-before-err.exit  |  1 +
 tests/qapi-schema/include-before-err.json  |  2 +
 tests/qapi-schema/include-before-err.out   |  0
 tests/qapi-schema/include-cycle-b.json |  1 +
 tests/qapi-schema/include-cycle-c.json |  1 +
 tests/qapi-schema/include-cycle.err|  3 ++
 tests/qapi-schema/include-cycle.exit   |  1 +
 tests/qapi-schema/include-cycle.json   |  1 +
 tests/qapi-schema/include-cycle.out|  0
 tests/qapi-schema/include-format-err.err   |  1 +
 tests/qapi-schema/include-format-err.exit  |  1 +
 tests/qapi-schema/include-format-err.json  |  2 +
 tests/qapi-schema/include-format-err.out   |  0
 tests/qapi-schema/include-nested-err.err   |  2 +
 tests/qapi-schema/include-nested-err.exit  |  1 +
 tests/qapi-schema/include-nested-err.json  |  1 +
 tests/qapi-schema/include-nested-err.out   |  0
 tests/qapi-schema/include-no-file.err  |  1 +
 tests/qapi-schema/include-no-file.exit |  1 +
 tests/qapi-schema/include-no-file.json |  1 +
 tests/qapi-schema/include-no-file.out  |  0
 tests/qapi-schema/include-non-file.err |  1 +
 tests/qapi-schema/include-non-file.exit|  1 +
 tests/qapi-schema/include-non-file.json|  1 +
 tests/qapi-schema/include-non-file.out |  0
 tests/qapi-schema/include-relpath-sub.json |  2 +
 tests/qapi-schema/include-relpath.err  |  0
 tests/qapi-schema/include-relpath.exit |  1 +
 tests/qapi-schema/include-relpath.json |  1 +
 tests/qapi-schema/include-relpath.out  |  3 ++
 tests/qapi-schema/include-self-cycle.err   |  1 +
 tests/qapi-schema/include-self-cycle.exit  |  1 +
 tests/qapi-schema/include-self-cycle.json  |  1 +
 tests/qapi-schema/include-self-cycle.out   |  0
 tests/qapi-schema/include-simple-sub.json  |  2 +
 tests/qapi-schema/include-simple.err   |  0
 tests/qapi-schema/include-simple.exit  |  1 +
 tests/qapi-schema/include-simple.json  |  1 +
 tests/qapi-schema/include-simple.out   |  3 ++
 tests/qapi-schema/include/relpath.json |  1 +
 44 files changed, 110 insertions(+), 13 deletions(-)
 create mode 100644 tests/qapi-schema/include-before-err.err
 create mode 100644 tests/qapi-schema/include-before-err.exit
 create mode 100644 tests/qapi-schema/include-before-err.json
 create mode 100644 tests/qapi-schema/include-before-err.out
 create mode 100644 tests/qapi-schema/include-cycle-b.json
 create mode 100644 tests/qapi-schema/include-cycle-c.json
 create mode 100644 tests/qapi-schema/include-cycle.err
 create mode 100644 tests/qapi-schema/include-cycle.exit
 create mode 100644 tests/qapi-schema/include-cycle.json
 create mode 100644 tests/qapi-schema/include-cycle.out
 create mode 100644 tests/qapi-schema/include-format-err.err
 create mode 100644 tests/qapi-schema/include-format-err.exit
 create mode 100644 tests/qapi-schema/include-format-err.json
 create mode 100644 tests/qapi-schema/include-format-err.out
 create mode 100644 tests/qapi-schema/include-nested-err.err
 create mode 100644 tests/qapi-schema/include-nested-err.exit
 create mode 100644 tests/qapi-schema/include-nested-err.json
 create mode 100644 tests/qapi-schema/include-nested-err.out
 create mode 100644 tests/qapi-schema/include-no-file.err
 create mode 100644 tests/qapi-schema/include-no-file.exit
 create mode 100644 tests/qapi-schema/include-no-file.json
 create mode 100644 tests/qapi-schema/include-no-file.out
 create mode 100644 tests/qapi-schema/include-non-file.err
 create mode 100644 tests/qapi-schema/include-non-file.exit
 create mode 100644 tests/qapi-schema/include-non-file.json
 create mode 100644 tests/qapi-schema/include-non-file.out
 create mode 100644 tests/qapi-schema/include-relpath-sub.json
 create mode 100644 tests/qapi-schema/include-relpath.err
 create mode 100644 tests/qapi-schema/include-relpath.exit
 create mode 100644 tests/qapi-schema/include-relpath.json
 create mode 100644 tests/qapi-schema/include-relpath.out
 create mode 100644 tests/qapi-schema/include-self-cycle.err
 create mode 100644 tests/qapi-schema/include-self-cycle.exit
 create mode 100644 tests/qapi-schema/include-self-cycle.json
 create mode 100644 tests/qapi-schema/include-self-cycle.out
 create mode 100644 tests/qapi-schema/include-simple-sub.json
 create mode 100644 tests/qapi-schema/include-simple.err
 create mode 100644 tests/qapi-schema/include-simple.exit
 create mode 100644 tests/qapi-schema/include-simple.json
 create mode 100644 tests/qapi-schema/include-simple.out
 create m

[Qemu-devel] [PULL 02/38] qapi: [trivial] Do not catch unknown exceptions in "test-qapi.py"

[Luiz Capitulino]

From: Lluís Vilanova 

Signed-off-by: Lluís Vilanova 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 tests/qapi-schema/test-qapi.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index b3d1e1d..5a26ef3 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -18,9 +18,6 @@ try:
 exprs = parse_schema(sys.stdin)
 except SystemExit:
 raise
-except:
-print >>sys.stderr, "Crashed:", sys.exc_info()[0]
-exit(1)
 
 pprint(exprs)
 pprint(enum_types)
-- 
1.9.0

[Qemu-devel] [PATCH 1/8] hw/intc/allwinner-a10-pic: Add missing 'break'

[Peter Maydell]

Add missing 'break' after handling of AW_A10_PIC_BASE_ADDR write.

Signed-off-by: Peter Maydell 
---
 hw/intc/allwinner-a10-pic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/intc/allwinner-a10-pic.c b/hw/intc/allwinner-a10-pic.c
index 0924d98..7b3e085 100644
--- a/hw/intc/allwinner-a10-pic.c
+++ b/hw/intc/allwinner-a10-pic.c
@@ -97,6 +97,7 @@ static void aw_a10_pic_write(void *opaque, hwaddr offset, 
uint64_t value,
 switch (offset) {
 case AW_A10_PIC_BASE_ADDR:
 s->base_addr = value & ~0x3;
+break;
 case AW_A10_PIC_PROTECT:
 s->protect = value;
 break;
-- 
1.9.2

Re: [Qemu-devel] virtio-serial-pci very expensive during live migration

[Chris Friesen]

On 05/08/2014 10:02 AM, Paolo Bonzini wrote:

Il 08/05/2014 17:57, Chris Friesen ha scritto:


The fact remains that qemu crashes when I apply the patch.  I also
tried
patching it as below in virtio_pci_vmstate_change().  That would allow
the VM to boot, but it would crash when I tried to do a live migration.


Can you give us your command line and a backtrace?


For the backtrace, you mean with a core file?  Or is there a better way?


You can attach gdb to the qemu process, and run "thread apply all bt"
when QEMU crashes.



For the commandline, it's coming from OpenStack via libvirt, so it's 
fairly complicated:


/usr/bin/kvm -c 0x0001 -n 4 
--proc-type=secondary --file-prefix=vs -- -enable-dpdk -name 
instance-0007 -S -machine pc-i440fx-1.4,accel=kvm,usb=off -m 512 
-mem-prealloc -mem-path /mnt/huge-2048kB/libvirt/qemu -smp 
2,sockets=2,cores=1,threads=1 -uuid b277d785-9c31-45be-91d3-15b7b1aca974 
-smbios type=1,manufacturer=OpenStack Foundation,product=OpenStack 
Nova,version=2013.2.2,serial=--3848-3342-4e5037313536,uuid=b277d785-9c31-45be-91d3-15b7b1aca974 
-no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/instance-0007.monitor,server,nowait 
-mon chardev=charmonitor,id=monitor,mode=control -rtc 
base=utc,driftfix=slew -no-kvm-pit-reinjection -no-shutdown -device 
piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device 
virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -drive 
file=/etc/nova/instances/b277d785-9c31-45be-91d3-15b7b1aca974/disk,if=none,id=drive-virtio-disk0,format=qcow2,cache=none 
-device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 
-chardev 
file,id=charserial0,path=/etc/nova/instances/b277d785-9c31-45be-91d3-15b7b1aca974/console.log 
-device isa-serial,chardev=charserial0,id=serial0 -chardev 
pty,id=charserial1 -device isa-serial,chardev=charserial1,id=serial1 
-chardev 
socket,id=charchannel0,path=/var/lib/libvirt/qemu/cgcs.heartbeat.instance-0007.sock,server,nowait 
-device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=cgcs.heartbeat 
-device usb-tablet,id=input0 -vnc 0.0.0.0:0 -k en-us -vga cirrus -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 -device 
avp,port=4a05cedd-3868-449a-9728-899bbea10863,id=avp4a05cedd-38





As for the backtrace, I can attach gdb to the qemu process, but when I 
hit "c" to continue it gives an error:



Reading symbols from /lib64/libnss_files.so.2...(no debugging symbols 
found)...done.

Loaded symbols for /lib64/libnss_files.so.2

warning: File "/lib64/libthread_db-1.0.so" auto-loading has been 
declined by your `auto-load safe-path' set to 
"$debugdir:$datadir/auto-load".


warning: Unable to find libthread_db matching inferior's thread library, 
thread debugging will not be available.

0x7f56c6366d23 in select () from /lib64/libc.so.6
(gdb) c
Continuing.
Warning:
Cannot insert breakpoint -1.
Error accessing memory address 0x2f7c50: Input/output error.


Chris

[Qemu-devel] [PULL 35/38] qmp: Don't use error_is_set() to suppress additional errors

[Luiz Capitulino]

From: Markus Armbruster 

Using error_is_set(errp) that way can sweep programming errors under
the carpet when we get called incorrectly with an error set.

encrypted_bdrv_it() does it, because there's no way to make
bdrv_iterate() break its loop.  Actually safe, because qmp_cont()
clears the error before the loop.  Clean it up anyway: replace
bdrv_iterate() by bdrv_next(), break the loop on error.

Replace both occurrences, for consistency.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 qmp.c | 32 +++-
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/qmp.c b/qmp.c
index 82acb89..a7f432b 100644
--- a/qmp.c
+++ b/qmp.c
@@ -146,24 +146,9 @@ SpiceInfo *qmp_query_spice(Error **errp)
 };
 #endif
 
-static void iostatus_bdrv_it(void *opaque, BlockDriverState *bs)
-{
-bdrv_iostatus_reset(bs);
-}
-
-static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
-{
-Error **errp = opaque;
-
-if (!error_is_set(errp) && bdrv_key_required(bs)) {
-error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
-  bdrv_get_encrypted_filename(bs));
-}
-}
-
 void qmp_cont(Error **errp)
 {
-Error *local_err = NULL;
+BlockDriverState *bs;
 
 if (runstate_needs_reset()) {
 error_setg(errp, "Resetting the Virtual Machine is required");
@@ -172,11 +157,16 @@ void qmp_cont(Error **errp)
 return;
 }
 
-bdrv_iterate(iostatus_bdrv_it, NULL);
-bdrv_iterate(encrypted_bdrv_it, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
+for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+bdrv_iostatus_reset(bs);
+}
+for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
+if (bdrv_key_required(bs)) {
+error_set(errp, QERR_DEVICE_ENCRYPTED,
+  bdrv_get_device_name(bs),
+  bdrv_get_encrypted_filename(bs));
+return;
+}
 }
 
 if (runstate_check(RUN_STATE_INMIGRATE)) {
-- 
1.9.0

[Qemu-devel] [PULL 08/38] pci-assign: accept Error from monitor_handle_fd_param2()

[Luiz Capitulino]

From: Laszlo Ersek 

Propagate any errors in monitor fd handling up to get_real_device(), and
report them there. We'll continue the propagation upwards when
get_real_device() becomes a leaf itself (when none of its callees will
report errors internally any longer when detecting and returning an
error).

Signed-off-by: Laszlo Ersek 
eviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index a825871..bfce97f 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -541,6 +541,7 @@ static int get_real_device(AssignedDevice *pci_dev)
 uint16_t id;
 PCIRegion *rp;
 PCIDevRegions *dev = &pci_dev->real_device;
+Error *local_err = NULL;
 
 dev->region_number = 0;
 
@@ -551,8 +552,12 @@ static int get_real_device(AssignedDevice *pci_dev)
 snprintf(name, sizeof(name), "%sconfig", dir);
 
 if (pci_dev->configfd_name && *pci_dev->configfd_name) {
-dev->config_fd = monitor_handle_fd_param(cur_mon, 
pci_dev->configfd_name);
-if (dev->config_fd < 0) {
+dev->config_fd = monitor_handle_fd_param2(cur_mon,
+  pci_dev->configfd_name,
+  &local_err);
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
 return 1;
 }
 } else {
-- 
1.9.0

[Qemu-devel] [PATCH v7 00/14] qemu-img: Implement commit like QMP

[Max Reitz]

qemu-img should use QMP commands whenever possible in order to ensure
feature completeness of both online and offline image operations. For
the "commit" command, this is relatively easy, so implement it first
(in the hope that indeed others will follow).

As qemu-img does not have access to QMP (due to QMP being intertwined
with basically everything in qemu), we cannot directly use QMP, but at
least use the functions the corresponding QMP commands are using (which
would be "block-commit", in this case).


This series (as of v7) depends on the "[PATCH v3 00/25] dataplane: use
QEMU block layer" series by Stefan.


v7:
- Patch 1: rephrased comment [Eric]
- Patch 3:
  - give BDRV_REQ_MAY_UNMAP to bdrv_write_zeroes() [Eric]
  - adjusted comment about calculation of total cluster count [Eric]
  - added error handling for bdrv_discard() [Eric]
- Patch 7: replaced qemu_aio_wait() by aio_poll()
  [Stefan's dataplane series]
- Patch 8: rephrased comment and manpage content [Eric]
- Patch 12: more test passes for implicit and explicit -d


git-backport-diff output against v6:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/14:[0004] [FC] 'qcow2: Allow "full" discard'
002/14:[] [--] 'qcow2: Implement bdrv_make_empty()'
003/14:[0014] [FC] 'qcow2: Optimize bdrv_make_empty()'
004/14:[] [--] 'blockjob: Introduce block_job_complete_sync()'
005/14:[] [--] 'blockjob: Add "ready" field'
006/14:[] [--] 'block/mirror: Improve progress report'
007/14:[0008] [FC] 'qemu-img: Implement commit like QMP'
008/14:[0012] [FC] 'qemu-img: Empty image after commit'
009/14:[] [-C] 'qemu-img: Enable progress output for commit'
010/14:[] [-C] 'qemu-img: Specify backing file for commit'
011/14:[] [--] 'iotests: Add _filter_qemu_img_map'
012/14:[0094] [FC] 'iotests: Add test for backing-chain commits'
013/14:[] [--] 'iotests: Add test for qcow2's bdrv_make_empty'
014/14:[] [--] 'iotests: Omit length/offset test in 040 and 041'


Max Reitz (14):
  qcow2: Allow "full" discard
  qcow2: Implement bdrv_make_empty()
  qcow2: Optimize bdrv_make_empty()
  blockjob: Introduce block_job_complete_sync()
  blockjob: Add "ready" field
  block/mirror: Improve progress report
  qemu-img: Implement commit like QMP
  qemu-img: Empty image after commit
  qemu-img: Enable progress output for commit
  qemu-img: Specify backing file for commit
  iotests: Add _filter_qemu_img_map
  iotests: Add test for backing-chain commits
  iotests: Add test for qcow2's bdrv_make_empty
  iotests: Omit length/offset test in 040 and 041

 block/Makefile.objs  |   2 +-
 block/mirror.c   |  32 ++--
 block/qcow2-cluster.c|  27 ++-
 block/qcow2-snapshot.c   |   2 +-
 block/qcow2.c| 388 ++-
 block/qcow2.h|   2 +-
 blockjob.c   |  46 -
 include/block/blockjob.h |  20 ++
 qapi-schema.json |   4 +-
 qemu-img-cmds.hx |   4 +-
 qemu-img.c   | 149 ---
 qemu-img.texi|  13 +-
 tests/qemu-iotests/040   |   4 +-
 tests/qemu-iotests/041   |   3 +-
 tests/qemu-iotests/092   | 122 
 tests/qemu-iotests/092.out   | 119 
 tests/qemu-iotests/093   |  72 
 tests/qemu-iotests/093.out   |  26 +++
 tests/qemu-iotests/common.filter |   7 +
 tests/qemu-iotests/group |   2 +
 tests/qemu-iotests/iotests.py|   3 +-
 21 files changed, 980 insertions(+), 67 deletions(-)
 create mode 100755 tests/qemu-iotests/092
 create mode 100644 tests/qemu-iotests/092.out
 create mode 100755 tests/qemu-iotests/093
 create mode 100644 tests/qemu-iotests/093.out

-- 
1.9.2

[Qemu-devel] [PATCH v7 02/14] qcow2: Implement bdrv_make_empty()

[Max Reitz]

Implement this function by making all clusters in the image file fall
through to the backing file (by using the recently extended discard).

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block/qcow2.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 68f70ea..fb427d1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2008,6 +2008,32 @@ fail:
 return ret;
 }
 
+static int qcow2_make_empty(BlockDriverState *bs)
+{
+int ret = 0;
+uint64_t start_sector;
+int sector_step = INT_MAX / BDRV_SECTOR_SIZE;
+
+for (start_sector = 0; start_sector < bs->total_sectors;
+ start_sector += sector_step)
+{
+/* As this function is generally used after committing an external
+ * snapshot, QCOW2_DISCARD_SNAPSHOT seems appropriate. Also, the
+ * default action for this kind of discard is to pass the discard,
+ * which will ideally result in an actually smaller image file, as
+ * is probably desired. */
+ret = qcow2_discard_clusters(bs, start_sector * BDRV_SECTOR_SIZE,
+ MIN(sector_step,
+ bs->total_sectors - start_sector),
+ QCOW2_DISCARD_SNAPSHOT, true);
+if (ret < 0) {
+break;
+}
+}
+
+return ret;
+}
+
 static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
 {
 BDRVQcowState *s = bs->opaque;
@@ -2390,6 +2416,7 @@ static BlockDriver bdrv_qcow2 = {
 .bdrv_co_discard= qcow2_co_discard,
 .bdrv_truncate  = qcow2_truncate,
 .bdrv_write_compressed  = qcow2_write_compressed,
+.bdrv_make_empty= qcow2_make_empty,
 
 .bdrv_snapshot_create   = qcow2_snapshot_create,
 .bdrv_snapshot_goto = qcow2_snapshot_goto,
-- 
1.9.2

[Qemu-devel] [PATCH 7/8] hw/arm/stellaris: Correct handling of GPTM TAR register

[Peter Maydell]

We don't implement very much of the GPTM TAR register, and what we
do is wrong. The "are we in RT mode?" field is in s->config, not
s->control. Correct this, use LOG_UNIMP rather than hw_error()
for the cases we don't support, and avoid an unlabelled fallthrough
that makes Coverity complain.

Signed-off-by: Peter Maydell 
---
 hw/arm/stellaris.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index d6cc77b..487ee72 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -185,12 +185,19 @@ static uint64_t gptm_read(void *opaque, hwaddr offset,
 case 0x44: /* TBPMR */
 return s->match_prescale[1];
 case 0x48: /* TAR */
-if (s->control == 1)
+if (s->config == 1) {
 return s->rtc;
+}
+qemu_log_mask(LOG_UNIMP,
+  "gptm_read of TAR but timer read not supported");
+return 0;
 case 0x4c: /* TBR */
-hw_error("TODO: Timer value read\n");
+qemu_log_mask(LOG_UNIMP,
+  "gptm_read of TBR but timer read not supported");
+return 0;
 default:
-hw_error("gptm_read: Bad offset 0x%x\n", (int)offset);
+qemu_log_mask(LOG_GUEST_ERROR,
+  "gptm_read: Bad offset 0x%x\n", (int)offset);
 return 0;
 }
 }
-- 
1.9.2

[Qemu-devel] [PATCH v7 06/14] block/mirror: Improve progress report

[Max Reitz]

Instead of taking the total length of the block device as the block
job's length, use the number of dirty sectors. The progress is now the
number of sectors mirrored to the target block device. Note that this
may result in the job's length increasing during operation, which is
however in fact desirable.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block/mirror.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 1c38aa8..5d9cc89 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -39,6 +39,7 @@ typedef struct MirrorBlockJob {
 int64_t sector_num;
 int64_t granularity;
 size_t buf_size;
+int64_t bdev_length;
 unsigned long *cow_bitmap;
 BdrvDirtyBitmap *dirty_bitmap;
 HBitmapIter hbi;
@@ -48,6 +49,7 @@ typedef struct MirrorBlockJob {
 
 unsigned long *in_flight_bitmap;
 int in_flight;
+int sectors_in_flight;
 int ret;
 } MirrorBlockJob;
 
@@ -81,6 +83,7 @@ static void mirror_iteration_done(MirrorOp *op, int ret)
 trace_mirror_iteration_done(s, op->sector_num, op->nb_sectors, ret);
 
 s->in_flight--;
+s->sectors_in_flight -= op->nb_sectors;
 iov = op->qiov.iov;
 for (i = 0; i < op->qiov.niov; i++) {
 MirrorBuffer *buf = (MirrorBuffer *) iov[i].iov_base;
@@ -92,8 +95,11 @@ static void mirror_iteration_done(MirrorOp *op, int ret)
 chunk_num = op->sector_num / sectors_per_chunk;
 nb_chunks = op->nb_sectors / sectors_per_chunk;
 bitmap_clear(s->in_flight_bitmap, chunk_num, nb_chunks);
-if (s->cow_bitmap && ret >= 0) {
-bitmap_set(s->cow_bitmap, chunk_num, nb_chunks);
+if (ret >= 0) {
+if (s->cow_bitmap) {
+bitmap_set(s->cow_bitmap, chunk_num, nb_chunks);
+}
+s->common.offset += (uint64_t)op->nb_sectors * BDRV_SECTOR_SIZE;
 }
 
 qemu_iovec_destroy(&op->qiov);
@@ -166,7 +172,7 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 hbitmap_next_sector = s->sector_num;
 sector_num = s->sector_num;
 sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
-end = s->common.len >> BDRV_SECTOR_BITS;
+end = s->bdev_length / BDRV_SECTOR_SIZE;
 
 /* Extend the QEMUIOVector to include all adjacent blocks that will
  * be copied in this operation.
@@ -278,6 +284,7 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 
 /* Copy the dirty cluster.  */
 s->in_flight++;
+s->sectors_in_flight += nb_sectors;
 trace_mirror_one_iteration(s, sector_num, nb_sectors);
 bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors,
mirror_read_complete, op);
@@ -323,13 +330,13 @@ static void coroutine_fn mirror_run(void *opaque)
 goto immediate_exit;
 }
 
-s->common.len = bdrv_getlength(bs);
-if (s->common.len <= 0) {
-ret = s->common.len;
+s->bdev_length = bdrv_getlength(bs);
+if (s->bdev_length <= 0) {
+ret = s->bdev_length;
 goto immediate_exit;
 }
 
-length = DIV_ROUND_UP(s->common.len, s->granularity);
+length = DIV_ROUND_UP(s->bdev_length, s->granularity);
 s->in_flight_bitmap = bitmap_new(length);
 
 /* If we have no backing file yet in the destination, we cannot let
@@ -349,7 +356,7 @@ static void coroutine_fn mirror_run(void *opaque)
 }
 }
 
-end = s->common.len >> BDRV_SECTOR_BITS;
+end = s->bdev_length / BDRV_SECTOR_SIZE;
 s->buf = qemu_blockalign(bs, s->buf_size);
 sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
 mirror_free_init(s);
@@ -389,6 +396,12 @@ static void coroutine_fn mirror_run(void *opaque)
 }
 
 cnt = bdrv_get_dirty_count(bs, s->dirty_bitmap);
+/* s->common.offset contains the number of bytes already processed so
+ * far, cnt is the number of dirty sectors remaining and
+ * s->sectors_in_flight is the number of sectors currently being
+ * processed; together those are the current total operation length */
+s->common.len = s->common.offset +
+(cnt + s->sectors_in_flight) * BDRV_SECTOR_SIZE;
 
 /* Note that even when no rate limit is applied we need to yield
  * periodically with no pending I/O so that qemu_aio_flush() returns.
@@ -424,7 +437,6 @@ static void coroutine_fn mirror_run(void *opaque)
  * report completion.  This way, block-job-cancel will leave
  * the target in a consistent state.
  */
-s->common.offset = end * BDRV_SECTOR_SIZE;
 if (!s->synced) {
 block_job_ready(&s->common);
 s->synced = true;
@@ -453,8 +465,6 @@ static void coroutine_fn mirror_run(void *opaque)
 ret = 0;
 trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
 if (!s->synced) {
-/* Publish progress */
-s->common.offset = (end -

[Qemu-devel] [PULL 11/38] pci-assign: propagate Error from check_irqchip_in_kernel()

[Luiz Capitulino]

From: Laszlo Ersek 

Rename check_irqchip_in_kernel() to verify_irqchip_in_kernel(), so that
the name reflects our expectation better. Rather than returning a bool,
make it do nothing or set an Error.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 997ef09..b4696aa 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -841,14 +841,12 @@ static int assign_device(AssignedDevice *dev)
 return r;
 }
 
-static bool check_irqchip_in_kernel(void)
+static void verify_irqchip_in_kernel(Error **errp)
 {
 if (kvm_irqchip_in_kernel()) {
-return true;
+return;
 }
-error_report("pci-assign: error: requires KVM with in-kernel irqchip "
- "enabled");
-return false;
+error_setg(errp, "pci-assign requires KVM with in-kernel irqchip enabled");
 }
 
 static int assign_intx(AssignedDevice *dev)
@@ -857,6 +855,7 @@ static int assign_intx(AssignedDevice *dev)
 PCIINTxRoute intx_route;
 bool intx_host_msi;
 int r;
+Error *local_err = NULL;
 
 /* Interrupt PIN 0 means don't use INTx */
 if (assigned_dev_pci_read_byte(&dev->dev, PCI_INTERRUPT_PIN) == 0) {
@@ -864,7 +863,10 @@ static int assign_intx(AssignedDevice *dev)
 return 0;
 }
 
-if (!check_irqchip_in_kernel()) {
+verify_irqchip_in_kernel(&local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return -ENOTSUP;
 }
 
@@ -1241,6 +1243,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
 PCIRegion *pci_region = dev->real_device.regions;
 int ret, pos;
+Error *local_err = NULL;
 
 /* Clear initial capabilities pointer and status copied from hw */
 pci_set_byte(pci_dev->config + PCI_CAPABILITY_LIST, 0);
@@ -1252,7 +1255,10 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
  * MSI capability is the 1st capability in capability config */
 pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
 if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
-if (!check_irqchip_in_kernel()) {
+verify_irqchip_in_kernel(&local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return -ENOTSUP;
 }
 dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
@@ -1281,7 +1287,10 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 int bar_nr;
 uint32_t msix_table_entry;
 
-if (!check_irqchip_in_kernel()) {
+verify_irqchip_in_kernel(&local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return -ENOTSUP;
 }
 dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
-- 
1.9.0

[Qemu-devel] [PATCH v7 09/14] qemu-img: Enable progress output for commit

[Max Reitz]

Implement progress output for the commit command by querying the
progress of the block job.

Signed-off-by: Max Reitz 
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.c   | 24 ++--
 qemu-img.texi|  2 +-
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index b31d81c..ea41d4f 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
 ETEXI
 
 DEF("commit", img_commit,
-"commit [-q] [-f fmt] [-t cache] [-d] filename")
+"commit [-q] [-f fmt] [-t cache] [-d] [-p] filename")
 STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-d] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-d] [-p] @var{filename}
 ETEXI
 
 DEF("compare", img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index d92ff27..c624d9c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -726,12 +726,18 @@ static void run_block_job(BlockJob *job, Error **errp)
 do {
 aio_poll(aio_context, true);
 
+qemu_progress_print((float)job->offset / job->len * 100.f, 0);
+
 if (!job->busy && !job->ready) {
 block_job_resume(job);
 }
 } while (!job->ready);
 
 block_job_complete_sync(job, errp);
+
+/* A block job may finish instantaneously without publishing any progress,
+ * so just signal completion here */
+qemu_progress_print(100.f, 0);
 }
 
 static int img_commit(int argc, char **argv)
@@ -739,14 +745,14 @@ static int img_commit(int argc, char **argv)
 int c, ret, flags;
 const char *filename, *fmt, *cache;
 BlockDriverState *bs, *base_bs;
-bool quiet = false, drop = false;
+bool progress = false, quiet = false, drop = false;
 Error *local_err = NULL;
 CommonBlockJobCBInfo cbi;
 
 fmt = NULL;
 cache = BDRV_DEFAULT_CACHE;
 for(;;) {
-c = getopt(argc, argv, "f:ht:dq");
+c = getopt(argc, argv, "f:ht:dpq");
 if (c == -1) {
 break;
 }
@@ -764,11 +770,20 @@ static int img_commit(int argc, char **argv)
 case 'd':
 drop = true;
 break;
+case 'p':
+progress = true;
+break;
 case 'q':
 quiet = true;
 break;
 }
 }
+
+/* Progress is not shown in Quiet mode */
+if (quiet) {
+progress = false;
+}
+
 if (optind != argc - 1) {
 error_exit("Expecting one image file name");
 }
@@ -786,6 +801,9 @@ static int img_commit(int argc, char **argv)
 return 1;
 }
 
+qemu_progress_init(progress, 1.f);
+qemu_progress_print(0.f, 100);
+
 /* This is different from QMP, which by default uses the deepest file in 
the
  * backing chain (i.e., the very base); however, the traditional behavior 
of
  * qemu-img commit is using the immediate backing file. */
@@ -834,6 +852,8 @@ unref_backing:
 }
 
 done:
+qemu_progress_end();
+
 bdrv_unref(bs);
 
 if (local_err) {
diff --git a/qemu-img.texi b/qemu-img.texi
index 69b2ffe..c60654f 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -140,7 +140,7 @@ this case. @var{backing_file} will never be modified unless 
you use the
 The size can also be specified using the @var{size} option with @code{-o},
 it doesn't need to be specified separately in this case.
 
-@item commit [-f @var{fmt}] [-t @var{cache}] [-d] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-d] [-p] @var{filename}
 
 Commit the changes recorded in @var{filename} in its base image or backing 
file.
 If the backing file is smaller than the snapshot, then the backing file will be
-- 
1.9.2

[Qemu-devel] [PATCH v7 04/14] blockjob: Introduce block_job_complete_sync()

[Max Reitz]

Implement block_job_complete_sync() by doing the exact same thing as
block_job_cancel_sync() does, only with calling block_job_complete()
instead of block_job_cancel().

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
---
 blockjob.c   | 39 ---
 include/block/blockjob.h | 15 +++
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index cd4784f..9c7b198 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -148,7 +148,7 @@ void block_job_iostatus_reset(BlockJob *job)
 }
 }
 
-struct BlockCancelData {
+struct BlockFinishData {
 BlockJob *job;
 BlockDriverCompletionFunc *cb;
 void *opaque;
@@ -156,19 +156,22 @@ struct BlockCancelData {
 int ret;
 };
 
-static void block_job_cancel_cb(void *opaque, int ret)
+static void block_job_finish_cb(void *opaque, int ret)
 {
-struct BlockCancelData *data = opaque;
+struct BlockFinishData *data = opaque;
 
 data->cancelled = block_job_is_cancelled(data->job);
 data->ret = ret;
 data->cb(data->opaque, ret);
 }
 
-int block_job_cancel_sync(BlockJob *job)
+static int block_job_finish_sync(BlockJob *job,
+ void (*finish)(BlockJob *, Error **errp),
+ Error **errp)
 {
-struct BlockCancelData data;
+struct BlockFinishData data;
 BlockDriverState *bs = job->bs;
+Error *local_err = NULL;
 
 assert(bs->job == job);
 
@@ -179,15 +182,37 @@ int block_job_cancel_sync(BlockJob *job)
 data.cb = job->cb;
 data.opaque = job->opaque;
 data.ret = -EINPROGRESS;
-job->cb = block_job_cancel_cb;
+job->cb = block_job_finish_cb;
 job->opaque = &data;
-block_job_cancel(job);
+finish(job, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return -EBUSY;
+}
 while (data.ret == -EINPROGRESS) {
 qemu_aio_wait();
 }
 return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
 }
 
+/* A wrapper around block_job_cancel() taking an Error ** parameter so it may 
be
+ * used with block_job_finish_sync() without the need for (rather nasty)
+ * function pointer casts there. */
+static void block_job_cancel_err(BlockJob *job, Error **errp)
+{
+block_job_cancel(job);
+}
+
+int block_job_cancel_sync(BlockJob *job)
+{
+return block_job_finish_sync(job, &block_job_cancel_err, NULL);
+}
+
+int block_job_complete_sync(BlockJob *job, Error **errp)
+{
+return block_job_finish_sync(job, &block_job_complete, errp);
+}
+
 void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
 {
 assert(job->busy);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index d76de62..626ea42 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -253,6 +253,21 @@ bool block_job_is_paused(BlockJob *job);
 int block_job_cancel_sync(BlockJob *job);
 
 /**
+ * block_job_complete_sync:
+ * @job: The job to be completed.
+ * @errp: Error object which may be set by block_job_complete(); this is not
+ *necessarily set on every error, the job return value has to be
+ *checked as well.
+ *
+ * Synchronously complete the job.  The completion callback is called before 
the
+ * function returns, unless it is NULL (which is permissible when using this
+ * function).
+ *
+ * Returns the return value from the job.
+ */
+int block_job_complete_sync(BlockJob *job, Error **errp);
+
+/**
  * block_job_iostatus_reset:
  * @job: The job whose I/O status should be reset.
  *
-- 
1.9.2

[Qemu-devel] [PULL 19/38] pci-assign: propagate errors from assign_device()

[Luiz Capitulino]

From: Laszlo Ersek 

Also, change the return type to "void"; the function is static (with a
sole caller) and the negative errno values are not distinguished from each
other.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 9aa92a1..0fedca8 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -795,7 +795,7 @@ fail:
 return g_strdup("Couldn't find out why.");
 }
 
-static int assign_device(AssignedDevice *dev)
+static void assign_device(AssignedDevice *dev, Error **errp)
 {
 uint32_t flags = KVM_DEV_ASSIGN_ENABLE_IOMMU;
 int r;
@@ -803,15 +803,15 @@ static int assign_device(AssignedDevice *dev)
 /* Only pass non-zero PCI segment to capable module */
 if (!kvm_check_extension(kvm_state, KVM_CAP_PCI_SEGMENT) &&
 dev->host.domain) {
-error_report("Can't assign device inside non-zero PCI segment "
- "as this KVM module doesn't support it.");
-return -ENODEV;
+error_setg(errp, "Can't assign device inside non-zero PCI segment "
+   "as this KVM module doesn't support it.");
+return;
 }
 
 if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
-error_report("No IOMMU found.  Unable to assign device \"%s\"",
- dev->dev.qdev.id);
-return -ENODEV;
+error_setg(errp, "No IOMMU found.  Unable to assign device \"%s\"",
+   dev->dev.qdev.id);
+return;
 }
 
 if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK &&
@@ -826,18 +826,17 @@ static int assign_device(AssignedDevice *dev)
 char *cause;
 
 cause = assign_failed_examine(dev);
-error_report("Failed to assign device \"%s\" : %s\n%s",
- dev->dev.qdev.id, strerror(-r), cause);
+error_setg_errno(errp, -r, "Failed to assign device \"%s\"\n%s",
+ dev->dev.qdev.id, cause);
 g_free(cause);
 break;
 }
 default:
-error_report("Failed to assign device \"%s\" : %s",
- dev->dev.qdev.id, strerror(-r));
+error_setg_errno(errp, -r, "Failed to assign device \"%s\"",
+ dev->dev.qdev.id);
 break;
 }
 }
-return r;
 }
 
 static void verify_irqchip_in_kernel(Error **errp)
@@ -1812,8 +1811,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 dev->intx_route.irq = -1;
 
 /* assign device to guest */
-r = assign_device(dev);
-if (r < 0) {
+assign_device(dev, &local_err);
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
 goto out;
 }
 
-- 
1.9.0

[Qemu-devel] [PATCH v7 14/14] iotests: Omit length/offset test in 040 and 041

[Max Reitz]

As the length of a mirror block job no longer directly depends on the
size of the block device, drop those checks from this test. Instead,
just check whether the final offset equals the block job length.

As 041 uses the wait_until_completed function from iotests.py, the same
applies there as well which in turn affects tests 030, 055 and 056. On
the other hand, a block job's length does not have to be related to the
length of the image file in the first place, so that check was
questionable anyway.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/040| 4 +---
 tests/qemu-iotests/041| 3 +--
 tests/qemu-iotests/iotests.py | 3 +--
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 734b6a6..ca2b82e 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -46,13 +46,11 @@ class ImageCommitTestCase(iotests.QMPTestCase):
 if event['event'] == 'BLOCK_JOB_COMPLETED':
 self.assert_qmp(event, 'data/type', 'commit')
 self.assert_qmp(event, 'data/device', 'drive0')
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
+self.assert_qmp(event, 'data/offset', event['data']['len'])
 completed = True
 elif event['event'] == 'BLOCK_JOB_READY':
 self.assert_qmp(event, 'data/type', 'commit')
 self.assert_qmp(event, 'data/device', 'drive0')
-self.assert_qmp(event, 'data/len', self.image_len)
 self.vm.qmp('block-job-complete', device='drive0')
 
 self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index ec470b2..59b958b 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -46,8 +46,7 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
 event = self.cancel_and_wait()
 self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
 self.assert_qmp(event, 'data/type', 'mirror')
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
+self.assert_qmp(event, 'data/offset', event['data']['len'])
 
 def complete_and_wait(self, drive='drive0', wait_ready=True):
 '''Complete a block job and wait for it to finish'''
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f6c437c..ff474d4 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -266,8 +266,7 @@ class QMPTestCase(unittest.TestCase):
 self.assert_qmp(event, 'data/device', drive)
 self.assert_qmp_absent(event, 'data/error')
 if check_offset:
-self.assert_qmp(event, 'data/offset', self.image_len)
-self.assert_qmp(event, 'data/len', self.image_len)
+self.assert_qmp(event, 'data/offset', 
event['data']['len'])
 completed = True
 
 self.assert_no_active_block_jobs()
-- 
1.9.2

Re: [Qemu-devel] [Bug 1316115] Re: linux-user qemu-arm NEON support

[Peter Maydell]

On 8 May 2014 20:54, Christopher Horler  wrote:
> I built Qt5 myself, and tested and it crashed again.
>
> I think the entry point getting set in the ELF header is probably
> invalid and leading to the crash - I'm going to try and fix that - but
> it's almost certainly not a qemu bug.

Does the same ARM binary work on real hardware?

thanks
-- PMM

[Qemu-devel] [PULL 12/38] pci: add Error-propagating pci_add_capability2()

[Luiz Capitulino]

From: Laszlo Ersek 

... and rebase pci_add_capability() to it.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/pci/pci.c | 32 ++--
 include/hw/pci/pci.h |  4 
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 517ff2a..22fe5ee 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2013,12 +2013,32 @@ static void pci_del_option_rom(PCIDevice *pdev)
 int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size)
 {
+int ret;
+Error *local_err = NULL;
+
+ret = pci_add_capability2(pdev, cap_id, offset, size, &local_err);
+if (local_err) {
+assert(ret < 0);
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
+} else {
+/* success implies a positive offset in config space */
+assert(ret > 0);
+}
+return ret;
+}
+
+int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
+   uint8_t offset, uint8_t size,
+   Error **errp)
+{
 uint8_t *config;
 int i, overlapping_cap;
 
 if (!offset) {
 offset = pci_find_space(pdev, size);
 if (!offset) {
+error_setg(errp, "out of PCI config space");
 return -ENOSPC;
 }
 } else {
@@ -2029,12 +2049,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
 for (i = offset; i < offset + size; i++) {
 overlapping_cap = pci_find_capability_at_offset(pdev, i);
 if (overlapping_cap) {
-fprintf(stderr, "ERROR: %s:%02x:%02x.%x "
-"Attempt to add PCI capability %x at offset "
-"%x overlaps existing capability %x at offset %x\n",
-pci_root_bus_path(pdev), pci_bus_num(pdev->bus),
-PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
-cap_id, offset, overlapping_cap, i);
+error_setg(errp, "%s:%02x:%02x.%x "
+   "Attempt to add PCI capability %x at offset "
+   "%x overlaps existing capability %x at offset %x",
+   pci_root_bus_path(pdev), pci_bus_num(pdev->bus),
+   PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn),
+   cap_id, offset, overlapping_cap, i);
 return -EINVAL;
 }
 }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 693dd6b..8c25ae5 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -6,6 +6,7 @@
 #include "hw/qdev.h"
 #include "exec/memory.h"
 #include "sysemu/dma.h"
+#include "qapi/error.h"
 
 /* PCI includes legacy ISA access.  */
 #include "hw/isa/isa.h"
@@ -308,6 +309,9 @@ pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int 
region_num);
 
 int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size);
+int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
+   uint8_t offset, uint8_t size,
+   Error **errp);
 
 void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
 
-- 
1.9.0

[Qemu-devel] [PULL 00/38] QMP queue

[Luiz Capitulino]

The following changes since commit 6b342cc9c872e82620fdd32730cd92affa8a19b3:

  Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' into staging 
(2014-05-08 10:57:25 +0100)

are available in the git repository at:


  git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8:

  Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()" 
(2014-05-08 14:20:26 -0400)


Amos Kong (1):
  qapi: treat all negative return of strtosz_suffix() as error

Eric Blake (2):
  qmp: use valid JSON in transaction example
  qapi: Document optional arguments' backwards compatibility

Laszlo Ersek (16):
  cutils: tighten qemu_parse_fd()
  monitor: add Error-propagating monitor_handle_fd_param2()
  pci-assign: accept Error from monitor_handle_fd_param2()
  pci-assign: make assign_failed_examine() just format the cause
  pci-assign: propagate errors from get_real_id()
  pci-assign: propagate Error from check_irqchip_in_kernel()
  pci: add Error-propagating pci_add_capability2()
  pci-assign: accept Error from pci_add_capability2()
  pci-assign: assignment should fail if we can't read config space
  pci-assign: propagate errors from get_real_device()
  pci-assign: propagate errors from assigned_device_pci_cap_init()
  pci-assign: propagate errors from assigned_dev_register_msix_mmio()
  pci-assign: propagate errors from assigned_dev_register_regions()
  pci-assign: propagate errors from assign_device()
  pci-assign: propagate errors from assign_intx()
  pci-assign: assigned_initfn(): set monitor error in common error handler

Lluís Vilanova (4):
  qapi: [trivial] Break long command lines
  qapi: [trivial] Do not catch unknown exceptions in "test-qapi.py"
  qapi: Use an explicit input file
  qapi: Add a primitive to include other files from a QAPI schema file

Markus Armbruster (14):
  qmp hmp: Consistently name Error * objects err, and not errp
  qga: Consistently name Error ** objects errp, and not err
  qmp: Consistently name Error ** objects errp, and not err
  error: Consistently name Error ** objects errp, and not err
  qga: Use return values instead of error_is_set(errp)
  hmp: Guard against misuse of hmp_handle_error()
  qapi: Drop redundant, unclean error_is_set()
  tests/qapi-schema: Drop superfluous error_is_set()
  qapi: Clean up fragile use of error_is_set()
  qga: Clean up fragile use of error_is_set()
  qga: Drop superfluous error_is_set()
  qemu-option: Clean up fragile use of error_is_set()
  dump: Drop pointless error_is_set(), DumpState member errp
  qmp: Don't use error_is_set() to suppress additional errors

Peter Lieven (1):
  Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()"

 Makefile   |  24 +-
 docs/qapi-code-gen.txt |  45 +++-
 docs/writing-qmp-commands.txt  |  28 +--
 dump.c |   6 +-
 hmp.c  | 141 +--
 hw/i386/kvm/pci-assign.c   | 273 +
 hw/pci/pci.c   |  32 ++-
 include/hw/pci/pci.h   |   4 +
 include/monitor/monitor.h  |   1 +
 include/qapi/error.h   |  27 +-
 include/qapi/qmp/dispatch.h|   2 +-
 monitor.c  |  27 +-
 qapi/opts-visitor.c|  11 +-
 qapi/qapi-dealloc-visitor.c|   4 +-
 qapi/qmp-dispatch.c|  24 +-
 qga/commands-posix.c   | 213 
 qga/commands-win32.c   | 123 +-
 qga/commands.c |   4 +-
 qga/main.c |   1 +
 qga/vss-win32.c|   4 +-
 qga/vss-win32.h|   2 +-
 qmp-commands.hx|   8 +-
 qmp.c  |  42 ++--
 scripts/qapi-commands.py   |   9 +-
 scripts/qapi-types.py  |   9 +-
 scripts/qapi-visit.py  |   9 +-
 scripts/qapi.py|  69 --
 tests/Makefile |  26 +-
 tests/qapi-schema/duplicate-key.err|   2 +-
 .../qapi-schema/flat-union-invalid-branch-key.err  |   2 +-
 .../flat-union-invalid-discriminator.err   |   2 +-
 tests/qapi-schema/flat-union-no-base.err   |   2 +-
 .../flat-union-string-discriminator.

Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent.

[Benoît Canet]

The Thursday 08 May 2014 à 20:30:33 (+0200), Markus Armbruster wrote :
> Humor me: no period at end of subject.
> 
> Benoît Canet  writes:
> 
> > The purpose of this change is to help create a json file containing
> > common definitions.
> 
> Please describe the new semantics of the include directive here, so
> mathematically challenged readers don't have to loop up "idempotent" in
> the dictionary :)
> 
> > The history used to detect multiple inclusions is not a stack anymore
> > but a regular list.
> 
> You need a stack to show the actual include cycle.
> 
> There are two reasons to detect cycles.  The technical one is preventing
> infinite expansion.  No longer applies with idempotent include.  The
> other, more abstract one is rejecting nonsensical use of the include
> directive.  I think that one still applies.
> 
> > The cycle detection check where updated and leaved in place to make
> > sure the code will never enter into an infinite recursion loop.
> 
> -ENOPARSE
> 
> Suggest to retry in active voice :)
> 
> > Signed-off-by: Benoit Canet 
> > ---
> >  scripts/qapi.py   |   13 ++---
> >  tests/Makefile|3 ++-
> >  tests/qapi-schema/include-cycle.err   |3 ---
> >  tests/qapi-schema/include-cycle.exit  |2 +-
> >  tests/qapi-schema/include-cycle.out   |3 +++
> >  tests/qapi-schema/include-idempotent.exit |1 +
> >  tests/qapi-schema/include-idempotent.json |3 +++
> >  tests/qapi-schema/include-idempotent.out  |3 +++
> >  tests/qapi-schema/include-self-cycle.err  |1 -
> >  tests/qapi-schema/include-self-cycle.exit |2 +-
> >  tests/qapi-schema/include-self-cycle.out  |3 +++
> >  tests/qapi-schema/sub-include-idempotent.json |3 +++
> >  12 files changed, 26 insertions(+), 14 deletions(-)
> >  create mode 100644 tests/qapi-schema/include-idempotent.err
> >  create mode 100644 tests/qapi-schema/include-idempotent.exit
> >  create mode 100644 tests/qapi-schema/include-idempotent.json
> >  create mode 100644 tests/qapi-schema/include-idempotent.out
> >  create mode 100644 tests/qapi-schema/sub-include-idempotent.json
> 
> Is this based on Luiz's queue-qmp?
> 
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index ec806aa..0ed44c8 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -79,7 +79,7 @@ class QAPISchema:
> >  input_relname = fp.name
> >  self.input_dir = os.path.dirname(input_fname)
> >  self.input_file = input_relname
> > -self.include_hist = include_hist + [(input_relname, input_fname)]
> > +include_hist.append((input_relname, input_fname))
> >  self.parent_info = parent_info
> >  self.src = fp.read()
> >  if self.src == '' or self.src[-1] != '\n':
> 
> Looks like you drop self.include_hist and simply keep it in local
> variable include_hist.  Have you considered doing that in a separate
> cleanup patch prior to this one?
> 
> > @@ -102,17 +102,16 @@ class QAPISchema:
> >  'Expected a file name (string), 
> > got: %s'
> >  % include)
> >  include_path = os.path.join(self.input_dir, include)
> > -if any(include_path == elem[1]
> > -   for elem in self.include_hist):
> > -raise QAPIExprError(expr_info, "Inclusion loop for %s"
> > -% include)
> > +# make include idempotent by skipping further includes
> > +if include_path in [x[1] for x in  include_hist]:
> > +continue
> 
> Loses cycle detection.

It simply also skip cycle includes. If cycle are skipped then cannot do no
harm.

> 
> Is permitting cycles necessary to solve your problem?  Or asking more
> directly: do you actually need cyclic includes?
> 
> >  try:
> >  fobj = open(include_path, 'r')
> >  except IOError as e:
> >  raise QAPIExprError(expr_info,
> >  '%s: %s' % (e.strerror, include))
> > -exprs_include = QAPISchema(fobj, include,
> > -   self.include_hist, expr_info)
> > +exprs_include = QAPISchema(fobj, include, include_hist,
> > +   expr_info)
> >  self.exprs.extend(exprs_include.exprs)
> >  else:
> >  expr_elem = {'expr': expr,
> > diff --git a/tests/Makefile b/tests/Makefile
> > index a8d45be..c587b4a 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
> >  flat-union-string-discriminator.json \
> >  include-simple.json include-relpath.json include-format-err.json \
> >  include-non

[Qemu-devel] [PATCH v2] vl.c: Unify MAX_CPUMASK_BITS and machine->max_cpus checks

[Eduardo Habkost]

If a given machine have max_cpus set, not just smp_cpus needs to be
limited, but the total number of CPUs (considering CPU hotplug) for the
machine.

We also had yet another max_cpus limit check at smp_parse(), ensuring
that max_cpus < MAX_CPUMASK_BITS.

This patch unifies the machine->max_cpus and MAX_CPUMASK_BITS checks,
and also moves the (smp_cpus <= max_cpus) outside smp_parse(), to keep
all checks in the same place.

With those changes, the new code ensures that:

  1 <= smp_cpus <= max_cpus <= machine->max_cpus <= MAX_CPUMASK_BITS

Signed-off-by: Eduardo Habkost 
Cc: Peter Maydell 
Cc: Andreas Färber 
Cc: Igor Mammedov 
Cc: Marcelo Tosatti 
---
Changes v1 -> v2:
 * v1 was: [PATCH] vl.c: Check max_cpus limit instead of smp_cpus
 * Unify machine->max_cpus and MAX_CPUMASK_BITS check
 * Move all checks outside smp_parse()
 * Add assert() lines ensuring the results are consistent
 * s/machine/machine_class/, after rebase to latest qemu.git
   (commit 6b342cc)
---
 vl.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/vl.c b/vl.c
index 73e0661..434cdc3 100644
--- a/vl.c
+++ b/vl.c
@@ -1425,15 +1425,6 @@ static void smp_parse(QemuOpts *opts)
 max_cpus = smp_cpus;
 }
 
-if (max_cpus > MAX_CPUMASK_BITS) {
-fprintf(stderr, "Unsupported number of maxcpus\n");
-exit(1);
-}
-if (max_cpus < smp_cpus) {
-fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
-exit(1);
-}
-
 }
 
 static void configure_realtime(QemuOpts *opts)
@@ -4055,13 +4046,24 @@ int main(int argc, char **argv, char **envp)
 smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
 machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
-if (smp_cpus > machine_class->max_cpus) {
-fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
-"supported by machine `%s' (%d)\n", smp_cpus,
+machine_class->max_cpus = MIN(machine_class->max_cpus, MAX_CPUMASK_BITS);
+
+if (max_cpus < smp_cpus) {
+fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
+exit(1);
+}
+if (max_cpus > machine_class->max_cpus) {
+fprintf(stderr, "Total number of CPUs (%d), exceeds maximum "
+"supported by machine `%s' (%d)\n", max_cpus,
 machine_class->name, machine_class->max_cpus);
 exit(1);
 }
 
+assert(1 <= smp_cpus);
+assert(smp_cpus <= max_cpus);
+assert(max_cpus <= machine_class->max_cpus);
+assert(machine_class->max_cpus <= MAX_CPUMASK_BITS);
+
 /*
  * Get the default machine options from the machine if it is not already
  * specified either by the configuration file or by the command line.
-- 
1.9.0

[Qemu-devel] [PULL 25/38] error: Consistently name Error ** objects errp, and not err

[Luiz Capitulino]

From: Markus Armbruster 

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 include/qapi/error.h | 27 ---
 util/error.c |  8 
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index c0f0c3b..7995801 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -27,14 +27,16 @@ typedef struct Error Error;
  * printf-style human message.  This function is not meant to be used outside
  * of QEMU.
  */
-void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) 
GCC_FMT_ATTR(3, 4);
+void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
+GCC_FMT_ATTR(3, 4);
 
 /**
  * Set an indirect pointer to an error given a ErrorClass value and a
  * printf-style human message, followed by a strerror() string if
  * @os_error is not zero.
  */
-void error_set_errno(Error **err, int os_error, ErrorClass err_class, const 
char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void error_set_errno(Error **errp, int os_error, ErrorClass err_class,
+ const char *fmt, ...) GCC_FMT_ATTR(4, 5);
 
 #ifdef _WIN32
 /**
@@ -42,19 +44,22 @@ void error_set_errno(Error **err, int os_error, ErrorClass 
err_class, const char
  * printf-style human message, followed by a g_win32_error_message() string if
  * @win32_err is not zero.
  */
-void error_set_win32(Error **err, int win32_err, ErrorClass err_class, const 
char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
+ const char *fmt, ...) GCC_FMT_ATTR(4, 5);
 #endif
 
 /**
  * Same as error_set(), but sets a generic error
  */
-#define error_setg(err, fmt, ...) \
-error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
-#define error_setg_errno(err, os_error, fmt, ...) \
-error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## 
__VA_ARGS__)
+#define error_setg(errp, fmt, ...) \
+error_set(errp, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
+#define error_setg_errno(errp, os_error, fmt, ...) \
+error_set_errno(errp, os_error, ERROR_CLASS_GENERIC_ERROR, \
+fmt, ## __VA_ARGS__)
 #ifdef _WIN32
-#define error_setg_win32(err, win32_err, fmt, ...) \
-error_set_win32(err, win32_err, ERROR_CLASS_GENERIC_ERROR, fmt, ## 
__VA_ARGS__)
+#define error_setg_win32(errp, win32_err, fmt, ...) \
+error_set_win32(errp, win32_err, ERROR_CLASS_GENERIC_ERROR, \
+fmt, ## __VA_ARGS__)
 #endif
 
 /**
@@ -66,7 +71,7 @@ void error_setg_file_open(Error **errp, int os_errno, const 
char *filename);
  * Returns true if an indirect pointer to an error is pointing to a valid
  * error object.
  */
-bool error_is_set(Error **err);
+bool error_is_set(Error **errp);
 
 /*
  * Get the error class of an error object.
@@ -88,7 +93,7 @@ const char *error_get_pretty(Error *err);
  * always transfer ownership of the error reference and handles the case where
  * dst_err is NULL correctly.  Errors after the first are discarded.
  */
-void error_propagate(Error **dst_err, Error *local_err);
+void error_propagate(Error **dst_errp, Error *local_err);
 
 /**
  * Free an error object.
diff --git a/util/error.c b/util/error.c
index 2bb42e1..66245cc 100644
--- a/util/error.c
+++ b/util/error.c
@@ -165,13 +165,13 @@ void error_free(Error *err)
 }
 }
 
-void error_propagate(Error **dst_err, Error *local_err)
+void error_propagate(Error **dst_errp, Error *local_err)
 {
-if (local_err && dst_err == &error_abort) {
+if (local_err && dst_errp == &error_abort) {
 error_report("%s", error_get_pretty(local_err));
 abort();
-} else if (dst_err && !*dst_err) {
-*dst_err = local_err;
+} else if (dst_errp && !*dst_errp) {
+*dst_errp = local_err;
 } else if (local_err) {
 error_free(local_err);
 }
-- 
1.9.0

[Qemu-devel] [Bug 1316115] Re: linux-user qemu-arm NEON support

[Christopher Horler]

I built Qt5 myself, and tested and it crashed again.

I think the entry point getting set in the ELF header is probably
invalid and leading to the crash - I'm going to try and fix that - but
it's almost certainly not a qemu bug.

I suggest closing the bug report.

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

Title:
  linux-user qemu-arm NEON support

Status in QEMU:
  New

Bug description:
  I was reading the mailing list and saw NEON support in QEmu was making
  progress.

  Is it not supported in user mode?  or am I running into something else
  here?  (I've tried to include some what may be useful information)

  using qemu from git (last commits as below):
  fdaad47 Merge remote-tracking branch 
'remotes/pmaydell/tags/pull-target-arm-20140501' into staging
  e50bf23 Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into 
staging
  c090c10 Merge remote-tracking branch 'remotes/cohuck/tags/kvm_cap_helpers' 
into staging

  (for completeness I should point out this is not actually
  libQtCore.so.4.6.2 - the SONAME shows libQt5Core.so.5).

  chorler@linux-foxtrot:~/projects/src/CustomFirmware> qemu-arm -L ./root 
./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2 
  qemu: unhandled CPU exception 0x2 - aborting
  R00= R01=f6c84fdd R02= R03=
  R04= R05= R06= R07=
  R08= R09= R10=f6ff9d80 R11=
  R12= R13=f6c84d90 R14= R15=f6cdef74
  PSR=0010  A usr32
  qemu: uncaught target signal 6 (Aborted) - core dumped
  Aborted

  
  chorler@linux-foxtrot:~/projects/src/CustomFirmware> 
arm-linux-gnueabihf-readelf -A 
./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2 
  Attribute Section: aeabi
  File Attributes
Tag_CPU_name: "7-A"
Tag_CPU_arch: v7
Tag_CPU_arch_profile: Application
Tag_ARM_ISA_use: Yes
Tag_THUMB_ISA_use: Thumb-2
Tag_FP_arch: VFPv3
Tag_Advanced_SIMD_arch: NEONv1
Tag_ABI_PCS_wchar_t: 4
Tag_ABI_FP_denormal: Needed
Tag_ABI_FP_exceptions: Needed
Tag_ABI_FP_number_model: IEEE 754
Tag_ABI_align_needed: 8-byte
Tag_ABI_align_preserved: 8-byte, except leaf SP
Tag_ABI_enum_size: int
Tag_ABI_HardFP_use: SP and DP
Tag_ABI_VFP_args: VFP registers
Tag_ABI_optimization_goals: Aggressive Speed
Tag_CPU_unaligned_access: v6
Tag_DIV_use: Not allowed


  chorler@linux-foxtrot:~/projects/src/CustomFirmware> gdb qemu-arm
  GNU gdb (GDB; openSUSE 13.1) 7.6.50.20130731-cvs
  Copyright (C) 2013 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later 
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
  and "show warranty" for details.
  This GDB was configured as "x86_64-suse-linux".
  Type "show configuration" for configuration details.
  For bug reporting instructions, please see:
  .
  Find the GDB manual and other documentation resources online at:
  .
  For help, type "help".
  Type "apropos word" to search for commands related to "word".
  ..
  Reading symbols from /home/chorler/projects/bin/qemu-arm...done.
  (gdb) list main.c:685
  680
  681 for(;;) {
  682 cpu_exec_start(cs);
  683 trapnr = cpu_arm_exec(env);
  684 cpu_exec_end(cs);
  685 switch(trapnr) {
  686 case EXCP_UDEF:
  687 {
  688 TaskState *ts = cs->opaque;
  689 uint32_t opcode;
  (gdb) break main.c:685
  Breakpoint 3 at 0x60059773: file 
/home/chorler/projects/src/qemu/linux-user/main.c, line 685.
  (gdb) run -L ./root 
./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2
  Starting program: /home/chorler/projects/bin/qemu-arm -L ./root 
./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib64/libthread_db.so.1".

  Breakpoint 3, cpu_loop (env=env@entry=0x6255e650) at 
/home/chorler/projects/src/qemu/linux-user/main.c:685
  685 switch(trapnr) {
  (gdb) print trapnr
  $1 = 2
  (gdb) n
  762 if (trapnr == EXCP_BKPT) {
  (gdb) n
  760 env->eabi = 1;
  (gdb) n
  762 if (trapnr == EXCP_BKPT) {
  (gdb) n
  775 if (env->thumb) {
  (gdb) n
  777 get_user_code_u16(insn, env->regs[15] - 2,
  (gdb) n
  775 if (env->thumb) {
  (gdb) n
  782 get_user_code_u32(insn, env->regs[15] - 4,
  (gdb) n
  784 n = insn & 0xff;
  (gdb) n
  788 if (n == ARM_NR_cac

Re: [Qemu-devel] [RFC] dataplane: IOThreads and writing dataplane-capable code

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (stefa...@gmail.com) wrote:
> On Thu, May 8, 2014 at 3:44 PM, Dr. David Alan Gilbert
>  wrote:
> > * Stefan Hajnoczi (stefa...@redhat.com) wrote:
> >
> > 
> >
> >> How to synchronize with an IOThread
> >> ---
> >> AioContext is not thread-safe so some rules must be followed when using 
> >> file
> >> descriptors, event notifiers, timers, or BHs across threads:
> >>
> >> 1. AioContext functions can be called safely from file descriptor, event
> >> notifier, timer, or BH callbacks invoked by the AioContext.  No locking is
> >> necessary.
> >>
> >> 2. Other threads wishing to access the AioContext must use
> >> aio_context_acquire()/aio_context_release() for mutual exclusion.  Once the
> >> context is acquired no other thread can access it or run event loop 
> >> iterations
> >> in this AioContext.
> >>
> >> aio_context_acquire()/aio_context_release() calls may be nested.  This
> >> means you can call them if you're not sure whether #1 applies.
> >>
> >> Side note: the best way to schedule a function call across threads is to 
> >> create
> >> a BH in the target AioContext beforehand and then call qemu_bh_schedule(). 
> >>  No
> >> acquire/release or locking is needed for the qemu_bh_schedule() call.  But 
> >> be
> >> sure to acquire the AioContext for aio_bh_new() if necessary.
> >
> > How do these IOThreads pause during migration?
> > Are they paused by the 'qemu_mutex_lock_iothread' that the migration thread 
> > calls?
> 
> Currently the only IOThread user is virtio-blk data-plane.  It has a
> VM state change listener registered that will stop using the IOThread
> during migration.
> 
> In the future we'll have to do more than that:
> It is possible to suspend all IOThreads simply by looping over
> IOThread objects and calling aio_context_acquire() on their
> AioContext.  You can release the AioContexts when you are done.  This
> would be suitable for a "stop the world" operation for migration
> hand-over.

That worries me for two reasons:
   1) I'm assuming there is some subtlety so that it doesn't deadlock when
 another thread is trying to get a couple of contexts.
   2) The migration code that has to pause everything is reasonably time
  critical (OK not super critical - but it worries if it gains more than a 
few
  ms).   Doing something to each thread in series where that thread might
  have to finish up a transaction sounds like it could add together to be 
quite
  large.

> For smaller one-off operations like block-migration.c it may also make
> sense to acquire/release the AioContext.  But that's not necessary
> today since dataplane is disabled during migration.

I guess it's probably right to hide this behind some interface on the Aio stuff
that migration can call and it can worry about speed, and locking order etc.

I also would we end up wanting some IOThreads to continue - e.g. could we be 
using
them for transport of the migration stream or are they strictly for the guests
use?

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

[Qemu-devel] [PATCH 8/8] hw/arm/omap_gpmc: Avoid buffer overrun filling prefetch FIFO

[Peter Maydell]

In fill_prefetch_fifo(), if the device we are reading from is 16 bit,
then we must not try to transfer an odd number of bytes into the FIFO.
This could otherwise have resulted in our overrunning the prefetch.fifo
array by one byte.

Signed-off-by: Peter Maydell 
---
Spotted by Coverity. I suspect Coverity is not smart enough
to figure out that this change really does prevent the overrun,
though :-(
---
 hw/misc/omap_gpmc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/misc/omap_gpmc.c b/hw/misc/omap_gpmc.c
index 2047274..cddea24 100644
--- a/hw/misc/omap_gpmc.c
+++ b/hw/misc/omap_gpmc.c
@@ -242,6 +242,10 @@ static void fill_prefetch_fifo(struct omap_gpmc_s *s)
 if (bytes > s->prefetch.count) {
 bytes = s->prefetch.count;
 }
+if (is16bit) {
+bytes &= ~1;
+}
+
 s->prefetch.count -= bytes;
 s->prefetch.fifopointer += bytes;
 fptr = 64 - s->prefetch.fifopointer;
-- 
1.9.2

[Qemu-devel] [Bug 1312561] Re: libstdc++-6.dll is missing from your computer

[Rupert Russell]

So the answer to my basic question, how to make this work is Download the mingw 
windows installer from here.
http://cznic.dl.sourceforge.net/project/mingwbuilds/mingw-builds-install/mingw-builds-install.exe

Install for W 64 posix or win32 and VERSION 4.6.3  (later one doesn't work with 
 qemu-w64-setup-20140418.exe.
Add to the environment path (Something like 
C:\Program Files\mingw-builds\x64-4.6.3-posix-sjlj-rev2\mingw\bin or 
C:\Program Files\mingw-builds\x64-4.6.3-win32-sjlj-rev2\mingw\bin   both seem 
to work.

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

Title:
  libstdc++-6.dll is missing from your computer

Status in QEMU:
  New

Bug description:
  qemu-w64-setup-20140418.exe

  Windows 7 64 bit PC.

  qemu-system-armw -kernel kernel-qemu -cpu arm1176 -m 256 -M
  versatilepb -no-reboot -serial stdio -append "root=/dev/sda2 panic=1
  rootfstype=ext4 rw" -hda c:\11\rasimg\test.vhd

  
  qemu-system-armw.exe - System Error
  The program can't start because libstdc++-6.dll is missing from your 
computer. 

  Try reinstalling the program to fix this problem.

  I tried reinstalling, but no change.

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

[Qemu-devel] [PULL 03/38] qapi: Use an explicit input file

[Luiz Capitulino]

From: Lluís Vilanova 

Use an explicit input file on the command-line instead of reading from standard
input.

It also outputs the proper file name when there's an error.

Signed-off-by: Lluís Vilanova 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 Makefile   | 12 ++--
 docs/qapi-code-gen.txt |  4 ++--
 scripts/qapi-commands.py   |  9 ++---
 scripts/qapi-types.py  |  9 ++---
 scripts/qapi-visit.py  |  9 ++---
 scripts/qapi.py|  5 +++--
 tests/Makefile | 11 ++-
 tests/qapi-schema/duplicate-key.err|  2 +-
 tests/qapi-schema/flat-union-invalid-branch-key.err|  2 +-
 tests/qapi-schema/flat-union-invalid-discriminator.err |  2 +-
 tests/qapi-schema/flat-union-no-base.err   |  2 +-
 tests/qapi-schema/flat-union-string-discriminator.err  |  2 +-
 tests/qapi-schema/funny-char.err   |  2 +-
 tests/qapi-schema/missing-colon.err|  2 +-
 tests/qapi-schema/missing-comma-list.err   |  2 +-
 tests/qapi-schema/missing-comma-object.err |  2 +-
 tests/qapi-schema/non-objects.err  |  2 +-
 tests/qapi-schema/quoted-structural-chars.err  |  2 +-
 tests/qapi-schema/test-qapi.py |  3 ++-
 tests/qapi-schema/trailing-comma-list.err  |  2 +-
 tests/qapi-schema/trailing-comma-object.err|  2 +-
 tests/qapi-schema/unclosed-list.err|  2 +-
 tests/qapi-schema/unclosed-object.err  |  2 +-
 tests/qapi-schema/unclosed-string.err  |  2 +-
 tests/qapi-schema/union-invalid-base.err   |  2 +-
 25 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/Makefile b/Makefile
index 2b8e312..a120aab 100644
--- a/Makefile
+++ b/Makefile
@@ -239,33 +239,33 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py 
$(SRC_PATH)/scripts/ordereddict.py
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
-   $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \
+   $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \
"  GEN   $@")
 qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
-   $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \
+   $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \
"  GEN   $@")
 qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py 
$(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
-   $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \
+   $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \
"  GEN   $@")
 
 qapi-types.c qapi-types.h :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
-   $(gen-out-type) -o "." -b < $<, \
+   $(gen-out-type) -o "." -b -i $<, \
"  GEN   $@")
 qapi-visit.c qapi-visit.h :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
-   $(gen-out-type) -o "." -b < $<, \
+   $(gen-out-type) -o "." -b -i $<, \
"  GEN   $@")
 qmp-commands.h qmp-marshal.c :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
-   $(gen-out-type) -o "." -m < $<, \
+   $(gen-out-type) -o "." -m -i $<, \
"  GEN   $@")
 
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h 
qga-qmp-commands.h)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index d78921f..63b03cf 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -221,7 +221,7 @@ created code.
 Example:
 
 mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \
-  --output-dir="qapi-generated" --prefix="example-" < example-schema.json
+  --output-dir="qapi-generated" --prefix="example-" 
--input-file=example-schema.json
 mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c
 /* AUTOMATICALLY GENERATED, DO NOT

[Qemu-devel] [PATCH v7 01/14] qcow2: Allow "full" discard

[Max Reitz]

Normally, discarded sectors should read back as zero. However, there are
cases in which a sector (or rather cluster) should be discarded as if
they were never written in the first place, that is, reading them should
fall through to the backing file again.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block/qcow2-cluster.c  | 27 +--
 block/qcow2-snapshot.c |  2 +-
 block/qcow2.c  |  2 +-
 block/qcow2.h  |  2 +-
 4 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 76d2bcf..283b88b 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1350,7 +1350,7 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
uint64_t cluster_offset)
  * clusters.
  */
 static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
-unsigned int nb_clusters, enum qcow2_discard_type type)
+unsigned int nb_clusters, enum qcow2_discard_type type, bool full_discard)
 {
 BDRVQcowState *s = bs->opaque;
 uint64_t *l2_table;
@@ -1372,23 +1372,30 @@ static int discard_single_l2(BlockDriverState *bs, 
uint64_t offset,
 old_l2_entry = be64_to_cpu(l2_table[l2_index + i]);
 
 /*
- * Make sure that a discarded area reads back as zeroes for v3 images
- * (we cannot do it for v2 without actually writing a zero-filled
- * buffer). We can skip the operation if the cluster is already marked
- * as zero, or if it's unallocated and we don't have a backing file.
+ * If full_discard is false, make sure that a discarded area reads back
+ * as zeroes for v3 images (we cannot do it for v2 without actually
+ * writing a zero-filled buffer). We can skip the operation if the
+ * cluster is already marked as zero, or if it's unallocated and we
+ * don't have a backing file.
  *
  * TODO We might want to use bdrv_get_block_status(bs) here, but we're
  * holding s->lock, so that doesn't work today.
+ *
+ * If full_discard is true, the sector should not read back as zeroes,
+ * but rather fall through to the backing file.
  */
 switch (qcow2_get_cluster_type(old_l2_entry)) {
 case QCOW2_CLUSTER_UNALLOCATED:
-if (!bs->backing_hd) {
+if (full_discard || !bs->backing_hd) {
 continue;
 }
 break;
 
 case QCOW2_CLUSTER_ZERO:
-continue;
+if (!full_discard) {
+continue;
+}
+break;
 
 case QCOW2_CLUSTER_NORMAL:
 case QCOW2_CLUSTER_COMPRESSED:
@@ -1400,7 +1407,7 @@ static int discard_single_l2(BlockDriverState *bs, 
uint64_t offset,
 
 /* First remove L2 entries */
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
-if (s->qcow_version >= 3) {
+if (!full_discard && s->qcow_version >= 3) {
 l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
 } else {
 l2_table[l2_index + i] = cpu_to_be64(0);
@@ -1419,7 +1426,7 @@ static int discard_single_l2(BlockDriverState *bs, 
uint64_t offset,
 }
 
 int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
-int nb_sectors, enum qcow2_discard_type type)
+int nb_sectors, enum qcow2_discard_type type, bool full_discard)
 {
 BDRVQcowState *s = bs->opaque;
 uint64_t end_offset;
@@ -1442,7 +1449,7 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t 
offset,
 
 /* Each L2 table is handled by its own loop iteration */
 while (nb_clusters > 0) {
-ret = discard_single_l2(bs, offset, nb_clusters, type);
+ret = discard_single_l2(bs, offset, nb_clusters, type, full_discard);
 if (ret < 0) {
 goto fail;
 }
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 0aa9def..c5ea2cd 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -436,7 +436,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 qcow2_discard_clusters(bs, qcow2_vm_state_offset(s),
align_offset(sn->vm_state_size, s->cluster_size)
 >> BDRV_SECTOR_BITS,
-   QCOW2_DISCARD_NEVER);
+   QCOW2_DISCARD_NEVER, false);
 
 #ifdef DEBUG_ALLOC
 {
diff --git a/block/qcow2.c b/block/qcow2.c
index a4b97e8..68f70ea 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1866,7 +1866,7 @@ static coroutine_fn int qcow2_co_discard(BlockDriverState 
*bs,
 
 qemu_co_mutex_lock(&s->lock);
 ret = qcow2_discard_clusters(bs, sector_num << BDRV_SECTOR_BITS,
-nb_sectors, QCOW2_DISCARD_REQUEST);
+nb_sectors, QCOW2_DISCARD_REQUEST, false);
 qemu_co_mutex_unlock(&s->lock);
 return ret;
 }
diff --git a/block/qcow2.h b/block/qcow2.h
index b49424b..2332634 100

[Qemu-devel] [PULL 37/38] qapi: Document optional arguments' backwards compatibility

[Luiz Capitulino]

From: Eric Blake 

Signed-off-by: Eric Blake 
Signed-off-by: Fam Zheng 
Signed-off-by: Luiz Capitulino 
---
 docs/qapi-code-gen.txt | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 051d109..26312d8 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -60,10 +60,34 @@ example of a complex type is:
  { 'type': 'MyType',
'data': { 'member1': 'str', 'member2': 'int', '*member3': 'str' } }
 
-The use of '*' as a prefix to the name means the member is optional.  Optional
-members should always be added to the end of the dictionary to preserve
-backwards compatibility.
-
+The use of '*' as a prefix to the name means the member is optional.
+
+The default initialization value of an optional argument should not be changed
+between versions of QEMU unless the new default maintains backward
+compatibility to the user-visible behavior of the old default.
+
+With proper documentation, this policy still allows some flexibility; for
+example, documenting that a default of 0 picks an optimal buffer size allows
+one release to declare the optimal size at 512 while another release declares
+the optimal size at 4096 - the user-visible behavior is not the bytes used by
+the buffer, but the fact that the buffer was optimal size.
+
+On input structures (only mentioned in the 'data' side of a command), changing
+from mandatory to optional is safe (older clients will supply the option, and
+newer clients can benefit from the default); changing from optional to
+mandatory is backwards incompatible (older clients may be omitting the option,
+and must continue to work).
+
+On output structures (only mentioned in the 'returns' side of a command),
+changing from mandatory to optional is in general unsafe (older clients may be
+expecting the field, and could crash if it is missing), although it can be done
+if the only way that the optional argument will be omitted is when it is
+triggered by the presence of a new input flag to the command that older clients
+don't know to send.  Changing from optional to mandatory is safe.
+
+A structure that is used in both input and output of various commands
+must consider the backwards compatibility constraints of both directions
+of use.
 
 A complex type definition can specify another complex type as its base.
 In this case, the fields of the base type are included as top-level fields
-- 
1.9.0

[Qemu-devel] [PULL 23/38] qga: Consistently name Error ** objects errp, and not err

[Luiz Capitulino]

From: Markus Armbruster 

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 qga/commands-posix.c | 199 ++-
 qga/commands-win32.c | 105 ++-
 qga/commands.c   |   4 +-
 qga/vss-win32.c  |   4 +-
 qga/vss-win32.h  |   2 +-
 5 files changed, 160 insertions(+), 154 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 935a4ec..f6af7d1 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -53,7 +53,7 @@ extern char **environ;
 #endif
 #endif
 
-static void ga_wait_child(pid_t pid, int *status, Error **err)
+static void ga_wait_child(pid_t pid, int *status, Error **errp)
 {
 pid_t rpid;
 
@@ -64,14 +64,15 @@ static void ga_wait_child(pid_t pid, int *status, Error 
**err)
 } while (rpid == -1 && errno == EINTR);
 
 if (rpid == -1) {
-error_setg_errno(err, errno, "failed to wait for child (pid: %d)", 
pid);
+error_setg_errno(errp, errno, "failed to wait for child (pid: %d)",
+ pid);
 return;
 }
 
 g_assert(rpid == pid);
 }
 
-void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
+void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
 {
 const char *shutdown_flag;
 Error *local_err = NULL;
@@ -86,7 +87,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, 
Error **err)
 } else if (strcmp(mode, "reboot") == 0) {
 shutdown_flag = "-r";
 } else {
-error_setg(err,
+error_setg(errp,
"mode is invalid (valid values are: halt|powerdown|reboot");
 return;
 }
@@ -103,23 +104,23 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, 
Error **err)
"hypervisor initiated shutdown", (char*)NULL, environ);
 _exit(EXIT_FAILURE);
 } else if (pid < 0) {
-error_setg_errno(err, errno, "failed to create child process");
+error_setg_errno(errp, errno, "failed to create child process");
 return;
 }
 
 ga_wait_child(pid, &status, &local_err);
 if (local_err) {
-error_propagate(err, local_err);
+error_propagate(errp, local_err);
 return;
 }
 
 if (!WIFEXITED(status)) {
-error_setg(err, "child process has terminated abnormally");
+error_setg(errp, "child process has terminated abnormally");
 return;
 }
 
 if (WEXITSTATUS(status)) {
-error_setg(err, "child process has failed to shutdown");
+error_setg(errp, "child process has failed to shutdown");
 return;
 }
 
@@ -234,7 +235,7 @@ static int64_t guest_file_handle_add(FILE *fh, Error **errp)
 return handle;
 }
 
-static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err)
+static GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp)
 {
 GuestFileHandle *gfh;
 
@@ -245,7 +246,7 @@ static GuestFileHandle *guest_file_handle_find(int64_t id, 
Error **err)
 }
 }
 
-error_setg(err, "handle '%" PRId64 "' has not been found", id);
+error_setg(errp, "handle '%" PRId64 "' has not been found", id);
 return NULL;
 }
 
@@ -275,7 +276,7 @@ static const struct {
 };
 
 static int
-find_open_flag(const char *mode_str, Error **err)
+find_open_flag(const char *mode_str, Error **errp)
 {
 unsigned mode;
 
@@ -292,7 +293,7 @@ find_open_flag(const char *mode_str, Error **err)
 }
 
 if (mode == ARRAY_SIZE(guest_file_open_modes)) {
-error_setg(err, "invalid file open mode '%s'", mode_str);
+error_setg(errp, "invalid file open mode '%s'", mode_str);
 return -1;
 }
 return guest_file_open_modes[mode].oflag_base | O_NOCTTY | O_NONBLOCK;
@@ -303,7 +304,7 @@ find_open_flag(const char *mode_str, Error **err)
S_IROTH | S_IWOTH)
 
 static FILE *
-safe_open_or_create(const char *path, const char *mode, Error **err)
+safe_open_or_create(const char *path, const char *mode, Error **errp)
 {
 Error *local_err = NULL;
 int oflag;
@@ -370,11 +371,12 @@ safe_open_or_create(const char *path, const char *mode, 
Error **err)
 }
 }
 
-error_propagate(err, local_err);
+error_propagate(errp, local_err);
 return NULL;
 }
 
-int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, 
Error **err)
+int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
+Error **errp)
 {
 FILE *fh;
 Error *local_err = NULL;
@@ -387,7 +389,7 @@ int64_t qmp_guest_file_open(const char *path, bool 
has_mode, const char *mode, E
 slog("guest-file-open called, filepath: %s, mode: %s", path, mode);
 fh = safe_open_or_create(path, mode, &local_err);
 if (local_err != NULL) {
-error_propagate(err, local_err);
+error_propagate(errp, local_err);
 return -1;
 }
 
@@ -398,14 +400,14 @@ int64_t qmp_

[Qemu-devel] [PATCH v7 13/14] iotests: Add test for qcow2's bdrv_make_empty

[Max Reitz]

Add a test for qcow2's fast bdrv_make_empty implementation on images
without internal snapshots.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/093 | 72 ++
 tests/qemu-iotests/093.out | 26 +
 tests/qemu-iotests/group   |  1 +
 3 files changed, 99 insertions(+)
 create mode 100755 tests/qemu-iotests/093
 create mode 100644 tests/qemu-iotests/093.out

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
new file mode 100755
index 000..85b8a42
--- /dev/null
+++ b/tests/qemu-iotests/093
@@ -0,0 +1,72 @@
+#!/bin/bash
+#
+# Test qcow2's bdrv_make_empty for images without internal snapshots
+#
+# Copyright (C) 2014 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 .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f "$TEST_DIR/blkdebug.conf"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+
+for event in l1_update reftable_update; do
+
+echo
+echo "=== $event ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+_make_test_img -b "$TEST_IMG.base" 64M
+
+cat > "$TEST_DIR/blkdebug.conf" <

[Qemu-devel] [PATCH v7 12/14] iotests: Add test for backing-chain commits

[Max Reitz]

Add a test for qemu-img commit on backing chains with more than two
images. This test also checks whether the top image is emptied (unless
this is prevented by specifying either -d or -b)  emptied and does
therefore not work for qed and vmdk which requires it to be separate
from 020.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/092 | 122 +
 tests/qemu-iotests/092.out | 119 +++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 242 insertions(+)
 create mode 100755 tests/qemu-iotests/092
 create mode 100644 tests/qemu-iotests/092.out

diff --git a/tests/qemu-iotests/092 b/tests/qemu-iotests/092
new file mode 100755
index 000..c7a613b
--- /dev/null
+++ b/tests/qemu-iotests/092
@@ -0,0 +1,122 @@
+#!/bin/bash
+#
+# Commit changes into backing chains and empty the top image if the
+# backing image is not explicitly specified
+#
+# Copyright (C) 2014 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 .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+_rm_test_img "$TEST_IMG.itmd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# Any format supporting backing files and bdrv_make_empty
+_supported_fmt qcow qcow2
+_supported_proto file
+_supported_os Linux
+
+
+# Four passes:
+#  0: Two-layer backing chain, commit to upper backing file (implicitly)
+# (in this case, the top image will be emptied)
+#  1: Two-layer backing chain, commit to upper backing file (explicitly)
+# (in this case, the top image will implicitly stay unchanged)
+#  2: Two-layer backing chain, commit to upper backing file (implicitly with 
-d)
+# (in this case, the top image will explicitly stay unchanged)
+#  3: Two-layer backing chain, commit to lower backing file
+# (in this case, the top image will implicitly stay unchanged)
+#
+# 020 already tests committing, so this only tests whether image chains are
+# working properly and that all images above the base are emptied; therefore,
+# no complicated patterns are necessary
+for i in 0 1 2 3; do
+
+echo
+echo "=== Test pass $i ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 64M
+_make_test_img -b "$TEST_IMG.itmd" 64M
+
+$QEMU_IO -c 'write -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'write -P 2 64k 128k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'write -P 3 128k 64k' "$TEST_IMG" | _filter_qemu_io
+
+if [ $i -lt 3 ]; then
+if [ $i == 0 ]; then
+# -b "$TEST_IMG.itmd" should be the default (that is, committing to the
+# first backing file in the chain)
+$QEMU_IMG commit "$TEST_IMG"
+elif [ $i == 1 ]; then
+# explicitly specify the commit target (this should imply -d)
+$QEMU_IMG commit -b "$TEST_IMG.itmd" "$TEST_IMG"
+else
+# do not explicitly specify the commit target, but use -d to leave the
+# top image unchanged
+$QEMU_IMG commit -d "$TEST_IMG"
+fi
+
+# Bottom should be unchanged
+$QEMU_IO -c 'read -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
+
+# Intermediate should contain changes from top
+$QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+
+# And in pass 0, the top image should be empty, whereas in both other 
passes
+# it should be unchanged (which is both checked by qemu-img map)
+else
+$QEMU_IMG commit -b "$TEST_IMG.base" "$TEST_IMG"
+
+# Bottom should contain all changes
+$QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.base" | _filter_qemu_io
+
+# Both top and intermediate should be unchanged
+fi
+
+$QEMU_IMG map "$TEST_IMG.base" | _filter_qemu_img_map
+$QEMU_IMG map "$TEST_IMG.itmd" | _filter_qemu_img_map
+$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
+
+done
+
+
+# success, all done
+echo "*** done

[Qemu-devel] [PULL 38/38] Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()"

[Luiz Capitulino]

From: Peter Lieven 

This reverts commit 25a7017555f1b4aeb543b5d323ff4afb8f9c5437.

Turns out the argument *can* be null: QEMU now segfaults if it
receives an invalid parameter via a qmp command instead of throwing an
error.

For example:
{ "execute": "blockdev-add",
 "arguments": { "options" : { "driver": "invalid-driver" } } }

CC: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 qapi/qapi-dealloc-visitor.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index d0ea118..dc53545 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -131,7 +131,9 @@ static void qapi_dealloc_end_list(Visitor *v, Error **errp)
 static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name,
   Error **errp)
 {
-g_free(*obj);
+if (obj) {
+g_free(*obj);
+}
 }
 
 static void qapi_dealloc_type_int(Visitor *v, int64_t *obj, const char *name,
-- 
1.9.0

[Qemu-devel] [PATCH v7 08/14] qemu-img: Empty image after commit

[Max Reitz]

After the top image has been committed, it should be emptied unless
specified otherwise.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.c   | 34 +++---
 qemu-img.texi|  6 +-
 3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index d029609..b31d81c 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
 ETEXI
 
 DEF("commit", img_commit,
-"commit [-q] [-f fmt] [-t cache] filename")
+"commit [-q] [-f fmt] [-t cache] [-d] filename")
 STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-d] @var{filename}
 ETEXI
 
 DEF("compare", img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index 75654b8..d92ff27 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -739,14 +739,14 @@ static int img_commit(int argc, char **argv)
 int c, ret, flags;
 const char *filename, *fmt, *cache;
 BlockDriverState *bs, *base_bs;
-bool quiet = false;
+bool quiet = false, drop = false;
 Error *local_err = NULL;
 CommonBlockJobCBInfo cbi;
 
 fmt = NULL;
 cache = BDRV_DEFAULT_CACHE;
 for(;;) {
-c = getopt(argc, argv, "f:ht:q");
+c = getopt(argc, argv, "f:ht:dq");
 if (c == -1) {
 break;
 }
@@ -761,6 +761,9 @@ static int img_commit(int argc, char **argv)
 case 't':
 cache = optarg;
 break;
+case 'd':
+drop = true;
+break;
 case 'q':
 quiet = true;
 break;
@@ -771,7 +774,7 @@ static int img_commit(int argc, char **argv)
 }
 filename = argv[optind++];
 
-flags = BDRV_O_RDWR;
+flags = BDRV_O_RDWR | BDRV_O_UNMAP;
 ret = bdrv_parse_cache_flags(cache, &flags);
 if (ret < 0) {
 error_report("Invalid cache option: %s", cache);
@@ -803,7 +806,32 @@ static int img_commit(int argc, char **argv)
 goto done;
 }
 
+/* The block job will swap base_bs and bs (which is not what we really want
+ * here, but okay) and unref base_bs (after the swap, i.e., the old top
+ * image). In order to still be able to empty that top image afterwards,
+ * increment the reference counter here preemptively. */
+if (!drop) {
+bdrv_ref(base_bs);
+}
+
 run_block_job(bs->job, &local_err);
+if (local_err) {
+goto unref_backing;
+}
+
+if (!drop && base_bs->drv->bdrv_make_empty) {
+ret = base_bs->drv->bdrv_make_empty(base_bs);
+if (ret) {
+error_setg_errno(&local_err, -ret, "Could not empty %s",
+ filename);
+goto unref_backing;
+}
+}
+
+unref_backing:
+if (!drop) {
+bdrv_unref(base_bs);
+}
 
 done:
 bdrv_unref(bs);
diff --git a/qemu-img.texi b/qemu-img.texi
index f84590e..69b2ffe 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -140,7 +140,7 @@ this case. @var{backing_file} will never be modified unless 
you use the
 The size can also be specified using the @var{size} option with @code{-o},
 it doesn't need to be specified separately in this case.
 
-@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-f @var{fmt}] [-t @var{cache}] [-d] @var{filename}
 
 Commit the changes recorded in @var{filename} in its base image or backing 
file.
 If the backing file is smaller than the snapshot, then the backing file will be
@@ -149,6 +149,10 @@ the backing file, the backing file will not be truncated.  
If you want the
 backing file to match the size of the smaller snapshot, you can safely truncate
 it yourself once the commit operation successfully completes.
 
+The image @var{filename} is emptied after the operation has succeeded. If you 
do
+not need @var{filename} afterwards and intend to drop it, you may skip emptying
+@var{filename} by specifying the @code{-d} flag.
+
 @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} 
@var{filename2}
 
 Check if two images have the same content. You can compare images with
-- 
1.9.2

[Qemu-devel] [PATCH v7 07/14] qemu-img: Implement commit like QMP

[Max Reitz]

qemu-img should use QMP commands whenever possible in order to ensure
feature completeness of both online and offline image operations. As
qemu-img itself has no access to QMP (since this would basically require
just everything being linked into qemu-img), imitate QMP's
implementation of block-commit by using commit_active_start() and then
waiting for the block job to finish.

Signed-off-by: Max Reitz 
---
 block/Makefile.objs |  2 +-
 qemu-img.c  | 83 +
 2 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index fd88c03..2c37e80 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -9,6 +9,7 @@ block-obj-y += snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+block-obj-y += mirror.o
 
 ifeq ($(CONFIG_POSIX),y)
 block-obj-y += nbd.o nbd-client.o sheepdog.o
@@ -22,7 +23,6 @@ endif
 
 common-obj-y += stream.o
 common-obj-y += commit.o
-common-obj-y += mirror.o
 common-obj-y += backup.o
 
 iscsi.o-cflags := $(LIBISCSI_CFLAGS)
diff --git a/qemu-img.c b/qemu-img.c
index 96f4463..75654b8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -30,6 +30,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/sysemu.h"
 #include "block/block_int.h"
+#include "block/blockjob.h"
 #include "block/qapi.h"
 #include 
 
@@ -701,12 +702,46 @@ fail:
 return ret;
 }
 
+typedef struct CommonBlockJobCBInfo {
+BlockDriverState *bs;
+Error **errp;
+} CommonBlockJobCBInfo;
+
+static void common_block_job_cb(void *opaque, int ret)
+{
+CommonBlockJobCBInfo *cbi = opaque;
+
+if (ret < 0) {
+error_setg_errno(cbi->errp, -ret, "Block job failed");
+}
+
+/* Drop this block job's reference */
+bdrv_unref(cbi->bs);
+}
+
+static void run_block_job(BlockJob *job, Error **errp)
+{
+AioContext *aio_context = bdrv_get_aio_context(job->bs);
+
+do {
+aio_poll(aio_context, true);
+
+if (!job->busy && !job->ready) {
+block_job_resume(job);
+}
+} while (!job->ready);
+
+block_job_complete_sync(job, errp);
+}
+
 static int img_commit(int argc, char **argv)
 {
 int c, ret, flags;
 const char *filename, *fmt, *cache;
-BlockDriverState *bs;
+BlockDriverState *bs, *base_bs;
 bool quiet = false;
+Error *local_err = NULL;
+CommonBlockJobCBInfo cbi;
 
 fmt = NULL;
 cache = BDRV_DEFAULT_CACHE;
@@ -747,29 +782,39 @@ static int img_commit(int argc, char **argv)
 if (!bs) {
 return 1;
 }
-ret = bdrv_commit(bs);
-switch(ret) {
-case 0:
-qprintf(quiet, "Image committed.\n");
-break;
-case -ENOENT:
-error_report("No disk inserted");
-break;
-case -EACCES:
-error_report("Image is read-only");
-break;
-case -ENOTSUP:
-error_report("Image is already committed");
-break;
-default:
-error_report("Error while committing image");
-break;
+
+/* This is different from QMP, which by default uses the deepest file in 
the
+ * backing chain (i.e., the very base); however, the traditional behavior 
of
+ * qemu-img commit is using the immediate backing file. */
+base_bs = bs->backing_hd;
+if (!base_bs) {
+error_set(&local_err, QERR_BASE_NOT_FOUND, "NULL");
+goto done;
+}
+
+cbi = (CommonBlockJobCBInfo){
+.errp = &local_err,
+.bs   = bs,
+};
+
+commit_active_start(bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
+common_block_job_cb, &cbi, &local_err);
+if (local_err) {
+goto done;
 }
 
+run_block_job(bs->job, &local_err);
+
+done:
 bdrv_unref(bs);
-if (ret) {
+
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
 return 1;
 }
+
+qprintf(quiet, "Image committed.\n");
 return 0;
 }
 
-- 
1.9.2

[Qemu-devel] [PATCH v7 11/14] iotests: Add _filter_qemu_img_map

[Max Reitz]

As different image formats most probably map guest addresses to
different host addresses, add a filter to filter the host addresses out;
also, the image filename should be filtered.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.filter | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 776985d..78ea9e5 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -169,5 +169,12 @@ _filter_qmp()
 -e 's#^{"QMP":.*}$#QMP_VERSION#'
 }
 
+# filter out offsets and file names from qemu-img map
+_filter_qemu_img_map()
+{
+sed -e 's/\([0-9a-fx]* *[0-9a-fx]* *\)[0-9a-fx]* */\1/g' \
+-e 's/Mapped to *//' | _filter_testdir | _filter_imgfmt
+}
+
 # make sure this script returns success
 /bin/true
-- 
1.9.2

[Qemu-devel] [PATCH v7 10/14] qemu-img: Specify backing file for commit

[Max Reitz]

Introduce a new parameter for qemu-img commit which may be used to
explicitly specify the backing file into which an image should be
committed if the backing chain has more than a single layer.

Signed-off-by: Max Reitz 
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.c   | 24 +---
 qemu-img.texi|  9 -
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index ea41d4f..4331949 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
 ETEXI
 
 DEF("commit", img_commit,
-"commit [-q] [-f fmt] [-t cache] [-d] [-p] filename")
+"commit [-q] [-f fmt] [-t cache] [-b base] [-d] [-p] filename")
 STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-d] [-p] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] 
@var{filename}
 ETEXI
 
 DEF("compare", img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index c624d9c..d34609d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -743,7 +743,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 static int img_commit(int argc, char **argv)
 {
 int c, ret, flags;
-const char *filename, *fmt, *cache;
+const char *filename, *fmt, *cache, *base;
 BlockDriverState *bs, *base_bs;
 bool progress = false, quiet = false, drop = false;
 Error *local_err = NULL;
@@ -751,8 +751,9 @@ static int img_commit(int argc, char **argv)
 
 fmt = NULL;
 cache = BDRV_DEFAULT_CACHE;
+base = NULL;
 for(;;) {
-c = getopt(argc, argv, "f:ht:dpq");
+c = getopt(argc, argv, "f:ht:b:dpq");
 if (c == -1) {
 break;
 }
@@ -767,6 +768,11 @@ static int img_commit(int argc, char **argv)
 case 't':
 cache = optarg;
 break;
+case 'b':
+base = optarg;
+/* -b implies -d */
+drop = true;
+break;
 case 'd':
 drop = true;
 break;
@@ -804,12 +810,16 @@ static int img_commit(int argc, char **argv)
 qemu_progress_init(progress, 1.f);
 qemu_progress_print(0.f, 100);
 
-/* This is different from QMP, which by default uses the deepest file in 
the
- * backing chain (i.e., the very base); however, the traditional behavior 
of
- * qemu-img commit is using the immediate backing file. */
-base_bs = bs->backing_hd;
+if (base) {
+base_bs = bdrv_find_backing_image(bs, base);
+} else {
+/* This is different from QMP, which by default uses the deepest file 
in
+ * the backing chain (i.e., the very base); however, the traditional
+ * behavior of qemu-img commit is using the immediate backing file. */
+base_bs = bs->backing_hd;
+}
 if (!base_bs) {
-error_set(&local_err, QERR_BASE_NOT_FOUND, "NULL");
+error_set(&local_err, QERR_BASE_NOT_FOUND, base ?: "NULL");
 goto done;
 }
 
diff --git a/qemu-img.texi b/qemu-img.texi
index c60654f..f268781 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -140,7 +140,7 @@ this case. @var{backing_file} will never be modified unless 
you use the
 The size can also be specified using the @var{size} option with @code{-o},
 it doesn't need to be specified separately in this case.
 
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-d] [-p] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{base}] [-d] [-p] 
@var{filename}
 
 Commit the changes recorded in @var{filename} in its base image or backing 
file.
 If the backing file is smaller than the snapshot, then the backing file will be
@@ -153,6 +153,13 @@ The image @var{filename} is emptied after the operation 
has succeeded. If you do
 not need @var{filename} afterwards and intend to drop it, you may skip emptying
 @var{filename} by specifying the @code{-d} flag.
 
+If the backing chain of the given image file @var{filename} has more than one
+layer, the backing file into which the changes will be committed may be
+specified as @var{base} (which has to be part of @var{filename}'s backing
+chain). If @var{base} is not specified, the immediate backing file of the top
+image (which is @var{filename}) will be used. For reasons of consistency,
+explicitly specifying @var{base} will always imply @code{-d}.
+
 @item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1} 
@var{filename2}
 
 Check if two images have the same content. You can compare images with
-- 
1.9.2

[Qemu-devel] [PATCH v7 05/14] blockjob: Add "ready" field

[Max Reitz]

When a block job signals readiness, this is currently reported only
through QMP. If qemu wants to use block jobs for internal tasks, there
needs to be another way to correctly detect when a block job may be
completed.

For this reason, introduce a bool "ready" which is set when the block
job may be completed.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Kevin Wolf 
---
 blockjob.c   | 7 ++-
 include/block/blockjob.h | 5 +
 qapi-schema.json | 4 +++-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 9c7b198..a4271c1 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -242,6 +242,7 @@ BlockJobInfo *block_job_query(BlockJob *job)
 info->offset= job->offset;
 info->speed = job->speed;
 info->io_status = job->iostatus;
+info->ready = job->ready;
 return info;
 }
 
@@ -270,7 +271,11 @@ QObject *qobject_from_block_job(BlockJob *job)
 
 void block_job_ready(BlockJob *job)
 {
-QObject *data = qobject_from_block_job(job);
+QObject *data;
+
+job->ready = true;
+
+data = qobject_from_block_job(job);
 monitor_protocol_event(QEVENT_BLOCK_JOB_READY, data);
 qobject_decref(data);
 }
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 626ea42..4a920e7 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -91,6 +91,11 @@ struct BlockJob {
  */
 bool busy;
 
+/**
+ * Set to true when the job is ready to be completed.
+ */
+bool ready;
+
 /** Status that is published by the query-block-jobs QMP API */
 BlockDeviceIoStatus iostatus;
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 36cb964..c9b9f09 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1578,12 +1578,14 @@
 #
 # @io-status: the status of the job (since 1.3)
 #
+# @ready: true if the job may be completed (since 2.1)
+#
 # Since: 1.1
 ##
 { 'type': 'BlockJobInfo',
   'data': {'type': 'str', 'device': 'str', 'len': 'int',
'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
-   'io-status': 'BlockDeviceIoStatus'} }
+   'io-status': 'BlockDeviceIoStatus', 'ready': 'bool'} }
 
 ##
 # @query-block-jobs:
-- 
1.9.2

[Qemu-devel] [PATCH v7 03/14] qcow2: Optimize bdrv_make_empty()

[Max Reitz]

bdrv_make_empty() is currently only called if the current image
represents an external snapshot that has been committed to its base
image; it is therefore unlikely to have internal snapshots. In this
case, bdrv_make_empty() can be greatly sped up by creating an empty L1
table and dropping all data clusters at once by recreating the refcount
structure accordingly instead of normally discarding all clusters.

If there are snapshots, fall back to the simple implementation (discard
all clusters).

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block/qcow2.c | 389 +++---
 1 file changed, 374 insertions(+), 15 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index fb427d1..ceea94e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2008,27 +2008,386 @@ fail:
 return ret;
 }
 
+/*
+ * Creates a reftable pointing to refblocks following right afterwards and an
+ * empty L1 table at the given @offset. @refblocks is the number of refblocks
+ * to create.
+ *
+ * This combination of structures (reftable+refblocks+L1) is here called a
+ * "blob".
+ */
+static int create_refcount_l1(BlockDriverState *bs, uint64_t offset,
+  uint64_t refblocks)
+{
+BDRVQcowState *s = bs->opaque;
+uint64_t *reftable = NULL;
+uint16_t *refblock = NULL;
+uint64_t reftable_clusters;
+uint64_t rbi;
+uint64_t blob_start, blob_end;
+uint64_t l2_tables, l1_clusters, l1_size2;
+uint8_t l1_size_and_offset[12];
+uint64_t rt_offset;
+int ret, i;
+
+reftable_clusters = DIV_ROUND_UP(refblocks, s->cluster_size / 8);
+l2_tables = DIV_ROUND_UP(bs->total_sectors / s->cluster_sectors,
+ s->cluster_size / 8);
+l1_clusters = DIV_ROUND_UP(l2_tables, s->cluster_size / 8);
+l1_size2 = l1_clusters << s->cluster_bits;
+
+blob_start = offset;
+blob_end = offset + ((reftable_clusters + refblocks + l1_clusters) <<
+s->cluster_bits);
+
+ret = qcow2_pre_write_overlap_check(bs, 0, blob_start,
+blob_end - blob_start);
+if (ret < 0) {
+return ret;
+}
+
+/* Create the reftable pointing with entries pointing consecutively to the
+ * following clusters */
+reftable = g_malloc0_n(reftable_clusters, s->cluster_size);
+
+for (rbi = 0; rbi < refblocks; rbi++) {
+reftable[rbi] = cpu_to_be64(offset + ((reftable_clusters + rbi) <<
+s->cluster_bits));
+}
+
+ret = bdrv_write(bs->file, offset >> BDRV_SECTOR_BITS, (uint8_t *)reftable,
+ reftable_clusters * s->cluster_sectors);
+if (ret < 0) {
+goto out;
+}
+
+offset += reftable_clusters << s->cluster_bits;
+
+/* Keep the reftable, as we will need it for the BDRVQcowState anyway */
+
+/* Now, create all the refblocks */
+refblock = g_malloc(s->cluster_size);
+
+for (rbi = 0; rbi < refblocks; rbi++) {
+for (i = 0; i < s->cluster_size / 2; i++) {
+uint64_t cluster_offset = ((rbi << (s->cluster_bits - 1)) + i)
+  << s->cluster_bits;
+
+/* Only 0 and 1 are possible as refcounts here */
+refblock[i] = cpu_to_be16(!cluster_offset ||
+  (cluster_offset >= blob_start &&
+   cluster_offset <  blob_end));
+}
+
+ret = bdrv_write(bs->file, offset >> BDRV_SECTOR_BITS,
+ (uint8_t *)refblock, s->cluster_sectors);
+if (ret < 0) {
+goto out;
+}
+
+offset += s->cluster_size;
+}
+
+g_free(refblock);
+refblock = NULL;
+
+/* The L1 table is very simple */
+ret = bdrv_write_zeroes(bs->file, offset >> BDRV_SECTOR_BITS,
+l1_clusters * s->cluster_sectors,
+BDRV_REQ_MAY_UNMAP);
+if (ret < 0) {
+goto out;
+}
+
+/* Now make sure all changes are stable and clear all caches */
+bdrv_flush(bs);
+
+/* This is probably not really necessary, but it cannot hurt, either */
+ret = qcow2_mark_clean(bs);
+if (ret < 0) {
+goto out;
+}
+
+ret = qcow2_cache_empty(bs, s->l2_table_cache);
+if (ret < 0) {
+goto out;
+}
+
+ret = qcow2_cache_empty(bs, s->refcount_block_cache);
+if (ret < 0) {
+goto out;
+}
+
+/* Modify the image header to point to the new blank L1 table. This will
+ * leak all currently existing data clusters, which is fine. */
+BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
+
+assert(l1_size2 / 8 <= UINT32_MAX);
+cpu_to_be32w((uint32_t *)&l1_size_and_offset[0], l1_size2 / 8);
+cpu_to_be64w((uint64_t *)&l1_size_and_offset[4], offset);
+ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, l1_size),
+   l1_size_and_offset, sizeof(l1_size_and_offset));
+if (re

[Qemu-devel] [PULL 13/38] pci-assign: accept Error from pci_add_capability2()

[Luiz Capitulino]

From: Laszlo Ersek 

Propagate any errors while adding PCI capabilities to
assigned_device_pci_cap_init(). We'll continue the propagation upwards
when assigned_device_pci_cap_init() becomes a leaf itself (when none of
its callees will report errors internally any longer when detecting and
returning them).

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 35 ---
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index b4696aa..f91d4fb 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1263,8 +1263,11 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 }
 dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
 /* Only 32-bit/no-mask currently supported */
-ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10);
+ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10,
+  &local_err);
 if (ret < 0) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return ret;
 }
 pci_dev->msi_cap = pos;
@@ -1294,8 +1297,11 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 return -ENOTSUP;
 }
 dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
-ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12);
+ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12,
+  &local_err);
 if (ret < 0) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return ret;
 }
 pci_dev->msix_cap = pos;
@@ -1322,8 +1328,11 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 if (pos) {
 uint16_t pmc;
 
-ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF);
+ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF,
+  &local_err);
 if (ret < 0) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return ret;
 }
 
@@ -1388,8 +1397,11 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 return -EINVAL;
 }
 
-ret = pci_add_capability(pci_dev, PCI_CAP_ID_EXP, pos, size);
+ret = pci_add_capability2(pci_dev, PCI_CAP_ID_EXP, pos, size,
+  &local_err);
 if (ret < 0) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return ret;
 }
 
@@ -1462,8 +1474,11 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 uint32_t status;
 
 /* Only expose the minimum, 8 byte capability */
-ret = pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8);
+ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PCIX, pos, 8,
+  &local_err);
 if (ret < 0) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return ret;
 }
 
@@ -1488,8 +1503,11 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VPD, 0);
 if (pos) {
 /* Direct R/W passthrough */
-ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8);
+ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VPD, pos, 8,
+  &local_err);
 if (ret < 0) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return ret;
 }
 
@@ -1504,8 +1522,11 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 pos += PCI_CAP_LIST_NEXT) {
 uint8_t len = pci_get_byte(pci_dev->config + pos + PCI_CAP_FLAGS);
 /* Direct R/W passthrough */
-ret = pci_add_capability(pci_dev, PCI_CAP_ID_VNDR, pos, len);
+ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VNDR, pos, len,
+  &local_err);
 if (ret < 0) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return ret;
 }
 
-- 
1.9.0

[Qemu-devel] [PATCH 4] block/raw-posix: Try both FIEMAP and SEEK_HOLE

[Max Reitz]

The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if
FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even
compiled in in this case. However, there may be implementations which
support the latter but not the former (e.g., NFSv4.2) as well as vice
versa.

To cover both cases, try FIEMAP first (as this will return -ENOTSUP if
not supported instead of returning a failsafe value (everything
allocated as a single extent)) and if that does not work, fall back to
SEEK_HOLE/SEEK_DATA.

Signed-off-by: Max Reitz 
---
v4:
 - If the order of FIEMAP and SEEK_HOLE are reversed (that is, kept the
   way they are now), the tristates from v3 are not required anymore, as
   FIEMAP can never be in an “unknown” state and the worst that
   SEEK_HOLE can do is to report everything as allocated (which is the
   final fallback anyway). This patch is thus basically the same as v2,
   only with the order of FIEMAP and SEEK_HOLE reversed (first FIEMAP,
   then SEEK_HOLE). [Paolo]
---
 block/raw-posix.c | 127 +-
 1 file changed, 77 insertions(+), 50 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 3ce026d..6586a0c 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -146,6 +146,9 @@ typedef struct BDRVRawState {
 bool has_discard:1;
 bool has_write_zeroes:1;
 bool discard_zeroes:1;
+#ifdef CONFIG_FIEMAP
+bool skip_fiemap;
+#endif
 } BDRVRawState;
 
 typedef struct BDRVRawReopenState {
@@ -1272,53 +1275,29 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options,
 return result;
 }
 
-/*
- * Returns true iff the specified sector is present in the disk image. Drivers
- * not implementing the functionality are assumed to not support backing files,
- * hence all their sectors are reported as allocated.
- *
- * If 'sector_num' is beyond the end of the disk image the return value is 0
- * and 'pnum' is set to 0.
- *
- * 'pnum' is set to the number of sectors (including and immediately following
- * the specified sector) that are known to be in the same
- * allocated/unallocated state.
- *
- * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
- * beyond the end of the disk image it will be clamped.
- */
-static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
-int64_t sector_num,
-int nb_sectors, int *pnum)
+static int64_t try_fiemap(BlockDriverState *bs, off_t start, off_t *data,
+  off_t *hole, int nb_sectors, int *pnum)
 {
-off_t start, data, hole;
-int64_t ret;
-
-ret = fd_open(bs);
-if (ret < 0) {
-return ret;
-}
-
-start = sector_num * BDRV_SECTOR_SIZE;
-ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
-
 #ifdef CONFIG_FIEMAP
-
 BDRVRawState *s = bs->opaque;
+int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
 struct {
 struct fiemap fm;
 struct fiemap_extent fe;
 } f;
 
+if (s->skip_fiemap) {
+return -ENOTSUP;
+}
+
 f.fm.fm_start = start;
 f.fm.fm_length = (int64_t)nb_sectors * BDRV_SECTOR_SIZE;
 f.fm.fm_flags = 0;
 f.fm.fm_extent_count = 1;
 f.fm.fm_reserved = 0;
 if (ioctl(s->fd, FS_IOC_FIEMAP, &f) == -1) {
-/* Assume everything is allocated.  */
-*pnum = nb_sectors;
-return ret;
+s->skip_fiemap = true;
+return -errno;
 }
 
 if (f.fm.fm_mapped_extents == 0) {
@@ -1326,44 +1305,92 @@ static int64_t coroutine_fn 
raw_co_get_block_status(BlockDriverState *bs,
  * f.fm.fm_start + f.fm.fm_length must be clamped to the file size!
  */
 off_t length = lseek(s->fd, 0, SEEK_END);
-hole = f.fm.fm_start;
-data = MIN(f.fm.fm_start + f.fm.fm_length, length);
+*hole = f.fm.fm_start;
+*data = MIN(f.fm.fm_start + f.fm.fm_length, length);
 } else {
-data = f.fe.fe_logical;
-hole = f.fe.fe_logical + f.fe.fe_length;
+*data = f.fe.fe_logical;
+*hole = f.fe.fe_logical + f.fe.fe_length;
 if (f.fe.fe_flags & FIEMAP_EXTENT_UNWRITTEN) {
 ret |= BDRV_BLOCK_ZERO;
 }
 }
 
-#elif defined SEEK_HOLE && defined SEEK_DATA
+return ret;
+#else
+return -ENOTSUP;
+#endif
+}
 
+static int64_t try_seek_hole(BlockDriverState *bs, off_t start, off_t *data,
+ off_t *hole, int *pnum)
+{
+#if defined SEEK_HOLE && defined SEEK_DATA
 BDRVRawState *s = bs->opaque;
 
-hole = lseek(s->fd, start, SEEK_HOLE);
-if (hole == -1) {
+*hole = lseek(s->fd, start, SEEK_HOLE);
+if (*hole == -1) {
 /* -ENXIO indicates that sector_num was past the end of the file.
  * There is a virtual hole there.  */
 assert(errno != -ENXIO);
 
-/* Most likely EINVAL.  Assume everything is allocated.  */
-*pnum = nb_secto

[Qemu-devel] [PULL 36/38] qmp: use valid JSON in transaction example

[Luiz Capitulino]

From: Eric Blake 

Our example should use the correct quotes to match what someone
could actually pass over the wire.

* qmp-commands.hx: Use correct JSON quotes.

Signed-off-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 qmp-commands.hx | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index f437937..cae890e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1165,19 +1165,19 @@ Example:
 
 -> { "execute": "transaction",
  "arguments": { "actions": [
- { 'type': 'blockdev-snapshot-sync', 'data' : { "device": "ide-hd0",
+ { "type": "blockdev-snapshot-sync", "data" : { "device": "ide-hd0",
  "snapshot-file": 
"/some/place/my-image",
  "format": "qcow2" } },
- { 'type': 'blockdev-snapshot-sync', 'data' : { "node-name": "myfile",
+ { "type": "blockdev-snapshot-sync", "data" : { "node-name": "myfile",
  "snapshot-file": 
"/some/place/my-image2",
  "snapshot-node-name": "node3432",
  "mode": "existing",
  "format": "qcow2" } },
- { 'type': 'blockdev-snapshot-sync', 'data' : { "device": "ide-hd1",
+ { "type": "blockdev-snapshot-sync", "data" : { "device": "ide-hd1",
  "snapshot-file": 
"/some/place/my-image2",
  "mode": "existing",
  "format": "qcow2" } },
- { 'type': 'blockdev-snapshot-internal-sync', 'data' : {
+ { "type": "blockdev-snapshot-internal-sync", "data" : {
  "device": "ide-hd2",
  "name": "snapshot0" } } ] } }
 <- { "return": {} }
-- 
1.9.0

[Qemu-devel] [PULL 34/38] dump: Drop pointless error_is_set(), DumpState member errp

[Luiz Capitulino]

From: Markus Armbruster 

In qmp_dump_guest_memory(), the error must be clear on entry, and we
always bail out after setting it, directly or via dump_init().
Therefore, both error_is_set() are always false.  Drop them.

DumpState member errp is now write-only.  Drop it, too.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 dump.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/dump.c b/dump.c
index 14b3d1d..e56b7cf 100644
--- a/dump.c
+++ b/dump.c
@@ -86,7 +86,6 @@ typedef struct DumpState {
 bool has_filter;
 int64_t begin;
 int64_t length;
-Error **errp;
 
 uint8_t *note_buf;  /* buffer for notes */
 size_t note_buf_offset; /* the writing place in note_buf */
@@ -1570,7 +1569,6 @@ static int dump_init(DumpState *s, int fd, bool 
has_format,
 nr_cpus++;
 }
 
-s->errp = errp;
 s->fd = fd;
 s->has_filter = has_filter;
 s->begin = begin;
@@ -1780,11 +1778,11 @@ void qmp_dump_guest_memory(bool paging, const char 
*file, bool has_begin,
 }
 
 if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) {
-if (create_kdump_vmcore(s) < 0 && !error_is_set(s->errp)) {
+if (create_kdump_vmcore(s) < 0) {
 error_set(errp, QERR_IO_ERROR);
 }
 } else {
-if (create_vmcore(s) < 0 && !error_is_set(s->errp)) {
+if (create_vmcore(s) < 0) {
 error_set(errp, QERR_IO_ERROR);
 }
 }
-- 
1.9.0

[Qemu-devel] [PULL 30/38] qapi: Clean up fragile use of error_is_set()

[Luiz Capitulino]

From: Markus Armbruster 

Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque.  It's wrong when ERRP
may be null, because errors go undetected when it is.  It's fragile
when proving ERRP non-null involves a non-local argument.  Else, it's
unnecessarily opaque (see commit 84d18f0).

The error_is_set(errp) in do_qmp_dispatch() is merely fragile, because
the caller never passes a null errp argument.

Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Michael Roth 
Signed-off-by: Luiz Capitulino 
---
 qapi/qmp-dispatch.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 187af56..168b083 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -62,6 +62,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, 
Error **errp)
 
 static QObject *do_qmp_dispatch(QObject *request, Error **errp)
 {
+Error *local_err = NULL;
 const char *command;
 QDict *args, *dict;
 QmpCommand *cmd;
@@ -93,13 +94,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error 
**errp)
 
 switch (cmd->type) {
 case QCT_NORMAL:
-cmd->fn(args, &ret, errp);
-if (!error_is_set(errp)) {
-if (cmd->options & QCO_NO_SUCCESS_RESP) {
-g_assert(!ret);
-} else if (!ret) {
-ret = QOBJECT(qdict_new());
-}
+cmd->fn(args, &ret, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+} else if (cmd->options & QCO_NO_SUCCESS_RESP) {
+g_assert(!ret);
+} else if (!ret) {
+ret = QOBJECT(qdict_new());
 }
 break;
 }
-- 
1.9.0

[Qemu-devel] [PULL 28/38] qapi: Drop redundant, unclean error_is_set()

[Luiz Capitulino]

From: Markus Armbruster 

do_qmp_dispatch()'s test for qmp_dispatch_check_obj() failure examines
both the return value and the error object.  The latter part is
unclean; it works only when do_qmp_dispatch()'s caller passes a
non-null errp argument.  That's the case, but it's not locally
obvious.  Unclean.

Cleanup would be easy enough, but since the unclean code is also
redundant, let's just drop it.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Michael Roth 
Signed-off-by: Luiz Capitulino 
---
 qapi/qmp-dispatch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 141a376..187af56 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -67,9 +67,8 @@ static QObject *do_qmp_dispatch(QObject *request, Error 
**errp)
 QmpCommand *cmd;
 QObject *ret = NULL;
 
-
 dict = qmp_dispatch_check_obj(request, errp);
-if (!dict || error_is_set(errp)) {
+if (!dict) {
 return NULL;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 26/38] qga: Use return values instead of error_is_set(errp)

[Luiz Capitulino]

From: Markus Armbruster 

Using error_is_set(errp) to check whether a function call failed is
fragile: it breaks when errp is null.  ga_get_fd_handle() and
guest_file_handle_add() don't return a useful value when they fail,
but that's just stupid.  Fix that, and check them instead.  As far
as I can tell, errp can't be null there, but this is more robust and
more obviously correct.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Michael Roth 
Signed-off-by: Luiz Capitulino 
---
 qga/commands-posix.c | 6 +++---
 qga/main.c   | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index f6af7d1..6af974f 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -223,8 +223,8 @@ static int64_t guest_file_handle_add(FILE *fh, Error **errp)
 int64_t handle;
 
 handle = ga_get_fd_handle(ga_state, errp);
-if (error_is_set(errp)) {
-return 0;
+if (handle < 0) {
+return -1;
 }
 
 gfh = g_malloc0(sizeof(GuestFileHandle));
@@ -407,7 +407,7 @@ int64_t qmp_guest_file_open(const char *path, bool 
has_mode, const char *mode,
 }
 
 handle = guest_file_handle_add(fh, errp);
-if (error_is_set(errp)) {
+if (handle < 0) {
 fclose(fh);
 return -1;
 }
diff --git a/qga/main.c b/qga/main.c
index 38219c9..8b927c9 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -910,6 +910,7 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
 
 if (!write_persistent_state(&s->pstate, s->pstate_filepath)) {
 error_setg(errp, "failed to commit persistent state to disk");
+return -1;
 }
 
 return handle;
-- 
1.9.0

[Qemu-devel] [PULL 27/38] hmp: Guard against misuse of hmp_handle_error()

[Luiz Capitulino]

From: Markus Armbruster 

Null errp argument makes no sense.  Assert it's not null, to make this
explicit, and guard against misuse.  All current callers pass non-null
errp.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hmp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index 9469163..5c4d612 100644
--- a/hmp.c
+++ b/hmp.c
@@ -28,7 +28,8 @@
 
 static void hmp_handle_error(Monitor *mon, Error **errp)
 {
-if (error_is_set(errp)) {
+assert(errp);
+if (*errp) {
 monitor_printf(mon, "%s\n", error_get_pretty(*errp));
 error_free(*errp);
 }
-- 
1.9.0

[Qemu-devel] [PULL 24/38] qmp: Consistently name Error ** objects errp, and not err

[Luiz Capitulino]

From: Markus Armbruster 

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 qmp.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/qmp.c b/qmp.c
index 233325d..82acb89 100644
--- a/qmp.c
+++ b/qmp.c
@@ -41,7 +41,7 @@ NameInfo *qmp_query_name(Error **errp)
 return info;
 }
 
-VersionInfo *qmp_query_version(Error **err)
+VersionInfo *qmp_query_version(Error **errp)
 {
 VersionInfo *info = g_malloc0(sizeof(*info));
 const char *version = QEMU_VERSION;
@@ -82,7 +82,7 @@ UuidInfo *qmp_query_uuid(Error **errp)
 return info;
 }
 
-void qmp_quit(Error **err)
+void qmp_quit(Error **errp)
 {
 no_shutdown = 0;
 qemu_system_shutdown_request();
@@ -153,10 +153,10 @@ static void iostatus_bdrv_it(void *opaque, 
BlockDriverState *bs)
 
 static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
 {
-Error **err = opaque;
+Error **errp = opaque;
 
-if (!error_is_set(err) && bdrv_key_required(bs)) {
-error_set(err, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
+if (!error_is_set(errp) && bdrv_key_required(bs)) {
+error_set(errp, QERR_DEVICE_ENCRYPTED, bdrv_get_device_name(bs),
   bdrv_get_encrypted_filename(bs));
 }
 }
@@ -405,12 +405,12 @@ static void qmp_change_vnc(const char *target, bool 
has_arg, const char *arg,
 #endif /* !CONFIG_VNC */
 
 void qmp_change(const char *device, const char *target,
-bool has_arg, const char *arg, Error **err)
+bool has_arg, const char *arg, Error **errp)
 {
 if (strcmp(device, "vnc") == 0) {
-qmp_change_vnc(target, has_arg, arg, err);
+qmp_change_vnc(target, has_arg, arg, errp);
 } else {
-qmp_change_blockdev(device, target, arg, err);
+qmp_change_blockdev(device, target, arg, errp);
 }
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 22/38] qmp hmp: Consistently name Error * objects err, and not errp

[Luiz Capitulino]

From: Markus Armbruster 

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 docs/writing-qmp-commands.txt  |  28 
 hmp.c  | 138 ++---
 include/qapi/qmp/dispatch.h|   2 +-
 qapi/qmp-dispatch.c|   6 +-
 tests/test-qmp-input-strict.c  |  72 +--
 tests/test-qmp-input-visitor.c |  64 -
 tests/test-qmp-output-visitor.c|  74 ++--
 tests/test-string-input-visitor.c  |  50 +++---
 tests/test-string-output-visitor.c |  46 ++---
 9 files changed, 240 insertions(+), 240 deletions(-)

diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 3930a9b..4d86c24 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -308,12 +308,12 @@ Here's the implementation of the "hello-world" HMP 
command:
 void hmp_hello_world(Monitor *mon, const QDict *qdict)
 {
 const char *message = qdict_get_try_str(qdict, "message");
-Error *errp = NULL;
+Error *err = NULL;
 
-qmp_hello_world(!!message, message, &errp);
-if (errp) {
-monitor_printf(mon, "%s\n", error_get_pretty(errp));
-error_free(errp);
+qmp_hello_world(!!message, message, &err);
+if (err) {
+monitor_printf(mon, "%s\n", error_get_pretty(err));
+error_free(err);
 return;
 }
 }
@@ -328,7 +328,7 @@ There are three important points to be noticed:
 2. hmp_hello_world() performs error checking. In this example we just print
the error description to the user, but we could do more, like taking
different actions depending on the error qmp_hello_world() returns
-3. The "errp" variable must be initialized to NULL before performing the
+3. The "err" variable must be initialized to NULL before performing the
QMP call
 
 There's one last step to actually make the command available to monitor users,
@@ -480,12 +480,12 @@ Here's the HMP counterpart of the query-alarm-clock 
command:
 void hmp_info_alarm_clock(Monitor *mon)
 {
 QemuAlarmClock *clock;
-Error *errp = NULL;
+Error *err = NULL;
 
-clock = qmp_query_alarm_clock(&errp);
-if (errp) {
+clock = qmp_query_alarm_clock(&err);
+if (err) {
 monitor_printf(mon, "Could not query alarm clock information\n");
-error_free(errp);
+error_free(err);
 return;
 }
 
@@ -631,12 +631,12 @@ has to traverse the list, it's shown below for reference:
 void hmp_info_alarm_methods(Monitor *mon)
 {
 TimerAlarmMethodList *method_list, *method;
-Error *errp = NULL;
+Error *err = NULL;
 
-method_list = qmp_query_alarm_methods(&errp);
-if (errp) {
+method_list = qmp_query_alarm_methods(&err);
+if (err) {
 monitor_printf(mon, "Could not query alarm methods\n");
-error_free(errp);
+error_free(err);
 return;
 }
 
diff --git a/hmp.c b/hmp.c
index 903e0a1..9469163 100644
--- a/hmp.c
+++ b/hmp.c
@@ -754,10 +754,10 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
 uint32_t size = qdict_get_int(qdict, "size");
 const char *filename = qdict_get_str(qdict, "filename");
 uint64_t addr = qdict_get_int(qdict, "val");
-Error *errp = NULL;
+Error *err = NULL;
 
-qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &errp);
-hmp_handle_error(mon, &errp);
+qmp_memsave(addr, size, filename, true, monitor_get_cpu_index(), &err);
+hmp_handle_error(mon, &err);
 }
 
 void hmp_pmemsave(Monitor *mon, const QDict *qdict)
@@ -765,21 +765,21 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
 uint32_t size = qdict_get_int(qdict, "size");
 const char *filename = qdict_get_str(qdict, "filename");
 uint64_t addr = qdict_get_int(qdict, "val");
-Error *errp = NULL;
+Error *err = NULL;
 
-qmp_pmemsave(addr, size, filename, &errp);
-hmp_handle_error(mon, &errp);
+qmp_pmemsave(addr, size, filename, &err);
+hmp_handle_error(mon, &err);
 }
 
 void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
 {
 const char *chardev = qdict_get_str(qdict, "device");
 const char *data = qdict_get_str(qdict, "data");
-Error *errp = NULL;
+Error *err = NULL;
 
-qmp_ringbuf_write(chardev, data, false, 0, &errp);
+qmp_ringbuf_write(chardev, data, false, 0, &err);
 
-hmp_handle_error(mon, &errp);
+hmp_handle_error(mon, &err);
 }
 
 void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
@@ -787,13 +787,13 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
 uint32_t size = qdict_get_int(qdict, "size");
 const char *chardev = qdict_get_str(qdict, "device");
 char *data;
-Error *errp = NULL;
+Error *err = NULL;
 int i;
 
-data = qmp_ringbuf_read(chardev, size, false, 0, &errp);
-if (errp) {
-monitor_printf(mon, "%s\n", error_get_pretty(errp));
-error_free(errp);
+data =

Re: [Qemu-devel] [PATCH 4] block/raw-posix: Try both FIEMAP and SEEK_HOLE

[Max Reitz]

Oops, somehow dropped the “v” from “v4” in the subject. I hope you're 
able to infer the meaning anyway. ;-)


Max

[Qemu-devel] [PULL 17/38] pci-assign: propagate errors from assigned_dev_register_msix_mmio()

[Luiz Capitulino]

From: Laszlo Ersek 

The return type is also changed from "int" to "void", because it was used
in a success vs. failure sense only (the caller didn't distinguish error
codes from each other, and even assigned_dev_register_msix_mmio() masked
mmap()'s errno values with a common -EFAULT).

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 2de6559..3a904e8 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1644,20 +1644,19 @@ static void assigned_dev_msix_reset(AssignedDevice *dev)
 }
 }
 
-static int assigned_dev_register_msix_mmio(AssignedDevice *dev)
+static void assigned_dev_register_msix_mmio(AssignedDevice *dev, Error **errp)
 {
 dev->msix_table = mmap(NULL, MSIX_PAGE_SIZE, PROT_READ|PROT_WRITE,
MAP_ANONYMOUS|MAP_PRIVATE, 0, 0);
 if (dev->msix_table == MAP_FAILED) {
-error_report("fail allocate msix_table! %s", strerror(errno));
-return -EFAULT;
+error_setg_errno(errp, errno, "failed to allocate msix_table");
+return;
 }
 
 assigned_dev_msix_reset(dev);
 
 memory_region_init_io(&dev->mmio, OBJECT(dev), &assigned_dev_msix_mmio_ops,
   dev, "assigned-dev-msix", MSIX_PAGE_SIZE);
-return 0;
 }
 
 static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
@@ -1788,7 +1787,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 
 /* intercept MSI-X entry page in the MMIO */
 if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
-if (assigned_dev_register_msix_mmio(dev)) {
+assigned_dev_register_msix_mmio(dev, &local_err);
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
 goto out;
 }
 }
-- 
1.9.0

[Qemu-devel] [PULL 32/38] qga: Drop superfluous error_is_set()

[Luiz Capitulino]

From: Markus Armbruster 

acquire_privilege(), execute_async() and check_suspend_mode() do
nothing when called with an error set.  Callers shouldn't do that, and
no caller does.  Drop the superfluous tests.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Michael Roth 
Signed-off-by: Luiz Capitulino 
---
 qga/commands-win32.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3483c0d..d793dd0 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -35,10 +35,6 @@ static void acquire_privilege(const char *name, Error **errp)
 TOKEN_PRIVILEGES priv;
 Error *local_err = NULL;
 
-if (error_is_set(errp)) {
-return;
-}
-
 if (OpenProcessToken(GetCurrentProcess(),
 TOKEN_ADJUST_PRIVILEGES|TOKEN_QUERY, &token))
 {
@@ -74,9 +70,6 @@ static void execute_async(DWORD WINAPI (*func)(LPVOID), 
LPVOID opaque,
 {
 Error *local_err = NULL;
 
-if (error_is_set(errp)) {
-return;
-}
 HANDLE thread = CreateThread(NULL, 0, func, opaque, 0, NULL);
 if (!thread) {
 error_set(&local_err, QERR_QGA_COMMAND_FAILED,
@@ -268,9 +261,6 @@ static void check_suspend_mode(GuestSuspendMode mode, Error 
**errp)
 SYSTEM_POWER_CAPABILITIES sys_pwr_caps;
 Error *local_err = NULL;
 
-if (error_is_set(errp)) {
-return;
-}
 ZeroMemory(&sys_pwr_caps, sizeof(sys_pwr_caps));
 if (!GetPwrCapabilities(&sys_pwr_caps)) {
 error_set(&local_err, QERR_QGA_COMMAND_FAILED,
-- 
1.9.0

[Qemu-devel] [PULL 20/38] pci-assign: propagate errors from assign_intx()

[Luiz Capitulino]

From: Laszlo Ersek 

Among the callers, only assigned_initfn() should set the  monitor's stored
error. Other callers may run in contexts where the monitor's stored error
makes no sense. For example:

assigned_dev_pci_write_config()
  assigned_dev_update_msix()
assign_intx()

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 39 ---
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 0fedca8..6891729 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -847,7 +847,7 @@ static void verify_irqchip_in_kernel(Error **errp)
 error_setg(errp, "pci-assign requires KVM with in-kernel irqchip enabled");
 }
 
-static int assign_intx(AssignedDevice *dev)
+static int assign_intx(AssignedDevice *dev, Error **errp)
 {
 AssignedIRQType new_type;
 PCIINTxRoute intx_route;
@@ -863,8 +863,7 @@ static int assign_intx(AssignedDevice *dev)
 
 verify_irqchip_in_kernel(&local_err);
 if (local_err) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
+error_propagate(errp, local_err);
 return -ENOTSUP;
 }
 
@@ -927,10 +926,11 @@ retry:
 dev->features |= ASSIGNED_DEVICE_PREFER_MSI_MASK;
 goto retry;
 }
-error_report("Failed to assign irq for \"%s\": %s",
- dev->dev.qdev.id, strerror(-r));
-error_report("Perhaps you are assigning a device "
- "that shares an IRQ with another device?");
+error_setg_errno(errp, -r,
+ "Failed to assign irq for \"%s\"\n"
+ "Perhaps you are assigning a device "
+ "that shares an IRQ with another device?",
+ dev->dev.qdev.id);
 return r;
 }
 
@@ -956,8 +956,11 @@ static void assigned_dev_update_irq_routing(PCIDevice *dev)
 Error *err = NULL;
 int r;
 
-r = assign_intx(assigned_dev);
+r = assign_intx(assigned_dev, &err);
 if (r < 0) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
+err = NULL;
 qdev_unplug(&dev->qdev, &err);
 assert(!err);
 }
@@ -1008,7 +1011,13 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
 assigned_dev->intx_route.irq = -1;
 assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSI;
 } else {
-assign_intx(assigned_dev);
+Error *local_err = NULL;
+
+assign_intx(assigned_dev, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
+}
 }
 }
 
@@ -1150,7 +1159,13 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
 assigned_dev->intx_route.irq = -1;
 assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSIX;
 } else {
-assign_intx(assigned_dev);
+Error *local_err = NULL;
+
+assign_intx(assigned_dev, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
+}
 }
 }
 
@@ -1819,8 +1834,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 }
 
 /* assign legacy INTx to the device */
-r = assign_intx(dev);
+r = assign_intx(dev, &local_err);
 if (r < 0) {
+qerror_report_err(local_err);
+error_free(local_err);
 goto assigned_out;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 16/38] pci-assign: propagate errors from assigned_device_pci_cap_init()

[Luiz Capitulino]

From: Laszlo Ersek 

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 45 +++--
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index c6d1094..2de6559 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1237,7 +1237,7 @@ static void assigned_dev_setup_cap_read(AssignedDevice 
*dev, uint32_t offset,
 assigned_dev_emulate_config_read(dev, offset + PCI_CAP_LIST_NEXT, 1);
 }
 
-static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
+static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
 {
 AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
 PCIRegion *pci_region = dev->real_device.regions;
@@ -1256,8 +1256,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
 verify_irqchip_in_kernel(&local_err);
 if (local_err) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
+error_propagate(errp, local_err);
 return -ENOTSUP;
 }
 dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
@@ -1265,8 +1264,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10,
   &local_err);
 if (ret < 0) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
+error_propagate(errp, local_err);
 return ret;
 }
 pci_dev->msi_cap = pos;
@@ -1291,16 +1289,14 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 
 verify_irqchip_in_kernel(&local_err);
 if (local_err) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
+error_propagate(errp, local_err);
 return -ENOTSUP;
 }
 dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
 ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12,
   &local_err);
 if (ret < 0) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
+error_propagate(errp, local_err);
 return ret;
 }
 pci_dev->msix_cap = pos;
@@ -1330,8 +1326,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF,
   &local_err);
 if (ret < 0) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
+error_propagate(errp, local_err);
 return ret;
 }
 
@@ -1369,8 +1364,8 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
  */
 size = MIN(0x3c, PCI_CONFIG_SPACE_SIZE - pos);
 if (size < 0x34) {
-error_report("%s: Invalid size PCIe cap-id 0x%x",
- __func__, PCI_CAP_ID_EXP);
+error_setg(errp, "Invalid size PCIe cap-id 0x%x",
+   PCI_CAP_ID_EXP);
 return -EINVAL;
 } else if (size != 0x3c) {
 error_report("WARNING, %s: PCIe cap-id 0x%x has "
@@ -1391,16 +1386,15 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 }
 
 if (size == 0) {
-error_report("%s: Unsupported PCI express capability version %d",
- __func__, version);
+error_setg(errp, "Unsupported PCI express capability version %d",
+   version);
 return -EINVAL;
 }
 
 ret = pci_add_capability2(pci_dev, PCI_CAP_ID_EXP, pos, size,
   &local_err);
 if (ret < 0) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
+error_propagate(errp, local_err);
 return ret;
 }
 
@@ -1410,8 +1404,8 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 type = (type & PCI_EXP_FLAGS_TYPE) >> 4;
 if (type != PCI_EXP_TYPE_ENDPOINT &&
 type != PCI_EXP_TYPE_LEG_END && type != PCI_EXP_TYPE_RC_END) {
-error_report("Device assignment only supports endpoint assignment,"
- " device type %d", type);
+error_setg(errp, "Device assignment only supports endpoint "
+   "assignment, device type %d", type);
 return -EINVAL;
 }
 
@@ -1476,8 +1470,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
 ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PCIX, pos, 8,
   &local_err);

[Qemu-devel] [PATCH 4/8] hw/arm/omap1: Avoid unintended sign extension writing omap_rtc YEARS_REG

[Peter Maydell]

When writing to the YEARS_REG register, if the year value is
99 then the multiplication by 31536000 will overflow into
the sign bit of a 32 bit value and then be erroneously
sign-extended if time_t is 64 bits. Add a cast to avoid this.

Signed-off-by: Peter Maydell 
---
 hw/arm/omap1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index b433748..b28e052 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -2709,8 +2709,8 @@ static void omap_rtc_write(void *opaque, hwaddr addr,
 s->ti += ti[1];
 } else {
 /* A less accurate version */
-s->ti -= (s->current_tm.tm_year % 100) * 31536000;
-s->ti += from_bcd(value) * 31536000;
+s->ti -= (time_t)(s->current_tm.tm_year % 100) * 31536000;
+s->ti += (time_t)from_bcd(value) * 31536000;
 }
 return;
 
-- 
1.9.2

[Qemu-devel] [PULL 29/38] tests/qapi-schema: Drop superfluous error_is_set()

[Luiz Capitulino]

From: Markus Armbruster 

visit_type_TestStruct() does nothing when called with an error set.
Callers shouldn't do that, and no caller does.  Drop the superfluous
test.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Michael Roth 
Signed-off-by: Luiz Capitulino 
---
 tests/test-qmp-input-visitor.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 1ebafc5..a58a3e6 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -196,21 +196,20 @@ static void visit_type_TestStruct(Visitor *v, TestStruct 
**obj,
   const char *name, Error **errp)
 {
 Error *err = NULL;
-if (!error_is_set(errp)) {
-visit_start_struct(v, (void **)obj, "TestStruct", name, 
sizeof(TestStruct),
-   &err);
-if (!err) {
-visit_type_int(v, &(*obj)->integer, "integer", &err);
-visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
-visit_type_str(v, &(*obj)->string, "string", &err);
-
-/* Always call end_struct if start_struct succeeded.  */
-error_propagate(errp, err);
-err = NULL;
-visit_end_struct(v, &err);
-}
+
+visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
+   &err);
+if (!err) {
+visit_type_int(v, &(*obj)->integer, "integer", &err);
+visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
+visit_type_str(v, &(*obj)->string, "string", &err);
+
+/* Always call end_struct if start_struct succeeded.  */
 error_propagate(errp, err);
+err = NULL;
+visit_end_struct(v, &err);
 }
+error_propagate(errp, err);
 }
 
 static void test_visitor_in_struct(TestInputVisitorData *data,
-- 
1.9.0

[Qemu-devel] [PULL 31/38] qga: Clean up fragile use of error_is_set()

[Luiz Capitulino]

From: Markus Armbruster 

Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque.  It's wrong when ERRP
may be null, because errors go undetected when it is.  It's fragile
when proving ERRP non-null involves a non-local argument.  Else, it's
unnecessarily opaque (see commit 84d18f0).

The error_is_set(errp) in the guest agent command handler functions
are merely fragile, because all chall chains (do_qmp_dispatch() via
the generated marshalling functions) pass a non-null errp argument.

Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Michael Roth 
Signed-off-by: Luiz Capitulino 
---
 qga/commands-posix.c | 22 --
 qga/commands-win32.c | 38 --
 2 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 6af974f..34ddba0 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1067,8 +1067,11 @@ out:
 
 void qmp_guest_suspend_disk(Error **errp)
 {
-bios_supports_mode("pm-is-supported", "--hibernate", "disk", errp);
-if (error_is_set(errp)) {
+Error *local_err = NULL;
+
+bios_supports_mode("pm-is-supported", "--hibernate", "disk", &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
 
@@ -1077,8 +1080,11 @@ void qmp_guest_suspend_disk(Error **errp)
 
 void qmp_guest_suspend_ram(Error **errp)
 {
-bios_supports_mode("pm-is-supported", "--suspend", "mem", errp);
-if (error_is_set(errp)) {
+Error *local_err = NULL;
+
+bios_supports_mode("pm-is-supported", "--suspend", "mem", &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
 
@@ -1087,8 +1093,12 @@ void qmp_guest_suspend_ram(Error **errp)
 
 void qmp_guest_suspend_hybrid(Error **errp)
 {
-bios_supports_mode("pm-is-supported", "--suspend-hybrid", NULL, errp);
-if (error_is_set(errp)) {
+Error *local_err = NULL;
+
+bios_supports_mode("pm-is-supported", "--suspend-hybrid", NULL,
+   &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
 
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index d0d8504..3483c0d 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -87,6 +87,7 @@ static void execute_async(DWORD WINAPI (*func)(LPVOID), 
LPVOID opaque,
 
 void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
 {
+Error *local_err = NULL;
 UINT shutdown_flag = EWX_FORCE;
 
 slog("guest-shutdown called, mode: %s", mode);
@@ -105,8 +106,9 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, 
Error **errp)
 
 /* Request a shutdown privilege, but try to shut down the system
anyway. */
-acquire_privilege(SE_SHUTDOWN_NAME, errp);
-if (error_is_set(errp)) {
+acquire_privilege(SE_SHUTDOWN_NAME, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
 return;
 }
 
@@ -191,14 +193,16 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
 /* cannot risk guest agent blocking itself on a write in this state */
 ga_set_frozen(ga_state);
 
-qga_vss_fsfreeze(&i, errp, true);
-if (error_is_set(errp)) {
+qga_vss_fsfreeze(&i, &local_err, true);
+if (local_err) {
+error_propagate(errp, local_err);
 goto error;
 }
 
 return i;
 
 error:
+local_err = NULL;
 qmp_guest_fsfreeze_thaw(&local_err);
 if (local_err) {
 g_debug("cleanup thaw: %s", error_get_pretty(local_err));
@@ -313,28 +317,32 @@ static DWORD WINAPI do_suspend(LPVOID opaque)
 
 void qmp_guest_suspend_disk(Error **errp)
 {
+Error *local_err = NULL;
 GuestSuspendMode *mode = g_malloc(sizeof(GuestSuspendMode));
 
 *mode = GUEST_SUSPEND_MODE_DISK;
-check_suspend_mode(*mode, errp);
-acquire_privilege(SE_SHUTDOWN_NAME, errp);
-execute_async(do_suspend, mode, errp);
+check_suspend_mode(*mode, &local_err);
+acquire_privilege(SE_SHUTDOWN_NAME, &local_err);
+execute_async(do_suspend, mode, &local_err);
 
-if (error_is_set(errp)) {
+if (local_err) {
+error_propagate(errp, local_err);
 g_free(mode);
 }
 }
 
 void qmp_guest_suspend_ram(Error **errp)
 {
+Error *local_err = NULL;
 GuestSuspendMode *mode = g_malloc(sizeof(GuestSuspendMode));
 
 *mode = GUEST_SUSPEND_MODE_RAM;
-check_suspend_mode(*mode, errp);
-acquire_privilege(SE_SHUTDOWN_NAME, errp);
-execute_async(do_suspend, mode, errp);
+check_suspend_mode(*mode, &local_err);
+acquire_privilege(SE_SHUTDOWN_NAME, &local_err);
+execute_async(do_suspend, mode, &local_err);
 
-if (error_is_set(errp)) {
+if (local_err) {
+error_propagate(errp, local_err);
 g_free(m

[Qemu-devel] [PULL 33/38] qemu-option: Clean up fragile use of error_is_set()

[Luiz Capitulino]

From: Markus Armbruster 

Using error_is_set(ERRP) to find out whether to bail out due to
previous error is either wrong, fragile, or unnecessarily opaque.
It's wrong when ERRP may be null, because errors go undetected when it
is.  It's fragile when proving ERRP non-null involves a non-local
argument.  Else, it's unnecessarily opaque (see commit 84d18f0).

The error_is_set(state->errp) in qemu_opts_from_qdict_1() is merely
fragile, because the callers never pass state argument with null
state->errp.

Make the code more robust and more obviously correct: test
*state->errp directly.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 util/qemu-option.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 8bbc3ad..324e4c5 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1036,7 +1036,7 @@ static void qemu_opts_from_qdict_1(const char *key, 
QObject *obj, void *opaque)
 const char *value;
 int n;
 
-if (!strcmp(key, "id") || error_is_set(state->errp)) {
+if (!strcmp(key, "id") || *state->errp) {
 return;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 05/38] qapi: treat all negative return of strtosz_suffix() as error

[Luiz Capitulino]

From: Amos Kong 

strtosz_suffix() might return negative error, this patch fixes
the error handling.

This patch also changes to handle error in the if statement
rather than handle success specially, this will make this use
of strtosz_suffix consistent with all other uses.

Signed-off-by: Amos Kong 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Luiz Capitulino 
---
 qapi/opts-visitor.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 5d830a2..87c1c78 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -472,13 +472,14 @@ opts_type_size(Visitor *v, uint64_t *obj, const char 
*name, Error **errp)
 
 val = strtosz_suffix(opt->str ? opt->str : "", &endptr,
  STRTOSZ_DEFSUFFIX_B);
-if (val != -1 && *endptr == '\0') {
-*obj = val;
-processed(ov, name);
+if (val < 0 || *endptr) {
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
+  "a size value representible as a non-negative int64");
 return;
 }
-error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
-  "a size value representible as a non-negative int64");
+
+*obj = val;
+processed(ov, name);
 }
 
 
-- 
1.9.0

[Qemu-devel] [PULL 21/38] pci-assign: assigned_initfn(): set monitor error in common error handler

[Luiz Capitulino]

From: Laszlo Ersek 

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 26 ++
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 6891729..e55421a 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1756,14 +1756,14 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 Error *local_err = NULL;
 
 if (!kvm_enabled()) {
-error_report("pci-assign: error: requires KVM support");
-return -1;
+error_setg(&local_err, "pci-assign requires KVM support");
+goto exit_with_error;
 }
 
 if (!dev->host.domain && !dev->host.bus && !dev->host.slot &&
 !dev->host.function) {
-error_report("pci-assign: error: no host device specified");
-return -1;
+error_setg(&local_err, "no host device specified");
+goto exit_with_error;
 }
 
 /*
@@ -1788,14 +1788,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 
 get_real_device(dev, &local_err);
 if (local_err) {
-qerror_report_err(local_err);
-error_free(local_err);
 goto out;
 }
 
 if (assigned_device_pci_cap_init(pci_dev, &local_err) < 0) {
-qerror_report_err(local_err);
-error_free(local_err);
 goto out;
 }
 
@@ -1803,8 +1799,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
 assigned_dev_register_msix_mmio(dev, &local_err);
 if (local_err) {
-qerror_report_err(local_err);
-error_free(local_err);
 goto out;
 }
 }
@@ -1814,8 +1808,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
   dev->real_device.region_number, dev,
   &local_err);
 if (local_err) {
-qerror_report_err(local_err);
-error_free(local_err);
 goto out;
 }
 
@@ -1828,16 +1820,12 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 /* assign device to guest */
 assign_device(dev, &local_err);
 if (local_err) {
-qerror_report_err(local_err);
-error_free(local_err);
 goto out;
 }
 
 /* assign legacy INTx to the device */
 r = assign_intx(dev, &local_err);
 if (r < 0) {
-qerror_report_err(local_err);
-error_free(local_err);
 goto assigned_out;
 }
 
@@ -1849,8 +1837,14 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 
 assigned_out:
 deassign_device(dev);
+
 out:
 free_assigned_device(dev);
+
+exit_with_error:
+assert(local_err);
+qerror_report_err(local_err);
+error_free(local_err);
 return -1;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 14/38] pci-assign: assignment should fail if we can't read config space

[Luiz Capitulino]

From: Laszlo Ersek 

assigned_initfn()
  get_real_device()
read()

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index f91d4fb..e89bb6a 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -576,6 +576,7 @@ again:
 goto again;
 }
 error_report("%s: read failed, errno = %d", __func__, errno);
+return 1;
 }
 
 /* Restore or clear multifunction, this is always controlled by qemu */
-- 
1.9.0

[Qemu-devel] [PULL 15/38] pci-assign: propagate errors from get_real_device()

[Luiz Capitulino]

From: Laszlo Ersek 

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index e89bb6a..c6d1094 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -532,7 +532,7 @@ static void get_real_device_id(const char *devpath, 
uint16_t *val,
 get_real_id(devpath, "device", val, errp);
 }
 
-static int get_real_device(AssignedDevice *pci_dev)
+static void get_real_device(AssignedDevice *pci_dev, Error **errp)
 {
 char dir[128], name[128];
 int fd, r = 0;
@@ -556,16 +556,15 @@ static int get_real_device(AssignedDevice *pci_dev)
   pci_dev->configfd_name,
   &local_err);
 if (local_err) {
-qerror_report_err(local_err);
-error_free(local_err);
-return 1;
+error_propagate(errp, local_err);
+return;
 }
 } else {
 dev->config_fd = open(name, O_RDWR);
 
 if (dev->config_fd == -1) {
-error_report("%s: %s: %m", __func__, name);
-return 1;
+error_setg_file_open(errp, errno, name);
+return;
 }
 }
 again:
@@ -575,8 +574,10 @@ again:
 if (errno == EINTR || errno == EAGAIN) {
 goto again;
 }
-error_report("%s: read failed, errno = %d", __func__, errno);
-return 1;
+error_setg_errno(errp, errno, "read(\"%s\")",
+ (pci_dev->configfd_name && *pci_dev->configfd_name) ?
+ pci_dev->configfd_name : name);
+return;
 }
 
 /* Restore or clear multifunction, this is always controlled by qemu */
@@ -596,8 +597,8 @@ again:
 
 f = fopen(name, "r");
 if (f == NULL) {
-error_report("%s: %s: %m", __func__, name);
-return 1;
+error_setg_file_open(errp, errno, name);
+return;
 }
 
 for (r = 0; r < PCI_ROM_SLOT; r++) {
@@ -642,9 +643,8 @@ again:
 /* read and fill vendor ID */
 get_real_vendor_id(dir, &id, &local_err);
 if (local_err) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
-return 1;
+error_propagate(errp, local_err);
+return;
 }
 pci_dev->dev.config[0] = id & 0xff;
 pci_dev->dev.config[1] = (id & 0xff00) >> 8;
@@ -652,9 +652,8 @@ again:
 /* read and fill device ID */
 get_real_device_id(dir, &id, &local_err);
 if (local_err) {
-error_report("%s", error_get_pretty(local_err));
-error_free(local_err);
-return 1;
+error_propagate(errp, local_err);
+return;
 }
 pci_dev->dev.config[2] = id & 0xff;
 pci_dev->dev.config[3] = (id & 0xff00) >> 8;
@@ -663,7 +662,6 @@ again:
  PCI_COMMAND_MASTER | 
PCI_COMMAND_INTX_DISABLE);
 
 dev->region_number = r;
-return 0;
 }
 
 static void free_msi_virqs(AssignedDevice *dev)
@@ -1751,6 +1749,7 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
 uint8_t e_intx;
 int r;
+Error *local_err = NULL;
 
 if (!kvm_enabled()) {
 error_report("pci-assign: error: requires KVM support");
@@ -1783,9 +1782,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 memcpy(dev->emulate_config_write, dev->emulate_config_read,
sizeof(dev->emulate_config_read));
 
-if (get_real_device(dev)) {
-error_report("pci-assign: Error: Couldn't get real device (%s)!",
- dev->dev.qdev.id);
+get_real_device(dev, &local_err);
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
 goto out;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 06/38] cutils: tighten qemu_parse_fd()

[Luiz Capitulino]

From: Laszlo Ersek 

qemu_parse_fd() used to handle at least the following strings incorrectly:
o "-2": simply let through
o "2147483648": returned as LONG_MAX==INT_MAX on ILP32 (with ERANGE
ignored); implementation-defined behavior on LP64

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 util/cutils.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/util/cutils.c b/util/cutils.c
index b337293..dbe7412 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -24,6 +24,8 @@
 #include "qemu-common.h"
 #include "qemu/host-utils.h"
 #include 
+#include 
+#include 
 
 #include "qemu/sockets.h"
 #include "qemu/iov.h"
@@ -457,11 +459,16 @@ int parse_uint_full(const char *s, unsigned long long 
*value, int base)
 
 int qemu_parse_fd(const char *param)
 {
-int fd;
-char *endptr = NULL;
+long fd;
+char *endptr;
 
+errno = 0;
 fd = strtol(param, &endptr, 10);
-if (*endptr || (fd == 0 && param == endptr)) {
+if (param == endptr /* no conversion performed */||
+errno != 0  /* not representable as long; possibly others */ ||
+*endptr != '\0' /* final string not empty */ ||
+fd < 0  /* invalid as file descriptor */ ||
+fd > INT_MAX/* not representable as int */) {
 return -1;
 }
 return fd;
-- 
1.9.0

[Qemu-devel] [PULL 18/38] pci-assign: propagate errors from assigned_dev_register_regions()

[Luiz Capitulino]

From: Laszlo Ersek 

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 3a904e8..9aa92a1 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -394,9 +394,10 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t 
cap, uint8_t start)
 return 0;
 }
 
-static int assigned_dev_register_regions(PCIRegion *io_regions,
- unsigned long regions_num,
- AssignedDevice *pci_dev)
+static void assigned_dev_register_regions(PCIRegion *io_regions,
+  unsigned long regions_num,
+  AssignedDevice *pci_dev,
+  Error **errp)
 {
 uint32_t i;
 PCIRegion *cur_region = io_regions;
@@ -425,9 +426,9 @@ static int assigned_dev_register_regions(PCIRegion 
*io_regions,
 
 if (pci_dev->v_addrs[i].u.r_virtbase == MAP_FAILED) {
 pci_dev->v_addrs[i].u.r_virtbase = NULL;
-error_report("%s: Error: Couldn't mmap 0x%" PRIx64 "!",
- __func__, cur_region->base_addr);
-return -1;
+error_setg_errno(errp, errno, "Couldn't mmap 0x%" PRIx64 "!",
+ cur_region->base_addr);
+return;
 }
 
 pci_dev->v_addrs[i].r_size = cur_region->size;
@@ -496,7 +497,6 @@ static int assigned_dev_register_regions(PCIRegion 
*io_regions,
 }
 
 /* success */
-return 0;
 }
 
 static void get_real_id(const char *devpath, const char *idname, uint16_t *val,
@@ -1796,9 +1796,12 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 }
 
 /* handle real device's MMIO/PIO BARs */
-if (assigned_dev_register_regions(dev->real_device.regions,
-  dev->real_device.region_number,
-  dev)) {
+assigned_dev_register_regions(dev->real_device.regions,
+  dev->real_device.region_number, dev,
+  &local_err);
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
 goto out;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 07/38] monitor: add Error-propagating monitor_handle_fd_param2()

[Luiz Capitulino]

From: Laszlo Ersek 

and rebase monitor_handle_fd_param() to it. (Note that this will slightly
change the behavior when the qemu_parse_fd() branch is selected and it
fails: we now report (and in case of QMP, set) the error immediately,
rather than allowing the caller to set its own error message (if any)).

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 include/monitor/monitor.h |  1 +
 monitor.c | 27 ++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 42d8671..1c1f56f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -75,6 +75,7 @@ int monitor_read_block_device_key(Monitor *mon, const char 
*device,
 
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
 int monitor_handle_fd_param(Monitor *mon, const char *fdname);
+int monitor_handle_fd_param2(Monitor *mon, const char *fdname, Error **errp);
 
 void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 GCC_FMT_ATTR(2, 0);
diff --git a/monitor.c b/monitor.c
index 2d3fb3f..9af6b0a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2611,16 +2611,33 @@ int monitor_handle_fd_param(Monitor *mon, const char 
*fdname)
 int fd;
 Error *local_err = NULL;
 
-if (!qemu_isdigit(fdname[0]) && mon) {
+fd = monitor_handle_fd_param2(mon, fdname, &local_err);
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
+}
+return fd;
+}
+
+int monitor_handle_fd_param2(Monitor *mon, const char *fdname, Error **errp)
+{
+int fd;
+Error *local_err = NULL;
 
+if (!qemu_isdigit(fdname[0]) && mon) {
 fd = monitor_get_fd(mon, fdname, &local_err);
+} else {
+fd = qemu_parse_fd(fdname);
 if (fd == -1) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
+error_setg(&local_err, "Invalid file descriptor number '%s'",
+   fdname);
 }
+}
+if (local_err) {
+error_propagate(errp, local_err);
+assert(fd == -1);
 } else {
-fd = qemu_parse_fd(fdname);
+assert(fd != -1);
 }
 
 return fd;
-- 
1.9.0

[Qemu-devel] [PULL 10/38] pci-assign: propagate errors from get_real_id()

[Luiz Capitulino]

From: Laszlo Ersek 

get_real_id() has two thin wrappers (and no other callers),
get_real_vendor_id() and get_real_device_id(); it's easiest to convert
them in one fell swoop.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 45 +++--
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 6b8db25..997ef09 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -499,7 +499,8 @@ static int assigned_dev_register_regions(PCIRegion 
*io_regions,
 return 0;
 }
 
-static int get_real_id(const char *devpath, const char *idname, uint16_t *val)
+static void get_real_id(const char *devpath, const char *idname, uint16_t *val,
+Error **errp)
 {
 FILE *f;
 char name[128];
@@ -508,34 +509,33 @@ static int get_real_id(const char *devpath, const char 
*idname, uint16_t *val)
 snprintf(name, sizeof(name), "%s%s", devpath, idname);
 f = fopen(name, "r");
 if (f == NULL) {
-error_report("%s: %s: %m", __func__, name);
-return -1;
+error_setg_file_open(errp, errno, name);
+return;
 }
 if (fscanf(f, "%li\n", &id) == 1) {
 *val = id;
 } else {
-fclose(f);
-return -1;
+error_setg(errp, "Failed to parse contents of '%s'", name);
 }
 fclose(f);
-
-return 0;
 }
 
-static int get_real_vendor_id(const char *devpath, uint16_t *val)
+static void get_real_vendor_id(const char *devpath, uint16_t *val,
+   Error **errp)
 {
-return get_real_id(devpath, "vendor", val);
+get_real_id(devpath, "vendor", val, errp);
 }
 
-static int get_real_device_id(const char *devpath, uint16_t *val)
+static void get_real_device_id(const char *devpath, uint16_t *val,
+   Error **errp)
 {
-return get_real_id(devpath, "device", val);
+get_real_id(devpath, "device", val, errp);
 }
 
 static int get_real_device(AssignedDevice *pci_dev)
 {
 char dir[128], name[128];
-int fd, r = 0, v;
+int fd, r = 0;
 FILE *f;
 uint64_t start, end, size, flags;
 uint16_t id;
@@ -639,16 +639,20 @@ again:
 fclose(f);
 
 /* read and fill vendor ID */
-v = get_real_vendor_id(dir, &id);
-if (v) {
+get_real_vendor_id(dir, &id, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return 1;
 }
 pci_dev->dev.config[0] = id & 0xff;
 pci_dev->dev.config[1] = (id & 0xff00) >> 8;
 
 /* read and fill device ID */
-v = get_real_device_id(dir, &id);
-if (v) {
+get_real_device_id(dir, &id, &local_err);
+if (local_err) {
+error_report("%s", error_get_pretty(local_err));
+error_free(local_err);
 return 1;
 }
 pci_dev->dev.config[2] = id & 0xff;
@@ -741,6 +745,7 @@ static char *assign_failed_examine(const AssignedDevice 
*dev)
 char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns;
 uint16_t vendor_id, device_id;
 int r;
+Error *local_err = NULL;
 
 snprintf(dir, sizeof(dir), "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/",
 dev->host.domain, dev->host.bus, dev->host.slot,
@@ -761,8 +766,12 @@ static char *assign_failed_examine(const AssignedDevice 
*dev)
 
 ns++;
 
-if (get_real_vendor_id(dir, &vendor_id) ||
-get_real_device_id(dir, &device_id)) {
+if ((get_real_vendor_id(dir, &vendor_id, &local_err), local_err) ||
+(get_real_device_id(dir, &device_id, &local_err), local_err)) {
+/* We're already analyzing an assignment error, so we suppress this
+ * one just like the others above.
+ */
+error_free(local_err);
 goto fail;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL 09/38] pci-assign: make assign_failed_examine() just format the cause

[Luiz Capitulino]

From: Laszlo Ersek 

This allows us to report the entire error with one error_report() call,
easing future error propagation.

Signed-off-by: Laszlo Ersek 
Reviewed-by: Eric Blake 
Signed-off-by: Luiz Capitulino 
---
 hw/i386/kvm/pci-assign.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index bfce97f..6b8db25 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -731,7 +731,12 @@ static void free_assigned_device(AssignedDevice *dev)
 free_msi_virqs(dev);
 }
 
-static void assign_failed_examine(AssignedDevice *dev)
+/* This function tries to determine the cause of the PCI assignment failure. It
+ * always returns the cause as a dynamically allocated, human readable string.
+ * If the function fails to determine the cause for any internal reason, then
+ * the returned string will state that fact.
+ */
+static char *assign_failed_examine(const AssignedDevice *dev)
 {
 char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns;
 uint16_t vendor_id, device_id;
@@ -761,8 +766,8 @@ static void assign_failed_examine(AssignedDevice *dev)
 goto fail;
 }
 
-error_printf("*** The driver '%s' is occupying your device "
-"%04x:%02x:%02x.%x.\n"
+return g_strdup_printf(
+"*** The driver '%s' is occupying your device %04x:%02x:%02x.%x.\n"
 "***\n"
 "*** You can try the following commands to free it:\n"
 "***\n"
@@ -778,10 +783,8 @@ static void assign_failed_examine(AssignedDevice *dev)
 ns, dev->host.domain, dev->host.bus, dev->host.slot,
 dev->host.function, vendor_id, device_id);
 
-return;
-
 fail:
-error_report("Couldn't find out why.");
+return g_strdup("Couldn't find out why.");
 }
 
 static int assign_device(AssignedDevice *dev)
@@ -810,14 +813,19 @@ static int assign_device(AssignedDevice *dev)
 
 r = kvm_device_pci_assign(kvm_state, &dev->host, flags, &dev->dev_id);
 if (r < 0) {
-error_report("Failed to assign device \"%s\" : %s",
- dev->dev.qdev.id, strerror(-r));
-
 switch (r) {
-case -EBUSY:
-assign_failed_examine(dev);
+case -EBUSY: {
+char *cause;
+
+cause = assign_failed_examine(dev);
+error_report("Failed to assign device \"%s\" : %s\n%s",
+ dev->dev.qdev.id, strerror(-r), cause);
+g_free(cause);
 break;
+}
 default:
+error_report("Failed to assign device \"%s\" : %s",
+ dev->dev.qdev.id, strerror(-r));
 break;
 }
 }
-- 
1.9.0

[Qemu-devel] [PULL 01/38] qapi: [trivial] Break long command lines

[Luiz Capitulino]

From: Lluís Vilanova 

Signed-off-by: Lluís Vilanova 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 Makefile   | 24 ++--
 tests/Makefile | 18 ++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 423e373..2b8e312 100644
--- a/Makefile
+++ b/Makefile
@@ -238,23 +238,35 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py 
$(SRC_PATH)/scripts/ordereddict.py
 
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py 
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+   $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \
+   "  GEN   $@")
 qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+   $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \
+   "  GEN   $@")
 qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py 
$(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
$(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+   $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \
+   "  GEN   $@")
 
 qapi-types.c qapi-types.h :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py 
$(gen-out-type) -o "." -b < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+   $(gen-out-type) -o "." -b < $<, \
+   "  GEN   $@")
 qapi-visit.c qapi-visit.h :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
$(gen-out-type) -o "." -b < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+   $(gen-out-type) -o "." -b < $<, \
+   "  GEN   $@")
 qmp-commands.h qmp-marshal.c :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
$(gen-out-type) -m -o "." < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+   $(gen-out-type) -o "." -m < $<, \
+   "  GEN   $@")
 
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h 
qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
diff --git a/tests/Makefile b/tests/Makefile
index 14ecf05..c7fb2df 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -242,13 +242,19 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
 
 tests/test-qapi-types.c tests/test-qapi-types.h :\
 $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-types.py
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py 
$(gen-out-type) -o tests -p "test-" < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+   $(gen-out-type) -o tests -p "test-" < $<, \
+   "  GEN   $@")
 tests/test-qapi-visit.c tests/test-qapi-visit.h :\
 $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-visit.py
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
$(gen-out-type) -o tests -p "test-" < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+   $(gen-out-type) -o tests -p "test-" < $<, \
+   "  GEN   $@")
 tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
 $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-commands.py
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
$(gen-out-type) -o tests -p "test-" < $<, "  GEN   $@")
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+   $(gen-out-type) -o tests -p "test-" < $<, \
+   "  GEN   $@")
 
 tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o 
$(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o 
$(test-qapi-obj-y) libqemuutil.a libqemustub.a
@@ -400,7 +406,11 @@ check-t

[Qemu-devel] [PATCH 5/8] hw/dma/omap_dma: Add (uint32_t) casts when shifting uint16_t by 16

[Peter Maydell]

Add missing (uint32_t) casts in cases where we're trying to
put a uint16_t value into the top half of a 32-bit field.
These were already present in some but not all places.

Signed-off-by: Peter Maydell 
---
For new code or code I cared about I'd use deposit32(); but omap
is pretty ancient and unloved, so this is the minimal fix.
---
 hw/dma/omap_dma.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c
index 0e8cccd..0f35c42 100644
--- a/hw/dma/omap_dma.c
+++ b/hw/dma/omap_dma.c
@@ -973,7 +973,7 @@ static int omap_dma_ch_reg_write(struct omap_dma_s *s,
 
 case 0x22: /* DMA_COLOR_U */
 ch->color &= 0x;
-ch->color |= value << 16;
+ch->color |= (uint32_t)value << 16;
 break;
 
 case 0x24: /* DMA_CCR2 */
@@ -1043,7 +1043,7 @@ static int omap_dma_3_2_lcd_write(struct 
omap_dma_lcd_channel_s *s, int offset,
 
 case 0xbca:/* TOP_B1_U */
 s->src_f1_top &= 0x;
-s->src_f1_top |= value << 16;
+s->src_f1_top |= (uint32_t)value << 16;
 break;
 
 case 0xbcc:/* BOT_B1_L */
@@ -1265,7 +1265,7 @@ static int omap_dma_3_1_lcd_write(struct 
omap_dma_lcd_channel_s *s, int offset,
 
 case 0x304:/* SYS_DMA_LCD_TOP_F1_U */
 s->src_f1_top &= 0x;
-s->src_f1_top |= value << 16;
+s->src_f1_top |= (uint32_t)value << 16;
 break;
 
 case 0x306:/* SYS_DMA_LCD_BOT_F1_L */
@@ -1275,7 +1275,7 @@ static int omap_dma_3_1_lcd_write(struct 
omap_dma_lcd_channel_s *s, int offset,
 
 case 0x308:/* SYS_DMA_LCD_BOT_F1_U */
 s->src_f1_bottom &= 0x;
-s->src_f1_bottom |= value << 16;
+s->src_f1_bottom |= (uint32_t)value << 16;
 break;
 
 case 0x30a:/* SYS_DMA_LCD_TOP_F2_L */
@@ -1285,7 +1285,7 @@ static int omap_dma_3_1_lcd_write(struct 
omap_dma_lcd_channel_s *s, int offset,
 
 case 0x30c:/* SYS_DMA_LCD_TOP_F2_U */
 s->src_f2_top &= 0x;
-s->src_f2_top |= value << 16;
+s->src_f2_top |= (uint32_t)value << 16;
 break;
 
 case 0x30e:/* SYS_DMA_LCD_BOT_F2_L */
@@ -1295,7 +1295,7 @@ static int omap_dma_3_1_lcd_write(struct 
omap_dma_lcd_channel_s *s, int offset,
 
 case 0x310:/* SYS_DMA_LCD_BOT_F2_U */
 s->src_f2_bottom &= 0x;
-s->src_f2_bottom |= value << 16;
+s->src_f2_bottom |= (uint32_t)value << 16;
 break;
 
 default:
-- 
1.9.2

Re: [Qemu-devel] [PATCH] Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()"

[Luiz Capitulino]

On Thu,  8 May 2014 18:03:15 +0200
Peter Lieven  wrote:

> This reverts commit 25a7017555f1b4aeb543b5d323ff4afb8f9c5437.
> 
> Turns out the argument *can* be null: QEMU now segfaults if it
> receives an invalid parameter via a qmp command instead of throwing an
> error.
> 
> For example:
> { "execute": "blockdev-add",
>  "arguments": { "options" : { "driver": "invalid-driver" } } }
> 
> CC: qemu-sta...@nongnu.org
> Signed-off-by: Peter Lieven 
> Reviewed-by: Eric Blake 
> Reviewed-by: Markus Armbruster 

Applied to the qmp branch, thanks.

> ---
>  qapi/qapi-dealloc-visitor.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index d0ea118..dc53545 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -131,7 +131,9 @@ static void qapi_dealloc_end_list(Visitor *v, Error 
> **errp)
>  static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name,
>Error **errp)
>  {
> -g_free(*obj);
> +if (obj) {
> +g_free(*obj);
> +}
>  }
>  
>  static void qapi_dealloc_type_int(Visitor *v, int64_t *obj, const char *name,

[Qemu-devel] [PATCH 6/8] hw/timer/exynos4210_mct: Avoid overflow in exynos4210_ltick_recalc_count

[Peter Maydell]

Add casts to avoid potentially overflowing the multiplications
of 32 bit quantities in exynos4210_ltick_recalc_count().

Signed-off-by: Peter Maydell 
---
 hw/timer/exynos4210_mct.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
index 86f4fcd..69dbecd 100644
--- a/hw/timer/exynos4210_mct.c
+++ b/hw/timer/exynos4210_mct.c
@@ -824,14 +824,14 @@ static void exynos4210_ltick_recalc_count(struct 
tick_timer *s)
  */
 
 if (s->last_tcnto) {
-to_count = s->last_tcnto * s->last_icnto;
+to_count = (uint64_t)s->last_tcnto * s->last_icnto;
 } else {
 to_count = s->last_icnto;
 }
 } else {
 /* distance is passed, recalculate with tcnto * icnto */
 if (s->icntb) {
-s->distance = s->tcntb * s->icntb;
+s->distance = (uint64_t)s->tcntb * s->icntb;
 } else {
 s->distance = s->tcntb;
 }
-- 
1.9.2

[Qemu-devel] [PATCH 2/8] hw/net/cadence_gem: Remove dead code

[Peter Maydell]

Commit 191946c moved the code to handle padding to minimum
length from after the handling of the CRC to before it.
This means that the CRC code doesn't need to cope with the
possibility that the size is less than 60; remove this
dead code.

Signed-off-by: Peter Maydell 
---
 hw/net/cadence_gem.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index cdb1825..afddc8a 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -717,7 +717,6 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
 rxbuf_ptr = (void *)buf;
 } else {
 unsigned crc_val;
-int  crc_offset;
 
 /* The application wants the FCS field, which QEMU does not provide.
  * We must try and caclculate one.
@@ -727,12 +726,7 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
 memset(rxbuf + size, 0, sizeof(rxbuf) - size);
 rxbuf_ptr = rxbuf;
 crc_val = cpu_to_le32(crc32(0, rxbuf, MAX(size, 60)));
-if (size < 60) {
-crc_offset = 60;
-} else {
-crc_offset = size;
-}
-memcpy(rxbuf + crc_offset, &crc_val, sizeof(crc_val));
+memcpy(rxbuf + size, &crc_val, sizeof(crc_val));
 
 bytes_to_copy += 4;
 size += 4;
-- 
1.9.2

[Qemu-devel] [PATCH 0/8] misc fixes for coverity warnings in ARM devices

[Peter Maydell]

This patchset is a mixed bag of minor fixes for various
Coverity warnings in ARM devices.

Peter Maydell (8):
  hw/intc/allwinner-a10-pic: Add missing 'break'
  hw/net/cadence_gem: Remove dead code
  hw/display/pxa2xx_lcd: Fix 16bpp+alpha and 18bpp+alpha palette formats
  hw/arm/omap1: Avoid unintended sign extension writing omap_rtc
YEARS_REG
  hw/dma/omap_dma: Add (uint32_t) casts when shifting uint16_t by 16
  hw/timer/exynos4210_mct: Avoid overflow in
exynos4210_ltick_recalc_count
  hw/arm/stellaris: Correct handling of GPTM TAR register
  hw/arm/omap_gpmc: Avoid buffer overrun filling prefetch FIFO

 hw/arm/omap1.c  |  4 ++--
 hw/arm/stellaris.c  | 13 ++---
 hw/display/pxa2xx_lcd.c | 14 +++---
 hw/dma/omap_dma.c   | 12 ++--
 hw/intc/allwinner-a10-pic.c |  1 +
 hw/misc/omap_gpmc.c |  4 
 hw/net/cadence_gem.c|  8 +---
 hw/timer/exynos4210_mct.c   |  4 ++--
 8 files changed, 33 insertions(+), 27 deletions(-)

-- 
1.9.2

[Qemu-devel] [PATCH 3/8] hw/display/pxa2xx_lcd: Fix 16bpp+alpha and 18bpp+alpha palette formats

[Peter Maydell]

The pxa2xx palette entry "16bpp plus transparency" format is
xxxTR000GG00B000, and "18bpp plus transparency" is
xxxTRR00GG00BB00.

Correct errors in the code for reading these and converting
them to the internal format. In particular, the buggy code
was attempting to mask out bit 24 of a uint16_t, which
Coverity spotted as an error.

Signed-off-by: Peter Maydell 
---
 hw/display/pxa2xx_lcd.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/display/pxa2xx_lcd.c b/hw/display/pxa2xx_lcd.c
index 09cdf17..fce013d 100644
--- a/hw/display/pxa2xx_lcd.c
+++ b/hw/display/pxa2xx_lcd.c
@@ -620,13 +620,13 @@ static void pxa2xx_palette_parse(PXA2xxLCDState *s, int 
ch, int bpp)
 src += 2;
 break;
 case 1: /* 16 bpp plus transparency */
-alpha = *(uint16_t *) src & (1 << 24);
+alpha = *(uint32_t *) src & (1 << 24);
 if (s->control[0] & LCCR0_CMS)
-r = g = b = *(uint16_t *) src & 0xff;
+r = g = b = *(uint32_t *) src & 0xff;
 else {
-r = (*(uint16_t *) src & 0xf800) >> 8;
-g = (*(uint16_t *) src & 0x07e0) >> 3;
-b = (*(uint16_t *) src & 0x001f) << 3;
+r = (*(uint32_t *) src & 0x7c) >> 15;
+g = (*(uint32_t *) src & 0x00fc00) >> 8;
+b = (*(uint32_t *) src & 0xf8);
 }
 src += 2;
 break;
@@ -635,9 +635,9 @@ static void pxa2xx_palette_parse(PXA2xxLCDState *s, int ch, 
int bpp)
 if (s->control[0] & LCCR0_CMS)
 r = g = b = *(uint32_t *) src & 0xff;
 else {
-r = (*(uint32_t *) src & 0xf8) >> 16;
+r = (*(uint32_t *) src & 0xfc) >> 16;
 g = (*(uint32_t *) src & 0x00fc00) >> 8;
-b = (*(uint32_t *) src & 0xf8);
+b = (*(uint32_t *) src & 0xfc);
 }
 src += 4;
 break;
-- 
1.9.2

Re: [Qemu-devel] [PATCH] vl.c: Check max_cpus limit instead of smp_cpus

[Eduardo Habkost]

On Thu, May 08, 2014 at 04:07:54PM +0100, Peter Maydell wrote:
> On 8 May 2014 16:00, Eduardo Habkost  wrote:
> > On Thu, May 08, 2014 at 09:26:36AM +0100, Peter Maydell wrote:
> >> On 7 May 2014 23:20, Andreas Färber  wrote:
> >> > Am 07.05.2014 23:48, schrieb Peter Maydell:
> >> >> This is confusing. What is max_cpus
> >> >
> >> > Specified via -smp, defaults to actual CPUs.
> >>
> >> I thought that was smp_cpus ...
> >
> > smp_cpus is the first argument to -smp. max_cpus is the "maxcpus" option
> > of -smp.
> 
> > Please don't do this. If the maxcpus value from the command-line is not
> > supported, QEMU should abort instead of silently ignoring the value
> > provided by the user.
> 
> Ah, I see. This code is confusing because we've split the
> handling of user errors in the -smp argument between
> the smp_parse() function and this bit of inline code.
> Ideally we should put all the error handling in one
> place...

The split makes sense to me: smp_parse() is just for basic parsing that
doesn't depend on any state other than the provided QemuOpts parameter,
and can run at any point in time.

The machine->max_cpus check, on the other hand depend on the machine
object to be created, and even changes the machine->max_cpus value. It
is more dependent on ordering of initialization. To me, it makes sense
to have it outside smp_parse().

But while looking at that code, I noticed that we can at least make the
MAX_CPUMASK_BITS check and the machine->max_cpus check a single one. I
will send a new patch.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2] block/raw-posix: Try both FIEMAP and SEEK_HOLE

[Max Reitz]

On 07.05.2014 07:56, Markus Armbruster wrote:

Eric Blake  writes:


On 05/06/2014 03:18 PM, Eric Blake wrote:


...if you are on a file system where SEEK_HOLE triggers the kernel
fallback of "entire file is allocated", but where FIEMAP is wired up for
that file system, would it make sense to have try_seek_hole return -1 in
situations where lseek(s->fd, 0, SEEK_HOLE) returns the end of the file?
  Even more, should skip_seek_hole be a tri-state?

On the other hand, such systems are getting vanishingly rare as people
upgrade to newer kernels.  Do we care about catering to them, or is it
fair game to just tell people to upgrade if they want performance?

My tolerance for code complications to speed up things under old kernels
isn't zero, but close.  Leave these games to downstreams sporting such
kernels.


As far as I see it, it isn't an optimization for older but for newer 
kernels. Including Paolo's comment for v3, we could get away without 
these tristate shenanigans if we'd keep the old (current) order, that 
is, first try FIEMAP which will return ENOTSUP if it's not supported, 
and then fall back to SEEK_HOLE which will always return some result (if 
available, otherwise we can't even use it, obviously).


But as SEEK_HOLE is apparently the more "standard" way to acquire the 
required information, using it first is an optimization for newer 
systems (older Linux systems and/or older drivers will only provide FIEMAP).


I think there is no big performance penality in trying FIEMAP first, as 
the first call to raw_co_get_block_status() will detect that it is not 
available for the given file and therefore will be skipped from then on 
anyway.


Considering this, I guess I'll send a v4 without any tristates and which 
keeps the old order (FIEMAP first and then SEEK_HOLE).


Max

Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent.

[Markus Armbruster]

Humor me: no period at end of subject.

Benoît Canet  writes:

> The purpose of this change is to help create a json file containing
> common definitions.

Please describe the new semantics of the include directive here, so
mathematically challenged readers don't have to loop up "idempotent" in
the dictionary :)

> The history used to detect multiple inclusions is not a stack anymore
> but a regular list.

You need a stack to show the actual include cycle.

There are two reasons to detect cycles.  The technical one is preventing
infinite expansion.  No longer applies with idempotent include.  The
other, more abstract one is rejecting nonsensical use of the include
directive.  I think that one still applies.

> The cycle detection check where updated and leaved in place to make
> sure the code will never enter into an infinite recursion loop.

-ENOPARSE

Suggest to retry in active voice :)

> Signed-off-by: Benoit Canet 
> ---
>  scripts/qapi.py   |   13 ++---
>  tests/Makefile|3 ++-
>  tests/qapi-schema/include-cycle.err   |3 ---
>  tests/qapi-schema/include-cycle.exit  |2 +-
>  tests/qapi-schema/include-cycle.out   |3 +++
>  tests/qapi-schema/include-idempotent.exit |1 +
>  tests/qapi-schema/include-idempotent.json |3 +++
>  tests/qapi-schema/include-idempotent.out  |3 +++
>  tests/qapi-schema/include-self-cycle.err  |1 -
>  tests/qapi-schema/include-self-cycle.exit |2 +-
>  tests/qapi-schema/include-self-cycle.out  |3 +++
>  tests/qapi-schema/sub-include-idempotent.json |3 +++
>  12 files changed, 26 insertions(+), 14 deletions(-)
>  create mode 100644 tests/qapi-schema/include-idempotent.err
>  create mode 100644 tests/qapi-schema/include-idempotent.exit
>  create mode 100644 tests/qapi-schema/include-idempotent.json
>  create mode 100644 tests/qapi-schema/include-idempotent.out
>  create mode 100644 tests/qapi-schema/sub-include-idempotent.json

Is this based on Luiz's queue-qmp?

> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index ec806aa..0ed44c8 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -79,7 +79,7 @@ class QAPISchema:
>  input_relname = fp.name
>  self.input_dir = os.path.dirname(input_fname)
>  self.input_file = input_relname
> -self.include_hist = include_hist + [(input_relname, input_fname)]
> +include_hist.append((input_relname, input_fname))
>  self.parent_info = parent_info
>  self.src = fp.read()
>  if self.src == '' or self.src[-1] != '\n':

Looks like you drop self.include_hist and simply keep it in local
variable include_hist.  Have you considered doing that in a separate
cleanup patch prior to this one?

> @@ -102,17 +102,16 @@ class QAPISchema:
>  'Expected a file name (string), got: 
> %s'
>  % include)
>  include_path = os.path.join(self.input_dir, include)
> -if any(include_path == elem[1]
> -   for elem in self.include_hist):
> -raise QAPIExprError(expr_info, "Inclusion loop for %s"
> -% include)
> +# make include idempotent by skipping further includes
> +if include_path in [x[1] for x in  include_hist]:
> +continue

Loses cycle detection.

Is permitting cycles necessary to solve your problem?  Or asking more
directly: do you actually need cyclic includes?

>  try:
>  fobj = open(include_path, 'r')
>  except IOError as e:
>  raise QAPIExprError(expr_info,
>  '%s: %s' % (e.strerror, include))
> -exprs_include = QAPISchema(fobj, include,
> -   self.include_hist, expr_info)
> +exprs_include = QAPISchema(fobj, include, include_hist,
> +   expr_info)
>  self.exprs.extend(exprs_include.exprs)
>  else:
>  expr_elem = {'expr': expr,
> diff --git a/tests/Makefile b/tests/Makefile
> index a8d45be..c587b4a 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
>  flat-union-string-discriminator.json \
>  include-simple.json include-relpath.json include-format-err.json \
>  include-non-file.json include-no-file.json include-before-err.json \
> -include-nested-err.json include-self-cycle.json include-cycle.json)
> +include-nested-err.json include-self-cycle.json include-cycle.json \
> +include-idempotent.json)
>  
>  GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h 
> tests/test-qmp-commands.h
>  
> diff