[Qemu-devel] [PATCH v5 09/10] qmp.py: Avoid overriding a builtin object

[Lukáš Doktor]

The "id" is a builtin method to get object's identity and should not be
overridden. This might bring some issues in case someone was directly
calling "cmd(..., id=id)" but I haven't found such usage on brief search
for "cmd\(.*id=".

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
---
 scripts/qmp/qmp.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index f2f5a9b..ef12e8a 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -177,19 +177,19 @@ class QEMUMonitorProtocol(object):
 print >>sys.stderr, "QMP:<<< %s" % resp
 return resp
 
-def cmd(self, name, args=None, id=None):
+def cmd(self, name, args=None, cmd_id=None):
 """
 Build a QMP command and send it to the QMP Monitor.
 
 @param name: command name (string)
 @param args: command arguments (dict)
-@param id: command id (dict, list, string or int)
+@param cmd_id: command id (dict, list, string or int)
 """
 qmp_cmd = {'execute': name}
 if args:
 qmp_cmd['arguments'] = args
-if id:
-qmp_cmd['id'] = id
+if cmd_id:
+qmp_cmd['id'] = cmd_id
 return self.cmd_obj(qmp_cmd)
 
 def command(self, cmd, **kwds):
-- 
2.9.4

[Qemu-devel] [PATCH v5 06/10] qmp.py: Couple of pylint/style fixes

[Lukáš Doktor]

No actual code changes, just initializing attributes earlier to avoid
AttributeError on early introspection, a few pylint/style fixes and
docstring clarifications.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Philippe Mathieu-Daudé 
---
 scripts/qmp/qmp.py | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 62d3651..782d1ac 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -13,19 +13,30 @@ import errno
 import socket
 import sys
 
+
 class QMPError(Exception):
 pass
 
+
 class QMPConnectError(QMPError):
 pass
 
+
 class QMPCapabilitiesError(QMPError):
 pass
 
+
 class QMPTimeoutError(QMPError):
 pass
 
+
 class QEMUMonitorProtocol:
+
+#: Socket's error class
+error = socket.error
+#: Socket's timeout
+timeout = socket.timeout
+
 def __init__(self, address, server=False, debug=False):
 """
 Create a QEMUMonitorProtocol class.
@@ -42,6 +53,7 @@ class QEMUMonitorProtocol:
 self.__address = address
 self._debug = debug
 self.__sock = self.__get_sock()
+self.__sockfile = None
 if server:
 self.__sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
 self.__sock.bind(self.__address)
@@ -56,7 +68,7 @@ class QEMUMonitorProtocol:
 
 def __negotiate_capabilities(self):
 greeting = self.__json_read()
-if greeting is None or not greeting.has_key('QMP'):
+if greeting is None or "QMP" not in greeting:
 raise QMPConnectError
 # Greeting seems ok, negotiate capabilities
 resp = self.cmd('qmp_capabilities')
@@ -78,8 +90,6 @@ class QEMUMonitorProtocol:
 continue
 return resp
 
-error = socket.error
-
 def __get_events(self, wait=False):
 """
 Check for new events in the stream and cache them in __events.
@@ -89,8 +99,8 @@ class QEMUMonitorProtocol:
 
 @raise QMPTimeoutError: If a timeout float is provided and the timeout
 period elapses.
-@raise QMPConnectError: If wait is True but no events could be 
retrieved
-or if some other error occurred.
+@raise QMPConnectError: If wait is True but no events could be
+retrieved or if some other error occurred.
 """
 
 # Check for new events regardless and pull them into the cache:
@@ -175,7 +185,7 @@ class QEMUMonitorProtocol:
 @param args: command arguments (dict)
 @param id: command id (dict, list, string or int)
 """
-qmp_cmd = { 'execute': name }
+qmp_cmd = {'execute': name}
 if args:
 qmp_cmd['arguments'] = args
 if id:
@@ -183,6 +193,9 @@ class QEMUMonitorProtocol:
 return self.cmd_obj(qmp_cmd)
 
 def command(self, cmd, **kwds):
+"""
+Build and send a QMP command to the monitor, report errors if any
+"""
 ret = self.cmd(cmd, kwds)
 if ret.has_key('error'):
 raise Exception(ret['error']['desc'])
@@ -190,15 +203,15 @@ class QEMUMonitorProtocol:
 
 def pull_event(self, wait=False):
 """
-Get and delete the first available QMP event.
+Pulls a single event.
 
 @param wait (bool): block until an event is available.
 @param wait (float): If wait is a float, treat it as a timeout value.
 
 @raise QMPTimeoutError: If a timeout float is provided and the timeout
 period elapses.
-@raise QMPConnectError: If wait is True but no events could be 
retrieved
-or if some other error occurred.
+@raise QMPConnectError: If wait is True but no events could be
+retrieved or if some other error occurred.
 
 @return The first available QMP event, or None.
 """
@@ -217,8 +230,8 @@ class QEMUMonitorProtocol:
 
 @raise QMPTimeoutError: If a timeout float is provided and the timeout
 period elapses.
-@raise QMPConnectError: If wait is True but no events could be 
retrieved
-or if some other error occurred.
+@raise QMPConnectError: If wait is True but no events could be
+retrieved or if some other error occurred.
 
 @return The list of available QMP events.
 """
@@ -235,8 +248,6 @@ class QEMUMonitorProtocol:
 self.__sock.close()
 self.__sockfile.close()
 
-timeout = socket.timeout
-
 def settimeout(self, timeout):
 self.__sock.settimeout(timeout)
 
-- 
2.9.4

[Qemu-devel] [PATCH v5 10/10] qtest.py: Few pylint/style fixes

[Lukáš Doktor]

No actual code changes, just few pylint/style fixes.

Signed-off-by: Lukáš Doktor 
Reviewed-by: John Snow 
---
 scripts/qtest.py | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/scripts/qtest.py b/scripts/qtest.py
index ab183c0..df0daf2 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -11,14 +11,11 @@
 # Based on qmp.py.
 #
 
-import errno
 import socket
-import string
 import os
-import subprocess
-import qmp.qmp
 import qemu
 
+
 class QEMUQtestProtocol(object):
 def __init__(self, address, server=False):
 """
@@ -83,8 +80,10 @@ class QEMUQtestMachine(qemu.QEMUMachine):
  socket_scm_helper=None):
 if name is None:
 name = "qemu-%d" % os.getpid()
-super(QEMUQtestMachine, self).__init__(binary, args, name=name, 
test_dir=test_dir,
-   
socket_scm_helper=socket_scm_helper)
+super(QEMUQtestMachine,
+  self).__init__(binary, args, name=name, test_dir=test_dir,
+ socket_scm_helper=socket_scm_helper)
+self._qtest = None
 self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
 
 def _base_args(self):
-- 
2.9.4

[Qemu-devel] [PATCH v5 08/10] qmp.py: Avoid "has_key" usage

[Lukáš Doktor]

The "has_key" is deprecated in favor of "__in__" operator.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Philippe Mathieu-Daudé 
---
 scripts/qmp/qmp.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 95ff5cc..f2f5a9b 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -197,7 +197,7 @@ class QEMUMonitorProtocol(object):
 Build and send a QMP command to the monitor, report errors if any
 """
 ret = self.cmd(cmd, kwds)
-if ret.has_key('error'):
+if "error" in ret:
 raise Exception(ret['error']['desc'])
 return ret['return']
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH for-2.10 v2 0/2] trace: fix simpletrace.stp flight recorder mode

[no-reply]

Hi,

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

Message-id: 20170815084430.7128-1-stefa...@redhat.com
Subject: [Qemu-devel] [PATCH for-2.10 v2 0/2] trace: fix simpletrace.stp flight 
recorder mode
Type: series

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

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

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
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20170815084430.7128-1-stefa...@redhat.com -> 
patchew/20170815084430.7128-1-stefa...@redhat.com
Switched to a new branch 'test'
7d6a4eae8d simpletrace: fix flight recorder --no-header option
b371ed247d trace: use static event ID mapping in simpletrace.stp

=== OUTPUT BEGIN ===
Checking PATCH 1/2: trace: use static event ID mapping in simpletrace.stp...
Checking PATCH 2/2: simpletrace: fix flight recorder --no-header option...
ERROR: line over 90 characters
#40: FILE: scripts/simpletrace.py:101:
+"""Deserialize trace records from a file, yielding record tuples 
(event_num, timestamp, pid, arg1, ..., arg6).

total: 1 errors, 0 warnings, 46 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

[Qemu-devel] [PATCH] hw/acpi: Select an node with memory for mapping memory hole to

[Dou Liyang]

Currently, Using the fisrt node without memory on the machine makes
QEMU unhappy. With this example command line:
  ... \
  -m 1024M,slots=4,maxmem=32G \
  -numa node,nodeid=0 \
  -numa node,mem=1024M,nodeid=1 \
  -numa node,nodeid=2 \
  -numa node,nodeid=3 \
Guest reports "No NUMA configuration found" and the NUMA topology is
wrong.

This is because when QEMU builds ACPI SRAT, it regards node0 as the
default node to deal with the memory hole(640K-1M). this means the
node0 must have some memory(>1M), but, actually it can have no
memory.

Fix this problem by replace the node0 with the first node which has
memory on it. Add a new function for each node. Also do some cleanup.

Signed-off-by: Dou Liyang 
---
 hw/i386/acpi-build.c | 76 +---
 1 file changed, 48 insertions(+), 28 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 98dd424..e5f57d2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2318,15 +2318,43 @@ build_tpm2(GArray *table_data, BIOSLinker *linker)
  (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL);
 }
 
+static uint64_t
+build_srat_node_entry(GArray *table_data, PCMachineState *pcms,
+int i, uint64_t mem_base, uint64_t mem_len)
+{
+AcpiSratMemoryAffinity *numamem;
+uint64_t next_base;
+
+next_base = mem_base + mem_len;
+
+/* Cut out the ACPI_PCI hole */
+if (mem_base <= pcms->below_4g_mem_size &&
+next_base > pcms->below_4g_mem_size) {
+mem_len -= next_base - pcms->below_4g_mem_size;
+if (mem_len > 0) {
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, mem_base, mem_len, i,
+  MEM_AFFINITY_ENABLED);
+}
+mem_base = 1ULL << 32;
+mem_len = next_base - pcms->below_4g_mem_size;
+next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+}
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, mem_base, mem_len, i,
+  MEM_AFFINITY_ENABLED);
+return next_base;
+}
+
 static void
 build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
 {
 AcpiSystemResourceAffinityTable *srat;
 AcpiSratMemoryAffinity *numamem;
 
-int i;
+int i, node;
 int srat_start, numa_start, slots;
-uint64_t mem_len, mem_base, next_base;
+uint64_t mem_len, mem_base;
 MachineClass *mc = MACHINE_GET_CLASS(machine);
 const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
 PCMachineState *pcms = PC_MACHINE(machine);
@@ -2370,36 +2398,28 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
MachineState *machine)
 /* the memory map is a bit tricky, it contains at least one hole
  * from 640k-1M and possibly another one from 3.5G-4G.
  */
-next_base = 0;
+
 numa_start = table_data->len;
 
+/* get the first node which has memory and  map the hole from 640K-1M */
+for (node = 0;
+node < pcms->numa_nodes && pcms->node_mem[node] == 0;
+node++);
 numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, 0, 640 * 1024, 0, MEM_AFFINITY_ENABLED);
-next_base = 1024 * 1024;
-for (i = 1; i < pcms->numa_nodes + 1; ++i) {
-mem_base = next_base;
-mem_len = pcms->node_mem[i - 1];
-if (i == 1) {
-mem_len -= 1024 * 1024;
-}
-next_base = mem_base + mem_len;
-
-/* Cut out the ACPI_PCI hole */
-if (mem_base <= pcms->below_4g_mem_size &&
-next_base > pcms->below_4g_mem_size) {
-mem_len -= next_base - pcms->below_4g_mem_size;
-if (mem_len > 0) {
-numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, mem_base, mem_len, i - 1,
-  MEM_AFFINITY_ENABLED);
-}
-mem_base = 1ULL << 32;
-mem_len = next_base - pcms->below_4g_mem_size;
-next_base += (1ULL << 32) - pcms->below_4g_mem_size;
+build_srat_memory(numamem, 0, 640 * 1024, node, MEM_AFFINITY_ENABLED);
+
+/* map the rest of memory from 1M */
+mem_base = 1024 * 1024;
+mem_len = pcms->node_mem[node] - mem_base;
+mem_base = build_srat_node_entry(table_data, pcms, node,
+mem_base, mem_len);
+
+for (i = 0; i < pcms->numa_nodes; i++) {
+if (i == node) {
+continue;
 }
-numamem = acpi_data_push(table_data, sizeof *numamem);
-build_srat_memory(numamem, mem_base, mem_len, i - 1,
-  MEM_AFFINITY_ENABLED);
+mem_base = build_srat_node_entry(table_data, pcms, i,
+mem_base, pcms->node_mem[i]);
 }
 slots = (table_data->len - numa_start) / sizeof *numamem;
 for (; slots 

[Qemu-devel] [PATCH v5 00/10] qemu.py: Pylint/style fixes

[Lukáš Doktor]

Hello guys,

I'm reading the available python modules to exercise qemu and while reading them
I fixed some issues that caught my attention. It usually starts with a simple
pylint/docstring fixes and slowly graduates to more controversial ones so I'm
open to suggestion to remove some of them.

Kind regards,
Lukáš

Changes in v2
- Squashed 2nd and 10th patches into 2nd one
- Use repr() in MonitorResponseError's description
- Improved commit message of the 6th patch
- Two tweaks to docstrings changed in the 6th patch
- Also updated qmp-shell to use new-style super calls (7th patch)
- Fixed the accidental rename of qmp `cmd_id` (kept the id->cmd_id patch)
- Changed the style of the style-fix in the 10th commit

Changes in v3
- Don't use repr in the 5th patch in MonitorResponseError

Changes in v4
- Use correct git base (remove unwanted commits)

Changes in v5
- Avoid bool comparison
- Change report to return in one docstring
- Removed the unnecessary spaces around single-line docstring

Lukáš Doktor (10):
  qemu.py: Pylint/style fixes
  qemu|qtest: Avoid dangerous arguments
  qemu.py: Use iteritems rather than keys()
  qemu.py: Simplify QMP key-conversion
  qemu.py: Use custom exceptions rather than Exception
  qmp.py: Couple of pylint/style fixes
  qmp.py: Use object-based class for QEMUMonitorProtocol
  qmp.py: Avoid "has_key" usage
  qmp.py: Avoid overriding a builtin object
  qtest.py: Few pylint/style fixes

 scripts/qemu.py   | 98 +--
 scripts/qmp/qmp-shell |  4 +--
 scripts/qmp/qmp.py| 49 --
 scripts/qtest.py  | 13 ---
 4 files changed, 109 insertions(+), 55 deletions(-)

-- 
2.9.4

Re: [Qemu-devel] [PATCH] numa: Add a check whether the node0 has memory or not

[Dou Liyang]

Hi Igor,

At 08/15/2017 04:04 PM, Igor Mammedov wrote:

On Tue, 15 Aug 2017 09:26:46 +0800
Dou Liyang  wrote:


Hi Eduardo,

At 08/14/2017 08:44 PM, Eduardo Habkost wrote:

On Mon, Aug 14, 2017 at 06:11:11PM +0800, Dou Liyang wrote:

Currently, Using the fisrt node without memory on the machine makes
QEMU unhappy. With this example command line:
  ... \
  -m 1024M,slots=4,maxmem=32G \
  -numa node,nodeid=0 \
  -numa node,mem=1024M,nodeid=1 \
  -numa node,nodeid=2 \
  -numa node,nodeid=3 \
Guest reports "No NUMA configuration found" and the NUMA topology is
wrong.

This is because when QEMU builds ACPI SRAT, it regards node0 as the
default node to deal with the memory hole(640K-1M). this means the
node0 must have some memory(>1M) firstly.

Add a check in parse_numa_opts to avoid this situation.

Signed-off-by: Dou Liyang 
---
 numa.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/numa.c b/numa.c
index e32af04..1d6f73f 100644
--- a/numa.c
+++ b/numa.c
@@ -464,6 +464,10 @@ void parse_numa_opts(MachineState *ms)
 if (i == nb_numa_nodes) {
 assert(mc->numa_auto_assign_ram);
 mc->numa_auto_assign_ram(mc, numa_info, nb_numa_nodes, ram_size);
+} else if (i != 0) {
+error_report("The first NUMA node must have some memory"
+  " for building ACPI SART");
+exit(1);


This doesn't belong to numa.c.  numa.c is generic code, and the
requirement you described is specific for PC.

Anyway, adding this check would make existing VM configurations
refuse to run after a QEMU upgrade.  I suggest fixing the bug in
the ACPI code instead.



I see.

If fixing the bug in the ACPI code, I have two solutions:

1). Add a check in build_srat(). If the first node has no memory, QEMU
will exit.

2). Fix the initialization of memory affinity structure to cover this
situation. Using the first node which has memory to deal with the memory
hole.

I prefer solution 2. what about you?

I'd go for 2nd solution



Yeah, I see. And I just sent the 2nd solution, waiting for your reply.

Thanks,
dou.



Thanks,
dou.

[Qemu-devel] [PATCH v5 01/10] qemu.py: Pylint/style fixes

[Lukáš Doktor]

No actual code changes, just several pylint/style fixes and docstring
clarifications.

Signed-off-by: Lukáš Doktor 
---
 scripts/qemu.py | 76 -
 1 file changed, 53 insertions(+), 23 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 880e3e8..466aaab 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -23,8 +23,22 @@ import qmp.qmp
 class QEMUMachine(object):
 '''A QEMU VM'''
 
-def __init__(self, binary, args=[], wrapper=[], name=None, 
test_dir="/var/tmp",
- monitor_address=None, socket_scm_helper=None, debug=False):
+def __init__(self, binary, args=[], wrapper=[], name=None,
+ test_dir="/var/tmp", monitor_address=None,
+ socket_scm_helper=None, debug=False):
+'''
+Create a QEMUMachine object
+
+@param binary: path to the qemu binary (str)
+@param args: initial list of extra arguments
+@param wrapper: list of arguments used as prefix to qemu binary
+@param name: name of this object (used for log/monitor/... file names)
+@param test_dir: base location to put log/monitor/... files in
+@param monitor_address: custom address for QMP monitor
+@param socket_scm_helper: path to scm_helper binary (to forward fds)
+@param debug: enable debug mode (forwarded to QMP helper and such)
+@note: Qemu process is not started until launch() is used.
+'''
 if name is None:
 name = "qemu-%d" % os.getpid()
 if monitor_address is None:
@@ -33,12 +47,13 @@ class QEMUMachine(object):
 self._qemu_log_path = os.path.join(test_dir, name + ".log")
 self._popen = None
 self._binary = binary
-self._args = list(args) # Force copy args in case we modify them
+self._args = list(args) # Force copy args in case we modify them
 self._wrapper = wrapper
 self._events = []
 self._iolog = None
 self._socket_scm_helper = socket_scm_helper
 self._debug = debug
+self._qmp = None
 
 # This can be used to add an unused monitor instance.
 def add_monitor_telnet(self, ip, port):
@@ -64,16 +79,16 @@ class QEMUMachine(object):
 if self._socket_scm_helper is None:
 print >>sys.stderr, "No path to socket_scm_helper set"
 return -1
-if os.path.exists(self._socket_scm_helper) == False:
+if not os.path.exists(self._socket_scm_helper):
 print >>sys.stderr, "%s does not exist" % self._socket_scm_helper
 return -1
 fd_param = ["%s" % self._socket_scm_helper,
 "%d" % self._qmp.get_sock_fd(),
 "%s" % fd_file_path]
 devnull = open('/dev/null', 'rb')
-p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
- stderr=sys.stderr)
-return p.wait()
+proc = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
+stderr=sys.stderr)
+return proc.wait()
 
 @staticmethod
 def _remove_if_exists(path):
@@ -99,8 +114,8 @@ class QEMUMachine(object):
 return self._popen.pid
 
 def _load_io_log(self):
-with open(self._qemu_log_path, "r") as fh:
-self._iolog = fh.read()
+with open(self._qemu_log_path, "r") as iolog:
+self._iolog = iolog.read()
 
 def _base_args(self):
 if isinstance(self._monitor_address, tuple):
@@ -114,8 +129,8 @@ class QEMUMachine(object):
 '-display', 'none', '-vga', 'none']
 
 def _pre_launch(self):
-self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, 
server=True,
-debug=self._debug)
+self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
+server=True, debug=self._debug)
 
 def _post_launch(self):
 self._qmp.accept()
@@ -131,9 +146,11 @@ class QEMUMachine(object):
 qemulog = open(self._qemu_log_path, 'wb')
 try:
 self._pre_launch()
-args = self._wrapper + [self._binary] + self._base_args() + 
self._args
+args = (self._wrapper + [self._binary] + self._base_args() +
+self._args)
 self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,
-   stderr=subprocess.STDOUT, 
shell=False)
+   stderr=subprocess.STDOUT,
+   shell=False)
 self._post_launch()
 except:
 if self.is_running():
@@ -149,28 +166,34 @@ class QEMUMachine(object):
 try:
 self._qmp.cmd('quit')
 self._qmp.close()
-except:
+except: # kill VM on any failure pylint: disable=W0702
 

[Qemu-devel] [PATCH v5 02/10] qemu|qtest: Avoid dangerous arguments

[Lukáš Doktor]

The list object is mutable in python and potentially might modify other
object's arguments when used as default argument. Reproducer:

>>> vm1 = QEMUMachine("qemu")
>>> vm2 = QEMUMachine("qemu")
>>> vm1._wrapper.append("foo")
>>> print vm2._wrapper
['foo']

In this case the `args` is actually copied so it would be safe to keep
it, but it's not a good practice to keep it. The same issue applies in
inherited qtest module.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Reviewed-by: John Snow 
---
 scripts/qemu.py  | 6 +-
 scripts/qtest.py | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 466aaab..888f5b3 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -23,7 +23,7 @@ import qmp.qmp
 class QEMUMachine(object):
 '''A QEMU VM'''
 
-def __init__(self, binary, args=[], wrapper=[], name=None,
+def __init__(self, binary, args=None, wrapper=None, name=None,
  test_dir="/var/tmp", monitor_address=None,
  socket_scm_helper=None, debug=False):
 '''
@@ -39,6 +39,10 @@ class QEMUMachine(object):
 @param debug: enable debug mode (forwarded to QMP helper and such)
 @note: Qemu process is not started until launch() is used.
 '''
+if args is None:
+args = []
+if wrapper is None:
+wrapper = []
 if name is None:
 name = "qemu-%d" % os.getpid()
 if monitor_address is None:
diff --git a/scripts/qtest.py b/scripts/qtest.py
index d5aecb5..ab183c0 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -79,7 +79,7 @@ class QEMUQtestProtocol(object):
 class QEMUQtestMachine(qemu.QEMUMachine):
 '''A QEMU VM'''
 
-def __init__(self, binary, args=[], name=None, test_dir="/var/tmp",
+def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
  socket_scm_helper=None):
 if name is None:
 name = "qemu-%d" % os.getpid()
-- 
2.9.4

[Qemu-devel] [PATCH v5 03/10] qemu.py: Use iteritems rather than keys()

[Lukáš Doktor]

Let's avoid creating an in-memory list of keys and query for each value
and use `iteritems` which is an iterator of key-value pairs.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Philippe Mathieu-Daudé 
---
 scripts/qemu.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 888f5b3..571f852 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -185,11 +185,11 @@ class QEMUMachine(object):
 def qmp(self, cmd, conv_keys=True, **args):
 '''Invoke a QMP command and return the result dict'''
 qmp_args = dict()
-for key in args.keys():
+for key, value in args.iteritems():
 if conv_keys:
-qmp_args[key.translate(self.underscore_to_dash)] = args[key]
+qmp_args[key.translate(self.underscore_to_dash)] = value
 else:
-qmp_args[key] = args[key]
+qmp_args[key] = value
 
 return self._qmp.cmd(cmd, args=qmp_args)
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH] hw/acpi: Select an node with memory for mapping memory hole to

[no-reply]

Hi,

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

Message-id: 1502787511-20894-1-git-send-email-douly.f...@cn.fujitsu.com
Subject: [Qemu-devel] [PATCH] hw/acpi: Select an node with memory for mapping 
memory hole to
Type: series

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

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

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
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/20170815085732.9794-1-ldok...@redhat.com -> 
patchew/20170815085732.9794-1-ldok...@redhat.com
Switched to a new branch 'test'
8d4d56ba54 hw/acpi: Select an node with memory for mapping memory hole to

=== OUTPUT BEGIN ===
Checking PATCH 1/1: hw/acpi: Select an node with memory for mapping memory hole 
to...
ERROR: trailing statements should be on next line
#88: FILE: hw/i386/acpi-build.c:2405:
+for (node = 0;
[...]
+node++);

total: 1 errors, 0 warnings, 99 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

[Qemu-devel] [RFC PATCH for 2.11] tests: Introduce generic device hot-plug/hot-unplug functions

[Thomas Huth]

A lot of tests provide code for adding and removing a device via the
device_add and device_del QMP commands. Maintaining this code in so
many places is cumbersome and error-prone (some of the code parts
check the responses in an incorrect way, for example), so let's
provide some proper generic qtest functions for adding and removing a
device instead.

Signed-off-by: Thomas Huth 
---
 I'm planning to use qtest_hot_plug_device() in a bigger patch series
 that I'm currently preparing. But since this clean-up patch is valid on
 its own already, I'd like to get some feedback (or even a Reviewed-by)
 for this patch here already if possible...

 tests/libqos/pci.c | 19 ++-
 tests/libqos/usb.c | 30 +--
 tests/libqtest.c   | 60 ++
 tests/libqtest.h   | 19 +++
 tests/usb-hcd-uhci-test.c  | 26 ++--
 tests/usb-hcd-xhci-test.c  | 52 
 tests/virtio-scsi-test.c   | 24 ++-
 tests/virtio-serial-test.c | 25 +++
 8 files changed, 98 insertions(+), 157 deletions(-)

diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
index 2dcdead..aada753 100644
--- a/tests/libqos/pci.c
+++ b/tests/libqos/pci.c
@@ -394,21 +394,6 @@ QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr)
 void qpci_plug_device_test(const char *driver, const char *id,
uint8_t slot, const char *opts)
 {
-QDict *response;
-char *cmd;
-
-cmd = g_strdup_printf("{'execute': 'device_add',"
-  " 'arguments': {"
-  "   'driver': '%s',"
-  "   'addr': '%d',"
-  "   %s%s"
-  "   'id': '%s'"
-  "}}", driver, slot,
-  opts ? opts : "", opts ? "," : "",
-  id);
-response = qmp(cmd);
-g_free(cmd);
-g_assert(response);
-g_assert(!qdict_haskey(response, "error"));
-QDECREF(response);
+qtest_hot_plug_device(driver, id, "'addr': '%d'%s%s", slot,
+  opts ? ", " : "", opts ? opts : "");
 }
diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c
index 0cdfaec..f8d0190 100644
--- a/tests/libqos/usb.c
+++ b/tests/libqos/usb.c
@@ -40,34 +40,16 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t 
expect)
 void usb_test_hotplug(const char *hcd_id, const int port,
   void (*port_check)(void))
 {
-QDict *response;
-char  *cmd;
+char  *id = g_strdup_printf("usbdev%d", port);
 
-cmd = g_strdup_printf("{'execute': 'device_add',"
-  " 'arguments': {"
-  "   'driver': 'usb-tablet',"
-  "   'port': '%d',"
-  "   'bus': '%s.0',"
-  "   'id': 'usbdev%d'"
-  "}}", port, hcd_id, port);
-response = qmp(cmd);
-g_free(cmd);
-g_assert(response);
-g_assert(!qdict_haskey(response, "error"));
-QDECREF(response);
+qtest_hot_plug_device("usb-tablet", id, "'port': '%d', 'bus': '%s.0'",
+  port, hcd_id);
 
 if (port_check) {
 port_check();
 }
 
-cmd = g_strdup_printf("{'execute': 'device_del',"
-   " 'arguments': {"
-   "   'id': 'usbdev%d'"
-   "}}", port);
-response = qmp(cmd);
-g_free(cmd);
-g_assert(response);
-g_assert(qdict_haskey(response, "event"));
-g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
-QDECREF(response);
+qtest_hot_unplug_device(id);
+
+g_free(id);
 }
diff --git a/tests/libqtest.c b/tests/libqtest.c
index b9a1f18..4339d97 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -987,3 +987,63 @@ void qtest_cb_for_every_machine(void (*cb)(const char 
*machine))
 qtest_end();
 QDECREF(response);
 }
