qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Daniil Tatianin
Subject: Re: [PATCH v1] chardev: introduce 'reconnect-ms' and deprecate 'reconnect'
Date: Fri, 13 Sep 2024 12:00:48 +0300
User-agent: Mozilla Thunderbird

On 9/13/24 11:57 AM, Markus Armbruster wrote:

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"}}

Oh wow, thanks for spotting! I'll be sure to take a look at these.

'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>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]