qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 2/2] nbd-client: enable TCP keepalive


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [PATCH 2/2] nbd-client: enable TCP keepalive
Date: Wed, 5 Jun 2019 17:28:05 +0000

05.06.2019 20:12, Eric Blake wrote:
> On 6/5/19 12:05 PM, Vladimir Sementsov-Ogievskiy wrote:
> 
>>> By enabling TCP keepalives we are explicitly making the connection
>>> less reliable by forcing it to be terminated when keepalive
>>> threshold triggers, instead of waiting longer for TCP to recover.
>>>
>>> The rationale s that once a connection has been in a hung state for
>>> so long that keepalive triggers, its (hopefully) not useful to the
>>> mgmt app to carry on waiting anyway.
>>>
>>> If the connection is terminated by keepalive & the mgmt app then
>>> spawns a new client to carry on with the work, what are the risks
>>> involved ? eg Could packets from the stuck, terminated, connection
>>> suddenly arrive later and trigger I/O with outdated data payload ?
>>
>> Hmm, I believe that tcp guarantees isolation between different connections
>>
>>>
>>> I guess this is no different a situation from an app explicitly
>>> killing the QEMU NBD client process instead & spawning a new one.
>>>
>>> I'm still feeling a little uneasy about enabling it unconditionally
>>> though, since pretty much everything I know which supports keepalives
>>> has a way to turn them on/off at least, even if you can't tune the
>>> individual timer settings.
>>
>> Hm. So, I can add bool keepalive parameter for nbd format with default to 
>> true.
>> And if needed, it may be later extended to be qapi 'alternate' of bool or 
>> struct with
>> three numeric parameters, corresponding to TCP_KEEPCNT, TCP_KEEPIDLE and 
>> TCP_KEEPINTVL .
>>
>> Opinions?
> 
> Adding a bool that could later turn into a qapi 'alternate' for
> fine-tuning seems reasonable. Defaulting the bool to true is not
> backwards-compatible; better would be defaulting it to false and letting
> users opt-in; introspection will also work to let you know whether the
> feature is present.
> 

Ok.

One more thing to discuss then. Should I add keepalive directly to 
BlockdevOptionsNbd?

Seems more useful to put it into SocketAddress, to be reused by other socket 
users..
But "SocketAddress" sounds like address, not like address+connection-options. On
the other hand, structure names are not part of API. So, finally, is 
InetSocketAddress
a good place for such thing?

-- 
Best regards,
Vladimir

reply via email to

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