[Qemu-devel] [PULL for 2.9 48/49] qapi: Make pylint a bit happier

2017-03-16 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Message-Id: <1489582656-31133-47-git-send-email-arm...@redhat.com>
---
 scripts/qapi-commands.py | 6 +++---
 scripts/qapi-visit.py| 1 -
 scripts/qapi.py  | 8 
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0c05449..1943de4 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -13,7 +13,6 @@
 # See the COPYING file in the top-level directory.
 
 from qapi import *
-import re
 
 
 def gen_command_decl(name, arg_type, boxed, ret_type):
@@ -84,7 +83,8 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, 
QObject **ret_out,
 
 
 def gen_marshal_proto(name):
-return 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % 
c_name(name)
+return ('void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)'
+% c_name(name))
 
 
 def gen_marshal_decl(name):
@@ -198,7 +198,7 @@ def gen_register_command(name, success_response):
 options = 'QCO_NO_SUCCESS_RESP'
 
 ret = mcgen('''
-qmp_register_command(cmds, "%(name)s", 
+qmp_register_command(cmds, "%(name)s",
  qmp_marshal_%(c_name)s, %(opts)s);
 ''',
 name=name, c_name=c_name(name),
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 3d3936e..5737aef 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -13,7 +13,6 @@
 # See the COPYING file in the top-level directory.
 
 from qapi import *
-import re
 
 
 def gen_visit_decl(name, scalar=False):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b798ae4..d19300d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,13 +11,13 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-import re
-from ordereddict import OrderedDict
 import errno
 import getopt
 import os
-import sys
+import re
 import string
+import sys
+from ordereddict import OrderedDict
 
 builtin_types = {
 'str':  'QTYPE_QSTRING',
@@ -1587,7 +1587,7 @@ class QAPISchema(object):
 tag_member = None
 if isinstance(base, dict):
 base = (self._make_implicit_object_type(
-name, info, doc, 'base', self._make_members(base, info)))
+name, info, doc, 'base', self._make_members(base, info)))
 if tag_name:
 variants = [self._make_variant(key, value)
 for (key, value) in data.iteritems()]
-- 
2.7.4




[Qemu-devel] [PULL for 2.9 44/49] qapi: enum_types is a list used like a dict, make it one

2017-03-16 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Message-Id: <1489582656-31133-43-git-send-email-arm...@redhat.com>
---
 scripts/qapi.py | 29 ++---
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1f79eb4..735ddaa 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -46,7 +46,7 @@ returns_whitelist = []
 # Whitelist of entities allowed to violate case conventions
 name_case_whitelist = []
 
-enum_types = []
+enum_types = {}
 struct_types = []
 union_types = []
 all_names = {}
@@ -567,7 +567,7 @@ def find_alternate_member_qtype(qapi_type):
 return builtin_types[qapi_type]
 elif find_struct(qapi_type):
 return 'QTYPE_QDICT'
-elif find_enum(qapi_type):
+elif qapi_type in enum_types:
 return 'QTYPE_QSTRING'
 elif find_union(qapi_type):
 return 'QTYPE_QDICT'
@@ -591,7 +591,7 @@ def discriminator_find_enum_define(expr):
 if not discriminator_type:
 return None
 
-return find_enum(discriminator_type)
+return enum_types.get(discriminator_type)
 
 
 # Names must be letters, numbers, -, and _.  They must start with letter,
@@ -664,23 +664,6 @@ def find_union(name):
 return None
 
 
-def add_enum(definition, info):
-global enum_types
-enum_types.append(definition)
-
-
-def find_enum(name):
-global enum_types
-for enum in enum_types:
-if enum['enum'] == name:
-return enum
-return None
-
-
-def is_enum(name):
-return find_enum(name) is not None
-
-
 def check_type(info, source, value, allow_array=False,
allow_dict=False, allow_optional=False,
allow_metas=[]):
@@ -799,7 +782,7 @@ def check_union(expr, info):
"Discriminator '%s' is not a member of base "
"struct '%s'"
% (discriminator, base))
-enum_define = find_enum(discriminator_type)
+enum_define = enum_types.get(discriminator_type)
 allow_metas = ['struct']
 # Do not allow string discriminator
 if not enum_define:
@@ -933,7 +916,7 @@ def check_exprs(exprs):
 if 'enum' in expr:
 meta = 'enum'
 check_keys(expr_elem, 'enum', ['data'], ['prefix'])
-add_enum(expr, info)
+enum_types[expr[meta]] = expr
 elif 'union' in expr:
 meta = 'union'
 check_keys(expr_elem, 'union', ['data'],
@@ -971,7 +954,7 @@ def check_exprs(exprs):
 name = '%sKind' % expr['alternate']
 else:
 continue
-add_enum({ 'enum': name }, expr_elem['info'])
+enum_types[name] = {'enum': name}
 add_name(name, info, 'enum', implicit=True)
 
 # Validate that exprs make sense
-- 
2.7.4




[Qemu-devel] [PULL for 2.9 38/49] tests/qapi-schema: Improve coverage of bogus member docs

2017-03-16 Thread Markus Armbruster
New test doc-bad-union-member.json shows we can fail to reject
documentation for nonexistent members.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Message-Id: <1489582656-31133-37-git-send-email-arm...@redhat.com>
---
 tests/Makefile.include  |  2 ++
 tests/qapi-schema/doc-bad-alternate-member.err  |  1 +
 tests/qapi-schema/doc-bad-alternate-member.exit |  1 +
 tests/qapi-schema/doc-bad-alternate-member.json |  9 +
 tests/qapi-schema/doc-bad-alternate-member.out  |  0
 tests/qapi-schema/doc-bad-union-member.err  |  0
 tests/qapi-schema/doc-bad-union-member.exit |  1 +
 tests/qapi-schema/doc-bad-union-member.json | 19 +++
 tests/qapi-schema/doc-bad-union-member.out  | 11 +++
 9 files changed, 44 insertions(+)
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.err
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.exit
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.json
 create mode 100644 tests/qapi-schema/doc-bad-alternate-member.out
 create mode 100644 tests/qapi-schema/doc-bad-union-member.err
 create mode 100644 tests/qapi-schema/doc-bad-union-member.exit
 create mode 100644 tests/qapi-schema/doc-bad-union-member.json
 create mode 100644 tests/qapi-schema/doc-bad-union-member.out

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 734c7ff..e6e00b4 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -367,8 +367,10 @@ qapi-schema += base-cycle-direct.json
 qapi-schema += base-cycle-indirect.json
 qapi-schema += command-int.json
 qapi-schema += comments.json
+qapi-schema += doc-bad-alternate-member.json
 qapi-schema += doc-bad-command-arg.json
 qapi-schema += doc-bad-symbol.json
+qapi-schema += doc-bad-union-member.json
 qapi-schema += doc-before-include.json
 qapi-schema += doc-before-pragma.json
 qapi-schema += doc-duplicated-arg.json
diff --git a/tests/qapi-schema/doc-bad-alternate-member.err 
b/tests/qapi-schema/doc-bad-alternate-member.err
new file mode 100644
index 000..387f782
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-alternate-member.err
@@ -0,0 +1 @@
+tests/qapi-schema/doc-bad-alternate-member.json:3: The following documented 
members are not in the declaration: aa, bb
diff --git a/tests/qapi-schema/doc-bad-alternate-member.exit 
b/tests/qapi-schema/doc-bad-alternate-member.exit
new file mode 100644
index 000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-alternate-member.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/doc-bad-alternate-member.json 
b/tests/qapi-schema/doc-bad-alternate-member.json
new file mode 100644
index 000..738635c
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-alternate-member.json
@@ -0,0 +1,9 @@
+# Arguments listed in the doc comment must exist in the actual schema
+
+##
+# @AorB:
+# @aa: a
+# @bb: b
+##
+{ 'alternate': 'AorB',
+  'data': { 'a': 'str', 'b': 'int' } }
diff --git a/tests/qapi-schema/doc-bad-alternate-member.out 
b/tests/qapi-schema/doc-bad-alternate-member.out
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/doc-bad-union-member.err 
b/tests/qapi-schema/doc-bad-union-member.err
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/doc-bad-union-member.exit 
b/tests/qapi-schema/doc-bad-union-member.exit
new file mode 100644
index 000..573541a
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-union-member.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/doc-bad-union-member.json 
b/tests/qapi-schema/doc-bad-union-member.json
new file mode 100644
index 000..d611435
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-union-member.json
@@ -0,0 +1,19 @@
+# Arguments listed in the doc comment must exist in the actual schema
+
+##
+# @Frob:
+# @a: a
+# @b: b
+##
+{ 'union': 'Frob',
+  'base': 'Base',
+  'discriminator': 'type',
+  'data': { 'nothing': 'Empty' } }
+
+{ 'struct': 'Base',
+  'data': { 'type': 'T' } }
+
+{ 'struct': 'Empty',
+  'data': { } }
+
+{ 'enum': 'T', 'data': ['nothing'] }
diff --git a/tests/qapi-schema/doc-bad-union-member.out 
b/tests/qapi-schema/doc-bad-union-member.out
new file mode 100644
index 000..2576ecd
--- /dev/null
+++ b/tests/qapi-schema/doc-bad-union-member.out
@@ -0,0 +1,11 @@
+object Base
+member type: T optional=False
+object Empty
+object Frob
+base Base
+tag type
+case nothing: Empty
+enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 
'qbool']
+prefix QTYPE
+enum T ['nothing']
+object q_empty
-- 
2.7.4




[Qemu-devel] [PULL for 2.9 05/49] qapi: Back out doc comments added just to please qapi.py

2017-03-16 Thread Markus Armbruster
This reverts commit 3313b61's changes to tests/qapi-schema/, except
for tests/qapi-schema/doc-*.

We could keep some of these doc comments to serve as positive test
cases.  However, they don't actually add to what we get from doc
comment use in actual schemas, as we we don't test output matches
expectations, and don't systematically cover doc comment features.
Proper positive test coverage would be nice.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Message-Id: <1489582656-31133-4-git-send-email-arm...@redhat.com>
---
 tests/qapi-schema/alternate-any.err|   2 +-
 tests/qapi-schema/alternate-any.json   |   4 -
 tests/qapi-schema/alternate-array.err  |   2 +-
 tests/qapi-schema/alternate-array.json |   7 -
 tests/qapi-schema/alternate-base.err   |   2 +-
 tests/qapi-schema/alternate-base.json  |   7 -
 tests/qapi-schema/alternate-clash.err  |   2 +-
 tests/qapi-schema/alternate-clash.json |   4 -
 tests/qapi-schema/alternate-conflict-dict.err  |   2 +-
 tests/qapi-schema/alternate-conflict-dict.json |  10 -
 tests/qapi-schema/alternate-conflict-string.err|   2 +-
 tests/qapi-schema/alternate-conflict-string.json   |   7 -
 tests/qapi-schema/alternate-empty.err  |   2 +-
 tests/qapi-schema/alternate-empty.json |   4 -
 tests/qapi-schema/alternate-nested.err |   2 +-
 tests/qapi-schema/alternate-nested.json|   7 -
 tests/qapi-schema/alternate-unknown.err|   2 +-
 tests/qapi-schema/alternate-unknown.json   |   4 -
 tests/qapi-schema/args-alternate.err   |   2 +-
 tests/qapi-schema/args-alternate.json  |   8 -
 tests/qapi-schema/args-any.err |   2 +-
 tests/qapi-schema/args-any.json|   4 -
 tests/qapi-schema/args-array-empty.err |   2 +-
 tests/qapi-schema/args-array-empty.json|   4 -
 tests/qapi-schema/args-array-unknown.err   |   2 +-
 tests/qapi-schema/args-array-unknown.json  |   4 -
 tests/qapi-schema/args-bad-boxed.err   |   2 +-
 tests/qapi-schema/args-bad-boxed.json  |   4 -
 tests/qapi-schema/args-boxed-anon.err  |   2 +-
 tests/qapi-schema/args-boxed-anon.json |   4 -
 tests/qapi-schema/args-boxed-empty.err |   2 +-
 tests/qapi-schema/args-boxed-empty.json|   8 -
 tests/qapi-schema/args-boxed-string.err|   2 +-
 tests/qapi-schema/args-boxed-string.json   |   4 -
 tests/qapi-schema/args-int.err |   2 +-
 tests/qapi-schema/args-int.json|   4 -
 tests/qapi-schema/args-invalid.err |   2 +-
 tests/qapi-schema/args-invalid.json|   3 -
 tests/qapi-schema/args-member-array-bad.err|   2 +-
 tests/qapi-schema/args-member-array-bad.json   |   4 -
 tests/qapi-schema/args-member-case.err |   2 +-
 tests/qapi-schema/args-member-case.json|   4 -
 tests/qapi-schema/args-member-unknown.err  |   2 +-
 tests/qapi-schema/args-member-unknown.json |   4 -
 tests/qapi-schema/args-name-clash.err  |   2 +-
 tests/qapi-schema/args-name-clash.json |   4 -
 tests/qapi-schema/args-union.err   |   2 +-
 tests/qapi-schema/args-union.json  |   7 -
 tests/qapi-schema/args-unknown.err |   2 +-
 tests/qapi-schema/args-unknown.json|   4 -
 tests/qapi-schema/bad-base.err |   2 +-
 tests/qapi-schema/bad-base.json|   7 -
 tests/qapi-schema/bad-data.err |   2 +-
 tests/qapi-schema/bad-data.json|   4 -
 tests/qapi-schema/bad-ident.err|   2 +-
 tests/qapi-schema/bad-ident.json   |   4 -
 tests/qapi-schema/bad-type-bool.err|   2 +-
 tests/qapi-schema/bad-type-bool.json   |   4 -
 tests/qapi-schema/bad-type-dict.err|   2 +-
 tests/qapi-schema/bad-type-dict.json   |   4 -
 tests/qapi-schema/base-cycle-direct.err|   2 +-
 tests/qapi-schema/base-cycle-direct.json   |   4 -
 tests/qapi-schema/base-cycle-indirect.err  |   2 +-
 tests/qapi-schema/base-cycle-indirect.json |   7 -
 tests/qapi-schema/command-int.err  |   2 +-
 tests/qapi-schema/command-int.json |   4 -
 tests/qapi-schema/comments.json|   4 -
 tests/qapi-schema/comments.out |   1 -
 tests/qapi-schema/double-type.err  |   2 +-
 tests/qapi-schema/double-type.json |   4 -
 tests/qapi-schema/enum-bad-name.err|   2 +-
 tests/qapi-schema/enum-bad-name.json   |   4 -
 tests/qapi-schema/enum-bad-prefix.err  |   2 +-
 tests/qapi-schema/enum-bad-prefix.json  

[Qemu-devel] [PULL for 2.9 43/49] qapi: Factor add_name() calls out of the meta conditional

2017-03-16 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Message-Id: <1489582656-31133-42-git-send-email-arm...@redhat.com>
---
 scripts/qapi.py | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3c6e137..1f79eb4 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -640,8 +640,6 @@ def add_name(name, info, meta, implicit=False):
 
 def add_struct(definition, info):
 global struct_types
-name = definition['struct']
-add_name(name, info, 'struct')
 struct_types.append(definition)
 
 
@@ -655,8 +653,6 @@ def find_struct(name):
 
 def add_union(definition, info):
 global union_types
-name = definition['union']
-add_name(name, info, 'union')
 union_types.append(definition)
 
 
@@ -670,8 +666,6 @@ def find_union(name):
 
 def add_enum(definition, info):
 global enum_types
-name = definition['enum']
-add_name(name, info, 'enum', 'data' not in definition)
 enum_types.append(definition)
 
 
@@ -937,34 +931,33 @@ def check_exprs(exprs):
"Expression missing documentation comment")
 
 if 'enum' in expr:
-name = expr['enum']
+meta = 'enum'
 check_keys(expr_elem, 'enum', ['data'], ['prefix'])
 add_enum(expr, info)
 elif 'union' in expr:
-name = expr['union']
+meta = 'union'
 check_keys(expr_elem, 'union', ['data'],
['base', 'discriminator'])
 add_union(expr, info)
 elif 'alternate' in expr:
-name = expr['alternate']
+meta = 'alternate'
 check_keys(expr_elem, 'alternate', ['data'])
-add_name(name, info, 'alternate')
 elif 'struct' in expr:
-name = expr['struct']
+meta = 'struct'
 check_keys(expr_elem, 'struct', ['data'], ['base'])
 add_struct(expr, info)
 elif 'command' in expr:
-name = expr['command']
+meta = 'command'
 check_keys(expr_elem, 'command', [],
['data', 'returns', 'gen', 'success-response', 'boxed'])
-add_name(name, info, 'command')
 elif 'event' in expr:
-name = expr['event']
+meta = 'event'
 check_keys(expr_elem, 'event', [], ['data', 'boxed'])
-add_name(name, info, 'event')
 else:
 raise QAPISemError(expr_elem['info'],
"Expression is missing metatype")
+name = expr[meta]
+add_name(name, info, meta)
 if doc and doc.symbol != name:
 raise QAPISemError(info, "Definition of '%s' follows documentation"
" for '%s'" % (name, doc.symbol))
@@ -979,6 +972,7 @@ def check_exprs(exprs):
 else:
 continue
 add_enum({ 'enum': name }, expr_elem['info'])
+add_name(name, info, 'enum', implicit=True)
 
 # Validate that exprs make sense
 for expr_elem in exprs:
-- 
2.7.4




[Qemu-devel] [PULL 2/3] makefile: generate trace-events-all upfront

2017-03-16 Thread Stefan Hajnoczi
From: "Daniel P. Berrange" 

Files should not be created in the build dir during the
'make install' phase. List 'trace-events-all' as a
generated file so that it gets created upfront during
build.

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Fam Zheng 
Message-id: 20170228122901.24520-3-berra...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index a8024c0..6291764 100644
--- a/Makefile
+++ b/Makefile
@@ -82,6 +82,7 @@ endif
 
 GENERATED_FILES += $(TRACE_HEADERS)
 GENERATED_FILES += $(TRACE_SOURCES)
+GENERATED_FILES += $(BUILD_DIR)/trace-events-all
 
 trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
 
@@ -592,8 +593,7 @@ endif
 endif
 
 
-install: all $(if $(BUILD_DOCS),install-doc) $(BUILD_DIR)/trace-events-all \
-install-datadir install-localstatedir
+install: all $(if $(BUILD_DOCS),install-doc) install-datadir 
install-localstatedir
 ifneq ($(TOOLS),)
$(call install-prog,$(subst 
qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
 endif
-- 
2.9.3




[Qemu-devel] [PULL 0/3] Tracing patches

2017-03-16 Thread Stefan Hajnoczi
The following changes since commit 1883ff34b540daacae948f493b0ba525edf5f642:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2017-03-15 18:44:05 +)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/tracing-pull-request

for you to fetch changes up to 8755b4afbdcf1c274cab7545a9f76d3d6c7f5c29:

  trace: ensure $(tracetool-y) is defined in top level makefile (2017-03-16 
11:51:26 +0800)


Pull request

Tracing makefile fixes for QEMU 2.9.



Daniel P. Berrange (3):
  makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables
  makefile: generate trace-events-all upfront
  trace: ensure $(tracetool-y) is defined in top level makefile

 Makefile   | 42 ++
 Makefile.target|  6 +++---
 target/s390x/Makefile.objs |  2 +-
 tests/Makefile.include |  2 +-
 trace/Makefile.objs|  8 
 5 files changed, 27 insertions(+), 33 deletions(-)

-- 
2.9.3




[Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables

2017-03-16 Thread Stefan Hajnoczi
From: "Daniel P. Berrange" 

The only functional difference between the GENERATED_HEADERS
and GENERATED_SOURCES variables is that 'Makefile' has a
dependancy on GENERATED_HEADERS, causing generated header files
to be created immediatey at the start of the build process.
There is no reason why this early creation should be restricted
to the .h files, and not include .c files too. Merge both of
the variables into a single GENERATED_FILES variable to make
it clear it is for any type of generated file.

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Fam Zheng 
Message-id: 20170228122901.24520-2-berra...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 Makefile   | 35 +--
 Makefile.target|  6 +++---
 target/s390x/Makefile.objs |  2 +-
 tests/Makefile.include |  2 +-
 4 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/Makefile b/Makefile
index 1c4c04f..a8024c0 100644
--- a/Makefile
+++ b/Makefile
@@ -50,24 +50,24 @@ endif
 
 include $(SRC_PATH)/rules.mak
 
-GENERATED_HEADERS = qemu-version.h config-host.h qemu-options.def
-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 += qmp-introspect.h
-GENERATED_SOURCES += qmp-introspect.c
+GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
+GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
+GENERATED_FILES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
+GENERATED_FILES += qmp-introspect.h
+GENERATED_FILES += qmp-introspect.c
 
-GENERATED_HEADERS += trace/generated-tcg-tracers.h
+GENERATED_FILES += trace/generated-tcg-tracers.h
 
-GENERATED_HEADERS += trace/generated-helpers-wrappers.h
-GENERATED_HEADERS += trace/generated-helpers.h
-GENERATED_SOURCES += trace/generated-helpers.c
+GENERATED_FILES += trace/generated-helpers-wrappers.h
+GENERATED_FILES += trace/generated-helpers.h
+GENERATED_FILES += trace/generated-helpers.c
 
 ifdef CONFIG_TRACE_UST
-GENERATED_HEADERS += trace-ust-all.h
-GENERATED_SOURCES += trace-ust-all.c
+GENERATED_FILES += trace-ust-all.h
+GENERATED_FILES += trace-ust-all.c
 endif
 
-GENERATED_HEADERS += module_block.h
+GENERATED_FILES += module_block.h
 
 TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
 TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
@@ -80,8 +80,8 @@ ifdef CONFIG_TRACE_UST
 TRACE_HEADERS += trace-ust-root.h $(trace-events-subdirs:%=%/trace-ust.h)
 endif
 
-GENERATED_HEADERS += $(TRACE_HEADERS)
-GENERATED_SOURCES += $(TRACE_SOURCES)
+GENERATED_FILES += $(TRACE_HEADERS)
+GENERATED_FILES += $(TRACE_SOURCES)
 
 trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
 
@@ -485,11 +485,10 @@ clean:
rm -f fsdev/*.pod
rm -f qemu-img-cmds.h
rm -f ui/shader/*-vert.h ui/shader/*-frag.h
-   @# May not be present in GENERATED_HEADERS
+   @# May not be present in GENERATED_FILES
rm -f trace/generated-tracers-dtrace.dtrace*
rm -f trace/generated-tracers-dtrace.h*
-   rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp)
-   rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp)
+   rm -f $(foreach f,$(GENERATED_FILES),$(f) $(f)-timestamp)
rm -rf qapi-generated
rm -rf qga/qapi-generated
for d in $(ALL_SUBDIRS); do \
@@ -784,7 +783,7 @@ endif # CONFIG_WIN
 # Add a dependency on the generated files, so that they are always
 # rebuilt before other object files
 ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if 
$(MAKECMDGOALS),,fail))
-Makefile: $(GENERATED_HEADERS)
+Makefile: $(GENERATED_FILES)
 endif
 
 .SECONDARY: $(TRACE_HEADERS) $(TRACE_HEADERS:%=%-timestamp) \
diff --git a/Makefile.target b/Makefile.target
index 924304c..7df2b8c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -162,7 +162,7 @@ else
 obj-y += hw/$(TARGET_BASE_ARCH)/
 endif
 
-GENERATED_HEADERS += hmp-commands.h hmp-commands-info.h
+GENERATED_FILES += hmp-commands.h hmp-commands-info.h
 
 endif # CONFIG_SOFTMMU
 
@@ -238,5 +238,5 @@ ifdef CONFIG_TRACE_SYSTEMTAP
$(INSTALL_DATA) $(QEMU_PROG)-simpletrace.stp 
"$(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG)-simpletrace.stp"
 endif
 
-GENERATED_HEADERS += config-target.h
-Makefile: $(GENERATED_HEADERS)
+GENERATED_FILES += config-target.h
+Makefile: $(GENERATED_FILES)
diff --git a/target/s390x/Makefile.objs b/target/s390x/Makefile.objs
index c573633..46f6a8c 100644
--- a/target/s390x/Makefile.objs
+++ b/target/s390x/Makefile.objs
@@ -8,7 +8,7 @@ obj-$(CONFIG_KVM) += kvm.o
 feat-src = $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/
 feat-dst = $(BUILD_DIR)/$(TARGET_DIR)
 ifneq ($(MAKECMDGOALS),clean)
-GENERATED_HEADERS += $(feat-dst)gen-features.h
+GENERATED_FILES += $(feat-dst)gen-features.h
 endif
 
 $(feat-dst)gen-features.h: $(feat-dst)gen-features.h-timestamp
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 346

[Qemu-devel] [PULL 3/3] trace: ensure $(tracetool-y) is defined in top level makefile

2017-03-16 Thread Stefan Hajnoczi
From: "Daniel P. Berrange" 

The build rules for trace files have a dependancy on $(tracetool-y).
This variable populated in the trace/Makefile.objs file and thus its
definition gets pulled into the top level makefile. This happens too
late in the process though, so by the time $(tracetool-y) is defined,
make has already evaluated $(tracetool-y) in the dependancies and
found it to be empty. The result is that when the tracetool source
is changed, the generated files are not rebuilt. The solution is to
define the variable in the top level makefile too

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Eric Blake 
Tested-by: Eric Blake 
Message-id: 20170315123421.28815-1-berra...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 Makefile| 3 +++
 trace/Makefile.objs | 8 
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 6291764..108b984 100644
--- a/Makefile
+++ b/Makefile
@@ -86,6 +86,9 @@ GENERATED_FILES += $(BUILD_DIR)/trace-events-all
 
 trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
 
+tracetool-y = $(SRC_PATH)/scripts/tracetool.py
+tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
+
 %/trace.h: %/trace.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 %/trace.h-timestamp: $(SRC_PATH)/%/trace-events $(tracetool-y)
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 7de840a..1b8eb4a 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -1,13 +1,5 @@
 # -*- mode: makefile -*-
 
-##
-# tracetool source files
-# Every rule that invokes tracetool must depend on this so code is regenerated
-# if tracetool itself changes.
-
-tracetool-y = $(SRC_PATH)/scripts/tracetool.py
-tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
-
 $(BUILD_DIR)/trace-events-all: $(trace-events-files)
$(call quiet-command,cat $^ > $@)
 
-- 
2.9.3




[Qemu-devel] [PATCH kernel v8 0/4] Extend virtio-balloon for fast (de)inflating & fast live migration

2017-03-16 Thread Wei Wang
This patch series implements two optimizations:
1) transfer pages in chuncks between the guest and host;
2) transfer the guest unused pages to the host so that they
can be skipped to migrate in live migration.

Please read each patch commit log for details.

Changes:
v7->v8:
1) Use only one chunk format, instead of two.
2) re-write the virtio-balloon implementation patch.
3) commit changes
4) patch re-org

Liang Li (4):
  virtio-balloon: deflate via a page list
  virtio-balloon: VIRTIO_BALLOON_F_CHUNK_TRANSFER
  mm: add inerface to offer info about unused pages
  virtio-balloon: VIRTIO_BALLOON_F_HOST_REQ_VQ

 drivers/virtio/virtio_balloon.c | 533 
 include/linux/mm.h  |   3 +
 include/uapi/linux/virtio_balloon.h |  31 +++
 mm/page_alloc.c | 114 
 4 files changed, 635 insertions(+), 46 deletions(-)

-- 
2.7.4




[Qemu-devel] [PATCH kernel v8 3/4] mm: add inerface to offer info about unused pages

2017-03-16 Thread Wei Wang
From: Liang Li 

This patch adds a function to provides a snapshot of the present system
unused pages. An important usage of this function is to provide the
unsused pages to the Live migration thread, which skips the transfer of
thoses unused pages. Newly used pages can be re-tracked by the dirty
page logging mechanisms.

Signed-off-by: Liang Li 
Signed-off-by: Wei Wang 
---
 include/linux/mm.h |   3 ++
 mm/page_alloc.c| 114 +
 2 files changed, 117 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index b84615b..869749d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1764,6 +1764,9 @@ extern void free_area_init(unsigned long * zones_size);
 extern void free_area_init_node(int nid, unsigned long * zones_size,
unsigned long zone_start_pfn, unsigned long *zholes_size);
 extern void free_initmem(void);
+extern int record_unused_pages(struct zone **start_zone, int order,
+  __le64 *pages, unsigned int size,
+  unsigned int *pos, bool part_fill);
 
 /*
  * Free reserved pages within range [PAGE_ALIGN(start), end & PAGE_MASK)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f3e0c69..b72a7ac 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4498,6 +4498,120 @@ void show_free_areas(unsigned int filter)
show_swap_cache_info();
 }
 
+static int __record_unused_pages(struct zone *zone, int order,
+__le64 *buf, unsigned int size,
+unsigned int *offset, bool part_fill)
+{
+   unsigned long pfn, flags;
+   int t, ret = 0;
+   struct list_head *curr;
+   __le64 *chunk;
+
+   if (zone_is_empty(zone))
+   return 0;
+
+   spin_lock_irqsave(&zone->lock, flags);
+
+   if (*offset + zone->free_area[order].nr_free > size && !part_fill) {
+   ret = -ENOSPC;
+   goto out;
+   }
+   for (t = 0; t < MIGRATE_TYPES; t++) {
+   list_for_each(curr, &zone->free_area[order].free_list[t]) {
+   pfn = page_to_pfn(list_entry(curr, struct page, lru));
+   chunk = buf + *offset;
+   if (*offset + 2 > size) {
+   ret = -ENOSPC;
+   goto out;
+   }
+   /* Align to the chunk format used in virtio-balloon */
+   *chunk = cpu_to_le64(pfn << 12);
+   *(chunk + 1) = cpu_to_le64((1 << order) << 12);
+   *offset += 2;
+   }
+   }
+
+out:
+   spin_unlock_irqrestore(&zone->lock, flags);
+
+   return ret;
+}
+
+/*
+ * The record_unused_pages() function is used to record the system unused
+ * pages. The unused pages can be skipped to transfer during live migration.
+ * Though the unused pages are dynamically changing, dirty page logging
+ * mechanisms are able to capture the newly used pages though they were
+ * recorded as unused pages via this function.
+ *
+ * This function scans the free page list of the specified order to record
+ * the unused pages, and chunks those continuous pages following the chunk
+ * format below:
+ * --
+ * |   Base (52-bit)   | Rsvd (12-bit) |
+ * --
+ * --
+ * |   Size (52-bit)   | Rsvd (12-bit) |
+ * --
+ *
+ * @start_zone: zone to start the record operation.
+ * @order: order of the free page list to record.
+ * @buf: buffer to record the unused page info in chunks.
+ * @size: size of the buffer in __le64 to record
+ * @offset: offset in the buffer to record.
+ * @part_fill: indicate if partial fill is used.
+ *
+ * return -EINVAL if parameter is invalid
+ * return -ENOSPC when the buffer is too small to record all the unsed pages
+ * return 0 when sccess
+ */
+int record_unused_pages(struct zone **start_zone, int order,
+   __le64 *buf, unsigned int size,
+   unsigned int *offset, bool part_fill)
+{
+   struct zone *zone;
+   int ret = 0;
+   bool skip_check = false;
+
+   /* Make sure all the parameters are valid */
+   if (buf == NULL || offset == NULL || order >= MAX_ORDER)
+   return -EINVAL;
+
+   if (*start_zone != NULL) {
+   bool found = false;
+
+   for_each_populated_zone(zone) {
+   if (zone != *start_zone)
+   continue;
+   found = true;
+   break;
+   }
+   if (!found)
+   return -EINVAL;
+   } else
+   skip_check = true;
+
+   for_each_populated_zone(zone) {
+   /* Start from *start_zone if it's not NULL */
+   if (!skip_check) {
+

[Qemu-devel] [PATCH kernel v8 2/4] virtio-balloon: VIRTIO_BALLOON_F_CHUNK_TRANSFER

2017-03-16 Thread Wei Wang
From: Liang Li 

The implementation of the current virtio-balloon is not very
efficient, because the ballooned pages are transferred to the
host one by one. Here is the breakdown of the time in percentage
spent on each step of the balloon inflating process (inflating
7GB of an 8GB idle guest).

1) allocating pages (6.5%)
2) sending PFNs to host (68.3%)
3) address translation (6.1%)
4) madvise (19%)

