[Qemu-devel] [PULL v1 09/21] moxie: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The bootloader can just pass EM_MOXIE directly, as that is architecture
specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Anthony Green 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---
Changed since V1
Use EM_MOXIE macro (rth) review

 hw/moxie/moxiesim.c | 3 +--
 target-moxie/cpu.h  | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/moxie/moxiesim.c b/hw/moxie/moxiesim.c
index 537e25e..83b45f1 100644
--- a/hw/moxie/moxiesim.c
+++ b/hw/moxie/moxiesim.c
@@ -53,8 +53,7 @@ static void load_kernel(MoxieCPU *cpu, LoaderParams 
*loader_params)
 ram_addr_t initrd_offset;
 
 kernel_size = load_elf(loader_params->kernel_filename,  NULL, NULL,
-   , _low, _high, 1,
-   ELF_MACHINE, 0);
+   , _low, _high, 1, EM_MOXIE, 0);
 
 if (kernel_size <= 0) {
 fprintf(stderr, "qemu: could not load kernel '%s'\n",
diff --git a/target-moxie/cpu.h b/target-moxie/cpu.h
index 7bb1ed9..e27a6a3 100644
--- a/target-moxie/cpu.h
+++ b/target-moxie/cpu.h
@@ -26,8 +26,6 @@
 
 #define CPUArchState struct CPUMoxieState
 
-#define ELF_MACHINE EM_MOXIE
-
 #define MOXIE_EX_DIV00
 #define MOXIE_EX_BAD 1
 #define MOXIE_EX_IRQ 2
-- 
1.9.1

[Qemu-devel] [PATCH v1 09/15] core: Convert tcg_enabled() uses to any/all variants

[Peter Crosthwaite]

Convert core code usages of tcg_enabled() which don't have a specific
CPU associated with, to either tcg_any_enabled() or tcg_all_enabled().
This is to prepare support for multiple tcg engines, where queries must
query a specific CPU or use global any/all logic.

Signed-off-by: Peter Crosthwaite 
---
Changed since RFCv3:
Tweak commit subject (too long)

 cpus.c  | 2 +-
 exec.c  | 4 ++--
 include/exec/ram_addr.h | 5 +++--
 memory.c| 8 
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/cpus.c b/cpus.c
index 9365e03..da4026e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1165,7 +1165,7 @@ void qemu_mutex_lock_iothread(void)
 /* In the simple case there is no need to bump the VCPU thread out of
  * TCG code execution.
  */
-if (!tcg_enabled() || qemu_in_vcpu_thread() ||
+if (!tcg_any_enabled() || qemu_in_vcpu_thread() ||
 !first_cpu || !first_cpu->thread) {
 qemu_mutex_lock(_global_mutex);
 atomic_dec(_requesting_mutex);
diff --git a/exec.c b/exec.c
index 54cd70a..c120730 100644
--- a/exec.c
+++ b/exec.c
@@ -926,7 +926,7 @@ bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t 
start,
 dirty = bitmap_test_and_clear_atomic(ram_list.dirty_memory[client],
  page, end - page);
 
-if (dirty && tcg_enabled()) {
+if (dirty && tcg_any_enabled()) {
 tlb_reset_dirty_range_all(start, length);
 }
 
@@ -2592,7 +2592,7 @@ void cpu_flush_icache_range(hwaddr start, int len)
  * so there is no need to flush anything. For KVM / Xen we need to flush
  * the host's instruction cache at least.
  */
-if (tcg_enabled()) {
+if (!tcg_all_enabled()) {
 return;
 }
 
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index c113f21..2e8fdd1 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -164,14 +164,15 @@ static inline void 
cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
 
 atomic_or([DIRTY_MEMORY_MIGRATION][page + k], temp);
 atomic_or([DIRTY_MEMORY_VGA][page + k], temp);
-if (tcg_enabled()) {
+if (tcg_any_enabled()) {
 atomic_or([DIRTY_MEMORY_CODE][page + k], temp);
 }
 }
 }
 xen_modified_memory(start, pages << TARGET_PAGE_BITS);
 } else {
-uint8_t clients = tcg_enabled() ? DIRTY_CLIENTS_ALL : 
DIRTY_CLIENTS_NOCODE;
+uint8_t clients = tcg_any_enabled() ? DIRTY_CLIENTS_ALL
+: DIRTY_CLIENTS_NOCODE;
 /*
  * bitmap-traveling is faster than memory-traveling (for addr...)
  * especially when most of the memory is not dirty.
diff --git a/memory.c b/memory.c
index 0d8b2d9..59c65f1 100644
--- a/memory.c
+++ b/memory.c
@@ -1198,7 +1198,7 @@ void memory_region_init_ram(MemoryRegion *mr,
 mr->terminates = true;
 mr->destructor = memory_region_destructor_ram;
 mr->ram_addr = qemu_ram_alloc(size, mr, errp);
-mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
+mr->dirty_log_mask = tcg_any_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
 void memory_region_init_resizeable_ram(MemoryRegion *mr,
@@ -1216,7 +1216,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
 mr->terminates = true;
 mr->destructor = memory_region_destructor_ram;
 mr->ram_addr = qemu_ram_alloc_resizeable(size, max_size, resized, mr, 
errp);
-mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
+mr->dirty_log_mask = tcg_any_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 
 #ifdef __linux__
@@ -1233,7 +1233,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 mr->terminates = true;
 mr->destructor = memory_region_destructor_ram;
 mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp);
-mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
+mr->dirty_log_mask = tcg_any_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 #endif
 
@@ -1247,7 +1247,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
 mr->ram = true;
 mr->terminates = true;
 mr->destructor = memory_region_destructor_ram_from_ptr;
-mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
+mr->dirty_log_mask = tcg_any_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 
 /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL.  */
 assert(ptr != NULL);
-- 
1.9.1

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[Jan Kiszka]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 2015-09-11 08:50, Jan Kiszka wrote:
> Hi Rita,
> 
> [CC'ing Andrew due to overlap with split irqchip topic]
> 
> On 2015-09-09 19:41, Rita Sinha wrote:
>> Hi Jan,
>> 
>>> 
>>> Most likely than not you'll work on the Intel IOMMU and I would
>>> suggest, if you wish to get your feet dirty, just start right
>>> away with the Intel IOMMU (In which case Jan could direct you
>>> better) instead of the AMD one.
>>> 
>> 
>> As suggested by David, I would request you to guide me as to how 
>> to get started with Intel IOMMU.
>> 
> 
> You can find the basic problem description and some links here:
> 
> http://wiki.qemu.org/Google_Summer_of_Code_2015#VT-d_interrupt_emulation_with_KVM_support
>
>
> 
Who much do you know about IOMMUs and interrupt remapping in
> general? The spec may provide a certain introduction to the topic, 
> but it also quickly dives into details. Let me know where you see 
> gaps to build up a better understanding of the topic.
> 
> Implementation-wise, you could start off with the existing 
> interrupt remapping patches and look into the missing error 
> reporting for IR on that basis. I've just rebased and pushed a new 
> version. Error reporting

Oh, another way to approach the topic could be developing unit tests
for the IOMMU. However, I'm not the expert on the related framework
but we'll find one if you like to look into this. You find such tests
in the, well, tests/ subdirectory, for various QEMU aspects in fact.

Jan

> is currently done in form of plain printf. The correct behaviour is
> described in the spec. Feel free to nag me with questions on how 
> things work around interrupt remapping in general and its QEMU 
> integration in particular.
> 
> The goal is to get this upstream eventually. So the current design 
> needs to be prepared for submission, reviewed by subsystem 
> maintainer, and then possibly reworked based on their feedback. We 
> will also need some command line switch (machine property, like 
> iommu=on) to control the availability of IR because the original 
> Q35 chipset didn't include this feature.
> 
> But there is also the aim to integrate it with KVM support, and 
> that's why I'm CC'ing Andrew. At Google, they are currently
> working on QEMU patches for their split KVM irqchip model. This
> will push the IOAPIC into QEMU hands, even when KVM in-kernel APIC 
> acceleration is used. And that will allow IR to be installed 
> easily, without any KVM kernel-side changes.
> 
> Andrew, what is the status of your QEMU patches? Are there chances 
> to preview them in order to asses if and how much integration work 
> is needed for interrupt remapping?
> 
> Rita, you probably noticed that this is no beginner's task, both 
> regarding the hardware background as well as the QEMU subsystems. 
> You have worked with several low-level software components before 
> and have some QEMU experience, so this topic could be a good match 
> for you. Still, if you have doubts after looking into details, let 
> me know so that we can discuss them.
> 
> Best regards, Jan
> 

-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iEYEARECAAYFAlXye3gACgkQitSsb3rl5xRW7wCdEOFPusbslxAjrAYUmka1ta4T
rAsAnioAseNIayFW8C1Faeztb3jRC7PQ
=NtRT
-END PGP SIGNATURE-

Re: [Qemu-devel] [PATCH RFC v5 00/32] qapi: QMP introspection

[Markus Armbruster]

I think I'm almost ready for non-RFC v6, just one more problem: my tests
caught a bug related to QOM and the 'any' type.  Working on it.

See my cry for QOM help "Re: Confused by QOM:
/machine/unattached/device[5]/dr-connector[255]/fdt".

Re: [Qemu-devel] [PATCH] ui/cocoa.m: Add Mount image file menu item

[Markus Armbruster]

Programmingkid  writes:

> On Sep 10, 2015, at 1:15 PM, Markus Armbruster wrote:
>
>> Programmingkid  writes:
>> 
>>> On Sep 10, 2015, at 3:21 AM, Markus Armbruster wrote:
>>> 
 Programmingkid  writes:
 
> Does this look about right?
> 
>   QDict *qdict;
>   Error *errp;
>   QObject **ret_data;
>   static int counter;
>   char *idString, *fileName;
> 
>   // The file variable is objective-c, left that code out
> 
>   fileName = g_strdup_printf("%s",
>   [file cStringUsingEncoding: 
> NSASCIIStringEncoding]);
> 
>   /* Create an unique id */
>   idString = g_strdup_printf("USB%d", counter++);
> 
>   /* Create the QDICT object */
>   qdict = qdict_new();
>   qdict_put_obj(qdict, "id", qstring_from_str(idString));
>   qdict_put_obj(qdict, "device", qstring_from_str(idString));
>   qdict_put_obj(qdict, "if", qstring_from_str("none"));
>   qdict_put_obj(qdict, "file", qstring_from_str(fileName));
>   qdict_put_obj(qdict, "driver", qstring_from_str("usb-storage"));
>   drive_add(IF_DEFAULT, 0, fileName, "none");
>   qmp_device_add(qdict, ret_data, );
>   handleAnyDeviceErrors(errp);
>   g_free(fileName);
>   g_free(idString);
> 
> This is a sample of what I am working on. For some reason, it crashes 
> QEMU. Any clues why? I think it might be because of qdict_put_obj(). 
 
 My crystal ball is down for maintenance today, so you'll have to gives
 us the clues yourself: a stack backtrace, for starters :)
>>> 
>>> Here is the error:
>>> 
>>> 2015-09-10 12:21:12.355 qemu-system-ppc[17603:903] HIToolbox: ignoring
>>> exception 'Uncaught system exception: signal 11' that raised inside
>>> Carbon event dispatch
>>> (
>>> 0 CoreFoundation 0x7fff83ad37b4 __exceptionPreprocess +
>>> 180
>>> 1 libobjc.A.dylib 0x7fff83567f03 objc_exception_throw + 45
>>> 2 CoreFoundation 0x7fff83b2b969 -[NSException raise] + 9
>>> 3 ExceptionHandling 0x7fff845082d3
>>> NSExceptionHandlerUncaughtSignalHandler + 37
>>> 4 libSystem.B.dylib 0x7fff825431ba _sigtramp + 26
>>> 5 ???  0x7fff5fc12dc0 0x0 + 140734799883712
>>> 6 qemu-system-ppc 0x0001003c4109 qdict_get_try_str + 58
>>> 7 qemu-system-ppc 0x0001003dba04 qemu_opts_from_qdict + 63
>>> 8 qemu-system-ppc 0x000100169388 qmp_device_add + 78
>> 
>> Crashes in qdict_get_try_str().  Use a debugger to find out what goes
>> wrong there.
>
> This is what it said:
>
> Program received signal EXC_BAD_ACCESS, Could not access memory.
> Reason: KERN_INVALID_ADDRESS at address: 0x01a7e130
> 0x0001003c39e9 in qobject_type (obj=0x1a7e130) at qobject.h:109
> 109   assert(obj->type != NULL);
> (gdb) bt
> #0  0x0001003c39e9 in qobject_type (obj=0x1a7e130) at qobject.h:109
> #1 0x0001003c4145 in qdict_get_try_str (qdict=0x102890a00,
> key=0x1003e8308 "id") at qobject/qdict.c:341
> #2 0x0001003dba44 in qemu_opts_from_qdict (list=0x1005a2f40,
> qdict=0x102890a00, errp=0x7fff5fbfcfe0) at util/qemu-option.c:968
> #3 0x0001001693b4 in qmp_device_add (qdict=0x102890a00,
> ret_data=0x7fff5fbfd038, errp=0x7fff5fbfd030) at qdev-monitor.c:767
>
> I do not know much about the QDict type. Did I use it right by using
> qstring_from_str() to set a key's value to another string?

I can't see your bug.

I'm happy to explain how things work and provide advice, I review
working patches touching my areas of expertise, but I can't do your
debugging for you, sorry.

For example code using QDict, check out tests/check-qdict.c.

[Qemu-devel] [PATCH v1 00/15] Multi-Arch Phase 1

[Peter Crosthwaite]

This is the first set of patches needed to enable Multi-arch system
emulation. For full context refer to RFCv3:

[PATCH v3 00/35] Multi Architecture System Emulation
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg03929.html

This is the first patch-pack intended for merge.

Original cover, as well as overall series state below for further
information.

Regards,
Peter

Original Multi-arch arch patch series cover:

***

This is target-multi, a system-mode build that can support multiple
cpu-types.

Two architectures are initially converted. Microblaze and ARM. Step
by step conversion in done for each. A microblaze is added to
Xilinx Zynq platform as a test case. This will be elaborted more in
future spins. This use case is valid, as Microblazes can be added (any
number of them!) in Zynq FPGA programmable logic configuration.

The general approach (radically different to approach in V1 RFC) is to build
and prelink an object (arch-obj.o) per-arch containing:

1: target-foo/*
2: All uses of env internals and CPU_GET_ENV
* cputlb, translate-all, cpu-exec
* TCG backend

This means cputlb and friends are compiled multiple times fo each arch. The
symbols for each of these pre-links are then localised to avoid link time name
collisions. This is based on Paolo's suggestion to templatify cputlb and
friends. Just the net of what to multi-compile is widened to include the TCG
stuff as well now.

Despite being some "major surgery" this approach actually solves many of big
the problems raised in V1. Big problems sovled:

1: With the multi-compile TCG backends there are now multiple tcg_ctx's for
each architecture. This solves the issue PMM raised WRT false positives on TB
hashing as archs no longer share translation context.

2: There is no longer a need to reorder the CPU_COMMON within the ENV or the ENV
within the CPU. This was flagged as a performance issue by multiple people in
V1.
All users of the env internals as well as ENV_GET_CPU are now in multi-compile
code and so multi-arch does not need to define a generic ENV nor does in need to
def the problematic ENV_GET_CPU.

3: With the prelink symbol localisation, link time namespace collision of
helpers from multiple arches is no longer an issue. No need to bloat all the
function names with arch specific prefixes.

4: The architecture specifics used/defined by cpu-defs can now vary from arch to
arch (incl. target_ulong) greatly reducing coversion effort needed. The list
of restrictions for multi-arch capability is much reduced since V1. No
target_long issues anymore.

include/exec/*.h and some of the common code needs some refactoring to setup
this single vs multi compile split. Mostly code movements.

Some functions (like tcg_enabled) need to be listified for each of the
now-multiple TCG engines.

The interface between the multi compile and single compiled files needs to be
virtualised using QOM cpu functions. But this is now a very low footprint
change as most of the virtualised hooks are now in mutli-compiled code (they
only exist as text once). There are more new hooks than before, but the per
target change pattern is reduced.

For the implementation of the series, the trickiest part is (still) cpu.h
inclusion management. There are now more than one cpu.h's and different
parts of the tree need a different include scheme. target-multi defines
it's own cpu.h which is bare minimum defs as needed by core code only.
target-foo/cpu.h are mostly the same but refactored to avoid collisions
with other cpu.h's. Inclusion scheme goes something like
this (for the multi-arch build):

*: Core code includes only target-multi/cpu.h
*: target-foo/ implementation code includes target-foo/cpu.h locally
*: System level code (e.g. mach models) can use multiple target-foo/cpu.h's

The hardest unasnwered Q is (still) what to do about bootloading. Currently
each arch has it's own architecture specific bootloading which may assume a
single architecture. I have applied some hacks to at least get this
RFC testable using a -kernel -firmware split but going forward being
able to associate an elf/image with a cpu explictitly needs to be
solved.

No support for KVM, im not sure if a mix of TCG and KVM is supported even for
a single arch? (which would be prerequisite to MA KVM).

***

Current review state of full multi-arch work in progress branch:

cpu-exec: Migrate some generic fns to cpu-exec-common
translate: Listify tcg_exec_init()  R:rth
translate-all: Move tcg_handle_interrupt() to -common   R:rth
tcg: split tcg_op_defs to -common
tcg: Move tcg_tb_ptr to -common
translate: move real_host_page setting to -common
cpus: Listify cpu_list() function
translate-common: Listify tcg_enabled()
core: Convert tcg_enabled() uses to any/all variants
exec-all: Move cpu_can_do_io() to qom/cpu.h R:rth
cputlb: move CPU_LOOP() for tlb_reset() to exec.c
cputlb: Change tlb_set_dirty() arg to cpu
include/exec: Move cputlb exec.c defs out 

[Qemu-devel] [PULL v1 14/21] xtensa: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The bootloaders can just pass EM_XTENSA directly, as that
is architecture specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Max Filippov 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 hw/xtensa/sim.c | 4 ++--
 hw/xtensa/xtfpga.c  | 2 +-
 target-xtensa/cpu.h | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
index 328d209..8bd0c22 100644
--- a/hw/xtensa/sim.c
+++ b/hw/xtensa/sim.c
@@ -93,10 +93,10 @@ static void xtensa_sim_init(MachineState *machine)
 uint64_t elf_lowaddr;
 #ifdef TARGET_WORDS_BIGENDIAN
 int success = load_elf(kernel_filename, translate_phys_addr, cpu,
-_entry, _lowaddr, NULL, 1, ELF_MACHINE, 0);
+_entry, _lowaddr, NULL, 1, EM_XTENSA, 0);
 #else
 int success = load_elf(kernel_filename, translate_phys_addr, cpu,
-_entry, _lowaddr, NULL, 0, ELF_MACHINE, 0);
+_entry, _lowaddr, NULL, 0, EM_XTENSA, 0);
 #endif
 if (success > 0) {
 env->pc = elf_entry;
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index ab4d0e4..ba67035 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -340,7 +340,7 @@ static void lx_init(const LxBoardDesc *board, MachineState 
*machine)
 uint64_t elf_entry;
 uint64_t elf_lowaddr;
 int success = load_elf(kernel_filename, translate_phys_addr, cpu,
-_entry, _lowaddr, NULL, be, ELF_MACHINE, 0);
+_entry, _lowaddr, NULL, be, EM_XTENSA, 0);
 if (success > 0) {
 entry_point = elf_entry;
 } else {
diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 96bfc82..4ec005e 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -30,7 +30,6 @@
 
 #define ALIGNED_ONLY
 #define TARGET_LONG_BITS 32
-#define ELF_MACHINE EM_XTENSA
 
 #define CPUArchState struct CPUXtensaState
 
-- 
1.9.1

[Qemu-devel] [PULL v1 02/21] linux-user: elfload: Provide default for elf_check_arch

[Peter Crosthwaite]

For many arch's this macro is defined as the predicatable behaviour
of checking the argument for eqaulity against ELF_ARCH. Provide a
default define as such, so only archs with special handling (usually
allowing multiple EM values) need to provide a def.

Arches that do any of:

1: provide this def exactly the same way as the new default
(alpha, x86_64)
2: check against ELF_MACHINE while defining ELF_ARCH == ELF_MACHINE
(arm, aarch64)
3: check against EM_FOO directly while defining ELF_ARCH == EM_FOO
(unicore32, sparc32, ppc32, mips, openrisc, sh4, cris, m86k)

have their elf_check_arch removed as the default will provide the
correct behaviour.

Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 linux-user/elfload.c | 28 
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 518ff05..1f4c054 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -145,7 +145,6 @@ static uint32_t get_elf_hwcap(void)
 
 #ifdef TARGET_X86_64
 #define ELF_START_MMAP 0x2ab000ULL
-#define elf_check_arch(x) ( ((x) == ELF_ARCH) )
 
 #define ELF_CLASS  ELFCLASS64
 #define ELF_ARCH   EM_X86_64
@@ -273,8 +272,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, 
const CPUX86State *en
 
 #define ELF_START_MMAP 0x8000
 
-#define elf_check_arch(x) ((x) == ELF_MACHINE)
-
 #define ELF_ARCHELF_MACHINE
 #define ELF_CLASS   ELFCLASS32
 
@@ -481,8 +478,6 @@ static uint32_t get_elf_hwcap2(void)
 /* 64 bit ARM definitions */
 #define ELF_START_MMAP 0x8000
 
-#define elf_check_arch(x) ((x) == ELF_MACHINE)
-
 #define ELF_ARCHELF_MACHINE
 #define ELF_CLASS   ELFCLASS64
 #define ELF_PLATFORM"aarch64"
@@ -556,8 +551,6 @@ static uint32_t get_elf_hwcap(void)
 
 #define ELF_START_MMAP  0x8000
 
-#define elf_check_arch(x)   ((x) == EM_UNICORE32)
-
 #define ELF_CLASS   ELFCLASS32
 #define ELF_DATAELFDATA2LSB
 #define ELF_ARCHEM_UNICORE32
@@ -666,7 +659,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 #define ELF_START_MMAP 0x8000
 #define ELF_HWCAP  (HWCAP_SPARC_FLUSH | HWCAP_SPARC_STBAR | HWCAP_SPARC_SWAP \
 | HWCAP_SPARC_MULDIV)
-#define elf_check_arch(x) ( (x) == EM_SPARC )
 
 #define ELF_CLASS   ELFCLASS32
 #define ELF_ARCHEM_SPARC
@@ -696,8 +688,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 
 #else
 
-#define elf_check_arch(x) ( (x) == EM_PPC )
-
 #define ELF_CLASS   ELFCLASS32
 
 #endif
@@ -875,8 +865,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, 
const CPUPPCState *en
 
 #define ELF_START_MMAP 0x8000
 
-#define elf_check_arch(x) ( (x) == EM_MIPS )
-
 #ifdef TARGET_MIPS64
 #define ELF_CLASS   ELFCLASS64
 #else
@@ -985,8 +973,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, 
const CPUMBState *env
 
 #define ELF_START_MMAP 0x0800
 
-#define elf_check_arch(x) ((x) == EM_OPENRISC)
-
 #define ELF_ARCH EM_OPENRISC
 #define ELF_CLASS ELFCLASS32
 #define ELF_DATA  ELFDATA2MSB
@@ -1026,8 +1012,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 
 #define ELF_START_MMAP 0x8000
 
-#define elf_check_arch(x) ( (x) == EM_SH )
-
 #define ELF_CLASS ELFCLASS32
 #define ELF_ARCH  EM_SH
 
@@ -1110,8 +1094,6 @@ static uint32_t get_elf_hwcap(void)
 
 #define ELF_START_MMAP 0x8000
 
-#define elf_check_arch(x) ( (x) == EM_CRIS )
-
 #define ELF_CLASS ELFCLASS32
 #define ELF_ARCH  EM_CRIS
 
@@ -1129,8 +,6 @@ static inline void init_thread(struct target_pt_regs 
*regs,
 
 #define ELF_START_MMAP 0x8000
 
-#define elf_check_arch(x) ( (x) == EM_68K )
-
 #define ELF_CLASS   ELFCLASS32
 #define ELF_ARCHEM_68K
 
@@ -1182,8 +1162,6 @@ static void elf_core_copy_regs(target_elf_gregset_t 
*regs, const CPUM68KState *e
 
 #define ELF_START_MMAP (0x300ULL)
 
-#define elf_check_arch(x) ( (x) == ELF_ARCH )
-
 #define ELF_CLASS  ELFCLASS64
 #define ELF_ARCH   EM_ALPHA
 
@@ -1203,8 +1181,6 @@ static inline void init_thread(struct target_pt_regs 
*regs,
 
 #define ELF_START_MMAP (0x200ULL)
 
-#define elf_check_arch(x) ( (x) == ELF_ARCH )
-
 #define ELF_CLASS  ELFCLASS64
 #define ELF_DATA   ELFDATA2MSB
 #define ELF_ARCH   EM_S390
@@ -1226,6 +1202,10 @@ static inline void init_thread(struct target_pt_regs 
*regs, struct image_info *i
 #define ELF_MACHINE ELF_ARCH
 #endif
 
+#ifndef elf_check_arch
+#define elf_check_arch(x) ((x) == ELF_ARCH)
+#endif
+
 #ifndef ELF_HWCAP
 #define ELF_HWCAP 0
 #endif
-- 
1.9.1

Re: [Qemu-devel] [PATCH RFC v5 30/32] qapi: New QMP command query-qmp-schema for QMP introspection

[Markus Armbruster]

Michael Roth  writes:

> Quoting Markus Armbruster (2015-09-07 05:16:41)
>> qapi/introspect.json defines the introspection schema.  It's designed
>> for QMP introspection, but should do for similar uses, such as QGA.
>> 
>> The introspection schema does not reflect all the rules and
>> restrictions that apply to QAPI schemata.  A valid QAPI schema has an
>> introspection value conforming to the introspection schema, but the
>> converse is not true.
>> 
>> Introspection lowers away a number of schema details, and makes
>> implicit things explicit:
>> 
>> * The built-in types are declared with their JSON type.
>> 
>>   TODO Should we map all the integer types to just int?
>> 
>> * Implicit type definitions are made explicit, and given
>>   auto-generated names:
>> 
>>   - Array types, named by appending "List" to the name of their
>> element type, like in generated C.
>> 
>>   - The enumeration types implicitly defined by simple union types,
>> named by appending "Kind" to the name of their simple union type,
>> like in generated C.
>> 
>>   - Types that don't occur in generated C.  Their names start with ':'
>> so they don't clash with the user's names.
>> 
>> * All type references are by name.
>> 
>> * The struct and union types are generalized into an object type.
>> 
>> * Base types are flattened.
>> 
>> * Commands take a single argument and return a single result.
>> 
>>   Dictionary argument or list result is an implicit type definition.
>> 
>>   The empty object type is used when a command takes no arguments or
>>   produces no results.
>> 
>>   The argument is always of object type, but the introspection schema
>>   doesn't reflect that.
>> 
>>   The 'gen': false directive is omitted as implementation detail.
>> 
>>   The 'success-response' directive is omitted as well for now, even
>>   though it's not an implementation detail, because it's not used by
>>   QMP.
>> 
>> * Events carry a single data value.
>> 
>>   Implicit type definition and empty object type use, just like for
>>   commands.
>> 
>>   The value is of object type, but the introspection schema doesn't
>>   reflect that.
>> 
>> * Types not used by commands or events are omitted.
>> 
>>   Indirect use counts as use.
>> 
>> * Optional members have a default, which can only be null right now
>> 
>>   Instead of a mandatory "optional" flag, we have an optional default.
>>   No default means mandatory, default null means optional without
>>   default value.  Non-null is available for optional with default
>>   (possible future extension).
>> 
>> * Clients should *not* look up types by name, because type names are
>>   not ABI.  Look up the command or event you're interested in, then
>>   follow the references.
>> 
>>   TODO Should we hide the type names to eliminate the temptation?
>> 
>> New generator scripts/qapi-introspect.py computes an introspection
>> value for its input, and generates a C variable holding it.
>> 
>> It can generate awfully long lines.  Marked TODO.
>
> Would be good if we could get this is for initial merge to help
> with debugging if anything pops up.

Unlikely, to be honest.  Patches depending on this series have been
piling up.  I'd be okay with delaying follow-up QAPI work some more, but
not work on other subsystems, especially not user-visible goodies.

Work-arounds:

* Take the C string from qmp-introspect.c, unquote it to get actual
  string contents, feed it to your favourite JSON pretty printer.  I've
  used Python's pprint.

* Run query-qmp-schema, feed the result to your favourite JSON pretty
  printer.

* Likewise, using the QMP monitor's integrated pretty printer, e.g.
  with --qmp-pretty.

>> A new test-qmp-input-visitor test case feeds its result for both
>> tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
>> QmpInputVisitor to verify it actually conforms to the schema.
>> 
>> New QMP command query-qmp-schema takes its return value from that
>> variable.  Its reply is some 85KiBytes for me right now.
>> 
>> If this turns out to be too much, we have a couple of options:
>> 
>> * We can use shorter names in the JSON.  Not the QMP style.
>> 
>> * Optionally return the sub-schema for commands and events given as
>>   arguments.
>> 
>>   Right now qmp_query_schema() sends the string literal computed by
>>   qmp-introspect.py.  To compute sub-schema at run time, we'd have to
>>   duplicate parts of qapi-introspect.py in C.  Unattractive.
>> 
>> * Let clients cache the output of query-qmp-schema.
>> 
>>   It changes only on QEMU upgrades, i.e. rarely.  Provide a command
>>   query-qmp-schema-hash.  Clients can have a cache indexed by hash,
>>   and re-query the schema only when they don't have it cached.  Even
>>   simpler: put the hash in the QMP greeting.
>
> Would probably be easier for management to build up their own structure
> for querying caps, so I think a computed hash seems best. But I don't
> think either is something that couldn't be 

Re: [Qemu-devel] [Qemu-ppc] [PATCH RFC 0/4] Mac OS 9 compatibility improvements

[Stewart Smith]

Mark Cave-Ayland  writes:
> I can confirm here that in combination with an updated OpenBIOS then the
> patchset gets qemu-system-ppc -M mac99 to the OS 9 loading screen, which
> is great progress!
>
> When booting OS 9 with this patchset I do see the following output on
> the console:
>
> Trying to write invalid spr 0 (0x000) at 00f113c0
> Trying to read invalid spr 0 (0x000) at 00f113c8
> Trying to write privileged spr 955 (0x3bb) at 00f164b8
> Trying to write invalid spr 959 (0x3bf) at 00f16520
> Trying to read invalid spr 959 (0x3bf) at 00f16528
> Trying to write privileged spr 955 (0x3bb) at 00f164b8
> Trying to write invalid spr 959 (0x3bf) at 00f16520
> Trying to read invalid spr 959 (0x3bf) at 00f16528

>From http://www.freescale.com/files/32bit/doc/data_sheet/MPC604.pdf it
looks like the 955 and 959 are SPRs for the Performance Monitor,
specifically SDA and SIA, Sampled Data/Instructian Address.

Some more info:
http://ieeexplore.ieee.org/stamp/stamp.jsp?tp==528812
and probably around some processor manuals.

[Qemu-devel] [PATCH v1 14/15] cpu-common: Define tb_page_addr_t for everyone

[Peter Crosthwaite]

In system mode emulation (at least) this definition has no architecture
specific dependencies. Move it to common code such that common code can
use it (primarily for defining function prototypes).

Signed-off-by: Peter Crosthwaite 
---
So this is the same as in RFCv2 and the comment there was that it will not
work for linux-user mode WRT to qom/cpu.h fn prototypes needed this def.
But the solution there is instead to conditionalise the def of those hooks on
NEED_CPU_H. Then this patch is just needed for some misc. system-mode-only
core code usages.

 include/exec/cpu-common.h | 4 
 include/exec/exec-all.h   | 2 --
 include/qom/cpu.h | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 9fb1d54..47d416d 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -53,6 +53,10 @@ typedef uintptr_t ram_addr_t;
 #  define RAM_ADDR_FMT "%" PRIxPTR
 #endif
 
+#ifndef CONFIG_USER_ONLY
+typedef ram_addr_t tb_page_addr_t;
+#endif
+
 extern ram_addr_t ram_size;
 ram_addr_t get_current_ram_size(void);
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 3dd9f31..63d0baf 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -30,8 +30,6 @@
type.  */
 #if defined(CONFIG_USER_ONLY)
 typedef abi_ulong tb_page_addr_t;
-#else
-typedef ram_addr_t tb_page_addr_t;
 #endif
 
 /* is_jmp field values */
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7e5f3b0..bcd4a3b 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -24,6 +24,7 @@
 #include 
 #include "hw/qdev-core.h"
 #include "disas/bfd.h"
+#include "exec/cpu-common.h"
 #include "exec/hwaddr.h"
 #include "exec/memattrs.h"
 #include "qemu/queue.h"
-- 
1.9.1

[Qemu-devel] [PULL v1 10/21] unicore: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The only generic code relying on this is linux-user, but linux users'
default behaviour of defaulting ELF_MACHINE to ELF_ARCH will handle
this.

This removes another architecture specific definition from the global
namespace.

Cc: Guan Xuetao 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 target-unicore32/cpu.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-unicore32/cpu.h b/target-unicore32/cpu.h
index 45e31e5..3c06996 100644
--- a/target-unicore32/cpu.h
+++ b/target-unicore32/cpu.h
@@ -17,8 +17,6 @@
 #define TARGET_PHYS_ADDR_SPACE_BITS 32
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 
-#define ELF_MACHINE EM_UNICORE32
-
 #define CPUArchStatestruct CPUUniCore32State
 
 #include "config.h"
-- 
1.9.1

Re: [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing

[Markus Armbruster]

Quick initial high-level feedback, since I'm afraid real review will
take a while (series is long, and I'm still swamped).

Peter Crosthwaite  writes:

> Hi Markus and all,
>
> This patch series adds support for automatically concatenating multiple
> errors to the one Error *.
>
> I'll start with what I am actually trying to do, which is get rid of the
> boilerplate:
>
> some_api_call(... , _err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> some_similar_api_call(... , _err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> some_similar_api_call(... , _err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> ...
>
> It is usually 1 LOC for the API and 4 LOC for the error handling boiler
> plate making our code hard reading. This is particularly bad in hw/arm
> where we have a good number of fully QOMified SoCs and machine models,
> each of which need these checks on recursive realize calls and friends.

The problem is real.  A way to avoid or even reduce the boiler plate
would be nice, provided it's reasonably hard to misuse, and doesn't mess
with the current usage patterns of the Error interface.

> The removed LOC just in ARM pays for the extra core code needed to
> implement this. And the number of ARM machines is only going to grow.
>
> So the plan is:
>
> 1: Allow an Error * to contain more that one actual error from API
> calls.

Can you explain *why* you want to support multiple errors?

> 2: Refactor key APIs (some_similar_api_call() in the above example)
> to not fatal when previous errors have occured in dependencies.

I'm afraid this is going to be non-trivial :)

For every place that now continues after errors, you need to show that
continuing is safe.

> Point 1 kind of got big on me. Patch 4 is the main event, listifying
> errors. The follow up issue however, is it tricky to get a sane
> definition of error_get_pretty for a multi-error. So instead the
> strategy is to remove error_get_pretty() and replace with some error
> API helper with well defined behaviour for multi-error. The two leading
> uses of error get pretty are prefixing an error, and reporting an error
> via a custom printf handler. So two new APIs are added for that (P5-6).
> There aren't many error_get_pretty's left after that, and they
> shouldn't be in the path of any multi-errors.
>
> I think the error_prefix is valuable it its own right, as it now means
> the code for report or propagating a prefixed error is now consistent
> with the non-prefixed variants.
>
> That is, we used to have:
>
> /* If we are prefixing we have to do it this way: */
> error_setg(errp, "some prefix %s", error_get_pretty(local_err));
> error_free(local_err);
>
> vs:
>
> /* but if not prefixing it is like this: */
> error_propagate(errp, local_err);
>
> Now with this patch series the two are much more recognisable as the same
> with:
>
> /* This code is almost the same as the above non-prefixed propagation */
> error_prefix(local_err, "some prefix"):
> error_propagate(errp, local_err);

Less flexible, but that might be a good thing.  Need to see the actual
uses to tell.

> Point 2 is less about error API and more about machine generation.
> Sysbus, Memory and Qdev all have APIs that depend on successful device-
> init and realize calls for argument devices. As we are trying to remove
> the error detection for those argument devs, those APIs need to tweaked
> to handle realize failure. This actually wasn't as bad as I thought it
> would be. See patches (7-9).
>
> All patches after that walk the various major subsystems converting
> error APIs to this new scheme and deleting now-unneeded boiler plate.
> ARM is first (P10-15) seeing good clean up of propagate handers.
>
> So the net result for these ARM machines, is error behaviour that is
> something like a compiler. If any one thing fails, then machine-init
> (compilation) fails. But an early fail does not stop machine-init
> (compilation), instead it proceeds to the end collecting subsequent
> errors as it goes.

Simple compilers stop on first error.  Not as bad as it may sound when
your machine gets from running "make" to compiler dying on the first
error real fast.

More ambitious compilers continue to diagnose more errors.  This isn't
trivial.  The compiler has to satisfy post conditions even after an
error, typically by synthesizing suitable error values.  It has to take
pains to avoid error cascades.  Experienced users recognize when that
effort fails, and typically ignore the remaining errors wholesale then.

In QEMU, error cascades might be less of a problem than with compilers.
To tell for sure, we'd have to try.

However, satisfying post conditions is at least as much of a problem.
More so since they're generally unstated.  Can you explain your strategy
for solving this one?

> Some other interesting food for thought is the qemu_fdt APIs which I
> have been wanting to 

[Qemu-devel] [PULL v1 13/21] tricore: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The bootloader can just pass EM_TRICORE directly, as that
is architecture specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Bastian Koppelmann 
Acked-By: Bastian Koppelmann 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 hw/tricore/tricore_testboard.c | 2 +-
 target-tricore/cpu.h   | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c
index a059a20..62b406b 100644
--- a/hw/tricore/tricore_testboard.c
+++ b/hw/tricore/tricore_testboard.c
@@ -44,7 +44,7 @@ static void tricore_load_kernel(CPUTriCoreState *env)
 kernel_size = load_elf(tricoretb_binfo.kernel_filename, NULL,
NULL, (uint64_t *), NULL,
NULL, 0,
-   ELF_MACHINE, 1);
+   EM_TRICORE, 1);
 if (kernel_size <= 0) {
 error_report("qemu: no kernel file '%s'",
 tricoretb_binfo.kernel_filename);
diff --git a/target-tricore/cpu.h b/target-tricore/cpu.h
index 916ee27..a72a7d8 100644
--- a/target-tricore/cpu.h
+++ b/target-tricore/cpu.h
@@ -25,8 +25,6 @@
 #include "exec/cpu-defs.h"
 #include "fpu/softfloat.h"
 
-#define ELF_MACHINE EM_TRICORE
-
 #define CPUArchState struct CPUTriCoreState
 
 struct CPUTriCoreState;
-- 
1.9.1

[Qemu-devel] [PULL v1 12/21] or32: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The only generic code relying on this is linux-user, but linux users'
default behaviour of defaulting ELF_MACHINE to ELF_ARCH will handle
this.

The bootloader can just pass EM_OPENRISC directly, as that is
architecture specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Jia Liu 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 hw/openrisc/openrisc_sim.c | 2 +-
 target-openrisc/cpu.h  | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 1da0657..fb6d8ea 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -68,7 +68,7 @@ static void cpu_openrisc_load_kernel(ram_addr_t ram_size,
 
 if (kernel_filename && !qtest_enabled()) {
 kernel_size = load_elf(kernel_filename, NULL, NULL,
-   _entry, NULL, NULL, 1, ELF_MACHINE, 1);
+   _entry, NULL, NULL, 1, EM_OPENRISC, 1);
 entry = elf_entry;
 if (kernel_size < 0) {
 kernel_size = load_uimage(kernel_filename,
diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index 36c4f20..ac8b969 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -21,7 +21,6 @@
 #define CPU_OPENRISC_H
 
 #define TARGET_LONG_BITS 32
-#define ELF_MACHINEEM_OPENRISC
 
 #define CPUArchState struct CPUOpenRISCState
 
-- 
1.9.1

[Qemu-devel] [PULL v1 00/21] Multi-arch queue

[Peter Crosthwaite]

Flush of multi-arch pre-requisite work.


multi-arch queue:
* add EM_MOXIE
* Cleanup ELF_MACHINE and remove from cpu.h


The following changes since commit 9d34158a5af734e8de0b42b0a7228200c426a8d0:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20150907' into 
staging (2015-09-07 16:07:47 +0100)

are available in the git repository at:


  git://github.com/pcrost/qemu multi-arch.1.2015.09.07

for you to fetch changes up to f4290e0cf89fa512df44e607f239d46e73d46576:

  ppc: Rename ELF_MACHINE to be PPC specific (2015-09-07 23:59:28 -0700)


Peter Crosthwaite (21):
  linux_user: elfload: Default ELF_MACHINE to ELF_ARCH
  linux-user: elfload: Provide default for elf_check_arch
  elf_ops: Fix coding style for EM alias case statement
  elf: Update EM_MOXIE definition
  arm: Remove ELF_MACHINE from cpu.h
  mb: Remove ELF_MACHINE from cpu.h
  m68k: Remove ELF_MACHINE from cpu.h
  cris: Remove ELF_MACHINE from cpu.h
  moxie: Remove ELF_MACHINE from cpu.h
  unicore: Remove ELF_MACHINE from cpu.h
  lm32: Remove ELF_MACHINE from cpu.h
  or32: Remove ELF_MACHINE from cpu.h
  tricore: Remove ELF_MACHINE from cpu.h
  xtensa: Remove ELF_MACHINE from cpu.h
  sh4: Remove ELF_MACHINE from cpu.h
  s390: Remove ELF_MACHINE from cpu.h
  sparc: Remove ELF_MACHINE from cpu.h
  mips: Remove ELF_MACHINE from cpu.h
  alpha: Remove ELF_MACHINE from cpu.h
  i386: Rename ELF_MACHINE to be x86 specific
  ppc: Rename ELF_MACHINE to be PPC specific

 hw/arm/armv7m.c|  2 +-
 hw/cris/boot.c |  2 +-
 hw/i386/multiboot.c|  2 +-
 hw/lm32/lm32_boards.c  |  4 ++--
 hw/lm32/milkymist.c|  2 +-
 hw/m68k/an5206.c   |  2 +-
 hw/m68k/dummy_m68k.c   |  2 +-
 hw/m68k/mcf5208.c  |  2 +-
 hw/microblaze/boot.c   |  4 ++--
 hw/mips/mips_fulong2e.c|  2 +-
 hw/mips/mips_malta.c   |  2 +-
 hw/mips/mips_mipssim.c |  2 +-
 hw/mips/mips_r4k.c |  2 +-
 hw/moxie/moxiesim.c|  4 ++--
 hw/openrisc/openrisc_sim.c |  2 +-
 hw/ppc/e500.c  |  2 +-
 hw/ppc/mac_newworld.c  |  4 ++--
 hw/ppc/mac_oldworld.c  |  4 ++--
 hw/ppc/ppc440_bamboo.c |  2 +-
 hw/ppc/prep.c  |  2 +-
 hw/ppc/spapr.c |  4 ++--
 hw/ppc/virtex_ml507.c  |  2 +-
 hw/s390x/ipl.c |  4 ++--
 hw/sparc/leon3.c   |  2 +-
 hw/sparc/sun4m.c   |  4 ++--
 hw/sparc64/sun4u.c |  4 ++--
 hw/tricore/tricore_testboard.c |  2 +-
 hw/xtensa/sim.c|  4 ++--
 hw/xtensa/xtfpga.c |  2 +-
 include/elf.h  |  3 +++
 include/hw/elf_ops.h   | 23 +--
 linux-user/elfload.c   | 37 +++--
 target-alpha/cpu.h |  2 --
 target-arm/cpu.h   |  2 --
 target-cris/cpu.h  |  2 --
 target-i386/cpu.h  |  4 ++--
 target-lm32/cpu.h  |  2 --
 target-m68k/cpu.h  |  2 --
 target-microblaze/cpu.h|  2 --
 target-mips/cpu.h  |  2 --
 target-moxie/cpu.h |  2 --
 target-openrisc/cpu.h  |  1 -
 target-ppc/cpu.h   |  4 ++--
 target-s390x/cpu.h |  1 -
 target-sh4/cpu.h   |  2 --
 target-sparc/cpu.h |  6 --
 target-tricore/cpu.h   |  2 --
 target-unicore32/cpu.h |  2 --
 target-xtensa/cpu.h|  1 -
 49 files changed, 74 insertions(+), 106 deletions(-)

[Qemu-devel] [PULL v1 18/21] mips: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The only generic code relying on this is linux-user, but linux users'
default behaviour of defaulting ELF_MACHINE to ELF_ARCH will handle
this.

The bootloaders can just pass EM_MIPS directly, as that is
architecture specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Aurelien Jarno 
Cc: Leon Alrae 
Reviewed-by: Aurelien Jarno 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 hw/mips/mips_fulong2e.c | 2 +-
 hw/mips/mips_malta.c| 2 +-
 hw/mips/mips_mipssim.c  | 2 +-
 hw/mips/mips_r4k.c  | 2 +-
 target-mips/cpu.h   | 2 --
 5 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index dea941a..a6c746a 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -116,7 +116,7 @@ static int64_t load_kernel (CPUMIPSState *env)
 
 if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
  (uint64_t *)_entry, (uint64_t *)_low,
- (uint64_t *)_high, 0, ELF_MACHINE, 1) < 0) {
+ (uint64_t *)_high, 0, EM_MIPS, 1) < 0) {
 fprintf(stderr, "qemu: could not load kernel '%s'\n",
 loaderparams.kernel_filename);
 exit(1);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 3082e75..bf477b6 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -795,7 +795,7 @@ static int64_t load_kernel (void)
 
 if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
  (uint64_t *)_entry, NULL, (uint64_t *)_high,
- big_endian, ELF_MACHINE, 1) < 0) {
+ big_endian, EM_MIPS, 1) < 0) {
 fprintf(stderr, "qemu: could not load kernel '%s'\n",
 loaderparams.kernel_filename);
 exit(1);
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 61f74a6..b08667d 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -69,7 +69,7 @@ static int64_t load_kernel(void)
 kernel_size = load_elf(loaderparams.kernel_filename, 
cpu_mips_kseg0_to_phys,
NULL, (uint64_t *), NULL,
(uint64_t *)_high, big_endian,
-   ELF_MACHINE, 1);
+   EM_MIPS, 1);
 if (kernel_size >= 0) {
 if ((entry & ~0x7fffULL) == 0x8000)
 entry = (int32_t)entry;
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index f4dcacd..239a9fd 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -87,7 +87,7 @@ static int64_t load_kernel(void)
 kernel_size = load_elf(loaderparams.kernel_filename, 
cpu_mips_kseg0_to_phys,
NULL, (uint64_t *), NULL,
(uint64_t *)_high, big_endian,
-   ELF_MACHINE, 1);
+   EM_MIPS, 1);
 if (kernel_size >= 0) {
 if ((entry & ~0x7fffULL) == 0x8000)
 entry = (int32_t)entry;
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index c91883d..258cccd 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -5,8 +5,6 @@
 
 #define ALIGNED_ONLY
 
-#define ELF_MACHINEEM_MIPS
-
 #define CPUArchState struct CPUMIPSState
 
 #include "config.h"
-- 
1.9.1

[Qemu-devel] [PULL v1 15/21] sh4: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The only generic code relying on this is linux-user, but linux users'
default behaviour of defaulting ELF_MACHINE to ELF_ARCH will handle
this.

This removes another architecture specific definition from the global
namespace.

Cc: Aurelien Jarno 
Acked-by: Aurelien Jarno 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 target-sh4/cpu.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index 34bb3d7..e2913b8 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -24,8 +24,6 @@
 
 #define TARGET_LONG_BITS 32
 
-#define ELF_MACHINEEM_SH
-
 /* CPU Subtypes */
 #define SH_CPU_SH7750  (1 << 0)
 #define SH_CPU_SH7750S (1 << 1)
-- 
1.9.1

[Qemu-devel] [PATCH] docs: update the usage example of "dtrace" backend in tracing.txt

[Lin Ma]

The usage example of dtrace is quite ancient, We have tracetool.py with
different parameters instead of the original tracetool shell script for
a long time, So update the old information.

Signed-off-by: Lin Ma 
---
 docs/tracing.txt | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index 7117c5e..3853a6a 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -258,11 +258,11 @@ is generated to make use in scripts more convenient.  
This step can also be
 performed manually after a build in order to change the binary name in the .stp
 probes:
 
-scripts/tracetool --dtrace --stap \
-  --binary path/to/qemu-binary \
-  --target-type system \
-  --target-name x86_64 \
-  qemu.stp
+scripts/tracetool.py --backends=dtrace --format=stap \
+ --binary path/to/qemu-binary \
+ --target-type system \
+ --target-name x86_64 \
+ qemu.stp
 
 == Trace event properties ==
 
-- 
2.1.4

[Qemu-devel] [PATCH v1 10/15] exec-all: Move cpu_can_do_io() to qom/cpu.h

[Peter Crosthwaite]

This function has no architecture specific dependencies and should be
callable from core code. Move it to qom/cpu.h.

Reviewed-by: Richard Henderson 
Signed-off-by: Peter Crosthwaite 
---

 include/qom/cpu.h | 21 +
 1 file changed, 21 insertions(+)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 39712ab..7e5f3b0 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -696,6 +696,27 @@ void QEMU_NORETURN cpu_abort(CPUState *cpu, const char 
*fmt, ...)
 GCC_FMT_ATTR(2, 3);
 void cpu_exec_exit(CPUState *cpu);
 
+/**
+ * cpu_can_do_io:
+ * @cpu: The CPU for which to check IO.
+ *
+ * Deterministic execution requires that IO only be performed on the last
+ * instruction of a TB so that interrupts take effect immediately.
+ *
+ * Returns: %true if memory-mapped IO is safe, %false otherwise.
+ */
+static inline bool cpu_can_do_io(CPUState *cpu)
+{
+if (!use_icount) {
+return true;
+}
+/* If not executing code then assume we are ok.  */
+if (cpu->current_tb == NULL) {
+return true;
+}
+return cpu->can_do_io != 0;
+}
+
 #ifdef CONFIG_SOFTMMU
 extern const struct VMStateDescription vmstate_cpu_common;
 #else
-- 
1.9.1

[Qemu-devel] [PULL v1 19/21] alpha: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

ELF_MACHINE is unused by target alpha.

Cc: Richard Henderson 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 target-alpha/cpu.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 3f1ece3..b9e9907 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -32,8 +32,6 @@
 
 #include "fpu/softfloat.h"
 
-#define ELF_MACHINE EM_ALPHA
-
 #define ICACHE_LINE_SIZE 32
 #define DCACHE_LINE_SIZE 32
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH v17 00/21] Deterministic replay core

[Pavel Dovgaluk]

Paolo,

Are these patches good enough?

Pavel Dovgalyuk

> -Original Message-
> From: Pavel Dovgalyuk [mailto:pavel.dovga...@ispras.ru]
> Sent: Monday, September 07, 2015 11:40 AM
> To: qemu-devel@nongnu.org
> Cc: edgar.igles...@xilinx.com; peter.mayd...@linaro.org; 
> igor.rubi...@gmail.com;
> ebl...@redhat.com; mark.bur...@greensocs.com; r...@ispras.ru; 
> batuz...@ispras.ru;
> maria.klimushenk...@ispras.ru; pavel.dovga...@ispras.ru; pbonz...@redhat.com; 
> hi...@cert.org;
> alex.ben...@linaro.org; fred.kon...@greensocs.com
> Subject: [PATCH v17 00/21] Deterministic replay core
> 
> This set of patches is related to the reverse execution and deterministic
> replay of qemu execution. This implementation of deterministic replay can
> be used for deterministic debugging of guest code through gdb remote
> interface.
> 
> Core set of patches does not include support for reverse debugging commands
> of gdb, block devices' operations, USB replay support.
> 
> These patches include only core function of the replay,
> excluding the support for replaying serial, audio, network, and USB devices'
> operations. Reverse debugging and monitor commands were also excluded to
> be submitted later as separate patches.
> 
> Execution recording writes non-deterministic events log, which can be later
> used for replaying the execution anywhere and for unlimited number of times.
> It also supports checkpointing for faster rewinding during reverse debugging.
> Execution replaying reads the log and replays all non-deterministic events
> including external input, hardware clocks, and interrupts.
> 
> Full version of deterministic replay has the following features:
>  * Deterministically replays whole system execution and all contents of the 
> memory,
>state of the hadrware devices, clocks, and screen of the VM.
>  * Writes execution log into the file for latter replaying for multiple times
>on different machines.
>  * Supports i386, x86_64, ARM, PowerPC, and MIPS hardware platforms.
>  * Performs deterministic replay of all operations with keyboard and mouse
>input devices.
>  * Supports auto-checkpointing for convenient reverse debugging.
> 
> Usage of the record/replay core:
>  * First, record the execution, by adding the following string to the command 
> line:
>'-icount shift=7,rr=record,rrfile=replay.bin -net none'.
>Block devices' images are not actually changed in the recording mode,
>because all of the changes are written to the temporary overlay file.
>  * Then you can replay it for the multiple times by using another command
>line option: '-icount shift=7,rr=replay,rrfile=replay.bin -net none'
>  * '-net none' option should also be specified if network replay patches
>are not applied.
>  * Do not add any disk images to VM, because they are not supported by
>the core patches.
> 
> Papers with description of deterministic replay implementation:
> http://www.computer.org/csdl/proceedings/csmr/2012/4666/00/4666a553-abs.html
> http://dl.acm.org/citation.cfm?id=2786805.2803179
> 
> Public repository with current version of the patches:
> https://github.com/Dovgalyuk/qemu/tree/rr-17
> 
> Modifications of qemu include:
>  * wrappers for clock and time functions to save their return values in the 
> log
>  * saving different asynchronous events (e.g. system shutdown) into the log
>  * synchronization of the threads from thread pool
>  * recording/replaying user input (mouse and keyboard)
>  * adding internal events for cpu and io synchronization
> 
> v17 changes:
>  * Removed useless stub functions (as suggested by Paolo Bonzini)
>  * Refined checkpoint-related code (as suggested by Paolo Bonzini)
>  * Improved icount processing (as suggested by Paolo Bonzini)
>  * Added checkpoint for suspend event (as suggested by Paolo Bonzini)
>  * Fixed linux-user configurations build
>  * Minor fixes
> 
> v16 changes:
>  * Several warnings were fixed
> 
> v15 changes:
>  * Tested record/replay with MIPS and PowerPC guests
>  * Published the patches on github
>  * Fixed replay mutex operation in icount mode
>  * Fixed timers processing in record/replay mode
> 
> v14 changes:
>  * Minor fixes
> 
> v13 changes:
>  * Introduced "ptimer trigger" event (as suggested by Paolo Bonzini)
> 
> v12 changes:
>  * Removed block patches from the core patches set.
> 
> v11 changes:
>  * Fixed instructions event processing.
>  * Added some mutex protection calls for replay.
>  * Fixed replaying read operations for qcow2.
>  * Fixed rtc reads on initializations stage.
>  * Eliminated some warnings in replay module.
>  * Fixed misprints in documentation for replay (as suggested by Eric Blake)
> 
> v10 changes:
>  * Fixed queue processing for bottom halves (as suggested by Paolo Bonzini)
>  * Rewritten several replay functions (as suggested by Paolo Bonzini)
>  * Some minor fixes.
> 
> v9 changes:
>  * Replaced fwrite/fread with putc/getc (as suggested by Paolo Bonzini)
>  * Stopping virtual machine in case of 

Re: [Qemu-devel] [PATCH v17 00/21] Deterministic replay core

[Paolo Bonzini]

On 11/09/2015 07:52, Pavel Dovgaluk wrote:
> Paolo,
> 
> Are these patches good enough?

Haven't reviewed them as I already have a 50 patch queue.  I will look
at them next week.

I can already tell you that I'd like some comments in front of clock
warp calls, but that can be added as a follow up.

Paolo

Re: [Qemu-devel] [PATCH] ide: fix ATAPI command permissions

[Michael Tokarev]

09.09.2015 19:28, John Snow wrote:
> We're a little too lenient with what we'll let an ATAPI drive handle.
> Clamp down on the IDE command execution table to remove CD_OK permissions
> from commands that are not and have never been ATAPI commands.

FWIW, this issue has been assigned CVE-2015-6855 identifier, which
can be reflected in the commit message when applying.  Since this
issue has security impact, it might be a good idea to add

Cc: qemu-sta...@nongnu.org

Thanks,

/mjt

[Qemu-devel] [PULL v1 07/21] m68k: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The only generic code relying on this is linux-user, but linux users'
default behaviour of defaulting ELF_MACHINE to ELF_ARCH will handle
this.

The machine model bootloaders can just pass EM_68K directly, as that
is architecture specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Laurent Vivier 
Cc: Greg Ungerer 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Reviewed-by: Greg Ungerer 
Reviewed-by: Laurent Vivier 
Signed-off-by: Peter Crosthwaite 
---

 hw/m68k/an5206.c | 2 +-
 hw/m68k/dummy_m68k.c | 2 +-
 hw/m68k/mcf5208.c| 2 +-
 target-m68k/cpu.h| 2 --
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index f63ab2b..59e89fe 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -70,7 +70,7 @@ static void an5206_init(MachineState *machine)
 }
 
 kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   NULL, NULL, 1, ELF_MACHINE, 0);
+   NULL, NULL, 1, EM_68K, 0);
 entry = elf_entry;
 if (kernel_size < 0) {
 kernel_size = load_uimage(kernel_filename, , NULL, NULL,
diff --git a/hw/m68k/dummy_m68k.c b/hw/m68k/dummy_m68k.c
index 5b77d93..3463913 100644
--- a/hw/m68k/dummy_m68k.c
+++ b/hw/m68k/dummy_m68k.c
@@ -49,7 +49,7 @@ static void dummy_m68k_init(MachineState *machine)
 /* Load kernel.  */
 if (kernel_filename) {
 kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   NULL, NULL, 1, ELF_MACHINE, 0);
+   NULL, NULL, 1, EM_68K, 0);
 entry = elf_entry;
 if (kernel_size < 0) {
 kernel_size = load_uimage(kernel_filename, , NULL, NULL,
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 326a42d..cb57cf9 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -275,7 +275,7 @@ static void mcf5208evb_init(MachineState *machine)
 }
 
 kernel_size = load_elf(kernel_filename, NULL, NULL, _entry,
-   NULL, NULL, 1, ELF_MACHINE, 0);
+   NULL, NULL, 1, EM_68K, 0);
 entry = elf_entry;
 if (kernel_size < 0) {
 kernel_size = load_uimage(kernel_filename, , NULL, NULL,
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 9a62f6c..ebbbeef 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -32,8 +32,6 @@
 
 #define MAX_QREGS 32
 
-#define ELF_MACHINEEM_68K
-
 #define EXCP_ACCESS 2   /* Access (MMU) error.  */
 #define EXCP_ADDRESS3   /* Address error.  */
 #define EXCP_ILLEGAL4   /* Illegal instruction.  */
-- 
1.9.1

[Qemu-devel] [PATCH] iscsi: Add chap and "initiator-name" etc as per drive options

[Fam Zheng]

Previously we use "-iscsi id=target-iqn,user=foo,password=bar,..." to
specify iscsi connection parameters, unfortunately it doesn't work with
qemu-img.

This patch adds per drive options to iscsi driver so that at least
qemu-img can use the "json:{...}" filename magic.

Signed-off-by: Fam Zheng 
---
 block/iscsi.c | 83 +--
 1 file changed, 64 insertions(+), 19 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 5002916..9efb9ec 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1011,7 +1011,9 @@ retry:
 return 0;
 }
 
-static void parse_chap(struct iscsi_context *iscsi, const char *target,
+static void parse_chap(struct iscsi_context *iscsi,
+   QemuOpts *img_opts,
+   const char *target,
Error **errp)
 {
 QemuOptsList *list;
@@ -1025,19 +1027,22 @@ static void parse_chap(struct iscsi_context *iscsi, 
const char *target,
 }
 
 opts = qemu_opts_find(list, target);
-if (opts == NULL) {
+if (!opts) {
 opts = QTAILQ_FIRST(>head);
-if (!opts) {
-return;
-}
 }
 
-user = qemu_opt_get(opts, "user");
+user = qemu_opt_get(img_opts, "user");
+if (!user && opts) {
+user = qemu_opt_get(opts, "user");
+}
 if (!user) {
 return;
 }
 
-password = qemu_opt_get(opts, "password");
+password = qemu_opt_get(img_opts, "password");
+if (!password && opts) {
+password = qemu_opt_get(opts, "password");
+}
 if (!password) {
 error_setg(errp, "CHAP username specified but no password was given");
 return;
@@ -1048,13 +1053,20 @@ static void parse_chap(struct iscsi_context *iscsi, 
const char *target,
 }
 }
 
-static void parse_header_digest(struct iscsi_context *iscsi, const char 
*target,
+static void parse_header_digest(struct iscsi_context *iscsi,
+QemuOpts *img_opts,
+const char *target,
 Error **errp)
 {
 QemuOptsList *list;
 QemuOpts *opts;
 const char *digest = NULL;
 
+digest = qemu_opt_get(img_opts, "header-digest");
+if (digest) {
+goto found;
+}
+
 list = qemu_find_opts("iscsi");
 if (!list) {
 return;
@@ -1073,6 +1085,7 @@ static void parse_header_digest(struct iscsi_context 
*iscsi, const char *target,
 return;
 }
 
+found:
 if (!strcmp(digest, "CRC32C")) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
 } else if (!strcmp(digest, "NONE")) {
@@ -1086,7 +1099,7 @@ static void parse_header_digest(struct iscsi_context 
*iscsi, const char *target,
 }
 }
 
-static char *parse_initiator_name(const char *target)
+static char *parse_initiator_name(QemuOpts *img_opts, const char *target)
 {
 QemuOptsList *list;
 QemuOpts *opts;
@@ -1094,6 +1107,11 @@ static char *parse_initiator_name(const char *target)
 char *iscsi_name;
 UuidInfo *uuid_info;
 
+name = qemu_opt_get(img_opts, "initiator-name");
+if (name) {
+return g_strdup(name);
+}
+
 list = qemu_find_opts("iscsi");
 if (list) {
 opts = qemu_opts_find(list, target);
@@ -1120,12 +1138,17 @@ static char *parse_initiator_name(const char *target)
 return iscsi_name;
 }
 
-static int parse_timeout(const char *target)
+static int parse_timeout(QemuOpts *img_opts, const char *target)
 {
 QemuOptsList *list;
 QemuOpts *opts;
 const char *timeout;
 
+timeout = qemu_opt_get(img_opts, "iscsi");
+if (timeout) {
+goto out;
+}
+
 list = qemu_find_opts("iscsi");
 if (list) {
 opts = qemu_opts_find(list, target);
@@ -1134,13 +1157,14 @@ static int parse_timeout(const char *target)
 }
 if (opts) {
 timeout = qemu_opt_get(opts, "timeout");
-if (timeout) {
-return atoi(timeout);
-}
 }
 }
-
-return 0;
+out:
+if (timeout) {
+return atoi(timeout);
+} else {
+return 0;
+}
 }
 
 static void iscsi_nop_timed_event(void *opaque)
@@ -1229,6 +1253,27 @@ static QemuOptsList runtime_opts = {
 .name = "filename",
 .type = QEMU_OPT_STRING,
 .help = "URL to the iscsi image",
+},{
+.name = "user",
+.type = QEMU_OPT_STRING,
+.help = "username for CHAP authentication to target",
+},{
+.name = "password",
+.type = QEMU_OPT_STRING,
+.help = "password for CHAP authentication to target",
+},{
+.name = "header-digest",
+.type = QEMU_OPT_STRING,
+.help = "HeaderDigest setting. "
+"{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
+},{
+.name = "initiator-name",
+.type = QEMU_OPT_STRING,
+.help = "Initiator iqn 

[Qemu-devel] [PATCH v1 12/15] cputlb: Change tlb_set_dirty() arg to cpu

[Peter Crosthwaite]

Change tlb_set_dirty() to accept a CPU instead of an env pointer. This
allows for removal of another CPUArchState usage from prototypes that
need to be QOMified.

Signed-off-by: Peter Crosthwaite 
---

 cputlb.c  | 3 ++-
 exec.c| 3 +--
 include/exec/cputlb.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 5ccd545..6264e49 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -293,8 +293,9 @@ static inline void tlb_set_dirty1(CPUTLBEntry *tlb_entry, 
target_ulong vaddr)
 
 /* update the TLB corresponding to virtual page vaddr
so that it is no longer dirty */
-void tlb_set_dirty(CPUArchState *env, target_ulong vaddr)
+void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
 {
+CPUArchState *env = cpu->env_ptr;
 int i;
 int mmu_idx;
 
diff --git a/exec.c b/exec.c
index e3568c3..567854e 100644
--- a/exec.c
+++ b/exec.c
@@ -1903,8 +1903,7 @@ static void notdirty_mem_write(void *opaque, hwaddr 
ram_addr,
 /* we remove the notdirty callback only if the code has been
flushed */
 if (!cpu_physical_memory_is_clean(ram_addr)) {
-CPUArchState *env = current_cpu->env_ptr;
-tlb_set_dirty(env, current_cpu->mem_io_vaddr);
+tlb_set_dirty(current_cpu, current_cpu->mem_io_vaddr);
 }
 }
 
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index c3aaa30..7ad5c9a 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -26,7 +26,7 @@ void tlb_unprotect_code(ram_addr_t ram_addr);
 void tlb_reset_dirty_range(CPUTLBEntry *tlb_entry, uintptr_t start,
uintptr_t length);
 void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
-void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
+void tlb_set_dirty(CPUState *cpu, target_ulong vaddr);
 extern int tlb_flush_count;
 
 /* exec.c */
-- 
1.9.1

[Qemu-devel] [PULL v1 21/21] ppc: Rename ELF_MACHINE to be PPC specific

[Peter Crosthwaite]

From: Peter Crosthwaite 

Rename ELF_MACHINE to be PPC specific. This is used as-is by the
various PPC bootloaders and is locally defined to ELF_MACHINE in linux
user in PPC specific ifdeffery.

This removes another architecture specific definition from the global
namespace (as desired by multi-arch).

Cc: Alexander Graf 
Cc: qemu-...@nongnu.org
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 hw/ppc/e500.c  | 2 +-
 hw/ppc/mac_newworld.c  | 4 ++--
 hw/ppc/mac_oldworld.c  | 4 ++--
 hw/ppc/ppc440_bamboo.c | 2 +-
 hw/ppc/prep.c  | 2 +-
 hw/ppc/spapr.c | 4 ++--
 hw/ppc/virtex_ml507.c  | 2 +-
 linux-user/elfload.c   | 1 +
 target-ppc/cpu.h   | 4 ++--
 9 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index d300846..8b45bf6 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1017,7 +1017,7 @@ void ppce500_init(MachineState *machine, PPCE500Params 
*params)
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 
 bios_size = load_elf(filename, NULL, NULL, _entry, , NULL,
- 1, ELF_MACHINE, 0);
+ 1, PPC_ELF_MACHINE, 0);
 if (bios_size < 0) {
 /*
  * Hrm. No ELF image? Try a uImage, maybe someone is giving us an
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 77d5c81..81d744c 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -219,7 +219,7 @@ static void ppc_core99_init(MachineState *machine)
 /* Load OpenBIOS (ELF) */
 if (filename) {
 bios_size = load_elf(filename, NULL, NULL, NULL,
- NULL, NULL, 1, ELF_MACHINE, 0);
+ NULL, NULL, 1, PPC_ELF_MACHINE, 0);
 
 g_free(filename);
 } else {
@@ -242,7 +242,7 @@ static void ppc_core99_init(MachineState *machine)
 kernel_base = KERNEL_LOAD_ADDR;
 
 kernel_size = load_elf(kernel_filename, translate_kernel_address, NULL,
-   NULL, , NULL, 1, ELF_MACHINE, 0);
+   NULL, , NULL, 1, PPC_ELF_MACHINE, 0);
 if (kernel_size < 0)
 kernel_size = load_aout(kernel_filename, kernel_base,
 ram_size - kernel_base, bswap_needed,
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 06fdbaf..00cb080 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -147,7 +147,7 @@ static void ppc_heathrow_init(MachineState *machine)
 /* Load OpenBIOS (ELF) */
 if (filename) {
 bios_size = load_elf(filename, 0, NULL, NULL, NULL, NULL,
- 1, ELF_MACHINE, 0);
+ 1, PPC_ELF_MACHINE, 0);
 g_free(filename);
 } else {
 bios_size = -1;
@@ -168,7 +168,7 @@ static void ppc_heathrow_init(MachineState *machine)
 #endif
 kernel_base = KERNEL_LOAD_ADDR;
 kernel_size = load_elf(kernel_filename, translate_kernel_address, NULL,
-   NULL, , NULL, 1, ELF_MACHINE, 0);
+   NULL, , NULL, 1, PPC_ELF_MACHINE, 0);
 if (kernel_size < 0)
 kernel_size = load_aout(kernel_filename, kernel_base,
 ram_size - kernel_base, bswap_needed,
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 032fa80..c54f79d 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -256,7 +256,7 @@ static void bamboo_init(MachineState *machine)
   NULL, NULL);
 if (success < 0) {
 success = load_elf(kernel_filename, NULL, NULL, _entry,
-   _lowaddr, NULL, 1, ELF_MACHINE, 0);
+   _lowaddr, NULL, 1, PPC_ELF_MACHINE, 0);
 entry = elf_entry;
 loadaddr = elf_lowaddr;
 }
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 45b5f62..b421bff 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -610,7 +610,7 @@ static void ppc_prep_init(MachineState *machine)
 bios_name = BIOS_FILENAME;
 }
 qdev_prop_set_string(dev, "bios-name", bios_name);
-qdev_prop_set_uint32(dev, "elf-machine", ELF_MACHINE);
+qdev_prop_set_uint32(dev, "elf-machine", PPC_ELF_MACHINE);
 pcihost = PCI_HOST_BRIDGE(dev);
 object_property_add_child(qdev_get_machine(), "raven", OBJECT(dev), NULL);
 qdev_init_nofail(dev);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bf0c64f..4500497 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1636,11 +1636,11 @@ static void ppc_spapr_init(MachineState *machine)
 uint64_t lowaddr = 0;
 
 kernel_size = load_elf(kernel_filename, translate_kernel_address, NULL,
-   NULL, , NULL, 1, ELF_MACHINE, 0);
+   NULL, , NULL, 1, 

[Qemu-devel] [PULL v1 08/21] cris: Remove ELF_MACHINE from cpu.h

[Peter Crosthwaite]

From: Peter Crosthwaite 

The only generic code relying on this is linux-user, but linux users'
default behaviour of defaulting ELF_MACHINE to ELF_ARCH will handle
this.

The bootloader can just pass EM_CRIS directly, as that is architecture
specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Edgar E. Iglesias 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
---

 hw/cris/boot.c| 2 +-
 target-cris/cpu.h | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/cris/boot.c b/hw/cris/boot.c
index 622f353..1cfa339 100644
--- a/hw/cris/boot.c
+++ b/hw/cris/boot.c
@@ -72,7 +72,7 @@ void cris_load_image(CRISCPU *cpu, struct cris_load_info *li)
 /* Boots a kernel elf binary, os/linux-2.6/vmlinux from the axis 
devboard SDK.  */
 image_size = load_elf(li->image_filename, translate_kernel_address, NULL,
-  , NULL, , 0, ELF_MACHINE, 0);
+  , NULL, , 0, EM_CRIS, 0);
 li->entry = entry;
 if (image_size < 0) {
 /* Takes a kimage from the axis devboard SDK.  */
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index d422e35..c22d16c 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -29,8 +29,6 @@
 
 #include "exec/cpu-defs.h"
 
-#define ELF_MACHINEEM_CRIS
-
 #define EXCP_NMI1
 #define EXCP_GURU   2
 #define EXCP_BUSFAULT   3
-- 
1.9.1

Re: [Qemu-devel] Aspirant for AMD IOMMU emulation project for Outreachy

[Jan Kiszka]

Hi Rita,

[CC'ing Andrew due to overlap with split irqchip topic]

On 2015-09-09 19:41, Rita Sinha wrote:
> Hi Jan,
> 
>>
>> Most likely than not you'll work on the Intel IOMMU and I would
>> suggest, if you wish to get your feet dirty, just start right away
>> with the Intel IOMMU (In which case Jan could direct you better)
>> instead of the AMD one.
>>
> 
> As suggested by David, I would request you to guide me as to how to
> get started with Intel IOMMU.
> 

You can find the basic problem description and some links here:

http://wiki.qemu.org/Google_Summer_of_Code_2015#VT-d_interrupt_emulation_with_KVM_support

Who much do you know about IOMMUs and interrupt remapping in general?
The spec may provide a certain introduction to the topic, but it also
quickly dives into details. Let me know where you see gaps to build up a
better understanding of the topic.

Implementation-wise, you could start off with the existing interrupt
remapping patches and look into the missing error reporting for IR on
that basis. I've just rebased and pushed a new version. Error reporting
is currently done in form of plain printf. The correct behaviour is
described in the spec. Feel free to nag me with questions on how things
work around interrupt remapping in general and its QEMU integration in
particular.

The goal is to get this upstream eventually. So the current design needs
to be prepared for submission, reviewed by subsystem maintainer, and
then possibly reworked based on their feedback. We will also need some
command line switch (machine property, like iommu=on) to control the
availability of IR because the original Q35 chipset didn't include this
feature.

But there is also the aim to integrate it with KVM support, and that's
why I'm CC'ing Andrew. At Google, they are currently working on QEMU
patches for their split KVM irqchip model. This will push the IOAPIC
into QEMU hands, even when KVM in-kernel APIC acceleration is used. And
that will allow IR to be installed easily, without any KVM kernel-side
changes.

Andrew, what is the status of your QEMU patches? Are there chances to
preview them in order to asses if and how much integration work is
needed for interrupt remapping?

Rita, you probably noticed that this is no beginner's task, both
regarding the hardware background as well as the QEMU subsystems. You
have worked with several low-level software components before and have
some QEMU experience, so this topic could be a good match for you.
Still, if you have doubts after looking into details, let me know so
that we can discuss them.

Best regards,
Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH PULL 02/11] tests: remove repetition in unit test object deps

[Eric Blake]

On 09/11/2015 06:10 AM, Daniel P. Berrange wrote:
> Most of the unit tests have identical sets of object deps.
> For example all block unit tests need to depend on
> 
>  $(block-obj-y) libqemuutil.a libqemustub.a
> 
> Currently each unit test repeats this list of test deps.
> This list of deps will grow as future patches add more
> modules to the build, so define some common variables
> that can be used by all unit tests to remove the
> repetition.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  tests/Makefile | 94 
> ++
>  1 file changed, 49 insertions(+), 45 deletions(-)
> 

> +++ b/tests/Makefile
> @@ -276,47 +276,51 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o 
> tests/check-qdict.o \
>   tests/test-opts-visitor.o tests/test-qmp-event.o \
>   tests/rcutorture.o tests/test-rcu-list.o

As it is, I found that we have a bug where we rely on undefined
$(qapi-py). So you may want to squash this in (otherwise, I have it as a
patch on my local tree that I still need to publish):


+++ b/tests/Makefile
@@ -1,5 +1,7 @@
 export SRC_PATH

+qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
+
 # Get the list of all supported sysemu targets
 SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \
$(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak)))


> +
> +# Deps that are common to various different sets of tests below
> +test-util-obj-y = libqemuutil.a libqemustub.a
> +test-qom-obj-y = qom/object.o qom/qom-qobject.o \
> + qom/container.o qom/object_interfaces.o \
> + $(test-util-obj-y)
> +test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
> + tests/test-qapi-event.o \
> + $(test-qom-obj-y)

as well as add $(qapi-py) to the prereqs of test-qapi-obj-y.

but overall looks sane.

-- 
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 2/2] tests: add a local test for guest agent

[Marc-André Lureau]

Hi

On Wed, Sep 9, 2015 at 11:53 PM, Michael Roth  wrote:
> Quoting marcandre.lur...@redhat.com (2015-08-27 05:52:07)
>> From: Marc-André Lureau 
>>
>> Add some local guest agent tests (as it is better than nothing).
>>
>> They can be run inside or outside a VM, when run inside a VM, they will
>> do a bit more side effects, such as freezing/thawing the FS or changing
>> the time.
>>
>> A better test would involve a managed VM (or container), but it might be
>> better to leave that off to autotest/avocado.
>>
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  tests/Makefile   |   3 +
>>  tests/test-qga.c | 686 
>> +++
>>  2 files changed, 689 insertions(+)
>>  create mode 100644 tests/test-qga.c
>>
>> diff --git a/tests/Makefile b/tests/Makefile
>> index 5271123..35671a1 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -76,6 +76,7 @@ check-unit-y += tests/test-write-threshold$(EXESUF)
>>  gcov-files-test-write-threshold-y = block/write-threshold.c
>>  check-unit-$(CONFIG_GNUTLS_HASH) += tests/test-crypto-hash$(EXESUF)
>>  check-unit-y += tests/test-crypto-cipher$(EXESUF)
>> +check-unit-y += tests/test-qga$(EXESUF)
>>
>>  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
>>
>> @@ -425,6 +426,8 @@ endif
>>  qtest-obj-y = tests/libqtest.o libqemuutil.a libqemustub.a
>>  $(check-qtest-y): $(qtest-obj-y)
>>
>> +tests/test-qga: tests/test-qga.o $(qtest-obj-y)
>> +
>>  .PHONY: check-help
>>  check-help:
>> @echo "Regression testing targets:"
>> diff --git a/tests/test-qga.c b/tests/test-qga.c
>> new file mode 100644
>> index 000..1fb58ec
>> --- /dev/null
>> +++ b/tests/test-qga.c
>> @@ -0,0 +1,686 @@
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "libqtest.h"
>> +#include "config-host.h"
>> +
>> +typedef struct {
>> +char *test_dir;
>> +GMainLoop *loop;
>> +int fd;
>> +GPid pid;
>> +} TestFixture;
>> +
>> +static int connect_qga(char *path)
>> +{
>> +int s, ret, len;
>> +struct sockaddr_un remote;
>> +
>> +s = socket(AF_UNIX, SOCK_STREAM, 0);
>> +g_assert(s != -1);
>> +
>> +remote.sun_family = AF_UNIX;
>> +do {
>> +strcpy(remote.sun_path, path);
>> +len = strlen(remote.sun_path) + sizeof(remote.sun_family);
>> +ret = connect(s, (struct sockaddr *), len);
>> +if (ret == -1) {
>> +g_usleep(G_USEC_PER_SEC);
>> +}
>> +} while (ret == -1);
>
> I think we should loop on ECONNREFUSED (or whatever corresponds to
> the socket not yet being created/bound), and the usual suspects
> like EINTR. If it's anything else I don't think we'd want this to
> loop indefinitely.

You get ENOENT while qga starts. I will just give up after 5 tries instead.

>> +
>> +return s;
>> +}
>> +
>> +static void qga_watch(GPid pid, gint status, gpointer user_data)
>> +{
>> +TestFixture *fixture = user_data;
>> +
>> +g_assert_cmpint(status, ==, 0);
>> +g_main_loop_quit(fixture->loop);
>> +}
>> +
>> +static void
>> +fixture_setup(TestFixture *fixture, gconstpointer data)
>> +{
>> +const gchar *extra_arg = data;
>> +GError *error = NULL;
>> +gchar *cwd, *path, *cmd, **argv = NULL;
>> +
>> +fixture->loop = g_main_loop_new(NULL, FALSE);
>> +
>> +fixture->test_dir = g_strdup("/tmp/qgatest.XX");
>> +g_assert_nonnull(g_mkdtemp(fixture->test_dir));
>> +
>> +path = g_build_filename(fixture->test_dir, "sock", NULL);
>> +cwd = g_get_current_dir();
>> +cmd = g_strdup_printf("%s%cqemu-ga -m unix-listen -t %s -p %s %s %s",
>
> Would be nice if we could also run qemu-ga.exe for a w32 build with
> binfmt set up like some of the other tests, but no idea how we'd get
> at it through wine. I guess it would have to be a serial connection
> or something...
>
> Probably too environment-specific for a useful unit test. I guess in
> the future we could plumb in custom qemu-ga invocation/script that
> does some extra setup on the backend (VM/container/wine) and exposes
> that via a local socket like we'd get via -chardev.

Perhaps it would be just too much at this point for a make check test,
better to leave that to autotest etc.

At this point, I propose we make it check-unit-$(CONFIG_LINUX).

>
>> +  cwd, G_DIR_SEPARATOR,
>> +  fixture->test_dir, path,
>> +  getenv("QTEST_LOG") ? "-v" : "",
>> +  extra_arg ?: "");
>> +g_shell_parse_argv(cmd, NULL, , );
>> +g_assert_no_error(error);
>> +
>> +g_spawn_async(fixture->test_dir, argv, NULL,
>> +  G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,
>> +  NULL, NULL, >pid, );
>> +g_assert_no_error(error);
>> +
>> +g_child_watch_add(fixture->pid, 

Re: [Qemu-devel] [PATCH v5 10/14] blockdev: make BlockJobTxn available to qmp 'transaction'

[Max Reitz]

On 07.09.2015 09:34, Fam Zheng wrote:
> From: Stefan Hajnoczi 
> 
> Provide a BlockJobTxn to actions executed in a qmp 'transaction'
> command.  This allows actions to make their block jobs either complete
> as a group or fail/cancel together.
> 
> The next patch adds the first user.
> 
> Signed-off-by: Stefan Hajnoczi 
> Reviewed-by: Fam Zheng 
> Reviewed-by: John Snow 

You can vouch for yourself, but can you vouch for John being fine with
the change from v4? ;-)

> Signed-off-by: Fam Zheng 
> ---
>  blockdev.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 4ebded8..3d5e5cd 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1285,6 +1285,7 @@ typedef struct BlkActionOps {
>  struct BlkActionState {
>  TransactionAction *action;
>  const BlkActionOps *ops;
> +BlockJobTxn *block_job_txn;
>  QSIMPLEQ_ENTRY(BlkActionState) entry;
>  };
>  
> @@ -1889,12 +1890,15 @@ static const BlkActionOps actions[] = {
>  void qmp_transaction(TransactionActionList *dev_list, Error **errp)
>  {
>  TransactionActionList *dev_entry = dev_list;
> +BlockJobTxn *block_job_txn;
>  BlkActionState *state, *next;
>  Error *local_err = NULL;
>  
>  QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
>  QSIMPLEQ_INIT(_bdrv_states);
>  
> +block_job_txn = block_job_txn_new();
> +
>  /* drain all i/o before any operations */
>  bdrv_drain_all();
>  
> @@ -1914,6 +1918,7 @@ void qmp_transaction(TransactionActionList *dev_list, 
> Error **errp)
>  state = g_malloc0(ops->instance_size);
>  state->ops = ops;
>  state->action = dev_info;
> +state->block_job_txn = block_job_txn;
>  QSIMPLEQ_INSERT_TAIL(_bdrv_states, state, entry);
>  
>  state->ops->prepare(state, _err);
> @@ -1940,6 +1945,7 @@ delete_and_fail:
>  }
>  }
>  exit:
> +block_job_txn_unref(block_job_txn);
>  QSIMPLEQ_FOREACH_SAFE(state, _bdrv_states, entry, next) {
>  if (state->ops->clean) {
>  state->ops->clean(state);
> 

I'd put the unref() call after the FOREACH loop, since the txn is
referenced by every state->block_job_txn, so the state->ops->clean()
operation may still try to access that field.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes

[Daniel P. Berrange]

On Wed, Sep 02, 2015 at 06:18:28PM +0200, Andreas Färber wrote:
> Am 26.08.2015 um 14:03 schrieb Daniel P. Berrange:
> > When there are many instances of a given class, registering
> > properties against the instance is wasteful of resources. The
> > majority of objects have a statically defined list of possible
> > properties, so most of the properties are easily registerable
> > against the class. Only those properties which are conditionally
> > registered at runtime need be recorded against the klass.
> > 
> > Registering properties against classes also makes it possible
> > to provide static introspection of QOM - currently introspection
> > is only possible after creating an instance of a class, which
> > severely limits its usefulness.
> > 
> > This impl only supports simple scalar properties. It does not
> > attempt to allow child object / link object properties against
> > the class. There are ways to support those too, but it would
> > make this patch more complicated, so it is left as an exercise
> > for the future.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  include/qom/object.h |  44 ++
> >  qom/object.c | 233 
> > +--
> >  2 files changed, 270 insertions(+), 7 deletions(-)
> > 

[snip]

> Did you do any benchmarks on performance impact?

I created a crude test benchmark, consisting of an object which
has 20 string properties registered against it.

I then timed how long it took to create and free 1,000,000 instances
of this object.

 - Props registered against object: 8.5 seconds
 - Props registered against class:  1.1 seconds

I created a second test that also timed object accesses but there
was no measurable difference.

As expected, there is a considerable memory benefit - with 20 properties,
each object consumed 2600 bytes less, when registered to the class, or
approx 130 bytes less per property.

AFAICT, the performance benefit is essentially due to not having the
overhead of mallocing the ObjectProperty struct and its contents for
every new object instance.


BTW in testing this I found the g_mem_set_vtable() call has a
quite considerable performance penalty, even when tracing is not
compiled in. It added 17% to my test running time!  We might want
to investigate that performance impact in QEMU as a whole and
check that it is actually acceptable for real deployment.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v14 2/5] intc/gic: Extract some reusable vGIC code

[Peter Maydell]

On 9 September 2015 at 08:49, Pavel Fedin  wrote:
> Some functions previously used only by vGICv2 are useful also for vGICv3
> implementation. Untie them from GICState and make accessible from within
> other modules:
> - kvm_arm_gic_set_irq()
> - kvm_gic_supports_attr() - moved to common code and renamed to
>   kvm_device_check_attr()
> - kvm_gic_access() - turned into GIC-independent kvm_device_access().
>   Data pointer changed to void * because some GICv3 registers are
>   64-bit wide
>
> Some of these changes are not used right now, but they will be helpful for
> implementing live migration.
>
> Actually kvm_dist_get() and kvm_dist_put() could also be made reusable, but
> they would require two extra parameters (s->dev_fd and s->num_cpu) as well as
> lots of typecasts of 's' to DeviceState * and back to GICState *. This makes
> the code very ugly so i decided to stop at this point. I tried also an
> approach with making a base class for all possible GICs, but it would contain
> only three variables (dev_fd, cpu_num and irq_num), and accessing them through
> the rest of the code would be again tedious (either ugly casts or qemu-style
> separate object pointer). So i disliked it too.
>
> Signed-off-by: Pavel Fedin 
> Tested-by: Ashok kumar 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 3/4] block: add a 'blockdev-snapshot' QMP command

[Max Reitz]

On 10.09.2015 15:39, Alberto Garcia wrote:
> One of the limitations of the 'blockdev-snapshot-sync' command is that
> it does not allow passing BlockdevOptions to the newly created
> snapshots, so they are always opened using the default values.
> 
> Extending the command to allow passing options is not a practical
> solution because there is overlap between those options and some of
> the existing parameters of the command.
> 
> This patch introduces a new 'blockdev-snapshot' command with a simpler
> interface: it just takes two references to existing block devices that
> will be used as the source and target for the snapshot.
> 
> Since the main difference between the two commands is that one of them
> creates and opens the target image, while the other uses an already
> opened one, the bulk of the implementation is shared.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  blockdev.c   | 163 
> ---
>  qapi-schema.json |   2 +
>  qapi/block-core.json |  26 
>  qmp-commands.hx  |  29 +
>  4 files changed, 160 insertions(+), 60 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 0/3] i.MX: Add GPIO devices to i.MX SOC

[Peter Maydell]

On 9 September 2015 at 21:08, Jean-Christophe Dubois
 wrote:
> Add GPIO devices to i.MX31 and i.MX25 SOC
>
> Jean-Christophe Dubois (3):
>   i.MX: Add GPIO device
>   i.MX: Add GPIO devices to i.MX31 SOC
>   i.MX: Add GPIO devices to i.MX25 SOC
>



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH PULL 00/11] Extract TLS handling code from VNC server

[Eric Blake]

On 09/11/2015 07:09 AM, Daniel P. Berrange wrote:

>> Apologies for not (re-)reviewing sooner, but I have comments on (at
>> least) patch 1, so I'm not sure this series should be merged just yet.
> 
> Ok, please make sure you re-review this series, and not the earlier
> v6 posting, as a few things have been fixed here.

Okay, I've gone through the rest of the pull request, and nothing else
jumped out at me beyond patch 2, although it was superficial enough that
I'm not comfortable with re-adding R-b on the patches where you dropped
it.  At this point, I'm trusting you as maintainer sending a valid
pullreq for your part of the tree.

I'm adding Markus in cc to make sure he is aware that your v2 pullreq
will affect introspection refactoring.

-- 
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] qcow2: Make qcow2_alloc_bytes() more explicit

[Kevin Wolf]

Am 11.09.2015 um 18:47 hat Max Reitz geschrieben:
> In case of -EAGAIN returned by update_refcount(), we should discard the
> cluster offset we were trying to allocate and request a new one, because
> in theory that old offset might now be taken by a refcount block.
> 
> In practice, this was not the case due to update_refcount() generally
> returning strictly monotonic increasing cluster offsets. However, this
> behavior is not set in stone, and it is also not obvious when looking at
> qcow2_alloc_bytes() alone, so we should not rely on it.
> 
> Reported-by: Kevin Wolf 
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2-refcount.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index e8430ec..c30bb14 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -949,11 +949,17 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int 
> size)
>  
>  if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) 
> {
>  offset = new_cluster;
> +free_in_cluster = s->cluster_size;
> +} else {
> +free_in_cluster += s->cluster_size;
>  }
>  }

This doesn't actually do anything except confuse the reader, but as
the value of free_in_cluster doesn't matter in the second iteration
because offset == 0, this is correct.

>  assert(offset);
>  ret = update_refcount(bs, offset, size, 1, false, 
> QCOW2_DISCARD_NEVER);
> +if (ret < 0) {
> +offset = 0;
> +}
>  } while (ret == -EAGAIN);
>  if (ret < 0) {
>  return ret;

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH v5 07/14] blockjob: Add "completed" and "ret" in BlockJob

[Max Reitz]

On 07.09.2015 09:34, Fam Zheng wrote:
> They are set when block_job_completed is called.
> 
> Signed-off-by: Fam Zheng 
> Reviewed-by: John Snow 
> ---
>  blockjob.c   | 3 +++
>  include/block/blockjob.h | 9 +
>  2 files changed, 12 insertions(+)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v1 1/1] xlnx-zynqmp: Remove unnecessary brackets around error messages

[Alistair Francis]

On Fri, Sep 11, 2015 at 8:36 AM, Peter Maydell  wrote:
> On 9 September 2015 at 01:32, Alistair Francis
>  wrote:
>> The errp and err variable have unnecessary brackets around them,
>> so remove the brackets.
>>
>> Signed-off-by: Alistair Francis 
>> ---
>>
>>  hw/arm/xlnx-zynqmp.c |   10 +-
>>  1 files changed, 5 insertions(+), 5 deletions(-)
>
>
>
> Applied to target-arm.next, thanks.

Thanks Peter and Peter

Alistair

>
> -- PMM
>

[Qemu-devel] [PULL 2/4] softmmu: add helper function to pass through retaddr

[Richard Henderson]

From: Pavel Dovgalyuk 

This patch introduces several helpers to pass return address
which points to the TB. Correct return address allows correct
restoring of the guest PC and icount. These functions should be used when
helpers embedded into TB invoke memory operations.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Pavel Dovgalyuk 
Message-Id: <20150710095650.13280.32255.stgit@PASHA-ISP>
Signed-off-by: Richard Henderson 
---
 include/exec/cpu_ldst_template.h  | 59 ++-
 include/exec/cpu_ldst_useronly_template.h | 25 +
 softmmu_template.h|  6 
 tcg/tcg.h | 23 
 4 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h
index 95ab750..3091c00 100644
--- a/include/exec/cpu_ldst_template.h
+++ b/include/exec/cpu_ldst_template.h
@@ -27,20 +27,24 @@
 #define SUFFIX q
 #define USUFFIX q
 #define DATA_TYPE uint64_t
+#define SHIFT 3
 #elif DATA_SIZE == 4
 #define SUFFIX l
 #define USUFFIX l
 #define DATA_TYPE uint32_t
+#define SHIFT 2
 #elif DATA_SIZE == 2
 #define SUFFIX w
 #define USUFFIX uw
 #define DATA_TYPE uint16_t
 #define DATA_STYPE int16_t
+#define SHIFT 1
 #elif DATA_SIZE == 1
 #define SUFFIX b
 #define USUFFIX ub
 #define DATA_TYPE uint8_t
 #define DATA_STYPE int8_t
+#define SHIFT 0
 #else
 #error unsupported data size
 #endif
@@ -54,27 +58,36 @@
 #ifdef SOFTMMU_CODE_ACCESS
 #define ADDR_READ addr_code
 #define MMUSUFFIX _cmmu
+#define URETSUFFIX SUFFIX
+#define SRETSUFFIX SUFFIX
 #else
 #define ADDR_READ addr_read
 #define MMUSUFFIX _mmu
+#define URETSUFFIX USUFFIX
+#define SRETSUFFIX glue(s, SUFFIX)
 #endif
 
 /* generic load/store macros */
 
 static inline RES_TYPE
-glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
+glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
+  target_ulong ptr,
+  uintptr_t retaddr)
 {
 int page_index;
 RES_TYPE res;
 target_ulong addr;
 int mmu_idx;
+TCGMemOpIdx oi;
 
 addr = ptr;
 page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
 mmu_idx = CPU_MMU_INDEX;
 if (unlikely(env->tlb_table[mmu_idx][page_index].ADDR_READ !=
  (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1) {
-res = glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(env, addr, mmu_idx);
+oi = make_memop_idx(SHIFT, mmu_idx);
+res = glue(glue(helper_ret_ld, URETSUFFIX), MMUSUFFIX)(env, addr,
+oi, retaddr);
 } else {
 uintptr_t hostaddr = addr + env->tlb_table[mmu_idx][page_index].addend;
 res = glue(glue(ld, USUFFIX), _p)((uint8_t *)hostaddr);
@@ -82,27 +95,43 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, 
target_ulong ptr)
 return res;
 }
 
+static inline RES_TYPE
+glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
+{
+return glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(env, ptr, 0);
+}
+
 #if DATA_SIZE <= 2
 static inline int
-glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
+glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
+  target_ulong ptr,
+  uintptr_t retaddr)
 {
 int res, page_index;
 target_ulong addr;
 int mmu_idx;
+TCGMemOpIdx oi;
 
 addr = ptr;
 page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
 mmu_idx = CPU_MMU_INDEX;
 if (unlikely(env->tlb_table[mmu_idx][page_index].ADDR_READ !=
  (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1) {
-res = (DATA_STYPE)glue(glue(helper_ld, SUFFIX),
-   MMUSUFFIX)(env, addr, mmu_idx);
+oi = make_memop_idx(SHIFT, mmu_idx);
+res = (DATA_STYPE)glue(glue(helper_ret_ld, SRETSUFFIX),
+   MMUSUFFIX)(env, addr, oi, retaddr);
 } else {
 uintptr_t hostaddr = addr + env->tlb_table[mmu_idx][page_index].addend;
 res = glue(glue(lds, SUFFIX), _p)((uint8_t *)hostaddr);
 }
 return res;
 }
+
+static inline int
+glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
+{
+return glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(env, ptr, 0);
+}
 #endif
 
 #ifndef SOFTMMU_CODE_ACCESS
@@ -110,25 +139,36 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, 
target_ulong ptr)
 /* generic store macro */
 
 static inline void
-glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong ptr,
-  RES_TYPE v)
+glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
+ 

Re: [Qemu-devel] [PATCH v3 1/4] block: rename BlockdevSnapshot to BlockdevSnapshotSync

[Max Reitz]

On 10.09.2015 15:39, Alberto Garcia wrote:
> We will introduce the 'blockdev-snapshot' command that will require
> its own struct for the parameters, so we need to rename this one in
> order to avoid name clashes.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Eric Blake 
> ---
>  blockdev.c   | 2 +-
>  qapi-schema.json | 2 +-
>  qapi/block-core.json | 8 
>  3 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 3/4] block: add a 'blockdev-snapshot' QMP command

[Eric Blake]

On 09/10/2015 07:39 AM, Alberto Garcia wrote:
> One of the limitations of the 'blockdev-snapshot-sync' command is that
> it does not allow passing BlockdevOptions to the newly created
> snapshots, so they are always opened using the default values.
> 
> Extending the command to allow passing options is not a practical
> solution because there is overlap between those options and some of
> the existing parameters of the command.
> 
> This patch introduces a new 'blockdev-snapshot' command with a simpler
> interface: it just takes two references to existing block devices that
> will be used as the source and target for the snapshot.
> 
> Since the main difference between the two commands is that one of them
> creates and opens the target image, while the other uses an already
> opened one, the bulk of the implementation is shared.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  blockdev.c   | 163 
> ---
>  qapi-schema.json |   2 +
>  qapi/block-core.json |  26 
>  qmp-commands.hx  |  29 +
>  4 files changed, 160 insertions(+), 60 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 6b787c1..78cfb79 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1183,6 +1183,18 @@ void qmp_blockdev_snapshot_sync(bool has_device, const 
> char *device,
> , errp);
>  }
>  
> +void qmp_blockdev_snapshot(const char *device, const char *snapshot,
> +   Error **errp)
> +{
> +BlockdevSnapshot snapshot_data = {
> +.device = (char *) device,
> +.snapshot = (char *) snapshot
> +};

Hmm. Sounds like you'd love to use my pending 'box':true qapi glue to
make this function have the saner signature of

void qmp_blockdev_snapshot(BlockdevSnapshot *arg, Error **errp);

rather than having to rebuild the struct yourself (with an annoying cast
to lose const) :)
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02599.html

But no need to hold this series up waiting for the qapi review queue to
flush. We can simplify later.

>  
> -options = qdict_new();
> -if (has_snapshot_node_name) {
> -qdict_put(options, "node-name",
> -  qstring_from_str(snapshot_node_name));
> +if (snapshot_node_name && bdrv_find_node(snapshot_node_name)) {
> +error_setg(errp, "New snapshot node name already existing");

Pre-existing, but s/existing/exists/ while you are reindenting this.


> +++ b/qapi/block-core.json
> @@ -705,6 +705,19 @@
>  '*format': 'str', '*mode': 'NewImageMode' } }
>  
>  ##
> +# @BlockdevSnapshot
> +#
> +# @device: device or node name to generate the snapshot from.
> +#
> +# @snapshot: reference to the existing block device that will be used
> +#for the snapshot.

Maybe mention that it must NOT have a current backing file, and point to
the "backing":"" trick to get it that way.

> +Create a snapshot, by installing 'device' as the backing image of
> +'snapshot'. Additionally, if 'device' is associated with a block
> +device, the block device changes to using 'snapshot' as its new active
> +image.

Still didn't answer the question from the earlier review of whether the
blockdev-snapshot-sync behavior of specifying the node name of an active
layer in order to not pivot the block device to the snapshot still makes
sense to support in the blockdev-snapshot case.  But we could always add
an optional boolean flag later if someone comes up for a use case where
they'd need to create a snapshot of the active layer without the block
device pivoting, so I don't think it should hold up this patch.

> +
> +Arguments:
> +
> +- "device": snapshot source (json-string)
> +- "snapshot": snapshot target (json-string)
> +
> +Example:
> +
> +-> { "execute": "blockdev-snapshot", "arguments": { "device": "ide-hd0",
> +"snapshot": "node1534" } 
> }
> +<- { "return": {} }

Maybe enhance the example to show the preliminary blockdev-add that
created node1534?

I've pointed out some potential wording improvements, but think they are
minor enough that you can have:

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 v5 05/14] blockjob: Introduce reference count