+
+/**
+ * Generic hot-plugging test via the device_add QMP command
+ */
+void qtest_hot_plug_device(const char *driver, const char *id,
+   const char *fmt, ...)
+{
+QDict *response;
+char *cmd, *opts = NULL;
+va_list va;
+
+if (fmt) {
+va_start(va, fmt);
+opts = g_strdup_vprintf(fmt, va);
+va_end(va);
+}
+
+cmd = g_strdup_printf("{'execute': 'device_add',"
+  " 'arguments': { 'driver': '%s', 'id': '%s'%s%s }}",
+  driver, id, opts ? ", " : "", opts ? opts : "");
+g_free(opts);
+
+response = qmp(cmd);
+g_free(cmd);
+g_assert(response);
+while (qdict_haskey(response, "event")) {
+/* We can get DEVICE_DELETED events in case something went wrong */
+g_assert_cmpstr(qdict_get_str(response, "event"), !=, 
"DEVICE_DELETED");
+QDECREF(response);
+response = qmp("");
+  

Re: [Qemu-devel] [RFC 5/6] migration: store listen task tag

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 04:50:06PM +0800, Peter Xu wrote:
> On Tue, Aug 15, 2017 at 09:37:14AM +0100, Daniel P. Berrange wrote:
> > On Tue, Aug 15, 2017 at 02:17:06PM +0800, Peter Xu wrote:
> > > Store the task tag for migration types: tcp/unix/fd/exec in current
> > > MigrationIncomingState struct.
> > > 
> > > For defered migration, no need to store task tag since there is no task
> > > running in the main loop at all. For RDMA, let's mark it as todo.
> > > 
> > > Signed-off-by: Peter Xu 
> > > ---
> > >  migration/migration.c | 22 ++
> > >  migration/migration.h |  2 ++
> > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index c9b7085..daf356b 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -171,6 +171,7 @@ void migration_incoming_state_destroy(void)
> > >  mis->from_src_file = NULL;
> > >  }
> > >  
> > > +mis->listen_task_tag = 0;
> > >  qemu_event_destroy(&mis->main_thread_load_event);
> > >  }
> > >  
> > > @@ -265,25 +266,31 @@ int 
> > > migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname,
> > >  void qemu_start_incoming_migration(const char *uri, Error **errp)
> > >  {
> > >  const char *p;
> > > +guint task_tag = 0;
> > > +MigrationIncomingState *mis = migration_incoming_get_current();
> > >  
> > >  qapi_event_send_migration(MIGRATION_STATUS_SETUP, &error_abort);
> > >  if (!strcmp(uri, "defer")) {
> > >  deferred_incoming_migration(errp);
> > >  } else if (strstart(uri, "tcp:", &p)) {
> > > -tcp_start_incoming_migration(p, errp);
> > > +task_tag = tcp_start_incoming_migration(p, errp);
> > >  #ifdef CONFIG_RDMA
> > >  } else if (strstart(uri, "rdma:", &p)) {
> > > +/* TODO: store task tag for RDMA migrations */
> > >  rdma_start_incoming_migration(p, errp);
> > >  #endif
> > >  } else if (strstart(uri, "exec:", &p)) {
> > > -exec_start_incoming_migration(p, errp);
> > > +task_tag = exec_start_incoming_migration(p, errp);
> > >  } else if (strstart(uri, "unix:", &p)) {
> > > -unix_start_incoming_migration(p, errp);
> > > +task_tag = unix_start_incoming_migration(p, errp);
> > >  } else if (strstart(uri, "fd:", &p)) {
> > > -fd_start_incoming_migration(p, errp);
> > > +task_tag = fd_start_incoming_migration(p, errp);
> > >  } else {
> > >  error_setg(errp, "unknown migration protocol: %s", uri);
> > > +return;
> > >  }
> > > +
> > > +mis->listen_task_tag = task_tag;
> > 
> > This is unsafe as currently written. The main loop watches that are
> > associated with these tags are all unregistered when incoming migration
> > starts. So if you save them, and then unregister later when postcopy
> > is "stuck", then you're likely unregistering a tag associated with
> > some other random part of QEMU, because the saved value is no longer
> > valid.
> 
> IIUC for incoming migration, the watch is not detached until
> migration_fd_process_incoming() completes. So:
> 
> - for precopy, the watch should be detached when migration completes
> 
> - for postcopy, the watch should be detached when postcopy starts,
>   then main loop thread returns, while the ram_load_thread starts to
>   continue the migration.
> 
> So basically the watch is detaching itself after
> migration_fd_process_incoming() completes. To handle that, this patch
> has this change:
> 
> @@ -422,6 +429,13 @@ void migration_fd_process_incoming(QEMUFile *f)
>  co = qemu_coroutine_create(process_incoming_migration_co, f);
>  qemu_coroutine_enter(co);
>  }
> +
> +/*
> + * When reach here, we should not need the listening port any
> + * more. We'll detach the listening task soon, let's reset the
> + * listen task tag.
> + */
> +mis->listen_task_tag = 0;
> 
> Would this make sure the listen_task_tag is always valid if nonzero?

Mixing 'return FALSE' from callbacks, with g_source_remove is a recipe
for hard to diagnose bugs, so should be avoided in general.

So, IMHO, you should change all the callbacks to 'return TRUE' so that
they keep the watch registered, and then explicitly call g_source_remove()
passing the listen tag, when incoming migration starts.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] Makefile: Let "make check-help" work without running ./configure

[Alex Bennée]

Fam Zheng  writes:

> On Thu, 08/10 08:59, Philippe Mathieu-Daudé wrote:
>> Hi Fam,
>>
>> thank for fixing this :)
>>
>> I think as a bugfix it should enter 2.10
>> (maybe through Alex's Travis series, CC'ing him)
>
> Since Alex hasn't replied, I'll send a pull request for -rc3.

Yes please. Sorry I got distracted by other stuff.

>
>>
>> On 08/10/2017 05:50 AM, Fam Zheng wrote:
>> > Currently if you do "make check-help" in a fresh checkout, only an error
>> > is printed which is not nice:
>> >
>> >  $ make check-help V=1
>> >  cc -nostdlib  -o check-help.mo
>> >  cc: fatal error: no input files
>> >  compilation terminated.
>> >  rules.mak:115: recipe for target 'check-help.mo' failed
>> >  make: *** [check-help.mo] Error 1
>> >
>> > Move the config-host.mak condition into the body of
>> > tests/Makefile.include and always include the rule for check-help.
>> >
>> > Reported-by: Philippe Mathieu-Daudé 
>> > Signed-off-by: Fam Zheng 
>>
>> Reviewed-by: Philippe Mathieu-Daudé 
>> Tested-by: Philippe Mathieu-Daudé 
>>
>
> Fam


--
Alex Bennée

[Qemu-devel] [PATCH v4 00/12] Convert over to use keycodemapdb

[Daniel P. Berrange]

An update of:

  v1: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02047.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02471.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02517.html

The keycodemap project[1] provides a database mapping between
many different keysym/keycode/scancode sets, along with a
tool to generate mapping/lookup tables in various programming
languages. It is already used by GTK-VNC, SPICE-GTK and
libvirt.

This series enables its use in QEMU, thus fixing a great
many bugs/ommissions in the 15+ key mapping tables people
have manually written for QEMU.

The keycodemapdb code is designed to be used as a git
sub-module, it is not an external dependancy you need
installed before use.

This series converts all the front ends and all the input
devices which are using the new InputEvent framework. A
handful of devices still use the legacy kbd handler

  $ git grep -l add_kbd_event_handler hw
  hw/arm/musicpal.c
  hw/arm/nseries.c
  hw/arm/palm.c
  hw/arm/spitz.c
  hw/input/pxa2xx_keypad.c
  hw/input/stellaris_input.c

and could be usefully converted too.

I've not done much realworld testing of this yet. I did
however write code that compared the mapping tables before
and after conversion to identify what mapping changes have
resulted in each frontend/backend.  What I still need to
go back and validate is the Print/Sysrq handling, because
that is special everywhere and I'm not entirely sure I've
done that correctly yet. The GTK frontend should now work
correctly when run on X11 servers on Win32 and OS-X, as
well as when run on native Win32/OS-X display backends.

[1] https://gitlab.com/keycodemap/keycodemapdb/

Changed in v4:

 - Run submodule update in source_dir for vpath builds (patchew)
 - Force submodule update in docker rules in case they
   are run without configure (patchew)

Changed in v3:

 - Ensure docker builds pull in keycodemapdb submodule (patchew)
 - Add compat with py26 for RHEL-6 in keycodemapdb tools (patchew)
 - Initialize submodule in configure script (patchew)

Changed in v2:

 - Change filename pattern to 'ui/input-keymap-$SRC-to-$DST.c'
   and map names 'qemu_input_map_$SRC_to_$DST'  (Eric)
 - Fix typos (Eric)
 - Drop changes to InputKeyEvent struct (Eric)
 - Fix VPATH build (patchew)
 - Fix code style errors (patchew)


Daniel P. Berrange (12):
  ui: add keycodemapdb repository as a GIT submodule
  ui: convert common input code to keycodemapdb
  ui: convert key events to QKeyCodes immediately
  ui: don't export qemu_input_event_new_key
  input: convert virtio-input-hid device to keycodemapdb
  input: convert ps2 device to keycodemapdb
  input: convert the adb device to keycodemapdb
  char: convert the escc device to keycodemapdb
  ui: convert cocoa frontend to keycodemapdb
  ui: convert the SDL2 frontend to keycodemapdb
  ui: convert GTK and SDL1 frontends to keycodemapdb
  display: convert XenInput keyboard to keycodemapdb

 .gitignore|   2 +
 .gitmodules   |   3 +
 configure |   2 +
 hw/char/escc.c| 126 +
 hw/display/xenfb.c| 133 --
 hw/input/adb.c| 124 +
 hw/input/ps2.c| 406 +-
 hw/input/virtio-input-hid.c   | 136 +-
 include/hw/input/adb-keys.h   | 141 ---
 include/ui/input.h|  57 +-
 tests/docker/Makefile.include |  11 +-
 tests/docker/run  |   4 +-
 ui/Makefile.objs  |  41 -
 ui/cocoa.m| 129 +-
 ui/gtk.c  | 205 -
 ui/input-keymap.c | 341 ---
 ui/input.c|   8 +-
 ui/keycodemapdb   |   1 +
 ui/sdl.c  | 105 ---
 ui/sdl2-input.c   |  16 +-
 ui/sdl2-keymap.h  | 267 ---
 ui/trace-events   |   9 +-
 ui/x_keymap.c | 250 ++
 ui/x_keymap.h |   8 +-
 24 files changed, 473 insertions(+), 2052 deletions(-)
 delete mode 100644 include/hw/input/adb-keys.h
 create mode 16 ui/keycodemapdb
 delete mode 100644 ui/sdl2-keymap.h

-- 
2.13.3

[Qemu-devel] [PATCH v4 02/12] ui: convert common input code to keycodemapdb

[Daniel P. Berrange]

Replace the number_to_qcode, qcode_to_number and linux_to_qcode
tables with automatically generated tables.

Missing entries in linux_to_qcode now fixed:

  KEY_LINEFEED -> Q_KEY_CODE_LF
  KEY_KPEQUAL -> Q_KEY_CODE_KP_EQUALS
  KEY_COMPOSE -> Q_KEY_CODE_COMPOSE
  KEY_AGAIN -> Q_KEY_CODE_AGAIN
  KEY_PROPS -> Q_KEY_CODE_PROPS
  KEY_UNDO -> Q_KEY_CODE_UNDO
  KEY_FRONT -> Q_KEY_CODE_FRONT
  KEY_COPY -> Q_KEY_CODE_COPY
  KEY_OPEN -> Q_KEY_CODE_OPEN
  KEY_PASTE -> Q_KEY_CODE_PASTE
  KEY_CUT -> Q_KEY_CODE_CUT
  KEY_HELP -> Q_KEY_CODE_HELP
  KEY_MEDIA -> Q_KEY_CODE_MEDIASELECT

In additionsome fixes:

 - KEY_PLAYPAUSE now maps to Q_KEY_CODE_AUDIOPLAY, instead of
   KEY_PLAYCD. KEY_PLAYPAUSE is defined across almost all scancodes
   sets, while KEY_PLAYCD only appears in AT set1, so the former is
   a more useful mapping.

Missing entries in qcode_to_number now fixed:

  Q_KEY_CODE_AGAIN -> 0x85
  Q_KEY_CODE_PROPS -> 0x86
  Q_KEY_CODE_UNDO -> 0x87
  Q_KEY_CODE_FRONT -> 0x8c
  Q_KEY_CODE_COPY -> 0xf8
  Q_KEY_CODE_OPEN -> 0x64
  Q_KEY_CODE_PASTE -> 0x65
  Q_KEY_CODE_CUT -> 0xbc
  Q_KEY_CODE_LF -> 0x5b
  Q_KEY_CODE_HELP -> 0xf5
  Q_KEY_CODE_COMPOSE -> 0xdd
  Q_KEY_CODE_KP_EQUALS -> 0x59
  Q_KEY_CODE_MEDIASELECT -> 0xed

In addition some fixes:

 - Q_KEY_CODE_MENU was incorrectly mapped to the compose
   scancode (0xdd) and is now mapped to 0x9e
 - Q_KEY_CODE_FIND was mapped to 0xe065 (Search) instead
   of to 0xe041 (Find)
 - Q_KEY_CODE_HIRAGANA was mapped to 0x70 (Katakanahiragana)
   instead of of 0x77 (Hirigana)
 - Q_KEY_CODE_PRINT was mapped to 0xb7 which is not a defined
   scan code in AT set 1, it is now mapped to 0x54 (sysrq)

Signed-off-by: Daniel P. Berrange 
---
 include/ui/input.h |  11 +-
 ui/Makefile.objs   |   3 +
 ui/input-keymap.c  | 326 +++--
 3 files changed, 28 insertions(+), 312 deletions(-)

diff --git a/include/ui/input.h b/include/ui/input.h
index c488585def..479cc46cfc 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -43,7 +43,7 @@ void qemu_input_event_send_key(QemuConsole *src, KeyValue 
*key, bool down);
 void qemu_input_event_send_key_number(QemuConsole *src, int num, bool down);
 void qemu_input_event_send_key_qcode(QemuConsole *src, QKeyCode q, bool down);
 void qemu_input_event_send_key_delay(uint32_t delay_ms);
