From cc57501dee37376d0a2fbc5921e0f3a9ed4b117d Mon Sep 17 00:00:00 2001 From: Igor Mammedov Date: Mon, 19 Oct 2015 19:11:11 +0200 Subject: [PATCH 01/16] file_ram_alloc: propagate error to caller instead of terminating QEMU QEMU shouldn't exits from file_ram_alloc() if -mem-prealloc option is specified and "object_add memory-backend-file,..." fails allocation during memory hotplug. Propagate error to a caller and let it decide what to do with allocation failure. That leaves QEMU alive if it can't create backend during hotplug time and kills QEMU at startup time if backends or initial memory were misconfigured/ too large. Signed-off-by: Igor Mammedov Message-Id: <1445274671-17704-1-git-send-email-imammedo@redhat.com> Signed-off-by: Paolo Bonzini --- exec.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/exec.c b/exec.c index 8af2570579..7431f2f449 100644 --- a/exec.c +++ b/exec.c @@ -1282,10 +1282,6 @@ static void *file_ram_alloc(RAMBlock *block, return area; error: - if (mem_prealloc) { - error_report("%s", error_get_pretty(*errp)); - exit(1); - } return NULL; } #endif From aa5ccadcca3e6018ebd9d2e8b0a0604f7cb0cd59 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 20 Oct 2015 15:38:46 +0800 Subject: [PATCH 02/16] scripts/text2pod.pl: Escape left brace Latest perl now deprecates "{" literal in regex and print warnings like "unescaped left brace in regex is deprecated". Add escapes to keep it happy. Signed-off-by: Fam Zheng Message-Id: <1445326726-16031-1-git-send-email-famz@redhat.com> Signed-off-by: Paolo Bonzini --- scripts/texi2pod.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/texi2pod.pl b/scripts/texi2pod.pl index 94097fb065..8767662d30 100755 --- a/scripts/texi2pod.pl +++ b/scripts/texi2pod.pl @@ -317,7 +317,7 @@ while(<$inf>) { @columns = (); for $column (split (/\s*\@tab\s*/, $1)) { # @strong{...} is used a @headitem work-alike - $column =~ s/^\@strong{(.*)}$/$1/; + $column =~ s/^\@strong\{(.*)\}$/$1/; push @columns, $column; } $_ = "\n=item ".join (" : ", @columns)."\n"; From 54c54f8b56047d3c2420e1ae06a6a8890c220ac4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 12 Oct 2015 11:50:27 +0200 Subject: [PATCH 03/16] target-i386: fix pcmpxstrx equal-ordered (strstr) mode In this mode, referring an invalid element of the source forces the result to false (table 4-7, last column) but referring an invalid element of the destination forces the result to true, so the outer loop should still be run even if some elements of the destination will be invalid. They will be avoided in the inner loop, which correctly bounds "i" to validd, but they will still contribute to a positive outcome of the search. This fixes tst_strstr in glibc 2.17. Reported-by: Florian Weimer Cc: Richard Henderson Cc: Eduardo Habkost Signed-off-by: Paolo Bonzini --- target-i386/ops_sse.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target-i386/ops_sse.h b/target-i386/ops_sse.h index 7aa693aee9..1780d1d791 100644 --- a/target-i386/ops_sse.h +++ b/target-i386/ops_sse.h @@ -2037,10 +2037,10 @@ static inline unsigned pcmpxstrx(CPUX86State *env, Reg *d, Reg *s, } break; case 3: - for (j = valids - validd; j >= 0; j--) { + for (j = valids; j >= 0; j--) { res <<= 1; v = 1; - for (i = MIN(upper - j, validd); i >= 0; i--) { + for (i = MIN(valids - j, validd); i >= 0; i--) { v &= (pcmp_val(s, ctrl, i + j) == pcmp_val(d, ctrl, i)); } res |= v; From 6f94b7d97f7e0e486a70fb06b703442e2c04a29a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 16 Oct 2015 15:08:34 +0200 Subject: [PATCH 04/16] ioport: do not use CPU_LOG_IOPORT These messages are disabled by default; a perfect usecase for tracepoints, which in fact already exist. Add the missing information to them and stop using qemu_log_mask. Signed-off-by: Paolo Bonzini --- ioport.c | 26 ++++++-------------------- trace-events | 4 ++-- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/ioport.c b/ioport.c index e39093edb9..193ef7685c 100644 --- a/ioport.c +++ b/ioport.c @@ -30,14 +30,6 @@ #include "exec/memory.h" #include "exec/address-spaces.h" -//#define DEBUG_IOPORT - -#ifdef DEBUG_IOPORT -# define LOG_IOPORT(...) qemu_log_mask(CPU_LOG_IOPORT, ## __VA_ARGS__) -#else -# define LOG_IOPORT(...) do { } while (0) -#endif - typedef struct MemoryRegionPortioList { MemoryRegion mr; void *portio_opaque; @@ -62,8 +54,7 @@ const MemoryRegionOps unassigned_io_ops = { void cpu_outb(pio_addr_t addr, uint8_t val) { - LOG_IOPORT("outb: %04"FMT_pioaddr" %02"PRIx8"\n", addr, val); - trace_cpu_out(addr, val); + trace_cpu_out(addr, 'b', val); address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, &val, 1); } @@ -72,8 +63,7 @@ void cpu_outw(pio_addr_t addr, uint16_t val) { uint8_t buf[2]; - LOG_IOPORT("outw: %04"FMT_pioaddr" %04"PRIx16"\n", addr, val); - trace_cpu_out(addr, val); + trace_cpu_out(addr, 'w', val); stw_p(buf, val); address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 2); @@ -83,8 +73,7 @@ void cpu_outl(pio_addr_t addr, uint32_t val) { uint8_t buf[4]; - LOG_IOPORT("outl: %04"FMT_pioaddr" %08"PRIx32"\n", addr, val); - trace_cpu_out(addr, val); + trace_cpu_out(addr, 'l', val); stl_p(buf, val); address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 4); @@ -96,8 +85,7 @@ uint8_t cpu_inb(pio_addr_t addr) address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, &val, 1); - trace_cpu_in(addr, val); - LOG_IOPORT("inb : %04"FMT_pioaddr" %02"PRIx8"\n", addr, val); + trace_cpu_in(addr, 'b', val); return val; } @@ -108,8 +96,7 @@ uint16_t cpu_inw(pio_addr_t addr) address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 2); val = lduw_p(buf); - trace_cpu_in(addr, val); - LOG_IOPORT("inw : %04"FMT_pioaddr" %04"PRIx16"\n", addr, val); + trace_cpu_in(addr, 'w', val); return val; } @@ -120,8 +107,7 @@ uint32_t cpu_inl(pio_addr_t addr) address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED, buf, 4); val = ldl_p(buf); - trace_cpu_in(addr, val); - LOG_IOPORT("inl : %04"FMT_pioaddr" %08"PRIx32"\n", addr, val); + trace_cpu_in(addr, 'l', val); return val; } diff --git a/trace-events b/trace-events index bdfe79f359..89b38aa1e4 100644 --- a/trace-events +++ b/trace-events @@ -139,8 +139,8 @@ paio_submit_co(int64_t sector_num, int nb_sectors, int type) "sector_num %"PRId6 paio_submit(void *acb, void *opaque, int64_t sector_num, int nb_sectors, int type) "acb %p opaque %p sector_num %"PRId64" nb_sectors %d type %d" # ioport.c -cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u" -cpu_out(unsigned int addr, unsigned int val) "addr %#x value %u" +cpu_in(unsigned int addr, char size, unsigned int val) "addr %#x(%c) value %u" +cpu_out(unsigned int addr, char size, unsigned int val) "addr %#x(%c) value %u" # balloon.c # Since requests are raised via monitor, not many tracepoints are needed. From ddcc8e9d51c415a7b7b2983c3552408d9a50be6e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 16 Oct 2015 15:09:01 +0200 Subject: [PATCH 05/16] qemu-log: remove -d ioport It was disabled at compile-time, and is now replaced by tracepoints. Signed-off-by: Paolo Bonzini --- include/qemu/log.h | 1 - qemu-log.c | 2 -- 2 files changed, 3 deletions(-) diff --git a/include/qemu/log.h b/include/qemu/log.h index 7de45001f2..362cbc4e67 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -35,7 +35,6 @@ static inline bool qemu_log_enabled(void) #define CPU_LOG_INT (1 << 4) #define CPU_LOG_EXEC (1 << 5) #define CPU_LOG_PCALL (1 << 6) -#define CPU_LOG_IOPORT (1 << 7) #define CPU_LOG_TB_CPU (1 << 8) #define CPU_LOG_RESET (1 << 9) #define LOG_UNIMP (1 << 10) diff --git a/qemu-log.c b/qemu-log.c index efd07c81ea..7cb01a802b 100644 --- a/qemu-log.c +++ b/qemu-log.c @@ -112,8 +112,6 @@ const QEMULogItem qemu_log_items[] = { "x86 only: show protected mode far calls/returns/exceptions" }, { CPU_LOG_RESET, "cpu_reset", "show CPU state before CPU resets" }, - { CPU_LOG_IOPORT, "ioport", - "show all i/o ports accesses" }, { LOG_UNIMP, "unimp", "log unimplemented functionality" }, { LOG_GUEST_ERROR, "guest_errors", From de796d93f59d363409dfd9e186ccd64a21f92204 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 30 Oct 2015 17:36:07 -0200 Subject: [PATCH 06/16] pc: Set hw_version on all machine classes In 2012, QEMU had a bug where it exposed QEMU version information to the guest, meaning a QEMU upgrade would expose different hardware to the guest OS even if the same machine-type is being used. The bug was fixed by commit 93bfef4c6e4b23caea9d51e1099d06433d8835a4, on all machines up to pc-1.0. But we kept introducing the same bug on all newer machines since then. That means we are breaking guest ABI every time QEMU was upgraded. Fix this by setting the hw_version on all PC machines, making sure the hardware won't change when upgrading QEMU. Note that QEMU_VERSION was "1.0" in QEMU 1.0, but starting on QEMU 1.1.0, it started following the "x.y.0" pattern. We have to follow it, to make sure we use the right QEMU_VERSION string from each QEMU release. The 2.5 machine classes could have hw_version unset, because the default value for qemu_get_version() is QEMU_VERSION. But I decided to set it explicitly to QEMU_VERSION so we don't forget to update it to "2.5.0" after we release 2.5.0 and create a 2.6 machine class. Reported-by: Laszlo Ersek Reviewed-by: Laszlo Ersek Signed-off-by: Eduardo Habkost Message-Id: <1446233769-7892-2-git-send-email-ehabkost@redhat.com> Signed-off-by: Paolo Bonzini --- hw/i386/pc_piix.c | 13 +++++++++++++ hw/i386/pc_q35.c | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 393dcc4544..07d0baa0fb 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -472,6 +472,7 @@ static void pc_i440fx_machine_options(MachineClass *m) static void pc_i440fx_2_5_machine_options(MachineClass *m) { pc_i440fx_machine_options(m); + m->hw_version = QEMU_VERSION; m->alias = "pc"; m->is_default = 1; } @@ -484,6 +485,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m) { PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_i440fx_2_5_machine_options(m); + m->hw_version = "2.4.0"; m->alias = NULL; m->is_default = 0; pcmc->broken_reserved_end = true; @@ -497,6 +499,7 @@ DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL, static void pc_i440fx_2_3_machine_options(MachineClass *m) { pc_i440fx_2_4_machine_options(m); + m->hw_version = "2.3.0"; m->alias = NULL; m->is_default = 0; SET_MACHINE_COMPAT(m, PC_COMPAT_2_3); @@ -509,6 +512,7 @@ DEFINE_I440FX_MACHINE(v2_3, "pc-i440fx-2.3", pc_compat_2_3, static void pc_i440fx_2_2_machine_options(MachineClass *m) { pc_i440fx_2_3_machine_options(m); + m->hw_version = "2.2.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_2_2); } @@ -519,6 +523,7 @@ DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2, static void pc_i440fx_2_1_machine_options(MachineClass *m) { pc_i440fx_2_2_machine_options(m); + m->hw_version = "2.1.0"; m->default_display = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_2_1); } @@ -531,6 +536,7 @@ DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1, static void pc_i440fx_2_0_machine_options(MachineClass *m) { pc_i440fx_2_1_machine_options(m); + m->hw_version = "2.0.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_2_0); } @@ -541,6 +547,7 @@ DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0, static void pc_i440fx_1_7_machine_options(MachineClass *m) { pc_i440fx_2_0_machine_options(m); + m->hw_version = "1.7.0"; m->default_machine_opts = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_1_7); } @@ -552,6 +559,7 @@ DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7, static void pc_i440fx_1_6_machine_options(MachineClass *m) { pc_i440fx_1_7_machine_options(m); + m->hw_version = "1.6.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_1_6); } @@ -562,6 +570,7 @@ DEFINE_I440FX_MACHINE(v1_6, "pc-i440fx-1.6", pc_compat_1_6, static void pc_i440fx_1_5_machine_options(MachineClass *m) { pc_i440fx_1_6_machine_options(m); + m->hw_version = "1.5.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_1_5); } @@ -572,6 +581,7 @@ DEFINE_I440FX_MACHINE(v1_5, "pc-i440fx-1.5", pc_compat_1_5, static void pc_i440fx_1_4_machine_options(MachineClass *m) { pc_i440fx_1_5_machine_options(m); + m->hw_version = "1.4.0"; m->hot_add_cpu = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_1_4); } @@ -604,6 +614,7 @@ DEFINE_I440FX_MACHINE(v1_4, "pc-i440fx-1.4", pc_compat_1_4, static void pc_i440fx_1_3_machine_options(MachineClass *m) { pc_i440fx_1_4_machine_options(m); + m->hw_version = "1.3.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_1_3); } @@ -642,6 +653,7 @@ DEFINE_I440FX_MACHINE(v1_3, "pc-1.3", pc_compat_1_3, static void pc_i440fx_1_2_machine_options(MachineClass *m) { pc_i440fx_1_3_machine_options(m); + m->hw_version = "1.2.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_1_2); } @@ -684,6 +696,7 @@ DEFINE_I440FX_MACHINE(v1_2, "pc-1.2", pc_compat_1_2, static void pc_i440fx_1_1_machine_options(MachineClass *m) { pc_i440fx_1_2_machine_options(m); + m->hw_version = "1.1.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_1_1); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 2f8f3963c4..0fdae09bdb 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -373,6 +373,7 @@ static void pc_q35_machine_options(MachineClass *m) static void pc_q35_2_5_machine_options(MachineClass *m) { pc_q35_machine_options(m); + m->hw_version = QEMU_VERSION; m->alias = "q35"; } @@ -383,6 +384,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m) { PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_q35_2_5_machine_options(m); + m->hw_version = "2.4.0"; m->alias = NULL; pcmc->broken_reserved_end = true; SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); @@ -395,6 +397,7 @@ DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL, static void pc_q35_2_3_machine_options(MachineClass *m) { pc_q35_2_4_machine_options(m); + m->hw_version = "2.3.0"; m->no_floppy = 0; m->no_tco = 1; m->alias = NULL; @@ -408,6 +411,7 @@ DEFINE_Q35_MACHINE(v2_3, "pc-q35-2.3", pc_compat_2_3, static void pc_q35_2_2_machine_options(MachineClass *m) { pc_q35_2_3_machine_options(m); + m->hw_version = "2.2.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_2_2); } @@ -418,6 +422,7 @@ DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2, static void pc_q35_2_1_machine_options(MachineClass *m) { pc_q35_2_2_machine_options(m); + m->hw_version = "2.1.0"; m->default_display = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_2_1); } @@ -429,6 +434,7 @@ DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1, static void pc_q35_2_0_machine_options(MachineClass *m) { pc_q35_2_1_machine_options(m); + m->hw_version = "2.0.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_2_0); } @@ -439,6 +445,7 @@ DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0, static void pc_q35_1_7_machine_options(MachineClass *m) { pc_q35_2_0_machine_options(m); + m->hw_version = "1.7.0"; m->default_machine_opts = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_1_7); } @@ -450,6 +457,7 @@ DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_7, static void pc_q35_1_6_machine_options(MachineClass *m) { pc_q35_machine_options(m); + m->hw_version = "1.6.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_1_6); } @@ -460,6 +468,7 @@ DEFINE_Q35_MACHINE(v1_6, "pc-q35-1.6", pc_compat_1_6, static void pc_q35_1_5_machine_options(MachineClass *m) { pc_q35_1_6_machine_options(m); + m->hw_version = "1.5.0"; SET_MACHINE_COMPAT(m, PC_COMPAT_1_5); } @@ -470,6 +479,7 @@ DEFINE_Q35_MACHINE(v1_5, "pc-q35-1.5", pc_compat_1_5, static void pc_q35_1_4_machine_options(MachineClass *m) { pc_q35_1_5_machine_options(m); + m->hw_version = "1.4.0"; m->hot_add_cpu = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_1_4); } From 35c2c8dc8c0899882a8e0d349d93bd657772f1e7 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 30 Oct 2015 17:36:08 -0200 Subject: [PATCH 07/16] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version() This makes the purpose of the function clearer: it is not about the version of QEMU that's running, but the version string exposed in the emulated hardware. Cc: Andrzej Zaborowski Cc: Peter Maydell Cc: John Snow Cc: Paolo Bonzini Reviewed-by: John Snow Signed-off-by: Eduardo Habkost Message-Id: <1446233769-7892-3-git-send-email-ehabkost@redhat.com> Signed-off-by: Paolo Bonzini --- hw/arm/nseries.c | 2 +- hw/ide/core.c | 2 +- hw/scsi/scsi-bus.c | 2 +- hw/scsi/scsi-disk.c | 2 +- include/qemu/osdep.h | 4 ++-- target-i386/cpu.c | 2 +- util/osdep.c | 10 +++++----- vl.c | 2 +- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index 6a6b3e6642..2a8835ec01 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -1275,7 +1275,7 @@ static int n8x0_atag_setup(void *p, int model) strcpy((void *) w, "hw-build"); /* char component[12] */ w += 6; strcpy((void *) w, "QEMU "); - pstrcat((void *) w, 12, qemu_get_version()); /* char version[12] */ + pstrcat((void *) w, 12, qemu_hw_version()); /* char version[12] */ w += 6; tag = (model == 810) ? "1.1.10-qemu" : "1.1.6-qemu"; diff --git a/hw/ide/core.c b/hw/ide/core.c index 317406dca3..364ba21e0c 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2312,7 +2312,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind, if (version) { pstrcpy(s->version, sizeof(s->version), version); } else { - pstrcpy(s->version, sizeof(s->version), qemu_get_version()); + pstrcpy(s->version, sizeof(s->version), qemu_hw_version()); } ide_reset(s); diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index d373c1b676..fd1171e481 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -453,7 +453,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r) r->buf[7] = 0x10 | (r->req.bus->info->tcq ? 0x02 : 0); /* Sync, TCQ. */ memcpy(&r->buf[8], "QEMU ", 8); memcpy(&r->buf[16], "QEMU TARGET ", 16); - pstrcpy((char *) &r->buf[32], 4, qemu_get_version()); + pstrcpy((char *) &r->buf[32], 4, qemu_hw_version()); } return true; } diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index bada9a7f62..707e7349a2 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2315,7 +2315,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) } if (!s->version) { - s->version = g_strdup(qemu_get_version()); + s->version = g_strdup(qemu_hw_version()); } if (!s->vendor) { s->vendor = g_strdup("QEMU"); diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index b56842420e..ab2d5d9d31 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -247,8 +247,8 @@ static inline void qemu_timersub(const struct timeval *val1, void qemu_set_cloexec(int fd); -void qemu_set_version(const char *); -const char *qemu_get_version(void); +void qemu_set_hw_version(const char *); +const char *qemu_hw_version(void); void fips_set_state(bool requested); bool fips_get_state(void); diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 9280bfc7d8..9d0eedf751 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2243,7 +2243,7 @@ void x86_cpudef_setup(void) pstrcpy(def->model_id, sizeof(def->model_id), "QEMU Virtual CPU version "); pstrcat(def->model_id, sizeof(def->model_id), - qemu_get_version()); + qemu_hw_version()); break; } } diff --git a/util/osdep.c b/util/osdep.c index 0092bb61b9..80c6bfeeb2 100644 --- a/util/osdep.c +++ b/util/osdep.c @@ -52,7 +52,7 @@ extern int madvise(caddr_t, size_t, int); static bool fips_enabled = false; -static const char *qemu_version = QEMU_VERSION; +static const char *hw_version = QEMU_VERSION; int socket_set_cork(int fd, int v) { @@ -311,14 +311,14 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen) return ret; } -void qemu_set_version(const char *version) +void qemu_set_hw_version(const char *version) { - qemu_version = version; + hw_version = version; } -const char *qemu_get_version(void) +const char *qemu_hw_version(void) { - return qemu_version; + return hw_version; } void fips_set_state(bool requested) diff --git a/vl.c b/vl.c index f5f7c3f7c4..d7cdf96468 100644 --- a/vl.c +++ b/vl.c @@ -4052,7 +4052,7 @@ int main(int argc, char **argv, char **envp) cpu_exec_init_all(); if (machine_class->hw_version) { - qemu_set_version(machine_class->hw_version); + qemu_set_hw_version(machine_class->hw_version); } /* Init CPU def lists, based on config From 69fbd0ea25d1f45ab2c8b0d3f431e83063f977f2 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 30 Oct 2015 17:36:09 -0200 Subject: [PATCH 08/16] megasas: Use qemu_hw_version() instead of QEMU_VERSION Guest visible data shouldn't change with a simple QEMU upgrade, so use qemu_hw_version() to ensure it won't change (as long as the machine class being used has hw_version set). Cc: Hannes Reinecke Cc: Paolo Bonzini Cc: qemu-block@nongnu.org Reviewed-by: Hannes Reinecke Acked-by: Laszlo Ersek Signed-off-by: Eduardo Habkost Message-Id: <1446233769-7892-4-git-send-email-ehabkost@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/megasas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index dcd724e6a5..d7dc6672ec 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -757,7 +757,7 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd) memcpy(info.product_name, base_class->product_name, 24); snprintf(info.serial_number, 32, "%s", s->hba_serial); - snprintf(info.package_version, 0x60, "%s-QEMU", QEMU_VERSION); + snprintf(info.package_version, 0x60, "%s-QEMU", qemu_hw_version()); memcpy(info.image_component[0].name, "APP", 3); snprintf(info.image_component[0].version, 10, "%s-QEMU", base_class->product_version); From 680a4783dc13f1059c03d11da58193d76c19ead6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 2 Nov 2015 09:23:52 +0100 Subject: [PATCH 09/16] memory: call begin, log_start and commit when registering a new listener This ensures that cpu_reload_memory_map() is called as soon as tcg_cpu_address_space_init() is called, and before cpu->memory_dispatch is used. qemu-system-s390x never changes the address spaces after tcg_cpu_address_space_init() is called, and thus tcg_commit() is never called. This causes a SIGSEGV. Because memory_map_init() will now call mem_commit(), we have to initialize io_mem_* before address_space_memory and friends. Reported-by: Philipp Kern Reviewed-by: Peter Maydell Fixes: 0a1c71cec63e95f9b8d0dc96d049d2daa00c5210 Signed-off-by: Paolo Bonzini --- exec.c | 2 +- memory.c | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/exec.c b/exec.c index 7431f2f449..819ecc3d64 100644 --- a/exec.c +++ b/exec.c @@ -2694,8 +2694,8 @@ void cpu_register_map_client(QEMUBH *bh) void cpu_exec_init_all(void) { qemu_mutex_init(&ram_list.mutex); - memory_map_init(); io_mem_init(); + memory_map_init(); qemu_mutex_init(&map_client_list_lock); } diff --git a/memory.c b/memory.c index 2eb1597518..c435c8827a 100644 --- a/memory.c +++ b/memory.c @@ -2036,6 +2036,9 @@ static void listener_add_address_space(MemoryListener *listener, return; } + if (listener->begin) { + listener->begin(listener); + } if (global_dirty_log) { if (listener->log_global_start) { listener->log_global_start(listener); @@ -2052,10 +2055,16 @@ static void listener_add_address_space(MemoryListener *listener, .offset_within_address_space = int128_get64(fr->addr.start), .readonly = fr->readonly, }; + if (fr->dirty_log_mask && listener->log_start) { + listener->log_start(listener, §ion, 0, fr->dirty_log_mask); + } if (listener->region_add) { listener->region_add(listener, §ion); } } + if (listener->commit) { + listener->commit(listener); + } flatview_unref(view); } From 0448f5f8b816923b198ab6c32286fd1f3b2f3e45 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 26 Sep 2015 13:23:26 +0200 Subject: [PATCH 10/16] cpu-exec: Fix compiler warning (-Werror=clobbered) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reloading of local variables after sigsetjmp is only needed for some buggy compilers. The code which should reload these variables causes compiler warnings with gcc 4.7 when compiler optimizations are enabled: cpu-exec.c:204:15: error: variable ‘cpu’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] cpu-exec.c:207:15: error: variable ‘cc’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] cpu-exec.c:202:28: error: argument ‘env’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] Now this code is only used for compilers which need it (and gcc 4.5.x, x > 0 which does not need it but won't give warnings). There were bug reports for clang and gcc 4.5.0, while gcc 4.5.1 was reported to work fine without the reload code. For clang it is not clear which versions are affected, so simply keep the status quo for all clang compilations. This can be improved later. Signed-off-by: Stefan Weil Message-Id: <1443266606-21400-1-git-send-email-sw@weilnetz.de> Signed-off-by: Paolo Bonzini --- cpu-exec.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 7eef0830fe..2cfb3d0ad9 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -539,15 +539,27 @@ int cpu_exec(CPUState *cpu) only be set by a memory fault) */ } /* for(;;) */ } else { - /* Reload env after longjmp - the compiler may have smashed all - * local variables as longjmp is marked 'noreturn'. */ +#if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6) + /* Some compilers wrongly smash all local variables after + * siglongjmp. There were bug reports for gcc 4.5.0 and clang. + * Reload essential local variables here for those compilers. + * Newer versions of gcc would complain about this code (-Wclobbered). */ cpu = current_cpu; cc = CPU_GET_CLASS(cpu); - cpu->can_do_io = 1; #ifdef TARGET_I386 x86_cpu = X86_CPU(cpu); env = &x86_cpu->env; #endif +#else /* buggy compiler */ + /* Assert that the compiler does not smash local variables. */ + g_assert(cpu == current_cpu); + g_assert(cc == CPU_GET_CLASS(cpu)); +#ifdef TARGET_I386 + g_assert(x86_cpu == X86_CPU(cpu)); + g_assert(env == &x86_cpu->env); +#endif +#endif /* buggy compiler */ + cpu->can_do_io = 1; tb_lock_reset(); } } /* for(;;) */ From 5e4dfd3d4e87e0464d599ecef06aa8fe78420a9b Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 28 Oct 2015 13:56:40 -0400 Subject: [PATCH 11/16] configure: disallow ccache during compile tests If the user is using ccache during the configuration step, it may interfere with some of the configuration tests, particularly the "Is ccache interfering with macro analysis" step, which is a bit of a poetic problem. 1) Disallow ccache from reading from the cache during configure, but don't disable it entirely to allow us to see if it causes other problems. 2) Force off CCACHE_CPP2 during the ccache test to get a deterministic answer over whether or not we need to enable that feature later. Signed-off-by: John Snow Message-Id: <1446055000-29150-1-git-send-email-jsnow@redhat.com> Signed-off-by: Paolo Bonzini --- configure | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/configure b/configure index 7a1d08dc4a..9c726ebada 100755 --- a/configure +++ b/configure @@ -8,6 +8,9 @@ CLICOLOR_FORCE= GREP_OPTIONS= unset CLICOLOR_FORCE GREP_OPTIONS +# Don't allow CCACHE, if present, to use cached results of compile tests! +export CCACHE_RECACHE=yes + # Temporary directory used for files created while # configure runs. Since it is in the build directory # we can safely blow away any previous version of it @@ -4412,6 +4415,7 @@ fi # check if ccache is interfering with # semantic analysis of macros +unset CCACHE_CPP2 ccache_cpp2=no cat > $TMPC << EOF static const int Z = 1; From 8d31d6b65a7448582c7bd320fd1b8cfc6cca2720 Mon Sep 17 00:00:00 2001 From: Pavel Fedin Date: Wed, 28 Oct 2015 12:54:07 +0300 Subject: [PATCH 12/16] backends/hostmem-file: Allow to specify full pathname for backing file This allows to explicitly specify file name to use with the backend. This is important when using it together with ivshmem in order to make it backed by hugetlbfs. By default filename is autogenerated using mkstemp(), and the file is unlink()ed after creation, effectively making it anonymous. This is not very useful with ivshmem because it ends up in a memory which cannot be accessed by something else. Distinction between directory and file name is done by stat() check. If an existing directory is given, the code keeps old behavior. Otherwise it creates or opens a file with the given pathname. Signed-off-by: Pavel Fedin Tested-by: Igor Skalkin Message-Id: <004301d11166$9672fe30$c358fa90$@samsung.com> Signed-off-by: Paolo Bonzini --- exec.c | 34 +++++++++++++++++++++------------- qemu-doc.texi | 2 +- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/exec.c b/exec.c index 819ecc3d64..1e8b51b48b 100644 --- a/exec.c +++ b/exec.c @@ -1205,6 +1205,7 @@ static void *file_ram_alloc(RAMBlock *block, const char *path, Error **errp) { + struct stat st; char *filename; char *sanitized_name; char *c; @@ -1233,26 +1234,33 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } - /* Make name safe to use with mkstemp by replacing '/' with '_'. */ - sanitized_name = g_strdup(memory_region_name(block->mr)); - for (c = sanitized_name; *c != '\0'; c++) { - if (*c == '/') - *c = '_'; + if (!stat(path, &st) && S_ISDIR(st.st_mode)) { + /* Make name safe to use with mkstemp by replacing '/' with '_'. */ + sanitized_name = g_strdup(memory_region_name(block->mr)); + for (c = sanitized_name; *c != '\0'; c++) { + if (*c == '/') { + *c = '_'; + } + } + + filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, + sanitized_name); + g_free(sanitized_name); + + fd = mkstemp(filename); + if (fd >= 0) { + unlink(filename); + } + g_free(filename); + } else { + fd = open(path, O_RDWR | O_CREAT, 0644); } - filename = g_strdup_printf("%s/qemu_back_mem.%s.XXXXXX", path, - sanitized_name); - g_free(sanitized_name); - - fd = mkstemp(filename); if (fd < 0) { error_setg_errno(errp, errno, "unable to create backing store for hugepages"); - g_free(filename); goto error; } - unlink(filename); - g_free(filename); memory = ROUND_UP(memory, hpagesize); diff --git a/qemu-doc.texi b/qemu-doc.texi index 3126abdcd3..460ab716ac 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -1299,7 +1299,7 @@ Instead of specifying the using POSIX shm, you may specify a memory backend that has hugepage support: @example -qemu-system-i386 -object memory-backend-file,size=1G,mem-path=/mnt/hugepages,id=mb1 +qemu-system-i386 -object memory-backend-file,size=1G,mem-path=/mnt/hugepages/my-shmem-file,id=mb1 -device ivshmem,memdev=mb1 @end example From b553a0428014636bce4951098e97c60dc18a83ed Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 3 Nov 2015 15:43:42 -0500 Subject: [PATCH 13/16] configure: disable FORTIFY_SOURCE under clang Some versions of clang may have difficulty compiling glibc headers when -D_FORTIFY_SOURCE is used. For example, Clang++ 3.5.0-9.fc22 cannot compile glibc's stdio headers when -D_FORTIFY_SOURCE=2 is used. This manifests currently as build failures with clang and any arm target. According to LLVM dev Richard Smith, clang does not target or support FORTIFY_SOURCE + glibc, and it should not be relied on. "It's still an unsupported combination, and while it might compile, some of the checks are unlikely to work because they require a frontend inliner to be useful" See: http://lists.llvm.org/pipermail/cfe-dev/2015-November/045846.html Conclusion: disable fortify-source if we appear to be using clang instead of testing for compile success or failure, which may be incidental or not indicative of proper support of the feature. Signed-off-by: John Snow Message-Id: <1446583422-10153-1-git-send-email-jsnow@redhat.com> Signed-off-by: Paolo Bonzini --- configure | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/configure b/configure index 9c726ebada..a1ac1c7699 100755 --- a/configure +++ b/configure @@ -264,6 +264,7 @@ rdma="" gprof="no" debug_tcg="no" debug="no" +fortify_source="" strip_opt="yes" tcg_interpreter="no" bigendian="no" @@ -879,6 +880,7 @@ for opt do debug_tcg="yes" debug="yes" strip_opt="no" + fortify_source="no" ;; --enable-sparse) sparse="yes" ;; @@ -4439,6 +4441,19 @@ if ! compile_object "-Werror"; then ccache_cpp2=yes fi +################################################# +# clang does not support glibc + FORTIFY_SOURCE. + +if test "$fortify_source" != "no"; then + if echo | $cc -dM -E - | grep __clang__ > /dev/null 2>&1 ; then + fortify_source="no"; + elif echo | $cxx -dM -E - | grep __clang__ > /dev/null 2>&1 ; then + fortify_source="no"; + else + fortify_source="yes" + fi +fi + ########################################## # End of CC checks # After here, no more $cc or $ld runs @@ -4446,7 +4461,7 @@ fi if test "$gcov" = "yes" ; then CFLAGS="-fprofile-arcs -ftest-coverage -g $CFLAGS" LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS" -elif test "$debug" = "no" ; then +elif test "$fortify_source" = "yes" ; then CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS" fi From 0fd7e098db30e302d27920487f0afec33be8982a Mon Sep 17 00:00:00 2001 From: Liang Li Date: Thu, 5 Nov 2015 11:51:03 +0800 Subject: [PATCH 14/16] kvmclock: add a new function to update env->tsc. The commit 317b0a6d8 fixed an issue which caused by the outdated env->tsc value, but the fix lead to 'cpu_synchronize_all_states()' called twice during live migration. The 'cpu_synchronize_all_states()' takes about 130us for a VM which has 4 vcpus, it's a bit expensive. Synchronize the whole CPU context just for updating env->tsc is too wasting, this patch use a new function to update the env->tsc. Comparing to 'cpu_synchronize_all_states()', it only takes about 20us. Signed-off-by: Liang Li Message-Id: <1446695464-27116-2-git-send-email-liang.z.li@intel.com> Signed-off-by: Paolo Bonzini --- hw/i386/kvm/clock.c | 18 ++--------------- target-i386/kvm.c | 45 ++++++++++++++++++++++++++++++++++++++++++ target-i386/kvm_i386.h | 1 + 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c index efdf165848..0593a3f1f5 100644 --- a/hw/i386/kvm/clock.c +++ b/hw/i386/kvm/clock.c @@ -17,7 +17,7 @@ #include "qemu/host-utils.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" -#include "sysemu/cpus.h" +#include "kvm_i386.h" #include "hw/sysbus.h" #include "hw/kvm/clock.h" @@ -125,21 +125,7 @@ static void kvmclock_vm_state_change(void *opaque, int running, return; } - cpu_synchronize_all_states(); - /* In theory, the cpu_synchronize_all_states() call above wouldn't - * affect the rest of the code, as the VCPU state inside CPUState - * is supposed to always match the VCPU state on the kernel side. - * - * In practice, calling cpu_synchronize_state() too soon will load the - * kernel-side APIC state into X86CPU.apic_state too early, APIC state - * won't be reloaded later because CPUState.vcpu_dirty==true, and - * outdated APIC state may be migrated to another host. - * - * The real fix would be to make sure outdated APIC state is read - * from the kernel again when necessary. While this is not fixed, we - * need the cpu_clean_all_dirty() call below. - */ - cpu_clean_all_dirty(); + kvm_synchronize_all_tsc(); ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data); if (ret < 0) { diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 64046cb69d..2a9953b2d4 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -111,6 +111,51 @@ bool kvm_allows_irq0_override(void) return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing(); } +static int kvm_get_tsc(CPUState *cs) +{ + X86CPU *cpu = X86_CPU(cs); + CPUX86State *env = &cpu->env; + struct { + struct kvm_msrs info; + struct kvm_msr_entry entries[1]; + } msr_data; + int ret; + + if (env->tsc_valid) { + return 0; + } + + msr_data.info.nmsrs = 1; + msr_data.entries[0].index = MSR_IA32_TSC; + env->tsc_valid = !runstate_is_running(); + + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data); + if (ret < 0) { + return ret; + } + + env->tsc = msr_data.entries[0].data; + return 0; +} + +static inline void do_kvm_synchronize_tsc(void *arg) +{ + CPUState *cpu = arg; + + kvm_get_tsc(cpu); +} + +void kvm_synchronize_all_tsc(void) +{ + CPUState *cpu; + + if (kvm_enabled()) { + CPU_FOREACH(cpu) { + run_on_cpu(cpu, do_kvm_synchronize_tsc, cpu); + } + } +} + static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max) { struct kvm_cpuid2 *cpuid; diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h index e557e94f44..c1b312ba2a 100644 --- a/target-i386/kvm_i386.h +++ b/target-i386/kvm_i386.h @@ -15,6 +15,7 @@ bool kvm_allows_irq0_override(void); bool kvm_has_smm(void); +void kvm_synchronize_all_tsc(void); void kvm_arch_reset_vcpu(X86CPU *cs); void kvm_arch_do_init_vcpu(X86CPU *cs); From 6388acc853b7862761d3c2864be99033e8b62dcc Mon Sep 17 00:00:00 2001 From: Liang Li Date: Thu, 5 Nov 2015 11:51:04 +0800 Subject: [PATCH 15/16] Revert "Introduce cpu_clean_all_dirty" This reverts commit de9d61e83d43be9069e6646fa9d57a3f47779d28. Now 'cpu_clean_all_dirty' is useless, we can revert the related code. Conflicts: include/sysemu/kvm.h Signed-off-by: Liang Li Message-Id: <1446695464-27116-3-git-send-email-liang.z.li@intel.com> Signed-off-by: Paolo Bonzini --- cpus.c | 9 --------- include/sysemu/cpus.h | 1 - include/sysemu/kvm.h | 8 -------- kvm-all.c | 5 ----- 4 files changed, 23 deletions(-) diff --git a/cpus.c b/cpus.c index d2e9e4fd96..c6a5d0e5e4 100644 --- a/cpus.c +++ b/cpus.c @@ -694,15 +694,6 @@ void cpu_synchronize_all_post_init(void) } } -void cpu_clean_all_dirty(void) -{ - CPUState *cpu; - - CPU_FOREACH(cpu) { - cpu_clean_state(cpu); - } -} - static int do_vm_stop(RunState state) { int ret = 0; diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h index 30ddd1220e..3d1e5ba1e1 100644 --- a/include/sysemu/cpus.h +++ b/include/sysemu/cpus.h @@ -11,7 +11,6 @@ void cpu_stop_current(void); void cpu_synchronize_all_states(void); void cpu_synchronize_all_post_reset(void); void cpu_synchronize_all_post_init(void); -void cpu_clean_all_dirty(void); void qtest_clock_warp(int64_t dest); diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 461ef65dea..4ac6176879 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -417,7 +417,6 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, void kvm_cpu_synchronize_state(CPUState *cpu); void kvm_cpu_synchronize_post_reset(CPUState *cpu); void kvm_cpu_synchronize_post_init(CPUState *cpu); -void kvm_cpu_clean_state(CPUState *cpu); /* generic hooks - to be moved/refactored once there are more users */ @@ -442,13 +441,6 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) } } -static inline void cpu_clean_state(CPUState *cpu) -{ - if (kvm_enabled()) { - kvm_cpu_clean_state(cpu); - } -} - int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg, PCIDevice *dev); int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg, PCIDevice *dev); diff --git a/kvm-all.c b/kvm-all.c index c442838668..1bc1273772 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1766,11 +1766,6 @@ void kvm_cpu_synchronize_post_init(CPUState *cpu) run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, cpu); } -void kvm_cpu_clean_state(CPUState *cpu) -{ - cpu->kvm_vcpu_dirty = false; -} - int kvm_cpu_exec(CPUState *cpu) { struct kvm_run *run = cpu->kvm_run; From e01dd3da5cf9aa90ae844d3b86c2c2762066edac Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 5 Nov 2015 13:00:09 +0800 Subject: [PATCH 16/16] iscsi: Translate scsi sense into error code Previously we return -EIO blindly when anything goes wrong. Add a helper function to parse sense fields and try to make the return code more meaningful. This also fixes the default werror configuration (enospc) when we're using qcow2 on an iscsi lun. The old -EIO not being treated as out of space error failed to trigger vm stop. Signed-off-by: Fam Zheng Message-Id: <1446699609-11376-1-git-send-email-famz@redhat.com> [libiscsi 1.9 compatibility - Paolo] Signed-off-by: Paolo Bonzini --- block/iscsi.c | 93 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 20 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 9a628b7c66..080ef52345 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -84,6 +84,7 @@ typedef struct IscsiTask { IscsiLun *iscsilun; QEMUTimer retry_timer; bool force_next_flush; + int err_code; } IscsiTask; typedef struct IscsiAIOCB { @@ -169,19 +170,70 @@ static inline unsigned exp_random(double mean) return -mean * log((double)rand() / RAND_MAX); } -/* SCSI_STATUS_TASK_SET_FULL and SCSI_STATUS_TIMEOUT were introduced - * in libiscsi 1.10.0 as part of an enum. The LIBISCSI_API_VERSION - * macro was introduced in 1.11.0. So use the API_VERSION macro as - * a hint that the macros are defined and define them ourselves - * otherwise to keep the required libiscsi version at 1.9.0 */ -#if !defined(LIBISCSI_API_VERSION) -#define QEMU_SCSI_STATUS_TASK_SET_FULL 0x28 -#define QEMU_SCSI_STATUS_TIMEOUT 0x0f000002 -#else -#define QEMU_SCSI_STATUS_TASK_SET_FULL SCSI_STATUS_TASK_SET_FULL -#define QEMU_SCSI_STATUS_TIMEOUT SCSI_STATUS_TIMEOUT +/* SCSI_SENSE_ASCQ_INVALID_FIELD_IN_PARAMETER_LIST was introduced in + * libiscsi 1.10.0, together with other constants we need. Use it as + * a hint that we have to define them ourselves if needed, to keep the + * minimum required libiscsi version at 1.9.0. We use an ASCQ macro for + * the test because SCSI_STATUS_* is an enum. + * + * To guard against future changes where SCSI_SENSE_ASCQ_* also becomes + * an enum, check against the LIBISCSI_API_VERSION macro, which was + * introduced in 1.11.0. If it is present, there is no need to define + * anything. + */ +#if !defined(SCSI_SENSE_ASCQ_INVALID_FIELD_IN_PARAMETER_LIST) && \ + !defined(LIBISCSI_API_VERSION) +#define SCSI_STATUS_TASK_SET_FULL 0x28 +#define SCSI_STATUS_TIMEOUT 0x0f000002 +#define SCSI_SENSE_ASCQ_INVALID_FIELD_IN_PARAMETER_LIST 0x2600 +#define SCSI_SENSE_ASCQ_PARAMETER_LIST_LENGTH_ERROR 0x1a00 #endif +static int iscsi_translate_sense(struct scsi_sense *sense) +{ + int ret; + + switch (sense->key) { + case SCSI_SENSE_NOT_READY: + return -EBUSY; + case SCSI_SENSE_DATA_PROTECTION: + return -EACCES; + case SCSI_SENSE_COMMAND_ABORTED: + return -ECANCELED; + case SCSI_SENSE_ILLEGAL_REQUEST: + /* Parse ASCQ */ + break; + default: + return -EIO; + } + switch (sense->ascq) { + case SCSI_SENSE_ASCQ_PARAMETER_LIST_LENGTH_ERROR: + case SCSI_SENSE_ASCQ_INVALID_OPERATION_CODE: + case SCSI_SENSE_ASCQ_INVALID_FIELD_IN_CDB: + case SCSI_SENSE_ASCQ_INVALID_FIELD_IN_PARAMETER_LIST: + ret = -EINVAL; + break; + case SCSI_SENSE_ASCQ_LBA_OUT_OF_RANGE: + ret = -ENOSPC; + break; + case SCSI_SENSE_ASCQ_LOGICAL_UNIT_NOT_SUPPORTED: + ret = -ENOTSUP; + break; + case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT: + case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT_TRAY_CLOSED: + case SCSI_SENSE_ASCQ_MEDIUM_NOT_PRESENT_TRAY_OPEN: + ret = -ENOMEDIUM; + break; + case SCSI_SENSE_ASCQ_WRITE_PROTECTED: + ret = -EACCES; + break; + default: + ret = -EIO; + break; + } + return ret; +} + static void iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, void *command_data, void *opaque) @@ -203,11 +255,11 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, goto out; } if (status == SCSI_STATUS_BUSY || - status == QEMU_SCSI_STATUS_TIMEOUT || - status == QEMU_SCSI_STATUS_TASK_SET_FULL) { + status == SCSI_STATUS_TIMEOUT || + status == SCSI_STATUS_TASK_SET_FULL) { unsigned retry_time = exp_random(iscsi_retry_times[iTask->retries - 1]); - if (status == QEMU_SCSI_STATUS_TIMEOUT) { + if (status == SCSI_STATUS_TIMEOUT) { /* make sure the request is rescheduled AFTER the * reconnect is initiated */ retry_time = EVENT_INTERVAL * 2; @@ -226,6 +278,7 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, return; } } + iTask->err_code = iscsi_translate_sense(&task->sense); error_report("iSCSI Failure: %s", iscsi_get_error(iscsi)); } else { iTask->iscsilun->force_next_flush |= iTask->force_next_flush; @@ -455,7 +508,7 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { - return -EIO; + return iTask.err_code; } iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors); @@ -644,7 +697,7 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { - return -EIO; + return iTask.err_code; } return 0; @@ -683,7 +736,7 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { - return -EIO; + return iTask.err_code; } return 0; @@ -703,7 +756,7 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, if (status < 0) { error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s", iscsi_get_error(iscsi)); - acb->status = -EIO; + acb->status = iscsi_translate_sense(&acb->task->sense); } acb->ioh->driver_status = 0; @@ -905,7 +958,7 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { - return -EIO; + return iTask.err_code; } iscsi_allocationmap_clear(iscsilun, sector_num, nb_sectors); @@ -999,7 +1052,7 @@ retry: } if (iTask.status != SCSI_STATUS_GOOD) { - return -EIO; + return iTask.err_code; } if (flags & BDRV_REQ_MAY_UNMAP) {