It takes about 4126ms for the inflating process to complete.
The above profiling shows that the bottlenecks are stage 2)
and stage 4).

This patch optimizes step 2) by transferring pages to the host in
chunks. A chunk consists of guest physically continuous pages, and
it is offered to the host via a base PFN (i.e. the start PFN of
those physically continuous pages) and the size (i.e. the total
number of the pages). A chunk is formated as below:


| Base (52 bit)| Rsvd (12 bit) |


| Size (52 bit)| Rsvd (12 bit) |


By doing so, step 4) can also be optimized by doing address
translation and madvise() in chunks rather than page by page.

This optimization requires the negotiation of a new feature bit,
VIRTIO_BALLOON_F_CHUNK_TRANSFER.

With this new feature, the above ballooning process takes ~590ms
resulting in an improvement of ~85%.

TODO: optimize stage 1) by allocating/freeing a chunk of pages
instead of a single page each time.

Signed-off-by: Liang Li 
Signed-off-by: Wei Wang 
Suggested-by: Michael S. Tsirkin 
---
 drivers/virtio/virtio_balloon.c | 371 +---
 include/uapi/linux/virtio_balloon.h |   9 +
 2 files changed, 353 insertions(+), 27 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index f59cb4f..3f4a161 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -42,6 +42,10 @@
 #define OOM_VBALLOON_DEFAULT_PAGES 256
 #define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
 
+#define PAGE_BMAP_SIZE (8 * PAGE_SIZE)
+#define PFNS_PER_PAGE_BMAP (PAGE_BMAP_SIZE * BITS_PER_BYTE)
+#define PAGE_BMAP_COUNT_MAX32
+
 static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
 module_param(oom_pages, int, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
@@ -50,6 +54,14 @@ MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
 static struct vfsmount *balloon_mnt;
 #endif
 
+#define BALLOON_CHUNK_BASE_SHIFT 12
+#define BALLOON_CHUNK_SIZE_SHIFT 12
+struct balloon_page_chunk {
+   __le64 base;
+   __le64 size;
+};
+
+typedef __le64 resp_data_t;
 struct virtio_balloon {
struct virtio_device *vdev;
struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
@@ -67,6 +79,31 @@ struct virtio_balloon {
 
/* Number of balloon pages we've told the Host we're not using. */
unsigned int num_pages;
+   /* Pointer to the response header. */
+   struct virtio_balloon_resp_hdr *resp_hdr;
+   /* Pointer to the start address of response data. */
+   resp_data_t *resp_data;
+   /* Size of response data buffer. */
+   unsigned int resp_buf_size;
+   /* Pointer offset of the response data. */
+   unsigned int resp_pos;
+   /* Bitmap used to record pages */
+   unsigned long *page_bmap[PAGE_BMAP_COUNT_MAX];
+   /* Number of split page bmaps */
+   unsigned int page_bmaps;
+
+   /*
+* The allocated page_bmap size may be smaller than the pfn range of
+* the ballooned pages. In this case, we need to use the page_bmap
+* multiple times to cover the entire pfn range. It's like using a
+* short ruler several times to finish measuring a long object.
+* The start location of the ruler in the next measurement is the end
+* location of the ruler in the previous measurement.
+*
+* pfn_max & pfn_min: forms the pfn range of the ballooned pages
+* pfn_start & pfn_stop: records the start and stop pfn in each cover
+*/
+   unsigned long pfn_min, pfn_max, pfn_start, pfn_stop;
/*
 * The pages we've told the Host we're not using are enqueued
 * at vb_dev_info->pages list.
@@ -110,20 +147,187 @@ static void balloon_ack(struct virtqueue *vq)
wake_up(&vb->acked);
 }
 
-static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
+static inline void init_page_bmap_range(struct virtio_balloon *vb)
+{
+   vb->pfn_min = ULONG_MAX;
+   vb->pfn_max = 0;
+}
+
+static inline void update_page_bmap_range(struct virtio_balloon *vb,
+ struct page *page)
+{
+   unsigned long balloon_pfn = page_to_balloon_pfn(page);
+
+   vb->pfn_min = min(balloon_pfn, vb->pfn_min);
+   vb->pfn_max = max(balloon_pfn, vb->pfn_max);
+}
+

[Qemu-devel] [PATCH kernel v8 4/4] virtio-balloon: VIRTIO_BALLOON_F_HOST_REQ_VQ

2017-03-16 Thread Wei Wang
From: Liang Li 

Add a new vq, host request vq. The host uses the vq to send
requests to the guest. Upon getting a request, the guest responds
what the host needs via this vq.

The patch implements the request of getting the unsed pages from the guest.
The unused guest pages are avoided to migrate in live migration. For an
idle guest with 8GB RAM, this optimization shorterns the total migration
time to 1/4.

Furthermore, it's also possible to drop the guest's page cache before
live migration. This optimization will be implemented on top of this
new feature in the future.

Signed-off-by: Liang Li 
Signed-off-by: Wei Wang 
---
 drivers/virtio/virtio_balloon.c | 140 ++--
 include/uapi/linux/virtio_balloon.h |  22 ++
 2 files changed, 157 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 3f4a161..bcf2baa 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -64,7 +64,7 @@ struct balloon_page_chunk {
 typedef __le64 resp_data_t;
 struct virtio_balloon {
struct virtio_device *vdev;
-   struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
+   struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *host_req_vq;
 
/* The balloon servicing is delegated to a freezable workqueue. */
struct work_struct update_balloon_stats_work;
@@ -104,6 +104,8 @@ struct virtio_balloon {
 * pfn_start & pfn_stop: records the start and stop pfn in each cover
 */
unsigned long pfn_min, pfn_max, pfn_start, pfn_stop;
+   /* Request header */
+   struct virtio_balloon_req_hdr req_hdr;
/*
 * The pages we've told the Host we're not using are enqueued
 * at vb_dev_info->pages list.
@@ -568,6 +570,81 @@ static void stats_handle_request(struct virtio_balloon *vb)
virtqueue_kick(vq);
 }
 
+static void __send_unused_pages(struct virtio_balloon *vb,
+   unsigned long req_id, unsigned int pos,
+   bool done)
+{
+   struct virtio_balloon_resp_hdr *hdr = vb->resp_hdr;
+   struct virtqueue *vq = vb->host_req_vq;
+
+   vb->resp_pos = pos;
+   hdr->cmd = BALLOON_GET_UNUSED_PAGES;
+   hdr->id = cpu_to_le16(req_id);
+   if (!done)
+   hdr->flag = BALLOON_FLAG_CONT;
+   else
+   hdr->flag = BALLOON_FLAG_DONE;
+
+   if (pos > 0 || done)
+   send_resp_data(vb, vq, true);
+
+}
+
+static void send_unused_pages(struct virtio_balloon *vb,
+ unsigned long req_id)
+{
+   struct scatterlist sg_in;
+   unsigned int pos = 0;
+   struct virtqueue *vq = vb->host_req_vq;
+   int ret, order;
+   struct zone *zone = NULL;
+   bool part_fill = false;
+
+   mutex_lock(&vb->balloon_lock);
+
+   for (order = MAX_ORDER - 1; order >= 0; order--) {
+   ret = record_unused_pages(&zone, order, vb->resp_data,
+ vb->resp_buf_size / sizeof(__le64),
+ &pos, part_fill);
+   if (ret == -ENOSPC) {
+   if (pos == 0) {
+   void *new_resp_data;
+
+   new_resp_data = kmalloc(2 * vb->resp_buf_size,
+   GFP_KERNEL);
+   if (new_resp_data) {
+   kfree(vb->resp_data);
+   vb->resp_data = new_resp_data;
+   vb->resp_buf_size *= 2;
+   } else {
+   part_fill = true;
+   dev_warn(&vb->vdev->dev,
+"%s: part fill order: %d\n",
+__func__, order);
+   }
+   } else {
+   __send_unused_pages(vb, req_id, pos, false);
+   pos = 0;
+   }
+
+   if (!part_fill) {
+   order++;
+   continue;
+   }
+   } else
+   zone = NULL;
+
+   if (order == 0)
+   __send_unused_pages(vb, req_id, pos, true);
+
+   }
+
+   mutex_unlock(&vb->balloon_lock);
+   sg_init_one(&sg_in, &vb->req_hdr, sizeof(vb->req_hdr));
+   virtqueue_add_inbuf(vq, &sg_in, 1, &vb->req_hdr, GFP_KERNEL);
+   virtqueue_kick(vq);
+}
+
 static void virtballoon_changed(struct virtio_device *vdev)
 {
struct virtio_balloon *vb = vdev->priv;
@@ -667,18 +744,53 @@ static void update_balloon_size_func(struct work_struct 
*work)
queue_work(system_freezable_wq, work);
 }
 
+stati

[Qemu-devel] [PATCH kernel v8 1/4] virtio-balloon: deflate via a page list

2017-03-16 Thread Wei Wang
From: Liang Li 

This patch saves the deflated pages to a list, instead of the PFN array.
Accordingly, the balloon_pfn_to_page() function is removed.

Signed-off-by: Liang Li 
Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Wei Wang 
---
 drivers/virtio/virtio_balloon.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 181793f..f59cb4f 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -103,12 +103,6 @@ static u32 page_to_balloon_pfn(struct page *page)
return pfn * VIRTIO_BALLOON_PAGES_PER_PAGE;
 }
 
-static struct page *balloon_pfn_to_page(u32 pfn)
-{
-   BUG_ON(pfn % VIRTIO_BALLOON_PAGES_PER_PAGE);
-   return pfn_to_page(pfn / VIRTIO_BALLOON_PAGES_PER_PAGE);
-}
-
 static void balloon_ack(struct virtqueue *vq)
 {
struct virtio_balloon *vb = vq->vdev->priv;
@@ -181,18 +175,16 @@ static unsigned fill_balloon(struct virtio_balloon *vb, 
size_t num)
return num_allocated_pages;
 }
 
-static void release_pages_balloon(struct virtio_balloon *vb)
+static void release_pages_balloon(struct virtio_balloon *vb,
+struct list_head *pages)
 {
-   unsigned int i;
-   struct page *page;
+   struct page *page, *next;
 
-   /* Find pfns pointing at start of each page, get pages and free them. */
-   for (i = 0; i < vb->num_pfns; i += VIRTIO_BALLOON_PAGES_PER_PAGE) {
-   page = balloon_pfn_to_page(virtio32_to_cpu(vb->vdev,
-  vb->pfns[i]));
+   list_for_each_entry_safe(page, next, pages, lru) {
if (!virtio_has_feature(vb->vdev,
VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
adjust_managed_page_count(page, 1);
+   list_del(&page->lru);
put_page(page); /* balloon reference */
}
 }
@@ -202,6 +194,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, 
size_t num)
unsigned num_freed_pages;
struct page *page;
struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
+   LIST_HEAD(pages);
 
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
@@ -215,6 +208,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, 
size_t num)
if (!page)
break;
set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+   list_add(&page->lru, &pages);
vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
}
 
@@ -226,7 +220,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, 
size_t num)
 */
if (vb->num_pfns != 0)
tell_host(vb, vb->deflate_vq);
-   release_pages_balloon(vb);
+   release_pages_balloon(vb, &pages);
mutex_unlock(&vb->balloon_lock);
return num_freed_pages;
 }
-- 
2.7.4




Re: [Qemu-devel] [RFC PATCH v2 30/30] trace: Force compiler warnings on trace parameter type mismatches

2017-03-16 Thread Stefan Hajnoczi
On Wed, Mar 15, 2017 at 02:59:55PM -0500, Eric Blake wrote:
> On 03/13/2017 02:55 PM, Eric Blake wrote:
> > 2. there are still failures under 'make docker-test-mingw@fedora'
> > due to more type mismatches that still need to be squashed. I'm
> > still working on fixing those, but wanted to at least post this
> > series for initial review, especially so the maintainer can weigh
> > in on how much (or little) belongs in 2.9
> 
> mingw is a nightmare.  Things like ntohl() having non-standard behavior
> on 32-bit mingw can only be worked around by hairy redefinitions in
> osdep.h or by casts at the callsites. See the conversation in 6/30, but
> I'm hoping gcc will give us a new knob to tone down the level of
> -Wformat warnings for two types that have different rank but the same
> width; if gcc gives us that, then we'd add a configure test, and
> conditionally generate the type-mismatch checking code ONLY when gcc is
> smart enough to not care about rank mismatch.  Until then, this RFC
> 30/30 proved useful for flushing out the earlier cleanups (many of which
> you can/should still accept), but should NOT be incorporated as-is, not
> even when 2.10 opens.

QEMU 2.9 regression fixes are appropriate at this stage in the release
process.

Fixes for bugs already in 2.8 are less likely to be merged as we head
towards -rc2/-rc3.

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [Qemu-ppc] qemu-system-ppc video artifacts since "tcg: drop global lock during TCG code execution"

2017-03-16 Thread Gerd Hoffmann
  Hi,

> >> Note that KVM has some similar hacks to avoid trapping all writes to
> >> video memory with its coalesced mmio mechanism however I'm not familiar
> >> with all the details.
> >
> > Normal linear framebuffer access doesn't use this.
> 
> Ahh OK - as I said I wasn't super familiar with what coalesced mmio was
> trying to achieve. I assume it is trying to avoid trapping on every
> single MMIO access?

You can't avoid the trap, but you can avoid notifying userspace about
each and every single trap and batch things.

But vga framebuffer is normal ram in most video modes, it doesn't trap
(except on first access with dirty bit clear, for dirty bit tracking).  

IIRC some of the historic video modes which are pretty much unused these
days (planar 4bit) trap on each access and perform better with coalesced
mmio.  Also nics which use mmio instead of dma for package xfer.

> > I suspect the memory_region_sync_dirty_bitmap call on tcg should reset
> > the fast path optimization, so the slow path can update the dirty bits
> > correctly.
> 
> Sure. And for the low level cputlb implementation we can clear those
> bits atomically. However when the memory region is synced we also need
> to flush any entries in the TLB that have already been hit and cleared
> TLB_NOTDIRTY to we can trigger the slow path again. This is tricky from
> outside of a vCPU context because we can't just queue the work and exit
> the vCPU run loop to reach a known CPU state.

Hmm, ok.

> The RFC PATCH I sent earlier solves this by ensuring the update runs in
> a quiescent period (i.e. when the vCPUs are not running) but it is
> sub-optimal as it means the display code has to have a basic knowledge
> of vCPUs and when they run.

Still the best option for 2.9 I guess ...

cheers,
  Gerd




Re: [Qemu-devel] [PATCH v2 02/30] trace: Fix incorrect megasas trace parameters

2017-03-16 Thread Stefan Hajnoczi
On Mon, Mar 13, 2017 at 02:55:19PM -0500, Eric Blake wrote:
> hw/scsi/trace-events lists cmd as the first parameter for both
> megasas_iovec_overflow and megasas_iovec_underflow, but the caller
> was mistakenly passing cmd->iov_size twice instead of the command
> index.  Also, trace_megasas_abort_invalid is called with parameters
> in the wrong order.  Broken since its introduction in commit
> e8f943c3.
> 
> Signed-off-by: Eric Blake 
> ---
>  hw/scsi/megasas.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, applied to my tracing-next tree:
https://github.com/stefanha/qemu/commits/tracing-next

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 01/30] trace: Fix backwards mirror_yield parameters

2017-03-16 Thread Stefan Hajnoczi
On Mon, Mar 13, 2017 at 02:55:18PM -0500, Eric Blake wrote:
> block/trace-events lists the parameters for mirror_yield
> consistently with other mirror events (cnt just after s, like in
> mirror_before_sleep; in_flight last, like in mirror_yield_in_flight).
> But the callers were passing parameters in the wrong order, leading
> to poor trace messages, including type truncation when there are
> more than 4G dirty sectors involved.  Broken since its introduction
> in commit bd48bde.
> 
> While touching this, ensure that all callers use the same type
> (uint64_t) for cnt, as a later patch will enable the compiler to do
> stricter type-checking.
> 
> Signed-off-by: Eric Blake 
> ---
>  block/mirror.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Thanks, applied to my tracing-next tree:
https://github.com/stefanha/qemu/commits/tracing-next

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 03/30] trace: Avoid abuse of amdvi_mmio_read

2017-03-16 Thread Stefan Hajnoczi
On Mon, Mar 13, 2017 at 02:55:20PM -0500, Eric Blake wrote:
> hw/i386/trace-events has an amdvi_mmio_read trace that is used for
> both normal reads (listing the register name, address, size, and
> offset) and for an error case (abusing the register name to show
> an error message, the address to show the maximum value supported,
> then shoehorning address and size into the size and offset
> parameters).  The change from a wide address to a narrower size
> parameter could truncate a (rather-large) bogus read attempt, so
> it's better to create a separate dedicated trace with correct types,
> rather than abusing the trace mechanism.  Broken since its
> introduction in commit d29a09c.
> 
> Signed-off-by: Eric Blake 
> ---
>  hw/i386/amd_iommu.c  | 3 +--
>  hw/i386/trace-events | 1 +
>  2 files changed, 2 insertions(+), 2 deletions(-)

Thanks, applied to my tracing-next tree:
https://github.com/stefanha/qemu/commits/tracing-next

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 29/30] trace: Fix parameter types in hw/virtio

2017-03-16 Thread Stefan Hajnoczi
On Mon, Mar 13, 2017 at 02:55:46PM -0500, Eric Blake wrote:
> An upcoming patch will let the compiler warn us when we are silently
> losing precision in traces; update the trace definitions to pass
> through the full value at the callsite.
> 
> Signed-off-by: Eric Blake 
> ---
>  hw/virtio/trace-events | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 6926eed..7b3743d 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -4,7 +4,7 @@
>  virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int 
> idx) "vq %p elem %p len %u idx %u"
>  virtqueue_flush(void *vq, unsigned int count) "vq %p count %u"
>  virtqueue_pop(void *vq, void *elem, unsigned int in_num, unsigned int 
> out_num) "vq %p elem %p in_num %u out_num %u"
> -virtio_queue_notify(void *vdev, int n, void *vq) "vdev %p n %d vq %p"
> +virtio_queue_notify(void *vdev, ptrdiff_t n, void *vq) "vdev %p n %td vq %p"

I'm not sure about this change.  Logically the virtqueue number, n, is
not a ptrdiff_t.  The type for n is always int in
include/hw/virtio/virtio.h and it should be the same for the trace
event.

It just happens to be that the caller calculates it using pointer
subtraction.  The trace event shouldn't reflect this.  One day the
caller might change the calculation and it would then be necessary to
change the type of the trace event.

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [Qemu-ppc] qemu-system-ppc video artifacts since "tcg: drop global lock during TCG code execution"

2017-03-16 Thread Alex Bennée

Paolo Bonzini  writes:

> On 14/03/2017 18:34, BALATON Zoltan wrote:
>> Like from the display controller models that use
>> memory_region_get_dirty() to check if the frambuffer needs to be
>> updated? But all display adaptors seem to do this and the problem was
>> only seem on ppc so it may be related to something ppc specific.
>
> You need to use test_and_clear_dirty instead of get_dirty/reset_dirty.
> Or alternatively you need to reset immediately after get_dirty.  At
> least cg3.c is doing
>
>   read dirty bitmap
>   read VRAM
>   clear dirty bitmap
>
> which has a race.

Are you saying this is also racy also in the KVM case or just that TCG
doesn't currently sync up with the current dirty bitmap mechanism?

