1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 0/2] nbd: enable keepalive
  4. View patch

[Qemu-devel] [PATCH 1/2] io/channel: add qio_channel_set_keepalive

Vladimir Sementsov-Ogievskiy posted 2 patches 6 years, 8 months ago
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eric Blake <eblake@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH 1/2] io/channel: add qio_channel_set_keepalive
Posted by Vladimir Sementsov-Ogievskiy 6 years, 8 months ago 
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
+ */
+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)
-- 
2.18.0
Re: [Qemu-devel] [PATCH 1/2] io/channel: add qio_channel_set_keepalive
Posted by Eric Blake 6 years, 8 months ago 
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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 1/2] io/channel: add qio_channel_set_keepalive
Posted by Daniel P. Berrangé 6 years, 8 months ago 
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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 1/2] io/channel: add qio_channel_set_keepalive
Posted by Daniel P. Berrangé 6 years, 8 months ago 
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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 1/2] io/channel: add qio_channel_set_keepalive
Posted by Vladimir Sementsov-Ogievskiy 6 years, 8 months ago 
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

-- 
Best regards,
Vladimir

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.