[Qemu-devel] [PATCH 00/12] QAPI prerequisites for the embedded NBD server

[Qemu-devel] [PATCH 00/12] QAPI prerequisites for the embedded NBD server

[Paolo Bonzini]

Dan's request for file descriptor passing in the embedded NBD server opened
a nice can of worms. :)

This is because there is no existing QAPI description of socket addresses
and similar things, plus QAPI-fication of qemu-sockets.c is a bit sparse.

This series tries to fix this.  It includes three extra patches at the
end that actually implement the NBD server and serve as an example of
the new socket creation APIs.  I would ask for your review of patches
14 and 15 anyway, so I don't mind if you take these three as well; I
terminated the series officially at patch 12 just because these three
require the changes from my outstanding NBD pull request.

Patches 1 and 2 add error propagation to named file descriptors.  These
should go in as a prerequisite to the QAPI conversion of add_client.

Patches 3 and 4 are small fixes to the QAPI scripts.  Nothing spectacular.

Patches 5 to 10 start moving qemu-sockets functions away from error_report
(or printf) and away from QemuOpts.  And after patch 11 harmonizes a
bit the Unix-socket functions, patch 12 introduces the new socket API.

Please review!

Paolo Bonzini (12):
  monitor: use monitor_handle_fd_param for non-Error-friendly users of named fds
  monitor: add Error * argument to monitor_get_fd
  qapi: do not protect enum values from namespace pollution
  qapi: add "unix" to the set of reserved words
  build: add QAPI files to the tools
  qapi: add socket address types
  qemu-sockets: add error propagation to inet_parse
  qemu-sockets: add error propagation to Unix socket functions
  qemu-sockets: return IPSocketAddress from inet_parse
  qemu-sockets: move block from QemuOpts to arguments
  qemu-sockets: add block and in_progress arguments to unix_connect_opts
  qemu-sockets: add socket_listen, socket_connect, socket_parse

 Makefile.objs         |   3 +-
 dump.c                |   5 +-
 hw/kvm/pci-assign.c   |   4 +-
 migration-fd.c        |   3 +-
 monitor.c             |  15 +--
 monitor.h             |   2 +-
 nbd.c                 |   4 +-
 qapi-schema.json      |  53 +++++++++
 qemu-char.c           |   6 +-
 qemu-sockets.c        | 289 ++++++++++++++++++++++++++++++++++++++------------
 qemu-tool.c           |   6 ++
 qemu_socket.h         |  14 ++-
 qga/channel-posix.c   |   2 +-
 scripts/qapi-types.py |   4 +-
 scripts/qapi-visit.py |   2 +-
 scripts/qapi.py       |  10 +-
 ui/vnc.c              |   4 +-
 17 file modificati, 321 inserzioni(+), 105 rimozioni(-)

-- 
1.7.12

[Qemu-devel] [PATCH 01/12] monitor: use monitor_handle_fd_param for non-Error-friendly users of named fds

[Paolo Bonzini]

monitor_handle_fd_param and monitor_get_fd are mostly the same, except
that monitor_handle_fd_param does error reporting wrong.  Use it in all
other places that do it wrong, instead of reinventing it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/kvm/pci-assign.c | 4 +---
 migration-fd.c      | 3 +--
 monitor.c           | 6 +++---
 3 file modificati, 5 inserzioni(+), 8 rimozioni(-)

diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
index 05b93d9..8b96a57 100644
--- a/hw/kvm/pci-assign.c
+++ b/hw/kvm/pci-assign.c
@@ -582,10 +582,8 @@ static int get_real_device(AssignedDevice *pci_dev, uint16_t r_seg,
         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);
+            dev->config_fd = monitor_handle_fd_param(cur_mon, pci_dev->configfd_name);
             if (dev->config_fd < 0) {
-                error_report("%s: (%s) unkown", __func__,
-                             pci_dev->configfd_name);
                 return 1;
             }
         }
diff --git a/migration-fd.c b/migration-fd.c
index 50138ed..3eb53d9 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -75,9 +75,8 @@ 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_handle_fd_param(cur_mon, fdname);
     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..4901600 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_handle_fd_param(mon, fdname);
         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_handle_fd_param(mon, fdname);
         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_handle_fd_param(mon, fdname);
 	if (qemu_chr_add_client(s, fd) < 0) {
 	    qerror_report(QERR_ADD_CLIENT_FAILED);
 	    return -1;
-- 
1.7.12

[Qemu-devel] [PATCH 02/12] monitor: add Error * argument to monitor_get_fd

[Paolo Bonzini]

Differentiate monitor_get_fd and monitor_handle_fd_param by doing
correct error propagation in the former and its callers.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 dump.c    | 5 +++--
 monitor.c | 9 ++++++---
 monitor.h | 2 +-
 3 file modificati, 10 inserzioni(+), 6 rimozioni(-)

diff --git a/dump.c b/dump.c
index 2bf8d8d..cc7aecd 100644
--- a/dump.c
+++ b/dump.c
@@ -824,6 +824,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
     int fd = -1;
     DumpState *s;
     int ret;
+    Error *local_err = NULL;
 
     if (has_begin && !has_length) {
         error_set(errp, QERR_MISSING_PARAMETER, "length");
@@ -836,9 +837,9 @@ 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, &local_err);
         if (fd == -1) {
-            error_set(errp, QERR_FD_NOT_FOUND, p);
+            error_propagate(errp, local_err);
             return;
         }
     }
diff --git a/monitor.c b/monitor.c
index 4901600..b73ae57 100644
--- a/monitor.c
+++ b/monitor.c
@@ -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_set(errp, QERR_FD_NOT_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

[Qemu-devel] [PATCH 03/12] qapi: do not protect enum values from namespace pollution

[Paolo Bonzini]

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>
---
 scripts/qapi-types.py | 4 ++--
 scripts/qapi-visit.py | 2 +-
 scripts/qapi.py       | 8 ++++----
 3 file modificati, 7 inserzioni(+), 7 rimozioni(-)

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

[Qemu-devel] [PATCH 04/12] qapi: add "unix" to the set of reserved words

[Paolo Bonzini]

It is #defined to 1.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/qapi.py | 4 +++-
 1 file modificato, 3 inserzioni(+). 1 rimozione(-)

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

[Qemu-devel] [PATCH 05/12] build: add QAPI files to the tools

[Paolo Bonzini]

We need them because qemu-sockets will soon be using SocketAddress.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs | 3 ++-
 1 file modificato, 2 inserzioni(+). 1 rimozione(-)

diff --git a/Makefile.objs b/Makefile.objs
index 4412757..03da150 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -47,6 +47,7 @@ block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 block-obj-y += block/
+block-obj-y += $(qapi-obj-y) qapi-types.o qapi-visit.o
 
 ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
 # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
@@ -241,9 +242,9 @@ QEMU_CFLAGS+=$(GLIB_CFLAGS)
 nested-vars += \
 	hw-obj-y \
 	qga-obj-y \
-	block-obj-y \
 	qom-obj-y \
 	qapi-obj-y \
+	block-obj-y \
 	user-obj-y \
 	common-obj-y \
 	extra-obj-y
-- 
1.7.12

[Qemu-devel] [PATCH 06/12] qapi: add socket address types

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi-schema.json | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file modificato, 53 inserzioni(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index a9f465a..1588372 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2278,6 +2278,59 @@
     'opts': 'NetClientOptions' } }
 
 ##
+# @IPSocketAddress
+#
+# Captures the destination address of an IP socket
+#
+# @host: host part of the address
+#
+# @port: port part of the address, or lowest port if @to is present
+#
+# @to: highest port to try
+#
+# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6
+#        #optional
+#
+# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6
+#        #optional
+#
+# Since 1.3
+##
+{ 'type': 'IPSocketAddress',
+  'data': {
+    'host': 'str',
+    'port': 'str',
+    '*to': 'uint16',
+    '*ipv4': 'bool',
+    '*ipv6': 'bool' } }
+
+##
+# @UnixSocketAddress
+#
+# Captures the destination address of a Unix socket
+#
+# @path: filesystem path to use
+#
+# Since 1.3
+##
+{ 'type': 'UnixSocketAddress',
+  'data': {
+    'path': 'str' } }
+
+##
+# @SocketAddress
+#
+# Captures the address of a socket, which could also be a named file descriptor
+#
+# Since 1.3
+##
+{ 'union': 'SocketAddress',
+  'data': {
+    'inet': 'IPSocketAddress',
+    'unix': 'UnixSocketAddress',
+    'fd': 'String' } }
+
+##
 # @getfd:
 #
 # Receive a file descriptor via SCM rights and assign it a name
