| Message ID | 20190605100913.34972-2-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | nbd: enable keepalive | expand |
On 6/5/19 5:09 AM, Vladimir Sementsov-Ogievskiy wrote: > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > include/io/channel.h | 13 +++++++++++++ > io/channel-socket.c | 19 +++++++++++++++++++ > io/channel.c | 14 ++++++++++++++ > 3 files changed, 46 insertions(+) Dan, if you'd like, I can take this through my NBD tree. Reviewed-by: Eric Blake <eblake@redhat.com
On Wed, Jun 05, 2019 at 01:09:12PM +0300, Vladimir Sementsov-Ogievskiy wrote: > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@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
On Wed, Jun 05, 2019 at 09:38:06AM -0500, Eric Blake wrote: > On 6/5/19 5:09 AM, Vladimir Sementsov-Ogievskiy wrote: > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > --- > > include/io/channel.h | 13 +++++++++++++ > > io/channel-socket.c | 19 +++++++++++++++++++ > > io/channel.c | 14 ++++++++++++++ > > 3 files changed, 46 insertions(+) > > Dan, if you'd like, I can take this through my NBD tree. > > Reviewed-by: Eric Blake <eblake@redhat.com There's a couple of small tweaks needed, but if you apply those then consider it to have my Acked-by: Daniel P. Berrangé <berrange@redhat.com> for you to merge directly. Regards, Daniel
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 <vsementsov@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
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 + */ +int qio_channel_set_keepalive(QIOChannel *ioc, + bool enabled, + Error **errp); + /** * qio_channel_close: * @ioc: the channel object 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"); + } + + 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 klass->io_set_keepalive(ioc, enabled, errp); +} + int qio_channel_close(QIOChannel *ioc, Error **errp)
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- include/io/channel.h | 13 +++++++++++++ io/channel-socket.c | 19 +++++++++++++++++++ io/channel.c | 14 ++++++++++++++ 3 files changed, 46 insertions(+)