Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
On Tue, May 29, 2018 at 10:52:40AM +0800, linzhecheng wrote:
> Signed-off-by: linzhecheng
>
> 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 :|
Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
> -Original Message- > From: Eric Blake [mailto:ebl...@redhat.com] > Sent: Wednesday, May 30, 2018 3:33 AM > To: linzhecheng ; Marc-André Lureau > > Cc: QEMU ; Paolo Bonzini ; > wangxin (U) ; Gonglei (Arei) > ; pet...@redhat.com; berra...@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
Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
On Tue, May 29, 2018 at 10:52:40AM +0800, linzhecheng wrote:
> Signed-off-by: linzhecheng
>
> 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
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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
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.lur...@gmail.com]
> 发送时间: 2018年5月29日 17:11
> 收件人: linzhecheng
> 抄送: QEMU ; Paolo Bonzini
> ; wangxin (U)
> 主题: Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals
> EAGAIN
>
> Hi
>
> On Tue, May 29, 2018 at 4:52 AM, linzhecheng
> wrote:
> > Signed-off-by: linzhecheng
> >
> > 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é
> Date: Thu Feb 22 12:13:51 2018 +
>
> chardev: fix handling of EAGAIN for TCP chardev
>
> > 2.12.2.windows.2
> >
> >
> >
>
>
>
> --
> Marc-André Lureau
Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
Hi
On Tue, May 29, 2018 at 4:52 AM, linzhecheng wrote:
> Signed-off-by: linzhecheng
>
> 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é
Date: Thu Feb 22 12:13:51 2018 +
chardev: fix handling of EAGAIN for TCP chardev
> 2.12.2.windows.2
>
>
>
--
Marc-André Lureau
Re: [Qemu-devel] [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
> 抄送: pbonz...@redhat.com; wangxin (U) ;
> linzhecheng
> 主题: [PATCH] socket: dont't free msgfds if error equals EAGAIN
>
> Signed-off-by: linzhecheng
>
> 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
>
Re: [Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
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: pbonz...@redhat.com; wangxin (U) ;
> berra...@redhat.com; pet...@redhat.com; marcandre.lur...@redhat.com;
> ebl...@redhat.com; Gonglei (Arei)
> 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
> > 抄送: pbonz...@redhat.com; wangxin (U)
> ;
> > linzhecheng
> > 主题: [PATCH] socket: dont't free msgfds if error equals EAGAIN
> >
> > Signed-off-by: linzhecheng
> >
> > 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
> >
[Qemu-devel] [PATCH] socket: dont't free msgfds if error equals EAGAIN
Signed-off-by: linzhecheng
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
