[PATCH v6 6/7] chardev/char-socket: move to .init + .connect api

Vladimir Sementsov-Ogievskiy posted 7 patches 1 week, 3 days ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>
[PATCH v6 6/7] chardev/char-socket: move to .init + .connect api
Posted by Vladimir Sementsov-Ogievskiy 1 week, 3 days ago
Move char-socket to new API. This will help to realize backend-transfer
feature for vhost-user-blk.

With this commit qemu_chr_fe_init() starts to do connecting, so we
should handle its errors instead of passing &error_abort.

Also, move qemu_chr_fe_init() in test-char.c, to trigger connect
before trying to get address.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 chardev/char-socket.c         | 55 ++++++++++++++++++++---------------
 chardev/char.c                |  7 +++--
 include/chardev/char-socket.h |  1 +
 tests/unit/test-char.c        | 14 ++++-----
 ui/dbus-chardev.c             | 12 ++++++--
 5 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index f3bc6290d2..0a5738c158 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1287,6 +1287,25 @@ static int qmp_chardev_open_socket_client(Chardev *chr,
 }
 
 
+static bool char_socket_connect(Chardev *chr, Error **errp)
+{
+    SocketChardev *s = SOCKET_CHARDEV(chr);
+
+    if (s->is_listen) {
+        if (qmp_chardev_open_socket_server(chr, s->is_telnet || s->is_tn3270,
+                                           s->is_waitconnect, errp) < 0) {
+            return false;
+        }
+    } else {
+        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
+
 static bool qmp_chardev_validate_socket(ChardevSocket *sock,
                                         SocketAddress *addr,
                                         Error **errp)
@@ -1363,10 +1382,9 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
 }
 
 
-static void qmp_chardev_open_socket(Chardev *chr,
-                                    ChardevBackend *backend,
-                                    bool *be_opened,
-                                    Error **errp)
+static bool char_socket_init(Chardev *chr,
+                             ChardevBackend *backend,
+                             Error **errp)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
     ChardevSocket *sock = backend->u.socket.data;
@@ -1374,7 +1392,6 @@ static void qmp_chardev_open_socket(Chardev *chr,
     bool is_listen      = sock->has_server  ? sock->server  : true;
     bool is_telnet      = sock->has_telnet  ? sock->telnet  : false;
     bool is_tn3270      = sock->has_tn3270  ? sock->tn3270  : false;
-    bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
     bool is_websock     = sock->has_websocket ? sock->websocket : false;
     SocketAddress *addr;
 
@@ -1383,6 +1400,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
     s->is_tn3270 = is_tn3270;
     s->is_websock = is_websock;
     s->do_nodelay = do_nodelay;
+    s->is_waitconnect = sock->has_wait ? sock->wait : false;
     s->reconnect_time_ms = sock->has_reconnect_ms ? sock->reconnect_ms : 0;
 
     if (sock->tls_creds) {
@@ -1392,7 +1410,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
         if (!creds) {
             error_setg(errp, "No TLS credentials with id '%s'",
                        sock->tls_creds);
-            return;
+            return false;
         }
         s->tls_creds = (QCryptoTLSCreds *)
             object_dynamic_cast(creds,
@@ -1400,7 +1418,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
         if (!s->tls_creds) {
             error_setg(errp, "Object with id '%s' is not TLS credentials",
                        sock->tls_creds);
-            return;
+            return false;
         }
         object_ref(OBJECT(s->tls_creds));
         if (!qcrypto_tls_creds_check_endpoint(s->tls_creds,
@@ -1408,7 +1426,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
                                           ? QCRYPTO_TLS_CREDS_ENDPOINT_SERVER
                                           : QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
                                           errp)) {
-            return;
+            return false;
         }
     }
     s->tls_authz = g_strdup(sock->tls_authz);
@@ -1416,7 +1434,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
     s->addr = addr = socket_address_flatten(sock->addr);
 
     if (!qmp_chardev_validate_socket(sock, addr, errp)) {
-        return;
+        return false;
     }
 
     qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
@@ -1433,26 +1451,14 @@ static void qmp_chardev_open_socket(Chardev *chr,
      */
     if (!chr->handover_yank_instance) {
         if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)) {
-            return;
+            return false;
         }
     }
     s->registered_yank = true;
 
-    /* be isn't opened until we get a connection */
-    *be_opened = false;
-
     update_disconnected_filename(s);
 
-    if (s->is_listen) {
-        if (qmp_chardev_open_socket_server(chr, is_telnet || is_tn3270,
-                                           is_waitconnect, errp) < 0) {
-            return;
-        }
-    } else {
-        if (qmp_chardev_open_socket_client(chr, errp) < 0) {
-            return;
-        }
-    }
+    return true;
 }
 
 static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
@@ -1576,7 +1582,8 @@ static void char_socket_class_init(ObjectClass *oc, const void *data)
     cc->supports_yank = true;
 
     cc->parse = qemu_chr_parse_socket;
-    cc->open = qmp_chardev_open_socket;
+    cc->init = char_socket_init;
+    cc->connect = char_socket_connect;
     cc->chr_wait_connected = tcp_chr_wait_connected;
     cc->chr_write = tcp_chr_write;
     cc->chr_sync_read = tcp_chr_sync_read;
diff --git a/chardev/char.c b/chardev/char.c
index 33eba0cdc0..a6212d57ed 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1222,12 +1222,15 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
     }
 
     chr->fe = NULL;
