Re: [Qemu-devel] [PATCH v6 00/11] Make 'dump-guest-memory' dump in kdump-compressed format

2014-01-06 Thread Qiao Nuohan

On 01/07/2014 03:27 PM, Laszlo Ersek wrote:

I haven't finished reviewing it yet, but thus far (up to&  including
patch 05) I'm OK with it.


I see, thanks for your work.

--
Regards
Qiao Nuohan



Re: [Qemu-devel] [PATCH v6 00/11] Make 'dump-guest-memory' dump in kdump-compressed format

2014-01-06 Thread Laszlo Ersek
On 01/07/14 07:32, Qiao Nuohan wrote:
> Hello Eric, Luiz and Laszlo,
> 
> What do you think about my series? And I have add the light-weight
> introspection in the last patch, do you have some comments on it?

I haven't finished reviewing it yet, but thus far (up to & including
patch 05) I'm OK with it.

Thanks
Laszlo




Re: [Qemu-devel] [PATCH V5] char: restore read callback on a reattached (hotplug) chardev

2014-01-06 Thread Amit Shah
On (Wed) 18 Dec 2013 [16:15:19], Gal Hammer wrote:
> Fix a bug that was introduced in commit 386a5a1e. A removal of a device
> set the chr handlers to NULL. However when the device is plugged back,
> its read callback is not restored so data can't be transferred from the
> host to the guest (e.g. via the virtio-serial port).
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1027181
> 
> Signed-off-by: Gal Hammer 
> 
> ---
>  qemu-char.c | 21 +
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> V5: - remove_fd_in_watch in fd_chr_update_read_handler as well.
> - fix pty backend.

Reviewed-by: Amit Shah 

Gerd, could you take a look as well?

Amit



Re: [Qemu-devel] [PATCH v6 05/11] dump: add support for lzo/snappy

2014-01-06 Thread Laszlo Ersek
On 01/07/14 07:25, Qiao Nuohan wrote:
> On 01/07/2014 03:25 AM, Laszlo Ersek wrote:
>> You could have displayed the lzo / snappy settings along with the other
>> settings in the "big echo block". But it's not too important; if you
>> want you can add it later.
> 
> You mean the following part? Thanks for pointing it out.
> 
> 
> echo "Standard options:"
> echo "  --help   print this message"
> echo "  --prefix=PREFIX  install in PREFIX [$prefix]"
> echo "  --interp-prefix=PREFIX   where to find shared libraries, etc."
> echo "   use %M for cpu name [$interp_prefix]"
> echo "  --target-list=LIST   set target list (default: build
> everything)"
> echo "Available targets: $default_target_list" | \
> fold -s -w 53 | sed -e 's/^/   /'
> echo ""
> echo "Advanced options (experts only):"
> echo "  --source-path=PATH   path of source code [$source_path]"
> echo "  --cross-prefix=PREFIXuse PREFIX for compile tools
> [$cross_prefix]"
> echo "  --cc=CC  use C compiler CC [$cc]"
> ...
> 
> 

This is the help text, I didn't mean that.

I meant this output, printed after configure finishes:

---
Install prefix/opt/qemu-installed
BIOS directory/opt/qemu-installed/share/qemu
binary directory  /opt/qemu-installed/bin
[...]
libssh2 support   no
TPM passthrough   no
QOM debugging yes
vhdx  yes
---

Thanks
Laszlo



Re: [Qemu-devel] [PATCH v6 00/11] Make 'dump-guest-memory' dump in kdump-compressed format

2014-01-06 Thread Qiao Nuohan

Hello Eric, Luiz and Laszlo,

What do you think about my series? And I have add the light-weight
introspection in the last patch, do you have some comments on it?



On 01/05/2014 03:27 PM, Qiao Nuohan wrote:

Hi, all

The last version is here:
http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg01376.html

Command 'dump-guest-memory' was introduced to dump guest's memory. But the
vmcore's format is only elf32 or elf64. The message is here:
http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03379.html

Compared with migration, the missing of compression feature means regression
to 'dump-guest-memory'. So we post these patches to make 'dump-guest-memory' be
able to dump guest's in kdump-compressed format. Then vmcore can be much
smaller, and easily to be delivered.

The kdump-compressed format is *linux specific* *linux standard* crash dump
format used in kdump framework. The kdump-compressed format is readable only
with the crash utility, and it can be smaller than the ELF format because of
the compression support. To get more detailed information about
kdump-compressed format, please refer to the following URL:
http://sourceforge.net/projects/makedumpfile/

Note, similar to 'dump-guest-memory':
1. The guest should be x86 or x86_64. The other arch is not supported now.
2. If the OS is in the second kernel, gdb may not work well, and crash can
work by specifying '--machdep phys_addr=xxx' in the command line. The
reason is that the second kernel will update the page table, and we can
not get the page table for the first kernel.
3. The cpu's state is stored in QEMU note.
4. The vmcore are able to be compressed with zlib, lzo or snappy. zlib is
available by default, and option '--enable-lzo' or '--enable-snappy'
should be specified with 'configure' to make lzo or snappy available.

Changelog:
Changes from v5 to v6:
1. add run-time check for compression format(lzo/snappy)
2. address Stefan's comments about reusing code and coding style
3. update the version of kdump-compressed format to 6th
4. resplit the patches
5. Add 'query-dump-guest-memory-capability' command

Changes from v4 to v5:
1. using flatten format to avoid using temporary files according to Stefan's
comments
2. Address Andreas's comments about coding style

Changes from v3 to v4:
1. change to avoid conflict with Andreas's patches
2. rebase

Changes from v2 to v3:
1. Address Eric's comment

Changes from v1 to v2:
1. Address Eric&  Daniel's comment: fix manner of string copy.
2. Address Eric's comment: replace reinventing new constants by using the
ready-made ones accoring.
3. Address Andreas's comment: remove useless include.

Qiao Nuohan (11):
   dump: Add argument to write_elfxx_notes
   dump: Add API to write header of flatten format
   dump: Add API to write vmcore
   dump: Add API to write elf notes to buffer
   dump: add support for lzo/snappy
   dump: add API to write dump header
   dump: Add API to write dump_bitmap
   dump: Add APIs to operate DataCache
   dump: Add API to write dump pages
   dump: Make kdump-compressed format available for 'dump-guest-memory'
   Add 'query-dump-guest-memory-capability' command

  configure |   50 +++
  dump.c|  929 -
  hmp-commands.hx   |   12 +-
  hmp.c |   23 ++-
  include/sysemu/dump.h |  138 
  qapi-schema.json  |   35 ++-
  qmp-commands.hx   |   13 +-
  7 files changed, 1178 insertions(+), 22 deletions(-)





--
Regards
Qiao Nuohan



Re: [Qemu-devel] [PATCH v6 05/11] dump: add support for lzo/snappy

2014-01-06 Thread Qiao Nuohan

On 01/07/2014 03:25 AM, Laszlo Ersek wrote:

You could have displayed the lzo / snappy settings along with the other
settings in the "big echo block". But it's not too important; if you
want you can add it later.


You mean the following part? Thanks for pointing it out.


echo "Standard options:"
echo "  --help   print this message"
echo "  --prefix=PREFIX  install in PREFIX [$prefix]"
echo "  --interp-prefix=PREFIX   where to find shared libraries, etc."
echo "   use %M for cpu name [$interp_prefix]"
echo "  --target-list=LIST   set target list (default: build everything)"
echo "Available targets: $default_target_list" | \
fold -s -w 53 | sed -e 's/^/   /'
echo ""
echo "Advanced options (experts only):"
echo "  --source-path=PATH   path of source code [$source_path]"
echo "  --cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]"
echo "  --cc=CC  use C compiler CC [$cc]"
...


--
Regards
Qiao Nuohan




Re: [Qemu-devel] [PATCH v6 04/11] dump: Add API to write elf notes to buffer

2014-01-06 Thread Qiao Nuohan

On 01/07/2014 02:46 AM, Laszlo Ersek wrote:

  static int dump_cleanup(DumpState *s)
>  @@ -754,6 +757,22 @@ static int write_buffer(int fd, bool flag_flatten, 
off_t offset, void *buf,
>return 0;
>}
>
>  +static int buf_write_note(void *buf, size_t size, void *opaque)
>  +{

"const void *buf" would have been more "elegant".


>  +DumpState *s = opaque;
>  +
>  +/* note_buf is not enough */
>  +if (s->note_buf_offset + size>  s->note_size) {
>  +return -1;
>  +}
>  +
>  +memcpy(s->note_buf + s->note_buf_offset, buf, size);

Giving type "uint8_t" to "*note_buf" would have been preferable.
Addition to a pointer-to-void is a constraint violation in standard C
("... operand shall be a pointer to an object type ..."), ie. it's a gcc
extension here, but I guess we can live with it.

Using s->note_size as limit seems correct.



Acked.

--
Regards
Qiao Nuohan



Re: [Qemu-devel] [PATCH v6 03/11] dump: Add API to write vmcore

2014-01-06 Thread Qiao Nuohan

On 01/07/2014 02:12 AM, Laszlo Ersek wrote:

@@ -726,6 +726,34 @@ static int write_end_flat_header(int fd)
>return 0;
>}
>
>  +static int write_buffer(int fd, bool flag_flatten, off_t offset, void *buf,
>  +size_t size)
>  +{

You might have wanted to const-qualify "*buf" here, but it certainly
doesn't warrant a respin.


Acked, I will reflect it in later version.

--
Regards
Qiao Nuohan



Re: [Qemu-devel] [PATCH v6 01/11] dump: Add argument to write_elfxx_notes

2014-01-06 Thread Qiao Nuohan

On 01/07/2014 01:03 AM, Laszlo Ersek wrote:

I assume the direct calls to fd_write_vmcore() (which we're not
replacing here) will be substituted / abstracted later on in the series.


Yes, patch 6 will reuse write_elf32_notes/wirte_elf64_notes, but write notes
into buffer instead of vmcore directly.

--
Regards
Qiao Nuohan



Re: [Qemu-devel] [RFC PATCH 1/3] target-i386: Add 486sx, old486, and old486sx CPU models

2014-01-06 Thread H. Peter Anvin
On 03/25/2013 01:56 PM, Eduardo Habkost wrote:
> 
>>
>> It needs to be possible to fix bugs
> 
> It is possible to fix them today: just write a compat function or add a
> global variable that is handled by cpu_x86_init(), and call it from the
> pc-1.3 machine-type init function. See disable_kvm_pv_eoi() and
> enable_compat_apic_id_mode(), for example.
> 
> The difference is that this will be much easier once we introduce the
> static properties: then you just need to add the compat property values
> to the compat_props field on the machine-type struct.
> 

Hi guys,

Any movement on this in the past year?

-hpa




[Qemu-devel] [PATCH] Fix typo of tiemr in timer.h

2014-01-06 Thread Namhyung Kim
Signed-off-by: Namhyung Kim 
---
 include/qemu/timer.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 5afcffc..7f9a074 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -405,7 +405,7 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg);
  * timer_init:
  * @ts: the timer to be initialised
  * @timer_list: the timer list to attach the timer to
- * @scale: the scale value for the tiemr
+ * @scale: the scale value for the timer
  * @cb: the callback to be called when the timer expires
  * @opaque: the opaque pointer to be passed to the callback
  *
@@ -422,7 +422,7 @@ void timer_init(QEMUTimer *ts,
 /**
  * timer_new_tl:
  * @timer_list: the timer list to attach the timer to
- * @scale: the scale value for the tiemr
+ * @scale: the scale value for the timer
  * @cb: the callback to be called when the timer expires
  * @opaque: the opaque pointer to be passed to the callback
  *
@@ -447,7 +447,7 @@ static inline QEMUTimer *timer_new_tl(QEMUTimerList 
*timer_list,
 /**
  * timer_new:
  * @type: the clock type to use
- * @scale: the scale value for the tiemr
+ * @scale: the scale value for the timer
  * @cb: the callback to be called when the timer expires
  * @opaque: the opaque pointer to be passed to the callback
  *
-- 
1.7.11.7




[Qemu-devel] [PATCH] qom: Introduce object_property_remove()

2014-01-06 Thread Namhyung Kim
As the object_property_del(), object_property_del_child() and
object_property_del_all() do similar job, factor out the common code
to a new object_property_remove().

It'll also remove unnecessary call to object_property_find() in case
of object_property_del_child() so that we can get rid of the unused
errp argument too.

Signed-off-by: Namhyung Kim 
---
 qom/object.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index fc19cf6..a38cf72 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -347,30 +347,35 @@ static inline bool object_property_is_link(ObjectProperty 
*prop)
 return strstart(prop->type, "link<", NULL);
 }
 
+static void object_property_remove(Object *obj, ObjectProperty *prop)
+{
+QTAILQ_REMOVE(&obj->properties, prop, node);
+
+if (prop->release) {
+prop->release(obj, prop->name, prop->opaque);
+}
+
+g_free(prop->name);
+g_free(prop->type);
+g_free(prop);
+}
+
 static void object_property_del_all(Object *obj)
 {
 while (!QTAILQ_EMPTY(&obj->properties)) {
 ObjectProperty *prop = QTAILQ_FIRST(&obj->properties);
 
-QTAILQ_REMOVE(&obj->properties, prop, node);
-
-if (prop->release) {
-prop->release(obj, prop->name, prop->opaque);
-}
-
-g_free(prop->name);
-g_free(prop->type);
-g_free(prop);
+object_property_remove(obj, prop);
 }
 }
 
-static void object_property_del_child(Object *obj, Object *child, Error **errp)
+static void object_property_del_child(Object *obj, Object *child)
 {
 ObjectProperty *prop;
 
 QTAILQ_FOREACH(prop, &obj->properties, node) {
 if (object_property_is_child(prop) && prop->opaque == child) {
-object_property_del(obj, prop->name, errp);
+object_property_remove(obj, prop);
 break;
 }
 }
@@ -387,7 +392,7 @@ void object_unparent(Object *obj)
 (obj->class->unparent)(obj);
 }
 if (obj->parent) {
-object_property_del_child(obj->parent, obj, NULL);
+object_property_del_child(obj->parent, obj);
 }
 object_unref(obj);
 }
@@ -763,15 +768,7 @@ void object_property_del(Object *obj, const char *name, 
Error **errp)
 return;
 }
 
-if (prop->release) {
-prop->release(obj, name, prop->opaque);
-}
-
-QTAILQ_REMOVE(&obj->properties, prop, node);
-
-g_free(prop->name);
-g_free(prop->type);
-g_free(prop);
+object_property_remove(obj, prop);
 }
 
 void object_property_get(Object *obj, Visitor *v, const char *name,
-- 
1.7.11.7




[Qemu-devel] [RFC PATCH] elf loader: exit if incompatible architecture is detected

2014-01-06 Thread Alexey Kardashevskiy
If we know for sure that the image in "-kernel" is an ELF and we know its
architecture and it is not supported by the current QEMU, there is no
point to continue trying booting this image so let's exit once we deteced
this fact.

Signed-off-by: Alexey Kardashevskiy 
---


One of our users tried an X86 image with qemu-system-ppc64. Instead of
printing some reasonable message (which is possible in this case as the image
is ELF), QEMU (spapr.c) simply copied the image in RAM as a raw image and
SLOF failed to boot from it.

The patch fixes the issue but there are still questions.

1. Do we need more sophisticated error checking here? Return -2 instead of 
exit(1)
and do exit(1) few levels up?

2. The patch does not handle x86's vmlinuz case - these images are not ELFs
but "Linux kernel x86 boot executable bzImage" and QEMU does not parse them.
As a result, SLOF crashes with the registers dump. Do we really care to handle 
this?


---
 include/hw/elf_ops.h | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index acc701e..6bcc61f 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -212,21 +212,21 @@ static int glue(load_elf, SZ)(const char *name, int fd,
 case EM_PPC64:
 if (EM_PPC64 != ehdr.e_machine)
 if (EM_PPC != ehdr.e_machine)
-goto fail;
+goto arch_fail;
 break;
 case EM_X86_64:
 if (EM_X86_64 != ehdr.e_machine)
 if (EM_386 != ehdr.e_machine)
-goto fail;
+goto arch_fail;
 break;
 case EM_MICROBLAZE:
 if (EM_MICROBLAZE != ehdr.e_machine)
 if (EM_MICROBLAZE_OLD != ehdr.e_machine)
-goto fail;
+goto arch_fail;
 break;
 default:
 if (elf_machine != ehdr.e_machine)
-goto fail;
+goto arch_fail;
 }
 
 if (pentry)
@@ -306,4 +306,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
 g_free(data);
 g_free(phdr);
 return -1;
+
+arch_fail:
+fprintf(stderr, "qemu: could not load arch-incompatible kernel '%s'\n",
+name);
+exit(1);
 }
-- 
1.8.4.rc4




Re: [Qemu-devel] [RFC PATCH V2 3/5] qapi script: add event support by qapi-event.py

2014-01-06 Thread Wenchao Xia

+
+
+# Following are the functions that generate event behavior control functions.
+# Those functions are put here in the qapi-event.c, since it need to include
+# qapi-event.h for the event enum type declaration, put them in other file
+# requiring other file include qapi-event.h, causing a cross including. For
+# example: if we have qmp-event.c and qmp-event.h, then qmp-event.c
+# ->qmp-event.h->qapi-event.h, qapi-event.c->qmp-event.h. Another problem
+# follow: test-qapi-event.c will meet event enum double declaration since it
+# include both test-qapi-event.h and qmp-event.h. One solution is putting event
+# enum declaration in a separate header file, but then qmp-event.h need to
+# include test-qapi-event.h or qapi-event.h on compile time condition. So the
+# easist way is, just generate them here.
+
+def generate_event_behavior_control_decl(event_enum_name):
+ret = mcgen('''
+
+typedef void (*QAPIEventFuncEmit)(%(event_enum_name)s ev,
+  QDict *dict,
+  Error **errp);


Why does the emit function need 'ev'? Doesn't 'dict' contain all the
info it needs? Also, it's better to rename it to 'event' or 'qmp_event'.



  ev is for rate limit. I didn't move rate limit logic from callback to
internal, mainly because it is a generated function, which seems
too complex. And the genrated function was not moved into a separte
file mainly because the enum and including issue, see the comments
in script above.
  The problem is enum is changing, so the new file include it need
to recompile, for example, test schema and qapi-schema.json may
force recompile twice, I am not sure if it is good to do
some tricks in build system.


+
+void qapi_event_set_func_emit(QAPIEventFuncEmit emit);
+
+QAPIEventFuncEmit qapi_event_get_func_emit(void);
+''',
+event_enum_name = event_enum_name)
+return ret;
+
+def generate_event_behavior_control_implement():
+ret = mcgen('''
+
+typedef struct QAPIEventFunctions {
+QAPIEventFuncEmit emit;
+} QAPIEventFunctions;
+
+QAPIEventFunctions qapi_event_functions;
+
+void qapi_event_set_func_emit(QAPIEventFuncEmit emit)
+{
+qapi_event_functions.emit = emit;
+}


If this function and the typedefs don't change, I think they shouldn't
be generated.


+
+QAPIEventFuncEmit qapi_event_get_func_emit(void)
+{
+return qapi_event_functions.emit;
+}
+''')
+return ret
+
+
+# Start the real job
+
+try:
+opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
+   ["source", "header", "builtins", "prefix=",
+"output-dir="])
+except getopt.GetoptError, err:
+print str(err)
+sys.exit(1)
+
+output_dir = ""
+prefix = ""
+c_file = 'qapi-event.c'
+h_file = 'qapi-event.h'
+
+do_c = False
+do_h = False
+do_builtins = False
+
+for o, a in opts:
+if o in ("-p", "--prefix"):
+prefix = a
+elif o in ("-o", "--output-dir"):
+output_dir = a + "/"
+elif o in ("-c", "--source"):
+do_c = True
+elif o in ("-h", "--header"):
+do_h = True
+elif o in ("-b", "--builtins"):
+do_builtins = True
+
+if not do_c and not do_h:
+do_c = True
+do_h = True
+
+c_file = output_dir + prefix + c_file
+h_file = output_dir + prefix + h_file
+
+try:
+os.makedirs(output_dir)
+except os.error, e:
+if e.errno != errno.EEXIST:
+raise
+
+def maybe_open(really, name, opt):
+if really:
+return open(name, opt)
+else:
+import StringIO
+return StringIO.StringIO()
+
+fdef = maybe_open(do_c, c_file, 'w')
+fdecl = maybe_open(do_h, h_file, 'w')
+
+fdef.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI event functions
+ *
+ * Copyright IBM, Corp. 2014
+ *
+ * Authors:
+ *  Wenchao Xia   
+ *
+ * This work is licensed under the terms of the GNU GPLv2+ or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "%(header)s"
+#include "%(prefix)sqapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp-event.h"
+
+''',
+ prefix=prefix, header=basename(h_file)))
+
+fdecl.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI event function
+ *
+ * Copyright IBM, Corp. 2014
+ *
+ * Authors:
+ *  Wenchao Xia  
+ *
+ * This work is licensed under the terms of the GNU GPLv2+ or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef %(guard)s
+#define %(guard)s
+
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "%(prefix)sqapi-types.h"
+
+''',
+  prefix=prefix, guard=guardname(h_file)))
+
+exprs = parse_schema(sys.stdin)
+
+event_enum_name = "QAPIEvent"
+event_enum_values = []
+event_enum_strings = []
+
+for expr in exprs:
+if expr.has_key('event'):
+event_name = expr['event']
+params = expr.get('data')
+ 

Re: [Qemu-devel] [RFC PATCH V2 3/5] qapi script: add event support by qapi-event.py

2014-01-06 Thread Wenchao Xia

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
new file mode 100644
index 000..7526366
--- /dev/null
+++ b/scripts/qapi-event.py
@@ -0,0 +1,432 @@
+#
+# QAPI event generator
+#
+# Copyright IBM, Corp. 2014
+#
+# Authors:
+#  Wenchao Xia 
+#
+# This work is licensed under the terms of the GNU GPLv2+ or later.
+# See the COPYING.LIB file in the top-level directory.
+
+from ordereddict import OrderedDict
+from qapi import *
+import sys
+import os
+import getopt
+import errno
+
+def _generate_event_api_name(event_name, params):


Why the underline? And, what you generate is a function declaration...



  I want to tip it is not a public API. It is generating API name, 
missing ";" for declaration, implement may want it without ";". :)



+api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
+l = len(api_name)
+
+if params:
+for argname, argentry, optional, structured in parse_args(params):
+if structured:
+sys.stderr.write("Nested structure define in event is not "
+ "supported now, event '%s', argname '%s'\n" %
+ (event_name, argname))
+sys.exit(1)
+continue
+
+if optional:
+api_name += "bool has_%s,\n" % c_var(argname)
+api_name += "".ljust(l)
+
+if argentry == "str":
+api_name += "const "
+api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
+api_name += "".ljust(l)
+
+api_name += "Error **errp)"
+return api_name;
+
+
+# Following are the core functions that transate user input into a qdict going


s/transate/translate

Although the comment doesn't make much sense to me.



  OK, let me improve.


+# to be emitted in the wire.
+
+def generate_event_declaration(api_name):
+return mcgen('''
+
+%(api_name)s;
+''',
+ api_name = api_name)
+
+def generate_event_implement(api_name, event_name, params):


I wonder if it would be clearer to to generate the declaration here.


  I think so, but a little slower at compile time. I use api_name to
avoid call _generate_event_api_name() twice. I am not sure which
way is better.




Re: [Qemu-devel] [RFC PATCH V2 2/5] qapi: add event helper functions

2014-01-06 Thread Wenchao Xia

于 2014/1/7 6:23, Luiz Capitulino 写道:

On Fri,  3 Jan 2014 07:10:31 +0800
Wenchao Xia  wrote:


This file hold some functions that do not need to be generated.

Signed-off-by: Wenchao Xia 
---
  include/qapi/qmp-event.h |   22 ++
  qapi/Makefile.objs   |1 +
  qapi/qmp-event.c |   56 ++
  3 files changed, 79 insertions(+), 0 deletions(-)
  create mode 100644 include/qapi/qmp-event.h
  create mode 100644 qapi/qmp-event.c

diff --git a/include/qapi/qmp-event.h b/include/qapi/qmp-event.h
new file mode 100644
index 000..2baf093
--- /dev/null
+++ b/include/qapi/qmp-event.h
@@ -0,0 +1,22 @@
+/*
+ * QMP Event related
+ *
+ * Copyright IBM, Corp. 2014
+ *
+ * Authors:
+ *  Wenchao Xia   
+ *
+ * This work is licensed under the terms of the GNU GPLv2+ or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QMP_EVENT_H
+#define QMP_EVENT_H
+
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+
+QDict *qmp_event_build_dict(const char *event_name);
+
+#endif
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 1f9c973..d14b769 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
  util-obj-y += string-input-visitor.o string-output-visitor.o

  util-obj-y += opts-visitor.o
+util-obj-y += qmp-event.o
diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
new file mode 100644
index 000..dc81ec2
--- /dev/null
+++ b/qapi/qmp-event.c
@@ -0,0 +1,56 @@
+/*
+ * QMP Event related
+ *
+ * Copyright IBM, Corp. 2014
+ *
+ * Authors:
+ *  Wenchao Xia   
+ *
+ * This work is licensed under the terms of the GNU GPLv2+ or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include 
+
+#include "qemu-common.h"
+#include "qapi/qmp-event.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qjson.h"
+
+#ifdef _WIN32
+#include "sysemu/os-win32.h"
+#endif
+
+#ifdef CONFIG_POSIX
+#include "sysemu/os-posix.h"
+#endif
+
+static void timestamp_put(QDict *qdict)
+{
+int err;
+QObject *obj;
+qemu_timeval tv;
+
+err = qemu_gettimeofday(&tv);
+if (err < 0) {
+return;
+}


Hmm, I see this has always existed (and I guess I did it myself), but it's
not quite right. Sending an event w/o time info wouldn't be complaint to
the protocol spec. It's a good idea to fix this now. We have three options:

  1. abort()

  2. Skip sending the event altogether

  3. Add a bogus time value (say seconds=0 and microseconds=0)

I don't know what's best, but I guess I'd do item 3. Although I wonder
if zero is any better then no info at all (it's certainly complaint, but
not a valid info). Maybe skip the event then?



  I think user want a way, to know error happens. If it is
skepted, then we should report it in stderr or a special message in
monitor. In my opinion, sending an event would be the easist way.
We can set time to zero and doc "timestamp == 0 means failure in
getting host time", and this avoid impacting existing user who always
seeking key "timestamp".




+
+obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
+"'microseconds': %" PRId64 " }",
+(int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
+qdict_put_obj(qdict, "timestamp", obj);
+}
+
+/*
+ * Build a QDict, then fill event name and time stamp, caller should free the
+ * QDict after usage.
+ */
+QDict *qmp_event_build_dict(const char *event_name)
+{
+QDict *dict = qdict_new();
+qdict_put(dict, "event", qstring_from_str(event_name));
+timestamp_put(dict);
+return dict;
+}







Re: [Qemu-devel] [PATCHv3 3/6] ui/vnc: optimize dirty bitmap tracking

2014-01-06 Thread Wenchao Xia
于 2014/1/6 21:31, Peter Lieven 写道:
> On 06.01.2014 11:08, Wenchao Xia wrote:
>> 于 2014/1/6 2:02, Peter Lieven 写道:
>>> vnc_update_client currently scans the dirty bitmap of each client
>>> bitwise which is a very costly operation if only few bits are dirty.
>>> vnc_refresh_server_surface does almost the same.
>>> this patch optimizes both by utilizing the heavily optimized
>>> function find_next_bit to find the offset of the next dirty
>>> bit in the dirty bitmaps.
>>>
>>> The following artifical test (just the bitmap operation part) running
>>> vnc_update_client 65536 times on a 2560x2048 surface illustrates the
>>> performance difference:
>>>
>>> All bits clean - vnc_update_client_new: 0.07 secs
>>>vnc_update_client_old: 10.98 secs
>>>
>>> All bits dirty - vnc_update_client_new: 11.26 secs
>>>vnc_update_client_old: 20.19 secs
>>>
>>> Few bits dirty - vnc_update_client_new: 0.08 secs
>>>vnc_update_client_old: 10.98 secs
>>>
>>> The case for all bits dirty is still rather slow, this
>>> is due to the implementation of find_and_clear_dirty_height.
>>> This will be addresses in a separate patch.
>>>
>>> Signed-off-by: Peter Lieven 
>>> ---
>>>ui/vnc.c |  154 
>>> +-
>>>ui/vnc.h |4 ++
>>>2 files changed, 87 insertions(+), 71 deletions(-)
>>>
>>> diff --git a/ui/vnc.c b/ui/vnc.c
>>> index 1d2aa1a..6a0c03e 100644
>>> --- a/ui/vnc.c
>>> +++ b/ui/vnc.c
>>> @@ -572,6 +572,14 @@ void *vnc_server_fb_ptr(VncDisplay *vd, int x, int y)
>>>ptr += x * VNC_SERVER_FB_BYTES;
>>>return ptr;
>>>}
>>> +/* this sets only the visible pixels of a dirty bitmap */
>>> +#define VNC_SET_VISIBLE_PIXELS_DIRTY(bitmap, w, h) {\
>>> +int y;\
>>> +memset(bitmap, 0x00, sizeof(bitmap));\
>>> +for (y = 0; y < h; y++) {\
>>> +bitmap_set(bitmap[y], 0, w / VNC_DIRTY_PIXELS_PER_BIT);\
>>Will it be a problem when vnc's width % VNC_DIRTY_PIXELS_PER_BIT != 0?
>> Although it is a rare case, but I think it is better round it up, since
>> "v" and "VNC_DIRTY_PIXELS_PER_BIT" are variables. A macro computing it
>> would be nice:
> Good point, I will use DIV_ROUND_UP here.
>>
>> #define VNC_DIRTY_BITS_FROM_WIDTH(w) (w + VNC_DIRTY_PIXELS_PER_BIT - 1/
>> VNC_DIRTY_PIXELS_PER_BIT)
>> #define VNC_DIRTY_BITS (VNC_DIRTY_BITS_FROM_WIDTH(VNC_MAX_WIDTH)
>>
>> then here:
>>  bitmap_set(bitmap[y], 0, VNC_DIRTY_BITS_FROM_WIDTH(w));
>>
>> Or simply warn or coredump when v % VNC_DIRTY_PIXELS_PER_BIT != 0.
>>
>> Also, in vnc.h:
>> /* VNC_MAX_WIDTH must be a multiple of 16. */
>> #define VNC_MAX_WIDTH 2560
>> #define VNC_MAX_HEIGHT 2048
>>
>> Maybe it should be updated as:
>> /* VNC_MAX_WIDTH must be a multiple of VNC_DIRTY_PIXELS_PER_BIT. */
> correct. will fix as well.
>>
>>> +} \
>>> +}
>>>
>>>static void vnc_dpy_switch(DisplayChangeListener *dcl,
>>>   DisplaySurface *surface)
>>> @@ -597,7 +605,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
>>>qemu_pixman_image_unref(vd->guest.fb);
>>>vd->guest.fb = pixman_image_ref(surface->image);
>>>vd->guest.format = surface->format;
>>> -memset(vd->guest.dirty, 0xFF, sizeof(vd->guest.dirty));
>>> +VNC_SET_VISIBLE_PIXELS_DIRTY(vd->guest.dirty,
>>> + surface_width(vd->ds),
>>> + surface_height(vd->ds));
>>>
>>>QTAILQ_FOREACH(vs, &vd->clients, next) {
>>>vnc_colordepth(vs);
>>> @@ -605,7 +615,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
>>>if (vs->vd->cursor) {
>>>vnc_cursor_define(vs);
>>>}
>>> -memset(vs->dirty, 0xFF, sizeof(vs->dirty));
>>> +VNC_SET_VISIBLE_PIXELS_DIRTY(vs->dirty,
>>> + surface_width(vd->ds),
>>> + surface_height(vd->ds));
>>>}
>>>}
>>>
>>> @@ -889,10 +901,9 @@ static int vnc_update_client(VncState *vs, int 
>>> has_dirty)
>>>VncDisplay *vd = vs->vd;
>>>VncJob *job;
>>>int y;
>>> -int width, height;
>>> +int height;
>>>int n = 0;
>>>
>>> -
>>>if (vs->output.offset && !vs->audio_cap && !vs->force_update)
>>>/* kernel send buffers are full -> drop frames to throttle */
>>>return 0;
>>> @@ -908,39 +919,27 @@ static int vnc_update_client(VncState *vs, int 
>>> has_dirty)
>>> */
>>>job = vnc_job_new(vs);
>>>
>>> -width = MIN(pixman_image_get_width(vd->server), vs->client_width);
>>>height = MIN(pixman_image_get_height(vd->server), 
>>> vs->client_height);
>>>
>>> -for (y = 0; y < height; y++) {
>>> -int x;
>>> -int last_x = -1;
>>> -for (x = 0; x < width / VNC_DIRTY_PIXELS_PER_BIT; x++) {
>>> -  

Re: [Qemu-devel] [RFC PATCH V2 3/5] qapi script: add event support by qapi-event.py

2014-01-06 Thread Luiz Capitulino
On Fri,  3 Jan 2014 07:10:32 +0800
Wenchao Xia  wrote:

> qapi-event.py will parse the schema and generate qapi-event.c, then
> the API in qapi-event.c can be used to handle event in qemu code.
> All API have prefix "qapi_event", all types have prefix "QAPIEvent".
> Examples can be found in following patches.
> 
> The script mainly include three parts: generate API for each event
> define, generate an enum type for all defined event, generate behavior
> control functions.
> 
> Since in some case the real emit behavior may change, for example,
> qemu-img would not send a event, a callback layer is added to
> control the behavior. As a result, the stubs at compile time
> can be saved, the binding of block layer code and monitor code
> will become looser.
> 
> Signed-off-by: Wenchao Xia 
> ---
>  Makefile  |9 +-
>  Makefile.objs |2 +-
>  scripts/qapi-event.py |  432 
> +
>  3 files changed, 439 insertions(+), 4 deletions(-)
>  create mode 100644 scripts/qapi-event.py
> 
> diff --git a/Makefile b/Makefile
> index bdff4e4..fa59765 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -45,8 +45,8 @@ endif
>  endif
>  
>  GENERATED_HEADERS = config-host.h qemu-options.def
> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>  
>  GENERATED_HEADERS += trace/generated-events.h
>  GENERATED_SOURCES += trace/generated-events.c
> @@ -185,7 +185,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
>  # Build libraries
>  
>  libqemustub.a: $(stub-obj-y)
> -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
>  
>  ##
>  
> @@ -226,6 +226,9 @@ $(SRC_PATH)/qapi-schema.json 
> $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
>  qapi-visit.c qapi-visit.h :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
> $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> +qapi-event.c qapi-event.h :\
> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py 
> $(gen-out-type) -o "." -b < $<, "  GEN   $@")
>  qmp-commands.h qmp-marshal.c :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
> $(gen-out-type) -m -o "." < $<, "  GEN   $@")
> diff --git a/Makefile.objs b/Makefile.objs
> index 2b6c1fe..33f5950 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
>  block-obj-$(CONFIG_POSIX) += aio-posix.o
>  block-obj-$(CONFIG_WIN32) += aio-win32.o
>  block-obj-y += block/
> -block-obj-y += qapi-types.o qapi-visit.o
> +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
>  block-obj-y += qemu-io-cmds.o
>  
>  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> new file mode 100644
> index 000..7526366
> --- /dev/null
> +++ b/scripts/qapi-event.py
> @@ -0,0 +1,432 @@
> +#
> +# QAPI event generator
> +#
> +# Copyright IBM, Corp. 2014
> +#
> +# Authors:
> +#  Wenchao Xia 
> +#
> +# This work is licensed under the terms of the GNU GPLv2+ or later.
> +# See the COPYING.LIB file in the top-level directory.
> +
> +from ordereddict import OrderedDict
> +from qapi import *
> +import sys
> +import os
> +import getopt
> +import errno
> +
> +def _generate_event_api_name(event_name, params):

