[Qemu-devel] [PATCH RFC 13/16] hw/arm/virt: don't use smp_cpus, max_cpus

[Andrew Jones]

Use MachineState.cpus or own copy from VirtBoardInfo.cpus instead.

(Congratulations mach-virt, you're the first machine type to be
 cpu topology globals free!)

Signed-off-by: Andrew Jones 
---
 hw/arm/virt.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 78d9aa996bafc..134b6e36623ba 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -467,7 +467,7 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, 
int type, bool secure)
 
 gicdev = qdev_create(NULL, gictype);
 qdev_prop_set_uint32(gicdev, "revision", type);
-qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus);
+qdev_prop_set_uint32(gicdev, "num-cpu", vbi->cpus);
 /* Note that the num-irq property counts both internal and external
  * interrupts; there are always 32 of the former (mandated by GIC spec).
  */
@@ -488,7 +488,7 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, 
int type, bool secure)
  * appropriate GIC PPI inputs, and the GIC's IRQ output to
  * the CPU's IRQ input.
  */
-for (i = 0; i < smp_cpus; i++) {
+for (i = 0; i < vbi->cpus; i++) {
 DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
 int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
 int irq;
@@ -509,7 +509,7 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, 
int type, bool secure)
 }
 
 sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, 
ARM_CPU_IRQ));
-sysbus_connect_irq(gicbusdev, i + smp_cpus,
+sysbus_connect_irq(gicbusdev, i + vbi->cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
 }
 
@@ -1163,14 +1163,14 @@ static void machvirt_init(MachineState *machine)
 virt_max_cpus = GIC_NCPU;
 }
 
-if (max_cpus > virt_max_cpus) {
+if (machine->maxcpus > virt_max_cpus) {
 error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
  "supported by machine 'mach-virt' (%d)",
- max_cpus, virt_max_cpus);
+ machine->maxcpus, virt_max_cpus);
 exit(1);
 }
 
-vbi->cpus = smp_cpus;
+vbi->cpus = machine->cpus;
 
 if (machine->ram_size > vbi->memmap[VIRT_MEM].size) {
 error_report("mach-virt: cannot model more than %dGB RAM", 
RAMLIMIT_GB);
@@ -1196,7 +1196,7 @@ static void machvirt_init(MachineState *machine)
 
 create_fdt(vbi);
 
-for (n = 0; n < smp_cpus; n++) {
+for (n = 0; n < vbi->cpus; n++) {
 ObjectClass *oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
 CPUClass *cc = CPU_CLASS(oc);
 Object *cpuobj;
@@ -1281,7 +1281,7 @@ static void machvirt_init(MachineState *machine)
 create_fw_cfg(vbi, &address_space_memory);
 rom_set_fw(fw_cfg_find());
 
-guest_info->cpus = smp_cpus;
+guest_info->cpus = vbi->cpus;
 guest_info->fw_cfg = fw_cfg_find();
 guest_info->memmap = vbi->memmap;
 guest_info->irqmap = vbi->irqmap;
@@ -1294,7 +1294,7 @@ static void machvirt_init(MachineState *machine)
 vbi->bootinfo.kernel_filename = machine->kernel_filename;
 vbi->bootinfo.kernel_cmdline = machine->kernel_cmdline;
 vbi->bootinfo.initrd_filename = machine->initrd_filename;
-vbi->bootinfo.nb_cpus = smp_cpus;
+vbi->bootinfo.nb_cpus = vbi->cpus;
 vbi->bootinfo.board_id = -1;
 vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base;
 vbi->bootinfo.get_dtb = machvirt_dtb;
-- 
2.4.11

[Qemu-devel] [PATCH RFC 07/16] qom/cpu: make nr-cores, nr-threads real properties

[Andrew Jones]

Signed-off-by: Andrew Jones 
---
 qom/cpu.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/qom/cpu.c b/qom/cpu.c
index 751e992de8823..024cda3eb98c8 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -28,6 +28,7 @@
 #include "exec/log.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
+#include "hw/qdev-properties.h"
 
 bool cpu_exists(int64_t id)
 {
@@ -342,6 +343,12 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu)
 return cpu->cpu_index;
 }
 
+static Property cpu_common_properties[] = {
+DEFINE_PROP_INT32("nr-cores", CPUState, nr_cores, 1),
+DEFINE_PROP_INT32("nr-threads", CPUState, nr_threads, 1),
+DEFINE_PROP_END_OF_LIST()
+};
+
 static void cpu_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -367,6 +374,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 k->cpu_exec_exit = cpu_common_noop;
 k->cpu_exec_interrupt = cpu_common_exec_interrupt;
 dc->realize = cpu_common_realizefn;
+dc->props = cpu_common_properties;
 /*
  * Reason: CPUs still need special care by board code: wiring up
  * IRQs, adding reset handlers, halting non-first CPUs, ...
-- 
2.4.11

[Qemu-devel] [PATCH RFC 15/16] smbios: don't use smp_cores, smp_threads

[Andrew Jones]

SMBIOS needs cpu topology for Type4 tables, so we need to pass
it in. There are several parameters so we use a structure. There
are two callers (of non-legacy, which generates Type4 tables),
x86 and arm, so we also update both to pass the topology
parameters from their MachineState properties (directly in the
case of x86, indirectly through VirtGuestInfo in the case of arm).

Signed-off-by: Andrew Jones 
---
 hw/arm/virt.c  |  9 -
 hw/i386/pc.c   | 13 ++---
 hw/smbios/smbios.c | 20 +++-
 include/hw/smbios/smbios.h | 10 ++
 4 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 769a49aa5be77..4482fab91c139 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1067,6 +1067,13 @@ static void virt_build_smbios(VirtGuestInfo *guest_info)
 uint8_t *smbios_tables, *smbios_anchor;
 size_t smbios_tables_len, smbios_anchor_len;
 const char *product = "QEMU Virtual Machine";
+struct smbios_cpu_topology topo = {
+.sockets = guest_info->sockets,
+.cores   = guest_info->cores,
+.threads = guest_info->threads,
+.maxcpus = guest_info->maxcpus,
+.cpus= guest_info->cpus,
+};
 
 if (!fw_cfg) {
 return;
@@ -1079,7 +1086,7 @@ static void virt_build_smbios(VirtGuestInfo *guest_info)
 smbios_set_defaults("QEMU", product,
 "1.0", false, true, SMBIOS_ENTRY_POINT_30);
 
-smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
+smbios_get_tables(NULL, 0, &topo, &smbios_tables, &smbios_tables_len,
   &smbios_anchor, &smbios_anchor_len);
 
 if (smbios_anchor) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4fa86d6387ce9..afea1a535a653 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -701,12 +701,19 @@ static uint32_t x86_cpu_apic_id_from_index(MachineState 
*ms,
 }
 }
 
-static void pc_build_smbios(FWCfgState *fw_cfg)
+static void pc_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
 {
 uint8_t *smbios_tables, *smbios_anchor;
 size_t smbios_tables_len, smbios_anchor_len;
 struct smbios_phys_mem_area *mem_array;
 unsigned i, array_count;
+struct smbios_cpu_topology topo = {
+.sockets = ms->sockets,
+.cores   = ms->cores,
+.threads = ms->threads,
+.maxcpus = ms->maxcpus,
+.cpus= ms->cpus,
+};
 
 smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
 if (smbios_tables) {
@@ -725,7 +732,7 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
 array_count++;
 }
 }
-smbios_get_tables(mem_array, array_count,
+smbios_get_tables(mem_array, array_count, &topo,
   &smbios_tables, &smbios_tables_len,
   &smbios_anchor, &smbios_anchor_len);
 g_free(mem_array);
@@ -1176,7 +1183,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 
 acpi_setup();
 if (pcms->fw_cfg) {
-pc_build_smbios(pcms->fw_cfg);
+pc_build_smbios(MACHINE(pcms), pcms->fw_cfg);
 }
 }
 
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index cf18ecfd8599c..99b5f984b945a 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -20,7 +20,6 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/cpus.h"
 #include "hw/smbios/smbios.h"
 #include "hw/loader.h"
 #include "exec/cpu-common.h"
@@ -64,7 +63,9 @@ static SmbiosEntryPoint ep;
 static int smbios_type4_count = 0;
 static bool smbios_immutable;
 static bool smbios_have_defaults;
-static uint32_t smbios_cpuid_version, smbios_cpuid_features, 
smbios_smp_sockets;
+static uint32_t smbios_cpuid_version, smbios_cpuid_features;
+
+static struct smbios_cpu_topology smbios_cpu_topology;
 
 static DECLARE_BITMAP(have_binfile_bitmap, SMBIOS_MAX_TYPE+1);
 static DECLARE_BITMAP(have_fields_bitmap, SMBIOS_MAX_TYPE+1);
@@ -325,7 +326,8 @@ opts_init(smbios_register_config);
 
 static void smbios_validate_table(void)
 {
-uint32_t expect_t4_count = smbios_legacy ? smp_cpus : smbios_smp_sockets;
+uint32_t expect_t4_count = smbios_legacy ? smp_cpus
+ : smbios_cpu_topology.sockets;
 
 if (smbios_type4_count && smbios_type4_count != expect_t4_count) {
 error_report("Expected %d SMBIOS Type 4 tables, got %d instead",
@@ -637,8 +639,8 @@ static void smbios_build_type_4_table(unsigned instance)
 SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
 SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
 SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
-t->core_count = t->core_enabled = smp_cores;
-t->thread_count = smp_threads;
+t->core_count = t->core_enabled = smbios_cpu_topology.cores;
+t->thread_count = smbios_cpu_topology.threads;
 t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
 t->processor_family2 = cpu_to_le16(0x01)

[Qemu-devel] [PATCH RFC 04/16] hw/core/machine: Introduce pre_init

[Andrew Jones]

From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
Signed-off-by: Andrew Jones 
---
 hw/core/machine.c   | 6 ++
 include/hw/boards.h | 1 +
 vl.c| 1 +
 3 files changed, 8 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index ccdd5fa3e7728..3dce9020e510a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -368,10 +368,16 @@ static void machine_init_notify(Notifier *notifier, void 
*data)
 foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
 }
 
+static void machine_pre_init(MachineState *ms)
+{
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 
+mc->pre_init = machine_pre_init;
+
 /* Default 128 MB as guest ram size */
 mc->default_ram_size = 128 * M_BYTE;
 mc->rom_file_has_mr = true;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index d268bd00a9f7d..4e8dc68b07a24 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -92,6 +92,7 @@ struct MachineClass {
 const char *alias;
 const char *desc;
 
+void (*pre_init)(MachineState *state);
 void (*init)(MachineState *state);
 void (*reset)(void);
 void (*hot_add_cpu)(const int64_t id, Error **errp);
diff --git a/vl.c b/vl.c
index 8d482cb1bf020..4849dd465d667 100644
--- a/vl.c
+++ b/vl.c
@@ -4500,6 +4500,7 @@ int main(int argc, char **argv, char **envp)
 current_machine->boot_order = boot_order;
 current_machine->cpu_model = cpu_model;
 
+machine_class->pre_init(current_machine);
 machine_class->init(current_machine);
 
 realtime_init();
-- 
2.4.11

[Qemu-devel] [PATCH RFC 03/16] hw/smbios/smbios: fix number of sockets calculation

[Andrew Jones]

The specification "sect. 7.5 Processor Information (Type 4)" says
 "NOTE One structure is provided for each processor instance in a
  system. For example, a system that supports up to two processors
  includes two Processor Information structures - even if only one
  processor is currently installed..."

We should use max_cpus in the calculation. The rounding is still
necessary, since smp_cores and smp_threads may have been calculated
based on smp_cpus, rather than max_cpus. The rounding is safe,
because smp_parse will fail when the result produces a topology
supporting more cpus than max_cpus.

Signed-off-by: Andrew Jones 
---
 hw/smbios/smbios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index cb8a029cf..cf18ecfd8599c 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -881,7 +881,7 @@ void smbios_get_tables(const struct smbios_phys_mem_area 
*mem_array,
 smbios_build_type_2_table();
 smbios_build_type_3_table();
 
-smbios_smp_sockets = DIV_ROUND_UP(smp_cpus, smp_cores * smp_threads);
+smbios_smp_sockets = DIV_ROUND_UP(max_cpus, smp_cores * smp_threads);
 assert(smbios_smp_sockets >= 1);
 
 for (i = 0; i < smbios_smp_sockets; i++) {
-- 
2.4.11

Re: [Qemu-devel] [PATCH] configure: Rename CONFIG_QGA_NTDDDISK into CONFIG_QGA_NTDDSCSI

[Michael Roth]

Quoting Thomas Huth (2016-06-10 10:25:54)
> There is no CONFIG_QGA_NTDDDISK define used anywhere in the QEMU
> sources. Looking at the changelog and qga/commands-win32.c, it
> seems like this should be called CONFIG_QGA_NTDDSCSI instead.
> 
> Signed-off-by: Thomas Huth 

Yikes, this does appear to be the case. Need to recheck the original
functionality since apparently it's been disabled ever since I added
this patch to the original series, but can take this in through my
tree.

Cc'ing qemu-stable

> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 8c2f90b..55019d6 100755
> --- a/configure
> +++ b/configure
> @@ -4965,7 +4965,7 @@ if test "$mingw32" = "yes" ; then
>  echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak
>fi
>if test "$guest_agent_ntddscsi" = "yes" ; then
> -echo "CONFIG_QGA_NTDDDISK=y" >> $config_host_mak
> +echo "CONFIG_QGA_NTDDSCSI=y" >> $config_host_mak
>fi
>if test "$guest_agent_msi" = "yes"; then
>  echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak  
> -- 
> 1.8.3.1
> 

[Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init

[Andrew Jones]

Move the guts of smp_parse() into hw/core/machine.c to operate on
smp machine properties, and to eventually allow it to be overridden
by machines. We leave the smp_parse function behind to handle the
(now deprecated) -smp option, but now it only needs to set the
machine properties.

Signed-off-by: Andrew Jones 
---
 hw/core/machine.c | 113 --
 vl.c  | 111 -
 2 files changed, 142 insertions(+), 82 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2625044002e57..75c5a1fdd7de1 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -17,6 +17,7 @@
 #include "qapi/visitor.h"
 #include "hw/sysbus.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/replay.h"
 #include "qemu/error-report.h"
 #include "qemu/cutils.h"
 
@@ -417,16 +418,122 @@ static void machine_init_notify(Notifier *notifier, void 
*data)
 
 static void machine_set_smp_parameters(MachineState *ms)
 {
-if (ms->sockets != -1 || ms->cores != -1 || ms->threads != -1 ||
-ms->maxcpus != -1 || ms->cpus != -1) {
+int sockets = ms->sockets;
+int cores   = ms->cores;
+int threads = ms->threads;
+int maxcpus = ms->maxcpus;
+int cpus= ms->cpus;
+bool sockets_input = sockets > 0;
+
+if (sockets == -1 && cores == -1 && threads == -1 &&
+maxcpus == -1 && cpus == -1) {
+ms->sockets = 1;
+ms->cores   = 1;
+ms->threads = 1;
+ms->maxcpus = 1;
+ms->cpus= 1;
+return;
+}
+
+if (sockets == -1 || cores == -1 || threads == -1 ||
+maxcpus == -1 || cpus == -1) {
+error_report("cpu topology: "
+ "all machine properties must be specified");
+exit(1);
+}
+
+/* If the deprecated -smp option was used without complete input,
+ * or a user input zeros (why would they do that?), then we compute
+ * missing values, preferring sockets over cores over threads.
+ */
+if (cpus == 0 || sockets == 0) {
+sockets = sockets > 0 ? sockets : 1;
+cores = cores > 0 ? cores : 1;
+threads = threads > 0 ? threads : 1;
+if (cpus == 0) {
+cpus = cores * threads * sockets;
+}
+} else if (cores == 0) {
+threads = threads > 0 ? threads : 1;
+cores = cpus / (sockets * threads);
+} else if (threads == 0) {
+threads = cpus / (cores * sockets);
+} else if (sockets * cores * threads < cpus) {
+error_report("cpu topology: "
+ "sockets (%u) * cores (%u) * threads (%u) < "
+ "smp_cpus (%u)",
+ sockets, cores, threads, cpus);
+exit(1);
+}
+
+maxcpus = maxcpus > 0 ? maxcpus : cpus;
+
+if (maxcpus > MAX_CPUMASK_BITS) {
+error_report("unsupported number of maxcpus");
+exit(1);
+}
+
+if (maxcpus < cpus) {
+error_report("maxcpus must be equal to or greater than smp");
+exit(1);
+}
+
+if (sockets * cores * threads > maxcpus) {
+error_report("cpu topology: "
+ "sockets (%u) * cores (%u) * threads (%u) > "
+ "maxcpus (%u)",
+ sockets, cores, threads, maxcpus);
+exit(1);
+}
+
+if (sockets_input && sockets * cores * threads != maxcpus) {
+unsigned sockets_rounded = DIV_ROUND_UP(maxcpus, cores * threads);
+
 error_report("warning: cpu topology: "
- "machine properties currently ignored");
+ "sockets (%u) * cores (%u) * threads (%u) != "
+ "maxcpus (%u). Trying sockets=%u.",
+ sockets, cores, threads, maxcpus, sockets_rounded);
+sockets = sockets_rounded;
+
+if (sockets * cores * threads > maxcpus) {
+error_report("cpu topology: "
+ "sockets (%u) * cores (%u) * threads (%u) > "
+ "maxcpus (%u)",
+ sockets, cores, threads, maxcpus);
+exit(1);
+}
 }
+
+ms->sockets = sockets;
+ms->cores   = cores;
+ms->threads = threads;
+ms->maxcpus = maxcpus;
+ms->cpus= cpus;
 }
 
 static void machine_pre_init(MachineState *ms)
 {
+MachineClass *mc = MACHINE_CLASS(object_get_class(OBJECT(ms)));
+
 machine_set_smp_parameters(ms);
+smp_cores   = ms->cores;
+smp_threads = ms->threads;
+max_cpus= ms->maxcpus;
+smp_cpus= ms->cpus;
+
+mc->max_cpus = mc->max_cpus ?: 1; /* Default to UP */
+if (ms->maxcpus > mc->max_cpus) {
+error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
+ "supported by machine '%s' (%d)", ms->maxcpus, mc->name,
+ mc->max_cpus);
+exit(1);
+}
+
+if (ms->cpus > 1) {
+Error *blocker = NULL;
+error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+

Re: [Qemu-devel] [PATCH 4/6] trace: Add per-vCPU tracing states for events with the 'vcpu' property

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Thu, Jun 09, 2016 at 04:17:11PM +0200, Lluís Vilanova wrote:
>> >> @@ -61,7 +69,7 @@ static inline bool 
>> >> trace_event_get_state_static(TraceEvent *ev)
>> >> static inline bool trace_event_get_state_dynamic_by_id(TraceEventID id)
>> >> {
>> >> /* it's on fast path, avoid consistency checks (asserts) */
>> >> -return unlikely(trace_events_enabled_count) && 
>> >> trace_events_dstate[id];
>> >> +return unlikely(trace_events_enabled_count) && 
>> >> (trace_events_dstate[id] > 0);
>> 
>> > typeof(trace_events_dstate[0]) is size_t, so trace_events_dstate[id] > 0
>> > is equivalent to trace_events_dstate[id] (due to unsigned).  Why change
>> > this line?
>> 
>> Sorry, I have a tendency to make this type of checks explicit when the types 
>> are
>> not boolean (for a maybe-false sense of future-proofing). I can leave it as 
>> it
>> was if it bothers you.

> When reviewing patches I try to understand each change.  When I don't
> see a reason for a change I need to ask.

> In general it's easier to leave code as-is unless there is a need to
> change it.  But there are no hard rules :).

I'll refrain from pushing my manias into QEMU :)


[...]
>> > The number of cpus has type int (see CPUState *qemu_get_cpu(int index)).
>> 
>> > Why did you choose size_t?
>> 
>> It just sounds proper to me to use size_t, since the state can never be 
>> negative
>> (it's either interpreted as a boolean or as an unsigned counter, depending on
>> the "vcpu" property).

> If you feel strongly about it, feel free to keep it.  Alternative
> reasoning about the type:

> int is the CPU index type used in qemu_get_cpu().  It is guaranteed to
> be large enough for the vcpu count.  IMO there's no need to select a new
> type, but there's more...

> size_t is larger than necessary on 64-bit machines and has an impact on
> the CPU cache performance that Paolo's optimization takes advantage of
> (if you trigger adjacent trace event IDs they will probably already be
> in cache).

> size_t made me have to think hard when reading the "int += bool -
> size_t" statement for updating trace_events_enabled_count.

> If int is used then it's clear that int = (int)bool - int will be one of
> [-1, 0, +1].

> But with size_t you have to starting wondering whether the type coercion
> is portable and works as expected:

> int = (int)((size_t)bool - size_t);

> In "6.3.1.3 Signed and unsigned integers" the C99 standard says:

>   [If] the new type is signed and the value cannot be represented in
>   it; either the result is implementation-defined or an
>   implementation-defined signal is raised.

> The size_t -> int conversion is therefore implementation-defined.  This
> is not portable although QEMU probably does it in many places.

> So for these reasons, I think int is the natural choice.

Fair point. But now I feel tempted to change both trace_events_dstate and
trace_events_enabled_count into unsigned int... it burns me when I see signed
types used only on their positives by design.

But don't worry, I'll change trace_events_dstate into int :)


Thanks!
  Lluis

[Qemu-devel] [PATCH RFC 09/16] hw/i386/pc: don't use smp_cores, smp_threads

[Andrew Jones]

Use CPUState nr_cores,nr_threads and MachineState
cores,threads instead.

Signed-off-by: Andrew Jones 
---
 hw/i386/pc.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7198ed533cc47..4fa86d6387ce9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -27,7 +27,6 @@
 #include "hw/char/serial.h"
 #include "hw/i386/apic.h"
 #include "hw/i386/topology.h"
-#include "sysemu/cpus.h"
 #include "hw/block/fdc.h"
 #include "hw/ide.h"
 #include "hw/pci/pci.h"
@@ -682,12 +681,14 @@ void enable_compat_apic_id_mode(void)
  * no concept of "CPU index", and the NUMA tables on fw_cfg need the APIC ID of
  * all CPUs up to max_cpus.
  */
-static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
+static uint32_t x86_cpu_apic_id_from_index(MachineState *ms,
+   unsigned int cpu_index)
 {
 uint32_t correct_id;
 static bool warned;
 
-correct_id = x86_apicid_from_cpu_idx(smp_cores, smp_threads, cpu_index);
+correct_id = x86_apicid_from_cpu_idx(ms->cores, ms->threads,
+ cpu_index);
 if (compat_apic_id_mode) {
 if (cpu_index != correct_id && !warned && !qtest_enabled()) {
 error_report("APIC IDs set in compatibility mode, "
@@ -778,7 +779,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, 
PCMachineState *pcms)
 numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
 numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
 for (i = 0; i < max_cpus; i++) {
-unsigned int apic_id = x86_cpu_apic_id_from_index(i);
+unsigned int apic_id = x86_cpu_apic_id_from_index(MACHINE(pcms), i);
 assert(apic_id < pcms->apic_id_limit);
 for (j = 0; j < nb_numa_nodes; j++) {
 if (test_bit(i, numa_info[j].node_cpu)) {
@@ -1066,7 +1067,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
 X86CPU *cpu;
 MachineState *machine = MACHINE(qdev_get_machine());
-int64_t apic_id = x86_cpu_apic_id_from_index(id);
+int64_t apic_id = x86_cpu_apic_id_from_index(machine, id);
 Error *local_err = NULL;
 
 if (id < 0) {
@@ -1123,7 +1124,7 @@ void pc_cpus_init(PCMachineState *pcms)
  *
  * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
  */
-pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
+pcms->apic_id_limit = x86_cpu_apic_id_from_index(machine, max_cpus - 1) + 
1;
 if (pcms->apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) {
 error_report("max_cpus is too large. APIC ID of last CPU is %u",
  pcms->apic_id_limit - 1);
@@ -1133,10 +1134,12 @@ void pc_cpus_init(PCMachineState *pcms)
 pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) +
 sizeof(CPUArchId) * max_cpus);
 for (i = 0; i < max_cpus; i++) {
-pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i);
+pcms->possible_cpus->cpus[i].arch_id =
+x86_cpu_apic_id_from_index(machine, i);
 pcms->possible_cpus->len++;
 if (i < smp_cpus) {
-cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i),
+cpu = pc_new_cpu(machine->cpu_model,
+ x86_cpu_apic_id_from_index(machine, i),
  &error_fatal);
 pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
 object_unref(OBJECT(cpu));
@@ -1193,7 +1196,7 @@ void pc_guest_info_init(PCMachineState *pcms)
  sizeof *pcms->node_cpu);
 
 for (i = 0; i < max_cpus; i++) {
-unsigned int apic_id = x86_cpu_apic_id_from_index(i);
+unsigned int apic_id = x86_cpu_apic_id_from_index(MACHINE(pcms), i);
 assert(apic_id < pcms->apic_id_limit);
 for (j = 0; j < nb_numa_nodes; j++) {
 if (test_bit(i, numa_info[j].node_cpu)) {
@@ -1940,9 +1943,10 @@ static void pc_machine_reset(void)
 
 static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
 {
+CPUState *cs = first_cpu;
 X86CPUTopoInfo topo;
-x86_topo_ids_from_idx(smp_cores, smp_threads, cpu_index,
-  &topo);
+
+x86_topo_ids_from_idx(cs->nr_cores, cs->nr_threads, cpu_index, &topo);
 return topo.pkg_id;
 }
 
-- 
2.4.11

[Qemu-devel] [PATCH RFC 05/16] hw/core/machine: add smp properites

[Andrew Jones]

Signed-off-by: Andrew Jones 
---
 hw/core/machine.c   | 81 +
 include/hw/boards.h |  6 
 2 files changed, 87 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 3dce9020e510a..2625044002e57 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -172,6 +172,53 @@ static void machine_set_dumpdtb(Object *obj, const char 
*value, Error **errp)
 ms->dumpdtb = g_strdup(value);
 }
 
+static void machine_get_smp(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+int64_t value;
+
+if (strncmp(name, "sockets", 7) == 0) {
+value = ms->sockets;
+} else if (strncmp(name, "cores", 5) == 0) {
+value = ms->cores;
+} else if (strncmp(name, "threads", 7) == 0) {
+value = ms->threads;
+} else if (strncmp(name, "maxcpus", 7) == 0) {
+value = ms->maxcpus;
+} else if (strncmp(name, "cpus", 4) == 0) {
+value = ms->cpus;
+}
+
+visit_type_int(v, name, &value, errp);
+}
+
+static void machine_set_smp(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+Error *error = NULL;
+int64_t value;
+
+visit_type_int(v, name, &value, &error);
+if (error) {
+error_propagate(errp, error);
+return;
+}
+
+if (strncmp(name, "sockets", 7) == 0) {
+ms->sockets = value;
+} else if (strncmp(name, "cores", 5) == 0) {
+ms->cores = value;;
+} else if (strncmp(name, "threads", 7) == 0) {
+ms->threads = value;
+} else if (strncmp(name, "maxcpus", 7) == 0) {
+ms->maxcpus = value;
+} else if (strncmp(name, "cpus", 4) == 0) {
+ms->cpus = value;
+}
+}
+
 static void machine_get_phandle_start(Object *obj, Visitor *v,
   const char *name, void *opaque,
   Error **errp)
@@ -368,8 +415,18 @@ static void machine_init_notify(Notifier *notifier, void 
*data)
 foreach_dynamic_sysbus_device(error_on_sysbus_device, NULL);
 }
 
+static void machine_set_smp_parameters(MachineState *ms)
+{
+if (ms->sockets != -1 || ms->cores != -1 || ms->threads != -1 ||
+ms->maxcpus != -1 || ms->cpus != -1) {
+error_report("warning: cpu topology: "
+ "machine properties currently ignored");
+}
+}
+
 static void machine_pre_init(MachineState *ms)
 {
+machine_set_smp_parameters(ms);
 }
 
 static void machine_class_init(ObjectClass *oc, void *data)
@@ -403,6 +460,11 @@ static void machine_initfn(Object *obj)
 ms->dump_guest_core = true;
 ms->mem_merge = true;
 ms->enable_graphics = true;
+ms->sockets = -1;
+ms->cores = -1;
+ms->threads = -1;
+ms->maxcpus = -1;
+ms->cpus = -1;
 
 object_property_add_str(obj, "accel",
 machine_get_accel, machine_set_accel, NULL);
@@ -462,6 +524,25 @@ static void machine_initfn(Object *obj)
 object_property_set_description(obj, "dt-compatible",
 "Overrides the \"compatible\" property of 
the dt root node",
 NULL);
+object_property_add(obj, "sockets", "int", machine_get_smp,
+machine_set_smp, NULL, NULL, NULL);
+object_property_set_description(obj, "sockets", "Number of sockets", NULL);
+object_property_add(obj, "cores", "int", machine_get_smp,
+machine_set_smp, NULL, NULL, NULL);
+object_property_set_description(obj, "cores",
+"Number of cores per socket", NULL);
+object_property_add(obj, "threads", "int", machine_get_smp,
+machine_set_smp, NULL, NULL, NULL);
+object_property_set_description(obj, "threads",
+"Number of threads per core", NULL);
+object_property_add(obj, "maxcpus", "int", machine_get_smp,
+machine_set_smp, NULL, NULL, NULL);
+object_property_set_description(obj, "maxcpus", "Maximum number of cpus",
+NULL);
+object_property_add(obj, "cpus", "int", machine_get_smp,
+machine_set_smp, NULL, NULL, NULL);
+object_property_set_description(obj, "cpus", "Number of online cpus",
+NULL);
 object_property_add_bool(obj, "dump-guest-core",
  machine_get_dump_guest_core,
  machine_set_dump_guest_core,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 4e8dc68b07a24..53adbfe2a3099 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -166,6 +166,12 @@ struct MachineState {
 char *initrd_filename;
 const char *cpu_model;
 AccelState *accelerator;
+
+int sockets;
+int cores;
+int

[Qemu-devel] [PATCH RFC 11/16] target-ppc: don't use smp_threads

[Andrew Jones]

Use CPUState nr_threads instead.

Signed-off-by: Andrew Jones 
---
 target-ppc/translate_init.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index a1db5009c4a83..f442b2fc934d1 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -24,7 +24,6 @@
 #include 
 #include "kvm_ppc.h"
 #include "sysemu/arch_init.h"
-#include "sysemu/cpus.h"
 #include "cpu-models.h"
 #include "mmu-hash32.h"
 #include "mmu-hash64.h"
@@ -9228,15 +9227,15 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 #endif
 
 #if !defined(CONFIG_USER_ONLY)
-if (smp_threads > max_smt) {
+if (cs->nr_threads > max_smt) {
 error_setg(errp, "Cannot support more than %d threads on PPC with %s",
max_smt, kvm_enabled() ? "KVM" : "TCG");
 return;
 }
-if (!is_power_of_2(smp_threads)) {
+if (!is_power_of_2(cs->nr_threads)) {
 error_setg(errp, "Cannot support %d threads on PPC with %s, "
"threads count must be a power of 2.",
-   smp_threads, kvm_enabled() ? "KVM" : "TCG");
+   cs->nr_threads, kvm_enabled() ? "KVM" : "TCG");
 return;
 }
 #endif
@@ -9248,14 +9247,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
-+ (cs->cpu_index % smp_threads);
+cpu->cpu_dt_id = (cs->cpu_index / cs->nr_threads) * max_smt
++ (cs->cpu_index % cs->nr_threads);
 
 if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
 error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
 error_append_hint(errp, "Adjust the number of cpus to %d "
   "or try to raise the number of threads per core\n",
-  cpu->cpu_dt_id * smp_threads / max_smt);
+  cpu->cpu_dt_id * cs->nr_threads / max_smt);
 return;
 }
 #endif
@@ -9496,7 +9495,7 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error 
**errp)
 
 int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
 {
-int ret = MIN(smp_threads, kvmppc_smt_threads());
+int ret = MIN(CPU(cpu)->nr_threads, kvmppc_smt_threads());
 
 switch (cpu->cpu_version) {
 case CPU_POWERPC_LOGICAL_2_05:
-- 
2.4.11

[Qemu-devel] [PATCH RFC 14/16] hw/arm/virt: stash cpu topo info in VirtGuestInfo

[Andrew Jones]

This is a first step to preparing mach-virt for configurable
cpu topology, and is necessary now to prepare to move smbios
code away from using cpu topology globals smp_cores,smp_threads.

Signed-off-by: Andrew Jones 
---
 hw/arm/virt.c| 6 +-
 include/hw/arm/virt-acpi-build.h | 4 
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 134b6e36623ba..769a49aa5be77 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1281,7 +1281,11 @@ static void machvirt_init(MachineState *machine)
 create_fw_cfg(vbi, &address_space_memory);
 rom_set_fw(fw_cfg_find());
 
-guest_info->cpus = vbi->cpus;
+guest_info->sockets = machine->sockets;
+guest_info->cores   = machine->cores;
+guest_info->threads = machine->threads;
+guest_info->maxcpus = machine->maxcpus;
+guest_info->cpus= machine->cpus;
 guest_info->fw_cfg = fw_cfg_find();
 guest_info->memmap = vbi->memmap;
 guest_info->irqmap = vbi->irqmap;
diff --git a/include/hw/arm/virt-acpi-build.h b/include/hw/arm/virt-acpi-build.h
index d6c5982960403..a34fb04230e66 100644
--- a/include/hw/arm/virt-acpi-build.h
+++ b/include/hw/arm/virt-acpi-build.h
@@ -27,6 +27,10 @@
 #define ACPI_GICC_ENABLED 1
 
 typedef struct VirtGuestInfo {
+int sockets;
+int cores;
+int threads;
+int maxcpus;
 int cpus;
 FWCfgState *fw_cfg;
 const MemMapEntry *memmap;
-- 
2.4.11

[Qemu-devel] [PATCH RFC 10/16] hw/ppc/spapr: don't use smp_cores, smp_threads

[Andrew Jones]

Use CPUState nr_cores,nr_threads and MachineState
cores,threads instead.

Signed-off-by: Andrew Jones 
---
 hw/ppc/spapr.c  | 9 +
 hw/ppc/spapr_rtas.c | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 063664234106e..f78276bb4b164 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -35,7 +35,6 @@
 #include "net/net.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/block-backend.h"
-#include "sysemu/cpus.h"
 #include "sysemu/kvm.h"
 #include "sysemu/device_tree.h"
 #include "kvm_ppc.h"
@@ -603,7 +602,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, 
int offset,
 uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 10;
 uint32_t page_sizes_prop[64];
 size_t page_sizes_prop_size;
-uint32_t vcpus_per_socket = smp_threads * smp_cores;
+uint32_t vcpus_per_socket = cs->nr_cores * cs->nr_threads;
 uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
 
 /* Note: we keep CI large pages off for now because a 64K capable guest
@@ -1774,7 +1773,7 @@ static void ppc_spapr_init(MachineState *machine)
 /* Set up Interrupt Controller before we create the VCPUs */
 spapr->icp = xics_system_init(machine,
   DIV_ROUND_UP(max_cpus * kvmppc_smt_threads(),
-   smp_threads),
+   machine->threads),
   XICS_IRQS, &error_fatal);
 
 if (smc->dr_lmb_enabled) {
@@ -2268,9 +2267,11 @@ static HotplugHandler 
*spapr_get_hotpug_handler(MachineState *machine,
 
 static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
 {
+CPUState *cs = first_cpu;
+
 /* Allocate to NUMA nodes on a "socket" basis (not that concept of
  * socket means much for the paravirtualized PAPR platform) */
-return cpu_index / smp_threads / smp_cores;
+return cpu_index / cs->nr_cores / cs->nr_threads;
 }
 
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 43e2c684fda8d..3fdfbb01a20dd 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -742,7 +742,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
rtas_addr,
 lrdr_capacity[1] = cpu_to_be32(max_hotplug_addr & 0x);
 lrdr_capacity[2] = 0;
 lrdr_capacity[3] = cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE);
-lrdr_capacity[4] = cpu_to_be32(max_cpus/smp_threads);
+lrdr_capacity[4] = cpu_to_be32(max_cpus / machine->threads);
 ret = qemu_fdt_setprop(fdt, "/rtas", "ibm,lrdr-capacity", lrdr_capacity,
  sizeof(lrdr_capacity));
 if (ret < 0) {
-- 
2.4.11

[Qemu-devel] [PATCH RFC 16/16] sysemu/cpus: bye, bye smp_cores, smp_threads

[Andrew Jones]

The smp_cores and smp_threads globals are no longer used.
Vanish them.

Signed-off-by: Andrew Jones 
---
 hw/core/machine.c |  2 --
 include/sysemu/cpus.h | 10 --
 vl.c  |  2 --
 3 files changed, 14 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 5427924d4c911..fdd28e5786685 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -529,8 +529,6 @@ static void machine_pre_init(MachineState *ms)
 };
 
 machine_set_smp_parameters(ms);
-smp_cores   = ms->cores;
-smp_threads = ms->threads;
 max_cpus= ms->maxcpus;
 smp_cpus= ms->cpus;
 
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index fe992a8946ed5..d3e19ca214564 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -27,16 +27,6 @@ void cpu_synchronize_all_post_init(void);
 
 void qtest_clock_warp(int64_t dest);
 
-#ifndef CONFIG_USER_ONLY
-/* vl.c */
-extern int smp_cores;
-extern int smp_threads;
-#else
-/* *-user doesn't have configurable SMP topology */
-#define smp_cores   1
-#define smp_threads 1
-#endif
-
 void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg);
 
 #endif
diff --git a/vl.c b/vl.c
index 843b7a9dff753..e73c66364932c 100644
--- a/vl.c
+++ b/vl.c
@@ -155,8 +155,6 @@ int win2k_install_hack = 0;
 int singlestep = 0;
 int smp_cpus = 1;
 int max_cpus = 1;
-int smp_cores = 1;
-int smp_threads = 1;
 int acpi_enabled = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
-- 
2.4.11

[Qemu-devel] [PATCH] macio: call dma_memory_unmap() at the end of each DMA transfer

[Mark Cave-Ayland]

This ensures that the underlying memory is marked dirty once the transfer
is complete and resolves cache coherency problems under MacOS 9.

Signed-off-by: Mark Cave-Ayland 
---
 hw/ide/macio.c |   46 +---
 include/hw/ppc/mac_dbdma.h |5 +
 2 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 78c10a0..fa57352 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -66,8 +66,7 @@ static void pmac_dma_read(BlockBackend *blk,
 DBDMA_io *io = opaque;
 MACIOIDEState *m = io->opaque;
 IDEState *s = idebus_active_if(&m->bus);
-dma_addr_t dma_addr, dma_len;
-void *mem;
+dma_addr_t dma_addr;
 int64_t sector_num;
 int nsector;
 uint64_t align = BDRV_SECTOR_SIZE;
@@ -84,9 +83,10 @@ static void pmac_dma_read(BlockBackend *blk,
   sector_num, nsector);
 
 dma_addr = io->addr;
-dma_len = io->len;
-mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len,
- DMA_DIRECTION_FROM_DEVICE);
+io->dir = DMA_DIRECTION_FROM_DEVICE;
+io->dma_len = io->len;
+io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len,
+ io->dir);
 
 if (offset & (align - 1)) {
 head_bytes = offset & (align - 1);
@@ -100,7 +100,7 @@ static void pmac_dma_read(BlockBackend *blk,
 offset = offset & ~(align - 1);
 }
 
-qemu_iovec_add(&io->iov, mem, io->len);
+qemu_iovec_add(&io->iov, io->dma_mem, io->len);
 
 if ((offset + bytes) & (align - 1)) {
 tail_bytes = (offset + bytes) & (align - 1);
@@ -130,8 +130,7 @@ static void pmac_dma_write(BlockBackend *blk,
 DBDMA_io *io = opaque;
 MACIOIDEState *m = io->opaque;
 IDEState *s = idebus_active_if(&m->bus);
-dma_addr_t dma_addr, dma_len;
-void *mem;
+dma_addr_t dma_addr;
 int64_t sector_num;
 int nsector;
 uint64_t align = BDRV_SECTOR_SIZE;
@@ -149,9 +148,10 @@ static void pmac_dma_write(BlockBackend *blk,
   sector_num, nsector);
 
 dma_addr = io->addr;
-dma_len = io->len;
-mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len,
- DMA_DIRECTION_TO_DEVICE);
+io->dir = DMA_DIRECTION_TO_DEVICE;
+io->dma_len = io->len;
+io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len,
+ io->dir);
 
 if (offset & (align - 1)) {
 head_bytes = offset & (align - 1);
@@ -163,7 +163,7 @@ static void pmac_dma_write(BlockBackend *blk,
 blk_pread(s->blk, (sector_num << 9), &io->head_remainder, align);
 
 qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes);
-qemu_iovec_add(&io->iov, mem, io->len);
+qemu_iovec_add(&io->iov, io->dma_mem, io->len);
 
 bytes += offset & (align - 1);
 offset = offset & ~(align - 1);
@@ -181,7 +181,7 @@ static void pmac_dma_write(BlockBackend *blk,
 blk_pread(s->blk, (sector_num << 9), &io->tail_remainder, align);
 
 if (!unaligned_head) {
-qemu_iovec_add(&io->iov, mem, io->len);
+qemu_iovec_add(&io->iov, io->dma_mem, io->len);
 }
 
 qemu_iovec_add(&io->iov, &io->tail_remainder + tail_bytes,
@@ -193,7 +193,7 @@ static void pmac_dma_write(BlockBackend *blk,
 }
 
 if (!unaligned_head && !unaligned_tail) {
-qemu_iovec_add(&io->iov, mem, io->len);
+qemu_iovec_add(&io->iov, io->dma_mem, io->len);
 }
 
 s->io_buffer_size -= io->len;
@@ -214,18 +214,18 @@ static void pmac_dma_trim(BlockBackend *blk,
 DBDMA_io *io = opaque;
 MACIOIDEState *m = io->opaque;
 IDEState *s = idebus_active_if(&m->bus);
-dma_addr_t dma_addr, dma_len;
-void *mem;
+dma_addr_t dma_addr;
 
 qemu_iovec_destroy(&io->iov);
 qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1);
 
 dma_addr = io->addr;
-dma_len = io->len;
-mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len,
- DMA_DIRECTION_TO_DEVICE);
+io->dir = DMA_DIRECTION_TO_DEVICE;
+io->dma_len = io->len;
+io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len,
+ io->dir);
 
-qemu_iovec_add(&io->iov, mem, io->len);
+qemu_iovec_add(&io->iov, io->dma_mem, io->len);
 s->io_buffer_size -= io->len;
 s->io_buffer_index += io->len;
 io->len = 0;
@@ -285,6 +285,9 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int 
ret)
 return;
 
 done:
+dma_memory_unmap(&address_space_memory, io->dma_mem, io->dma_len,
+ io->dir, io->dma_len);
+
 if (ret < 0) {
 block_acct_failed(blk_get_stats(s->blk), &s->acct);
 } else {
@@ -351,6 +354,9 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
 return;
 
 done:
+dma_memory_unmap(&address_space_memory, io->dma_mem, io->dma_len,
+

Re: [Qemu-devel] [PATCH 07/10] target-sparc: cpu: use sparc_cpu_parse_features() directly

[Mark Cave-Ayland]

On 10/06/16 12:51, Eduardo Habkost wrote:

> On Wed, Jun 08, 2016 at 01:30:11PM -0300, Eduardo Habkost wrote:
>> On Mon, Jun 06, 2016 at 05:16:49PM +0200, Igor Mammedov wrote:
>>> make SPARC target use sparc_cpu_parse_features() directly
>>> so it won't get in the way of switching other propertified
>>> targets to handling features as global properties.
>>>
>>> Signed-off-by: Igor Mammedov 
>>
>> I would like to apply this to the x86 tree, to allow the
>> remaining patches to be applied. May I get an Acked-by from the
>> SPARC maintainers?
> 
> I hear no objections, I will queue it on x86-next.

Given that I've never used CPU options for SPARC, this is probably okay
as long the standard sun4m/sun4u guests fire up with the same command lines.

Apologies for the delay on reviewing, my QEMU development is relegated
to time outside of work and the recent breakage on PPC/SPARC has eaten
huge amounts of my available time over the past week :/


ATB,

Mark.

[Qemu-devel] [PATCH v3 0/5] block/mirror: Fix (?) target backing BDS

[Max Reitz]

Issue #1: If the target image does not have a backing BDS before mirror
completion, qemu tries really hard to give it a backing BDS. If the
source has a backing BDS, it will actually always "succeed".
In some cases, the target is not supposed to have a backing BDS, though
(absolute-paths: because of sync=full; existing: because the target
image does not have a backing file; blockdev-mirror: because of an
explicit "backing": ""). Then, this is pretty bad behavior.

This should generally not change the target's visible data, but it still
is ugly.

Issue #2: Currently the backing chain of the target is basically opened
using bdrv_open_backing_file() (except for sometimes™). This results in
multiple BDSs for a single physical file, which is bad. In most use
cases, this is only temporary, but it still is bad.

If we can reuse the existing backing chain of the source (which is with
drive-mirror in "absolute-paths" mode), we should just do so.


v3:
- Patch 1:
  - More verbose commit message [Kevin]
  - Changed comment to match code [Kevin]
- Patch 2:
  - Do not force use of the source backing chain for the target in
"existing" mode or with blockdev-mirror [Kevin]
- Instead keep doing what we've been doing for
  drive-mirror/existing, only that we should still drop the
  bdrv_set_backing_hd() in bdrv_replace_in_backing_chain()
- And for blockdev-mirror, just do not change the current backing
  chain at all; this is what we've been doing until now, unless the
  target BDS did not have a backing BDS yet
- Patch 3: Added, because it makes the next test a bit nicer
- Patch 4: Adjusted to v3 behavior, and added a new test for
  blockdev-mirror with a target whose backing file has been overridden
  using the "backing" option
- Patch 5: Added [Kevin]


git-backport-diff against v2:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/5:[0005] [FC] 'block: Allow replacement of a BDS by its overlay'
002/5:[0057] [FC] 'block/mirror: Fix target backing BDS'
003/5:[down] 'block/null: Implement bdrv_refresh_filename()'
004/5:[0073] [FC] 'iotests: Add test for post-mirror backing chains'
005/5:[down] 'iotests: Add test for oVirt-like storage migration'


Max Reitz (5):
  block: Allow replacement of a BDS by its overlay
  block/mirror: Fix target backing BDS
  block/null: Implement bdrv_refresh_filename()
  iotests: Add test for post-mirror backing chains
  iotests: Add test for oVirt-like storage migration

 block.c|  24 +++--
 block/mirror.c |  39 +--
 block/null.c   |  20 
 blockdev.c |  15 ++-
 include/block/block_int.h  |  18 +++-
 tests/qemu-iotests/155 | 263 +
 tests/qemu-iotests/155.out |   5 +
 tests/qemu-iotests/156 | 174 ++
 tests/qemu-iotests/156.out |  48 +
 tests/qemu-iotests/group   |   2 +
 10 files changed, 584 insertions(+), 24 deletions(-)
 create mode 100755 tests/qemu-iotests/155
 create mode 100644 tests/qemu-iotests/155.out
 create mode 100755 tests/qemu-iotests/156
 create mode 100644 tests/qemu-iotests/156.out

-- 
2.8.3

[Qemu-devel] [PATCH v3 1/5] block: Allow replacement of a BDS by its overlay

[Max Reitz]

change_parent_backing_link() asserts that the BDS to be replaced is not
used as a backing file. However, we may want to replace a BDS by its
overlay in which case that very link should not be redirected.

For instance, when doing a sync=none drive-mirror operation, we may have
the following BDS/BB forest before block job completion:

  target

  base <- source <- BlockBackend

During job completion, we want to establish the source BDS as the
target's backing node:

  target
|
v
  base <- source <- BlockBackend

This makes the target a valid replacement for the source:

  target <- BlockBackend
|
v
  base <- source

Without this modification to change_parent_backing_link() we have to
inject the target into the graph before the source is its backing node,
thus temporarily creating a wrong graph:

  target <- BlockBackend

  base <- source

Signed-off-by: Max Reitz 
---
 block.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index f54bc25..dc76c159 100644
--- a/block.c
+++ b/block.c
@@ -2224,9 +2224,23 @@ void bdrv_close_all(void)
 static void change_parent_backing_link(BlockDriverState *from,
BlockDriverState *to)
 {
-BdrvChild *c, *next;
+BdrvChild *c, *next, *to_c;
 
 QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
+if (c->role == &child_backing) {
+/* @from is generally not allowed to be a backing file, except for
+ * when @to is the overlay. In that case, @from may not be replaced
+ * by @to as @to's backing node. */
+QLIST_FOREACH(to_c, &to->children, next) {
+if (to_c == c) {
+break;
+}
+}
+if (to_c) {
+continue;
+}
+}
+
 assert(c->role != &child_backing);
 bdrv_ref(to);
 bdrv_replace_child(c, to);
-- 
2.8.3

[Qemu-devel] [PATCH v3 5/5] iotests: Add test for oVirt-like storage migration

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/156 | 174 +
 tests/qemu-iotests/156.out |  48 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 223 insertions(+)
 create mode 100755 tests/qemu-iotests/156
 create mode 100644 tests/qemu-iotests/156.out

diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156
new file mode 100755
index 000..cc95ff1
--- /dev/null
+++ b/tests/qemu-iotests/156
@@ -0,0 +1,174 @@
+#!/bin/bash
+#
+# Tests oVirt-like storage migration:
+#  - Create snapshot
+#  - Create target image with (not yet existing) target backing chain
+#(i.e. just write the name of a soon-to-be-copied-over backing file into 
it)
+#  - drive-mirror the snapshot to the target with mode=existing and sync=top
+#  - In the meantime, copy the original source files to the destination via
+#conventional means (i.e. outside of qemu)
+#  - Complete the drive-mirror job
+#  - Delete all source images
+#
+# Copyright (C) 2016 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 .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1   # failure is the default!
+
+_cleanup()
+{
+rm -f "$TEST_IMG{,.target}{,.backing,.overlay}"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2 qed
+_supported_proto generic
+_supported_os Linux
+
+# Create source disk
+TEST_IMG="$TEST_IMG.backing" _make_test_img 1M
+_make_test_img -b "$TEST_IMG.backing" 1M
+
+$QEMU_IO -c 'write -P 1 0 256k' "$TEST_IMG.backing" | _filter_qemu_io
+$QEMU_IO -c 'write -P 2 64k 192k' "$TEST_IMG" | _filter_qemu_io
+
+_launch_qemu -drive if=none,id=source,file="$TEST_IMG"
+
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'qmp_capabilities' }" \
+'return'
+
+# Create snapshot
+TEST_IMG="$TEST_IMG.overlay" _make_test_img -b "$TEST_IMG" 1M
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'blockdev-snapshot-sync',
+   'arguments': { 'device': 'source',
+  'snapshot-file': '$TEST_IMG.overlay',
+  'format': '$IMGFMT',
+  'mode': 'existing' } }" \
+'return'
+
+# Write something to the snapshot
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'human-monitor-command',
+   'arguments': { 'command-line':
+  'qemu-io source \"write -P 3 128k 128k\"' } }" \
+'return'
+
+# Create target image
+TEST_IMG="$TEST_IMG.target.overlay" _make_test_img -b "$TEST_IMG.target" 1M
+
+# Mirror snapshot
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'drive-mirror',
+   'arguments': { 'device': 'source',
+  'target': '$TEST_IMG.target.overlay',
+  'mode': 'existing',
+  'sync': 'top' } }" \
+'return'
+
+# Wait for convergence
+_send_qemu_cmd $QEMU_HANDLE \
+'' \
+'BLOCK_JOB_READY'
+
+# Write some more
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'human-monitor-command',
+   'arguments': { 'command-line':
+  'qemu-io source \"write -P 4 192k 64k\"' } }" \
+'return'
+
+# Copy source backing chain to the target before completing the job
+cp "$TEST_IMG.backing" "$TEST_IMG.target.backing"
+cp "$TEST_IMG" "$TEST_IMG.target"
+$QEMU_IMG rebase -u -b "$TEST_IMG.target.backing" "$TEST_IMG.target"
+
+# Complete block job
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'block-job-complete',
+   'arguments': { 'device': 'source' } }" \
+''
+
+_send_qemu_cmd $QEMU_HANDLE \
+'' \
+'BLOCK_JOB_COMPLETED'
+
+# Remove the source images
+rm -f "$TEST_IMG{,.backing,.overlay}"
+
+echo
+
+# Check online disk contents
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'human-monitor-command',
+   'arguments': { 'command-line':
+  'qemu-io source \"read -P 1 0k 64k\"' } }" \
+'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'human-monitor-command',
+   'arguments': { 'command-line':
+  'qemu-io source \"read -P 2 64k 64k\"' } }" \
+'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+"{ 'execute': 'human-monitor-command',
+   'arguments': { 'command-line':
+  'qemu-io source \"read -P 3 128k 64k\"' } }" \
+'return'
+
+_send_

[Qemu-devel] [PATCH v3 4/5] iotests: Add test for post-mirror backing chains

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/155 | 263 +
 tests/qemu-iotests/155.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 269 insertions(+)
 create mode 100755 tests/qemu-iotests/155
 create mode 100644 tests/qemu-iotests/155.out

diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
new file mode 100755
index 000..06ddc5f
--- /dev/null
+++ b/tests/qemu-iotests/155
@@ -0,0 +1,263 @@
+#!/usr/bin/env python
+#
+# Test whether the backing BDSs are correct after completion of a
+# mirror block job; in "existing" modes (drive-mirror with
+# mode=existing and blockdev-mirror) the backing chain should not be
+# overridden.
+#
+# Copyright (C) 2016 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 .
+#
+
+import os
+import stat
+import time
+import iotests
+from iotests import qemu_img
+
+back0_img = os.path.join(iotests.test_dir, 'back0.' + iotests.imgfmt)
+back1_img = os.path.join(iotests.test_dir, 'back1.' + iotests.imgfmt)
+back2_img = os.path.join(iotests.test_dir, 'back2.' + iotests.imgfmt)
+source_img = os.path.join(iotests.test_dir, 'source.' + iotests.imgfmt)
+target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt)
+
+
+# Class variables for controlling its behavior:
+#
+# existing: If True, explicitly create the target image and blockdev-add it
+# target_backing: If existing is True: Use this filename as the backing file
+# of the target image
+# (None: no backing file)
+# target_blockdev_backing: If existing is True: Pass this dict as "backing"
+#  for the blockdev-add command
+#  (None: do not pass "backing")
+# target_real_backing: If existing is True: The real filename of the backing
+#  image during runtime, only makes sense if
+#  target_blockdev_backing is not None
+#  (None: same as target_backing)
+
+class BaseClass(iotests.QMPTestCase):
+target_blockdev_backing = None
+target_real_backing = None
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, back0_img, '1M')
+qemu_img('create', '-f', iotests.imgfmt, '-b', back0_img, back1_img)
+qemu_img('create', '-f', iotests.imgfmt, '-b', back1_img, back2_img)
+qemu_img('create', '-f', iotests.imgfmt, '-b', back2_img, source_img)
+
+self.vm = iotests.VM()
+self.vm.add_drive(None, '', 'none')
+self.vm.launch()
+
+# Add the BDS via blockdev-add so it stays around after the mirror 
block
+# job has been completed
+result = self.vm.qmp('blockdev-add',
+ options={'node-name': 'source',
+  'driver': iotests.imgfmt,
+  'file': {'driver': 'file',
+   'filename': source_img}})
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('x-blockdev-insert-medium',
+ device='drive0', node_name='source')
+self.assert_qmp(result, 'return', {})
+
+self.assertIntactSourceBackingChain()
+
+if self.existing:
+if self.target_backing:
+qemu_img('create', '-f', iotests.imgfmt,
+ '-b', self.target_backing, target_img, '1M')
+else:
+qemu_img('create', '-f', iotests.imgfmt, target_img, '1M')
+
+if self.cmd == 'blockdev-mirror':
+options = { 'node-name': 'target',
+'driver': iotests.imgfmt,
+'file': { 'driver': 'file',
+  'filename': target_img } }
+if self.target_blockdev_backing:
+options['backing'] = self.target_blockdev_backing
+
+result = self.vm.qmp('blockdev-add', options=options)
+self.assert_qmp(result, 'return', {})
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(source_img)
+os.remove(back2_img)
+os.remove(back1_img)
+os.remove(back0_img)
+try:
+os.remove(target_img)
+except OSError:
+pass
+
+def findBlockNode(self, node_name, id=None):
+if id:
+

[Qemu-devel] [PATCH v3 2/5] block/mirror: Fix target backing BDS

[Max Reitz]

Currently, we are trying to move the backing BDS from the source to the
target in bdrv_replace_in_backing_chain() which is called from
mirror_exit(). However, mirror_complete() already tries to open the
target's backing chain with a call to bdrv_open_backing_file().

First, we should only set the target's backing BDS once. Second, the
mirroring block job has a better idea of what to set it to than the
generic code in bdrv_replace_in_backing_chain() (in fact, the latter's
conditions on when to move the backing BDS from source to target are not
really correct).

Therefore, remove that code from bdrv_replace_in_backing_chain() and
leave it to mirror_complete().

Depending on what kind of mirroring is performed, we furthermore want to
use different strategies to open the target's backing chain:

- If blockdev-mirror is used, we can assume the user made sure that the
  target already has the correct backing chain. In particular, we should
  not try to open a backing file if the target does not have any yet.

- If drive-mirror with mode=absolute-paths is used, we can and should
  reuse the already existing chain of nodes that the source BDS is in.
  In case of sync=full, no backing BDS is required; with sync=top, we
  just link the source's backing BDS to the target, and with sync=none,
  we use the source BDS as the target's backing BDS.
  We should not try to open these backing files anew because this would
  lead to two BDSs existing per physical file in the backing chain, and
  we would like to avoid such concurrent access.

- If drive-mirror with mode=existing is used, we have to use the
  information provided in the physical image file which means opening
  the target's backing chain completely anew, just as it has been done
  already.
  If the target's backing chain shares images with the source, this may
  lead to multiple BDSs per physical image file. But since we cannot
  reliably ascertain this case, there is nothing we can do about it.

Signed-off-by: Max Reitz 
---
 block.c   |  8 
 block/mirror.c| 39 ---
 blockdev.c| 15 ---
 include/block/block_int.h | 18 +-
 4 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/block.c b/block.c
index dc76c159..2691c2f 100644
--- a/block.c
+++ b/block.c
@@ -2289,14 +2289,6 @@ void bdrv_replace_in_backing_chain(BlockDriverState 
*old, BlockDriverState *new)
 
 change_parent_backing_link(old, new);
 
-/* Change backing files if a previously independent node is added to the
- * chain. For active commit, we replace top by its own (indirect) backing
- * file and don't do anything here so we don't build a loop. */
-if (new->backing == NULL && !bdrv_chain_contains(backing_bs(old), new)) {
-bdrv_set_backing_hd(new, backing_bs(old));
-bdrv_set_backing_hd(old, NULL);
-}
-
 bdrv_unref(old);
 }
 
diff --git a/block/mirror.c b/block/mirror.c
index 80fd3c7..13abe8c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -44,6 +44,7 @@ typedef struct MirrorBlockJob {
 /* Used to block operations on the drive-mirror-replace target */
 Error *replace_blocker;
 bool is_none_mode;
+BlockMirrorBackingMode backing_mode;
 BlockdevOnError on_source_error, on_target_error;
 bool synced;
 bool should_complete;
@@ -742,20 +743,26 @@ static void mirror_set_speed(BlockJob *job, int64_t 
speed, Error **errp)
 static void mirror_complete(BlockJob *job, Error **errp)
 {
 MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
-Error *local_err = NULL;
-int ret;
+BlockDriverState *src, *target;
+
+src = blk_bs(job->blk);
+target = blk_bs(s->target);
 
-ret = bdrv_open_backing_file(blk_bs(s->target), NULL, "backing",
- &local_err);
-if (ret < 0) {
-error_propagate(errp, local_err);
-return;
-}
 if (!s->synced) {
 error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
 return;
 }
 
+if (s->backing_mode == MIRROR_OPEN_BACKING_CHAIN) {
+int ret;
+
+assert(!target->backing);
+ret = bdrv_open_backing_file(target, NULL, "backing", errp);
+if (ret < 0) {
+return;
+}
+}
+
 /* check the target bs is not blocked and block all operations on it */
 if (s->replaces) {
 AioContext *replace_aio_context;
@@ -777,6 +784,13 @@ static void mirror_complete(BlockJob *job, Error **errp)
 aio_context_release(replace_aio_context);
 }
 
+if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
+BlockDriverState *backing = s->is_none_mode ? src : s->base;
+if (backing_bs(target) != backing) {
+bdrv_set_backing_hd(target, backing);
+}
+}
+
 s->should_complete = true;
 block_job_enter(&s->common);
 }
@@ -799,6 +813,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState

[Qemu-devel] [PATCH v3 3/5] block/null: Implement bdrv_refresh_filename()

[Max Reitz]

Signed-off-by: Max Reitz 
---
 block/null.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/block/null.c b/block/null.c
index 396500b..b511010 100644
--- a/block/null.c
+++ b/block/null.c
@@ -12,6 +12,8 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
 #include "block/block_int.h"
 
 #define NULL_OPT_LATENCY "latency-ns"
@@ -223,6 +225,20 @@ static int64_t coroutine_fn 
null_co_get_block_status(BlockDriverState *bs,
 }
 }
 
+static void null_refresh_filename(BlockDriverState *bs, QDict *opts)
+{
+QINCREF(opts);
+qdict_del(opts, "filename");
+
+if (!qdict_size(opts)) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename), "%s://",
+ bs->drv->format_name);
+}
+
+qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
+bs->full_open_options = opts;
+}
+
 static BlockDriver bdrv_null_co = {
 .format_name= "null-co",
 .protocol_name  = "null-co",
@@ -238,6 +254,8 @@ static BlockDriver bdrv_null_co = {
 .bdrv_reopen_prepare= null_reopen_prepare,
 
 .bdrv_co_get_block_status   = null_co_get_block_status,
+
+.bdrv_refresh_filename  = null_refresh_filename,
 };
 
 static BlockDriver bdrv_null_aio = {
@@ -255,6 +273,8 @@ static BlockDriver bdrv_null_aio = {
 .bdrv_reopen_prepare= null_reopen_prepare,
 
 .bdrv_co_get_block_status   = null_co_get_block_status,
+
+.bdrv_refresh_filename  = null_refresh_filename,
 };
 
 static void bdrv_null_init(void)
-- 
2.8.3

Re: [Qemu-devel] [PULL 0/8] migration: fixes

[Dr. David Alan Gilbert]

* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On 10 June 2016 at 14:25, Peter Maydell  wrote:
> > On 10 June 2016 at 12:48, Amit Shah  wrote:
> >>
> >> The following changes since commit 
> >> 0c33682d5f29b0a4ae53bdec4c8e52e4fae37b34:
> >>
> >>   target-i386: Move user-mode exception actions out of user-exec.c 
> >> (2016-06-09 15:55:02 +0100)
> >>
> >> are available in the git repository at:
> >>
> >>   https://git.kernel.org/pub/scm/virt/qemu/amit/migration.git 
> >> tags/migration-for-2.7-3
> >>
> >> for you to fetch changes up to cfac638acf903f7618b285dc3f36de348554c8ad:
> >>
> >>   Postcopy: Check for support when setting the capability (2016-06-10 
> >> 17:13:32 +0530)
> >>
> >> 
> >> Migration:
> >>
> >> - Fixes for TLS series
> >> - Postcopy: Add stats, fix, test case
> >>
> >> 
> >
> > Fails to build on OSX
> 
> Also fails to build on AArch64 Linux:

OK, I see why I missed this.
My two weirder build cases I checked with previously were building on RHEL6 
(that's
too old for userfault) and an ARM box.  However, the tests include the headers 
from
qemu's linux-header/ subdirectory and that includes __NR_userfault for
both x86 and 32bit ARM, so I wasn't hitting the other side of the ifdef
in my testing.

Dave

> 
> /home/petmay01/qemu/tests/postcopy-test.c: In function 'return_or_event':
> /home/petmay01/qemu/tests/postcopy-test.c:177:9: error: 'got_stop'
> undeclared (first use in this function)
>  got_stop = true;
>  ^
> /home/petmay01/qemu/tests/postcopy-test.c:177:9: note: each undeclared
> identifier is reported only once for each function it appears in
> /home/petmay01/qemu/tests/postcopy-test.c: In function
> 'wait_for_migration_pass':
> /home/petmay01/qemu/tests/postcopy-test.c:235:13: error: 'got_stop'
> undeclared (first use in this function)
>  if (got_stop || initial_pass) {
>  ^
> /home/petmay01/qemu/tests/postcopy-test.c: In function 'check_guests_ram':
> /home/petmay01/qemu/tests/postcopy-test.c:262:33: error:
> 'start_address' undeclared (first use in this function)
>  qtest_memread(global_qtest, start_address, &first_byte, 1);
>  ^
> /home/petmay01/qemu/tests/postcopy-test.c:265:52: error: 'end_address'
> undeclared (first use in this function)
>  for (address = start_address + 4096; address < end_address;
> address += 4096)
> ^
> /home/petmay01/qemu/tests/postcopy-test.c: In function 'test_migrate':
> /home/petmay01/qemu/tests/postcopy-test.c:307:5: error: 'got_stop'
> undeclared (first use in this function)
>  got_stop = false;
>  ^
> /home/petmay01/qemu/tests/postcopy-test.c:395:23: error:
> 'start_address' undeclared (first use in this function)
>  qtest_memread(to, start_address, &dest_byte_a, 1);
>^
> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] nbd: Don't use cpu_to_*w() functions

[Eric Blake]

On 06/10/2016 10:15 AM, Peter Maydell wrote:
> The cpu_to_*w() functions just compose a pointer dereference
> with a byteswap. Instead use st*_p(), which handles potential
> pointer misalignment and avoids the need to cast the pointer.
> 
> Signed-off-by: Peter Maydell 
> ---
>  nbd/client.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] migration: Don't use *_to_cpup() and cpu_to_*w()

[Eric Blake]

On 06/10/2016 10:09 AM, Peter Maydell wrote:
> The *_to_cpup() and cpu_to_*w() functions just compose a pointer
> dereference with a byteswap. Instead use ld*_p() and st*_p(),
> which handle potential pointer misalignment and avoid the need
> to cast the pointer.
> 
> Signed-off-by: Peter Maydell 
> ---
> Changes v1->v2: fix cpu_to_*w() uses too.
> 
> The motivation here is that I'd like to get rid of _to_cpup()
> and cpu_to_*w() entirely: we don't have many places that use them.
> ---
>  migration/migration.c | 12 ++--
>  migration/savevm.c|  4 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1589153] Re: qemu-system-x86_64 version 2.5.0 freezes during windows 7 installation in lubuntu 16.04

[Phil Troy]

Please see
http://ubuntuforums.org/showthread.php?t=2325843&p=13499322#post13499322
for a similar discussion and for a workaround.  But please note that to
the best I can tell it is still a bug.

Phil

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

Title:
  qemu-system-x86_64 version 2.5.0 freezes during windows 7 installation
  in lubuntu 16.04

Status in QEMU:
  New

Bug description:
  Hi!

  I have been using qemu - kvm for several years in different versions
  of ubuntu (lubuntu). I am trying to migrate from 15.04 to 16.04 and am
  having a problem. In particular, on my machine (a samsung series 9
  with dual core i7 processor and 8gb ram) the following commands worked
  in 15.04 but do not work in 15.10 and 16.04. FYI, I tested them on a
  clean machine, where I have created a 60GB image file in its own
  partition.. In particular, I am using the command to start installing
  windows 7 and it works in a clean install of 15.04 (yesterday) but not
  in 15.10 (yesterday) or 16.04 (the day before). I do not get any error
  messages in my xterminal when running this and do not know how to
  check for windows error messages. By not working I mean that after
  loading files it gets to a windows screen and then stays there
  forever.

  The command lines used to invoke qemu is:
  echo "*** Installing windows 7 virtual machine - Step 2"

  
  echo "*** Try command for slow mouse"
  export SDL_VIDEO_X11_DGAMOUSE=0

  sudo qemu-system-x86_64 \
-enable-kvm \
-machine pc,accel=kvm \
-cdrom  
/home/Archives/Software/OperatingSystems.Windows7HP.64/Windows7HP64_Install.iso 
\
-boot d \
-net nic,macaddr=56:44:45:30:31:34 \
-net user \
-cpu host \
-vga qxl \
-spice port=5900,disable-ticketing \
-uuid 8373c3d6-1e6c-f022-38e2-b94e6e14e170 \
-smp cpus=2,maxcpus=3 \
-m 6144 \
-name DrPhilSS9AWin7VM \
-hda /mnt/Windows7Image/Windows7Guest.img \
-localtime \
-k en-us \
-usb \
-usbdevice tablet&
  sleep 10
  spicy --host 127.0.0.1 --port 5900

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

Re: [Qemu-devel] [Bug 1589153] Re: qemu-system-x86_64 version 2.5.0 freezes during windows 7 installation in lubuntu 16.04

[Phil Troy]

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

Title:
  qemu-system-x86_64 version 2.5.0 freezes during windows 7 installation
  in lubuntu 16.04

Status in QEMU:
  New

Bug description:
  Hi!

  I have been using qemu - kvm for several years in different versions
  of ubuntu (lubuntu). I am trying to migrate from 15.04 to 16.04 and am
  having a problem. In particular, on my machine (a samsung series 9
  with dual core i7 processor and 8gb ram) the following commands worked
  in 15.04 but do not work in 15.10 and 16.04. FYI, I tested them on a
  clean machine, where I have created a 60GB image file in its own
  partition.. In particular, I am using the command to start installing
  windows 7 and it works in a clean install of 15.04 (yesterday) but not
  in 15.10 (yesterday) or 16.04 (the day before). I do not get any error
  messages in my xterminal when running this and do not know how to
  check for windows error messages. By not working I mean that after
  loading files it gets to a windows screen and then stays there
  forever.

  The command lines used to invoke qemu is:
  echo "*** Installing windows 7 virtual machine - Step 2"

  
  echo "*** Try command for slow mouse"
  export SDL_VIDEO_X11_DGAMOUSE=0

  sudo qemu-system-x86_64 \
-enable-kvm \
-machine pc,accel=kvm \
-cdrom  
/home/Archives/Software/OperatingSystems.Windows7HP.64/Windows7HP64_Install.iso 
\
-boot d \
-net nic,macaddr=56:44:45:30:31:34 \
-net user \
-cpu host \
-vga qxl \
-spice port=5900,disable-ticketing \
-uuid 8373c3d6-1e6c-f022-38e2-b94e6e14e170 \
-smp cpus=2,maxcpus=3 \
-m 6144 \
-name DrPhilSS9AWin7VM \
-hda /mnt/Windows7Image/Windows7Guest.img \
-localtime \
-k en-us \
-usb \
-usbdevice tablet&
  sleep 10
  spicy --host 127.0.0.1 --port 5900

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

Re: [Qemu-devel] [PULL 00/15] tb hash improvements

[Emilio G. Cota]

On Fri, Jun 10, 2016 at 17:41:26 +0100, Peter Maydell wrote:
> On 10 June 2016 at 17:34, Emilio G. Cota  wrote:
> > On Fri, Jun 10, 2016 at 16:33:10 +0100, Peter Maydell wrote:
> >> Fails to build on ppc64be :-(
> >>
> >> In file included from /home/pm215/qemu/include/qemu/thread.h:4:0,
> >>  from /home/pm215/qemu/include/block/aio.h:20,
> >>  from /home/pm215/qemu/include/block/block.h:4,
> >>  from /home/pm215/qemu/include/monitor/monitor.h:6,
> >>  from /home/pm215/qemu/trace/control.c:23:
> >> /home/pm215/qemu/include/qemu/processor.h:24:35: error: expected
> >> identifier or ‘(’ before string constant
> >>"or 2, 2, 2;" ::: "memory")
> >
> > On Fri, Jun 10, 2016 at 16:57:19 +0100, Peter Maydell wrote:
> >> Also fails trying to build a test on 32-bit:
> >>
> >> /home/petmay01/qemu/tests/qht-bench.c: In function 'pr_params':
> >> /home/petmay01/qemu/tests/qht-bench.c:270:5: error: format '%zu'
> >> expects argument of type 'size_t', but argument 2 has type 'long
> >> unsigned int' [-Werror=format=]
> >>  printf(" lookup range:  %zu\n", lookup_range);
> >
> > Can you please test again after applying the appended delta?
> 
> I will test.
> 
> For the PPC asm, is it not just wanting the "\n" between instructions?
> Shouldn't be necessary to use two separate asm() lines...

I can't test on ppc so I was just being paranoid to avoid
wasting your time :-)

E.

Re: [Qemu-devel] [PATCH v4] spapr: Ensure all LMBs are represented in ibm, dynamic-memory

[Michael Roth]

Quoting Bharata B Rao (2016-06-10 00:14:48)
> Memory hotplug can fail for some combinations of RAM and maxmem when
> DDW is enabled in the presence of devices like nec-usb-xhci. DDW depends
> on maximum addressable memory returned by guest and this value is currently
> being calculated wrongly by the guest kernel routine memory_hotplug_max().
> While there is an attempt to fix the guest kernel, this patch works
> around the problem within QEMU itself.
> 
> memory_hotplug_max() routine in the guest kernel arrives at max
> addressable memory by multiplying lmb-size with the lmb-count obtained
> from ibm,dynamic-memory property. There are two assumptions here:
> 
> - All LMBs are part of ibm,dynamic memory: This is not true for PowerKVM
>   where only hot-pluggable LMBs are present in this property.
> - The memory area comprising of RAM and hotplug region is contiguous: This
>   needn't be true always for PowerKVM as there can be gap between
>   boot time RAM and hotplug region.
> 
> To work around this guest kernel bug, ensure that ibm,dynamic-memory
> has information about all the LMBs (RMA, boot-time LMBs, future
> hotpluggable LMBs, and dummy LMBs to cover the gap between RAM and
> hotpluggable region).
> 
> RMA is represented separately by memory@0 node. Hence mark RMA LMBs
> and also the LMBs for the gap b/n RAM and hotpluggable region as
> reserved and as having no valid DRC so that these LMBs are not considered
> by the guest.
> 
> Signed-off-by: Bharata B Rao 

Reviewed-by: Michael Roth 

> ---
> Changes in v4:
> 
> - Included address information for all LMBs in ibm,dynamic-memory.
> - Use both RESERVED and DRC_INVALID flag bits for non-hotpluggable LMBs.
> 
> v3: https://lists.gnu.org/archive/html/qemu-ppc/2016-06/msg00187.html
> 
>  hw/ppc/spapr.c | 57 
> --
>  include/hw/ppc/spapr.h |  6 --
>  2 files changed, 41 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0636642..9a4a803 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -762,14 +762,17 @@ static int 
> spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
>  int ret, i, offset;
>  uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
>  uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
> -uint32_t nr_lmbs = (machine->maxram_size - machine->ram_size)/lmb_size;
> +uint32_t hotplug_lmb_start = spapr->hotplug_memory.base / lmb_size;
> +uint32_t nr_lmbs = (spapr->hotplug_memory.base +
> +   memory_region_size(&spapr->hotplug_memory.mr)) /
> +   lmb_size;
>  uint32_t *int_buf, *cur_index, buf_len;
>  int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
> 
>  /*
> - * Don't create the node if there are no DR LMBs.
> + * Don't create the node if there is no hotpluggable memory
>   */
> -if (!nr_lmbs) {
> +if (machine->ram_size == machine->maxram_size) {
>  return 0;
>  }
> 
> @@ -803,26 +806,40 @@ static int 
> spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
>  int_buf[0] = cpu_to_be32(nr_lmbs);
>  cur_index++;
>  for (i = 0; i < nr_lmbs; i++) {
> -sPAPRDRConnector *drc;
> -sPAPRDRConnectorClass *drck;
> -uint64_t addr = i * lmb_size + spapr->hotplug_memory.base;;
> +uint64_t addr = i * lmb_size;
>  uint32_t *dynamic_memory = cur_index;
> 
> -drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
> -   addr/lmb_size);
> -g_assert(drc);
> -drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -
> -dynamic_memory[0] = cpu_to_be32(addr >> 32);
> -dynamic_memory[1] = cpu_to_be32(addr & 0x);
> -dynamic_memory[2] = cpu_to_be32(drck->get_index(drc));
> -dynamic_memory[3] = cpu_to_be32(0); /* reserved */
> -dynamic_memory[4] = cpu_to_be32(numa_get_node(addr, NULL));
> -if (addr < machine->ram_size ||
> -memory_region_present(get_system_memory(), addr)) {
> -dynamic_memory[5] = cpu_to_be32(SPAPR_LMB_FLAGS_ASSIGNED);
> +if (i >= hotplug_lmb_start) {
> +sPAPRDRConnector *drc;
> +sPAPRDRConnectorClass *drck;
> +
> +drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB, i);
> +g_assert(drc);
> +drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +
> +dynamic_memory[0] = cpu_to_be32(addr >> 32);
> +dynamic_memory[1] = cpu_to_be32(addr & 0x);
> +dynamic_memory[2] = cpu_to_be32(drck->get_index(drc));
> +dynamic_memory[3] = cpu_to_be32(0); /* reserved */
> +dynamic_memory[4] = cpu_to_be32(numa_get_node(addr, NULL));
> +if (memory_region_present(get_system_memory(), addr)) {
> +dynamic_memory[5] = cpu_to_be32(SPAPR_LMB_FLAGS_ASSIGNED);
> +} else {
> +dyna

[Qemu-devel] [PATCH v2 0/3] coccinelle: Clean up error checks and return value variables

[Eduardo Habkost]

v2 of the previous "error: Remove NULL checks on
error_propagate() calls" patch, now it became a series.

Changes v1 -> v2:
* The Coccinelle scripts were simplified by using "when"
  constraints to detect when a variable is not used elsewhere
  inside the function.
* Added script to remove unnecessary variables for function
  return value.
* Coccinelle scripts added to scripts/coccinelle.

Eduardo Habkost (3):
  error: Remove NULL checks on error_propagate() calls
  error: Remove unnecessary local_err variables
  [RFC] Remove unnecessary variables for function return value

 audio/audio.c | 10 ++-
 block.c   | 26 -
 block/archipelago.c   |  4 +--
 block/qcow2-cluster.c |  7 ++---
 block/qcow2-refcount.c|  7 ++---
 block/qcow2.c |  4 +--
 block/quorum.c|  4 +--
 block/raw-posix.c | 24 +++
 block/raw_bsd.c   |  9 +-
 block/rbd.c   |  5 +---
 block/snapshot.c  |  4 +--
 block/vmdk.c  |  6 ++--
 block/vvfat.c |  5 +---
 blockdev.c| 26 +
 bootdevice.c  |  4 +--
 dump.c|  4 +--
 hw/acpi/aml-build.c   | 13 ++---
 hw/audio/intel-hda.c  |  5 +---
 hw/display/vga.c  |  4 +--
 hw/ide/qdev.c |  4 +--
 hw/intc/s390_flic_kvm.c   |  5 ++--
 hw/net/ne2000-isa.c   |  4 +--
 hw/pci-host/uninorth.c|  5 +---
 hw/ppc/spapr_vio.c|  7 +
 hw/s390x/s390-virtio-ccw.c|  5 +---
 hw/s390x/virtio-ccw.c | 42 +--
 hw/scsi/megasas.c | 10 +--
 hw/scsi/scsi-generic.c|  5 +---
 hw/timer/mc146818rtc.c|  5 +---
 hw/usb/dev-storage.c  |  4 +--
 hw/virtio/virtio-pci.c|  4 +--
 linux-user/signal.c   | 15 +++---
 page_cache.c  |  5 +---
 qga/commands-posix.c  |  4 +--
 qga/commands-win32.c  | 14 ++---
 qobject/qlist.c   |  5 +---
 qom/object.c  |  4 +--
 scripts/coccinelle/error_propagate_null.cocci | 10 +++
 scripts/coccinelle/remove_local_err.cocci | 27 +
 scripts/coccinelle/return_directly.cocci  | 19 
 target-i386/cpu.c |  4 +--
 target-i386/fpu_helper.c  | 10 ++-
 target-i386/kvm.c |  5 ++--
 target-mips/dsp_helper.c  | 15 ++
 target-mips/op_helper.c   |  4 +--
 target-s390x/helper.c |  6 +---
 target-sparc/cc_helper.c  | 25 
 target-tricore/op_helper.c| 13 +++--
 tests/display-vga-test.c  |  6 +---
 tests/endianness-test.c   |  5 +---
 tests/i440fx-test.c   |  4 +--
 tests/intel-hda-test.c|  6 +---
 tests/test-filter-redirector.c|  6 +---
 tests/virtio-blk-test.c   |  5 +---
 tests/virtio-console-test.c   |  6 +---
 tests/virtio-net-test.c   |  6 +---
 tests/virtio-scsi-test.c  |  6 +---
 tests/wdt_ib700-test.c|  6 +---
 ui/cursor.c   | 10 ++-
 ui/qemu-pixman.c  | 11 ++-
 util/module.c |  6 +---
 vl.c  |  5 +---
 62 files changed, 160 insertions(+), 384 deletions(-)
 create mode 100644 scripts/coccinelle/error_propagate_null.cocci
 create mode 100644 scripts/coccinelle/remove_local_err.cocci
 create mode 100644 scripts/coccinelle/return_directly.cocci

-- 
2.5.5

[Qemu-devel] [PATCH v2 1/3] error: Remove NULL checks on error_propagate() calls

[Eduardo Habkost]

error_propagate() already ignores local_err==NULL, so there's no
need to check it before calling.

Coccinelle patch used to perform the changes added to
scripts/coccinelle/error_propagate_null.cocci.

Signed-off-by: Eduardo Habkost 
---
 block.c   | 20 +--
 block/qcow2.c |  4 +---
 block/quorum.c|  4 +---
 block/raw-posix.c | 16 ---
 block/raw_bsd.c   |  4 +---
 block/snapshot.c  |  4 +---
 blockdev.c| 12 +++-
 bootdevice.c  |  4 +---
 dump.c|  4 +---
 hw/ide/qdev.c |  4 +---
 hw/net/ne2000-isa.c   |  4 +---
 hw/s390x/virtio-ccw.c | 28 +++
 hw/usb/dev-storage.c  |  4 +---
 qga/commands-win32.c  |  8 ++--
 qom/object.c  |  4 +---
 scripts/coccinelle/error_propagate_null.cocci | 10 ++
 16 files changed, 41 insertions(+), 93 deletions(-)
 create mode 100644 scripts/coccinelle/error_propagate_null.cocci

diff --git a/block.c b/block.c
index f54bc25..ecca55a 100644
--- a/block.c
+++ b/block.c
@@ -301,9 +301,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
 assert(cco->drv);
 
 ret = cco->drv->bdrv_create(cco->filename, cco->opts, &local_err);
-if (local_err) {
-error_propagate(&cco->err, local_err);
-}
+error_propagate(&cco->err, local_err);
 cco->ret = ret;
 }
 
@@ -364,9 +362,7 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, 
Error **errp)
 }
 
 ret = bdrv_create(drv, filename, opts, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 return ret;
 }
 
@@ -1760,18 +1756,14 @@ fail:
 QDECREF(options);
 bs->options = NULL;
 bdrv_unref(bs);
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 return NULL;
 
 close_and_fail:
 bdrv_unref(bs);
 QDECREF(snapshot_options);
 QDECREF(options);
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 return NULL;
 }
 
@@ -3591,9 +3583,7 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 out:
 qemu_opts_del(opts);
 qemu_opts_free(create_opts);
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 }
 
 AioContext *bdrv_get_aio_context(BlockDriverState *bs)
