Re: [Qemu-devel] [PATCH] ppc: xics: fix compilation with CentOS 6

[David Gibson]

On Mon, May 08, 2017 at 09:25:19AM +0200, Paolo Bonzini wrote:
> 
> 
> On 08/05/2017 08:13, David Gibson wrote:
> >>
> >>Please start including Docker-based tests in your pre-pull-request 
> >> tests.
> > Um.. is there some doco on how to execute these.  I tried "make
> > docker-test" and "make docker-image" and they both just seem to hang
> > at:
> > 
> > umbus:~/src/qemu (ppc-for-2.10)$ make docker-image
> >   BUILD   travis
> > 
> > strace shows docker blocked on a futex.
> 
> Try "make docker-test-full@centos6 V=1".

So, the problem I hit above was just that it was taking time to
download all the image data - without V=1, it wasn't showing progress
so it looked like it wasn't doing anything.

With that done, I get further, but it's still a bit of a pain in the
arse.  If I run as a normal user things fail, because the sudo
authorization runs out before a test is complete.  If I run as root it
screws up permissions on my git tree so I have to re-chown before
operating as a normal user again.  And testing the full set of images
takes an age - I've been running this all day, and AFAICT it's barely
through half the available images.  If I'm testing for multiple days
it means what I'm finally sending a pullreq for is no longer against
the latest master, which I don't really like.

Any ideas to make this less painful?

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


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v2] target/ppc: Avoid printing wrong aliases in CPU help text

[Thomas Huth]

When running with KVM, we update the "family" CPU alias to point
to the right host CPU type, so that it for example possible to
use "-cpu POWER8" on a POWER8NVL host. However, the function for
printing the list of available CPU models is called earlier than
the KVM setup code, so the output of "-cpu help" is wrong in that
case. Since it would be somewhat ugly anyway to have different
help texts depending on whether "-enable-kvm" has been specified
or not, we should better always print the same text, so fix this
issue by printing "alias for preferred XXX CPU" instead.

Reviewed-by: Eduardo Habkost 
Signed-off-by: Thomas Huth 
---
 v2:
 - Rebased to master

 target/ppc/cpu.h|  1 +
 target/ppc/kvm.c| 12 
 target/ppc/translate_init.c | 27 +--
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e0ff041..c2d3b8d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1221,6 +1221,7 @@ static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState 
*env)
 
 PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr);
 PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr);
+PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc);
 
 struct PPCVirtualHypervisor {
 Object parent;
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 8574c36..07f0d1f 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2413,18 +2413,6 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
 return cap_mmu_hash_v3;
 }
 
-static PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
-{
-ObjectClass *oc = OBJECT_CLASS(pcc);
-
-while (oc && !object_class_is_abstract(oc)) {
-oc = object_class_get_parent(oc);
-}
-assert(oc);
-
-return POWERPC_CPU_CLASS(oc);
-}
-
 PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
 {
 uint32_t host_pvr = mfpvr();
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index e82e3e6..4332b7c 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10285,6 +10285,18 @@ PowerPCCPU *cpu_ppc_init(const char *cpu_model)
 return POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, cpu_model));
 }
 
+PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
+{
+ObjectClass *oc = OBJECT_CLASS(pcc);
+
+while (oc && !object_class_is_abstract(oc)) {
+oc = object_class_get_parent(oc);
+}
+assert(oc);
+
+return POWERPC_CPU_CLASS(oc);
+}
+
 /* Sort by PVR, ordering special case "host" last. */
 static gint ppc_cpu_list_compare(gconstpointer a, gconstpointer b)
 {
@@ -10316,6 +10328,7 @@ static void ppc_cpu_list_entry(gpointer data, gpointer 
user_data)
 ObjectClass *oc = data;
 CPUListState *s = user_data;
 PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+DeviceClass *family = DEVICE_CLASS(ppc_cpu_get_family_class(pcc));
 const char *typename = object_class_get_name(oc);
 char *name;
 int i;
@@ -10338,8 +10351,18 @@ static void ppc_cpu_list_entry(gpointer data, gpointer 
user_data)
 if (alias_oc != oc) {
 continue;
 }
-(*s->cpu_fprintf)(s->file, "PowerPC %-16s (alias for %s)\n",
-  alias->alias, name);
+/*
+ * If running with KVM, we might update the family alias later, so
+ * avoid printing the wrong alias here and use "preferred" instead
+ */
+if (strcmp(alias->alias, family->desc) == 0) {
+(*s->cpu_fprintf)(s->file,
+  "PowerPC %-16s (alias for preferred %s CPU)\n",
+  alias->alias, family->desc);
+} else {
+(*s->cpu_fprintf)(s->file, "PowerPC %-16s (alias for %s)\n",
+  alias->alias, name);
+}
 }
 g_free(name);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v6 2/3] utils: provide size_to_str()

[Peter Xu]

Moving the algorithm from print_type_size() into size_to_str() so that
other component can also leverage it. With that, refactor
print_type_size().

Signed-off-by: Peter Xu 
---
 include/qemu-common.h|  1 +
 qapi/string-output-visitor.c | 22 ++
 util/cutils.c| 23 +++
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index d218821..d7d0448 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -145,6 +145,7 @@ void qemu_hexdump(const char *buf, FILE *fp, const char 
*prefix, size_t size);
 int parse_debug_env(const char *name, int max, int initial);
 
 const char *qemu_ether_ntoa(const MACAddr *mac);
+char *size_to_str(double val);
 void page_size_init(void);
 
 /* returns non-zero if dump is in progress, otherwise zero is
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 94ac821..53c2175 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -211,10 +211,8 @@ static void print_type_size(Visitor *v, const char *name, 
uint64_t *obj,
 Error **errp)
 {
 StringOutputVisitor *sov = to_sov(v);
-static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
-uint64_t div, val;
-char *out;
-int i;
+uint64_t val;
+char *out, *psize;
 
 if (!sov->human) {
 out = g_strdup_printf("%"PRIu64, *obj);
@@ -223,19 +221,11 @@ static void print_type_size(Visitor *v, const char *name, 
uint64_t *obj,
 }
 
 val = *obj;
-
-/* The exponent (returned in i) minus one gives us
- * floor(log2(val * 1024 / 1000).  The correction makes us
- * switch to the higher power when the integer part is >= 1000.
- */
-frexp(val / (1000.0 / 1024.0), );
-i = (i - 1) / 10;
-assert(i < ARRAY_SIZE(suffixes));
-div = 1ULL << (i * 10);
-
-out = g_strdup_printf("%"PRIu64" (%0.3g %c%s)", val,
-  (double)val/div, suffixes[i], i ? "iB" : "");
+psize = size_to_str(val);
+out = g_strdup_printf("%"PRIu64" (%s)", val, psize);
 string_output_set(sov, out);
+
+g_free(psize);
 }
 
 static void print_type_bool(Visitor *v, const char *name, bool *obj,
diff --git a/util/cutils.c b/util/cutils.c
index 50ad179..c562d87 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -619,3 +619,26 @@ const char *qemu_ether_ntoa(const MACAddr *mac)
 
 return ret;
 }
+
+/*
+ * Please free the buffer after use.
+ */
+char *size_to_str(double val)
+{
+static const char *suffixes[] = { "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei" };
+unsigned long div;
+int i;
+
+/*
+ * The exponent (returned in i) minus one gives us
+ * floor(log2(val * 1024 / 1000).  The correction makes us
+ * switch to the higher power when the integer part is >= 1000.
+ * (see e41b509d68afb1f for more info)
+ */
+frexp(val / (1000.0 / 1024.0), );
+i = (i - 1) / 10;
+assert(i < ARRAY_SIZE(suffixes));
+div = 1ULL << (i * 10);
+
+return g_strdup_printf("%0.3g %sB", val / div, suffixes[i]);
+}
-- 
2.7.4

[Qemu-devel] [PATCH v6 0/3] ramblock: add hmp command "info ramblock"

[Peter Xu]

v6
- patch 2: instead of create a new size_to_str(), abstract the logic
  out from print_type_size(), refactor it, to make sure
  print_type_size() dumps exactly the same thing as before. (a simple
  test with info qtree is done)
- let suffixes be an array of strings [Markus]

v5
- add r-b for Dave on first patch (which I forgot in v4, so I got it
  again)
- add one more patch to introduce size_to_str() as patch 2 [Dave]
- let the last patch use the new interface

v4:
- move page_size_to_str() into util/cutil.c [Dave]

v3:
- cast the three PRIx64 addresses using (uint64_t) [Fam]
- add more comment in patch 2 to emphasize that this command is only
  suitable for HMP, not QMP [Markus]

v2:
- replace "lx" with "PRIx64" in three places

Sometimes I would like to know ramblock info for a VM. This command
would help. It provides a way to dump ramblock info. Currently the
list is by default sorted by size, though I think it's good enough.

Please review, thanks.

Peter Xu (3):
  ramblock: add RAMBLOCK_FOREACH()
  utils: provide size_to_str()
  ramblock: add new hmp command "info ramblock"

 exec.c   | 44 +---
 hmp-commands-info.hx | 14 ++
 hmp.c|  6 ++
 hmp.h|  1 +
 include/exec/ramlist.h   |  6 ++
 include/qemu-common.h|  1 +
 migration/ram.c  | 15 ---
 qapi/string-output-visitor.c | 22 ++
 util/cutils.c| 23 +++
 9 files changed, 98 insertions(+), 34 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v6 1/3] ramblock: add RAMBLOCK_FOREACH()

[Peter Xu]

So that it can simplifies the iterators.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Peter Xu 
---
 exec.c | 22 +++---
 include/exec/ramlist.h |  5 +
 migration/ram.c| 15 ---
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/exec.c b/exec.c
index eac6085..50519ae 100644
--- a/exec.c
+++ b/exec.c
@@ -978,7 +978,7 @@ static RAMBlock *qemu_get_ram_block(ram_addr_t addr)
 if (block && addr - block->offset < block->max_length) {
 return block;
 }
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 if (addr - block->offset < block->max_length) {
 goto found;
 }
@@ -1578,12 +1578,12 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
 return 0;
 }
 
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 ram_addr_t end, next = RAM_ADDR_MAX;
 
 end = block->offset + block->max_length;
 
-QLIST_FOREACH_RCU(next_block, _list.blocks, next) {
+RAMBLOCK_FOREACH(next_block) {
 if (next_block->offset >= end) {
 next = MIN(next, next_block->offset);
 }
@@ -1609,7 +1609,7 @@ unsigned long last_ram_page(void)
 ram_addr_t last = 0;
 
 rcu_read_lock();
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 last = MAX(last, block->offset + block->max_length);
 }
 rcu_read_unlock();
@@ -1659,7 +1659,7 @@ void qemu_ram_set_idstr(RAMBlock *new_block, const char 
*name, DeviceState *dev)
 pstrcat(new_block->idstr, sizeof(new_block->idstr), name);
 
 rcu_read_lock();
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 if (block != new_block &&
 !strcmp(block->idstr, new_block->idstr)) {
 fprintf(stderr, "RAMBlock \"%s\" already registered, abort!\n",
@@ -1693,7 +1693,7 @@ size_t qemu_ram_pagesize_largest(void)
 RAMBlock *block;
 size_t largest = 0;
 
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 largest = MAX(largest, qemu_ram_pagesize(block));
 }
 
@@ -1839,7 +1839,7 @@ static void ram_block_add(RAMBlock *new_block, Error 
**errp)
  * QLIST (which has an RCU-friendly variant) does not have insertion at
  * tail, so save the last element in last_block.
  */
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 last_block = block;
 if (block->max_length < new_block->max_length) {
 break;
@@ -2021,7 +2021,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 int flags;
 void *area, *vaddr;
 
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 offset = addr - block->offset;
 if (offset < block->max_length) {
 vaddr = ramblock_ptr(block, offset);
@@ -2167,7 +2167,7 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool 
round_offset,
 goto found;
 }
 
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 /* This case append when the block is not mapped. */
 if (block->host == NULL) {
 continue;
@@ -2200,7 +2200,7 @@ RAMBlock *qemu_ram_block_by_name(const char *name)
 {
 RAMBlock *block;
 
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 if (!strcmp(name, block->idstr)) {
 return block;
 }
@@ -3424,7 +3424,7 @@ int qemu_ram_foreach_block(RAMBlockIterFunc func, void 
*opaque)
 int ret = 0;
 
 rcu_read_lock();
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 ret = func(block->idstr, block->host, block->offset,
block->used_length, opaque);
 if (ret) {
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index c59880d..f1c6b45 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -4,6 +4,7 @@
 #include "qemu/queue.h"
 #include "qemu/thread.h"
 #include "qemu/rcu.h"
+#include "qemu/rcu_queue.h"
 
 typedef struct RAMBlockNotifier RAMBlockNotifier;
 
@@ -54,6 +55,10 @@ typedef struct RAMList {
 } RAMList;
 extern RAMList ram_list;
 
+/* Should be holding either ram_list.mutex, or the RCU lock. */
+#define  RAMBLOCK_FOREACH(block)  \
+QLIST_FOREACH_RCU(block, _list.blocks, next)
+
 void qemu_mutex_lock_ramlist(void);
 void qemu_mutex_unlock_ramlist(void);
 
diff --git a/migration/ram.c b/migration/ram.c
index f48664e..7ba5d7e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -672,7 +672,7 @@ uint64_t ram_pagesize_summary(void)
 RAMBlock *block;
 uint64_t summary = 0;
 
-QLIST_FOREACH_RCU(block, _list.blocks, next) {
+RAMBLOCK_FOREACH(block) {
 summary |= block->page_size;
 }
 
@@ -700,7 +700,7 @@ static void migration_bitmap_sync(RAMState *rs)
 
   

[Qemu-devel] [PATCH v6 3/3] ramblock: add new hmp command "info ramblock"

[Peter Xu]

To dump information about ramblocks. It looks like:

(qemu) info ramblock
  Block NamePSize  Offset   Used
  Total
/objects/mem2 MiB  0x 0x8000 
0x8000
vga.vram4 KiB  0x8006 0x0100 
0x0100
/rom@etc/acpi/tables4 KiB  0x810b 0x0002 
0x0020
 pc.bios4 KiB  0x8000 0x0004 
0x0004
  :00:03.0/e1000.rom4 KiB  0x8107 0x0004 
0x0004
  pc.rom4 KiB  0x8004 0x0002 
0x0002
:00:02.0/vga.rom4 KiB  0x8106 0x0001 
0x0001
   /rom@etc/table-loader4 KiB  0x812b 0x1000 
0x1000
  /rom@etc/acpi/rsdp4 KiB  0x812b1000 0x1000 
0x1000

Ramblock is something hidden internally in QEMU implementation, and this
command should only be used by mostly QEMU developers on RAM stuff. It
is not a command suitable for QMP interface. So only HMP interface is
provided for it.

Signed-off-by: Peter Xu 
---
 exec.c | 22 ++
 hmp-commands-info.hx   | 14 ++
 hmp.c  |  6 ++
 hmp.h  |  1 +
 include/exec/ramlist.h |  1 +
 5 files changed, 44 insertions(+)

diff --git a/exec.c b/exec.c
index 50519ae..821bef3 100644
--- a/exec.c
+++ b/exec.c
@@ -71,6 +71,8 @@
 #include "qemu/mmap-alloc.h"
 #endif
 
+#include "monitor/monitor.h"
+
 //#define DEBUG_SUBPAGE
 
 #if !defined(CONFIG_USER_ONLY)
@@ -1333,6 +1335,26 @@ void qemu_mutex_unlock_ramlist(void)
 qemu_mutex_unlock(_list.mutex);
 }
 
+void ram_block_dump(Monitor *mon)
+{
+RAMBlock *block;
+char *psize;
+
+rcu_read_lock();
+monitor_printf(mon, "%24s %8s  %18s %18s %18s\n",
+   "Block Name", "PSize", "Offset", "Used", "Total");
+RAMBLOCK_FOREACH(block) {
+psize = size_to_str(block->page_size);
+monitor_printf(mon, "%24s %8s  0x%016" PRIx64 " 0x%016" PRIx64
+   " 0x%016" PRIx64 "\n", block->idstr, psize,
+   (uint64_t)block->offset,
+   (uint64_t)block->used_length,
+   (uint64_t)block->max_length);
+g_free(psize);
+}
+rcu_read_unlock();
+}
+
 #ifdef __linux__
 /*
  * FIXME TOCTTOU: this iterates over memory backends' mem-path, which
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index a53f105..ae16901 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -788,6 +788,20 @@ Display the latest dump status.
 ETEXI
 
 {
+.name   = "ramblock",
+.args_type  = "",
+.params = "",
+.help   = "Display system ramblock information",
+.cmd= hmp_info_ramblock,
+},
+
+STEXI
+@item info ramblock
+@findex ramblock
+Dump all the ramblocks of the system.
+ETEXI
+
+{
 .name   = "hotpluggable-cpus",
 .args_type  = "",
 .params = "",
diff --git a/hmp.c b/hmp.c
index ab407d6..8369388 100644
--- a/hmp.c
+++ b/hmp.c
@@ -37,6 +37,7 @@
 #include "qemu-io.h"
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
+#include "exec/ramlist.h"
 #include "hw/intc/intc.h"
 
 #ifdef CONFIG_SPICE
@@ -2563,6 +2564,11 @@ void hmp_info_dump(Monitor *mon, const QDict *qdict)
 qapi_free_DumpQueryResult(result);
 }
 
+void hmp_info_ramblock(Monitor *mon, const QDict *qdict)
+{
+ram_block_dump(mon);
+}
+
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 799fd37..7353b67 100644
--- a/hmp.h
+++ b/hmp.h
@@ -136,6 +136,7 @@ void hmp_rocker_ports(Monitor *mon, const QDict *qdict);
 void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict);
 void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict);
 void hmp_info_dump(Monitor *mon, const QDict *qdict);
+void hmp_info_ramblock(Monitor *mon, const QDict *qdict);
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index f1c6b45..2e2ac6c 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -73,5 +73,6 @@ void ram_block_notifier_remove(RAMBlockNotifier *n);
 void ram_block_notify_add(void *host, size_t size);
 void ram_block_notify_remove(void *host, size_t size);
 
+void ram_block_dump(Monitor *mon);
 
 #endif /* RAMLIST_H */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 5/6] target/s390x: Use atomic operations for COMPARE SWAP

[Thomas Huth]

On 09.05.2017 20:07, Richard Henderson wrote:
> Reviewed-by: Aurelien Jarno 
> Signed-off-by: Richard Henderson 
> ---
>  target/s390x/helper.h  |  1 +
>  target/s390x/insn-data.def | 10 +++---
>  target/s390x/mem_helper.c  | 39 ++
>  target/s390x/translate.c   | 83 
> --
>  4 files changed, 59 insertions(+), 74 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 01adb50..0b70770 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -25,6 +25,7 @@ DEF_HELPER_3(cxgb, i64, env, s64, i32)
>  DEF_HELPER_3(celgb, i64, env, i64, i32)
>  DEF_HELPER_3(cdlgb, i64, env, i64, i32)
>  DEF_HELPER_3(cxlgb, i64, env, i64, i32)
> +DEF_HELPER_4(cdsg, void, env, i64, i32, i32)
>  DEF_HELPER_FLAGS_3(aeb, TCG_CALL_NO_WG, i64, env, i64, i64)
>  DEF_HELPER_FLAGS_3(adb, TCG_CALL_NO_WG, i64, env, i64, i64)
>  DEF_HELPER_FLAGS_5(axb, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i64)
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 0909060..5e5fcc5 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -239,12 +239,12 @@
>  D(0xec7d, CLGIJ,   RIE_c, GIE, r1_o, i2_8u, 0, 0, cj, 0, 1)
>  
>  /* COMPARE AND SWAP */
> -D(0xba00, CS,  RS_a,  Z,   r3_32u, r1_32u, new, r1_32, cs, 0, 0)
> -D(0xeb14, CSY, RSY_a, LD,  r3_32u, r1_32u, new, r1_32, cs, 0, 0)
> -D(0xeb30, CSG, RSY_a, Z,   r3_o, r1_o, new, r1, cs, 0, 1)
> +D(0xba00, CS,  RS_a,  Z,   r3_32u, r1_32u, new, r1_32, cs, 0, 
> MO_TEUL)
> +D(0xeb14, CSY, RSY_a, LD,  r3_32u, r1_32u, new, r1_32, cs, 0, 
> MO_TEUL)
> +D(0xeb30, CSG, RSY_a, Z,   r3_o, r1_o, new, r1, cs, 0, MO_TEQ)
>  /* COMPARE DOUBLE AND SWAP */
> -D(0xbb00, CDS, RS_a,  Z,   r3_D32, r1_D32, new, r1_D32, cs, 0, 1)
> -D(0xeb31, CDSY,RSY_a, LD,  r3_D32, r1_D32, new, r1_D32, cs, 0, 1)
> +D(0xbb00, CDS, RS_a,  Z,   r3_D32, r1_D32, new, r1_D32, cs, 0, 
> MO_TEQ)
> +D(0xeb31, CDSY,RSY_a, LD,  r3_D32, r1_D32, new, r1_D32, cs, 0, 
> MO_TEQ)
>  C(0xeb3e, CDSG,RSY_a, Z,   0, 0, 0, 0, cdsg, 0)
>  
>  /* COMPARE AND TRAP */
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 675aba2..c74ded3 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -844,6 +844,45 @@ uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, 
> uint64_t array,
>  return cc;
>  }
>  
> +void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
> +  uint32_t r1, uint32_t r3)
> +{
> +uintptr_t ra = GETPC();
> +Int128 cmpv = int128_make128(env->regs[r1 + 1], env->regs[r1]);
> +Int128 newv = int128_make128(env->regs[r3 + 1], env->regs[r3]);
> +int mem_idx = cpu_mmu_index(env, false);
> +Int128 oldv;
> +bool fail;
> +
> +if (parallel_cpus) {
> +#ifndef CONFIG_ATOMIC128
> +cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> +#else
> +TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
> +oldv = helper_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
> +fail = !int128_eq(oldv, cmpv);
> +#endif
> +} else {

Have you seen the compilation issue that patchew reported? I think
you've got to move the definition of mem_idx into the "#else" part, too.

 Thomas

Re: [Qemu-devel] [PATCH v5 2/3] utils: provide size_to_str()

[Peter Xu]

On Tue, May 09, 2017 at 04:50:26PM +0200, Markus Armbruster wrote:
> Peter Xu  writes:
> 
> > I stole the algorithm from print_type_size(). I didn't generalize it
> > since that's using [KM...]iB while here we need [KM...]B to finally
> > be able to stands for page sizes (and even more general).
> 
> Can you explain why we need units without the 'i' here?

Oops. I misunderstood XiB... My fault. Page sizes needs exactly "i".

> 
> >
> > Signed-off-by: Peter Xu 
> > ---
> >  include/qemu-common.h |  1 +
> >  util/cutils.c | 26 ++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/include/qemu-common.h b/include/qemu-common.h
> > index d218821..d7d0448 100644
> > --- a/include/qemu-common.h
> > +++ b/include/qemu-common.h
> > @@ -145,6 +145,7 @@ void qemu_hexdump(const char *buf, FILE *fp, const char 
> > *prefix, size_t size);
> >  int parse_debug_env(const char *name, int max, int initial);
> >  
> >  const char *qemu_ether_ntoa(const MACAddr *mac);
> > +char *size_to_str(double val);
> >  void page_size_init(void);
> >  
> >  /* returns non-zero if dump is in progress, otherwise zero is
> > diff --git a/util/cutils.c b/util/cutils.c
> > index 50ad179..5aaf370 100644
> > --- a/util/cutils.c
> > +++ b/util/cutils.c
> > @@ -619,3 +619,29 @@ const char *qemu_ether_ntoa(const MACAddr *mac)
> >  
> >  return ret;
> >  }
> > +
> > +/*
> > + * Sample output:
> > + *
> > + * 1  -> "1 B"
> > + * 528-> "0.516 KB"
> > + * 4096   -> "4 KB"
> > + * 2402958-> "2.29 MB"
> > + * 1073741824 -> "1 GB"
> > + *
> > + * Please free the buffer after use.
> > + */
> > +char *size_to_str(double val)
> > +{
> > +static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
> > +unsigned long div;
> > +int i;
> > +
> > +frexp(val, );
> 
> The ignored return value is in [0.5,1), and multiplying it by 2^i yields
> val.  i is close to the binary logarithm.
> 
> > +i /= 10;
> 
> Now it's close to base-1024 logarithm.
> 
> Figuring this out requires too much thought for comfort.  Recommend
> steal the comment from print_type_size(), too.

Let me do it even simpler - I'll just move the whole logic in
print_type_size() into size_to_str(), then I'll refactor
print_type_size().

> 
> > +assert(i < sizeof(suffixes));
> > +div = 1ULL << (i * 10);
> > +
> > +return g_strdup_printf("%0.3g %c%s", val / div,
> > +   suffixes[i], i ? "B" : "");
> 
> The conditional is a bit confusing.  To avoid it, we could make
> suffixes[] an array of strings, with suffixes[0] = "".

I can fix this. Thanks reviewing!

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 23/24] numa: add '-numa cpu, ...' option for property based node mapping

[David Gibson]

On Tue, May 09, 2017 at 05:58:29PM +0200, Igor Mammedov wrote:
> On Mon, 8 May 2017 15:40:04 +1000
> David Gibson  wrote:
> 
> > On Wed, May 03, 2017 at 02:57:17PM +0200, Igor Mammedov wrote:
> > > legacy cpu to node mapping is using cpu index values to map
> > > VCPU to node with help of '-numa node,nodeid=node,cpus=x[-y]'
> > > option. However cpu index is internal concept and QEMU users
> > > have to guess /reimplement qemu's logic/ to map it to
> > > a concrete cpu socket/core/thread to make sane CPUs
> > > placement across numa nodes.
> > > 
> > > This patch allows to map cpu objects to numa nodes using
> > > the same properties as used for cpus with -device/device_add
> > > (socket-id/core-id/thread-id/node-id).
> > > 
> > > At present valid properties/values to address CPUs could be
> > > fetched using hotpluggable-cpus monitor/qmp command, it will
> > > require user to start qemu twice when creating domain to fetch
> > > possible CPUs for a machine type/-smp layout first and
> > > then the second time with numa explicit mapping for actual
> > > usage. The first step results could be saved and reused to
> > > set/change mapping later as far as machine type/-smp stays
> > > the same.
> > > 
> > > Proposed impl. supports exact and wildcard matching to
> > > simplify CLI and allow to set mapping for a specific cpu
> > > or group of cpu objects specified by matched properties.
> > > 
> > > For example:
> > > 
> > ># exact mapping x86
> > >-numa cpu,node-id=x,socket-id=y,core-id=z,thread-id=n
> > > 
> > ># exact mapping SPAPR
> > >-numa cpu,node-id=x,core-id=y
> > > 
> > ># wildcard mapping, all cpu objects that match socket-id=y
> > ># are mapped to node-id=x
> > >-numa cpu,node-id=x,socket-id=y
> > > 
> > > Signed-off-by: Igor Mammedov 
> > > ---
> > > v2:
> > >   - use new NumaCpuOptions instead of CpuInstanceProperties in
> > > NumaOptions, so that in future we could decouple both
> > > if needed. (Eduardo Habkost )
> > >   - clarify effect of NumaCpuOptions.node-id in qapi-schema.json
> > > ---
> > >  numa.c   | 25 +
> > >  qapi-schema.json | 21 +++--
> > >  qemu-options.hx  | 23 ++-
> > >  3 files changed, 66 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/numa.c b/numa.c
> > > index 40e9f44..61521f5 100644
> > > --- a/numa.c
> > > +++ b/numa.c
> > > @@ -227,6 +227,7 @@ static int parse_numa(void *opaque, QemuOpts *opts, 
> > > Error **errp)
> > >  NumaOptions *object = NULL;
> > >  MachineState *ms = opaque;
> > >  Error *err = NULL;
> > > +CpuInstanceProperties cpu;
> > >  
> > >  {
> > >  Visitor *v = opts_visitor_new(opts);
> > > @@ -246,6 +247,30 @@ static int parse_numa(void *opaque, QemuOpts *opts, 
> > > Error **errp)
> > >  }
> > >  nb_numa_nodes++;
> > >  break;
> > > +case NUMA_OPTIONS_TYPE_CPU:
> > > +if (!object->u.cpu.has_node_id) {
> > > +error_setg(, "Missing mandatory node-id property");
> > > +goto end;
> > > +}
> > > +if (!numa_info[object->u.cpu.node_id].present) {
> > > +error_setg(, "Invalid node-id=%" PRId64 ", NUMA node 
> > > must be "
> > > +"defined with -numa node,nodeid=ID before it's used with 
> > > "
> > > +"-numa cpu,node-id=ID", object->u.cpu.node_id);
> > > +goto end;
> > > +}
> > > +
> > > +memset(, 0, sizeof(cpu));
> > > +cpu.has_node_id = object->u.cpu.has_node_id;
> > > +cpu.node_id = object->u.cpu.node_id;
> > > +cpu.has_socket_id = object->u.cpu.has_socket_id;
> > > +cpu.socket_id = object->u.cpu.socket_id;
> > > +cpu.has_core_id = object->u.cpu.has_core_id;
> > > +cpu.core_id = object->u.cpu.core_id;
> > > +cpu.has_thread_id = object->u.cpu.has_thread_id;
> > > +cpu.thread_id = object->u.cpu.thread_id;
> > > +
> > > +machine_set_cpu_numa_node(ms, , );  
> > 
> > It's possible I've confused myself by not looking at this whole series
> > at once.
> > 
> > But, would it be possible to make a single machine hook which maps a
> > constructed cpu property set to a "canonical" cpu property set from
> > the table of CPU slots (or errors, of course).  That would let you do
> > what you need here, and I suspect in other places, without multiple
> > hooks.
> Not sure that get question.
> 
> if it's about machine_set_cpu_numa_node() than it's a single generic
> setter of MachineState::possible_cpus(node-id) from non machine code
> and the only place it's supposed to be used is from numa configuration
> code. I don't see any need to reuse it for something/somewhere else
> so far.
> 
> More over it's not 1:1 mapping wrt input:affected slots,
> it's 1:[1-n] where input could affect multiple slots.

Ah, good point.

> I think I've tried to get 

Re: [Qemu-devel] [PATCH qemu v6] memory/iommu: QOM'fy IOMMU MemoryRegion

[David Gibson]

On Tue, May 09, 2017 at 09:35:23PM +1000, Alexey Kardashevskiy wrote:
> On 08/05/17 15:53, David Gibson wrote:
> > On Fri, May 05, 2017 at 08:19:30PM +1000, Alexey Kardashevskiy wrote:
> >> This defines new QOM object - IOMMUMemoryRegion - with MemoryRegion
> >> as a parent.
> >>
> >> This moves IOMMU-related fields from MR to IOMMU MR. However to avoid
> >> dymanic QOM casting in fast path (address_space_translate, etc),
> >> this adds an @is_iommu boolean flag to MR and provides new helper to
> >> do simple cast to IOMMU MR - memory_region_get_iommu. The flag
> >> is set in the instance init callback. This defines
> >> memory_region_is_iommu as memory_region_get_iommu()!=NULL.
> >>
> >> This switches MemoryRegion to IOMMUMemoryRegion in most places except
> >> the ones where MemoryRegion may be an alias.
> >>
> >> This defines memory_region_init_iommu_type() to allow creating
> >> IOMMUMemoryRegion subclasses. In order to support custom QOM type,
> >> this splits memory_region_init() to object_initialize() +
> >> memory_region_do_init.
> >>
> >> Signed-off-by: Alexey Kardashevskiy 
> > 
> > Reviewed-by: David Gibson 
> > 
> > With the caveat that this will conflict with Peter Xu's outstanding
> > patches which adjust the semantics of the IOMMU translate function.
> 
> Sigh... So I'll wait till that reaches upstream and repost.

No, that's fine - your patches are much simpler and closer to ready
than Peter's, so I think you go first.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v8 3/4] qemu-img: introduce --target-image-opts for 'convert' command

[Fam Zheng]

On Tue, 05/09 10:48, Daniel P. Berrange wrote:
> The '--image-opts' flag indicates whether the source filename
> includes options. The target filename has to remain in the
> plain filename format though, since it needs to be passed to
> bdrv_create().  When using --skip-create though, it would be
> possible to use image-opts syntax. This adds --target-image-opts
> to indicate that the target filename includes options. Currently
> this mandates use of the --skip-create flag too.
> 
> Signed-off-by: Daniel P. Berrange 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH] vhost-user: fix watcher need be removed when vhost-user hotplug

[wangyunjian]

On Tue, May 9, 2017 at 5:04 PM Yunjian Wang  wrote:

> > From: w00273186 
> >
> > "nc" is freed after hotplug vhost-user, but the watcher don't be removed.
> >
> 
> 
> > The QEMU crash when the watcher access the "nc" on socket disconnect.
> >
> >
> Do you have a reproducer? thanks
>

reproduce steps:

1. virsh attach-device vm0 vhost-user.xml
2. virsh detach-device vm0 vhost-user.xml
3. virsh attach-device vm0 vhost-user.xml
4. service openvswitch restart
5. repeat step 2~4

The vhost-user xml:


  
  
  
  
　

Thanks

> 
> > Call Trace:
> > #0  object_get_class (obj=obj@entry=0x2) at qom/object.c:751
> > #1  0x7fc031c79f41 in qemu_chr_fe_disconnect (be=)
> > at chardev/char.c:1048
> > #2  0x7fc031bd62e0 in net_vhost_user_watch (chan=,
> > cond=, opaque=) at net/vhost-user.c:191
> > #3  0x7fc02c23e99a in g_main_context_dispatch () from
> > /lib64/libglib-2.0.so.0
> > #4  0x7fc031ccfc0c in glib_pollfds_poll () at util/main-loop.c:213
> > #5  os_host_main_loop_wait (timeout=) at
> > util/main-loop.c:261
> > #6  main_loop_wait (nonblocking=nonblocking@entry=0) at
> > util/main-loop.c:517
> > #7  0x7fc03193bc87 in main_loop () at vl.c:1899
> > #8  main (argc=, argv=, envp= > out>) at vl.c:4719
> >
> > Signed-off-by: Yunjian Wang 
> > ---
> >  net/vhost-user.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 00a0c1c..5cc2178 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -155,6 +155,10 @@ static void vhost_user_cleanup(NetClientState *nc)
> >
> >  qemu_chr_fe_deinit(>chr);
> >  object_unparent(OBJECT(chr));
> > +if (s->watch) {
> > +g_source_remove(s->watch);
> > +s->watch = 0;
> > +}
> >  }
> >
> >  qemu_purge_queued_packets(nc);
> > --
> > 1.8.3.1
> >
> >
> >
> > --
> Marc-André Lureau

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: Avoid printing wrong aliases in CPU help text

[David Gibson]

On Tue, May 09, 2017 at 06:49:29AM +0200, Thomas Huth wrote:
> On 08.03.2017 20:05, Thomas Huth wrote:
> > When running with KVM, we update the "family" CPU alias to point
> > to the right host CPU type, so that it is for example possible to
> > use "-cpu POWER8" on a POWER8NVL host. However, the function for
> > printing the list of available CPU models is called earlier than
> > the KVM setup code, so the output of "-cpu help" is wrong in that
> > case. Since it would be somewhat ugly anyway to have different
> > help texts depending on whether "-enable-kvm" has been specified
> > or not, we should better always print the same text, so fix this
> > issue by printing "alias for preferred XXX CPU" instead.
> > 
> > Signed-off-by: Thomas Huth 
> > ---
> >  target/ppc/cpu.h|  1 +
> >  target/ppc/kvm.c| 12 
> >  target/ppc/translate_init.c | 27 +--
> >  3 files changed, 26 insertions(+), 14 deletions(-)
> > 
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index 7c4a1f5..21752ff 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1216,6 +1216,7 @@ static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState 
> > *env)
> >  
> >  PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr);
> >  PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr);
> > +PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc);
> >  
> >  struct PPCVirtualHypervisor {
> >  Object parent;
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 9f1f132..585e6d3 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -2304,18 +2304,6 @@ bool kvmppc_has_cap_htm(void)
> >  return cap_htm;
> >  }
> >  
> > -static PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
> > -{
> > -ObjectClass *oc = OBJECT_CLASS(pcc);
> > -
> > -while (oc && !object_class_is_abstract(oc)) {
> > -oc = object_class_get_parent(oc);
> > -}
> > -assert(oc);
> > -
> > -return POWERPC_CPU_CLASS(oc);
> > -}
> > -
> >  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
> >  {
> >  uint32_t host_pvr = mfpvr();
> > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > index c1a9014..9e9c37f 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -10249,6 +10249,18 @@ PowerPCCPU *cpu_ppc_init(const char *cpu_model)
> >  return POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, cpu_model));
> >  }
> >  
> > +PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
> > +{
> > +ObjectClass *oc = OBJECT_CLASS(pcc);
> > +
> > +while (oc && !object_class_is_abstract(oc)) {
> > +oc = object_class_get_parent(oc);
> > +}
> > +assert(oc);
> > +
> > +return POWERPC_CPU_CLASS(oc);
> > +}
> > +
> >  /* Sort by PVR, ordering special case "host" last. */
> >  static gint ppc_cpu_list_compare(gconstpointer a, gconstpointer b)
> >  {
> > @@ -10280,6 +10292,7 @@ static void ppc_cpu_list_entry(gpointer data, 
> > gpointer user_data)
> >  ObjectClass *oc = data;
> >  CPUListState *s = user_data;
> >  PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> > +DeviceClass *family = DEVICE_CLASS(ppc_cpu_get_family_class(pcc));
> >  const char *typename = object_class_get_name(oc);
> >  char *name;
> >  int i;
> > @@ -10302,8 +10315,18 @@ static void ppc_cpu_list_entry(gpointer data, 
> > gpointer user_data)
> >  if (alias_oc != oc) {
> >  continue;
> >  }
> > -(*s->cpu_fprintf)(s->file, "PowerPC %-16s (alias for %s)\n",
> > -  alias->alias, name);
> > +/*
> > + * If running with KVM, we might update the family alias later, so
> > + * avoid printing the wrong alias here and use "preferred" instead
> > + */
> > +if (strcmp(alias->alias, family->desc) == 0) {
> > +(*s->cpu_fprintf)(s->file,
> > +  "PowerPC %-16s (alias for preferred %s 
> > CPU)\n",
> > +  alias->alias, family->desc);
> > +} else {
> > +(*s->cpu_fprintf)(s->file, "PowerPC %-16s (alias for %s)\n",
> > +  alias->alias, name);
> > +}
> >  }
> >  g_free(name);
> >  }
> > 
> 
> Ping!
> 
> ... I think this got somehow lost in the 2.9 rush ...