Why the underline? And, what you generate is a function declaration...

> +api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
> +l = len(api_name)
> +
> +if params:
> +for argname, argentry, optional, structured in parse_args(params):
> +if structured:
> +sys.stderr.write("Nested structure define in event is not "
> + "supported now, event '%s', argname '%s'\n" 
> %
> + (event_name, argname))
> +sys.exit(1)
> +continue
> +
> +if optional:
> +api_name += "bool has_%s,\n" % c_var(argname)
> +api_name += "".ljust(l)
> +
> +if argentry == "str":
> +api_name += "const "
> +api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
> +api_name += "".ljust(l)
> +
> +api_name += "Error **errp)"
> +return api_name;
> +
> +
> +# Following are the core functions that transate user input into a qdict 
> going

s/transate/translate

Although t

Re: [Qemu-devel] [RFC PATCH V2 0/5] add direct support of event in qapi schema

2014-01-06 Thread Luiz Capitulino
On Fri,  3 Jan 2014 07:10:29 +0800
Wenchao Xia  wrote:

> This series add support for tag/keyword 'event' in qapi-schema.
> A new file was created to store some helper functions in patch 2, patch 4 is
> the test case, patch 5 is a convert example.
> 
> The implemention is done by generate API and a batch of parameters for each
> event define, it doesn't generate a struture and visit function in the
> background for every event, so it doesn't support nested structure in the
> define to avoid trouble. A callback layer is added to control the behavior.
> More detail can be found in patch 3's message and incode comments.

The general approach seems good to me. Would be nice to get another
reviewer though, maybe Eric and/or Michael.



Re: [Qemu-devel] [RFC PATCH V2 3/5] qapi script: add event support by qapi-event.py

2014-01-06 Thread Luiz Capitulino

[Pressed enter too soon, forgot two things]

On Mon, 6 Jan 2014 18:10:04 -0500
Luiz Capitulino  wrote:

> On Fri,  3 Jan 2014 07:10:32 +0800
> Wenchao Xia  wrote:
> 
> > qapi-event.py will parse the schema and generate qapi-event.c, then
> > the API in qapi-event.c can be used to handle event in qemu code.
> > All API have prefix "qapi_event", all types have prefix "QAPIEvent".
> > Examples can be found in following patches.
> > 
> > The script mainly include three parts: generate API for each event
> > define, generate an enum type for all defined event, generate behavior
> > control functions.
> > 
> > Since in some case the real emit behavior may change, for example,
> > qemu-img would not send a event, a callback layer is added to
> > control the behavior. As a result, the stubs at compile time
> > can be saved, the binding of block layer code and monitor code
> > will become looser.
> > 
> > Signed-off-by: Wenchao Xia 
> > ---
> >  Makefile  |9 +-
> >  Makefile.objs |2 +-
> >  scripts/qapi-event.py |  432 
> > +
> >  3 files changed, 439 insertions(+), 4 deletions(-)
> >  create mode 100644 scripts/qapi-event.py
> > 
> > diff --git a/Makefile b/Makefile
> > index bdff4e4..fa59765 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -45,8 +45,8 @@ endif
> >  endif
> >  
> >  GENERATED_HEADERS = config-host.h qemu-options.def
> > -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> > -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
> > +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> > +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
> >  
> >  GENERATED_HEADERS += trace/generated-events.h
> >  GENERATED_SOURCES += trace/generated-events.c
> > @@ -185,7 +185,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
> >  # Build libraries
> >  
> >  libqemustub.a: $(stub-obj-y)
> > -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> > +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
> >  
> >  ##
> >  
> > @@ -226,6 +226,9 @@ $(SRC_PATH)/qapi-schema.json 
> > $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> >  qapi-visit.c qapi-visit.h :\
> >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
> > $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> > +qapi-event.c qapi-event.h :\
> > +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> > +   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py 
> > $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> >  qmp-commands.h qmp-marshal.c :\
> >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py 
> > $(qapi-py)
> > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
> > $(gen-out-type) -m -o "." < $<, "  GEN   $@")
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 2b6c1fe..33f5950 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
> >  block-obj-$(CONFIG_POSIX) += aio-posix.o
> >  block-obj-$(CONFIG_WIN32) += aio-win32.o
> >  block-obj-y += block/
> > -block-obj-y += qapi-types.o qapi-visit.o
> > +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
> >  block-obj-y += qemu-io-cmds.o
> >  
> >  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> > diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> > new file mode 100644
> > index 000..7526366
> > --- /dev/null
> > +++ b/scripts/qapi-event.py
> > @@ -0,0 +1,432 @@
> > +#
> > +# QAPI event generator
> > +#
> > +# Copyright IBM, Corp. 2014
> > +#
> > +# Authors:
> > +#  Wenchao Xia 
> > +#
> > +# This work is licensed under the terms of the GNU GPLv2+ or later.
> > +# See the COPYING.LIB file in the top-level directory.
> > +
> > +from ordereddict import OrderedDict
> > +from qapi import *
> > +import sys
> > +import os
> > +import getopt
> > +import errno
> > +
> > +def _generate_event_api_name(event_name, params):
> 
> Why the underline? And, what you generate is a function declaration...
> 
> > +api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
> > +l = len(api_name)
> > +
> > +if params:
> > +for argname, argentry, optional, structured in parse_args(params):
> > +if structured:
> > +sys.stderr.write("Nested structure define in event is not "
> > + "supported now, event '%s', argname 
> > '%s'\n" %
> > + (event_name, argname))
> > +sys.exit(1)
> > +continue
> > +
> > +if optional:
> > +api_name += "bool has_%s,\n" % c_var(argname)
> > +api_name += "".ljust(l)
> > +
> > +if argentry =

Re: [Qemu-devel] [PATCH] spapr-pci: remove io ports workaround

2014-01-06 Thread Alexey Kardashevskiy
On 01/06/2014 10:12 PM, Greg Kurz wrote:
> On Fri, 03 Jan 2014 09:08:21 +1100
> Alexey Kardashevskiy  wrote:
>>
>> Please read the rest of this thread. It does not visibly break things but
>> with this patch QEMU starts calling unassigned_mem_accepts() (normally
>> silent) which is not a good sign.
>>
>>
>>
> 
> Hmm... this is only because this patch moves the PHB io region from the
> system IO to the system memory space, but the bogus(?) write to unassigned
> memory already exists.
> 
> I have tested against the current ppc-next (62d529a), with no
> additional patch:
> 
> qemu-system-ppc64 \
> -snapshot -S -monitor stdio -serial pty \
> -nographic -nodefaults \
> -machine type=pseries,accel=kvm -smp 1 -m 4G \
> -device virtio-blk-pci,id=virtioiblk0,drive=drive0,bootindex=20,ioeventfd=on \
> -drive file=/local/greg/qemu/fedora-be.qcow2,if=none,id=drive0,readonly=off,\
>format=qcow2,media=disk,werror=stop,rerror=stop,discard=on
> 
> where fedora-be.qcow2 contains a stock fedora 19 for ppc64.
> 
> I have attached gdb to qemu and set a breakpoint in unassigned_io_write(), and
> here is what I get again:
> 
> (gdb) b unassigned_io_write 
> Breakpoint 1 at 0x1045d308: file 
> /home/greg/Work/ibm/linux/qemu-agraf/ioport.c, line 54.
> (gdb) c
> Continuing.
> [Thread 0x1c5deef0 (LWP 11946) exited]
> [New Thread 0x1c5deef0 (LWP 11955)]
> [Switching to Thread 0x1bdaeef0 (LWP 11947)]
> 
> Breakpoint 1, unassigned_io_write (opaque=0x0, addr=82, val=128, size=1) at 
> /home/greg/Work/ibm/linux/qemu-agraf/ioport.c:54
> 54  {
> (gdb) where
> #0  unassigned_io_write (opaque=0x0, addr=82, val=128, size=1) at 
> /home/greg/Work/ibm/linux/qemu-agraf/ioport.c:54
> #1  0x10468f38 in memory_region_write_accessor (mr=0x10027615940, 
> addr=82, value=0x1bdadd68, size=1, shift=0, mask=255) at 
> /home/greg/Work/ibm/linux/qemu-agraf/memory.c:440
> #2  0x104690c4 in access_with_adjusted_size (addr=82, 
> value=0x1bdadd68, size=1, access_size_min=1, access_size_max=4, 
> access=@0x107ca670: 0x10468e5c , 
> mr=0x10027615940)
> at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:472
> #3  0x1046bc64 in memory_region_dispatch_write (mr=0x10027615940, 
> addr=82, data=128, size=1) at 
> /home/greg/Work/ibm/linux/qemu-agraf/memory.c:984
> #4  0x1046fdc4 in io_mem_write (mr=0x10027615940, addr=82, val=128, 
> size=1) at /home/greg/Work/ibm/linux/qemu-agraf/memory.c:1749
> #5  0x103aca0c in address_space_rw (as=0x10c19638 
> , addr=1101659111506, buf=0x1bdae117 "\200", len=1, 
> is_write=true) at /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2002
> #6  0x103acf3c in cpu_physical_memory_rw (addr=1101659111506, 
> buf=0x1bdae117 "\200", len=1, is_write=1) at 
> /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2071
> #7  0x103a44c4 in cpu_physical_memory_write (addr=1101659111506, 
> buf=0x1bdae117, len=1) at 
> /home/greg/Work/ibm/linux/qemu-agraf/include/exec/cpu-common.h:68
> #8  0x103aeb2c in stb_phys (addr=1101659111506, val=128) at 
> /home/greg/Work/ibm/linux/qemu-agraf/exec.c:2600
> #9  0x10438550 in h_logical_store (cpu=0x10027d0f0d0, 
> spapr=0x100276bb210, opcode=64, args=0x1ff80030) at 
> /home/greg/Work/ibm/linux/qemu-agraf/hw/ppc/spapr_hcall.c:564
> #10 0x10438e74 in spapr_hypercall (cpu=0x10027d0f0d0, opcode=64, 
> args=0x1ff80030) at 
> /home/greg/Work/ibm/linux/qemu-agraf/hw/ppc/spapr_hcall.c:737
> #11 0x104cf424 in kvm_arch_handle_exit (cs=0x10027d0f0d0, 
> run=0x1ff8) at 
> /home/greg/Work/ibm/linux/qemu-agraf/target-ppc/kvm.c:1223
> #12 0x104648a4 in kvm_cpu_exec (cpu=0x10027d0f0d0) at 
> /home/greg/Work/ibm/linux/qemu-agraf/kvm-all.c:1736
> #13 0x10397f00 in qemu_kvm_cpu_thread_fn (arg=0x10027d0f0d0) at 
> /home/greg/Work/ibm/linux/qemu-agraf/cpus.c:874
> #14 0x1f92c29c in start_thread (arg=0x1bdaeef0) at 
> pthread_create.c:310
> #15 0x1de5de10 in .__clone ()
> at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:111
> 
> All I can say for the moment, is that I don't get that if I run qemu with
> -kernel/-append/-initrd instead of following the grub2 path.
> 
> Any clue ?


I've got nothing... Can you try without "ioeventfd=on"?
If you post gdb output next time, do "set radix 0x10" first :)


-- 
Alexey



[Qemu-devel] [PULL 09/14] qdev: Delete dead code

2014-01-06 Thread Luiz Capitulino
From: Peter Crosthwaite 

This is unreachable code, as it's already asserted that no errors have
occurred. Delete.

Signed-off-by: Peter Crosthwaite 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 hw/core/qdev.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e374a93..adbff18 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -746,11 +746,6 @@ static void device_initfn(Object *obj)
 }
 class = object_class_get_parent(class);
 } while (class != object_class_by_name(TYPE_DEVICE));