[Max Reitz]

On 07.09.2015 09:34, Fam Zheng wrote:
> So that block_job_complete_sync can be simplified.
> 
> Signed-off-by: Fam Zheng 
> ---
>  block/mirror.c   |  2 +-
>  blockjob.c   | 22 ++
>  include/block/blockjob.h | 18 +++---
>  3 files changed, 30 insertions(+), 12 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 09/14] block: Add block job transactions

[Max Reitz]

On 07.09.2015 09:34, Fam Zheng wrote:
> Sometimes block jobs must execute as a transaction group.  Finishing
> jobs wait until all other jobs are ready to complete successfully.
> Failure or cancellation of one job cancels the other jobs in the group.
> 
> Signed-off-by: Stefan Hajnoczi 
> [Rewrite the implementation which is now contained in block_job_completed.
> --Fam]
> Signed-off-by: Fam Zheng 
> ---
>  blockjob.c   | 134 
> ++-
>  include/block/block.h|   1 +
>  include/block/blockjob.h |  38 ++
>  3 files changed, 171 insertions(+), 2 deletions(-)
> 

[snip]

> diff --git a/include/block/block.h b/include/block/block.h
> index e619b99..f80d984 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h

[snip]

> @@ -395,4 +398,39 @@ void block_job_defer_to_main_loop(BlockJob *job,
>BlockJobDeferToMainLoopFn *fn,
>void *opaque);
>  
> +/**
> + * block_job_txn_new:
> + *
> + * Allocate and return a new block job transaction.  Jobs can be added to the
> + * transaction using block_job_txn_add_job().
> + *
> + * The transaction is automatically freed when the last job completes or is
> + * cancelled.
> + *
> + * All jobs in the transaction either complete successfully or fail/cancel 
> as a
> + * group.  Jobs wait for each other before completing.  Cancelling one job
> + * cancels all jobs in the transaction.
> + */
> +BlockJobTxn *block_job_txn_new(void);
> +
> +/**
> + * block_job_txn_unref:
> + *
> + * Release a reference that was previously acquired with 
> block_job_txn_add_job
> + * or block_job_txn_new. If it's the last reference to the object, it will be
> + * freed.
> + */
> +void block_job_txn_unref(BlockJobTxn *txn);
> +
> +/**
> + * block_job_txn_add_job:
> + * @txn: The transaction (may be NULL)
> + * @job: Job to add to the transaction
> + *
> + * Add @job to the transaction.  The @job must not already be in a 
> transaction.
> + * The block job driver must call block_job_txn_prepare_to_complete() before
> + * final cleanup and completion.

This comment still needs a fixup (block_job_txn_prepare_to_complete()
does not exist).

Looks good other than that.

Max

> + */
> +void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job);
> +
>  #endif
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH v1 05/25] error: Add error prefix API