diff --git a/block/qcow2.c b/block/qcow2.c
index 6f5fb81..4504846 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2394,9 +2394,7 @@ static int qcow2_create(const char *filename, QemuOpts 
*opts, Error **errp)
 ret = qcow2_create2(filename, size, backing_file, backing_fmt, flags,
 cluster_size, prealloc, opts, version, refcount_order,
 &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 
 finish:
 g_free(backing_file);
diff --git a/block/quorum.c b/block/quorum.c
index ec6f3b9..331b726 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -971,9 +971,7 @@ close_exit:
 exit:
 qemu_opts_del(opts);
 /* propagate error */
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 return ret;
 }
 
diff --git a/block/raw-posix.c b/block/raw-posix.c
index ce2e20f..cb663d8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -587,9 +587,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 
 s->type = FTYPE_FILE;
 ret = raw_open_common(bs, options, flags, 0, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 return ret;
 }
 
@@ -2239,9 +2237,7 @@ hdev_open_Mac_error:
 
 ret = raw_open_common(bs, options, flags, 0, &local_err);
 if (ret < 0) {
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 #if defined(__APPLE__) && defined(__MACH__)
 if (*bsd_path) {
 filename = bsd_path;
@@ -2453,9 +2449,7 @@ static int cdrom_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 /* open will not fail even if no CD is inserted, so add O_NONBLOCK */
 ret = raw_open_common(bs, options, flags, O_NONBLOCK, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-}
+error_propagate(errp, local_err);
 return ret;
 }
 
@@ -2573,9 +2567,7 @@ static int cdrom_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 re

[Qemu-devel] [RFC v2 3/3] Remove unnecessary variables for function return value

[Eduardo Habkost]

Use Coccinelle script to replace 'ret = E; return ret' with
'return E'. The script will do the substitution only when the
function return type and variable type are the same.

Sending as RFC because the patch looks more intrusive than the
others. Probably better to split it per subsystem and let each
maintainer review and apply it?

Signed-off-by: Eduardo Habkost 
---
 audio/audio.c| 10 ++
 block.c  |  4 +---
 block/archipelago.c  |  4 +---
 block/qcow2-cluster.c|  7 ++-
 block/qcow2-refcount.c   |  7 ++-
 block/raw-posix.c|  8 ++--
 block/raw_bsd.c  |  5 +
 block/rbd.c  |  5 +
 block/vmdk.c |  6 ++
 block/vvfat.c|  5 +
 hw/acpi/aml-build.c  | 13 +++--
 hw/audio/intel-hda.c |  5 +
 hw/display/vga.c |  4 +---
 hw/intc/s390_flic_kvm.c  |  5 ++---
 hw/pci-host/uninorth.c   |  5 +
 hw/ppc/spapr_vio.c   |  7 +--
 hw/scsi/megasas.c| 10 +-
 hw/scsi/scsi-generic.c   |  5 +
 hw/timer/mc146818rtc.c   |  5 +
 hw/virtio/virtio-pci.c   |  4 +---
 linux-user/signal.c  | 15 ---
 page_cache.c |  5 +
 qga/commands-posix.c |  4 +---
 qga/commands-win32.c |  6 +-
 qobject/qlist.c  |  5 +
 scripts/coccinelle/return_directly.cocci | 19 +++
 target-i386/fpu_helper.c | 10 ++
 target-i386/kvm.c|  5 ++---
 target-mips/dsp_helper.c | 15 +++
 target-mips/op_helper.c  |  4 +---
 target-s390x/helper.c|  6 +-
 target-sparc/cc_helper.c | 25 +
 target-tricore/op_helper.c   | 13 -
 tests/display-vga-test.c |  6 +-
 tests/endianness-test.c  |  5 +
 tests/i440fx-test.c  |  4 +---
 tests/intel-hda-test.c   |  6 +-
 tests/test-filter-redirector.c   |  6 +-
 tests/virtio-blk-test.c  |  5 +
 tests/virtio-console-test.c  |  6 +-
 tests/virtio-net-test.c  |  6 +-
 tests/virtio-scsi-test.c |  6 +-
 tests/wdt_ib700-test.c   |  6 +-
 ui/cursor.c  | 10 ++
 ui/qemu-pixman.c | 11 +++
 util/module.c|  6 +-
 vl.c |  5 +
 47 files changed, 90 insertions(+), 254 deletions(-)
 create mode 100644 scripts/coccinelle/return_directly.cocci

diff --git a/audio/audio.c b/audio/audio.c
index e60c124..9d4dcc7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1131,8 +1131,6 @@ static void audio_timer (void *opaque)
  */
 int AUD_write (SWVoiceOut *sw, void *buf, int size)
 {
-int bytes;
-
 if (!sw) {
 /* XXX: Consider options */
 return size;
@@ -1143,14 +1141,11 @@ int AUD_write (SWVoiceOut *sw, void *buf, int size)
 return 0;
 }
 
-bytes = sw->hw->pcm_ops->write (sw, buf, size);
-return bytes;
+return sw->hw->pcm_ops->write(sw, buf, size);
 }
 
 int AUD_read (SWVoiceIn *sw, void *buf, int size)
 {
-int bytes;
-
 if (!sw) {
 /* XXX: Consider options */
 return size;
@@ -1161,8 +1156,7 @@ int AUD_read (SWVoiceIn *sw, void *buf, int size)
 return 0;
 }
 
-bytes = sw->hw->pcm_ops->read (sw, buf, size);
-return bytes;
+return sw->hw->pcm_ops->read(sw, buf, size);
 }
 
 int AUD_get_buffer_size_out (SWVoiceOut *sw)
diff --git a/block.c b/block.c
index d516ab6..c537307 100644
--- a/block.c
+++ b/block.c
@@ -351,15 +351,13 @@ out:
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
 {
 BlockDriver *drv;
-int ret;
 
 drv = bdrv_find_protocol(filename, true, errp);
 if (drv == NULL) {
 return -ENOENT;
 }
 
-ret = bdrv_create(drv, filename, opts, errp);
-return ret;
+return bdrv_create(drv, filename, opts, errp);
 }
 
 /**
diff --git a/block/archipelago.c b/block/archipelago.c
index b9f5e69..37b8aca 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -974,11 +974,9 @@ err_exit2:
 
 static int64_t qemu_archipelago_getlength(BlockDriverState *bs)
 {
-int64_t ret;
 BDRVArchipelagoState *s = bs->opaque;
 
-ret = archipelago_volume_info(s);
-return ret;
+return archipelago_volume_info(s);
 }
 
 static int qemu_archipelago_truncate(BlockDriverState *b

Re: [Qemu-devel] [PATCH v6 8/9] target-mips: Add nan2008 flavor of ..

[Maciej W. Rozycki]

On Fri, 10 Jun 2016, Aleksandar Markovic wrote:

> The changes that make QEMU behavior the same as hardware behavior (in 
> relation to CEIL, CVT, FLOOR, ROUND, TRUNC Mips instructions) are 
> already contained in this patch.

 Good, however that means that you've really combined two logically 
separate changes into a single patch:

1. A bug fix for SoftFloat legacy-NaN (original) MIPS support, which has 
   been there probably since forever (i.e. since the MIPS target was added 
   to QEMU).

2. A new feature for 2008-NaN MIPS support.

To me it really looks like the two need to be separate patches, with the 
bug fix applied first (or among any other bug fixes at the beginning) in 
the patch set, or even as a separate change marked as a prerequisite for 
the rest of the changes.

 The bug fix will then be self-contained and more prominently exposed, 
rather than being buried among feature additions.  It can then be 
independently reviewed and likely more easily accepted as long as it is 
technically correct.  It can also be cherry-picked and backported easily 
if necessary, perhaps outside the upstream tree.

 Review of the new feature set can then follow, once the bug(s) have been 
fixed.

> I just mentioned Mips-A / Mips-B / SoftFloat differences as an 
> explanation/observation related to the change in this patch.

 Maybe it's just myself, but from your description I got the impression 
that your change preserves the status quo and the explanation merely 
serves the purpose of documenting it.  Please consider rewriting it such 
that it is unambiguous that the SoftFloat bug is being fixed with your 
change.

 Obviously once you've made the bug fix a separate change, it'll become 
unambiguous naturally, as then you won't have the 2008-NaN feature along 
it obfuscating the picture.

  Maciej

[Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

[Eduardo Habkost]

This patch simplifies code that uses a local_err variable just to
immediately use it for an error_propagate() call.

Coccinelle patch used to perform the changes added to
scripts/coccinelle/remove_local_err.cocci.

Signed-off-by: Eduardo Habkost 
---
 block.c   |  8 ++--
 block/raw-posix.c |  8 ++--
 block/raw_bsd.c   |  4 +---
 blockdev.c| 16 +---
 hw/s390x/s390-virtio-ccw.c|  5 +
 hw/s390x/virtio-ccw.c | 28 +++-
 scripts/coccinelle/remove_local_err.cocci | 27 +++
 target-i386/cpu.c |  4 +---
 8 files changed, 46 insertions(+), 54 deletions(-)
 create mode 100644 scripts/coccinelle/remove_local_err.cocci

diff --git a/block.c b/block.c
index ecca55a..d516ab6 100644
--- a/block.c
+++ b/block.c
@@ -294,14 +294,12 @@ typedef struct CreateCo {
 
 static void coroutine_fn bdrv_create_co_entry(void *opaque)
 {
-Error *local_err = NULL;
 int ret;
 
 CreateCo *cco = opaque;
 assert(cco->drv);
 
-ret = cco->drv->bdrv_create(cco->filename, cco->opts, &local_err);
-error_propagate(&cco->err, local_err);
+ret = cco->drv->bdrv_create(cco->filename, cco->opts, &cco->err);
 cco->ret = ret;
 }
 
@@ -353,7 +351,6 @@ out:
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
 {
 BlockDriver *drv;
-Error *local_err = NULL;
 int ret;
 
 drv = bdrv_find_protocol(filename, true, errp);
@@ -361,8 +358,7 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, 
Error **errp)
 return -ENOENT;
 }
 
-ret = bdrv_create(drv, filename, opts, &local_err);
-error_propagate(errp, local_err);
+ret = bdrv_create(drv, filename, opts, errp);
 return ret;
 }
 
diff --git a/block/raw-posix.c b/block/raw-posix.c
index cb663d8..d7397bf 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -582,12 +582,10 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 Error **errp)
 {
 BDRVRawState *s = bs->opaque;
-Error *local_err = NULL;
 int ret;
 
 s->type = FTYPE_FILE;
-ret = raw_open_common(bs, options, flags, 0, &local_err);
-error_propagate(errp, local_err);
+ret = raw_open_common(bs, options, flags, 0, errp);
 return ret;
 }
 
@@ -2442,14 +2440,12 @@ static int cdrom_open(BlockDriverState *bs, QDict 
*options, int flags,
   Error **errp)
 {
 BDRVRawState *s = bs->opaque;
-Error *local_err = NULL;
 int ret;
 
 s->type = FTYPE_CD;
 
 /* open will not fail even if no CD is inserted, so add O_NONBLOCK */
-ret = raw_open_common(bs, options, flags, O_NONBLOCK, &local_err);
-error_propagate(errp, local_err);
+ret = raw_open_common(bs, options, flags, O_NONBLOCK, errp);
 return ret;
 }
 
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 5af11b6..b51ac98 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -190,11 +190,9 @@ static int raw_has_zero_init(BlockDriverState *bs)
 
 static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
