05.06.2019 19:02, Daniel P. Berrangé wrote:
> On Wed, Jun 05, 2019 at 01:09:12PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>> ---
>> include/io/channel.h | 13 +++++++++++++
>> io/channel-socket.c | 19 +++++++++++++++++++
>> io/channel.c | 14 ++++++++++++++
>> 3 files changed, 46 insertions(+)
>>
>> diff --git a/include/io/channel.h b/include/io/channel.h
>> index 59460cb1ec..34d871a414 100644
>> --- a/include/io/channel.h
>> +++ b/include/io/channel.h
>> @@ -124,6 +124,9 @@ struct QIOChannelClass {
>> int (*io_set_blocking)(QIOChannel *ioc,
>> bool enabled,
>> Error **errp);
>> + int (*io_set_keepalive)(QIOChannel *ioc,
>> + bool enabled,
>> + Error **errp);
>>
>> /* Optional callbacks */
>> int (*io_shutdown)(QIOChannel *ioc,
>> @@ -490,6 +493,16 @@ int qio_channel_set_blocking(QIOChannel *ioc,
>> bool enabled,
>> Error **errp);
>>
>> +/*
>> + * qio_channel_set_keepalive:
>> + * @ioc: the channel object
>> + * @enabled: the keepalive flag state
>> + * @errp: pointer to a NULL-initialized error object
>> + */
>
> Missing docs for the return value. SHould be
>
> "Returns 0 on success, -1 on error."
>
> note we do *not* return "-errno" values in QIOChannel APIs
>
>> +int qio_channel_set_keepalive(QIOChannel *ioc,
>> + bool enabled,
>> + Error **errp);
>> +
>
>> diff --git a/io/channel-socket.c b/io/channel-socket.c
>> index bc5f80e780..5c1ea08660 100644
>> --- a/io/channel-socket.c
>> +++ b/io/channel-socket.c
>> @@ -656,6 +656,24 @@ qio_channel_socket_set_blocking(QIOChannel *ioc,
>> }
>>
>>
>> +static int
>> +qio_channel_socket_set_keepalive(QIOChannel *ioc,
>> + bool enabled,
>> + Error **errp)
>> +{
>> + QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
>> + int val = enabled;
>> + int ret = qemu_setsockopt(sioc->fd, SOL_SOCKET, SO_KEEPALIVE,
>> + &val, sizeof(val));
>> +
>> + if (ret < 0) {
>> + error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
>
> Add 'return -1' there to be explicit, avoiding need to read up on
> whether qemu_setsockopt returns -1 or -errno. (It returns -1)
>
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +
>> static void
>> qio_channel_socket_set_delay(QIOChannel *ioc,
>> bool enabled)
>> @@ -762,6 +780,7 @@ static void qio_channel_socket_class_init(ObjectClass
>> *klass,
>> ioc_klass->io_writev = qio_channel_socket_writev;
>> ioc_klass->io_readv = qio_channel_socket_readv;
>> ioc_klass->io_set_blocking = qio_channel_socket_set_blocking;
>> + ioc_klass->io_set_keepalive = qio_channel_socket_set_keepalive;
>> ioc_klass->io_close = qio_channel_socket_close;
>> ioc_klass->io_shutdown = qio_channel_socket_shutdown;
>> ioc_klass->io_set_cork = qio_channel_socket_set_cork;
>> diff --git a/io/channel.c b/io/channel.c
>> index 2a26c2a2c0..0f0b2b7b65 100644
>> --- a/io/channel.c
>> +++ b/io/channel.c
>> @@ -265,6 +265,20 @@ int qio_channel_set_blocking(QIOChannel *ioc,
>> return klass->io_set_blocking(ioc, enabled, errp);
>> }
>>
>> +int qio_channel_set_keepalive(QIOChannel *ioc,
>> + bool enabled,
>> + Error **errp)
>> +{
>> + QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
>> +
>> + if (!klass->io_set_keepalive) {
>> + error_setg(errp, "KEEPALIVE is not supported by IO channel");
>> + return -ENOTSUP;
>
> return -1;
>
>> + }
>> +
>> + return klass->io_set_keepalive(ioc, enabled, errp);
>> +}
>> +
>>
>> int qio_channel_close(QIOChannel *ioc,
>> Error **errp)
>
> Regards,
> Daniel
>
Thank you, I'll resend
--
Best regards,
Vladimir