-- 
1.7.12

[Qemu-devel] [PATCH 07/12] qemu-sockets: add error propagation to inet_parse

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 35 +++++++++++++++++------------------
 1 file modificato, 17 inserzioni(+), 18 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 361d890..5942aef 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -407,7 +407,7 @@ err:
 }
 
 /* compatibility wrapper */
-static int inet_parse(QemuOpts *opts, const char *str)
+static void inet_parse(QemuOpts *opts, const char *str, Error **errp)
 {
     const char *optstr, *h;
     char addr[64];
@@ -419,32 +419,28 @@ static int inet_parse(QemuOpts *opts, const char *str)
         /* no host given */
         addr[0] = '\0';
         if (1 != sscanf(str,":%32[^,]%n",port,&pos)) {
-            fprintf(stderr, "%s: portonly parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing port in address '%s'", str);
+            return;
         }
     } else if (str[0] == '[') {
         /* IPv6 addr */
         if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",addr,port,&pos)) {
-            fprintf(stderr, "%s: ipv6 parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing IPv6 address '%s'", str);
+            return;
         }
         qemu_opt_set(opts, "ipv6", "on");
     } else if (qemu_isdigit(str[0])) {
         /* IPv4 addr */
         if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",addr,port,&pos)) {
-            fprintf(stderr, "%s: ipv4 parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing IPv4 address '%s'", str);
+            return;
         }
         qemu_opt_set(opts, "ipv4", "on");
     } else {
         /* hostname */
         if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) {
-            fprintf(stderr, "%s: hostname parse error (%s)\n",
-                    __FUNCTION__, str);
-            return -1;
+            error_setg(errp, "error parsing address '%s'", str);
+            return;
         }
     }
     qemu_opt_set(opts, "host", addr);
@@ -459,7 +455,6 @@ static int inet_parse(QemuOpts *opts, const char *str)
         qemu_opt_set(opts, "ipv4", "on");
     if (strstr(optstr, ",ipv6"))
         qemu_opt_set(opts, "ipv6", "on");