Yes, I think so.  Can you make sure it's rebased and repost, please.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] target/ppc: Allow workarounds for POWER9 DD1

[David Gibson]

On Tue, May 09, 2017 at 06:41:23AM +0200, Thomas Huth wrote:
> On 09.05.2017 05:45, David Gibson wrote:
> > POWER9 DD1 silicon has some bugs which mean it a) isn't really compliant
> > with the ISA v3.00 and b) require a number of special workarounds in the
> > kernel.
> > 
> > At the moment, qemu isn't aware of DD1.  For TCG we don't really want it to
> > be (why bother emulating buggy silicon).  But with KVM, the guest does need
> > to be aware of DD1 so it can apply the necessary workarounds.
> > 
> > Meanwhile, the feature negotiation between qemu and the guest strongly
> > favours architected compatibility modes to "raw" CPU modes.  In combination
> > with the above, this means the guest sees architected POWER9 mode, and
> > doesn't apply the DD1 workarounds.  Well, unless it has yet another
> > workaround to partially ignore what qemu tells it.
> > 
> > This patch addresses this by disabling support for compatibility modes when
> > using KVM on a POWER9 DD1 host.
> 
> I first though: Hey, it should be fixed in the guest kernel instead, but
> thinking about this twice, I think you're right. If the CPU is not fully
> compatible to the ISA, we really should not announce it as "architected
> / compatible POWER9" in QEMU.
> So basically ACK to your patch, I've just got a cosmetic request below...
> 
> > Signed-off-by: David Gibson 
> > ---
> >  target/ppc/kvm.c | 11 +++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index 8574c36..591b5b5 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -2380,6 +2380,17 @@ static void kvmppc_host_cpu_class_init(ObjectClass 
> > *oc, void *data)
> >  
> >  #if defined(TARGET_PPC64)
> >  pcc->radix_page_info = kvm_get_radix_page_info();
> > +
> > +if ((pcc->pvr & 0xff00) == 0x004e0100) {
> 
> Could you please add a proper #define for that magic DD1.0 value to
> cpu-models.h, please?

Good point, done.

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


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] trace: add sanity check

[Anthony Xu]

If trace backend is set to TRACE_NOP, trace_get_vcpu_event_count
returns 0, cause bitmap_new call abort.

Signed-off-by: Anthony Xu 
---
 qom/cpu.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index f02e9c0..f9111a0 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -382,6 +382,7 @@ static void cpu_common_unrealizefn(DeviceState *dev, Error 
**errp)
 
 static void cpu_common_initfn(Object *obj)
 {
+uint32_t count;
 CPUState *cpu = CPU(obj);
 CPUClass *cc = CPU_GET_CLASS(obj);
 
@@ -396,7 +397,10 @@ static void cpu_common_initfn(Object *obj)
 QTAILQ_INIT(>breakpoints);
 QTAILQ_INIT(>watchpoints);
 
-cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
+count = trace_get_vcpu_event_count();
+if (count) {
+cpu->trace_dstate = bitmap_new(count);
+}
 
 cpu_exec_initfn(cpu);
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 14/14] target/sh4: trap unaligned accesses

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

+void superh_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+MMUAccessType access_type,
+int mmu_idx, uintptr_t retaddr)
+{
+if (retaddr) {
+cpu_restore_state(cs, retaddr);
+}

...

+cpu_loop_exit(cs);
+}
+


cpu_loop_exit_restore?

Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 12/14] target/sh4: implement tas.b using atomic helper

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

We only emulate UP SH4, however as the tas.b instruction is used in the GNU
libc, this improve linux-user emulation.

Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 19 +++
  1 file changed, 7 insertions(+), 12 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 13/14] target/sh4: movua.l is an SH4-A only instruction

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

At the same time change the comment describing the instruction the same
way than other instruction, so that the code is easier to read and search.

Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 26 +++---
  1 file changed, 15 insertions(+), 11 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 11/14] target/sh4: generate fences for SH4

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

synco is a SH4-A only instruction.

Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 10/14] target/sh4: optimize gen_write_sr using extract op

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

This doesn't change the generated code on x86, but optimizes it on most
RISC architectures and makes the code simpler to read.

Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 9 +++--
  1 file changed, 3 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 09/14] target/sh4: optimize gen_store_fpr64

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

+tcg_gen_extrl_i64_i32(cpu_fregs[reg + 1], t);
+tcg_gen_extrh_i64_i32(cpu_fregs[reg], t);


This is

  tcg_gen_extr_i64_i32(cpu_fregs[reg + 1], cpu_fregs[reg], t);

Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 08/14] target/sh4: fold ctx->bstate = BS_BRANCH into gen_conditional_jump

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 9 +++--
  1 file changed, 3 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 07/14] target/sh4: only save flags state at the end of the TB

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

There is no need to save flags when entering and exiting the delay slot.
They can be saved only when reaching the end of the TB. If the TB is
interrupted before by an exception, they will be restored using
restore_state_to_opc.

Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 69 --
  1 file changed, 33 insertions(+), 36 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 06/14] target/sh4: fix BS_EXCP exit

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

In case of exception, there is no need to call tcg_gen_exit_tb as the
exception helper won't return.

Also fix a few cases where BS_BRANCH is called instead of BS_EXCP.

Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 16 +++-
  1 file changed, 7 insertions(+), 9 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 05/14] target/sh4: fix BS_STOP exit

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

When stopping the translation because the state has changed, goto_tb
should not be used as it might link TB with different flags.

Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 04/14] target/sh4: move DELAY_SLOT_TRUE flag into a separate global

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

Instead of using one bit of the env flags to store the condition of the
next delay slot, use a separate global. It simplifies reading and
writing the flags variable and also removes some confusion between
ctx->envflags and env->flags.

Note that the global is first transfered to a temp in order to be
able to discard the global before the brcond.

Signed-off-by: Aurelien Jarno
---
  target/sh4/cpu.h   | 10 ++
  target/sh4/helper.c|  2 +-
  target/sh4/translate.c | 22 +-
  3 files changed, 16 insertions(+), 18 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 02/14] target/sh4: get rid of DELAY_SLOT_CLEARME

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

Now that ctx->flags has been split, it becomes clear that
DELAY_SLOT_CLEARME has not impact on the code generation: in both case
ctx->envflags is cleared, either by clearing all the flags, or by
setting it to 0. This is left-over from pre-TCG era.

Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Aurelien Jarno
---
  target/sh4/cpu.h   |  3 +--
  target/sh4/helper.c|  2 --
  target/sh4/translate.c | 17 +
  3 files changed, 6 insertions(+), 16 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 03/14] target/sh4: do not include DELAY_SLOT_TRUE in the TB state

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

DELAY_SLOT_TRUE is used as a dynamic condition for the branch after the
delay slot instruction. It is not used in code generation, so there is
no need to including in the TB state.

Signed-off-by: Aurelien Jarno
---
  target/sh4/cpu.h | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 01/14] target/sh4: split ctx->flags into ctx->tbflags and ctx->envflags

[Richard Henderson]

On 05/06/2017 04:14 AM, Aurelien Jarno wrote:

There is a confusion (and not only in the SH4 target) between tb->flags,
env->flags and ctx->flags. To avoid it, split ctx->flags into
ctx->tbflags and ctx->envflags. ctx->tbflags stays unchanged during the
whole TB translation, while ctx->envflags evolves and is kept in sync
with env->flags using TCG instructions. ctx->envflags now only contains
the part that of env->flags that is contained in the TB state, i.e. the
DELAY_SLOT* flags.

Signed-off-by: Aurelien Jarno
---
  target/sh4/translate.c | 161 +
  1 file changed, 82 insertions(+), 79 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 3/3] xen: call qemu_set_cloexec instead of fcntl

[Greg Kurz]

On Tue,  9 May 2017 12:04:53 -0700
Stefano Stabellini  wrote:

> Use the common utility function, which contains checks on return values,

... and first calls F_GETFD as recommended by POSIX.1-2001.

http://pubs.opengroup.org/onlinepubs/009695399/functions/fcntl.html

"The arg values to F_GETFD, F_SETFD, F_GETFL, and F_SETFL all represent
flag values to allow for future growth. Applications using these functions
should do a read-modify-write operation on them, rather than assuming that
only the values defined by this volume of IEEE Std 1003.1-2001 are valid.
It is a common error to forget this, particularly in the case of F_SETFD."

> instead of manually calling fcntl.
> 
> CID: 1374831
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: gr...@kaod.org
> CC: aneesh.ku...@linux.vnet.ibm.com
> CC: Eric Blake 
> ---
>  hw/9pfs/xen-9p-backend.c | 2 +-
>  hw/xen/xen_backend.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Anyway,

Reviewed-by: Greg Kurz 

> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index a1fdede..5df97c9 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -380,7 +380,7 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
>  if (xen_9pdev->rings[i].evtchndev == NULL) {
>  goto out;
>  }
> -fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev), F_SETFD,
> FD_CLOEXEC);
> +qemu_set_cloexec(xenevtchn_fd(xen_9pdev->rings[i].evtchndev));
>  xen_9pdev->rings[i].local_port = xenevtchn_bind_interdomain
>  (xen_9pdev->rings[i].evtchndev,
>   xendev->dom,
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index c85f163..2cac47d 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -147,7 +147,7 @@ static struct XenDevice *xen_be_get_xendev(const char
> *type, int dom, int dev, qdev_unplug(DEVICE(xendev), NULL);
>  return NULL;
>  }
> -fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
> +qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
>  
>  if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
>  xendev->gnttabdev = xengnttab_open(NULL, 0);



pgpLazThXV4yn.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 0/1] slirp: add SOCKS5 support

[Samuel Thibault]

Hello,

Laurent Vivier, on mar. 09 mai 2017 21:31:11 +0200, wrote:
> This patch implements the SOCKS5 client part for "-net user" backend.

Thanks for the changes!

It seems from the bot result that windows doesn't define in_port_t.
Could you post a revised patch which uses sizeof(uint16_t) instead of
sizeof(in_port_t), to trigger the bot again and confirm that this is the
last needed fix for windows?

Samuel

Re: [Qemu-devel] [PATCH v2 2/3] Check the return value of fcntl in qemu_set_cloexec

[Greg Kurz]

On Tue,  9 May 2017 12:04:52 -0700
Stefano Stabellini  wrote:

> Assert that the return value is not an error. This issue was found by
> Coverity.
> 
> CID: 1374831
> 
> Signed-off-by: Stefano Stabellini 
> CC: gr...@kaod.org
> CC: pbonz...@redhat.com
> CC: Eric Blake 
> ---
>  util/oslib-posix.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Greg Kurz 

> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 4d9189e..16894ad 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -182,7 +182,9 @@ void qemu_set_cloexec(int fd)
>  {
>  int f;
>  f = fcntl(fd, F_GETFD);
> -fcntl(fd, F_SETFD, f | FD_CLOEXEC);
> +assert(f != -1);
> +f = fcntl(fd, F_SETFD, f | FD_CLOEXEC);
> +assert(f != -1);
>  }
>  
>  /*


pgphWhfK8FVLV.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity

[Greg Kurz]

On Tue,  9 May 2017 12:04:51 -0700
Stefano Stabellini  wrote:

> CID: 1374836
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: gr...@kaod.org
> CC: aneesh.ku...@linux.vnet.ibm.com
> ---
>  hw/9pfs/xen-9p-backend.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index 9c7f41a..a1fdede 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -332,12 +332,14 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
>  str = g_strdup_printf("ring-ref%u", i);
>  if (xenstore_read_fe_int(_9pdev->xendev, str,
>   _9pdev->rings[i].ref) == -1) {
> +g_free(str);
>  goto out;
>  }
>  g_free(str);

I would rather do something like:

int ret;

[...]

str = g_strdup_printf("ring-ref%u", i);
ret = xenstore_read_fe_int(_9pdev->xendev, str,
   _9pdev->rings[i].ref);
g_free(str);
if (ret ==  -1) {
goto out;
}

but this is a matter of taste and you own this code so:

Reviewed-by: Greg Kurz 

>  str = g_strdup_printf("event-channel-%u", i);
>  if (xenstore_read_fe_int(_9pdev->xendev, str,
>   _9pdev->rings[i].evtchn) == -1) {
> +g_free(str);
>  goto out;
>  }
>  g_free(str);



pgpSpemgmCMyg.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 0/2] Support CPUID signature for TCG

[Eduardo Habkost]

On Tue, May 09, 2017 at 03:13:37PM +0100, Richard W.M. Jones wrote:
> On Tue, May 09, 2017 at 07:05:51AM -0700, Richard Henderson wrote:
> > On 05/09/2017 06:27 AM, Daniel P. Berrange wrote:
> > >This enables report of a signature in CPUID for the TCG
> > >interpretor.
> > >
> > >Changed in v4:
> > >
> > >  - Report 0x4001 in EAX for 0x4000 index (Eduardo)
> > >  - Report all zeros for 0x4001 index (Eduardo)
> > >  - Make code style consistent when checking limits (Eduardo)
> > >
> > >Changed in v3:
> > >
> > >  - Simplify CPU limit code still further (Eduardo)
> > >
> > >Changed in v2:
> > >
> > >  - Rewrite the way we bounds check / cap the CPUID index
> > >to use a flat switch, instead of nested ifs (Eduardo)
> > >  - Add a 'tcg-cpuid' property to allow it to be hidden
> > >(Eduardo)
> > >  - Hide the TCG signature for old machine types
> > >  - Force code to a no-op if tcg_enabled() is false (Eduardo)
> > >
> > >
> > >NB, I did not introduce a general 'hypervisor-cpuid' property
> > >to obsolete the existing 'kvm=off|on' -cpu property, since it
> > >appears impossible to get the back compat semantics right,
> > >as described in a previous reply.
> > >
> > >Daniel P. Berrange (2):
> > >   i386: rewrite way CPUID index is validated
> > >   i386: expose "TCGTCGTCGTCG" in the 0x4000 CPUID leaf
> > 
> > I probably should have commented earlier but...  what's the point?
> > 
> > If you want the guest os to actually do anything with this, what do
> > you gain for advertising TCG over KVM?
> 
> I can see this being useful from virt-what, since it would allow
> vendors to diagnose problems being caused by TCG (since as Dan
> mentions in the other reply and as you are already well acquainted
> with, TCG and KVM don't present or emulate quite the same thing).
> 
> That said, I don't think it's anything more than a nice to have.

We also have systemd-detect-virt, and the ConditionVirtualization
option in systemd.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v5 0/1] slirp: add SOCKS5 support

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH v5 0/1] slirp: add SOCKS5 support
Message-id: 20170509193112.16613-1-laur...@vivier.eu
Type: series

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

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

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

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
b857ae8 slirp: add SOCKS5 support

=== OUTPUT BEGIN ===
Checking PATCH 1/1: slirp: add SOCKS5 support...
WARNING: line over 80 characters
#231: FILE: slirp/slirp.c:662:
+  slirp->proxy_password, 
so->fhost.ss,

ERROR: suspect code indent for conditional statements (4, 6)
#801: FILE: slirp/tcp_subr.c:428:
+if (so->so_proxy_state) {
+  ret = socks5_connect(s, slirp->proxy_server, slirp->proxy_port,

total: 1 errors, 1 warnings, 747 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] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

[Manos Pitsidianakis]

On Tue, May 09, 2017 at 02:52:38PM -0500, Eric Blake wrote:

On 05/09/2017 02:43 PM, Manos Pitsidianakis wrote:

On Thu, May 04, 2017 at 04:00:06PM +0200, Peter Krempa wrote:

+cookie_secret = qemu_opt_get(opts, CURL_BLOCK_OPT_COOKIE_SECRET);
+
+if (cookie && cookie_secret) {
+error_setg(errp,
+   "curl driver cannot handle both cookie and cookie
secret");
+goto out_noclean;
+}
+
+if (cookie_secret) {
+s->cookie = qcrypto_secret_lookup_as_utf8(cookie_secret, errp);
+if (!s->cookie) {
+goto out_noclean;
+}
+} else {
+s->cookie = g_strdup(cookie);
+}


There's no check here for if both cookie and cookie_secret are NULL.


Is that a problem?  s->cookie ends up as NULL (thanks to g_strdup()
semantics), which merely means there's no cookie to be sent after all.


Ah yes, g_strdup(NULL) returns NULL. Apologies for the noise.



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






signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

[Eric Blake]

On 05/09/2017 02:43 PM, Manos Pitsidianakis wrote:
> On Thu, May 04, 2017 at 04:00:06PM +0200, Peter Krempa wrote:
>> +cookie_secret = qemu_opt_get(opts, CURL_BLOCK_OPT_COOKIE_SECRET);
>> +
>> +if (cookie && cookie_secret) {
>> +error_setg(errp,
>> +   "curl driver cannot handle both cookie and cookie
>> secret");
>> +goto out_noclean;
>> +}
>> +
>> +if (cookie_secret) {
>> +s->cookie = qcrypto_secret_lookup_as_utf8(cookie_secret, errp);
>> +if (!s->cookie) {
>> +goto out_noclean;
>> +}
>> +} else {
>> +s->cookie = g_strdup(cookie);
>> +}
> 
> There's no check here for if both cookie and cookie_secret are NULL.

Is that a problem?  s->cookie ends up as NULL (thanks to g_strdup()
semantics), which merely means there's no cookie to be sent after all.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity

[Stefano Stabellini]

On Tue, 9 May 2017, Eric Blake wrote:
> On 05/09/2017 02:20 PM, Eric Blake wrote:
> > On 05/09/2017 02:04 PM, Stefano Stabellini wrote:
> >> CID: 1374836
> >>
> >> Signed-off-by: Stefano Stabellini 
> >> CC: anthony.per...@citrix.com
> >> CC: gr...@kaod.org
> >> CC: aneesh.ku...@linux.vnet.ibm.com
> >> ---
> >>  hw/9pfs/xen-9p-backend.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> > 
> > Reviewed-by: Eric Blake 
> 
> By the way, you forgot to send a 0/3 cover letter (at least, there was
> no In-Reply-To: header in your 1/3 mail).  That makes it harder to
> automate handling of your series; I was going to reply to the series as
> a whole.
> 
> While it is inconvenient for human readers, it is even worse for some of
> the automated patch tooling we have that expects cover letters for any
> multi-patch series.  More patch submission tips at
> http://wiki.qemu.org/Contribute/SubmitAPatch include how to use 'git
> config' to automate the creation of a cover letter.

Sorry about that, and thanks for the review.

Re: [Qemu-devel] [PULL 00/23] Trivial patches for 2017-05-07

[Stefan Hajnoczi]

On Mon, May 08, 2017 at 01:30:31PM -0400, Stefan Hajnoczi wrote:
> On Sun, May 07, 2017 at 10:02:03AM +0300, Michael Tokarev wrote:
> > The following changes since commit 12a95f320a36ef66f724a49bb05e4fb553ac5dbe:
> > 
> >   Merge remote-tracking branch 'kwolf/tags/for-upstream' into staging 
> > (2017-05-04 13:44:32 +0100)
> > 
> > are available in the git repository at:
> > 
> >   git://git.corpit.ru/qemu.git tags/trivial-patches-fetch
> > 
> > for you to fetch changes up to 311875781e549af91a3216d34c6ef40420bab435:
> > 
> >   tests: Remove redundant assignment (2017-05-07 09:57:51 +0300)
> > 
> > 
> > trivial patches for 2017-05-07
> > 
> > 
> > Andreas Grapentin (1):
> >   use _Static_assert in QEMU_BUILD_BUG_ON
> > 
> > Chris Webb (1):
> >   virtfs: allow a device id to be specified in the -virtfs option
> > 
> > Craig Jellick (1):
> >   Add 'none' as type for drive's if option
> > 
> > Eric Blake (2):
> >   tests: Ignore more test executables
> >   tests: Ignore another built executable (test-hmp)
> > 
> > Fam Zheng (3):
> >   block: Make 'replication_state' an enum
> >   virtio-blk: Remove useless condition around g_free()
> >   tests: Remove redundant assignment
> > 
> > Ishani Chugh (1):
> >   Remove reduntant qemu: from error functions
> > 
> > KONRAD Frederic (1):
> >   ppc_booke: drop useless assignment
> > 
> > Kamil Rytarowski (2):
> >   scripts/qemu-binfmt-conf.sh: Fix shell portability issue
> >   scripts: Switch to more portable Perl shebang
> > 
> > Marc-André Lureau (1):
> >   doc: fix function spelling
> > 
> > Paolo Bonzini (3):
> >   jazz_led: fix bad snprintf
> >   MAINTAINERS: Update paths for main loop
> >   MAINTAINERS: Update paths for AioContext implementation
> > 
> > Philippe Mathieu-Daudé (3):
> >   usb-ccid: make ccid_write_data_block() cope with null buffers
> >   device_tree: fix compiler warnings (clang 5)
> >   qga: fix compiler warnings (clang 5)
> > 
> > Saurav Sachidanand (1):
> >   util: Use g_malloc/g_free in envlist.c
> > 
> > Thomas Huth (2):
> >   qemu-doc: Fix broken URLs of amnhltm.zip and dosidle210.zip
> >   hw/core/generic-loader: Fix crash when running without CPU
> > 
> > sochin.jiang fix wrong parameter comments in channel-file.h (1):
> >   channel-file: fix wrong parameter comments
> > 
> >  MAINTAINERS |  8 +++
> >  block/replication.c | 44 +++---
> >  bsd-user/main.c | 14 
> >  configure   | 18 
> >  device_tree.c   |  1 +
> >  hw/block/virtio-blk.c   |  4 +---
> >  hw/core/generic-loader.c|  9 
> >  hw/display/jazz_led.c   |  4 ++--
> >  hw/microblaze/boot.c|  2 +-
> >  hw/nios2/boot.c |  2 +-
> >  hw/ppc/pnv.c|  2 +-
> >  hw/ppc/ppc_booke.c  |  1 -
> >  hw/s390x/sclp.c |  4 ++--
> >  hw/tricore/tricore_testboard.c  |  2 +-
> >  hw/usb/dev-smartcard-reader.c   |  5 -
> >  include/io/channel-file.h   |  2 +-
> >  include/io/channel.h|  2 +-
> >  include/qemu/compiler.h |  4 +++-
> >  linux-user/main.c   |  9 +++-
> >  numa.c  |  4 ++--
> >  qemu-doc.texi   | 10 -
> >  qemu-options.hx |  4 ++--
> >  qga/commands-posix.c|  8 ---
> >  scripts/checkpatch.pl   |  3 ++-
> >  scripts/clean-header-guards.pl  |  3 ++-
> >  scripts/cleanup-trace-events.pl |  2 +-
> >  scripts/disas-objdump.pl|  4 +++-
> >  scripts/get_maintainer.pl   |  3 ++-
> >  scripts/qemu-binfmt-conf.sh |  4 ++--
> >  scripts/shaderinclude.pl|  2 +-
> >  scripts/switch-timer-api|  2 +-
> >  scripts/texi2pod.pl |  4 +++-
> >  tests/.gitignore|  4 
> >  tests/postcopy-test.c   |  2 +-
> >  util/envlist.c  | 47 
> > +
> >  vl.c|  5 +++--
> >  36 files changed, 133 insertions(+), 115 deletions(-)
> > 
> 
> Thanks, applied to my staging tree:
> https://github.com/stefanha/qemu/commits/staging

Dropped for now, please see my reply to "channel-file: fix wrong parameter 
comments".


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 11/23] channel-file: fix wrong parameter comments

[Stefan Hajnoczi]

On Sun, May 07, 2017 at 10:02:14AM +0300, Michael Tokarev wrote:
> From: "sochin.jiang fix wrong parameter comments in channel-file.h" 
> 

Hi Michael,
This patch has a corrupted From field.  I do not modify pull requests
myself when merging.  Could you please resend?

Thanks,
Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH] block: curl: Allow passing cookies via QCryptoSecret

[Manos Pitsidianakis]

On Thu, May 04, 2017 at 04:00:06PM +0200, Peter Krempa wrote:

+cookie_secret = qemu_opt_get(opts, CURL_BLOCK_OPT_COOKIE_SECRET);
+
+if (cookie && cookie_secret) {
+error_setg(errp,
+   "curl driver cannot handle both cookie and cookie secret");
+goto out_noclean;
+}
+
+if (cookie_secret) {
+s->cookie = qcrypto_secret_lookup_as_utf8(cookie_secret, errp);
+if (!s->cookie) {
+goto out_noclean;
+}
+} else {
+s->cookie = g_strdup(cookie);
+}


There's no check here for if both cookie and cookie_secret are NULL. 

[Qemu-devel] [PATCH v5 1/1] slirp: add SOCKS5 support

[Laurent Vivier]

When the VM is used behind a firewall, This allows
the use of a SOCKS5 proxy server to connect the VM IP stack
directly to the Internet.

This implementation doesn't manage UDP packets, so they
are simply dropped (as with restrict=on), except for
the localhost as we need it for DNS.

Signed-off-by: Laurent Vivier 
---
 net/slirp.c |  39 +-
 qapi-schema.json|   9 ++
 qemu-options.hx |  11 ++
 slirp/Makefile.objs |   2 +-
 slirp/ip_icmp.c |   2 +-
 slirp/libslirp.h|   3 +
 slirp/slirp.c   |  65 +
 slirp/slirp.h   |   6 +
 slirp/socket.h  |   4 +
 slirp/socks5.c  | 379 
 slirp/socks5.h  |  31 +
 slirp/tcp_subr.c|  22 ++-
 slirp/udp.c |   9 ++
 slirp/udp6.c|   8 ++
 14 files changed, 583 insertions(+), 7 deletions(-)
 create mode 100644 slirp/socks5.c
 create mode 100644 slirp/socks5.h

diff --git a/net/slirp.c b/net/slirp.c
index c705a60..06a32f7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -41,6 +41,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 #include "qapi/error.h"
+#include "crypto/secret.h"
 
 static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
 {
@@ -139,6 +140,33 @@ static void net_slirp_cleanup(NetClientState *nc)
 QTAILQ_REMOVE(_stacks, s, entry);
 }
 
+static int net_slirp_add_proxy(SlirpState *s, const char *proxy_server,
+   const char *proxy_user,
+   const char *proxy_secretid)
+{
+InetSocketAddress *addr;
+char *password = NULL;
+int ret;
+
+if (proxy_server == NULL) {
+return 0;
+}
+
+if (proxy_secretid) {
+password = qcrypto_secret_lookup_as_utf8(proxy_secretid, _fatal);
+}
+
+addr = inet_parse(proxy_server, _fatal);
+
+ret = slirp_add_proxy(s->slirp, addr->host, atoi(addr->port),
+  proxy_user, password);
+
+qapi_free_InetSocketAddress(addr);
+g_free(password);
+
+return ret;
+}
+
 static NetClientInfo net_slirp_info = {
 .type = NET_CLIENT_DRIVER_USER,
 .size = sizeof(SlirpState),
@@ -155,7 +183,8 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
   const char *bootfile, const char *vdhcp_start,
   const char *vnameserver, const char *vnameserver6,
   const char *smb_export, const char *vsmbserver,
-  const char **dnssearch)
+  const char **dnssearch, const char *proxy_server,
+  const char *proxy_user, const char *proxy_secretid)
 {
 /* default settings according to historic slirp */
 struct in_addr net  = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
