diff mbox series

[for-4.0,1/6] char-socket: Enable "wait" option for client mode

Message ID 20181206063552.6701-2-xieyongji@baidu.com (mailing list archive)
State New, archived
Headers show
Series vhost-user-blk: Add support for backend reconnecting | expand

Commit Message

Yongji Xie Dec. 6, 2018, 6:35 a.m. UTC
From: Xie Yongji <xieyongji@baidu.com>

Now we attempt to connect asynchronously for "reconnect socket"
during open(). But vhost-user device prefer a connected socket
during initialization. That means we may still need to support
sync connection during open() for the "reconnect socket".

Signed-off-by: Xie Yongji <xieyongji@baidu.com>
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
---
 chardev/char-socket.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Marc-André Lureau Dec. 6, 2018, 7:23 a.m. UTC | #1
Hi

On Thu, Dec 6, 2018 at 10:38 AM <elohimes@gmail.com> wrote:
>
> From: Xie Yongji <xieyongji@baidu.com>
>
> Now we attempt to connect asynchronously for "reconnect socket"
> during open(). But vhost-user device prefer a connected socket
> during initialization. That means we may still need to support
> sync connection during open() for the "reconnect socket".
>
> Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>

I am not sure this makes much sense, since upon reconnect it won't
"wait" (if I am not mistaken)

> ---
>  chardev/char-socket.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index eaa8e8b68f..f2819d52e7 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1072,7 +1072,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>          s->reconnect_time = reconnect;
>      }
>
> -    if (s->reconnect_time) {
> +    if (s->reconnect_time && !is_waitconnect) {
>          tcp_chr_connect_async(chr);
>      } else {
>          if (s->is_listen) {
> @@ -1120,7 +1120,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
>                                    Error **errp)
>  {
>      bool is_listen      = qemu_opt_get_bool(opts, "server", false);
> -    bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
> +    bool is_waitconnect = is_listen ? qemu_opt_get_bool(opts, "wait", true) :
> +                          qemu_opt_get_bool(opts, "wait", false);
>      bool is_telnet      = qemu_opt_get_bool(opts, "telnet", false);
>      bool is_tn3270      = qemu_opt_get_bool(opts, "tn3270", false);
>      bool is_websock     = qemu_opt_get_bool(opts, "websocket", false);
> --
> 2.17.1
>
>


--
Marc-André Lureau
Yongji Xie Dec. 6, 2018, 7:53 a.m. UTC | #2
On Thu, 6 Dec 2018 at 15:23, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Thu, Dec 6, 2018 at 10:38 AM <elohimes@gmail.com> wrote:
> >
> > From: Xie Yongji <xieyongji@baidu.com>
> >
> > Now we attempt to connect asynchronously for "reconnect socket"
> > during open(). But vhost-user device prefer a connected socket
> > during initialization. That means we may still need to support
> > sync connection during open() for the "reconnect socket".
> >
> > Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> > Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
>
> I am not sure this makes much sense, since upon reconnect it won't
> "wait" (if I am not mistaken)
>

Yes, qemu won't wait when reconnecting. The "wait" here just means that
we should wait connection at first time (during open()). I'm not sure whether
reuse the "wait" option is OK here.

If no this option, current qemu will fail to initialize vhost-user-blk
device when
"reconnect" option is specified no matter the backend is running or not.

Thanks,
Yongji
Yury Kotov Dec. 6, 2018, 9:31 a.m. UTC | #3
Hi,

06.12.2018, 09:37, "elohimes@gmail.com" <elohimes@gmail.com>:
> From: Xie Yongji <xieyongji@baidu.com>
>
> Now we attempt to connect asynchronously for "reconnect socket"
> during open(). But vhost-user device prefer a connected socket
> during initialization. That means we may still need to support
> sync connection during open() for the "reconnect socket".
>
> Signed-off-by: Xie Yongji <xieyongji@baidu.com>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> ---
>  chardev/char-socket.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index eaa8e8b68f..f2819d52e7 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1072,7 +1072,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
>          s->reconnect_time = reconnect;
>      }
>
> - if (s->reconnect_time) {
> + if (s->reconnect_time && !is_waitconnect) {
>          tcp_chr_connect_async(chr);

Does reconnect+wait allow to start QEMU when the first connection was failed?
I think it was the main reason to use tcp_chr_connect_async for reconnect case.

>      } else {
>          if (s->is_listen) {
> @@ -1120,7 +1120,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
>                                    Error **errp)
>  {
>      bool is_listen = qemu_opt_get_bool(opts, "server", false);
> - bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
> + bool is_waitconnect = is_listen ? qemu_opt_get_bool(opts, "wait", true) :
> + qemu_opt_get_bool(opts, "wait", false);
>      bool is_telnet = qemu_opt_get_bool(opts, "telnet", false);
>      bool is_tn3270 = qemu_opt_get_bool(opts, "tn3270", false);
>      bool is_websock = qemu_opt_get_bool(opts, "websocket", false);
> --
> 2.17.1

Regards,
Yury
diff mbox series

Patch

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index eaa8e8b68f..f2819d52e7 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1072,7 +1072,7 @@  static void qmp_chardev_open_socket(Chardev *chr,
         s->reconnect_time = reconnect;
     }
 
-    if (s->reconnect_time) {
+    if (s->reconnect_time && !is_waitconnect) {
         tcp_chr_connect_async(chr);
     } else {
         if (s->is_listen) {
@@ -1120,7 +1120,8 @@  static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
                                   Error **errp)
 {
     bool is_listen      = qemu_opt_get_bool(opts, "server", false);
-    bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
+    bool is_waitconnect = is_listen ? qemu_opt_get_bool(opts, "wait", true) :
+                          qemu_opt_get_bool(opts, "wait", false);
     bool is_telnet      = qemu_opt_get_bool(opts, "telnet", false);
     bool is_tn3270      = qemu_opt_get_bool(opts, "tn3270", false);
     bool is_websock     = qemu_opt_get_bool(opts, "websocket", false);