| Message ID | 20180628110254.6725-3-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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
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
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