* [PATCH] qemu-nbd: set timeout to qemu-nbd socket
@ 2022-09-25 13:53 luzhipeng
2022-09-26 10:05 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 6+ messages in thread
From: luzhipeng @ 2022-09-25 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eric Blake, Vladimir Sementsov-Ogievskiy, qemu-block, lu zhipeng
From: lu zhipeng <luzhipeng@cestc.cn>
Prevent the NBD socket stuck all the time, So
set timeout.
Signed-off-by: lu zhipeng <luzhipeng@cestc.cn>
---
nbd/client.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/nbd/client.c b/nbd/client.c
index 30d5383cb1..89dde53a0f 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -24,6 +24,8 @@
#include "nbd-internal.h"
#include "qemu/cutils.h"
+#define NBD_DEFAULT_TIMEOUT 30
+
/* Definitions for opaque data types */
static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
@@ -1301,6 +1303,12 @@ int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,
}
}
+ if (ioctl(fd, NBD_SET_TIMEOUT, NBD_DEFAULT_TIMEOUT) < 0) {
+ int serrno = errno;
+ error_setg(errp, "Failed setting timeout");
+ return -serrno;
+ }
+
trace_nbd_init_finish();
return 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] qemu-nbd: set timeout to qemu-nbd socket
2022-09-25 13:53 [PATCH] qemu-nbd: set timeout to qemu-nbd socket luzhipeng
@ 2022-09-26 10:05 ` Vladimir Sementsov-Ogievskiy
2022-09-26 11:34 ` Denis V. Lunev
0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-09-26 10:05 UTC (permalink / raw)
To: luzhipeng, qemu-devel; +Cc: Eric Blake, qemu-block, Denis V. Lunev
[+ Den]
On 9/25/22 16:53, luzhipeng wrote:
> From: lu zhipeng <luzhipeng@cestc.cn>
>
> Prevent the NBD socket stuck all the time, So
> set timeout.
>
> Signed-off-by: lu zhipeng <luzhipeng@cestc.cn>
> ---
> nbd/client.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index 30d5383cb1..89dde53a0f 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -24,6 +24,8 @@
> #include "nbd-internal.h"
> #include "qemu/cutils.h"
>
> +#define NBD_DEFAULT_TIMEOUT 30
> +
> /* Definitions for opaque data types */
>
> static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
> @@ -1301,6 +1303,12 @@ int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,
> }
> }
>
> + if (ioctl(fd, NBD_SET_TIMEOUT, NBD_DEFAULT_TIMEOUT) < 0) {
> + int serrno = errno;
> + error_setg(errp, "Failed setting timeout");
> + return -serrno;
> + }
> +
> trace_nbd_init_finish();
>
> return 0;
Personally, I don't see a problem in enabling timeout by default.. But probably we need a new option instead?
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qemu-nbd: set timeout to qemu-nbd socket
2022-09-26 10:05 ` Vladimir Sementsov-Ogievskiy
@ 2022-09-26 11:34 ` Denis V. Lunev
2022-09-26 12:44 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 6+ messages in thread
From: Denis V. Lunev @ 2022-09-26 11:34 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, luzhipeng, qemu-devel
Cc: Eric Blake, qemu-block
On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
> [+ Den]
>
> On 9/25/22 16:53, luzhipeng wrote:
>> From: lu zhipeng <luzhipeng@cestc.cn>
>>
>> Prevent the NBD socket stuck all the time, So
>> set timeout.
>>
>> Signed-off-by: lu zhipeng <luzhipeng@cestc.cn>
>> ---
>> nbd/client.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/nbd/client.c b/nbd/client.c
>> index 30d5383cb1..89dde53a0f 100644
>> --- a/nbd/client.c
>> +++ b/nbd/client.c
>> @@ -24,6 +24,8 @@
>> #include "nbd-internal.h"
>> #include "qemu/cutils.h"
>> +#define NBD_DEFAULT_TIMEOUT 30
>> +
>> /* Definitions for opaque data types */
>> static QTAILQ_HEAD(, NBDExport) exports =
>> QTAILQ_HEAD_INITIALIZER(exports);
>> @@ -1301,6 +1303,12 @@ int nbd_init(int fd, QIOChannelSocket *sioc,
>> NBDExportInfo *info,
>> }
>> }
>> + if (ioctl(fd, NBD_SET_TIMEOUT, NBD_DEFAULT_TIMEOUT) < 0) {
>> + int serrno = errno;
>> + error_setg(errp, "Failed setting timeout");
>> + return -serrno;
>> + }
>> +
>> trace_nbd_init_finish();
>> return 0;
>
>
> Personally, I don't see a problem in enabling timeout by default.. But
> probably we need a new option instead?
>
>
I believe that this should be the same story as we have had with
KEEPALIVE. This should be set as an option and downstream
will change its default when necessary.
Den
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qemu-nbd: set timeout to qemu-nbd socket
2022-09-26 11:34 ` Denis V. Lunev
@ 2022-09-26 12:44 ` Vladimir Sementsov-Ogievskiy
2022-09-27 3:20 ` luzhipeng
2022-09-27 6:16 ` luzhipeng
0 siblings, 2 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-09-26 12:44 UTC (permalink / raw)
To: Denis V. Lunev, luzhipeng, qemu-devel; +Cc: Eric Blake, qemu-block
On 9/26/22 14:34, Denis V. Lunev wrote:
> On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
>> [+ Den]
>>
>> On 9/25/22 16:53, luzhipeng wrote:
>>> From: lu zhipeng <luzhipeng@cestc.cn>
>>>
>>> Prevent the NBD socket stuck all the time, So
>>> set timeout.
>>>
>>> Signed-off-by: lu zhipeng <luzhipeng@cestc.cn>
>>> ---
>>> nbd/client.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/nbd/client.c b/nbd/client.c
>>> index 30d5383cb1..89dde53a0f 100644
>>> --- a/nbd/client.c
>>> +++ b/nbd/client.c
>>> @@ -24,6 +24,8 @@
>>> #include "nbd-internal.h"
>>> #include "qemu/cutils.h"
>>> +#define NBD_DEFAULT_TIMEOUT 30
>>> +
>>> /* Definitions for opaque data types */
>>> static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
>>> @@ -1301,6 +1303,12 @@ int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,
>>> }
>>> }
>>> + if (ioctl(fd, NBD_SET_TIMEOUT, NBD_DEFAULT_TIMEOUT) < 0) {
>>> + int serrno = errno;
>>> + error_setg(errp, "Failed setting timeout");
>>> + return -serrno;
>>> + }
>>> +
>>> trace_nbd_init_finish();
>>> return 0;
>>
>>
>> Personally, I don't see a problem in enabling timeout by default.. But probably we need a new option instead?
>>
>>
> I believe that this should be the same story as we have had with
> KEEPALIVE. This should be set as an option and downstream
> will change its default when necessary.
>
It's also interesting, how NBD_SET_TIMEOUT would interfere with keep-alive options set on the socket. Isn't existing keep-alive option already enough, do we need both timeouts?
(and yes, if we need both ways for different cases, we definitely should keep a possibility for the user to enable only one timeout, so now I agree, that we need an option for this new feature)
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qemu-nbd: set timeout to qemu-nbd socket
2022-09-26 12:44 ` Vladimir Sementsov-Ogievskiy
@ 2022-09-27 3:20 ` luzhipeng
2022-09-27 6:16 ` luzhipeng
1 sibling, 0 replies; 6+ messages in thread
From: luzhipeng @ 2022-09-27 3:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, Denis V. Lunev, qemu-devel
Cc: Eric Blake, qemu-block
在 2022/9/26 20:44, Vladimir Sementsov-Ogievskiy 写道:
> On 9/26/22 14:34, Denis V. Lunev wrote:
>> On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
>>> [+ Den]
>>>
>>> On 9/25/22 16:53, luzhipeng wrote:
>>>> From: lu zhipeng <luzhipeng@cestc.cn>
>>>>
>>>> Prevent the NBD socket stuck all the time, So
>>>> set timeout.
>>>>
>>>> Signed-off-by: lu zhipeng <luzhipeng@cestc.cn>
>>>> ---
>>>> nbd/client.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/nbd/client.c b/nbd/client.c
>>>> index 30d5383cb1..89dde53a0f 100644
>>>> --- a/nbd/client.c
>>>> +++ b/nbd/client.c
>>>> @@ -24,6 +24,8 @@
>>>> #include "nbd-internal.h"
>>>> #include "qemu/cutils.h"
>>>> +#define NBD_DEFAULT_TIMEOUT 30
>>>> +
>>>> /* Definitions for opaque data types */
>>>> static QTAILQ_HEAD(, NBDExport) exports =
>>>> QTAILQ_HEAD_INITIALIZER(exports);
>>>> @@ -1301,6 +1303,12 @@ int nbd_init(int fd, QIOChannelSocket *sioc,
>>>> NBDExportInfo *info,
>>>> }
>>>> }
>>>> + if (ioctl(fd, NBD_SET_TIMEOUT, NBD_DEFAULT_TIMEOUT) < 0) {
>>>> + int serrno = errno;
>>>> + error_setg(errp, "Failed setting timeout");
>>>> + return -serrno;
>>>> + }
>>>> +
>>>> trace_nbd_init_finish();
>>>> return 0;
>>>
>>>
>>> Personally, I don't see a problem in enabling timeout by default..
>>> But probably we need a new option instead?
>>>
>>>
>> I believe that this should be the same story as we have had with
>> KEEPALIVE. This should be set as an option and downstream
>> will change its default when necessary.
>>
>
> It's also interesting, how NBD_SET_TIMEOUT would interfere with
> keep-alive options set on the socket. Isn't existing keep-alive option
> already enough, do we need both timeouts?
>
> (and yes, if we need both ways for different cases, we definitely should
> keep a possibility for the user to enable only one timeout, so now I
> agree, that we need an option for this new feature)
>
>
keep-alive is only valid for tcp mode, unix domain is not valid
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] qemu-nbd: set timeout to qemu-nbd socket
2022-09-26 12:44 ` Vladimir Sementsov-Ogievskiy
2022-09-27 3:20 ` luzhipeng
@ 2022-09-27 6:16 ` luzhipeng
1 sibling, 0 replies; 6+ messages in thread
From: luzhipeng @ 2022-09-27 6:16 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, Denis V. Lunev, qemu-devel
Cc: Eric Blake, qemu-block
在 2022/9/26 20:44, Vladimir Sementsov-Ogievskiy 写道:
> On 9/26/22 14:34, Denis V. Lunev wrote:
>> On 9/26/22 12:05, Vladimir Sementsov-Ogievskiy wrote:
>>> [+ Den]
>>>
>>> On 9/25/22 16:53, luzhipeng wrote:
>>>> From: lu zhipeng <luzhipeng@cestc.cn>
>>>>
>>>> Prevent the NBD socket stuck all the time, So
>>>> set timeout.
>>>>
>>>> Signed-off-by: lu zhipeng <luzhipeng@cestc.cn>
>>>> ---
>>>> nbd/client.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/nbd/client.c b/nbd/client.c
>>>> index 30d5383cb1..89dde53a0f 100644
>>>> --- a/nbd/client.c
>>>> +++ b/nbd/client.c
>>>> @@ -24,6 +24,8 @@
>>>> #include "nbd-internal.h"
>>>> #include "qemu/cutils.h"
>>>> +#define NBD_DEFAULT_TIMEOUT 30
>>>> +
>>>> /* Definitions for opaque data types */
>>>> static QTAILQ_HEAD(, NBDExport) exports =
>>>> QTAILQ_HEAD_INITIALIZER(exports);
>>>> @@ -1301,6 +1303,12 @@ int nbd_init(int fd, QIOChannelSocket *sioc,
>>>> NBDExportInfo *info,
>>>> }
>>>> }
>>>> + if (ioctl(fd, NBD_SET_TIMEOUT, NBD_DEFAULT_TIMEOUT) < 0) {
>>>> + int serrno = errno;
>>>> + error_setg(errp, "Failed setting timeout");
>>>> + return -serrno;
>>>> + }
>>>> +
>>>> trace_nbd_init_finish();
>>>> return 0;
>>>
>>>
>>> Personally, I don't see a problem in enabling timeout by default..
>>> But probably we need a new option instead?
>>>
>>>
>> I believe that this should be the same story as we have had with
>> KEEPALIVE. This should be set as an option and downstream
>> will change its default when necessary.
>>
>
> It's also interesting, how NBD_SET_TIMEOUT would interfere with
> keep-alive options set on the socket. Isn't existing keep-alive option
> already enough, do we need both timeouts?
>
> (and yes, if we need both ways for different cases, we definitely should
> keep a possibility for the user to enable only one timeout, so now I
> agree, that we need an option for this new feature)
Keep alive is only valid for tcp, but not for unix sockets
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-27 6:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-25 13:53 [PATCH] qemu-nbd: set timeout to qemu-nbd socket luzhipeng
2022-09-26 10:05 ` Vladimir Sementsov-Ogievskiy
2022-09-26 11:34 ` Denis V. Lunev
2022-09-26 12:44 ` Vladimir Sementsov-Ogievskiy
2022-09-27 3:20 ` luzhipeng
2022-09-27 6:16 ` luzhipeng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.