-if (err != NULL) {
-qerror_report_err(err);
-error_free(err);
-exit(1);
-}
 
 object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
  (Object **)&dev->parent_bus, &err);
-- 
1.8.1.4




[Qemu-devel] [PULL 12/14] qemu-option: Remove qemu_opts_create_nofail

2014-01-06 Thread Luiz Capitulino
From: Peter Crosthwaite 

This is a boiler-plate _nofail variant of qemu_opts_create. Remove and
use error_abort in call sites.

null/0 arguments needs to be added for the id and fail_if_exists fields
in affected callsites due to argument inconsistency between the normal and
no_fail variants.

Signed-off-by: Peter Crosthwaite 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 block/blkdebug.c   |  2 +-
 block/blkverify.c  |  2 +-
 block/curl.c   |  2 +-
 block/gluster.c|  2 +-
 block/iscsi.c  |  2 +-
 block/nbd.c|  3 ++-
 block/qcow2.c  |  2 +-
 block/raw-posix.c  |  2 +-
 block/raw-win32.c  |  5 +++--
 block/rbd.c|  2 +-
 block/sheepdog.c   |  2 +-
 block/vvfat.c  |  2 +-
 blockdev.c |  6 --
 hw/watchdog/watchdog.c |  3 ++-
 include/qemu/option.h  |  1 -
 qdev-monitor.c |  2 +-
 qemu-img.c |  2 +-
 util/qemu-config.c |  2 +-
 util/qemu-option.c |  9 -
 util/qemu-sockets.c| 18 +-
 vl.c   | 15 +--
 21 files changed, 42 insertions(+), 44 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 957be2c..ebc5f13 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -359,7 +359,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
*options, int flags,
 const char *filename, *config;
 int ret;
 
-opts = qemu_opts_create_nofail(&runtime_opts);
+opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
 qemu_opts_absorb_qdict(opts, options, &local_err);
 if (error_is_set(&local_err)) {
 error_propagate(errp, local_err);
diff --git a/block/blkverify.c b/block/blkverify.c
index 3c63528..1c1637f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -125,7 +125,7 @@ static int blkverify_open(BlockDriverState *bs, QDict 
*options, int flags,
 const char *filename, *raw;
 int ret;
 
-opts = qemu_opts_create_nofail(&runtime_opts);
+opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
 qemu_opts_absorb_qdict(opts, options, &local_err);
 if (error_is_set(&local_err)) {
 error_propagate(errp, local_err);
diff --git a/block/curl.c b/block/curl.c
index 5a46f97..a603936 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -413,7 +413,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 return -EROFS;
 }
 
-opts = qemu_opts_create_nofail(&runtime_opts);
+opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
 qemu_opts_absorb_qdict(opts, options, &local_err);
 if (error_is_set(&local_err)) {
 qerror_report_err(local_err);
diff --git a/block/gluster.c b/block/gluster.c
index 877686a..563d497 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -298,7 +298,7 @@ static int qemu_gluster_open(BlockDriverState *bs,  QDict 
*options,
 Error *local_err = NULL;
 const char *filename;
 
-opts = qemu_opts_create_nofail(&runtime_opts);
+opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
 qemu_opts_absorb_qdict(opts, options, &local_err);
 if (error_is_set(&local_err)) {
 qerror_report_err(local_err);
diff --git a/block/iscsi.c b/block/iscsi.c
index fa69408..02eba5d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1109,7 +1109,7 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 return -EINVAL;
 }
 
-opts = qemu_opts_create_nofail(&runtime_opts);
+opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
 qemu_opts_absorb_qdict(opts, options, &local_err);
 if (error_is_set(&local_err)) {
 qerror_report_err(local_err);
diff --git a/block/nbd.c b/block/nbd.c
index 4455a13..327e913 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -205,7 +205,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options, char 
**export)
 return -EINVAL;
 }
 
-s->socket_opts = qemu_opts_create_nofail(&socket_optslist);
+s->socket_opts = qemu_opts_create(&socket_optslist, NULL, 0,
+  &error_abort);
 
 qemu_opts_absorb_qdict(s->socket_opts, options, &local_err);
 if (error_is_set(&local_err)) {
diff --git a/block/qcow2.c b/block/qcow2.c
index f29aa88..8ec9db1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -669,7 +669,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 
 /* Enable lazy_refcounts according to image and command line options */
-opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
+opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 0, &error_abort);
 qemu_opts_absorb_qdict(opts, options, &local_err);
 if (error_is_set(&local_err)) {
 error_propagate(errp, local_err);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 10c6b34..0676037 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -287,7 +287,7 @@ static int raw_open_common(BlockDriverState *bs

Re: [Qemu-devel] [RFC PATCH V2 2/5] qapi: add event helper functions

2014-01-06 Thread Luiz Capitulino
On Fri,  3 Jan 2014 07:10:31 +0800
Wenchao Xia  wrote:

> This file hold some functions that do not need to be generated.
> 
> Signed-off-by: Wenchao Xia 
> ---
>  include/qapi/qmp-event.h |   22 ++
>  qapi/Makefile.objs   |1 +
>  qapi/qmp-event.c |   56 
> ++
>  3 files changed, 79 insertions(+), 0 deletions(-)
>  create mode 100644 include/qapi/qmp-event.h
>  create mode 100644 qapi/qmp-event.c
> 
> diff --git a/include/qapi/qmp-event.h b/include/qapi/qmp-event.h
> new file mode 100644
> index 000..2baf093
> --- /dev/null
> +++ b/include/qapi/qmp-event.h
> @@ -0,0 +1,22 @@
> +/*
> + * QMP Event related
> + *
> + * Copyright IBM, Corp. 2014
> + *
> + * Authors:
> + *  Wenchao Xia   
> + *
> + * This work is licensed under the terms of the GNU GPLv2+ or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef QMP_EVENT_H
> +#define QMP_EVENT_H
> +
> +#include "qapi/error.h"
> +#include "qapi/qmp/qdict.h"
> +
> +QDict *qmp_event_build_dict(const char *event_name);
> +
> +#endif
> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> index 1f9c973..d14b769 100644
> --- a/qapi/Makefile.objs
> +++ b/qapi/Makefile.objs
> @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o 
> qmp-dispatch.o
>  util-obj-y += string-input-visitor.o string-output-visitor.o
>  
>  util-obj-y += opts-visitor.o
> +util-obj-y += qmp-event.o
> diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
> new file mode 100644
> index 000..dc81ec2
> --- /dev/null
> +++ b/qapi/qmp-event.c
> @@ -0,0 +1,56 @@
> +/*
> + * QMP Event related
> + *
> + * Copyright IBM, Corp. 2014
> + *
> + * Authors:
> + *  Wenchao Xia   
> + *
> + * This work is licensed under the terms of the GNU GPLv2+ or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include 
> +
> +#include "qemu-common.h"
> +#include "qapi/qmp-event.h"
> +#include "qapi/qmp/qstring.h"
> +#include "qapi/qmp/qjson.h"
> +
> +#ifdef _WIN32
> +#include "sysemu/os-win32.h"
> +#endif
> +
> +#ifdef CONFIG_POSIX
> +#include "sysemu/os-posix.h"
> +#endif
> +
> +static void timestamp_put(QDict *qdict)
> +{
> +int err;
> +QObject *obj;
> +qemu_timeval tv;
> +
> +err = qemu_gettimeofday(&tv);
> +if (err < 0) {
> +return;
> +}

Hmm, I see this has always existed (and I guess I did it myself), but it's
not quite right. Sending an event w/o time info wouldn't be complaint to
the protocol spec. It's a good idea to fix this now. We have three options:

 1. abort()

 2. Skip sending the event altogether

 3. Add a bogus time value (say seconds=0 and microseconds=0)

I don't know what's best, but I guess I'd do item 3. Although I wonder
if zero is any better then no info at all (it's certainly complaint, but
not a valid info). Maybe skip the event then?


> +
> +obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
> +"'microseconds': %" PRId64 " }",
> +(int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
> +qdict_put_obj(qdict, "timestamp", obj);
> +}
> +
> +/*
> + * Build a QDict, then fill event name and time stamp, caller should free the
> + * QDict after usage.
> + */
> +QDict *qmp_event_build_dict(const char *event_name)
> +{
> +QDict *dict = qdict_new();
> +qdict_put(dict, "event", qstring_from_str(event_name));
> +timestamp_put(dict);
> +return dict;
> +}




[Qemu-devel] [PULL 04/14] qom: fix leak for objects created with -object

2014-01-06 Thread Luiz Capitulino
From: Paolo Bonzini 

The object must be unref-ed when its variable goes out of scope.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Igor Mammedov 
Tested-by: Igor Mammedov 
Signed-off-by: Luiz Capitulino 
---
 vl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 0f67545..2170a5e 100644
--- a/vl.c
+++ b/vl.c
@@ -2810,12 +2810,13 @@ static int object_create(QemuOpts *opts, void *opaque)
 
 obj = object_new(type);
 if (qemu_opt_foreach(opts, object_set_property, obj, 1) < 0) {
+object_unref(obj);
 return -1;
 }
 
 object_property_add_child(container_get(object_get_root(), "/objects"),
   id, obj, NULL);
-
+object_unref(obj);
 return 0;
 }
 
-- 
1.8.1.4




[Qemu-devel] [PULL 02/14] qemu-monitor: HMP cpu-add wrapper

2014-01-06 Thread Luiz Capitulino
From: "Jason J. Herne" 

Add HMP cpu-add wrapper to allow cpu hot plugging via monitor.

Signed-off-by: Jason J. Herne 
Reviewed-by: Igor Mammedov 
Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx | 13 +
 hmp.c   | 10 ++
 hmp.h   |  1 +
 3 files changed, 24 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ebe8e78..929550d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1620,6 +1620,19 @@ Executes a qemu-io command on the given block device.
 ETEXI
 
 {
+.name   = "cpu-add",
+.args_type  = "id:i",
+.params = "id",
+.help   = "add cpu",
+.mhandler.cmd  = hmp_cpu_add,
+},
+
+STEXI
+@item cpu-add @var{id}
+Add CPU with id @var{id}
+ETEXI
+
+{
 .name   = "info",
 .args_type  = "item:s?",
 .params = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 32ee285..c513f9b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1525,6 +1525,16 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict 
*qdict)
 hmp_handle_error(mon, &errp);
 }
 
+void hmp_cpu_add(Monitor *mon, const QDict *qdict)
+{
+int cpuid;
+Error *err = NULL;
+
+cpuid = qdict_get_int(qdict, "id");
+qmp_cpu_add(cpuid, &err);
+hmp_handle_error(mon, &err);
+}
+
 void hmp_chardev_add(Monitor *mon, const QDict *qdict)
 {
 const char *args = qdict_get_str(qdict, "args");
diff --git a/hmp.h b/hmp.h
index 54cf71f..f92fc89 100644
--- a/hmp.h
+++ b/hmp.h
@@ -89,5 +89,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
 void hmp_chardev_add(Monitor *mon, const QDict *qdict);
 void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 
 #endif
-- 
1.8.1.4




[Qemu-devel] [PULL 01/14] vl: add missing transition debug->finish_migrate

2014-01-06 Thread Luiz Capitulino
From: Paolo Bonzini 

This fixes an abort if you invoke the "migrate" command while the
guest is being debugged.

Cc: qemu-sta...@nongnu.org
Cc: lcapitul...@redhat.com
Signed-off-by: Paolo Bonzini 
Signed-off-by: Luiz Capitulino 
---
 vl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/vl.c b/vl.c
index 7511e70..0f67545 100644
--- a/vl.c
+++ b/vl.c
@@ -591,6 +591,7 @@ typedef struct {
 static const RunStateTransition runstate_transitions_def[] = {
 /* from  -> to  */
 { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
+{ RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
 
 { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
 { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
-- 
1.8.1.4




Re: [Qemu-devel] [PULL 00/14] QMP queue

2014-01-06 Thread Peter Maydell
On 6 January 2014 22:03, Luiz Capitulino  wrote:
> Peter Crosthwaite (6):
>   error: Add error_abort
>   qdev: Delete dead code
>   hw: Remove assert_no_error usages
>   target-i386: Remove assert_no_error usage
>   qemu-option: Remove qemu_opts_create_nofail
>   qerror: Remove assert_no_error()
>
>  target-arm/cpu.c |  7 ++--
>  target-i386/cpu.c|  4 +--

It took me a while to figure out where the target-arm/ change in this
pullreq was, because it's been stuffed into the "hw:" patch (whereas
target-i386 got its own patch). However, the change itself is OK so
it's fine.

thanks
-- PMM



[Qemu-devel] [PULL 05/14] qom: catch errors in object_property_add_child

2014-01-06 Thread Luiz Capitulino
From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Reviewed-by: Igor Mammedov 
Tested-by: Igor Mammedov 
Signed-off-by: Luiz Capitulino 
---
 qom/object.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index fc19cf6..4dee02b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -988,17 +988,22 @@ static void object_finalize_child_property(Object *obj, 
const char *name,
 void object_property_add_child(Object *obj, const char *name,
Object *child, Error **errp)
 {
+Error *local_err = NULL;
 gchar *type;
 
 type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child)));
 
-object_property_add(obj, name, type, object_get_child_property,
-NULL, object_finalize_child_property, child, errp);
-
+object_property_add(obj, name, type, object_get_child_property, NULL,
+object_finalize_child_property, child, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+goto out;
+}
 object_ref(child);
 g_assert(child->parent == NULL);
 child->parent = obj;
 
+out:
 g_free(type);
 }
 
-- 
1.8.1.4




[Qemu-devel] [PULL 03/14] rng: initialize file descriptor to -1

2014-01-06 Thread Luiz Capitulino
From: Paolo Bonzini 

The file descriptor is never initialized to -1, which makes rng-random
close stdin if an object is created and immediately destroyed.  If we
change it to -1, we also need to protect qemu_set_fd_handler from
receiving a bogus file descriptor.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Igor Mammedov 
Tested-by: Igor Mammedov 
Signed-off-by: Luiz Capitulino 
---
 backends/rng-random.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/backends/rng-random.c b/backends/rng-random.c
index 68dfc8a..136499d 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -123,15 +123,15 @@ static void rng_random_init(Object *obj)
 NULL);
 
 s->filename = g_strdup("/dev/random");
+s->fd = -1;
 }
 
 static void rng_random_finalize(Object *obj)
 {
 RndRandom *s = RNG_RANDOM(obj);
 
-qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
-
 if (s->fd != -1) {
+qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
 qemu_close(s->fd);
 }
 
-- 
1.8.1.4




Re: [Qemu-devel] [PATCHv2 04/18] qemu-iotests: fix test 013 to work with any protocol

2014-01-06 Thread Peter Lieven


> Am 06.01.2014 um 21:40 schrieb Jeff Cody :
> 
>> On Mon, Jan 06, 2014 at 07:48:25AM +0100, Peter Lieven wrote:
>>> On 06.01.2014 06:31, Fam Zheng wrote:
 On 2014年01月06日 01:21, Peter Lieven wrote:
 Signed-off-by: Peter Lieven 
 ---
 tests/qemu-iotests/013 |9 -
 tests/qemu-iotests/013.out |2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)
 
 diff --git a/tests/qemu-iotests/013 b/tests/qemu-iotests/013
 index ea3cab9..0dbc934 100755
 --- a/tests/qemu-iotests/013
 +++ b/tests/qemu-iotests/013
 @@ -41,14 +41,14 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # much of this could be generic for any format supporting compression.
 _supported_fmt qcow qcow2
 -_supported_proto file
 +_supported_proto generic
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
 TEST_OPS="writev read write readv"
 CLUSTER_SIZE=4096
>>> 
>>> I think dropping these three TEST_IMG overriding change...
>>> 
 -_make_test_img 6G
 +TEST_IMG=$TEST_IMG.orig _make_test_img 6G
>>> 
>>> #1
>>> 
 
 echo "Testing empty image"
 echo
 @@ -56,16 +56,15 @@ echo
 for offset in $TEST_OFFSETS; do
 echo "At offset $offset:"
 for op in $TEST_OPS; do
 -io_test $op $offset $CLUSTER_SIZE 8
 +TEST_IMG=$TEST_IMG.orig io_test $op $offset $CLUSTER_SIZE 8
>>> 
>>> #2
>>> 
 done
 -_check_test_img
 +TEST_IMG=$TEST_IMG.orig _check_test_img
>>> 
>>> #3
>>> 
 done
 
 
 echo "Compressing image"
 echo
 
 -mv "$TEST_IMG" "$TEST_IMG.orig"
>>> 
>>> and changing this to
>>> 
>>> TEST_IMG=$TEST_IMG.orig _make_test_img 6G
>>> 
>>> Should work.
>> Unfortunately it doesn't. All subsequent commands will then work
>> on $TEST_IMG.orig altough they shouldn't. In case of
>> 013 this is io_test, _check_test_img and the cleanup at the end.
>> 
>> There are 3 options:
>> - override it in every line that should use an alternate $TEST_IMG
>> - save the original $TEST_IMG and restore it.
>> - rework all commands to take the file as parameter and not use
>>   a global variable for it.
>> 
>> I choosed the first one because it makes clear which $TEST_IMG is acutally
>> used. You see from the output and the code that you are dealing with the
>> file that is later used as $TEST_IMG.orig. If you see $TEST_IMG there you
>> can't distinguish if its the backing or original file or the actual image.
> 
> There more I've read through the patches, my opinion is that something
> more along the lines of option #3 would be best.  If that is done, it
> may be nice for the file to be an optional argument to the function.
> That way, for tests that only support IMGPROTO=file, the current
> default operation does not need to change.
> 
> My fear is the current method (#1) seems a bit unwieldy; I
> foresee scripts often forgetting to do these manual override steps.
> Then again, the default IMGPROTO was set to 'file', so perhaps my fear
> is unfounded.

another question would be if we really want that all these tests work with all 
protocols. i think their main purpose is to test the IMGFORMAT, but maybe they 
could help to trigger subtile bugs in IMGPROTO != file that the more generic 
tests don't...

> 
>> 
>> But I thought that this would be controversal. This is I why I splitted the 
>> patch
>> into individual ones. So its possible to drop all these patches and still be 
>> able
>> to proceed with the integration of the NFS protocol driver.
>>> 
 $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -c "$TEST_IMG.orig" "$TEST_IMG"
 
 echo "Testing compressed image"
 diff --git a/tests/qemu-iotests/013.out b/tests/qemu-iotests/013.out
 index 43a414c..763cb0c 100644
 --- a/tests/qemu-iotests/013.out
 +++ b/tests/qemu-iotests/013.out
 @@ -1,5 +1,5 @@
 QA output created by 013
 -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=6442450944
 +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=6442450944
>>> 
>>> So this is not necessary.
>>> 
>>> Fam
>> Peter



[Qemu-devel] [PULL 14/14] migration: qmp_migrate(): keep working after syntax error

2014-01-06 Thread Luiz Capitulino
If a user or QMP client enter a bad syntax for the migrate
command in QMP/HMP, then the migrate command will never succeed
from that point on.

For example, if you enter:

(qemu) migrate tcp;0:
migrate: Parameter 'uri' expects a valid migration protocol

Then the migrate command will always fail from now on:

(qemu) migrate tcp:0:
migrate: There's a migration process in progress

The problem is that qmp_migrate() sets the migration status to
MIG_STATE_SETUP and doesn't reset it on syntax error. This bug
was introduced by commit 29ae8a4133082e16970c9d4be09f4b6a15034617.

Reviewed-by: Michael R. Hines 
Signed-off-by: Luiz Capitulino 
---
 migration.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration.c b/migration.c
index 2b1ab20..557195a 100644
--- a/migration.c
+++ b/migration.c
@@ -437,6 +437,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 #endif
 } else {
 error_set(errp, QERR_INVALID_PARAMETER_VALUE, "uri", "a valid 
migration protocol");
+s->state = MIG_STATE_ERROR;
 return;
 }
 
-- 
1.8.1.4




[Qemu-devel] [PULL 00/14] QMP queue

2014-01-06 Thread Luiz Capitulino
The following changes since commit f976b09ea2493fd41c98aaf6512908db0bae:

  PPC: Fix compilation with TCG debug (2013-12-22 19:15:55 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

for you to fetch changes up to c950114286ea358a93ce632db0421945e1008395:

  migration: qmp_migrate(): keep working after syntax error (2014-01-06 
15:02:30 -0500)


Jason J. Herne (1):
  qemu-monitor: HMP cpu-add wrapper

Luiz Capitulino (1):
  migration: qmp_migrate(): keep working after syntax error

Paolo Bonzini (6):
  vl: add missing transition debug->finish_migrate
  rng: initialize file descriptor to -1
  qom: fix leak for objects created with -object
  qom: catch errors in object_property_add_child
  monitor: add object-del (QMP) and object_del (HMP) command
  monitor: add object-add (QMP) and object_add (HMP) command

Peter Crosthwaite (6):
  error: Add error_abort
  qdev: Delete dead code
  hw: Remove assert_no_error usages
  target-i386: Remove assert_no_error usage
  qemu-option: Remove qemu_opts_create_nofail
  qerror: Remove assert_no_error()

 backends/rng-random.c|  4 +--
 block/blkdebug.c |  2 +-
 block/blkverify.c|  2 +-
 block/curl.c |  2 +-
 block/gluster.c  |  2 +-
 block/iscsi.c|  2 +-
 block/nbd.c  |  3 +-
 block/qcow2.c|  2 +-
 block/raw-posix.c|  2 +-
 block/raw-win32.c|  5 +--
 block/rbd.c  |  2 +-
 block/sheepdog.c |  2 +-
 block/vvfat.c|  2 +-
 blockdev.c   |  6 ++--
 hmp-commands.hx  | 41 +
 hmp.c| 77 
 hmp.h|  3 ++
 hw/core/qdev-properties-system.c |  8 ++---
 hw/core/qdev-properties.c| 40 ++---
 hw/core/qdev.c   | 28 ---
 hw/dma/xilinx_axidma.c   | 13 +++
 hw/net/xilinx_axienet.c  | 13 +++
 hw/watchdog/watchdog.c   |  3 +-
 include/hw/xilinx.h  | 14 +++-
 include/monitor/monitor.h|  3 ++
 include/qapi/error.h |  6 
 include/qapi/qmp/qerror.h|  1 -
 include/qapi/visitor.h   |  3 +-
 include/qemu/option.h|  1 -
 include/qemu/typedefs.h  |  2 ++
 migration.c  |  1 +
 qapi-schema.json | 34 ++
 qdev-monitor.c   |  2 +-
 qemu-img.c   |  2 +-
 qmp-commands.hx  | 51 ++
 qmp.c| 76 +++
 qobject/qerror.c |  8 -
 qom/object.c | 11 --
 target-arm/cpu.c |  7 ++--
 target-i386/cpu.c|  4 +--
 util/error.c | 22 +++-
 util/qemu-config.c   |  2 +-
 util/qemu-option.c   |  9 -
 util/qemu-sockets.c  | 18 +-
 vl.c | 19 ++
 45 files changed, 405 insertions(+), 155 deletions(-)



Re: [Qemu-devel] [PATCHv2 02/18] qemu-iotests: enable support for NFS protocol

2014-01-06 Thread Peter Lieven


> Am 06.01.2014 um 21:14 schrieb Jeff Cody :
> 
>> On Sun, Jan 05, 2014 at 06:21:52PM +0100, Peter Lieven wrote:
>> Signed-off-by: Peter Lieven 
>> ---
>> tests/qemu-iotests/common|   22 +++---
>> tests/qemu-iotests/common.rc |3 +++
>> 2 files changed, 22 insertions(+), 3 deletions(-)
>> 
>> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
>> index 8b4e22c..e19673b 100644
>> --- a/tests/qemu-iotests/common
>> +++ b/tests/qemu-iotests/common
>> @@ -144,10 +144,12 @@ check options
>> -vpctest vpc
>> -vhdx   test vhdx
>> -vmdk   test vmdk
>> +-file   test file (default)
>> -rbdtest rbd
>> -sheepdog   test sheepdog
>> -nbdtest nbd
>> -sshtest ssh
>> +-nfstest nfs
>> -xdiff  graphical mode diff
>> -nocacheuse O_DIRECT on backing file
>> -misalign   misalign memory allocations
>> @@ -211,22 +213,36 @@ testlist options
>> xpand=false
>> ;;
>> 
>> +-file)
>> +IMGFMT=file
> 
> This should be IMGPROTO, right?

of course, you are right. its not necessary at all because the check script 
does not complain about unknown options, but i added it in the sake of 
completeness.

Peter

> 
>> +xpand=false
>> +;;
>> +
>> -rbd)
>> IMGPROTO=rbd
>> xpand=false
>> ;;
>> +
>> -sheepdog)
>> IMGPROTO=sheepdog
>> xpand=false
>> ;;
>> +
>> -nbd)
>> IMGPROTO=nbd
>> xpand=false
>> ;;
>> +
>> -ssh)
>> IMGPROTO=ssh
>> xpand=false
>> ;;
>> +
>> +-nfs)
>> +IMGPROTO=nfs
>> +xpand=false
>> +;;
>> +
>> -nocache)
>> CACHEMODE="none"
>> CACHEMODE_IS_DEFAULT=false
>> @@ -238,10 +254,10 @@ testlist options
>> xpand=false
>> ;;
>> 
>> --valgrind)
>> -valgrind=true
>> +-valgrind)
>> +valgrind=true
>> xpand=false
>> -;;
>> +;;
>> 
>> -g)# -g group ... pick from group file
>> group=true
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 28ba0d9..940b863 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -61,6 +61,9 @@ elif [ "$IMGPROTO" = "nbd" ]; then
>> elif [ "$IMGPROTO" = "ssh" ]; then
>> TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE"
>> +elif [ "$IMGPROTO" = "nfs" ]; then
>> +TEST_DIR="nfs://127.0.0.1/$TEST_DIR"
>> +TEST_IMG=$TEST_DIR/t.$IMGFMT
>> else
>> TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
>> fi
>> -- 
>> 1.7.9.5
>> 



