Re: [PATCH 1/2] vchan-socket-proxy: add reconnect marker support

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Mon, Sep 5, 2022 at 9:50 AM Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> When vchan client reconnect quickly, the server may not notice it. This
> means, it won't reconnect the UNIX socket either. For QMP, it will
> prevent the client to see the QMP protocol handshake, and the
> communication will timeout.
> Solve the issue by sending in-band connect marker. Whenever server sees
> one (elsewhere than the first byte in the connection), handle it as a
> client had reconnected. The marker is a one byte, and the user need to
> choose something that doesn't appear in the data stream elsewhere.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Jason Andryuk <jandryuk@xxxxxxxxx>

Here are some of my thoughts on the correctness of this approach:
The QMP data is json - ascii (utf-8?) - so using \x01 is fine as it
won't be in the data stream.  The dropping of any partial message
works since the close and re-open of the QMP socket resets the data
stream.  There shouldn't be any partial data in vchan-socket-proxy to
drop since the client side should not have sent any new data before it
closed and re-opened.  If there is partial data, we shouldn't send it
into QEMU since the new QMP client isn't expecting any response it
would generate.  So that all seems okay.

Regards,
Jason