[Qemu-devel] [PULL 0/9] Misc changes for 2015-01-03 (icount, migration, iscsi, sdhci-pci)

2015-01-03 Thread Paolo Bonzini 
The following changes since commit c95f3901b4ead79f3fe2c641fda7d2c70fc84c72:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-sdl-20141219-1' into 
staging (2014-12-21 23:17:00 +)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 5aa8136020f47fbd38c458b9834c783cb7063db8:

  pci: move REDHAT_SDHCI device ID to make room for Rocker (2015-01-03 09:22:13 
+0100)


More migration fixes and more record/replay preparations.  Also moves
the sdhci-pci device id to make space for the rocker device.


Paolo Bonzini (7):
  atomic: fix position of volatile qualifier
  target-ppc: pass DisasContext to SPR generator functions
  cpu-exec: add a new CF_USE_ICOUNT cflag
  translate: check cflags instead of use_icount global
  gen-icount: check cflags instead of use_icount global
  serial: refine serial_thr_ipending_needed
  pckbd: set bits 2-3-6-7 of the output port by default

Peter Wu (1):
  block/iscsi: fix uninitialized variable

Scott Feldman (1):
  pci: move REDHAT_SDHCI device ID to make room for Rocker

 block/iscsi.c |   2 +-
 docs/specs/pci-ids.txt|   2 +-
 hw/char/serial.c  |  13 +-
 hw/input/pckbd.c  |  10 +-
 include/exec/exec-all.h   |   5 +-
 include/exec/gen-icount.h |   6 +-
 include/hw/pci/pci.h  |   2 +-
 include/qemu/atomic.h |   4 +-
 target-alpha/translate.c  |  10 +-
 target-arm/translate-a64.c|   6 +-
 target-arm/translate.c|   6 +-
 target-cris/translate.c   |   2 +-
 target-i386/translate.c   |  52 
 target-lm32/translate.c   |  10 +-
 target-m68k/translate.c   |   2 +-
 target-microblaze/translate.c |   2 +-
 target-mips/translate.c   |  26 ++--
 target-moxie/translate.c  |   2 +-
 target-openrisc/translate.c   |   2 +-
 target-ppc/cpu.h  |  13 +-
 target-ppc/translate.c|  12 +-
 target-ppc/translate_init.c   | 271 +-
 target-s390x/translate.c  |   2 +-
 target-sh4/translate.c|   2 +-
 target-sparc/translate.c  |   2 +-
 target-tricore/translate.c|   2 +-
 target-unicore32/translate.c  |   2 +-
 target-xtensa/translate.c |   2 +-
 translate-all.c   |   5 +-
 29 files changed, 253 insertions(+), 224 deletions(-)
-- 
1.8.3.1

[Qemu-devel] [PULL 3/9] cpu-exec: add a new CF_USE_ICOUNT cflag

2015-01-03 Thread Paolo Bonzini 
Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Paolo Bonzini 
---
 include/exec/exec-all.h | 5 +++--
 translate-all.c | 3 +++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 38a8a09..6a15448 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -142,10 +142,12 @@ struct TranslationBlock {
 uint64_t flags; /* flags defining in which context the code was generated 
*/
 uint16_t size;  /* size of target code for this block (1 <=
size <= TARGET_PAGE_SIZE) */
-uint16_t cflags;/* compile flags */
+uint16_t icount;
+uint32_t cflags;/* compile flags */
 #define CF_COUNT_MASK  0x7fff
 #define CF_LAST_IO 0x8000 /* Last insn may be an IO access.  */
 #define CF_NOCACHE 0x1 /* To be freed after execution */
+#define CF_USE_ICOUNT  0x2
 
 void *tc_ptr;/* pointer to the translated code */
 /* next matching tb for physical address. */
@@ -169,7 +171,6 @@ struct TranslationBlock {
jmp_first */
 struct TranslationBlock *jmp_next[2];
 struct TranslationBlock *jmp_first;
-uint32_t icount;
 };
 
 #include "exec/spinlock.h"
diff --git a/translate-all.c b/translate-all.c
index c24cfe8..db2102d 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1045,6 +1045,9 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 int code_gen_size;
 
 phys_pc = get_page_addr_code(env, pc);
+if (use_icount) {
+cflags |= CF_USE_ICOUNT;
+}
 tb = tb_alloc(pc);
 if (!tb) {
 /* flush must be done */
-- 
1.8.3.1

[Qemu-devel] [PULL 1/9] atomic: fix position of volatile qualifier

2015-01-03 Thread Paolo Bonzini 
What needs to be volatile is not the pointer, but the pointed-to
value!

Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
 include/qemu/atomic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 492bce1..93c2ae2 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -122,11 +122,11 @@
 #endif
 
 #ifndef atomic_read
-#define atomic_read(ptr)   (*(__typeof__(*ptr) *volatile) (ptr))
+#define atomic_read(ptr)   (*(__typeof__(*ptr) volatile*) (ptr))
 #endif
 
 #ifndef atomic_set
-#define atomic_set(ptr, i) ((*(__typeof__(*ptr) *volatile) (ptr)) = (i))
+#define atomic_set(ptr, i) ((*(__typeof__(*ptr) volatile*) (ptr)) = (i))
 #endif
 
 /* These have the same semantics as Java volatile variables.
-- 
1.8.3.1

[Qemu-devel] [PULL 4/9] translate: check cflags instead of use_icount global

2015-01-03 Thread Paolo Bonzini 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Paolo Bonzini 
---
 target-alpha/translate.c|  8 
 target-arm/translate-a64.c  |  4 ++--
 target-arm/translate.c  |  4 ++--
 target-i386/translate.c | 50 +++--
 target-lm32/translate.c |  8 
 target-mips/translate.c | 24 +-
 target-ppc/translate_init.c | 24 +++---
 translate-all.c |  2 +-
 8 files changed, 69 insertions(+), 55 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 76658a0..5387b93 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -1285,7 +1285,7 @@ static int cpu_pr_data(int pr)
 return 0;
 }
 
-static ExitStatus gen_mfpr(TCGv va, int regno)
+static ExitStatus gen_mfpr(DisasContext *ctx, TCGv va, int regno)
 {
 int data = cpu_pr_data(regno);
 
@@ -1295,7 +1295,7 @@ static ExitStatus gen_mfpr(TCGv va, int regno)
if (regno == 249) {
helper = gen_helper_get_vmtime;
}
-if (use_icount) {
+if (ctx->tb->cflags & CF_USE_ICOUNT) {
 gen_io_start();
 helper(va);
 gen_io_end();
@@ -2283,7 +2283,7 @@ static ExitStatus translate_one(DisasContext *ctx, 
uint32_t insn)
 case 0xC000:
 /* RPCC */
 va = dest_gpr(ctx, ra);
-if (use_icount) {
+if (ctx->tb->cflags & CF_USE_ICOUNT) {
 gen_io_start();
 gen_helper_load_pcc(va, cpu_env);
 gen_io_end();
@@ -2317,7 +2317,7 @@ static ExitStatus translate_one(DisasContext *ctx, 
uint32_t insn)
 #ifndef CONFIG_USER_ONLY
 REQUIRE_TB_FLAG(TB_FLAGS_PAL_MODE);
 va = dest_gpr(ctx, ra);
-ret = gen_mfpr(va, insn & 0x);
+ret = gen_mfpr(ctx, va, insn & 0x);
 break;
 #else
 goto invalid_opc;
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 80d2c07..c78ebde 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1372,7 +1372,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, 
bool isread,
 break;
 }
 
-if (use_icount && (ri->type & ARM_CP_IO)) {
+if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) {
 gen_io_start();
 }
 
@@ -1403,7 +1403,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, 
bool isread,
 }
 }
 
-if (use_icount && (ri->type & ARM_CP_IO)) {
+if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) {
 /* I/O operations must end the TB here (whether read or write) */
 gen_io_end();
 s->is_jmp = DISAS_UPDATE;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index b52c758..f5a8482 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7170,7 +7170,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t 
insn)
 break;
 }
 
-if (use_icount && (ri->type & ARM_CP_IO)) {
+if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) {
 gen_io_start();
 }
 
@@ -7261,7 +7261,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t 
insn)
 }
 }
 