-    return 0;
 }
 
 int inet_listen(const char *str, char *ostr, int olen,
@@ -468,9 +463,11 @@ int inet_listen(const char *str, char *ostr, int olen,
     QemuOpts *opts;
     char *optstr;
     int sock = -1;
+    Error *local_err = NULL;
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    if (inet_parse(opts, str) == 0) {
+    inet_parse(opts, str, &local_err);
+    if (local_err == NULL) {
         sock = inet_listen_opts(opts, port_offset, errp);
         if (sock != -1 && ostr) {
             optstr = strchr(str, ',');
@@ -487,7 +484,7 @@ int inet_listen(const char *str, char *ostr, int olen,
             }
         }
     } else {
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_propagate(errp, local_err);
     }
     qemu_opts_del(opts);
     return sock;
@@ -497,15 +494,17 @@ int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
 {
     QemuOpts *opts;
     int sock = -1;
+    Error *local_err = NULL;
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    if (inet_parse(opts, str) == 0) {
+    inet_parse(opts, str, &local_err);
+    if (local_err == NULL) {
         if (block) {
             qemu_opt_set(opts, "block", "on");
         }
         sock = inet_connect_opts(opts, in_progress, errp);
     } else {
-        error_set(errp, QERR_SOCKET_CREATE_FAILED);
+        error_propagate(errp, local_err);
     }
     qemu_opts_del(opts);
     return sock;
-- 
1.7.12

[Qemu-devel] [PATCH 08/12] qemu-sockets: add error propagation to Unix socket functions

[Paolo Bonzini]

This patch starts harmonizing unix_* and inet_* functions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c               |  4 ++--
 qemu-char.c         |  4 ++--
 qemu-sockets.c      | 40 ++++++++++++++++++++--------------------
 qemu_socket.h       |  8 ++++----
 qga/channel-posix.c |  2 +-
 ui/vnc.c            |  4 ++--
 6 file modificati, 31 inserzioni(+), 31 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 57edfde..6b0d2a3 100644
--- a/nbd.c
+++ b/nbd.c
@@ -230,12 +230,12 @@ int unix_socket_incoming(const char *path)
     char *ostr = NULL;
     int olen = 0;
 
-    return unix_listen(path, ostr, olen);
+    return unix_listen(path, ostr, olen, NULL);
 }
 
 int unix_socket_outgoing(const char *path)
 {
-    return unix_connect(path);
+    return unix_connect(path, NULL);
 }
 
 /* Basic flow for negotiation
diff --git a/qemu-char.c b/qemu-char.c
index 767da93..b886f3b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2451,9 +2451,9 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
 
     if (is_unix) {
         if (is_listen) {
-            fd = unix_listen_opts(opts);
+            fd = unix_listen_opts(opts, NULL);
         } else {
-            fd = unix_connect_opts(opts);
+            fd = unix_connect_opts(opts, NULL);
         }
     } else {
         if (is_listen) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 5942aef..2da1602 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -512,7 +512,7 @@ int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
 
 #ifndef _WIN32
 
-int unix_listen_opts(QemuOpts *opts)
+int unix_listen_opts(QemuOpts *opts, Error **errp)
 {
     struct sockaddr_un un;
     const char *path = qemu_opt_get(opts, "path");
@@ -520,7 +520,7 @@ int unix_listen_opts(QemuOpts *opts)
 
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
-        perror("socket(unix)");
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
 
@@ -545,11 +545,11 @@ int unix_listen_opts(QemuOpts *opts)
 
     unlink(un.sun_path);
     if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
-        fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, strerror(errno));
+        error_set(errp, QERR_SOCKET_BIND_FAILED);
         goto err;
     }
     if (listen(sock, 1) < 0) {
-        fprintf(stderr, "listen(unix:%s): %s\n", un.sun_path, strerror(errno));
+        error_set(errp, QERR_SOCKET_LISTEN_FAILED);
         goto err;
     }
 
@@ -560,20 +560,20 @@ err:
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts)
+int unix_connect_opts(QemuOpts *opts, Error **errp)
 {
     struct sockaddr_un un;
     const char *path = qemu_opt_get(opts, "path");
     int sock;
 
     if (NULL == path) {
-        fprintf(stderr, "unix connect: no path specified\n");
+        error_setg(errp, "unix connect: no path specified\n");
         return -1;
     }
 
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
-        perror("socket(unix)");
+        error_set(errp, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
 
@@ -581,7 +581,7 @@ int unix_connect_opts(QemuOpts *opts)
     un.sun_family = AF_UNIX;
     snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
     if (connect(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
-        fprintf(stderr, "connect(unix:%s): %s\n", path, strerror(errno));
+        error_set(errp, QERR_SOCKET_CONNECT_FAILED);
         close(sock);
 	return -1;
     }
@@ -590,7 +590,7 @@ int unix_connect_opts(QemuOpts *opts)
 }
 
 /* compatibility wrapper */
-int unix_listen(const char *str, char *ostr, int olen)
+int unix_listen(const char *str, char *ostr, int olen, Error **errp)
 {
     QemuOpts *opts;
     char *path, *optstr;
@@ -611,7 +611,7 @@ int unix_listen(const char *str, char *ostr, int olen)
         qemu_opt_set(opts, "path", str);
     }
 
-    sock = unix_listen_opts(opts);
+    sock = unix_listen_opts(opts, errp);
 
     if (sock != -1 && ostr)
         snprintf(ostr, olen, "%s%s", qemu_opt_get(opts, "path"), optstr ? optstr : "");
@@ -619,44 +619,44 @@ int unix_listen(const char *str, char *ostr, int olen)
     return sock;
 }
 
-int unix_connect(const char *path)
+int unix_connect(const char *path, Error **errp)
 {
     QemuOpts *opts;
     int sock;
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
     qemu_opt_set(opts, "path", path);
-    sock = unix_connect_opts(opts);
+    sock = unix_connect_opts(opts, errp);
     qemu_opts_del(opts);
     return sock;
 }
 
 #else
 
-int unix_listen_opts(QemuOpts *opts)
+int unix_listen_opts(QemuOpts *opts, Error **errp)
 {
-    fprintf(stderr, "unix sockets are not available on windows\n");
+    error_setg(errp, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts)
+int unix_connect_opts(QemuOpts *opts, Error **errp)
 {
-    fprintf(stderr, "unix sockets are not available on windows\n");
+    error_setg(errp, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
     return -1;
 }
 
-int unix_listen(const char *path, char *ostr, int olen)
+int unix_listen(const char *path, char *ostr, int olen, Error **errp)
 {
-    fprintf(stderr, "unix sockets are not available on windows\n");
+    error_setg(errp, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
     return -1;
 }
 
-int unix_connect(const char *path)
+int unix_connect(const char *path, Error **errp)
 {
-    fprintf(stderr, "unix sockets are not available on windows\n");
+    error_setg(errp, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
     return -1;
 }
diff --git a/qemu_socket.h b/qemu_socket.h
index 30ae6af..6e06982 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -47,10 +47,10 @@ int inet_connect(const char *str, bool block, bool *in_progress, Error **errp);
 int inet_dgram_opts(QemuOpts *opts);
 const char *inet_strfamily(int family);
 
-int unix_listen_opts(QemuOpts *opts);
-int unix_listen(const char *path, char *ostr, int olen);
-int unix_connect_opts(QemuOpts *opts);
-int unix_connect(const char *path);
+int unix_listen_opts(QemuOpts *opts, Error **errp);
+int unix_listen(const char *path, char *ostr, int olen, Error **errp);
+int unix_connect_opts(QemuOpts *opts, Error **errp);
+int unix_connect(const char *path, Error **errp);
 
 /* Old, ipv4 only bits.  Don't use for new code. */
 int parse_host_port(struct sockaddr_in *saddr, const char *str);
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 57eea06..e22eee6 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -181,7 +181,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod
         break;
     }
     case GA_CHANNEL_UNIX_LISTEN: {
-        int fd = unix_listen(path, NULL, strlen(path));
+        int fd = unix_listen(path, NULL, strlen(path), NULL);
         if (fd == -1) {
             g_critical("error opening path: %s", strerror(errno));
             return false;
diff --git a/ui/vnc.c b/ui/vnc.c
index 385e345..3b3a6ef 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3059,7 +3059,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
     if (reverse) {
         /* connect to viewer */
         if (strncmp(display, "unix:", 5) == 0)
-            vs->lsock = unix_connect(display+5);
+            vs->lsock = unix_connect(display+5, NULL);
         else
             vs->lsock = inet_connect(display, true, NULL, NULL);
         if (-1 == vs->lsock) {
@@ -3079,7 +3079,7 @@ int vnc_display_open(DisplayState *ds, const char *display)
         dpy = g_malloc(256);
         if (strncmp(display, "unix:", 5) == 0) {
             pstrcpy(dpy, 256, "unix:");
-            vs->lsock = unix_listen(display+5, dpy+5, 256-5);
+            vs->lsock = unix_listen(display+5, dpy+5, 256-5, NULL);
         } else {
             vs->lsock = inet_listen(display, dpy, 256,
                                     SOCK_STREAM, 5900, NULL);
-- 
1.7.12

[Qemu-devel] [PATCH 09/12] qemu-sockets: return IPSocketAddress from inet_parse

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 103 ++++++++++++++++++++++++++++++++++++++-------------------
 1 file modificato, 69 inserzioni(+), 34 rimozioni(-)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 2da1602..c213cc8 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -407,54 +407,91 @@ err:
 }
 
 /* compatibility wrapper */
-static void inet_parse(QemuOpts *opts, const char *str, Error **errp)
+static IPSocketAddress *inet_parse(const char *str, Error **errp)
 {
+    IPSocketAddress *addr;
     const char *optstr, *h;
-    char addr[64];
+    char host[64];
     char port[33];
+    int to;
     int pos;
 
+    addr = g_new(IPSocketAddress, 1);
+
     /* parse address */
     if (str[0] == ':') {
         /* no host given */
-        addr[0] = '\0';
+        host[0] = '\0';
         if (1 != sscanf(str,":%32[^,]%n",port,&pos)) {
             error_setg(errp, "error parsing port in address '%s'", str);
-            return;
+            goto fail;
         }
     } else if (str[0] == '[') {
         /* IPv6 addr */
-        if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",addr,port,&pos)) {
+        if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",host,port,&pos)) {
             error_setg(errp, "error parsing IPv6 address '%s'", str);
-            return;
+            goto fail;
         }
-        qemu_opt_set(opts, "ipv6", "on");
+        addr->ipv6 = addr->has_ipv6 = true;
     } else if (qemu_isdigit(str[0])) {
         /* IPv4 addr */
-        if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",addr,port,&pos)) {
+        if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",host,port,&pos)) {
             error_setg(errp, "error parsing IPv4 address '%s'", str);
-            return;
+            goto fail;
         }
-        qemu_opt_set(opts, "ipv4", "on");
+        addr->ipv4 = addr->has_ipv4 = true;
     } else {
         /* hostname */
-        if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) {
+        if (2 != sscanf(str,"%64[^:]:%32[^,]%n",host,port,&pos)) {
             error_setg(errp, "error parsing address '%s'", str);
-            return;
+            goto fail;
         }
     }
-    qemu_opt_set(opts, "host", addr);
-    qemu_opt_set(opts, "port", port);
+
+    addr->host = g_strdup(host);
+    addr->port = g_strdup(port);
 
     /* parse options */
     optstr = str + pos;
     h = strstr(optstr, ",to=");
-    if (h)
-        qemu_opt_set(opts, "to", h+4);
-    if (strstr(optstr, ",ipv4"))
-        qemu_opt_set(opts, "ipv4", "on");
-    if (strstr(optstr, ",ipv6"))
-        qemu_opt_set(opts, "ipv6", "on");
+    if (h) {
+        if (1 != sscanf(str, "%d%n", &to, &pos) ||
+            (str[pos] != '\0' && str[pos] != ',')) {
+            error_setg(errp, "error parsing to= argument");
+            goto fail;
+        }
+        addr->has_to = true;
+        addr->to = to;
+    }
+    if (strstr(optstr, ",ipv4")) {
+        addr->ipv4 = addr->has_ipv4 = true;
+    }
+    if (strstr(optstr, ",ipv6")) {
+        addr->ipv6 = addr->has_ipv6 = true;
+    }
+    return addr;
+
+fail:
+    qapi_free_IPSocketAddress(addr);
+    return NULL;
+}
+
+static void inet_addr_to_opts(QemuOpts *opts, IPSocketAddress *addr)
+{
+    bool ipv4 = addr->ipv4 || !addr->has_ipv4;
+    bool ipv6 = addr->ipv6 || !addr->has_ipv6;
+
+    if (!ipv4 || !ipv6) {
+        qemu_opt_set_bool(opts, "ipv4", ipv4);
+        qemu_opt_set_bool(opts, "ipv6", ipv6);
+    }
+    if (addr->has_to) {
+        char to[20];
+        snprintf(to, sizeof(to), "%d", addr->to);
+        qemu_opt_set(opts, "to", to);
+    }
+    qemu_opt_set(opts, "host", addr->host);
+    qemu_opt_set(opts, "port", addr->port);
 }
 
 int inet_listen(const char *str, char *ostr, int olen,
@@ -463,11 +498,13 @@ int inet_listen(const char *str, char *ostr, int olen,
     QemuOpts *opts;
     char *optstr;
     int sock = -1;
-    Error *local_err = NULL;
+    IPSocketAddress *addr;
 
-    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    inet_parse(opts, str, &local_err);
-    if (local_err == NULL) {
+    addr = inet_parse(str, errp);
+    if (addr != NULL) {
+        opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+        inet_addr_to_opts(opts, addr);
+        qapi_free_IPSocketAddress(addr);
         sock = inet_listen_opts(opts, port_offset, errp);
         if (sock != -1 && ostr) {
             optstr = strchr(str, ',');
@@ -483,10 +521,8 @@ int inet_listen(const char *str, char *ostr, int olen,
                          optstr ? optstr : "");
             }
         }
-    } else {
-        error_propagate(errp, local_err);
+        qemu_opts_del(opts);
     }
-    qemu_opts_del(opts);
     return sock;
 }
 
@@ -494,19 +529,19 @@ int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
 {
     QemuOpts *opts;
     int sock = -1;
-    Error *local_err = NULL;
+    IPSocketAddress *addr;
 
-    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
-    inet_parse(opts, str, &local_err);
-    if (local_err == NULL) {
+    addr = inet_parse(str, errp);
+    if (addr != NULL) {
+        opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+        inet_addr_to_opts(opts, addr);
+        qapi_free_IPSocketAddress(addr);
         if (block) {
             qemu_opt_set(opts, "block", "on");
         }
         sock = inet_connect_opts(opts, in_progress, errp);
-    } else {
-        error_propagate(errp, local_err);
+        qemu_opts_del(opts);
     }
-    qemu_opts_del(opts);
     return sock;
 }
 
-- 
1.7.12

[Qemu-devel] [PATCH 10/12] qemu-sockets: move block from QemuOpts to arguments

[Paolo Bonzini]

This option is not accessible from anywhere in the command-line, so it
need not be a QemuOpt.  Make it a separate argument, which will be the
same convention used by socket_connect.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c    |  2 +-
 qemu-sockets.c | 12 ++----------
 qemu_socket.h  |  2 +-
 3 file modificati, 4 inserzioni(+), 12 rimozioni(-)

diff --git a/qemu-char.c b/qemu-char.c
index b886f3b..b33bdaa 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2459,7 +2459,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
         if (is_listen) {
             fd = inet_listen_opts(opts, 0, NULL);
         } else {
-            fd = inet_connect_opts(opts, NULL, NULL);
+            fd = inet_connect_opts(opts, false, NULL, NULL);
         }
     }
     if (fd < 0) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index c213cc8..8a7c5d4 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -54,9 +54,6 @@ static QemuOptsList dummy_opts = {
         },{
             .name = "ipv6",
             .type = QEMU_OPT_BOOL,
-        },{
-            .name = "block",
-            .type = QEMU_OPT_BOOL,
         },
         { /* end if list */ }
     },
@@ -209,7 +206,7 @@ listen:
     return slisten;
 }
 
-int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
+int inet_connect_opts(QemuOpts *opts, bool block, bool *in_progress, Error **errp)
 {
     struct addrinfo ai,*res,*e;
     const char *addr;
@@ -217,7 +214,6 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
     char uaddr[INET6_ADDRSTRLEN+1];
     char uport[33];
     int sock,rc;
-    bool block;
 
     memset(&ai,0, sizeof(ai));
     ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
@@ -230,7 +226,6 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
 
     addr = qemu_opt_get(opts, "host");
     port = qemu_opt_get(opts, "port");
-    block = qemu_opt_get_bool(opts, "block", 0);
     if (addr == NULL || port == NULL) {
         fprintf(stderr, "inet_connect: host and/or port not specified\n");
         error_set(errp, QERR_SOCKET_CREATE_FAILED);
@@ -536,10 +531,7 @@ int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
         opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
         inet_addr_to_opts(opts, addr);
         qapi_free_IPSocketAddress(addr);
-        if (block) {
-            qemu_opt_set(opts, "block", "on");
-        }
-        sock = inet_connect_opts(opts, in_progress, errp);
+        sock = inet_connect_opts(opts, block, in_progress, errp);
         qemu_opts_del(opts);
     }
     return sock;
diff --git a/qemu_socket.h b/qemu_socket.h
index 6e06982..ad9e342 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -42,7 +42,7 @@ int send_all(int fd, const void *buf, int len1);
 int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
 int inet_listen(const char *str, char *ostr, int olen,
                 int socktype, int port_offset, Error **errp);
-int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp);
+int inet_connect_opts(QemuOpts *opts, bool block, bool *in_progress, Error **errp);
 int inet_connect(const char *str, bool block, bool *in_progress, Error **errp);
 int inet_dgram_opts(QemuOpts *opts);
 const char *inet_strfamily(int family);
-- 
1.7.12

[Qemu-devel] [PATCH 11/12] qemu-sockets: add block and in_progress arguments to unix_connect_opts

[Paolo Bonzini]

Make unix_connect_opts have the same signature as inet_connect_opts.
This is required because both will be wrapped by socket_connect.

The logic is the same as inet_connect_opts, and qemu-char is already
able to use the new functionality since it doesn't care about the kind
of socket (unix vs. IP).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c    |  2 +-
 qemu-sockets.c | 32 ++++++++++++++++++++++++++++----
 qemu_socket.h  |  2 +-
 3 file modificati, 30 inserzioni(+), 6 rimozioni(-)

diff --git a/qemu-char.c b/qemu-char.c
index b33bdaa..665df93 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2453,7 +2453,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
         if (is_listen) {
             fd = unix_listen_opts(opts, NULL);
         } else {
-            fd = unix_connect_opts(opts, NULL);
+            fd = unix_connect_opts(opts, false, NULL, NULL);
         }
     } else {
         if (is_listen) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 8a7c5d4..778428f 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -587,27 +587,51 @@ err:
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts, Error **errp)
+int unix_connect_opts(QemuOpts *opts, bool block, bool *in_progress, Error **errp)
 {
     struct sockaddr_un un;
     const char *path = qemu_opt_get(opts, "path");
     int sock;
+    int rc;
 
     if (NULL == path) {
         error_setg(errp, "unix connect: no path specified\n");
         return -1;
     }
 
+    if (in_progress) {
+        *in_progress = false;
+    }
+
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
         error_set(errp, QERR_SOCKET_CREATE_FAILED);
         return -1;
     }
+    if (!block) {
+        socket_set_nonblock(sock);
+    }
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
     snprintf(un.sun_path, sizeof(un.sun_path), "%s", path);
-    if (connect(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
+    do {
+        rc = 0;
+        if (connect(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
+            rc = -socket_error();
+        }
+    } while (rc == -EINTR);
+
+#ifdef _WIN32
+    if (!block && (rc == -EINPROGRESS || rc == -EWOULDBLOCK
+                   || rc == -WSAEALREADY)) {
+#else
+    if (!block && (rc == -EINPROGRESS)) {
+#endif
+        if (in_progress) {
+            *in_progress = true;
+        }
+    } else if (rc < 0) {
         error_set(errp, QERR_SOCKET_CONNECT_FAILED);
         close(sock);
 	return -1;
@@ -653,7 +677,7 @@ int unix_connect(const char *path, Error **errp)
 
     opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
     qemu_opt_set(opts, "path", path);
-    sock = unix_connect_opts(opts, errp);
+    sock = unix_connect_opts(opts, false, NULL, errp);
     qemu_opts_del(opts);
     return sock;
 }
@@ -667,7 +691,7 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
     return -1;
 }
 
-int unix_connect_opts(QemuOpts *opts, Error **errp)
+int unix_connect_opts(QemuOpts *opts, bool block, bool *in_progress, Error **errp)
 {
     error_setg(errp, "unix sockets are not available on windows\n");
     errno = ENOTSUP;
diff --git a/qemu_socket.h b/qemu_socket.h
index ad9e342..37dcafd 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -49,7 +49,7 @@ const char *inet_strfamily(int family);
 
 int unix_listen_opts(QemuOpts *opts, Error **errp);
 int unix_listen(const char *path, char *ostr, int olen, Error **errp);
-int unix_connect_opts(QemuOpts *opts, Error **errp);
+int unix_connect_opts(QemuOpts *opts, bool block, bool *in_progress, Error **errp);
 int unix_connect(const char *path, Error **errp);
 
 /* Old, ipv4 only bits.  Don't use for new code. */
-- 
1.7.12

[Qemu-devel] [PATCH 12/12] qemu-sockets: add socket_listen, socket_connect, socket_parse

[Paolo Bonzini]

These are QAPI-friendly versions of the qemu-sockets functions.  They
support IP sockets, Unix sockets, and named file descriptors, using a
QAPI union to dispatch to the correct function.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-sockets.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-tool.c    |  6 ++++
 qemu_socket.h  |  4 +++
 3 file modificati, 109 inserzioni(+)

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 778428f..d3bc712 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -22,6 +22,7 @@
 #include <errno.h>
 #include <unistd.h>
 
+#include "monitor.h"
 #include "qemu_socket.h"
 #include "qemu-common.h" /* for qemu_isdigit */
 
@@ -714,6 +715,104 @@ int unix_connect(const char *path, Error **errp)
 
 #endif
 
+SocketAddress *socket_parse(const char *str, Error **errp)
+{
+    SocketAddress *addr = NULL;
+
+    addr = g_new(SocketAddress, 1);
+    if (strstart(str, "unix:", NULL)) {
+        if (str[5] == '\0') {
+            error_setg(errp, "invalid Unix socket address\n");
+            goto fail;
+        } else {
+            addr->kind = SOCKET_ADDRESS_KIND_UNIX;
+            addr->q_unix = g_new(UnixSocketAddress, 1);
+            addr->q_unix->path = g_strdup(str + 5);
+        }
+    } else if (strstart(str, "fd:", NULL)) {
+        if (str[3] == '\0') {
+            error_setg(errp, "invalid file descriptor address\n");
+            goto fail;
+        } else {
+            addr->kind = SOCKET_ADDRESS_KIND_FD;
+            addr->fd = g_new(String, 1);
+            addr->fd->str = g_strdup(str + 3);
+        }
+    } else {
+        addr->kind = SOCKET_ADDRESS_KIND_INET;
+        addr->inet = g_new(IPSocketAddress, 1);
+        addr->inet = inet_parse(str, errp);
+        if (addr->inet == NULL) {
+            goto fail;
+        }
+    }
+    return addr;
+
+fail:
+    qapi_free_SocketAddress(addr);
+    return NULL;
+}
+
+int socket_connect(SocketAddress *addr, bool block, bool *in_progress, Error **errp)
+{
+    QemuOpts *opts;
+    int fd;
+
+    if (in_progress) {
+        *in_progress = false;
+    }
+
+    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+    switch (addr->kind) {
+    case SOCKET_ADDRESS_KIND_INET:
+        inet_addr_to_opts(opts, addr->inet);
+        fd = inet_connect_opts(opts, block, in_progress, errp);
+        break;
+
+    case SOCKET_ADDRESS_KIND_UNIX:
+        qemu_opt_set(opts, "path", addr->q_unix->path);
+        fd = unix_connect_opts(opts, block, in_progress, errp);
+        break;
+
+    case SOCKET_ADDRESS_KIND_FD:
+        fd = monitor_get_fd(cur_mon, addr->fd->str, errp);
+        break;
+
+    default:
+        abort();
+    }
+    qemu_opts_del(opts);
+    return fd;
+}
+
+int socket_listen(SocketAddress *addr, Error **errp)
+{
+    QemuOpts *opts;
+    int fd;
+
+    opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL);
+    switch (addr->kind) {
+    case SOCKET_ADDRESS_KIND_INET:
+        inet_addr_to_opts(opts, addr->inet);
+        fd = inet_listen_opts(opts, 0, errp);
+        break;
+
+    case SOCKET_ADDRESS_KIND_UNIX:
+        qemu_opt_set(opts, "path", addr->q_unix->path);
+        fd = unix_listen_opts(opts, errp);
+        break;
+
+    case SOCKET_ADDRESS_KIND_FD:
+        fd = monitor_get_fd(cur_mon, addr->fd->str, errp);
+        break;
+
+    default:
+        abort();
+    }
+    qemu_opts_del(opts);
+    return fd;
+}
+
 #ifdef _WIN32
 static void socket_cleanup(void)
 {
diff --git a/qemu-tool.c b/qemu-tool.c
index 18205ba..7658fde 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -42,6 +42,12 @@ int monitor_cur_is_qmp(void)
     return 0;
 }
 
+int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
+{
+    error_setg(errp, "only QEMU supports file descriptor passing");
+    return -1;
+}
+
 void monitor_set_error(Monitor *mon, QError *qerror)
 {
 }
diff --git a/qemu_socket.h b/qemu_socket.h
index 37dcafd..9cd99f2 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -52,6 +52,10 @@ int unix_listen(const char *path, char *ostr, int olen, Error **errp);
 int unix_connect_opts(QemuOpts *opts, bool block, bool *in_progress, Error **errp);
 int unix_connect(const char *path, Error **errp);
 
+SocketAddress *socket_parse(const char *str, Error **errp);
+int socket_connect(SocketAddress *addr, bool block, bool *in_progress, Error **errp);
+int socket_listen(SocketAddress *addr, Error **errp);
+
 /* Old, ipv4 only bits.  Don't use for new code. */
 int parse_host_port(struct sockaddr_in *saddr, const char *str);
 int socket_init(void);
-- 
1.7.12

[Qemu-devel] [PATCH 13/12] block: add close notifiers

[Paolo Bonzini]

The first user of close notifiers will be the embedded NBD server.
It is possible to use them to do some of the ad hoc processing
(e.g. for block jobs and I/O limits) that is currently done by
bdrv_close.

Acked-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs |  3 ++-
 block.c       | 19 ++++++++++++++-----
 block.h       |  1 +
 block_int.h   |  2 ++
 4 file modificati, 19 inserzioni(+), 6 rimozioni(-)

diff --git a/Makefile.objs b/Makefile.objs
index 03da150..124f783 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -43,6 +43,7 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
 
 block-obj-y = cutils.o iov.o cache-utils.o qemu-option.o module.o async.o
 block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o
+block-obj-y += notify.o
 block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -92,7 +93,7 @@ common-obj-y += bt-host.o bt-vhci.o
 
 common-obj-y += iov.o acl.o
 common-obj-$(CONFIG_POSIX) += compatfd.o
-common-obj-y += notify.o event_notifier.o
+common-obj-y += event_notifier.o
 common-obj-y += qemu-timer.o qemu-timer-common.o
 
 common-obj-$(CONFIG_SLIRP) += slirp/
diff --git a/block.c b/block.c
index 470bdcc..cb44dac 100644
--- a/block.c
+++ b/block.c
@@ -28,6 +28,7 @@
 #include "block_int.h"
 #include "module.h"
 #include "qjson.h"
+#include "notify.h"
 #include "qemu-coroutine.h"
 #include "qmp-commands.h"
 #include "qemu-timer.h"
@@ -310,9 +311,16 @@ BlockDriverState *bdrv_new(const char *device_name)
         QTAILQ_INSERT_TAIL(&bdrv_states, bs, list);
     }
     bdrv_iostatus_disable(bs);
+    notifier_list_init(&bs->close_notifiers);
+
     return bs;
 }
 
+void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
+{
+    notifier_list_add(&bs->close_notifiers, notify);
+}
+
 BlockDriver *bdrv_find_format(const char *format_name)
 {
     BlockDriver *drv1;
@@ -862,12 +870,13 @@ unlink_and_fail:
 void bdrv_close(BlockDriverState *bs)
 {
     bdrv_flush(bs);
-    if (bs->drv) {
-        if (bs->job) {
-            block_job_cancel_sync(bs->job);
-        }
-        bdrv_drain_all();
+    if (bs->job) {
+        block_job_cancel_sync(bs->job);
+    }
+    bdrv_drain_all();
+    notifier_list_notify(&bs->close_notifiers, bs);
 
+    if (bs->drv) {
         if (bs == bs_snapshots) {
             bs_snapshots = NULL;
         }
diff --git a/block.h b/block.h
index 2e2be11..921c72e 100644
--- a/block.h
+++ b/block.h
@@ -131,6 +131,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
               BlockDriver *drv);
 void bdrv_close(BlockDriverState *bs);
+void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify);
 int bdrv_attach_dev(BlockDriverState *bs, void *dev);
 void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
 void bdrv_detach_dev(BlockDriverState *bs, void *dev);
diff --git a/block_int.h b/block_int.h
index 4452f6f..4eb79c2 100644
--- a/block_int.h
+++ b/block_int.h
@@ -293,6 +293,8 @@ struct BlockDriverState {
     BlockDriverState *backing_hd;
     BlockDriverState *file;
 
+    NotifierList close_notifiers;
+
     /* number of in-flight copy-on-read requests */
     unsigned int copy_on_read_in_flight;
 
-- 
1.7.12

[Qemu-devel] [PATCH 14/12] qmp: add NBD server commands

[Paolo Bonzini]

Adding an NBD server inside QEMU is trivial, since all the logic is
in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
The main difference is that qemu-nbd serves a single unnamed export,
while QEMU serves named exports.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs    |   2 +-
 blockdev-nbd.c   | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |  43 ++++++++++++++++++++
 qmp-commands.hx  |  16 ++++++++
 4 file modificati, 177 inserzioni(+). 1 rimozione(-)
 create mode 100644 blockdev-nbd.c

diff --git a/Makefile.objs b/Makefile.objs
index 124f783..40384ff 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -61,7 +61,7 @@ endif
 # suppress *all* target specific code in case of system emulation, i.e. a
 # single QEMU executable should support all CPUs and machines.
 
-common-obj-y = $(block-obj-y) blockdev.o
+common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o
 common-obj-y += net.o net/
 common-obj-y += qom/
 common-obj-y += readline.o console.o cursor.o
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
new file mode 100644
index 0000000..ef8a34a
--- /dev/null
+++ b/blockdev-nbd.c
@@ -0,0 +1,119 @@
+/*
+ * Serving QEMU block devices via NBD
+ *
+ * Copyright (c) 2012 Red Hat, Inc.
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "blockdev.h"
+#include "hw/block-common.h"
+#include "monitor.h"
+#include "qerror.h"
+#include "sysemu.h"
+#include "qmp-commands.h"
+#include "trace.h"
+#include "nbd.h"
+#include "qemu_socket.h"
+
+static int server_fd = -1;
+
+static void nbd_accept(void *opaque)
+{
+    struct sockaddr_in addr;
+    socklen_t addr_len = sizeof(addr);
+
+    int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+    if (fd >= 0) {
+        nbd_client_new(NULL, fd, nbd_client_put);
+    }
+}
+
+void qmp_nbd_server_start(SocketAddress *addr, Error **errp)
+{
+    if (server_fd != -1) {
+        error_setg(errp, "NBD server already running");
+        return;
+    }
+
+    server_fd = socket_listen(addr, errp);
+    if (server_fd != -1) {
+        qemu_set_fd_handler2(server_fd, NULL, nbd_accept, NULL, NULL);
+    }
+}
+
+/* Hook into the BlockDriverState notifiers to close the export when
+ * the file is closed.
+ */
+typedef struct NBDCloseNotifier {
+    Notifier n;
+    NBDExport *exp;
+    QTAILQ_ENTRY(NBDCloseNotifier) next;
+} NBDCloseNotifier;
+
+static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
+    QTAILQ_HEAD_INITIALIZER(close_notifiers);
+
+static void nbd_close_notifier(Notifier *n, void *data)
+{
+    NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
+
+    notifier_remove(&cn->n);
+    QTAILQ_REMOVE(&close_notifiers, cn, next);
+
+    nbd_export_close(cn->exp);
+    nbd_export_put(cn->exp);
+    g_free(cn);
+}
+
+static void nbd_server_put_ref(NBDExport *exp)
+{
+    BlockDriverState *bs = nbd_export_get_blockdev(exp);
+    drive_put_ref(drive_get_by_blockdev(bs));
+}
+
+void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
+                        Error **errp)
+{
+    BlockDriverState *bs;
+    NBDExport *exp;
+    NBDCloseNotifier *n;
+
+    if (nbd_export_find(device)) {
+        error_setg(errp, "NBD server already exporting device '%s'", device);
+        return;
+    }
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY,
+                         nbd_server_put_ref);
+
+    nbd_export_set_name(exp, device);
+    drive_get_ref(drive_get_by_blockdev(bs));
+
+    n = g_malloc0(sizeof(NBDCloseNotifier));
+    n->n.notify = nbd_close_notifier;
+    n->exp = exp;
+    bdrv_add_close_notifier(bs, &n->n);
+    QTAILQ_INSERT_TAIL(&close_notifiers, n, next);
+}
+
+void qmp_nbd_server_stop(Error **errp)
+{
+    while (!QTAILQ_EMPTY(&close_notifiers)) {
+        NBDCloseNotifier *cn = QTAILQ_FIRST(&close_notifiers);
+        nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp));
+    }
+
+    qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
+    close(server_fd);
+    server_fd = -1;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 1588372..e384924 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2608,3 +2608,46 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @nbd-server-start:
+#
+# Start an NBD server listening on the given host and port.  Block
+# devices can then be exported using @nbd-server-add.  The NBD
+# server will present them as named exports; for example, another
+# QEMU instance could refer to them as "nbd:HOST:PORT:exportname=NAME".
+#
+# @addr: Address on which to listen.
+#
+# Returns: error if the server is already running.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-start',
+  'data': { 'addr': 'SocketAddress' } }
+
+##
+# @nbd-server-add:
+#
+# Export a device to QEMU's embedded NBD server.
+#
+# @device: Block device to be exported
+#
+# @writable: Whether clients should be able to write to the device via the
+#     NBD connection (default false). #optional
+#
+# Returns: error if the device is already marked for export.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} }
+
+##
+# @nbd-server-stop:
+#
+# Stop QEMU's embedded NBD server, and unregister all devices previously
+# added via @nbd-server-add.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-stop' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6e21ddb..7ab8a34 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2506,6 +2506,22 @@ EQMP
     },
 
     {
+        .name       = "nbd-server-start",
+        .args_type  = "addr:q",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_start,
+    },
+    {
+        .name       = "nbd-server-add",
+        .args_type  = "device:B,writable:b?",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_add,
+    },
+    {
+        .name       = "nbd-server-stop",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_stop,
+    },
+
+    {
         .name       = "change-vnc-password",
         .args_type  = "password:s",
         .mhandler.cmd_new = qmp_marshal_input_change_vnc_password,
-- 
1.7.12

[Qemu-devel] [PATCH 15/12] hmp: add NBD server commands

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hmp-commands.hx | 29 +++++++++++++++++++++++++++++
 hmp.c           | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hmp.h           |  2 ++
 3 file modificati, 86 inserzioni(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed67e99..b0af484 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1247,6 +1247,35 @@ Remove all matches from the access control list, and set the default
 policy back to @code{deny}.
 ETEXI
 
+    {
+        .name       = "nbd_server_start",
+        .args_type  = "writable:-w,uri:s",
+        .params     = "nbd_server_start [-w] host:port",
+        .help       = "serve block devices on the given host and port",
+        .mhandler.cmd = hmp_nbd_server_start,
+    },
+STEXI
+@item nbd_server_start @var{host}:@var{port}
+@findex nbd_server_start
+Start an NBD server on the given host and/or port, and serve all of the
+virtual machine's block devices that have an inserted media on it.
+The @option{-w} option makes the devices writable.
+ETEXI
+
+    {
+        .name       = "nbd_server_stop",
+        .args_type  = "",
+        .params     = "nbd_server_stop",
+        .help       = "stop serving block devices using the NBD protocol",
+        .mhandler.cmd = hmp_nbd_server_stop,
+    },
+STEXI
+@item nbd_server_stop
+@findex nbd_server_stop
+Stop the QEMU embedded NBD server.
+ETEXI
+
+
 #if defined(TARGET_I386)
 
     {
diff --git a/hmp.c b/hmp.c
index ba6fbd3..f047b5d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -18,6 +18,7 @@
 #include "qemu-option.h"
 #include "qemu-timer.h"
 #include "qmp-commands.h"
+#include "qemu_socket.h"
 #include "monitor.h"
 #include "console.h"
 
@@ -1168,3 +1169,57 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
+{
+    const char *uri = qdict_get_str(qdict, "uri");
+    int writable = qdict_get_try_bool(qdict, "writable", 0);
+    Error *local_err = NULL;
+    BlockInfoList *block_list, *info;
+    SocketAddress *addr;
+
+    /* First check if the address is valid and start the server.  */
+    addr = socket_parse(uri, &local_err);
+    if (local_err != NULL) {
+        goto exit;
+    }
+
+    qmp_nbd_server_start(addr, &local_err);
+    qapi_free_SocketAddress(addr);
+    if (local_err != NULL) {
+        goto exit;
+    }
+
+    /* Then try adding all block devices.  If one fails, close all and
+     * exit.
+     */
+    block_list = qmp_query_block(NULL);
+
+    for (info = block_list; info; info = info->next) {
+        if (!info->value->has_inserted) {
+            continue;
+        }
+
+        qmp_nbd_server_add(info->value->device,
+                           true, !info->value->inserted->ro && writable,
+                           &local_err);
+
+        if (local_err != NULL) {
+            qmp_nbd_server_stop(NULL);
+            break;
+        }
+    }
+
+    qapi_free_BlockInfoList(block_list);
+
+exit:
+    hmp_handle_error(mon, &local_err);
+}
+
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
+{
+    Error *errp = NULL;
+
+    qmp_nbd_server_stop(&errp);
+    hmp_handle_error(mon, &errp);
+}
diff --git a/hmp.h b/hmp.h
index 48b9c59..0d02479 100644
--- a/hmp.h
+++ b/hmp.h
@@ -73,5 +73,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
 
 #endif
-- 
1.7.12

Re: [Qemu-devel] [PATCH 04/12] qapi: add "unix" to the set of reserved words

[Peter Maydell]

On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> It is #defined to 1.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/qapi.py | 4 +++-
>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>
> 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("*")
>

I can't help thinking this is fighting a losing battle, and we should just
always prefix everything to avoid clashes.

-- PMM

Re: [Qemu-devel] [PATCH 04/12] qapi: add "unix" to the set of reserved words

[Paolo Bonzini]

Il 19/09/2012 17:46, Peter Maydell ha scritto:
> On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> It is #defined to 1.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  scripts/qapi.py | 4 +++-
>>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>>
>> 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("*")
>>
> 
> I can't help thinking this is fighting a losing battle, and we should just
> always prefix everything to avoid clashes.

That would be so ugly that it would be almost useless.  Plus there would
be a huge amount of code to convert.

Paolo

Re: [Qemu-devel] [PATCH 04/12] qapi: add "unix" to the set of reserved words

[Paolo Bonzini]

Il 19/09/2012 17:58, Paolo Bonzini ha scritto:
> Il 19/09/2012 17:46, Peter Maydell ha scritto:
>> On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> It is #defined to 1.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  scripts/qapi.py | 4 +++-
>>>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>>>
>>> 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("*")
>>>
>>
>> I can't help thinking this is fighting a losing battle, and we should just
>> always prefix everything to avoid clashes.
> 
> That would be so ugly that it would be almost useless.  Plus there would
> be a huge amount of code to convert.

Also, not really that bad:

$ gcc -dM -x c /dev/null -E|grep define\ [^_]
#define unix 1
#define linux 1

I don't expect other OSes to be significantly worse.  Remember this
breakage is not limited to QAPI-generated code, it would happen in
normal code as well.  I learnt today that a variable named "unix" is not
kosher.

Paolo

Re: [Qemu-devel] [PATCH 06/12] qapi: add socket address types

[Eric Blake]

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

On 09/19/2012 08:31 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qapi-schema.json | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file modificato, 53 inserzioni(+)
> 

>  ##
> +# @IPSocketAddress
> +#
> +# Captures the destination address of an IP socket

Given the presence of @to, would this read better as:
IP socket or socket range

> +#
> +# @host: host part of the address
> +#
> +# @port: port part of the address, or lowest port if @to is present
> +#
> +# @to: highest port to try
> +#
> +# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6
> +#        #optional

Isn't the #optional supposed to occur first after the colon, as in:

@ipv4: #optional whether to accept IPv4...

> +#
> +# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6
> +#        #optional
> +#
> +# Since 1.3
> +##
> +{ 'type': 'IPSocketAddress',
> +  'data': {
> +    'host': 'str',
> +    'port': 'str',
> +    '*to': 'uint16',

Why is 'port' a string, but 'to' a uint16?  Shouldn't they both be uint16?

-- 
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] [PATCH 14/12] qmp: add NBD server commands

[Eric Blake]

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

On 09/19/2012 08:31 AM, Paolo Bonzini wrote:
> Adding an NBD server inside QEMU is trivial, since all the logic is
> in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
> The main difference is that qemu-nbd serves a single unnamed export,
> while QEMU serves named exports.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

> +##
> +# @nbd-server-add:
> +#
> +# Export a device to QEMU's embedded NBD server.
> +#
> +# @device: Block device to be exported
> +#
> +# @writable: Whether clients should be able to write to the device via the
> +#     NBD connection (default false). #optional

Again, shouldn't #optional be first after the colon?

> +#
> +# Returns: error if the device is already marked for export.
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} }
> +
> +##
> +# @nbd-server-stop:
> +#
> +# Stop QEMU's embedded NBD server, and unregister all devices previously
> +# added via @nbd-server-add.

Do we need a way to unregister a single device, rather than having to
stop the NBD server to unregister all devices?

-- 
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] [PATCH 15/12] hmp: add NBD server commands

[Eric Blake]

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

On 09/19/2012 08:31 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hmp-commands.hx | 29 +++++++++++++++++++++++++++++
>  hmp.c           | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hmp.h           |  2 ++
>  3 file modificati, 86 inserzioni(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index ed67e99..b0af484 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1247,6 +1247,35 @@ Remove all matches from the access control list, and set the default
>  policy back to @code{deny}.
>  ETEXI
>  
> +    {
> +        .name       = "nbd_server_start",

> +ETEXI
> +
> +    {
> +        .name       = "nbd_server_stop",

No nbd_server_add?

-- 
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] [PATCH 04/12] qapi: add "unix" to the set of reserved words

[Blue Swirl]

On Wed, Sep 19, 2012 at 4:02 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 19/09/2012 17:58, Paolo Bonzini ha scritto:
>> Il 19/09/2012 17:46, Peter Maydell ha scritto:
>>> On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> It is #defined to 1.
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> ---
>>>>  scripts/qapi.py | 4 +++-
>>>>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>>>>
>>>> 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("*")
>>>>
>>>
>>> I can't help thinking this is fighting a losing battle, and we should just
>>> always prefix everything to avoid clashes.
>>
>> That would be so ugly that it would be almost useless.  Plus there would
>> be a huge amount of code to convert.
>
> Also, not really that bad:
>
> $ gcc -dM -x c /dev/null -E|grep define\ [^_]
> #define unix 1
> #define linux 1
>
> I don't expect other OSes to be significantly worse.  Remember this
> breakage is not limited to QAPI-generated code, it would happen in
> normal code as well.  I learnt today that a variable named "unix" is not
> kosher.

I got only this from OpenBSD:
#define sparc 1

Mingw has these:
#define WIN32 1
#define WINNT 1
#define i386 1

I'd suppose the full list from GCC is this:
$ grep -hr 'builtin_define_std' gcc/config |sed -n
's/.*"\([^"]*\)".*/\1/p'|sort -u
AVR
BFIN
CRIS
GNU_CRIS
GO32
LANGUAGE_ASSEMBLY
LANGUAGE_C
LANGUAGE_C_PLUS_PLUS
LANGUAGE_OBJECTIVE_C
MACH
MIPSEB
MIPSEL
MOXIE
MSDOS
PPC
R3000
R4000
REVARGV
SYSTYPE_BSD
SYSTYPE_SVR4
VMS
WIN32
WIN64
WINNT
bfin
cris
fr30
h8300
host_mips
hp800
hp9000
hp9k8
hpux
i386
linux
mc68000
mc68010
mc68020
mc68030
mc68040
mc68060
mc68332
mcpu32
mep
moxie
pdp11
powerpc
sgi
sparc
spectrum
sun
tpf
unix
vms
xstormy16

>
> Paolo
>

Re: [Qemu-devel] [PATCH 01/12] monitor: use monitor_handle_fd_param for non-Error-friendly users of named fds

[Luiz Capitulino]

On Wed, 19 Sep 2012 16:31:04 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> monitor_handle_fd_param and monitor_get_fd are mostly the same, except
> that monitor_handle_fd_param does error reporting wrong.  Use it in all
> other places that do it wrong, instead of reinventing it.

Hmm, why do we want to do this?

As far as I understand it the main difference between the two functions
is that if fdname is a number (for a weak definition of number),
monitor_handle_fd_param() assumes that the fd already exists in qemu
(eg. it was passed by the parent process). 

I don't like much the idea of spreading its usage because if it's
used by a qmp command (and this patch does just that) then clients
using that command won't be able to set the first character of fdname
to a number, IOW it's an incompatible change (unless this doesn't work
today for some reason, but I believe it does).

Another side effect is that you add the possibility of functions
changing from monitor_get_fd() to monitor_handle_fd_param() to also take
fds passed by the parent process. Might be positive, but I wonder if that's
useful for the commands you're changing.

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/kvm/pci-assign.c | 4 +---
>  migration-fd.c      | 3 +--
>  monitor.c           | 6 +++---
>  3 file modificati, 5 inserzioni(+), 8 rimozioni(-)
> 
> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
> index 05b93d9..8b96a57 100644
> --- a/hw/kvm/pci-assign.c
> +++ b/hw/kvm/pci-assign.c
> @@ -582,10 +582,8 @@ static int get_real_device(AssignedDevice *pci_dev, uint16_t r_seg,
>          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);
> +            dev->config_fd = monitor_handle_fd_param(cur_mon, pci_dev->configfd_name);
>              if (dev->config_fd < 0) {
> -                error_report("%s: (%s) unkown", __func__,
> -                             pci_dev->configfd_name);
>                  return 1;
>              }
>          }
> diff --git a/migration-fd.c b/migration-fd.c
> index 50138ed..3eb53d9 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -75,9 +75,8 @@ 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_handle_fd_param(cur_mon, fdname);
>      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..4901600 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_handle_fd_param(mon, fdname);
>          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_handle_fd_param(mon, fdname);
>          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_handle_fd_param(mon, fdname);
>  	if (qemu_chr_add_client(s, fd) < 0) {
>  	    qerror_report(QERR_ADD_CLIENT_FAILED);
>  	    return -1;

Re: [Qemu-devel] [PATCH 02/12] monitor: add Error * argument to monitor_get_fd

[Luiz Capitulino]

On Wed, 19 Sep 2012 16:31:05 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Differentiate monitor_get_fd and monitor_handle_fd_param by doing
> correct error propagation in the former and its callers.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  dump.c    | 5 +++--
>  monitor.c | 9 ++++++---
>  monitor.h | 2 +-
>  3 file modificati, 10 inserzioni(+), 6 rimozioni(-)
> 
> diff --git a/dump.c b/dump.c
> index 2bf8d8d..cc7aecd 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -824,6 +824,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
>      int fd = -1;
>      DumpState *s;
>      int ret;
> +    Error *local_err = NULL;
>  
>      if (has_begin && !has_length) {
>          error_set(errp, QERR_MISSING_PARAMETER, "length");
> @@ -836,9 +837,9 @@ 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, &local_err);
>          if (fd == -1) {
> -            error_set(errp, QERR_FD_NOT_FOUND, p);
> +            error_propagate(errp, local_err);
>              return;
>          }
>      }
> diff --git a/monitor.c b/monitor.c
> index 4901600..b73ae57 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -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_set(errp, QERR_FD_NOT_FOUND, fdname);

Let's use error_setg() instead, also allows for dropping QERR_FD_NOT_FOUND.

Looks good otherwise.

>      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)

Re: [Qemu-devel] [PATCH 06/12] qapi: add socket address types

[Paolo Bonzini]

Il 19/09/2012 19:20, Eric Blake ha scritto:
>> > +{ 'type': 'IPSocketAddress',
>> > +  'data': {
>> > +    'host': 'str',
>> > +    'port': 'str',
>> > +    '*to': 'uint16',
> Why is 'port' a string, but 'to' a uint16?  Shouldn't they both be uint16?

port can be a service name.

Paolo

Re: [Qemu-devel] [PATCH 14/12] qmp: add NBD server commands

[Paolo Bonzini]

Il 19/09/2012 19:48, Eric Blake ha scritto:
> Do we need a way to unregister a single device, rather than having to
> stop the NBD server to unregister all devices?

Possibly, but it can be added later.

Paolo

Re: [Qemu-devel] [PATCH 01/12] monitor: use monitor_handle_fd_param for non-Error-friendly users of named fds

[Paolo Bonzini]

Il 19/09/2012 22:42, Luiz Capitulino ha scritto:
> On Wed, 19 Sep 2012 16:31:04 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> monitor_handle_fd_param and monitor_get_fd are mostly the same, except
>> that monitor_handle_fd_param does error reporting wrong.  Use it in all
>> other places that do it wrong, instead of reinventing it.
> 
> Hmm, why do we want to do this?
> 
> As far as I understand it the main difference between the two functions
> is that if fdname is a number (for a weak definition of number),
> monitor_handle_fd_param() assumes that the fd already exists in qemu
> (eg. it was passed by the parent process). 

Oops, right, I remembered it was the other way round (i.e. first search
for a named file descriptor, and fall back to a numeric one).

> Another side effect is that you add the possibility of functions
> changing from monitor_get_fd() to monitor_handle_fd_param() to also take
> fds passed by the parent process. Might be positive, but I wonder if that's
> useful for the commands you're changing.

Making behavior more uniform may be a useful thing on its own.  We might
even consider moving it to monitor_get_fd (with the above tweak for
compatibility).  BTW, pci-assign.c is open-coding
monitor_handle_fd_param (including the numeric file descriptor behavior)
and we can remove the surrounding if.

Paolo

>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  hw/kvm/pci-assign.c | 4 +---
>>  migration-fd.c      | 3 +--
>>  monitor.c           | 6 +++---
>>  3 file modificati, 5 inserzioni(+), 8 rimozioni(-)
>>
>> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
>> index 05b93d9..8b96a57 100644
>> --- a/hw/kvm/pci-assign.c
>> +++ b/hw/kvm/pci-assign.c
>> @@ -582,10 +582,8 @@ static int get_real_device(AssignedDevice *pci_dev, uint16_t r_seg,
>>          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);
>> +            dev->config_fd = monitor_handle_fd_param(cur_mon, pci_dev->configfd_name);
>>              if (dev->config_fd < 0) {
>> -                error_report("%s: (%s) unkown", __func__,
>> -                             pci_dev->configfd_name);
>>                  return 1;
>>              }
>>          }
>> diff --git a/migration-fd.c b/migration-fd.c
>> index 50138ed..3eb53d9 100644
>> --- a/migration-fd.c
>> +++ b/migration-fd.c
>> @@ -75,9 +75,8 @@ 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_handle_fd_param(cur_mon, fdname);
>>      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..4901600 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_handle_fd_param(mon, fdname);
>>          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_handle_fd_param(mon, fdname);
>>          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_handle_fd_param(mon, fdname);
>>  	if (qemu_chr_add_client(s, fd) < 0) {
>>  	    qerror_report(QERR_ADD_CLIENT_FAILED);
>>  	    return -1;
> 

Re: [Qemu-devel] [PATCH 01/12] monitor: use monitor_handle_fd_param for non-Error-friendly users of named fds

[Luiz Capitulino]

On Thu, 20 Sep 2012 10:09:54 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 19/09/2012 22:42, Luiz Capitulino ha scritto:
> > On Wed, 19 Sep 2012 16:31:04 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 
> >> monitor_handle_fd_param and monitor_get_fd are mostly the same, except
> >> that monitor_handle_fd_param does error reporting wrong.  Use it in all
> >> other places that do it wrong, instead of reinventing it.
> > 
> > Hmm, why do we want to do this?
> > 
> > As far as I understand it the main difference between the two functions
> > is that if fdname is a number (for a weak definition of number),
> > monitor_handle_fd_param() assumes that the fd already exists in qemu
> > (eg. it was passed by the parent process). 
> 
> Oops, right, I remembered it was the other way round (i.e. first search
> for a named file descriptor, and fall back to a numeric one).

That's a lot better, IMO.

> > Another side effect is that you add the possibility of functions
> > changing from monitor_get_fd() to monitor_handle_fd_param() to also take
> > fds passed by the parent process. Might be positive, but I wonder if that's
> > useful for the commands you're changing.
> 
> Making behavior more uniform may be a useful thing on its own.  We might
> even consider moving it to monitor_get_fd (with the above tweak for
> compatibility).

Yes, although I can think of another issue: suppose an mngt app passes an
fd with fdname=9, but the fd doesn't reach qemu for some reason. Then the
mngt app executes a qmp command with fdname=9 and fd 9 turns out to exist
in qemu... Actually, a mngt app could do this even w/o passing an fd to
qemu.

Not sure how relevant this issue is though, as this can happen today
with qmp commands using monitor_handle_fd_param().

> BTW, pci-assign.c is open-coding
> monitor_handle_fd_param (including the numeric file descriptor behavior)
> and we can remove the surrounding if.

Yes, that's fine.

Re: [Qemu-devel] [PATCH 03/12] qapi: do not protect enum values from namespace pollution

[Luiz Capitulino]

On Wed, 19 Sep 2012 16:31:06 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 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>

Cherry-picked into qmp branch, thanks.

> ---
>  scripts/qapi-types.py | 4 ++--
>  scripts/qapi-visit.py | 2 +-
>  scripts/qapi.py       | 8 ++++----
>  3 file modificati, 7 inserzioni(+), 7 rimozioni(-)
> 
> 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

Re: [Qemu-devel] [PATCH 04/12] qapi: add "unix" to the set of reserved words

[Luiz Capitulino]

On Wed, 19 Sep 2012 16:31:07 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> It is #defined to 1.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Cherry-picked into qmp branch, thanks.

> ---
>  scripts/qapi.py | 4 +++-
>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
> 
> 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("*")
>  

Re: [Qemu-devel] [PATCH 01/12] monitor: use monitor_handle_fd_param for non-Error-friendly users of named fds

[Paolo Bonzini]

[snip]

Ok, I'll drop this patch and do everything in a single patch adding Error *.

Paolo