@@ -361,6 +390,11 @@ static int net_slirp_init(NetClientState *peer, const char 
*model,
 }
 #endif
 
+if (net_slirp_add_proxy(s, proxy_server,
+proxy_user, proxy_secretid) < 0) {
+goto error;
+}
+
 s->exit_notifier.notify = slirp_smb_exit;
 qemu_add_exit_notifier(>exit_notifier);
 return 0;
@@ -882,7 +916,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
  user->ipv6_host, user->hostname, user->tftp,
  user->bootfile, user->dhcpstart,
  user->dns, user->ipv6_dns, user->smb,
- user->smbserver, dnssearch);
+ user->smbserver, dnssearch, user->proxy_server,
+ user->proxy_user, user->proxy_secretid);
 
 while (slirp_configs) {
 config = slirp_configs;
diff --git a/qapi-schema.json b/qapi-schema.json
index 01b087f..bcaf85b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3661,6 +3661,12 @@
 #
 # @guestfwd: forward guest TCP connections
 #
+# @proxy-server: address of the SOCKS5 proxy server to use (since 2.10)
+#
+# @proxy-user: username to use with the proxy server (since 2.10)
+#
+# @proxy-secretid: secret id to use for the proxy server password (since 2.10)
+#
 # Since: 1.2
 ##
 { 'struct': 'NetdevUserOptions',
@@ -3683,6 +3689,9 @@
 '*ipv6-dns': 'str',
 '*smb':   'str',
 '*smbserver': 'str',
+'*proxy-server': 'str',
+'*proxy-user':   'str',
+'*proxy-secretid': 'str',
 '*hostfwd':   ['String'],
 '*guestfwd':  ['String'] } }
 
diff --git a/qemu-options.hx b/qemu-options.hx
index f68829f..0a26a62 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1682,6 +1682,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
 #ifndef _WIN32
  "[,smb=dir[,smbserver=addr]]\n"
 #endif
+" [,proxy-server=addr:port[,proxy-user=user,proxy-secretid=id]]\n"
 "configure a user mode network backend with ID 'str',\n"
 "its DHCP server and optional services\n"
 #endif
@@ -1920,6 +1921,16 @@ 

[Qemu-devel] [PATCH v5 0/1] slirp: add SOCKS5 support

[Laurent Vivier]

This patch implements the SOCKS5 client part for "-net user" backend.

It allows to route all internet traffic of the virtual machine
to a SOCKS5 server.

But all the local traffic (to the host) is sent to the host.
It is needed because this SOCKS5 client doesn't route UDP traffic,
and this allows to use the host DNS server.

I've tested this using public SOCKS5 proxy list found on the WEB, and
using TOR server on my host.

Used with TOR, all the TCP connections are sent to the TOR network and
this allows to insert a virtual machine directly in the TOR network
without needing more configuration in the virtual machine.

But be aware that all DNS requests will be sent to the host that can
forward them to internet with its own IP address. So confidentiality
will not be as good as with the TOR browser which hides in the TOR
network all the DNS requests.

If you want to test this:

- with a public SOCKS5 server, ask google for "socks5 proxy address"
  and start QEMU with, for instance:

  qemu-system-x86_64 -net nic,model=e1000 -net 
user,proxy-server=46.105.121.37:63066 ...

  if needed, you can provide user/password using secret objects framework:
  "-object secret,id=sec0,data=password,format=raw \
   -net user,...,proxy-user=user,proxy-secretid=sec0"

- with a local TOR proxy:

  sudo systemctl start tor
  qemu-system-x86_64 -net nic,model=e1000 -net user,proxy-server=localhost:9050 
...

You can check your IP address is the one of the proxy by connecting
to http://check.torproject.org with a browser inside the VM.

v6:
  - fix error case in slirp_add_proxy()
  - add copyright and license in new files
  - add guards around socks5_recv() and socks5_send()

v5:
  - remove useless #includes

v4:
  - rebase
  - define some macros to use instead of meaningless values
  - better management of error cases
  - add some error logs

v3:
  - rewrite SOCKS5 functions from scratch and drop code from nmap/ncat
  - don't drop the IPv6 UDP local traffic
  - don't pass to the proxy the IPv6 TCP local traffic
  - add some comments...

v2:
  - use secret objects framework to provide password
  - add documentation for new parameters in qapi-schema.json
  - s/passwd/password/g
  - I didn't move proxy paramaters to a substruct as I didn't
find a way to set them from the command line :(

Laurent Vivier (1):
  slirp: add SOCKS5 support

 net/slirp.c |  39 +-
 qapi-schema.json|   9 ++
 qemu-options.hx |  11 ++
 slirp/Makefile.objs |   2 +-
 slirp/ip_icmp.c |   2 +-
 slirp/libslirp.h|   3 +
 slirp/slirp.c   |  65 +
 slirp/slirp.h   |   6 +
 slirp/socket.h  |   4 +
 slirp/socks5.c  | 379 
 slirp/socks5.h  |  31 +
 slirp/tcp_subr.c|  22 ++-
 slirp/udp.c |   9 ++
 slirp/udp6.c|   8 ++
 14 files changed, 583 insertions(+), 7 deletions(-)
 create mode 100644 slirp/socks5.c
 create mode 100644 slirp/socks5.h

-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity

[Eric Blake]

On 05/09/2017 02:20 PM, Eric Blake wrote:
> On 05/09/2017 02:04 PM, Stefano Stabellini wrote:
>> CID: 1374836
>>
>> Signed-off-by: Stefano Stabellini 
>> CC: anthony.per...@citrix.com
>> CC: gr...@kaod.org
>> CC: aneesh.ku...@linux.vnet.ibm.com
>> ---
>>  hw/9pfs/xen-9p-backend.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> Reviewed-by: Eric Blake 

By the way, you forgot to send a 0/3 cover letter (at least, there was
no In-Reply-To: header in your 1/3 mail).  That makes it harder to
automate handling of your series; I was going to reply to the series as
a whole.

While it is inconvenient for human readers, it is even worse for some of
the automated patch tooling we have that expects cover letters for any
multi-patch series.  More patch submission tips at
http://wiki.qemu.org/Contribute/SubmitAPatch include how to use 'git
config' to automate the creation of a cover letter.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/3] Check the return value of fcntl in qemu_set_cloexec

[Eric Blake]

On 05/09/2017 02:04 PM, Stefano Stabellini wrote:
> Assert that the return value is not an error. This issue was found by
> Coverity.
> 
> CID: 1374831
> 
> Signed-off-by: Stefano Stabellini 
> CC: gr...@kaod.org
> CC: pbonz...@redhat.com
> CC: Eric Blake 
> ---
>  util/oslib-posix.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 1/1] slirp: add SOCKS5 support

[Samuel Thibault]

Hello,

Laurent Vivier, on mar. 09 mai 2017 09:21:09 +0200, wrote:
> Le 08/05/2017 à 22:09, Samuel Thibault a écrit :
> > Laurent Vivier, on sam. 06 mai 2017 15:19:44 +0200, wrote:
> >>> Laurent Vivier, on sam. 06 mai 2017 00:48:33 +0200, wrote:
> >> The check is done previously by:
> >>
> >> @@ -442,6 +443,10 @@ void slirp_pollfds_fill(GArray *pollfds, uint32_t
> >> *timeout)
> >>  .fd = so->s,
> >>  .events = G_IO_OUT | G_IO_ERR,
> >>  };
> >> +if (so->so_proxy_state &&
> >> +so->so_proxy_state != SOCKS5_STATE_ERROR) {
> >> +pfd.events |= G_IO_IN;
> >> +}
> >>
> >> We can enter in socks5_recv() only if so->so_proxy_state is in a valid
> >> state because G_IO_IN triggers that.
> > 
> > I don't understand: the socks5_recv gets called not only when pfd.events
> > contains G_IO_IN, but also when it contains G_IO_HUP or G_IO_ERR.  Also,
> > so_proxy_state being non-nul is not the only way to have G_IO_IN set in
> > pfd.events, so_state & SS_FACCEPTCONN and CONN_CANFRCV(so) also trigger
> > that.
> 
> This is filtered by (so_state & SS_ISFCONNECTING). The only case when we
> enter in the receiving part with SS_ISFCONNECTING is when socks5 part
> has been enabled.

The code snippet above is guarded by (so_state & SS_ISFCONNECTING),
but that won't prevent socks5_recv from being called here in
slirp_pollfds_poll in the non-socks5 case:

 else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
+if (so->so_state & SS_ISFCONNECTING) {
+socks5_recv(so->s, >so_proxy_state);
+continue;
+}
 /*

e.g. when (so->so_state & SS_FACCEPTCONN) or when CONN_CANFRCV(so) in
slirp_pollfds_fill, which both set G_IO_IN too.

>  @@ -645,11 +654,19 @@ void slirp_pollfds_poll(GArray *pollfds, int 
>  select_error)
>   /*
>    * Check for non-blocking, still-connecting sockets
>    */
>  -if (so->so_state & SS_ISFCONNECTING) {
>  -/* Connected */
>  -so->so_state &= ~SS_ISFCONNECTING;
>   
>  -ret = send(so->s, (const void *) , 0, 0);
>  +if (so->so_state & SS_ISFCONNECTING) {
>  +ret = socks5_send(so->s, slirp->proxy_user,
> >>>
> >>> Ditto.
> >>
> >> if so_proxy_state is 0 the function does nothing
> > 
> > Well, that's quite weak reason to me, and will confuse readers, because
> > they'll think that the code is for the socks5 case only.
> 
> OK, I'm going to add a "if (so->so_proxy_state && so->so_proxy_state !=
> SOCKS5_STATE_ERROR)" here.

Thanks :)

>  +++ b/slirp/socks5.c
>  @@ -0,0 +1,371 @@
> >>>
> >>> In v2 of the patch, this was said to have "some parts from nmap/ncat
> >>> GPLv2".  Is that really not true any more?  If any part of the file is
> >>> not original, it *has* to wear proper copyright notices, otherwise it's
> >>> copyright infrigement.
> >>
> >> No, I've re-written entirely this part from RFC.
> > 
> > Ok.
> > 
> >> for ncat.h
> > 
> > Which code comes from ncat.h?
> 
> I haven't been clear: none. I've entirely re-written this part.

Ah, ok.

Samuel

Re: [Qemu-devel] [PATCH v2 3/3] xen: call qemu_set_cloexec instead of fcntl

[Eric Blake]

On 05/09/2017 02:04 PM, Stefano Stabellini wrote:
> Use the common utility function, which contains checks on return values,
> instead of manually calling fcntl.
> 
> CID: 1374831
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: gr...@kaod.org
> CC: aneesh.ku...@linux.vnet.ibm.com
> CC: Eric Blake 
> ---
>  hw/9pfs/xen-9p-backend.c | 2 +-
>  hw/xen/xen_backend.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity

[Eric Blake]

On 05/09/2017 02:04 PM, Stefano Stabellini wrote:
> CID: 1374836
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: gr...@kaod.org
> CC: aneesh.ku...@linux.vnet.ibm.com
> ---
>  hw/9pfs/xen-9p-backend.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 2/3] Check the return value of fcntl in qemu_set_cloexec

[Stefano Stabellini]

Assert that the return value is not an error. This issue was found by
Coverity.

CID: 1374831

Signed-off-by: Stefano Stabellini 
CC: gr...@kaod.org
CC: pbonz...@redhat.com
CC: Eric Blake 
---
 util/oslib-posix.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 4d9189e..16894ad 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -182,7 +182,9 @@ void qemu_set_cloexec(int fd)
 {
 int f;
 f = fcntl(fd, F_GETFD);
-fcntl(fd, F_SETFD, f | FD_CLOEXEC);
+assert(f != -1);
+f = fcntl(fd, F_SETFD, f | FD_CLOEXEC);
+assert(f != -1);
 }
 
 /*
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/3] xen: call qemu_set_cloexec instead of fcntl

[Stefano Stabellini]

Use the common utility function, which contains checks on return values,
instead of manually calling fcntl.

CID: 1374831

Signed-off-by: Stefano Stabellini 
CC: anthony.per...@citrix.com
CC: gr...@kaod.org
CC: aneesh.ku...@linux.vnet.ibm.com
CC: Eric Blake 
---
 hw/9pfs/xen-9p-backend.c | 2 +-
 hw/xen/xen_backend.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index a1fdede..5df97c9 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -380,7 +380,7 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
 if (xen_9pdev->rings[i].evtchndev == NULL) {
 goto out;
 }
-fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev), F_SETFD, 
FD_CLOEXEC);
+qemu_set_cloexec(xenevtchn_fd(xen_9pdev->rings[i].evtchndev));
 xen_9pdev->rings[i].local_port = xenevtchn_bind_interdomain
 (xen_9pdev->rings[i].evtchndev,
  xendev->dom,
diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index c85f163..2cac47d 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -147,7 +147,7 @@ static struct XenDevice *xen_be_get_xendev(const char 
*type, int dom, int dev,
 qdev_unplug(DEVICE(xendev), NULL);
 return NULL;
 }
-fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
+qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
 
 if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
 xendev->gnttabdev = xengnttab_open(NULL, 0);
-- 
1.9.1

[Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity

[Stefano Stabellini]

CID: 1374836

Signed-off-by: Stefano Stabellini 
CC: anthony.per...@citrix.com
CC: gr...@kaod.org
CC: aneesh.ku...@linux.vnet.ibm.com
---
 hw/9pfs/xen-9p-backend.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 9c7f41a..a1fdede 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -332,12 +332,14 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
 str = g_strdup_printf("ring-ref%u", i);
 if (xenstore_read_fe_int(_9pdev->xendev, str,
  _9pdev->rings[i].ref) == -1) {
+g_free(str);
 goto out;
 }
 g_free(str);
 str = g_strdup_printf("event-channel-%u", i);
 if (xenstore_read_fe_int(_9pdev->xendev, str,
  _9pdev->rings[i].evtchn) == -1) {
+g_free(str);
 goto out;
 }
 g_free(str);
-- 
1.9.1

Re: [Qemu-devel] [PATCH RESEND V3 5/6] migration: calculate downtime on dst side

[Dr. David Alan Gilbert]

* Alexey (a.pereva...@samsung.com) wrote:
> On Tue, May 09, 2017 at 10:40:34AM +0100, Dr. David Alan Gilbert wrote:
> > * Peter Xu (pet...@redhat.com) wrote:
> > > On Mon, May 08, 2017 at 12:08:07PM +0300, Alexey wrote:
> > > > On Mon, May 08, 2017 at 02:29:06PM +0800, Peter Xu wrote:
> > > > > On Fri, Apr 28, 2017 at 02:11:19PM +0300, Alexey Perevalov wrote:
> > > > > > On 04/28/2017 01:00 PM, Peter Xu wrote:
> > > > > > >On Fri, Apr 28, 2017 at 09:57:37AM +0300, Alexey Perevalov wrote:
> > > > > > >>This patch provides downtime calculation per vCPU,
> > > > > > >>as a summary and as a overlapped value for all vCPUs.
> > > > > > >>
> > > > > > >>This approach was suggested by Peter Xu, as an improvements of
> > > > > > >>previous approch where QEMU kept tree with faulted page address 
> > > > > > >>and cpus bitmask
> > > > > > >>in it. Now QEMU is keeping array with faulted page address as 
> > > > > > >>value and vCPU
> > > > > > >>as index. It helps to find proper vCPU at UFFD_COPY time. Also it 
> > > > > > >>keeps
> > > > > > >>list for downtime per vCPU (could be traced with page_fault_addr)
> > > > > > >>
> > > > > > >>For more details see comments for get_postcopy_total_downtime
> > > > > > >>implementation.
> > > > > > >>
> > > > > > >>Downtime will not calculated if postcopy_downtime field of
> > > > > > >>MigrationIncomingState wasn't initialized.
> > > > > > >>
> > > > > > >>Signed-off-by: Alexey Perevalov 
> > > > > > >>---
> > > > > > >>  include/migration/migration.h |   3 ++
> > > > > > >>  migration/migration.c | 103 
> > > > > > >> ++
> > > > > > >>  migration/postcopy-ram.c  |  20 +++-
> > > > > > >>  migration/trace-events|   6 ++-
> > > > > > >>  4 files changed, 130 insertions(+), 2 deletions(-)
> > > > > > >>
> > > > > > >>diff --git a/include/migration/migration.h 
> > > > > > >>b/include/migration/migration.h
> > > > > > >>index e8fb68f..a22f9ce 100644
> > > > > > >>--- a/include/migration/migration.h
> > > > > > >>+++ b/include/migration/migration.h
> > > > > > >>@@ -139,6 +139,9 @@ void migration_incoming_state_destroy(void);
> > > > > > >>   * Functions to work with downtime context
> > > > > > >>   */
> > > > > > >>  struct DowntimeContext *downtime_context_new(void);
> > > > > > >>+void mark_postcopy_downtime_begin(uint64_t addr, int cpu);
> > > > > > >>+void mark_postcopy_downtime_end(uint64_t addr);
> > > > > > >>+uint64_t get_postcopy_total_downtime(void);
> > > > > > >>  struct MigrationState
> > > > > > >>  {
> > > > > > >>diff --git a/migration/migration.c b/migration/migration.c
> > > > > > >>index ec76e5c..2c6f150 100644
> > > > > > >>--- a/migration/migration.c
> > > > > > >>+++ b/migration/migration.c
> > > > > > >>@@ -2150,3 +2150,106 @@ PostcopyState 
> > > > > > >>postcopy_state_set(PostcopyState new_state)
> > > > > > >>  return atomic_xchg(_postcopy_state, new_state);
> > > > > > >>  }
> > > > > > >>+void mark_postcopy_downtime_begin(uint64_t addr, int cpu)
> > > > > > >>+{
> > > > > > >>+MigrationIncomingState *mis = 
> > > > > > >>migration_incoming_get_current();
> > > > > > >>+DowntimeContext *dc;
> > > > > > >>+if (!mis->downtime_ctx || cpu < 0) {
> > > > > > >>+return;
> > > > > > >>+}
> > > > > > >>+dc = mis->downtime_ctx;
> > > > > > >>+dc->vcpu_addr[cpu] = addr;
> > > > > > >>+dc->last_begin = dc->page_fault_vcpu_time[cpu] =
> > > > > > >>+qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> > > > > > >>+
> > > > > > >>+trace_mark_postcopy_downtime_begin(addr, dc, 
> > > > > > >>dc->page_fault_vcpu_time[cpu],
> > > > > > >>+cpu);
> > > > > > >>+}
> > > > > > >>+
> > > > > > >>+void mark_postcopy_downtime_end(uint64_t addr)
> > > > > > >>+{
> > > > > > >>+MigrationIncomingState *mis = 
> > > > > > >>migration_incoming_get_current();
> > > > > > >>+DowntimeContext *dc;
> > > > > > >>+int i;
> > > > > > >>+bool all_vcpu_down = true;
> > > > > > >>+int64_t now;
> > > > > > >>+
> > > > > > >>+if (!mis->downtime_ctx) {
> > > > > > >>+return;
> > > > > > >>+}
> > > > > > >>+dc = mis->downtime_ctx;
> > > > > > >>+now = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> > > > > > >>+
> > > > > > >>+/* check all vCPU down,
> > > > > > >>+ * QEMU has bitmap.h, but even with bitmap_and
> > > > > > >>+ * will be a cycle */
> > > > > > >>+for (i = 0; i < smp_cpus; i++) {
> > > > > > >>+if (dc->vcpu_addr[i]) {
> > > > > > >>+continue;
> > > > > > >>+}
> > > > > > >>+all_vcpu_down = false;
> > > > > > >>+break;
> > > > > > >>+}
> > > > > > >>+
> > > > > > >>+if (all_vcpu_down) {
> > > > > > >>+dc->total_downtime += now - dc->last_begin;
> > > > > > >Shall we do this accouting only if we are sure the copied page 
> > > > > > >address
> > > > > > >is one of the page faulted addresses? Can it be some other page? I
> > > > 

Re: [Qemu-devel] VhostUserRequest # 20

[Marc-André Lureau]

HI

- Original Message -
> Hi,
>   libvhost-user.h defines:
>VHOST_USER_INPUT_GET_CONFIG = 20,

That slipped by mistake from my vhost-user-input series WIP, please send a fix.
(luckily, this is only internal header for now)

thanks

> while
>   vhost-user.c defines:
>VHOST_USER_NET_SET_MTU = 20,
> 
> who wins?
> 
> Dave
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> 

Re: [Qemu-devel] [PATCH 03/17] tests: remove alt num-int cases

[Eric Blake]

On 05/09/2017 12:35 PM, Marc-André Lureau wrote:
> There are no real users of this case, and it's going to be invalid after
> merging of QFloat and QInt use the same QNum type in the following patch.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  tests/test-keyval.c |  3 ---
>  tests/test-qobject-input-visitor.c  | 26 --
>  tests/qapi-schema/qapi-schema-test.json |  2 --
>  tests/qapi-schema/qapi-schema-test.out  |  8 
>  4 files changed, 39 deletions(-)

Is it worth adding tests/qapi-schema/alternate-num-int.json that
attempts to create an alternate on 'int' and 'number' to (eventually)
show that we give a decent error message at QAPI generation time that
such a type is invalid?  Such a test would (eventually be) a negative
replacement test for the (currently positive) test that you are deleting
here, and make it easier to see the point in the series where we flip
the concept from supported to rejected.

That said, it doesn't have to necessarily be done at this point in the
series (since I haven't even yet seen how QNum will look), so it doesn't
hold up review of this particular patch.

Also, this may interact with Eduardo's current attempt (or at least
start of an attempt) to tighten the QAPI parser to reject alternates
that would otherwise be ambiguous when passed through the string input
visitor (for example, forbidding the combination of an enum starting
with a digit in an alternate that also accepts integers).  So be aware
that we may have some integration things to think about down the road.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1689499] Re: copy-storage-all/inc does not easily converge with load going on

[Dr. David Alan Gilbert]

OK, it'll be interesting if you get something repeatable, but sometimes
these type of bugs go and hide in a dark corner until someone trips over
them in a more critical situation.

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

Title:
  copy-storage-all/inc does not easily converge with load going on

Status in QEMU:
  Incomplete

Bug description:
  Hi,
  for now this is more a report to discuss than a "bug", but I wanted to be 
sure if there are things I might overlook.

  I'm regularly testing the qemu's we have in Ubuntu which currently are
  2.0, 2.5, 2.6.1, 2.8 plus a bunch of patches. And for all sorts of
  verification upstream every now and then.

  I recently realized that the migration options around 
--copy-storage-[all/inc] seem to have got worse at converging on migration. 
Although it is not a hard commit that is to be found, it just seems more likely 
to occur the newer the qemu versions is. I assume that is partially due to 
guest performance optimization that keep it busy.
  To a user it appears as a hanging migration being locked up.

  But let me outline what actually happens:
  - Setup without shared storage
  - Migration using --copy-storage-all/--copy-storage-inc
  - Working fine with idle guests
  - If the guests is busy the migration does take like forever (1 vCPU that are 
busy with 1 CPU, 1 memory and one disk hogging processes)
  - statistically seems to trigger more likely on newer qemu's (might be a red 
herring)

  The background workloads are most trivial burners:
  - cpu: md5sum /dev/urandom
  - memory: stress-ng -m 1 --vm-keep --vm-bytes 256M
  - disk: while /bin/true; do dd if=/dev/urandom of=/var/tmp/mjb.1 bs=4M 