[Eric Blake]

On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> Add an API to prefix an already set error with a caller-centric
> message.
> 
> If multiple errors are set, all are prefixed individually.
> 

Might be nice to rebase your series to add this first, prior to
multi-error support.  (That is, while prefixing multiple errors requires
multi-error support, adding caller-centric prefix might be useful even
now without multi-error).

> Signed-off-by: Peter Crosthwaite 
> ---
> 
>  include/qapi/error.h |  6 ++
>  util/error.c | 26 ++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index f44c451..b25c72f 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -78,6 +78,12 @@ ErrorClass error_get_class(const Error *err);
>  Error *error_copy(const Error *err);
>  
>  /**
> + * Prefix an error message with a formatted string.
> + */
> +
> +void error_prefix(Error *err, const char *fmt, ...);

The string is basically only adding details for human consumption.

> +++ b/util/error.c
> @@ -19,6 +19,7 @@ struct Error
>  char *msg;
>  ErrorClass err_class;
>  struct Error *next;
> +bool prefixed;
>  };

I'm not sure I follow why this field is needed.

>  
>  Error *error_abort;
> @@ -142,6 +143,26 @@ const char *error_get_pretty(Error *err)
>  return err->msg;
>  }
>  
> +void error_prefix(Error *err, const char *fmt, ...) {

Brace on its own line.

> +char *msg;
> +char *fmt_full;
> +va_list ap;
> +
> +if (!err || err->prefixed) {
> +return;
> +}
> +err->prefixed = true;
> +
> +msg = err->msg;
> +fmt_full =  g_strdup_printf("%s%%s", fmt);
> +
> +va_start(ap, fmt);
> +err->msg = g_strdup_printf(fmt_full, ap, msg);

I don't think that is portable.  Passing va_list as an argument to plain
printf just isn't going to do what you think.
(There has been a request to add recursive printf via %r,
http://austingroupbugs.net/view.php?id=800, but glibc does not support
the extension yet)

It's not to say that you can't chain, just that you can't chain like
this.  I don't know if using GString internally to error would make the
matter any easier (but one of the patches that will probably land before
yours, and thus affect your need to rebase, is mine which adds use of
GString for adding a human-readable SUFFIX rather than prefix:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02818.html)

> +va_end(ap);
> +g_free(fmt_full);
> +g_free(msg);
> +}
> +
>  void error_report_err(Error *err)
>  {
>  error_report("%s", error_get_pretty(err));
> @@ -170,6 +191,11 @@ void error_propagate(Error **dst_errp, Error *local_err)
>  
>  *dst_errp = local_err;
>  for (i = local_err; i; i = i->next) {
> +/* Propagation implies that the caller is no longer the owner of 
> the
> + * error. Therefore reset prefixes, so higher level handlers can
> + * prefix again.
> + */
> +i->prefixed = false;

I'm still not quite sure I buy the semantics of this flag.

>  dst_errp = >next;
>  }
>  *dst_errp = old_dst_err;
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 3/4] softmmu: remove now unused functions