[Qemu-devel] [PULL 06/14] monitor: add object-del (QMP) and object_del (HMP) command

2014-01-06 Thread Luiz Capitulino
From: Paolo Bonzini 

These two commands invoke the "unparent" method of Object.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Igor Mammedov 
Tested-by: Igor Mammedov 
Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx  | 14 ++
 hmp.c|  9 +
 hmp.h|  1 +
 qapi-schema.json | 14 ++
 qmp-commands.hx  | 25 +
 qmp.c| 14 ++
 6 files changed, 77 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 929550d..4d5e2b5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1243,6 +1243,20 @@ STEXI
 Remove host network device.
 ETEXI
 
+{
+.name   = "object_del",
+.args_type  = "id:s",
+.params = "id",
+.help   = "destroy QOM object",
+.mhandler.cmd = hmp_object_del,
+},
+
+STEXI
+@item object_del
+@findex object_del
+Destroy QOM object.
+ETEXI
+
 #ifdef CONFIG_SLIRP
 {
 .name   = "hostfwd_add",
diff --git a/hmp.c b/hmp.c
index c513f9b..fe05c6b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1574,3 +1574,12 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
 
 hmp_handle_error(mon, &err);
 }
+
+void hmp_object_del(Monitor *mon, const QDict *qdict)
+{
+const char *id = qdict_get_str(qdict, "id");
+Error *err = NULL;
+
+qmp_object_del(id, &err);
+hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index f92fc89..7a11f68 100644
--- a/hmp.h
+++ b/hmp.h
@@ -90,5 +90,6 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
 void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 void hmp_qemu_io(Monitor *mon, const QDict *qdict);
 void hmp_cpu_add(Monitor *mon, const QDict *qdict);
+void hmp_object_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index c3c939c..af3a83b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2759,6 +2759,20 @@
 { 'command': 'netdev_del', 'data': {'id': 'str'} }
 
 ##
+# @object-del:
+#
+# Remove a QOM object.
+#
+# @id: the name of the QOM object to remove
+#
+# Returns: Nothing on success
+#  Error if @id is not a valid id for a QOM object
+#
+# Since: 2.0
+##
+{ 'command': 'object-del', 'data': {'id': 'str'} }
+
+##
 # @NetdevNoneOptions
 #
 # Use it alone to have zero network devices.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fba15cd..71422cd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -879,6 +879,31 @@ Example:
 EQMP
 
 {
+.name   = "object-del",
+.args_type  = "id:s",
+.mhandler.cmd_new = qmp_marshal_input_object_del,
+},
+
+SQMP
+object-del
+--
+
+Remove QOM object.
+
+Arguments:
+
+- "id": the object's ID (json-string)
+
+Example:
+
+-> { "execute": "object-del", "arguments": { "id": "rng1" } }
+<- { "return": {} }
+
+
+EQMP
+
+
+{
 .name   = "block_resize",
 .args_type  = "device:B,size:o",
 .mhandler.cmd_new = qmp_marshal_input_block_resize,
diff --git a/qmp.c b/qmp.c
index 1d7a04d..73aab58 100644
--- a/qmp.c
+++ b/qmp.c
@@ -529,3 +529,17 @@ void qmp_add_client(const char *protocol, const char 
*fdname,
 error_setg(errp, "protocol '%s' is invalid", protocol);
 close(fd);
 }
+
+void qmp_object_del(const char *id, Error **errp)
+{
+Object *container;
+Object *obj;
+
+container = container_get(object_get_root(), "/objects");
+obj = object_resolve_path_component(container, id);
+if (!obj) {
+error_setg(errp, "object id not found");
+return;
+}
+object_unparent(obj);
+}
-- 
1.8.1.4




[Qemu-devel] [PULL 13/14] qerror: Remove assert_no_error()

2014-01-06 Thread Luiz Capitulino
From: Peter Crosthwaite 

This is no longer needed, and is obsoleted by error_abort. Remove.

Signed-off-by: Peter Crosthwaite 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 include/qapi/qmp/qerror.h | 1 -
 qobject/qerror.c  | 8 
 2 files changed, 9 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index c30c2f6..73c67b7 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -29,7 +29,6 @@ typedef struct QError {
 QString *qerror_human(const QError *qerror);
 void qerror_report(ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(2, 
3);
 void qerror_report_err(Error *err);
-void assert_no_error(Error *err);
 
 /*
  * QError class list
diff --git a/qobject/qerror.c b/qobject/qerror.c
index fc8331a..e3608e2 100644
--- a/qobject/qerror.c
+++ b/qobject/qerror.c
@@ -121,14 +121,6 @@ void qerror_report_err(Error *err)
 }
 }
 
-void assert_no_error(Error *err)
-{
-if (err) {
-qerror_report_err(err);
-abort();
-}
-}
-
 /**
  * qobject_to_qerror(): Convert a QObject into a QError
  */
-- 
1.8.1.4




[Qemu-devel] [PULL 07/14] monitor: add object-add (QMP) and object_add (HMP) command

2014-01-06 Thread Luiz Capitulino
From: Paolo Bonzini 

Add two commands that are the monitor counterparts of -object.  The commands
have the same Visitor-based implementation, but use different kinds of
visitors so that the HMP command has a DWIM string-based syntax, while
the QMP variant accepts a stricter JSON-based properties dictionary.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Igor Mammedov 
Tested-by: Igor Mammedov 
Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx   | 14 +++
 hmp.c | 58 
 hmp.h |  1 +
 include/monitor/monitor.h |  3 +++
 include/qapi/visitor.h|  3 +--
 include/qemu/typedefs.h   |  2 ++
 qapi-schema.json  | 20 +++
 qmp-commands.hx   | 26 
 qmp.c | 62 +++
 9 files changed, 187 insertions(+), 2 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 4d5e2b5..feca084 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1244,6 +1244,20 @@ Remove host network device.
 ETEXI
 
 {
+.name   = "object_add",
+.args_type  = "object:O",
+.params = "[qom-type=]type,id=str[,prop=value][,...]",
+.help   = "create QOM object",
+.mhandler.cmd = hmp_object_add,
+},
+
+STEXI
+@item object_add
+@findex object_add
+Create QOM object.
+ETEXI
+
+{
 .name   = "object_del",
 .args_type  = "id:s",
 .params = "id",
diff --git a/hmp.c b/hmp.c
index fe05c6b..79f9c7d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -21,6 +21,7 @@
 #include "qmp-commands.h"
 #include "qemu/sockets.h"
 #include "monitor/monitor.h"
+#include "qapi/opts-visitor.h"
 #include "ui/console.h"
 #include "block/qapi.h"
 #include "qemu-io.h"
@@ -1354,6 +1355,63 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, &err);
 }
 
+void hmp_object_add(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+QemuOpts *opts;
+char *type = NULL;
+char *id = NULL;
+void *dummy = NULL;
+OptsVisitor *ov;
+QDict *pdict;
+
+opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
+if (err) {
+goto out;
+}
+
+ov = opts_visitor_new(opts);
+pdict = qdict_clone_shallow(qdict);
+
+visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
+if (err) {
+goto out_clean;
+}
+
+qdict_del(pdict, "qom-type");
+visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
+if (err) {
+goto out_clean;
+}
+
+qdict_del(pdict, "id");
+visit_type_str(opts_get_visitor(ov), &id, "id", &err);
+if (err) {
+goto out_clean;
+}
+
+object_add(type, id, pdict, opts_get_visitor(ov), &err);
+if (err) {
+goto out_clean;
+}
+visit_end_struct(opts_get_visitor(ov), &err);
+if (err) {
+qmp_object_del(id, NULL);
+}
+
+out_clean:
+opts_visitor_cleanup(ov);
+
+QDECREF(pdict);
+qemu_opts_del(opts);
+g_free(id);
+g_free(type);
+g_free(dummy);
+
+out:
+hmp_handle_error(mon, &err);
+}
+
 void hmp_getfd(Monitor *mon, const QDict *qdict)
 {
 const char *fdname = qdict_get_str(qdict, "fdname");
diff --git a/hmp.h b/hmp.h
index 7a11f68..ed58f0e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -90,6 +90,7 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
 void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 void hmp_qemu_io(Monitor *mon, const QDict *qdict);
 void hmp_cpu_add(Monitor *mon, const QDict *qdict);
+void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 10fa0e3..22d8b8f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -93,6 +93,9 @@ int monitor_read_password(Monitor *mon, ReadLineFunc 
*readline_func,
 int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
 
 int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
+int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret);
+void object_add(const char *type, const char *id, const QDict *qdict,
+Visitor *v, Error **errp);
 
 AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
 bool has_opaque, const char *opaque,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 48a2a2e..29da211 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -13,6 +13,7 @@
 #ifndef QAPI_VISITOR_CORE_H
 #define QAPI_VISITOR_CORE_H
 
+#include "qemu/typedefs.h"
 #include "qapi/qmp/qobject.h"
 #include "qapi/error.h"
 #include 
@@ -26,8 +27,6 @@ typedef struct GenericList
 struct GenericList *next;
 } GenericList;
 
-typedef struct Visitor Visitor;
-
 void visit_start_handle(Visitor *v, void **obj, const char *kind,
 const 

[Qemu-devel] [PULL 11/14] target-i386: Remove assert_no_error usage

2014-01-06 Thread Luiz Capitulino
From: Peter Crosthwaite 

Replace an assert_no_error() usage with the error_abort system.

Signed-off-by: Peter Crosthwaite 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 target-i386/cpu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bb98f6d..6b7b1a9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1600,7 +1600,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t 
*x86_cpu_def,
 const char *name)
 {
 x86_def_t *def;
-Error *err = NULL;
 int i;
 
 if (name == NULL) {
@@ -1608,8 +1607,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t 
*x86_cpu_def,
 }
 if (kvm_enabled() && strcmp(name, "host") == 0) {
 kvm_cpu_fill_host(x86_cpu_def);
-object_property_set_bool(OBJECT(cpu), true, "pmu", &err);
-assert_no_error(err);
+object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
 return 0;
 }
 
-- 
1.8.1.4




[Qemu-devel] [PULL 10/14] hw: Remove assert_no_error usages

2014-01-06 Thread Luiz Capitulino
From: Peter Crosthwaite 

Replace assert_no_error() usages with the error_abort system.
&error_abort is passed into API calls to signal to the Error sub-system
that any errors are fatal. Removes need for caller assertions.

Signed-off-by: Peter Crosthwaite 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 hw/core/qdev-properties-system.c |  8 ++--
 hw/core/qdev-properties.c| 40 ++--
 hw/core/qdev.c   | 23 +++
 hw/dma/xilinx_axidma.c   | 13 -
 hw/net/xilinx_axienet.c  | 13 -
 include/hw/xilinx.h  | 14 --
 target-arm/cpu.c |  7 ++-
 7 files changed, 33 insertions(+), 85 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 729efa8..3f29b49 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -352,21 +352,17 @@ void qdev_prop_set_drive_nofail(DeviceState *dev, const 
char *name,
 void qdev_prop_set_chr(DeviceState *dev, const char *name,
CharDriverState *value)
 {
-Error *errp = NULL;
 assert(!value || value->label);
 object_property_set_str(OBJECT(dev),
-value ? value->label : "", name, &errp);
-assert_no_error(errp);
+value ? value->label : "", name, &error_abort);
 }
 
 void qdev_prop_set_netdev(DeviceState *dev, const char *name,
   NetClientState *value)
 {
-Error *errp = NULL;
 assert(!value || value->name);
 object_property_set_str(OBJECT(dev),
-value ? value->name : "", name, &errp);
-assert_no_error(errp);
+value ? value->name : "", name, &error_abort);
 }
 
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dc8ae69..b949f0e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1003,73 +1003,55 @@ void qdev_prop_parse(DeviceState *dev, const char 
*name, const char *value,
 
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
 {
-Error *errp = NULL;
-object_property_set_bool(OBJECT(dev), value, name, &errp);
-assert_no_error(errp);
+object_property_set_bool(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
 {
-Error *errp = NULL;
-object_property_set_int(OBJECT(dev), value, name, &errp);
-assert_no_error(errp);
+object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value)
 {
-Error *errp = NULL;
-object_property_set_int(OBJECT(dev), value, name, &errp);
-assert_no_error(errp);
+object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value)
 {
-Error *errp = NULL;
-object_property_set_int(OBJECT(dev), value, name, &errp);
-assert_no_error(errp);
+object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value)
 {
-Error *errp = NULL;
-object_property_set_int(OBJECT(dev), value, name, &errp);
-assert_no_error(errp);
+object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value)
 {
-Error *errp = NULL;
-object_property_set_int(OBJECT(dev), value, name, &errp);
-assert_no_error(errp);
+object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_string(DeviceState *dev, const char *name, const char 
*value)
 {
-Error *errp = NULL;
-object_property_set_str(OBJECT(dev), value, name, &errp);
-assert_no_error(errp);
+object_property_set_str(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
 {
-Error *errp = NULL;
 char str[2 * 6 + 5 + 1];
 snprintf(str, sizeof(str), "%02x:%02x:%02x:%02x:%02x:%02x",
  value[0], value[1], value[2], value[3], value[4], value[5]);
 
-object_property_set_str(OBJECT(dev), str, name, &errp);
-assert_no_error(errp);
+object_property_set_str(OBJECT(dev), str, name, &error_abort);
 }
 
 void qdev_prop_set_enum(DeviceState *dev, const char *name, int value)
 {
 Property *prop;
-Error *errp = NULL;
 
 prop = qdev_prop_find(dev, name);
 object_property_set_str(OBJECT(dev), prop->info->enum_table[value],
-name, &errp);
-assert_no_error(errp);
+name, &error_abort);
 }
 
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
@@ -1161,12 +1143,10 @@ static void set_siz

[Qemu-devel] [PULL 08/14] error: Add error_abort

2014-01-06 Thread Luiz Capitulino
From: Peter Crosthwaite 

Add a special Error * that can be passed to error handling APIs to
signal that any errors are fatal and should abort QEMU. There are two
advantages to this:

- allows for brevity when wishing to assert success of Error **
  accepting APIs. No need for this pattern:
Error * local_err = NULL;
api_call(foo, bar, &local_err);
assert_no_error(local_err);
  This also removes the need for _nofail variants of APIs with
  asserting call sites now reduced to 1LOC.
- SIGABRT happens from within the offending API. When a fatal error
  occurs in an API call (when the caller is asserting sucess) failure
  often means the API itself is broken. With the abort happening in the
  API call now, the stack frames into the call are available at debug
  time. In the assert_no_error scheme the abort happens after the fact.

The exact semantic is that when an error is raised, if the argument
Error ** matches &error_abort, then the abort occurs immediately. The
error messaged is reported.

For error_propagate, if the destination error is &error_abort, then
the abort happens at propagation time.

Signed-off-by: Peter Crosthwaite 
Reviewed-by: Markus Armbruster 
Signed-off-by: Luiz Capitulino 
---
 include/qapi/error.h |  6 ++
 util/error.c | 22 +-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 7d4c696..c0f0c3b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -95,4 +95,10 @@ void error_propagate(Error **dst_err, Error *local_err);
  */
 void error_free(Error *err);
 
+/**
+ * If passed to error_set and friends, abort().
+ */
+
+extern Error *error_abort;
+
 #endif
diff --git a/util/error.c b/util/error.c
index 3ee362a..f11f1d5 100644
--- a/util/error.c
+++ b/util/error.c
@@ -23,6 +23,8 @@ struct Error
 ErrorClass err_class;
 };
 
+Error *error_abort;
+
 void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
 {
 Error *err;
@@ -41,6 +43,11 @@ void error_set(Error **errp, ErrorClass err_class, const 
char *fmt, ...)
 va_end(ap);
 err->err_class = err_class;
 
+if (errp == &error_abort) {
+error_report("%s", error_get_pretty(err));
+abort();
+}
+
 *errp = err;
 
 errno = saved_errno;
@@ -72,6 +79,11 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass 
err_class,
 va_end(ap);
 err->err_class = err_class;
 
+if (errp == &error_abort) {
+error_report("%s", error_get_pretty(err));
+abort();
+}
+
 *errp = err;
 
 errno = saved_errno;
@@ -112,6 +124,11 @@ void error_set_win32(Error **errp, int win32_err, 
ErrorClass err_class,
 va_end(ap);
 err->err_class = err_class;
 
+if (errp == &error_abort) {
+error_report("%s", error_get_pretty(err));
+abort();
+}
+
 *errp = err;
 }
 
@@ -153,7 +170,10 @@ void error_free(Error *err)
 
 void error_propagate(Error **dst_err, Error *local_err)
 {
-if (dst_err && !*dst_err) {
+if (local_err && dst_err == &error_abort) {
+error_report("%s", error_get_pretty(local_err));
+abort();
+} else if (dst_err && !*dst_err) {
 *dst_err = local_err;
 } else if (local_err) {
 error_free(local_err);
-- 
1.8.1.4




Re: [Qemu-devel] [Xen-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Paolo Bonzini
Il 06/01/2014 16:04, Peter Maydell ha scritto:
> On 6 January 2014 14:54, Stefano Stabellini
>  wrote:
>> How would you avoid the compilation of all the
>> unnecessary emulated devices?
> 
> Didn't we have some patches for doing a Kconfig-style
> "select the devices you need" build recently?

It was really "select what boards you need" and "select any additional
-device-capable devices you need".  So the covered devices were
basically ISA, PCI, USB and I2C.  Other files (e.g. scsi-bus.c or sd.c)
were picked up via dependencies so they could still be left out unless
necessary.

The final plan was to keep the default-configs/ files, and add
dependency-handling via the Kconfig language but without Kconfig sources
itself.  Akos, did you go any further with your mini_kconfig.py parser?

Paolo



Re: [Qemu-devel] [Xen-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Paolo Bonzini
Il 06/01/2014 19:00, Andreas Färber ha scritto:
> Am 06.01.2014 16:39, schrieb Anthony Liguori:
>> We already have accel=xen.  I'm echoing Peter's suggestion of having the
>> ability to compile out accel=tcg.
> 
> Didn't you and Paolo even have patches for that a while ago?

Yes, but some code shuffling is required in each target to make sure you
can compile out translate-all.c, cputlb.c, etc.  So my patches only
worked for x86 at the time.

Paolo



Re: [Qemu-devel] [PATCH v3] linux-user: Support the accept4 socketcall

2014-01-06 Thread Erik de Castro Lopo
André Hentschel wrote:

> From: André Hentschel 
> Cc: Riku Voipio 
> Signed-off-by: André Hentschel 


Reviewed-by: Erik de Castro Lopo 

-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/



Re: [Qemu-devel] [PATCHv2 04/18] qemu-iotests: fix test 013 to work with any protocol

2014-01-06 Thread Jeff Cody
On Mon, Jan 06, 2014 at 07:48:25AM +0100, Peter Lieven wrote:
> On 06.01.2014 06:31, Fam Zheng wrote:
> >On 2014年01月06日 01:21, Peter Lieven wrote:
> >>Signed-off-by: Peter Lieven 
> >>---
> >>  tests/qemu-iotests/013 |9 -
> >>  tests/qemu-iotests/013.out |2 +-
> >>  2 files changed, 5 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/tests/qemu-iotests/013 b/tests/qemu-iotests/013
> >>index ea3cab9..0dbc934 100755
> >>--- a/tests/qemu-iotests/013
> >>+++ b/tests/qemu-iotests/013
> >>@@ -41,14 +41,14 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> >>
> >>  # much of this could be generic for any format supporting compression.
> >>  _supported_fmt qcow qcow2
> >>-_supported_proto file
> >>+_supported_proto generic
> >>  _supported_os Linux
> >>
> >>  TEST_OFFSETS="0 4294967296"
> >>  TEST_OPS="writev read write readv"
> >>  CLUSTER_SIZE=4096
> >>
> >
> >I think dropping these three TEST_IMG overriding change...
> >
> >>-_make_test_img 6G
> >>+TEST_IMG=$TEST_IMG.orig _make_test_img 6G
> >
> >#1
> >
> >>
> >>  echo "Testing empty image"
> >>  echo
> >>@@ -56,16 +56,15 @@ echo
> >>  for offset in $TEST_OFFSETS; do
> >>  echo "At offset $offset:"
> >>  for op in $TEST_OPS; do
> >>-io_test $op $offset $CLUSTER_SIZE 8
> >>+TEST_IMG=$TEST_IMG.orig io_test $op $offset $CLUSTER_SIZE 8
> >
> >#2
> >
> >>  done
> >>-_check_test_img
> >>+TEST_IMG=$TEST_IMG.orig _check_test_img
> >
> >#3
> >
> >>  done
> >>
> >>
> >>  echo "Compressing image"
> >>  echo
> >>
> >>-mv "$TEST_IMG" "$TEST_IMG.orig"
> >
> >and changing this to
> >
> >TEST_IMG=$TEST_IMG.orig _make_test_img 6G
> >
> >Should work.
> Unfortunately it doesn't. All subsequent commands will then work
> on $TEST_IMG.orig altough they shouldn't. In case of
> 013 this is io_test, _check_test_img and the cleanup at the end.
> 
> There are 3 options:
>  - override it in every line that should use an alternate $TEST_IMG
>  - save the original $TEST_IMG and restore it.
>  - rework all commands to take the file as parameter and not use
>a global variable for it.
> 
> I choosed the first one because it makes clear which $TEST_IMG is acutally
> used. You see from the output and the code that you are dealing with the
> file that is later used as $TEST_IMG.orig. If you see $TEST_IMG there you
> can't distinguish if its the backing or original file or the actual image.

There more I've read through the patches, my opinion is that something
more along the lines of option #3 would be best.  If that is done, it
may be nice for the file to be an optional argument to the function.
That way, for tests that only support IMGPROTO=file, the current
default operation does not need to change.

My fear is the current method (#1) seems a bit unwieldy; I
foresee scripts often forgetting to do these manual override steps.
Then again, the default IMGPROTO was set to 'file', so perhaps my fear
is unfounded.

> 
> But I thought that this would be controversal. This is I why I splitted the 
> patch
> into individual ones. So its possible to drop all these patches and still be 
> able
> to proceed with the integration of the NFS protocol driver.
> >
> >>  $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -c "$TEST_IMG.orig" "$TEST_IMG"
> >>
> >>  echo "Testing compressed image"
> >>diff --git a/tests/qemu-iotests/013.out b/tests/qemu-iotests/013.out
> >>index 43a414c..763cb0c 100644
> >>--- a/tests/qemu-iotests/013.out
> >>+++ b/tests/qemu-iotests/013.out
> >>@@ -1,5 +1,5 @@
> >>  QA output created by 013
> >>-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=6442450944
> >>+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=6442450944
> >
> >So this is not necessary.
> >
> >Fam
> Peter



Re: [Qemu-devel] [PATCH v3] linux-user: Support the accept4 socketcall

2014-01-06 Thread Peter Maydell
On 6 January 2014 19:18, André Hentschel  wrote:
> From: André Hentschel 
> Cc: Riku Voipio 
> Signed-off-by: André Hentschel 

Reviewed-by: Peter Maydell 

thanks
-- PMM



Re: [Qemu-devel] [PATCHv2 02/18] qemu-iotests: enable support for NFS protocol

2014-01-06 Thread Jeff Cody
On Sun, Jan 05, 2014 at 06:21:52PM +0100, Peter Lieven wrote:
> Signed-off-by: Peter Lieven 
> ---
>  tests/qemu-iotests/common|   22 +++---
>  tests/qemu-iotests/common.rc |3 +++
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
> index 8b4e22c..e19673b 100644
> --- a/tests/qemu-iotests/common
> +++ b/tests/qemu-iotests/common
> @@ -144,10 +144,12 @@ check options
>  -vpctest vpc
>  -vhdx   test vhdx
>  -vmdk   test vmdk
> +-file   test file (default)
>  -rbdtest rbd
>  -sheepdog   test sheepdog
>  -nbdtest nbd
>  -sshtest ssh
> +-nfstest nfs
>  -xdiff  graphical mode diff
>  -nocacheuse O_DIRECT on backing file
>  -misalign   misalign memory allocations
> @@ -211,22 +213,36 @@ testlist options
>  xpand=false
>  ;;
>  
> +-file)
> +IMGFMT=file

This should be IMGPROTO, right?

> +xpand=false
> +;;
> +
>  -rbd)
>  IMGPROTO=rbd
>  xpand=false
>  ;;
> +
>  -sheepdog)
>  IMGPROTO=sheepdog
>  xpand=false
>  ;;
> +
>  -nbd)
>  IMGPROTO=nbd
>  xpand=false
>  ;;
> +
>  -ssh)
>  IMGPROTO=ssh
>  xpand=false
>  ;;
> +
> +-nfs)
> +IMGPROTO=nfs
> +xpand=false
> +;;
> +
>  -nocache)
>  CACHEMODE="none"
>  CACHEMODE_IS_DEFAULT=false
> @@ -238,10 +254,10 @@ testlist options
>  xpand=false
>  ;;
>  
> --valgrind)
> -valgrind=true
> +-valgrind)
> +valgrind=true
>  xpand=false
> -;;
> +;;
>  
>  -g)# -g group ... pick from group file
>  group=true
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 28ba0d9..940b863 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -61,6 +61,9 @@ elif [ "$IMGPROTO" = "nbd" ]; then
>  elif [ "$IMGPROTO" = "ssh" ]; then
>  TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>  TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE"
> +elif [ "$IMGPROTO" = "nfs" ]; then
> +TEST_DIR="nfs://127.0.0.1/$TEST_DIR"
> +TEST_IMG=$TEST_DIR/t.$IMGFMT
>  else
>  TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
>  fi
> -- 
> 1.7.9.5
> 



Re: [Qemu-devel] [V3 PATCH 10/14] target-ppc: Fix and enable fri[mnpz]

2014-01-06 Thread Tom Musta
On 12/24/2013 10:02 AM, Richard Henderson wrote:
> I'll also note that frin can't properly be implemented with
> float_round_nearest_even because it doesn't round to even.

Good catch.  I should be able to use Peter's ties away rounding
mode patches.





Re: [Qemu-devel] [PATCH qmp v4 0/6] Add error_abort and associated cleanups

2014-01-06 Thread Luiz Capitulino
On Mon, 06 Jan 2014 20:18:45 +0100
Andreas Färber  wrote:

> Am 06.01.2014 20:13, schrieb Luiz Capitulino:
> > On Wed,  1 Jan 2014 18:46:24 -0800
> > Peter Crosthwaite  wrote:
> > 
> >> Following our discussion RE self asserting API calls, here is a spin of
> >> my proposal. This series obsoletes the need for _nofail variants for
> >> Error ** accepting APIs. Is also greatly reduces the verbosity of calls
> >> sites that are currently asserting against errors.
> >>
> >> Patch 1 is the main event - addition of error_abort. The following
> >> patches then cleanup uses of _nofail and assert_no_error().
> > 
> > Applied to the qmp branch, thanks.
> 
> Please normalize "hw/core/qdev:" to just "qdev:", thanks.

Done. You still have time for a Reviewed-by :)



Re: [Qemu-devel] [PULL 0/2] QMP queue

2014-01-06 Thread Luiz Capitulino
On Fri, 20 Dec 2013 11:38:54 -0500
Luiz Capitulino  wrote:

> I've dropped the two series which need to be respun. As the fix I have from
> Paolo is pending for a long time, I figure it's better to send what I have
> in the queue now.

Please, don't pull this one. I'm working on a more complete pull request,
will send it in some minutes.



Re: [Qemu-devel] [PATCH v6 05/11] dump: add support for lzo/snappy

2014-01-06 Thread Laszlo Ersek
On 01/05/14 08:27, Qiao Nuohan wrote:
> kdump-compressed format supports three compression format, zlib/lzo/snappy.
> Currently, only zlib is available. This patch is used to support lzo/snappy.
> '--enable-lzo/--enable-snappy' is needed to be specified with configure to 
> make
> lzo/snappy available for qemu
> 
> Signed-off-by: Qiao Nuohan 
> ---
>  configure |   50 ++
>  1 files changed, 50 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index edfea95..b7a28b7 100755
> --- a/configure
> +++ b/configure
> @@ -245,6 +245,8 @@ libusb=""
>  usb_redir=""
>  glx=""
>  zlib="yes"
> +lzo="no"
> +snappy="no"
>  guest_agent=""
>  guest_agent_with_vss="no"
>  vss_win32_sdk=""
> @@ -947,6 +949,10 @@ for opt do
>;;
>--disable-zlib-test) zlib="no"
>;;
> +  --enable-lzo) lzo="yes"
> +  ;;
> +  --enable-snappy) snappy="yes"
> +  ;;
>--enable-guest-agent) guest_agent="yes"
>;;
>--disable-guest-agent) guest_agent="no"
> @@ -1538,6 +1544,42 @@ fi
>  libs_softmmu="$libs_softmmu -lz"
>  
>  ##
> +# lzo check
> +
> +if test "$lzo" != "no" ; then
> +cat > $TMPC << EOF
> +#include 
> +int main(void) { lzo_version(); return 0; }
> +EOF
> +if compile_prog "" "-llzo2" ; then
> +:
> +else
> +error_exit "lzo check failed" \
> +"Make sure to have the lzo libs and headers installed."
> +fi
> +
> +libs_softmmu="$libs_softmmu -llzo2"
> +fi
> +
> +##
> +# snappy check
> +
> +if test "$snappy" != "no" ; then
> +cat > $TMPC << EOF
> +#include 
> +int main(void) { snappy_max_compressed_length(4096); return 0; }
> +EOF
> +if compile_prog "" "-lsnappy" ; then
> +:
> +else
> +error_exit "snappy check failed" \
> +"Make sure to have the snappy libs and headers installed."
> +fi
> +
> +libs_softmmu="$libs_softmmu -lsnappy"
> +fi
> +
> +##
>  # libseccomp check
>  
>  if test "$seccomp" != "no" ; then
> @@ -4135,6 +4177,14 @@ if test "$glx" = "yes" ; then
>echo "GLX_LIBS=$glx_libs" >> $config_host_mak
>  fi
>  
> +if test "$lzo" = "yes" ; then
> +  echo "CONFIG_LZO=y" >> $config_host_mak
> +fi
> +
> +if test "$snappy" = "yes" ; then
> +  echo "CONFIG_SNAPPY=y" >> $config_host_mak
> +fi
> +
>  if test "$libiscsi" = "yes" ; then
>echo "CONFIG_LIBISCSI=y" >> $config_host_mak
>  fi
> 

