Re: [PATCH v1] chardev: introduce 'reconnect-ms' and deprecate 'reconnect'

[Markus Armbruster]

Daniil Tatianin <d-tatianin@yandex-team.ru> writes:

> The 'reconnect' option only allows to specify the time in seconds,
> which is way too long for certain workflows.
>
> We have a lightweight disk backend server, which takes about 20ms to
> live update, but due to this limitation in QEMU, previously the guest
> disk controller would hang for one second because it would take this
> long for QEMU to reinitialize the socket connection.
>
> Introduce a new option called 'reconnect-ms', which is the same as
> 'reconnect', except the value is treated as milliseconds. These are
> mutually exclusive and specifying both results in an error.

Good:

    $ upstream-qemu -nodefaults -chardev 
socket,id=chr0,path=test-hmp,server=off,reconnect=1,reconnect-ms=2
    upstream-qemu: -chardev 
socket,id=chr0,path=test-hmp,server=off,reconnect=1,reconnect-ms=2: 'reconnect' 
and 'reconnect-ms' are mutually exclusive

Bad:

    $ upstream-qemu -nodefaults -S -display none -qmp stdio
    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 9}, 
"package": "v9.1.0-211-ga0866249bd"}, "capabilities": ["oob"]}}
    {"execute": "qmp_capabilities", "arguments": {"enable": ["oob"]}}
    {"return": {}}
    {"execute":"chardev-add", "arguments": {"id":"chr0", "backend": {"type": 
"socket", "data": {"server": false, "addr": {"type": "unix", "data": {"path": 
"xyz"}}, "reconnect": 1, "reconnect-ms": 2}}}}
    {"return": {}}
    upstream-qemu: Unable to connect character device chr0: Failed to connect 
to 'xyz': No such file or directory

We're not rejecting simultaneous use of @reconnect and @reconnect-ms
here.

Moreover, you somehow regressed the handling of the "unable to connect"
error.  Before the patch, behavior is correct:

    $ upstream-qemu -nodefaults -S -display none -qmp stdio
    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 9}, 
"package": "v9.1.0-210-g4b7ea33074"}, "capabilities": ["oob"]}}
    {"execute": "qmp_capabilities", "arguments": {"enable": ["oob"]}}
    {"return": {}}
    {"execute":"chardev-add", "arguments": {"id":"chr0", "backend": {"type": 
"socket", "data": {"server": false, "addr": {"type": "unix", "data": {"path": 
"xyz"}}}}}}
    {"error": {"class": "GenericError", "desc": "Failed to add chardev 'chr0': 
Failed to connect to 'xyz': No such file or directory"}}

> 'reconnect' is also deprecated by this commit to make it possible to
> remove it in the future as to not keep two options that control the
> same thing.
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Acked-by: Peter Krempa <pkrempa@redhat.com>
> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>