-if (use_icount && (ri->type & ARM_CP_IO)) {
+if ((s->tb->cflags & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) {
 /* I/O operations must end the TB here (whether read or write) */
 gen_io_end();
 gen_lookup_tb(s);
diff --git a/target-i386/translate.c b/target-i386/translate.c
index fc75da7..0792bd0 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -1168,8 +1168,9 @@ static inline void gen_cmps(DisasContext *s, TCGMemOp ot)
 
 static inline void gen_ins(DisasContext *s, TCGMemOp ot)
 {
-if (use_icount)
+if (s->tb->cflags & CF_USE_ICOUNT) {
 gen_io_start();
+}
 gen_string_movl_A0_EDI(s);
 /* Note: we must do this dummy write first to be restartable in
case of page fault. */
@@ -1181,14 +1182,16 @@ static inline void gen_ins(DisasContext *s, TCGMemOp ot)
 gen_op_st_v(s, ot, cpu_T[0], cpu_A0);
 gen_op_movl_T0_Dshift(ot);
 gen_op_add_reg_T0(s->aflag, R_EDI);
-if (use_icount)
+if (s->tb->cflags & CF_USE_ICOUNT) {
 gen_io_end();
+}
 }
 
 static inline void gen_outs(DisasContext *s, TCGMemOp ot)
 {
-if (use_icount)
+if (s->tb->cflags & CF_USE_ICOUNT) {
 gen_io_start();
+}
 gen_string_movl_A0_ESI(s);
 gen_op_ld_v(s, ot, cpu_T[0], cpu_A0);
 
@@ -1199,8 +1202,9 @@ static inline void gen_outs(DisasContext *s, TCGMemOp ot)
 
 gen_op_movl_T0_Dshift(ot);
 gen_op_add_reg_T0(s->aflag, R_ESI);
-if (use_icount)
+if (s->tb->cflags & CF_USE_ICOUNT) {
 gen_io_end();
+}
 }
 
 /* same method as Valgrind : we generate jumps to current or next
@@ -6278,7 +6282,7 @@ static target_ulong disas_insn(CPUX86State *env,

[Qemu-devel] [PULL 6/9] serial: refine serial_thr_ipending_needed

2015-01-03 Thread Paolo Bonzini 
If the THR interrupt is disabled, there is no need to migrate thr_ipending
because LSR.THRE will be sampled again when the interrupt is enabled.
(This is the behavior that is not documented in the datasheet, but
relied on by Windows!)

Note that in this case IIR will never be 0x2 so, if thr_ipending were
to be one, QEMU would produce the subsection.

Reported-by: Igor Mammedov 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
 hw/char/serial.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 6d522ff..3aca874 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -645,8 +645,17 @@ static int serial_post_load(void *opaque, int version_id)
 static bool serial_thr_ipending_needed(void *opaque)
 {
 SerialState *s = opaque;
-bool expected_value = ((s->iir & UART_IIR_ID) == UART_IIR_THRI);
-return s->thr_ipending != expected_value;
+
+if (s->ier & UART_IER_THRI) {
+bool expected_value = ((s->iir & UART_IIR_ID) == UART_IIR_THRI);
+return s->thr_ipending != expected_value;
+} else {
+/* LSR.THRE will be sampled again when the interrupt is
+ * enabled.  thr_ipending is not used in this case, do
+ * not migrate it.
+ */
+return false;
+}
 }
 
 const VMStateDescription vmstate_serial_thr_ipending = {
-- 
1.8.3.1

[Qemu-devel] [PULL 2/9] target-ppc: pass DisasContext to SPR generator functions

2015-01-03 Thread Paolo Bonzini 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Alexander Graf 
Signed-off-by: Paolo Bonzini 
---
 target-ppc/cpu.h|  13 +--
 target-ppc/translate.c  |  10 +-
 target-ppc/translate_init.c | 247 ++--
 3 files changed, 133 insertions(+), 137 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 068fcb2..f42589c 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -320,6 +320,7 @@ typedef struct opc_handler_t opc_handler_t;
 /*/
 /* Types used to describe some PowerPC registers */
 typedef struct CPUPPCState CPUPPCState;
+typedef struct DisasContext DisasContext;
 typedef struct ppc_tb_t ppc_tb_t;
 typedef struct ppc_spr_t ppc_spr_t;
 typedef struct ppc_dcr_t ppc_dcr_t;
@@ -328,13 +329,13 @@ typedef union ppc_tlb_t ppc_tlb_t;
 
 /* SPR access micro-ops generations callbacks */
 struct ppc_spr_t {
-void (*uea_read)(void *opaque, int gpr_num, int spr_num);
-void (*uea_write)(void *opaque, int spr_num, int gpr_num);
+void (*uea_read)(DisasContext *ctx, int gpr_num, int spr_num);
+void (*uea_write)(DisasContext *ctx, int spr_num, int gpr_num);
 #if !defined(CONFIG_USER_ONLY)
-void (*oea_read)(void *opaque, int gpr_num, int spr_num);
-void (*oea_write)(void *opaque, int spr_num, int gpr_num);
-void (*hea_read)(void *opaque, int gpr_num, int spr_num);
-void (*hea_write)(void *opaque, int spr_num, int gpr_num);
+void (*oea_read)(DisasContext *ctx, int gpr_num, int spr_num);
+void (*oea_write)(DisasContext *ctx, int spr_num, int gpr_num);
+void (*hea_read)(DisasContext *ctx, int gpr_num, int spr_num);
+void (*hea_write)(DisasContext *ctx, int spr_num, int gpr_num);
 #endif
 const char *name;
 target_ulong default_value;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index d381632..f22a116 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -183,7 +183,7 @@ void ppc_translate_init(void)
 }
 
 /* internal defines */
-typedef struct DisasContext {
+struct DisasContext {
 struct TranslationBlock *tb;
 target_ulong nip;
 uint32_t opcode;
@@ -207,7 +207,7 @@ typedef struct DisasContext {
 int singlestep_enabled;
 uint64_t insns_flags;
 uint64_t insns_flags2;
-} DisasContext;
+};
 
 /* Return true iff byteswap is needed in a scalar memop */
 static inline bool need_byteswap(const DisasContext *ctx)
@@ -4206,7 +4206,7 @@ static void gen_mfmsr(DisasContext *ctx)
 #endif
 }
 
-static void spr_noaccess(void *opaque, int gprn, int sprn)
+static void spr_noaccess(DisasContext *ctx, int gprn, int sprn)
 {
 #if 0
 sprn = ((sprn >> 5) & 0x1F) | ((sprn & 0x1F) << 5);
@@ -4218,7 +4218,7 @@ static void spr_noaccess(void *opaque, int gprn, int sprn)
 /* mfspr */
 static inline void gen_op_mfspr(DisasContext *ctx)
 {
-void (*read_cb)(void *opaque, int gprn, int sprn);
+void (*read_cb)(DisasContext *ctx, int gprn, int sprn);
 uint32_t sprn = SPR(ctx->opcode);
 
 #if !defined(CONFIG_USER_ONLY)
@@ -4369,7 +4369,7 @@ static void gen_mtmsr(DisasContext *ctx)
 /* mtspr */
 static void gen_mtspr(DisasContext *ctx)
 {
-void (*write_cb)(void *opaque, int sprn, int gprn);
+void (*write_cb)(DisasContext *ctx, int sprn, int gprn);
 uint32_t sprn = SPR(ctx->opcode);
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 1fece7b..5908a95 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -65,7 +65,7 @@ static void spr_load_dump_spr(int sprn)
 #endif
 }
 
-static void spr_read_generic (void *opaque, int gprn, int sprn)
+static void spr_read_generic (DisasContext *ctx, int gprn, int sprn)
 {
 gen_load_spr(cpu_gpr[gprn], sprn);
 spr_load_dump_spr(sprn);
@@ -80,14 +80,14 @@ static void spr_store_dump_spr(int sprn)
 #endif
 }
 
-static void spr_write_generic (void *opaque, int sprn, int gprn)
+static void spr_write_generic (DisasContext *ctx, int sprn, int gprn)
 {
 gen_store_spr(sprn, cpu_gpr[gprn]);
 spr_store_dump_spr(sprn);
 }
 
 #if !defined(CONFIG_USER_ONLY)
-static void spr_write_generic32(void *opaque, int sprn, int gprn)
+static void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
 {
 #ifdef TARGET_PPC64
 TCGv t0 = tcg_temp_new();
@@ -96,11 +96,11 @@ static void spr_write_generic32(void *opaque, int sprn, int 
gprn)
 tcg_temp_free(t0);
 spr_store_dump_spr(sprn);
 #else
-spr_write_generic(opaque, sprn, gprn);
+spr_write_generic(ctx, sprn, gprn);
 #endif
 }
 
-static void spr_write_clear (void *opaque, int sprn, int gprn)
+static void spr_write_clear (DisasContext *ctx, int sprn, int gprn)
 {
 TCGv t0 = tcg_temp_new();
 TCGv t1 = tcg_temp_new();
@@ -112,7 +112,7 @@ static void spr_write_clear (void *opaque, int sprn, int 
gprn)
 tcg_temp_free(t1);
 }
 
-static void spr_access_nop(void *opaque, int spr

[Qemu-devel] [PULL 9/9] pci: move REDHAT_SDHCI device ID to make room for Rocker

2015-01-03 Thread Paolo Bonzini 
From: Scott Feldman 

The rocker device uses same PCI device ID as sdhci.  Since rocker device driver
has already been accepted into Linux 3.18, and REDHAT_SDHCI device ID isn't
used by any drivers, it's safe to move REDHAT_SDHCI device ID, avoiding
conflict with rocker.

Signed-off-by: Scott Feldman 
Signed-off-by: Jiri Pirko 
Signed-off-by: Paolo Bonzini 
---
 docs/specs/pci-ids.txt | 2 +-
 include/hw/pci/pci.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt
index 9b57d5e..c6732fe 100644
--- a/docs/specs/pci-ids.txt
+++ b/docs/specs/pci-ids.txt
@@ -45,7 +45,7 @@ PCI devices (other than virtio):
 1b36:0003  PCI Dual-port 16550A adapter (docs/specs/pci-serial.txt)
 1b36:0004  PCI Quad-port 16550A adapter (docs/specs/pci-serial.txt)
 1b36:0005  PCI test device (docs/specs/pci-testdev.txt)
-1b36:0006  PCI SD Card Host Controller Interface (SDHCI)
+1b36:0007  PCI SD Card Host Controller Interface (SDHCI)
 
 All these devices are documented in docs/specs.
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 97e4257..97a83d3 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -88,7 +88,7 @@
 #define PCI_DEVICE_ID_REDHAT_SERIAL2 0x0003
 #define PCI_DEVICE_ID_REDHAT_SERIAL4 0x0004
 #define PCI_DEVICE_ID_REDHAT_TEST0x0005
-#define PCI_DEVICE_ID_REDHAT_SDHCI   0x0006
+#define PCI_DEVICE_ID_REDHAT_SDHCI   0x0007
 #define PCI_DEVICE_ID_REDHAT_QXL 0x0100
 
 #define FMT_PCIBUS  PRIx64
-- 
1.8.3.1

[Qemu-devel] [PULL 5/9] gen-icount: check cflags instead of use_icount global

2015-01-03 Thread Paolo Bonzini 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Pavel Dovgalyuk 
Signed-off-by: Paolo Bonzini 
---
 include/exec/gen-icount.h | 6 +++---
 target-alpha/translate.c  | 2 +-
 target-arm/translate-a64.c| 2 +-
 target-arm/translate.c| 2 +-
 target-cris/translate.c   | 2 +-
 target-i386/translate.c   | 2 +-
 target-lm32/translate.c   | 2 +-
 target-m68k/translate.c   | 2 +-
 target-microblaze/translate.c | 2 +-
 target-mips/translate.c   | 2 +-
 target-moxie/translate.c  | 2 +-
 target-openrisc/translate.c   | 2 +-
 target-ppc/translate.c| 2 +-
 target-s390x/translate.c  | 2 +-
 target-sh4/translate.c| 2 +-
 target-sparc/translate.c  | 2 +-
 target-tricore/translate.c| 2 +-
 target-unicore32/translate.c  | 2 +-
 target-xtensa/translate.c | 2 +-
 19 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index da53395..221aad0 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -9,7 +9,7 @@ static TCGArg *icount_arg;
 static int icount_label;
 static int exitreq_label;
 
-static inline void gen_tb_start(void)
+static inline void gen_tb_start(TranslationBlock *tb)
 {
 TCGv_i32 count;
 TCGv_i32 flag;
@@ -21,7 +21,7 @@ static inline void gen_tb_start(void)
 tcg_gen_brcondi_i32(TCG_COND_NE, flag, 0, exitreq_label);
 tcg_temp_free_i32(flag);
 
-if (!use_icount)
+if (!(tb->cflags & CF_USE_ICOUNT))
 return;
 
 icount_label = gen_new_label();
@@ -43,7 +43,7 @@ static void gen_tb_end(TranslationBlock *tb, int num_insns)
 gen_set_label(exitreq_label);
 tcg_gen_exit_tb((uintptr_t)tb + TB_EXIT_REQUESTED);
 
-if (use_icount) {
+if (tb->cflags & CF_USE_ICOUNT) {
 *icount_arg = num_insns;
 gen_set_label(icount_label);
 tcg_gen_exit_tb((uintptr_t)tb + TB_EXIT_ICOUNT_EXPIRED);
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 5387b93..f888367 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2828,7 +2828,7 @@ static inline void 
gen_intermediate_code_internal(AlphaCPU *cpu,
 pc_mask = ~TARGET_PAGE_MASK;
 }
 
-gen_tb_start();
+gen_tb_start(tb);
 do {
 if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) {
 QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index c78ebde..80d2359 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -10962,7 +10962,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
 max_insns = CF_COUNT_MASK;
 }
 
-gen_tb_start();
+gen_tb_start(tb);
 
 tcg_clear_temp_count();
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f5a8482..bdfcdf1 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11080,7 +11080,7 @@ static inline void 
gen_intermediate_code_internal(ARMCPU *cpu,
 if (max_insns == 0)
 max_insns = CF_COUNT_MASK;
 
-gen_tb_start();
+gen_tb_start(tb);
 
 tcg_clear_temp_count();
 
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 76406af..b675ed0 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3202,7 +3202,7 @@ gen_intermediate_code_internal(CRISCPU *cpu, 
TranslationBlock *tb,
 max_insns = CF_COUNT_MASK;
 }
 
-gen_tb_start();
+gen_tb_start(tb);
 do {
 check_breakpoint(env, dc);
 
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 0792bd0..ebdc350 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -8002,7 +8002,7 @@ static inline void gen_intermediate_code_internal(X86CPU 
*cpu,
 if (max_insns == 0)
 max_insns = CF_COUNT_MASK;
 
-gen_tb_start();
+gen_tb_start(tb);
 for(;;) {
 if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) {
 QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index f748f96..a7579dc 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1095,7 +1095,7 @@ void gen_intermediate_code_internal(LM32CPU *cpu,
 max_insns = CF_COUNT_MASK;
 }
 
-gen_tb_start();
+gen_tb_start(tb);
 do {
 check_breakpoint(env, dc);
 
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index efd4cfc..47edc7a 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3010,7 +3010,7 @@ gen_intermediate_code_internal(M68kCPU *cpu, 
TranslationBlock *tb,
 if (max_insns == 0)
 max_insns = CF_COUNT_MASK;
 
-gen_tb_start();
+gen_tb_start(tb);
 do {
 pc_offset = dc->pc - pc_start;
 gen_throws_exception = NULL;
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index fd2b771..69ce4df 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1720,7 +1720,7 @@ gen_intermediate_code_in

[Qemu-devel] [PULL 8/9] block/iscsi: fix uninitialized variable

2015-01-03 Thread Paolo Bonzini 
From: Peter Wu 

'ret' was never initialized in the success path.

Signed-off-by: Peter Wu 
Signed-off-by: Paolo Bonzini 
---
 block/iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index ed375fc..12ddbfb 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1286,7 +1286,7 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 QemuOpts *opts;
 Error *local_err = NULL;
 const char *filename;
-int i, ret;
+int i, ret = 0;
 
 if ((BDRV_SECTOR_SIZE % 512) != 0) {
 error_setg(errp, "iSCSI: Invalid BDRV_SECTOR_SIZE. "
-- 
1.8.3.1

[Qemu-devel] [PULL 7/9] pckbd: set bits 2-3-6-7 of the output port by default

2015-01-03 Thread Paolo Bonzini 
OSes typically write 0xdd/0xdf to turn the A20 line off and on.  This
has bits 2-3-6-7 on, so that the output port subsection is migrated.
Change the reset value and migration default to include those four
bits, thus avoiding that the subsection is migrated.

This strictly speaking changes guest ABI, but the long time during which
we have not migrated the value means that the guests really do not care
much; so the change is for all machine types.

Reported-by: Igor Mammedov 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
 hw/input/pckbd.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 2b0cd3d..9b9a7d7 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -101,6 +101,12 @@
 #define KBD_OUT_OBF 0x10/* Keyboard output buffer full */
 #define KBD_OUT_MOUSE_OBF   0x20/* Mouse output buffer full */
 
+/* OSes typically write 0xdd/0xdf to turn the A20 line off and on.
+ * We make the default value of the outport include these four bits,
+ * so that the subsection is rarely necessary.
+ */
+#define KBD_OUT_ONES0xcc
+
 /* Mouse Commands */
 #define AUX_SET_SCALE110xE6/* Set 1:1 scaling */
 #define AUX_SET_SCALE210xE7/* Set 2:1 scaling */
@@ -367,13 +373,13 @@ static void kbd_reset(void *opaque)
 
 s->mode = KBD_MODE_KBD_INT | KBD_MODE_MOUSE_INT;
 s->status = KBD_STAT_CMD | KBD_STAT_UNLOCKED;
-s->outport = KBD_OUT_RESET | KBD_OUT_A20;
+s->outport = KBD_OUT_RESET | KBD_OUT_A20 | KBD_OUT_ONES;
 s->outport_present = false;
 }
 
 static uint8_t kbd_outport_default(KBDState *s)
 {
-return KBD_OUT_RESET | KBD_OUT_A20
+return KBD_OUT_RESET | KBD_OUT_A20 | KBD_OUT_ONES
| (s->status & KBD_STAT_OBF ? KBD_OUT_OBF : 0)
| (s->status & KBD_STAT_MOUSE_OBF ? KBD_OUT_MOUSE_OBF : 0);
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2.3 0/8] Linked list for tcg ops

2015-01-03 Thread Paolo Bonzini 


On 11/11/2014 17:24, Richard Henderson wrote:
> Currently tcg ops are simply placed in a buffer in order.  Which is
> fine until we want to actually do something with the opcode stream,
> such as optimize them.  Note the horrible things like call opcodes
> needing their argument count both prefixed and postfixed so that we
> can iterate across the call either forward or backward.
> 
> While I'm changing this, I also move quite a lot of tcg-op.h out of
> line.  There is very little benefit to having most of them be inline,
> since their arguments are extracted from the guest instructions being
> translated, and thus their values are not really predictable.
> 
> I chose a cutoff of one function call.  If a tcg-op.h functionconsists
> of a single function call, inline it, otherwise move it out of line.
> 
> This also removes a bit of boilerplate from each target.
> 
> I haven't been able to measure a performance difference with this
> patch set.  I wouldn't really expect any, as the complexity level
> remains the same.  I simply find the link list significantly more
> maintainable.
> 
> Of course this isn't intended for the upcoming 2.2 release.
> 
> Comments?

Happy new year! :) Are you going to submit this now?

Paolo

Re: [Qemu-devel] [PATCH 01/10] block/dmg: properly detect the UDIF trailer

2015-01-03 Thread Peter Wu 
On Friday 02 January 2015 18:58:00 John Snow wrote:
> 
> On 12/27/2014 10:01 AM, Peter Wu wrote:
> > DMG files have a variable length with a UDIF trailer at the end of a
> > file. This UDIF trailer is essential as it describes the contents of
> > the image. At the moment however, the start of this trailer is almost
> > always incorrect as bdrv_getlength() returns a multiple of the block
> > size (rounded up). This results in a failure to recognize DMG files,
> > resulting in Invalid argument (EINVAL) errors.
> >
> > As there is no API to retrieve the real file size, look for the magic
> > header in the last two sectors to find the start of this 512-byte UDIF
> > trailer (the "koly" block).
> >
> > The resource fork offset ("info_begin") has its offset adjusted as the
> > initial value of offset does not mean "end of file" anymore, but "begin
> > of UDIF trailer".
> >
> > Signed-off-by: Peter Wu 
> > ---
> >   block/dmg.c | 40 
> >   1 file changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/block/dmg.c b/block/dmg.c
> > index e455886..df274f9 100644
> > --- a/block/dmg.c
> > +++ b/block/dmg.c
> > @@ -131,6 +131,39 @@ static void update_max_chunk_size(BDRVDMGState *s, 
> > uint32_t chunk,
> >   }
> >   }
> >
> > +static int64_t dmg_find_koly_offset(BlockDriverState *file_bs)
> > +{
> > +int64_t length;
> > +int64_t offset = 0;
> > +uint8_t buffer[515];
> > +int i, ret;
> > +
> > +/* bdrv_getlength returns a multiple of block size (512), rounded up. 
> > Since
> > + * dmg images can have odd sizes, try to look for the "koly" magic 
> > which
> > + * marks the begin of the UDIF trailer (512 bytes). This magic can be 
> > found
> > + * in the last 511 bytes of the second-last sector or the first 4 
> > bytes of
> > + * the last sector (search space: 515 bytes) */
> > +length = bdrv_getlength(file_bs);
> > +if (length < 512) {
> > +return length < 0 ? length : -EINVAL;
> > +}
> > +if (length > 511 + 512) {
> > +offset = length - 511 - 512;
> > +}
> > +length = length < 515 ? length : 515;
> > +ret = bdrv_pread(file_bs, offset, buffer, length);
> > +if (ret < 4) {
> > +return ret < 0 ? ret : -EINVAL;
> > +}
> > +for (i = 0; i < length - 3; i++) {
> > +if (buffer[i] == 'k' && buffer[i+1] == 'o' &&
> > +buffer[i+2] == 'l' && buffer[i+3] == 'y') {
> > +return offset + i;
> > +}
> > +}
> > +return -EINVAL;
> > +}
> > +
> >   static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
> >   Error **errp)
> >   {
> > @@ -145,15 +178,14 @@ static int dmg_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >   s->n_chunks = 0;
> >   s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
> >
> > -/* read offset of info blocks */
> > -offset = bdrv_getlength(bs->file);
> > +/* locate the UDIF trailer */
> > +offset = dmg_find_koly_offset(bs->file);
> >   if (offset < 0) {
> >   ret = offset;
> >   goto fail;
> >   }
> > -offset -= 0x1d8;
> >
> > -ret = read_uint64(bs, offset, &info_begin);
> > +ret = read_uint64(bs, offset + 0x28, &info_begin);
> >   if (ret < 0) {
> >   goto fail;
> >   } else if (info_begin == 0) {
> >
> 
> If there really is no convenient way to retrieve the real length ...
> (Stefan: Would that be difficult to add?)
> 
> Reviewed-by: John Snow 

The real length can be stored, but it takes more effort to do so. See
the stalled work on this bdrv-getlength-conversion branch[1] and in
particular "block: do not directly set total_sectors"[2].
-- 
Kind regards,
Peter
https://lekensteyn.nl

 [1]: https://git.lekensteyn.nl/peter/qemu/log/?h=bdrv-getlength-conversion
 [2]: 
https://git.lekensteyn.nl/peter/qemu/commit/?h=bdrv-getlength-conversion&id=e5164735e5b674a10134894589a060a0f5f32ccc

Re: [Qemu-devel] [PATCH 02/10] block/dmg: extract mish block decoding functionality

2015-01-03 Thread Peter Wu 
On Friday 02 January 2015 18:59:38 John Snow wrote:
> 
> On 12/27/2014 10:01 AM, Peter Wu wrote:
> > Extract the mish block decoder such that this can be used for other
> > formats in the future. A new DmgHeaderState struct is introduced to
> > share state while decoding.
> >
> > The code is kept unchanged as much as possible, a "fail" label is added
> > for example where a simple return would probably do.
> >
> > Signed-off-by: Peter Wu 
> > ---
> >   block/dmg.c | 216 
> > +++-
> >   1 file changed, 125 insertions(+), 91 deletions(-)
> >
> > diff --git a/block/dmg.c b/block/dmg.c
> > index df274f9..6dc6dbb 100644
> > --- a/block/dmg.c
> > +++ b/block/dmg.c
> > @@ -164,19 +164,137 @@ static int64_t dmg_find_koly_offset(BlockDriverState 
> > *file_bs)
> >   return -EINVAL;
> >   }
> >
> > +/* used when building the sector table */
> > +typedef struct DmgHeaderState {
> > +/* used internally by dmg_read_mish_block to remember offsets of blocks
> > + * across calls */
> > +uint64_t last_in_offset;
> > +uint64_t last_out_offset;
> > +/* exported for dmg_open */
> > +uint32_t max_compressed_size;
> > +uint32_t max_sectors_per_chunk;
> > +} DmgHeaderState;
> > +
> > +static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds,
> > +   int64_t offset, uint32_t count)
> > +{
> > +BDRVDMGState *s = bs->opaque;
> > +uint32_t type, i;
> > +int ret;
> > +size_t new_size;
> > +uint32_t chunk_count;
> > +
> > +ret = read_uint32(bs, offset, &type);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +
> > +/* skip data that is not a valid MISH block (invalid magic or too 
> > small) */
> > +if (type != 0x6d697368 || count < 244) {
> > +/* assume success for now */
> > +return 0;
> > +}
> > +
> > +offset += 4;
> > +offset += 200;
> > +
> > +chunk_count = (count - 204) / 40;
> > +new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
> > +s->types = g_realloc(s->types, new_size / 2);
> > +s->offsets = g_realloc(s->offsets, new_size);
> > +s->lengths = g_realloc(s->lengths, new_size);
> > +s->sectors = g_realloc(s->sectors, new_size);
> > +s->sectorcounts = g_realloc(s->sectorcounts, new_size);
> > +
> > +for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
> > +ret = read_uint32(bs, offset, &s->types[i]);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +offset += 4;
> > +if (s->types[i] != 0x8005 && s->types[i] != 1 &&
> > +s->types[i] != 2) {
> > +if (s->types[i] == 0x && i > 0) {
> > +ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
> > +ds->last_out_offset = s->sectors[i - 1] +
> > +  s->sectorcounts[i - 1];
> > +}
> > +chunk_count--;
> > +i--;
> > +offset += 36;
> > +continue;
> > +}
> > +offset += 4;
> > +
> > +ret = read_uint64(bs, offset, &s->sectors[i]);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +s->sectors[i] += ds->last_out_offset;
> > +offset += 8;
> > +
> > +ret = read_uint64(bs, offset, &s->sectorcounts[i]);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +offset += 8;
> > +
> > +if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
> > +error_report("sector count %" PRIu64 " for chunk %" PRIu32
> > + " is larger than max (%u)",
> > + s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +
> > +ret = read_uint64(bs, offset, &s->offsets[i]);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +s->offsets[i] += ds->last_in_offset;
> > +offset += 8;
> > +
> > +ret = read_uint64(bs, offset, &s->lengths[i]);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +offset += 8;
> > +
> > +if (s->lengths[i] > DMG_LENGTHS_MAX) {
> > +error_report("length %" PRIu64 " for chunk %" PRIu32
> > + " is larger than max (%u)",
> > + s->lengths[i], i, DMG_LENGTHS_MAX);
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +
> > +update_max_chunk_size(s, i, &ds->max_compressed_size,
> > +  &ds->max_sectors_per_chunk);
> > +}
> > +s->n_chunks += chunk_count;
> > +return 0;
> > +
> > +fail:
> > +return ret;
> > +}
> > +
> >   static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
> >   Error **errp)
> >   {
> >   BDRVDMGState *s = bs->opaque;
> > -uint64_t info

Re: [Qemu-devel] [PATCH 03/10] block/dmg: extract processing of resource forks

2015-01-03 Thread Peter Wu 
On Friday 02 January 2015 19:01:06 John Snow wrote:
> On 12/27/2014 10:01 AM, Peter Wu wrote:
> > Besides the offset, also read the resource length. This length is now
> > used in the extracted function to verify the end of the resource fork
> > against "count" from the resource fork.
> >
> > Signed-off-by: Peter Wu 
> > ---
> >   block/dmg.c | 90 
> > -
> >   1 file changed, 59 insertions(+), 31 deletions(-)
> >
> > diff --git a/block/dmg.c b/block/dmg.c
> > index 6dc6dbb..7f49388 100644
> > --- a/block/dmg.c
> > +++ b/block/dmg.c
> > @@ -278,38 +278,13 @@ fail:
> >   return ret;
> >   }
> >
> > -static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
> > -Error **errp)
> > +static int dmg_read_resource_fork(BlockDriverState *bs, DmgHeaderState *ds,
> > +  uint64_t info_begin, uint64_t 
> > info_length)
> >   {
> > -BDRVDMGState *s = bs->opaque;
> > -DmgHeaderState ds;
> > -uint64_t info_begin, info_end;
> > -uint32_t count, tmp;
> > -int64_t offset;
> >   int ret;
> > -
> > -bs->read_only = 1;
> > -s->n_chunks = 0;
> > -s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
> > -ds.last_in_offset = 0;
> > -ds.last_out_offset = 0;
> > -ds.max_compressed_size = 1;
> > -ds.max_sectors_per_chunk = 1;
> > -
> > -/* locate the UDIF trailer */
> > -offset = dmg_find_koly_offset(bs->file);
> > -if (offset < 0) {
> > -ret = offset;
> > -goto fail;
> > -}
> > -
> > -ret = read_uint64(bs, offset + 0x28, &info_begin);
> > -if (ret < 0) {
> > -goto fail;
> > -} else if (info_begin == 0) {
> > -ret = -EINVAL;
> > -goto fail;
> > -}
> > +uint32_t count, tmp;
> > +uint64_t info_end;
> > +uint64_t offset;
> >
> >   ret = read_uint32(bs, info_begin, &tmp);
> >   if (ret < 0) {
> > @@ -326,6 +301,10 @@ static int dmg_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >   ret = -EINVAL;
> >   goto fail;
> >   }
> > +if (count > info_length) {
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> >   info_end = info_begin + count;
> >
> >   /* begin of mish block */
> > @@ -342,12 +321,61 @@ static int dmg_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >   }
> >   offset += 4;
> >
> > -ret = dmg_read_mish_block(bs, &ds, offset, count);
> > +ret = dmg_read_mish_block(bs, ds, offset, count);
> >   if (ret < 0) {
> >   goto fail;
> >   }
> >   offset += count;
> >   }
> > +return 0;
> > +
> > +fail:
> > +return ret;
> > +}
> > +
> > +static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
> > +Error **errp)
> > +{
> > +BDRVDMGState *s = bs->opaque;
> > +DmgHeaderState ds;
> > +uint64_t rsrc_fork_offset, rsrc_fork_length;
> > +int64_t offset;
> > +int ret;
> > +
> > +bs->read_only = 1;
> > +s->n_chunks = 0;
> > +s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
> > +ds.last_in_offset = 0;
> > +ds.last_out_offset = 0;
> > +ds.max_compressed_size = 1;
> > +ds.max_sectors_per_chunk = 1;
> > +
> > +/* locate the UDIF trailer */
> > +offset = dmg_find_koly_offset(bs->file);
> > +if (offset < 0) {
> > +ret = offset;
> > +goto fail;
> > +}
> > +
> > +/* offset of resource fork (RsrcForkOffset) */
> > +ret = read_uint64(bs, offset + 0x28, &rsrc_fork_offset);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +ret = read_uint64(bs, offset + 0x30, &rsrc_fork_length);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +if (rsrc_fork_offset != 0 && rsrc_fork_length != 0) {
> > +ret = dmg_read_resource_fork(bs, &ds,
> > + rsrc_fork_offset, rsrc_fork_length);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +} else {
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> >
> >   /* initialize zlib engine */
> >   s->compressed_chunk = qemu_try_blockalign(bs->file,
> >
> 
> I know your refactor hardly touches the code here, but this is a good 
> place to mention it:
> 
>  From what I can tell from the Resource Fork format
> https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151
> 
> there are four fields of interest in the header of the resourcefork 
> before we start looping trying to find mish data:
> 
> uint32_t data_offset;
> uint32_t map_offset;
> uint32_t data_length;
> uint32_t map_length;
> 
> We are treating the map which comes right after the data as the "data 
> length" which is not too far from the truth, but may include extra bytes.

The rsrc_fork_offset and rsrc_fork_length values above come from the
trailer, not

Re: [Qemu-devel] [PATCH 06/10] block/dmg: process XML plists

2015-01-03 Thread Peter Wu 
On Friday 02 January 2015 19:04:32 John Snow wrote:
> On 12/27/2014 10:01 AM, Peter Wu wrote:
> > The format is simple enough to avoid using a full-blown XML parser.
> > The offsets are based on the description at
> > http://newosxbook.com/DMG.html
> >
> > Signed-off-by: Peter Wu 
> > ---
> >   block/dmg.c | 69 
> > +
> >   1 file changed, 69 insertions(+)
> >
> > diff --git a/block/dmg.c b/block/dmg.c
> > index 19e4fe2..c03ea01 100644
> > --- a/block/dmg.c
> > +++ b/block/dmg.c
> > @@ -26,6 +26,7 @@
> >   #include "qemu/bswap.h"
> >   #include "qemu/module.h"
> >   #include 
> > +#include 
> >
> >   enum {
> >   /* Limit chunk sizes to prevent unreasonable amounts of memory being 
> > used
> > @@ -333,12 +334,66 @@ fail:
> >   return ret;
> >   }
> >
> > +static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
> > +  uint64_t info_begin, uint64_t info_length)
> > +{
> > +BDRVDMGState *s = bs->opaque;
> > +int ret;
> > +uint8_t *buffer = NULL;
> > +char *data_begin, *data_end;
> > +
> > +/* Have at least some length to avoid NULL for g_malloc. Attempt to 
> > set a
> > + * safe upper cap on the data length. A test sample had a XML length of
> > + * about 1 MiB. */
> > +if (info_length == 0 || info_length > 16 * 1024 * 1024) {
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +
> > +buffer = g_malloc(info_length + 1);
> > +buffer[info_length] = '\0';
> > +ret = bdrv_pread(bs->file, info_begin, buffer, info_length);
> > +if (ret != info_length) {
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +
> > +/* look for  The data is 284 (0x11c) bytes after 
> > base64
> > + * decode. The actual data element has 431 (0x1af) bytes which 
> > includes tabs
> > + * and line feeds. */
> > +data_end = (char *)buffer;
> > +while ((data_begin = strstr(data_end, "")) != NULL) {
> > +gsize out_len = 0;
> > +
> > +data_begin += 6;
> > +data_end = strstr(data_begin, "");
> > +/* malformed XML? */
> > +if (data_end == NULL) {
> > +ret = -EINVAL;
> > +goto fail;
> > +}
> > +*data_end++ = '\0';
> > +g_base64_decode_inplace(data_begin, &out_len);
> > +ret = dmg_read_mish_block(s, ds, (uint8_t *)data_begin,
> > +  (uint32_t)out_len);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +}
> > +ret = 0;
> > +
> > +fail:
> > +g_free(buffer);
> > +return ret;
> > +}
> > +
> 
> This starts to make me a little nervous, because we're ignoring so much 
> of the XML document structure here and just effectively performing a 
> regular search for "(.*)".
> 
> Can we guarantee that the ONLY time the data element is used in this 
> document is when it is being used in the exact context we are expecting 
> here, where it contains the b64 mish data we expect it to?
> 
> i.e. it is always in a path like this as detailed by 
> http://newosxbook.com/DMG.html :
> 
> plist/dict/key[text()='resource-fork']/following-sibling::dict/key[text()='blkx']/following-sibling::array/dict/key[text()='data']/following-sibling::data
> 
> I notice that this document says other sections MAY be present, do any 
> of them ever need to be parsed? Has anyone written about them before?
> 
> Do we know if any use data sections?
> 
> I suppose at the very least, sections of interest are always going to 
> include the "mish" magic, so that should probably keep us from doing 
> anything too stupid ...

I did not find DMG files with  elements at other locations. If it
would occur, at worst we would fail to parse a DMG file. I think that
introducing a XML parser here would introduce a risk for a minor benefit
(being prepared for future cases).

Since this is a property list, in theory people could include all kinds
of data for different keys (which would then be matched by the current
implementation). But how likely is this for a disk image?

FWIW, I looked into the dmg2img program and that also looks for the
strings "" and "". Nobody has raised a bug for that program
so far.

Do you think that it is worth to use a XML parser on potentially
insecure data? I suggest to keep it as it, and reconsider a different
approach in case a problem is encountered.

Kind regards,
Peter

> >   static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
> >   Error **errp)
> >   {
> >   BDRVDMGState *s = bs->opaque;
> >   DmgHeaderState ds;
> >   uint64_t rsrc_fork_offset, rsrc_fork_length;
> > +uint64_t plist_xml_offset, plist_xml_length;
> >   int64_t offset;
> >   int ret;
> >
> > @@ -366,12 +421,26 @@ static int dmg_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >   if (ret < 0) {
> >   goto fail;
> >   }
> > +/* offset of pr

Re: [Qemu-devel] [PATCH 08/10] block/dmg: fix sector data offset calculation

2015-01-03 Thread Peter Wu 
On Friday 02 January 2015 19:05:29 John Snow wrote:
> On 12/27/2014 10:01 AM, Peter Wu wrote:
> > This patch addresses two issues:
> >
> >   - The data fork offset was not taken into account, resulting in failure
> > to read an InstallESD.dmg file (5164763151 bytes) which had a
> > non-zero DataForkOffset field.
> >   - The offset of the previous block ("partition") was unconditionally
> > added to the current block because older files would start the input
> > offset of a new block at zero. Newer files (including vlc-2.1.5.dmg,
> > tuxpaint-0.9.15-macosx.dmg and OS X Yosemite [MAS].dmg) failed in
> > reads because these files have a continuous input offset.
> >
> 
> What does "continuous input offset" mean? This change is not as clear to 
> me, see below.

By "continuous" I mean that the new files have absolute offsets while
the offsets in older files were relative to the previous block.

> > Signed-off-by: Peter Wu 
> > ---
> >   block/dmg.c | 16 +++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/dmg.c b/block/dmg.c
> > index 984997f..93b597f 100644
> > --- a/block/dmg.c
> > +++ b/block/dmg.c
> > @@ -179,6 +179,7 @@ static int64_t dmg_find_koly_offset(BlockDriverState 
> > *file_bs)
> >   typedef struct DmgHeaderState {
> >   /* used internally by dmg_read_mish_block to remember offsets of 
> > blocks
> >* across calls */
> > +uint64_t data_fork_offset;
> >   uint64_t last_in_offset;
> >   uint64_t last_out_offset;
> >   /* exported for dmg_open */
> > @@ -194,6 +195,7 @@ static int dmg_read_mish_block(BDRVDMGState *s, 
> > DmgHeaderState *ds,
> >   size_t new_size;
> >   uint32_t chunk_count;
> >   int64_t offset = 0;
> > +uint64_t in_offset = ds->data_fork_offset;
> >
> >   type = buff_read_uint32(buffer, offset);
> >   /* skip data that is not a valid MISH block (invalid magic or too 
> > small) */
> > @@ -246,7 +248,12 @@ static int dmg_read_mish_block(BDRVDMGState *s, 
> > DmgHeaderState *ds,
> >   }
> >
> >   s->offsets[i] = buff_read_uint64(buffer, offset);
> > -s->offsets[i] += ds->last_in_offset;
> > +/* If this offset is below the previous chunk end, then assume 
> > that all
> > + * following offsets are after the previous chunks. */
> > +if (s->offsets[i] + in_offset < ds->last_in_offset) {
> > +in_offset = ds->last_in_offset;
> > +}
> > +s->offsets[i] += in_offset;
> 
> I take it that all of the offsets referenced in the mish structures are 
> relative to the start of the data fork block, which is why we are taking 
> a value from the koly block and applying it to mish block values.
> 
> correct?

Correct, the mish block describes the contents of the data fork.
http://newosxbook.com/DMG.html says:

typedef struct {
// ...
uint64_t CompressedOffset;  // Start of chunk in data fork
uint64_t CompressedLength;  // Count of bytes of chunk, in data fork
} __attribute__((__packed__)) BLKXChunkEntry;

> >   offset += 8;
> >
> >   s->lengths[i] = buff_read_uint64(buffer, offset);
> > @@ -400,6 +407,7 @@ static int dmg_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >   bs->read_only = 1;
> >   s->n_chunks = 0;
> >   s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
> > +ds.data_fork_offset = 0;
> >   ds.last_in_offset = 0;
> >   ds.last_out_offset = 0;
> >   ds.max_compressed_size = 1;
> > @@ -412,6 +420,12 @@ static int dmg_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >   goto fail;
> >   }
> >
> > +/* offset of data fork (DataForkOffset) */
> > +ret = read_uint64(bs, offset + 0x18, &ds.data_fork_offset);
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +
> >   /* offset of resource fork (RsrcForkOffset) */
> >   ret = read_uint64(bs, offset + 0x28, &rsrc_fork_offset);
> >   if (ret < 0) {
> >
> 
> A general question here:
> 
> Are we ever reading the preamble of the mish block? I see we are reading 
> the 'n' items of 40-byte chunk data, but is there a reason we skip the 
> first 200 bytes of mish data, or have I misread the documents on DMG 
> that exist?

We only use the Signature field to verify that we indeed have a BLKX
entry (required since the XML fork may yield other kind of data which
does not have this magic).

The UDIFChecksum field is 136 bytes (confirmed by a internet search).
Adding the other fields (version..reserved6 and NumberOfBlockChunks)
results in 200 (+4 for the Signature).

> It looks like there are some good fields here: SectorNumber, 
> SectorCount, DataOffset, and BlockDescriptors -- can these not be used 
> to provide a more explicit error-checking of offsets, allowing us to 
> make less assumptions about where these blocks begin and end?
> 
> Is there some reason they are unreliable?

As far as I know this is not checked b

[Qemu-devel] [PATCH 1/3] target-tricore: Fix new typos

2015-01-03 Thread Stefan Weil 
adress -> address
managment -> management

Cc: Bastian Koppelmann 
Signed-off-by: Stefan Weil 
---
 target-tricore/csfr.def  |2 +-
 target-tricore/translate.c   |2 +-
 target-tricore/tricore-opcodes.h |4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-tricore/csfr.def b/target-tricore/csfr.def
index 5b219b4..05c45dd 100644
--- a/target-tricore/csfr.def
+++ b/target-tricore/csfr.def
@@ -90,7 +90,7 @@ A(0xE200, CPM0, TRICORE_FEATURE_13)
 A(0xE280, CPM1, TRICORE_FEATURE_13)
 A(0xE300, CPM2, TRICORE_FEATURE_13)
 A(0xE380, CPM3, TRICORE_FEATURE_13)
-/* memory Managment Registers */
+/* memory management registers */
 A(0x8000, MMU_CON, TRICORE_FEATURE_13)
 A(0x8004, MMU_ASI, TRICORE_FEATURE_13)
 A(0x800C, MMU_TVA, TRICORE_FEATURE_13)
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index dbcf87e..d20922b 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -5022,7 +5022,7 @@ static void decode_32Bit_opc(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPCM_32_RR_LOGICAL_SHIFT:
 decode_rr_logical_shift(env, ctx);
 break;
-case OPCM_32_RR_ADRESS:
+case OPCM_32_RR_ADDRESS:
 decode_rr_address(env, ctx);
 break;
 case OPCM_32_RR_IDIRECT:
diff --git a/target-tricore/tricore-opcodes.h b/target-tricore/tricore-opcodes.h
index 919063e..82bd161 100644
--- a/target-tricore/tricore-opcodes.h
+++ b/target-tricore/tricore-opcodes.h
@@ -503,7 +503,7 @@ enum {
 /* RR Format */
 OPCM_32_RR_LOGICAL_SHIFT = 0x0f,
 OPCM_32_RR_ACCUMULATOR   = 0x0b,
-OPCM_32_RR_ADRESS= 0x01,
+OPCM_32_RR_ADDRESS   = 0x01,
 OPCM_32_RR_DIVIDE= 0x4b,
 OPCM_32_RR_IDIRECT   = 0x2d,
 /* RR1 Format */
@@ -1082,7 +1082,7 @@ enum {
 OPC2_32_RR_XOR_LT_U  = 0x32,
 OPC2_32_RR_XOR_NE= 0x30,
 };
-/* OPCM_32_RR_ADRESS*/
+/* OPCM_32_RR_ADDRESS   */
 enum {
 OPC2_32_RR_ADD_A = 0x01,
 OPC2_32_RR_ADDSC_A   = 0x60,
-- 
1.7.10.4

[Qemu-devel] [PATCH 0/3] Fix new typos found by codespell

2015-01-03 Thread Stefan Weil 
Happy New Year to everybody!

Regards
Stefan

[PATCH 1/3] target-tricore: Fix new typos
[PATCH 2/3] target-arm: Fix typo in comment (seperately -> separately)
[PATCH 3/3] misc: Fix new typos in comments

[Qemu-devel] [PATCH 2/3] target-arm: Fix typo in comment (seperately -> separately)

2015-01-03 Thread Stefan Weil 
Cc: Peter Maydell 
Cc: Greg Bellows 
Signed-off-by: Stefan Weil 
---
 target-arm/helper.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3ef0f1f..1a5e067 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -972,7 +972,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
  * The override is necessary because of the overly-broad TLB_LOCKDOWN
  * definition.
  */
- /* MAIR0/1 are defined seperately from their 64-bit counterpart which
+ /* MAIR0/1 are defined separately from their 64-bit counterpart which
   * allows them to assign the correct fieldoffset based on the endianness
   * handled in the field definitions.
   */
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/3] misc: Fix new typos in comments

2015-01-03 Thread Stefan Weil 
recieve -> receive
suprise -> surprise

Cc: Igor Mammedov 
Cc: John Snow 
Signed-off-by: Stefan Weil 
---
 include/hw/hotplug.h |2 +-
 tests/ahci-test.c|2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index 050d2f0..2db025d 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -52,7 +52,7 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
  *  require asynchronous unplug handling.
  * @unplug: unplug callback.
  *  Used for device removal with devices that implement
- *  asynchronous and synchronous (suprise) removal.
+ *  asynchronous and synchronous (surprise) removal.
  */
 typedef struct HotplugHandlerClass {
 /*  */
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index e77fa3a..b1a59f2 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1289,7 +1289,7 @@ static void ahci_test_identify(QPCIDevice *ahci, void 
*hba_base)
 PX_WREG(i, AHCI_PX_IS, reg);
 g_assert_cmphex(PX_RREG(i, AHCI_PX_IS), ==, 0);
 
-/* Wipe the FIS-Recieve Buffer */
+/* Wipe the FIS-Receive Buffer */
 fb = PX_RREG(i, AHCI_PX_FB);
 g_assert_cmphex(fb, !=, 0);
 qmemset(fb, 0x00, 0x100);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v5 1/3] qcow2: Add qcow2_shrink_l1_and_l2_table for qcow2 shrinking

2015-01-03 Thread Jun Li 
On Fri, 11/21 11:56, Max Reitz wrote:
> On 2014-10-26 at 16:20, Jun Li wrote:
> >This patch is the realization of new function qcow2_shrink_l1_and_l2_table.
> >This function will shrink/discard l1 and l2 table when do qcow2 shrinking.
> >
> >Signed-off-by: Jun Li 
> >---
> >v5:
> >   Do some modifications based on MAX's suggestion. Thanks for MAX.
> >   In v5, do l2 shrinking firstly, then do l1 shrinking in function 
> > qcow2_shrink_l1_and_l2_table. As do l1 shrinking need to allocate some 
> > clusters for new l1 table, so in v5 it can re-use the freed clusters come 
> > from l2 shrinking.
> >
> >v4:
> >   Add deal with COW clusters in l2 table. When using COW, some of (l2_entry 
> > >>
> >s->cluster_bits) will larger than s->refcount_table_size, so need to discard
> >this l2_entry.
> >
> >v3:
> >   Fixed host cluster leak.
> >---
> >  block/qcow2-cluster.c | 182 
> > ++
> >  block/qcow2.c |  37 +-
> >  block/qcow2.h |   2 +
> >  3 files changed, 218 insertions(+), 3 deletions(-)
> >
> >diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> >index 4d888c7..28d2d62 100644
> >--- a/block/qcow2-cluster.c
> >+++ b/block/qcow2-cluster.c
> >@@ -29,6 +29,9 @@
> >  #include "block/qcow2.h"
> >  #include "trace.h"
> >+static int l2_load(BlockDriverState *bs, uint64_t l2_offset,
> >+   uint64_t **l2_table);
> >+
> >  int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
> >  bool exact_size)
> >  {
> >@@ -135,6 +138,185 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
> >min_size,
> >  return ret;
> >  }
> >+int qcow2_shrink_l1_and_l2_table(BlockDriverState *bs, uint64_t new_l1_size,
> >+ int new_l2_index, int64_t boundary_size)
> >+{
> >+BDRVQcowState *s = bs->opaque;
> >+int new_l1_size2, ret, i;
> 
> gcc gives me a warning (which is made an error by -Werror) here due to
> uninitialized use of "ret". "ret" is not necessarily set in the while ()
> loop and there are some "goto fail"s afterwards which do not set it.
> 
> >+uint64_t *new_l1_table;
> >+int64_t new_l1_table_offset;
> >+int64_t old_l1_table_offset, old_l1_size;
> >+uint8_t data[12];
> >+uint64_t l2_offset;
> >+uint64_t *l2_table, l2_entry;
> >+int64_t l2_free_entry; /* The entry of l2 table need to free from */
> >+uint64_t *old_l1_table = s->l1_table;
> >+int num = s->l1_size - new_l1_size;
> >+
> >+assert(new_l1_size <= s->l1_size);
> >+while ((num >= -1) && (s->l1_size + num - 1 >= 0)) {
> >+l2_free_entry = 0;
> >+l2_offset = old_l1_table[s->l1_size + num - 1] & L1E_OFFSET_MASK;
> >+
> >+if (l2_offset == 0) {
> >+goto retry;
> >+}
> >+
> >+if (num == 0) {
> >+if (new_l2_index == 0) {
> >+goto retry;
> >+}
> >+l2_free_entry = new_l2_index;
> >+}
> >+
> >+/* load l2_table into cache */
> >+ret = l2_load(bs, l2_offset, &l2_table);
> >+
> >+if (ret < 0) {
> >+return ret;
> >+}
> >+
> >+for (i = s->l2_size - 1; i >= 0; i--) {
> >+l2_entry = be64_to_cpu(l2_table[i]);
> 
> & L2E_OFFSET_MASK is missing. OFLAG_COPIED will break this without.
> 
> >+
> >+/* Due to COW, the clusters in l2 table will
> >+ * not in sequential order, so there will be
> >+ * some l2_entry >= boundary_size when perform shrinking.
> >+ */
> >+if (num == -1) {
> >+if (l2_entry >= boundary_size) {
> 
> boundary_size is set to "offset" by the caller. "offset" is the guest disk
> size. l2_entry (or should be) a host offset. They are not comparable.
> 
> >+goto free_cluster;
> >+} else {
> >+continue;
> >+}
> >+}
> >+
> >+/* Deal with COW clusters in l2 table when num == 0 */
> >+if (i <= l2_free_entry - 1) {
> >+if (l2_entry >= boundary_size) {
> >+goto free_cluster;
> >+}
> >+continue;
> >+}
> >+
> >+switch (qcow2_get_cluster_type(l2_entry)) {
> >+case QCOW2_CLUSTER_UNALLOCATED:
> >+if (!bs->backing_hd) {
> >+continue;
> >+}
> >+break;
> >+
> >+case QCOW2_CLUSTER_ZERO:
> >+continue;
> >+
> >+case QCOW2_CLUSTER_NORMAL:
> >+case QCOW2_CLUSTER_COMPRESSED:
> >+break;
> >+
> >+default:
> >+abort();
> >+}
> >+
> >+free_cluster:
> >+qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
> >+
> >+if (s->qcow_version >= 3) {
> >+l2_table[i] = cpu_to_be64(QCOW_OFLAG_ZERO);
> >+} e

Re: [Qemu-devel] [PATCH] Fix irq route entries exceed KVM_MAX_IRQ_ROUTES

2015-01-03 Thread William Dauchy 
On Dec31 03:45, kevinnma(马文霜) wrote:
> diff --git a/kvm-all.c b/kvm-all.c
> index 18cc6b4..f47e1b1 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1123,6 +1123,17 @@ static int kvm_irqchip_get_virq(KVMState *s)
>  int i, bit;
>  bool retry = true;
>  
> +/*
> + * PIC and IOAPIC share the first 15 GSI numbers,available GSI
> + * numbers greater than IRQ route entries. If allocate GSI number
> + * succeeds, a new route entry can be added, so total IRQ route
> + * enties can exceed gsi_count, flush dynamic MSI entries when
> + * IRQ route entries arrive gsi_count.
> + */
> +if (!s->direct_msi && s->irq_routes->nr == s->gsi_count) {
> +kvm_flush_dynamic_msi_routes(s);
> +}
> +
>  again:
>  /* Return the lowest unused GSI in the bitmap */
>  for (i = 0; i < max_words; i++) {

Any comments on this patch?

-- 
William


signature.asc
Description: Digital signature

[Qemu-devel] [PATCH v4 2/7] qemu-iotests: Remove 091 from quick group

2015-01-03 Thread Fam Zheng 
For the purpose of allowing running quick group on tmpfs.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/group | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index a4742c6..08099b9 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -97,7 +97,7 @@
 088 rw auto quick
 089 rw auto quick
 090 rw auto quick
-091 rw auto quick
+091 rw auto
 092 rw auto quick
 095 rw auto quick
 097 rw auto backing
-- 
1.9.3

[Qemu-devel] [PATCH v4 6/7] tests/Makefile: Add check-block to make check on Linux

2015-01-03 Thread Fam Zheng 
"make check-block" does nothing on other platforms, but still takes some
time to enumerate all the tests. So let's only add it for Linux for now.

Signed-off-by: Fam Zheng 
---
 tests/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/Makefile b/tests/Makefile
index e4ddb6a..b3ee364 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -467,6 +467,9 @@ check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
 check: check-qapi-schema check-unit check-qtest
+ifeq ($(CONFIG_LINUX),y)
+check: check-block
+endif
 check-clean:
$(MAKE) -C tests/tcg clean
rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
-- 
1.9.3

[Qemu-devel] [PATCH v4 0/7] tests: Add check-block to "make check"

2015-01-03 Thread Fam Zheng 
qemu-iotests contains useful tests that have a nice coverage of block layer
code. Adding check-block (which calls tests/qemu-iotests-quick.sh) to "make
check" is good for developers' self-testing.

v4: 06: Use CONFIG_LINUX instead of custom "uname" in tests/Makefile. (Peter)

v2: Take care of other platforms, basically by keeping them unchanged, and only
add "make check-block" to "make check" on Linux. (Peter)


Fam Zheng (7):
  .gitignore: Ignore generated "common.env"
  qemu-iotests: Remove 091 from quick group
  qemu-iotests: Replace "/bin/true" with "true"
  qemu-iotests: Add "_supported_os Linux" to 058
  qemu-iotests: Speed up make check-block
  tests/Makefile: Add check-block to make check on Linux
  qemu-iotests: Add supported os parameter for python tests

 .gitignore   | 1 +
 tests/Makefile   | 3 +++
 tests/qemu-iotests-quick.sh  | 2 +-
 tests/qemu-iotests/058   | 1 +
 tests/qemu-iotests/check | 1 +
 tests/qemu-iotests/common.config | 2 +-
 tests/qemu-iotests/common.filter | 2 +-
 tests/qemu-iotests/common.rc | 2 +-
 tests/qemu-iotests/group | 2 +-
 tests/qemu-iotests/iotests.py| 5 -
 10 files changed, 15 insertions(+), 6 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v4 1/7] .gitignore: Ignore generated "common.env"

2015-01-03 Thread Fam Zheng 
Signed-off-by: Fam Zheng 
---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index e32a584..090f974 100644
--- a/.gitignore
+++ b/.gitignore
@@ -109,3 +109,4 @@ cscope.*
 tags
 TAGS
 *~
+/tests/qemu-iotests/common.env
-- 
1.9.3

[Qemu-devel] [PATCH v4 3/7] qemu-iotests: Replace "/bin/true" with "true"

2015-01-03 Thread Fam Zheng 
The former is not portable because on Mac OSX it is /usr/bin/true.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/common.config | 2 +-
 tests/qemu-iotests/common.filter | 2 +-
 tests/qemu-iotests/common.rc | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 91a5ef6..a1973ad 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -155,4 +155,4 @@ _readlink()
 }
 
 # make sure this script returns success
-/bin/true
+true
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 6c14590..a2cb9fb 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -223,4 +223,4 @@ _filter_qemu_img_map()
 }
 
 # make sure this script returns success
-/bin/true
+true
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 3b14053..aa093d9 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -490,4 +490,4 @@ _die()
 }
 
 # make sure this script returns success
-/bin/true
+true
-- 
1.9.3

[Qemu-devel] [PATCH v4 5/7] qemu-iotests: Speed up make check-block

2015-01-03 Thread Fam Zheng 
Using /tmp, which is usually mounted as tmpfs, the quick group can be
quicker.

On my laptop (Lenovo T430s with Fedora 20), this reduces the time from
50s to 30s.

Signed-off-by: Fam Zheng 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests-quick.sh | 2 +-
 tests/qemu-iotests/check| 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh
index 12af731..0e554bb 100755
--- a/tests/qemu-iotests-quick.sh
+++ b/tests/qemu-iotests-quick.sh
@@ -3,6 +3,6 @@
 cd tests/qemu-iotests
 
 ret=0
-./check -T -qcow2 -g quick || ret=1
+TEST_DIR=${TEST_DIR:-/tmp/qemu-iotests-quick-$$} ./check -T -qcow2 -g quick || 
ret=1
 
 exit $ret
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 8ca4011..baeae80 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -238,6 +238,7 @@ QEMU_NBD  -- $QEMU_NBD
 IMGFMT-- $FULL_IMGFMT_DETAILS
 IMGPROTO  -- $FULL_IMGPROTO_DETAILS
 PLATFORM  -- $FULL_HOST_DETAILS
+TEST_DIR  -- $TEST_DIR
 SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
 
 EOF
-- 
1.9.3

[Qemu-devel] [PATCH v4 7/7] qemu-iotests: Add supported os parameter for python tests

2015-01-03 Thread Fam Zheng 
If I understand correctly, qemu-iotests never meant to be portable. We
only support Linux for all the shell cases, but didn't specify it for
python tests. Now add this and default all the python tests as Linux
only. If we cares enough later, we can override the parameter in
individual cases.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/iotests.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f57f154..87002e0 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -282,12 +282,15 @@ def notrun(reason):
 print '%s not run: %s' % (seq, reason)
 sys.exit(0)
 
-def main(supported_fmts=[]):
+def main(supported_fmts=[], supported_oses=['linux']):
 '''Run tests'''
 
 if supported_fmts and (imgfmt not in supported_fmts):
 notrun('not suitable for this image format: %s' % imgfmt)
 
+if sys.platform not in supported_oses:
+notrun('not suitable for this OS: %s' % sys.platform)
+
 # We need to filter out the time taken from the output so that qemu-iotest
 # can reliably diff the results against master output.
 import StringIO
-- 
1.9.3

[Qemu-devel] [PATCH v4 4/7] qemu-iotests: Add "_supported_os Linux" to 058

2015-01-03 Thread Fam Zheng 
Other cases have this, and this test is not portable as well, as we want
to add "make check-block" to "make check", it shouldn't fail on Mac OS
X.

Reported-by: Peter Maydell 
Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/058 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index 2d5ca85..a60b34b 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -87,6 +87,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto file
+_supported_os Linux
 _require_command QEMU_NBD
 
 # Use -f raw instead of -f $IMGFMT for the NBD connection
-- 
1.9.3

Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net tap option

2015-01-03 Thread Jason Wang 

On 12/29/2014 03:38 PM, Roy Vardi wrote:
>
>  
>
>  
>
> > -Original Message-
>
> > From: Jason Wang [mailto:jasow...@redhat.com]
>
> > Sent: Tuesday, December 23, 2014 11:13 AM
>
> > To: Roy Vardi
>
> > Cc: qemu-devel@nongnu.org; stefa...@redhat.com; Noam Camus;
>
> > arm...@redhat.com; aligu...@amazon.com; lcapitul...@redhat.com
>
> > Subject: Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net
> tap option
>
> >
>
> >
>
> >
>
> > On Tue, Dec 23, 2014 at 4:44 PM, Roy Vardi  > wrote:
>
> > >
>
> > >
>
> > >>  -Original Message-
>
> > >>  From: Jason Wang [mailto:jasow...@redhat.com]
>
> > >>  Sent: Monday, December 22, 2014 8:33 AM
>
> > >>  To: Roy Vardi; qemu-devel@nongnu.org 
>
> > >>  Cc: aligu...@amazon.com ;
> arm...@redhat.com ; lcapitul...@redhat.com
> ;
>
> > >> Noam Camus; stefa...@redhat.com 
>
> > >>  Subject: Re: [Qemu-devel] [PATCH] net: Add persistent flag to -net
>
> > >> tap option
>
> > >>
>
> > >>
>
> > >>  On 12/21/2014 03:48 PM, Roy Vardi wrote:
>
> > >>  > From: Roy Vardi mailto:r...@ezchip.com>>
>
> > >>  >
>
> > >>  > Add 'persistent' boolean flag to -net tap option.
>
> > >>  > When set to off - tap interface will be released on shutdown
>
> > >>  > When set to on\not specified - tap interface will remain
>
> > >>
>
> > >>  I'm interested of the user cases in the case. Usually, persistent
>
> > >> flag was used to  let privileged application to create/configure the
>
> > >> device and then it could be  used by non-privileged users. If qemu
>
> > >> has already had privilege, why need set  persistent in this case?
>
> > >
>
> > > We're running qemu as users, non-privilege...
>
> > > Our work flow includes:
>
> > > 1. Running an internal tool for opening a persistent tap interface 2.
>
> > > Running qemu with the tap interface we got from above Our environment
>
> > > includes a lot of such qemu runs, and we want to avoid "zombie" tap
>
> > > interfaces, and we achieve it with this new flag I've added.
>
> >
>
> > I get the case, thanks for the explaining. But qemu has already had
> method to
>
> > solve this. Try downscript for tap, this external script can do
> cleanup before
>
> > closing tap fd.
>
> >
>
> > E.g. in your case, you may need to run tunctl -d.
>
>  
>
> Thanks for the reference.
>
> I've checked the downscript option, but found it unsuitable for us:
> Qemu runs the downscript before closing the fd, so a script which
> removes the interface will fail due to busy device.
>

Right, tunctl needs another TUNSETIFF ioctl(). You may want to delete it
through ip link del link dev $1 in your qemu-ifdown.
>
> I've changed the order in the qemu code so that the script is called
> after the descriptor is closed and it works for us.
>
>  
>
> What do you think about the following patch:
>
>  
>

This change behaviour which may break existing down scripts.

Thanks
>
> --- a/net/tap.c
>
> +++ b/net/tap.c
>
> @@ -296,12 +296,11 @@ static void tap_cleanup(NetClientState *nc)
>
>  qemu_purge_queued_packets(nc);
>
> -if (s->down_script[0])
>
> -launch_script(s->down_script, s->down_script_arg, s->fd);
>
> -
>
>  tap_read_poll(s, false);
>
>  tap_write_poll(s, false);
>
>  close(s->fd);
>
> +if (s->down_script[0])
>
> +launch_script(s->down_script, s->down_script_arg, s->fd);
>
>  s->fd = -1;
>
> }
>
>  
>
> ?
>
>  
>
> Thanks,
>
> Roy Vardi
>