You could have displayed the lzo / snappy settings along with the other
settings in the "big echo block". But it's not too important; if you
want you can add it later.

Reviewed-by: Laszlo Ersek 



[Qemu-devel] [PATCH v3] linux-user: Support the accept4 socketcall

2014-01-06 Thread André Hentschel
From: André Hentschel 
Cc: Riku Voipio 
Signed-off-by: André Hentschel 
---
See 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/net.h
 for the value.

 linux-user/syscall.c  | 16 
 linux-user/syscall_defs.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index efd1453..0ac05b8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2245,6 +2245,22 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
 ret = do_accept4(sockfd, target_addr, target_addrlen, 0);
 }
 break;
+case SOCKOP_accept4:
+{
+abi_ulong sockfd;
+abi_ulong target_addr, target_addrlen;
+abi_ulong flags;
+
+if (get_user_ual(sockfd, vptr)
+|| get_user_ual(target_addr, vptr + n)
+|| get_user_ual(target_addrlen, vptr + 2 * n)
+|| get_user_ual(flags, vptr + 3 * n)) {
+return -TARGET_EFAULT;
+}
+
+ret = do_accept4(sockfd, target_addr, target_addrlen, flags);
+}
+break;
 case SOCKOP_getsockname:
 {
 abi_ulong sockfd;
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index cf08db5..ae30476 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -27,6 +27,7 @@
 #define SOCKOP_getsockopt   15
 #define SOCKOP_sendmsg  16
 #define SOCKOP_recvmsg  17
+#define SOCKOP_accept4  18
 
 #define IPCOP_semop1
 #define IPCOP_semget   2
-- 
1.8.1.2




Re: [Qemu-devel] [PATCH qmp v4 0/6] Add error_abort and associated cleanups

2014-01-06 Thread Andreas Färber
Am 06.01.2014 20:13, schrieb Luiz Capitulino:
> On Wed,  1 Jan 2014 18:46:24 -0800
> Peter Crosthwaite  wrote:
> 
>> Following our discussion RE self asserting API calls, here is a spin of
>> my proposal. This series obsoletes the need for _nofail variants for
>> Error ** accepting APIs. Is also greatly reduces the verbosity of calls
>> sites that are currently asserting against errors.
>>
>> Patch 1 is the main event - addition of error_abort. The following
>> patches then cleanup uses of _nofail and assert_no_error().
> 
> Applied to the qmp branch, thanks.

Please normalize "hw/core/qdev:" to just "qdev:", thanks.

Andreas

>> Peter Crosthwaite (6):
>>   error: Add error_abort
>>   hw/core/qdev: Delete dead code
>>   hw: Remove assert_no_error usages
>>   target-i386: Remove assert_no_error usage
>>   qemu-option: Remove qemu_opts_create_nofail
>>   qerror: Remove assert_no_error()

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH qmp v4 0/6] Add error_abort and associated cleanups

2014-01-06 Thread Luiz Capitulino
On Wed,  1 Jan 2014 18:46:24 -0800
Peter Crosthwaite  wrote:

> Following our discussion RE self asserting API calls, here is a spin of
> my proposal. This series obsoletes the need for _nofail variants for
> Error ** accepting APIs. Is also greatly reduces the verbosity of calls
> sites that are currently asserting against errors.
> 
> Patch 1 is the main event - addition of error_abort. The following
> patches then cleanup uses of _nofail and assert_no_error().

Applied to the qmp branch, thanks.

> 
> To give it a smoke test, I introduce a (critical) bug into QOM:
> 
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -797,7 +797,7 @@ void object_property_set(Object *obj, Visitor *v,
> const char *name,
>  return;
>  }
> 
> -if (!prop->set) {
> +if (prop->set) {
>  error_set(errp, QERR_PERMISSION_DENIED);
>  } else {
>  prop->set(obj, v, prop->opaque, name, errp);
> 
> And run QEMU:
> 
> $ gdb --args ./microblazeel-softmmu/qemu-system-microblazeel -M 
> petalogix-ml605 -nographic
> 
> Without application of this series, the bug manifests as:
> 
> qemu-system-microblazeel: Insufficient permission to perform this operation
> 
> Program received signal SIGABRT, Aborted.
> (gdb) bt
> 0  0x755f7425 in __GI_raise (sig=) at 
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> 1  0x755fab8b in __GI_abort () at abort.c:91
> 2  0x557073da in assert_no_error (err=) at 
> qobject/qerror.c:128
> 3  0x55615159 in qdev_property_add_static (dev=0x55e9e6a0, 
> prop=0x559ea4c0, errp=0x7fffe3c0) at hw/core/qdev.c:666
> 4  0x55615355 in device_initfn (obj=) at 
> hw/core/qdev.c:744
> 
> With this series, we now get a the full backtrace into the QOM API when
> the assertion occurs (note stack frames 2-4 giving visibility into the
> broken QOM API):
> 
> qemu-system-microblazeel: Insufficient permission to perform this operation
> 
> Program received signal SIGABRT, Aborted.
> (gdb) bt
> 0  0x755f7425 in __GI_raise (sig=) at 
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> 1  0x755fab8b in __GI_abort () at abort.c:91
> 2  0x5570c5a4 in error_set (errp=0x55e1b5f0, 
> err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x5571f5c0 "Insufficient 
> permission to perform this operation") at util/error.c:47
> 3  0x556778e5 in object_property_set_qobject (obj=0x55e9e6a0, 
> value=, name=0x5574e7a6 "xlnx.base-vectors", 
> errp=0x55e1b5f0) at qom/qom-qobject.c:24
> 4  0x5567674d in object_property_set_int (obj=0x55e9e6a0, 
> value=, name=0x5574e7a6 "xlnx.base-vectors", 
> errp=0x55e1b5f0) at qom/object.c:898
> 5  0x55615192 in qdev_property_add_static (dev=0x55e9e6a0, 
> prop=0x559ea4c0, errp=0x55e1b5f0) at hw/core/qdev.c:664
> 6  0x5561534f in device_initfn (obj=) at 
> hw/core/qdev.c:741
> 
> Changed since v2:
> Removed new assert_no_error usages in target-arm/cpu.c
> Changed since v1:
> Addressed Markus review.
> Guarded against erroneous setting of error_abort != NULL.
> 
> 
> Peter Crosthwaite (6):
>   error: Add error_abort
>   hw/core/qdev: Delete dead code
>   hw: Remove assert_no_error usages
>   target-i386: Remove assert_no_error usage
>   qemu-option: Remove qemu_opts_create_nofail
>   qerror: Remove assert_no_error()
> 
>  block/blkdebug.c |  2 +-
>  block/blkverify.c|  2 +-
>  block/curl.c |  2 +-
>  block/gluster.c  |  2 +-
>  block/iscsi.c|  2 +-
>  block/nbd.c  |  3 ++-
>  block/qcow2.c|  2 +-
>  block/raw-posix.c|  2 +-
>  block/raw-win32.c|  5 +++--
>  block/rbd.c  |  2 +-
>  block/sheepdog.c |  2 +-
>  block/vvfat.c|  2 +-
>  blockdev.c   |  6 --
>  hw/core/qdev-properties-system.c |  8 ++--
>  hw/core/qdev-properties.c| 40 
> ++--
>  hw/core/qdev.c   | 28 +++-
>  hw/dma/xilinx_axidma.c   | 13 -
>  hw/net/xilinx_axienet.c  | 13 -
>  hw/watchdog/watchdog.c   |  3 ++-
>  include/hw/xilinx.h  | 14 --
>  include/qapi/error.h |  6 ++
>  include/qapi/qmp/qerror.h|  1 -
>  include/qemu/option.h|  1 -
>  qdev-monitor.c   |  2 +-
>  qemu-img.c   |  2 +-
>  qobject/qerror.c |  8 
>  target-arm/cpu.c |  7 ++-
>  target-i386/cpu.c|  4 +---
>  util/error.c | 22 +-
>  util/qemu-config.c   |  2 +-
>  util/qemu-option.c   |  9 -
>  util/qemu-sockets.c  | 18 +-
>  vl.c | 15 +--
>  33 files changed, 103 in

Re: [Qemu-devel] [PATCH v3 0/5] Monitor commands for object-add/del

2014-01-06 Thread Luiz Capitulino
On Fri, 20 Dec 2013 23:21:05 +0100
Paolo Bonzini  wrote:

> These allow hotplugging (and hot-unplugging without leaking an object)
> virtio-rng devices.  They can also be used for memory hotplug.
> 
> v1->v2: fix mistyped underscores in qapi-schema.json
> 
> v2->v3: preserve bisectability by inverting patches 4 and 5, keep lines
> under 80 characters since it can be done easily, drop error_is_set

Applied to the qmp branch, thanks.

> 
> Paolo Bonzini (5):
>   rng: initialize file descriptor to -1
>   qom: fix leak for objects created with -object
>   qom: catch errors in object_property_add_child
>   monitor: add object-del (QMP) and object_del (HMP) command
>   monitor: add object-add (QMP) and object_add (HMP) command
> 
>  backends/rng-random.c |  4 +--
>  hmp-commands.hx   | 28 +
>  hmp.c | 67 +
>  hmp.h |  2 ++
>  include/monitor/monitor.h |  3 ++
>  include/qapi/visitor.h|  3 +-
>  include/qemu/typedefs.h   |  2 ++
>  qapi-schema.json  | 34 +
>  qmp-commands.hx   | 51 +++
>  qmp.c | 76 
> +++
>  qom/object.c  |  9 --
>  vl.c  |  3 +-
>  12 files changed, 275 insertions(+), 7 deletions(-)
> 




Re: [Qemu-devel] [PATCH v2 24/24] target-arm: A64: Add support for FCVT between half, single and double

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> Add support for FCVT between half, single and double precision.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/helper.c| 20 +
>  target-arm/helper.h|  2 ++
>  target-arm/translate-a64.c | 75 
> +-
>  3 files changed, 96 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 21/24] target-arm: A64: Add "Floating-point<->fixed-point" instructions

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> From: Alexander Graf 
> 
> This patch adds emulation for the instruction group labeled
> "Floating-point <-> fixed-point conversions" in the ARM ARM.
> 
> Namely this includes the instructions SCVTF, UCVTF, FCVTZS, FCVTZU
> (scalar, fixed-point).
> 
> Signed-off-by: Alexander Graf 
> [WN: Commit message tweak, rebased, updated to new infrastructure.
>  Applied bug fixes from Michael Matz and Janne Grunau.]
> Signed-off-by: Will Newton 
> [PMM: significant cleanup]
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/helper.c|  13 
>  target-arm/helper.h|   2 +
>  target-arm/translate-a64.c | 186 
> -
>  3 files changed, 200 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 23/24] target-arm: A64: Add 1-source 32-to-32 and 64-to-64 FP instructions

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> +DEF_HELPER_FLAGS_2(rints_exact, TCG_CALL_NO_RWG_SE, f32, f32, ptr)
> +DEF_HELPER_FLAGS_2(rintd_exact, TCG_CALL_NO_RWG_SE, f64, f64, ptr)
> +DEF_HELPER_FLAGS_2(rints, TCG_CALL_NO_RWG_SE, f32, f32, ptr)
> +DEF_HELPER_FLAGS_2(rintd, TCG_CALL_NO_RWG_SE, f64, f64, ptr)

These have side effects in the fp_status.

Otherwise,

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 22/24] target-arm: A64: Add floating-point<->integer conversion instructions

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> From: Will Newton 
> 
> Add support for the AArch64 floating-point <-> integer conversion
> instructions to disas_fpintconv. In the process we can rearrange
> and simplify the detection of unallocated encodings a little.
> We also correct a typo in the instruction encoding diagram for this
> instruction group: bit 21 is 1, not 0.
> 
> Signed-off-by: Will Newton 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/translate-a64.c | 23 ---
>  1 file changed, 20 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v6 04/11] dump: Add API to write elf notes to buffer

2014-01-06 Thread Laszlo Ersek
some tangential comments:

