[Qemu-devel] [PULL 00/15]: QMP queue

[Qemu-devel] [PULL 00/15]: QMP queue

[Luiz Capitulino]

The changes (since ac05f3492421caeb05809ffa02c6198ede179e43) are available
in the following repository:

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

Luiz Capitulino (7):
  qapi: convert add_client
  qmp: dump-guest-memory: improve schema doc (again)
  qmp: dump-guest-memory: don't spin if non-blocking fd would block
  hmp: dump-guest-memory: hardcode protocol argument to "file:"
  input: qmp_send_key(): simplify
  qmp: qmp_send_key(): accept key codes in hex
  input: index_from_key(): drop unused code

Paolo Bonzini (5):
  qapi: do not protect enum values from namespace pollution
  qapi: add "unix" to the set of reserved words
  pci-assign: use monitor_handle_fd_param
  monitor: add Error * argument to monitor_get_fd
  block: live snapshot documentation tweaks

Ryota Ozaki (3):
  Make negotiation optional in QEMUMonitorProtocol
  Support settimeout in QEMUMonitorProtocol
  Add qemu-ga-client script

 QMP/qemu-ga-client    | 299 ++++++++++++++++++++++++++++++++++++++++++++++++++
 QMP/qmp.py            |  12 +-
 dump.c                |  18 +--
 hmp-commands.hx       |   8 +-
 hmp.c                 |  51 ++++++---
 hw/kvm/pci-assign.c   |  12 +-
 input.c               |  75 ++++++-------
 migration-fd.c        |   2 +-
 monitor.c             |  48 +-------
 monitor.h             |   2 +-
 qapi-schema.json      |  81 +++++++++++---
 qmp-commands.hx       |   5 +-
 qmp.c                 |  43 ++++++++
 scripts/qapi-types.py |   4 +-
 scripts/qapi-visit.py |   2 +-
 scripts/qapi.py       |  10 +-
 16 files changed, 517 insertions(+), 155 deletions(-)
 create mode 100755 QMP/qemu-ga-client

-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 01/15] Make negotiation optional in QEMUMonitorProtocol

[Luiz Capitulino]

From: Ryota Ozaki <ozaki.ryota@gmail.com>

This is a preparation for qemu-ga-client which uses
QEMUMonitorProtocol class. The class tries to
negotiate capabilities on connect, however, qemu-ga
doesn't suppose it and fails.

This change makes the negotiation optional, though
it's still performed by default for compatibility.

Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index 36ecc1d..5a573e1 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -49,7 +49,6 @@ class QEMUMonitorProtocol:
         return socket.socket(family, socket.SOCK_STREAM)
 
     def __negotiate_capabilities(self):
-        self.__sockfile = self.__sock.makefile()
         greeting = self.__json_read()
         if greeting is None or not greeting.has_key('QMP'):
             raise QMPConnectError
@@ -73,7 +72,7 @@ class QEMUMonitorProtocol:
 
     error = socket.error
 
-    def connect(self):
+    def connect(self, negotiate=True):
         """
         Connect to the QMP Monitor and perform capabilities negotiation.
 
@@ -83,7 +82,9 @@ class QEMUMonitorProtocol:
         @raise QMPCapabilitiesError if fails to negotiate capabilities
         """
         self.__sock.connect(self.__address)
