[Qemu-devel] [PATCH 1/3] chardev: prevent extra connection attempt in tcp_chr_machine_done_hook

Yury Kotov posted 3 patches 7 years, 2 months ago
[Qemu-devel] [PATCH 1/3] chardev: prevent extra connection attempt in tcp_chr_machine_done_hook
Posted by Yury Kotov 7 years, 2 months ago
Usually chardev connects to specified address during the initialization.
But if reconnect_time is specified then connection will be postponed until
machine done event.

Thus if reconnect is specified and some device forces connection during
initialization, tcp_chr_machine_done_hook will do useless connection attempt.

So add a check to prevent it.

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
Signed-off-by: Evgeny Yakovlev <wrfsh@yandex-team.ru>
---
 chardev/char-socket.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index efbad6e..116dcc4 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1165,7 +1165,10 @@ static int tcp_chr_machine_done_hook(Chardev *chr)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
 
-    if (s->reconnect_time) {
+    /* It's possible that connection was established during the device
+     * initialization. So check if the socket is already connected to
+     * prevent extra connection attempt. */
+    if (!s->connected && s->reconnect_time) {
         tcp_chr_connect_async(chr);
     }
 
-- 
2.7.4


Re: [Qemu-devel] [PATCH 1/3] chardev: prevent extra connection attempt in tcp_chr_machine_done_hook
Posted by Marc-André Lureau 7 years, 2 months ago
Hi

On Thu, Aug 16, 2018 at 5:32 PM, Yury Kotov <yury-kotov@yandex-team.ru> wrote:
> Usually chardev connects to specified address during the initialization.
> But if reconnect_time is specified then connection will be postponed until
> machine done event.
>
> Thus if reconnect is specified and some device forces connection during
> initialization, tcp_chr_machine_done_hook will do useless connection attempt.
>
> So add a check to prevent it.
>
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Signed-off-by: Evgeny Yakovlev <wrfsh@yandex-team.ru>

It would be better with a test case in tests/test-char.c, but lgtm,

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  chardev/char-socket.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index efbad6e..116dcc4 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -1165,7 +1165,10 @@ static int tcp_chr_machine_done_hook(Chardev *chr)
>  {
>      SocketChardev *s = SOCKET_CHARDEV(chr);
>
> -    if (s->reconnect_time) {
> +    /* It's possible that connection was established during the device
> +     * initialization. So check if the socket is already connected to
> +     * prevent extra connection attempt. */
> +    if (!s->connected && s->reconnect_time) {
>          tcp_chr_connect_async(chr);
>      }
>
> --
> 2.7.4
>