AIUI the memory regions are under RCU so you always get a consistent
view (with updates after you take a copy going to the next iteration).
What I think needs doing is hooking into the ->log-sync mechanism to
reset SoftMMU TLB entries so the dirty detection carries on for the next
sync point?

--
Alex Bennée



Re: [Qemu-devel] [Qemu-ppc] qemu-system-ppc video artifacts since "tcg: drop global lock during TCG code execution"

2017-03-16 Thread Alex Bennée

Gerd Hoffmann  writes:

>   Hi,
>
>> >> Note that KVM has some similar hacks to avoid trapping all writes to
>> >> video memory with its coalesced mmio mechanism however I'm not familiar
>> >> with all the details.
>> >
>> > Normal linear framebuffer access doesn't use this.
>>
>> Ahh OK - as I said I wasn't super familiar with what coalesced mmio was
>> trying to achieve. I assume it is trying to avoid trapping on every
>> single MMIO access?
>
> You can't avoid the trap, but you can avoid notifying userspace about
> each and every single trap and batch things.
>
> But vga framebuffer is normal ram in most video modes, it doesn't trap
> (except on first access with dirty bit clear, for dirty bit tracking).
>
> IIRC some of the historic video modes which are pretty much unused these
> days (planar 4bit) trap on each access and perform better with coalesced
> mmio.  Also nics which use mmio instead of dma for package xfer.
>
>> > I suspect the memory_region_sync_dirty_bitmap call on tcg should reset
>> > the fast path optimization, so the slow path can update the dirty bits
>> > correctly.
>>
>> Sure. And for the low level cputlb implementation we can clear those
>> bits atomically. However when the memory region is synced we also need
>> to flush any entries in the TLB that have already been hit and cleared
>> TLB_NOTDIRTY to we can trigger the slow path again. This is tricky from
>> outside of a vCPU context because we can't just queue the work and exit
>> the vCPU run loop to reach a known CPU state.
>
> Hmm, ok.
>
>> The RFC PATCH I sent earlier solves this by ensuring the update runs in
>> a quiescent period (i.e. when the vCPUs are not running) but it is
>> sub-optimal as it means the display code has to have a basic knowledge
>> of vCPUs and when they run.
>
> Still the best option for 2.9 I guess ...

I think so although Paolo has pointed out potential issues with the
other display adaptors.

As this dirty tracking is also used for live migration I also need to
check if the BQL change has broken that for TCG guests or that it just
never worked anyway. If it is the later then that gives a bit more
breathing room for plumbing in the full fix to keep the memory_regions
view of the world consistent with the SoftMMU TLB.

>
> cheers,
>   Gerd


--
Alex Bennée



Re: [Qemu-devel] [PATCH v2] Change the method to calculate dirty-pages-rate

2017-03-16 Thread Juan Quintela
Chao Fan  wrote:
> In function cpu_physical_memory_sync_dirty_bitmap, file
> include/exec/ram_addr.h:
>
> if (src[idx][offset]) {
> unsigned long bits = atomic_xchg(&src[idx][offset], 0);
> unsigned long new_dirty;
> new_dirty = ~dest[k];
> dest[k] |= bits;
> new_dirty &= bits;
> num_dirty += ctpopl(new_dirty);
> }
>
> After these codes executed, only the pages not dirtied in bitmap(dest),
> but dirtied in dirty_memory[DIRTY_MEMORY_MIGRATION] will be calculated.
> For example:
> When ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] = 0b,
> and atomic_rcu_read(&migration_bitmap_rcu)->bmap = 0b0011,
> the new_dirty will be 0b1100, and this function will return 2 but not
> 4 which is expected.
> the dirty pages in dirty_memory[DIRTY_MEMORY_MIGRATION] are all new,
> so these should be calculated also.
>
> Signed-off-by: Chao Fan 
> Signed-off-by: Li Zhijian 
>
> ---
> v2: Remove the parameter 'num_dirty_pages_init'
> Fix incoming parameters of trace_migration_bitmap_sync_end

queued



Re: [Qemu-devel] [PATCH for 2.9] migration: use "" as the default for tls-creds/hostname

2017-03-16 Thread Juan Quintela
"Daniel P. Berrange"  wrote:
> The tls-creds parameter has a default value of NULL indicating
> that TLS should not be used. Setting it to non-NULL enables
> use of TLS. Once tls-creds are set to a non-NULL value via the
> monitor, it isn't possible to set them back to NULL again, due
> to current implementation limitations. The empty string is not
> a valid QObject identifier, so this switches to use "" as the
> default, indicating that TLS will not be used
>
> The tls-hostname parameter has a default value of NULL indicating
> the the hostname from the migrate connection URI should be used.
> Again, once tls-hostname is set non-NULL, to override the default
> hostname for x509 cert validation, it isn't possible to reset it
> back to NULL via the monitor. The empty string is not a valid
> hostname, so this switches to use "" as the default, indicating
> that the migrate URI hostname should be used.
>
> Using "" as the default for both, also means that the monitor
> commands "info migrate_parameters" / "query-migrate-parameters"
> will report existance of tls-creds/tls-parameters even when set
> to their default values.
>
> Signed-off-by: Daniel P. Berrange 

queued



Re: [Qemu-devel] [PATCH v2] migration/block: Avoid invoking blk_drain too frequently

2017-03-16 Thread Juan Quintela
Fam Zheng  wrote:
> On Wed, 03/15 17:31, Dr. David Alan Gilbert wrote:
>> * Fam Zheng (f...@redhat.com) wrote:
>> > On Wed, 03/15 11:37, Lidong Chen wrote:
>> > > Increase bmds->cur_dirty after submit io, so reduce the frequency
>> > > involve into blk_drain, and improve the performance obviously
>> > > when block migration.
>> > > 
>> > > The performance test result of this patch:
>> > > 
>> > > During the block dirty save phase, this patch improve guest os IOPS
>> > > from 4.0K to 9.5K. and improve the migration speed from
>> > > 505856 rsec/s to 855756 rsec/s.
>> > > 
>> > > Signed-off-by: Lidong Chen 
>> > > ---
>> > >  migration/block.c | 3 +++
>> > >  1 file changed, 3 insertions(+)
>> > > 
>> > > diff --git a/migration/block.c b/migration/block.c
>> > > index 6741228..7734ff7 100644
>> > > --- a/migration/block.c
>> > > +++ b/migration/block.c
>> > > @@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, 
>> > > BlkMigDevState *bmds,
>> > >  }
>> > >  
>> > >  bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, 
>> > > nr_sectors);
>> > > +sector += nr_sectors;
>> > > +bmds->cur_dirty = sector;
>> > > +
>> > >  break;
>> > >  }
>> > >  sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
>> > > -- 
>> > > 1.8.3.1
>> > > 
>> > 
>> > Nice catch above all, thank you!
>> > 
>> > Reviewed-by: Fam Zheng 
>> 
>> Are you taking that via a block pull?
>
> I can do that, but I'm not sure whether it should go to 2.9. This is a
> performance improvement, which usually doesn't qualify as bug fixes. But this
> also looks like a mistake in original code.
>
> Fam

I am taking it through migration and push it.  I agree with your description.



Re: [Qemu-devel] [PATCH v1 1/1] vmstate: fix failed iotests case 68 and 91

2017-03-16 Thread Juan Quintela
QingFeng Hao  wrote:
> This problem affects s390x only if we are running without KVM.
> Basically, S390CPU.irqstate is unused if we do not use KVM,
> and thus no buffer is allocated.
> This causes size=0, first_elem=NULL and n_elems=1 in
> vmstate_load_state and vmstate_save_state. And the assert fails.
> With this fix we can go back to the old behavior and support
> VMS_VBUFFER with size 0 and nullptr.
>
> Signed-off-by: QingFeng Hao 
> Signed-off-by: Halil Pasic 

queued



Re: [Qemu-devel] [RFC patch 1/3] blockjob: add block_job_start_shim

2017-03-16 Thread Paolo Bonzini


On 16/03/2017 01:46, John Snow wrote:
> +/**
> + * This code must run after the job's coroutine has been entered,
> + * but not before.
> + */
> +static void coroutine_fn block_job_start_shim(void *opaque)
> +{
> +BlockJob *job = opaque;
> +
> +assert(job && job->driver && job->driver->start);
> +block_job_pause_point(job);
> +job->driver->start(job);
> +}

Poor function, don't call it a shim :)  Let's just call it
block_job_co_entry.

Paolo

>  void block_job_start(BlockJob *job)
>  {
>  assert(job && !block_job_started(job) && job->paused &&
> -   !job->busy && job->driver->start);
> -job->co = qemu_coroutine_create(job->driver->start, job);
> -if (--job->pause_count == 0) {
> -job->paused = false;
> -job->busy = true;
> -qemu_coroutine_enter(job->co);
> -}
> +   job->driver && job->driver->start);
> +job->co = qemu_coroutine_create(block_job_start_shim, job);
> +job->pause_count--;
> +job->busy = true;
> +job->paused = false;
> +qemu_coroutine_enter(job->co);
>  }
>  
>  void block_job_ref(BlockJob *job)



Re: [Qemu-devel] [RFC patch 2/3] block-backend: add drained_begin / drained_end ops

2017-03-16 Thread Paolo Bonzini


On 16/03/2017 01:46, John Snow wrote:
> 
> - Does the presence of blk->quiesce_counter relieve the burden of needing
>   blk->public.io_limits_disabled? I could probably eliminate this counter
>   entirely and just spy on the root node's quiescent state at key moments
>   instead. I am confident I'm traipsing on delicate drain semantics.

Tricky question.  I believe io_limits_disabled is a bit of a hack.

Certainly you could get rid of disable_external now that we have
drained_begin/drained_end, but it would be a separate patch.

> - Should I treat the separation of a quisced BDS/BB as a drained_end request
>   as an analogue to how blk_insert_bs (in this patch) handles this?

I think so.

Paolo



Re: [Qemu-devel] [RFC patch 3/3] blockjob: add devops to blockjob backends

2017-03-16 Thread Paolo Bonzini


On 16/03/2017 01:46, John Snow wrote:
> - BlkDevOps is traditionally only for Qdev devices, and a BlockJob is not
>   currently a 'device'... Do we want to loosen this restriction, find another
>   way to deliver callbacks to BlockJobs attached to BlkBackends, or do 
> something
>   crazy like make a BlockJob device object?
> 
>   struct JobDevice {
> DeviceState parent_obj;
> BlockJob *job;
>   } ...??

I think we want to loosen it.

> - Not so sure about leaving the initial job refcount at 1, since we are giving
>   away a handle to this job as dev_opaque. By incrementing it to 2, however,
>   it's not clear whose responsibility it is to decrement it again.

It would be another callback, sent at the time the dev_ops are removed.
But devices have been treating dev_opaque as a weak reference, it makes
sense to do the same for jobs.

Paolo



Re: [Qemu-devel] [Qemu-ppc] qemu-system-ppc video artifacts since "tcg: drop global lock during TCG code execution"

2017-03-16 Thread Paolo Bonzini


On 16/03/2017 08:51, Alex Bennée wrote:
> 
> Paolo Bonzini  writes:
> 
>> On 14/03/2017 18:34, BALATON Zoltan wrote:
>>> Like from the display controller models that use
>>> memory_region_get_dirty() to check if the frambuffer needs to be
>>> updated? But all display adaptors seem to do this and the problem was
>>> only seem on ppc so it may be related to something ppc specific.
>>
>> You need to use test_and_clear_dirty instead of get_dirty/reset_dirty.
>> Or alternatively you need to reset immediately after get_dirty.  At
>> least cg3.c is doing
>>
>>  read dirty bitmap
>>  read VRAM
>>  clear dirty bitmap
>>
>> which has a race.
> 
> Are you saying this is also racy also in the KVM case or just that TCG
> doesn't currently sync up with the current dirty bitmap mechanism?

It's okay for KVM because the dirty bitmap is copied from KVM by the
device itself, before updating the screen (with
memory_region_sync_dirty_bitmap).  For TCG, on the other hand, there is
full concurrency between the CPU that sets the bits and the device that
clears them.

> AIUI the memory regions are under RCU so you always get a consistent
> view (with updates after you take a copy going to the next iteration).

No, RCU only protects against resizes of the bitmap.  The bitmap is not
copied on every access (of course :)).

> What I think needs doing is hooking into the ->log-sync mechanism to
> reset SoftMMU TLB entries so the dirty detection carries on for the next
> sync point?

It's much simpler than that, just clear the dirty bitmap bit before
reading the memory.

Paolo



Re: [Qemu-devel] [PATCH v3 00/11] MTTCG fix-ups for 2.9

2017-03-16 Thread Pavel Dovgalyuk
> From: Alex Bennée [mailto:alex.ben...@linaro.org]
> Pavel Dovgalyuk  writes:
> >> From: Alex Bennée [mailto:alex.ben...@linaro.org]
> >> Pavel Dovgalyuk  writes:
> >> >> From: mttcg-requ...@listserver.greensocs.com [mailto:mttcg-
> >> requ...@listserver.greensocs.com]
> >> >>
> >> >> The next thing on my list it to look at the icount problems and review
> >> >> Paolo's fixes for it. However those fixes should go in a separate
> >> >> series and I assume via Paolo's tree.
> >> >
> >> > Do you mean those problems that completely broke icount?
> >>
> >> Completely broke is a bit strong. Certainly I tested icount on my
> >> patches but I missed the pathological failure mode that led to the
> >> interaction between the BQL lock breaking patch and running a real
> >> guest. It didn't break icount so much as slow down guests so much they
> >> never got round to finishing their IRQ handling.
> >
> > That might look as slowing down in regular icount mode.
> > But it becomes non-deterministic in record/replay mode.
> > Therefore none of the recorded scenarios may be replayed.
> >
> > I tried the simplest command lines:
> >
> > qemu-system-i386.exe -icount shift=7,rr=record,rrfile=replay.bin -net
> > none
> 
> Running this against 2421f381dc (pre-merge of MTTCG) from the source
> tree generates no output. My command is on Linux:
> 
>   /x86_64-softmmu/qemu-system-x86_64 -icount 
> shift=7,rr=record,rrfile=replay.bin -net none
> 
> Do I need to specify the BIOS manually?

No, this command line works well for me. BIOS executes and shows some messages.
Do you have any graphical output for QEMU?

> > qemu-system-i386.exe -icount shift=7,rr=replay,rrfile=replay.bin -net none
> >
> > First one was used to record execution until BIOS will print its startup 
> > info.
> > The second one is for replay, but it can replay about 20 instructions
> > until the problems with icount manifest itself - the execution does
> > not proceed.
> 
> What error message does it give? How do you know how many instructions
> have been executed?

It hangs after executing about 20 instructions.
I checked -d exec logs.

> I ran the following test on both pre-mttcg-merge and my current HEAD
> which includes Paolo's fix series:
> 
>  ./arm-softmmu/qemu-system-arm -machine type=virt \
> -display none -smp 1 -m 4096 -cpu cortex-a15 \
> -kernel ../images/aarch32-current-linux-initrd-guest.img
> -append "console=ttyAMA0" -serial mon:stdio \
> -net none \
> -icount shift=7,rr=record,rrfile=replay.bin
> 
> And then:
> 
>  ./arm-softmmu/qemu-system-arm -machine type=virt \
> -display none -smp 1 -m 4096 -cpu cortex-a15 \
> -kernel ../images/aarch32-current-linux-initrd-guest.img
> -append "console=ttyAMA0" -serial mon:stdio \
> -net none \
> -icount shift=7,rr=replay,rrfile=replay.bin
> 
> And they both ran the same. However I kept running into:
> 
>  [3.542408] RPC: Registered tcp NFSv4.1 backchannel transport module.
> qemu-system-arm: Missing character write event in the replay log
> 
> This seems to be a pre-existing

Does it mean that qemu-arm platform includes some serial devices that were
not detected by the replay?

Pavel Dovgalyuk




[Qemu-devel] [PATCH v2] x86: Allow to set NUMA distance for different NUMA nodes

2017-03-16 Thread He Chen
Current, QEMU does not provide a clear command to set vNUMA distance for
guest although we already have `-numa` command to set vNUMA nodes.

vNUMA distance makes sense in certain scenario.
But now, if we create a guest that has 4 vNUMA nodes, when we check NUMA
info via `numactl -H`, we will see:

node distance:
node0123
  0:   10   20   20   20
  1:   20   10   20   20
  2:   20   20   10   20
  3:   20   20   20   10

Guest kernel regards all local node as distance 10, and all remote node
as distance 20 when there is no SLIT table since QEMU doesn't build it.
It looks like a little strange when you have seen the distance in an
actual physical machine that contains 4 NUMA nodes. My machine shows:

node distance:
node0123
  0:   10   21   31   41
  1:   21   10   21   31
  2:   31   21   10   21
  3:   41   31   21   10

To set vNUMA distance, guest should see a complete SLIT table.
I found QEMU has provide `-acpitable` command that allows users to add
a ACPI table into guest, but it requires users building ACPI table by
themselves first. Using `-acpitable` to add a SLIT table may be not so
straightforward or flexible, imagine that when the vNUMA configuration
is changes and we need to generate another SLIT table manually. It may
not be friendly to users or upper software like libvirt.

This patch is going to add SLIT table support in QEMU, and provides
additional option `dist` for command `-numa` to allow user set vNUMA
distance by QEMU command.

With this patch, when a user wants to create a guest that contains
several vNUMA nodes and also wants to set distance among those nodes,
the QEMU command would like:

```
-object 
memory-backend-ram,size=1G,prealloc=yes,host-nodes=0,policy=bind,id=node0 \
-numa node,nodeid=0,cpus=0,memdev=node0 \
-object 
memory-backend-ram,size=1G,prealloc=yes,host-nodes=1,policy=bind,id=node1 \
-numa node,nodeid=1,cpus=1,memdev=node1 \
-object 
memory-backend-ram,size=1G,prealloc=yes,host-nodes=2,policy=bind,id=node2 \
-numa node,nodeid=2,cpus=2,memdev=node2 \
-object 
memory-backend-ram,size=1G,prealloc=yes,host-nodes=3,policy=bind,id=node3 \
-numa node,nodeid=3,cpus=3,memdev=node3 \
-numa dist,src=0,dst=1,val=21 \
-numa dist,src=0,dst=2,val=31 \
-numa dist,src=0,dst=3,val=41 \
-numa dist,src=1,dst=0,val=21 \
...
```

Signed-off-by: He Chen 
---
 hw/i386/acpi-build.c| 27 +++
 include/sysemu/numa.h   |  1 +
 include/sysemu/sysemu.h |  3 +++
 numa.c  | 44 
 qapi-schema.json| 24 ++--
 qemu-options.hx | 12 +++-
 6 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2073108..50906b9 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2395,6 +2395,31 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
  table_data->len - srat_start, 1, NULL, NULL);
 }
 
+/*
+ * ACPI spec 5.2.17 System Locality Distance Information Table
+ * (Revision 2.0 or later)
+ */
+static void
+build_slit(GArray *table_data, BIOSLinker *linker, MachineState *machine)
+{
+int slit_start, i, j;
+slit_start = table_data->len;
+
+acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+build_append_int_noprefix(table_data, nb_numa_nodes, 8);
+for (i = 0; i < nb_numa_nodes; i++) {
+for (j = 0; j < nb_numa_nodes; j++) {
+build_append_int_noprefix(table_data, numa_info[i].distance[j], 1);
+}
+}
+
+build_header(linker, table_data,
+ (void *)(table_data->data + slit_start),
+ "SLIT",
+ table_data->len - slit_start, 1, NULL, NULL);
+}
+
 static void
 build_mcfg_q35(GArray *table_data, BIOSLinker *linker, AcpiMcfgInfo *info)
 {
@@ -2678,6 +2703,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
 if (pcms->numa_nodes) {
 acpi_add_table(table_offsets, tables_blob);
 build_srat(tables_blob, tables->linker, machine);
+acpi_add_table(table_offsets, tables_blob);
+build_slit(tables_blob, tables->linker, machine);
 }
 if (acpi_get_mcfg(&mcfg)) {
 acpi_add_table(table_offsets, tables_blob);
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index 8f09dcf..2f7a941 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -21,6 +21,7 @@ typedef struct node_info {
 struct HostMemoryBackend *node_memdev;
 bool present;
 QLIST_HEAD(, numa_addr_range) addr; /* List to store address ranges */
+uint8_t distance[MAX_NODES];
 } NodeInfo;
 
 extern NodeInfo numa_info[MAX_NODES];
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 576c7ce..d674287 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -169,6 +169,9 @@ extern int mem_prealloc;
 
 #define MAX_NODES 128
 #define NUMA_NODE_UNASSIGNED MAX_NODES
+#define MIN_NUMA_DISTANCE 

[Qemu-devel] [PATCH] Add page-size to output in 'info migrate'

2017-03-16 Thread Chao Fan
The number of dirty pages outputed in 'pages' in the command
'info migrate', so add page-size to calculate the number of dirty
pages in bytes.

Signed-off-by: Chao Fan 
Signed-off-by: Li Zhijian 
---
 hmp.c | 3 +++
 include/migration/migration.h | 1 +
 migration/migration.c | 2 ++
 migration/ram.c   | 2 ++
 qapi-schema.json  | 5 -
 5 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index edb8970..be75e71 100644
--- a/hmp.c
+++ b/hmp.c
@@ -215,6 +215,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->ram->normal_bytes >> 10);
 monitor_printf(mon, "dirty sync count: %" PRIu64 "\n",
info->ram->dirty_sync_count);
+monitor_printf(mon, "page size: %" PRIu64 " kbytes\n",
+   info->ram->page_size >> 10);
+
 if (info->ram->dirty_pages_rate) {
 monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
info->ram->dirty_pages_rate);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 5720c88..9fffe73 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -172,6 +172,7 @@ struct MigrationState
 int64_t xbzrle_cache_size;
 int64_t setup_time;
 int64_t dirty_sync_count;
+int64_t page_size;
 /* Count of requests incoming from destination */
 int64_t postcopy_requests;
 
diff --git a/migration/migration.c b/migration/migration.c
index 3dab684..a1cb123 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -645,6 +645,7 @@ static void populate_ram_info(MigrationInfo *info, 
MigrationState *s)
 info->ram->mbps = s->mbps;
 info->ram->dirty_sync_count = s->dirty_sync_count;
 info->ram->postcopy_requests = s->postcopy_requests;
+info->ram->page_size = s->page_size;
 
 if (s->state != MIGRATION_STATUS_COMPLETED) {
 info->ram->remaining = ram_bytes_remaining();
@@ -1115,6 +1116,7 @@ MigrationState *migrate_init(const MigrationParams 
*params)
 s->last_req_rb = NULL;
 error_free(s->error);
 s->error = NULL;
+s->page_size = 0;
 
 migrate_set_state(&s->state, MIGRATION_STATUS_NONE, 
MIGRATION_STATUS_SETUP);
 
diff --git a/migration/ram.c b/migration/ram.c
index 719425b..028cc4c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1998,7 +1998,9 @@ static int ram_save_init_globals(void)
 static int ram_save_setup(QEMUFile *f, void *opaque)
 {
 RAMBlock *block;
+MigrationState *s = migrate_get_current();
 
+s->page_size = TARGET_PAGE_SIZE;
 /* migration has already setup the bitmap, reuse it. */
 if (!migration_in_colo_state()) {
 if (ram_save_init_globals() < 0) {
diff --git a/qapi-schema.json b/qapi-schema.json
index 32b4a4b..4658c3d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -575,6 +575,9 @@
 # @postcopy-requests: The number of page requests received from the destination
 #(since 2.7)
 #
+# @page-size: The number of bytes in page of this gutst.
+#(since 2.10)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'MigrationStats',
@@ -582,7 +585,7 @@
'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
'normal-bytes': 'int', 'dirty-pages-rate' : 'int',
'mbps' : 'number', 'dirty-sync-count' : 'int',
-   'postcopy-requests' : 'int' } }
+   'postcopy-requests' : 'int', 'page-size' : 'int' } }
 
 ##
 # @XBZRLECacheStats:
-- 
2.9.3






[Qemu-devel] [Bug 1673373] [NEW] qemu -version output is incorrect with configure --with-pkgversion

2017-03-16 Thread Jordan Justen
Public bug reported:

Since qemu v2.7.0, up to the current master
(1883ff34b540daacae948f493b0ba525edf5f642)
the pkgversion feature appears to have a bug:

$ ./configure --target-list=x86_64-softmmu --with-pkgversion=foo

Results in this output:

$ x86_64-softmmu/qemu-system-x86_64 -version
QEMU emulator version 2.8.90(foo)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

This appears to have been introduced in:

67a1de0d19 Makefile: Derive "PKGVERSION" from "git describe" by default

The previous commit (077de81a4c) produces this output:

$ x86_64-softmmu/qemu-system-x86_64 -version
QEMU emulator version 2.6.50 (foo), Copyright (c) 2003-2008 Fabrice Bellard

** Affects: qemu
 Importance: Undecided
 Assignee: Jordan Justen (jljusten)
 Status: New

** Changed in: qemu
 Assignee: (unassigned) => Jordan Justen (jljusten)

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

Title:
  qemu -version output is incorrect with configure --with-pkgversion

Status in QEMU:
  New

Bug description:
  Since qemu v2.7.0, up to the current master
  (1883ff34b540daacae948f493b0ba525edf5f642)
  the pkgversion feature appears to have a bug:

  $ ./configure --target-list=x86_64-softmmu --with-pkgversion=foo

  Results in this output:

  $ x86_64-softmmu/qemu-system-x86_64 -version
  QEMU emulator version 2.8.90(foo)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  This appears to have been introduced in:

  67a1de0d19 Makefile: Derive "PKGVERSION" from "git describe" by
  default

  The previous commit (077de81a4c) produces this output:

  $ x86_64-softmmu/qemu-system-x86_64 -version
  QEMU emulator version 2.6.50 (foo), Copyright (c) 2003-2008 Fabrice Bellard

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



[Qemu-devel] [PATCH] qemu: Fix -version with configure --with-pkgversion

2017-03-16 Thread Jordan Justen
This appears to have regressed in 67a1de0d19.

When the configure --with-pkgversion=foo option is used, the output
from -version will look like:

QEMU emulator version 2.8.90(foo)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

After this patch, it will be:

QEMU emulator version 2.8.90 (foo)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

Cc: Paolo Bonzini 
Cc: Fam Zheng 
Fixes: https://bugs.launchpad.net/bugs/1673373
Cc: 1673...@bugs.launchpad.net
Signed-off-by: Jordan Justen 
---
 Makefile  | 2 +-
 configure | 2 +-
 vl.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 1c4c04f6f2..c9df119798 100644
--- a/Makefile
+++ b/Makefile
@@ -289,7 +289,7 @@ qemu-version.h: FORCE
printf '"$(PKGVERSION)"\n'; \
else \
if test -d .git; then \
-   printf '" ('; \
+   printf '"('; \
git describe --match 'v*' 2>/dev/null | tr -d 
'\n'; \
if ! git diff-index --quiet HEAD &>/dev/null; 
then \
printf -- '-dirty'; \
diff --git a/configure b/configure
index 99d8bece70..e94b06b5fc 100755
--- a/configure
+++ b/configure
@@ -1004,7 +1004,7 @@ for opt do
   ;;
   --disable-blobs) blobs="no"
   ;;
