Re: [PATCH v1] chardev: introduce 'reconnect-ms' and deprecate 'reconnect'
Daniil Tatianin <d-tatianin(a)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(a)yandex-team.ru>
Acked-by: Peter Krempa <pkrempa(a)redhat.com>
Signed-off-by: Daniil Tatianin <d-tatianin(a)yandex-team.ru>