-Error *local_err = NULL;
 int ret;
 
-ret = bdrv_create_file(filename, opts, &local_err);
-error_propagate(errp, local_err);
+ret = bdrv_create_file(filename, opts, errp);
 return ret;
 }
 
diff --git a/blockdev.c b/blockdev.c
index 028dba3..3b6d242 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3654,7 +3654,6 @@ void qmp_blockdev_mirror(const char *device, const char 
*target,
 BlockBackend *blk;
 BlockDriverState *target_bs;
 AioContext *aio_context;
-Error *local_err = NULL;
 
 blk = blk_by_name(device);
 if (!blk) {
@@ -3678,16 +3677,11 @@ void qmp_blockdev_mirror(const char *device, const char 
*target,
 
 bdrv_set_aio_context(target_bs, aio_context);
 
-blockdev_mirror_common(bs, target_bs,
-   has_replaces, replaces, sync,
-   has_speed, speed,
-   has_granularity, granularity,
-   has_buf_size, buf_size,
-   has_on_source_error, on_source_error,
-   has_on_target_error, on_target_error,
-   true, true,
-   &local_err);
-error_propagate(errp, local_err);
+blockdev_mirror_common(bs, target_bs, has_replaces, replaces, sync,
+   has_speed, speed, has_granularity, granularity,
+   has_buf_size, buf_size, has_on_source_error,
+   on_source_error, has_on_target_error,
+   on_target_error, true, true, errp);
 
 aio_context_release(aio_context);
 }
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 95ff5e3..b7112d0 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s

Re: [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv()

[Eric Blake]

On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> We already have a byte-based bdrv_pwritev(), but the read counterpart
> was still missing. This commit adds it.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/io.c| 20 +---
>  include/block/block.h |  1 +
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 

Worth adding a flags argument while at it? But bdrv_pwritev() lacks one,
so for symmetry reasons, I'm okay if you don't bother.

> +int bdrv_preadv(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
> +{
> +int ret;
> +
> +ret = bdrv_prwv_co(bs, offset, qiov, false, 0);
> +if (ret < 0) {
> +return ret;
> +}
> +
> +return qiov->size;

This implies we never have a short read, it's an all-or-none error or
success.  Matches what we've done elsewhere, so I guess it's right.

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/3] error: Remove NULL checks on error_propagate() calls

[Eric Blake]

On 06/10/2016 02:12 PM, Eduardo Habkost wrote:
> error_propagate() already ignores local_err==NULL, so there's no
> need to check it before calling.
> 
> Coccinelle patch used to perform the changes added to
> scripts/coccinelle/error_propagate_null.cocci.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  block.c   | 20 +--
>  block/qcow2.c |  4 +---
>  block/quorum.c|  4 +---
>  block/raw-posix.c | 16 ---
>  block/raw_bsd.c   |  4 +---
>  block/snapshot.c  |  4 +---
>  blockdev.c| 12 +++-
>  bootdevice.c  |  4 +---
>  dump.c|  4 +---
>  hw/ide/qdev.c |  4 +---
>  hw/net/ne2000-isa.c   |  4 +---
>  hw/s390x/virtio-ccw.c | 28 
> +++
>  hw/usb/dev-storage.c  |  4 +---
>  qga/commands-win32.c  |  8 ++--
>  qom/object.c  |  4 +---
>  scripts/coccinelle/error_propagate_null.cocci | 10 ++
>  16 files changed, 41 insertions(+), 93 deletions(-)
>  create mode 100644 scripts/coccinelle/error_propagate_null.cocci

You can do:
git config diff.orderFile /path/to/file

and then set up a list of globs in /path/to/file in order to influence
your diffs; in my case, I stuck 'scripts/coccinelle/*' near the top of
my order file, as I find that to be a more useful part of the patch than
the churn from running it.  But it doesn't affect patch correctness,
just ease of review.

Reviewed-by: Eric Blake 

> +++ b/scripts/coccinelle/error_propagate_null.cocci
> @@ -0,0 +1,10 @@
> +// error_propagate() already ignores local_err==NULL, so there's
> +// no need to check it before calling.
> +
> +@@
> +identifier L;
> +expression E;
> +@@
> +-if (L) {
> + error_propagate(E, L);
> +-}
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

[Eric Blake]

On 06/10/2016 02:12 PM, Eduardo Habkost wrote:
> This patch simplifies code that uses a local_err variable just to
> immediately use it for an error_propagate() call.
> 
> Coccinelle patch used to perform the changes added to
> scripts/coccinelle/remove_local_err.cocci.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  block.c   |  8 ++--
>  block/raw-posix.c |  8 ++--
>  block/raw_bsd.c   |  4 +---
>  blockdev.c| 16 +---
>  hw/s390x/s390-virtio-ccw.c|  5 +
>  hw/s390x/virtio-ccw.c | 28 +++-
>  scripts/coccinelle/remove_local_err.cocci | 27 +++
>  target-i386/cpu.c |  4 +---
>  8 files changed, 46 insertions(+), 54 deletions(-)
>  create mode 100644 scripts/coccinelle/remove_local_err.cocci
> 

> +++ b/block.c
> @@ -294,14 +294,12 @@ typedef struct CreateCo {
>  
>  static void coroutine_fn bdrv_create_co_entry(void *opaque)
>  {
> -Error *local_err = NULL;
>  int ret;
>  
>  CreateCo *cco = opaque;
>  assert(cco->drv);
>  
> -ret = cco->drv->bdrv_create(cco->filename, cco->opts, &local_err);
> -error_propagate(&cco->err, local_err);
> +ret = cco->drv->bdrv_create(cco->filename, cco->opts, &cco->err);
>  cco->ret = ret;

This hunk doesn't get simplified by 3/3; you may want to consider a
manual followup to drop 'int ret' and just assign
cco->drv->bdrv_create() directly to cco->ret.  But doesn't change this
patch.


> +++ b/blockdev.c
> @@ -3654,7 +3654,6 @@ void qmp_blockdev_mirror(const char *device, const char 
> *target,
>  BlockBackend *blk;
>  BlockDriverState *target_bs;
>  AioContext *aio_context;
> -Error *local_err = NULL;
>  
>  blk = blk_by_name(device);
>  if (!blk) {
> @@ -3678,16 +3677,11 @@ void qmp_blockdev_mirror(const char *device, const 
> char *target,
>  
>  bdrv_set_aio_context(target_bs, aio_context);
>  
> -blockdev_mirror_common(bs, target_bs,
> -   has_replaces, replaces, sync,
> -   has_speed, speed,
> -   has_granularity, granularity,
> -   has_buf_size, buf_size,
> -   has_on_source_error, on_source_error,
> -   has_on_target_error, on_target_error,
> -   true, true,
> -   &local_err);
> -error_propagate(errp, local_err);
> +blockdev_mirror_common(bs, target_bs, has_replaces, replaces, sync,
> +   has_speed, speed, has_granularity, granularity,
> +   has_buf_size, buf_size, has_on_source_error,
> +   on_source_error, has_on_target_error,
> +   on_target_error, true, true, errp);

Coccinelle messes a bit with the formatting (the old way explicitly
tried to pair related has_foo with foo). But I'm going to mess with it
again with my qapi patches for passing a boxed parameter rather than
lots of arguments, so don't worry about it.

> +++ b/scripts/coccinelle/remove_local_err.cocci
> @@ -0,0 +1,27 @@
> +// Replace unnecessary usage of local_err variable with
> +// direct usage of errp argument
> +
> +@@
> +expression list ARGS;
> +expression F2;
> +identifier LOCAL_ERR;
> +expression ERRP;
> +idexpression V;
> +typedef Error;
> +expression I;
> +@@
> + {
> + ...
> +-Error *LOCAL_ERR;
> + ... when != LOCAL_ERR
> +(
> +-F2(ARGS, &LOCAL_ERR);
> +-error_propagate(ERRP, LOCAL_ERR);
> ++F2(ARGS, ERRP);
> +|
> +-V = F2(ARGS, &LOCAL_ERR);
> +-error_propagate(ERRP, LOCAL_ERR);
> ++V = F2(ARGS, ERRP);
> +)
> + ... when != LOCAL_ERR
> + }

Looks good.
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC v2 3/3] Remove unnecessary variables for function return value

[Eric Blake]

On 06/10/2016 02:12 PM, Eduardo Habkost wrote:
> Use Coccinelle script to replace 'ret = E; return ret' with
> 'return E'. The script will do the substitution only when the
> function return type and variable type are the same.
> 
> Sending as RFC because the patch looks more intrusive than the
> others. Probably better to split it per subsystem and let each
> maintainer review and apply it?

Borderline on size, so yeah, splitting it across several subsystems may
ease review (although then the patch will be committed in piecemeal
fashion, and you'd have to ensure the script/coccinelle/ patch goes in
first...)

At any rate, it's fairly mechanical, so I'll review it as is:

> 
> Signed-off-by: Eduardo Habkost 
> ---

>  47 files changed, 90 insertions(+), 254 deletions(-)
>  create mode 100644 scripts/coccinelle/return_directly.cocci

Nice diffstat.

> +++ b/block/qcow2-cluster.c
> @@ -154,11 +154,8 @@ static int l2_load(BlockDriverState *bs, uint64_t 
> l2_offset,
>  uint64_t **l2_table)
>  {
>  BDRVQcow2State *s = bs->opaque;
> -int ret;
> -
> -ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset, (void**) 
> l2_table);
> -
> -return ret;
> +return qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
> +   (void **)l2_table);

Coccinelle changed spacing of the cast. I don't care strongly enough to
require a touchup if this is the only thing, but may be worth fixing if
you have to respin (for example to split up by submaintainers).

> +++ b/block/raw_bsd.c
> @@ -190,10 +190,7 @@ static int raw_has_zero_init(BlockDriverState *bs)
>  
>  static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
>  {
> -int ret;
> -
> -ret = bdrv_create_file(filename, opts, errp);
> -return ret;
> +return bdrv_create_file(filename, opts, errp);
>  }

Potential followup patch: delete raw_create(), and:
- .bdrv_create = &raw_create,
+ .bdrv_create = bdrv_create_file,

but doesn't affect this patch.

> +++ b/block/rbd.c
> @@ -875,10 +875,7 @@ static int qemu_rbd_snap_rollback(BlockDriverState *bs,
>const char *snapshot_name)
>  {
>  BDRVRBDState *s = bs->opaque;
> -int r;
> -
> -r = rbd_snap_rollback(s->image, snapshot_name);
> -return r;
> +return rbd_snap_rollback(s->image, snapshot_name);

Coccinelle lost the blank line between declarations and statements;
might be nice to manually touch that up and add it back in.

> +++ b/hw/ppc/spapr_vio.c
> @@ -57,12 +57,7 @@ static char *spapr_vio_get_dev_name(DeviceState *qdev)
>  {
>  VIOsPAPRDevice *dev = VIO_SPAPR_DEVICE(qdev);
>  VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
> -char *name;
> -
> -/* Device tree style name device@reg */
> -name = g_strdup_printf("%s@%x", pc->dt_name, dev->reg);
> -
> -return name;
> +return g_strdup_printf("%s@%x", pc->dt_name, dev->reg);

Coccinelle lost the comment; might be worth keeping it.

> +++ b/hw/scsi/megasas.c
> @@ -410,17 +410,9 @@ static void megasas_encode_lba(uint8_t *cdb, uint64_t 
> lba,
>  static uint64_t megasas_fw_time(void)
>  {
>  struct tm curtime;
> -uint64_t bcd_time;
>  
>  qemu_get_timedate(&curtime, 0);
> -bcd_time = ((uint64_t)curtime.tm_sec & 0xff) << 48 |
> -((uint64_t)curtime.tm_min & 0xff)  << 40 |
> -((uint64_t)curtime.tm_hour & 0xff) << 32 |
> -((uint64_t)curtime.tm_mday & 0xff) << 24 |
> -((uint64_t)curtime.tm_mon & 0xff)  << 16 |
> -((uint64_t)(curtime.tm_year + 1900) & 0x);
> -
> -return bcd_time;
> +return ((uint64_t)curtime.tm_sec & 0xff) << 48 | 
> ((uint64_t)curtime.tm_min & 0xff) << 40 | ((uint64_t)curtime.tm_hour & 0xff) 
> << 32 | ((uint64_t)curtime.tm_mday & 0xff) << 24 | ((uint64_t)curtime.tm_mon 
> & 0xff) << 16 | ((uint64_t)(curtime.tm_year + 1900) & 0x);

Eww. Coccinelle botched that formatting.  You'll need to manually fix
this one.

> +++ b/hw/timer/mc146818rtc.c
> @@ -105,12 +105,9 @@ static inline bool rtc_running(RTCState *s)
>  
>  static uint64_t get_guest_rtc_ns(RTCState *s)
>  {
> -uint64_t guest_rtc;
>  uint64_t guest_clock = qemu_clock_get_ns(rtc_clock);
>  
> -guest_rtc = s->base_rtc * NANOSECONDS_PER_SECOND +
> -guest_clock - s->last_update + s->offset;
> -return guest_rtc;
> +return s->base_rtc * NANOSECONDS_PER_SECOND + guest_clock - 
> s->last_update + s->offset;
>  }

Worth wrapping that line again (not as bad as the megasas one, though).

> +++ b/qga/commands-win32.c
> @@ -1150,7 +1150,6 @@ out:
>  int64_t qmp_guest_get_time(Error **errp)
>  {
>  SYSTEMTIME ts = {0};
> -int64_t time_ns;
>  FILETIME tf;
>  
>  GetSystemTime(&ts);
> @@ -1164,10 +1163,7 @@ int64_t qmp_guest_get_time(Error **errp)
>  return -1;
>  }
>  
> -time_ns = int64_t)tf.dwHighDateTime << 32) | tf.dwLowDateTime)
> -- W32_FT_OFFSET) * 100;
> -
> -return time_ns;
> +return int64_t)tf.

Re: [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored

[Eric Blake]

On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> This brings it in line with .bdrv_save_vmstate().
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/io.c| 26 +-
>  block/qcow2.c |  6 +++---
>  block/sheepdog.c  | 13 ++---
>  include/block/block.h |  1 +
>  include/block/block_int.h |  4 ++--
>  5 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 11510cf..602c7d3 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1862,13 +1862,29 @@ int bdrv_writev_vmstate(BlockDriverState *bs, 
> QEMUIOVector *qiov, int64_t pos)
>  int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
>int64_t pos, int size)
>  {
> +QEMUIOVector qiov;
> +struct iovec iov = {
> +.iov_base   = buf,
> +.iov_len= size,
> +};
> +int ret;

Dead variable.

> +
> +qemu_iovec_init_external(&qiov, &iov, 1);
> +return bdrv_readv_vmstate(bs, &qiov, pos);
> +}
> +
> +int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
> +{
>  BlockDriver *drv = bs->drv;
> -if (!drv)
> +
> +if (!drv) {
>  return -ENOMEDIUM;
> -if (drv->bdrv_load_vmstate)
> -return drv->bdrv_load_vmstate(bs, buf, pos, size);
> -if (bs->file)
> -return bdrv_load_vmstate(bs->file->bs, buf, pos, size);
> +} else if (drv->bdrv_load_vmstate) {
> +return drv->bdrv_load_vmstate(bs, qiov, pos);
> +} else if (bs->file) {
> +return bdrv_readv_vmstate(bs->file->bs, qiov, pos);
> +}

Don't know that I would have used 'else if' after a return, but it's not
wrong, so no need to change.

With the dead 'ret' gone,
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno

[Eric Blake]

On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> The return value of .bdrv_load/save_vmstate() can be any non-negative
> number in case of success now. It used to be bytes/-errno.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/io.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 602c7d3..bca244c 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1839,9 +1839,16 @@ int bdrv_save_vmstate(BlockDriverState *bs, const 
> uint8_t *buf,
>  .iov_base   = (void *) buf,
>  .iov_len= size,
>  };
> +int ret;
>  
>  qemu_iovec_init_external(&qiov, &iov, 1);
> -return bdrv_writev_vmstate(bs, &qiov, pos);
> +
> +ret = bdrv_writev_vmstate(bs, &qiov, pos);
> +if (ret < 0) {
> +return ret;
> +}
> +
> +return size;
>  }
>  
>  int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t 
> pos)
> @@ -1870,7 +1877,12 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t 
> *buf,
>  int ret;

Aha, my complaint in v2 about it being dead means you need to reinstate
it here.

>  
>  qemu_iovec_init_external(&qiov, &iov, 1);
> -return bdrv_readv_vmstate(bs, &qiov, pos);
> +ret = bdrv_readv_vmstate(bs, &qiov, pos);
> +if (ret < 0) {
> +return ret;
> +}
> +
> +return size;
>  }
>  
>  int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
> 

Matches the semantics we have elsewhere (I'm not sure if 'size' is the
best choice if we ever need to support short read/write, but doesn't
seem to hurt).

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 01/31] Add optionrom compatible with fw_cfg DMA version

[Richard W.M. Jones]

On Fri, May 27, 2016 at 04:09:32PM +0200, Paolo Bonzini wrote:
> From: Marc Marí 
> 
> This optionrom is based on linuxboot.S.
> 
> Signed-off-by: Marc Marí 
> Signed-off-by: Richard W.M. Jones 
> Message-Id: <1464027093-24073-2-git-send-email-rjo...@redhat.com>
> [Add -fno-toplevel-reorder and fix Win32 compilation. - Paolo]
> Signed-off-by: Paolo Bonzini 
[...]

Hi Paolo,

Did this patch get dropped again?  It hasn't appeared upstream.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

[Qemu-devel] [PATCH] block-backend: allow flush on devices with open tray

[John Snow]

If a device still has an attached BDS because the medium has not yet
been removed, we will be unable to migrate to a new host because
blk_flush will return an error for that backend.

Replace the call to blk_is_available to blk_is_inserted to weaken
the check and allow flushes from the backend to work, while still
disallowing flushes from the frontend/device model to work.

This fixes a regression present in 2.6.0 caused by the following commit:
fe1a9cbc339bb54d20f1ca4c1e8788d16944d5cf
block: Move some bdrv_*_all() functions to BB

Signed-off-by: John Snow 
---
 block/block-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 34500e6..d1e875e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1122,7 +1122,7 @@ int blk_co_flush(BlockBackend *blk)
 
 int blk_flush(BlockBackend *blk)
 {
-if (!blk_is_available(blk)) {
+if (!blk_is_inserted(blk)) {
 return -ENOMEDIUM;
 }
 
-- 
2.4.11

Re: [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns

[Eric Blake]

On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> This allows drivers to share code between normal I/O and vmstate
> accesses.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/io.c| 80 
> ++-
>  include/block/block_int.h | 10 +++---
>  2 files changed, 64 insertions(+), 26 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly

[Eric Blake]

On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> We don't really want to go through the block layer in order to read from
> or write to the vmstate in a qcow2 image. Doing so required a few ugly
> hacks like saving and restoring the old image size (because writing to
> vmstate offsets would increase the image size) or disabling the "reads
> after EOF = zeroes" logic. When calling the right functions directly,
> these hacks aren't necessary any more.
> 
> Note that .bdrv_vmstate_load/save() return 0 instead of the number of
> bytes in case of success now.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/qcow2.c | 24 
>  1 file changed, 4 insertions(+), 20 deletions(-)
> 

> +++ b/block/qcow2.c
> @@ -2903,36 +2903,20 @@ static int qcow2_save_vmstate(BlockDriverState *bs, 
> QEMUIOVector *qiov,
>int64_t pos)
>  {
>  BDRVQcow2State *s = bs->opaque;
> -int64_t total_sectors = bs->total_sectors;
> -bool zero_beyond_eof = bs->zero_beyond_eof;
> -int ret;
>  
>  BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
> -bs->zero_beyond_eof = false;
> -ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
> -bs->zero_beyond_eof = zero_beyond_eof;
> -
> -/* bdrv_co_do_writev will have increased the total_sectors value to 
> include
> - * the VM state - the VM state is however not an actual part of the block
> - * device, therefore, we need to restore the old value. */
> -bs->total_sectors = total_sectors;
> -
> -return ret;
> +return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
> +qiov->size, qiov, 0);
>  }

bs->drv->bdrv_co_pwritev() is an optional interface; not all the drivers
have it yet.  Should you be asserting that it exists, and/or returning
an error if it does not?

>  
>  static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
>int64_t pos)
>  {
>  BDRVQcow2State *s = bs->opaque;
> -bool zero_beyond_eof = bs->zero_beyond_eof;
> -int ret;
>  
>  BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
> -bs->zero_beyond_eof = false;
> -ret = bdrv_preadv(bs, qcow2_vm_state_offset(s) + pos, qiov);
> -bs->zero_beyond_eof = zero_beyond_eof;
> -
> -return ret;
> +return bs->drv->bdrv_co_preadv(bs, qcow2_vm_state_offset(s) + pos,
> +   qiov->size, qiov, 0);

Ditto.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

[Eduardo Habkost]

On Fri, Jun 10, 2016 at 02:59:55PM -0600, Eric Blake wrote:
> On 06/10/2016 02:12 PM, Eduardo Habkost wrote:
> > This patch simplifies code that uses a local_err variable just to
> > immediately use it for an error_propagate() call.
> > 
> > Coccinelle patch used to perform the changes added to
> > scripts/coccinelle/remove_local_err.cocci.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  block.c   |  8 ++--
> >  block/raw-posix.c |  8 ++--
> >  block/raw_bsd.c   |  4 +---
> >  blockdev.c| 16 +---
> >  hw/s390x/s390-virtio-ccw.c|  5 +
> >  hw/s390x/virtio-ccw.c | 28 +++-
> >  scripts/coccinelle/remove_local_err.cocci | 27 +++
> >  target-i386/cpu.c |  4 +---
> >  8 files changed, 46 insertions(+), 54 deletions(-)
> >  create mode 100644 scripts/coccinelle/remove_local_err.cocci
> > 
> 
> > +++ b/block.c
> > @@ -294,14 +294,12 @@ typedef struct CreateCo {
> >  
> >  static void coroutine_fn bdrv_create_co_entry(void *opaque)
> >  {
> > -Error *local_err = NULL;
> >  int ret;
> >  
> >  CreateCo *cco = opaque;
> >  assert(cco->drv);
> >  
> > -ret = cco->drv->bdrv_create(cco->filename, cco->opts, &local_err);
> > -error_propagate(&cco->err, local_err);
> > +ret = cco->drv->bdrv_create(cco->filename, cco->opts, &cco->err);
> >  cco->ret = ret;
> 
> This hunk doesn't get simplified by 3/3; you may want to consider a
> manual followup to drop 'int ret' and just assign
> cco->drv->bdrv_create() directly to cco->ret.  But doesn't change this
> patch.

This could become yet another Coccinelle script, but we need to
be careful about type conversions, and tell it to do it only if
the types of 'ret', 'cc->drv->bdrv_create()' and 'cco->ret' are
the same.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof

[Eric Blake]

On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> It is always true for open images now.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   |  2 --
>  block/io.c| 51 
> +--
>  include/block/block_int.h |  3 ---
>  3 files changed, 23 insertions(+), 33 deletions(-)
> 

> +++ b/block/io.c
> @@ -1000,40 +1000,35 @@ static int coroutine_fn 
> bdrv_aligned_preadv(BlockDriverState *bs,
>  }
>  
>  /* Forward the request to the BlockDriver */
> -if (!bs->zero_beyond_eof) {
> -ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
> -} else {
> -/* Read zeros after EOF */
> -int64_t total_bytes, max_bytes;
> +int64_t total_bytes, max_bytes;

This declaration is now in the middle of the function.  Shouldn't you
hoist it to the beginning?

That's minor enough to fix on pull request, so:
Reviewed-by: Eric Blake 

I'll rebase my pending byte-based BlockLimits series on top of this.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] block-backend: allow flush on devices with open tray

[Eric Blake]

On 06/10/2016 03:59 PM, John Snow wrote:
> If a device still has an attached BDS because the medium has not yet
> been removed, we will be unable to migrate to a new host because
> blk_flush will return an error for that backend.
> 
> Replace the call to blk_is_available to blk_is_inserted to weaken
> the check and allow flushes from the backend to work, while still
> disallowing flushes from the frontend/device model to work.
> 
> This fixes a regression present in 2.6.0 caused by the following commit:
> fe1a9cbc339bb54d20f1ca4c1e8788d16944d5cf
> block: Move some bdrv_*_all() functions to BB
> 
> Signed-off-by: John Snow 
> ---
>  block/block-backend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Worth testsuite coverage to prevent future regressions?

At any rate,
Reviewed-by: Eric Blake 

> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 34500e6..d1e875e 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1122,7 +1122,7 @@ int blk_co_flush(BlockBackend *blk)
>  
>  int blk_flush(BlockBackend *blk)
>  {
> -if (!blk_is_available(blk)) {
> +if (!blk_is_inserted(blk)) {
>  return -ENOMEDIUM;
>  }
>  
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 4/4] linux-user: update get_thread_area/set_thread_area strace

[Laurent Vivier]

   int get_thread_area(struct user_desc *u_info);
   int set_thread_area(struct user_desc *u_info);

Signed-off-by: Laurent Vivier 
---
 linux-user/strace.list | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.list b/linux-user/strace.list
index 7c54dc6..aa967a2 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -337,7 +337,8 @@
 { TARGET_NR_getsockopt, "getsockopt" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_get_thread_area
-{ TARGET_NR_get_thread_area, "get_thread_area" , NULL, NULL, NULL },
+{ TARGET_NR_get_thread_area, "get_thread_area", "%s(0x"TARGET_ABI_FMT_lx")",
+  NULL, NULL },
 #endif
 #ifdef TARGET_NR_gettid
 { TARGET_NR_gettid, "gettid" , NULL, NULL, NULL },
@@ -1234,7 +1235,8 @@
 { TARGET_NR_setsockopt, "setsockopt" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_set_thread_area
-{ TARGET_NR_set_thread_area, "set_thread_area" , NULL, NULL, NULL },
+{ TARGET_NR_set_thread_area, "set_thread_area", "%s(0x"TARGET_ABI_FMT_lx")",
+  NULL, NULL },
 #endif
 #ifdef TARGET_NR_set_tid_address
 { TARGET_NR_set_tid_address, "set_tid_address" , NULL, NULL, NULL },
-- 
2.5.5

[Qemu-devel] [PATCH v2 1/4] linux-user: add socketcall() strace

[Laurent Vivier]

Signed-off-by: Laurent Vivier 
---
 linux-user/strace.c   | 549 ++
 linux-user/strace.list|   2 +-
 linux-user/syscall_defs.h |  22 +-
 3 files changed, 568 insertions(+), 5 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index c5980a1..46391c8 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -6,6 +6,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include "qemu.h"
 
@@ -58,10 +61,15 @@ UNUSED static void print_open_flags(abi_long, int);
 UNUSED static void print_syscall_prologue(const struct syscallname *);
 UNUSED static void print_syscall_epilogue(const struct syscallname *);
 UNUSED static void print_string(abi_long, int);
+UNUSED static void print_buf(abi_long addr, abi_long len, int last);
 UNUSED static void print_raw_param(const char *, abi_long, int);
 UNUSED static void print_timeval(abi_ulong, int);
 UNUSED static void print_number(abi_long, int);
 UNUSED static void print_signal(abi_ulong, int);
+UNUSED static void print_sockaddr(abi_ulong addr, abi_long addrlen);
+UNUSED static void print_socket_domain(int domain);
+UNUSED static void print_socket_type(int type);
+UNUSED static void print_socket_protocol(int domain, int type, int protocol);
 
 /*
  * Utility functions
@@ -147,6 +155,165 @@ print_signal(abi_ulong arg, int last)
 gemu_log("%s%s", signal_name, get_comma(last));
 }
 
+static void
+print_sockaddr(abi_ulong addr, abi_long addrlen)
+{
+struct target_sockaddr *sa;
+int i;
+int sa_family;
+
+sa = lock_user(VERIFY_READ, addr, addrlen, 1);
+if (sa) {
+sa_family = tswap16(sa->sa_family);
+switch (sa_family) {
+case AF_UNIX: {
+struct target_sockaddr_un *un = (struct target_sockaddr_un *)sa;
+int i;
+gemu_log("{sun_family=AF_UNIX,sun_path=\"");
+for (i = 0; i < addrlen -
+offsetof(struct target_sockaddr_un, sun_path) &&
+ un->sun_path[i]; i++) {
+gemu_log("%c", un->sun_path[i]);
+}
+gemu_log("\"}");
+break;
+}
+case AF_INET: {
+struct target_sockaddr_in *in = (struct target_sockaddr_in *)sa;
+uint8_t *c = (uint8_t *)&in->sin_addr.s_addr;
+gemu_log("{sin_family=AF_INET,sin_port=htons(%d),",
+ ntohs(in->sin_port));
+gemu_log("sin_addr=inet_addr(\"%d.%d.%d.%d\")",
+ c[0], c[1], c[2], c[3]);
+gemu_log("}");
+break;
+}
+case AF_PACKET: {
+struct target_sockaddr_ll *ll = (struct target_sockaddr_ll *)sa;
+uint8_t *c = (uint8_t *)&ll->sll_addr;
+gemu_log("{sll_family=AF_PACKET,"
+ "sll_protocol=htons(0x%04x),if%d,pkttype=",
+ ntohs(ll->sll_protocol), ll->sll_ifindex);
+switch (ll->sll_pkttype) {
+case PACKET_HOST:
+gemu_log("PACKET_HOST");
+break;
+case PACKET_BROADCAST:
+gemu_log("PACKET_BROADCAST");
+break;
+case PACKET_MULTICAST:
+gemu_log("PACKET_MULTICAST");
+break;
+case PACKET_OTHERHOST:
+gemu_log("PACKET_OTHERHOST");
+break;
+case PACKET_OUTGOING:
+gemu_log("PACKET_OUTGOING");
+break;
+default:
+gemu_log("%d", ll->sll_pkttype);
+break;
+}
+gemu_log(",sll_addr=%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x",
+ c[0], c[1], c[2], c[3], c[4], c[5], c[6], c[7]);
+gemu_log("}");
+break;
+}
+default:
+gemu_log("{sa_family=%d, sa_data={", sa->sa_family);
+for (i = 0; i < 13; i++) {
+gemu_log("%02x, ", sa->sa_data[i]);
+}
+gemu_log("%02x}", sa->sa_data[i]);
+gemu_log("}");
+break;
+}
+unlock_user(sa, addr, 0);
+} else {
+print_raw_param("0x"TARGET_ABI_FMT_lx, addr, 0);
+}
+gemu_log(", "TARGET_ABI_FMT_ld, addrlen);
+}
+
+static void
+print_socket_domain(int domain)
+{
+switch (domain) {
+case PF_UNIX:
+gemu_log("PF_UNIX");
+break;
+case PF_INET:
+gemu_log("PF_INET");
+break;
+case PF_PACKET:
+gemu_log("PF_PACKET");
+break;
+default:
+gemu_log("%d", domain);
+break;
+}
+}
+
+static void
+print_socket_type(int type)
+{
+switch (type) {
+case TARGET_SOCK_DGRAM:
+gemu_log("SOCK_DGRAM");
+break;
+case TARGET_SOCK_STREAM:
+gemu_log("SOCK_STREAM");
+break;
+case TARGET_SOCK_RAW:
+gemu_log("SOCK_RAW");
+break;
+case TARGET_SOCK_RDM:
+gemu_log("SOCK_RDM");

[Qemu-devel] [PATCH v2 0/4] linux-user: some strace improvements

[Laurent Vivier]

These patches for linux-user strace are living for years in my tree.

v2:

- remove abi_nothl, use tswap16() instead,
- check TARGET_SOCK_PACKET only with domain AF_INET (like in syscall.c)
- use an array in print_socketcall() to display the information
  according to SOCKOP number,
- Merge PATCH 2/5 into PATCH v2 1/4
- use TARGET_ABI_FMT_lx in set_thread_area/get_thread_area
- add a do_print_clone() to follow do_fork() order.

Laurent Vivier (4):
  linux-user: add socketcall() strace
  linux-user: add socket() strace
  linux-user: fix clone() strace
  linux-user: update get_thread_area/set_thread_area strace

 linux-user/strace.c   | 614 --
 linux-user/strace.list|  10 +-
 linux-user/syscall_defs.h |  22 +-
 3 files changed, 616 insertions(+), 30 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH v2 3/4] linux-user: fix clone() strace

[Laurent Vivier]

Signed-off-by: Laurent Vivier 
---
 linux-user/strace.c | 42 --
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 5a9df46..e032a3a 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -958,33 +958,31 @@ print_chmod(const struct syscallname *name,
 #endif
 
 #ifdef TARGET_NR_clone
+static void do_print_clone(unsigned int flags, abi_ulong newsp,
+   abi_ulong parent_tidptr, target_ulong newtls,
+   abi_ulong child_tidptr)
+{
+print_flags(clone_flags, flags, 0);
+print_raw_param("child_stack=0x" TARGET_ABI_FMT_lx, newsp, 0);
+print_raw_param("parent_tidptr=0x" TARGET_ABI_FMT_lx, parent_tidptr, 0);
+print_raw_param("tls=0x" TARGET_ABI_FMT_lx, newtls, 0);
+print_raw_param("child_tidptr=0x" TARGET_ABI_FMT_lx, child_tidptr, 1);
+}
+
 static void
 print_clone(const struct syscallname *name,
-abi_long arg0, abi_long arg1, abi_long arg2,
-abi_long arg3, abi_long arg4, abi_long arg5)
+abi_long arg1, abi_long arg2, abi_long arg3,
+abi_long arg4, abi_long arg5, abi_long arg6)
 {
 print_syscall_prologue(name);
-#if defined(TARGET_M68K)
-print_flags(clone_flags, arg0, 0);
-print_raw_param("newsp=0x" TARGET_ABI_FMT_lx, arg1, 1);
-#elif defined(TARGET_SH4) || defined(TARGET_ALPHA)
-print_flags(clone_flags, arg0, 0);
-print_raw_param("child_stack=0x" TARGET_ABI_FMT_lx, arg1, 0);
-print_raw_param("parent_tidptr=0x" TARGET_ABI_FMT_lx, arg2, 0);
-print_raw_param("child_tidptr=0x" TARGET_ABI_FMT_lx, arg3, 0);
-print_raw_param("tls=0x" TARGET_ABI_FMT_lx, arg4, 1);
-#elif defined(TARGET_CRIS)
-print_raw_param("child_stack=0x" TARGET_ABI_FMT_lx, arg0, 0);
-print_flags(clone_flags, arg1, 0);
-print_raw_param("parent_tidptr=0x" TARGET_ABI_FMT_lx, arg2, 0);
-print_raw_param("tls=0x" TARGET_ABI_FMT_lx, arg3, 0);
-print_raw_param("child_tidptr=0x" TARGET_ABI_FMT_lx, arg4, 1);
+#if defined(TARGET_MICROBLAZE)
+do_print_clone(arg1, arg2, arg4, arg6, arg5);
+#elif defined(TARGET_CLONE_BACKWARDS)
+do_print_clone(arg1, arg2, arg3, arg4, arg5);
+#elif defined(TARGET_CLONE_BACKWARDS2)
+do_print_clone(arg2, arg1, arg3, arg5, arg4);
 #else
-print_flags(clone_flags, arg0, 0);
-print_raw_param("child_stack=0x" TARGET_ABI_FMT_lx, arg1, 0);
-print_raw_param("parent_tidptr=0x" TARGET_ABI_FMT_lx, arg2, 0);
-print_raw_param("tls=0x" TARGET_ABI_FMT_lx, arg3, 0);
-print_raw_param("child_tidptr=0x" TARGET_ABI_FMT_lx, arg4, 1);
+do_print_clone(arg1, arg2, arg3, arg5, arg4);
 #endif
 print_syscall_epilogue(name);
 }
-- 
2.5.5

[Qemu-devel] [PATCH v2 2/4] linux-user: add socket() strace

[Laurent Vivier]

Signed-off-by: Laurent Vivier 
---
 linux-user/strace.c| 23 +++
 linux-user/strace.list |  2 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 46391c8..5a9df46 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1221,6 +1221,29 @@ print__llseek(const struct syscallname *name,
 }
 #endif
 
+#if defined(TARGET_NR_socket)
+static void
+print_socket(const struct syscallname *name,
+ abi_long arg0, abi_long arg1, abi_long arg2,
+ abi_long arg3, abi_long arg4, abi_long arg5)
+{
+abi_ulong domain = arg0, type = arg1, protocol = arg2;
+
+print_syscall_prologue(name);
+print_socket_domain(domain);
+gemu_log(",");
+print_socket_type(type);
+gemu_log(",");
+if (domain == AF_PACKET ||
+(domain == AF_INET && type == TARGET_SOCK_PACKET)) {
+protocol = tswap16(protocol);
+}
+print_socket_protocol(domain, type, protocol);
+print_syscall_epilogue(name);
+}
+
+#endif
+
 #if defined(TARGET_NR_socketcall)
 
 #define get_user_ualx(x, gaddr, idx) \
diff --git a/linux-user/strace.list b/linux-user/strace.list
index b379497..7c54dc6 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1291,7 +1291,7 @@
 { TARGET_NR_sigsuspend, "sigsuspend" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_socket
-{ TARGET_NR_socket, "socket" , NULL, NULL, NULL },
+{ TARGET_NR_socket, "socket" , NULL, print_socket, NULL },
 #endif
 #ifdef TARGET_NR_socketcall
 { TARGET_NR_socketcall, "socketcall" , NULL, print_socketcall, NULL },
-- 
2.5.5

Re: [Qemu-devel] [PATCH RFC 00/16] Rework SMP parameters

[Thomas Huth]

On 10.06.2016 19:40, Andrew Jones wrote:
> This series is a first step in eliminating smp_* global
> variables (the last patch gets rid of two of them!) And, it's
> a first step in deprecating '-smp' in favor of using machine
> properties, e.g.
>  qemu -machine pc,sockets=2,cores=2,threads=2,maxcpus=8,cpus=8 ...

Why isn't '-smp' good enough anymore so that it got to be deprecated?

 Thomas

Re: [Qemu-devel] [PATCH RFC 07/16] qom/cpu: make nr-cores, nr-threads real properties

[Thomas Huth]

On 10.06.2016 19:40, Andrew Jones wrote:
> Signed-off-by: Andrew Jones 
> ---
>  qom/cpu.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 751e992de8823..024cda3eb98c8 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -28,6 +28,7 @@
>  #include "exec/log.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/qdev-properties.h"
>  
>  bool cpu_exists(int64_t id)
>  {
> @@ -342,6 +343,12 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu)
>  return cpu->cpu_index;
>  }
>  
> +static Property cpu_common_properties[] = {
> +DEFINE_PROP_INT32("nr-cores", CPUState, nr_cores, 1),
> +DEFINE_PROP_INT32("nr-threads", CPUState, nr_threads, 1),
> +DEFINE_PROP_END_OF_LIST()
> +};

Are you aware of the current CPU hotplug discussion that is going on?
I'm not very involved there, but I think some of these reworks also move
"nr_threads" into the CPU state already, e.g. see:

https://github.com/dgibson/qemu/commit/9d07719784ecbeebea71

... so you might want to check these patches first to see whether you
can base your rework on them?

 Thomas