-  --with-pkgversion=*) pkgversion=" ($optarg)"
+  --with-pkgversion=*) pkgversion="($optarg)"
   ;;
   --with-coroutine=*) coroutine="$optarg"
   ;;
diff --git a/vl.c b/vl.c
index 0b4ed5241c..3e60e61905 100644
--- a/vl.c
+++ b/vl.c
@@ -1904,7 +1904,7 @@ static void main_loop(void)
 
 static void version(void)
 {
-printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION "\n"
+printf("QEMU emulator version " QEMU_VERSION " " QEMU_PKGVERSION "\n"
QEMU_COPYRIGHT "\n");
 }
 
-- 
2.11.0




Re: [Qemu-devel] [PATCH] blockdev: fix bitmap clear undo

2017-03-16 Thread Kevin Wolf
Am 15.03.2017 um 22:28 hat John Snow geschrieben:
> Only undo the action if we actually prepared the action.
> 
> Signed-off-by: John Snow 

Thanks, applied to the block branch.

Kevin



[Qemu-devel] [PULL 5/6] RAMBlocks: qemu_ram_is_shared

2017-03-16 Thread Juan Quintela
From: "Dr. David Alan Gilbert" 

Provide a helper to say whether a RAMBlock was created as a
shared mapping.

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 exec.c| 5 +
 include/exec/cpu-common.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/exec.c b/exec.c
index a22f5a0..e57a8a2 100644
--- a/exec.c
+++ b/exec.c
@@ -1561,6 +1561,11 @@ const char *qemu_ram_get_idstr(RAMBlock *rb)
 return rb->idstr;
 }
 
+bool qemu_ram_is_shared(RAMBlock *rb)
+{
+return rb->flags & RAM_SHARED;
+}
+
 /* Called with iothread lock held.  */
 void qemu_ram_set_idstr(RAMBlock *new_block, const char *name, DeviceState 
*dev)
 {
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index b62f0d8..4d45a72 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -69,6 +69,7 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool 
round_offset,
 void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
 void qemu_ram_unset_idstr(RAMBlock *block);
 const char *qemu_ram_get_idstr(RAMBlock *rb);
+bool qemu_ram_is_shared(RAMBlock *rb);
 size_t qemu_ram_pagesize(RAMBlock *block);
 size_t qemu_ram_pagesize_largest(void);
 
-- 
2.9.3




[Qemu-devel] [PULL 1/6] Change the method to calculate dirty-pages-rate

2017-03-16 Thread Juan Quintela
From: Chao Fan 

In function cpu_physical_memory_sync_dirty_bitmap, file
include/exec/ram_addr.h:

if (src[idx][offset]) {
unsigned long bits = atomic_xchg(&src[idx][offset], 0);
unsigned long new_dirty;
new_dirty = ~dest[k];
dest[k] |= bits;
new_dirty &= bits;
num_dirty += ctpopl(new_dirty);
}

After these codes executed, only the pages not dirtied in bitmap(dest),
but dirtied in dirty_memory[DIRTY_MEMORY_MIGRATION] will be calculated.
For example:
When ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] = 0b,
and atomic_rcu_read(&migration_bitmap_rcu)->bmap = 0b0011,
the new_dirty will be 0b1100, and this function will return 2 but not
4 which is expected.
the dirty pages in dirty_memory[DIRTY_MEMORY_MIGRATION] are all new,
so these should be calculated also.

Signed-off-by: Chao Fan 
Signed-off-by: Li Zhijian 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 include/exec/ram_addr.h |  5 -
 migration/ram.c | 12 +---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index cd432e7..b05dc84 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -355,7 +355,8 @@ static inline void 
cpu_physical_memory_clear_dirty_range(ram_addr_t start,
 static inline
 uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest,
ram_addr_t start,
-   ram_addr_t length)
+   ram_addr_t length,
+   int64_t *real_dirty_pages)
 {
 ram_addr_t addr;
 unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
@@ -379,6 +380,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned 
long *dest,
 if (src[idx][offset]) {
 unsigned long bits = atomic_xchg(&src[idx][offset], 0);
 unsigned long new_dirty;
+*real_dirty_pages += ctpopl(bits);
 new_dirty = ~dest[k];
 dest[k] |= bits;
 new_dirty &= bits;
@@ -398,6 +400,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned 
long *dest,
 start + addr,
 TARGET_PAGE_SIZE,
 DIRTY_MEMORY_MIGRATION)) {
+*real_dirty_pages += 1;
 long k = (start + addr) >> TARGET_PAGE_BITS;
 if (!test_and_set_bit(k, dest)) {
 num_dirty++;
diff --git a/migration/ram.c b/migration/ram.c
index 719425b..de1e0a3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -576,18 +576,18 @@ static inline bool 
migration_bitmap_clear_dirty(ram_addr_t addr)
 return ret;
 }
 
+static int64_t num_dirty_pages_period;
 static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
 {
 unsigned long *bitmap;
 bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap;
-migration_dirty_pages +=
-cpu_physical_memory_sync_dirty_bitmap(bitmap, start, length);
+migration_dirty_pages += cpu_physical_memory_sync_dirty_bitmap(bitmap,
+ start, length, &num_dirty_pages_period);
 }
 
 /* Fix me: there are too many global variables used in migration process. */
 static int64_t start_time;
 static int64_t bytes_xfer_prev;
-static int64_t num_dirty_pages_period;
 static uint64_t xbzrle_cache_miss_prev;
 static uint64_t iterations_prev;
 
@@ -620,7 +620,6 @@ uint64_t ram_pagesize_summary(void)
 static void migration_bitmap_sync(void)
 {
 RAMBlock *block;
-uint64_t num_dirty_pages_init = migration_dirty_pages;
 MigrationState *s = migrate_get_current();
 int64_t end_time;
 int64_t bytes_xfer_now;
@@ -646,9 +645,8 @@ static void migration_bitmap_sync(void)
 rcu_read_unlock();
 qemu_mutex_unlock(&migration_bitmap_mutex);
 
-trace_migration_bitmap_sync_end(migration_dirty_pages
-- num_dirty_pages_init);
-num_dirty_pages_period += migration_dirty_pages - num_dirty_pages_init;
+trace_migration_bitmap_sync_end(num_dirty_pages_period);
+
 end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
 
 /* more than 1 second = 1000 millisecons */
-- 
2.9.3




[Qemu-devel] [PULL 0/6] Migration fixes

2017-03-16 Thread Juan Quintela
Hi

This are this week fixes for Migration:
- fix calculation of dirty-pages-rate (it was being too optimistic)
- Make sure we can remove tls params for migration (Dan)
- Disable postcopy with shared memory (Dave)
- migration-block: Don't call blk_drain too much (Lidong)
- Fix failed iotests cases (QingFeng)

Please apply, Juan.

The following changes since commit 1883ff34b540daacae948f493b0ba525edf5f642:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2017-03-15 18:44:05 +)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20170316

for you to fetch changes up to 8679638b0e231f97ad3456c681bf569ca7f7f6d5:

  postcopy: Check for shared memory (2017-03-16 09:02:26 +0100)


migration/next for 20170316


Chao Fan (1):
  Change the method to calculate dirty-pages-rate

Daniel P. Berrange (1):
  migration: use "" as the default for tls-creds/hostname

Dr. David Alan Gilbert (2):
  RAMBlocks: qemu_ram_is_shared
  postcopy: Check for shared memory

Lidong Chen (1):
  migration/block: Avoid invoking blk_drain too frequently

QingFeng Hao (1):
  vmstate: fix failed iotests case 68 and 91

 exec.c|  5 +
 include/exec/cpu-common.h |  1 +
 include/exec/ram_addr.h   |  5 -
 migration/block.c |  3 +++
 migration/migration.c |  4 
 migration/postcopy-ram.c  | 18 ++
 migration/ram.c   | 12 +---
 migration/tls.c   |  2 +-
 migration/vmstate.c   |  8 
 qapi-schema.json  |  4 
 10 files changed, 49 insertions(+), 13 deletions(-)



[Qemu-devel] [PULL 4/6] vmstate: fix failed iotests case 68 and 91

2017-03-16 Thread Juan Quintela
From: QingFeng Hao 

This problem affects s390x only if we are running without KVM.
Basically, S390CPU.irqstate is unused if we do not use KVM,
and thus no buffer is allocated.
This causes size=0, first_elem=NULL and n_elems=1 in
vmstate_load_state and vmstate_save_state. And the assert fails.
With this fix we can go back to the old behavior and support
VMS_VBUFFER with size 0 and nullptr.

Signed-off-by: QingFeng Hao 
Signed-off-by: Halil Pasic 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Juan Quintela 
---
 migration/vmstate.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 78b3cd4..7b4a607 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -109,7 +109,7 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 vmstate_handle_alloc(first_elem, field, opaque);
 if (field->flags & VMS_POINTER) {
 first_elem = *(void **)first_elem;
-assert(first_elem  || !n_elems);
+assert(first_elem || !n_elems || !size);
 }
 for (i = 0; i < n_elems; i++) {
 void *curr_elem = first_elem + size * i;
@@ -117,7 +117,7 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 if (field->flags & VMS_ARRAY_OF_POINTER) {
 curr_elem = *(void **)curr_elem;
 }
-if (!curr_elem) {
+if (!curr_elem && size) {
 /* if null pointer check placeholder and do not follow */
 assert(field->flags & VMS_ARRAY_OF_POINTER);
 ret = vmstate_info_nullptr.get(f, curr_elem, size, NULL);
@@ -325,7 +325,7 @@ void vmstate_save_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
 if (field->flags & VMS_POINTER) {
 first_elem = *(void **)first_elem;
-assert(first_elem  || !n_elems);
+assert(first_elem || !n_elems || !size);
 }
 for (i = 0; i < n_elems; i++) {
 void *curr_elem = first_elem + size * i;
@@ -336,7 +336,7 @@ void vmstate_save_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 assert(curr_elem);
 curr_elem = *(void **)curr_elem;
 }
-if (!curr_elem) {
+if (!curr_elem && size) {
 /* if null pointer write placeholder and do not follow */
 assert(field->flags & VMS_ARRAY_OF_POINTER);
 vmstate_info_nullptr.put(f, curr_elem, size, NULL, NULL);
-- 
2.9.3




[Qemu-devel] [PULL 6/6] postcopy: Check for shared memory

2017-03-16 Thread Juan Quintela
From: "Dr. David Alan Gilbert" 

Postcopy doesn't support migration of RAM shared with another process
yet (we've got a bunch of things to understand).
Check for the case and don't allow postcopy to be enabled.

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/postcopy-ram.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index effbeb6..dc80dbb 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -95,6 +95,19 @@ static bool ufd_version_check(int ufd)
 return true;
 }
 
+/* Callback from postcopy_ram_supported_by_host block iterator.
+ */
+static int test_range_shared(const char *block_name, void *host_addr,
+ ram_addr_t offset, ram_addr_t length, void 
*opaque)
+{
+if (qemu_ram_is_shared(qemu_ram_block_by_name(block_name))) {
+error_report("Postcopy on shared RAM (%s) is not yet supported",
+ block_name);
+return 1;
+}
+return 0;
+}
+
 /*
  * Note: This has the side effect of munlock'ing all of RAM, that's
  * normally fine since if the postcopy succeeds it gets turned back on at the
@@ -127,6 +140,11 @@ bool postcopy_ram_supported_by_host(void)
 goto out;
 }
 
+/* We don't support postcopy with shared RAM yet */
+if (qemu_ram_foreach_block(test_range_shared, NULL)) {
+goto out;
+}
+
 /*
  * userfault and mlock don't go together; we'll put it back later if
  * it was enabled.
-- 
2.9.3




[Qemu-devel] [PULL 3/6] migration/block: Avoid invoking blk_drain too frequently

2017-03-16 Thread Juan Quintela
From: Lidong Chen 

Increase bmds->cur_dirty after submit io, so reduce the frequency
involve into blk_drain, and improve the performance obviously
when block migration.

The performance test result of this patch:

During the block dirty save phase, this patch improve guest os IOPS
from 4.0K to 9.5K. and improve the migration speed from
505856 rsec/s to 855756 rsec/s.

Signed-off-by: Lidong Chen 
Reviewed-by: Fam Zheng 
Signed-off-by: Juan Quintela 
---
 migration/block.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/block.c b/migration/block.c
index 6741228..7734ff7 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -576,6 +576,9 @@ static int mig_save_device_dirty(QEMUFile *f, 
BlkMigDevState *bmds,
 }
 
 bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, sector, nr_sectors);
+sector += nr_sectors;
+bmds->cur_dirty = sector;
+
 break;
 }
 sector += BDRV_SECTORS_PER_DIRTY_CHUNK;
-- 
2.9.3




[Qemu-devel] [PULL 2/6] migration: use "" as the default for tls-creds/hostname

2017-03-16 Thread Juan Quintela
From: "Daniel P. Berrange" 

The tls-creds parameter has a default value of NULL indicating
that TLS should not be used. Setting it to non-NULL enables
use of TLS. Once tls-creds are set to a non-NULL value via the
monitor, it isn't possible to set them back to NULL again, due
to current implementation limitations. The empty string is not
a valid QObject identifier, so this switches to use "" as the
default, indicating that TLS will not be used

The tls-hostname parameter has a default value of NULL indicating
the the hostname from the migrate connection URI should be used.
Again, once tls-hostname is set non-NULL, to override the default
hostname for x509 cert validation, it isn't possible to reset it
back to NULL via the monitor. The empty string is not a valid
hostname, so this switches to use "" as the default, indicating
that the migrate URI hostname should be used.

Using "" as the default for both, also means that the monitor
commands "info migrate_parameters" / "query-migrate-parameters"
will report existance of tls-creds/tls-parameters even when set
to their default values.

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Eric Blake 

Signed-off-by: Juan Quintela 
---
 migration/migration.c | 4 
 migration/tls.c   | 2 +-
 qapi-schema.json  | 4 
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 3dab684..54060f7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -110,6 +110,8 @@ MigrationState *migrate_get_current(void)
 
 if (!once) {
 qemu_mutex_init(¤t_migration.src_page_req_mutex);
+current_migration.parameters.tls_creds = g_strdup("");
+current_migration.parameters.tls_hostname = g_strdup("");
 once = true;
 }
 return ¤t_migration;
@@ -458,6 +460,7 @@ void migration_channel_process_incoming(MigrationState *s,
 ioc, object_get_typename(OBJECT(ioc)));
 
 if (s->parameters.tls_creds &&
+*s->parameters.tls_creds &&
 !object_dynamic_cast(OBJECT(ioc),
  TYPE_QIO_CHANNEL_TLS)) {
 Error *local_err = NULL;
@@ -480,6 +483,7 @@ void migration_channel_connect(MigrationState *s,
 ioc, object_get_typename(OBJECT(ioc)), hostname);
 
 if (s->parameters.tls_creds &&
+*s->parameters.tls_creds &&
 !object_dynamic_cast(OBJECT(ioc),
  TYPE_QIO_CHANNEL_TLS)) {
 Error *local_err = NULL;
diff --git a/migration/tls.c b/migration/tls.c
index 203c11d..45bec44 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -141,7 +141,7 @@ void migration_tls_channel_connect(MigrationState *s,
 return;
 }
 
-if (s->parameters.tls_hostname) {
+if (s->parameters.tls_hostname && *s->parameters.tls_hostname) {
 hostname = s->parameters.tls_hostname;
 }
 if (!hostname) {
diff --git a/qapi-schema.json b/qapi-schema.json
index 32b4a4b..eb9bf67 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1036,6 +1036,8 @@
 # credentials must be for a 'server' endpoint. Setting this
 # will enable TLS for all migrations. The default is unset,
 # resulting in unsecured migration at the QEMU level. (Since 2.7)
+# An empty string means that QEMU will use plain text mode for
+# migration, rather than TLS (Since 2.9)
 #
 # @tls-hostname: #optional hostname of the target host for the migration. This
 #is required when using x509 based TLS credentials and the
@@ -1043,6 +1045,8 @@
 #example if using fd: or exec: based migration, the
 #hostname must be provided so that the server's x509
 #certificate identity can be validated. (Since 2.7)
+#An empty string means that QEMU will use the hostname
+#associated with the migration URI, if any. (Since 2.9)
 #
 # @max-bandwidth: to set maximum speed for migration. maximum speed in
 # bytes per second. (Since 2.8)
-- 
2.9.3




Re: [Qemu-devel] [PULL 1/3] makefile: merge GENERATED_HEADERS & GENERATED_SOURCES variables

2017-03-16 Thread Markus Armbruster
Sorry for chiming in late, I had missed this change.

Stefan Hajnoczi  writes:

> From: "Daniel P. Berrange" 
>
> The only functional difference between the GENERATED_HEADERS
> and GENERATED_SOURCES variables is that 'Makefile' has a
> dependancy on GENERATED_HEADERS, causing generated header files
> to be created immediatey at the start of the build process.
> There is no reason why this early creation should be restricted
> to the .h files, and not include .c files too.

Actually, there is.

Any prerequisites of Makefile are made even by make -n.  Restricting
them to the ones make -n absolutely needs is good practice.

Generated headers must be prerequisites of Makefile, because automatic
dependency generation may fail without them.

There is no such reason for generated non-headers.

>Merge both of
> the variables into a single GENERATED_FILES variable to make
> it clear it is for any type of generated file.

I don't hate this quite enough for an outright NAK at this late stage.
I do hate it enough to ask you to think about it once more.



Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] util: Use g_malloc/g_free in envlist.c

2017-03-16 Thread Stefan Hajnoczi
On Sun, Mar 05, 2017 at 02:56:05AM +, Saurav Sachidanand wrote:
> diff --git a/util/envlist.c b/util/envlist.c
> index e86857e70a..a42eefa5fe 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -17,16 +17,14 @@ static int envlist_parse(envlist_t *envlist,
>  const char *env, int (*)(envlist_t *, const char *));
> 
>  /*
> - * Allocates new envlist and returns pointer to that or
> - * NULL in case of error.
> + * Allocates new envlist and returns pointer to it.
>   */
>  envlist_t *
>  envlist_create(void)
>  {
>   envlist_t *envlist;
> 
> - if ((envlist = malloc(sizeof (*envlist))) == NULL)
> - return (NULL);
> +  envlist = g_malloc(sizeof(*envlist));

Please use tabs to indent in this file.  Normally QEMU coding style uses
4-space indentation but this file contains old code and it's easiest to
leave it undisturbed.

> 
>   QLIST_INIT(&envlist->el_entries);
>   envlist->el_count = 0;
> @@ -49,9 +47,9 @@ envlist_free(envlist_t *envlist)
>   QLIST_REMOVE(entry, ev_link);
> 
>   free((char *)entry->ev_var);
> - free(entry);
> +g_free(entry);
>   }
> - free(envlist);
> +  g_free(envlist);
>  }
> 
>  /*
> @@ -156,15 +154,15 @@ envlist_setenv(envlist_t *envlist, const char *env)
>   if (entry != NULL) {
>   QLIST_REMOVE(entry, ev_link);
>   free((char *)entry->ev_var);
> - free(entry);
> +g_free(entry);
>   } else {
>   envlist->el_count++;
>   }
> 
> - if ((entry = malloc(sizeof (*entry))) == NULL)
> - return (errno);
> - if ((entry->ev_var = strdup(env)) == NULL) {
> - free(entry);
> +  entry = g_malloc(sizeof(*entry));
> +  entry->ev_var = strdup(env);

Could this be converted to g_strdup() in a separate patch?


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH for 2.9] migration: use "" as the default for tls-creds/hostname

2017-03-16 Thread Markus Armbruster
"Dr. David Alan Gilbert"  writes:

> * Daniel P. Berrange (berra...@redhat.com) wrote:
>> The tls-creds parameter has a default value of NULL indicating
>> that TLS should not be used. Setting it to non-NULL enables
>> use of TLS. Once tls-creds are set to a non-NULL value via the
>> monitor, it isn't possible to set them back to NULL again, due
>> to current implementation limitations. The empty string is not
>> a valid QObject identifier, so this switches to use "" as the
>> default, indicating that TLS will not be used
>> 
>> The tls-hostname parameter has a default value of NULL indicating
>> the the hostname from the migrate connection URI should be used.
>> Again, once tls-hostname is set non-NULL, to override the default
>> hostname for x509 cert validation, it isn't possible to reset it
>> back to NULL via the monitor. The empty string is not a valid
>> hostname, so this switches to use "" as the default, indicating
>> that the migrate URI hostname should be used.
>> 
>> Using "" as the default for both, also means that the monitor
>> commands "info migrate_parameters" / "query-migrate-parameters"
>> will report existance of tls-creds/tls-parameters even when set
>> to their default values.
>> 
>> Signed-off-by: Daniel P. Berrange 
>
> Yes, simple enough.
>
> Reviewed-by: Dr. David Alan Gilbert 
>
> Markus, Eric - are you OK with that?

No objections.



[Qemu-devel] [PATCH v3 02/21] mux: simplfy muxes_realize_done

2017-03-16 Thread Marc-André Lureau
mux_chr_event() already send events to all backends, rename it,
export it, and use it from muxes_realize_done. This should help abstract
away mux implementation.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
---
 chardev/char-mux.h |  2 +-
 chardev/char-mux.c | 11 ---
 chardev/char.c |  9 ++---
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/chardev/char-mux.h b/chardev/char-mux.h
index 9a2fffce91..3f41dfcfd2 100644
--- a/chardev/char-mux.h
+++ b/chardev/char-mux.h
@@ -58,6 +58,6 @@ typedef struct MuxChardev {
 
 void mux_chr_set_handlers(Chardev *chr, GMainContext *context);
 void mux_set_focus(Chardev *chr, int focus);
-void mux_chr_send_event(MuxChardev *d, int mux_nr, int event);
+void mux_chr_send_all_event(Chardev *chr, int event);
 
 #endif /* CHAR_MUX_H */
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 5547a36a0a..37d42c65c6 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -114,7 +114,7 @@ static void mux_print_help(Chardev *chr)
 }
 }
 
-void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
+static void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
 {
 CharBackend *be = d->backends[mux_nr];
 
@@ -222,9 +222,9 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, 
int size)
 
 bool muxes_realized;
 
-static void mux_chr_event(void *opaque, int event)
+void mux_chr_send_all_event(Chardev *chr, int event)
 {
-MuxChardev *d = MUX_CHARDEV(opaque);
+MuxChardev *d = MUX_CHARDEV(chr);
 int i;
 
 if (!muxes_realized) {
@@ -237,6 +237,11 @@ static void mux_chr_event(void *opaque, int event)
 }
 }
 
+static void mux_chr_event(void *opaque, int event)
+{
+mux_chr_send_all_event(CHARDEV(opaque), event);
+}
+
 static GSource *mux_chr_add_watch(Chardev *s, GIOCondition cond)
 {
 MuxChardev *d = MUX_CHARDEV(s);
diff --git a/chardev/char.c b/chardev/char.c
index aad639b620..674c097fbe 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -451,22 +451,17 @@ static void muxes_realize_done(Notifier *notifier, void 
*unused)
 {
 Chardev *chr;
 
+muxes_realized = true;
 QTAILQ_FOREACH(chr, &chardevs, next) {
 if (CHARDEV_IS_MUX(chr)) {
-MuxChardev *d = MUX_CHARDEV(chr);
-int i;
-
 /* send OPENED to all already-attached FEs */
-for (i = 0; i < d->mux_cnt; i++) {
-mux_chr_send_event(d, i, CHR_EVENT_OPENED);
-}
+mux_chr_send_all_event(CHARDEV(chr), CHR_EVENT_OPENED);
 /* mark mux as OPENED so any new FEs will immediately receive
  * OPENED event
  */
 qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 }
 }
-muxes_realized = true;
 }
 
 static Notifier muxes_realize_notify = {
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 01/21] char: remove qemu_chr_be_generic_open

2017-03-16 Thread Marc-André Lureau
The function simply alias and hides the real event function.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
---
 include/sysemu/char.h |  1 -
 chardev/char-pty.c|  2 +-
 chardev/char-socket.c |  2 +-
 chardev/char.c| 10 ++
 ui/console.c  |  2 +-
 ui/gtk.c  |  2 +-
 6 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 450881d42c..a30ff3fa80 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -427,7 +427,6 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
  */
 void qemu_chr_fe_take_focus(CharBackend *b);
 
-void qemu_chr_be_generic_open(Chardev *s);
 void qemu_chr_fe_accept_input(CharBackend *be);
 int qemu_chr_add_client(Chardev *s, int fd);
 Chardev *qemu_chr_find(const char *name);
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index a6337be8aa..aa9d0cb2c3 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -185,7 +185,7 @@ static gboolean qemu_chr_be_generic_open_func(gpointer 
opaque)
 PtyChardev *s = PTY_CHARDEV(opaque);
 
 s->open_tag = 0;
-qemu_chr_be_generic_open(chr);
+qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 return FALSE;
 }
 
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index d7e92e1bd3..dc3d3532a7 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -486,7 +486,7 @@ static void tcp_chr_connect(void *opaque)
tcp_chr_read,
chr, NULL);
 }
-qemu_chr_be_generic_open(chr);
+qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 }
 
 static void tcp_chr_update_read_handler(Chardev *chr,
diff --git a/chardev/char.c b/chardev/char.c
index 3df116350b..aad639b620 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -66,12 +66,6 @@ void qemu_chr_be_event(Chardev *s, int event)
 be->chr_event(be->opaque, event);
 }
 
-void qemu_chr_be_generic_open(Chardev *s)
-{
-qemu_chr_be_event(s, CHR_EVENT_OPENED);
-}
-
-
 /* Not reporting errors from writing to logfile, as logs are
  * defined to be "best effort" only */
 static void qemu_chr_fe_write_log(Chardev *s,
@@ -469,7 +463,7 @@ static void muxes_realize_done(Notifier *notifier, void 
*unused)
 /* mark mux as OPENED so any new FEs will immediately receive
  * OPENED event
  */
-qemu_chr_be_generic_open(chr);
+qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 }
 }
 muxes_realized = true;
@@ -581,7 +575,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
 /* We're connecting to an already opened device, so let's make sure we
also get the open event */
 if (s->be_open) {
-qemu_chr_be_generic_open(s);
+qemu_chr_be_event(s, CHR_EVENT_OPENED);
 }
 }
 
diff --git a/ui/console.c b/ui/console.c
index d1ff7504ec..d88c52efe5 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2070,7 +2070,7 @@ static void text_console_do_init(Chardev *chr, 
DisplayState *ds)
 s->t_attrib = s->t_attrib_default;
 }
 
-qemu_chr_be_generic_open(chr);
+qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 }
 
 static void vc_chr_open(Chardev *chr,
diff --git a/ui/gtk.c b/ui/gtk.c
index a86848f3b0..7479ceef35 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1868,7 +1868,7 @@ static GSList *gd_vc_vte_init(GtkDisplayState *s, 
VirtualConsole *vc,
 gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), vc->tab_item,
  gtk_label_new(vc->label));
 
-qemu_chr_be_generic_open(vc->vte.chr);
+qemu_chr_be_event(vc->vte.chr, CHR_EVENT_OPENED);
 
 return group;
 }
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 00/21] chardev clean-ups & tests (after 2.9)

2017-03-16 Thread Marc-André Lureau
Hi,

The following series contains various patches:
- replace "chardevs" list for a /chardevs container object
- add a few read-only socket properties mainly useful for testing
- some chardev related clean-ups
- add various chardev tests

This series is part of a larger refactoring series that I try to keep
up to date here: https://github.com/elmarco/qemu/commits/chrfe

v3:
- open code object_new_with_props() as suggest by Paolo
- added some r-b tags
- rebased

v2:
- replaced root container unref with a TODO
- call object_unparent() directly instead of qemu_chr_delete()
- remove bad qcow2 NULL check removal
- rebased

Marc-André Lureau (21):
  char: remove qemu_chr_be_generic_open
  mux: simplfy muxes_realize_done
  xen: use a better chardev type check
  container: don't leak container reference
  char: add a /chardevs container
  vl: add todo note about root container cleanup
  char: use /chardevs container instead of chardevs list
  char: remove qemu_chardev_add
  char: remove chardevs list
  char: useless NULL check
  char-socket: introduce update_disconnected_filename()
  char-socket: update local address after listen
  char-socket: add 'addr' property
  char-socket: add 'connected' property
  char-udp: flush as much buffer as possible
  tests: add alias check in /char/ringbuf
  tests: add /char/pipe test
  tests: add /char/file test
  tests: add /char/socket test
  tests: add /char/udp test
  tests: add /char/console test

 chardev/char-mux.h  |   2 +-
 include/sysemu/char.h   |  10 --
 chardev/char-mux.c  |  11 +-
 chardev/char-pty.c  |   2 +-
 chardev/char-socket.c   |  46 +-
 chardev/char-udp.c  |  26 ++--
 chardev/char.c  | 148 --
 gdbstub.c   |   4 +-
 hw/usb/ccid-card-passthru.c |   2 +-
 hw/usb/redirect.c   |   2 +-
 net/vhost-user.c|   2 +-
 qom/container.c |   1 +
 tests/test-char.c   | 366 +++-
 tests/vhost-user-test.c |   2 +-
 ui/console.c|   2 +-
 ui/gtk.c|   2 +-
 vl.c|   1 +
 xen-common.c|   2 +-
 18 files changed, 504 insertions(+), 127 deletions(-)

-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 09/21] char: remove chardevs list

2017-03-16 Thread Marc-André Lureau
The list is now empty, the chardev cleanup is taken care of by the unref
of the root container.

Signed-off-by: Marc-André Lureau 
---
 include/sysemu/char.h | 1 -
 chardev/char.c| 6 --
 2 files changed, 7 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 98903f31e4..729d186d01 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -95,7 +95,6 @@ struct Chardev {
 int be_open;
 guint fd_in_tag;
 DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
-QTAILQ_ENTRY(Chardev) next;
 };
 
 /**
diff --git a/chardev/char.c b/chardev/char.c
index 276d27f403..aee8b4555d 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -42,9 +42,6 @@
 /***/
 /* character device */
 
-static QTAILQ_HEAD(ChardevHead, Chardev) chardevs =
-QTAILQ_HEAD_INITIALIZER(chardevs);
-
 static Object *get_chardevs_root(void)
 {
 return container_get(object_get_root(), "/chardevs");
@@ -418,9 +415,6 @@ static void char_finalize(Object *obj)
 {
 Chardev *chr = CHARDEV(obj);
 
-if (QTAILQ_IN_USE(chr, next)) {
-QTAILQ_REMOVE(&chardevs, chr, next);
-}
 if (chr->be) {
 chr->be->chr = NULL;
 }
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 04/21] container: don't leak container reference

2017-03-16 Thread Marc-André Lureau
object_property_add_child() references the child, unref it after to
avoid ref leaks.

Signed-off-by: Marc-André Lureau 
---
 qom/container.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qom/container.c b/qom/container.c
index c9eb49b01e..f6ccaf7ea7 100644
--- a/qom/container.c
+++ b/qom/container.c
@@ -40,6 +40,7 @@ Object *container_get(Object *root, const char *path)
 if (!child) {
 child = object_new("container");
 object_property_add_child(obj, parts[i], child, NULL);
+object_unref(child);
 }
 }
 
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 15/21] char-udp: flush as much buffer as possible

2017-03-16 Thread Marc-André Lureau
Instead of flushing the buffer byte by byte, call qemu_chr_be_write()
with as much byte possible accepted by the front-end.

Factor out buffer flushing in a common function udp_chr_flush_buffer().

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
---
 chardev/char-udp.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/chardev/char-udp.c b/chardev/char-udp.c
index 804bd22efa..1958c36de4 100644
--- a/chardev/char-udp.c
+++ b/chardev/char-udp.c
@@ -51,6 +51,18 @@ static int udp_chr_write(Chardev *chr, const uint8_t *buf, 
int len)
 s->ioc, (const char *)buf, len, NULL);
 }
 
+static void udp_chr_flush_buffer(UdpChardev *s)
+{
+Chardev *chr = CHARDEV(s);
+
+while (s->max_size > 0 && s->bufptr < s->bufcnt) {
+int n = MIN(s->max_size, s->bufcnt - s->bufptr);
+qemu_chr_be_write(chr, &s->buf[s->bufptr], n);
+s->bufptr += n;
+s->max_size = qemu_chr_be_can_write(chr);
+}
+}
+
 static int udp_chr_read_poll(void *opaque)
 {
 Chardev *chr = CHARDEV(opaque);
@@ -61,11 +73,8 @@ static int udp_chr_read_poll(void *opaque)
 /* If there were any stray characters in the queue process them
  * first
  */
-while (s->max_size > 0 && s->bufptr < s->bufcnt) {
-qemu_chr_be_write(chr, &s->buf[s->bufptr], 1);
-s->bufptr++;
-s->max_size = qemu_chr_be_can_write(chr);
-}
+udp_chr_flush_buffer(s);
+
 return s->max_size;
 }
 
@@ -85,13 +94,8 @@ static gboolean udp_chr_read(QIOChannel *chan, GIOCondition 
cond, void *opaque)
 return FALSE;
 }
 s->bufcnt = ret;
-
 s->bufptr = 0;
-while (s->max_size > 0 && s->bufptr < s->bufcnt) {
-qemu_chr_be_write(chr, &s->buf[s->bufptr], 1);
-s->bufptr++;
-s->max_size = qemu_chr_be_can_write(chr);
-}
+udp_chr_flush_buffer(s);
 
 return TRUE;
 }
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 06/21] vl: add todo note about root container cleanup

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 vl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/vl.c b/vl.c
index 0b4ed5241c..5440e1eb47 100644
--- a/vl.c
+++ b/vl.c
@@ -4719,6 +4719,7 @@ int main(int argc, char **argv, char **envp)
 audio_cleanup();
 monitor_cleanup();
 qemu_chr_cleanup();
+/* TODO: unref root container, check all devices are ok */
 
 return 0;
 }
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 05/21] char: add a /chardevs container

2017-03-16 Thread Marc-André Lureau
Add a /chardevs container object to hold the list of chardevs.
(Note: QTAILQ chardevs is going away in the following commits)

Signed-off-by: Marc-André Lureau 
---
 include/sysemu/char.h   |  8 
 chardev/char.c  | 50 +
 gdbstub.c   |  4 ++--
 hw/usb/ccid-card-passthru.c |  2 +-
 hw/usb/redirect.c   |  2 +-
 net/vhost-user.c|  2 +-
 tests/test-char.c   |  8 
 tests/vhost-user-test.c |  2 +-
 8 files changed, 42 insertions(+), 36 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index a30ff3fa80..98903f31e4 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -171,14 +171,6 @@ int qemu_chr_fe_wait_connected(CharBackend *be, Error 
**errp);
 Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
 
 /**
- * @qemu_chr_delete:
- *
- * Destroy a character backend and remove it from the list of
- * identified character backends.
- */
-void qemu_chr_delete(Chardev *chr);
-
-/**
  * @qemu_chr_fe_set_echo:
  *
  * Ask the backend to override its normal echo setting.  This only really
diff --git a/chardev/char.c b/chardev/char.c
index 674c097fbe..5456fb2e3a 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -45,6 +45,11 @@
 static QTAILQ_HEAD(ChardevHead, Chardev) chardevs =
 QTAILQ_HEAD_INITIALIZER(chardevs);
 
+static Object *get_chardevs_root(void)
+{
+return container_get(object_get_root(), "/chardevs");
+}
+
 void qemu_chr_be_event(Chardev *s, int event)
 {
 CharBackend *be = s->be;
@@ -413,6 +418,9 @@ static void char_finalize(Object *obj)
 {
 Chardev *chr = CHARDEV(obj);
 
+if (QTAILQ_IN_USE(chr, next)) {
+QTAILQ_REMOVE(&chardevs, chr, next);
+}
 if (chr->be) {
 chr->be->chr = NULL;
 }
@@ -946,7 +954,7 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
 backend->u.mux.data->chardev = g_strdup(bid);
 mux = qemu_chardev_add(id, TYPE_CHARDEV_MUX, backend, errp);
 if (mux == NULL) {
-qemu_chr_delete(chr);
+object_unparent(OBJECT(chr));
 chr = NULL;
 goto out;
 }
@@ -1060,12 +1068,6 @@ void qemu_chr_fe_disconnect(CharBackend *be)
 }
 }
 
-void qemu_chr_delete(Chardev *chr)
-{
-QTAILQ_REMOVE(&chardevs, chr, next);
-object_unref(OBJECT(chr));
-}
-
 ChardevInfoList *qmp_query_chardev(Error **errp)
 {
 ChardevInfoList *chr_list = NULL;
@@ -1225,22 +1227,23 @@ void qemu_chr_set_feature(Chardev *chr,
 }
 
 Chardev *qemu_chardev_new(const char *id, const char *typename,
-  ChardevBackend *backend, Error **errp)
+  ChardevBackend *backend,
+  Error **errp)
 {
+Object *obj;
 Chardev *chr = NULL;
 Error *local_err = NULL;
 bool be_opened = true;
 
 assert(g_str_has_prefix(typename, "chardev-"));
 
-chr = CHARDEV(object_new(typename));
+obj = object_new(typename);
+chr = CHARDEV(obj);
 chr->label = g_strdup(id);
 
 qemu_char_open(chr, backend, &be_opened, &local_err);
 if (local_err) {
-error_propagate(errp, local_err);
-object_unref(OBJECT(chr));
-return NULL;
+goto end;
 }
 
 if (!chr->filename) {
@@ -1250,6 +1253,21 @@ Chardev *qemu_chardev_new(const char *id, const char 
*typename,
 qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 }
 
+if (id) {
+object_property_add_child(get_chardevs_root(), id, obj, &local_err);
+if (local_err) {
+goto end;
+}
+object_unref(obj);
+}
+
+end:
+if (local_err) {
+error_propagate(errp, local_err);
+object_unref(obj);
+return NULL;
+}
+
 return chr;
 }
 
@@ -1298,16 +1316,12 @@ void qmp_chardev_remove(const char *id, Error **errp)
 "Chardev '%s' cannot be unplugged in record/replay mode", id);
 return;
 }
-qemu_chr_delete(chr);
+object_unparent(OBJECT(chr));
 }
 
 void qemu_chr_cleanup(void)
 {
-Chardev *chr, *tmp;
-
-QTAILQ_FOREACH_SAFE(chr, &chardevs, next, tmp) {
-qemu_chr_delete(chr);
-}
+object_unparent(get_chardevs_root());
 }
 
 static void register_types(void)
diff --git a/gdbstub.c b/gdbstub.c
index 991115361e..07ebfe9626 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1611,7 +1611,7 @@ void gdb_exit(CPUArchState *env, int code)
 
 #ifndef CONFIG_USER_ONLY
   qemu_chr_fe_deinit(&s->chr);
-  qemu_chr_delete(chr);
+  object_unparent(OBJECT(chr));
 #endif
 }
 
@@ -1912,7 +1912,7 @@ int gdbserver_start(const char *device)
 monitor_init(mon_chr, 0);
 } else {
 if (qemu_chr_fe_get_driver(&s->chr)) {
-qemu_chr_delete(qemu_chr_fe_get_driver(&s->chr));
+object_unparent(OBJECT(qemu_chr_fe_get_driver(&s->chr)));
 }
 mon_chr = s->mon_chr;
 memset(s, 0, sizeof(GDBState));
diff --git a/hw/usb/ccid-card-passthru.c b/hw/

[Qemu-devel] [PATCH v3 03/21] xen: use a better chardev type check

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
---
 xen-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen-common.c b/xen-common.c
index fd2c92847e..d46685ef4e 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -34,7 +34,7 @@ static int store_dev_info(int domid, Chardev *cs, const char 
*string)
 int ret = -1;
 
 /* Only continue if we're talking to a pty. */
-if (strncmp(cs->filename, "pty:", 4)) {
+if (!CHARDEV_IS_PTY(cs)) {
 return 0;
 }
 pts = cs->filename + 4;
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 08/21] char: remove qemu_chardev_add

2017-03-16 Thread Marc-André Lureau
qemu_chardev_new() now uses object_new_with_props() with /chardevs
parent container. It will fail to insert the object if the same "id"
already exists. "chardevs" list usage has been removed in previous
commits.

Signed-off-by: Marc-André Lureau 
---
 chardev/char.c | 27 ---
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 1859680bea..276d27f403 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -805,26 +805,6 @@ static const ChardevClass *char_get_class(const char 
*driver, Error **errp)
 return cc;
 }
 
-static Chardev *qemu_chardev_add(const char *id, const char *typename,
- ChardevBackend *backend, Error **errp)
-{
-Chardev *chr;
-
-chr = qemu_chr_find(id);
-if (chr) {
-error_setg(errp, "Chardev '%s' already exists", id);
-return NULL;
-}
-
-chr = qemu_chardev_new(id, typename, backend, errp);
-if (!chr) {
-return NULL;
-}
-
-QTAILQ_INSERT_TAIL(&chardevs, chr, next);
-return chr;
-}
-
 static const struct ChardevAlias {
 const char *typename;
 const char *alias;
@@ -941,9 +921,10 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
 backend->u.null.data = ccom; /* Any ChardevCommon member would work */
 }
 
-chr = qemu_chardev_add(bid ? bid : id,
+chr = qemu_chardev_new(bid ? bid : id,
object_class_get_name(OBJECT_CLASS(cc)),
backend, errp);
+
 if (chr == NULL) {
 goto out;
 }
@@ -955,7 +936,7 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
 backend->type = CHARDEV_BACKEND_KIND_MUX;
 backend->u.mux.data = g_new0(ChardevMux, 1);
 backend->u.mux.data->chardev = g_strdup(bid);
-mux = qemu_chardev_add(id, TYPE_CHARDEV_MUX, backend, errp);
+mux = qemu_chardev_new(id, TYPE_CHARDEV_MUX, backend, errp);
 if (mux == NULL) {
 object_unparent(OBJECT(chr));
 chr = NULL;
@@ -1289,7 +1270,7 @@ ChardevReturn *qmp_chardev_add(const char *id, 
ChardevBackend *backend,
 return NULL;
 }
 
-chr = qemu_chardev_add(id, object_class_get_name(OBJECT_CLASS(cc)),
+chr = qemu_chardev_new(id, object_class_get_name(OBJECT_CLASS(cc)),
backend, errp);
 if (!chr) {
 return NULL;
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 07/21] char: use /chardevs container instead of chardevs list

2017-03-16 Thread Marc-André Lureau
Use object_resolve_path_component() and object_child_foreach() on
/chardevs container instead of iterating over chardevs list.

Signed-off-by: Marc-André Lureau 
---
 chardev/char.c | 66 --
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 5456fb2e3a..1859680bea 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -455,21 +455,24 @@ static const TypeInfo char_type_info = {
  * mux will receive CHR_EVENT_OPENED notifications for the BE
  * immediately.
  */
-static void muxes_realize_done(Notifier *notifier, void *unused)
+static int open_muxes(Object *child, void *opaque)
 {
-Chardev *chr;
+if (CHARDEV_IS_MUX(child)) {
+/* send OPENED to all already-attached FEs */
+mux_chr_send_all_event(CHARDEV(child), CHR_EVENT_OPENED);
+/* mark mux as OPENED so any new FEs will immediately receive
+ * OPENED event
+ */
+qemu_chr_be_event(CHARDEV(child), CHR_EVENT_OPENED);
+}
 
+return 0;
+}
+
+static void muxes_realize_done(Notifier *notifier, void *unused)
+{
 muxes_realized = true;
-QTAILQ_FOREACH(chr, &chardevs, next) {
-if (CHARDEV_IS_MUX(chr)) {
-/* send OPENED to all already-attached FEs */
-mux_chr_send_all_event(CHARDEV(chr), CHR_EVENT_OPENED);
-/* mark mux as OPENED so any new FEs will immediately receive
- * OPENED event
- */
-qemu_chr_be_event(chr, CHR_EVENT_OPENED);
-}
-}
+object_child_foreach(get_chardevs_root(), open_muxes, NULL);
 }
 
 static Notifier muxes_realize_notify = {
@@ -1068,21 +1071,29 @@ void qemu_chr_fe_disconnect(CharBackend *be)
 }
 }
 
+static int qmp_query_chardev_foreach(Object *obj, void *data)
+{
+Chardev *chr = CHARDEV(obj);
+ChardevInfoList **list = data;
+ChardevInfoList *info = g_malloc0(sizeof(*info));
+
+info->value = g_malloc0(sizeof(*info->value));
+info->value->label = g_strdup(chr->label);
+info->value->filename = g_strdup(chr->filename);
+info->value->frontend_open = chr->be && chr->be->fe_open;
+
+info->next = *list;
+*list = info;
+
+return 0;
+}
+
 ChardevInfoList *qmp_query_chardev(Error **errp)
 {
 ChardevInfoList *chr_list = NULL;
-Chardev *chr;
-
-QTAILQ_FOREACH(chr, &chardevs, next) {
-ChardevInfoList *info = g_malloc0(sizeof(*info));
-info->value = g_malloc0(sizeof(*info->value));
-info->value->label = g_strdup(chr->label);
-info->value->filename = g_strdup(chr->filename);
-info->value->frontend_open = chr->be && chr->be->fe_open;
 
-info->next = chr_list;
-chr_list = info;
-}
+object_child_foreach(get_chardevs_root(),
+ qmp_query_chardev_foreach, &chr_list);
 
 return chr_list;
 }
@@ -1110,14 +1121,9 @@ ChardevBackendInfoList *qmp_query_chardev_backends(Error 
**errp)
 
 Chardev *qemu_chr_find(const char *name)
 {
-Chardev *chr;
+Object *obj = object_resolve_path_component(get_chardevs_root(), name);
 
-QTAILQ_FOREACH(chr, &chardevs, next) {
-if (strcmp(chr->label, name) != 0)
-continue;
-return chr;
-}
-return NULL;
+return obj ? CHARDEV(obj) : NULL;
 }
 
 QemuOptsList qemu_chardev_opts = {
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 13/21] char-socket: add 'addr' property

2017-03-16 Thread Marc-André Lureau
Add a property to lookup the connection details.

Signed-off-by: Marc-André Lureau 
---
 chardev/char-socket.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 4325a05387..81021c5863 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1006,6 +1006,15 @@ static void qemu_chr_parse_socket(QemuOpts *opts, 
ChardevBackend *backend,
 sock->addr = addr;
 }
 
+static void
+char_socket_get_addr(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+SocketChardev *s = SOCKET_CHARDEV(obj);
+
+visit_type_SocketAddress(v, name, &s->addr, errp);
+}
+
 static void char_socket_class_init(ObjectClass *oc, void *data)
 {
 ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -1021,6 +1030,10 @@ static void char_socket_class_init(ObjectClass *oc, void 
*data)
 cc->chr_add_client = tcp_chr_add_client;
 cc->chr_add_watch = tcp_chr_add_watch;
 cc->chr_update_read_handler = tcp_chr_update_read_handler;
+
+object_class_property_add(oc, "addr", "SocketAddress",
+  char_socket_get_addr, NULL,
+  NULL, NULL, &error_abort);
 }
 
 static const TypeInfo char_socket_type_info = {
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 10/21] char: useless NULL check

2017-03-16 Thread Marc-André Lureau
g_strdup(NULL) returns NULL already.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
---
 chardev/char.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char.c b/chardev/char.c
index aee8b4555d..33dbab6981 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -764,7 +764,7 @@ void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon 
*backend)
 const char *logfile = qemu_opt_get(opts, "logfile");
 
 backend->has_logfile = logfile != NULL;
-backend->logfile = logfile ? g_strdup(logfile) : NULL;
+backend->logfile = g_strdup(logfile);
 
 backend->has_logappend = true;
 backend->logappend = qemu_opt_get_bool(opts, "logappend", false);
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 11/21] char-socket: introduce update_disconnected_filename()

2017-03-16 Thread Marc-André Lureau
This helper will be used in yet another place in the following patch.

Signed-off-by: Marc-André Lureau 
---
 chardev/char-socket.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index dc3d3532a7..e3b5288af7 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -363,6 +363,15 @@ static char *SocketAddress_to_str(const char *prefix, 
SocketAddress *addr,
 }
 }
 
+static void update_disconnected_filename(SocketChardev *s)
+{
+Chardev *chr = CHARDEV(s);
+
+g_free(chr->filename);
+chr->filename = SocketAddress_to_str("disconnected:", s->addr,
+ s->is_listen, s->is_telnet);
+}
+
 static void tcp_chr_disconnect(Chardev *chr)
 {
 SocketChardev *s = SOCKET_CHARDEV(chr);
@@ -377,8 +386,7 @@ static void tcp_chr_disconnect(Chardev *chr)
 s->listen_tag = qio_channel_add_watch(
 QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
 }
-chr->filename = SocketAddress_to_str("disconnected:", s->addr,
- s->is_listen, s->is_telnet);
+update_disconnected_filename(s);
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 if (s->reconnect_time) {
 qemu_chr_socket_restart_timer(chr);
@@ -872,8 +880,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
 /* be isn't opened until we get a connection */
 *be_opened = false;
 
-chr->filename = SocketAddress_to_str("disconnected:",
- addr, is_listen, is_telnet);
+update_disconnected_filename(s);
 
 if (is_listen) {
 if (is_telnet) {
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 14/21] char-socket: add 'connected' property

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 chardev/char-socket.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 81021c5863..06389393fa 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1015,6 +1015,14 @@ char_socket_get_addr(Object *obj, Visitor *v, const char 
*name,
 visit_type_SocketAddress(v, name, &s->addr, errp);
 }
 
+static bool
+char_socket_get_connected(Object *obj, Error **errp)
+{
+SocketChardev *s = SOCKET_CHARDEV(obj);
+
+return s->connected;
+}
+
 static void char_socket_class_init(ObjectClass *oc, void *data)
 {
 ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -1034,6 +1042,9 @@ static void char_socket_class_init(ObjectClass *oc, void 
*data)
 object_class_property_add(oc, "addr", "SocketAddress",
   char_socket_get_addr, NULL,
   NULL, NULL, &error_abort);
+
+object_class_property_add_bool(oc, "connected", char_socket_get_connected,
+   NULL, &error_abort);
 }
 
 static const TypeInfo char_socket_type_info = {
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PULL for-2.9 5/7] cirrus: fix cirrus_invalidate_region

2017-03-16 Thread Gerd Hoffmann
off_cur_end is exclusive, so off_cur_end == cirrus_addr_mask is valid.
Fix calculation to make sure to allow that, otherwise the assert added
by commit f153b563f8cf121aebf5a2fff5f0110faf58ccb3 can trigger for valid
blits.

Test case: boot windows nt 4.0

Signed-off-by: Gerd Hoffmann 
Message-id: 1489579606-26020-1-git-send-email-kra...@redhat.com
---
 hw/display/cirrus_vga.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 326d511..a9f6d5b 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -667,11 +667,11 @@ static void cirrus_invalidate_region(CirrusVGAState * s, 
int off_begin,
 }
 
 for (y = 0; y < lines; y++) {
-   off_cur = off_begin;
-   off_cur_end = (off_cur + bytesperline) & s->cirrus_addr_mask;
+off_cur = off_begin;
+off_cur_end = ((off_cur + bytesperline - 1) & s->cirrus_addr_mask) + 1;
 assert(off_cur_end >= off_cur);
 memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - off_cur);
-   off_begin += off_pitch;
+off_begin += off_pitch;
 }
 }
 
-- 
1.8.3.1




[Qemu-devel] [PATCH v3 12/21] char-socket: update local address after listen

2017-03-16 Thread Marc-André Lureau
This is mainly useful to know the actual bound port when using port 0.

For example, when starting qemu with socket on port 0, before:
QEMU waiting for connection on: disconnected:tcp:localhost:0,server
After:
QEMU waiting for connection on: disconnected:tcp:localhost:32454,server

Signed-off-by: Marc-André Lureau 
---
 chardev/char-socket.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index e3b5288af7..4325a05387 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -908,6 +908,11 @@ static void qmp_chardev_open_socket(Chardev *chr,
 if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) {
 goto error;
 }
+
+qapi_free_SocketAddress(s->addr);
+s->addr = socket_local_address(sioc->fd, errp);
+update_disconnected_filename(s);
+
 s->listen_ioc = sioc;
 if (is_waitconnect &&
 qemu_chr_wait_connected(chr, errp) < 0) {
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 18/21] tests: add /char/file test

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 tests/test-char.c | 71 +++
 1 file changed, 71 insertions(+)

diff --git a/tests/test-char.c b/tests/test-char.c
index 2b155ffcb7..87a4e2986d 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -277,6 +277,76 @@ static void char_pipe_test(void)
 }
 #endif
 
+static void char_file_test(void)
+{
+char *tmp_path = g_dir_make_tmp("qemu-test-char.XX", NULL);
+char *out = g_build_filename(tmp_path, "out", NULL);
+char *contents = NULL;
+ChardevFile file = { .out = out };
+ChardevBackend backend = { .type = CHARDEV_BACKEND_KIND_FILE,
+   .u.file.data = &file };
+Chardev *chr;
+gsize length;
+int ret;
+
+chr = qemu_chardev_new(NULL, TYPE_CHARDEV_FILE, &backend,
+   &error_abort);
+ret = qemu_chr_write_all(chr, (uint8_t *)"hello!", 6);
+g_assert_cmpint(ret, ==, 6);
+object_unref(OBJECT(chr));
+
+ret = g_file_get_contents(out, &contents, &length, NULL);
+g_assert(ret == TRUE);
+g_assert_cmpint(length, ==, 6);
+g_assert(strncmp(contents, "hello!", 6) == 0);
+g_free(contents);
+
+#ifndef _WIN32
+{
+CharBackend be;
+FeHandler fe = { 0, };
+char *fifo = g_build_filename(tmp_path, "fifo", NULL);
+int fd;
+
+if (mkfifo(fifo, 0600) < 0) {
+abort();
+}
+
+fd = open(fifo, O_RDWR);
+ret = write(fd, "fifo-in", 8);
+g_assert_cmpint(ret, ==, 8);
+
+file.in = fifo;
+file.has_in = true;
+chr = qemu_chardev_new(NULL, TYPE_CHARDEV_FILE, &backend,
+   &error_abort);
+
+qemu_chr_fe_init(&be, chr, &error_abort);
+qemu_chr_fe_set_handlers(&be,
+ fe_can_read,
+ fe_read,
+ fe_event,
+ &fe, NULL, true);
+
+main_loop();
+
+close(fd);
+
+g_assert_cmpint(fe.read_count, ==, 8);
+g_assert_cmpstr(fe.read_buf, ==, "fifo-in");
+qemu_chr_fe_deinit(&be);
+object_unref(OBJECT(chr));
+g_unlink(fifo);
+g_free(fifo);
+}
+#endif
+
+g_unlink(out);
+g_rmdir(tmp_path);
+g_free(tmp_path);
+g_free(out);
+}
+
 static void char_null_test(void)
 {
 Error *err = NULL;
@@ -348,6 +418,7 @@ int main(int argc, char **argv)
 #ifndef _WIN32
 g_test_add_func("/char/pipe", char_pipe_test);
 #endif
+g_test_add_func("/char/file", char_file_test);
 
 return g_test_run();
 }
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 16/21] tests: add alias check in /char/ringbuf

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 tests/test-char.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/test-char.c b/tests/test-char.c
index 71de4b35ee..2811644bcd 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -104,6 +104,16 @@ static void char_ringbuf_test(void)
 
 qemu_chr_fe_deinit(&be);
 object_unparent(OBJECT(chr));
+
+/* check alias */
+opts = qemu_opts_create(qemu_find_opts("chardev"), "memory-label",
+1, &error_abort);
+qemu_opt_set(opts, "backend", "memory", &error_abort);
+qemu_opt_set(opts, "size", "2", &error_abort);
+chr = qemu_chr_new_from_opts(opts, NULL);
+g_assert_nonnull(chr);
+object_unparent(OBJECT(chr));
+qemu_opts_del(opts);
 }
 
 static void char_mux_test(void)
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PULL for-2.9 2/7] cirrus/vnc: zap bitblit support from console code.

2017-03-16 Thread Gerd Hoffmann
There is a special code path (dpy_gfx_copy) to allow graphic emulation
notify user interface code about bitblit operations carryed out by
guests.  It is supported by cirrus and vnc server.  The intended purpose
is to optimize display scrolls and just send over the scroll op instead
of a full display update.

This is rarely used these days though because modern guests simply don't
use the cirrus blitter any more.  Any linux guest using the cirrus drm
driver doesn't.  Any windows guest newer than winxp doesn't ship with a
cirrus driver any more and thus uses the cirrus as simple framebuffer.

So this code tends to bitrot and bugs can go unnoticed for a long time.
See for example commit "3e10c3e vnc: fix qemu crash because of SIGSEGV"
which fixes a bug lingering in the code for almost a year, added by
commit "c7628bf vnc: only alloc server surface with clients connected".

Also the vnc server will throttle the frame rate in case it figures the
network can't keep up (send buffers are full).  This doesn't work with
dpy_gfx_copy, for any copy operation sent to the vnc client we have to
send all outstanding updates beforehand, otherwise the vnc client might
run the client side blit on outdated data and thereby corrupt the
display.  So this dpy_gfx_copy "optimization" might even make things
worse on slow network links.

Lets kill it once for all.

Oh, and one more reason: Turns out (after writing the patch) we have a
security bug in that code path ...

Fixes: CVE-2016-9603
Signed-off-by: Gerd Hoffmann 
Message-id: 1489494419-14340-1-git-send-email-kra...@redhat.com
---
 hw/display/cirrus_vga.c |  12 ++
 include/ui/console.h|   7 
 ui/console.c|  28 --
 ui/vnc.c| 100 
 4 files changed, 3 insertions(+), 144 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index b9e7cb1..c90a4a3 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -796,21 +796,15 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int 
src, int w, int h)
 }
 }
 
-/* we have to flush all pending changes so that the copy
-   is generated at the appropriate moment in time */
-if (notify)
-graphic_hw_update(s->vga.con);
-
 (*s->cirrus_rop) (s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
   s->vga.vram_ptr + s->cirrus_blt_srcaddr,
  s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
  s->cirrus_blt_width, s->cirrus_blt_height);
 
 if (notify) {
-qemu_console_copy(s->vga.con,
- sx, sy, dx, dy,
- s->cirrus_blt_width / depth,
- s->cirrus_blt_height);
+dpy_gfx_update(s->vga.con, dx, dy,
+   s->cirrus_blt_width / depth,
+   s->cirrus_blt_height);
 }
 
 /* we don't have to notify the display that this portion has
diff --git a/include/ui/console.h b/include/ui/console.h
index ac2895c..d759338 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -189,9 +189,6 @@ typedef struct DisplayChangeListenerOps {
int x, int y, int w, int h);
 void (*dpy_gfx_switch)(DisplayChangeListener *dcl,
struct DisplaySurface *new_surface);
-void (*dpy_gfx_copy)(DisplayChangeListener *dcl,
- int src_x, int src_y,
- int dst_x, int dst_y, int w, int h);
 bool (*dpy_gfx_check_format)(DisplayChangeListener *dcl,
  pixman_format_code_t format);
 
@@ -277,8 +274,6 @@ int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info);
 void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h);
 void dpy_gfx_replace_surface(QemuConsole *con,
  DisplaySurface *surface);
-void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y,
-  int dst_x, int dst_y, int w, int h);
 void dpy_text_cursor(QemuConsole *con, int x, int y);
 void dpy_text_update(QemuConsole *con, int x, int y, int w, int h);
 void dpy_text_resize(QemuConsole *con, int w, int h);
@@ -411,8 +406,6 @@ void qemu_console_set_window_id(QemuConsole *con, int 
window_id);
 
 void console_select(unsigned int index);
 void qemu_console_resize(QemuConsole *con, int width, int height);
-void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
-   int dst_x, int dst_y, int w, int h);
 DisplaySurface *qemu_console_surface(QemuConsole *con);
 
 /* console-gl.c */
diff --git a/ui/console.c b/ui/console.c
index d1ff750..4c70d8b 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1586,27 +1586,6 @@ static void dpy_refresh(DisplayState *s)
 }
 }
 
-void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y,
-  int dst_x, int dst_y, int w, int h)
-{
-DisplayState *s = con->ds;
-DisplayChangeListener *dcl;
-
-if (!qemu_console_is

[Qemu-devel] [PATCH v3 19/21] tests: add /char/socket test

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 tests/test-char.c | 114 +-
 1 file changed, 112 insertions(+), 2 deletions(-)

diff --git a/tests/test-char.c b/tests/test-char.c
index 87a4e2986d..9971498391 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -3,9 +3,11 @@
 
 #include "qemu-common.h"
 #include "qemu/config-file.h"
+#include "qemu/sockets.h"
 #include "sysemu/char.h"
 #include "sysemu/sysemu.h"
 #include "qapi/error.h"
+#include "qom/qom-qobject.h"
 #include "qmp-commands.h"
 
 static bool quit;
@@ -16,7 +18,6 @@ typedef struct FeHandler {
 char read_buf[128];
 } FeHandler;
 
-#ifndef _WIN32
 static void main_loop(void)
 {
 bool nonblocking;
@@ -28,7 +29,6 @@ static void main_loop(void)
 last_io = main_loop_wait(nonblocking);
 } while (!quit);
 }
-#endif
 
 static int fe_can_read(void *opaque)
 {
@@ -211,6 +211,114 @@ static void char_mux_test(void)
 object_unparent(OBJECT(chr));
 }
 
+typedef struct SocketIdleData {
+GMainLoop *loop;
+Chardev *chr;
+bool conn_expected;
+CharBackend *be;
+CharBackend *client_be;
+} SocketIdleData;
+
+static gboolean char_socket_test_idle(gpointer user_data)
+{
+SocketIdleData *data = user_data;
+
+if (object_property_get_bool(OBJECT(data->chr), "connected", NULL)
+== data->conn_expected) {
+quit = true;
+return FALSE;
+}
+
+return TRUE;
+}
+
+static void socket_read(void *opaque, const uint8_t *buf, int size)
+{
+SocketIdleData *data = opaque;
+
+g_assert_cmpint(size, ==, 1);
+g_assert_cmpint(*buf, ==, 'Z');
+
+size = qemu_chr_fe_write(data->be, (const uint8_t *)"hello", 5);
+g_assert_cmpint(size, ==, 5);
+}
+
+static int socket_can_read(void *opaque)
+{
+return 10;
+}
+
+static void socket_read_hello(void *opaque, const uint8_t *buf, int size)
+{
+g_assert_cmpint(size, ==, 5);
+g_assert(strncmp((char *)buf, "hello", 5) == 0);
+
+quit = true;
+}
+
+static int socket_can_read_hello(void *opaque)
+{
+return 10;
+}
+
+static void char_socket_test(void)
+{
+Chardev *chr = qemu_chr_new("server", "tcp:127.0.0.1:0,server,nowait");
+Chardev *chr_client;
+QObject *addr;
+QDict *qdict, *data;
+const char *port;
+SocketIdleData d = { .chr = chr };
+CharBackend be;
+CharBackend client_be;
+char *tmp;
+
+d.be = &be;
+d.client_be = &be;
+
+g_assert_nonnull(chr);
+g_assert(!object_property_get_bool(OBJECT(chr), "connected", 
&error_abort));
+
+addr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
+qdict = qobject_to_qdict(addr);
+data = qdict_get_qdict(qdict, "data");
+port = qdict_get_str(data, "port");
+tmp = g_strdup_printf("tcp:127.0.0.1:%s", port);
+QDECREF(qdict);
+
+qemu_chr_fe_init(&be, chr, &error_abort);
+qemu_chr_fe_set_handlers(&be, socket_can_read, socket_read,
+ NULL, &d, NULL, true);
+
+chr_client = qemu_chr_new("client", tmp);
+qemu_chr_fe_init(&client_be, chr_client, &error_abort);
+qemu_chr_fe_set_handlers(&client_be, socket_can_read_hello,
+ socket_read_hello,
+ NULL, &d, NULL, true);
+g_free(tmp);
+
+d.conn_expected = true;
+guint id = g_idle_add(char_socket_test_idle, &d);
+g_source_set_name_by_id(id, "test-idle");
+g_assert_cmpint(id, >, 0);
+main_loop();
+
+g_assert(object_property_get_bool(OBJECT(chr), "connected", &error_abort));
+g_assert(object_property_get_bool(OBJECT(chr_client),
+  "connected", &error_abort));
+
+qemu_chr_write_all(chr_client, (const uint8_t *)"Z", 1);
+main_loop();
+
+object_unparent(OBJECT(chr_client));
+
+d.conn_expected = false;
+g_idle_add(char_socket_test_idle, &d);
+main_loop();
+
+object_unparent(OBJECT(chr));
+}
+
 #ifndef _WIN32
 static void char_pipe_test(void)
 {
@@ -401,6 +509,7 @@ static void char_invalid_test(void)
 int main(int argc, char **argv)
 {
 qemu_init_main_loop(&error_abort);
+socket_init();
 
 g_test_init(&argc, &argv, NULL);
 
@@ -419,6 +528,7 @@ int main(int argc, char **argv)
 g_test_add_func("/char/pipe", char_pipe_test);
 #endif
 g_test_add_func("/char/file", char_file_test);
+g_test_add_func("/char/socket", char_socket_test);
 
 return g_test_run();
 }
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 17/21] tests: add /char/pipe test

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 tests/test-char.c | 90 +++
 1 file changed, 90 insertions(+)

diff --git a/tests/test-char.c b/tests/test-char.c
index 2811644bcd..2b155ffcb7 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -1,4 +1,5 @@
 #include "qemu/osdep.h"
+#include 
 
 #include "qemu-common.h"
 #include "qemu/config-file.h"
@@ -7,12 +8,28 @@
 #include "qapi/error.h"
 #include "qmp-commands.h"
 
+static bool quit;
+
 typedef struct FeHandler {
 int read_count;
 int last_event;
 char read_buf[128];
 } FeHandler;
 
+#ifndef _WIN32
+static void main_loop(void)
+{
+bool nonblocking;
+int last_io = 0;
+
+quit = false;
+do {
+nonblocking = last_io > 0;
+last_io = main_loop_wait(nonblocking);
+} while (!quit);
+}
+#endif
+
 static int fe_can_read(void *opaque)
 {
 FeHandler *h = opaque;
@@ -28,6 +45,7 @@ static void fe_read(void *opaque, const uint8_t *buf, int 
size)
 
 memcpy(h->read_buf + h->read_count, buf, size);
 h->read_count += size;
+quit = true;
 }
 
 static void fe_event(void *opaque, int event)
@@ -35,6 +53,7 @@ static void fe_event(void *opaque, int event)
 FeHandler *h = opaque;
 
 h->last_event = event;
+quit = true;
 }
 
 #ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
@@ -192,6 +211,72 @@ static void char_mux_test(void)
 object_unparent(OBJECT(chr));
 }
 
+#ifndef _WIN32
+static void char_pipe_test(void)
+{
+gchar *tmp_path = g_dir_make_tmp("qemu-test-char.XX", NULL);
+gchar *tmp, *in, *out, *pipe = g_build_filename(tmp_path, "pipe", NULL);
+Chardev *chr;
+CharBackend be;
+int ret, fd;
+char buf[10];
+FeHandler fe = { 0, };
+
+in = g_strdup_printf("%s.in", pipe);
+if (mkfifo(in, 0600) < 0) {
+abort();
+}
+out = g_strdup_printf("%s.out", pipe);
+if (mkfifo(out, 0600) < 0) {
+abort();
+}
+
+tmp = g_strdup_printf("pipe:%s", pipe);
+chr = qemu_chr_new("pipe", tmp);
+g_assert_nonnull(chr);
+g_free(tmp);
+
+qemu_chr_fe_init(&be, chr, &error_abort);
+
+ret = qemu_chr_fe_write(&be, (void *)"pipe-out", 9);
+g_assert_cmpint(ret, ==, 9);
+
+fd = open(out, O_RDWR);
+ret = read(fd, buf, sizeof(buf));
+g_assert_cmpint(ret, ==, 9);
+g_assert_cmpstr(buf, ==, "pipe-out");
+close(fd);
+
+fd = open(in, O_WRONLY);
+ret = write(fd, "pipe-in", 8);
+g_assert_cmpint(ret, ==, 8);
+close(fd);
+
+qemu_chr_fe_set_handlers(&be,
+ fe_can_read,
+ fe_read,
+ fe_event,
+ &fe,
+ NULL, true);
+
+main_loop();
+
+g_assert_cmpint(fe.read_count, ==, 8);
+g_assert_cmpstr(fe.read_buf, ==, "pipe-in");
+
+qemu_chr_fe_deinit(&be);
+object_unparent(OBJECT(chr));
+
+g_assert(g_unlink(in) == 0);
+g_assert(g_unlink(out) == 0);
+g_assert(g_rmdir(tmp_path) == 0);
+g_free(in);
+g_free(out);
+g_free(tmp_path);
+g_free(pipe);
+}
+#endif
+
 static void char_null_test(void)
 {
 Error *err = NULL;
@@ -245,6 +330,8 @@ static void char_invalid_test(void)
 
 int main(int argc, char **argv)
 {
+qemu_init_main_loop(&error_abort);
+
 g_test_init(&argc, &argv, NULL);
 
 module_call_init(MODULE_INIT_QOM);
@@ -258,6 +345,9 @@ int main(int argc, char **argv)
 g_test_add_func("/char/stdio/subprocess", char_stdio_test_subprocess);
 g_test_add_func("/char/stdio", char_stdio_test);
 #endif
+#ifndef _WIN32
+g_test_add_func("/char/pipe", char_pipe_test);
+#endif
 
 return g_test_run();
 }
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 21/21] tests: add /char/console test

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 tests/test-char.c | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tests/test-char.c b/tests/test-char.c
index 19707bfcda..773a1c36ba 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -57,6 +57,32 @@ static void fe_event(void *opaque, int event)
 }
 
 #ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+#ifdef _WIN32
+static void char_console_test_subprocess(void)
+{
+QemuOpts *opts;
+Chardev *chr;
+
+opts = qemu_opts_create(qemu_find_opts("chardev"), "console-label",
+1, &error_abort);
+qemu_opt_set(opts, "backend", "console", &error_abort);
+
+chr = qemu_chr_new_from_opts(opts, NULL);
+g_assert_nonnull(chr);
+
+qemu_chr_write_all(chr, (const uint8_t *)"CONSOLE", 7);
+
+qemu_opts_del(opts);
+object_unparent(OBJECT(chr));
+}
+
+static void char_console_test(void)
+{
+g_test_trap_subprocess("/char/console/subprocess", 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stdout("CONSOLE");
+}
+#endif
 static void char_stdio_test_subprocess(void)
 {
 Chardev *chr;
@@ -83,7 +109,6 @@ static void char_stdio_test(void)
 }
 #endif
 
-
 static void char_ringbuf_test(void)
 {
 QemuOpts *opts;
@@ -566,6 +591,10 @@ int main(int argc, char **argv)
 g_test_add_func("/char/ringbuf", char_ringbuf_test);
 g_test_add_func("/char/mux", char_mux_test);
 #ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+#ifdef _WIN32
+g_test_add_func("/char/console/subprocess", char_console_test_subprocess);
+g_test_add_func("/char/console", char_console_test);
+#endif
 g_test_add_func("/char/stdio/subprocess", char_stdio_test_subprocess);
 g_test_add_func("/char/stdio", char_stdio_test);
 #endif
-- 
2.12.0.191.gc5d8de91d




[Qemu-devel] [PATCH v3 20/21] tests: add /char/udp test

2017-03-16 Thread Marc-André Lureau
Signed-off-by: Marc-André Lureau 
---
 tests/test-char.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/tests/test-char.c b/tests/test-char.c
index 9971498391..19707bfcda 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -385,6 +385,51 @@ static void char_pipe_test(void)
 }
 #endif
 
+static void char_udp_test(void)
+{
+struct sockaddr_in addr = { 0, }, other;
+SocketIdleData d = { 0, };
+Chardev *chr;
+CharBackend be;
+socklen_t alen = sizeof(addr);
+int ret, sock = qemu_socket(PF_INET, SOCK_DGRAM, 0);
+char buf[10];
+char *tmp;
+
+g_assert_cmpint(sock, >, 0);
+addr.sin_family = AF_INET ;
+addr.sin_addr.s_addr = htonl(INADDR_ANY);
+addr.sin_port = 0;
+ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr));
+g_assert_cmpint(ret, ==, 0);
+ret = getsockname(sock, (struct sockaddr *)&addr, &alen);
+g_assert_cmpint(ret, ==, 0);
+
+tmp = g_strdup_printf("udp:127.0.0.1:%d",
+  ntohs(addr.sin_port));
+chr = qemu_chr_new("client", tmp);
+g_assert_nonnull(chr);
+
+d.chr = chr;
+qemu_chr_fe_init(&be, chr, &error_abort);
+qemu_chr_fe_set_handlers(&be, socket_can_read_hello, socket_read_hello,
+ NULL, &d, NULL, true);
+ret = qemu_chr_write_all(chr, (uint8_t *)"hello", 5);
+g_assert_cmpint(ret, ==, 5);
+
+alen = sizeof(addr);
+ret = recvfrom(sock, buf, sizeof(buf), 0,
+   (struct sockaddr *)&other, &alen);
+g_assert_cmpint(ret, ==, 5);
+ret = sendto(sock, buf, 5, 0, (struct sockaddr *)&other, alen);
+g_assert_cmpint(ret, ==, 5);
+
+main_loop();
+
+close(sock);
+g_free(tmp);
+}
+
 static void char_file_test(void)
 {
 char *tmp_path = g_dir_make_tmp("qemu-test-char.XX", NULL);
@@ -529,6 +574,7 @@ int main(int argc, char **argv)
 #endif
 g_test_add_func("/char/file", char_file_test);
 g_test_add_func("/char/socket", char_socket_test);
+g_test_add_func("/char/udp", char_udp_test);
 
 return g_test_run();
 }
-- 
2.12.0.191.gc5d8de91d




Re: [Qemu-devel] [PATCH RFC v3 00/15] basic vfio-ccw infrastructure

2017-03-16 Thread Cornelia Huck
On Mon, 13 Mar 2017 15:16:52 +0800
Dong Jia Shi  wrote:

> Beside the fixes for former comments form you and Alex, I will also do
> the following stuff in the next version:
> 1. Remove the "RFC" tag from the mail subject.
> 2. Cc the s390 CIO layer maintainers for patch 1 and 2:
> Sebastian Ott 
> Peter Oberparleiter 
> 3. For the new created files, update MAINTAINERS by adding:
> S390 VFIO-CCW DRIVER
> M:Cornelia Huck 

Please add

M: Dong Jia Shi 

as well :)

> L:linux-s...@vger.kernel.org
> L:k...@vger.kernel.org
> S:Supported
> F:drivers/s390/cio/vfio_ccw*
> F:Documentation/s390/vfio-ccw.txt
> F:include/uapi/linux/vfio_ccw.h




[Qemu-devel] [PULL for-2.9 4/7] cirrus: add option to disable blitter

2017-03-16 Thread Gerd Hoffmann
Ok, we have this beast in the cirrus code which is not used at all by
modern guests, except when you try to find security holes in qemu.  So,
add an option to disable blitter altogether.  Guests released within
the last ten years should not show any rendering issues if you turn off
blitter support.

There are no known bugs in the cirrus blitter code.  But in the past we
hoped a few times already that we've finally nailed the last issue.  So
having some easy way to mitigate in case yet another blitter issue shows
up certainly makes me sleep a bit better at night.

For completeness:  The by far better way to mitigate is to switch away
from cirrus and use stdvga instead.  Or something more modern like
virtio-vga in case your guest has support for it.

