* [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
@ 2018-05-29 2:52 linzhecheng
2018-05-29 9:11 ` Marc-André Lureau
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: linzhecheng @ 2018-05-29 2:52 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, wangxinxin.wang, linzhecheng
Signed-off-by: linzhecheng <linzhecheng@huawei.com>
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 159e69c3b1..17519ec589 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
s->write_msgfds,
s->write_msgfds_num);
- /* free the written msgfds, no matter what */
- if (s->write_msgfds_num) {
+ /* free the written msgfds in any cases other than errno==EAGAIN */
+ if (EAGAIN != errno && s->write_msgfds_num) {
g_free(s->write_msgfds);
s->write_msgfds = 0;
s->write_msgfds_num = 0;
--
2.12.2.windows.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
2018-05-29 2:52 [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN linzhecheng
@ 2018-05-29 9:11 ` Marc-André Lureau
2018-05-30 2:56 ` Peter Xu
2018-06-01 11:46 ` Daniel P. Berrangé
2 siblings, 0 replies; 9+ messages in thread
From: Marc-André Lureau @ 2018-05-29 9:11 UTC (permalink / raw)
To: linzhecheng; +Cc: QEMU, Paolo Bonzini, wangxinxin.wang
Hi
On Tue, May 29, 2018 at 4:52 AM, linzhecheng <linzhecheng@huawei.com> wrote:
> Signed-off-by: linzhecheng <linzhecheng@huawei.com>
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 159e69c3b1..17519ec589 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
> s->write_msgfds,
> s->write_msgfds_num);
>
> - /* free the written msgfds, no matter what */
> - if (s->write_msgfds_num) {
> + /* free the written msgfds in any cases other than errno==EAGAIN */
> + if (EAGAIN != errno && s->write_msgfds_num) {
> g_free(s->write_msgfds);
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
> --
It is already fix since v2.12:
commit c863fdec6aff6b5a4ca8fff1537b80d9f8b97726
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Thu Feb 22 12:13:51 2018 +0000
chardev: fix handling of EAGAIN for TCP chardev
> 2.12.2.windows.2
>
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
2018-05-29 2:52 [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN linzhecheng
2018-05-29 9:11 ` Marc-André Lureau
@ 2018-05-30 2:56 ` Peter Xu
2018-06-01 11:46 ` Daniel P. Berrangé
2 siblings, 0 replies; 9+ messages in thread
From: Peter Xu @ 2018-05-30 2:56 UTC (permalink / raw)
To: linzhecheng; +Cc: qemu-devel, pbonzini, wangxinxin.wang
On Tue, May 29, 2018 at 10:52:40AM +0800, linzhecheng wrote:
> Signed-off-by: linzhecheng <linzhecheng@huawei.com>
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 159e69c3b1..17519ec589 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
> s->write_msgfds,
> s->write_msgfds_num);
>
> - /* free the written msgfds, no matter what */
> - if (s->write_msgfds_num) {
> + /* free the written msgfds in any cases other than errno==EAGAIN */
> + if (EAGAIN != errno && s->write_msgfds_num) {
This makes sense to me.
Meanwhile this reminds me that we didn't release the fds in
tcp_chr_disconnect(). I'm thinking whether we should do that no
matter what. E.g., what if we set_msgfds() then another operation
instead of write() failed?
Then here we can only release the msgfds when write succeeded...
if (ret >= 0 && s->write_msgfds_num) {
...as long as we'll also cleanup the msgfds in tcp_chr_disconnect()
always.
I'll see how Marc-Andre and Paolo see this though.
Thanks,
> g_free(s->write_msgfds);
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
> --
> 2.12.2.windows.2
>
>
>
--
Peter Xu
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
2018-05-29 2:52 [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN linzhecheng
2018-05-29 9:11 ` Marc-André Lureau
2018-05-30 2:56 ` Peter Xu
@ 2018-06-01 11:46 ` Daniel P. Berrangé
2 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2018-06-01 11:46 UTC (permalink / raw)
To: linzhecheng; +Cc: qemu-devel, pbonzini, wangxinxin.wang
On Tue, May 29, 2018 at 10:52:40AM +0800, linzhecheng wrote:
> Signed-off-by: linzhecheng <linzhecheng@huawei.com>
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 159e69c3b1..17519ec589 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
> s->write_msgfds,
> s->write_msgfds_num);
>
> - /* free the written msgfds, no matter what */
> - if (s->write_msgfds_num) {
> + /* free the written msgfds in any cases other than errno==EAGAIN */
> + if (EAGAIN != errno && s->write_msgfds_num) {
> g_free(s->write_msgfds);
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
Thanks, queued with an updated commit message
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 :|
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
2018-05-29 19:33 ` Eric Blake
@ 2018-05-30 9:05 ` Gonglei (Arei)
0 siblings, 0 replies; 9+ messages in thread
From: Gonglei (Arei) @ 2018-05-30 9:05 UTC (permalink / raw)
To: Eric Blake, linzhecheng, Marc-André Lureau
Cc: QEMU, Paolo Bonzini, wangxin (U), peterx, berrange
> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Wednesday, May 30, 2018 3:33 AM
> To: linzhecheng <linzhecheng@huawei.com>; Marc-André Lureau
> <marcandre.lureau@gmail.com>
> Cc: QEMU <qemu-devel@nongnu.org>; Paolo Bonzini <pbonzini@redhat.com>;
> wangxin (U) <wangxinxin.wang@huawei.com>; Gonglei (Arei)
> <arei.gonglei@huawei.com>; peterx@redhat.com; berrange@redhat.com
> Subject: Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals
> EAGAIN
>
> On 05/29/2018 04:33 AM, linzhecheng wrote:
> > I think this patch doesn't fix my issue. For more details, please see Gonglei's
> reply.
> > https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06296.html
>
> Your mailer is not honoring threading (it failed to include
> 'In-Reply-To:' and 'References:' headers that refer to the message you
> are replying to), and you are top-posting, both of which make it
> difficult to follow your comments on a technical list.
>
>
Agree.
@Zhecheng, pls resend a patch with commit message. Ccing these guys.
Regards,
-Gonglei
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
2018-05-29 9:33 linzhecheng
@ 2018-05-29 19:33 ` Eric Blake
2018-05-30 9:05 ` Gonglei (Arei)
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2018-05-29 19:33 UTC (permalink / raw)
To: linzhecheng, Marc-André Lureau
Cc: QEMU, Paolo Bonzini, wangxin (U), Gonglei (Arei), peterx, berrange
On 05/29/2018 04:33 AM, linzhecheng wrote:
> I think this patch doesn't fix my issue. For more details, please see Gonglei's reply.
> https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06296.html
Your mailer is not honoring threading (it failed to include
'In-Reply-To:' and 'References:' headers that refer to the message you
are replying to), and you are top-posting, both of which make it
difficult to follow your comments on a technical list.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
@ 2018-05-29 9:33 linzhecheng
2018-05-29 19:33 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: linzhecheng @ 2018-05-29 9:33 UTC (permalink / raw)
To: Marc-André Lureau
Cc: QEMU, Paolo Bonzini, wangxin (U), Gonglei (Arei),
peterx, eblake, berrange
I think this patch doesn't fix my issue. For more details, please see Gonglei's reply.
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06296.html
> -----邮件原件-----
> 发件人: Marc-André Lureau [mailto:marcandre.lureau@gmail.com]
> 发送时间: 2018年5月29日 17:11
> 收件人: linzhecheng <linzhecheng@huawei.com>
> 抄送: QEMU <qemu-devel@nongnu.org>; Paolo Bonzini
> <pbonzini@redhat.com>; wangxin (U) <wangxinxin.wang@huawei.com>
> 主题: Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals
> EAGAIN
>
> Hi
>
> On Tue, May 29, 2018 at 4:52 AM, linzhecheng <linzhecheng@huawei.com>
> wrote:
> > Signed-off-by: linzhecheng <linzhecheng@huawei.com>
> >
> > diff --git a/chardev/char-socket.c b/chardev/char-socket.c index
> > 159e69c3b1..17519ec589 100644
> > --- a/chardev/char-socket.c
> > +++ b/chardev/char-socket.c
> > @@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t
> *buf, int len)
> > s->write_msgfds,
> > s->write_msgfds_num);
> >
> > - /* free the written msgfds, no matter what */
> > - if (s->write_msgfds_num) {
> > + /* free the written msgfds in any cases other than errno==EAGAIN */
> > + if (EAGAIN != errno && s->write_msgfds_num) {
> > g_free(s->write_msgfds);
> > s->write_msgfds = 0;
> > s->write_msgfds_num = 0;
> > --
>
> It is already fix since v2.12:
>
> commit c863fdec6aff6b5a4ca8fff1537b80d9f8b97726
> Author: Daniel P. Berrangé <berrange@redhat.com>
> Date: Thu Feb 22 12:13:51 2018 +0000
>
> chardev: fix handling of EAGAIN for TCP chardev
>
> > 2.12.2.windows.2
> >
> >
> >
>
>
>
> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
2018-05-29 8:20 linzhecheng
@ 2018-05-29 8:41 ` Gonglei (Arei)
0 siblings, 0 replies; 9+ messages in thread
From: Gonglei (Arei) @ 2018-05-29 8:41 UTC (permalink / raw)
To: linzhecheng, qemu-devel
Cc: pbonzini, wangxin (U), berrange, peterx, marcandre.lureau, eblake
Hi all,
The issue is easy to reproduce when we confiugred multi-queue function for vhost-user nics.
The main backtrace is as follows:
vhost_user_write ==> 0) sets s->write_msgfds_num to 8
qemu_chr_fe_write_all
qemu_chr_fe_write_buffer ==> 4) rewrite because (ret <0 && errno is EAGAIN)
tcp_chr_write ==> 3) clear resource about s->write_msgfds and set s->write_msgfds_num to 0
io_channel_send_full ==> 2) errno = EAGAIN and return -1
qio_channel_socket_writev ==> 1) returns QIO_CHANNEL_ERR_BLOCK when ret <0 && errno == EAGAIN
Then at the above step 4) may cause undefined behaviors on the vhost-user server side because null control message is sent.
So, we submit a patch to fix it. What's your opinion?
Regards,
-Gonglei
> -----Original Message-----
> From: linzhecheng
> Sent: Tuesday, May 29, 2018 4:20 PM
> To: qemu-devel@nongnu.org
> Cc: pbonzini@redhat.com; wangxin (U) <wangxinxin.wang@huawei.com>;
> berrange@redhat.com; peterx@redhat.com; marcandre.lureau@redhat.com;
> eblake@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>
> Subject: RE: [PATCH] socket: dont't free msgfds if error equals EAGAIN
>
> CC'ing Daniel P. Berrangé , Peter Xu, Marc-André Lureau, Eric Blake, Gonglei
>
> > -----邮件原件-----
> > 发件人: linzhecheng
> > 发送时间: 2018年5月29日 10:53
> > 收件人: qemu-devel@nongnu.org
> > 抄送: pbonzini@redhat.com; wangxin (U)
> <wangxinxin.wang@huawei.com>;
> > linzhecheng <linzhecheng@huawei.com>
> > 主题: [PATCH] socket: dont't free msgfds if error equals EAGAIN
> >
> > Signed-off-by: linzhecheng <linzhecheng@huawei.com>
> >
> > diff --git a/chardev/char-socket.c b/chardev/char-socket.c index
> > 159e69c3b1..17519ec589 100644
> > --- a/chardev/char-socket.c
> > +++ b/chardev/char-socket.c
> > @@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t
> > *buf, int len)
> > s->write_msgfds,
> > s->write_msgfds_num);
> >
> > - /* free the written msgfds, no matter what */
> > - if (s->write_msgfds_num) {
> > + /* free the written msgfds in any cases other than errno==EAGAIN
> */
> > + if (EAGAIN != errno && s->write_msgfds_num) {
> > g_free(s->write_msgfds);
> > s->write_msgfds = 0;
> > s->write_msgfds_num = 0;
> > --
> > 2.12.2.windows.2
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
@ 2018-05-29 8:20 linzhecheng
2018-05-29 8:41 ` Gonglei (Arei)
0 siblings, 1 reply; 9+ messages in thread
From: linzhecheng @ 2018-05-29 8:20 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, wangxin (U),
berrange, peterx, marcandre.lureau, eblake, Gonglei (Arei)
CC'ing Daniel P. Berrangé , Peter Xu, Marc-André Lureau, Eric Blake, Gonglei
> -----邮件原件-----
> 发件人: linzhecheng
> 发送时间: 2018年5月29日 10:53
> 收件人: qemu-devel@nongnu.org
> 抄送: pbonzini@redhat.com; wangxin (U) <wangxinxin.wang@huawei.com>;
> linzhecheng <linzhecheng@huawei.com>
> 主题: [PATCH] socket: dont't free msgfds if error equals EAGAIN
>
> Signed-off-by: linzhecheng <linzhecheng@huawei.com>
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c index
> 159e69c3b1..17519ec589 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -134,8 +134,8 @@ static int tcp_chr_write(Chardev *chr, const uint8_t
> *buf, int len)
> s->write_msgfds,
> s->write_msgfds_num);
>
> - /* free the written msgfds, no matter what */
> - if (s->write_msgfds_num) {
> + /* free the written msgfds in any cases other than errno==EAGAIN */
> + if (EAGAIN != errno && s->write_msgfds_num) {
> g_free(s->write_msgfds);
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
> --
> 2.12.2.windows.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-06-01 11:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-29 2:52 [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN linzhecheng
2018-05-29 9:11 ` Marc-André Lureau
2018-05-30 2:56 ` Peter Xu
2018-06-01 11:46 ` Daniel P. Berrangé
2018-05-29 8:20 linzhecheng
2018-05-29 8:41 ` Gonglei (Arei)
2018-05-29 9:33 linzhecheng
2018-05-29 19:33 ` Eric Blake
2018-05-30 9:05 ` Gonglei (Arei)
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.