[Richard Henderson]

From: Pavel Dovgalyuk 

Now that the cpu_ld/st_* function directly call helper_ret_ld/st, we can
drop the old helper_ld/st functions.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Pavel Dovgalyuk 
Message-Id: <20150710095656.13280.7085.stgit@PASHA-ISP>
Signed-off-by: Richard Henderson 
---
 include/exec/cpu_ldst.h | 19 ---
 softmmu_template.h  | 16 
 2 files changed, 35 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index ceac168..b573df5 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -113,25 +113,6 @@
 /* The memory helpers for tcg-generated code need tcg_target_long etc.  */
 #include "tcg.h"
 
-uint8_t helper_ldb_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint16_t helper_ldw_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint32_t helper_ldl_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint64_t helper_ldq_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-
-void helper_stb_mmu(CPUArchState *env, target_ulong addr,
-uint8_t val, int mmu_idx);
-void helper_stw_mmu(CPUArchState *env, target_ulong addr,
-uint16_t val, int mmu_idx);
-void helper_stl_mmu(CPUArchState *env, target_ulong addr,
-uint32_t val, int mmu_idx);
-void helper_stq_mmu(CPUArchState *env, target_ulong addr,
-uint64_t val, int mmu_idx);
-
-uint8_t helper_ldb_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint16_t helper_ldw_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint32_t helper_ldl_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint64_t helper_ldq_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-
 #ifdef MMU_MODE0_SUFFIX
 #define CPU_MMU_INDEX 0
 #define MEMSUFFIX MMU_MODE0_SUFFIX
diff --git a/softmmu_template.h b/softmmu_template.h
index a071822..6803890 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -329,14 +329,6 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, 
target_ulong addr,
 }
 #endif /* DATA_SIZE > 1 */
 
-DATA_TYPE
-glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
- int mmu_idx)
-{
-TCGMemOpIdx oi = make_memop_idx(SHIFT, mmu_idx);
-return helper_te_ld_name (env, addr, oi, GETRA());
-}
-
 #ifndef SOFTMMU_CODE_ACCESS
 
 /* Provide signed versions of the load routines as well.  We can of course
@@ -534,14 +526,6 @@ void helper_be_st_name(CPUArchState *env, target_ulong 
addr, DATA_TYPE val,
 }
 #endif /* DATA_SIZE > 1 */
 
-void
-glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
- DATA_TYPE val, int mmu_idx)
-{
-TCGMemOpIdx oi = make_memop_idx(SHIFT, mmu_idx);
-helper_te_st_name(env, addr, val, oi, GETRA());
-}
-
 #if DATA_SIZE == 1
 /* Probe for whether the specified guest write access is permitted.
  * If it is not permitted then an exception will be taken in the same
-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 2/4] block: Add 'ignore-backing' field to BlockdevOptionsGenericCOWFormat

[Max Reitz]

On 11.09.2015 19:28, Kevin Wolf wrote:
> Am 11.09.2015 um 19:21 hat Max Reitz geschrieben:
>> On 10.09.2015 15:39, Alberto Garcia wrote:
>>> If set to true, the image will be opened with the BDRV_O_NO_BACKING
>>> flag. This is useful for creating snapshots using images opened with
>>> blockdev-add, since they are not supposed to have a backing image
>>> before the operation.
>>>
>>> Signed-off-by: Alberto Garcia 
>>> ---
>>>  block.c  | 5 +
>>>  qapi/block-core.json | 6 +-
>>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> Ignorant of any possible previous discussion that might have taken
>> place: The documentation for @backing says it may be set to the empty
>> string in order to achieve exactly that.
>>
>> So why do we need the new flag? Because "backing: ''" is ugly?
> 
> I guess it's just because you're the only one who actually reads the
> documentation. When discussing this, I didn't remember that we already
> had a way to express this (an additional bool wouldn't have been my
> favourite solution anyway). Thanks for catching this.

I read the patch, it was part of the context. ;-)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 4/4] block: add tests for the 'blockdev-snapshot' command

[Eric Blake]

On 09/10/2015 07:39 AM, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia 
> ---
>  tests/qemu-iotests/085 | 97 
> +++---
>  tests/qemu-iotests/085.out | 34 +++-
>  2 files changed, 123 insertions(+), 8 deletions(-)
> 

>  
> +# ${1}: unique identifier for the snapshot filename
> +# ${2}: true: ignore backing images (default); false: don't ignore them
> +function add_snapshot_image()
> +{
> +base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
> +snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
> +_make_test_img -b "${base_image}" "$size"
> +mv "${TEST_IMG}" "${snapshot_file}"
> +cmd="{ 'execute': 'blockdev-add', 'arguments':
> +   { 'options':
> + { 'driver': 'qcow2', 'node-name': 'snap_"${1}"',
> +   'ignore-backing': "${2:-true}", 'file':

Needs to be reworked to use 'backing':'' instead of 'ignore-backing'.

But overall looks like a sane set of tests to cover both positive and
negative expected behavior.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 0/4] Queued tcg related patches

[Richard Henderson]

Cherry pick some reviewed patches that touch generic TCG.

Hopefully this unsticks both patch sets, since they now touch only
target-* code, which can be independently reviewed and committed.


r~


The following changes since commit ba9cef7b6e487a5a8969db81d09b8eec8a2b50c6:

  Merge remote-tracking branch 
'remotes/mjt/tags/pull-trivial-patches-2015-09-11' into staging (2015-09-11 
12:07:29 +0100)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20150911

for you to fetch changes up to 1c3c8af1fb40a481c07749e0448644d9b7700415:

  cpu-exec: introduce loop exit with restore function (2015-09-11 08:16:16 
-0700)


queued tcg related patches


Benjamin Herrenschmidt (1):
  tlb: Add "ifetch" argument to cpu_mmu_index()

Pavel Dovgalyuk (3):
  softmmu: add helper function to pass through retaddr
  softmmu: remove now unused functions
  cpu-exec: introduce loop exit with restore function

 cpu-exec.c|  9 +
 cputlb.c  |  2 +-
 include/exec/cpu_ldst.h   | 23 ++--
 include/exec/cpu_ldst_template.h  | 59 ++-
 include/exec/cpu_ldst_useronly_template.h | 25 +
 include/exec/exec-all.h   |  1 +
 softmmu_template.h| 22 
 target-alpha/cpu.h|  2 +-
 target-alpha/translate.c  |  2 +-
 target-arm/cpu.h  |  4 +--
 target-arm/helper.c   |  4 +--
 target-cris/cpu.h |  2 +-
 target-cris/translate.c   |  6 ++--
 target-cris/translate_v10.c   |  2 +-
 target-i386/cpu.h |  2 +-
 target-i386/translate.c   |  2 +-
 target-lm32/cpu.h |  2 +-
 target-m68k/cpu.h |  2 +-
 target-microblaze/cpu.h   |  2 +-
 target-microblaze/mmu.c   |  2 +-
 target-microblaze/translate.c | 16 -
 target-mips/cpu.h |  2 +-
 target-mips/op_helper.c   |  4 +--
 target-moxie/cpu.h|  2 +-
 target-openrisc/cpu.h |  2 +-
 target-openrisc/translate.c   |  2 +-
 target-ppc/cpu.h  |  2 +-
 target-s390x/cpu.h|  2 +-
 target-s390x/mem_helper.c |  4 +--
 target-sh4/cpu.h  |  2 +-
 target-sparc/cpu.h|  2 +-
 target-sparc/mmu_helper.c |  2 +-
 target-sparc/translate.c  |  2 +-
 target-tricore/cpu.h  |  2 +-
 target-tricore/translate.c|  2 +-
 target-unicore32/cpu.h|  2 +-
 target-xtensa/cpu.h   |  2 +-
 tcg/tcg.h | 23 
 38 files changed, 155 insertions(+), 95 deletions(-)

Re: [Qemu-devel] [PATCH v2 0/4] scripts/qemu-gdb: Split into modules

[Peter Maydell]

On 2 September 2015 at 15:05, Stefan Hajnoczi  wrote:
> On Fri, Aug 14, 2015 at 06:46:28PM +0100, Peter Maydell wrote:
>> This patch series splits scripts/qemu-gdb into separate
>> files for each command, to make it easier to add new
>> commands in future without them all going into a single
>> huge file.
>>
>> Patches 1 and 2 do that split, and have been on the list
>> before (and reviewed by Stefan).
>>
>> Patch 3 makes the script tell gdb not to stop on SIGUSR1,
>> since that's our SIGIPI and happens all the time.
>>
>> Patch 4 adds a brief comment explaining how to source the
>> script. NB that I haven't attempted to make the script
>> work with automatic-sourcing based on the executable
>> being debugged. Somebody who uses the script that way
>> can do that :-)
>>
>> I've dropped the script which added support for setting
>> breakpoints on QEMU tracepoints for the moment (pending
>> investigating static probe point support which might
>> render it unnecessary). Mostly I'd like to get these
>> into master so that future debug commands can go into
>> the right place -- IIRC David Gilbert had a patchset
>> which added another command.
>>
>> thanks
>> -- PMM
>>
>> Peter Maydell (4):
>>   scripts/qemu-gdb: Split MtreeCommand into its own module
>>   scripts/qemu-gdb: Split CoroutineCommand into its own file
>>   scripts/qemu-gdb: Silently pass through SIGUSR1
>>   scripts/qemu-gdb: Add brief comment describing usage
>>
>>  scripts/qemu-gdb.py  | 146 
>> ---
>>  scripts/qemugdb/__init__.py  |  28 +
>>  scripts/qemugdb/coroutine.py |  91 +++
>>  scripts/qemugdb/mtree.py |  82 
>>  4 files changed, 212 insertions(+), 135 deletions(-)
>>  create mode 100644 scripts/qemugdb/__init__.py
>>  create mode 100644 scripts/qemugdb/coroutine.py
>>  create mode 100644 scripts/qemugdb/mtree.py
>>
>> --
>> 1.9.1
>>
>
> Reviewed-by: Stefan Hajnoczi 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v3 2/4] block: Add 'ignore-backing' field to BlockdevOptionsGenericCOWFormat

[Eric Blake]

On 09/10/2015 07:39 AM, Alberto Garcia wrote:
> If set to true, the image will be opened with the BDRV_O_NO_BACKING
> flag. This is useful for creating snapshots using images opened with
> blockdev-add, since they are not supposed to have a backing image
> before the operation.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block.c  | 5 +
>  qapi/block-core.json | 6 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)

> 
> diff --git a/block.c b/block.c
> index 22d3b0e..4be32fb 100644
> --- a/block.c
> +++ b/block.c
> @@ -1469,6 +1469,11 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
> const char *filename,
>  
>  assert(drvname || !(flags & BDRV_O_PROTOCOL));
>  
> +if (qdict_get_try_bool(options, "ignore-backing", false)) {
> +flags |= BDRV_O_NO_BACKING;
> +}
> +qdict_del(options, "ignore-backing");

What happens if the user specified "ignore-backing":true, "backing":...?
 Should that be a hard error?

>  { 'struct': 'BlockdevOptionsGenericCOWFormat',
>'base': 'BlockdevOptionsGenericFormat',
> -  'data': { '*backing': 'BlockdevRef' } }
> +  'data': { '*backing': 'BlockdevRef',
> +'*ignore-backing': 'bool' } }

Depending on whether the answer to my question is that we already behave
sanely and don't leave a BlockdevRef dangling if the caller mixes the
two approaches, then:
Reviewed-by: Eric Blake 

But design-wise, would it make sense to support:

"backing":null

as an explicit request to not open a backing file?  Right now, qapi does
not have a way to express 'null' as part of an alternate type; but if it
did, BlockdevRef would merely add 'null' as one of its allowed
alternates.  Then we wouldn't need ignore-backing from the QMP
perspective. But I'm still not sure how it would map to the command line
perspective.

-- 
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 v5 04/14] backup: Extract dirty bitmap handling as a separate function

[Max Reitz]

On 07.09.2015 09:34, Fam Zheng wrote:
> This will be reused by the coming new transactional completion code.
> 
> Signed-off-by: Fam Zheng 
> Reviewed-by: John Snow 
> ---
>  block/backup.c | 26 --
>  1 file changed, 16 insertions(+), 10 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH] spapr: Reduce creation of LMB DR connectors from O(n^3) to O(n^2)

[Bharata B Rao]

On Thu, Sep 10, 2015 at 04:28:25PM +1000, David Gibson wrote:
> The dynamic reconfiguration (hotplug) code for the pseries machine type
> uses a "DR connector" QOM object for each resource it will be possible
> to hotplug.  Each of these is added to its owner using
> object_property_add_child(owner, "dr-connector[*], ...);
> 
> This works ok for most cases, but gets ugly when allowing large amounts of
> hotplugged RAM.  For RAM, there's a DR connector object for every 256MB of
> potential memory.  So if maxmem=2T, for example, there are >250,000 objects
> under the same parent.

There is one LMB DRC object for every 256MB, so with 2T maxmem, there will be
max 8192 LMB DRC objects.

> 
> The QOM interfaces aren't really designed for this.  In particular
> object_property_add() has O(n^2) time complexity (in the number of existing
> children) for the [*] case.  First it has a linear search through array
> indices to find a free slot, each of which is attempted to a recursive call
> to object_property_add() with a specific [N].  Those calls are O(n) because
> there's a linear search through all properties to check for duplicates.
> 
> For the specific case of DR connectors, we already have a sufficiently
> unique index, so we don't need to use the [*] special behaviour.  That lets
> us reduce the total time for creating the DR objects from O(n^3) to O(n^2).
> 
> O(n^2) is still kind of crappy, but it's enough to reduce the startup time
> of qemu with maxmem=2T from ~20 minutes to ~4 seconds.
> 
> Signed-off-by: David Gibson 
> Cc: Bharata B Rao 
> ---
>  hw/ppc/spapr_drc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index c1f664f..4cf3a9b 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -463,14 +463,16 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
>  {
>  sPAPRDRConnector *drc =
>  SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
> +char *prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", id);

This works only if memory hotplug alone is present. If CPU hotplug is also
present, the lookup of DRC object for LMB DRC fails from ibm,cas call when
the guest is booting.

I don't fully understand why it fails, but the object lookup doesn't seem to
like duplicate names that we end up having here. With the above change, we
can have duplicate prop_name under the same owner object (spapr machine
object) due to both CPU and LMB DRC objects coming under the same parent.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium

[Max Reitz]

On 11.09.2015 09:30, Wen Congyang wrote:
> On 09/11/2015 03:09 AM, Max Reitz wrote:
>> On 10.09.2015 03:12, Wen Congyang wrote:
>>> On 09/09/2015 08:59 PM, Max Reitz wrote:
 On 09.09.2015 12:01, Wen Congyang wrote:
> On 09/09/2015 05:20 AM, Max Reitz wrote:
>> On 08.09.2015 11:13, Wen Congyang wrote:
>>> On 07/21/2015 01:45 AM, Max Reitz wrote:
 And a helper function for that, which directly takes a pointer to the
 BDS to be inserted instead of its node-name (which will be used for
 implementing 'change' using blockdev-insert-medium).

 Signed-off-by: Max Reitz 
 ---
  blockdev.c   | 48 
 
  qapi/block-core.json | 17 +
  qmp-commands.hx  | 37 +
  3 files changed, 102 insertions(+)

 diff --git a/blockdev.c b/blockdev.c
 index 481760a..a80d0e2 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char 
 *device, Error **errp)
  }
  }
  
 +static void qmp_blockdev_insert_anon_medium(const char *device,
 +BlockDriverState *bs, 
 Error **errp)
 +{
 +BlockBackend *blk;
 +bool has_device;
 +
 +blk = blk_by_name(device);
 +if (!blk) {
 +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
 +  "Device '%s' not found", device);
 +return;
 +}
 +
 +/* For BBs without a device, we can exchange the BDS tree at will 
 */
 +has_device = blk_get_attached_dev(blk);
 +
 +if (has_device && !blk_dev_has_removable_media(blk)) {
 +error_setg(errp, "Device '%s' is not removable", device);
 +return;
 +}
 +
 +if (has_device && !blk_dev_is_tray_open(blk)) {
 +error_setg(errp, "Tray of device '%s' is not open", device);
 +return;
 +}
 +
 +if (blk_bs(blk)) {
 +error_setg(errp, "There already is a medium in device '%s'", 
 device);
 +return;
 +}
 +
 +blk_insert_bs(blk, bs);
 +}
 +
 +void qmp_blockdev_insert_medium(const char *device, const char 
 *node_name,
 +Error **errp)
 +{
 +BlockDriverState *bs;
 +
 +bs = bdrv_find_node(node_name);
 +if (!bs) {
 +error_setg(errp, "Node '%s' not found", node_name);
 +return;
 +}
>>>
>>> Hmm, it is OK if the bs is not top BDS?
>>
>> I think so, yes. Generally, there's probably no reason to do that, but I
>> don't know why we should not allow that case. For instance, you might
>> want to make a backing file available read-only somewhere.
>>
>> It should be impossible to make it available writable, and it should not
>> be allowed to start a block-commit operation while the backing file can
>> be accessed by the guest, but this should be achieved using op blockers.
>>
>> What we need for this to work are fine-grained op blockers, I think. But
>> working around that for now by only allowing to insert top BDS won't
>> work, since you can still start block jobs which target top BDS, too
>> (e.g. blockdev-backup can write to a BDS/BB that is visible to the 
>> guest).
>>
>> All in all, I think it's fine to insert non-top BDS, but we should
>> definitely worry about which exact BDS one can insert once we have
>> fine-grained op blockers.
>
> A BDS can be written by its parent, its block backend, a block job..
> So I think we should have some way to avoid more than two sources writing
> to it, otherwise the data may be corrupted.

 Yes, and that would be op blockers.

 As I said, using blockdev-backup you can write to a BB that can be
 written to by the guest as well. I think this is a bug, but it is a bug
 that needs to be fixed by having better op blockers in place, which Jeff
 Cody is working on.

 Regarding this series, I don't consider this to be too big of an issue.
 Yes, if you are working with floppy disks, you can have the case of a
 block job and the guest writing to the BDS at the same time. But I can't
 really imagine who would use floppy disks and block jobs at the same
 time (people who still use floppy disks for their VMs don't strike me as
 the kind of people who use the management features of qemu, especially
 not for those floppy disks).

 Other than 

Re: [Qemu-devel] [PATCH v3 2/4] block: Add 'ignore-backing' field to BlockdevOptionsGenericCOWFormat

[Max Reitz]

On 10.09.2015 15:39, Alberto Garcia wrote:
> If set to true, the image will be opened with the BDRV_O_NO_BACKING
> flag. This is useful for creating snapshots using images opened with
> blockdev-add, since they are not supposed to have a backing image
> before the operation.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block.c  | 5 +
>  qapi/block-core.json | 6 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)

