[Qemu-devel] qemu-system-ppc regression booting MacOS 9.2.1 image

[Mark Cave-Ayland]

Hi Nikunj,

Testing git master locally I see the following segfault when trying to
boot my test MacOS 9.2.1 image:


$ gdb --args ./qemu-system-ppc -bios
/home/build/src/openbios/openbios.git/openbios/obj-ppc/openbios-qemu.elf.nostrip
-cdrom /home/build/src/qemu/image/ppc/MacOS921.iso -boot d -m 512 -M mac99
GNU gdb (GDB) 7.4.1-debian
Copyright (C) 2012 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-linux-gnu".
For bug reporting instructions, please see:
...
Reading symbols from /home/build/rel-qemu-git/bin/qemu-system-ppc...done.
(gdb) r
Starting program: /home/build/rel-qemu-git/bin/qemu-system-ppc -bios
/home/build/src/openbios/openbios.git/openbios/obj-ppc/openbios-qemu.elf.nostrip
-cdrom /home/build/src/qemu/image/ppc/MacOS921.iso -boot d -m 512 -M mac99
warning: no loadable sections found in added symbol-file system-supplied
DSO at 0x77ffa000
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7fffe9eee700 (LWP 29713)]
[New Thread 0x7fffe6bf5700 (LWP 29714)]
[New Thread 0x7fffe63f4700 (LWP 29715)]
Trying to write invalid spr 0 (0x000) at 00f113c0
Trying to read invalid spr 0 (0x000) at 00f113c8
Trying to write privileged spr 955 (0x3bb) at 00f164b8
Trying to write invalid spr 959 (0x3bf) at 00f16520
Trying to read invalid spr 959 (0x3bf) at 00f16528
Trying to write privileged spr 955 (0x3bb) at 00f164b8
Trying to write invalid spr 959 (0x3bf) at 00f16520
Trying to read invalid spr 959 (0x3bf) at 00f16528

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe63f4700 (LWP 29715)]
0x77e20010 in ?? ()
(gdb) bt
#0  0x77e20010 in ?? ()
#1  0x0002 in ?? ()
#2  0x68090040 in ?? ()
#3  0x0002 in ?? ()
#4  0x6b67829c063b8d00 in ?? ()
#5  0x6b67829c063b8d00 in ?? ()
#6  0x0043986d in tcg_temp_new_internal_i32
(temp_local=temp_local@entry=0) at
/home/build/src/qemu/git/qemu/tcg/tcg.c:632
#7  0x004434a5 in tcg_temp_new_i32 () at
/home/build/src/qemu/git/qemu/tcg/tcg.h:807
#8  tcg_gen_andc_i32 (ret=0xa, arg1=0x7fffe63f3848, arg2=0x3f) at
/home/build/src/qemu/git/qemu/tcg/tcg-op.c:411
#9  0x005099ad in gen_op_arith_compute_ov (arg0=arg0@entry=0xd7,
arg1=arg1@entry=0x6a, arg2=0xd8, arg2@entry=0xa, sub=sub@entry=0,
ctx=)
at /home/build/src/qemu/git/qemu/target/ppc/translate.c:821
#10 0x005631b1 in gen_op_arith_add (compute_rc0=true,
compute_ov=true, compute_ca=true, add_ca=false, arg2=0xa, arg1=0x6a,
ret=0x3a, ctx=0x7fffe63f3800) at
/home/build/src/qemu/git/qemu/target/ppc/translate.c:895
#11 gen_addco (ctx=0x7fffe63f3800) at
/home/build/src/qemu/git/qemu/target/ppc/translate.c:931
#12 0x005796ba in gen_intermediate_code
(env=env@entry=0x77e282a0, tb=tb@entry=0x7fffe6db6a80) at
/home/build/src/qemu/git/qemu/target/ppc/translate.c:7287
#13 0x0043381a in tb_gen_code (cpu=cpu@entry=0x77e20010,
pc=pc@entry=1745420352, cs_base=cs_base@entry=0, flags=16432,
cflags=cflags@entry=0) at /home/build/src/qemu/git/qemu/translate-all.c:1281
#14 0x00435a32 in tb_find (tb_exit=0, last_tb=0x0,
cpu=0x77e20010) at /home/build/src/qemu/git/qemu/cpu-exec.c:370
#15 cpu_exec (cpu=cpu@entry=0x77e20010) at
/home/build/src/qemu/git/qemu/cpu-exec.c:685
#16 0x0047457e in tcg_cpu_exec (cpu=0x77e20010) at
/home/build/src/qemu/git/qemu/cpus.c:1251
#17 0x004748b4 in qemu_tcg_rr_cpu_thread_fn (arg=) at /home/build/src/qemu/git/qemu/cpus.c:1347
#18 0x72a50b50 in start_thread (arg=) at
pthread_create.c:304
#19 0x7279afbd in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#20 0x in ?? ()
(gdb)

git bisect points to the following commit:

commit dc0ad84449a4e2f28d2cc055998cb10c1a4d89a9
Author: Nikunj A Dadhania 
Date:   Mon Feb 27 10:27:57 2017 +0530

target/ppc: update overflow flags for add/sub

* SO and OV reflects overflow of the 64-bit result in 64-bit mode
  and overflow of the low-order 32-bit result in 32-bit mode

* OV32 reflects overflow of the low-order 32-bit independent of
  the mode

Signed-off-by: Nikunj A Dadhania 
Signed-off-by: David Gibson 

Interestingly enough if I recompile with CFLAGS="-O0 -g" to try and get
a full backtrace then the segfault goes away which suggests this could
be tickling a compiler bug somewhere - although even in this
configuration, I am seeing video artifacts during OS 9 boot which
suggests something still isn't quite right.

This is on a Debian wheezy x86_64 system with gcc 4.7.2.


ATB,

Mark.

[Qemu-devel] [Bug 597362] Re: qemu-system-sparc singlestep not work in gdbstub

[Mark Cave-Ayland]

I'm sure this has been fixed years ago (and qemu-system-sparc single-
step seems fine in local tests) so I'm marking this as "Fix released".


** Changed in: qemu
   Status: Expired => Fix Released

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

Title:
  qemu-system-sparc singlestep not work in gdbstub

Status in QEMU:
  Fix Released

Bug description:
  Debugging with gdb-stub does not work with qemu-system-sparc target

  Qemu compiled from current git tree.

  execution string: qemu-system-sparc.exe -s -S -m 256 -L Bios -hda
  sparc.img -boot c
  connect with telnet localhost 1234
  enter '$s#73' (without quotes, this is single step command to gdb stub)
  gdb stub reply '+' (without quotes, as it accept command)
  After this qemu continuously execute instructions in single step mode
  and does not exit to gdb stub after each executed instruction with
  interrupt signal
  ("T%02xthread:%02x;" /gdb_vm_state_change in gdbstub.c/ );

  If we look at target-sparc/translate.c, we can see that
  gen_helper_debug() is not called in single step mode:

  
  if ((pc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
  (npc & TARGET_PAGE_MASK) == (tb->pc & TARGET_PAGE_MASK) &&
  !s->singlestep)  {
  /* jump to same page: we can use a direct jump */
  tcg_gen_goto_tb(tb_num);
  tcg_gen_movi_tl(cpu_pc, pc);
  tcg_gen_movi_tl(cpu_npc, npc);
  tcg_gen_exit_tb((long)tb + tb_num);
  } else {
  /* jump to another page: currently not optimized */
  tcg_gen_movi_tl(cpu_pc, pc);
  tcg_gen_movi_tl(cpu_npc, npc);
  tcg_gen_exit_tb(0);
  }
  =

  
  /* if single step mode, we generate only one instruction and
 generate an exception */
  if (dc->singlestep) {
  break;
  }
  

  If we look similar code at target-sh4/translate.c we can see that is
  called in this cases:

  
  if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) &&
!ctx->singlestep_enabled) {
/* Use a direct jump if in same page and singlestep not enabled */
  tcg_gen_goto_tb(n);
  tcg_gen_movi_i32(cpu_pc, dest);
  tcg_gen_exit_tb((long) tb + n);
  } else {
  tcg_gen_movi_i32(cpu_pc, dest);
  if (ctx->singlestep_enabled)
  gen_helper_debug();
  tcg_gen_exit_tb(0);
  }
  

  
  if (tb->cflags & CF_LAST_IO)
  gen_io_end();
  if (env->singlestep_enabled) {
  tcg_gen_movi_i32(cpu_pc, ctx.pc);
  gen_helper_debug();
  } else {
  ==

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

Re: [Qemu-devel] Incorrect memory region address with large 64-bit PCI BARs

[Mark Cave-Ayland]

On 12/03/17 03:56, Michael S. Tsirkin wrote:

> After looking at it some more, I think the issue is merely with how info
> mtree presents information, which confuses instead of helping when
> overlap triggers. Specifically
> 01ff-01ff (prio 0, i/o): pci-mmio
>   ...
>   01fe0404-01fe04043fff (prio 1, i/o): virtio-pci
> 
> really means that virtio-pci is not visible at all, this
> happens because it starts at offset 0404 which is
> outside the parent.
> 
> I think that the cleanest fix is probably to show 128 bit addresses,
> then user will see the real addresses:
> 
> 01ff-01ff (prio 0, i/o): pci-mmio
>   ...
>   101fe0404-101fe04043fff (prio 1, i/o): virtio-pci
>   
> and now it's clear what is going on: virtio-pci is outside pci-mmio.
> 
> This would have pointed Mark in the right direction earlier.
> 
> Thoughts? Patch?

Presumably if someone tried to do this on real hardware, the BAR address
would lie outside of the pci-mmio region and effectively isn't mapped?

If this is the case I'd be happy with a simple qemu_log() showing the
full 64-bit address and region name explaining that it couldn't be
mapped underneath its parent because it was outside its parent region,
and skip the mapping.


ATB,

Mark.

[Qemu-devel] [Bug 1622547] Re: qemu-system-sparc fatal error Trap 0x29 on Solaris 2.6

[Mark Cave-Ayland]

The fix has now been applied to git master (with a CC to qemu-stable)
and so should appear in the upcoming 2.9.0 release as well as the stable
2.8.1 release.

** Changed in: qemu
   Status: New => Fix Committed

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

Title:
  qemu-system-sparc fatal error Trap 0x29 on Solaris 2.6

Status in QEMU:
  Fix Committed

Bug description:
  When trying to install Solaris 2.6 from original CDROM, qemu fail with
  the following error :

  qemu: fatal: Trap 0x29 while interrupts disabled, Error state
  pc: f0041280  npc: f0041284
  %g0-7:  f0281800 0800   f0243b88 0001 f0244020
  %o0-7: 40400ce2 40400ce2  404000e2 f0243b88  f023ffd8 
f0057914 
  %l0-7: 4cc2 f009645c f0096460 0002 0209 0004 0007 
f023ff90 
  %i0-7: 0042 404000e3  404000e3 e000 f028192a f0240038 
f0096448 
  %f00:     
  %f08:     
  %f16:     
  %f24:     
  psr: 40400cc2 (icc: -Z-- SPE: SP-) wim: 0002
  fsr:  y: 

  The command line was :

  qemu-system-sparc -nographic -bios ./openbios-sparc32 -M SS-20 -hda
  ./36G.disk -m 512 -cdrom Solaris_2.6_Software_05_98.img -boot d
  -serial telnet:0.0.0.0:3000,server -smp 2,cores=2 -monitor null

  It fails with a similar output when using bios ss20_v2.25_rom.

  ▶ qemu-system-sparc --version
  QEMU emulator version 2.7.0, Copyright (c) 2003-2016 Fabrice Bellard and the 
QEMU Project developers

  ▶ uname -a
  Linux xxx 4.7.1-1-ARCH #1 SMP PREEMPT Wed Aug 17 08:13:35 CEST 2016 x86_64 
GNU/Linux

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

Re: [Qemu-devel] Risu and the PowerPC 970

[Peter Maydell]

On 12 March 2017 at 06:10, G 3  wrote:
> Does Risu support the PowerPC 970? I tried compiling it but I saw this
> error:
>
> gcc  -Wall -D_GNU_SOURCE -DARCH=ppc64 -g  -o risu.o -c risu.c
> In file included from risu.c:29:
> risu.h:27:30: error: risu_reginfo_ppc64.h: No such file or directory
> make: *** [Makefile:44: risu.o] Error 1
>
> The PowerPC 970 is a 64-bit PowerPC CPU.

It doesn't support ppc64 bigendian yet, only little endian.
(There's some patches on list for the BE support which I
haven't yet got to.)

thanks
-- PMM

[Qemu-devel] [PATCH] memory: use 128 bit in info mtree

[Michael S. Tsirkin]

info mtree is doing 64 bit math to figure out
addresses from offsets, this does not work ncorrectly
incase of overflow.

Overflow usually indicates a guest bug, so this is unusual
but reporting correct addresses makes it easier to discover
what is going on.

Reported-by: Mark Cave-Ayland 
Cc: Paolo Bonzini 
Signed-off-by: Michael S. Tsirkin 
---
 include/qemu/int128.h | 15 +++
 memory.c  | 28 +---
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index 5c9890d..8be5328 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -302,4 +302,19 @@ static inline void int128_subfrom(Int128 *a, Int128 b)
 }
 
 #endif /* CONFIG_INT128 */
+
+#define INT128_FMT1_plx "0x%" PRIx64
+#define INT128_FMT2_plx "%015" PRIx64
+
+static inline uint64_t int128_printf1(Int128 a)
+{
+/* We assume 4 highest bits are clear and safe to ignore */
+return (int128_gethi(a) << 4) | (int128_getlo(a) >> 60);
+}
+
+static inline uint64_t int128_printf2(Int128 a)
+{
+return (int128_getlo(a) << 4) >> 4;
+}
+
 #endif /* INT128_H */
diff --git a/memory.c b/memory.c
index d61caee..b73a671 100644
--- a/memory.c
+++ b/memory.c
@@ -2487,13 +2487,14 @@ typedef QTAILQ_HEAD(queue, MemoryRegionList) 
MemoryRegionListHead;
 
 static void mtree_print_mr(fprintf_function mon_printf, void *f,
const MemoryRegion *mr, unsigned int level,
-   hwaddr base,
+   Int128 base,
MemoryRegionListHead *alias_print_queue)
 {
 MemoryRegionList *new_ml, *ml, *next_ml;
 MemoryRegionListHead submr_print_queue;
 const MemoryRegion *submr;
 unsigned int i;
+Int128 start, end;
 
 if (!mr) {
 return;
@@ -2503,6 +2504,9 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 mon_printf(f, MTREE_INDENT);
 }
 
+start = int128_add(base, int128_make64(mr->addr));
+end = int128_add(start, mr->size);
+
 if (mr->alias) {
 MemoryRegionList *ml;
 bool found = false;
@@ -2519,11 +2523,12 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 ml->mr = mr->alias;
 QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
 }
-mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
+mon_printf(f, INT128_FMT1_plx INT128_FMT2_plx
+  "-" INT128_FMT1_plx INT128_FMT2_plx
" (prio %d, %s): alias %s @%s " TARGET_FMT_plx
"-" TARGET_FMT_plx "%s\n",
-   base + mr->addr,
-   base + mr->addr + MR_SIZE(mr->size),
+   int128_printf1(start), int128_printf2(start),
+   int128_printf1(end), int128_printf2(end),
mr->priority,
memory_region_type((MemoryRegion *)mr),
memory_region_name(mr),
@@ -2532,10 +2537,11 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
mr->alias_offset + MR_SIZE(mr->size),
mr->enabled ? "" : " [disabled]");
 } else {
-mon_printf(f,
-   TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s\n",
-   base + mr->addr,
-   base + mr->addr + MR_SIZE(mr->size),
+mon_printf(f, INT128_FMT1_plx INT128_FMT2_plx
+  "-" INT128_FMT1_plx INT128_FMT2_plx
+   " (prio %d, %s): %s%s\n",
+   int128_printf1(start), int128_printf2(start),
+   int128_printf1(end), int128_printf2(end),
mr->priority,
memory_region_type((MemoryRegion *)mr),
memory_region_name(mr),
@@ -2562,7 +2568,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 }
 
 QTAILQ_FOREACH(ml, &submr_print_queue, queue) {
-mtree_print_mr(mon_printf, f, ml->mr, level + 1, base + mr->addr,
+mtree_print_mr(mon_printf, f, ml->mr, level + 1, start,
alias_print_queue);
 }
 
@@ -2620,14 +2626,14 @@ void mtree_info(fprintf_function mon_printf, void *f, 
bool flatview)
 
 QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
 mon_printf(f, "address-space: %s\n", as->name);
-mtree_print_mr(mon_printf, f, as->root, 1, 0, &ml_head);
+mtree_print_mr(mon_printf, f, as->root, 1, int128_zero(), &ml_head);
 mon_printf(f, "\n");
 }
 
 /* print aliased regions */
 QTAILQ_FOREACH(ml, &ml_head, queue) {
 mon_printf(f, "memory-region: %s\n", memory_region_name(ml->mr));
-mtree_print_mr(mon_printf, f, ml->mr, 1, 0, &ml_head);
+mtree_print_mr(mon_printf, f, ml->mr, 1, int128_zero(), &ml_head);
 mon_printf(f, "\n");
 }
 
-- 
MST

Re: [Qemu-devel] [PATCH] memory: use 128 bit in info mtree

[no-reply]

Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH] memory: use 128 bit in info mtree
Message-id: 1489345956-29167-1-git-send-email-...@redhat.com

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

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

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/1489345956-29167-1-git-send-email-...@redhat.com 
-> patchew/1489345956-29167-1-git-send-email-...@redhat.com
Switched to a new branch 'test'
d2f72b7 memory: use 128 bit in info mtree

=== OUTPUT BEGIN ===
Checking PATCH 1/1: memory: use 128 bit in info mtree...
ERROR: code indent should never use tabs
#79: FILE: memory.c:2527:
+^I^I   "-" INT128_FMT1_plx INT128_FMT2_plx$

ERROR: code indent should never use tabs
#98: FILE: memory.c:2541:
+^I^I   "-" INT128_FMT1_plx INT128_FMT2_plx$

total: 2 errors, 0 warnings, 97 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH] memory: use 128 bit in info mtree

[Peter Maydell]

On 12 March 2017 at 20:12, Michael S. Tsirkin  wrote:
> info mtree is doing 64 bit math to figure out
> addresses from offsets, this does not work ncorrectly
> incase of overflow.
>
> Overflow usually indicates a guest bug, so this is unusual
> but reporting correct addresses makes it easier to discover
> what is going on.
>
> Reported-by: Mark Cave-Ayland 
> Cc: Paolo Bonzini 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/qemu/int128.h | 15 +++
>  memory.c  | 28 +---
>  2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/include/qemu/int128.h b/include/qemu/int128.h
> index 5c9890d..8be5328 100644
> --- a/include/qemu/int128.h
> +++ b/include/qemu/int128.h
> @@ -302,4 +302,19 @@ static inline void int128_subfrom(Int128 *a, Int128 b)
>  }
>
>  #endif /* CONFIG_INT128 */
> +
> +#define INT128_FMT1_plx "0x%" PRIx64
> +#define INT128_FMT2_plx "%015" PRIx64
> +
> +static inline uint64_t int128_printf1(Int128 a)
> +{
> +/* We assume 4 highest bits are clear and safe to ignore */
> +return (int128_gethi(a) << 4) | (int128_getlo(a) >> 60);
> +}
> +
> +static inline uint64_t int128_printf2(Int128 a)
> +{
> +return (int128_getlo(a) << 4) >> 4;
> +}

I'm confused -- I was expecting these to just
be the two halves of the 128 bit integer to be
printed, but they seem to be doing something
more complicated...

thanks
-- PMM

[Qemu-devel] KVM call for 2017-03-14

[Juan Quintela]

Hi

Please, send any topic that you are interested in covering.

So far the agenda is:

- Direction of QEMU and toolstack in light of Google Cloud blog:
  
https://cloudplatform.googleblog.com/2017/01/7-ways-we-harden-our-KVM-hypervisor-at-Google-Cloud-security-in-plaintext.html




After discussions on the QEMU Summit, we are going to have always open a
KVM call where you can add topics.

 Call details:

By popular demand, a google calendar public entry with it

  
https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ

(Let me know if you have any problems with the calendar entry.  I just
gave up about getting right at the same time CEST, CET, EDT and DST).

If you need phone number details,  contact me privately

Thanks, Juan.

Re: [Qemu-devel] [PATCH V2] migration: expose qemu_announce_self() via qmp

[Germano Veit Michel]

On Fri, Mar 3, 2017 at 10:06 PM, Markus Armbruster  wrote:
>
> Please drop this line.
>
>>> +#
>>> +# Example:
>>> +#
>>> +# -> { "execute": "announce-self" }
>>> +# <- { "return": {} }
>>> +#
>>> +# Since: 2.9
>>> +##
>>> +{ 'command': 'announce-self' }
>>> +
>

Hi Markus,

Are you talking about the whole example section or just the empty line
at the end?

I'm preparing a V3 with hmp commands and g_new0.

Thanks,
Germano

Re: [Qemu-devel] [PATCH] memory: use 128 bit in info mtree

[Peter Xu]

On Sun, Mar 12, 2017 at 09:12:43PM +0200, Michael S. Tsirkin wrote:
> info mtree is doing 64 bit math to figure out
> addresses from offsets, this does not work ncorrectly
> incase of overflow.
> 
> Overflow usually indicates a guest bug, so this is unusual
> but reporting correct addresses makes it easier to discover
> what is going on.

A tiny issue would be that we will always dump 128 bits even if
nothing went wrong. IMHO That's slightly awkward. Not sure whether
that will confuse people since they should be thinking why we need
that on 64bit systems...

Do you like below one instead? It'll keep the old interface, but just
warn user explicity when something wrong happens, and it's much easier
and obvious imho (along with a tiny cleanup):

(the code is not tested even for compile)

-8<---
diff --git a/memory.c b/memory.c
index 284894b..64b0a60 100644
--- a/memory.c
+++ b/memory.c
@@ -2494,6 +2494,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 MemoryRegionListHead submr_print_queue;
 const MemoryRegion *submr;
 unsigned int i;
+hwaddr cur_start, cur_end;

 if (!mr) {
 return;
@@ -2503,6 +2504,18 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 mon_printf(f, MTREE_INDENT);
 }