On 01/05/14 08:27, Qiao Nuohan wrote:
> the function can be used by write_elf32_notes/write_elf64_notes to write notes
> to a buffer. If fd_write_vmcore is used, write_elf32_notes/write_elf64_notes
> will write elf notes to vmcore directly. Instead, if buf_write_note is used,
> elf notes will be written to opaque->note_buf at first.
> 
> Signed-off-by: Qiao Nuohan 
> ---
>  dump.c |   19 +++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 764db39..3b9cf00 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -76,6 +76,9 @@ typedef struct DumpState {
>  int64_t begin;
>  int64_t length;
>  Error **errp;
> +
> +void *note_buf;
> +size_t note_buf_offset;
>  } DumpState;
>  
>  static int dump_cleanup(DumpState *s)
> @@ -754,6 +757,22 @@ static int write_buffer(int fd, bool flag_flatten, off_t 
> offset, void *buf,
>  return 0;
>  }
>  
> +static int buf_write_note(void *buf, size_t size, void *opaque)
> +{

"const void *buf" would have been more "elegant".

> +DumpState *s = opaque;
> +
> +/* note_buf is not enough */
> +if (s->note_buf_offset + size > s->note_size) {
> +return -1;
> +}
> +
> +memcpy(s->note_buf + s->note_buf_offset, buf, size);

Giving type "uint8_t" to "*note_buf" would have been preferable.
Addition to a pointer-to-void is a constraint violation in standard C
("... operand shall be a pointer to an object type ..."), ie. it's a gcc
extension here, but I guess we can live with it.

Using s->note_size as limit seems correct.

> +
> +s->note_buf_offset += size;
> +
> +return 0;
> +}
> +
>  static ram_addr_t get_start_block(DumpState *s)
>  {
>  GuestPhysBlock *block;
> 

Reviewed-by: Laszlo Ersek 



Re: [Qemu-devel] [PATCH v2 17/24] target-arm: Prepare VFP_CONV_FIX helpers for A64 uses

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> From: Will Newton 
> 
> Make the VFP_CONV_FIX helpers a little more flexible in
> preparation for the A64 uses. This requires two changes:
>  * use the correct softfloat conversion function based on itype
>rather than always the int32 one; this is possible now that
>softfloat provides int16 versions and necessary for the
>future conversion-to-int64 A64 variants. This also allows
>us to drop the awkward 'sign' macro argument.
>  * split the 'fsz' argument which currently controls both
>width of the input float type and width of the output
>integer type into two; this will allow us to specify the
>A64 64-bit-int-to-single conversion function, where the
>two widths are different.
> 
> We can also drop the (itype##_t) cast now that softfloat
> guarantees that all the itype##_to_float* functions take
> an integer argument of exactly the correct type.
> 
> Signed-off-by: Will Newton 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/helper.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 16/24] softfloat: Add support for ties-away rounding

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> IEEE754-2008 specifies a new rounding mode:
> 
> "roundTiesToAway: the floating-point number nearest to the infinitely
> precise result shall be delivered; if the two nearest floating-point
> numbers bracketing an unrepresentable infinitely precise result are
> equally near, the one with larger magnitude shall be delivered."
> 
> Implement this new mode (it is needed for ARM). The general principle
> is that the required code is exactly like the ties-to-even code,
> except that we do not need to do the "in case of exact tie clear LSB
> to round-to-even", because the rounding operation naturally causes
> the exact tie to round up in magnitude.
> 
> Signed-off-by: Peter Maydell 
> ---
>  fpu/softfloat.c | 54 
> +
>  include/fpu/softfloat.h |  3 ++-
>  2 files changed, 56 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 15/24] softfloat: Refactor code handling various rounding modes

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> Refactor the code in various functions which calculates rounding
> increments given the current rounding mode, so that instead of a
> set of nested if statements we have a simple switch statement.
> This will give us a clean place to add the case for the new
> tiesAway rounding mode.
> 
> Signed-off-by: Peter Maydell 
> ---
>  fpu/softfloat.c | 405 
> +---
>  1 file changed, 241 insertions(+), 164 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 14/24] softfloat: Add float16 <=> float64 conversion functions

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> Add the conversion functions float16_to_float64() and
> float64_to_float16(), which will be needed for the ARM
> A64 instruction set.
> 
> Signed-off-by: Peter Maydell 
> ---
>  fpu/softfloat.c | 75 
> +
>  include/fpu/softfloat.h |  2 ++
>  2 files changed, 77 insertions(+)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 13/24] softfloat: Factor out RoundAndPackFloat16 and NormalizeFloat16Subnormal

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> In preparation for adding conversions between float16 and float64,
> factor out code currently done inline in the float16<=>float32
> conversion functions into functions RoundAndPackFloat16 and
> NormalizeFloat16Subnormal along the lines of the existing versions
> for the other float types.
> 
> Note that we change the handling of zExp from the inline code
> to match the API of the other RoundAndPackFloat functions; however
> we leave the positioning of the binary point between bits 22 and 23
> rather than shifting it up to the high end of the word.
> 
> Signed-off-by: Peter Maydell 
> ---
>  fpu/softfloat.c | 209 
> +---
>  1 file changed, 125 insertions(+), 84 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Wei Liu
On Mon, Jan 06, 2014 at 07:12:07PM +0100, Andreas Färber wrote:
> Am 06.01.2014 16:12, schrieb Wei Liu:
> > On Mon, Jan 06, 2014 at 01:30:20PM +, Peter Maydell wrote:
> >> On 6 January 2014 12:54, Wei Liu  wrote:
> >>> In fact I've already hacked a prototype during Christmas. What's I've
> >>> done so far:
> >>>
> >>> 1. create target-null which only has some stubs to CPU emulation
> >>>framework.
> >>>
> >>> 2. add a few lines to configure / Makefiles*, create
> >>>default-configs/null-softmmu
> >>
> >> I think it would be better to add support to allow you to
> >> configure with --disable-tcg. This would match the existing
> >> --disable/--enable switches for KVM and Xen, and then you
> >> could configure --disable-kvm --disable-tcg --enable-xen
> >> and get a qemu-system-i386 or qemu-system-arm with only
> >> the Xen support and none of the TCG emulation code.
> >>
> > 
> > In this case the architecture-specific code in target-* is still
> > included which might not help reduce the size much.
> 
> Define target-specific code in target-*? Most of that is TCG-specific
> and wouldn't be compiled in in that case. The KVM-specific bits don't
> get compiled in with --disable-kvm today already save for a few stubs.
> 

Probably I used the wrong terminology. I meant, say,
target-i386/translate.c, exec.c etc, which won't be necessary for Xen. I
guess that's what you mean by TCG-specific.  I see the possibility to
create some stubs for them, if that's what you mean.

> Adding yet another separate binary with no added functional value
> doesn't strike me as the most helpful idea for the community, compared
> to configure-optimizing the binaries built today.
> 
> Who would use the stripped-down binaries anyway? Just Citrix? Because
> SUSE is headed for sharing QEMU packages between Xen and KVM, so we
> couldn't enable such Xen-only-optimized binaries.
> 

No, I don't speak for Citrix. I work for opensource Xen project, I just
happen to be hired by Citrix. The general idea is to have an option for
user to create smaller binary, without those unnecessary code compiled /
linked in. How vendor makes their choice is out of my reach. :-)

Wei.

> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v2 08/24] softfloat: Add float32_to_uint64()

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> +return (int64_t)LIT64(0x);

Silly but harmless cast, considering the uint64_t return type.

Otherwise,

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 06/24] softfloat: Only raise Invalid when conversions to int are out of range

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> We implement a number of float-to-integer conversions using conversion
> to an integer type with a wider range and then a check against the
> narrower range we are actually converting to. If we find the result to
> be out of range we correctly raise the Invalid exception, but we must
> also suppress other exceptions which might have been raised by the
> conversion function we called.
> 
> This won't throw away exceptions we should have preserved, because for
> the 'core' exception flags the IEEE spec mandates that the only valid
> combinations of exception that can be raised by a single operation are
> Inexact + Overflow and Inexact + Underflow. For the non-IEEE softfloat
> flag for input denormals, we can guarantee that that flag won't have
> been set for out of range float-to-int conversions because a squashed
> denormal by definition goes to plus or minus zero, which is always in
> range after conversion to integer zero.
> 
> This bug has been fixed for some of the float-to-int conversion routines
> by previous patches; fix it for the remaining functions as well, so
> that they all restore the pre-conversion status flags prior to raising
> Invalid.
> 
> Signed-off-by: Peter Maydell 
> Reviewed-by: Aurelien Jarno 
> ---
>  fpu/softfloat.c | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson 


r~



[Qemu-devel] [RFC] PPC: KVM: add support for LPCR

2014-01-06 Thread Greg Kurz
The LPCR special purpose register was introduced with the PowerPC 970MP family.

This patch initializes LPCR for the following families:
- 970 MP
- POWER5+
- POWER7
- POWER8

Signed-off-by: Greg Kurz 
---

Along with Alexey's fixes for the get/put one reg API, I could drop
the target-ppc/kvm.c bits from:

http://patchwork.ozlabs.org/patch/300210/

and have cross-endian virtio working.

Thanks Alexey. :)

 target-ppc/translate_init.c |   16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 149a932..35470d4 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -2582,7 +2582,6 @@ static void gen_spr_8xx (CPUPPCState *env)
  * HSRR0   => SPR 314 (Power 2.04 hypv)
  * HSRR1   => SPR 315 (Power 2.04 hypv)
  * LPIDR   => SPR 317 (970)
- * LPCR=> SPR 318 (970)
  * EPR => SPR 702 (Power 2.04 emb)
  * perf=> 768-783 (Power 2.04)
  * perf=> 784-799 (Power 2.04)
@@ -6831,6 +6830,11 @@ static void init_proc_970MP (CPUPPCState *env)
  SPR_NOACCESS, SPR_NOACCESS,
  &spr_read_hior, &spr_write_hior,
  0x);
+/* Logical partitionning */
+spr_register_kvm(env, SPR_LPCR, "LPCR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG_PPC_LPCR, 0x);
 #if !defined(CONFIG_USER_ONLY)
 env->slb_nr = 32;
 #endif
@@ -6915,6 +6919,11 @@ static void init_proc_power5plus(CPUPPCState *env)
  &spr_read_generic, &spr_write_generic,
  &spr_read_generic, &spr_write_generic,
  0x);
+/* Logical partitionning */
+spr_register_kvm(env, SPR_LPCR, "LPCR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG_PPC_LPCR, 0x);
 #if !defined(CONFIG_USER_ONLY)
 env->slb_nr = 64;
 #endif
@@ -7019,6 +7028,11 @@ static void init_proc_POWER7 (CPUPPCState *env)
  &spr_read_generic, &spr_write_generic,
  &spr_read_generic, &spr_write_generic,
  0x);
+/* Logical partitionning */
+spr_register_kvm(env, SPR_LPCR, "LPCR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG_PPC_LPCR, 0x);
 #if !defined(CONFIG_USER_ONLY)
 env->slb_nr = 32;
 #endif




Re: [Qemu-devel] [PATCH v2 05/24] softfloat: Fix float64_to_uint64

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> From: Tom Musta 
> 
> The comment preceding the float64_to_uint64 routine suggests that
> the implementation is broken.  And this is, indeed, the case.
> 
> This patch properly implements the conversion of a 64-bit floating
> point number to an unsigned, 64 bit integer.
> 
> This contribution can be licensed under either the softfloat-2a or -2b
> license.
> 
> Signed-off-by: Tom Musta 
> Reviewed-by: Peter Maydell 
> Signed-off-by: Peter Maydell 
> ---
>  fpu/softfloat.c | 101 
> +++-
>  1 file changed, 93 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 04/24] softfloat: Make the int-to-float functions take exact-width types

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> Currently the int-to-float functions take types which are specified
> as "at least X bits wide", rather than "exactly X bits wide". This is
> confusing and unhelpful since it means that the callers have to include
> an explicit cast to [u]intXX_t to ensure the correct behaviour. Fix
> them all to take the exactly-X-bits-wide types instead.
> 
> Note that this doesn't change behaviour at all since at the moment
> we happen to define the 'int32' and 'uint32' types as exactly 32 bits
> wide, and the 'int64' and 'uint64' types as exactly 64 bits wide.
> 
> Signed-off-by: Peter Maydell 
> ---
>  fpu/softfloat.c | 26 +-
>  include/fpu/softfloat.h | 26 +-
>  2 files changed, 26 insertions(+), 26 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v6 03/11] dump: Add API to write vmcore

2014-01-06 Thread Laszlo Ersek
On 01/05/14 08:27, Qiao Nuohan wrote:
> Function is used to write vmcore. If flag_flatten is specified, flatten format
> will be used. In flatten format, data is written block by block in vmcore.
> struct MakedumpfileDataHeader is used to indicate the offset and size of a 
> data
> block.
> 
> struct MakedumpfileDataHeader {
> int64_t offset;
> int64_t buf_size;
> };
> 
> Signed-off-by: Qiao Nuohan 
> ---
>  dump.c |   28 
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 89baeab..764db39 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -726,6 +726,34 @@ static int write_end_flat_header(int fd)
>  return 0;
>  }
>  
> +static int write_buffer(int fd, bool flag_flatten, off_t offset, void *buf,
> +size_t size)
> +{

You might have wanted to const-qualify "*buf" here, but it certainly
doesn't warrant a respin.

Reviewed-by: Laszlo Ersek 



Re: [Qemu-devel] [PATCH v2 03/24] softfloat: Add 16 bit integer to float conversions

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:11 AM, Peter Maydell wrote:
> Add the float to 16 bit integer conversion routines. These can be
> trivially implemented in terms of the int32_to_float* routines, but
> providing them makes our API more symmetrical and can simplify callers.
> 
> Signed-off-by: Peter Maydell 
> ---
>  include/fpu/softfloat.h | 21 +
>  1 file changed, 21 insertions(+)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v2 01/24] softfloat: Fix exception flag handling for float32_to_float16()

2014-01-06 Thread Richard Henderson
On 01/06/2014 05:10 AM, Peter Maydell wrote:
> Our float32 to float16 conversion routine was generating the correct
> numerical answers, but not always setting the right set of exception
> flags. Fix this, mostly by rearranging the code to more closely
> resemble RoundAndPackFloat*, and in particular:
>  * non-IEEE halfprec always raises Invalid for input NaNs
>  * we need to check for the overflow case before underflow
>  * we weren't getting the tininess-detected-after-rounding
>case correct (somewhat academic since only ARM uses halfprec
>and it is always tininess-detected-before-rounding)
>  * non-IEEE halfprec overflow raises only Invalid, not
>Invalid + Inexact
>  * we weren't setting Inexact when we should
> 
> Also add some clarifying comments about what the code is doing.
> 
> Signed-off-by: Peter Maydell 
> ---
>  fpu/softfloat.c | 105 
> +++-
>  1 file changed, 66 insertions(+), 39 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Andreas Färber
Am 06.01.2014 16:12, schrieb Wei Liu:
> On Mon, Jan 06, 2014 at 01:30:20PM +, Peter Maydell wrote:
>> On 6 January 2014 12:54, Wei Liu  wrote:
>>> In fact I've already hacked a prototype during Christmas. What's I've
>>> done so far:
>>>
>>> 1. create target-null which only has some stubs to CPU emulation
>>>framework.
>>>
>>> 2. add a few lines to configure / Makefiles*, create
>>>default-configs/null-softmmu
>>
>> I think it would be better to add support to allow you to
>> configure with --disable-tcg. This would match the existing
>> --disable/--enable switches for KVM and Xen, and then you
>> could configure --disable-kvm --disable-tcg --enable-xen
>> and get a qemu-system-i386 or qemu-system-arm with only
>> the Xen support and none of the TCG emulation code.
>>
> 
> In this case the architecture-specific code in target-* is still
> included which might not help reduce the size much.

Define target-specific code in target-*? Most of that is TCG-specific
and wouldn't be compiled in in that case. The KVM-specific bits don't
get compiled in with --disable-kvm today already save for a few stubs.

Adding yet another separate binary with no added functional value
doesn't strike me as the most helpful idea for the community, compared
to configure-optimizing the binaries built today.

Who would use the stripped-down binaries anyway? Just Citrix? Because
SUSE is headed for sharing QEMU packages between Xen and KVM, so we
couldn't enable such Xen-only-optimized binaries.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Peter Maydell
On 6 January 2014 17:34, Stefano Stabellini
 wrote:
> On Mon, 6 Jan 2014, Peter Maydell wrote:
>> However I don't think we can have a qemu-system-null
>> (regardless of use cases) until/unless we get rid of
>> all the things which are compile-time decided by
>> the system config. In an ideal world we wouldn't have
>> any of those (and perhaps you could even build
>> support for more than one kind of CPU into one QEMU
>> binary), but as it is we do have them, and so a
>> qemu-system-null is not possible.
>
> What are these compile-time things you are referring to?

The identifiers poisoned by include/qemu/poison.h are
an initial but not complete list. Host and target
endianness is a particularly obvious one, as is the
size of a target long. You may not use these things
in your Xen devices, but "qemu-system-null" implies
more than "weird special purpose thing which only
has Xen devices in it".

Speaking of odd Xen special cases, it's pretty ugly
that builds with Xen enabled override the size of
ram_addr_t... maybe we should get rid of that special
casing one way or the other.

thanks
-- PMM



Re: [Qemu-devel] [Xen-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Stefano Stabellini
On Mon, 6 Jan 2014, Anthony Liguori wrote:
> On Mon, Jan 6, 2014 at 7:57 AM, Stefano Stabellini
>  wrote:
> > On Mon, 6 Jan 2014, Anthony Liguori wrote:
> >> On Jan 6, 2014 6:55 AM, "Stefano Stabellini" 
> >>  wrote:
> >> >
> >> > On Mon, 6 Jan 2014, Anthony Liguori wrote:
> >> > > On Jan 6, 2014 6:23 AM, "Peter Maydell"  
> >> > > wrote:
> >> > > >
> >> > > > On 6 January 2014 14:17, Stefano Stabellini
> >> > > >  wrote:
> >> > > > > It doesn't do any emulation so it is not specific to any 
> >> > > > > architecture or
> >> > > > > any cpu.
> >> > > >
> >> > > > You presumably still care about the compiled in values of
> >> > > > TARGET_WORDS_BIGENDIAN, TARGET_LONG_SIZE, and so on...
> >> >
> >> > Actually it only uses XC_PAGE_SIZE and the endianness is the host
> >> > endianness.
> >>
> >> If blkif in QEMU is relying on host endianness thats a bug.
> >
> > Why? Xen doesn't support a different guest/host endianness.
> 
> For the same reason that the virtio devices do not rely on host endianness.
> 
> It should be possible to use the Xen devices with TCG.  It isn't today
> simply because the code wasn't structured that way but it could be
> refactored to do this.
> 
> >> > > Yup.  It's still accel=xen just with no VCPUs.
> >> >
> >> > Are you talking about introducing accel=xen to Wei's target-null?
> >> > I guess that would work OK.
> >>
> >> We already have accel=xen.  I'm echoing Peter's suggestion of having the 
> >> ability to compile out accel=tcg.
> >>
> >> >
> >> > On the other hand if you are thinking of avoiding the introduction of a
> >> > new target-null, how would you make xen_machine_pv.c available to
> >> > multiple architectures?
> >>
> >> Why does qdisk need a full machine?
> >
> > qdisk is just one device, xen_machine_pv is the machine that initializes
> > the backend infrastructure (one of the backends is qdisk).
> > It doesn't make sense to use a full-blown machine like pc.c just to
> > start few backends, right?
> 
> What prevents xen_machine_pv from being compiled for multiple targets?
>  If there i386 specific things in it, they surely could be refactored
> a bit, no?

Not at all, but I thought that at the moment one machine has to be tied
to one architecture. If I am wrong, then there is no issue.



Re: [Qemu-devel] [Xen-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Andreas Färber
Am 06.01.2014 16:39, schrieb Anthony Liguori:
> We already have accel=xen.  I'm echoing Peter's suggestion of having the
> ability to compile out accel=tcg.

Didn't you and Paolo even have patches for that a while ago?

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [Xen-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Anthony Liguori
On Mon, Jan 6, 2014 at 7:57 AM, Stefano Stabellini
 wrote:
> On Mon, 6 Jan 2014, Anthony Liguori wrote:
>> On Jan 6, 2014 6:55 AM, "Stefano Stabellini" 
>>  wrote:
>> >
>> > On Mon, 6 Jan 2014, Anthony Liguori wrote:
>> > > On Jan 6, 2014 6:23 AM, "Peter Maydell"  wrote:
>> > > >
>> > > > On 6 January 2014 14:17, Stefano Stabellini
>> > > >  wrote:
>> > > > > It doesn't do any emulation so it is not specific to any 
>> > > > > architecture or
>> > > > > any cpu.
>> > > >
>> > > > You presumably still care about the compiled in values of
>> > > > TARGET_WORDS_BIGENDIAN, TARGET_LONG_SIZE, and so on...
>> >
>> > Actually it only uses XC_PAGE_SIZE and the endianness is the host
>> > endianness.
>>
>> If blkif in QEMU is relying on host endianness thats a bug.
>
> Why? Xen doesn't support a different guest/host endianness.

For the same reason that the virtio devices do not rely on host endianness.

It should be possible to use the Xen devices with TCG.  It isn't today
simply because the code wasn't structured that way but it could be
refactored to do this.

>> > > Yup.  It's still accel=xen just with no VCPUs.
>> >
>> > Are you talking about introducing accel=xen to Wei's target-null?
>> > I guess that would work OK.
>>
>> We already have accel=xen.  I'm echoing Peter's suggestion of having the 
>> ability to compile out accel=tcg.
>>
>> >
>> > On the other hand if you are thinking of avoiding the introduction of a
>> > new target-null, how would you make xen_machine_pv.c available to
>> > multiple architectures?
>>
>> Why does qdisk need a full machine?
>
> qdisk is just one device, xen_machine_pv is the machine that initializes
> the backend infrastructure (one of the backends is qdisk).
> It doesn't make sense to use a full-blown machine like pc.c just to
> start few backends, right?

What prevents xen_machine_pv from being compiled for multiple targets?
 If there i386 specific things in it, they surely could be refactored
a bit, no?

Regards,

Anthony Liguori

>
>
>> How would you avoid the compilation of all the
>> > unnecessary emulated devices?
>>
>> Device config files.



Re: [Qemu-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Stefano Stabellini
On Mon, 6 Jan 2014, Peter Maydell wrote:
> On 6 January 2014 15:11, Wei Liu  wrote:
> > On Mon, Jan 06, 2014 at 11:23:24PM +1000, Peter Crosthwaite wrote:
> > [...]
> >> >
> >> > Finally I got a qemu-system-null. And the effect is immediately visible
> >>
> >> qemu-system-null has been on my wish-list in the past, although my
> >> reasons were slightly different to yours. Specifically, the goal was
> >> to test CPUs in an RTL simulator interacting with RAM and peripheral
> >> devices hosted in QEMU.
> 
> > Cool. However small this is still a valid usecase.
> 
> However I don't think we can have a qemu-system-null
> (regardless of use cases) until/unless we get rid of
> all the things which are compile-time decided by
> the system config. In an ideal world we wouldn't have
> any of those (and perhaps you could even build
> support for more than one kind of CPU into one QEMU
> binary), but as it is we do have them, and so a
> qemu-system-null is not possible.

What are these compile-time things you are referring to?



Re: [Qemu-devel] [PATCHv3 3/6] ui/vnc: optimize dirty bitmap tracking

2014-01-06 Thread Peter Lieven
On 06.01.2014 11:08, Wenchao Xia wrote:
> 于 2014/1/6 2:02, Peter Lieven 写道:
>> vnc_update_client currently scans the dirty bitmap of each client
>> bitwise which is a very costly operation if only few bits are dirty.
>> vnc_refresh_server_surface does almost the same.
>> this patch optimizes both by utilizing the heavily optimized
>> function find_next_bit to find the offset of the next dirty
>> bit in the dirty bitmaps.
>>
>> The following artifical test (just the bitmap operation part) running
>> vnc_update_client 65536 times on a 2560x2048 surface illustrates the
>> performance difference:
>>
>> All bits clean - vnc_update_client_new: 0.07 secs
>>   vnc_update_client_old: 10.98 secs
>>
>> All bits dirty - vnc_update_client_new: 11.26 secs
>>   vnc_update_client_old: 20.19 secs
>>
>> Few bits dirty - vnc_update_client_new: 0.08 secs
>>   vnc_update_client_old: 10.98 secs
>>
>> The case for all bits dirty is still rather slow, this
>> is due to the implementation of find_and_clear_dirty_height.
>> This will be addresses in a separate patch.
>>
>> Signed-off-by: Peter Lieven 
>> ---
>>   ui/vnc.c |  154 
>> +-
>>   ui/vnc.h |4 ++
>>   2 files changed, 87 insertions(+), 71 deletions(-)
>>
>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index 1d2aa1a..6a0c03e 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -572,6 +572,14 @@ void *vnc_server_fb_ptr(VncDisplay *vd, int x, int y)
>>   ptr += x * VNC_SERVER_FB_BYTES;
>>   return ptr;
>>   }
>> +/* this sets only the visible pixels of a dirty bitmap */
>> +#define VNC_SET_VISIBLE_PIXELS_DIRTY(bitmap, w, h) {\
>> +int y;\
>> +memset(bitmap, 0x00, sizeof(bitmap));\
>> +for (y = 0; y < h; y++) {\
>> +bitmap_set(bitmap[y], 0, w / VNC_DIRTY_PIXELS_PER_BIT);\
>   Will it be a problem when vnc's width % VNC_DIRTY_PIXELS_PER_BIT != 0?
> Although it is a rare case, but I think it is better round it up, since
> "v" and "VNC_DIRTY_PIXELS_PER_BIT" are variables. A macro computing it
> would be nice:
>
> #define VNC_DIRTY_BITS_FROM_WIDTH(w) (w + VNC_DIRTY_PIXELS_PER_BIT - 1/
> VNC_DIRTY_PIXELS_PER_BIT)
> #define VNC_DIRTY_BITS (VNC_DIRTY_BITS_FROM_WIDTH(VNC_MAX_WIDTH)
>
> then here:
> bitmap_set(bitmap[y], 0, VNC_DIRTY_BITS_FROM_WIDTH(w));
>
> Or simply warn or coredump when v % VNC_DIRTY_PIXELS_PER_BIT != 0.
>
> Also, in vnc.h:
> /* VNC_MAX_WIDTH must be a multiple of 16. */
> #define VNC_MAX_WIDTH 2560
> #define VNC_MAX_HEIGHT 2048
>
> Maybe it should be updated as:
> /* VNC_MAX_WIDTH must be a multiple of VNC_DIRTY_PIXELS_PER_BIT. */
>
>> +} \
>> +}
>>
>>   static void vnc_dpy_switch(DisplayChangeListener *dcl,
>>  DisplaySurface *surface)
>> @@ -597,7 +605,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
>>   qemu_pixman_image_unref(vd->guest.fb);
>>   vd->guest.fb = pixman_image_ref(surface->image);
>>   vd->guest.format = surface->format;
>> -memset(vd->guest.dirty, 0xFF, sizeof(vd->guest.dirty));
>> +VNC_SET_VISIBLE_PIXELS_DIRTY(vd->guest.dirty,
>> + surface_width(vd->ds),
>> + surface_height(vd->ds));
>>
>>   QTAILQ_FOREACH(vs, &vd->clients, next) {
>>   vnc_colordepth(vs);
>> @@ -605,7 +615,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
>>   if (vs->vd->cursor) {
>>   vnc_cursor_define(vs);
>>   }
>> -memset(vs->dirty, 0xFF, sizeof(vs->dirty));
>> +VNC_SET_VISIBLE_PIXELS_DIRTY(vs->dirty,
>> + surface_width(vd->ds),
>> + surface_height(vd->ds));
>>   }
>>   }
>>
>> @@ -889,10 +901,9 @@ static int vnc_update_client(VncState *vs, int 
>> has_dirty)
>>   VncDisplay *vd = vs->vd;
>>   VncJob *job;
>>   int y;
>> -int width, height;
>> +int height;
>>   int n = 0;
>>
>> -
>>   if (vs->output.offset && !vs->audio_cap && !vs->force_update)
>>   /* kernel send buffers are full -> drop frames to throttle */
>>   return 0;
>> @@ -908,39 +919,27 @@ static int vnc_update_client(VncState *vs, int 
>> has_dirty)
>>*/
>>   job = vnc_job_new(vs);
>>
>> -width = MIN(pixman_image_get_width(vd->server), vs->client_width);
>>   height = MIN(pixman_image_get_height(vd->server), 
>> vs->client_height);
>>
>> -for (y = 0; y < height; y++) {
>> -int x;
>> -int last_x = -1;
>> -for (x = 0; x < width / VNC_DIRTY_PIXELS_PER_BIT; x++) {
>> -if (test_and_clear_bit(x, vs->dirty[y])) {
>> -if (last_x == -1) {
>> -last_x = x;
>> -}
>> -} else {
>> -if (last_x != -1) {
>> -int h = f

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] target-ppc: fix Authority Mask Register init value

2014-01-06 Thread Greg Kurz
On Mon,  6 Jan 2014 16:36:39 +1100
Alexey Kardashevskiy  wrote:
> The existing default value (-1) of the AMR register forbids data access
> to all 32 classes. Since the guest linux does not change this register,
> we end up with the guest hanging right after switching from the real to
> protected mode.
> 
> This sets the default AMR value to zero what enables data access for all
> classes.
> 
> The only reason for not hitting this bug before is that
> kvm_arch_put_registers() did not put any SPR to KVM due to missing
> assignment of @one_reg_id in _spr_register() (which is going to be fixed
> by a separate patch).
> 
> Signed-off-by: Alexey Kardashevskiy 

Reviewed-by: Greg Kurz 

> ---
>  target-ppc/translate_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 93ad762..144de3d 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -1064,7 +1064,7 @@ static void gen_spr_amr (CPUPPCState *env)
>  spr_register_kvm(env, SPR_AMR, "AMR",
>   SPR_NOACCESS, SPR_NOACCESS,
>   &spr_read_generic, &spr_write_generic,
> - KVM_REG_PPC_AMR, 0xULL);
> + KVM_REG_PPC_AMR, 0);
>  spr_register_kvm(env, SPR_UAMOR, "UAMOR",
>   SPR_NOACCESS, SPR_NOACCESS,
>   &spr_read_generic, &spr_write_generic,



-- 
Gregory Kurz kurzg...@fr.ibm.com
 gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
Alan Moore.




Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/2] PPC: KVM: fix "set one register"

2014-01-06 Thread Greg Kurz
On Mon,  6 Jan 2014 16:36:40 +1100
Alexey Kardashevskiy  wrote:

> Due to missing @one_reg_id assignment in _spr_register(),
> the kvm_get_one_reg/kvm_set_one_reg API has never really been working.
> 
> This enabled the API and removes use of the API for LPCR as
> kvm_arch_get_registers/kvm_arch_put_registers run a loop for all 1024 SPR
> and LPCR is one of them.
> 
> Signed-off-by: Alexey Kardashevskiy 

Reviewed-by: Greg Kurz 

> ---
>  target-ppc/translate_init.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 144de3d..149a932 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -628,6 +628,9 @@ static inline void _spr_register(CPUPPCState *env,
> int num, spr->oea_read = oea_read;
>  spr->oea_write = oea_write;
>  #endif
> +#if defined(CONFIG_KVM)
> +spr->one_reg_id = one_reg_id,
> +#endif
>  env->spr[num] = initial_value;
>  }
> 



-- 
Gregory Kurz kurzg...@fr.ibm.com
 gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
Alan Moore.




Re: [Qemu-devel] [PATCH v2] linux-user: Support the accept4 socketcall

2014-01-06 Thread Peter Maydell
On 6 January 2014 17:07, Laurent Vivier  wrote:
>
>> Le 6 janvier 2014 à 17:15, André Hentschel  a écrit :
>>
>>
>> From: André Hentschel 
>> Cc: Riku Voipio 
>> Signed-off-by: André Hentschel 
>> ---
>> See
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/net.h
>> for the value.
>>
>> linux-user/syscall.c | 16 
>> linux-user/syscall_defs.h | 1 +
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index efd1453..1a848a6 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -2245,6 +2245,22 @@ static abi_long do_socketcall(int num, abi_ulong
>> vptr)
>> ret = do_accept4(sockfd, target_addr, target_addrlen, 0);
>> }
>> break;
>> + case SOCKOP_accept4:
>> + {
>> + abi_ulong sockfd;
>> + abi_ulong target_addr, target_addrlen;
>> + int flags;
>> +
>> + if (get_user_ual(sockfd, vptr)
>> + || get_user_ual(target_addr, vptr + n)
>> + || get_user_ual(target_addrlen, vptr + 2 * n)
>> + || get_user_ual(flags, vptr + 3 * n)) {
>
> I'm not sure, but I think as get_user_ual() get an abi_ulong, flags should
> be an abi_ulong.

It's not required for correctness (since get_user_ual
always does an abi_ulong sized access of guest memory
regardless of the size of its first argument) but I
think it would be clearer, yes.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v6 02/11] dump: Add API to write header of flatten format

2014-01-06 Thread Laszlo Ersek
On 01/05/14 08:27, Qiao Nuohan wrote:
> flatten format may be used when writing kdump-compressed format. The format is
> also used by makedumpfile, you can refer to the following URL to get more
> detailed information about flatten format of kdump-compressed format:
> http://sourceforge.net/projects/makedumpfile/
> 
> The two functions here are used to write start flat header and end flat header
> to vmcore, and they will be called later if flatten format is used.
> 
> struct MakedumpfileHeader stored at the head of vmcore is used to indicate the
> vmcore is in flatten format.
> 
> struct MakedumpfileHeader {
> char signature[16]; /* = "makedumpfile" */
> int64_t type;   /* = 1 */
> int64_t version;/* = 1 */
> };
> 
> And struct MakedumpfileDataHeader, with offset and buf_size set to -1, is used
> to indicate the end of vmcore in flatten format.
> 
> struct MakedumpfileDataHeader {
> int64_t offset; /* = -1 */
> int64_t buf_size;   /* = -1 */
> };
> 
> Signed-off-by: Qiao Nuohan 
> ---
>  dump.c|   40 
>  include/sysemu/dump.h |   17 +
>  2 files changed, 57 insertions(+), 0 deletions(-)
> 
> diff --git a/dump.c b/dump.c
> index 1fa12a2..89baeab 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -686,6 +686,46 @@ static int create_vmcore(DumpState *s)
>  return 0;
>  }
>  
> +static int write_start_flat_header(int fd)
> +{
> +char buf[MAX_SIZE_MDF_HEADER];
> +MakedumpfileHeader mh;
> +
> +memset(&mh, 0, sizeof(mh));
> +strncpy(mh.signature, MAKEDUMPFILE_SIGNATURE,
> +strlen(MAKEDUMPFILE_SIGNATURE));
> +
> +mh.type = cpu_to_be64(TYPE_FLAT_HEADER);
> +mh.version = cpu_to_be64(VERSION_FLAT_HEADER);
> +
> +memset(buf, 0, sizeof(buf));
> +memcpy(buf, &mh, sizeof(mh));
> +
> +size_t written_size;
> +written_size = qemu_write_full(fd, buf, sizeof(buf));
> +if (written_size != sizeof(buf)) {
> +return -1;
> +}
> +
> +return 0;
> +}