Signed-off-by: Gerd Hoffmann 
Message-id: 1489494540-15745-1-git-send-email-kra...@redhat.com
---
 hw/display/cirrus_vga.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 6ffe64f..326d511 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -205,6 +205,7 @@ typedef struct CirrusVGAState {
 uint32_t cirrus_bank_base[2];
 uint32_t cirrus_bank_limit[2];
 uint8_t cirrus_hidden_palette[48];
+bool enable_blitter;
 int cirrus_blt_pixelwidth;
 int cirrus_blt_width;
 int cirrus_blt_height;
@@ -960,6 +961,10 @@ static void cirrus_bitblt_start(CirrusVGAState * s)
 {
 uint8_t blt_rop;
 
+if (!s->enable_blitter) {
+goto bitblt_ignore;
+}
+
 s->vga.gr[0x31] |= CIRRUS_BLT_BUSY;
 
 s->cirrus_blt_width = (s->vga.gr[0x20] | (s->vga.gr[0x21] << 8)) + 1;
@@ -3024,6 +3029,8 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, 
Error **errp)
 static Property isa_cirrus_vga_properties[] = {
 DEFINE_PROP_UINT32("vgamem_mb", struct ISACirrusVGAState,
cirrus_vga.vga.vram_size_mb, 4),
+DEFINE_PROP_BOOL("blitter", struct ISACirrusVGAState,
+   cirrus_vga.enable_blitter, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -3093,6 +3100,8 @@ static void pci_cirrus_vga_realize(PCIDevice *dev, Error 
**errp)
 static Property pci_vga_cirrus_properties[] = {
 DEFINE_PROP_UINT32("vgamem_mb", struct PCICirrusVGAState,
cirrus_vga.vga.vram_size_mb, 4),
+DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
+ cirrus_vga.enable_blitter, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.8.3.1




[Qemu-devel] [PULL for-2.9 1/7] fix :cirrus_vga fix OOB read case qemu Segmentation fault

2017-03-16 Thread Gerd Hoffmann
From: hangaohuai 

check the validity of parameters in cirrus_bitblt_rop_fwd_transp_xxx
and cirrus_bitblt_rop_fwd_xxx to avoid the OOB read which causes qemu 
Segmentation fault.

After the fix, we will touch the assert in
cirrus_invalidate_region:
assert(off_cur_end >= off_cur);

Signed-off-by: fangying 
Signed-off-by: hangaohuai 
Message-id: 20170314063919.16200-1-hangaoh...@huawei.com
Signed-off-by: Gerd Hoffmann 
---
 hw/display/cirrus_vga_rop.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/display/cirrus_vga_rop.h b/hw/display/cirrus_vga_rop.h
index 0925a00..b7447f8 100644
--- a/hw/display/cirrus_vga_rop.h
+++ b/hw/display/cirrus_vga_rop.h
@@ -97,6 +97,11 @@ glue(glue(cirrus_bitblt_rop_fwd_transp_, 
ROP_NAME),_8)(CirrusVGAState *s,
 uint8_t p;
 dstpitch -= bltwidth;
 srcpitch -= bltwidth;
+
+if (bltheight > 1 && (dstpitch < 0 || srcpitch < 0)) {
+return;
+}
+
 for (y = 0; y < bltheight; y++) {
 for (x = 0; x < bltwidth; x++) {
p = *dst;
@@ -143,6 +148,11 @@ glue(glue(cirrus_bitblt_rop_fwd_transp_, 
ROP_NAME),_16)(CirrusVGAState *s,
 uint8_t p1, p2;
 dstpitch -= bltwidth;
 srcpitch -= bltwidth;
+
+if (bltheight > 1 && (dstpitch < 0 || srcpitch < 0)) {
+return;
+}
+
 for (y = 0; y < bltheight; y++) {
 for (x = 0; x < bltwidth; x+=2) {
p1 = *dst;
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH RFC v3 00/15] basic vfio-ccw infrastructure

2017-03-16 Thread Dong Jia Shi
* Cornelia Huck  [2017-03-16 10:25:16 +0100]:

> On Mon, 13 Mar 2017 15:16:52 +0800
> Dong Jia Shi  wrote:
> 
> > Beside the fixes for former comments form you and Alex, I will also do
> > the following stuff in the next version:
> > 1. Remove the "RFC" tag from the mail subject.
> > 2. Cc the s390 CIO layer maintainers for patch 1 and 2:
> > Sebastian Ott 
> > Peter Oberparleiter 
> > 3. For the new created files, update MAINTAINERS by adding:
> > S390 VFIO-CCW DRIVER
> > M:  Cornelia Huck 
> 
> Please add
> 
> M: Dong Jia Shi 
> 
> as well :)
> 
NP. :)

> > L:  linux-s...@vger.kernel.org
> > L:  k...@vger.kernel.org
> > S:  Supported
> > F:  drivers/s390/cio/vfio_ccw*
> > F:  Documentation/s390/vfio-ccw.txt
> > F:  include/uapi/linux/vfio_ccw.h

-- 
Dong Jia




[Qemu-devel] [PULL for-2.9 3/7] cirrus: switch to 4 MB video memory by default

2017-03-16 Thread Gerd Hoffmann
Quoting cirrus source code:
   Follow real hardware, cirrus card emulated has 4 MB video memory.
   Also accept 8 MB/16 MB for backward compatibility.

So just use 4MB by default.  We decided to leave that at 8MB by default
a while ago, for live migration compatibility reasons.  But we have
compat properties to handle that, so that isn't a compeling reason.

This also removes some sanity check inconsistencies in the cirrus code.
Some places check against the allocated video memory, some places check
against the 4MB physical hardware has.  Guest code can trigger asserts
because of that.

Signed-off-by: Gerd Hoffmann 
Message-id: 1489494514-15606-1-git-send-email-kra...@redhat.com
---
 hw/display/cirrus_vga.c | 4 ++--
 include/hw/compat.h | 8 
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index c90a4a3..6ffe64f 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -3023,7 +3023,7 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, 
Error **errp)
 
 static Property isa_cirrus_vga_properties[] = {
 DEFINE_PROP_UINT32("vgamem_mb", struct ISACirrusVGAState,
-   cirrus_vga.vga.vram_size_mb, 8),
+   cirrus_vga.vga.vram_size_mb, 4),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -3092,7 +3092,7 @@ static void pci_cirrus_vga_realize(PCIDevice *dev, Error 
**errp)
 
 static Property pci_vga_cirrus_properties[] = {
 DEFINE_PROP_UINT32("vgamem_mb", struct PCICirrusVGAState,
-   cirrus_vga.vga.vram_size_mb, 8),
+   cirrus_vga.vga.vram_size_mb, 4),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/compat.h b/include/hw/compat.h
index b7db438..b7e0e9f 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -18,6 +18,14 @@
 .driver   = "pci-bridge",\
 .property = "shpc",\
 .value= "on",\
+},{\
+.driver   = "cirrus-vga",\
+.property = "vgamem_mb",\
+.value= "8",\
+},{\
+.driver   = "isa-cirrus-vga",\
+.property = "vgamem_mb",\
+.value= "8",\
 },
 
 #define HW_COMPAT_2_7 \
-- 
1.8.3.1




[Qemu-devel] [PULL for-2.9 0/7] cirrus: more blitter security fixes.

2017-03-16 Thread Gerd Hoffmann
  Hi,

Another pile of cirrus blitter fixes, including cve fixes for known
issues, so clearly 2.9 material.

Patches 6+7 implement a new approach to blitter memory access sanity
checking.  We pass around offsets not pointers, and at the place where
the actual memory access happens we mask the offset to the valid
range before calculating the pointer.

That should put an end to security holes due to blit_is_unsafe() sanity
checks failing to calculate some special case correctly, or due to
blit_is_unsafe() calls missing, and kill any dragons which might still
be lurking in the code.  In theory this even obsoletes blit_is_unsafe(),
but I don't feel like ripping it out right away ...

please pull,
  Gerd

The following changes since commit 1883ff34b540daacae948f493b0ba525edf5f642:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2017-03-15 18:44:05 +)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-cirrus-20170316-1

for you to fetch changes up to ffaf857778286ca54e3804432a2369a279e73aa7:

  cirrus: stop passing around src pointers in the blitter (2017-03-16 08:58:16 
+0100)


cirrus: blitter fixes.


Gerd Hoffmann (6):
  cirrus/vnc: zap bitblit support from console code.
  cirrus: switch to 4 MB video memory by default
  cirrus: add option to disable blitter
  cirrus: fix cirrus_invalidate_region
  cirrus: stop passing around dst pointers in the blitter
  cirrus: stop passing around src pointers in the blitter

hangaohuai (1):
  fix :cirrus_vga fix OOB read case qemu Segmentation fault

 hw/display/cirrus_vga.c  | 106 
 hw/display/cirrus_vga_rop.h  | 191 ++-
 hw/display/cirrus_vga_rop2.h | 125 ++--
 include/hw/compat.h  |   8 ++
 include/ui/console.h |   7 --
 ui/console.c |  28 ---
 ui/vnc.c | 100 --
 7 files changed, 259 insertions(+), 306 deletions(-)



[Qemu-devel] [PULL for-2.9 6/7] cirrus: stop passing around dst pointers in the blitter

2017-03-16 Thread Gerd Hoffmann
Instead pass around the address (aka offset into vga memory).  Calculate
the pointer in the rop_* functions, after applying the mask to the
address, to make sure the address stays within the valid range.

Signed-off-by: Gerd Hoffmann 
Message-id: 1489574872-8679-1-git-send-email-kra...@redhat.com
---
 hw/display/cirrus_vga.c  |  20 +++---
 hw/display/cirrus_vga_rop.h  | 161 +--
 hw/display/cirrus_vga_rop2.h |  97 +-
 3 files changed, 153 insertions(+), 125 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index a9f6d5b..9ae1d4b 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -178,11 +178,12 @@
 
 struct CirrusVGAState;
 typedef void (*cirrus_bitblt_rop_t) (struct CirrusVGAState *s,
- uint8_t * dst, const uint8_t * src,
+ uint32_t dstaddr, const uint8_t *src,
 int dstpitch, int srcpitch,
 int bltwidth, int bltheight);
 typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
-  uint8_t *dst, int dst_pitch, int width, int 
height);
+  uint32_t dstaddr, int dst_pitch,
+  int width, int height);
 
 typedef struct CirrusVGAState {
 VGACommonState vga;
@@ -321,14 +322,14 @@ static bool blit_is_unsafe(struct CirrusVGAState *s, bool 
dst_only)
 }
 
 static void cirrus_bitblt_rop_nop(CirrusVGAState *s,
-  uint8_t *dst,const uint8_t *src,
+  uint32_t dstaddr, const uint8_t *src,
   int dstpitch,int srcpitch,
   int bltwidth,int bltheight)
 {
 }
 
 static void cirrus_bitblt_fill_nop(CirrusVGAState *s,
-   uint8_t *dst,
+   uint32_t dstaddr,
int dstpitch, int bltwidth,int bltheight)
 {
 }
@@ -678,11 +679,8 @@ static void cirrus_invalidate_region(CirrusVGAState * s, 
int off_begin,
 static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s, bool videosrc)
 {
 uint32_t patternsize;
-uint8_t *dst;
 uint8_t *src;
 
-dst = s->vga.vram_ptr + s->cirrus_blt_dstaddr;
-
 if (videosrc) {
 switch (s->vga.get_bpp(&s->vga)) {
 case 8:
@@ -711,7 +709,7 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState 
*s, bool videosrc)
 return 0;
 }
 
-(*s->cirrus_rop) (s, dst, src,
+(*s->cirrus_rop) (s, s->cirrus_blt_dstaddr, src,
   s->cirrus_blt_dstpitch, 0,
   s->cirrus_blt_width, s->cirrus_blt_height);
 cirrus_invalidate_region(s, s->cirrus_blt_dstaddr,
@@ -730,7 +728,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int 
blt_rop)
 return 0;
 }
 rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 
1];
-rop_func(s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
+rop_func(s, s->cirrus_blt_dstaddr,
  s->cirrus_blt_dstpitch,
  s->cirrus_blt_width, s->cirrus_blt_height);
 cirrus_invalidate_region(s, s->cirrus_blt_dstaddr,
@@ -797,7 +795,7 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int 
src, int w, int h)
 }
 }
 
-(*s->cirrus_rop) (s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
+(*s->cirrus_rop) (s, s->cirrus_blt_dstaddr,
   s->vga.vram_ptr + s->cirrus_blt_srcaddr,
  s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
  s->cirrus_blt_width, s->cirrus_blt_height);
@@ -848,7 +846,7 @@ static void cirrus_bitblt_cputovideo_next(CirrusVGAState * 
s)
 } else {
 /* at least one scan line */
 do {
-(*s->cirrus_rop)(s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
+(*s->cirrus_rop)(s, s->cirrus_blt_dstaddr,
   s->cirrus_bltbuf, 0, 0, s->cirrus_blt_width, 
1);
 cirrus_invalidate_region(s, s->cirrus_blt_dstaddr, 0,
  s->cirrus_blt_width, 1);
diff --git a/hw/display/cirrus_vga_rop.h b/hw/display/cirrus_vga_rop.h
index b7447f8..1aa778d 100644
--- a/hw/display/cirrus_vga_rop.h
+++ b/hw/display/cirrus_vga_rop.h
@@ -22,31 +22,65 @@
  * THE SOFTWARE.
  */
 
-static inline void glue(rop_8_,ROP_NAME)(uint8_t *dst, uint8_t src)
+static inline void glue(rop_8_, ROP_NAME)(CirrusVGAState *s,
+  uint32_t dstaddr, uint8_t src)
 {
+uint8_t *dst = &s->vga.vram_ptr[dstaddr & s->cirrus_addr_mask];
 *dst = ROP_FN(*dst, src);
 }
 
-static inline void glue(rop_16_,ROP_NAME)(uint16_t *dst, uint16_t src)
+static inline void glue(rop_tr_8_, ROP_NAME)(CirrusVGAState *s,
+ uint32_t dstaddr, uint8_t src,
+   

[Qemu-devel] [PULL for-2.9 7/7] cirrus: stop passing around src pointers in the blitter

2017-03-16 Thread Gerd Hoffmann
Does basically the same as "cirrus: stop passing around dst pointers in
the blitter", just for the src pointer instead of the dst pointer.

For the src we have to care about cputovideo blits though and fetch the
data from s->cirrus_bltbuf instead of vga memory.  The cirrus_src*()
helper functions handle that.

Signed-off-by: Gerd Hoffmann 
Message-id: 1489584487-3489-1-git-send-email-kra...@redhat.com
---
 hw/display/cirrus_vga.c  | 61 +++-
 hw/display/cirrus_vga_rop.h  | 48 +-
 hw/display/cirrus_vga_rop2.h | 38 ++-
 3 files changed, 93 insertions(+), 54 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 9ae1d4b..afc290a 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -178,7 +178,7 @@
 
 struct CirrusVGAState;
 typedef void (*cirrus_bitblt_rop_t) (struct CirrusVGAState *s,
- uint32_t dstaddr, const uint8_t *src,
+ uint32_t dstaddr, uint32_t srcaddr,
 int dstpitch, int srcpitch,
 int bltwidth, int bltheight);
 typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
@@ -322,7 +322,7 @@ static bool blit_is_unsafe(struct CirrusVGAState *s, bool 
dst_only)
 }
 
 static void cirrus_bitblt_rop_nop(CirrusVGAState *s,
-  uint32_t dstaddr, const uint8_t *src,
+  uint32_t dstaddr, uint32_t srcaddr,
   int dstpitch,int srcpitch,
   int bltwidth,int bltheight)
 {
@@ -334,6 +334,45 @@ static void cirrus_bitblt_fill_nop(CirrusVGAState *s,
 {
 }
 
+static inline uint8_t cirrus_src(CirrusVGAState *s, uint32_t srcaddr)
+{
+if (s->cirrus_srccounter) {
+/* cputovideo */
+return s->cirrus_bltbuf[srcaddr & (CIRRUS_BLTBUFSIZE - 1)];
+} else {
+/* videotovideo */
+return s->vga.vram_ptr[srcaddr & s->cirrus_addr_mask];
+}
+}
+
+static inline uint16_t cirrus_src16(CirrusVGAState *s, uint32_t srcaddr)
+{
+uint16_t *src;
+
+if (s->cirrus_srccounter) {
+/* cputovideo */
+src = (void *)&s->cirrus_bltbuf[srcaddr & (CIRRUS_BLTBUFSIZE - 1) & 
~1];
+} else {
+/* videotovideo */
+src = (void *)&s->vga.vram_ptr[srcaddr & s->cirrus_addr_mask & ~1];
+}
+return *src;
+}
+
+static inline uint32_t cirrus_src32(CirrusVGAState *s, uint32_t srcaddr)
+{
+uint32_t *src;
+
+if (s->cirrus_srccounter) {
+/* cputovideo */
+src = (void *)&s->cirrus_bltbuf[srcaddr & (CIRRUS_BLTBUFSIZE - 1) & 
~3];
+} else {
+/* videotovideo */
+src = (void *)&s->vga.vram_ptr[srcaddr & s->cirrus_addr_mask & ~3];
+}
+return *src;
+}
+
 #define ROP_NAME 0
 #define ROP_FN(d, s) 0
 #include "cirrus_vga_rop.h"
@@ -676,10 +715,10 @@ static void cirrus_invalidate_region(CirrusVGAState * s, 
int off_begin,
 }
 }
 
-static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s, bool videosrc)
+static int cirrus_bitblt_common_patterncopy(CirrusVGAState *s)
 {
 uint32_t patternsize;
-uint8_t *src;
+bool videosrc = !s->cirrus_srccounter;
 
 if (videosrc) {
 switch (s->vga.get_bpp(&s->vga)) {
@@ -700,16 +739,14 @@ static int 
cirrus_bitblt_common_patterncopy(CirrusVGAState *s, bool videosrc)
 if (s->cirrus_blt_srcaddr + patternsize > s->vga.vram_size) {
 return 0;
 }
-src = s->vga.vram_ptr + s->cirrus_blt_srcaddr;
-} else {
-src = s->cirrus_bltbuf;
 }
 
 if (blit_is_unsafe(s, true)) {
 return 0;
 }
 
-(*s->cirrus_rop) (s, s->cirrus_blt_dstaddr, src,
+(*s->cirrus_rop) (s, s->cirrus_blt_dstaddr,
+  videosrc ? s->cirrus_blt_srcaddr : 0,
   s->cirrus_blt_dstpitch, 0,
   s->cirrus_blt_width, s->cirrus_blt_height);
 cirrus_invalidate_region(s, s->cirrus_blt_dstaddr,
@@ -746,7 +783,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int 
blt_rop)
 
 static int cirrus_bitblt_videotovideo_patterncopy(CirrusVGAState * s)
 {
-return cirrus_bitblt_common_patterncopy(s, true);
+return cirrus_bitblt_common_patterncopy(s);
 }
 
 static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
@@ -796,7 +833,7 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int 
src, int w, int h)
 }
 
 (*s->cirrus_rop) (s, s->cirrus_blt_dstaddr,
-  s->vga.vram_ptr + s->cirrus_blt_srcaddr,
+  s->cirrus_blt_srcaddr,
  s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
  s->cirrus_blt_width, s->cirrus_blt_height);
 
@@ -839,7 +876,7 @@ static void cirrus_bitblt_cputovideo_next(CirrusVGAState * 
s)
 
 if (s->cirrus_srccounter > 0) {
 if (s->cirrus_blt_mode & CIRRUS

[Qemu-devel] Device 'id' property not getting set for virtio-net-pci device type

2017-03-16 Thread Gaurav Sharma
I am using qemu 2.6.2 and i have the following option in my command line
"-device virtio-net-pci,netdev=net1,mac=XX:YY:XX:XX:99:99,id=n1". In the
realize function for virtio-net-pci the value of 'id' is not getting
reflected.

Is this a known issue or am i missing something ?

-Gaurav


Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

2017-03-16 Thread Janne Huttunen
On Wed, 2017-03-15 at 08:24 +0100, Gerd Hoffmann wrote:
> >
> > The short answer: emulating real hardware.
> 
> Ok, that is reason enough.
> 
> Adding bootonceindex everywhere doesn't look like the best plan to me
> though.  Possibly we can pimp up bootindex in a backward-compatible
> way?
> Something like bootindex=[.] ?

That would (likely) avoid modifying all devices, but wouldn't
that make the 'bootindex' property a string (now: 'int32')
and thus change the QOM API?

I did consider making device_add_bootindex_property() to
automatically add the new property too, but since that API
is currently such that the _caller_ provides the name of the
added property, it would mean that the function would need
to generate the second name using some magic mangling rule
and that didn't seem very nice to me. Of course the API could
be modified so that the caller provides two names, but then
we are already back to modifying all relevant devices.





Re: [Qemu-devel] [PULL for-2.9 4/7] cirrus: add option to disable blitter

2017-03-16 Thread 李强
Hello Gerd,

> -Original Message-
> From: Qemu-devel
> [mailto:qemu-devel-bounces+liqiang6-s=360...@nongnu.org] On Behalf Of
> Gerd Hoffmann
> Sent: Thursday, March 16, 2017 5:31 PM
> To: qemu-devel@nongnu.org
> Cc: Gerd Hoffmann
> Subject: [Qemu-devel] [PULL for-2.9 4/7] cirrus: add option to disable blitter
> 
> Ok, we have this beast in the cirrus code which is not used at all by modern
> guests, except when you try to find security holes in qemu.  So, add an option
> to disable blitter altogether.  Guests released within the last ten years 
> should
> not show any rendering issues if you turn off blitter support.
> 
> There are no known bugs in the cirrus blitter code.  But in the past we hoped 
> a
> few times already that we've finally nailed the last issue.  So having some 
> easy
> way to mitigate in case yet another blitter issue shows up certainly makes me
> sleep a bit better at night.
> 
> For completeness:  The by far better way to mitigate is to switch away from
> cirrus and use stdvga instead.  Or something more modern like virtio-vga in
> case your guest has support for it.
> 
> Signed-off-by: Gerd Hoffmann 
> Message-id: 1489494540-15745-1-git-send-email-kra...@redhat.com
> ---
>  hw/display/cirrus_vga.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index
> 6ffe64f..326d511 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -205,6 +205,7 @@ typedef struct CirrusVGAState {
>  uint32_t cirrus_bank_base[2];
>  uint32_t cirrus_bank_limit[2];
>  uint8_t cirrus_hidden_palette[48];
> +bool enable_blitter;
>  int cirrus_blt_pixelwidth;
>  int cirrus_blt_width;
>  int cirrus_blt_height;
> @@ -960,6 +961,10 @@ static void cirrus_bitblt_start(CirrusVGAState * s)  {
>  uint8_t blt_rop;
> 
> +if (!s->enable_blitter) {
> +goto bitblt_ignore;
> +}
> +
>  s->vga.gr[0x31] |= CIRRUS_BLT_BUSY;
> 
>  s->cirrus_blt_width = (s->vga.gr[0x20] | (s->vga.gr[0x21] << 8)) + 1; @@
> -3024,6 +3029,8 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev,
> Error **errp)  static Property isa_cirrus_vga_properties[] = {
>  DEFINE_PROP_UINT32("vgamem_mb", struct ISACirrusVGAState,
> cirrus_vga.vga.vram_size_mb, 4),
> +DEFINE_PROP_BOOL("blitter", struct ISACirrusVGAState,
> +   cirrus_vga.enable_blitter, true),
>  DEFINE_PROP_END_OF_LIST(),
>  };
> 
> @@ -3093,6 +3100,8 @@ static void pci_cirrus_vga_realize(PCIDevice *dev,
> Error **errp)  static Property pci_vga_cirrus_properties[] = {
>  DEFINE_PROP_UINT32("vgamem_mb", struct PCICirrusVGAState,
> cirrus_vga.vga.vram_size_mb, 4),
> +DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
> + cirrus_vga.enable_blitter, true),

The default is 'ENABLE'? I think there should be 'false'.


Thanks.

Qiang

>  DEFINE_PROP_END_OF_LIST(),
>  };
> 
> --
> 1.8.3.1
> 



[Qemu-devel] [PATCH 1/3] COLO-proxy: Add virtio-net packet parse function

2017-03-16 Thread Zhang Chen
Change parse_packet_early(Packet *pkt) to parse_packet_early(Packet *pkt, int 
offset)
that we can skip virtio-net header.

Signed-off-by: Zhang Chen 
---
 net/colo-compare.c| 11 +++
 net/colo.c|  8 
 net/colo.h|  5 -
 net/filter-rewriter.c | 15 ++-
 4 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 54e6d40..ce0cd12 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -126,10 +126,13 @@ static int packet_enqueue(CompareState *s, int mode)
 pkt = packet_new(s->sec_rs.buf, s->sec_rs.packet_len);
 }
 
-if (parse_packet_early(pkt)) {
-packet_destroy(pkt, NULL);
-pkt = NULL;
-return -1;
+/* Try to parse the virtio-net-pci packet */
+if (parse_packet_early(pkt, VIRTIO_NET_HEADER)) {
+if (parse_packet_early(pkt, 0)) {
+packet_destroy(pkt, NULL);
+pkt = NULL;
+return -1;
+}
 }
 fill_connection_key(pkt, &key);
 
diff --git a/net/colo.c b/net/colo.c
index 8cc166b..060e822 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -39,15 +39,15 @@ int connection_key_equal(const void *key1, const void *key2)
 return memcmp(key1, key2, sizeof(ConnectionKey)) == 0;
 }
 
-int parse_packet_early(Packet *pkt)
+int parse_packet_early(Packet *pkt, int offset)
 {
 int network_length;
 static const uint8_t vlan[] = {0x81, 0x00};
-uint8_t *data = pkt->data;
+uint8_t *data = pkt->data + offset;
 uint16_t l3_proto;
 ssize_t l2hdr_len = eth_get_l2_hdr_length(data);
 
-if (pkt->size < ETH_HLEN) {
+if (pkt->size < ETH_HLEN + offset) {
 trace_colo_proxy_main("pkt->size < ETH_HLEN");
 return 1;
 }
@@ -73,7 +73,7 @@ int parse_packet_early(Packet *pkt)
 }
 
 network_length = pkt->ip->ip_hl * 4;
-if (pkt->size < l2hdr_len + network_length) {
+if (pkt->size < l2hdr_len + network_length + offset) {
 trace_colo_proxy_main("pkt->size < network_header + network_length");
 return 1;
 }
diff --git a/net/colo.h b/net/colo.h
index 7c524f3..b713f87 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -33,6 +33,9 @@
 #define IPPROTO_UDPLITE 136
 #endif
 
+/* virtio-net header length */
+#define VIRTIO_NET_HEADER 12
+
 typedef struct Packet {
 void *data;
 union {
@@ -73,7 +76,7 @@ typedef struct Connection {
 
 uint32_t connection_key_hash(const void *opaque);
 int connection_key_equal(const void *opaque1, const void *opaque2);
-int parse_packet_early(Packet *pkt);
+int parse_packet_early(Packet *pkt, int offset);
 void fill_connection_key(Packet *pkt, ConnectionKey *key);
 void reverse_connection_key(ConnectionKey *key);
 Connection *connection_new(ConnectionKey *key);
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index afa06e8..c3f0261 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -51,12 +51,17 @@ static void filter_rewriter_flush(NetFilterState *nf)
  */
 static int is_tcp_packet(Packet *pkt)
 {
-if (!parse_packet_early(pkt) &&
-pkt->ip->ip_p == IPPROTO_TCP) {
-return 1;
-} else {
-return 0;
+if (!parse_packet_early(pkt, VIRTIO_NET_HEADER) ||
+!parse_packet_early(pkt, 0)) {
+if (pkt->ip->ip_p == IPPROTO_TCP) {
+return 1;
+} else {
+goto out;
+}
 }
+
+out:
+return 0;
 }
 
 /* handle tcp packet from primary guest */
-- 
2.7.4






[Qemu-devel] [PATCH 3/3] COLO-compare: Add virtio-net packet compare support

2017-03-16 Thread Zhang Chen
If packet is virtio-net packet we will skip the virtio-net header
compare packet's payload.

Signed-off-by: Zhang Chen 
---
 net/colo-compare.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index ce0cd12..34af11a 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -237,7 +237,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet 
*ppkt)
 }
 
 if (ptcp->th_sum == stcp->th_sum) {
-res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
+if (ppkt->is_virtio_net_pkt) {
+res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN
+ + VIRTIO_NET_HEADER);
+} else {
+res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
+}
 } else {
 res = -1;
 }
@@ -285,8 +290,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
*ppkt)
  * other field like TOS,TTL,IP Checksum. we only need to compare
  * the ip payload here.
  */
-ret = colo_packet_compare_common(ppkt, spkt,
- network_header_length + ETH_HLEN);
+if (ppkt->is_virtio_net_pkt) {
+ret = colo_packet_compare_common(ppkt, spkt,
+ network_header_length
+ + VIRTIO_NET_HEADER + ETH_HLEN);
+} else {
+ret = colo_packet_compare_common(ppkt, spkt,
+ network_header_length + ETH_HLEN);
+}
 
 if (ret) {
 trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -309,6 +320,7 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
*ppkt)
 static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
 {
 int network_header_length = ppkt->ip->ip_hl * 4;
+int ret;
 
 trace_colo_compare_main("compare icmp");
 
@@ -322,8 +334,17 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet 
*ppkt)
  * other field like TOS,TTL,IP Checksum. we only need to compare
  * the ip payload here.
  */
-if (colo_packet_compare_common(ppkt, spkt,
-   network_header_length + ETH_HLEN)) {
+
+if (ppkt->is_virtio_net_pkt) {
+ret = colo_packet_compare_common(ppkt, spkt,
+ network_header_length
+ + VIRTIO_NET_HEADER + ETH_HLEN);
+} else {
+ret = colo_packet_compare_common(ppkt, spkt,
+ network_header_length + ETH_HLEN);
+}
+
+if (ret) {
 trace_colo_compare_icmp_miscompare("primary pkt size",
ppkt->size);
 trace_colo_compare_icmp_miscompare("Secondary pkt size",
-- 
2.7.4






[Qemu-devel] [PATCH 0/3] Add COLO-proxy virtio-net support

2017-03-16 Thread Zhang Chen
If user use -device virtio-net-pci, virtio-net driver will add a header
to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to compare
the sent packet that primary guest's to secondary guest's.


Zhang Chen (3):
  COLO-proxy: Add virtio-net packet parse function
  COLO-proxy: Add a tag to mark virtio-net packet
  COLO-compare: Add virtio-net packet compare support

 net/colo-compare.c| 42 +-
 net/colo.c| 14 ++
 net/colo.h|  7 ++-
 net/filter-rewriter.c | 15 ++-
 4 files changed, 59 insertions(+), 19 deletions(-)

-- 
2.7.4






[Qemu-devel] [PATCH 2/3] COLO-proxy: Add a tag to mark virtio-net packet

2017-03-16 Thread Zhang Chen
Add this tag that compare can recognize virtio-net packet.

Signed-off-by: Zhang Chen 
---
 net/colo.c | 6 ++
 net/colo.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/net/colo.c b/net/colo.c
index 060e822..d2b3683 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -79,6 +79,12 @@ int parse_packet_early(Packet *pkt, int offset)
 }
 pkt->transport_header = pkt->network_header + network_length;
 
+if (offset == VIRTIO_NET_HEADER) {
+pkt->is_virtio_net_pkt = true;
+} else {
+pkt->is_virtio_net_pkt = false;
+}
+
 return 0;
 }
 
diff --git a/net/colo.h b/net/colo.h
index b713f87..535793d 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -46,6 +46,8 @@ typedef struct Packet {
 int size;
 /* Time of packet creation, in wall clock ms */
 int64_t creation_ms;
+/* Mark this packet as a virtio net packet or not */
+bool is_virtio_net_pkt;
 } Packet;
 
 typedef struct ConnectionKey {
-- 
2.7.4






[Qemu-devel] [PATCH for-2.9 1/1] s390x/css: reassign subchannel if schid is changed after migration

2017-03-16 Thread Cornelia Huck
From: Dong Jia Shi 

The subchannel is a means to access a device. While the device number is
assigned by the administrator, the subchannel number is assigned by
the channel subsystem in an ascending order on cold and hot plug.
When doing unplug and replug operations, the same device may end up on
a different subchannel; for example

- We start with a device fe.1., which ends up at subchannel
  fe.1..
- Now we detach the device, attach a device fe.1. (which would get
  the now-free subchannel fe.1.), re-attach fe.1. (which ends
  up at subchannel fe.1.0001) and detach fe.1..
- We now have the same device (fe.1.) available to the guest; it
  just shows up on a different subchannel.

In such a case, the subchannel numbers are different from what a
QEMU would create during cold plug when parsing the command line.

As this would cause a guest visible change on migration, we do restore
the source system's value of the subchannel number on load.

So we are now fine from the guest perspective. From the host
perspective this will cause an inconsistent state in our internal data
structures, though.

For example, the subchannel 0 might not be at array position 0. This will
lead to problems when we continue doing hot (un/re) plug operations.

Let's fix this by cleaning up our internal data structures.

Reported-by: Cornelia Huck 
Signed-off-by: Dong Jia Shi 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Cornelia Huck 
---
 hw/s390x/css.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index e32b2a4d42..37caa98195 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1675,12 +1675,27 @@ void subch_device_save(SubchDev *s, QEMUFile *f)
 
 int subch_device_load(SubchDev *s, QEMUFile *f)
 {
+SubchDev *old_s;
+uint16_t old_schid = s->schid;
 int i;
 
 s->cssid = qemu_get_byte(f);
 s->ssid = qemu_get_byte(f);
 s->schid = qemu_get_be16(f);
 s->devno = qemu_get_be16(f);
+/* Re-assign subch. */
+if (old_schid != s->schid) {
+old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid];
+/*
+ * (old_s != s) means that some other device has its correct
+ * subchannel already assigned (in load).
+ */
+if (old_s == s) {
+css_subch_assign(s->cssid, s->ssid, old_schid, s->devno, NULL);
+}
+/* It's OK to re-assign without a prior de-assign. */
+css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s);
+}
 s->thinint_active = qemu_get_byte(f);
 /* SCHIB */
 /* PMCW */
-- 
2.11.0




[Qemu-devel] [PATCH for-2.9 0/1] bugfix for s390x

2017-03-16 Thread Cornelia Huck
A fix for a bug in the s390x css implementation.

The especially annoying thing about that bug is that things seem
to work fine after migration -- until you get funny errors when
you try to add/delete some devices...

Dong Jia Shi (1):
  s390x/css: reassign subchannel if schid is changed after migration

 hw/s390x/css.c | 15 +++
 1 file changed, 15 insertions(+)

-- 
2.11.0




Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

2017-03-16 Thread Gerd Hoffmann
On Do, 2017-03-16 at 11:46 +0200, Janne Huttunen wrote:
> On Wed, 2017-03-15 at 08:24 +0100, Gerd Hoffmann wrote:
> > >
> > > The short answer: emulating real hardware.
> > 
> > Ok, that is reason enough.
> > 
> > Adding bootonceindex everywhere doesn't look like the best plan to me
> > though.  Possibly we can pimp up bootindex in a backward-compatible
> > way?
> > Something like bootindex=[.] ?
> 
> That would (likely) avoid modifying all devices, but wouldn't
> that make the 'bootindex' property a string (now: 'int32')
> and thus change the QOM API?

Good point.  I was thinking about the cmd line only where it is a string
anyway.

> I did consider making device_add_bootindex_property() to
> automatically add the new property too, but since that API
> is currently such that the _caller_ provides the name of the
> added property, it would mean that the function would need
> to generate the second name using some magic mangling rule
> and that didn't seem very nice to me.

I think the only case where this is something != "bootindex" is the
floppy controller, which has bootindexA and bootindexB for the two
drives.  So name mangling doesn't look too bad to me.  Maybe we could
just add a "first-" or "once-" prefix.  But the second bootindex still
needs to be stored somewhere in the device state struct ...

> Of course the API could
> be modified so that the caller provides two names, but then
> we are already back to modifying all relevant devices.

... so I guess there isn't really some way around that.

cheers,
  Gerd




Re: [Qemu-devel] [PULL for-2.9] Update OpenBIOS images

2017-03-16 Thread Peter Maydell
On 15 March 2017 at 21:07, Mark Cave-Ayland
 wrote:
> Hi Peter,
>
> This update contains just the 64-bit PCI BAR fix which enables virtio modern 
> devices to
> work once again. Please pull.
>
>
> ATB,
>
> Mark.
>
>
> The following changes since commit 1883ff34b540daacae948f493b0ba525edf5f642:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
> (2017-03-15 18:44:05 +)
>
> are available in the git repository at:
>
>
>   https://github.com/mcayland/qemu.git tags/qemu-openbios-signed
>
> for you to fetch changes up to 111308e789b7a3e2771a61f68bd057f8f8b1bc22:
>
>   Update OpenBIOS images to f233c3f built from submodule. (2017-03-15 
> 19:42:08 +)
>
> 
> Update OpenBIOS images
>
> 
> Mark Cave-Ayland (1):
>   Update OpenBIOS images to f233c3f built from submodule.
>
>  pc-bios/openbios-ppc |  Bin 750840 -> 750840 bytes
>  pc-bios/openbios-sparc32 |  Bin 382048 -> 382048 bytes
>  pc-bios/openbios-sparc64 |  Bin 1593408 -> 1593408 bytes
>  roms/openbios|2 +-
>  4 files changed, 1 insertion(+), 1 deletion(-)


Applied, thanks.

-- PMM



Re: [Qemu-devel] [Bug 1217339] [PATCH] Unix signal to send ACPI-shutdown to Guest

2017-03-16 Thread Simon

Hi Peter,


Why can't we use SIGHUP, again?


I suppose for all those people who use non ACPI-aware guests in non
daemonized Qemu instances and usually close their terminal without
stopping Qemu first on the assumption that Qemu will stop itself
automatically: this wouldn't work anymore...

More seriously, Qemu is a very versatile piece of software. I can only
rely on the word of experienced Qemu maintainers who have a better view
than me on the general Qemu user-base to determine whether a change of
SIGHUP handling will have a negative effect or not.

Using SIGHUP was my initial proposition and is the easy way to go.

In case it is determined that this would have too much side effects and
that no other signal is usable, the only other way I see is to add a new
command-line option to let users select Qemu behavior.

Regards,
Simon.



Re: [Qemu-devel] [Bug 1217339] [PATCH] Unix signal to send ACPI-shutdown to Guest

2017-03-16 Thread Daniel P. Berrange
On Wed, Mar 15, 2017 at 06:45:57PM +, Peter Maydell wrote:
> On 15 March 2017 at 18:08, Daniel P. Berrange  wrote:
> > On Wed, Mar 15, 2017 at 06:00:40PM +, Peter Maydell wrote:
> >> On 15 March 2017 at 17:46, Simon  wrote:
> >> > OK for not using SIGHUP and keep SIGTERM, SIGINT and SIGHUP to have the
> >> > same behavior.
> >> >
> >> > SIGQUIT is reserved for core files generation.
> >> >
> >> > SIGUSR1 is already used in 'util/qemu-progress.c' to trigger a report
> >> > on ongoing jobs, so it does not seem usable.
> >> >
> >> > SIGUSR2 is temporarily used in 'util/coroutine-sigaltstack.c' which
> >> > takes care however to preserve the original handler. I did not saw
> >> > any other place where it is used, so it seems to be a better candidate.
> >>
> >> I don't think we can use SIGUSR2 here -- as you say, it's used
> >> in the sigaltstack version of coroutines, and so there would
> >> be races if the user tried to use SIGUSR2 to power down the
> >> machine while we happened to be starting a new coroutine.
> >>
> >> SIGUSR1 is also no good, as it is used by main-loop.c as
> >> the SIG_IPI.
> >
> > Which means we'd be into the realm of having to pick  SIGRTMIN + N for
> > some arbitrary N >= 0
> 
> That's pretty nasty. Why can't we use SIGHUP, again?

Because QEMU already designate that as doing an immediate stop - ie it'll
allow QEMU block layer to flush pending I/O, but it will not wait for the
guest to shutdown.  If we change that behaviour we'll break anyone who
is already relying on SIGHUP - qemu might never exit at all if the guest
ignores the ACPI request


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



[Qemu-devel] [PATCH] Dead Code Removal: removing support for DEPTH != 32.

2017-03-16 Thread iwona260909
From: Iwona Kotlarska 

Signed-off-by: Iwona Kotlarska 
---
 hw/display/sm501.c  | 37 -
 hw/display/sm501_template.h |  8 +---
 2 files changed, 1 insertion(+), 44 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 040a0b93f2..8935ea758a 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1173,23 +1173,6 @@ typedef void draw_line_func(uint8_t *d, const uint8_t *s,
 typedef void draw_hwc_line_func(SM501State * s, int crt, uint8_t * palette,
 int c_y, uint8_t *d, int width);
 
-#define DEPTH 8
-#include "sm501_template.h"
-
-#define DEPTH 15
-#include "sm501_template.h"
-
-#define BGR_FORMAT
-#define DEPTH 15
-#include "sm501_template.h"
-
-#define DEPTH 16
-#include "sm501_template.h"
-
-#define BGR_FORMAT
-#define DEPTH 16
-#include "sm501_template.h"
-
 #define DEPTH 32
 #include "sm501_template.h"
 
@@ -1198,43 +1181,23 @@ typedef void draw_hwc_line_func(SM501State * s, int 
crt, uint8_t * palette,
 #include "sm501_template.h"
 
 static draw_line_func * draw_line8_funcs[] = {
-draw_line8_8,
-draw_line8_15,
-draw_line8_16,
 draw_line8_32,
 draw_line8_32bgr,
-draw_line8_15bgr,
-draw_line8_16bgr,
 };
 
 static draw_line_func * draw_line16_funcs[] = {
-draw_line16_8,
-draw_line16_15,
-draw_line16_16,
 draw_line16_32,
 draw_line16_32bgr,
-draw_line16_15bgr,
-draw_line16_16bgr,
 };
 
 static draw_line_func * draw_line32_funcs[] = {
-draw_line32_8,
-draw_line32_15,
-draw_line32_16,
 draw_line32_32,
 draw_line32_32bgr,
-draw_line32_15bgr,
-draw_line32_16bgr,
 };
 
 static draw_hwc_line_func * draw_hwc_line_funcs[] = {
-draw_hwc_line_8,
-draw_hwc_line_15,
-draw_hwc_line_16,
 draw_hwc_line_32,
 draw_hwc_line_32bgr,
-draw_hwc_line_15bgr,
-draw_hwc_line_16bgr,
 };
 
 static inline int get_depth_index(DisplaySurface *surface)
diff --git a/hw/display/sm501_template.h b/hw/display/sm501_template.h
index f33e499be4..4e5801ec3e 100644
--- a/hw/display/sm501_template.h
+++ b/hw/display/sm501_template.h
@@ -22,13 +22,7 @@
  * THE SOFTWARE.
  */
 
-#if DEPTH == 8
-#define BPP 1
-#define PIXEL_TYPE uint8_t
-#elif DEPTH == 15 || DEPTH == 16
-#define BPP 2
-#define PIXEL_TYPE uint16_t
-#elif DEPTH == 32
+#if DEPTH == 32
 #define BPP 4
 #define PIXEL_TYPE uint32_t
 #else
-- 
2.12.0




[Qemu-devel] [PATCH] puv3: always compile-check debug printf

2017-03-16 Thread Anishka0107
To prevent bitrot of the format string of the debug statement, files with
  conditional debug statements should ensure that printf is compiled always,
  and enclosed within if(0) statements and not in #ifdef.

Signed-off-by: Anishka Gupta 
---
 include/hw/unicore32/puv3.h | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/hw/unicore32/puv3.h b/include/hw/unicore32/puv3.h
index 5a4839f..e268484 100644
--- a/include/hw/unicore32/puv3.h
+++ b/include/hw/unicore32/puv3.h
@@ -41,10 +41,14 @@
 #define PUV3_IRQS_OST0  (26)
 
 /* All puv3_*.c use DPRINTF for debug. */
-#ifdef DEBUG_PUV3
-#define DPRINTF(fmt, ...) printf("%s: " fmt , __func__, ## __VA_ARGS__)
-#else
-#define DPRINTF(fmt, ...) do {} while (0)
-#endif
+#define DEBUG_PUV3 0
+
+#define DPRINTF(fmt, ...)
+if (DEBUG_PUV3) {
+fprintf(stderr, "%s: " fmt , __func__, ## __VA_ARGS__)
+}
+else {
+do {} while (0)
+}
 
 #endif /* QEMU_HW_PUV3_H */
-- 
2.5.0




[Qemu-devel] [PATCH] Dead Code Removal: removing support for DEPTH != 32

2017-03-16 Thread iwona260909
Removing support for DEPTH != 32 - task from BiteSizedTasks/Dead code removal



Re: [Qemu-devel] [PATCH] puv3: always compile-check debug printf

2017-03-16 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH] puv3: always compile-check debug printf
Message-id: 1489638621-31978-1-git-send-email-rimjhim0...@gmail.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
016f432 puv3: always compile-check debug printf

=== OUTPUT BEGIN ===
Checking PATCH 1/1: puv3: always compile-check debug printf...
ERROR: else should follow close brace '}'
#32: FILE: include/hw/unicore32/puv3.h:50:
+}
+else {

total: 1 errors, 0 warnings, 19 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH] Dead Code Removal: removing support for DEPTH != 32.

2017-03-16 Thread Peter Maydell
On 16 March 2017 at 09:20,   wrote:
> From: Iwona Kotlarska 
>
> Signed-off-by: Iwona Kotlarska 
> ---
>  hw/display/sm501.c  | 37 -
>  hw/display/sm501_template.h |  8 +---
>  2 files changed, 1 insertion(+), 44 deletions(-)

Hi; thanks for this patch. Unfortunately Gerd already
sent a patchset a couple of weeks ago to do this for this
device:
http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg01172.html

thanks
-- PMM



Re: [Qemu-devel] [PULL for-rc1 0/3] Ide patches

2017-03-16 Thread Peter Maydell
On 16 March 2017 at 00:52, John Snow  wrote:
> The following changes since commit 1883ff34b540daacae948f493b0ba525edf5f642:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
> (2017-03-15 18:44:05 +)
>
> are available in the git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to d68f0f778e7f4fbd674627274267f269e40f0b04:
>
>   ide: ahci: call cleanup function in ahci unit (2017-03-15 20:50:14 -0400)
>
> 
>
> 
>

Applied, thanks.

-- PMM



Re: [Qemu-devel] [PULL for-2.9 4/7] cirrus: add option to disable blitter

2017-03-16 Thread Thomas Huth
On 16.03.2017 10:51, 李强 wrote:
> Hello Gerd,
> 
>> -Original Message-
>> From: Qemu-devel
>> [mailto:qemu-devel-bounces+liqiang6-s=360...@nongnu.org] On Behalf Of
>> Gerd Hoffmann
>> Sent: Thursday, March 16, 2017 5:31 PM
>> To: qemu-devel@nongnu.org
>> Cc: Gerd Hoffmann
>> Subject: [Qemu-devel] [PULL for-2.9 4/7] cirrus: add option to disable 
>> blitter
>>
>> Ok, we have this beast in the cirrus code which is not used at all by modern
>> guests, except when you try to find security holes in qemu.  So, add an 
>> option
>> to disable blitter altogether.  Guests released within the last ten years 
>> should
>> not show any rendering issues if you turn off blitter support.
>>
>> There are no known bugs in the cirrus blitter code.  But in the past we 
>> hoped a
>> few times already that we've finally nailed the last issue.  So having some 
>> easy
>> way to mitigate in case yet another blitter issue shows up certainly makes me
>> sleep a bit better at night.
>>
>> For completeness:  The by far better way to mitigate is to switch away from
>> cirrus and use stdvga instead.  Or something more modern like virtio-vga in
>> case your guest has support for it.
>>
>> Signed-off-by: Gerd Hoffmann 
>> Message-id: 1489494540-15745-1-git-send-email-kra...@redhat.com
>> ---
>>  hw/display/cirrus_vga.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index
>> 6ffe64f..326d511 100644
>> --- a/hw/display/cirrus_vga.c
>> +++ b/hw/display/cirrus_vga.c
>> @@ -205,6 +205,7 @@ typedef struct CirrusVGAState {
>>  uint32_t cirrus_bank_base[2];
>>  uint32_t cirrus_bank_limit[2];
>>  uint8_t cirrus_hidden_palette[48];
>> +bool enable_blitter;
>>  int cirrus_blt_pixelwidth;
>>  int cirrus_blt_width;
>>  int cirrus_blt_height;
>> @@ -960,6 +961,10 @@ static void cirrus_bitblt_start(CirrusVGAState * s)  {
>>  uint8_t blt_rop;
>>
>> +if (!s->enable_blitter) {
>> +goto bitblt_ignore;
>> +}
>> +
>>  s->vga.gr[0x31] |= CIRRUS_BLT_BUSY;
>>
>>  s->cirrus_blt_width = (s->vga.gr[0x20] | (s->vga.gr[0x21] << 8)) + 1; @@
>> -3024,6 +3029,8 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev,
>> Error **errp)  static Property isa_cirrus_vga_properties[] = {
>>  DEFINE_PROP_UINT32("vgamem_mb", struct ISACirrusVGAState,
>> cirrus_vga.vga.vram_size_mb, 4),
>> +DEFINE_PROP_BOOL("blitter", struct ISACirrusVGAState,
>> +   cirrus_vga.enable_blitter, true),
>>  DEFINE_PROP_END_OF_LIST(),
>>  };
>>
>> @@ -3093,6 +3100,8 @@ static void pci_cirrus_vga_realize(PCIDevice *dev,
>> Error **errp)  static Property pci_vga_cirrus_properties[] = {
>>  DEFINE_PROP_UINT32("vgamem_mb", struct PCICirrusVGAState,
>> cirrus_vga.vga.vram_size_mb, 4),
>> +DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
>> + cirrus_vga.enable_blitter, true),
> 
> The default is 'ENABLE'? I think there should be 'false'.

I think it has to be enabled at least for the older machine types -
otherwise you change the hardware of guests during migration.

 Thomas




[Qemu-devel] [PATCH v2] puv3: always compile-check debug printf

2017-03-16 Thread Anishka0107
 To prevent bitrot of the format string of the debug statement, files with
 conditional debug statements should ensure that printf is compiled always,
 and enclosed within if(0) statements and not in #ifdef.

Signed-off-by: Anishka Gupta 
---
 include/hw/unicore32/puv3.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/hw/unicore32/puv3.h b/include/hw/unicore32/puv3.h
index e268484..9fdf0a1 100644
--- a/include/hw/unicore32/puv3.h
+++ b/include/hw/unicore32/puv3.h
@@ -46,8 +46,7 @@
 #define DPRINTF(fmt, ...)
 if (DEBUG_PUV3) {
 fprintf(stderr, "%s: " fmt , __func__, ## __VA_ARGS__)
-}
-else {
+} else {
 do {} while (0)
 }
 
-- 
2.5.0




[Qemu-devel] [PATCH] List SASL config file under the cryptography maintainer's realm

2017-03-16 Thread Daniel P. Berrange
No one is listed as maintainer for qemu.sasl. It is used by the
VNC server for SASL auth, but since it is cryptography related,
list it under the crytography maintainer's realm, rather than
under the UI maintainer.

Signed-off-by: Daniel P. Berrange 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bf1aafb..ecf8273 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1462,6 +1462,7 @@ S: Maintained
 F: crypto/
 F: include/crypto/
 F: tests/test-crypto-*
+F: qemu.sasl
 
 Coroutines
 M: Stefan Hajnoczi 
-- 
2.9.3




  1   2   3   >