-    qemu_chr_fe_init(fe, chr_new, &error_abort);
+    if (!qemu_chr_fe_init(fe, chr_new, errp)) {
+        object_unref(OBJECT(chr_new));
+        return NULL;
+    }
 
     if (fe->chr_be_change(fe->opaque) < 0) {
         error_setg(errp, "Chardev '%s' change failed", chr_new->label);
         chr_new->fe = NULL;
-        qemu_chr_fe_init(fe, chr, &error_abort);
+        qemu_chr_fe_init(fe, chr, NULL);
         if (closed_sent) {
             qemu_chr_be_event(chr, CHR_EVENT_OPENED);
         }
diff --git a/include/chardev/char-socket.h b/include/chardev/char-socket.h
index d6d13ad37f..0109727eaa 100644
--- a/include/chardev/char-socket.h
+++ b/include/chardev/char-socket.h
@@ -68,6 +68,7 @@ struct SocketChardev {
     bool is_listen;
     bool is_telnet;
     bool is_tn3270;
+    bool is_waitconnect;
     GSource *telnet_source;
     TCPChardevTelnetInit *telnet_init;
 
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index 8a98e42cad..9ecd5ccfe6 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -845,6 +845,7 @@ static void char_websock_test(void)
                               0xef, 0xaa, 0xc5, 0x97, /* Masking key */
                               0xec, 0x42              /* Status code */ };
 
+    qemu_chr_fe_init(&fe, chr, &error_abort);
     addr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
     qdict = qobject_to(QDict, addr);
     port = qdict_get_str(qdict, "port");
@@ -852,7 +853,6 @@ static void char_websock_test(void)
     handshake_port = g_strdup_printf(handshake, port, port);
     qobject_unref(qdict);
 
-    qemu_chr_fe_init(&fe, chr, &error_abort);
     qemu_chr_fe_set_handlers(&fe, websock_server_can_read, websock_server_read,
                              NULL, NULL, chr, NULL, true);
 
@@ -1216,6 +1216,8 @@ static void char_socket_server_test(gconstpointer opaque)
     g_assert_nonnull(chr);
     g_assert(!object_property_get_bool(OBJECT(chr), "connected", &error_abort));
 
+    qemu_chr_fe_init(&c, chr, &error_abort);
+
     qaddr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
     g_assert_nonnull(qaddr);
 
@@ -1224,8 +1226,6 @@ static void char_socket_server_test(gconstpointer opaque)
     visit_free(v);
     qobject_unref(qaddr);
 
-    qemu_chr_fe_init(&c, chr, &error_abort);
-
  reconnect:
     data.event = -1;
     data.fe = &c;
@@ -1417,6 +1417,8 @@ static void char_socket_client_test(gconstpointer opaque)
     qemu_opts_del(opts);
     g_assert_nonnull(chr);
 
+    qemu_chr_fe_init(&c, chr, &error_abort);
+
     if (config->reconnect) {
         /*
          * If reconnect is set, the connection will be
@@ -1431,8 +1433,6 @@ static void char_socket_client_test(gconstpointer opaque)
                                           &error_abort));
     }
 
-    qemu_chr_fe_init(&c, chr, &error_abort);
-
  reconnect:
     data.event = -1;
     data.fe = &c;
@@ -1550,6 +1550,8 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
     g_assert_nonnull(chr);
     g_assert(!object_property_get_bool(OBJECT(chr), "connected", &error_abort));
 
+    qemu_chr_fe_init(&c, chr, &error_abort);
+
     qaddr = object_property_get_qobject(OBJECT(chr), "addr", &error_abort);
     g_assert_nonnull(qaddr);
 
@@ -1558,8 +1560,6 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
     visit_free(v);
     qobject_unref(qaddr);
 
-    qemu_chr_fe_init(&c, chr, &error_abort);
-
     qemu_chr_fe_set_handlers(&c, char_socket_can_read, char_socket_discard_read,
                              count_closed_event, NULL,
                              &closed, NULL, true);
diff --git a/ui/dbus-chardev.c b/ui/dbus-chardev.c
index d05dddaf81..23cf9d6ee9 100644
--- a/ui/dbus-chardev.c
+++ b/ui/dbus-chardev.c
@@ -210,8 +210,14 @@ dbus_chr_open(Chardev *chr, ChardevBackend *backend,
     if (*errp) {
         return;
     }
-    CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->open(
-        chr, be, be_opened, errp);
+    if (!CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->init(
+        chr, be, errp)) {
+        return;
+    }
+    if (!CHARDEV_CLASS(object_class_by_name(TYPE_CHARDEV_SOCKET))->connect(
+        chr, errp)) {
+        return;
+    }
 }
 
 static void
@@ -276,6 +282,8 @@ char_dbus_class_init(ObjectClass *oc, const void *data)
 
     cc->parse = dbus_chr_parse;
     cc->open = dbus_chr_open;
+    cc->init = NULL;
+    cc->connect = NULL;
     cc->chr_set_fe_open = dbus_chr_set_fe_open;
     cc->chr_set_echo = dbus_chr_set_echo;
     klass->parent_chr_be_event = cc->chr_be_event;
-- 
2.48.1