-        return self.__negotiate_capabilities()
+        self.__sockfile = self.__sock.makefile()
+        if negotiate:
+            return self.__negotiate_capabilities()
 
     def accept(self):
         """
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 02/15] Support settimeout in QEMUMonitorProtocol

[Luiz Capitulino]

From: Ryota Ozaki <ozaki.ryota@gmail.com>

This method is used in the following qemu-ga-client script
to implement non-blocking operations.

Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index 5a573e1..33c7d36 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -162,3 +162,8 @@ class QEMUMonitorProtocol:
     def close(self):
         self.__sock.close()
         self.__sockfile.close()
+
+    timeout = socket.timeout
+
+    def settimeout(self, timeout):
+        self.__sock.settimeout(timeout)
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 03/15] Add qemu-ga-client script

[Luiz Capitulino]

From: Ryota Ozaki <ozaki.ryota@gmail.com>

This is an easy-to-use QEMU guest agent client written in
Python. It simply provides commands to call guest agent
functions like ping, fsfreeze and shutdown. Additionally,
it provides extra useful commands, e.g, cat, ifconfig and
reboot, by using guet agent functions.

Examples:
  $ export QGA_CLIENT_ADDRESS=/tmp/qga.sock
  $ qemu-ga-client ping

  $ qemu-ga-client cat /etc/resolv.conf
  # Generated by NetworkManager
  nameserver 10.0.2.3

  $ qemu-ga-client fsfreeze status
  thawed
  $ qemu-ga-client fsfreeze freeze
  2 filesystems frozen

The script communicates with a guest agent by means of
qmp.QEMUMonitorProtocol. Every commands are called with
timeout (3 sec.) to avoid blocking. The script always
calls sync command prior to issuing an actual command
(except for ping which doesn't need sync).

Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qemu-ga-client | 299 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 299 insertions(+)
 create mode 100755 QMP/qemu-ga-client

diff --git a/QMP/qemu-ga-client b/QMP/qemu-ga-client
new file mode 100755
index 0000000..46676c3
--- /dev/null
+++ b/QMP/qemu-ga-client
@@ -0,0 +1,299 @@
+#!/usr/bin/python
+
+# QEMU Guest Agent Client
+#
+# Copyright (C) 2012 Ryota Ozaki <ozaki.ryota@gmail.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+# Usage:
+#
+# Start QEMU with:
+#
+# # qemu [...] -chardev socket,path=/tmp/qga.sock,server,nowait,id=qga0 \
+#   -device virtio-serial -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0
+#
+# Run the script:
+#
+# $ qemu-ga-client --address=/tmp/qga.sock <command> [args...]
+#
+# or
+#
+# $ export QGA_CLIENT_ADDRESS=/tmp/qga.sock
+# $ qemu-ga-client <command> [args...]
+#
+# For example:
+#
+# $ qemu-ga-client cat /etc/resolv.conf
+# # Generated by NetworkManager
+# nameserver 10.0.2.3
+# $ qemu-ga-client fsfreeze status
+# thawed
+# $ qemu-ga-client fsfreeze freeze
+# 2 filesystems frozen
+#
+# See also: http://wiki.qemu.org/Features/QAPI/GuestAgent
+#
+
+import base64
+import random
+
+import qmp
+
+
+class QemuGuestAgent(qmp.QEMUMonitorProtocol):
+    def __getattr__(self, name):
+        def wrapper(**kwds):
+            return self.command('guest-' + name.replace('_', '-'), **kwds)
+        return wrapper
+
+
+class QemuGuestAgentClient:
+    error = QemuGuestAgent.error
+
+    def __init__(self, address):
+        self.qga = QemuGuestAgent(address)
+        self.qga.connect(negotiate=False)
+
+    def sync(self, timeout=3):
+        # Avoid being blocked forever
+        if not self.ping(timeout):
+            raise EnvironmentError('Agent seems not alive')
+        uid = random.randint(0, (1 << 32) - 1)
+        while True:
+            ret = self.qga.sync(id=uid)
+            if isinstance(ret, int) and int(ret) == uid:
+                break
+
+    def __file_read_all(self, handle):
+        eof = False
+        data = ''
+        while not eof:
+            ret = self.qga.file_read(handle=handle, count=1024)
+            _data = base64.b64decode(ret['buf-b64'])
+            data += _data
+            eof = ret['eof']
+        return data
+
+    def read(self, path):
+        handle = self.qga.file_open(path=path)
+        try:
+            data = self.__file_read_all(handle)
+        finally:
+            self.qga.file_close(handle=handle)
+        return data
+
+    def info(self):
+        info = self.qga.info()
+
+        msgs = []
+        msgs.append('version: ' + info['version'])
+        msgs.append('supported_commands:')
+        enabled = [c['name'] for c in info['supported_commands'] if c['enabled']]
+        msgs.append('\tenabled: ' + ', '.join(enabled))
+        disabled = [c['name'] for c in info['supported_commands'] if not c['enabled']]
+        msgs.append('\tdisabled: ' + ', '.join(disabled))
+
+        return '\n'.join(msgs)
+
+    def __gen_ipv4_netmask(self, prefixlen):
+        mask = int('1' * prefixlen + '0' * (32 - prefixlen), 2)
+        return '.'.join([str(mask >> 24),
+                         str((mask >> 16) & 0xff),
+                         str((mask >> 8) & 0xff),
+                         str(mask & 0xff)])
+
+    def ifconfig(self):
+        nifs = self.qga.network_get_interfaces()
+
+        msgs = []
+        for nif in nifs:
+            msgs.append(nif['name'] + ':')
+            if 'ip-addresses' in nif:
+                for ipaddr in nif['ip-addresses']:
+                    if ipaddr['ip-address-type'] == 'ipv4':
+                        addr = ipaddr['ip-address']
+                        mask = self.__gen_ipv4_netmask(int(ipaddr['prefix']))
+                        msgs.append("\tinet %s  netmask %s" % (addr, mask))
+                    elif ipaddr['ip-address-type'] == 'ipv6':
+                        addr = ipaddr['ip-address']
+                        prefix = ipaddr['prefix']
+                        msgs.append("\tinet6 %s  prefixlen %s" % (addr, prefix))
+            if nif['hardware-address'] != '00:00:00:00:00:00':
+                msgs.append("\tether " + nif['hardware-address'])
+
+        return '\n'.join(msgs)
+
+    def ping(self, timeout):
+        self.qga.settimeout(timeout)
+        try:
+            self.qga.ping()
+        except self.qga.timeout:
+            return False
+        return True
+
+    def fsfreeze(self, cmd):
+        if cmd not in ['status', 'freeze', 'thaw']:
+            raise StandardError('Invalid command: ' + cmd)
+
+        return getattr(self.qga, 'fsfreeze' + '_' + cmd)()
+
+    def fstrim(self, minimum=0):
+        return getattr(self.qga, 'fstrim')(minimum=minimum)
+
+    def suspend(self, mode):
+        if mode not in ['disk', 'ram', 'hybrid']:
+            raise StandardError('Invalid mode: ' + mode)
+
+        try:
+            getattr(self.qga, 'suspend' + '_' + mode)()
+            # On error exception will raise
+        except self.qga.timeout:
+            # On success command will timed out
+            return
+
+    def shutdown(self, mode='powerdown'):
+        if mode not in ['powerdown', 'halt', 'reboot']:
+            raise StandardError('Invalid mode: ' + mode)
+
+        try:
+            self.qga.shutdown(mode=mode)
+        except self.qga.timeout:
+            return
+
+
+def _cmd_cat(client, args):
+    if len(args) != 1:
+        print('Invalid argument')
+        print('Usage: cat <file>')
+        sys.exit(1)
+    print(client.read(args[0]))
+
+
+def _cmd_fsfreeze(client, args):
+    usage = 'Usage: fsfreeze status|freeze|thaw'
+    if len(args) != 1:
+        print('Invalid argument')
+        print(usage)
+        sys.exit(1)
+    if args[0] not in ['status', 'freeze', 'thaw']:
+        print('Invalid command: ' + args[0])
+        print(usage)
+        sys.exit(1)
+    cmd = args[0]
+    ret = client.fsfreeze(cmd)
+    if cmd == 'status':
+        print(ret)
+    elif cmd == 'freeze':
+        print("%d filesystems frozen" % ret)
+    else:
+        print("%d filesystems thawed" % ret)
+
+
+def _cmd_fstrim(client, args):
+    if len(args) == 0:
+        minimum = 0
+    else:
+        minimum = int(args[0])
+    print(client.fstrim(minimum))
+
+
+def _cmd_ifconfig(client, args):
+    print(client.ifconfig())
+
+
+def _cmd_info(client, args):
+    print(client.info())
+
+
+def _cmd_ping(client, args):
+    if len(args) == 0:
+        timeout = 3
+    else:
+        timeout = float(args[0])
+    alive = client.ping(timeout)
+    if not alive:
+        print("Not responded in %s sec" % args[0])
+        sys.exit(1)
+
+
+def _cmd_suspend(client, args):
+    usage = 'Usage: suspend disk|ram|hybrid'
+    if len(args) != 1:
+        print('Less argument')
+        print(usage)
+        sys.exit(1)
+    if args[0] not in ['disk', 'ram', 'hybrid']:
+        print('Invalid command: ' + args[0])
+        print(usage)
+        sys.exit(1)
+    client.suspend(args[0])
+
+
+def _cmd_shutdown(client, args):
+    client.shutdown()
+_cmd_powerdown = _cmd_shutdown
+
+
+def _cmd_halt(client, args):
+    client.shutdown('halt')
+
+
+def _cmd_reboot(client, args):
+    client.shutdown('reboot')
+
+
+commands = [m.replace('_cmd_', '') for m in dir() if '_cmd_' in m]
+
+
+def main(address, cmd, args):
+    if not os.path.exists(address):
+        print('%s not found' % address)
+        sys.exit(1)
+
+    if cmd not in commands:
+        print('Invalid command: ' + cmd)
+        print('Available commands: ' + ', '.join(commands))
+        sys.exit(1)
+
+    try:
+        client = QemuGuestAgentClient(address)
+    except QemuGuestAgent.error, e:
+        import errno
+
+        print(e)
+        if e.errno == errno.ECONNREFUSED:
+            print('Hint: qemu is not running?')
+        sys.exit(1)
+
+    if cmd != 'ping':
+        client.sync()
+
+    globals()['_cmd_' + cmd](client, args)
+
+
+if __name__ == '__main__':
+    import sys
+    import os
+    import optparse
+
+    address = os.environ['QGA_CLIENT_ADDRESS'] if 'QGA_CLIENT_ADDRESS' in os.environ else None
+
+    usage = "%prog [--address=<unix_path>|<ipv4_address>] <command> [args...]\n"
+    usage += '<command>: ' + ', '.join(commands)
+    parser = optparse.OptionParser(usage=usage)
+    parser.add_option('--address', action='store', type='string',
+                      default=address, help='Specify a ip:port pair or a unix socket path')
+    options, args = parser.parse_args()
+
+    address = options.address
+    if address is None:
+        parser.error('address is not specified')
+        sys.exit(1)
+
+    if len(args) == 0:
+        parser.error('Less argument')
+        sys.exit(1)
+
+    main(address, args[0], args[1:])
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 04/15] qapi: do not protect enum values from namespace pollution

[Luiz Capitulino]

From: Paolo Bonzini <pbonzini@redhat.com>

Enum values are always preceded by the uppercase name of the enum, so
they do not conflict with reserved words.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi-types.py | 4 ++--
 scripts/qapi-visit.py | 2 +-
 scripts/qapi.py       | 8 ++++----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 49ef569..1b84834 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -91,9 +91,9 @@ const char *%(name)s_lookup[] = {
 
 def generate_enum_name(name):
     if name.isupper():
-        return c_fun(name)
+        return c_fun(name, False)
     new_name = ''
-    for c in c_fun(name):
+    for c in c_fun(name, False):
         if c.isupper():
             new_name += '_'
         new_name += c
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e2093e8..a360de7 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -173,7 +173,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
                 break;
 ''',
                 abbrev = de_camel_case(name).upper(),
-                enum = c_fun(de_camel_case(key)).upper(),
+                enum = c_fun(de_camel_case(key),False).upper(),
                 c_type=members[key],
                 c_name=c_fun(key))
 
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 122b4cb..057332e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -141,7 +141,7 @@ def camel_case(name):
             new_name += ch.lower()
     return new_name
 
-def c_var(name):
+def c_var(name, protect=True):
     # ANSI X3J11/88-090, 3.1.1
     c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
                      'default', 'do', 'double', 'else', 'enum', 'extern', 'float',
@@ -156,12 +156,12 @@ def c_var(name):
     # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
     # excluding _.*
     gcc_words = set(['asm', 'typeof'])
-    if name in c89_words | c99_words | c11_words | gcc_words:
+    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
         return "q_" + name
     return name.replace('-', '_').lstrip("*")
 
-def c_fun(name):
-    return c_var(name).replace('.', '_')
+def c_fun(name, protect=True):
+    return c_var(name, protect).replace('.', '_')
 
 def c_list_type(name):
     return '%sList' % name
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 05/15] qapi: add "unix" to the set of reserved words

[Luiz Capitulino]

From: Paolo Bonzini <pbonzini@redhat.com>

It is #defined to 1.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 057332e..afc5f32 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -156,7 +156,9 @@ def c_var(name, protect=True):
     # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
     # excluding _.*
     gcc_words = set(['asm', 'typeof'])
-    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
+    # namespace pollution:
+    polluted_words = set(['unix'])
+    if protect and (name in c89_words | c99_words | c11_words | gcc_words | polluted_words):
         return "q_" + name
     return name.replace('-', '_').lstrip("*")
 
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 06/15] pci-assign: use monitor_handle_fd_param

[Luiz Capitulino]

From: Paolo Bonzini <pbonzini@redhat.com>

There is no need to open-code the choice between a file descriptor
number or a named one.  Just use monitor_handle_fd_param, which
also takes care of printing the error message.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/kvm/pci-assign.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
index 05b93d9..7a0998c 100644
--- a/hw/kvm/pci-assign.c
+++ b/hw/kvm/pci-assign.c
@@ -579,15 +579,9 @@ static int get_real_device(AssignedDevice *pci_dev, uint16_t r_seg,
     snprintf(name, sizeof(name), "%sconfig", dir);
 
     if (pci_dev->configfd_name && *pci_dev->configfd_name) {
-        if (qemu_isdigit(pci_dev->configfd_name[0])) {
-            dev->config_fd = strtol(pci_dev->configfd_name, NULL, 0);
-        } else {
-            dev->config_fd = monitor_get_fd(cur_mon, pci_dev->configfd_name);
-            if (dev->config_fd < 0) {
-                error_report("%s: (%s) unkown", __func__,
-                             pci_dev->configfd_name);
-                return 1;
-            }
+        dev->config_fd = monitor_handle_fd_param(cur_mon, pci_dev->configfd_name);
+        if (dev->config_fd < 0) {
+            return 1;
         }
     } else {
         dev->config_fd = open(name, O_RDWR);
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 07/15] monitor: add Error * argument to monitor_get_fd

[Luiz Capitulino]

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 dump.c         |  3 +--
 migration-fd.c |  2 +-
 monitor.c      | 15 +++++++++------
 monitor.h      |  2 +-
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/dump.c b/dump.c
index 2bf8d8d..1a3c716 100644
--- a/dump.c
+++ b/dump.c
@@ -836,9 +836,8 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
 
 #if !defined(WIN32)
     if (strstart(file, "fd:", &p)) {
-        fd = monitor_get_fd(cur_mon, p);
+        fd = monitor_get_fd(cur_mon, p, errp);
         if (fd == -1) {
-            error_set(errp, QERR_FD_NOT_FOUND, p);
             return;
         }
     }
diff --git a/migration-fd.c b/migration-fd.c
index 50138ed..7335167 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -75,7 +75,7 @@ static int fd_close(MigrationState *s)
 
 int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
 {
-    s->fd = monitor_get_fd(cur_mon, fdname);
+    s->fd = monitor_get_fd(cur_mon, fdname, NULL);
     if (s->fd == -1) {
         DPRINTF("fd_migration: invalid file descriptor identifier\n");
         goto err_after_get_fd;
diff --git a/monitor.c b/monitor.c
index 67064e2..c24235e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -951,7 +951,7 @@ static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_d
     CharDriverState *s;
 
     if (strcmp(protocol, "spice") == 0) {
-        int fd = monitor_get_fd(mon, fdname);
+        int fd = monitor_get_fd(mon, fdname, NULL);
         int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
         int tls = qdict_get_try_bool(qdict, "tls", 0);
         if (!using_spice) {
@@ -965,13 +965,13 @@ static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_d
         return 0;
 #ifdef CONFIG_VNC
     } else if (strcmp(protocol, "vnc") == 0) {
-	int fd = monitor_get_fd(mon, fdname);
+	int fd = monitor_get_fd(mon, fdname, NULL);
         int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
 	vnc_display_add_client(NULL, fd, skipauth);
 	return 0;
 #endif
     } else if ((s = qemu_chr_find(protocol)) != NULL) {
-	int fd = monitor_get_fd(mon, fdname);
+	int fd = monitor_get_fd(mon, fdname, NULL);
 	if (qemu_chr_add_client(s, fd) < 0) {
 	    qerror_report(QERR_ADD_CLIENT_FAILED);
 	    return -1;
@@ -2118,7 +2118,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
     }
 }
 
-int monitor_get_fd(Monitor *mon, const char *fdname)
+int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
 {
     mon_fd_t *monfd;
 
@@ -2139,6 +2139,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
         return fd;
     }
 
+    error_setg(errp, "File descriptor named '%s' has not been found", fdname);
     return -1;
 }
 
@@ -2410,12 +2411,14 @@ int monitor_fdset_dup_fd_remove(int dup_fd)
 int monitor_handle_fd_param(Monitor *mon, const char *fdname)
 {
     int fd;
+    Error *local_err = NULL;
 
     if (!qemu_isdigit(fdname[0]) && mon) {
 
-        fd = monitor_get_fd(mon, fdname);
+        fd = monitor_get_fd(mon, fdname, &local_err);
         if (fd == -1) {
-            error_report("No file descriptor named %s found", fdname);
+            qerror_report_err(local_err);
+            error_free(local_err);
             return -1;
         }
     } else {
diff --git a/monitor.h b/monitor.h
index 64c1561..e240c3f 100644
--- a/monitor.h
+++ b/monitor.h
@@ -66,7 +66,7 @@ int monitor_read_block_device_key(Monitor *mon, const char *device,
                                   BlockDriverCompletionFunc *completion_cb,
                                   void *opaque);
 
-int monitor_get_fd(Monitor *mon, const char *fdname);
+int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
 int monitor_handle_fd_param(Monitor *mon, const char *fdname);
 
 void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 08/15] qapi: convert add_client

[Luiz Capitulino]

Also fixes a few issues while there:

 1. The fd returned by monitor_get_fd() leaks in most error conditions
 2. monitor_get_fd() return value is not checked. Best case we get
    an error that is not correctly reported, worse case one of the
    functions using the fd (with value of -1) will explode
 3. A few error conditions aren't reported
 4. We now "use up" @fdname always.  Before, it was left alone for
    invalid @protocol

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c        | 39 ---------------------------------------
 qapi-schema.json | 25 +++++++++++++++++++++++++
 qmp-commands.hx  |  5 +----
 qmp.c            | 43 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 69 insertions(+), 43 deletions(-)

diff --git a/monitor.c b/monitor.c
index c24235e..c9f460a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -944,45 +944,6 @@ static void do_trace_print_events(Monitor *mon)
     trace_print_events((FILE *)mon, &monitor_fprintf);
 }
 
-static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-    const char *protocol  = qdict_get_str(qdict, "protocol");
-    const char *fdname = qdict_get_str(qdict, "fdname");
-    CharDriverState *s;
-
-    if (strcmp(protocol, "spice") == 0) {
-        int fd = monitor_get_fd(mon, fdname, NULL);
-        int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
-        int tls = qdict_get_try_bool(qdict, "tls", 0);
-        if (!using_spice) {
-            /* correct one? spice isn't a device ,,, */
-            qerror_report(QERR_DEVICE_NOT_ACTIVE, "spice");
-            return -1;
-        }
-        if (qemu_spice_display_add_client(fd, skipauth, tls) < 0) {
-            close(fd);
-        }
-        return 0;
-#ifdef CONFIG_VNC
-    } else if (strcmp(protocol, "vnc") == 0) {
-	int fd = monitor_get_fd(mon, fdname, NULL);
-        int skipauth = qdict_get_try_bool(qdict, "skipauth", 0);
-	vnc_display_add_client(NULL, fd, skipauth);
-	return 0;
-#endif
-    } else if ((s = qemu_chr_find(protocol)) != NULL) {
-	int fd = monitor_get_fd(mon, fdname, NULL);
-	if (qemu_chr_add_client(s, fd) < 0) {
-	    qerror_report(QERR_ADD_CLIENT_FAILED);
-	    return -1;
-	}
-	return 0;
-    }
-
-    qerror_report(QERR_INVALID_PARAMETER, "protocol");
-    return -1;
-}
-
 static int client_migrate_info(Monitor *mon, const QDict *qdict,
                                MonitorCompletion cb, void *opaque)
 {
diff --git a/qapi-schema.json b/qapi-schema.json
index 14e4419..191d921 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -33,6 +33,31 @@
             'MigrationExpected' ] }
 
 ##
+# @add_client
+#
+# Allow client connections for VNC, Spice and socket based
+# character devices to be passed in to QEMU via SCM_RIGHTS.
+#
+# @protocol: protocol name. Valid names are "vnc", "spice" or the
+#            name of a character device (eg. from -chardev id=XXXX)
+#
+# @fdname: file descriptor name previously passed via 'getfd' command
+#
+# @skipauth: #optional whether to skip authentication. Only applies
+#            to "vnc" and "spice" protocols
+#
+# @tls: #optional whether to perform TLS. Only applies to the "spice"
+#       protocol
+#
+# Returns: nothing on success.
+#
+# Since: 0.14.0
+##
+{ 'command': 'add_client',
+  'data': { 'protocol': 'str', 'fdname': 'str', '*skipauth': 'bool',
+            '*tls': 'bool' } }
+
+##
 # @NameInfo:
 #
 # Guest name information.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6e21ddb..36e08d9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1231,10 +1231,7 @@ EQMP
     {
         .name       = "add_client",
         .args_type  = "protocol:s,fdname:s,skipauth:b?,tls:b?",
-        .params     = "protocol fdname skipauth tls",
-        .help       = "add a graphics client",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = add_graphics_client,
+        .mhandler.cmd_new = qmp_marshal_input_add_client,
     },
 
 SQMP
diff --git a/qmp.c b/qmp.c
index 8463922..36c54c5 100644
--- a/qmp.c
+++ b/qmp.c
@@ -479,3 +479,46 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
     return arch_query_cpu_definitions(errp);
 }
 
+void qmp_add_client(const char *protocol, const char *fdname,
+                    bool has_skipauth, bool skipauth, bool has_tls, bool tls,
+                    Error **errp)
+{
+    CharDriverState *s;
+    int fd;
+
+    fd = monitor_get_fd(cur_mon, fdname, errp);
+    if (fd < 0) {
+        return;
+    }
+
+    if (strcmp(protocol, "spice") == 0) {
+        if (!using_spice) {
+            error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
+            close(fd);
+            return;
+        }
+        skipauth = has_skipauth ? skipauth : false;
+        tls = has_tls ? tls : false;
+        if (qemu_spice_display_add_client(fd, skipauth, tls) < 0) {
+            error_setg(errp, "spice failed to add client");
+            close(fd);
+        }
+        return;
+#ifdef CONFIG_VNC
+    } else if (strcmp(protocol, "vnc") == 0) {
+        skipauth = has_skipauth ? skipauth : false;
+        vnc_display_add_client(NULL, fd, skipauth);
+        return;
+#endif
+    } else if ((s = qemu_chr_find(protocol)) != NULL) {
+        if (qemu_chr_add_client(s, fd) < 0) {
+            error_setg(errp, "failed to add client");
+            close(fd);
+            return;
+        }
+        return;
+    }
+
+    error_setg(errp, "protocol '%s' is invalid", protocol);
+    close(fd);
+}
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 09/15] qmp: dump-guest-memory: improve schema doc (again)

[Luiz Capitulino]

 o Add a note about memory allocation with paging=true
 o Fix indentation

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 qapi-schema.json | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 191d921..c6a6767 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2007,26 +2007,33 @@
 # supported on i386 and x86_64.
 #
 # @paging: if true, do paging to get guest's memory mapping. This allows
-# using gdb to process the core file. However, setting @paging to false
-# may be desirable because of two reasons:
+#          using gdb to process the core file.
 #
-#   1. The guest may be in a catastrophic state or can have corrupted
-#      memory, which cannot be trusted
-#   2. The guest can be in real-mode even if paging is enabled. For example,
-#      the guest uses ACPI to sleep, and ACPI sleep state goes in real-mode
+#          IMPORTANT: this option can make QEMU allocate several gigabytes
+#                     of RAM. This can happen for a large guest, or a
+#                     malicious guest pretending to be large.
+#
+#          Also, paging=true has the following limitations:
+#
+#             1. The guest may be in a catastrophic state or can have corrupted
+#                memory, which cannot be trusted
+#             2. The guest can be in real-mode even if paging is enabled. For
+#                example, the guest uses ACPI to sleep, and ACPI sleep state
+#                goes in real-mode
 #
 # @protocol: the filename or file descriptor of the vmcore. The supported
-# protocols are:
+#            protocols are:
 #
-#   1. file: the protocol starts with "file:", and the following string is
-#      the file's path.
-#   2. fd: the protocol starts with "fd:", and the following string is the
-#      fd's name.
+#            1. file: the protocol starts with "file:", and the following
+#               string is the file's path.
+#            2. fd: the protocol starts with "fd:", and the following string
+#               is the fd's name.
 #
 # @begin: #optional if specified, the starting physical address.
 #
 # @length: #optional if specified, the memory size, in bytes. If you don't
-# want to dump all guest's memory, please specify the start @begin and @length
+#          want to dump all guest's memory, please specify the start @begin
+#          and @length
 #
 # Returns: nothing on success
 #
@@ -2035,6 +2042,7 @@
 { 'command': 'dump-guest-memory',
   'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
             '*length': 'int' } }
+
 ##
 # @netdev_add:
 #
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 10/15] qmp: dump-guest-memory: don't spin if non-blocking fd would block

[Luiz Capitulino]

fd_write_vmcore() will indefinitely spin for a non-blocking
file-descriptor that would block. However, if the fd is non-blocking,
how does it make sense to spin?

Change this behavior to return an error instead.

Note that this can only happen with an fd provided by a management
application. The fd opened internally by dump-guest-memory is blocking.

While there, also fix 'writen_size' variable name.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 dump.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/dump.c b/dump.c
index 1a3c716..6b7c127 100644
--- a/dump.c
+++ b/dump.c
@@ -100,18 +100,11 @@ static void dump_error(DumpState *s, const char *reason)
 static int fd_write_vmcore(void *buf, size_t size, void *opaque)
 {
     DumpState *s = opaque;
-    int fd = s->fd;
-    size_t writen_size;
+    size_t written_size;
 
-    /* The fd may be passed from user, and it can be non-blocked */
-    while (size) {
-        writen_size = qemu_write_full(fd, buf, size);
-        if (writen_size != size && errno != EAGAIN) {
-            return -1;
-        }
-
-        buf += writen_size;
-        size -= writen_size;
+    written_size = qemu_write_full(s->fd, buf, size);
+    if (written_size != size) {
+        return -1;
     }
 
     return 0;
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 11/15] hmp: dump-guest-memory: hardcode protocol argument to "file:"

[Luiz Capitulino]

Today, it's necessary to specify the protocol you want to use
when dumping the guest memory, for example:

 (qemu) dump-guest-memory file:/tmp/guest-memory

This has a few issues:

 1. It's cumbersome to type
 2. We loose file path autocompletion
 3. Being able to specify fd:X in HMP makes little sense for humans

Because of these reasons, hardcode the 'protocol' argument to
'file:' in HMP.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hmp-commands.hx | 8 +++-----
 hmp.c           | 8 ++++++--
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed67e99..0302458 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -914,12 +914,11 @@ ETEXI
 #if defined(CONFIG_HAVE_CORE_DUMP)
     {
         .name       = "dump-guest-memory",
-        .args_type  = "paging:-p,protocol:s,begin:i?,length:i?",
-        .params     = "[-p] protocol [begin] [length]",
+        .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
+        .params     = "[-p] filename [begin] [length]",
         .help       = "dump guest memory to file"
                       "\n\t\t\t begin(optional): the starting physical address"
                       "\n\t\t\t length(optional): the memory size, in bytes",
-        .user_print = monitor_user_noop,
         .mhandler.cmd = hmp_dump_guest_memory,
     },
 
@@ -929,8 +928,7 @@ STEXI
 @findex dump-guest-memory
 Dump guest memory to @var{protocol}. The file can be processed with crash or
 gdb.
-  protocol: destination file(started with "file:") or destination file
-            descriptor (started with "fd:")
+  filename: dump file name
     paging: do paging to get guest's memory mapping
      begin: the starting physical address. It's optional, and should be
             specified with length together.
diff --git a/hmp.c b/hmp.c
index ba6fbd3..2de3140 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
     Error *errp = NULL;
     int paging = qdict_get_try_bool(qdict, "paging", 0);
-    const char *file = qdict_get_str(qdict, "protocol");
+    const char *file = qdict_get_str(qdict, "filename");
     bool has_begin = qdict_haskey(qdict, "begin");
     bool has_length = qdict_haskey(qdict, "length");
     int64_t begin = 0;
     int64_t length = 0;
+    char *prot;
 
     if (has_begin) {
         begin = qdict_get_int(qdict, "begin");
@@ -1055,9 +1056,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
         length = qdict_get_int(qdict, "length");
     }
 
-    qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length,
+    prot = g_strconcat("file:", file, NULL);
+
+    qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
                           &errp);
     hmp_handle_error(mon, &errp);
+    g_free(prot);
 }
 
 void hmp_netdev_add(Monitor *mon, const QDict *qdict)
-- 
1.7.12.315.g682ce8b

Re: [Qemu-devel] [PULL 11/15] hmp: dump-guest-memory: hardcode protocol argument to "file:"

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On 09/27/2012 07:28 AM, Luiz Capitulino wrote:
> Today, it's necessary to specify the protocol you want to use
> when dumping the guest memory, for example:
> 
>  (qemu) dump-guest-memory file:/tmp/guest-memory
> 
> This has a few issues:
> 
>  1. It's cumbersome to type
>  2. We loose file path autocompletion

If it's not too late...

s/loose/lose/

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

[Qemu-devel] [PULL 12/15] input: qmp_send_key(): simplify

[Luiz Capitulino]

The current code duplicates the QKeyCodeList keys in order to store
the key values for release_keys() late run. This is a bit complicated
though, as we have to care about correct ordering and then release_keys()
will have to index key_defs[] over again.

Switch to an array of integers, which is dynamically allocated and stores
the already converted key value.

This simplifies the current code and the next commit.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 input.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/input.c b/input.c
index c4b0619..32c6057 100644
--- a/input.c
+++ b/input.c
@@ -224,30 +224,31 @@ int index_from_keycode(int code)
     return i;
 }
 
-static QKeyCodeList *keycodes;
+static int *keycodes;
+static int keycodes_size;
 static QEMUTimer *key_timer;
 
 static void release_keys(void *opaque)
 {
-    int keycode;
-    QKeyCodeList *p;
+    int i;
 
-    for (p = keycodes; p != NULL; p = p->next) {
-        keycode = key_defs[p->value];
-        if (keycode & 0x80) {
+    for (i = 0; i < keycodes_size; i++) {
+        if (keycodes[i] & 0x80) {
             kbd_put_keycode(0xe0);
         }
-        kbd_put_keycode(keycode | 0x80);
+        kbd_put_keycode(keycodes[i]| 0x80);
     }
-    qapi_free_QKeyCodeList(keycodes);
+
+    g_free(keycodes);
     keycodes = NULL;
+    keycodes_size = 0;
 }
 
 void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
                   Error **errp)
 {
     int keycode;
-    QKeyCodeList *p, *keylist, *head = NULL, *tmp = NULL;
+    QKeyCodeList *p;
 
     if (!key_timer) {
         key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
@@ -257,31 +258,22 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
         qemu_del_timer(key_timer);
         release_keys(NULL);
     }
+
     if (!has_hold_time) {
         hold_time = 100;
     }
 
     for (p = keys; p != NULL; p = p->next) {
-        keylist = g_malloc0(sizeof(*keylist));
-        keylist->value = p->value;
-        keylist->next = NULL;
-
-        if (!head) {
-            head = keylist;
-        }
-        if (tmp) {
-            tmp->next = keylist;
-        }
-        tmp = keylist;
-
         /* key down events */
         keycode = key_defs[p->value];
         if (keycode & 0x80) {
             kbd_put_keycode(0xe0);
         }
         kbd_put_keycode(keycode & 0x7f);
+
+        keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1));
+        keycodes[keycodes_size++] = keycode;
     }
-    keycodes = head;
 
     /* delayed key up events */
     qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) +
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes in hex

[Luiz Capitulino]

Before the qapi conversion, the sendkey command could be used to
send key codes in hex directly to the guest. In HMP, this would
be like:

 (qemu) sendkey 0xdc

However, the qapi conversion broke this, as it only supports sending
QKeyCode values to the guest. That's a regression.

This commit fixes the problem by adding hex value support down
the QMP interface, qmp_send_key().

In more detail, this commit:

 1. Adds the KeyValue union. This can represent an hex value or
    a QKeyCode value

 2. *Changes* the QMP send-key command to take an KeyValue argument
    instead of a QKeyCode one

 3. Adapt hmp_send_key() to the QMP interface changes

Item 2 is an incompatible change, but as we're in development phase
(and this command has been merged a few weeks ago) this shouldn't be
a problem.

Finally, it's not possible to split this commit without breaking the
build.

Reported-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hmp.c            | 43 +++++++++++++++++++++++++++++--------------
 input.c          | 33 +++++++++++++++++++++++++++------
 qapi-schema.json | 20 +++++++++++++++++---
 3 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/hmp.c b/hmp.c
index 2de3140..3306bcd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1113,13 +1113,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
 void hmp_send_key(Monitor *mon, const QDict *qdict)
 {
     const char *keys = qdict_get_str(qdict, "keys");
-    QKeyCodeList *keylist, *head = NULL, *tmp = NULL;
+    KeyValueList *keylist, *head = NULL, *tmp = NULL;
     int has_hold_time = qdict_haskey(qdict, "hold-time");
     int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
     Error *err = NULL;
     char keyname_buf[16];
     char *separator;
-    int keyname_len, idx;
+    int keyname_len;
 
     while (1) {
         separator = strchr(keys, '-');
@@ -1133,15 +1133,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
         }
         keyname_buf[keyname_len] = 0;
 
-        idx = index_from_key(keyname_buf);
-        if (idx == Q_KEY_CODE_MAX) {
-            monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
-            break;
-        }
-
         keylist = g_malloc0(sizeof(*keylist));
-        keylist->value = idx;
-        keylist->next = NULL;
+        keylist->value = g_malloc0(sizeof(*keylist->value));
 
         if (!head) {
             head = keylist;
@@ -1151,17 +1144,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
         }
         tmp = keylist;
 
+        if (strstart(keyname_buf, "0x", NULL)) {
+            char *endp;
+            int value = strtoul(keyname_buf, &endp, 0);
+            if (*endp != '\0') {
+                goto err_out;
+            }
+            keylist->value->kind = KEY_VALUE_KIND_NUMBER;
+            keylist->value->number = value;
+        } else {
+            int idx = index_from_key(keyname_buf);
+            if (idx == Q_KEY_CODE_MAX) {
+                goto err_out;
+            }
+            keylist->value->kind = KEY_VALUE_KIND_QCODE;
+            keylist->value->qcode = idx;
+        }
+
         if (!separator) {
             break;
         }
         keys = separator + 1;
     }
 
-    if (idx != Q_KEY_CODE_MAX) {
-        qmp_send_key(head, has_hold_time, hold_time, &err);
-    }
+    qmp_send_key(head, has_hold_time, hold_time, &err);
     hmp_handle_error(mon, &err);
-    qapi_free_QKeyCodeList(head);
+
+out:
+    qapi_free_KeyValueList(head);
+    return;
+
+err_out:
+    monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
+    goto out;
 }
 
 void hmp_screen_dump(Monitor *mon, const QDict *qdict)
diff --git a/input.c b/input.c
index 32c6057..76ade64 100644
--- a/input.c
+++ b/input.c
@@ -228,6 +228,23 @@ static int *keycodes;
 static int keycodes_size;
 static QEMUTimer *key_timer;
 
+static int keycode_from_keyvalue(const KeyValue *value)
+{
+    if (value->kind == KEY_VALUE_KIND_QCODE) {
+        return key_defs[value->qcode];
+    } else {
+        assert(value->kind == KEY_VALUE_KIND_NUMBER);
+        return value->number;
+    }
+}
+
+static void free_keycodes(void)
+{
+    g_free(keycodes);
+    keycodes = NULL;
+    keycodes_size = 0;
+}
+
 static void release_keys(void *opaque)
 {
     int i;
@@ -239,16 +256,14 @@ static void release_keys(void *opaque)
         kbd_put_keycode(keycodes[i]| 0x80);
     }
 
-    g_free(keycodes);
-    keycodes = NULL;
-    keycodes_size = 0;
+    free_keycodes();
 }
 
-void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
+void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time,
                   Error **errp)
 {
     int keycode;
-    QKeyCodeList *p;
+    KeyValueList *p;
 
     if (!key_timer) {
         key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
@@ -265,7 +280,13 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
 
     for (p = keys; p != NULL; p = p->next) {
         /* key down events */
-        keycode = key_defs[p->value];
+        keycode = keycode_from_keyvalue(p->value);
+        if (keycode < 0x01 || keycode > 0xff) {
+            error_setg(errp, "invalid hex keycode 0x%x\n", keycode);
+            free_keycodes();
+            return;
+        }
+
         if (keycode & 0x80) {
             kbd_put_keycode(0xe0);
         }
diff --git a/qapi-schema.json b/qapi-schema.json
index c6a6767..28d8815 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2621,12 +2621,26 @@
              'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
 
 ##
+# @KeyValue
+#
+# Represents a keyboard key.
+#
+# Since: 1.3.0
+##
+{ 'union': 'KeyValue',
+  'data': {
+    'number': 'int',
+    'qcode': 'QKeyCode' } }
+
+##
 # @send-key:
 #
 # Send keys to guest.
 #
-# @keys: key sequence. 'keys' is the name of the key. Use a JSON array to
-#        press several keys simultaneously.
+# @keys: An array of @KeyValue elements. All @KeyValues in this array are
+#        simultaneously sent to the guest. A @KeyValue.number value is sent
+#        directly to the guest, while @KeyValue.qcode must be a valid
+#        @QKeyCode value
 #
 # @hold-time: #optional time to delay key up events, milliseconds. Defaults
 #             to 100
@@ -2638,7 +2652,7 @@
 #
 ##
 { 'command': 'send-key',
-  'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } }
+  'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
 
 ##
 # @screendump:
-- 
1.7.12.315.g682ce8b

Re: [Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes in hex

[Amos Kong]

On 27/09/12 21:28, Luiz Capitulino wrote:
>

Sorry the delay review.

> Before the qapi conversion, the sendkey command could be used to
> send key codes in hex directly to the guest. In HMP, this would
> be like:
>
>   (qemu) sendkey 0xdc

'0xdc' is not a available keycode, I didn't find '0xdc' in old key_defs[]
(before my send-key series, commit: 
8db972cfa469b4e4afd9c65e54e796b83b5ce3a2)


(latest upstream code: 6f8fd2530e9a530f237240daf1c981fa5df7f978)
(qemu) sendkey 0x10
'q' will be inputted to guest
(qemu) sendkey 0x1d-0x38-0xd3
guest will be reset

hex isn't support when using qmp monitor.


> However, the qapi conversion broke this, as it only supports sending
> QKeyCode values to the guest. That's a regression.
>
> This commit fixes the problem by adding hex value support down
> the QMP interface, qmp_send_key().
>
> In more detail, this commit:
>
>   1. Adds the KeyValue union. This can represent an hex value or
>      a QKeyCode value
>
>   2. *Changes* the QMP send-key command to take an KeyValue argument
>      instead of a QKeyCode one
>
>   3. Adapt hmp_send_key() to the QMP interface changes
>
> Item 2 is an incompatible change, but as we're in development phase
> (and this command has been merged a few weeks ago) this shouldn't be
> a problem.
>
> Finally, it's not possible to split this commit without breaking the
> build.

> Reported-by: Avi Kivity <avi@redhat.com>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hmp.c            | 43 +++++++++++++++++++++++++++++--------------
>   input.c          | 33 +++++++++++++++++++++++++++------
>   qapi-schema.json | 20 +++++++++++++++++---
>   3 files changed, 73 insertions(+), 23 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 2de3140..3306bcd 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1113,13 +1113,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
>   void hmp_send_key(Monitor *mon, const QDict *qdict)
>   {
>       const char *keys = qdict_get_str(qdict, "keys");
> -    QKeyCodeList *keylist, *head = NULL, *tmp = NULL;
> +    KeyValueList *keylist, *head = NULL, *tmp = NULL;
>       int has_hold_time = qdict_haskey(qdict, "hold-time");
>       int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
>       Error *err = NULL;
>       char keyname_buf[16];
>       char *separator;
> -    int keyname_len, idx;
> +    int keyname_len;
>
>       while (1) {
>           separator = strchr(keys, '-');
> @@ -1133,15 +1133,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
>           }
>           keyname_buf[keyname_len] = 0;
>
> -        idx = index_from_key(keyname_buf);
> -        if (idx == Q_KEY_CODE_MAX) {
> -            monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
> -            break;
> -        }
> -
>           keylist = g_malloc0(sizeof(*keylist));
> -        keylist->value = idx;
> -        keylist->next = NULL;
> +        keylist->value = g_malloc0(sizeof(*keylist->value));
>
>           if (!head) {
>               head = keylist;
> @@ -1151,17 +1144,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
>           }
>           tmp = keylist;
>
> +        if (strstart(keyname_buf, "0x", NULL)) {
> +            char *endp;
> +            int value = strtoul(keyname_buf, &endp, 0);
> +            if (*endp != '\0') {
> +                goto err_out;
> +            }
> +            keylist->value->kind = KEY_VALUE_KIND_NUMBER;
> +            keylist->value->number = value;
> +        } else {
> +            int idx = index_from_key(keyname_buf);
> +            if (idx == Q_KEY_CODE_MAX) {
> +                goto err_out;
> +            }
> +            keylist->value->kind = KEY_VALUE_KIND_QCODE;
> +            keylist->value->qcode = idx;
> +        }
> +
>           if (!separator) {
>               break;
>           }
>           keys = separator + 1;
>       }
>
> -    if (idx != Q_KEY_CODE_MAX) {
> -        qmp_send_key(head, has_hold_time, hold_time, &err);
> -    }
> +    qmp_send_key(head, has_hold_time, hold_time, &err);
>       hmp_handle_error(mon, &err);
> -    qapi_free_QKeyCodeList(head);
> +
> +out:
> +    qapi_free_KeyValueList(head);
> +    return;
> +
> +err_out:
> +    monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
> +    goto out;
>   }
>
>   void hmp_screen_dump(Monitor *mon, const QDict *qdict)
> diff --git a/input.c b/input.c
> index 32c6057..76ade64 100644
> --- a/input.c
> +++ b/input.c
> @@ -228,6 +228,23 @@ static int *keycodes;
>   static int keycodes_size;
>   static QEMUTimer *key_timer;
>
> +static int keycode_from_keyvalue(const KeyValue *value)
> +{
> +    if (value->kind == KEY_VALUE_KIND_QCODE) {
> +        return key_defs[value->qcode];
> +    } else {
> +        assert(value->kind == KEY_VALUE_KIND_NUMBER);
> +        return value->number;
> +    }
> +}
> +
> +static void free_keycodes(void)
> +{
> +    g_free(keycodes);
> +    keycodes = NULL;
> +    keycodes_size = 0;
> +}
> +
>   static void release_keys(void *opaque)
>   {
>       int i;
> @@ -239,16 +256,14 @@ static void release_keys(void *opaque)
>           kbd_put_keycode(keycodes[i]| 0x80);
>       }
>
> -    g_free(keycodes);
> -    keycodes = NULL;
> -    keycodes_size = 0;
> +    free_keycodes();
>   }
>
> -void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
> +void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time,
>                     Error **errp)
>   {
>       int keycode;
> -    QKeyCodeList *p;
> +    KeyValueList *p;
>
>       if (!key_timer) {
>           key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
> @@ -265,7 +280,13 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
>
>       for (p = keys; p != NULL; p = p->next) {
>           /* key down events */
> -        keycode = key_defs[p->value];
> +        keycode = keycode_from_keyvalue(p->value);
> +        if (keycode < 0x01 || keycode > 0xff) {
> +            error_setg(errp, "invalid hex keycode 0x%x\n", keycode);
> +            free_keycodes();
> +            return;
> +        }
> +
>           if (keycode & 0x80) {
>               kbd_put_keycode(0xe0);
>           }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index c6a6767..28d8815 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2621,12 +2621,26 @@
>                'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
>
>   ##
> +# @KeyValue
> +#
> +# Represents a keyboard key.
> +#
> +# Since: 1.3.0
> +##
> +{ 'union': 'KeyValue',
> +  'data': {
> +    'number': 'int',

It's 'int' not hex format.

==> works with 'int'
{ "execute": "send-key", "arguments": { 'keys': [{'type':'hex', 'data': 
29}, {'type':'hex', 'data': 56}, {'type':'hex', 'data': 211}]}}
{
     "return": {
     }
}
{
     "timestamp": {
         "seconds": 1348826804,
         "microseconds": 777545
     },
     "event": "RESET"
}

==> doesn't work with 'hex'
{ "execute": "send-key", "arguments": { 'keys': [{'type':'hex', 'data': 
0x1d}, {'type':'hex', 'data': 0x38}, {'type':'hex', 'data': 0xd3}]}}
{
     "error": {
         "class": "GenericError",
         "desc": "Invalid JSON syntax"
     }
}


> +    'qcode': 'QKeyCode' } }
> +
> +##
>   # @send-key:
>   #
>   # Send keys to guest.
>   #
> -# @keys: key sequence. 'keys' is the name of the key. Use a JSON array to
> -#        press several keys simultaneously.
> +# @keys: An array of @KeyValue elements. All @KeyValues in this array are
> +#        simultaneously sent to the guest. A @KeyValue.number value is sent
> +#        directly to the guest, while @KeyValue.qcode must be a valid
> +#        @QKeyCode value
>   #
>   # @hold-time: #optional time to delay key up events, milliseconds. Defaults
>   #             to 100
> @@ -2638,7 +2652,7 @@
>   #
>   ##
>   { 'command': 'send-key',
> -  'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } }
> +  'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
>
>   ##
>   # @screendump:


qmp-commands.hx also needs to be updated with latest examples:


>

-- 
			Amos.

Re: [Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes in hex

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

On 09/28/2012 04:36 AM, Amos Kong wrote:
> On 27/09/12 21:28, Luiz Capitulino wrote:
>>
> 
> Sorry the delay review.
> 

> 
> hex isn't support when using qmp monitor.
> 

>> +{ 'union': 'KeyValue',
>> +  'data': {
>> +    'number': 'int',
> 
> It's 'int' not hex format.

Indeed - I just re-read the JSON overview at http://www.json.org/, which
is explicit that numbers are decimal only (no octal or hexadecimal
support, and no support for a leading 0 except when the number is
exactly 0).  [Serves me right for not realizing this aspect of JSON when
I did my review earlier.]

I don't think this invalidates the QMP (libvirt already sends decimal,
and wasn't planning on sending hex), but you DO have a point that:

> 
> qmp-commands.hx also needs to be updated with latest examples:

Since this is already in the PULL request, I think the docs touchup
could be a separate patch.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

Re: [Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes in hex

[Luiz Capitulino]

On Fri, 28 Sep 2012 07:01:31 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 09/28/2012 04:36 AM, Amos Kong wrote:
> > On 27/09/12 21:28, Luiz Capitulino wrote:
> >>
> > 
> > Sorry the delay review.
> > 
> 
> > 
> > hex isn't support when using qmp monitor.
> > 
> 
> >> +{ 'union': 'KeyValue',
> >> +  'data': {
> >> +    'number': 'int',
> > 
> > It's 'int' not hex format.
> 
> Indeed - I just re-read the JSON overview at http://www.json.org/, which
> is explicit that numbers are decimal only (no octal or hexadecimal
> support, and no support for a leading 0 except when the number is
> exactly 0).  [Serves me right for not realizing this aspect of JSON when
> I did my review earlier.]

Sorry if I wasn't clear or if my use of "hex" caused confusion, but
it's known that the only way of having an hex value on the wire is
to make it a string. As the base number we use on the wire is pretty
irrelevant (at least for this case) I don't see any issue here (actually,
for json this is pretty natural).

> I don't think this invalidates the QMP (libvirt already sends decimal,
> and wasn't planning on sending hex), but you DO have a point that:
> 
> > 
> > qmp-commands.hx also needs to be updated with latest examples:
> 
> Since this is already in the PULL request, I think the docs touchup
> could be a separate patch.

Yes.

Re: [Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes in hex

[Luiz Capitulino]

On Fri, 28 Sep 2012 18:36:53 +0800
Amos Kong <akong@redhat.com> wrote:

> On 27/09/12 21:28, Luiz Capitulino wrote:
> >
> 
> Sorry the delay review.
> 
> > Before the qapi conversion, the sendkey command could be used to
> > send key codes in hex directly to the guest. In HMP, this would
> > be like:
> >
> >   (qemu) sendkey 0xdc
> 
> '0xdc' is not a available keycode, I didn't find '0xdc' in old key_defs[]
> (before my send-key series, commit: 
> 8db972cfa469b4e4afd9c65e54e796b83b5ce3a2)

Yes, that's the point of this series. Before the qapi conversion the
sendkey command would send "unknown" key codes such as 0xdc directly
to the guest. The conversion just dropped that feature by rejecting
such key codes.

> (latest upstream code: 6f8fd2530e9a530f237240daf1c981fa5df7f978)
> (qemu) sendkey 0x10
> 'q' will be inputted to guest
> (qemu) sendkey 0x1d-0x38-0xd3
> guest will be reset
> 
> hex isn't support when using qmp monitor.

The HMP monitor is just a UI on top of qmp. It's the qmp interface
that has to be changed in order to support sending key codes
directly to the guest.

> > However, the qapi conversion broke this, as it only supports sending
> > QKeyCode values to the guest. That's a regression.
> >
> > This commit fixes the problem by adding hex value support down
> > the QMP interface, qmp_send_key().
> >
> > In more detail, this commit:
> >
> >   1. Adds the KeyValue union. This can represent an hex value or
> >      a QKeyCode value
> >
> >   2. *Changes* the QMP send-key command to take an KeyValue argument
> >      instead of a QKeyCode one
> >
> >   3. Adapt hmp_send_key() to the QMP interface changes
> >
> > Item 2 is an incompatible change, but as we're in development phase
> > (and this command has been merged a few weeks ago) this shouldn't be
> > a problem.
> >
> > Finally, it's not possible to split this commit without breaking the
> > build.
> 
> > Reported-by: Avi Kivity <avi@redhat.com>
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >   hmp.c            | 43 +++++++++++++++++++++++++++++--------------
> >   input.c          | 33 +++++++++++++++++++++++++++------
> >   qapi-schema.json | 20 +++++++++++++++++---
> >   3 files changed, 73 insertions(+), 23 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index 2de3140..3306bcd 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -1113,13 +1113,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
> >   void hmp_send_key(Monitor *mon, const QDict *qdict)
> >   {
> >       const char *keys = qdict_get_str(qdict, "keys");
> > -    QKeyCodeList *keylist, *head = NULL, *tmp = NULL;
> > +    KeyValueList *keylist, *head = NULL, *tmp = NULL;
> >       int has_hold_time = qdict_haskey(qdict, "hold-time");
> >       int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
> >       Error *err = NULL;
> >       char keyname_buf[16];
> >       char *separator;
> > -    int keyname_len, idx;
> > +    int keyname_len;
> >
> >       while (1) {
> >           separator = strchr(keys, '-');
> > @@ -1133,15 +1133,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
> >           }
> >           keyname_buf[keyname_len] = 0;
> >
> > -        idx = index_from_key(keyname_buf);
> > -        if (idx == Q_KEY_CODE_MAX) {
> > -            monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
> > -            break;
> > -        }
> > -
> >           keylist = g_malloc0(sizeof(*keylist));
> > -        keylist->value = idx;
> > -        keylist->next = NULL;
> > +        keylist->value = g_malloc0(sizeof(*keylist->value));
> >
> >           if (!head) {
> >               head = keylist;
> > @@ -1151,17 +1144,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
> >           }
> >           tmp = keylist;
> >
> > +        if (strstart(keyname_buf, "0x", NULL)) {
> > +            char *endp;
> > +            int value = strtoul(keyname_buf, &endp, 0);
> > +            if (*endp != '\0') {
> > +                goto err_out;
> > +            }
> > +            keylist->value->kind = KEY_VALUE_KIND_NUMBER;
> > +            keylist->value->number = value;
> > +        } else {
> > +            int idx = index_from_key(keyname_buf);
> > +            if (idx == Q_KEY_CODE_MAX) {
> > +                goto err_out;
> > +            }
> > +            keylist->value->kind = KEY_VALUE_KIND_QCODE;
> > +            keylist->value->qcode = idx;
> > +        }
> > +
> >           if (!separator) {
> >               break;
> >           }
> >           keys = separator + 1;
> >       }
> >
> > -    if (idx != Q_KEY_CODE_MAX) {
> > -        qmp_send_key(head, has_hold_time, hold_time, &err);
> > -    }
> > +    qmp_send_key(head, has_hold_time, hold_time, &err);
> >       hmp_handle_error(mon, &err);
> > -    qapi_free_QKeyCodeList(head);
> > +
> > +out:
> > +    qapi_free_KeyValueList(head);
> > +    return;
> > +
> > +err_out:
> > +    monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
> > +    goto out;
> >   }
> >
> >   void hmp_screen_dump(Monitor *mon, const QDict *qdict)
> > diff --git a/input.c b/input.c
> > index 32c6057..76ade64 100644
> > --- a/input.c
> > +++ b/input.c
> > @@ -228,6 +228,23 @@ static int *keycodes;
> >   static int keycodes_size;
> >   static QEMUTimer *key_timer;
> >
> > +static int keycode_from_keyvalue(const KeyValue *value)
> > +{
> > +    if (value->kind == KEY_VALUE_KIND_QCODE) {
> > +        return key_defs[value->qcode];
> > +    } else {
> > +        assert(value->kind == KEY_VALUE_KIND_NUMBER);
> > +        return value->number;
> > +    }
> > +}
> > +
> > +static void free_keycodes(void)
> > +{
> > +    g_free(keycodes);
> > +    keycodes = NULL;
> > +    keycodes_size = 0;
> > +}
> > +
> >   static void release_keys(void *opaque)
> >   {
> >       int i;
> > @@ -239,16 +256,14 @@ static void release_keys(void *opaque)
> >           kbd_put_keycode(keycodes[i]| 0x80);
> >       }
> >
> > -    g_free(keycodes);
> > -    keycodes = NULL;
> > -    keycodes_size = 0;
> > +    free_keycodes();
> >   }
> >
> > -void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
> > +void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time,
> >                     Error **errp)
> >   {
> >       int keycode;
> > -    QKeyCodeList *p;
> > +    KeyValueList *p;
> >
> >       if (!key_timer) {
> >           key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
> > @@ -265,7 +280,13 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
> >
> >       for (p = keys; p != NULL; p = p->next) {
> >           /* key down events */
> > -        keycode = key_defs[p->value];
> > +        keycode = keycode_from_keyvalue(p->value);
> > +        if (keycode < 0x01 || keycode > 0xff) {
> > +            error_setg(errp, "invalid hex keycode 0x%x\n", keycode);
> > +            free_keycodes();
> > +            return;
> > +        }
> > +
> >           if (keycode & 0x80) {
> >               kbd_put_keycode(0xe0);
> >           }
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index c6a6767..28d8815 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -2621,12 +2621,26 @@
> >                'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
> >
> >   ##
> > +# @KeyValue
> > +#
> > +# Represents a keyboard key.
> > +#
> > +# Since: 1.3.0
> > +##
> > +{ 'union': 'KeyValue',
> > +  'data': {
> > +    'number': 'int',
> 
> It's 'int' not hex format.

On the wire we support decimal numbers, and on the C interface
"number" can have any base.

The way I used "hex" to describe this feature might be a bit confusing,
as I mostly had in mind HMP (which does use hex format) and the QMP C
interface (where you usually want to specify "number" in hex).

But that's just a minor terminology confusion, there's absolutely no
changes in the feature's semantics.

> ==> works with 'int'
> { "execute": "send-key", "arguments": { 'keys': [{'type':'hex', 'data': 
> 29}, {'type':'hex', 'data': 56}, {'type':'hex', 'data': 211}]}}
> {
>      "return": {
>      }
> }
> {
>      "timestamp": {
>          "seconds": 1348826804,
>          "microseconds": 777545
>      },
>      "event": "RESET"
> }
> 
> ==> doesn't work with 'hex'
> { "execute": "send-key", "arguments": { 'keys': [{'type':'hex', 'data': 
> 0x1d}, {'type':'hex', 'data': 0x38}, {'type':'hex', 'data': 0xd3}]}}
> {
>      "error": {
>          "class": "GenericError",
>          "desc": "Invalid JSON syntax"
>      }
> }

That's correct, you should use decimal numbers on the wire.

> > +    'qcode': 'QKeyCode' } }
> > +
> > +##
> >   # @send-key:
> >   #
> >   # Send keys to guest.
> >   #
> > -# @keys: key sequence. 'keys' is the name of the key. Use a JSON array to
> > -#        press several keys simultaneously.
> > +# @keys: An array of @KeyValue elements. All @KeyValues in this array are
> > +#        simultaneously sent to the guest. A @KeyValue.number value is sent
> > +#        directly to the guest, while @KeyValue.qcode must be a valid
> > +#        @QKeyCode value
> >   #
> >   # @hold-time: #optional time to delay key up events, milliseconds. Defaults
> >   #             to 100
> > @@ -2638,7 +2652,7 @@
> >   #
> >   ##
> >   { 'command': 'send-key',
> > -  'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } }
> > +  'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
> >
> >   ##
> >   # @screendump:
> 
> 
> qmp-commands.hx also needs to be updated with latest examples:

Good catch, but that can be done on a follow up patch.

Re: [Qemu-devel] [PULL 13/15] qmp: qmp_send_key(): accept key codes in hex

[Amos Kong]

----- Original Message -----
> On Fri, 28 Sep 2012 18:36:53 +0800
> Amos Kong <akong@redhat.com> wrote:
> 
> > On 27/09/12 21:28, Luiz Capitulino wrote:
> > >
> > 
> > Sorry the delay review.
> > 
> > > Before the qapi conversion, the sendkey command could be used to
> > > send key codes in hex directly to the guest. In HMP, this would
> > > be like:
> > >
> > >   (qemu) sendkey 0xdc
> > 
> > '0xdc' is not a available keycode, I didn't find '0xdc' in old
> > key_defs[]
> > (before my send-key series, commit:
> > 8db972cfa469b4e4afd9c65e54e796b83b5ce3a2)
> 
> Yes, that's the point of this series. Before the qapi conversion the
> sendkey command would send "unknown" key codes such as 0xdc directly
> to the guest. The conversion just dropped that feature by rejecting
> such key codes.


Got it.

> > (latest upstream code: 6f8fd2530e9a530f237240daf1c981fa5df7f978)
> > (qemu) sendkey 0x10
> > 'q' will be inputted to guest
> > (qemu) sendkey 0x1d-0x38-0xd3
> > guest will be reset
> > 
> > hex isn't support when using qmp monitor.
> 
> The HMP monitor is just a UI on top of qmp. It's the qmp interface
> that has to be changed in order to support sending key codes
> directly to the guest.
> 
> > > However, the qapi conversion broke this, as it only supports
> > > sending
> > > QKeyCode values to the guest. That's a regression.
> > >
> > > This commit fixes the problem by adding hex value support down
> > > the QMP interface, qmp_send_key().
> > >
> > > In more detail, this commit:
> > >
> > >   1. Adds the KeyValue union. This can represent an hex value or
> > >      a QKeyCode value
> > >
> > >   2. *Changes* the QMP send-key command to take an KeyValue
> > >   argument
> > >      instead of a QKeyCode one
> > >
> > >   3. Adapt hmp_send_key() to the QMP interface changes
> > >
> > > Item 2 is an incompatible change, but as we're in development
> > > phase
> > > (and this command has been merged a few weeks ago) this shouldn't
> > > be
> > > a problem.
> > >
> > > Finally, it's not possible to split this commit without breaking
> > > the
> > > build.
> > 
> > > Reported-by: Avi Kivity <avi@redhat.com>
> > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > > Reviewed-by: Eric Blake <eblake@redhat.com>
> > > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > > ---
> > >   hmp.c            | 43
> > >   +++++++++++++++++++++++++++++--------------
> > >   input.c          | 33 +++++++++++++++++++++++++++------
> > >   qapi-schema.json | 20 +++++++++++++++++---
> > >   3 files changed, 73 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/hmp.c b/hmp.c
> > > index 2de3140..3306bcd 100644
> > > --- a/hmp.c
> > > +++ b/hmp.c
> > > @@ -1113,13 +1113,13 @@ void hmp_closefd(Monitor *mon, const
> > > QDict *qdict)
> > >   void hmp_send_key(Monitor *mon, const QDict *qdict)
> > >   {
> > >       const char *keys = qdict_get_str(qdict, "keys");
> > > -    QKeyCodeList *keylist, *head = NULL, *tmp = NULL;
> > > +    KeyValueList *keylist, *head = NULL, *tmp = NULL;
> > >       int has_hold_time = qdict_haskey(qdict, "hold-time");
> > >       int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
> > >       Error *err = NULL;
> > >       char keyname_buf[16];
> > >       char *separator;
> > > -    int keyname_len, idx;
> > > +    int keyname_len;
> > >
> > >       while (1) {
> > >           separator = strchr(keys, '-');
> > > @@ -1133,15 +1133,8 @@ void hmp_send_key(Monitor *mon, const
> > > QDict *qdict)
> > >           }
> > >           keyname_buf[keyname_len] = 0;
> > >
> > > -        idx = index_from_key(keyname_buf);
> > > -        if (idx == Q_KEY_CODE_MAX) {
> > > -            monitor_printf(mon, "invalid parameter: %s\n",
> > > keyname_buf);
> > > -            break;
> > > -        }
> > > -
> > >           keylist = g_malloc0(sizeof(*keylist));
> > > -        keylist->value = idx;
> > > -        keylist->next = NULL;
> > > +        keylist->value = g_malloc0(sizeof(*keylist->value));
> > >
> > >           if (!head) {
> > >               head = keylist;
> > > @@ -1151,17 +1144,39 @@ void hmp_send_key(Monitor *mon, const
> > > QDict *qdict)
> > >           }
> > >           tmp = keylist;
> > >
> > > +        if (strstart(keyname_buf, "0x", NULL)) {
> > > +            char *endp;
> > > +            int value = strtoul(keyname_buf, &endp, 0);
> > > +            if (*endp != '\0') {
> > > +                goto err_out;
> > > +            }
> > > +            keylist->value->kind = KEY_VALUE_KIND_NUMBER;
> > > +            keylist->value->number = value;
> > > +        } else {
> > > +            int idx = index_from_key(keyname_buf);
> > > +            if (idx == Q_KEY_CODE_MAX) {
> > > +                goto err_out;
> > > +            }
> > > +            keylist->value->kind = KEY_VALUE_KIND_QCODE;
> > > +            keylist->value->qcode = idx;
> > > +        }
> > > +
> > >           if (!separator) {
> > >               break;
> > >           }
> > >           keys = separator + 1;
> > >       }
> > >
> > > -    if (idx != Q_KEY_CODE_MAX) {
> > > -        qmp_send_key(head, has_hold_time, hold_time, &err);
> > > -    }
> > > +    qmp_send_key(head, has_hold_time, hold_time, &err);
> > >       hmp_handle_error(mon, &err);
> > > -    qapi_free_QKeyCodeList(head);
> > > +
> > > +out:
> > > +    qapi_free_KeyValueList(head);
> > > +    return;
> > > +
> > > +err_out:
> > > +    monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
> > > +    goto out;
> > >   }
> > >
> > >   void hmp_screen_dump(Monitor *mon, const QDict *qdict)
> > > diff --git a/input.c b/input.c
> > > index 32c6057..76ade64 100644
> > > --- a/input.c
> > > +++ b/input.c
> > > @@ -228,6 +228,23 @@ static int *keycodes;
> > >   static int keycodes_size;
> > >   static QEMUTimer *key_timer;
> > >
> > > +static int keycode_from_keyvalue(const KeyValue *value)
> > > +{
> > > +    if (value->kind == KEY_VALUE_KIND_QCODE) {
> > > +        return key_defs[value->qcode];
> > > +    } else {
> > > +        assert(value->kind == KEY_VALUE_KIND_NUMBER);
> > > +        return value->number;
> > > +    }
> > > +}
> > > +
> > > +static void free_keycodes(void)
> > > +{
> > > +    g_free(keycodes);
> > > +    keycodes = NULL;
> > > +    keycodes_size = 0;
> > > +}
> > > +
> > >   static void release_keys(void *opaque)
> > >   {
> > >       int i;
> > > @@ -239,16 +256,14 @@ static void release_keys(void *opaque)
> > >           kbd_put_keycode(keycodes[i]| 0x80);
> > >       }
> > >
> > > -    g_free(keycodes);
> > > -    keycodes = NULL;
> > > -    keycodes_size = 0;
> > > +    free_keycodes();
> > >   }
> > >
> > > -void qmp_send_key(QKeyCodeList *keys, bool has_hold_time,
> > > int64_t hold_time,
> > > +void qmp_send_key(KeyValueList *keys, bool has_hold_time,
> > > int64_t hold_time,
> > >                     Error **errp)
> > >   {
> > >       int keycode;
> > > -    QKeyCodeList *p;
> > > +    KeyValueList *p;
> > >
> > >       if (!key_timer) {
> > >           key_timer = qemu_new_timer_ns(vm_clock, release_keys,
> > >           NULL);
> > > @@ -265,7 +280,13 @@ void qmp_send_key(QKeyCodeList *keys, bool
> > > has_hold_time, int64_t hold_time,
> > >
> > >       for (p = keys; p != NULL; p = p->next) {
> > >           /* key down events */
> > > -        keycode = key_defs[p->value];
> > > +        keycode = keycode_from_keyvalue(p->value);
> > > +        if (keycode < 0x01 || keycode > 0xff) {
> > > +            error_setg(errp, "invalid hex keycode 0x%x\n",
> > > keycode);
> > > +            free_keycodes();
> > > +            return;
> > > +        }
> > > +
> > >           if (keycode & 0x80) {
> > >               kbd_put_keycode(0xe0);
> > >           }
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index c6a6767..28d8815 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -2621,12 +2621,26 @@
> > >                'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }
> > >
> > >   ##
> > > +# @KeyValue
> > > +#
> > > +# Represents a keyboard key.
> > > +#
> > > +# Since: 1.3.0
> > > +##
> > > +{ 'union': 'KeyValue',
> > > +  'data': {
> > > +    'number': 'int',
> > 
> > It's 'int' not hex format.
> 
> On the wire we support decimal numbers, and on the C interface
> "number" can have any base.
> 
> The way I used "hex" to describe this feature might be a bit
> confusing,
> as I mostly had in mind HMP (which does use hex format) and the QMP C
> interface (where you usually want to specify "number" in hex).
> 
> But that's just a minor terminology confusion, there's absolutely no
> changes in the feature's semantics.
> 
> > ==> works with 'int'
> > { "execute": "send-key", "arguments": { 'keys': [{'type':'hex',
> > 'data':
> > 29}, {'type':'hex', 'data': 56}, {'type':'hex', 'data': 211}]}}
> > {
> >      "return": {
> >      }
> > }
> > {
> >      "timestamp": {
> >          "seconds": 1348826804,
> >          "microseconds": 777545
> >      },
> >      "event": "RESET"
> > }
> > 
> > ==> doesn't work with 'hex'
> > { "execute": "send-key", "arguments": { 'keys': [{'type':'hex',
> > 'data':
> > 0x1d}, {'type':'hex', 'data': 0x38}, {'type':'hex', 'data':
> > 0xd3}]}}
> > {
> >      "error": {
> >          "class": "GenericError",
> >          "desc": "Invalid JSON syntax"
> >      }
> > }
> 
> That's correct, you should use decimal numbers on the wire.
> 
> > > +    'qcode': 'QKeyCode' } }
> > > +
> > > +##
> > >   # @send-key:
> > >   #
> > >   # Send keys to guest.
> > >   #
> > > -# @keys: key sequence. 'keys' is the name of the key. Use a JSON
> > > array to
> > > -#        press several keys simultaneously.
> > > +# @keys: An array of @KeyValue elements. All @KeyValues in this
> > > array are
> > > +#        simultaneously sent to the guest. A @KeyValue.number
> > > value is sent
> > > +#        directly to the guest, while @KeyValue.qcode must be a
> > > valid
> > > +#        @QKeyCode value
> > >   #
> > >   # @hold-time: #optional time to delay key up events,
> > >   milliseconds. Defaults
> > >   #             to 100
> > > @@ -2638,7 +2652,7 @@
> > >   #
> > >   ##
> > >   { 'command': 'send-key',
> > > -  'data': { 'keys': ['QKeyCode'], '*hold-time': 'int' } }
> > > +  'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
> > >
> > >   ##
> > >   # @screendump:
> > 
> > 
> > qmp-commands.hx also needs to be updated with latest examples:
> 
> Good catch, but that can be done on a follow up patch.

Ok, according to your comments, the only issue is doc.

Acked-by: Amos Kong <akong@redhat.com>

[Qemu-devel] [PULL 14/15] input: index_from_key(): drop unused code

[Luiz Capitulino]

The hex key conversion is unused since last commit.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 input.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/input.c b/input.c
index 76ade64..25d3973 100644
--- a/input.c
+++ b/input.c
@@ -186,8 +186,7 @@ static const int key_defs[] = {
 
 int index_from_key(const char *key)
 {
-    int i, keycode;
-    char *endp;
+    int i;
 
     for (i = 0; QKeyCode_lookup[i] != NULL; i++) {
         if (!strcmp(key, QKeyCode_lookup[i])) {
@@ -195,17 +194,6 @@ int index_from_key(const char *key)
         }
     }
 
-    if (strstart(key, "0x", NULL)) {
-        keycode = strtoul(key, &endp, 0);
-        if (*endp == '\0' && keycode >= 0x01 && keycode <= 0xff) {
-            for (i = 0; i < Q_KEY_CODE_MAX; i++) {
-                if (keycode == key_defs[i]) {
-                    break;
-                }
-            }
-        }
-    }
-
     /* Return Q_KEY_CODE_MAX if the key is invalid */
     return i;
 }
-- 
1.7.12.315.g682ce8b

[Qemu-devel] [PULL 15/15] block: live snapshot documentation tweaks

[Luiz Capitulino]

From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qapi-schema.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 28d8815..f4c2185 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1399,7 +1399,7 @@
 # @format: #optional the format of the snapshot image, default is 'qcow2'.
 #
 # @mode: #optional whether and how QEMU should create a new image, default is
-# 'absolute-paths'.
+#        'absolute-paths'.
 ##
 { 'type': 'BlockdevSnapshot',
   'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
@@ -1453,7 +1453,7 @@
 # @format: #optional the format of the snapshot image, default is 'qcow2'.
 #
 # @mode: #optional whether and how QEMU should create a new image, default is
-# 'absolute-paths'.
+#        'absolute-paths'.
 #
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound
-- 
1.7.12.315.g682ce8b

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

[Anthony Liguori]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> The changes (since ac05f3492421caeb05809ffa02c6198ede179e43) are available
> in the following repository:
>
>     git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
>

Pulled. Thanks.

Regards,

Anthony Liguori

> Luiz Capitulino (7):
>   qapi: convert add_client
>   qmp: dump-guest-memory: improve schema doc (again)
>   qmp: dump-guest-memory: don't spin if non-blocking fd would block
>   hmp: dump-guest-memory: hardcode protocol argument to "file:"
>   input: qmp_send_key(): simplify
>   qmp: qmp_send_key(): accept key codes in hex
>   input: index_from_key(): drop unused code
>
> Paolo Bonzini (5):
>   qapi: do not protect enum values from namespace pollution
>   qapi: add "unix" to the set of reserved words
>   pci-assign: use monitor_handle_fd_param
>   monitor: add Error * argument to monitor_get_fd
>   block: live snapshot documentation tweaks
>
> Ryota Ozaki (3):
>   Make negotiation optional in QEMUMonitorProtocol
>   Support settimeout in QEMUMonitorProtocol
>   Add qemu-ga-client script
>
>  QMP/qemu-ga-client    | 299 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  QMP/qmp.py            |  12 +-
>  dump.c                |  18 +--
>  hmp-commands.hx       |   8 +-
>  hmp.c                 |  51 ++++++---
>  hw/kvm/pci-assign.c   |  12 +-
>  input.c               |  75 ++++++-------
>  migration-fd.c        |   2 +-
>  monitor.c             |  48 +-------
>  monitor.h             |   2 +-
>  qapi-schema.json      |  81 +++++++++++---
>  qmp-commands.hx       |   5 +-
>  qmp.c                 |  43 ++++++++
>  scripts/qapi-types.py |   4 +-
>  scripts/qapi-visit.py |   2 +-
>  scripts/qapi.py       |  10 +-
>  16 files changed, 517 insertions(+), 155 deletions(-)
>  create mode 100755 QMP/qemu-ga-client
>
> -- 
> 1.7.12.315.g682ce8b