* [Qemu-devel] [PATCH 71/99] target/arm: Implement vector shifted SCVF/UCVF for fp16
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Richard Henderson, Peter Maydell
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Richard Henderson <richard.henderson@linaro.org>
While we have some of the scalar paths for *CVF for fp16,
we failed to decode the fp16 version of these instructions.
Cc: qemu-stable@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20180502221552.3873-2-richard.henderson@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
(cherry picked from commit a6117fae4576edfe7a5a5b802a742c33112c0993)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
target/arm/translate-a64.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index c91329249d..f0fa6045e4 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -7159,13 +7159,26 @@ static void handle_simd_shift_intfp_conv(DisasContext *s, bool is_scalar,
int immh, int immb, int opcode,
int rn, int rd)
{
- bool is_double = extract32(immh, 3, 1);
- int size = is_double ? MO_64 : MO_32;
- int elements;
+ int size, elements, fracbits;
int immhb = immh << 3 | immb;
- int fracbits = (is_double ? 128 : 64) - immhb;
- if (!extract32(immh, 2, 2)) {
+ if (immh & 8) {
+ size = MO_64;
+ if (!is_scalar && !is_q) {
+ unallocated_encoding(s);
+ return;
+ }
+ } else if (immh & 4) {
+ size = MO_32;
+ } else if (immh & 2) {
+ size = MO_16;
+ if (!arm_dc_feature(s, ARM_FEATURE_V8_FP16)) {
+ unallocated_encoding(s);
+ return;
+ }
+ } else {
+ /* immh == 0 would be a failure of the decode logic */
+ g_assert(immh == 1);
unallocated_encoding(s);
return;
}
@@ -7173,20 +7186,14 @@ static void handle_simd_shift_intfp_conv(DisasContext *s, bool is_scalar,
if (is_scalar) {
elements = 1;
} else {
- elements = is_double ? 2 : is_q ? 4 : 2;
- if (is_double && !is_q) {
- unallocated_encoding(s);
- return;
- }
+ elements = (8 << is_q) >> size;
}
+ fracbits = (16 << size) - immhb;
if (!fp_access_check(s)) {
return;
}
- /* immh == 0 would be a failure of the decode logic */
- g_assert(immh);
-
handle_simd_intfp_conv(s, rd, rn, elements, !is_u, fracbits, size);
}
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 06/99] tcg/arm: Fix memory barrier encoding
From: Michael Roth @ 2018-07-23 20:16 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Henry Wertz, Richard Henderson
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Henry Wertz <hwertz10@gmail.com>
I found with qemu 2.11.x or newer that I would get an illegal instruction
error running some Intel binaries on my ARM chromebook. On investigation,
I found it was quitting on memory barriers.
qemu instruction:
mb $0x31
was translating as:
0x604050cc: 5bf07ff5 blpl #0x600250a8
After patch it gives:
0x604050cc: f57ff05b dmb ish
In short, I found INSN_DMB_ISH (memory barrier for ARMv7) appeared to be
correct based on online docs, but due to some endian-related shenanigans it
had to be byte-swapped to suit qemu; it appears INSN_DMB_MCR (memory
barrier for ARMv6) also should be byte swapped (and this patch does so).
I have not checked for correctness of aarch64's barrier instruction.
Cc: qemu-stable@nongnu.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Henry Wertz <hwertz10@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 3f814b803797c007abfe5c4041de754e01723031)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
tcg/arm/tcg-target.inc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
index dc83f3e5be..56a32a470f 100644
--- a/tcg/arm/tcg-target.inc.c
+++ b/tcg/arm/tcg-target.inc.c
@@ -159,8 +159,8 @@ typedef enum {
INSN_STRD_IMM = 0x004000f0,
INSN_STRD_REG = 0x000000f0,
- INSN_DMB_ISH = 0x5bf07ff5,
- INSN_DMB_MCR = 0xba0f07ee,
+ INSN_DMB_ISH = 0xf57ff05b,
+ INSN_DMB_MCR = 0xee070fba,
/* Architected nop introduced in v6k. */
/* ??? This is an MSR (imm) 0,0,0 insn. Anyone know if this
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 70/99] fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Peter Maydell
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Peter Maydell <peter.maydell@linaro.org>
In float-to-integer conversion, if the floating point input
converts exactly to the largest or smallest integer that
fits in to the result type, this is not an overflow.
In this situation we were producing the correct result value,
but were incorrectly setting the Invalid flag.
For example for Arm A64, "FCVTAS w0, d0" on an input of
0x41dfffffffc00000 should produce 0x7fffffff and set no flags.
Fix the boundary case to take the right half of the if()
statements.
This fixes a regression from 2.11 introduced by the softfloat
refactoring.
Cc: qemu-stable@nongnu.org
Fixes: ab52f973a50
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20180510140141.12120-1-peter.maydell@linaro.org
(cherry picked from commit 333583757c5e910b040bef793974773635ce1918)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
fpu/softfloat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 8401b37bd4..9bcaaebe4f 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1368,14 +1368,14 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
r = UINT64_MAX;
}
if (p.sign) {
- if (r < -(uint64_t) min) {
+ if (r <= -(uint64_t) min) {
return -r;
} else {
s->float_exception_flags = orig_flags | float_flag_invalid;
return min;
}
} else {
- if (r < max) {
+ if (r <= max) {
return r;
} else {
s->float_exception_flags = orig_flags | float_flag_invalid;
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 69/99] target/arm: Fix fp_status_f16 tininess before rounding
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Peter Maydell
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Peter Maydell <peter.maydell@linaro.org>
In commit d81ce0ef2c4f105 we added an extra float_status field
fp_status_fp16 for Arm, but forgot to initialize it correctly
by setting it to float_tininess_before_rounding. This currently
will only cause problems for the new V8_FP16 feature, since the
float-to-float conversion code doesn't use it yet. The effect
would be that we failed to set the Underflow IEEE exception flag
in all the cases where we should.
Add the missing initialization.
Fixes: d81ce0ef2c4f105
Cc: qemu-stable@nongnu.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20180512004311.9299-16-richard.henderson@linaro.org
(cherry picked from commit bcc531f0364796104df4443d17f99b5fb494eca2)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
target/arm/cpu.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 022d8c5787..7ebe3fcadf 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -311,6 +311,8 @@ static void arm_cpu_reset(CPUState *s)
&env->vfp.fp_status);
set_float_detect_tininess(float_tininess_before_rounding,
&env->vfp.standard_fp_status);
+ set_float_detect_tininess(float_tininess_before_rounding,
+ &env->vfp.fp_status_f16);
#ifndef CONFIG_USER_ONLY
if (kvm_enabled()) {
kvm_arm_reset_vcpu(cpu);
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 68/99] blockjob: expose error string via query
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, John Snow, Kevin Wolf
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: John Snow <jsnow@redhat.com>
When we've reached the concluded state, we need to expose the error
state if applicable. Add the new field.
This should be sufficient for determining if a job completed
successfully or not after concluding; if we want to discriminate
based on how it failed more mechanically, we can always add an
explicit return code enumeration later.
I didn't bother to make it only show up if we are in the concluded
state; I don't think it's necessary.
Cc: qemu-stable@nongnu.org
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit ab9ba614556ac5b0f8d96b99e0dba19f1e28d6c2)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
blockjob.c | 2 ++
qapi/block-core.json | 6 +++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/blockjob.c b/blockjob.c
index 27f957e571..4de48166b2 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -831,6 +831,8 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
info->status = job->status;
info->auto_finalize = job->auto_finalize;
info->auto_dismiss = job->auto_dismiss;
+ info->has_error = job->ret != 0;
+ info->error = job->ret ? g_strdup(strerror(-job->ret)) : NULL;
return info;
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c50517bff3..7da3bea6bc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1172,6 +1172,9 @@
# @auto-dismiss: Job will dismiss itself when CONCLUDED, moving to the NULL
# state and disappearing from the query list. (since 2.12)
#
+# @error: Error information if the job did not complete successfully.
+# Not set if the job completed successfully. (since 2.12.1)
+#
# Since: 1.1
##
{ 'struct': 'BlockJobInfo',
@@ -1179,7 +1182,8 @@
'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
'io-status': 'BlockDeviceIoStatus', 'ready': 'bool',
'status': 'BlockJobStatus',
- 'auto-finalize': 'bool', 'auto-dismiss': 'bool' } }
+ 'auto-finalize': 'bool', 'auto-dismiss': 'bool',
+ '*error': 'str' } }
##
# @query-block-jobs:
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 67/99] RISC-V: Minimal QEMU 2.12 fix for sifive_u machine
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Michael Clark, Sagar Karandikar, Bastian Koppelmann,
Palmer Dabbelt, Alistair Francis
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Michael Clark <mjc@sifive.com>
The 'sifive_u' board has a bug where the ROM is
created as RAM at the wrong address and marked
readonly. The bug renders the board unusable.
This is a minimal fix and allows booting Linux.
5aec3247c190f10654250203a1742490ae7343a2
"RISC-V: Mark ROM read-only after copying in code"
contains a comprehensive fix using the ROM APIs
memory_region_init_rom and rom_add_blob_fixed_as
which could be backported.
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Clark <mjc@sifive.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/riscv/sifive_u.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 1c2deefa6c..19b034449c 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -250,9 +250,9 @@ static void riscv_sifive_u_init(MachineState *machine)
/* boot rom */
memory_region_init_ram(boot_rom, NULL, "riscv.sifive.u.mrom",
- memmap[SIFIVE_U_MROM].base, &error_fatal);
- memory_region_set_readonly(boot_rom, true);
- memory_region_add_subregion(sys_memory, 0x0, boot_rom);
+ memmap[SIFIVE_U_MROM].size, &error_fatal);
+ memory_region_add_subregion(sys_memory, memmap[SIFIVE_U_MROM].base,
+ boot_rom);
if (machine->kernel_filename) {
load_kernel(machine->kernel_filename);
@@ -282,6 +282,7 @@ static void riscv_sifive_u_init(MachineState *machine)
qemu_fdt_dumpdtb(s->fdt, s->fdt_size);
cpu_physical_memory_write(memmap[SIFIVE_U_MROM].base +
sizeof(reset_vec), s->fdt, s->fdt_size);
+ memory_region_set_readonly(boot_rom, true);
/* MMIO */
s->plic = sifive_plic_create(memmap[SIFIVE_U_PLIC].base,
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 66/99] tcg: Limit the number of ops in a TB
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Richard Henderson
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Richard Henderson <richard.henderson@linaro.org>
In 6001f7729e12 we partially attempt to address the branch
displacement overflow caused by 15fa08f845.
However, gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vqtbX.c
is a testcase that contains a TB so large as to overflow anyway.
The limit here of 8000 ops produces a maximum output TB size of
24112 bytes on a ppc64le host with that test case. This is still
much less than the maximum forward branch distance of 32764 bytes.
Cc: qemu-stable@nongnu.org
Fixes: 15fa08f845 ("tcg: Dynamically allocate TCGOps")
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit abebf92597186be2bc48d487235da28b1127860f)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
tcg/tcg.c | 3 +++
tcg/tcg.h | 8 +++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index bb24526c93..66997cc653 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -866,6 +866,7 @@ void tcg_func_start(TCGContext *s)
/* No temps have been previously allocated for size or locality. */
memset(s->free_temps, 0, sizeof(s->free_temps));
+ s->nb_ops = 0;
s->nb_labels = 0;
s->current_frame_offset = s->frame_start;
@@ -1983,6 +1984,7 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
{
QTAILQ_REMOVE(&s->ops, op, link);
QTAILQ_INSERT_TAIL(&s->free_ops, op, link);
+ s->nb_ops--;
#ifdef CONFIG_PROFILER
atomic_set(&s->prof.del_op_count, s->prof.del_op_count + 1);
@@ -2002,6 +2004,7 @@ static TCGOp *tcg_op_alloc(TCGOpcode opc)
}
memset(op, 0, offsetof(TCGOp, link));
op->opc = opc;
+ s->nb_ops++;
return op;
}
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 30896ca304..17cf764565 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -655,6 +655,7 @@ struct TCGContext {
int nb_globals;
int nb_temps;
int nb_indirects;
+ int nb_ops;
/* goto_tb support */
tcg_insn_unit *code_buf;
@@ -844,7 +845,12 @@ static inline TCGOp *tcg_last_op(void)
/* Test for whether to terminate the TB for using too many opcodes. */
static inline bool tcg_op_buf_full(void)
{
- return false;
+ /* This is not a hard limit, it merely stops translation when
+ * we have produced "enough" opcodes. We want to limit TB size
+ * such that a RISC host can reasonably use a 16-bit signed
+ * branch within the TB.
+ */
+ return tcg_ctx->nb_ops >= 8000;
}
/* pool based memory allocation */
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 65/99] softfloat: Handle default NaN mode after pickNaNMulAdd, not before
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Peter Maydell
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Peter Maydell <peter.maydell@linaro.org>
It is implementation defined whether a multiply-add of
(0,inf,qnan) or (inf,0,qnan) raises InvalidaOperation or
not, so we let the target-specific pickNaNMulAdd function
handle this. This means that we must do the "return the
default NaN in default NaN mode" check after the call,
not before. Correct the ordering, and restore the comment
from the old propagateFloat64MulAddNaN() that warned about
this corner case.
This fixes a regression from 2.11 for Arm guests where we would
incorrectly fail to set the Invalid flag for these cases.
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Message-id: 20180504100547.14621-1-peter.maydell@linaro.org
(cherry picked from commit 1839189bbf89889076aadf0c793c1b57977b28d7)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
fpu/softfloat.c | 48 ++++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 70e0c40a1c..8401b37bd4 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -602,34 +602,42 @@ static FloatParts pick_nan(FloatParts a, FloatParts b, float_status *s)
static FloatParts pick_nan_muladd(FloatParts a, FloatParts b, FloatParts c,
bool inf_zero, float_status *s)
{
+ int which;
+
if (is_snan(a.cls) || is_snan(b.cls) || is_snan(c.cls)) {
s->float_exception_flags |= float_flag_invalid;
}
+ which = pickNaNMulAdd(is_qnan(a.cls), is_snan(a.cls),
+ is_qnan(b.cls), is_snan(b.cls),
+ is_qnan(c.cls), is_snan(c.cls),
+ inf_zero, s);
+
if (s->default_nan_mode) {
+ /* Note that this check is after pickNaNMulAdd so that function
+ * has an opportunity to set the Invalid flag.
+ */
a.cls = float_class_dnan;
- } else {
- switch (pickNaNMulAdd(is_qnan(a.cls), is_snan(a.cls),
- is_qnan(b.cls), is_snan(b.cls),
- is_qnan(c.cls), is_snan(c.cls),
- inf_zero, s)) {
- case 0:
- break;
- case 1:
- a = b;
- break;
- case 2:
- a = c;
- break;
- case 3:
- a.cls = float_class_dnan;
- return a;
- default:
- g_assert_not_reached();
- }
+ return a;
+ }
- a.cls = float_class_msnan;
+ switch (which) {
+ case 0:
+ break;
+ case 1:
+ a = b;
+ break;
+ case 2:
+ a = c;
+ break;
+ case 3:
+ a.cls = float_class_dnan;
+ return a;
+ default:
+ g_assert_not_reached();
}
+ a.cls = float_class_msnan;
+
return a;
}
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 64/99] tcg/i386: Fix dup_vec in non-AVX2 codepath
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Peter Maydell, Richard Henderson
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Peter Maydell <peter.maydell@linaro.org>
The VPUNPCKLD* instructions are all "non-destructive source",
indicated by "NDS" in the encoding string in the x86 ISA manual.
This means that they take two source operands, one of which is
encoded in the VEX.vvvv field. We were incorrectly treating them
as if they were destructive-source and passing 0 as the 'v'
argument of tcg_out_vex_modrm(). This meant we were always
using %xmm0 as one of the source operands, causing incorrect
results if the register allocator happened to want to use
something else. For instance the input AArch64 insn:
DUP v26.16b, w21
which becomes TCG IR ops:
dup_vec v128,e8,tmp2,x21
st_vec v128,e8,tmp2,env,$0xa40
was assembled to:
0x607c568c: c4 c1 7a 7e 86 e8 00 00 vmovq 0xe8(%r14), %xmm0
0x607c5694: 00
0x607c5695: c5 f9 60 c8 vpunpcklbw %xmm0, %xmm0, %xmm1
0x607c5699: c5 f9 61 c9 vpunpcklwd %xmm1, %xmm0, %xmm1
0x607c569d: c5 f9 70 c9 00 vpshufd $0, %xmm1, %xmm1
0x607c56a2: c4 c1 7a 7f 8e 40 0a 00 vmovdqu %xmm1, 0xa40(%r14)
0x607c56aa: 00
when the vpunpcklwd insn should be "%xmm1, %xmm1, %xmm1".
This resulted in our incorrectly setting the output vector to
q26=0000320000003200:0000320000003200
when given an input of x21 == 0000000002803200
rather than the expected all-zeroes.
Pass the correct source register number to tcg_out_vex_modrm()
for these insns.
Fixes: 770c2fc7bb70804a
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20180504153431.5169-1-peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
(cherry picked from commit 7eb30ef0ba2eb59e7430d4848ae8d4bf4e50f768)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
tcg/i386/tcg-target.inc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index d7e59e79c5..5357909fff 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -854,11 +854,11 @@ static void tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
switch (vece) {
case MO_8:
/* ??? With zero in a register, use PSHUFB. */
- tcg_out_vex_modrm(s, OPC_PUNPCKLBW, r, 0, a);
+ tcg_out_vex_modrm(s, OPC_PUNPCKLBW, r, a, a);
a = r;
/* FALLTHRU */
case MO_16:
- tcg_out_vex_modrm(s, OPC_PUNPCKLWD, r, 0, a);
+ tcg_out_vex_modrm(s, OPC_PUNPCKLWD, r, a, a);
a = r;
/* FALLTHRU */
case MO_32:
@@ -867,7 +867,7 @@ static void tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
tcg_out8(s, 0);
break;
case MO_64:
- tcg_out_vex_modrm(s, OPC_PUNPCKLQDQ, r, 0, a);
+ tcg_out_vex_modrm(s, OPC_PUNPCKLQDQ, r, a, a);
break;
default:
g_assert_not_reached();
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 63/99] nbd/client: Relax handling of large NBD_CMD_BLOCK_STATUS reply
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Eric Blake
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Eric Blake <eblake@redhat.com>
The NBD spec is proposing a relaxation of NBD_CMD_BLOCK_STATUS
where a server may have the final extent per context give a
length beyond the original request, if it can easily prove that
subsequent bytes have the same status, on the grounds that a
client can take advantage of this information for fewer block
status requests. Since qemu 2.12 as a client always sends
NBD_CMD_FLAG_REQ_ONE, and rejects a server that sends extra
length, the upstream NBD spec will probably limit this behavior
to clients that don't request REQ_ONE semantics; but it doesn't
hurt to relax qemu to always be permissive of this server
behavior, even if it continues to use REQ_ONE.
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180503222626.1303410-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
(cherry picked from commit acfd8f7a5f92e703d2d046cbe3d510008a697194)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
block/nbd-client.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index e7caf49fbb..8d69eaaa32 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -259,14 +259,18 @@ static int nbd_parse_blockstatus_payload(NBDClientSession *client,
if (extent->length == 0 ||
(client->info.min_block && !QEMU_IS_ALIGNED(extent->length,
- client->info.min_block)) ||
- extent->length > orig_length)
- {
+ client->info.min_block))) {
error_setg(errp, "Protocol error: server sent status chunk with "
"invalid length");
return -EINVAL;
}
+ /* The server is allowed to send us extra information on the final
+ * extent; just clamp it to the length we requested. */
+ if (extent->length > orig_length) {
+ extent->length = orig_length;
+ }
+
return 0;
}
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 62/99] riscv: requires libfdt
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, KONRAD Frederic, Michael Clark
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: KONRAD Frederic <frederic.konrad@adacore.com>
When compiling on a machine without libfdt installed the configure script
should try to get libfdt from the git or should die because otherwise
CONFIG_LIBFDT is not set and the build process end in an error in the link
phase.. eg:
hw/riscv/virt.o: In function `riscv_virt_board_init':
qemu/src/hw/riscv/virt.c:317: undefined reference to `qemu_fdt_setprop_cell'
qemu/src/hw/riscv/virt.c:319: undefined reference to `qemu_fdt_setprop_cell'
qemu/src/hw/riscv/virt.c:345: undefined reference to `qemu_fdt_dumpdtb'
collect2: error: ld returned 1 exit status
make[1]: *** [qemu-system-riscv64] Error 1
make: *** [subdir-riscv64-softmmu] Error 2
Cc: qemu-stable@nongnu.org
Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Michael Clark <mjc@sifive.com>
Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
Signed-off-by: Michael Clark <mjc@sifive.com>
Message-Id: <1525360636-18229-4-git-send-email-frederic.konrad@adacore.com>
(cherry picked from commit a666409f0df5dce113a5bd2c4c144a0792f2a4a3)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 44bf1fef04..457684a7e6 100755
--- a/configure
+++ b/configure
@@ -3734,7 +3734,7 @@ fi
fdt_required=no
for target in $target_list; do
case $target in
- aarch64*-softmmu|arm*-softmmu|ppc*-softmmu|microblaze*-softmmu|mips64el-softmmu)
+ aarch64*-softmmu|arm*-softmmu|ppc*-softmmu|microblaze*-softmmu|mips64el-softmmu|riscv*-softmmu)
fdt_required=yes
;;
esac
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 61/99] riscv: htif: increase the priority of the htif subregion
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, KONRAD Frederic, Michael Clark
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: KONRAD Frederic <frederic.konrad@adacore.com>
The htif device is supposed to be mapped over an other subregion. So increase
its priority to one to avoid any conflict.
Here is the output of info mtree:
Before:
(qemu) info mtree
address-space: memory
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000000000000f (prio 0, i/o): riscv.htif.uart
0000000000000000-0000000000011fff (prio 0, ram): riscv.spike.bootrom
0000000002000000-000000000200ffff (prio 0, i/o): riscv.sifive.clint
0000000080000000-0000000087ffffff (prio 0, ram): riscv.spike.ram
address-space: I/O
0000000000000000-000000000000ffff (prio 0, i/o): io
address-space: cpu-memory-0
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000000000000f (prio 0, i/o): riscv.htif.uart
0000000000000000-0000000000011fff (prio 0, ram): riscv.spike.bootrom
0000000002000000-000000000200ffff (prio 0, i/o): riscv.sifive.clint
0000000080000000-0000000087ffffff (prio 0, ram): riscv.spike.ram
After:
(qemu) info mtree
address-space: memory
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000000000000f (prio 1, i/o): riscv.htif.uart
0000000000000000-0000000000011fff (prio 0, ram): riscv.spike.bootrom
0000000002000000-000000000200ffff (prio 0, i/o): riscv.sifive.clint
0000000080000000-0000000087ffffff (prio 0, ram): riscv.spike.ram
address-space: I/O
0000000000000000-000000000000ffff (prio 0, i/o): io
address-space: cpu-memory-0
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000000000000f (prio 1, i/o): riscv.htif.uart
0000000000000000-0000000000011fff (prio 0, ram): riscv.spike.bootrom
0000000002000000-000000000200ffff (prio 0, i/o): riscv.sifive.clint
0000000080000000-0000000087ffffff (prio 0, ram): riscv.spike.ram
Reviewed-by: Michael Clark <mjc@sifive.com>
Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
Signed-off-by: Michael Clark <mjc@sifive.com>
Message-Id: <1525360636-18229-3-git-send-email-frederic.konrad@adacore.com>
(cherry picked from commit 6fad7d1893f6ea926063067af957009bc320406f)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/riscv/riscv_htif.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/riscv_htif.c b/hw/riscv/riscv_htif.c
index be252ec8cc..f73512941f 100644
--- a/hw/riscv/riscv_htif.c
+++ b/hw/riscv/riscv_htif.c
@@ -253,8 +253,9 @@ HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
htif_be_change, s, NULL, true);
if (address_symbol_set == 3) {
memory_region_init_io(&s->mmio, NULL, &htif_mm_ops, s,
- TYPE_HTIF_UART, size);
- memory_region_add_subregion(address_space, base, &s->mmio);
+ TYPE_HTIF_UART, size);
+ memory_region_add_subregion_overlap(address_space, base,
+ &s->mmio, 1);
}
return s;
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 05/99] s390-ccw: force diag 308 subcode to unsigned long
From: Michael Roth @ 2018-07-23 20:16 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Cornelia Huck, Thomas Huth
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Cornelia Huck <cohuck@redhat.com>
We currently pass an integer as the subcode parameter. However,
the upper bits of the register containing the subcode need to
be 0, which is not guaranteed unless we explicitly specify the
subcode to be an unsigned long value.
Fixes: d046c51dad3 ("pc-bios/s390-ccw: Get device address via diag 308/6")
Cc: qemu-stable@nongnu.org
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
(cherry picked from commit 63d8b5ace31c1e1f3996fe4cd551d6d377594d5a)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
pc-bios/s390-ccw/iplb.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 5357a36d51..ded20c834e 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -101,10 +101,11 @@ static inline bool manage_iplb(IplParameterBlock *iplb, bool store)
{
register unsigned long addr asm("0") = (unsigned long) iplb;
register unsigned long rc asm("1") = 0;
+ unsigned long subcode = store ? 6 : 5;
asm volatile ("diag %0,%2,0x308\n"
: "+d" (addr), "+d" (rc)
- : "d" (store ? 6 : 5)
+ : "d" (subcode)
: "memory", "cc");
return rc == 0x01;
}
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 60/99] riscv: spike: allow base == 0
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, KONRAD Frederic, Michael Clark
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: KONRAD Frederic <frederic.konrad@adacore.com>
The sanity check on base doesn't allow htif to be mapped @0. Check if the
symbol exists instead so we can map it where we want.
Reviewed-by: Michael Clark <mjc@sifive.com>
Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com>
Signed-off-by: Michael Clark <mjc@sifive.com>
Message-Id: <1525360636-18229-2-git-send-email-frederic.konrad@adacore.com>
(cherry picked from commit 17b9751e85b9989cc841ed387794d7f1e8aa5e46)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/riscv/riscv_htif.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/riscv_htif.c b/hw/riscv/riscv_htif.c
index 3e17f30251..be252ec8cc 100644
--- a/hw/riscv/riscv_htif.c
+++ b/hw/riscv/riscv_htif.c
@@ -41,17 +41,20 @@
} while (0)
static uint64_t fromhost_addr, tohost_addr;
+static int address_symbol_set;
void htif_symbol_callback(const char *st_name, int st_info, uint64_t st_value,
- uint64_t st_size)
+ uint64_t st_size)
{
if (strcmp("fromhost", st_name) == 0) {
+ address_symbol_set |= 1;
fromhost_addr = st_value;
if (st_size != 8) {
error_report("HTIF fromhost must be 8 bytes");
exit(1);
}
} else if (strcmp("tohost", st_name) == 0) {
+ address_symbol_set |= 2;
tohost_addr = st_value;
if (st_size != 8) {
error_report("HTIF tohost must be 8 bytes");
@@ -248,7 +251,7 @@ HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
qemu_chr_fe_init(&s->chr, chr, &error_abort);
qemu_chr_fe_set_handlers(&s->chr, htif_can_recv, htif_recv, htif_event,
htif_be_change, s, NULL, true);
- if (base) {
+ if (address_symbol_set == 3) {
memory_region_init_io(&s->mmio, NULL, &htif_mm_ops, s,
TYPE_HTIF_UART, size);
memory_region_add_subregion(address_space, base, &s->mmio);
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 59/99] iotests: Add test for cancelling a mirror job
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Max Reitz, Jeff Cody
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Max Reitz <mreitz@redhat.com>
We already have an extensive mirror test (041) which does cover
cancelling a mirror job, especially after it has emitted the READY
event. However, it does not check what exact events are emitted after
block-job-cancel is executed. More importantly, it does not use
throttling to ensure that it covers the case of block-job-cancel before
READY.
It would be possible to add this case to 041, but considering it is
already our largest test file, it makes sense to create a new file for
these cases.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180501220509.14152-3-mreitz@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
(cherry picked from commit dc885fff972c447f51572afc4c921a26b880731b)
Conflicts:
tests/qemu-iotests/group
* fix minor conflicts with test groups
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/218.out | 30 ++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 169 insertions(+)
create mode 100644 tests/qemu-iotests/218
create mode 100644 tests/qemu-iotests/218.out
diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
new file mode 100644
index 0000000000..92c331b6fb
--- /dev/null
+++ b/tests/qemu-iotests/218
@@ -0,0 +1,138 @@
+#!/usr/bin/env python
+#
+# This test covers what happens when a mirror block job is cancelled
+# in various phases of its existence.
+#
+# Note that this test only checks the emitted events (i.e.
+# BLOCK_JOB_COMPLETED vs. BLOCK_JOB_CANCELLED), it does not compare
+# whether the target is in sync with the source when the
+# BLOCK_JOB_COMPLETED event occurs. This is covered by other tests
+# (such as 041).
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# Creator/Owner: Max Reitz <mreitz@redhat.com>
+
+import iotests
+from iotests import log
+
+iotests.verify_platform(['linux'])
+
+
+# Launches the VM, adds two null-co nodes (source and target), and
+# starts a blockdev-mirror job on them.
+#
+# Either both or none of speed and buf_size must be given.
+
+def start_mirror(vm, speed=None, buf_size=None):
+ vm.launch()
+
+ ret = vm.qmp('blockdev-add',
+ node_name='source',
+ driver='null-co',
+ size=1048576)
+ assert ret['return'] == {}
+
+ ret = vm.qmp('blockdev-add',
+ node_name='target',
+ driver='null-co',
+ size=1048576)
+ assert ret['return'] == {}
+
+ if speed is not None:
+ ret = vm.qmp('blockdev-mirror',
+ job_id='mirror',
+ device='source',
+ target='target',
+ sync='full',
+ speed=speed,
+ buf_size=buf_size)
+ else:
+ ret = vm.qmp('blockdev-mirror',
+ job_id='mirror',
+ device='source',
+ target='target',
+ sync='full')
+
+ assert ret['return'] == {}
+
+
+log('')
+log('=== Cancel mirror job before convergence ===')
+log('')
+
+log('--- force=false ---')
+log('')
+
+with iotests.VM() as vm:
+ # Low speed so it does not converge
+ start_mirror(vm, 65536, 65536)
+
+ log('Cancelling job')
+ log(vm.qmp('block-job-cancel', device='mirror', force=False))
+
+ log(vm.event_wait('BLOCK_JOB_CANCELLED'),
+ filters=[iotests.filter_qmp_event])
+
+log('')
+log('--- force=true ---')
+log('')
+
+with iotests.VM() as vm:
+ # Low speed so it does not converge
+ start_mirror(vm, 65536, 65536)
+
+ log('Cancelling job')
+ log(vm.qmp('block-job-cancel', device='mirror', force=True))
+
+ log(vm.event_wait('BLOCK_JOB_CANCELLED'),
+ filters=[iotests.filter_qmp_event])
+
+
+log('')
+log('=== Cancel mirror job after convergence ===')
+log('')
+
+log('--- force=false ---')
+log('')
+
+with iotests.VM() as vm:
+ start_mirror(vm)
+
+ log(vm.event_wait('BLOCK_JOB_READY'),
+ filters=[iotests.filter_qmp_event])
+
+ log('Cancelling job')
+ log(vm.qmp('block-job-cancel', device='mirror', force=False))
+
+ log(vm.event_wait('BLOCK_JOB_COMPLETED'),
+ filters=[iotests.filter_qmp_event])
+
+log('')
+log('--- force=true ---')
+log('')
+
+with iotests.VM() as vm:
+ start_mirror(vm)
+
+ log(vm.event_wait('BLOCK_JOB_READY'),
+ filters=[iotests.filter_qmp_event])
+
+ log('Cancelling job')
+ log(vm.qmp('block-job-cancel', device='mirror', force=True))
+
+ log(vm.event_wait('BLOCK_JOB_CANCELLED'),
+ filters=[iotests.filter_qmp_event])
diff --git a/tests/qemu-iotests/218.out b/tests/qemu-iotests/218.out
new file mode 100644
index 0000000000..7dbf78e682
--- /dev/null
+++ b/tests/qemu-iotests/218.out
@@ -0,0 +1,30 @@
+
+=== Cancel mirror job before convergence ===
+
+--- force=false ---
+
+Cancelling job
+{u'return': {}}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'}
+
+--- force=true ---
+
+Cancelling job
+{u'return': {}}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'}
+
+=== Cancel mirror job after convergence ===
+
+--- force=false ---
+
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'}
+Cancelling job
+{u'return': {}}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_COMPLETED'}
+
+--- force=true ---
+
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'}
+Cancelling job
+{u'return': {}}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_CANCELLED'}
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index fc10a72192..6bb961f4a4 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -212,4 +212,5 @@
211 rw auto quick
212 rw auto quick
213 rw auto quick
+218 rw auto quick
221 rw auto quick
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 58/99] block/mirror: Make cancel always cancel pre-READY
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Max Reitz, Jeff Cody
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Max Reitz <mreitz@redhat.com>
Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 made the mirror block
job respect block-job-cancel's @force flag: With that flag set, it would
now always really cancel, even post-READY.
Unfortunately, it had a side effect: Without that flag set, it would now
never cancel, not even before READY. Considering that is an
incompatible change and not noted anywhere in the commit or the
description of block-job-cancel's @force parameter, this seems
unintentional and we should revert to the previous behavior, which is to
immediately cancel the job when block-job-cancel is called before source
and target are in sync (i.e. before the READY event).
Cc: qemu-stable@nongnu.org
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1572856
Reported-by: Yanan Fu <yfu@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180501220509.14152-2-mreitz@redhat.com
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
(cherry picked from commit eb36639f7bbc16055e551593b81365e8ae3b0b05)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
block/mirror.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/mirror.c b/block/mirror.c
index 9436a8d5ee..99da9c0858 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -874,7 +874,9 @@ static void coroutine_fn mirror_run(void *opaque)
}
trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
block_job_sleep_ns(&s->common, delay_ns);
- if (block_job_is_cancelled(&s->common) && s->common.force) {
+ if (block_job_is_cancelled(&s->common) &&
+ (!s->synced || s->common.force))
+ {
break;
}
s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 57/99] qapi: fill in CpuInfoFast.arch in query-cpus-fast
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, Laszlo Ersek, Cornelia Huck, Eric Blake,
Markus Armbruster, Viktor VM Mihajlovski
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Laszlo Ersek <lersek@redhat.com>
* Commit ca230ff33f89 added the @arch field to @CpuInfoFast, but it failed
to set the new field in qmp_query_cpus_fast(), when TARGET_S390X was not
defined. The updated @query-cpus-fast example in "qapi-schema.json"
showed "arch":"x86" only because qmp_query_cpus_fast() calls g_malloc0()
to allocate @CpuInfoFast, and the CPU_INFO_ARCH_X86 enum constant is
generated with value 0.
All @arch values other than @s390 implied the @CpuInfoOther sub-struct
for @CpuInfoFast -- at the time of writing the patch --, thus no fields
other than @arch needed to be set when TARGET_S390X was not defined. Set
@arch now, by copying the corresponding assignments from
qmp_query_cpus().
* Commit 25fa194b7b11 added the @riscv enum constant to @CpuInfoArch (used
in both @CpuInfo and @CpuInfoFast -- the return types of the @query-cpus
and @query-cpus-fast commands, respectively), and assigned, in both
return structures, the @CpuInfoRISCV sub-structure to the new enum
value.
However, qmp_query_cpus_fast() would not populate either the @arch field
or the @CpuInfoRISCV sub-structure, when TARGET_RISCV was defined; only
qmp_query_cpus() would.
Assign @CpuInfoOther to the @riscv enum constant in @CpuInfoFast, and
populate only the @arch field in qmp_query_cpus_fast(). Getting CPU
state without interrupting KVM is an exceptional thing that only S390X
does currently. Quoting Cornelia Huck <cohuck@redhat.com>, "s390x is
exceptional in that it has state in QEMU that is actually interesting
for upper layers and can be retrieved without performance penalty". See
also
<https://www.redhat.com/archives/libvir-list/2018-February/msg00121.html>.
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Viktor VM Mihajlovski <mihajlov@linux.vnet.ibm.com>
Cc: qemu-stable@nongnu.org
Fixes: ca230ff33f89bf7102cbfbc2328716da6750aaed
Fixes: 25fa194b7b11901561532e435beb83d046899f7a
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180427192852.15013-2-lersek@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
(cherry picked from commit 96054f56396eaa0b9b5c681fc3e42a0004b17ade)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
cpus.c | 16 +++++++++++++++-
qapi/misc.json | 2 +-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/cpus.c b/cpus.c
index e1d94038fd..6fa701e423 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2218,11 +2218,25 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
info->value->props = props;
}
-#if defined(TARGET_S390X)
+#if defined(TARGET_I386)
+ info->value->arch = CPU_INFO_ARCH_X86;
+#elif defined(TARGET_PPC)
+ info->value->arch = CPU_INFO_ARCH_PPC;
+#elif defined(TARGET_SPARC)
+ info->value->arch = CPU_INFO_ARCH_SPARC;
+#elif defined(TARGET_MIPS)
+ info->value->arch = CPU_INFO_ARCH_MIPS;
+#elif defined(TARGET_TRICORE)
+ info->value->arch = CPU_INFO_ARCH_TRICORE;
+#elif defined(TARGET_S390X)
s390_cpu = S390_CPU(cpu);
env = &s390_cpu->env;
info->value->arch = CPU_INFO_ARCH_S390;
info->value->u.s390.cpu_state = env->cpu_state;
+#elif defined(TARGET_RISCV)
+ info->value->arch = CPU_INFO_ARCH_RISCV;
+#else
+ info->value->arch = CPU_INFO_ARCH_OTHER;
#endif
if (!cur_item) {
head = cur_item = info;
diff --git a/qapi/misc.json b/qapi/misc.json
index 5636f4a149..104d013adb 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -573,7 +573,7 @@
'mips': 'CpuInfoOther',
'tricore': 'CpuInfoOther',
's390': 'CpuInfoS390',
- 'riscv': 'CpuInfoRISCV',
+ 'riscv': 'CpuInfoOther',
'other': 'CpuInfoOther' } }
##
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 56/99] migration/block-dirty-bitmap: fix memory leak in dirty_bitmap_load_bits
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Vladimir Sementsov-Ogievskiy, Eric Blake
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Release buf on error path too.
Bug was introduced in b35ebdf076d697bc "migration: add postcopy
migration of dirty bitmaps" with the whole function.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20180427142002.21930-3-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 16a2227893dc1d5cad78ed376ad1d7e300978fbe)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
migration/block-dirty-bitmap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index dd04f102d8..8819aabe3a 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -600,6 +600,7 @@ static int dirty_bitmap_load_bits(QEMUFile *f, DirtyBitmapLoadState *s)
ret = qemu_get_buffer(f, buf, buf_size);
if (ret != buf_size) {
error_report("Failed to read bitmap bits");
+ g_free(buf);
return -EIO;
}
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 55/99] nbd/client: fix nbd_negotiate_simple_meta_context
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Vladimir Sementsov-Ogievskiy, Eric Blake
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Initialize received variable. Otherwise, is is possible for server to
answer without any contexts, but we will set context_id to something
random (received_id is not initialized too) and return 1, which is
wrong.
To solve it, just initialize received to false. Initialize received_id
too, just to make all possible checkers happy.
Bug was introduced in 78a33ab58782efdb206de14 "nbd: BLOCK_STATUS for
standard get_block_status function: client part" with the whole
function.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20180427142002.21930-2-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 89aa0d87634e2cb98517509dc8bdb876f26ecf8b)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
nbd/client.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 3523c863fe..232ff4f46d 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -619,8 +619,8 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
{
int ret;
NBDOptionReply reply;
- uint32_t received_id;
- bool received;
+ uint32_t received_id = 0;
+ bool received = false;
uint32_t export_len = strlen(export);
uint32_t context_len = strlen(context);
uint32_t data_len = sizeof(export_len) + export_len +
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 54/99] cpus: tcg: fix never exiting loop on unplug
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Cédric Le Goater, Paolo Bonzini
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Cédric Le Goater <clg@kaod.org>
Commit 9b0605f9837b ("cpus: tcg: unregister thread with RCU, fix
exiting of loop on unplug") changed the exit condition of the loop in
the vCPU thread function but forgot to remove the beginning 'while (1)'
statement. The resulting code :
while (1) {
...
} while (!cpu->unplug || cpu_can_run(cpu));
is a sequence of two distinct two while() loops, the first not exiting
in case of an unplug event.
Remove the first while (1) to fix CPU unplug.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20180425131828.15604-1-clg@kaod.org>
Cc: qemu-stable@nongnu.org
Fixes: 9b0605f9837b68fd56c7fc7c96a3a1a3b983687d
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit 54961aac190df28d311802364d19e18d5cda8bab)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
cpus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c
index 38eba8bff3..e1d94038fd 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1648,7 +1648,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
/* process any pending work */
cpu->exit_request = 1;
- while (1) {
+ do {
if (cpu_can_run(cpu)) {
int r;
qemu_mutex_unlock_iothread();
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 53/99] block/mirror: honor ratelimit again
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Stefan Hajnoczi, Liang Li, Jeff Cody, Kevin Wolf
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 ("block/mirror: change
the semantic of 'force' of block-job-cancel") accidentally removed the
ratelimit in the mirror job.
Reintroduce the ratelimit but keep the block-job-cancel force=true
behavior that was added in commit
b76e4458b1eb3c32e9824fe6aa51f67d2b251748.
Note that block_job_sleep_ns() returns immediately when the job is
cancelled. Therefore it's safe to unconditionally call
block_job_sleep_ns() - a cancelled job does not sleep.
This commit fixes the non-deterministic qemu-iotests 185 output. The
test relies on the ratelimit to make the job sleep until the 'quit'
command is processed. Previously the job could complete before the
'quit' command was received since there was no ratelimit.
Cc: Liang Li <liliang.opensource@gmail.com>
Cc: Jeff Cody <jcody@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20180424123527.19168-1-stefanha@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
(cherry picked from commit ddc4115efdfa6619689fe18871aa2d37890b3463)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
block/mirror.c | 8 +++++---
tests/qemu-iotests/185.out | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 820f512c7b..9436a8d5ee 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -868,12 +868,14 @@ static void coroutine_fn mirror_run(void *opaque)
}
ret = 0;
+
+ if (s->synced && !should_complete) {
+ delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
+ }
trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
+ block_job_sleep_ns(&s->common, delay_ns);
if (block_job_is_cancelled(&s->common) && s->common.force) {
break;
- } else if (!should_complete) {
- delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
- block_job_sleep_ns(&s->common, delay_ns);
}
s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
}
diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out
index 2c4b04de73..992162f418 100644
--- a/tests/qemu-iotests/185.out
+++ b/tests/qemu-iotests/185.out
@@ -36,9 +36,9 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 backing_file=TEST_DIR/t.q
{"return": {}}
Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 lazy_refcounts=off refcount_bits=16
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}}
=== Start backup job and exit qemu ===
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 52/99] vnc: fix use-after-free
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Gerd Hoffmann
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Gerd Hoffmann <kraxel@redhat.com>
When vnc_client_read() return value is -1
vs is not valid any more.
Fixes: d49b87f0d1e0520443a990fc610d0f02bc63c556
Reported-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180420084820.3873-1-kraxel@redhat.com
(cherry picked from commit 1bc3117abad28d6465ecdb2c944b22943df0e4f3)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
ui/vnc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index e164eb798c..5526e54f48 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1539,13 +1539,14 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED,
VncState *vs = opaque;
if (condition & G_IO_IN) {
if (vnc_client_read(vs) < 0) {
- goto end;
+ /* vs is free()ed here */
+ return TRUE;
}
}
if (condition & G_IO_OUT) {
vnc_client_write(vs);
}
-end:
+
if (vs->disconnecting) {
if (vs->ioc_tag != 0) {
g_source_remove(vs->ioc_tag);
--
2.17.1
^ permalink raw reply related
* Re: [Qemu-devel] [PATCH v2 for-3.0] tests/libqtest: Improve kill_qemu()
From: Richard Henderson @ 2018-07-23 20:20 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: peter.maydell, mst, alex.bennee, f4bug
In-Reply-To: <20180723193530.20891-1-eblake@redhat.com>
On 07/23/2018 12:35 PM, Eric Blake wrote:
> In kill_qemu() we have an assert that checks that the QEMU process
> didn't dump core:
> assert(!WCOREDUMP(wstatus));
>
> Unfortunately the WCOREDUMP macro here means the resulting message
> is not very easy to comprehend on at least some systems:
>
> ahci-test: tests/libqtest.c:113: kill_qemu: Assertion `!(((__extension__ (((union { __typeof(wstatus) __in; int __i; }) { .__in = (wstatus) }).__i))) & 0x80)' failed.
>
> and it doesn't identify what signal the process took.
>
> Furthermore, we are NOT detecting EINTR (while EINTR shouldn't be
> happening if we didn't install signal handlers, it's still better
> to always be robust), and also want to log unexpected non-zero status
> that was not accompanied by a core dump.
>
> Instead of using a raw assert, print the information in an
> easier to understand way:
>
> /i386/ahci/sanity: tests/libqtest.c:119: kill_qemu() detected QEMU death with core dump from signal 11 (Segmentation fault)
> Aborted (core dumped)
>
> (Of course, the really useful information would be why the QEMU
> process dumped core in the first place, but we don't have that
> by the time the test program has picked up the exit status.)
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> I've taken the ideas from Peter's patch:
> https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg04430.html
> as well as fixing a related issue brought up last time this was touched:
> https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05710.html
>
> tests/libqtest.c | 39 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 37 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply
* [Qemu-devel] [PATCH 51/99] usb/dev-mtp: Fix use of uninitialized values
From: Michael Roth @ 2018-07-23 20:17 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Philippe Mathieu-Daudé, Gerd Hoffmann
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
This fixes:
hw/usb/dev-mtp.c:971:5: warning: 4th function call argument is an uninitialized value
trace_usb_mtp_op_get_partial_object(s->dev.addr, o->handle, o->path,
c->argv[1], c->argv[2]);
^~~~~~~~~~
and:
hw/usb/dev-mtp.c:981:12: warning: Assigned value is garbage or undefined
offset = c->argv[1];
^ ~~~~~~~~~~
Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180604151421.23385-3-f4bug@amsat.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
(cherry picked from commit 62713a2e50f653162387451034f1a2490e87be88)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/usb/dev-mtp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6ecf70a79b..c50d0bc15e 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1017,12 +1017,16 @@ static MTPData *usb_mtp_get_object(MTPState *s, MTPControl *c,
static MTPData *usb_mtp_get_partial_object(MTPState *s, MTPControl *c,
MTPObject *o)
{
- MTPData *d = usb_mtp_data_alloc(c);
+ MTPData *d;
off_t offset;
+ if (c->argc <= 2) {
+ return NULL;
+ }
trace_usb_mtp_op_get_partial_object(s->dev.addr, o->handle, o->path,
c->argv[1], c->argv[2]);
+ d = usb_mtp_data_alloc(c);
d->fd = open(o->path, O_RDONLY);
if (d->fd == -1) {
usb_mtp_data_free(d);
--
2.17.1
^ permalink raw reply related
* [Qemu-devel] [PATCH 04/99] nbd/client: Fix error messages during NBD_INFO_BLOCK_SIZE
From: Michael Roth @ 2018-07-23 20:16 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Eric Blake
In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com>
From: Eric Blake <eblake@redhat.com>
A missing space makes for poor error messages, and sizes can't
go negative. Also, we missed diagnosing a server that sends
a maximum block size less than the minimum.
Fixes: 081dd1fe
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180501154654.943782-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
(cherry picked from commit e475d108f1b3d3163f0affea67cdedbe5fc9752b)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
nbd/client.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index b9e175d1c2..3523c863fe 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -435,8 +435,8 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
}
be32_to_cpus(&info->min_block);
if (!is_power_of_2(info->min_block)) {
- error_setg(errp, "server minimum block size %" PRId32
- "is not a power of two", info->min_block);
+ error_setg(errp, "server minimum block size %" PRIu32
+ " is not a power of two", info->min_block);
nbd_send_opt_abort(ioc);
return -1;
}
@@ -450,8 +450,8 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
be32_to_cpus(&info->opt_block);
if (!is_power_of_2(info->opt_block) ||
info->opt_block < info->min_block) {
- error_setg(errp, "server preferred block size %" PRId32
- "is not valid", info->opt_block);
+ error_setg(errp, "server preferred block size %" PRIu32
+ " is not valid", info->opt_block);
nbd_send_opt_abort(ioc);
return -1;
}
@@ -462,6 +462,12 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
return -1;
}
be32_to_cpus(&info->max_block);
+ if (info->max_block < info->min_block) {
+ error_setg(errp, "server maximum block size %" PRIu32
+ " is not valid", info->max_block);
+ nbd_send_opt_abort(ioc);
+ return -1;
+ }
trace_nbd_opt_go_info_block_size(info->min_block, info->opt_block,
info->max_block);
break;
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.