I might have coded this in different style (using a union etc), but
that's not your problem :)

> [...]

Reviewed-by: Laszlo Ersek 




Re: [Qemu-devel] [PATCH v2] linux-user: Support the accept4 socketcall

2014-01-06 Thread Laurent Vivier

> Le 6 janvier 2014 à 17:15, André Hentschel  a écrit :
>
>
> From: André Hentschel 
> Cc: Riku Voipio 
> Signed-off-by: André Hentschel 
> ---
> See
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/net.h
> for the value.
>
> linux-user/syscall.c | 16 
> linux-user/syscall_defs.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index efd1453..1a848a6 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2245,6 +2245,22 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
> ret = do_accept4(sockfd, target_addr, target_addrlen, 0);
> }
> break;
> + case SOCKOP_accept4:
> + {
> + abi_ulong sockfd;
> + abi_ulong target_addr, target_addrlen;
> + int flags;
> +
> + if (get_user_ual(sockfd, vptr)
> + || get_user_ual(target_addr, vptr + n)
> + || get_user_ual(target_addrlen, vptr + 2 * n)
> + || get_user_ual(flags, vptr + 3 * n)) {

I'm not sure, but I think as get_user_ual() get an abi_ulong, flags should be an
abi_ulong. Peter ?

> + return -TARGET_EFAULT;
> + }
> +
> + ret = do_accept4(sockfd, target_addr, target_addrlen, flags);
> + }
> + break;
> case SOCKOP_getsockname:
> {
> abi_ulong sockfd;
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index cf08db5..ae30476 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -27,6 +27,7 @@
> #define SOCKOP_getsockopt 15
> #define SOCKOP_sendmsg 16
> #define SOCKOP_recvmsg 17
> +#define SOCKOP_accept4 18
>
> #define IPCOP_semop 1
> #define IPCOP_semget 2
> --
> 1.8.1.2
>
>

Re: [Qemu-devel] [PATCH 07/13] mxs/imx23: Implements the pin mux, GPIOs

2014-01-06 Thread Peter Maydell
On 11 December 2013 13:56, Michel Pollet  wrote:
> Implements the pinctrl and GPIO block for the imx23
> It handles GPIO output, and GPIO input from qemu translated
> into pin values and interrupts, if appropriate.
>
> Signed-off-by: Michel Pollet 
> ---
>  hw/arm/Makefile.objs   |   2 +-
>  hw/arm/imx23_pinctrl.c | 293 
> +
>  2 files changed, 294 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/imx23_pinctrl.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 9adcb96..ea53988 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -5,4 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o 
> z2.o
>
>  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>  obj-y += omap1.o omap2.o strongarm.o
> -obj-$(CONFIG_MXS) += imx23_digctl.o
> +obj-$(CONFIG_MXS) += imx23_digctl.o imx23_pinctrl.o
> diff --git a/hw/arm/imx23_pinctrl.c b/hw/arm/imx23_pinctrl.c
> new file mode 100644
> index 000..ecfb755
> --- /dev/null
> +++ b/hw/arm/imx23_pinctrl.c
> @@ -0,0 +1,293 @@
> +/*
> + * imx23_pinctrl.c
> + *
> + * Copyright: Michel Pollet 
> + *
> + * QEMU Licence
> + */
> +
> +/*
> + * Implements the pinctrl and GPIO block for the imx23
> + * It handles GPIO output, and GPIO input from qemu translated
> + * into pin values and interrupts, if appropriate.
> + */
> +#include "hw/sysbus.h"
> +#include "hw/arm/mxs.h"
> +
> +#define D(w)
> +
> +enum {
> +PINCTRL_BANK_COUNT = 3,
> +
> +PINCTRL_CTRL = 0,
> +PINCTRL_BANK_MUXSEL = 0x10,
> +PINCTRL_BANK_BASE = 0x40,
> +
> +/* these are not << 4 register numbers, these are << 8 register numbers 
> */
> +PINCTRL_BANK_PULL = 0x4,
> +PINCTRL_BANK_OUT = 0x5,
> +PINCTRL_BANK_DIN = 0x6,
> +PINCTRL_BANK_DOE = 0x7,
> +PINCTRL_BANK_PIN2IRQ = 0x8,
> +PINCTRL_BANK_IRQEN = 0x9,
> +PINCTRL_BANK_IRQLEVEL = 0xa,
> +PINCTRL_BANK_IRQPOL = 0xb,
> +PINCTRL_BANK_IRQSTAT = 0xc,
> +
> +PINCTRL_BANK_INTERNAL_STATE = 0xd,
> +PINCTRL_MAX = 0xe0,
> +};
> +
> +#define PINCTRL_BANK_REG(_bank, _reg) ((_reg << 8) | (_bank << 4))
> +
> +enum {
> +MUX_GPIO = 0x3,
> +};
> +
> +
> +typedef struct imx23_pinctrl_state {
> +SysBusDevice busdev;
> +MemoryRegion iomem;
> +
> +uint32_t r[PINCTRL_MAX];
> +qemu_irq irq_in[3];
> +qemu_irq irq_out[PINCTRL_BANK_COUNT * 32];
> +
> +uint32_t state[PINCTRL_BANK_COUNT];
> +} imx23_pinctrl_state;
> +
> +static uint64_t imx23_pinctrl_read(
> +void *opaque, hwaddr offset, unsigned size)
> +{
> +imx23_pinctrl_state *s = (imx23_pinctrl_state *) opaque;
> +uint32_t res = 0;
> +
> +switch (offset >> 4) {
> +case 0 ... PINCTRL_MAX:
> +res = s->r[offset >> 4];
> +break;
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: bad offset 0x%x\n", __func__, (int) offset);
> +break;
> +}
> +
> +return res;
> +}
> +
> +static uint8_t imx23_pinctrl_getmux(
> +imx23_pinctrl_state *s, int pin)
> +{
> +int base = pin / 16, offset = pin % 16;
> +return (s->r[PINCTRL_BANK_MUXSEL + base] >> (offset * 2)) & 0x3;
> +}
> +
> +/*
> + * usage imx23_pinctrl_getbit(s, PINCTRL_BANK_IRQEN, 48)...
> + */
> +static uint8_t imx23_pinctrl_getbit(
> +imx23_pinctrl_state *s, uint16_t reg, int pin)
> +{
> +int bank = pin / 32, offset = pin % 32;
> +uint32_t * latch = &s->r[PINCTRL_BANK_REG(bank, reg) >> 4];
> +//printf("%s bank %d offset %d reg %d : %04x=%08x\n", __func__, bank, 
> offset, reg,
> +//PINCTRL_BANK_REG(bank, reg),
> +//*latch);
> +return (*latch >> offset) & 0x1;
> +}
> +
> +static void imx23_pinctrl_setbit(
> +imx23_pinctrl_state *s, uint16_t reg, int pin, int value)
> +{
> +int bank = pin / 32, offset = pin % 32;
> +uint32_t * latch = &s->r[PINCTRL_BANK_REG(bank, reg) >> 4];
> +*latch = (*latch & ~(1 << offset)) | (!!value << offset);

deposit32() will make this clearer to read.

> +}
> +
> +static void imx23_pinctrl_write_bank(
> +imx23_pinctrl_state *s, int bank,
> +int reg, uint32_t value,
> +uint32_t mask)
> +{
> +int set, pin;
> +switch (reg) {
> +/*
> + * Linux has a way of using the DOE&PULL register to toggle the pin
> + */

Why is this comment here? We should ideally not care what
guest OS we run, we should just implement the h/w correctly.

> +case PINCTRL_BANK_PULL:
> +case PINCTRL_BANK_DOE:
> +/*
> + * Writing to the Data OUT register just triggers the
> + * output qemu IRQ for any further peripherals
> + */
> +case PINCTRL_BANK_OUT: {
> +while ((set = ffs(mask)) > 0) {
> +set--;
> +mask &= ~(1 << set);
> +pin = (bank * 32) + set;
> +/*
> + * For a reason that is not clear, the pullup bit

Re: [Qemu-devel] [PATCH v6 01/11] dump: Add argument to write_elfxx_notes

2014-01-06 Thread Laszlo Ersek
On 01/05/14 08:27, Qiao Nuohan wrote:
> write_elf32_notes/wirte_elf64_notes use fd_write_vmcore to write elf notes to
> vmcore. Adding parameter "WriteCoreDumpFunction f" makes it available to 
> choose
> the method of writing elf notes
> 
> Signed-off-by: Qiao Nuohan 
> ---
>  dump.c |   16 
>  1 files changed, 8 insertions(+), 8 deletions(-)

I assume the direct calls to fd_write_vmcore() (which we're not
replacing here) will be substituted / abstracted later on in the series.

Reviewed-by: Laszlo Ersek 




Re: [Qemu-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Peter Maydell
On 6 January 2014 15:11, Wei Liu  wrote:
> On Mon, Jan 06, 2014 at 11:23:24PM +1000, Peter Crosthwaite wrote:
> [...]
>> >
>> > Finally I got a qemu-system-null. And the effect is immediately visible
>>
>> qemu-system-null has been on my wish-list in the past, although my
>> reasons were slightly different to yours. Specifically, the goal was
>> to test CPUs in an RTL simulator interacting with RAM and peripheral
>> devices hosted in QEMU.

> Cool. However small this is still a valid usecase.

However I don't think we can have a qemu-system-null
(regardless of use cases) until/unless we get rid of
all the things which are compile-time decided by
the system config. In an ideal world we wouldn't have
any of those (and perhaps you could even build
support for more than one kind of CPU into one QEMU
binary), but as it is we do have them, and so a
qemu-system-null is not possible.

thanks
-- PMM



[Qemu-devel] [PATCH v2] linux-user: Support the accept4 socketcall

2014-01-06 Thread André Hentschel
From: André Hentschel 
Cc: Riku Voipio 
Signed-off-by: André Hentschel 
---
See 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/net.h
 for the value.

 linux-user/syscall.c  | 16 
 linux-user/syscall_defs.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index efd1453..1a848a6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2245,6 +2245,22 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
 ret = do_accept4(sockfd, target_addr, target_addrlen, 0);
 }
 break;
+case SOCKOP_accept4:
+{
+abi_ulong sockfd;
+abi_ulong target_addr, target_addrlen;
+int flags;
+
+if (get_user_ual(sockfd, vptr)
+|| get_user_ual(target_addr, vptr + n)
+|| get_user_ual(target_addrlen, vptr + 2 * n)
+|| get_user_ual(flags, vptr + 3 * n)) {
+return -TARGET_EFAULT;
+}
+
+ret = do_accept4(sockfd, target_addr, target_addrlen, flags);
+}
+break;
 case SOCKOP_getsockname:
 {
 abi_ulong sockfd;
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index cf08db5..ae30476 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -27,6 +27,7 @@
 #define SOCKOP_getsockopt   15
 #define SOCKOP_sendmsg  16
 #define SOCKOP_recvmsg  17
+#define SOCKOP_accept4  18
 
 #define IPCOP_semop1
 #define IPCOP_semget   2
-- 
1.8.1.2




Re: [Qemu-devel] [PATCH resend] linux-user: Support the accept4 socketcall

2014-01-06 Thread Peter Maydell
On 6 January 2014 15:38, André Hentschel  wrote:
> This warning seems wrong:
>  - the if statement has no braces and only one arm
>  - the if statement looks like the others around it, i just try to keep the 
> same style

The warning is saying that you need braces. "all arms" for an
if() with only one arm means "on that one arm". See the
CODING_STYLE file's section on 'block structure'.

QEMU includes a fair amount of legacy code which doesn't
follow our coding standards. Our general policy on this is that
new code follows the standards, and when a patch touches
old code it updates it (at least sufficiently to get checkpatch
to be happy). We don't generally do large-scale "update whole
file to new standard" patches as this breaks git blame and
can cause unnecessary patch conflicts.

thanks
-- PMM



Re: [Qemu-devel] [Xen-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Stefano Stabellini
On Mon, 6 Jan 2014, Anthony Liguori wrote:
> On Jan 6, 2014 6:55 AM, "Stefano Stabellini" 
>  wrote:
> >
> > On Mon, 6 Jan 2014, Anthony Liguori wrote:
> > > On Jan 6, 2014 6:23 AM, "Peter Maydell"  wrote:
> > > >
> > > > On 6 January 2014 14:17, Stefano Stabellini
> > > >  wrote:
> > > > > It doesn't do any emulation so it is not specific to any architecture 
> > > > > or
> > > > > any cpu.
> > > >
> > > > You presumably still care about the compiled in values of
> > > > TARGET_WORDS_BIGENDIAN, TARGET_LONG_SIZE, and so on...
> >
> > Actually it only uses XC_PAGE_SIZE and the endianness is the host
> > endianness.
> 
> If blkif in QEMU is relying on host endianness thats a bug.

Why? Xen doesn't support a different guest/host endianness.


> > > Yup.  It's still accel=xen just with no VCPUs.
> >
> > Are you talking about introducing accel=xen to Wei's target-null?
> > I guess that would work OK.
> 
> We already have accel=xen.  I'm echoing Peter's suggestion of having the 
> ability to compile out accel=tcg.
> 
> >
> > On the other hand if you are thinking of avoiding the introduction of a
> > new target-null, how would you make xen_machine_pv.c available to
> > multiple architectures?
> 
> Why does qdisk need a full machine?

qdisk is just one device, xen_machine_pv is the machine that initializes
the backend infrastructure (one of the backends is qdisk).
It doesn't make sense to use a full-blown machine like pc.c just to
start few backends, right?


> How would you avoid the compilation of all the
> > unnecessary emulated devices?
> 
> Device config files.

Re: [Qemu-devel] [PATCH] discard rbd error output when not relevant in qemu-iotests

2014-01-06 Thread Loic Dachary


On 06/01/2014 03:23, Stefan Hajnoczi wrote:
> On Mon, Dec 30, 2013 at 01:33:34AM +0100, Loic Dachary wrote:
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 28ba0d9..af66bbd 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -189,7 +189,11 @@ _cleanup_test_img()
>>  ;;
>>  
>>  rbd)
>> -rbd rm "$TEST_DIR/t.$IMGFMT" > /dev/null
> 
> stderr will be displayed.  Why isn't this enough?
> 

Because the progress of the removal operation will be displayed on stderr. rbd 
outputs on stderr even when there is no error.

Happy new year !

-- 
Loïc Dachary, Artisan Logiciel Libre



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 06/13] mxs/imx23: Add digctl driver

2014-01-06 Thread Peter Maydell
On 11 December 2013 13:56, Michel Pollet  wrote:
> This implements just enough of the digctl IO block to allow
> linux to believe it's running on (currently only) an imx23.
>
> Signed-off-by: Michel Pollet 
> ---
>  hw/arm/Makefile.objs  |   1 +
>  hw/arm/imx23_digctl.c | 110 
> ++
>  2 files changed, 111 insertions(+)
>  create mode 100644 hw/arm/imx23_digctl.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 78b5614..9adcb96 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o 
> z2.o
>
>  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>  obj-y += omap1.o omap2.o strongarm.o
> +obj-$(CONFIG_MXS) += imx23_digctl.o
> diff --git a/hw/arm/imx23_digctl.c b/hw/arm/imx23_digctl.c
> new file mode 100644
> index 000..b7cd1ff
> --- /dev/null
> +++ b/hw/arm/imx23_digctl.c
> @@ -0,0 +1,110 @@
> +/*
> + * imx23_digctl.c
> + *
> + * Copyright: Michel Pollet 
> + *
> + * QEMU Licence
> + */
> +
> +/*
> + * This module implements a very basic IO block for the digctl of the imx23
> + * Basically there is no real logic, just constant registers return, the most
> + * used one bing the "chip id" that is used by the various linux drivers
> + * to differentiate between imx23 and 28.
> + *
> + * The module consists mostly of read/write registers that the bootloader and
> + * kernel are quite happy to 'set' to whatever value they believe they set...
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm/mxs.h"
> +
> +enum {
> +HW_DIGCTL_RAMCTL = 0x3,
> +HW_DIGCTL_CHIPID = 0x31,
> +};
> +
> +typedef struct imx23_digctl_state {
> +SysBusDevice busdev;
> +MemoryRegion iomem;
> +
> +uint32_t   reg[0x2000 / 4];
> +} imx23_digctl_state;

I'm not generally a fan of "big reg[] array" like this.
In real hardware are these typically constant read/only
registers, or do they actually do something? Does the
hardware really have a full set of 2048 registers here,
or are there gaps?

I'd rather have us implement just the minimal set
required for things to boot, with LOG_UNIMP (and
read-zero/write-ignored) for the rest. That makes
it easier to add actual implementations later
(and your migration state is not 0x2000 bytes of random
undifferentiated stuff).

thanks
-- PMM



Re: [Qemu-devel] [PATCH 04/13] mxs/imx23: Add DMA driver

2014-01-06 Thread Peter Maydell
On 11 December 2013 13:56, Michel Pollet  wrote:
> This driver works sufficiently well that linux can use it to access
> the SD card using the SD->DMA->SSI->SD. It hasn't been tested for
> much else.
>
> Signed-off-by: Michel Pollet 
> ---
>  hw/dma/Makefile.objs |   1 +
>  hw/dma/mxs_dma.c | 347 
> +++
>  2 files changed, 348 insertions(+)
>  create mode 100644 hw/dma/mxs_dma.c
>
> diff --git a/hw/dma/Makefile.objs b/hw/dma/Makefile.objs
> index 0e65ed0..3373aa1 100644
> --- a/hw/dma/Makefile.objs
> +++ b/hw/dma/Makefile.objs
> @@ -8,6 +8,7 @@ common-obj-$(CONFIG_XILINX_AXI) += xilinx_axidma.o
>  common-obj-$(CONFIG_ETRAXFS) += etraxfs_dma.o
>  common-obj-$(CONFIG_STP2000) += sparc32_dma.o
>  common-obj-$(CONFIG_SUN4M) += sun4m_iommu.o
> +common-obj-$(CONFIG_MXS) += mxs_dma.o
>
>  obj-$(CONFIG_OMAP) += omap_dma.o soc_dma.o
>  obj-$(CONFIG_PXA2XX) += pxa2xx_dma.o
> diff --git a/hw/dma/mxs_dma.c b/hw/dma/mxs_dma.c
> new file mode 100644
> index 000..9ac787b
> --- /dev/null
> +++ b/hw/dma/mxs_dma.c
> @@ -0,0 +1,347 @@
> +/*
> + * mxs_dma.c
> + *
> + * Copyright: Michel Pollet 
> + *
> + * QEMU Licence
> + */
> +
> +/*
> + * Implements the DMA block of the mxs.
> + * The current implementation can run chains of commands etc, however it's 
> only
> + * been tested with SSP for SD/MMC card access. It ought to work with normal 
> SPI
> + * too, and possibly other peripherals, however it's entirely untested
> + */
> +#include "hw/sysbus.h"
> +#include "hw/arm/mxs.h"
> +
> +/*
> + * DMA IO block register numbers
> + */
> +enum {
> +DMA_CTRL0 = 0x0,
> +DMA_CTRL1 = 0x1,
> +DMA_CTRL2 = 0x2,
> +DMA_DEVSEL1 = 0x3,
> +DMA_DEVSEL2 = 0x4,
> +DMA_MAX,
> +
> +/*
> + * The DMA block for APBH and APBX have a different base address,
> + * but they share a 7 words stride between channels.
> + */
> +DMA_STRIDE = 0x70,
> +/*
> + * Neither blocks uses that many, but there is space for them...
> + */
> +DMA_MAX_CHANNELS = 16,
> +};
> +
> +/*
> + * DMA channel register numbers
> + */
> +enum {
> +CH_CURCMD = 0,
> +CH_NEXTCMD = 1,
> +CH_CMD = 2,
> +CH_BUFFER_ADDR = 3,
> +CH_SEMA = 4,
> +CH_DEBUG1 = 5,
> +CH_DEBUG2 = 6,
> +};
> +
> +/*
> + * Channel command bit numbers
> + */
> +enum {
> +CH_CMD_IRQ_COMPLETE = 3,
> +CH_CMD_SEMAPHORE = 6,
> +};
> +
> +/*
> + * nicked from linux

You don't need to say that, it doesn't really add any
information to the reader.

> + * this is the memory representation of a DMA request
> + */
> +struct mxs_dma_ccw {
> +uint32_t next;
> +uint16_t bits;
> +uint16_t xfer_bytes;
> +#define MAX_XFER_BYTES 0xff00

I'd rather have the #defines before the struct than
interleaved, personally.

> +uint32_t bufaddr;
> +#define MXS_PIO_WORDS  16
> +uint32_t pio_words[MXS_PIO_WORDS];
> +}__attribute__((packed));

If you need to use packed then always use the QEMU_PACKED
macro; this ensures the same layout on all host platforms
(Windows behaviour of plain attribute packed is different).


> +
> +/*
> + * Per channel DMA description
> + */
> +typedef struct mxs_dma_channel {
> +QEMUTimer *timer;
> +struct mxs_dma_state *dma;
> +int channel; // channel index
> +hwaddr base; // base of peripheral
> +hwaddr dataoffset; // offset of the true in/out data latch register

No C++-style comments, please.

> +uint32_t r[10];
> +qemu_irq irq;
> +} mxs_dma_channel;
> +
> +
> +typedef struct mxs_dma_state {
> +SysBusDevice busdev;
> +MemoryRegion iomem;
> +const char * name;
> +
> +struct soc_dma_s * dma;
> +uint32_t r[DMA_MAX];
> +
> +hwaddr base; // base of peripheral
> +mxs_dma_channel channel[DMA_MAX_CHANNELS];
> +} mxs_dma_state;
> +
> +static void mxs_dma_ch_update(mxs_dma_channel *s)
> +{
> +struct mxs_dma_ccw req;
> +int i;
> +
> +/* increment the semaphore, if needed */
> +s->r[CH_SEMA] = (((s->r[CH_SEMA] >> 16) & 0xff) +
> +(s->r[CH_SEMA] & 0xff)) << 16;

This is confusing -- what's it actually doing?

> +if (!((s->r[CH_SEMA] >> 16) & 0xff)) {
> +return;
> +}
> +/* read the request from memory */
> +cpu_physical_memory_read(s->r[CH_NEXTCMD], &req, sizeof(req));

This will not work if your host and target have opposite
endianness. If you're going to read the whole struct in
at once like this you need to use the cpu_to_le* functions
to swap all its members to the host endianness.
Similarly anywhere you write a block of memory back.
Alternatively you can use the ld*_le_phys and st*_le_phys
to read and write specifically sized bytes/shorts/words
to little-endian guest order.

> +/* update the latch registers accordingly */
> +s->r[CH_CURCMD] = s->r[CH_NEXTCMD];
> +s->r[CH_NEXTCMD] = req.next;
> +s->r[CH_CMD] = (req.xfer_bytes << 16) | req.bits;
> +s->r[CH_BUFFER_ADDR] = req.bufaddr;
> +
> +/* write PIO registers first, if any */

Re: [Qemu-devel] [PATCH 05/13] mxs/imx23: Add the interrupt collector

2014-01-06 Thread Peter Maydell
On 11 December 2013 13:56, Michel Pollet  wrote:
> Implements the interrupt collector IO block
>
> Signed-off-by: Michel Pollet 
> ---
>  hw/intc/Makefile.objs |   1 +
>  hw/intc/mxs_icoll.c   | 200 
> ++
>  2 files changed, 201 insertions(+)
>  create mode 100644 hw/intc/mxs_icoll.c
>
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 47ac442..e934b3c 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -24,3 +24,4 @@ obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
>  obj-$(CONFIG_SH4) += sh_intc.o
>  obj-$(CONFIG_XICS) += xics.o
>  obj-$(CONFIG_XICS_KVM) += xics_kvm.o
> +obj-$(CONFIG_MXS) += mxs_icoll.o
> diff --git a/hw/intc/mxs_icoll.c b/hw/intc/mxs_icoll.c
> new file mode 100644
> index 000..a1fd7d9
> --- /dev/null
> +++ b/hw/intc/mxs_icoll.c
> @@ -0,0 +1,200 @@
> +/*
> + * mxs_icoll.c
> + *
> + * Copyright: Michel Pollet 
> + *
> + * QEMU Licence
> + */

I'm not going to keep making the same remarks about reset,
licence header, coding style, vmstate, but you can assume
they apply to all these patches.

> +
> +/*
> + * This block implements the interrupt collector of the mxs
> + * Currently no priority is handled, as linux doesn't use them anyway
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm/mxs.h"
> +
> +enum {
> +ICOLL_VECTOR = 0,
> +ICOLL_LEVELACK = 1,
> +ICOLL_CTRL = 2,
> +// 3, reserved?
> +ICOLL_VBASE = 4,
> +ICOLL_STAT = 7,
> +
> +ICOLL_REG_MAX,
> +
> +ICOLL_RAW0 = 0xa,
> +ICOLL_RAW1,
> +ICOLL_RAW2,
> +ICOLL_RAW3,
> +
> +ICOLL_INT0 = 0x12,
> +ICOLL_INT127 = 0x91,
> +};
> +
> +typedef struct mxs_icoll_state {
> +SysBusDevice busdev;
> +MemoryRegion iomem;
> +uint32_t   reg[ICOLL_REG_MAX];
> +
> +uint32_t   raised[4];
> +uint32_t   fiq[4];
> +uint32_t   irq[4];
> +
> +uint8_tr[128];
> +
> +qemu_irq parent_irq;
> +qemu_irq parent_fiq;
> +} mxs_icoll_state;
> +
> +static void mxs_icoll_update(mxs_icoll_state *s)
> +{
> +int fiq = 0, irq = 0;
> +int i;
> +
> +for (i = 0; i < 4; i++) {
> +int id = ffs(s->raised[i]);
> +int vector = (i * 32) + id - 1;
> +if (s->raised[i] & s->fiq[i]) {
> +fiq++;
> +s->reg[ICOLL_STAT] = vector;
> +break;
> +}
> +if (s->raised[i] & s->irq[i]) {
> +irq++;
> +s->reg[ICOLL_STAT] = vector;
> +break;
> +}
> +}
> +qemu_set_irq(s->parent_irq, irq != 0);
> +qemu_set_irq(s->parent_fiq, fiq != 0);
> +}
> +
> +static void mxs_icoll_set_irq(void *opaque, int irq, int level)
> +{
> +mxs_icoll_state *s = (mxs_icoll_state *) opaque;
> +if (level)
> +s->raised[(irq / 32)] |= 1 << (irq % 32);
> +else
> +s->raised[(irq / 32)] &= ~(1 << (irq % 32));

This if needs braces around both arms.

> +mxs_icoll_update(s);
> +}
> +
> +static uint64_t mxs_icoll_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +mxs_icoll_state *s = (mxs_icoll_state *) opaque;
> +
> +switch (offset >> 4) {
> +case 0 ... ICOLL_REG_MAX:
> +return s->reg[offset >> 4];
> +case ICOLL_RAW0 ... ICOLL_RAW3:
> +return s->raised[(offset >> 4) - ICOLL_RAW0];
> +case ICOLL_INT0 ... ICOLL_INT127:
> +return s->r[(offset >> 4) - ICOLL_INT0];
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: bad offset 0x%x\n", __func__, (int) offset);
> +break;
> +}
> +return 0;
> +}
> +
> +static void mxs_icoll_write(
> +void *opaque, hwaddr offset, uint64_t value, unsigned size)
> +{
> +mxs_icoll_state *s = (mxs_icoll_state *) opaque;
> +uint32_t irqval, irqi = 0;
> +uint32_t * dst = NULL;
> +uint32_t oldvalue = 0;
> +
> +switch (offset >> 4) {
> +case 0 ... ICOLL_REG_MAX:
> +dst = s->reg + (offset >> 4);
> +break;
> +case ICOLL_INT0 ... ICOLL_INT127:
> +irqi = (offset >> 4) - ICOLL_INT0;
> +irqval = s->r[irqi];
> +dst = &irqval;
> +break;
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: bad offset 0x%x\n", __func__, (int) offset);
> +break;
> +}
> +if (!dst) {
> +return;
> +}
> +oldvalue = mxs_write(dst, offset, value, size);
> +
> +switch (offset >> 4) {
> +case ICOLL_CTRL:
> +if ((oldvalue ^ s->r[ICOLL_CTRL]) == 0x8000
> +&& !(oldvalue & 0x8000)) {
> +// printf("%s reseting, anding clockgate\n", __func__);
> +s->r[ICOLL_CTRL] |= 0x4000;
> +}

I've seen this bit of magic in several patches now. It could
use explanation and maybe being factored out somehow?

> +break;
> +case ICOLL_LEVELACK:
> +irqi = s->reg[ICOLL_STAT] & 0x7f;

[Qemu-devel] KVM call agenda for 2014-01-07

2014-01-06 Thread Juan Quintela
Hi

First of all, poll told to move the call earlier.

9:00 EST (15:00 CET or 6:00 Pacific)


Please, send any topic that you are interested in covering.

Happy new year

Thanks, Juan.



Re: [Qemu-devel] [Xen-devel] Project idea: make QEMU more flexible

2014-01-06 Thread Anthony Liguori
On Jan 6, 2014 6:55 AM, "Stefano Stabellini" <
stefano.stabell...@eu.citrix.com> wrote:
>
> On Mon, 6 Jan 2014, Anthony Liguori wrote:
> > On Jan 6, 2014 6:23 AM, "Peter Maydell" 
wrote:
> > >
> > > On 6 January 2014 14:17, Stefano Stabellini
> > >  wrote:
> > > > It doesn't do any emulation so it is not specific to any
architecture or
> > > > any cpu.
> > >
> > > You presumably still care about the compiled in values of
> > > TARGET_WORDS_BIGENDIAN, TARGET_LONG_SIZE, and so on...
>
> Actually it only uses XC_PAGE_SIZE and the endianness is the host
> endianness.

If blkif in QEMU is relying on host endianness thats a bug.

>
>
> > Yup.  It's still accel=xen just with no VCPUs.
>
> Are you talking about introducing accel=xen to Wei's target-null?
> I guess that would work OK.

We already have accel=xen.  I'm echoing Peter's suggestion of having the
ability to compile out accel=tcg.

>
> On the other hand if you are thinking of avoiding the introduction of a
> new target-null, how would you make xen_machine_pv.c available to
> multiple architectures?

Why does qdisk need a full machine?

How would you avoid the compilation of all the
> unnecessary emulated devices?

Device config files.

Regards,

Anthony Liguori


Re: [Qemu-devel] [PATCH 03/13] mxs/imx23: Add uart driver

2014-01-06 Thread Peter Maydell
On 11 December 2013 13:56, Michel Pollet  wrote:
> Prototype driver for the mxs/imx23 uart IO block. This has no
> real 'uart' functional code, apart from letting itself be
> initialized by linux without generating a timeout error.
>
> Signed-off-by: Michel Pollet 

Hi; there are some minor code style/formatting errors
with this patch. You can catch these by running the
scripts/checkpatch.pl script on your patches. (It
doesn't catch everything, and sometimes it gets
confused and gives bogus results, but it's a good
sanity check.)

> ---
>  hw/char/Makefile.objs |   1 +
>  hw/char/mxs_uart.c| 146 
> ++
>  2 files changed, 147 insertions(+)
>  create mode 100644 hw/char/mxs_uart.c
>
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index cbd6a00..8ea5670 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -19,6 +19,7 @@ common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
>  common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
>  common-obj-$(CONFIG_GRLIB) += grlib_apbuart.o
>  common-obj-$(CONFIG_IMX) += imx_serial.o
> +common-obj-$(CONFIG_MXS) += mxs_uart.o

This should be a CONFIG_MXS_UART (see remark on earlier patch).

>  common-obj-$(CONFIG_LM32) += lm32_juart.o
>  common-obj-$(CONFIG_LM32) += lm32_uart.o
>  common-obj-$(CONFIG_MILKYMIST) += milkymist-uart.o
> diff --git a/hw/char/mxs_uart.c b/hw/char/mxs_uart.c
> new file mode 100644
> index 000..79b2582
> --- /dev/null
> +++ b/hw/char/mxs_uart.c
> @@ -0,0 +1,146 @@
> +/*
> + * mxs_uart.c
> + *
> + * Copyright: Michel Pollet 
> + *
> + * QEMU Licence

This is too vague. If you mean GPLv2 please say so.

> + */
> +
> +/*
> + * Work in progress ! Right now there's just enough so that linux driver
> + * will instantiate after a probe, there is no functional code.
> + */
> +#include "hw/sysbus.h"
> +#include "hw/arm/mxs.h"
> +
> +#define D(w) w

Please get rid of this. You can use a similar DPRINTF
type macro as other devices do, or no debug tracing at
all, as you wish.

> +
> +enum {
> +UART_CTRL = 0x0,
> +UART_CTRL1 = 0x1,
> +UART_CTRL2 = 0x2,
> +UART_LINECTRL = 0x3,
> +UART_LINECTRL2 = 0x4,
> +UART_INTR = 0x5,
> +UART_APP_DATA = 0x6,
> +UART_APP_STAT = 0x7,
> +UART_APP_DEBUG = 0x8,
> +UART_APP_VERSION = 0x9,
> +UART_APP_AUTOBAUD = 0xa,
> +
> +UART_MAX,
> +};
> +typedef struct mxs_uart_state {
> +SysBusDevice busdev;
> +MemoryRegion iomem;
> +
> +uint32_t r[UART_MAX];
> +
> +struct {
> +uint16_t b[16];
> +int w, r;
> +} fifo[2];
> +qemu_irq irq;
> +CharDriverState *chr;
> +} mxs_uart_state;

Structured type names should be in CamelCase;
see CODING_STYLE.

> +static uint64_t mxs_uart_read(
> +void *opaque, hwaddr offset, unsigned size)
> +{
> +mxs_uart_state *s = (mxs_uart_state *) opaque;
> +uint32_t res = 0;
> +
> +D(printf("%s %04x (%d) = ", __func__, (int)offset, size);)
> +switch (offset >> 4) {
> +case 0 ... UART_MAX:

This indent is wrong, as checkpatch.pl will tell you.

> +res = s->r[offset >> 4];
> +break;
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: bad offset 0x%x\n", __func__, (int) offset);
> +break;
> +}
> +D(printf("%08x\n", res);)
> +
> +return res;
> +}
> +
> +static void mxs_uart_write(void *opaque, hwaddr offset,
> +uint64_t value, unsigned size)
> +{
> +mxs_uart_state *s = (mxs_uart_state *) opaque;
> +uint32_t oldvalue = 0;
> +
> +D(printf("%s %04x %08x(%d)\n", __func__, (int)offset, (int)value, size);)
> +switch (offset >> 4) {
> +case 0 ... UART_MAX:
> +mxs_write(&s->r[offset >> 4], offset, value, size);
> +break;
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> +"%s: bad offset 0x%x\n", __func__, (int) offset);
> +break;
> +}
> +switch (offset >> 4) {
> +case UART_CTRL:
> +if ((oldvalue ^ s->r[UART_CTRL]) == 0x8000
> +&& !(oldvalue & 0x8000)) {
> +printf("%s reseting, anding clockgate\n", __func__);

Stray debug printout.

> +s->r[UART_CTRL] |= 0x4000;
> +}
> +break;
> +}
> +}
> +
> +static void mxs_uart_set_irq(void *opaque, int irq, int level)
> +{
> +//mxs_uart_state *s = (mxs_uart_state *)opaque;

Don't leave commented out code in your patches, please.

> +printf("%s %3d = %d\n", __func__, irq, level);
> +}
> +
> +static const MemoryRegionOps mxs_uart_ops = {
> +.read = mxs_uart_read,
> +.write = mxs_uart_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +
> +static int mxs_uart_init(SysBusDevice *dev)
> +{
> +mxs_uart_state *s = OBJECT_CHECK(mxs_uart_state, dev, "mxs_uart");
> +DeviceState *qdev = DEVICE(dev);
> +
> +qdev_init_gpio_in(qdev, mxs_uart_set_irq, 32 * 3);

Why h

Re: [Qemu-devel] [RFC PATCH v4 0/8] Support arm-gic-kvm save/restore

2014-01-06 Thread Christoffer Dall
On Mon, Jan 06, 2014 at 11:09:28AM +, Peter Maydell wrote:
> On 21 December 2013 06:09, Christoffer Dall  
> wrote:
> > Implement support to save/restore the ARM KVM VGIC state from the
> > kernel.  The basic appraoch is to transfer state from the in-kernel VGIC
> > to the emulated arm-gic state representation and let the standard QEMU
> > vmstate save/restore handle saving the arm-gic state.  Restore works by
> > reversing the process.
> >
> > The first patches adds missing features and fixes issues with the
> > arm-gic implementation in qemu in preparation for the actual
> > save/restore logic.
> 
> I still need to review this series, but I'm going to take the
> first two patches into target-arm.next now since they're already
> reviewed and they're straightforward cleanup.
> 
ok, no worries about the review.  The kernel series went into Marcelo's
queue over the holiday, so I expect it to appear in kvm.next any time
soon.

-Christoffer



Re: [Qemu-devel] [PATCH resend] linux-user: Support the accept4 socketcall

2014-01-06 Thread André Hentschel
> This looks ok, except that scripts/checkpatch.pl says:
> 
> WARNING: braces {} are necessary for all arms of this statement
> #36: FILE: linux-user/syscall.c:2254:
> +if (get_user_ual(sockfd, vptr)
> [...]
> 
> total: 0 errors, 1 warnings, 30 lines checked
> 
> Fix that and I'll be happy to slap a "reviewed-by" sticker on it. Be sure
> to CC me on the fixed version of the patch.
> 
> 
> Cheers,
> Erik

This warning seems wrong:
 - the if statement has no braces and only one arm
 - the if statement looks like the others around it, i just try to keep the 
same style


Am 06.01.2014 11:21, schrieb Laurent Vivier:
> 
>> Le 6 janvier 2014 à 10:14, Peter Maydell  a écrit :
>>
>>
>> On 6 January 2014 08:45, Laurent Vivier  wrote:
>> >
>> >> Le 6 janvier 2014 à 02:57, André Hentschel  a écrit :
>> >> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> >> index cf08db5..b36f99c 100644
>> >> --- a/linux-user/syscall_defs.h
>> >> +++ b/linux-user/syscall_defs.h
>> >> @@ -27,6 +27,9 @@
>> >> #define SOCKOP_getsockopt 15
>> >> #define SOCKOP_sendmsg 16
>> >> #define SOCKOP_recvmsg 17
>> >> +#define SOCKOP_accept4 18
>> >> +#define SOCKOP_recvmmsg 19
>> >> +#define SOCKOP_sendmmsg 20
>> >
>> > Don't add these both defines here as they are not used in this patch.
>>
>> It doesn't seem that unreasonable to add them. We add things
>> to the main syscall number #define list even if we aren't
>> actually implementing them, for example.
> 
> IMHO, you should not : if you implement these syscalls and then revert this 
> patch (because it is broken, for instance), you will break the build. The 
> defines must come with the implementation.

good point for removing them and add them separatly.




  1   2   3   >