-int qemu_input_key_number_to_qcode(uint8_t nr);
+int qemu_input_key_number_to_qcode(unsigned int nr);
 int qemu_input_key_value_to_number(const KeyValue *value);
 int qemu_input_key_value_to_qcode(const KeyValue *value);
 int qemu_input_key_value_to_scancode(const KeyValue *value, bool down,
@@ -69,4 +69,13 @@ void qemu_input_check_mode_change(void);
 void qemu_add_mouse_mode_change_notifier(Notifier *notify);
 void qemu_remove_mouse_mode_change_notifier(Notifier *notify);
 
+extern const guint qemu_input_map_linux_to_qcode_len;
+extern const guint16 qemu_input_map_linux_to_qcode[];
+
+extern const guint qemu_input_map_qcode_to_qnum_len;
+extern const guint16 qemu_input_map_qcode_to_qnum[];
+
+extern const guint qemu_input_map_qnum_to_qcode_len;
+extern const guint16 qemu_input_map_qnum_to_qcode[];
+
 #endif /* INPUT_H */
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 6796a9f98b..8c357b488f 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -54,6 +54,9 @@ KEYCODEMAP_GEN = $(SRC_PATH)/ui/keycodemapdb/tools/keymap-gen
 KEYCODEMAP_CSV = $(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv
 
 KEYCODEMAP_FILES = \
+ui/input-keymap-linux-to-qcode.c \
+ui/input-keymap-qcode-to-qnum.c \
+ui/input-keymap-qnum-to-qcode.c \
 $(NULL)
 
 GENERATED_FILES += $(KEYCODEMAP_FILES)
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index cf979c2ce9..3a19a169f5 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -5,333 +5,37 @@
 
 #include "standard-headers/linux/input.h"
 
-static int linux_to_qcode[KEY_CNT] = {
-[KEY_ESC]= Q_KEY_CODE_ESC,
-[KEY_1]  = Q_KEY_CODE_1,
-[KEY_2]  = Q_KEY_CODE_2,
-[KEY_3]  = Q_KEY_CODE_3,
-[KEY_4]  = Q_KEY_CODE_4,
-[KEY_5]  = Q_KEY_CODE_5,
-[KEY_6]  = Q_KEY_CODE_6,
-[KEY_7]  = Q_KEY_CODE_7,
-[KEY_8]  = Q_KEY_CODE_8,
-[KEY_9]  = Q_KEY_CODE_9,
-[KEY_0]  = Q_KEY_CODE_0,
-[KEY_MINUS]  = Q_KEY_CODE_MINUS,
-[KEY_EQUAL]  = Q_KEY_CODE_EQUAL,
-[KEY_BACKSPACE]  = Q_KEY_CODE_BACKSPACE,
-[KEY_TAB]= Q_KEY_CODE_TAB,
-[KEY_Q]  = Q_KEY_CODE_Q,
-[KEY_W]  = Q_KEY_CODE_W,
-[KEY_E]  = Q_KEY_CODE_E,
-[KEY_R]  = Q_KEY_CODE_R,
-[KEY_T]  = Q_KEY_CODE_T,
-[KEY_Y]  = Q_KEY_CODE_Y,
-[KEY_U]  = Q_KEY_CODE_U,
-[KEY_I]  = Q_KEY_CODE_I,
-[KEY_O]  = Q_KEY_CODE_O,
-[KEY_P]  = Q_KEY_CODE_P,
-   

[Qemu-devel] [PATCH v4 01/12] ui: add keycodemapdb repository as a GIT submodule

[Daniel P. Berrange]

The https://gitlab.com/keycodemap/keycodemapdb/ repo contains a
data file mapping between all the different scancode/keycode/keysym
sets that are known, and a tool to auto-generate lookup tables for
different combinations.

It is used by GTK-VNC, SPICE-GTK and libvirt for mapping keys.
Using it in QEMU will let us replace many hand written lookup
tables with auto-generated tables from a master data source,
reducing bugs. Adding new QKeyCodes will now only require the
master table to be updated, all ~20 other tables will be
automatically updated to follow.

Signed-off-by: Daniel P. Berrange 
---
 .gitignore|  2 ++
 .gitmodules   |  3 +++
 configure |  2 ++
 tests/docker/Makefile.include | 11 +++
 tests/docker/run  |  4 +++-
 ui/Makefile.objs  | 18 ++
 ui/keycodemapdb   |  1 +
 7 files changed, 36 insertions(+), 5 deletions(-)
 create mode 16 ui/keycodemapdb

diff --git a/.gitignore b/.gitignore
index cf65316863..6e5a1202c8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,6 +14,8 @@
 /trace/generated-tcg-tracers.h
 /ui/shader/texture-blit-frag.h
 /ui/shader/texture-blit-vert.h
+/ui/keycodemap_*.c
+/ui/input-keymap-*.c
 *-timestamp
 /*-softmmu
 /*-darwin-user
diff --git a/.gitmodules b/.gitmodules
index 5b0c212622..369989f19e 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -37,3 +37,6 @@
 [submodule "roms/QemuMacDrivers"]
path = roms/QemuMacDrivers
url = git://git.qemu.org/QemuMacDrivers.git
+[submodule "ui/keycodemapdb"]
+   path = ui/keycodemapdb
+   url = https://gitlab.com/keycodemap/keycodemapdb.git
diff --git a/configure b/configure
index dd73cce62f..bd373ec2b4 100755
--- a/configure
+++ b/configure
@@ -5258,6 +5258,8 @@ echo_version() {
 fi
 }
 
+(cd $source_path && git submodule update --init ui/keycodemapdb)
+
 # prepend pixman and ftd flags after all config tests are done
 QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
 libs_softmmu="$pixman_libs $libs_softmmu"
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index aaab1a4208..2920edb79a 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -18,21 +18,24 @@ TESTS ?= %
 IMAGES ?= %
 
 # Make archive from git repo $1 to tar.gz $2
-make-archive-maybe = $(if $(wildcard $1/*), \
-   $(call quiet-command, \
+make-archive = $(call quiet-command, \
(cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \
git archive -1 HEAD --format=tar.gz; \
else \
git archive -1 $$(git stash create) --format=tar.gz; \
fi) > $2, \
-   "ARCHIVE","$(notdir $2)"))
+   "ARCHIVE","$(notdir $2)")
+make-archive-maybe = $(if $(wildcard $1/*), \
+   $(call make-archive, $1, $2))
 
 CUR_TIME := $(shell date +%Y-%m-%d-%H.%M.%S.)
 DOCKER_SRC_COPY := docker-src.$(CUR_TIME)
 
 $(DOCKER_SRC_COPY):
@mkdir $@
-   $(call make-archive-maybe, $(SRC_PATH), $@/qemu.tgz)
+   $(call quiet-command, git submodule update --init 
$(SRC_PATH)/ui/keycodemapdb, "GIT", "ui/keycodemapdb")
+   $(call make-archive, $(SRC_PATH), $@/qemu.tgz)
+   $(call make-archive, $(SRC_PATH)/ui/keycodemapdb, $@/keycodemapdb.tgz)
$(call make-archive-maybe, $(SRC_PATH)/dtc, $@/dtc.tgz)
$(call make-archive-maybe, $(SRC_PATH)/pixman, $@/pixman.tgz)
$(call quiet-command, cp $(SRC_PATH)/tests/docker/run $@/run, \
diff --git a/tests/docker/run b/tests/docker/run
index c1e4513bce..c30352df4a 100755
--- a/tests/docker/run
+++ b/tests/docker/run
@@ -33,13 +33,15 @@ mkdir -p $TEST_DIR/{src,build,install}
 
 # Extract the source tarballs
 tar -C $TEST_DIR/src -xzf $BASE/qemu.tgz
-for p in dtc pixman; do
+for p in dtc pixman ; do
 if test -f $BASE/$p.tgz; then
 tar -C $TEST_DIR/src/$p -xzf $BASE/$p.tgz
 export FEATURES="$FEATURES $p"
 fi
 done
 
+tar -C $TEST_DIR/src/ui/keycodemapdb -xzf $BASE/keycodemapdb.tgz
+
 if test -n "$SHOW_ENV"; then
 if test -f /packages.txt; then
 echo "Packages installed:"
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 3369451285..6796a9f98b 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -49,3 +49,21 @@ gtk-egl.o-libs += $(OPENGL_LIBS)
 shader.o-libs += $(OPENGL_LIBS)
 console-gl.o-libs += $(OPENGL_LIBS)
 egl-helpers.o-libs += $(OPENGL_LIBS)
+
+KEYCODEMAP_GEN = $(SRC_PATH)/ui/keycodemapdb/tools/keymap-gen
+KEYCODEMAP_CSV = $(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv
+
+KEYCODEMAP_FILES = \
+$(NULL)
+
+GENERATED_FILES += $(KEYCODEMAP_FILES)
+
+ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) 
$(SRC_PATH)/ui/Makefile.objs
+   $(call quiet-command,\
+   src=$$(echo $@ | sed -E -e 
"s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\1,") && \
+   dst=$$(echo $@ | sed -E -e 
"s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\2,") && \
+   $(PYTHON) 

[Qemu-devel] [PATCH v4 06/12] input: convert ps2 device to keycodemapdb

[Daniel P. Berrange]

Replace the qcode_to_keycode_set1, qcode_to_keycode_set2,
and qcode_to_keycode_set3 tables with automatically
generated tables.

Missing entries in qcode_to_keycode_set1 now fixed:

 - Q_KEY_CODE_SYSRQ -> 0x54
 - Q_KEY_CODE_PRINT -> 0x54 (NB ignored due to special case)
 - Q_KEY_CODE_AGAIN -> 0xe005
 - Q_KEY_CODE_PROPS -> 0xe006
 - Q_KEY_CODE_UNDO -> 0xe007
 - Q_KEY_CODE_FRONT -> 0xe00c
 - Q_KEY_CODE_COPY -> 0xe078
 - Q_KEY_CODE_OPEN -> 0x64
 - Q_KEY_CODE_PASTE -> 0x65
 - Q_KEY_CODE_CUT -> 0xe03c
 - Q_KEY_CODE_LF -> 0x5b
 - Q_KEY_CODE_HELP -> 0xe075
 - Q_KEY_CODE_COMPOSE -> 0xe05d
 - Q_KEY_CODE_PAUSE -> 0xe046
 - Q_KEY_CODE_KP_EQUALS -> 0x59

And some mistakes corrected:

 - Q_KEY_CODE_HIRAGANA was mapped to 0x70 (Katakanahiragana)
   instead of of 0x77 (Hirigana)
 - Q_KEY_CODE_MENU was incorrectly mapped to the compose
   scancode (0xe05d) and is now mapped to 0xe01e
 - Q_KEY_CODE_FIND was mapped to 0xe065 (Search) instead
   of to 0xe041 (Find)
 - Q_KEY_CODE_POWER, SLEEP & WAKE had 0x0e instead of 0xe0
   as the prefix

Missing entries in qcode_to_keycode_set2 now fixed:

 - Q_KEY_CODE_PRINT -> 0x7f (NB ignored due to special case)
 - Q_KEY_CODE_COMPOSE -> 0xe02f
 - Q_KEY_CODE_PAUSE -> 0xe077
 - Q_KEY_CODE_KP_EQUALS -> 0x0f

And some mistakes corrected:

 - Q_KEY_CODE_HIRAGANA was mapped to 0x13 (Katakanahiragana)
   instead of of 0x62 (Hirigana)
 - Q_KEY_CODE_MENU was incorrectly mapped to the compose
   scancode (0xe02f) and is now not mapped
 - Q_KEY_CODE_FIND was mapped to 0xe010 (Search) and is now
   not mapped.
 - Q_KEY_CODE_POWER, SLEEP & WAKE had 0x0e instead of 0xe0
   as the prefix

Missing entries in qcode_to_keycode_set3 now fixed:

 - Q_KEY_CODE_ASTERISK -> 0x7e
 - Q_KEY_CODE_SYSRQ -> 0x57
 - Q_KEY_CODE_LESS -> 0x13
 - Q_KEY_CODE_STOP -> 0x0a
 - Q_KEY_CODE_AGAIN -> 0x0b
 - Q_KEY_CODE_PROPS -> 0x0c
 - Q_KEY_CODE_UNDO -> 0x10
 - Q_KEY_CODE_COPY -> 0x18
 - Q_KEY_CODE_OPEN -> 0x20
 - Q_KEY_CODE_PASTE -> 0x28
 - Q_KEY_CODE_FIND -> 0x30
 - Q_KEY_CODE_CUT -> 0x38
 - Q_KEY_CODE_HELP -> 0x09
 - Q_KEY_CODE_COMPOSE -> 0x8d
 - Q_KEY_CODE_AUDIONEXT -> 0x93
 - Q_KEY_CODE_AUDIOPREV -> 0x94
 - Q_KEY_CODE_AUDIOSTOP -> 0x98
 - Q_KEY_CODE_AUDIOMUTE -> 0x9c
 - Q_KEY_CODE_VOLUMEUP -> 0x95
 - Q_KEY_CODE_VOLUMEDOWN -> 0x9d
 - Q_KEY_CODE_CALCULATOR -> 0xa3
 - Q_KEY_CODE_AC_HOME -> 0x97

And some mistakes corrected:

 - Q_KEY_CODE_MENU was incorrectly mapped to the compose
   scancode (0x8d) and is now 0x91

Signed-off-by: Daniel P. Berrange 
---
 hw/input/ps2.c | 406 +
 include/ui/input.h |   9 ++
 ui/Makefile.objs   |   3 +
 ui/input-keymap.c  |   3 +
 4 files changed, 22 insertions(+), 399 deletions(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 77906d5f46..7eeadc144d 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -115,401 +115,6 @@ typedef struct {
 uint8_t mouse_buttons;
 } PS2MouseState;
 
-/* Table to convert from QEMU codes to scancodes.  */
-static const uint16_t qcode_to_keycode_set1[Q_KEY_CODE__MAX] = {
-[0 ... Q_KEY_CODE__MAX - 1] = 0,
-
-[Q_KEY_CODE_A] = 0x1e,
-[Q_KEY_CODE_B] = 0x30,
-[Q_KEY_CODE_C] = 0x2e,
-[Q_KEY_CODE_D] = 0x20,
-[Q_KEY_CODE_E] = 0x12,
-[Q_KEY_CODE_F] = 0x21,
-[Q_KEY_CODE_G] = 0x22,
-[Q_KEY_CODE_H] = 0x23,
-[Q_KEY_CODE_I] = 0x17,
-[Q_KEY_CODE_J] = 0x24,
-[Q_KEY_CODE_K] = 0x25,
-[Q_KEY_CODE_L] = 0x26,
-[Q_KEY_CODE_M] = 0x32,
-[Q_KEY_CODE_N] = 0x31,
-[Q_KEY_CODE_O] = 0x18,
-[Q_KEY_CODE_P] = 0x19,
-[Q_KEY_CODE_Q] = 0x10,
-[Q_KEY_CODE_R] = 0x13,
-[Q_KEY_CODE_S] = 0x1f,
-[Q_KEY_CODE_T] = 0x14,
-[Q_KEY_CODE_U] = 0x16,
-[Q_KEY_CODE_V] = 0x2f,
-[Q_KEY_CODE_W] = 0x11,
-[Q_KEY_CODE_X] = 0x2d,
-[Q_KEY_CODE_Y] = 0x15,
-[Q_KEY_CODE_Z] = 0x2c,
-[Q_KEY_CODE_0] = 0x0b,
-[Q_KEY_CODE_1] = 0x02,
-[Q_KEY_CODE_2] = 0x03,
-[Q_KEY_CODE_3] = 0x04,
-[Q_KEY_CODE_4] = 0x05,
-[Q_KEY_CODE_5] = 0x06,
-[Q_KEY_CODE_6] = 0x07,
-[Q_KEY_CODE_7] = 0x08,
-[Q_KEY_CODE_8] = 0x09,
-[Q_KEY_CODE_9] = 0x0a,
-[Q_KEY_CODE_GRAVE_ACCENT] = 0x29,
-[Q_KEY_CODE_MINUS] = 0x0c,
-[Q_KEY_CODE_EQUAL] = 0x0d,
-[Q_KEY_CODE_BACKSLASH] = 0x2b,
-[Q_KEY_CODE_BACKSPACE] = 0x0e,
-[Q_KEY_CODE_SPC] = 0x39,
-[Q_KEY_CODE_TAB] = 0x0f,
-[Q_KEY_CODE_CAPS_LOCK] = 0x3a,
-[Q_KEY_CODE_SHIFT] = 0x2a,
-[Q_KEY_CODE_CTRL] = 0x1d,
-[Q_KEY_CODE_META_L] = 0xe05b,
-[Q_KEY_CODE_ALT] = 0x38,
-[Q_KEY_CODE_SHIFT_R] = 0x36,
-[Q_KEY_CODE_CTRL_R] = 0xe01d,
-[Q_KEY_CODE_META_R] = 0xe05c,
-[Q_KEY_CODE_ALT_R] = 0xe038,
-[Q_KEY_CODE_MENU] = 0xe05d,
-[Q_KEY_CODE_RET] = 0x1c,
-[Q_KEY_CODE_ESC] = 0x01,
-[Q_KEY_CODE_F1] = 0x3b,
-[Q_KEY_CODE_F2] = 0x3c,
-[Q_KEY_CODE_F3] = 0x3d,
-[Q_KEY_CODE_F4] = 0x3e,
-[Q_KEY_CODE_F5] = 0x3f,
-[Q_KEY_CODE_F6] = 0x40,
-[Q_KEY_CODE_F7] = 0x41,
-[Q_KEY_CODE_F8] = 0x42,
-[Q_KEY_CODE_F9] = 0x43,
-[Q_KEY_CODE_F10] = 0x44

[Qemu-devel] [PATCH v4 04/12] ui: don't export qemu_input_event_new_key

[Daniel P. Berrange]

All public code should use qemu_input_event_send_key* functions
instead of creating an event directly.

Signed-off-by: Daniel P. Berrange 
---
 include/ui/input.h | 1 -
 ui/input.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/ui/input.h b/include/ui/input.h
index 479cc46cfc..f8cee43f65 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -38,7 +38,6 @@ void qemu_input_event_send_impl(QemuConsole *src, InputEvent 
*evt);
 void qemu_input_event_sync(void);
 void qemu_input_event_sync_impl(void);
 
-InputEvent *qemu_input_event_new_key(KeyValue *key, bool down);
 void qemu_input_event_send_key(QemuConsole *src, KeyValue *key, bool down);
 void qemu_input_event_send_key_number(QemuConsole *src, int num, bool down);
 void qemu_input_event_send_key_qcode(QemuConsole *src, QKeyCode q, bool down);
diff --git a/ui/input.c b/ui/input.c
index 64e9103a61..ba85bf01a9 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -374,7 +374,7 @@ void qemu_input_event_sync(void)
 replay_input_sync_event();
 }
 
-InputEvent *qemu_input_event_new_key(KeyValue *key, bool down)
+static InputEvent *qemu_input_event_new_key(KeyValue *key, bool down)
 {
 InputEvent *evt = g_new0(InputEvent, 1);
 evt->u.key.data = g_new0(InputKeyEvent, 1);
-- 
2.13.3

[Qemu-devel] [PATCH v4 03/12] ui: convert key events to QKeyCodes immediately

[Daniel P. Berrange]

Always use QKeyCode in the InputKeyEvent struct, by converting key
numbers to QKeyCode at the time the event is created.

Signed-off-by: Daniel P. Berrange 
---
 ui/input.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ui/input.c b/ui/input.c
index af05f06368..64e9103a61 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -400,10 +400,8 @@ void qemu_input_event_send_key(QemuConsole *src, KeyValue 
*key, bool down)
 
 void qemu_input_event_send_key_number(QemuConsole *src, int num, bool down)
 {
-KeyValue *key = g_new0(KeyValue, 1);
-key->type = KEY_VALUE_KIND_NUMBER;
-key->u.number.data = num;
-qemu_input_event_send_key(src, key, down);
+QKeyCode code = qemu_input_key_number_to_qcode(num);
+qemu_input_event_send_key_qcode(src, code, down);
 }
 
 void qemu_input_event_send_key_qcode(QemuConsole *src, QKeyCode q, bool down)
-- 
2.13.3

[Qemu-devel] [PATCH v4 05/12] input: convert virtio-input-hid device to keycodemapdb

[Daniel P. Berrange]

Replace the keymap_qcode table with automatically generated
tables.

Missing entries in keymap_qcode now fixed:

  Q_KEY_CODE_ASTERISK -> KEY_KPASTERISK
  Q_KEY_CODE_KP_MULTIPLY -> KEY_KPASTERISK
  Q_KEY_CODE_STOP -> KEY_STOP
  Q_KEY_CODE_AGAIN -> KEY_AGAIN
  Q_KEY_CODE_PROPS -> KEY_PROPS
  Q_KEY_CODE_UNDO -> KEY_UNDO
  Q_KEY_CODE_FRONT -> KEY_FRONT
  Q_KEY_CODE_COPY -> KEY_COPY
  Q_KEY_CODE_OPEN -> KEY_OPEN
  Q_KEY_CODE_PASTE -> KEY_PASTE
  Q_KEY_CODE_FIND -> KEY_FIND
  Q_KEY_CODE_CUT -> KEY_CUT
  Q_KEY_CODE_LF -> KEY_LINEFEED
  Q_KEY_CODE_HELP -> KEY_HELP
  Q_KEY_CODE_COMPOSE -> KEY_COMPOSE
  Q_KEY_CODE_RO -> KEY_RO
  Q_KEY_CODE_HIRAGANA -> KEY_HIRAGANA
  Q_KEY_CODE_HENKAN -> KEY_HENKAN
  Q_KEY_CODE_YEN -> KEY_YEN
  Q_KEY_CODE_KP_COMMA -> KEY_KPCOMMA
  Q_KEY_CODE_KP_EQUALS -> KEY_KPEQUAL
  Q_KEY_CODE_POWER -> KEY_POWER
  Q_KEY_CODE_SLEEP -> KEY_SLEEP
  Q_KEY_CODE_WAKE -> KEY_WAKEUP
  Q_KEY_CODE_AUDIONEXT -> KEY_NEXTSONG
  Q_KEY_CODE_AUDIOPREV -> KEY_PREVIOUSSONG
  Q_KEY_CODE_AUDIOSTOP -> KEY_STOPCD
  Q_KEY_CODE_AUDIOPLAY -> KEY_PLAYPAUSE
  Q_KEY_CODE_AUDIOMUTE -> KEY_MUTE
  Q_KEY_CODE_VOLUMEUP -> KEY_VOLUMEUP
  Q_KEY_CODE_VOLUMEDOWN -> KEY_VOLUMEDOWN
  Q_KEY_CODE_MEDIASELECT -> KEY_MEDIA
  Q_KEY_CODE_MAIL -> KEY_MAIL
  Q_KEY_CODE_CALCULATOR -> KEY_CALC
  Q_KEY_CODE_COMPUTER -> KEY_COMPUTER
  Q_KEY_CODE_AC_HOME -> KEY_HOMEPAGE
  Q_KEY_CODE_AC_BACK -> KEY_BACK
  Q_KEY_CODE_AC_FORWARD -> KEY_FORWARD
  Q_KEY_CODE_AC_REFRESH -> KEY_REFRESH
  Q_KEY_CODE_AC_BOOKMARKS -> KEY_BOOKMARKS

Signed-off-by: Daniel P. Berrange 
---
 hw/input/virtio-input-hid.c | 136 +++-
 include/ui/input.h  |   3 +
 ui/Makefile.objs|   1 +
 ui/input-keymap.c   |   1 +
 4 files changed, 14 insertions(+), 127 deletions(-)

diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index 46c038110c..3e5a2051ed 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -22,126 +22,7 @@
 
 /* - */
 
-static const unsigned int keymap_qcode[Q_KEY_CODE__MAX] = {
-[Q_KEY_CODE_ESC] = KEY_ESC,
-[Q_KEY_CODE_1]   = KEY_1,
-[Q_KEY_CODE_2]   = KEY_2,
-[Q_KEY_CODE_3]   = KEY_3,
-[Q_KEY_CODE_4]   = KEY_4,
-[Q_KEY_CODE_5]   = KEY_5,
-[Q_KEY_CODE_6]   = KEY_6,
-[Q_KEY_CODE_7]   = KEY_7,
-[Q_KEY_CODE_8]   = KEY_8,
-[Q_KEY_CODE_9]   = KEY_9,
-[Q_KEY_CODE_0]   = KEY_0,
-[Q_KEY_CODE_MINUS]   = KEY_MINUS,
-[Q_KEY_CODE_EQUAL]   = KEY_EQUAL,
-[Q_KEY_CODE_BACKSPACE]   = KEY_BACKSPACE,
-
-[Q_KEY_CODE_TAB] = KEY_TAB,
-[Q_KEY_CODE_Q]   = KEY_Q,
-[Q_KEY_CODE_W]   = KEY_W,
-[Q_KEY_CODE_E]   = KEY_E,
-[Q_KEY_CODE_R]   = KEY_R,
-[Q_KEY_CODE_T]   = KEY_T,
-[Q_KEY_CODE_Y]   = KEY_Y,
-[Q_KEY_CODE_U]   = KEY_U,
-[Q_KEY_CODE_I]   = KEY_I,
-[Q_KEY_CODE_O]   = KEY_O,
-[Q_KEY_CODE_P]   = KEY_P,
-[Q_KEY_CODE_BRACKET_LEFT]= KEY_LEFTBRACE,
-[Q_KEY_CODE_BRACKET_RIGHT]   = KEY_RIGHTBRACE,
-[Q_KEY_CODE_RET] = KEY_ENTER,
-
-[Q_KEY_CODE_CTRL]= KEY_LEFTCTRL,
-[Q_KEY_CODE_A]   = KEY_A,
-[Q_KEY_CODE_S]   = KEY_S,
-[Q_KEY_CODE_D]   = KEY_D,
-[Q_KEY_CODE_F]   = KEY_F,
-[Q_KEY_CODE_G]   = KEY_G,
-[Q_KEY_CODE_H]   = KEY_H,
-[Q_KEY_CODE_J]   = KEY_J,
-[Q_KEY_CODE_K]   = KEY_K,
-[Q_KEY_CODE_L]   = KEY_L,
-[Q_KEY_CODE_SEMICOLON]   = KEY_SEMICOLON,
-[Q_KEY_CODE_APOSTROPHE]  = KEY_APOSTROPHE,
-[Q_KEY_CODE_GRAVE_ACCENT]= KEY_GRAVE,
-
-[Q_KEY_CODE_SHIFT]   = KEY_LEFTSHIFT,
-[Q_KEY_CODE_BACKSLASH]   = KEY_BACKSLASH,
-[Q_KEY_CODE_LESS]= KEY_102ND,
-[Q_KEY_CODE_Z]   = KEY_Z,
-[Q_KEY_CODE_X]   = KEY_X,
-[Q_KEY_CODE_C]   = KEY_C,
-[Q_KEY_CODE_V]   = KEY_V,
-[Q_KEY_CODE_B]   = KEY_B,
-[Q_KEY_CODE_N]   = KEY_N,
-[Q_KEY_CODE_M]   = KEY_M,
-[Q_KEY_CODE_COMMA]   = KEY_COMMA,
-[Q_KEY_CODE_DOT] = KEY_DOT,
-[Q_KEY_CODE_SLASH]   = KEY_SLASH,
-[Q_KEY_CODE_SHIFT_R] = KEY_RIGHTSHIFT,
-
-[Q_KEY_CODE_ALT] = KEY_LEFTALT,
-[Q_KEY_CODE_SPC] = KEY_SPACE,
-[Q_KEY_CODE_CAPS_LOCK]   = KEY_CAPSLOCK,
-
-[Q_KEY_CODE

[Qemu-devel] [PATCH v4 07/12] input: convert the adb device to keycodemapdb

[Daniel P. Berrange]

Replace the qcode_to_adb_keycode table with automatically
generated tables.

Missing entries in qcode_to_adb_keycode now fixed:

 - Q_KEY_CODE_KP_COMMA -> 0x47

Signed-off-by: Daniel P. Berrange 
---
 hw/input/adb.c  | 124 +-
 include/hw/input/adb-keys.h | 141 
 include/ui/input.h  |   3 +
 ui/Makefile.objs|   1 +
 ui/input-keymap.c   |   1 +
 5 files changed, 7 insertions(+), 263 deletions(-)
 delete mode 100644 include/hw/input/adb-keys.h

diff --git a/hw/input/adb.c b/hw/input/adb.c
index fcca3a8eb9..1fe5d298a3 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -25,7 +25,6 @@
 #include "hw/hw.h"
 #include "hw/input/adb.h"
 #include "ui/console.h"
-#include "include/hw/input/adb-keys.h"
 #include "ui/input.h"
 #include "sysemu/sysemu.h"
 
@@ -193,125 +192,6 @@ typedef struct ADBKeyboardClass {
 DeviceRealize parent_realize;
 } ADBKeyboardClass;
 
-int qcode_to_adb_keycode[] = {
- /* Make sure future additions are automatically set to NO_KEY */
-[0 ... 0xff]   = NO_KEY,
-
-[Q_KEY_CODE_SHIFT] = ADB_KEY_LEFT_SHIFT,
-[Q_KEY_CODE_SHIFT_R]   = ADB_KEY_RIGHT_SHIFT,
-[Q_KEY_CODE_ALT]   = ADB_KEY_LEFT_OPTION,
-[Q_KEY_CODE_ALT_R] = ADB_KEY_RIGHT_OPTION,
-[Q_KEY_CODE_CTRL]  = ADB_KEY_LEFT_CONTROL,
-[Q_KEY_CODE_CTRL_R]= ADB_KEY_RIGHT_CONTROL,
-[Q_KEY_CODE_META_L]= ADB_KEY_COMMAND,
-[Q_KEY_CODE_META_R]= ADB_KEY_COMMAND,
-[Q_KEY_CODE_SPC]   = ADB_KEY_SPACEBAR,
-
-[Q_KEY_CODE_ESC]   = ADB_KEY_ESC,
-[Q_KEY_CODE_1] = ADB_KEY_1,
-[Q_KEY_CODE_2] = ADB_KEY_2,
-[Q_KEY_CODE_3] = ADB_KEY_3,
-[Q_KEY_CODE_4] = ADB_KEY_4,
-[Q_KEY_CODE_5] = ADB_KEY_5,
-[Q_KEY_CODE_6] = ADB_KEY_6,
-[Q_KEY_CODE_7] = ADB_KEY_7,
-[Q_KEY_CODE_8] = ADB_KEY_8,
-[Q_KEY_CODE_9] = ADB_KEY_9,
-[Q_KEY_CODE_0] = ADB_KEY_0,
-[Q_KEY_CODE_MINUS] = ADB_KEY_MINUS,
-[Q_KEY_CODE_EQUAL] = ADB_KEY_EQUAL,
-[Q_KEY_CODE_BACKSPACE] = ADB_KEY_DELETE,
-[Q_KEY_CODE_TAB]   = ADB_KEY_TAB,
-[Q_KEY_CODE_Q] = ADB_KEY_Q,
-[Q_KEY_CODE_W] = ADB_KEY_W,
-[Q_KEY_CODE_E] = ADB_KEY_E,
-[Q_KEY_CODE_R] = ADB_KEY_R,
-[Q_KEY_CODE_T] = ADB_KEY_T,
-[Q_KEY_CODE_Y] = ADB_KEY_Y,
-[Q_KEY_CODE_U] = ADB_KEY_U,
-[Q_KEY_CODE_I] = ADB_KEY_I,
-[Q_KEY_CODE_O] = ADB_KEY_O,
-[Q_KEY_CODE_P] = ADB_KEY_P,
-[Q_KEY_CODE_BRACKET_LEFT]  = ADB_KEY_LEFT_BRACKET,
-[Q_KEY_CODE_BRACKET_RIGHT] = ADB_KEY_RIGHT_BRACKET,
-[Q_KEY_CODE_RET]   = ADB_KEY_RETURN,
-[Q_KEY_CODE_A] = ADB_KEY_A,
-[Q_KEY_CODE_S] = ADB_KEY_S,
-[Q_KEY_CODE_D] = ADB_KEY_D,
-[Q_KEY_CODE_F] = ADB_KEY_F,
-[Q_KEY_CODE_G] = ADB_KEY_G,
-[Q_KEY_CODE_H] = ADB_KEY_H,
-[Q_KEY_CODE_J] = ADB_KEY_J,
-[Q_KEY_CODE_K] = ADB_KEY_K,
-[Q_KEY_CODE_L] = ADB_KEY_L,
-[Q_KEY_CODE_SEMICOLON] = ADB_KEY_SEMICOLON,
-[Q_KEY_CODE_APOSTROPHE]= ADB_KEY_APOSTROPHE,
-[Q_KEY_CODE_GRAVE_ACCENT]  = ADB_KEY_GRAVE_ACCENT,
-[Q_KEY_CODE_BACKSLASH] = ADB_KEY_BACKSLASH,
-[Q_KEY_CODE_Z] = ADB_KEY_Z,
-[Q_KEY_CODE_X] = ADB_KEY_X,
-[Q_KEY_CODE_C] = ADB_KEY_C,
-[Q_KEY_CODE_V] = ADB_KEY_V,
-[Q_KEY_CODE_B] = ADB_KEY_B,
-[Q_KEY_CODE_N] = ADB_KEY_N,
-[Q_KEY_CODE_M] = ADB_KEY_M,
-[Q_KEY_CODE_COMMA] = ADB_KEY_COMMA,
-[Q_KEY_CODE_DOT]   = ADB_KEY_PERIOD,
-[Q_KEY_CODE_SLASH] = ADB_KEY_FORWARD_SLASH,
-[Q_KEY_CODE_ASTERISK]  = ADB_KEY_KP_MULTIPLY,
-[Q_KEY_CODE_CAPS_LOCK] = ADB_KEY_CAPS_LOCK,
-
-[Q_KEY_CODE_F1]= ADB_KEY_F1,
-[Q_KEY_CODE_F2]= ADB_KEY_F2,
-[Q_KEY_CODE_F3]= ADB_KEY_F3,
-[Q_KEY_CODE_F4]= ADB_KEY_F4,
-[Q_KEY_CODE_F5]= ADB_KEY_F5,
-[Q_KEY_CODE_F6]= ADB_KEY_F6,
-[Q_KEY_CODE_F7]= ADB_KEY_F7,
-[Q_KEY_CODE_F8]= ADB_KEY_F8,
-[Q_KEY_CODE_F9]= ADB_KEY_F9,
-[Q_KEY_CODE_F10]   = ADB_KEY_F10,
-[Q_KEY_CODE_F11]   = ADB_KEY_F11,
-[Q_KEY_CODE_F12]   = ADB_KEY_F12,
-[Q_KEY_CODE_PRINT] = ADB_KEY_F13,
-[Q_KEY_CODE_SYSRQ] = ADB_KEY_F13,
-[Q_KEY_CODE_SCROLL_LOCK]   = ADB_KEY_F14,
-[Q_KEY_CODE_PAUSE] = ADB_KEY_F15,
-
-[Q_KEY_CODE_NUM_LOCK]  = ADB_KEY_KP_CLEAR,
-[Q_KEY_CODE_KP_EQUALS] = ADB_KEY_KP_EQUAL,
-[Q_KEY_CODE_KP_D

[Qemu-devel] [PATCH v4 08/12] char: convert the escc device to keycodemapdb

[Daniel P. Berrange]

Replace the qcode_to_keycode table with automatically
generated tables.

Missing entries in qcode_to_keycode now fixed:

 - Q_KEY_CODE_KP_COMMA -> 0x2d

Signed-off-by: Daniel P. Berrange 
---
 hw/char/escc.c | 126 +++--
 include/ui/input.h |   3 ++
 ui/Makefile.objs   |   1 +
 ui/input-keymap.c  |   1 +
 4 files changed, 10 insertions(+), 121 deletions(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 1aca564e33..05665d85e7 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -717,126 +717,6 @@ MemoryRegion *escc_init(hwaddr base, qemu_irq irqA, 
qemu_irq irqB,
 return &d->mmio;
 }
 
-static const uint8_t qcode_to_keycode[Q_KEY_CODE__MAX] = {
-[Q_KEY_CODE_SHIFT] = 99,
-[Q_KEY_CODE_SHIFT_R]   = 110,
-[Q_KEY_CODE_ALT]   = 19,
-[Q_KEY_CODE_ALT_R] = 13,
-[Q_KEY_CODE_CTRL]  = 76,
-[Q_KEY_CODE_CTRL_R]= 76,
-[Q_KEY_CODE_ESC]   = 29,
-[Q_KEY_CODE_1] = 30,
-[Q_KEY_CODE_2] = 31,
-[Q_KEY_CODE_3] = 32,
-[Q_KEY_CODE_4] = 33,
-[Q_KEY_CODE_5] = 34,
-[Q_KEY_CODE_6] = 35,
-[Q_KEY_CODE_7] = 36,
-[Q_KEY_CODE_8] = 37,
-[Q_KEY_CODE_9] = 38,
-[Q_KEY_CODE_0] = 39,
-[Q_KEY_CODE_MINUS] = 40,
-[Q_KEY_CODE_EQUAL] = 41,
-[Q_KEY_CODE_BACKSPACE] = 43,
-[Q_KEY_CODE_TAB]   = 53,
-[Q_KEY_CODE_Q] = 54,
-[Q_KEY_CODE_W] = 55,
-[Q_KEY_CODE_E] = 56,
-[Q_KEY_CODE_R] = 57,
-[Q_KEY_CODE_T] = 58,
-[Q_KEY_CODE_Y] = 59,
-[Q_KEY_CODE_U] = 60,
-[Q_KEY_CODE_I] = 61,
-[Q_KEY_CODE_O] = 62,
-[Q_KEY_CODE_P] = 63,
-[Q_KEY_CODE_BRACKET_LEFT]  = 64,
-[Q_KEY_CODE_BRACKET_RIGHT] = 65,
-[Q_KEY_CODE_RET]   = 89,
-[Q_KEY_CODE_A] = 77,
-[Q_KEY_CODE_S] = 78,
-[Q_KEY_CODE_D] = 79,
-[Q_KEY_CODE_F] = 80,
-[Q_KEY_CODE_G] = 81,
-[Q_KEY_CODE_H] = 82,
-[Q_KEY_CODE_J] = 83,
-[Q_KEY_CODE_K] = 84,
-[Q_KEY_CODE_L] = 85,
-[Q_KEY_CODE_SEMICOLON] = 86,
-[Q_KEY_CODE_APOSTROPHE]= 87,
-[Q_KEY_CODE_GRAVE_ACCENT]  = 42,
-[Q_KEY_CODE_BACKSLASH] = 88,
-[Q_KEY_CODE_Z] = 100,
-[Q_KEY_CODE_X] = 101,
-[Q_KEY_CODE_C] = 102,
-[Q_KEY_CODE_V] = 103,
-[Q_KEY_CODE_B] = 104,
-[Q_KEY_CODE_N] = 105,
-[Q_KEY_CODE_M] = 106,
-[Q_KEY_CODE_COMMA] = 107,
-[Q_KEY_CODE_DOT]   = 108,
-[Q_KEY_CODE_SLASH] = 109,
-[Q_KEY_CODE_ASTERISK]  = 47,
-[Q_KEY_CODE_SPC]   = 121,
-[Q_KEY_CODE_CAPS_LOCK] = 119,
-[Q_KEY_CODE_F1]= 5,
-[Q_KEY_CODE_F2]= 6,
-[Q_KEY_CODE_F3]= 8,
-[Q_KEY_CODE_F4]= 10,
-[Q_KEY_CODE_F5]= 12,
-[Q_KEY_CODE_F6]= 14,
-[Q_KEY_CODE_F7]= 16,
-[Q_KEY_CODE_F8]= 17,
-[Q_KEY_CODE_F9]= 18,
-[Q_KEY_CODE_F10]   = 7,
-[Q_KEY_CODE_NUM_LOCK]  = 98,
-[Q_KEY_CODE_SCROLL_LOCK]   = 23,
-[Q_KEY_CODE_KP_DIVIDE] = 46,
-[Q_KEY_CODE_KP_MULTIPLY]   = 47,
-[Q_KEY_CODE_KP_SUBTRACT]   = 71,
-[Q_KEY_CODE_KP_ADD]= 125,
-[Q_KEY_CODE_KP_ENTER]  = 90,
-[Q_KEY_CODE_KP_DECIMAL]= 50,
-[Q_KEY_CODE_KP_0]  = 94,
-[Q_KEY_CODE_KP_1]  = 112,
-[Q_KEY_CODE_KP_2]  = 113,
-[Q_KEY_CODE_KP_3]  = 114,
-[Q_KEY_CODE_KP_4]  = 91,
-[Q_KEY_CODE_KP_5]  = 92,
-[Q_KEY_CODE_KP_6]  = 93,
-[Q_KEY_CODE_KP_7]  = 68,
-[Q_KEY_CODE_KP_8]  = 69,
-[Q_KEY_CODE_KP_9]  = 70,
-[Q_KEY_CODE_LESS]  = 124,
-[Q_KEY_CODE_F11]   = 9,
-[Q_KEY_CODE_F12]   = 11,
-[Q_KEY_CODE_HOME]  = 52,
-[Q_KEY_CODE_PGUP]  = 96,
-[Q_KEY_CODE_PGDN]  = 123,
-[Q_KEY_CODE_END]   = 74,
-[Q_KEY_CODE_LEFT]  = 24,
-[Q_KEY_CODE_UP]= 20,
-[Q_KEY_CODE_DOWN]  = 27,
-[Q_KEY_CODE_RIGHT] = 28,
-[Q_KEY_CODE_INSERT]= 44,
-[Q_KEY_CODE_DELETE]= 66,
-[Q_KEY_CODE_STOP]  = 1,
-[Q_KEY_CODE_AGAIN] = 3,
-[Q_KEY_CODE_PROPS] = 25,
-[Q_KEY_CODE_UNDO]  = 26,
-[Q_KEY_CODE_FRONT] = 49,
-[Q_KEY_CODE_COPY]  = 51,
-[Q_KEY_CODE_OPEN]  = 72,
-[Q_KEY_CODE_PASTE] = 73,
-[Q_KEY_CODE_FIND]  = 95,
-[Q_KEY_CODE_CUT]   = 97,
-[Q_KEY_CODE_LF]= 111,
-[Q_KEY_CODE_HE

[Qemu-devel] [PATCH v4 09/12] ui: convert cocoa frontend to keycodemapdb

[Daniel P. Berrange]

Replace the mac_to_qkeycode_map table with automatically
generated table.

Signed-off-by: Daniel P. Berrange 
---
 include/ui/input.h |   3 ++
 ui/Makefile.objs   |   1 +
 ui/cocoa.m | 129 +
 ui/input-keymap.c  |   1 +
 4 files changed, 7 insertions(+), 127 deletions(-)

diff --git a/include/ui/input.h b/include/ui/input.h
index 4ba7340b7b..df3eb439bf 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -71,6 +71,9 @@ void qemu_remove_mouse_mode_change_notifier(Notifier *notify);
 extern const guint qemu_input_map_linux_to_qcode_len;
 extern const guint16 qemu_input_map_linux_to_qcode[];
 
+extern const guint qemu_input_map_osx_to_qcode_len;
+extern const guint16 qemu_input_map_osx_to_qcode[];
+
 extern const guint qemu_input_map_qcode_to_adb_len;
 extern const guint16 qemu_input_map_qcode_to_adb[];
 
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 478a039398..d35634cdc7 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -55,6 +55,7 @@ KEYCODEMAP_CSV = $(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv
 
 KEYCODEMAP_FILES = \
 ui/input-keymap-linux-to-qcode.c \
+ui/input-keymap-osx-to-qcode.c \
 ui/input-keymap-qcode-to-adb.c \
 ui/input-keymap-qcode-to-atset1.c \
 ui/input-keymap-qcode-to-atset2.c \
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 93e56d0518..e966605914 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -106,139 +106,14 @@ bool stretch_video;
 NSTextField *pauseLabel;
 NSArray * supportedImageFileTypes;
 
-// Mac to QKeyCode conversion
-const int mac_to_qkeycode_map[] = {
-[kVK_ANSI_A] = Q_KEY_CODE_A,
-[kVK_ANSI_B] = Q_KEY_CODE_B,
-[kVK_ANSI_C] = Q_KEY_CODE_C,
-[kVK_ANSI_D] = Q_KEY_CODE_D,
-[kVK_ANSI_E] = Q_KEY_CODE_E,
-[kVK_ANSI_F] = Q_KEY_CODE_F,
-[kVK_ANSI_G] = Q_KEY_CODE_G,
-[kVK_ANSI_H] = Q_KEY_CODE_H,
-[kVK_ANSI_I] = Q_KEY_CODE_I,
-[kVK_ANSI_J] = Q_KEY_CODE_J,
-[kVK_ANSI_K] = Q_KEY_CODE_K,
-[kVK_ANSI_L] = Q_KEY_CODE_L,
-[kVK_ANSI_M] = Q_KEY_CODE_M,
-[kVK_ANSI_N] = Q_KEY_CODE_N,
-[kVK_ANSI_O] = Q_KEY_CODE_O,
-[kVK_ANSI_P] = Q_KEY_CODE_P,
-[kVK_ANSI_Q] = Q_KEY_CODE_Q,
-[kVK_ANSI_R] = Q_KEY_CODE_R,
-[kVK_ANSI_S] = Q_KEY_CODE_S,
-[kVK_ANSI_T] = Q_KEY_CODE_T,
-[kVK_ANSI_U] = Q_KEY_CODE_U,
-[kVK_ANSI_V] = Q_KEY_CODE_V,
-[kVK_ANSI_W] = Q_KEY_CODE_W,
-[kVK_ANSI_X] = Q_KEY_CODE_X,
-[kVK_ANSI_Y] = Q_KEY_CODE_Y,
-[kVK_ANSI_Z] = Q_KEY_CODE_Z,
-
-[kVK_ANSI_0] = Q_KEY_CODE_0,
-[kVK_ANSI_1] = Q_KEY_CODE_1,
-[kVK_ANSI_2] = Q_KEY_CODE_2,
-[kVK_ANSI_3] = Q_KEY_CODE_3,
-[kVK_ANSI_4] = Q_KEY_CODE_4,
-[kVK_ANSI_5] = Q_KEY_CODE_5,
-[kVK_ANSI_6] = Q_KEY_CODE_6,
-[kVK_ANSI_7] = Q_KEY_CODE_7,
-[kVK_ANSI_8] = Q_KEY_CODE_8,
-[kVK_ANSI_9] = Q_KEY_CODE_9,
-
-[kVK_ANSI_Grave] = Q_KEY_CODE_GRAVE_ACCENT,
-[kVK_ANSI_Minus] = Q_KEY_CODE_MINUS,
-[kVK_ANSI_Equal] = Q_KEY_CODE_EQUAL,
-[kVK_Delete] = Q_KEY_CODE_BACKSPACE,
-[kVK_CapsLock] = Q_KEY_CODE_CAPS_LOCK,
-[kVK_Tab] = Q_KEY_CODE_TAB,
-[kVK_Return] = Q_KEY_CODE_RET,
-[kVK_ANSI_LeftBracket] = Q_KEY_CODE_BRACKET_LEFT,
-[kVK_ANSI_RightBracket] = Q_KEY_CODE_BRACKET_RIGHT,
-[kVK_ANSI_Backslash] = Q_KEY_CODE_BACKSLASH,
-[kVK_ANSI_Semicolon] = Q_KEY_CODE_SEMICOLON,
-[kVK_ANSI_Quote] = Q_KEY_CODE_APOSTROPHE,
-[kVK_ANSI_Comma] = Q_KEY_CODE_COMMA,
-[kVK_ANSI_Period] = Q_KEY_CODE_DOT,
-[kVK_ANSI_Slash] = Q_KEY_CODE_SLASH,
-[kVK_Shift] = Q_KEY_CODE_SHIFT,
-[kVK_RightShift] = Q_KEY_CODE_SHIFT_R,
-[kVK_Control] = Q_KEY_CODE_CTRL,
-[kVK_RightControl] = Q_KEY_CODE_CTRL_R,
-[kVK_Option] = Q_KEY_CODE_ALT,
-[kVK_RightOption] = Q_KEY_CODE_ALT_R,
-[kVK_Command] = Q_KEY_CODE_META_L,
-[0x36] = Q_KEY_CODE_META_R, /* There is no kVK_RightCommand */
-[kVK_Space] = Q_KEY_CODE_SPC,
-
-[kVK_ANSI_Keypad0] = Q_KEY_CODE_KP_0,
-[kVK_ANSI_Keypad1] = Q_KEY_CODE_KP_1,
-[kVK_ANSI_Keypad2] = Q_KEY_CODE_KP_2,
-[kVK_ANSI_Keypad3] = Q_KEY_CODE_KP_3,
-[kVK_ANSI_Keypad4] = Q_KEY_CODE_KP_4,
-[kVK_ANSI_Keypad5] = Q_KEY_CODE_KP_5,
-[kVK_ANSI_Keypad6] = Q_KEY_CODE_KP_6,
-[kVK_ANSI_Keypad7] = Q_KEY_CODE_KP_7,
-[kVK_ANSI_Keypad8] = Q_KEY_CODE_KP_8,
-[kVK_ANSI_Keypad9] = Q_KEY_CODE_KP_9,
-[kVK_ANSI_KeypadDecimal] = Q_KEY_CODE_KP_DECIMAL,
-[kVK_ANSI_KeypadEnter] = Q_KEY_CODE_KP_ENTER,
-[kVK_ANSI_KeypadPlus] = Q_KEY_CODE_KP_ADD,
-[kVK_ANSI_KeypadMinus] = Q_KEY_CODE_KP_SUBTRACT,
-[kVK_ANSI_KeypadMultiply] = Q_KEY_CODE_KP_MULTIPLY,
-[kVK_ANSI_KeypadDivide] = Q_KEY_CODE_KP_DIVIDE,
-[kVK_ANSI_KeypadEquals] = Q_KEY_CODE_KP_EQUALS,
-[kVK_ANSI_KeypadClear] = Q_KEY_CODE_NUM_LOCK,
-
-[kVK_UpArrow] = Q_KEY_CODE_UP,
-[kVK_DownArrow] = Q_KEY_CODE_DOWN,
-[kVK_LeftArrow] = Q_KEY_CODE_LEFT,
-[kVK_RightArrow] = Q_KEY_CODE_RIGHT,
-
-[kVK_Help] = Q_KEY_CODE_INSERT,
-

[Qemu-devel] [PATCH v4 11/12] ui: convert GTK and SDL1 frontends to keycodemapdb

[Daniel P. Berrange]

The x_keycode_to_pc_keycode and evdev_keycode_to_pc_keycode
tables are replaced with automatically generated tables.
In addition the X11 heuristics are improved to detect running
on XQuartz and XWin X11 servers, to activate the correct OS-X
and Win32 keycode maps.

Signed-off-by: Daniel P. Berrange 
---
 include/ui/input.h |  21 +
 ui/Makefile.objs   |  12 ++-
 ui/gtk.c   | 205 +--
 ui/input-keymap.c  |   7 ++
 ui/sdl.c   | 105 +++---
 ui/trace-events|   9 +-
 ui/x_keymap.c  | 250 -
 ui/x_keymap.h  |   8 +-
 8 files changed, 300 insertions(+), 317 deletions(-)

diff --git a/include/ui/input.h b/include/ui/input.h
index dcf7e0e731..1c1731ef18 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -68,6 +68,9 @@ void qemu_input_check_mode_change(void);
 void qemu_add_mouse_mode_change_notifier(Notifier *notify);
 void qemu_remove_mouse_mode_change_notifier(Notifier *notify);
 
+extern const guint qemu_input_map_atset1_to_qcode_len;
+extern const guint16 qemu_input_map_atset1_to_qcode[];
+
 extern const guint qemu_input_map_linux_to_qcode_len;
 extern const guint16 qemu_input_map_linux_to_qcode[];
 
@@ -101,4 +104,22 @@ extern const guint16 qemu_input_map_qnum_to_qcode[];
 extern const guint qemu_input_map_usb_to_qcode_len;
 extern const guint16 qemu_input_map_usb_to_qcode[];
 
+extern const guint qemu_input_map_win32_to_qcode_len;
+extern const guint16 qemu_input_map_win32_to_qcode[];
+
+extern const guint qemu_input_map_x11_to_qcode_len;
+extern const guint16 qemu_input_map_x11_to_qcode[];
+
+extern const guint qemu_input_map_xorgevdev_to_qcode_len;
+extern const guint16 qemu_input_map_xorgevdev_to_qcode[];
+
+extern const guint qemu_input_map_xorgkbd_to_qcode_len;
+extern const guint16 qemu_input_map_xorgkbd_to_qcode[];
+
+extern const guint qemu_input_map_xorgxquartz_to_qcode_len;
+extern const guint16 qemu_input_map_xorgxquartz_to_qcode[];
+
+extern const guint qemu_input_map_xorgxwin_to_qcode_len;
+extern const guint16 qemu_input_map_xorgxwin_to_qcode[];
+
 #endif /* INPUT_H */
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 0e5821f6eb..6cd39bf49d 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -11,11 +11,12 @@ common-obj-y += keymaps.o console.o cursor.o qemu-pixman.o
 common-obj-y += input.o input-keymap.o input-legacy.o
 common-obj-$(CONFIG_LINUX) += input-linux.o
 common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
-common-obj-$(CONFIG_SDL) += sdl.mo x_keymap.o
+common-obj-$(CONFIG_SDL) += sdl.mo
 common-obj-$(CONFIG_COCOA) += cocoa.o
 common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
-common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o
+common-obj-$(CONFIG_GTK) += gtk.o
+common-obj-$(if $(CONFIG_WIN32),n,$(if $(CONFIG_SDL),y,$(CONFIG_GTK))) += 
x_keymap.o
 
 ifeq ($(CONFIG_SDLABI),1.2)
 sdl.mo-objs := sdl.o sdl_zoom.o
@@ -54,6 +55,7 @@ KEYCODEMAP_GEN = $(SRC_PATH)/ui/keycodemapdb/tools/keymap-gen
 KEYCODEMAP_CSV = $(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv
 
 KEYCODEMAP_FILES = \
+ui/input-keymap-atset1-to-qcode.c \
 ui/input-keymap-linux-to-qcode.c \
 ui/input-keymap-osx-to-qcode.c \
 ui/input-keymap-qcode-to-adb.c \
@@ -65,6 +67,12 @@ KEYCODEMAP_FILES = \
 ui/input-keymap-qcode-to-sun.c \
 ui/input-keymap-qnum-to-qcode.c \
 ui/input-keymap-usb-to-qcode.c \
+ui/input-keymap-win32-to-qcode.c \
+ui/input-keymap-x11-to-qcode.c \
+ui/input-keymap-xorgevdev-to-qcode.c \
+ui/input-keymap-xorgkbd-to-qcode.c \
+ui/input-keymap-xorgxquartz-to-qcode.c \
+ui/input-keymap-xorgxwin-to-qcode.c \
 $(NULL)
 
 GENERATED_FILES += $(KEYCODEMAP_FILES)
diff --git a/ui/gtk.c b/ui/gtk.c
index 5bd87c265a..398f3aad81 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -52,7 +52,6 @@
 #include "ui/input.h"
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
-#include "x_keymap.h"
 #include "keymaps.h"
 #include "chardev/char.h"
 #include "qom/object.h"
@@ -65,6 +64,48 @@
 #define VC_SCALE_MIN0.25
 #define VC_SCALE_STEP   0.25
 
+#ifdef GDK_WINDOWING_X11
+#include "ui/x_keymap.h"
+
+/* Gtk2 compat */
+#ifndef GDK_IS_X11_DISPLAY
+#define GDK_IS_X11_DISPLAY(dpy) (dpy != NULL)
+#endif
+#endif
+
+
+#ifdef GDK_WINDOWING_WAYLAND
+/* Gtk2 compat */
+#ifndef GDK_IS_WAYLAND_DISPLAY
+#define GDK_IS_WAYLAND_DISPLAY(dpy) (dpy != NULL)
+#endif
+#endif
+
+
+#ifdef GDK_WINDOWING_WIN32
+/* Gtk2 compat */
+#ifndef GDK_IS_WIN32_DISPLAY
+#define GDK_IS_WIN32_DISPLAY(dpy) (dpy != NULL)
+#endif
+#endif
+
+
+#ifdef GDK_WINDOWING_BROADWAY
+/* Gtk2 compat */
+#ifndef GDK_IS_BROADWAY_DISPLAY
+#define GDK_IS_BROADWAY_DISPLAY(dpy) (dpy != NULL)
+#endif
+#endif
+
+
+#ifdef GDK_WINDOWING_QUARTZ
+/* Gtk2 compat */
+#ifndef GDK_IS_QUAR

[Qemu-devel] [PATCH v4 12/12] display: convert XenInput keyboard to keycodemapdb

[Daniel P. Berrange]

Replace the scancode2linux table with an automatically
generated table. In doing so, the XenFB keyboard
handler is also converted to the modern InputEvent
framework.

Signed-off-by: Daniel P. Berrange 
---
 hw/display/xenfb.c | 133 -
 1 file changed, 30 insertions(+), 103 deletions(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index df8b78f6f4..b4668dfce4 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -28,6 +28,7 @@
 
 #include "hw/hw.h"
 #include "ui/console.h"
+#include "ui/input.h"
 #include "hw/xen/xen_backend.h"
 
 #include 
@@ -52,7 +53,7 @@ struct XenInput {
 struct common c;
 int abs_pointer_wanted; /* Whether guest supports absolute pointer */
 int button_state;   /* Last seen pointer button state */
-int extended;
+QemuInputHandlerState *qkbd;
 QEMUPutMouseEntry *qmouse;
 };
 
@@ -120,78 +121,6 @@ static void common_unbind(struct common *c)
 
 /*  */
 
-#if 0
-/*
- * These two tables are not needed any more, but left in here
- * intentionally as documentation, to show how scancode2linux[]
- * was generated.
- *
- * Tables to map from scancode to Linux input layer keycode.
- * Scancodes are hardware-specific.  These maps assumes a
- * standard AT or PS/2 keyboard which is what QEMU feeds us.
- */
-const unsigned char atkbd_set2_keycode[512] = {
-
- 0, 67, 65, 63, 61, 59, 60, 88,  0, 68, 66, 64, 62, 15, 41,117,
- 0, 56, 42, 93, 29, 16,  2,  0,  0,  0, 44, 31, 30, 17,  3,  0,
- 0, 46, 45, 32, 18,  5,  4, 95,  0, 57, 47, 33, 20, 19,  6,183,
- 0, 49, 48, 35, 34, 21,  7,184,  0,  0, 50, 36, 22,  8,  9,185,
- 0, 51, 37, 23, 24, 11, 10,  0,  0, 52, 53, 38, 39, 25, 12,  0,
- 0, 89, 40,  0, 26, 13,  0,  0, 58, 54, 28, 27,  0, 43,  0, 85,
- 0, 86, 91, 90, 92,  0, 14, 94,  0, 79,124, 75, 71,121,  0,  0,
-82, 83, 80, 76, 77, 72,  1, 69, 87, 78, 81, 74, 55, 73, 70, 99,
-
-  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
-217,100,255,  0, 97,165,  0,  0,156,  0,  0,  0,  0,  0,  0,125,
-173,114,  0,113,  0,  0,  0,126,128,  0,  0,140,  0,  0,  0,127,
-159,  0,115,  0,164,  0,  0,116,158,  0,150,166,  0,  0,  0,142,
-157,  0,  0,  0,  0,  0,  0,  0,155,  0, 98,  0,  0,163,  0,  0,
-226,  0,  0,  0,  0,  0,  0,  0,  0,255, 96,  0,  0,  0,143,  0,
-  0,  0,  0,  0,  0,  0,  0,  0,  0,107,  0,105,102,  0,  0,112,
-110,111,108,112,106,103,  0,119,  0,118,109,  0, 99,104,119,  0,
-
-};
-
-const unsigned char atkbd_unxlate_table[128] = {
-
-  0,118, 22, 30, 38, 37, 46, 54, 61, 62, 70, 69, 78, 85,102, 13,
- 21, 29, 36, 45, 44, 53, 60, 67, 68, 77, 84, 91, 90, 20, 28, 27,
- 35, 43, 52, 51, 59, 66, 75, 76, 82, 14, 18, 93, 26, 34, 33, 42,
- 50, 49, 58, 65, 73, 74, 89,124, 17, 41, 88,  5,  6,  4, 12,  3,
- 11,  2, 10,  1,  9,119,126,108,117,125,123,107,115,116,121,105,
-114,122,112,113,127, 96, 97,120,  7, 15, 23, 31, 39, 47, 55, 63,
- 71, 79, 86, 94,  8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 87,111,
- 19, 25, 57, 81, 83, 92, 95, 98, 99,100,101,103,104,106,109,110
-
-};
-#endif
-
-/*
- * for (i = 0; i < 128; i++) {
- * scancode2linux[i] = atkbd_set2_keycode[atkbd_unxlate_table[i]];
- * scancode2linux[i | 0x80] = atkbd_set2_keycode[atkbd_unxlate_table[i] | 
0x80];
- * }
- */
-static const unsigned char scancode2linux[512] = {
-  0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
- 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
- 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
- 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
- 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79,
- 80, 81, 82, 83, 99,  0, 86, 87, 88,117,  0,  0, 95,183,184,185,
-  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
- 93,  0,  0, 89,  0,  0, 85, 91, 90, 92,  0, 94,  0,124,121,  0,
-
-  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
-165,  0,  0,  0,  0,  0,  0,  0,  0,163,  0,  0, 96, 97,  0,  0,
-113,140,164,  0,166,  0,  0,  0,  0,  0,255,  0,  0,  0,114,  0,
-115,  0,150,  0,  0, 98,255, 99,100,  0,  0,  0,  0,  0,  0,  0,
-  0,  0,  0,  0,  0,119,119,102,103,104,  0,105,112,106,118,107,
-108,109,110,111,  0,  0,  0,  0,  0,  0,  0,125,126,127,116,142,
-  0,  0,  0,143,  0,217,156,173,128,159,158,157,155,226,  0,112,
-  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
-};
-
 /* Send an event to the keyboard frontend driver */
 static int xenfb_kbd_event(struct XenInput *xenfb,
   union xenkbd_in_event *event)
@@ -260,37 +189,21 @@ static int xenfb_send_position(struct XenInput *xenfb,
 return xenfb_kbd_event(xenfb, &event);
 }
 
-/*
- * Send a key event from the client to the guest OS
- * QEMU gives us a raw scancode from an AT / PS/2 style keyboard.
- * 

[Qemu-devel] [PATCH v4 10/12] ui: convert the SDL2 frontend to keycodemapdb

[Daniel P. Berrange]

The SDL2 scancodes are conveniently identical to the USB
scancodes. Replace the sdl2_scancode_to_qcode table with
an automatically generated table.

Missing entries in sdl2_scancode_to_qcode now fixed:

  - 0x32 -> Q_KEY_CODE_BACKSLASH
  - 0x66 -> Q_KEY_CODE_POWER
  - 0x67 -> Q_KEY_CODE_KP_EQUALS
  - 0x74 -> Q_KEY_CODE_OPEN
  - 0x77 -> Q_KEY_CODE_FRONT
  - 0x7f -> Q_KEY_CODE_AUDIOMUTE
  - 0x80 -> Q_KEY_CODE_VOLUMEUP
  - 0x81 -> Q_KEY_CODE_VOLUMEDOWN
  - 0x85 -> Q_KEY_CODE_KP_COMMA
  - 0x87 -> Q_KEY_CODE_RO
  - 0x89 -> Q_KEY_CODE_YEN
  - 0x8a -> Q_KEY_CODE_HENKAN
  - 0x93 -> Q_KEY_CODE_HIRAGANA
  - 0xe8 -> Q_KEY_CODE_AUDIOPLAY
  - 0xe9 -> Q_KEY_CODE_AUDIOSTOP
  - 0xea -> Q_KEY_CODE_AUDIOPREV
  - 0xeb -> Q_KEY_CODE_AUDIONEXT
  - 0xed -> Q_KEY_CODE_VOLUMEUP
  - 0xee -> Q_KEY_CODE_VOLUMEDOWN
  - 0xef -> Q_KEY_CODE_AUDIOMUTE
  - 0xf1 -> Q_KEY_CODE_AC_BACK
  - 0xf2 -> Q_KEY_CODE_AC_FORWARD
  - 0xf3 -> Q_KEY_CODE_STOP
  - 0xf4 -> Q_KEY_CODE_FIND
  - 0xf8 -> Q_KEY_CODE_SLEEP
  - 0xfa -> Q_KEY_CODE_AC_REFRESH
  - 0xfb -> Q_KEY_CODE_CALCULATOR

And some mistakes corrected:

  - 0x65 -> Q_KEY_CODE_COMPOSE, not duplicating Q_KEY_CODE_MENU

Signed-off-by: Daniel P. Berrange 
---
 include/ui/input.h |   3 +
 ui/Makefile.objs   |   1 +
 ui/input-keymap.c  |   1 +
 ui/sdl2-input.c|  16 +++-
 ui/sdl2-keymap.h   | 267 -
 5 files changed, 16 insertions(+), 272 deletions(-)
 delete mode 100644 ui/sdl2-keymap.h

diff --git a/include/ui/input.h b/include/ui/input.h
index df3eb439bf..dcf7e0e731 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -98,4 +98,7 @@ extern const guint16 qemu_input_map_qcode_to_sun[];
 extern const guint qemu_input_map_qnum_to_qcode_len;
 extern const guint16 qemu_input_map_qnum_to_qcode[];
 
+extern const guint qemu_input_map_usb_to_qcode_len;
+extern const guint16 qemu_input_map_usb_to_qcode[];
+
 #endif /* INPUT_H */
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index d35634cdc7..0e5821f6eb 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -64,6 +64,7 @@ KEYCODEMAP_FILES = \
 ui/input-keymap-qcode-to-qnum.c \
 ui/input-keymap-qcode-to-sun.c \
 ui/input-keymap-qnum-to-qcode.c \
+ui/input-keymap-usb-to-qcode.c \
 $(NULL)
 
 GENERATED_FILES += $(KEYCODEMAP_FILES)
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index de11205dca..ad98b153cf 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -15,6 +15,7 @@
 #include "ui/input-keymap-qcode-to-qnum.c"
 #include "ui/input-keymap-qcode-to-sun.c"
 #include "ui/input-keymap-qnum-to-qcode.c"
+#include "ui/input-keymap-usb-to-qcode.c"
 
 int qemu_input_linux_to_qcode(unsigned int lnx)
 {
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 6e315ae800..605d781971 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -30,8 +30,6 @@
 #include "ui/sdl2.h"
 #include "sysemu/sysemu.h"
 
-#include "sdl2-keymap.h"
-
 static uint8_t modifiers_state[SDL_NUM_SCANCODES];
 
 void sdl2_reset_keys(struct sdl2_console *scon)
@@ -39,9 +37,11 @@ void sdl2_reset_keys(struct sdl2_console *scon)
 QemuConsole *con = scon ? scon->dcl.con : NULL;
 int i;
 
-for (i = 0; i < SDL_NUM_SCANCODES; i++) {
+for (i = 0 ;
+ i < SDL_NUM_SCANCODES && i < qemu_input_map_usb_to_qcode_len ;
+ i++) {
 if (modifiers_state[i]) {
-int qcode = sdl2_scancode_to_qcode[i];
+int qcode = qemu_input_map_usb_to_qcode[i];
 qemu_input_event_send_key_qcode(con, qcode, false);
 modifiers_state[i] = 0;
 }
@@ -51,9 +51,15 @@ void sdl2_reset_keys(struct sdl2_console *scon)
 void sdl2_process_key(struct sdl2_console *scon,
   SDL_KeyboardEvent *ev)
 {
-int qcode = sdl2_scancode_to_qcode[ev->keysym.scancode];
+int qcode;
 QemuConsole *con = scon ? scon->dcl.con : NULL;
 
+if (ev->keysym.scancode >= qemu_input_map_usb_to_qcode_len) {
+return;
+}
+
+qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
+
 if (!qemu_console_is_graphic(con)) {
 if (ev->type == SDL_KEYDOWN) {
 switch (ev->keysym.scancode) {
diff --git a/ui/sdl2-keymap.h b/ui/sdl2-keymap.h
deleted file mode 100644
index cbedaa477d..00
--- a/ui/sdl2-keymap.h
+++ /dev/null
@@ -1,267 +0,0 @@
-
-/* map SDL2 scancodes to QKeyCode */
-
-static const int sdl2_scancode_to_qcode[SDL_NUM_SCANCODES] = {
-[SDL_SCANCODE_A] = Q_KEY_CODE_A,
-[SDL_SCANCODE_B] = Q_KEY_CODE_B,
-[SDL_SCANCODE_C] = Q_KEY_CODE_C,
-[SDL_SCANCODE_D] = Q_KEY_CODE_D,
-[SDL_SCANCODE_E] = Q_KEY_CODE_E,
-[SDL_SCANCODE_F] = Q_KEY_CODE_F,
-[SDL_SCANCODE_G] = Q_KEY_CODE_G,
-[SDL_SCANCODE_H] = Q_KEY_CODE_H,
-[SDL_SCANCODE_I] = Q_KEY_CODE_I,
-[SDL_SCANCODE_J] = Q_KEY_CODE_J

Re: [Qemu-devel] [PULL for-2.10 0/1] bugfix for s390x

[Peter Maydell]

On 15 August 2017 at 08:31, Cornelia Huck  wrote:
> The following changes since commit 83c3a1f61673ef554facf4d6d29ed56c5a219f9d:
>
>   xlnx-qspi: add a property for mmio-execution (2017-08-14 14:17:18 +0100)
>
> are available in the git repository at:
>
>   git://github.com/cohuck/qemu tags/s390x-20170815
>
> for you to fetch changes up to 88b3739acdec0b8ccdbb65425b0d31235e2c7ca3:
>
>   pc-bios/s390-ccw: Use rm command during make clean (2017-08-15 09:06:01 
> +0200)
>
> 
> Regression fix for 'make clean' on s390x.
>
> 
>
> Eric Farman (1):
>   pc-bios/s390-ccw: Use rm command during make clean
>
>  pc-bios/s390-ccw/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [RFC 5/6] migration: store listen task tag

[Peter Xu]

On Tue, Aug 15, 2017 at 10:27:07AM +0100, Daniel P. Berrange wrote:
> On Tue, Aug 15, 2017 at 04:50:06PM +0800, Peter Xu wrote:
> > On Tue, Aug 15, 2017 at 09:37:14AM +0100, Daniel P. Berrange wrote:
> > > On Tue, Aug 15, 2017 at 02:17:06PM +0800, Peter Xu wrote:
> > > > Store the task tag for migration types: tcp/unix/fd/exec in current
> > > > MigrationIncomingState struct.
> > > > 
> > > > For defered migration, no need to store task tag since there is no task
> > > > running in the main loop at all. For RDMA, let's mark it as todo.
> > > > 
> > > > Signed-off-by: Peter Xu 
> > > > ---
> > > >  migration/migration.c | 22 ++
> > > >  migration/migration.h |  2 ++
> > > >  2 files changed, 20 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index c9b7085..daf356b 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -171,6 +171,7 @@ void migration_incoming_state_destroy(void)
> > > >  mis->from_src_file = NULL;
> > > >  }
> > > >  
> > > > +mis->listen_task_tag = 0;
> > > >  qemu_event_destroy(&mis->main_thread_load_event);
> > > >  }
> > > >  
> > > > @@ -265,25 +266,31 @@ int 
> > > > migrate_send_rp_req_pages(MigrationIncomingState *mis, const char 
> > > > *rbname,
> > > >  void qemu_start_incoming_migration(const char *uri, Error **errp)
> > > >  {
> > > >  const char *p;
> > > > +guint task_tag = 0;
> > > > +MigrationIncomingState *mis = migration_incoming_get_current();
> > > >  
> > > >  qapi_event_send_migration(MIGRATION_STATUS_SETUP, &error_abort);
> > > >  if (!strcmp(uri, "defer")) {
> > > >  deferred_incoming_migration(errp);
> > > >  } else if (strstart(uri, "tcp:", &p)) {
> > > > -tcp_start_incoming_migration(p, errp);
> > > > +task_tag = tcp_start_incoming_migration(p, errp);
> > > >  #ifdef CONFIG_RDMA
> > > >  } else if (strstart(uri, "rdma:", &p)) {
> > > > +/* TODO: store task tag for RDMA migrations */
> > > >  rdma_start_incoming_migration(p, errp);
> > > >  #endif
> > > >  } else if (strstart(uri, "exec:", &p)) {
> > > > -exec_start_incoming_migration(p, errp);
> > > > +task_tag = exec_start_incoming_migration(p, errp);
> > > >  } else if (strstart(uri, "unix:", &p)) {
> > > > -unix_start_incoming_migration(p, errp);
> > > > +task_tag = unix_start_incoming_migration(p, errp);
> > > >  } else if (strstart(uri, "fd:", &p)) {
> > > > -fd_start_incoming_migration(p, errp);
> > > > +task_tag = fd_start_incoming_migration(p, errp);
> > > >  } else {
> > > >  error_setg(errp, "unknown migration protocol: %s", uri);
> > > > +return;
> > > >  }
> > > > +
> > > > +mis->listen_task_tag = task_tag;
> > > 
> > > This is unsafe as currently written. The main loop watches that are
> > > associated with these tags are all unregistered when incoming migration
> > > starts. So if you save them, and then unregister later when postcopy
> > > is "stuck", then you're likely unregistering a tag associated with
> > > some other random part of QEMU, because the saved value is no longer
> > > valid.
> > 
> > IIUC for incoming migration, the watch is not detached until
> > migration_fd_process_incoming() completes. So:
> > 
> > - for precopy, the watch should be detached when migration completes
> > 
> > - for postcopy, the watch should be detached when postcopy starts,
> >   then main loop thread returns, while the ram_load_thread starts to
> >   continue the migration.
> > 
> > So basically the watch is detaching itself after
> > migration_fd_process_incoming() completes. To handle that, this patch
> > has this change:
> > 
> > @@ -422,6 +429,13 @@ void migration_fd_process_incoming(QEMUFile *f)
> >  co = qemu_coroutine_create(process_incoming_migration_co, f);
> >  qemu_coroutine_enter(co);
> >  }
> > +
> > +/*
> > + * When reach here, we should not need the listening port any
> > + * more. We'll detach the listening task soon, let's reset the
> > + * listen task tag.
> > + */
> > +mis->listen_task_tag = 0;
> > 
> > Would this make sure the listen_task_tag is always valid if nonzero?
> 
> Mixing 'return FALSE' from callbacks, with g_source_remove is a recipe
> for hard to diagnose bugs, so should be avoided in general.

Makes sense.

> 
> So, IMHO, you should change all the callbacks to 'return TRUE' so that
> they keep the watch registered, and then explicitly call g_source_remove()
> passing the listen tag, when incoming migration starts.

Yes this sounds better.  Thanks!

-- 
Peter Xu

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Markus Armbruster]

Eduardo Habkost  writes:

> It makes command-line parsing and generation of help text much
> simpler.

There's really no excuse for parsing command line arguments by hand in
Python.

> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).

We have a few uses of argparse in the tree.  Are they okay?

We also use getopt in places.  Perhaps we should pick one way to parse
command lines and stick to it.

>
> Signed-off-by: Eduardo Habkost 

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 11:47:28AM +0200, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > It makes command-line parsing and generation of help text much
> > simpler.
> 
> There's really no excuse for parsing command line arguments by hand in
> Python.
> 
> > The optparse module is deprecated since Python 2.7, but argparse
> > is not available in Python 2.6 (the minimum Python version
> > required for building QEMU).
> 
> We have a few uses of argparse in the tree.  Are they okay?
> 
> We also use getopt in places.  Perhaps we should pick one way to parse
> command lines and stick to it.

I just came up against this problem with keycodemapdb which I'm adding as
a submodule to qemu.  I used argparse there because it is the modern
recommended API for python >= 2.7.   I examined possibilty of using
optparse instead, but it lacks key features - in particular the idea
of 'subcommands' is entirely missing from optparse.

The 'argparse' module is part of python core, but is *also* available
as a standalone module that is implemented in terms of optparse.

So, I would suggest that we just copy the 'argparse' module into the
QEMU 'scripts/thirdparty' directory.

Then in any files which need argparse, you can do this

  try:
  import argparse
  except:
  import os, sys
  sys.path.append(os.path.join(os.path.dirname(__file__), "thirdparty"))
  import argparse

so that it tries to use the argparse provided by python, and falls back
to pulling in the one in our scripts/thirdparty directory.

When we finally bump our min python to 2.7, we can simply drop this
compat code for the import statement.  This avoids need for us to
re-write code to use a deprecated API, with a worse feature set.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.11 v2 1/5] qmp-shell: Use optparse module

[Daniel P. Berrange]

On Tue, Aug 08, 2017 at 05:39:31PM -0300, Eduardo Habkost wrote:
> It makes command-line parsing and generation of help text much
> simpler.
> 
> The optparse module is deprecated since Python 2.7, but argparse
> is not available in Python 2.6 (the minimum Python version
> required for building QEMU).
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Use optparse module, as the minimum Python version for building
>   QEMU is 2.6
>   * Reported-by: Stefan Hajnoczi 
>   * Suggested-by: "Daniel P. Berrange" 

I no longer suggest this approach :-)

I'd prefer if we just bundled a copy of 'argparse.py' in the
local scripts/thirdparty directory, and add that to the
python import path, if the system python lacks argparse.

eg if we had $QEMU-GIT/scripts/thirdparty/argparse.py you can
use:

try:
import argparse
except:
import os, sys
sys.path.append(os.path.join(os.path.dirname(__file__), "../thirdparty"))
import argparse

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Peter Maydell]

On 14 August 2017 at 21:44, Eric Farman  wrote:
> How often does one really do a "make clean" ?  Rather infrequently,
> as I only stumbled on this today.

FWIW, one of my standard pre-merge build tests does a
"make clean" and then a make (mostly as a check that we
do build from clean as well as incrementally). Not sure
why it didn't catch this...

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.11 v2 4/5] qmp-shell: Accept QMP command as argument

[Markus Armbruster]

Eduardo Habkost  writes:

Suggest to insert here:

  If additional arguments QMP-COMMAND ARG=VAL... are given, run just
  that QMP command instead of the REPL.

Question: is this limited to simple arguments?  If no, how would I write
an object argument?  For instance, how would I do

{ "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver": 
"nbd", "server": { "type": "inet", "host": "localhost", "port": "12345" } } }

?

> This is useful for testing QMP commands in scripts.
>
> Example usage, combined with 'jq' for filtering the results:
>
>   $ ./scripts/qmp/qmp-shell /tmp/qmp qom-list path=/ | jq -r .return[].name
>   machine
>   type
>   chardevs
>   backend

What's jq?

>   $

Let's drop this line.

>
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Rewritten using optparse module
> ---
>  scripts/qmp/qmp-shell | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
> index 4b9a420..4b7374e 100755
> --- a/scripts/qmp/qmp-shell
> +++ b/scripts/qmp/qmp-shell
> @@ -400,7 +400,7 @@ def die(msg):
>  
>  def main():
>  parser = optparse.OptionParser(description='QMP shell utility')
> -parser.set_usage("%prog [options]  |  address:port>")
> +parser.set_usage("%prog [options]  |  address:port> [COMMAND [ARG=VALUE]...]")
>  parser.add_option('-v', action='store_true', dest='verbose',
>  help='Verbose (echo command sent and received)')
>  parser.add_option('-p', action='store_true', dest='pretty',
> @@ -411,10 +411,11 @@ def main():
>  default=True, help='Skip negotiate (for qemu-ga)')
>  opts,args = parser.parse_args()
>  
> -if len(args) != 1:
> +if len(args) < 1:
>  parser.print_help(sys.stderr)
>  sys.exit(1)
>  addr = args[0]
> +cmdargs = args[1:]
>  
>  try:
>  if opts.hmp:
> @@ -433,10 +434,13 @@ def main():
>  except qemu.error:
>  die('Could not connect to %s' % addr)
>  
> -qemu.show_banner()
>  qemu.set_verbosity(opts.verbose)
> -while qemu.read_exec_command(qemu.get_prompt()):
> -pass
> +if len(cmdargs):

Superfluous len().

> +qemu.execute_cmdargs(cmdargs)
> +else:
> +qemu.show_banner()
> +while qemu.read_exec_command(qemu.get_prompt()):
> +pass
>  qemu.close()
>  
>  if __name__ == '__main__':

Re: [Qemu-devel] [PATCH v4 01/12] ui: add keycodemapdb repository as a GIT submodule

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 10:36:04AM +0100, Daniel P. Berrange wrote:
> The https://gitlab.com/keycodemap/keycodemapdb/ repo contains a
> data file mapping between all the different scancode/keycode/keysym
> sets that are known, and a tool to auto-generate lookup tables for
> different combinations.
> 
> It is used by GTK-VNC, SPICE-GTK and libvirt for mapping keys.
> Using it in QEMU will let us replace many hand written lookup
> tables with auto-generated tables from a master data source,
> reducing bugs. Adding new QKeyCodes will now only require the
> master table to be updated, all ~20 other tables will be
> automatically updated to follow.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  .gitignore|  2 ++
>  .gitmodules   |  3 +++
>  configure |  2 ++
>  tests/docker/Makefile.include | 11 +++
>  tests/docker/run  |  4 +++-
>  ui/Makefile.objs  | 18 ++
>  ui/keycodemapdb   |  1 +
>  7 files changed, 36 insertions(+), 5 deletions(-)
>  create mode 16 ui/keycodemapdb
> 

> diff --git a/configure b/configure
> index dd73cce62f..bd373ec2b4 100755
> --- a/configure
> +++ b/configure
> @@ -5258,6 +5258,8 @@ echo_version() {
>  fi
>  }
>  
> +(cd $source_path && git submodule update --init ui/keycodemapdb)
> +

Urgh, no, this won't work because of course you don't have to
have a git checkout when running configure.

Any suggestions on the "best" way to ensure that the ui/keycodemapdb
git submodule is always checked out, without requiring developers to
do something manually ?

I could try

  (cd $source_path && test -d .git && git submodule update --init 
ui/keycodemapdb)

but wonder if there's a better way ?

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] ppc: fix ppc_set_compat() with KVM PR

[Greg Kurz]

On Tue, 15 Aug 2017 14:04:09 +1000
David Gibson  wrote:

> On Mon, Aug 14, 2017 at 07:49:16PM +0200, Greg Kurz wrote:
> > When running in KVM PR mode, kvmppc_set_compat() always fail because the
> > current PR implementation doesn't handle KVM_REG_PPC_ARCH_COMPAT. Now that
> > the machine code inconditionally calls ppc_set_compat_all() at reset time
> > to restore the compat mode default value (commit 66d5c492dd3a9), it is
> > impossible to start a guest with PR:
> > 
> > qemu-system-ppc64: Unable to set CPU compatibility mode in KVM:
> >  Invalid argument
> > 
> > A tentative patch [1] was recently sent by Suraj to address the issue, but
> > it would prevent the compat mode to be turned off on reset. And we really
> > don't want to explicitely check for KVM PR. During the patch's review,
> > David suggested that we should only call the KVM ioctl() if the compat
> > PVR changes. This allows at least to run with KVM PR, provided no compat
> > mode is requested from the command line (which should be the case when
> > running PR nested). This is what this patch does.  
> 
> I'm not sure that's true any more, since we now prefer compat modes
> during CAS.  I guess we could exclude the compat modes from the

Oops, I wanted to talk about CAS and I... forgot :(

So, indeed, CAS fails to apply compat mode and immediately returns H_HARDWARE
to the guest, ie the rest of CAS isn't done :-\. Linux guests print a warning:

WARNING: ibm,client-architecture-support call FAILED!
 done

and (luckily?) continue to boot anyway (at least that's what happens with
RHEL7 guests).

> advertised list based on what KVM supports, but that seems pretty
> awkward.
> 

I'll re-read the details in LoPAPR and try to come up with something.

Cheers,

--
Greg

> > While here, we also fix the side effect where KVM would fail but we would
> > change the CPU state in QEMU anyway.
> > 
> > [1] http://patchwork.ozlabs.org/patch/782039/
> > 
> > Signed-off-by: Greg Kurz   
> 
> In any case, this makes things better than they were, so I'm applying.
> 
> > ---
> >  target/ppc/compat.c |9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> > index f1b67faa97e3..f8729fe46d61 100644
> > --- a/target/ppc/compat.c
> > +++ b/target/ppc/compat.c
> > @@ -140,16 +140,17 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t 
> > compat_pvr, Error **errp)
> >  
> >  cpu_synchronize_state(CPU(cpu));
> >  
> > -cpu->compat_pvr = compat_pvr;
> > -env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
> > -
> > -if (kvm_enabled()) {
> > +if (kvm_enabled() && cpu->compat_pvr != compat_pvr) {
> >  int ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
> >  if (ret < 0) {
> >  error_setg_errno(errp, -ret,
> >   "Unable to set CPU compatibility mode in 
> > KVM");
> > +return;
> >  }
> >  }
> > +
> > +cpu->compat_pvr = compat_pvr;
> > +env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
> >  }
> >  
> >  typedef struct {
> >   
> 



pgpYFhxJWm3lm.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Eric Farman]

On 08/15/2017 04:26 AM, Thomas Huth wrote:

On 15.08.2017 10:01, Thomas Huth wrote:

On 15.08.2017 09:03, Cornelia Huck wrote:

On Tue, 15 Aug 2017 07:02:10 +0200
Thomas Huth  wrote:


On 14.08.2017 22:44, Eric Farman wrote:

How often does one really do a "make clean" ?  Rather infrequently,
as I only stumbled on this today.

Perhaps I have missed the RM variable somewhere, as I see similar syntax
in some of the tests/tcg/ Makefiles, but I don't see it being set here.
My configure statement isn't terribly interesting, just enabling debug
for an s390x target, and as such there's no RM variable in its output.
I'll trust that Thomas will chime in with where it should have been.
In the meantime, this does the trick for me.


RM is one of the variables that should be pre-initialized by Make, and
AFAIK should be used to increase portability (well, it's likely not
important for QEMU since we require a posix-shell like built environment
anyway).

According to the info page of Make, chapter "10.3 Variables Used by
Implicit Rules":

`RM'
  Command to remove a file; default `rm -f'.

I've also checked it again and "make clean" works fine here (using GNU
Make 3.82). Which version of Make (and Linux distro) are you using?


Not that it matters now, but make 4.1 on F24



Interesting. It fails for me with GNU Make 3.82 on my RHEL guest as
well.


Anyway, maybe I also simply missed something, so I'm certainly also fine
with the patch to revert it to "rm -f".


Given that other bios makefiles use rm -f as well, let's just change
back until we figure out what's wrong.


I just discovered that it fails for me as well when I do "make clean"
from the top directory. So far I was only doing "make clean" after doing
a "cd pc-bios/s390-ccw" first, and that works fine. Weird. Something
seems to unset the RM variable in our build system, but I fail to find
the spot where this happens...


Ok, just found it: It's this line in rules.mak:

MAKEFLAGS += -rR

The parameter -R disables the built-in variables, so RM can indeed not
work here. Sorry, I wasn't aware of that setting yet, so your patch is
indeed the right fix here (or we should maybe define RM in rules.mak, too).


Excellent find, Thomas!  I was not aware of that either.  Thanks to both 
you and Cornelia for this.


 - Eric

Re: [Qemu-devel] [PATCH v4 01/12] ui: add keycodemapdb repository as a GIT submodule

[Fam Zheng]

On Tue, 08/15 11:04, Daniel P. Berrange wrote:
> On Tue, Aug 15, 2017 at 10:36:04AM +0100, Daniel P. Berrange wrote:
> > The https://gitlab.com/keycodemap/keycodemapdb/ repo contains a
> > data file mapping between all the different scancode/keycode/keysym
> > sets that are known, and a tool to auto-generate lookup tables for
> > different combinations.
> > 
> > It is used by GTK-VNC, SPICE-GTK and libvirt for mapping keys.
> > Using it in QEMU will let us replace many hand written lookup
> > tables with auto-generated tables from a master data source,
> > reducing bugs. Adding new QKeyCodes will now only require the
> > master table to be updated, all ~20 other tables will be
> > automatically updated to follow.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  .gitignore|  2 ++
> >  .gitmodules   |  3 +++
> >  configure |  2 ++
> >  tests/docker/Makefile.include | 11 +++
> >  tests/docker/run  |  4 +++-
> >  ui/Makefile.objs  | 18 ++
> >  ui/keycodemapdb   |  1 +
> >  7 files changed, 36 insertions(+), 5 deletions(-)
> >  create mode 16 ui/keycodemapdb
> > 
> 
> > diff --git a/configure b/configure
> > index dd73cce62f..bd373ec2b4 100755
> > --- a/configure
> > +++ b/configure
> > @@ -5258,6 +5258,8 @@ echo_version() {
> >  fi
> >  }
> >  
> > +(cd $source_path && git submodule update --init ui/keycodemapdb)
> > +
> 
> Urgh, no, this won't work because of course you don't have to
> have a git checkout when running configure.
> 
> Any suggestions on the "best" way to ensure that the ui/keycodemapdb
> git submodule is always checked out, without requiring developers to
> do something manually ?
> 
> I could try
> 
>   (cd $source_path && test -d .git && git submodule update --init 
> ui/keycodemapdb)
> 
> but wonder if there's a better way ?

I think the way dtc handles this is okay: probe headers/libs, if failed, hint
the "git submodule update" command in the error message.  This is also a
familiar/friendly user interface to developers.

(If you looks at the test snippet that patchew runs, there is an explicit
submodule command:

#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora

as the libfdt devel package is not available in every docker images.)

Fam

Re: [Qemu-devel] [PATCH v4 01/12] ui: add keycodemapdb repository as a GIT submodule

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 06:47:22PM +0800, Fam Zheng wrote:
> On Tue, 08/15 11:04, Daniel P. Berrange wrote:
> > On Tue, Aug 15, 2017 at 10:36:04AM +0100, Daniel P. Berrange wrote:
> > > The https://gitlab.com/keycodemap/keycodemapdb/ repo contains a
> > > data file mapping between all the different scancode/keycode/keysym
> > > sets that are known, and a tool to auto-generate lookup tables for
> > > different combinations.
> > > 
> > > It is used by GTK-VNC, SPICE-GTK and libvirt for mapping keys.
> > > Using it in QEMU will let us replace many hand written lookup
> > > tables with auto-generated tables from a master data source,
> > > reducing bugs. Adding new QKeyCodes will now only require the
> > > master table to be updated, all ~20 other tables will be
> > > automatically updated to follow.
> > > 
> > > Signed-off-by: Daniel P. Berrange 
> > > ---
> > >  .gitignore|  2 ++
> > >  .gitmodules   |  3 +++
> > >  configure |  2 ++
> > >  tests/docker/Makefile.include | 11 +++
> > >  tests/docker/run  |  4 +++-
> > >  ui/Makefile.objs  | 18 ++
> > >  ui/keycodemapdb   |  1 +
> > >  7 files changed, 36 insertions(+), 5 deletions(-)
> > >  create mode 16 ui/keycodemapdb
> > > 
> > 
> > > diff --git a/configure b/configure
> > > index dd73cce62f..bd373ec2b4 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -5258,6 +5258,8 @@ echo_version() {
> > >  fi
> > >  }
> > >  
> > > +(cd $source_path && git submodule update --init ui/keycodemapdb)
> > > +
> > 
> > Urgh, no, this won't work because of course you don't have to
> > have a git checkout when running configure.
> > 
> > Any suggestions on the "best" way to ensure that the ui/keycodemapdb
> > git submodule is always checked out, without requiring developers to
> > do something manually ?
> > 
> > I could try
> > 
> >   (cd $source_path && test -d .git && git submodule update --init 
> > ui/keycodemapdb)
> > 
> > but wonder if there's a better way ?
> 
> I think the way dtc handles this is okay: probe headers/libs, if failed, hint
> the "git submodule update" command in the error message.  This is also a
> familiar/friendly user interface to developers.

I don't think that's acceptable in this case. Few people will ever need
to setup the dtc submodule as distros commonly have that available.

For the keycodemapdb *EVERY* single person who builds QEMU from git will
need it, and they'll also need to update it periodically when the submodule
hash changes. So we need to have it automated IMHO.

> (If you looks at the test snippet that patchew runs, there is an explicit
> submodule command:
> 
> #!/bin/bash
> set -e
> git submodule update --init dtc
> # Let docker tests dump environment info
> export SHOW_ENV=1
> export J=8
> time make docker-test-quick@centos6
> time make docker-test-build@min-glib
> time make docker-test-mingw@fedora
> 
> as the libfdt devel package is not available in every docker images.)

IMHO that is a bad approach because someone typing
'make docker-test-mingw@fedora' is  going to be missing the dtc
module. You'll see in this case I extended the docker/Makefile
to always pull in keycodemapdb, not rely on someone remembering
todo it manually.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [PULL 2/5] docker: use one package per line in CentOS config

[Fam Zheng]

From: Philippe Mathieu-Daudé 

This ease rebase/cherry-pick, also it is faster to visually find if a package
is here.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20170728233316.13352-2-f4...@amsat.org>
Signed-off-by: Fam Zheng 
---
 tests/docker/dockerfiles/centos6.docker | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tests/docker/dockerfiles/centos6.docker 
b/tests/docker/dockerfiles/centos6.docker
index 17a4d24d54..9b91e832c2 100644
--- a/tests/docker/dockerfiles/centos6.docker
+++ b/tests/docker/dockerfiles/centos6.docker
@@ -1,8 +1,18 @@
 FROM centos:6
 RUN yum install -y epel-release
-ENV PACKAGES libfdt-devel ccache \
-tar git make gcc g++ flex bison \
-zlib-devel glib2-devel SDL-devel pixman-devel \
-epel-release
+ENV PACKAGES \
+bison \
+ccache \
+flex \
+g++ \
+gcc \
+git \
+glib2-devel \
+libfdt-devel \
+make \
+pixman-devel \
+SDL-devel \
+tar \
+zlib-devel
 RUN yum install -y $PACKAGES
 RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.13.4

[Qemu-devel] [PULL 0/5] Build and test patches

[Fam Zheng]

The following changes since commit 83c3a1f61673ef554facf4d6d29ed56c5a219f9d:

  xlnx-qspi: add a property for mmio-execution (2017-08-14 14:17:18 +0100)

are available in the git repository at:

  git://github.com/famz/qemu.git tags/build-and-test-pull-request

for you to fetch changes up to a8132a2f288c260fb43243eb3c092b5186d84968:

  docker: add centos7 image (2017-08-15 15:16:45 +0800)



One fix and a few docker test enhancements. Not 2.10 blocker, but it's
good to have them.



Fam Zheng (1):
  Makefile: Let "make check-help" work without running ./configure

Philippe Mathieu-Daudé (4):
  docker: use one package per line in CentOS config
  docker: add Xen libs to centos6 image
  docker: install more packages on CentOS to extend code coverage
  docker: add centos7 image

 Makefile|  2 --
 tests/Makefile.include  | 46 ++---
 tests/docker/dockerfiles/centos6.docker | 31 ++
 tests/docker/dockerfiles/centos7.docker | 31 ++
 4 files changed, 82 insertions(+), 28 deletions(-)
 create mode 100644 tests/docker/dockerfiles/centos7.docker

-- 
2.13.4

[Qemu-devel] [PULL 3/5] docker: add Xen libs to centos6 image

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20170728233316.13352-3-f4...@amsat.org>
Signed-off-by: Fam Zheng 
---
 tests/docker/dockerfiles/centos6.docker | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/centos6.docker 
b/tests/docker/dockerfiles/centos6.docker
index 9b91e832c2..8588a12eab 100644
--- a/tests/docker/dockerfiles/centos6.docker
+++ b/tests/docker/dockerfiles/centos6.docker
@@ -1,5 +1,5 @@
 FROM centos:6
-RUN yum install -y epel-release
+RUN yum install -y epel-release centos-release-xen
 ENV PACKAGES \
 bison \
 ccache \
@@ -13,6 +13,7 @@ ENV PACKAGES \
 pixman-devel \
 SDL-devel \
 tar \
+xen-devel \
 zlib-devel
 RUN yum install -y $PACKAGES
 RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.13.4

[Qemu-devel] [PULL 1/5] Makefile: Let "make check-help" work without running ./configure

[Fam Zheng]

Currently if you do "make check-help" in a fresh checkout, only an error
is printed which is not nice:

$ make check-help V=1
cc -nostdlib  -o check-help.mo
cc: fatal error: no input files
compilation terminated.
rules.mak:115: recipe for target 'check-help.mo' failed
make: *** [check-help.mo] Error 1

Move the config-host.mak condition into the body of
tests/Makefile.include and always include the rule for check-help.

Reported-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
Message-Id: <20170810085025.14076-1-f...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 Makefile   |  2 --
 tests/Makefile.include | 46 +-
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/Makefile b/Makefile
index 97a58a0f4e..81447b1f08 100644
--- a/Makefile
+++ b/Makefile
@@ -281,9 +281,7 @@ dummy := $(call unnest-vars,, \
 common-obj-m \
 trace-obj-y)
 
-ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/tests/Makefile.include
-endif
 
 all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all modules
 
diff --git a/tests/Makefile.include b/tests/Makefile.include
index eb4895f94a..37c1bed683 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1,3 +1,26 @@
+
+.PHONY: check-help
+check-help:
+   @echo "Regression testing targets:"
+   @echo
+   @echo " make checkRun all tests"
+   @echo " make check-qtest-TARGET   Run qtest tests for given target"
+   @echo " make check-qtest  Run qtest tests"
+   @echo " make check-unit   Run qobject tests"
+   @echo " make check-speed  Run qobject speed tests"
+   @echo " make check-qapi-schemaRun QAPI schema tests"
+   @echo " make check-block  Run block tests"
+   @echo " make check-report.htmlGenerates an HTML test report"
+   @echo " make check-clean  Clean the tests"
+   @echo
+   @echo "Please note that HTML reports do not regenerate if the unit 
tests"
+   @echo "has not changed."
+   @echo
+   @echo "The variable SPEED can be set to control the gtester speed 
setting."
+   @echo "Default options are -k and (for make V=1) --verbose; they can be"
+   @echo "changed with variable GTESTER_OPTIONS."
+
+ifneq ($(wildcard config-host.mak),)
 export SRC_PATH
 
 qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
@@ -802,27 +825,6 @@ $(check-qtest-y): $(qtest-obj-y)
 
 tests/test-qga: tests/test-qga.o $(qtest-obj-y)
 
-.PHONY: check-help
-check-help:
-   @echo "Regression testing targets:"
-   @echo
-   @echo " make checkRun all tests"
-   @echo " make check-qtest-TARGET   Run qtest tests for given target"
-   @echo " make check-qtest  Run qtest tests"
-   @echo " make check-unit   Run qobject tests"
-   @echo " make check-speed  Run qobject speed tests"
-   @echo " make check-qapi-schemaRun QAPI schema tests"
-   @echo " make check-block  Run block tests"
-   @echo " make check-report.htmlGenerates an HTML test report"
-   @echo " make check-clean  Clean the tests"
-   @echo
-   @echo "Please note that HTML reports do not regenerate if the unit 
tests"
-   @echo "has not changed."
-   @echo
-   @echo "The variable SPEED can be set to control the gtester speed 
setting."
-   @echo "Default options are -k and (for make V=1) --verbose; they can be"
-   @echo "changed with variable GTESTER_OPTIONS."
-
 SPEED = quick
 GTESTER_OPTIONS = -k $(if $(V),--verbose,-q)
 GCOV_OPTIONS = -n $(if $(V),-f,)
@@ -917,3 +919,5 @@ all: $(QEMU_IOTESTS_HELPERS-y)
 
 -include $(wildcard tests/*.d)
 -include $(wildcard tests/libqos/*.d)
+
+endif
-- 
2.13.4

[Qemu-devel] [PULL 4/5] docker: install more packages on CentOS to extend code coverage

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20170728233316.13352-4-f4...@amsat.org>
Signed-off-by: Fam Zheng 
---
 tests/docker/dockerfiles/centos6.docker | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tests/docker/dockerfiles/centos6.docker 
b/tests/docker/dockerfiles/centos6.docker
index 8588a12eab..f6aae13f29 100644
--- a/tests/docker/dockerfiles/centos6.docker
+++ b/tests/docker/dockerfiles/centos6.docker
@@ -2,17 +2,27 @@ FROM centos:6
 RUN yum install -y epel-release centos-release-xen
 ENV PACKAGES \
 bison \
+bzip2-devel \
 ccache \
+csnappy-devel \
 flex \
 g++ \
 gcc \
 git \
 glib2-devel \
+libepoxy-devel \
 libfdt-devel \
+librdmacm-devel \
+lzo-devel \
 make \
+mesa-libEGL-devel \
+mesa-libgbm-devel \
 pixman-devel \
 SDL-devel \
+spice-glib-devel \
+spice-server-devel \
 tar \
+vte-devel \
 xen-devel \
 zlib-devel
 RUN yum install -y $PACKAGES
-- 
2.13.4

[Qemu-devel] [PULL 5/5] docker: add centos7 image

[Fam Zheng]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20170728233316.13352-5-f4...@amsat.org>
Signed-off-by: Fam Zheng 
---
 tests/docker/dockerfiles/centos7.docker | 31 +++
 1 file changed, 31 insertions(+)
 create mode 100644 tests/docker/dockerfiles/centos7.docker

diff --git a/tests/docker/dockerfiles/centos7.docker 
b/tests/docker/dockerfiles/centos7.docker
new file mode 100644
index 00..0b59aa2f26
--- /dev/null
+++ b/tests/docker/dockerfiles/centos7.docker
@@ -0,0 +1,31 @@
+FROM centos:7
+RUN yum install -y epel-release centos-release-xen
+RUN yum -y update
+ENV PACKAGES \
+bison \
+bzip2-devel \
+ccache \
+csnappy-devel \
+flex \
+g++ \
+gcc \
+git \
+glib2-devel \
+libepoxy-devel \
+libfdt-devel \
+librdmacm-devel \
+lzo-devel \
+make \
+mesa-libEGL-devel \
+mesa-libgbm-devel \
+pixman-devel \
+SDL-devel \
+spice-glib-devel \
+spice-server-devel \
+tar \
+vte-devel \
+xen-devel \
+zlib-devel
+RUN yum install -y $PACKAGES
+RUN rpm -q $PACKAGES | sort > /packages.txt
+
-- 
2.13.4

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Cornelia Huck]

On Tue, 15 Aug 2017 11:00:51 +0100
Peter Maydell  wrote:

> On 14 August 2017 at 21:44, Eric Farman  wrote:
> > How often does one really do a "make clean" ?  Rather infrequently,
> > as I only stumbled on this today.  
> 
> FWIW, one of my standard pre-merge build tests does a
> "make clean" and then a make (mostly as a check that we
> do build from clean as well as incrementally). Not sure
> why it didn't catch this...

Probably because you don't build on s390x... I'll add it to my
workflow, though.

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Peter Maydell]

On 15 August 2017 at 12:07, Cornelia Huck  wrote:
> On Tue, 15 Aug 2017 11:00:51 +0100
> Peter Maydell  wrote:
>
>> On 14 August 2017 at 21:44, Eric Farman  wrote:
>> > How often does one really do a "make clean" ?  Rather infrequently,
>> > as I only stumbled on this today.
>>
>> FWIW, one of my standard pre-merge build tests does a
>> "make clean" and then a make (mostly as a check that we
>> do build from clean as well as incrementally). Not sure
>> why it didn't catch this...
>
> Probably because you don't build on s390x...

I do, but not with a 'make clean' step :-)

thanks
-- PMM

[Qemu-devel] [Bug 1708442] Re: Crash(assert) during reading image from http url through qemu-nbd

[Andrey Smetanin]

ping

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

Title:
  Crash(assert) during reading  image from http url through qemu-nbd

Status in QEMU:
  New

Bug description:
  Description:
  During reading image from nbd device mounted by qemu-nbd server with url 
backend I/O error happens
  "blk_update_request: I/O error, dev nbd0, sector 42117" dmesg. After some 
investigation I found that qemu-nbd server aborts in aio_co_enter() assert in 
util/async.c:468.

  Steps to reproduce:

  1) sudo go run qemu-nbd-bug-report/qemu-nbd-bug.go (see qemu-nbd-bug-
  report.tar.gz)

  or try directly

  1) qemu-nbd -c /dev/nbd0 -r -v --aio=native -f qcow2 
json:{"file.driver":"http","file.url":"http://localhost:9666/image","file.readahead":3276800
  2) try read whole nbd device while error "blk_update_request: I/O error, dev 
nbd0, sector 42117" appears in dmesg

  Versions:

  1) qemu built from sources(/configure --target-list=x86_64-softmmu 
--disable-user --enable-curl --enable-linux-aio --enable-virtfs --enable-debug 
--disable-pie
  , top commit 5619c179057e24195ff19c8fe6d6a6cbcb16ed28):

  qemu-nbd -v
  qemu-nbd 2.9.90 (v2.10.0-rc0-67-g5619c17)

  2) libcurl(built from sources, top commit
  1767adf4399bb3be29121435e1bb1cc2bc05f7bf):

  curl -V
  curl 7.55.0-DEV (Linux) libcurl/7.55.0-DEV OpenSSL/1.0.2g zlib/1.2.8

  Backtrace:
  (gdb) bt
  #0  0x7f7131426428 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
  #1  0x7f713142802a in __GI_abort () at abort.c:89
  #2  0x7f713141ebd7 in __assert_fail_base (fmt=, 
assertion=assertion@entry=0x54c924 "self != co",
  file=file@entry=0x54c871 "util/async.c", line=line@entry=468,
  function=function@entry=0x54c980 <__PRETTY_FUNCTION__.24766> 
"aio_co_enter") at assert.c:92
  #3  0x7f713141ec82 in __GI___assert_fail (assertion=0x54c924 "self != 
co", file=0x54c871 "util/async.c", line=468,
  function=0x54c980 <__PRETTY_FUNCTION__.24766> "aio_co_enter") at 
assert.c:101
  #4  0x004fe6a2 in aio_co_enter (ctx=0xf0ddb0, co=0xf14650) at 
util/async.c:468
  #5  0x004fe637 in aio_co_wake (co=0xf14650) at util/async.c:456
  #6  0x00495c8a in curl_read_cb (ptr=0xf566d9, size=1, nmemb=16135, 
opaque=0xf1cb90) at block/curl.c:275
  #7  0x7f713242ac24 in Curl_client_chop_write () from 
/usr/lib/x86_64-linux-gnu/libcurl.so
  #8  0x7f713242ae03 in Curl_client_write () from 
/usr/lib/x86_64-linux-gnu/libcurl.so
  #9  0x7f713244e1cf in readwrite_data () from 
/usr/lib/x86_64-linux-gnu/libcurl.so
  #10 0x7f713244eb6f in Curl_readwrite () from 
/usr/lib/x86_64-linux-gnu/libcurl.so
  #11 0x7f713245c1bb in multi_runsingle () from 
/usr/lib/x86_64-linux-gnu/libcurl.so
  #12 0x7f713245d819 in multi_socket () from 
/usr/lib/x86_64-linux-gnu/libcurl.so
  #13 0x7f713245e067 in curl_multi_socket_action () from 
/usr/lib/x86_64-linux-gnu/libcurl.so
  #14 0x00497555 in curl_setup_preadv (bs=0xf16820, acb=0x7f712d379860) 
at block/curl.c:918
  #15 0x004975fb in curl_co_preadv (bs=0xf16820, offset=6556160, 
bytes=512, qiov=0x7f712d379b40, flags=0) at block/curl.c:935
  #16 0x0047730f in bdrv_driver_preadv (bs=0xf16820, offset=6556160, 
bytes=512, qiov=0x7f712d379b40, flags=0) at block/io.c:836
  #17 0x00477c1f in bdrv_aligned_preadv (child=0xf1be20, 
req=0x7f712d379a60, offset=6556160, bytes=512, align=1,
  qiov=0x7f712d379b40, flags=0) at block/io.c:1086
  #18 0x00478109 in bdrv_co_preadv (child=0xf1be20, offset=6556160, 
bytes=512, qiov=0x7f712d379b40, flags=0) at block/io.c:1180
  #19 0x00437498 in qcow2_co_preadv (bs=0xf0fdc0, offset=21563904, 
bytes=512, qiov=0x7f712d379e80, flags=0)
  at block/qcow2.c:1812
  #20 0x0047730f in bdrv_driver_preadv (bs=0xf0fdc0, offset=21563904, 
bytes=512, qiov=0x7f712d379e80, flags=0)
  at block/io.c:836
  #21 0x00477c1f in bdrv_aligned_preadv (child=0xf1c0d0, 
req=0x7f712d379d30, offset=21563904, bytes=512, align=1,
  qiov=0x7f712d379e80, flags=0) at block/io.c:1086
  #22 0x00478109 in bdrv_co_preadv (child=0xf1c0d0, offset=21563904, 
bytes=512, qiov=0x7f712d379e80, flags=0)
  at block/io.c:1180
  #23 0x004645ad in blk_co_preadv (blk=0xf1be90, offset=21563904, 
bytes=512, qiov=0x7f712d379e80, flags=0)
  at block/block-backend.c:991
  #24 0x004646fa in blk_read_entry (opaque=0x7f712d379ea0) at 
block/block-backend.c:1038
  #25 0x0046481c in blk_prw (blk=0xf1be90, offset=21563904,
  ---Type  to continue, or q  to quit---
  buf=0xf7f000 
"2,NV\241t!\ti\312\vp\364\017Kl*\354\021\a\177\021\260\b\027\212\347\027\004\322\nG\340b\\\306pG\332\313\060\341;\002\360\063L\240\027T
 
\211\341\305\022АE\230\356DǮ}\211\bx\016\a\b\313\350\316\064.\017\372\032-R\376z\261\263\350|cQ<\016S_L\340A\221\366~L#\001+\271\204\065~\327\023\027I\211\343\361\276zT$4\3

[Qemu-devel] [PATCH for-2.10 v2 2/2] Revert "ACPI: don't call acpi_pcihp_device_plug_cb on xen"

[Anthony PERARD]

This reverts commit 153eba4726dfa1bdfc31d1fe973b2a61b9035492.

This patch prevents PCI passthrough hotplug on Xen. Even if the Xen tool
stack prepares its own ACPI tables, we still rely on QEMU for hotplug
ACPI notifications.

The original issue is fixed by the previous patch (hw/acpi: Call
acpi_set_pci_info when no ACPI tables needed).

Signed-off-by: Anthony PERARD 
---
CC: Stefano Stabellini 
CC: Bruce Rogers 
---
 hw/acpi/piix4.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index f276967365..f4fd5907b8 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -385,10 +385,7 @@ static void piix4_device_plug_cb(HotplugHandler 
*hotplug_dev,
 dev, errp);
 }
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
-if (!xen_enabled()) {
-acpi_pcihp_device_plug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev,
-  errp);
-}
+acpi_pcihp_device_plug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev, 
errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
 if (s->cpu_hotplug_legacy) {
 legacy_acpi_cpu_plug_cb(hotplug_dev, &s->gpe_cpu, dev, errp);
@@ -411,10 +408,8 @@ static void piix4_device_unplug_request_cb(HotplugHandler 
*hotplug_dev,
 acpi_memory_unplug_request_cb(hotplug_dev, &s->acpi_memory_hotplug,
   dev, errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
-if (!xen_enabled()) {
-acpi_pcihp_device_unplug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev,
-errp);
-}
+acpi_pcihp_device_unplug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev,
+errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
!s->cpu_hotplug_legacy) {
 acpi_cpu_unplug_request_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
-- 
Anthony PERARD

[Qemu-devel] [PATCH for-2.10 v2 1/2] hw/acpi: Call acpi_set_pci_info when no ACPI tables needed

[Anthony PERARD]

To do PCI passthrough with Xen, the property acpi-pcihp-bsel needs to be
set, but this was done only when ACPI tables are built which is not
needed for a Xen guest. The need for the property starts with commit
"pc: pcihp: avoid adding ACPI_PCIHP_PROP_BSEL twice"
(f0c9d64a68b776374ec4732424a3e27753ce37b6).

Set pci info before checking for the needs to build ACPI tables.

Assign bsel=0 property only to the root bus on Xen as there is no
support in the Xen ACPI tables for a different value.

Reported-by: Sander Eikelenboom 
Signed-off-by: Anthony PERARD 

---
Changes in V2:
  - check for acpi_enabled before calling acpi_set_pci_info.
  - set the property on the root bus only.

This patch would be a canditade to backport to 2.9.

CC: Stefano Stabellini 
CC: Bruce Rogers 
---
 hw/i386/acpi-build.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 98dd424678..c0483b96cf 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -46,6 +46,7 @@
 #include "sysemu/tpm_backend.h"
 #include "hw/timer/mc146818rtc_regs.h"
 #include "sysemu/numa.h"
+#include "hw/xen/xen.h"
 
 /* Supported chipsets: */
 #include "hw/acpi/piix4.h"
@@ -518,8 +519,13 @@ static void acpi_set_pci_info(void)
 unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
 
 if (bus) {
-/* Scan all PCI buses. Set property to enable acpi based hotplug. */
-pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc);
+if (xen_enabled()) {
+/* Assign BSEL property to root bus only. */
+acpi_set_bsel(bus, &bsel_alloc);
+} else {
+/* Scan all PCI buses. Set property to enable acpi based hotplug. 
*/
+pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, 
&bsel_alloc);
+}
 }
 }
 
@@ -2871,6 +2877,14 @@ void acpi_setup(void)
 AcpiBuildState *build_state;
 Object *vmgenid_dev;
 
+if (!acpi_enabled) {
+ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n");
+return;
+}
+
+/* Assign BSEL property on hotpluggable PCI buses. */
+acpi_set_pci_info();
+
 if (!pcms->fw_cfg) {
 ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
 return;
@@ -2881,15 +2895,8 @@ void acpi_setup(void)
 return;
 }
 
-if (!acpi_enabled) {
-ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n");
-return;
-}
-
 build_state = g_malloc0(sizeof *build_state);
 
-acpi_set_pci_info();
-
 acpi_build_tables_init(&tables);
 acpi_build(&tables, MACHINE(pcms));
 
-- 
Anthony PERARD

[Qemu-devel] [PATCH for-2.10 v2 0/2] Fix hotplug of PCI passthrought device on Xen

[Anthony PERARD]

Adding PCI passthrough before the guest start works fine (broken in 2.9 but now
fixed), but hotplug does not work anymore.

Anthony PERARD (2):
  hw/acpi: Call acpi_set_pci_info when no ACPI tables needed
  Revert "ACPI: don't call acpi_pcihp_device_plug_cb on xen"

 hw/acpi/piix4.c  | 11 +++
 hw/i386/acpi-build.c | 25 -
 2 files changed, 19 insertions(+), 17 deletions(-)

-- 
Anthony PERARD

Re: [Qemu-devel] RFH: difference in read-only mapped bios.bin - memory corruption?

[Laszlo Ersek]

On 08/14/17 20:39, Dr. David Alan Gilbert wrote:
> * Philipp Hahn (h...@univention.de) wrote:
>> Hello,
>>
>> I'm currently investigating a problem, were a Linux VM does not reboot
>> and gets stuck in the SeaBIOS reboot code:
>>
>> I'm using SeaBIOS-1.7 from Debian with a more modern qemu-2.8
>>
>>> virsh # qemu-monitor-command --hmp ucs41-414 info roms
>>> fw=genroms/kvmvapic.bin size=0x002400 name="kvmvapic.bin"
>>> addr=fffe size=0x02 mem=rom name="bios.bin"
>>
>> which (to my understanding) is mapped at two physical locations:
>>> virsh # qemu-monitor-command --hmp ucs41-414 info mtree
>> ...> memory-region: system
>> ...>   000e-000f (prio 1, R-): alias
>> isa-bios @pc.bios -0001
>>>   fffe- (prio 0, R-): pc.bios
>>
>> If I dump both regions and compare them, I get a difference:
>>> virsh # qemu-monitor-command --pretty --domain ucs41-414 
>>> '{"execute":"pmemsave","arguments":{"val":917504,"size":131072,"filename":"/tmp/bios-low.dump"}}'
>>> virsh # qemu-monitor-command --pretty --domain ucs41-414 
>>> '{"execute":"pmemsave","arguments":{"val":4294836224,"size":131072,"filename":"/tmp/bios-high.dump"}}'
>>> # diff --suppress-common-lines -y <(od -Ax -tx1 -w1 -v /tmp/bios-low.dump) 
>>> <(od -Ax -tx1 -w1 -v /tmp/bios-high.dump)
>>> 00f798 fa | 00f798 80
>>> 00f799 7a | 00f799 89
>>> 00f79a f4 | 00f79a f2
>>> 016d40 00 | 016d40 ff
>>> 016d41 00 | 016d41 ff
>>> 016d42 00 | 016d42 ff
>>> 016d43 00 | 016d43 ff
>>
>> The high address dump is the same as the original:
> 
> You might want seabios commit c68aff5 and b837e6 that got fixed after
> I tracked down some reboot hangs - although they were rare, not every
> time.  c68aff5 did certainly cause a corruption, and the address of that
> corruption was determined at link time and could overlay random useful
> bits of code if you were unlucky.
> 
>>> # cmp -l /tmp/bios-high.dump /usr/share/seabios/bios.bin
>>
>>> virsh # qemu-monitor-command --hmp ucs41-414 x/6i 0x000ef78f
>>> 0x000ef78f:  mov$0xcf8,%esi
>>> 0x000ef794:  mov$0xfa00,%eax
>>> 0x000ef799:  jp 0xef78f
>>^^ BUG: endless loop
>>> 0x000ef79b:  out%eax,(%dx)
>>> 0x000ef79c:  mov$0xfe,%dl
>>> 0x000ef79e:  in (%dx),%ax
>>
>>> virsh # qemu-monitor-command --hmp ucs41-414 xp/6i 0xfffef78f
>>> 0xfffef78f:  mov$0xcf8,%esi
>>> 0xfffef794:  mov$0x8000,%eax
>>> 0xfffef799:  mov%esi,%edx
>> CORRECT original code
>>> 0xfffef79b:  out%eax,(%dx)
>>> 0xfffef79c:  mov$0xfe,%dl
>>> 0xfffef79e:  in (%dx),%ax
>>
>> (That's some code from seabios-1.7.0/src/pci.c)
>>
>> I had exactly the same run some weeks ago, but I also get different
>> patterns:
>>> # diff --suppress-common-lines -y <(od -Ax -tx1 -w1 -v /tmp/bios2.dump) 
>>> <(od -Ax -tx1 -w1 -v bios.bin)
>>> 00f798 f0 | 00f798 80
>>> 00f799 8d | 00f799 89
>>> 00f79a f3 | 00f79a f2
>>> 016d40 00 | 016d40 ff
>>> 016d41 00 | 016d41 ff
>>> 016d42 00 | 016d42 ff
>>> 016d43 00 | 016d43 ff
>>
>> Not all runs lead to reboot problems, but I don't know if any other
>> corruption happened there.
>>
>> I had a similar problem with OVMF back in June
>> ,
>> which I "solved" by upgrading the OVMF version: I have not seen the
>> problem there since than, but this problems looks very similar.
>>
>>
>> 1. How can it be, that the low-mem ROM mapping is modified?
> 
> I can't remember all the details, but PC ROM is shadowed and mapped over
> with RAM at various times,

Right. I don't remember for sure, but I believe the state of the PAM
registers doesn't only affect what the VCPUs see in that address range,
but also what your monitor commands will dump. (This would be the
logical choice -- make the monitor output what the VCPUs see anyway, at
the moment, dependent on the PAM settings.)

Thanks
Laszlo

> and the bioses play lots of silly tricks of
> being copied and then reusing bits of the copied space as temporaries
> and. oh it's just a mess.
> 
> D

Re: [Qemu-devel] [PATCH 04/28] sparc: convert cpu models to SPARC cpu subclasses

[Mark Cave-Ayland]

On 14/08/17 08:56, Igor Mammedov wrote:

> On Fri, 14 Jul 2017 15:51:55 +0200
> Igor Mammedov  wrote:
> 
>> QOMfy cpu models handling introducing propper cpu types
>> for each cpu model.
>>
>> Signed-off-by: Igor Mammedov 
>> ---
>> with this and conversion of features to properties,
>> it would be possible to replace cpu_sparc_init() with
>> cpu_generic_init() and reuse common -cpu handling
>> infrastructure.
>>
>> CC: Mark Cave-Ayland 
>> CC: Artyom Tarasenko 
> 
> ping,
> 
> Mark, Artoym,
> 
> As SPARC maintainers, cloud you please review/test patches 4-10/28.

Yes, sorry - it has been a fairly busy few weeks. I should have some
time to look at these towards the end of the week.


ATB,

Mark.

Re: [Qemu-devel] [PATCH] target-i386/cpu: Add new EYPC CPU model

[Eduardo Habkost]

Hi,

Thanks for the patch.

On Mon, Aug 14, 2017 at 10:52:17AM -0500, Brijesh Singh wrote:
> Add a new base CPU model called 'EPYC' to model processors from AMD EPYC
> family (which includes EPYC 76xx,75xx,74xx,73xx and 72xx).

I suggest enumerating in the commit message which features were
added to the CPU model in comparison to Opteron_G5.

> 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Tom Lendacky 
> Signed-off-by: Brijesh Singh 
> ---
>  target/i386/cpu.c | 44 
>  1 file changed, 44 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ddc45ab..ed1708b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1522,6 +1522,50 @@ static X86CPUDefinition builtin_x86_defs[] = {
>  .xlevel = 0x801A,
>  .model_id = "AMD Opteron 63xx class CPU",
>  },
> +{
> +.name = "EPYC",
> +.level = 0xd,
> +.vendor = CPUID_VENDOR_AMD,
> +.family = 23,
> +.model = 1,
> +.stepping = 2,
[...]
> +/* Missing: XSAVES (not supported by some Linux versions,
> + * including v4.1 to v4.12).
> + * KVM doesn't yet expose any XSAVES state save component.
> + */

Do you know which supervisor state components are available in
EPYC CPUs?  Do you have a pointer to public AMD documentation
about XSAVES?


> +.features[FEAT_XSAVE] =
> +CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
> +CPUID_XSAVE_XGETBV1,
> +.features[FEAT_6_EAX] =
> +CPUID_6_EAX_ARAT,
> +.xlevel = 0x801F,

All CPUID leaves from 0x800B to 0x801F return all-zeroes
today.  If we set xlevel to 0x801F before we actually
implement those CPUID leaves, we will be forced to add extra
machine-type compat code when we finally implement them.

I suggest setting it to 0x800A, and increasing it only after
we actually implement the new CPUID leaves.


> +.model_id = "AMD EYPC Processor",
> +},
>  };
>  
>  typedef struct PropValue {
> -- 
> 2.9.4
> 

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH 08/12] hw/ide: Emulate SiI3112 SATA controller

[David Gibson]

On Mon, Aug 14, 2017 at 01:16:13PM +0200, BALATON Zoltan wrote:
> On Mon, 14 Aug 2017, David Gibson wrote:
> > On Sun, Aug 13, 2017 at 07:04:38PM +0200, BALATON Zoltan wrote:
> > > Signed-off-by: BALATON Zoltan 
> > 
> > Needs a commit message: where does this controller appear? what's
> > distinctive about it? is there a link to a datasheet or programming manual?
> 
> This is just a generic PCI SATA controller, quite common and also used in
> PCs. Nothing special about it apart from guests running on Sam460ex usually
> have a driver for it (unlike other already emulated SATA controllers) and it
> is simpler to implement than the on board SATA controller of the 460EX and
> also has two ports instead of one so more useful to have.
> 
> I could add this as commit message if you think that would be better or just
> submit it separately through the IDE branch then use from these patch
> series.

Please do add that as a commit message.  We probably should get an ack
from Jon Snow for this.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 0/6] ppc patch queue 2017-08-09

[David Gibson]

On Mon, Aug 14, 2017 at 05:20:21PM +0300, Michael S. Tsirkin wrote:
> On Fri, Aug 11, 2017 at 09:59:00AM +0100, Peter Maydell wrote:
> > On 11 August 2017 at 09:51, David Gibson  
> > wrote:
> > > On Thu, Aug 10, 2017 at 01:47:03PM +0100, Peter Maydell wrote:
> > >> On 9 August 2017 at 08:03, David Gibson  
> > >> wrote:
> > >> > I haven't completed a Travis build for this, which is part of my usual
> > >> > test regime, since the first dozen or so Travis builds are failing
> > >> > more often than not on master as well.  I don't know why this is -
> > >> > seems to be failing some of the x86 tests.
> > >>
> > >> Doesn't look much worse than usual to me -- of the last 24 travis
> > >> builds for master 4 failed and 20 passed.
> > >
> > > Uh.. that's not been my experience.  I was a bit unclear in my
> > > description, though.  Until maybe a week and a half ago I found the
> > > Travis build was fairly reliable, though there were occasional
> > > transien failures.  Now, essentially every Travis build is failing for
> > > me.  Specifically most of the first dozen or so of the batch of build
> > > configurations on Travis fail (that's "X" not "!").  These seem to be
> > > transient failures in the sense that if I rebuild enough times they'll
> > > eventually pass, but unlike earlier when the builds would suceed most
> > > of the time, they now seem to succeed at best around 1/3 of the time
> > > (that's ~1/3 of the time for each configuration sub-build, so with a
> > > dozen or so apparently affected that means a complete passing build
> > > essentially never).
> > >
> > > See for example https://travis-ci.org/dgibson/qemu/builds/263312174
> > > where 5 subbuilds have failed (which is relatively few).  In each case
> > > the failing error seems to be something like:
> > >
> > > ERROR:tests/vhost-user-test.c:807:test_connect_fail: child process 
> > > (/i386/vhost-user/connect-fail/subprocess [55883]) failed unexpectedly
> > >
> > > Seems to be i386 in some cases and x86_64 in others.
> 
> And is it always the connect-fail test?

I think so, though I haven't checked every single failing output to
make sure.

> 
> > Weird. https://travis-ci.org/qemu/qemu/builds is the master
> > build history and as you can see it's mostly greens.
> > Not sure why your setup would be significantly worse:
> > maybe one of the travis submaintainers has an idea.
> > It could just be our vhost-user-test is flaky but I don't
> > know why that should hit you much more than master...
> > 
> > 
> > thanks
> > -- PMM
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10 3/4] block-backend: Defer shared_perm tightening migration completion

[Stefan Hajnoczi]

On Tue, Aug 15, 2017 at 12:04:53PM +0800, Fam Zheng wrote:
> @@ -147,6 +166,24 @@ static void blk_root_activate(BdrvChild *child, Error 
> **errp)
>  
>  blk->disable_perm = false;
>  
> +blk_set_perm(blk, blk->perm, BLK_PERM_ALL, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +blk->disable_perm = true;
> +return;
> +}
> +
> +if (runstate_check(RUN_STATE_INMIGRATE)) {
> +/* Activation can happen when migration process is still active, for
> + * example when nbd_server_add is called during non-shared storage
> + * migration. Defer the shared_perm update to migration completion. 
> */
> +if (!blk->vmsh) {
> +blk->vmsh = 
> qemu_add_vm_change_state_handler(blk_vm_state_changed,
> + blk);

Please add a qemu_del_vm_change_state_handler() call to cover the case
where the BB is deleted before the migration state changes.

This is necessary to prevent a memory leak and a crash when the change
state handler is invoked.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10 v2 0/2] trace: fix simpletrace.stp flight recorder mode

[Stefan Hajnoczi]

On Tue, Aug 15, 2017 at 09:44:28AM +0100, Stefan Hajnoczi wrote:
> v2:
>  * Don't emit event ID mapping records in simpletrace.stp [Daniel Berrange]
> 
> The SystemTap flight recorder mode no longer works with simpletrace.stp 
> because
> the event ID mapping records are emitted the first time an event fires.
> Chances are, the event ID mapping record will not be in ring buffer when the
> users wants to print the trace.
> 
> This series solves the issue by using the trace-events-all global event
> ordering for event IDs for simpletrace.stp.
> 
> Stefan Hajnoczi (2):
>   trace: use static event ID mapping in simpletrace.stp
>   simpletrace: fix flight recorder --no-header option
> 
>  scripts/simpletrace.py   | 24 +++--
>  scripts/tracetool/format/simpletrace_stap.py | 31 
> ++--
>  2 files changed, 20 insertions(+), 35 deletions(-)
> 
> -- 
> 2.13.4
> 
> 

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

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10 v2 0/2] trace: fix simpletrace.stp flight recorder mode

[Stefan Hajnoczi]

On Tue, Aug 15, 2017 at 01:59:50AM -0700, no-re...@patchew.org wrote:
> Checking PATCH 2/2: simpletrace: fix flight recorder --no-header option...
> ERROR: line over 90 characters
> #40: FILE: scripts/simpletrace.py:101:
> +"""Deserialize trace records from a file, yielding record tuples 
> (event_num, timestamp, pid, arg1, ..., arg6).

This line was not modified, so it wasn't introduced by this patch
series.  I think the reason it's a single line is because Python
docstrings must start with a single line.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v5 01/10] qemu.py: Pylint/style fixes

[Stefan Hajnoczi]

On Tue, Aug 15, 2017 at 10:57:23AM +0200, Lukáš Doktor wrote:
> No actual code changes, just several pylint/style fixes and docstring
> clarifications.
> 
> Signed-off-by: Lukáš Doktor 
> ---
>  scripts/qemu.py | 76 
> -
>  1 file changed, 53 insertions(+), 23 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10 v2 1/2] hw/acpi: Call acpi_set_pci_info when no ACPI tables needed

[Igor Mammedov]

On Tue, 15 Aug 2017 12:15:48 +0100
Anthony PERARD  wrote:

> To do PCI passthrough with Xen, the property acpi-pcihp-bsel needs to be
> set, but this was done only when ACPI tables are built which is not
> needed for a Xen guest. The need for the property starts with commit
> "pc: pcihp: avoid adding ACPI_PCIHP_PROP_BSEL twice"
> (f0c9d64a68b776374ec4732424a3e27753ce37b6).
> 
> Set pci info before checking for the needs to build ACPI tables.
> 
> Assign bsel=0 property only to the root bus on Xen as there is no
> support in the Xen ACPI tables for a different value.

looking at hw/acpi/pcihp.c and bsel usage there it looks like
bsel property is owned by it and not by ACPI tables, so instead of
shuffling it in acpi_setup(), how about moving bsel initialization
to hw/acpi/pcihp.c and initialize it there unconditionally?

It could be as simple as moving acpi_set_pci_info()/acpi_set_bsel()
there and calling it from acpi_pcihp_reset().

Then there won't be need for Xen specific branches, as root bus
will have bsel set automatically which is sufficient for Xen and
the rest of bsel-s (bridges) will be just unused by Xen,
which could later extend its ACPI table implementation to utilize them. 


> Reported-by: Sander Eikelenboom 
> Signed-off-by: Anthony PERARD 
> 
> ---
> Changes in V2:
>   - check for acpi_enabled before calling acpi_set_pci_info.
>   - set the property on the root bus only.
> 
> This patch would be a canditade to backport to 2.9.
> 
> CC: Stefano Stabellini 
> CC: Bruce Rogers 
> ---
>  hw/i386/acpi-build.c | 25 -
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 98dd424678..c0483b96cf 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -46,6 +46,7 @@
>  #include "sysemu/tpm_backend.h"
>  #include "hw/timer/mc146818rtc_regs.h"
>  #include "sysemu/numa.h"
> +#include "hw/xen/xen.h"
>  
>  /* Supported chipsets: */
>  #include "hw/acpi/piix4.h"
> @@ -518,8 +519,13 @@ static void acpi_set_pci_info(void)
>  unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
>  
>  if (bus) {
> -/* Scan all PCI buses. Set property to enable acpi based hotplug. */
> -pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, &bsel_alloc);
> +if (xen_enabled()) {
> +/* Assign BSEL property to root bus only. */
> +acpi_set_bsel(bus, &bsel_alloc);
> +} else {
> +/* Scan all PCI buses. Set property to enable acpi based 
> hotplug. */
> +pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL, 
> &bsel_alloc);
> +}
>  }
>  }
>  
> @@ -2871,6 +2877,14 @@ void acpi_setup(void)
>  AcpiBuildState *build_state;
>  Object *vmgenid_dev;
>  
> +if (!acpi_enabled) {
> +ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n");
> +return;
> +}
> +
> +/* Assign BSEL property on hotpluggable PCI buses. */
> +acpi_set_pci_info();
> +
>  if (!pcms->fw_cfg) {
>  ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
>  return;
> @@ -2881,15 +2895,8 @@ void acpi_setup(void)
>  return;
>  }
>  
> -if (!acpi_enabled) {
> -ACPI_BUILD_DPRINTF("ACPI disabled. Bailing out.\n");
> -return;
> -}
> -
>  build_state = g_malloc0(sizeof *build_state);
>  
> -acpi_set_pci_info();
> -
>  acpi_build_tables_init(&tables);
>  acpi_build(&tables, MACHINE(pcms));
>  

Re: [Qemu-devel] [PULL 0/5] Build and test patches

[Peter Maydell]

On 15 August 2017 at 11:57, Fam Zheng  wrote:
> The following changes since commit 83c3a1f61673ef554facf4d6d29ed56c5a219f9d:
>
>   xlnx-qspi: add a property for mmio-execution (2017-08-14 14:17:18 +0100)
>
> are available in the git repository at:
>
>   git://github.com/famz/qemu.git tags/build-and-test-pull-request
>
> for you to fetch changes up to a8132a2f288c260fb43243eb3c092b5186d84968:
>
>   docker: add centos7 image (2017-08-15 15:16:45 +0800)
>
> 
>
> One fix and a few docker test enhancements. Not 2.10 blocker, but it's
> good to have them.
>
> 
>
> Fam Zheng (1):
>   Makefile: Let "make check-help" work without running ./configure
>
> Philippe Mathieu-Daudé (4):
>   docker: use one package per line in CentOS config
>   docker: add Xen libs to centos6 image
>   docker: install more packages on CentOS to extend code coverage
>   docker: add centos7 image
>
>  Makefile|  2 --
>  tests/Makefile.include  | 46 
> ++---
>  tests/docker/dockerfiles/centos6.docker | 31 ++
>  tests/docker/dockerfiles/centos7.docker | 31 ++
>  4 files changed, 82 insertions(+), 28 deletions(-)
>  create mode 100644 tests/docker/dockerfiles/centos7.docker
Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH for-2.10 1/4] stubs: Add vm state change handler stubs

[Eric Blake]

On 08/14/2017 11:04 PM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 

A bit sparse on the 'why' - presumably, upcoming patches will fail to
compile if the stub is not present, but mentioning what dependency this
solves never hurts.

> ---
>  stubs/Makefile.objs  |  1 +
>  stubs/change-state-handler.c | 14 ++
>  2 files changed, 15 insertions(+)
>  create mode 100644 stubs/change-state-handler.c
> 

> +++ b/stubs/change-state-handler.c
> @@ -0,0 +1,14 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "sysemu/sysemu.h"
> +
> +VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler 
> *cb,
> + void *opaque)
> +{
> +return g_malloc0(1);
> +}

Hmm - this is NOT a VMChangeStateEntry; if it ever gets dereferenced,
the caller is (probably) accessing memory out of bounds.  Presumably,
since it is a stub, this should never be called - and if that's the
case, can we just get away with returning NULL instead (I'd rather have
the caller SEGFAULT than dereference out-of-bounds into the heap, if
this stub gets used inappropriately).

> +
> +void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
> +{
> +g_free(e);

And of course, if you don't allocate anything, this can be a no-op.

> +}
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 01/10] qemu.py: Pylint/style fixes

[Markus Armbruster]

Lukáš Doktor  writes:

> No actual code changes, just several pylint/style fixes and docstring
> clarifications.
>
> Signed-off-by: Lukáš Doktor 
> ---
>  scripts/qemu.py | 76 
> -
>  1 file changed, 53 insertions(+), 23 deletions(-)
>
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 880e3e8..466aaab 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -23,8 +23,22 @@ import qmp.qmp
>  class QEMUMachine(object):
>  '''A QEMU VM'''
>  
> -def __init__(self, binary, args=[], wrapper=[], name=None, 
> test_dir="/var/tmp",
> - monitor_address=None, socket_scm_helper=None, debug=False):
> +def __init__(self, binary, args=[], wrapper=[], name=None,
> + test_dir="/var/tmp", monitor_address=None,
> + socket_scm_helper=None, debug=False):
> +'''
> +Create a QEMUMachine object

Initialize a QEMUMachine

Rationale: it's __init__, not __create__, and "object" is redundant.

> +
> +@param binary: path to the qemu binary (str)

Drop (str), because what else could it be?

> +@param args: initial list of extra arguments

If this is the initial list, then what's the final list?

> +@param wrapper: list of arguments used as prefix to qemu binary
> +@param name: name of this object (used for log/monitor/... file 
> names)
prefix for socket and log file names (default: qemu-PID)

> +@param test_dir: base location to put log/monitor/... files in

where to create socket and log file

Aside: test_dir is a lousy name.

> +@param monitor_address: custom address for QMP monitor

Yes, but what *is* a custom address?  Its user _base_args() appears to
expect either a pair of strings (host, port) or a string filename.

> +@param socket_scm_helper: path to scm_helper binary (to forward fds)

What is an scm_helper, and why would I want to use it?

> +@param debug: enable debug mode (forwarded to QMP helper and such)

What is a QMP helper?  To what else is debug forwarded?

> +@note: Qemu process is not started until launch() is used.

until launch().

> +'''

It's an improvement.

>  if name is None:
>  name = "qemu-%d" % os.getpid()
>  if monitor_address is None:
> @@ -33,12 +47,13 @@ class QEMUMachine(object):
>  self._qemu_log_path = os.path.join(test_dir, name + ".log")
>  self._popen = None
>  self._binary = binary
> -self._args = list(args) # Force copy args in case we modify them
> +self._args = list(args) # Force copy args in case we modify them
>  self._wrapper = wrapper
>  self._events = []
>  self._iolog = None
>  self._socket_scm_helper = socket_scm_helper
>  self._debug = debug
> +self._qmp = None
>  
>  # This can be used to add an unused monitor instance.
>  def add_monitor_telnet(self, ip, port):
> @@ -64,16 +79,16 @@ class QEMUMachine(object):
>  if self._socket_scm_helper is None:
>  print >>sys.stderr, "No path to socket_scm_helper set"
>  return -1
> -if os.path.exists(self._socket_scm_helper) == False:
> +if not os.path.exists(self._socket_scm_helper):
>  print >>sys.stderr, "%s does not exist" % self._socket_scm_helper
>  return -1
>  fd_param = ["%s" % self._socket_scm_helper,
>  "%d" % self._qmp.get_sock_fd(),
>  "%s" % fd_file_path]
>  devnull = open('/dev/null', 'rb')
> -p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
> - stderr=sys.stderr)
> -return p.wait()
> +proc = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
> +stderr=sys.stderr)
> +return proc.wait()
>  
>  @staticmethod
>  def _remove_if_exists(path):
> @@ -99,8 +114,8 @@ class QEMUMachine(object):
>  return self._popen.pid
>  
>  def _load_io_log(self):
> -with open(self._qemu_log_path, "r") as fh:
> -self._iolog = fh.read()
> +with open(self._qemu_log_path, "r") as iolog:
> +self._iolog = iolog.read()
>  
>  def _base_args(self):
>  if isinstance(self._monitor_address, tuple):
> @@ -114,8 +129,8 @@ class QEMUMachine(object):
>  '-display', 'none', '-vga', 'none']
>  
>  def _pre_launch(self):
> -self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, 
> server=True,
> -debug=self._debug)
> +self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
> +server=True, 
> debug=self._debug)

Recommend to break the line between the last two arguments.

>  
>  def _post_launch(self):
>  self._qmp.accept()
> @@ -131,9 +146,11 @@ class QEMUMac

Re: [Qemu-devel] [PATCH v4 01/12] ui: add keycodemapdb repository as a GIT submodule

[Fam Zheng]

On Tue, 08/15 11:53, Daniel P. Berrange wrote:
> On Tue, Aug 15, 2017 at 06:47:22PM +0800, Fam Zheng wrote:
> > On Tue, 08/15 11:04, Daniel P. Berrange wrote:
> > > On Tue, Aug 15, 2017 at 10:36:04AM +0100, Daniel P. Berrange wrote:
> > > > The https://gitlab.com/keycodemap/keycodemapdb/ repo contains a
> > > > data file mapping between all the different scancode/keycode/keysym
> > > > sets that are known, and a tool to auto-generate lookup tables for
> > > > different combinations.
> > > > 
> > > > It is used by GTK-VNC, SPICE-GTK and libvirt for mapping keys.
> > > > Using it in QEMU will let us replace many hand written lookup
> > > > tables with auto-generated tables from a master data source,
> > > > reducing bugs. Adding new QKeyCodes will now only require the
> > > > master table to be updated, all ~20 other tables will be
> > > > automatically updated to follow.
> > > > 
> > > > Signed-off-by: Daniel P. Berrange 
> > > > ---
> > > >  .gitignore|  2 ++
> > > >  .gitmodules   |  3 +++
> > > >  configure |  2 ++
> > > >  tests/docker/Makefile.include | 11 +++
> > > >  tests/docker/run  |  4 +++-
> > > >  ui/Makefile.objs  | 18 ++
> > > >  ui/keycodemapdb   |  1 +
> > > >  7 files changed, 36 insertions(+), 5 deletions(-)
> > > >  create mode 16 ui/keycodemapdb
> > > > 
> > > 
> > > > diff --git a/configure b/configure
> > > > index dd73cce62f..bd373ec2b4 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -5258,6 +5258,8 @@ echo_version() {
> > > >  fi
> > > >  }
> > > >  
> > > > +(cd $source_path && git submodule update --init ui/keycodemapdb)
> > > > +
> > > 
> > > Urgh, no, this won't work because of course you don't have to
> > > have a git checkout when running configure.
> > > 
> > > Any suggestions on the "best" way to ensure that the ui/keycodemapdb
> > > git submodule is always checked out, without requiring developers to
> > > do something manually ?
> > > 
> > > I could try
> > > 
> > >   (cd $source_path && test -d .git && git submodule update --init 
> > > ui/keycodemapdb)
> > > 
> > > but wonder if there's a better way ?
> > 
> > I think the way dtc handles this is okay: probe headers/libs, if failed, 
> > hint
> > the "git submodule update" command in the error message.  This is also a
> > familiar/friendly user interface to developers.
> 
> I don't think that's acceptable in this case. Few people will ever need
> to setup the dtc submodule as distros commonly have that available.

It's not available in the case of RHEL/CentOS 7, so it is similar in a degree
which is why I mentioned it.

But certainly I'll be happy to see a way to have this automated.  FWIW I've no
objection to the "test -d .git" idea.

For docker tests to work, I'm inclined to change the Makefile rules so that
every initialized submodule under $SRC_PATH is git-archived and copied in.

Fam

Re: [Qemu-devel] [PATCH for-2.10 1/4] stubs: Add vm state change handler stubs

[Fam Zheng]

On Tue, 08/15 07:26, Eric Blake wrote:
> On 08/14/2017 11:04 PM, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng 
> 
> A bit sparse on the 'why' - presumably, upcoming patches will fail to
> compile if the stub is not present, but mentioning what dependency this
> solves never hurts.
> 
> > ---
> >  stubs/Makefile.objs  |  1 +
> >  stubs/change-state-handler.c | 14 ++
> >  2 files changed, 15 insertions(+)
> >  create mode 100644 stubs/change-state-handler.c
> > 
> 
> > +++ b/stubs/change-state-handler.c
> > @@ -0,0 +1,14 @@
> > +#include "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "sysemu/sysemu.h"
> > +
> > +VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler 
> > *cb,
> > + void *opaque)
> > +{
> > +return g_malloc0(1);
> > +}
> 
> Hmm - this is NOT a VMChangeStateEntry; if it ever gets dereferenced,
> the caller is (probably) accessing memory out of bounds.  Presumably,
> since it is a stub, this should never be called - and if that's the
> case, can we just get away with returning NULL instead (I'd rather have
> the caller SEGFAULT than dereference out-of-bounds into the heap, if
> this stub gets used inappropriately).

Good point, will update this patch.

> 
> > +
> > +void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
> > +{
> > +g_free(e);
> 
> And of course, if you don't allocate anything, this can be a no-op.
> 
> > +}
> > 
> 

Fam

[Qemu-devel] [PATCH for-2.10] qemu-iotests: step clock after each test iteration

[Stefan Hajnoczi]

The 093 throttling test submits twice as many requests as the throttle
limit in order to ensure that we reach the limit.  The remaining
requests are left in-flight at the end of each test iteration.

Commit 452589b6b47e8dc6353df257fc803dfc1383bed8 ("vl.c/exit: pause cpus
before closing block devices") exposed a hang in 093.  This happens
because requests are still in flight when QEMU terminates but
QEMU_CLOCK_VIRTUAL time is frozen.  bdrv_drain_all() hangs forever since
throttled requests cannot complete.

Step the clock at the end of each test iteration so in-flight requests
actually finish.  This solves the hang and is cleaner than leaving tests
in-flight.

Note that this could also be "fixed" by disabling throttling when drives
are closed in QEMU.  That approach has two issues:

1. We must drain requests before disabling throttling, so the hang
   cannot be easily avoided!

2. Any time QEMU disables throttling internally there is a chance that
   malicious users can abuse the code path to bypass throttling limits.

Therefore it makes more sense to fix the test case than to modify QEMU.

Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/093 | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 2ed393a548..ef3997206b 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -133,6 +133,10 @@ class ThrottleTestCase(iotests.QMPTestCase):
 self.assertTrue(check_limit(params['iops_rd'], rd_iops))
 self.assertTrue(check_limit(params['iops_wr'], wr_iops))
 
+# Allow remaining requests to finish.  We submitted twice as many to
+# ensure the throttle limit is reached.
+self.vm.qtest("clock_step %d" % ns)
+
 # Connect N drives to a VM and test I/O in all of them
 def test_all(self):
 params = {"bps": 4096,
-- 
2.13.4

[Qemu-devel] [PATCH for-2.10 v2 0/4] block: Fix non-shared storage migration

[Fam Zheng]

v2: Don't leak blk->vmsh when BB is deleted before the callback is called.
[Stefan]
From stub functions, don't return g_malloc0(1) which is risky, return NULL.
[Eric]

"nbd-server-add -w" doesn't work when we are in "-incoming defer" state:

(qemu) nbd_server_add -w drive-virtio-disk0
Block node is read-only

Two problems are faced:

  - nbd_export_new() calls bdrv_invalidate_cache() too late.
  - bdrv_invalidate_cache() restores qdev permission (which are temporarily
masked by BlockBackend.disable_perm during INMIGRATE) too early.

Fix both, and add a regression iotest.

Fam Zheng (3):
  stubs: Add vm state change handler stubs
  block-backend: Defer shared_perm tightening migration completion
  iotests: Add non-shared storage migration case 192

Kevin Wolf (1):
  nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cache

 block/block-backend.c| 41 
 nbd/server.c | 20 +++---
 stubs/Makefile.objs  |  1 +
 stubs/change-state-handler.c | 14 ++
 tests/qemu-iotests/192   | 63 
 tests/qemu-iotests/192.out   |  7 +
 tests/qemu-iotests/group |  1 +
 7 files changed, 138 insertions(+), 9 deletions(-)
 create mode 100644 stubs/change-state-handler.c
 create mode 100755 tests/qemu-iotests/192
 create mode 100644 tests/qemu-iotests/192.out

-- 
2.13.4

[Qemu-devel] [PATCH for-2.10 v2 1/4] stubs: Add vm state change handler stubs

[Fam Zheng]

They will be used by BlockBackend code in block-obj-y, which doesn't
always get linked with common-obj-y. Add stubs to keep ld happy.

Signed-off-by: Fam Zheng 
---
 stubs/Makefile.objs  |  1 +
 stubs/change-state-handler.c | 14 ++
 2 files changed, 15 insertions(+)
 create mode 100644 stubs/change-state-handler.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index f5b47bfd74..e69c217aff 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -19,6 +19,7 @@ stub-obj-y += is-daemonized.o
 stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 stub-obj-y += machine-init-done.o
 stub-obj-y += migr-blocker.o
+stub-obj-y += change-state-handler.o
 stub-obj-y += monitor.o
 stub-obj-y += notify-event.o
 stub-obj-y += qtest.o
diff --git a/stubs/change-state-handler.c b/stubs/change-state-handler.c
new file mode 100644
index 00..01b1c6986d
--- /dev/null
+++ b/stubs/change-state-handler.c
@@ -0,0 +1,14 @@
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+
+VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
+ void *opaque)
+{
+return NULL;
+}
+
+void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
+{
+/* Nothing to do. */
+}
-- 
2.13.4

[Qemu-devel] [PATCH for-2.10 v2 3/4] block-backend: Defer shared_perm tightening migration completion

[Fam Zheng]

As in the case of nbd_export_new(), bdrv_invalidate_cache() can be
called when migration is still in progress. In this case we are not
ready to tighten the shared permissions fenced by blk->disable_perm.

Defer to a VM state change handler.

Signed-off-by: Fam Zheng 
---
 block/block-backend.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 968438c149..e9798e897d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -20,6 +20,7 @@
 #include "qapi-event.h"
 #include "qemu/id.h"
 #include "trace.h"
+#include "migration/misc.h"
 
 /* Number of coroutines to reserve per attached device model */
 #define COROUTINE_POOL_RESERVATION 64
@@ -68,6 +69,7 @@ struct BlockBackend {
 NotifierList remove_bs_notifiers, insert_bs_notifiers;
 
 int quiesce_counter;
+VMChangeStateEntry *vmsh;
 };
 
 typedef struct BlockBackendAIOCB {
@@ -129,6 +131,23 @@ static const char *blk_root_get_name(BdrvChild *child)
 return blk_name(child->opaque);
 }
 
+static void blk_vm_state_changed(void *opaque, int running, RunState state)
+{
+Error *local_err = NULL;
+BlockBackend *blk = opaque;
+
+if (state == RUN_STATE_INMIGRATE) {
+return;
+}
+
+qemu_del_vm_change_state_handler(blk->vmsh);
+blk->vmsh = NULL;
+blk_set_perm(blk, blk->perm, blk->shared_perm, &local_err);
+if (local_err) {
+error_report_err(local_err);
+}
+}
+
 /*
  * Notifies the user of the BlockBackend that migration has completed. qdev
  * devices can tighten their permissions in response (specifically revoke
@@ -147,6 +166,24 @@ static void blk_root_activate(BdrvChild *child, Error 
**errp)
 
 blk->disable_perm = false;
 
+blk_set_perm(blk, blk->perm, BLK_PERM_ALL, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+blk->disable_perm = true;
+return;
+}
+
+if (runstate_check(RUN_STATE_INMIGRATE)) {
+/* Activation can happen when migration process is still active, for
+ * example when nbd_server_add is called during non-shared storage
+ * migration. Defer the shared_perm update to migration completion. */
+if (!blk->vmsh) {
+blk->vmsh = qemu_add_vm_change_state_handler(blk_vm_state_changed,
+ blk);
+}
+return;
+}
+
 blk_set_perm(blk, blk->perm, blk->shared_perm, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
@@ -291,6 +328,10 @@ static void blk_delete(BlockBackend *blk)
 if (blk->root) {
 blk_remove_bs(blk);
 }
+if (blk->vmsh) {
+qemu_del_vm_change_state_handler(blk->vmsh);
+blk->vmsh = NULL;
+}
 assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers));
 assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers));
 QTAILQ_REMOVE(&block_backends, blk, link);
-- 
2.13.4

[Qemu-devel] [PATCH for-2.10 v2 4/4] iotests: Add non-shared storage migration case 192

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/192 | 63 ++
 tests/qemu-iotests/192.out |  7 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 71 insertions(+)
 create mode 100755 tests/qemu-iotests/192
 create mode 100644 tests/qemu-iotests/192.out

diff --git a/tests/qemu-iotests/192 b/tests/qemu-iotests/192
new file mode 100755
index 00..b50a2c0c8e
--- /dev/null
+++ b/tests/qemu-iotests/192
@@ -0,0 +1,63 @@
+#!/bin/bash
+#
+# Test NBD export with -incoming (non-shared storage migration use case from
+# libvirt)
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=f...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
+_notrun "Requires a PC machine"
+fi
+
+size=64M
+_make_test_img $size
+
+{
+echo "nbd_server_start unix:$TEST_DIR/nbd"
+echo "nbd_server_add -w drive0"
+echo "q"
+} | $QEMU -nodefaults -display none -monitor stdio \
+-drive format=$IMGFMT,file=$TEST_IMG,if=ide,id=drive0 \
+-incoming defer 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/192.out b/tests/qemu-iotests/192.out
new file mode 100644
index 00..1e0be4c4d7
--- /dev/null
+++ b/tests/qemu-iotests/192.out
@@ -0,0 +1,7 @@
+QA output created by 192
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) nbd_server_start unix:TEST_DIR/nbd
+(qemu) nbd_server_add -w drive0
+(qemu) q
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1848077932..afbdc427ea 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -186,3 +186,4 @@
 188 rw auto quick
 189 rw auto
 190 rw auto quick
+192 rw auto quick
-- 
2.13.4

[Qemu-devel] [PATCH for-2.10 v2 2/4] nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cache

[Fam Zheng]

From: Kevin Wolf 

The "inactive" state of BDS affects whether the permissions can be
granted, we must call bdrv_invalidate_cache before bdrv_set_perm to
support "-incoming defer" case.

Reported-by: Christian Ehrhardt 
Signed-off-by: Fam Zheng 
---
 nbd/server.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 82a78bf439..b68b6fb535 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1045,11 +1045,22 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
dev_offset, off_t size,
   bool writethrough, BlockBackend *on_eject_blk,
   Error **errp)
 {
+AioContext  *ctx;
 BlockBackend *blk;
 NBDExport *exp = g_malloc0(sizeof(NBDExport));
 uint64_t perm;
 int ret;
 
+/*
+ * NBD exports are used for non-shared storage migration.  Make sure
+ * that BDRV_O_INACTIVE is cleared and the image is ready for write
+ * access since the export could be available before migration handover.
+ */
+ctx = bdrv_get_aio_context(bs);
+aio_context_acquire(ctx);
+bdrv_invalidate_cache(bs, NULL);
+aio_context_release(ctx);
+
 /* Don't allow resize while the NBD server is running, otherwise we don't
  * care what happens with the node. */
 perm = BLK_PERM_CONSISTENT_READ;
@@ -1087,15 +1098,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
dev_offset, off_t size,
 exp->eject_notifier.notify = nbd_eject_notifier;
 blk_add_remove_bs_notifier(on_eject_blk, &exp->eject_notifier);
 }
-
-/*
- * NBD exports are used for non-shared storage migration.  Make sure
- * that BDRV_O_INACTIVE is cleared and the image is ready for write
- * access since the export could be available before migration handover.
- */
-aio_context_acquire(exp->ctx);
-blk_invalidate_cache(blk, NULL);
-aio_context_release(exp->ctx);
 return exp;
 
 fail:
-- 
2.13.4

Re: [Qemu-devel] [PATCH v2 1/6] block: skip implicit nodes in snapshots, blockjobs

[Alberto Garcia]

On Wed 09 Aug 2017 04:02:51 PM CEST, Manos Pitsidianakis wrote:
> @@ -2988,6 +3004,9 @@ void qmp_block_stream(bool has_job_id, const char 
> *job_id, const char *device,
>  return;
>  }
>  
> +/* Skip implicit filter nodes */
> +bs = bdrv_get_first_explicit(bs);
> +
>  aio_context = bdrv_get_aio_context(bs);
>  aio_context_acquire(aio_context);

This change works here because right now the only implicit node that we
could have in practice is the throttle node, but I wonder if this is
good enough for any kind of implicit node in general.

> +static inline BlockDriverState *child_bs(BlockDriverState *bs)
> +{
> +BdrvChild *child = QLIST_FIRST(&bs->children);
> +assert(child && !QLIST_NEXT(child, next));
> +return child->bs;
> +}

This aborts if the bs has a number of children != 1. That's not
something that I would expect from a function named like that.

Considering that you're only using it in bdrv_get_first_explicit(), why
don't you simply move the code there?

The other question is of course whether we can rely for the future on
the assumption that implicit nodes only have one children.

Berto

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

[Stefan Hajnoczi]

The following changes since commit 5681da292242550f37ba4c03f46a8a6f8ee9278a:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170815' into 
staging (2017-08-15 09:39:14 +0100)

are available in the git repository at:

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

for you to fetch changes up to 840d8351774664d01c328d31ed33b0e2d85c866e:

  simpletrace: fix flight recorder --no-header option (2017-08-15 12:50:29 
+0100)





Stefan Hajnoczi (2):
  trace: use static event ID mapping in simpletrace.stp
  simpletrace: fix flight recorder --no-header option

 scripts/simpletrace.py   | 24 +++--
 scripts/tracetool/format/simpletrace_stap.py | 31 ++--
 2 files changed, 20 insertions(+), 35 deletions(-)

-- 
2.13.4

[Qemu-devel] [PULL 1/2] trace: use static event ID mapping in simpletrace.stp

[Stefan Hajnoczi]

This is a partial revert of commit
7f1b588f20d027730676e627713ae3bbf6baab04 ("trace: emit name <-> ID
mapping in simpletrace header"), which broke the SystemTap flight
recorder because event mapping records may not be present in the ring
buffer when the trace is analyzed.  This means simpletrace.py
--no-header does not know the event ID mapping needed to pretty-print
the trace.

Instead of numbering events dynamically, use a static event ID mapping
as dictated by the event order in the trace-events-all file.

The simpletrace.py script also uses trace-events-all so the next patch
will fix the simpletrace.py --no-header option to take advantage of this
knowledge.

Cc: Daniel P. Berrange 
Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Daniel P. Berrange 
Message-id: 20170815084430.7128-2-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 scripts/tracetool/format/simpletrace_stap.py | 31 ++--
 1 file changed, 2 insertions(+), 29 deletions(-)

diff --git a/scripts/tracetool/format/simpletrace_stap.py 
b/scripts/tracetool/format/simpletrace_stap.py
index 144b704bcd..e7e44842ca 100644
--- a/scripts/tracetool/format/simpletrace_stap.py
+++ b/scripts/tracetool/format/simpletrace_stap.py
@@ -22,33 +22,8 @@ def global_var_name(name):
 return probeprefix().replace(".", "_") + "_" + name
 
 def generate(events, backend, group):
-id_map = global_var_name("event_name_to_id_map")
-next_id = global_var_name("event_next_id")
-map_func = global_var_name("simple_trace_map_event")
 out('/* This file is autogenerated by tracetool, do not edit. */',
-'',
-'global %(id_map)s',
-'global %(next_id)s',
-'function %(map_func)s(name)',
-'',
-'{',
-'if (!([name] in %(id_map)s)) {',
-'%(id_map)s[name] = %(next_id)s',
-'name_len = strlen(name)',
-'printf("%%8b%%8b%%4b%%.*s", 0, ',
-'   %(next_id)s, name_len, name_len, name)',
-'%(next_id)s = %(next_id)s + 1',
-'}',
-'return %(id_map)s[name]',
-'}',
-'probe begin',
-'{',
-'printf("%%8b%%8b%%8b", 0x, 0xf2b177cb0aa429b4, 
4)',
-'}',
-'',
-id_map=id_map,
-next_id=next_id,
-map_func=map_func)
+'')
 
 for event_id, e in enumerate(events):
 if 'disable' in e.properties:
@@ -56,9 +31,7 @@ def generate(events, backend, group):
 
 out('probe %(probeprefix)s.simpletrace.%(name)s = 
%(probeprefix)s.%(name)s ?',
 '{',
-'id = %(map_func)s("%(name)s")',
 probeprefix=probeprefix(),
-map_func=map_func,
 name=e.name)
 
 # Calculate record size
@@ -77,7 +50,7 @@ def generate(events, backend, group):
 sizestr = ' + '.join(sizes)
 
 # Generate format string and value pairs for record header and 
arguments
-fields = [('8b', 'id'),
+fields = [('8b', str(event_id)),
   ('8b', 'gettimeofday_ns()'),
   ('4b', sizestr),
   ('4b', 'pid()')]
-- 
2.13.4

[Qemu-devel] [PULL 2/2] simpletrace: fix flight recorder --no-header option

[Stefan Hajnoczi]

The simpletrace.py script can pretty-print flight recorder ring buffers.
These are not full simpletrace binary trace files but just the end of a
trace file.  There is no header and the event ID mapping information is
often unavailable since the ring buffer may have filled up and discarded
event ID mapping records.

The simpletrace.stp script that generates ring buffer traces uses the
same trace-events-all input file as simpletrace.py.  Therefore both
scripts have the same global ordering of trace events.  A dynamic event
ID mapping isn't necessary: just use the trace-events-all file as the
reference for how event IDs are numbered.

It is now possible to analyze simpletrace.stp ring buffers again using:

  $ ./simpletrace.py trace-events-all path/to/ring-buffer

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Daniel P. Berrange 
Message-id: 20170815084430.7128-3-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 scripts/simpletrace.py | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
index 2a977e2ab9..a3a6315055 100755
--- a/scripts/simpletrace.py
+++ b/scripts/simpletrace.py
@@ -97,11 +97,17 @@ def read_trace_header(fobj):
 raise ValueError('Log format %d not supported with this QEMU release!'
  % log_version)
 
-def read_trace_records(edict, fobj):
-"""Deserialize trace records from a file, yielding record tuples 
(event_num, timestamp, pid, arg1, ..., arg6)."""
-idtoname = {
-dropped_event_id: "dropped"
-}
+def read_trace_records(edict, idtoname, fobj):
+"""Deserialize trace records from a file, yielding record tuples 
(event_num, timestamp, pid, arg1, ..., arg6).
+
+Note that `idtoname` is modified if the file contains mapping records.
+
+Args:
+edict (str -> Event): events dict, indexed by name
+idtoname (int -> str): event names dict, indexed by event ID
+fobj (file): input file
+
+"""
 while True:
 t = fobj.read(8)
 if len(t) == 0:
@@ -171,10 +177,16 @@ def process(events, log, analyzer, read_header=True):
 
 dropped_event = Event.build("Dropped_Event(uint64_t num_events_dropped)")
 edict = {"dropped": dropped_event}
+idtoname = {dropped_event_id: "dropped"}
 
 for event in events:
 edict[event.name] = event
 
+# If there is no header assume event ID mapping matches events list
+if not read_header:
+for event_id, event in enumerate(events):
+idtoname[event_id] = event.name
+
 def build_fn(analyzer, event):
 if isinstance(event, str):
 return analyzer.catchall
@@ -197,7 +209,7 @@ def process(events, log, analyzer, read_header=True):
 
 analyzer.begin()
 fn_cache = {}
-for rec in read_trace_records(edict, log):
+for rec in read_trace_records(edict, idtoname, log):
 event_num = rec[0]
 event = edict[event_num]
 if event_num not in fn_cache:
-- 
2.13.4

Re: [Qemu-devel] [PATCH for 2.10?] qxl: call qemu_spice_display_init_common for secondary devices

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 01:15:52AM +0200, Paolo Bonzini wrote:
> Fixes this 2.10 regression:
> 
>   $ qemu-system-x86_64  -cpu host -m 6144 -vga qxl -device qxl
>   qemu-system-x86_64: util/qemu-thread-posix.c:64: qemu_mutex_lock: Assertion 
> `mutex->initialized' failed.
> 
> Reported-by: adema...@redhat.com
> Cc: kra...@redhat.com
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/display/qxl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 7f8c73b56d..ae3677fd1e 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -2054,6 +2054,7 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error 
> **errp)
>  uint32_t pci_device_rev;
>  uint32_t io_size;
>  
> +qemu_spice_display_init_common(&qxl->ssd);
>  qxl->mode = QXL_MODE_UNDEFINED;
>  qxl->generation = 1;
>  qxl->num_memslots = NUM_MEMSLOTS;
> @@ -2176,7 +2177,6 @@ static void qxl_realize_primary(PCIDevice *dev, Error 
> **errp)
>  portio_list_add(&qxl->vga_port_list, pci_address_space_io(dev), 0x3b0);
>  
>  vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
> -qemu_spice_display_init_common(&qxl->ssd);
>  
>  qxl_realize_common(qxl, &local_err);
>  if (local_err) {

IIUC, we hit the abort because we have 2 QXL devices, and only
qxl_realize_primary() is calling  qemu_spice_display_init_common().
By moving the qemu_spice_display_init_common() call upto into
the qxl_realize_common() method, it also gets triggered by
qxl_realize_secondary(). It makes that we need to initialize the
'ssd' field for both primary and secondary displays

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for 2.10?] qxl: call qemu_spice_display_init_common for secondary devices

[Stefan Hajnoczi]

On Tue, Aug 15, 2017 at 01:15:52AM +0200, Paolo Bonzini wrote:
> Fixes this 2.10 regression:
> 
>   $ qemu-system-x86_64  -cpu host -m 6144 -vga qxl -device qxl
>   qemu-system-x86_64: util/qemu-thread-posix.c:64: qemu_mutex_lock: Assertion 
> `mutex->initialized' failed.
> 
> Reported-by: adema...@redhat.com
> Cc: kra...@redhat.com
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/display/qxl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I have audited the code and cannot see any bad side-effects.  Looks good
to me!

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10] qemu-iotests: step clock after each test iteration

[Eric Blake]

On 08/15/2017 08:05 AM, Stefan Hajnoczi wrote:
> The 093 throttling test submits twice as many requests as the throttle
> limit in order to ensure that we reach the limit.  The remaining
> requests are left in-flight at the end of each test iteration.
> 
> Commit 452589b6b47e8dc6353df257fc803dfc1383bed8 ("vl.c/exit: pause cpus
> before closing block devices") exposed a hang in 093.  This happens
> because requests are still in flight when QEMU terminates but
> QEMU_CLOCK_VIRTUAL time is frozen.  bdrv_drain_all() hangs forever since
> throttled requests cannot complete.
> 
> Step the clock at the end of each test iteration so in-flight requests
> actually finish.  This solves the hang and is cleaner than leaving tests
> in-flight.
> 
> Note that this could also be "fixed" by disabling throttling when drives
> are closed in QEMU.  That approach has two issues:
> 
> 1. We must drain requests before disabling throttling, so the hang
>cannot be easily avoided!
> 
> 2. Any time QEMU disables throttling internally there is a chance that
>malicious users can abuse the code path to bypass throttling limits.
> 
> Therefore it makes more sense to fix the test case than to modify QEMU.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/qemu-iotests/093 | 4 
>  1 file changed, 4 insertions(+)
> 

Reviewed-by: Eric Blake 

I can take this through the NBD tree (since that's one environment that
trips up on the test), if Peter doesn't apply it directly.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 0/2] Tracing patches

[no-reply]

Hi,

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

Message-id: 20170815132746.12540-1-stefa...@redhat.com
Subject: [Qemu-devel] [PULL 0/2] Tracing patches
Type: series

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

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

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
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20170814231552.24593-1-pbonz...@redhat.com 
-> patchew/20170814231552.24593-1-pbonz...@redhat.com
 t [tag update]patchew/20170815130502.8736-1-stefa...@redhat.com -> 
patchew/20170815130502.8736-1-stefa...@redhat.com
Switched to a new branch 'test'
c8a963888c simpletrace: fix flight recorder --no-header option
272f609607 trace: use static event ID mapping in simpletrace.stp

=== OUTPUT BEGIN ===
Checking PATCH 1/2: trace: use static event ID mapping in simpletrace.stp...
Checking PATCH 2/2: simpletrace: fix flight recorder --no-header option...
ERROR: line over 90 characters
#42: FILE: scripts/simpletrace.py:101:
+"""Deserialize trace records from a file, yielding record tuples 
(event_num, timestamp, pid, arg1, ..., arg6).

total: 1 errors, 0 warnings, 46 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 v3 00/12] Convert over to use keycodemapdb

[Programmingkid]

> On Aug 15, 2017, at 4:38 AM, Daniel P. Berrange  wrote:
> 
> On Mon, Aug 14, 2017 at 02:46:22PM -0400, Programmingkid wrote:
>> Sorry but there appears to be an issue with your patchset. I ran this 
>> command:
>> 
>> ./configure --target-list=ppc-softmmu,i386-softmmu
>> 
>> Then saw this error message: 
>> 
>> error: pathspec 'ui/keycodemapdb' did not match any file(s) known to git.
>> 
>> Running 'make' I saw this error message:
>> 
>> make: *** No rule to make target `ui/input-keymap-atset1-to-qcode.c', needed 
>> by `Makefile'.  Stop.
>> make: *** Waiting for unfinished jobs
>> 
>> Running the 'make clean' command produces this error message:
>> 
>> make[1]: *** No rule to make target `ui/input-keymap-atset1-to-qcode.c', 
>> needed by `Makefile'.  Stop.
>> make: *** [clean] Error 1
>> 
>> Perhaps a missing patch?
> 
> No, its a missing "git submodule update --init ui/keycodemapdb" call

Thank you for replying. I tried this command and saw this error message:

error: pathspec 'ui/keycodemapdb' did not match any file(s) known to git.

Note: I'm using version 3. Will test version 4 soon. 

I suggest adding the git submodule update info to your cover letter saying it 
is required. 

Re: [Qemu-devel] [PATCH v3 00/12] Convert over to use keycodemapdb

[Daniel P. Berrange]

On Tue, Aug 15, 2017 at 09:49:00AM -0400, Programmingkid wrote:
> 
> > On Aug 15, 2017, at 4:38 AM, Daniel P. Berrange  wrote:
> > 
> > On Mon, Aug 14, 2017 at 02:46:22PM -0400, Programmingkid wrote:
> >> Sorry but there appears to be an issue with your patchset. I ran this 
> >> command:
> >> 
> >> ./configure --target-list=ppc-softmmu,i386-softmmu
> >> 
> >> Then saw this error message: 
> >> 
> >> error: pathspec 'ui/keycodemapdb' did not match any file(s) known to git.
> >> 
> >> Running 'make' I saw this error message:
> >> 
> >> make: *** No rule to make target `ui/input-keymap-atset1-to-qcode.c', 
> >> needed by `Makefile'.  Stop.
> >> make: *** Waiting for unfinished jobs
> >> 
> >> Running the 'make clean' command produces this error message:
> >> 
> >> make[1]: *** No rule to make target `ui/input-keymap-atset1-to-qcode.c', 
> >> needed by `Makefile'.  Stop.
> >> make: *** [clean] Error 1
> >> 
> >> Perhaps a missing patch?
> > 
> > No, its a missing "git submodule update --init ui/keycodemapdb" call
> 
> Thank you for replying. I tried this command and saw this error message:
> 
> error: pathspec 'ui/keycodemapdb' did not match any file(s) known to git.
> 
> Note: I'm using version 3. Will test version 4 soon. 
> 
> I suggest adding the git submodule update info to your cover letter
> saying it is required.

It isn't supposed to be required - its a bug, hence why the tests are
failing.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 1/6] block: skip implicit nodes in snapshots, blockjobs

[Manos Pitsidianakis]

On Tue, Aug 15, 2017 at 03:24:38PM +0200, Alberto Garcia wrote:

On Wed 09 Aug 2017 04:02:51 PM CEST, Manos Pitsidianakis wrote:

@@ -2988,6 +3004,9 @@ void qmp_block_stream(bool has_job_id, const char 
*job_id, const char *device,
 return;
 }

+/* Skip implicit filter nodes */
+bs = bdrv_get_first_explicit(bs);
+
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);


This change works here because right now the only implicit node that we
could have in practice is the throttle node, but I wonder if this is
good enough for any kind of implicit node in general.


It does feel a bit sloppy but block jobs should work the same with
implicit nodes and without, so all we can do is ignore them.




+static inline BlockDriverState *child_bs(BlockDriverState *bs)
+{
+BdrvChild *child = QLIST_FIRST(&bs->children);
+assert(child && !QLIST_NEXT(child, next));
+return child->bs;
+}


This aborts if the bs has a number of children != 1. That's not
something that I would expect from a function named like that.

Considering that you're only using it in bdrv_get_first_explicit(), why
don't you simply move the code there?


It felt useful to have a function that also returns file->bs (we have 
backing_bs() already) instead of doing backing_bs(bs) ? : file_bs(bs)


The other question is of course whether we can rely for the future on
the assumption that implicit nodes only have one children.


This is only to get either bs->backing or bs->file (we can't have both
anyway). I wanted to document it with something like "Used for filter 
drivers with only one child" which fits with what implicit nodes we have 
so far (mirror, commit, throttle and in the future backup)


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.10 v2 1/4] stubs: Add vm state change handler stubs

[Eric Blake]

On 08/15/2017 08:07 AM, Fam Zheng wrote:
> They will be used by BlockBackend code in block-obj-y, which doesn't
> always get linked with common-obj-y. Add stubs to keep ld happy.
> 
> Signed-off-by: Fam Zheng 
> ---
>  stubs/Makefile.objs  |  1 +
>  stubs/change-state-handler.c | 14 ++
>  2 files changed, 15 insertions(+)
>  create mode 100644 stubs/change-state-handler.c

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 for 2.10] block/nbd-client: always return EIO on and after the first io channel error

[Eric Blake]

On 08/15/2017 02:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> The only doubt: is it possible to hang on nbd_rwv because some fail in
>>> connection or server?
>> We _already_ have the bug that we are hanging in trying to talk to a
>> broken server, which is a regression introduced in 2.9 and not present
>> in 2.8.  And that's what I'm most worried about getting fixed before
>> 2.10 is released.
>>
>> I don't think that sending any more data to the server is necessarily
>> going to cause a hang, so much as trying to read data that is going to
>> be sent in reply or failing to manipulate the coroutine handlers
>> correctly (that is, our current hang is because even after we detect
>> failure, we are still sending NBD_CMD_FLUSH but no longer have a
>> coroutine in place to read the reply, so we no longer make progress to
>> the point of sending NBD_CMD_DISC and closing the connection).
> 
> but we will not try to read.
> However, I'm convinced, ok, let's send nothing to the broken server.

Can you offer a review on my proposed patch? I'm hoping to send an NBD
pull request in the next couple of hours, to make -rc3.
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02593.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 0/2] Tracing patches

[Peter Maydell]

On 15 August 2017 at 14:27, Stefan Hajnoczi  wrote:
> The following changes since commit 5681da292242550f37ba4c03f46a8a6f8ee9278a:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170815' into 
> staging (2017-08-15 09:39:14 +0100)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/tracing-pull-request
>
> for you to fetch changes up to 840d8351774664d01c328d31ed33b0e2d85c866e:
>
>   simpletrace: fix flight recorder --no-header option (2017-08-15 12:50:29 
> +0100)
>
> 
>
> 
>
> Stefan Hajnoczi (2):
>   trace: use static event ID mapping in simpletrace.stp
>   simpletrace: fix flight recorder --no-header option
>
>  scripts/simpletrace.py   | 24 +++--
>  scripts/tracetool/format/simpletrace_stap.py | 31 
> ++--
>  2 files changed, 20 insertions(+), 35 deletions(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH for-2.10 v2 2/4] nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cache

[Eric Blake]

On 08/15/2017 08:07 AM, Fam Zheng wrote:
> From: Kevin Wolf 
> 
> The "inactive" state of BDS affects whether the permissions can be
> granted, we must call bdrv_invalidate_cache before bdrv_set_perm to
> support "-incoming defer" case.
> 
> Reported-by: Christian Ehrhardt 
> Signed-off-by: Fam Zheng 
> ---
>  nbd/server.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index 82a78bf439..b68b6fb535 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1045,11 +1045,22 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
> dev_offset, off_t size,
>bool writethrough, BlockBackend *on_eject_blk,
>Error **errp)
>  {
> +AioContext  *ctx;

Odd spacing; can fix up as maintainer.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/6] block: add options parameter to bdrv_new_open_driver()

[Alberto Garcia]

On Wed 09 Aug 2017 04:02:52 PM CEST, Manos Pitsidianakis wrote:
> Allow passing a QDict *options parameter to bdrv_new_open_driver() so
> that it can be used if a driver needs it upon creation. The previous
> behaviour (empty bs->options and bs->explicit_options) remains when
> options is NULL.
>
> Signed-off-by: Manos Pitsidianakis 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [PATCH for-2.10 v2 3/4] block-backend: Defer shared_perm tightening migration completion

[Eric Blake]

On 08/15/2017 08:07 AM, Fam Zheng wrote:
> As in the case of nbd_export_new(), bdrv_invalidate_cache() can be
> called when migration is still in progress. In this case we are not
> ready to tighten the shared permissions fenced by blk->disable_perm.
> 
> Defer to a VM state change handler.
> 
> Signed-off-by: Fam Zheng 
> ---
>  block/block-backend.c | 41 +
>  1 file changed, 41 insertions(+)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.10 v2 4/4] iotests: Add non-shared storage migration case 192

[Eric Blake]

On 08/15/2017 08:07 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  tests/qemu-iotests/192 | 63 
> ++
>  tests/qemu-iotests/192.out |  7 ++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 71 insertions(+)
>  create mode 100755 tests/qemu-iotests/192
>  create mode 100644 tests/qemu-iotests/192.out

I tested that this catches the changes made in both 2/4 (+Block node is
read-only) and 3/4 (+Conflicts with use by drive0 as 'root', which does
not allow 'write' on #block121) - so it looks like we've solved the issue.

Tested-by: Eric Blake 
Reviewed-by: Eric Blake 

I'll take this series through my NBD tree, and send a pull request
shortly in time for -rc3

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH for-2.10] mmio-interface: Mark as not user creatable

[Peter Maydell]

The mmio-interface device is not something we want to allow
users to create on the command line:
 * it is intended as an implementation detail of the memory
   subsystem, which gets created and deleted by that
   subsystem on demand; it makes no sense to create it
   by hand on the command line
 * it uses a pointer property 'host_ptr' which can't be
   set on the command line

Mark the device as not user_creatable to avoid confusion.

Signed-off-by: Peter Maydell 
---
 hw/misc/mmio_interface.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c
index da154e5..894e980 100644
--- a/hw/misc/mmio_interface.c
+++ b/hw/misc/mmio_interface.c
@@ -111,6 +111,11 @@ static void mmio_interface_class_init(ObjectClass *oc, 
void *data)
 dc->realize = mmio_interface_realize;
 dc->unrealize = mmio_interface_unrealize;
 dc->props = mmio_interface_properties;
+/* Reason: pointer property "host_ptr", and this device
+ * is an implementation detail of the memory subsystem,
+ * not intended to be created directly by the user.
+ */
+dc->user_creatable = false;
 }
 
 static const TypeInfo mmio_interface_info = {
-- 
2.7.4

Re: [Qemu-devel] [PATCH for 2.10?] qxl: call qemu_spice_display_init_common for secondary devices

[Peter Maydell]

On 15 August 2017 at 14:39, Stefan Hajnoczi  wrote:
> On Tue, Aug 15, 2017 at 01:15:52AM +0200, Paolo Bonzini wrote:
>> Fixes this 2.10 regression:
>>
>>   $ qemu-system-x86_64  -cpu host -m 6144 -vga qxl -device qxl
>>   qemu-system-x86_64: util/qemu-thread-posix.c:64: qemu_mutex_lock: 
>> Assertion `mutex->initialized' failed.
>>
>> Reported-by: adema...@redhat.com
>> Cc: kra...@redhat.com
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  hw/display/qxl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> I have audited the code and cannot see any bad side-effects.  Looks good
> to me!
>
> Reviewed-by: Stefan Hajnoczi 

Thanks; applied to master.

-- PMM

Re: [Qemu-devel] [PATCH 01/26] qapi: fix type_seen key error

[Markus Armbruster]

Marc-André Lureau  writes:

> The type_seen member can be of a different type than the 'qtype' being
> checked, since a string create several conflicts. Lookup the real
> conflicting type in the conflict set, that one must be present in
> type_seen.
>
> This fixes the following error, reproducible with the modified test:
>
> Traceback (most recent call last):
>   File "/home/elmarco/src/qq/tests/qapi-schema/test-qapi.py", line 56, in 
> 
> schema = QAPISchema(sys.argv[1])
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 1470, in __init__
> self.exprs = check_exprs(parser.exprs)
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 959, in check_exprs
> check_alternate(expr, info)
>   File "/home/elmarco/src/qq/scripts/qapi.py", line 831, in check_alternate
> % (name, key, types_seen[qtype]))
> KeyError: 'QTYPE_QSTRING'
>
> Signed-off-by: Marc-André Lureau 
> ---
>  scripts/qapi.py  | 4 +++-
>  tests/qapi-schema/alternate-conflict-string.json | 4 ++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 8aa2775f12..4ecc19e944 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -825,7 +825,9 @@ def check_alternate(expr, info):
>  else:
>  conflicting.add('QTYPE_QNUM')
>  conflicting.add('QTYPE_QBOOL')
> -if conflicting & set(types_seen):
> +conflict = conflicting & set(types_seen)
> +if conflict:
> +qtype = list(conflict)[0]
>  raise QAPISemError(info, "Alternate '%s' member '%s' can't "
> "be distinguished from member '%s'"
> % (name, key, types_seen[qtype]))

I see.

The fix is uninvasive, but I'm not thrilled by repurposing @qtype, and
the meaning of list(dict) is less obvious than dict.keys().  What about

   conflict = conflicting & set(types_seen)
   if conflict:
   conflicting_member_name = conflict.keys()[0]
   raise QAPISemError(info, "Alternate '%s' member '%s' can't "
  "be distinguished from member '%s'"
  % (name, key, conflicting_member_name))

Alternatively, list all conflicting names (I wouldn't bother).

> diff --git a/tests/qapi-schema/alternate-conflict-string.json 
> b/tests/qapi-schema/alternate-conflict-string.json
> index 85adbd4adc..bb2702978e 100644
> --- a/tests/qapi-schema/alternate-conflict-string.json
> +++ b/tests/qapi-schema/alternate-conflict-string.json
> @@ -1,4 +1,4 @@
>  # alternate branches of 'str' type conflict with all scalar types
>  { 'alternate': 'Alt',
> -  'data': { 'one': 'str',
> -'two': 'int' } }
> +  'data': { 'one': 'int',
> +'two': 'str' } }

I had to think for several minutes to convince myself that this is a
better test, not just a test that happens to demonstrate a particular
bug.  It's hot, I'm slow :)

Re: [Qemu-devel] [PATCH v3 00/12] Convert over to use keycodemapdb

[Programmingkid]

> On Aug 15, 2017, at 9:49 AM, Daniel P. Berrange  wrote:
> 
> On Tue, Aug 15, 2017 at 09:49:00AM -0400, Programmingkid wrote:
>> 
>>> On Aug 15, 2017, at 4:38 AM, Daniel P. Berrange  wrote:
>>> 
>>> On Mon, Aug 14, 2017 at 02:46:22PM -0400, Programmingkid wrote:
 Sorry but there appears to be an issue with your patchset. I ran this 
 command:
 
 ./configure --target-list=ppc-softmmu,i386-softmmu
 
 Then saw this error message: 
 
 error: pathspec 'ui/keycodemapdb' did not match any file(s) known to git.
 
 Running 'make' I saw this error message:
 
 make: *** No rule to make target `ui/input-keymap-atset1-to-qcode.c', 
 needed by `Makefile'.  Stop.
 make: *** Waiting for unfinished jobs
 
 Running the 'make clean' command produces this error message:
 
 make[1]: *** No rule to make target `ui/input-keymap-atset1-to-qcode.c', 
 needed by `Makefile'.  Stop.
 make: *** [clean] Error 1
 
 Perhaps a missing patch?
>>> 
>>> No, its a missing "git submodule update --init ui/keycodemapdb" call
>> 
>> Thank you for replying. I tried this command and saw this error message:
>> 
>> error: pathspec 'ui/keycodemapdb' did not match any file(s) known to git.
>> 
>> Note: I'm using version 3. Will test version 4 soon. 
>> 
>> I suggest adding the git submodule update info to your cover letter
>> saying it is required.
> 
> It isn't supposed to be required - its a bug, hence why the tests are
> failing.
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Version four appears to have the same problem as version three. I still see 
this error message:

make: *** No rule to make target `ui/input-keymap-atset1-to-qcode.c', needed by 
`Makefile'.  Stop.
make: *** Waiting for unfinished jobs

Re: [Qemu-devel] [PATCH for-2.10 v2] nbd-client: Fix regression when server sends garbage

[Vladimir Sementsov-Ogievskiy]

15.08.2017 00:34, Eric Blake wrote:

When we switched NBD to use coroutines for qemu 2.9 (in particular,
commit a12a712a), we introduced a regression: if a server sends us
garbage (such as a corrupted magic number), we quit the read loop
but do not stop sending further queued commands, resulting in the
client hanging when it never reads the response to those additional
commands.  In qemu 2.8, we properly detected that the server is no
longer reliable, and cancelled all existing pending commands with
EIO, then tore down the socket so that all further command attempts
get EPIPE.

Restore the proper behavior of quitting (almost) all communication
with a broken server: Once we know we are out of sync or otherwise
can't trust the server, we must assume that any further incoming
data is unreliable and therefore end all pending commands with EIO,
and quit trying to send any further commands.  As an exception, we
still (try to) send NBD_CMD_DISC to let the server know we are going
away (in part, because it is easier to do that than to further
refactor nbd_teardown_connection, and in part because it is the
only command where we do not have to wait for a reply).

Based on a patch by Vladimir Sementsov-Ogievskiy.

A malicious server can be created with the following hack,
followed by setting NBD_SERVER_DEBUG to a non-zero value in the
environment when running qemu-nbd:

| --- a/nbd/server.c
| +++ b/nbd/server.c
| @@ -919,6 +919,17 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply 
*reply, Error **errp)
|  stl_be_p(buf + 4, reply->error);
|  stq_be_p(buf + 8, reply->handle);
|
| +static int debug;
| +static int count;
| +if (!count++) {
| +const char *str = getenv("NBD_SERVER_DEBUG");
| +if (str) {
| +debug = atoi(str);
| +}
| +}
| +if (debug && !(count % debug)) {
| +buf[0] = 0;
| +}
|  return nbd_write(ioc, buf, sizeof(buf), errp);
|  }

Reported-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Eric Blake 
---

Supercedes both Vladimir and my earlier attempts:
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02131.html
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01501.html

  block/nbd-client.h |  1 +
  block/nbd-client.c | 14 ++
  2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index df80771357..1935ffbcaa 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -29,6 +29,7 @@ typedef struct NBDClientSession {

  Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
  NBDReply reply;
+bool quit;
  } NBDClientSession;

  NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 25dd28406b..bb17e3da45 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -73,7 +73,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
  int ret;
  Error *local_err = NULL;

-for (;;) {
+while (!s->quit) {


looks like quit will never be true here


  assert(s->reply.handle == 0);
  ret = nbd_receive_reply(s->ioc, &s->reply, &local_err);
  if (ret < 0) {
@@ -107,6 +107,9 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
  qemu_coroutine_yield();
  }

+if (ret < 0) {
+s->quit = true;


so, you set quit only here.. if we fail on some write, reading coroutine 
will not

know about it and will continue reading..


+}
  nbd_recv_coroutines_enter_all(s);
  s->read_reply_co = NULL;
  }
@@ -135,6 +138,10 @@ static int nbd_co_send_request(BlockDriverState *bs,
  assert(i < MAX_NBD_REQUESTS);
  request->handle = INDEX_TO_HANDLE(s, i);

+if (s->quit) {
+qemu_co_mutex_unlock(&s->send_mutex);
+return -EIO;
+}
  if (!s->ioc) {
  qemu_co_mutex_unlock(&s->send_mutex);
  return -EPIPE;
@@ -143,7 +150,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
  if (qiov) {
  qio_channel_set_cork(s->ioc, true);
  rc = nbd_send_request(s->ioc, request);
-if (rc >= 0) {
+if (rc >= 0 && !s->quit) {
  ret = nbd_rwv(s->ioc, qiov->iov, qiov->niov, request->len, false,
NULL);
  if (ret != request->len) {
@@ -168,8 +175,7 @@ static void nbd_co_receive_reply(NBDClientSession *s,
  /* Wait until we're woken up by nbd_read_reply_entry.  */
  qemu_coroutine_yield();
  *reply = s->reply;
-if (reply->handle != request->handle ||
-!s->ioc) {
+if (reply->handle != request->handle || !s->ioc || s->quit) {
  reply->error = EIO;
  } else {
  if (qiov && reply->error == 0) {



--
Best regards,
Vladimir

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 v2] nbd-client: Fix regression when server sends garbage

[Stefan Hajnoczi]

On Mon, Aug 14, 2017 at 04:34:26PM -0500, Eric Blake wrote:
> When we switched NBD to use coroutines for qemu 2.9 (in particular,
> commit a12a712a), we introduced a regression: if a server sends us
> garbage (such as a corrupted magic number), we quit the read loop
> but do not stop sending further queued commands, resulting in the
> client hanging when it never reads the response to those additional
> commands.  In qemu 2.8, we properly detected that the server is no
> longer reliable, and cancelled all existing pending commands with
> EIO, then tore down the socket so that all further command attempts
> get EPIPE.
> 
> Restore the proper behavior of quitting (almost) all communication
> with a broken server: Once we know we are out of sync or otherwise
> can't trust the server, we must assume that any further incoming
> data is unreliable and therefore end all pending commands with EIO,
> and quit trying to send any further commands.  As an exception, we
> still (try to) send NBD_CMD_DISC to let the server know we are going
> away (in part, because it is easier to do that than to further
> refactor nbd_teardown_connection, and in part because it is the
> only command where we do not have to wait for a reply).
> 
> Based on a patch by Vladimir Sementsov-Ogievskiy.
> 
> A malicious server can be created with the following hack,
> followed by setting NBD_SERVER_DEBUG to a non-zero value in the
> environment when running qemu-nbd:
> 
> | --- a/nbd/server.c
> | +++ b/nbd/server.c
> | @@ -919,6 +919,17 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply 
> *reply, Error **errp)
> |  stl_be_p(buf + 4, reply->error);
> |  stq_be_p(buf + 8, reply->handle);
> |
> | +static int debug;
> | +static int count;
> | +if (!count++) {
> | +const char *str = getenv("NBD_SERVER_DEBUG");
> | +if (str) {
> | +debug = atoi(str);
> | +}
> | +}
> | +if (debug && !(count % debug)) {
> | +buf[0] = 0;
> | +}
> |  return nbd_write(ioc, buf, sizeof(buf), errp);
> |  }
> 
> Reported-by: Vladimir Sementsov-Ogievskiy 
> Signed-off-by: Eric Blake 
> ---
> 
> Supercedes both Vladimir and my earlier attempts:
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02131.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01501.html
> 
>  block/nbd-client.h |  1 +
>  block/nbd-client.c | 14 ++
>  2 files changed, 11 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 for-2.11 1/2] tests/boot-sector: Do not overwrite the x86 buffer on other architectures

[Cornelia Huck]

On Fri, 11 Aug 2017 07:57:55 +0200
Thomas Huth  wrote:

> Re-using the boot_sector code buffer from x86 for other architectures
> is not very nice, especially if we add more architectures later. It's
> also ugly that the test uses a huge pre-initialized array at all - the
> size of the executable is very huge due to this array. So let's use a
> separate buffer for each architecture instead, allocated from the heap,
> so that we really just use the memory that we need.
> 
> Suggested-by: Michael Tsirkin 
> Signed-off-by: Thomas Huth 
> ---
>  tests/boot-sector.c | 41 ++---
>  1 file changed, 26 insertions(+), 15 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH for-2.10 v2] nbd-client: Fix regression when server sends garbage

[Eric Blake]

On 08/15/2017 09:45 AM, Vladimir Sementsov-Ogievskiy wrote:
> 15.08.2017 00:34, Eric Blake wrote:
>> When we switched NBD to use coroutines for qemu 2.9 (in particular,
>> commit a12a712a), we introduced a regression: if a server sends us
>> garbage (such as a corrupted magic number), we quit the read loop
>> but do not stop sending further queued commands, resulting in the
>> client hanging when it never reads the response to those additional
>> commands.  In qemu 2.8, we properly detected that the server is no
>> longer reliable, and cancelled all existing pending commands with
>> EIO, then tore down the socket so that all further command attempts
>> get EPIPE.
>>
>> Restore the proper behavior of quitting (almost) all communication
>> with a broken server: Once we know we are out of sync or otherwise
>> can't trust the server, we must assume that any further incoming
>> data is unreliable and therefore end all pending commands with EIO,
>> and quit trying to send any further commands.  As an exception, we
>> still (try to) send NBD_CMD_DISC to let the server know we are going
>> away (in part, because it is easier to do that than to further
>> refactor nbd_teardown_connection, and in part because it is the
>> only command where we do not have to wait for a reply).
>>
>> Based on a patch by Vladimir Sementsov-Ogievskiy.
>>

>> +++ b/block/nbd-client.c
>> @@ -73,7 +73,7 @@ static coroutine_fn void nbd_read_reply_entry(void
>> *opaque)
>>   int ret;
>>   Error *local_err = NULL;
>>
>> -for (;;) {
>> +while (!s->quit) {
> 
> looks like quit will never be true here
> 
>>   assert(s->reply.handle == 0);
>>   ret = nbd_receive_reply(s->ioc, &s->reply, &local_err);
>>   if (ret < 0) {
>> @@ -107,6 +107,9 @@ static coroutine_fn void nbd_read_reply_entry(void
>> *opaque)
>>   qemu_coroutine_yield();
>>   }
>>
>> +if (ret < 0) {
>> +s->quit = true;
> 
> so, you set quit only here.. if we fail on some write, reading coroutine
> will not
> know about it and will continue reading..

Looks like you are correct - your version set the flag in more places
than me, but it looks like you're right that we DO want to set the flag
when writing hits a known failure.

Here's what I plan to squash in:

diff --git i/block/nbd-client.c w/block/nbd-client.c
index bb17e3da45..422ecb4307 100644
--- i/block/nbd-client.c
+++ w/block/nbd-client.c
@@ -161,6 +161,9 @@ static int nbd_co_send_request(BlockDriverState *bs,
 } else {
 rc = nbd_send_request(s->ioc, request);
 }
+if (rc < 0) {
+s->quit = true;
+}
 qemu_co_mutex_unlock(&s->send_mutex);
 return rc;
 }

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 for-2.10 2/3] target/arm: Correct load exclusive pair atomicity

[Richard Henderson]

We are not providing the required single-copy atomic semantics for
the 64-bit operation that is the 32-bit paired load.

At the same time, leave the entire 64-bit value in cpu_exclusive_val
and stop writing to cpu_exclusive_high.  This means that we do not
have to re-assemble the 64-bit quantity when it comes time to store.

At the same time, drop a redundant temporary and perform all loads
directly into the cpu_exclusive_* globals.

Tested-by: Alistair Francis 
Reviewed-by: Alistair Francis 
Signed-off-by: Richard Henderson 
---
 target/arm/translate-a64.c | 60 --
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 113e2e172b..eac545e4f2 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1853,29 +1853,42 @@ static void disas_b_exc_sys(DisasContext *s, uint32_t 
insn)
 static void gen_load_exclusive(DisasContext *s, int rt, int rt2,
TCGv_i64 addr, int size, bool is_pair)
 {
-TCGv_i64 tmp = tcg_temp_new_i64();
-TCGMemOp memop = s->be_data + size;
+int idx = get_mem_index(s);
+TCGMemOp memop = s->be_data;
 
 g_assert(size <= 3);
-tcg_gen_qemu_ld_i64(tmp, addr, get_mem_index(s), memop);
-
 if (is_pair) {
-TCGv_i64 addr2 = tcg_temp_new_i64();
-TCGv_i64 hitmp = tcg_temp_new_i64();
-
 g_assert(size >= 2);
-tcg_gen_addi_i64(addr2, addr, 1 << size);
-tcg_gen_qemu_ld_i64(hitmp, addr2, get_mem_index(s), memop);
-tcg_temp_free_i64(addr2);
-tcg_gen_mov_i64(cpu_exclusive_high, hitmp);
-tcg_gen_mov_i64(cpu_reg(s, rt2), hitmp);
-tcg_temp_free_i64(hitmp);
-}
+if (size == 2) {
+/* The pair must be single-copy atomic for the doubleword.  */
+memop |= MO_64;
+tcg_gen_qemu_ld_i64(cpu_exclusive_val, addr, idx, memop);
+if (s->be_data == MO_LE) {
+tcg_gen_extract_i64(cpu_reg(s, rt), cpu_exclusive_val, 0, 32);
+tcg_gen_extract_i64(cpu_reg(s, rt2), cpu_exclusive_val, 32, 
32);
+} else {
+tcg_gen_extract_i64(cpu_reg(s, rt), cpu_exclusive_val, 32, 32);
+tcg_gen_extract_i64(cpu_reg(s, rt2), cpu_exclusive_val, 0, 32);
+}
+} else {
+/* The pair must be single-copy atomic for *each* doubleword,
+   but not the entire quadword.  */
+memop |= MO_64;
+tcg_gen_qemu_ld_i64(cpu_exclusive_val, addr, idx, memop);
 
-tcg_gen_mov_i64(cpu_exclusive_val, tmp);
-tcg_gen_mov_i64(cpu_reg(s, rt), tmp);
+TCGv_i64 addr2 = tcg_temp_new_i64();
+tcg_gen_addi_i64(addr2, addr, 8);
+tcg_gen_qemu_ld_i64(cpu_exclusive_high, addr2, idx, memop);
+tcg_temp_free_i64(addr2);
 
-tcg_temp_free_i64(tmp);
+tcg_gen_mov_i64(cpu_reg(s, rt), cpu_exclusive_val);
+tcg_gen_mov_i64(cpu_reg(s, rt2), cpu_exclusive_high);
+}
+} else {
+memop |= size;
+tcg_gen_qemu_ld_i64(cpu_exclusive_val, addr, idx, memop);
+tcg_gen_mov_i64(cpu_reg(s, rt), cpu_exclusive_val);
+}
 tcg_gen_mov_i64(cpu_exclusive_addr, addr);
 }
 
@@ -1908,14 +1921,15 @@ static void gen_store_exclusive(DisasContext *s, int 
rd, int rt, int rt2,
 tmp = tcg_temp_new_i64();
 if (is_pair) {
 if (size == 2) {
-TCGv_i64 val = tcg_temp_new_i64();
-tcg_gen_concat32_i64(tmp, cpu_reg(s, rt), cpu_reg(s, rt2));
-tcg_gen_concat32_i64(val, cpu_exclusive_val, cpu_exclusive_high);
-tcg_gen_atomic_cmpxchg_i64(tmp, addr, val, tmp,
+if (s->be_data == MO_LE) {
+tcg_gen_concat32_i64(tmp, cpu_reg(s, rt), cpu_reg(s, rt2));
+} else {
+tcg_gen_concat32_i64(tmp, cpu_reg(s, rt2), cpu_reg(s, rt));
+}
+tcg_gen_atomic_cmpxchg_i64(tmp, addr, cpu_exclusive_val, tmp,
get_mem_index(s),
MO_64 | MO_ALIGN | s->be_data);
-tcg_gen_setcond_i64(TCG_COND_NE, tmp, tmp, val);
-tcg_temp_free_i64(val);
+tcg_gen_setcond_i64(TCG_COND_NE, tmp, tmp, cpu_exclusive_val);
 } else if (s->be_data == MO_LE) {
 gen_helper_paired_cmpxchg64_le(tmp, cpu_env, addr, cpu_reg(s, rt),
cpu_reg(s, rt2));
-- 
2.13.4