Ignorant of any possible previous discussion that might have taken
place: The documentation for @backing says it may be set to the empty
string in order to achieve exactly that.

So why do we need the new flag? Because "backing: ''" is ugly?

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/4] block: Add 'ignore-backing' field to BlockdevOptionsGenericCOWFormat

[Kevin Wolf]

Am 11.09.2015 um 19:21 hat Max Reitz geschrieben:
> On 10.09.2015 15:39, Alberto Garcia wrote:
> > If set to true, the image will be opened with the BDRV_O_NO_BACKING
> > flag. This is useful for creating snapshots using images opened with
> > blockdev-add, since they are not supposed to have a backing image
> > before the operation.
> > 
> > Signed-off-by: Alberto Garcia 
> > ---
> >  block.c  | 5 +
> >  qapi/block-core.json | 6 +-
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> Ignorant of any possible previous discussion that might have taken
> place: The documentation for @backing says it may be set to the empty
> string in order to achieve exactly that.
> 
> So why do we need the new flag? Because "backing: ''" is ugly?

I guess it's just because you're the only one who actually reads the
documentation. When discussing this, I didn't remember that we already
had a way to express this (an additional bool wouldn't have been my
favourite solution anyway). Thanks for catching this.

Kevin


pgpOUwI1Idwpg.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH] specs/vhost-user.txt: fix name of messages

[Michael Tokarev]

08.09.2015 23:29, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  docs/specs/vhost-user.txt | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 0322bcf..1bc6adb 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -112,19 +112,19 @@ The communication consists of master sending message 
> requests and slave sending
>  message replies. Most of the requests don't require replies. Here is a list 
> of
>  the ones that do:
>  
> - * VHOST_GET_FEATURES
> - * VHOST_GET_PROTOCOL_FEATURES
> - * VHOST_GET_VRING_BASE
> + * VHOST_USER_GET_FEATURES
> + * VHOST_USER_GET_PROTOCOL_FEATURES
> + * VHOST_USER_GET_VRING_BASE

The patch doesn't apply to qemu master as vhost-user.txt does not
have VHOST_GET_PROTOCOL_FEATURES in here.  What treee you're basing
your patch on?

Besides, this is quite confusing.  Both VHOST_USER_GET_FEATURES and
VHOST_GET_FEATURES #defines exists and used, which one is which?

Thanks,

/mjt

Re: [Qemu-devel] [PATCH v1 00/15] Multi-Arch Phase 1

[Paolo Bonzini]

On 11/09/2015 07:39, Peter Crosthwaite wrote:
> This is the first set of patches needed to enable Multi-arch system
> emulation. For full context refer to RFCv3:
> 
> [PATCH v3 00/35] Multi Architecture System Emulation
> https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg03929.html
> 
> This is the first patch-pack intended for merge.
> 
> Original cover, as well as overall series state below for further
> information.

I think we can already merge patches 1, 3, 4, 5, 6, 11, 12, 13, 15
(plus, patch 10 is gone). The others do not make much sense without
multiarch support.

I suppose the next part of the surgery could be

  target-*: Don't redefine cpu_exec()
  target-*: cpu.h: Undefine core code symbols
  arm: cpu: static inline cpu_arm_init()
  target-arm: Split cp helper API to new C file
  hw: arm: Explicitly include cpu.h for consumers
  hw: mb: Explicitly include cpu.h for consumers

Paolo

> 
> Regards,
> Peter
> 
> Original Multi-arch arch patch series cover:
> 
> ***
> 
> This is target-multi, a system-mode build that can support multiple
> cpu-types.
> 
> Two architectures are initially converted. Microblaze and ARM. Step
> by step conversion in done for each. A microblaze is added to
> Xilinx Zynq platform as a test case. This will be elaborted more in
> future spins. This use case is valid, as Microblazes can be added (any
> number of them!) in Zynq FPGA programmable logic configuration.
> 
> The general approach (radically different to approach in V1 RFC) is to build
> and prelink an object (arch-obj.o) per-arch containing:
> 
> 1: target-foo/*
> 2: All uses of env internals and CPU_GET_ENV
> * cputlb, translate-all, cpu-exec
> * TCG backend
> 
> This means cputlb and friends are compiled multiple times fo each arch. The
> symbols for each of these pre-links are then localised to avoid link time name
> collisions. This is based on Paolo's suggestion to templatify cputlb and
> friends. Just the net of what to multi-compile is widened to include the TCG
> stuff as well now.
> 
> Despite being some "major surgery" this approach actually solves many of big
> the problems raised in V1. Big problems sovled:
> 
> 1: With the multi-compile TCG backends there are now multiple tcg_ctx's for
> each architecture. This solves the issue PMM raised WRT false positives on TB
> hashing as archs no longer share translation context.
> 
> 2: There is no longer a need to reorder the CPU_COMMON within the ENV or the 
> ENV
> within the CPU. This was flagged as a performance issue by multiple people in
> V1.
> All users of the env internals as well as ENV_GET_CPU are now in multi-compile
> code and so multi-arch does not need to define a generic ENV nor does in need 
> to
> def the problematic ENV_GET_CPU.
> 
> 3: With the prelink symbol localisation, link time namespace collision of
> helpers from multiple arches is no longer an issue. No need to bloat all the
> function names with arch specific prefixes.
> 
> 4: The architecture specifics used/defined by cpu-defs can now vary from arch 
> to
> arch (incl. target_ulong) greatly reducing coversion effort needed. The list
> of restrictions for multi-arch capability is much reduced since V1. No
> target_long issues anymore.
> 
> include/exec/*.h and some of the common code needs some refactoring to setup
> this single vs multi compile split. Mostly code movements.
> 
> Some functions (like tcg_enabled) need to be listified for each of the
> now-multiple TCG engines.
> 
> The interface between the multi compile and single compiled files needs to be
> virtualised using QOM cpu functions. But this is now a very low footprint
> change as most of the virtualised hooks are now in mutli-compiled code (they
> only exist as text once). There are more new hooks than before, but the per
> target change pattern is reduced.
> 
> For the implementation of the series, the trickiest part is (still) cpu.h
> inclusion management. There are now more than one cpu.h's and different
> parts of the tree need a different include scheme. target-multi defines
> it's own cpu.h which is bare minimum defs as needed by core code only.
> target-foo/cpu.h are mostly the same but refactored to avoid collisions
> with other cpu.h's. Inclusion scheme goes something like
> this (for the multi-arch build):
> 
> *: Core code includes only target-multi/cpu.h
> *: target-foo/ implementation code includes target-foo/cpu.h locally
> *: System level code (e.g. mach models) can use multiple target-foo/cpu.h's
> 
> The hardest unasnwered Q is (still) what to do about bootloading. Currently
> each arch has it's own architecture specific bootloading which may assume a
> single architecture. I have applied some hacks to at least get this
> RFC testable using a -kernel -firmware split but going forward being
> able to associate an elf/image with a cpu explictitly needs to be
> solved.
> 
> No support for KVM, im not sure if a mix of TCG and KVM is supported even for
> a single arch? (which would be 

Re: [Qemu-devel] [PATCH 2/6] hw/virtio/virtio-balloon: Remove meaningless blank Property

[Michael Tokarev]

12.05.2015 05:25, shannon.z...@linaro.org wrote:
> From: Shannon Zhao 
> 
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> ---
>  hw/virtio/virtio-balloon.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 484c3c3..a3f5709 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -417,16 +417,11 @@ static void virtio_balloon_device_unrealize(DeviceState 
> *dev, Error **errp)
>  virtio_cleanup(vdev);
>  }
>  
> -static Property virtio_balloon_properties[] = {
> -DEFINE_PROP_END_OF_LIST(),
> -};

Current balloon properties list isnt' empty, so this patch isn't needed anymore.

/mjt

Re: [Qemu-devel] [PATCH RFC v5 30/32] qapi: New QMP command query-qmp-schema for QMP introspection

[Daniel P. Berrange]

On Fri, Sep 11, 2015 at 09:02:15AM +0200, Markus Armbruster wrote:
> Michael Roth  writes:
> 
> > Quoting Markus Armbruster (2015-09-07 05:16:41)
> >> A new test-qmp-input-visitor test case feeds its result for both
> >> tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
> >> QmpInputVisitor to verify it actually conforms to the schema.
> >> 
> >> New QMP command query-qmp-schema takes its return value from that
> >> variable.  Its reply is some 85KiBytes for me right now.
> >> 
> >> If this turns out to be too much, we have a couple of options:
> >> 
> >> * We can use shorter names in the JSON.  Not the QMP style.
> >> 
> >> * Optionally return the sub-schema for commands and events given as
> >>   arguments.
> >> 
> >>   Right now qmp_query_schema() sends the string literal computed by
> >>   qmp-introspect.py.  To compute sub-schema at run time, we'd have to
> >>   duplicate parts of qapi-introspect.py in C.  Unattractive.
> >> 
> >> * Let clients cache the output of query-qmp-schema.
> >> 
> >>   It changes only on QEMU upgrades, i.e. rarely.  Provide a command
> >>   query-qmp-schema-hash.  Clients can have a cache indexed by hash,
> >>   and re-query the schema only when they don't have it cached.  Even
> >>   simpler: put the hash in the QMP greeting.
> >
> > Would probably be easier for management to build up their own structure
> > for querying caps, so I think a computed hash seems best. But I don't
> > think either is something that couldn't be added later if need be.
> 
> I also think a hash is the way to go, and I'd like to provide one early,
> but I don't want to delay this series for it.

FWIW, libvirt already caches the results of its query of QEMU and will
only re-query QEMU if the timestamp of the QEMU binary on disk has
changed, or if libvirt itself has changed.  Also when querying the QEMU
capabilities there are quite a few commands we run, we want to cache
the full set, not just query-qmp-schema, so we can avoid launching QEMU
at all. o at least from Libvirt's POV, I don't think we have a need for
a hash of query-qmp-schema.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v1 10/15] exec-all: Move cpu_can_do_io() to qom/cpu.h

[Paolo Bonzini]

On 11/09/2015 07:39, Peter Crosthwaite wrote:
> This function has no architecture specific dependencies and should be
> callable from core code.

It also does not exist anymore. :-P

Paolo

Re: [Qemu-devel] [PATCH] imx_serial: Generate interrupt on tx empty if enabled

[Michael Tokarev]

Can we please have some r-b or ACK for this? :)

20.08.2015 18:52, Guenter Roeck wrote:
> Generate an interrupt if the tx buffer is empty and the tx empty interrupt
> is enabled. This fixes a problem seen when running a Linux image since
> Linux commit 55c3cb1358e ("serial: imx: remove unneeded imx_transmit_buffer()
> from imx_start_tx()"). Linux now waits for the tx empty interrupt before
> starting to send data, causing transmit stalls until there is an interrupt
> for another reason.
> 
> Signed-off-by: Guenter Roeck 
> ---
>  hw/char/imx_serial.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
> index f3fbc77..8dc791d 100644
> --- a/hw/char/imx_serial.c
> +++ b/hw/char/imx_serial.c
> @@ -145,7 +145,9 @@ static void imx_update(IMXSerialState *s)
>  uint32_t flags;
>  
>  flags = (s->usr1 & s->ucr1) & (USR1_TRDY|USR1_RRDY);
> -if (!(s->ucr1 & UCR1_TXMPTYEN)) {
> +if (s->ucr1 & UCR1_TXMPTYEN) {
> +flags |= (s->uts1 & UTS1_TXEMPTY);
> +} else {
>  flags &= ~USR1_TRDY;
>  }
>  
> 

Re: [Qemu-devel] [PATCH v1 14/15] cpu-common: Define tb_page_addr_t for everyone

[Paolo Bonzini]

On 11/09/2015 07:39, Peter Crosthwaite wrote:
> So this is the same as in RFCv2 and the comment there was that it will not
> work for linux-user mode WRT to qom/cpu.h fn prototypes needed this def.
> But the solution there is instead to conditionalise the def of those hooks on
> NEED_CPU_H. Then this patch is just needed for some misc. system-mode-only
> core code usages.

Sounds good, please squash that in this patch.

Paolo

Re: [Qemu-devel] [Qemu-block] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Alberto Garcia]

On Fri 11 Sep 2015 11:14:33 AM CEST, Fam Zheng  wrote:

>> > Another advantage for bdrv_aio_poll() is, in main loop we will not
>> > need a separate AioContext in changes like:
>> > 
>> > http://patchwork.ozlabs.org/patch/514968/
>> > 
>> > Because nested aio_poll will automatically be limited to only
>> > process block layer events. My idea is to eventually let main loop
>> > use aio_poll
>> 
>> That would be a step back.  Using GSource is useful because it lets
>> you integrate libraries such as GTK+.
>
> Can we move GTK to a separate GSource thread?

I think that GTK should always run in the main thread, or at least the
one running the default main loop / GMainContext.

Berto

Re: [Qemu-devel] typofixes

[Michael Tokarev]

08.09.2015 11:26, Peter Maydell wrote:
> On 7 September 2015 at 22:45, Veres Lajos  wrote:
>> Hi,
>>
>> Please find attached a couple of typo fixes, mostly in comments/docs.
>> (Please cc me directly, I am not on the list.)
> 
> That patch includes at least one change to linux-headers/,
> which is definitely wrong. Typos in the kernel headers need
> to be sent to the kernel list, and we will get them next time
> we do a header sync. It also has a change to disas/libvixl,
> which is also a bit pointless as that is an external project
> which we sync with. Further, it mixes changes to comments and
> docs (totally safe) with changes to code function names etc
> (needs closer review), and it's pretty huge so it's hard to
> review the latter very easily.
> 
> MJT: can you drop this patch from the trivial queue, please?

Dropped now :)  I apologize for not seeing the obvious problems
you and others found, this is the result of me being too relaxed
after returning from vacation :)

Thanks,

/mjt

Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium

[Wen Congyang]

On 09/11/2015 03:09 AM, Max Reitz wrote:
> On 10.09.2015 03:12, Wen Congyang wrote:
>> On 09/09/2015 08:59 PM, Max Reitz wrote:
>>> On 09.09.2015 12:01, Wen Congyang wrote:
 On 09/09/2015 05:20 AM, Max Reitz wrote:
> On 08.09.2015 11:13, Wen Congyang wrote:
>> On 07/21/2015 01:45 AM, Max Reitz wrote:
>>> And a helper function for that, which directly takes a pointer to the
>>> BDS to be inserted instead of its node-name (which will be used for
>>> implementing 'change' using blockdev-insert-medium).
>>>
>>> Signed-off-by: Max Reitz 
>>> ---
>>>  blockdev.c   | 48 
>>> 
>>>  qapi/block-core.json | 17 +
>>>  qmp-commands.hx  | 37 +
>>>  3 files changed, 102 insertions(+)
>>>
>>> diff --git a/blockdev.c b/blockdev.c
>>> index 481760a..a80d0e2 100644
>>> --- a/blockdev.c
>>> +++ b/blockdev.c
>>> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char 
>>> *device, Error **errp)
>>>  }
>>>  }
>>>  
>>> +static void qmp_blockdev_insert_anon_medium(const char *device,
>>> +BlockDriverState *bs, 
>>> Error **errp)
>>> +{
>>> +BlockBackend *blk;
>>> +bool has_device;
>>> +
>>> +blk = blk_by_name(device);
>>> +if (!blk) {
>>> +error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>>> +  "Device '%s' not found", device);
>>> +return;
>>> +}
>>> +
>>> +/* For BBs without a device, we can exchange the BDS tree at will 
>>> */
>>> +has_device = blk_get_attached_dev(blk);
>>> +
>>> +if (has_device && !blk_dev_has_removable_media(blk)) {
>>> +error_setg(errp, "Device '%s' is not removable", device);
>>> +return;
>>> +}
>>> +
>>> +if (has_device && !blk_dev_is_tray_open(blk)) {
>>> +error_setg(errp, "Tray of device '%s' is not open", device);
>>> +return;
>>> +}
>>> +
>>> +if (blk_bs(blk)) {
>>> +error_setg(errp, "There already is a medium in device '%s'", 
>>> device);
>>> +return;
>>> +}
>>> +
>>> +blk_insert_bs(blk, bs);
>>> +}
>>> +
>>> +void qmp_blockdev_insert_medium(const char *device, const char 
>>> *node_name,
>>> +Error **errp)
>>> +{
>>> +BlockDriverState *bs;
>>> +
>>> +bs = bdrv_find_node(node_name);
>>> +if (!bs) {
>>> +error_setg(errp, "Node '%s' not found", node_name);
>>> +return;
>>> +}
>>
>> Hmm, it is OK if the bs is not top BDS?
>
> I think so, yes. Generally, there's probably no reason to do that, but I
> don't know why we should not allow that case. For instance, you might
> want to make a backing file available read-only somewhere.
>
> It should be impossible to make it available writable, and it should not
> be allowed to start a block-commit operation while the backing file can
> be accessed by the guest, but this should be achieved using op blockers.
>
> What we need for this to work are fine-grained op blockers, I think. But
> working around that for now by only allowing to insert top BDS won't
> work, since you can still start block jobs which target top BDS, too
> (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest).
>
> All in all, I think it's fine to insert non-top BDS, but we should
> definitely worry about which exact BDS one can insert once we have
> fine-grained op blockers.

 A BDS can be written by its parent, its block backend, a block job..
 So I think we should have some way to avoid more than two sources writing
 to it, otherwise the data may be corrupted.
>>>
>>> Yes, and that would be op blockers.
>>>
>>> As I said, using blockdev-backup you can write to a BB that can be
>>> written to by the guest as well. I think this is a bug, but it is a bug
>>> that needs to be fixed by having better op blockers in place, which Jeff
>>> Cody is working on.
>>>
>>> Regarding this series, I don't consider this to be too big of an issue.
>>> Yes, if you are working with floppy disks, you can have the case of a
>>> block job and the guest writing to the BDS at the same time. But I can't
>>> really imagine who would use floppy disks and block jobs at the same
>>> time (people who still use floppy disks for their VMs don't strike me as
>>> the kind of people who use the management features of qemu, especially
>>> not for those floppy disks).
>>>
>>> Other than that, this function (blockdev-insert-medium) can only be used
>>> for optical ROM devices (I don't think we have CD/DVD-RW support, do
>>> we?), so it's much less of an 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/2] spapr: Add support for hwrng when available

[Thomas Huth]

On 11/09/15 02:45, David Gibson wrote:
> On Thu, Sep 10, 2015 at 02:03:39PM +0200, Thomas Huth wrote:
>> On 10/09/15 12:40, David Gibson wrote:
>>> On Thu, Sep 10, 2015 at 09:33:21AM +0200, Thomas Huth wrote:
 On 09/09/15 23:10, Thomas Huth wrote:
> On 08/09/15 07:15, David Gibson wrote:
 ...
>> At this point rather than just implementing them as discrete machine
>> options, I suspect it will be more maintainable to split out the
>> h-random implementation as a pseudo-device with its own qdev and so
>> forth.  We already do similarly for the RTAS time of day functions
>> (spapr-rtc).
>
> I gave that I try, but it does not work as expected. To be able to
> specify the options, I'd need to instantiate this device with the
> "-device" option, right? Something like:
>
>   -device spapr-rng,backend=rng0,usekvm=0
>
> Now this does not work when I use TYPE_SYS_BUS_DEVICE as parent class
> like it is done for spapr-rtc, since the user apparently can not plug
> device to this bus on machine spapr (you can also not plug an spapr-rtc
> device this way!).
>
> The spapr-vlan, spapr-vty, etc. devices are TYPE_VIO_SPAPR_DEVICE, so I
> also tried that instead, but then the rng device suddenly shows up under
> /vdevice in the device tree - that's also not what we want, I guess.

 I did some more tests, and I think I can get this working with one small
 modification to spapr_vio.c
>> ...
 i.e. when the dt_name has not been set, the device won't be added to the
 /vdevice device tree node. If that's acceptable, I'll continue with this
 approach.
>>>
>>> A bit hacky.
>>>
>>> I think it would be preferable to build it under SysBus by default,
>>> like spapr-rtc.  Properties can be set on the device using -global (or
>>> -set, but -global is easier).
>>
>> If anyhow possible, I'd prefere to use "-device" for this instead, because
>>
>> a) it's easier to use for the user, for example you can simply use
>>"-device spapr-rng,?" to get the list of properties - this
>>does not seem to work with spapr-rtc (it has a "date" property
>>which does not show up in the help text?)
> 
> Actually, I don't think that's got anything to do with -device versus
> otherwise.  "date" doesn't appear because it's an "object" property
> rather than a "qdev" property - that distinction is subtle and
> confusing, yes.

At least it is not very friendly for the user ... if a configuration
property does not show up in the help text, you've got to document it
somewhere else or nobody will be aware of it.

>> b) unlike the rtc device which is always instantiated, the rng
>>device is rather optional, so it is IMHO more intuitive if
>>created via the -device option.
> 
> Hrm, that's true though.  And.. we're back at the perrenial question
> of what "standard" devices should be constructed by default.  And what
> "default" means.
> 
> It seems to me that while the random device is optional, it should be
> created by default.  But with -device there's not really a way to do
> that.  But then again if it's constructed internally there's not
> really a way to turn it off short of hacky machine options.  Ugh.
> 
>> So I'd like to give it a try with the TYPE_VIO_SPAPR_DEVICE first ... if
>> you then still don't like the patches at all, I can still rework them to
>> use TYPE_SYS_BUS_DEVICE instead.
> 
> I still dislike putting it on the VIO "bus", since PAPR doesn't
> consider it a VIO device.

Hmm, that's also a valid point.

After doing some more research, I think I've found yet another
possibility (why isn't there a proper documentation/howto for all this
QOM stuff ... or did I just miss it?) :
Instead of using a bus, simply set parent = TYPE_DEVICE, so that it is a
"bus-less" device. Seems to work fine at a first glance, so unless
somebody tells me that this is a very bad idea, I'll try to rework my
patches accordingly...

 Thomas




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] virtio-net: unbreak self announcement and guest offloads after migration

[Cornelia Huck]

On Fri, 11 Sep 2015 16:01:56 +0800
Jason Wang  wrote:

> After commit 019a3edbb25f1571e876f8af1ce4c55412939e5d ("virtio: make
> features 64bit wide"). Device's guest_features was actually set after
> vdc->load(). This breaks the assumption that device specific load()
> function can check guest_features. For virtio-net, self announcement
> and guest offloads won't work after migration.
> 
> Fixing this by defer them to virtio_net_load() where guest_features
> were guaranteed to be set. Other virtio devices looks fine.
> 
> Fixes: 019a3edbb25f1571e876f8af1ce4c55412939e5d
>("virtio: make features 64bit wide")
> Cc: qemu-sta...@nongnu.org
> Cc: Gerd Hoffmann 
> Signed-off-by: Jason Wang 
> ---
>  hw/net/virtio-net.c | 40 +++-
>  1 file changed, 23 insertions(+), 17 deletions(-)

Migration support for virtio is really a twisty maze, it's easy to make
mistakes like that :(

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH] virtio-vga: enable for i386

[Michael Tokarev]

11.09.2015 12:13, Gerd Hoffmann пишет:
> On Fr, 2015-09-11 at 10:30 +0300, Michael Tokarev wrote:
>> 13.07.2015 14:58, Gerd Hoffmann wrote:
>>> Cc: Paolo Bonzini 
>>> Cc: qemu-triv...@nongnu.org
>>> Signed-off-by: Gerd Hoffmann 
>>
>> Gerd, -trivial received 3 patches like this enabling various virtio
>> bits for various architectures.  I never got any reviewed-by or ACK
>> on these patches, so they're not applied still.  Maybe you can add
>> some explanation as to why you think it's a good idea, or else it
>> doesn't seem like a trivial change? :)
> 
> This one just syncs x86_64 and i386.  Dunno about the other ones, have a
> pointer?

Ok, applied this one to -trivial (with a commit comment :)

The other 2 are actually 2 incarnations of the same patch,
http://patchwork.ozlabs.org/patch/494250/

Since the replies hasn't been seen at qemu-trivial@ I haven't
found the replies.  Now I did, and it looks like the patch
needs some more work ;)

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-block] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Daniel P. Berrange]

On Fri, Sep 11, 2015 at 11:36:10AM +0200, Alberto Garcia wrote:
> On Fri 11 Sep 2015 11:14:33 AM CEST, Fam Zheng  wrote:
> 
> >> > Another advantage for bdrv_aio_poll() is, in main loop we will not
> >> > need a separate AioContext in changes like:
> >> > 
> >> > http://patchwork.ozlabs.org/patch/514968/
> >> > 
> >> > Because nested aio_poll will automatically be limited to only
> >> > process block layer events. My idea is to eventually let main loop
> >> > use aio_poll
> >> 
> >> That would be a step back.  Using GSource is useful because it lets
> >> you integrate libraries such as GTK+.
> >
> > Can we move GTK to a separate GSource thread?
> 
> I think that GTK should always run in the main thread, or at least the
> one running the default main loop / GMainContext.

In theory GTK can run from a separate thread, but my experiance with
GTK and threads is that you end up in a world of hurt if you try to
anything except use the main thread, so I'd really recommend against
it. Even if you do extensive testing locally, things still break across
different GTK versions people have, so its just not worth the pain.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/2] spapr: Add support for hwrng when available

[Alexander Graf]

On 11.09.15 02:46, David Gibson wrote:
> On Thu, Sep 10, 2015 at 02:13:26PM +0200, Alexander Graf wrote:
>>
>>
>>> Am 10.09.2015 um 14:03 schrieb Thomas Huth :
>>>
 On 10/09/15 12:40, David Gibson wrote:
> On Thu, Sep 10, 2015 at 09:33:21AM +0200, Thomas Huth wrote:
>> On 09/09/15 23:10, Thomas Huth wrote:
>> On 08/09/15 07:15, David Gibson wrote:
> ...
>>> At this point rather than just implementing them as discrete machine
>>> options, I suspect it will be more maintainable to split out the
>>> h-random implementation as a pseudo-device with its own qdev and so
>>> forth.  We already do similarly for the RTAS time of day functions
>>> (spapr-rtc).
>>
>> I gave that I try, but it does not work as expected. To be able to
>> specify the options, I'd need to instantiate this device with the
>> "-device" option, right? Something like:
>>
>>-device spapr-rng,backend=rng0,usekvm=0
>>
>> Now this does not work when I use TYPE_SYS_BUS_DEVICE as parent class
>> like it is done for spapr-rtc, since the user apparently can not plug
>> device to this bus on machine spapr (you can also not plug an spapr-rtc
>> device this way!).
>>
>> The spapr-vlan, spapr-vty, etc. devices are TYPE_VIO_SPAPR_DEVICE, so I
>> also tried that instead, but then the rng device suddenly shows up under
>> /vdevice in the device tree - that's also not what we want, I guess.
>
> I did some more tests, and I think I can get this working with one small
> modification to spapr_vio.c
>>> ...
> i.e. when the dt_name has not been set, the device won't be added to the
> /vdevice device tree node. If that's acceptable, I'll continue with this
> approach.

 A bit hacky.

 I think it would be preferable to build it under SysBus by default,
 like spapr-rtc.  Properties can be set on the device using -global (or
 -set, but -global is easier).
>>>
>>> If anyhow possible, I'd prefere to use "-device" for this instead, because
>>>
>>> a) it's easier to use for the user, for example you can simply use
>>>   "-device spapr-rng,?" to get the list of properties - this
>>>   does not seem to work with spapr-rtc (it has a "date" property
>>>   which does not show up in the help text?)
>>>
>>> b) unlike the rtc device which is always instantiated, the rng
>>>   device is rather optional, so it is IMHO more intuitive if
>>>   created via the -device option.
>>>
>>> So I'd like to give it a try with the TYPE_VIO_SPAPR_DEVICE first ... if
>>> you then still don't like the patches at all, I can still rework them to
>>> use TYPE_SYS_BUS_DEVICE instead.
>>
>> Please don't use sysbus. If the vio device approach turns ugly,
>> create a new spapr hcall bus instead. We should have had that from
>> the beginning really.
> 
> Ok.. why?
> 
> It's a system (pseudo-)device that doesn't have any common bus
> infrastructure with anything else.  Isn't that what SysBus is for?

No, sysbus means "A device that has MMIO and/or PIO connected via a bus
I'm too lazy to model" really. These devices have neither.

Back in the days before QOM, sysbus was our lowest common denominator,
but now that we have TYPE_DEVICE and can branch off of that, we really
shouldn't abuse sysbus devices for things they aren't.


Alex

Re: [Qemu-devel] [Qemu-block] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Fam Zheng]

On Fri, 09/11 11:36, Alberto Garcia wrote:
> On Fri 11 Sep 2015 11:14:33 AM CEST, Fam Zheng  wrote:
> 
> >> > Another advantage for bdrv_aio_poll() is, in main loop we will not
> >> > need a separate AioContext in changes like:
> >> > 
> >> > http://patchwork.ozlabs.org/patch/514968/
> >> > 
> >> > Because nested aio_poll will automatically be limited to only
> >> > process block layer events. My idea is to eventually let main loop
> >> > use aio_poll
> >> 
> >> That would be a step back.  Using GSource is useful because it lets
> >> you integrate libraries such as GTK+.
> >
> > Can we move GTK to a separate GSource thread?
> 
> I think that GTK should always run in the main thread, or at least the
> one running the default main loop / GMainContext.

Yeah it's basically GMainContext staying in the main thread and
block/net/chardev I/O put in a new AioContext thread.

Fam

Re: [Qemu-devel] [PATCH] virtio-vga: enable for i386

[Michael Tokarev]

13.07.2015 14:58, Gerd Hoffmann wrote:
> Cc: Paolo Bonzini 
> Cc: qemu-triv...@nongnu.org
> Signed-off-by: Gerd Hoffmann 

Gerd, -trivial received 3 patches like this enabling various virtio
bits for various architectures.  I never got any reviewed-by or ACK
on these patches, so they're not applied still.  Maybe you can add
some explanation as to why you think it's a good idea, or else it
doesn't seem like a trivial change? :)

Thanks,

/mjt

>  default-configs/i386-softmmu.mak | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/default-configs/i386-softmmu.mak 
> b/default-configs/i386-softmmu.mak
> index 48b5762..6f042e2 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -7,6 +7,7 @@ CONFIG_QXL=$(CONFIG_SPICE)
>  CONFIG_VGA_ISA=y
>  CONFIG_VGA_CIRRUS=y
>  CONFIG_VMWARE_VGA=y
> +CONFIG_VIRTIO_VGA=y
>  CONFIG_VMMOUSE=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
> 

Re: [Qemu-devel] [PATCH v4 00/11] qemu-log, perfmap and other logging tweaks

[Michael Tokarev]

03.08.2015 12:14, Alex Bennée wrote:
> Hi,
> 
> This is mostly just a re-base to keep current with master. I've added
> a couple of outstanding s-o-b and r-b tags. There are also two new
> patches in this series which seemed to be worth keeping together:
> 
>  - a simple patch to dump invocation info in the log
>  - the cputlb logging clean-up posted as an RFC last month
> 
> Could folks interested in TCG stuff have a look over the light
> re-factor and perf map patches please?
> 
> It would be nice to get these in the next cycle but I'm unsure who's
> tree I should be targeting? Most of the top level files touched
> (cputlb, translate-all, qemu-log) just have the list marked as the
> maintainer ;-)

With this many review comments and somewhat hot discussion about some
patches, I think I have a good excuse to not apply this series
to qemu-tivial tree ;)

Keep up the good work please!

/mjt

Re: [Qemu-devel] [PATCH 5/6] hw/arm/spitz: Remove meaningless blank Property

[Michael Tokarev]

Aplied (finally!) to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH 01/23] userfaultfd: linux/Documentation/vm/userfaultfd.txt

[Michael Kerrisk (man-pages)]

On 05/14/2015 07:30 PM, Andrea Arcangeli wrote:
> Add documentation.

Hi Andrea,

I do not recall... Did you write a man page also for this new system call?

Thanks,

Michael


> Signed-off-by: Andrea Arcangeli 
> ---
>  Documentation/vm/userfaultfd.txt | 140 
> +++
>  1 file changed, 140 insertions(+)
>  create mode 100644 Documentation/vm/userfaultfd.txt
> 
> diff --git a/Documentation/vm/userfaultfd.txt 
> b/Documentation/vm/userfaultfd.txt
> new file mode 100644
> index 000..c2f5145
> --- /dev/null
> +++ b/Documentation/vm/userfaultfd.txt
> @@ -0,0 +1,140 @@
> += Userfaultfd =
> +
> +== Objective ==
> +
> +Userfaults allow the implementation of on-demand paging from userland
> +and more generally they allow userland to take control various memory
> +page faults, something otherwise only the kernel code could do.
> +
> +For example userfaults allows a proper and more optimal implementation
> +of the PROT_NONE+SIGSEGV trick.
> +
> +== Design ==
> +
> +Userfaults are delivered and resolved through the userfaultfd syscall.
> +
> +The userfaultfd (aside from registering and unregistering virtual
> +memory ranges) provides two primary functionalities:
> +
> +1) read/POLLIN protocol to notify a userland thread of the faults
> +   happening
> +
> +2) various UFFDIO_* ioctls that can manage the virtual memory regions
> +   registered in the userfaultfd that allows userland to efficiently
> +   resolve the userfaults it receives via 1) or to manage the virtual
> +   memory in the background
> +
> +The real advantage of userfaults if compared to regular virtual memory
> +management of mremap/mprotect is that the userfaults in all their
> +operations never involve heavyweight structures like vmas (in fact the
> +userfaultfd runtime load never takes the mmap_sem for writing).
> +
> +Vmas are not suitable for page- (or hugepage) granular fault tracking
> +when dealing with virtual address spaces that could span
> +Terabytes. Too many vmas would be needed for that.
> +
> +The userfaultfd once opened by invoking the syscall, can also be
> +passed using unix domain sockets to a manager process, so the same
> +manager process could handle the userfaults of a multitude of
> +different processes without them being aware about what is going on
> +(well of course unless they later try to use the userfaultfd
> +themselves on the same region the manager is already tracking, which
> +is a corner case that would currently return -EBUSY).
> +
> +== API ==
> +
> +When first opened the userfaultfd must be enabled invoking the
> +UFFDIO_API ioctl specifying a uffdio_api.api value set to UFFD_API (or
> +a later API version) which will specify the read/POLLIN protocol
> +userland intends to speak on the UFFD. The UFFDIO_API ioctl if
> +successful (i.e. if the requested uffdio_api.api is spoken also by the
> +running kernel), will return into uffdio_api.features and
> +uffdio_api.ioctls two 64bit bitmasks of respectively the activated
> +feature of the read(2) protocol and the generic ioctl available.
> +
> +Once the userfaultfd has been enabled the UFFDIO_REGISTER ioctl should
> +be invoked (if present in the returned uffdio_api.ioctls bitmask) to
> +register a memory range in the userfaultfd by setting the
> +uffdio_register structure accordingly. The uffdio_register.mode
> +bitmask will specify to the kernel which kind of faults to track for
> +the range (UFFDIO_REGISTER_MODE_MISSING would track missing
> +pages). The UFFDIO_REGISTER ioctl will return the
> +uffdio_register.ioctls bitmask of ioctls that are suitable to resolve
> +userfaults on the range registered. Not all ioctls will necessarily be
> +supported for all memory types depending on the underlying virtual
> +memory backend (anonymous memory vs tmpfs vs real filebacked
> +mappings).
> +
> +Userland can use the uffdio_register.ioctls to manage the virtual
> +address space in the background (to add or potentially also remove
> +memory from the userfaultfd registered range). This means a userfault
> +could be triggering just before userland maps in the background the
> +user-faulted page.
> +
> +The primary ioctl to resolve userfaults is UFFDIO_COPY. That
> +atomically copies a page into the userfault registered range and wakes
> +up the blocked userfaults (unless uffdio_copy.mode &
> +UFFDIO_COPY_MODE_DONTWAKE is set). Other ioctl works similarly to
> +UFFDIO_COPY.
> +
> +== QEMU/KVM ==
> +
> +QEMU/KVM is using the userfaultfd syscall to implement postcopy live
> +migration. Postcopy live migration is one form of memory
> +externalization consisting of a virtual machine running with part or
> +all of its memory residing on a different node in the cloud. The
> +userfaultfd abstraction is generic enough that not a single line of
> +KVM kernel code had to be modified in order to add postcopy live
> +migration to QEMU.
> +
> +Guest async page faults, FOLL_NOWAIT and all other GUP features work
> +just fine in 

[Qemu-devel] [PATCH v3] ppc/spapr: Implement H_RANDOM hypercall in QEMU

[Thomas Huth]

The PAPR interface defines a hypercall to pass high-quality
hardware generated random numbers to guests. Recent kernels can
already provide this hypercall to the guest if the right hardware
random number generator is available. But in case the user wants
to use another source like EGD, or QEMU is running with an older
kernel, we should also have this call in QEMU, so that guests that
do not support virtio-rng yet can get good random numbers, too.

This patch now adds a new pseude-device to QEMU that either
directly provides this hypercall to the guest or is able to
enable the in-kernel hypercall if available. The in-kernel
hypercall can be enabled with the use-kvm property, e.g.:

 qemu-system-ppc64 -device spapr-rng,use-kvm=true

For handling the hypercall in QEMU instead, a RngBackend is required
since the hypercall should provide "good" random data instead of
pseudo-random (like from a "simple" library function like rand()
or g_random_int()). Since there are multiple RngBackends available,
the user must select an appropriate backend via the "backend"
property of the device, e.g.:

 qemu-system-ppc64 -object rng-random,filename=/dev/hwrng,id=rng0 \
   -device spapr-rng,backend=rng0 ...

See http://wiki.qemu-project.org/Features-Done/VirtIORNG for
other example of specifying RngBackends.

Signed-off-by: Thomas Huth 
---
 v3:
 - Completely reworked the patch set accordingly to discussion
   on the mailing list, so that the code is now encapsulated
   as a QEMU device in a separate file.

 hw/ppc/Makefile.objs   |   2 +-
 hw/ppc/spapr.c |   8 +++
 hw/ppc/spapr_rng.c | 178 +
 include/hw/ppc/spapr.h |   4 ++
 target-ppc/kvm.c   |   9 +++
 target-ppc/kvm_ppc.h   |   5 ++
 6 files changed, 205 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/spapr_rng.c

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index c8ab06e..c1ffc77 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -3,7 +3,7 @@ obj-y += ppc.o ppc_booke.o
 # IBM pSeries (sPAPR)
 obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
 obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
-obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
+obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
 ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
 obj-y += spapr_pci_vfio.o
 endif
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bf0c64f..34e7d24 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -768,6 +768,14 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
 exit(1);
 }
 
+if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
+ret = spapr_rng_populate_dt(fdt);
+if (ret < 0) {
+fprintf(stderr, "couldn't setup rng device in fdt\n");
+exit(1);
+}
+}
+
 QLIST_FOREACH(phb, >phbs, list) {
 ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt);
 }
diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
new file mode 100644
index 000..d4923bc
--- /dev/null
+++ b/hw/ppc/spapr_rng.c
@@ -0,0 +1,178 @@
+/*
+ * QEMU sPAPR random number generator "device" for H_RANDOM hypercall
+ *
+ * Copyright 2015 Thomas Huth, 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 .
+ */
+
+#include "qemu/error-report.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/rng.h"
+#include "hw/ppc/spapr.h"
+#include "kvm_ppc.h"
+
+#define SPAPR_RNG(obj) \
+OBJECT_CHECK(sPAPRRngState, (obj), TYPE_SPAPR_RNG)
+
+typedef struct sPAPRRngState {
+/*< private >*/
+DeviceState ds;
+RngBackend *backend;
+bool use_kvm;
+} sPAPRRngState;
+
+typedef struct HRandomData {
+QemuSemaphore sem;
+union {
+uint64_t v64;
+uint8_t v8[8];
+} val;
+int received;
+} HRandomData;
+
+/* Callback function for the RngBackend */
+static void random_recv(void *dest, const void *src, size_t size)
+{
+HRandomData *hrdp = dest;
+
+if (src && size > 0) {
+assert(size + hrdp->received <= sizeof(hrdp->val.v8));
+memcpy(>val.v8[hrdp->received], src, size);
+hrdp->received += size;
+}
+
+qemu_sem_post(>sem);
+}
+
+/* Handler for the H_RANDOM hypercall */
+static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState 

Re: [Qemu-devel] [Qemu-block] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Paolo Bonzini]

On 11/09/2015 11:44, Fam Zheng wrote:
> > > > That would be a step back.  Using GSource is useful because it lets
> > > > you integrate libraries such as GTK+.
> > >
> > > Can we move GTK to a separate GSource thread?
> > 
> > I think that GTK should always run in the main thread, or at least the
> > one running the default main loop / GMainContext.
> 
> Yeah it's basically GMainContext staying in the main thread and
> block/net/chardev I/O put in a new AioContext thread.

Why?  The point of an event loop is that you can multiplex everything on
the same thread.  Unless we have specific needs (e.g. scalability) one
thread is the way to go and keep things simple.

The tool we have for scalability is AioContext + dataplane threads, and
because we _can_ integrate it into the main thread (AioContext is a
GSource) we can write code once for the main thread and for external
dataplane threads.

Using AioContext instead of duplicating the code is great.  Moving SLIRP
to Win32 would get rid of all the duplicated select() code between
aio-win32.c and main-loop.c.  A lot of main-loop.c code can go away
(probably not all because unlike glib we support nanosecond timeouts
with TimerListGroup, but who knows).

However, there is no need to deviate the event loop from the well-known,
standardized glib architecture.

Paolo

Re: [Qemu-devel] [PATCH] Makefile: suppress command echoing in silent mode (make -s)

[Michael Tokarev]

02.09.2015 12:14, Jerome Forissier wrote:
> Signed-off-by: Jerome Forissier 

Please Cc qemu-devel on patches.

>  rules.mak | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/rules.mak b/rules.mak
> index 4551b9e..a44ba29 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -125,7 +125,18 @@ modules:
>  %.a:
>   $(call quiet-command,rm -f $@ && $(AR) rcs $@ $^,"  AR
> $(TARGET_DIR)$@")
>  
> -quiet-command = $(if $(V),$1,$(if $(2),@echo $2 && $1, @$1))
> +# Suppress command echoing in silent mode (make -s)
> +ifneq ($(filter 4.%,$(MAKE_VERSION)),)  # make-4
> +ifneq ($(filter %s ,$(firstword x$(MAKEFLAGS))),)
> +SILENT := 1
> +endif
> +else# make-3.8x
> +ifneq ($(findstring s, $(MAKEFLAGS)),)
> +SILENT := 1
> +endif
> +endif
> +
> +quiet-command = $(if $(V),$1,$(if $(2),$(if $(SILENT),@$1,@echo $2 && 
> $1),@$1))

I'm not sure this is needed.  We have V=1/V=0 way of controlling silent mode
already, why add another way, which is also twisted (depends on make version
and complicates already complex makefiles)?

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] qapi-schema: remove legacy<> from doc

[Michael Tokarev]

04.09.2015 22:41, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> The legacy<> type is no longer used since 7ce7ffe02.

Applied to -trivial, with the typo fix suggested by Eric.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: Add Mount image file menu item

[Markus Armbruster]

Only reviewing around qmp_device_add() and such, ignoring the GUI part.

Programmingkid  writes:

> Add "Mount Image File..." and a "Eject Image File" menu items to
> cocoa interface. This patch makes sharing files between the
> host and the guest user-friendly.
>
> The "Mount Image File..." menu item displays a dialog box having the
> user pick an image file to use in QEMU. The image file is setup as
> a USB flash drive. The user can do the equivalent of removing the
> flash drive by selecting the file in the "Eject Image File" submenu.
>
> Signed-off-by: John Arbuckle 
>
> ---
> Detects if the user actually specified the -usb option.
> Removed the sendMonitorCommand() function. 
> Replaced a lot of code with C interface equivalent of monitor commands
> drive_add and device_add. 
>
>  ui/cocoa.m |  228 
> +++-
>  1 files changed, 227 insertions(+), 1 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 334e6f6..df1faea 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -31,6 +31,8 @@
>  #include "sysemu/sysemu.h"
>  #include "qmp-commands.h"
>  #include "sysemu/blockdev.h"
> +#include "include/monitor/qdev.h"
> +#include "hw/boards.h"
>  
>  #ifndef MAC_OS_X_VERSION_10_5
>  #define MAC_OS_X_VERSION_10_5 1050
> @@ -52,6 +54,8 @@
>  #endif
>  
>  #define cgrect(nsrect) (*(CGRect *)&(nsrect))
> +#define USB_DISK_ID "USB_DISK"
> +#define EJECT_IMAGE_FILE_TAG 2099
>  
>  typedef struct {
>  int width;
> @@ -829,6 +833,9 @@ QemuCocoaView *cocoaView;
>  - (void)powerDownQEMU:(id)sender;
>  - (void)ejectDeviceMedia:(id)sender;
>  - (void)changeDeviceMedia:(id)sender;
> +- (void)mountImageFile:(id)sender;
> +- (void)ejectImageFile:(id)sender;
> +- (void)updateEjectImageMenuItems;
>  @end
>  
>  @implementation QemuCocoaAppController
> @@ -1125,6 +1132,179 @@ QemuCocoaView *cocoaView;
>  }
>  }
>  
> +/* Displays a dialog box asking the user for an image file to mount */
> +- (void)mountImageFile:(id)sender
> +{
> +/* Display the file open dialog */
> +NSOpenPanel * openPanel;
> +openPanel = [NSOpenPanel openPanel];
> +[openPanel setCanChooseFiles: YES];
> +[openPanel setAllowsMultipleSelection: NO];
> +[openPanel setAllowedFileTypes: supportedImageFileTypes];
> +if([openPanel runModal] == NSFileHandlingPanelOKButton) {
> +NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
> +if(file == nil) {
> +NSBeep();
> +QEMU_Alert(@"Failed to convert URL to file path!");
> +return;
> +}
> +
> +static int usbDiskCount;
> +char *idString, *fileName, *driveOptions, *fileNameHint;
> +
> +/* Create the file name hint */
> +NSString *stringBuffer;
> +const int fileNameHintSize = 10;
> +stringBuffer = [file lastPathComponent];
> +stringBuffer = [stringBuffer stringByDeletingPathExtension];
> +if([stringBuffer length] > fileNameHintSize) {
> +stringBuffer = [stringBuffer substringToIndex: fileNameHintSize];
> +}
> +fileNameHint = g_strdup_printf("%s",
> +[stringBuffer cStringUsingEncoding: 
> NSASCIIStringEncoding]);
> +
> +idString = g_strdup_printf("%s_%s_%d", USB_DISK_ID, fileNameHint,
> +
> usbDiskCount++);

Indentation is off, long line.

We don't use CamelCase for variable names in QEMU.  See CODING_STYLE,
section 3. Naming.

More of the same elsewhere.

Your system-generated ID can theoretically clash with a user-specified
ID.  We should reserve a name space for system-generated IDs.  Related:
our discussion Re: Should we auto-generate IDs?  Anyway, wider issue,
not something this patch can solve.

> +
> +fileName = g_strdup_printf("%s",
> +[file cStringUsingEncoding: 
> NSASCIIStringEncoding]);
> +
> +/* drive_add equivalent code */
> +driveOptions = g_strdup_printf("if=none,id=%s,file=%s", idString,
> + 
> fileName);
> +DriveInfo *dinfo;
> +QemuOpts *opts;
> +MachineClass *mc;

No declarations in the middle of blocks, please.  See CODING_STYLE
section 5. Declarations.

More of the same elsewhere.

> +
> +opts = drive_def(driveOptions);

Breaks when fileName contains a comma.  Fine example of why formatting
stuff only to parse it apart again is a bad idea.  Use drive_add()
instead.  Roughly like

opts = drive_add(IF_NONE, -1, file_name, "");

You could've found this yourself easily had you slowed down a bit to
examine how we create drive QemuOpts elsewhere.

> +if (!opts) {
> +QEMU_Alert(@"Error: could not create QemuOpts object!");
> +return;
> +}
> +
> +mc = MACHINE_GET_CLASS(current_machine);
> +dinfo = 

[Qemu-devel] [PATCH] virtio-net: unbreak self announcement and guest offloads after migration

[Jason Wang]

After commit 019a3edbb25f1571e876f8af1ce4c55412939e5d ("virtio: make
features 64bit wide"). Device's guest_features was actually set after
vdc->load(). This breaks the assumption that device specific load()
function can check guest_features. For virtio-net, self announcement
and guest offloads won't work after migration.

Fixing this by defer them to virtio_net_load() where guest_features
were guaranteed to be set. Other virtio devices looks fine.

Fixes: 019a3edbb25f1571e876f8af1ce4c55412939e5d
   ("virtio: make features 64bit wide")
Cc: qemu-sta...@nongnu.org
Cc: Gerd Hoffmann 
Signed-off-by: Jason Wang 
---
 hw/net/virtio-net.c | 40 +++-
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f72eebf..2775e6a 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1458,11 +1458,33 @@ static int virtio_net_load(QEMUFile *f, void *opaque, 
int version_id)
 {
 VirtIONet *n = opaque;
 VirtIODevice *vdev = VIRTIO_DEVICE(n);
+int ret;
 
 if (version_id < 2 || version_id > VIRTIO_NET_VM_VERSION)
 return -EINVAL;
 
-return virtio_load(vdev, f, version_id);
+ret = virtio_load(vdev, f, version_id);
+if (ret) {
+return ret;
+}
+
+if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) {
+n->curr_guest_offloads = qemu_get_be64(f);
+} else {
+n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
+}
+
+if (peer_has_vnet_hdr(n)) {
+virtio_net_apply_guest_offloads(n);
+}
+
+if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) &&
+virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) {
+n->announce_counter = SELF_ANNOUNCE_ROUNDS;
+timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL));
+}
+
+return 0;
 }
 
 static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f,
@@ -1559,16 +1581,6 @@ static int virtio_net_load_device(VirtIODevice *vdev, 
QEMUFile *f,
 }
 }
 
-if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) {
-n->curr_guest_offloads = qemu_get_be64(f);
-} else {
-n->curr_guest_offloads = virtio_net_supported_guest_offloads(n);
-}
-
-if (peer_has_vnet_hdr(n)) {
-virtio_net_apply_guest_offloads(n);
-}
-
 virtio_net_set_queues(n);
 
 /* Find the first multicast entry in the saved MAC filter */
@@ -1586,12 +1598,6 @@ static int virtio_net_load_device(VirtIODevice *vdev, 
QEMUFile *f,
 qemu_get_subqueue(n->nic, i)->link_down = link_down;
 }
 
-if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) &&
-virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) {
-n->announce_counter = SELF_ANNOUNCE_ROUNDS;
-timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL));
-}
-
 return 0;
 }
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH 4/6] hw/gpio/zaurus: Remove meaningless blank Property

[Michael Tokarev]

Applied (finally!) to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH 3/6] hw/virtio/virtio-pci: Remove meaningless blank Property

[Michael Tokarev]

Applied (finally!) to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Paolo Bonzini]

On 09/09/2015 05:22, Fam Zheng wrote:
> Another advantage for bdrv_aio_poll() is, in main loop we will not need
> a separate AioContext in changes like:
> 
> http://patchwork.ozlabs.org/patch/514968/
> 
> Because nested aio_poll will automatically be limited to only process block
> layer events. My idea is to eventually let main loop use aio_poll

That would be a step back.  Using GSource is useful because it lets you
integrate libraries such as GTK+.

Paolo

> , which means
> we would also move chardev onto it. It would be neat to put all fds of the 
> main
> thread into a single AioContext.

Re: [Qemu-devel] [PATCH] gtk: use setlocale() for LC_MESSAGES only

[Gerd Hoffmann]

>  void early_gtk_display_init(int opengl)
>  {
> +/* The QEMU code relies on the assumption that it's always run in
> + * the C locale. Therefore it is not prepared to deal with
> + * operations that produce different results depending on the
> + * locale, such as printf's formatting of decimal numbers, and
> + * possibly others.
> + *
> + * Since GTK+ calls setlocale() by default -importing the locale
> + * settings from the environment- we must prevent it from doing so
> + * using gtk_disable_setlocale().
> + *
> + * QEMU's GTK+ UI, however, _does_ have translations for some of
> + * the menu items. As a trade-off between a functionally correct
> + * QEMU and a fully internationalized UI we support importing
> + * LC_MESSAGES from the environment (see the setlocale() call
> + * earlier in this file). This allows us to display translated
> + * messages leaving everything else untouched.
> + */
> +gtk_disable_setlocale();

Thanks.  Replacing my version with this one.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] virtio-vga: enable for i386

[Gerd Hoffmann]

On Fr, 2015-09-11 at 10:30 +0300, Michael Tokarev wrote:
> 13.07.2015 14:58, Gerd Hoffmann wrote:
> > Cc: Paolo Bonzini 
> > Cc: qemu-triv...@nongnu.org
> > Signed-off-by: Gerd Hoffmann 
> 
> Gerd, -trivial received 3 patches like this enabling various virtio
> bits for various architectures.  I never got any reviewed-by or ACK
> on these patches, so they're not applied still.  Maybe you can add
> some explanation as to why you think it's a good idea, or else it
> doesn't seem like a trivial change? :)

This one just syncs x86_64 and i386.  Dunno about the other ones, have a
pointer?

cheers,
  Gerd

Re: [Qemu-devel] [Qemu-block] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Paolo Bonzini]

On 11/09/2015 11:36, Alberto Garcia wrote:
> > > > Because nested aio_poll will automatically be limited to only
> > > > process block layer events. My idea is to eventually let main loop
> > > > use aio_poll
> > > 
> > > That would be a step back.  Using GSource is useful because it lets
> > > you integrate libraries such as GTK+.
> >
> > Can we move GTK to a separate GSource thread?
>
> I think that GTK should always run in the main thread, or at least the
> one running the default main loop / GMainContext.

Agreed.  The glib main loop is a positive thing, not a negative one.

Paolo

Re: [Qemu-devel] [PATCH] virtio-vga: enable for i386

[Paolo Bonzini]

On 11/09/2015 11:23, Michael Tokarev wrote:
> Ok, applied this one to -trivial (with a commit comment :)
> 
> The other 2 are actually 2 incarnations of the same patch,
> http://patchwork.ozlabs.org/patch/494250/
> 
> Since the replies hasn't been seen at qemu-trivial@ I haven't
> found the replies.  Now I did, and it looks like the patch
> needs some more work ;)

I think the sys/ioctl.h issue has been fixed now:

$ grep -Fr ioctl.h include/standard-headers/
$

Paolo

Re: [Qemu-devel] [PATCH] typofixes - v4 - https://github.com/vlajos/misspell_fixer

[Michael Tokarev]

09.09.2015 00:45, Veres Lajos wrote:
> Signed-off-by: Veres Lajos 

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH 1/6] hw/s390x/s390-virtio-bus: Remove meaningless blank Property

[Michael Tokarev]

Applied (finally!) to -trivial, without already fixed -net and -scsi bits.
Thanks!

/mjt

Re: [Qemu-devel] [PATCH 6/6] Add comma after DEFINE_PROP_END_OF_LIST()

[Michael Tokarev]

12.05.2015 05:25, shannon.z...@linaro.org wrote:
> From: Shannon Zhao 
[]
> -DEFINE_PROP_END_OF_LIST()
> +DEFINE_PROP_END_OF_LIST(),
[]

I don't see a reason doing this.

Thanks,

/mjt

Re: [Qemu-devel] [ipxe-devel] EFI_PXE_BASE_CODE_PROTOCOL

[Gerd Hoffmann]

> > https://ftp.fau.de/centos/7.1.1503/os/x86_64/EFI/BOOT/
> 
> That grubx64.efi binary makes no attempt to even look for an 
> EFI_PXE_BASE_CODE_PROTOCOL.  Here's the complete log of calls that it makes:

With grubx86.efi loaded via tftp I assume?

> OpenProtocol ( 
> PciRoot(0x0)/Pci(0x3,0x0)/MAC(525400123456,0x1)/grubx64.efi, 
> LoadedImage, 
> PciRoot(0x0)/Pci(0x3,0x0)/MAC(525400123456,0x1)/grubx64.efi, , G ) 
> = 0 ( 0x3f1aac00 ) -> 0x3e90349b

... yes, seems so.

The very same grub binary boots from network and disk, so I guess here
it tries to figure how it was loaded to then go fetch the grub.cfg file
from the same location.

On a successfull boot (with EFI_DOWNGRADE_UX) I see this in the ovmf
log:

[ ... lot of stuff snipped ... ]
Booting EFI Network
InstallProtocolInterface: 245DCA21-FB7B-11D3-8F01-00A0C969723B 6FD60E0
InstallProtocolInterface: 41D94CD2-35B6-455A-8258-D4E51334AADD 7073B20
InstallProtocolInterface: 3AD9DF29-4501-478D-B1F8-7F7FE70E50F3 7074BF8
InstallProtocolInterface: F4B427BB-BA21-4F16-BC4E-43E416AB619C 70798B0
InstallProtocolInterface: 245DCA21-FB7B-11D3-8F01-00A0C969723B 6FD60E0
InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70ADB40
Loading driver at 0x6605000 EntryPoint=0x6605400 
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7094898
PixelBlueGreenRedReserved8BitPerColor

The guids translate to:

245DCA21-FB7B-11D3-8F01-00A0C969723B gEfiPxeBaseCodeCallbackProtocolGuid
41D94CD2-35B6-455A-8258-D4E51334AADD gEfiIp4ProtocolGuid
3AD9DF29-4501-478D-B1F8-7F7FE70E50F3 gEfiUdp4ProtocolGuid
F4B427BB-BA21-4F16-BC4E-43E416AB619C gEfiArpProtocolGuid
245DCA21-FB7B-11D3-8F01-00A0C969723B gEfiPxeBaseCodeCallbackProtocolGuid
5B1B31A1-9562-11D2-8E3F-00A0C969723B gEfiLoadedImageProtocolGuid
BC62157E-3E33-4FEC-9920-2D3B36D750DF
gEfiLoadedImageDevicePathProtocolGuid

Hmm, EfiPxeBaseCode*Callback*Protocol?

> Do you have a set of instructions (starting from "git clone 
> git://git.savannah.gnu.org/grub.git") which will allow me to rebuild 
> that grubx64.efi binary from the source?

The rpm spec file for grub2 is here:

https://git.centos.org/blob/rpms!
grub2/c695181b7bb165516116ba7c7d94805c799ae977/SPECS!grub2.spec

grubx64.efi is created with grub-mkimage:


  ./grub-mkimage -O %{grubefiarch} -o %{grubefiname}.orig \
-p /EFI/%{efidir} -d grub-core ${GRUB_MODULES}


cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 11/11] block: Only poll block layer fds in bdrv_aio_poll

[Fam Zheng]

On Fri, 09/11 10:15, Paolo Bonzini wrote:
> 
> 
> On 09/09/2015 05:22, Fam Zheng wrote:
> > Another advantage for bdrv_aio_poll() is, in main loop we will not need
> > a separate AioContext in changes like:
> > 
> > http://patchwork.ozlabs.org/patch/514968/
> > 
> > Because nested aio_poll will automatically be limited to only process block
> > layer events. My idea is to eventually let main loop use aio_poll
> 
> That would be a step back.  Using GSource is useful because it lets you
> integrate libraries such as GTK+.

Can we move GTK to a separate GSource thread?

Fam

Re: [Qemu-devel] [PULL v1 00/21] Multi-arch queue

[Peter Maydell]

On 11 September 2015 at 06:44, Peter Crosthwaite
 wrote:
> Flush of multi-arch pre-requisite work.
>
> 
> multi-arch queue:
> * add EM_MOXIE
> * Cleanup ELF_MACHINE and remove from cpu.h
> 
>
> The following changes since commit 9d34158a5af734e8de0b42b0a7228200c426a8d0:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20150907' into 
> staging (2015-09-07 16:07:47 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/pcrost/qemu multi-arch.1.2015.09.07
>
> for you to fetch changes up to f4290e0cf89fa512df44e607f239d46e73d46576:
>
>   ppc: Rename ELF_MACHINE to be PPC specific (2015-09-07 23:59:28 -0700)

Hi. This looks like it's a branch, not a gpg signed tag?
I only take signed pullreqs these days...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qom/object.h: remove some child/parent doc

[Michael Tokarev]

07.09.2015 18:46, Andreas Färber wrote:
> Am 04.09.2015 um 23:01 schrieb marcandre.lur...@redhat.com:
>> From: Marc-André Lureau 
>>
>> It looks like this documentation is obsolete: a child object may lookup
>> its parent stored in the Object struct.
>>
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  include/qom/object.h | 3 ---
>>  1 file changed, 3 deletions(-)
> 
> Either once again you are trying to do stuff behind my back, or your
> setup is really broken: I double-checked that include/qom/ is listed in
> MAINTAINERS, so I should've been CC'ed rather than just -trivial.
> 
> It's been a valid rule not to mess with these internal fields, therefore
> this is not trivial at all, and that's one reason why my x86 CPU series
> using it was an RFC. We should either come up with a proper wrapper
> function object_get_parent(), or with a wrapper function adding a link<>
> property (where we would need to be careful with ref counts) - long time
> only the composition tree needed to mess with an object's parent.
> 
> If you have a concrete use case of parent access, please point to it.

So, should the current documentation remain, or should the patch be
applied?  If the latter, Andreas, please take care of it.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] q35: Remove old machine versions

[Eduardo Habkost]

Ping?

So, what's the reason we are still keeping those old machines in the
code?


On Tue, Aug 18, 2015 at 04:11:42PM -0700, Eduardo Habkost wrote:
> Migration with q35 was not possible before commit
> 04329029a8c539eb5f75dcb6d8b016f0c53a031a, because q35 unconditionally creates
> an ich9-ahci device, that was marked as unmigratable. So all q35 machines
> before pc-q35-2.4 were unmigratable, and there's no point in keeping
> compatibility code for them.
> 
> Remove all old pc-q35 machine classes and keep only pc-q35-2.4.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  hw/i386/pc_q35.c | 153 
> ---
>  1 file changed, 153 deletions(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 4ee653e..e482f2f 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -272,60 +272,6 @@ static void pc_q35_init(MachineState *machine)
>  }
>  }
>  
> -static void pc_compat_2_3(MachineState *machine)
> -{
> -PCMachineState *pcms = PC_MACHINE(machine);
> -savevm_skip_section_footers();
> -if (kvm_enabled()) {
> -pcms->smm = ON_OFF_AUTO_OFF;
> -}
> -global_state_set_optional();
> -savevm_skip_configuration();
> -}
> -
> -static void pc_compat_2_2(MachineState *machine)
> -{
> -pc_compat_2_3(machine);
> -machine->suppress_vmdesc = true;
> -}
> -
> -static void pc_compat_2_1(MachineState *machine)
> -{
> -PCMachineState *pcms = PC_MACHINE(machine);
> -
> -pc_compat_2_2(machine);
> -pcms->enforce_aligned_dimm = false;
> -x86_cpu_compat_kvm_no_autodisable(FEAT_8000_0001_ECX, CPUID_EXT3_SVM);
> -}
> -
> -static void pc_compat_2_0(MachineState *machine)
> -{
> -pc_compat_2_1(machine);
> -}
> -
> -static void pc_compat_1_7(MachineState *machine)
> -{
> -pc_compat_2_0(machine);
> -option_rom_has_mr = true;
> -x86_cpu_compat_kvm_no_autoenable(FEAT_1_ECX, CPUID_EXT_X2APIC);
> -}
> -
> -static void pc_compat_1_6(MachineState *machine)
> -{
> -pc_compat_1_7(machine);
> -rom_file_has_mr = false;
> -}
> -
> -static void pc_compat_1_5(MachineState *machine)
> -{
> -pc_compat_1_6(machine);
> -}
> -
> -static void pc_compat_1_4(MachineState *machine)
> -{
> -pc_compat_1_5(machine);
> -}
> -
>  #define DEFINE_Q35_MACHINE(suffix, name, compatfn, optionfn) \
>  static void pc_init_##suffix(MachineState *machine) \
>  { \
> @@ -358,102 +304,3 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
>  
>  DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
> pc_q35_2_4_machine_options);
> -
> -
> -static void pc_q35_2_3_machine_options(MachineClass *m)
> -{
> -pc_q35_2_4_machine_options(m);
> -m->no_floppy = 0;
> -m->no_tco = 1;
> -m->alias = NULL;
> -SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
> -}
> -
> -DEFINE_Q35_MACHINE(v2_3, "pc-q35-2.3", pc_compat_2_3,
> -   pc_q35_2_3_machine_options);
> -
> -
> -static void pc_q35_2_2_machine_options(MachineClass *m)
> -{
> -PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -pc_q35_2_3_machine_options(m);
> -SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
> -pcmc->rsdp_in_ram = false;
> -}
> -
> -DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2,
> -   pc_q35_2_2_machine_options);
> -
> -
> -static void pc_q35_2_1_machine_options(MachineClass *m)
> -{
> -PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -pc_q35_2_2_machine_options(m);
> -m->default_display = NULL;
> -SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
> -pcmc->smbios_uuid_encoded = false;
> -}
> -
> -DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
> -   pc_q35_2_1_machine_options);
> -
> -
> -static void pc_q35_2_0_machine_options(MachineClass *m)
> -{
> -PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -pc_q35_2_1_machine_options(m);
> -SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
> -pcmc->has_reserved_memory = false;
> -pcmc->smbios_legacy_mode = true;
> -pcmc->acpi_data_size = 0x1;
> -}
> -
> -DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
> -   pc_q35_2_0_machine_options);
> -
> -
> -static void pc_q35_1_7_machine_options(MachineClass *m)
> -{
> -PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -pc_q35_2_0_machine_options(m);
> -m->default_machine_opts = NULL;
> -SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
> -pcmc->smbios_defaults = false;
> -pcmc->gigabyte_align = false;
> -}
> -
> -DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_7,
> -   pc_q35_1_7_machine_options);
> -
> -
> -static void pc_q35_1_6_machine_options(MachineClass *m)
> -{
> -PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> -pc_q35_machine_options(m);
> -SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
> -pcmc->has_acpi_build = false;
> -}
> -
> -DEFINE_Q35_MACHINE(v1_6, "pc-q35-1.6", pc_compat_1_6,
> -   pc_q35_1_6_machine_options);
> -
> -
> -static void pc_q35_1_5_machine_options(MachineClass *m)
> 

Re: [Qemu-devel] [PATCH v5 11/14] block/backup: support block job transactions

[Max Reitz]

On 07.09.2015 09:34, Fam Zheng wrote:
> From: Stefan Hajnoczi 
> 
> Join the transaction when the 'transactional-cancel' QMP argument is
> true.
> 
> This ensures that the sync bitmap is not thrown away if another block
> job in the transaction is cancelled or fails.  This is critical so
> incremental backup with multiple disks can be retried in case of
> cancellation/failure.
> 
> Signed-off-by: Stefan Hajnoczi 
> Signed-off-by: Fam Zheng 
> ---
>  block/backup.c|  25 +++--
>  blockdev.c| 139 
> --
>  hmp.c |   2 +-
>  include/block/block_int.h |   3 +-
>  qapi/block-core.json  |  16 +-
>  5 files changed, 148 insertions(+), 37 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 9776d9c..3a3dccc 100644
> --- a/block/backup.c
> +++ b/block/backup.c

[snip]

> @@ -545,6 +559,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
> *target,
> sync_bitmap : NULL;
>  job->common.len = len;
>  job->common.co = qemu_coroutine_create(backup_run);
> +block_job_txn_add_job(txn, >common);

You're calling this for every single job, so the reference count of txn
will probably be job count + 1. However, block_job_txn_unref() is only
called for one block job by block_job_completed_txn_{abort,success}(). I
think you need to call block_job_txn_unref() for every completed block
job, otherwise it will be leaked if there is more than a single job in
the txn.

Max

>  qemu_coroutine_enter(job->common.co, job);
>  return;
> 



signature.asc
Description: OpenPGP digital signature