* Re: [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices
2015-07-21 7:34 [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices Paolo Bonzini
@ 2015-07-21 8:10 ` Peter Maydell
2015-07-21 8:16 ` Paolo Bonzini
2015-07-21 8:36 ` Amit Shah
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2015-07-21 8:10 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Amit Shah, QEMU Developers
On 21 July 2015 at 08:34, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index b2b43c5..d956f8d 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2798,7 +2798,10 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> #ifdef MSG_CMSG_CLOEXEC
> flags |= MSG_CMSG_CLOEXEC;
> #endif
> - ret = recvmsg(s->fd, &msg, flags);
> + do {
> + ret = recvmsg(s->fd, &msg, flags);
> + } while (ret == -1 && errno == EINTR);
> +
> if (ret > 0 && s->is_unix) {
> unix_process_msgfd(chr, &msg);
> }
> @@ -2809,7 +2812,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> {
> TCPCharDriver *s = chr->opaque;
> - return qemu_recv(s->fd, buf, len, 0);
> + ssize_t ret;
> +
> + do {
> + ret = qemu_recv(s->fd, buf, len, 0);
> + } while (ret == -1 && socket_error() == EINTR);
> +
> + return ret;
> }
...can you get EINTR on Windows sockets?
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices
2015-07-21 8:10 ` Peter Maydell
@ 2015-07-21 8:16 ` Paolo Bonzini
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2015-07-21 8:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: Amit Shah, QEMU Developers
On 21/07/2015 10:10, Peter Maydell wrote:
> On 21 July 2015 at 08:34, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> qemu-char.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index b2b43c5..d956f8d 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -2798,7 +2798,10 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
>> #ifdef MSG_CMSG_CLOEXEC
>> flags |= MSG_CMSG_CLOEXEC;
>> #endif
>> - ret = recvmsg(s->fd, &msg, flags);
>> + do {
>> + ret = recvmsg(s->fd, &msg, flags);
>> + } while (ret == -1 && errno == EINTR);
>> +
>> if (ret > 0 && s->is_unix) {
>> unix_process_msgfd(chr, &msg);
>> }
>> @@ -2809,7 +2812,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
>> static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
>> {
>> TCPCharDriver *s = chr->opaque;
>> - return qemu_recv(s->fd, buf, len, 0);
>> + ssize_t ret;
>> +
>> + do {
>> + ret = qemu_recv(s->fd, buf, len, 0);
>> + } while (ret == -1 && socket_error() == EINTR);
>> +
>> + return ret;
>> }
>
> ...can you get EINTR on Windows sockets?
No idea, but WSAEINTR exists and even has some hits on Google. :)
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices
2015-07-21 7:34 [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices Paolo Bonzini
2015-07-21 8:10 ` Peter Maydell
@ 2015-07-21 8:36 ` Amit Shah
2015-07-21 8:45 ` Nils Carlson
2015-07-21 8:51 ` Thomas Huth
3 siblings, 0 replies; 7+ messages in thread
From: Amit Shah @ 2015-07-21 8:36 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On (Tue) 21 Jul 2015 [09:34:29], Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Amit
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices
2015-07-21 7:34 [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices Paolo Bonzini
2015-07-21 8:10 ` Peter Maydell
2015-07-21 8:36 ` Amit Shah
@ 2015-07-21 8:45 ` Nils Carlson
2015-07-21 8:51 ` Thomas Huth
3 siblings, 0 replies; 7+ messages in thread
From: Nils Carlson @ 2015-07-21 8:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: amit.shah, qemu-devel
Agree, this is more elegant, we keep the recv in a tight loop.
Acked-by: Nils Carlson <pyssling@ludd.ltu.se>
On Tue, 21 Jul 2015, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index b2b43c5..d956f8d 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2798,7 +2798,10 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> #ifdef MSG_CMSG_CLOEXEC
> flags |= MSG_CMSG_CLOEXEC;
> #endif
> - ret = recvmsg(s->fd, &msg, flags);
> + do {
> + ret = recvmsg(s->fd, &msg, flags);
> + } while (ret == -1 && errno == EINTR);
> +
> if (ret > 0 && s->is_unix) {
> unix_process_msgfd(chr, &msg);
> }
> @@ -2809,7 +2812,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> {
> TCPCharDriver *s = chr->opaque;
> - return qemu_recv(s->fd, buf, len, 0);
> + ssize_t ret;
> +
> + do {
> + ret = qemu_recv(s->fd, buf, len, 0);
> + } while (ret == -1 && socket_error() == EINTR);
> +
> + return ret;
> }
> #endif
>
> --
> 2.4.3
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices
2015-07-21 7:34 [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices Paolo Bonzini
` (2 preceding siblings ...)
2015-07-21 8:45 ` Nils Carlson
@ 2015-07-21 8:51 ` Thomas Huth
2015-07-21 8:55 ` Paolo Bonzini
3 siblings, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2015-07-21 8:51 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: amit.shah
On 21/07/15 09:34, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index b2b43c5..d956f8d 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2798,7 +2798,10 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> #ifdef MSG_CMSG_CLOEXEC
> flags |= MSG_CMSG_CLOEXEC;
> #endif
> - ret = recvmsg(s->fd, &msg, flags);
> + do {
> + ret = recvmsg(s->fd, &msg, flags);
> + } while (ret == -1 && errno == EINTR);
> +
> if (ret > 0 && s->is_unix) {
> unix_process_msgfd(chr, &msg);
> }
I think you could also use the TFR() macro from include/qemu-common.h
here instead (TFR = temp. failure retry, I think)
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] qemu-char: handle EINTR for TCP character devices
2015-07-21 8:51 ` Thomas Huth
@ 2015-07-21 8:55 ` Paolo Bonzini
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2015-07-21 8:55 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: amit.shah
On 21/07/2015 10:51, Thomas Huth wrote:
> On 21/07/15 09:34, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> qemu-char.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index b2b43c5..d956f8d 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -2798,7 +2798,10 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
>> #ifdef MSG_CMSG_CLOEXEC
>> flags |= MSG_CMSG_CLOEXEC;
>> #endif
>> - ret = recvmsg(s->fd, &msg, flags);
>> + do {
>> + ret = recvmsg(s->fd, &msg, flags);
>> + } while (ret == -1 && errno == EINTR);
>> +
>> if (ret > 0 && s->is_unix) {
>> unix_process_msgfd(chr, &msg);
>> }
>
> I think you could also use the TFR() macro from include/qemu-common.h
> here instead (TFR = temp. failure retry, I think)
Interesting macro. :) I wonder why it's being used only for open (and
qemu_open), though.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread