* [Qemu-devel] [PATCH v2 0/3] chardev cleanups
@ 2016-06-16 19:28 marcandre.lureau
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving marcandre.lureau
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: marcandre.lureau @ 2016-06-16 19:28 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi
A small series to do some chardev cleanup when removing them and
leaving qemu.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1347077
v1->v2:
- use atexit() for qemu_chr_cleanup()
- add missing braces
Marc-André Lureau (3):
char: clean up remaining chardevs when leaving
socket: add listen feature
socket: unlink unix socket on remove
include/io/channel.h | 1 +
include/qemu/sockets.h | 1 +
io/channel-socket.c | 17 +++++++++++++++++
qemu-char.c | 11 +++++++++++
tests/test-io-channel-socket.c | 2 +-
util/qemu-sockets.c | 18 ++++++++++++++++++
6 files changed, 49 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving
2016-06-16 19:28 [Qemu-devel] [PATCH v2 0/3] chardev cleanups marcandre.lureau
@ 2016-06-16 19:28 ` marcandre.lureau
2016-06-23 9:34 ` Daniel P. Berrange
2016-06-29 10:53 ` Paolo Bonzini
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 2/3] socket: add listen feature marcandre.lureau
` (2 subsequent siblings)
3 siblings, 2 replies; 19+ messages in thread
From: marcandre.lureau @ 2016-06-16 19:28 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This helps to remove various chardev resources leaks when leaving qemu.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qemu-char.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index c926e9a..98dcd49 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4541,6 +4541,15 @@ void qmp_chardev_remove(const char *id, Error **errp)
qemu_chr_delete(chr);
}
+static void qemu_chr_cleanup(void)
+{
+ CharDriverState *chr;
+
+ QTAILQ_FOREACH(chr, &chardevs, next) {
+ qemu_chr_delete(chr);
+ }
+}
+
static void register_types(void)
{
register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL,
@@ -4587,6 +4596,8 @@ static void register_types(void)
* is specified
*/
qemu_add_machine_init_done_notifier(&muxes_realize_notify);
+
+ atexit(qemu_chr_cleanup);
}
type_init(register_types);
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] socket: add listen feature
2016-06-16 19:28 [Qemu-devel] [PATCH v2 0/3] chardev cleanups marcandre.lureau
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving marcandre.lureau
@ 2016-06-16 19:28 ` marcandre.lureau
2016-06-23 9:35 ` Daniel P. Berrange
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove marcandre.lureau
2016-06-17 12:34 ` [Qemu-devel] [PATCH v2 0/3] chardev cleanups Paolo Bonzini
3 siblings, 1 reply; 19+ messages in thread
From: marcandre.lureau @ 2016-06-16 19:28 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add a flag to tell whether the channel socket is listening.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/io/channel.h | 1 +
io/channel-socket.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/include/io/channel.h b/include/io/channel.h
index d37acd2..e52f059 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -42,6 +42,7 @@ typedef enum QIOChannelFeature QIOChannelFeature;
enum QIOChannelFeature {
QIO_CHANNEL_FEATURE_FD_PASS = (1 << 0),
QIO_CHANNEL_FEATURE_SHUTDOWN = (1 << 1),
+ QIO_CHANNEL_FEATURE_LISTEN = (1 << 2),
};
diff --git a/io/channel-socket.c b/io/channel-socket.c
index ca8bc20..1cd5848 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -71,6 +71,9 @@ qio_channel_socket_set_fd(QIOChannelSocket *sioc,
int fd,
Error **errp)
{
+ int val;
+ socklen_t len = sizeof(val);
+
if (sioc->fd != -1) {
error_setg(errp, "Socket is already open");
return -1;
@@ -106,6 +109,10 @@ qio_channel_socket_set_fd(QIOChannelSocket *sioc,
ioc->features |= (1 << QIO_CHANNEL_FEATURE_FD_PASS);
}
#endif /* WIN32 */
+ if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &val, &len) == 0 && val) {
+ QIOChannel *ioc = QIO_CHANNEL(sioc);
+ ioc->features |= (1 << QIO_CHANNEL_FEATURE_LISTEN);
+ }
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-16 19:28 [Qemu-devel] [PATCH v2 0/3] chardev cleanups marcandre.lureau
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving marcandre.lureau
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 2/3] socket: add listen feature marcandre.lureau
@ 2016-06-16 19:28 ` marcandre.lureau
2016-06-23 4:41 ` Michael S. Tsirkin
2016-06-23 9:36 ` Daniel P. Berrange
2016-06-17 12:34 ` [Qemu-devel] [PATCH v2 0/3] chardev cleanups Paolo Bonzini
3 siblings, 2 replies; 19+ messages in thread
From: marcandre.lureau @ 2016-06-16 19:28 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
qemu leaves unix socket files behind when removing a listening chardev
or leaving. qemu could clean that up, even if doing so isn't race-free.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1347077
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/qemu/sockets.h | 1 +
io/channel-socket.c | 10 ++++++++++
tests/test-io-channel-socket.c | 2 +-
util/qemu-sockets.c | 18 ++++++++++++++++++
4 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 1bd9218..5dd2648 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -51,6 +51,7 @@ SocketAddress *socket_parse(const char *str, Error **errp);
int socket_connect(SocketAddress *addr, Error **errp,
NonBlockingConnectHandler *callback, void *opaque);
int socket_listen(SocketAddress *addr, Error **errp);
+void socket_listen_cleanup(int fd, Error **errp);
int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
/* Old, ipv4 only bits. Don't use for new code. */
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 1cd5848..6ec87f8 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -400,7 +400,17 @@ static void qio_channel_socket_init(Object *obj)
static void qio_channel_socket_finalize(Object *obj)
{
QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj);
+
if (ioc->fd != -1) {
+ if (QIO_CHANNEL(ioc)->features & QIO_CHANNEL_FEATURE_LISTEN) {
+ Error *err = NULL;
+
+ socket_listen_cleanup(ioc->fd, &err);
+ if (err) {
+ error_report_err(err);
+ err = NULL;
+ }
+ }
#ifdef WIN32
WSAEventSelect(ioc->fd, NULL, 0);
#endif
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index 855306b..f73e063 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -383,7 +383,7 @@ static void test_io_channel_unix(bool async)
qapi_free_SocketAddress(listen_addr);
qapi_free_SocketAddress(connect_addr);
- unlink(TEST_SOCKET);
+ g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS) == FALSE);
}
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 0d6cd1f..5d03695 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -997,6 +997,24 @@ int socket_listen(SocketAddress *addr, Error **errp)
return fd;
}
+void socket_listen_cleanup(int fd, Error **errp)
+{
+ SocketAddress *addr;
+
+ addr = socket_local_address(fd, errp);
+
+ if (addr->type == SOCKET_ADDRESS_KIND_UNIX
+ && addr->u.q_unix.data->path) {
+ if (unlink(addr->u.q_unix.data->path) < 0 && errno != ENOENT) {
+ error_setg_errno(errp, errno,
+ "Failed to unlink socket %s",
+ addr->u.q_unix.data->path);
+ }
+ }
+
+ g_free(addr);
+}
+
int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
{
int fd;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] chardev cleanups
2016-06-16 19:28 [Qemu-devel] [PATCH v2 0/3] chardev cleanups marcandre.lureau
` (2 preceding siblings ...)
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove marcandre.lureau
@ 2016-06-17 12:34 ` Paolo Bonzini
2016-06-23 9:36 ` Daniel P. Berrange
3 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2016-06-17 12:34 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
On 16/06/2016 21:28, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi
>
> A small series to do some chardev cleanup when removing them and
> leaving qemu.
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1347077
>
> v1->v2:
> - use atexit() for qemu_chr_cleanup()
> - add missing braces
>
> Marc-André Lureau (3):
> char: clean up remaining chardevs when leaving
> socket: add listen feature
> socket: unlink unix socket on remove
>
> include/io/channel.h | 1 +
> include/qemu/sockets.h | 1 +
> io/channel-socket.c | 17 +++++++++++++++++
> qemu-char.c | 11 +++++++++++
> tests/test-io-channel-socket.c | 2 +-
> util/qemu-sockets.c | 18 ++++++++++++++++++
> 6 files changed, 49 insertions(+), 1 deletion(-)
>
Looks good, I'll queue them if Dan acks patch 2.
Paolo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove marcandre.lureau
@ 2016-06-23 4:41 ` Michael S. Tsirkin
2016-06-23 4:51 ` Michael S. Tsirkin
2016-06-23 9:36 ` Daniel P. Berrange
1 sibling, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2016-06-23 4:41 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, pbonzini
On Thu, Jun 16, 2016 at 09:28:52PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> qemu leaves unix socket files behind when removing a listening chardev
> or leaving. qemu could clean that up, even if doing so isn't race-free.
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1347077
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
We normally only unlink what we created.
unlinking files we didn't create
looks like a silent change that might easily break
existing users.
> ---
> include/qemu/sockets.h | 1 +
> io/channel-socket.c | 10 ++++++++++
> tests/test-io-channel-socket.c | 2 +-
> util/qemu-sockets.c | 18 ++++++++++++++++++
> 4 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 1bd9218..5dd2648 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -51,6 +51,7 @@ SocketAddress *socket_parse(const char *str, Error **errp);
> int socket_connect(SocketAddress *addr, Error **errp,
> NonBlockingConnectHandler *callback, void *opaque);
> int socket_listen(SocketAddress *addr, Error **errp);
> +void socket_listen_cleanup(int fd, Error **errp);
> int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
>
> /* Old, ipv4 only bits. Don't use for new code. */
> diff --git a/io/channel-socket.c b/io/channel-socket.c
> index 1cd5848..6ec87f8 100644
> --- a/io/channel-socket.c
> +++ b/io/channel-socket.c
> @@ -400,7 +400,17 @@ static void qio_channel_socket_init(Object *obj)
> static void qio_channel_socket_finalize(Object *obj)
> {
> QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj);
> +
> if (ioc->fd != -1) {
> + if (QIO_CHANNEL(ioc)->features & QIO_CHANNEL_FEATURE_LISTEN) {
> + Error *err = NULL;
> +
> + socket_listen_cleanup(ioc->fd, &err);
> + if (err) {
> + error_report_err(err);
> + err = NULL;
> + }
> + }
> #ifdef WIN32
> WSAEventSelect(ioc->fd, NULL, 0);
> #endif
> diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
> index 855306b..f73e063 100644
> --- a/tests/test-io-channel-socket.c
> +++ b/tests/test-io-channel-socket.c
> @@ -383,7 +383,7 @@ static void test_io_channel_unix(bool async)
>
> qapi_free_SocketAddress(listen_addr);
> qapi_free_SocketAddress(connect_addr);
> - unlink(TEST_SOCKET);
> + g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS) == FALSE);
> }
>
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 0d6cd1f..5d03695 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -997,6 +997,24 @@ int socket_listen(SocketAddress *addr, Error **errp)
> return fd;
> }
>
> +void socket_listen_cleanup(int fd, Error **errp)
> +{
> + SocketAddress *addr;
> +
> + addr = socket_local_address(fd, errp);
> +
> + if (addr->type == SOCKET_ADDRESS_KIND_UNIX
> + && addr->u.q_unix.data->path) {
> + if (unlink(addr->u.q_unix.data->path) < 0 && errno != ENOENT) {
> + error_setg_errno(errp, errno,
> + "Failed to unlink socket %s",
> + addr->u.q_unix.data->path);
> + }
> + }
> +
> + g_free(addr);
> +}
> +
> int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
> {
> int fd;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-23 4:41 ` Michael S. Tsirkin
@ 2016-06-23 4:51 ` Michael S. Tsirkin
2016-06-23 9:08 ` Marc-André Lureau
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2016-06-23 4:51 UTC (permalink / raw)
To: marcandre.lureau; +Cc: pbonzini, qemu-devel
On Thu, Jun 23, 2016 at 07:41:55AM +0300, Michael S. Tsirkin wrote:
> On Thu, Jun 16, 2016 at 09:28:52PM +0200, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > qemu leaves unix socket files behind when removing a listening chardev
> > or leaving. qemu could clean that up, even if doing so isn't race-free.
> >
> > Fixes:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1347077
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> We normally only unlink what we created.
>
> unlinking files we didn't create
> looks like a silent change that might easily break
> existing users.
Maybe what you want is a need_unlink feature.
Set it for unix sockets only, that would make some sense.
>
> > ---
> > include/qemu/sockets.h | 1 +
> > io/channel-socket.c | 10 ++++++++++
> > tests/test-io-channel-socket.c | 2 +-
> > util/qemu-sockets.c | 18 ++++++++++++++++++
> > 4 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> > index 1bd9218..5dd2648 100644
> > --- a/include/qemu/sockets.h
> > +++ b/include/qemu/sockets.h
> > @@ -51,6 +51,7 @@ SocketAddress *socket_parse(const char *str, Error **errp);
> > int socket_connect(SocketAddress *addr, Error **errp,
> > NonBlockingConnectHandler *callback, void *opaque);
> > int socket_listen(SocketAddress *addr, Error **errp);
> > +void socket_listen_cleanup(int fd, Error **errp);
> > int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
> >
> > /* Old, ipv4 only bits. Don't use for new code. */
> > diff --git a/io/channel-socket.c b/io/channel-socket.c
> > index 1cd5848..6ec87f8 100644
> > --- a/io/channel-socket.c
> > +++ b/io/channel-socket.c
> > @@ -400,7 +400,17 @@ static void qio_channel_socket_init(Object *obj)
> > static void qio_channel_socket_finalize(Object *obj)
> > {
> > QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj);
> > +
> > if (ioc->fd != -1) {
> > + if (QIO_CHANNEL(ioc)->features & QIO_CHANNEL_FEATURE_LISTEN) {
> > + Error *err = NULL;
> > +
> > + socket_listen_cleanup(ioc->fd, &err);
> > + if (err) {
> > + error_report_err(err);
> > + err = NULL;
> > + }
> > + }
> > #ifdef WIN32
> > WSAEventSelect(ioc->fd, NULL, 0);
> > #endif
> > diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
> > index 855306b..f73e063 100644
> > --- a/tests/test-io-channel-socket.c
> > +++ b/tests/test-io-channel-socket.c
> > @@ -383,7 +383,7 @@ static void test_io_channel_unix(bool async)
> >
> > qapi_free_SocketAddress(listen_addr);
> > qapi_free_SocketAddress(connect_addr);
> > - unlink(TEST_SOCKET);
> > + g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS) == FALSE);
> > }
> >
> >
> > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > index 0d6cd1f..5d03695 100644
> > --- a/util/qemu-sockets.c
> > +++ b/util/qemu-sockets.c
> > @@ -997,6 +997,24 @@ int socket_listen(SocketAddress *addr, Error **errp)
> > return fd;
> > }
> >
> > +void socket_listen_cleanup(int fd, Error **errp)
> > +{
> > + SocketAddress *addr;
> > +
> > + addr = socket_local_address(fd, errp);
> > +
> > + if (addr->type == SOCKET_ADDRESS_KIND_UNIX
> > + && addr->u.q_unix.data->path) {
> > + if (unlink(addr->u.q_unix.data->path) < 0 && errno != ENOENT) {
> > + error_setg_errno(errp, errno,
> > + "Failed to unlink socket %s",
> > + addr->u.q_unix.data->path);
> > + }
> > + }
> > +
> > + g_free(addr);
> > +}
> > +
> > int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
> > {
> > int fd;
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-23 4:51 ` Michael S. Tsirkin
@ 2016-06-23 9:08 ` Marc-André Lureau
2016-06-23 17:01 ` Michael S. Tsirkin
0 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2016-06-23 9:08 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: marcandre lureau, pbonzini, qemu-devel
Hi
----- Original Message -----
> On Thu, Jun 23, 2016 at 07:41:55AM +0300, Michael S. Tsirkin wrote:
> > On Thu, Jun 16, 2016 at 09:28:52PM +0200, marcandre.lureau@redhat.com
> > wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > qemu leaves unix socket files behind when removing a listening chardev
> > > or leaving. qemu could clean that up, even if doing so isn't race-free.
> > >
> > > Fixes:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1347077
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > We normally only unlink what we created.
What do you mean by "we", qemu currently unlink before creating the unix socket. No further cleanup.
> >
> > unlinking files we didn't create
> > looks like a silent change that might easily break
> > existing users.
That shouldn't remove files we didn't create. But a bad user could recreate the unix socket after qemu did, and that would remove it. I don't think that's a valid use case though, so it should be fine.
> Maybe what you want is a need_unlink feature.
> Set it for unix sockets only, that would make some sense.
Oh perhaps what you mean is that if the fd was passed, we should cleanup the unix socket? Yes, I think we should do that then. I'll update the series.
thanks
>
> >
> > > ---
> > > include/qemu/sockets.h | 1 +
> > > io/channel-socket.c | 10 ++++++++++
> > > tests/test-io-channel-socket.c | 2 +-
> > > util/qemu-sockets.c | 18 ++++++++++++++++++
> > > 4 files changed, 30 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> > > index 1bd9218..5dd2648 100644
> > > --- a/include/qemu/sockets.h
> > > +++ b/include/qemu/sockets.h
> > > @@ -51,6 +51,7 @@ SocketAddress *socket_parse(const char *str, Error
> > > **errp);
> > > int socket_connect(SocketAddress *addr, Error **errp,
> > > NonBlockingConnectHandler *callback, void *opaque);
> > > int socket_listen(SocketAddress *addr, Error **errp);
> > > +void socket_listen_cleanup(int fd, Error **errp);
> > > int socket_dgram(SocketAddress *remote, SocketAddress *local, Error
> > > **errp);
> > >
> > > /* Old, ipv4 only bits. Don't use for new code. */
> > > diff --git a/io/channel-socket.c b/io/channel-socket.c
> > > index 1cd5848..6ec87f8 100644
> > > --- a/io/channel-socket.c
> > > +++ b/io/channel-socket.c
> > > @@ -400,7 +400,17 @@ static void qio_channel_socket_init(Object *obj)
> > > static void qio_channel_socket_finalize(Object *obj)
> > > {
> > > QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj);
> > > +
> > > if (ioc->fd != -1) {
> > > + if (QIO_CHANNEL(ioc)->features & QIO_CHANNEL_FEATURE_LISTEN) {
> > > + Error *err = NULL;
> > > +
> > > + socket_listen_cleanup(ioc->fd, &err);
> > > + if (err) {
> > > + error_report_err(err);
> > > + err = NULL;
> > > + }
> > > + }
> > > #ifdef WIN32
> > > WSAEventSelect(ioc->fd, NULL, 0);
> > > #endif
> > > diff --git a/tests/test-io-channel-socket.c
> > > b/tests/test-io-channel-socket.c
> > > index 855306b..f73e063 100644
> > > --- a/tests/test-io-channel-socket.c
> > > +++ b/tests/test-io-channel-socket.c
> > > @@ -383,7 +383,7 @@ static void test_io_channel_unix(bool async)
> > >
> > > qapi_free_SocketAddress(listen_addr);
> > > qapi_free_SocketAddress(connect_addr);
> > > - unlink(TEST_SOCKET);
> > > + g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS) == FALSE);
> > > }
> > >
> > >
> > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > > index 0d6cd1f..5d03695 100644
> > > --- a/util/qemu-sockets.c
> > > +++ b/util/qemu-sockets.c
> > > @@ -997,6 +997,24 @@ int socket_listen(SocketAddress *addr, Error **errp)
> > > return fd;
> > > }
> > >
> > > +void socket_listen_cleanup(int fd, Error **errp)
> > > +{
> > > + SocketAddress *addr;
> > > +
> > > + addr = socket_local_address(fd, errp);
> > > +
> > > + if (addr->type == SOCKET_ADDRESS_KIND_UNIX
> > > + && addr->u.q_unix.data->path) {
> > > + if (unlink(addr->u.q_unix.data->path) < 0 && errno != ENOENT) {
> > > + error_setg_errno(errp, errno,
> > > + "Failed to unlink socket %s",
> > > + addr->u.q_unix.data->path);
> > > + }
> > > + }
> > > +
> > > + g_free(addr);
> > > +}
> > > +
> > > int socket_dgram(SocketAddress *remote, SocketAddress *local, Error
> > > **errp)
> > > {
> > > int fd;
> > > --
> > > 2.7.4
> > >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving marcandre.lureau
@ 2016-06-23 9:34 ` Daniel P. Berrange
2016-06-29 10:53 ` Paolo Bonzini
1 sibling, 0 replies; 19+ messages in thread
From: Daniel P. Berrange @ 2016-06-23 9:34 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, pbonzini
On Thu, Jun 16, 2016 at 09:28:50PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> This helps to remove various chardev resources leaks when leaving qemu.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qemu-char.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] socket: add listen feature
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 2/3] socket: add listen feature marcandre.lureau
@ 2016-06-23 9:35 ` Daniel P. Berrange
0 siblings, 0 replies; 19+ messages in thread
From: Daniel P. Berrange @ 2016-06-23 9:35 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, pbonzini
On Thu, Jun 16, 2016 at 09:28:51PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Add a flag to tell whether the channel socket is listening.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/io/channel.h | 1 +
> io/channel-socket.c | 7 +++++++
> 2 files changed, 8 insertions(+)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove marcandre.lureau
2016-06-23 4:41 ` Michael S. Tsirkin
@ 2016-06-23 9:36 ` Daniel P. Berrange
1 sibling, 0 replies; 19+ messages in thread
From: Daniel P. Berrange @ 2016-06-23 9:36 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel, pbonzini
On Thu, Jun 16, 2016 at 09:28:52PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> qemu leaves unix socket files behind when removing a listening chardev
> or leaving. qemu could clean that up, even if doing so isn't race-free.
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1347077
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/qemu/sockets.h | 1 +
> io/channel-socket.c | 10 ++++++++++
> tests/test-io-channel-socket.c | 2 +-
> util/qemu-sockets.c | 18 ++++++++++++++++++
> 4 files changed, 30 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] chardev cleanups
2016-06-17 12:34 ` [Qemu-devel] [PATCH v2 0/3] chardev cleanups Paolo Bonzini
@ 2016-06-23 9:36 ` Daniel P. Berrange
0 siblings, 0 replies; 19+ messages in thread
From: Daniel P. Berrange @ 2016-06-23 9:36 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: marcandre.lureau, qemu-devel
On Fri, Jun 17, 2016 at 02:34:41PM +0200, Paolo Bonzini wrote:
>
>
> On 16/06/2016 21:28, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Hi
> >
> > A small series to do some chardev cleanup when removing them and
> > leaving qemu.
> >
> > Fixes:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1347077
> >
> > v1->v2:
> > - use atexit() for qemu_chr_cleanup()
> > - add missing braces
> >
> > Marc-André Lureau (3):
> > char: clean up remaining chardevs when leaving
> > socket: add listen feature
> > socket: unlink unix socket on remove
> >
> > include/io/channel.h | 1 +
> > include/qemu/sockets.h | 1 +
> > io/channel-socket.c | 17 +++++++++++++++++
> > qemu-char.c | 11 +++++++++++
> > tests/test-io-channel-socket.c | 2 +-
> > util/qemu-sockets.c | 18 ++++++++++++++++++
> > 6 files changed, 49 insertions(+), 1 deletion(-)
> >
>
> Looks good, I'll queue them if Dan acks patch 2.
Go ahead, its all fine with me.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-23 9:08 ` Marc-André Lureau
@ 2016-06-23 17:01 ` Michael S. Tsirkin
2016-06-24 12:08 ` Marc-André Lureau
0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2016-06-23 17:01 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: marcandre lureau, pbonzini, qemu-devel
On Thu, Jun 23, 2016 at 05:08:03AM -0400, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
> > On Thu, Jun 23, 2016 at 07:41:55AM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Jun 16, 2016 at 09:28:52PM +0200, marcandre.lureau@redhat.com
> > > wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > >
> > > > qemu leaves unix socket files behind when removing a listening chardev
> > > > or leaving. qemu could clean that up, even if doing so isn't race-free.
> > > >
> > > > Fixes:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1347077
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > We normally only unlink what we created.
>
> What do you mean by "we", qemu currently unlink before creating the unix socket. No further cleanup.
>
> > >
> > > unlinking files we didn't create
> > > looks like a silent change that might easily break
> > > existing users.
>
> That shouldn't remove files we didn't create. But a bad user could recreate the unix socket after qemu did, and that would remove it. I don't think that's a valid use case though, so it should be fine.
>
> > Maybe what you want is a need_unlink feature.
> > Set it for unix sockets only, that would make some sense.
>
> Oh perhaps what you mean is that if the fd was passed, we should cleanup the unix socket? Yes, I think we should do that then. I'll update the series.
>
> thanks
I'd like it better contained - that's all. So let's set a flag that says
"must unlink" as opposed to "it's listening".
> >
> > >
> > > > ---
> > > > include/qemu/sockets.h | 1 +
> > > > io/channel-socket.c | 10 ++++++++++
> > > > tests/test-io-channel-socket.c | 2 +-
> > > > util/qemu-sockets.c | 18 ++++++++++++++++++
> > > > 4 files changed, 30 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> > > > index 1bd9218..5dd2648 100644
> > > > --- a/include/qemu/sockets.h
> > > > +++ b/include/qemu/sockets.h
> > > > @@ -51,6 +51,7 @@ SocketAddress *socket_parse(const char *str, Error
> > > > **errp);
> > > > int socket_connect(SocketAddress *addr, Error **errp,
> > > > NonBlockingConnectHandler *callback, void *opaque);
> > > > int socket_listen(SocketAddress *addr, Error **errp);
> > > > +void socket_listen_cleanup(int fd, Error **errp);
> > > > int socket_dgram(SocketAddress *remote, SocketAddress *local, Error
> > > > **errp);
> > > >
> > > > /* Old, ipv4 only bits. Don't use for new code. */
> > > > diff --git a/io/channel-socket.c b/io/channel-socket.c
> > > > index 1cd5848..6ec87f8 100644
> > > > --- a/io/channel-socket.c
> > > > +++ b/io/channel-socket.c
> > > > @@ -400,7 +400,17 @@ static void qio_channel_socket_init(Object *obj)
> > > > static void qio_channel_socket_finalize(Object *obj)
> > > > {
> > > > QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj);
> > > > +
> > > > if (ioc->fd != -1) {
> > > > + if (QIO_CHANNEL(ioc)->features & QIO_CHANNEL_FEATURE_LISTEN) {
> > > > + Error *err = NULL;
> > > > +
> > > > + socket_listen_cleanup(ioc->fd, &err);
> > > > + if (err) {
> > > > + error_report_err(err);
> > > > + err = NULL;
> > > > + }
> > > > + }
> > > > #ifdef WIN32
> > > > WSAEventSelect(ioc->fd, NULL, 0);
> > > > #endif
> > > > diff --git a/tests/test-io-channel-socket.c
> > > > b/tests/test-io-channel-socket.c
> > > > index 855306b..f73e063 100644
> > > > --- a/tests/test-io-channel-socket.c
> > > > +++ b/tests/test-io-channel-socket.c
> > > > @@ -383,7 +383,7 @@ static void test_io_channel_unix(bool async)
> > > >
> > > > qapi_free_SocketAddress(listen_addr);
> > > > qapi_free_SocketAddress(connect_addr);
> > > > - unlink(TEST_SOCKET);
> > > > + g_assert(g_file_test(TEST_SOCKET, G_FILE_TEST_EXISTS) == FALSE);
> > > > }
> > > >
> > > >
> > > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > > > index 0d6cd1f..5d03695 100644
> > > > --- a/util/qemu-sockets.c
> > > > +++ b/util/qemu-sockets.c
> > > > @@ -997,6 +997,24 @@ int socket_listen(SocketAddress *addr, Error **errp)
> > > > return fd;
> > > > }
> > > >
> > > > +void socket_listen_cleanup(int fd, Error **errp)
> > > > +{
> > > > + SocketAddress *addr;
> > > > +
> > > > + addr = socket_local_address(fd, errp);
> > > > +
> > > > + if (addr->type == SOCKET_ADDRESS_KIND_UNIX
> > > > + && addr->u.q_unix.data->path) {
> > > > + if (unlink(addr->u.q_unix.data->path) < 0 && errno != ENOENT) {
> > > > + error_setg_errno(errp, errno,
> > > > + "Failed to unlink socket %s",
> > > > + addr->u.q_unix.data->path);
> > > > + }
> > > > + }
> > > > +
> > > > + g_free(addr);
> > > > +}
> > > > +
> > > > int socket_dgram(SocketAddress *remote, SocketAddress *local, Error
> > > > **errp)
> > > > {
> > > > int fd;
> > > > --
> > > > 2.7.4
> > > >
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-23 17:01 ` Michael S. Tsirkin
@ 2016-06-24 12:08 ` Marc-André Lureau
2016-06-24 12:13 ` Daniel P. Berrange
2016-06-24 22:07 ` Michael S. Tsirkin
0 siblings, 2 replies; 19+ messages in thread
From: Marc-André Lureau @ 2016-06-24 12:08 UTC (permalink / raw)
To: Michael S. Tsirkin, Daniel P. Berrange; +Cc: QEMU, Paolo Bonzini
On Thu, Jun 23, 2016 at 7:01 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > Maybe what you want is a need_unlink feature.
>> > Set it for unix sockets only, that would make some sense.
>>
>> Oh perhaps what you mean is that if the fd was passed, we should cleanup the unix socket? Yes, I think we should do that then. I'll update the series.
Actually it's not possible to pass a listening fd to a socket chardev
today (the path argument doesn't understand /dev/fdset), so only path
created by qemu will be cleaned up.
>
> I'd like it better contained - that's all. So let's set a flag that says
> "must unlink" as opposed to "it's listening".
You suggest to rename QIO_CHANNEL_FEATURE_LISTEN to
QIO_CHANNEL_FEATURE_LISTEN_MUST_UNLINK ? Or to add another feature
flag? I don't think that brings anything useful here.
What would you think Daniel?
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-24 12:08 ` Marc-André Lureau
@ 2016-06-24 12:13 ` Daniel P. Berrange
2016-06-24 22:07 ` Michael S. Tsirkin
1 sibling, 0 replies; 19+ messages in thread
From: Daniel P. Berrange @ 2016-06-24 12:13 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Michael S. Tsirkin, QEMU, Paolo Bonzini
On Fri, Jun 24, 2016 at 02:08:52PM +0200, Marc-André Lureau wrote:
> On Thu, Jun 23, 2016 at 7:01 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > Maybe what you want is a need_unlink feature.
> >> > Set it for unix sockets only, that would make some sense.
> >>
> >> Oh perhaps what you mean is that if the fd was passed, we should cleanup the unix socket? Yes, I think we should do that then. I'll update the series.
>
> Actually it's not possible to pass a listening fd to a socket chardev
> today (the path argument doesn't understand /dev/fdset), so only path
> created by qemu will be cleaned up.
>
> >
> > I'd like it better contained - that's all. So let's set a flag that says
> > "must unlink" as opposed to "it's listening".
>
> You suggest to rename QIO_CHANNEL_FEATURE_LISTEN to
> QIO_CHANNEL_FEATURE_LISTEN_MUST_UNLINK ? Or to add another feature
> flag? I don't think that brings anything useful here.
IMHO the existing QIO_CHANNEL_FEATURE_LISTEN makes more sense than
a QIO_CHANNEL_FEATURE_MUST_UNLINK, so I'd like to see your current
patches go in as is.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-24 12:08 ` Marc-André Lureau
2016-06-24 12:13 ` Daniel P. Berrange
@ 2016-06-24 22:07 ` Michael S. Tsirkin
2016-06-27 8:10 ` Daniel P. Berrange
1 sibling, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2016-06-24 22:07 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Daniel P. Berrange, QEMU, Paolo Bonzini
On Fri, Jun 24, 2016 at 02:08:52PM +0200, Marc-André Lureau wrote:
> On Thu, Jun 23, 2016 at 7:01 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > Maybe what you want is a need_unlink feature.
> >> > Set it for unix sockets only, that would make some sense.
> >>
> >> Oh perhaps what you mean is that if the fd was passed, we should cleanup the unix socket? Yes, I think we should do that then. I'll update the series.
>
> Actually it's not possible to pass a listening fd to a socket chardev
> today (the path argument doesn't understand /dev/fdset), so only path
> created by qemu will be cleaned up.
>
> >
> > I'd like it better contained - that's all. So let's set a flag that says
> > "must unlink" as opposed to "it's listening".
>
> You suggest to rename QIO_CHANNEL_FEATURE_LISTEN to
> QIO_CHANNEL_FEATURE_LISTEN_MUST_UNLINK ?
QIO_CHANNEL_FEATURE_UNLINK_ON_CLOSE
or something like this.
Or maybe QIO_CHANNEL_FEATURE_BOUND
> Or to add another feature
> flag? I don't think that brings anything useful here.
The point is that in the future we might be listening on sockets where
we did not bind it. I would think that in that case, we do not want to
unlink it. So name should reflect this somehow.
> What would you think Daniel?
>
> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove
2016-06-24 22:07 ` Michael S. Tsirkin
@ 2016-06-27 8:10 ` Daniel P. Berrange
0 siblings, 0 replies; 19+ messages in thread
From: Daniel P. Berrange @ 2016-06-27 8:10 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Marc-André Lureau, QEMU, Paolo Bonzini
On Sat, Jun 25, 2016 at 01:07:10AM +0300, Michael S. Tsirkin wrote:
> On Fri, Jun 24, 2016 at 02:08:52PM +0200, Marc-André Lureau wrote:
> > On Thu, Jun 23, 2016 at 7:01 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > >> > Maybe what you want is a need_unlink feature.
> > >> > Set it for unix sockets only, that would make some sense.
> > >>
> > >> Oh perhaps what you mean is that if the fd was passed, we should cleanup the unix socket? Yes, I think we should do that then. I'll update the series.
> >
> > Actually it's not possible to pass a listening fd to a socket chardev
> > today (the path argument doesn't understand /dev/fdset), so only path
> > created by qemu will be cleaned up.
> >
> > >
> > > I'd like it better contained - that's all. So let's set a flag that says
> > > "must unlink" as opposed to "it's listening".
> >
> > You suggest to rename QIO_CHANNEL_FEATURE_LISTEN to
> > QIO_CHANNEL_FEATURE_LISTEN_MUST_UNLINK ?
>
> QIO_CHANNEL_FEATURE_UNLINK_ON_CLOSE
>
> or something like this.
>
> Or maybe QIO_CHANNEL_FEATURE_BOUND
>
> > Or to add another feature
> > flag? I don't think that brings anything useful here.
>
> The point is that in the future we might be listening on sockets where
> we did not bind it. I would think that in that case, we do not want to
> unlink it. So name should reflect this somehow.
There is no possibility of listening on sockets without binding to them
with the current design of the code. If & when we want such support we
can adapt as needed, but I don't see any real point in trying to support
something that is impossible right now.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving marcandre.lureau
2016-06-23 9:34 ` Daniel P. Berrange
@ 2016-06-29 10:53 ` Paolo Bonzini
2016-06-29 11:40 ` Marc-André Lureau
1 sibling, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2016-06-29 10:53 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel
On 16/06/2016 21:28, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> This helps to remove various chardev resources leaks when leaving qemu.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qemu-char.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index c926e9a..98dcd49 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4541,6 +4541,15 @@ void qmp_chardev_remove(const char *id, Error **errp)
> qemu_chr_delete(chr);
> }
>
> +static void qemu_chr_cleanup(void)
> +{
> + CharDriverState *chr;
> +
> + QTAILQ_FOREACH(chr, &chardevs, next) {
> + qemu_chr_delete(chr);
> + }
> +}
FYI, this patch is necessary on top:
diff --git a/qemu-char.c b/qemu-char.c
index 016badb..bc04ced 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4551,9 +4551,9 @@
static void qemu_chr_cleanup(void)
{
- CharDriverState *chr;
+ CharDriverState *chr, *tmp;
- QTAILQ_FOREACH(chr, &chardevs, next) {
+ QTAILQ_FOREACH_SAFE(chr, &chardevs, next, tmp) {
qemu_chr_delete(chr);
}
}
(Reproducer: start QEMU with MALLOC_PERTURB_=42 and type "quit" on the
monitor).
Thanks,
Paolo
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving
2016-06-29 10:53 ` Paolo Bonzini
@ 2016-06-29 11:40 ` Marc-André Lureau
0 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2016-06-29 11:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU
Hi
On Wed, Jun 29, 2016 at 12:53 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 16/06/2016 21:28, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> This helps to remove various chardev resources leaks when leaving qemu.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>> qemu-char.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index c926e9a..98dcd49 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -4541,6 +4541,15 @@ void qmp_chardev_remove(const char *id, Error **errp)
>> qemu_chr_delete(chr);
>> }
>>
>> +static void qemu_chr_cleanup(void)
>> +{
>> + CharDriverState *chr;
>> +
>> + QTAILQ_FOREACH(chr, &chardevs, next) {
>> + qemu_chr_delete(chr);
>> + }
>> +}
>
> FYI, this patch is necessary on top:
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 016badb..bc04ced 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4551,9 +4551,9 @@
>
> static void qemu_chr_cleanup(void)
> {
> - CharDriverState *chr;
> + CharDriverState *chr, *tmp;
>
> - QTAILQ_FOREACH(chr, &chardevs, next) {
> + QTAILQ_FOREACH_SAFE(chr, &chardevs, next, tmp) {
> qemu_chr_delete(chr);
> }
> }
>
ack, I guess you'll have it in your pull request? squash or seperate, anyhow.
thanks
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-06-29 11:40 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16 19:28 [Qemu-devel] [PATCH v2 0/3] chardev cleanups marcandre.lureau
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 1/3] char: clean up remaining chardevs when leaving marcandre.lureau
2016-06-23 9:34 ` Daniel P. Berrange
2016-06-29 10:53 ` Paolo Bonzini
2016-06-29 11:40 ` Marc-André Lureau
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 2/3] socket: add listen feature marcandre.lureau
2016-06-23 9:35 ` Daniel P. Berrange
2016-06-16 19:28 ` [Qemu-devel] [PATCH v2 3/3] socket: unlink unix socket on remove marcandre.lureau
2016-06-23 4:41 ` Michael S. Tsirkin
2016-06-23 4:51 ` Michael S. Tsirkin
2016-06-23 9:08 ` Marc-André Lureau
2016-06-23 17:01 ` Michael S. Tsirkin
2016-06-24 12:08 ` Marc-André Lureau
2016-06-24 12:13 ` Daniel P. Berrange
2016-06-24 22:07 ` Michael S. Tsirkin
2016-06-27 8:10 ` Daniel P. Berrange
2016-06-23 9:36 ` Daniel P. Berrange
2016-06-17 12:34 ` [Qemu-devel] [PATCH v2 0/3] chardev cleanups Paolo Bonzini
2016-06-23 9:36 ` Daniel P. Berrange
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.