On Thu, Aug 29, 2024 at 13:56:43 +0200, Markus Armbruster wrote: > Daniil Tatianin <d-tatia...@yandex-team.ru> writes:
[...] So firstly, libvirt models the timeout in the XML in seconds for now so making use of this will require some XML design plumbing making use of it if there'll be any users wanting it. When libvirt would want to make use of this the amount of work for any of the options below is almost the same (capability detection, adding of the new plumbing, etc). The only difference is what to do if nobody shows interest in exposing the sub-second intervals in libvirt. > > @@ -287,7 +292,8 @@ > > '*telnet': 'bool', > > '*tn3270': 'bool', > > '*websocket': 'bool', > > - '*reconnect': 'int' }, > > + '*reconnect': 'int', > > + '*reconnect-is-ms': 'bool' }, > > 'base': 'ChardevCommon' } > > > > ## > > I don't like this interface. > > PRO: compatible extension; no management application updates needed > unless they want to support sub-second delays. > > CON: specifying a sub-second delay takes two parameters, which is > awkward. > > CON: trap in combination with -set. Before the patch, something like > -set chardev.ID.reconnect=N means N seconds no matter what. > Afterwards, it depends on the value of reconnect-is-ms, which may be > set far away. Mitigating factor: -set is obscure. Here we'd have to do nothing. > Alternatives: > > 1. Change @reconnect to 'number', specify sub-second delays as > fractions. > > PRO: compatible extension; no management application updates needed > unless they want to support sub-second delays. > > CON: first use of floating-point for time in seconds in QAPI, as far > as I can see. > > CON: QemuOpts can't do floating-point. Same here. > > 2. Deprecate @reconnect in favour of a new member with a more suitable > unit. Error if both are present. > > PRO: after @reconnect is gone, the interface is what it arguably > should've been from the start. > > CON: incompatible change. Management application updates needed > within the deprecation grace period. This one will require change to libvirt including a capability addition even when libvirt might not want to use the new field. (Add capability detection, switch to new interface if present. Libvirt doesn't want to use deprecated interfaces to avoid future breakage.) But we've done this multiple times so it's not a big deal. > Let's get additional input from management application developers. I > cc'ed some. From libvirt's point of view I'd say either option is viable. We're okay with deprecating the old interface libvirt is used to do that. I'd personally prefer if floating point numbers were avoided.
