[Qemu-devel] [PULL 0/2] Qio next patches

[Qemu-devel] [PULL 0/2] Qio next patches

[Daniel P. Berrangé]

The following changes since commit 00928a421d47f49691cace1207481b7aad31b1f1:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180626' into staging (2018-06-26 18:23:49 +0100)

are available in the Git repository at:

  https://github.com/berrange/qemu tags/qio-next-pull-request

for you to fetch changes up to 39321d9ed89eccdde73bb7b20cb834506c3bd750:

  Delete AF_UNIX socket after close (2018-06-28 11:59:51 +0100)

----------------------------------------------------------------
Merge qio 2018-06-28

Misc bug fixes for sockets channels

----------------------------------------------------------------

Pavel Balaev (1):
  Delete AF_UNIX socket after close

linzhecheng (1):
  socket: dont't free msgfds if error equals EAGAIN

 chardev/char-socket.c |  4 ++--
 io/channel-socket.c   | 18 +++++++++++++++++-
 2 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.17.1

[Qemu-devel] [PULL 1/2] socket: dont't free msgfds if error equals EAGAIN

[Daniel P. Berrangé]

From: linzhecheng <linzhecheng@huawei.com>

If we see EAGAIN, no data was sent over the socket, so we still have to
retry sending of msgfds next time.

Signed-off-by: linzhecheng <linzhecheng@huawei.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 chardev/char-socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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.17.1

[Qemu-devel] [PULL 2/2] Delete AF_UNIX socket after close

[Daniel P. Berrangé]

From: Pavel Balaev <mail@void.so>

This is a second attempt at sending this patch:

http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg04697.html

Signed-off-by: Pavel Balaev <mail@void.so>
---
 io/channel-socket.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index 57cfb4d3a6..b50e63a053 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -685,8 +685,10 @@ qio_channel_socket_close(QIOChannel *ioc,
                          Error **errp)
 {
     QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
+    int rc = 0;
 
     if (sioc->fd != -1) {
+        SocketAddress *addr = socket_local_address(sioc->fd, errp);
 #ifdef WIN32
         WSAEventSelect(sioc->fd, NULL, 0);
 #endif
@@ -697,8 +699,22 @@ qio_channel_socket_close(QIOChannel *ioc,
             return -1;
         }
         sioc->fd = -1;
+
+        if (addr && addr->type == SOCKET_ADDRESS_TYPE_UNIX
+            && addr->u.q_unix.path) {
+            if (unlink(addr->u.q_unix.path) < 0 && errno != ENOENT) {
+                error_setg_errno(errp, errno,
+                                 "Failed to unlink socket %s",
+                                 addr->u.q_unix.path);
+                rc = -1;
+            }
+        }
+
+        if (addr) {
+            qapi_free_SocketAddress(addr);
+        }
     }
-    return 0;
+    return rc;
 }
 
 static int
-- 
2.17.1

Re: [Qemu-devel] [PULL 2/2] Delete AF_UNIX socket after close

[Eric Blake]

On 06/28/2018 06:02 AM, Daniel P. Berrangé wrote:
> From: Pavel Balaev <mail@void.so>
> 
> This is a second attempt at sending this patch:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg04697.html

I'm not stopping the pull request, but this particular commit message is 
not very useful. A year from now, looking through 'git log' will tell us 
nothing about the "why" for this patch (the subject line only covers the 
"what").  And at that time, no one will care how many failed attempts 
went through the list, but only what actually got committed.  Better 
would have been just directly using the message from that mail:

>> Since version 2.12.0 AF_UNIX socket created for QMP exchange is not
>> deleted on instance shutdown.
>> 
>> This is due to the fact that function qio_channel_socket_finalize() is
>> called after qio_channel_socket_close().

As a hint for future patches, mentioning that a post is a second version 
and replaces an earlier post to the list is best done after the --- line 
(where it is still readable on list as an aid to reviewers, but dropped 
by the maintainer using 'git am' as unnecessary fluff for the git log). 
More patch submission tips at: https://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PULL 2/2] Delete AF_UNIX socket after close

[Daniel P. Berrangé]

On Thu, Jun 28, 2018 at 07:02:38AM -0500, Eric Blake wrote:
> On 06/28/2018 06:02 AM, Daniel P. Berrangé wrote:
> > From: Pavel Balaev <mail@void.so>
> > 
> > This is a second attempt at sending this patch:
> > 
> > http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg04697.html
> 
> I'm not stopping the pull request, but this particular commit message is not
> very useful. A year from now, looking through 'git log' will tell us nothing
> about the "why" for this patch (the subject line only covers the "what").
> And at that time, no one will care how many failed attempts went through the
> list, but only what actually got committed.  Better would have been just
> directly using the message from that mail:
> 
> > > Since version 2.12.0 AF_UNIX socket created for QMP exchange is not
> > > deleted on instance shutdown.
> > > 
> > > This is due to the fact that function qio_channel_socket_finalize() is
> > > called after qio_channel_socket_close().
> 
> As a hint for future patches, mentioning that a post is a second version and
> replaces an earlier post to the list is best done after the --- line (where
> it is still readable on list as an aid to reviewers, but dropped by the
> maintainer using 'git am' as unnecessary fluff for the git log). More patch
> submission tips at: https://wiki.qemu.org/Contribute/SubmitAPatch

Yeah, sorry my bad - i had intended to fix up the commit message but
completely forgot.

Peter, please ignore this PR, I will resend.

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] [PULL 0/2] Qio next patches

[Eric Blake]

On 06/28/2018 06:02 AM, Daniel P. Berrangé wrote:
> The following changes since commit 00928a421d47f49691cace1207481b7aad31b1f1:
> 
>    Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180626' into staging (2018-06-26 18:23:49 +0100)
> 
> are available in the Git repository at:
> 
>    https://github.com/berrange/qemu tags/qio-next-pull-request
> 
> for you to fetch changes up to 39321d9ed89eccdde73bb7b20cb834506c3bd750:
> 
>    Delete AF_UNIX socket after close (2018-06-28 11:59:51 +0100)
> 
> ----------------------------------------------------------------
> Merge qio 2018-06-28
> 
> Misc bug fixes for sockets channels
> 
> ----------------------------------------------------------------
> 
> Pavel Balaev (1):
>    Delete AF_UNIX socket after close

Replying here to catch Peter's attention - based on comments on 2/2, Dan 
will be submitting a v2 pull request.

> 
> linzhecheng (1):
>    socket: dont't free msgfds if error equals EAGAIN

Dan - while you're at it, s/dont't/don't/

> 
>   chardev/char-socket.c |  4 ++--
>   io/channel-socket.c   | 18 +++++++++++++++++-
>   2 files changed, 19 insertions(+), 3 deletions(-)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org