count=100; done

  We are talking about ~1-2 minutes on qemu 2.5 (4 tries x 3
  architectures) and 2-10+ hours on >=qemu 2.6.1.

  I say it is likely not a bug, but more a discussion as I can easily avoid 
hanging via either:
  - timeouts (--timeout, ...) to abort or suspend to migrate it
  - --auto-converge ( I had only one try, but it seemed to help by slowing down 
the load generators)

  So you might say "that is all as it should be, and the users can use
  the further options to mitigate" and I'm all fine with that. In that
  case the bug still serves as a "searchable" document of some kind for
  others triggering the same case. But if anything comes to your mind
  that need better handling around this case lets start to discuss more
  deeply about it.

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

[Qemu-devel] VhostUserRequest # 20

[Dr. David Alan Gilbert]

Hi,
  libvhost-user.h defines:
   VHOST_USER_INPUT_GET_CONFIG = 20,
while
  vhost-user.c defines:
   VHOST_USER_NET_SET_MTU = 20,

who wins?

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

Re: [Qemu-devel] [PATCH 02/17] object: fix potential leak in getters

[Eric Blake]

On 05/09/2017 12:35 PM, Marc-André Lureau wrote:
> If the property is not of the requested type, the getters will leak a
> QObject.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  qom/object.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 01/17] qdev: remove PropertyInfo.qtype field

[Eric Blake]

On 05/09/2017 12:35 PM, Marc-André Lureau wrote:
> Remove dependency on qapi qtype, replace a field by a few helper
> functions to determine the default value type (introduced in commit
> 4f2d3d7).
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  include/hw/qdev-core.h   |  1 -
>  include/hw/qdev-properties.h |  5 -
>  hw/core/qdev.c   | 32 ++--
>  3 files changed, 26 insertions(+), 12 deletions(-)
> 

> +++ b/hw/core/qdev.c
> @@ -755,6 +755,30 @@ static void qdev_property_add_legacy(DeviceState *dev, 
> Property *prop,
>  g_free(name);
>  }
>  
> +static bool prop_info_is_bool(const PropertyInfo *info)
> +{
> +return info == _prop_bit
> +|| info == _prop_bit64
> +|| info == _prop_bool;
> +}

I guess we can expand these helpers if we add more property types later.

> +
> +static bool prop_info_is_int(const PropertyInfo *info)
> +{
> +return info == _prop_uint8
> +|| info == _prop_uint16
> +|| info == _prop_uint32
> +|| info == _prop_int32
> +|| info == _prop_uint64

Interesting dissimilarity between existing types, but not the fault of
your patch.

> +|| info == _prop_size
> +|| info == _prop_pci_devfn

Okay so far.

> +|| info == _prop_on_off_auto
> +|| info == _prop_losttickpolicy
> +|| info == _prop_blockdev_on_error
> +|| info == _prop_bios_chs_trans

Aren't these four enums rather than ints?

> +|| info == _prop_blocksize
> +|| info == _prop_arraylen;
> +}
> +

> @@ -794,16 +818,12 @@ void qdev_property_add_static(DeviceState *dev, 
> Property *prop,
>  prop->info->description,
>  _abort);
>  
> -if (prop->qtype == QTYPE_NONE) {
> -return;
> -}
> -
> -if (prop->qtype == QTYPE_QBOOL) {
> +if (prop_info_is_bool(prop->info)) {
>  object_property_set_bool(obj, prop->defval, prop->name, 
> _abort);
>  } else if (prop->info->enum_table) {
>  object_property_set_str(obj, prop->info->enum_table[prop->defval],
>  prop->name, _abort);
> -} else if (prop->qtype == QTYPE_QINT) {
> +} else if (prop_info_is_int(prop->info)) {
>  object_property_set_int(obj, prop->defval, prop->name, _abort);

So enum_table has priority over prop_info_is_int(), in which case, the
four types I pointed out as being enums will still use set_str() rather
than set_int().

I'm not necessarily sold on this patch - previously, the type was local
to the declaration of the struct (you could tell by reading
qdev_prop_bit that it was a boolean type); now the type is remote (you
have to hunt elsewhere to see how the property is categorized).  I'm not
rejecting it (I see how getting rid of a dependency on QType makes it
easier for the rest of the series to change QType underpinnings), but
wonder if that is a strong enough justification.

But if we DO keep it, you'll want a v2 that cleans up the
prop_info_is_int() impedance mismatch.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 1/6] target/s390x: Implement STORE FACILITIES LIST EXTENDED

[Aurelien Jarno]

On 2017-05-09 11:07, Richard Henderson wrote:
> At the same time, improve STORE FACILITIES LIST
> so that we don't hard-code the list for all cpus.
> 
> Signed-off-by: Richard Henderson 
> ---
>  target/s390x/helper.h  |  2 ++
>  target/s390x/insn-data.def |  2 ++
>  target/s390x/misc_helper.c | 59 
> ++
>  target/s390x/translate.c   | 17 ++---
>  4 files changed, 72 insertions(+), 8 deletions(-)

Reviewed-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [RFC PATCH 02/11] hw/pci: define msi_nonbroken in pci-stub

[Thomas Huth]

 Hi Philippe,

On 09.05.2017 18:49, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 05/09/2017 02:19 AM, Thomas Huth wrote:
>> On 09.05.2017 01:39, Philippe Mathieu-Daudé wrote:
>>> This field is accessed in hw/intc/arm_gicv[23*].c
>>
>> default-configs/arm-softmmu.mak sets CONFIG_PCI, so this should not be
>> necessary, I think. Otherwise, you should extend your patch description,
>> to elaborate on what you're trying to do here.
>>
>>  Thomas
> 
> Sure, I'm willing to add basic support for the TI Hercules MCU which is
> a big endian SoC (without PCI support).
> 
> I'm using the following config:
> 
> $ cat default-configs/armeb-softmmu.mak

$ cat default-configs/armeb-softmmu.mak
cat: default-configs/armeb-softmmu.mak: No such file or directory

... so you're planning for a new armeb config file here? That's the kind
of information that I was missing in the patch description.

[...]
> As I understand the 'msi_nonbroken' is a kludge (there is no intrinsic
> relationship between PCI/MSI and this particular interrupt controller).
> Since it is declared as extern in "hw/pci/msi.h" why not define it in
> pci-stub?

Sure, your patch is fine, IMHO it just lacks some rationale in its
description.

 Thomas

Re: [Qemu-devel] [PATCH v2] qemu-img: Check for backing image if specified during create

[John Snow]

On 05/09/2017 02:19 PM, Eric Blake wrote:
> On 05/09/2017 01:17 PM, Eric Blake wrote:
>> On 05/09/2017 01:09 PM, John Snow wrote:
>>> Or, rather, force the open of a backing image if one was specified
>>> for creation. Using a similar -unsafe option as rebase, allow qemu-img
>>> to ignore the backing file validation if possible.
>>>
> 
>>> +++ b/qemu-img.c
>>> @@ -145,9 +145,11 @@ static void QEMU_NORETURN help(void)
>>> "  'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
>>> "instead\n"
>>> "  '-c' indicates that target image must be compressed (qcow 
>>> format only)\n"
>>> -   "  '-u' enables unsafe rebasing. It is assumed that old and new 
>>> backing file\n"
>>> -   "   match exactly. The image doesn't need a working backing 
>>> file before\n"
>>> -   "   rebasing in this case (useful for renaming the backing 
>>> file)\n"
>>> +   "  '-u' allows unsafe backing chains. For rebasing, it is 
>>> assumed that old and new\n"
>>> +   "   backing file match exactly. The image doesn't need a 
>>> working backing file\n"
>>> +   "   before rebasing in this case (useful for renaming the 
>>> backing file)\n"
> 
> Also, a '.' to end this sentence would be nice.
> 

d'oh.

I can respin if that's more convenient, as I am assuming Max or Kevin
will have something to say.

If not, either edit it if that's easier, or ask me to re-spin.

>>> +   "   For image creation, allow creating without attempting 
>>> to"
>>
>> Missing \n
>>
>> With that fixed,
>> Reviewed-by: Eric Blake 
>>
> 

Re: [Qemu-devel] [PATCH v2] qemu-img: Check for backing image if specified during create

[Eric Blake]

On 05/09/2017 01:17 PM, Eric Blake wrote:
> On 05/09/2017 01:09 PM, John Snow wrote:
>> Or, rather, force the open of a backing image if one was specified
>> for creation. Using a similar -unsafe option as rebase, allow qemu-img
>> to ignore the backing file validation if possible.
>>

>> +++ b/qemu-img.c
>> @@ -145,9 +145,11 @@ static void QEMU_NORETURN help(void)
>> "  'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
>> "instead\n"
>> "  '-c' indicates that target image must be compressed (qcow 
>> format only)\n"
>> -   "  '-u' enables unsafe rebasing. It is assumed that old and new 
>> backing file\n"
>> -   "   match exactly. The image doesn't need a working backing 
>> file before\n"
>> -   "   rebasing in this case (useful for renaming the backing 
>> file)\n"
>> +   "  '-u' allows unsafe backing chains. For rebasing, it is 
>> assumed that old and new\n"
>> +   "   backing file match exactly. The image doesn't need a 
>> working backing file\n"
>> +   "   before rebasing in this case (useful for renaming the 
>> backing file)\n"

Also, a '.' to end this sentence would be nice.

>> +   "   For image creation, allow creating without attempting to"
> 
> Missing \n
> 
> With that fixed,
> Reviewed-by: Eric Blake 
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] qemu-img: Check for backing image if specified during create

[Eric Blake]

On 05/09/2017 01:09 PM, John Snow wrote:
> Or, rather, force the open of a backing image if one was specified
> for creation. Using a similar -unsafe option as rebase, allow qemu-img
> to ignore the backing file validation if possible.
> 
> It may not always be possible, as in the existing case when a filesize
> for the new image was not specified.
> 
> This is accomplished by shifting around the conditionals in
> bdrv_img_create, such that a backing file is always opened unless we
> provide BDRV_O_NO_BACKING. qemu-img is adjusted to pass this new flag
> when -u is provided to create.
> 
> Sorry for the heinous looking diffstat, but it's mostly whitespace.
> 
> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1213786
> 
> Signed-off-by: John Snow 
> ---
> 
> v2: Rebased for 2.10
> Corrected some of my less cromulent grammar
> 

> +++ b/qemu-img.c
> @@ -145,9 +145,11 @@ static void QEMU_NORETURN help(void)
> "  'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
> "instead\n"
> "  '-c' indicates that target image must be compressed (qcow 
> format only)\n"
> -   "  '-u' enables unsafe rebasing. It is assumed that old and new 
> backing file\n"
> -   "   match exactly. The image doesn't need a working backing 
> file before\n"
> -   "   rebasing in this case (useful for renaming the backing 
> file)\n"
> +   "  '-u' allows unsafe backing chains. For rebasing, it is assumed 
> that old and new\n"
> +   "   backing file match exactly. The image doesn't need a 
> working backing file\n"
> +   "   before rebasing in this case (useful for renaming the 
> backing file)\n"
> +   "   For image creation, allow creating without attempting to"

Missing \n

With that fixed,
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] acpi-defs: clean up open brace usage

[Michael S. Tsirkin]

patchew has been saying:
ERROR: open brace '{' following struct go on the same line

Fix up acpi-defs.h to follow this rule.

Signed-off-by: Michael S. Tsirkin 
---
 include/hw/acpi/acpi-defs.h | 37 +
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index dff6d4f..7316a02 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -81,8 +81,8 @@ typedef struct AcpiRsdpDescriptor AcpiRsdpDescriptor;
 uint32_t asl_compiler_revision;  /* ASL compiler revision number */
 
 
-struct AcpiTableHeader /* ACPI common table header */
-{
+/* ACPI common table header */
+struct AcpiTableHeader {
 ACPI_TABLE_HEADER_DEF
 } QEMU_PACKED;
 typedef struct AcpiTableHeader AcpiTableHeader;
@@ -133,8 +133,7 @@ typedef struct AcpiTableHeader AcpiTableHeader;
 uint8_t  mon_alrm; /* Index to month-of-year alarm in RTC CMOS RAM */ \
 uint8_t  century;  /* Index to century in RTC CMOS RAM */
 
-struct AcpiFadtDescriptorRev1
-{
+struct AcpiFadtDescriptorRev1 {
 ACPI_FADT_COMMON_DEF
 uint8_t  reserved4;  /* Reserved */
 uint8_t  reserved4a; /* Reserved */
@@ -229,8 +228,7 @@ typedef struct AcpiSerialPortConsoleRedirection
 /*
  * ACPI 1.0 Root System Description Table (RSDT)
  */
-struct AcpiRsdtDescriptorRev1
-{
+struct AcpiRsdtDescriptorRev1 {
 ACPI_TABLE_HEADER_DEF   /* ACPI common table header */
 uint32_t table_offset_entry[0];  /* Array of pointers to other */
 /* ACPI tables */
@@ -240,8 +238,7 @@ typedef struct AcpiRsdtDescriptorRev1 
AcpiRsdtDescriptorRev1;
 /*
  * ACPI 2.0 eXtended System Description Table (XSDT)
  */
-struct AcpiXsdtDescriptorRev2
-{
+struct AcpiXsdtDescriptorRev2 {
 ACPI_TABLE_HEADER_DEF   /* ACPI common table header */
 uint64_t table_offset_entry[0];  /* Array of pointers to other */
 /* ACPI tables */
@@ -251,8 +248,7 @@ typedef struct AcpiXsdtDescriptorRev2 
AcpiXsdtDescriptorRev2;
 /*
  * ACPI 1.0 Firmware ACPI Control Structure (FACS)
  */
-struct AcpiFacsDescriptorRev1
-{
+struct AcpiFacsDescriptorRev1 {
 uint32_t signature;   /* ACPI Signature */
 uint32_t length; /* Length of structure, in bytes */
 uint32_t hardware_signature; /* Hardware configuration signature */
@@ -278,8 +274,7 @@ typedef struct AcpiFacsDescriptorRev1 
AcpiFacsDescriptorRev1;
 
 /* Master MADT */
 
-struct AcpiMultipleApicTable
-{
+struct AcpiMultipleApicTable {
 ACPI_TABLE_HEADER_DEF /* ACPI common table header */
 uint32_t local_apic_address; /* Physical address of local APIC */
 uint32_t flags;
@@ -315,8 +310,7 @@ typedef struct AcpiMultipleApicTable AcpiMultipleApicTable;
 
 /* Sub-structures for MADT */
 
-struct AcpiMadtProcessorApic
-{
+struct AcpiMadtProcessorApic {
 ACPI_SUB_HEADER_DEF
 uint8_t  processor_id;   /* ACPI processor id */
 uint8_t  local_apic_id;  /* Processor's local APIC id */
@@ -324,8 +318,7 @@ struct AcpiMadtProcessorApic
 } QEMU_PACKED;
 typedef struct AcpiMadtProcessorApic AcpiMadtProcessorApic;
 
-struct AcpiMadtIoApic
-{
+struct AcpiMadtIoApic {
 ACPI_SUB_HEADER_DEF
 uint8_t  io_apic_id; /* I/O APIC ID */
 uint8_t  reserved;   /* Reserved - must be zero */
@@ -478,8 +471,7 @@ typedef struct Acpi20Hpet Acpi20Hpet;
  * SRAT (NUMA topology description) table
  */
 
-struct AcpiSystemResourceAffinityTable
-{
+struct AcpiSystemResourceAffinityTable {
 ACPI_TABLE_HEADER_DEF
 uint32_treserved1;
 uint32_treserved2[2];
@@ -491,8 +483,7 @@ typedef struct AcpiSystemResourceAffinityTable 
AcpiSystemResourceAffinityTable;
 #define ACPI_SRAT_PROCESSOR_x2APIC   2
 #define ACPI_SRAT_PROCESSOR_GICC 3
 
-struct AcpiSratProcessorAffinity
-{
+struct AcpiSratProcessorAffinity {
 ACPI_SUB_HEADER_DEF
 uint8_t proximity_lo;
 uint8_t local_apic_id;
@@ -514,8 +505,7 @@ struct AcpiSratProcessorX2ApicAffinity {
 } QEMU_PACKED;
 typedef struct AcpiSratProcessorX2ApicAffinity AcpiSratProcessorX2ApicAffinity;
 
-struct AcpiSratMemoryAffinity
-{
+struct AcpiSratMemoryAffinity {
 ACPI_SUB_HEADER_DEF
 uint32_tproximity;
 uint16_treserved1;
@@ -527,8 +517,7 @@ struct AcpiSratMemoryAffinity
 } QEMU_PACKED;
 typedef struct AcpiSratMemoryAffinity AcpiSratMemoryAffinity;
 
-struct AcpiSratProcessorGiccAffinity
-{
+struct AcpiSratProcessorGiccAffinity {
 ACPI_SUB_HEADER_DEF
 uint32_tproximity;
 uint32_tacpi_processor_uid;
-- 
MST

[Qemu-devel] [PATCH v3 5/6] target/s390x: Use atomic operations for COMPARE SWAP

[Richard Henderson]

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 target/s390x/helper.h  |  1 +
 target/s390x/insn-data.def | 10 +++---
 target/s390x/mem_helper.c  | 39 ++
 target/s390x/translate.c   | 83 --
 4 files changed, 59 insertions(+), 74 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 01adb50..0b70770 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -25,6 +25,7 @@ DEF_HELPER_3(cxgb, i64, env, s64, i32)
 DEF_HELPER_3(celgb, i64, env, i64, i32)
 DEF_HELPER_3(cdlgb, i64, env, i64, i32)
 DEF_HELPER_3(cxlgb, i64, env, i64, i32)
+DEF_HELPER_4(cdsg, void, env, i64, i32, i32)
 DEF_HELPER_FLAGS_3(aeb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(adb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_5(axb, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 0909060..5e5fcc5 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -239,12 +239,12 @@
 D(0xec7d, CLGIJ,   RIE_c, GIE, r1_o, i2_8u, 0, 0, cj, 0, 1)
 
 /* COMPARE AND SWAP */
-D(0xba00, CS,  RS_a,  Z,   r3_32u, r1_32u, new, r1_32, cs, 0, 0)
-D(0xeb14, CSY, RSY_a, LD,  r3_32u, r1_32u, new, r1_32, cs, 0, 0)
-D(0xeb30, CSG, RSY_a, Z,   r3_o, r1_o, new, r1, cs, 0, 1)
+D(0xba00, CS,  RS_a,  Z,   r3_32u, r1_32u, new, r1_32, cs, 0, MO_TEUL)
+D(0xeb14, CSY, RSY_a, LD,  r3_32u, r1_32u, new, r1_32, cs, 0, MO_TEUL)
+D(0xeb30, CSG, RSY_a, Z,   r3_o, r1_o, new, r1, cs, 0, MO_TEQ)
 /* COMPARE DOUBLE AND SWAP */
-D(0xbb00, CDS, RS_a,  Z,   r3_D32, r1_D32, new, r1_D32, cs, 0, 1)
-D(0xeb31, CDSY,RSY_a, LD,  r3_D32, r1_D32, new, r1_D32, cs, 0, 1)
+D(0xbb00, CDS, RS_a,  Z,   r3_D32, r1_D32, new, r1_D32, cs, 0, MO_TEQ)
+D(0xeb31, CDSY,RSY_a, LD,  r3_D32, r1_D32, new, r1_D32, cs, 0, MO_TEQ)
 C(0xeb3e, CDSG,RSY_a, Z,   0, 0, 0, 0, cdsg, 0)
 
 /* COMPARE AND TRAP */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 675aba2..c74ded3 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -844,6 +844,45 @@ uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, 
uint64_t array,
 return cc;
 }
 
+void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
+  uint32_t r1, uint32_t r3)
+{
+uintptr_t ra = GETPC();
+Int128 cmpv = int128_make128(env->regs[r1 + 1], env->regs[r1]);
+Int128 newv = int128_make128(env->regs[r3 + 1], env->regs[r3]);
+int mem_idx = cpu_mmu_index(env, false);
+Int128 oldv;
+bool fail;
+
+if (parallel_cpus) {
+#ifndef CONFIG_ATOMIC128
+cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+#else
+TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
+oldv = helper_atomic_cmpxchgo_be_mmu(env, addr, cmpv, newv, oi, ra);
+fail = !int128_eq(oldv, cmpv);
+#endif
+} else {
+uint64_t oldh, oldl;
+
+oldh = cpu_ldq_data_ra(env, addr + 0, ra);
+oldl = cpu_ldq_data_ra(env, addr + 8, ra);
+
+oldv = int128_make128(oldl, oldh);
+fail = !int128_eq(oldv, cmpv);
+if (fail) {
+newv = oldv;
+}
+
+cpu_stq_data_ra(env, addr + 0, int128_gethi(newv), ra);
+cpu_stq_data_ra(env, addr + 8, int128_getlo(newv), ra);
+}
+
+env->cc_op = fail;
+env->regs[r1] = int128_gethi(oldv);
+env->regs[r1 + 1] = int128_getlo(oldv);
+}
+
 #if !defined(CONFIG_USER_ONLY)
 void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 8de0177..ec250bb 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1943,102 +1943,47 @@ static ExitStatus op_cps(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_cs(DisasContext *s, DisasOps *o)
 {
-/* FIXME: needs an atomic solution for CONFIG_USER_ONLY.  */
 int d2 = get_field(s->fields, d2);
 int b2 = get_field(s->fields, b2);
-int is_64 = s->insn->data;
-TCGv_i64 addr, mem, cc, z;
+TCGv_i64 addr, cc;
 
 /* Note that in1 = R3 (new value) and
in2 = (zero-extended) R1 (expected value).  */
 
-/* Load the memory into the (temporary) output.  While the PoO only talks
-   about moving the memory to R1 on inequality, if we include equality it
-   means that R1 is equal to the memory in all conditions.  */
 addr = get_address(s, 0, b2, d2);
-if (is_64) {
-tcg_gen_qemu_ld64(o->out, addr, get_mem_index(s));
-} else {
-tcg_gen_qemu_ld32u(o->out, addr, get_mem_index(s));
-}
+tcg_gen_atomic_cmpxchg_i64(o->out, addr, o->in2, o->in1,
+   get_mem_index(s), s->insn->data | MO_ALIGN);
+tcg_temp_free_i64(addr);
 
 /* Are the memory and expected values (un)equal?  Note that this setcond
   

Re: [Qemu-devel] [PULL 00/23] Trivial patches for 2017-05-07

[Eric Blake]

On 05/08/2017 12:30 PM, Stefan Hajnoczi wrote:
> On Sun, May 07, 2017 at 10:02:03AM +0300, Michael Tokarev wrote:
>> The following changes since commit 12a95f320a36ef66f724a49bb05e4fb553ac5dbe:
>>
>>   Merge remote-tracking branch 'kwolf/tags/for-upstream' into staging 
>> (2017-05-04 13:44:32 +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.corpit.ru/qemu.git tags/trivial-patches-fetch
>>
>> for you to fetch changes up to 311875781e549af91a3216d34c6ef40420bab435:
>>
>>   tests: Remove redundant assignment (2017-05-07 09:57:51 +0300)
>>
>> 
>> trivial patches for 2017-05-07
>>

>> sochin.jiang fix wrong parameter comments in channel-file.h (1):
>>   channel-file: fix wrong parameter comments

See my question on 11/23

> 
> Thanks, applied to my staging tree:
> https://github.com/stefanha/qemu/commits/staging

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2] qemu-img: Check for backing image if specified during create

[John Snow]

Or, rather, force the open of a backing image if one was specified
for creation. Using a similar -unsafe option as rebase, allow qemu-img
to ignore the backing file validation if possible.

It may not always be possible, as in the existing case when a filesize
for the new image was not specified.

This is accomplished by shifting around the conditionals in
bdrv_img_create, such that a backing file is always opened unless we
provide BDRV_O_NO_BACKING. qemu-img is adjusted to pass this new flag
when -u is provided to create.

Sorry for the heinous looking diffstat, but it's mostly whitespace.

Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1213786

Signed-off-by: John Snow 
---

v2: Rebased for 2.10
Corrected some of my less cromulent grammar


 block.c| 75 +++---
 qemu-img-cmds.hx   |  4 +--
 qemu-img.c | 16 ++
 tests/qemu-iotests/082 |  4 +--
 tests/qemu-iotests/082.out |  4 +--
 5 files changed, 55 insertions(+), 48 deletions(-)

diff --git a/block.c b/block.c
index 6c6bb3e..ca3e195 100644
--- a/block.c
+++ b/block.c
@@ -4284,38 +4284,38 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 // The size for the image must always be specified, with one exception:
 // If we are using a backing file, we can obtain the size from there
 size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
-if (size == -1) {
-if (backing_file) {
-BlockDriverState *bs;
-char *full_backing = g_new0(char, PATH_MAX);
-int64_t size;
-int back_flags;
-QDict *backing_options = NULL;
-
-bdrv_get_full_backing_filename_from_filename(filename, 
backing_file,
- full_backing, 
PATH_MAX,
- _err);
-if (local_err) {
-g_free(full_backing);
-goto out;
-}
-
-/* backing files always opened read-only */
-back_flags = flags;
-back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
-
-if (backing_fmt) {
-backing_options = qdict_new();
-qdict_put(backing_options, "driver",
-  qstring_from_str(backing_fmt));
-}
-
-bs = bdrv_open(full_backing, NULL, backing_options, back_flags,
-   _err);
+if (backing_file && !(flags & BDRV_O_NO_BACKING)) {
+BlockDriverState *bs;
+char *full_backing = g_new0(char, PATH_MAX);
+int back_flags;
+QDict *backing_options = NULL;
+
+bdrv_get_full_backing_filename_from_filename(filename, backing_file,
+ full_backing, PATH_MAX,
+ _err);
+if (local_err) {
 g_free(full_backing);
-if (!bs) {
-goto out;
-}
+goto out;
+}
+
+/* backing files always opened read-only */
+back_flags = flags;
+back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+
+if (backing_fmt) {
+backing_options = qdict_new();
+qdict_put(backing_options, "driver",
+  qstring_from_str(backing_fmt));
+}
+
+bs = bdrv_open(full_backing, NULL, backing_options, back_flags,
+   _err);
+g_free(full_backing);
+if (!bs) {
+goto out;
+}
+
+if (size == -1) {
 size = bdrv_getlength(bs);
 if (size < 0) {
 error_setg_errno(errp, -size, "Could not get size of '%s'",
@@ -4323,14 +4323,15 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 bdrv_unref(bs);
 goto out;
 }
-
 qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, _abort);
-
-bdrv_unref(bs);
-} else {
-error_setg(errp, "Image creation needs a size parameter");
-goto out;
 }
+
+bdrv_unref(bs);
+}
+
+if (size == -1) {
+error_setg(errp, "Image creation needs a size parameter");
+goto out;
 }
 
 if (!quiet) {
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index bf4ce59..1d230c6 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
 ETEXI
 
 DEF("create", img_create,
-"create [-q] [--object objectdef] [-f fmt] [-b backing_file] [-F 
backing_fmt] [-o options] filename [size]")
+"create [-q] [--object objectdef] [-f fmt] [-b backing_file] [-F 
backing_fmt] [-u] [-o options] filename [size]")
 STEXI
-@item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-b 
@var{backing_file}] [-F @var{backing_fmt}] [-o @var{options}] @var{filename} 
[@var{size}]
+@item 

[Qemu-devel] [PATCH v3 4/6] target/s390x: Implement LOAD PAIR DISJOINT

[Richard Henderson]

From: Eric Bischoff 

Reviewed-by: Aurelien Jarno 
Signed-off-by: Eric Bischoff 
Message-Id: <20170228120134.7921-1-ebisch...@suse.com>
[rth: Combine the two via insn->data; free the address temps.]
Signed-off-by: Richard Henderson 
---
 target/s390x/insn-data.def |  4 +++-
 target/s390x/translate.c   | 42 ++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 43c5707..0909060 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -504,7 +504,9 @@
 C(0xb9e2, LOCGR,   RRF_c, LOC, r1, r2, r1, 0, loc, 0)
 C(0xebf2, LOC, RSY_b, LOC, r1, m2_32u, new, r1_32, loc, 0)
 C(0xebe2, LOCG,RSY_b, LOC, r1, m2_64, r1, 0, loc, 0)
-/* LOAD PAIR DISJOINT TODO */
+/* LOAD PAIR DISJOINT */
+D(0xc804, LPD, SSF,   ILA, 0, 0, new_P, r3_P32, lpd, 0, MO_TEUL)
+D(0xc805, LPDG,SSF,   ILA, 0, 0, new_P, r3_P64, lpd, 0, MO_TEQ)
 /* LOAD POSITIVE */
 C(0x1000, LPR, RR_a,  Z,   0, r2_32s, new, r1_32, abs, abs32)
 C(0xb900, LPGR,RRE,   Z,   0, r2, r1, 0, abs, abs64)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 2b66a4e..8de0177 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2559,6 +2559,7 @@ static ExitStatus op_lctlg(DisasContext *s, DisasOps *o)
 tcg_temp_free_i32(r3);
 return NO_EXIT;
 }
+
 static ExitStatus op_lra(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
@@ -2759,6 +2760,31 @@ static ExitStatus op_lm64(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_lpd(DisasContext *s, DisasOps *o)
+{
+TCGv_i64 a1, a2;
+TCGMemOp mop = s->insn->data;
+
+/* In a parallel context, stop the world and single step.  */
+if (parallel_cpus) {
+potential_page_fault(s);
+gen_helper_exit_atomic(cpu_env);
+return EXIT_NORETURN;
+}
+
+/* In a serial context, perform the two loads ... */
+a1 = get_address(s, 0, get_field(s->fields, b1), get_field(s->fields, d1));
+a2 = get_address(s, 0, get_field(s->fields, b2), get_field(s->fields, d2));
+tcg_gen_qemu_ld_i64(o->out, a1, get_mem_index(s), mop | MO_ALIGN);
+tcg_gen_qemu_ld_i64(o->out2, a2, get_mem_index(s), mop | MO_ALIGN);
+tcg_temp_free_i64(a1);
+tcg_temp_free_i64(a2);
+
+/* ... and indicate that we performed them while interlocked.  */
+gen_op_movi_cc(s, 0);
+return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_lura(DisasContext *s, DisasOps *o)
 {
@@ -4430,6 +4456,22 @@ static void wout_r1_D32(DisasContext *s, DisasFields *f, 
DisasOps *o)
 }
 #define SPEC_wout_r1_D32 SPEC_r1_even
 
+static void wout_r3_P32(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+int r3 = get_field(f, r3);
+store_reg32_i64(r3, o->out);
+store_reg32_i64(r3 + 1, o->out2);
+}
+#define SPEC_wout_r3_P32 SPEC_r3_even
+
+static void wout_r3_P64(DisasContext *s, DisasFields *f, DisasOps *o)
+{
+int r3 = get_field(f, r3);
+store_reg(r3, o->out);
+store_reg(r3 + 1, o->out2);
+}
+#define SPEC_wout_r3_P64 SPEC_r3_even
+
 static void wout_e1(DisasContext *s, DisasFields *f, DisasOps *o)
 {
 store_freg32_i64(get_field(f, r1), o->out);
-- 
2.9.3

[Qemu-devel] [PATCH v3 2/6] target/s390x: Implement LOAD PROGRAM PARAMETER

[Richard Henderson]

From: Miroslav Benes 

Linux arch/s390/kernel/head(64).S uses LPP instruction if it is
available in facilities list provided by stfl/stfle instruction.
This is the case of newer z/System generations and their qemu
definition.

The description of LPP is at
http://www-01.ibm.com/support/docview.wss?uid=isg26fcd1cc32246f4c8852574ce0044734a

Reviewed-by: Aurelien Jarno 
Signed-off-by: Miroslav Benes 
Message-Id: <20170227085353.20787-1-mbe...@suse.cz>
Signed-off-by: Richard Henderson 
---
 target/s390x/insn-data.def | 2 ++
 target/s390x/translate.c   | 9 +
 2 files changed, 11 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index b6702da..43c5707 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -845,6 +845,8 @@
 /* LOAD CONTROL */
 C(0xb700, LCTL,RS_a,  Z,   0, a2, 0, 0, lctl, 0)
 C(0xeb2f, LCTLG,   RSY_a, Z,   0, a2, 0, 0, lctlg, 0)
+/* LOAD PROGRAM PARAMETER */
+C(0xb280, LPP, S,   LPP,   0, m2_64, 0, 0, lpp, 0)
 /* LOAD PSW */
 C(0x8200, LPSW,S, Z,   0, a2, 0, 0, lpsw, 0)
 /* LOAD PSW EXTENDED */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 69940e3..2b66a4e 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1194,6 +1194,7 @@ typedef enum DisasFacility {
 FAC_SCF,/* store clock fast */
 FAC_SFLE,   /* store facility list extended */
 FAC_ILA,/* interlocked access facility 1 */
+FAC_LPP,/* load-program-parameter */
 } DisasFacility;
 
 struct DisasInsn {
@@ -2567,6 +2568,14 @@ static ExitStatus op_lra(DisasContext *s, DisasOps *o)
 return NO_EXIT;
 }
 
+static ExitStatus op_lpp(DisasContext *s, DisasOps *o)
+{
+check_privileged(s);
+
+tcg_gen_st_i64(o->in2, cpu_env, offsetof(CPUS390XState, pp));
+return NO_EXIT;
+}
+
 static ExitStatus op_lpsw(DisasContext *s, DisasOps *o)
 {
 TCGv_i64 t1, t2;
-- 
2.9.3

[Qemu-devel] [PATCH v3 6/6] target/s390x: Use atomic operations for LOAD AND OP

[Richard Henderson]

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 target/s390x/insn-data.def | 20 ++--
 target/s390x/translate.c   | 78 +-
 2 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 5e5fcc5..55a7c52 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -390,20 +390,20 @@
 /* LOAD ADDRESS RELATIVE LONG */
 C(0xc000, LARL,RIL_b, Z,   0, ri2, 0, r1, mov2, 0)
 /* LOAD AND ADD */
-C(0xebf8, LAA, RSY_a, ILA, r3_32s, m2_32s_atomic, new, 
m2_32_r1_atomic, add, adds32)
-C(0xebe8, LAAG,RSY_a, ILA, r3, m2_64_atomic, new, m2_64_r1_atomic, 
add, adds64)
+D(0xebf8, LAA, RSY_a, ILA, r3_32s, a2, new, in2_r1_32, laa, adds32, 
MO_TESL)
+D(0xebe8, LAAG,RSY_a, ILA, r3, a2, new, in2_r1, laa, adds64, MO_TEQ)
 /* LOAD AND ADD LOGICAL */
-C(0xebfa, LAAL,RSY_a, ILA, r3_32s, m2_32s_atomic, new, 
m2_32_r1_atomic, add, addu32)
-C(0xebea, LAALG,   RSY_a, ILA, r3, m2_64_atomic, new, m2_64_r1_atomic, 
add, addu64)
+D(0xebfa, LAAL,RSY_a, ILA, r3_32u, a2, new, in2_r1_32, laa, addu32, 
MO_TEUL)
+D(0xebea, LAALG,   RSY_a, ILA, r3, a2, new, in2_r1, laa, addu64, MO_TEQ)
 /* LOAD AND AND */
-C(0xebf4, LAN, RSY_a, ILA, r3_32s, m2_32s_atomic, new, 
m2_32_r1_atomic, and, nz32)
-C(0xebe4, LANG,RSY_a, ILA, r3, m2_64_atomic, new, m2_64_r1_atomic, 
and, nz64)
+D(0xebf4, LAN, RSY_a, ILA, r3_32s, a2, new, in2_r1_32, lan, nz32, 
MO_TESL)
+D(0xebe4, LANG,RSY_a, ILA, r3, a2, new, in2_r1, lan, nz64, MO_TEQ)
 /* LOAD AND EXCLUSIVE OR */
-C(0xebf7, LAX, RSY_a, ILA, r3_32s, m2_32s_atomic, new, 
m2_32_r1_atomic, xor, nz32)
-C(0xebe7, LAXG,RSY_a, ILA, r3, m2_64_atomic, new, m2_64_r1_atomic, 
xor, nz64)
+D(0xebf7, LAX, RSY_a, ILA, r3_32s, a2, new, in2_r1_32, lax, nz32, 
MO_TESL)
+D(0xebe7, LAXG,RSY_a, ILA, r3, a2, new, in2_r1, lax, nz64, MO_TEQ)
 /* LOAD AND OR */
-C(0xebf6, LAO, RSY_a, ILA, r3_32s, m2_32s_atomic, new, 
m2_32_r1_atomic, or, nz32)
-C(0xebe6, LAOG,RSY_a, ILA, r3, m2_64_atomic, new, m2_64_r1_atomic, or, 
nz64)
+D(0xebf6, LAO, RSY_a, ILA, r3_32s, a2, new, in2_r1_32, lao, nz32, 
MO_TESL)
+D(0xebe6, LAOG,RSY_a, ILA, r3, a2, new, in2_r1, lao, nz64, MO_TEQ)
 /* LOAD AND TEST */
 C(0x1200, LTR, RR_a,  Z,   0, r2_o, 0, cond_r1r2_32, mov2, s32)
 C(0xb902, LTGR,RRE,   Z,   0, r2_o, 0, r1, mov2, s64)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index ec250bb..f738c7b 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2309,6 +2309,50 @@ static ExitStatus op_iske(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static ExitStatus op_laa(DisasContext *s, DisasOps *o)
+{
+/* The real output is indeed the original value in memory;
+   recompute the addition for the computation of CC.  */
+tcg_gen_atomic_fetch_add_i64(o->in2, o->in2, o->in1, get_mem_index(s),
+ s->insn->data | MO_ALIGN);
+/* However, we need to recompute the addition for setting CC.  */
+tcg_gen_add_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
+static ExitStatus op_lan(DisasContext *s, DisasOps *o)
+{
+/* The real output is indeed the original value in memory;
+   recompute the addition for the computation of CC.  */
+tcg_gen_atomic_fetch_and_i64(o->in2, o->in2, o->in1, get_mem_index(s),
+ s->insn->data | MO_ALIGN);
+/* However, we need to recompute the operation for setting CC.  */
+tcg_gen_and_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
+static ExitStatus op_lao(DisasContext *s, DisasOps *o)
+{
+/* The real output is indeed the original value in memory;
+   recompute the addition for the computation of CC.  */
+tcg_gen_atomic_fetch_or_i64(o->in2, o->in2, o->in1, get_mem_index(s),
+s->insn->data | MO_ALIGN);
+/* However, we need to recompute the operation for setting CC.  */
+tcg_gen_or_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
+static ExitStatus op_lax(DisasContext *s, DisasOps *o)
+{
+/* The real output is indeed the original value in memory;
+   recompute the addition for the computation of CC.  */
+tcg_gen_atomic_fetch_xor_i64(o->in2, o->in2, o->in1, get_mem_index(s),
+ s->insn->data | MO_ALIGN);
+/* However, we need to recompute the operation for setting CC.  */
+tcg_gen_xor_i64(o->out, o->in1, o->in2);
+return NO_EXIT;
+}
+
 static ExitStatus op_ldeb(DisasContext *s, DisasOps *o)
 {
 gen_helper_ldeb(o->out, cpu_env, o->in2);
@@ -4483,21 +4527,17 @@ static void wout_m2_32(DisasContext *s, DisasFields *f, 
DisasOps *o)
 }
 #define SPEC_wout_m2_32 0
 
-static void wout_m2_32_r1_atomic(DisasContext *s, DisasFields *f, DisasOps *o)
+static void 

[Qemu-devel] [PATCH v3 3/6] target/s390x: Diagnose specification exception for atomics

[Richard Henderson]

All of the interlocked access facility instructions raise a
specification exception for unaligned accesses.  Do this by
using the (previously unused) unaligned_access hook.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 target/s390x/cpu.c|  1 +
 target/s390x/cpu.h|  3 +++
 target/s390x/helper.c | 16 
 3 files changed, 20 insertions(+)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 066dcd1..a1bf2ba 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -430,6 +430,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 cc->write_elf64_note = s390_cpu_write_elf64_note;
 cc->cpu_exec_interrupt = s390_cpu_exec_interrupt;
 cc->debug_excp_handler = s390x_cpu_debug_excp_handler;
+cc->do_unaligned_access = s390x_cpu_do_unaligned_access;
 #endif
 cc->disas_set_info = s390_cpu_disas_set_info;
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 058ddad..bbed320 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -480,6 +480,9 @@ int s390_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, 
int rw,
 
 #ifndef CONFIG_USER_ONLY
 void do_restart_interrupt(CPUS390XState *env);
+void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+   MMUAccessType access_type,
+   int mmu_idx, uintptr_t retaddr);
 
 static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
uint8_t *ar)
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 68bd2f9..9978490 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -718,4 +718,20 @@ void s390x_cpu_debug_excp_handler(CPUState *cs)
 cpu_loop_exit_noexc(cs);
 }
 }
+
+/* Unaligned accesses are only diagnosed with MO_ALIGN.  At the moment,
+   this is only for the atomic operations, for which we want to raise a
+   specification exception.  */
+void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+   MMUAccessType access_type,
+   int mmu_idx, uintptr_t retaddr)
+{
+S390CPU *cpu = S390_CPU(cs);
+CPUS390XState *env = >env;
+
+if (retaddr) {
+cpu_restore_state(cs, retaddr);
+}
+program_interrupt(env, PGM_SPECIFICATION, ILEN_LATER);
+}
 #endif /* CONFIG_USER_ONLY */
-- 
2.9.3

[Qemu-devel] [PATCH v3 1/6] target/s390x: Implement STORE FACILITIES LIST EXTENDED

[Richard Henderson]

At the same time, improve STORE FACILITIES LIST
so that we don't hard-code the list for all cpus.

Signed-off-by: Richard Henderson 
---
 target/s390x/helper.h  |  2 ++
 target/s390x/insn-data.def |  2 ++
 target/s390x/misc_helper.c | 59 ++
 target/s390x/translate.c   | 17 ++---
 4 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 9102071..01adb50 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -83,6 +83,8 @@ DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, 
i32, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
 DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
+DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
+DEF_HELPER_2(stfle, i32, env, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 075ff59..b6702da 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -747,6 +747,8 @@
 C(0xe33e, STRV,RXY_a, Z,   la2, r1_32u, new, m1_32, rev32, 0)
 C(0xe32f, STRVG,   RXY_a, Z,   la2, r1_o, new, m1_64, rev64, 0)
 
+/* STORE FACILITY LIST EXTENDED */
+C(0xb2b0, STFLE,   S,  SFLE,   0, a2, 0, 0, stfle, 0)
 /* STORE FPC */
 C(0xb29c, STFPC,   S, Z,   0, a2, new, m2_32, efpc, 0)
 
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index eca8244..bd94242 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -678,3 +678,62 @@ void HELPER(per_ifetch)(CPUS390XState *env, uint64_t addr)
 }
 }
 #endif
+
+/* The maximum bit defined at the moment is 129.  */
+#define MAX_STFL_WORDS  3
+
+/* Canonicalize the current cpu's features into the 64-bit words required
+   by STFLE.  Return the index-1 of the max word that is non-zero.  */
+static unsigned do_stfle(CPUS390XState *env, uint64_t words[MAX_STFL_WORDS])
+{
+S390CPU *cpu = s390_env_get_cpu(env);
+const unsigned long *features = cpu->model->features;
+unsigned max_bit = 0;
+S390Feat feat;
+
+memset(words, 0, sizeof(uint64_t) * MAX_STFL_WORDS);
+
+if (test_bit(S390_FEAT_ZARCH, features)) {
+/* z/Architecture is always active if around */
+words[0] = 1ull << (63 - 2);
+}
+
+for (feat = find_first_bit(features, S390_FEAT_MAX);
+ feat < S390_FEAT_MAX;
+ feat = find_next_bit(features, S390_FEAT_MAX, feat + 1)) {
+const S390FeatDef *def = s390_feat_def(feat);
+if (def->type == S390_FEAT_TYPE_STFL) {
+unsigned bit = def->bit;
+if (bit > max_bit) {
+max_bit = bit;
+}
+assert(bit / 64 < MAX_STFL_WORDS);
+words[bit / 64] |= 1ULL << (63 - bit % 64);
+}
+}
+
+return max_bit / 64;
+}
+
+void HELPER(stfl)(CPUS390XState *env)
+{
+uint64_t words[MAX_STFL_WORDS];
+
+do_stfle(env, words);
+cpu_stl_data(env, 200, words[0] >> 32);
+}
+
+uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
+{
+uint64_t words[MAX_STFL_WORDS];
+unsigned count_m1 = env->regs[0] & 0xff;
+unsigned max_m1 = do_stfle(env, words);
+unsigned i;
+
+for (i = 0; i <= count_m1; ++i) {
+cpu_stq_data(env, addr + 8 * i, words[i]);
+}
+
+env->regs[0] = deposit64(env->regs[0], 0, 8, max_m1);
+return (count_m1 >= max_m1 ? 0 : 3);
+}
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 01c6217..69940e3 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3628,15 +3628,8 @@ static ExitStatus op_spt(DisasContext *s, DisasOps *o)
 
 static ExitStatus op_stfl(DisasContext *s, DisasOps *o)
 {
-TCGv_i64 f, a;
-/* We really ought to have more complete indication of facilities
-   that we implement.  Address this when STFLE is implemented.  */
 check_privileged(s);
-f = tcg_const_i64(0xc000);
-a = tcg_const_i64(200);
-tcg_gen_qemu_st32(f, a, get_mem_index(s));
-tcg_temp_free_i64(f);
-tcg_temp_free_i64(a);
+gen_helper_stfl(cpu_env);
 return NO_EXIT;
 }
 
@@ -3802,6 +3795,14 @@ static ExitStatus op_sturg(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static ExitStatus op_stfle(DisasContext *s, DisasOps *o)
+{
+potential_page_fault(s);
+gen_helper_stfle(cc_op, cpu_env, o->in2);
+set_cc_static(s);
+return NO_EXIT;
+}
+
 static ExitStatus op_st8(DisasContext *s, DisasOps *o)
 {
 tcg_gen_qemu_st8(o->in1, o->in2, get_mem_index(s));
-- 
2.9.3

[Qemu-devel] [PATCH v3 0/6] target/s390x patches

[Richard Henderson]

Changes since v2:
  * Fix STFLE patch as pointed out by Aurelien.
  * Adjust CS patch to change is_64 boolean in
s->insn->data to TCGMemOp.


r~


Eric Bischoff (1):
  target/s390x: Implement LOAD PAIR DISJOINT

Miroslav Benes (1):
  target/s390x: Implement LOAD PROGRAM PARAMETER

Richard Henderson (4):
  target/s390x: Implement STORE FACILITIES LIST EXTENDED
  target/s390x: Diagnose specification exception for atomics
  target/s390x: Use atomic operations for COMPARE SWAP
  target/s390x: Use atomic operations for LOAD AND OP

 target/s390x/cpu.c |   1 +
 target/s390x/cpu.h |   3 +
 target/s390x/helper.c  |  16 
 target/s390x/helper.h  |   3 +
 target/s390x/insn-data.def |  38 
 target/s390x/mem_helper.c  |  39 
 target/s390x/misc_helper.c |  59 
 target/s390x/translate.c   | 229 -
 8 files changed, 267 insertions(+), 121 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] [PULL 0/8] pci, virtio, vhost: fixes

[Michael S. Tsirkin]

On Tue, May 09, 2017 at 08:45:35AM -0700, no-re...@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Subject: [Qemu-devel] [PULL 0/8] pci, virtio, vhost: fixes
> Type: series
> Message-id: 1494330527-24163-1-git-send-email-...@redhat.com
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> 
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
> then
> failed=1
> echo
> fi
> n=$((n+1))
> done
> 
> exit $failed
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> Switched to a new branch 'test'
> 1aaf234 ACPI: don't call acpi_pcihp_device_plug_cb on xen
> 3b6e1b3 iommu: Don't crash if machine is not PC_MACHINE
> 5e3f027 pc: add 2.10 machine type
> a611b9d pc/fwcfg: unbreak migration from qemu-2.5 and qemu-2.6 during 
> firmware boot
> 2ece0f6 libvhost-user: fix crash when rings aren't ready
> c9d2f84 hw/virtio: fix vhost user fails to startup when MQ
> f24260b hw/arm/virt: generate 64-bit addressable ACPI objects
> 6f01b84 hw/acpi-defs: replace leading X with x_ in FADT field names
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/8: hw/acpi-defs: replace leading X with x_ in FADT field 
> names...
> Checking PATCH 2/8: hw/arm/virt: generate 64-bit addressable ACPI objects...
> ERROR: open brace '{' following struct go on the same line
> #157: FILE: include/hw/acpi/acpi-defs.h:244:
> +struct AcpiXsdtDescriptorRev2
> +{
> 
> total: 1 errors, 0 warnings, 125 lines checked

Yes but it's consistent with Rev1. So I'll keep it as is
and apply a tweak on top.

> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 3/8: hw/virtio: fix vhost user fails to startup when MQ...
> Checking PATCH 4/8: libvhost-user: fix crash when rings aren't ready...
> Checking PATCH 5/8: pc/fwcfg: unbreak migration from qemu-2.5 and qemu-2.6 
> during firmware boot...
> Checking PATCH 6/8: pc: add 2.10 machine type...
> Checking PATCH 7/8: iommu: Don't crash if machine is not PC_MACHINE...
> Checking PATCH 8/8: ACPI: don't call acpi_pcihp_device_plug_cb on xen...
> === 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] [PULL 11/23] channel-file: fix wrong parameter comments

[Eric Blake]

On 05/07/2017 02:02 AM, Michael Tokarev wrote:
> From: "sochin.jiang fix wrong parameter comments in channel-file.h" 
> 

Do we really want this HUGE email name, or can we get it touched up
before stefanha promotes his staging branch to master?

> 
> Signed-off-by: sochin.jiang 
> Reviewed-by: Marc-André Lureau 
> Signed-off-by: Michael Tokarev 
> ---
>  include/io/channel-file.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 


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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] ipxe for qemu maintainance

[Jeff Cody]

On Wed, Feb 22, 2017 at 02:42:34PM +0100, Gerd Hoffmann wrote:
>   Hi folks,
> 
> I'd like to start maintaining qemu-specific branches in our ipxe.git
> repo.  The reason for this are some problems with the upstream ipxe
> maintainance:
> 
>  * The ipxe maintainer apparently is pretty busy.  Often (but not
>always) it takes weeks or even months to get patches merged upstream,
>which is bad in case we need a fix included quickly due to freeze
>deadline approaching ...
>  * There is no release management whatsoever.  No stable branches,
>no release tags.  Picking up a fix upstream means rebasing to
>a snapshot which includes the fix.
> 
> So I'd like to improve that downstream with qemu branches, where we can
> commit not-yet merged patches, revert broken patches and cherry-pick
> bugfixes.
> 
> This is *NOT* meant to be a replacement for working with upstream to get
> patches merged and bugs fixed.  But it will allow us to handle things in
> a timely manner without having to depend on the upstream maintainer.
> And we can be more selective about the ipxe patches we accept during
> freeze.
> 
> I plan to also add qemu release tags to the repo, so you can easily
> figure what is included in each qemu release.
> 
> Laszlo created a wiki page for this, naming conventions for branches and
> tags are listed there too:
> http://wiki.qemu-project.org/IpxeDownstreamForQemu
> 
> Comments?
>

It looks like there were no objections, so I've gone ahead and put you and
Ladi Prosek in the ipxe.git committers.   Let me know if you have any
issues.

Thanks,
Jeff
 

Re: [Qemu-devel] [PATCH] mirror: add sync mode incremental to drive-mirror and blockdev-mirror

[Stefan Hajnoczi]

On Mon, May 08, 2017 at 05:07:18PM -0400, John Snow wrote:
> 
> 
> On 05/08/2017 05:02 PM, Denis V. Lunev wrote:
> > On 05/08/2017 10:35 PM, Stefan Hajnoczi wrote:
> >> On Thu, May 04, 2017 at 12:54:40PM +0200, Daniel Kucera wrote:
> >>
> >> Seems like a logical extension along the same lines as the backup block
> >> job's dirty bitmap sync mode.
> >>
> >>> parameter bitmap chooses existing dirtymap instead of newly created
> >>> in mirror_start_job
> >>>
> >>> Signed-off-by: Daniel Kucera 
> > Can you pls describe the use case pls in a bit more details.
> > 
> > For now this could be a bit strange:
> > - dirty bitmap, which can be found via bdrv_create_dirty_bitmap
> >   could be read-only or read-write, i.e. being modified by writes
> >   or be read-only, which should not be modified. Thus adding
> >   r/o bitmap to the mirror could result in interesting things.
> > 
> 
> This patch as it was submitted does not put the bitmap into a read-only
> mode; it leaves it RW and modifies it as it processes the mirror command.
> 
> Though you do raise a good point; this bitmap is now in-use by a job and
> should not be allowed to be deleted by the user, but our existing
> mechanism treats a locked bitmap as one that is also in R/O mode. This
> would be a different use case.
> 
> > Minimally we should prohibit usage of r/o bitmaps this way.
> > 
> > So, why to use mirror, not backup for the case?
> > 
> 
> My guess is for pivot semantics.

Daniel posted his workflow in a previous revision of this series:

He is doing a variation on non-shared storage migration with the mirror
block job, but using the ZFS send operation to transfer the initial copy
of the disk.

Once ZFS send completes it's necessary to transfer all the blocks that
were dirtied while the transfer was taking place.

1. Create dirty bitmap and start tracking dirty blocks in QEMU.
2. Snapshot and send ZFS volume.
3. mirror sync=bitmap after ZFS send completes.
4. Live migrate.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL v3 00/28] QAPI patches for 2017-05-04

[Stefan Hajnoczi]

On Tue, May 09, 2017 at 10:06:17AM +0200, Markus Armbruster wrote:
> [v3]: Rebase & resolve semantic conflicts, with help from Eric Blake
> [v2]: Fix trailing space, note tweaks to PATCH 12 properly in the
> commit message 
> 
> The following changes since commit dd1559bb267becbb838de41132ef60771d183e5d:
> 
>   Merge remote-tracking branch 'elmarco/tags/chr-tests-pull-request' into 
> staging (2017-05-05 17:07:55 +0100)
> 
> are available in the git repository at:
> 
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-05-04-v3
> 
> for you to fetch changes up to dcd3b25d656d346205dc0f2254723fccf0264e45:
> 
>   qmp-shell: improve help (2017-05-09 09:14:41 +0200)
> 
> 
> QAPI patches for 2017-05-04
> 
> 
> Dr. David Alan Gilbert (1):
>   tests/check-qdict: Fix missing brackets
> 
> Eric Blake (10):
>   pci: Use struct instead of QDict to pass back parameters
>   pci: Reduce scope of error injection
>   coccinelle: Add script to remove useless QObject casts
>   qobject: Drop useless QObject casts
>   qobject: Add helper macros for common scalar insertions
>   qobject: Use simpler QDict/QList scalar insertion macros
>   block: Simplify bdrv_append_temp_snapshot() logic
>   QemuOpts: Simplify qemu_opts_to_qdict()
>   fdc-test: Avoid deprecated 'change' command
>   test-qga: Actually test 0xff sync bytes
> 
> John Snow (1):
>   qmp-shell: add persistent command history
> 
> Marc-André Lureau (5):
>   test-keyval: fix leaks
>   qmp-shell: add -N option to skip negotiate
>   qmp-shell: Cope with query-commands error
>   qmp-shell: don't show version greeting if unavailable
>   qmp-shell: improve help
> 
> Markus Armbruster (11):
>   sockets: Prepare vsock_parse() for flattened SocketAddress
>   sockets: Prepare inet_parse() for flattened SocketAddress
>   qapi: New QAPI_CLONE_MEMBERS()
>   sockets: Rename SocketAddress to SocketAddressLegacy
>   sockets: Rename SocketAddressFlat to SocketAddress
>   sockets: Limit SocketAddressLegacy to external interfaces
>   sockets: Delete unused helper socket_address_crumple()
>   qmp: Improve QMP dispatch error messages
>   qobject-input-visitor: Document full_name_nth()
>   qapi: Document intended use of @name within alternate visits
>   qobject-input-visitor: Catch misuse of end_struct vs. end_list
> 
>  MAINTAINERS |   1 +
>  block.c |  70 ++
>  block/blkdebug.c|   8 +-
>  block/blkverify.c   |  11 +--
>  block/curl.c|   2 +-
>  block/file-posix.c  |   8 +-
>  block/file-win32.c  |   4 +-
>  block/gluster.c |  48 +-
>  block/nbd.c |  77 ---
>  block/nfs.c |  43 -
>  block/null.c|   2 +-
>  block/qcow2.c   |   4 +-
>  block/quorum.c  |  16 ++--
>  block/rbd.c |  16 ++--
>  block/sheepdog.c|  20 ++--
>  block/snapshot.c|   2 +-
>  block/ssh.c |  16 ++--
>  block/vvfat.c   |  10 +-
>  block/vxhs.c|   6 +-
>  blockdev-nbd.c  |  21 +++--
>  blockdev.c  |  30 +++---
>  chardev/char-socket.c   |  36 +++
>  chardev/char-udp.c  |  20 ++--
>  hmp.c   |   3 +-
>  hw/block/xen_disk.c |   2 +-
>  hw/pci/pcie_aer.c   |  48 ++
>  hw/usb/xen-usb.c|  12 +--
>  include/block/nbd.h |   3 +
>  include/hw/pci/pcie_aer.h   |   4 -
>  include/io/channel-socket.h |   4 +-
>  include/qapi/clone-visitor.h|  14 +++
>  include/qapi/qmp/qdict.h|   8 ++
>  include/qapi/qmp/qlist.h|   8 ++
>  include/qapi/visitor.h  |   6 +-
>  include/qemu/sockets.h  |  16 ++--
>  io/dns-resolver.c   |  17 ++--
>  migration/rdma.c|   4 +-
>  migration/socket.c  |  20 ++--
>  monitor.c   |  23 +++--
>  qapi-schema.json|  35 +++
>  qapi/block-core.json|   6 +-
>  qapi/block.json |   2 +-
>  qapi/qapi-clone-visitor.c   |  13 +++
>  qapi/qmp-dispatch.c |  14 +--
>  qapi/qmp-event.c|   2 +-
>  qapi/qobject-input-visitor.c|  32 ++-
>  qemu-img.c  |   6 +-
>  qemu-io.c   |   2 +-
>  qemu-nbd.c  |  11 +--
>  qga/main.c   

Re: [Qemu-devel] [PATCH v5 2/5] iotests: fix remainining tests to work with LUKS

[Eric Blake]

On 05/09/2017 12:33 PM, Daniel P. Berrange wrote:
> The tests 033, 140, 145 and 157 were all broken
> when run with LUKS, since they did not correctly use
> the required image opts args syntax to specify the
> decryption secret. Further, the 120 test simply does
> not make sense to run with luks, as the scenario
> exercised is not relevant.
> 
> The test 181 was broken when run with LUKS because
> it didn't take account of fact that $TEST_IMG was
> already in image opts syntax. The launch_qemu
> helper also didn't register the secret object
> providing the LUKS password.
> 
> Signed-off-by: Daniel P. Berrange 
> ---

> +++ b/tests/qemu-iotests/145
> @@ -43,8 +43,23 @@ _supported_proto generic
>  _supported_os Linux
>  
>  _make_test_img 1M
> -echo quit | $QEMU -nographic -hda "$TEST_IMG" -incoming 'exec:true' 
> -snapshot -serial none -monitor stdio |
> -_filter_qemu | _filter_hmp
> +
> +if test "$IMGOPTSSYNTAX" = "true"
> +then
> +SYSEMU_DRIVE_ARG=if=none,$TEST_IMG
> +SYSEMU_EXTRA_ARGS=""
> +if [ -n "$IMGKEYSECRET" ]; then
> + SECRET_ARG="secret,id=keysec0,data=$IMGKEYSECRET"
> + SYSEMU_EXTRA_ARGS="-object $SECRET_ARG"
> +fi
> +else
> +SYSEMU_DRIVE_ARG=if=none,file="$TEST_IMG",driver=$IMGFMT
> +SYSEMU_EXTRA_ARGS=""
> +fi
> +
> +echo quit | $QEMU -nographic $SYSEMU_EXTRA_ARGS  -drive $SYSEMU_DRIVE_ARG \

2 spaces before -drive looks a bit odd, but not a showstopper.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 14/17] acpi: fix s3/s4 disabled type

[Marc-André Lureau]

Use a more specific bool type.

Signed-off-by: Marc-André Lureau 
---
 hw/acpi/ich9.c   | 24 
 hw/acpi/piix4.c  |  8 
 hw/i386/acpi-build.c |  5 +++--
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 5c279bbaca..3bd8c4bcf5 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -359,9 +359,9 @@ static void ich9_pm_get_disable_s3(Object *obj, Visitor *v, 
const char *name,
void *opaque, Error **errp)
 {
 ICH9LPCPMRegs *pm = opaque;
-uint8_t value = pm->disable_s3;
+bool value = pm->disable_s3;
 
-visit_type_uint8(v, name, , errp);
+visit_type_bool(v, name, , errp);
 }
 
 static void ich9_pm_set_disable_s3(Object *obj, Visitor *v, const char *name,
@@ -369,9 +369,9 @@ static void ich9_pm_set_disable_s3(Object *obj, Visitor *v, 
const char *name,
 {
 ICH9LPCPMRegs *pm = opaque;
 Error *local_err = NULL;
-uint8_t value;
+bool value;
 
-visit_type_uint8(v, name, , _err);
+visit_type_bool(v, name, , _err);
 if (local_err) {
 goto out;
 }
@@ -384,9 +384,9 @@ static void ich9_pm_get_disable_s4(Object *obj, Visitor *v, 
const char *name,
void *opaque, Error **errp)
 {
 ICH9LPCPMRegs *pm = opaque;
-uint8_t value = pm->disable_s4;
+bool value = pm->disable_s4;
 
-visit_type_uint8(v, name, , errp);
+visit_type_bool(v, name, , errp);
 }
 
 static void ich9_pm_set_disable_s4(Object *obj, Visitor *v, const char *name,
@@ -394,9 +394,9 @@ static void ich9_pm_set_disable_s4(Object *obj, Visitor *v, 
const char *name,
 {
 ICH9LPCPMRegs *pm = opaque;
 Error *local_err = NULL;
-uint8_t value;
+bool value;
 
-visit_type_uint8(v, name, , _err);
+visit_type_bool(v, name, , _err);
 if (local_err) {
 goto out;
 }
@@ -447,8 +447,8 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm, 
Error **errp)
 static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
 pm->acpi_memory_hotplug.is_enabled = true;
 pm->cpu_hotplug_legacy = true;
-pm->disable_s3 = 0;
-pm->disable_s4 = 0;
+pm->disable_s3 = false;
+pm->disable_s4 = false;
 pm->s4_val = 2;
 
 object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
@@ -466,11 +466,11 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs 
*pm, Error **errp)
  ich9_pm_get_cpu_hotplug_legacy,
  ich9_pm_set_cpu_hotplug_legacy,
  NULL);
-object_property_add(obj, ACPI_PM_PROP_S3_DISABLED, "uint8",
+object_property_add(obj, ACPI_PM_PROP_S3_DISABLED, "bool",
 ich9_pm_get_disable_s3,
 ich9_pm_set_disable_s3,
 NULL, pm, NULL);
-object_property_add(obj, ACPI_PM_PROP_S4_DISABLED, "uint8",
+object_property_add(obj, ACPI_PM_PROP_S4_DISABLED, "bool",
 ich9_pm_get_disable_s4,
 ich9_pm_set_disable_s4,
 NULL, pm, NULL);
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index a553a7e110..74692336b5 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -83,8 +83,8 @@ typedef struct PIIX4PMState {
 AcpiPciHpState acpi_pci_hotplug;
 bool use_acpi_pci_hotplug;
 
-uint8_t disable_s3;
-uint8_t disable_s4;
+bool disable_s3;
+bool disable_s4;
 uint8_t s4_val;
 
 bool cpu_hotplug_legacy;
@@ -670,8 +670,8 @@ static void piix4_send_gpe(AcpiDeviceIf *adev, 
AcpiEventStatusBits ev)
 
 static Property piix4_pm_properties[] = {
 DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
-DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
-DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 0),
+DEFINE_PROP_BOOL(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 
false),
+DEFINE_PROP_BOOL(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 
false),
 DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
 DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState,
  use_acpi_pci_hotplug, true),
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1707fae9bf..27ad420914 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -22,6 +22,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qapi/qmp/types.h"
 #include "acpi-build.h"
 #include "qemu-common.h"
 #include "qemu/bitmap.h"
@@ -149,14 +150,14 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
 /* Fill in optional s3/s4 related properties */
 o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
 if (o) {
-pm->s3_disabled = qnum_get_uint(qobject_to_qnum(o), _abort);
+pm->s3_disabled = qbool_get_bool(qobject_to_qbool(o));
 } else {
 pm->s3_disabled = false;
 

[Qemu-devel] [PATCH 17/17] qobject: move dump_qobject() from block/ to qobject/

[Marc-André Lureau]

The dump functions could be generally useful for any qobject user or for
debugging etc.

Signed-off-by: Marc-André Lureau 
---
 include/qapi/qmp/qdict.h   |  2 ++
 include/qapi/qmp/qlist.h   |  2 ++
 include/qapi/qmp/qobject.h |  7 
 block/qapi.c   | 90 +++---
 qobject/qdict.c| 30 
 qobject/qlist.c| 23 
 qobject/qobject.c  | 19 ++
 tests/check-qjson.c| 19 ++
 8 files changed, 106 insertions(+), 86 deletions(-)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 8c7c2b762b..1ef3bc8cda 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -91,4 +91,6 @@ QObject *qdict_crumple(const QDict *src, Error **errp);
 
 void qdict_join(QDict *dest, QDict *src, bool overwrite);
 
+char *qdict_to_string(QDict *dict, int indent);
+
 #endif /* QDICT_H */
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 2f2c199632..105b16f074 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -59,6 +59,8 @@ size_t qlist_size(const QList *qlist);
 QList *qobject_to_qlist(const QObject *obj);
 void qlist_destroy_obj(QObject *obj);
 
+char *qlist_to_string(QList *list, int indent);
+
 static inline const QListEntry *qlist_first(const QList *qlist)
 {
 return QTAILQ_FIRST(>head);
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index b8ddbca405..0d6ae5048a 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -101,4 +101,11 @@ static inline QObject *qnull(void)
 return _;
 }
 
+char *qobject_to_string_indent(QObject *obj, int indent);
+
+static inline char *qobject_to_string(QObject *obj)
+{
+return qobject_to_string_indent(obj, 0);
+}
+
 #endif /* QOBJECT_H */
diff --git a/block/qapi.c b/block/qapi.c
index 2050df29e4..9b7d42e50a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -586,101 +586,19 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, 
void *f,
 }
 }
 
-static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
-   QDict *dict);
-static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
-   QList *list);
-
-static void dump_qobject(fprintf_function func_fprintf, void *f,
- int comp_indent, QObject *obj)
-{
-switch (qobject_type(obj)) {
-case QTYPE_QNUM: {
-QNum *value = qobject_to_qnum(obj);
-char *tmp = qnum_to_string(value);
-func_fprintf(f, "%s", tmp);
-g_free(tmp);
-break;
-}
-case QTYPE_QSTRING: {
-QString *value = qobject_to_qstring(obj);
-func_fprintf(f, "%s", qstring_get_str(value));
-break;
-}
-case QTYPE_QDICT: {
-QDict *value = qobject_to_qdict(obj);
-dump_qdict(func_fprintf, f, comp_indent, value);
-break;
-}
-case QTYPE_QLIST: {
-QList *value = qobject_to_qlist(obj);
-dump_qlist(func_fprintf, f, comp_indent, value);
-break;
-}
-case QTYPE_QBOOL: {
-QBool *value = qobject_to_qbool(obj);
-func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
-break;
-}
-default:
-abort();
-}
-}
-
-static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
-   QList *list)
-{
-const QListEntry *entry;
-int i = 0;
-
-for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i,
- composite ? '\n' : ' ');
-dump_qobject(func_fprintf, f, indentation + 1, entry->value);
-if (!composite) {
-func_fprintf(f, "\n");
-}
-}
-}
-
-static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
-   QDict *dict)
-{
-const QDictEntry *entry;
-
-for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-char *key = g_malloc(strlen(entry->key) + 1);
-int i;
-
-/* replace dashes with spaces in key (variable) names */
-for (i = 0; entry->key[i]; i++) {
-key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
-}
-key[i] = 0;
-func_fprintf(f, "%*s%s:%c", indentation * 4, "", key,
- composite ? '\n' : ' ');
-dump_qobject(func_fprintf, f, indentation + 1, entry->value);
-if (!composite) {
-func_fprintf(f, "\n");
-}
-

[Qemu-devel] [PATCH 10/17] object: add uint property setter/getter

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau 
---
 include/qom/object.h | 23 +++
 qom/object.c | 33 +
 2 files changed, 56 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index cd0f412ce9..abaeb8cf4e 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1094,6 +1094,29 @@ int64_t object_property_get_int(Object *obj, const char 
*name,
 Error **errp);
 
 /**
+ * object_property_set_uint:
+ * @value: the value to be written to the property
+ * @name: the name of the property
+ * @errp: returns an error if this function fails
+ *
+ * Writes an unsigned integer value to a property.
+ */
+void object_property_set_uint(Object *obj, uint64_t value,
+  const char *name, Error **errp);
+
+/**
+ * object_property_get_uint:
+ * @obj: the object
+ * @name: the name of the property
+ * @errp: returns an error if this function fails
+ *
+ * Returns: the value of the property, converted to an unsigned integer, or 0
+ * an error occurs (including when the property value is not an integer).
+ */
+uint64_t object_property_get_uint(Object *obj, const char *name,
+  Error **errp);
+
+/**
  * object_property_get_enum:
  * @obj: the object
  * @name: the name of the property
diff --git a/qom/object.c b/qom/object.c
index c1644dbcb7..a9259e330d 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1221,6 +1221,39 @@ int64_t object_property_get_int(Object *obj, const char 
*name,
 return retval;
 }
 
+void object_property_set_uint(Object *obj, uint64_t value,
+ const char *name, Error **errp)
+{
+QNum *qn = qnum_from_uint(value);
+object_property_set_qobject(obj, QOBJECT(qn), name, errp);
+QDECREF(qn);
+}
+
+uint64_t object_property_get_uint(Object *obj, const char *name,
+  Error **errp)
+{
+QObject *ret = object_property_get_qobject(obj, name, errp);
+Error *err = NULL;
+QNum *qnum;
+uint64_t retval;
+
+if (!ret) {
+return 0;
+}
+qnum = qobject_to_qnum(ret);
+if (qnum) {
+retval = qnum_get_uint(qnum, );
+}
+
+if (!qnum || err) {
+error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "uint");
+retval = 0;
+}
+
+qobject_decref(ret);
+return retval;
+}
+
 typedef struct EnumProperty {
 const char * const *strings;
 int (*get)(Object *, Error **);
-- 
2.13.0.rc1.16.gd80b50c3f

[Qemu-devel] [PATCH 16/17] RFC: qdict: add uint

[Marc-André Lureau]

Similar to int support, add uint support.

Note this is RFC because this is currently unused in qemu, I haven't
found a good user for it yet (kaslr qemu-ga code did use it though).

Signed-off-by: Marc-André Lureau 
---
 include/qapi/qmp/qdict.h |  5 +
 qobject/qdict.c  | 38 ++
 tests/check-qdict.c  | 32 
 3 files changed, 75 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 363e431106..8c7c2b762b 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -56,6 +56,8 @@ void qdict_destroy_obj(QObject *obj);
 /* Helpers for int, bool, and string */
 #define qdict_put_int(qdict, key, value) \
 qdict_put(qdict, key, qnum_from_int(value))
+#define qdict_put_uint(qdict, key, value)\
+qdict_put(qdict, key, qnum_from_uint(value))
 #define qdict_put_bool(qdict, key, value) \
 qdict_put(qdict, key, qbool_from_bool(value))
 #define qdict_put_str(qdict, key, value) \
@@ -64,12 +66,15 @@ void qdict_destroy_obj(QObject *obj);
 /* High level helpers */
 double qdict_get_double(const QDict *qdict, const char *key);
 int64_t qdict_get_int(const QDict *qdict, const char *key);
+uint64_t qdict_get_uint(const QDict *qdict, const char *key);
 bool qdict_get_bool(const QDict *qdict, const char *key);
 QList *qdict_get_qlist(const QDict *qdict, const char *key);
 QDict *qdict_get_qdict(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
   int64_t def_value);
+uint64_t qdict_get_try_uint(const QDict *qdict, const char *key,
+uint64_t def_value);
 bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
diff --git a/qobject/qdict.c b/qobject/qdict.c
index ad5bab9572..34478fbdd4 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -202,6 +202,19 @@ int64_t qdict_get_int(const QDict *qdict, const char *key)
 }
 
 /**
+ * qdict_get_uint(): Get an unsigned integer mapped by 'key'
+ *
+ * This function assumes that 'key' exists and it stores a
+ * QNum int object.
+ *
+ * Return unsigned integer mapped by 'key'.
+ */
+uint64_t qdict_get_uint(const QDict *qdict, const char *key)
+{
+return qnum_get_uint(qobject_to_qnum(qdict_get(qdict, key)), _abort);
+}
+
+/**
  * qdict_get_bool(): Get a bool mapped by 'key'
  *
  * This function assumes that 'key' exists and it stores a
@@ -270,6 +283,31 @@ int64_t qdict_get_try_int(const QDict *qdict, const char 
*key,
 }
 
 /**
+ * qdict_get_try_uint(): Try to get usigned integer mapped by 'key'
+ *
+ * Return unsigned integer mapped by 'key', if it is not present in
+ * the dictionary or if the stored object is not of QNum type
+ * 'def_value' will be returned.
+ */
+uint64_t qdict_get_try_uint(const QDict *qdict, const char *key,
+uint64_t def_value)
+{
+Error *err = NULL;
+QNum *qnum = qobject_to_qnum(qdict_get(qdict, key));
+uint64_t val = def_value;
+
+if (qnum) {
+val = qnum_get_uint(qnum, );
+}
+if (err) {
+error_free(err);
+val = def_value;
+}
+
+return val;
+}
+
+/**
  * qdict_get_try_bool(): Try to get a bool mapped by 'key'
  *
  * Return bool mapped by 'key', if it is not present in the
diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index bcd06a7d6b..7f9ea67862 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -105,6 +105,21 @@ static void qdict_get_int_test(void)
 QDECREF(tests_dict);
 }
 
+static void qdict_get_uint_test(void)
+{
+uint64_t ret;
+const uint64_t value = UINT64_MAX;
+const char *key = "uint";
+QDict *tests_dict = qdict_new();
+
+qdict_put_uint(tests_dict, key, value);
+
+ret = qdict_get_uint(tests_dict, key);
+g_assert(ret == value);
+
+QDECREF(tests_dict);
+}
+
 static void qdict_get_try_int_test(void)
 {
 int ret;
@@ -120,6 +135,21 @@ static void qdict_get_try_int_test(void)
 QDECREF(tests_dict);
 }
 
+static void qdict_get_try_uint_test(void)
+{
+uint64_t ret;
+const uint64_t value = UINT64_MAX;
+const char *key = "uint";
+QDict *tests_dict = qdict_new();
+
+qdict_put_uint(tests_dict, key, value);
+
+ret = qdict_get_try_uint(tests_dict, key, 0);
+g_assert(ret == value);
+
+QDECREF(tests_dict);
+}
+
 static void qdict_get_str_test(void)
 {
 const char *p;
@@ -852,7 +882,9 @@ int main(int argc, char **argv)
 /* Continue, but now with fixtures */
 g_test_add_func("/public/get", qdict_get_test);
 g_test_add_func("/public/get_int", qdict_get_int_test);
+g_test_add_func("/public/get_uint", qdict_get_uint_test);
 g_test_add_func("/public/get_try_int", qdict_get_try_int_test);
+g_test_add_func("/public/get_try_uint", 

[Qemu-devel] [PATCH 07/17] json: learn to parse uint64 numbers

[Marc-André Lureau]

Switch strtoll() usage to qemu_strtoi64() helper while at it.

Replace temporarily the error in qnum_get_int() with values >INT64_MAX
until the visitor is updated.

Add a few tests for large numbers.

Signed-off-by: Marc-André Lureau 
---
 qobject/json-lexer.c   |  4 
 qobject/json-parser.c  | 30 ++
 qobject/qnum.c |  4 ++--
 tests/check-qjson.c| 28 
 tests/check-qnum.c |  9 +
 tests/test-qobject-input-visitor.c |  7 ++-
 6 files changed, 63 insertions(+), 19 deletions(-)

diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c
index af4a75e05b..a0beb0b106 100644
--- a/qobject/json-lexer.c
+++ b/qobject/json-lexer.c
@@ -227,15 +227,18 @@ static const uint8_t json_lexer[][256] =  {
 /* escape */
 [IN_ESCAPE_LL] = {
 ['d'] = JSON_ESCAPE,
+['u'] = JSON_ESCAPE,
 },
 
 [IN_ESCAPE_L] = {
 ['d'] = JSON_ESCAPE,
+['u'] = JSON_ESCAPE,
 ['l'] = IN_ESCAPE_LL,
 },
 
 [IN_ESCAPE_I64] = {
 ['d'] = JSON_ESCAPE,
+['u'] = JSON_ESCAPE,
 },
 
 [IN_ESCAPE_I6] = {
@@ -247,6 +250,7 @@ static const uint8_t json_lexer[][256] =  {
 },
 
 [IN_ESCAPE] = {
+['u'] = JSON_ESCAPE,
 ['d'] = JSON_ESCAPE,
 ['i'] = JSON_ESCAPE,
 ['p'] = JSON_ESCAPE,
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index f431854ba1..fa15c762d3 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "qapi/qmp/types.h"
@@ -472,6 +473,13 @@ static QObject *parse_escape(JSONParserContext *ctxt, 
va_list *ap)
 } else if (!strcmp(token->str, "%lld") ||
!strcmp(token->str, "%I64d")) {
 return QOBJECT(qnum_from_int(va_arg(*ap, long long)));
+} else if (!strcmp(token->str, "%u")) {
+return QOBJECT(qnum_from_uint(va_arg(*ap, unsigned int)));
+} else if (!strcmp(token->str, "%lu")) {
+return QOBJECT(qnum_from_uint(va_arg(*ap, unsigned long)));
+} else if (!strcmp(token->str, "%llu") ||
+   !strcmp(token->str, "%I64u")) {
+return QOBJECT(qnum_from_uint(va_arg(*ap, unsigned long long)));
 } else if (!strcmp(token->str, "%s")) {
 return QOBJECT(qstring_from_str(va_arg(*ap, const char *)));
 } else if (!strcmp(token->str, "%f")) {
@@ -494,22 +502,28 @@ static QObject *parse_literal(JSONParserContext *ctxt)
 /* A possibility exists that this is a whole-valued float where the
  * fractional part was left out due to being 0 (.0). It's not a big
  * deal to treat these as ints in the parser, so long as users of the
- * resulting QObject know to expect a QNum in place of a QNum in
- * cases like these.
+ * resulting QObject know to expect a QNum that will handle
+ * implicit conversions to the expected type.
  *
- * However, in some cases these values will overflow/underflow a
- * QNum/int64 container, thus we should assume these are to be handled
- * as QNums/doubles rather than silently changing their values.
+ * However, in some cases these values will overflow/underflow
+ * a QNum/int64 container, thus we should assume these are to
+ * be handled as QNum/uint64 or QNums/doubles rather than
+ * silently changing their values.
  *
- * strtoll() indicates these instances by setting errno to ERANGE
+ * qemu_strto*() indicates these instances by setting errno to ERANGE
  */
 int64_t value;
+uint64_t uvalue;
 
-errno = 0; /* strtoll doesn't set errno on success */
-value = strtoll(token->str, NULL, 10);
+qemu_strtoi64(token->str, NULL, 10, );
 if (errno != ERANGE) {
 return QOBJECT(qnum_from_int(value));
 }
+
+qemu_strtou64(token->str, NULL, 10, );
+if (errno != ERANGE) {
+return QOBJECT(qnum_from_uint(uvalue));
+}
 /* fall through to JSON_FLOAT */
 }
 case JSON_FLOAT:
diff --git a/qobject/qnum.c b/qobject/qnum.c
index be6307accf..2f87952db8 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -76,8 +76,8 @@ int64_t qnum_get_int(const QNum *qn, Error **errp)
 return qn->u.i64;
 case QNUM_U64:
 if (qn->u.u64 > INT64_MAX) {
-error_setg(errp, "The number is too large, use qnum_get_uint()");
-return 0;
+/* temporarily accepts to cast to i64 until visitor is switched */
+error_report("The number is too large, use qnum_get_uint()");
 }
 return qn->u.u64;
 case QNUM_DOUBLE:
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index c432aebf13..57c2366dc3 100644
--- a/tests/check-qjson.c
+++ 

[Qemu-devel] [PATCH 11/17] object: use more specific property type names

[Marc-André Lureau]

Use the actual unsigned integer type name (this should't break since
property type aren't directly accessed from outside it seems).

Signed-off-by: Marc-André Lureau 
---
 backends/cryptodev.c |  2 +-
 hw/pci-host/piix.c   |  8 
 hw/pci-host/q35.c| 10 +-
 hw/ppc/pnv.c |  2 +-
 net/dump.c   |  2 +-
 net/filter-buffer.c  |  2 +-
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 832f056266..1764c179fe 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -222,7 +222,7 @@ cryptodev_backend_can_be_deleted(UserCreatable *uc, Error 
**errp)
 
 static void cryptodev_backend_instance_init(Object *obj)
 {
-object_property_add(obj, "queues", "int",
+object_property_add(obj, "queues", "uint32",
   cryptodev_backend_get_queues,
   cryptodev_backend_set_queues,
   NULL, NULL, NULL);
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9218aa952..9aed6225bf 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -279,19 +279,19 @@ static void i440fx_pcihost_initfn(Object *obj)
 memory_region_init_io(>data_mem, obj, _host_data_le_ops, s,
   "pci-conf-data", 4);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
 i440fx_pcihost_get_pci_hole_start,
 NULL, NULL, NULL, NULL);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_END, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_END, "uint32",
 i440fx_pcihost_get_pci_hole_end,
 NULL, NULL, NULL, NULL);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_START, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_START, "uint64",
 i440fx_pcihost_get_pci_hole64_start,
 NULL, NULL, NULL, NULL);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_END, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_END, "uint64",
 i440fx_pcihost_get_pci_hole64_end,
 NULL, NULL, NULL, NULL);
 }
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 344f77b10c..5438be8253 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -176,23 +176,23 @@ static void q35_host_initfn(Object *obj)
 qdev_prop_set_uint32(DEVICE(>mch), "addr", PCI_DEVFN(0, 0));
 qdev_prop_set_bit(DEVICE(>mch), "multifunction", false);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
 q35_host_get_pci_hole_start,
 NULL, NULL, NULL, NULL);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_END, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_END, "uint32",
 q35_host_get_pci_hole_end,
 NULL, NULL, NULL, NULL);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_START, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_START, "uint64",
 q35_host_get_pci_hole64_start,
 NULL, NULL, NULL, NULL);
 
-object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_END, "int",
+object_property_add(obj, PCI_HOST_PROP_PCI_HOLE64_END, "uint64",
 q35_host_get_pci_hole64_end,
 NULL, NULL, NULL, NULL);
 
-object_property_add(obj, PCIE_HOST_MCFG_SIZE, "int",
+object_property_add(obj, PCIE_HOST_MCFG_SIZE, "uint32",
 q35_host_get_mmcfg_size,
 NULL, NULL, NULL, NULL);
 
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d4bcdb027f..a964354081 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1110,7 +1110,7 @@ static void powernv_machine_initfn(Object *obj)
 
 static void powernv_machine_class_props_init(ObjectClass *oc)
 {
-object_class_property_add(oc, "num-chips", "uint32_t",
+object_class_property_add(oc, "num-chips", "uint32",
   pnv_get_num_chips, pnv_set_num_chips,
   NULL, NULL, NULL);
 object_class_property_set_description(oc, "num-chips",
diff --git a/net/dump.c b/net/dump.c
index 89a149b5dd..7f4d3fda52 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -325,7 +325,7 @@ static void filter_dump_instance_init(Object *obj)
 
 nfds->maxlen = 65536;
 
-object_property_add(obj, "maxlen", "int", filter_dump_get_maxlen,
+object_property_add(obj, "maxlen", "uint32", filter_dump_get_maxlen,
 filter_dump_set_maxlen, NULL, NULL, NULL);
 object_property_add_str(obj, "file", file_dump_get_filename,
 file_dump_set_filename, NULL);
diff --git a/net/filter-buffer.c b/net/filter-buffer.c

[Qemu-devel] [PATCH 12/17] qdev: use int and uint properties as appropriate

[Marc-André Lureau]

Use unsigned type for various properties.

Signed-off-by: Marc-André Lureau 
---
 include/hw/qdev-core.h   |  5 +++-
 include/hw/qdev-properties.h | 67 ++--
 hw/block/fdc.c   | 54 +--
 hw/core/qdev-properties.c|  8 +++---
 hw/core/qdev.c   | 24 ++--
 hw/net/e1000e.c  | 14 -
 6 files changed, 96 insertions(+), 76 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0f21a500cd..ac10458650 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -225,7 +225,10 @@ struct Property {
 PropertyInfo *info;
 ptrdiff_toffset;
 uint8_t  bitnr;
-int64_t  defval;
+union {
+int64_t i;
+uint64_t u;
+} defval;
 int  arrayoffset;
 PropertyInfo *arrayinfo;
 int  arrayfieldsize;
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 16d5d0629b..ca9c550fa3 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -37,36 +37,47 @@ extern PropertyInfo qdev_prop_arraylen;
 .offset= offsetof(_state, _field)\
 + type_check(_type, typeof_field(_state, _field)),   \
 }
-#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
+
+#define DEFINE_PROP_INT(_name, _state, _field, _defval, _prop, _type) { \
 .name  = (_name),   \
 .info  = &(_prop),  \
 .offset= offsetof(_state, _field)   \
 + type_check(_type,typeof_field(_state, _field)),   \
-.defval= (_type)_defval,\
+.defval.i  = (_type)_defval,\
 }
-#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
-.name  = (_name),\
-.info  = &(qdev_prop_bit),   \
-.bitnr= (_bit),  \
-.offset= offsetof(_state, _field)\
-+ type_check(uint32_t,typeof_field(_state, _field)), \
-.defval= (bool)_defval,  \
+
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
+.name  = (_name),   \
+.info  = &(qdev_prop_bit),  \
+.bitnr= (_bit), \
+.offset= offsetof(_state, _field)   \
++ type_check(uint32_t, typeof_field(_state, _field)),\
+.defval.u   = (bool)_defval,\
 }
+
+#define DEFINE_PROP_UINT(_name, _state, _field, _defval, _prop, _type) { \
+.name  = (_name),   \
+.info  = &(_prop),  \
+.offset= offsetof(_state, _field)   \
++ type_check(_type, typeof_field(_state, _field)),  \
+.defval.u  = (_type)_defval,\
+}
+
 #define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) {   \
 .name  = (_name),   \
 .info  = &(qdev_prop_bit64),\
 .bitnr= (_bit), \
 .offset= offsetof(_state, _field)   \
 + type_check(uint64_t, typeof_field(_state, _field)),   \
-.defval= (bool)_defval, \
+.defval.u  = (bool)_defval, \
 }
 
-#define DEFINE_PROP_BOOL(_name, _state, _field, _defval) {   \
-.name  = (_name),\
-.info  = &(qdev_prop_bool),  \
-.offset= offsetof(_state, _field)\
-+ type_check(bool, typeof_field(_state, _field)),\
-.defval= (bool)_defval,  \
+#define DEFINE_PROP_BOOL(_name, _state, _field, _defval) {  \
+.name  = (_name),   \
+.info  = &(qdev_prop_bool), \
+.offset= offsetof(_state, _field)   \
++ type_check(bool, typeof_field(_state, _field)),   \
+.defval.u= (bool)_defval,   \
 }
 
 #define PROP_ARRAY_LEN_PREFIX "len-"
@@ -107,19 +118,19 @@ extern PropertyInfo qdev_prop_arraylen;
 }
 
 #define 

[Qemu-devel] [PATCH 15/17] Use uint property getter/setter where appropriate

[Marc-André Lureau]

All those property usages are associated with unsigned integers, so use
appropriate getter/setter.

Signed-off-by: Marc-André Lureau 
---
 include/hw/isa/isa.h |  2 +-
 hw/acpi/memory_hotplug.c | 10 
 hw/acpi/nvdimm.c | 10 
 hw/acpi/pcihp.c  |  6 ++---
 hw/arm/aspeed.c  |  4 ++--
 hw/arm/bcm2835_peripherals.c |  9 
 hw/arm/raspi.c   |  4 ++--
 hw/core/platform-bus.c   |  2 +-
 hw/i386/acpi-build.c | 55 ++--
 hw/i386/pc.c |  6 ++---
 hw/i386/xen/xen-hvm.c|  6 ++---
 hw/intc/arm_gicv3_common.c   |  2 +-
 hw/mem/pc-dimm.c |  5 ++--
 hw/misc/auxbus.c |  2 +-
 hw/misc/pvpanic.c|  2 +-
 hw/ppc/pnv_core.c|  2 +-
 hw/ppc/spapr.c   |  8 ---
 numa.c   |  6 ++---
 target/i386/cpu.c|  4 ++--
 ui/console.c |  4 ++--
 20 files changed, 77 insertions(+), 72 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index c2fdd70cdc..95593408ef 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -29,7 +29,7 @@ static inline uint16_t applesmc_port(void)
 Object *obj = object_resolve_path_type("", TYPE_APPLE_SMC, NULL);
 
 if (obj) {
-return object_property_get_int(obj, APPLESMC_PROP_IO_BASE, NULL);
+return object_property_get_uint(obj, APPLESMC_PROP_IO_BASE, NULL);
 }
 return 0;
 }
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 210073d283..db3c89ceab 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -83,11 +83,12 @@ static uint64_t acpi_memory_hotplug_read(void *opaque, 
hwaddr addr,
 o = OBJECT(mdev->dimm);
 switch (addr) {
 case 0x0: /* Lo part of phys address where DIMM is mapped */
-val = o ? object_property_get_int(o, PC_DIMM_ADDR_PROP, NULL) : 0;
+val = o ? object_property_get_uint(o, PC_DIMM_ADDR_PROP, NULL) : 0;
 trace_mhp_acpi_read_addr_lo(mem_st->selector, val);
 break;
 case 0x4: /* Hi part of phys address where DIMM is mapped */
-val = o ? object_property_get_int(o, PC_DIMM_ADDR_PROP, NULL) >> 32 : 
0;
+val =
+o ? object_property_get_uint(o, PC_DIMM_ADDR_PROP, NULL) >> 32 : 0;
 trace_mhp_acpi_read_addr_hi(mem_st->selector, val);
 break;
 case 0x8: /* Lo part of DIMM size */
@@ -95,11 +96,12 @@ static uint64_t acpi_memory_hotplug_read(void *opaque, 
hwaddr addr,
 trace_mhp_acpi_read_size_lo(mem_st->selector, val);
 break;
 case 0xc: /* Hi part of DIMM size */
-val = o ? object_property_get_int(o, PC_DIMM_SIZE_PROP, NULL) >> 32 : 
0;
+val =
+o ? object_property_get_int(o, PC_DIMM_SIZE_PROP, NULL) >> 32 : 0;
 trace_mhp_acpi_read_size_hi(mem_st->selector, val);
 break;
 case 0x10: /* node proximity for _PXM method */
-val = o ? object_property_get_int(o, PC_DIMM_NODE_PROP, NULL) : 0;
+val = o ? object_property_get_uint(o, PC_DIMM_NODE_PROP, NULL) : 0;
 trace_mhp_acpi_read_pxm(mem_st->selector, val);
 break;
 case 0x14: /* pack and return is_* fields */
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 8e7d6ec034..e57027149d 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -236,14 +236,14 @@ static void
 nvdimm_build_structure_spa(GArray *structures, DeviceState *dev)
 {
 NvdimmNfitSpa *nfit_spa;
-uint64_t addr = object_property_get_int(OBJECT(dev), PC_DIMM_ADDR_PROP,
-NULL);
+uint64_t addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP,
+ NULL);
 uint64_t size = object_property_get_int(OBJECT(dev), PC_DIMM_SIZE_PROP,
 NULL);
-uint32_t node = object_property_get_int(OBJECT(dev), PC_DIMM_NODE_PROP,
-NULL);
+uint32_t node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP,
+ NULL);
 int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP,
-NULL);
+   NULL);
 
 nfit_spa = acpi_data_push(structures, sizeof(*nfit_spa));
 
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 3a531a4416..c420a388ea 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -62,10 +62,10 @@ typedef struct AcpiPciHpFind {
 static int acpi_pcihp_get_bsel(PCIBus *bus)
 {
 Error *local_err = NULL;
-int64_t bsel = object_property_get_int(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
-   _err);
+uint64_t bsel = object_property_get_uint(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
+ _err);
 
-if (local_err || bsel < 0 

[Qemu-devel] [PATCH 06/17] qnum: add uint type

[Marc-André Lureau]

In order to store integer values superior to INT64_MAX, add a u64
internal representation.

Signed-off-by: Marc-André Lureau 
---
 include/qapi/qmp/qnum.h |  4 
 qobject/qnum.c  | 49 +++
 tests/check-qnum.c  | 51 +
 3 files changed, 104 insertions(+)

diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index 0e51427821..89c14e040f 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -17,6 +17,7 @@
 
 typedef enum {
 QNUM_I64,
+QNUM_U64,
 QNUM_DOUBLE
 } QNumType;
 
@@ -25,14 +26,17 @@ typedef struct QNum {
 QNumType type;
 union {
 int64_t i64;
+uint64_t u64;
 double dbl;
 } u;
 } QNum;
 
 QNum *qnum_from_int(int64_t value);
+QNum *qnum_from_uint(uint64_t value);
 QNum *qnum_from_double(double value);
 
 int64_t qnum_get_int(const QNum *qi, Error **errp);
+uint64_t qnum_get_uint(const QNum *qi, Error **errp);
 double qnum_get_double(QNum *qn);
 
 char *qnum_to_string(QNum *qn);
diff --git a/qobject/qnum.c b/qobject/qnum.c
index 8e9dd38350..be6307accf 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qobject.h"
 #include "qemu-common.h"
@@ -34,6 +35,22 @@ QNum *qnum_from_int(int64_t value)
 }
 
 /**
+ * qnum_from_uint(): Create a new QNum from an uint64_t
+ *
+ * Return strong reference.
+ */
+QNum *qnum_from_uint(uint64_t value)
+{
+QNum *qn = g_new(QNum, 1);
+
+qobject_init(QOBJECT(qn), QTYPE_QNUM);
+qn->type = QNUM_U64;
+qn->u.u64 = value;
+
+return qn;
+}
+
+/**
  * qnum_from_double(): Create a new QNum from a double
  *
  * Return strong reference.
@@ -57,6 +74,34 @@ int64_t qnum_get_int(const QNum *qn, Error **errp)
 switch (qn->type) {
 case QNUM_I64:
 return qn->u.i64;
+case QNUM_U64:
+if (qn->u.u64 > INT64_MAX) {
+error_setg(errp, "The number is too large, use qnum_get_uint()");
+return 0;
+}
+return qn->u.u64;
+case QNUM_DOUBLE:
+error_setg(errp, "The number is a float");
+return 0;
+}
+
+g_assert_not_reached();
+}
+
+/**
+ * qnum_get_uint(): Get an unsigned integer from the number
+ */
+uint64_t qnum_get_uint(const QNum *qn, Error **errp)
+{
+switch (qn->type) {
+case QNUM_I64:
+if (qn->u.i64 < 0) {
+error_setg(errp, "The number is negative");
+return 0;
+}
+return qn->u.i64;
+case QNUM_U64:
+return qn->u.u64;
 case QNUM_DOUBLE:
 error_setg(errp, "The number is a float");
 return 0;
@@ -73,6 +118,8 @@ double qnum_get_double(QNum *qn)
 switch (qn->type) {
 case QNUM_I64:
 return qn->u.i64;
+case QNUM_U64:
+return qn->u.u64;
 case QNUM_DOUBLE:
 return qn->u.dbl;
 }
@@ -88,6 +135,8 @@ char *qnum_to_string(QNum *qn)
 switch (qn->type) {
 case QNUM_I64:
 return g_strdup_printf("%" PRId64, qn->u.i64);
+case QNUM_U64:
+return g_strdup_printf("%" PRIu64, qn->u.u64);
 case QNUM_DOUBLE:
 /* FIXME: snprintf() is locale dependent; but JSON requires
  * numbers to be formatted as if in the C locale. Dependence
diff --git a/tests/check-qnum.c b/tests/check-qnum.c
index d08d35e85a..8199546f99 100644
--- a/tests/check-qnum.c
+++ b/tests/check-qnum.c
@@ -36,6 +36,21 @@ static void qnum_from_int_test(void)
 g_free(qi);
 }
 
+static void qnum_from_uint_test(void)
+{
+QNum *qu;
+const int value = UINT_MAX;
+
+qu = qnum_from_int(value);
+g_assert(qu != NULL);
+g_assert(qu->u.u64 == value);
+g_assert(qu->base.refcnt == 1);
+g_assert(qobject_type(QOBJECT(qu)) == QTYPE_QNUM);
+
+// destroy doesn't exit yet
+g_free(qu);
+}
+
 static void qnum_from_double_test(void)
 {
 QNum *qf;
@@ -73,6 +88,37 @@ static void qnum_get_int_test(void)
 QDECREF(qi);
 }
 
+static void qnum_get_uint_test(void)
+{
+QNum *qn;
+const int value = 123456;
+Error *err = NULL;
+
+qn = qnum_from_uint(value);
+g_assert(qnum_get_uint(qn, _abort) == value);
+QDECREF(qn);
+
+qn = qnum_from_int(value);
+g_assert(qnum_get_uint(qn, _abort) == value);
+QDECREF(qn);
+
+qn = qnum_from_int(-1);
+qnum_get_uint(qn, );
+error_free_or_abort();
+QDECREF(qn);
+
+qn = qnum_from_uint(-1ULL);
+qnum_get_int(qn, );
+error_free_or_abort();
+QDECREF(qn);
+
+/* invalid case */
+qn = qnum_from_double(0.42);
+qnum_get_uint(qn, );
+error_free_or_abort();
+QDECREF(qn);
+}
+
 static void qobject_to_qnum_test(void)
 {
 QNum *qn;
@@ -111,6 +157,9 @@ static void qnum_destroy_test(void)
 qn = qnum_from_int(0);
 QDECREF(qn);
 
+qn = qnum_from_uint(0);
+QDECREF(qn);
+
 qn = 

[Qemu-devel] [PATCH 08/17] qapi: update the qobject visitor to use QUInt

[Marc-André Lureau]

Switch to use QNum/uint where appropriate to remove i64 limitation.

The input visitor will cast i64 input to u64 for compatibility
reasons (existing json QMP client already use negative i64 for large
u64, and expect an implicit cast in qemu).

Signed-off-by: Marc-André Lureau 
---
 qapi/qobject-input-visitor.c| 13 +++--
 qapi/qobject-output-visitor.c   |  3 +--
 tests/test-qobject-output-visitor.c | 21 -
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 785949ebab..72cefcf677 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -420,9 +420,9 @@ static void qobject_input_type_int64_keyval(Visitor *v, 
const char *name,
 static void qobject_input_type_uint64(Visitor *v, const char *name,
   uint64_t *obj, Error **errp)
 {
-/* FIXME: qobject_to_qnum mishandles values over INT64_MAX */
 QObjectInputVisitor *qiv = to_qiv(v);
 QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
+Error *err = NULL;
 QNum *qnum;
 
 if (!qobj) {
@@ -435,7 +435,16 @@ static void qobject_input_type_uint64(Visitor *v, const 
char *name,
 return;
 }
 
-*obj = qnum_get_int(qnum, errp);
+/* XXX: compatibility case, accept negative values as u64 */
+*obj = qnum_get_int(qnum, );
+
+if (err) {
+error_free(err);
+err = NULL;
+*obj = qnum_get_uint(qnum, );
+}
+
+error_propagate(errp, err);
 }
 
 static void qobject_input_type_uint64_keyval(Visitor *v, const char *name,
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index 2ca5093b22..70be84ccb5 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -150,9 +150,8 @@ static void qobject_output_type_int64(Visitor *v, const 
char *name,
 static void qobject_output_type_uint64(Visitor *v, const char *name,
uint64_t *obj, Error **errp)
 {
-/* FIXME values larger than INT64_MAX become negative */
 QObjectOutputVisitor *qov = to_qov(v);
-qobject_output_add(qov, name, qnum_from_int(*obj));
+qobject_output_add(qov, name, qnum_from_uint(*obj));
 }
 
 static void qobject_output_type_bool(Visitor *v, const char *name, bool *obj,
diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index 66a682d5a8..767818e393 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -595,15 +595,26 @@ static void check_native_list(QObject *qobj,
 qlist = qlist_copy(qobject_to_qlist(qdict_get(qdict, "data")));
 
 switch (kind) {
-case USER_DEF_NATIVE_LIST_UNION_KIND_S8:
-case USER_DEF_NATIVE_LIST_UNION_KIND_S16:
-case USER_DEF_NATIVE_LIST_UNION_KIND_S32:
-case USER_DEF_NATIVE_LIST_UNION_KIND_S64:
 case USER_DEF_NATIVE_LIST_UNION_KIND_U8:
 case USER_DEF_NATIVE_LIST_UNION_KIND_U16:
 case USER_DEF_NATIVE_LIST_UNION_KIND_U32:
 case USER_DEF_NATIVE_LIST_UNION_KIND_U64:
-/* all integer elements in JSON arrays get stored into QNums when
+for (i = 0; i < 32; i++) {
+QObject *tmp;
+QNum *qvalue;
+tmp = qlist_peek(qlist);
+g_assert(tmp);
+qvalue = qobject_to_qnum(tmp);
+g_assert_cmpuint(qnum_get_uint(qvalue, _abort), ==, i);
+qobject_decref(qlist_pop(qlist));
+}
+break;
+
+case USER_DEF_NATIVE_LIST_UNION_KIND_S8:
+case USER_DEF_NATIVE_LIST_UNION_KIND_S16:
+case USER_DEF_NATIVE_LIST_UNION_KIND_S32:
+case USER_DEF_NATIVE_LIST_UNION_KIND_S64:
+/* all integer elements in JSON arrays get stored into QInts when
  * we convert to QObjects, so we can check them all in the same
  * fashion, so simply fall through here
  */
-- 
2.13.0.rc1.16.gd80b50c3f

[Qemu-devel] [PATCH 09/17] qnum: fix get_int() with values > INT64_MAX

[Marc-André Lureau]

Now that the visitor has been switch to use qnum_uint, fix the bad
get_int() to use get_uint() instead. Remove compatibility code.

Signed-off-by: Marc-André Lureau 
---
 hw/i386/acpi-build.c | 2 +-
 qobject/qnum.c   | 4 ++--
 tests/check-qnum.c   | 9 -
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ec3ae7fa85..767da5d78e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2585,7 +2585,7 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
 if (!o) {
 return false;
 }
-mcfg->mcfg_base = qnum_get_int(qobject_to_qnum(o), _abort);
+mcfg->mcfg_base = qnum_get_uint(qobject_to_qnum(o), _abort);
 qobject_decref(o);
 
 o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
diff --git a/qobject/qnum.c b/qobject/qnum.c
index 2f87952db8..be6307accf 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -76,8 +76,8 @@ int64_t qnum_get_int(const QNum *qn, Error **errp)
 return qn->u.i64;
 case QNUM_U64:
 if (qn->u.u64 > INT64_MAX) {
-/* temporarily accepts to cast to i64 until visitor is switched */
-error_report("The number is too large, use qnum_get_uint()");
+error_setg(errp, "The number is too large, use qnum_get_uint()");
+return 0;
 }
 return qn->u.u64;
 case QNUM_DOUBLE:
diff --git a/tests/check-qnum.c b/tests/check-qnum.c
index 9a22af3d0e..8199546f99 100644
--- a/tests/check-qnum.c
+++ b/tests/check-qnum.c
@@ -107,11 +107,10 @@ static void qnum_get_uint_test(void)
 error_free_or_abort();
 QDECREF(qn);
 
-/* temporarily disabled until visitor is switched */
-/* qn = qnum_from_uint(-1ULL); */
-/* qnum_get_int(qn, ); */
-/* error_free_or_abort(); */
-/* QDECREF(qn); */
+qn = qnum_from_uint(-1ULL);
+qnum_get_int(qn, );
+error_free_or_abort();
+QDECREF(qn);
 
 /* invalid case */
 qn = qnum_from_double(0.42);
-- 
2.13.0.rc1.16.gd80b50c3f

[Qemu-devel] [PATCH 13/17] qdev: use appropriate getter/setters type

[Marc-André Lureau]

Based on underlying property type, use the appropriate getters/setters.

Signed-off-by: Marc-André Lureau 
---
 hw/i386/acpi-build.c  | 12 ++--
 hw/pci-host/gpex.c|  2 +-
 hw/pci-host/q35.c |  2 +-
 hw/pci-host/xilinx-pcie.c |  2 +-
 target/i386/cpu.c |  2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 767da5d78e..1707fae9bf 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -149,21 +149,21 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
 /* Fill in optional s3/s4 related properties */
 o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
 if (o) {
-pm->s3_disabled = qnum_get_int(qobject_to_qnum(o), _abort);
+pm->s3_disabled = qnum_get_uint(qobject_to_qnum(o), _abort);
 } else {
 pm->s3_disabled = false;
 }
 qobject_decref(o);
 o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_DISABLED, NULL);
 if (o) {
-pm->s4_disabled = qnum_get_int(qobject_to_qnum(o), _abort);
+pm->s4_disabled = qnum_get_uint(qobject_to_qnum(o), _abort);
 } else {
 pm->s4_disabled = false;
 }
 qobject_decref(o);
 o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_VAL, NULL);
 if (o) {
-pm->s4_val = qnum_get_int(qobject_to_qnum(o), _abort);
+pm->s4_val = qnum_get_uint(qobject_to_qnum(o), _abort);
 } else {
 pm->s4_val = false;
 }
@@ -499,7 +499,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, 
PCIBus *bus,
 
 bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL, 
NULL);
 if (bsel) {
-int64_t bsel_val = qnum_get_int(qobject_to_qnum(bsel), _abort);
+uint64_t bsel_val = qnum_get_uint(qobject_to_qnum(bsel), _abort);
 
 aml_append(parent_scope, aml_name_decl("BSEL", aml_int(bsel_val)));
 notify_method = aml_method("DVNT", 2, AML_NOTSERIALIZED);
@@ -609,7 +609,7 @@ static void build_append_pci_bus_devices(Aml *parent_scope, 
PCIBus *bus,
 
 /* If bus supports hotplug select it and notify about local events */
 if (bsel) {
-int64_t bsel_val = qnum_get_int(qobject_to_qnum(bsel), _abort);
+uint64_t bsel_val = qnum_get_uint(qobject_to_qnum(bsel), _abort);
 aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
 aml_append(method,
 aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check */)
@@ -2590,7 +2590,7 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
 
 o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
 assert(o);
-mcfg->mcfg_size = qnum_get_int(qobject_to_qnum(o), _abort);
+mcfg->mcfg_size = qnum_get_uint(qobject_to_qnum(o), _abort);
 qobject_decref(o);
 return true;
 }
diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 66055ee5cc..8d3a64008c 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -94,7 +94,7 @@ static void gpex_host_initfn(Object *obj)
 
 object_initialize(root, sizeof(*root), TYPE_GPEX_ROOT_DEVICE);
 object_property_add_child(obj, "gpex_root", OBJECT(root), NULL);
-qdev_prop_set_uint32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
+qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
 qdev_prop_set_bit(DEVICE(root), "multifunction", false);
 }
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 5438be8253..3cbe8cfcea 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -173,7 +173,7 @@ static void q35_host_initfn(Object *obj)
 
 object_initialize(>mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE);
 object_property_add_child(OBJECT(s), "mch", OBJECT(>mch), NULL);
-qdev_prop_set_uint32(DEVICE(>mch), "addr", PCI_DEVFN(0, 0));
+qdev_prop_set_int32(DEVICE(>mch), "addr", PCI_DEVFN(0, 0));
 qdev_prop_set_bit(DEVICE(>mch), "multifunction", false);
 
 object_property_add(obj, PCI_HOST_PROP_PCI_HOLE_START, "uint32",
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 8b71e2d950..461ed41151 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -150,7 +150,7 @@ static void xilinx_pcie_host_init(Object *obj)
 
 object_initialize(root, sizeof(*root), TYPE_XILINX_PCIE_ROOT);
 object_property_add_child(obj, "root", OBJECT(root), NULL);
-qdev_prop_set_uint32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
+qdev_prop_set_int32(DEVICE(root), "addr", PCI_DEVFN(0, 0));
 qdev_prop_set_bit(DEVICE(root), "multifunction", false);
 }
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1e8a5b55c0..5bb8131bb8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3191,7 +3191,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
   OBJECT(cpu->apic_state), _abort);
 object_unref(OBJECT(cpu->apic_state));
 
-qdev_prop_set_uint32(cpu->apic_state, "id", cpu->apic_id);
+

[Qemu-devel] [PATCH 00/17] qobject/qapi: add uint type

[Marc-André Lureau]

Hi,

In previously sent series "[PATCH 00/21] WIP: dump: add kaslr support
(for after 2.9)", I proposed changes to accept uint64 values from
json, by adding a QUint type. During review, it was suggested to
introduce a QNum type to hold various number representations.

This series introduces the QNum type, adds uint representation to it,
and make uint64 values over json possible (while keeping json negative
int for values >INT64_MAX to unsigned cast compatibility). It also
improves some int vs uint usage for object properties and other
QObject related clean-ups.

The series is on top of http://repo.or.cz/qemu/armbru.git qapi-next

Marc-André Lureau (17):
  qdev: remove PropertyInfo.qtype field
  object: fix potential leak in getters
  tests: remove alt num-int cases
  qapi: merge QInt and QFloat in QNum
  qapi: remove promote_int
  qnum: add uint type
  json: learn to parse uint64 numbers
  qapi: update the qobject visitor to use QUInt
  qnum: fix get_int() with values > INT64_MAX
  object: add uint property setter/getter
  object: use more specific property type names
  qdev: use int and uint properties as appropriate
  qdev: use appropriate getter/setters type
  acpi: fix s3/s4 disabled type
  Use uint property getter/setter where appropriate
  RFC: qdict: add uint
  qobject: move dump_qobject() from block/ to qobject/

 include/qapi/visitor.h   |   4 +-
 include/qapi/visitor-impl.h  |   2 +-
 scripts/qapi.py  |  30 ++---
 scripts/qapi-visit.py|  12 +-
 include/hw/isa/isa.h |   2 +-
 include/hw/qdev-core.h   |   6 +-
 include/hw/qdev-properties.h |  72 ++--
 include/qapi/qmp/qdict.h |  10 +-
 include/qapi/qmp/qfloat.h|  29 -
 include/qapi/qmp/qint.h  |  28 -
 include/qapi/qmp/qlist.h |   4 +-
 include/qapi/qmp/qnum.h  |  47 
 include/qapi/qmp/qobject.h   |   7 ++
 include/qapi/qmp/types.h |   3 +-
 include/qapi/qobject-input-visitor.h |   2 +-
 include/qapi/qobject-output-visitor.h|   8 +-
 include/qom/object.h |  23 
 qapi/qapi-visit-core.c   |   6 +-
 backends/cryptodev.c |   2 +-
 block/blkdebug.c |   1 -
 block/nbd.c  |   1 -
 block/nfs.c  |   1 -
 block/qapi.c |  93 +--
 block/quorum.c   |   1 -
 block/sheepdog.c |   1 -
 block/ssh.c  |   1 -
 block/vvfat.c|   1 -
 blockdev.c   |   5 +-
 hw/acpi/ich9.c   |  24 ++--
 hw/acpi/memory_hotplug.c |  10 +-
 hw/acpi/nvdimm.c |  10 +-
 hw/acpi/pcihp.c  |   7 +-
 hw/acpi/piix4.c  |   8 +-
 hw/arm/aspeed.c  |   4 +-
 hw/arm/bcm2835_peripherals.c |   9 +-
 hw/arm/raspi.c   |   4 +-
 hw/block/fdc.c   |  54 -
 hw/core/platform-bus.c   |   2 +-
 hw/core/qdev-properties.c|   8 +-
 hw/core/qdev.c   |  44 ++--
 hw/i386/acpi-build.c |  71 ++--
 hw/i386/pc.c |   6 +-
 hw/i386/xen/xen-hvm.c|   6 +-
 hw/intc/arm_gicv3_common.c   |   2 +-
 hw/mem/pc-dimm.c |   5 +-
 hw/misc/auxbus.c |   2 +-
 hw/misc/pvpanic.c|   2 +-
 hw/net/e1000e.c  |  14 +--
 hw/pci-host/gpex.c   |   2 +-
 hw/pci-host/piix.c   |   8 +-
 hw/pci-host/q35.c|  12 +-
 hw/pci-host/xilinx-pcie.c|   2 +-
 hw/ppc/pnv.c |   2 +-
 hw/ppc/pnv_core.c|   2 +-
 hw/ppc/spapr.c   |   8 +-
 hw/usb/xen-usb.c |   1 -
 monitor.c|   2 +-
 net/dump.c   |   2 +-
 net/filter-buffer.c  |   2 +-
 numa.c   |   6 +-
 qapi/qapi-clone-visitor.c|   2 +-
 qapi/qapi-dealloc-visitor.c  |   2 +-
 qapi/qobject-input-visitor.c |  47 
 qapi/qobject-output-visitor.c|   7 +-
 qga/commands.c   |   2 +-
 qga/main.c   |   1 -
 qobject/json-lexer.c |   4 +
 qobject/json-parser.c|  42 ---
 qobject/qdict.c  | 106 ++
 qobject/qfloat.c |  62 

[Qemu-devel] [PATCH 01/17] qdev: remove PropertyInfo.qtype field

[Marc-André Lureau]

Remove dependency on qapi qtype, replace a field by a few helper
functions to determine the default value type (introduced in commit
4f2d3d7).

Signed-off-by: Marc-André Lureau 
---
 include/hw/qdev-core.h   |  1 -
 include/hw/qdev-properties.h |  5 -
 hw/core/qdev.c   | 32 ++--
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 4bf86b0ad8..0f21a500cd 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -225,7 +225,6 @@ struct Property {
 PropertyInfo *info;
 ptrdiff_toffset;
 uint8_t  bitnr;
-QTypeqtype;
 int64_t  defval;
 int  arrayoffset;
 PropertyInfo *arrayinfo;
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 1d69fa7a8f..16d5d0629b 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -42,7 +42,6 @@ extern PropertyInfo qdev_prop_arraylen;
 .info  = &(_prop),  \
 .offset= offsetof(_state, _field)   \
 + type_check(_type,typeof_field(_state, _field)),   \
-.qtype = QTYPE_QINT,\
 .defval= (_type)_defval,\
 }
 #define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
@@ -51,7 +50,6 @@ extern PropertyInfo qdev_prop_arraylen;
 .bitnr= (_bit),  \
 .offset= offsetof(_state, _field)\
 + type_check(uint32_t,typeof_field(_state, _field)), \
-.qtype = QTYPE_QBOOL,\
 .defval= (bool)_defval,  \
 }
 #define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) {   \
@@ -60,7 +58,6 @@ extern PropertyInfo qdev_prop_arraylen;
 .bitnr= (_bit), \
 .offset= offsetof(_state, _field)   \
 + type_check(uint64_t, typeof_field(_state, _field)),   \
-.qtype = QTYPE_QBOOL,   \
 .defval= (bool)_defval, \
 }
 
@@ -69,7 +66,6 @@ extern PropertyInfo qdev_prop_arraylen;
 .info  = &(qdev_prop_bool),  \
 .offset= offsetof(_state, _field)\
 + type_check(bool, typeof_field(_state, _field)),\
-.qtype = QTYPE_QBOOL,\
 .defval= (bool)_defval,  \
 }
 
@@ -105,7 +101,6 @@ extern PropertyInfo qdev_prop_arraylen;
 .info = &(qdev_prop_arraylen),  \
 .offset = offsetof(_state, _field)  \
 + type_check(uint32_t, typeof_field(_state, _field)),   \
-.qtype = QTYPE_QINT,\
 .arrayinfo = &(_arrayprop), \
 .arrayfieldsize = sizeof(_arraytype),   \
 .arrayoffset = offsetof(_state, _arrayfield),   \
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 02b632f6b3..83b0297755 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -755,6 +755,30 @@ static void qdev_property_add_legacy(DeviceState *dev, 
Property *prop,
 g_free(name);
 }
 
+static bool prop_info_is_bool(const PropertyInfo *info)
+{
+return info == _prop_bit
+|| info == _prop_bit64
+|| info == _prop_bool;
+}
+
+static bool prop_info_is_int(const PropertyInfo *info)
+{
+return info == _prop_uint8
+|| info == _prop_uint16
+|| info == _prop_uint32
+|| info == _prop_int32
+|| info == _prop_uint64
+|| info == _prop_size
+|| info == _prop_pci_devfn
+|| info == _prop_on_off_auto
+|| info == _prop_losttickpolicy
+|| info == _prop_blockdev_on_error
+|| info == _prop_bios_chs_trans
+|| info == _prop_blocksize
+|| info == _prop_arraylen;
+}
+
 /**
  * qdev_property_add_static:
  * @dev: Device to add the property to.
@@ -794,16 +818,12 @@ void qdev_property_add_static(DeviceState *dev, Property 
*prop,
 prop->info->description,
 _abort);
 
-if (prop->qtype == QTYPE_NONE) {
-return;
-}
-
-if (prop->qtype == QTYPE_QBOOL) {
+if (prop_info_is_bool(prop->info)) {
 object_property_set_bool(obj, prop->defval, prop->name, _abort);
 } else if (prop->info->enum_table) {
 object_property_set_str(obj, prop->info->enum_table[prop->defval],
 prop->name, 

[Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum

[Marc-André Lureau]

We would like to use a same QObject type to represent numbers, whether
they are int, uint, or floats. getters will allow some compatibility
between the various types if the number fits other representations

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi.py  |  30 +++
 scripts/qapi-visit.py|   2 +-
 include/qapi/qmp/qdict.h |   3 +-
 include/qapi/qmp/qfloat.h|  29 ---
 include/qapi/qmp/qint.h  |  28 ---
 include/qapi/qmp/qlist.h |   2 +-
 include/qapi/qmp/qnum.h  |  43 ++
 include/qapi/qmp/types.h |   3 +-
 include/qapi/qobject-input-visitor.h |   2 +-
 include/qapi/qobject-output-visitor.h|   8 +-
 block/blkdebug.c |   1 -
 block/nbd.c  |   1 -
 block/nfs.c  |   1 -
 block/qapi.c |  13 ++-
 block/quorum.c   |   1 -
 block/sheepdog.c |   1 -
 block/ssh.c  |   1 -
 block/vvfat.c|   1 -
 blockdev.c   |   5 +-
 hw/acpi/pcihp.c  |   1 -
 hw/i386/acpi-build.c |  15 ++--
 hw/usb/xen-usb.c |   1 -
 monitor.c|   2 +-
 qapi/qobject-input-visitor.c |  36 +++-
 qapi/qobject-output-visitor.c|   6 +-
 qga/commands.c   |   2 +-
 qga/main.c   |   1 -
 qobject/json-parser.c|  18 ++--
 qobject/qdict.c  |  38 -
 qobject/qfloat.c |  62 --
 qobject/qint.c   |  61 --
 qobject/qjson.c  |  37 +
 qobject/qnum.c   | 138 +++
 qobject/qobject.c|   3 +-
 qom/object.c |  21 +++--
 target/i386/cpu.c|   6 +-
 tests/check-qdict.c  |  23 +++---
 tests/check-qfloat.c |  53 
 tests/check-qint.c   |  87 ---
 tests/check-qjson.c  |  84 +--
 tests/check-qlist.c  |  15 ++--
 tests/check-qnum.c   | 131 +
 tests/test-keyval.c  |   1 -
 tests/test-qmp-commands.c|   6 +-
 tests/test-qmp-event.c   |   7 +-
 tests/test-qobject-input-visitor.c   |  30 +++
 tests/test-qobject-output-visitor.c  |  56 ++---
 tests/test-x86-cpuid-compat.c|  13 ++-
 ui/spice-core.c  |   1 -
 ui/vnc-enc-tight.c   |   1 -
 util/qemu-option.c   |  20 ++---
 MAINTAINERS  |   3 +-
 qobject/Makefile.objs|   2 +-
 scripts/coccinelle/qobject.cocci |   4 +-
 tests/.gitignore |   3 +-
 tests/Makefile.include   |  13 ++-
 tests/qapi-schema/comments.out   |   2 +-
 tests/qapi-schema/doc-good.out   |   2 +-
 tests/qapi-schema/empty.out  |   2 +-
 tests/qapi-schema/event-case.out |   2 +-
 tests/qapi-schema/ident-with-escape.out  |   2 +-
 tests/qapi-schema/include-relpath.out|   2 +-
 tests/qapi-schema/include-repetition.out |   2 +-
 tests/qapi-schema/include-simple.out |   2 +-
 tests/qapi-schema/indented-expr.out  |   2 +-
 tests/qapi-schema/qapi-schema-test.out   |   2 +-
 66 files changed, 557 insertions(+), 639 deletions(-)
 delete mode 100644 include/qapi/qmp/qfloat.h
 delete mode 100644 include/qapi/qmp/qint.h
 create mode 100644 include/qapi/qmp/qnum.h
 delete mode 100644 qobject/qfloat.c
 delete mode 100644 qobject/qint.c
 create mode 100644 qobject/qnum.c
 delete mode 100644 tests/check-qfloat.c
 delete mode 100644 tests/check-qint.c
 create mode 100644 tests/check-qnum.c

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6c4d554165..01fd0027a5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -21,18 +21,18 @@ from ordereddict import OrderedDict
 
 builtin_types = {
 'str':  'QTYPE_QSTRING',
-'int':  'QTYPE_QINT',
-'number':   'QTYPE_QFLOAT',
+'int':  'QTYPE_QNUM',
+'number':   'QTYPE_QNUM',
 'bool': 'QTYPE_QBOOL',
-'int8': 'QTYPE_QINT',
-'int16':'QTYPE_QINT',
-'int32':'QTYPE_QINT',
-'int64':'QTYPE_QINT',
-'uint8':'QTYPE_QINT',
-'uint16':   'QTYPE_QINT',
-'uint32':   'QTYPE_QINT',
-'uint64':   'QTYPE_QINT',
-'size': 'QTYPE_QINT',
+'int8': 'QTYPE_QNUM',
+'int16':'QTYPE_QNUM',
+'int32':   

[Qemu-devel] [PATCH 05/17] qapi: remove promote_int

[Marc-André Lureau]

qnum_to_double() implicity promotes int now.

Signed-off-by: Marc-André Lureau 
---
 include/qapi/visitor.h |  4 +---
 include/qapi/visitor-impl.h|  2 +-
 scripts/qapi-visit.py  | 12 +++-
 qapi/qapi-visit-core.c |  6 +++---
 qapi/qapi-clone-visitor.c  |  2 +-
 qapi/qapi-dealloc-visitor.c|  2 +-
 qapi/qobject-input-visitor.c   |  2 +-
 tests/test-qobject-input-visitor.c |  2 +-
 qapi/trace-events  |  2 +-
 9 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index b0e233df76..0caed24942 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -410,15 +410,13 @@ void visit_end_list(Visitor *v, void **list);
  * the qtype of the next thing to be visited, stored in (*@obj)->type.
  * Other visitors will leave @obj unchanged.
  *
- * If @promote_int, treat integers as QTYPE_FLOAT.
- *
  * If successful, this must be paired with visit_end_alternate() with
  * the same @obj to clean up, even if visiting the contents of the
  * alternate fails.
  */
 void visit_start_alternate(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
-   bool promote_int, Error **errp);
+   Error **errp);
 
 /*
  * Finish visiting an alternate type.
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index e87709db5c..dcd656ab76 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -71,7 +71,7 @@ struct Visitor
  * optional for output visitors. */
 void (*start_alternate)(Visitor *v, const char *name,
 GenericAlternate **obj, size_t size,
-bool promote_int, Error **errp);
+Error **errp);
 
 /* Optional, needed for dealloc visitor */
 void (*end_alternate)(Visitor *v, void **obj);
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index cc447ecacc..bd0b742236 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -161,20 +161,14 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, 
%(c_name)s *obj, Error
 
 
 def gen_visit_alternate(name, variants):
-promote_int = 'true'
-ret = ''
-for var in variants.variants:
-if var.type.alternate_qtype() == 'QTYPE_QNUM':
-promote_int = 'false'
-
-ret += mcgen('''
+ret = mcgen('''
 
 void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, 
Error **errp)
 {
 Error *err = NULL;
 
 visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj),
-  %(promote_int)s, );
+  );
 if (err) {
 goto out;
 }
@@ -183,7 +177,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, 
%(c_name)s **obj, Error
 }
 switch ((*obj)->type) {
 ''',
- c_name=c_name(name), promote_int=promote_int)
+ c_name=c_name(name))
 
 for var in variants.variants:
 ret += mcgen('''
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 43a09d147d..935a2c5bc9 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -106,15 +106,15 @@ void visit_end_list(Visitor *v, void **obj)
 
 void visit_start_alternate(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
-   bool promote_int, Error **errp)
+   Error **errp)
 {
 Error *err = NULL;
 
 assert(obj && size >= sizeof(GenericAlternate));
 assert(!(v->type & VISITOR_OUTPUT) || *obj);
-trace_visit_start_alternate(v, name, obj, size, promote_int);
+trace_visit_start_alternate(v, name, obj, size);
 if (v->start_alternate) {
-v->start_alternate(v, name, obj, size, promote_int, );
+v->start_alternate(v, name, obj, size, );
 }
 if (v->type & VISITOR_INPUT) {
 assert(v->start_alternate && !err != !*obj);
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index de756bfb33..ed16d3a17f 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -70,7 +70,7 @@ static GenericList *qapi_clone_next_list(Visitor *v, 
GenericList *tail,
 
 static void qapi_clone_start_alternate(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
-   bool promote_int, Error **errp)
+   Error **errp)
 {
 qapi_clone_start_struct(v, name, (void **)obj, size, errp);
 }
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index e39457bc79..fd6f9fb61c 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -38,7 +38,7 @@ static void qapi_dealloc_end_struct(Visitor *v, void **obj)
 
 static void qapi_dealloc_start_alternate(Visitor *v, 

[Qemu-devel] [PATCH v5 5/5] iotests: chown LUKS device before qemu-io launches

[Daniel P. Berrange]

On some distros, whenever you close a block device file
descriptor there is a udev rule that resets the file
permissions. This can race with the test script when
we run qemu-io multiple times against the same block
device. Occasionally the second qemu-io invocation
will find udev has reset the permissions causing failure.

Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/149 |  13 +-
 tests/qemu-iotests/149.out | 344 ++---
 2 files changed, 178 insertions(+), 179 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 5faf585..bc628ce 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -23,6 +23,7 @@
 import subprocess
 import os
 import os.path
+import time
 
 import base64
 
@@ -186,7 +187,7 @@ def chown(config):
 msg = proc.communicate()[0]
 
 if proc.returncode != 0:
-raise Exception("Cannot change owner on %s" % path)
+raise Exception(msg)
 
 
 def cryptsetup_open(config):
@@ -271,6 +272,8 @@ def qemu_io_image_args(config, dev=False):
 def qemu_io_write_pattern(config, pattern, offset_mb, size_mb, dev=False):
 """Write a pattern of data to a LUKS image or device"""
 
+if dev:
+chown(config)
 args = ["-c", "write -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
 args.extend(qemu_io_image_args(config, dev))
 iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
@@ -281,6 +284,8 @@ def qemu_io_write_pattern(config, pattern, offset_mb, 
size_mb, dev=False):
 def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
 """Read a pattern of data to a LUKS image or device"""
 
+if dev:
+chown(config)
 args = ["-c", "read -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
 args.extend(qemu_io_image_args(config, dev))
 iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
@@ -331,9 +336,6 @@ def test_once(config, qemu_img=False):
 cryptsetup_open(config)
 
 try:
-iotests.log("# Set dev owner")
-chown(config)
-
 iotests.log("# Write test pattern 0xa7")
 qemu_io_write_pattern(config, 0xa7, lowOffsetMB, 10, dev=True)
 iotests.log("# Write test pattern 0x13")
@@ -365,9 +367,6 @@ def test_once(config, qemu_img=False):
 cryptsetup_open(config)
 
 try:
-iotests.log("# Set dev owner")
-chown(config)
-
 iotests.log("# Read test pattern 0x91")
 qemu_io_read_pattern(config, 0x91, lowOffsetMB, 10, dev=True)
 iotests.log("# Read test pattern 0x5e")
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index 2f0454f..5dea00b 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -5,14 +5,14 @@ truncate TEST_DIR/luks-aes-256-xts-plain64-sha1.img --size 
4194304MB
 sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
-# Set dev owner
-sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 # Write test pattern 0xa7
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c write -P 0xa7 100M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 wrote 10485760/10485760 bytes at offset 104857600
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 # Write test pattern 0x13
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c write -P 0x13 3145728M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 wrote 10485760/10485760 bytes at offset 3298534883328
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -41,14 +41,14 @@ wrote 10485760/10485760 bytes at offset 3298534883328
 
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
-# Set dev owner
-sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 # Read test pattern 0x91
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c read -P 0x91 100M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 read 10485760/10485760 bytes at offset 104857600
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 # Read test pattern 0x5e
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c read -P 0x5e 3145728M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 read 10485760/10485760 bytes at offset 3298534883328
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -65,14 +65,14 @@ Formatting 

[Qemu-devel] [PATCH 02/17] object: fix potential leak in getters

[Marc-André Lureau]

If the property is not of the requested type, the getters will leak a
QObject.

Signed-off-by: Marc-André Lureau 
---
 qom/object.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index eb4bc924ff..c7b8079df6 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1122,7 +1122,7 @@ char *object_property_get_str(Object *obj, const char 
*name,
 retval = g_strdup(qstring_get_str(qstring));
 }
 
-QDECREF(qstring);
+qobject_decref(ret);
 return retval;
 }
 
@@ -1183,7 +1183,7 @@ bool object_property_get_bool(Object *obj, const char 
*name,
 retval = qbool_get_bool(qbool);
 }
 
-QDECREF(qbool);
+qobject_decref(ret);
 return retval;
 }
 
@@ -1214,7 +1214,7 @@ int64_t object_property_get_int(Object *obj, const char 
*name,
 retval = qint_get_int(qint);
 }
 
-QDECREF(qint);
+qobject_decref(ret);
 return retval;
 }
 
-- 
2.13.0.rc1.16.gd80b50c3f

[Qemu-devel] [PATCH 03/17] tests: remove alt num-int cases

[Marc-André Lureau]

There are no real users of this case, and it's going to be invalid after
merging of QFloat and QInt use the same QNum type in the following patch.

Signed-off-by: Marc-André Lureau 
---
 tests/test-keyval.c |  3 ---
 tests/test-qobject-input-visitor.c  | 26 --
 tests/qapi-schema/qapi-schema-test.json |  2 --
 tests/qapi-schema/qapi-schema-test.out  |  8 
 4 files changed, 39 deletions(-)

diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index c556b1b117..df0ff831b5 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -615,7 +615,6 @@ static void test_keyval_visit_alternate(void)
 Visitor *v;
 QDict *qdict;
 AltNumStr *ans;
-AltNumInt *ani;
 
 /*
  * Can't do scalar alternate variants other than string.  You get
@@ -629,8 +628,6 @@ static void test_keyval_visit_alternate(void)
 g_assert_cmpint(ans->type, ==, QTYPE_QSTRING);
 g_assert_cmpstr(ans->u.s, ==, "1");
 qapi_free_AltNumStr(ans);
-visit_type_AltNumInt(v, "a", , );
-error_free_or_abort();
 visit_end_struct(v, NULL);
 visit_free(v);
 }
diff --git a/tests/test-qobject-input-visitor.c 
b/tests/test-qobject-input-visitor.c
index f965743b6e..a30e2d5e95 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -586,8 +586,6 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,
 AltStrNum *asn;
 AltNumStr *ans;
 AltStrInt *asi;
-AltIntNum *ain;
-AltNumInt *ani;
 
 /* Parsing an int */
 
@@ -614,18 +612,6 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,
 g_assert_cmpint(asi->u.i, ==, 42);
 qapi_free_AltStrInt(asi);
 
-v = visitor_input_test_init(data, "42");
-visit_type_AltIntNum(v, NULL, , _abort);
-g_assert_cmpint(ain->type, ==, QTYPE_QINT);
-g_assert_cmpint(ain->u.i, ==, 42);
-qapi_free_AltIntNum(ain);
-
-v = visitor_input_test_init(data, "42");
-visit_type_AltNumInt(v, NULL, , _abort);
-g_assert_cmpint(ani->type, ==, QTYPE_QINT);
-g_assert_cmpint(ani->u.i, ==, 42);
-qapi_free_AltNumInt(ani);
-
 /* Parsing a double */
 
 v = visitor_input_test_init(data, "42.5");
@@ -649,18 +635,6 @@ static void 
test_visitor_in_alternate_number(TestInputVisitorData *data,
 visit_type_AltStrInt(v, NULL, , );
 error_free_or_abort();
 qapi_free_AltStrInt(asi);
-
-v = visitor_input_test_init(data, "42.5");
-visit_type_AltIntNum(v, NULL, , _abort);
-g_assert_cmpint(ain->type, ==, QTYPE_QFLOAT);
-g_assert_cmpfloat(ain->u.n, ==, 42.5);
-qapi_free_AltIntNum(ain);
-
-v = visitor_input_test_init(data, "42.5");
-visit_type_AltNumInt(v, NULL, , _abort);
-g_assert_cmpint(ani->type, ==, QTYPE_QFLOAT);
-g_assert_cmpfloat(ani->u.n, ==, 42.5);
-qapi_free_AltNumInt(ani);
 }
 
 static void test_native_list_integer_helper(TestInputVisitorData *data,
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index 842ea3c5e3..9ad09e3758 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -103,8 +103,6 @@
 { 'alternate': 'AltStrNum', 'data': { 's': 'str', 'n': 'number' } }
 { 'alternate': 'AltNumStr', 'data': { 'n': 'number', 's': 'str' } }
 { 'alternate': 'AltStrInt', 'data': { 's': 'str', 'i': 'int' } }
-{ 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } }
-{ 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } }
 
 # for testing native lists
 { 'union': 'UserDefNativeListUnion',
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 9d99c4eebb..5c6655a5c3 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -1,11 +1,3 @@
-alternate AltIntNum
-tag type
-case i: int
-case n: number
-alternate AltNumInt
-tag type
-case n: number
-case i: int
 alternate AltNumStr
 tag type
 case n: number
-- 
2.13.0.rc1.16.gd80b50c3f

[Qemu-devel] [PATCH v5 2/5] iotests: fix remainining tests to work with LUKS

[Daniel P. Berrange]

The tests 033, 140, 145 and 157 were all broken
when run with LUKS, since they did not correctly use
the required image opts args syntax to specify the
decryption secret. Further, the 120 test simply does
not make sense to run with luks, as the scenario
exercised is not relevant.

The test 181 was broken when run with LUKS because
it didn't take account of fact that $TEST_IMG was
already in image opts syntax. The launch_qemu
helper also didn't register the secret object
providing the LUKS password.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/033 | 12 ++--
 tests/qemu-iotests/120 |  1 +
 tests/qemu-iotests/140 | 11 ++-
 tests/qemu-iotests/145 | 19 +--
 tests/qemu-iotests/157 | 17 ++---
 tests/qemu-iotests/157.out | 16 
 tests/qemu-iotests/174 |  2 +-
 tests/qemu-iotests/181 | 21 -
 tests/qemu-iotests/common.qemu |  9 +++--
 9 files changed, 84 insertions(+), 24 deletions(-)

diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033
index 16edcf2..2cdfd13 100755
--- a/tests/qemu-iotests/033
+++ b/tests/qemu-iotests/033
@@ -50,10 +50,18 @@ do_test()
local align=$1
local iocmd=$2
local img=$3
+   if [ "$IMGOPTSSYNTAX" = "true" ]
+   then
+   IO_OPEN_ARG="$img"
+   IO_EXTRA_ARGS="--image-opts"
+   else
+   IO_OPEN_ARG="-o driver=$IMGFMT,file.align=$align blkdebug::$img"
+   IO_EXTRA_ARGS=""
+   fi
{
-   echo "open -o driver=$IMGFMT,file.align=$align blkdebug::$img"
+   echo "open $IO_OPEN_ARG"
echo $iocmd
-   } | $QEMU_IO
+   } | $QEMU_IO $IO_EXTRA_ARGS
 }
 
 for write_zero_cmd in "write -z" "aio_write -z"; do
diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
index 4f88a67..f40b97d 100755
--- a/tests/qemu-iotests/120
+++ b/tests/qemu-iotests/120
@@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt generic
 _supported_proto file
 _supported_os Linux
+_unsupported_fmt luks
 
 _make_test_img 64M
 
diff --git a/tests/qemu-iotests/140 b/tests/qemu-iotests/140
index 8c80a5a..0a2105c 100755
--- a/tests/qemu-iotests/140
+++ b/tests/qemu-iotests/140
@@ -52,8 +52,17 @@ _make_test_img 64k
 
 $QEMU_IO -c 'write -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
 
+if test "$IMGOPTSSYNTAX" = "true"
+then
+SYSEMU_DRIVE_ARG=if=none,media=cdrom,id=drv,$TEST_IMG
+SYSEMU_EXTRA_ARGS=""
+else
+SYSEMU_DRIVE_ARG=if=none,media=cdrom,id=drv,file="$TEST_IMG",driver=$IMGFMT
+SYSEMU_EXTRA_ARGS=""
+fi
+
 keep_stderr=y \
-_launch_qemu -drive if=none,media=cdrom,id=drv,file="$TEST_IMG",format=$IMGFMT 
\
+_launch_qemu $SYSEMU_EXTRA_ARGS -drive $SYSEMU_DRIVE_ARG \
 2> >(_filter_nbd)
 
 _send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/145 b/tests/qemu-iotests/145
index e6c6bc4..9cfa940 100755
--- a/tests/qemu-iotests/145
+++ b/tests/qemu-iotests/145
@@ -43,8 +43,23 @@ _supported_proto generic
 _supported_os Linux
 
 _make_test_img 1M
-echo quit | $QEMU -nographic -hda "$TEST_IMG" -incoming 'exec:true' -snapshot 
-serial none -monitor stdio |
-_filter_qemu | _filter_hmp
+
+if test "$IMGOPTSSYNTAX" = "true"
+then
+SYSEMU_DRIVE_ARG=if=none,$TEST_IMG
+SYSEMU_EXTRA_ARGS=""
+if [ -n "$IMGKEYSECRET" ]; then
+   SECRET_ARG="secret,id=keysec0,data=$IMGKEYSECRET"
+   SYSEMU_EXTRA_ARGS="-object $SECRET_ARG"
+fi
+else
+SYSEMU_DRIVE_ARG=if=none,file="$TEST_IMG",driver=$IMGFMT
+SYSEMU_EXTRA_ARGS=""
+fi
+
+echo quit | $QEMU -nographic $SYSEMU_EXTRA_ARGS  -drive $SYSEMU_DRIVE_ARG \
+  -incoming 'exec:true' -snapshot -serial none -monitor stdio \
+  | _filter_qemu | _filter_hmp
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/157 b/tests/qemu-iotests/157
index 8d939cb..f5d22fa 100755
--- a/tests/qemu-iotests/157
+++ b/tests/qemu-iotests/157
@@ -43,7 +43,6 @@ _supported_os Linux
 
 function do_run_qemu()
 {
-echo Testing: "$@"
 (
 if ! test -t 0; then
 while read cmd; do
@@ -63,7 +62,18 @@ function run_qemu()
 
 
 size=128M
-drive="if=none,file=$TEST_IMG,driver=$IMGFMT"
+if test "$IMGOPTSSYNTAX" = "true"
+then
+SYSEMU_DRIVE_ARG=if=none,$TEST_IMG
+SYSEMU_EXTRA_ARGS=""
+if [ -n "$IMGKEYSECRET" ]; then
+   SECRET_ARG="secret,id=keysec0,data=$IMGKEYSECRET"
+   SYSEMU_EXTRA_ARGS="-object $SECRET_ARG"
+fi
+else
+SYSEMU_DRIVE_ARG=if=none,file="$TEST_IMG",driver=$IMGFMT
+SYSEMU_EXTRA_ARGS=""
+fi
 
 _make_test_img $size
 
@@ -76,8 +86,9 @@ echo
 
 for cache in "writeback" "writethrough"; do
 for wce in "" ",write-cache=auto" ",write-cache=on" ",write-cache=off"; do
+echo "Testing: cache='$cache' wce='$wce'"
 echo "info block" \
-| run_qemu -drive "$drive,cache=$cache" \
+| run_qemu $SYSEMU_EXTRA_ARGS -drive 

[Qemu-devel] [PATCH v5 3/5] iotests: reduce PBKDF iterations when testing LUKS

[Daniel P. Berrange]

By default the PBKDF algorithm used with LUKS is tuned
based on the number of iterations to produce 1 second
of running time. This makes running the I/O test with
the LUKS format orders of magnitude slower than with
qcow2/raw formats.

When creating LUKS images, set the iteration time to
a 10ms to reduce the time overhead for LUKS, since
security does not matter in I/O tests.

Previously a full 'check -luks' would take

  $ time ./check -luks
  Passed all 22 tests

  real  23m9.988s
  user  21m46.223s
  sys   0m22.841s

Now it takes

  $ time ./check -luks
  Passed all 22 tests

  real  4m39.235s
  user  3m29.590s
  sys   0m24.234s

Still slow compared to qcow2/raw, but much improved
none the less.

Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/149   |   3 +
 tests/qemu-iotests/149.out   | 118 +++
 tests/qemu-iotests/common.filter |   3 +-
 tests/qemu-iotests/common.rc |   3 +
 4 files changed, 67 insertions(+), 60 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 8407251..f62e618 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -136,6 +136,7 @@ def cryptsetup_add_password(config, slot):
 args = ["luksAddKey", config.image_path(),
 "--key-slot", slot,
 "--key-file", "-",
+"--iter-time", "10",
 pwfile]
 
 cryptsetup(args, password)
@@ -164,6 +165,7 @@ def cryptsetup_format(config):
 args.extend(["--hash", config.hash])
 args.extend(["--key-slot", slot])
 args.extend(["--key-file", "-"])
+args.extend(["--iter-time", "10"])
 args.append(config.image_path())
 
 cryptsetup(args, password)
@@ -230,6 +232,7 @@ def qemu_img_create(config, size_mb):
 
 opts = [
 "key-secret=sec0",
+"iter-time=10",
 "cipher-alg=%s-%d" % (config.cipher, config.keylen),
 "cipher-mode=%s" % config.mode,
 "ivgen-alg=%s" % config.ivgen,
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index 90b5b55..b18c4e4 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -2,7 +2,7 @@
 # Create image
 truncate TEST_DIR/luks-aes-256-xts-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - TEST_DIR/luks-aes-256-xts-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
 # Set dev owner
@@ -60,8 +60,8 @@ unlink TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 
 # = qemu-img aes-256-xts-plain64-sha1 =
 # Create image
-qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-xts-plain64-sha1.img 4194304M
-Formatting 'TEST_DIR/luks-aes-256-xts-plain64-sha1.img', fmt=luks 
size=4398046511104 key-secret=sec0 cipher-alg=aes-256 cipher-mode=xts 
ivgen-alg=plain64 hash-alg=sha1
+qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-xts-plain64-sha1.img 4194304M
+Formatting 'TEST_DIR/luks-aes-256-xts-plain64-sha1.img', fmt=luks 
size=4398046511104 key-secret=sec0 cipher-alg=aes-256 cipher-mode=xts 
ivgen-alg=plain64 hash-alg=sha1 iter-time=10
 
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
@@ -122,7 +122,7 @@ unlink TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Create image
 truncate TEST_DIR/luks-twofish-256-xts-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher twofish-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - 
TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --cipher twofish-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 
qiotest-145-twofish-256-xts-plain64-sha1
 # Set dev owner
@@ -180,8 +180,8 @@ unlink TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 
 # = qemu-img twofish-256-xts-plain64-sha1 =
 # Create image
-qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,cipher-alg=twofish-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 4194304M
-Formatting