+cur_start = base + mr->addr;
+cur_end = cur_start + MR_SIZE(mr->size);
+
+/*
+ * Try to detect overflow of memory ranges. This should never
+ * happen normally. When it happens, we dump something to warn the
+ * user who is observing this.
+ */
+if (cur_start < base || cur_end < cur_start) {
+mon_printf(f, "[DETECTED OVERFLOW!] ");
+}
+
 if (mr->alias) {
 MemoryRegionList *ml;
 bool found = false;
@@ -2522,8 +2535,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
" (prio %d, %s): alias %s @%s " TARGET_FMT_plx
"-" TARGET_FMT_plx "%s\n",
-   base + mr->addr,
-   base + mr->addr + MR_SIZE(mr->size),
+   cur_start, cur_end,
mr->priority,
memory_region_type((MemoryRegion *)mr),
memory_region_name(mr),
@@ -2534,8 +2546,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 } else {
 mon_printf(f,
TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s\n",
-   base + mr->addr,
-   base + mr->addr + MR_SIZE(mr->size),
+   cur_start, cur_end,
mr->priority,
memory_region_type((MemoryRegion *)mr),
memory_region_name(mr),
@@ -2562,7 +2573,7 @@ static void mtree_print_mr(fprintf_function mon_printf, 
void *f,
 }

 QTAILQ_FOREACH(ml, &submr_print_queue, queue) {
-mtree_print_mr(mon_printf, f, ml->mr, level + 1, base + mr->addr,
+mtree_print_mr(mon_printf, f, ml->mr, level + 1, cur_start,
alias_print_queue);
 }
->8---

Thanks,

-- peterx

[Qemu-devel] [PATCH 1/2] Revert "virtio: unbreak virtio-pci with IOMMU after caching ring translations"

[Jason Wang]

This reverts commit
96a8821d21411f10d77ea994af369c6e5c35a2cc. Following patch will come a
better solution which does not require a strict order between virtio
and IOMMU.

CC: Paolo Bonzini 
Signed-off-by: Jason Wang 
---
 hw/virtio/virtio-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index b76f3f6..5ce42af 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1153,7 +1153,7 @@ static AddressSpace *virtio_pci_get_dma_as(DeviceState *d)
 VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
 PCIDevice *dev = &proxy->pci_dev;
 
-return pci_device_iommu_address_space(dev);
+return pci_get_address_space(dev);
 }
 
 static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
-- 
2.7.4

[Qemu-devel] [PATCH 2/2] pci: introduce a bus master container

[Jason Wang]

96a8821d2141 ("virtio: unbreak virtio-pci with IOMMU after caching ring
translations") tries to make IOMMU works with virtio memory region
cache, but it requires IOMMU to be created before any virtio
devices. This is sub optimal, fixing this by introduce a bus master
container to make sure address space can be initialized during device
registering, and then we can safely set alias and make
bus_master_enable_region as its subregion during bus master
initialization.

Cc: Paolo Bonzini 
Signed-off-by: Jason Wang 
---
 hw/pci/pci.c | 9 +++--
 include/hw/pci/pci.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 273f1e4..ad46390 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -88,8 +88,8 @@ static void pci_init_bus_master(PCIDevice *pci_dev)
  OBJECT(pci_dev), "bus master",
  dma_as->root, 0, 
memory_region_size(dma_as->root));
 memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
-address_space_init(&pci_dev->bus_master_as,
-   &pci_dev->bus_master_enable_region, pci_dev->name);
+memory_region_add_subregion(&pci_dev->bus_master_container_region, 0,
+&pci_dev->bus_master_enable_region);
 }
 
 static void pcibus_machine_done(Notifier *notifier, void *data)
@@ -995,6 +995,11 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
 pci_dev->devfn = devfn;
 pci_dev->requester_id_cache = pci_req_id_cache_get(pci_dev);
 
+memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev),
+   "bus master container", UINT64_MAX);
+address_space_init(&pci_dev->bus_master_as,
+   &pci_dev->bus_master_container_region, pci_dev->name);
+
 if (qdev_hotplug) {
 pci_init_bus_master(pci_dev);
 }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 9349acb..713ede0 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -284,6 +284,7 @@ struct PCIDevice {
 char name[64];
 PCIIORegion io_regions[PCI_NUM_REGIONS];
 AddressSpace bus_master_as;
+MemoryRegion bus_master_container_region;
 MemoryRegion bus_master_enable_region;
 
 /* do not access the following fields */
-- 
2.7.4

Re: [Qemu-devel] [PATCH RFC 1/1] block: Handle NULL options correctly in raw_open

[Dong Jia Shi]

* Dong Jia Shi  [2017-03-08 17:31:05 +0800]:

> * Kevin Wolf  [2017-03-08 10:13:46 +0100]:
> 
> > Am 08.03.2017 um 03:15 hat Dong Jia Shi geschrieben:
> > > A normal call for raw_open should always pass in a non-NULL @options,
> > > but for some certain cases (e.g. trying to applying snapshot on a RBD
> > > image), they call raw_open with a NULL @options right after the calling
> > > for raw_close.
> > > 
> > > Let's take the NULL @options as a sign of trying to do raw_open again,
> > > and just simply return a success code.
> > > 
> > > Signed-off-by: Dong Jia Shi 
> > 
> > I think we rather need to fix bdrv_snapshot_goto() so that it doesn't
> > pass NULL, but the actual options that were given for the node (i.e.
> > bs->options).
> I've tried that before the current try. bs->options does not have the
> "file" key-value pair, so that leads to a fail too. Should we put "file"
> in to the options manually? I noticed that it was removed from
> bs->options during the calling of bdrv_open_inherit.
> 
Hi Kevin,

After thinking for quite some time, I still don't think we need to fix
the caller. The reason is that raw_close always does nothing, so no
matter what the caller passing in, raw_open should do nothing but just
return 0.

The following is another proposal. Looking forward for your comments.
Thanks,

diff --git a/block/raw-format.c b/block/raw-format.c
index 86fbc65..c309d4c 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -384,6 +384,11 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 BDRVRawState *s = bs->opaque;
 int ret;
 
+if (!bs->file) {
+return 0;
+}
+
+assert(options != NULL);
 bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
false, errp);
 if (!bs->file) {

> > 
> > Kevin
> > 
> 
> -- 
> Dong Jia

-- 
Dong Jia

[Qemu-devel] Swap disks virtualization

[Christopher Pereira]

We are currently providing Linux swap disks with QEMU driver, virtio bus 
and qcow2 type.


Do you know any alternatives that provide better performance?

Considering that swap disks contain less critical data, QEMU could avoid 
disk IOs by using unassigned memory whenever possible. This could also 
be handled by the underlying linux filesystem of course. Can we tell 
QEMU to use the linux swap disk for creating a guest swap disk? What's 
the state of the art regarding swap disks virtualization?


Best regards,

--

*J. Christopher Pereira*
IMATRONIX S.A.
www.imatronix.com

[Qemu-devel] [Bug 1256826] Re: INT instruction bug in WindowsXP

[Launchpad Bug Tracker]

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

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

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

Title:
  INT instruction bug in WindowsXP

Status in QEMU:
  Expired

Bug description:
  This bug is in -no-kvm mode.

  In windowsXP at IDT entry 2&8 is Task gate

  when application use INT 2 or INT 8 it will cause blue screen in XP.

  I found it should cause #GP but not generate hw interrupt.

  also I check this bug with -enable-kvm & my PC and works correctly.

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

[Qemu-devel] [PATCHv2] hw/net: implement MIB counters in mcf_fec driver

[Greg Ungerer]

The FEC ethernet hardware module used on ColdFire SoC parts contains a
block of RAM used to maintain hardware counters. This block is accessible
via the usual FEC register address space. There is currently no support
for this in the QEMU mcf_fec driver.

Add support for storing a MIB RAM block, and provide register level
access to it. Also implement a basic set of stats collection functions
to populate MIB data fields.

This support tested running a Linux target and using the net-tools
"ethtool -S" option. As of linux-4.9 the kernels FEC driver makes
accesses to the MIB counters during its initialization (which it never
did before), and so this version of Linux will now fail with the QEMU
error:

qemu: hardware error: mcf_fec_read: Bad address 0x200

This MIB counter support fixes this problem.

Signed-off-by: Greg Ungerer 
Reviewed-by: Laurent Vivier 
---
 hw/net/mcf_fec.c | 115 +++
 1 file changed, 115 insertions(+)

v2: clean up checkpatch issues

diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index a3eca7e..bfa6b4b 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -27,6 +27,7 @@ do { printf("mcf_fec: " fmt , ## __VA_ARGS__); } while (0)
 
 #define FEC_MAX_DESC 1024
 #define FEC_MAX_FRAME_SIZE 2032
+#define FEC_MIB_SIZE 64
 
 typedef struct {
 SysBusDevice parent_obj;
@@ -51,6 +52,7 @@ typedef struct {
 uint32_t erdsr;
 uint32_t etdsr;
 uint32_t emrbr;
+uint32_t mib[FEC_MIB_SIZE];
 } mcf_fec_state;
 
 #define FEC_INT_HB   0x8000
@@ -111,6 +113,63 @@ typedef struct {
 #define FEC_BD_OV   0x0002
 #define FEC_BD_TR   0x0001
 
+#define MIB_RMON_T_DROP 0
+#define MIB_RMON_T_PACKETS  1
+#define MIB_RMON_T_BC_PKT   2
+#define MIB_RMON_T_MC_PKT   3
+#define MIB_RMON_T_CRC_ALIGN4
+#define MIB_RMON_T_UNDERSIZE5
+#define MIB_RMON_T_OVERSIZE 6
+#define MIB_RMON_T_FRAG 7
+#define MIB_RMON_T_JAB  8
+#define MIB_RMON_T_COL  9
+#define MIB_RMON_T_P64  10
+#define MIB_RMON_T_P65TO127 11
+#define MIB_RMON_T_P128TO25512
+#define MIB_RMON_T_P256TO51113
+#define MIB_RMON_T_P512TO1023   14
+#define MIB_RMON_T_P1024TO2047  15
+#define MIB_RMON_T_P_GTE204816
+#define MIB_RMON_T_OCTETS   17
+#define MIB_IEEE_T_DROP 18
+#define MIB_IEEE_T_FRAME_OK 19
+#define MIB_IEEE_T_1COL 20
+#define MIB_IEEE_T_MCOL 21
+#define MIB_IEEE_T_DEF  22
+#define MIB_IEEE_T_LCOL 23
+#define MIB_IEEE_T_EXCOL24
+#define MIB_IEEE_T_MACERR   25
+#define MIB_IEEE_T_CSERR26
+#define MIB_IEEE_T_SQE  27
+#define MIB_IEEE_T_FDXFC28
+#define MIB_IEEE_T_OCTETS_OK29
+
+#define MIB_RMON_R_DROP 32
+#define MIB_RMON_R_PACKETS  33
+#define MIB_RMON_R_BC_PKT   34
+#define MIB_RMON_R_MC_PKT   35
+#define MIB_RMON_R_CRC_ALIGN36
+#define MIB_RMON_R_UNDERSIZE37
+#define MIB_RMON_R_OVERSIZE 38
+#define MIB_RMON_R_FRAG 39
+#define MIB_RMON_R_JAB  40
+#define MIB_RMON_R_RESVD_0  41
+#define MIB_RMON_R_P64  42
+#define MIB_RMON_R_P65TO127 43
+#define MIB_RMON_R_P128TO25544
+#define MIB_RMON_R_P256TO51145
+#define MIB_RMON_R_P512TO1023   46
+#define MIB_RMON_R_P1024TO2047  47
+#define MIB_RMON_R_P_GTE204848
+#define MIB_RMON_R_OCTETS   49
+#define MIB_IEEE_R_DROP 50
+#define MIB_IEEE_R_FRAME_OK 51
+#define MIB_IEEE_R_CRC  52
+#define MIB_IEEE_R_ALIGN53
+#define MIB_IEEE_R_MACERR   54
+#define MIB_IEEE_R_FDXFC55
+#define MIB_IEEE_R_OCTETS_OK56
+
 static void mcf_fec_read_bd(mcf_fec_bd *bd, uint32_t addr)
 {
 cpu_physical_memory_read(addr, bd, sizeof(*bd));
@@ -147,6 +206,31 @@ static void mcf_fec_update(mcf_fec_state *s)
 s->irq_state = active;
 }
 
+static void mcf_fec_tx_stats(mcf_fec_state *s, int size)
+{
+s->mib[MIB_RMON_T_PACKETS]++;
+s->mib[MIB_RMON_T_OCTETS] += size;
+if (size < 64) {
+s->mib[MIB_RMON_T_FRAG]++;
+} else if (size == 64) {
+s->mib[MIB_RMON_T_P64]++;
+} else if (size < 128) {
+s->mib[MIB_RMON_T_P65TO127]++;
+} else if (size < 256) {
+s->mib[MIB_RMON_T_P128TO255]++;
+} else if (size < 512) {
+s->mib[MIB_RMON_T_P256TO511]++;
+} else if (size < 1024) {
+s->mib[MIB_RMON_T_P512TO1023]++;
+} else if (size < 2048) {
+s->mib[MIB_RMON_T_P1024TO2047]++;
+} else {
+s->mib[MIB_RMON_T_P_GTE2048]++;
+}
+s->mib[MIB_IEEE_T_FRAME_OK]++;
+s->mib[MIB_IEEE_T_OCTETS_OK] += size;
+}
+
 static void mcf_fec_do_tx(mcf_fec_state *s)
 {
 uint32_t addr;
@@ -180,6 +264,7 @@ static void mcf_fec_do_tx(mcf_fec_state *s)
 /* Last buffer in frame.  */
 DPRINTF("Sending packet\n");
 qemu_send_packet(qemu_get_queue(s->nic), frame, frame_size);
+mcf_fec_tx_stats(s, frame_size);
 ptr = frame;
 frame_size = 0;
 s

Re: [Qemu-devel] [PATCH V2] migration: expose qemu_announce_self() via qmp

[Markus Armbruster]

Germano Veit Michel  writes:

> On Fri, Mar 3, 2017 at 10:06 PM, Markus Armbruster  wrote:
 diff --git a/qapi-schema.json b/qapi-schema.json
 index baa0d26..0d9bffd 100644
 --- a/qapi-schema.json
 +++ b/qapi-schema.json
 @@ -6080,3 +6080,21 @@
  #
  ##
  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
 +
 +##
 +# @announce-self:
 +#
 +# Trigger generation of broadcast RARP frames to update network switches.
 +# This can be useful when network bonds fail-over the active slave.
 +#
 +# Arguments: None.
>>
>> Please drop this line.
>>
 +#
 +# Example:
 +#
 +# -> { "execute": "announce-self" }
 +# <- { "return": {} }
 +#
 +# Since: 2.9
 +##
 +{ 'command': 'announce-self' }
 +
>>
>
> Hi Markus,
>
> Are you talking about the whole example section or just the empty line
> at the end?

The "Arguments: None" line.

[...]

Re: [Qemu-devel] qemu-system-ppc regression booting MacOS 9.2.1 image

[aNikunj A Dadhania]

Hi Mark,

Mark Cave-Ayland  writes:
> Hi Nikunj,
>
> Testing git master locally I see the following segfault when trying to
> boot my test MacOS 9.2.1 image:
>
>
> $ gdb --args ./qemu-system-ppc -bios
> /home/build/src/openbios/openbios.git/openbios/obj-ppc/openbios-qemu.elf.nostrip
> -cdrom /home/build/src/qemu/image/ppc/MacOS921.iso -boot d -m 512 -M mac99

Is it possible for you to share the iso image for MAC ?

> GNU gdb (GDB) 7.4.1-debian
> Copyright (C) 2012 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-linux-gnu".
> For bug reporting instructions, please see:
> ...
> Reading symbols from /home/build/rel-qemu-git/bin/qemu-system-ppc...done.
> (gdb) r
> Starting program: /home/build/rel-qemu-git/bin/qemu-system-ppc -bios
> /home/build/src/openbios/openbios.git/openbios/obj-ppc/openbios-qemu.elf.nostrip
> -cdrom /home/build/src/qemu/image/ppc/MacOS921.iso -boot d -m 512 -M mac99
> warning: no loadable sections found in added symbol-file system-supplied
> DSO at 0x77ffa000
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> [New Thread 0x7fffe9eee700 (LWP 29713)]
> [New Thread 0x7fffe6bf5700 (LWP 29714)]
> [New Thread 0x7fffe63f4700 (LWP 29715)]
> Trying to write invalid spr 0 (0x000) at 00f113c0
> Trying to read invalid spr 0 (0x000) at 00f113c8
> Trying to write privileged spr 955 (0x3bb) at 00f164b8
> Trying to write invalid spr 959 (0x3bf) at 00f16520
> Trying to read invalid spr 959 (0x3bf) at 00f16528
> Trying to write privileged spr 955 (0x3bb) at 00f164b8
> Trying to write invalid spr 959 (0x3bf) at 00f16520
> Trying to read invalid spr 959 (0x3bf) at 00f16528
>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fffe63f4700 (LWP 29715)]
> 0x77e20010 in ?? ()
> (gdb) bt
> #0  0x77e20010 in ?? ()
> #1  0x0002 in ?? ()
> #2  0x68090040 in ?? ()
> #3  0x0002 in ?? ()
> #4  0x6b67829c063b8d00 in ?? ()
> #5  0x6b67829c063b8d00 in ?? ()
> #6  0x0043986d in tcg_temp_new_internal_i32
> (temp_local=temp_local@entry=0) at
> /home/build/src/qemu/git/qemu/tcg/tcg.c:632
> #7  0x004434a5 in tcg_temp_new_i32 () at
> /home/build/src/qemu/git/qemu/tcg/tcg.h:807
> #8  tcg_gen_andc_i32 (ret=0xa, arg1=0x7fffe63f3848, arg2=0x3f) at
> /home/build/src/qemu/git/qemu/tcg/tcg-op.c:411
> #9  0x005099ad in gen_op_arith_compute_ov (arg0=arg0@entry=0xd7,
> arg1=arg1@entry=0x6a, arg2=0xd8, arg2@entry=0xa, sub=sub@entry=0,
> ctx=)
> at /home/build/src/qemu/git/qemu/target/ppc/translate.c:821
> #10 0x005631b1 in gen_op_arith_add (compute_rc0=true,
> compute_ov=true, compute_ca=true, add_ca=false, arg2=0xa, arg1=0x6a,
> ret=0x3a, ctx=0x7fffe63f3800) at
> /home/build/src/qemu/git/qemu/target/ppc/translate.c:895
> #11 gen_addco (ctx=0x7fffe63f3800) at
> /home/build/src/qemu/git/qemu/target/ppc/translate.c:931
> #12 0x005796ba in gen_intermediate_code
> (env=env@entry=0x77e282a0, tb=tb@entry=0x7fffe6db6a80) at
> /home/build/src/qemu/git/qemu/target/ppc/translate.c:7287
> #13 0x0043381a in tb_gen_code (cpu=cpu@entry=0x77e20010,
> pc=pc@entry=1745420352, cs_base=cs_base@entry=0, flags=16432,
> cflags=cflags@entry=0) at /home/build/src/qemu/git/qemu/translate-all.c:1281
> #14 0x00435a32 in tb_find (tb_exit=0, last_tb=0x0,
> cpu=0x77e20010) at /home/build/src/qemu/git/qemu/cpu-exec.c:370
> #15 cpu_exec (cpu=cpu@entry=0x77e20010) at
> /home/build/src/qemu/git/qemu/cpu-exec.c:685
> #16 0x0047457e in tcg_cpu_exec (cpu=0x77e20010) at
> /home/build/src/qemu/git/qemu/cpus.c:1251
> #17 0x004748b4 in qemu_tcg_rr_cpu_thread_fn (arg= out>) at /home/build/src/qemu/git/qemu/cpus.c:1347
> #18 0x72a50b50 in start_thread (arg=) at
> pthread_create.c:304
> #19 0x7279afbd in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
> #20 0x in ?? ()
> (gdb)
>
> git bisect points to the following commit:
>
> commit dc0ad84449a4e2f28d2cc055998cb10c1a4d89a9
> Author: Nikunj A Dadhania 
> Date:   Mon Feb 27 10:27:57 2017 +0530
>
> target/ppc: update overflow flags for add/sub
>
> * SO and OV reflects overflow of the 64-bit result in 64-bit mode
>   and overflow of the low-order 32-bit result in 32-bit mode
>
> * OV32 reflects overflow of the low-order 32-bit independent of
>   the mode
>
> Signed-off-by: Nikunj A Dadhania 
> Signed-off-by: David Gibson 
>
> Interestingly enough if I recompile with CFLAGS="-O0 -g" to try and get
> a full backtrace then the segfault goes away which suggests this could
> be tickling a compiler bug so

Re: [Qemu-devel] [PATCH v2] qmp-shell: add persistent command history

[Markus Armbruster]

Nir Soffer  writes:

> On Wed, Mar 8, 2017 at 8:29 AM, Markus Armbruster  wrote:
>> John Snow  writes:
>>
>>> On 03/07/2017 03:16 AM, Markus Armbruster wrote:
 John Snow  writes:

> On 03/06/2017 03:18 AM, Markus Armbruster wrote:
>> Nir Soffer  writes:
>>
>>> On Fri, Mar 3, 2017 at 9:29 PM, John Snow  wrote:


 On 03/03/2017 02:26 PM, Nir Soffer wrote:
> On Fri, Mar 3, 2017 at 8:54 PM, John Snow  wrote:
>> Use the existing readline history function we are utilizing
>> to provide persistent command history across instances of qmp-shell.
>>
>> This assists entering debug commands across sessions that may be
>> interrupted by QEMU sessions terminating, where the qmp-shell has
>> to be relaunched.
>>
>> Signed-off-by: John Snow 
>> ---
>>
>> v2: Adjusted the errors to whine about non-ENOENT errors, but still
>> intercept all errors as non-fatal.
>> Save history atexit() to match bash standard behavior
>>
>>  scripts/qmp/qmp-shell | 19 +++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
>> index 0373b24..55a8285 100755
>> --- a/scripts/qmp/qmp-shell
>> +++ b/scripts/qmp/qmp-shell
>> @@ -70,6 +70,9 @@ import json
>>  import ast
>>  import readline
>>  import sys
>> +import os
>> +import errno
>> +import atexit
>>
>>  class QMPCompleter(list):
>>  def complete(self, text, state):
>> @@ -109,6 +112,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>>  self._pretty = pretty
>>  self._transmode = False
>>  self._actions = list()
>> +self._histfile = os.path.join(os.path.expanduser('~'), 
>> '.qmp_history')
>>
>> I selfishly object to this filename, because I'm using it with
>>
>> $ socat UNIX:/work/armbru/images/test-qmp 
>> READLINE,history=$HOME/.qmp_history,prompt='QMP> '
>>
>> Just kidding.  But seriously, shouldn't this be named after the
>> *application* (qmp-shell) rather than the protocol (qmp)?
>>
>
> Seeing as the history itself is the qmp-shell syntax, you are correct.
>
> (Hey, it would be interesting to store the generated QMP into the
> qmp_history, though...!)

 Hah!  Saving it would be easy enough, but reloading it...  okay, call it
 a "backup" and declare victory when saving works.

>>
>>  def __get_address(self, arg):
>>  """
>> @@ -137,6 +141,21 @@ class QMPShell(qmp.QEMUMonitorProtocol):
>>  # XXX: default delimiters conflict with some command names 
>> (eg. query-),
>>  # clearing everything as it doesn't seem to matter
>>  readline.set_completer_delims('')
>> +try:
>> +readline.read_history_file(self._histfile)
>> +except Exception as e:
>> +if isinstance(e, IOError) and e.errno == errno.ENOENT:
>> +# File not found. No problem.
>> +pass
>> +else:
>> +print "Failed to read history '%s'; %s" % 
>> (self._histfile, e)
>
> I would handle only IOError, since any other error means a bug in 
> this code
> or in the underlying readline library, and the best way to handle 
> this is to
> let it fail loudly.
>

 Disagree. No reason to stop the shell from working because a QOL 
 feature
 didn't initialize correctly.

 The warning will be enough to solicit reports and fixes if necessary.
>>>
>>> I agree, the current solution is good tradeoff.
>>
>> For what it's worth, bash seems to silently ignore a history file it
>> can't read.  Tested by running "HISTFILE=xxx bash", then chmod 0 xxx, da
>> capo.
>>
>
> Right, done by example.
>
>>> One thing missing, is a call to readline.set_history_length, without
>>> it the history
>>> will grow without limit, see:
>>> https://docs.python.org/2/library/readline.html#readline.set_history_length
>>
>> Should this be addressed for 2.9?
>>
>
> You can add a limit if you want to; I don't have suggestions for which
> completely arbitrary limit makes sense, so I left it blank intentionally.

 For what it's worth, bash defaults HISTSIZE to 500.

 GNU Readline lets users configure it in ~/.inputrc.  Conditional
 configuration is possible, i.e. something like

 $if Qmp-shell
 set history-size 5000
 $endif

 should work, provided qmp-shell sets 

Re: [Qemu-devel] [PATCH] COLO-compare: Fix trace_event print bug

[Jason Wang]

On 2017年03月09日 15:40, Zhang Chen wrote:

Because of inet_ntoa() return a statically allocated buffer,
subsequent calls will overwrite, So we fix this bug.

Signed-off-by: Zhang Chen 
---
  net/colo-compare.c | 33 +
  1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 282727b..54e6d40 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -182,10 +182,18 @@ static int packet_enqueue(CompareState *s, int mode)
   */
  static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
  {
-trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
-   inet_ntoa(ppkt->ip->ip_dst), spkt->size,
-   inet_ntoa(spkt->ip->ip_src),
-   inet_ntoa(spkt->ip->ip_dst));
+if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
+
+strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
+strcpy(pri_ip_dst, inet_ntoa(ppkt->ip->ip_dst));
+strcpy(sec_ip_src, inet_ntoa(spkt->ip->ip_src));
+strcpy(sec_ip_dst, inet_ntoa(spkt->ip->ip_dst));
+
+trace_colo_compare_ip_info(ppkt->size, pri_ip_src,
+   pri_ip_dst, spkt->size,
+   sec_ip_src, sec_ip_dst);
+}
  
  if (ppkt->size == spkt->size) {

  return memcmp(ppkt->data + offset, spkt->data + offset,
@@ -336,10 +344,19 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet 
*ppkt)
  static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
  {
  trace_colo_compare_main("compare other");
-trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
-   inet_ntoa(ppkt->ip->ip_dst), spkt->size,
-   inet_ntoa(spkt->ip->ip_src),
-   inet_ntoa(spkt->ip->ip_dst));
+if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
+
+strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
+strcpy(pri_ip_dst, inet_ntoa(ppkt->ip->ip_dst));
+strcpy(sec_ip_src, inet_ntoa(spkt->ip->ip_src));
+strcpy(sec_ip_dst, inet_ntoa(spkt->ip->ip_dst));
+
+trace_colo_compare_ip_info(ppkt->size, pri_ip_src,
+   pri_ip_dst, spkt->size,
+   sec_ip_src, sec_ip_dst);
+}
+
  return colo_packet_compare_common(ppkt, spkt, 0);
  }
  


Applied, thanks.

Re: [Qemu-devel] [PATCHv2] hw/net: implement MIB counters in mcf_fec driver

[Jason Wang]

On 2017年03月13日 12:56, Greg Ungerer wrote:

The FEC ethernet hardware module used on ColdFire SoC parts contains a
block of RAM used to maintain hardware counters. This block is accessible
via the usual FEC register address space. There is currently no support
for this in the QEMU mcf_fec driver.

Add support for storing a MIB RAM block, and provide register level
access to it. Also implement a basic set of stats collection functions
to populate MIB data fields.

This support tested running a Linux target and using the net-tools
"ethtool -S" option. As of linux-4.9 the kernels FEC driver makes
accesses to the MIB counters during its initialization (which it never
did before), and so this version of Linux will now fail with the QEMU
error:

 qemu: hardware error: mcf_fec_read: Bad address 0x200

This MIB counter support fixes this problem.

Signed-off-by: Greg Ungerer
Reviewed-by: Laurent Vivier
---
  hw/net/mcf_fec.c | 115 +++
  1 file changed, 115 insertions(+)

v2: clean up checkpatch issues


Applied, thanks.

[Qemu-devel] [PATCH for-2.9 01/47] qapi: Factor QAPISchemaParser._include() out of .__init__()

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 45 +++--
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 53a4477..345cde1 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -268,34 +268,15 @@ class QAPISchemaParser(object):
 continue
 
 expr = self.get_expr(False)
-if isinstance(expr, dict) and "include" in expr:
+if 'include' in expr:
 if len(expr) != 1:
 raise QAPISemError(info, "Invalid 'include' directive")
 include = expr["include"]
 if not isinstance(include, str):
 raise QAPISemError(info,
"Value of 'include' must be a string")
-incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
-  include)
-# catch inclusion cycle
-inf = info
-while inf:
-if incl_abs_fname == os.path.abspath(inf['file']):
-raise QAPISemError(info, "Inclusion loop for %s"
-   % include)
-inf = inf['parent']
-
-# skip multiple include of the same file
-if incl_abs_fname in previously_included:
-continue
-try:
-fobj = open(incl_abs_fname, 'r')
-except IOError as e:
-raise QAPISemError(info, '%s: %s' % (e.strerror, include))
-exprs_include = QAPISchemaParser(fobj, previously_included,
- info)
-self.exprs.extend(exprs_include.exprs)
-self.docs.extend(exprs_include.docs)
+self._include(include, info, os.path.dirname(abs_fname),
+  previously_included)
 else:
 expr_elem = {'expr': expr,
  'info': info}
@@ -307,6 +288,26 @@ class QAPISchemaParser(object):
 
 self.exprs.append(expr_elem)
 
+def _include(self, include, info, base_dir, previously_included):
+incl_abs_fname = os.path.join(base_dir, include)
+# catch inclusion cycle
+inf = info
+while inf:
+if incl_abs_fname == os.path.abspath(inf['file']):
+raise QAPISemError(info, "Inclusion loop for %s" % include)
+inf = inf['parent']
+
+# skip multiple include of the same file
+if incl_abs_fname in previously_included:
+return
+try:
+fobj = open(incl_abs_fname, 'r')
+except IOError as e:
+raise QAPISemError(info, '%s: %s' % (e.strerror, include))
+exprs_include = QAPISchemaParser(fobj, previously_included, info)
+self.exprs.extend(exprs_include.exprs)
+self.docs.extend(exprs_include.docs)
+
 def accept(self, skip_comment=True):
 while True:
 self.tok = self.src[self.cursor]
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 08/47] tests/qapi-schema: Cover empty union base

[Markus Armbruster]

The new test case shows off qapi.py choking on an empty union base.

Signed-off-by: Markus Armbruster 
---
 tests/Makefile.include  |  1 +
 tests/qapi-schema/union-base-empty.err  | 10 ++
 tests/qapi-schema/union-base-empty.exit |  1 +
 tests/qapi-schema/union-base-empty.json |  9 +
 tests/qapi-schema/union-base-empty.out  |  0
 5 files changed, 21 insertions(+)
 create mode 100644 tests/qapi-schema/union-base-empty.err
 create mode 100644 tests/qapi-schema/union-base-empty.exit
 create mode 100644 tests/qapi-schema/union-base-empty.json
 create mode 100644 tests/qapi-schema/union-base-empty.out

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 736dd15..9f4e890 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -470,6 +470,7 @@ qapi-schema += unclosed-list.json
 qapi-schema += unclosed-object.json
 qapi-schema += unclosed-string.json
 qapi-schema += unicode-str.json
+qapi-schema += union-base-empty.json
 qapi-schema += union-base-no-discriminator.json
 qapi-schema += union-branch-case.json
 qapi-schema += union-clash-branches.json
diff --git a/tests/qapi-schema/union-base-empty.err 
b/tests/qapi-schema/union-base-empty.err
new file mode 100644
index 000..61e6ec6
--- /dev/null
+++ b/tests/qapi-schema/union-base-empty.err
@@ -0,0 +1,10 @@
+Traceback (most recent call last):
+  File "tests/qapi-schema/test-qapi.py", line 56, in 
+schema = QAPISchema(sys.argv[1])
+  File "scripts/qapi.py", line 1483, in __init__
+self.exprs = check_exprs(parser.exprs)
+  File "scripts/qapi.py", line 917, in check_exprs
+check_union(expr, info)
+  File "scripts/qapi.py", line 734, in check_union
+assert base_members
+AssertionError
diff --git a/tests/qapi-schema/union-base-empty.exit 
b/tests/qapi-schema/union-base-empty.exit
new file mode 100644
index 000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-base-empty.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-base-empty.json 
b/tests/qapi-schema/union-base-empty.json
new file mode 100644
index 000..d1843d3
--- /dev/null
+++ b/tests/qapi-schema/union-base-empty.json
@@ -0,0 +1,9 @@
+# Flat union with empty base and therefore without discriminator
+
+{ 'struct': 'Empty', 'data': { } }
+
+{ 'union': 'TestUnion',
+  'base': 'Empty',
+  'discriminator': 'type',
+  'data': { 'value1': 'int',
+'value2': 'str' } }
diff --git a/tests/qapi-schema/union-base-empty.out 
b/tests/qapi-schema/union-base-empty.out
new file mode 100644
index 000..e69de29
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 04/47] docs/qapi-code-gen.txt: Drop confusing reference to 'gen'

[Markus Armbruster]

Section "Commands" qualifies its rules on permitted argument and
return types "with one exception noted below when 'gen' is used".  The
note went away in commit 2d21291.  Clean up the dangling references.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 88de5c7..d9c1f91 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -555,22 +555,20 @@ The 'data' argument maps to the "arguments" dictionary 
passed in as
 part of a Client JSON Protocol command.  The 'data' member is optional
 and defaults to {} (an empty dictionary).  If present, it must be the
 string name of a complex type, or a dictionary that declares an
-anonymous type with the same semantics as a 'struct' expression, with
-one exception noted below when 'gen' is used.
+anonymous type with the same semantics as a 'struct' expression.
 
 The 'returns' member describes what will appear in the "return" member
 of a Client JSON Protocol reply on successful completion of a command.
 The member is optional from the command declaration; if absent, the
 "return" member will be an empty dictionary.  If 'returns' is present,
 it must be the string name of a complex or built-in type, a
-one-element array containing the name of a complex or built-in type,
-with one exception noted below when 'gen' is used.  Although it is
-permitted to have the 'returns' member name a built-in type or an
-array of built-in types, any command that does this cannot be extended
-to return additional information in the future; thus, new commands
-should strongly consider returning a dictionary-based type or an array
-of dictionaries, even if the dictionary only contains one member at the
-present.
+one-element array containing the name of a complex or built-in type.
+Although it is permitted to have the 'returns' member name a built-in
+type or an array of built-in types, any command that does this cannot
+be extended to return additional information in the future; thus, new
+commands should strongly consider returning a dictionary-based type or
+an array of dictionaries, even if the dictionary only contains one
+member at the present.
 
 All commands in Client JSON Protocol use a dictionary to report
 failure, with no way to specify that in QAPI.  Where the error return
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 09/47] qapi: Fix to reject empty union base gracefully

[Markus Armbruster]

Common Python pitfall: 'assert base_members' fires on [] in addition
to None.  Correct to 'assert base_members is not None'.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py|  2 +-
 tests/qapi-schema/union-base-empty.err | 11 +--
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e98fd0c..eec7bfb 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -731,7 +731,7 @@ def check_union(expr, info):
 raise QAPISemError(info, "Flat union '%s' must have a base"
% name)
 base_members = find_base_members(base)
-assert base_members
+assert base_members is not None
 
 # The value of member 'discriminator' must name a non-optional
 # member of the base struct.
diff --git a/tests/qapi-schema/union-base-empty.err 
b/tests/qapi-schema/union-base-empty.err
index 61e6ec6..7695806 100644
--- a/tests/qapi-schema/union-base-empty.err
+++ b/tests/qapi-schema/union-base-empty.err
@@ -1,10 +1 @@
-Traceback (most recent call last):
-  File "tests/qapi-schema/test-qapi.py", line 56, in 
-schema = QAPISchema(sys.argv[1])
-  File "scripts/qapi.py", line 1483, in __init__
-self.exprs = check_exprs(parser.exprs)
-  File "scripts/qapi.py", line 917, in check_exprs
-check_union(expr, info)
-  File "scripts/qapi.py", line 734, in check_union
-assert base_members
-AssertionError
+tests/qapi-schema/union-base-empty.json:5: Discriminator 'type' is not a 
member of base struct 'Empty'
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 02/47] qapi: Make doc comments optional where we don't need them

[Markus Armbruster]

Since we added the documentation generator in commit 3313b61, doc
comments are mandatory.  That's a very good idea for a schema that
needs to be documented, but has proven to be annoying for testing.

Make doc comments optional again, but add a new directive

{ 'pragma': { 'doc-required': true } }

to let a QAPI schema require them.

Require documentation in the schemas we actually want documented:
qapi-schema.json and qga/qapi-schema.json.

We could probably make qapi2texi.py cope with incomplete
documentation, but for now, simply make it refuse to run unless the
schema has 'doc-required': true.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt | 40 +-
 qapi-schema.json   |  2 ++
 qga/qapi-schema.json   |  2 ++
 scripts/qapi.py| 20 ++-
 scripts/qapi2texi.py   |  3 +++
 tests/Makefile.include |  1 +
 tests/qapi-schema/doc-missing.err  |  1 +
 tests/qapi-schema/doc-missing.exit |  1 +
 tests/qapi-schema/doc-missing.json |  5 +
 tests/qapi-schema/doc-missing.out  |  0
 10 files changed, 61 insertions(+), 14 deletions(-)
 create mode 100644 tests/qapi-schema/doc-missing.err
 create mode 100644 tests/qapi-schema/doc-missing.exit
 create mode 100644 tests/qapi-schema/doc-missing.json
 create mode 100644 tests/qapi-schema/doc-missing.out

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 9514d93..88de5c7 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -117,10 +117,13 @@ Example:
 
  Expression documentation 
 
-Each expression that isn't an include directive must be preceded by a
+Each expression that isn't an include directive may be preceded by a
 documentation block.  Such blocks are called expression documentation
 blocks.
 
+When documentation is required (see pragma 'doc-required'), expression
+documentation blocks are mandatory.
+
 The documentation block consists of a first line naming the
 expression, an optional overview, a description of each argument (for
 commands and events) or member (for structs, unions and alternates),
@@ -204,17 +207,17 @@ once.  It is permissible for the schema to contain 
additional types
 not used by any commands or events in the Client JSON Protocol, for
 the side effect of generated C code used internally.
 
-There are seven top-level expressions recognized by the parser:
-'include', 'command', 'struct', 'enum', 'union', 'alternate', and
-'event'.  There are several groups of types: simple types (a number of
-built-in types, such as 'int' and 'str'; as well as enumerations),
-complex types (structs and two flavors of unions), and alternate types
-(a choice between other types).  The 'command' and 'event' expressions
-can refer to existing types by name, or list an anonymous type as a
-dictionary. Listing a type name inside an array refers to a
-single-dimension array of that type; multi-dimension arrays are not
-directly supported (although an array of a complex struct that
-contains an array member is possible).
+There are eight top-level expressions recognized by the parser:
+'include', 'pragma', 'command', 'struct', 'enum', 'union',
+'alternate', and 'event'.  There are several groups of types: simple
+types (a number of built-in types, such as 'int' and 'str'; as well as
+enumerations), complex types (structs and two flavors of unions), and
+alternate types (a choice between other types).  The 'command' and
+'event' expressions can refer to existing types by name, or list an
+anonymous type as a dictionary. Listing a type name inside an array
+refers to a single-dimension array of that type; multi-dimension
+arrays are not directly supported (although an array of a complex
+struct that contains an array member is possible).
 
 All names must begin with a letter, and contain only ASCII letters,
 digits, hyphen, and underscore.  There are two exceptions: enum values
@@ -282,7 +285,7 @@ The following types are predefined, and map to C as follows:
   QType QType  JSON string matching enum QType values
 
 
-=== Includes ===
+=== Include directives ===
 
 Usage: { 'include': STRING }
 
@@ -302,6 +305,17 @@ an outer file.  The parser may be made stricter in the 
future to
 prevent incomplete include files.
 
 
+=== Pragma directives ===
+
+Usage: { 'pragma': DICT }
+
+The pragma directive lets you control optional generator behavior.
+The dictionary's entries are pragma names and values.
+
+Pragma 'doc-required' takes a boolean value.  If true, documentation
+is required.  Default is false.
+
+
 === Struct types ===
 
 Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME }
diff --git a/qapi-schema.json b/qapi-schema.json
index 32b4a4b..d5438ee 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -49,6 +49,8 @@
 #
 ##
 
+{ 'pragma': { 'doc-required': true } }
+
 # QAPI common definitions
 { 'include': 'qapi/common.json' }
 
diff --git a/qga/qapi-schema.json b

[Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Markus Armbruster]

I'm proposing this is 2.9 because it fixes a documentation regression.
It affects only documentation; generated C code is unchanged except
for the removal of trailing space in PATCH 46.

Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.

Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
the QAPI schema and generate their replacements from the schema
(commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
also was a step back: the documentation lost information on JSON
types, because I didn't like Marc-André's patch to add it.  He
reposted it for further review afterwards:

Subject: [PATCH 0/2] qapi2texi: add type information
Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html

His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
descriptions in a new formal language to the generated documentation.
Quoting the commit message:

Array types have the following syntax: type[]. Ex: str[].

- Struct, commands and events use the following members syntax:

  { 'member': type, ('foo': str), ... }

Optional members are under parentheses.

A structure with a base type will have 'BaseStruct +' prepended.

- Alternates use the following syntax:

  [ 'foo': type, 'bar': type, ... ]

- Simple unions use the following syntax:

  { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] }

- Flat unions use the following syntax:

  BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]

End quote.  Looks like this in generated documentation:

 -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
  VncBasicInfo}

 Emitted when a VNC client establishes a connection
 ''server''
  server information
 ''client''
  client information

 Note: This event is emitted before any authentication takes place,
 thus the authentication ID is not provided
[...]

 -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}

 The network connection information for server
 ''auth'' (optional)
  authentication method used for the plain (non-websocket) VNC
  server

 Since: 2.1

 -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = ['inet':
  InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
  VsockSocketAddress, 'fd': String] }

 Captures the address of a socket, which could also be a named file
 descriptor

 Since: 1.3

Here's my counter-proposal: instead of inventing a formal language,
fix the natural language documentation to actually mention *all*
members, and add type information in a plain, easy-to-understand way.
Looks like this:

 -- Event: VNC_CONNECTED

 Emitted when a VNC client establishes a connection

 Arguments:
 'server: VncServerInfo'
  server information
 'client: VncBasicInfo'
  client information

 Note: This event is emitted before any authentication takes place,
 thus the authentication ID is not provided
[...]

 -- Object: VncServerInfo

 The network connection information for server

 Members:
 'auth: string' (optional)
  authentication method used for the plain (non-websocket) VNC
  server
 The members of 'VncBasicInfo'

 Since: 2.1

 -- Object: SocketAddress

 Captures the address of a socket, which could also be a named file
 descriptor

 Members:
 'type'
  One of "inet", "unix", "vsock", "fd"
 'data: InetSocketAddress' when 'type' is "inet"
 'data: UnixSocketAddress' when 'type' is "unix"
 'data: VsockSocketAddress' when 'type' is "vsock"
 'data: String' when 'type' is "fd"

 Since: 1.3

Additionally, my series fixes a number of bugs and cleans up along the
way.  In particular, it converts qapi2texi.py from parse trees to the
visitor interface the other generators use.

Future generated documentation work includes eliding types that aren't
visible in QMP (like introspection does), and making uses of type
names links in HTML.

Markus Armbruster (47):
  qapi: Factor QAPISchemaParser._include() out of .__init__()
  qapi: Make doc comments optional where we don't need them
  qapi: Back out doc comments added just to please qapi.py
  docs/qapi-code-gen.txt: Drop confusing reference to 'gen'
  qapi: Have each QAPI schema declare its returns white-list
  qapi: Have each QAPI schema declare its name rule violations
  qapi: Clean up build of generated documentation
  tests/qapi-schema: Cover empty union base
  qapi: Fix to reject empty union base gracefully
  qapi2texi: Fix up output around #optional
  qapi: Avoid unwanted blank lines in QAPIDoc
  qapi/rocker: Fix up doc comment notes on optional members
  qapi: Fix QAPISchemaEnumType.is_implicit() for 'QType'
  qapi: Prepare for requiring more complete documentation
  qapi: Conjure up QAPIDoc.ArgSection for undocumented members
 

[Qemu-devel] [PATCH for-2.9 13/47] qapi: Fix QAPISchemaEnumType.is_implicit() for 'QType'

[Markus Armbruster]

Missed in commit 7264f5c.  Harmless, because nothing checks whether an
enumeration type is implicit so far.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e6d023f..7a2b6ab 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1148,8 +1148,8 @@ class QAPISchemaEnumType(QAPISchemaType):
 v.check_clash(self.info, seen)
 
 def is_implicit(self):
-# See QAPISchema._make_implicit_enum_type()
-return self.name.endswith('Kind')
+# See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
+return self.name.endswith('Kind') or self.name == 'QType'
 
 def c_type(self):
 return c_name(self.name)
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 10/47] qapi2texi: Fix up output around #optional

[Markus Armbruster]

We use tag #optional to mark optional members, like this:

# @name: #optional The name of the guest

texi_body() strips #optional, but not whitespace around it.  For the
above, we get in qemu-qmp-qapi.texi

@item @code{'name'} (optional)
 The name of the guest
@end table

The extra space can lead to artifacts in output, e.g in
qemu-qmp-ref.7.pod

=item C<'name'> (optional)

 The name of the guest

and then in qemu-qmp-ref.7

.IX Item "name (optional)"
.Vb 1
\& The name of the guest
.Ve

instead of intended plain

.IX Item "name (optional)"
The name of the guest

Get rid of these artifacts by removing whitespace around #optional
along with it.

This turns three minus signs in qapi-schema.json into markup, because
they're now at the beginning of the line.  Drop them, they're unwanted
there.

Signed-off-by: Markus Armbruster 
---
 qapi-schema.json | 6 +++---
 scripts/qapi2texi.py | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 17c766e..52141cd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3779,11 +3779,11 @@
 #
 # @dstport: #optional destination port - mandatory for udp, optional for ip
 #
-# @ipv6: #optional - force the use of ipv6
+# @ipv6: #optional force the use of ipv6
 #
-# @udp: #optional - use the udp version of l2tpv3 encapsulation
+# @udp: #optional use the udp version of l2tpv3 encapsulation
 #
-# @cookie64: #optional - use 64 bit coookies
+# @cookie64: #optional use 64 bit coookies
 #
 # @counter: #optional have sequence counter
 #
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 06d6abf..0f3e573 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -137,7 +137,8 @@ def texi_body(doc):
 desc = str(section)
 opt = ''
 if "#optional" in desc:
-desc = desc.replace("#optional", "")
+desc = re.sub(r'^ *#optional *\n?|\n? *#optional *$|#optional',
+  '', desc)
 opt = ' (optional)'
 body += "@item @code{'%s'}%s\n%s\n" % (arg, opt,
texi_format(desc))
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 11/47] qapi: Avoid unwanted blank lines in QAPIDoc

[Markus Armbruster]

We silently fix missing #optional tags for QAPIDoc by appending a line
"#optional" to the section's .content.  However, this interferes with
.__repr__ stripping trailing blank lines from .content.

Use new ArgSection instance variable .optional instead, and leave
.content alone.

To permit testing .optional in texi_body(), clean up texi_enum()'s
hack to add empty documentation for undocumented enum values: add an
ArgSection instead of ''.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py  | 5 +++--
 scripts/qapi2texi.py | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index eec7bfb..e6d023f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -107,6 +107,7 @@ class QAPIDoc(object):
 self.name = name
 # the list of lines for this section
 self.content = []
+self.optional = False
 
 def append(self, line):
 self.content.append(line)
@@ -978,15 +979,15 @@ def check_definition_doc(doc, expr, info):
 desc = doc.args.get(arg)
 if not desc:
 continue
+desc.optional = opt
 desc_opt = "#optional" in str(desc)
 if desc_opt and not opt:
 raise QAPISemError(info, "Description has #optional, "
"but the declaration doesn't")
 if not desc_opt and opt:
-# silently fix the doc
 # TODO either fix the schema and make this an error,
 # or drop #optional entirely
-desc.append("#optional")
+pass
 
 doc_args = set(doc.args.keys())
 args = set([name.strip('*') for name in args])
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 0f3e573..0aaf45c 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -136,7 +136,7 @@ def texi_body(doc):
 for arg, section in doc.args.iteritems():
 desc = str(section)
 opt = ''
-if "#optional" in desc:
+if section.optional:
 desc = re.sub(r'^ *#optional *\n?|\n? *#optional *$|#optional',
   '', desc)
 opt = ' (optional)'
@@ -185,7 +185,7 @@ def texi_enum(expr, doc):
 """Format an enum to texi"""
 for i in expr['data']:
 if i not in doc.args:
-doc.args[i] = ''
+doc.args[i] = qapi.QAPIDoc.ArgSection(i)
 body = texi_body(doc)
 return TYPE_FMT(type="Enum",
 name=doc.symbol,
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 18/47] qapi: Use raw strings for regular expressions consistently

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index c580e76..6d39ec9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -550,7 +550,7 @@ def discriminator_find_enum_define(expr):
 # Names must be letters, numbers, -, and _.  They must start with letter,
 # except for downstream extensions which must start with __RFQDN_.
 # Dots are only valid in the downstream extension prefix.
-valid_name = re.compile('^(__[a-zA-Z0-9.-]+_)?'
+valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?'
 '[a-zA-Z][a-zA-Z0-9_-]*$')
 
 
@@ -1827,10 +1827,10 @@ def cgen(code, **kwds):
 if indent_level:
 indent = genindent(indent_level)
 # re.subn() lacks flags support before Python 2.7, use re.compile()
-raw = re.subn(re.compile("^.", re.MULTILINE),
+raw = re.subn(re.compile(r'^.', re.MULTILINE),
   indent + r'\g<0>', raw)
 raw = raw[0]
-return re.sub(re.escape(eatspace) + ' *', '', raw)
+return re.sub(re.escape(eatspace) + r' *', '', raw)
 
 
 def mcgen(code, **kwds):
@@ -1964,7 +1964,7 @@ def parse_command_line(extra_options="", 
extra_long_options=[]):
 for oa in opts:
 o, a = oa
 if o in ("-p", "--prefix"):
-match = re.match('([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
+match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
 if match.end() != len(a):
 print >>sys.stderr, \
 "%s: 'funny character '%s' in argument of --prefix" \
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 06/47] qapi: Have each QAPI schema declare its name rule violations

[Markus Armbruster]

qapi.py has a hardcoded white-list of type names that may violate the
rule on use of upper and lower case.  Add a new pragma directive
'name-case-whitelist', and use it to replace the hard-coded
white-list.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt  |  6 ++
 qapi-schema.json| 11 ++-
 scripts/qapi.py | 22 ++
 tests/qapi-schema/enum-member-case.err  |  2 +-
 tests/qapi-schema/enum-member-case.json |  1 +
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index e907e57..349dc02 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -252,6 +252,9 @@ Any name (command, event, type, member, or enum value) 
beginning with
 "x-" is marked experimental, and may be withdrawn or changed
 incompatibly in a future release.
 
+Pragma 'name-case-whitelist' lets you violate the rules on use of
+upper and lower case.  Use for new code is strongly discouraged.
+
 In the rest of this document, usage lines are given for each
 expression type, with literal strings written in lower case and
 placeholders written in capitals.  If a literal string includes a
@@ -318,6 +321,9 @@ is required.  Default is false.
 Pragma 'returns-whitelist' takes a list of command names that may
 violate the rules on permitted return types.  Default is none.
 
+Pragma 'name-case-whitelist' takes a list of names that may violate
+rules on use of upper- vs. lower-case letters.  Default is none.
+
 
 === Struct types ===
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 93e9e98..17c766e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -61,7 +61,16 @@
 'query-migrate-cache-size',
 'query-tpm-models',
 'query-tpm-types',
-'ringbuf-read' ] } }
+'ringbuf-read' ],
+'name-case-whitelist': [
+'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
+'CpuInfoMIPS',  # PC, visible through query-cpu
+'CpuInfoTricore',   # PC, visible through query-cpu
+'QapiErrorClass',   # all members, visible through errors
+'UuidInfo', # UUID, visible through query-uuid
+'X86CPURegister32', # all members, visible indirectly through 
qom-get
+'q_obj_CpuInfo-base'# CPU, visible through query-cpu
+] } }
 
 # QAPI common definitions
 { 'include': 'qapi/common.json' }
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a90b682..e98fd0c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -44,16 +44,7 @@ doc_required = False
 returns_whitelist = []
 
 # Whitelist of entities allowed to violate case conventions
-case_whitelist = [
-# From QMP:
-'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
-'CpuInfoMIPS',  # PC, visible through query-cpu
-'CpuInfoTricore',   # PC, visible through query-cpu
-'QapiErrorClass',   # all members, visible through errors
-'UuidInfo', # UUID, visible through query-uuid
-'X86CPURegister32', # all members, visible indirectly through qom-get
-'q_obj_CpuInfo-base',   # CPU, visible through query-cpu
-]
+name_case_whitelist = []
 
 enum_types = []
 struct_types = []
@@ -298,7 +289,7 @@ class QAPISchemaParser(object):
 self.docs.extend(exprs_include.docs)
 
 def _pragma(self, name, value, info):
-global doc_required, returns_whitelist
+global doc_required, returns_whitelist, name_case_whitelist
 if name == 'doc-required':
 if not isinstance(value, bool):
 raise QAPISemError(info,
@@ -311,6 +302,13 @@ class QAPISchemaParser(object):
"Pragma returns-whitelist must be"
" a list of strings")
 returns_whitelist = value
+elif name == 'name-case-whitelist':
+if (not isinstance(value, list)
+or any([not isinstance(elt, str) for elt in value])):
+raise QAPISemError(info,
+   "Pragma name-case-whitelist must be"
+   " a list of strings")
+name_case_whitelist = value
 else:
 raise QAPISemError(info, "Unknown pragma '%s'" % name)
 
@@ -1283,7 +1281,7 @@ class QAPISchemaMember(object):
 
 def check_clash(self, info, seen):
 cname = c_name(self.name)
-if cname.lower() != cname and self.owner not in case_whitelist:
+if cname.lower() != cname and self.owner not in name_case_whitelist:
 raise QAPISemError(info,
"%s should not use uppercase" % self.describe())
 if cname in seen:
diff --git a/tests/qapi-schema/enum-member-case.err 
b/tests/qapi-schema/enum-member-case.err
index b652e9a..3c67a3a 100644
--- a/tests/qapi-schema/enum-member-case.err
+++ b/tests/qapi-sche

[Qemu-devel] [PATCH for-2.9 05/47] qapi: Have each QAPI schema declare its returns white-list

[Markus Armbruster]

qapi.py has a hardcoded white-list of command names that may violate
the rules on permitted return types.  Add a new pragma directive
'returns-whitelist', and use it to replace the hard-coded white-list.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt   | 13 +++--
 qapi-schema.json | 12 
 qga/qapi-schema.json | 15 +++
 scripts/qapi.py  | 30 +-
 tests/qapi-schema/qapi-schema-test.json  |  7 +++
 tests/qapi-schema/returns-whitelist.err  |  2 +-
 tests/qapi-schema/returns-whitelist.json |  4 
 7 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index d9c1f91..e907e57 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -315,6 +315,9 @@ The dictionary's entries are pragma names and values.
 Pragma 'doc-required' takes a boolean value.  If true, documentation
 is required.  Default is false.
 
+Pragma 'returns-whitelist' takes a list of command names that may
+violate the rules on permitted return types.  Default is none.
+
 
 === Struct types ===
 
@@ -563,12 +566,10 @@ The member is optional from the command declaration; if 
absent, the
 "return" member will be an empty dictionary.  If 'returns' is present,
 it must be the string name of a complex or built-in type, a
 one-element array containing the name of a complex or built-in type.
-Although it is permitted to have the 'returns' member name a built-in
-type or an array of built-in types, any command that does this cannot
-be extended to return additional information in the future; thus, new
-commands should strongly consider returning a dictionary-based type or
-an array of dictionaries, even if the dictionary only contains one
-member at the present.
+To return anything else, you have to list the command in pragma
+'returns-whitelist'.  If you do this, the command cannot be extended
+to return additional information in the future.  Use of
+'returns-whitelist' for new commands is strongly discouraged.
 
 All commands in Client JSON Protocol use a dictionary to report
 failure, with no way to specify that in QAPI.  Where the error return
diff --git a/qapi-schema.json b/qapi-schema.json
index d5438ee..93e9e98 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -51,6 +51,18 @@
 
 { 'pragma': { 'doc-required': true } }
 
+# Whitelists to permit QAPI rule violations; think twice before you
+# add to them!
+{ 'pragma': {
+# Commands allowed to return a non-dictionary:
+'returns-whitelist': [
+'human-monitor-command',
+'qom-get',
+'query-migrate-cache-size',
+'query-tpm-models',
+'query-tpm-types',
+'ringbuf-read' ] } }
+
 # QAPI common definitions
 { 'include': 'qapi/common.json' }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 3f3d428..a8e4bda 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -13,6 +13,21 @@
 
 { 'pragma': { 'doc-required': true } }
 
+# Whitelists to permit QAPI rule violations; think twice before you
+# add to them!
+{ 'pragma': {
+# Commands allowed to return a non-dictionary:
+'returns-whitelist': [
+'guest-file-open',
+'guest-fsfreeze-freeze',
+'guest-fsfreeze-freeze-list',
+'guest-fsfreeze-status',
+'guest-fsfreeze-thaw',
+'guest-get-time',
+'guest-set-vcpus',
+'guest-sync',
+'guest-sync-delimited' ] } }
+
 ##
 # @guest-sync-delimited:
 #
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 29a8b77..a90b682 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -41,26 +41,7 @@ builtin_types = {
 doc_required = False
 
 # Whitelist of commands allowed to return a non-dictionary
-returns_whitelist = [
-# From QMP:
-'human-monitor-command',
-'qom-get',
-'query-migrate-cache-size',
-'query-tpm-models',
-'query-tpm-types',
-'ringbuf-read',
-
-# From QGA:
-'guest-file-open',
-'guest-fsfreeze-freeze',
-'guest-fsfreeze-freeze-list',
-'guest-fsfreeze-status',
-'guest-fsfreeze-thaw',
-'guest-get-time',
-'guest-set-vcpus',
-'guest-sync',
-'guest-sync-delimited',
-]
+returns_whitelist = []
 
 # Whitelist of entities allowed to violate case conventions
 case_whitelist = [
@@ -317,12 +298,19 @@ class QAPISchemaParser(object):
 self.docs.extend(exprs_include.docs)
 
 def _pragma(self, name, value, info):
-global doc_required
+global doc_required, returns_whitelist
 if name == 'doc-required':
 if not isinstance(value, bool):
 raise QAPISemError(info,
"Pragma 'doc-required' must be boolean")
 doc_required = value
+elif name == 'returns-whitelist':
+if (not isinstance(value, list)
+or any([not isinstance(elt, str) for elt in value]))

[Qemu-devel] [PATCH for-2.9 26/47] qapi2texi: Generate reference to base type members

[Markus Armbruster]

The generated documentation doesn't mention object type members
inherited from a base type.  Fix that.

Example change (qemu-qmp-ref.txt):

  -- Struct: VncServerInfo

  The network connection information for server

  Members:
  'auth' (optional)
   authentication method used for the plain (non-websocket) VNC
   server
+ The members of 'VncBasicInfo'

  Since: 2.1

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 993b652..7083d0c 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -143,7 +143,7 @@ def texi_member(member):
 ' (optional)' if member.optional else '')
 
 
-def texi_members(doc, what, member_func):
+def texi_members(doc, what, base, member_func):
 """Format the table of members"""
 items = ''
 for section in doc.args.itervalues():
@@ -152,6 +152,8 @@ def texi_members(doc, what, member_func):
 else:
 desc = 'Not documented'
 items += member_func(section.member) + texi_format(desc) + '\n'
+if base:
+items += '@item The members of @code{%s}\n' % base.doc_type()
 if not items:
 return ''
 return '\n@b{%s:}\n@table @asis\n%s@end table\n' % (what, items)
@@ -174,9 +176,9 @@ def texi_sections(doc):
 return body
 
 
-def texi_entity(doc, what, member_func=texi_member):
+def texi_entity(doc, what, base=None, member_func=texi_member):
 return (texi_body(doc)
-+ texi_members(doc, what, member_func)
++ texi_members(doc, what, base, member_func)
 + texi_sections(doc))
 
 
@@ -205,11 +207,13 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 typ = 'Flat Union'
 else:
 typ = 'Simple Union'
+if base and base.is_implicit():
+base = None
 if self.out:
 self.out += '\n'
 self.out += TYPE_FMT(type=typ,
  name=doc.symbol,
- body=texi_entity(doc, 'Members'))
+ body=texi_entity(doc, 'Members', base))
 
 def visit_alternate_type(self, name, info, variants):
 doc = self.cur_doc
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 07/47] qapi: Clean up build of generated documentation

[Markus Armbruster]

Rename intermediate qemu-qapi.texi to qemu-qmp-qapi.texi to match its
user qemu-qmp-ref.texi, just like qemu-ga-qapi.texi matches
qemu-ga-ref.texi.

Build the intermediate .texi next to the sources and the final output
in docs/ instead of dumping them into the build root.

Fix version.texi dependencies so that only the targets that actually
need it depend on it.

Signed-off-by: Markus Armbruster 
---
 .gitignore | 10 +-
 Makefile   | 27 +++
 docs/qemu-qmp-ref.texi |  2 +-
 rules.mak  |  2 +-
 4 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/.gitignore b/.gitignore
index 2849d75..0e99e6a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -103,11 +103,11 @@
 /docs/qemu-ga-ref.txt
 /docs/qemu-qmp-ref.html
 /docs/qemu-qmp-ref.txt
-docs/qemu-ga-ref.info*
-docs/qemu-qmp-ref.info*
-/qemu-ga-qapi.texi
-/qemu-qapi.texi
-/version.texi
+/docs/qemu-ga-ref.info*
+/docs/qemu-qmp-ref.info*
+/docs/qemu-ga-qapi.texi
+/docs/qemu-qmp-qapi.texi
+/docs/version.texi
 *.tps
 .stgit-*
 cscope.*
diff --git a/Makefile b/Makefile
index 1c4c04f..35d32ee 100644
--- a/Makefile
+++ b/Makefile
@@ -516,7 +516,7 @@ distclean: clean
rm -f qemu-doc.vr qemu-doc.txt
rm -f config.log
rm -f linux-headers/asm
-   rm -f qemu-ga-qapi.texi qemu-qapi.texi version.texi
+   rm -f docs/qemu-ga-qapi.texi docs/qemu-qmp-qapi.texi docs/version.texi
rm -f docs/qemu-qmp-ref.7 docs/qemu-ga-ref.7
rm -f docs/qemu-qmp-ref.txt docs/qemu-ga-ref.txt
rm -f docs/qemu-qmp-ref.pdf docs/qemu-ga-ref.pdf
@@ -663,25 +663,28 @@ ui/console-gl.o: $(SRC_PATH)/ui/console-gl.c \
 
 # documentation
 MAKEINFO=makeinfo
-MAKEINFOFLAGS=--no-split --number-sections
+MAKEINFOFLAGS=--no-split --number-sections -I docs
 TEXIFLAG=$(if $(V),,--quiet)
 
-version.texi: $(SRC_PATH)/VERSION
+docs/version.texi: $(SRC_PATH)/VERSION
$(call quiet-command,echo "@set VERSION $(VERSION)" > $@,"GEN","$@")
 
-%.html: %.texi version.texi
+%.html: %.texi
$(call quiet-command,LC_ALL=C $(MAKEINFO) $(MAKEINFOFLAGS) --no-headers 
\
--html $< -o $@,"GEN","$@")
 
-%.info: %.texi version.texi
+%.info: %.texi
$(call quiet-command,$(MAKEINFO) $(MAKEINFOFLAGS) $< -o $@,"GEN","$@")
 
-%.txt: %.texi version.texi
+%.txt: %.texi
$(call quiet-command,LC_ALL=C $(MAKEINFO) $(MAKEINFOFLAGS) --no-headers 
\
--plaintext $< -o $@,"GEN","$@")
 
-%.pdf: %.texi version.texi
-   $(call quiet-command,texi2pdf $(TEXIFLAG) -I $(SRC_PATH) -I . $< -o 
$@,"GEN","$@")
+%.pdf: %.texi
+   $(call quiet-command,texi2pdf $(TEXIFLAG) -I $(SRC_PATH) -I docs $< -o 
$@,"GEN","$@")
+
+docs/qemu-ga-ref.html docs/qemu-ga-ref.info docs/qemu-ga-ref.txt 
docs/qemu-ga-ref.pdf docs/qemu-ga-ref.7.pod: docs/version.texi
+docs/qemu-qmp-ref.html docs/qemu-qmp-ref.info docs/qemu-qmp-ref.txt 
docs/qemu-qmp-ref.pdf docs/qemu-qmp-ref.pod: docs/version.texi
 
 qemu-options.texi: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > 
$@,"GEN","$@")
@@ -695,10 +698,10 @@ qemu-monitor-info.texi: $(SRC_PATH)/hmp-commands-info.hx 
$(SRC_PATH)/scripts/hxt
 qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > 
$@,"GEN","$@")
 
-qemu-qapi.texi: $(qapi-modules) $(qapi-py)
+docs/qemu-qmp-qapi.texi: $(qapi-modules) $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
 
-qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py)
+docs/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
 
 qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi 
qemu-monitor-info.texi
@@ -719,10 +722,10 @@ qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \
qemu-monitor-info.texi
 
 docs/qemu-ga-ref.dvi docs/qemu-ga-ref.html docs/qemu-ga-ref.info 
docs/qemu-ga-ref.pdf docs/qemu-ga-ref.txt docs/qemu-ga-ref.7: \
-docs/qemu-ga-ref.texi qemu-ga-qapi.texi
+docs/qemu-ga-ref.texi docs/qemu-ga-qapi.texi
 
 docs/qemu-qmp-ref.dvi docs/qemu-qmp-ref.html docs/qemu-qmp-ref.info 
docs/qemu-qmp-ref.pdf docs/qemu-qmp-ref.txt docs/qemu-qmp-ref.7: \
-docs/qemu-qmp-ref.texi qemu-qapi.texi
+docs/qemu-qmp-ref.texi docs/qemu-qmp-qapi.texi
 
 
 ifdef CONFIG_WIN32
diff --git a/docs/qemu-qmp-ref.texi b/docs/qemu-qmp-ref.texi
index 0a00569..bb25758 100644
--- a/docs/qemu-qmp-ref.texi
+++ b/docs/qemu-qmp-ref.texi
@@ -65,7 +65,7 @@ along with this manual.  If not, see 
http://www.gnu.org/licenses/.
 @c for texi2pod:
 @c man begin DESCRIPTION
 
-@include qemu-qapi.texi
+@include qemu-qmp-qapi.texi
 
 @c man end
 
diff --git a/rules.mak b/rules.mak
index 83d6dd1..1c0eabb 100644
--- a/rules.mak
+++ b/rules.mak
@@ -380,7 +380,7 @@ define unnest-vars
 endef
 
 TEXI2MAN = $(call quiet-command, \
-   perl -

[Qemu-devel] [PATCH for-2.9 29/47] qapi2texi: Use category "Object" for all object types

[Markus Armbruster]

At the protocol level, the distinction between struct, flat union and
simple union is meaningless, they are all JSON objects.  Document them
that way.

Example change (qemu-qmp-ref.txt):

- -- Simple Union: InputEvent
+ -- Object: InputEvent

  Input event union.

This also fixes the completely broken headings for flat and simple
unions in qemu-qmp-ref.7 and qemu-ga-ref.7, by sidestepping a bug in
texi2pod.pl.  For instance, it mistranslates "@deftp {Simple Union}
InputEvent" to "B (Simple)", but translates "@deftp Object
InputEvent" to "B (Object)".

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 282adf4..8eed11a 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -219,17 +219,11 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 
 def visit_object_type(self, name, info, base, members, variants):
 doc = self.cur_doc
-if not variants:
-typ = 'Struct'
-elif variants._tag_name:# TODO unclean member access
-typ = 'Flat Union'
-else:
-typ = 'Simple Union'
 if base and base.is_implicit():
 base = None
 if self.out:
 self.out += '\n'
-self.out += TYPE_FMT(type=typ,
+self.out += TYPE_FMT(type='Object',
  name=doc.symbol,
  body=texi_entity(doc, 'Members', base, variants))
 
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 20/47] qapi2texi: Plainer enum value and member name formatting

[Markus Armbruster]

Use @code{%s} instead of @code{'%s'}.  Impact, using @id as example:

* Texinfo
  -@item @code{'id'}
  +@item @code{id}

* HTML
  -'id'
  +id

* POD (for manual pages):
  -=item C<'id'>
  +=item C

* Formatted manual pages:
  -'id'
  +"id"

* Plain text:
  - ''id''
  + 'id'

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 91cd593..ecfaeda 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -130,12 +130,12 @@ def texi_body(doc):
 
 def texi_enum_value(value):
 """Format a table of members item for an enumeration value"""
-return "@item @code{'%s'}\n" % value.name
+return '@item @code{%s}\n' % value.name
 
 
 def texi_member(member):
 """Format a table of members item for an object type member"""
-return "@item @code{'%s'}%s\n" % (
+return '@item @code{%s}%s\n' % (
 member.name, ' (optional)' if member.optional else '')
 
 
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 42/47] qapi: enum_types is a list used like a dict, make it one

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 29 ++---
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index f06e3c4..5a3a606 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -46,7 +46,7 @@ returns_whitelist = []
 # Whitelist of entities allowed to violate case conventions
 name_case_whitelist = []
 
-enum_types = []
+enum_types = {}
 struct_types = []
 union_types = []
 all_names = {}
@@ -562,7 +562,7 @@ def find_alternate_member_qtype(qapi_type):
 return builtin_types[qapi_type]
 elif find_struct(qapi_type):
 return 'QTYPE_QDICT'
-elif find_enum(qapi_type):
+elif qapi_type in enum_types:
 return 'QTYPE_QSTRING'
 elif find_union(qapi_type):
 return 'QTYPE_QDICT'
@@ -586,7 +586,7 @@ def discriminator_find_enum_define(expr):
 if not discriminator_type:
 return None
 
-return find_enum(discriminator_type)
+return enum_types.get(discriminator_type)
 
 
 # Names must be letters, numbers, -, and _.  They must start with letter,
@@ -659,23 +659,6 @@ def find_union(name):
 return None
 
 
-def add_enum(definition, info):
-global enum_types
-enum_types.append(definition)
-
-
-def find_enum(name):
-global enum_types
-for enum in enum_types:
-if enum['enum'] == name:
-return enum
-return None
-
-
-def is_enum(name):
-return find_enum(name) is not None
-
-
 def check_type(info, source, value, allow_array=False,
allow_dict=False, allow_optional=False,
allow_metas=[]):
@@ -794,7 +777,7 @@ def check_union(expr, info):
"Discriminator '%s' is not a member of base "
"struct '%s'"
% (discriminator, base))
-enum_define = find_enum(discriminator_type)
+enum_define = enum_types.get(discriminator_type)
 allow_metas = ['struct']
 # Do not allow string discriminator
 if not enum_define:
@@ -928,7 +911,7 @@ def check_exprs(exprs):
 if 'enum' in expr:
 meta = 'enum'
 check_keys(expr_elem, 'enum', ['data'], ['prefix'])
-add_enum(expr, info)
+enum_types[expr[meta]] = expr
 elif 'union' in expr:
 meta = 'union'
 check_keys(expr_elem, 'union', ['data'],
@@ -966,7 +949,7 @@ def check_exprs(exprs):
 name = '%sKind' % expr['alternate']
 else:
 continue
-add_enum({ 'enum': name }, expr_elem['info'])
+enum_types[name] = { 'enum': name }
 add_name(name, info, 'enum', implicit=True)
 
 # Validate that exprs make sense
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 23/47] qapi2texi: Don't hide undocumented members and arguments

[Markus Armbruster]

Show undocumented object, alternate type members and command, event
arguments exactly like undocumented enumeration type values.

Example change (qemu-qmp-ref.txt):

  -- Command: query-rocker

  Return rocker switch information.

+ Arguments:
+ 'name'
+  Not documented
+
  Returns: 'Rocker' information

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 1f01817..df87441 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -139,12 +139,10 @@ def texi_member(member):
 member.name, ' (optional)' if member.optional else '')
 
 
-def texi_members(doc, what, member_func, show_undocumented):
+def texi_members(doc, what, member_func):
 """Format the table of members"""
 items = ''
 for section in doc.args.itervalues():
-if not section.content and not show_undocumented:
-continue  # Undocumented TODO require doc and drop
 if section.content:
 desc = str(section)
 else:
@@ -172,10 +170,9 @@ def texi_sections(doc):
 return body
 
 
-def texi_entity(doc, what, member_func=texi_member,
-show_undocumented=False):
+def texi_entity(doc, what, member_func=texi_member):
 return (texi_body(doc)
-+ texi_members(doc, what, member_func, show_undocumented)
++ texi_members(doc, what, member_func)
 + texi_sections(doc))
 
 
@@ -194,8 +191,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 self.out += TYPE_FMT(type='Enum',
  name=doc.symbol,
  body=texi_entity(doc, 'Values',
-  member_func=texi_enum_value,
-  show_undocumented=True))
+  member_func=texi_enum_value))
 
 def visit_object_type(self, name, info, base, members, variants):
 doc = self.cur_doc
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 22/47] qapi2texi: Explain enum value undocumentedness more clearly

[Markus Armbruster]

Instead of not saying anything when we have no documentation, say "Not
documented".

Example change (qemu-qmp-ref.txt):

  -- Enum: GuestPanicAction

  An enumeration of the actions taken when guest OS panic is detected

  Values:
  'pause'
   system pauses
  'poweroff'
+  Not documented

  Since: 2.1 (poweroff since 2.8)

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 35e182d..1f01817 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -145,7 +145,10 @@ def texi_members(doc, what, member_func, 
show_undocumented):
 for section in doc.args.itervalues():
 if not section.content and not show_undocumented:
 continue  # Undocumented TODO require doc and drop
-desc = str(section)
+if section.content:
+desc = str(section)
+else:
+desc = 'Not documented'
 items += member_func(section.member) + texi_format(desc) + '\n'
 if not items:
 return ''
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 25/47] qapi2texi: Include member type in generated documentation

[Markus Armbruster]

The recent merge of docs/qmp-commands.txt and docs/qmp-events.txt into
the schema lost type information.  Fix this documentation regression.

Example change (qemu-qmp-ref.txt):

  -- Struct: InputKeyEvent

  Keyboard input event.

  Members:
- 'button'
+ 'button: InputButton'
   Which button this event is for.
- 'down'
+ 'down: boolean'
   True for key-down and false for key-up events.

  Since: 2.0

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py  | 14 ++
 scripts/qapi2texi.py |  8 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9430493..b82a2a6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1101,6 +1101,11 @@ class QAPISchemaType(QAPISchemaEntity):
 }
 return json2qtype.get(self.json_type())
 
+def doc_type(self):
+if self.is_implicit():
+return None
+return self.name
+
 
 class QAPISchemaBuiltinType(QAPISchemaType):
 def __init__(self, name, json_type, c_type):
@@ -1125,6 +1130,9 @@ class QAPISchemaBuiltinType(QAPISchemaType):
 def json_type(self):
 return self._json_type_name
 
+def doc_type(self):
+return self.json_type()
+
 def visit(self, visitor):
 visitor.visit_builtin_type(self.name, self.info, self.json_type())
 
@@ -1184,6 +1192,12 @@ class QAPISchemaArrayType(QAPISchemaType):
 def json_type(self):
 return 'array'
 
+def doc_type(self):
+elt_doc_type = self.element_type.doc_type()
+if not elt_doc_type:
+return None
+return 'array of ' + elt_doc_type
+
 def visit(self, visitor):
 visitor.visit_array_type(self.name, self.info, self.element_type)
 
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 3dd0146..993b652 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -135,8 +135,12 @@ def texi_enum_value(value):
 
 def texi_member(member):
 """Format a table of members item for an object type member"""
-return '@item @code{%s}%s\n' % (
-member.name, ' (optional)' if member.optional else '')
+typ = member.type.doc_type()
+return '@item @code{%s%s%s}%s\n' % (
+member.name,
+': ' if typ else '',
+typ if typ else '',
+' (optional)' if member.optional else '')
 
 
 def texi_members(doc, what, member_func):
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 28/47] qapi2texi: Generate descriptions for simple union tags

[Markus Armbruster]

Simple union tags carry no type information, because their type is
implicit.  Their description should make up for it, but many have
none.  Generate one automatically then.

Example change (qemu-qmp-ref.txt):

  -- Simple Union: ImageInfoSpecific

  A discriminated record of image format specific information
  structures.

  Members:
  'type'
-  Not documented
+  One of "qcow2", "vmdk", "luks"
  'data: ImageInfoSpecificQCow2' when 'type' is "qcow2"
  'data: ImageInfoSpecificVmdk' when 'type' is "vmdk"
  'data: QCryptoBlockInfoLUKS' when 'type' is "luks"

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index ab6b6cd..282adf4 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -148,11 +148,16 @@ def texi_members(doc, what, base, variants, member_func):
 """Format the table of members"""
 items = ''
 for section in doc.args.itervalues():
+# TODO Drop fallbacks when undocumented members are outlawed
 if section.content:
-desc = str(section)
+desc = texi_format(str(section))
+elif (variants and variants.tag_member == section.member
+  and not section.member.type.doc_type()):
+values = section.member.type.member_names()
+desc = 'One of ' + ', '.join(['@t{"%s"}' % v for v in values])
 else:
 desc = 'Not documented'
-items += member_func(section.member) + texi_format(desc) + '\n'
+items += member_func(section.member) + desc + '\n'
 if base:
 items += '@item The members of @code{%s}\n' % base.doc_type()
 if variants:
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 12/47] qapi/rocker: Fix up doc comment notes on optional members

[Markus Armbruster]

Talking about #optional like this

# Note: fields are marked #optional to indicate that they may or may
# not appear ...

doesn't work so well in generated documentation, because the #optional
tag is not visible there.  Replace by

# Note: optional members may or may not appear ...

Signed-off-by: Markus Armbruster 
---
 qapi/rocker.json | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/qapi/rocker.json b/qapi/rocker.json
index 97e2b83..f374038 100644
--- a/qapi/rocker.json
+++ b/qapi/rocker.json
@@ -1,3 +1,5 @@
+# -*- Mode: Python -*-
+
 ##
 # = Rocker switch device
 ##
@@ -137,8 +139,8 @@
 #
 # @ip-dst: #optional IP header destination address
 #
-# Note: fields are marked #optional to indicate that they may or may not
-# appear in the flow key depending if they're relevant to the flow key.
+# Note: optional members may or may not appear in the flow key
+# depending if they're relevant to the flow key.
 #
 # Since: 2.4
 ##
@@ -167,8 +169,8 @@
 #
 # @ip-tos: #optional IP header TOS field
 #
-# Note: fields are marked #optional to indicate that they may or may not
-# appear in the flow mask depending if they're relevant to the flow mask.
+# Note: optional members may or may not appear in the flow mask
+# depending if they're relevant to the flow mask.
 #
 # Since: 2.4
 ##
@@ -194,8 +196,8 @@
 #
 # @out-pport: #optional physical output port
 #
-# Note: fields are marked #optional to indicate that they may or may not
-# appear in the flow action depending if they're relevant to the flow action.
+# Note: optional members may or may not appear in the flow action
+# depending if they're relevant to the flow action.
 #
 # Since: 2.4
 ##
@@ -288,8 +290,8 @@
 #
 # @ttl-check: #optional perform TTL check
 #
-# Note: fields are marked #optional to indicate that they may or may not
-# appear in the group depending if they're relevant to the group type.
+# Note: optional members may or may not appear in the group depending
+# if they're relevant to the group type.
 #
 # Since: 2.4
 ##
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 35/47] tests/qapi-schema: Rename doc-bad-args to doc-bad-command-arg

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 tests/Makefile.include| 2 +-
 tests/qapi-schema/doc-bad-args.err| 1 -
 tests/qapi-schema/doc-bad-command-arg.err | 1 +
 tests/qapi-schema/{doc-bad-args.exit => doc-bad-command-arg.exit} | 0
 tests/qapi-schema/{doc-bad-args.json => doc-bad-command-arg.json} | 0
 tests/qapi-schema/{doc-bad-args.out => doc-bad-command-arg.out}   | 0
 6 files changed, 2 insertions(+), 2 deletions(-)
 delete mode 100644 tests/qapi-schema/doc-bad-args.err
 create mode 100644 tests/qapi-schema/doc-bad-command-arg.err
 rename tests/qapi-schema/{doc-bad-args.exit => doc-bad-command-arg.exit} (100%)
 rename tests/qapi-schema/{doc-bad-args.json => doc-bad-command-arg.json} (100%)
 rename tests/qapi-schema/{doc-bad-args.out => doc-bad-command-arg.out} (100%)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 800acbf..8ce9d3d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -367,7 +367,7 @@ qapi-schema += base-cycle-direct.json
 qapi-schema += base-cycle-indirect.json
 qapi-schema += command-int.json
 qapi-schema += comments.json
-qapi-schema += doc-bad-args.json
+qapi-schema += doc-bad-command-arg.json
 qapi-schema += doc-bad-expr.json
 qapi-schema += doc-bad-symbol.json
 qapi-schema += doc-duplicated-arg.json
diff --git a/tests/qapi-schema/doc-bad-args.err 
b/tests/qapi-schema/doc-bad-args.err
deleted file mode 100644
index 5d44d9b..000
--- a/tests/qapi-schema/doc-bad-args.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/doc-bad-args.json:3: The following documented members are 
not in the declaration: b
diff --git a/tests/qapi-schema/doc-bad-command-arg.err 
b/tests/qapi-schema/doc-bad-command-arg.err
new file mode 100644
index 000..8075b14
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-command-arg.err
@@ -0,0 +1 @@
+tests/qapi-schema/doc-bad-command-arg.json:3: The following documented members 
are not in the declaration: b
diff --git a/tests/qapi-schema/doc-bad-args.exit 
b/tests/qapi-schema/doc-bad-command-arg.exit
similarity index 100%
rename from tests/qapi-schema/doc-bad-args.exit
rename to tests/qapi-schema/doc-bad-command-arg.exit
diff --git a/tests/qapi-schema/doc-bad-args.json 
b/tests/qapi-schema/doc-bad-command-arg.json
similarity index 100%
rename from tests/qapi-schema/doc-bad-args.json
rename to tests/qapi-schema/doc-bad-command-arg.json
diff --git a/tests/qapi-schema/doc-bad-args.out 
b/tests/qapi-schema/doc-bad-command-arg.out
similarity index 100%
rename from tests/qapi-schema/doc-bad-args.out
rename to tests/qapi-schema/doc-bad-command-arg.out
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 15/47] qapi: Conjure up QAPIDoc.ArgSection for undocumented members

[Markus Armbruster]

qapi2texi.py already conjures up ArgSections for undocumented
enumeration values, in texi_enum).  Drop that, and conjure them up for
all kinds of "arguments" (enumeration values, object and alternate
type members) in qapi.py instead.

Take care to keep generated documentation exactly the same for now.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py  |  5 ++---
 scripts/qapi2texi.py | 31 ---
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 4886417..2a2b33d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -224,9 +224,8 @@ class QAPIDoc(object):
 def connect_member(self, member):
 if not member.name in self.args:
 # Undocumented TODO outlaw
-pass
-else:
-self.args[member.name].connect(member)
+self.args[member.name] = QAPIDoc.ArgSection(member.name)
+self.args[member.name].connect(member)
 
 
 class QAPISchemaParser(object):
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 0aaf45c..299dcf9 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -123,7 +123,7 @@ def texi_format(doc):
 return "\n".join(lines)
 
 
-def texi_body(doc):
+def texi_body(doc, only_documented=False):
 """
 Format the body of a symbol documentation:
 - main body
@@ -131,17 +131,21 @@ def texi_body(doc):
 - followed by "Returns/Notes/Since/Example" sections
 """
 body = texi_format(str(doc.body)) + "\n"
-if doc.args:
+
+args = ''
+for name, section in doc.args.iteritems():
+if not section.content and not only_documented:
+continue# Undocumented TODO require doc and drop
+desc = str(section)
+opt = ''
+if section.optional:
+desc = re.sub(r'^ *#optional *\n?|\n? *#optional *$|#optional',
+  '', desc)
+opt = ' (optional)'
+args += "@item @code{'%s'}%s\n%s\n" % (name, opt, texi_format(desc))
+if args:
 body += "@table @asis\n"
-for arg, section in doc.args.iteritems():
-desc = str(section)
-opt = ''
-if section.optional:
-desc = re.sub(r'^ *#optional *\n?|\n? *#optional *$|#optional',
-  '', desc)
-opt = ' (optional)'
-body += "@item @code{'%s'}%s\n%s\n" % (arg, opt,
-   texi_format(desc))
+body += args
 body += "@end table\n"
 
 for section in doc.sections:
@@ -183,10 +187,7 @@ def texi_union(expr, doc):
 
 def texi_enum(expr, doc):
 """Format an enum to texi"""
-for i in expr['data']:
-if i not in doc.args:
-doc.args[i] = qapi.QAPIDoc.ArgSection(i)
-body = texi_body(doc)
+body = texi_body(doc, True)
 return TYPE_FMT(type="Enum",
 name=doc.symbol,
 body=body)
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 34/47] qapi: Move empty doc section checking to doc parser

[Markus Armbruster]

Results in a more precise error location, but the real reason is
emptying out check_docs() step by step.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 20 ++--
 tests/qapi-schema/doc-empty-section.err |  2 +-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8d34651..e53701a 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -173,6 +173,9 @@ class QAPIDoc(object):
 else:
 self._append_freeform(line)
 
+def end_comment(self):
+self._end_section()
+
 def _append_symbol_line(self, line):
 name = line.split(' ', 1)[0]
 
@@ -200,6 +203,7 @@ class QAPIDoc(object):
 raise QAPIParseError(self.parser,
  "'@%s:' can't follow '%s' section"
  % (name, self.sections[0].name))
+self._end_section()
 self.section = QAPIDoc.ArgSection(name)
 self.args[name] = self.section
 
@@ -207,9 +211,18 @@ class QAPIDoc(object):
 if name in ('Returns', 'Since') and self.has_section(name):
 raise QAPIParseError(self.parser,
  "Duplicated '%s' section" % name)
+self._end_section()
 self.section = QAPIDoc.Section(name)
 self.sections.append(self.section)
 
+def _end_section(self):
+if self.section:
+contents = str(self.section)
+if self.section.name and (not contents or contents.isspace()):
+raise QAPIParseError(self.parser, "Empty doc section '%s'"
+ % self.section.name)
+self.section = None
+
 def _append_freeform(self, line):
 in_arg = isinstance(self.section, QAPIDoc.ArgSection)
 if (in_arg and self.section.content
@@ -507,6 +520,7 @@ class QAPISchemaParser(object):
 if self.val != '##':
 raise QAPIParseError(self, "Junk after '##' at end of "
  "documentation comment")
+doc.end_comment()
 self.accept()
 return doc
 else:
@@ -1007,12 +1021,6 @@ def check_definition_doc(doc, expr, info):
 
 def check_docs(docs):
 for doc in docs:
-for section in doc.args.values() + doc.sections:
-content = str(section)
-if not content or content.isspace():
-raise QAPISemError(doc.info,
-   "Empty doc section '%s'" % section.name)
-
 if doc.expr:
 check_definition_doc(doc, doc.expr, doc.info)
 
diff --git a/tests/qapi-schema/doc-empty-section.err 
b/tests/qapi-schema/doc-empty-section.err
index 00ad625..b61e4a7 100644
--- a/tests/qapi-schema/doc-empty-section.err
+++ b/tests/qapi-schema/doc-empty-section.err
@@ -1 +1 @@
-tests/qapi-schema/doc-empty-section.json:3: Empty doc section 'Note'
+tests/qapi-schema/doc-empty-section.json:7:1: Empty doc section 'Note'
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 21/47] qapi2texi: Present the table of members more clearly

[Markus Armbruster]

The table of members follows the main descriptive text immediately.
Makes it hard to see what it is about.  Start a new paragraph, and
lead with a line "Members:" for object and alternate types, "Values:"
for enumeration types, and "Arguments:" for commands and events.

Example change (qemu-qmp-ref.txt):

  -- Command: set_link

  Sets the link status of a virtual network adapter.
+
+ Arguments:
  'name'
   the device name of the virtual network adapter
  'up'
   true to set the link status to be up

  Returns: Nothing on success If 'name' is not a valid network
  device, DeviceNotFound

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index ecfaeda..35e182d 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -139,7 +139,7 @@ def texi_member(member):
 member.name, ' (optional)' if member.optional else '')
 
 
-def texi_members(doc, member_func, show_undocumented):
+def texi_members(doc, what, member_func, show_undocumented):
 """Format the table of members"""
 items = ''
 for section in doc.args.itervalues():
@@ -149,7 +149,7 @@ def texi_members(doc, member_func, show_undocumented):
 items += member_func(section.member) + texi_format(desc) + '\n'
 if not items:
 return ''
-return '@table @asis\n' + items + '@end table\n'
+return '\n@b{%s:}\n@table @asis\n%s@end table\n' % (what, items)
 
 
 def texi_sections(doc):
@@ -169,9 +169,10 @@ def texi_sections(doc):
 return body
 
 
-def texi_entity(doc, member_func=texi_member, show_undocumented=False):
+def texi_entity(doc, what, member_func=texi_member,
+show_undocumented=False):
 return (texi_body(doc)
-+ texi_members(doc, member_func, show_undocumented)
++ texi_members(doc, what, member_func, show_undocumented)
 + texi_sections(doc))
 
 
@@ -189,7 +190,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 self.out += '\n'
 self.out += TYPE_FMT(type='Enum',
  name=doc.symbol,
- body=texi_entity(doc,
+ body=texi_entity(doc, 'Values',
   member_func=texi_enum_value,
   show_undocumented=True))
 
@@ -205,7 +206,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 self.out += '\n'
 self.out += TYPE_FMT(type=typ,
  name=doc.symbol,
- body=texi_entity(doc))
+ body=texi_entity(doc, 'Members'))
 
 def visit_alternate_type(self, name, info, variants):
 doc = self.cur_doc
@@ -213,7 +214,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 self.out += '\n'
 self.out += TYPE_FMT(type='Alternate',
  name=doc.symbol,
- body=texi_entity(doc))
+ body=texi_entity(doc, 'Members'))
 
 def visit_command(self, name, info, arg_type, ret_type,
   gen, success_response, boxed):
@@ -222,7 +223,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 self.out += '\n'
 self.out += MSG_FMT(type='Command',
 name=doc.symbol,
-body=texi_entity(doc))
+body=texi_entity(doc, 'Arguments'))
 
 def visit_event(self, name, info, arg_type, boxed):
 doc = self.cur_doc
@@ -230,7 +231,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 self.out += '\n'
 self.out += MSG_FMT(type='Event',
 name=doc.symbol,
-body=texi_entity(doc))
+body=texi_entity(doc, 'Arguments'))
 
 def symbol(self, doc, entity):
 self.cur_doc = doc
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 24/47] qapi2texi: Implement boxed argument documentation

[Markus Armbruster]

This replaces manual references like "For the arguments, see the
documentation of ..." by a generated reference "Arguments: the members
of ...".

Signed-off-by: Markus Armbruster 
---
 qapi-schema.json |  2 +-
 qapi/block-core.json | 10 --
 scripts/qapi2texi.py |  8 +++-
 3 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index d693033..1d7b1cd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1017,7 +1017,7 @@
 ##
 # @migrate-set-parameters:
 #
-# Set various migration parameters.  See MigrationParameters for details.
+# Set various migration parameters.
 #
 # Since: 2.4
 #
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 539649c..83601db 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1340,8 +1340,6 @@
 # The operation can be stopped before it has completed using the
 # block-job-cancel command.
 #
-# For the arguments, see the documentation of DriveBackup.
-#
 # Returns: nothing on success
 #  If @device is not a valid block device, GenericError
 #
@@ -1368,8 +1366,6 @@
 # The operation can be stopped before it has completed using the
 # block-job-cancel command.
 #
-# For the arguments, see the documentation of BlockdevBackup.
-#
 # Returns: nothing on success
 #  If @device is not a valid block device, DeviceNotFound
 #
@@ -1457,8 +1453,6 @@
 # format of the mirror image, default is to probe if mode='existing',
 # else the format of the source.
 #
-# See DriveMirror for parameter descriptions
-#
 # Returns: nothing on success
 #  If @device is not a valid block device, GenericError
 #
@@ -1730,8 +1724,6 @@
 # the device will be removed from its group and the rest of its
 # members will not be affected. The 'group' parameter is ignored.
 #
-# See BlockIOThrottle for parameter descriptions.
-#
 # Returns: Nothing on success
 #  If @device is not a valid block device, DeviceNotFound
 #
@@ -2944,8 +2936,6 @@
 # BlockBackend will be created; otherwise, @node-name is mandatory at the top
 # level and no BlockBackend will be created.
 #
-# For the arguments, see the documentation of BlockdevOptions.
-#
 # Note: This command is still a work in progress.  It doesn't support all
 # block drivers among other things.  Stay away from it unless you want
 # to help with its development.
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index df87441..3dd0146 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -220,9 +220,15 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 doc = self.cur_doc
 if self.out:
 self.out += '\n'
+if boxed:
+body = texi_body(doc)
+body += '\n@b{Arguments:} the members of @code{%s}' % arg_type.name
+body += texi_sections(doc)
+else:
+body = texi_entity(doc, 'Arguments')
 self.out += MSG_FMT(type='Command',
 name=doc.symbol,
-body=texi_entity(doc, 'Arguments'))
+body=body)
 
 def visit_event(self, name, info, arg_type, boxed):
 doc = self.cur_doc
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 40/47] qapi: Simplify what gets stored in enum_types

[Markus Armbruster]

Don't invent a new dictionary structure just for enum_types, simply
store the defining expression, like we do for struct_types and
union_types.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 12b1bda..ffd30d2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -663,16 +663,17 @@ def find_union(name):
 return None
 
 
-def add_enum(name, info, enum_values=None, implicit=False):
+def add_enum(definition, info):
 global enum_types
-add_name(name, info, 'enum', implicit)
-enum_types.append({'enum_name': name, 'enum_values': enum_values})
+name = definition['enum']
+add_name(name, info, 'enum', 'data' not in definition)
+enum_types.append(definition)
 
 
 def find_enum(name):
 global enum_types
 for enum in enum_types:
-if enum['enum_name'] == name:
+if enum['enum'] == name:
 return enum
 return None
 
@@ -820,15 +821,15 @@ def check_union(expr, info):
 # If the discriminator names an enum type, then all members
 # of 'data' must also be members of the enum type.
 if enum_define:
-if key not in enum_define['enum_values']:
+if key not in enum_define['data']:
 raise QAPISemError(info,
"Discriminator value '%s' is not found in "
"enum '%s'"
-   % (key, enum_define['enum_name']))
+   % (key, enum_define['enum']))
 
 # If discriminator is user-defined, ensure all values are covered
 if enum_define:
-for value in enum_define['enum_values']:
+for value in enum_define['data']:
 if value not in members.keys():
 raise QAPISemError(info, "Union '%s' data missing '%s' branch"
% (name, value))
@@ -933,7 +934,7 @@ def check_exprs(exprs):
 if 'enum' in expr:
 name = expr['enum']
 check_keys(expr_elem, 'enum', ['data'], ['prefix'])
-add_enum(name, info, expr['data'])
+add_enum(expr, info)
 elif 'union' in expr:
 name = expr['union']
 check_keys(expr_elem, 'union', ['data'],
@@ -966,13 +967,13 @@ def check_exprs(exprs):
 # Try again for hidden UnionKind enum
 for expr_elem in exprs:
 expr = expr_elem['expr']
-if 'union' in expr:
-if not discriminator_find_enum_define(expr):
-add_enum('%sKind' % expr['union'], expr_elem['info'],
- implicit=True)
+if 'union' in expr and not discriminator_find_enum_define(expr):
+name = '%sKind' % expr['union']
 elif 'alternate' in expr:
-add_enum('%sKind' % expr['alternate'], expr_elem['info'],
- implicit=True)
+name = '%sKind' % expr['alternate']
+else:
+continue
+add_enum({ 'enum': name }, expr_elem['info'])
 
 # Validate that exprs make sense
 for expr_elem in exprs:
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 27/47] qapi2texi: Generate documentation for variant members

[Markus Armbruster]

A flat union's branch brings in the members of another type.  Generate
a suitable reference to that type.

Example change (qemu-qmp-ref.txt):

  -- Flat Union: QCryptoBlockOpenOptions

  The options that are available for all encryption formats when
  opening an existing volume

  Members:
  The members of 'QCryptoBlockOptionsBase'
+ The members of 'QCryptoBlockOptionsQCow' when 'format' is "qcow"
+ The members of 'QCryptoBlockOptionsLUKS' when 'format' is "luks"

  Since: 2.6

A simple union's branch adds a member 'data' of some other type.
Generate documentation for that member.

Example change (qemu-qmp-ref.txt):

  -- Simple Union: SocketAddress

  Captures the address of a socket, which could also be a named file
  descriptor

  Members:
  'type'
   Not documented
+ 'data: InetSocketAddress' when 'type' is "inet"
+ 'data: UnixSocketAddress' when 'type' is "unix"
+ 'data: VsockSocketAddress' when 'type' is "vsock"
+ 'data: String' when 'type' is "fd"

  Since: 1.3

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 7083d0c..ab6b6cd 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -133,17 +133,18 @@ def texi_enum_value(value):
 return '@item @code{%s}\n' % value.name
 
 
-def texi_member(member):
+def texi_member(member, suffix=''):
 """Format a table of members item for an object type member"""
 typ = member.type.doc_type()
-return '@item @code{%s%s%s}%s\n' % (
+return '@item @code{%s%s%s}%s%s\n' % (
 member.name,
 ': ' if typ else '',
 typ if typ else '',
-' (optional)' if member.optional else '')
+' (optional)' if member.optional else '',
+suffix)
 
 
-def texi_members(doc, what, base, member_func):
+def texi_members(doc, what, base, variants, member_func):
 """Format the table of members"""
 items = ''
 for section in doc.args.itervalues():
@@ -154,6 +155,17 @@ def texi_members(doc, what, base, member_func):
 items += member_func(section.member) + texi_format(desc) + '\n'
 if base:
 items += '@item The members of @code{%s}\n' % base.doc_type()
+if variants:
+for v in variants.variants:
+when = ' when @code{%s} is @t{"%s"}' % (
+variants.tag_member.name, v.name)
+if v.type.is_implicit():
+assert not v.type.base and not v.type.variants
+for m in v.type.local_members:
+items += member_func(m, when)
+else:
+items += '@item The members of @code{%s}%s\n' % (
+v.type.doc_type(), when)
 if not items:
 return ''
 return '\n@b{%s:}\n@table @asis\n%s@end table\n' % (what, items)
@@ -176,9 +188,10 @@ def texi_sections(doc):
 return body
 
 
-def texi_entity(doc, what, base=None, member_func=texi_member):
+def texi_entity(doc, what, base=None, variants=None,
+member_func=texi_member):
 return (texi_body(doc)
-+ texi_members(doc, what, base, member_func)
++ texi_members(doc, what, base, variants, member_func)
 + texi_sections(doc))
 
 
@@ -213,7 +226,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
 self.out += '\n'
 self.out += TYPE_FMT(type=typ,
  name=doc.symbol,
- body=texi_entity(doc, 'Members', base))
+ body=texi_entity(doc, 'Members', base, variants))
 
 def visit_alternate_type(self, name, info, variants):
 doc = self.cur_doc
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 39/47] qapi: Drop unused variable events

[Markus Armbruster]

Missed in commit e98859a

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0da426a..12b1bda 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -49,7 +49,6 @@ name_case_whitelist = []
 enum_types = []
 struct_types = []
 union_types = []
-events = []
 all_names = {}
 
 #
@@ -751,14 +750,12 @@ def check_command(expr, info):
 
 
 def check_event(expr, info):
-global events
 name = expr['event']
 boxed = expr.get('boxed', False)
 
 meta = ['struct']
 if boxed:
 meta += ['union', 'alternate']
-events.append(name)
 check_type(info, "'data' for event '%s'" % name,
expr.get('data'), allow_dict=not boxed, allow_optional=True,
allow_metas=meta)
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 03/47] qapi: Back out doc comments added just to please qapi.py

[Markus Armbruster]

This reverts commit 3313b61's changes to tests/qapi-schema/, except
for tests/qapi-schema/doc-*.

Signed-off-by: Markus Armbruster 
---
 tests/qapi-schema/alternate-any.err|   2 +-
 tests/qapi-schema/alternate-any.json   |   4 -
 tests/qapi-schema/alternate-array.err  |   2 +-
 tests/qapi-schema/alternate-array.json |   7 -
 tests/qapi-schema/alternate-base.err   |   2 +-
 tests/qapi-schema/alternate-base.json  |   7 -
 tests/qapi-schema/alternate-clash.err  |   2 +-
 tests/qapi-schema/alternate-clash.json |   4 -
 tests/qapi-schema/alternate-conflict-dict.err  |   2 +-
 tests/qapi-schema/alternate-conflict-dict.json |  10 -
 tests/qapi-schema/alternate-conflict-string.err|   2 +-
 tests/qapi-schema/alternate-conflict-string.json   |   7 -
 tests/qapi-schema/alternate-empty.err  |   2 +-
 tests/qapi-schema/alternate-empty.json |   4 -
 tests/qapi-schema/alternate-nested.err |   2 +-
 tests/qapi-schema/alternate-nested.json|   7 -
 tests/qapi-schema/alternate-unknown.err|   2 +-
 tests/qapi-schema/alternate-unknown.json   |   4 -
 tests/qapi-schema/args-alternate.err   |   2 +-
 tests/qapi-schema/args-alternate.json  |   8 -
 tests/qapi-schema/args-any.err |   2 +-
 tests/qapi-schema/args-any.json|   4 -
 tests/qapi-schema/args-array-empty.err |   2 +-
 tests/qapi-schema/args-array-empty.json|   4 -
 tests/qapi-schema/args-array-unknown.err   |   2 +-
 tests/qapi-schema/args-array-unknown.json  |   4 -
 tests/qapi-schema/args-bad-boxed.err   |   2 +-
 tests/qapi-schema/args-bad-boxed.json  |   4 -
 tests/qapi-schema/args-boxed-anon.err  |   2 +-
 tests/qapi-schema/args-boxed-anon.json |   4 -
 tests/qapi-schema/args-boxed-empty.err |   2 +-
 tests/qapi-schema/args-boxed-empty.json|   8 -
 tests/qapi-schema/args-boxed-string.err|   2 +-
 tests/qapi-schema/args-boxed-string.json   |   4 -
 tests/qapi-schema/args-int.err |   2 +-
 tests/qapi-schema/args-int.json|   4 -
 tests/qapi-schema/args-invalid.err |   2 +-
 tests/qapi-schema/args-invalid.json|   3 -
 tests/qapi-schema/args-member-array-bad.err|   2 +-
 tests/qapi-schema/args-member-array-bad.json   |   4 -
 tests/qapi-schema/args-member-case.err |   2 +-
 tests/qapi-schema/args-member-case.json|   4 -
 tests/qapi-schema/args-member-unknown.err  |   2 +-
 tests/qapi-schema/args-member-unknown.json |   4 -
 tests/qapi-schema/args-name-clash.err  |   2 +-
 tests/qapi-schema/args-name-clash.json |   4 -
 tests/qapi-schema/args-union.err   |   2 +-
 tests/qapi-schema/args-union.json  |   7 -
 tests/qapi-schema/args-unknown.err |   2 +-
 tests/qapi-schema/args-unknown.json|   4 -
 tests/qapi-schema/bad-base.err |   2 +-
 tests/qapi-schema/bad-base.json|   7 -
 tests/qapi-schema/bad-data.err |   2 +-
 tests/qapi-schema/bad-data.json|   4 -
 tests/qapi-schema/bad-ident.err|   2 +-
 tests/qapi-schema/bad-ident.json   |   4 -
 tests/qapi-schema/bad-type-bool.err|   2 +-
 tests/qapi-schema/bad-type-bool.json   |   4 -
 tests/qapi-schema/bad-type-dict.err|   2 +-
 tests/qapi-schema/bad-type-dict.json   |   4 -
 tests/qapi-schema/base-cycle-direct.err|   2 +-
 tests/qapi-schema/base-cycle-direct.json   |   4 -
 tests/qapi-schema/base-cycle-indirect.err  |   2 +-
 tests/qapi-schema/base-cycle-indirect.json |   7 -
 tests/qapi-schema/command-int.err  |   2 +-
 tests/qapi-schema/command-int.json |   4 -
 tests/qapi-schema/comments.json|   4 -
 tests/qapi-schema/comments.out |   1 -
 tests/qapi-schema/double-type.err  |   2 +-
 tests/qapi-schema/double-type.json |   4 -
 tests/qapi-schema/enum-bad-name.err|   2 +-
 tests/qapi-schema/enum-bad-name.json   |   4 -
 tests/qapi-schema/enum-bad-prefix.err  |   2 +-
 tests/qapi-schema/enum-bad-prefix.json |   4 -
 tests/qapi-schema/enum-clash-member.err|   2 +-
 tests/qapi-schema/enum-clash-member.json   |   4 -
 tests/qapi-schema/enum-dict-member.err |   2 +-
 tests/qapi-schema/enum-dict-member.json|   4 -
 tests/qapi-schema/enum-member-case.err |   2 +-
 tests/qapi-schema/enum-member-case.json|   7 -
 tests/qapi-schema/enum-

[Qemu-devel] [PATCH for-2.9 43/47] qapi: struct_types is a list used like a dict, make it one

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 21 -
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 5a3a606..ab266db 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -47,7 +47,7 @@ returns_whitelist = []
 name_case_whitelist = []
 
 enum_types = {}
-struct_types = []
+struct_types = {}
 union_types = []
 all_names = {}
 
@@ -550,7 +550,7 @@ class QAPISchemaParser(object):
 def find_base_members(base):
 if isinstance(base, dict):
 return base
-base_struct_define = find_struct(base)
+base_struct_define = struct_types.get(base)
 if not base_struct_define:
 return None
 return base_struct_define['data']
@@ -560,7 +560,7 @@ def find_base_members(base):
 def find_alternate_member_qtype(qapi_type):
 if qapi_type in builtin_types:
 return builtin_types[qapi_type]
-elif find_struct(qapi_type):
+elif qapi_type in struct_types:
 return 'QTYPE_QDICT'
 elif qapi_type in enum_types:
 return 'QTYPE_QSTRING'
@@ -633,19 +633,6 @@ def add_name(name, info, meta, implicit=False):
 all_names[name] = meta
 
 
-def add_struct(definition, info):
-global struct_types
-struct_types.append(definition)
-
-
-def find_struct(name):
-global struct_types
-for struct in struct_types:
-if struct['struct'] == name:
-return struct
-return None
-
-
 def add_union(definition, info):
 global union_types
 union_types.append(definition)
@@ -923,7 +910,7 @@ def check_exprs(exprs):
 elif 'struct' in expr:
 meta = 'struct'
 check_keys(expr_elem, 'struct', ['data'], ['base'])
-add_struct(expr, info)
+struct_types[expr[meta]] = expr
 elif 'command' in expr:
 meta = 'command'
 check_keys(expr_elem, 'command', [],
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 38/47] qapi: Eliminate check_docs() and drop QAPIDoc.expr

[Markus Armbruster]

Move what's left in check_docs() to check_expr().  Delegate the actual
checking to new QAPIDoc.check_expr().

QAPIDoc.expr is now unused; drop it.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 27 ++-
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0b0ba59..0da426a 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -138,8 +138,6 @@ class QAPIDoc(object):
 self.sections = []
 # the current section
 self.section = self.body
-# associated expression (to be set by expression parser)
-self.expr = None
 
 def has_section(self, name):
 """Return True if we have a section with this name."""
@@ -249,6 +247,11 @@ class QAPIDoc(object):
 self.args[member.name] = QAPIDoc.ArgSection(member.name)
 self.args[member.name].connect(member)
 
+def check_expr(self, expr):
+if self.has_section('Returns') and 'command' not in expr:
+raise QAPISemError(self.info,
+   "'Returns:' is only valid for commands")
+
 def check(self):
 bogus = [name for name, section in self.args.iteritems()
  if not section.member]
@@ -311,7 +314,6 @@ class QAPISchemaParser(object):
 raise QAPISemError(
 self.cur_doc.info,
 "Expression documentation required")
-self.cur_doc.expr = expr
 expr_elem['doc'] = self.cur_doc
 self.exprs.append(expr_elem)
 self.cur_doc = None
@@ -979,6 +981,7 @@ def check_exprs(exprs):
 for expr_elem in exprs:
 expr = expr_elem['expr']
 info = expr_elem['info']
+doc = expr_elem.get('doc')
 
 if 'enum' in expr:
 check_enum(expr, info)
@@ -995,22 +998,12 @@ def check_exprs(exprs):
 else:
 assert False, 'unexpected meta type'
 
+if doc:
+doc.check_expr(expr)
+
 return exprs
 
 
-def check_definition_doc(doc, expr, info):
-if doc.has_section('Returns') and 'command' not in expr:
-raise QAPISemError(info, "'Returns:' is only valid for commands")
-
-
-def check_docs(docs):
-for doc in docs:
-if doc.expr:
-check_definition_doc(doc, doc.expr, doc.info)
-
-return docs
-
-
 #
 # Schema compiler frontend
 #
@@ -1506,7 +1499,7 @@ class QAPISchema(object):
 try:
 parser = QAPISchemaParser(open(fname, 'r'))
 self.exprs = check_exprs(parser.exprs)
-self.docs = check_docs(parser.docs)
+self.docs = parser.docs
 self._entity_dict = {}
 self._predefining = True
 self._def_predefineds()
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 31/47] qapi: Fix detection of doc / expression mismatch

[Markus Armbruster]

This fixes the errors uncovered by the previous commit.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py| 34 +-
 tests/qapi-schema/doc-bad-expr.err |  1 +
 tests/qapi-schema/doc-bad-expr.exit|  2 +-
 tests/qapi-schema/doc-bad-expr.json|  1 -
 tests/qapi-schema/doc-bad-expr.out |  4 
 tests/qapi-schema/doc-missing-expr.err |  2 +-
 tests/qapi-schema/doc-no-symbol.err|  2 +-
 tests/qapi-schema/doc-no-symbol.json   |  1 -
 8 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index b82a2a6..1052274 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -248,18 +248,21 @@ class QAPISchemaParser(object):
 self.line_pos = 0
 self.exprs = []
 self.docs = []
+self.cur_doc = None
 self.accept()
 
 while self.tok is not None:
 info = {'file': fname, 'line': self.line,
 'parent': self.incl_info}
 if self.tok == '#':
-doc = self.get_doc(info)
-self.docs.append(doc)
+self.reject_expr_doc()
+self.cur_doc = self.get_doc(info)
+self.docs.append(self.cur_doc)
 continue
 
 expr = self.get_expr(False)
 if 'include' in expr:
+self.reject_expr_doc()
 if len(expr) != 1:
 raise QAPISemError(info, "Invalid 'include' directive")
 include = expr['include']
@@ -276,13 +279,23 @@ class QAPISchemaParser(object):
 else:
 expr_elem = {'expr': expr,
  'info': info}
-if (self.docs
-and self.docs[-1].info['file'] == fname
-and not self.docs[-1].expr):
-self.docs[-1].expr = expr
-expr_elem['doc'] = self.docs[-1]
-
+if self.cur_doc:
+if not self.cur_doc.symbol:
+raise QAPISemError(
+self.cur_doc.info,
+"Expression documentation required")
+self.cur_doc.expr = expr
+expr_elem['doc'] = self.cur_doc
 self.exprs.append(expr_elem)
+self.cur_doc = None
+self.reject_expr_doc()
+
+def reject_expr_doc(self):
+if self.cur_doc and self.cur_doc.symbol:
+raise QAPISemError(
+self.cur_doc.info,
+"Documentation for '%s' is not followed by the definition"
+% self.cur_doc.symbol)
 
 def _include(self, include, info, base_dir, previously_included):
 incl_abs_fname = os.path.join(base_dir, include)
@@ -946,11 +959,6 @@ def check_exprs(exprs):
 
 
 def check_freeform_doc(doc):
-if doc.symbol:
-raise QAPISemError(doc.info,
-   "Documention for '%s' is not followed"
-   " by the definition" % doc.symbol)
-
 body = str(doc.body)
 if re.search(r'@\S+:', body, re.MULTILINE):
 raise QAPISemError(doc.info,
diff --git a/tests/qapi-schema/doc-bad-expr.err 
b/tests/qapi-schema/doc-bad-expr.err
index e69de29..65e9d1f 100644
--- a/tests/qapi-schema/doc-bad-expr.err
+++ b/tests/qapi-schema/doc-bad-expr.err
@@ -0,0 +1 @@
+tests/qapi-schema/doc-bad-expr.json:3: Documentation for 'foo' is not followed 
by the definition
diff --git a/tests/qapi-schema/doc-bad-expr.exit 
b/tests/qapi-schema/doc-bad-expr.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/doc-bad-expr.exit
+++ b/tests/qapi-schema/doc-bad-expr.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/doc-bad-expr.json 
b/tests/qapi-schema/doc-bad-expr.json
index ec1fbf2..0caa0ae 100644
--- a/tests/qapi-schema/doc-bad-expr.json
+++ b/tests/qapi-schema/doc-bad-expr.json
@@ -1,5 +1,4 @@
 # Doc comment separated from defining expression by non-defining expression
-# BUG: not rejected
 
 ##
 # @foo:
diff --git a/tests/qapi-schema/doc-bad-expr.out 
b/tests/qapi-schema/doc-bad-expr.out
index 236a849..e69de29 100644
--- a/tests/qapi-schema/doc-bad-expr.out
+++ b/tests/qapi-schema/doc-bad-expr.out
@@ -1,4 +0,0 @@
-enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 
'qbool']
-prefix QTYPE
-object foo
-object q_empty
diff --git a/tests/qapi-schema/doc-missing-expr.err 
b/tests/qapi-schema/doc-missing-expr.err
index c0e687c..c909e26 100644
--- a/tests/qapi-schema/doc-missing-expr.err
+++ b/tests/qapi-schema/doc-missing-expr.err
@@ -1 +1 @@
-tests/qapi-schema/doc-missing-expr.json:3: Documention for 'bar' is not 
followed by the definition
+tests/qapi-schema/doc-missing-expr.json:3: Documentation for 'bar' is not 
followed by the definition
diff --git a/tests/qapi-schema/doc-no-symbol.err 
b/tests/qapi-schema/doc-no-symbol.err
index 727966c..75f032a 100644
--- a/tests/qapi-schema/doc-no-sym

[Qemu-devel] [PATCH for-2.9 46/47] qapi: Make pylint a bit happier

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-commands.py | 6 +++---
 scripts/qapi-visit.py| 1 -
 scripts/qapi.py  | 8 
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0c05449..1943de4 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -13,7 +13,6 @@
 # See the COPYING file in the top-level directory.
 
 from qapi import *
-import re
 
 
 def gen_command_decl(name, arg_type, boxed, ret_type):
@@ -84,7 +83,8 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, 
QObject **ret_out,
 
 
 def gen_marshal_proto(name):
-return 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % 
c_name(name)
+return ('void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)'
+% c_name(name))
 
 
 def gen_marshal_decl(name):
@@ -198,7 +198,7 @@ def gen_register_command(name, success_response):
 options = 'QCO_NO_SUCCESS_RESP'
 
 ret = mcgen('''
-qmp_register_command(cmds, "%(name)s", 
+qmp_register_command(cmds, "%(name)s",
  qmp_marshal_%(c_name)s, %(opts)s);
 ''',
 name=name, c_name=c_name(name),
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 3d3936e..5737aef 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -13,7 +13,6 @@
 # See the COPYING file in the top-level directory.
 
 from qapi import *
-import re
 
 
 def gen_visit_decl(name, scalar=False):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 4102011..a4bf1ea 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,13 +11,13 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-import re
-from ordereddict import OrderedDict
 import errno
 import getopt
 import os
-import sys
+import re
 import string
+import sys
+from ordereddict import OrderedDict
 
 builtin_types = {
 'str':  'QTYPE_QSTRING',
@@ -1582,7 +1582,7 @@ class QAPISchema(object):
 tag_member = None
 if isinstance(base, dict):
 base = (self._make_implicit_object_type(
-name, info, doc, 'base', self._make_members(base, info)))
+name, info, doc, 'base', self._make_members(base, info)))
 if tag_name:
 variants = [self._make_variant(key, value)
 for (key, value) in data.iteritems()]
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 14/47] qapi: Prepare for requiring more complete documentation

[Markus Armbruster]

We currently neglect to check all enumeration values, common members
of object types and members of alternate types are documented.
Unsurprisingly, many aren't.

Add the necessary plumbing to find undocumented ones, except for
variant members of object types.  Don't enforce anything just yet, but
connect each QAPIDoc.ArgSection to its QAPISchemaMember.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 110 +---
 1 file changed, 65 insertions(+), 45 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7a2b6ab..4886417 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -116,7 +116,12 @@ class QAPIDoc(object):
 return "\n".join(self.content).strip()
 
 class ArgSection(Section):
-pass
+def __init__(self, name):
+QAPIDoc.Section.__init__(self, name)
+self.member = None
+
+def connect(self, member):
+self.member = member
 
 def __init__(self, parser, info):
 # self.parser is used to report errors with QAPIParseError.  The
@@ -216,6 +221,13 @@ class QAPIDoc(object):
 line = line.strip()
 self.section.append(line)
 
+def connect_member(self, member):
+if not member.name in self.args:
+# Undocumented TODO outlaw
+pass
+else:
+self.args[member.name].connect(member)
+
 
 class QAPISchemaParser(object):
 
@@ -1017,7 +1029,7 @@ def check_docs(docs):
 #
 
 class QAPISchemaEntity(object):
-def __init__(self, name, info):
+def __init__(self, name, info, doc):
 assert isinstance(name, str)
 self.name = name
 # For explicitly defined entities, info points to the (explicit)
@@ -1026,6 +1038,7 @@ class QAPISchemaEntity(object):
 # triggered the implicit definition (there may be more than one
 # such place).
 self.info = info
+self.doc = doc
 
 def c_name(self):
 return c_name(self.name)
@@ -1107,7 +1120,7 @@ class QAPISchemaType(QAPISchemaEntity):
 
 class QAPISchemaBuiltinType(QAPISchemaType):
 def __init__(self, name, json_type, c_type):
-QAPISchemaType.__init__(self, name, None)
+QAPISchemaType.__init__(self, name, None, None)
 assert not c_type or isinstance(c_type, str)
 assert json_type in ('string', 'number', 'int', 'boolean', 'null',
  'value')
@@ -1133,8 +1146,8 @@ class QAPISchemaBuiltinType(QAPISchemaType):
 
 
 class QAPISchemaEnumType(QAPISchemaType):
-def __init__(self, name, info, values, prefix):
-QAPISchemaType.__init__(self, name, info)
+def __init__(self, name, info, doc, values, prefix):
+QAPISchemaType.__init__(self, name, info, doc)
 for v in values:
 assert isinstance(v, QAPISchemaMember)
 v.set_owner(name)
@@ -1146,6 +1159,8 @@ class QAPISchemaEnumType(QAPISchemaType):
 seen = {}
 for v in self.values:
 v.check_clash(self.info, seen)
+if self.doc:
+self.doc.connect_member(v)
 
 def is_implicit(self):
 # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
@@ -1167,7 +1182,7 @@ class QAPISchemaEnumType(QAPISchemaType):
 
 class QAPISchemaArrayType(QAPISchemaType):
 def __init__(self, name, info, element_type):
-QAPISchemaType.__init__(self, name, info)
+QAPISchemaType.__init__(self, name, info, None)
 assert isinstance(element_type, str)
 self._element_type_name = element_type
 self.element_type = None
@@ -1190,11 +1205,11 @@ class QAPISchemaArrayType(QAPISchemaType):
 
 
 class QAPISchemaObjectType(QAPISchemaType):
-def __init__(self, name, info, base, local_members, variants):
+def __init__(self, name, info, doc, base, local_members, variants):
 # struct has local_members, optional base, and no variants
 # flat union has base, variants, and no local_members
 # simple union has local_members, variants, and no base
-QAPISchemaType.__init__(self, name, info)
+QAPISchemaType.__init__(self, name, info, doc)
 assert base is None or isinstance(base, str)
 for m in local_members:
 assert isinstance(m, QAPISchemaObjectTypeMember)
@@ -1224,6 +1239,8 @@ class QAPISchemaObjectType(QAPISchemaType):
 for m in self.local_members:
 m.check(schema)
 m.check_clash(self.info, seen)
+if self.doc:
+self.doc.connect_member(m)
 self.members = seen.values()
 if self.variants:
 self.variants.check(schema, seen)
@@ -1378,8 +1395,8 @@ class 
QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
 
 
 class QAPISchemaAlternateType(QAPISchemaType):
-def __init__(self, name, info, variants):
-QAPISchemaType.__init__(self, name, info)
+def __init__(self, name, info, doc, variants):
+QAPISchemaT

[Qemu-devel] [PATCH for-2.9 32/47] qapi: Move detection of doc / expression name mismatch

[Markus Armbruster]

Move the check whether the doc matches the expression name from
check_definition_doc() to check_exprs().  This changes the error
location from the comment to the expression.  Makes sense as the
message talks about the expresion: "Definition of '%s' follows
documentation for '%s'".  It's also a step towards getting rid of
check_docs().

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py  | 28 ++--
 tests/qapi-schema/doc-bad-symbol.err |  2 +-
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1052274..ca8d1f0 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -889,40 +889,52 @@ def check_keys(expr_elem, meta, required, optional=[]):
 def check_exprs(exprs):
 global all_names
 
-# Learn the types and check for valid expression keys
+# Populate name table with names of built-in types
 for builtin in builtin_types.keys():
 all_names[builtin] = 'built-in'
+
+# Learn the types and check for valid expression keys
 for expr_elem in exprs:
 expr = expr_elem['expr']
 info = expr_elem['info']
+doc = expr_elem.get('doc')
 
-if 'doc' not in expr_elem and doc_required:
+if not doc and doc_required:
 raise QAPISemError(info,
"Expression missing documentation comment")
 
 if 'enum' in expr:
+name = expr['enum']
 check_keys(expr_elem, 'enum', ['data'], ['prefix'])
-add_enum(expr['enum'], info, expr['data'])
+add_enum(name, info, expr['data'])
 elif 'union' in expr:
+name = expr['union']
 check_keys(expr_elem, 'union', ['data'],
['base', 'discriminator'])
 add_union(expr, info)
 elif 'alternate' in expr:
+name = expr['alternate']
 check_keys(expr_elem, 'alternate', ['data'])
-add_name(expr['alternate'], info, 'alternate')
+add_name(name, info, 'alternate')
 elif 'struct' in expr:
+name = expr['struct']
 check_keys(expr_elem, 'struct', ['data'], ['base'])
 add_struct(expr, info)
 elif 'command' in expr:
+name = expr['command']
 check_keys(expr_elem, 'command', [],
['data', 'returns', 'gen', 'success-response', 'boxed'])
-add_name(expr['command'], info, 'command')
+add_name(name, info, 'command')
 elif 'event' in expr:
+name = expr['event']
 check_keys(expr_elem, 'event', [], ['data', 'boxed'])
-add_name(expr['event'], info, 'event')
+add_name(name, info, 'event')
 else:
 raise QAPISemError(expr_elem['info'],
"Expression is missing metatype")
+if doc and doc.symbol != name:
+raise QAPISemError(info, "Definition of '%s' follows documentation"
+   " for '%s'" % (name, doc.symbol))
 
 # Try again for hidden UnionKind enum
 for expr_elem in exprs:
@@ -972,10 +984,6 @@ def check_definition_doc(doc, expr, info):
 meta = i
 break
 
-name = expr[meta]
-if doc.symbol != name:
-raise QAPISemError(info, "Definition of '%s' follows documentation"
-   " for '%s'" % (name, doc.symbol))
 if doc.has_section('Returns') and 'command' not in expr:
 raise QAPISemError(info, "'Returns:' is only valid for commands")
 
diff --git a/tests/qapi-schema/doc-bad-symbol.err 
b/tests/qapi-schema/doc-bad-symbol.err
index ac4e566..8472030 100644
--- a/tests/qapi-schema/doc-bad-symbol.err
+++ b/tests/qapi-schema/doc-bad-symbol.err
@@ -1 +1 @@
-tests/qapi-schema/doc-bad-symbol.json:3: Definition of 'foo' follows 
documentation for 'food'
+tests/qapi-schema/doc-bad-symbol.json:6: Definition of 'foo' follows 
documentation for 'food'
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 37/47] qapi: Fix detection of bogus member documentation

[Markus Armbruster]

check_definition_doc() checks for member documentation without a
matching member.  It laboriously second-guesses what members
QAPISchema._def_exprs() will create.  That's a stupid game.

Move the check into QAPISchema.check(), where the members are known.
Delegate the actual checking to new QAPIDoc.check().

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 38 ++---
 tests/qapi-schema/doc-bad-union-member.err  |  1 +
 tests/qapi-schema/doc-bad-union-member.exit |  2 +-
 tests/qapi-schema/doc-bad-union-member.out  | 11 -
 4 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e53701a..0b0ba59 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -249,6 +249,15 @@ class QAPIDoc(object):
 self.args[member.name] = QAPIDoc.ArgSection(member.name)
 self.args[member.name].connect(member)
 
+def check(self):
+bogus = [name for name, section in self.args.iteritems()
+ if not section.member]
+if bogus:
+raise QAPISemError(
+self.info,
+"The following documented members are not in "
+"the declaration: %s" % ", ".join(bogus))
+
 
 class QAPISchemaParser(object):
 
@@ -990,34 +999,9 @@ def check_exprs(exprs):
 
 
 def check_definition_doc(doc, expr, info):
-for i in ('enum', 'union', 'alternate', 'struct', 'command', 'event'):
-if i in expr:
-meta = i
-break
-
 if doc.has_section('Returns') and 'command' not in expr:
 raise QAPISemError(info, "'Returns:' is only valid for commands")
 
-if meta == 'union':
-args = expr.get('base', [])
-else:
-args = expr.get('data', [])
-if isinstance(args, str):
-return
-if isinstance(args, dict):
-args = args.keys()
-assert isinstance(args, list)
-
-if (meta == 'alternate'
-or (meta == 'union' and not expr.get('discriminator'))):
-args.append('type')
-
-doc_args = set(doc.args.keys())
-args = set([name.strip('*') for name in args])
-if not doc_args.issubset(args):
-raise QAPISemError(info, "The following documented members are not in "
-   "the declaration: %s" % ', '.join(doc_args - args))
-
 
 def check_docs(docs):
 for doc in docs:
@@ -1263,6 +1247,8 @@ class QAPISchemaObjectType(QAPISchemaType):
 self.variants.check(schema, seen)
 assert self.variants.tag_member in self.members
 self.variants.check_clash(schema, self.info, seen)
+if self.doc:
+self.doc.check()
 
 # Check that the members of this type do not cause duplicate JSON members,
 # and update seen to track the members seen so far. Report any errors
@@ -1432,6 +1418,8 @@ class QAPISchemaAlternateType(QAPISchemaType):
 v.check_clash(self.info, seen)
 if self.doc:
 self.doc.connect_member(v)
+if self.doc:
+self.doc.check()
 
 def c_type(self):
 return c_name(self.name) + pointer_suffix
diff --git a/tests/qapi-schema/doc-bad-union-member.err 
b/tests/qapi-schema/doc-bad-union-member.err
index e69de29..4b016df 100644
--- a/tests/qapi-schema/doc-bad-union-member.err
+++ b/tests/qapi-schema/doc-bad-union-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/doc-bad-union-member.json:3: The following documented 
members are not in the declaration: a, b
diff --git a/tests/qapi-schema/doc-bad-union-member.exit 
b/tests/qapi-schema/doc-bad-union-member.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/doc-bad-union-member.exit
+++ b/tests/qapi-schema/doc-bad-union-member.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/doc-bad-union-member.out 
b/tests/qapi-schema/doc-bad-union-member.out
index 2576ecd..e69de29 100644
--- a/tests/qapi-schema/doc-bad-union-member.out
+++ b/tests/qapi-schema/doc-bad-union-member.out
@@ -1,11 +0,0 @@
-object Base
-member type: T optional=False
-object Empty
-object Frob
-base Base
-tag type
-case nothing: Empty
-enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 
'qbool']
-prefix QTYPE
-enum T ['nothing']
-object q_empty
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 16/47] qapi2texi: Convert to QAPISchemaVisitor

[Markus Armbruster]

qapi2texi works with schema expression trees.  Such a tight coupling
to schema language syntax is not a good idea.  Convert it to the visitor
interface the other generators use.

No change to generated documentation.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi2texi.py | 228 ++-
 1 file changed, 118 insertions(+), 110 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 299dcf9..6d4e757 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -123,31 +123,39 @@ def texi_format(doc):
 return "\n".join(lines)
 
 
-def texi_body(doc, only_documented=False):
-"""
-Format the body of a symbol documentation:
-- main body
-- table of arguments
-- followed by "Returns/Notes/Since/Example" sections
-"""
-body = texi_format(str(doc.body)) + "\n"
-
-args = ''
-for name, section in doc.args.iteritems():
-if not section.content and not only_documented:
-continue# Undocumented TODO require doc and drop
-desc = str(section)
-opt = ''
-if section.optional:
-desc = re.sub(r'^ *#optional *\n?|\n? *#optional *$|#optional',
-  '', desc)
-opt = ' (optional)'
-args += "@item @code{'%s'}%s\n%s\n" % (name, opt, texi_format(desc))
-if args:
-body += "@table @asis\n"
-body += args
-body += "@end table\n"
+def texi_body(doc):
+"""Format the main documentation body"""
+return texi_format(str(doc.body)) + '\n'
 
+
+def texi_enum_value(value):
+"""Format a table of members item for an enumeration value"""
+return "@item @code{'%s'}\n" % value.name
+
+
+def texi_member(member):
+"""Format a table of members item for an object type member"""
+return "@item @code{'%s'}%s\n" % (
+member.name, ' (optional)' if member.optional else '')
+
+
+def texi_members(doc, member_func, show_undocumented):
+"""Format the table of members"""
+items = ''
+for section in doc.args.itervalues():
+if not section.content and not show_undocumented:
+continue  # Undocumented TODO require doc and drop
+desc = re.sub(r'^ *#optional *\n?|\n? *#optional *$|#optional',
+  '', str(section))
+items += member_func(section.member) + texi_format(desc) + '\n'
+if not items:
+return ''
+return '@table @asis\n' + items + '@end table\n'
+
+
+def texi_sections(doc):
+"""Format additional sections following arguments"""
+body = ''
 for section in doc.sections:
 name, doc = (section.name, str(section))
 func = texi_format
@@ -159,94 +167,94 @@ def texi_body(doc, only_documented=False):
 body += "\n\n@b{%s:}\n" % name
 
 body += func(doc)
-
 return body
 
 
-def texi_alternate(expr, doc):
-"""Format an alternate to texi"""
-body = texi_body(doc)
-return TYPE_FMT(type="Alternate",
-name=doc.symbol,
-body=body)
-
-
-def texi_union(expr, doc):
-"""Format a union to texi"""
-discriminator = expr.get("discriminator")
-if discriminator:
-union = "Flat Union"
-else:
-union = "Simple Union"
-
-body = texi_body(doc)
-return TYPE_FMT(type=union,
-name=doc.symbol,
-body=body)
-
-
-def texi_enum(expr, doc):
-"""Format an enum to texi"""
-body = texi_body(doc, True)
-return TYPE_FMT(type="Enum",
-name=doc.symbol,
-body=body)
-
-
-def texi_struct(expr, doc):
-"""Format a struct to texi"""
-body = texi_body(doc)
-return TYPE_FMT(type="Struct",
-name=doc.symbol,
-body=body)
-
-
-def texi_command(expr, doc):
-"""Format a command to texi"""
-body = texi_body(doc)
-return MSG_FMT(type="Command",
-   name=doc.symbol,
-   body=body)
-
-
-def texi_event(expr, doc):
-"""Format an event to texi"""
-body = texi_body(doc)
-return MSG_FMT(type="Event",
-   name=doc.symbol,
-   body=body)
-
-
-def texi_expr(expr, doc):
-"""Format an expr to texi"""
-(kind, _) = expr.items()[0]
-
-fmt = {"command": texi_command,
-   "struct": texi_struct,
-   "enum": texi_enum,
-   "union": texi_union,
-   "alternate": texi_alternate,
-   "event": texi_event}[kind]
-
-return fmt(expr, doc)
-
-
-def texi(docs):
-"""Convert QAPI schema expressions to texi documentation"""
-res = []
-for doc in docs:
-expr = doc.expr
-if not expr:
-res.append(texi_body(doc))
-continue
-try:
-doc = texi_expr(expr, doc)
-res.append(doc)
-except:
-print >>sys.stderr, "error at @%s" % doc.info
-raise
-
-return '\n'.join(res)
+d

[Qemu-devel] [PATCH for-2.9 36/47] tests/qapi-schema: Improve coverage of bogus member docs

[Markus Armbruster]

New test doc-bad-union-member.json shows we can fail to reject
documentation for nonexistent members.

Signed-off-by: Markus Armbruster 
---
 tests/Makefile.include  |  2 ++
 tests/qapi-schema/doc-bad-alternate-member.err  |  1 +
 tests/qapi-schema/doc-bad-alternate-member.exit |  1 +
 tests/qapi-schema/doc-bad-alternate-member.json |  9 +
 tests/qapi-schema/doc-bad-alternate-member.out  |  0
 tests/qapi-schema/doc-bad-union-member.err  |  0
 tests/qapi-schema/doc-bad-union-member.exit |  1 +
 tests/qapi-schema/doc-bad-union-member.json | 19 +++
 tests/qapi-schema/doc-bad-union-member.out  | 11 +++
 9 files changed, 44 insertions(+)
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.err
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.exit
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.json
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.out
 create mode 100644 tests/qapi-schema/doc-bad-union-member.err
 create mode 100644 tests/qapi-schema/doc-bad-union-member.exit
 create mode 100644 tests/qapi-schema/doc-bad-union-member.json
 create mode 100644 tests/qapi-schema/doc-bad-union-member.out

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8ce9d3d..5edf3fa 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -367,9 +367,11 @@ qapi-schema += base-cycle-direct.json
 qapi-schema += base-cycle-indirect.json
 qapi-schema += command-int.json
 qapi-schema += comments.json
+qapi-schema += doc-bad-alternate-member.json
 qapi-schema += doc-bad-command-arg.json
 qapi-schema += doc-bad-expr.json
 qapi-schema += doc-bad-symbol.json
+qapi-schema += doc-bad-union-member.json
 qapi-schema += doc-duplicated-arg.json
 qapi-schema += doc-duplicated-return.json
 qapi-schema += doc-duplicated-since.json
diff --git a/tests/qapi-schema/doc-bad-alternate-member.err 
b/tests/qapi-schema/doc-bad-alternate-member.err
new file mode 100644
index 000..387f782
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-alternate-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/doc-bad-alternate-member.json:3: The following documented 
members are not in the declaration: aa, bb
diff --git a/tests/qapi-schema/doc-bad-alternate-member.exit 
b/tests/qapi-schema/doc-bad-alternate-member.exit
new file mode 100644
index 000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-alternate-member.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/doc-bad-alternate-member.json 
b/tests/qapi-schema/doc-bad-alternate-member.json
new file mode 100644
index 000..738635c
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-alternate-member.json
@@ -0,0 +1,9 @@
+# Arguments listed in the doc comment must exist in the actual schema
+
+##
+# @AorB:
+# @aa: a
+# @bb: b
+##
+{ 'alternate': 'AorB',
+  'data': { 'a': 'str', 'b': 'int' } }
diff --git a/tests/qapi-schema/doc-bad-alternate-member.out 
b/tests/qapi-schema/doc-bad-alternate-member.out
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/doc-bad-union-member.err 
b/tests/qapi-schema/doc-bad-union-member.err
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/doc-bad-union-member.exit 
b/tests/qapi-schema/doc-bad-union-member.exit
new file mode 100644
index 000..573541a
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-union-member.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/doc-bad-union-member.json 
b/tests/qapi-schema/doc-bad-union-member.json
new file mode 100644
index 000..d611435
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-union-member.json
@@ -0,0 +1,19 @@
+# Arguments listed in the doc comment must exist in the actual schema
+
+##
+# @Frob:
+# @a: a
+# @b: b
+##
+{ 'union': 'Frob',
+  'base': 'Base',
+  'discriminator': 'type',
+  'data': { 'nothing': 'Empty' } }
+
+{ 'struct': 'Base',
+  'data': { 'type': 'T' } }
+
+{ 'struct': 'Empty',
+  'data': { } }
+
+{ 'enum': 'T', 'data': ['nothing'] }
diff --git a/tests/qapi-schema/doc-bad-union-member.out 
b/tests/qapi-schema/doc-bad-union-member.out
new file mode 100644
index 000..2576ecd
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-union-member.out
@@ -0,0 +1,11 @@
+object Base
+member type: T optional=False
+object Empty
+object Frob
+base Base
+tag type
+case nothing: Empty
+enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 
'qbool']
+prefix QTYPE
+enum T ['nothing']
+object q_empty
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 45/47] qapi: Drop unused .check_clash() parameter schema

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 870ff4e..4102011 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1178,7 +1178,7 @@ class QAPISchemaObjectType(QAPISchemaType):
 self.base = schema.lookup_type(self._base_name)
 assert isinstance(self.base, QAPISchemaObjectType)
 self.base.check(schema)
-self.base.check_clash(schema, self.info, seen)
+self.base.check_clash(self.info, seen)
 for m in self.local_members:
 m.check(schema)
 m.check_clash(self.info, seen)
@@ -1188,14 +1188,14 @@ class QAPISchemaObjectType(QAPISchemaType):
 if self.variants:
 self.variants.check(schema, seen)
 assert self.variants.tag_member in self.members
-self.variants.check_clash(schema, self.info, seen)
+self.variants.check_clash(self.info, seen)
 if self.doc:
 self.doc.check()
 
 # Check that the members of this type do not cause duplicate JSON members,
 # and update seen to track the members seen so far. Report any errors
 # on behalf of info, which is not necessarily self.info
-def check_clash(self, schema, info, seen):
+def check_clash(self, info, seen):
 assert not self.variants   # not implemented
 for m in self.members:
 m.check_clash(info, seen)
@@ -1324,12 +1324,12 @@ class QAPISchemaObjectTypeVariants(object):
 assert isinstance(v.type, QAPISchemaObjectType)
 v.type.check(schema)
 
-def check_clash(self, schema, info, seen):
+def check_clash(self, info, seen):
 for v in self.variants:
 # Reset seen map for each variant, since qapi names from one
 # branch do not affect another branch
 assert isinstance(v.type, QAPISchemaObjectType)
-v.type.check_clash(schema, info, dict(seen))
+v.type.check_clash(info, dict(seen))
 
 
 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 30/47] tests/qapi-schema: Improve doc / expression mismatch coverage

[Markus Armbruster]

New test doc-bad-expr.json shows we fail to reject a misplaced
expression comment.

New test doc-no-symbol.json shows a bad error message.

Signed-off-by: Markus Armbruster 
---
 tests/Makefile.include   | 2 ++
 tests/qapi-schema/doc-bad-expr.err   | 0
 tests/qapi-schema/doc-bad-expr.exit  | 1 +
 tests/qapi-schema/doc-bad-expr.json  | 8 
 tests/qapi-schema/doc-bad-expr.out   | 4 
 tests/qapi-schema/doc-no-symbol.err  | 1 +
 tests/qapi-schema/doc-no-symbol.exit | 1 +
 tests/qapi-schema/doc-no-symbol.json | 7 +++
 tests/qapi-schema/doc-no-symbol.out  | 0
 9 files changed, 24 insertions(+)
 create mode 100644 tests/qapi-schema/doc-bad-expr.err
 create mode 100644 tests/qapi-schema/doc-bad-expr.exit
 create mode 100644 tests/qapi-schema/doc-bad-expr.json
 create mode 100644 tests/qapi-schema/doc-bad-expr.out
 create mode 100644 tests/qapi-schema/doc-no-symbol.err
 create mode 100644 tests/qapi-schema/doc-no-symbol.exit
 create mode 100644 tests/qapi-schema/doc-no-symbol.json
 create mode 100644 tests/qapi-schema/doc-no-symbol.out

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7230977..800acbf 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -368,6 +368,7 @@ qapi-schema += base-cycle-indirect.json
 qapi-schema += command-int.json
 qapi-schema += comments.json
 qapi-schema += doc-bad-args.json
+qapi-schema += doc-bad-expr.json
 qapi-schema += doc-bad-symbol.json
 qapi-schema += doc-duplicated-arg.json
 qapi-schema += doc-duplicated-return.json
@@ -385,6 +386,7 @@ qapi-schema += doc-missing.json
 qapi-schema += doc-missing-colon.json
 qapi-schema += doc-missing-expr.json
 qapi-schema += doc-missing-space.json
+qapi-schema += doc-no-symbol.json
 qapi-schema += double-data.json
 qapi-schema += double-type.json
 qapi-schema += duplicate-key.json
diff --git a/tests/qapi-schema/doc-bad-expr.err 
b/tests/qapi-schema/doc-bad-expr.err
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/doc-bad-expr.exit 
b/tests/qapi-schema/doc-bad-expr.exit
new file mode 100644
index 000..573541a
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-expr.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/doc-bad-expr.json 
b/tests/qapi-schema/doc-bad-expr.json
new file mode 100644
index 000..ec1fbf2
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-expr.json
@@ -0,0 +1,8 @@
+# Doc comment separated from defining expression by non-defining expression
+# BUG: not rejected
+
+##
+# @foo:
+##
+{ 'include': 'empty.json' }
+{ 'struct': 'foo', 'data': {} }
diff --git a/tests/qapi-schema/doc-bad-expr.out 
b/tests/qapi-schema/doc-bad-expr.out
new file mode 100644
index 000..236a849
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-expr.out
@@ -0,0 +1,4 @@
+enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 
'qbool']
+prefix QTYPE
+object foo
+object q_empty
diff --git a/tests/qapi-schema/doc-no-symbol.err 
b/tests/qapi-schema/doc-no-symbol.err
new file mode 100644
index 000..727966c
--- /dev/null
+++ b/tests/qapi-schema/doc-no-symbol.err
@@ -0,0 +1 @@
+tests/qapi-schema/doc-no-symbol.json:4: Definition of 'foo' follows 
documentation for 'None'
diff --git a/tests/qapi-schema/doc-no-symbol.exit 
b/tests/qapi-schema/doc-no-symbol.exit
new file mode 100644
index 000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/doc-no-symbol.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/doc-no-symbol.json 
b/tests/qapi-schema/doc-no-symbol.json
new file mode 100644
index 000..ee86ca1
--- /dev/null
+++ b/tests/qapi-schema/doc-no-symbol.json
@@ -0,0 +1,7 @@
+# Documentation for expression lacks symbol
+# BUG: Error message claims it has symbol 'None'
+
+##
+# foo:
+##
+{ 'command': 'foo', 'data': {'a': 'int'} }
diff --git a/tests/qapi-schema/doc-no-symbol.out 
b/tests/qapi-schema/doc-no-symbol.out
new file mode 100644
index 000..e69de29
-- 
2.7.4

[Qemu-devel] [PATCH V2 1/3] virtio: guard against NULL pfn

[Jason Wang]

To avoid access stale memory region cache after reset, this patch
check the existence of virtqueue pfn for all exported virtqueue access
helpers before trying to use them.

Cc: Cornelia Huck 
Cc: Paolo Bonzini 
Signed-off-by: Jason Wang 
---
 hw/virtio/virtio.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index efce4b3..76cc81b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -322,6 +322,10 @@ static int virtio_queue_empty_rcu(VirtQueue *vq)
 return 0;
 }
 
+if (unlikely(!vq->vring.avail)) {
+return 0;
+}
+
 return vring_avail_idx(vq) == vq->last_avail_idx;
 }
 
@@ -333,6 +337,10 @@ int virtio_queue_empty(VirtQueue *vq)
 return 0;
 }
 
+if (unlikely(!vq->vring.avail)) {
+return 0;
+}
+
 rcu_read_lock();
 empty = vring_avail_idx(vq) == vq->last_avail_idx;
 rcu_read_unlock();
@@ -431,6 +439,10 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement 
*elem,
 return;
 }
 
+if (unlikely(!vq->vring.used)) {
+return;
+}
+
 idx = (idx + vq->used_idx) % vq->vring.num;
 
 uelem.id = elem->index;
@@ -448,6 +460,10 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count)
 return;
 }
 
+if (unlikely(!vq->vring.used)) {
+return;
+}
+
 /* Make sure buffer is written before we update index. */
 smp_wmb();
 trace_virtqueue_flush(vq, count);
@@ -546,6 +562,11 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 int64_t len = 0;
 int rc;
 
+if (unlikely(!vq->vring.desc)) {
+*in_bytes = *out_bytes = 0;
+return;
+}
+
 rcu_read_lock();
 idx = vq->last_avail_idx;
 total_bufs = in_total = out_total = 0;
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 41/47] qapi: Factor add_name() calls out of the meta conditional

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index ffd30d2..f06e3c4 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -635,8 +635,6 @@ def add_name(name, info, meta, implicit=False):
 
 def add_struct(definition, info):
 global struct_types
-name = definition['struct']
-add_name(name, info, 'struct')
 struct_types.append(definition)
 
 
@@ -650,8 +648,6 @@ def find_struct(name):
 
 def add_union(definition, info):
 global union_types
-name = definition['union']
-add_name(name, info, 'union')
 union_types.append(definition)
 
 
@@ -665,8 +661,6 @@ def find_union(name):
 
 def add_enum(definition, info):
 global enum_types
-name = definition['enum']
-add_name(name, info, 'enum', 'data' not in definition)
 enum_types.append(definition)
 
 
@@ -932,34 +926,33 @@ def check_exprs(exprs):
"Expression missing documentation comment")
 
 if 'enum' in expr:
-name = expr['enum']
+meta = 'enum'
 check_keys(expr_elem, 'enum', ['data'], ['prefix'])
 add_enum(expr, info)
 elif 'union' in expr:
-name = expr['union']
+meta = 'union'
 check_keys(expr_elem, 'union', ['data'],
['base', 'discriminator'])
 add_union(expr, info)
 elif 'alternate' in expr:
-name = expr['alternate']
+meta = 'alternate'
 check_keys(expr_elem, 'alternate', ['data'])
-add_name(name, info, 'alternate')
 elif 'struct' in expr:
-name = expr['struct']
+meta = 'struct'
 check_keys(expr_elem, 'struct', ['data'], ['base'])
 add_struct(expr, info)
 elif 'command' in expr:
-name = expr['command']
+meta = 'command'
 check_keys(expr_elem, 'command', [],
['data', 'returns', 'gen', 'success-response', 'boxed'])
-add_name(name, info, 'command')
 elif 'event' in expr:
-name = expr['event']
+meta = 'event'
 check_keys(expr_elem, 'event', [], ['data', 'boxed'])
-add_name(name, info, 'event')
 else:
 raise QAPISemError(expr_elem['info'],
"Expression is missing metatype")
+name = expr[meta]
+add_name(name, info, meta)
 if doc and doc.symbol != name:
 raise QAPISemError(info, "Definition of '%s' follows documentation"
" for '%s'" % (name, doc.symbol))
@@ -974,6 +967,7 @@ def check_exprs(exprs):
 else:
 continue
 add_enum({ 'enum': name }, expr_elem['info'])
+add_name(name, info, 'enum', implicit=True)
 
 # Validate that exprs make sense
 for expr_elem in exprs:
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 47/47] qapi: Fix a misleading parser error message

[Markus Armbruster]

When choking on a token where an expression is expected, we report
'Expected "{", "[" or string'.  Close, but no cigar.  Fix it to
Expected '"{", "[", string, boolean or "null"'.

Missed in commit e53188a.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py   | 3 ++-
 tests/qapi-schema/trailing-comma-list.err | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index a4bf1ea..1dc33c9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -514,7 +514,8 @@ class QAPISchemaParser(object):
 expr = self.val
 self.accept()
 else:
-raise QAPIParseError(self, 'Expected "{", "[" or string')
+raise QAPIParseError(self, 'Expected "{", "[", string, '
+ 'boolean or "null"')
 return expr
 
 def get_doc(self, info):
diff --git a/tests/qapi-schema/trailing-comma-list.err 
b/tests/qapi-schema/trailing-comma-list.err
index 24c24b0..212e14a 100644
--- a/tests/qapi-schema/trailing-comma-list.err
+++ b/tests/qapi-schema/trailing-comma-list.err
@@ -1 +1 @@
-tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[" or string
+tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[", string, 
boolean or "null"
-- 
2.7.4

[Qemu-devel] [PATCH for-2.9 33/47] qapi: Improve error message on @NAME: in free-form doc

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py   | 17 ++---
 tests/qapi-schema/doc-invalid-section.err |  2 +-
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index ca8d1f0..8d34651 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -219,6 +219,11 @@ class QAPIDoc(object):
 if (in_arg or not self.section.name
 or not self.section.name.startswith('Example')):
 line = line.strip()
+match = re.match(r'(@\S+:)', line)
+if match:
+raise QAPIParseError(self.parser,
+ "'%s' not allowed in free-form documentation"
+ % match.group(1))
 # TODO Drop this once the dust has settled
 if (isinstance(self.section, QAPIDoc.ArgSection)
 and '#optional' in line):
@@ -970,14 +975,6 @@ def check_exprs(exprs):
 return exprs
 
 
-def check_freeform_doc(doc):
-body = str(doc.body)
-if re.search(r'@\S+:', body, re.MULTILINE):
-raise QAPISemError(doc.info,
-   "Free-form documentation block must not contain"
-   " @NAME: sections")
-
-
 def check_definition_doc(doc, expr, info):
 for i in ('enum', 'union', 'alternate', 'struct', 'command', 'event'):
 if i in expr:
@@ -1016,9 +1013,7 @@ def check_docs(docs):
 raise QAPISemError(doc.info,
"Empty doc section '%s'" % section.name)
 
-if not doc.expr:
-check_freeform_doc(doc)
-else:
+if doc.expr:
 check_definition_doc(doc, doc.expr, doc.info)
 
 return docs
diff --git a/tests/qapi-schema/doc-invalid-section.err 
b/tests/qapi-schema/doc-invalid-section.err
index 85bb67b..bda93b4 100644
--- a/tests/qapi-schema/doc-invalid-section.err
+++ b/tests/qapi-schema/doc-invalid-section.err
@@ -1 +1 @@
-tests/qapi-schema/doc-invalid-section.json:3: Free-form documentation block 
must not contain @NAME: sections
+tests/qapi-schema/doc-invalid-section.json:5:1: '@note:' not allowed in 
free-form documentation
-- 
2.7.4

Re: [Qemu-devel] [PATCH 0/3] COLO-compare: Make COLO-compare support Xen

[Zhang Chen]

Hi~~~ All~

No news for a long time, anyone can give me some comments?


Thanks

Zhang Chen


On 02/28/2017 06:48 PM, Zhang Chen wrote:

This series focus on COLO Xen support.
We add a new chardev socket in colo-compare as the way
of communicate with Xen COLO-frame.
Xen part:
https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg02067.html

This series is a frame, have some TODO job depend on this patch:
https://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05055.html


Zhang Chen (3):
   COLO-compare: Add new parameter for communicate with Xen colo-frame
   COLO-compare: Add Xen notify chardev socket handler frame
   COLO-compare: Add colo-compare Xen notify

  net/colo-compare.c | 77 ++
  qemu-options.hx| 38 ---
  2 files changed, 111 insertions(+), 4 deletions(-)



--
Thanks
Zhang Chen

[Qemu-devel] [PATCH for-2.9 44/47] qapi: union_types is a list used like a dict, make it one

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index ab266db..870ff4e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -48,7 +48,7 @@ name_case_whitelist = []
 
 enum_types = {}
 struct_types = {}
-union_types = []
+union_types = {}
 all_names = {}
 
 #
@@ -564,7 +564,7 @@ def find_alternate_member_qtype(qapi_type):
 return 'QTYPE_QDICT'
 elif qapi_type in enum_types:
 return 'QTYPE_QSTRING'
-elif find_union(qapi_type):
+elif qapi_type in union_types:
 return 'QTYPE_QDICT'
 return None
 
@@ -633,19 +633,6 @@ def add_name(name, info, meta, implicit=False):
 all_names[name] = meta
 
 
-def add_union(definition, info):
-global union_types
-union_types.append(definition)
-
-
-def find_union(name):
-global union_types
-for union in union_types:
-if union['union'] == name:
-return union
-return None
-
-
 def check_type(info, source, value, allow_array=False,
allow_dict=False, allow_optional=False,
allow_metas=[]):
@@ -903,7 +890,7 @@ def check_exprs(exprs):
 meta = 'union'
 check_keys(expr_elem, 'union', ['data'],
['base', 'discriminator'])
-add_union(expr, info)
+union_types[expr[meta]] = expr
 elif 'alternate' in expr:
 meta = 'alternate'
 check_keys(expr_elem, 'alternate', ['data'])
-- 
2.7.4

Re: [Qemu-devel] [PATCH for-2.9 34/47] qapi: Move empty doc section checking to doc parser

[Markus Armbruster]

Markus Armbruster  writes:

> Results in a more precise error location, but the real reason is
> emptying out check_docs() step by step.
>
> Signed-off-by: Markus Armbruster 

Perhaps we should simply drop this error condition.  Are empty sections
this a mistake users make accidentally?

[Qemu-devel] [PATCH for-2.9 19/47] qapi: Prefer single-quoted strings more consistently

[Markus Armbruster]

PEP 8 advises:

In Python, single-quoted strings and double-quoted strings are the
same.  This PEP does not make a recommendation for this.  Pick a
rule and stick to it.  When a string contains single or double
quote characters, however, use the other one to avoid backslashes
in the string.  It improves readability.

The QAPI generators succeed at picking a rule, but fail at sticking to
it.  Convert a bunch of double-quoted strings to single-quoted ones.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-event.py  |  2 +-
 scripts/qapi-introspect.py |  4 +-
 scripts/qapi-types.py  |  4 +-
 scripts/qapi-visit.py  |  4 +-
 scripts/qapi.py| 96 +++---
 scripts/qapi2texi.py   | 46 +++---
 6 files changed, 78 insertions(+), 78 deletions(-)

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index f4eb7f8..0485e39 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -223,7 +223,7 @@ fdecl.write(mcgen('''
 ''',
   prefix=prefix))
 
-event_enum_name = c_name(prefix + "QAPIEvent", protect=False)
+event_enum_name = c_name(prefix + 'QAPIEvent', protect=False)
 
 schema = QAPISchema(input_file)
 gen = QAPISchemaGenEventVisitor()
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index fb72c61..032bcea 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -170,10 +170,10 @@ const char %(c_name)s[] = %(c_string)s;
 opt_unmask = False
 
 (input_file, output_dir, do_c, do_h, prefix, opts) = \
-parse_command_line("u", ["unmask-non-abi-names"])
+parse_command_line('u', ['unmask-non-abi-names'])
 
 for o, a in opts:
-if o in ("-u", "--unmask-non-abi-names"):
+if o in ('-u', '--unmask-non-abi-names'):
 opt_unmask = True
 
 c_comment = '''
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index dabc42e..b45e7b5 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -244,10 +244,10 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
 do_builtins = False
 
 (input_file, output_dir, do_c, do_h, prefix, opts) = \
-parse_command_line("b", ["builtins"])
+parse_command_line('b', ['builtins'])
 
 for o, a in opts:
-if o in ("-b", "--builtins"):
+if o in ('-b', '--builtins'):
 do_builtins = True
 
 c_comment = '''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 330b9f3..3d3936e 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -335,10 +335,10 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
 do_builtins = False
 
 (input_file, output_dir, do_c, do_h, prefix, opts) = \
-parse_command_line("b", ["builtins"])
+parse_command_line('b', ['builtins'])
 
 for o, a in opts:
-if o in ("-b", "--builtins"):
+if o in ('-b', '--builtins'):
 do_builtins = True
 
 c_comment = '''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6d39ec9..9430493 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -58,9 +58,9 @@ all_names = {}
 
 
 def error_path(parent):
-res = ""
+res = ''
 while parent:
-res = ("In file included from %s:%d:\n" % (parent['file'],
+res = ('In file included from %s:%d:\n' % (parent['file'],
parent['line'])) + res
 parent = parent['parent']
 return res
@@ -76,10 +76,10 @@ class QAPIError(Exception):
 self.msg = msg
 
 def __str__(self):
-loc = "%s:%d" % (self.fname, self.line)
+loc = '%s:%d' % (self.fname, self.line)
 if self.col is not None:
-loc += ":%s" % self.col
-return error_path(self.info) + "%s: %s" % (loc, self.msg)
+loc += ':%s' % self.col
+return error_path(self.info) + '%s: %s' % (loc, self.msg)
 
 
 class QAPIParseError(QAPIError):
@@ -113,7 +113,7 @@ class QAPIDoc(object):
 self.content.append(line)
 
 def __repr__(self):
-return "\n".join(self.content).strip()
+return '\n'.join(self.content).strip()
 
 class ArgSection(Section):
 def __init__(self, name):
@@ -163,8 +163,8 @@ class QAPIDoc(object):
 # recognized, and get silently treated as ordinary text
 if self.symbol:
 self._append_symbol_line(line)
-elif not self.body.content and line.startswith("@"):
-if not line.endswith(":"):
+elif not self.body.content and line.startswith('@'):
+if not line.endswith(':'):
 raise QAPIParseError(self.parser, "Line should end with :")
 self.symbol = line[1:-1]
 # FIXME invalid names other than the empty string aren't flagged
@@ -176,14 +176,14 @@ class QAPIDoc(object):
 def _append_symbol_line(self, line):
 name = line.split(' ', 1)[0]
 
-if name.startswith("@") and name.endswith(":"):
+if name.startswith('@') and name.endswith(':'):
 line = line[len(name)

Re: [Qemu-devel] [PATCH 0/3] COLO-compare: Make COLO-compare support Xen

[Jason Wang]

On 2017年03月13日 14:18, Zhang Chen wrote:

Hi~~~ All~

No news for a long time, anyone can give me some comments?


Hi,

A question is why use two kinds of colo-frames? This seems not good as 
lots of the code were duplicated.


Thanks




Thanks

Zhang Chen


On 02/28/2017 06:48 PM, Zhang Chen wrote:

This series focus on COLO Xen support.
We add a new chardev socket in colo-compare as the way
of communicate with Xen COLO-frame.
Xen part:
https://lists.xenproject.org/archives/html/xen-devel/2017-02/msg02067.html 



This series is a frame, have some TODO job depend on this patch:
https://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05055.html


Zhang Chen (3):
   COLO-compare: Add new parameter for communicate with Xen colo-frame
   COLO-compare: Add Xen notify chardev socket handler frame
   COLO-compare: Add colo-compare Xen notify

  net/colo-compare.c | 77 
++

  qemu-options.hx| 38 ---
  2 files changed, 111 insertions(+), 4 deletions(-)

[Qemu-devel] [PATCH for-2.9 17/47] qapi: The #optional tag is redundant, drop

[Markus Armbruster]

We traditionally mark optional members #optional in the doc comment.
Before commit 3313b61, this was entirely manual.

Commit 3313b61 added some automation because its qapi2texi.py relied
on #optional to determine whether a member is optional.  This is no
longer the case since the previous commit: the only thing qapi2texi.py
still does with #optional is stripping it out.  We still reject bogus
qapi-schema.json and six places for qga/qapi-schema.json.

Thus, you can't actually rely on #optional to see whether something is
optional.  Yet we still make people add it manually.  That's just
busy-work.

Drop the code to check, fix up and strip out #optional, along with all
instances of #optional.  To keep it out, add code to reject it, to be
dropped again once the dust settles.

No change to generated documentation.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt  |  16 +-
 docs/writing-qmp-commands.txt   |   4 +-
 qapi-schema.json| 378 
 qapi/block-core.json| 418 ++--
 qapi/block.json |   8 +-
 qapi/crypto.json|  22 +-
 qapi/event.json |  10 +-
 qapi/introspect.json|   6 +-
 qapi/rocker.json|  70 +++---
 qapi/trace.json |   6 +-
 qga/qapi-schema.json|  38 ++--
 scripts/qapi.py |  23 +-
 scripts/qapi2texi.py|   3 +-
 tests/Makefile.include  |   1 -
 tests/qapi-schema/doc-optional.err  |   1 -
 tests/qapi-schema/doc-optional.exit |   1 -
 tests/qapi-schema/doc-optional.json |   7 -
 tests/qapi-schema/doc-optional.out  |   0
 18 files changed, 491 insertions(+), 521 deletions(-)
 delete mode 100644 tests/qapi-schema/doc-optional.err
 delete mode 100644 tests/qapi-schema/doc-optional.exit
 delete mode 100644 tests/qapi-schema/doc-optional.json
 delete mode 100644 tests/qapi-schema/doc-optional.out

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 349dc02..4767825 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -131,10 +131,8 @@ and optional tagged sections.
 
 FIXME: the parser accepts these things in almost any order.
 
-Optional arguments / members are tagged with the phrase '#optional',
-often with their default value; and extensions added after the
-expression was first released are also given a '(since x.y.z)'
-comment.
+Extensions added after the expression was first released carry a
+'(since x.y.z)' comment.
 
 A tagged section starts with one of the following words:
 "Note:"/"Notes:", "Since:", "Example"/"Examples", "Returns:", "TODO:".
@@ -150,10 +148,10 @@ For example:
 #
 # Statistics of a virtual block device or a block backing device.
 #
-# @device: #optional If the stats are for a virtual block device, the name
-#  corresponding to the virtual block device.
+# @device: If the stats are for a virtual block device, the name
+# corresponding to the virtual block device.
 #
-# @node-name: #optional The node name of the device. (since 2.3)
+# @node-name: The node name of the device. (since 2.3)
 #
 # ... more members ...
 #
@@ -168,8 +166,8 @@ For example:
 #
 # Query the @BlockStats for all virtual block devices.
 #
-# @query-nodes: #optional If true, the command will query all the
-#   block nodes ... explain, explain ...  (since 2.3)
+# @query-nodes: If true, the command will query all the block nodes
+# ... explain, explain ...  (since 2.3)
 #
 # Returns: A list of @BlockStats for each virtual block devices.
 #
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 44c14db..1e63754 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -252,7 +252,7 @@ here goes "hello-world"'s new entry for the 
qapi-schema.json file:
 #
 # Print a client provided string to the standard output stream.
 #
-# @message: #optional string to be printed
+# @message: string to be printed
 #
 # Returns: Nothing on success.
 #
@@ -358,7 +358,7 @@ The best way to return that data is to create a new QAPI 
type, as shown below:
 #
 # @clock-name: The alarm clock method's name.
 #
-# @next-deadline: #optional The time (in nanoseconds) the next alarm will fire.
+# @next-deadline: The time (in nanoseconds) the next alarm will fire.
 #
 # Since: 1.0
 ##
diff --git a/qapi-schema.json b/qapi-schema.json
index 52141cd..d693033 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -150,10 +150,10 @@
 #
 # @fdname: file descriptor name previously passed via 'getfd' command
 #
-# @skipauth: #optional whether to skip authentication. Only applies
+# @skipauth: whether to skip authentication. Only applies
 #to "vnc" and "spice" protocols
 #
-# @tls: #optional whether to perform TLS. Only applies to the "spice"
+# @tls: whether to perform TLS. Only applies to the "spice"
 #   protocol
 #
 # Returns: nothing on s

[Qemu-devel] [PATCH V2 3/3] virtio: validate address space cache during init

[Jason Wang]

We don't check the return value of address_space_cache_init(), this
may lead buggy driver use incorrect region caches. Instead of
triggering an assert, catch and warn this early in
virtio_init_region_cache().

Cc: Cornelia Huck 
Cc: Paolo Bonzini 
Signed-off-by: Jason Wang 
---
 hw/virtio/virtio.c | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f086452..dc5bec7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -131,6 +131,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, 
int n)
 VRingMemoryRegionCaches *new;
 hwaddr addr, size;
 int event_size;
+int64_t len;
 
 event_size = virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX) ? 
2 : 0;
 
@@ -140,21 +141,41 @@ static void virtio_init_region_cache(VirtIODevice *vdev, 
int n)
 }
 new = g_new0(VRingMemoryRegionCaches, 1);
 size = virtio_queue_get_desc_size(vdev, n);
-address_space_cache_init(&new->desc, vdev->dma_as,
- addr, size, false);
+len = address_space_cache_init(&new->desc, vdev->dma_as,
+   addr, size, false);
+if (len < size) {
+virtio_error(vdev, "Cannot map desc");
+goto err_desc;
+}
 
 size = virtio_queue_get_used_size(vdev, n) + event_size;
-address_space_cache_init(&new->used, vdev->dma_as,
- vq->vring.used, size, true);
+len = address_space_cache_init(&new->used, vdev->dma_as,
+   vq->vring.used, size, true);
+if (len < size) {
+virtio_error(vdev, "Cannot map used");
+goto err_used;
+}
 
 size = virtio_queue_get_avail_size(vdev, n) + event_size;
-address_space_cache_init(&new->avail, vdev->dma_as,
- vq->vring.avail, size, false);
+len = address_space_cache_init(&new->avail, vdev->dma_as,
+   vq->vring.avail, size, false);
+if (len < size) {
+virtio_error(vdev, "Cannot map avail");
+goto err_avail;
+}
 
 atomic_rcu_set(&vq->vring.caches, new);
 if (old) {
 call_rcu(old, virtio_free_region_cache, rcu);
 }
+return;
+
+err_avail:
+address_space_cache_destroy(&new->used);
+err_used:
+address_space_cache_destroy(&new->desc);
+err_desc:
+g_free(new);
 }
 
 /* virt queue functions */
-- 
2.7.4

[Qemu-devel] [PATCH V2 2/3] virtio: destroy region cache during reset

[Jason Wang]

We don't destroy region cache during reset which can make the maps
of previous driver leaked to a buggy or malicious driver that don't
set vring address before starting to use the device. Fix this by
destroy the region cache during reset and validate it before trying to
see them.

Cc: Cornelia Huck 
Cc: Paolo Bonzini 
Signed-off-by: Jason Wang 
---
Changes from v1:
- switch to use rcu in virtio_virtqueue_region_cache()
- use unlikely() when needed
---
 hw/virtio/virtio.c | 60 +++---
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 76cc81b..f086452 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -190,6 +190,10 @@ static inline uint16_t vring_avail_flags(VirtQueue *vq)
 {
 VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
 hwaddr pa = offsetof(VRingAvail, flags);
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map avail flags");
+return 0;
+}
 return virtio_lduw_phys_cached(vq->vdev, &caches->avail, pa);
 }
 
@@ -198,6 +202,10 @@ static inline uint16_t vring_avail_idx(VirtQueue *vq)
 {
 VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
 hwaddr pa = offsetof(VRingAvail, idx);
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map avail idx");
+return vq->shadow_avail_idx;
+}
 vq->shadow_avail_idx = virtio_lduw_phys_cached(vq->vdev, &caches->avail, 
pa);
 return vq->shadow_avail_idx;
 }
@@ -207,6 +215,10 @@ static inline uint16_t vring_avail_ring(VirtQueue *vq, int 
i)
 {
 VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
 hwaddr pa = offsetof(VRingAvail, ring[i]);
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map avail ring");
+return 0;
+}
 return virtio_lduw_phys_cached(vq->vdev, &caches->avail, pa);
 }
 
@@ -222,6 +234,10 @@ static inline void vring_used_write(VirtQueue *vq, 
VRingUsedElem *uelem,
 {
 VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
 hwaddr pa = offsetof(VRingUsed, ring[i]);
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map used ring");
+return;
+}
 virtio_tswap32s(vq->vdev, &uelem->id);
 virtio_tswap32s(vq->vdev, &uelem->len);
 address_space_write_cached(&caches->used, pa, uelem, 
sizeof(VRingUsedElem));
@@ -233,6 +249,10 @@ static uint16_t vring_used_idx(VirtQueue *vq)
 {
 VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
 hwaddr pa = offsetof(VRingUsed, idx);
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map used ring");
+return 0;
+}
 return virtio_lduw_phys_cached(vq->vdev, &caches->used, pa);
 }
 
@@ -241,6 +261,10 @@ static inline void vring_used_idx_set(VirtQueue *vq, 
uint16_t val)
 {
 VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
 hwaddr pa = offsetof(VRingUsed, idx);
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map used idx");
+return;
+}
 virtio_stw_phys_cached(vq->vdev, &caches->used, pa, val);
 address_space_cache_invalidate(&caches->used, pa, sizeof(val));
 vq->used_idx = val;
@@ -252,8 +276,13 @@ static inline void vring_used_flags_set_bit(VirtQueue *vq, 
int mask)
 VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
 VirtIODevice *vdev = vq->vdev;
 hwaddr pa = offsetof(VRingUsed, flags);
-uint16_t flags = virtio_lduw_phys_cached(vq->vdev, &caches->used, pa);
+uint16_t flags;
 
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map used flags");
+return;
+}
+flags = virtio_lduw_phys_cached(vq->vdev, &caches->used, pa);
 virtio_stw_phys_cached(vdev, &caches->used, pa, flags | mask);
 address_space_cache_invalidate(&caches->used, pa, sizeof(flags));
 }
@@ -266,6 +295,10 @@ static inline void vring_used_flags_unset_bit(VirtQueue 
*vq, int mask)
 hwaddr pa = offsetof(VRingUsed, flags);
 uint16_t flags = virtio_lduw_phys_cached(vq->vdev, &caches->used, pa);
 
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map used flags");
+return;
+}
 virtio_stw_phys_cached(vdev, &caches->used, pa, flags & ~mask);
 address_space_cache_invalidate(&caches->used, pa, sizeof(flags));
 }
@@ -280,6 +313,10 @@ static inline void vring_set_avail_event(VirtQueue *vq, 
uint16_t val)
 }
 
 caches = atomic_rcu_read(&vq->vring.caches);
+if (unlikely(!caches)) {
+virtio_error(vq->vdev, "Cannot map avail event");
+return;
+}
 pa = offsetof(VRingUsed, ring[vq->vring.num]);
 virtio_stw_phys_cached(vq->vdev, &caches->used, pa, val);
 address_space_cache_invalidate(&caches->used, pa, sizeof(val));
@@ -573,7 +610,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 
 max = vq-

[Qemu-devel] [PATCH] qemu-img: show help for invalid global options

[Stefan Hajnoczi]

The qemu-img sub-command executes regardless of invalid global options:

  $ qemu-img --foo info test.img
  qemu-img: unrecognized option '--foo'
  image: test.img
  ...

The unrecognized option warning may be missed by the user.  This can
hide incorrect command-lines in scripts and confuse users.

This patch prints the help information and terminates instead of
executing the sub-command.

Signed-off-by: Stefan Hajnoczi 
---
 qemu-img.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-img.c b/qemu-img.c
index 98b836b..ce293a4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4339,6 +4339,7 @@ int main(int argc, char **argv)
 while ((c = getopt_long(argc, argv, "+hVT:", long_options, NULL)) != -1) {
 switch (c) {
 case 'h':
+case '?':
 help();
 return 0;
 case 'V':
-- 
2.9.3

Re: [Qemu-devel] [RFC 0/4] qemu-img: add max-size subcommand

[Stefan Hajnoczi]

On Sat, Mar 04, 2017 at 12:15:00AM +0200, Nir Soffer wrote:
> On Sat, Mar 4, 2017 at 12:02 AM, John Snow  wrote:
> >
> >
> > On 03/03/2017 04:38 PM, Nir Soffer wrote:
> >> On Fri, Mar 3, 2017 at 3:51 PM, Stefan Hajnoczi  
> >> wrote:
> >>>
> >>> RFCv1:
> >>>  * Publishing patch series with just raw support, no qcow2 yet.  Please 
> >>> review
> >>>the command-line interface and let me know if you are happy with this
> >>>approach.
> >>>
> >>> Users and management tools sometimes need to know the size required for a 
> >>> new
> >>> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of 
> >>> time.
> >>> Image formats like qcow2 have non-trivial metadata that makes it hard to
> >>> estimate the exact size without knowledge of file format internals.
> >>>
> >>> This patch series introduces a new qemu-img subcommand that calculates the
> >>> required size for both image creation and conversion scenarios.
> >>>
> >>> The conversion scenario is:
> >>>
> >>>   $ qemu-img max-size -f raw -O qcow2 input.img
> >>>   107374184448
> >>
> >> Isn't this the minimal size required to convert input.img?
> >>
> >
> > It's an upper bound for the property being measured, which is current
> > allocation size, not maximum potential size after growth.
> 
> From my point of view, this is the minimal size you must allocate if you
> want to convert the image to logical volume.
> 
> >
> >>>
> >>> Here an existing image file is taken and the output includes the space 
> >>> required
> >>> for data from the input image file.
> >>>
> >>> The creation scenario is:
> >>>
> >>>   $ qemu-img max-size -O qcow2 --size 5G
> >>>   196688
> >>
> >> Again, this is the minimal size.
> >>
> >> So maybe use min-size?
> >>
> >> Or:
> >>
> >> qemu-img measure -f raw -O qcow2 input.img
> >>
> >> Works nicely with other verbs like create, convert, check.
> >>
> >
> > Measure what? This is strictly less descriptive even if "max-size" isn't
> > a verb.
> 
> measure-size?

You're right, the sub-command should be a verb.

> >> Now about the return value, do we want to return both the minimum size
> >> and the maximum size?
> >>
> >> For ovirt use case, we currently calculate the maximum size by multiplying
> >> by 1.1. We use this when doing automatic extending of ovirt thin 
> >> provisioned
> >> disk. We start with 1G lv, and extend it each time it becomes full, 
> >> stopping
> >> when we reach virtual size * 1.1. Using more accurate calculation instead
> >> can be nicer.
> >>
> >> So we can retrun:
> >>
> >> {
> >> "min-size": 196688,
> >> "max-size": 5905580032
> >> }
> >>
> >> Anyway thanks for working on this!
> >>
> >
> > It sounds like you want something different from what was intuited by
> > Maor Lipchuck. There are two things to estimate:
> >
> > (A) An estimate of the possible size of an image after conversion to a
> > different format, and
> > (B) An estimate of the possible size after full allocation.
> >
> > I got the sense that Maor was asking for (A), but perhaps I am wrong
> > about that. However, both are "maximums" in different senses.
> 
> Both are minimum when you have to allocate the space.
> 
> Maor ask about A because he is working on fixing allocation when
> converting existing files, but we also have other use cases like B.

Daniel Berrange is also interested in the size of a fully populated
image file.  I'll expand the patch series to report both sizes.

Stefan


signature.asc
Description: PGP signature