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

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

 



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

Daniil Tatianin <d-tatianin@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
Acked-by: Peter Krempa <pkrempa@xxxxxxxxxx>
Signed-off-by: Daniil Tatianin <d-tatianin@xxxxxxxxxxxxxx>



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux